Re: [PATCH v3 12/13] KVM: selftests: Dedup assembly code for VMLAUNCH and VMRESUME

[Sean Christopherson]

On Thu, Aug 27, 2026, Yosry Ahmed wrote:
> On Thu, Aug 27, 2026 at 9:41 AM Sean Christopherson <seanjc@xxxxxxxxxx> wrote:
> >   #define __BUILD_VMX_VM_ENTRY_HELPER(insn, prefix, vmwrite_insn, vmwrite_operand,      \
> >                                     __host_rsp, __host_rip)                             \
> 
> The only part I really hate is vmwrite_operand. Why do we need it?  Seems
> like all the current helpers use "r", I feel like I am missing something.

...

> >   static int __##prefix##_##insn(void)                                                  \
> >   {                                                                                     \
> >         int ret;                                                                        \
> >                                                                                         \
> >         __asm__ __volatile__("push $0;"                                                 \
> >                              __stringify(vmwrite_insn) " %%rsp, %[host_rsp];"           \
> >                              "lea 1f(%%rip), %%rax;"                                    \
> >                              __stringify(vmwrite_insn) " %%rax, %[host_rip];"           \
> >                              VMX_SWITCH_GPRS_ASM                                        \
> >                              __stringify(insn)";"                                       \
> >                              "incq (%%rsp);"                                            \
> >                              "1: ;"                                                     \
> >                              VMX_SWITCH_GPRS_ASM                                        \
> >                              "pop %%rax;"                                               \
> >                              : [ret]"=&a"(ret)                                          \
> >                              : [host_rsp]__stringify(vmwrite_operand)(__host_rsp),      \
> >                                [host_rip]__stringify(vmwrite_operand)(__host_rip),      \
> >                                GUEST_REGS_OFFSETS                                       \
> >                              : "memory", "cc");                                         \
> >         return ret;                                                                     \
> >   }
> >
> >   #define BUILD_VMX_VM_ENTRY_HELPER(insn) \
> >         __BUILD_VMX_VM_ENTRY_HELPER(insn, _, vmwrite, r, (u64)HOST_RSP, (u64)HOST_RIP)  \
> >         __BUILD_VMX_VM_ENTRY_HELPER(insn, __evmcs, mov, m,                              \
> >                                     current_evmcs->host_rsp, current_evmcs->host_rip)

eVMCS is a memory operand, i.e. it's MOV RSP, [evmcs->host_rip].  The existing
evmcs_{vmresume,vmlaunch}() code is rather stupid and loads the address into a
register, and then manually encodes MOV RSP, [<reg>].

P.S. just in case you didn't already think this code is evil, the '&' in the
     above "[ret]"=&a"(ret)" is an earlyclobber that tells the compiler that
     RAX may be clobbered before all inputs and processed, i.e. prevents the
     compiler from using RAX for VMWRITE's register operand, or from using RAX
     to compute the memory operand for eVMCS's MOV.

P.P.S. even though the prototype for VMWRITE is "VMWRITE r64, r/m64", Intel is
       ass-backwards when it comes to syntax, i.e r/m64 is the source, not the
       dest, and so the field encoding *must* be a register operand.  I.e. we
       can't do something like this:

diff --git tools/testing/selftests/kvm/lib/x86/vmx.c tools/testing/selftests/kvm/lib/x86/vmx.c
index 1a8515de42b0..b5efd10950c7 100644
--- tools/testing/selftests/kvm/lib/x86/vmx.c
+++ tools/testing/selftests/kvm/lib/x86/vmx.c
@@ -179,7 +179,10 @@ void load_vmcs(struct vmx_pages *vmx)
        vmclear(vmx->shadow_vmcs_gpa);
 }
 
-#define __BUILD_VMX_VM_ENTRY_HELPER(insn, prefix, vmwrite_insn, vmwrite_operand,       \
+const u64 HOST_RSP_ENCODING = HOST_RSP;
+const u64 HOST_RIP_ENCODING = HOST_RIP;
+
+#define __BUILD_VMX_VM_ENTRY_HELPER(insn, prefix, vmwrite_insn,        \
                                    __host_rsp, __host_rip)                             \
 static int __##prefix##_##insn(void)                                                   \
 {                                                                                      \
@@ -196,16 +199,17 @@ static int __##prefix##_##insn(void)                                                      \
                             VMX_SWITCH_GPRS_ASM                                        \
                             "pop %%rax;"                                               \
                             : [ret]"=&a"(ret)                                          \
-                            : [host_rsp]__stringify(vmwrite_operand)(__host_rsp),      \
-                              [host_rip]__stringify(vmwrite_operand)(__host_rip),      \
+                            : [host_rsp]"m"(__host_rsp),                               \
+                              [host_rip]"m"(__host_rip),                               \
                               GUEST_REGS_OFFSETS                                       \
                             : "memory", "cc");                                         \
        return ret;                                                                     \
 }
 
 #define BUILD_VMX_VM_ENTRY_HELPER(insn) \
-       __BUILD_VMX_VM_ENTRY_HELPER(insn, _, vmwrite, r, (u64)HOST_RSP, (u64)HOST_RIP)  \
-       __BUILD_VMX_VM_ENTRY_HELPER(insn, __evmcs, mov, m,                              \
+       __BUILD_VMX_VM_ENTRY_HELPER(insn, _, vmwrite,                                   \
+                                   HOST_RSP_ENCODING, HOST_RIP_ENCODING)               \
+       __BUILD_VMX_VM_ENTRY_HELPER(insn, __evmcs, mov,                                 \
                                    current_evmcs->host_rsp, current_evmcs->host_rip)
 
 BUILD_VMX_VM_ENTRY_HELPER(vmlaunch)