Re: [PATCH] powerpc: Use nid as fallback for chip_id
* Michael Ellerman [2019-07-29 22:41:55]: > > > > + chip_id = of_get_ibm_chip_id(np); > > + if (chip_id == -1) > > + chip_id = of_node_to_nid(np); > > + > > of_node_put(np); > > - return of_get_ibm_chip_id(np); > > + return chip_id; > > } > > A nid is not a chip-id. > Agree that nid is not a chip-id. > This obviously happens to work for the case you've identified above but > it's not something I'm happy to merge in general. > Okay. > We could do a similar change in the topology code, but I'd probably like > it to be restricted to when we're running under PowerVM and there are no > chip-ids found at all. > So for PowerNV case and KVM guest, of_get_ibm_chip_id() always seems to returns a valid chip-id. Its *only* in the PowerVM case that we are returning nid as the fallback chip-id. Do you think checking for OPAL firmware would help? chip_id = of_get_ibm_chip_id(np); if (chip_id == -1 && !firmware_has_feature(FW_FEATURE_OPAL)) chip_id = of_node_to_nid(np); of_node_put(np); or should we do int topology_physical_package_id(int cpu) { int chip_id = cpu_to_chip_id(cpu) if (chip_id == -1 && !firmware_has_feature(FW_FEATURE_OPAL)) //Fallback to nid instead of chip-id. return chip_id; } > I'm also not clear how it will interact with migration. > On migration, this function would be triggered when the cpumasks are getting updated. So I would expect this to continue working. Or Am I missing someother migration related quirk? > cheers > The other alternative that I see is -- Thanks and Regards Srikar Dronamraju
Re: [PATCH] powerpc: Use nid as fallback for chip_id
Srikar Dronamraju writes: > One of the uses of chip_id is to find out all cores that are part of the same > chip. However ibm,chip_id property is not present in device-tree of PowerVM > Lpars. Hence lscpu output shows one core per socket and multiple cores. > > Before the patch. > # lscpu > Architecture:ppc64le > Byte Order: Little Endian > CPU(s): 128 > On-line CPU(s) list: 0-127 > Thread(s) per core: 8 > Core(s) per socket: 1 > Socket(s): 16 > NUMA node(s):2 > Model: 2.2 (pvr 004e 0202) > Model name: POWER9 (architected), altivec supported > Hypervisor vendor: pHyp > Virtualization type: para > L1d cache: 32K > L1i cache: 32K > L2 cache:512K > L3 cache:10240K > NUMA node0 CPU(s): 0-63 > NUMA node1 CPU(s): 64-127 > > # cat /sys/devices/system/cpu/cpu0/topology/physical_package_id > -1 > > Signed-off-by: Srikar Dronamraju > --- > arch/powerpc/kernel/prom.c | 10 -- > 1 file changed, 8 insertions(+), 2 deletions(-) > > diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c > index 7159e791a70d..0b8918b43580 100644 > --- a/arch/powerpc/kernel/prom.c > +++ b/arch/powerpc/kernel/prom.c > @@ -867,18 +867,24 @@ EXPORT_SYMBOL(of_get_ibm_chip_id); > * @cpu: The logical cpu number. > * > * Return the value of the ibm,chip-id property corresponding to the given > - * logical cpu number. If the chip-id can not be found, returns -1. > + * logical cpu number. If the chip-id can not be found, return nid. > + * > */ > int cpu_to_chip_id(int cpu) > { > struct device_node *np; > + int chip_id = -1; > > np = of_get_cpu_node(cpu, NULL); > if (!np) > return -1; > > + chip_id = of_get_ibm_chip_id(np); > + if (chip_id == -1) > + chip_id = of_node_to_nid(np); > + > of_node_put(np); > - return of_get_ibm_chip_id(np); > + return chip_id; > } A nid is not a chip-id. This obviously happens to work for the case you've identified above but it's not something I'm happy to merge in general. We could do a similar change in the topology code, but I'd probably like it to be restricted to when we're running under PowerVM and there are no chip-ids found at all. I'm also not clear how it will interact with migration. cheers
[PATCH] powerpc: Use nid as fallback for chip_id
One of the uses of chip_id is to find out all cores that are part of the same chip. However ibm,chip_id property is not present in device-tree of PowerVM Lpars. Hence lscpu output shows one core per socket and multiple cores. Before the patch. # lscpu Architecture:ppc64le Byte Order: Little Endian CPU(s): 128 On-line CPU(s) list: 0-127 Thread(s) per core: 8 Core(s) per socket: 1 Socket(s): 16 NUMA node(s):2 Model: 2.2 (pvr 004e 0202) Model name: POWER9 (architected), altivec supported Hypervisor vendor: pHyp Virtualization type: para L1d cache: 32K L1i cache: 32K L2 cache:512K L3 cache:10240K NUMA node0 CPU(s): 0-63 NUMA node1 CPU(s): 64-127 # cat /sys/devices/system/cpu/cpu0/topology/physical_package_id -1 Signed-off-by: Srikar Dronamraju --- arch/powerpc/kernel/prom.c | 10 -- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c index 7159e791a70d..0b8918b43580 100644 --- a/arch/powerpc/kernel/prom.c +++ b/arch/powerpc/kernel/prom.c @@ -867,18 +867,24 @@ EXPORT_SYMBOL(of_get_ibm_chip_id); * @cpu: The logical cpu number. * * Return the value of the ibm,chip-id property corresponding to the given - * logical cpu number. If the chip-id can not be found, returns -1. + * logical cpu number. If the chip-id can not be found, return nid. + * */ int cpu_to_chip_id(int cpu) { struct device_node *np; + int chip_id = -1; np = of_get_cpu_node(cpu, NULL); if (!np) return -1; + chip_id = of_get_ibm_chip_id(np); + if (chip_id == -1) + chip_id = of_node_to_nid(np); + of_node_put(np); - return of_get_ibm_chip_id(np); + return chip_id; } EXPORT_SYMBOL(cpu_to_chip_id); -- 2.18.1