Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1740255 > unrolled thread

[PATCH 0/3] arm64: cpuinfo: make /proc/cpuinfo more human-readable

Back to article view | Back to linux.kernel

Contents

  [PATCH 0/3] arm64: cpuinfo: make /proc/cpuinfo more human-readable Al Stone <ahs3@redhat.com> - 2017-09-27 00:30 +0200
    [PATCH 1/3] arm64: cpuinfo: add MPIDR value to /proc/cpuinfo Al Stone <ahs3@redhat.com> - 2017-09-27 00:30 +0200
      Re: [PATCH 1/3] arm64: cpuinfo: add MPIDR value to /proc/cpuinfo Mark Rutland <mark.rutland@arm.com> - 2017-09-27 13:40 +0200
    [PATCH 3/3] arm64: cpuinfo: display product info in /proc/cpuinfo Al Stone <ahs3@redhat.com> - 2017-09-27 00:30 +0200
      Re: [PATCH 3/3] arm64: cpuinfo: display product info in /proc/cpuinfo Florian Fainelli <f.fainelli@gmail.com> - 2017-09-27 02:50 +0200
      Re: [PATCH 3/3] arm64: cpuinfo: display product info in /proc/cpuinfo Robin Murphy <robin.murphy@arm.com> - 2017-09-27 12:50 +0200
        Re: [PATCH 3/3] arm64: cpuinfo: display product info in /proc/cpuinfo Mark Rutland <mark.rutland@arm.com> - 2017-09-27 15:50 +0200
      Re: [PATCH 3/3] arm64: cpuinfo: display product info in /proc/cpuinfo Mark Rutland <mark.rutland@arm.com> - 2017-09-27 13:40 +0200
    [PATCH 2/3] arm64: cpuinfo: add human readable CPU names to /proc/cpuinfo Al Stone <ahs3@redhat.com> - 2017-09-27 00:30 +0200
      Re: [PATCH 2/3] arm64: cpuinfo: add human readable CPU names to  /proc/cpuinfo Robin Murphy <robin.murphy@arm.com> - 2017-09-27 12:40 +0200
      Re: [PATCH 2/3] arm64: cpuinfo: add human readable CPU names to  /proc/cpuinfo Mark Rutland <mark.rutland@arm.com> - 2017-09-27 13:30 +0200
    Re: [PATCH 0/3] arm64: cpuinfo: make /proc/cpuinfo more  human-readable Mark Rutland <mark.rutland@arm.com> - 2017-09-27 12:40 +0200

#1740255 — [PATCH 0/3] arm64: cpuinfo: make /proc/cpuinfo more human-readable

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?

These patches respond to those questions.  For good or ill, some of the
automation used to manage systems in data centers (as well as many of
the humans involved) need to have text; this helps them simply slide 
into place and become usable quickly.

Patch 1/2 provides the MPIDR as basic topology info in /proc/cpuinfo
when using ACPI, perhaps until such time as the more robust ACPI
implementation is available [0]; this is helpful in automating the
selection of multi-CPU systems when many choices are available (for
example, in automated testing systems).  While it is yet another hex
value, it does provide some topology information without interfering
with what [0] will ultimately provide, and is helpful in sorting out
ACPI table issues that use the MPIDR for identifying CPUs.

Patches 2/3 and 3/3 are similar in that they provide a more human-
readable version of the info already available; this allows admin
tools to provide proper strings to display in inventory systems, for
example, or when a human is using a CI system and needs to be provided
a list of possible systems to test on.

In all of the patches, I have avoided replacing or interfering with
any existing output so as not to affect systems already in use.

Tested on AMD Seattle, APM Mustang and Cavium ThunderX systems.


[0] https://marc.info/?l=linux-pm&m=150584702021552&w=2


Al Stone (3):
  arm64: cpuinfo: add MPIDR value to /proc/cpuinfo
  arm64: cpuinfo: add human readable CPU names to /proc/cpuinfo
  arm64: cpuinfo: display product info in /proc/cpuinfo

 arch/arm64/include/asm/cpu.h |   1 +
 arch/arm64/kernel/cpuinfo.c  | 225 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 226 insertions(+)

-- 
2.13.5

[toc] | [next] | [standalone]

#1740257 — [PATCH 1/3] arm64: cpuinfo: add MPIDR value to /proc/cpuinfo

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.

Signed-off-by: Al Stone <ahs3@redhat.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
Cc: Suzuki K Poulose <suzuki.poulose@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
---
 arch/arm64/include/asm/cpu.h | 1 +
 arch/arm64/kernel/cpuinfo.c  | 8 ++++++++
 2 files changed, 9 insertions(+)

