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 <[email protected]> > --- > 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
signature.asc
Description: PGP signature
