Srikar Dronamraju <[email protected]> writes:
> 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 <[email protected]>
> ---
> Changelog from v1:
> https://lkml.kernel.org/r/[email protected]
> - 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;
> }
>
> +/*
> + * 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.
this function is mainly only needed in __init, can we mark it so.
> +{
> + 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?
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.
setup_arch
mem_topology_setup
parse_numa_properties
detect_and_enable_coregroup() {...
// coregroup_enabled = 0/1
index = of_read_number(associativity, 1);
if (index > primary_domain_index + 1) {
coregroup_enabled = 1;
return index;
}
}
<...>
find_possible_nodes() {...
prop_length /= sizeof(int);
if (prop_length > primary_domain_index + 2)
coregroup_enabled = 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);
> } else {
> + struct device_node *cpu;
>
> /*
> * Don't fall back to default_nid yet -- we will plug
> @@ -948,6 +969,7 @@ static int __init parse_numa_properties(void)
> associativity = of_get_associativity(cpu);
> if (associativity) {
> nid = associativity_to_nid(associativity);
> + index =
> detect_and_enable_coregroup(associativity, index);
> initialize_form1_numa_distance(associativity);
> }
> of_node_put(cpu);
> @@ -1445,7 +1467,9 @@ static long vphn_get_associativity(unsigned long cpu,
>
> int cpu_to_coregroup_id(int cpu)
> {
> - __be32 associativity[VPHN_ASSOC_BUFSIZE] = {0};
> + int coregroup_id = cpu_to_core_id(cpu);
> + struct device_node *cpunode = NULL;
> + const __be32 *associativity;
> int index;
>
> if (cpu < 0 || cpu > nr_cpu_ids)
> @@ -1454,19 +1478,31 @@ int cpu_to_coregroup_id(int cpu)
> if (!coregroup_enabled)
> goto out;
>
> - if (!firmware_has_feature(FW_FEATURE_VPHN))
> - goto out;
> + if (firmware_has_feature(FW_FEATURE_VPHN)) {
> + __be32 tmp[VPHN_ASSOC_BUFSIZE] = {0};
>
> - if (vphn_get_associativity(cpu, associativity))
> + if (vphn_get_associativity(cpu, tmp))
> + goto out;
> +
> + associativity = tmp;
> +
> + } else {
> + cpunode = of_get_cpu_node(cpu, NULL);
> + if (!cpunode)
> + goto out;
> +
> + associativity = of_get_associativity(cpunode);
> + }
> + if (!associativity)
> goto out;
>
> index = of_read_number(associativity, 1);
> if (index > primary_domain_index + 1)
> - return of_read_number(&associativity[index - 1], 1);
> + coregroup_id = of_read_number(&associativity[index - 1], 1);
>
> out:
> - return cpu_to_core_id(cpu);
> -}
Looks like leftover removed from previous patch. This change should be
fixed in patch-2 itself.
> + if (cpunode)
> + of_node_put(cpunode);
>
> return coregroup_id;
> }
> --
> 2.43.0
-ritesh