diff --git a/arch/arm64/include/asm/cpu.h b/arch/arm64/include/asm/cpu.h
index 889226b4c6e1..ac40894df247 100644
--- a/arch/arm64/include/asm/cpu.h
+++ b/arch/arm64/include/asm/cpu.h
@@ -32,6 +32,7 @@ struct cpuinfo_arm64 {
 	u32		reg_midr;
 	u32		reg_revidr;
 
+	u64		reg_id_aa64mpidr;
 	u64		reg_id_aa64dfr0;
 	u64		reg_id_aa64dfr1;
 	u64		reg_id_aa64isar0;
diff --git a/arch/arm64/kernel/cpuinfo.c b/arch/arm64/kernel/cpuinfo.c
index 4a6f875ac854..e505007138eb 100644
--- a/arch/arm64/kernel/cpuinfo.c
+++ b/arch/arm64/kernel/cpuinfo.c
@@ -120,6 +120,7 @@ static int c_show(struct seq_file *m, void *v)
 	for_each_online_cpu(i) {
 		struct cpuinfo_arm64 *cpuinfo = &per_cpu(cpu_data, i);
 		u32 midr = cpuinfo->reg_midr;
+		u64 mpidr = cpuinfo->reg_id_aa64mpidr;
 
 		/*
 		 * glibc reads /proc/cpuinfo to determine the number of
@@ -159,6 +160,12 @@ static int c_show(struct seq_file *m, void *v)
 		}
 		seq_puts(m, "\n");
 
+		seq_printf(m, "CPU MPIDR\t: 0x%016llx ", mpidr);
+		seq_printf(m, "(Aff3 %d Aff2 %d Aff1 %d Aff0 %d)\n",
+			   (u8) MPIDR_AFFINITY_LEVEL(mpidr, 3),
+			   (u8) MPIDR_AFFINITY_LEVEL(mpidr, 2),
+			   (u8) MPIDR_AFFINITY_LEVEL(mpidr, 1),
+			   (u8) MPIDR_AFFINITY_LEVEL(mpidr, 0));
 		seq_printf(m, "CPU implementer\t: 0x%02x\n",
 			   MIDR_IMPLEMENTOR(midr));
 		seq_printf(m, "CPU architecture: 8\n");
@@ -372,6 +379,7 @@ static void __cpuinfo_store_cpu(struct cpuinfo_arm64 *info)
 	info->reg_midr = read_cpuid_id();
 	info->reg_revidr = read_cpuid(REVIDR_EL1);
 
+	info->reg_id_aa64mpidr = read_cpuid_mpidr();
 	info->reg_id_aa64dfr0 = read_cpuid(ID_AA64DFR0_EL1);
 	info->reg_id_aa64dfr1 = read_cpuid(ID_AA64DFR1_EL1);
 	info->reg_id_aa64isar0 = read_cpuid(ID_AA64ISAR0_EL1);
-- 
2.13.5

[toc] | [prev] | [next] | [standalone]

#1740627 — Re: [PATCH 1/3] arm64: cpuinfo: add MPIDR value to /proc/cpuinfo

On Tue, Sep 26, 2017 at 04:23:22PM -0600, Al Stone wrote:
> 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.

As mentioend in the cover letter, NAK to modifiying /proc/cpuinfo.

However, I do think that we could expose this elsewhere in a structured
way.

For debugging bringup issues, I think we can update our secondary
bringup messages in dmesg to log the MPIDR (as we do for the boot CPU).
I'll send a patch for that.

> @@ -159,6 +160,12 @@ static int c_show(struct seq_file *m, void *v)
>  		}
>  		seq_puts(m, "\n");
>  
> +		seq_printf(m, "CPU MPIDR\t: 0x%016llx ", mpidr);
> +		seq_printf(m, "(Aff3 %d Aff2 %d Aff1 %d Aff0 %d)\n",
> +			   (u8) MPIDR_AFFINITY_LEVEL(mpidr, 3),
> +			   (u8) MPIDR_AFFINITY_LEVEL(mpidr, 2),
> +			   (u8) MPIDR_AFFINITY_LEVEL(mpidr, 1),
> +			   (u8) MPIDR_AFFINITY_LEVEL(mpidr, 0));

Please don't decode the register like this. We're stuck doing it with
the MIDR for historical reasons, but we shouldn't do it for new
registers.

It's possible (and I suspect likely) that MPIDR will gain more fields in
future, and it creates futher ABI problems (e.g. adding them might break
applications).

If we're going to expose this, we should expose the raw value under
sysfs. Users who require this information will know how to decode it.

Thanks,
Mark.

[toc] | [prev] | [next] | [standalone]

#1740261 — [PATCH 3/3] arm64: cpuinfo: display product info in /proc/cpuinfo

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 <ahs3@redhat.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
Cc: Suzuki K Poulose <suzuki.poulose@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
---
 arch/arm64/kernel/cpuinfo.c | 135 +++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 134 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/kernel/cpuinfo.c b/arch/arm64/kernel/cpuinfo.c
index 0b4261884862..6a9dbad5ee3f 100644
--- a/arch/arm64/kernel/cpuinfo.c
+++ b/arch/arm64/kernel/cpuinfo.c
@@ -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;
 			}
 		}
-- 
2.13.5

[toc] | [prev] | [next] | [standalone]

#1740302 — Re: [PATCH 3/3] arm64: cpuinfo: display product info in /proc/cpuinfo

On 09/26/2017 03:23 PM, Al Stone wrote:
> 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 <ahs3@redhat.com>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will.deacon@arm.com>
> Cc: Suzuki K Poulose <suzuki.poulose@arm.com>
> Cc: Mark Rutland <mark.rutland@arm.com>
> ---

[snip]

> +/* 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);
> +}

Don't you need all of these DMI-related functions you defined to be also
enclosed within an #if IS_ENABLED(CONFIG_DMI) otherwise chances are that
we are going to get defined but unused warnings?


>  		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;
>  			}

Should this hunk be part of patch 3?
-- 
Florian

[toc] | [prev] | [next] | [standalone]

#1740598 — Re: [PATCH 3/3] arm64: cpuinfo: display product info in /proc/cpuinfo

On 26/09/17 23:23, Al Stone wrote:
> 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 <ahs3@redhat.com>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will.deacon@arm.com>
> Cc: Suzuki K Poulose <suzuki.poulose@arm.com>
> Cc: Mark Rutland <mark.rutland@arm.com>
> ---
[...]
> @@ -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));

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

Robin.

> +
> +			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) {

[toc] | [prev] | [next] | [standalone]

#1740746 — Re: [PATCH 3/3] arm64: cpuinfo: display product info in /proc/cpuinfo

Hi,

On Wed, Sep 27, 2017 at 11:42:07AM +0100, Robin Murphy wrote:
> On 26/09/17 23:23, Al Stone wrote:
> > +			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?

Just to make Robin's point a little clearer, MIDR_EL1.Variant is
IMPLEMENTATION DEFINED, and doesn't describe the ARMv8.x architecture
revision.

For example, on Cortex A57 is contains the major revision number of the
CPU, and is 1 for any r1pY Cortex-A57 (e.g. those on Juno R1).

For better or worse, the architecture provides us no mechanism to
determine the architecture revision.

Thanks,
Mark.

[toc] | [prev] | [next] | [standalone]

#1740633 — Re: [PATCH 3/3] arm64: cpuinfo: display product info in /proc/cpuinfo

Hi Al,

On Tue, Sep 26, 2017 at 04:23:24PM -0600, Al Stone wrote:
> 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.

As mentioned on the cover letter, NAK to modifiying /proc/cpuinfo.

I note this is all DMI information, which I thought we already exposed
under sysfs (in an architecture-neutral format).

Is that not the case?

... or do existing tools not pick that up today?

Thanks,
Mark.

> 
> Signed-off-by: Al Stone <ahs3@redhat.com>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will.deacon@arm.com>
> Cc: Suzuki K Poulose <suzuki.poulose@arm.com>
> Cc: Mark Rutland <mark.rutland@arm.com>
> ---
>  arch/arm64/kernel/cpuinfo.c | 135 +++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 134 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/kernel/cpuinfo.c b/arch/arm64/kernel/cpuinfo.c
> index 0b4261884862..6a9dbad5ee3f 100644
> --- a/arch/arm64/kernel/cpuinfo.c
> +++ b/arch/arm64/kernel/cpuinfo.c
> @@ -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;
>  			}
>  		}
> -- 
> 2.13.5
> 

[toc] | [prev] | [next] | [standalone]

#1740262 — [PATCH 2/3] arm64: cpuinfo: add human readable CPU names to /proc/cpuinfo

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 <ahs3@redhat.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
Cc: Suzuki K Poulose <suzuki.poulose@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
---
 arch/arm64/kernel/cpuinfo.c | 84 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 84 insertions(+)

diff --git a/arch/arm64/kernel/cpuinfo.c b/arch/arm64/kernel/cpuinfo.c
index e505007138eb..0b4261884862 100644
--- a/arch/arm64/kernel/cpuinfo.c
+++ b/arch/arm64/kernel/cpuinfo.c
@@ -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);
-- 
2.13.5

[toc] | [prev] | [next] | [standalone]

#1740594 — Re: [PATCH 2/3] arm64: cpuinfo: add human readable CPU names to /proc/cpuinfo

On 26/09/17 23:23, Al Stone wrote:
> 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

How valuable is an incomplete interface really? If users who want to
decode MIDRs are going to have to rely on (pretty trivial ) userspace
tools anyway when their stable distro kernel doesn't know their spangly
new hardware, why does the kernel need to bother at all.

The fact is that we already do the exact same thing as x86 - we print
exactly what the ID registers say. The fact that on x86 some of those
values happen to form a readable ASCII string is a different matter.

> Signed-off-by: Al Stone <ahs3@redhat.com>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will.deacon@arm.com>
> Cc: Suzuki K Poulose <suzuki.poulose@arm.com>
> Cc: Mark Rutland <mark.rutland@arm.com>
> ---
>  arch/arm64/kernel/cpuinfo.c | 84 +++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 84 insertions(+)
> 
> diff --git a/arch/arm64/kernel/cpuinfo.c b/arch/arm64/kernel/cpuinfo.c
> index e505007138eb..0b4261884862 100644
> --- a/arch/arm64/kernel/cpuinfo.c
> +++ b/arch/arm64/kernel/cpuinfo.c
> @@ -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 */

So for a fair chunk of *current* server-class hardware, we'll be
printing "unknown" already. Great.

Robin.

> +};
> +
> +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);
> 

