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

[Sean Christopherson <seanjc@google.com>]

On Wed, Jul 22, 2026, Yan Zhao wrote:
> On Tue, Jul 21, 2026 at 11:57:06AM -0700, Sean Christopherson wrote:
> > > 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()).
> > 
> > Ya, these concerns got raised by others.  pKVM on arm64 in particular wants to
> > hook reclaim but not conversion.  The plan is to keep the reclaim and end up with
> > this implementation for x86:
> > 
> > 	#ifdef CONFIG_HAVE_KVM_ARCH_GMEM_CONVERT
> > 	int kvm_arch_gmem_make_private(struct kvm *kvm, gfn_t gfn, kvm_pfn_t pfn,
> > 				kvm_pfn_t nr_pages, int max_order)
> > 	{
> > 		return kvm_x86_call(gmem_make_private)(kvm, gfn, pfn, nr_pages, max_order);
> > 	}
> > 	int kvm_arch_gmem_make_shared(kvm_pfn_t pfn, kvm_pfn_t nr_pages, int max_order,
> > 				bool to_private)
> > 	{
> > 		kvm_x86_call(gmem_make_shared)(pfn, nr_pages, max_order);
> > 		return 0;
> > 	}
> For TDX huge pages, if we want to trigger private huge page splitting before
> converting to shared, should we invoke the hooks like this?
> 
> __kvm_gmem_set_attributes(to shared)
> 	|->kvm_arch_gmem_make_shared
> 		|->kvm_x86_call(gmem_make_shared)(pfn, nr_pages, max_order);
> 
> But TDX needs kvm pointer, and splitting pages may fail.

Ya, but those are very solvable problems.  They just don't need to be addressed
today, because SNP is the only user of the conversion APIs.

> > 	#endif
> > 
> > 	#ifdef CONFIG_HAVE_KVM_ARCH_GMEM_RECLAIM
> > 	void kvm_arch_gmem_reclaim(kvm_pfn_t pfn, kvm_pfn_t nr_pages, int max_order)
> > 	{
> > 		kvm_x86_call(gmem_make_shared)(pfn, nr_pages, max_order);
> > 	}
> > 	#endif
> Is this kvm_arch_gmem_reclaim() invoked by kvm_gmem_free_folio(), and should TDX
> not define CONFIG_HAVE_KVM_ARCH_GMEM_RECLAIM?

Correct.

> As in [1], for TDX huge pages, you suggested pretending a to-shared conversion
> in kvm_gmem_punch_hole(). In that case, should we provide a new CONFIG_xxx to
> prevent it from being invoked by SNP?

No?  That code was purely for demonstration purpose, I there was zero intent to
ever land it.  The patch was tagged *** DO NOT MERGE *** for a reason :-)

> @@ -253,13 +294,18 @@ static long kvm_gmem_punch_hole(struct inode *inode, loff_t offset, loff_t len)
> 
>  	kvm_gmem_invalidate_begin(inode, start, end);
> 
> -	truncate_inode_pages_range(inode->i_mapping, offset, offset + len - 1);
> +	/*
> +	 * For demonstration purposes, pretend this is a private=>shared conversion.
> +	 */
> +	r = kvm_gmem_convert(inode, start, end, false);
> +	if (!r)
> +		truncate_inode_pages_range(inode->i_mapping, offset, offset + len - 1);
> 
>  	kvm_gmem_invalidate_end(inode, start, end);
> 
>  	filemap_invalidate_unlock(inode->i_mapping);
> 
> -	return 0;
> +	return r;
>  }
> [1] https://lore.kernel.org/all/20260129011517.3545883-44-seanjc@google.com/
> 
> Or would the following approach acceptable to you ? It renames .gmem_convert()
> to .gmem_prezap() and invokes it before each kvm_gmem_zap(), so TDX can hook it
> to perform page splitting before the actual zaps on private pages.
> Per my understanding, this op servers a different purpose from
> .gmem_make_private()/.gmem_make_shared() in this patch.

Isn't that just kvm_arch_gmem_invalidate_range()?  Which was added to fix the
SNP VMSA mess.  The only thing that's missing is graceful handling of failure.