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
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
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
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
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
© 2016 - 2024 Red Hat, Inc.