Re: [PATCH v2 01/16] KVM: x86: Rename register accessors to be GPR-specific

[Sean Christopherson <seanjc@google.com>]

On Fri, Mar 06, 2026, Chang S. Bae wrote:
> On 3/4/2026 5:35 PM, Sean Christopherson wrote:
> > On Mon, Jan 12, 2026, Chang S. Bae wrote:
> > > Refactor the VCPU register state accessors to make them explicitly
> > > GPR-only.
> > 
> > I like "register" though.
> 
> Yeah, agree that it is more general.
> 
> > 
> > > The existing register accessors operate on the cached VCPU register
> > > state. That cache holds GPRs and RIP. RIP has its own interface already.
> > 
> > Isn't it possible that e.g. get_vmx_mem_address() will do kvm_register_read()
> > for a RIP-relative address?

Answering my own question: no, this isn't possible, specifically because RIP can't
be addressed via "normal" methods (as Chang points out below, KVM's index of 16
is completely arbitrary).

Instead, for RIP relative addressing, the full "offset" gets reported via
EXIT_QUALIFICATION.

> One could RIP isn't a pure GPR, but it's also not something entirely different either.
> 
> The 'reg' argument has historically matched the index of the register cache
> array, vcpu::arch::regs[]. When extending the accessors to support EGPRs, it
> looked smooth to keep using it as a register ID, since that wires up cleanly
> with VMX instruction info and emulator sites. But then reg=16 immediately
> conflicts with RIP.
> 
> Separating accessors for RIP and GPRs was an option. Yes, the usages are
> very close and EGPRs are strictly not *legacy* GPRs.
> 
> Then, another option would be adjust RIP numbering. For example, use
> something like VCPU_REGS_RIP=32 for the accessor, while keeping a separate
> value like __VCPU_REGS_RIP=16 for the reg cache index. But there are many
> sites directly referencing regs[] and the change looked rather ugly -- two
> numberings for RIP alone.

Oh, yikes, I didn't even see that this series is playing games with the register
indices.

Whatever we do, the changelog asbolutely needs to call out the real motiviation.
Because nothing in here screams "KVM's APX implementation depends on this and
things will break horribly if kvm_gpr_read() is called with VCPU_REGS_RIP".

  The existing register accessors operate on the cached VCPU register
  state. That cache holds GPRs and RIP. RIP has its own interface already.
  This renaming clarifies GPR access only.

I'll try to come back to this tomorrow with more complete thoughts and hopefully
an idea or two on where to go, but I am most definitely against the current
implementation drops the safeguards provided by kvm_register_{read,write}_raw().
E.g. passing in VCPU_REGS_RIP to kvm_gpr_read() will compile just fine, but will
read the wrong register on APX capable hardware.

There's still kinda sorta some protection there as kvm_read_egpr() will WARN on
!APX hardware, but the hole scheme is kludgy at best.