Skip to contentSkip to conversation

PPIN (Protected Processor Inventory Number) updates

PATCH Superseded Tony Luck 5 participants 48 messages Jan 7, 2022 – Apr 25, 2022

Subsystems (4)
lore
Share & tools
lore

Tony Luck

author maintainer
#0 lore raw

cover PPIN (Protected Processor Inventory Number) updates

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

Signature · 1 line
2.31.1
author maintainer
#1 lore raw

PATCH 1/5 x86/ras: Merge Intel and AMD ppin_init() functions

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.

---
 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(-)
arch/x86/kernel/cpu/amd.c +0 -30
index 4edb6f0f628c..bad0fa4c1779 100644
@@ -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);
 
arch/x86/kernel/cpu/common.c +76 -0

88 lines not on this page · raw message

arch/x86/kernel/cpu/mce/intel.c +0 -41

53 lines not on this page · raw message

Signature · 1 line
2.31.1
maintainer
#2 lore raw

Re: [PATCH 1/5] x86/ras: Merge Intel and AMD ppin_init() functions

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.

arch/x86/kernel/cpu/common.c EXPORT_SYMBOL_GPL(get_llc_id); · line 94

Quoted 12 lines
arch/x86/kernel/cpu/common.c +4 -0
index 0083464de5e3..a1e29c0844d1 100644
@@ -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.

arch/x86/kernel/cpu/common.c EXPORT_SYMBOL_GPL(get_llc_id); · line 101

Quoted 7 lines
arch/x86/kernel/cpu/common.c +7 -0
+} 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? :)

arch/x86/kernel/cpu/common.c EXPORT_SYMBOL_GPL(get_llc_id); · line 109

Quoted 8 lines
arch/x86/kernel/cpu/common.c +8 -0
+		.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.