[toc] | [prev] | [next] | [standalone]

#1740624 — Re: [PATCH 2/3] arm64: cpuinfo: add human readable CPU names to /proc/cpuinfo

Hi Al,

On Tue, Sep 26, 2017 at 04:23:23PM -0600, Al Stone wrote:
> 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

As mentioned in the cover letter, and as with previous patches
attempting to do the same thing:

NAK to this patch.

In general, modifying /proc/cpuinfo comes with the risk of breaking
applications that (for better or worse) try to parse it mechanically.
Further, such modifications add to the problem by introducing yet more
ABI state applications may or may not end up depending on to function
(even if they don't require or use the information at all).

Specifically regarding mapping MIDRs to strings in the kernel, there are
a number of problems that make this undesirable, including (but not
limited to):

* As mentioned in the commit message, this cannot be complete, and is
  thus unreliable. We cannot know the set of CPUs that will exist in the
  future, and thus this practically mandates a kernel upgrade to use new
  CPUs, solely for a nicety.

* The names of CPUs can change, so there's not necessarily a single
  correct name to expose. As seen with Cortex-A12 and Cortex-A17 [1],
  where some users were upset if their CPU was considered to be a
  Cortex-A12 rather than a Cortex-A17.

  This embeds a political problem into kernel ABI.

* No matter how many times we tell them not to, applications *will* try
  to parse this. Any change (e.g. typo fixing or naming updates) will
  break some applications. An unexpectedly long CPU name may break
  parses using a small buffer.

  Worse, this could change over a kernel update, as a cpu goes from
  being:
  
    CPU name: unknown
  
  ... to being:

    CPU name: awesome super CPU 2000xx-super-mega-long-name-edition

  ... which could break applications, and some could argue that this is
  a kernel-side regression breaking userspace.

* This information can be derived from the MIDR and REVIDR. The decoded
  MIDR has always been exposed under /proc/cpuinfo, and both are exposed
  in a structured way under:

  /sys/devices/system/cpu/cpu${N}/regs/identification

  This duplicates information that way already expose, but in an
  unreliable fashion.

* This requires us to maintain an ever growing mapping of CPU IDs to
  strings.

* For similar reasons, this information is not exposed by 32-bit ARM,
  and further bifurcates our /proc/cpuinfo formats.

These problems cannot be solved kernel-side, and given this, I will
continue to NAK patches which attempt to decode the MIDR to human
readable strings.

Thanks,
Mark.

[1] https://community.arm.com/processors/b/blog/posts/arm-cortex-a17-cortex-a12-processor-update?CommentId=85c9dea4-1c9a-460c-a7b6-dcf59caab43d&pi10955=99

> 
> Signed-off-by: Al Stone <ahs3@redhat.com>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will.deacon@arm.com>
> Cc: Suzuki K Poulose <suzuki.poulose@arm.com>
> Cc: Mark Rutland <mark.rutland@arm.com>
> ---
>  arch/arm64/kernel/cpuinfo.c | 84 +++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 84 insertions(+)
> 
> diff --git a/arch/arm64/kernel/cpuinfo.c b/arch/arm64/kernel/cpuinfo.c
> index e505007138eb..0b4261884862 100644
> --- a/arch/arm64/kernel/cpuinfo.c
> +++ b/arch/arm64/kernel/cpuinfo.c
> @@ -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);
> -- 
> 2.13.5
> 

[toc] | [prev] | [next] | [standalone]

#1740593 — Re: [PATCH 0/3] arm64: cpuinfo: make /proc/cpuinfo more human-readable

Hi Al,

On Tue, Sep 26, 2017 at 04:23:21PM -0600, Al Stone wrote:
> 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?

Thanks,
Mark.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web