On 29/02/2024 12:47 pm, Jan Beulich wrote: > 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.
That comment is going in the next patch irrespective. But yes - this will change leaf 0xb from being host-conditionally-visible to always hidden. PV guests don't have any coherent idea of topology. Linux (with the topo fixes) now explicitly ignores everything it can see and just fakes up a flat non-SMT topology in a single package. For PV dom0, where it sees the host MADT, the values in leaf 0xb are bogus as they don't come from the same enumeration space. >> 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 <[email protected]> >> --- >> CC: Jan Beulich <[email protected]> >> CC: Roger Pau Monné <[email protected]> >> CC: Wei Liu <[email protected]> >> >> 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. 4.14 was when we got the proper default vs max split. Before then, this block of logic was an opencoded "max(ish) for tookstacks which know about it" kind of thing. ~Andrew
