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

[Yosry Ahmed]

On Thu, Aug 27, 2026 at 9:41 AM Sean Christopherson <seanjc@xxxxxxxxxx> wrote:
>
> On Wed, Aug 26, 2026, Yosry Ahmed wrote:
> > On Wed, Aug 26, 2026 at 4:39 PM Sean Christopherson <seanjc@xxxxxxxxxx> wrote:
> > >
> > > Dedup the assembly code from VMLAUNCH vs. VMRESUME, the difference is
> > > literally only the actual VM-Enter instruction.
> >
> > You couldn't resist macrofiying this too.
>
> It's either that or throw a branch in the asm blob, which is surprisingly
> difficult because VMX_SWITCH_GPRS_ASM subtly clobbers RFLAGS and obviously
> clobbers GPRs.  Which basically leaves pushing the value on the stack, which is
> totally doable, but still annoying, and needs to be done immediately, before
> vmcs.HOST_RSP is loaded.  Branching at runtime also makes the generated code
> harder to read, e.g. when debugging, as it's not immediately obvious which
> instruction was actually attempted.
>
> That said, looking at this again made me realize evmcs_{vmlaunch,vmresume}() have
> the same core copy+paste mess.  Blech.  Macrofying the standard flows but not the
> eVMCS flows is rather silly.
>
> Inlining these blobs *twice* (VMCS vs. eVMCS) at every VM-Enter is also ridiculous.
> E.g. at a glance, it's responsible for something ~100k bytes of code in the compiled
> state_test.  While the code footprint of selftests isn't a priority, that's still
> absurd.
>
> So instead of macrofying everything, how about this?  Depending on how one feels
> about macro shenanigans, it's either horrific or amazing.  Or both.  But IMO it's
> worth eliminating all of the copy+paste, and it makes the control flow much easier
> to read, which in practice is likely what most people care about?  At least until
> the entry/exit sequence fails and they have to debug macro hell :-)
>
> vmx.c:
>
>
>   #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)
>
>   BUILD_VMX_VM_ENTRY_HELPER(vmlaunch)
>   BUILD_VMX_VM_ENTRY_HELPER(vmresume)
>
>   int __vmlaunch(void)
>   {
>         if (enable_evmcs) {
>                 current_evmcs->hv_clean_fields = 0;
>                 return ____evmcs_vmlaunch();
>         }
>
>         return ____vmlaunch();
>   }
>
>   int __vmresume(void)
>   {
>         if (enable_evmcs) {
>                 /* HOST_RIP */
>                 current_evmcs->hv_clean_fields &= ~HV_VMX_ENLIGHTENED_CLEAN_FIELD_HOST_GRP1;
>                 /* HOST_RSP */
>                 current_evmcs->hv_clean_fields &= ~HV_VMX_ENLIGHTENED_CLEAN_FIELD_HOST_POINTER;
>                 return ____evmcs_vmresume();
>         }
>
>         return ____vmresume();
>   }
>
> vmx.h:
>
>   int __vmlaunch(void);
>   int __vmresume(void);
>
>   static inline int vmlaunch(void)
>   {
>         return __vmlaunch();
>   }
>
>   static inline int vmresume(void)
>   {
>         return __vmresume();
>   }