| Message ID | 1562062765-31104-1-git-send-email-srikar@linux.vnet.ibm.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | powerpc: Use nid as fallback for chip_id | expand |
| Context | Check | Description |
|---|---|---|
| snowpatch_ozlabs/apply_patch | success | Successfully applied on branch next (c7d64b560ce80d8c44f082eee8352f0778a73195) |
| snowpatch_ozlabs/build-ppc64le | success | Build succeeded |
| snowpatch_ozlabs/build-ppc64be | success | Build succeeded |
| snowpatch_ozlabs/build-ppc64e | success | Build succeeded |
| snowpatch_ozlabs/build-pmac32 | success | Build succeeded |
| snowpatch_ozlabs/checkpatch | warning | total: 0 errors, 1 warnings, 0 checks, 26 lines checked |
Srikar Dronamraju <srikar@linux.vnet.ibm.com> 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 <srikar@linux.vnet.ibm.com> > --- > 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
* Michael Ellerman <mpe@ellerman.id.au> [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
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);
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 <srikar@linux.vnet.ibm.com> --- arch/powerpc/kernel/prom.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)