Thread (121 messages) flat view 121 messages, 10 authors, 3h ago

Re: [PATCH v10 12/41] KVM: guest_memfd: Call arch make_shared callback for to-shared conversion

From: Ackerley Tng <hidden>
Date: 2026-08-17 21:10:45
Also in: kvm, linux-coco, linux-doc, linux-kselftest, linux-mm, lkml

Sean Christopherson [off-list ref] writes:
quoted hunk ↗ jump to hunk
On Sun, Aug 16, 2026, Ackerley Tng wrote:
quoted
Sean Christopherson [off-list ref] writes:
quoted
On Thu, Aug 13, 2026, Ackerley Tng wrote:
quoted
Sean Christopherson [off-list ref] writes:
quoted
That's why I think it's worth analyzing the cost: if it's in the
noise, leave it alone.  If it's meaningful, figure out a not-too-gross way to skip
the entire thing if kvm_arch_gmem_make_shared() is a glorified nop in the end.
Is noise defined relative to the entire conversion process? Would this
benchmark look like

1. Convert 4G to shared on TDX with CONFIG_AMD_SEV defined
2. Convert 4G to shared on TDX without CONFIG_AMD_SEV defined

and then compare the difference in time taken?
That'd work, though I was envisioning something even simpler: use rdtsc() to
count the cycles it takes to iterate over various ranges of memory.  Do whatever
is easiest for you though.
I made some changes to add rdtsc() for the conversion process as Sean
suggested [1], and exercised conversion like this [2]:

1. Initialize some memory as private
2. Get the guest to fault them into Secure EPTs
3. Converts the memory to shared <<== this is being benchmarked
4. Converts memory back to private

I made it build the VM once and convert 5 times:

./gmem_benchmark_tdx_convert --iterations=5 --size=1g
...
quoted
And here's the above, tabulated:

  nr_pages      make_shared            total    percentage
----------  ---------------  ---------------  ------------
         1              930            39278       2.3677%
         1              252            28060       0.8981%
         1              176            26952       0.6530%
         1              176            27038       0.6509%
         1              176            26980       0.6523%
         1             1072            37236       2.8789%
         1              316            28338       1.1151%
         1              176            27182       0.6475%
         1              176            26972       0.6525%
         1              176            26886       0.6546%
    262144         15041018       6616067680       0.2273%
    262144         14937462       6608542680       0.2260%
    262144         15138858       6599494898       0.2294%
    262144         15721972       6610219850       0.2378%
    262144         15000406       6615114540       0.2268%
   1048576         61902982      26400884028       0.2345%
   1048576         61746114      26401170984       0.2339%
   1048576         61096794      26404409058       0.2314%
   1048576         61446290      26447461896       0.2323%
   1048576         61774646      26444608360       0.2336%

Looks to me it is within noise.

I also actually tried measuring the conversion time from userspace with
CONFIG_AMD_SEV enabled and disabled. Converting a 1G-sized TD was faster
by 0.2%, which is in line with the above table. Interestingly, when
converting a 4G-sized TD, skipping kvm_gmem_make_shared() was _slower_
over 2 runs. I don't have an explanation for that.
Might be some cache/memory locality benefits?  Though with a conversion that big,
it could also be nothing more than bad luck.
quoted
I think the code was correct. (If it makes a difference, I skipped
kvm_gmem_make_shared() using a custom guest_memfd creation time flag and
skipped make_shared if the flag was set on the inode.)

I thought adding a kvm_arch_has_gmem_make_shared(), defaulting it to
I would do kvm_arch_has_gmem_convert() for consistency with the Kconfigs, and
because the cost of the reclaim invocation is a non-issue.
quoted
false for all archs and having x86 override with
!!kvm_x86_ops.gmem_make_shared is not too bad either:
+ doesn't leak anything, since the function being called is
  kvm_arch_gmem_make_shared and the accompanying function is
  kvm_arch_has_gmem_make_shared. Or maybe just a little, since all the
  other ops don't have the accompanying _has_ function
+ it's a kernel-internal thing
+ not too many lines of code, not too complex
It also provides a good excuse to kill off the #idfefs in guest_memfd.c.
Compile tested only, but I'm thinking this?

