[PATCH v4 0/9] KVM: SVM: Fixes for VMCB12 checks and mapping

[PATCH v4 0/9] KVM: SVM: Fixes for VMCB12 checks and mapping

[Yosry Ahmed]

Jim pointed out that VMRUN/VMLOAD/VMSAVE injecting a #GP when the vmcb12
GPA is valid but not mappable is not architectural [1]. The series
handles them as emulation failures and (mostly) exits to userspace
instead. It also fixes the checks performed on the vmcb12 GPA (i.e. RAX)
in a few places.

Note that there's a few other bugs that this series leaves alone, mostly
to keep it focused on fixing the non-architectrual #GPs, this includes:
- KVM synthesizing #VMEXIT(VMLOAD/VMSAVE) to L1 when intercepting #GP
  from L2 on VMLOAD/VMSAVE, even if L1 does not intercept VMLOAD/VMSAVE
  (e.g. VLS enabled).
- KVM not respecting priority of #GP vs. #UD when EFER.SVME is disabled
  by the guest.

However, the series documents these bugs with FIXMEs so that they can
hopefully be fixed up later.

v3 -> v4:
- Fix the RAX check in the emulator instead of dropping it [Paolo].
- Add refactoring of SVM instructions on #GP intercept to facilitate
  following fixes.
- Use kvm_register_read() to handle 32-bit correctly and avoid false
  negatives.
- Do #UD and #GP checks before synthesizing #VMEXIT to L1 when KVM
  intercepts #GP on SVM instruction in L2.
- Update changelogs.

v3: https://lore.kernel.org/kvm/20260306210900.1933788-1-yosry@kernel.org/

[1]https://lore.kernel.org/kvm/CALMp9eSMtzDJn7tGtbj=zLYpcU7Tc7XjcWBRZH7Aa5YihSmN7g@mail.gmail.com/

Yosry Ahmed (9):
  KVM: SVM: Properly check RAX in the emulator for SVM instructions
  KVM: SVM: Refactor SVM instruction handling on #GP intercept
  KVM: SVM: Properly check RAX on #GP intercept of SVM instructions
  KVM: SVM: Move RAX legality check to SVM insn interception handlers
  KVM: SVM: Check EFER.SVME and CPL on #GP intercept of SVM instructions
  KVM: SVM: Treat mapping failures equally in VMLOAD/VMSAVE emulation
  KVM: nSVM: Fail emulation of VMRUN/VMLOAD/VMSAVE if mapping vmcb12
    fails
  KVM: selftests: Rework svm_nested_invalid_vmcb12_gpa
  KVM: selftests: Drop 'invalid' from svm_nested_invalid_vmcb12_gpa's
    name

 arch/x86/kvm/emulate.c                        |   3 +-
 arch/x86/kvm/kvm_emulate.h                    |   2 +
 arch/x86/kvm/svm/nested.c                     |  12 +-
 arch/x86/kvm/svm/svm.c                        | 107 +++++------
 arch/x86/kvm/x86.c                            |   6 +
 tools/testing/selftests/kvm/Makefile.kvm      |   2 +-
 .../kvm/x86/svm_nested_invalid_vmcb12_gpa.c   |  98 ----------
 .../selftests/kvm/x86/svm_nested_vmcb12_gpa.c | 176 ++++++++++++++++++
 8 files changed, 244 insertions(+), 162 deletions(-)
 delete mode 100644 tools/testing/selftests/kvm/x86/svm_nested_invalid_vmcb12_gpa.c
 create mode 100644 tools/testing/selftests/kvm/x86/svm_nested_vmcb12_gpa.c


base-commit: 3d6cdcc8883b5726513d245eef0e91cabfc397f7
-- 
2.53.0.851.ga537e3e6e9-goog

[PATCH v4 1/9] KVM: SVM: Properly check RAX in the emulator for SVM instructions

[Yosry Ahmed]

Architecturally, VMRUN/VMLOAD/VMSAVE should generate a #GP if the
physical address in RAX is not supported. check_svme_pa() hardcodes this
to checking that bits 63-48 are not set. This is incorrect on HW
supporting 52 bits of physical address space. Additionally, the emulator
does not check if the address is not aligned, which should also result
in #GP.

Use page_address_valid() which properly checks alignment and the address
legality based on the guest's MAXPHYADDR. Plumb it through
x86_emulate_ops, similar to is_canonical_addr(), to avoid directly
accessing the vCPU object in emulator code.

