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

[Marc Zyngier <maz@kernel.org>]

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...