Re: [PATCH] x86/Intel: use CPUID bit to determine PPIN availability

[Andrew Cooper <amc96@xxxxxxxx>]

On 17/01/2022 15:30, Jan Beulich wrote:

> As of SDM revision 076 there is a CPUID bit for this functionality. Use
> it to amend the existing model-based logic.
>
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> ---
> In xen-cpuid.c I wasn't sure whether it's better to put the 7b1 table
> next to the 7a1 one, or whether tables should continue to be placed by
> feature set ABI identifier.
>     
    
    They're in FEATURESET_* order, which is also chronological order. 
    str_7b1 wants to be after str_e21a.
    
> --- a/tools/misc/xen-cpuid.c
> +++ b/tools/misc/xen-cpuid.c
> @@ -156,7 +156,7 @@ static const char *const str_e8b[32] =
>      [18] = "ibrs-fast",        [19] = "ibrs-same-mode",
>  
>      [20] = "no-lmsl",
> -    /* [22] */                 [23] = "ppin",
> +    /* [22] */                 [23] = "amd-ppin",
>     
    
    We don't retrofit names like this.  If we did, loads of the
    speculation bits would need to change.
    
    The Intel vs AMD split is clear from the leaf index, and the only
    reason we have the distinction is to fit the two bits into the same
    namespace.
    
    Please leave this as was, to match its name in the source code.
    
> --- a/xen/arch/x86/cpu/mcheck/mce_intel.c
> +++ b/xen/arch/x86/cpu/mcheck/mce_intel.c
> @@ -865,6 +865,13 @@ static void intel_init_ppin(const struct
>      {
>          uint64_t val;
>  
> +    default:
>     
    
    Considering the comment above this switch statement, which you
    haven't edited at all, this wants a note saying that a CPUID bit was
    added in Sapphire Rapids, but older CPUs still require
    model-specific enumeration.
    
    ~Andrew