Re: [PATCH 1/2] x86/cpuid: Infrastructure for leaves 7:1{ecx,edx}

[Jan Beulich <jbeulich@xxxxxxxx>]

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.

Jan