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

Alexandru Elisei
2020-09-17T09:55:32Z

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.

[1]
https://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git/commit/?id=3102bc0e6ac7

Thanks,
Alex