[PATCH] powerpc: Use nid as fallback for chip_id

[PATCH] powerpc: Use nid as fallback for chip_id

[Srikar Dronamraju]

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;
 }
 EXPORT_SYMBOL(cpu_to_chip_id);
 
-- 
2.18.1

Re: [PATCH] powerpc: Use nid as fallback for chip_id

[Michael Ellerman]

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

Re: [PATCH] powerpc: Use nid as fallback for chip_id

[Srikar Dronamraju]

* 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 




-- 
Thanks and Regards
Srikar Dronamraju