Re: [PATCH v4 18/18] KVM: guest_memfd: Combine .gmem_prepare()+.gmem_invalidate() into .gmem_convert()

[Yan Zhao]

Sorry, I just noticed this patch.
Though it may be too late, I have some questions that have been puzzling me for
a while :)

On Thu, Jul 09, 2026 at 01:49:48PM -0700, Sean Christopherson wrote:
> Smush guest_memfd's prepare() and invalidate() hooks into a common
> convert() flow, as they are effectively two sides of the same coin: they're
> invoked when private/shared memory is about to made accessible visible to
> the guest/host.  I.e. prepare() is really "make private", and invalidate()
> is really "make shared".
> 
> Using a single API fixes the wart where invalidate() isn't actually doing
> invalidation, avoids adding yet another gmem Kconfig, avoids having to come
> up with a name to describe the "make shared" behavior, and will yield more
> intuitive code when in-place conversion comes along.
This is reasonable for SEV-SNP, and there is no functional change.

> @@ -802,7 +801,7 @@ int kvm_gmem_get_pfn(struct kvm *kvm, struct kvm_memory_slot *slot,
>  		folio_mark_uptodate(folio);
>  	}
>  
> -	r = kvm_gmem_prepare_folio(kvm, slot, gfn, folio);
> +	r = kvm_gmem_make_private(kvm, slot, gfn, folio);
What puzzles me is that kvm_gmem_get_pfn() can be invoked to fault in pages for
gmem-only slots in normal VMs, where kvm_arch_has_private_mem() == false.

Would it be odd to invoke .gmem_convert() with to_private being true in such
VMs? Or do you plan to add something similar to the following in the future?
        if (kvm_gmem_is_private_mem(inode, index))
	     r = kvm_gmem_make_private(kvm, slot, gfn, folio);
	else
	     r = kvm_gmem_make_shared(kvm, slot, gfn, folio);

Otherwise, would it look asymmetric with just:
        if (kvm_gmem_is_private_mem(inode, index))
	     r = kvm_gmem_make_private(kvm, slot, gfn, folio);
in kvm_gmem_get_pfn()?

Asking this also because there is a .gmem_convert() for TDX huge pages [1].
In [1], .gmem_convert() is invoked to emulate a to-shared conversion in
kvm_gmem_punch_hole(). However, the per-gmem memory attribute for the range to
convert may not be shared after the punch hole. Is it acceptable?
(To me, the .gmem_convert() in [1] behaves more like .gmem_prezap()).

Additionally, would the .gmem_convert() to shared in punch hole be duplicated
with the one in kvm_gmem_free_folio() when CONFIG_HAVE_KVM_ARCH_GMEM_RECLAIM is
true?

So, I'm writing to understand your considerations behind this for our future
direction.

[1] https://lore.kernel.org/all/20260129011517.3545883-44-seanjc@xxxxxxxxxx

>  	folio_unlock(folio);
>  
> -- 
> 2.55.0.795.g602f6c329a-goog
> 
>