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