All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/4] KVM: nVMX: Adjust VMPTRLD/VMPTRST behavior with active eVMCS
@ 2026-08-18 16:52 Vitaly Kuznetsov
  2026-08-18 16:52 ` [PATCH v2 1/4] KVM: nVMX: Make VMPTRLD result in #UD when eVMCS is used Vitaly Kuznetsov
                   ` (3 more replies)
  0 siblings, 4 replies; 9+ messages in thread
From: Vitaly Kuznetsov @ 2026-08-18 16:52 UTC (permalink / raw)
  To: kvm, Sean Christopherson, Paolo Bonzini; +Cc: f734222792, linux-kernel

Changes since v1:
- 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]

It was reported (https://bugzilla.kernel.org/show_bug.cgi?id=221841) that
KVM's emulation of VMPTRLD/VMPTRST instructions when Enlightened VMCS is
active is sub-optimal: returning '1' does not make the L1 guest skip the
instruction so a hang is likely to happen. While TLFS does not really
specify the expected behavior, genuine Hyper-V seems to return #UD for
VMPTRLD and eVMCS pointer for VMPTRST. Alter the behavior in KVM and check
it in selftests.

Note: this is not a security issue. Misbehaving L1 can only do self-harm
by doing VMPTRLD/VMPTRST when it also uses eVMCS.

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 -----
 tools/testing/selftests/kvm/include/x86/vmx.h | 22 +++++++++----
 .../testing/selftests/kvm/x86/hyperv_evmcs.c  | 31 ++++++++++++++-----
 4 files changed, 56 insertions(+), 29 deletions(-)

-- 
2.55.0


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v2 1/4] KVM: nVMX: Make VMPTRLD result in #UD when eVMCS is used
  2026-08-18 16:52 [PATCH v2 0/4] KVM: nVMX: Adjust VMPTRLD/VMPTRST behavior with active eVMCS Vitaly Kuznetsov
@ 2026-08-18 16:52 ` Vitaly Kuznetsov
  2026-08-18 17:03   ` sashiko-bot
  2026-08-18 16:52 ` [PATCH v2 2/4] KVM: nVMX: Make VMPTRST return eVMCS GPA when it " Vitaly Kuznetsov
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 9+ messages in thread
From: Vitaly Kuznetsov @ 2026-08-18 16:52 UTC (permalink / raw)
  To: kvm, Sean Christopherson, Paolo Bonzini; +Cc: f734222792, linux-kernel

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>
---
 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 ddf6df7bee93..216f54a0b8ae 100644
--- a/arch/x86/kvm/vmx/nested.c
+++ b/arch/x86/kvm/vmx/nested.c
@@ -5879,6 +5879,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;
 
@@ -5888,10 +5894,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


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v2 2/4] KVM: nVMX: Make VMPTRST return eVMCS GPA when it is used
  2026-08-18 16:52 [PATCH v2 0/4] KVM: nVMX: Adjust VMPTRLD/VMPTRST behavior with active eVMCS Vitaly Kuznetsov
  2026-08-18 16:52 ` [PATCH v2 1/4] KVM: nVMX: Make VMPTRLD result in #UD when eVMCS is used Vitaly Kuznetsov
