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

[Sean Christopherson <seanjc@google.com>]

On Wed, Apr 22, 2026, Yosry Ahmed wrote:
> 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@google.com>
> > > > ---
> > > >  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@kernel.org/
> > > 
> > > 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,

Yes, but it'll largely be one-time pain.

> and we'll occassionally need to move things back. 

Maybe.  I don't think this is likely, so long as we limit the header to including
only kvm_host.h.  If we're at all careful, there should be no need to move things
back into x86.h, e.g. to get access to a structure/asset that's defined in x86.h.

Somewhat related: I've tried, oh so many times, to move stuff out of kvm_host.h
and into x86.h, or some other header in arch/x86/kvm, so that KVM-internal stuff
isn't visible to the external world.  And _that_ is fraught with dependencies
because that are all but impossible to unravel.

I mention that because I do agree that some movement between headers is doomed
to fail, but I don't think that will be a problem here, because what few dependencies
the header would have are pretty much guaranteed to say in kvm_host.h, i.e. there's
basically zero chance of moving key structures/definitions out of kvm_host.h and
into x86.h

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

IMO, it's pretty clear cut that those have no business being in kvm_regs.h or
whatever.  They don't track register state.

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

Yes, but we'll nitpick random crap no matter what :-)

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

For me, the extra effort is worth it (even though in this case I'm not convinced
the extra effort will be meaningful).  I view it as a "write once, read many" case.

Code is read far, far more often than it's modified.  If we throw everything into
one (or few) giant header, then reading code becomes more difficult.  And so I'm
willing to risk some pain and churn in order to yield a code base is that is easier
to read and understand.

> So my (very unimportant) opinion is still to merge kvm_cache_regs.h into

Your opinions aren't unimportant, quite the opposite.  In general, but especially
for highly subjective things like this, input from others provides a _lot_ of
value.  At the very least, it provides an opportunity for me to explain what I'm
thinking, my motivations, long-term plans, etc.  But more often than not, input
and pushback yields a better end result, sometimes a _significantly_ better end
result.

> x86.h, and we can always revisit later if that starts becoming a real pain.

In theory, yes.  In practice, peeling apart a file is almost always much more
difficult than smashing two or more files together.