diff mbox series

[v2,4/6] spapr_numa: change reference-points and maxdomain settings

Message ID 20200924195058.362984-5-danielhb413@gmail.com
State New
Headers show
Series pseries NUMA distance calculation | expand

Commit Message

Daniel Henrique Barboza Sept. 24, 2020, 7:50 p.m. UTC
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 | 40 ++++++++++++++++++++++++++++++++--------
 1 file changed, 32 insertions(+), 8 deletions(-)

Comments

David Gibson Sept. 25, 2020, 2:38 a.m. UTC | #1
On Thu, Sep 24, 2020 at 04:50:56PM -0300, Daniel Henrique Barboza 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>

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

Although..

> ---
>  hw/ppc/spapr_numa.c | 40 ++++++++++++++++++++++++++++++++--------
>  1 file changed, 32 insertions(+), 8 deletions(-)
> 
> diff --git a/hw/ppc/spapr_numa.c b/hw/ppc/spapr_numa.c
> index 990a5fce08..ea33439a15 100644
> --- a/hw/ppc/spapr_numa.c
> +++ b/hw/ppc/spapr_numa.c
> @@ -222,24 +222,48 @@ 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 maxdomain = ms->numa_state->num_nodes + spapr->gpu_numa_id;
>      uint32_t maxdomains[] = {
>          cpu_to_be32(4),
> -        maxdomain,
> -        maxdomain,
> -        maxdomain,
> -        cpu_to_be32(spapr->gpu_numa_id),
> +        cpu_to_be32(maxdomain),
> +        cpu_to_be32(maxdomain),
> +        cpu_to_be32(maxdomain),
> +        cpu_to_be32(maxdomain)
>      };
>  
> -    if (smc->pre_5_1_assoc_refpoints) {
> -        nr_refpoints = 2;
> +    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 = spapr->gpu_numa_id > 1 ? 1 : 0;
> +        uint32_t legacy_maxdomains[] = {
> +            cpu_to_be32(4),
> +            cpu_to_be32(legacy_maxdomain),
> +            cpu_to_be32(legacy_maxdomain),
> +            cpu_to_be32(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));

It would be nice to have a G_STATIC_ASSERT() or QEMU_BUILD_BUG_MSG()
ro ensure that the two structures are the same size, if they became
different the memcpy is wildly unsafe.

> +        /* pseries-5.0 and older reference-points array is {0x4, 0x4} */
> +        if (smc->pre_5_1_assoc_refpoints) {
> +            nr_refpoints = 2;
> +        }
>      }
>  
>      _FDT(fdt_setprop(fdt, rtas, "ibm,associativity-reference-points",
Greg Kurz Sept. 25, 2020, 1:16 p.m. UTC | #2
On Thu, 24 Sep 2020 16:50: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>
> ---

Reviewed-by: Greg Kurz <groug@kaod.org>

>  hw/ppc/spapr_numa.c | 40 ++++++++++++++++++++++++++++++++--------
>  1 file changed, 32 insertions(+), 8 deletions(-)
> 
> diff --git a/hw/ppc/spapr_numa.c b/hw/ppc/spapr_numa.c
> index 990a5fce08..ea33439a15 100644
> --- a/hw/ppc/spapr_numa.c
> +++ b/hw/ppc/spapr_numa.c
> @@ -222,24 +222,48 @@ 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 maxdomain = ms->numa_state->num_nodes + spapr->gpu_numa_id;
>      uint32_t maxdomains[] = {
>          cpu_to_be32(4),
> -        maxdomain,
> -        maxdomain,
> -        maxdomain,
> -        cpu_to_be32(spapr->gpu_numa_id),
> +        cpu_to_be32(maxdomain),
> +        cpu_to_be32(maxdomain),
> +        cpu_to_be32(maxdomain),
> +        cpu_to_be32(maxdomain)
>      };
>  
> -    if (smc->pre_5_1_assoc_refpoints) {
> -        nr_refpoints = 2;
> +    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 = spapr->gpu_numa_id > 1 ? 1 : 0;
> +        uint32_t legacy_maxdomains[] = {
> +            cpu_to_be32(4),
> +            cpu_to_be32(legacy_maxdomain),
> +            cpu_to_be32(legacy_maxdomain),
> +            cpu_to_be32(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));
> +
> +        /* pseries-5.0 and older reference-points array is {0x4, 0x4} */
> +        if (smc->pre_5_1_assoc_refpoints) {
> +            nr_refpoints = 2;
> +        }
>      }
>  
>      _FDT(fdt_setprop(fdt, rtas, "ibm,associativity-reference-points",
diff mbox series

Patch

diff --git a/hw/ppc/spapr_numa.c b/hw/ppc/spapr_numa.c
index 990a5fce08..ea33439a15 100644
--- a/hw/ppc/spapr_numa.c
+++ b/hw/ppc/spapr_numa.c
@@ -222,24 +222,48 @@  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 maxdomain = ms->numa_state->num_nodes + spapr->gpu_numa_id;
     uint32_t maxdomains[] = {
         cpu_to_be32(4),
-        maxdomain,
-        maxdomain,
-        maxdomain,
-        cpu_to_be32(spapr->gpu_numa_id),
+        cpu_to_be32(maxdomain),
+        cpu_to_be32(maxdomain),
+        cpu_to_be32(maxdomain),
+        cpu_to_be32(maxdomain)
     };
 
-    if (smc->pre_5_1_assoc_refpoints) {
-        nr_refpoints = 2;
+    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 = spapr->gpu_numa_id > 1 ? 1 : 0;
+        uint32_t legacy_maxdomains[] = {
+            cpu_to_be32(4),
+            cpu_to_be32(legacy_maxdomain),
+            cpu_to_be32(legacy_maxdomain),
+            cpu_to_be32(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));
+
+        /* pseries-5.0 and older reference-points array is {0x4, 0x4} */
+        if (smc->pre_5_1_assoc_refpoints) {
+            nr_refpoints = 2;
+        }
     }
 
     _FDT(fdt_setprop(fdt, rtas, "ibm,associativity-reference-points",