Re: [PATCH v2 2/4] KVM: SEV: Drop page refcount early during RMP fault handling

Michael Roth [off-list ref] writes:

On Tue, Aug 18, 2026 at 09:15:53AM +0000, Ackerley Tng wrote:

quoted

From: Sean Christopherson <seanjc@google.com>

When handling an RMP fault, KVM retrieves the PFN for a private GPA from
guest_memfd.

Drop the page reference immediately after retrieving the PFN instead of
holding it across the entire handler so that the later patch can follow up
with completely not returning refcounted pages from kvm_gmem_get_pfn().

Regarding this point:

quoted

On a first look, existing RMP table handling (psmash and checking for
errors) might seem like it works fine, since truncation of the page from
guest_memfd would have called rmp_make_shared() and removed the PFN from
the RMP table. However, that is insufficient since a freed page may already
be used in a different SNP VM.

In the code this patch is applied on top of, I think the kvm_gmem_get_pfn()
ref is enough to avoid the reused-by-another-SNP-VM scenario until after
caller releases the ref, so I think the above explanation should be adjusted
to also be preparatory for "the later patch".

I think this sequence of events is possible:

CPU 0: sev_handle_rmp_fault()
CPU 0:   kvm_gmem_get_pfn()
CPU 0:     filemap_invalidate_lock()
CPU 0:     refcount++
CPU 0:     filemap_invalidate_unlock()
CPU 0:     refcount-- <<== because kvm_release_page_unused(page);

CPU 1: truncate()
CPU 1:   filemap_invalidate_lock()
CPU 1:   refcount--
CPU 1:     kvm_gmem_free_folio()
CPU 1:       sev_gmem_make_shared()
CPU 1:     folio is freed
CPU 1:   filemap_invalidate_unlock()

CPU 2: in some other SNP VM,
CPU 2: kvm_gmem_get_pfn() gets the freed folio
CPU 2:   sev_gmem_make_private()

CPU 0:   snp_lookup_rmpentry(pfn, &assigned, &rmp_level);
           <<== it's assigned but to some other SNP VM
CPU 0:   snp_rmptable_psmash(pfn);
           <<== this psmash would be smashing in some other SNP VM

And hence I think we do need to check for invalidations using the MMU
invalidation protocol.

Other than that:

Reviewed-by: Michael Roth <redacted>

quoted

Hence, adopt the MMU invalidation protocol to guard committing anything
based on the PFN.

Signed-off-by: Sean Christopherson <seanjc@google.com>
Signed-off-by: Ackerley Tng <redacted>
---
 arch/x86/kvm/svm/sev.c | 39 ++++++++++++++++++++++++---------------
 1 file changed, 24 insertions(+), 15 deletions(-)

diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index b2738362a928b..b34b11d7f8fad 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c

@@ -5003,6 +5003,7 @@ void sev_handle_rmp_fault(struct kvm_vcpu *vcpu, gpa_t gpa, u64 error_code)
 	struct kvm_memory_slot *slot;
 	struct kvm *kvm = vcpu->kvm;
 	int order, rmp_level, ret;
+	unsigned long mmu_seq;
 	struct page *page;
 	bool assigned;
 	kvm_pfn_t pfn;

@@ -5030,18 +5031,22 @@ void sev_handle_rmp_fault(struct kvm_vcpu *vcpu, gpa_t gpa, u64 error_code)
 		return;
 	}

+	mmu_seq = kvm->mmu_invalidate_seq;
+	smp_rmb();
+
 	ret = kvm_gmem_get_pfn(kvm, slot, gfn, &pfn, &page, &order);
 	if (ret) {
 		pr_warn_ratelimited("SEV: Unexpected RMP fault, no backing page for private GPA 0x%llx\n",
 				    gpa);
 		return;
 	}
+	kvm_release_page_unused(page);

 	ret = snp_lookup_rmpentry(pfn, &assigned, &rmp_level);
 	if (ret || !assigned) {
 		pr_warn_ratelimited("SEV: Unexpected RMP fault, no assigned RMP entry found for GPA 0x%llx PFN 0x%llx error %d\n",
 				    gpa, pfn, ret);
-		goto out_no_trace;
+		return;
 	}

 	/*

@@ -5069,27 +5074,31 @@ void sev_handle_rmp_fault(struct kvm_vcpu *vcpu, gpa_t gpa, u64 error_code)
 	if (rmp_level == PG_LEVEL_4K)
 		goto out;

-	ret = snp_rmptable_psmash(pfn);
-	if (ret) {
-		/*
-		 * Look it up again. If it's 4K now then the PSMASH may have
-		 * raced with another process and the issue has already resolved
-		 * itself. If it's not assigned, then this must have raced with
-		 * another process that made this page shared.
-		 */
-		if (!snp_lookup_rmpentry(pfn, &assigned, &rmp_level) &&
-		    ((assigned && rmp_level == PG_LEVEL_4K) || !assigned))
+	scoped_guard(read_lock, &kvm->mmu_lock) {
+		if (mmu_invalidate_retry_gfn(kvm, mmu_seq, gfn))
 			goto out;

-		pr_warn_ratelimited("SEV: Unable to split RMP entry for GPA 0x%llx PFN 0x%llx ret %d\n",
-				    gpa, pfn, ret);
+		ret = snp_rmptable_psmash(pfn);
+		if (ret) {
+			/*
+			 * Look it up again. If it's 4K now then the PSMASH may
+			 * have raced with another process and the issue has
+			 * already resolved itself. If it's not assigned, then
+			 * this must have raced with another process that made
+			 * this page shared.
+			 */
+			if (!snp_lookup_rmpentry(pfn, &assigned, &rmp_level) &&
+			    ((assigned && rmp_level == PG_LEVEL_4K) || !assigned))
+				goto out;
+
+			pr_warn_ratelimited("SEV: Unable to split RMP entry for GPA 0x%llx PFN 0x%llx ret %d\n",
+					    gpa, pfn, ret);
+		}
 	}

 	kvm_zap_gfn_range(kvm, gfn, gfn + PTRS_PER_PMD);
 out:
 	trace_kvm_rmp_fault(vcpu, gpa, pfn, error_code, rmp_level, ret);
-out_no_trace:
-	kvm_release_page_unused(page);
 }

 static bool is_pfn_range_shared(kvm_pfn_t start, kvm_pfn_t end)

--

2.55.0.699.gb54405d56f-goog