lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20250205182402.2147495-8-yosry.ahmed@linux.dev>
Date: Wed,  5 Feb 2025 18:23:56 +0000
From: Yosry Ahmed <yosry.ahmed@...ux.dev>
To: Sean Christopherson <seanjc@...gle.com>,
	Paolo Bonzini <pbonzini@...hat.com>
Cc: kvm@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: [RFC PATCH 07/13] KVM: nSVM: Handle INVLPGA interception correctly

Currently, INVPLGA interception handles it like INVLPG, which flushes
L1's TLB translations for the address. It was implemented in this way
because L1 and L2 shared an ASID. Now, L1 and L2 have separate ASIDs. It
is still harmless to flush L1's translations, but it's only correct
because all translations are flushed on nested transitions anyway.

In preparation for stopping unconditional flushes on nested transitions,
handle INVPLGA interception properly. If L1 specified zero as the ASID,
this is equivalent to INVLPG, so handle it as such. Otherwise, use
INVPLGA to flush the translations of the appropriate ASID tracked by
KVM, if any. Sync the shadow MMU as well, as L1 invalidated L2's
mappings.

Opportunistically update svm_flush_tlb_gva() to use
svm->current_vmcb->asid instead of svm->vmcb->control.asid for
consistency. The two should always be in sync except when KVM allocates
a new ASID in pre_svm_run(), and they are shortly brought back in sync
in svm_vcpu_run(). However, if future changes add more code paths where
KVM allocates a new ASID, flushing the potentially old ASID in
svm->vmcb->control.asid would be unnecessary overhead (although probably
not much different from flushing the newly allocated ASID).

Signed-off-by: Yosry Ahmed <yosry.ahmed@...ux.dev>
---
 arch/x86/include/asm/kvm_host.h |  2 ++
 arch/x86/kvm/mmu/mmu.c          |  5 +++--
 arch/x86/kvm/svm/svm.c          | 40 ++++++++++++++++++++++++++++++---
 3 files changed, 42 insertions(+), 5 deletions(-)

diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 5193c3dfbce15..1e147bb2e560f 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -2213,6 +2213,8 @@ int kvm_mmu_page_fault(struct kvm_vcpu *vcpu, gpa_t cr2_or_gpa, u64 error_code,
 		       void *insn, int insn_len);
 void kvm_mmu_print_sptes(struct kvm_vcpu *vcpu, gpa_t gpa, const char *msg);
 void kvm_mmu_invlpg(struct kvm_vcpu *vcpu, gva_t gva);
+void __kvm_mmu_invalidate_addr(struct kvm_vcpu *vcpu, struct kvm_mmu *mmu,
+			       u64 addr, unsigned long roots, bool gva_flush);
 void kvm_mmu_invalidate_addr(struct kvm_vcpu *vcpu, struct kvm_mmu *mmu,
 			     u64 addr, unsigned long roots);
 void kvm_mmu_invpcid_gva(struct kvm_vcpu *vcpu, gva_t gva, unsigned long pcid);
diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
index ac133abc9c173..f5e0d2c8f4bbe 100644
--- a/arch/x86/kvm/mmu/mmu.c
+++ b/arch/x86/kvm/mmu/mmu.c
@@ -6158,8 +6158,8 @@ static void kvm_mmu_invalidate_addr_in_root(struct kvm_vcpu *vcpu,
 	write_unlock(&vcpu->kvm->mmu_lock);
 }
 
-static void __kvm_mmu_invalidate_addr(struct kvm_vcpu *vcpu, struct kvm_mmu *mmu,
-				      u64 addr, unsigned long roots, bool gva_flush)
+void __kvm_mmu_invalidate_addr(struct kvm_vcpu *vcpu, struct kvm_mmu *mmu,
+			       u64 addr, unsigned long roots, bool gva_flush)
 {
 	int i;
 
@@ -6185,6 +6185,7 @@ static void __kvm_mmu_invalidate_addr(struct kvm_vcpu *vcpu, struct kvm_mmu *mmu
 			kvm_mmu_invalidate_addr_in_root(vcpu, mmu, addr, mmu->prev_roots[i].hpa);
 	}
 }
+EXPORT_SYMBOL_GPL(__kvm_mmu_invalidate_addr);
 
 void kvm_mmu_invalidate_addr(struct kvm_vcpu *vcpu, struct kvm_mmu *mmu,
 			     u64 addr, unsigned long roots)
diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index a2d601cd4c283..9e29f87d3bd93 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -2483,6 +2483,7 @@ static int clgi_interception(struct kvm_vcpu *vcpu)
 
 static int invlpga_interception(struct kvm_vcpu *vcpu)
 {
+	struct vcpu_svm *svm = to_svm(vcpu);
 	gva_t gva = kvm_rax_read(vcpu);
 	u32 asid = kvm_rcx_read(vcpu);
 
@@ -2492,8 +2493,41 @@ static int invlpga_interception(struct kvm_vcpu *vcpu)
 
 	trace_kvm_invlpga(to_svm(vcpu)->vmcb->save.rip, asid, gva);
 
-	/* Let's treat INVLPGA the same as INVLPG (can be optimized!) */
-	kvm_mmu_invlpg(vcpu, gva);
+	/*
+	 * APM is silent about using INVLPGA to flush the host ASID (i.e. 0).
+	 * Do the logical thing and handle it like INVLPG.
+	 */
+	if (asid == 0) {
+		kvm_mmu_invlpg(vcpu, gva);
+		return kvm_skip_emulated_instruction(vcpu);
+	}
+
+	/*
+	 * Check if L1 specified the L2 ASID we are currently tracking. If it
+	 * isn't, do nothing as we have to handle the TLB flush when switching
+	 * to the new ASID anyway. APM mentions that INVLPGA is typically only
+	 * meaningful with shadow paging, so also do nothing if L1 is using
+	 * nested NPT.
+	 */
+	if (!nested_npt_enabled(svm) && asid == svm->nested.last_asid)
+		invlpga(gva, svm->nested.vmcb02.asid);
+
+	/*
+	 * If NPT is disabled, sync the shadow page tables as L1 is invalidating
+	 * mappings for L2. Sync all roots as ASIDs are not tracked in the MMU
+	 * role.
+	 *
+	 * As we are not flushing the current context, skip the gva flush from
+	 * __kvm_mmu_invalidate_addr(), it would flush the wrong ASID anyway.
+	 * The correct TLB flush was done above (if needed).
+	 *
+	 * This always operates on root_mmu because L1 and L2 share an MMU when
+	 * NPT is disabled. This can be optimized by invalidating guest roots
+	 * only.
+	 */
+	if (!npt_enabled)
+		__kvm_mmu_invalidate_addr(vcpu, &vcpu->arch.root_mmu, gva,
+					  KVM_MMU_ROOTS_ALL, false);
 
 	return kvm_skip_emulated_instruction(vcpu);
 }
@@ -4017,7 +4051,7 @@ static void svm_flush_tlb_gva(struct kvm_vcpu *vcpu, gva_t gva)
 {
 	struct vcpu_svm *svm = to_svm(vcpu);
 
-	invlpga(gva, svm->vmcb->control.asid);
+	invlpga(gva, svm->current_vmcb->asid);
 }
 
 static void svm_flush_tlb_guest(struct kvm_vcpu *vcpu)
-- 
2.48.1.362.g079036d154-goog


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