On 18.10.2023 12:59, Andrew Cooper wrote: > On 17/08/2023 7:22 am, Jan Beulich wrote: >> --- a/xen/lib/x86/cpuid.c >> +++ b/xen/lib/x86/cpuid.c >> @@ -104,6 +104,22 @@ void x86_cpu_featureset_to_policy( >> p->feat._7d1 = fs[FEATURESET_7d1]; >> p->arch_caps.lo = fs[FEATURESET_m10Al]; >> p->arch_caps.hi = fs[FEATURESET_m10Ah]; >> + >> + /* >> + * We may force-enable certain features, which then needs reflecting in >> + * respective max leaf / subleaf values. >> + * >> + * ARCH_CAPS lives in 7d0. >> + */ >> + if ( p->feat._7d0 && p->basic.max_leaf < 7 ) >> + p->basic.max_leaf = 7; >> + >> + /* >> + * AMD's speculation related features (e.g. LFENCE_DISPATCH) live in >> + * leaf e21a. >> + */ >> + if ( p->extd.e21a && p->extd.max_leaf < 0x80000021 ) >> + p->extd.max_leaf = 0x80000021; > > This logic cannot live here - this function is a simple deserialisation > of an array. > > Such logic belongs in create compatible policy, the patch for which has > been pending even longer.
What's that patch's status? Also the extended leaf bumping previously lived in calculate_host_policy() alone, which isn't anywhere near "create compatible policy" either. > The toolstack does need to take when extending like this, and it is not > safe to do it automatically like this. Feels like there's a word missing in the sentence, so it leaves some guessing room. Question is - if it can't be done automatically (see also the RFC: remark in the patch), how do we achieve a consistent resulting policy? Plus I'm not sure we can distinguish the tool stack requesting certain max leaf values merely because of finding them this way, from a lower value being the result of a guest config setting. Jan
