Re: [PATCH 17/17] KVM: TDX: Move external page table freeing to TDX code

[Edgecombe, Rick P]

On Mon, 2026-03-30 at 15:49 +0800, Yan Zhao wrote:
> > diff --git a/arch/x86/kvm/vmx/tdx.c b/arch/x86/kvm/vmx/tdx.c
> > index d064b40a6b31..1346e891ca94 100644
> > --- a/arch/x86/kvm/vmx/tdx.c
> > +++ b/arch/x86/kvm/vmx/tdx.c
> > @@ -1782,7 +1782,16 @@ static void tdx_sept_free_private_spt(struct kvm
> > *kvm, gfn_t gfn,
> >   	 */
> >   	if (KVM_BUG_ON(is_hkid_assigned(to_kvm_tdx(kvm)), kvm) ||
> >   	    tdx_reclaim_page(virt_to_page(sp->external_spt)))
> > -		sp->external_spt = NULL;
> > +		goto out;
> > +
> > +	/*
> > +	 * Immediately free the S-EPT page as the TDX subsystem doesn't
> > support
> > +	 * freeing pages from RCU callbacks, and more importantly because
> > +	 * TDH.PHYMEM.PAGE.RECLAIM ensures there are no outstanding
> > readers.
> > +	 */
> > +	free_page((unsigned long)sp->external_spt);
> Since sp->external_spt is allocated in kvm_mmu_alloc_external_spt() in a
> similar
> way as sp->spt in tdp_mmu_alloc_sp(), why bother making freeing of
> sp->external_spt special? i.e., what's the downside of freeing it together
> with
> sp->spt in tdp_mmu_free_sp() ?

I agree it's questionable as a cleanup. The change originally came from your
comment here actually:
https://lore.kernel.org/kvm/aYW5CbUvZrLogsWF@xxxxxxxxxxxxxxxxxxxxxxxxx/

So for a more concrete DPAMT reason. That said I agree the reasoning as a stand
alone cleanup is pretty weak. I tried to put a brave face on it.

We have to think if we should try to include it in the initial set or include it
with the later DPAMT parts. I thought to try at least to do it earlier and leave
it at the end. It also kind of helps to see more of the overhaul together.
Thoughts?