Re: [Xen-devel] [PATCH] x86/apic: Improve current_local_apic_mode()

[Jan Beulich <jbeulich@suse.com>]

On 14.02.2020 18:59, Andrew Cooper wrote:
> On 28/01/2020 14:10, Jan Beulich wrote:
>> On 28.01.2020 13:52, Andrew Cooper wrote:
>>> boot_cpu_has(X86_FEATURE_X2APIC) doesn't need checking to interpret
>>> APIC_BASE_EXTD.
>> Hmm, the comment you remove ...
>>
>>> --- a/xen/arch/x86/apic.c
>>> +++ b/xen/arch/x86/apic.c
>>> @@ -1534,18 +1534,14 @@ void __init record_boot_APIC_mode(void)
>>>  /* Look at the bits in MSR_APIC_BASE and work out which APIC mode we are in */
>>>  enum apic_mode current_local_apic_mode(void)
>>>  {
>>> -    u64 msr_contents;
>>> +    uint32_t high, low;
>>>  
>>> -    rdmsrl(MSR_APIC_BASE, msr_contents);
>>> +    rdmsr(MSR_APIC_BASE, low, high);
>>>  
>>> -    /* Reading EXTD bit from the MSR is only valid if CPUID
>>> -     * says so, else reserved */
>> ... states the situation correctly, I think. I guess there's no hardware
>> allowing the bit to be set without the feature being there, but a virtual
>> or emulated environment could go and set the bit without violating any
>> specification, as long as the CPUID bit is clear.
> 
> It is unrealistic to expect that some emulated environment supports
> preserving of a reserved bit when real hardware uses #GP.

However unrealistic it may be, don't you agree it to be best in
questionable cases if we stay as closely to the spec as possible?
Also I intentionally didn't talk about the bit being preserved,
but the bit perhaps be uniformly set. Simplistic x2APIC
virtualization could always set this bit (without violating the
spec), relying on consumers to actually inspect the CPUID bit
first. One less conditional wherever the value of the MSR gets
calculated.

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel