* [PATCH 0/5] PPIN (Protected Processor Inventory Number) updates @ 2022-01-07 22:54 Tony Luck 2022-01-07 22:54 ` [PATCH 1/5] x86/ras: Merge Intel and AMD ppin_init() functions Tony Luck ` (5 more replies) 0 siblings, 6 replies; 42+ messages in thread From: Tony Luck @ 2022-01-07 22:54 UTC (permalink / raw) To: Borislav Petkov Cc: x86, linux-kernel, Greg Kroah-Hartman, Smita Koralahalli Channabasappa, Wei Huang, Tom Lendacky, patches, Tony Luck This series starts out with two changes that I expect are uncontroversial. Later parts get progressively more "RFC". 1) Simple cleanup to merge Intel and AMD duplicated code to test for presence of PPIN and check whether it is enabled. 2) Long overdue update from Intel to enumerate the PPIN and PPIN_CTL MSRs. See the December 2021 Software Developers Manual {RFC factor moves to medium here} 3) Code to scan machine check banks re-reads the PPIN every time banks are scanned (whether for a machine check, a CMCI, or just a periodic poll). Since PPIN never changes, this seems like unnecessary overhead. Read the MSR once (per CPU) and save to memory. {RFC factor moves to high for last two parts} 4) Refactor as prep for last part. 5) Add "ppin" to /sys/devices/system/cpu/cpu*/topology/ppin The big question for this part is whether there is a better place to expose this value. I'm open to other suggestions. I do think it is useful to do so. An "inventory" number that stays hidden until there is an error that causes it to show up in a machine check log is user hostile. Tony Luck (5): x86/ras: Merge Intel and AMD ppin_init() functions x86/ras: X86_FEATURE_INTEL_PPIN finally has a CPUID bit x86/ras: Read/save PPIN MSR during initialization x86/sysfs: Add format parameter to macro defining "show" functions for proc x86/sysfs: Add PPIN in sysfs under cpu topology .../ABI/stable/sysfs-devices-system-cpu | 4 + .../ABI/testing/sysfs-devices-system-cpu | 6 ++ arch/x86/include/asm/processor.h | 2 + arch/x86/include/asm/topology.h | 1 + arch/x86/kernel/cpu/amd.c | 30 ------- arch/x86/kernel/cpu/common.c | 78 +++++++++++++++++++ arch/x86/kernel/cpu/mce/core.c | 7 +- arch/x86/kernel/cpu/mce/intel.c | 41 ---------- arch/x86/kernel/cpu/scattered.c | 1 + drivers/base/topology.c | 20 +++-- include/linux/topology.h | 3 + 11 files changed, 108 insertions(+), 85 deletions(-) base-commit: c9e6606c7fe92b50a02ce51dda82586ebdf99b48 -- 2.31.1 ^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH 1/5] x86/ras: Merge Intel and AMD ppin_init() functions 2022-01-07 22:54 [PATCH 0/5] PPIN (Protected Processor Inventory Number) updates Tony Luck @ 2022-01-07 22:54 ` Tony Luck 2022-01-18 20:02 ` Borislav Petkov 2022-01-07 22:54 ` [PATCH 2/5] x86/ras: X86_FEATURE_INTEL_PPIN finally has a CPUID bit Tony Luck ` (4 subsequent siblings) 5 siblings, 1 reply; 42+ messages in thread From: Tony Luck @ 2022-01-07 22:54 UTC (permalink / raw) To: Borislav Petkov Cc: x86, linux-kernel, Greg Kroah-Hartman, Smita Koralahalli Channabasappa, Wei Huang, Tom Lendacky, patches, Tony Luck The code to decide whether a system supports the PPIN (Protected Processor Inventory Number) MSR was cloned from the Intel implementation. Apart from the X86_FEATURE bit and the MSR numbers it is identical. Merge the two functions into common x86 code, but use x86_match_cpu() instead of the switch (c->x86_model) that was used by the old Intel code. No functional change. Signed-off-by: Tony Luck <tony.luck@intel.com> --- arch/x86/kernel/cpu/amd.c | 30 ------------- arch/x86/kernel/cpu/common.c | 76 +++++++++++++++++++++++++++++++++ arch/x86/kernel/cpu/mce/intel.c | 41 ------------------ 3 files changed, 76 insertions(+), 71 deletions(-) diff --git a/arch/x86/kernel/cpu/amd.c b/arch/x86/kernel/cpu/amd.c index 4edb6f0f628c..bad0fa4c1779 100644 --- a/arch/x86/kernel/cpu/amd.c +++ b/arch/x86/kernel/cpu/amd.c @@ -394,35 +394,6 @@ static void amd_detect_cmp(struct cpuinfo_x86 *c) per_cpu(cpu_llc_id, cpu) = c->cpu_die_id = c->phys_proc_id; } -static void amd_detect_ppin(struct cpuinfo_x86 *c) -{ - unsigned long long val; - - if (!cpu_has(c, X86_FEATURE_AMD_PPIN)) - return; - - /* When PPIN is defined in CPUID, still need to check PPIN_CTL MSR */ - if (rdmsrl_safe(MSR_AMD_PPIN_CTL, &val)) - goto clear_ppin; - - /* PPIN is locked in disabled mode, clear feature bit */ - if ((val & 3UL) == 1UL) - goto clear_ppin; - - /* If PPIN is disabled, try to enable it */ - if (!(val & 2UL)) { - wrmsrl_safe(MSR_AMD_PPIN_CTL, val | 2UL); - rdmsrl_safe(MSR_AMD_PPIN_CTL, &val); - } - - /* If PPIN_EN bit is 1, return from here; otherwise fall through */ - if (val & 2UL) - return; - -clear_ppin: - clear_cpu_cap(c, X86_FEATURE_AMD_PPIN); -} - u32 amd_get_nodes_per_socket(void) { return nodes_per_socket; @@ -947,7 +918,6 @@ static void init_amd(struct cpuinfo_x86 *c) amd_detect_cmp(c); amd_get_topology(c); srat_detect_node(c); - amd_detect_ppin(c); init_amd_cacheinfo(c); diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c index 0083464de5e3..a1e29c0844d1 100644 --- a/arch/x86/kernel/cpu/common.c +++ b/arch/x86/kernel/cpu/common.c @@ -88,6 +88,80 @@ EXPORT_SYMBOL_GPL(get_llc_id); /* L2 cache ID of each logical CPU */ DEFINE_PER_CPU_READ_MOSTLY(u16, cpu_l2c_id) = BAD_APICID; +static struct ppin_info { + int feature; + int msr_ppin_ctl; + int msr_ppin; +} ppin_info[] = { + [X86_VENDOR_INTEL] = { + .feature = X86_FEATURE_INTEL_PPIN, + .msr_ppin_ctl = MSR_PPIN_CTL, + .msr_ppin = MSR_PPIN + }, + [X86_VENDOR_AMD] = { + .feature = X86_FEATURE_AMD_PPIN, + .msr_ppin_ctl = MSR_AMD_PPIN_CTL, + .msr_ppin = MSR_AMD_PPIN + }, +}; + +static const struct x86_cpu_id ppin_cpuids[] = { + X86_MATCH_VENDOR_FEATURE(AMD, X86_FEATURE_AMD_PPIN, &ppin_info[X86_VENDOR_AMD]), + + /* Legacy models without CPUID enumeration */ + X86_MATCH_INTEL_FAM6_MODEL(IVYBRIDGE_X, &ppin_info[X86_VENDOR_INTEL]), + X86_MATCH_INTEL_FAM6_MODEL(HASWELL_X, &ppin_info[X86_VENDOR_INTEL]), + X86_MATCH_INTEL_FAM6_MODEL(BROADWELL_D, &ppin_info[X86_VENDOR_INTEL]), + X86_MATCH_INTEL_FAM6_MODEL(BROADWELL_X, &ppin_info[X86_VENDOR_INTEL]), + X86_MATCH_INTEL_FAM6_MODEL(SKYLAKE_X, &ppin_info[X86_VENDOR_INTEL]), + X86_MATCH_INTEL_FAM6_MODEL(ICELAKE_X, &ppin_info[X86_VENDOR_INTEL]), + X86_MATCH_INTEL_FAM6_MODEL(SAPPHIRERAPIDS_X, &ppin_info[X86_VENDOR_INTEL]), + X86_MATCH_INTEL_FAM6_MODEL(XEON_PHI_KNL, &ppin_info[X86_VENDOR_INTEL]), + X86_MATCH_INTEL_FAM6_MODEL(XEON_PHI_KNM, &ppin_info[X86_VENDOR_INTEL]), + + {} +}; + +static void ppin_init(struct cpuinfo_x86 *c) +{ + const struct x86_cpu_id *id; + unsigned long long val; + struct ppin_info *info; + + id = x86_match_cpu(ppin_cpuids); + if (!id) + return; + + /* + * Testing the prescence of the MSR is not enough. Need to check + * that the PPIN_CTL allows reading of the PPIN. + */ + info = (struct ppin_info *)id->driver_data; + + if (rdmsrl_safe(info->msr_ppin_ctl, &val)) + goto clear_ppin; + + if ((val & 3UL) == 1UL) { + /* PPIN locked in disabled mode */ + goto clear_ppin; + } + + /* If PPIN is disabled, try to enable */ + if (!(val & 2UL)) { + wrmsrl_safe(info->msr_ppin_ctl, val | 2UL); + rdmsrl_safe(info->msr_ppin_ctl, &val); + } + + /* Is the enable bit set? */ + if (val & 2UL) { + set_cpu_cap(c, info->feature); + return; + } + +clear_ppin: + clear_cpu_cap(c, info->feature); +} + /* correctly size the local cpu masks */ void __init setup_cpu_local_masks(void) { @@ -1655,6 +1729,8 @@ static void identify_cpu(struct cpuinfo_x86 *c) c->x86_capability[i] |= boot_cpu_data.x86_capability[i]; } + ppin_init(c); + /* Init Machine Check Exception if available. */ mcheck_cpu_init(c); diff --git a/arch/x86/kernel/cpu/mce/intel.c b/arch/x86/kernel/cpu/mce/intel.c index bb9a46a804bf..95275a5e57e0 100644 --- a/arch/x86/kernel/cpu/mce/intel.c +++ b/arch/x86/kernel/cpu/mce/intel.c @@ -470,46 +470,6 @@ void intel_clear_lmce(void) wrmsrl(MSR_IA32_MCG_EXT_CTL, val); } -static void intel_ppin_init(struct cpuinfo_x86 *c) -{ - unsigned long long val; - - /* - * Even if testing the presence of the MSR would be enough, we don't - * want to risk the situation where other models reuse this MSR for - * other purposes. - */ - switch (c->x86_model) { - case INTEL_FAM6_IVYBRIDGE_X: - case INTEL_FAM6_HASWELL_X: - case INTEL_FAM6_BROADWELL_D: - case INTEL_FAM6_BROADWELL_X: - case INTEL_FAM6_SKYLAKE_X: - case INTEL_FAM6_ICELAKE_X: - case INTEL_FAM6_SAPPHIRERAPIDS_X: - case INTEL_FAM6_XEON_PHI_KNL: - case INTEL_FAM6_XEON_PHI_KNM: - - if (rdmsrl_safe(MSR_PPIN_CTL, &val)) - return; - - if ((val & 3UL) == 1UL) { - /* PPIN locked in disabled mode */ - return; - } - - /* If PPIN is disabled, try to enable */ - if (!(val & 2UL)) { - wrmsrl_safe(MSR_PPIN_CTL, val | 2UL); - rdmsrl_safe(MSR_PPIN_CTL, &val); - } - - /* Is the enable bit set? */ - if (val & 2UL) - set_cpu_cap(c, X86_FEATURE_INTEL_PPIN); - } -} - /* * Enable additional error logs from the integrated * memory controller on processors that support this. @@ -534,7 +494,6 @@ void mce_intel_feature_init(struct cpuinfo_x86 *c) { intel_init_cmci(); intel_init_lmce(); - intel_ppin_init(c); intel_imc_init(c); } -- 2.31.1 ^ permalink raw reply related [flat|nested] 42+ messages in thread
* Re: [PATCH 1/5] x86/ras: Merge Intel and AMD ppin_init() functions 2022-01-07 22:54 ` [PATCH 1/5] x86/ras: Merge Intel and AMD ppin_init() functions Tony Luck @ 2022-01-18 20:02 ` Borislav Petkov 2022-01-18 21:03 ` Luck, Tony 0 siblings, 1 reply; 42+ messages in thread From: Borislav Petkov @ 2022-01-18 20:02 UTC (permalink / raw) To: Tony Luck Cc: x86, linux-kernel, Greg Kroah-Hartman, Smita Koralahalli Channabasappa, Wei Huang, Tom Lendacky, patches On Fri, Jan 07, 2022 at 02:54:38PM -0800, Tony Luck wrote: Make that subject prefix "x86/cpu" please. Same for patches 2 and 3. Patch 4 is probably topology/sysfs - at least this is what past patches say. If you're not sure about the prefix, use git log on the files you're touching. > diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c > index 0083464de5e3..a1e29c0844d1 100644 > --- a/arch/x86/kernel/cpu/common.c > +++ b/arch/x86/kernel/cpu/common.c > @@ -88,6 +88,80 @@ EXPORT_SYMBOL_GPL(get_llc_id); > /* L2 cache ID of each logical CPU */ > DEFINE_PER_CPU_READ_MOSTLY(u16, cpu_l2c_id) = BAD_APICID; > > +static struct ppin_info { > + int feature; > + int msr_ppin_ctl; > + int msr_ppin; That thing is unused in this patch and only accessed in patch 3. Please add it there. > +} ppin_info[] = { > + [X86_VENDOR_INTEL] = { > + .feature = X86_FEATURE_INTEL_PPIN, > + .msr_ppin_ctl = MSR_PPIN_CTL, > + .msr_ppin = MSR_PPIN > + }, > + [X86_VENDOR_AMD] = { X86_VENDOR_AMD is index 2 not 1 so ppin_info[1] is empty. I wouldn't mind swapping the numbers here in a pre-patch: #define X86_VENDOR_CYRIX 1 #define X86_VENDOR_AMD 2 nothing would depend on those naked numbers, right? :) > + .feature = X86_FEATURE_AMD_PPIN, > + .msr_ppin_ctl = MSR_AMD_PPIN_CTL, > + .msr_ppin = MSR_AMD_PPIN > + }, > +}; > + > +static const struct x86_cpu_id ppin_cpuids[] = { > + X86_MATCH_VENDOR_FEATURE(AMD, X86_FEATURE_AMD_PPIN, &ppin_info[X86_VENDOR_AMD]), X86_MATCH_FEATURE() I guess. > + /* Legacy models without CPUID enumeration */ > + X86_MATCH_INTEL_FAM6_MODEL(IVYBRIDGE_X, &ppin_info[X86_VENDOR_INTEL]), > + X86_MATCH_INTEL_FAM6_MODEL(HASWELL_X, &ppin_info[X86_VENDOR_INTEL]), > + X86_MATCH_INTEL_FAM6_MODEL(BROADWELL_D, &ppin_info[X86_VENDOR_INTEL]), > + X86_MATCH_INTEL_FAM6_MODEL(BROADWELL_X, &ppin_info[X86_VENDOR_INTEL]), > + X86_MATCH_INTEL_FAM6_MODEL(SKYLAKE_X, &ppin_info[X86_VENDOR_INTEL]), > + X86_MATCH_INTEL_FAM6_MODEL(ICELAKE_X, &ppin_info[X86_VENDOR_INTEL]), > + X86_MATCH_INTEL_FAM6_MODEL(SAPPHIRERAPIDS_X, &ppin_info[X86_VENDOR_INTEL]), > + X86_MATCH_INTEL_FAM6_MODEL(XEON_PHI_KNL, &ppin_info[X86_VENDOR_INTEL]), > + X86_MATCH_INTEL_FAM6_MODEL(XEON_PHI_KNM, &ppin_info[X86_VENDOR_INTEL]), > + > + {} > +}; > + > +static void ppin_init(struct cpuinfo_x86 *c) > +{ > + const struct x86_cpu_id *id; > + unsigned long long val; > + struct ppin_info *info; > + > + id = x86_match_cpu(ppin_cpuids); > + if (!id) > + return; > + > + /* > + * Testing the prescence of the MSR is not enough. Need to check Unknown word [prescence] in comment. Suggestions: ['presence', 'prescience', 'putrescence', 'prepubescence', 'excrescence', 'concrescence'] > + * that the PPIN_CTL allows reading of the PPIN. > + */ > + info = (struct ppin_info *)id->driver_data; > + > + if (rdmsrl_safe(info->msr_ppin_ctl, &val)) > + goto clear_ppin; > + > + if ((val & 3UL) == 1UL) { > + /* PPIN locked in disabled mode */ > + goto clear_ppin; > + } > + > + /* If PPIN is disabled, try to enable */ > + if (!(val & 2UL)) { > + wrmsrl_safe(info->msr_ppin_ctl, val | 2UL); > + rdmsrl_safe(info->msr_ppin_ctl, &val); > + } > + > + /* Is the enable bit set? */ > + if (val & 2UL) { > + set_cpu_cap(c, info->feature); > + return; > + } > + > +clear_ppin: > + clear_cpu_cap(c, info->feature); > +} > + > /* correctly size the local cpu masks */ > void __init setup_cpu_local_masks(void) > { > @@ -1655,6 +1729,8 @@ static void identify_cpu(struct cpuinfo_x86 *c) > c->x86_capability[i] |= boot_cpu_data.x86_capability[i]; > } > > + ppin_init(c); I can't say that I'm crazy about all those miscellaneous feature-initializing functions sprinkled allround here but I don't have a better idea ... yet. Thx. -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH 1/5] x86/ras: Merge Intel and AMD ppin_init() functions 2022-01-18 20:02 ` Borislav Petkov @ 2022-01-18 21:03 ` Luck, Tony 2022-01-18 21:15 ` Borislav Petkov 0 siblings, 1 reply; 42+ messages in thread From: Luck, Tony @ 2022-01-18 21:03 UTC (permalink / raw) To: Borislav Petkov Cc: x86, linux-kernel, Greg Kroah-Hartman, Smita Koralahalli Channabasappa, Wei Huang, Tom Lendacky, patches On Tue, Jan 18, 2022 at 09:02:41PM +0100, Borislav Petkov wrote: > On Fri, Jan 07, 2022 at 02:54:38PM -0800, Tony Luck wrote: > > +} ppin_info[] = { > > + [X86_VENDOR_INTEL] = { > > + .feature = X86_FEATURE_INTEL_PPIN, > > + .msr_ppin_ctl = MSR_PPIN_CTL, > > + .msr_ppin = MSR_PPIN > > + }, > > + [X86_VENDOR_AMD] = { > > X86_VENDOR_AMD is index 2 not 1 so ppin_info[1] is empty. > > I wouldn't mind swapping the numbers here in a pre-patch: > > #define X86_VENDOR_CYRIX 1 > #define X86_VENDOR_AMD 2 > > nothing would depend on those naked numbers, right? :) You are much braver than I :-) How confident are you that nobody implicitly depends on those? Is it worth the risk/churn just to save 12 bytes in the ppin_info[] array? I'll fix up all the other stuff you found and post a V2 soon. Thanks for the review. -Tony ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH 1/5] x86/ras: Merge Intel and AMD ppin_init() functions 2022-01-18 21:03 ` Luck, Tony @ 2022-01-18 21:15 ` Borislav Petkov 0 siblings, 0 replies; 42+ messages in thread From: Borislav Petkov @ 2022-01-18 21:15 UTC (permalink / raw) To: Luck, Tony Cc: x86, linux-kernel, Greg Kroah-Hartman, Smita Koralahalli Channabasappa, Wei Huang, Tom Lendacky, patches On Tue, Jan 18, 2022 at 01:03:19PM -0800, Luck, Tony wrote: > You are much braver than I :-) How confident are you > that nobody implicitly depends on those? So I'm willing to do it just to piss off all those who do, frankly. Because that's a kernel-internal define and *nothing* should use it... But looking at print_cpu_modalias(), that thing issues uevents and am I willing to guarantee that nothing in luserspace uses those naked numbers? Probably not. ;-\ That's how it looks like here and there's ven0002. /sys/devices/system/cpu/cpu11/uevent:MODALIAS=cpu:type:x86,ven0002fam0017mod0008:feature:,0000,0001,0002,0003,0004,0005,0006, 0007,0008,0009,000B,000C,000D,000E,000F,0010,0011,0013,0017,0018,0019,001A,001C,0020,0021,0022,0023,0024,0025,0026,0027,0028, 0029,002B,002C,002D,002E,002F,0030,0031,0034,0036,0037,0038,0039,003A,003B,003D,0064,0068,006E,0070,0072,0074,0075,0078,0079, 007A,007C,007D,0080,0081,0083,0089,008C,008D,0093,0094,0096,0097,0099,009A,009B,009C,009D,009E,00C0,00C1,00C2,00C3,00C4,00C5, 00C6,00C7,00C8,00C9,00CC,00CD,00D1,00D6,00D7,00D8,00DA,00DC,00DD,00E2,00E8,00EC,00ED,00F1,00F3,00F5,00F8,00FA,00FC,010F,0120, 0123,0125,0127,0128,0132,0133,0134,0137,013D,0140,0141,0142,0143,0165,01A0,01A1,01A2,01AC,01C2,01E0,01E1,01E2,01E3,01E4,01E5, 01E6,01E7,01EA,01EB,01EC,01ED,01EF,01F0,0220,0221,0223,0224,0260,0261,0262,0263 > Is it worth the risk/churn just to save 12 bytes in the ppin_info[] > array? It doesn't look like it. > I'll fix up all the other stuff you found and post a V2 soon. Thanks > for the review. Please hold off until I've looked at the rest. Thx. -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette ^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH 2/5] x86/ras: X86_FEATURE_INTEL_PPIN finally has a CPUID bit 2022-01-07 22:54 [PATCH 0/5] PPIN (Protected Processor Inventory Number) updates Tony Luck 2022-01-07 22:54 ` [PATCH 1/5] x86/ras: Merge Intel and AMD ppin_init() functions Tony Luck @ 2022-01-07 22:54 ` Tony Luck 2022-01-20 13:32 ` Borislav Petkov 2022-01-07 22:54 ` [PATCH 3/5] x86/ras: Read/save PPIN MSR during initialization Tony Luck ` (3 subsequent siblings) 5 siblings, 1 reply; 42+ messages in thread From: Tony Luck @ 2022-01-07 22:54 UTC (permalink / raw) To: Borislav Petkov Cc: x86, linux-kernel, Greg Kroah-Hartman, Smita Koralahalli Channabasappa, Wei Huang, Tom Lendacky, patches, Tony Luck After nine generations of adding to model specific list of CPUS that support PPIN (Protected Processor Inventory Number) Intel allocated a CPUID bit to enumerate the MSRs. CPUID(EAX=7, ECX=1).EBX bit 0 enumerates presence of MSR_PPIN_CTL and MSR_PPIN. Add it to the "scattered" CPUID bits and add an entry to the ppin_cpuids[] x86_match_cpu() array to catch Intel CPUs that implement it. Signed-off-by: Tony Luck <tony.luck@intel.com> --- arch/x86/kernel/cpu/common.c | 1 + arch/x86/kernel/cpu/scattered.c | 1 + 2 files changed, 2 insertions(+) diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c index a1e29c0844d1..3688f70ee0a2 100644 --- a/arch/x86/kernel/cpu/common.c +++ b/arch/x86/kernel/cpu/common.c @@ -107,6 +107,7 @@ static struct ppin_info { static const struct x86_cpu_id ppin_cpuids[] = { X86_MATCH_VENDOR_FEATURE(AMD, X86_FEATURE_AMD_PPIN, &ppin_info[X86_VENDOR_AMD]), + X86_MATCH_VENDOR_FEATURE(INTEL, X86_FEATURE_INTEL_PPIN, &ppin_info[X86_VENDOR_INTEL]), /* Legacy models without CPUID enumeration */ X86_MATCH_INTEL_FAM6_MODEL(IVYBRIDGE_X, &ppin_info[X86_VENDOR_INTEL]), diff --git a/arch/x86/kernel/cpu/scattered.c b/arch/x86/kernel/cpu/scattered.c index 21d1f062895a..4143b1e4c5c6 100644 --- a/arch/x86/kernel/cpu/scattered.c +++ b/arch/x86/kernel/cpu/scattered.c @@ -26,6 +26,7 @@ struct cpuid_bit { static const struct cpuid_bit cpuid_bits[] = { { X86_FEATURE_APERFMPERF, CPUID_ECX, 0, 0x00000006, 0 }, { X86_FEATURE_EPB, CPUID_ECX, 3, 0x00000006, 0 }, + { X86_FEATURE_INTEL_PPIN, CPUID_EBX, 0, 0x00000007, 1 }, { X86_FEATURE_CQM_LLC, CPUID_EDX, 1, 0x0000000f, 0 }, { X86_FEATURE_CQM_OCCUP_LLC, CPUID_EDX, 0, 0x0000000f, 1 }, { X86_FEATURE_CQM_MBM_TOTAL, CPUID_EDX, 1, 0x0000000f, 1 }, -- 2.31.1 ^ permalink raw reply related [flat|nested] 42+ messages in thread
* Re: [PATCH 2/5] x86/ras: X86_FEATURE_INTEL_PPIN finally has a CPUID bit 2022-01-07 22:54 ` [PATCH 2/5] x86/ras: X86_FEATURE_INTEL_PPIN finally has a CPUID bit Tony Luck @ 2022-01-20 13:32 ` Borislav Petkov 0 siblings, 0 replies; 42+ messages in thread From: Borislav Petkov @ 2022-01-20 13:32 UTC (permalink / raw) To: Tony Luck Cc: x86, linux-kernel, Greg Kroah-Hartman, Smita Koralahalli Channabasappa, Wei Huang, Tom Lendacky, patches On Fri, Jan 07, 2022 at 02:54:39PM -0800, Tony Luck wrote: > After nine generations of adding to model specific list of CPUS that CPUs -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette ^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH 3/5] x86/ras: Read/save PPIN MSR during initialization 2022-01-07 22:54 [PATCH 0/5] PPIN (Protected Processor Inventory Number) updates Tony Luck 2022-01-07 22:54 ` [PATCH 1/5] x86/ras: Merge Intel and AMD ppin_init() functions Tony Luck 2022-01-07 22:54 ` [PATCH 2/5] x86/ras: X86_FEATURE_INTEL_PPIN finally has a CPUID bit Tony Luck @ 2022-01-07 22:54 ` Tony Luck 2022-01-07 22:54 ` [PATCH 4/5] x86/sysfs: Add format parameter to macro defining "show" functions for proc Tony Luck ` (2 subsequent siblings) 5 siblings, 0 replies; 42+ messages in thread From: Tony Luck @ 2022-01-07 22:54 UTC (permalink / raw) To: Borislav Petkov Cc: x86, linux-kernel, Greg Kroah-Hartman, Smita Koralahalli Channabasappa, Wei Huang, Tom Lendacky, patches, Tony Luck Currently the PPIN (Protected Processor Inventory Number) MSR is read by every CPU that processes a machine check, CMCI, or just polls machine check banks from a periodic timer. This is not a "fast" MSR, so this adds to overhead of processing errors. Add a new "ppin" field to the cpuinfo_x86 structure. Read and save the PPIN during initialization. Use this copy in mce_setup() instead of reading the MSR. Signed-off-by: Tony Luck <tony.luck@intel.com> --- arch/x86/include/asm/processor.h | 2 ++ arch/x86/kernel/cpu/common.c | 1 + arch/x86/kernel/cpu/mce/core.c | 7 +------ 3 files changed, 4 insertions(+), 6 deletions(-) diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h index 355d38c0cf60..1db5bb3413ae 100644 --- a/arch/x86/include/asm/processor.h +++ b/arch/x86/include/asm/processor.h @@ -119,6 +119,8 @@ struct cpuinfo_x86 { int x86_cache_mbm_width_offset; int x86_power; unsigned long loops_per_jiffy; + /* protected processor identification number */ + u64 ppin; /* cpuid returned max cores value: */ u16 x86_max_cores; u16 apicid; diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c index 3688f70ee0a2..9a90f96257d1 100644 --- a/arch/x86/kernel/cpu/common.c +++ b/arch/x86/kernel/cpu/common.c @@ -155,6 +155,7 @@ static void ppin_init(struct cpuinfo_x86 *c) /* Is the enable bit set? */ if (val & 2UL) { + c->ppin = __rdmsr(info->msr_ppin); set_cpu_cap(c, info->feature); return; } diff --git a/arch/x86/kernel/cpu/mce/core.c b/arch/x86/kernel/cpu/mce/core.c index 6ed365337a3b..ec38c0c6c235 100644 --- a/arch/x86/kernel/cpu/mce/core.c +++ b/arch/x86/kernel/cpu/mce/core.c @@ -139,12 +139,7 @@ noinstr void mce_setup(struct mce *m) m->socketid = cpu_data(m->extcpu).phys_proc_id; m->apicid = cpu_data(m->extcpu).initial_apicid; m->mcgcap = __rdmsr(MSR_IA32_MCG_CAP); - - if (this_cpu_has(X86_FEATURE_INTEL_PPIN)) - m->ppin = __rdmsr(MSR_PPIN); - else if (this_cpu_has(X86_FEATURE_AMD_PPIN)) - m->ppin = __rdmsr(MSR_AMD_PPIN); - + m->ppin = cpu_data(m->extcpu).ppin; m->microcode = boot_cpu_data.microcode; } -- 2.31.1 ^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH 4/5] x86/sysfs: Add format parameter to macro defining "show" functions for proc 2022-01-07 22:54 [PATCH 0/5] PPIN (Protected Processor Inventory Number) updates Tony Luck ` (2 preceding siblings ...) 2022-01-07 22:54 ` [PATCH 3/5] x86/ras: Read/save PPIN MSR during initialization Tony Luck @ 2022-01-07 22:54 ` Tony Luck 2022-01-20 13:32 ` Borislav Petkov 2022-01-07 22:54 ` [PATCH 5/5] x86/sysfs: Add PPIN in sysfs under cpu topology Tony Luck 2022-01-21 17:47 ` [PATCH v2 0/6] PPIN (Protected Processor Inventory Number) updates Tony Luck 5 siblings, 1 reply; 42+ messages in thread From: Tony Luck @ 2022-01-07 22:54 UTC (permalink / raw) To: Borislav Petkov Cc: x86, linux-kernel, Greg Kroah-Hartman, Smita Koralahalli Channabasappa, Wei Huang, Tom Lendacky, patches, Tony Luck All the simple (non-mask and non-list files in /sys/devices/system/cpu/cpu0/topology/ are currently printed as decimal integers. Refactor the macro that generates the "show" functions to take a format parameter to allow future files to display in other formats. No functional change. Signed-off-by: Tony Luck <tony.luck@intel.com> --- drivers/base/topology.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/drivers/base/topology.c b/drivers/base/topology.c index 8f2b641d0b8c..793c592e533a 100644 --- a/drivers/base/topology.c +++ b/drivers/base/topology.c @@ -14,11 +14,11 @@ #include <linux/hardirq.h> #include <linux/topology.h> -#define define_id_show_func(name) \ +#define define_id_show_func(name, fmt) \ static ssize_t name##_show(struct device *dev, \ struct device_attribute *attr, char *buf) \ { \ - return sysfs_emit(buf, "%d\n", topology_##name(dev->id)); \ + return sysfs_emit(buf, fmt "\n", topology_##name(dev->id)); \ } #define define_siblings_read_func(name, mask) \ @@ -42,16 +42,16 @@ static ssize_t name##_list_read(struct file *file, struct kobject *kobj, \ off, count); \ } -define_id_show_func(physical_package_id); +define_id_show_func(physical_package_id, "%d"); static DEVICE_ATTR_RO(physical_package_id); -define_id_show_func(die_id); +define_id_show_func(die_id, "%d"); static DEVICE_ATTR_RO(die_id); -define_id_show_func(cluster_id); +define_id_show_func(cluster_id, "%d"); static DEVICE_ATTR_RO(cluster_id); -define_id_show_func(core_id); +define_id_show_func(core_id, "%d"); static DEVICE_ATTR_RO(core_id); define_siblings_read_func(thread_siblings, sibling_cpumask); @@ -79,7 +79,7 @@ static BIN_ATTR_RO(package_cpus, 0); static BIN_ATTR_RO(package_cpus_list, 0); #ifdef CONFIG_SCHED_BOOK -define_id_show_func(book_id); +define_id_show_func(book_id, "%d"); static DEVICE_ATTR_RO(book_id); define_siblings_read_func(book_siblings, book_cpumask); static BIN_ATTR_RO(book_siblings, 0); @@ -87,7 +87,7 @@ static BIN_ATTR_RO(book_siblings_list, 0); #endif #ifdef CONFIG_SCHED_DRAWER -define_id_show_func(drawer_id); +define_id_show_func(drawer_id, "%d"); static DEVICE_ATTR_RO(drawer_id); define_siblings_read_func(drawer_siblings, drawer_cpumask); static BIN_ATTR_RO(drawer_siblings, 0); -- 2.31.1 ^ permalink raw reply related [flat|nested] 42+ messages in thread
* Re: [PATCH 4/5] x86/sysfs: Add format parameter to macro defining "show" functions for proc 2022-01-07 22:54 ` [PATCH 4/5] x86/sysfs: Add format parameter to macro defining "show" functions for proc Tony Luck @ 2022-01-20 13:32 ` Borislav Petkov 0 siblings, 0 replies; 42+ messages in thread From: Borislav Petkov @ 2022-01-20 13:32 UTC (permalink / raw) To: Tony Luck Cc: x86, linux-kernel, Greg Kroah-Hartman, Smita Koralahalli Channabasappa, Wei Huang, Tom Lendacky, patches On Fri, Jan 07, 2022 at 02:54:41PM -0800, Tony Luck wrote: > All the simple (non-mask and non-list files in > /sys/devices/system/cpu/cpu0/topology/ are currently printed as decimal > integers. > > Refactor the macro that generates the "show" functions to take a format > parameter to allow future files to display in other formats. > > No functional change. > > Signed-off-by: Tony Luck <tony.luck@intel.com> > --- > drivers/base/topology.c | 16 ++++++++-------- > 1 file changed, 8 insertions(+), 8 deletions(-) This one doesn't apply anymore. -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette ^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH 5/5] x86/sysfs: Add PPIN in sysfs under cpu topology 2022-01-07 22:54 [PATCH 0/5] PPIN (Protected Processor Inventory Number) updates Tony Luck ` (3 preceding siblings ...) 2022-01-07 22:54 ` [PATCH 4/5] x86/sysfs: Add format parameter to macro defining "show" functions for proc Tony Luck @ 2022-01-07 22:54 ` Tony Luck 2022-01-20 13:35 ` Borislav Petkov 2022-01-21 17:47 ` [PATCH v2 0/6] PPIN (Protected Processor Inventory Number) updates Tony Luck 5 siblings, 1 reply; 42+ messages in thread From: Tony Luck @ 2022-01-07 22:54 UTC (permalink / raw) To: Borislav Petkov Cc: x86, linux-kernel, Greg Kroah-Hartman, Smita Koralahalli Channabasappa, Wei Huang, Tom Lendacky, patches, Tony Luck PPIN is the Protected Processor Identification Number. This is used to identify the socket as a Field Replaceable Unit (FRU). Existing code only displays this when reporting errors. But this makes it inconvenient for large clusters to use it for its intended purpose of inventory control. There are several privacy concerns associated with a unique platform identifier. But making the PPIN available shouldn't change anything important. Notes: 1) The PPIN is only enabled on server CPUs (E.g. Intel Xeon "-E5", "-E7" and "-SP" parts). 2) The PPIN MSR is may be implemented on some desktop/laptop parts. But this is for OEM inventory control. Production BIOS versions leave the PPIN_CTL MSR in the "locked disabled" mode. 3) There may be a BIOS option to lock the MSR in disabled mode to prevent Linux from reading it. 4) The /sys file added here is readable only by "root". Signed-off-by: Tony Luck <tony.luck@intel.com> --- Documentation/ABI/stable/sysfs-devices-system-cpu | 4 ++++ Documentation/ABI/testing/sysfs-devices-system-cpu | 6 ++++++ arch/x86/include/asm/topology.h | 1 + drivers/base/topology.c | 4 ++++ include/linux/topology.h | 3 +++ 5 files changed, 18 insertions(+) diff --git a/Documentation/ABI/stable/sysfs-devices-system-cpu b/Documentation/ABI/stable/sysfs-devices-system-cpu index 3965ce504484..902392d7eddf 100644 --- a/Documentation/ABI/stable/sysfs-devices-system-cpu +++ b/Documentation/ABI/stable/sysfs-devices-system-cpu @@ -86,6 +86,10 @@ What: /sys/devices/system/cpu/cpuX/topology/die_cpus Description: internal kernel map of CPUs within the same die. Values: hexadecimal bitmask. +What: /sys/devices/system/cpu/cpuX/topology/ppin +Description: per-socket protected processor inventory number +Values: hexadecimal. + What: /sys/devices/system/cpu/cpuX/topology/die_cpus_list Description: human-readable list of CPUs within the same die. The format is like 0-3, 8-11, 14,17. diff --git a/Documentation/ABI/testing/sysfs-devices-system-cpu b/Documentation/ABI/testing/sysfs-devices-system-cpu index 69c65da16dff..18af7459adf8 100644 --- a/Documentation/ABI/testing/sysfs-devices-system-cpu +++ b/Documentation/ABI/testing/sysfs-devices-system-cpu @@ -73,6 +73,7 @@ What: /sys/devices/system/cpu/cpuX/topology/core_id /sys/devices/system/cpu/cpuX/topology/physical_package_id /sys/devices/system/cpu/cpuX/topology/thread_siblings /sys/devices/system/cpu/cpuX/topology/thread_siblings_list + /sys/devices/system/cpu/cpuX/topology/ppin Date: December 2008 Contact: Linux kernel mailing list <linux-kernel@vger.kernel.org> Description: CPU topology files that describe a logical CPU's relationship @@ -103,6 +104,11 @@ Description: CPU topology files that describe a logical CPU's relationship thread_siblings_list: human-readable list of cpuX's hardware threads within the same core as cpuX + ppin: human-readable Protected Processor Identification + Number of the socket the cpu# belongs to. There should be + one per physical_package_id. File is readable only to + admin. + See Documentation/admin-guide/cputopology.rst for more information. diff --git a/arch/x86/include/asm/topology.h b/arch/x86/include/asm/topology.h index cc164777e661..caba7db8c7b6 100644 --- a/arch/x86/include/asm/topology.h +++ b/arch/x86/include/asm/topology.h @@ -110,6 +110,7 @@ extern const struct cpumask *cpu_clustergroup_mask(int cpu); #define topology_logical_die_id(cpu) (cpu_data(cpu).logical_die_id) #define topology_die_id(cpu) (cpu_data(cpu).cpu_die_id) #define topology_core_id(cpu) (cpu_data(cpu).cpu_core_id) +#define topology_ppin(cpu) (cpu_data(cpu).ppin) extern unsigned int __max_die_per_package; diff --git a/drivers/base/topology.c b/drivers/base/topology.c index 793c592e533a..4c8674715d36 100644 --- a/drivers/base/topology.c +++ b/drivers/base/topology.c @@ -54,6 +54,9 @@ static DEVICE_ATTR_RO(cluster_id); define_id_show_func(core_id, "%d"); static DEVICE_ATTR_RO(core_id); +define_id_show_func(ppin, "%llx"); +static DEVICE_ATTR_ADMIN_RO(ppin); + define_siblings_read_func(thread_siblings, sibling_cpumask); static BIN_ATTR_RO(thread_siblings, 0); static BIN_ATTR_RO(thread_siblings_list, 0); @@ -129,6 +132,7 @@ static struct attribute *default_attrs[] = { #ifdef CONFIG_SCHED_DRAWER &dev_attr_drawer_id.attr, #endif + &dev_attr_ppin.attr, NULL }; diff --git a/include/linux/topology.h b/include/linux/topology.h index 0b3704ad13c8..789a900039e0 100644 --- a/include/linux/topology.h +++ b/include/linux/topology.h @@ -192,6 +192,9 @@ static inline int cpu_to_mem(int cpu) #ifndef topology_core_id #define topology_core_id(cpu) ((void)(cpu), 0) #endif +#ifndef topology_ppin +#define topology_ppin(cpu) ((void)(cpu), 0ull) +#endif #ifndef topology_sibling_cpumask #define topology_sibling_cpumask(cpu) cpumask_of(cpu) #endif -- 2.31.1 ^ permalink raw reply related [flat|nested] 42+ messages in thread
* Re: [PATCH 5/5] x86/sysfs: Add PPIN in sysfs under cpu topology 2022-01-07 22:54 ` [PATCH 5/5] x86/sysfs: Add PPIN in sysfs under cpu topology Tony Luck @ 2022-01-20 13:35 ` Borislav Petkov 2022-01-20 18:01 ` Luck, Tony 0 siblings, 1 reply; 42+ messages in thread From: Borislav Petkov @ 2022-01-20 13:35 UTC (permalink / raw) To: Tony Luck Cc: x86, linux-kernel, Greg Kroah-Hartman, Smita Koralahalli Channabasappa, Wei Huang, Tom Lendacky, patches On Fri, Jan 07, 2022 at 02:54:42PM -0800, Tony Luck wrote: > PPIN is the Protected Processor Identification Number. > This is used to identify the socket as a Field Replaceable Unit (FRU). > > Existing code only displays this when reporting errors. But this makes > it inconvenient for large clusters to use it for its intended purpose > of inventory control. Do you have any concrete use cases you can cite here or this is one of those: "let's make it available and see who'll use it" thing? Because defeaturing a user-visible thing later is always a pain. > There are several privacy concerns associated with a unique > platform identifier. But making the PPIN available shouldn't > change anything important. Notes: > > 1) The PPIN is only enabled on server CPUs (E.g. Intel Xeon > "-E5", "-E7" and "-SP" parts). Can't use that as an argument - that can easily change in the future. > 2) The PPIN MSR is may be implemented on some desktop/laptop parts. s/is // > But this is for OEM inventory control. Production BIOS versions > leave the PPIN_CTL MSR in the "locked disabled" mode. That either. Never let the BIOS do your work for you. :-) > 3) There may be a BIOS option to lock the MSR in disabled mode > to prevent Linux from reading it. > > 4) The /sys file added here is readable only by "root". Yap, that's the argument: your patch simply makes what is already accessible to root through rdmsr in a more user-friendly way. > Signed-off-by: Tony Luck <tony.luck@intel.com> > --- > Documentation/ABI/stable/sysfs-devices-system-cpu | 4 ++++ > Documentation/ABI/testing/sysfs-devices-system-cpu | 6 ++++++ > arch/x86/include/asm/topology.h | 1 + > drivers/base/topology.c | 4 ++++ > include/linux/topology.h | 3 +++ > 5 files changed, 18 insertions(+) > diff --git a/drivers/base/topology.c b/drivers/base/topology.c > index 793c592e533a..4c8674715d36 100644 > --- a/drivers/base/topology.c > +++ b/drivers/base/topology.c > @@ -54,6 +54,9 @@ static DEVICE_ATTR_RO(cluster_id); > define_id_show_func(core_id, "%d"); > static DEVICE_ATTR_RO(core_id); > > +define_id_show_func(ppin, "%llx"); "0x%llx" Otherwise it is ambiguous. > diff --git a/arch/x86/include/asm/topology.h b/arch/x86/include/asm/topology.h > index cc164777e661..caba7db8c7b6 100644 > --- a/arch/x86/include/asm/topology.h > +++ b/arch/x86/include/asm/topology.h > @@ -110,6 +110,7 @@ extern const struct cpumask *cpu_clustergroup_mask(int cpu); > #define topology_logical_die_id(cpu) (cpu_data(cpu).logical_die_id) > #define topology_die_id(cpu) (cpu_data(cpu).cpu_die_id) > #define topology_core_id(cpu) (cpu_data(cpu).cpu_core_id) > +#define topology_ppin(cpu) (cpu_data(cpu).ppin) That looks unused. No need to add it. -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH 5/5] x86/sysfs: Add PPIN in sysfs under cpu topology 2022-01-20 13:35 ` Borislav Petkov @ 2022-01-20 18:01 ` Luck, Tony 2022-01-20 22:48 ` Borislav Petkov 0 siblings, 1 reply; 42+ messages in thread From: Luck, Tony @ 2022-01-20 18:01 UTC (permalink / raw) To: Borislav Petkov Cc: x86, linux-kernel, Greg Kroah-Hartman, Smita Koralahalli Channabasappa, Wei Huang, Tom Lendacky, patches On Thu, Jan 20, 2022 at 02:35:51PM +0100, Borislav Petkov wrote: > On Fri, Jan 07, 2022 at 02:54:42PM -0800, Tony Luck wrote: > > +#define topology_ppin(cpu) (cpu_data(cpu).ppin) > > That looks unused. No need to add it. It "looks" unused, But thanks to the obscurity of CPP macros using ## to concatenate tokens it is needed. This harmless looking line: define_id_show_func(ppin, "%llx"); is transmogrified by this: #define define_id_show_func(name, fmt) \ static ssize_t name##_show(struct device *dev, \ struct device_attribute *attr, char *buf) \ { \ return sysfs_emit(buf, fmt "\n", topology_##name(dev->id)); \ ^^^^^^^^^^^^^^^ This becomes topology_ppin } Will fix the other stuff and rebase to latest so part 4 applies. Thanks for the review. -Tony ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH 5/5] x86/sysfs: Add PPIN in sysfs under cpu topology 2022-01-20 18:01 ` Luck, Tony @ 2022-01-20 22:48 ` Borislav Petkov 0 siblings, 0 replies; 42+ messages in thread From: Borislav Petkov @ 2022-01-20 22:48 UTC (permalink / raw) To: Luck, Tony Cc: x86, linux-kernel, Greg Kroah-Hartman, Smita Koralahalli Channabasappa, Wei Huang, Tom Lendacky, patches On Thu, Jan 20, 2022 at 10:01:51AM -0800, Luck, Tony wrote: > It "looks" unused, But thanks to the obscurity of CPP macros using ## to > concatenate tokens it is needed. Ofcourse it is. It crossed my mind for a second that, hey, that is *probably* used by some macro magic there... oh well. -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette ^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v2 0/6] PPIN (Protected Processor Inventory Number) updates 2022-01-07 22:54 [PATCH 0/5] PPIN (Protected Processor Inventory Number) updates Tony Luck ` (4 preceding siblings ...) 2022-01-07 22:54 ` [PATCH 5/5] x86/sysfs: Add PPIN in sysfs under cpu topology Tony Luck @ 2022-01-21 17:47 ` Tony Luck 2022-01-21 17:47 ` [PATCH v2 1/6] x86/cpu: Add Xeon Icelake-D to list of CPUs that support PPIN Tony Luck ` (7 more replies) 5 siblings, 8 replies; 42+ messages in thread From: Tony Luck @ 2022-01-21 17:47 UTC (permalink / raw) To: Borislav Petkov Cc: x86, linux-kernel, Greg Kroah-Hartman, Smita Koralahalli Channabasappa, Wei Huang, Tom Lendacky, patches, Tony Luck v2 starts with an extra patch over v1. Adding INTEL_FAM6_ICELAKE_D to the list of legacy processors that support PPIN. I've marked this patch for stable. I don't think the rest of the patches are candidates for stable or long term support kernels. The rest of the patches are updates from v1 of the series starts out with two changes that I expect are uncontroversial. Later parts get progressively more "RFC". 1) Simple cleanup to merge Intel and AMD duplicated code to test for presence of PPIN and check whether it is enabled. 2) Long overdue update from Intel to enumerate the PPIN and PPIN_CTL MSRs. See the December 2021 Software Developers Manual {RFC factor moves to medium here} 3) Code to scan machine check banks re-reads the PPIN every time banks are scanned (whether for a machine check, a CMCI, or just a periodic poll). Since PPIN never changes, this seems like unnecessary overhead. Read the MSR once (per CPU) and save to memory. {RFC factor moves to high for last two parts} 4) Refactor as prep for last part. 5) Add "ppin" to /sys/devices/system/cpu/cpu*/topology/ppin The big question for this part is whether there is a better place to expose this value. I'm open to other suggestions. I do think it is useful to do so. An "inventory" number that stays hidden until there is an error that causes it to show up in a machine check log is user hostile. Changes since V1: ----------------- Added INTEL_FAM6_ICELAKE_D Use X86_MATCH_FEATURE() instead of X86_MATCH_VENDOR_FEATURE Spelling: s/prescence/presence/ s/CPUS/CPUs/ Print the hex value of PPIN with a leading "0x" Fix the Subject line commit prefixes to use "x86/cpu" and "topology/sysfs" Move the introduction of the "msr_ppin" field to the patch where used. Rewrite the commit comment justifying adding ppin to /sys/.../topology Upcoming use case for user accessible ppin is for reporting issues found by on-line testing of CPU cores. The MSRs for this are public in the latest SDM (look for MSR_ACTIVATE_SCAN and a bunch of others in the same section) but the SDM is currently light on details on what it does or how to use it. Linux patches to enable coming soon. Tony Luck (6): x86/cpu: Add Xeon Icelake-D to list of CPUs that support PPIN x86/cpu: Merge Intel and AMD ppin_init() functions x86/cpu: X86_FEATURE_INTEL_PPIN finally has a CPUID bit x86/cpu: Read/save PPIN MSR during initialization topology/sysfs: Add format parameter to macro defining "show" functions for proc topology/sysfs: Add PPIN in sysfs under cpu topology .../ABI/stable/sysfs-devices-system-cpu | 4 + .../ABI/testing/sysfs-devices-system-cpu | 6 ++ arch/x86/include/asm/processor.h | 2 + arch/x86/include/asm/topology.h | 1 + arch/x86/kernel/cpu/amd.c | 30 ------- arch/x86/kernel/cpu/common.c | 79 +++++++++++++++++++ arch/x86/kernel/cpu/mce/core.c | 7 +- arch/x86/kernel/cpu/mce/intel.c | 41 ---------- arch/x86/kernel/cpu/scattered.c | 1 + drivers/base/topology.c | 20 +++-- include/linux/topology.h | 3 + 11 files changed, 109 insertions(+), 85 deletions(-) base-commit: 2c271fe77d52a0555161926c232cd5bc07178b39 -- 2.31.1 ^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v2 1/6] x86/cpu: Add Xeon Icelake-D to list of CPUs that support PPIN 2022-01-21 17:47 ` [PATCH v2 0/6] PPIN (Protected Processor Inventory Number) updates Tony Luck @ 2022-01-21 17:47 ` Tony Luck 2022-01-21 17:47 ` [PATCH v2 2/6] x86/cpu: Merge Intel and AMD ppin_init() functions Tony Luck ` (6 subsequent siblings) 7 siblings, 0 replies; 42+ messages in thread From: Tony Luck @ 2022-01-21 17:47 UTC (permalink / raw) To: Borislav Petkov Cc: x86, linux-kernel, Greg Kroah-Hartman, Smita Koralahalli Channabasappa, Wei Huang, Tom Lendacky, patches, Tony Luck, Ailin Xu Missed adding the Icelake-D CPU to the list. It uses the same MSRs to control and read the inventory number as all the other models. Reported-by: Ailin Xu <ailin.xu@intel.com> Fixes: dc6b025de95b ("x86/mce: Add Xeon Icelake to list of CPUs that support PPIN") Cc: <stable@vger.kernel.org> Signed-off-by: Tony Luck <tony.luck@intel.com> --- arch/x86/kernel/cpu/mce/intel.c | 1 + 1 file changed, 1 insertion(+) diff --git a/arch/x86/kernel/cpu/mce/intel.c b/arch/x86/kernel/cpu/mce/intel.c index bb9a46a804bf..baafbb37be67 100644 --- a/arch/x86/kernel/cpu/mce/intel.c +++ b/arch/x86/kernel/cpu/mce/intel.c @@ -486,6 +486,7 @@ static void intel_ppin_init(struct cpuinfo_x86 *c) case INTEL_FAM6_BROADWELL_X: case INTEL_FAM6_SKYLAKE_X: case INTEL_FAM6_ICELAKE_X: + case INTEL_FAM6_ICELAKE_D: case INTEL_FAM6_SAPPHIRERAPIDS_X: case INTEL_FAM6_XEON_PHI_KNL: case INTEL_FAM6_XEON_PHI_KNM: -- 2.31.1 ^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH v2 2/6] x86/cpu: Merge Intel and AMD ppin_init() functions 2022-01-21 17:47 ` [PATCH v2 0/6] PPIN (Protected Processor Inventory Number) updates Tony Luck 2022-01-21 17:47 ` [PATCH v2 1/6] x86/cpu: Add Xeon Icelake-D to list of CPUs that support PPIN Tony Luck @ 2022-01-21 17:47 ` Tony Luck 2022-01-27 10:22 ` Borislav Petkov 2022-01-21 17:47 ` [PATCH v2 3/6] x86/cpu: X86_FEATURE_INTEL_PPIN finally has a CPUID bit Tony Luck ` (5 subsequent siblings) 7 siblings, 1 reply; 42+ messages in thread From: Tony Luck @ 2022-01-21 17:47 UTC (permalink / raw) To: Borislav Petkov Cc: x86, linux-kernel, Greg Kroah-Hartman, Smita Koralahalli Channabasappa, Wei Huang, Tom Lendacky, patches, Tony Luck The code to decide whether a system supports the PPIN (Protected Processor Inventory Number) MSR was cloned from the Intel implementation. Apart from the X86_FEATURE bit and the MSR numbers it is identical. Merge the two functions into common x86 code, but use x86_match_cpu() instead of the switch (c->x86_model) that was used by the old Intel code. No functional change. Signed-off-by: Tony Luck <tony.luck@intel.com> --- arch/x86/kernel/cpu/amd.c | 30 ------------- arch/x86/kernel/cpu/common.c | 76 +++++++++++++++++++++++++++++++++ arch/x86/kernel/cpu/mce/intel.c | 42 ------------------ 3 files changed, 76 insertions(+), 72 deletions(-) diff --git a/arch/x86/kernel/cpu/amd.c b/arch/x86/kernel/cpu/amd.c index 4edb6f0f628c..bad0fa4c1779 100644 --- a/arch/x86/kernel/cpu/amd.c +++ b/arch/x86/kernel/cpu/amd.c @@ -394,35 +394,6 @@ static void amd_detect_cmp(struct cpuinfo_x86 *c) per_cpu(cpu_llc_id, cpu) = c->cpu_die_id = c->phys_proc_id; } -static void amd_detect_ppin(struct cpuinfo_x86 *c) -{ - unsigned long long val; - - if (!cpu_has(c, X86_FEATURE_AMD_PPIN)) - return; - - /* When PPIN is defined in CPUID, still need to check PPIN_CTL MSR */ - if (rdmsrl_safe(MSR_AMD_PPIN_CTL, &val)) - goto clear_ppin; - - /* PPIN is locked in disabled mode, clear feature bit */ - if ((val & 3UL) == 1UL) - goto clear_ppin; - - /* If PPIN is disabled, try to enable it */ - if (!(val & 2UL)) { - wrmsrl_safe(MSR_AMD_PPIN_CTL, val | 2UL); - rdmsrl_safe(MSR_AMD_PPIN_CTL, &val); - } - - /* If PPIN_EN bit is 1, return from here; otherwise fall through */ - if (val & 2UL) - return; - -clear_ppin: - clear_cpu_cap(c, X86_FEATURE_AMD_PPIN); -} - u32 amd_get_nodes_per_socket(void) { return nodes_per_socket; @@ -947,7 +918,6 @@ static void init_amd(struct cpuinfo_x86 *c) amd_detect_cmp(c); amd_get_topology(c); srat_detect_node(c); - amd_detect_ppin(c); init_amd_cacheinfo(c); diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c index 7b8382c11788..b7700a47eadd 100644 --- a/arch/x86/kernel/cpu/common.c +++ b/arch/x86/kernel/cpu/common.c @@ -88,6 +88,80 @@ EXPORT_SYMBOL_GPL(get_llc_id); /* L2 cache ID of each logical CPU */ DEFINE_PER_CPU_READ_MOSTLY(u16, cpu_l2c_id) = BAD_APICID; +static struct ppin_info { + int feature; + int msr_ppin_ctl; +} ppin_info[] = { + [X86_VENDOR_INTEL] = { + .feature = X86_FEATURE_INTEL_PPIN, + .msr_ppin_ctl = MSR_PPIN_CTL, + .msr_ppin = MSR_PPIN + }, + [X86_VENDOR_AMD] = { + .feature = X86_FEATURE_AMD_PPIN, + .msr_ppin_ctl = MSR_AMD_PPIN_CTL, + .msr_ppin = MSR_AMD_PPIN + }, +}; + +static const struct x86_cpu_id ppin_cpuids[] = { + X86_MATCH_FEATURE(X86_FEATURE_AMD_PPIN, &ppin_info[X86_VENDOR_AMD]), + + /* Legacy models without CPUID enumeration */ + X86_MATCH_INTEL_FAM6_MODEL(IVYBRIDGE_X, &ppin_info[X86_VENDOR_INTEL]), + X86_MATCH_INTEL_FAM6_MODEL(HASWELL_X, &ppin_info[X86_VENDOR_INTEL]), + X86_MATCH_INTEL_FAM6_MODEL(BROADWELL_D, &ppin_info[X86_VENDOR_INTEL]), + X86_MATCH_INTEL_FAM6_MODEL(BROADWELL_X, &ppin_info[X86_VENDOR_INTEL]), + X86_MATCH_INTEL_FAM6_MODEL(SKYLAKE_X, &ppin_info[X86_VENDOR_INTEL]), + X86_MATCH_INTEL_FAM6_MODEL(ICELAKE_X, &ppin_info[X86_VENDOR_INTEL]), + X86_MATCH_INTEL_FAM6_MODEL(ICELAKE_D, &ppin_info[X86_VENDOR_INTEL]), + X86_MATCH_INTEL_FAM6_MODEL(SAPPHIRERAPIDS_X, &ppin_info[X86_VENDOR_INTEL]), + X86_MATCH_INTEL_FAM6_MODEL(XEON_PHI_KNL, &ppin_info[X86_VENDOR_INTEL]), + X86_MATCH_INTEL_FAM6_MODEL(XEON_PHI_KNM, &ppin_info[X86_VENDOR_INTEL]), + + {} +}; + +static void ppin_init(struct cpuinfo_x86 *c) +{ + const struct x86_cpu_id *id; + unsigned long long val; + struct ppin_info *info; + + id = x86_match_cpu(ppin_cpuids); + if (!id) + return; + + /* + * Testing the presence of the MSR is not enough. Need to check + * that the PPIN_CTL allows reading of the PPIN. + */ + info = (struct ppin_info *)id->driver_data; + + if (rdmsrl_safe(info->msr_ppin_ctl, &val)) + goto clear_ppin; + + if ((val & 3UL) == 1UL) { + /* PPIN locked in disabled mode */ + goto clear_ppin; + } + + /* If PPIN is disabled, try to enable */ + if (!(val & 2UL)) { + wrmsrl_safe(info->msr_ppin_ctl, val | 2UL); + rdmsrl_safe(info->msr_ppin_ctl, &val); + } + + /* Is the enable bit set? */ + if (val & 2UL) { + set_cpu_cap(c, info->feature); + return; + } + +clear_ppin: + clear_cpu_cap(c, info->feature); +} + /* correctly size the local cpu masks */ void __init setup_cpu_local_masks(void) { @@ -1655,6 +1729,8 @@ static void identify_cpu(struct cpuinfo_x86 *c) c->x86_capability[i] |= boot_cpu_data.x86_capability[i]; } + ppin_init(c); + /* Init Machine Check Exception if available. */ mcheck_cpu_init(c); diff --git a/arch/x86/kernel/cpu/mce/intel.c b/arch/x86/kernel/cpu/mce/intel.c index baafbb37be67..95275a5e57e0 100644 --- a/arch/x86/kernel/cpu/mce/intel.c +++ b/arch/x86/kernel/cpu/mce/intel.c @@ -470,47 +470,6 @@ void intel_clear_lmce(void) wrmsrl(MSR_IA32_MCG_EXT_CTL, val); } -static void intel_ppin_init(struct cpuinfo_x86 *c) -{ - unsigned long long val; - - /* - * Even if testing the presence of the MSR would be enough, we don't - * want to risk the situation where other models reuse this MSR for - * other purposes. - */ - switch (c->x86_model) { - case INTEL_FAM6_IVYBRIDGE_X: - case INTEL_FAM6_HASWELL_X: - case INTEL_FAM6_BROADWELL_D: - case INTEL_FAM6_BROADWELL_X: - case INTEL_FAM6_SKYLAKE_X: - case INTEL_FAM6_ICELAKE_X: - case INTEL_FAM6_ICELAKE_D: - case INTEL_FAM6_SAPPHIRERAPIDS_X: - case INTEL_FAM6_XEON_PHI_KNL: - case INTEL_FAM6_XEON_PHI_KNM: - - if (rdmsrl_safe(MSR_PPIN_CTL, &val)) - return; - - if ((val & 3UL) == 1UL) { - /* PPIN locked in disabled mode */ - return; - } - - /* If PPIN is disabled, try to enable */ - if (!(val & 2UL)) { - wrmsrl_safe(MSR_PPIN_CTL, val | 2UL); - rdmsrl_safe(MSR_PPIN_CTL, &val); - } - - /* Is the enable bit set? */ - if (val & 2UL) - set_cpu_cap(c, X86_FEATURE_INTEL_PPIN); - } -} - /* * Enable additional error logs from the integrated * memory controller on processors that support this. @@ -535,7 +494,6 @@ void mce_intel_feature_init(struct cpuinfo_x86 *c) { intel_init_cmci(); intel_init_lmce(); - intel_ppin_init(c); intel_imc_init(c); } -- 2.31.1 ^ permalink raw reply related [flat|nested] 42+ messages in thread
* Re: [PATCH v2 2/6] x86/cpu: Merge Intel and AMD ppin_init() functions 2022-01-21 17:47 ` [PATCH v2 2/6] x86/cpu: Merge Intel and AMD ppin_init() functions Tony Luck @ 2022-01-27 10:22 ` Borislav Petkov 2022-01-27 16:52 ` Luck, Tony 0 siblings, 1 reply; 42+ messages in thread From: Borislav Petkov @ 2022-01-27 10:22 UTC (permalink / raw) To: Tony Luck Cc: x86, linux-kernel, Greg Kroah-Hartman, Smita Koralahalli Channabasappa, Wei Huang, Tom Lendacky, patches On Fri, Jan 21, 2022 at 09:47:39AM -0800, Tony Luck wrote: > diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c > index 7b8382c11788..b7700a47eadd 100644 > --- a/arch/x86/kernel/cpu/common.c > +++ b/arch/x86/kernel/cpu/common.c > @@ -88,6 +88,80 @@ EXPORT_SYMBOL_GPL(get_llc_id); > /* L2 cache ID of each logical CPU */ > DEFINE_PER_CPU_READ_MOSTLY(u16, cpu_l2c_id) = BAD_APICID; > > +static struct ppin_info { > + int feature; > + int msr_ppin_ctl; > +} ppin_info[] = { > + [X86_VENDOR_INTEL] = { > + .feature = X86_FEATURE_INTEL_PPIN, > + .msr_ppin_ctl = MSR_PPIN_CTL, > + .msr_ppin = MSR_PPIN > + }, > + [X86_VENDOR_AMD] = { > + .feature = X86_FEATURE_AMD_PPIN, > + .msr_ppin_ctl = MSR_AMD_PPIN_CTL, > + .msr_ppin = MSR_AMD_PPIN ^^^^^^^^^ You forgot to rebuild after removing that guy here in the struct definition. I'll fix it up now so that I can continue going through them but pls fix in the next submission so that all patches build separately. Thx. arch/x86/kernel/cpu/common.c:98:4: error: ‘struct ppin_info’ has no member named ‘msr_ppin’; did you mean ‘msr_ppin_ctl’? 98 | .msr_ppin = MSR_PPIN | ^~~~~~~~ | msr_ppin_ctl In file included from ./arch/x86/include/asm/msr.h:5, from ./arch/x86/include/asm/processor.h:22, from ./arch/x86/include/asm/cpufeature.h:5, from ./arch/x86/include/asm/thread_info.h:53, from ./include/linux/thread_info.h:60, from ./arch/x86/include/asm/preempt.h:7, from ./include/linux/preempt.h:78, from ./include/linux/spinlock.h:55, from ./include/linux/mmzone.h:8, from ./include/linux/gfp.h:6, from ./include/linux/mm.h:10, from ./include/linux/memblock.h:12, from arch/x86/kernel/cpu/common.c:5: ./arch/x86/include/asm/msr-index.h:59:20: warning: excess elements in struct initializer 59 | #define MSR_PPIN 0x0000004f | ^~~~~~~~~~ arch/x86/kernel/cpu/common.c:98:15: note: in expansion of macro ‘MSR_PPIN’ 98 | .msr_ppin = MSR_PPIN | ^~~~~~~~ ./arch/x86/include/asm/msr-index.h:59:20: note: (near initialization for ‘ppin_info[0]’) 59 | #define MSR_PPIN 0x0000004f | ^~~~~~~~~~ arch/x86/kernel/cpu/common.c:98:15: note: in expansion of macro ‘MSR_PPIN’ 98 | .msr_ppin = MSR_PPIN | ^~~~~~~~ arch/x86/kernel/cpu/common.c:103:4: error: ‘struct ppin_info’ has no member named ‘msr_ppin’; did you mean ‘msr_ppin_ctl’? 103 | .msr_ppin = MSR_AMD_PPIN | ^~~~~~~~ | msr_ppin_ctl In file included from ./arch/x86/include/asm/msr.h:5, from ./arch/x86/include/asm/processor.h:22, from ./arch/x86/include/asm/cpufeature.h:5, from ./arch/x86/include/asm/thread_info.h:53, from ./include/linux/thread_info.h:60, from ./arch/x86/include/asm/preempt.h:7, from ./include/linux/preempt.h:78, from ./include/linux/spinlock.h:55, from ./include/linux/mmzone.h:8, from ./include/linux/gfp.h:6, from ./include/linux/mm.h:10, from ./include/linux/memblock.h:12, from arch/x86/kernel/cpu/common.c:5: ./arch/x86/include/asm/msr-index.h:455:24: warning: excess elements in struct initializer 455 | #define MSR_AMD_PPIN 0xc00102f1 | ^~~~~~~~~~ arch/x86/kernel/cpu/common.c:103:15: note: in expansion of macro ‘MSR_AMD_PPIN’ 103 | .msr_ppin = MSR_AMD_PPIN | ^~~~~~~~~~~~ ./arch/x86/include/asm/msr-index.h:455:24: note: (near initialization for ‘ppin_info[2]’) 455 | #define MSR_AMD_PPIN 0xc00102f1 | ^~~~~~~~~~ arch/x86/kernel/cpu/common.c:103:15: note: in expansion of macro ‘MSR_AMD_PPIN’ 103 | .msr_ppin = MSR_AMD_PPIN | ^~~~~~~~~~~~ make[3]: *** [scripts/Makefile.build:288: arch/x86/kernel/cpu/common.o] Error 1 make[3]: *** Waiting for unfinished jobs.... make[2]: *** [scripts/Makefile.build:550: arch/x86/kernel/cpu] Error 2 make[1]: *** [scripts/Makefile.build:550: arch/x86/kernel] Error 2 make[1]: *** Waiting for unfinished jobs.... make: *** [Makefile:1831: arch/x86] Error 2 make: *** Waiting for unfinished jobs.... -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette ^ permalink raw reply [flat|nested] 42+ messages in thread
* RE: [PATCH v2 2/6] x86/cpu: Merge Intel and AMD ppin_init() functions 2022-01-27 10:22 ` Borislav Petkov @ 2022-01-27 16:52 ` Luck, Tony 0 siblings, 0 replies; 42+ messages in thread From: Luck, Tony @ 2022-01-27 16:52 UTC (permalink / raw) To: Borislav Petkov Cc: x86, linux-kernel, Greg Kroah-Hartman, Smita Koralahalli Channabasappa, Wei Huang, Tom Lendacky, patches >> + [X86_VENDOR_AMD] = { >> + .feature = X86_FEATURE_AMD_PPIN, >> + .msr_ppin_ctl = MSR_AMD_PPIN_CTL, >> + .msr_ppin = MSR_AMD_PPIN > ^^^^^^^^^ > > You forgot to rebuild after removing that guy here in the struct > definition. I'll fix it up now so that I can continue going through them > but pls fix in the next submission so that all patches build separately. Oops. Sorry. Yes. I will move those initializations out of this patch and into the later patch when the .msr_ppin field is added. I'll wait on posting v3 until you finish with the rest of v2. Thanks -Tony ^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v2 3/6] x86/cpu: X86_FEATURE_INTEL_PPIN finally has a CPUID bit 2022-01-21 17:47 ` [PATCH v2 0/6] PPIN (Protected Processor Inventory Number) updates Tony Luck 2022-01-21 17:47 ` [PATCH v2 1/6] x86/cpu: Add Xeon Icelake-D to list of CPUs that support PPIN Tony Luck 2022-01-21 17:47 ` [PATCH v2 2/6] x86/cpu: Merge Intel and AMD ppin_init() functions Tony Luck @ 2022-01-21 17:47 ` Tony Luck 2022-01-21 17:47 ` [PATCH v2 4/6] x86/cpu: Read/save PPIN MSR during initialization Tony Luck ` (4 subsequent siblings) 7 siblings, 0 replies; 42+ messages in thread From: Tony Luck @ 2022-01-21 17:47 UTC (permalink / raw) To: Borislav Petkov Cc: x86, linux-kernel, Greg Kroah-Hartman, Smita Koralahalli Channabasappa, Wei Huang, Tom Lendacky, patches, Tony Luck After nine generations of adding to model specific list of CPUs that support PPIN (Protected Processor Inventory Number) Intel allocated a CPUID bit to enumerate the MSRs. CPUID(EAX=7, ECX=1).EBX bit 0 enumerates presence of MSR_PPIN_CTL and MSR_PPIN. Add it to the "scattered" CPUID bits and add an entry to the ppin_cpuids[] x86_match_cpu() array to catch Intel CPUs that implement it. Signed-off-by: Tony Luck <tony.luck@intel.com> --- arch/x86/kernel/cpu/common.c | 1 + arch/x86/kernel/cpu/scattered.c | 1 + 2 files changed, 2 insertions(+) diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c index b7700a47eadd..8a039d1ea57f 100644 --- a/arch/x86/kernel/cpu/common.c +++ b/arch/x86/kernel/cpu/common.c @@ -106,6 +106,7 @@ static struct ppin_info { static const struct x86_cpu_id ppin_cpuids[] = { X86_MATCH_FEATURE(X86_FEATURE_AMD_PPIN, &ppin_info[X86_VENDOR_AMD]), + X86_MATCH_FEATURE(X86_FEATURE_INTEL_PPIN, &ppin_info[X86_VENDOR_INTEL]), /* Legacy models without CPUID enumeration */ X86_MATCH_INTEL_FAM6_MODEL(IVYBRIDGE_X, &ppin_info[X86_VENDOR_INTEL]), diff --git a/arch/x86/kernel/cpu/scattered.c b/arch/x86/kernel/cpu/scattered.c index 21d1f062895a..4143b1e4c5c6 100644 --- a/arch/x86/kernel/cpu/scattered.c +++ b/arch/x86/kernel/cpu/scattered.c @@ -26,6 +26,7 @@ struct cpuid_bit { static const struct cpuid_bit cpuid_bits[] = { { X86_FEATURE_APERFMPERF, CPUID_ECX, 0, 0x00000006, 0 }, { X86_FEATURE_EPB, CPUID_ECX, 3, 0x00000006, 0 }, + { X86_FEATURE_INTEL_PPIN, CPUID_EBX, 0, 0x00000007, 1 }, { X86_FEATURE_CQM_LLC, CPUID_EDX, 1, 0x0000000f, 0 }, { X86_FEATURE_CQM_OCCUP_LLC, CPUID_EDX, 0, 0x0000000f, 1 }, { X86_FEATURE_CQM_MBM_TOTAL, CPUID_EDX, 1, 0x0000000f, 1 }, -- 2.31.1 ^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH v2 4/6] x86/cpu: Read/save PPIN MSR during initialization 2022-01-21 17:47 ` [PATCH v2 0/6] PPIN (Protected Processor Inventory Number) updates Tony Luck ` (2 preceding siblings ...) 2022-01-21 17:47 ` [PATCH v2 3/6] x86/cpu: X86_FEATURE_INTEL_PPIN finally has a CPUID bit Tony Luck @ 2022-01-21 17:47 ` Tony Luck 2022-01-21 17:47 ` [PATCH v2 5/6] topology/sysfs: Add format parameter to macro defining "show" functions for proc Tony Luck ` (3 subsequent siblings) 7 siblings, 0 replies; 42+ messages in thread From: Tony Luck @ 2022-01-21 17:47 UTC (permalink / raw) To: Borislav Petkov Cc: x86, linux-kernel, Greg Kroah-Hartman, Smita Koralahalli Channabasappa, Wei Huang, Tom Lendacky, patches, Tony Luck Currently the PPIN (Protected Processor Inventory Number) MSR is read by every CPU that processes a machine check, CMCI, or just polls machine check banks from a periodic timer. This is not a "fast" MSR, so this adds to overhead of processing errors. Add a new "ppin" field to the cpuinfo_x86 structure. Read and save the PPIN during initialization. Use this copy in mce_setup() instead of reading the MSR. Signed-off-by: Tony Luck <tony.luck@intel.com> --- arch/x86/include/asm/processor.h | 2 ++ arch/x86/kernel/cpu/common.c | 2 ++ arch/x86/kernel/cpu/mce/core.c | 7 +------ 3 files changed, 5 insertions(+), 6 deletions(-) diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h index 2c5f12ae7d04..a87e7c33d5ac 100644 --- a/arch/x86/include/asm/processor.h +++ b/arch/x86/include/asm/processor.h @@ -119,6 +119,8 @@ struct cpuinfo_x86 { int x86_cache_mbm_width_offset; int x86_power; unsigned long loops_per_jiffy; + /* protected processor identification number */ + u64 ppin; /* cpuid returned max cores value: */ u16 x86_max_cores; u16 apicid; diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c index 8a039d1ea57f..64deb7727d00 100644 --- a/arch/x86/kernel/cpu/common.c +++ b/arch/x86/kernel/cpu/common.c @@ -91,6 +91,7 @@ DEFINE_PER_CPU_READ_MOSTLY(u16, cpu_l2c_id) = BAD_APICID; static struct ppin_info { int feature; int msr_ppin_ctl; + int msr_ppin; } ppin_info[] = { [X86_VENDOR_INTEL] = { .feature = X86_FEATURE_INTEL_PPIN, @@ -155,6 +156,7 @@ static void ppin_init(struct cpuinfo_x86 *c) /* Is the enable bit set? */ if (val & 2UL) { + c->ppin = __rdmsr(info->msr_ppin); set_cpu_cap(c, info->feature); return; } diff --git a/arch/x86/kernel/cpu/mce/core.c b/arch/x86/kernel/cpu/mce/core.c index 5818b837fd4d..4f1e825033ce 100644 --- a/arch/x86/kernel/cpu/mce/core.c +++ b/arch/x86/kernel/cpu/mce/core.c @@ -138,12 +138,7 @@ void mce_setup(struct mce *m) m->socketid = cpu_data(m->extcpu).phys_proc_id; m->apicid = cpu_data(m->extcpu).initial_apicid; m->mcgcap = __rdmsr(MSR_IA32_MCG_CAP); - - if (this_cpu_has(X86_FEATURE_INTEL_PPIN)) - m->ppin = __rdmsr(MSR_PPIN); - else if (this_cpu_has(X86_FEATURE_AMD_PPIN)) - m->ppin = __rdmsr(MSR_AMD_PPIN); - + m->ppin = cpu_data(m->extcpu).ppin; m->microcode = boot_cpu_data.microcode; } -- 2.31.1 ^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH v2 5/6] topology/sysfs: Add format parameter to macro defining "show" functions for proc 2022-01-21 17:47 ` [PATCH v2 0/6] PPIN (Protected Processor Inventory Number) updates Tony Luck ` (3 preceding siblings ...) 2022-01-21 17:47 ` [PATCH v2 4/6] x86/cpu: Read/save PPIN MSR during initialization Tony Luck @ 2022-01-21 17:47 ` Tony Luck 2022-01-31 11:34 ` Greg Kroah-Hartman 2022-01-21 17:47 ` [PATCH v2 6/6] topology/sysfs: Add PPIN in sysfs under cpu topology Tony Luck ` (2 subsequent siblings) 7 siblings, 1 reply; 42+ messages in thread From: Tony Luck @ 2022-01-21 17:47 UTC (permalink / raw) To: Borislav Petkov Cc: x86, linux-kernel, Greg Kroah-Hartman, Smita Koralahalli Channabasappa, Wei Huang, Tom Lendacky, patches, Tony Luck All the simple (non-mask and non-list files in /sys/devices/system/cpu/cpu0/topology/ are currently printed as decimal integers. Refactor the macro that generates the "show" functions to take a format parameter to allow future files to display in other formats. No functional change. Signed-off-by: Tony Luck <tony.luck@intel.com> --- drivers/base/topology.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/drivers/base/topology.c b/drivers/base/topology.c index fc24e89f9592..044f3664f8f2 100644 --- a/drivers/base/topology.c +++ b/drivers/base/topology.c @@ -14,11 +14,11 @@ #include <linux/hardirq.h> #include <linux/topology.h> -#define define_id_show_func(name) \ +#define define_id_show_func(name, fmt) \ static ssize_t name##_show(struct device *dev, \ struct device_attribute *attr, char *buf) \ { \ - return sysfs_emit(buf, "%d\n", topology_##name(dev->id)); \ + return sysfs_emit(buf, fmt "\n", topology_##name(dev->id)); \ } #define define_siblings_read_func(name, mask) \ @@ -42,20 +42,20 @@ static ssize_t name##_list_read(struct file *file, struct kobject *kobj, \ off, count); \ } -define_id_show_func(physical_package_id); +define_id_show_func(physical_package_id, "%d"); static DEVICE_ATTR_RO(physical_package_id); #ifdef TOPOLOGY_DIE_SYSFS -define_id_show_func(die_id); +define_id_show_func(die_id, "%d"); static DEVICE_ATTR_RO(die_id); #endif #ifdef TOPOLOGY_CLUSTER_SYSFS -define_id_show_func(cluster_id); +define_id_show_func(cluster_id, "%d"); static DEVICE_ATTR_RO(cluster_id); #endif -define_id_show_func(core_id); +define_id_show_func(core_id, "%d"); static DEVICE_ATTR_RO(core_id); define_siblings_read_func(thread_siblings, sibling_cpumask); @@ -87,7 +87,7 @@ static BIN_ATTR_RO(package_cpus, 0); static BIN_ATTR_RO(package_cpus_list, 0); #ifdef TOPOLOGY_BOOK_SYSFS -define_id_show_func(book_id); +define_id_show_func(book_id, "%d"); static DEVICE_ATTR_RO(book_id); define_siblings_read_func(book_siblings, book_cpumask); static BIN_ATTR_RO(book_siblings, 0); @@ -95,7 +95,7 @@ static BIN_ATTR_RO(book_siblings_list, 0); #endif #ifdef TOPOLOGY_DRAWER_SYSFS -define_id_show_func(drawer_id); +define_id_show_func(drawer_id, "%d"); static DEVICE_ATTR_RO(drawer_id); define_siblings_read_func(drawer_siblings, drawer_cpumask); static BIN_ATTR_RO(drawer_siblings, 0); -- 2.31.1 ^ permalink raw reply related [flat|nested] 42+ messages in thread
* Re: [PATCH v2 5/6] topology/sysfs: Add format parameter to macro defining "show" functions for proc 2022-01-21 17:47 ` [PATCH v2 5/6] topology/sysfs: Add format parameter to macro defining "show" functions for proc Tony Luck @ 2022-01-31 11:34 ` Greg Kroah-Hartman 0 siblings, 0 replies; 42+ messages in thread From: Greg Kroah-Hartman @ 2022-01-31 11:34 UTC (permalink / raw) To: Tony Luck Cc: Borislav Petkov, x86, linux-kernel, Smita Koralahalli Channabasappa, Wei Huang, Tom Lendacky, patches On Fri, Jan 21, 2022 at 09:47:42AM -0800, Tony Luck wrote: > All the simple (non-mask and non-list files in > /sys/devices/system/cpu/cpu0/topology/ are currently printed as decimal > integers. > > Refactor the macro that generates the "show" functions to take a format > parameter to allow future files to display in other formats. > > No functional change. > > Signed-off-by: Tony Luck <tony.luck@intel.com> > --- > drivers/base/topology.c | 16 ++++++++-------- > 1 file changed, 8 insertions(+), 8 deletions(-) Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> ^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v2 6/6] topology/sysfs: Add PPIN in sysfs under cpu topology 2022-01-21 17:47 ` [PATCH v2 0/6] PPIN (Protected Processor Inventory Number) updates Tony Luck ` (4 preceding siblings ...) 2022-01-21 17:47 ` [PATCH v2 5/6] topology/sysfs: Add format parameter to macro defining "show" functions for proc Tony Luck @ 2022-01-21 17:47 ` Tony Luck 2022-01-31 11:35 ` Greg Kroah-Hartman 2022-01-31 12:31 ` [PATCH v2 0/6] PPIN (Protected Processor Inventory Number) updates Borislav Petkov 2022-01-31 23:01 ` [PATCH v3 0/5] PPIN (Protected Processor Inventory Number) updates Tony Luck 7 siblings, 1 reply; 42+ messages in thread From: Tony Luck @ 2022-01-21 17:47 UTC (permalink / raw) To: Borislav Petkov Cc: x86, linux-kernel, Greg Kroah-Hartman, Smita Koralahalli Channabasappa, Wei Huang, Tom Lendacky, patches, Tony Luck PPIN is the Protected Processor Identification Number. This is used to identify the socket as a Field Replaceable Unit (FRU). Existing code only displays this when reporting errors. But this makes it inconvenient for large clusters to use it for its intended purpose of inventory control. Add ppin to /sys/devices/system/cpu/cpu*/topology to make what is already available using RDMSR more easily accessible. Make the file read only for root in case there are still people concerned about making a unique system "serial number" available. Signed-off-by: Tony Luck <tony.luck@intel.com> --- Documentation/ABI/stable/sysfs-devices-system-cpu | 4 ++++ Documentation/ABI/testing/sysfs-devices-system-cpu | 6 ++++++ arch/x86/include/asm/topology.h | 1 + drivers/base/topology.c | 4 ++++ include/linux/topology.h | 3 +++ 5 files changed, 18 insertions(+) diff --git a/Documentation/ABI/stable/sysfs-devices-system-cpu b/Documentation/ABI/stable/sysfs-devices-system-cpu index 3965ce504484..902392d7eddf 100644 --- a/Documentation/ABI/stable/sysfs-devices-system-cpu +++ b/Documentation/ABI/stable/sysfs-devices-system-cpu @@ -86,6 +86,10 @@ What: /sys/devices/system/cpu/cpuX/topology/die_cpus Description: internal kernel map of CPUs within the same die. Values: hexadecimal bitmask. +What: /sys/devices/system/cpu/cpuX/topology/ppin +Description: per-socket protected processor inventory number +Values: hexadecimal. + What: /sys/devices/system/cpu/cpuX/topology/die_cpus_list Description: human-readable list of CPUs within the same die. The format is like 0-3, 8-11, 14,17. diff --git a/Documentation/ABI/testing/sysfs-devices-system-cpu b/Documentation/ABI/testing/sysfs-devices-system-cpu index 61f5676a7429..74962c200790 100644 --- a/Documentation/ABI/testing/sysfs-devices-system-cpu +++ b/Documentation/ABI/testing/sysfs-devices-system-cpu @@ -73,6 +73,7 @@ What: /sys/devices/system/cpu/cpuX/topology/core_id /sys/devices/system/cpu/cpuX/topology/physical_package_id /sys/devices/system/cpu/cpuX/topology/thread_siblings /sys/devices/system/cpu/cpuX/topology/thread_siblings_list + /sys/devices/system/cpu/cpuX/topology/ppin Date: December 2008 Contact: Linux kernel mailing list <linux-kernel@vger.kernel.org> Description: CPU topology files that describe a logical CPU's relationship @@ -103,6 +104,11 @@ Description: CPU topology files that describe a logical CPU's relationship thread_siblings_list: human-readable list of cpuX's hardware threads within the same core as cpuX + ppin: human-readable Protected Processor Identification + Number of the socket the cpu# belongs to. There should be + one per physical_package_id. File is readable only to + admin. + See Documentation/admin-guide/cputopology.rst for more information. diff --git a/arch/x86/include/asm/topology.h b/arch/x86/include/asm/topology.h index 2f0b6be8eaab..43a89476a522 100644 --- a/arch/x86/include/asm/topology.h +++ b/arch/x86/include/asm/topology.h @@ -110,6 +110,7 @@ extern const struct cpumask *cpu_clustergroup_mask(int cpu); #define topology_logical_die_id(cpu) (cpu_data(cpu).logical_die_id) #define topology_die_id(cpu) (cpu_data(cpu).cpu_die_id) #define topology_core_id(cpu) (cpu_data(cpu).cpu_core_id) +#define topology_ppin(cpu) (cpu_data(cpu).ppin) extern unsigned int __max_die_per_package; diff --git a/drivers/base/topology.c b/drivers/base/topology.c index 044f3664f8f2..e9d1efcda89b 100644 --- a/drivers/base/topology.c +++ b/drivers/base/topology.c @@ -58,6 +58,9 @@ static DEVICE_ATTR_RO(cluster_id); define_id_show_func(core_id, "%d"); static DEVICE_ATTR_RO(core_id); +define_id_show_func(ppin, "0x%llx"); +static DEVICE_ATTR_ADMIN_RO(ppin); + define_siblings_read_func(thread_siblings, sibling_cpumask); static BIN_ATTR_RO(thread_siblings, 0); static BIN_ATTR_RO(thread_siblings_list, 0); @@ -145,6 +148,7 @@ static struct attribute *default_attrs[] = { #ifdef TOPOLOGY_DRAWER_SYSFS &dev_attr_drawer_id.attr, #endif + &dev_attr_ppin.attr, NULL }; diff --git a/include/linux/topology.h b/include/linux/topology.h index a6e201758ae9..f19bc3626297 100644 --- a/include/linux/topology.h +++ b/include/linux/topology.h @@ -211,6 +211,9 @@ static inline int cpu_to_mem(int cpu) #ifndef topology_drawer_id #define topology_drawer_id(cpu) ((void)(cpu), -1) #endif +#ifndef topology_ppin +#define topology_ppin(cpu) ((void)(cpu), 0ull) +#endif #ifndef topology_sibling_cpumask #define topology_sibling_cpumask(cpu) cpumask_of(cpu) #endif -- 2.31.1 ^ permalink raw reply related [flat|nested] 42+ messages in thread
* Re: [PATCH v2 6/6] topology/sysfs: Add PPIN in sysfs under cpu topology 2022-01-21 17:47 ` [PATCH v2 6/6] topology/sysfs: Add PPIN in sysfs under cpu topology Tony Luck @ 2022-01-31 11:35 ` Greg Kroah-Hartman 0 siblings, 0 replies; 42+ messages in thread From: Greg Kroah-Hartman @ 2022-01-31 11:35 UTC (permalink / raw) To: Tony Luck Cc: Borislav Petkov, x86, linux-kernel, Smita Koralahalli Channabasappa, Wei Huang, Tom Lendacky, patches On Fri, Jan 21, 2022 at 09:47:43AM -0800, Tony Luck wrote: > PPIN is the Protected Processor Identification Number. > This is used to identify the socket as a Field Replaceable Unit (FRU). > > Existing code only displays this when reporting errors. But this makes > it inconvenient for large clusters to use it for its intended purpose > of inventory control. > > Add ppin to /sys/devices/system/cpu/cpu*/topology to make what > is already available using RDMSR more easily accessible. Make > the file read only for root in case there are still people > concerned about making a unique system "serial number" available. > > Signed-off-by: Tony Luck <tony.luck@intel.com> > --- > Documentation/ABI/stable/sysfs-devices-system-cpu | 4 ++++ > Documentation/ABI/testing/sysfs-devices-system-cpu | 6 ++++++ > arch/x86/include/asm/topology.h | 1 + > drivers/base/topology.c | 4 ++++ > include/linux/topology.h | 3 +++ > 5 files changed, 18 insertions(+) Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v2 0/6] PPIN (Protected Processor Inventory Number) updates 2022-01-21 17:47 ` [PATCH v2 0/6] PPIN (Protected Processor Inventory Number) updates Tony Luck ` (5 preceding siblings ...) 2022-01-21 17:47 ` [PATCH v2 6/6] topology/sysfs: Add PPIN in sysfs under cpu topology Tony Luck @ 2022-01-31 12:31 ` Borislav Petkov 2022-01-31 17:23 ` Luck, Tony 2022-01-31 23:01 ` [PATCH v3 0/5] PPIN (Protected Processor Inventory Number) updates Tony Luck 7 siblings, 1 reply; 42+ messages in thread From: Borislav Petkov @ 2022-01-31 12:31 UTC (permalink / raw) To: Tony Luck Cc: x86, linux-kernel, Greg Kroah-Hartman, Smita Koralahalli Channabasappa, Wei Huang, Tom Lendacky, patches On Fri, Jan 21, 2022 at 09:47:37AM -0800, Tony Luck wrote: > ... They look good so far on my PPIN-enabled AMD box. > 5) Add "ppin" to /sys/devices/system/cpu/cpu*/topology/ppin > > The big question for this part is whether there is a better > place to expose this value. I'm open to other suggestions. Yeah, I'm not sure about that either. I have $ grep -r . /sys/devices/system/cpu/cpu*/topology/ppin | cut -d: -f 2 | uniq -c 32 0xxxxx 32 times the same number. Wouldn't /sys/devices/system/node/ be a better place? Even if those were logical nodes, it would still be less needless replication and that would be one more way for root to figure out which logical nodes belong to the same physical package... :-) Hmm. -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v2 0/6] PPIN (Protected Processor Inventory Number) updates 2022-01-31 12:31 ` [PATCH v2 0/6] PPIN (Protected Processor Inventory Number) updates Borislav Petkov @ 2022-01-31 17:23 ` Luck, Tony 2022-01-31 18:18 ` Borislav Petkov 0 siblings, 1 reply; 42+ messages in thread From: Luck, Tony @ 2022-01-31 17:23 UTC (permalink / raw) To: Borislav Petkov Cc: x86, linux-kernel, Greg Kroah-Hartman, Smita Koralahalli Channabasappa, Wei Huang, Tom Lendacky, patches On Mon, Jan 31, 2022 at 01:31:49PM +0100, Borislav Petkov wrote: > On Fri, Jan 21, 2022 at 09:47:37AM -0800, Tony Luck wrote: > > ... > > They look good so far on my PPIN-enabled AMD box. > > > 5) Add "ppin" to /sys/devices/system/cpu/cpu*/topology/ppin > > > > The big question for this part is whether there is a better > > place to expose this value. I'm open to other suggestions. > > Yeah, I'm not sure about that either. I have > > $ grep -r . /sys/devices/system/cpu/cpu*/topology/ppin | cut -d: -f 2 | uniq -c > 32 0xxxxx > > 32 times the same number. > > Wouldn't > > /sys/devices/system/node/ > > be a better place? Maybe. > Even if those were logical nodes, it would still be less needless > replication and that would be one more way for root to figure out which > logical nodes belong to the same physical package... :-) That would work for existing products. There are some cases where a single package appears as mutiple nodes. But that's ok ... as you say, one more clue to the topology. I'm worried that some future thing might reverse that and have a "package" id for each die in a multi-die package which still appears as a single node. That would distort the meaning of "package", so it isn't supposed to happen. But if it did, Linux would be stuck just reporting one of the "package" ids. -Tony ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v2 0/6] PPIN (Protected Processor Inventory Number) updates 2022-01-31 17:23 ` Luck, Tony @ 2022-01-31 18:18 ` Borislav Petkov 2022-01-31 18:49 ` Luck, Tony 0 siblings, 1 reply; 42+ messages in thread From: Borislav Petkov @ 2022-01-31 18:18 UTC (permalink / raw) To: Luck, Tony Cc: x86, linux-kernel, Greg Kroah-Hartman, Smita Koralahalli Channabasappa, Wei Huang, Tom Lendacky, patches On Mon, Jan 31, 2022 at 09:23:20AM -0800, Luck, Tony wrote: > I'm worried that some future thing might reverse that and have > a "package" id for each die in a multi-die package which still > appears as a single node. That would distort the meaning of "package", > so it isn't supposed to happen. But if it did, Linux would be stuck > just reporting one of the "package" ids. Hmm, so we write that we don't really care about the physical socket in software: Documentation/x86/topology.rst: "The kernel does not care about the concept of physical sockets because a socket has no relevance to software. It's an electromechanical component. In the past a socket always contained a single package (see below), but with the advent of Multi Chip Modules (MCM) a socket can hold more than one package. So there might be still references to sockets in the code, but they are of historical nature and should be cleaned up." and the PPIN is a physical socket property. So there's no proper way for us to tie to anything that represents the physical socket. So, the use case you're imagining would be, what? The FRU code glue would go: "I got an MCE on CPU X... Lemme see which PPIN is it: # cat /sys/devices/system/cpu/cpuX/topology/ppin BLA ah ok, lemme report it: You just had an MCE on CPU X, socket BLA" Something like that? But that FRU glue software would have to run as root so that it reads the ppin sysfs file. But we don't want to expose that processor serial number to !root users so we're forcing the people to run the FRU thing as root. This feels like this guy here: https://c.tenor.com/fDZOE4okO3EAAAAC/homer-simpsons.gif -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v2 0/6] PPIN (Protected Processor Inventory Number) updates 2022-01-31 18:18 ` Borislav Petkov @ 2022-01-31 18:49 ` Luck, Tony 2022-01-31 19:10 ` Borislav Petkov 0 siblings, 1 reply; 42+ messages in thread From: Luck, Tony @ 2022-01-31 18:49 UTC (permalink / raw) To: Borislav Petkov Cc: x86, linux-kernel, Greg Kroah-Hartman, Smita Koralahalli Channabasappa, Wei Huang, Tom Lendacky, patches On Mon, Jan 31, 2022 at 07:18:46PM +0100, Borislav Petkov wrote: > On Mon, Jan 31, 2022 at 09:23:20AM -0800, Luck, Tony wrote: > > I'm worried that some future thing might reverse that and have > > a "package" id for each die in a multi-die package which still > > appears as a single node. That would distort the meaning of "package", > > so it isn't supposed to happen. But if it did, Linux would be stuck > > just reporting one of the "package" ids. > > Hmm, so we write that we don't really care about the physical socket in > software: > > Documentation/x86/topology.rst: > "The kernel does not care about the concept of physical sockets because > a socket has no relevance to software. It's an electromechanical > component. In the past a socket always contained a single package > (see below), but with the advent of Multi Chip Modules (MCM) a socket > can hold more than one package. So there might be still references to > sockets in the code, but they are of historical nature and should be > cleaned up." > > and the PPIN is a physical socket property. So there's no proper way for > us to tie to anything that represents the physical socket. > > So, the use case you're imagining would be, what? > > The FRU code glue would go: > > "I got an MCE on CPU X... > > Lemme see which PPIN is it: > > # cat /sys/devices/system/cpu/cpuX/topology/ppin > BLA > > ah ok, lemme report it: > > You just had an MCE on CPU X, socket BLA" > > Something like that? Yes. We also have a new thing (patches coming soon) other than MCE that would follow that same flow. So in general it is: "problem with CPU X, go read the ppin file for that CPU to file the report". For the new thing, only root can trigger it, so not a problem for root to create the report. > But that FRU glue software would have to run as root so that it reads > the ppin sysfs file. > > But we don't want to expose that processor serial number to !root users > so we're forcing the people to run the FRU thing as root. > > This feels like this guy here: > > https://c.tenor.com/fDZOE4okO3EAAAAC/homer-simpsons.gif I think any paranoia about having a user readable "serial number" should be gone by now. Those wacky web folks found a dozen different ways to track your every move on the internet so that adverts for whatever you just searched for will follow you for days. It seems highly unlikely that browser writers will bother reading ppin and adding it to cookies. But I didn't want to get distracted by that, so made the file mode 0400. -Tony ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v2 0/6] PPIN (Protected Processor Inventory Number) updates 2022-01-31 18:49 ` Luck, Tony @ 2022-01-31 19:10 ` Borislav Petkov 2022-01-31 19:29 ` Luck, Tony 0 siblings, 1 reply; 42+ messages in thread From: Borislav Petkov @ 2022-01-31 19:10 UTC (permalink / raw) To: Luck, Tony Cc: x86, linux-kernel, Greg Kroah-Hartman, Smita Koralahalli Channabasappa, Wei Huang, Tom Lendacky, patches On Mon, Jan 31, 2022 at 10:49:32AM -0800, Luck, Tony wrote: > I think any paranoia about having a user readable "serial number" > should be gone by now. Those wacky web folks found a dozen different ways > to track your every move on the internet so that adverts for whatever > you just searched for will follow you for days. It seems highly > unlikely that browser writers will bother reading ppin and adding it > to cookies. > > But I didn't want to get distracted by that, so made the file mode 0400. So by that logic, having it root-only would be only nuisance for FRU or whatever software accesses it, so why not simply make it readable by everyone then? Lemme be clear: I'm being the devil's advocate here on purpose because I want to make sure we don't walk into some privacy thing we haven't thought about at the time. So I guess 0400, root:root would be the correct thing to do - admins can then change permissions later or so. Rather than making it readable by everyone by default and leaving it to people to tighten it after boot. Hmmm. -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette ^ permalink raw reply [flat|nested] 42+ messages in thread
* RE: [PATCH v2 0/6] PPIN (Protected Processor Inventory Number) updates 2022-01-31 19:10 ` Borislav Petkov @ 2022-01-31 19:29 ` Luck, Tony 2022-01-31 21:58 ` Borislav Petkov 0 siblings, 1 reply; 42+ messages in thread From: Luck, Tony @ 2022-01-31 19:29 UTC (permalink / raw) To: Borislav Petkov Cc: x86, linux-kernel, Greg Kroah-Hartman, Smita Koralahalli Channabasappa, Wei Huang, Tom Lendacky, patches > Lemme be clear: I'm being the devil's advocate here on purpose because > I want to make sure we don't walk into some privacy thing we haven't > thought about at the time. Sure. It's good to look at this from other perspectives. There may be some software-as-a-service thing where the provider of the service doesn't want a simple way to reveal that jobs are being migrated around a pool of systems. > So I guess 0400, root:root would be the correct thing to do - admins can > then change permissions later or so. Rather than making it readable by > everyone by default and leaving it to people to tighten it after boot. Yup. If someone has a tool that needs ppin, but they don't want to run as root they can just add either of: chown notrootadmin /sys/devices/system/cpu/cpu*/topology/ppin or chmod 444 /sys/devices/system/cpu/cpu*/topology/ppin to some /etc/rc* file. -Tony ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v2 0/6] PPIN (Protected Processor Inventory Number) updates 2022-01-31 19:29 ` Luck, Tony @ 2022-01-31 21:58 ` Borislav Petkov 2022-01-31 22:03 ` Luck, Tony 2022-04-06 22:01 ` [PATCH] topology/sysfs: Hide PPIN on systems that do not support it Tony Luck 0 siblings, 2 replies; 42+ messages in thread From: Borislav Petkov @ 2022-01-31 21:58 UTC (permalink / raw) To: Luck, Tony Cc: x86, linux-kernel, Greg Kroah-Hartman, Smita Koralahalli Channabasappa, Wei Huang, Tom Lendacky, patches On Mon, Jan 31, 2022 at 07:29:55PM +0000, Luck, Tony wrote: > Yup. If someone has a tool that needs ppin, but they don't want to run > as root they can just add either of: ... Ok then. I guess I can queue your next version and we'll see what happens. Thx. -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette ^ permalink raw reply [flat|nested] 42+ messages in thread
* RE: [PATCH v2 0/6] PPIN (Protected Processor Inventory Number) updates 2022-01-31 21:58 ` Borislav Petkov @ 2022-01-31 22:03 ` Luck, Tony 2022-04-06 22:01 ` [PATCH] topology/sysfs: Hide PPIN on systems that do not support it Tony Luck 1 sibling, 0 replies; 42+ messages in thread From: Luck, Tony @ 2022-01-31 22:03 UTC (permalink / raw) To: Borislav Petkov Cc: x86, linux-kernel, Greg Kroah-Hartman, Smita Koralahalli Channabasappa, Wei Huang, Tom Lendacky, patches > Ok then. I guess I can queue your next version and we'll see what > happens. Thanks. I'll move those bogus initializations of .msr_ppin to the right patch. Add Greg's Ack's and repost a new version after re-testing. -Tony ^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH] topology/sysfs: Hide PPIN on systems that do not support it. 2022-01-31 21:58 ` Borislav Petkov 2022-01-31 22:03 ` Luck, Tony @ 2022-04-06 22:01 ` Tony Luck 2022-04-25 22:46 ` Andrew Morton 1 sibling, 1 reply; 42+ messages in thread From: Tony Luck @ 2022-04-06 22:01 UTC (permalink / raw) To: Borislav Petkov Cc: x86, linux-kernel, Greg Kroah-Hartman, Smita Koralahalli Channabasappa, Wei Huang, Tom Lendacky, patches, Tony Luck Systems that do not support a Protected Processor Identification Number currently report: # cat /sys/devices/system/cpu/cpu0/topology/ppin 0x0 which is confusing/wrong. Add a ".is_visible" function to suppress inclusion of the ppin file. Fixes: ab28e944197f ("topology/sysfs: Add PPIN in sysfs under cpu topology") Signed-off-by: Tony Luck <tony.luck@intel.com> --- drivers/base/topology.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/drivers/base/topology.c b/drivers/base/topology.c index e9d1efcda89b..706dbf8bf249 100644 --- a/drivers/base/topology.c +++ b/drivers/base/topology.c @@ -152,9 +152,21 @@ static struct attribute *default_attrs[] = { NULL }; +static umode_t topology_is_visible(struct kobject *kobj, + struct attribute *attr, int unused) +{ + struct device *dev = kobj_to_dev(kobj); + + if (attr == &dev_attr_ppin.attr && !topology_ppin(dev->id)) + return 0; + + return attr->mode; +} + static const struct attribute_group topology_attr_group = { .attrs = default_attrs, .bin_attrs = bin_attrs, + .is_visible = topology_is_visible, .name = "topology" }; -- 2.35.1 ^ permalink raw reply related [flat|nested] 42+ messages in thread
* Re: [PATCH] topology/sysfs: Hide PPIN on systems that do not support it. 2022-04-06 22:01 ` [PATCH] topology/sysfs: Hide PPIN on systems that do not support it Tony Luck @ 2022-04-25 22:46 ` Andrew Morton 2022-04-25 22:56 ` Luck, Tony 0 siblings, 1 reply; 42+ messages in thread From: Andrew Morton @ 2022-04-25 22:46 UTC (permalink / raw) To: Tony Luck Cc: Borislav Petkov, x86, linux-kernel, Greg Kroah-Hartman, Smita Koralahalli Channabasappa, Wei Huang, Tom Lendacky, patches On Wed, 6 Apr 2022 15:01:50 -0700 Tony Luck <tony.luck@intel.com> wrote: > Systems that do not support a Protected Processor Identification Number > currently report: > > # cat /sys/devices/system/cpu/cpu0/topology/ppin > 0x0 > > which is confusing/wrong. > > Add a ".is_visible" function to suppress inclusion of the ppin file. > > --- a/drivers/base/topology.c > +++ b/drivers/base/topology.c > @@ -152,9 +152,21 @@ static struct attribute *default_attrs[] = { > NULL > }; > > +static umode_t topology_is_visible(struct kobject *kobj, > + struct attribute *attr, int unused) > +{ > + struct device *dev = kobj_to_dev(kobj); > + > + if (attr == &dev_attr_ppin.attr && !topology_ppin(dev->id)) > + return 0; > + > + return attr->mode; > +} > + > static const struct attribute_group topology_attr_group = { > .attrs = default_attrs, > .bin_attrs = bin_attrs, > + .is_visible = topology_is_visible, > .name = "topology" > }; x86_64 allnoconfig: drivers/base/topology.c: In function 'topology_is_visible': drivers/base/topology.c:158:24: warning: unused variable 'dev' [-Wunused-variable] 158 | struct device *dev = kobj_to_dev(kobj); | ^~~ I suggest this be fixed in the topology_ppin() stub implementation. Do it as a nice inlined C function which avoids such problems. Rather than as a crappy macro which causes them... ^ permalink raw reply [flat|nested] 42+ messages in thread
* RE: [PATCH] topology/sysfs: Hide PPIN on systems that do not support it. 2022-04-25 22:46 ` Andrew Morton @ 2022-04-25 22:56 ` Luck, Tony 0 siblings, 0 replies; 42+ messages in thread From: Luck, Tony @ 2022-04-25 22:56 UTC (permalink / raw) To: Andrew Morton Cc: Borislav Petkov, x86, linux-kernel, Greg Kroah-Hartman, Smita Koralahalli Channabasappa, Wei Huang, Tom Lendacky, patches > I suggest this be fixed in the topology_ppin() stub implementation. Do > it as a nice inlined C function which avoids such problems. Rather > than as a crappy macro which causes them... Greg already fixed this in his tree by dropping the local variable. Fix should propagate to linux-next today. -Tony ^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v3 0/5] PPIN (Protected Processor Inventory Number) updates 2022-01-21 17:47 ` [PATCH v2 0/6] PPIN (Protected Processor Inventory Number) updates Tony Luck ` (6 preceding siblings ...) 2022-01-31 12:31 ` [PATCH v2 0/6] PPIN (Protected Processor Inventory Number) updates Borislav Petkov @ 2022-01-31 23:01 ` Tony Luck 2022-01-31 23:01 ` [PATCH v3 1/5] x86/cpu: Merge Intel and AMD ppin_init() functions Tony Luck ` (4 more replies) 7 siblings, 5 replies; 42+ messages in thread From: Tony Luck @ 2022-01-31 23:01 UTC (permalink / raw) To: Borislav Petkov Cc: x86, linux-kernel, Greg Kroah-Hartman, Smita Koralahalli Channabasappa, Wei Huang, Tom Lendacky, patches, Tony Luck The series starts out with two changes that I expect are uncontroversial. Later parts get progressively more "RFC" - but seem to have survived discussions on LKML. 1) Simple cleanup to merge Intel and AMD duplicated code to test for presence of PPIN and check whether it is enabled. 2) Long overdue update from Intel to enumerate the PPIN and PPIN_CTL MSRs. See the December 2021 Software Developers Manual 3) Code to scan machine check banks re-reads the PPIN every time banks are scanned (whether for a machine check, a CMCI, or just a periodic poll). Since PPIN never changes, this seems like unnecessary overhead. Read the MSR once (per CPU) and save to memory. 4) Refactor as prep for last part. 5) Add "ppin" to /sys/devices/system/cpu/cpu*/topology/ppin Use case: ppin is already useful when reporting machine check errors. Patches for new "in field scan" tests will appear soon. If a CPU fails such a test it will be useful to provide the ppin along with the test failure data. Changes since V2: ---------------- 1) Part 1 of V2 (adding ICELAKE_D) merged upstream, so dropped here 2) Rebased to v5.17-rc2 (to build on top of that merged patch) 3) Fixed bisection breakage reported by Boris where .msr_ppin structure fields were initialized two patches before the field was added. 4) Add Ack's from GregKH to parts 4 & 5 Tony Luck (5): x86/cpu: Merge Intel and AMD ppin_init() functions x86/cpu: X86_FEATURE_INTEL_PPIN finally has a CPUID bit x86/cpu: Read/save PPIN MSR during initialization topology/sysfs: Add format parameter to macro defining "show" functions for proc topology/sysfs: Add PPIN in sysfs under cpu topology .../ABI/stable/sysfs-devices-system-cpu | 4 + .../ABI/testing/sysfs-devices-system-cpu | 6 ++ arch/x86/include/asm/processor.h | 2 + arch/x86/include/asm/topology.h | 1 + arch/x86/kernel/cpu/amd.c | 30 ------- arch/x86/kernel/cpu/common.c | 79 +++++++++++++++++++ arch/x86/kernel/cpu/mce/core.c | 7 +- arch/x86/kernel/cpu/mce/intel.c | 42 ---------- arch/x86/kernel/cpu/scattered.c | 1 + drivers/base/topology.c | 20 +++-- include/linux/topology.h | 3 + 11 files changed, 109 insertions(+), 86 deletions(-) base-commit: 26291c54e111ff6ba87a164d85d4a4e134b7315c -- 2.31.1 ^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v3 1/5] x86/cpu: Merge Intel and AMD ppin_init() functions 2022-01-31 23:01 ` [PATCH v3 0/5] PPIN (Protected Processor Inventory Number) updates Tony Luck @ 2022-01-31 23:01 ` Tony Luck 2022-01-31 23:01 ` [PATCH v3 2/5] x86/cpu: X86_FEATURE_INTEL_PPIN finally has a CPUID bit Tony Luck ` (3 subsequent siblings) 4 siblings, 0 replies; 42+ messages in thread From: Tony Luck @ 2022-01-31 23:01 UTC (permalink / raw) To: Borislav Petkov Cc: x86, linux-kernel, Greg Kroah-Hartman, Smita Koralahalli Channabasappa, Wei Huang, Tom Lendacky, patches, Tony Luck The code to decide whether a system supports the PPIN (Protected Processor Inventory Number) MSR was cloned from the Intel implementation. Apart from the X86_FEATURE bit and the MSR numbers it is identical. Merge the two functions into common x86 code, but use x86_match_cpu() instead of the switch (c->x86_model) that was used by the old Intel code. No functional change. Signed-off-by: Tony Luck <tony.luck@intel.com> --- arch/x86/kernel/cpu/amd.c | 30 ------------- arch/x86/kernel/cpu/common.c | 74 +++++++++++++++++++++++++++++++++ arch/x86/kernel/cpu/mce/intel.c | 42 ------------------- 3 files changed, 74 insertions(+), 72 deletions(-) diff --git a/arch/x86/kernel/cpu/amd.c b/arch/x86/kernel/cpu/amd.c index 4edb6f0f628c..bad0fa4c1779 100644 --- a/arch/x86/kernel/cpu/amd.c +++ b/arch/x86/kernel/cpu/amd.c @@ -394,35 +394,6 @@ static void amd_detect_cmp(struct cpuinfo_x86 *c) per_cpu(cpu_llc_id, cpu) = c->cpu_die_id = c->phys_proc_id; } -static void amd_detect_ppin(struct cpuinfo_x86 *c) -{ - unsigned long long val; - - if (!cpu_has(c, X86_FEATURE_AMD_PPIN)) - return; - - /* When PPIN is defined in CPUID, still need to check PPIN_CTL MSR */ - if (rdmsrl_safe(MSR_AMD_PPIN_CTL, &val)) - goto clear_ppin; - - /* PPIN is locked in disabled mode, clear feature bit */ - if ((val & 3UL) == 1UL) - goto clear_ppin; - - /* If PPIN is disabled, try to enable it */ - if (!(val & 2UL)) { - wrmsrl_safe(MSR_AMD_PPIN_CTL, val | 2UL); - rdmsrl_safe(MSR_AMD_PPIN_CTL, &val); - } - - /* If PPIN_EN bit is 1, return from here; otherwise fall through */ - if (val & 2UL) - return; - -clear_ppin: - clear_cpu_cap(c, X86_FEATURE_AMD_PPIN); -} - u32 amd_get_nodes_per_socket(void) { return nodes_per_socket; @@ -947,7 +918,6 @@ static void init_amd(struct cpuinfo_x86 *c) amd_detect_cmp(c); amd_get_topology(c); srat_detect_node(c); - amd_detect_ppin(c); init_amd_cacheinfo(c); diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c index 7b8382c11788..b0bd8a6b5beb 100644 --- a/arch/x86/kernel/cpu/common.c +++ b/arch/x86/kernel/cpu/common.c @@ -88,6 +88,78 @@ EXPORT_SYMBOL_GPL(get_llc_id); /* L2 cache ID of each logical CPU */ DEFINE_PER_CPU_READ_MOSTLY(u16, cpu_l2c_id) = BAD_APICID; +static struct ppin_info { + int feature; + int msr_ppin_ctl; +} ppin_info[] = { + [X86_VENDOR_INTEL] = { + .feature = X86_FEATURE_INTEL_PPIN, + .msr_ppin_ctl = MSR_PPIN_CTL, + }, + [X86_VENDOR_AMD] = { + .feature = X86_FEATURE_AMD_PPIN, + .msr_ppin_ctl = MSR_AMD_PPIN_CTL, + }, +}; + +static const struct x86_cpu_id ppin_cpuids[] = { + X86_MATCH_FEATURE(X86_FEATURE_AMD_PPIN, &ppin_info[X86_VENDOR_AMD]), + + /* Legacy models without CPUID enumeration */ + X86_MATCH_INTEL_FAM6_MODEL(IVYBRIDGE_X, &ppin_info[X86_VENDOR_INTEL]), + X86_MATCH_INTEL_FAM6_MODEL(HASWELL_X, &ppin_info[X86_VENDOR_INTEL]), + X86_MATCH_INTEL_FAM6_MODEL(BROADWELL_D, &ppin_info[X86_VENDOR_INTEL]), + X86_MATCH_INTEL_FAM6_MODEL(BROADWELL_X, &ppin_info[X86_VENDOR_INTEL]), + X86_MATCH_INTEL_FAM6_MODEL(SKYLAKE_X, &ppin_info[X86_VENDOR_INTEL]), + X86_MATCH_INTEL_FAM6_MODEL(ICELAKE_X, &ppin_info[X86_VENDOR_INTEL]), + X86_MATCH_INTEL_FAM6_MODEL(ICELAKE_D, &ppin_info[X86_VENDOR_INTEL]), + X86_MATCH_INTEL_FAM6_MODEL(SAPPHIRERAPIDS_X, &ppin_info[X86_VENDOR_INTEL]), + X86_MATCH_INTEL_FAM6_MODEL(XEON_PHI_KNL, &ppin_info[X86_VENDOR_INTEL]), + X86_MATCH_INTEL_FAM6_MODEL(XEON_PHI_KNM, &ppin_info[X86_VENDOR_INTEL]), + + {} +}; + +static void ppin_init(struct cpuinfo_x86 *c) +{ + const struct x86_cpu_id *id; + unsigned long long val; + struct ppin_info *info; + + id = x86_match_cpu(ppin_cpuids); + if (!id) + return; + + /* + * Testing the presence of the MSR is not enough. Need to check + * that the PPIN_CTL allows reading of the PPIN. + */ + info = (struct ppin_info *)id->driver_data; + + if (rdmsrl_safe(info->msr_ppin_ctl, &val)) + goto clear_ppin; + + if ((val & 3UL) == 1UL) { + /* PPIN locked in disabled mode */ + goto clear_ppin; + } + + /* If PPIN is disabled, try to enable */ + if (!(val & 2UL)) { + wrmsrl_safe(info->msr_ppin_ctl, val | 2UL); + rdmsrl_safe(info->msr_ppin_ctl, &val); + } + + /* Is the enable bit set? */ + if (val & 2UL) { + set_cpu_cap(c, info->feature); + return; + } + +clear_ppin: + clear_cpu_cap(c, info->feature); +} + /* correctly size the local cpu masks */ void __init setup_cpu_local_masks(void) { @@ -1655,6 +1727,8 @@ static void identify_cpu(struct cpuinfo_x86 *c) c->x86_capability[i] |= boot_cpu_data.x86_capability[i]; } + ppin_init(c); + /* Init Machine Check Exception if available. */ mcheck_cpu_init(c); diff --git a/arch/x86/kernel/cpu/mce/intel.c b/arch/x86/kernel/cpu/mce/intel.c index baafbb37be67..95275a5e57e0 100644 --- a/arch/x86/kernel/cpu/mce/intel.c +++ b/arch/x86/kernel/cpu/mce/intel.c @@ -470,47 +470,6 @@ void intel_clear_lmce(void) wrmsrl(MSR_IA32_MCG_EXT_CTL, val); } -static void intel_ppin_init(struct cpuinfo_x86 *c) -{ - unsigned long long val; - - /* - * Even if testing the presence of the MSR would be enough, we don't - * want to risk the situation where other models reuse this MSR for - * other purposes. - */ - switch (c->x86_model) { - case INTEL_FAM6_IVYBRIDGE_X: - case INTEL_FAM6_HASWELL_X: - case INTEL_FAM6_BROADWELL_D: - case INTEL_FAM6_BROADWELL_X: - case INTEL_FAM6_SKYLAKE_X: - case INTEL_FAM6_ICELAKE_X: - case INTEL_FAM6_ICELAKE_D: - case INTEL_FAM6_SAPPHIRERAPIDS_X: - case INTEL_FAM6_XEON_PHI_KNL: - case INTEL_FAM6_XEON_PHI_KNM: - - if (rdmsrl_safe(MSR_PPIN_CTL, &val)) - return; - - if ((val & 3UL) == 1UL) { - /* PPIN locked in disabled mode */ - return; - } - - /* If PPIN is disabled, try to enable */ - if (!(val & 2UL)) { - wrmsrl_safe(MSR_PPIN_CTL, val | 2UL); - rdmsrl_safe(MSR_PPIN_CTL, &val); - } - - /* Is the enable bit set? */ - if (val & 2UL) - set_cpu_cap(c, X86_FEATURE_INTEL_PPIN); - } -} - /* * Enable additional error logs from the integrated * memory controller on processors that support this. @@ -535,7 +494,6 @@ void mce_intel_feature_init(struct cpuinfo_x86 *c) { intel_init_cmci(); intel_init_lmce(); - intel_ppin_init(c); intel_imc_init(c); } -- 2.31.1 ^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH v3 2/5] x86/cpu: X86_FEATURE_INTEL_PPIN finally has a CPUID bit 2022-01-31 23:01 ` [PATCH v3 0/5] PPIN (Protected Processor Inventory Number) updates Tony Luck 2022-01-31 23:01 ` [PATCH v3 1/5] x86/cpu: Merge Intel and AMD ppin_init() functions Tony Luck @ 2022-01-31 23:01 ` Tony Luck 2022-01-31 23:01 ` [PATCH v3 3/5] x86/cpu: Read/save PPIN MSR during initialization Tony Luck ` (2 subsequent siblings) 4 siblings, 0 replies; 42+ messages in thread From: Tony Luck @ 2022-01-31 23:01 UTC (permalink / raw) To: Borislav Petkov Cc: x86, linux-kernel, Greg Kroah-Hartman, Smita Koralahalli Channabasappa, Wei Huang, Tom Lendacky, patches, Tony Luck After nine generations of adding to model specific list of CPUs that support PPIN (Protected Processor Inventory Number) Intel allocated a CPUID bit to enumerate the MSRs. CPUID(EAX=7, ECX=1).EBX bit 0 enumerates presence of MSR_PPIN_CTL and MSR_PPIN. Add it to the "scattered" CPUID bits and add an entry to the ppin_cpuids[] x86_match_cpu() array to catch Intel CPUs that implement it. Signed-off-by: Tony Luck <tony.luck@intel.com> --- arch/x86/kernel/cpu/common.c | 1 + arch/x86/kernel/cpu/scattered.c | 1 + 2 files changed, 2 insertions(+) diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c index b0bd8a6b5beb..0681c69a1f09 100644 --- a/arch/x86/kernel/cpu/common.c +++ b/arch/x86/kernel/cpu/common.c @@ -104,6 +104,7 @@ static struct ppin_info { static const struct x86_cpu_id ppin_cpuids[] = { X86_MATCH_FEATURE(X86_FEATURE_AMD_PPIN, &ppin_info[X86_VENDOR_AMD]), + X86_MATCH_FEATURE(X86_FEATURE_INTEL_PPIN, &ppin_info[X86_VENDOR_INTEL]), /* Legacy models without CPUID enumeration */ X86_MATCH_INTEL_FAM6_MODEL(IVYBRIDGE_X, &ppin_info[X86_VENDOR_INTEL]), diff --git a/arch/x86/kernel/cpu/scattered.c b/arch/x86/kernel/cpu/scattered.c index 21d1f062895a..4143b1e4c5c6 100644 --- a/arch/x86/kernel/cpu/scattered.c +++ b/arch/x86/kernel/cpu/scattered.c @@ -26,6 +26,7 @@ struct cpuid_bit { static const struct cpuid_bit cpuid_bits[] = { { X86_FEATURE_APERFMPERF, CPUID_ECX, 0, 0x00000006, 0 }, { X86_FEATURE_EPB, CPUID_ECX, 3, 0x00000006, 0 }, + { X86_FEATURE_INTEL_PPIN, CPUID_EBX, 0, 0x00000007, 1 }, { X86_FEATURE_CQM_LLC, CPUID_EDX, 1, 0x0000000f, 0 }, { X86_FEATURE_CQM_OCCUP_LLC, CPUID_EDX, 0, 0x0000000f, 1 }, { X86_FEATURE_CQM_MBM_TOTAL, CPUID_EDX, 1, 0x0000000f, 1 }, -- 2.31.1 ^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH v3 3/5] x86/cpu: Read/save PPIN MSR during initialization 2022-01-31 23:01 ` [PATCH v3 0/5] PPIN (Protected Processor Inventory Number) updates Tony Luck 2022-01-31 23:01 ` [PATCH v3 1/5] x86/cpu: Merge Intel and AMD ppin_init() functions Tony Luck 2022-01-31 23:01 ` [PATCH v3 2/5] x86/cpu: X86_FEATURE_INTEL_PPIN finally has a CPUID bit Tony Luck @ 2022-01-31 23:01 ` Tony Luck 2022-01-31 23:01 ` [PATCH v3 4/5] topology/sysfs: Add format parameter to macro defining "show" functions for proc Tony Luck 2022-01-31 23:01 ` [PATCH v3 5/5] topology/sysfs: Add PPIN in sysfs under cpu topology Tony Luck 4 siblings, 0 replies; 42+ messages in thread From: Tony Luck @ 2022-01-31 23:01 UTC (permalink / raw) To: Borislav Petkov Cc: x86, linux-kernel, Greg Kroah-Hartman, Smita Koralahalli Channabasappa, Wei Huang, Tom Lendacky, patches, Tony Luck Currently the PPIN (Protected Processor Inventory Number) MSR is read by every CPU that processes a machine check, CMCI, or just polls machine check banks from a periodic timer. This is not a "fast" MSR, so this adds to overhead of processing errors. Add a new "ppin" field to the cpuinfo_x86 structure. Read and save the PPIN during initialization. Use this copy in mce_setup() instead of reading the MSR. Signed-off-by: Tony Luck <tony.luck@intel.com> --- arch/x86/include/asm/processor.h | 2 ++ arch/x86/kernel/cpu/common.c | 4 ++++ arch/x86/kernel/cpu/mce/core.c | 7 +------ 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h index 2c5f12ae7d04..a87e7c33d5ac 100644 --- a/arch/x86/include/asm/processor.h +++ b/arch/x86/include/asm/processor.h @@ -119,6 +119,8 @@ struct cpuinfo_x86 { int x86_cache_mbm_width_offset; int x86_power; unsigned long loops_per_jiffy; + /* protected processor identification number */ + u64 ppin; /* cpuid returned max cores value: */ u16 x86_max_cores; u16 apicid; diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c index 0681c69a1f09..64deb7727d00 100644 --- a/arch/x86/kernel/cpu/common.c +++ b/arch/x86/kernel/cpu/common.c @@ -91,14 +91,17 @@ DEFINE_PER_CPU_READ_MOSTLY(u16, cpu_l2c_id) = BAD_APICID; static struct ppin_info { int feature; int msr_ppin_ctl; + int msr_ppin; } ppin_info[] = { [X86_VENDOR_INTEL] = { .feature = X86_FEATURE_INTEL_PPIN, .msr_ppin_ctl = MSR_PPIN_CTL, + .msr_ppin = MSR_PPIN }, [X86_VENDOR_AMD] = { .feature = X86_FEATURE_AMD_PPIN, .msr_ppin_ctl = MSR_AMD_PPIN_CTL, + .msr_ppin = MSR_AMD_PPIN }, }; @@ -153,6 +156,7 @@ static void ppin_init(struct cpuinfo_x86 *c) /* Is the enable bit set? */ if (val & 2UL) { + c->ppin = __rdmsr(info->msr_ppin); set_cpu_cap(c, info->feature); return; } diff --git a/arch/x86/kernel/cpu/mce/core.c b/arch/x86/kernel/cpu/mce/core.c index 5818b837fd4d..4f1e825033ce 100644 --- a/arch/x86/kernel/cpu/mce/core.c +++ b/arch/x86/kernel/cpu/mce/core.c @@ -138,12 +138,7 @@ void mce_setup(struct mce *m) m->socketid = cpu_data(m->extcpu).phys_proc_id; m->apicid = cpu_data(m->extcpu).initial_apicid; m->mcgcap = __rdmsr(MSR_IA32_MCG_CAP); - - if (this_cpu_has(X86_FEATURE_INTEL_PPIN)) - m->ppin = __rdmsr(MSR_PPIN); - else if (this_cpu_has(X86_FEATURE_AMD_PPIN)) - m->ppin = __rdmsr(MSR_AMD_PPIN); - + m->ppin = cpu_data(m->extcpu).ppin; m->microcode = boot_cpu_data.microcode; } -- 2.31.1 ^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH v3 4/5] topology/sysfs: Add format parameter to macro defining "show" functions for proc 2022-01-31 23:01 ` [PATCH v3 0/5] PPIN (Protected Processor Inventory Number) updates Tony Luck ` (2 preceding siblings ...) 2022-01-31 23:01 ` [PATCH v3 3/5] x86/cpu: Read/save PPIN MSR during initialization Tony Luck @ 2022-01-31 23:01 ` Tony Luck 2022-01-31 23:01 ` [PATCH v3 5/5] topology/sysfs: Add PPIN in sysfs under cpu topology Tony Luck 4 siblings, 0 replies; 42+ messages in thread From: Tony Luck @ 2022-01-31 23:01 UTC (permalink / raw) To: Borislav Petkov Cc: x86, linux-kernel, Greg Kroah-Hartman, Smita Koralahalli Channabasappa, Wei Huang, Tom Lendacky, patches, Tony Luck All the simple (non-mask and non-list files in /sys/devices/system/cpu/cpu0/topology/ are currently printed as decimal integers. Refactor the macro that generates the "show" functions to take a format parameter to allow future files to display in other formats. No functional change. Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Signed-off-by: Tony Luck <tony.luck@intel.com> --- drivers/base/topology.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/drivers/base/topology.c b/drivers/base/topology.c index fc24e89f9592..044f3664f8f2 100644 --- a/drivers/base/topology.c +++ b/drivers/base/topology.c @@ -14,11 +14,11 @@ #include <linux/hardirq.h> #include <linux/topology.h> -#define define_id_show_func(name) \ +#define define_id_show_func(name, fmt) \ static ssize_t name##_show(struct device *dev, \ struct device_attribute *attr, char *buf) \ { \ - return sysfs_emit(buf, "%d\n", topology_##name(dev->id)); \ + return sysfs_emit(buf, fmt "\n", topology_##name(dev->id)); \ } #define define_siblings_read_func(name, mask) \ @@ -42,20 +42,20 @@ static ssize_t name##_list_read(struct file *file, struct kobject *kobj, \ off, count); \ } -define_id_show_func(physical_package_id); +define_id_show_func(physical_package_id, "%d"); static DEVICE_ATTR_RO(physical_package_id); #ifdef TOPOLOGY_DIE_SYSFS -define_id_show_func(die_id); +define_id_show_func(die_id, "%d"); static DEVICE_ATTR_RO(die_id); #endif #ifdef TOPOLOGY_CLUSTER_SYSFS -define_id_show_func(cluster_id); +define_id_show_func(cluster_id, "%d"); static DEVICE_ATTR_RO(cluster_id); #endif -define_id_show_func(core_id); +define_id_show_func(core_id, "%d"); static DEVICE_ATTR_RO(core_id); define_siblings_read_func(thread_siblings, sibling_cpumask); @@ -87,7 +87,7 @@ static BIN_ATTR_RO(package_cpus, 0); static BIN_ATTR_RO(package_cpus_list, 0); #ifdef TOPOLOGY_BOOK_SYSFS -define_id_show_func(book_id); +define_id_show_func(book_id, "%d"); static DEVICE_ATTR_RO(book_id); define_siblings_read_func(book_siblings, book_cpumask); static BIN_ATTR_RO(book_siblings, 0); @@ -95,7 +95,7 @@ static BIN_ATTR_RO(book_siblings_list, 0); #endif #ifdef TOPOLOGY_DRAWER_SYSFS -define_id_show_func(drawer_id); +define_id_show_func(drawer_id, "%d"); static DEVICE_ATTR_RO(drawer_id); define_siblings_read_func(drawer_siblings, drawer_cpumask); static BIN_ATTR_RO(drawer_siblings, 0); -- 2.31.1 ^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH v3 5/5] topology/sysfs: Add PPIN in sysfs under cpu topology 2022-01-31 23:01 ` [PATCH v3 0/5] PPIN (Protected Processor Inventory Number) updates Tony Luck ` (3 preceding siblings ...) 2022-01-31 23:01 ` [PATCH v3 4/5] topology/sysfs: Add format parameter to macro defining "show" functions for proc Tony Luck @ 2022-01-31 23:01 ` Tony Luck 4 siblings, 0 replies; 42+ messages in thread From: Tony Luck @ 2022-01-31 23:01 UTC (permalink / raw) To: Borislav Petkov Cc: x86, linux-kernel, Greg Kroah-Hartman, Smita Koralahalli Channabasappa, Wei Huang, Tom Lendacky, patches, Tony Luck PPIN is the Protected Processor Identification Number. This is used to identify the socket as a Field Replaceable Unit (FRU). Existing code only displays this when reporting errors. But this makes it inconvenient for large clusters to use it for its intended purpose of inventory control. Add ppin to /sys/devices/system/cpu/cpu*/topology to make what is already available using RDMSR more easily accessible. Make the file read only for root in case there are still people concerned about making a unique system "serial number" available. Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Signed-off-by: Tony Luck <tony.luck@intel.com> --- Documentation/ABI/stable/sysfs-devices-system-cpu | 4 ++++ Documentation/ABI/testing/sysfs-devices-system-cpu | 6 ++++++ arch/x86/include/asm/topology.h | 1 + drivers/base/topology.c | 4 ++++ include/linux/topology.h | 3 +++ 5 files changed, 18 insertions(+) diff --git a/Documentation/ABI/stable/sysfs-devices-system-cpu b/Documentation/ABI/stable/sysfs-devices-system-cpu index 3965ce504484..902392d7eddf 100644 --- a/Documentation/ABI/stable/sysfs-devices-system-cpu +++ b/Documentation/ABI/stable/sysfs-devices-system-cpu @@ -86,6 +86,10 @@ What: /sys/devices/system/cpu/cpuX/topology/die_cpus Description: internal kernel map of CPUs within the same die. Values: hexadecimal bitmask. +What: /sys/devices/system/cpu/cpuX/topology/ppin +Description: per-socket protected processor inventory number +Values: hexadecimal. + What: /sys/devices/system/cpu/cpuX/topology/die_cpus_list Description: human-readable list of CPUs within the same die. The format is like 0-3, 8-11, 14,17. diff --git a/Documentation/ABI/testing/sysfs-devices-system-cpu b/Documentation/ABI/testing/sysfs-devices-system-cpu index 61f5676a7429..74962c200790 100644 --- a/Documentation/ABI/testing/sysfs-devices-system-cpu +++ b/Documentation/ABI/testing/sysfs-devices-system-cpu @@ -73,6 +73,7 @@ What: /sys/devices/system/cpu/cpuX/topology/core_id /sys/devices/system/cpu/cpuX/topology/physical_package_id /sys/devices/system/cpu/cpuX/topology/thread_siblings /sys/devices/system/cpu/cpuX/topology/thread_siblings_list + /sys/devices/system/cpu/cpuX/topology/ppin Date: December 2008 Contact: Linux kernel mailing list <linux-kernel@vger.kernel.org> Description: CPU topology files that describe a logical CPU's relationship @@ -103,6 +104,11 @@ Description: CPU topology files that describe a logical CPU's relationship thread_siblings_list: human-readable list of cpuX's hardware threads within the same core as cpuX + ppin: human-readable Protected Processor Identification + Number of the socket the cpu# belongs to. There should be + one per physical_package_id. File is readable only to + admin. + See Documentation/admin-guide/cputopology.rst for more information. diff --git a/arch/x86/include/asm/topology.h b/arch/x86/include/asm/topology.h index 2f0b6be8eaab..43a89476a522 100644 --- a/arch/x86/include/asm/topology.h +++ b/arch/x86/include/asm/topology.h @@ -110,6 +110,7 @@ extern const struct cpumask *cpu_clustergroup_mask(int cpu); #define topology_logical_die_id(cpu) (cpu_data(cpu).logical_die_id) #define topology_die_id(cpu) (cpu_data(cpu).cpu_die_id) #define topology_core_id(cpu) (cpu_data(cpu).cpu_core_id) +#define topology_ppin(cpu) (cpu_data(cpu).ppin) extern unsigned int __max_die_per_package; diff --git a/drivers/base/topology.c b/drivers/base/topology.c index 044f3664f8f2..e9d1efcda89b 100644 --- a/drivers/base/topology.c +++ b/drivers/base/topology.c @@ -58,6 +58,9 @@ static DEVICE_ATTR_RO(cluster_id); define_id_show_func(core_id, "%d"); static DEVICE_ATTR_RO(core_id); +define_id_show_func(ppin, "0x%llx"); +static DEVICE_ATTR_ADMIN_RO(ppin); + define_siblings_read_func(thread_siblings, sibling_cpumask); static BIN_ATTR_RO(thread_siblings, 0); static BIN_ATTR_RO(thread_siblings_list, 0); @@ -145,6 +148,7 @@ static struct attribute *default_attrs[] = { #ifdef TOPOLOGY_DRAWER_SYSFS &dev_attr_drawer_id.attr, #endif + &dev_attr_ppin.attr, NULL }; diff --git a/include/linux/topology.h b/include/linux/topology.h index a6e201758ae9..f19bc3626297 100644 --- a/include/linux/topology.h +++ b/include/linux/topology.h @@ -211,6 +211,9 @@ static inline int cpu_to_mem(int cpu) #ifndef topology_drawer_id #define topology_drawer_id(cpu) ((void)(cpu), -1) #endif +#ifndef topology_ppin +#define topology_ppin(cpu) ((void)(cpu), 0ull) +#endif #ifndef topology_sibling_cpumask #define topology_sibling_cpumask(cpu) cpumask_of(cpu) #endif -- 2.31.1 ^ permalink raw reply related [flat|nested] 42+ messages in thread
end of thread, other threads:[~2022-04-25 22:56 UTC | newest] Thread overview: 42+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-01-07 22:54 [PATCH 0/5] PPIN (Protected Processor Inventory Number) updates Tony Luck 2022-01-07 22:54 ` [PATCH 1/5] x86/ras: Merge Intel and AMD ppin_init() functions Tony Luck 2022-01-18 20:02 ` Borislav Petkov 2022-01-18 21:03 ` Luck, Tony 2022-01-18 21:15 ` Borislav Petkov 2022-01-07 22:54 ` [PATCH 2/5] x86/ras: X86_FEATURE_INTEL_PPIN finally has a CPUID bit Tony Luck 2022-01-20 13:32 ` Borislav Petkov 2022-01-07 22:54 ` [PATCH 3/5] x86/ras: Read/save PPIN MSR during initialization Tony Luck 2022-01-07 22:54 ` [PATCH 4/5] x86/sysfs: Add format parameter to macro defining "show" functions for proc Tony Luck 2022-01-20 13:32 ` Borislav Petkov 2022-01-07 22:54 ` [PATCH 5/5] x86/sysfs: Add PPIN in sysfs under cpu topology Tony Luck 2022-01-20 13:35 ` Borislav Petkov 2022-01-20 18:01 ` Luck, Tony 2022-01-20 22:48 ` Borislav Petkov 2022-01-21 17:47 ` [PATCH v2 0/6] PPIN (Protected Processor Inventory Number) updates Tony Luck 2022-01-21 17:47 ` [PATCH v2 1/6] x86/cpu: Add Xeon Icelake-D to list of CPUs that support PPIN Tony Luck 2022-01-21 17:47 ` [PATCH v2 2/6] x86/cpu: Merge Intel and AMD ppin_init() functions Tony Luck 2022-01-27 10:22 ` Borislav Petkov 2022-01-27 16:52 ` Luck, Tony 2022-01-21 17:47 ` [PATCH v2 3/6] x86/cpu: X86_FEATURE_INTEL_PPIN finally has a CPUID bit Tony Luck 2022-01-21 17:47 ` [PATCH v2 4/6] x86/cpu: Read/save PPIN MSR during initialization Tony Luck 2022-01-21 17:47 ` [PATCH v2 5/6] topology/sysfs: Add format parameter to macro defining "show" functions for proc Tony Luck 2022-01-31 11:34 ` Greg Kroah-Hartman 2022-01-21 17:47 ` [PATCH v2 6/6] topology/sysfs: Add PPIN in sysfs under cpu topology Tony Luck 2022-01-31 11:35 ` Greg Kroah-Hartman 2022-01-31 12:31 ` [PATCH v2 0/6] PPIN (Protected Processor Inventory Number) updates Borislav Petkov 2022-01-31 17:23 ` Luck, Tony 2022-01-31 18:18 ` Borislav Petkov 2022-01-31 18:49 ` Luck, Tony 2022-01-31 19:10 ` Borislav Petkov 2022-01-31 19:29 ` Luck, Tony 2022-01-31 21:58 ` Borislav Petkov 2022-01-31 22:03 ` Luck, Tony 2022-04-06 22:01 ` [PATCH] topology/sysfs: Hide PPIN on systems that do not support it Tony Luck 2022-04-25 22:46 ` Andrew Morton 2022-04-25 22:56 ` Luck, Tony 2022-01-31 23:01 ` [PATCH v3 0/5] PPIN (Protected Processor Inventory Number) updates Tony Luck 2022-01-31 23:01 ` [PATCH v3 1/5] x86/cpu: Merge Intel and AMD ppin_init() functions Tony Luck 2022-01-31 23:01 ` [PATCH v3 2/5] x86/cpu: X86_FEATURE_INTEL_PPIN finally has a CPUID bit Tony Luck 2022-01-31 23:01 ` [PATCH v3 3/5] x86/cpu: Read/save PPIN MSR during initialization Tony Luck 2022-01-31 23:01 ` [PATCH v3 4/5] topology/sysfs: Add format parameter to macro defining "show" functions for proc Tony Luck 2022-01-31 23:01 ` [PATCH v3 5/5] topology/sysfs: Add PPIN in sysfs under cpu topology Tony Luck
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).