[PATCH 0/2] KVM: nSVM: Minor post-war fixups

[PATCH 0/2] KVM: nSVM: Minor post-war fixups

[Yosry Ahmed]

A couple of fixups in the aftermath of all nSVM patches, the first one
is just a cleanup suggested offlist by Sean, and the second is a fix for
the test to make sure it's checking #GP on VMRUN not VMLOAD.

In all honestly, I am not sure *why* the test was passing and a #GP was
generated on VMLOAD with a very large but valid GPA. vls=1, so KVM
should not be intercepting VMLOAD (in which case it would inject the
#GP). A #NPF is generated on the VMLOAD, and through tracing I found out
that kvm_mmu_page_fault() returns 1 (RETRY) to npf_interception(). There
shouldn't be a corresponding memslot, so I am not sure if KVM stuffed an
invalid mapping in the NPTs, or if KVM did nothing and the CPU #GP due
to an infinite #NPF loop (although npf_interception() was only called
once). Anyway, figuring that out is irrelevant to the fixup, which makes
sure we're actually getting #GP on VMRUN.

Yosry Ahmed (2):
  KVM: nSVM: Simplify error handling of
    nested_svm_copy_vmcb12_to_cache()
  KVM: selftests: Actually check #GP on VMRUN with invalid vmcb12

 arch/x86/kvm/svm/nested.c                     | 23 +++++++-------
 .../kvm/x86/svm_nested_invalid_vmcb12_gpa.c   | 31 +++++++++----------
 2 files changed, 26 insertions(+), 28 deletions(-)


base-commit: 5128b972fb2801ad9aca54d990a75611ab5283a9
-- 
2.53.0.473.g4a7958ca14-goog

[PATCH 1/2] KVM: nSVM: Simplify error handling of nested_svm_copy_vmcb12_to_cache()

[Yosry Ahmed]

nested_svm_vmrun() currently stores the return value of
nested_svm_copy_vmcb12_to_cache() in a local variable 'err', separate
from the generally used 'ret' variable. This is done to have a single
call to kvm_skip_emulated_instruction(), such that we can store the
return value of kvm_skip_emulated_instruction() in 'ret', and then
re-check the return value of nested_svm_copy_vmcb12_to_cache() in 'err'.

The code is unnecessarily confusing. Instead, call
kvm_skip_emulated_instruction() in the failure path of
nested_svm_copy_vmcb12_to_cache() if the return value is not -EFAULT,
and drop 'err'.

Suggested-by: Sean Christopherson <seanjc@google.com>
Signed-off-by: Yosry Ahmed <yosry@kernel.org>
---
 arch/x86/kvm/svm/nested.c | 23 ++++++++++++-----------
 1 file changed, 12 insertions(+), 11 deletions(-)

diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
index b191c6cab57db..54227bacc12e4 100644
--- a/arch/x86/kvm/svm/nested.c
+++ b/arch/x86/kvm/svm/nested.c
@@ -1079,7 +1079,7 @@ static int nested_svm_copy_vmcb12_to_cache(struct kvm_vcpu *vcpu, u64 vmcb12_gpa
 int nested_svm_vmrun(struct kvm_vcpu *vcpu)
 {
 	struct vcpu_svm *svm = to_svm(vcpu);
-	int ret, err;
+	int ret;
 	u64 vmcb12_gpa;
 	struct vmcb *vmcb01 = svm->vmcb01.ptr;
 
@@ -1104,19 +1104,20 @@ int nested_svm_vmrun(struct kvm_vcpu *vcpu)
 		return -EINVAL;
 
 	vmcb12_gpa = svm->vmcb->save.rax;
-	err = nested_svm_copy_vmcb12_to_cache(vcpu, vmcb12_gpa);
-	if (err == -EFAULT) {
-		kvm_inject_gp(vcpu, 0);
-		return 1;
+	ret = nested_svm_copy_vmcb12_to_cache(vcpu, vmcb12_gpa);
+	if (ret) {
+		/*
+		 * Advance RIP if #GP or #UD are not injected, but otherwise
+		 * stop if copying and checking vmcb12 failed.
+		 */
+		if (ret == -EFAULT) {
+			kvm_inject_gp(vcpu, 0);
+			return 1;
+		}
+		return kvm_skip_emulated_instruction(vcpu);
 	}
 
-	/*
-	 * Advance RIP if #GP or #UD are not injected, but otherwise stop if
-	 * copying and checking vmcb12 failed.
-	 */
 	ret = kvm_skip_emulated_instruction(vcpu);
-	if (err)
-		return ret;
 
 	/*
 	 * Since vmcb01 is not in use, we can use it to store some of the L1
-- 
2.53.0.473.g4a7958ca14-goog

[PATCH 2/2] KVM: selftests: Actually check #GP on VMRUN with invalid vmcb12

[Yosry Ahmed]

in svm_nested_invalid_vmcb12_gpa test, run_guest() is called with an
unmappable vmcb12 GPA to make sure KVM injects a #GP. However,
run_guest() executes VMLOAD first, so the #GP does not actually come
from the VMRUN handler.

Execute VMRUN directly from L1 code with the invalid GPA instead of
calling into run_guest(), and have the #GP handler skip over it (instead
of fixing up the VMCBA GPA). A separate run_guest() call is then done
for the remaining test cases. Also assert that #GP happened on VMRUN to
avoid falling into the same problem.

Opportunisitically drop the GUEST_SYNC() from the #GP handler, as L1
already asserts gp_triggered is 1.

Signed-off-by: Yosry Ahmed <yosry@kernel.org>
---
 .../kvm/x86/svm_nested_invalid_vmcb12_gpa.c   | 31 +++++++++----------
 1 file changed, 14 insertions(+), 17 deletions(-)

diff --git a/tools/testing/selftests/kvm/x86/svm_nested_invalid_vmcb12_gpa.c b/tools/testing/selftests/kvm/x86/svm_nested_invalid_vmcb12_gpa.c
index c6d5f712120d1..8b681796b48ef 100644
--- a/tools/testing/selftests/kvm/x86/svm_nested_invalid_vmcb12_gpa.c
+++ b/tools/testing/selftests/kvm/x86/svm_nested_invalid_vmcb12_gpa.c
@@ -10,23 +10,25 @@
 
 #define L2_GUEST_STACK_SIZE 64
 
-#define SYNC_GP 101
-#define SYNC_L2_STARTED 102
+#define VMRUN_OPCODE 0x000f01d8
 
-u64 valid_vmcb12_gpa;
 int gp_triggered;
 
 static void guest_gp_handler(struct ex_regs *regs)
 {
+	unsigned char *insn = (unsigned char *)regs->rip;
+	u32 opcode = (insn[0] << 16) | (insn[1] << 8) | insn[2];
+
+	GUEST_ASSERT_EQ(opcode, VMRUN_OPCODE);
 	GUEST_ASSERT(!gp_triggered);
-	GUEST_SYNC(SYNC_GP);
+
 	gp_triggered = 1;
-	regs->rax = valid_vmcb12_gpa;
+	regs->rip += 3; /* Skip over VMRUN */
 }
 
 static void l2_guest_code(void)
 {
-	GUEST_SYNC(SYNC_L2_STARTED);
+	GUEST_SYNC(1);
 	vmcall();
 }
 
@@ -37,11 +39,12 @@ static void l1_guest_code(struct svm_test_data *svm, u64 invalid_vmcb12_gpa)
 	generic_svm_setup(svm, l2_guest_code,
 			  &l2_guest_stack[L2_GUEST_STACK_SIZE]);
 
-	valid_vmcb12_gpa = svm->vmcb_gpa;
+	asm volatile ("vmrun %[invalid_vmcb12_gpa]" :
+		      : [invalid_vmcb12_gpa] "a" (invalid_vmcb12_gpa)
+		      : "memory");
+	GUEST_ASSERT_EQ(gp_triggered, 1);
 
-	run_guest(svm->vmcb, invalid_vmcb12_gpa); /* #GP */
-
-	/* GP handler should jump here */
+	run_guest(svm->vmcb, svm->vmcb_gpa);
 	GUEST_ASSERT(svm->vmcb->control.exit_code == SVM_EXIT_VMMCALL);
 	GUEST_DONE();
 }
@@ -70,12 +73,6 @@ int main(int argc, char *argv[])
 	vcpu_alloc_svm(vm, &nested_gva);
 	vcpu_args_set(vcpu, 2, nested_gva, max_legal_gpa);
 
-	/* VMRUN with max_legal_gpa, KVM injects a #GP */
-	vcpu_run(vcpu);
-	TEST_ASSERT_KVM_EXIT_REASON(vcpu, KVM_EXIT_IO);
-	TEST_ASSERT_EQ(get_ucall(vcpu, &uc), UCALL_SYNC);
-	TEST_ASSERT_EQ(uc.args[1], SYNC_GP);
-
 	/*
 	 * Enter L2 (with a legit vmcb12 GPA), then overwrite vmcb12 GPA with
 	 * max_legal_gpa. KVM will fail to map vmcb12 on nested VM-Exit and
@@ -84,7 +81,7 @@ int main(int argc, char *argv[])
 	vcpu_run(vcpu);
 	TEST_ASSERT_KVM_EXIT_REASON(vcpu, KVM_EXIT_IO);
 	TEST_ASSERT_EQ(get_ucall(vcpu, &uc), UCALL_SYNC);
-	TEST_ASSERT_EQ(uc.args[1], SYNC_L2_STARTED);
+	TEST_ASSERT_EQ(uc.args[1], 1);
 
 	state = vcpu_save_state(vcpu);
 	state->nested.hdr.svm.vmcb_pa = max_legal_gpa;
-- 
2.53.0.473.g4a7958ca14-goog

Re: [PATCH 0/2] KVM: nSVM: Minor post-war fixups

[Yosry Ahmed]

On Thu, Mar 5, 2026 at 12:30 PM Yosry Ahmed <yosry@kernel.org> wrote:
>
> A couple of fixups in the aftermath of all nSVM patches, the first one
> is just a cleanup suggested offlist by Sean, and the second is a fix for
> the test to make sure it's checking #GP on VMRUN not VMLOAD.
>
> In all honestly, I am not sure *why* the test was passing and a #GP was
> generated on VMLOAD with a very large but valid GPA. vls=1, so KVM
> should not be intercepting VMLOAD (in which case it would inject the
> #GP). A #NPF is generated on the VMLOAD, and through tracing I found out
> that kvm_mmu_page_fault() returns 1 (RETRY) to npf_interception(). There
> shouldn't be a corresponding memslot, so I am not sure if KVM stuffed an
> invalid mapping in the NPTs, or if KVM did nothing and the CPU #GP due
> to an infinite #NPF loop (although npf_interception() was only called
> once). Anyway, figuring that out is irrelevant to the fixup, which makes
> sure we're actually getting #GP on VMRUN.

The answer is here:
https://lore.kernel.org/kvm/CAO9r8zPZ7ezHSHfksZPu4Bj8O7WTmDfO-Wu8fUAEebDFV4EoRw@mail.gmail.com/T/#u.

TL;DR the emulator is injecting the #GP, I didn't catch it initially
because I was tracing kvm_queue_exception_e() and I think it's being
inlined into inject_emulated_exception().

Anyway, ignore this version. I will send a new version with fixes for
#GP on non-existent vmcb12 GPA on top of patch 1, and then the test
patch will be replaced with a minor fix (to actually test VMRUN),
followed by a change to test the new behavior (emulation failure)
instead of #GP. I will probably also rename it from "invalid" vmcb12
to "unmappable" since all these discussions made the distinction more
clear architecturally. The test uses a valid GPA, just not one that
KVM can map because userspace did not create a memslot for it.

>
> Yosry Ahmed (2):
>   KVM: nSVM: Simplify error handling of
>     nested_svm_copy_vmcb12_to_cache()
>   KVM: selftests: Actually check #GP on VMRUN with invalid vmcb12
>
>  arch/x86/kvm/svm/nested.c                     | 23 +++++++-------
>  .../kvm/x86/svm_nested_invalid_vmcb12_gpa.c   | 31 +++++++++----------
>  2 files changed, 26 insertions(+), 28 deletions(-)
>
>
> base-commit: 5128b972fb2801ad9aca54d990a75611ab5283a9
> --
> 2.53.0.473.g4a7958ca14-goog
>