DT: CPU h/w id parsing clean-ups and cacheinfo id support

There are various open coded implementions parsing the CPU node 'reg' property which contains the CPU's hardware ID. Introduce a new function, of_get_cpu_hwid(), to read the hardware ID.

All the callers should be DT only code, so no need for an empty function.

Replace open coded parsing of CPU nodes 'reg' property with of_get_cpu_hwid().

Cc Florian Fainelli <[email protected]>

Cc Ray Jui <[email protected]>

Cc Scott Branden <[email protected]>

Cc bcm-kernel-feedback-list at broadcom.com

Cc Russell King <[email protected]>

Signed-off-by Rob Herring <[email protected]>

Replace the open coded parsing of CPU nodes' 'reg' property with of_get_cpu_hwid().

This change drops an error message for missing 'reg' property, but that should not be necessary as the DT tools will ensure 'reg' is present.

Cc Catalin Marinas <[email protected]>

Cc Will Deacon <[email protected]>

Signed-off-by Rob Herring <[email protected]>

---
 arch/arm64/kernel/smp.c | 31 ++-----------------------------
 1 file changed, 2 insertions(+), 29 deletions(-)

Replace the open coded parsing of CPU nodes' 'reg' property with of_get_cpu_hwid().

This change drops an error message for missing 'reg' property, but that should not be necessary as the DT tools will ensure 'reg' is present.

Replace open coded parsing of CPU nodes' 'reg' property with of_get_cpu_hwid().

Cc Jonas Bonn <[email protected]>

Cc Stefan Kristiansson <[email protected]>

Cc Stafford Horne <[email protected]>

Cc openrisc at lists.librecores.org

Signed-off-by Rob Herring <[email protected]>

---
 arch/openrisc/kernel/smp.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

arch/openrisc/kernel/smp.c +0 -0

index 415e209732a3..7d5a4f303a5a 100644

arch/openrisc/kernel/smp.c +0 -0

 		if (cpu_id < NR_CPUS)
 			set_cpu_possible(cpu_id, true);
 	}

Signature · 1 line

2.30.2

Quoted 19 lines · Rob Herring

Replace open coded parsing of CPU nodes' 'reg' property with of_get_cpu_hwid().

Cc Jonas Bonn <[email protected]>

Cc Stefan Kristiansson <[email protected]>

Cc Stafford Horne <[email protected]>

Cc openrisc at lists.librecores.org

Signed-off-by Rob Herring <[email protected]>

---
 arch/openrisc/kernel/smp.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

arch/openrisc/kernel/smp.c +0 -0

index 415e209732a3..7d5a4f303a5a 100644
@@ -65,11 +65,7 @@ void __init smp_init_cpus(void)
      u32 cpu_id;
 

arch/openrisc/kernel/smp.c void __init smp_init_cpus(void) · line 68

Quoted 7 lines

arch/openrisc/kernel/smp.c +1 -5

      for_each_of_cpu_node(cpu) {
-             if (of_property_read_u32(cpu, "reg", &cpu_id)) {
-                     pr_warn("%s missing reg property", cpu->full_name);
-                     continue;
-             }
-
+             cpu_id = of_get_cpu_hwid(cpu);

You have defined of_get_cpu_hwid to return u64, will this create compiler warnings when since we are storing a u64 into a u32?

It seems only if we make with W=3.

I thought we usually warned on this. Oh well, for the openrisc bits.

Acked-by Stafford Horne <[email protected]>

Quoted 6 lines

arch/openrisc/kernel/smp.c +0 -1

              if (cpu_id < NR_CPUS)
                      set_cpu_possible(cpu_id, true);
      }
--

2.30.2

Quoted 21 lines · Stafford Horne

Quoted 19 lines · Rob Herring

Replace open coded parsing of CPU nodes' 'reg' property with of_get_cpu_hwid().

Cc Jonas Bonn <[email protected]>

Cc Stefan Kristiansson <[email protected]>

Cc Stafford Horne <[email protected]>

Cc openrisc at lists.librecores.org

Signed-off-by Rob Herring <[email protected]>

---
 arch/openrisc/kernel/smp.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

arch/openrisc/kernel/smp.c +0 -0

index 415e209732a3..7d5a4f303a5a 100644
@@ -65,11 +65,7 @@ void __init smp_init_cpus(void)
      u32 cpu_id;
 

Quoted 7 lines

arch/openrisc/kernel/smp.c void __init smp_init_cpus(void) · line 68

Quoted 7 lines

