[Xen-devel] [PATCH 0/2] x86/mce: PPIN adjustments coming from Linux

Jan Beulich posted 2 patches 4 years ago
Only 0 patches received!
[Xen-devel] [PATCH 0/2] x86/mce: PPIN adjustments coming from Linux
Posted by Jan Beulich 4 years ago
1: add Xeon Icelake to list of CPUs that support PPIN
2: fix logic and comments around MSR_PPIN_CTL

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH 1/2] x86/mce: add Xeon Icelake to list of CPUs that support PPIN
Posted by Jan Beulich 4 years ago
From: Tony Luck <tony.luck@intel.com>

New CPU model, same MSRs to control and read the inventory number.

Signed-off-by: Tony Luck <tony.luck@intel.com>
[Linux commit dc6b025de95bcd22ff37c4fabb022ec8a027abf1]
Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/arch/x86/cpu/mcheck/mce_intel.c
+++ b/xen/arch/x86/cpu/mcheck/mce_intel.c
@@ -871,6 +871,7 @@ static void intel_init_ppin(const struct
     case 0x55: /* Skylake X */
     case 0x56: /* Broadwell Xeon D */
     case 0x57: /* Knights Landing */
+    case 0x6a: /* Icelake X */
     case 0x85: /* Knights Mill */
 
         if ( (c != &boot_cpu_data && !ppin_msr) ||


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 1/2] x86/mce: add Xeon Icelake to list of CPUs that support PPIN
Posted by Andrew Cooper 4 years ago
On 02/03/2020 14:06, Jan Beulich wrote:
> From: Tony Luck <tony.luck@intel.com>
>
> New CPU model, same MSRs to control and read the inventory number.
>
> Signed-off-by: Tony Luck <tony.luck@intel.com>
> [Linux commit dc6b025de95bcd22ff37c4fabb022ec8a027abf1]
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH 2/2] x86/mce: fix logic and comments around MSR_PPIN_CTL
Posted by Jan Beulich 4 years ago
From: Tony Luck <tony.luck@intel.com>

There are two implemented bits in the PPIN_CTL MSR:

Bit0: LockOut (R/WO)
      Set 1 to prevent further writes to MSR_PPIN_CTL.

Bit 1: Enable_PPIN (R/W)
       If 1, enables MSR_PPIN to be accessible using RDMSR.
       If 0, an attempt to read MSR_PPIN will cause #GP.

So there are four defined values:
	0: PPIN is disabled, PPIN_CTL may be updated
	1: PPIN is disabled. PPIN_CTL is locked against updates
	2: PPIN is enabled. PPIN_CTL may be updated
	3: PPIN is enabled. PPIN_CTL is locked against updates

Code would only enable the X86_FEATURE_INTEL_PPIN feature for case "2".
When it should have done so for both case "2" and case "3".

Fix the final test to just check for the enable bit.
Also fix some of the other comments in this function.

Signed-off-by: Tony Luck <tony.luck@intel.com>
[Linux commit ???]

One of the adjusted comments doesn't exist in our code, and I disagree
with the adjustment to the other one and its associate code change: I
don't think there's a point trying to enable PPIN if the locked bit is
set. Hence it's just the main code change that gets pulled in, plus it
gets cloned to the AMD side.

Requested-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
Note: If available by the time of committing I mean to insert the Linux
      commit hash.

--- a/xen/arch/x86/cpu/mcheck/mce_intel.c
+++ b/xen/arch/x86/cpu/mcheck/mce_intel.c
@@ -885,7 +885,7 @@ static void intel_init_ppin(const struct
             rdmsr_safe(MSR_PPIN_CTL, val);
         }
 
-        if ( (val & (PPIN_ENABLE | PPIN_LOCKOUT)) != PPIN_ENABLE )
+        if ( !(val & PPIN_ENABLE) )
             ppin_msr = 0;
         else if ( c == &boot_cpu_data )
             ppin_msr = MSR_PPIN;
--- a/xen/arch/x86/cpu/mcheck/mce_amd.c
+++ b/xen/arch/x86/cpu/mcheck/mce_amd.c
@@ -329,7 +329,7 @@ amd_mcheck_init(struct cpuinfo_x86 *ci)
             rdmsrl(MSR_AMD_PPIN_CTL, val);
         }
 
-        if ( (val & (PPIN_ENABLE | PPIN_LOCKOUT)) != PPIN_ENABLE )
+        if ( !(val & PPIN_ENABLE) )
             ppin_msr = 0;
         else if ( ci == &boot_cpu_data )
             ppin_msr = MSR_AMD_PPIN;


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 2/2] x86/mce: fix logic and comments around MSR_PPIN_CTL
Posted by Andrew Cooper 4 years ago
On 02/03/2020 14:07, Jan Beulich wrote:
> From: Tony Luck <tony.luck@intel.com>
>
> There are two implemented bits in the PPIN_CTL MSR:
>
> Bit0: LockOut (R/WO)
>       Set 1 to prevent further writes to MSR_PPIN_CTL.
>
> Bit 1: Enable_PPIN (R/W)
>        If 1, enables MSR_PPIN to be accessible using RDMSR.
>        If 0, an attempt to read MSR_PPIN will cause #GP.
>
> So there are four defined values:
> 	0: PPIN is disabled, PPIN_CTL may be updated
> 	1: PPIN is disabled. PPIN_CTL is locked against updates
> 	2: PPIN is enabled. PPIN_CTL may be updated
> 	3: PPIN is enabled. PPIN_CTL is locked against updates
>
> Code would only enable the X86_FEATURE_INTEL_PPIN feature for case "2".
> When it should have done so for both case "2" and case "3".
>
> Fix the final test to just check for the enable bit.
> Also fix some of the other comments in this function.
>
> Signed-off-by: Tony Luck <tony.luck@intel.com>
> [Linux commit ???]
>
> One of the adjusted comments doesn't exist in our code, and I disagree
> with the adjustment to the other one and its associate code change: I
> don't think there's a point trying to enable PPIN if the locked bit is
> set. Hence it's just the main code change that gets pulled in, plus it
> gets cloned to the AMD side.
>
> Requested-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

I agree.  If it is locked, there is no point trying to change it.

Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel