diff mbox series

[1/4] KVM: vmx, svm, mmu: Fix MMIO during event delivery handling

Message ID 20240923141810.76331-2-iorlov@amazon.com
State New
Headers show
Series Process some MMIO-related errors without KVM exit | expand

Commit Message

Ivan Orlov Sept. 23, 2024, 2:18 p.m. UTC
Currently, the situation when guest accesses MMIO during event delivery
is handled differently in VMX and SVM: on VMX KVM returns internal error
with suberror = KVM_INTERNAL_ERROR_DELIVERY_EV, when SVM simply goes
into infinite loop trying to execute faulty instruction again and again.

Such a situation can happen when the guest sets the IDTR (or GDTR)
descriptor base to point to MMIO region, and as this issue can be
triggered from the guest it is not an "internal" KVM error and it
should be gracefully handled by KVM.

Eliminate the SVM/VMX difference by triggering triple fault when MMIO
happens during event delivery. As we don't have a reliable way to
detect MMIO operation on SVM before actually looking at the GPA,
move the problem detection into the common KVM x86 layer (into the
kvm_mmu_page_fault function) and add the PFERR_EVT_DELIVERY flag
which gets set in the SVM/VMX specific vmexit handler to signal
that we are in the middle of the event delivery.

Signed-off-by: Ivan Orlov <iorlov@amazon.com>
---
 arch/x86/include/asm/kvm_host.h |  6 ++++++
 arch/x86/kvm/mmu/mmu.c          | 13 ++++++++++++-
 arch/x86/kvm/svm/svm.c          |  4 ++++
 arch/x86/kvm/vmx/vmx.c          | 21 ++++++++-------------
 4 files changed, 30 insertions(+), 14 deletions(-)
diff mbox series

Patch

diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 4a68cb3eba78..292657fda063 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -284,6 +284,12 @@  enum x86_intercept_stage;
 				 PFERR_WRITE_MASK |		\
 				 PFERR_PRESENT_MASK)
 
+/*
+ * EVT_DELIVERY is a KVM-defined flag used to indicate that vmexit occurred
+ * during event delivery.
+ */
+#define PFERR_EVT_DELIVERY		BIT_ULL(50)
+
 /* apic attention bits */
 #define KVM_APIC_CHECK_VAPIC	0
 /*
diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
index 7813d28b082f..80db379766fb 100644
--- a/arch/x86/kvm/mmu/mmu.c
+++ b/arch/x86/kvm/mmu/mmu.c
@@ -5992,8 +5992,19 @@  int noinline kvm_mmu_page_fault(struct kvm_vcpu *vcpu, gpa_t cr2_or_gpa, u64 err
 			return -EFAULT;
 
 		r = handle_mmio_page_fault(vcpu, cr2_or_gpa, direct);
-		if (r == RET_PF_EMULATE)
+		if (r == RET_PF_EMULATE) {
+			/*
+			 * Request triple fault if guest accesses MMIO during event delivery.
+			 * It could happen if the guest sets the IDTR base to point to an MMIO
+			 * range. This is not allowed and there is no way to recover after it.
+			 */
+			if (error_code & PFERR_EVT_DELIVERY) {
+				pr_warn("Guest accesses MMIO during event delivery. Requesting triple fault.\n");
+				kvm_make_request(KVM_REQ_TRIPLE_FAULT, vcpu);
+				return 1;
+			}
 			goto emulate;
