Vcpus have an additional paramenter to be appended, vcpu_id. This
also changes the size of the of property itself, which is being
represented in index 0 of numa_assoc_array[cpu->node_id],
and defaults to MAX_DISTANCE_REF_POINTS for all cases but
vcpus.

All this logic makes more sense in spapr_numa.c, where we handle
everything NUMA and associativity. A new helper spapr_numa_fixup_cpu_dt()
was added, and spapr.c uses it the same way as it was using the former
spapr_fixup_cpu_numa_dt().

Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
---
 hw/ppc/spapr.c              | 17 +----------------
 hw/ppc/spapr_numa.c         | 27 +++++++++++++++++++++++++++
 include/hw/ppc/spapr_numa.h |  2 ++
 3 files changed, 30 insertions(+), 16 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 1ad6f59863..badfa86319 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -202,21 +202,6 @@ static int spapr_fixup_cpu_smt_dt(void *fdt, int offset, PowerPCCPU *cpu,
     return ret;
 }
 
-static int spapr_fixup_cpu_numa_dt(void *fdt, int offset, PowerPCCPU *cpu)
-{
-    int index = spapr_get_vcpu_id(cpu);
-    uint32_t associativity[] = {cpu_to_be32(0x5),
-                                cpu_to_be32(0x0),
-                                cpu_to_be32(0x0),
-                                cpu_to_be32(0x0),
-                                cpu_to_be32(cpu->node_id),
-                                cpu_to_be32(index)};
-
-    /* Advertise NUMA via ibm,associativity */
-    return fdt_setprop(fdt, offset, "ibm,associativity", associativity,
-                          sizeof(associativity));
-}
-
 static void spapr_dt_pa_features(SpaprMachineState *spapr,
                                  PowerPCCPU *cpu,
                                  void *fdt, int offset)
@@ -785,7 +770,7 @@ static void spapr_dt_cpu(CPUState *cs, void *fdt, int offset,
                       pft_size_prop, sizeof(pft_size_prop))));
 
     if (ms->numa_state->num_nodes > 1) {
-        _FDT(spapr_fixup_cpu_numa_dt(fdt, offset, cpu));
+        _FDT(spapr_numa_fixup_cpu_dt(spapr, fdt, offset, cpu));
     }
 
     _FDT(spapr_fixup_cpu_smt_dt(fdt, offset, cpu, compat_smt));
diff --git a/hw/ppc/spapr_numa.c b/hw/ppc/spapr_numa.c
index f6b6fe648f..1a1ec8bcff 100644
--- a/hw/ppc/spapr_numa.c
+++ b/hw/ppc/spapr_numa.c
@@ -45,6 +45,33 @@ void spapr_numa_write_associativity_dt(SpaprMachineState *spapr, void *fdt,
                       sizeof(spapr->numa_assoc_array[nodeid]))));
 }
 
+int spapr_numa_fixup_cpu_dt(SpaprMachineState *spapr, void *fdt,
+                            int offset, PowerPCCPU *cpu)
+{
+    uint vcpu_assoc_size = NUMA_ASSOC_SIZE + 1;
+    uint32_t vcpu_assoc[vcpu_assoc_size];
+    int index = spapr_get_vcpu_id(cpu);
+    int i;
+
+    /*
+     * VCPUs have an extra 'cpu_id' value in ibm,associativity
+     * compared to other resources. Increment the size at index
+     * 0, copy all associativity domains already set, then put
+     * cpu_id last.
+     */
+    vcpu_assoc[0] = cpu_to_be32(MAX_DISTANCE_REF_POINTS + 1);
+
+    for (i = 1; i <= MAX_DISTANCE_REF_POINTS; i++) {
+        vcpu_assoc[i] = spapr->numa_assoc_array[cpu->node_id][i];
+    }
+
+    vcpu_assoc[vcpu_assoc_size - 1] = cpu_to_be32(index);
+
+    /* Advertise NUMA via ibm,associativity */
+    return fdt_setprop(fdt, offset, "ibm,associativity",
+                       vcpu_assoc, sizeof(vcpu_assoc));
+}
+
 /*
  * Helper that writes ibm,associativity-reference-points and
  * max-associativity-domains in the RTAS pointed by @rtas
diff --git a/include/hw/ppc/spapr_numa.h b/include/hw/ppc/spapr_numa.h
index a2a4df55f7..43c6a16fe3 100644
--- a/include/hw/ppc/spapr_numa.h
+++ b/include/hw/ppc/spapr_numa.h
@@ -27,5 +27,7 @@ void spapr_numa_associativity_init(SpaprMachineState *spapr,
 void spapr_numa_write_rtas_dt(SpaprMachineState *spapr, void *fdt, int rtas);
 void spapr_numa_write_associativity_dt(SpaprMachineState *spapr, void *fdt,
                                        int offset, int nodeid);
+int spapr_numa_fixup_cpu_dt(SpaprMachineState *spapr, void *fdt,
+                            int offset, PowerPCCPU *cpu);
 
 #endif /* HW_SPAPR_NUMA_H */