@ 2026-08-18 16:52 ` Vitaly Kuznetsov
  2026-08-18 16:52 ` [PATCH v2 3/4] KVM: selftests: Adapt to the updated VMPTRST behavior when eVMCS " Vitaly Kuznetsov
  2026-08-18 16:52 ` [PATCH v2 4/4] KVM: selftests: Check VMPTRLD with active eVMCS Vitaly Kuznetsov
  3 siblings, 0 replies; 9+ messages in thread
From: Vitaly Kuznetsov @ 2026-08-18 16:52 UTC (permalink / raw)
  To: kvm, Sean Christopherson, Paolo Bonzini; +Cc: f734222792, linux-kernel

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>
---
 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 216f54a0b8ae..4b56df4c57f1 100644
--- a/arch/x86/kvm/vmx/nested.c
+++ b/arch/x86/kvm/vmx/nested.c
@@ -5946,7 +5946,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;
@@ -5954,8 +5954,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


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v2 3/4] KVM: selftests: Adapt to the updated VMPTRST behavior when eVMCS is used
  2026-08-18 16:52 [PATCH v2 0/4] KVM: nVMX: Adjust VMPTRLD/VMPTRST behavior with active eVMCS Vitaly Kuznetsov
  2026-08-18 16:52 ` [PATCH v2 1/4] KVM: nVMX: Make VMPTRLD result in #UD when eVMCS is used Vitaly Kuznetsov
  2026-08-18 16:52 ` [PATCH v2 2/4] KVM: nVMX: Make VMPTRST return eVMCS GPA when it " Vitaly Kuznetsov
@ 2026-08-18 16:52 ` Vitaly Kuznetsov
  2026-08-18 16:52 ` [PATCH v2 4/4] KVM: selftests: Check VMPTRLD with active eVMCS Vitaly Kuznetsov
  3 siblings, 0 replies; 9+ messages in thread
From: Vitaly Kuznetsov @ 2026-08-18 16:52 UTC (permalink / raw)
  To: kvm, Sean Christopherson, Paolo Bonzini; +Cc: f734222792, linux-kernel

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>
---
 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 be79bda024bf..8b14a5dfa04b 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 90fffaf91595..047d02aa9688 100644
--- a/tools/testing/selftests/kvm/include/x86/vmx.h
+++ b/tools/testing/selftests/kvm/include/x86/vmx.h
@@ -341,9 +341,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 c7fa114aee20..88262ddf7fcb 100644
--- a/tools/testing/selftests/kvm/x86/hyperv_evmcs.c
+++ b/tools/testing/selftests/kvm/x86/hyperv_evmcs.c
@@ -95,16 +95,15 @@ 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,
 		     &l2_guest_stack[L2_GUEST_STACK_SIZE]);
 
 	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


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v2 4/4] KVM: selftests: Check VMPTRLD with active eVMCS
  2026-08-18 16:52 [PATCH v2 0/4] KVM: nVMX: Adjust VMPTRLD/VMPTRST behavior with active eVMCS Vitaly Kuznetsov
                   ` (2 preceding siblings ...)
  2026-08-18 16:52 ` [PATCH v2 3/4] KVM: selftests: Adapt to the updated VMPTRST behavior when eVMCS " Vitaly Kuznetsov
@ 2026-08-18 16:52 ` Vitaly Kuznetsov
  2026-08-18 17:05   ` sashiko-bot
  3 siblings, 1 reply; 9+ messages in thread
From: Vitaly Kuznetsov @ 2026-08-18 16:52 UTC (permalink / raw)
  To: kvm, Sean Christopherson, Paolo Bonzini; +Cc: f734222792, linux-kernel

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.

Assisted-by: Claude:claude-sonnet-4-6
Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.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 047d02aa9688..e6adde3970d4 100644
--- a/tools/testing/selftests/kvm/include/x86/vmx.h
+++ b/tools/testing/selftests/kvm/include/x86/vmx.h
@@ -325,9 +325,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)
@@ -336,6 +333,22 @@ static inline int vmptrld(u64 vmcs_pa)
 	return ret;
 }
 
+static inline int vmptrld_safe(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 88262ddf7fcb..cff27638834b 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)
@@ -182,7 +186,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_safe(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);
@@ -256,7 +268,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");
@@ -286,7 +297,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);
 		}
 
@@ -296,9 +307,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


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 1/4] KVM: nVMX: Make VMPTRLD result in #UD when eVMCS is used
  2026-08-18 16:52 ` [PATCH v2 1/4] KVM: nVMX: Make VMPTRLD result in #UD when eVMCS is used Vitaly Kuznetsov
@ 2026-08-18 17:03   ` sashiko-bot
  0 siblings, 0 replies; 9+ messages in thread
From: sashiko-bot @ 2026-08-18 17:03 UTC (permalink / raw)
  To: Vitaly Kuznetsov; +Cc: kvm

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] The patch resolves an infinite VM-exit loop for `VMPTRLD` by injecting a `#UD` exception when eVMCS is active, but fails to apply the same fix to `handle_vmptrst`. In `handle_vmptrst`, returning 1 without skipping the instruction or queuing an exception resumes the guest at the exact same instruction, leading to a permanent infinite VM-exit loop (soft lockup) for the vCPU.
--

--- Patch [1]: [PATCH v2 1/4] KVM: nVMX: Make VMPTRLD result in #UD when eVMCS is used ---
commit b2e2c0bdf82798edec492517d5390cf5a8348a25
Author: Vitaly Kuznetsov <vkuznets@redhat.com>

KVM: nVMX: Make VMPTRLD result in #UD when eVMCS is used

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.

Link: https://bugzilla.kernel.org/show_bug.cgi?id=221841

> diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c
> index 6957bb6f5cf7e..c78c67525ac9b 100644
> --- a/arch/x86/kvm/vmx/nested.c
> +++ b/arch/x86/kvm/vmx/nested.c
> @@ -5883,6 +5883,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;
>  
> @@ -5892,10 +5898,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;
> -

[Severity: High]
This isn't a bug introduced by this patch, but does handle_vmptrst() suffer
from the same issue that this patch fixes in handle_vmptrld()?

Looking at handle_vmptrst():

arch/x86/kvm/vmx/nested.c:handle_vmptrst() {
...
	if (unlikely(nested_vmx_is_evmptr12_valid(to_vmx(vcpu))))
		return 1;
...
}

