The current implementation of h_home_node_associativity hard codes
the values of associativity domains of the vcpus. Let's make
it consider the values already initialized in spapr->numa_assoc_array,
via the spapr_numa_get_vcpu_assoc() helper.

We want to set it and forget it, and for that we also need to
assert that we don't overflow the registers of the hypercall.
From R4 to R9 we can squeeze in 12 associativity domains, so
let's assert that MAX_DISTANCE_REF_POINTS isn't greater
than that.

Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
---
 hw/ppc/spapr_numa.c | 33 +++++++++++++++++++++++++--------
 1 file changed, 25 insertions(+), 8 deletions(-)

diff --git a/hw/ppc/spapr_numa.c b/hw/ppc/spapr_numa.c
index abc7361921..850e61bf98 100644
--- a/hw/ppc/spapr_numa.c
+++ b/hw/ppc/spapr_numa.c
@@ -185,10 +185,12 @@ target_ulong h_home_node_associativity(PowerPCCPU *cpu,
                                        target_ulong opcode,
                                        target_ulong *args)
 {
+    g_autofree uint32_t *vcpu_assoc = NULL;
     target_ulong flags = args[0];
     target_ulong procno = args[1];
     PowerPCCPU *tcpu;
-    int idx;
+    uint vcpu_assoc_size;
+    int idx, assoc_idx;
 
     /* only support procno from H_REGISTER_VPA */
     if (flags != 0x1) {
@@ -200,16 +202,31 @@ target_ulong h_home_node_associativity(PowerPCCPU *cpu,
         return H_P2;
     }
 
-    /* sequence is the same as in the "ibm,associativity" property */
+    /*
+     * Given that we want to be flexible with the sizes and indexes,
+     * we must consider that there is a hard limit of how many
+     * associativities domain we can fit in R4 up to o R9, which
+     * would be 12. Assert and bail if that's not the case.
+     */
+    g_assert(MAX_DISTANCE_REF_POINTS <= 12);
+
+    vcpu_assoc = spapr_numa_get_vcpu_assoc(spapr, tcpu, &vcpu_assoc_size);
+    vcpu_assoc_size /= sizeof(uint32_t);
+    /* assoc_idx starts at 1 to skip associativity size */
+    assoc_idx = 1;
 
-    idx = 0;
 #define ASSOCIATIVITY(a, b) (((uint64_t)(a) << 32) | \
                              ((uint64_t)(b) & 0xffffffff))
-    args[idx++] = ASSOCIATIVITY(0, 0);
-    args[idx++] = ASSOCIATIVITY(0, tcpu->node_id);
-    args[idx++] = ASSOCIATIVITY(procno, -1);
-    for ( ; idx < 6; idx++) {
-        args[idx] = -1;
+
+    for (idx = 0; idx < 6; idx++) {
+        int8_t a, b;
+
+        a = assoc_idx < vcpu_assoc_size ?
+            be32_to_cpu(vcpu_assoc[assoc_idx++]) : -1;
+        b = assoc_idx < vcpu_assoc_size ?
+            be32_to_cpu(vcpu_assoc[assoc_idx++]) : -1;
+
+        args[idx] = ASSOCIATIVITY(a, b);
     }
 #undef ASSOCIATIVITY
 
-- 
2.26.2

On Thu, Sep 03, 2020 at 07:06:39PM -0300, Daniel Henrique Barboza wrote:
> The current implementation of h_home_node_associativity hard codes
> the values of associativity domains of the vcpus. Let's make
> it consider the values already initialized in spapr->numa_assoc_array,
> via the spapr_numa_get_vcpu_assoc() helper.
> 
> We want to set it and forget it, and for that we also need to
> assert that we don't overflow the registers of the hypercall.
> >From R4 to R9 we can squeeze in 12 associativity domains, so
> let's assert that MAX_DISTANCE_REF_POINTS isn't greater
> than that.
> 
> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
> ---
>  hw/ppc/spapr_numa.c | 33 +++++++++++++++++++++++++--------
>  1 file changed, 25 insertions(+), 8 deletions(-)
> 
> diff --git a/hw/ppc/spapr_numa.c b/hw/ppc/spapr_numa.c
> index abc7361921..850e61bf98 100644
> --- a/hw/ppc/spapr_numa.c
> +++ b/hw/ppc/spapr_numa.c
> @@ -185,10 +185,12 @@ target_ulong h_home_node_associativity(PowerPCCPU *cpu,
>                                         target_ulong opcode,
>                                         target_ulong *args)
>  {
> +    g_autofree uint32_t *vcpu_assoc = NULL;
>      target_ulong flags = args[0];
>      target_ulong procno = args[1];
>      PowerPCCPU *tcpu;
> -    int idx;
> +    uint vcpu_assoc_size;
> +    int idx, assoc_idx;
>  
>      /* only support procno from H_REGISTER_VPA */
>      if (flags != 0x1) {
> @@ -200,16 +202,31 @@ target_ulong h_home_node_associativity(PowerPCCPU *cpu,
>          return H_P2;
>      }
>  
> -    /* sequence is the same as in the "ibm,associativity" property */
> +    /*
> +     * Given that we want to be flexible with the sizes and indexes,
> +     * we must consider that there is a hard limit of how many
> +     * associativities domain we can fit in R4 up to o R9, which
typo  ...............................................   ^

> +     * would be 12. Assert and bail if that's not the case.
> +     */
> +    g_assert(MAX_DISTANCE_REF_POINTS <= 12);

Since MAX_DISTANCE_REF_POINTS is a compile time constant, you could
use G_STATIC_ASSERT() to make this a compile rather than runtime
error.

> +
> +    vcpu_assoc = spapr_numa_get_vcpu_assoc(spapr, tcpu, &vcpu_assoc_size);
> +    vcpu_assoc_size /= sizeof(uint32_t);
> +    /* assoc_idx starts at 1 to skip associativity size */
> +    assoc_idx = 1;
>  
> -    idx = 0;
>  #define ASSOCIATIVITY(a, b) (((uint64_t)(a) << 32) | \
>                               ((uint64_t)(b) & 0xffffffff))
> -    args[idx++] = ASSOCIATIVITY(0, 0);
> -    args[idx++] = ASSOCIATIVITY(0, tcpu->node_id);
> -    args[idx++] = ASSOCIATIVITY(procno, -1);
> -    for ( ; idx < 6; idx++) {
> -        args[idx] = -1;
> +
> +    for (idx = 0; idx < 6; idx++) {
> +        int8_t a, b;

Do you really want int8_t, rather than say int32_t?

> +
> +        a = assoc_idx < vcpu_assoc_size ?
> +            be32_to_cpu(vcpu_assoc[assoc_idx++]) : -1;
> +        b = assoc_idx < vcpu_assoc_size ?
> +            be32_to_cpu(vcpu_assoc[assoc_idx++]) : -1;
> +
> +        args[idx] = ASSOCIATIVITY(a, b);
>      }
>  #undef ASSOCIATIVITY
>  

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson