On 03/03/2023 7:23 am, Jan Beulich wrote: > On 04.01.2023 12:11, Andrew Cooper wrote: >> --- a/xen/include/public/arch-x86/cpufeatureset.h >> +++ b/xen/include/public/arch-x86/cpufeatureset.h >> @@ -288,6 +288,9 @@ XEN_CPUFEATURE(NSCB, 11*32+ 6) /*A Null >> Selector Clears Base (and >> /* Intel-defined CPU features, CPUID level 0x00000007:1.ebx, word 12 */ >> XEN_CPUFEATURE(INTEL_PPIN, 12*32+ 0) /* Protected Processor >> Inventory Number */ >> >> +/* Intel-defined CPU features, CPUID level 0x00000007:1.ecx, word 14 */ >> +/* Intel-defined CPU features, CPUID level 0x00000007:1.edx, word 15 */ > While committing the backports of this (where I normally test-build > every commit individually) I came to notice that this introduces a > transient (until the next commit) build breakage: FEATURESET_NR_ENTRIES > is calculated from the highest entry found; the comments here don't > matter at all. Therefore ... > >> @@ -343,6 +352,8 @@ static inline void cpuid_policy_to_featureset( >> fs[FEATURESET_e21a] = p->extd.e21a; >> fs[FEATURESET_7b1] = p->feat._7b1; >> fs[FEATURESET_7d2] = p->feat._7d2; >> + fs[FEATURESET_7c1] = p->feat._7c1; >> + fs[FEATURESET_7d1] = p->feat._7d1; >> } >> >> /* Fill in a CPUID policy from a featureset bitmap. */ >> @@ -363,6 +374,8 @@ static inline void cpuid_featureset_to_policy( >> p->extd.e21a = fs[FEATURESET_e21a]; >> p->feat._7b1 = fs[FEATURESET_7b1]; >> p->feat._7d2 = fs[FEATURESET_7d2]; >> + p->feat._7c1 = fs[FEATURESET_7c1]; >> + p->feat._7d1 = fs[FEATURESET_7d1]; >> } > ... the compiler legitimately complains about out-of-bounds array > accesses here. This is just fyi for the future (to arrange patch > splitting differently); I've left the backports as they were.
Hmm. c/s e3662437eb43 was designed to specifically allow CPUID patches to be split like this. Which compiler? I think I agree with your analysis, but I've never seen a complaint, hence not noticing. I suspect we actually want FEATURESET_NR_ENTRIES defined in C, next to the FEATURESET_* defines, and we want to BUILD_BUG_ON() if the autogen length is larger than the C length. ~Andrew