Returning 1 here without skipping the instruction or queueing an exception
seems to resume the guest at the exact same VMPTRST instruction, potentially
leading to a permanent infinite VM-exit loop (soft lockup) for the vCPU.

Could handle_vmptrst() also be updated to inject a #UD exception?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260818165258.2613603-1-vkuznets@redhat.com?part=1

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 4/4] KVM: selftests: Check VMPTRLD with active eVMCS
  2026-08-18 16:52 ` [PATCH v2 4/4] KVM: selftests: Check VMPTRLD with active eVMCS Vitaly Kuznetsov
@ 2026-08-18 17:05   ` sashiko-bot
  2026-08-18 17:16     ` Sean Christopherson
  0 siblings, 1 reply; 9+ messages in thread
From: sashiko-bot @ 2026-08-18 17:05 UTC (permalink / raw)
  To: Vitaly Kuznetsov; +Cc: kvm

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Code relying on CPU flags after `KVM_ASM_SAFE` incorrectly evaluates them because the macro's cleanup instructions unconditionally clobber the flags.
--

--- Patch [4]: [PATCH v2 4/4] KVM: selftests: Check VMPTRLD with active eVMCS ---
commit d5821824a958c354147206d0cf6cf26eaeb71cc6
Author: Vitaly Kuznetsov <vkuznets@redhat.com>

KVM: selftests: Check VMPTRLD with active eVMCS

This commit verifies that VMPTRLD triggers a #UD when eVMCS is active.
It uses the KVM_ASM_SAFE framework to safely handle the expected
exceptions from VMPTRLD testing.

> diff --git a/tools/testing/selftests/kvm/include/x86/vmx.h b/tools/testing/selftests/kvm/include/x86/vmx.h
> index 047d02aa96889..e6adde3970d4d 100644
> --- a/tools/testing/selftests/kvm/include/x86/vmx.h
> +++ b/tools/testing/selftests/kvm/include/x86/vmx.h
[ ... ]
> @@ -336,6 +333,22 @@ static inline int vmptrld(u64 vmcs_pa)
>  	return ret;
>  }
>  
> +static inline int vmptrld_safe(u64 vmcs_pa)
> +{
> +	u64 error_code;
> +	u8 vector;
> +	u8 failed;
> +
> +	asm volatile(KVM_ASM_SAFE("vmptrld %[pa]")
> +		     "\n\tsetna %[failed]"

[Severity: Medium]
Will this setna instruction incorrectly evaluate CPU flags?

The KVM_ASM_SAFE macro executes xor %%r9, %%r9 on its success path, which
unconditionally clears the Carry Flag (CF=0) and sets the Zero Flag (ZF=1).

Because setna evaluates flags immediately after KVM_ASM_SAFE finishes its
cleanup instructions, it captures these clobbered flags and always evaluates
to true.

Does this cause vmptrld_safe() to incorrectly return -EINVAL upon successful
execution?

> +		     : 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;
> +}
> +

[Severity: Medium]
This isn't a bug introduced by this patch, but is a pre-existing issue in
guest_execute_fastop_div() located in
tools/testing/selftests/kvm/x86/fastops_test.c:

Does guest_execute_fastop_div() suffer from a similar flag evaluation issue?

__asm__ __volatile__(fastop(__KVM_ASM_SAFE(insn " %[denom]"))

The fastop macro invokes __KVM_ASM_SAFE, and captures flags via pushfq.
Does capturing flags after the macro result in saving the clobbered flags
from the internal xor instruction rather than the actual result of the
instruction under test?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260818165258.2613603-1-vkuznets@redhat.com?part=4

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 4/4] KVM: selftests: Check VMPTRLD with active eVMCS
  2026-08-18 17:05   ` sashiko-bot
@ 2026-08-18 17:16     ` Sean Christopherson
  2026-08-18 21:56       ` Sean Christopherson
  0 siblings, 1 reply; 9+ messages in thread
From: Sean Christopherson @ 2026-08-18 17:16 UTC (permalink / raw)
  To: sashiko-reviews; +Cc: Vitaly Kuznetsov, kvm

