| Message ID | 20200923193458.203186-5-danielhb413@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | pseries NUMA distance calculation | expand |
On Wed, 23 Sep 2020 16:34:56 -0300 Daniel Henrique Barboza <danielhb413@gmail.com> wrote: > This is the first guest visible change introduced in > spapr_numa.c. The previous settings of both reference-points > and maxdomains were too restrictive, but enough for the > existing associativity we're setting in the resources. > > We'll change that in the following patches, populating the > associativity arrays based on user input. For those changes > to be effective, reference-points and maxdomains must be > more flexible. After this patch, we'll have 4 distinct > levels of NUMA (0x4, 0x3, 0x2, 0x1) and maxdomains will > allow for any type of configuration the user intends to > do - under the scope and limitations of PAPR itself, of > course. > > Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> > --- > hw/ppc/spapr_numa.c | 27 ++++++++++++++++++--------- > 1 file changed, 18 insertions(+), 9 deletions(-) > > diff --git a/hw/ppc/spapr_numa.c b/hw/ppc/spapr_numa.c > index 180800b2f3..688391278e 100644 > --- a/hw/ppc/spapr_numa.c > +++ b/hw/ppc/spapr_numa.c > @@ -222,21 +222,30 @@ int spapr_numa_write_assoc_lookup_arrays(SpaprMachineState *spapr, void *fdt, > */ > void spapr_numa_write_rtas_dt(SpaprMachineState *spapr, void *fdt, int rtas) > { > + MachineState *ms = MACHINE(spapr); > SpaprMachineClass *smc = SPAPR_MACHINE_GET_CLASS(spapr); > uint32_t refpoints[] = { > cpu_to_be32(0x4), > - cpu_to_be32(0x4), > + cpu_to_be32(0x3), > cpu_to_be32(0x2), > + cpu_to_be32(0x1), > }; > uint32_t nr_refpoints = ARRAY_SIZE(refpoints); > - uint32_t maxdomain = cpu_to_be32(spapr->gpu_numa_id > 1 ? 1 : 0); > - uint32_t maxdomains[] = { > - cpu_to_be32(4), > - maxdomain, > - maxdomain, > - maxdomain, > - cpu_to_be32(spapr->gpu_numa_id), > - }; > + uint32_t maxdomain = cpu_to_be32(ms->numa_state->num_nodes + > + spapr->gpu_numa_id); > + uint32_t maxdomains[] = {0x4, maxdomain, maxdomain, maxdomain, maxdomain}; > + It seems maxdomains[0] should be cpu_to_be32(0x4) and spaces are missing. Maybe keep the previous multi-line declaration style ? This seems to produce a nicer diff for the reviewer: uint32_t maxdomains[] = { cpu_to_be32(4), maxdomain, maxdomain, maxdomain, - cpu_to_be32(spapr->gpu_numa_id), + maxdomain, }; > + if (spapr_machine_using_legacy_numa(spapr)) { > + refpoints[1] = cpu_to_be32(0x4); > + refpoints[2] = cpu_to_be32(0x2); I'd rather have an explicit view of the legacy layouts for clarity... > + nr_refpoints = 3; > + > + maxdomain = cpu_to_be32(spapr->gpu_numa_id > 1 ? 1 : 0); > + maxdomains[1] = maxdomain; > + maxdomains[2] = maxdomain; > + maxdomains[3] = maxdomain; > + maxdomains[4] = cpu_to_be32(spapr->gpu_numa_id); ... and here. eg. if (spapr_machine_using_legacy_numa(spapr)) { uint32_t legacy_refpoints[] = { cpu_to_be32(0x4), cpu_to_be32(0x4), cpu_to_be32(0x2), }; uint32_t legacy_maxdomain = cpu_to_be32(spapr->gpu_numa_id > 1 ? 1 : 0); uint32_t legacy_maxdomains[] = { cpu_to_be32(4), legacy_maxdomain, legacy_maxdomain, legacy_maxdomain, cpu_to_be32(spapr->gpu_numa_id), }; nr_refpoints = 3; memcpy(refpoints, legacy_refpoints, sizeof(legacy_refpoints)); memcpy(maxdomains, legacy_maxdomains, sizeof(legacy_maxdomains)); } This allows to instantly see how things are expected to appear in the FDT, without having to mentally patch the refpoints[] and maxdomains[] arrays. This also makes the diff easier to review. > + } > > if (smc->pre_5_1_assoc_refpoints) { > nr_refpoints = 2;
diff --git a/hw/ppc/spapr_numa.c b/hw/ppc/spapr_numa.c index 180800b2f3..688391278e 100644 --- a/hw/ppc/spapr_numa.c +++ b/hw/ppc/spapr_numa.c @@ -222,21 +222,30 @@ int spapr_numa_write_assoc_lookup_arrays(SpaprMachineState *spapr, void *fdt, */ void spapr_numa_write_rtas_dt(SpaprMachineState *spapr, void *fdt, int rtas) { + MachineState *ms = MACHINE(spapr); SpaprMachineClass *smc = SPAPR_MACHINE_GET_CLASS(spapr); uint32_t refpoints[] = { cpu_to_be32(0x4), - cpu_to_be32(0x4), + cpu_to_be32(0x3), cpu_to_be32(0x2), + cpu_to_be32(0x1), }; uint32_t nr_refpoints = ARRAY_SIZE(refpoints); - uint32_t maxdomain = cpu_to_be32(spapr->gpu_numa_id > 1 ? 1 : 0); - uint32_t maxdomains[] = { - cpu_to_be32(4), - maxdomain, - maxdomain, - maxdomain, - cpu_to_be32(spapr->gpu_numa_id), - }; + uint32_t maxdomain = cpu_to_be32(ms->numa_state->num_nodes + + spapr->gpu_numa_id); + uint32_t maxdomains[] = {0x4, maxdomain, maxdomain, maxdomain, maxdomain}; + + if (spapr_machine_using_legacy_numa(spapr)) { + refpoints[1] = cpu_to_be32(0x4); + refpoints[2] = cpu_to_be32(0x2); + nr_refpoints = 3; + + maxdomain = cpu_to_be32(spapr->gpu_numa_id > 1 ? 1 : 0); + maxdomains[1] = maxdomain; + maxdomains[2] = maxdomain; + maxdomains[3] = maxdomain; + maxdomains[4] = cpu_to_be32(spapr->gpu_numa_id); + } if (smc->pre_5_1_assoc_refpoints) { nr_refpoints = 2;
This is the first guest visible change introduced in spapr_numa.c. The previous settings of both reference-points and maxdomains were too restrictive, but enough for the existing associativity we're setting in the resources. We'll change that in the following patches, populating the associativity arrays based on user input. For those changes to be effective, reference-points and maxdomains must be more flexible. After this patch, we'll have 4 distinct levels of NUMA (0x4, 0x3, 0x2, 0x1) and maxdomains will allow for any type of configuration the user intends to do - under the scope and limitations of PAPR itself, of course. Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- hw/ppc/spapr_numa.c | 27 ++++++++++++++++++--------- 1 file changed, 18 insertions(+), 9 deletions(-)