Re: [PATCH v2 4/4] KVM: guest_memfd: Stop returning struct page from PFN lookup
Suzuki K Poulose <[email protected]>
| Newsgroups | dev.linux.lists.kvmarm,org.infradead.lists.linux-arm-kernel,org.kernel.vger.kvm,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <[email protected]> |
On 18/08/2026 10:15, Ackerley Tng wrote: > From: Sean Christopherson <[email protected]> > > KVM currently expects guest_memfd PFN lookups to return a refcounted > struct page, which callers hold across fault handling. > > Holding a page reference across fault handling is problematic for > guest_memfd. In-place memory conversions between confidential > computing shared and private states inspect folio refcounts to ensure > exclusive ownership by guest_memfd. A concurrent guest page fault > taking a reference on the folio causes conversions to fail due to an > elevated refcount. > > guest_memfd already notifies KVM of page invalidations, so callers > within KVM only need to respect the MMU invalidation protocol to safely > rely on guest_memfd for page presence. > > Furthermore, removing struct page from the guest_memfd PFN lookup moves > KVM closer toward supporting memory backends that are not backed by > struct page. > > Drop the folio reference immediately before returning from the > guest_memfd PFN lookup, and stop returning the struct page pointer. > > For ARM, initialize the local page pointer to NULL so that the shared > cleanup path that releases fault-in pages safely no-ops for guest_memfd. > > For x86, no additional changes are required in the MMU fault path > because the page fault tracking structure is zero-initialized at the > start of page fault handling, ensuring the refcounted page pointer is > already NULL. > > Reported-by: Yan Zhao <[email protected]> > Closes: https://lore.kernel.org/all/[email protected]/ > Signed-off-by: Sean Christopherson <[email protected]> > Co-developed-by: Yan Zhao <[email protected]> > Signed-off-by: Yan Zhao <[email protected]> > Co-developed-by: Ackerley Tng <[email protected]> > Signed-off-by: Ackerley Tng <[email protected]> > --- > arch/arm64/kvm/mmu.c | 4 ++-- > arch/arm64/kvm/nested.c | 4 ++-- > arch/x86/kvm/mmu/mmu.c | 2 +- > arch/x86/kvm/svm/sev.c | 8 ++------ > include/linux/kvm_host.h | 6 ++---- > virt/kvm/guest_memfd.c | 9 ++------- > 6 files changed, 11 insertions(+), 22 deletions(-) > > diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c > index 6c941aaa10c63..e5d637a5ec558 100644 > --- a/arch/arm64/kvm/mmu.c > +++ b/arch/arm64/kvm/mmu.c > @@ -1613,7 +1613,7 @@ static int gmem_abort(const struct kvm_s2_fault_desc *s2fd) > enum kvm_pgtable_prot prot = KVM_PGTABLE_PROT_R; > struct kvm_pgtable *pgt = s2fd->vcpu->arch.hw_mmu->pgt; > unsigned long mmu_seq; > - struct page *page; > + struct page *page = NULL; > struct kvm *kvm = s2fd->vcpu->kvm; > void *memcache = NULL; > kvm_pfn_t pfn; > @@ -1641,7 +1641,7 @@ static int gmem_abort(const struct kvm_s2_fault_desc *s2fd) > /* Pairs with the smp_wmb() in kvm_mmu_invalidate_end(). */ > smp_rmb(); > > - ret = kvm_gmem_get_pfn(kvm, s2fd->memslot, gfn, &pfn, &page, NULL); > + ret = kvm_gmem_get_pfn(kvm, s2fd->memslot, gfn, &pfn, NULL); > if (ret) { > kvm_prepare_memory_fault_exit(s2fd->vcpu, s2fd->fault_ipa, PAGE_SIZE, > write_fault, exec_fault, false); Since this function only deals with the gmem backed aborts, you could remove the variable and the call to kvm_release_faultin_page() below. > diff --git a/arch/arm64/kvm/nested.c b/arch/arm64/kvm/nested.c > index fb54f6dad995c..43523bb17621a 100644 > --- a/arch/arm64/kvm/nested.c > +++ b/arch/arm64/kvm/nested.c > @@ -1360,7 +1360,7 @@ static int kvm_translate_vncr(struct kvm_vcpu *vcpu, bool *is_gmem) > bool write_fault, writable; > unsigned long mmu_seq; > struct vncr_tlb *vt; > - struct page *page; > + struct page *page = NULL; > u64 va, pfn, gfn; > int ret; > > @@ -1411,7 +1411,7 @@ static int kvm_translate_vncr(struct kvm_vcpu *vcpu, bool *is_gmem) > if (is_error_noslot_pfn(pfn) || (write_fault && !writable)) > return -EFAULT; > } else { > - ret = kvm_gmem_get_pfn(vcpu->kvm, memslot, gfn, &pfn, &page, NULL); > + ret = kvm_gmem_get_pfn(vcpu->kvm, memslot, gfn, &pfn, NULL); > if (ret) { > kvm_prepare_memory_fault_exit(vcpu, vt->wr.pa, PAGE_SIZE, > write_fault, false, false); This is safe too, as we only use the page for kvm_release_faultin_page(), and it can tolerate a NULL page. So, this looks fine to me. With the cleanup above, Reviewed-by: Suzuki K Poulose <[email protected]>