Re: [RFC PATCH v2 12/23] KVM: x86/mmu: Introduce kvm_split_cross_boundary_leafs()

[Yan Zhao <yan.y.zhao@intel.com>]

On Tue, Nov 11, 2025 at 06:42:55PM +0800, Huang, Kai wrote:
> On Thu, 2025-08-07 at 17:43 +0800, Yan Zhao wrote:
> >  static int tdp_mmu_split_huge_pages_root(struct kvm *kvm,
> >  					 struct kvm_mmu_page *root,
> >  					 gfn_t start, gfn_t end,
> > -					 int target_level, bool shared)
> > +					 int target_level, bool shared,
> > +					 bool only_cross_bounday, bool *flush)
> >  {
> >  	struct kvm_mmu_page *sp = NULL;
> >  	struct tdp_iter iter;
> > @@ -1589,6 +1596,13 @@ static int tdp_mmu_split_huge_pages_root(struct kvm *kvm,
> >  	 * level into one lower level. For example, if we encounter a 1GB page
> >  	 * we split it into 512 2MB pages.
> >  	 *
> > +	 * When only_cross_bounday is true, just split huge pages above the
> > +	 * target level into one lower level if the huge pages cross the start
> > +	 * or end boundary.
> > +	 *
> > +	 * No need to update @flush for !only_cross_bounday cases, which rely
> > +	 * on the callers to do the TLB flush in the end.
> > +	 *
> 
> s/only_cross_bounday/only_cross_boundary
> 
> From tdp_mmu_split_huge_pages_root()'s perspective, it's quite odd to only
> update 'flush' when 'only_cross_bounday' is true, because
> 'only_cross_bounday' can only results in less splitting.
I have to say it's a reasonable point.

> I understand this is because splitting S-EPT mapping needs flush (at least
> before non-block DEMOTE is implemented?).  Would it better to also let the
Actually the flush is only required for !TDX cases.

For TDX, either the flush has been performed internally within
tdx_sept_split_private_spt() or the flush is not required for future non-block
DEMOTE. So, the flush in KVM core on the mirror root may be skipped as a future
optimization for TDX if necessary.

> caller to decide whether TLB flush is needed?  E.g., we can make
> tdp_mmu_split_huge_pages_root() return whether any split has been done or
> not.  I think this should also work?
Do you mean just skipping the changes in the below "Hunk 1"?

Since tdp_mmu_split_huge_pages_root() originally did not do flush by itself,
which relied on the end callers (i.e.,kvm_mmu_slot_apply_flags(),
kvm_clear_dirty_log_protect(), and kvm_get_dirty_log_protect()) to do the flush
unconditionally, tdp_mmu_split_huge_pages_root() previously did not return
whether any split has been done or not.

So, if we want callers of kvm_split_cross_boundary_leafs() to do flush only
after splitting occurs, we have to return whether flush is required.

Then, in this patch, seems only the changes in "Hunk 1" can be dropped.

Hunk 1
-------------------------------
        for_each_tdp_pte_min_level(iter, kvm, root, target_level + 1, start, end) {
 retry:
-               if (tdp_mmu_iter_cond_resched(kvm, &iter, false, shared))
+               if (tdp_mmu_iter_cond_resched(kvm, &iter, *flush, shared)) {
+                       if (only_cross_bounday)
+                               *flush = false;
                        continue;
+               }

                if (!is_shadow_present_pte(iter.old_spte) || !is_large_pte(iter.old_spte))
                        continue;

Hunk 2 
-------------------------------
+               if (only_cross_bounday &&
+                   !iter_cross_boundary(&iter, start, end))
+                       continue;
+
                if (!sp) {
                        rcu_read_unlock();

Hunk 3 
-------------------------------
@@ -1637,6 +1658,8 @@ static int tdp_mmu_split_huge_pages_root(struct kvm *kvm,
                        goto retry;

                sp = NULL;
+               if (only_cross_bounday)
+                       *flush = true;
        }


Do you think dropping of "Hunk 1" is worthwhile?
Would it be less odd if I make the following change?

diff --git a/arch/x86/kvm/mmu/tdp_mmu.c b/arch/x86/kvm/mmu/tdp_mmu.c
index 9f479832a981..7bc1d1a5f3ce 100644
--- a/arch/x86/kvm/mmu/tdp_mmu.c
+++ b/arch/x86/kvm/mmu/tdp_mmu.c
@@ -1589,6 +1589,7 @@ static int tdp_mmu_split_huge_pages_root(struct kvm *kvm,
 {
        struct kvm_mmu_page *sp = NULL;
        struct tdp_iter iter;
+       bool caller_unconditional_flush = !only_cross_bounday;

        rcu_read_lock();

@@ -1613,7 +1614,7 @@ static int tdp_mmu_split_huge_pages_root(struct kvm *kvm,
        for_each_tdp_pte_min_level(iter, kvm, root, target_level + 1, start, end) {
 retry:
                if (tdp_mmu_iter_cond_resched(kvm, &iter, *flush, shared)) {
-                       if (only_cross_bounday)
+                       if (!caller_unconditional_flush)
                                *flush = false;
                        continue;
                }
@@ -1659,7 +1660,7 @@ static int tdp_mmu_split_huge_pages_root(struct kvm *kvm,
                        goto retry;

                sp = NULL;
-               if (only_cross_bounday)
+               if (!caller_unconditional_flush)
                        *flush = true;
        }