Re: [PATCH 10/29] KVM: share statistics for same vCPU id on different planes

[Paolo Bonzini]

On Fri, Jun 6, 2025 at 6:23 PM Sean Christopherson <seanjc@xxxxxxxxxx> wrote:
> Rather than special case invidiual fields, I think we should give kvm_vcpu the
> same treatment as "struct kvm", and have kvm_vcpu represent the overall vCPU,
> with an array of planes to hold the sub-vCPUs.

Yes, I agree. This is also the direction that Roy took in
https://patchew.org/linux/cover.1726506534.git.roy.hopkins@xxxxxxxx/.
I thought it wasn't necessary, but it's bad for all the reasons you
mention before.

While he kept the struct on all planes for simplicity, something which
I stole for __stat here, the idea is the same as what you mention
below.

> Having "kvm_vcpu" represent a plane, while "kvm" represents the overall VM, is
> conceptually messy.  And more importantly, I think the approach taken here will
> be nigh impossible to maintain, and will have quite a bit of baggage.  E.g. planes1+
> will be filled with dead memory, and we also risk goofs where KVM could access
> __stat in a plane1+ vCPU.

Well, that's the reason for the __ so I don't think it's too risky -
but it's not possible to add __ to all fields of course.

Besides, if you have a zillion pointers to fields you might as well
have a single pointer to the common fields.

> Extracing fields to a separate kvm_vcpu_plane will obviously require a *lot* more
> churn, but I think in the long run it will be less work in total, because we won't
> spend as much time chasing down bugs.
>
> Very little per-plane state is in "struct kvm_vcpu", so I think we can do the big
> conversion on a per-arch basis via a small amount of #ifdefs, i.e. not be force to
> immediatedly convert all architectures to a kvm_vcpu vs. kvm_vcpu_plane world.

Roy didn't even have a struct that is per-arch and common to all
planes. He did have a flag-day conversion to add "->common"
everywhere, but I agree that it's better to add something like

struct kvm_vcpu_plane {
...
#ifndef KVM_HAS_PLANES
#include "kvm_common_fields.h"
#endif
}

#ifdef KVM_HAS_PLANES
struct kvm_vcpu {
#include "kvm_common_fields.h"
}
#else
#define kvm_vcpu kvm_vcpu_plane
#endif

Paolo