[RFC PATCH v2 0/1] KVM: x86/mmu: Zap empty TDP leaf page tables

[RFC PATCH v2 0/1] KVM: x86/mmu: Zap empty TDP leaf page tables

[Hao Zhang]

From: Hao Zhang <zhanghao1@kylinos.cn>

Hi,

v1 tried to avoid repeated walks of retained empty TDP leaf page
tables by adding a per-shadow-page hint.  The main v1 feedback was to
avoid adding more metadata to struct kvm_mmu_page and to look for a more
generic iterator-based approach.

Previous RFC:
https://lore.kernel.org/all/an7J9Gh2DTrtP5vT@192.168.1.215/

This RFC v2 is a metadata-free alternative.  It adds an opt-in TDP
iterator event that tells the caller when the walk has completed a
child page table.  tdp_mmu_zap_leafs() uses that event to unlink an
empty 4K leaf page table after a fully covered 2MiB range has been
walked.

The tradeoff is intentional:

  * v1/hint keeps the page-table page linked and remembers that it is
    empty across later invalidations.
  * v2/unlink avoids persistent per-MMU-page metadata by making the
    parent SPTE non-present, so later invalidations naturally stop
    descending into that empty 4K leaf page table.

That also means v2 can pay allocation/free/refault cost if the same
leaf page table is repeatedly emptied and repopulated.  To limit that
cost, v2 does not unlink a leaf page table if a leaf SPTE in that page
table was zapped in the current pass.

When a shadow page is unlinked, tdp_mmu_zap_leafs() also services
pending TLB invalidations with kvm_flush_remote_tlbs() before dropping
RCU protection.  The incoming @flush state can reflect earlier zaps in
the walk, not only the just-completed child range, so this RFC uses the
existing full flush primitive instead of trying to narrow the flush to
the child range.

The patch also avoids changing TDX/mirror external page-table lifetime:
mirror shadow pages are not unlinked by this optimization.  Please
confirm whether that is the preferred boundary, or whether unlinking
mirror leaf page tables is safe after the TDX maintainers audit the
external SPT lifetime.

The main RFC question is whether this metadata-free unlink tradeoff is
acceptable, or whether KVM should keep retaining empty 4K TDP leaf page
tables unless there is an explicit per-page-table hint.

Test setup
==========

All numbers below are from the same x86 host and the same 4-vCPU, 1GiB
guest workload.  Guest vCPUs continuously fault/write guest memory while
the host repeatedly invalidates guest memory with MADV_DONTNEED.  The
reported values are medians from five repetitions.

"sp-only" is the shadow-present-only iterator prototype that skips
non-present shadow-present SPTEs without unlinking empty page tables.

Measured paths:

  * tdp_mmu_zap_leafs() duration via paired perf probes.
  * vCPU fault-side mmu_lock wait time via queued_read_lock_slowpath()
    perf probes.

Scenarios:

  * same16/same4: repeatedly invalidate the same 16MiB or 4MiB range.
  * c16/c4/c2/c1: rolling 16MiB, 4MiB, 2MiB, and 1MiB chunks across
    the 1GiB guest.

zap_total_ms_med
================

  scenario   original   sp-only   v1 hint   v2 unlink
  --------   --------   -------   -------   ---------
  same16       330.3     221.8      35.3       61.9
  same4        341.3     234.2      54.4       52.4
  c16         1456.2    1696.0    1386.1     1549.9
  c4          1252.0    1290.6    1243.8     1575.7
  c2          1197.0    1173.9    1246.2     1501.3
  c1          1352.8    1198.9    1138.4     1304.2

Relative to the original kernel, v2 reduces repeated same-range zap
time by 81% for same16 and 85% for same4.  Rolling workloads are mixed:
c16 is +6%, c4 is +26%, c2 is +25%, and c1 is -4%.
Compared to the shadow-present-only iterator change, v2 is mixed on
rolling workloads: better on c16, worse on c4/c2/c1.  Same-range zap
time remains 3.6x faster for same16 and 4.5x faster for same4.

