[PATCH 2/2] KVM: x86: Reset MMU back to the root MMU when leaving guest mode

[Paolo Bonzini]

From: Sean Christopherson <seanjc@xxxxxxxxxx>

Always reset a vCPU's MMU pointer back to the root MMU when leaving guest
mode, to ensure that KVM doesn't try to run L1 with a nested TDP MMU.
Dereferencing the MMU between putting the vCPU into L1 and restoring L1,
or even pivoting on mmu_is_nested(), would be a KVM bug, as the vCPU state
is guaranteed to be stale for one flow or another.

The risk of immediately resetting the MMU pointer is that doing so could
introduce a regression if either nSVM or nVMX has a correctness dependency
on buggy code, i.e. if KVM relies on the nested MMU being live between
leave_guest_mode() and the calls to nested{ept,svm}_uninit_mmu_context().
But for such a dependency to exist, KVM would either need to explicitly
check for nested TDP or make some very blind assumptions about the MMU.

A manual audit of the code confirmed that vcpu->arch.mmu isn't dereferenced
in the described window, and hacking KVM to poison the pointer and WARN if
KVM tries to pivot on the pointer with half-baked state didn't trigger any
splats while running KVM-Unit-Tests, KVM selftests, and Linux guests.  I.e.
running with these hacks didn't trigger any badness:

  static inline bool mmu_is_nested(struct kvm_vcpu *vcpu)
  {
	WARN_ON_ONCE(vcpu->arch.mmu == &vcpu->arch.guest_mmu &&
		     !is_guest_mode(vcpu));
	return vcpu->arch.mmu == &vcpu->arch.guest_mmu;
  }

  static inline void leave_guest_mode(struct kvm_vcpu *vcpu)
  {
	if (vcpu->arch.mmu != &vcpu->arch.root_mmu)
		vcpu->arch.mmu = LIST_POISON1;

	vcpu->arch.hflags &= ~HF_GUEST_MASK;

	if (vcpu->arch.load_eoi_exitmap_pending) {
		vcpu->arch.load_eoi_exitmap_pending = false;
		kvm_make_request(KVM_REQ_LOAD_EOI_EXITMAP, vcpu);
	}

	vcpu->stat.guest_mode = 0;
  }

Signed-off-by: Sean Christopherson <seanjc@xxxxxxxxxx>
Link: https://patch.msgid.link/20261001214356.3825100-3-seanjc@xxxxxxxxxx
Signed-off-by: Paolo Bonzini <pbonzini@xxxxxxxxxx>
---
 arch/x86/kvm/regs.h       |  2 ++
 arch/x86/kvm/svm/nested.c |  8 --------
 arch/x86/kvm/vmx/nested.c | 12 +-----------
 3 files changed, 3 insertions(+), 19 deletions(-)

diff --git a/arch/x86/kvm/regs.h b/arch/x86/kvm/regs.h
index 447f0ec3e63e..b29dd24a4531 100644
--- a/arch/x86/kvm/regs.h
+++ b/arch/x86/kvm/regs.h
@@ -463,6 +463,8 @@ static inline void enter_guest_mode(struct kvm_vcpu *vcpu)
 
 static inline void leave_guest_mode(struct kvm_vcpu *vcpu)
 {
+	vcpu->arch.mmu = &vcpu->arch.root_mmu;
+
 	vcpu->arch.hflags &= ~HF_GUEST_MASK;
 
 	if (vcpu->arch.load_eoi_exitmap_pending) {
diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
index f9090b601efa..50f12cdeaa7c 100644
--- a/arch/x86/kvm/svm/nested.c
+++ b/arch/x86/kvm/svm/nested.c
@@ -119,11 +119,6 @@ static void nested_svm_init_mmu_context(struct kvm_vcpu *vcpu)
 	vcpu->arch.ngpa_walk.inject_page_fault = nested_svm_inject_npf_exit;
 }
 
-static void nested_svm_uninit_mmu_context(struct kvm_vcpu *vcpu)
-{
-	vcpu->arch.mmu = &vcpu->arch.root_mmu;
-}
-
 static bool nested_vmcb_needs_vls_intercept(struct vcpu_svm *svm)
 {
 	if (!guest_cpu_cap_has(&svm->vcpu, X86_FEATURE_V_VMSAVE_VMLOAD))
@@ -1432,8 +1427,6 @@ void nested_svm_vmexit(struct vcpu_svm *svm)
 
 	nested_svm_transition_tlb_flush(vcpu);
 
-	nested_svm_uninit_mmu_context(vcpu);
-
 	if (nested_svm_load_cr3(vcpu, vmcb01->save.cr3, false, true))
 		kvm_make_request(KVM_REQ_TRIPLE_FAULT, vcpu);
 
@@ -1549,7 +1542,6 @@ void svm_leave_nested(struct kvm_vcpu *vcpu)
 
 		svm_switch_vmcb(svm, &svm->vmcb01);
 
-		nested_svm_uninit_mmu_context(vcpu);
 		vmcb_mark_all_dirty(svm->vmcb);
 
 		svm_set_gif(svm, true);
diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c
index 5165275128fc..6f18c236e3c6 100644
--- a/arch/x86/kvm/vmx/nested.c
+++ b/arch/x86/kvm/vmx/nested.c
@@ -525,11 +525,6 @@ static void nested_ept_init_mmu_context(struct kvm_vcpu *vcpu)
 	vcpu->arch.ngpa_walk.inject_page_fault = nested_ept_inject_page_fault;
 }
 
-static void nested_ept_uninit_mmu_context(struct kvm_vcpu *vcpu)
-{
-	vcpu->arch.mmu = &vcpu->arch.root_mmu;
-}
-
 static bool nested_vmx_is_page_fault_vmexit(struct vmcs12 *vmcs12,
 					    u16 error_code)
 {
@@ -3770,10 +3765,8 @@ enum nvmx_vmentry_status nested_vmx_enter_non_root_mode(struct kvm_vcpu *vcpu,
 vmentry_fail_vmexit:
 	vmx_switch_vmcs(vcpu, &vmx->vmcs01);
 
-	if (!from_vmentry) {
-		nested_ept_uninit_mmu_context(vcpu);
+	if (!from_vmentry)
 		return NVMX_VMENTRY_VMEXIT;
-	}
 
 	nested_put_vmcs12_pages(vcpu);
 
@@ -4833,8 +4826,6 @@ static void load_vmcs12_host_state(struct kvm_vcpu *vcpu,
 	vcpu->arch.cr4_guest_owned_bits = ~vmcs_readl(CR4_GUEST_HOST_MASK);
 	vmx_set_cr4(vcpu, vmcs12->host_cr4);
 
-	nested_ept_uninit_mmu_context(vcpu);
-
 	/*
 	 * Only PDPTE load can fail as the value of cr3 was checked on entry and
 	 * couldn't have changed.
@@ -4998,7 +4989,6 @@ static void nested_vmx_restore_host_state(struct kvm_vcpu *vcpu)
 	vcpu->arch.cr4_guest_owned_bits = ~vmcs_readl(CR4_GUEST_HOST_MASK);
 	vmx_set_cr4(vcpu, vmcs_readl(CR4_READ_SHADOW));
 
-	nested_ept_uninit_mmu_context(vcpu);
 	vcpu->arch.cr3 = vmx->nested.pre_vmenter_cr3;
 	kvm_register_mark_available(vcpu, VCPU_REG_CR3);
 
-- 
2.56.0