Re: [Qemu-arm] [PATCH] hw/arm/virt: gicv3: use all target-list bits

[Peter Maydell]

On 21 June 2016 at 19:58, Andrew Jones <address@hidden> wrote:
> Signed-off-by: Andrew Jones <address@hidden>

I think this commit message could be improved...it's both
very short and a bit off the mark.

> ---
>  hw/arm/virt.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
>
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index c5c125e9204a0..53f545921003c 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -1271,6 +1271,16 @@ static void machvirt_init(MachineState *machine)
>          }
>          cpuobj = object_new(object_class_get_name(oc));
>
> +        /* Adjust MPIDR per the GIC's target-list size. */
> +        if (gic_version == 3) {
> +            CPUState *cs = CPU(cpuobj);
> +            uint8_t Aff1 = cs->cpu_index / 16;
> +            uint8_t Aff0 = cs->cpu_index % 16;
> +
> +            object_property_set_int(cpuobj, (Aff1 << ARM_AFF1_SHIFT) | Aff0,
> +                                    "mp-affinity", NULL);
> +        }

We still don't have support in KVM for telling the CPU what
affinity to use, so these may get overridden later if KVM's
idea of affinity and ours differ. I guess that's no different
to what we have today, though.

I think it would be better to:
 * use the loop index 'n' rather than fishing the cpu_index
   out of the CPUState.
 * do this regardless of GIC version (if it's GICv2 we only
   have 8 CPUs max anyway)
 * comment it as "Create our CPUs in clusters of 16; this suits
   the GICv3's target list limitations, and matches how KVM
   assigns them"
 * for 32-bit, set the mp-affinity in the same arrangement the
   kernel does for KVM, which is clusters of 4 CPUs
 * note also in the comment that for KVM these will be overridden
   by the hard-coded topology in the kernel when the CPU is
   realized

[is changing mp-affinity a migration compat break?]

thanks
-- PMM