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

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

On Thu, 2026-07-09 at 10:50 +0800, Yan Zhao wrote:
> > > > 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
> > > What "this" stands for is not clear and a comma is missing after
> > > "happens".
> > 
> > Really this is not clear? The previous sentence is all about a senario being
> > possible "multiple callers can concurrently operate on 4KB pages that fall
> > within the same 2MB region". So I thought this would be clear.
> Hmm, sorry for nitpicking.
> But the previous sentence says "multiple callers can concurrently operate on
> 4KB pages", where "operate" doesn't necessarily imply allocating pages.
> However, the latter sentence assumes PAMT pages need to be installed. That's
> why I think "it" is not clear.

Not sure what you mean by "it", but I guess you are saying that "this" could
refer to the first sentence and not the one that immediately preceded the "this"
reference? That part still reads ok to me, but if you still feel it's unclear
I'll change it to.

Actually, another problem with this paragraph is focuses too much on concurrent
callers, when the main thing this patch does is address get-ing multiple pages.
The concurrency is added to make the refcounts work. I'd like to address that
too. How is this to you?

x86/virt/tdx: Handle multiple callers in tdx_pamt_get/put()

tdx_pamt_get()/tdx_pamt_put() unconditionally add or remove Dynamic PAMT 
backing for the 2MB region covering the passed page. However, multiple 
callers can add or remove 4KB pages that fall within the same 2MB region
and in that scenario only a single PAMT entry is required.

Make the helpers handle only adding/removing Dynamic PAMT backing when 
required, by refcounting each 2MB range. 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 the Dynamic PAMT add
and remove SEAMCALLs take internal TDX module locks for the 2MB ranges of
the specified PFN and the PAMT page pair PFNs. So simultaneous attempts on
the same 2MB ranges of the PFNs would otherwise encounter and error, which
would not be handleable in the put case.

The lock is global and heavyweight. Use simple conditional logic to keep
correctness obvious. This will be optimized in a later change.

The pamt_refcount[]s are atomic_t's. They do not strictly need to be
because all access is protected by pamt_lock. The overhead of an atomic_t
in this situation is minuscule compared to the global lock. Leave the
atomic_t in place to enable future optimization with minimal churn.