Re: [PATCH] x86/cpu-policy: Fix x2APIC visibility for PV guests

[Jan Beulich <jbeulich@suse.com>]

On 29.02.2024 11:43, Andrew Cooper wrote:
> Right now, the host x2APIC setting filters into the PV max and default
> policies, yet PV guests cannot set MSR_APIC_BASE.EXTD or access any of the
> x2APIC MSR range.  Therefore they absolutely shouldn't see the x2APIC bit.
> 
> Linux has workarounds for the collateral damage caused by this leakage; it
> unconditionally filters out the x2APIC CPUID bit, and EXTD when reading
> MSR_APIC_BASE.
> 
> Hide the x2APIC bit in the PV default policy, but for compatibility, tolerate
> incoming VMs which already saw the bit.  This is logic from before the
> default/max split in Xen 4.14 which wasn't correctly adjusted at the time.

What about guest_cpuid()'s handling of leaf 0xb then? The %edx value
will change once a guest is rebooted, aiui. The comment in
recalculate_cpuid_policy() that you update refers to that.

> Update the annotation from !A to !S which slightly better describes that it
> doesn't really exist in PV guests.  HVM guests, for which x2APIC can be
> emulated completely, already has it unconditionally set in the max policy.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> ---
> CC: Jan Beulich <JBeulich@suse.com>
> CC: Roger Pau Monné <roger.pau@citrix.com>
> CC: Wei Liu <wl@xen.org>
> 
> This wants backporting as far as people can tollerate, but it's really not
> obvious which commit in 4.14 should be referenced in a Fixes: tag.

Why 4.14? In 4.7.0 I see ...

> @@ -830,11 +846,10 @@ void recalculate_cpuid_policy(struct domain *d)
>      }
>  
>      /*
> -     * Allow the toolstack to set HTT, X2APIC and CMP_LEGACY.  These bits
> +     * Allow the toolstack to set HTT and CMP_LEGACY.  These bits
>       * affect how to interpret topology information in other cpuid leaves.
>       */
>      __set_bit(X86_FEATURE_HTT, max_fs);
> -    __set_bit(X86_FEATURE_X2APIC, max_fs);
>      __set_bit(X86_FEATURE_CMP_LEGACY, max_fs);
>  
>      /*

... these adjustments, just still in calculate_pv_featureset(). I
haven't gone further backwards to check if/when this exposure has
really appeared. I wouldn't be surprised if it's been like that
for all the time since we gained x2APIC support in the hypervisor.

Jan