[PATCH] powerpc/numa: Fix topology_physical_package_id() on pSeries

Subsystems: linux for powerpc (32-bit and 64-bit), the rest

STALE1979d REVIEWED: 4 (0M)

4 review trailers.

2 messages, 2 authors, 2021-03-22 · open the first message on its own page

[PATCH] powerpc/numa: Fix topology_physical_package_id() on pSeries

From: Cédric Le Goater <clg@kaod.org>
Date: 2021-03-16 12:25:22

The topology-id of a CPU in a pSeries machine can be queried from
sysfs but under PowerVM the value is always -1 even if NUMA nodes are
defined. This is because the topology_physical_package_id() routine is
using the "ibm,chip-id" property which is not specified in PAPR.

Under QEMU/KVM, things are different because QEMU populates the CPU DT
node with "ibm,chip-id" property. However, its value can be incorrect
for uncommon SMT configuration and expose a bogus topology-id value in
sysfs.

The use of cpu_to_node() guarantees to have a correct NUMA node id
under both environments QEMU/KVM and PowerVM. This introduces a slight
change for the QEMU/KVM guest, as the topology-id now matches the NUMA
node and not the socket-id as before. Since QEMU also needs to remove
"ibm,chip-id" property for the DT to follow the PAPR specs, both
hypervisor environments will be in sync.

On the PowerNV side, the NUMA node id returned by cpu_to_node() is
computed from the "ibm,associativity" property of the CPU. Its value
is built from the OPAL chip id and is equivalent to "ibm,chip-id".

Cc: Nathan Lynch <redacted>
Cc: Srikar Dronamraju <redacted>
Cc: Vasant Hegde <redacted>
Reviewed-by: Greg Kurz <redacted>
Reviewed-by: Daniel Henrique Barboza <redacted>
Tested-by: Daniel Henrique Barboza <redacted>
Reviewed-by: Srikar Dronamraju <redacted>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 arch/powerpc/include/asm/topology.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/include/asm/topology.h b/arch/powerpc/include/asm/topology.h
index 3beeb030cd78..887c42a4e43d 100644
--- a/arch/powerpc/include/asm/topology.h
+++ b/arch/powerpc/include/asm/topology.h
@@ -123,7 +123,7 @@ static inline int cpu_to_coregroup_id(int cpu)
 #ifdef CONFIG_PPC64
 #include <asm/smp.h>
 
-#define topology_physical_package_id(cpu)	(cpu_to_chip_id(cpu))
+#define topology_physical_package_id(cpu)	(cpu_to_node(cpu))
 
 #define topology_sibling_cpumask(cpu)	(per_cpu(cpu_sibling_map, cpu))
 #define topology_core_cpumask(cpu)	(cpu_cpu_mask(cpu))
-- 
2.26.2

Re: [PATCH] powerpc/numa: Fix topology_physical_package_id() on pSeries

From: David Gibson <hidden>
Date: 2021-03-22 05:25:03

On Tue, Mar 16, 2021 at 01:24:37PM +0100, Cédric Le Goater wrote:
The topology-id of a CPU in a pSeries machine can be queried from
sysfs but under PowerVM the value is always -1 even if NUMA nodes are
defined. This is because the topology_physical_package_id() routine is
using the "ibm,chip-id" property which is not specified in PAPR.

Under QEMU/KVM, things are different because QEMU populates the CPU DT
node with "ibm,chip-id" property. However, its value can be incorrect
for uncommon SMT configuration and expose a bogus topology-id value in
sysfs.
Incorrect in what sense?  It's still indicating the (admittedly
arbitrary) qemu socket number, isn't it?  And isn't that what it
should be?
The use of cpu_to_node() guarantees to have a correct NUMA node id
under both environments QEMU/KVM and PowerVM. This introduces a slight
change for the QEMU/KVM guest, as the topology-id now matches the NUMA
node and not the socket-id as before. Since QEMU also needs to remove
"ibm,chip-id" property for the DT to follow the PAPR specs, both
hypervisor environments will be in sync.

On the PowerNV side, the NUMA node id returned by cpu_to_node() is
computed from the "ibm,associativity" property of the CPU. Its value
is built from the OPAL chip id and is equivalent to "ibm,chip-id".
Like mpe, I'm not convinced this is the right approach.  "physical
packate" and NUMA node are not the same thing, except sometimes by
accident.
quoted hunk
Cc: Nathan Lynch <redacted>
Cc: Srikar Dronamraju <redacted>
Cc: Vasant Hegde <redacted>
Reviewed-by: Greg Kurz <redacted>
Reviewed-by: Daniel Henrique Barboza <redacted>
Tested-by: Daniel Henrique Barboza <redacted>
Reviewed-by: Srikar Dronamraju <redacted>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 arch/powerpc/include/asm/topology.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/include/asm/topology.h b/arch/powerpc/include/asm/topology.h
index 3beeb030cd78..887c42a4e43d 100644
--- a/arch/powerpc/include/asm/topology.h
+++ b/arch/powerpc/include/asm/topology.h
@@ -123,7 +123,7 @@ static inline int cpu_to_coregroup_id(int cpu)
 #ifdef CONFIG_PPC64
 #include <asm/smp.h>
 
-#define topology_physical_package_id(cpu)	(cpu_to_chip_id(cpu))
+#define topology_physical_package_id(cpu)	(cpu_to_node(cpu))
 
 #define topology_sibling_cpumask(cpu)	(per_cpu(cpu_sibling_map, cpu))
 #define topology_core_cpumask(cpu)	(cpu_cpu_mask(cpu))
-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help