+		}
 	}
 
 	if (r == RET_PF_INVALID) {
diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index 5ab2c92c7331..b83ca69b0e57 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -2058,6 +2058,7 @@  static int npf_interception(struct kvm_vcpu *vcpu)
 
 	u64 fault_address = svm->vmcb->control.exit_info_2;
 	u64 error_code = svm->vmcb->control.exit_info_1;
+	u32 int_type = svm->vmcb->control.exit_int_info & SVM_EXITINTINFO_TYPE_MASK;
 
 	/*
 	 * WARN if hardware generates a fault with an error code that collides
@@ -2071,6 +2072,9 @@  static int npf_interception(struct kvm_vcpu *vcpu)
 	if (sev_snp_guest(vcpu->kvm) && (error_code & PFERR_GUEST_ENC_MASK))
 		error_code |= PFERR_PRIVATE_ACCESS;
 
+	if (int_type)
+		error_code |= PFERR_EVT_DELIVERY;
+
 	trace_kvm_page_fault(vcpu, fault_address, error_code);
 	rc = kvm_mmu_page_fault(vcpu, fault_address, error_code,
 				static_cpu_has(X86_FEATURE_DECODEASSISTS) ?
diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index 733a0c45d1a6..4d136fee7d63 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -5823,7 +5823,12 @@  static int handle_ept_violation(struct kvm_vcpu *vcpu)
 
 static int handle_ept_misconfig(struct kvm_vcpu *vcpu)
 {
+	struct vcpu_vmx *vmx = to_vmx(vcpu);
 	gpa_t gpa;
+	u64 error_code = PFERR_RSVD_MASK;
+
+	if (vmx->idt_vectoring_info & VECTORING_INFO_VALID_MASK)
+		error_code |= PFERR_EVT_DELIVERY;
 
 	if (vmx_check_emulate_instruction(vcpu, EMULTYPE_PF, NULL, 0))
 		return 1;
@@ -5839,7 +5844,7 @@  static int handle_ept_misconfig(struct kvm_vcpu *vcpu)
 		return kvm_skip_emulated_instruction(vcpu);
 	}
 
-	return kvm_mmu_page_fault(vcpu, gpa, PFERR_RSVD_MASK, NULL, 0);
+	return kvm_mmu_page_fault(vcpu, gpa, error_code, NULL, 0);
 }
 
 static int handle_nmi_window(struct kvm_vcpu *vcpu)
@@ -6532,20 +6537,14 @@  static int __vmx_handle_exit(struct kvm_vcpu *vcpu, fastpath_t exit_fastpath)
 		return 0;
 	}
 
-	/*
-	 * Note:
-	 * Do not try to fix EXIT_REASON_EPT_MISCONFIG if it caused by
-	 * delivery event since it indicates guest is accessing MMIO.
-	 * The vm-exit can be triggered again after return to guest that
-	 * will cause infinite loop.
-	 */
 	if ((vectoring_info & VECTORING_INFO_VALID_MASK) &&
 	    (exit_reason.basic != EXIT_REASON_EXCEPTION_NMI &&
 	     exit_reason.basic != EXIT_REASON_EPT_VIOLATION &&
 	     exit_reason.basic != EXIT_REASON_PML_FULL &&
 	     exit_reason.basic != EXIT_REASON_APIC_ACCESS &&
 	     exit_reason.basic != EXIT_REASON_TASK_SWITCH &&
-	     exit_reason.basic != EXIT_REASON_NOTIFY)) {
+	     exit_reason.basic != EXIT_REASON_NOTIFY &&
+	     exit_reason.basic != EXIT_REASON_EPT_MISCONFIG)) {
 		int ndata = 3;
 
 		vcpu->run->exit_reason = KVM_EXIT_INTERNAL_ERROR;
@@ -6553,10 +6552,6 @@  static int __vmx_handle_exit(struct kvm_vcpu *vcpu, fastpath_t exit_fastpath)
 		vcpu->run->internal.data[0] = vectoring_info;
 		vcpu->run->internal.data[1] = exit_reason.full;
 		vcpu->run->internal.data[2] = vmx_get_exit_qual(vcpu);
-		if (exit_reason.basic == EXIT_REASON_EPT_MISCONFIG) {
-			vcpu->run->internal.data[ndata++] =
-				vmcs_read64(GUEST_PHYSICAL_ADDRESS);
-		}
 		vcpu->run->internal.data[ndata++] = vcpu->arch.last_vmentry_cpu;
 		vcpu->run->internal.ndata = ndata;
 		return 0;