On Tue, Aug 18, 2026, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] Code relying on CPU flags after `KVM_ASM_SAFE` incorrectly evaluates them because the macro's cleanup instructions unconditionally clobber the flags.
> --
> 
> --- Patch [4]: [PATCH v2 4/4] KVM: selftests: Check VMPTRLD with active eVMCS ---
> commit d5821824a958c354147206d0cf6cf26eaeb71cc6
> Author: Vitaly Kuznetsov <vkuznets@redhat.com>
> 
> KVM: selftests: Check VMPTRLD with active eVMCS
> 
> This commit verifies that VMPTRLD triggers a #UD when eVMCS is active.
> It uses the KVM_ASM_SAFE framework to safely handle the expected
> exceptions from VMPTRLD testing.
> 
> > diff --git a/tools/testing/selftests/kvm/include/x86/vmx.h b/tools/testing/selftests/kvm/include/x86/vmx.h
> > index 047d02aa96889..e6adde3970d4d 100644
> > --- a/tools/testing/selftests/kvm/include/x86/vmx.h
> > +++ b/tools/testing/selftests/kvm/include/x86/vmx.h
> [ ... ]
> > @@ -336,6 +333,22 @@ static inline int vmptrld(u64 vmcs_pa)
> >  	return ret;
> >  }
> >  
> > +static inline int vmptrld_safe(u64 vmcs_pa)
> > +{
> > +	u64 error_code;
> > +	u8 vector;
> > +	u8 failed;
> > +
> > +	asm volatile(KVM_ASM_SAFE("vmptrld %[pa]")
> > +		     "\n\tsetna %[failed]"
> 
> [Severity: Medium]
> Will this setna instruction incorrectly evaluate CPU flags?
> 
> The KVM_ASM_SAFE macro executes xor %%r9, %%r9 on its success path, which
> unconditionally clears the Carry Flag (CF=0) and sets the Zero Flag (ZF=1).

Shame on me, I should know better after dealing with ba5ca5e5e6a1 ("x86/retpoline:
Don't clobber RFLAGS during srso_safe_ret()").

Untested, but I think the fix is simply:

diff --git tools/testing/selftests/kvm/include/x86/processor.h tools/testing/selftests/kvm/include/x86/processor.h
index 6e6f70035508..119715ffe93f 100644
--- tools/testing/selftests/kvm/include/x86/processor.h
+++ 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, %%r9\n\t"                                      \
        "2:\n\t"                                                \
        "mov  %%r9b, %[vector]\n\t"                             \
        "mov  %%r10, %[error_code]\n\t"

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 4/4] KVM: selftests: Check VMPTRLD with active eVMCS
  2026-08-18 17:16     ` Sean Christopherson
@ 2026-08-18 21:56       ` Sean Christopherson
  0 siblings, 0 replies; 9+ messages in thread
From: Sean Christopherson @ 2026-08-18 21:56 UTC (permalink / raw)
  To: sashiko-reviews; +Cc: Vitaly Kuznetsov, kvm

On Tue, Aug 18, 2026, Sean Christopherson wrote:
> On Tue, Aug 18, 2026, sashiko-bot@kernel.org wrote:
> > > +	asm volatile(KVM_ASM_SAFE("vmptrld %[pa]")
> > > +		     "\n\tsetna %[failed]"
> > 
> > [Severity: Medium]
> > Will this setna instruction incorrectly evaluate CPU flags?
> > 
> > The KVM_ASM_SAFE macro executes xor %%r9, %%r9 on its success path, which
> > unconditionally clears the Carry Flag (CF=0) and sets the Zero Flag (ZF=1).
> 
> Shame on me, I should know better after dealing with ba5ca5e5e6a1 ("x86/retpoline:
> Don't clobber RFLAGS during srso_safe_ret()").
> 
> Untested, but I think the fix is simply:
> 
> diff --git tools/testing/selftests/kvm/include/x86/processor.h tools/testing/selftests/kvm/include/x86/processor.h
> index 6e6f70035508..119715ffe93f 100644
> --- tools/testing/selftests/kvm/include/x86/processor.h
> +++ 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, %%r9\n\t"                                      \
>         "2:\n\t"                                                \
>         "mov  %%r9b, %[vector]\n\t"                             \
>         "mov  %%r10, %[error_code]\n\t"

Vitaly, I'll send a v3 of this series with a rather large pile of additional
selftests changes.  As often seems to be the case, my desire validate a simple
fix has snowballed...

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2026-08-18 21:56 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-18 16:52 [PATCH v2 0/4] KVM: nVMX: Adjust VMPTRLD/VMPTRST behavior with active eVMCS Vitaly Kuznetsov
2026-08-18 16:52 ` [PATCH v2 1/4] KVM: nVMX: Make VMPTRLD result in #UD when eVMCS is used Vitaly Kuznetsov
2026-08-18 17:03   ` sashiko-bot
2026-08-18 16:52 ` [PATCH v2 2/4] KVM: nVMX: Make VMPTRST return eVMCS GPA when it " Vitaly Kuznetsov
2026-08-18 16:52 ` [PATCH v2 3/4] KVM: selftests: Adapt to the updated VMPTRST behavior when eVMCS " Vitaly Kuznetsov
2026-08-18 16:52 ` [PATCH v2 4/4] KVM: selftests: Check VMPTRLD with active eVMCS Vitaly Kuznetsov
2026-08-18 17:05   ` sashiko-bot
2026-08-18 17:16     ` Sean Christopherson
2026-08-18 21:56       ` Sean Christopherson

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.