[PATCH v3 00/13] KVM: nVMX: Adjust VMPTRLD/VMPTRST behavior with active eVMCS

[PATCH v3 00/13] KVM: nVMX: Adjust VMPTRLD/VMPTRST behavior with active eVMCS

[Sean Christopherson]

Vitaly's series to adjust KVM's emulation of VMPTRLD/VMPTRST when the vCPU
has an active eVMCS to match Hyper-V, and more importantly to not put the
vCPU into an infinite loop.

The patches from Vitaly remain the same from v2.  The churn I piled on top
came from wanting to use KVM_ASM_SAFE() for more VMX instructions to verify
the RFLAGS bug+fix Sashiko pointed out, and I lack impulse control when it
comes to cleanups.

v3:
 - See above.

v2:
 - https://lore.kernel.org/all/20260818165258.2613603-1-vkuznets@redhat.com
 - Fix build when !CONFIG_KVM_HYPERV [Sashiko]
 - Drop now-unneeded evmcs_vmptrst() [Sashiko]
 - Avoid hardcoding VMPTRLD insn length in hyperv_evmcs [Sashiko], use
   KVM_ASM_SAFE framework instead [Sean]

v1: https://lore.kernel.org/all/20260814162221.2144473-1-vkuznets@redhat.com

Sean Christopherson (9):
  KVM: selftests: Don't clobber RFLAGS in happy path of __KVM_ASM_SAFE()
  KVM: selftests: Assert success in vmptrst(), kill off vmptrstz()
  KVM: selftests: Always assert that vmxon() and
    prepare_for_vmx_operation() succeed
  KVM: selftests: Always assert that vmclear() succeeds
  KVM: selftests: Always assert that vmptrld() succeeds
  KVM: selftests: Drop useless return code from load_vmcs()
  KVM: selftests: Add macros to handle simple VMX instructions
  KVM: selftests: Dedup assembly code for VMLAUNCH and VMRESUME
  KVM: selftests: Add and use double-underscore versions of vmlaunch()
    and vmresume()

Vitaly Kuznetsov (4):
  KVM: nVMX: Make VMPTRLD result in #UD when eVMCS is used
  KVM: nVMX: Make VMPTRST return eVMCS GPA when it is used
  KVM: selftests: Adapt to the updated VMPTRST behavior when eVMCS is
    used
  KVM: selftests: Check VMPTRLD with active eVMCS

 arch/x86/kvm/vmx/nested.c                     |  24 ++-
 .../testing/selftests/kvm/include/x86/evmcs.h |   8 -
 .../selftests/kvm/include/x86/processor.h     |   2 +-
 tools/testing/selftests/kvm/include/x86/vmx.h | 173 +++++++-----------
 .../testing/selftests/kvm/lib/x86/memstress.c |   6 +-
 tools/testing/selftests/kvm/lib/x86/vmx.c     |  20 +-
 .../selftests/kvm/x86/aperfmperf_test.c       |   6 +-
 .../kvm/x86/evmcs_smm_controls_test.c         |   4 +-
 .../testing/selftests/kvm/x86/hyperv_evmcs.c  |  57 +++---
 .../selftests/kvm/x86/kvm_buslock_test.c      |   6 +-
 .../selftests/kvm/x86/nested_close_kvm_test.c |   6 +-
 .../selftests/kvm/x86/nested_dirty_log_test.c |   6 +-
 .../selftests/kvm/x86/nested_emulation_test.c |   6 +-
 .../kvm/x86/nested_exceptions_test.c          |   6 +-
 .../kvm/x86/nested_invalid_cr3_test.c         |   8 +-
 .../selftests/kvm/x86/nested_tdp_fault_test.c |   6 +-
 .../kvm/x86/nested_tsc_adjust_test.c          |   6 +-
 .../kvm/x86/nested_tsc_scaling_test.c         |   6 +-
 .../kvm/x86/save_restore_pf_stress_test.c     |   8 +-
 tools/testing/selftests/kvm/x86/smm_test.c    |   4 +-
 tools/testing/selftests/kvm/x86/state_test.c  |  42 ++---
 .../kvm/x86/triple_fault_event_test.c         |   6 +-
 .../selftests/kvm/x86/vmx_apic_access_test.c  |   8 +-
 .../kvm/x86/vmx_apicv_updates_test.c          |   8 +-
 .../kvm/x86/vmx_invalid_nested_guest_state.c  |   6 +-
 .../kvm/x86/vmx_nested_la57_state_test.c      |   6 +-
 .../kvm/x86/vmx_preemption_timer_test.c       |  10 +-
 27 files changed, 214 insertions(+), 240 deletions(-)


base-commit: 76671054f9a1ff6abb976583cd8da37650acdc97
-- 
2.55.0.887.g758fc8c411-goog

[PATCH v3 01/13] KVM: nVMX: Make VMPTRLD result in #UD when eVMCS is used

[Sean Christopherson]

From: Vitaly Kuznetsov <vkuznets@redhat.com>

VMPTRLD with active eVMCS is already forbidden, however, returning 1
without skipping the instruction or queuing an exception will likely
result in L1 getting stuck. Genuine Hyper-V seems to inject #UD under
similar circumstances, mimic the behavior in KVM.

Reported-by: f734222792@gmail.com
Link: https://bugzilla.kernel.org/show_bug.cgi?id=221841
Suggested-by: Sean Christopherson <seanjc@google.com>
Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/kvm/vmx/nested.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c
index 151873407abd..ad3726fd4fda 100644
--- a/arch/x86/kvm/vmx/nested.c
+++ b/arch/x86/kvm/vmx/nested.c
@@ -5897,6 +5897,12 @@ static int handle_vmptrld(struct kvm_vcpu *vcpu)
 	if (!nested_vmx_check_permission(vcpu))
 		return 1;
 
+	/* Forbid normal VMPTRLD if Enlightened version was used */
+	if (nested_vmx_is_evmptr12_valid(vmx)) {
+		kvm_queue_exception(vcpu, UD_VECTOR);
+		return 1;
+	}
+
 	if (nested_vmx_get_vmptr(vcpu, &vmptr, &r))
 		return r;
 
@@ -5906,10 +5912,6 @@ static int handle_vmptrld(struct kvm_vcpu *vcpu)
 	if (vmptr == vmx->nested.vmxon_ptr)
 		return nested_vmx_fail(vcpu, VMXERR_VMPTRLD_VMXON_POINTER);
 