fwait_p999_us_med
=================

  scenario   original   sp-only   v1 hint   v2 unlink
  --------   --------   -------   -------   ---------
  same16          78       162        61          74
  same4           62       152        38          57
  c16             72       242        77         114
  c4             102       248       105         134
  c2             108       270       107         148
  c1              92       266        93          95

An additional no-unlink control on top of v2's iterator changes brought
same16/same4 back to the original kernel's zap time, while also bringing
rolling fwait back close to original/v1 levels.  That confirms the
same-range win and the rolling cost both come from unlinking, not from
the iterator event itself.

Changes since v1
================

  * Replace the per-kvm_mmu_page empty-child hint with a metadata-free
    unlink-based approach.
  * Add an opt-in TDP iterator event for callers that want notification
    after completing a child page table.
  * Do not unlink leaf page tables that had a leaf SPTE zapped in the
    current pass, to reduce refault churn in rolling invalidations.
  * Skip mirror shadow pages, so this optimization does not alter TDX
    external SPT lifetime.
  * Drop the unused skip-child iterator API from the v1/hint direction.
  * Move the event-valid fast gate to the caller.

Hao Zhang (1):
  KVM: x86/mmu: Zap empty TDP leaf page tables

 arch/x86/kvm/mmu/tdp_iter.c | 29 +++++++++++++--
 arch/x86/kvm/mmu/tdp_iter.h | 10 ++++++
 arch/x86/kvm/mmu/tdp_mmu.c  | 88 +++++++++++++++++++++++++++++++++++++--------
 3 files changed, 109 insertions(+), 18 deletions(-)

base-commit: 45c13f3f9e3bb15fd89ff2864c6f627a3b4b4229
-- 
2.15.0

[RFC PATCH v2 1/1] KVM: x86/mmu: Zap empty TDP leaf page tables

[Hao Zhang]

From: Hao Zhang <zhanghao1@kylinos.cn>

KVM's TDP MMU intentionally zaps only leaf SPTEs for some invalidations,
leaving the page table hierarchy intact so that subsequent faults can reuse
the existing shadow pages.

That behavior can be pathological if a later invalidation covers a range
whose retained 4K leaf tables contain only non-present SPTEs.  KVM still
descends into every retained leaf page table and scans all 512 entries even
though there is nothing left to zap.

Add an opt-in TDP iterator event that reports when the walk has finished a
child page table, and use that event in tdp_mmu_zap_leafs() to unlink empty
4K leaf page tables that are fully covered by the zap.

Do not unlink a leaf page table that had a leaf SPTE zapped in the current
pass.  Doing so would add allocation/free churn for rolling invalidation
workloads that refault the same page table.

Because unlinking a shadow page changes the old leaf-only RCU assumption,
flush any pending remote TLBs before dropping RCU protection if a shadow
page was removed.

Signed-off-by: Hao Zhang <zhanghao1@kylinos.cn>
---
 arch/x86/kvm/mmu/tdp_iter.c | 29 +++++++++++++--
 arch/x86/kvm/mmu/tdp_iter.h | 10 ++++++
 arch/x86/kvm/mmu/tdp_mmu.c  | 88 +++++++++++++++++++++++++++++++++++++--------
 3 files changed, 109 insertions(+), 18 deletions(-)

diff --git a/arch/x86/kvm/mmu/tdp_iter.c b/arch/x86/kvm/mmu/tdp_iter.c
index 9e17bfa80901..d9be4298410d 100644
--- a/arch/x86/kvm/mmu/tdp_iter.c
+++ b/arch/x86/kvm/mmu/tdp_iter.c
@@ -132,7 +132,8 @@ static bool try_step_side(struct tdp_iter *iter)
  * can continue from the next entry in the parent page table. Returns true on a
  * successful step up, false if already in the root page.
  */
