Re: [sos-linux-ext-patches] [RFC 05/14] x86/apic: Initialize APIC ID for Secure AVIC

[Neeraj Upadhyay <Neeraj.Upadhyay@amd.com>]

>>  static void init_backing_page(void *backing_page)
>>  {
>> +	u32 hv_apic_id;
>> +	u32 apic_id;
>>  	u32 val;
>>  	int i;
>>  
>> @@ -220,6 +223,13 @@ static void init_backing_page(void *backing_page)
>>  
>>  	val = read_msr_from_hv(APIC_LDR);
>>  	set_reg(backing_page, APIC_LDR, val);
>> +
>> +	/* Read APIC ID from Extended Topology Enumeration CPUID */
>> +	apic_id = cpuid_edx(0x0000000b);
>> +	hv_apic_id = read_msr_from_hv(APIC_ID);
>> +	WARN_ONCE(hv_apic_id != apic_id, "Inconsistent APIC_ID values: %d (cpuid), %d (msr)",
>> +			apic_id, hv_apic_id);
>> +	set_reg(backing_page, APIC_ID, apic_id);
>>  }
>>  
> With this warning that hv_apic_id and apic_id  is different, do you still want to set_reg after that? If so, wonder why we have this warning?
> 

"apic_id" as read from cpuid is the source of truth for guest and is the one
guest would be  using for its interrupt/IPI flow.

Guest IPI flow does below:

1. Source vCPU updates the IRR bit in the destination vCPU's backing page.
2. Source vCPU takes an Automatic Exit to hv by doing ICR wrmsr operation.
   The destination APIC ID in ICR write data contains "apic_id".
3. Hv uses "apic_id" to either kick the corresponding destination vCPU (
   if not running) or write to AVIC doorbell to notify the running
   destination vCPU about the new interrupt.

Given that in step 3, hv uses "apic_id" (provided by guest) to find the
corresponding vCPU information, "apic_id" and "hv_apic_id" need to match.
Mismatch is not considered as a fatal event for guest (snp_abort() is not
triggered) and a warning is raise, as even if hv fails to kick or notify
the target vCPU, the IPI (though delayed) will get handled the next time
target vCPU vmrun happens.

I will include this information in commit message and change WARN_ONCE() to
pr_warn() (while at it, will change the format specifiers from %d to %u).


- Neeraj