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

["Edgecombe, Rick P" <rick.p.edgecombe@intel.com>]

On Mon, 2026-09-28 at 17:10 +0800, Yan Zhao wrote:
> KVM needs to allocate enough DPAMT page pairs in the pamt_cache for
> consumption by both page table pages and guest pages. Since the DPAMT page
> pair for the S-EPT root page is already allocated during TD initialization,
> there is no need to allocate the DPAMT page pair for the S-EPT root page.
> Therefore, previously the DPAMT page pairs required equals
> "min_nr_spts - 1 + 1".
> 
> When splitting S-EPT, min_nr_spts does not include the root SPT. So, limit
> the -1 calculation to when min_nr_spts equals root_level, though this will
> cause one pair over-allocation in the normal page fault path because
> PT64_ROOT_MAX_LEVEL is always passed even when launching a 4-level TD.
> 
> Additionally, KVM may need to retry tdh_mem_page_demote() a second time,
> causing the DPAMT page pair for the guest private pages to be drawn from
> the pamt_cache twice in the worst case. Therefore, add an extra +1 to cover
> this worst-case scenario.
> 
> The slight over-allocation is acceptable since KVM already pre-allocates
> more pages than needed (e.g., when mapping huge pages) in case of the
> worst-case scenario.

This is assumes way too much about the callers and way too convoluted. The
existing code already did but, now this is just too far. We need another
solution.

Questions below on what that might be.

> 
> Signed-off-by: Yan Zhao <yan.y.zhao@intel.com>
> ---
>  arch/x86/kvm/vmx/tdx.c | 26 ++++++++++++++++++++++----
>  1 file changed, 22 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/x86/kvm/vmx/tdx.c b/arch/x86/kvm/vmx/tdx.c
> index 3186c4808cae..397a308b834c 100644
> --- a/arch/x86/kvm/vmx/tdx.c
> +++ b/arch/x86/kvm/vmx/tdx.c
> @@ -1630,16 +1630,34 @@ void tdx_load_mmu_pgd(struct kvm_vcpu *vcpu, hpa_t root_hpa, int pgd_level)
>  
>  static int tdx_topup_external_pamt_cache(struct kvm_vcpu *vcpu, int min_nr_spts)
>  {
> +	int dpamt_pairs;
> +
>  	if (WARN_ON_ONCE(!vcpu))
>  		return -EIO;
>  
> +	/* Exclude the root SPT, as its DPAMT page pair is already installed */
> +	if (min_nr_spts == vcpu->kvm->arch.mirror_root_level)
> +		min_nr_spts -= 1;

Over allocating a bit is not the end of the world...

> +
> +	/*
> +	 * Each S-EPT page table page + 4KB guest private page needs a pair of
> +	 * DPAMT pages.
> +	 */
> +	dpamt_pairs = min_nr_spts + 1;
> +
>  	/*
> -	 * Minus one page to exclude the root SPT, but plus one page for a
> -	 * possible 4KB private mapping.
> +	 * After each topup, KVM may invoke DEMOTE at most twice.
> 

Why twice? You mean the BUSY retry attempt, right? If you do, couldn't we fix
this problem within tdh_mem_page_demote()?

>  The first
> +	 * DEMOTE invocation draws two pairs of pages from the cache: one for
> +	 * the S-EPT page table page and one for the guest private memory.
> +	 * Since these pages are not returned to the cache, the second DEMOTE
> +	 * invocation still needs to consume one additional pair for the guest
> +	 * private memory (the pair for the S-EPT page table page is reused for
> +	 * the 2nd invocation).
> 
> 
> 

Another idea, change the op to be:
int topup_external_cache(struct kvm_vcpu *vcpu, bool root, bool private_page,
int min_nr_spts);

Normal topup can set:
root=true
private_page=true
min_nr_spts = PT64_ROOT_MAX_LEVEL - 1

Then we can calculate exactly what we need. And even better, the existing code
won't nee a comment to explain the weirdness.

>  Increase the topup count to account for this
> +	 * worst-case scenario.
>  	 */
> -	min_nr_spts += -1 + 1;
> +	dpamt_pairs += 1;
>  
> -	return tdx_topup_pamt_cache(&to_tdx(vcpu)->pamt_cache, min_nr_spts);
> +	return tdx_topup_pamt_cache(&to_tdx(vcpu)->pamt_cache, dpamt_pairs);
>  }
>  
>  static int tdx_mem_page_add(struct kvm *kvm, gfn_t gfn, enum pg_level level,