On Fri, Mar 01, 2024 at 11:28:29AM +0000, Andrew Cooper wrote: > The block in recalculate_cpuid_policy() predates the proper split between > default and max policies, and was a "slightly max for a toolstack which knows > about it" capability. It didn't get transformed properly in Xen 4.14. > > Because Xen will accept a VM with HTT/CMP_LEGACY seen, they should be visible > in the max polices. Keep the default policy matching host settings. > > This manifested as an incorrectly-rejected migration across XenServer's Xen > 4.13 -> 4.17 upgrade, as Xapi is slowly growing the logic to check a VM > against the target max policy. > > Signed-off-by: Andrew Cooper <[email protected]>
Reviewed-by: Roger Pau Monné <[email protected]> I have one question below. > --- > CC: Jan Beulich <[email protected]> > CC: Roger Pau Monné <[email protected]> > CC: Wei Liu <[email protected]> > --- > xen/arch/x86/cpu-policy.c | 29 ++++++++++++++++++++++------- > 1 file changed, 22 insertions(+), 7 deletions(-) > > diff --git a/xen/arch/x86/cpu-policy.c b/xen/arch/x86/cpu-policy.c > index c9b32bc17849..4f558e502e01 100644 > --- a/xen/arch/x86/cpu-policy.c > +++ b/xen/arch/x86/cpu-policy.c > @@ -464,6 +464,16 @@ static void __init > guest_common_max_feature_adjustments(uint32_t *fs) > raw_cpu_policy.feat.clwb ) > __set_bit(X86_FEATURE_CLWB, fs); > } > + > + /* > + * Topology information inside the guest is entirely at the toolstack's > + * disgression, and bears no relationship to the host we're running on. > + * > + * HTT identifies p->basic.lppp as valid > + * CMP_LEGACY identifies p->extd.nc as valid > + */ > + __set_bit(X86_FEATURE_HTT, fs); > + __set_bit(X86_FEATURE_CMP_LEGACY, fs); > } > > static void __init guest_common_default_feature_adjustments(uint32_t *fs) > @@ -514,6 +524,18 @@ static void __init > guest_common_default_feature_adjustments(uint32_t *fs) > __clear_bit(X86_FEATURE_CLWB, fs); > } > > + /* > + * Topology information is at the toolstack's discretion so these are > + * unconditionally set in max, but pick a default which matches the host. > + */ > + __clear_bit(X86_FEATURE_HTT, fs); > + if ( cpu_has_htt ) > + __set_bit(X86_FEATURE_HTT, fs); > + > + __clear_bit(X86_FEATURE_CMP_LEGACY, fs); > + if ( cpu_has_cmp_legacy ) > + __set_bit(X86_FEATURE_CMP_LEGACY, fs); Not that I oppose to it, but does it make sense to expose this options to PV guests by default? Those guests don't really have an APIC ID, and I think we don't expect any of the topology information to be usable by them in the first place. Thanks, Roger.
