Re: [PATCH v4 05/17] KVM: x86/tdp_mmu: Alloc external_spt page for mirror page table splitting

[Yan Zhao]

On Wed, Oct 07, 2026 at 08:33:27AM +0800, Edgecombe, Rick P wrote:
> On Mon, 2026-09-28 at 17:09 +0800, Yan Zhao wrote:
> > From: Isaku Yamahata <isaku.yamahata@xxxxxxxxx>
> > 
> > Enhance tdp_mmu_alloc_sp_for_split() to allocate a page table page for the
> > external page table in preparation for splitting the mirror page table.
> > 
> > When the mirror page table is split in tdp_mmu_split_huge_page(), the
> > corresponding external page table also needs to be split. Therefore,
> > allocate external_spt in tdp_mmu_alloc_sp_for_split() to prepare for the
> > splitting.
> > 
> > The external_spt will be gifted to the TDX module and mapped as a page
> > table page in the S-EPT during splitting. Since the TDX module will
> > initialize the page content in the DEMOTE SEAMCALL, there is no need to
> > zero external_spt.
> > 
> > Signed-off-by: Isaku Yamahata <isaku.yamahata@xxxxxxxxx>
> > [sean: use __get_free_page(), let is_mirror_root be const and called once]
> > Signed-off-by: Sean Christopherson <seanjc@xxxxxxxxxx>
> > Co-developed-by: Yan Zhao <yan.y.zhao@xxxxxxxxx>
> > Signed-off-by: Yan Zhao <yan.y.zhao@xxxxxxxxx>
> > ---
> > v4:
> > - "bool mirror" --> "bool is_mirror_sp". (Sean)
> > - Use __get_free_page() instead of get_zeroed_page(). (Sean)
> > - let is_mirror_root be const and called once. (Sean)
> > 
> > v3:
> > - Removed unnecessary declaration of tdp_mmu_alloc_sp_for_split(). (Kai)
> > - Fixed a typo in the patch log. (Kai)
> > 
> > RFC v2:
> > - NO change.
> > 
> > RFC v1:
> > - Rebased and simplified the code.
> > ---
> >  arch/x86/kvm/mmu/tdp_mmu.c | 14 ++++++++++++--
> >  1 file changed, 12 insertions(+), 2 deletions(-)
> > 
> > diff --git a/arch/x86/kvm/mmu/tdp_mmu.c b/arch/x86/kvm/mmu/tdp_mmu.c
> > index 001449142d32..f3311317a63a 100644
> > --- a/arch/x86/kvm/mmu/tdp_mmu.c
> > +++ b/arch/x86/kvm/mmu/tdp_mmu.c
> > @@ -1466,7 +1466,7 @@ bool kvm_tdp_mmu_wrprot_slot(struct kvm *kvm,
> >  	return spte_set;
> >  }
> >  
> > -static struct kvm_mmu_page *tdp_mmu_alloc_sp_for_split(void)
> > +static struct kvm_mmu_page *tdp_mmu_alloc_sp_for_split(bool is_mirror_sp)
> >  {
> >  	struct kvm_mmu_page *sp;
> >  
> > @@ -1480,6 +1480,15 @@ static struct kvm_mmu_page *tdp_mmu_alloc_sp_for_split(void)
> >  		return NULL;
> >  	}
> >  
> > +	if (is_mirror_sp) {
> > +		sp->external_spt = (void *)__get_free_page(GFP_KERNEL_ACCOUNT);
> > +		if (!sp->external_spt) {
> > +			free_page((unsigned long)sp->spt);
> > +			kmem_cache_free(mmu_page_header_cache, sp);
> > +			return NULL;
> > +		}
> > +	}
> > +
> >  	return sp;
> >  }
> >  
> > @@ -1527,6 +1536,7 @@ static int tdp_mmu_split_huge_pages_root(struct kvm *kvm,
> >  					 gfn_t start, gfn_t end,
> >  					 int target_level, bool shared)
> >  {
> > +	const bool is_mirror_root = is_mirror_sp(root);
> 
> Nit:
> 
> is_mirror_root and is_mirror_sp(root) almost even read the same. And the local
> var is only every used once even at the end of this series. I wonder about just
> having tdp_mmu_alloc_sp_for_split(is_mirror_sp(root)), or even just passing the
LGTM since is_mirror_sp() is a "static inline" function.

> root in.
 
> Otherwise:
> 
> Reviewed-by: Rick Edgecombe <rick.p.edgecombe@xxxxxxxxx>
> 
> >  	struct kvm_mmu_page *sp = NULL;
> >  	struct tdp_iter iter;
> >  
> > @@ -1559,7 +1569,7 @@ static int tdp_mmu_split_huge_pages_root(struct kvm *kvm,
> >  			else
> >  				write_unlock(&kvm->mmu_lock);
> >  
> > -			sp = tdp_mmu_alloc_sp_for_split();
> > +			sp = tdp_mmu_alloc_sp_for_split(is_mirror_root);
> >  
> >  			if (shared)
> >  				read_lock(&kvm->mmu_lock);
>