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

[Jan Beulich <jbeulich@xxxxxxxx>]

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. (Afaict we still allow
PV guests to see the host MSR_APIC_BASE contents, yet such guests
wouldn't see the CPUID flag set. We've had a customer inferring from the
set bit in the MSR that the other x2APIC [host] MSRs can also be read.
They wouldn't have run into their issue if they had followed its
"reserved" semantics in such a case.) I guess I'm missing some further
aspect here.

Jan

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