Fixes: 01de8b09e606 ("KVM: SVM: Add intercept checks for SVM instructions")
Suggested-by: Sean Christopherson <seanjc@google.com>
Signed-off-by: Yosry Ahmed <yosry@kernel.org>
---
 arch/x86/kvm/emulate.c     | 3 +--
 arch/x86/kvm/kvm_emulate.h | 2 ++
 arch/x86/kvm/x86.c         | 6 ++++++
 3 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index 6145dac4a605a..c8c6cc0406d6d 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -3887,8 +3887,7 @@ static int check_svme_pa(struct x86_emulate_ctxt *ctxt)
 {
 	u64 rax = reg_read(ctxt, VCPU_REGS_RAX);
 
-	/* Valid physical address? */
-	if (rax & 0xffff000000000000ULL)
+	if (!ctxt->ops->page_address_valid(ctxt, rax))
 		return emulate_gp(ctxt, 0);
 
 	return check_svme(ctxt);
diff --git a/arch/x86/kvm/kvm_emulate.h b/arch/x86/kvm/kvm_emulate.h
index fb3dab4b5a53e..0abff36d09942 100644
--- a/arch/x86/kvm/kvm_emulate.h
+++ b/arch/x86/kvm/kvm_emulate.h
@@ -245,6 +245,8 @@ struct x86_emulate_ops {
 
 	bool (*is_canonical_addr)(struct x86_emulate_ctxt *ctxt, gva_t addr,
 				  unsigned int flags);
+
+	bool (*page_address_valid)(struct x86_emulate_ctxt *ctxt, gpa_t gpa);
 };
 
 /* Type, address-of, and value of an instruction's operand. */
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 0b5d48e75b657..11d5bd84e323d 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -8916,6 +8916,11 @@ static bool emulator_is_canonical_addr(struct x86_emulate_ctxt *ctxt,
 	return !is_noncanonical_address(addr, emul_to_vcpu(ctxt), flags);
 }
 
+static bool emulator_page_address_valid(struct x86_emulate_ctxt *ctxt, gpa_t gpa)
+{
+	return page_address_valid(emul_to_vcpu(ctxt), gpa);
+}
+
 static const struct x86_emulate_ops emulate_ops = {
 	.vm_bugged           = emulator_vm_bugged,
 	.read_gpr            = emulator_read_gpr,
@@ -8963,6 +8968,7 @@ static const struct x86_emulate_ops emulate_ops = {
 	.set_xcr             = emulator_set_xcr,
 	.get_untagged_addr   = emulator_get_untagged_addr,
 	.is_canonical_addr   = emulator_is_canonical_addr,
+	.page_address_valid  = emulator_page_address_valid,
 };
 
 static void toggle_interruptibility(struct kvm_vcpu *vcpu, u32 mask)
-- 
2.53.0.851.ga537e3e6e9-goog

[PATCH v4 2/9] KVM: SVM: Refactor SVM instruction handling on #GP intercept

[Yosry Ahmed]

Instead of returning an opcode from svm_instr_opcode() and then passing
it to emulate_svm_instr(), which uses it to find the corresponding exit
code and intercept handler, return the exit code directly from
svm_instr_opcode(), and rename it to svm_instr_exit_code().

emulate_svm_instr() boils down to synthesizing a #VMEXIT or calling the
intercept handler, so open-code it in gp_interception(), and use
svm_invoke_exit_handler() to call the intercept handler based on
the exit code. This allows for dropping the SVM_INSTR_* enum, and the
const array mapping its values to exit codes and intercept handlers.

In gp_intercept(), handle SVM instructions first with an early return,
un-indenting the rest of the code.

No functional change intended.

Signed-off-by: Yosry Ahmed <yosry@kernel.org>
---
 arch/x86/kvm/svm/svm.c | 78 +++++++++++++++---------------------------
 1 file changed, 27 insertions(+), 51 deletions(-)

diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index d2ca226871c2f..392a5088f20bf 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -2233,54 +2233,26 @@ static int vmrun_interception(struct kvm_vcpu *vcpu)
 	return nested_svm_vmrun(vcpu);
 }
 
-enum {
-	NONE_SVM_INSTR,
-	SVM_INSTR_VMRUN,
-	SVM_INSTR_VMLOAD,
-	SVM_INSTR_VMSAVE,
-};
-
-/* Return NONE_SVM_INSTR if not SVM instrs, otherwise return decode result */
-static int svm_instr_opcode(struct kvm_vcpu *vcpu)
+/* Return 0 if not SVM instr, otherwise return associated exit_code */
+static u64 svm_instr_exit_code(struct kvm_vcpu *vcpu)
 {
 	struct x86_emulate_ctxt *ctxt = vcpu->arch.emulate_ctxt;
 
 	if (ctxt->b != 0x1 || ctxt->opcode_len != 2)
-		return NONE_SVM_INSTR;
+		return 0;
 
 	switch (ctxt->modrm) {
 	case 0xd8: /* VMRUN */
-		return SVM_INSTR_VMRUN;
+		return SVM_EXIT_VMRUN;
 	case 0xda: /* VMLOAD */
-		return SVM_INSTR_VMLOAD;
+		return SVM_EXIT_VMLOAD;
 	case 0xdb: /* VMSAVE */
-		return SVM_INSTR_VMSAVE;
+		return SVM_EXIT_VMSAVE;
 	default:
 		break;
 	}
 
-	return NONE_SVM_INSTR;
-}
-
-static int emulate_svm_instr(struct kvm_vcpu *vcpu, int opcode)
-{
-	const int guest_mode_exit_codes[] = {
-		[SVM_INSTR_VMRUN] = SVM_EXIT_VMRUN,
-		[SVM_INSTR_VMLOAD] = SVM_EXIT_VMLOAD,
-		[SVM_INSTR_VMSAVE] = SVM_EXIT_VMSAVE,
-	};
-	int (*const svm_instr_handlers[])(struct kvm_vcpu *vcpu) = {
-		[SVM_INSTR_VMRUN] = vmrun_interception,
-		[SVM_INSTR_VMLOAD] = vmload_interception,
-		[SVM_INSTR_VMSAVE] = vmsave_interception,
-	};
-	struct vcpu_svm *svm = to_svm(vcpu);
-
-	if (is_guest_mode(vcpu)) {
-		nested_svm_simple_vmexit(svm, guest_mode_exit_codes[opcode]);
-		return 1;
-	}
-	return svm_instr_handlers[opcode](vcpu);
+	return 0;
 }
 
 /*
@@ -2295,7 +2267,7 @@ static int gp_interception(struct kvm_vcpu *vcpu)
 {
 	struct vcpu_svm *svm = to_svm(vcpu);
 	u32 error_code = svm->vmcb->control.exit_info_1;
-	int opcode;
+	u64 svm_exit_code;
 
 	/* Both #GP cases have zero error_code */
 	if (error_code)
@@ -2305,27 +2277,31 @@ static int gp_interception(struct kvm_vcpu *vcpu)
 	if (x86_decode_emulated_instruction(vcpu, 0, NULL, 0) != EMULATION_OK)
 		goto reinject;
 
-	opcode = svm_instr_opcode(vcpu);
-
-	if (opcode == NONE_SVM_INSTR) {
-		if (!enable_vmware_backdoor)
-			goto reinject;
-
-		/*
-		 * VMware backdoor emulation on #GP interception only handles
-		 * IN{S}, OUT{S}, and RDPMC.
-		 */
-		if (!is_guest_mode(vcpu))
-			return kvm_emulate_instruction(vcpu,
-				EMULTYPE_VMWARE_GP | EMULTYPE_NO_DECODE);
-	} else {
+	svm_exit_code = svm_instr_exit_code(vcpu);
+	if (svm_exit_code) {
 		/* All SVM instructions expect page aligned RAX */
 		if (svm->vmcb->save.rax & ~PAGE_MASK)
 			goto reinject;
 
-		return emulate_svm_instr(vcpu, opcode);
+		if (is_guest_mode(vcpu)) {
+			nested_svm_simple_vmexit(svm, svm_exit_code);
+			return 1;
+		}
+
+		return svm_invoke_exit_handler(vcpu, svm_exit_code);
 	}
 
+	if (!enable_vmware_backdoor)
+		goto reinject;
+
+	/*
+	 * VMware backdoor emulation on #GP interception only handles
+	 * IN{S}, OUT{S}, and RDPMC.
+	 */
+	if (!is_guest_mode(vcpu))
+		return kvm_emulate_instruction(vcpu,
+				EMULTYPE_VMWARE_GP | EMULTYPE_NO_DECODE);
+
 reinject:
 	kvm_queue_exception_e(vcpu, GP_VECTOR, error_code);
 	return 1;
-- 
2.53.0.851.ga537e3e6e9-goog

[PATCH v4 3/9] KVM: SVM: Properly check RAX on #GP intercept of SVM instructions

[Yosry Ahmed]

When KVM intercepts #GP on an SVM instruction, it re-injects the #GP if
the instruction was executed with a mis-algined RAX. However, a #GP
should also be reinjected if RAX contains an illegal GPA, according to
the APM, one of #GP conditions is:

  rAX referenced a physical address above the maximum
  supported physical address.

Replace the PAGE_MASK check with page_address_valid(), which checks both
page-alignment as well as the legality of the GPA based on the vCPU's
MAXPHYADDR. Use kvm_register_read() to read RAX to avoid
page_address_valid() failing on 32-bit due to garbage in the higher
bits.

Note that this is currently only a problem if KVM is running an L2 guest
and ends up synthesizing a #VMEXIT to L1, as the RAX check takes
precedence over the intercept. Otherwise, if KVM emulates the
instruction, kvm_vcpu_map() should fail on illegal GPAs and inject a #GP
anyway. However, following patches will change the failure behavior of
kvm_vcpu_map(), so make sure the #GP interception handler does this
appropriately.

Opportunistically drop a teaser FIXME about the SVM instructions
handling on #GP belonging in the emulator.

Fixes: 82a11e9c6fa2 ("KVM: SVM: Add emulation support for #GP triggered by SVM instructions")
Fixes: d1cba6c92237 ("KVM: x86: nSVM: test eax for 4K alignment for GP errata workaround")
Suggested-by: Sean Christopherson <seanjc@google.com>
Signed-off-by: Yosry Ahmed <yosry@kernel.org>
---
 arch/x86/kvm/svm/svm.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index 392a5088f20bf..3122a98745ab7 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -2277,10 +2277,12 @@ static int gp_interception(struct kvm_vcpu *vcpu)
 	if (x86_decode_emulated_instruction(vcpu, 0, NULL, 0) != EMULATION_OK)
 		goto reinject;
 
+	/* FIXME: Handle SVM instructions through the emulator */
 	svm_exit_code = svm_instr_exit_code(vcpu);
 	if (svm_exit_code) {
-		/* All SVM instructions expect page aligned RAX */
-		if (svm->vmcb->save.rax & ~PAGE_MASK)
+		unsigned long rax = kvm_register_read(vcpu, VCPU_REGS_RAX);
+
+		if (!page_address_valid(vcpu, rax))
 			goto reinject;
 
 		if (is_guest_mode(vcpu)) {
-- 
2.53.0.851.ga537e3e6e9-goog

[PATCH v4 4/9] KVM: SVM: Move RAX legality check to SVM insn interception handlers

[Yosry Ahmed]

When #GP is intercepted by KVM, the #GP interception handler checks
whether the GPA in RAX is legal and reinjects the #GP accordingly.
Otherwise, it calls into the appropriate interception handler for
VMRUN/VMLOAD/VMSAVE. The intercept handlers do not check RAX.

However, the intercept handlers need to do the RAX check, because if the
guest has a smaller MAXPHYADDR, RAX could be legal from the hardware
perspective (i.e. CPU does not inject #GP), but not from the vCPU's
perspective. Note that with allow_smaller_maxphyaddr, both NPT and VLS
cannot be used, so VMLOAD/VMSAVE have to be intercepted, and RAX can
always be checked against the vCPU's MAXPHYADDR.

Move the check into the interception handlers for VMRUN/VMLOAD/VMSAVE as
the CPU does not check RAX before the interception. Read RAX using
kvm_register_read() to avoid a false negative on page_address_valid() on
32-bit due to garbage in the higher bits.

Keep the check in the #GP intercept handler in the nested case where
a #VMEXIT is synthesized into L1, as the RAX check is still needed there
and takes precedence over the intercept.

Opportunistically add a FIXME about the #VMEXIT being synthesized into
L1, as it needs to be conditional.

Signed-off-by: Yosry Ahmed <yosry@kernel.org>
---
 arch/x86/kvm/svm/nested.c |  6 +++++-
 arch/x86/kvm/svm/svm.c    | 21 +++++++++++++++++----
 2 files changed, 22 insertions(+), 5 deletions(-)

diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
index 5ff01d2ac85e4..75943a607777c 100644
--- a/arch/x86/kvm/svm/nested.c
+++ b/arch/x86/kvm/svm/nested.c
@@ -1113,7 +1113,11 @@ int nested_svm_vmrun(struct kvm_vcpu *vcpu)
 	if (WARN_ON_ONCE(!svm->nested.initialized))
 		return -EINVAL;
 
-	vmcb12_gpa = svm->vmcb->save.rax;
+	vmcb12_gpa = kvm_register_read(vcpu, VCPU_REGS_RAX);
+	if (!page_address_valid(vcpu, vmcb12_gpa)) {
+		kvm_inject_gp(vcpu, 0);
+		return 1;
+	}
 
 	ret = nested_svm_copy_vmcb12_to_cache(vcpu, vmcb12_gpa);
 	if (ret) {
diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index 3122a98745ab7..a511ee1139725 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -2182,6 +2182,7 @@ static int intr_interception(struct kvm_vcpu *vcpu)
 
 static int vmload_vmsave_interception(struct kvm_vcpu *vcpu, bool vmload)
 {
+	u64 vmcb12_gpa = kvm_register_read(vcpu, VCPU_REGS_RAX);
 	struct vcpu_svm *svm = to_svm(vcpu);
 	struct vmcb *vmcb12;
 	struct kvm_host_map map;
@@ -2190,7 +2191,12 @@ static int vmload_vmsave_interception(struct kvm_vcpu *vcpu, bool vmload)
 	if (nested_svm_check_permissions(vcpu))
 		return 1;
 
-	ret = kvm_vcpu_map(vcpu, gpa_to_gfn(svm->vmcb->save.rax), &map);
+	if (!page_address_valid(vcpu, vmcb12_gpa)) {
+		kvm_inject_gp(vcpu, 0);
+		return 1;
+	}
+
+	ret = kvm_vcpu_map(vcpu, gpa_to_gfn(vmcb12_gpa), &map);
 	if (ret) {
 		if (ret == -EINVAL)
 			kvm_inject_gp(vcpu, 0);
@@ -2282,10 +2288,17 @@ static int gp_interception(struct kvm_vcpu *vcpu)
 	if (svm_exit_code) {
 		unsigned long rax = kvm_register_read(vcpu, VCPU_REGS_RAX);
 
-		if (!page_address_valid(vcpu, rax))
-			goto reinject;
-
 		if (is_guest_mode(vcpu)) {
+			if (!page_address_valid(vcpu, rax))
+				goto reinject;
+
+			/*
+			 * FIXME: Only synthesize a #VMEXIT if L1 sets the
+			 * intercept, but only after the VMLOAD/VMSAVE exit
+			 * handlers can properly handle VMLOAD/VMSAVE from L2
+			 * with VLS enabled in L1 (i.e. RAX is an L2 GPA that
+			 * needs translation through L1's NPT).
+			 */
 			nested_svm_simple_vmexit(svm, svm_exit_code);
 			return 1;
 		}
-- 
2.53.0.851.ga537e3e6e9-goog

[PATCH v4 5/9] KVM: SVM: Check EFER.SVME and CPL on #GP intercept of SVM instructions

[Yosry Ahmed]

When KVM intercepts #GP on an SVM instruction from L2, it checks the
legality of RAX, and injects a #GP if RAX is illegal, or otherwise
synthesizes a #VMEXIT to L1. However, checking EFER.SVME and CPL takes
precedence over both the RAX check and the intercept. Call
nested_svm_check_permissions() first to cover both.

Note that if #GP is intercepted on SVM instruction in L1, the intercept
handlers of VMRUN/VMLOAD/VMSAVE already perform these checks.

Note #2, if KVM does not intercept #GP, the check for EFER.SVME is not
done in the correct order, because KVM handles it by intercepting the
instructions when EFER.SVME=0 and injecting #UD.  However, a #GP
injected by hardware would happen before the instruction intercept,
leading to #GP taking precedence over #UD from the guest's perspective.
Opportunistically add a FIXME for this.

Fixes: 82a11e9c6fa2 ("KVM: SVM: Add emulation support for #GP triggered by SVM instructions")
Signed-off-by: Yosry Ahmed <yosry@kernel.org>
---
 arch/x86/kvm/svm/svm.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index a511ee1139725..bb0bb0f9c858f 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -1052,6 +1052,11 @@ static void svm_recalc_instruction_intercepts(struct kvm_vcpu *vcpu)
 	 * No need to toggle any of the vgif/vls/etc. enable bits here, as they
 	 * are set when the VMCB is initialized and never cleared (if the
 	 * relevant intercepts are set, the enablements are meaningless anyway).
+	 *
+	 * FIXME: When #GP is not intercepted, a #GP on these instructions (e.g.
+	 * due to CPL > 0) could be injected by hardware before the instruction
+	 * is intercepted, leading to #GP taking precedence over #UD from the
+	 * guest's perspective.
 	 */
 	if (!(vcpu->arch.efer & EFER_SVME)) {
 		svm_set_intercept(svm, INTERCEPT_VMLOAD);
@@ -2289,6 +2294,9 @@ static int gp_interception(struct kvm_vcpu *vcpu)
 		unsigned long rax = kvm_register_read(vcpu, VCPU_REGS_RAX);
 
 		if (is_guest_mode(vcpu)) {
+			if (nested_svm_check_permissions(vcpu))
+				return 1;
+
 			if (!page_address_valid(vcpu, rax))
 				goto reinject;
 
-- 
2.53.0.851.ga537e3e6e9-goog

[PATCH v4 6/9] KVM: SVM: Treat mapping failures equally in VMLOAD/VMSAVE emulation

[Yosry Ahmed]

Currently, a #GP is only injected if kvm_vcpu_map() fails with -EINVAL.
But it could also fail with -EFAULT if creating a host mapping failed.
Inject a #GP in all cases, no reason to treat failure modes differently.

Similar to commit 01ddcdc55e09 ("KVM: nSVM: Always inject a #GP if
mapping VMCB12 fails on nested VMRUN"), treat all failures equally.

Fixes: 8c5fbf1a7231 ("KVM/nSVM: Use the new mapping API for mapping guest memory")
Signed-off-by: Yosry Ahmed <yosry@kernel.org>
---
 arch/x86/kvm/svm/svm.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index bb0bb0f9c858f..9f6f60fc8c133 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -2201,10 +2201,8 @@ static int vmload_vmsave_interception(struct kvm_vcpu *vcpu, bool vmload)
 		return 1;
 	}
 
-	ret = kvm_vcpu_map(vcpu, gpa_to_gfn(vmcb12_gpa), &map);
-	if (ret) {
-		if (ret == -EINVAL)
-			kvm_inject_gp(vcpu, 0);
+	if (kvm_vcpu_map(vcpu, gpa_to_gfn(vmcb12_gpa), &map)) {
+		kvm_inject_gp(vcpu, 0);
 		return 1;
 	}
 
-- 
2.53.0.851.ga537e3e6e9-goog

[PATCH v4 7/9] KVM: nSVM: Fail emulation of VMRUN/VMLOAD/VMSAVE if mapping vmcb12 fails

[Yosry Ahmed]

KVM currently injects a #GP if mapping vmcb12 fails when emulating
VMRUN/VMLOAD/VMSAVE. This is not architectural behavior, as #GP should
only be injected if the physical address is not supported or not
aligned. Instead, handle it as an emulation failure, similar to how nVMX
handles failures to read/write guest memory in several emulation paths.

When virtual VMLOAD/VMSAVE is enabled, if vmcb12's GPA is not mapped in
the NPTs a VMEXIT(#NPF) will be generated, and KVM will install an MMIO
SPTE and emulate the instruction if there is no corresponding memslot.
x86_emulate_insn() will return EMULATION_FAILED as VMLOAD/VMSAVE are not
handled as part of the twobyte_insn cases.

Even though this will also result in an emulation failure, it will only
result in a straight return to userspace if
KVM_CAP_EXIT_ON_EMULATION_FAILURE is set. Otherwise, it would inject #UD
and only exit to userspace if not in guest mode. So the behavior is
slightly different if virtual VMLOAD/VMSAVE is enabled.

Fixes: 3d6368ef580a ("KVM: SVM: Add VMRUN handler")
Reported-by: Jim Mattson <jmattson@google.com>
Signed-off-by: Yosry Ahmed <yosry@kernel.org>
---
 arch/x86/kvm/svm/nested.c | 6 ++----
 arch/x86/kvm/svm/svm.c    | 6 ++----
 2 files changed, 4 insertions(+), 8 deletions(-)

diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
index 75943a607777c..73c0df0a81f0e 100644
--- a/arch/x86/kvm/svm/nested.c
+++ b/arch/x86/kvm/svm/nested.c
@@ -1121,10 +1121,8 @@ int nested_svm_vmrun(struct kvm_vcpu *vcpu)
 
 	ret = nested_svm_copy_vmcb12_to_cache(vcpu, vmcb12_gpa);
 	if (ret) {
-		if (ret == -EFAULT) {
-			kvm_inject_gp(vcpu, 0);
-			return 1;
-		}
+		if (ret == -EFAULT)
+			return kvm_handle_memory_failure(vcpu, X86EMUL_IO_NEEDED, NULL);
 
 		/* Advance RIP past VMRUN as part of the nested #VMEXIT. */
 		return kvm_skip_emulated_instruction(vcpu);
diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index 9f6f60fc8c133..1843627fa4cac 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -2201,10 +2201,8 @@ static int vmload_vmsave_interception(struct kvm_vcpu *vcpu, bool vmload)
 		return 1;
 	}
 
-	if (kvm_vcpu_map(vcpu, gpa_to_gfn(vmcb12_gpa), &map)) {
-		kvm_inject_gp(vcpu, 0);
-		return 1;
-	}
+	if (kvm_vcpu_map(vcpu, gpa_to_gfn(vmcb12_gpa), &map))
+		return kvm_handle_memory_failure(vcpu, X86EMUL_IO_NEEDED, NULL);
 
 	vmcb12 = map.hva;
 
-- 
2.53.0.851.ga537e3e6e9-goog

[PATCH v4 8/9] KVM: selftests: Rework svm_nested_invalid_vmcb12_gpa

[Yosry Ahmed]

The test currently allegedly makes sure that VMRUN causes a #GP in
vmcb12 GPA is valid but unmappable. However, it calls run_guest() with
an the test vmcb12 GPA, and the #GP is produced from VMLOAD, not VMRUN.

Additionally, the underlying logic just changed to match architectural
behavior, and all of VMRUN/VMLOAD/VMSAVE fail emulation if vmcb12 cannot
be mapped. The CPU still injects a #GP if the vmcb12 GPA exceeds
maxphyaddr.

Rework the test such to use the KVM_ONE_VCPU_TEST[_SUITE] harness, and
test all of VMRUN/VMLOAD/VMSAVE with both an invalid GPA (-1ULL) causing
a #GP, and a valid but unmappable GPA causing emulation failure. Execute
the instructions directly from L1 instead of run_guest() to make sure
the #GP or emulation failure is produced by the right instruction.

Leave the #VMEXIT with unmappable GPA test case as-is, but wrap it with
a test harness as well.

Opportunisitically drop gp_triggered, as the test already checks that
a #GP was injected through a SYNC. Also, use the first unmapped GPA
instead of the maximum legal GPA, as some CPUs inject a #GP for the
maximum legal GPA (likely in a reserved area).

Signed-off-by: Yosry Ahmed <yosry@kernel.org>
---
 .../kvm/x86/svm_nested_invalid_vmcb12_gpa.c   | 152 +++++++++++++-----
 1 file changed, 115 insertions(+), 37 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..569869bed20b5 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
@@ -6,6 +6,8 @@
 #include "vmx.h"
 #include "svm_util.h"
 #include "kselftest.h"
+#include "kvm_test_harness.h"
+#include "test_util.h"
 
 
 #define L2_GUEST_STACK_SIZE 64
@@ -13,86 +15,162 @@
 #define SYNC_GP 101
 #define SYNC_L2_STARTED 102
 
-u64 valid_vmcb12_gpa;
-int gp_triggered;
+static unsigned long l2_guest_stack[L2_GUEST_STACK_SIZE];
 
 static void guest_gp_handler(struct ex_regs *regs)
 {
-	GUEST_ASSERT(!gp_triggered);
 	GUEST_SYNC(SYNC_GP);
-	gp_triggered = 1;
-	regs->rax = valid_vmcb12_gpa;
 }
 
-static void l2_guest_code(void)
+static void l2_code(void)
 {
 	GUEST_SYNC(SYNC_L2_STARTED);
 	vmcall();
 }
 
-static void l1_guest_code(struct svm_test_data *svm, u64 invalid_vmcb12_gpa)
+static void l1_vmrun(struct svm_test_data *svm, u64 gpa)
 {
-	unsigned long l2_guest_stack[L2_GUEST_STACK_SIZE];
+	generic_svm_setup(svm, l2_code, &l2_guest_stack[L2_GUEST_STACK_SIZE]);
 
-	generic_svm_setup(svm, l2_guest_code,
-			  &l2_guest_stack[L2_GUEST_STACK_SIZE]);
+	asm volatile ("vmrun %[gpa]" : : [gpa] "a" (gpa) : "memory");
+}
 
-	valid_vmcb12_gpa = svm->vmcb_gpa;
+static void l1_vmload(struct svm_test_data *svm, u64 gpa)
+{
+	generic_svm_setup(svm, l2_code, &l2_guest_stack[L2_GUEST_STACK_SIZE]);
 
-	run_guest(svm->vmcb, invalid_vmcb12_gpa); /* #GP */
+	asm volatile ("vmload %[gpa]" : : [gpa] "a" (gpa) : "memory");
+}
+
+static void l1_vmsave(struct svm_test_data *svm, u64 gpa)
+{
+	generic_svm_setup(svm, l2_code, &l2_guest_stack[L2_GUEST_STACK_SIZE]);
+
+	asm volatile ("vmsave %[gpa]" : : [gpa] "a" (gpa) : "memory");
+}
+
+static void l1_vmexit(struct svm_test_data *svm, u64 gpa)
+{
+	generic_svm_setup(svm, l2_code, &l2_guest_stack[L2_GUEST_STACK_SIZE]);
 
-	/* GP handler should jump here */
+	run_guest(svm->vmcb, svm->vmcb_gpa);
 	GUEST_ASSERT(svm->vmcb->control.exit_code == SVM_EXIT_VMMCALL);
 	GUEST_DONE();
 }
 
-int main(int argc, char *argv[])
+static u64 unmappable_gpa(struct kvm_vcpu *vcpu)
+{
+	struct userspace_mem_region *region;
+	u64 region_gpa_end, vm_gpa_end = 0;
+	int i;
+
+	hash_for_each(vcpu->vm->regions.slot_hash, i, region, slot_node) {
+		region_gpa_end = region->region.guest_phys_addr + region->region.memory_size;
+		vm_gpa_end = max(vm_gpa_end, region_gpa_end);
+	}
+
+	return vm_gpa_end;
+}
+
+static void test_invalid_vmcb12(struct kvm_vcpu *vcpu)
 {
-	struct kvm_x86_state *state;
 	vm_vaddr_t nested_gva = 0;
-	struct kvm_vcpu *vcpu;
-	uint32_t maxphyaddr;
-	u64 max_legal_gpa;
-	struct kvm_vm *vm;
 	struct ucall uc;
 
-	TEST_REQUIRE(kvm_cpu_has(X86_FEATURE_SVM));
 
-	vm = vm_create_with_one_vcpu(&vcpu, l1_guest_code);
 	vm_install_exception_handler(vcpu->vm, GP_VECTOR, guest_gp_handler);
-
-	/*
-	 * Find the max legal GPA that is not backed by a memslot (i.e. cannot
-	 * be mapped by KVM).
-	 */
-	maxphyaddr = kvm_cpuid_property(vcpu->cpuid, X86_PROPERTY_MAX_PHY_ADDR);
-	max_legal_gpa = BIT_ULL(maxphyaddr) - PAGE_SIZE;
-	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_alloc_svm(vcpu->vm, &nested_gva);
+	vcpu_args_set(vcpu, 2, nested_gva, -1ULL);
 	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);
+}
+
+static void test_unmappable_vmcb12(struct kvm_vcpu *vcpu)
+{
+	vm_vaddr_t nested_gva = 0;
+
+	vcpu_alloc_svm(vcpu->vm, &nested_gva);
+	vcpu_args_set(vcpu, 2, nested_gva, unmappable_gpa(vcpu));
+	vcpu_run(vcpu);
+
+	TEST_ASSERT_KVM_EXIT_REASON(vcpu, KVM_EXIT_INTERNAL_ERROR);
+	TEST_ASSERT_EQ(vcpu->run->emulation_failure.suberror, KVM_INTERNAL_ERROR_EMULATION);
+}
+
+static void test_unmappable_vmcb12_vmexit(struct kvm_vcpu *vcpu)
+{
+	struct kvm_x86_state *state;
+	vm_vaddr_t nested_gva = 0;
+	struct ucall uc;
 
 	/*
-	 * 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
+	 * Enter L2 (with a legit vmcb12 GPA), then overwrite vmcb12 GPA with an
+	 * unmappable GPA. KVM will fail to map vmcb12 on nested VM-Exit and
 	 * cause a shutdown.
 	 */
+	vcpu_alloc_svm(vcpu->vm, &nested_gva);
+	vcpu_args_set(vcpu, 2, nested_gva, unmappable_gpa(vcpu));
 	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);
 
 	state = vcpu_save_state(vcpu);
-	state->nested.hdr.svm.vmcb_pa = max_legal_gpa;
+	state->nested.hdr.svm.vmcb_pa = unmappable_gpa(vcpu);
 	vcpu_load_state(vcpu, state);
 	vcpu_run(vcpu);
 	TEST_ASSERT_KVM_EXIT_REASON(vcpu, KVM_EXIT_SHUTDOWN);
 
 	kvm_x86_state_cleanup(state);
-	kvm_vm_free(vm);
-	return 0;
+}
+
+KVM_ONE_VCPU_TEST_SUITE(vmcb12_gpa);
+
+KVM_ONE_VCPU_TEST(vmcb12_gpa, vmrun_invalid, l1_vmrun)
+{
+	test_invalid_vmcb12(vcpu);
+}
+
+KVM_ONE_VCPU_TEST(vmcb12_gpa, vmload_invalid, l1_vmload)
+{
+	test_invalid_vmcb12(vcpu);
+}
+
+KVM_ONE_VCPU_TEST(vmcb12_gpa, vmsave_invalid, l1_vmsave)
+{
+	test_invalid_vmcb12(vcpu);
+}
+
+KVM_ONE_VCPU_TEST(vmcb12_gpa, vmrun_unmappable, l1_vmrun)
+{
+	test_unmappable_vmcb12(vcpu);
+}
+
+KVM_ONE_VCPU_TEST(vmcb12_gpa, vmload_unmappable, l1_vmload)
+{
+	test_unmappable_vmcb12(vcpu);
+}
+
+KVM_ONE_VCPU_TEST(vmcb12_gpa, vmsave_unmappable, l1_vmsave)
+{
+	test_unmappable_vmcb12(vcpu);
+}
+
+/*
+ * Invalid vmcb12_gpa cannot be test for #VMEXIT as KVM_SET_NESTED_STATE will
+ * reject it.
+ */
+KVM_ONE_VCPU_TEST(vmcb12_gpa, vmexit_unmappable, l1_vmexit)
+{
+	test_unmappable_vmcb12_vmexit(vcpu);
+}
+
+int main(int argc, char *argv[])
+{
+	TEST_REQUIRE(kvm_cpu_has(X86_FEATURE_SVM));
+
+	return test_harness_run(argc, argv);
 }
-- 
2.53.0.851.ga537e3e6e9-goog

[PATCH v4 9/9] KVM: selftests: Drop 'invalid' from svm_nested_invalid_vmcb12_gpa's name

[Yosry Ahmed]

The test checks both invalid GPAs as well as unmappable GPAs, so drop
'invalid' from its name.

Signed-off-by: Yosry Ahmed <yosry@kernel.org>
---
 tools/testing/selftests/kvm/Makefile.kvm                        | 2 +-
 ...{svm_nested_invalid_vmcb12_gpa.c => svm_nested_vmcb12_gpa.c} | 0
 2 files changed, 1 insertion(+), 1 deletion(-)
 rename tools/testing/selftests/kvm/x86/{svm_nested_invalid_vmcb12_gpa.c => svm_nested_vmcb12_gpa.c} (100%)

diff --git a/tools/testing/selftests/kvm/Makefile.kvm b/tools/testing/selftests/kvm/Makefile.kvm
index 3d372d78a2756..02fa13d4ad4f1 100644
--- a/tools/testing/selftests/kvm/Makefile.kvm
+++ b/tools/testing/selftests/kvm/Makefile.kvm
@@ -112,9 +112,9 @@ TEST_GEN_PROGS_x86 += x86/vmx_preemption_timer_test
 TEST_GEN_PROGS_x86 += x86/svm_vmcall_test
 TEST_GEN_PROGS_x86 += x86/svm_int_ctl_test
 TEST_GEN_PROGS_x86 += x86/svm_nested_clear_efer_svme
-TEST_GEN_PROGS_x86 += x86/svm_nested_invalid_vmcb12_gpa
 TEST_GEN_PROGS_x86 += x86/svm_nested_shutdown_test
 TEST_GEN_PROGS_x86 += x86/svm_nested_soft_inject_test
+TEST_GEN_PROGS_x86 += x86/svm_nested_vmcb12_gpa
 TEST_GEN_PROGS_x86 += x86/svm_lbr_nested_state
 TEST_GEN_PROGS_x86 += x86/tsc_scaling_sync
 TEST_GEN_PROGS_x86 += x86/sync_regs_test
diff --git a/tools/testing/selftests/kvm/x86/svm_nested_invalid_vmcb12_gpa.c b/tools/testing/selftests/kvm/x86/svm_nested_vmcb12_gpa.c
similarity index 100%
rename from tools/testing/selftests/kvm/x86/svm_nested_invalid_vmcb12_gpa.c
rename to tools/testing/selftests/kvm/x86/svm_nested_vmcb12_gpa.c
-- 
2.53.0.851.ga537e3e6e9-goog

Re: [PATCH v4 1/9] KVM: SVM: Properly check RAX in the emulator for SVM instructions

[Yosry Ahmed]

On Mon, Mar 16, 2026 at 1:27 PM Yosry Ahmed <yosry@kernel.org> wrote:
>
> Architecturally, VMRUN/VMLOAD/VMSAVE should generate a #GP if the
> physical address in RAX is not supported. check_svme_pa() hardcodes this
> to checking that bits 63-48 are not set. This is incorrect on HW
> supporting 52 bits of physical address space. Additionally, the emulator
> does not check if the address is not aligned, which should also result
> in #GP.
>
> Use page_address_valid() which properly checks alignment and the address
> legality based on the guest's MAXPHYADDR. Plumb it through
> x86_emulate_ops, similar to is_canonical_addr(), to avoid directly
> accessing the vCPU object in emulator code.
>
> Fixes: 01de8b09e606 ("KVM: SVM: Add intercept checks for SVM instructions")
> Suggested-by: Sean Christopherson <seanjc@google.com>

I should have dropped this Suggested-by tag.

Re: [PATCH v4 3/9] KVM: SVM: Properly check RAX on #GP intercept of SVM instructions

[Sean Christopherson]

On Mon, Mar 16, 2026, Yosry Ahmed wrote:
> Replace the PAGE_MASK check with page_address_valid(), which checks both
> page-alignment as well as the legality of the GPA based on the vCPU's
> MAXPHYADDR. Use kvm_register_read() to read RAX to avoid
> page_address_valid() failing on 32-bit due to garbage in the higher
> bits.

Nit, not "on" 32-bit, correct?  I think you actually mean "to avoid false positives
when the vCPU is in 32-bit mode, in the unlikely case the vCPU transitioned from
64-bit back to 32-bit, without writing EAX".  Because regs[] is an unsigned long,
so the upper bits of save.rax will be cleared by svm_vcpu_run() on every VM-Entry,
and it should be impossible for a purely 32-bit guest to get a non-zero value in
RAX[63:32].

And even for a 64-bit host with a 32-bit guest, the only way to get a non-zero
value in RAX[63:32] while in 32-bit mode would be to transition from 64-bit mode,
back to 32-bit mode, without writing EAX.

> Note that this is currently only a problem if KVM is running an L2 guest
> and ends up synthesizing a #VMEXIT to L1, as the RAX check takes
> precedence over the intercept. Otherwise, if KVM emulates the
> instruction, kvm_vcpu_map() should fail on illegal GPAs and inject a #GP
> anyway. However, following patches will change the failure behavior of
> kvm_vcpu_map(), so make sure the #GP interception handler does this
> appropriately.
> 
> Opportunistically drop a teaser FIXME about the SVM instructions
> handling on #GP belonging in the emulator.
> 
> Fixes: 82a11e9c6fa2 ("KVM: SVM: Add emulation support for #GP triggered by SVM instructions")
> Fixes: d1cba6c92237 ("KVM: x86: nSVM: test eax for 4K alignment for GP errata workaround")
> Suggested-by: Sean Christopherson <seanjc@google.com>
> Signed-off-by: Yosry Ahmed <yosry@kernel.org>
> ---
>  arch/x86/kvm/svm/svm.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
> index 392a5088f20bf..3122a98745ab7 100644
> --- a/arch/x86/kvm/svm/svm.c
> +++ b/arch/x86/kvm/svm/svm.c
> @@ -2277,10 +2277,12 @@ static int gp_interception(struct kvm_vcpu *vcpu)
>  	if (x86_decode_emulated_instruction(vcpu, 0, NULL, 0) != EMULATION_OK)
>  		goto reinject;
>  
> +	/* FIXME: Handle SVM instructions through the emulator */
>  	svm_exit_code = svm_instr_exit_code(vcpu);
>  	if (svm_exit_code) {
> -		/* All SVM instructions expect page aligned RAX */
> -		if (svm->vmcb->save.rax & ~PAGE_MASK)
> +		unsigned long rax = kvm_register_read(vcpu, VCPU_REGS_RAX);
> +
> +		if (!page_address_valid(vcpu, rax))

Eh, let it poke out, i.e.

		if (!page_address_valid(vcpu, kvm_register_read(vcpu, VCPU_REGS_RAX)))
			goto reinject;

>  			goto reinject;
>  
>  		if (is_guest_mode(vcpu)) {
> -- 
> 2.53.0.851.ga537e3e6e9-goog
> 

Re: [PATCH v4 2/9] KVM: SVM: Refactor SVM instruction handling on #GP intercept

[Sean Christopherson]

On Mon, Mar 16, 2026, Yosry Ahmed wrote:
> +/* Return 0 if not SVM instr, otherwise return associated exit_code */
> +static u64 svm_instr_exit_code(struct kvm_vcpu *vcpu)

To make it very clear what this is doing how about:

static u64 svm_get_decoded_instr_exit_code(struct kvm_vcpu *vcpu)

>  {
>  	struct x86_emulate_ctxt *ctxt = vcpu->arch.emulate_ctxt;
>  
>  	if (ctxt->b != 0x1 || ctxt->opcode_len != 2)
> -		return NONE_SVM_INSTR;
> +		return 0;

This should assert to ensure there's no collision with '0', i.e.

	BUILD_BUG_ON(!SVM_EXIT_VMRUN || !SVM_EXIT_VMLOAD || !SVM_EXIT_VMSAVE);


> +	if (!is_guest_mode(vcpu))
> +		return kvm_emulate_instruction(vcpu,
> +				EMULTYPE_VMWARE_GP | EMULTYPE_NO_DECODE);

Since you're moving this anyways:

	if (!is_guest_mode(vcpu))
		return kvm_emulate_instruction(vcpu, EMULTYPE_VMWARE_GP |
						     EMULTYPE_NO_DECODE);


Actually!  Better idea, for this code and for the page_address_valid() checks:
invert the checks to reduce indentation, i.e. end up with:

	/* FIXME: Handle SVM instructions through the emulator */
	svm_exit_code = svm_get_decoded_instr_exit_code(vcpu);
	if (svm_exit_code) {
		if (!is_guest_mode(vcpu))
			return svm_invoke_exit_handler(vcpu, svm_exit_code);

		if (nested_svm_check_permissions(vcpu))
			return 1;

		if (!page_address_valid(vcpu, kvm_register_read(vcpu, VCPU_REGS_RAX)))
			goto reinject;

		/*
		 * FIXME: Only synthesize a #VMEXIT if L1 sets the intercept,
		 * but only after the VMLOAD/VMSAVE exit handlers can properly
		 * handle VMLOAD/VMSAVE from L2 with VLS enabled in L1 (i.e.
		 * RAX is an L2 GPA that needs translation through L1's NPT).
		 */
		nested_svm_simple_vmexit(svm, svm_exit_code);
		return 1;
	}

	/*
	 * VMware backdoor emulation on #GP interception only handles
	 * IN{S}, OUT{S}, and RDPMC, and only for L1.
	 */
	if (!enable_vmware_backdoor || is_guest_mode(vcpu))
		goto reinject;

	return kvm_emulate_instruction(vcpu, EMULTYPE_VMWARE_GP | EMULTYPE_NO_DECODE);

> +
>  reinject:
>  	kvm_queue_exception_e(vcpu, GP_VECTOR, error_code);
>  	return 1;
> -- 
> 2.53.0.851.ga537e3e6e9-goog
> 

Re: [PATCH v4 3/9] KVM: SVM: Properly check RAX on #GP intercept of SVM instructions

[Sean Christopherson]

On Fri, Apr 03, 2026, Sean Christopherson wrote:
> On Mon, Mar 16, 2026, Yosry Ahmed wrote:
> > Replace the PAGE_MASK check with page_address_valid(), which checks both
> > page-alignment as well as the legality of the GPA based on the vCPU's
> > MAXPHYADDR. Use kvm_register_read() to read RAX to avoid
> > page_address_valid() failing on 32-bit due to garbage in the higher
> > bits.
> 
> Nit, not "on" 32-bit, correct?  I think you actually mean "to avoid false positives
> when the vCPU is in 32-bit mode, in the unlikely case the vCPU transitioned from
> 64-bit back to 32-bit, without writing EAX".  Because regs[] is an unsigned long,
> so the upper bits of save.rax will be cleared by svm_vcpu_run() on every VM-Entry,
> and it should be impossible for a purely 32-bit guest to get a non-zero value in
> RAX[63:32].
> 
> And even for a 64-bit host with a 32-bit guest, the only way to get a non-zero
> value in RAX[63:32] while in 32-bit mode would be to transition from 64-bit mode,
> back to 32-bit mode, without writing EAX.
> 
> > Note that this is currently only a problem if KVM is running an L2 guest
> > and ends up synthesizing a #VMEXIT to L1, as the RAX check takes
> > precedence over the intercept. Otherwise, if KVM emulates the
> > instruction, kvm_vcpu_map() should fail on illegal GPAs and inject a #GP
> > anyway. However, following patches will change the failure behavior of
> > kvm_vcpu_map(), so make sure the #GP interception handler does this
> > appropriately.
> > 
> > Opportunistically drop a teaser FIXME about the SVM instructions
> > handling on #GP belonging in the emulator.
> > 
> > Fixes: 82a11e9c6fa2 ("KVM: SVM: Add emulation support for #GP triggered by SVM instructions")
> > Fixes: d1cba6c92237 ("KVM: x86: nSVM: test eax for 4K alignment for GP errata workaround")
> > Suggested-by: Sean Christopherson <seanjc@google.com>
> > Signed-off-by: Yosry Ahmed <yosry@kernel.org>
> > ---
> >  arch/x86/kvm/svm/svm.c | 6 ++++--
> >  1 file changed, 4 insertions(+), 2 deletions(-)
> > 
> > diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
> > index 392a5088f20bf..3122a98745ab7 100644
> > --- a/arch/x86/kvm/svm/svm.c
> > +++ b/arch/x86/kvm/svm/svm.c
> > @@ -2277,10 +2277,12 @@ static int gp_interception(struct kvm_vcpu *vcpu)
> >  	if (x86_decode_emulated_instruction(vcpu, 0, NULL, 0) != EMULATION_OK)
> >  		goto reinject;
> >  
> > +	/* FIXME: Handle SVM instructions through the emulator */
> >  	svm_exit_code = svm_instr_exit_code(vcpu);
> >  	if (svm_exit_code) {
> > -		/* All SVM instructions expect page aligned RAX */
> > -		if (svm->vmcb->save.rax & ~PAGE_MASK)
> > +		unsigned long rax = kvm_register_read(vcpu, VCPU_REGS_RAX);
> > +
> > +		if (!page_address_valid(vcpu, rax))
> 
> Eh, let it poke out, i.e.
> 
> 		if (!page_address_valid(vcpu, kvm_register_read(vcpu, VCPU_REGS_RAX)))

Argh, looking at the rest of this series, and at KVM's existing code, having to
use kvm_register_read() is awful.  This really should be able to use kvm_rax_read(),
but that won't handle the truncation.

There are only a handful of likely-benign goofs due to this mess, but there is a
pile of manual truncation and casting going on.  In addition to _raw() variants,
and mode-aware defaults, add "e" versions would be helpful, as many of the
explicit truncation flows are cases where e.g. EAX, ECX, and EDX are architecturally
accessed.

I'll put together patches, and think more on how to handle this series (the
dependencies aren't terrible, but they certainly are annoying).  I'm tempted
to squeeze this into 7.1 to make future life easier...

> 			goto reinject;
> 
> >  			goto reinject;
> >  
> >  		if (is_guest_mode(vcpu)) {
> > -- 
> > 2.53.0.851.ga537e3e6e9-goog
> > 

Re: [PATCH v4 0/9] KVM: SVM: Fixes for VMCB12 checks and mapping

[Sean Christopherson]

On Mon, Mar 16, 2026, Yosry Ahmed wrote:
> Jim pointed out that VMRUN/VMLOAD/VMSAVE injecting a #GP when the vmcb12
> GPA is valid but not mappable is not architectural [1]. The series
> handles them as emulation failures and (mostly) exits to userspace
> instead. It also fixes the checks performed on the vmcb12 GPA (i.e. RAX)
> in a few places.
> 
> Note that there's a few other bugs that this series leaves alone, mostly
> to keep it focused on fixing the non-architectrual #GPs, this includes:
> - KVM synthesizing #VMEXIT(VMLOAD/VMSAVE) to L1 when intercepting #GP
>   from L2 on VMLOAD/VMSAVE, even if L1 does not intercept VMLOAD/VMSAVE
>   (e.g. VLS enabled).
> - KVM not respecting priority of #GP vs. #UD when EFER.SVME is disabled
>   by the guest.
> 
> However, the series documents these bugs with FIXMEs so that they can
> hopefully be fixed up later.
> 
> v3 -> v4:
> - Fix the RAX check in the emulator instead of dropping it [Paolo].
> - Add refactoring of SVM instructions on #GP intercept to facilitate
>   following fixes.
> - Use kvm_register_read() to handle 32-bit correctly and avoid false
>   negatives.
> - Do #UD and #GP checks before synthesizing #VMEXIT to L1 when KVM
>   intercepts #GP on SVM instruction in L2.
> - Update changelogs.
> 
> v3: https://lore.kernel.org/kvm/20260306210900.1933788-1-yosry@kernel.org/
> 
> [1]https://lore.kernel.org/kvm/CALMp9eSMtzDJn7tGtbj=zLYpcU7Tc7XjcWBRZH7Aa5YihSmN7g@mail.gmail.com/
> 
> Yosry Ahmed (9):
>   KVM: SVM: Properly check RAX in the emulator for SVM instructions
>   KVM: SVM: Refactor SVM instruction handling on #GP intercept
>   KVM: SVM: Properly check RAX on #GP intercept of SVM instructions
>   KVM: SVM: Move RAX legality check to SVM insn interception handlers
>   KVM: SVM: Check EFER.SVME and CPL on #GP intercept of SVM instructions
>   KVM: SVM: Treat mapping failures equally in VMLOAD/VMSAVE emulation
>   KVM: nSVM: Fail emulation of VMRUN/VMLOAD/VMSAVE if mapping vmcb12
>     fails
>   KVM: selftests: Rework svm_nested_invalid_vmcb12_gpa
>   KVM: selftests: Drop 'invalid' from svm_nested_invalid_vmcb12_gpa's
>     name

Some nits and suggestsions, but I can clean them up when applying.  I'm leaning
strongly toward applying this for 7.1 even though it's super late in the cycle,
as there's very little risk (e.g. compared to the gPTA or host/guest PMU series),
and getting the VCPU_REGS_RAX landed would make my life easier for 7.2.

Re: [PATCH v4 3/9] KVM: SVM: Properly check RAX on #GP intercept of SVM instructions

[Yosry Ahmed]

On Fri, Apr 3, 2026 at 12:00 PM Sean Christopherson <seanjc@google.com> wrote:
>
> On Fri, Apr 03, 2026, Sean Christopherson wrote:
> > On Mon, Mar 16, 2026, Yosry Ahmed wrote:
> > > Replace the PAGE_MASK check with page_address_valid(), which checks both
> > > page-alignment as well as the legality of the GPA based on the vCPU's
> > > MAXPHYADDR. Use kvm_register_read() to read RAX to avoid
> > > page_address_valid() failing on 32-bit due to garbage in the higher
> > > bits.
> >
> > Nit, not "on" 32-bit, correct?  I think you actually mean "to avoid false positives
> > when the vCPU is in 32-bit mode, in the unlikely case the vCPU transitioned from
> > 64-bit back to 32-bit, without writing EAX".  Because regs[] is an unsigned long,
> > so the upper bits of save.rax will be cleared by svm_vcpu_run() on every VM-Entry,
> > and it should be impossible for a purely 32-bit guest to get a non-zero value in
> > RAX[63:32].
> >
> > And even for a 64-bit host with a 32-bit guest, the only way to get a non-zero
> > value in RAX[63:32] while in 32-bit mode would be to transition from 64-bit mode,
> > back to 32-bit mode, without writing EAX.
> >
> > > Note that this is currently only a problem if KVM is running an L2 guest
> > > and ends up synthesizing a #VMEXIT to L1, as the RAX check takes
> > > precedence over the intercept. Otherwise, if KVM emulates the
> > > instruction, kvm_vcpu_map() should fail on illegal GPAs and inject a #GP
> > > anyway. However, following patches will change the failure behavior of
> > > kvm_vcpu_map(), so make sure the #GP interception handler does this
> > > appropriately.
> > >
> > > Opportunistically drop a teaser FIXME about the SVM instructions
> > > handling on #GP belonging in the emulator.
> > >
> > > Fixes: 82a11e9c6fa2 ("KVM: SVM: Add emulation support for #GP triggered by SVM instructions")
> > > Fixes: d1cba6c92237 ("KVM: x86: nSVM: test eax for 4K alignment for GP errata workaround")
> > > Suggested-by: Sean Christopherson <seanjc@google.com>
> > > Signed-off-by: Yosry Ahmed <yosry@kernel.org>
> > > ---
> > >  arch/x86/kvm/svm/svm.c | 6 ++++--
> > >  1 file changed, 4 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
> > > index 392a5088f20bf..3122a98745ab7 100644
> > > --- a/arch/x86/kvm/svm/svm.c
> > > +++ b/arch/x86/kvm/svm/svm.c
> > > @@ -2277,10 +2277,12 @@ static int gp_interception(struct kvm_vcpu *vcpu)
> > >     if (x86_decode_emulated_instruction(vcpu, 0, NULL, 0) != EMULATION_OK)
> > >             goto reinject;
> > >
> > > +   /* FIXME: Handle SVM instructions through the emulator */
> > >     svm_exit_code = svm_instr_exit_code(vcpu);
> > >     if (svm_exit_code) {
> > > -           /* All SVM instructions expect page aligned RAX */
> > > -           if (svm->vmcb->save.rax & ~PAGE_MASK)
> > > +           unsigned long rax = kvm_register_read(vcpu, VCPU_REGS_RAX);
> > > +
> > > +           if (!page_address_valid(vcpu, rax))
> >
> > Eh, let it poke out, i.e.
> >
> >               if (!page_address_valid(vcpu, kvm_register_read(vcpu, VCPU_REGS_RAX)))
>
> Argh, looking at the rest of this series, and at KVM's existing code, having to
> use kvm_register_read() is awful.  This really should be able to use kvm_rax_read(),
> but that won't handle the truncation.
>
> There are only a handful of likely-benign goofs due to this mess, but there is a
> pile of manual truncation and casting going on.  In addition to _raw() variants,
> and mode-aware defaults, add "e" versions would be helpful, as many of the
> explicit truncation flows are cases where e.g. EAX, ECX, and EDX are architecturally
> accessed.
>
> I'll put together patches, and think more on how to handle this series (the
> dependencies aren't terrible, but they certainly are annoying).  I'm tempted
> to squeeze this into 7.1 to make future life easier...

Just to make sure I understand this correctly, you'll keep this series
using kvm_register_read() and send patches on top to make
kvm_rax_read() a viable alternative and switch it, right?

Re: [PATCH v4 2/9] KVM: SVM: Refactor SVM instruction handling on #GP intercept

[Yosry Ahmed]

On Fri, Apr 3, 2026 at 11:18 AM Sean Christopherson <seanjc@google.com> wrote:
>
> On Mon, Mar 16, 2026, Yosry Ahmed wrote:
> > +/* Return 0 if not SVM instr, otherwise return associated exit_code */
> > +static u64 svm_instr_exit_code(struct kvm_vcpu *vcpu)
>
> To make it very clear what this is doing how about:
>
> static u64 svm_get_decoded_instr_exit_code(struct kvm_vcpu *vcpu)

Yeah I like this better.

>
> >  {
> >       struct x86_emulate_ctxt *ctxt = vcpu->arch.emulate_ctxt;
> >
> >       if (ctxt->b != 0x1 || ctxt->opcode_len != 2)
> > -             return NONE_SVM_INSTR;
> > +             return 0;
>
> This should assert to ensure there's no collision with '0', i.e.
>
>         BUILD_BUG_ON(!SVM_EXIT_VMRUN || !SVM_EXIT_VMLOAD || !SVM_EXIT_VMSAVE);

I was gonna add this but I don't think these values can change anyway.
It's good for readability tho so I am fine with it.

>
>
> > +     if (!is_guest_mode(vcpu))
> > +             return kvm_emulate_instruction(vcpu,
> > +                             EMULTYPE_VMWARE_GP | EMULTYPE_NO_DECODE);
>
> Since you're moving this anyways:
>
>         if (!is_guest_mode(vcpu))
>                 return kvm_emulate_instruction(vcpu, EMULTYPE_VMWARE_GP |
>                                                      EMULTYPE_NO_DECODE);
>
>
> Actually!  Better idea, for this code and for the page_address_valid() checks:
> invert the checks to reduce indentation, i.e. end up with:
>
>         /* FIXME: Handle SVM instructions through the emulator */
>         svm_exit_code = svm_get_decoded_instr_exit_code(vcpu);
>         if (svm_exit_code) {
>                 if (!is_guest_mode(vcpu))
>                         return svm_invoke_exit_handler(vcpu, svm_exit_code);
>
>                 if (nested_svm_check_permissions(vcpu))
>                         return 1;
>
>                 if (!page_address_valid(vcpu, kvm_register_read(vcpu, VCPU_REGS_RAX)))
>                         goto reinject;
>
>                 /*
>                  * FIXME: Only synthesize a #VMEXIT if L1 sets the intercept,
>                  * but only after the VMLOAD/VMSAVE exit handlers can properly
>                  * handle VMLOAD/VMSAVE from L2 with VLS enabled in L1 (i.e.
>                  * RAX is an L2 GPA that needs translation through L1's NPT).
>                  */
>                 nested_svm_simple_vmexit(svm, svm_exit_code);
>                 return 1;
>         }
>
>         /*
>          * VMware backdoor emulation on #GP interception only handles
>          * IN{S}, OUT{S}, and RDPMC, and only for L1.
>          */
>         if (!enable_vmware_backdoor || is_guest_mode(vcpu))
>                 goto reinject;
>
>         return kvm_emulate_instruction(vcpu, EMULTYPE_VMWARE_GP | EMULTYPE_NO_DECODE);
>
> > +
> >  reinject:
> >       kvm_queue_exception_e(vcpu, GP_VECTOR, error_code);
> >       return 1;


The end result looks good to me.

Re: [PATCH v4 0/9] KVM: SVM: Fixes for VMCB12 checks and mapping

[Yosry Ahmed]

> > Yosry Ahmed (9):
> >   KVM: SVM: Properly check RAX in the emulator for SVM instructions
> >   KVM: SVM: Refactor SVM instruction handling on #GP intercept
> >   KVM: SVM: Properly check RAX on #GP intercept of SVM instructions
> >   KVM: SVM: Move RAX legality check to SVM insn interception handlers
> >   KVM: SVM: Check EFER.SVME and CPL on #GP intercept of SVM instructions
> >   KVM: SVM: Treat mapping failures equally in VMLOAD/VMSAVE emulation
> >   KVM: nSVM: Fail emulation of VMRUN/VMLOAD/VMSAVE if mapping vmcb12
> >     fails
> >   KVM: selftests: Rework svm_nested_invalid_vmcb12_gpa
> >   KVM: selftests: Drop 'invalid' from svm_nested_invalid_vmcb12_gpa's
> >     name
>
> Some nits and suggestsions, but I can clean them up when applying.  I'm leaning
> strongly toward applying this for 7.1 even though it's super late in the cycle,
> as there's very little risk (e.g. compared to the gPTA or host/guest PMU series),
> and getting the VCPU_REGS_RAX landed would make my life easier for 7.2.

Thank you!

Re: [PATCH v4 3/9] KVM: SVM: Properly check RAX on #GP intercept of SVM instructions

[Sean Christopherson]

On Fri, Apr 03, 2026, Yosry Ahmed wrote:
> On Fri, Apr 3, 2026 at 12:00 PM Sean Christopherson <seanjc@google.com> wrote:
> > On Fri, Apr 03, 2026, Sean Christopherson wrote:
> > > >     svm_exit_code = svm_instr_exit_code(vcpu);
> > > >     if (svm_exit_code) {
> > > > -           /* All SVM instructions expect page aligned RAX */
> > > > -           if (svm->vmcb->save.rax & ~PAGE_MASK)
> > > > +           unsigned long rax = kvm_register_read(vcpu, VCPU_REGS_RAX);
> > > > +
> > > > +           if (!page_address_valid(vcpu, rax))
> > >
> > > Eh, let it poke out, i.e.
> > >
> > >               if (!page_address_valid(vcpu, kvm_register_read(vcpu, VCPU_REGS_RAX)))
> >
> > Argh, looking at the rest of this series, and at KVM's existing code, having to
> > use kvm_register_read() is awful.  This really should be able to use kvm_rax_read(),
> > but that won't handle the truncation.
> >
> > There are only a handful of likely-benign goofs due to this mess, but there is a
> > pile of manual truncation and casting going on.  In addition to _raw() variants,
> > and mode-aware defaults, add "e" versions would be helpful, as many of the
> > explicit truncation flows are cases where e.g. EAX, ECX, and EDX are architecturally
> > accessed.
> >
> > I'll put together patches, and think more on how to handle this series (the
> > dependencies aren't terrible, but they certainly are annoying).  I'm tempted
> > to squeeze this into 7.1 to make future life easier...
> 
> Just to make sure I understand this correctly, you'll keep this series
> using kvm_register_read() and send patches on top to make
> kvm_rax_read() a viable alternative and switch it, right?

Yep, exactly!