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
