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

[Sean Christopherson]

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 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,
};

And I think it makes sense to squeeze this in with the other architectural
definitions, e.g.

diff --git tools/testing/selftests/kvm/include/x86/processor.h tools/testing/selftests/kvm/include/x86/processor.h
index 7d3a27bc0d84..bbb7c71bb768 100644
--- tools/testing/selftests/kvm/include/x86/processor.h
+++ tools/testing/selftests/kvm/include/x86/processor.h
@@ -57,6 +57,26 @@ const char *ex_str(int vector);
 #define X86_EFLAGS_VIP         BIT(20) /* Virtual Interrupt Pending */
 #define X86_EFLAGS_ID          BIT(21) /* CPUID detection */
 
+enum x86_general_purpose_reg {
+       X86_REGS_RAX,
+       VCPU_REGS_RCX,
+       VCPU_REGS_RDX,
+       VCPU_REGS_RBX,
+       VCPU_REGS_RSP,
+       VCPU_REGS_RBP,
+       VCPU_REGS_RSI,
+       VCPU_REGS_RDI,
+       VCPU_REGS_R8,
+       VCPU_REGS_R9,
+       VCPU_REGS_R10,
+       VCPU_REGS_R11,
+       VCPU_REGS_R12,
+       VCPU_REGS_R13,
+       VCPU_REGS_R14,
+       VCPU_REGS_R15,
+       NR_X86_GENERAL_PURPOSE_REGS,
+};
+
 #define X86_CR4_VME            (1ul << 0)
 #define X86_CR4_PVI            (1ul << 1)
 #define X86_CR4_TSD            (1ul << 2)