This is as per Linux commit a331f5fdd36d ("x86/mce: Add Xeon Sapphire
Rapids to list of CPUs that support PPIN") just in case a subsequent
change making use of the respective new CPUID bit doesn't cover this
model.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
It is unclear to me whether this change is actually made obsolete by the
subsequent one adding support for the respective new CPUID bit.

It also continues to be unclear for which CPU models, if any, the
PPIN_CAP bit in PLATFORM_INFO could be used in favor of a model check.
---
v2: New.

--- a/xen/arch/x86/cpu/mcheck/mce_intel.c
+++ b/xen/arch/x86/cpu/mcheck/mce_intel.c
@@ -873,6 +873,7 @@ static void intel_init_ppin(const struct
     case 0x57: /* Knights Landing */
     case 0x6a: /* Icelake X */
     case 0x85: /* Knights Mill */
+    case 0x8f: /* Sapphire Rapids X */
 
         if ( (c != &boot_cpu_data && !ppin_msr) ||
              rdmsr_safe(MSR_PPIN_CTL, val) )

On 20/01/2022 14:16, Jan Beulich wrote:
> This is as per Linux commit a331f5fdd36d ("x86/mce: Add Xeon Sapphire
> Rapids to list of CPUs that support PPIN") just in case a subsequent
> change making use of the respective new CPUID bit doesn't cover this
> model.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Sadly,
https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git/commit/?h=x86/urgent&id=e464121f2d40eabc7d11823fb26db807ce945df4


IceLake-D too.

Preferably with this fixed, Acked-by: Andrew Cooper
<andrew.cooper3@citrix.com> (to save a trivial repost), but ...

> ---
> It is unclear to me whether this change is actually made obsolete by the
> subsequent one adding support for the respective new CPUID bit.

... Sapphire Rapids doesn't enumerate PPIN.  Hopefully Granite Rapids
will, but everything SPR and older will have to rely on model checks only.

Probably best to drop the second half of the commit message to remove
the uncertainty.

> It also continues to be unclear for which CPU models, if any, the
> PPIN_CAP bit in PLATFORM_INFO could be used in favor of a model check.

Presumably none, because you need the same set of model checks to
interpret the PPIN bit in PLATFORM_INFO.  It does beg the question what
the point of the bit is...

~Andrew

On 27.01.2022 00:01, Andrew Cooper wrote:
> On 20/01/2022 14:16, Jan Beulich wrote:
>> This is as per Linux commit a331f5fdd36d ("x86/mce: Add Xeon Sapphire
>> Rapids to list of CPUs that support PPIN") just in case a subsequent
>> change making use of the respective new CPUID bit doesn't cover this
>> model.
>>
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> 
> Sadly,
> https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git/commit/?h=x86/urgent&id=e464121f2d40eabc7d11823fb26db807ce945df4
> 
> 
> IceLake-D too.
> 
> Preferably with this fixed, Acked-by: Andrew Cooper
> <andrew.cooper3@citrix.com> (to save a trivial repost),

Sure, added. And thanks.

> but ...
> 
>> ---
>> It is unclear to me whether this change is actually made obsolete by the
>> subsequent one adding support for the respective new CPUID bit.
> 
> ... Sapphire Rapids doesn't enumerate PPIN.  Hopefully Granite Rapids
> will, but everything SPR and older will have to rely on model checks only.

At least in theory I suppose they could address this by as simple as
a microcode update?

>> It also continues to be unclear for which CPU models, if any, the
>> PPIN_CAP bit in PLATFORM_INFO could be used in favor of a model check.
> 
> Presumably none, because you need the same set of model checks to
> interpret the PPIN bit in PLATFORM_INFO.  It does beg the question what
> the point of the bit is...

Well, if the bit never had a different meaning, then a model check
wouldn't be necessary. Just like e.g. probe_cpuid_faulting() doesn't
have one.

Jan