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:
> We don't actually need ecx yet, but adding it in now will reduce the amount to
> which leaf 7 is out of order in a featureset.
> 
> cpufeatureset.h remains in leaf architectrual order for the sanity of anyone
> trying to locate where to insert new rows.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>

Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
albeit ...

> --- 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 */
> +

... I'm not convinced getting these ordered by other than their word indexes
is quite reasonable. We can't really predict in what order elements / leaves
get populated, as can best be seen from ...

>  /* Intel-defined CPU features, CPUID level 0x00000007:2.edx, word 13 */
>  XEN_CPUFEATURE(INTEL_PSFD,         13*32+ 0) /*A  MSR_SPEC_CTRL.PSFD */
>  XEN_CPUFEATURE(IPRED_CTRL,         13*32+ 1) /*   MSR_SPEC_CTRL.IPRED_DIS_* 
> */

... subleaf 2 already having one entry, and that one not being eax, ebx,
nor ecx, but edx. AMD (extended) leaves would also always be sprinkled into
the middle of any such sequence. To me it ends up more confusing (perhaps
not right away, but in a couple of years time) if one needs to go hunt for
what the next free index value would be. Similarly the need to re-base stuff
using non-upstream index values (like is the case for the KeyLocker leaves
that I have pending locally) is easier to notice when new sets are put at
the end of the existing list.

In any event, what I'd like to ask for as a minimum is that you insert a
blank line between the two new comments.

Jan