Re: [PATCH 2/2] kvm/arm: Add mp_affinity for arm vcpu

[Marc Zyngier <maz@xxxxxxxxxx>]

On 2020-09-17 10:47, Alexandru Elisei wrote:
> Hi,
>
> On 9/17/20 9:42 AM, Marc Zyngier wrote:
> > On 2020-09-17 09:04, Andrew Jones wrote:
> > > On Thu, Sep 17, 2020 at 08:47:42AM +0100, Marc Zyngier wrote:
> > > > On 2020-09-17 03:30, Ying Fang wrote:
> > > > > Allow userspace to set MPIDR using vcpu ioctl KVM_ARM_SET_MP_AFFINITY,
> > > > > so that we can support cpu topology for arm.
> > > >
> > > > MPIDR has *nothing* to do with CPU topology in the ARM architecture.
> > > > I encourage you to have a look at the ARM ARM and find out how often
> > > > the word "topology" is used in conjunction with the MPIDR_EL1 
> > > > register.
> > >
> > > Hi Marc,
> > >
> > > I mostly agree. However, the CPU topology descriptions use MPIDR to
> > > identify PEs. If userspace wants to build topology descriptions then
> > > it either needs to
> > >
> > > 1) build them after instantiating all KVM VCPUs in order to query KVM
> > >    for each MPIDR, or
> > > 2) have a way to ask KVM for an MPIDR of given VCPU ID in advance
> > >    (maybe just a scratch VCPU), or
> > > 3) have control over the MPIDRs so it can choose them when it likes,
> > >    use them for topology descriptions, and then instantiate KVM VCPUs
> > >    with them.
> > >
> > > I think (3) is the most robust approach, and it has the least 
> > > overhead.
> >
> > I don't disagree with the goal, and not even with the choice of
> > implementation (though I have huge reservations about its quality).
> >
> > But the key word here is *userspace*. Only userspace has a notion of
> > how MPIDR values map to the assumed topology. That's not something
> > that KVM does nor should interpret (aside from the GIC-induced Aff0
> > brain-damage). So talking of "topology" in a KVM kernel patch sends
> > the wrong message, and that's all this remark was about.
>
> There's also a patch queued for next which removes using MPIDR as a 
> source of
> information about CPU topology [1]: "arm64: topology: Stop using MPIDR 
> for
> topology information".
>
> I'm not really sure how useful KVM fiddling with the guest MPIDR will 
> be going
> forward, at least for a Linux guest.

I think these are two orthogonal things. There is value in setting MPIDR
to something different as a way to replicate an existing system, for
example. But deriving *any* sort of topology information from MPIDR 
isn't
reliable at all, and is better expressed by firmware tables (and even
that isn't great).

As far as I am concerned, this patch fits in the "cosmetic" department.
It's a "nice to have", but doesn't really solve much. Firmware tables
and userspace placement of the vcpus are key.

Thanks,

        M.
--
Jazz is not dead. It just smells funny...