-	/* Forbid normal VMPTRLD if Enlightened version was used */
-	if (nested_vmx_is_evmptr12_valid(vmx))
-		return 1;
-
 	if (vmx->nested.current_vmptr != vmptr) {
 		struct gfn_to_hva_cache *ghc = &vmx->nested.vmcs12_cache;
 		struct vmcs_hdr hdr;
-- 
2.55.0.887.g758fc8c411-goog

[PATCH v3 02/13] KVM: nVMX: Make VMPTRST return eVMCS GPA when it is used

[Sean Christopherson]

From: Vitaly Kuznetsov <vkuznets@redhat.com>

VMPTRST with active eVMCS is currently forbidden, however, returning 1
without skipping the instruction will likely result in L1 getting
stuck. While TLFS does not specify the expected behavior, genuine Hyper-V
seems to be returning eVMCS GPA. Implement the same behavior in KVM.

Reported-by: f734222792@gmail.com
Link: https://bugzilla.kernel.org/show_bug.cgi?id=221841
Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/kvm/vmx/nested.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c
index ad3726fd4fda..3058efa65c82 100644
--- a/arch/x86/kvm/vmx/nested.c
+++ b/arch/x86/kvm/vmx/nested.c
@@ -5964,7 +5964,7 @@ static int handle_vmptrst(struct kvm_vcpu *vcpu)
 {
 	unsigned long exit_qual = vmx_get_exit_qual(vcpu);
 	u32 instr_info = vmcs_read32(VMX_INSTRUCTION_INFO);
-	gpa_t current_vmptr = to_vmx(vcpu)->nested.current_vmptr;
+	gpa_t current_vmptr;
 	struct x86_exception e;
 	gva_t gva;
 	int r;
@@ -5972,8 +5972,16 @@ static int handle_vmptrst(struct kvm_vcpu *vcpu)
 	if (!nested_vmx_check_permission(vcpu))
 		return 1;
 
-	if (unlikely(nested_vmx_is_evmptr12_valid(to_vmx(vcpu))))
-		return 1;
+	/*
+	 * Hyper-V TLFS does not specify the behavior of VMPTRST when eVMCS is used
+	 * but genuine Hyper-V seems to be returning eVMCS GPA.
+	 */
+#ifdef CONFIG_KVM_HYPERV
+	if (nested_vmx_is_evmptr12_valid(to_vmx(vcpu)))
+		current_vmptr = to_vmx(vcpu)->nested.hv_evmcs_vmptr;
+	else
+#endif
+		current_vmptr = to_vmx(vcpu)->nested.current_vmptr;
 
 	if (get_vmx_mem_address(vcpu, exit_qual, instr_info,
 				true, sizeof(gpa_t), &gva))
-- 
2.55.0.887.g758fc8c411-goog

[PATCH v3 03/13] KVM: selftests: Don't clobber RFLAGS in happy path of __KVM_ASM_SAFE()

[Sean Christopherson]

Use a straight MOV instead of an XOR to zero R9 in the non-faulting path of
__KVM_ASM_SAFE() so as not to clobber RFLAGS.  XOR generates smaller code,
but optimizing performance by minimizing the code footprint is just about
the last thing that KVM selftests care about.

The bug is relatively benign in the current code base, e.g. it marginally
degrades the coverage in the fastops test, but otherwise does no harm.  But
the bug will become actively problematic if/when the "safe assembly" macros
are used for instructions that communicate results via RFLAGS, e.g. for VMX
instructions, which signal failure via RFLAGS.CF and RFLAGS.ZF.

Opportunistically use R9D instead of the full R9, as bits 63:32 of GPRs are
zeroed on writes to bit 31:0.

Reported-by: Sashiko Bot <sashiko-bot@kernel.org>
Closes: https://lore.kernel.org/all/20260818165258.2613603-5-vkuznets@redhat.com
Cc: Vitaly Kuznetsov <vkuznets@redhat.com>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 tools/testing/selftests/kvm/include/x86/processor.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/testing/selftests/kvm/include/x86/processor.h b/tools/testing/selftests/kvm/include/x86/processor.h
index 6e6f70035508..282b8aaff799 100644
--- a/tools/testing/selftests/kvm/include/x86/processor.h
+++ b/tools/testing/selftests/kvm/include/x86/processor.h
@@ -1318,7 +1318,7 @@ gva_t vm_alloc_stack(struct kvm_vm *vm, int nr_pages);
 	"lea 1f(%%rip), %%r10\n\t"				\
 	"lea 2f(%%rip), %%r11\n\t"				\
 	fep "1: " insn "\n\t"					\
-	"xor %%r9, %%r9\n\t"					\
+	"mov $0, %%r9d\n\t"					\
 	"2:\n\t"						\
 	"mov  %%r9b, %[vector]\n\t"				\
 	"mov  %%r10, %[error_code]\n\t"
-- 
2.55.0.887.g758fc8c411-goog

[PATCH v3 04/13] KVM: selftests: Adapt to the updated VMPTRST behavior when eVMCS is used

[Sean Christopherson]

From: Vitaly Kuznetsov <vkuznets@redhat.com>

Previously, VMPTRST was forbidden with eVMCS and selftests were mocking the
correct behavior in vmptrst() by returning enlightened vmptr directly.
Since KVM's behavior has changed to match genuine Hyper-V, adjust evmcs
test accordingly.

Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 tools/testing/selftests/kvm/include/x86/evmcs.h | 8 --------
 tools/testing/selftests/kvm/include/x86/vmx.h   | 3 ---
 tools/testing/selftests/kvm/x86/hyperv_evmcs.c  | 5 ++---
 3 files changed, 2 insertions(+), 14 deletions(-)

diff --git a/tools/testing/selftests/kvm/include/x86/evmcs.h b/tools/testing/selftests/kvm/include/x86/evmcs.h
index 82a8ea6b661f..c5b9d26f6cdb 100644
--- a/tools/testing/selftests/kvm/include/x86/evmcs.h
+++ b/tools/testing/selftests/kvm/include/x86/evmcs.h
@@ -265,14 +265,6 @@ static inline bool load_evmcs(struct hyperv_test_pages *hv)
 	return true;
 }
 
-static inline int evmcs_vmptrst(u64 *value)
-{
-	*value = current_vp_assist->current_nested_vmcs &
-		~HV_X64_MSR_VP_ASSIST_PAGE_ENABLE;
-
-	return 0;
-}
-
 static inline int evmcs_vmread(u64 encoding, u64 *value)
 {
 	switch (encoding) {
diff --git a/tools/testing/selftests/kvm/include/x86/vmx.h b/tools/testing/selftests/kvm/include/x86/vmx.h
index 04f5e34dea3a..ac7d48d1387e 100644
--- a/tools/testing/selftests/kvm/include/x86/vmx.h
+++ b/tools/testing/selftests/kvm/include/x86/vmx.h
@@ -358,9 +358,6 @@ static inline int vmptrst(u64 *value)
 	u64 tmp;
 	u8 ret;
 
-	if (enable_evmcs)
-		return evmcs_vmptrst(value);
-
 	__asm__ __volatile__("vmptrst %[value]; setna %[ret]"
 		: [value]"=m"(tmp), [ret]"=rm"(ret)
 		: : "cc", "memory");
diff --git a/tools/testing/selftests/kvm/x86/hyperv_evmcs.c b/tools/testing/selftests/kvm/x86/hyperv_evmcs.c
index 63ea1533e4ea..6224d859d7bb 100644
--- a/tools/testing/selftests/kvm/x86/hyperv_evmcs.c
+++ b/tools/testing/selftests/kvm/x86/hyperv_evmcs.c
@@ -92,15 +92,14 @@ void guest_code(struct vmx_pages *vmx_pages, struct hyperv_test_pages *hv_pages,
 	GUEST_ASSERT(prepare_for_vmx_operation(vmx_pages));
 	GUEST_SYNC(3);
 	GUEST_ASSERT(load_evmcs(hv_pages));
-	GUEST_ASSERT(vmptrstz() == hv_pages->enlightened_vmcs_gpa);
+	/* VMPTRST returns -1 until VMLAUNCH with eVMCS ptr set */
+	GUEST_ASSERT(vmptrstz() == -1);
 
 	GUEST_SYNC(4);
-	GUEST_ASSERT(vmptrstz() == hv_pages->enlightened_vmcs_gpa);
 
 	prepare_vmcs(vmx_pages, l2_guest_code);
 
 	GUEST_SYNC(5);
-	GUEST_ASSERT(vmptrstz() == hv_pages->enlightened_vmcs_gpa);
 	current_evmcs->revision_id = -1u;
 	GUEST_ASSERT(vmlaunch());
 	current_evmcs->revision_id = EVMCS_VERSION;
-- 
2.55.0.887.g758fc8c411-goog

[PATCH v3 05/13] KVM: selftests: Check VMPTRLD with active eVMCS

[Sean Christopherson]

From: Vitaly Kuznetsov <vkuznets@redhat.com>

Check that VMPTRLD when eVMCS is active results in #UD. This matches
genuine Hyper-V's behavior.

Use KVM_ASM_SAFE framework to handle #UD from VMPTRLD.  Unfortunately, the
same trick cannot be applied to the existing #UD check on VMLAUNCH as
VMLAUNCH clobbers all registers which KVM_ASM_SAFE depends on.  Keep
VMLAUNCH handling separately.  Deliberately use a double-underscores
prefix instead of a "safe" postfix for the helper, to communicate that the
inner/outer helpers handle more than just exceptions, they also handle
VM-Fail conditions.

Assisted-by: Claude:claude-sonnet-4-6
Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
[sean: use __ instead of _safe]
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 tools/testing/selftests/kvm/include/x86/vmx.h | 19 +++++++++++---
 .../testing/selftests/kvm/x86/hyperv_evmcs.c  | 26 +++++++++++++++----
 2 files changed, 37 insertions(+), 8 deletions(-)

diff --git a/tools/testing/selftests/kvm/include/x86/vmx.h b/tools/testing/selftests/kvm/include/x86/vmx.h
index ac7d48d1387e..a161dbee7e04 100644
--- a/tools/testing/selftests/kvm/include/x86/vmx.h
+++ b/tools/testing/selftests/kvm/include/x86/vmx.h
@@ -342,9 +342,6 @@ static inline int vmptrld(u64 vmcs_pa)
 {
 	u8 ret;
 
-	if (enable_evmcs)
-		return -1;
-
 	__asm__ __volatile__ ("vmptrld %[pa]; setna %[ret]"
 		: [ret]"=rm"(ret)
 		: [pa]"m"(vmcs_pa)
@@ -353,6 +350,22 @@ static inline int vmptrld(u64 vmcs_pa)
 	return ret;
 }
 
+static inline int __vmptrld(u64 vmcs_pa)
+{
+	u64 error_code;
+	u8 vector;
+	u8 failed;
+
+	asm volatile(KVM_ASM_SAFE("vmptrld %[pa]")
+		     "\n\tsetna %[failed]"
+		     : KVM_ASM_SAFE_OUTPUTS(vector, error_code),
+		       [failed]"=qm"(failed)
+		     : [pa]"m"(vmcs_pa)
+		     : "cc", "memory", KVM_ASM_SAFE_CLOBBERS);
+
+	return vector ? vector : failed ? -EINVAL : 0;
+}
+
 static inline int vmptrst(u64 *value)
 {
 	u64 tmp;
diff --git a/tools/testing/selftests/kvm/x86/hyperv_evmcs.c b/tools/testing/selftests/kvm/x86/hyperv_evmcs.c
index 6224d859d7bb..f953a9755a76 100644
--- a/tools/testing/selftests/kvm/x86/hyperv_evmcs.c
+++ b/tools/testing/selftests/kvm/x86/hyperv_evmcs.c
@@ -23,7 +23,11 @@ static int ud_count;
 static void guest_ud_handler(struct ex_regs *regs)
 {
 	ud_count++;
-	regs->rip += 3; /* VMLAUNCH */
+	/*
+	 * VMLAUNCH insn can't be easily covered by KVM_ASM_SAFE framework but
+	 * luckily the instruction is always three bytes.
+	 */
+	regs->rip += 3;
 }
 
 static void guest_nmi_handler(struct ex_regs *regs)
@@ -178,7 +182,15 @@ void guest_code(struct vmx_pages *vmx_pages, struct hyperv_test_pages *hv_pages,
 	GUEST_ASSERT(vmreadz(VM_EXIT_REASON) == EXIT_REASON_VMCALL);
 	GUEST_SYNC(11);
 
-	/* Try enlightened vmptrld with an incorrect GPA */
+	/* VMPTRLD instruction causes #UD after enlightened VMLAUNCH */
+	GUEST_ASSERT(__vmptrld(hv_pages->enlightened_vmcs_gpa) == UD_VECTOR);
+
+	/*
+	 * Try enlightened vmptrld with an incorrect GPA. GUEST_SYNC(12) signals
+	 * the host to enable guest_ud_handler() which cannot be enabled beforehand
+	 * to not override the default fixup handler from KVM_ASM_SAFE().
+	 */
+	GUEST_SYNC(12);
 	evmcs_vmptrld(0xdeadbeef, hv_pages->enlightened_vmcs);
 	GUEST_ASSERT(vmlaunch());
 	GUEST_ASSERT(ud_count == 1);
@@ -252,7 +264,6 @@ int main(int argc, char *argv[])
 	vcpu_args_set(vcpu, 3, vmx_pages_gva, hv_pages_gva, addr_gva2gpa(vm, hcall_page));
 	vcpu_set_msr(vcpu, HV_X64_MSR_VP_INDEX, vcpu->id);
 
-	vm_install_exception_handler(vm, UD_VECTOR, guest_ud_handler);
 	vm_install_exception_handler(vm, NMI_VECTOR, guest_nmi_handler);
 
 	pr_info("Running L1 which uses EVMCS to run L2\n");
@@ -282,7 +293,7 @@ int main(int argc, char *argv[])
 
 		/* Force immediate L2->L1 exit before resuming */
 		if (stage == 8) {
-			pr_info("Injecting NMI into L1 before L2 had a chance to run after restore\n");
+			pr_debug("Injecting NMI into L1 before L2 had a chance to run after restore\n");
 			inject_nmi(vcpu);
 		}
 
@@ -292,9 +303,14 @@ int main(int argc, char *argv[])
 		 * KVM_STATE_NESTED_EVMCS is not lost.
 		 */
 		if (stage == 9) {
-			pr_info("Trying extra KVM_GET_NESTED_STATE/KVM_SET_NESTED_STATE cycle\n");
+			pr_debug("Trying extra KVM_GET_NESTED_STATE/KVM_SET_NESTED_STATE cycle\n");
 			vcpu = save_restore_vm(vm, vcpu);
 		}
+
+		if (stage == 12) {
+			pr_debug("Trying enlightened VMLAUNCH with an invalid PTR\n");
+			vm_install_exception_handler(vm, UD_VECTOR, guest_ud_handler);
+		}
 	}
 
 done:
-- 
2.55.0.887.g758fc8c411-goog

[PATCH v3 06/13] KVM: selftests: Assert success in vmptrst(), kill off vmptrstz()

[Sean Christopherson]

Assert success in vmptrst() instead of punting to the caller, as literally
every user expects VMPTRST to succeed (and because '0' is a valid physical
address).

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 tools/testing/selftests/kvm/include/x86/vmx.h | 19 ++++---------------
 .../testing/selftests/kvm/x86/hyperv_evmcs.c  |  4 ++--
 tools/testing/selftests/kvm/x86/state_test.c  |  8 ++++----
 .../kvm/x86/vmx_preemption_timer_test.c       |  2 +-
 4 files changed, 11 insertions(+), 22 deletions(-)

diff --git a/tools/testing/selftests/kvm/include/x86/vmx.h b/tools/testing/selftests/kvm/include/x86/vmx.h
index a161dbee7e04..04f638fcd3ba 100644
--- a/tools/testing/selftests/kvm/include/x86/vmx.h
+++ b/tools/testing/selftests/kvm/include/x86/vmx.h
@@ -366,27 +366,16 @@ static inline int __vmptrld(u64 vmcs_pa)
 	return vector ? vector : failed ? -EINVAL : 0;
 }
 
-static inline int vmptrst(u64 *value)
+static inline u64 vmptrst(void)
 {
-	u64 tmp;
+	u64 value = 0;
 	u8 ret;
 
 	__asm__ __volatile__("vmptrst %[value]; setna %[ret]"
-		: [value]"=m"(tmp), [ret]"=rm"(ret)
+		: [value]"=m"(value), [ret]"=rm"(ret)
 		: : "cc", "memory");
 
-	*value = tmp;
-	return ret;
-}
-
-/*
- * A wrapper around vmptrst that ignores errors and returns zero if the
- * vmptrst instruction fails.
- */
-static inline u64 vmptrstz(void)
-{
-	u64 value = 0;
-	vmptrst(&value);
+	__GUEST_ASSERT(!ret, "vmptrst failed");
 	return value;
 }
 
diff --git a/tools/testing/selftests/kvm/x86/hyperv_evmcs.c b/tools/testing/selftests/kvm/x86/hyperv_evmcs.c
index f953a9755a76..0c9cf620c87d 100644
--- a/tools/testing/selftests/kvm/x86/hyperv_evmcs.c
+++ b/tools/testing/selftests/kvm/x86/hyperv_evmcs.c
@@ -97,7 +97,7 @@ void guest_code(struct vmx_pages *vmx_pages, struct hyperv_test_pages *hv_pages,
 	GUEST_SYNC(3);
 	GUEST_ASSERT(load_evmcs(hv_pages));
 	/* VMPTRST returns -1 until VMLAUNCH with eVMCS ptr set */
-	GUEST_ASSERT(vmptrstz() == -1);
+	GUEST_ASSERT(vmptrst() == -1);
 
 	GUEST_SYNC(4);
 
@@ -123,7 +123,7 @@ void guest_code(struct vmx_pages *vmx_pages, struct hyperv_test_pages *hv_pages,
 	GUEST_ASSERT(!vmlaunch());
 	GUEST_ASSERT_EQ(vmreadz(VM_EXIT_REASON), EXIT_REASON_EXCEPTION_NMI);
 	GUEST_ASSERT_EQ((vmreadz(VM_EXIT_INTR_INFO) & 0xff), NMI_VECTOR);
-	GUEST_ASSERT(vmptrstz() == hv_pages->enlightened_vmcs_gpa);
+	GUEST_ASSERT(vmptrst() == hv_pages->enlightened_vmcs_gpa);
 
 	/*
 	 * NMI forces L2->L1 exit, resuming L2 and hope that EVMCS is
diff --git a/tools/testing/selftests/kvm/x86/state_test.c b/tools/testing/selftests/kvm/x86/state_test.c
index 4a1056a6cb8d..9d4e0c1f7c18 100644
--- a/tools/testing/selftests/kvm/x86/state_test.c
+++ b/tools/testing/selftests/kvm/x86/state_test.c
@@ -78,17 +78,17 @@ static void vmx_l1_guest_code(struct vmx_pages *vmx_pages)
 	GUEST_ASSERT(prepare_for_vmx_operation(vmx_pages));
 	GUEST_SYNC(3);
 	GUEST_ASSERT(load_vmcs(vmx_pages));
-	GUEST_ASSERT(vmptrstz() == vmx_pages->vmcs_gpa);
+	GUEST_ASSERT(vmptrst() == vmx_pages->vmcs_gpa);
 
 	GUEST_SYNC(4);
-	GUEST_ASSERT(vmptrstz() == vmx_pages->vmcs_gpa);
+	GUEST_ASSERT(vmptrst() == vmx_pages->vmcs_gpa);
 
 	prepare_vmcs(vmx_pages, vmx_l2_guest_code);
 
 	GUEST_SYNC(5);
-	GUEST_ASSERT(vmptrstz() == vmx_pages->vmcs_gpa);
+	GUEST_ASSERT(vmptrst() == vmx_pages->vmcs_gpa);
 	GUEST_ASSERT(!vmlaunch());
-	GUEST_ASSERT(vmptrstz() == vmx_pages->vmcs_gpa);
+	GUEST_ASSERT(vmptrst() == vmx_pages->vmcs_gpa);
 	GUEST_ASSERT(vmreadz(VM_EXIT_REASON) == EXIT_REASON_VMCALL);
 
 	/* Check that the launched state is preserved.  */
diff --git a/tools/testing/selftests/kvm/x86/vmx_preemption_timer_test.c b/tools/testing/selftests/kvm/x86/vmx_preemption_timer_test.c
index eb8021c33cd4..523aab667d85 100644
--- a/tools/testing/selftests/kvm/x86/vmx_preemption_timer_test.c
+++ b/tools/testing/selftests/kvm/x86/vmx_preemption_timer_test.c
@@ -73,7 +73,7 @@ void l1_guest_code(struct vmx_pages *vmx_pages)
 	GUEST_ASSERT(vmx_pages->vmcs_gpa);
 	GUEST_ASSERT(prepare_for_vmx_operation(vmx_pages));
 	GUEST_ASSERT(load_vmcs(vmx_pages));
-	GUEST_ASSERT(vmptrstz() == vmx_pages->vmcs_gpa);
+	GUEST_ASSERT(vmptrst() == vmx_pages->vmcs_gpa);
 
 	prepare_vmcs(vmx_pages, l2_guest_code);
 
-- 
2.55.0.887.g758fc8c411-goog

[PATCH v3 07/13] KVM: selftests: Always assert that vmxon() and prepare_for_vmx_operation() succeed

[Sean Christopherson]

Assert that vmxon() succeeds, and by extension prepare_for_vmx_operation().
Forcing every caller to assert that a core preparation helper succeeds is
absurd and generates hard-to-read code.

For all intents and purposes, no functional change intended.

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 tools/testing/selftests/kvm/include/x86/vmx.h              | 6 +++---
 tools/testing/selftests/kvm/lib/x86/memstress.c            | 2 +-
 tools/testing/selftests/kvm/lib/x86/vmx.c                  | 7 ++-----
 tools/testing/selftests/kvm/x86/aperfmperf_test.c          | 2 +-
 tools/testing/selftests/kvm/x86/evmcs_smm_controls_test.c  | 2 +-
 tools/testing/selftests/kvm/x86/hyperv_evmcs.c             | 2 +-
 tools/testing/selftests/kvm/x86/kvm_buslock_test.c         | 2 +-
 tools/testing/selftests/kvm/x86/nested_close_kvm_test.c    | 2 +-
 tools/testing/selftests/kvm/x86/nested_dirty_log_test.c    | 2 +-
 tools/testing/selftests/kvm/x86/nested_emulation_test.c    | 2 +-
 tools/testing/selftests/kvm/x86/nested_exceptions_test.c   | 2 +-
 tools/testing/selftests/kvm/x86/nested_invalid_cr3_test.c  | 2 +-
 tools/testing/selftests/kvm/x86/nested_tdp_fault_test.c    | 2 +-
 tools/testing/selftests/kvm/x86/nested_tsc_adjust_test.c   | 2 +-
 tools/testing/selftests/kvm/x86/nested_tsc_scaling_test.c  | 2 +-
 .../selftests/kvm/x86/save_restore_pf_stress_test.c        | 2 +-
 tools/testing/selftests/kvm/x86/smm_test.c                 | 2 +-
 tools/testing/selftests/kvm/x86/state_test.c               | 2 +-
 tools/testing/selftests/kvm/x86/triple_fault_event_test.c  | 2 +-
 tools/testing/selftests/kvm/x86/vmx_apic_access_test.c     | 2 +-
 tools/testing/selftests/kvm/x86/vmx_apicv_updates_test.c   | 2 +-
 .../selftests/kvm/x86/vmx_invalid_nested_guest_state.c     | 2 +-
 .../testing/selftests/kvm/x86/vmx_nested_la57_state_test.c | 2 +-
 .../testing/selftests/kvm/x86/vmx_preemption_timer_test.c  | 2 +-
 24 files changed, 27 insertions(+), 30 deletions(-)

diff --git a/tools/testing/selftests/kvm/include/x86/vmx.h b/tools/testing/selftests/kvm/include/x86/vmx.h
index 04f638fcd3ba..f7ca6646dfbe 100644
--- a/tools/testing/selftests/kvm/include/x86/vmx.h
+++ b/tools/testing/selftests/kvm/include/x86/vmx.h
@@ -309,7 +309,7 @@ struct vmx_msr_entry {
 
 #include "evmcs.h"
 
-static inline int vmxon(u64 phys)
+static inline void vmxon(u64 phys)
 {
 	u8 ret;
 
@@ -318,7 +318,7 @@ static inline int vmxon(u64 phys)
 		: [pa]"m"(phys)
 		: "cc", "memory");
 
-	return ret;
+	__GUEST_ASSERT(!ret, "vmxon [0x%lx] failed", phys);
 }
 
 static inline void vmxoff(void)
@@ -549,7 +549,7 @@ union vmx_ctrl_msr {
 };
 
 struct vmx_pages *vcpu_alloc_vmx(struct kvm_vm *vm, gva_t *p_vmx_gva);
-bool prepare_for_vmx_operation(struct vmx_pages *vmx);
+void prepare_for_vmx_operation(struct vmx_pages *vmx);
 void prepare_vmcs(struct vmx_pages *vmx, void *guest_rip);
 bool load_vmcs(struct vmx_pages *vmx);
 
diff --git a/tools/testing/selftests/kvm/lib/x86/memstress.c b/tools/testing/selftests/kvm/lib/x86/memstress.c
index e19e8b5a09c5..2fa39470c1b3 100644
--- a/tools/testing/selftests/kvm/lib/x86/memstress.c
+++ b/tools/testing/selftests/kvm/lib/x86/memstress.c
@@ -33,7 +33,7 @@ __asm__(
 static void l1_vmx_code(struct vmx_pages *vmx, u64 vcpu_id)
 {
 	GUEST_ASSERT(vmx->vmcs_gpa);
-	GUEST_ASSERT(prepare_for_vmx_operation(vmx));
+	prepare_for_vmx_operation(vmx);
 	GUEST_ASSERT(load_vmcs(vmx));
 	GUEST_ASSERT(ept_1g_pages_supported());
 
diff --git a/tools/testing/selftests/kvm/lib/x86/vmx.c b/tools/testing/selftests/kvm/lib/x86/vmx.c
index 089e1a8af53f..03d61b1159f8 100644
--- a/tools/testing/selftests/kvm/lib/x86/vmx.c
+++ b/tools/testing/selftests/kvm/lib/x86/vmx.c
@@ -125,7 +125,7 @@ vcpu_alloc_vmx(struct kvm_vm *vm, gva_t *p_vmx_gva)
 	return vmx;
 }
 
-bool prepare_for_vmx_operation(struct vmx_pages *vmx)
+void prepare_for_vmx_operation(struct vmx_pages *vmx)
 {
 	u64 feature_control;
 	u64 required;
@@ -163,10 +163,7 @@ bool prepare_for_vmx_operation(struct vmx_pages *vmx)
 
 	/* Enter VMX root operation. */
 	*(u32 *)(vmx->vmxon) = vmcs_revision();
-	if (vmxon(vmx->vmxon_gpa))
-		return false;
-
-	return true;
+	vmxon(vmx->vmxon_gpa);
 }
 
 bool load_vmcs(struct vmx_pages *vmx)
diff --git a/tools/testing/selftests/kvm/x86/aperfmperf_test.c b/tools/testing/selftests/kvm/x86/aperfmperf_test.c
index 845cb685f174..e32131cd6ed1 100644
--- a/tools/testing/selftests/kvm/x86/aperfmperf_test.c
+++ b/tools/testing/selftests/kvm/x86/aperfmperf_test.c
@@ -70,7 +70,7 @@ static void l1_svm_code(struct svm_test_data *svm)
 
 static void l1_vmx_code(struct vmx_pages *vmx)
 {
-	GUEST_ASSERT_EQ(prepare_for_vmx_operation(vmx), true);
+	prepare_for_vmx_operation(vmx);
 	GUEST_ASSERT_EQ(load_vmcs(vmx), true);
 
 	prepare_vmcs(vmx, NULL);
diff --git a/tools/testing/selftests/kvm/x86/evmcs_smm_controls_test.c b/tools/testing/selftests/kvm/x86/evmcs_smm_controls_test.c
index aa7f3b405fd3..cafb58a37023 100644
--- a/tools/testing/selftests/kvm/x86/evmcs_smm_controls_test.c
+++ b/tools/testing/selftests/kvm/x86/evmcs_smm_controls_test.c
@@ -55,7 +55,7 @@ static void guest_code(struct vmx_pages *vmx_pages,
 	enable_vp_assist(hv_pages->vp_assist_gpa, hv_pages->vp_assist);
 	evmcs_enable();
 
-	GUEST_ASSERT(prepare_for_vmx_operation(vmx_pages));
+	prepare_for_vmx_operation(vmx_pages);
 	GUEST_ASSERT(load_evmcs(hv_pages));
 	prepare_vmcs(vmx_pages, l2_guest_code);
 
diff --git a/tools/testing/selftests/kvm/x86/hyperv_evmcs.c b/tools/testing/selftests/kvm/x86/hyperv_evmcs.c
index 0c9cf620c87d..d0e857b75a47 100644
--- a/tools/testing/selftests/kvm/x86/hyperv_evmcs.c
+++ b/tools/testing/selftests/kvm/x86/hyperv_evmcs.c
@@ -93,7 +93,7 @@ void guest_code(struct vmx_pages *vmx_pages, struct hyperv_test_pages *hv_pages,
 	enable_vp_assist(hv_pages->vp_assist_gpa, hv_pages->vp_assist);
 	evmcs_enable();
 
-	GUEST_ASSERT(prepare_for_vmx_operation(vmx_pages));
+	prepare_for_vmx_operation(vmx_pages);
 	GUEST_SYNC(3);
 	GUEST_ASSERT(load_evmcs(hv_pages));
 	/* VMPTRST returns -1 until VMLAUNCH with eVMCS ptr set */
diff --git a/tools/testing/selftests/kvm/x86/kvm_buslock_test.c b/tools/testing/selftests/kvm/x86/kvm_buslock_test.c
index 25a182be00a9..2e006ce95896 100644
--- a/tools/testing/selftests/kvm/x86/kvm_buslock_test.c
+++ b/tools/testing/selftests/kvm/x86/kvm_buslock_test.c
@@ -42,7 +42,7 @@ static void l1_svm_code(struct svm_test_data *svm)
 
 static void l1_vmx_code(struct vmx_pages *vmx)
 {
-	GUEST_ASSERT_EQ(prepare_for_vmx_operation(vmx), true);
+	prepare_for_vmx_operation(vmx);
 	GUEST_ASSERT_EQ(load_vmcs(vmx), true);
 
 	prepare_vmcs(vmx, NULL);
diff --git a/tools/testing/selftests/kvm/x86/nested_close_kvm_test.c b/tools/testing/selftests/kvm/x86/nested_close_kvm_test.c
index b974cfb347d6..479e9adcc20b 100644
--- a/tools/testing/selftests/kvm/x86/nested_close_kvm_test.c
+++ b/tools/testing/selftests/kvm/x86/nested_close_kvm_test.c
@@ -30,7 +30,7 @@ static void l2_guest_code(void)
 
 static void l1_vmx_code(struct vmx_pages *vmx_pages)
 {
-	GUEST_ASSERT(prepare_for_vmx_operation(vmx_pages));
+	prepare_for_vmx_operation(vmx_pages);
 	GUEST_ASSERT(load_vmcs(vmx_pages));
 
 	/* Prepare the VMCS for L2 execution. */
diff --git a/tools/testing/selftests/kvm/x86/nested_dirty_log_test.c b/tools/testing/selftests/kvm/x86/nested_dirty_log_test.c
index 26b474bf1353..57f49471f4aa 100644
--- a/tools/testing/selftests/kvm/x86/nested_dirty_log_test.c
+++ b/tools/testing/selftests/kvm/x86/nested_dirty_log_test.c
@@ -93,7 +93,7 @@ void l1_vmx_code(struct vmx_pages *vmx)
 	void *l2_rip;
 
 	GUEST_ASSERT(vmx->vmcs_gpa);
-	GUEST_ASSERT(prepare_for_vmx_operation(vmx));
+	prepare_for_vmx_operation(vmx);
 	GUEST_ASSERT(load_vmcs(vmx));
 
 	if (vmx->eptp_gpa)
diff --git a/tools/testing/selftests/kvm/x86/nested_emulation_test.c b/tools/testing/selftests/kvm/x86/nested_emulation_test.c
index e08c6b0697e5..6f27930bb6b5 100644
--- a/tools/testing/selftests/kvm/x86/nested_emulation_test.c
+++ b/tools/testing/selftests/kvm/x86/nested_emulation_test.c
@@ -66,7 +66,7 @@ static void guest_code(void *test_data)
 					   BIT_ULL(INTERCEPT_HLT);
 		vmcb->control.intercept_exceptions = 0;
 	} else {
-		GUEST_ASSERT(prepare_for_vmx_operation(test_data));
+		prepare_for_vmx_operation(test_data);
 		GUEST_ASSERT(load_vmcs(test_data));
 
 		prepare_vmcs(test_data, NULL);
diff --git a/tools/testing/selftests/kvm/x86/nested_exceptions_test.c b/tools/testing/selftests/kvm/x86/nested_exceptions_test.c
index aeec3121c8e8..6a36cf4d1eab 100644
--- a/tools/testing/selftests/kvm/x86/nested_exceptions_test.c
+++ b/tools/testing/selftests/kvm/x86/nested_exceptions_test.c
@@ -125,7 +125,7 @@ static void vmx_run_l2(void *l2_code, int vector, u32 error_code)
 
 static void l1_vmx_code(struct vmx_pages *vmx)
 {
-	GUEST_ASSERT_EQ(prepare_for_vmx_operation(vmx), true);
+	prepare_for_vmx_operation(vmx);
 
 	GUEST_ASSERT_EQ(load_vmcs(vmx), true);
 
diff --git a/tools/testing/selftests/kvm/x86/nested_invalid_cr3_test.c b/tools/testing/selftests/kvm/x86/nested_invalid_cr3_test.c
index 8c2ba9674558..749a93c318fe 100644
--- a/tools/testing/selftests/kvm/x86/nested_invalid_cr3_test.c
+++ b/tools/testing/selftests/kvm/x86/nested_invalid_cr3_test.c
@@ -40,7 +40,7 @@ static void l1_vmx_code(struct vmx_pages *vmx_pages)
 {
 	uintptr_t save_cr3;
 
-	GUEST_ASSERT(prepare_for_vmx_operation(vmx_pages));
+	prepare_for_vmx_operation(vmx_pages);
 	GUEST_ASSERT(load_vmcs(vmx_pages));
 
 	prepare_vmcs(vmx_pages, l2_guest_code);
diff --git a/tools/testing/selftests/kvm/x86/nested_tdp_fault_test.c b/tools/testing/selftests/kvm/x86/nested_tdp_fault_test.c
index 2e04563790ff..b143ebe710e7 100644
--- a/tools/testing/selftests/kvm/x86/nested_tdp_fault_test.c
+++ b/tools/testing/selftests/kvm/x86/nested_tdp_fault_test.c
@@ -55,7 +55,7 @@ static void l1_vmx_code(struct vmx_pages *vmx, u64 expected_fault_gpa,
 	u64 exit_qual;
 
 	GUEST_ASSERT(vmx->vmcs_gpa);
-	GUEST_ASSERT(prepare_for_vmx_operation(vmx));
+	prepare_for_vmx_operation(vmx);
 	GUEST_ASSERT(load_vmcs(vmx));
 
 	prepare_vmcs(vmx, l2_entry);
diff --git a/tools/testing/selftests/kvm/x86/nested_tsc_adjust_test.c b/tools/testing/selftests/kvm/x86/nested_tsc_adjust_test.c
index cb79d7b9619c..3d2eae848ba6 100644
--- a/tools/testing/selftests/kvm/x86/nested_tsc_adjust_test.c
+++ b/tools/testing/selftests/kvm/x86/nested_tsc_adjust_test.c
@@ -86,7 +86,7 @@ static void l1_guest_code(void *data)
 		struct vmx_pages *vmx_pages = data;
 		u32 control;
 
-		GUEST_ASSERT(prepare_for_vmx_operation(vmx_pages));
+		prepare_for_vmx_operation(vmx_pages);
 		GUEST_ASSERT(load_vmcs(vmx_pages));
 
 		prepare_vmcs(vmx_pages, l2_guest_code);
diff --git a/tools/testing/selftests/kvm/x86/nested_tsc_scaling_test.c b/tools/testing/selftests/kvm/x86/nested_tsc_scaling_test.c
index 18f765835bf4..584071146b69 100644
--- a/tools/testing/selftests/kvm/x86/nested_tsc_scaling_test.c
+++ b/tools/testing/selftests/kvm/x86/nested_tsc_scaling_test.c
@@ -105,7 +105,7 @@ static void l1_vmx_code(struct vmx_pages *vmx_pages)
 	/* check that L1's frequency looks alright before launching L2 */
 	check_tsc_freq(UCHECK_L1);
 
-	GUEST_ASSERT(prepare_for_vmx_operation(vmx_pages));
+	prepare_for_vmx_operation(vmx_pages);
 	GUEST_ASSERT(load_vmcs(vmx_pages));
 
 	/* prepare the VMCS for L2 execution */
diff --git a/tools/testing/selftests/kvm/x86/save_restore_pf_stress_test.c b/tools/testing/selftests/kvm/x86/save_restore_pf_stress_test.c
index 507391ab2c93..6ab73a0bd34a 100644
--- a/tools/testing/selftests/kvm/x86/save_restore_pf_stress_test.c
+++ b/tools/testing/selftests/kvm/x86/save_restore_pf_stress_test.c
@@ -98,7 +98,7 @@ static void l1_svm_code(struct svm_test_data *svm)
 
 static void l1_vmx_code(struct vmx_pages *vmx)
 {
-	GUEST_ASSERT(prepare_for_vmx_operation(vmx));
+	prepare_for_vmx_operation(vmx);
 	GUEST_ASSERT(load_vmcs(vmx));
 	prepare_vmcs(vmx, guest_access_memory);
 
diff --git a/tools/testing/selftests/kvm/x86/smm_test.c b/tools/testing/selftests/kvm/x86/smm_test.c
index d1edafd5af75..be5662792ec8 100644
--- a/tools/testing/selftests/kvm/x86/smm_test.c
+++ b/tools/testing/selftests/kvm/x86/smm_test.c
@@ -78,7 +78,7 @@ static void guest_code(void *arg)
 		if (this_cpu_has(X86_FEATURE_SVM)) {
 			generic_svm_setup(svm, l2_guest_code);
 		} else {
-			GUEST_ASSERT(prepare_for_vmx_operation(vmx_pages));
+			prepare_for_vmx_operation(vmx_pages);
 			GUEST_ASSERT(load_vmcs(vmx_pages));
 			prepare_vmcs(vmx_pages, l2_guest_code);
 		}
diff --git a/tools/testing/selftests/kvm/x86/state_test.c b/tools/testing/selftests/kvm/x86/state_test.c
index 9d4e0c1f7c18..203584fcaf62 100644
--- a/tools/testing/selftests/kvm/x86/state_test.c
+++ b/tools/testing/selftests/kvm/x86/state_test.c
@@ -75,7 +75,7 @@ void vmx_l2_guest_code(void)
 static void vmx_l1_guest_code(struct vmx_pages *vmx_pages)
 {
 	GUEST_ASSERT(vmx_pages->vmcs_gpa);
-	GUEST_ASSERT(prepare_for_vmx_operation(vmx_pages));
+	prepare_for_vmx_operation(vmx_pages);
 	GUEST_SYNC(3);
 	GUEST_ASSERT(load_vmcs(vmx_pages));
 	GUEST_ASSERT(vmptrst() == vmx_pages->vmcs_gpa);
diff --git a/tools/testing/selftests/kvm/x86/triple_fault_event_test.c b/tools/testing/selftests/kvm/x86/triple_fault_event_test.c
index 0d83516f4bd0..57e79922b16b 100644
--- a/tools/testing/selftests/kvm/x86/triple_fault_event_test.c
+++ b/tools/testing/selftests/kvm/x86/triple_fault_event_test.c
@@ -25,7 +25,7 @@ void l1_guest_code_vmx(struct vmx_pages *vmx)
 {
 
 	GUEST_ASSERT(vmx->vmcs_gpa);
-	GUEST_ASSERT(prepare_for_vmx_operation(vmx));
+	prepare_for_vmx_operation(vmx);
 	GUEST_ASSERT(load_vmcs(vmx));
 
 	prepare_vmcs(vmx, l2_guest_code);
diff --git a/tools/testing/selftests/kvm/x86/vmx_apic_access_test.c b/tools/testing/selftests/kvm/x86/vmx_apic_access_test.c
index 463f73aa9159..7df9ab833cb2 100644
--- a/tools/testing/selftests/kvm/x86/vmx_apic_access_test.c
+++ b/tools/testing/selftests/kvm/x86/vmx_apic_access_test.c
@@ -38,7 +38,7 @@ static void l1_guest_code(struct vmx_pages *vmx_pages, unsigned long high_gpa)
 {
 	u32 control;
 
-	GUEST_ASSERT(prepare_for_vmx_operation(vmx_pages));
+	prepare_for_vmx_operation(vmx_pages);
 	GUEST_ASSERT(load_vmcs(vmx_pages));
 
 	/* Prepare the VMCS for L2 execution. */
diff --git a/tools/testing/selftests/kvm/x86/vmx_apicv_updates_test.c b/tools/testing/selftests/kvm/x86/vmx_apicv_updates_test.c
index f9b88a6f6113..8e4dc304533a 100644
--- a/tools/testing/selftests/kvm/x86/vmx_apicv_updates_test.c
+++ b/tools/testing/selftests/kvm/x86/vmx_apicv_updates_test.c
@@ -33,7 +33,7 @@ static void l1_guest_code(struct vmx_pages *vmx_pages)
 {
 	u32 control;
 
-	GUEST_ASSERT(prepare_for_vmx_operation(vmx_pages));
+	prepare_for_vmx_operation(vmx_pages);
 	GUEST_ASSERT(load_vmcs(vmx_pages));
 
 	/* Prepare the VMCS for L2 execution. */
diff --git a/tools/testing/selftests/kvm/x86/vmx_invalid_nested_guest_state.c b/tools/testing/selftests/kvm/x86/vmx_invalid_nested_guest_state.c
index c8379124b317..8999f0458aa1 100644
--- a/tools/testing/selftests/kvm/x86/vmx_invalid_nested_guest_state.c
+++ b/tools/testing/selftests/kvm/x86/vmx_invalid_nested_guest_state.c
@@ -41,7 +41,7 @@ static void l2_guest_code(void)
 
 static void l1_guest_code(struct vmx_pages *vmx_pages)
 {
-	GUEST_ASSERT(prepare_for_vmx_operation(vmx_pages));
+	prepare_for_vmx_operation(vmx_pages);
 	GUEST_ASSERT(load_vmcs(vmx_pages));
 
 	/* Prepare the VMCS for L2 execution. */
diff --git a/tools/testing/selftests/kvm/x86/vmx_nested_la57_state_test.c b/tools/testing/selftests/kvm/x86/vmx_nested_la57_state_test.c
index 75073efa926d..444596ab1592 100644
--- a/tools/testing/selftests/kvm/x86/vmx_nested_la57_state_test.c
+++ b/tools/testing/selftests/kvm/x86/vmx_nested_la57_state_test.c
@@ -37,7 +37,7 @@ static void l1_guest_code(struct vmx_pages *vmx_pages)
 	GUEST_ASSERT(rdmsr(MSR_GS_BASE) == LA57_GS_BASE);
 
 	GUEST_ASSERT(vmx_pages->vmcs_gpa);
-	GUEST_ASSERT(prepare_for_vmx_operation(vmx_pages));
+	prepare_for_vmx_operation(vmx_pages);
 	GUEST_ASSERT(load_vmcs(vmx_pages));
 
 	prepare_vmcs(vmx_pages, l2_guest_code);
diff --git a/tools/testing/selftests/kvm/x86/vmx_preemption_timer_test.c b/tools/testing/selftests/kvm/x86/vmx_preemption_timer_test.c
index 523aab667d85..09f9686657ad 100644
--- a/tools/testing/selftests/kvm/x86/vmx_preemption_timer_test.c
+++ b/tools/testing/selftests/kvm/x86/vmx_preemption_timer_test.c
@@ -71,7 +71,7 @@ void l1_guest_code(struct vmx_pages *vmx_pages)
 	u64 l1_tsc_deadline, l2_tsc_deadline;
 
 	GUEST_ASSERT(vmx_pages->vmcs_gpa);
-	GUEST_ASSERT(prepare_for_vmx_operation(vmx_pages));
+	prepare_for_vmx_operation(vmx_pages);
 	GUEST_ASSERT(load_vmcs(vmx_pages));
 	GUEST_ASSERT(vmptrst() == vmx_pages->vmcs_gpa);
 
-- 
2.55.0.887.g758fc8c411-goog

[PATCH v3 08/13] KVM: selftests: Always assert that vmclear() succeeds

[Sean Christopherson]

Continue removing dead error "handling", and assert that VMCLEAR
succeeds instead of forcing callers of load_vmcs() to assert success.

For all intents and purposes, no functional change intended.

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 tools/testing/selftests/kvm/include/x86/vmx.h | 4 ++--
 tools/testing/selftests/kvm/lib/x86/vmx.c     | 6 ++----
 2 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/tools/testing/selftests/kvm/include/x86/vmx.h b/tools/testing/selftests/kvm/include/x86/vmx.h
index f7ca6646dfbe..cf37a03a93b9 100644
--- a/tools/testing/selftests/kvm/include/x86/vmx.h
+++ b/tools/testing/selftests/kvm/include/x86/vmx.h
@@ -326,7 +326,7 @@ static inline void vmxoff(void)
 	__asm__ __volatile__("vmxoff");
 }
 
-static inline int vmclear(u64 vmcs_pa)
+static inline void vmclear(u64 vmcs_pa)
 {
 	u8 ret;
 
@@ -335,7 +335,7 @@ static inline int vmclear(u64 vmcs_pa)
 		: [pa]"m"(vmcs_pa)
 		: "cc", "memory");
 
-	return ret;
+	__GUEST_ASSERT(!ret, "vmclear [0x%lx] failed\n", vmcs_pa);
 }
 
 static inline int vmptrld(u64 vmcs_pa)
diff --git a/tools/testing/selftests/kvm/lib/x86/vmx.c b/tools/testing/selftests/kvm/lib/x86/vmx.c
index 03d61b1159f8..57a8287de0d2 100644
--- a/tools/testing/selftests/kvm/lib/x86/vmx.c
+++ b/tools/testing/selftests/kvm/lib/x86/vmx.c
@@ -170,16 +170,14 @@ bool load_vmcs(struct vmx_pages *vmx)
 {
 	/* Load a VMCS. */
 	*(u32 *)(vmx->vmcs) = vmcs_revision();
-	if (vmclear(vmx->vmcs_gpa))
-		return false;
+	vmclear(vmx->vmcs_gpa);
 
 	if (vmptrld(vmx->vmcs_gpa))
 		return false;
 
 	/* Setup shadow VMCS, do not load it yet. */
 	*(u32 *)(vmx->shadow_vmcs) = vmcs_revision() | 0x80000000ul;
-	if (vmclear(vmx->shadow_vmcs_gpa))
-		return false;
+	vmclear(vmx->shadow_vmcs_gpa);
 
 	return true;
 }
-- 
2.55.0.887.g758fc8c411-goog

[PATCH v3 09/13] KVM: selftests: Always assert that vmptrld() succeeds

[Sean Christopherson]

Assert that vmptrld() succeeds instead of forcing all callers to do the
same, and opportunistically provide a macro to print (hopefully) useful
information on VMX instruction failures.

Leave load_vmcs() as-is for the moment to minimize the amount of churn
since the vmptrld() changes aren't completely trivial.

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 tools/testing/selftests/kvm/include/x86/vmx.h | 23 +++++++++----------
 tools/testing/selftests/kvm/lib/x86/vmx.c     |  3 +--
 tools/testing/selftests/kvm/x86/state_test.c  |  6 ++---
 3 files changed, 15 insertions(+), 17 deletions(-)

diff --git a/tools/testing/selftests/kvm/include/x86/vmx.h b/tools/testing/selftests/kvm/include/x86/vmx.h
index cf37a03a93b9..8f004b946826 100644
--- a/tools/testing/selftests/kvm/include/x86/vmx.h
+++ b/tools/testing/selftests/kvm/include/x86/vmx.h
@@ -309,6 +309,10 @@ struct vmx_msr_entry {
 
 #include "evmcs.h"
 
+#define GUEST_ASSERT_VMX_INSN_SUCCEEDED(insn, __r, __pa)		\
+	__GUEST_ASSERT(!__r, __stringify(insn) "[0x%lx] hit %s",	\
+		       __pa, __r < 0 ? "VM-Fail" : ex_str(__r))
+
 static inline void vmxon(u64 phys)
 {
 	u8 ret;
@@ -338,18 +342,6 @@ static inline void vmclear(u64 vmcs_pa)
 	__GUEST_ASSERT(!ret, "vmclear [0x%lx] failed\n", vmcs_pa);
 }
 
-static inline int vmptrld(u64 vmcs_pa)
-{
-	u8 ret;
-
-	__asm__ __volatile__ ("vmptrld %[pa]; setna %[ret]"
-		: [ret]"=rm"(ret)
-		: [pa]"m"(vmcs_pa)
-		: "cc", "memory");
-
-	return ret;
-}
-
 static inline int __vmptrld(u64 vmcs_pa)
 {
 	u64 error_code;
@@ -366,6 +358,13 @@ static inline int __vmptrld(u64 vmcs_pa)
 	return vector ? vector : failed ? -EINVAL : 0;
 }
 
+static inline void vmptrld(u64 vmcs_pa)
+{
+	int ret = __vmptrld(vmcs_pa);
+
+	GUEST_ASSERT_VMX_INSN_SUCCEEDED(vmptrld, ret, vmcs_pa);
+}
+
 static inline u64 vmptrst(void)
 {
 	u64 value = 0;
diff --git a/tools/testing/selftests/kvm/lib/x86/vmx.c b/tools/testing/selftests/kvm/lib/x86/vmx.c
index 57a8287de0d2..e65024b9ea0d 100644
--- a/tools/testing/selftests/kvm/lib/x86/vmx.c
+++ b/tools/testing/selftests/kvm/lib/x86/vmx.c
@@ -172,8 +172,7 @@ bool load_vmcs(struct vmx_pages *vmx)
 	*(u32 *)(vmx->vmcs) = vmcs_revision();
 	vmclear(vmx->vmcs_gpa);
 
-	if (vmptrld(vmx->vmcs_gpa))
-		return false;
+	vmptrld(vmx->vmcs_gpa);
 
 	/* Setup shadow VMCS, do not load it yet. */
 	*(u32 *)(vmx->shadow_vmcs) = vmcs_revision() | 0x80000000ul;
diff --git a/tools/testing/selftests/kvm/x86/state_test.c b/tools/testing/selftests/kvm/x86/state_test.c
index 203584fcaf62..a7786f478be0 100644
--- a/tools/testing/selftests/kvm/x86/state_test.c
+++ b/tools/testing/selftests/kvm/x86/state_test.c
@@ -108,7 +108,7 @@ static void vmx_l1_guest_code(struct vmx_pages *vmx_pages)
 	vmwrite(SECONDARY_VM_EXEC_CONTROL, SECONDARY_EXEC_SHADOW_VMCS);
 	vmwrite(VMCS_LINK_POINTER, vmx_pages->shadow_vmcs_gpa);
 
-	GUEST_ASSERT(!vmptrld(vmx_pages->shadow_vmcs_gpa));
+	vmptrld(vmx_pages->shadow_vmcs_gpa);
 	GUEST_ASSERT(vmlaunch());
 	GUEST_SYNC(8);
 	GUEST_ASSERT(vmlaunch());
@@ -118,11 +118,11 @@ static void vmx_l1_guest_code(struct vmx_pages *vmx_pages)
 	GUEST_SYNC(9);
 	GUEST_ASSERT(vmreadz(GUEST_RIP) == 0xc0ffee);
 
-	GUEST_ASSERT(!vmptrld(vmx_pages->vmcs_gpa));
+	vmptrld(vmx_pages->vmcs_gpa);
 	GUEST_ASSERT(!vmresume());
 	GUEST_ASSERT(vmreadz(VM_EXIT_REASON) == EXIT_REASON_VMCALL);
 
-	GUEST_ASSERT(!vmptrld(vmx_pages->shadow_vmcs_gpa));
+	vmptrld(vmx_pages->shadow_vmcs_gpa);
 	GUEST_ASSERT(vmreadz(GUEST_RIP) == 0xc0ffffee);
 	GUEST_ASSERT(vmlaunch());
 	GUEST_ASSERT(vmresume());
-- 
2.55.0.887.g758fc8c411-goog

[PATCH v3 10/13] KVM: selftests: Drop useless return code from load_vmcs()

[Sean Christopherson]

Now that load_vmcs() either asserts or returns success, drop the return
code and the many asserts in its callers.

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 tools/testing/selftests/kvm/include/x86/vmx.h                 | 2 +-
 tools/testing/selftests/kvm/lib/x86/memstress.c               | 2 +-
 tools/testing/selftests/kvm/lib/x86/vmx.c                     | 4 +---
 tools/testing/selftests/kvm/x86/aperfmperf_test.c             | 2 +-
 tools/testing/selftests/kvm/x86/kvm_buslock_test.c            | 2 +-
 tools/testing/selftests/kvm/x86/nested_close_kvm_test.c       | 2 +-
 tools/testing/selftests/kvm/x86/nested_dirty_log_test.c       | 2 +-
 tools/testing/selftests/kvm/x86/nested_emulation_test.c       | 2 +-
 tools/testing/selftests/kvm/x86/nested_exceptions_test.c      | 2 +-
 tools/testing/selftests/kvm/x86/nested_invalid_cr3_test.c     | 2 +-
 tools/testing/selftests/kvm/x86/nested_tdp_fault_test.c       | 2 +-
 tools/testing/selftests/kvm/x86/nested_tsc_adjust_test.c      | 2 +-
 tools/testing/selftests/kvm/x86/nested_tsc_scaling_test.c     | 2 +-
 tools/testing/selftests/kvm/x86/save_restore_pf_stress_test.c | 2 +-
 tools/testing/selftests/kvm/x86/smm_test.c                    | 2 +-
 tools/testing/selftests/kvm/x86/state_test.c                  | 2 +-
 tools/testing/selftests/kvm/x86/triple_fault_event_test.c     | 2 +-
 tools/testing/selftests/kvm/x86/vmx_apic_access_test.c        | 2 +-
 tools/testing/selftests/kvm/x86/vmx_apicv_updates_test.c      | 2 +-
 .../selftests/kvm/x86/vmx_invalid_nested_guest_state.c        | 2 +-
 tools/testing/selftests/kvm/x86/vmx_nested_la57_state_test.c  | 2 +-
 tools/testing/selftests/kvm/x86/vmx_preemption_timer_test.c   | 2 +-
 22 files changed, 22 insertions(+), 24 deletions(-)

diff --git a/tools/testing/selftests/kvm/include/x86/vmx.h b/tools/testing/selftests/kvm/include/x86/vmx.h
index 8f004b946826..5c46ae95c4c3 100644
--- a/tools/testing/selftests/kvm/include/x86/vmx.h
+++ b/tools/testing/selftests/kvm/include/x86/vmx.h
@@ -550,7 +550,7 @@ union vmx_ctrl_msr {
 struct vmx_pages *vcpu_alloc_vmx(struct kvm_vm *vm, gva_t *p_vmx_gva);
 void prepare_for_vmx_operation(struct vmx_pages *vmx);
 void prepare_vmcs(struct vmx_pages *vmx, void *guest_rip);
-bool load_vmcs(struct vmx_pages *vmx);
+void load_vmcs(struct vmx_pages *vmx);
 
 bool ept_1g_pages_supported(void);
 
diff --git a/tools/testing/selftests/kvm/lib/x86/memstress.c b/tools/testing/selftests/kvm/lib/x86/memstress.c
index 2fa39470c1b3..207350ab6114 100644
--- a/tools/testing/selftests/kvm/lib/x86/memstress.c
+++ b/tools/testing/selftests/kvm/lib/x86/memstress.c
@@ -34,7 +34,7 @@ static void l1_vmx_code(struct vmx_pages *vmx, u64 vcpu_id)
 {
 	GUEST_ASSERT(vmx->vmcs_gpa);
 	prepare_for_vmx_operation(vmx);
-	GUEST_ASSERT(load_vmcs(vmx));
+	load_vmcs(vmx);
 	GUEST_ASSERT(ept_1g_pages_supported());
 
 	*(u64 *)vmx->stack = vcpu_id;
diff --git a/tools/testing/selftests/kvm/lib/x86/vmx.c b/tools/testing/selftests/kvm/lib/x86/vmx.c
index e65024b9ea0d..5c6cadc08405 100644
--- a/tools/testing/selftests/kvm/lib/x86/vmx.c
+++ b/tools/testing/selftests/kvm/lib/x86/vmx.c
@@ -166,7 +166,7 @@ void prepare_for_vmx_operation(struct vmx_pages *vmx)
 	vmxon(vmx->vmxon_gpa);
 }
 
-bool load_vmcs(struct vmx_pages *vmx)
+void load_vmcs(struct vmx_pages *vmx)
 {
 	/* Load a VMCS. */
 	*(u32 *)(vmx->vmcs) = vmcs_revision();
@@ -177,8 +177,6 @@ bool load_vmcs(struct vmx_pages *vmx)
 	/* Setup shadow VMCS, do not load it yet. */
 	*(u32 *)(vmx->shadow_vmcs) = vmcs_revision() | 0x80000000ul;
 	vmclear(vmx->shadow_vmcs_gpa);
-
-	return true;
 }
 
 static bool ept_vpid_cap_supported(u64 mask)
diff --git a/tools/testing/selftests/kvm/x86/aperfmperf_test.c b/tools/testing/selftests/kvm/x86/aperfmperf_test.c
index e32131cd6ed1..11f5894d8ac7 100644
--- a/tools/testing/selftests/kvm/x86/aperfmperf_test.c
+++ b/tools/testing/selftests/kvm/x86/aperfmperf_test.c
@@ -71,7 +71,7 @@ static void l1_svm_code(struct svm_test_data *svm)
 static void l1_vmx_code(struct vmx_pages *vmx)
 {
 	prepare_for_vmx_operation(vmx);
-	GUEST_ASSERT_EQ(load_vmcs(vmx), true);
+	load_vmcs(vmx);
 
 	prepare_vmcs(vmx, NULL);
 
diff --git a/tools/testing/selftests/kvm/x86/kvm_buslock_test.c b/tools/testing/selftests/kvm/x86/kvm_buslock_test.c
index 2e006ce95896..22993ec697fa 100644
--- a/tools/testing/selftests/kvm/x86/kvm_buslock_test.c
+++ b/tools/testing/selftests/kvm/x86/kvm_buslock_test.c
@@ -43,7 +43,7 @@ static void l1_svm_code(struct svm_test_data *svm)
 static void l1_vmx_code(struct vmx_pages *vmx)
 {
 	prepare_for_vmx_operation(vmx);
-	GUEST_ASSERT_EQ(load_vmcs(vmx), true);
+	load_vmcs(vmx);
 
 	prepare_vmcs(vmx, NULL);
 
diff --git a/tools/testing/selftests/kvm/x86/nested_close_kvm_test.c b/tools/testing/selftests/kvm/x86/nested_close_kvm_test.c
index 479e9adcc20b..309cab009929 100644
--- a/tools/testing/selftests/kvm/x86/nested_close_kvm_test.c
+++ b/tools/testing/selftests/kvm/x86/nested_close_kvm_test.c
@@ -31,7 +31,7 @@ static void l2_guest_code(void)
 static void l1_vmx_code(struct vmx_pages *vmx_pages)
 {
 	prepare_for_vmx_operation(vmx_pages);
-	GUEST_ASSERT(load_vmcs(vmx_pages));
+	load_vmcs(vmx_pages);
 
 	/* Prepare the VMCS for L2 execution. */
 	prepare_vmcs(vmx_pages, l2_guest_code);
diff --git a/tools/testing/selftests/kvm/x86/nested_dirty_log_test.c b/tools/testing/selftests/kvm/x86/nested_dirty_log_test.c
index 57f49471f4aa..dca2bfc8b158 100644
--- a/tools/testing/selftests/kvm/x86/nested_dirty_log_test.c
+++ b/tools/testing/selftests/kvm/x86/nested_dirty_log_test.c
@@ -94,7 +94,7 @@ void l1_vmx_code(struct vmx_pages *vmx)
 
 	GUEST_ASSERT(vmx->vmcs_gpa);
 	prepare_for_vmx_operation(vmx);
-	GUEST_ASSERT(load_vmcs(vmx));
+	load_vmcs(vmx);
 
 	if (vmx->eptp_gpa)
 		l2_rip = l2_guest_code_tdp_enabled;
diff --git a/tools/testing/selftests/kvm/x86/nested_emulation_test.c b/tools/testing/selftests/kvm/x86/nested_emulation_test.c
index 6f27930bb6b5..2b38eff4f516 100644
--- a/tools/testing/selftests/kvm/x86/nested_emulation_test.c
+++ b/tools/testing/selftests/kvm/x86/nested_emulation_test.c
@@ -67,7 +67,7 @@ static void guest_code(void *test_data)
 		vmcb->control.intercept_exceptions = 0;
 	} else {
 		prepare_for_vmx_operation(test_data);
-		GUEST_ASSERT(load_vmcs(test_data));
+		load_vmcs(test_data);
 
 		prepare_vmcs(test_data, NULL);
 		GUEST_ASSERT(!vmwrite(GUEST_IDTR_LIMIT, 0));
diff --git a/tools/testing/selftests/kvm/x86/nested_exceptions_test.c b/tools/testing/selftests/kvm/x86/nested_exceptions_test.c
index 6a36cf4d1eab..50c271a03692 100644
--- a/tools/testing/selftests/kvm/x86/nested_exceptions_test.c
+++ b/tools/testing/selftests/kvm/x86/nested_exceptions_test.c
@@ -127,7 +127,7 @@ static void l1_vmx_code(struct vmx_pages *vmx)
 {
 	prepare_for_vmx_operation(vmx);
 
-	GUEST_ASSERT_EQ(load_vmcs(vmx), true);
+	load_vmcs(vmx);
 
 	prepare_vmcs(vmx, NULL);
 	GUEST_ASSERT_EQ(vmwrite(GUEST_IDTR_LIMIT, 0), 0);
diff --git a/tools/testing/selftests/kvm/x86/nested_invalid_cr3_test.c b/tools/testing/selftests/kvm/x86/nested_invalid_cr3_test.c
index 749a93c318fe..43deb3b358ea 100644
--- a/tools/testing/selftests/kvm/x86/nested_invalid_cr3_test.c
+++ b/tools/testing/selftests/kvm/x86/nested_invalid_cr3_test.c
@@ -41,7 +41,7 @@ static void l1_vmx_code(struct vmx_pages *vmx_pages)
 	uintptr_t save_cr3;
 
 	prepare_for_vmx_operation(vmx_pages);
-	GUEST_ASSERT(load_vmcs(vmx_pages));
+	load_vmcs(vmx_pages);
 
 	prepare_vmcs(vmx_pages, l2_guest_code);
 
diff --git a/tools/testing/selftests/kvm/x86/nested_tdp_fault_test.c b/tools/testing/selftests/kvm/x86/nested_tdp_fault_test.c
index b143ebe710e7..52df467e9c44 100644
--- a/tools/testing/selftests/kvm/x86/nested_tdp_fault_test.c
+++ b/tools/testing/selftests/kvm/x86/nested_tdp_fault_test.c
@@ -56,7 +56,7 @@ static void l1_vmx_code(struct vmx_pages *vmx, u64 expected_fault_gpa,
 
 	GUEST_ASSERT(vmx->vmcs_gpa);
 	prepare_for_vmx_operation(vmx);
-	GUEST_ASSERT(load_vmcs(vmx));
+	load_vmcs(vmx);
 
 	prepare_vmcs(vmx, l2_entry);
 
diff --git a/tools/testing/selftests/kvm/x86/nested_tsc_adjust_test.c b/tools/testing/selftests/kvm/x86/nested_tsc_adjust_test.c
index 3d2eae848ba6..05194762ed33 100644
--- a/tools/testing/selftests/kvm/x86/nested_tsc_adjust_test.c
+++ b/tools/testing/selftests/kvm/x86/nested_tsc_adjust_test.c
@@ -87,7 +87,7 @@ static void l1_guest_code(void *data)
 		u32 control;
 
 		prepare_for_vmx_operation(vmx_pages);
-		GUEST_ASSERT(load_vmcs(vmx_pages));
+		load_vmcs(vmx_pages);
 
 		prepare_vmcs(vmx_pages, l2_guest_code);
 		control = vmreadz(CPU_BASED_VM_EXEC_CONTROL);
diff --git a/tools/testing/selftests/kvm/x86/nested_tsc_scaling_test.c b/tools/testing/selftests/kvm/x86/nested_tsc_scaling_test.c
index 584071146b69..b9fae268a9ec 100644
--- a/tools/testing/selftests/kvm/x86/nested_tsc_scaling_test.c
+++ b/tools/testing/selftests/kvm/x86/nested_tsc_scaling_test.c
@@ -106,7 +106,7 @@ static void l1_vmx_code(struct vmx_pages *vmx_pages)
 	check_tsc_freq(UCHECK_L1);
 
 	prepare_for_vmx_operation(vmx_pages);
-	GUEST_ASSERT(load_vmcs(vmx_pages));
+	load_vmcs(vmx_pages);
 
 	/* prepare the VMCS for L2 execution */
 	prepare_vmcs(vmx_pages, l2_guest_code);
diff --git a/tools/testing/selftests/kvm/x86/save_restore_pf_stress_test.c b/tools/testing/selftests/kvm/x86/save_restore_pf_stress_test.c
index 6ab73a0bd34a..c0e4d6e156ad 100644
--- a/tools/testing/selftests/kvm/x86/save_restore_pf_stress_test.c
+++ b/tools/testing/selftests/kvm/x86/save_restore_pf_stress_test.c
@@ -99,7 +99,7 @@ static void l1_svm_code(struct svm_test_data *svm)
 static void l1_vmx_code(struct vmx_pages *vmx)
 {
 	prepare_for_vmx_operation(vmx);
-	GUEST_ASSERT(load_vmcs(vmx));
+	load_vmcs(vmx);
 	prepare_vmcs(vmx, guest_access_memory);
 
 	GUEST_ASSERT(!vmwrite(EXCEPTION_BITMAP, BIT(UD_VECTOR)));
diff --git a/tools/testing/selftests/kvm/x86/smm_test.c b/tools/testing/selftests/kvm/x86/smm_test.c
index be5662792ec8..377df09e582a 100644
--- a/tools/testing/selftests/kvm/x86/smm_test.c
+++ b/tools/testing/selftests/kvm/x86/smm_test.c
@@ -79,7 +79,7 @@ static void guest_code(void *arg)
 			generic_svm_setup(svm, l2_guest_code);
 		} else {
 			prepare_for_vmx_operation(vmx_pages);
-			GUEST_ASSERT(load_vmcs(vmx_pages));
+			load_vmcs(vmx_pages);
 			prepare_vmcs(vmx_pages, l2_guest_code);
 		}
 
diff --git a/tools/testing/selftests/kvm/x86/state_test.c b/tools/testing/selftests/kvm/x86/state_test.c
index a7786f478be0..bd5e9e36b2c9 100644
--- a/tools/testing/selftests/kvm/x86/state_test.c
+++ b/tools/testing/selftests/kvm/x86/state_test.c
@@ -77,7 +77,7 @@ static void vmx_l1_guest_code(struct vmx_pages *vmx_pages)
 	GUEST_ASSERT(vmx_pages->vmcs_gpa);
 	prepare_for_vmx_operation(vmx_pages);
 	GUEST_SYNC(3);
-	GUEST_ASSERT(load_vmcs(vmx_pages));
+	load_vmcs(vmx_pages);
 	GUEST_ASSERT(vmptrst() == vmx_pages->vmcs_gpa);
 
 	GUEST_SYNC(4);
diff --git a/tools/testing/selftests/kvm/x86/triple_fault_event_test.c b/tools/testing/selftests/kvm/x86/triple_fault_event_test.c
index 57e79922b16b..a9fb7046ea4f 100644
--- a/tools/testing/selftests/kvm/x86/triple_fault_event_test.c
+++ b/tools/testing/selftests/kvm/x86/triple_fault_event_test.c
@@ -26,7 +26,7 @@ void l1_guest_code_vmx(struct vmx_pages *vmx)
 
 	GUEST_ASSERT(vmx->vmcs_gpa);
 	prepare_for_vmx_operation(vmx);
-	GUEST_ASSERT(load_vmcs(vmx));
+	load_vmcs(vmx);
 
 	prepare_vmcs(vmx, l2_guest_code);
 
diff --git a/tools/testing/selftests/kvm/x86/vmx_apic_access_test.c b/tools/testing/selftests/kvm/x86/vmx_apic_access_test.c
index 7df9ab833cb2..1793f7b02219 100644
--- a/tools/testing/selftests/kvm/x86/vmx_apic_access_test.c
+++ b/tools/testing/selftests/kvm/x86/vmx_apic_access_test.c
@@ -39,7 +39,7 @@ static void l1_guest_code(struct vmx_pages *vmx_pages, unsigned long high_gpa)
 	u32 control;
 
 	prepare_for_vmx_operation(vmx_pages);
-	GUEST_ASSERT(load_vmcs(vmx_pages));
+	load_vmcs(vmx_pages);
 
 	/* Prepare the VMCS for L2 execution. */
 	prepare_vmcs(vmx_pages, l2_guest_code);
diff --git a/tools/testing/selftests/kvm/x86/vmx_apicv_updates_test.c b/tools/testing/selftests/kvm/x86/vmx_apicv_updates_test.c
index 8e4dc304533a..f7ebc9b15629 100644
--- a/tools/testing/selftests/kvm/x86/vmx_apicv_updates_test.c
+++ b/tools/testing/selftests/kvm/x86/vmx_apicv_updates_test.c
@@ -34,7 +34,7 @@ static void l1_guest_code(struct vmx_pages *vmx_pages)
 	u32 control;
 
 	prepare_for_vmx_operation(vmx_pages);
-	GUEST_ASSERT(load_vmcs(vmx_pages));
+	load_vmcs(vmx_pages);
 
 	/* Prepare the VMCS for L2 execution. */
 	prepare_vmcs(vmx_pages, l2_guest_code);
diff --git a/tools/testing/selftests/kvm/x86/vmx_invalid_nested_guest_state.c b/tools/testing/selftests/kvm/x86/vmx_invalid_nested_guest_state.c
index 8999f0458aa1..4a6445958cf4 100644
--- a/tools/testing/selftests/kvm/x86/vmx_invalid_nested_guest_state.c
+++ b/tools/testing/selftests/kvm/x86/vmx_invalid_nested_guest_state.c
@@ -42,7 +42,7 @@ static void l2_guest_code(void)
 static void l1_guest_code(struct vmx_pages *vmx_pages)
 {
 	prepare_for_vmx_operation(vmx_pages);
-	GUEST_ASSERT(load_vmcs(vmx_pages));
+	load_vmcs(vmx_pages);
 
 	/* Prepare the VMCS for L2 execution. */
 	prepare_vmcs(vmx_pages, l2_guest_code);
diff --git a/tools/testing/selftests/kvm/x86/vmx_nested_la57_state_test.c b/tools/testing/selftests/kvm/x86/vmx_nested_la57_state_test.c
index 444596ab1592..04c2d5113e84 100644
--- a/tools/testing/selftests/kvm/x86/vmx_nested_la57_state_test.c
+++ b/tools/testing/selftests/kvm/x86/vmx_nested_la57_state_test.c
@@ -38,7 +38,7 @@ static void l1_guest_code(struct vmx_pages *vmx_pages)
 
 	GUEST_ASSERT(vmx_pages->vmcs_gpa);
 	prepare_for_vmx_operation(vmx_pages);
-	GUEST_ASSERT(load_vmcs(vmx_pages));
+	load_vmcs(vmx_pages);
 
 	prepare_vmcs(vmx_pages, l2_guest_code);
 
diff --git a/tools/testing/selftests/kvm/x86/vmx_preemption_timer_test.c b/tools/testing/selftests/kvm/x86/vmx_preemption_timer_test.c
index 09f9686657ad..50fc9309158f 100644
--- a/tools/testing/selftests/kvm/x86/vmx_preemption_timer_test.c
+++ b/tools/testing/selftests/kvm/x86/vmx_preemption_timer_test.c
@@ -72,7 +72,7 @@ void l1_guest_code(struct vmx_pages *vmx_pages)
 
 	GUEST_ASSERT(vmx_pages->vmcs_gpa);
 	prepare_for_vmx_operation(vmx_pages);
-	GUEST_ASSERT(load_vmcs(vmx_pages));
+	load_vmcs(vmx_pages);
 	GUEST_ASSERT(vmptrst() == vmx_pages->vmcs_gpa);
 
 	prepare_vmcs(vmx_pages, l2_guest_code);
-- 
2.55.0.887.g758fc8c411-goog

[PATCH v3 11/13] KVM: selftests: Add macros to handle simple VMX instructions

[Sean Christopherson]

Add macros to build helpers for VMXON, VMPTRLD, and VMCLEAR, i.e. the VMX
instructions that take a VMCS address as input, and don't have outputs.
Getting the params right is annoying, especially when using KVM_ASM_SAFE().

As a bonus, this adds non-asserting double-underscores versions of VMXON
and VMCLEAR for "free".

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 tools/testing/selftests/kvm/include/x86/vmx.h | 71 +++++++------------
 1 file changed, 26 insertions(+), 45 deletions(-)

diff --git a/tools/testing/selftests/kvm/include/x86/vmx.h b/tools/testing/selftests/kvm/include/x86/vmx.h
index 5c46ae95c4c3..3def8df902a2 100644
--- a/tools/testing/selftests/kvm/include/x86/vmx.h
+++ b/tools/testing/selftests/kvm/include/x86/vmx.h
@@ -313,58 +313,39 @@ struct vmx_msr_entry {
 	__GUEST_ASSERT(!__r, __stringify(insn) "[0x%lx] hit %s",	\
 		       __pa, __r < 0 ? "VM-Fail" : ex_str(__r))
 
-static inline void vmxon(u64 phys)
-{
-	u8 ret;
-
-	__asm__ __volatile__ ("vmxon %[pa]; setna %[ret]"
-		: [ret]"=rm"(ret)
-		: [pa]"m"(phys)
-		: "cc", "memory");
-
-	__GUEST_ASSERT(!ret, "vmxon [0x%lx] failed", phys);
+#define BUILD_VMCS_ASM_HELPERS(insn)					\
+static inline int __##insn(u64 vmcs_pa)					\
+{									\
+	u64 error_code;							\
+	u8 vector;							\
+	u8 failed;							\
+									\
+	asm volatile(KVM_ASM_SAFE(__stringify(insn) " %[pa]")		\
+		     "\n\tsetna %[failed]"				\
+		     : KVM_ASM_SAFE_OUTPUTS(vector, error_code),	\
+		       [failed]"=qm"(failed)				\
+		     : [pa]"m"(vmcs_pa)					\
+		     : "cc", "memory", KVM_ASM_SAFE_CLOBBERS);		\
+									\
+	return vector ? vector : failed ? -EINVAL : 0;			\
+}									\
+									\
+static inline void insn(u64 vmcs_pa)					\
+{									\
+	int ret = __##insn(vmcs_pa);					\
+									\
+	GUEST_ASSERT_VMX_INSN_SUCCEEDED(insn, ret, vmcs_pa);		\
 }
 
+BUILD_VMCS_ASM_HELPERS(vmxon)
+BUILD_VMCS_ASM_HELPERS(vmptrld)
+BUILD_VMCS_ASM_HELPERS(vmclear)
+
 static inline void vmxoff(void)
 {
 	__asm__ __volatile__("vmxoff");
 }
 
-static inline void vmclear(u64 vmcs_pa)
-{
-	u8 ret;
-
-	__asm__ __volatile__ ("vmclear %[pa]; setna %[ret]"
-		: [ret]"=rm"(ret)
-		: [pa]"m"(vmcs_pa)
-		: "cc", "memory");
-
-	__GUEST_ASSERT(!ret, "vmclear [0x%lx] failed\n", vmcs_pa);
-}
-
-static inline int __vmptrld(u64 vmcs_pa)
-{
-	u64 error_code;
-	u8 vector;
-	u8 failed;
-
-	asm volatile(KVM_ASM_SAFE("vmptrld %[pa]")
-		     "\n\tsetna %[failed]"
-		     : KVM_ASM_SAFE_OUTPUTS(vector, error_code),
-		       [failed]"=qm"(failed)
-		     : [pa]"m"(vmcs_pa)
-		     : "cc", "memory", KVM_ASM_SAFE_CLOBBERS);
-
-	return vector ? vector : failed ? -EINVAL : 0;
-}
-
-static inline void vmptrld(u64 vmcs_pa)
-{
-	int ret = __vmptrld(vmcs_pa);
-
-	GUEST_ASSERT_VMX_INSN_SUCCEEDED(vmptrld, ret, vmcs_pa);
-}
-
 static inline u64 vmptrst(void)
 {
 	u64 value = 0;
-- 
2.55.0.887.g758fc8c411-goog

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

[Sean Christopherson]

Dedup the assembly code from VMLAUNCH vs. VMRESUME, the difference is
literally only the actual VM-Enter instruction.

No functional change intended.

Cc: Yosry Ahmed <yosry@kernel.org>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 tools/testing/selftests/kvm/include/x86/vmx.h | 74 +++++++------------
 1 file changed, 26 insertions(+), 48 deletions(-)

diff --git a/tools/testing/selftests/kvm/include/x86/vmx.h b/tools/testing/selftests/kvm/include/x86/vmx.h
index 3def8df902a2..fa01341e22bc 100644
--- a/tools/testing/selftests/kvm/include/x86/vmx.h
+++ b/tools/testing/selftests/kvm/include/x86/vmx.h
@@ -359,55 +359,33 @@ static inline u64 vmptrst(void)
 	return value;
 }
 
-static inline int vmlaunch(void)
-{
-	int ret;
-
-	if (enable_evmcs)
-		return evmcs_vmlaunch();
-
-	__asm__ __volatile__("push $0;"
-			     "vmwrite %%rsp, %[host_rsp];"
-			     "lea 1f(%%rip), %%rax;"
-			     "vmwrite %%rax, %[host_rip];"
-			     VMX_SWITCH_GPRS_ASM
-			     "vmlaunch;"
-			     "incq (%%rsp);"
-			     "1: ;"
-			     VMX_SWITCH_GPRS_ASM
-			     "pop %%rax;"
-			     : [ret]"=&a"(ret)
-			     : [host_rsp]"r"((u64)HOST_RSP),
-			       [host_rip]"r"((u64)HOST_RIP),
-			       GUEST_REGS_OFFSETS
-			     : "memory", "cc");
-	return ret;
-}
-
-static inline int vmresume(void)
-{
-	int ret;
-
-	if (enable_evmcs)
-		return evmcs_vmresume();
-
-	__asm__ __volatile__("push $0;"
-			     "vmwrite %%rsp, %[host_rsp];"
-			     "lea 1f(%%rip), %%rax;"
-			     "vmwrite %%rax, %[host_rip];"
-			     VMX_SWITCH_GPRS_ASM
-			     "vmresume;"
-			     "incq (%%rsp);"
-			     "1: ;"
-			     VMX_SWITCH_GPRS_ASM
-			     "pop %%rax;"
-			     : [ret]"=&a"(ret)
-			     : [host_rsp]"r"((u64)HOST_RSP),
-			       [host_rip]"r"((u64)HOST_RIP),
-			       GUEST_REGS_OFFSETS
-			     : "memory", "cc");
-	return ret;
+#define BUILD_VMX_VM_ENTRY_HELPERS(insn)				\
+static inline int insn(void)						\
+{									\
+	int ret;							\
+									\
+	if (enable_evmcs)						\
+		return evmcs_##insn();					\
+									\
+	__asm__ __volatile__("push $0;"					\
+			     "vmwrite %%rsp, %[host_rsp];"		\
+			     "lea 1f(%%rip), %%rax;"			\
+			     "vmwrite %%rax, %[host_rip];"		\
+			     VMX_SWITCH_GPRS_ASM			\
+			     __stringify(insn)";"			\
+			     "incq (%%rsp);"				\
+			     "1: ;"					\
+			     VMX_SWITCH_GPRS_ASM			\
+			     "pop %%rax;"				\
+			     : [ret]"=&a"(ret)				\
+			     : [host_rsp]"r"((u64)HOST_RSP),		\
+			       [host_rip]"r"((u64)HOST_RIP),		\
+			       GUEST_REGS_OFFSETS			\
+			     : "memory", "cc");				\
+	return ret;							\
 }
+BUILD_VMX_VM_ENTRY_HELPERS(vmlaunch)
+BUILD_VMX_VM_ENTRY_HELPERS(vmresume)
 
 static inline void vmcall(void)
 {
-- 
2.55.0.887.g758fc8c411-goog

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

[Yosry Ahmed]

On Wed, Aug 26, 2026 at 4:39 PM Sean Christopherson <seanjc@google.com> 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.

>
> No functional change intended.
>
> Cc: Yosry Ahmed <yosry@kernel.org>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
>  tools/testing/selftests/kvm/include/x86/vmx.h | 74 +++++++------------
>  1 file changed, 26 insertions(+), 48 deletions(-)
>
> diff --git a/tools/testing/selftests/kvm/include/x86/vmx.h b/tools/testing/selftests/kvm/include/x86/vmx.h
> index 3def8df902a2..fa01341e22bc 100644
> --- a/tools/testing/selftests/kvm/include/x86/vmx.h
> +++ b/tools/testing/selftests/kvm/include/x86/vmx.h
> @@ -359,55 +359,33 @@ static inline u64 vmptrst(void)
>         return value;
>  }
>
> -static inline int vmlaunch(void)
> -{
> -       int ret;
> -
> -       if (enable_evmcs)
> -               return evmcs_vmlaunch();
> -
> -       __asm__ __volatile__("push $0;"
> -                            "vmwrite %%rsp, %[host_rsp];"
> -                            "lea 1f(%%rip), %%rax;"
> -                            "vmwrite %%rax, %[host_rip];"
> -                            VMX_SWITCH_GPRS_ASM
> -                            "vmlaunch;"
> -                            "incq (%%rsp);"
> -                            "1: ;"
> -                            VMX_SWITCH_GPRS_ASM
> -                            "pop %%rax;"
> -                            : [ret]"=&a"(ret)
> -                            : [host_rsp]"r"((u64)HOST_RSP),
> -                              [host_rip]"r"((u64)HOST_RIP),
> -                              GUEST_REGS_OFFSETS
> -                            : "memory", "cc");
> -       return ret;
> -}
> -
> -static inline int vmresume(void)
> -{
> -       int ret;
> -
> -       if (enable_evmcs)
> -               return evmcs_vmresume();
> -
> -       __asm__ __volatile__("push $0;"
> -                            "vmwrite %%rsp, %[host_rsp];"
> -                            "lea 1f(%%rip), %%rax;"
> -                            "vmwrite %%rax, %[host_rip];"
> -                            VMX_SWITCH_GPRS_ASM
> -                            "vmresume;"
> -                            "incq (%%rsp);"
> -                            "1: ;"
> -                            VMX_SWITCH_GPRS_ASM
> -                            "pop %%rax;"
> -                            : [ret]"=&a"(ret)
> -                            : [host_rsp]"r"((u64)HOST_RSP),
> -                              [host_rip]"r"((u64)HOST_RIP),
> -                              GUEST_REGS_OFFSETS
> -                            : "memory", "cc");
> -       return ret;
> +#define BUILD_VMX_VM_ENTRY_HELPERS(insn)                               \

s/HELPERS/HELPER?

or maybe better: DEFINE_VMX_VM_ENTRY_HELPER()?

With the rename:

Reviewed-by: Yosry Ahmed <yosry@kernel.org>

(Who's f734222792@gmail.com?)

> +static inline int insn(void)                                           \
> +{                                                                      \
> +       int ret;                                                        \
> +                                                                       \
> +       if (enable_evmcs)                                               \
> +               return evmcs_##insn();                                  \
> +                                                                       \
> +       __asm__ __volatile__("push $0;"                                 \
> +                            "vmwrite %%rsp, %[host_rsp];"              \
> +                            "lea 1f(%%rip), %%rax;"                    \
> +                            "vmwrite %%rax, %[host_rip];"              \
> +                            VMX_SWITCH_GPRS_ASM                        \
> +                            __stringify(insn)";"                       \
> +                            "incq (%%rsp);"                            \
> +                            "1: ;"                                     \
> +                            VMX_SWITCH_GPRS_ASM                        \
> +                            "pop %%rax;"                               \
> +                            : [ret]"=&a"(ret)                          \
> +                            : [host_rsp]"r"((u64)HOST_RSP),            \
> +                              [host_rip]"r"((u64)HOST_RIP),            \
> +                              GUEST_REGS_OFFSETS                       \
> +                            : "memory", "cc");                         \
> +       return ret;                                                     \
>  }
> +BUILD_VMX_VM_ENTRY_HELPERS(vmlaunch)
> +BUILD_VMX_VM_ENTRY_HELPERS(vmresume)
>
>  static inline void vmcall(void)
>  {
> --
> 2.55.0.887.g758fc8c411-goog
>

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

[Sean Christopherson]

On Wed, Aug 26, 2026, Yosry Ahmed wrote:
> On Wed, Aug 26, 2026 at 4:39 PM Sean Christopherson <seanjc@google.com> 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)				\
  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();
  }

And then the next patch becomes:

    KVM: selftests: Assert success in vmlaunch() and vmresume()
    
    Assert success in the "outer" APIs for VMLAUNCH and VMRESUME to dedup a
    pile of code, and switch to the double-underscores versions as necessary,
    to make it more obvious which paths expect VM-Enter *failure*.
    
    Signed-off-by: Sean Christopherson <seanjc@google.com>

diff --git tools/testing/selftests/kvm/include/x86/vmx.h tools/testing/selftests/kvm/include/x86/vmx.h
index d0ac8e128d98..1419043b41dd 100644
--- tools/testing/selftests/kvm/include/x86/vmx.h
+++ tools/testing/selftests/kvm/include/x86/vmx.h
@@ -362,14 +362,14 @@ static inline u64 vmptrst(void)
 int __vmlaunch(void);
 int __vmresume(void);
 
-static inline int vmlaunch(void)
+static inline void vmlaunch(void)
 {
-	return __vmlaunch();
+	__GUEST_ASSERT(!__vmlaunch(), "vmlaunch hit VM-Fail");
 }
 
-static inline int vmresume(void)
+static inline void vmresume(void)
 {
-	return __vmresume();
+	__GUEST_ASSERT(!__vmresume(), "vmresume hit VM-Fail");
 }
 
 static inline void vmcall(void)

> > +#define BUILD_VMX_VM_ENTRY_HELPERS(insn)                               \
> 
> s/HELPERS/HELPER?
> 
> or maybe better: DEFINE_VMX_VM_ENTRY_HELPER()?

Because after the next patch, there are two helpers for each instruction, vmresume()
and __vmresume(), and churning the macros to add the 'S' felt silly.  But, with the
above, this is a moot point.

> With the rename:
> 
> Reviewed-by: Yosry Ahmed <yosry@kernel.org>
> 
> (Who's f734222792@gmail.com?)

An anonymous bug reporter: https://bugzilla.kernel.org/show_bug.cgi?id=221841

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@google.com> wrote:
>
> On Wed, Aug 26, 2026, Yosry Ahmed wrote:
> > On Wed, Aug 26, 2026 at 4:39 PM Sean Christopherson <seanjc@google.com> 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();
>   }

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@google.com> 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)

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

[Yosry Ahmed]

On Thu, Aug 27, 2026 at 10:17 AM Sean Christopherson <seanjc@google.com> wrote:
>
> On Thu, Aug 27, 2026, Yosry Ahmed wrote:
> > On Thu, Aug 27, 2026 at 9:41 AM Sean Christopherson <seanjc@google.com> 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>].

Can't we keep it as a register operand just to avoid vmwrite_operand?
Does it actually hurt in any way?

>
> 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.

TIL.

>
> 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:

Right, I was suggested using register operands for everything.

>
> 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),                               \

I suppose we can use "r" here though?

>                                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)
>

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 10:17 AM Sean Christopherson <seanjc@google.com> wrote:
> > 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>].
> 
> Can't we keep it as a register operand just to avoid vmwrite_operand?
> Does it actually hurt in any way?

...

> > 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),                               \
> 
> I suppose we can use "r" here though?

No, because then the encoding for VMWRITE needs to be:

  vmwrite %%rsp, %[host_rsp]

but for MOV/eVMCS needs to be:

  mov %%rsp, (%[host_rsp])

Have fun feeding the '(' and ')' into the asm blob :-)

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

[Yosry Ahmed]

On Thu, Aug 27, 2026 at 10:33 AM Sean Christopherson <seanjc@google.com> wrote:
>
> On Thu, Aug 27, 2026, Yosry Ahmed wrote:
> > On Thu, Aug 27, 2026 at 10:17 AM Sean Christopherson <seanjc@google.com> wrote:
> > > 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>].
> >
> > Can't we keep it as a register operand just to avoid vmwrite_operand?
> > Does it actually hurt in any way?
>
> ...
>
> > > 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),                               \
> >
> > I suppose we can use "r" here though?
>
> No, because then the encoding for VMWRITE needs to be:
>
>   vmwrite %%rsp, %[host_rsp]
>
> but for MOV/eVMCS needs to be:
>
>   mov %%rsp, (%[host_rsp])
>
> Have fun feeding the '(' and ')' into the asm blob :-)

Semi-joking, what if we pass in the entire instruction instead?

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 10:33 AM Sean Christopherson <seanjc@google.com> wrote:
> >
> > On Thu, Aug 27, 2026, Yosry Ahmed wrote:
> > > On Thu, Aug 27, 2026 at 10:17 AM Sean Christopherson <seanjc@google.com> wrote:
> > > > 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>].
> > >
> > > Can't we keep it as a register operand just to avoid vmwrite_operand?
> > > Does it actually hurt in any way?
> >
> > ...
> >
> > > > 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),                               \
> > >
> > > I suppose we can use "r" here though?
> >
> > No, because then the encoding for VMWRITE needs to be:
> >
> >   vmwrite %%rsp, %[host_rsp]
> >
> > but for MOV/eVMCS needs to be:
> >
> >   mov %%rsp, (%[host_rsp])
> >
> > Have fun feeding the '(' and ')' into the asm blob :-)
> 
> Semi-joking, what if we pass in the entire instruction instead?

Then there needs to be separate parameters for RSP vs. RIP, and we still need to
pass different operands, *and* it bleeds information into the callers since they
would need to hardcode use of %%rax and of the named constraints.

As ugly as the proposed code is, IMO the maintenance implications of the below
is far worse than the subtle 'r' vs. 'm' (and on principle, I dislike passing an
address in a register and then manually encoding a memory operand).

#define __BUILD_VMX_VM_ENTRY_HELPER(insn, prefix, vmwrite_rsp, vmwrite_rip,		\
				    __host_rsp, __host_rip)				\
static int __##prefix##_##insn(void)							\
{											\
	int ret;									\
											\
	__asm__ __volatile__("push $0;"							\
			     vmwrite_rsp						\
			     "lea 1f(%%rip), %%rax;"					\
			     vmwrite_rip						\
			     VMX_SWITCH_GPRS_ASM					\
			     __stringify(insn)";"					\
			     "incq (%%rsp);"						\
			     "1: ;"							\
			     VMX_SWITCH_GPRS_ASM					\
			     "pop %%rax;"						\
			     : [ret]"=&a"(ret)						\
			     : [host_rsp]"r"((u64)__host_rsp),				\
			       [host_rip]"r"((u64)__host_rip),				\
			       GUEST_REGS_OFFSETS					\
			     : "memory", "cc");						\
	return ret;									\
}

#define BUILD_VMX_VM_ENTRY_HELPER(insn) 						\
	__BUILD_VMX_VM_ENTRY_HELPER(insn, _,						\
				    "vmwrite %%rsp, %[host_rsp];",			\
				    "vmwrite %%rax, %[host_rip];",			\
				    HOST_RSP, HOST_RIP)					\
	__BUILD_VMX_VM_ENTRY_HELPER(insn, __evmcs,					\
				    "mov %%rsp, (%[host_rsp]);",			\
				    "mov %%rsp, (%[host_rip]);",			\
				    &current_evmcs->host_rsp, &current_evmcs->host_rip)

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

[Yosry Ahmed]

On Thu, Aug 27, 2026 at 11:07 AM Sean Christopherson <seanjc@google.com> wrote:
>
> On Thu, Aug 27, 2026, Yosry Ahmed wrote:
> > On Thu, Aug 27, 2026 at 10:33 AM Sean Christopherson <seanjc@google.com> wrote:
> > >
> > > On Thu, Aug 27, 2026, Yosry Ahmed wrote:
> > > > On Thu, Aug 27, 2026 at 10:17 AM Sean Christopherson <seanjc@google.com> wrote:
> > > > > 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>].
> > > >
> > > > Can't we keep it as a register operand just to avoid vmwrite_operand?
> > > > Does it actually hurt in any way?
> > >
> > > ...
> > >
> > > > > 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),                               \
> > > >
> > > > I suppose we can use "r" here though?
> > >
> > > No, because then the encoding for VMWRITE needs to be:
> > >
> > >   vmwrite %%rsp, %[host_rsp]
> > >
> > > but for MOV/eVMCS needs to be:
> > >
> > >   mov %%rsp, (%[host_rsp])
> > >
> > > Have fun feeding the '(' and ')' into the asm blob :-)
> >
> > Semi-joking, what if we pass in the entire instruction instead?
>
> Then there needs to be separate parameters for RSP vs. RIP, and we still need to
> pass different operands, *and* it bleeds information into the callers since they
> would need to hardcode use of %%rax and of the named constraints.
>
> As ugly as the proposed code is, IMO the maintenance implications of the below
> is far worse than the subtle 'r' vs. 'm' (and on principle, I dislike passing an
> address in a register and then manually encoding a memory operand).

Yeah this isn't pretty either. I don't feel strongly either way tbh,
so whatever you think is best.

One other option is accepting some duplication, and letting the eVMCS
variants be separate. Basically keep your original patch as-is, and
just add a similar one for eVMCS variants.

Again, no strong preference, and I generally prefer your proposal for
deduplicating all of them if it wasn't for the subtle 'r' vs 'm'
thingy :/

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

[PATCH v3 13/13] KVM: selftests: Add and use double-underscore versions of vmlaunch() and vmresume()

[Sean Christopherson]

Add double-underscore helpers for VMLAUNCH and VMRESUME, and have the
"outer" APIs assert success, as is common through KVM selftests.  In
addition to providing more familiar interfaces, this makes it more obvious
which paths expect VM-Enter *failure*.

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 tools/testing/selftests/kvm/include/x86/vmx.h | 10 +++++++-
 .../testing/selftests/kvm/lib/x86/memstress.c |  2 +-
 .../selftests/kvm/x86/aperfmperf_test.c       |  2 +-
 .../kvm/x86/evmcs_smm_controls_test.c         |  2 +-
 .../testing/selftests/kvm/x86/hyperv_evmcs.c  | 22 ++++++++---------
 .../selftests/kvm/x86/kvm_buslock_test.c      |  2 +-
 .../selftests/kvm/x86/nested_close_kvm_test.c |  2 +-
 .../selftests/kvm/x86/nested_dirty_log_test.c |  2 +-
 .../selftests/kvm/x86/nested_emulation_test.c |  2 +-
 .../kvm/x86/nested_exceptions_test.c          |  2 +-
 .../kvm/x86/nested_invalid_cr3_test.c         |  4 ++--
 .../selftests/kvm/x86/nested_tdp_fault_test.c |  2 +-
 .../kvm/x86/nested_tsc_adjust_test.c          |  2 +-
 .../kvm/x86/nested_tsc_scaling_test.c         |  2 +-
 .../kvm/x86/save_restore_pf_stress_test.c     |  4 ++--
 tools/testing/selftests/kvm/x86/state_test.c  | 24 +++++++++----------
 .../kvm/x86/triple_fault_event_test.c         |  2 +-
 .../selftests/kvm/x86/vmx_apic_access_test.c  |  4 ++--
 .../kvm/x86/vmx_apicv_updates_test.c          |  4 ++--
 .../kvm/x86/vmx_invalid_nested_guest_state.c  |  2 +-
 .../kvm/x86/vmx_nested_la57_state_test.c      |  2 +-
 .../kvm/x86/vmx_preemption_timer_test.c       |  4 ++--
 22 files changed, 56 insertions(+), 48 deletions(-)

diff --git a/tools/testing/selftests/kvm/include/x86/vmx.h b/tools/testing/selftests/kvm/include/x86/vmx.h
index fa01341e22bc..cd82a6be6963 100644
--- a/tools/testing/selftests/kvm/include/x86/vmx.h
+++ b/tools/testing/selftests/kvm/include/x86/vmx.h
@@ -360,7 +360,7 @@ static inline u64 vmptrst(void)
 }
 
 #define BUILD_VMX_VM_ENTRY_HELPERS(insn)				\
-static inline int insn(void)						\
+static inline int __##insn(void)					\
 {									\
 	int ret;							\
 									\
@@ -383,6 +383,14 @@ static inline int insn(void)						\
 			       GUEST_REGS_OFFSETS			\
 			     : "memory", "cc");				\
 	return ret;							\
+}									\
+									\
+static inline void insn(void)						\
+{									\
+	int ret;							\
+									\
+	ret = __##insn();						\
+	__GUEST_ASSERT(!ret, __stringify(insn) " hit VM-Fail");		\
 }
 BUILD_VMX_VM_ENTRY_HELPERS(vmlaunch)
 BUILD_VMX_VM_ENTRY_HELPERS(vmresume)
diff --git a/tools/testing/selftests/kvm/lib/x86/memstress.c b/tools/testing/selftests/kvm/lib/x86/memstress.c
index 207350ab6114..6a1815fb97e9 100644
--- a/tools/testing/selftests/kvm/lib/x86/memstress.c
+++ b/tools/testing/selftests/kvm/lib/x86/memstress.c
@@ -40,7 +40,7 @@ static void l1_vmx_code(struct vmx_pages *vmx, u64 vcpu_id)
 	*(u64 *)vmx->stack = vcpu_id;
 	prepare_vmcs(vmx, memstress_l2_guest_entry);
 
-	GUEST_ASSERT(!vmlaunch());
+	vmlaunch();
 	GUEST_ASSERT_EQ(vmreadz(VM_EXIT_REASON), EXIT_REASON_VMCALL);
 	GUEST_DONE();
 }
diff --git a/tools/testing/selftests/kvm/x86/aperfmperf_test.c b/tools/testing/selftests/kvm/x86/aperfmperf_test.c
index 11f5894d8ac7..a082658c1e48 100644
--- a/tools/testing/selftests/kvm/x86/aperfmperf_test.c
+++ b/tools/testing/selftests/kvm/x86/aperfmperf_test.c
@@ -83,7 +83,7 @@ static void l1_vmx_code(struct vmx_pages *vmx)
 		vmreadz(CPU_BASED_VM_EXEC_CONTROL) | CPU_BASED_USE_MSR_BITMAPS);
 
 	GUEST_ASSERT(!vmwrite(GUEST_RIP, (u64)l2_guest_code));
-	GUEST_ASSERT(!vmlaunch());
+	vmlaunch();
 }
 
 static void guest_code(void *nested_test_data)
diff --git a/tools/testing/selftests/kvm/x86/evmcs_smm_controls_test.c b/tools/testing/selftests/kvm/x86/evmcs_smm_controls_test.c
index cafb58a37023..a06f37e0bd06 100644
--- a/tools/testing/selftests/kvm/x86/evmcs_smm_controls_test.c
+++ b/tools/testing/selftests/kvm/x86/evmcs_smm_controls_test.c
@@ -59,7 +59,7 @@ static void guest_code(struct vmx_pages *vmx_pages,
 	GUEST_ASSERT(load_evmcs(hv_pages));
 	prepare_vmcs(vmx_pages, l2_guest_code);
 
-	GUEST_ASSERT(!vmlaunch());
+	vmlaunch();
 
 	/* L2 exits via vmcall if test fails */
 	sync_with_host(2);
diff --git a/tools/testing/selftests/kvm/x86/hyperv_evmcs.c b/tools/testing/selftests/kvm/x86/hyperv_evmcs.c
index d0e857b75a47..b7c2a254291e 100644
--- a/tools/testing/selftests/kvm/x86/hyperv_evmcs.c
+++ b/tools/testing/selftests/kvm/x86/hyperv_evmcs.c
@@ -105,7 +105,7 @@ void guest_code(struct vmx_pages *vmx_pages, struct hyperv_test_pages *hv_pages,
 
 	GUEST_SYNC(5);
 	current_evmcs->revision_id = -1u;
-	GUEST_ASSERT(vmlaunch());
+	GUEST_ASSERT(__vmlaunch());
 	current_evmcs->revision_id = EVMCS_VERSION;
 	GUEST_SYNC(6);
 
@@ -120,7 +120,7 @@ void guest_code(struct vmx_pages *vmx_pages, struct hyperv_test_pages *hv_pages,
 	current_vp_assist->nested_control.features.directhypercall = 1;
 	*(u32 *)(hv_pages->partition_assist) = 0;
 
-	GUEST_ASSERT(!vmlaunch());
+	vmlaunch();
 	GUEST_ASSERT_EQ(vmreadz(VM_EXIT_REASON), EXIT_REASON_EXCEPTION_NMI);
 	GUEST_ASSERT_EQ((vmreadz(VM_EXIT_INTR_INFO) & 0xff), NMI_VECTOR);
 	GUEST_ASSERT(vmptrst() == hv_pages->enlightened_vmcs_gpa);
@@ -130,7 +130,7 @@ void guest_code(struct vmx_pages *vmx_pages, struct hyperv_test_pages *hv_pages,
 	 * up-to-date (RIP points where it should and not at the beginning
 	 * of l2_guest_code(). GUEST_SYNC(9) checks that.
 	 */
-	GUEST_ASSERT(!vmresume());
+	vmresume();
 
 	GUEST_SYNC(10);
 
@@ -141,13 +141,13 @@ void guest_code(struct vmx_pages *vmx_pages, struct hyperv_test_pages *hv_pages,
 	vmwrite(CPU_BASED_VM_EXEC_CONTROL, vmreadz(CPU_BASED_VM_EXEC_CONTROL) |
 		CPU_BASED_USE_MSR_BITMAPS);
 	__set_bit(MSR_FS_BASE & 0x1fff, vmx_pages->msr + 0x400);
-	GUEST_ASSERT(!vmresume());
+	vmresume();
 	GUEST_ASSERT(vmreadz(VM_EXIT_REASON) == EXIT_REASON_MSR_READ);
 	current_evmcs->guest_rip += 2; /* rdmsr */
 
 	/* Enable enlightened MSR bitmap */
 	current_evmcs->hv_enlightenments_control.msr_bitmap = 1;
-	GUEST_ASSERT(!vmresume());
+	vmresume();
 	GUEST_ASSERT(vmreadz(VM_EXIT_REASON) == EXIT_REASON_MSR_READ);
 	current_evmcs->guest_rip += 2; /* rdmsr */
 
@@ -155,14 +155,14 @@ void guest_code(struct vmx_pages *vmx_pages, struct hyperv_test_pages *hv_pages,
 	__set_bit(MSR_GS_BASE & 0x1fff, vmx_pages->msr + 0x400);
 	/* Make sure HV_VMX_ENLIGHTENED_CLEAN_FIELD_MSR_BITMAP is set */
 	current_evmcs->hv_clean_fields |= HV_VMX_ENLIGHTENED_CLEAN_FIELD_MSR_BITMAP;
-	GUEST_ASSERT(!vmresume());
+	vmresume();
 	/* Make sure we don't see EXIT_REASON_MSR_READ here so eMSR bitmap works */
 	GUEST_ASSERT(vmreadz(VM_EXIT_REASON) == EXIT_REASON_VMCALL);
 	current_evmcs->guest_rip += 3; /* vmcall */
 
 	/* Now tell KVM we've changed MSR-Bitmap */
 	current_evmcs->hv_clean_fields &= ~HV_VMX_ENLIGHTENED_CLEAN_FIELD_MSR_BITMAP;
-	GUEST_ASSERT(!vmresume());
+	vmresume();
 	GUEST_ASSERT(vmreadz(VM_EXIT_REASON) == EXIT_REASON_MSR_READ);
 	current_evmcs->guest_rip += 2; /* rdmsr */
 
@@ -170,15 +170,15 @@ void guest_code(struct vmx_pages *vmx_pages, struct hyperv_test_pages *hv_pages,
 	 * L2 TLB flush test. First VMCALL should be handled directly by L0,
 	 * no VMCALL exit expected.
 	 */
-	GUEST_ASSERT(!vmresume());
+	vmresume();
 	GUEST_ASSERT(vmreadz(VM_EXIT_REASON) == EXIT_REASON_MSR_READ);
 	current_evmcs->guest_rip += 2; /* rdmsr */
 	/* Enable synthetic vmexit */
 	*(u32 *)(hv_pages->partition_assist) = 1;
-	GUEST_ASSERT(!vmresume());
+	vmresume();
 	GUEST_ASSERT(vmreadz(VM_EXIT_REASON) == HV_VMX_SYNTHETIC_EXIT_REASON_TRAP_AFTER_FLUSH);
 
-	GUEST_ASSERT(!vmresume());
+	vmresume();
 	GUEST_ASSERT(vmreadz(VM_EXIT_REASON) == EXIT_REASON_VMCALL);
 	GUEST_SYNC(11);
 
@@ -192,7 +192,7 @@ void guest_code(struct vmx_pages *vmx_pages, struct hyperv_test_pages *hv_pages,
 	 */
 	GUEST_SYNC(12);
 	evmcs_vmptrld(0xdeadbeef, hv_pages->enlightened_vmcs);
-	GUEST_ASSERT(vmlaunch());
+	GUEST_ASSERT(__vmlaunch());
 	GUEST_ASSERT(ud_count == 1);
 	GUEST_DONE();
 }
diff --git a/tools/testing/selftests/kvm/x86/kvm_buslock_test.c b/tools/testing/selftests/kvm/x86/kvm_buslock_test.c
index 22993ec697fa..337d3c28bc6d 100644
--- a/tools/testing/selftests/kvm/x86/kvm_buslock_test.c
+++ b/tools/testing/selftests/kvm/x86/kvm_buslock_test.c
@@ -48,7 +48,7 @@ static void l1_vmx_code(struct vmx_pages *vmx)
 	prepare_vmcs(vmx, NULL);
 
 	GUEST_ASSERT(!vmwrite(GUEST_RIP, (u64)l2_guest_code));
-	GUEST_ASSERT(!vmlaunch());
+	vmlaunch();
 }
 
 static void guest_code(void *test_data)
diff --git a/tools/testing/selftests/kvm/x86/nested_close_kvm_test.c b/tools/testing/selftests/kvm/x86/nested_close_kvm_test.c
index 309cab009929..1f4a5e44a6ef 100644
--- a/tools/testing/selftests/kvm/x86/nested_close_kvm_test.c
+++ b/tools/testing/selftests/kvm/x86/nested_close_kvm_test.c
@@ -36,7 +36,7 @@ static void l1_vmx_code(struct vmx_pages *vmx_pages)
 	/* Prepare the VMCS for L2 execution. */
 	prepare_vmcs(vmx_pages, l2_guest_code);
 
-	GUEST_ASSERT(!vmlaunch());
+	vmlaunch();
 	GUEST_ASSERT(0);
 }
 
diff --git a/tools/testing/selftests/kvm/x86/nested_dirty_log_test.c b/tools/testing/selftests/kvm/x86/nested_dirty_log_test.c
index dca2bfc8b158..7987ce50c382 100644
--- a/tools/testing/selftests/kvm/x86/nested_dirty_log_test.c
+++ b/tools/testing/selftests/kvm/x86/nested_dirty_log_test.c
@@ -104,7 +104,7 @@ void l1_vmx_code(struct vmx_pages *vmx)
 	prepare_vmcs(vmx, l2_rip);
 
 	GUEST_SYNC(TEST_SYNC_NO_FAULT);
-	GUEST_ASSERT(!vmlaunch());
+	vmlaunch();
 	GUEST_SYNC(TEST_SYNC_NO_FAULT);
 	GUEST_ASSERT_EQ(vmreadz(VM_EXIT_REASON), EXIT_REASON_VMCALL);
 	GUEST_DONE();
diff --git a/tools/testing/selftests/kvm/x86/nested_emulation_test.c b/tools/testing/selftests/kvm/x86/nested_emulation_test.c
index 2b38eff4f516..11fe4a31eda0 100644
--- a/tools/testing/selftests/kvm/x86/nested_emulation_test.c
+++ b/tools/testing/selftests/kvm/x86/nested_emulation_test.c
@@ -102,7 +102,7 @@ static void guest_code(void *test_data)
 			exit_insn_len = vmcb->control.next_rip - vmcb->save.rip;
 			GUEST_ASSERT_EQ(vmcb->save.rip, (u64)l2_instruction);
 		} else {
-			GUEST_ASSERT_EQ(i ? vmresume() : vmlaunch(), 0);
+			i ? vmresume() : vmlaunch();
 			exit_reason = vmreadz(VM_EXIT_REASON);
 			exit_insn_len = vmreadz(VM_EXIT_INSTRUCTION_LEN);
 			GUEST_ASSERT_EQ(vmreadz(GUEST_RIP), (u64)l2_instruction);
diff --git a/tools/testing/selftests/kvm/x86/nested_exceptions_test.c b/tools/testing/selftests/kvm/x86/nested_exceptions_test.c
index 50c271a03692..c36c11b5f518 100644
--- a/tools/testing/selftests/kvm/x86/nested_exceptions_test.c
+++ b/tools/testing/selftests/kvm/x86/nested_exceptions_test.c
@@ -112,7 +112,7 @@ static void vmx_run_l2(void *l2_code, int vector, u32 error_code)
 {
 	GUEST_ASSERT(!vmwrite(GUEST_RIP, (u64)l2_code));
 
-	GUEST_ASSERT_EQ(vector == SS_VECTOR ? vmlaunch() : vmresume(), 0);
+	vector == SS_VECTOR ? vmlaunch() : vmresume();
 
 	if (vector == FAKE_TRIPLE_FAULT_VECTOR)
 		return;
diff --git a/tools/testing/selftests/kvm/x86/nested_invalid_cr3_test.c b/tools/testing/selftests/kvm/x86/nested_invalid_cr3_test.c
index 43deb3b358ea..101c1fe79eeb 100644
--- a/tools/testing/selftests/kvm/x86/nested_invalid_cr3_test.c
+++ b/tools/testing/selftests/kvm/x86/nested_invalid_cr3_test.c
@@ -48,13 +48,13 @@ static void l1_vmx_code(struct vmx_pages *vmx_pages)
 	/* Try to run L2 with invalid CR3 and make sure it fails */
 	save_cr3 = vmreadz(GUEST_CR3);
 	vmwrite(GUEST_CR3, -1ull);
-	GUEST_ASSERT(!vmlaunch());
+	vmlaunch();
 	GUEST_ASSERT(vmreadz(VM_EXIT_REASON) ==
 		     (EXIT_REASON_FAILED_VMENTRY | EXIT_REASON_INVALID_STATE));
 
 	/* Now restore CR3 and make sure L2 runs successfully */
 	vmwrite(GUEST_CR3, save_cr3);
-	GUEST_ASSERT(!vmlaunch());
+	vmlaunch();
 	GUEST_ASSERT(vmreadz(VM_EXIT_REASON) == EXIT_REASON_VMCALL);
 
 	GUEST_DONE();
diff --git a/tools/testing/selftests/kvm/x86/nested_tdp_fault_test.c b/tools/testing/selftests/kvm/x86/nested_tdp_fault_test.c
index 52df467e9c44..d6c443e3d147 100644
--- a/tools/testing/selftests/kvm/x86/nested_tdp_fault_test.c
+++ b/tools/testing/selftests/kvm/x86/nested_tdp_fault_test.c
@@ -60,7 +60,7 @@ static void l1_vmx_code(struct vmx_pages *vmx, u64 expected_fault_gpa,
 
 	prepare_vmcs(vmx, l2_entry);
 
-	GUEST_ASSERT(!vmlaunch());
+	vmlaunch();
 
 	/* Verify we got an EPT violation exit */
 	__GUEST_ASSERT(vmreadz(VM_EXIT_REASON) == EXIT_REASON_EPT_VIOLATION,
diff --git a/tools/testing/selftests/kvm/x86/nested_tsc_adjust_test.c b/tools/testing/selftests/kvm/x86/nested_tsc_adjust_test.c
index 05194762ed33..c60dea899624 100644
--- a/tools/testing/selftests/kvm/x86/nested_tsc_adjust_test.c
+++ b/tools/testing/selftests/kvm/x86/nested_tsc_adjust_test.c
@@ -95,7 +95,7 @@ static void l1_guest_code(void *data)
 		vmwrite(CPU_BASED_VM_EXEC_CONTROL, control);
 		vmwrite(TSC_OFFSET, TSC_OFFSET_VALUE);
 
-		GUEST_ASSERT(!vmlaunch());
+		vmlaunch();
 		GUEST_ASSERT(vmreadz(VM_EXIT_REASON) == EXIT_REASON_VMCALL);
 	} else {
 		struct svm_test_data *svm = data;
diff --git a/tools/testing/selftests/kvm/x86/nested_tsc_scaling_test.c b/tools/testing/selftests/kvm/x86/nested_tsc_scaling_test.c
index b9fae268a9ec..b2fc7f0b5b3c 100644
--- a/tools/testing/selftests/kvm/x86/nested_tsc_scaling_test.c
+++ b/tools/testing/selftests/kvm/x86/nested_tsc_scaling_test.c
@@ -125,7 +125,7 @@ static void l1_vmx_code(struct vmx_pages *vmx_pages)
 	vmwrite(TSC_MULTIPLIER_HIGH, TSC_MULTIPLIER_L2 >> 32);
 
 	/* launch L2 */
-	GUEST_ASSERT(!vmlaunch());
+	vmlaunch();
 	GUEST_ASSERT(vmreadz(VM_EXIT_REASON) == EXIT_REASON_VMCALL);
 
 	/* check that L1's frequency still looks good */
diff --git a/tools/testing/selftests/kvm/x86/save_restore_pf_stress_test.c b/tools/testing/selftests/kvm/x86/save_restore_pf_stress_test.c
index c0e4d6e156ad..f88427e925fc 100644
--- a/tools/testing/selftests/kvm/x86/save_restore_pf_stress_test.c
+++ b/tools/testing/selftests/kvm/x86/save_restore_pf_stress_test.c
@@ -104,11 +104,11 @@ static void l1_vmx_code(struct vmx_pages *vmx)
 
 	GUEST_ASSERT(!vmwrite(EXCEPTION_BITMAP, BIT(UD_VECTOR)));
 
-	GUEST_ASSERT(!vmlaunch());
+	vmlaunch();
 	while (1) {
 		GUEST_ASSERT_EQ(vmreadz(VM_EXIT_REASON), EXIT_REASON_EXCEPTION_NMI);
 		GUEST_ASSERT_EQ(vmreadz(VM_EXIT_INTR_INFO) & 0xff, UD_VECTOR);
-		GUEST_ASSERT(!vmresume());
+		vmresume();
 	}
 }
 
diff --git a/tools/testing/selftests/kvm/x86/state_test.c b/tools/testing/selftests/kvm/x86/state_test.c
index bd5e9e36b2c9..38c9689ac601 100644
--- a/tools/testing/selftests/kvm/x86/state_test.c
+++ b/tools/testing/selftests/kvm/x86/state_test.c
@@ -87,20 +87,20 @@ static void vmx_l1_guest_code(struct vmx_pages *vmx_pages)
 
 	GUEST_SYNC(5);
 	GUEST_ASSERT(vmptrst() == vmx_pages->vmcs_gpa);
-	GUEST_ASSERT(!vmlaunch());
+	vmlaunch();
 	GUEST_ASSERT(vmptrst() == vmx_pages->vmcs_gpa);
 	GUEST_ASSERT(vmreadz(VM_EXIT_REASON) == EXIT_REASON_VMCALL);
 
 	/* Check that the launched state is preserved.  */
-	GUEST_ASSERT(vmlaunch());
+	GUEST_ASSERT(__vmlaunch());
 
-	GUEST_ASSERT(!vmresume());
+	vmresume();
 	GUEST_ASSERT(vmreadz(VM_EXIT_REASON) == EXIT_REASON_VMCALL);
 
 	GUEST_SYNC(7);
 	GUEST_ASSERT(vmreadz(VM_EXIT_REASON) == EXIT_REASON_VMCALL);
 
-	GUEST_ASSERT(!vmresume());
+	vmresume();
 	GUEST_ASSERT(vmreadz(VM_EXIT_REASON) == EXIT_REASON_VMCALL);
 
 	vmwrite(GUEST_RIP, vmreadz(GUEST_RIP) + 3);
@@ -109,27 +109,27 @@ static void vmx_l1_guest_code(struct vmx_pages *vmx_pages)
 	vmwrite(VMCS_LINK_POINTER, vmx_pages->shadow_vmcs_gpa);
 
 	vmptrld(vmx_pages->shadow_vmcs_gpa);
-	GUEST_ASSERT(vmlaunch());
+	GUEST_ASSERT(__vmlaunch());
 	GUEST_SYNC(8);
-	GUEST_ASSERT(vmlaunch());
-	GUEST_ASSERT(vmresume());
+	GUEST_ASSERT(__vmlaunch());
+	GUEST_ASSERT(__vmresume());
 
 	vmwrite(GUEST_RIP, 0xc0ffee);
 	GUEST_SYNC(9);
 	GUEST_ASSERT(vmreadz(GUEST_RIP) == 0xc0ffee);
 
 	vmptrld(vmx_pages->vmcs_gpa);
-	GUEST_ASSERT(!vmresume());
+	vmresume();
 	GUEST_ASSERT(vmreadz(VM_EXIT_REASON) == EXIT_REASON_VMCALL);
 
 	vmptrld(vmx_pages->shadow_vmcs_gpa);
 	GUEST_ASSERT(vmreadz(GUEST_RIP) == 0xc0ffffee);
-	GUEST_ASSERT(vmlaunch());
-	GUEST_ASSERT(vmresume());
+	GUEST_ASSERT(__vmlaunch());
+	GUEST_ASSERT(__vmresume());
 	GUEST_SYNC(13);
 	GUEST_ASSERT(vmreadz(GUEST_RIP) == 0xc0ffffee);
-	GUEST_ASSERT(vmlaunch());
-	GUEST_ASSERT(vmresume());
+	GUEST_ASSERT(__vmlaunch());
+	GUEST_ASSERT(__vmresume());
 }
 
 static void __attribute__((__flatten__)) guest_code(void *arg)
diff --git a/tools/testing/selftests/kvm/x86/triple_fault_event_test.c b/tools/testing/selftests/kvm/x86/triple_fault_event_test.c
index a9fb7046ea4f..bb31a1ba8932 100644
--- a/tools/testing/selftests/kvm/x86/triple_fault_event_test.c
+++ b/tools/testing/selftests/kvm/x86/triple_fault_event_test.c
@@ -30,7 +30,7 @@ void l1_guest_code_vmx(struct vmx_pages *vmx)
 
 	prepare_vmcs(vmx, l2_guest_code);
 
-	GUEST_ASSERT(!vmlaunch());
+	vmlaunch();
 	/* L2 should triple fault after a triple fault event injected. */
 	GUEST_ASSERT(vmreadz(VM_EXIT_REASON) == EXIT_REASON_TRIPLE_FAULT);
 	GUEST_DONE();
diff --git a/tools/testing/selftests/kvm/x86/vmx_apic_access_test.c b/tools/testing/selftests/kvm/x86/vmx_apic_access_test.c
index 1793f7b02219..fe29c869a235 100644
--- a/tools/testing/selftests/kvm/x86/vmx_apic_access_test.c
+++ b/tools/testing/selftests/kvm/x86/vmx_apic_access_test.c
@@ -53,14 +53,14 @@ static void l1_guest_code(struct vmx_pages *vmx_pages, unsigned long high_gpa)
 
 	/* Try to launch L2 with the memory-backed APIC-access address. */
 	GUEST_SYNC(vmreadz(APIC_ACCESS_ADDR));
-	GUEST_ASSERT(!vmlaunch());
+	vmlaunch();
 	GUEST_ASSERT(vmreadz(VM_EXIT_REASON) == EXIT_REASON_VMCALL);
 
 	vmwrite(APIC_ACCESS_ADDR, high_gpa);
 
 	/* Try to resume L2 with the unbacked APIC-access address. */
 	GUEST_SYNC(vmreadz(APIC_ACCESS_ADDR));
-	GUEST_ASSERT(!vmresume());
+	vmresume();
 	GUEST_ASSERT(vmreadz(VM_EXIT_REASON) == EXIT_REASON_VMCALL);
 
 	GUEST_DONE();
diff --git a/tools/testing/selftests/kvm/x86/vmx_apicv_updates_test.c b/tools/testing/selftests/kvm/x86/vmx_apicv_updates_test.c
index f7ebc9b15629..e578221da247 100644
--- a/tools/testing/selftests/kvm/x86/vmx_apicv_updates_test.c
+++ b/tools/testing/selftests/kvm/x86/vmx_apicv_updates_test.c
@@ -62,7 +62,7 @@ static void l1_guest_code(struct vmx_pages *vmx_pages)
 	 * Run L2 to switch to x2APIC mode, which in turn will uninhibit APICv,
 	 * as KVM should force the APIC ID back to its default.
 	 */
-	GUEST_ASSERT(!vmlaunch());
+	vmlaunch();
 	GUEST_ASSERT(vmreadz(VM_EXIT_REASON) == EXIT_REASON_VMCALL);
 	vmwrite(GUEST_RIP, vmreadz(GUEST_RIP) + vmreadz(VM_EXIT_INSTRUCTION_LEN));
 	GUEST_ASSERT(rdmsr(MSR_IA32_APICBASE) & MSR_IA32_APICBASE_EXTD);
@@ -90,7 +90,7 @@ static void l1_guest_code(struct vmx_pages *vmx_pages)
 	 * handles the x2APIC => xAPIC transition and inhibits APICv while L2
 	 * is active.
 	 */
-	GUEST_ASSERT(!vmresume());
+	vmresume();
 	GUEST_ASSERT(vmreadz(VM_EXIT_REASON) == EXIT_REASON_VMCALL);
 	GUEST_ASSERT(!(rdmsr(MSR_IA32_APICBASE) & MSR_IA32_APICBASE_EXTD));
 
diff --git a/tools/testing/selftests/kvm/x86/vmx_invalid_nested_guest_state.c b/tools/testing/selftests/kvm/x86/vmx_invalid_nested_guest_state.c
index 4a6445958cf4..3017938fa993 100644
--- a/tools/testing/selftests/kvm/x86/vmx_invalid_nested_guest_state.c
+++ b/tools/testing/selftests/kvm/x86/vmx_invalid_nested_guest_state.c
@@ -55,7 +55,7 @@ static void l1_guest_code(struct vmx_pages *vmx_pages)
 	GUEST_ASSERT(!(vmreadz(CPU_BASED_VM_EXEC_CONTROL) & CPU_BASED_ACTIVATE_SECONDARY_CONTROLS) ||
 		     !(vmreadz(SECONDARY_VM_EXEC_CONTROL) & SECONDARY_EXEC_UNRESTRICTED_GUEST));
 
-	GUEST_ASSERT(!vmlaunch());
+	vmlaunch();
 
 	/* L2 should triple fault after main() stuffs invalid guest state. */
 	GUEST_ASSERT(vmreadz(VM_EXIT_REASON) == EXIT_REASON_TRIPLE_FAULT);
diff --git a/tools/testing/selftests/kvm/x86/vmx_nested_la57_state_test.c b/tools/testing/selftests/kvm/x86/vmx_nested_la57_state_test.c
index 04c2d5113e84..b3651c5ab138 100644
--- a/tools/testing/selftests/kvm/x86/vmx_nested_la57_state_test.c
+++ b/tools/testing/selftests/kvm/x86/vmx_nested_la57_state_test.c
@@ -56,7 +56,7 @@ static void l1_guest_code(struct vmx_pages *vmx_pages)
 	guest_cr4 &= ~X86_CR4_LA57;
 	vmwrite(GUEST_CR4, guest_cr4);
 
-	GUEST_ASSERT(!vmlaunch());
+	vmlaunch();
 
 	exit_reason = vmreadz(VM_EXIT_REASON);
 	GUEST_ASSERT(exit_reason == EXIT_REASON_VMCALL);
diff --git a/tools/testing/selftests/kvm/x86/vmx_preemption_timer_test.c b/tools/testing/selftests/kvm/x86/vmx_preemption_timer_test.c
index 50fc9309158f..cda53df33888 100644
--- a/tools/testing/selftests/kvm/x86/vmx_preemption_timer_test.c
+++ b/tools/testing/selftests/kvm/x86/vmx_preemption_timer_test.c
@@ -90,7 +90,7 @@ void l1_guest_code(struct vmx_pages *vmx_pages)
 	    !(ctrl_exit_rev.clr & VM_EXIT_SAVE_VMX_PREEMPTION_TIMER))
 		return;
 
-	GUEST_ASSERT(!vmlaunch());
+	vmlaunch();
 	GUEST_ASSERT(vmreadz(VM_EXIT_REASON) == EXIT_REASON_VMCALL);
 	vmwrite(GUEST_RIP, vmreadz(GUEST_RIP) + vmreadz(VM_EXIT_INSTRUCTION_LEN));
 
@@ -110,7 +110,7 @@ void l1_guest_code(struct vmx_pages *vmx_pages)
 
 	l1_vmx_pt_start = (rdtsc() >> vmx_pt_rate) << vmx_pt_rate;
 
-	GUEST_ASSERT(!vmresume());
+	vmresume();
 
 	l1_vmx_pt_finish = rdtsc();
 
-- 
2.55.0.887.g758fc8c411-goog

Re: [PATCH v3 13/13] KVM: selftests: Add and use double-underscore versions of vmlaunch() and vmresume()

[Yosry Ahmed]

On Wed, Aug 26, 2026 at 4:39 PM Sean Christopherson <seanjc@google.com> wrote:
>
> Add double-underscore helpers for VMLAUNCH and VMRESUME, and have the
> "outer" APIs assert success, as is common through KVM selftests.  In
> addition to providing more familiar interfaces, this makes it more obvious
> which paths expect VM-Enter *failure*.

Nice.

>
> Signed-off-by: Sean Christopherson <seanjc@google.com>

Reviewed-by: Yosry Ahmed <yosry@kernel.org>

Few semi-related comments below.

[..]
> diff --git a/tools/testing/selftests/kvm/x86/aperfmperf_test.c b/tools/testing/selftests/kvm/x86/aperfmperf_test.c
> index 11f5894d8ac7..a082658c1e48 100644
> --- a/tools/testing/selftests/kvm/x86/aperfmperf_test.c
> +++ b/tools/testing/selftests/kvm/x86/aperfmperf_test.c
> @@ -83,7 +83,7 @@ static void l1_vmx_code(struct vmx_pages *vmx)
>                 vmreadz(CPU_BASED_VM_EXEC_CONTROL) | CPU_BASED_USE_MSR_BITMAPS);
>
>         GUEST_ASSERT(!vmwrite(GUEST_RIP, (u64)l2_guest_code));

Do we wanna give the same treatment to vmwrite()?

> -       GUEST_ASSERT(!vmlaunch());
> +       vmlaunch();
>  }
>
>  static void guest_code(void *nested_test_data)
[..]
> diff --git a/tools/testing/selftests/kvm/x86/nested_emulation_test.c b/tools/testing/selftests/kvm/x86/nested_emulation_test.c
> index 2b38eff4f516..11fe4a31eda0 100644
> --- a/tools/testing/selftests/kvm/x86/nested_emulation_test.c
> +++ b/tools/testing/selftests/kvm/x86/nested_emulation_test.c
> @@ -102,7 +102,7 @@ static void guest_code(void *test_data)
>                         exit_insn_len = vmcb->control.next_rip - vmcb->save.rip;
>                         GUEST_ASSERT_EQ(vmcb->save.rip, (u64)l2_instruction);
>                 } else {
> -                       GUEST_ASSERT_EQ(i ? vmresume() : vmlaunch(), 0);
> +                       i ? vmresume() : vmlaunch();

Ewww

>                         exit_reason = vmreadz(VM_EXIT_REASON);
>                         exit_insn_len = vmreadz(VM_EXIT_INSTRUCTION_LEN);
>                         GUEST_ASSERT_EQ(vmreadz(GUEST_RIP), (u64)l2_instruction);
> diff --git a/tools/testing/selftests/kvm/x86/nested_exceptions_test.c b/tools/testing/selftests/kvm/x86/nested_exceptions_test.c
> index 50c271a03692..c36c11b5f518 100644
> --- a/tools/testing/selftests/kvm/x86/nested_exceptions_test.c
> +++ b/tools/testing/selftests/kvm/x86/nested_exceptions_test.c
> @@ -112,7 +112,7 @@ static void vmx_run_l2(void *l2_code, int vector, u32 error_code)
>  {
>         GUEST_ASSERT(!vmwrite(GUEST_RIP, (u64)l2_code));
>
> -       GUEST_ASSERT_EQ(vector == SS_VECTOR ? vmlaunch() : vmresume(), 0);
> +       vector == SS_VECTOR ? vmlaunch() : vmresume();

Ewwwww

Can we make these if/else statements? It was already ugly, but it
looks even more ugly outside of GUEST_ASSERT_EQ().

>
>         if (vector == FAKE_TRIPLE_FAULT_VECTOR)
>                 return;

Re: [PATCH v3 13/13] KVM: selftests: Add and use double-underscore versions of vmlaunch() and vmresume()

[Sean Christopherson]

On Wed, Aug 26, 2026, Yosry Ahmed wrote:
> On Wed, Aug 26, 2026 at 4:39 PM Sean Christopherson <seanjc@google.com> wrote:
> > diff --git a/tools/testing/selftests/kvm/x86/aperfmperf_test.c b/tools/testing/selftests/kvm/x86/aperfmperf_test.c
> > index 11f5894d8ac7..a082658c1e48 100644
> > --- a/tools/testing/selftests/kvm/x86/aperfmperf_test.c
> > +++ b/tools/testing/selftests/kvm/x86/aperfmperf_test.c
> > @@ -83,7 +83,7 @@ static void l1_vmx_code(struct vmx_pages *vmx)
> >                 vmreadz(CPU_BASED_VM_EXEC_CONTROL) | CPU_BASED_USE_MSR_BITMAPS);
> >
> >         GUEST_ASSERT(!vmwrite(GUEST_RIP, (u64)l2_guest_code));
> 
> Do we wanna give the same treatment to vmwrite()?

Yes, and vmreadz().  Assuming '0' is simultaneously a reasonable failure and "safe"
value is so stupid, especially since AFAICT literally none of the users actually
need to gracefully tolerate failure.

LOL, and it can't possibly work, because vmread() clobbers the zeroed value on
VM-Fail.  Hilarious.

I didn't include those changes purely because I had "#$*@ this code" fatigue. :-)
But since this is going to conflict the world over, yeah, it makes sense to just
fix everything.

> > -       GUEST_ASSERT(!vmlaunch());
> > +       vmlaunch();
> >  }
> >
> >  static void guest_code(void *nested_test_data)
> [..]
> > diff --git a/tools/testing/selftests/kvm/x86/nested_emulation_test.c b/tools/testing/selftests/kvm/x86/nested_emulation_test.c
> > index 2b38eff4f516..11fe4a31eda0 100644
> > --- a/tools/testing/selftests/kvm/x86/nested_emulation_test.c
> > +++ b/tools/testing/selftests/kvm/x86/nested_emulation_test.c
> > @@ -102,7 +102,7 @@ static void guest_code(void *test_data)
> >                         exit_insn_len = vmcb->control.next_rip - vmcb->save.rip;
> >                         GUEST_ASSERT_EQ(vmcb->save.rip, (u64)l2_instruction);
> >                 } else {
> > -                       GUEST_ASSERT_EQ(i ? vmresume() : vmlaunch(), 0);
> > +                       i ? vmresume() : vmlaunch();
> 
> Ewww
> 
> >                         exit_reason = vmreadz(VM_EXIT_REASON);
> >                         exit_insn_len = vmreadz(VM_EXIT_INSTRUCTION_LEN);
> >                         GUEST_ASSERT_EQ(vmreadz(GUEST_RIP), (u64)l2_instruction);
> > diff --git a/tools/testing/selftests/kvm/x86/nested_exceptions_test.c b/tools/testing/selftests/kvm/x86/nested_exceptions_test.c
> > index 50c271a03692..c36c11b5f518 100644
> > --- a/tools/testing/selftests/kvm/x86/nested_exceptions_test.c
> > +++ b/tools/testing/selftests/kvm/x86/nested_exceptions_test.c
> > @@ -112,7 +112,7 @@ static void vmx_run_l2(void *l2_code, int vector, u32 error_code)
> >  {
> >         GUEST_ASSERT(!vmwrite(GUEST_RIP, (u64)l2_code));
> >
> > -       GUEST_ASSERT_EQ(vector == SS_VECTOR ? vmlaunch() : vmresume(), 0);
> > +       vector == SS_VECTOR ? vmlaunch() : vmresume();
> 
> Ewwwww
> 
> Can we make these if/else statements? It was already ugly, but it
> looks even more ugly outside of GUEST_ASSERT_EQ().

Yeah.  Another idea would be:

  void vmenter(bool do_vmlaunch)
  {
	if (do_vmlaunch)
		vmlaunch();
	else
		vmresume();
  }

To yield:

	vmenter(vector == SS_VECTOR);
	vmenter(!i);

Never mind, that's a terrible idea, the call sites are inscrutable.

Re: [PATCH v3 13/13] KVM: selftests: Add and use double-underscore versions of vmlaunch() and vmresume()

[Yosry Ahmed]

On Thu, Aug 27, 2026 at 1:28 PM Sean Christopherson <seanjc@google.com> wrote:
>
> On Wed, Aug 26, 2026, Yosry Ahmed wrote:
> > On Wed, Aug 26, 2026 at 4:39 PM Sean Christopherson <seanjc@google.com> wrote:
> > > diff --git a/tools/testing/selftests/kvm/x86/aperfmperf_test.c b/tools/testing/selftests/kvm/x86/aperfmperf_test.c
> > > index 11f5894d8ac7..a082658c1e48 100644
> > > --- a/tools/testing/selftests/kvm/x86/aperfmperf_test.c
> > > +++ b/tools/testing/selftests/kvm/x86/aperfmperf_test.c
> > > @@ -83,7 +83,7 @@ static void l1_vmx_code(struct vmx_pages *vmx)
> > >                 vmreadz(CPU_BASED_VM_EXEC_CONTROL) | CPU_BASED_USE_MSR_BITMAPS);
> > >
> > >         GUEST_ASSERT(!vmwrite(GUEST_RIP, (u64)l2_guest_code));
> >
> > Do we wanna give the same treatment to vmwrite()?
>
> Yes, and vmreadz().  Assuming '0' is simultaneously a reasonable failure and "safe"
> value is so stupid, especially since AFAICT literally none of the users actually
> need to gracefully tolerate failure.
>
> LOL, and it can't possibly work, because vmread() clobbers the zeroed value on
> VM-Fail.  Hilarious.
>
> I didn't include those changes purely because I had "#$*@ this code" fatigue. :-)
> But since this is going to conflict the world over, yeah, it makes sense to just
> fix everything.
>
> > > -       GUEST_ASSERT(!vmlaunch());
> > > +       vmlaunch();
> > >  }
> > >
> > >  static void guest_code(void *nested_test_data)
> > [..]
> > > diff --git a/tools/testing/selftests/kvm/x86/nested_emulation_test.c b/tools/testing/selftests/kvm/x86/nested_emulation_test.c
> > > index 2b38eff4f516..11fe4a31eda0 100644
> > > --- a/tools/testing/selftests/kvm/x86/nested_emulation_test.c
> > > +++ b/tools/testing/selftests/kvm/x86/nested_emulation_test.c
> > > @@ -102,7 +102,7 @@ static void guest_code(void *test_data)
> > >                         exit_insn_len = vmcb->control.next_rip - vmcb->save.rip;
> > >                         GUEST_ASSERT_EQ(vmcb->save.rip, (u64)l2_instruction);
> > >                 } else {
> > > -                       GUEST_ASSERT_EQ(i ? vmresume() : vmlaunch(), 0);
> > > +                       i ? vmresume() : vmlaunch();
> >
> > Ewww
> >
> > >                         exit_reason = vmreadz(VM_EXIT_REASON);
> > >                         exit_insn_len = vmreadz(VM_EXIT_INSTRUCTION_LEN);
> > >                         GUEST_ASSERT_EQ(vmreadz(GUEST_RIP), (u64)l2_instruction);
> > > diff --git a/tools/testing/selftests/kvm/x86/nested_exceptions_test.c b/tools/testing/selftests/kvm/x86/nested_exceptions_test.c
> > > index 50c271a03692..c36c11b5f518 100644
> > > --- a/tools/testing/selftests/kvm/x86/nested_exceptions_test.c
> > > +++ b/tools/testing/selftests/kvm/x86/nested_exceptions_test.c
> > > @@ -112,7 +112,7 @@ static void vmx_run_l2(void *l2_code, int vector, u32 error_code)
> > >  {
> > >         GUEST_ASSERT(!vmwrite(GUEST_RIP, (u64)l2_code));
> > >
> > > -       GUEST_ASSERT_EQ(vector == SS_VECTOR ? vmlaunch() : vmresume(), 0);
> > > +       vector == SS_VECTOR ? vmlaunch() : vmresume();
> >
> > Ewwwww
> >
> > Can we make these if/else statements? It was already ugly, but it
> > looks even more ugly outside of GUEST_ASSERT_EQ().
>
> Yeah.  Another idea would be:
>
>   void vmenter(bool do_vmlaunch)
>   {
>         if (do_vmlaunch)
>                 vmlaunch();
>         else
>                 vmresume();
>   }
>
> To yield:
>
>         vmenter(vector == SS_VECTOR);
>         vmenter(!i);
>
> Never mind, that's a terrible idea, the call sites are inscrutable.

Yeah, but I do agree that vmlaunch() vs vmresume() handling in
selftests is annoying (e.g. see run_l2() in [1]).

If you wanna go scorched earth on the nested VMX infra, what if we
track launch vs resume (e.g. set launch=true in prepare_vmcs() and
lauch=false in vmlaunch()), then provide a single vmenter() API for
tests (with no arguments)?

[1]https://lore.kernel.org/kvm/20260728003557.1136583-29-yosry@kernel.org/

Re: [PATCH v3 13/13] KVM: selftests: Add and use double-underscore versions of vmlaunch() and vmresume()

[Sean Christopherson]

On Thu, Aug 27, 2026, Yosry Ahmed wrote:
> On Thu, Aug 27, 2026 at 1:28 PM Sean Christopherson <seanjc@google.com> wrote:
> > Yeah.  Another idea would be:
> >
> >   void vmenter(bool do_vmlaunch)
> >   {
> >         if (do_vmlaunch)
> >                 vmlaunch();
> >         else
> >                 vmresume();
> >   }
> >
> > To yield:
> >
> >         vmenter(vector == SS_VECTOR);
> >         vmenter(!i);
> >
> > Never mind, that's a terrible idea, the call sites are inscrutable.
> 
> Yeah, but I do agree that vmlaunch() vs vmresume() handling in
> selftests is annoying (e.g. see run_l2() in [1]).
> 
> If you wanna go scorched earth on the nested VMX infra, what if we
> track launch vs resume (e.g. set launch=true in prepare_vmcs() and
> lauch=false in vmlaunch()), then provide a single vmenter() API for
> tests (with no arguments)?

For selftests, it would be much more difficult to do correctly, as compared to
KUT.  KUT has a "fixed" number of vCPUs, doesn't need to worry about pCPU:vCPU
bindings, and has a strong need for per-vCPU state for other reasons (because
it's more like per-pCPU state than per-vCPU state).

Selftests doesn't have per-vCPU state, so making the tracking thread-safe would
be challenging.  And I don't really want to add per-vCPU state because making
GS (or FS) off-limits is actually problematic when the goal is often to do "bad"
things to vCPU state (I've debugged weird KUT failures due to GS getting clobbered
more than once).
 
> [1]https://lore.kernel.org/kvm/20260728003557.1136583-29-yosry@kernel.org

Re: [PATCH v3 13/13] KVM: selftests: Add and use double-underscore versions of vmlaunch() and vmresume()

[Yosry Ahmed]

On Thu, Aug 27, 2026 at 1:48 PM Sean Christopherson <seanjc@google.com> wrote:
>
> On Thu, Aug 27, 2026, Yosry Ahmed wrote:
> > On Thu, Aug 27, 2026 at 1:28 PM Sean Christopherson <seanjc@google.com> wrote:
> > > Yeah.  Another idea would be:
> > >
> > >   void vmenter(bool do_vmlaunch)
> > >   {
> > >         if (do_vmlaunch)
> > >                 vmlaunch();
> > >         else
> > >                 vmresume();
> > >   }
> > >
> > > To yield:
> > >
> > >         vmenter(vector == SS_VECTOR);
> > >         vmenter(!i);
> > >
> > > Never mind, that's a terrible idea, the call sites are inscrutable.
> >
> > Yeah, but I do agree that vmlaunch() vs vmresume() handling in
> > selftests is annoying (e.g. see run_l2() in [1]).
> >
> > If you wanna go scorched earth on the nested VMX infra, what if we
> > track launch vs resume (e.g. set launch=true in prepare_vmcs() and
> > lauch=false in vmlaunch()), then provide a single vmenter() API for
> > tests (with no arguments)?
>
> For selftests, it would be much more difficult to do correctly, as compared to
> KUT.  KUT has a "fixed" number of vCPUs, doesn't need to worry about pCPU:vCPU
> bindings, and has a strong need for per-vCPU state for other reasons (because
> it's more like per-pCPU state than per-vCPU state).
>
> Selftests doesn't have per-vCPU state, so making the tracking thread-safe would
> be challenging.  And I don't really want to add per-vCPU state because making
> GS (or FS) off-limits is actually problematic when the goal is often to do "bad"
> things to vCPU state (I've debugged weird KUT failures due to GS getting clobbered
> more than once).

Isn't vmx_pages effectively per-vCPU state (for nested)?

>
> > [1]https://lore.kernel.org/kvm/20260728003557.1136583-29-yosry@kernel.org

Re: [PATCH v3 13/13] KVM: selftests: Add and use double-underscore versions of vmlaunch() and vmresume()

[Sean Christopherson]

On Thu, Aug 27, 2026, Yosry Ahmed wrote:
> On Thu, Aug 27, 2026 at 1:48 PM Sean Christopherson <seanjc@google.com> wrote:
> >
> > On Thu, Aug 27, 2026, Yosry Ahmed wrote:
> > > On Thu, Aug 27, 2026 at 1:28 PM Sean Christopherson <seanjc@google.com> wrote:
> > > > Yeah.  Another idea would be:
> > > >
> > > >   void vmenter(bool do_vmlaunch)
> > > >   {
> > > >         if (do_vmlaunch)
> > > >                 vmlaunch();
> > > >         else
> > > >                 vmresume();
> > > >   }
> > > >
> > > > To yield:
> > > >
> > > >         vmenter(vector == SS_VECTOR);
> > > >         vmenter(!i);
> > > >
> > > > Never mind, that's a terrible idea, the call sites are inscrutable.
> > >
> > > Yeah, but I do agree that vmlaunch() vs vmresume() handling in
> > > selftests is annoying (e.g. see run_l2() in [1]).
> > >
> > > If you wanna go scorched earth on the nested VMX infra, what if we
> > > track launch vs resume (e.g. set launch=true in prepare_vmcs() and
> > > lauch=false in vmlaunch()), then provide a single vmenter() API for
> > > tests (with no arguments)?
> >
> > For selftests, it would be much more difficult to do correctly, as compared to
> > KUT.  KUT has a "fixed" number of vCPUs, doesn't need to worry about pCPU:vCPU
> > bindings, and has a strong need for per-vCPU state for other reasons (because
> > it's more like per-pCPU state than per-vCPU state).
> >
> > Selftests doesn't have per-vCPU state, so making the tracking thread-safe would
> > be challenging.  And I don't really want to add per-vCPU state because making
> > GS (or FS) off-limits is actually problematic when the goal is often to do "bad"
> > things to vCPU state (I've debugged weird KUT failures due to GS getting clobbered
> > more than once).
> 
> Isn't vmx_pages effectively per-vCPU state (for nested)?

Yeah, but you said "with no arguments", there are tests that use helpers, and
IMO passing in a vmx_pages pointer is uglier than implicitly tracking launched
state. 

Re: [PATCH v3 13/13] KVM: selftests: Add and use double-underscore versions of vmlaunch() and vmresume()

[Yosry Ahmed]

On Thu, Aug 27, 2026 at 2:02 PM Sean Christopherson <seanjc@google.com> wrote:
>
> On Thu, Aug 27, 2026, Yosry Ahmed wrote:
> > On Thu, Aug 27, 2026 at 1:48 PM Sean Christopherson <seanjc@google.com> wrote:
> > >
> > > On Thu, Aug 27, 2026, Yosry Ahmed wrote:
> > > > On Thu, Aug 27, 2026 at 1:28 PM Sean Christopherson <seanjc@google.com> wrote:
> > > > > Yeah.  Another idea would be:
> > > > >
> > > > >   void vmenter(bool do_vmlaunch)
> > > > >   {
> > > > >         if (do_vmlaunch)
> > > > >                 vmlaunch();
> > > > >         else
> > > > >                 vmresume();
> > > > >   }
> > > > >
> > > > > To yield:
> > > > >
> > > > >         vmenter(vector == SS_VECTOR);
> > > > >         vmenter(!i);
> > > > >
> > > > > Never mind, that's a terrible idea, the call sites are inscrutable.
> > > >
> > > > Yeah, but I do agree that vmlaunch() vs vmresume() handling in
> > > > selftests is annoying (e.g. see run_l2() in [1]).
> > > >
> > > > If you wanna go scorched earth on the nested VMX infra, what if we
> > > > track launch vs resume (e.g. set launch=true in prepare_vmcs() and
> > > > lauch=false in vmlaunch()), then provide a single vmenter() API for
> > > > tests (with no arguments)?
> > >
> > > For selftests, it would be much more difficult to do correctly, as compared to
> > > KUT.  KUT has a "fixed" number of vCPUs, doesn't need to worry about pCPU:vCPU
> > > bindings, and has a strong need for per-vCPU state for other reasons (because
> > > it's more like per-pCPU state than per-vCPU state).
> > >
> > > Selftests doesn't have per-vCPU state, so making the tracking thread-safe would
> > > be challenging.  And I don't really want to add per-vCPU state because making
> > > GS (or FS) off-limits is actually problematic when the goal is often to do "bad"
> > > things to vCPU state (I've debugged weird KUT failures due to GS getting clobbered
> > > more than once).
> >
> > Isn't vmx_pages effectively per-vCPU state (for nested)?
>
> Yeah, but you said "with no arguments", there are tests that use helpers, and
> IMO passing in a vmx_pages pointer is uglier than implicitly tracking launched
> state.

That's fair.