-static bool try_step_up(struct tdp_iter *iter)
+static bool try_step_up(struct tdp_iter *iter,
+			struct tdp_iter_child_event *event)
 {
 	if (iter->level == iter->root_level)
 		return false;
@@ -141,6 +142,14 @@ static bool try_step_up(struct tdp_iter *iter)
 	iter->gfn = gfn_round_for_level(iter->gfn, iter->level);
 	tdp_iter_refresh_sptep(iter);
 
+	if (event && !event->valid) {
+		event->sptep = iter->sptep;
+		event->old_spte = iter->old_spte;
+		event->gfn = iter->gfn;
+		event->level = iter->level;
+		event->valid = true;
+	}
+
 	return true;
 }
 
@@ -160,8 +169,12 @@ static bool try_step_up(struct tdp_iter *iter)
  *    SPTE will have already been visited, and so the iterator must also step
  *    to the side again.
  */
-void tdp_iter_next(struct tdp_iter *iter)
+static void __tdp_iter_next(struct tdp_iter *iter,
+			    struct tdp_iter_child_event *event)
 {
+	if (event)
+		event->valid = false;
+
 	if (iter->yielded) {
 		tdp_iter_restart(iter);
 		return;
@@ -173,7 +186,17 @@ void tdp_iter_next(struct tdp_iter *iter)
 	do {
 		if (try_step_side(iter))
 			return;
-	} while (try_step_up(iter));
+	} while (try_step_up(iter, event));
 	iter->valid = false;
 }
 
+void tdp_iter_next(struct tdp_iter *iter)
+{
+	__tdp_iter_next(iter, NULL);
+}
+
+void tdp_iter_next_post_order(struct tdp_iter *iter,
+			      struct tdp_iter_child_event *event)
+{
+	__tdp_iter_next(iter, event);
+}
diff --git a/arch/x86/kvm/mmu/tdp_iter.h b/arch/x86/kvm/mmu/tdp_iter.h
index f898d8d0d93c..b7e2605fbf9e 100644
--- a/arch/x86/kvm/mmu/tdp_iter.h
+++ b/arch/x86/kvm/mmu/tdp_iter.h
@@ -123,6 +123,14 @@ struct tdp_iter {
 	bool yielded;
 };
 
+struct tdp_iter_child_event {
+	tdp_ptep_t sptep;
+	u64 old_spte;
+	gfn_t gfn;
+	int level;
+	bool valid;
+};
+
 /*
  * Iterates over every SPTE mapping the GFN range [start, end) in a
  * preorder traversal.
@@ -145,6 +153,8 @@ tdp_ptep_t spte_to_child_pt(u64 pte, int level);
 void tdp_iter_start(struct tdp_iter *iter, struct kvm_mmu_page *root,
 		    int min_level, gfn_t next_last_level_gfn, gfn_t gfn_bits);
 void tdp_iter_next(struct tdp_iter *iter);
+void tdp_iter_next_post_order(struct tdp_iter *iter,
+			      struct tdp_iter_child_event *event);
 void tdp_iter_restart(struct tdp_iter *iter);
 
 #endif /* __KVM_X86_MMU_TDP_ITER_H */
diff --git a/arch/x86/kvm/mmu/tdp_mmu.c b/arch/x86/kvm/mmu/tdp_mmu.c
index 44dad106fad1..82446c4737f7 100644
--- a/arch/x86/kvm/mmu/tdp_mmu.c
+++ b/arch/x86/kvm/mmu/tdp_mmu.c
@@ -923,6 +923,38 @@ bool kvm_tdp_mmu_zap_possible_nx_huge_page(struct kvm *kvm,
 	return true;
 }
 
