Re: [PATCH v4 08/17] KVM: TDX: Adjust the topup count of DPAMT page pairs for splitting S-EPT

[Yan Zhao]

On Fri, Oct 23, 2026 at 07:59:53AM +0800, Yan Zhao wrote:
> On Thu, Oct 22, 2026 at 11:56:47PM +0800, Edgecombe, Rick P wrote:
> > On Thu, 2026-10-22 at 15:24 +0800, Yan Zhao wrote:
> > > static int tdx_topup_external_pamt_cache(struct kvm *kvm, struct kvm_vcpu
> > > *vcpu,
> > >                                          bool with_root_spt, int min_nr_spts)
> > > {
> > >         struct tdx_pamt_cache *pamt_cache;
> > >         int dpamt_pairs = min_nr_spts;
> > > 
> > >         pamt_cache = tdx_get_pamt_cache(kvm, vcpu);
> > >         if (!pamt_cache)
	      if (KVM_BUG_ON(!pamt_cache, kvm))

If no objection, I'll add KVM_BUG_ON() here to avoid unnecessary repeated retries
of topup() after such an error occurs, as it would indicate a KVM bug.

> > >                 return -EIO;
> > > 
> > >         /* DPAMT page pair for the root SPT has already been installed */
> > >         if (with_root_spt)
> > >                 dpamt_pairs -= 1;
> > > 
> > >         /* +1 pair for 4KB guest private page */
> > >         dpamt_pairs += 1;
> > > 
> > >         return tdx_topup_pamt_cache(pamt_cache, dpamt_pairs);
> > > }
> > 
> > Yes this looks much clearer than even the upstream code. Do you have any better
> > ideas than "4KB guest private page" though? Because in the split case it is
> > actually for 512 guest private pages. We should put an artful comment to explain
> > both cases while not making it look like something that has a better solution.
> > Because I'm thinking there isn't one.
> 
> What about a comment like
> 	/*
>          * +1 pair to account for mapping a new 4KB guest page or splitting a
>          * 2MB guest page into 512 4KB pages.
>          */
> ?
> 
> > > In the fault path, mmu_topup_memory_caches() calls:
> > > r = kvm_x86_call(topup_external_cache)(vcpu->kvm, vcpu, true,
> > > PT64_ROOT_MAX_LEVEL);
> > > 
> > > In the split path, tdp_mmu_alloc_sp_for_split() calls:
> > > kvm_x86_call(topup_external_cache)(kvm, kvm_get_running_vcpu(), false, 1);
> >