On 03.03.2023 19:32, Andrew Cooper wrote: > 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.
gcc 12 > 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. Hmm, yes, this may be the best we can do. Jan
