Re: [PATCH v2 2/4] KVM: SEV: Drop page refcount early during RMP fault handling
Michael Roth <[email protected]>
| Newsgroups | gmane.linux.kernel,gmane.comp.emulators.kvm.devel,gmane.linux.ports.arm.kernel |
|---|---|
| Message-ID | <j6bfvbzjivlcsu3slbnaoomxxqyypmfl2bupo3k5ipnt6fc5lo@e5ntbx7yd4qw> |
On Tue, Aug 18, 2026 at 09:15:53AM +0000, Ackerley Tng wrote: > From: Sean Christopherson <[email protected]> > > 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: > > 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". Other than that: Reviewed-by: Michael Roth <[email protected]> > > Hence, adopt the MMU invalidation protocol to guard committing anything > based on the PFN. > > Signed-off-by: Sean Christopherson <[email protected]> > Signed-off-by: Ackerley Tng <[email protected]> > --- > 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 >