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

[Yan Zhao]

On Tue, Jul 21, 2026 at 11:57:06AM -0700, Sean Christopherson wrote:
> On Wed, Jul 15, 2026, Yan Zhao wrote:
> > > @@ -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()?
> 
> Heh, I should have read this sooner.  With help from Xiaoyao and Ackerley, I
> eventually got to the same place.
> 
> https://lore.kernel.org/all/al-aDRTbDOPT1UbM@xxxxxxxxxx
Thanks for the link.
Let me just reply here for easier follow-up of the concern regarding TDX huge
pages, which I think is not addressed by the code below.

> > 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.

> 	#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?

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?

@@ -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.

commit 96bf42273e12297fb2192d16add160b169d76157
Author: Sean Christopherson <seanjc@xxxxxxxxxx>
Date:   Fri Jun 26 15:41:24 2026 +0800

    KVM: guest_memfd: Add pre-zap arch hook

    Add a gmem "pre-zap" hook to allow arch code to take action before a zap,
    e.g., for shared<=>private conversion, and just as importantly, to let arch
    code reject/fail a zap, e.g. if the conversion requires new page tables and
    KVM hits in OOM situation.

    The arch code will be used by TDX to split hugepages as necessary to avoid
    overzapping PTEs, which for all intents and purposes corrupts guest data
    for TDX VMs (memory is wiped when private PTEs are removed).

    Signed-off-by: Sean Christopherson <seanjc@xxxxxxxxxx>
    [Yan: Renamed to prezap(), used attr_filter to specify root private info ]
    Signed-off-by: Yan Zhao <yan.y.zhao@xxxxxxxxx>
    ---
    rebased to gmem in-place conversion v8

diff --git a/arch/x86/include/asm/kvm-x86-ops.h b/arch/x86/include/asm/kvm-x86-ops.h
index 588563dfe88d..f3228419437f 100644
--- a/arch/x86/include/asm/kvm-x86-ops.h
+++ b/arch/x86/include/asm/kvm-x86-ops.h
@@ -149,6 +149,7 @@ KVM_X86_OP_OPTIONAL(alloc_apic_backing_page)
 KVM_X86_OP_OPTIONAL_RET0(gmem_prepare)
 KVM_X86_OP_OPTIONAL_RET0(gmem_max_mapping_level)
 KVM_X86_OP_OPTIONAL(gmem_invalidate)
+KVM_X86_OP_OPTIONAL_RET0(gmem_prezap)
 #endif

 #undef KVM_X86_OP
diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 382a1db48a3d..f762e51dcd31 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -2010,6 +2010,8 @@ struct kvm_x86_ops {
        gva_t (*get_untagged_addr)(struct kvm_vcpu *vcpu, gva_t gva, unsigned int flags);
        void *(*alloc_apic_backing_page)(struct kvm_vcpu *vcpu);
        int (*gmem_prepare)(struct kvm *kvm, kvm_pfn_t pfn, gfn_t gfn, int max_order);
+       int (*gmem_prezap)(struct kvm *kvm, gfn_t start, gfn_t end,
+                          enum kvm_gfn_range_filter attr_filter);
        void (*gmem_invalidate)(kvm_pfn_t start, kvm_pfn_t end);
        int (*gmem_max_mapping_level)(struct kvm *kvm, kvm_pfn_t pfn, bool is_private);
 };
diff --git a/arch/x86/kvm/Kconfig b/arch/x86/kvm/Kconfig
index a3c189d76515..331951e50eac 100644
--- a/arch/x86/kvm/Kconfig
+++ b/arch/x86/kvm/Kconfig
@@ -147,6 +147,7 @@ config KVM_INTEL_TDX
        default y
        depends on INTEL_TDX_HOST
        select HAVE_KVM_ARCH_GMEM_POPULATE
+       select HAVE_KVM_ARCH_GMEM_PREZAP
        help
          Provides support for launching Intel Trust Domain Extensions (TDX)
          confidential VMs on Intel processors.
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 414b1a2b4317..d5bb7fe43328 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -14176,6 +14176,13 @@ void kvm_arch_gmem_invalidate(kvm_pfn_t start, kvm_pfn_t end)
        kvm_x86_call(gmem_invalidate)(start, end);
 }
 #endif
+#ifdef CONFIG_HAVE_KVM_ARCH_GMEM_PREZAP
+int kvm_arch_gmem_prezap(struct kvm *kvm, gfn_t start, gfn_t end,
+                        enum kvm_gfn_range_filter attr_filter)
+{
+       return kvm_x86_call(gmem_prezap)(kvm, start, end, attr_filter);
+}
+#endif
 #endif

 int kvm_spec_ctrl_test_value(u64 value)
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 9c1cf1a6559e..95350f6cec0b 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -2624,6 +2624,11 @@ long kvm_gmem_populate(struct kvm *kvm, gfn_t start_gfn, void __user *src,
                       kvm_gmem_populate_cb post_populate, void *opaque);
 #endif

