Re: [PATCH RFC v3 48/88] samples/kvm: Support dynamic revocation via kvm_gmem_revoke

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Low] samples/kvm: stale identifier names and patch references in
  commit message
--

commit 42b1d24c6caccfeaf35dd92048463c002a041484
Author: Ackerley Tng <ackerleytng@google.com>

> samples/kvm: Support dynamic revocation via kvm_gmem_revoke
>
> Support dynamic revocation in the sample memory provider module by
> calling kvm_gmem_revoke() when pages are marked absent via
> MPS_SET_PRESENT.
>
> Note down the attaching gmem inode during .attach() and disallow double
> attachment to the same provider file.
>
> Clear the revoked pages after kvm_gmem_revoke() using mps_zero_range()
> to ensure that stale data is wiped and cannot be observed when pages are
> later marked present or reused.
>
> Regarding locking and ordering:
> - In mps_set_present(), mark the absent bitmap first before revoking so
>   any concurrent page fault immediately sees the page as absent and
>   rejects the lookup with -EFAULT.
> - kvm_gmem_revoke() is called under info->lock to fully serialize
>   MPS_SET_PRESENT operations against each other and prevent races
>   between marking pages absent and present.
> - The PFN passed to kvm_gmem_revoke() is derived from info->base_pfn +
>   start. The mapping between page index and physical PFN is static and
>   immutable for the lifetime of the provider file, ensuring that the
>   pfn mapping remains completely stable across the revoke call.
> - Passing pfn directly to kvm_gmem_revoke() allows guest_memfd to
>   reclaim the physical range directly without calling back into the
>   provider. This keeps the lock hierarchy strictly top-down (info->lock
>   -> filemap_invalidate_lock -> mmu_lock) with no callbacks or deadlocks.
>
> For now, nothing needs to be done when setting a page present, since users
> should (re-)fault and (re-)obtain the PFN via .get_pfn(), to be added in a
> later patch.
>
> For users that don't support faulting, like the IOMMU, the counterpart to
> kvm_gmem_revoke() will be made available for the first user to indicate
> that the memory is now available. guest_memfd should then do the page table
> mappings as necessary, based on the shared/private status.
>
> Signed-off-by: Ackerley Tng <ackerleytng@google.com>

[Severity: Low]
This isn't a bug, but the commit message has a few stale references from an
earlier version of the series:

- MPS_SET_PRESENT, mps_zero_range(), and mps_set_present() were renamed to
  MP_SET_PRESENT, mp_zero_range(), and mp_set_present().
- Recording info->gmem_inode in .attach() and disallowing double attachment
  was already done in commit "samples/kvm: Implement attach and release
  provider ops in sample provider", not in this patch.
- .get_pfn() was already added earlier in the series in commit "samples/kvm:
  Implement get_pfn provider operation", rather than in a later patch.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261106-gmem-alternative-providers-v3-0-8484dde66a7e@google.com?part=48