+static bool tdp_mmu_zap_completed_empty_leaf_pt(struct kvm *kvm,
+						struct tdp_iter_child_event *event,
+						gfn_t start, gfn_t end,
+						tdp_ptep_t zapped_leaf_pt)
+{
+	struct kvm_mmu_page *child_sp;
+
+	if (event->level != PG_LEVEL_2M ||
+	    !is_shadow_present_pte(event->old_spte) ||
+	    is_last_spte(event->old_spte, event->level))
+		return false;
+
+	if (event->gfn < start ||
+	    event->gfn + KVM_PAGES_PER_HPAGE(event->level) > end)
+		return false;
+
+	/* Leave productive leaf tables in place to avoid refault churn. */
+	if (zapped_leaf_pt == event->sptep)
+		return false;
+
+	child_sp = spte_to_child_sp(event->old_spte);
+	if (is_mirror_sp(child_sp))
+		return false;
+
+	if (child_sp->role.level != PG_LEVEL_4K)
+		return false;
+
+	tdp_mmu_set_spte(kvm, event->sptep, event->old_spte,
+			 SHADOW_NONPRESENT_VALUE, event->gfn, event->level);
+	return true;
+}
+
 /*
  * If can_yield is true, will release the MMU lock and reschedule if the
  * scheduler needs the CPU or there is contention on the MMU lock. If this
@@ -933,7 +965,10 @@ bool kvm_tdp_mmu_zap_possible_nx_huge_page(struct kvm *kvm,
 static bool tdp_mmu_zap_leafs(struct kvm *kvm, struct kvm_mmu_page *root,
 			      gfn_t start, gfn_t end, bool can_yield, bool flush)
 {
+	struct tdp_iter_child_event event = {};
 	struct tdp_iter iter;
+	tdp_ptep_t zapped_leaf_pt = NULL;
+	bool removed_sp = false;
 
 	end = min(end, tdp_mmu_max_gfn_exclusive());
 
@@ -941,33 +976,56 @@ static bool tdp_mmu_zap_leafs(struct kvm *kvm, struct kvm_mmu_page *root,
 
 	rcu_read_lock();
 
-	for_each_tdp_pte_min_level(iter, kvm, root, PG_LEVEL_4K, start, end) {
+	tdp_iter_start(&iter, root, PG_LEVEL_4K, start,
+		       kvm_gfn_root_bits(kvm, root));
+	while (iter.valid && iter.gfn < end) {
 		if (can_yield &&
 		    tdp_mmu_iter_cond_resched(kvm, &iter, flush, false)) {
 			flush = false;
+			tdp_iter_next(&iter);
 			continue;
 		}
 
-		if (!is_shadow_present_pte(iter.old_spte) ||
-		    !is_last_spte(iter.old_spte, iter.level))
-			continue;
+		if (is_shadow_present_pte(iter.old_spte) &&
+		    is_last_spte(iter.old_spte, iter.level)) {
+			if (iter.level == PG_LEVEL_4K) {
+				struct kvm_mmu_page *sp;
 
-		tdp_mmu_iter_set_spte(kvm, &iter, SHADOW_NONPRESENT_VALUE);
+				sp = sptep_to_sp(rcu_dereference(iter.sptep));
+				zapped_leaf_pt = sp->ptep;
+			}
 
-		/*
-		 * Zappings SPTEs in invalid roots doesn't require a TLB flush,
-		 * see kvm_tdp_mmu_zap_invalidated_roots() for details.
-		 */
-		if (!root->role.invalid)
-			flush = true;
-	}
+			tdp_mmu_iter_set_spte(kvm, &iter,
+					      SHADOW_NONPRESENT_VALUE);
 
-	rcu_read_unlock();
+			/*
+			 * Zappings SPTEs in invalid roots doesn't require a TLB flush,
+			 * see kvm_tdp_mmu_zap_invalidated_roots() for details.
+			 */
+			if (!root->role.invalid)
+				flush = true;
+		}
+
+		tdp_iter_next_post_order(&iter, &event);
+		if (event.valid &&
+		    tdp_mmu_zap_completed_empty_leaf_pt(kvm, &event, start,
+							end, zapped_leaf_pt)) {
+			if (!root->role.invalid)
+				flush = true;
+			removed_sp = true;
+		}
+	}
 
 	/*
-	 * Because this flow zaps _only_ leaf SPTEs, the caller doesn't need
-	 * to provide RCU protection as no 'struct kvm_mmu_page' will be freed.
+	 * If any shadow pages were removed, service pending TLB flushes before
+	 * dropping RCU protection, as required by the TDP MMU iterator.
 	 */
+	if (removed_sp && flush) {
+		kvm_flush_remote_tlbs(kvm);
+		flush = false;
+	}
+
+	rcu_read_unlock();
 	return flush;
 }
 
-- 
2.15.0