From: Sean Christopherson <seanjc@google.com>
Date: Mon, 17 Aug 2026 12:31:50 -0700
Subject: [PATCH] KVM: guest_memfd: Optimize away conversion overheads via
 dead-code elimination

Add and use kvm_arch_has_gmem_convert() to guard guest_memfd's invocation
of arch hooks related to converting memory between private and shared, as
only one half of the x86 CoCo duo needs the runtime hooks (any pre-work is
pure overhead for TDX).  At this exact moment, the overhead is negligible,
but that will change when in-place conversion comes along, at which point
to-shared conversions will "need" to find all affected folios prior to
calling into arch code.  In quotes because very technically that work could
be pushed to arch code, but that would bleed guest_memfd details into arch
code and would be far worse than adding yet another kvm_arch_has... hook.

Opportunistically provide the kvm_arch_gmem_make_private() declaration, and
rely on dead-code elimination to eliminate the call to non-existent code
when CONFIG_HAVE_KVM_ARCH_GMEM_CONVERT=n.

Reported-by: Binbin Wu <redacted>
Closes: https://lore.kernel.org/all/1ec08cd8-3072-4753-ad5e-cd34956647f8@linux.intel.com (local)
Suggested-by: Ackerley Tng <redacted>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/include/asm/kvm_host.h | 3 +++
 include/linux/kvm_host.h        | 3 ++-
 virt/kvm/guest_memfd.c          | 5 ++---
 3 files changed, 7 insertions(+), 4 deletions(-)
diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 283847619ff8..5d5a7723abb6 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -1854,6 +1854,9 @@ enum kvm_intr_type {
 #ifdef CONFIG_KVM_GENERIC_MEMORY_ATTRIBUTES
 #define kvm_arch_has_private_mem(kvm) ((kvm)->arch.has_private_mem)
 #endif
+#ifdef CONFIG_HAVE_KVM_ARCH_GMEM_CONVERT
+#define kvm_arch_has_gmem_convert() (!!kvm_x86_ops.gmem_make_private)
+#endif

 #define kvm_arch_has_readonly_mem(kvm) (!(kvm)->arch.has_protected_state)
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 03bfc92864b6..e824ba59c60c 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -2599,9 +2599,10 @@ static inline int kvm_gmem_get_pfn(struct kvm *kvm,
 }
 #endif /* CONFIG_KVM_GUEST_MEMFD */

-#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);
+#ifndef CONFIG_HAVE_KVM_ARCH_GMEM_CONVERT
+#define kvm_arch_has_gmem_convert() false
 #endif

 #ifdef CONFIG_HAVE_KVM_ARCH_GMEM_POPULATE
diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c
index b596486d184c..39d94938b5f6 100644
--- a/virt/kvm/guest_memfd.c
+++ b/virt/kvm/guest_memfd.c
@@ -773,11 +773,10 @@ int kvm_gmem_get_pfn(struct kvm *kvm, struct kvm_memory_slot *slot,
 		folio_mark_uptodate(folio);
 	}

-#ifdef CONFIG_HAVE_KVM_ARCH_GMEM_CONVERT
-	if (kvm_gmem_is_private_mem(file_inode(file), index))
+	if (kvm_arch_has_gmem_convert() &&
+	    kvm_gmem_is_private_mem(file_inode(file), index))
 		r = kvm_arch_gmem_make_private(kvm, gfn, *pfn,
 					       (kvm_pfn_t)1 << *max_order);
-#endif

 	folio_unlock(folio);

base-commit: 1b731e5ded480bd1e5546aed35584238661ce72e
--
I'll inserted this as the first patch for v11.

Then, the later patch "Call arch make_shared callback for to-shared
conversion" is now:

	if (!to_private && kvm_arch_has_gmem_convert())
		kvm_gmem_make_shared(inode, start, end);

kvm_gmem_make_shared()'s definition is still guarded by #ifdef
CONFIG_HAVE_KVM_ARCH_GMEM_CONVERT, and there's still a stub for
kvm_gmem_make_shared() since kvm_gmem_make_shared() is defined and used
in the same file and the optimizing out only happens later.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help