Re: [PATCH v3 03/10] KVM: selftests: Use an array for guest_regs (and fix offsets)

[Yosry Ahmed]

On Fri, Jul 24, 2026 at 8:33 AM Sean Christopherson <seanjc@xxxxxxxxxx> wrote:
>
> On Mon, Jun 29, 2026, Yosry Ahmed wrote:
> > The assembly code defined by SAVE_GPR_C uses the wrong offsets for some
> > registers in guest_regs. For example, the offset of RCX should 0x08 not
> > 0x10. Also, the last offset in the struct (R15) is 0x78, not 0x80, so
> > the code actually saves and restore beyond the end of gpr64_regs.
> >
> > Eliminate hardcoded offsets by using an array instead of a struct
> > (similar to KVM's per-vCPU regs), and use the array index to generate
> > the offset. While at it, rename SAVE_GPR_C and LOAD_GPR_C to a single
> > macro, SVM_SWITCH_GPRS_ASM.
> >
> > Signed-off-by: Yosry Ahmed <yosry@xxxxxxxxxx>
> > ---
> >  .../selftests/kvm/include/x86/processor.h     | 36 +++++++-------
> >  tools/testing/selftests/kvm/lib/x86/svm.c     | 47 ++++++++++---------
> >  2 files changed, 41 insertions(+), 42 deletions(-)
> >
> > diff --git a/tools/testing/selftests/kvm/include/x86/processor.h b/tools/testing/selftests/kvm/include/x86/processor.h
> > index 7d3a27bc0d842..535f26e077570 100644
> > --- a/tools/testing/selftests/kvm/include/x86/processor.h
> > +++ b/tools/testing/selftests/kvm/include/x86/processor.h
> > @@ -396,25 +396,23 @@ static inline unsigned int x86_model(unsigned int eax)
> >  #define PTE_GET_PA(pte)              ((pte) & PHYSICAL_PAGE_MASK)
> >  #define PTE_GET_PFN(pte)        (PTE_GET_PA(pte) >> PAGE_SHIFT)
> >
> > -/* General Registers in 64-Bit Mode */
> > -struct gpr64_regs {
> > -     u64 rax;
> > -     u64 rcx;
> > -     u64 rdx;
> > -     u64 rbx;
> > -     u64 rsp;
> > -     u64 rbp;
> > -     u64 rsi;
> > -     u64 rdi;
> > -     u64 r8;
> > -     u64 r9;
> > -     u64 r10;
> > -     u64 r11;
> > -     u64 r12;
> > -     u64 r13;
> > -     u64 r14;
> > -     u64 r15;
> > -};
> > +#define GUEST_REGS_RAX       0
> > +#define GUEST_REGS_RCX       1
> > +#define GUEST_REGS_RDX       2
> > +#define GUEST_REGS_RBX       3
> > +#define GUEST_REGS_RSP       4
> > +#define GUEST_REGS_RBP       5
> > +#define GUEST_REGS_RSI       6
> > +#define GUEST_REGS_RDI       7
> > +#define GUEST_REGS_R8        8
> > +#define GUEST_REGS_R9        9
> > +#define GUEST_REGS_R10       10
> > +#define GUEST_REGS_R11       11
> > +#define GUEST_REGS_R12       12
> > +#define GUEST_REGS_R13       13
> > +#define GUEST_REGS_R14       14
> > +#define GUEST_REGS_R15       15
> > +#define NR_GUEST_REGS        (GUEST_REGS_R15 + 1)
>
> Eh, these aren't guest specific, they're architectural x86 values.  Ideally,
> KVM's own VCPU_REGS_xxx would be X86_REGS_xxx, which is possible now that KVM
> doesn't do pretend RIP is a GPR.

I just called these GUEST_REGS_* because they are used to index
guest_regs, but yeah I can name them X86_REGS_*.

>
> I also prefer arch/x86/include/asm/kvm_host.h's approach of using an enum, e.g.
>
> enum x86_general_purpose_reg {
>         X86_REGS_RAX,
>         X86_REGS_RCX,
>         X86_REGS_RDX,
>         X86_REGS_RBX,
>         X86_REGS_RSP,
>         X86_REGS_RBP,
>         X86_REGS_RSI,
>         X86_REGS_RDI,
>         X86_REGS_R8,
>         X86_REGS_R9,
>         X86_REGS_R10,
>         X86_REGS_R11,
>         X86_REGS_R12,
>         X86_REGS_R13,
>         X86_REGS_R14,
>         X86_REGS_R15,
>         NR_X86_GENERAL_PURPOSE_REGS,
> };

Oh I wanted to, but the compiler wouldn't let me use enum values
directly in the preprocessor, see:
https://lore.kernel.org/kvm/CAO9r8zPz8WbijVN6mgLb8OaU6=tNSSz1NgNBjZCsC36D4+TO+w@xxxxxxxxxxxxxx/.

I can do an enum *and* macros using enum values if you prefer so, like:

#define __GUEST_REGS_RAX 0
...

enum guest_regs {
    GUEST_REGS_RAX = __GUEST_REGS_RAX,
    ...
    NR_GUEST_REGS,
};