Re: [PATCH v4 02/17] x86/virt/tdx: Add a SEAMCALL wrapper to demote a 2MB huge page

[Yan Zhao]

On Thu, Oct 15, 2026 at 05:28:28PM +0800, Yan Zhao wrote:
> On Thu, Oct 15, 2026 at 02:11:51PM +0800, Yan Zhao wrote:
> > > > u64 tdh_mem_page_demote_2mb(struct tdx_td *td, u64 gpa, kvm_pfn_t pfn, struct page *new_sept_pt,
> > > >                             struct tdx_pamt_cache *pamt_cache, u64 *ext_err1, u64 *ext_err2)
> > > > {
> > > >         struct tdx_module_args args = {
> > > >                 .rcx = gpa | pg_level_to_tdx_sept_level(PG_LEVEL_2M),
> > > >                 .rdx = tdx_tdr_pa(td),
> > > >                 .r8 = page_to_phys(new_sept_pt),
> > > >         };
> > > >         atomic_t *dpamt_refcount = tdx_find_dpamt_refcount(pfn);
> Oh, when DPAMT is not enabled, this line makes it erroneous to access the
> uninitialized dpamt_refcounts.
> 
> > > >         struct page *dpamt_pages[TDX_DPAMT_ENTRY_PAGE_CNT];
> > > >         *ext_err1 = 0;
> > > >         *ext_err2 = 0;
> > > >         u64 ret;
> > > > 
> > > >         if (!tdx_supports_dynamic_pamt(&tdx_sysinfo))
> > > >                 return TDX_SW_ERROR;
> > > 
> > > Do we need to handle this logically? The below will return some error in any
> So, we need this check and should move tdx_find_dpamt_refcount() after it.
> 
> Would the below version look good to you?
> 
> Changes vs the version in
> https://lore.kernel.org/all/as9OHnM4mceEre%2F2@xxxxxxxxxxxxxxxxxxxxxxxxx:
> - Added WARN_ON_ONCE() for tdx_supports_dynamic_pamt() and moved
>   tdx_find_dpamt_refcount() after checking DPAMT is enabled.
> - Added WARN_ON_ONCE() for DEMOTE SEAMCALL failure.
> - atomic_cmpxchg_release() to atomic_set() and added
>   WARN_ON_ONCE(atomic_read(dpamt_refcount)).
> 
> (I didn't add the warning for alloc_pamt_array(). Will add it if you think the
> justification in
> https://lore.kernel.org/all/atBupzmYqvnB0Qcu@xxxxxxxxxxxxxxxxxxxxxxxxx makes sense.
> 
> 
> u64 tdh_mem_page_demote_2mb(struct tdx_td *td, u64 gpa, kvm_pfn_t pfn, struct page *new_sept_pt,
>                             struct tdx_pamt_cache *pamt_cache, u64 *ext_err1, u64 *ext_err2)
> {
>         struct tdx_module_args args = {
>                 .rcx = gpa | pg_level_to_tdx_sept_level(PG_LEVEL_2M),
>                 .rdx = tdx_tdr_pa(td),
>                 .r8 = page_to_phys(new_sept_pt),
>         };
>         struct page *dpamt_pages[TDX_DPAMT_ENTRY_PAGE_CNT];
>         atomic_t *dpamt_refcount;
>         u64 ret;
> 
>         *ext_err1 = 0;
>         *ext_err2 = 0;
> 
>         if (WARN_ON_ONCE(!tdx_supports_dynamic_pamt(&tdx_sysinfo)))
>                 return TDX_SW_ERROR;
> 
>         dpamt_refcount = tdx_find_dpamt_refcount(pfn);
Can move this line after alloc_pamt_array() to mirror the sequence in
tdx_pamt_get().

>         ret = alloc_pamt_array(dpamt_pages, pamt_cache);
>         if (ret)
>                 return TDX_SW_ERROR;
		  return ret;
> 
>         args.r12 = page_to_phys(dpamt_pages[0]),
>         args.r13 = page_to_phys(dpamt_pages[1]),
> 
>         /*
>          * The dpamt_lock is used to avoid contention between the
>          * TDH_MEM_PAGE_DEMOTE and TDH_PHYMEM_PAMT_{ADD/REMOVE} SEAMCALLs.
>          */
>         spin_lock(&dpamt_lock);
> 
>         WARN_ON_ONCE(atomic_read(dpamt_refcount));
>
>         ret = seamcall_saved_ret(TDH_MEM_PAGE_DEMOTE, &args);
> 
>         *ext_err1 = args.rcx;
>         *ext_err2 = args.rdx;
> 
>         if (WARN_ON_ONCE(ret != TDX_SUCCESS))
>                 goto out_free;
> 
>         atomic_set(dpamt_refcount, PTRS_PER_PMD);
>         spin_unlock(&dpamt_lock);
> 
>         return TDX_SUCCESS;
> 
> out_free:
>         spin_unlock(&dpamt_lock);
>         reinsert_pamt_array(pamt_cache, dpamt_pages);
> 
>         return ret;
> }
> EXPORT_SYMBOL_FOR_KVM(tdh_mem_page_demote_2mb);
>