Re: [PATCH v3 3/3] powerpc/numa: Support coregroup on PowerNV

[Srikar Dronamraju]

* Shrikanth Hegde <sshegde@xxxxxxxxxxxxx> [2026-09-04 15:34:50]:

> On 9/2/26 6:05 PM, Srikar Dronamraju wrote:
> > Coregroup support on powerpc has so far been limited to PowerVM LPARs.
> > However, PowerNV can also support coregroups when firmware exposes the
> > required coregroup information through the associativity hierarchy.
> > 
> 
> Existing firmware does expose this info already?
> 
> > Detect coregroup support by checking whether primary_domain_index is the
> > penultimate domain in the CPU node's ibm,associativity property. On
> > PowerNV, a non-penultimate primary_domain_index indicates that firmware
> > provides an additional level for coregroup information.
> > 
> > This keeps the logic compatible with PowerVM systems, where
> > primary_domain_index is likewise not the penultimate associativity
> > domain.
> > 
> 
> Could you please put the ibm,associativity on this powernv? as well
> PowerVM's so that one understands it better?
> 
> > Signed-off-by: Srikar Dronamraju <srikar@xxxxxxxxxxxxx>
> > ---
> > Changelog from v2:
> > - Handle comments from Ritesh (one hunk needed to be moved from patch 2 to
> >    patch 3 to build correctly)
> > 
> > Changelog from v1: https://lkml.kernel.org/r/20260524010017.140408-1-srikar@xxxxxxxxxxxxx
> > - Handle comments from Christophe Leroy; make code more flat
> > 
> >   arch/powerpc/mm/numa.c | 58 ++++++++++++++++++++++++++++++++++--------
> >   1 file changed, 48 insertions(+), 10 deletions(-)
> > 
> > diff --git a/arch/powerpc/mm/numa.c b/arch/powerpc/mm/numa.c
> > index 5f326b005a2a..e97b624203ea 100644
> > --- a/arch/powerpc/mm/numa.c
> > +++ b/arch/powerpc/mm/numa.c
> > @@ -889,12 +889,32 @@ static int __init numa_setup_drmem_lmb(struct drmem_lmb *lmb,
> >   	return 0;
> >   }
> > +/*
> > + * If hierarchy extends beyond primary_domain_index + 1, then next
> > + * level corresponds to coregroup.
> > + */
> > +static int detect_and_enable_coregroup(const __be32 *associativity, int index)
> > +{
> > +	if (!associativity || index == -1)
> > +		goto out;
> > +
> > +	index = of_read_number(associativity, 1);
> > +
> > +	if (index > primary_domain_index + 1) {
> > +		coregroup_enabled = 1;
> > +		return index;
> > +	}
> > +out:
> > +	coregroup_enabled = 0;
> > +	return -1;
> > +}
> > +
> >   static int __init parse_numa_properties(void)
> >   {
> >   	struct device_node *memory, *pci;
> > -	int default_nid = 0;
> > -	unsigned long i;
> > +	int default_nid = 0, index = 0;
> >   	const __be32 *associativity;
> > +	unsigned long i;
> >   	if (numa_enabled == 0) {
> >   		pr_warn("disabled by user\n");
> > @@ -927,7 +947,6 @@ static int __init parse_numa_properties(void)
> >   	 */
> >   	for_each_present_cpu(i) {
> >   		__be32 vphn_assoc[VPHN_ASSOC_BUFSIZE];
> > -		struct device_node *cpu;
> >   		int nid = NUMA_NO_NODE;
> >   		memset(vphn_assoc, 0, VPHN_ASSOC_BUFSIZE * sizeof(__be32));
> > @@ -935,7 +954,9 @@ static int __init parse_numa_properties(void)
> >   		if (__vphn_get_associativity(i, vphn_assoc) == 0) {
> >   			nid = associativity_to_nid(vphn_assoc);
> >   			initialize_form1_numa_distance(vphn_assoc);
> > +			index = detect_and_enable_coregroup(vphn_assoc, index);
> 
> nit: I don't see return value being used.

index is getting used within the loop. Do note, we are passing index as a
parameter to detect_and_enable_coregroup().
For example, index is set to -1, in the first iteration, then
in the next iterations,  detect_and_enable_coregroup will not retry.
So the return value gets used.

> 
> Other than that, rest looks good to me.
> Reviewed-by: Shrikanth Hegde <sshegde@xxxxxxxxxxxxx>
> 
> >   		} else {

-- 
Thanks and Regards
Srikar Dronamraju