Re: [PATCH v2 4/4] KVM: guest_memfd: Stop returning struct page from PFN lookup

[Ackerley Tng]

Yan Zhao <yan.y.zhao@xxxxxxxxx> writes:

> On Tue, Aug 18, 2026 at 09:15:55AM +0000, 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.
>>
>> 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.
> Nit:
> As this series is based on kvm-x86/next, where there's no in-place memory
> conversion yet, kvm_gmem_get_pfn() does not hold shared filemap invalidate lock.
>
> However, the benefit of dropping the folio reference immediately before
> returning from the guest_memfd PFN lookup -- preventing conversion failures due
> to an elevated refcount -- should be effective only if the reference is dropped
> before releasing the shared filemap invalidate lock.
>
> Do we need to make this info clear, since I think it's important? :)
>

Is this what you meant?

  In-place conversions uses the filemap_invalidate_lock() for
  synchronization of shared/private state. In kvm_gmem_get_pfn(), the
  PFN needs to be prepared according to its shared/private state. Hence,
  the filemap_invalidate_lock() is held while guest_memfd gets a folio
  and decides to make private before returning a PFN.

  In kvm_gmem_get_pfn(), the folio refcount is dropped before releasing
  filemap_invalidate_lock(). This ensures that a competing conversion
  grabbing the filemap_invalidate_lock() will never see an elevated
  refcount due to guest_memfd's folio-getting process.

It seems a bit weird to fit this into the commit message for this
patch. I think I could put the above two paragraphs into the patch that
introduces the filemap_invalidate_lock in kvm_gmem_get_pfn()?

>> 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.
>
> Could we also explain why the lack of SetPageDirty() (and mark_page_accessed())
> for a gmem page, due to NULL being passed to kvm_release_faultin_page(), is
> harmless?
>

Sounds good. What do you think of this, continuing from the paragraph
beginning "Furthermore":

  Drop the folio reference immediately before returning from the
  guest_memfd PFN lookup, and stop returning the struct page pointer.

  ARM's gmem_abort() is guest_memfd specific. Since guest_memfd no longer
  returns a page pointer, there's also no need to do any
  freeing. kvm_release_faultin_page() 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(), 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.

>>
>> [...snip...]
>>