arm64: cpuinfo: make /proc/cpuinfo more human-readable

When displaying cpuinfo on an arm64 system, include the MPIDR register value which can be used to understand the CPU topology on a platform. This can serve as a cross-check to the information provided in sysfs, and has been useful in understanding CPU and NUMA allocation issues.

In the interest of making things easier for humans to use, add a "CPU name" line to /proc/cpuinfo for each CPU that uses plain old words instead of hex values. For example, instead of printing only CPU implementer 0x43 and CPU part 0x0A1, print also "CPU name :
Cavium ThunderX".

Note that this is not meant to be an exhaustive list of all possible implementers or CPUs (I'm not even sure that is knowable); this patch is intentionally limited to only those willing to provide info in arch/arm64/include/asm/cputype.h

Signed-off-by Al Stone <[email protected]>

Cc Catalin Marinas <[email protected]>

Cc Will Deacon <[email protected]>

Cc Suzuki K Poulose <[email protected]>

Cc Mark Rutland <[email protected]>

---
 arch/arm64/kernel/cpuinfo.c | 84 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 84 insertions(+)

arch/arm64/kernel/cpuinfo.c +2 -0

index e505007138eb..0b4261884862 100644
@@ -75,6 +75,61 @@ static const char *const hwcap_str[] = {
 	NULL
 };
 
+struct hw_part {
+	u16	id;

arch/arm64/kernel/cpuinfo.c +72 -0

+};
+
+static const struct hw_part apm_hw_part[] = {
+	{ APM_CPU_PART_POTENZA,		"Potenza" },
+	{ (-1), "unknown" }		/* Potenza == 0, unfortunately */
+};
+
+static const struct hw_part brcm_hw_part[] = {
+	{ BRCM_CPU_PART_VULCAN,		"Vulcan" },
+	{ (-1), "unknown" }		/* Potenza == 0, unfortunately */
+};
+
+static const struct hw_part cavium_hw_part[] = {
+	{ CAVIUM_CPU_PART_THUNDERX,	 "ThunderX" },
+	{ CAVIUM_CPU_PART_THUNDERX_81XX, "ThunderX 81XX" },
+	{ CAVIUM_CPU_PART_THUNDERX_83XX, "ThunderX 83XX" },
+	{ (-1), "unknown" }		/* Potenza == 0, unfortunately */
+};
+
+static const struct hw_part qcom_hw_part[] = {
+	{ QCOM_CPU_PART_FALKOR_V1,	"Falkor v1" },
+	{ (-1), "unknown" }		/* Potenza == 0, unfortunately */
+};
+
+static const struct hw_part unknown_hw_part[] = {
+	{ (-1), "unknown" }		/* Potenza == 0, unfortunately */
+};
+
+struct hw_impl {
+	u8			id;
+	const struct hw_part	*parts;
+	char			*name;
+};
+
+static const struct hw_impl hw_implementer[] = {
+	{ ARM_CPU_IMP_ARM,	arm_hw_part,	"ARM Ltd." },
+	{ ARM_CPU_IMP_APM,	apm_hw_part,	"Applied Micro" },
+	{ ARM_CPU_IMP_CAVIUM,	cavium_hw_part,	"Cavium" },
+	{ ARM_CPU_IMP_BRCM,	brcm_hw_part,	"Broadcom" },
+	{ ARM_CPU_IMP_QCOM,	qcom_hw_part,	"Qualcomm" },
+	{ 0, unknown_hw_part, "unknown" }
+};
+
 #ifdef CONFIG_COMPAT
 static const char *const compat_hwcap_str[] = {
 	"swp",

@@ -116,6 +171,9 @@ static int c_show(struct seq_file *m, void *v)
 {
 	int i, j;
 	bool compat = personality(current->personality) == PER_LINUX32;
+	u8 impl;
+	u16 part;
+	const struct hw_part *parts;
 
 	for_each_online_cpu(i) {
 		struct cpuinfo_arm64 *cpuinfo = &per_cpu(cpu_data, i);

@@ -132,6 +190,32 @@ static int c_show(struct seq_file *m, void *v)
 			seq_printf(m, "model name\t: ARMv8 Processor rev %d (%s)\n",
 				   MIDR_REVISION(midr), COMPAT_ELF_PLATFORM);
 
+		impl = (u8) MIDR_IMPLEMENTOR(midr);
+		for (j = 0; hw_implementer[j].id != 0; j++) {
+			if (hw_implementer[j].id == impl) {
+				seq_printf(m, "CPU name\t: %s ",
+					   hw_implementer[j].name);
+				parts = hw_implementer[j].parts;
+				break;
+			}
+		}
+		if (hw_implementer[j].id == 0) {
+			seq_printf(m, "CPU name\t: %s ",
+				   hw_implementer[j].name);
+			parts = hw_implementer[j].parts;
+		}
+
+		part = (u16) MIDR_PARTNUM(midr);
+		for (j = 0; parts[j].id != (-1); j++) {
+			if (parts[j].id == part) {
+				seq_printf(m, "%s\n", parts[j].name);
+				break;
+			}
+		}
+		if (parts[j].id == (-1))
+			seq_printf(m, "%s", parts[j].name);
+		seq_puts(m, "\n");
+
 		seq_printf(m, "BogoMIPS\t: %lu.%02lu\n",
 			   loops_per_jiffy / (500000UL/HZ),
 			   loops_per_jiffy / (5000UL/HZ) % 100);

In the interest of making things easier for humans to use, add a "CPU name" line to /proc/cpuinfo for each CPU that uses plain old words instead of hex values. For example, instead of printing only CPU implementer 0x43 and CPU part 0x0A1, print also "CPU name :
Cavium ThunderX".

Note that this is not meant to be an exhaustive list of all possible implementers or CPUs (I'm not even sure that is knowable); this patch is intentionally limited to only those willing to provide info in arch/arm64/include/asm/cputype.h

Signed-off-by Al Stone <[email protected]>

Cc Catalin Marinas <[email protected]>

Cc Will Deacon <[email protected]>

Cc Suzuki K Poulose <[email protected]>

Cc Mark Rutland <[email protected]>

---
 arch/arm64/kernel/cpuinfo.c | 84 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 84 insertions(+)

arch/arm64/kernel/cpuinfo.c +84 -0

index e505007138eb..0b4261884862 100644
@@ -75,6 +75,61 @@ static const char *const hwcap_str[] = {
 	NULL
 };
 
+struct hw_part {
+	u16	id;
+	char	*name;
+};
+
+static const struct hw_part arm_hw_part[] = {
+	{ ARM_CPU_PART_AEM_V8,		"AEMv8 Model" },
+	{ ARM_CPU_PART_FOUNDATION,	"Foundation Model" },
+	{ ARM_CPU_PART_CORTEX_A57,	"Cortex A57" },
+	{ ARM_CPU_PART_CORTEX_A53,	"Cortex A53" },
+	{ ARM_CPU_PART_CORTEX_A73,	"Cortex A73" },
+	{ (-1), "unknown" }		/* Potenza == 0, unfortunately */
+};
+
+static const struct hw_part apm_hw_part[] = {
+	{ APM_CPU_PART_POTENZA,		"Potenza" },
+	{ (-1), "unknown" }		/* Potenza == 0, unfortunately */
+};
+
+static const struct hw_part brcm_hw_part[] = {
+	{ BRCM_CPU_PART_VULCAN,		"Vulcan" },
+	{ (-1), "unknown" }		/* Potenza == 0, unfortunately */
+};
+
+static const struct hw_part cavium_hw_part[] = {
+	{ CAVIUM_CPU_PART_THUNDERX,	 "ThunderX" },
+	{ CAVIUM_CPU_PART_THUNDERX_81XX, "ThunderX 81XX" },
+	{ CAVIUM_CPU_PART_THUNDERX_83XX, "ThunderX 83XX" },
+	{ (-1), "unknown" }		/* Potenza == 0, unfortunately */
+};
+
+static const struct hw_part qcom_hw_part[] = {
+	{ QCOM_CPU_PART_FALKOR_V1,	"Falkor v1" },
+	{ (-1), "unknown" }		/* Potenza == 0, unfortunately */
+};
+
+static const struct hw_part unknown_hw_part[] = {
+	{ (-1), "unknown" }		/* Potenza == 0, unfortunately */
+};
+
+struct hw_impl {
+	u8			id;
+	const struct hw_part	*parts;
+	char			*name;
+};
+
+static const struct hw_impl hw_implementer[] = {
+	{ ARM_CPU_IMP_ARM,	arm_hw_part,	"ARM Ltd." },
+	{ ARM_CPU_IMP_APM,	apm_hw_part,	"Applied Micro" },
+	{ ARM_CPU_IMP_CAVIUM,	cavium_hw_part,	"Cavium" },
+	{ ARM_CPU_IMP_BRCM,	brcm_hw_part,	"Broadcom" },
+	{ ARM_CPU_IMP_QCOM,	qcom_hw_part,	"Qualcomm" },
+	{ 0, unknown_hw_part, "unknown" }
+};
+
 #ifdef CONFIG_COMPAT
 static const char *const compat_hwcap_str[] = {
 	"swp",

@@ -116,6 +171,9 @@ static int c_show(struct seq_file *m, void *v)
 {
 	int i, j;
 	bool compat = personality(current->personality) == PER_LINUX32;
+	u8 impl;
+	u16 part;
+	const struct hw_part *parts;
 
 	for_each_online_cpu(i) {
 		struct cpuinfo_arm64 *cpuinfo = &per_cpu(cpu_data, i);

@@ -132,6 +190,32 @@ static int c_show(struct seq_file *m, void *v)
 			seq_printf(m, "model name\t: ARMv8 Processor rev %d (%s)\n",
 				   MIDR_REVISION(midr), COMPAT_ELF_PLATFORM);
 
+		impl = (u8) MIDR_IMPLEMENTOR(midr);
+		for (j = 0; hw_implementer[j].id != 0; j++) {
+			if (hw_implementer[j].id == impl) {
+				seq_printf(m, "CPU name\t: %s ",
+					   hw_implementer[j].name);
+				parts = hw_implementer[j].parts;
+				break;
+			}
+		}
+		if (hw_implementer[j].id == 0) {
+			seq_printf(m, "CPU name\t: %s ",
+				   hw_implementer[j].name);
+			parts = hw_implementer[j].parts;
+		}
+
+		part = (u16) MIDR_PARTNUM(midr);
+		for (j = 0; parts[j].id != (-1); j++) {
+			if (parts[j].id == part) {
+				seq_printf(m, "%s\n", parts[j].name);
+				break;
+			}
+		}
+		if (parts[j].id == (-1))
+			seq_printf(m, "%s", parts[j].name);
+		seq_puts(m, "\n");
+
 		seq_printf(m, "BogoMIPS\t: %lu.%02lu\n",
 			   loops_per_jiffy / (500000UL/HZ),
 			   loops_per_jiffy / (5000UL/HZ) % 100);

Signature · 1 line

2.13.5

In the interest of making things easier for humans to use, add a "CPU name" line to /proc/cpuinfo for each CPU that uses plain old words instead of hex values. For example, instead of printing only CPU implementer 0x43 and CPU part 0x0A1, print also "CPU name :
Cavium ThunderX".

Note that this is not meant to be an exhaustive list of all possible implementers or CPUs (I'm not even sure that is knowable); this patch is intentionally limited to only those willing to provide info in arch/arm64/include/asm/cputype.h

Signed-off-by Al Stone <[email protected]>

Cc Catalin Marinas <[email protected]>

Cc Will Deacon <[email protected]>

Cc Suzuki K Poulose <[email protected]>

Cc Mark Rutland <[email protected]>

---
 arch/arm64/kernel/cpuinfo.c | 84 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 84 insertions(+)

arch/arm64/kernel/cpuinfo.c +0 -0

index e505007138eb..0b4261884862 100644

arch/arm64/kernel/cpuinfo.c +9 -0

+static const struct hw_part apm_hw_part[] = {
+       { APM_CPU_PART_POTENZA,         "Potenza" },
+       { (-1), "unknown" }             /* Potenza == 0, unfortunately */
+};
+
+static const struct hw_part brcm_hw_part[] = {
+       { BRCM_CPU_PART_VULCAN,         "Vulcan" },
+       { (-1), "unknown" }             /* Potenza == 0, unfortunately */
+};

arch/arm64/kernel/cpuinfo.c +4 -0

+               impl = (u8) MIDR_IMPLEMENTOR(midr);
+               for (j = 0; hw_implementer[j].id != 0; j++) {
+                       if (hw_implementer[j].id == impl) {
+                               seq_printf(m, "CPU name\t: %s ",

While it is very useful to know what CPU is being used, it is also useful to know who made the platform being used. On servers, this can point to the right person to contact when a server is having trouble.

Go get the product info -- manufacturer, product name and version -- from DMI (aka SMBIOS) and display it in /proc/cpuinfo. To look more like other server platforms, include the CPU type and frequency when displaying the product info, too.

Signed-off-by Al Stone <[email protected]>

Cc Catalin Marinas <[email protected]>

Cc Will Deacon <[email protected]>

Cc Suzuki K Poulose <[email protected]>

Cc Mark Rutland <[email protected]>

---

While it is very useful to know what CPU is being used, it is also useful to know who made the platform being used. On servers, this can point to the right person to contact when a server is having trouble.

Go get the product info -- manufacturer, product name and version -- from DMI (aka SMBIOS) and display it in /proc/cpuinfo. To look more like other server platforms, include the CPU type and frequency when displaying the product info, too.

Signed-off-by Al Stone <[email protected]>

Cc Catalin Marinas <[email protected]>

Cc Will Deacon <[email protected]>

Cc Suzuki K Poulose <[email protected]>

Cc Mark Rutland <[email protected]>

---

arch/arm64/kernel/cpuinfo.c +4 -0

@@ -190,6 +298,31 @@ static int c_show(struct seq_file *m, void *v)
 			seq_printf(m, "model name\t: ARMv8 Processor rev %d (%s)\n",
 				   MIDR_REVISION(midr), COMPAT_ELF_PLATFORM);
 
+		if (IS_ENABLED(CONFIG_DMI)) {
+			seq_puts(m, "product name\t: ");
+
+			if (!dmi_product_info)

arch/arm64/kernel/cpuinfo.c +11 -0

+
+			if (!dmi_max_mhz)
+				dmi_max_mhz = get_dmi_max_mhz();
+			if (dmi_max_mhz)
+				seq_printf(m, " @ %d.%02dGHz\n",
+					   (int)(dmi_max_mhz / 1000),
+					   (int)(dmi_max_mhz % 1000));
+			else
+				seq_puts(m, " @ <unknown>GHz\n");
+		}
+
 		impl = (u8) MIDR_IMPLEMENTOR(midr);
 		for (j = 0; hw_implementer[j].id != 0; j++) {
 			if (hw_implementer[j].id == impl) {

arch/arm64/kernel/cpuinfo.c static int c_show(struct seq_file *m, void *v) · line 314

Quoted 4 lines · Al Stone

arch/arm64/kernel/cpuinfo.c +4 -0

+			seq_printf(m, ", ARM 8.%d (r%dp%d) CPU",
+				   MIDR_VARIANT(midr),
+				   MIDR_VARIANT(midr),
+				   MIDR_REVISION(midr));

What is "ARM 8.1" meant to infer for, say, a typical Cortex-A57?

While it is very useful to know what CPU is being used, it is also useful to know who made the platform being used. On servers, this can point to the right person to contact when a server is having trouble.

Go get the product info -- manufacturer, product name and version -- from DMI (aka SMBIOS) and display it in /proc/cpuinfo. To look more like other server platforms, include the CPU type and frequency when displaying the product info, too.

Signed-off-by Al Stone <[email protected]>

Cc Catalin Marinas <[email protected]>

Cc Will Deacon <[email protected]>

Cc Suzuki K Poulose <[email protected]>

Cc Mark Rutland <[email protected]>

---
 arch/arm64/kernel/cpuinfo.c | 135 +++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 134 insertions(+), 1 deletion(-)

arch/arm64/kernel/cpuinfo.c +134 -1

index 0b4261884862..6a9dbad5ee3f 100644
@@ -19,10 +19,12 @@
 #include <asm/cpu.h>
 #include <asm/cputype.h>
 #include <asm/cpufeature.h>
+#include <asm/unaligned.h>
 
 #include <linux/bitops.h>
 #include <linux/bug.h>
 #include <linux/compat.h>
+#include <linux/dmi.h>
 #include <linux/elf.h>
 #include <linux/init.h>
 #include <linux/kernel.h>

@@ -31,6 +33,7 @@
 #include <linux/printk.h>
 #include <linux/seq_file.h>
 #include <linux/sched.h>
+#include <linux/slab.h>
 #include <linux/smp.h>
 #include <linux/delay.h>
 #include <linux/cpuinfo.h>

@@ -167,6 +170,111 @@ static const char *const compat_hwcap2_str[] = {
 };
 #endif /* CONFIG_COMPAT */
 
+/* Details needed when extracting fields from DMI info */
+#define DMI_ENTRY_BASEBOARD_MIN_LENGTH	8
+#define DMI_ENTRY_PROCESSOR_MIN_LENGTH	48
+
+#define DMI_BASEBOARD_MANUFACTURER	0x04
+#define DMI_BASEBOARD_PRODUCT		0x05
+#define DMI_BASEBOARD_VERSION		0x06
+#define DMI_PROCESSOR_MAX_SPEED		0x14
+
+#define DMI_MAX_STRLEN			80
+
+/* Values captured from DMI info */
+static u64 dmi_max_mhz;
+static char *dmi_product_info;
+
+/* Callback function used to retrieve the max frequency from DMI */
+static void find_dmi_mhz(const struct dmi_header *dm, void *private)
+{
+	const u8 *dmi_data = (const u8 *)dm;
+	u16 *mhz = (u16 *)private;
+
+	if (dm->type == DMI_ENTRY_PROCESSOR &&
+	    dm->length >= DMI_ENTRY_PROCESSOR_MIN_LENGTH) {
+		u16 val = (u16)get_unaligned((const u16 *)
+				(dmi_data + DMI_PROCESSOR_MAX_SPEED));
+		*mhz = val > *mhz ? val : *mhz;
+	}
+}
+
+/* Look up the max frequency in DMI */
+static u64 get_dmi_max_mhz(void)
+{
+	u16 mhz = 0;
+
+	dmi_walk(find_dmi_mhz, &mhz);
+
+	/*
+	 * Real stupid fallback value, just in case there is no
+	 * actual value set.
+	 */
+	mhz = mhz ? mhz : 1;
+
+	return (u64)mhz;
+}
+
+/* Helper function for the product info callback */
+static char *copy_string_n(char *dst, char *table, int idx)
+{
+	char *d = dst;
+	char *ptr = table;
+	int ii;
+
+	/* skip the first idx-1 strings */
+	for (ii = 1; ii < idx; ii++) {
+		while (*ptr)
+			ptr++;
+		ptr++;
+	}
+
+	/* copy in the string we need */
+	while (*ptr && (d - dst) < (DMI_MAX_STRLEN - 2))
+		*d++ = *ptr++;
+
+	return d;
+}
+
+/* Callback function used to retrieve the product info DMI */
+static void find_dmi_product_info(const struct dmi_header *dm, void *private)
+{
+	const u8 *dmi_data = (const u8 *)dm;
+	char *ptr = (char *)private;
+
+	if (dm->type == DMI_ENTRY_BASEBOARD &&
+	    dm->length >= DMI_ENTRY_BASEBOARD_MIN_LENGTH) {
+		int idx;
+
+		idx = (int)get_unaligned((const u8 *)
+				(dmi_data + DMI_BASEBOARD_MANUFACTURER));
+		ptr = copy_string_n(ptr, (char *)(dmi_data + dm->length), idx);
+		*ptr++ = ' ';
+
+		idx = (int)get_unaligned((const u8 *)
+				(dmi_data + DMI_BASEBOARD_PRODUCT));
+		ptr = copy_string_n(ptr, (char *)(dmi_data + dm->length), idx);
+		*ptr++ = ' ';
+
+		idx = (int)get_unaligned((const u8 *)
+				(dmi_data + DMI_BASEBOARD_VERSION));
+		ptr = copy_string_n(ptr, (char *)(dmi_data + dm->length), idx);
+	}
+}
+
+/* Look up the baseboard info in DMI */
+static void get_dmi_product_info(void)
+{
+	if (!dmi_product_info) {
+		dmi_product_info = kcalloc(DMI_MAX_STRLEN,
+					   sizeof(char), GFP_KERNEL);
+		if (!dmi_product_info)
+			return;
+	}
+
+	dmi_walk(find_dmi_product_info, dmi_product_info);
+}
+
 static int c_show(struct seq_file *m, void *v)
 {
 	int i, j;

@@ -190,6 +298,31 @@ static int c_show(struct seq_file *m, void *v)
 			seq_printf(m, "model name\t: ARMv8 Processor rev %d (%s)\n",
 				   MIDR_REVISION(midr), COMPAT_ELF_PLATFORM);
 
+		if (IS_ENABLED(CONFIG_DMI)) {
+			seq_puts(m, "product name\t: ");
+
+			if (!dmi_product_info)
+				get_dmi_product_info();
+			if (dmi_product_info)
+				seq_printf(m, "%s", dmi_product_info);
+			else
+				seq_puts(m, "<unknown>");
+
+			seq_printf(m, ", ARM 8.%d (r%dp%d) CPU",
+				   MIDR_VARIANT(midr),
+				   MIDR_VARIANT(midr),
+				   MIDR_REVISION(midr));
+
+			if (!dmi_max_mhz)
+				dmi_max_mhz = get_dmi_max_mhz();
+			if (dmi_max_mhz)
+				seq_printf(m, " @ %d.%02dGHz\n",
+					   (int)(dmi_max_mhz / 1000),
+					   (int)(dmi_max_mhz % 1000));
+			else
+				seq_puts(m, " @ <unknown>GHz\n");
+		}
+
 		impl = (u8) MIDR_IMPLEMENTOR(midr);
 		for (j = 0; hw_implementer[j].id != 0; j++) {
 			if (hw_implementer[j].id == impl) {

@@ -208,7 +341,7 @@ static int c_show(struct seq_file *m, void *v)
 		part = (u16) MIDR_PARTNUM(midr);
 		for (j = 0; parts[j].id != (-1); j++) {
 			if (parts[j].id == part) {
-				seq_printf(m, "%s\n", parts[j].name);
+				seq_printf(m, "%s ", parts[j].name);
 				break;
 			}
 		}

Signature · 1 line

2.13.5

As ARMv8 servers get deployed, I keep getting the same set of questions from end-users of those systems: what do all the hex numbers mean in /proc/cpuinfo and could you make them so I don't have to carry a cheat sheet with me all the time?

Quoted 4 lines · Al Stone

As ARMv8 servers get deployed, I keep getting the same set of questions from end-users of those systems: what do all the hex numbers mean in /proc/cpuinfo and could you make them so I don't have to carry a cheat sheet with me all the time?

I appreciate that /proc/cpuinfo can be opaque to end users, but I do not believe that this is the right solution to that problem.

There are a number of issues stemming from the face that /proc/cpuinfo is ill-defined and overused for a number of cases. Changes to it almost certainly violate brittle de-facto ABI details people are relying on, and there's a very long tail on fallout resulting from this. In addition, many niceties come at an excessive maintenance cost, and are simply unreliable as an ABI.

So, as with all other patches modifying /proc/cpuinfo, I must NAK this series.

For the MPIDR and product info, I think we can expose this in a more structured way (e.g. under sysfs). IIUC the product info is all derived from DMI -- do we not expose that already?

For the human-readable names I must NAK such patches. This is an extremely brittle ABI that cannot be forwards compatible, and comes with hilarious political problems. This should be managed in userspace alone.

I thought tools like lscpu and lshw were intended to gather such information for users. What are these missing today?

Hi Al,

Quoted 4 lines · Al Stone

As ARMv8 servers get deployed, I keep getting the same set of questions from end-users of those systems: what do all the hex numbers mean in /proc/cpuinfo and could you make them so I don't have to carry a cheat sheet with me all the time?

I appreciate that /proc/cpuinfo can be opaque to end users, but I do not believe that this is the right solution to that problem.

There are a number of issues stemming from the face that /proc/cpuinfo is ill-defined and overused for a number of cases. Changes to it almost certainly violate brittle de-facto ABI details people are relying on, and there's a very long tail on fallout resulting from this. In addition, many niceties come at an excessive maintenance cost, and are simply unreliable as an ABI.

So, as with all other patches modifying /proc/cpuinfo, I must NAK this series.

Quoted 16 lines · Mark Rutland

Quoted 4 lines · Al Stone

As ARMv8 servers get deployed, I keep getting the same set of questions from end-users of those systems: what do all the hex numbers mean in /proc/cpuinfo and could you make them so I don't have to carry a cheat sheet with me all the time?

I appreciate that /proc/cpuinfo can be opaque to end users, but I do not believe that this is the right solution to that problem.

There are a number of issues stemming from the face that /proc/cpuinfo is ill-defined and overused for a number of cases. Changes to it almost certainly violate brittle de-facto ABI details people are relying on, and there's a very long tail on fallout resulting from this. In addition, many niceties come at an excessive maintenance cost, and are simply unreliable as an ABI.

Quoted 2 lines

So, as with all other patches modifying /proc/cpuinfo, I must NAK this series.

Qualcomm Datacenter Technologies is very interested in seeing these patches (or some variant of them) accepted. Updates to /proc/cpuinfo are long overdue, and I'm asking you to reconsider your objections.

We're willing to work with distro vendors to get this information added to their products while upstream is left behind, but I hope that won't be necessary.

I would even go so far as to say that we should be making /proc/cpuinfo for ARM match the x86 output as closely as possible,

even using their terminology. We should be providing information like frequencies and product names.

Having a human-readable /proc/cpuinfo with extensive details of the CPU spelled out is very useful, and Al's reasoning is valid. The fact that it's "fragile" is not as important as the fact that on x86, /proc/cpuinfo is much more useful.

Hi Timur,

Quoted 17 lines · Timur Tabi

Quoted 16 lines · Mark Rutland

Quoted 4 lines · Al Stone

As ARMv8 servers get deployed, I keep getting the same set of questions from end-users of those systems: what do all the hex numbers mean in /proc/cpuinfo and could you make them so I don't have to carry a cheat sheet with me all the time?

I appreciate that /proc/cpuinfo can be opaque to end users, but I do not believe that this is the right solution to that problem.

There are a number of issues stemming from the face that /proc/cpuinfo is ill-defined and overused for a number of cases. Changes to it almost certainly violate brittle de-facto ABI details people are relying on, and there's a very long tail on fallout resulting from this. In addition, many niceties come at an excessive maintenance cost, and are simply unreliable as an ABI.

Quoted 6 lines

Quoted 2 lines

So, as with all other patches modifying /proc/cpuinfo, I must NAK this series.

Qualcomm Datacenter Technologies is very interested in seeing these patches (or some variant of them) accepted. Updates to /proc/cpuinfo are long overdue, and I'm asking you to reconsider your objections.

I more than appreciate that there is information that people want access to, and I'd really like to expose that in a consistent and structured manner.

So please propose exposing the information elsewhere, which I would be more than happy with for information that the kernel already has access to.

I object to /proc/cpuinfo changes because they impose an ABI break, and because some of the proposed changes impose new and requirements on the kernel that cannot be maintained in a forwards-compatible manner (which is liable to result in other ABI breaks).

For better or worse, we're stuck with the existing arch-specific format of /proc/cpuinfo, as this is parsed by a wide variety of applications, and any change risks breaking these.

Quoted 3 lines

We're willing to work with distro vendors to get this information added to their products while upstream is left behind, but I hope that won't be necessary.

Please understand that this is an ABI break, and that is why it is being NAK'd.

I don't have the power to stop Red Hat or other vendors from deliberately breaking ABI, but I would very strongly recommend that they do not.

Quoted 2 lines

I would even go so far as to say that we should be making /proc/cpuinfo for ARM match the x86 output as closely as possible,

As has been stated before, /proc/cpuinfo is an existing arch-specific ABI.

For better or worse, we're stuck with it as-is, regardless of what could be nicer had we done something different from the outset.

It has never been portable across architectures, and applications relying upon it have never been portable except by chance.

Quoted 2 lines

even using their terminology. We should be providing information like frequencies and product names.

As has been stated before, the problem with exposing these is that we don't have the relevant information.

We have no consistent mechanism for acquiring product names, and for those cases where it is truly necessary to identify an implementation, the MIDR+REVIDR provide the exact same information.

For frequencies, I'd be more than happy to expose this information when we have it, in new files. In practice today, we do not have this information most of the time.

Quoted 4 lines

Having a human-readable /proc/cpuinfo with extensive details of the CPU spelled out is very useful, and Al's reasoning is valid. The fact that it's "fragile" is not as important as the fact that on x86, /proc/cpuinfo is much more useful.

I must disagree.

We care about that fragility so that we do not break userspace, which is the most important upstream rule.

I certainly agree that exposing the information that we have is useful, as I have stated several times. I'm not NAKing exposing this information elsewhere.

If you want a consistent cross-architecture interface for this information, then you need to propose a new one. That was we can actually solve the underlying issues, for all architectures, without breaking ABI.

I would be *very* interested in such an interface, and would be more than happy to help.

Thanks,
Mark.

Quoted 9 lines · Mark Rutland

I certainly agree that exposing the information that we have is useful, as I have stated several times. I'm not NAKing exposing this information elsewhere.

If you want a consistent cross-architecture interface for this information, then you need to propose a new one. That was we can actually solve the underlying issues, for all architectures, without breaking ABI.

Quoted 2 lines

I would be *very* interested in such an interface, and would be more than happy to help.

I'm playing with some patches that do very similar things in sysfs, vs proc. Is that better :)?

Obviously, you'll have to see the patches to
properly answer that, but what I'm playing with at present is placing this info in new entries in /sys/devices/cpu and/or /sys/devices/system, and generating some of the content based on what's already in header files (e.g., in cputype.h).

The idea of course is to keep this new info from touching any existing info so we don't break compatibility -- does that feel like a better direction, at least?

Quoted 5 lines · Al Stone

Obviously, you'll have to see the patches to
properly answer that, but what I'm playing with at present is placing this info in new entries in /sys/devices/cpu and/or /sys/devices/system, and generating some of the content based on what's already in header files (e.g., in cputype.h).

My opposition to MIDR -> string mapping applies regardless of location...

1). The first thing people do when they get an Arm server is to cat /proc/cpuinfo. They then come complaining that it's not like x86. They can't get the output their looking for and this results in bug filing, and countless hours on phone calls discussing over and over again. Worse, there are some parts of the stack that really need this.

Hi Al,

Quoted 10 lines · Al Stone

Quoted 9 lines · Mark Rutland

I certainly agree that exposing the information that we have is useful, as I have stated several times. I'm not NAKing exposing this information elsewhere.

If you want a consistent cross-architecture interface for this information, then you need to propose a new one. That was we can actually solve the underlying issues, for all architectures, without breaking ABI.

Quoted 5 lines

Quoted 2 lines

I would be *very* interested in such an interface, and would be more than happy to help.

I'm playing with some patches that do very similar things in sysfs, vs proc. Is that better :)?

Exposing data under sysfs is certainly better, yes. :)

Quoted 5 lines

Obviously, you'll have to see the patches to
properly answer that, but what I'm playing with at present is placing this info in new entries in /sys/devices/cpu and/or /sys/devices/system, and generating some of the content based on what's already in header files (e.g., in cputype.h).

My opposition to MIDR -> string mapping applies regardless of location...

Quoted 3 lines

The idea of course is to keep this new info from touching any existing info so we don't break compatibility -- does that feel like a better direction, at least?

... but otherwise this sounds good to me!

Thanks,
Mark.