[PATCH] arm64: cpuinfo: Include cleartext implementer and part strings

[PATCH] arm64: cpuinfo: Include cleartext implementer and part strings

[Olof Johansson]

There's some use in printing out what the implementer and part numbers
decode to for cases where they're known.

I filled in the table based on public information; mostly from ARM TRMs
and other tools (and some of the SSBD tables in the kernel, etc).

Apple IDs came from
https://github.com/apple/darwin-xnu/blob/master/osfmk/arm/cpuid.h

Signed-off-by: Olof Johansson <olof@lixom.net>
---
 arch/arm64/kernel/cpuinfo.c | 79 +++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 76 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/kernel/cpuinfo.c b/arch/arm64/kernel/cpuinfo.c
index e9ab7b3..9a7c25d 100644
--- a/arch/arm64/kernel/cpuinfo.c
+++ b/arch/arm64/kernel/cpuinfo.c
@@ -121,6 +121,67 @@ static const char *const compat_hwcap2_str[] = {
 };
 #endif /* CONFIG_COMPAT */
 
+struct midr_info {
+	unsigned int mask;
+	unsigned int val;
+	char *id;
+};
+
+static const struct midr_info midr_part[] = {
+	// ARM
+	{ .mask = 0xff00fff0, .val = 0x4100d030, .id = "Cortex-A53" },
+	{ .mask = 0xff00fff0, .val = 0x4100d040, .id = "Cortex-A35" },
+	{ .mask = 0xff00fff0, .val = 0x4100d070, .id = "Cortex-A57" },
+	{ .mask = 0xff00fff0, .val = 0x4100d080, .id = "Cortex-A72" },
+	{ .mask = 0xff00fff0, .val = 0x4100d090, .id = "Cortex-A73" },
+	{ .mask = 0xff00fff0, .val = 0x4100d0a0, .id = "Cortex-A75" },
+	{ .mask = 0xff00fff0, .val = 0x4100d0f0, .id = "Cortex-A55" },
+	// Broadcom
+	{ .mask = 0xff00fff0, .val = 0x42001000, .id = "Brahma-B53" },
+	{ .mask = 0xff00fff0, .val = 0x42005160, .id = "ThunderX2" },
+	// Cavium
+	{ .mask = 0xff00fff0, .val = 0x43000a00, .id = "ThunderX" },
+	{ .mask = 0xff00fff0, .val = 0x43000a10, .id = "ThunderX 88xx" },
+	{ .mask = 0xff00fff0, .val = 0x43000a20, .id = "ThunderX 81xx" },
+	{ .mask = 0xff00fff0, .val = 0x43000a30, .id = "ThunderX 83xx" },
+	{ .mask = 0xff00fff0, .val = 0x43000af0, .id = "ThunderX2 99xx" },
+	// Nvidia
+	{ .mask = 0xff00fff0, .val = 0x4e000000, .id = "Denver" },
+	{ .mask = 0xff00fff0, .val = 0x4e000030, .id = "Denver 2" },
+	// Applied Micro
+	{ .mask = 0xff00fff0, .val = 0x50000000, .id = "X-Gene" },
+	// Qualcomm
+	{ .mask = 0xff00fff0, .val = 0x51002010, .id = "Kryo" },
+	{ .mask = 0xff00fff0, .val = 0x51002050, .id = "Kryo" },
+	{ .mask = 0xff00fff0, .val = 0x51002110, .id = "Kryo" },
+	{ .mask = 0xff00fff0, .val = 0x51008000, .id = "Falkor V1/Kryo" },
+	{ .mask = 0xff00fff0, .val = 0x51008010, .id = "Kryo V2" },
+	{ .mask = 0xff00fff0, .val = 0x5100c000, .id = "Falkor" },
+	{ .mask = 0xff00fff0, .val = 0x5100c010, .id = "Saphira" },
+	// Samsung
+	{ .mask = 0xff00fff0, .val = 0x53000010, .id = "M1" },
+	// Apple
+	{ .mask = 0xff00fff0, .val = 0x61000010, .id = "Cyclone" },
+	{ .mask = 0xff00fff0, .val = 0x61000020, .id = "Typhoon" },
+	{ .mask = 0xff00fff0, .val = 0x61000030, .id = "Typhoon/Capri" },
+	{ .mask = 0xff00fff0, .val = 0x61000040, .id = "Twister" },
+	{ .mask = 0xff00fff0, .val = 0x61000050, .id = "Twister/Elba/Malta" },
+	{ .mask = 0xff00fff0, .val = 0x61000060, .id = "Hurricane" },
+	{ .mask = 0xff00fff0, .val = 0x61000070, .id = "Hurricane/Myst" },
+};
+
+static const struct midr_info midr_impl[] = {
+	{ .mask = 0xff000000, .val = 0x41000000, .id = "ARM" },
+	{ .mask = 0xff000000, .val = 0x42000000, .id = "Broadcom" },
+	{ .mask = 0xff000000, .val = 0x43000000, .id = "Cavium" },
+	{ .mask = 0xff000000, .val = 0x4d000000, .id = "Motorola" },
+	{ .mask = 0xff000000, .val = 0x4e000000, .id = "Nvidia" },
+	{ .mask = 0xff000000, .val = 0x50000000, .id = "Applied Micro" },
+	{ .mask = 0xff000000, .val = 0x51000000, .id = "Qualcomm" },
+	{ .mask = 0xff000000, .val = 0x53000000, .id = "Samsung" },
+	{ .mask = 0xff000000, .val = 0x61000000, .id = "Apple" },
+};
+
 static int c_show(struct seq_file *m, void *v)
 {
 	int i, j;
@@ -129,6 +190,16 @@ 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;
+		char *impl = NULL;
+		char *part = NULL;
+
+		for (j = 0; !impl && j < ARRAY_SIZE(midr_impl); j++)
+			if ((midr & midr_impl[j].mask) == midr_impl[j].val)
+				impl = midr_impl[j].id;
+
+		for (j = 0; !part && j < ARRAY_SIZE(midr_part); j++)
+			if ((midr & midr_part[j].mask) == midr_part[j].val)
+				part = midr_part[j].id;
 
 		/*
 		 * glibc reads /proc/cpuinfo to determine the number of
@@ -168,11 +239,12 @@ static int c_show(struct seq_file *m, void *v)
 		}
 		seq_puts(m, "\n");
 
-		seq_printf(m, "CPU implementer\t: 0x%02x\n",
-			   MIDR_IMPLEMENTOR(midr));
+		seq_printf(m, "CPU implementer\t: 0x%02x (%s)\n",
+			   MIDR_IMPLEMENTOR(midr), impl ? : "unknown");
 		seq_printf(m, "CPU architecture: 8\n");
 		seq_printf(m, "CPU variant\t: 0x%x\n", MIDR_VARIANT(midr));
-		seq_printf(m, "CPU part\t: 0x%03x\n", MIDR_PARTNUM(midr));
+		seq_printf(m, "CPU part\t: 0x%03x (%s)\n", MIDR_PARTNUM(midr),
+			   part ? : "unknown");
 		seq_printf(m, "CPU revision\t: %d\n\n", MIDR_REVISION(midr));
 	}
 
@@ -300,6 +372,7 @@ static int __init cpuinfo_regs_init(void)
 	}
 	return 0;
 }
+
 static void cpuinfo_detect_icache_policy(struct cpuinfo_arm64 *info)
 {
 	unsigned int cpu = smp_processor_id();
-- 
2.8.6

[PATCH] arm64: cpuinfo: Include cleartext implementer and part strings

[Olof Johansson]

There's some use in printing out what the implementer and part numbers
decode to for cases where they're known.

I filled in the table based on public information; mostly from ARM TRMs
and other tools (and some of the SSBD tables in the kernel, etc).

Apple IDs came from
https://github.com/apple/darwin-xnu/blob/master/osfmk/arm/cpuid.h

Signed-off-by: Olof Johansson <olof@lixom.net>
---
 arch/arm64/kernel/cpuinfo.c | 79 +++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 76 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/kernel/cpuinfo.c b/arch/arm64/kernel/cpuinfo.c
index e9ab7b3..9a7c25d 100644
--- a/arch/arm64/kernel/cpuinfo.c
+++ b/arch/arm64/kernel/cpuinfo.c
@@ -121,6 +121,67 @@ static const char *const compat_hwcap2_str[] = {
 };
 #endif /* CONFIG_COMPAT */
 
+struct midr_info {
+	unsigned int mask;
+	unsigned int val;
+	char *id;
+};
+
+static const struct midr_info midr_part[] = {
+	// ARM
+	{ .mask = 0xff00fff0, .val = 0x4100d030, .id = "Cortex-A53" },
+	{ .mask = 0xff00fff0, .val = 0x4100d040, .id = "Cortex-A35" },
+	{ .mask = 0xff00fff0, .val = 0x4100d070, .id = "Cortex-A57" },
+	{ .mask = 0xff00fff0, .val = 0x4100d080, .id = "Cortex-A72" },
+	{ .mask = 0xff00fff0, .val = 0x4100d090, .id = "Cortex-A73" },
+	{ .mask = 0xff00fff0, .val = 0x4100d0a0, .id = "Cortex-A75" },
+	{ .mask = 0xff00fff0, .val = 0x4100d0f0, .id = "Cortex-A55" },
+	// Broadcom
+	{ .mask = 0xff00fff0, .val = 0x42001000, .id = "Brahma-B53" },
+	{ .mask = 0xff00fff0, .val = 0x42005160, .id = "ThunderX2" },
+	// Cavium
+	{ .mask = 0xff00fff0, .val = 0x43000a00, .id = "ThunderX" },
+	{ .mask = 0xff00fff0, .val = 0x43000a10, .id = "ThunderX 88xx" },
+	{ .mask = 0xff00fff0, .val = 0x43000a20, .id = "ThunderX 81xx" },
+	{ .mask = 0xff00fff0, .val = 0x43000a30, .id = "ThunderX 83xx" },
+	{ .mask = 0xff00fff0, .val = 0x43000af0, .id = "ThunderX2 99xx" },
+	// Nvidia
+	{ .mask = 0xff00fff0, .val = 0x4e000000, .id = "Denver" },
+	{ .mask = 0xff00fff0, .val = 0x4e000030, .id = "Denver 2" },
+	// Applied Micro
+	{ .mask = 0xff00fff0, .val = 0x50000000, .id = "X-Gene" },
+	// Qualcomm
+	{ .mask = 0xff00fff0, .val = 0x51002010, .id = "Kryo" },
+	{ .mask = 0xff00fff0, .val = 0x51002050, .id = "Kryo" },
+	{ .mask = 0xff00fff0, .val = 0x51002110, .id = "Kryo" },
+	{ .mask = 0xff00fff0, .val = 0x51008000, .id = "Falkor V1/Kryo" },
+	{ .mask = 0xff00fff0, .val = 0x51008010, .id = "Kryo V2" },
+	{ .mask = 0xff00fff0, .val = 0x5100c000, .id = "Falkor" },
+	{ .mask = 0xff00fff0, .val = 0x5100c010, .id = "Saphira" },
+	// Samsung
+	{ .mask = 0xff00fff0, .val = 0x53000010, .id = "M1" },
+	// Apple
+	{ .mask = 0xff00fff0, .val = 0x61000010, .id = "Cyclone" },
+	{ .mask = 0xff00fff0, .val = 0x61000020, .id = "Typhoon" },
+	{ .mask = 0xff00fff0, .val = 0x61000030, .id = "Typhoon/Capri" },
+	{ .mask = 0xff00fff0, .val = 0x61000040, .id = "Twister" },
+	{ .mask = 0xff00fff0, .val = 0x61000050, .id = "Twister/Elba/Malta" },
+	{ .mask = 0xff00fff0, .val = 0x61000060, .id = "Hurricane" },
+	{ .mask = 0xff00fff0, .val = 0x61000070, .id = "Hurricane/Myst" },
+};
+
+static const struct midr_info midr_impl[] = {
+	{ .mask = 0xff000000, .val = 0x41000000, .id = "ARM" },
+	{ .mask = 0xff000000, .val = 0x42000000, .id = "Broadcom" },
+	{ .mask = 0xff000000, .val = 0x43000000, .id = "Cavium" },
+	{ .mask = 0xff000000, .val = 0x4d000000, .id = "Motorola" },
+	{ .mask = 0xff000000, .val = 0x4e000000, .id = "Nvidia" },
+	{ .mask = 0xff000000, .val = 0x50000000, .id = "Applied Micro" },
+	{ .mask = 0xff000000, .val = 0x51000000, .id = "Qualcomm" },
+	{ .mask = 0xff000000, .val = 0x53000000, .id = "Samsung" },
+	{ .mask = 0xff000000, .val = 0x61000000, .id = "Apple" },
+};
+
 static int c_show(struct seq_file *m, void *v)
 {
 	int i, j;
@@ -129,6 +190,16 @@ 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;
+		char *impl = NULL;
+		char *part = NULL;
+
+		for (j = 0; !impl && j < ARRAY_SIZE(midr_impl); j++)
+			if ((midr & midr_impl[j].mask) == midr_impl[j].val)
+				impl = midr_impl[j].id;
+
+		for (j = 0; !part && j < ARRAY_SIZE(midr_part); j++)
+			if ((midr & midr_part[j].mask) == midr_part[j].val)
+				part = midr_part[j].id;
 
 		/*
 		 * glibc reads /proc/cpuinfo to determine the number of
@@ -168,11 +239,12 @@ static int c_show(struct seq_file *m, void *v)
 		}
 		seq_puts(m, "\n");
 
-		seq_printf(m, "CPU implementer\t: 0x%02x\n",
-			   MIDR_IMPLEMENTOR(midr));
+		seq_printf(m, "CPU implementer\t: 0x%02x (%s)\n",
+			   MIDR_IMPLEMENTOR(midr), impl ? : "unknown");
 		seq_printf(m, "CPU architecture: 8\n");
 		seq_printf(m, "CPU variant\t: 0x%x\n", MIDR_VARIANT(midr));
-		seq_printf(m, "CPU part\t: 0x%03x\n", MIDR_PARTNUM(midr));
+		seq_printf(m, "CPU part\t: 0x%03x (%s)\n", MIDR_PARTNUM(midr),
+			   part ? : "unknown");
 		seq_printf(m, "CPU revision\t: %d\n\n", MIDR_REVISION(midr));
 	}
 
@@ -300,6 +372,7 @@ static int __init cpuinfo_regs_init(void)
 	}
 	return 0;
 }
+
 static void cpuinfo_detect_icache_policy(struct cpuinfo_arm64 *info)
 {
 	unsigned int cpu = smp_processor_id();
-- 
2.8.6

Re: [PATCH] arm64: cpuinfo: Include cleartext implementer and part strings

[Marc Zyngier]

Hi Olof,

On 15/07/18 04:53, Olof Johansson wrote:
> There's some use in printing out what the implementer and part numbers
> decode to for cases where they're known.
> 
> I filled in the table based on public information; mostly from ARM TRMs
> and other tools (and some of the SSBD tables in the kernel, etc).
> 
> Apple IDs came from
> https://github.com/apple/darwin-xnu/blob/master/osfmk/arm/cpuid.h
> 
> Signed-off-by: Olof Johansson <olof@lixom.net>
> ---
>  arch/arm64/kernel/cpuinfo.c | 79 +++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 76 insertions(+), 3 deletions(-)

[...]

The same thing pops up every so often. And the answer is the same each time:

- it breaks an existing userspace ABI by changing the format of cpuinfo
- it is unmaintainable in the long run
- userspace already has this information by simply running "lscpu"

What has changed?

	M.
-- 
Jazz is not dead. It just smells funny...

[PATCH] arm64: cpuinfo: Include cleartext implementer and part strings

[Marc Zyngier]

Hi Olof,

On 15/07/18 04:53, Olof Johansson wrote:
> There's some use in printing out what the implementer and part numbers
> decode to for cases where they're known.
> 
> I filled in the table based on public information; mostly from ARM TRMs
> and other tools (and some of the SSBD tables in the kernel, etc).
> 
> Apple IDs came from
> https://github.com/apple/darwin-xnu/blob/master/osfmk/arm/cpuid.h
> 
> Signed-off-by: Olof Johansson <olof@lixom.net>
> ---
>  arch/arm64/kernel/cpuinfo.c | 79 +++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 76 insertions(+), 3 deletions(-)

[...]

The same thing pops up every so often. And the answer is the same each time:

- it breaks an existing userspace ABI by changing the format of cpuinfo
- it is unmaintainable in the long run
- userspace already has this information by simply running "lscpu"

What has changed?

	M.
-- 
Jazz is not dead. It just smells funny...

Re: [PATCH] arm64: cpuinfo: Include cleartext implementer and part strings

[Olof Johansson]

On Mon, Jul 16, 2018 at 2:17 AM, Marc Zyngier <marc.zyngier@arm.com> wrote:
> Hi Olof,
>
> On 15/07/18 04:53, Olof Johansson wrote:
>> There's some use in printing out what the implementer and part numbers
>> decode to for cases where they're known.
>>
>> I filled in the table based on public information; mostly from ARM TRMs
>> and other tools (and some of the SSBD tables in the kernel, etc).
>>
>> Apple IDs came from
>> https://github.com/apple/darwin-xnu/blob/master/osfmk/arm/cpuid.h
>>
>> Signed-off-by: Olof Johansson <olof@lixom.net>
>> ---
>>  arch/arm64/kernel/cpuinfo.c | 79 +++++++++++++++++++++++++++++++++++++++++++--
>>  1 file changed, 76 insertions(+), 3 deletions(-)
>
> [...]
>
> The same thing pops up every so often. And the answer is the same each time:

Ever thought about why it pops up?

> - it breaks an existing userspace ABI by changing the format of cpuinfo

Most tools likely rely on a per-line format, and in this case they're
likely interpreting the contents after the ':' as an integer. Adding
something to the line might or might not break them, agreed.

How about introducing a "CPU model" line similar to x86's cleartext one?

> - it is unmaintainable in the long run

False. Chances are you already need details more fine-grained than
this for errata and quirks, we already do for SSBD. And if it lacks an
entry it's not the end of the world.

> - userspace already has this information by simply running "lscpu"
>
> What has changed?

What has changed is that ARM(64) is moving from a
custom-kernel-custom-userspace world to one where you might be
upreving kernels and userspace separately, and you might be using an
older userspace with a newer machine and a newer kernel. Having to
update lscpu is an extra step and extra friction to making it behave
nicely.

There's also a growing expectation of the system to behave more like
x86, especially when it comes to trivial ways of detecting what kind
of machine you are running on. On x86 it's been trivial to look at
/proc/cpuinfo since it is provided by the platform there. Nobody wants
to change their implementation from having to read a file to exec a
program, dealing with errors, and parsing its output (sure, easier
from scripts, but more awkward from real source).

Having the information exported from the kernel is a reasonable way to
do it. The other extreme is that the kernel should just ever have
exported the raw MIDR value.


-Olof

[PATCH] arm64: cpuinfo: Include cleartext implementer and part strings

[Olof Johansson]

On Mon, Jul 16, 2018 at 2:17 AM, Marc Zyngier <marc.zyngier@arm.com> wrote:
> Hi Olof,
>
> On 15/07/18 04:53, Olof Johansson wrote:
>> There's some use in printing out what the implementer and part numbers
>> decode to for cases where they're known.
>>
>> I filled in the table based on public information; mostly from ARM TRMs
>> and other tools (and some of the SSBD tables in the kernel, etc).
>>
>> Apple IDs came from
>> https://github.com/apple/darwin-xnu/blob/master/osfmk/arm/cpuid.h
>>
>> Signed-off-by: Olof Johansson <olof@lixom.net>
>> ---
>>  arch/arm64/kernel/cpuinfo.c | 79 +++++++++++++++++++++++++++++++++++++++++++--
>>  1 file changed, 76 insertions(+), 3 deletions(-)
>
> [...]
>
> The same thing pops up every so often. And the answer is the same each time:

Ever thought about why it pops up?

> - it breaks an existing userspace ABI by changing the format of cpuinfo

Most tools likely rely on a per-line format, and in this case they're
likely interpreting the contents after the ':' as an integer. Adding
something to the line might or might not break them, agreed.

How about introducing a "CPU model" line similar to x86's cleartext one?

> - it is unmaintainable in the long run

False. Chances are you already need details more fine-grained than
this for errata and quirks, we already do for SSBD. And if it lacks an
entry it's not the end of the world.

> - userspace already has this information by simply running "lscpu"
>
> What has changed?

What has changed is that ARM(64) is moving from a
custom-kernel-custom-userspace world to one where you might be
upreving kernels and userspace separately, and you might be using an
older userspace with a newer machine and a newer kernel. Having to
update lscpu is an extra step and extra friction to making it behave
nicely.

There's also a growing expectation of the system to behave more like
x86, especially when it comes to trivial ways of detecting what kind
of machine you are running on. On x86 it's been trivial to look at
/proc/cpuinfo since it is provided by the platform there. Nobody wants
to change their implementation from having to read a file to exec a
program, dealing with errors, and parsing its output (sure, easier
from scripts, but more awkward from real source).

Having the information exported from the kernel is a reasonable way to
do it. The other extreme is that the kernel should just ever have
exported the raw MIDR value.


-Olof

Re: [PATCH] arm64: cpuinfo: Include cleartext implementer and part strings

[Mark Rutland]

Hi Olof,

On Mon, Jul 16, 2018 at 07:34:05AM -0700, Olof Johansson wrote:
> On Mon, Jul 16, 2018 at 2:17 AM, Marc Zyngier <marc.zyngier@arm.com> wrote:
> > On 15/07/18 04:53, Olof Johansson wrote:
> >> There's some use in printing out what the implementer and part numbers
> >> decode to for cases where they're known.
> >>
> >> I filled in the table based on public information; mostly from ARM TRMs
> >> and other tools (and some of the SSBD tables in the kernel, etc).
> >>
> >> Apple IDs came from
> >> https://github.com/apple/darwin-xnu/blob/master/osfmk/arm/cpuid.h

I appreciate that the format of /proc/cpuinfo isn't as nice as one might
like as a human reader.

For better or worse, the vast majority of readers of /proc/cpuinfo are
applications (be they scripts or binaries), and we must treat the format
and contents of /proc/cpuinfo as ABI. We've been burned by /proc/cpuinfo
format changes in the past, so we want to minimize the potential for ABI
issues.

This change is only useful to human readers of /proc/cpuinfo, mandates
perpetual maintenance work, and comes with a very strong risk of subtle
ABI breakage (both for this patch and future patches it implies will be
necessary).

So sorry, but I must NAK this patch, as with prior patches changing the
format of /proc/cpuinfo.

> > The same thing pops up every so often. And the answer is the same each time:
> 
> Ever thought about why it pops up?

In some cases because the person in question isn't sure what hardware
they're on. In most cases because people are used to x86, and haven't
considered what exactly they're trying to achieve.

We've repeatedly been asked for strings *by application developers*,
despite the MIDR being sufficient and already available. Generally,
developers are happy to use MIDRs once they realise this.

Having the string leads people to try to parse it, even though it cannot
be reliable.

> > - it breaks an existing userspace ABI by changing the format of cpuinfo
> 
> Most tools likely rely on a per-line format, and in this case they're
> likely interpreting the contents after the ':' as an integer. Adding
> something to the line might or might not break them, agreed.
> 
> How about introducing a "CPU model" line similar to x86's cleartext one?
> 
> > - it is unmaintainable in the long run
> 
> False. Chances are you already need details more fine-grained than
> this for errata and quirks, we already do for SSBD.

For SSBD, we don't look at the MIDR, and rely solely on the FW to
identify the presence of a workaround. Going forwards, we'll do likewise
as this is the only way to be future-proof and generic.

> And if it lacks an entry it's not the end of the world.

I don't think we can be that relaxed about this; it's quite easy to see
that people *will* write software that relies on the string (or
properties thereof) in ways that we cannot manage.

People will do silly things like try to read the string into a
fixed-size buffer. That works when the string is "Unknown", but falls
apart on the next kernel release when it's "awesome super CPU
2000xx-super-mega-long-name-edition". We don't discover this problem for
months, whereupon other applications are reliant upon the new string,
and now we're stuck with a horrible ABI bug that we can't fix.

> > - userspace already has this information by simply running "lscpu"
> >
> > What has changed?
> 
> What has changed is that ARM(64) is moving from a
> custom-kernel-custom-userspace world to one where you might be
> upreving kernels and userspace separately, and you might be using an
> older userspace with a newer machine and a newer kernel.

Sure, and this is exactly why we're trying to ensure that the behaviour
is *guaranteed* to be consistent across kernels -- so whichever kernel
is used, userspace should behave the same.

If userspace *needs* to know about a CPU, it can identify the CPU based
on the MIDR (and REVIDR), and this will work regardless of kernel
version. GCC does this today.

If userspace *wants* to know about a CPU (e.g. to expose a
human-friendly name), then a userspace database is fine. In the majority
of cases it's *vastly* easier to update a userspace application than it
is to update the kernel, and it's always possible to build the latest
binary yourself.

There are a number of cases where using strings is going to be
problematic in practice, e.g.

* Every out-of-tree SoC port is going to invent values that may not
  match the values mainline settles on. Then people will complain that
  their software only works on one or the other, because it makes some
  decision base on the model name.

* Every distro is going to differ in the set of IDs they backport, if
  any.  If your software relies on the MIDR, it will work everywhere. If
  it tries to parse the string, it will behave erratically across
  distros.

* We may get the matching logic for wrong, and be overly permissive when
  matching MIDR -> string. e.g. we ignore the low 4 bits for some range
  of MIDRs, but the vendor only uses 0x0-0xc, and assigns 0xd-0xf a
  different name.

* Parts (and vendors) don't always have a fix name. What happens when a
  vendor renames their product (or themselves)? Remember Cortex-A12?

> There's also a growing expectation of the system to behave more like
> x86, especially when it comes to trivial ways of detecting what kind
> of machine you are running on. On x86 it's been trivial to look at
> /proc/cpuinfo since it is provided by the platform there.

It's trivial because *the CPU provides this*, via CPUID. See
get_model_name() in arch/x86/kernel/cpu/common.c. That is how x86 can
expose a model name in a future-proof manner, and without a maintenance
nightmare.

On arm, we have the MIDR_EL1 and REVIDR_EL1. We simply do not have the
same information, and trying to guess at it is a losing game.

> Nobody wants to change their implementation from having to read a file
> to exec a program, dealing with errors, and parsing its output (sure,
> easier from scripts, but more awkward from real source).

Unfortunately, this ship has long sailed. The format of /proc/cpuinfo
differs across architectures, and it is not something that can be relied
upon by portable applications.

> Having the information exported from the kernel is a reasonable way to
> do it.

While I can appreciate this may seem simple, it comes with far more
problems than it solves.

> The other extreme is that the kernel should just ever have exported
> the raw MIDR value.

We expose the raw MIDR and REVIDR values under sysfs. We expose the
decomposed value (in the format defined by the architecture) because we
have to for compatibility with existing software.

To be honest, my personal preference would be to delete /proc/cpuinfo if
we could.

Thanks,
Mark.

[PATCH] arm64: cpuinfo: Include cleartext implementer and part strings

[Mark Rutland]

Hi Olof,

On Mon, Jul 16, 2018 at 07:34:05AM -0700, Olof Johansson wrote:
> On Mon, Jul 16, 2018 at 2:17 AM, Marc Zyngier <marc.zyngier@arm.com> wrote:
> > On 15/07/18 04:53, Olof Johansson wrote:
> >> There's some use in printing out what the implementer and part numbers
> >> decode to for cases where they're known.
> >>
> >> I filled in the table based on public information; mostly from ARM TRMs
> >> and other tools (and some of the SSBD tables in the kernel, etc).
> >>
> >> Apple IDs came from
> >> https://github.com/apple/darwin-xnu/blob/master/osfmk/arm/cpuid.h

I appreciate that the format of /proc/cpuinfo isn't as nice as one might
like as a human reader.

For better or worse, the vast majority of readers of /proc/cpuinfo are
applications (be they scripts or binaries), and we must treat the format
and contents of /proc/cpuinfo as ABI. We've been burned by /proc/cpuinfo
format changes in the past, so we want to minimize the potential for ABI
issues.

This change is only useful to human readers of /proc/cpuinfo, mandates
perpetual maintenance work, and comes with a very strong risk of subtle
ABI breakage (both for this patch and future patches it implies will be
necessary).

So sorry, but I must NAK this patch, as with prior patches changing the
format of /proc/cpuinfo.

> > The same thing pops up every so often. And the answer is the same each time:
> 
> Ever thought about why it pops up?

In some cases because the person in question isn't sure what hardware
they're on. In most cases because people are used to x86, and haven't
considered what exactly they're trying to achieve.

We've repeatedly been asked for strings *by application developers*,
despite the MIDR being sufficient and already available. Generally,
developers are happy to use MIDRs once they realise this.

Having the string leads people to try to parse it, even though it cannot
be reliable.

> > - it breaks an existing userspace ABI by changing the format of cpuinfo
> 
> Most tools likely rely on a per-line format, and in this case they're
> likely interpreting the contents after the ':' as an integer. Adding
> something to the line might or might not break them, agreed.
> 
> How about introducing a "CPU model" line similar to x86's cleartext one?
> 
> > - it is unmaintainable in the long run
> 
> False. Chances are you already need details more fine-grained than
> this for errata and quirks, we already do for SSBD.

For SSBD, we don't look at the MIDR, and rely solely on the FW to
identify the presence of a workaround. Going forwards, we'll do likewise
as this is the only way to be future-proof and generic.

> And if it lacks an entry it's not the end of the world.

I don't think we can be that relaxed about this; it's quite easy to see
that people *will* write software that relies on the string (or
properties thereof) in ways that we cannot manage.

People will do silly things like try to read the string into a
fixed-size buffer. That works when the string is "Unknown", but falls
apart on the next kernel release when it's "awesome super CPU
2000xx-super-mega-long-name-edition". We don't discover this problem for
months, whereupon other applications are reliant upon the new string,
and now we're stuck with a horrible ABI bug that we can't fix.

> > - userspace already has this information by simply running "lscpu"
> >
> > What has changed?
> 
> What has changed is that ARM(64) is moving from a
> custom-kernel-custom-userspace world to one where you might be
> upreving kernels and userspace separately, and you might be using an
> older userspace with a newer machine and a newer kernel.

Sure, and this is exactly why we're trying to ensure that the behaviour
is *guaranteed* to be consistent across kernels -- so whichever kernel
is used, userspace should behave the same.

If userspace *needs* to know about a CPU, it can identify the CPU based
on the MIDR (and REVIDR), and this will work regardless of kernel
version. GCC does this today.

If userspace *wants* to know about a CPU (e.g. to expose a
human-friendly name), then a userspace database is fine. In the majority
of cases it's *vastly* easier to update a userspace application than it
is to update the kernel, and it's always possible to build the latest
binary yourself.

There are a number of cases where using strings is going to be
problematic in practice, e.g.

* Every out-of-tree SoC port is going to invent values that may not
  match the values mainline settles on. Then people will complain that
  their software only works on one or the other, because it makes some
  decision base on the model name.

* Every distro is going to differ in the set of IDs they backport, if
  any.  If your software relies on the MIDR, it will work everywhere. If
  it tries to parse the string, it will behave erratically across
  distros.

* We may get the matching logic for wrong, and be overly permissive when
  matching MIDR -> string. e.g. we ignore the low 4 bits for some range
  of MIDRs, but the vendor only uses 0x0-0xc, and assigns 0xd-0xf a
  different name.

* Parts (and vendors) don't always have a fix name. What happens when a
  vendor renames their product (or themselves)? Remember Cortex-A12?

> There's also a growing expectation of the system to behave more like
> x86, especially when it comes to trivial ways of detecting what kind
> of machine you are running on. On x86 it's been trivial to look at
> /proc/cpuinfo since it is provided by the platform there.

It's trivial because *the CPU provides this*, via CPUID. See
get_model_name() in arch/x86/kernel/cpu/common.c. That is how x86 can
expose a model name in a future-proof manner, and without a maintenance
nightmare.

On arm, we have the MIDR_EL1 and REVIDR_EL1. We simply do not have the
same information, and trying to guess at it is a losing game.

> Nobody wants to change their implementation from having to read a file
> to exec a program, dealing with errors, and parsing its output (sure,
> easier from scripts, but more awkward from real source).

Unfortunately, this ship has long sailed. The format of /proc/cpuinfo
differs across architectures, and it is not something that can be relied
upon by portable applications.

> Having the information exported from the kernel is a reasonable way to
> do it.

While I can appreciate this may seem simple, it comes with far more
problems than it solves.

> The other extreme is that the kernel should just ever have exported
> the raw MIDR value.

We expose the raw MIDR and REVIDR values under sysfs. We expose the
decomposed value (in the format defined by the architecture) because we
have to for compatibility with existing software.

To be honest, my personal preference would be to delete /proc/cpuinfo if
we could.

Thanks,
Mark.

Re: [PATCH] arm64: cpuinfo: Include cleartext implementer and part strings

[Robin Murphy]

On 16/07/18 15:34, Olof Johansson wrote:
[...]
> There's also a growing expectation of the system to behave more like
> x86, especially when it comes to trivial ways of detecting what kind
> of machine you are running on. On x86 it's been trivial to look at
> /proc/cpuinfo since it is provided by the platform there. Nobody wants
> to change their implementation from having to read a file to exec a
> program, dealing with errors, and parsing its output (sure, easier
> from scripts, but more awkward from real source).

The thing with the "be more like x86" argument is that it never actually 
stands up to scrutiny. What is proposed here (again) is for the kernel 
to decode the raw MIDR description of the *CPU core microarchitecture* 
to a human-readable string. What does the nearest Intel box say about 
its microarchitecture name?

cpu family	: 6
model		: 79

OK, I had to go and look up that that apparently implies "Broadwell", so 
the x86 kernel is in fact no more helpful in that regard than arm64.

Furthermore, telling the user that they have have 4 "Cortex-A53" cores 
vs. 4 "part: 0xd03" cores doesn't actually give them any more meaningful 
information about their *system*, because that example reflects about 
half the low-to-mid-range SoCs on the market today. What the "model 
name" on x86 describes is a specific processor SKU, of which Arm has no 
*architectural* equivalent, for reasons which should hopefully be 
obvious from the fundamental differences in the business models driving 
the respective architectures.

And yet whenever /proc/cpuinfo/ comes up, nobody ever seems to ask for 
some kind of SoC identifier in there; it's always just decoding MIDR 
into a microarchitecture name, which implies that all these people want 
is to see a string for the sake of seeing a string, regardless of how 
meaningful it actually might be (and yes, I do realise that in *some* 
cases a CPU core is essentially unique to a particular SoC, but we have 
to look at the big picture here).

In summary; when a small minority of users are complaining that oranges 
aren't apples, the sensible response is not to start debating whether to 
paint the oranges green or red.

Robin.

[PATCH] arm64: cpuinfo: Include cleartext implementer and part strings

[Robin Murphy]

On 16/07/18 15:34, Olof Johansson wrote:
[...]
> There's also a growing expectation of the system to behave more like
> x86, especially when it comes to trivial ways of detecting what kind
> of machine you are running on. On x86 it's been trivial to look at
> /proc/cpuinfo since it is provided by the platform there. Nobody wants
> to change their implementation from having to read a file to exec a
> program, dealing with errors, and parsing its output (sure, easier
> from scripts, but more awkward from real source).

The thing with the "be more like x86" argument is that it never actually 
stands up to scrutiny. What is proposed here (again) is for the kernel 
to decode the raw MIDR description of the *CPU core microarchitecture* 
to a human-readable string. What does the nearest Intel box say about 
its microarchitecture name?

cpu family	: 6
model		: 79

OK, I had to go and look up that that apparently implies "Broadwell", so 
the x86 kernel is in fact no more helpful in that regard than arm64.

Furthermore, telling the user that they have have 4 "Cortex-A53" cores 
vs. 4 "part: 0xd03" cores doesn't actually give them any more meaningful 
information about their *system*, because that example reflects about 
half the low-to-mid-range SoCs on the market today. What the "model 
name" on x86 describes is a specific processor SKU, of which Arm has no 
*architectural* equivalent, for reasons which should hopefully be 
obvious from the fundamental differences in the business models driving 
the respective architectures.

And yet whenever /proc/cpuinfo/ comes up, nobody ever seems to ask for 
some kind of SoC identifier in there; it's always just decoding MIDR 
into a microarchitecture name, which implies that all these people want 
is to see a string for the sake of seeing a string, regardless of how 
meaningful it actually might be (and yes, I do realise that in *some* 
cases a CPU core is essentially unique to a particular SoC, but we have 
to look at the big picture here).

In summary; when a small minority of users are complaining that oranges 
aren't apples, the sensible response is not to start debating whether to 
paint the oranges green or red.

Robin.