On Thu, Jun 23, 2016 at 12:15:59PM +0100, Peter Maydell wrote:
> On 21 June 2016 at 19:58, Andrew Jones <drjo...@redhat.com> wrote:
> > Signed-off-by: Andrew Jones <drjo...@redhat.com>
>
> 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
Actually this depends on the host. KVM will use all 8 tlist bits
with ARM guests, when running on an AArch64 host. As a guest doesn't
really have to worry about clusters for shared cache, then ignoring
the 4 per cluster requirement is likely fine (is that a hard
requirement anyway?), but it could confuse a guest.
So we can either
a) play it safe and always use clusters of 4 for ARM guests, and
KVM will get "fixed" when we start managing the guest's MPIDR
from userspace, or
b) use 8 here, as TCG always has, and KVM does for AArch32 guests.
This might be less safe, but also improves SGI efficiency.
Thanks,
drew
> * 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
>