-- 
2.26.2

On Thu, Sep 03, 2020 at 07:06:34PM -0300, Daniel Henrique Barboza wrote:
> Vcpus have an additional paramenter to be appended, vcpu_id. This
> also changes the size of the of property itself, which is being
> represented in index 0 of numa_assoc_array[cpu->node_id],
> and defaults to MAX_DISTANCE_REF_POINTS for all cases but
> vcpus.
> 
> All this logic makes more sense in spapr_numa.c, where we handle
> everything NUMA and associativity. A new helper spapr_numa_fixup_cpu_dt()
> was added, and spapr.c uses it the same way as it was using the former
> spapr_fixup_cpu_numa_dt().
> 
> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
> ---
>  hw/ppc/spapr.c              | 17 +----------------
>  hw/ppc/spapr_numa.c         | 27 +++++++++++++++++++++++++++
>  include/hw/ppc/spapr_numa.h |  2 ++
>  3 files changed, 30 insertions(+), 16 deletions(-)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 1ad6f59863..badfa86319 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -202,21 +202,6 @@ static int spapr_fixup_cpu_smt_dt(void *fdt, int offset, PowerPCCPU *cpu,
>      return ret;
>  }
>  
> -static int spapr_fixup_cpu_numa_dt(void *fdt, int offset, PowerPCCPU *cpu)
> -{
> -    int index = spapr_get_vcpu_id(cpu);
> -    uint32_t associativity[] = {cpu_to_be32(0x5),
> -                                cpu_to_be32(0x0),
> -                                cpu_to_be32(0x0),
> -                                cpu_to_be32(0x0),
> -                                cpu_to_be32(cpu->node_id),
> -                                cpu_to_be32(index)};
> -
> -    /* Advertise NUMA via ibm,associativity */
> -    return fdt_setprop(fdt, offset, "ibm,associativity", associativity,
> -                          sizeof(associativity));
> -}
> -
>  static void spapr_dt_pa_features(SpaprMachineState *spapr,
>                                   PowerPCCPU *cpu,
>                                   void *fdt, int offset)
> @@ -785,7 +770,7 @@ static void spapr_dt_cpu(CPUState *cs, void *fdt, int offset,
>                        pft_size_prop, sizeof(pft_size_prop))));
>  
>      if (ms->numa_state->num_nodes > 1) {
> -        _FDT(spapr_fixup_cpu_numa_dt(fdt, offset, cpu));
> +        _FDT(spapr_numa_fixup_cpu_dt(spapr, fdt, offset, cpu));
>      }
>  
>      _FDT(spapr_fixup_cpu_smt_dt(fdt, offset, cpu, compat_smt));
> diff --git a/hw/ppc/spapr_numa.c b/hw/ppc/spapr_numa.c
> index f6b6fe648f..1a1ec8bcff 100644
> --- a/hw/ppc/spapr_numa.c
> +++ b/hw/ppc/spapr_numa.c
> @@ -45,6 +45,33 @@ void spapr_numa_write_associativity_dt(SpaprMachineState *spapr, void *fdt,
>                        sizeof(spapr->numa_assoc_array[nodeid]))));
>  }
>  
> +int spapr_numa_fixup_cpu_dt(SpaprMachineState *spapr, void *fdt,
> +                            int offset, PowerPCCPU *cpu)
> +{
> +    uint vcpu_assoc_size = NUMA_ASSOC_SIZE + 1;
> +    uint32_t vcpu_assoc[vcpu_assoc_size];
> +    int index = spapr_get_vcpu_id(cpu);
> +    int i;
> +
> +    /*
> +     * VCPUs have an extra 'cpu_id' value in ibm,associativity
> +     * compared to other resources. Increment the size at index
> +     * 0, copy all associativity domains already set, then put
> +     * cpu_id last.
> +     */
> +    vcpu_assoc[0] = cpu_to_be32(MAX_DISTANCE_REF_POINTS + 1);
> +
> +    for (i = 1; i <= MAX_DISTANCE_REF_POINTS; i++) {
> +        vcpu_assoc[i] = spapr->numa_assoc_array[cpu->node_id][i];
> +    }

You could use a single memcpy() here as well.

> +
> +    vcpu_assoc[vcpu_assoc_size - 1] = cpu_to_be32(index);
> +
> +    /* Advertise NUMA via ibm,associativity */
> +    return fdt_setprop(fdt, offset, "ibm,associativity",
> +                       vcpu_assoc, sizeof(vcpu_assoc));
> +}
> +
>  /*
>   * Helper that writes ibm,associativity-reference-points and
>   * max-associativity-domains in the RTAS pointed by @rtas
> diff --git a/include/hw/ppc/spapr_numa.h b/include/hw/ppc/spapr_numa.h
> index a2a4df55f7..43c6a16fe3 100644
> --- a/include/hw/ppc/spapr_numa.h
> +++ b/include/hw/ppc/spapr_numa.h
> @@ -27,5 +27,7 @@ void spapr_numa_associativity_init(SpaprMachineState *spapr,
>  void spapr_numa_write_rtas_dt(SpaprMachineState *spapr, void *fdt, int rtas);
>  void spapr_numa_write_associativity_dt(SpaprMachineState *spapr, void *fdt,
>                                         int offset, int nodeid);
> +int spapr_numa_fixup_cpu_dt(SpaprMachineState *spapr, void *fdt,
> +                            int offset, PowerPCCPU *cpu);
>  
>  #endif /* HW_SPAPR_NUMA_H */

