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

[Suzuki K Poulose]

On 18/08/2026 10:15, 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.
>
> 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 <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>
> 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, 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 <suzuki.poulose@xxxxxxx>