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

[Yan Zhao]

On Thu, Jul 23, 2026 at 11:47:42AM -0700, Sean Christopherson wrote:
> 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.
Ok. I'm ok with the change for today's usages. My concern is regarding future
TDX huge page support, as I am currently preparing TDX huge page v4. :)

Sorry for the confusion -- I should have stated my intention more clearly.

Previously, for TDX huge pages, you suggested introducing .gmem_convert() to
trigger splitting before zapping S-EPT.
With this new direction, should TDX huge pages instead leverage the
.gmem_make_shared() op for that purpose?

If so, should we introduce a CONFIG_HAVE_KVM_ARCH_GMEM_PREZAP guard around the
.gmem_make_shared() invocation to serve TDX's splitting purpose, in order to
keep the two use cases (SNP and TDX) clearly separated?

> > > 	#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.
If TDX also provides a .gmem_make_shared() callback for the pre-zap (splitting)
step during private-to-shared conversions, my concern is that TDX could be
inadvertently affected if CONFIG_HAVE_KVM_ARCH_GMEM_RECLAIM is unintentionally
selected.
Should we provide a way to prevent TDX from accidentally enabling
CONFIG_HAVE_KVM_ARCH_GMEM_RECLAIM?

> > 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 :-)
Understood, I noted that it is tagged as *** DO NOT MERGE ***.

However, for TDX huge page v4, should I continue tagging the patch as
*** DO NOT MERGE *** and keep it in the series? The reason I ask is that patch
[2], which immediately follows the *** DO NOT MERGE *** patch, depends on it, as
patch [2] adds the following line in kvm_arch_gmem_convert():
	return kvm_x86_call(gmem_convert)(kvm, start, end, to_private);

[2]https://lore.kernel.org/all/aXt_L6QKB9CSTZcW@xxxxxxxxxx/

> > @@ -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@xxxxxxxxxx/
> > 
> > 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.
Hmm, if .gmem_make_shared() will be used by both SNP and TDX in the future, then
it will be invoked:
- in __kvm_gmem_set_attributes() before kvm_gmem_zap() (which can fail) for TDX.
- in __kvm_gmem_set_attributes() after kvm_gmem_zap() (where can't fail) for SNP.
- in kvm_gmem_punch_hole() before kvm_gmem_zap() for TDX.
- in kvm_gmem_free_folio() for SNP.

Since both TDX and SNP would provide callbacks for the .gmem_make_shared() op
but invoke it at different points with different failure semantics, we need to
provide different CONFIGs and prevent them from being inadvertently selected by
an unintended user.

So for simplicity, could I just rename .gmem_convert() to .gmem_prezap()
(instead of to .gmem_make_shared()) for TDX in TDX huge page v4?