Re: [PATCH 1/3] x86/cpuid: Disentangle logic for new feature leaves

[Andrew Cooper <Andrew.Cooper3@xxxxxxxxxx>]

On 27/01/2022 16:09, Andrew Cooper wrote:
> Adding a new feature leaf is a reasonable amount of boilerplate and for the
> patch to build, at least one feature from the new leaf needs defining.  This
> typically causes two non-trivial changes to be merged together.
>
> First, have gen-cpuid.py write out some extra placeholder defines:
>
>   #define CPUID_BITFIELD_11 bool :1, :1, lfence_dispatch:1, ...
>   #define CPUID_BITFIELD_12 uint32_t :32 /* placeholder */
>   #define CPUID_BITFIELD_13 uint32_t :32 /* placeholder */
>   #define CPUID_BITFIELD_14 uint32_t :32 /* placeholder */
>   #define CPUID_BITFIELD_15 uint32_t :32 /* placeholder */
>
> This allows DECL_BITFIELD() to be added to struct cpuid_policy without
> requiring a XEN_CPUFEATURE() declared for the leaf.  The choice of 4 is
> arbitrary, and allows us to add more than one leaf at a time if necessary.
>
> Second, rework generic_identify() to not use feature leaf names.

This should say "not use specific feature names."

Fixed locally.

~Andrew

> The choice of deriving the index from a feature was to avoid mismatches, but
> its correctness depends on bugs like c/s 249e0f1d8f20 ("x86/cpuid: Fix
> TSXLDTRK definition") not happening.
>
> Switch to using FEATURESET_* just like the policy/featureset helpers.  This
> breaks the cognitive complexity of needing to know which leaf a specifically
> named feature should reside in, and is shorter to write.  It is also far
> easier to identify as correct at a glance, given the correlation with the
> CPUID leaf being read.
>
> In addition, tidy up some other bits of generic_identify()
>  * Drop leading zeros from leaf numbers.
>  * Don't use a locked update for X86_FEATURE_APERFMPERF.
>  * Rework extended_cpuid_level calculation to avoid setting it twice.
>  * Use "leaf >= $N" consistently so $N matches with the CPUID input.
>
> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>