-- 
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

On 9/3/20 8:23 PM, David Gibson wrote:
> On Thu, Sep 03, 2020 at 07:06:34PM -0300, Daniel Henrique Barboza wrote:
>> Vcpus have an additional paramenter to be appended, vcpu_id. This
>> also changes the size of the of property itself, which is being
>> represented in index 0 of numa_assoc_array[cpu->node_id],
>> and defaults to MAX_DISTANCE_REF_POINTS for all cases but
>> vcpus.
>>
>> All this logic makes more sense in spapr_numa.c, where we handle
>> everything NUMA and associativity. A new helper spapr_numa_fixup_cpu_dt()
>> was added, and spapr.c uses it the same way as it was using the former
>> spapr_fixup_cpu_numa_dt().
>>
>> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
>> ---
>>   hw/ppc/spapr.c              | 17 +----------------
>>   hw/ppc/spapr_numa.c         | 27 +++++++++++++++++++++++++++
>>   include/hw/ppc/spapr_numa.h |  2 ++
>>   3 files changed, 30 insertions(+), 16 deletions(-)
>>
>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
>> index 1ad6f59863..badfa86319 100644
>> --- a/hw/ppc/spapr.c
>> +++ b/hw/ppc/spapr.c
>> @@ -202,21 +202,6 @@ static int spapr_fixup_cpu_smt_dt(void *fdt, int offset, PowerPCCPU *cpu,
>>       return ret;
>>   }
>>   
>> -static int spapr_fixup_cpu_numa_dt(void *fdt, int offset, PowerPCCPU *cpu)
>> -{
>> -    int index = spapr_get_vcpu_id(cpu);
>> -    uint32_t associativity[] = {cpu_to_be32(0x5),
>> -                                cpu_to_be32(0x0),
>> -                                cpu_to_be32(0x0),
>> -                                cpu_to_be32(0x0),
>> -                                cpu_to_be32(cpu->node_id),
>> -                                cpu_to_be32(index)};
>> -
>> -    /* Advertise NUMA via ibm,associativity */
>> -    return fdt_setprop(fdt, offset, "ibm,associativity", associativity,
>> -                          sizeof(associativity));
>> -}
>> -
>>   static void spapr_dt_pa_features(SpaprMachineState *spapr,
>>                                    PowerPCCPU *cpu,
>>                                    void *fdt, int offset)
>> @@ -785,7 +770,7 @@ static void spapr_dt_cpu(CPUState *cs, void *fdt, int offset,
>>                         pft_size_prop, sizeof(pft_size_prop))));
>>   
>>       if (ms->numa_state->num_nodes > 1) {
>> -        _FDT(spapr_fixup_cpu_numa_dt(fdt, offset, cpu));
>> +        _FDT(spapr_numa_fixup_cpu_dt(spapr, fdt, offset, cpu));
>>       }
>>   
>>       _FDT(spapr_fixup_cpu_smt_dt(fdt, offset, cpu, compat_smt));
>> diff --git a/hw/ppc/spapr_numa.c b/hw/ppc/spapr_numa.c
>> index f6b6fe648f..1a1ec8bcff 100644
>> --- a/hw/ppc/spapr_numa.c
>> +++ b/hw/ppc/spapr_numa.c
>> @@ -45,6 +45,33 @@ void spapr_numa_write_associativity_dt(SpaprMachineState *spapr, void *fdt,
>>                         sizeof(spapr->numa_assoc_array[nodeid]))));
>>   }
>>   
>> +int spapr_numa_fixup_cpu_dt(SpaprMachineState *spapr, void *fdt,
>> +                            int offset, PowerPCCPU *cpu)
>> +{
>> +    uint vcpu_assoc_size = NUMA_ASSOC_SIZE + 1;
>> +    uint32_t vcpu_assoc[vcpu_assoc_size];
>> +    int index = spapr_get_vcpu_id(cpu);
>> +    int i;
>> +
>> +    /*
>> +     * VCPUs have an extra 'cpu_id' value in ibm,associativity
>> +     * compared to other resources. Increment the size at index
>> +     * 0, copy all associativity domains already set, then put
>> +     * cpu_id last.
>> +     */
>> +    vcpu_assoc[0] = cpu_to_be32(MAX_DISTANCE_REF_POINTS + 1);
>> +
>> +    for (i = 1; i <= MAX_DISTANCE_REF_POINTS; i++) {
>> +        vcpu_assoc[i] = spapr->numa_assoc_array[cpu->node_id][i];
>> +    }
> 
> You could use a single memcpy() here as well.

I'm moving this code to a new function in patch 06. I'll change this for memcpy()
there for v4.


Thanks,


DHB




> 
>> +
>> +    vcpu_assoc[vcpu_assoc_size - 1] = cpu_to_be32(index);
>> +
>> +    /* Advertise NUMA via ibm,associativity */
>> +    return fdt_setprop(fdt, offset, "ibm,associativity",
>> +                       vcpu_assoc, sizeof(vcpu_assoc));
>> +}
>> +
>>   /*
>>    * Helper that writes ibm,associativity-reference-points and
>>    * max-associativity-domains in the RTAS pointed by @rtas
>> diff --git a/include/hw/ppc/spapr_numa.h b/include/hw/ppc/spapr_numa.h
>> index a2a4df55f7..43c6a16fe3 100644
>> --- a/include/hw/ppc/spapr_numa.h
>> +++ b/include/hw/ppc/spapr_numa.h
>> @@ -27,5 +27,7 @@ void spapr_numa_associativity_init(SpaprMachineState *spapr,
>>   void spapr_numa_write_rtas_dt(SpaprMachineState *spapr, void *fdt, int rtas);
>>   void spapr_numa_write_associativity_dt(SpaprMachineState *spapr, void *fdt,
>>                                          int offset, int nodeid);
>> +int spapr_numa_fixup_cpu_dt(SpaprMachineState *spapr, void *fdt,
>> +                            int offset, PowerPCCPU *cpu);
>>   
>>   #endif /* HW_SPAPR_NUMA_H */
>