From: Sean Christopherson <seanjc@google.com>
To: Paolo Bonzini <pbonzini@redhat.com>
Cc: linux-kernel@vger.kernel.org, kvm@vger.kernel.org,
chang.seok.bae@intel.com, x86@kernel.org
Subject: Re: [PATCH 1/3] KVM: VMX: Macrofy GPR swapping in __vmx_vcpu_run()
Date: Thu, 14 May 2026 12:30:32 -0700 [thread overview]
Message-ID: <agYi2Lc7UbGiw_zL@google.com> (raw)
In-Reply-To: <20260513174948.726812-2-pbonzini@redhat.com>
On Wed, May 13, 2026, Paolo Bonzini wrote:
> From: "Chang S. Bae" <chang.seok.bae@intel.com>
>
> Convert the repeated register save/restore sequences into macros to
> simplify the VM entry code.
This is not simpler. Maybe it ends up being less verbose, but I don't see how
anyone can claim this is simpler. E.g. for people like me that aren't already
familiar with the .ifc directive, the R32_NUM macro is inscrutable. I can at
least suss out what e.g. LOAD_REGS and friends are doing, but claiming the code
is "simpler" is rather ridiculous.
> This can also make it easier to extend for additional registers.
For me, this needs to be the focal point of the changelog, and it needs to be
written with --verbose, because the impact of APX on the VM-Entry/VM-Exit code
isn't so obvious that a one-line "this makes future life easier" sufficiently
justifies the macro magic. And that's coming from someone that generally loves
macro magic :-)
> diff --git a/arch/x86/include/asm/kvm_vcpu_regs.h b/arch/x86/include/asm/kvm_vcpu_regs.h
> index 1af2cb59233b..7c12a1fd1ffc 100644
> --- a/arch/x86/include/asm/kvm_vcpu_regs.h
> +++ b/arch/x86/include/asm/kvm_vcpu_regs.h
> @@ -22,4 +22,126 @@
> #define __VCPU_REGS_R15 15
> #endif
>
> +#ifdef __ASSEMBLER__
> +
> +#define REG_NUM_INVALID 100
> +
> + .macro R32_NUM opd r32
> + \opd = REG_NUM_INVALID
> + .ifc \r32,%eax
> + \opd = __VCPU_REGS_EAX
> + .endif
> + .ifc \r32,%ecx
> + \opd = __VCPU_REGS_ECX
> + .endif
> + .ifc \r32,%edx
> + \opd = __VCPU_REGS_EDX
> + .endif
> + .ifc \r32,%ebx
> + \opd = __VCPU_REGS_EBX
> + .endif
> + .ifc \r32,%esp
> + \opd = __VCPU_REGS_ESP
> + .endif
> + .ifc \r32,%ebp
> + \opd = __VCPU_REGS_EBP
> + .endif
> + .ifc \r32,%esi
> + \opd = __VCPU_REGS_ESI
> + .endif
> + .ifc \r32,%edi
> + \opd = __VCPU_REGS_EDI
> + .endif
> +#ifdef CONFIG_X86_64
> + .ifc \r32,%r8d
> + \opd = 8
> + .endif
> + .ifc \r32,%r9d
> + \opd = 9
> + .endif
> + .ifc \r32,%r10d
> + \opd = 10
> + .endif
> + .ifc \r32,%r11d
> + \opd = 11
> + .endif
> + .ifc \r32,%r12d
> + \opd = 12
> + .endif
> + .ifc \r32,%r13d
> + \opd = 13
> + .endif
> + .ifc \r32,%r14d
> + \opd = 14
> + .endif
> + .ifc \r32,%r15d
> + \opd = 15
> + .endif
> +#endif
> + .endm
> +
> + .macro R64_NUM opd r64
> + \opd = REG_NUM_INVALID
> +#ifdef CONFIG_X86_64
> + .ifc \r64,%rax
> + \opd = __VCPU_REGS_RAX
> + .endif
> + .ifc \r64,%rcx
> + \opd = __VCPU_REGS_RCX
> + .endif
> + .ifc \r64,%rdx
> + \opd = __VCPU_REGS_RDX
> + .endif
> + .ifc \r64,%rbx
> + \opd = __VCPU_REGS_RBX
> + .endif
> + .ifc \r64,%rsp
> + \opd = __VCPU_REGS_RSP
> + .endif
> + .ifc \r64,%rbp
> + \opd = __VCPU_REGS_RBP
> + .endif
> + .ifc \r64,%rsi
> + \opd = __VCPU_REGS_RSI
> + .endif
> + .ifc \r64,%rdi
> + \opd = __VCPU_REGS_RDI
> + .endif
> + .ifc \r64,%r8
> + \opd = 8
> + .endif
> + .ifc \r64,%r9
> + \opd = 9
> + .endif
> + .ifc \r64,%r10
> + \opd = 10
> + .endif
> + .ifc \r64,%r11
> + \opd = 11
> + .endif
> + .ifc \r64,%r12
> + \opd = 12
> + .endif
> + .ifc \r64,%r13
> + \opd = 13
> + .endif
> + .ifc \r64,%r14
> + \opd = 14
> + .endif
> + .ifc \r64,%r15
> + \opd = 15
> + .endif
> +#endif
> + .endm
> +
> +.macro REG_NUM reg_num reg
> +#ifdef CONFIG_X86_64
> + R64_NUM \reg_num \reg
> +#else
> + R32_NUM \reg_num \reg
> +#endif
> +.endm
> +
> +#endif
> +
> #endif /* _ASM_X86_KVM_VCPU_REGS_H */
> diff --git a/arch/x86/kvm/svm/vmenter.S b/arch/x86/kvm/svm/vmenter.S
> index f523d9e49839..6a91e1383e8f 100644
> --- a/arch/x86/kvm/svm/vmenter.S
> +++ b/arch/x86/kvm/svm/vmenter.S
> @@ -4,13 +4,10 @@
> #include <asm/asm-offsets.h>
> #include <asm/bitsperlong.h>
> #include <asm/frame.h>
> -#include <asm/kvm_vcpu_regs.h>
> #include <asm/nospec-branch.h>
> #include "kvm-asm-offsets.h"
> #include "vmenter.h"
>
> -#define WORD_SIZE (BITS_PER_LONG / 8)
> -
> /* Intentionally omit RAX as it's context switched by hardware */
> #define VCPU_RCX (SVM_vcpu_arch_regs + __VCPU_REGS_RCX * WORD_SIZE)
> #define VCPU_RDX (SVM_vcpu_arch_regs + __VCPU_REGS_RDX * WORD_SIZE)
> diff --git a/arch/x86/kvm/vmenter.h b/arch/x86/kvm/vmenter.h
> index ba3f71449c62..44574806cb02 100644
> --- a/arch/x86/kvm/vmenter.h
> +++ b/arch/x86/kvm/vmenter.h
> @@ -2,6 +2,8 @@
> #ifndef __KVM_X86_VMENTER_H
> #define __KVM_X86_VMENTER_H
>
> +#include <asm/kvm_vcpu_regs.h>
> +
> #define KVM_ENTER_VMRESUME BIT(0)
> #define KVM_ENTER_SAVE_SPEC_CTRL BIT(1)
> #define KVM_ENTER_CLEAR_CPU_BUFFERS_FOR_MMIO BIT(2)
> @@ -76,5 +78,46 @@
> wrmsr
> .endm
>
> +#define WORD_SIZE (BITS_PER_LONG / 8)
> +
> +.macro LOAD_REGS src:req, regs_ofs:req, regs:vararg
> +.irp reg, \regs
> + REG_NUM reg_num \reg
> + .if reg_num <> REG_NUM_INVALID
> + mov (\regs_ofs + reg_num * WORD_SIZE)(\src), \reg
> + .else
> + .err invalid register \reg
> + .endif
> +.endr
> +.endm
> +
> +.macro STORE_REGS dst:req, regs_ofs:req, regs:vararg
> +.irp reg, \regs
> + REG_NUM reg_num \reg
> + .if reg_num <> REG_NUM_INVALID
> + mov \reg, (\regs_ofs + reg_num * WORD_SIZE)(\dst)
> + .else
> + .err invalid register \reg
> + .endif
> +.endr
> +.endm
> +
> +.macro POP_REGS dst:req, regs_ofs:req, regs:vararg
> +.irp reg, \regs
> + REG_NUM reg_num \reg
> + .if reg_num <> REG_NUM_INVALID
> + pop (\regs_ofs + reg_num * WORD_SIZE)(\dst)
> + .else
> + .err invalid register \reg
> + .endif
> +.endr
> +.endm
> +
> +.macro CLEAR_REGS regs:vararg
> +.irp reg, \regs
> + xorl \reg, \reg
> +.endr
> +.endm
> +
> #endif /* __ASSEMBLER__ */
> #endif /* __KVM_X86_VMENTER_H */
> diff --git a/arch/x86/kvm/vmx/vmenter.S b/arch/x86/kvm/vmx/vmenter.S
> index 7e4dc17fc0b8..4b7aaa7430fb 100644
> --- a/arch/x86/kvm/vmx/vmenter.S
> +++ b/arch/x86/kvm/vmx/vmenter.S
> @@ -2,35 +2,12 @@
> #include <linux/linkage.h>
> #include <asm/asm.h>
> #include <asm/bitsperlong.h>
> -#include <asm/kvm_vcpu_regs.h>
> #include <asm/nospec-branch.h>
> #include <asm/percpu.h>
> #include <asm/segment.h>
> #include "kvm-asm-offsets.h"
> #include "vmenter.h"
>
> -#define WORD_SIZE (BITS_PER_LONG / 8)
> -
> -#define VCPU_RAX (VMX_vcpu_arch_regs + __VCPU_REGS_RAX * WORD_SIZE)
> -#define VCPU_RCX (VMX_vcpu_arch_regs + __VCPU_REGS_RCX * WORD_SIZE)
> -#define VCPU_RDX (VMX_vcpu_arch_regs + __VCPU_REGS_RDX * WORD_SIZE)
> -#define VCPU_RBX (VMX_vcpu_arch_regs + __VCPU_REGS_RBX * WORD_SIZE)
> -/* Intentionally omit RSP as it's context switched by hardware */
> -#define VCPU_RBP (VMX_vcpu_arch_regs + __VCPU_REGS_RBP * WORD_SIZE)
> -#define VCPU_RSI (VMX_vcpu_arch_regs + __VCPU_REGS_RSI * WORD_SIZE)
> -#define VCPU_RDI (VMX_vcpu_arch_regs + __VCPU_REGS_RDI * WORD_SIZE)
> -
> -#ifdef CONFIG_X86_64
> -#define VCPU_R8 (VMX_vcpu_arch_regs + __VCPU_REGS_R8 * WORD_SIZE)
> -#define VCPU_R9 (VMX_vcpu_arch_regs + __VCPU_REGS_R9 * WORD_SIZE)
> -#define VCPU_R10 (VMX_vcpu_arch_regs + __VCPU_REGS_R10 * WORD_SIZE)
> -#define VCPU_R11 (VMX_vcpu_arch_regs + __VCPU_REGS_R11 * WORD_SIZE)
> -#define VCPU_R12 (VMX_vcpu_arch_regs + __VCPU_REGS_R12 * WORD_SIZE)
> -#define VCPU_R13 (VMX_vcpu_arch_regs + __VCPU_REGS_R13 * WORD_SIZE)
> -#define VCPU_R14 (VMX_vcpu_arch_regs + __VCPU_REGS_R14 * WORD_SIZE)
> -#define VCPU_R15 (VMX_vcpu_arch_regs + __VCPU_REGS_R15 * WORD_SIZE)
> -#endif
> -
> .macro VMX_DO_EVENT_IRQOFF call_insn call_target
> /*
> * Unconditionally create a stack frame, getting the correct RSP on the
> @@ -114,25 +91,18 @@ SYM_FUNC_START(__vmx_vcpu_run)
> * an LFENCE to stop speculation from skipping the wrmsr.
> */
>
> - /* Load guest registers. Don't clobber flags. */
> - mov VCPU_RAX(%_ASM_DI), %_ASM_AX
> - mov VCPU_RCX(%_ASM_DI), %_ASM_CX
> - mov VCPU_RDX(%_ASM_DI), %_ASM_DX
> - mov VCPU_RBX(%_ASM_DI), %_ASM_BX
> - mov VCPU_RBP(%_ASM_DI), %_ASM_BP
> - mov VCPU_RSI(%_ASM_DI), %_ASM_SI
> + /*
> + * Load guest registers. Don't clobber flags. Intentionally omit
> + * %_ASM_SP as it's context switched by hardware
> + */
> + LOAD_REGS %_ASM_DI, VMX_vcpu_arch_regs, \
> + %_ASM_AX, %_ASM_CX, %_ASM_DX, %_ASM_BX, %_ASM_BP, %_ASM_SI
> #ifdef CONFIG_X86_64
> - mov VCPU_R8 (%_ASM_DI), %r8
> - mov VCPU_R9 (%_ASM_DI), %r9
> - mov VCPU_R10(%_ASM_DI), %r10
> - mov VCPU_R11(%_ASM_DI), %r11
> - mov VCPU_R12(%_ASM_DI), %r12
> - mov VCPU_R13(%_ASM_DI), %r13
> - mov VCPU_R14(%_ASM_DI), %r14
> - mov VCPU_R15(%_ASM_DI), %r15
> + LOAD_REGS %_ASM_DI, VMX_vcpu_arch_regs, \
> + %r8, %r9, %r10, %r11, %r12, %r13, %r14, %r15
> #endif
> /* Load guest RDI. This kills the @vmx pointer! */
> - mov VCPU_RDI(%_ASM_DI), %_ASM_DI
> + LOAD_REGS %_ASM_DI, VMX_vcpu_arch_regs, %_ASM_DI
>
> /*
> * Note, ALTERNATIVE_2 works in reverse order. If CLEAR_CPU_BUF_VM is
> @@ -187,23 +157,16 @@ SYM_INNER_LABEL_ALIGN(vmx_vmexit, SYM_L_GLOBAL)
> /* Reload @vmx to RDI. */
> mov 2*WORD_SIZE(%_ASM_SP), %_ASM_DI
>
> - /* Save all guest registers, including RDI from the stack */
> - mov %_ASM_AX, VCPU_RAX(%_ASM_DI)
> - mov %_ASM_CX, VCPU_RCX(%_ASM_DI)
> - mov %_ASM_DX, VCPU_RDX(%_ASM_DI)
> - mov %_ASM_BX, VCPU_RBX(%_ASM_DI)
> - mov %_ASM_BP, VCPU_RBP(%_ASM_DI)
> - mov %_ASM_SI, VCPU_RSI(%_ASM_DI)
> - pop VCPU_RDI(%_ASM_DI)
> + /*
> + * Save all guest registers, including RDI from the stack. Intentionally
> + * omit %_ASM_SP as it's context switched by hardware
> + */
> + STORE_REGS %_ASM_DI, VMX_vcpu_arch_regs, \
> + %_ASM_AX, %_ASM_CX, %_ASM_DX, %_ASM_BX, %_ASM_BP, %_ASM_SI
> + POP_REGS %_ASM_DI, VMX_vcpu_arch_regs, %_ASM_DI
> #ifdef CONFIG_X86_64
> - mov %r8, VCPU_R8 (%_ASM_DI)
> - mov %r9, VCPU_R9 (%_ASM_DI)
> - mov %r10, VCPU_R10(%_ASM_DI)
> - mov %r11, VCPU_R11(%_ASM_DI)
> - mov %r12, VCPU_R12(%_ASM_DI)
> - mov %r13, VCPU_R13(%_ASM_DI)
> - mov %r14, VCPU_R14(%_ASM_DI)
> - mov %r15, VCPU_R15(%_ASM_DI)
> + STORE_REGS %_ASM_DI, VMX_vcpu_arch_regs, \
> + %r8, %r9, %r10, %r11, %r12, %r13, %r14, %r15
> #endif
>
> /* Clear return value to indicate VM-Exit (as opposed to VM-Fail). */
> @@ -220,21 +183,9 @@ SYM_INNER_LABEL_ALIGN(vmx_vmexit, SYM_L_GLOBAL)
> * VM-Exit and RBX is explicitly loaded with 0 or 1 to hold the return
> * value.
> */
> - xor %eax, %eax
> - xor %ecx, %ecx
> - xor %edx, %edx
> - xor %ebp, %ebp
> - xor %esi, %esi
> - xor %edi, %edi
> + CLEAR_REGS %eax, %ecx, %edx, %ebp, %esi, %edi
> #ifdef CONFIG_X86_64
> - xor %r8d, %r8d
> - xor %r9d, %r9d
> - xor %r10d, %r10d
> - xor %r11d, %r11d
> - xor %r12d, %r12d
> - xor %r13d, %r13d
> - xor %r14d, %r14d
> - xor %r15d, %r15d
> + CLEAR_REGS %r8d, %r9d, %r10d, %r11d, %r12d, %r13d, %r14d, %r15d
> #endif
>
> /*
> --
> 2.52.0
>
>
next prev parent reply other threads:[~2026-05-14 19:30 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-05-13 17:49 [PATCH 0/3] KVM: Macrofy GPR swapping in assembly code Paolo Bonzini
2026-05-13 17:49 ` [PATCH 1/3] KVM: VMX: Macrofy GPR swapping in __vmx_vcpu_run() Paolo Bonzini
2026-05-14 19:30 ` Sean Christopherson [this message]
2026-05-15 2:04 ` Chang S. Bae
2026-05-13 17:49 ` [PATCH 2/3] KVM: SVM: Macrofy GPR swapping in __svm_vcpu_run() Paolo Bonzini
2026-05-13 17:49 ` [PATCH 3/3] KVM: SEV: Macrofy GPR swapping in __svm_sev_es_vcpu_run() Paolo Bonzini
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=agYi2Lc7UbGiw_zL@google.com \
--to=seanjc@google.com \
--cc=chang.seok.bae@intel.com \
--cc=kvm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=pbonzini@redhat.com \
--cc=x86@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox