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. > > Cc: Nathan Lynch > Cc: Srikar Dronamraju > Cc: Vasant Hegde > Reviewed-by: Greg Kurz > Reviewed-by: Daniel Henrique Barboza > Tested-by: Daniel Henrique Barboza > Reviewed-by: Srikar Dronamraju > Signed-off-by: Cédric Le Goater > --- > 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 > > -#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