arch/openrisc/kernel/smp.c +1 -5

      for_each_of_cpu_node(cpu) {
-             if (of_property_read_u32(cpu, "reg", &cpu_id)) {
-                     pr_warn("%s missing reg property", cpu->full_name);
-                     continue;
-             }
-
+             cpu_id = of_get_cpu_hwid(cpu);

Oops, that should be: of_get_cpu_hwid(cpu, 0);

I thought I double checked all those...

Quoted 2 lines

You have defined of_get_cpu_hwid to return u64, will this create compiler warnings when since we are storing a u64 into a u32?

I'm counting on the caller to know the max size for their platform.

Quoted 4 lines

It seems only if we make with W=3.

I thought we usually warned on this. Oh well, for the openrisc bits.

That's only on ptr truncation I think.

You have defined of_get_cpu_hwid to return u64, will this create compiler warnings when since we are storing a u64 into a u32?

It seems only if we make with W=3.

I thought we usually warned on this.

Quoted 4 lines · Stafford Horne

You have defined of_get_cpu_hwid to return u64, will this create compiler warnings when since we are storing a u64 into a u32?

It seems only if we make with W=3.

Yes. This is done by -Wconversion, "Warn for implicit conversions that may alter a value."

Quoted 1 line

I thought we usually warned on this.

This warning is not in -Wall or -Wextra either, it suffers too much from false positives. It is very natural to just ignore the high bits of modulo types (which is what "unsigned" types *are*). Or the bits that "fall off" on a conversion. The C standard makes this required behaviour, it is useful, and it is the only convenient way of getting this!

Sent: 06 October 2021 22:27

Quoted 4 lines · Stafford Horne

You have defined of_get_cpu_hwid to return u64, will this create compiler warnings when since we are storing a u64 into a u32?

It seems only if we make with W=3.

Yes. This is done by -Wconversion, "Warn for implicit conversions that may alter a value."

Quoted 1 line

I thought we usually warned on this.

This warning is not in -Wall or -Wextra either, it suffers too much from false positives. It is very natural to just ignore the high bits of modulo types (which is what "unsigned" types *are*). Or the bits that "fall off" on a conversion. The C standard makes this required behaviour, it is useful, and it is the only convenient way of getting this!

Replace open coded parsing of CPU nodes' 'reg' property with of_get_cpu_hwid().

Cc Michael Ellerman <[email protected]>

Cc Benjamin Herrenschmidt <[email protected]>

Cc Paul Mackerras <[email protected]>

Cc linuxppc-dev at lists.ozlabs.org

Signed-off-by Rob Herring <[email protected]>

---
 arch/powerpc/kernel/smp.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

arch/powerpc/kernel/smp.c +0 -1

index 9cc7d3dbf439..d96b0e361a73 100644
@@ -1313,18 +1313,13 @@ int __cpu_up(unsigned int cpu, struct task_struct *tidle)
 int cpu_to_core_id(int cpu)
 {
 	struct device_node *np;
-	const __be32 *reg;
 	int id = -1;
 
 	np = of_get_cpu_node(cpu, NULL);
 	if (!np)
 		goto out;

Replace open coded parsing of CPU nodes' 'reg' property with of_get_cpu_hwid().

Cc Yoshinori Sato <[email protected]>

Cc Rich Felker <[email protected]>

Cc linux-sh at vger.kernel.org

Signed-off-by Rob Herring <[email protected]>

---
 arch/sh/boards/of-generic.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

arch/sh/boards/of-generic.c +2 -3

index 921d76fc3358..f7f3e618e85b 100644
@@ -62,9 +62,8 @@ static void sh_of_smp_probe(void)
 	init_cpu_possible(cpumask_of(0));
 
 	for_each_of_cpu_node(np) {
-		const __be32 *cell = of_get_property(np, "reg", NULL);
-		u64 id = -1;
-		if (cell) id = of_read_number(cell, of_n_addr_cells(np));
+		u64 id = of_get_cpu_hwid(np, 0);
+
 		if (id < NR_CPUS) {
 			if (!method)
 				of_property_read_string(np, "enable-method", &method);

Signature · 1 line

2.30.2

In preparation to set the cache 'id' based on the CPU h/w ids, allow for 64-bit bit 'id' value. The only case that needs this is arm64, so unsigned long is sufficient.

Use the minimum CPU h/w id of the CPUs associated with the cache for the cache 'id'. This will provide a stable id value for a given system. As we need to check all possible CPUs, we can't use the shared_cpu_map which is just online CPUs. As there's not a cache to CPUs mapping in DT, we have to walk all CPU nodes and then walk cache levels.

The first 10 patches add a new function, of_get_cpu_hwid(), which parses CPU DT node 'reg' property, and then use it to replace all the open coded versions of parsing CPU node 'reg' properties.

The last 2 patches add support for populating the cacheinfo 'id' on DT platforms. The minimum associated CPU hwid is used for the id. The id is optional, but necessary for resctrl which is being adapted for Arm MPAM.

Tested on arm64. Compile tested on arm, x86 and powerpc.

The first 10 patches add a new function, of_get_cpu_hwid(), which parses CPU DT node 'reg' property, and then use it to replace all the open coded versions of parsing CPU node 'reg' properties.

The last 2 patches add support for populating the cacheinfo 'id' on DT platforms. The minimum associated CPU hwid is used for the id. The id is optional, but necessary for resctrl which is being adapted for Arm MPAM.

Tested on arm64. Compile tested on arm, x86 and powerpc.

Rob

Rob Herring (12):
of: Add of_get_cpu_hwid() to read hardware ID from CPU nodes
ARM: Use of_get_cpu_hwid()
ARM: broadcom: Use of_get_cpu_hwid()
arm64: Use of_get_cpu_hwid()
csky: Use of_get_cpu_hwid()
openrisc: Use of_get_cpu_hwid()
powerpc: Use of_get_cpu_hwid()
riscv: Use of_get_cpu_hwid()
sh: Use of_get_cpu_hwid()
x86: dt: Use of_get_cpu_hwid()
cacheinfo: Allow for >32-bit cache 'id'
cacheinfo: Set cache 'id' based on DT data