+#ifdef CONFIG_HAVE_KVM_ARCH_GMEM_PREZAP
+int kvm_arch_gmem_prezap(struct kvm *kvm, gfn_t start, gfn_t end,
+                        enum kvm_gfn_range_filter attr_filter);
+#endif
+
 #ifdef CONFIG_HAVE_KVM_ARCH_GMEM_INVALIDATE
 void kvm_arch_gmem_invalidate(kvm_pfn_t start, kvm_pfn_t end);
 #endif
diff --git a/include/linux/kvm_types.h b/include/linux/kvm_types.h
index a568d8e6f4e8..4e4d40e13177 100644
--- a/include/linux/kvm_types.h
+++ b/include/linux/kvm_types.h
@@ -49,7 +49,7 @@ struct kvm_vcpu_init;
 struct kvm_memslots;

 enum kvm_mr_change;
-
+enum kvm_gfn_range_filter;
 /*
  * Address types:
  *
diff --git a/virt/kvm/Kconfig b/virt/kvm/Kconfig
index cfa2c78ba5fb..ea43bac4e51e 100644
--- a/virt/kvm/Kconfig
+++ b/virt/kvm/Kconfig
@@ -116,3 +116,7 @@ config HAVE_KVM_ARCH_GMEM_INVALIDATE
 config HAVE_KVM_ARCH_GMEM_POPULATE
        bool
        depends on KVM_GUEST_MEMFD
+
+config HAVE_KVM_ARCH_GMEM_PREZAP
+       bool
+       depends on KVM_GUEST_MEMFD
diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c
index ef331500569b..d0a61a130bb1 100644
--- a/virt/kvm/guest_memfd.c
+++ b/virt/kvm/guest_memfd.c
@@ -247,6 +247,46 @@ static enum kvm_gfn_range_filter kvm_gmem_get_invalidate_filter(
        return filter;
 }

+#ifdef CONFIG_HAVE_KVM_ARCH_GMEM_PREZAP
+static int __kvm_gmem_prezap(struct gmem_file *f, pgoff_t start, pgoff_t end,
+                            enum kvm_gfn_range_filter attr_filter)
+{
+       struct kvm_memory_slot *slot;
+       unsigned long index;
+       int r;
+
+       xa_for_each_range(&f->bindings, index, slot, start, end - 1) {
+               r = kvm_arch_gmem_prezap(f->kvm,
+                                        kvm_gmem_get_start_gfn(slot, start),
+                                        kvm_gmem_get_end_gfn(slot, end),
+                                        attr_filter);
+               if (r)
+                       return r;
+       }
+       return 0;
+}
+
+static int kvm_gmem_prezap(struct inode *inode, pgoff_t start, pgoff_t end)
+{
+       enum kvm_gfn_range_filter attr_filter;
+       struct gmem_file *f;
+       int r;
+
+       attr_filter = kvm_gmem_get_invalidate_filter(inode, start, end);
+       kvm_gmem_for_each_file(f, inode) {
+               r = __kvm_gmem_prezap(f, start, end, attr_filter);
+               if (r)
+                       return r;
+       }
+       return 0;
+}
+#else
+static int kvm_gmem_prezap(struct inode *inode, pgoff_t start, pgoff_t end)
+{
+       return 0;
+}
+#endif
+
 static void __kvm_gmem_zap(struct gmem_file *f, pgoff_t start, pgoff_t end,
                           enum kvm_gfn_range_filter attr_filter)
 {
@@ -351,6 +391,7 @@ static long kvm_gmem_punch_hole(struct inode *inode, loff_t offset, loff_t len)
        pgoff_t start = offset >> PAGE_SHIFT;
        pgoff_t end = (offset + len) >> PAGE_SHIFT;
        struct gmem_inode *gi = GMEM_I(inode);
+       int r;

        /*
         * gi->page_order is 0 by default and is set to PMD order only when the
@@ -368,14 +409,19 @@ static long kvm_gmem_punch_hole(struct inode *inode, loff_t offset, loff_t len)

        kvm_gmem_invalidate_start(inode, start, end);

+       r = kvm_gmem_prezap(inode, start, end);
+       if (!r)
+               goto out;
+
        kvm_gmem_zap(inode, start, end);
        truncate_inode_pages_range(inode->i_mapping, offset, offset + len - 1);

+out:
        kvm_gmem_invalidate_end(inode, start, end);

        filemap_invalidate_unlock(inode->i_mapping);

-       return 0;
+       return r;
 }

 static long kvm_gmem_allocate(struct inode *inode, loff_t offset, loff_t len)
@@ -802,6 +848,13 @@ static int __kvm_gmem_set_attributes(struct inode *inode, pgoff_t start,

        kvm_gmem_invalidate_start(inode, start, end);

+       r = kvm_gmem_prezap(inode, start, end);
+       if (r) {
+               mas_destroy(&mas);
+               kvm_gmem_invalidate_end(inode, start, end);
+               goto out;
+       }
+
        kvm_gmem_zap(inode, start, end);
        if (!to_private)
                kvm_gmem_invalidate(inode, start, end);