[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 <redacted>
---
 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

Srikar Dronamraju [off-list ref] writes:

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 [off-list ref] [2019-07-29 22:41:55]:

quoted

 
+	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