* [PATCH 1/3] KVM: nVMX: Clear EXIT_QUALIFICATION when injecting an EPT Misconfig
2024-02-09 22:16 [PATCH 0/3] KVM: nVMX: nEPT injection fixes and cleanups Sean Christopherson
@ 2024-02-09 22:16 ` Sean Christopherson
2024-02-09 22:16 ` [PATCH 2/3] KVM: x86: Move nEPT exit_qualification field from kvm_vcpu_arch to x86_exception Sean Christopherson
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: Sean Christopherson @ 2024-02-09 22:16 UTC (permalink / raw)
To: Sean Christopherson, Paolo Bonzini; +Cc: kvm, linux-kernel
Explicitly clear the EXIT_QUALIFCATION field when injecting an EPT
misconfig into L1, as required by the VMX architecture. Per the SDM:
This field is saved for VM exits due to the following causes:
debug exceptions; page-fault exceptions; start-up IPIs (SIPIs);
system-management interrupts (SMIs) that arrive immediately after the
execution of I/O instructions; task switches; INVEPT; INVLPG; INVPCID;
INVVPID; LGDT; LIDT; LLDT; LTR; SGDT; SIDT; SLDT; STR; VMCLEAR; VMPTRLD;
VMPTRST; VMREAD; VMWRITE; VMXON; WBINVD; WBNOINVD; XRSTORS; XSAVES;
control-register accesses; MOV DR; I/O instructions; MWAIT; accesses to
the APIC-access page; EPT violations; EOI virtualization; APIC-write
emulation; page-modification log full; SPP-related events; and
instruction timeout. For all other VM exits, this field is cleared.
Generating EXIT_QUALIFICATION from vcpu->arch.exit_qualification is wrong
for all (two) paths that lead to nested_ept_inject_page_fault(). For EPT
violations (the common case), vcpu->arch.exit_qualification will have been
set by handle_ept_violation() to vmcs02.EXIT_QUALIFICATION, i.e. contains
the information of a EPT violation and thus is likely non-zero.
For an EPT misconfig, which can reach FNAME(walk_addr_generic) and thus
inject a nEPT misconfig if KVM created an MMIO SPTE that became stale,
vcpu->arch.exit_qualification will hold the information from the last EPT
violation VM-Exit, as vcpu->arch.exit_qualification is _only_ written by
handle_ept_violation().
Fixes: 4704d0befb07 ("KVM: nVMX: Exiting from L2 to L1")
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
arch/x86/kvm/vmx/nested.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c
index 994e014f8a50..1eebed84bb65 100644
--- a/arch/x86/kvm/vmx/nested.c
+++ b/arch/x86/kvm/vmx/nested.c
@@ -417,10 +417,12 @@ static void nested_ept_inject_page_fault(struct kvm_vcpu *vcpu,
vmx->nested.pml_full = false;
exit_qualification &= INTR_INFO_UNBLOCK_NMI;
} else {
- if (fault->error_code & PFERR_RSVD_MASK)
+ if (fault->error_code & PFERR_RSVD_MASK) {
vm_exit_reason = EXIT_REASON_EPT_MISCONFIG;
- else
+ exit_qualification = 0;
+ } else {
vm_exit_reason = EXIT_REASON_EPT_VIOLATION;
+ }
/*
* Although the caller (kvm_inject_emulated_page_fault) would
--
2.43.0.687.g38aa6559b0-goog
^ permalink raw reply [flat|nested] 5+ messages in thread* [PATCH 2/3] KVM: x86: Move nEPT exit_qualification field from kvm_vcpu_arch to x86_exception
2024-02-09 22:16 [PATCH 0/3] KVM: nVMX: nEPT injection fixes and cleanups Sean Christopherson
2024-02-09 22:16 ` [PATCH 1/3] KVM: nVMX: Clear EXIT_QUALIFICATION when injecting an EPT Misconfig Sean Christopherson
@ 2024-02-09 22:16 ` Sean Christopherson
2024-02-09 22:17 ` [PATCH 3/3] KVM: nVMX: Add a sanity check that nested PML Full stems from EPT Violations Sean Christopherson
2024-04-10 0:20 ` [PATCH 0/3] KVM: nVMX: nEPT injection fixes and cleanups Sean Christopherson
3 siblings, 0 replies; 5+ messages in thread
From: Sean Christopherson @ 2024-02-09 22:16 UTC (permalink / raw)
To: Sean Christopherson, Paolo Bonzini; +Cc: kvm, linux-kernel
Move the exit_qualification field that is used to track information about
in-flight nEPT violations from "struct kvm_vcpu_arch" to "x86_exception",
i.e. associate the information with the actual nEPT violation instead of
the vCPU. To handle bits that are pulled from vmcs.EXIT_QUALIFICATION,
i.e. that are propagated from the "original" EPT violation VM-Exit, simply
grab them from the VMCS on-demand when injecting a nEPT Violation or a PML
Full VM-exit.
Aside from being ugly, having an exit_qualification field in kvm_vcpu_arch
is outright dangerous, e.g. see commit d7f0a00e438d ("KVM: VMX: Report
up-to-date exit qualification to userspace").
Opportunstically add a comment to call out that PML Full and EPT Violation
VM-Exits use the same bit to report NMI blocking information.
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
arch/x86/include/asm/kvm_host.h | 3 ---
arch/x86/kvm/kvm_emulate.h | 1 +
arch/x86/kvm/mmu/paging_tmpl.h | 14 +++++++-------
arch/x86/kvm/vmx/nested.c | 14 ++++++++++++--
arch/x86/kvm/vmx/vmx.c | 2 --
5 files changed, 20 insertions(+), 14 deletions(-)
diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index ad5319a503f0..7ef4715d43d6 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -993,9 +993,6 @@ struct kvm_vcpu_arch {
u64 msr_kvm_poll_control;
- /* set at EPT violation at this point */
- unsigned long exit_qualification;
-
/* pv related host specific info */
struct {
bool pv_unhalted;
diff --git a/arch/x86/kvm/kvm_emulate.h b/arch/x86/kvm/kvm_emulate.h
index 4351149484fb..b5791a66637e 100644
--- a/arch/x86/kvm/kvm_emulate.h
+++ b/arch/x86/kvm/kvm_emulate.h
@@ -26,6 +26,7 @@ struct x86_exception {
bool nested_page_fault;
u64 address; /* cr2 or nested page fault gpa */
u8 async_page_fault;
+ unsigned long exit_qualification;
};
/*
diff --git a/arch/x86/kvm/mmu/paging_tmpl.h b/arch/x86/kvm/mmu/paging_tmpl.h
index 4d4e98fe4f35..7a87097cb45b 100644
--- a/arch/x86/kvm/mmu/paging_tmpl.h
+++ b/arch/x86/kvm/mmu/paging_tmpl.h
@@ -497,21 +497,21 @@ static int FNAME(walk_addr_generic)(struct guest_walker *walker,
* The other bits are set to 0.
*/
if (!(errcode & PFERR_RSVD_MASK)) {
- vcpu->arch.exit_qualification &= (EPT_VIOLATION_GVA_IS_VALID |
- EPT_VIOLATION_GVA_TRANSLATED);
+ walker->fault.exit_qualification = 0;
+
if (write_fault)
- vcpu->arch.exit_qualification |= EPT_VIOLATION_ACC_WRITE;
+ walker->fault.exit_qualification |= EPT_VIOLATION_ACC_WRITE;
if (user_fault)
- vcpu->arch.exit_qualification |= EPT_VIOLATION_ACC_READ;
+ walker->fault.exit_qualification |= EPT_VIOLATION_ACC_READ;
if (fetch_fault)
- vcpu->arch.exit_qualification |= EPT_VIOLATION_ACC_INSTR;
+ walker->fault.exit_qualification |= EPT_VIOLATION_ACC_INSTR;
/*
* Note, pte_access holds the raw RWX bits from the EPTE, not
* ACC_*_MASK flags!
*/
- vcpu->arch.exit_qualification |= (pte_access & VMX_EPT_RWX_MASK) <<
- EPT_VIOLATION_RWX_SHIFT;
+ walker->fault.exit_qualification |= (pte_access & VMX_EPT_RWX_MASK) <<
+ EPT_VIOLATION_RWX_SHIFT;
}
#endif
walker->fault.address = addr;
diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c
index 1eebed84bb65..4d0561136e70 100644
--- a/arch/x86/kvm/vmx/nested.c
+++ b/arch/x86/kvm/vmx/nested.c
@@ -409,18 +409,28 @@ static void nested_ept_inject_page_fault(struct kvm_vcpu *vcpu,
{
struct vmcs12 *vmcs12 = get_vmcs12(vcpu);
struct vcpu_vmx *vmx = to_vmx(vcpu);
+ unsigned long exit_qualification;
u32 vm_exit_reason;
- unsigned long exit_qualification = vcpu->arch.exit_qualification;
if (vmx->nested.pml_full) {
vm_exit_reason = EXIT_REASON_PML_FULL;
vmx->nested.pml_full = false;
- exit_qualification &= INTR_INFO_UNBLOCK_NMI;
+
+ /*
+ * PML Full and EPT Violation VM-Exits both use bit 12 to report
+ * "NMI unblocking due to IRET", i.e. the bit can be propagated
+ * as-is from the original EXIT_QUALIFICATION.
+ */
+ exit_qualification = vmx_get_exit_qual(vcpu) & INTR_INFO_UNBLOCK_NMI;
} else {
if (fault->error_code & PFERR_RSVD_MASK) {
vm_exit_reason = EXIT_REASON_EPT_MISCONFIG;
exit_qualification = 0;
} else {
+ exit_qualification = fault->exit_qualification;
+ exit_qualification |= vmx_get_exit_qual(vcpu) &
+ (EPT_VIOLATION_GVA_IS_VALID |
+ EPT_VIOLATION_GVA_TRANSLATED);
vm_exit_reason = EXIT_REASON_EPT_VIOLATION;
}
diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index d4e6625e0a9a..a24fba6b7741 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -5771,8 +5771,6 @@ static int handle_ept_violation(struct kvm_vcpu *vcpu)
error_code |= (exit_qualification & EPT_VIOLATION_GVA_TRANSLATED) != 0 ?
PFERR_GUEST_FINAL_MASK : PFERR_GUEST_PAGE_MASK;
- vcpu->arch.exit_qualification = exit_qualification;
-
/*
* Check that the GPA doesn't exceed physical memory limits, as that is
* a guest page fault. We have to emulate the instruction here, because
--
2.43.0.687.g38aa6559b0-goog
^ permalink raw reply [flat|nested] 5+ messages in thread* [PATCH 3/3] KVM: nVMX: Add a sanity check that nested PML Full stems from EPT Violations
2024-02-09 22:16 [PATCH 0/3] KVM: nVMX: nEPT injection fixes and cleanups Sean Christopherson
2024-02-09 22:16 ` [PATCH 1/3] KVM: nVMX: Clear EXIT_QUALIFICATION when injecting an EPT Misconfig Sean Christopherson
2024-02-09 22:16 ` [PATCH 2/3] KVM: x86: Move nEPT exit_qualification field from kvm_vcpu_arch to x86_exception Sean Christopherson
@ 2024-02-09 22:17 ` Sean Christopherson
2024-04-10 0:20 ` [PATCH 0/3] KVM: nVMX: nEPT injection fixes and cleanups Sean Christopherson
3 siblings, 0 replies; 5+ messages in thread
From: Sean Christopherson @ 2024-02-09 22:17 UTC (permalink / raw)
To: Sean Christopherson, Paolo Bonzini; +Cc: kvm, linux-kernel
Add a WARN_ON_ONCE() sanity check to verify that a nested PML Full VM-Exit
is only synthesized when the original VM-Exit from L2 was an EPT Violation.
While KVM can fallthrough to kvm_mmu_do_page_fault() if an EPT Misconfig
occurs on a stale MMIO SPTE, KVM should not treat the access as a write
(there isn't enough information to know *what* the access was), i.e. KVM
should never try to insert a PML entry in that case.
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
arch/x86/kvm/vmx/nested.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c
index 4d0561136e70..29df186dac84 100644
--- a/arch/x86/kvm/vmx/nested.c
+++ b/arch/x86/kvm/vmx/nested.c
@@ -416,6 +416,16 @@ static void nested_ept_inject_page_fault(struct kvm_vcpu *vcpu,
vm_exit_reason = EXIT_REASON_PML_FULL;
vmx->nested.pml_full = false;
+ /*
+ * It should be impossible to trigger a nested PML Full VM-Exit
+ * for anything other than an EPT Violation from L2. KVM *can*
+ * trigger nEPT page fault injection in response to an EPT
+ * Misconfig, e.g. if the MMIO SPTE was stale and L1's EPT
+ * tables also changed, but KVM should not treat EPT Misconfig
+ * VM-Exits as writes.
+ */
+ WARN_ON_ONCE(vmx->exit_reason.basic != EXIT_REASON_EPT_VIOLATION);
+
/*
* PML Full and EPT Violation VM-Exits both use bit 12 to report
* "NMI unblocking due to IRET", i.e. the bit can be propagated
--
2.43.0.687.g38aa6559b0-goog
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 0/3] KVM: nVMX: nEPT injection fixes and cleanups
2024-02-09 22:16 [PATCH 0/3] KVM: nVMX: nEPT injection fixes and cleanups Sean Christopherson
` (2 preceding siblings ...)
2024-02-09 22:17 ` [PATCH 3/3] KVM: nVMX: Add a sanity check that nested PML Full stems from EPT Violations Sean Christopherson
@ 2024-04-10 0:20 ` Sean Christopherson
3 siblings, 0 replies; 5+ messages in thread
From: Sean Christopherson @ 2024-04-10 0:20 UTC (permalink / raw)
To: Sean Christopherson, Paolo Bonzini; +Cc: kvm, linux-kernel
On Fri, 09 Feb 2024 14:16:57 -0800, Sean Christopherson wrote:
> Fix a minor bug where KVM doesn't clear EXIT_QUALIFICATION when injecting
> an EPT Misconfig into L1, and then move exit_qualification out of
> kvm_vcpu_arch to avoid recurrences of the bug fixed by commit d7f0a00e438d
> ("KVM: VMX: Report up-to-date exit qualification to userspace").
>
> Sean Christopherson (3):
> KVM: nVMX: Clear EXIT_QUALIFICATION when injecting an EPT Misconfig
> KVM: x86: Move nEPT exit_qualification field from kvm_vcpu_arch to
> x86_exception
> KVM: nVMX: Add a sanity check that nested PML Full stems from EPT
> Violations
>
> [...]
Applied to kvm-x86 vmx, thanks!
[1/3] KVM: nVMX: Clear EXIT_QUALIFICATION when injecting an EPT Misconfig
https://github.com/kvm-x86/linux/commit/0c4765140351
[2/3] KVM: x86: Move nEPT exit_qualification field from kvm_vcpu_arch to x86_exception
https://github.com/kvm-x86/linux/commit/a9466078687f
[3/3] KVM: nVMX: Add a sanity check that nested PML Full stems from EPT Violations
https://github.com/kvm-x86/linux/commit/23ffe4bbf807
--
https://github.com/kvm-x86/linux/tree/next
^ permalink raw reply [flat|nested] 5+ messages in thread