Re: [PATCH 06/11] KVM: x86: Move kvm_<reg>_{read,write}() definitions to x86.h

[Yosry Ahmed]

On Tue, Apr 21, 2026 at 05:40:00PM -0700, Sean Christopherson wrote:
> On Tue, Apr 21, 2026, Yosry Ahmed wrote:
> > On Thu, Apr 09, 2026 at 04:56:17PM -0700, Sean Christopherson wrote:
> > > Move the direct GPR accessors to x86.h so that they can use
> > > is_64_bit_mode().
> > > 
> > > No functional change intended.
> > > 
> > > Signed-off-by: Sean Christopherson <seanjc@xxxxxxxxxx>
> > > ---
> > >  arch/x86/kvm/kvm_cache_regs.h | 34 ----------------------------------
> > >  arch/x86/kvm/x86.h            | 34 ++++++++++++++++++++++++++++++++++
> > 
> > That's a shame, register accessors semantically fit in kvm_cache_regs.h,
> 
> Sort of.  I actually had the opposite reaction.  KVM very specifically doesn't
> do available/dirty tracking for GPRs, in large part because it's not a caching
> behavior per se: vcpu->arch.regs[] is _the_ source of truth for GPRs (modulo
> RSP on Intel).
> 
> > but taking a step back.. is it even worth having kvm_cache_regs.h
> > anymore?
> > 
> > At some point I needed to move guest mode helpers out of there too (as
> > they should be):
> > 
> > https://lore.kernel.org/kvm/20260326031150.3774017-3-yosry@xxxxxxxxxx/
> > 
> > With this patch and that one, we'd have <200 lines left. Is there a
> > reason why it can't just be merged with x86.h? I don't see any of the
> > headers included by x86.h including kvm_cache_regs.h.
> 
> What if we go in the opposite direction?  I didn't try moving e.g. is_64_bit_mode()
> into kvm_cache_regs.h because that reaaaaaly stretches the meaning of "cache regs".
> 
> But if we rename kvm_cache_regs.h to something like kvm_regs.h, then I think we
> can move more of the simpler accessors (and mutators?) into kvm_regs.h.  The
> motivation would be to keep x86.h from becoming a chonker.  One of the regrets
> with x86.c is that it became a dumping ground for everything that didn't have an
> obvious home, and now it's sitting at nearly 15k LoC.
> 
> Naming is hard, but if we can come up with a kvm_blah.{h,c} pair, maybe we can
> kill two birds with one stone?  Or at least kill one, and injure the other :-)

We can do that, and name it kvm_regs.h or even kvm_accessors.h or
something. But then we'll sink some time into figuring out what needs to
be moved from x86.h to that header, and we'll occassionally need to move
things back. For example, the patch I mentioned above needed to move
guest mode helpers to update the PMU from there. We may not need that
anymore, but I think it's possible we run into this situation in the
future again.

We'll also spend more time nitpicking where new definitions need to go.

I understand wanting to split things up as much as possible and keeping
the files small-ish, but for headers specifically I wonder if it
introduces more problems than it solves, specifically because of
circular dependency issues becoming easier to run into with a larger
number of smaller headers.

So my (very unimportant) opinion is still to merge kvm_cache_regs.h into
x86.h, and we can always revisit later if that starts becoming a real
pain.