Re: [PATCH v3 2/7] spapr, spapr_numa: handle vcpu ibm,associativity

[Daniel Henrique Barboza]

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