[PATCH v4 03/12] KVM: selftests: Rework GPR registers switching for SVM (and fix offsets)

[Yosry Ahmed <yosry@kernel.org>]

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 be 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 dynamically generating offsets using
offset_of() and using macros to pass the offsets to assembly as asm
constraints.

To avoid register conflicts in inline assembly (since almost all GPRs are
context-switched), access guest_regs via absolute symbol addressing
(guest_regs + offset) rather than using a base register which could
get overwritten mid-assembly.

While at it, rename SAVE_GPR_C and LOAD_GPR_C to a single macro,
SVM_SWITCH_GPRS_ASM, rename gpr64_regs to guest_regs, and expose it in
processor.h (in preparation for reusing it for VMX).

Assisted-by: Gemini:Gemini-Next
Signed-off-by: Yosry Ahmed <yosry@kernel.org>
---
 .../selftests/kvm/include/x86/processor.h     | 31 +++++++++++-
 .../testing/selftests/kvm/lib/x86/processor.c |  2 +
 tools/testing/selftests/kvm/lib/x86/svm.c     | 49 +++++++++----------
 3 files changed, 54 insertions(+), 28 deletions(-)

diff --git a/tools/testing/selftests/kvm/include/x86/processor.h b/tools/testing/selftests/kvm/include/x86/processor.h
index b161174ece453..d7f1acaddd4ee 100644
--- a/tools/testing/selftests/kvm/include/x86/processor.h
+++ b/tools/testing/selftests/kvm/include/x86/processor.h
@@ -398,8 +398,7 @@ 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 {
+struct guest_regs {
 	u64 rax;
 	u64 rcx;
 	u64 rdx;
@@ -418,6 +417,34 @@ struct gpr64_regs {
 	u64 r15;
 };
 
+extern struct guest_regs guest_regs;
+
+#define GUEST_REG_OFFSET(name) \
+	[off_##name] "i" (offsetof(struct guest_regs, name))
+
+#define GUEST_REGS_OFFSETS	\
+	GUEST_REG_OFFSET(rax),	\
+	GUEST_REG_OFFSET(rcx),	\
+	GUEST_REG_OFFSET(rdx),	\
+	GUEST_REG_OFFSET(rbx),	\
+	GUEST_REG_OFFSET(rsp),	\
+	GUEST_REG_OFFSET(rbp),	\
+	GUEST_REG_OFFSET(rsi),	\
+	GUEST_REG_OFFSET(rdi),	\
+	GUEST_REG_OFFSET(r8),	\
+	GUEST_REG_OFFSET(r9),	\
+	GUEST_REG_OFFSET(r10),	\
+	GUEST_REG_OFFSET(r11),	\
+	GUEST_REG_OFFSET(r12),	\
+	GUEST_REG_OFFSET(r13),	\
+	GUEST_REG_OFFSET(r14),	\
+	GUEST_REG_OFFSET(r15)
+
+#define GUEST_REG(name) "guest_regs + %c[off_" #name "]"
+
+#define GUEST_SWITCH_GPR_ASM(name) \
+	"xchg %%" #name ", " GUEST_REG(name) "\n\t"
+
 struct desc64 {
 	u16 limit0;
 	u16 base0;
diff --git a/tools/testing/selftests/kvm/lib/x86/processor.c b/tools/testing/selftests/kvm/lib/x86/processor.c
index ef56dcefe0119..1f9201590f5b3 100644
--- a/tools/testing/selftests/kvm/lib/x86/processor.c
+++ b/tools/testing/selftests/kvm/lib/x86/processor.c
@@ -29,6 +29,8 @@ bool host_cpu_is_amd_compatible;
 bool is_forced_emulation_enabled;
 u64 guest_tsc_khz;
 
+struct guest_regs guest_regs;
+
 const char *ex_str(int vector)
 {
 	switch (vector) {
diff --git a/tools/testing/selftests/kvm/lib/x86/svm.c b/tools/testing/selftests/kvm/lib/x86/svm.c
index 766d15f1d534a..7db38faded89e 100644
--- a/tools/testing/selftests/kvm/lib/x86/svm.c
+++ b/tools/testing/selftests/kvm/lib/x86/svm.c
@@ -13,7 +13,6 @@
 
 #define SEV_DEV_PATH "/dev/sev"
 
-struct gpr64_regs guest_regs;
 u64 rflags;
 
 /* Allocate memory regions for nested SVM tests.
@@ -137,27 +136,25 @@ void generic_svm_setup(struct svm_test_data *svm, void *guest_rip)
  * save/restore 64-bit general registers except rax, rip, rsp
  * which are directly handed through the VMCB guest processor state
  */
-#define SAVE_GPR_C				\
-	"xchg %%rbx, guest_regs+0x20\n\t"	\
-	"xchg %%rcx, guest_regs+0x10\n\t"	\
-	"xchg %%rdx, guest_regs+0x18\n\t"	\
-	"xchg %%rbp, guest_regs+0x30\n\t"	\
-	"xchg %%rsi, guest_regs+0x38\n\t"	\
-	"xchg %%rdi, guest_regs+0x40\n\t"	\
-	"xchg %%r8,  guest_regs+0x48\n\t"	\
-	"xchg %%r9,  guest_regs+0x50\n\t"	\
-	"xchg %%r10, guest_regs+0x58\n\t"	\
-	"xchg %%r11, guest_regs+0x60\n\t"	\
-	"xchg %%r12, guest_regs+0x68\n\t"	\
-	"xchg %%r13, guest_regs+0x70\n\t"	\
-	"xchg %%r14, guest_regs+0x78\n\t"	\
-	"xchg %%r15, guest_regs+0x80\n\t"
-
-#define LOAD_GPR_C      SAVE_GPR_C
+#define SVM_SWITCH_GPRS_ASM \
+	GUEST_SWITCH_GPR_ASM(rbx) \
+	GUEST_SWITCH_GPR_ASM(rcx) \
+	GUEST_SWITCH_GPR_ASM(rdx) \
+	GUEST_SWITCH_GPR_ASM(rbp) \
+	GUEST_SWITCH_GPR_ASM(rsi) \
+	GUEST_SWITCH_GPR_ASM(rdi) \
+	GUEST_SWITCH_GPR_ASM(r8)  \
+	GUEST_SWITCH_GPR_ASM(r9)  \
+	GUEST_SWITCH_GPR_ASM(r10) \
+	GUEST_SWITCH_GPR_ASM(r11) \
+	GUEST_SWITCH_GPR_ASM(r12) \
+	GUEST_SWITCH_GPR_ASM(r13) \
+	GUEST_SWITCH_GPR_ASM(r14) \
+	GUEST_SWITCH_GPR_ASM(r15)
 
 /*
  * selftests do not use interrupts so we dropped clgi/sti/cli/stgi
- * for now. registers involved in LOAD/SAVE_GPR_C are eventually
+ * for now. Registers involved in SVM_SWITCH_GPRS_ASM are eventually
  * unmodified so they do not need to be in the clobber list.
  */
 void run_guest(struct vmcb *vmcb, u64 vmcb_gpa)
@@ -166,20 +163,20 @@ void run_guest(struct vmcb *vmcb, u64 vmcb_gpa)
 		"vmload %[vmcb_gpa]\n\t"
 		"mov rflags, %%r15\n\t"
 		"mov %%r15, %[vmcb_rflags]\n\t"
-		"mov %[guest_regs_rax], %%r15\n\t"
+		"mov " GUEST_REG(rax) ", %%r15\n\t"
 		"mov %%r15, %[vmcb_rax]\n\t"
-		LOAD_GPR_C
+		SVM_SWITCH_GPRS_ASM
 		"vmrun %[vmcb_gpa]\n\t"
-		SAVE_GPR_C
+		SVM_SWITCH_GPRS_ASM
 		"mov %[vmcb_rflags], %%r15\n\t"
 		"mov %%r15, rflags\n\t"
 		"mov %[vmcb_rax], %%r15\n\t"	// rax
-		"mov %%r15, %[guest_regs_rax]\n\t"
+		"mov %%r15, " GUEST_REG(rax) "\n\t"
 		"vmsave %[vmcb_gpa]\n\t"
 		: [vmcb_rflags] "+m" (vmcb->save.rflags),
-		  [vmcb_rax] "+m" (vmcb->save.rax),
-		  [guest_regs_rax] "+rm" (guest_regs.rax)
-		: [vmcb_gpa] "a" (vmcb_gpa)
+		  [vmcb_rax] "+m" (vmcb->save.rax)
+		: [vmcb_gpa] "a" (vmcb_gpa),
+		  GUEST_REGS_OFFSETS
 		: "r15", "memory");
 }
 
-- 
2.55.0.229.g6434b31f56-goog