Quoted 16 lines
arch/x86/kernel/cpu/common.c +16 -0
+	/* 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)
+{

arch/x86/kernel/cpu/common.c EXPORT_SYMBOL_GPL(get_llc_id); · line 136

Quoted 10 lines
arch/x86/kernel/cpu/common.c +10 -0
+	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']

Quoted 32 lines
arch/x86/kernel/cpu/common.c +28 -0
+	 * 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)
 {

arch/x86/kernel/cpu/common.c static void identify_cpu(struct cpuinfo_x86 *c) · line 1732

Quoted 4 lines
arch/x86/kernel/cpu/common.c +1 -0
@@ -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.

Signature · 4 lines
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette
author maintainer
#3 lore raw

Re: [PATCH 1/5] x86/ras: Merge Intel and AMD ppin_init() functions

Quoted 11 lines · Borislav Petkov

arch/x86/kernel/cpu/common.c EXPORT_SYMBOL_GPL(get_llc_id); · line 101

Quoted 7 lines · Tony Luck
arch/x86/kernel/cpu/common.c +7 -0
+} 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.

Quoted 6 lines

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

maintainer
#4 lore raw

Re: [PATCH 1/5] x86/ras: Merge Intel and AMD ppin_init() functions

Quoted 2 lines · Tony

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

Quoted 2 lines

Is it worth the risk/churn just to save 12 bytes in the ppin_info[] array?

It doesn't look like it.

Quoted 2 lines

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.

Signature · 4 lines
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette
author maintainer
#5 lore raw

PATCH 2/5 x86/ras: X86_FEATURE_INTEL_PPIN finally has a CPUID bit

In reply to 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.

---
 arch/x86/kernel/cpu/common.c    | 1 +
 arch/x86/kernel/cpu/scattered.c | 1 +
 2 files changed, 2 insertions(+)
arch/x86/kernel/cpu/common.c +1 -0
index a1e29c0844d1..3688f70ee0a2 100644
@@ -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]),
arch/x86/kernel/cpu/scattered.c +1 -0
index 21d1f062895a..4143b1e4c5c6 100644
@@ -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 },
Signature · 1 line
2.31.1
maintainer
#6 lore raw

Re: [PATCH 2/5] x86/ras: X86_FEATURE_INTEL_PPIN finally has a CPUID bit

Quoted 1 line · Tony Luck

After nine generations of adding to model specific list of CPUS that

CPUs

Signature · 4 lines
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette
author maintainer
#7 lore raw

PATCH 3/5 x86/ras: Read/save PPIN MSR during initialization

In reply to 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.

---
 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(-)
arch/x86/include/asm/processor.h +2 -0
index 355d38c0cf60..1db5bb3413ae 100644
@@ -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;
arch/x86/kernel/cpu/common.c +1 -0
index 3688f70ee0a2..9a90f96257d1 100644
@@ -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;
 	}
arch/x86/kernel/cpu/mce/core.c +1 -6
index 6ed365337a3b..ec38c0c6c235 100644
@@ -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;
 }
 
Signature · 1 line
2.31.1
author maintainer
#8 lore raw

PATCH 4/5 x86/sysfs: Add format parameter to macro defining "show" functions for proc

In reply to 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.

---
 drivers/base/topology.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)
drivers/base/topology.c +8 -8
index 8f2b641d0b8c..793c592e533a 100644
@@ -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);
Signature · 1 line
2.31.1
maintainer
#9 lore raw

Re: [PATCH 4/5] x86/sysfs: Add format parameter to macro defining "show" functions for proc

Quoted 7 lines · 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.

Quoted 6 lines

No functional change.

---
 drivers/base/topology.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

This one doesn't apply anymore.

Signature · 4 lines
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette
author maintainer
#10 lore raw

PATCH 5/5 x86/sysfs: Add PPIN in sysfs under cpu topology

In reply to 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".

---
 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(+)
Documentation/ABI/stable/sysfs-devices-system-cpu +4 -0
index 3965ce504484..902392d7eddf 100644
@@ -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.
Documentation/ABI/testing/sysfs-devices-system-cpu +6 -0
index 69c65da16dff..18af7459adf8 100644
@@ -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 <[email protected]>
 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.
 
 
arch/x86/include/asm/topology.h +1 -0
index cc164777e661..caba7db8c7b6 100644
@@ -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;
 
drivers/base/topology.c +4 -0
index 793c592e533a..4c8674715d36 100644
@@ -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
 };
 
include/linux/topology.h +3 -0
index 0b3704ad13c8..789a900039e0 100644
@@ -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
Signature · 1 line
2.31.1
maintainer
#11 lore raw

Re: [PATCH 5/5] x86/sysfs: Add PPIN in sysfs under cpu topology

Quoted 6 lines · 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.

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.

Quoted 6 lines

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.

Quoted 1 line

2) The PPIN MSR is may be implemented on some desktop/laptop parts.

s/is //

Quoted 2 lines

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. :-)

Quoted 4 lines

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.

Quoted 14 lines
---
 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(+)
drivers/base/topology.c +0 -0
index 793c592e533a..4c8674715d36 100644

drivers/base/topology.c static DEVICE_ATTR_RO(cluster_id); · line 57

Quoted 4 lines
drivers/base/topology.c +1 -0
@@ -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.

arch/x86/include/asm/topology.h extern const struct cpumask *cpu_clustergroup_mask(int cpu); · line 113

Quoted 9 lines
arch/x86/include/asm/topology.h +1 -0
index cc164777e661..caba7db8c7b6 100644
@@ -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.

Signature · 4 lines
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette
author maintainer
#12 lore raw

Re: [PATCH 5/5] x86/sysfs: Add PPIN in sysfs under cpu topology

Quoted 4 lines · Borislav Petkov
Quoted 1 line · Tony Luck

+#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

maintainer
#13 lore raw

Re: [PATCH 5/5] x86/sysfs: Add PPIN in sysfs under cpu topology

Quoted 2 lines · Tony

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.

Signature · 4 lines
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette
author maintainer
#14 lore raw

v2 cover PPIN (Protected Processor Inventory Number) updates

In reply to 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

Signature · 1 line
2.31.1
author maintainer
#15 lore raw

PATCH v2 1/6 x86/cpu: Add Xeon Icelake-D to list of CPUs that support PPIN

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.

Fixes dc6b025de95b ("x86/mce: Add Xeon Icelake to list of CPUs that support PPIN")
---
 arch/x86/kernel/cpu/mce/intel.c | 1 +
 1 file changed, 1 insertion(+)
arch/x86/kernel/cpu/mce/intel.c +1 -0
index bb9a46a804bf..baafbb37be67 100644
@@ -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:
Signature · 1 line
2.31.1
merge bot Applied
#16 lore raw

[tip: x86/urgent] x86/cpu: Add Xeon Icelake-D to list of CPUs that support PPIN

The following commit has been merged into the x86/urgent branch of tip:

The following commit has been merged into the x86/urgent branch of tip:

Commit-ID:     e464121f2d40eabc7d11823fb26db807ce945df4
Gitweb:        https://git.kernel.org/tip/e464121f2d40eabc7d11823fb26db807ce945df4
Author:        Tony Luck <[email protected]>
AuthorDate:    Fri, 21 Jan 2022 09:47:38 -08:00
Committer:     Borislav Petkov <[email protected]>
CommitterDate: Tue, 25 Jan 2022 18:40:30 +01:00

x86/cpu: Add Xeon Icelake-D to list of CPUs that support PPIN

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.

Fixes dc6b025de95b ("x86/mce: Add Xeon Icelake to list of CPUs that support PPIN")
---
 arch/x86/kernel/cpu/mce/intel.c | 1 +
 1 file changed, 1 insertion(+)
arch/x86/kernel/cpu/mce/intel.c +1 -0
index bb9a46a..baafbb3 100644
@@ -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:
author maintainer
#17 lore raw

PATCH v2 2/6 x86/cpu: Merge Intel and AMD ppin_init() functions

In reply to 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.

---
 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(-)
arch/x86/kernel/cpu/amd.c +0 -30
index 4edb6f0f628c..bad0fa4c1779 100644
@@ -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);
 
arch/x86/kernel/cpu/common.c +76 -0

88 lines not on this page · raw message

arch/x86/kernel/cpu/mce/intel.c +0 -42

54 lines not on this page · raw message

Signature · 1 line
2.31.1
maintainer
#18 lore raw

Re: [PATCH v2 2/6] x86/cpu: Merge Intel and AMD ppin_init() functions

Quoted 11 lines · Tony Luck
arch/x86/kernel/cpu/common.c +3 -0
index 7b8382c11788..b7700a47eadd 100644
@@ -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;

arch/x86/kernel/cpu/common.c EXPORT_SYMBOL_GPL(get_llc_id); · line 103

Quoted 10 lines
arch/x86/kernel/cpu/common.c +10 -0
+} 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....

Signature · 4 lines
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette
author maintainer
#19 lore raw

RE: [PATCH v2 2/6] x86/cpu: Merge Intel and AMD ppin_init() functions

Quoted 9 lines

arch/x86/kernel/cpu/common.c EXPORT_SYMBOL_GPL(get_llc_id); · line 103

Quoted 4 lines
arch/x86/kernel/cpu/common.c +4 -0
+	[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

author maintainer
#20 lore raw

PATCH v2 3/6 x86/cpu: X86_FEATURE_INTEL_PPIN finally has a CPUID bit

In reply to 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.

---
 arch/x86/kernel/cpu/common.c    | 1 +
 arch/x86/kernel/cpu/scattered.c | 1 +
 2 files changed, 2 insertions(+)
arch/x86/kernel/cpu/common.c +1 -0
index b7700a47eadd..8a039d1ea57f 100644
@@ -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]),
arch/x86/kernel/cpu/scattered.c +1 -0
index 21d1f062895a..4143b1e4c5c6 100644
@@ -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 },
Signature · 1 line
2.31.1
author maintainer
#21 lore raw

PATCH v2 4/6 x86/cpu: Read/save PPIN MSR during initialization

In reply to 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.

---
 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(-)
arch/x86/include/asm/processor.h +2 -0
index 2c5f12ae7d04..a87e7c33d5ac 100644
@@ -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;
arch/x86/kernel/cpu/common.c +2 -0
index 8a039d1ea57f..64deb7727d00 100644
@@ -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;
 	}
arch/x86/kernel/cpu/mce/core.c +1 -6
index 5818b837fd4d..4f1e825033ce 100644
@@ -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;
 }
 
Signature · 1 line
2.31.1
author maintainer
#22 lore raw

PATCH v2 5/6 topology/sysfs: Add format parameter to macro defining "show" functions for proc

In reply to 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.

---
 drivers/base/topology.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)
drivers/base/topology.c +8 -8
index fc24e89f9592..044f3664f8f2 100644
@@ -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);
Signature · 1 line
2.31.1
maintainer Acked-by on v2 5/6
#23 lore raw

Re: [PATCH v2 5/6] topology/sysfs: Add format parameter to macro defining "show" functions for proc

Quoted 7 lines · 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.

Quoted 6 lines

No functional change.

---
 drivers/base/topology.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)
author maintainer
#24 lore raw

PATCH v2 6/6 topology/sysfs: Add PPIN in sysfs under cpu topology

In reply to 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.

---
 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(+)
Documentation/ABI/stable/sysfs-devices-system-cpu +4 -0
index 3965ce504484..902392d7eddf 100644
@@ -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.
Documentation/ABI/testing/sysfs-devices-system-cpu +6 -0
index 61f5676a7429..74962c200790 100644
@@ -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 <[email protected]>
 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.
 
 
arch/x86/include/asm/topology.h +1 -0
index 2f0b6be8eaab..43a89476a522 100644
@@ -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;
 
drivers/base/topology.c +4 -0
index 044f3664f8f2..e9d1efcda89b 100644
@@ -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
 };
 
include/linux/topology.h +3 -0
index a6e201758ae9..f19bc3626297 100644
@@ -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
Signature · 1 line
2.31.1
maintainer Acked-by on v2 6/6
#25 lore raw

Re: [PATCH v2 6/6] topology/sysfs: Add PPIN in sysfs under cpu topology

Quoted 12 lines · 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.

Quoted 8 lines
---
 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(+)
maintainer
#26 lore raw
In reply to Tony Luck
Quoted 1 line · Tony Luck

...

They look good so far on my PPIN-enabled AMD box.

Quoted 4 lines

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.

Signature · 4 lines
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette
author maintainer
#27 lore raw
Quoted 15 lines · Borislav Petkov
Quoted 1 line · Tony Luck

...

They look good so far on my PPIN-enabled AMD box.

Quoted 4 lines

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
Quoted 7 lines

32 times the same number.

Wouldn't

/sys/devices/system/node/

be a better place?

Maybe.

Quoted 3 lines

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

maintainer
#28 lore raw
Quoted 5 lines · Tony

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

Signature · 4 lines
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette
author maintainer
#29 lore raw
Quoted 30 lines · Borislav Petkov
Quoted 5 lines · Tony

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:

Quoted 8 lines
# 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.

Quoted 9 lines

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

maintainer
#30 lore raw
Quoted 8 lines · Tony

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.

Signature · 4 lines
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette
author maintainer
#31 lore raw
Quoted 3 lines

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.

Quoted 3 lines

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

maintainer
#32 lore raw
Quoted 2 lines · Tony

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.

Signature · 4 lines
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette
author maintainer
#33 lore raw
Quoted 2 lines

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

author maintainer
#34 lore raw

PATCH 1/1 topology/sysfs: Hide PPIN on systems that do not support it.

In reply to Borislav Petkov

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")
---
 drivers/base/topology.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)
drivers/base/topology.c +12 -0
index e9d1efcda89b..706dbf8bf249 100644
@@ -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"
 };
 
Signature · 1 line
2.35.1
#35 lore raw

Re: [PATCH] topology/sysfs: Hide PPIN on systems that do not support it.

Quoted 23 lines · On Wed, 6 Apr 2022 15:01:50 -0700 Tony Luck <[email protected]> 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.

drivers/base/topology.c +7 -0
@@ -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;

drivers/base/topology.c static struct attribute *default_attrs[] = { · line 171

Quoted 10 lines
drivers/base/topology.c +5 -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...

author maintainer
#36 lore raw

RE: [PATCH] topology/sysfs: Hide PPIN on systems that do not support it.

Quoted 3 lines

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

author maintainer
#37 lore raw

v3 cover PPIN (Protected Processor Inventory Number) updates

In reply to 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

Signature · 1 line
2.31.1
author maintainer
#38 lore raw

PATCH v3 1/5 x86/cpu: Merge Intel and AMD ppin_init() functions

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.

---
 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(-)
arch/x86/kernel/cpu/amd.c +0 -30
index 4edb6f0f628c..bad0fa4c1779 100644
@@ -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);
 
arch/x86/kernel/cpu/common.c +74 -0

86 lines not on this page · raw message

arch/x86/kernel/cpu/mce/intel.c +0 -42

54 lines not on this page · raw message

Signature · 1 line
2.31.1
merge bot Applied
#39 lore raw

[tip: x86/cpu] x86/cpu: Merge Intel and AMD ppin_init() functions

The following commit has been merged into the x86/cpu branch of tip:

The following commit has been merged into the x86/cpu branch of tip:

Commit-ID:     0dcab41d3487acadf64d0667398e032341bd9918
Gitweb:        https://git.kernel.org/tip/0dcab41d3487acadf64d0667398e032341bd9918
Author:        Tony Luck <[email protected]>
AuthorDate:    Mon, 31 Jan 2022 15:01:07 -08:00
Committer:     Borislav Petkov <[email protected]>
CommitterDate: Tue, 01 Feb 2022 12:56:23 +01:00

x86/cpu: Merge Intel and AMD ppin_init() functions

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.

---
 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(-)
arch/x86/kernel/cpu/amd.c +0 -30
index 4edb6f0..bad0fa4 100644
@@ -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);
 
arch/x86/kernel/cpu/common.c +74 -0

86 lines not on this page · raw message

arch/x86/kernel/cpu/mce/intel.c +0 -42

54 lines not on this page · raw message

author maintainer
#40 lore raw

PATCH v3 2/5 x86/cpu: X86_FEATURE_INTEL_PPIN finally has a CPUID bit

In reply to 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.

---
 arch/x86/kernel/cpu/common.c    | 1 +
 arch/x86/kernel/cpu/scattered.c | 1 +
 2 files changed, 2 insertions(+)
arch/x86/kernel/cpu/common.c +1 -0
index b0bd8a6b5beb..0681c69a1f09 100644
@@ -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]),
arch/x86/kernel/cpu/scattered.c +1 -0
index 21d1f062895a..4143b1e4c5c6 100644
@@ -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 },
Signature · 1 line
2.31.1
merge bot Applied
#41 lore raw

[tip: x86/cpu] x86/cpu: X86_FEATURE_INTEL_PPIN finally has a CPUID bit

The following commit has been merged into the x86/cpu branch of tip:

The following commit has been merged into the x86/cpu branch of tip:

Commit-ID:     00a2f23eef7d1fa6c2dfdc613857b84fbf5e2b3b
Gitweb:        https://git.kernel.org/tip/00a2f23eef7d1fa6c2dfdc613857b84fbf5e2b3b
Author:        Tony Luck <[email protected]>
AuthorDate:    Mon, 31 Jan 2022 15:01:08 -08:00
Committer:     Borislav Petkov <[email protected]>
CommitterDate: Tue, 01 Feb 2022 16:15:19 +01:00

x86/cpu: X86_FEATURE_INTEL_PPIN finally has a CPUID bit

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.

---
 arch/x86/kernel/cpu/common.c    | 1 +
 arch/x86/kernel/cpu/scattered.c | 1 +
 2 files changed, 2 insertions(+)
arch/x86/kernel/cpu/common.c +1 -0
index b0bd8a6..0681c69 100644
@@ -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]),
arch/x86/kernel/cpu/scattered.c +1 -0
index 21d1f06..4143b1e 100644
@@ -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 },
author maintainer
#42 lore raw

PATCH v3 3/5 x86/cpu: Read/save PPIN MSR during initialization

In reply to 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.

---
 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(-)
arch/x86/include/asm/processor.h +2 -0
index 2c5f12ae7d04..a87e7c33d5ac 100644
@@ -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;
arch/x86/kernel/cpu/common.c +4 -0
index 0681c69a1f09..64deb7727d00 100644
@@ -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;
 	}
arch/x86/kernel/cpu/mce/core.c +1 -6
index 5818b837fd4d..4f1e825033ce 100644
@@ -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;
 }
 
Signature · 1 line
2.31.1
merge bot Applied
#43 lore raw

[tip: x86/cpu] x86/cpu: Read/save PPIN MSR during initialization

The following commit has been merged into the x86/cpu branch of tip:

The following commit has been merged into the x86/cpu branch of tip:

Commit-ID:     822ccfade55b6be7977b364356fcf2d78d8a373a
Gitweb:        https://git.kernel.org/tip/822ccfade55b6be7977b364356fcf2d78d8a373a
Author:        Tony Luck <[email protected]>
AuthorDate:    Mon, 31 Jan 2022 15:01:09 -08:00
Committer:     Borislav Petkov <[email protected]>
CommitterDate: Tue, 01 Feb 2022 16:29:26 +01:00

x86/cpu: Read/save PPIN MSR during initialization

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.

---
 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(-)
arch/x86/include/asm/processor.h +2 -0
index 2c5f12a..a87e7c3 100644
@@ -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;
arch/x86/kernel/cpu/common.c +4 -0
index 0681c69..64deb77 100644
@@ -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;
 	}
arch/x86/kernel/cpu/mce/core.c +1 -6
index 5818b83..4f1e825 100644
@@ -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;
 }
 
author maintainer
#44 lore raw

PATCH v3 4/5 topology/sysfs: Add format parameter to macro defining "show" functions for proc

In reply to 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.

---
 drivers/base/topology.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)
drivers/base/topology.c +8 -8
index fc24e89f9592..044f3664f8f2 100644
@@ -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);
Signature · 1 line
2.31.1
merge bot Applied
#45 lore raw

[tip: x86/cpu] topology/sysfs: Add format parameter to macro defining "show" functions for proc

The following commit has been merged into the x86/cpu branch of tip:

The following commit has been merged into the x86/cpu branch of tip:

Commit-ID:     182ecfaf757de234a5262f51c0e699bec7258a67
Gitweb:        https://git.kernel.org/tip/182ecfaf757de234a5262f51c0e699bec7258a67
Author:        Tony Luck <[email protected]>
AuthorDate:    Mon, 31 Jan 2022 15:01:10 -08:00
Committer:     Borislav Petkov <[email protected]>
CommitterDate: Tue, 01 Feb 2022 16:35:21 +01:00

topology/sysfs: Add format parameter to macro defining "show" functions for proc

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.

---
 drivers/base/topology.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)
drivers/base/topology.c +8 -8
index fc24e89..044f366 100644
@@ -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);
author maintainer
#46 lore raw

PATCH v3 5/5 topology/sysfs: Add PPIN in sysfs under cpu topology

In reply to 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.

---
 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(+)
Documentation/ABI/stable/sysfs-devices-system-cpu +4 -0
index 3965ce504484..902392d7eddf 100644
@@ -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.
Documentation/ABI/testing/sysfs-devices-system-cpu +6 -0
index 61f5676a7429..74962c200790 100644
@@ -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 <[email protected]>
 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.
 
 
arch/x86/include/asm/topology.h +1 -0
index 2f0b6be8eaab..43a89476a522 100644
@@ -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;
 
drivers/base/topology.c +4 -0
index 044f3664f8f2..e9d1efcda89b 100644
@@ -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
 };
 
include/linux/topology.h +3 -0
index a6e201758ae9..f19bc3626297 100644
@@ -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
Signature · 1 line
2.31.1
merge bot Applied
#47 lore raw

[tip: x86/cpu] topology/sysfs: Add PPIN in sysfs under cpu topology

The following commit has been merged into the x86/cpu branch of tip:

The following commit has been merged into the x86/cpu branch of tip:

Commit-ID:     ab28e944197fa78e6af7c4a0ffd6bba9a5bbacf0
Gitweb:        https://git.kernel.org/tip/ab28e944197fa78e6af7c4a0ffd6bba9a5bbacf0
Author:        Tony Luck <[email protected]>
AuthorDate:    Mon, 31 Jan 2022 15:01:11 -08:00
Committer:     Borislav Petkov <[email protected]>
CommitterDate: Tue, 01 Feb 2022 16:36:42 +01:00

topology/sysfs: Add PPIN in sysfs under cpu topology

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.

---
 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(+)
Documentation/ABI/stable/sysfs-devices-system-cpu +4 -0
index 3965ce5..902392d 100644
@@ -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.
Documentation/ABI/testing/sysfs-devices-system-cpu +6 -0
index 61f5676..74962c2 100644
@@ -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 <[email protected]>
 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.
 
 
arch/x86/include/asm/topology.h +1 -0
index 2f0b6be..43a8947 100644
@@ -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;
 
drivers/base/topology.c +4 -0
index 044f366..e9d1efc 100644
@@ -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
 };
 
include/linux/topology.h +3 -0
index a6e2017..f19bc36 100644
@@ -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