Re: [PATCH v6 05/11] x86/virt/tdx: Handle concurrent callers in tdx_pamt_get/put()

[Edgecombe, Rick P]

On Thu, 2026-07-02 at 15:39 +0800, Binbin Wu wrote:
> On 5/26/2026 10:35 AM, Rick Edgecombe wrote:
> > From: "Kirill A. Shutemov" <kirill.shutemov@xxxxxxxxxxxxxxx>
> > 
> > tdx_pamt_get()/tdx_pamt_put() unconditionally add or remove Dynamic PAMT
> > backing for the 2MB region covering the passed pfn. However, multiple
> > callers can concurrently operate on 4KB pages that fall within the same
> > 2MB region. When this happens only one Dynamic PAMT page pair needs to be
> > installed to cover the 2MB range. And when one page is freed, the Dynamic
> > PAMT backing cannot be freed until all pages in the range are no longer in
> > use. Make the helpers handle these races internally.
> > 
> > Use the per-2MB refcounts from previous changes to track how many 4KB
> > pages are in use within each region. Gate the actual Dynamic PAMT add and
> > remove on refcount transitions (0->1 and 1->0). Serialize the refcount
> > check and SEAMCALL with a global spinlock so the read-decide-act sequence
> > is atomic. This also avoids TDX module BUSY errors, as Dynamic PAMT add
> > and remove SEAMCALLs take an internal TDX module locks at 2MB granularity,
> > so simultaneous attempts on the same region would conflict.
> > 
> > The lock is global and heavyweight. Use simple conditional logic to keep
> > correctness obvious. This will be optimized in a later change.
> > 
> > Assisted-by: GitHub Copilot:claude-opus-4-6 Claude:claude-opus-4-7
> > Signed-off-by: Kirill A. Shutemov <kirill.shutemov@xxxxxxxxxxxxxxx>
> > Co-developed-by: Rick Edgecombe <rick.p.edgecombe@xxxxxxxxx>
> > Signed-off-by: Rick Edgecombe <rick.p.edgecombe@xxxxxxxxx>
> 
> Reviewed-by: Binbin Wu <binbin.wu@xxxxxxxxxxxxxxx>
> 
> One nit below.
> 
> [...]
> 
> >   
> > @@ -2057,10 +2061,26 @@ static int tdx_pamt_get(kvm_pfn_t pfn)
> >   	if (ret)
> >   		return ret;
> >   
> > -	tdx_status = tdh_phymem_pamt_add(pfn, pamt_pages);
> > -	if (tdx_status != TDX_SUCCESS) {
> > -		ret = -EIO;
> > -		goto out_free;
> > +	pamt_refcount = tdx_find_pamt_refcount(pfn);
> 
> tdx_find_pamt_refcount() is tagged as __maybe_unused in the previous patch.
> I think the tag should be removed.

Oh yea, that was the intention. Will fix.