Re: [PATCH v4 5/5] KVM: guest_memfd: Stop returning struct page from PFN lookup

[Gavin Shan]

On 8/26/26 7:01 PM, Ackerley Tng wrote:
> From: Sean Christopherson <seanjc@xxxxxxxxxx>
>
> KVM currently expects guest_memfd PFN lookups to return a refcounted struct
> page, which callers hold across fault handling.
>
> Drop the page's refcount before returning from kvm_gmem_get_pfn() to
> prepare for the in-place conversions series.
>
> CoCo shared-to-private conversion handling must inspect folio refcounts to
> ensure exclusive ownership by guest_memfd. A concurrent guest page fault
> taking a temporary reference on the folio causes conversions to fail due to
> an elevated refcount.
>
> While this refcount is also taken on host userspace page faults, that
> refcount is taken on behalf of the host userspace page tables. This
> refcount will be dropped when conversions unmaps the page. Either way, once
> there's an mmap() or userspace mapping, the pages are open to way more
> refcounts, transient or not. This patch focuses on just dropping refcounts
> before handing KVM a page.
>
> 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.
>
> Since the page refcounts are dropped, don't return the struct page pointer.
>
> Not returning the struct page from the guest_memfd PFN lookup moves KVM
> closer toward supporting memory backends that are not backed by struct
> page.
>
> Here are some notes on the cleanup in the callers of kvm_gmem_get_pfn():
>
> kvm_release_faultin_page() in ARM's gmem_abort() originally also serves to
> set the page dirty and accessed under some conditions. The dirty and
> accessed flags don't matter for guest_memfd anyway, so it is safe to just
> drop the call to kvm_release_faultin_page().
>
> For ARM's kvm_translate_vncr(), the local page pointer must be initialized
> to NULL so that the shared cleanup path that releases faulted-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 <yan.y.zhao@xxxxxxxxx>
> Closes: https://lore.kernel.org/all/anZ4W9o5pTWIEgMY@xxxxxxxxxxxxxxxxxxxxxxxxx/
> Signed-off-by: Sean Christopherson <seanjc@xxxxxxxxxx>
> Co-developed-by: Yan Zhao <yan.y.zhao@xxxxxxxxx>
> Signed-off-by: Yan Zhao <yan.y.zhao@xxxxxxxxx>
> Reviewed-by: Suzuki K Poulose <suzuki.poulose@xxxxxxx>
> Reviewed-by: Michael Roth <michael.roth@xxxxxxx>
> Tested-by: Michael Roth <michael.roth@xxxxxxx>
> Tested-by: Yan Zhao <yan.y.zhao@xxxxxxxxx>
> Reviewed-by: Fuad Tabba <fuad.tabba@xxxxxxxxx>
> Tested-by: Fuad Tabba <fuad.tabba@xxxxxxxxx>
> Co-developed-by: Ackerley Tng <ackerleytng@xxxxxxxxxx>
> Signed-off-by: Ackerley Tng <ackerleytng@xxxxxxxxxx>
> ---
>   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, 10 insertions(+), 23 deletions(-)

Reviewed-by: Gavin Shan <gshan@xxxxxxxxxx>