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

Srikar Dronamraju [off-list ref] writes:

* Ritesh Harjani [off-list ref] [2026-07-10 11:18:12]:

quoted

Srikar Dronamraju [off-list ref] writes:

quoted

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.

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.

Signed-off-by: Srikar Dronamraju <redacted>
---
Changelog from v1: https://lkml.kernel.org/r/20260524010017.140408-1-srikar@linux.ibm.com
- Handle comments from Christophe Leroy; make code more flat

 arch/powerpc/mm/numa.c | 56 ++++++++++++++++++++++++++++++++++--------
 1 file changed, 46 insertions(+), 10 deletions(-)

diff --git a/arch/powerpc/mm/numa.c b/arch/powerpc/mm/numa.c
index 9aa71eb7e96b..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;
 }

Hi Ritesh,

Thanks for the review.

quoted

quoted

 
+/*
+ * 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)

Do we care about it's return value? We are not reading that in the
patch.

Yes, We do care about the return value. If the index is set to -1, we don't
retry enabling the coregroup.

yes, my bad. Agreed it is being used.

quoted

this function is mainly only needed in __init, can we mark it so.

Yes, this will be done.

quoted

quoted

+{
+	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;
+}

For PowerVM, we now have two places which will enable coregroup_enabled
during mem_topology_setup(). Is there some way we can unify that?

For PowerVM, we have two extra associativity properties
ibm,ibm,current-associativity-domains and ibm,max-associativity-domains.
On PowerNV, these two properties are not used/exported.

All we depend is the layout of these properties to determine if coregroup is
enabled. If the layout tells us that there is place after
primary_domain_index for coregroup, we assume coregroup is enabled.

So in this patch, we hook at the place we look at each of the CPU
associativity. This should work for both PowerVM and PowerNV.

So, I can think of two options.
1. Remove the previous logic of depending on PowerVM specific code.
2. Allow the previous logic to be around. Since its not going to hurt
functionally or performance wise.

quoted

This also means we enable coregroup in case of PowerVM with SPLPAR when
per-cpu VPHN associativity index > primary_domain_index+1. But this
isn't reflected in your commit msg. The commit msg only says this
affects PowerNV.

I don't think, I said this affects PowerNV only. But I still don't think
the logic would change. The logic to enable coregroup remains the same.
Just that we may now be depending on the 1st CPU associativity instead of
the PowerVM specific properties.

So let's just add this info in the commit msg please. Because it was not
clear otherwise.

-ritesh