AMD General
> -----Original Message-----
> From: Koenig, Christian <[email protected]>
> Sent: Friday, July 3, 2026 1:08 PM
> To: SHANMUGAM, SRINIVASAN <[email protected]>;
> Deucher, Alexander <[email protected]>
> Cc: [email protected]
> Subject: Re: [PATCH v4 1/3] drm/amdgpu: Allow PASID allocator to store fpriv
> owner
>
> On 7/3/26 08:18, Srinivasan Shanmugam wrote:
> > AMDGPU already has a global PASID xarray used by the PASID allocator.
> >
> > Allow amdgpu_pasid_alloc() to optionally store the owning DRM
> > file-private object directly.
> >
> > Initial callers pass NULL and keep the current dummy allocation marker
> > behavior. A later patch in this series passes the DRM file-private
> > object for DRM PASIDs.
> >
> > This prepares for using:
> >
> > PASID -> fpriv -> VM
> >
> > instead of:
> >
> > PASID -> VM
> >
> > Also clear any stored owner from amdgpu_pasid_free_delayed() before
> > waiting for fences, so PASID lookups cannot observe a stale fpriv
> > while the PASID number itself is still pending delayed release.
> >
> > v4: (per Christian)
> > - Add fpriv as an optional parameter to amdgpu_pasid_alloc().
> > - Drop separate amdgpu_pasid_set_fpriv()/clear_fpriv() helpers.
> > - Clear PASID owner from amdgpu_pasid_free_delayed().
> >
> > Cc: Alex Deucher <[email protected]>
> > Suggested-by: Christian König <[email protected]>
> > Signed-off-by: Srinivasan Shanmugam <[email protected]>
> > ---
> > drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c | 82
> > +++++++++++++++++++++++-- drivers/gpu/drm/amd/amdgpu/amdgpu_ids.h |
> > 6 +- drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 2 +-
> > drivers/gpu/drm/amd/amdgpu/mes_v12_1.c | 2 +-
> > 4 files changed, 85 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c
> > index 684f40fce73f..669d0fff8cbc 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c
> > @@ -52,6 +52,7 @@ struct amdgpu_pasid_cb {
> > /**
> > * amdgpu_pasid_alloc - Allocate a PASID
> > * @bits: Maximum width of the PASID in bits, must be at least 1
> > + * @fpriv: optional DRM file-private owner
> > *
> > * Uses kernel's IDR cyclic allocator (same as PID allocation).
> > * Allocates sequentially with automatic wrap-around.
> > @@ -60,17 +61,19 @@ struct amdgpu_pasid_cb {
> > * Returns %-ENOSPC if no PASID was available. Returns %-ENOMEM on
> > * memory allocation failure.
> > */
> > -int amdgpu_pasid_alloc(unsigned int bits)
> > +int amdgpu_pasid_alloc(unsigned int bits, struct amdgpu_fpriv *fpriv)
> > {
> > u32 pasid;
> > int r;
> > + void *entry;
> >
> > if (bits == 0)
> > return -EINVAL;
> >
> > - r = xa_alloc_cyclic_irq(&amdgpu_pasid_xa, &pasid, xa_mk_value(0),
> > - XA_LIMIT(1, (1U << bits) - 1),
> > - &amdgpu_pasid_xa_next, GFP_KERNEL);
> > + entry = fpriv ? fpriv : xa_mk_value(0);
>
> That's problematic I think. The xa_mk_value(0) value is not NULL and needs to
> be
> filtered out when somebody looks the fpriv up using the array.
>
> But xa_insert() can handle NULL entries, but I'm not sure if
> xa_alloc_cyclic_irq()
> can do that as well.
Thanks Christian.
I checked the XArray implementation. xa_alloc_cyclic_irq() goes through
__xa_alloc_cyclic(), which calls __xa_alloc(). __xa_alloc() converts a
NULL entry to XA_ZERO_ENTRY before storing it, and xa_load() converts
XA_ZERO_ENTRY back to NULL through xa_zero_to_null().
So NULL should work here as well. I'll update the patch to pass fpriv
directly to xa_alloc_cyclic_irq(), remove xa_mk_value(0), and simplify
the fpriv lookup path.
int __xa_alloc(struct xarray *xa, u32 *id, void *entry,
struct xa_limit limit, gfp_t gfp)
{
...
if (!entry)
entry = XA_ZERO_ENTRY;
...
xas_store(&xas, entry);
...
}
static inline void *xa_zero_to_null(void *entry)
{
return xa_is_zero(entry) ? NULL : entry;
}
void *xa_load(struct xarray *xa, unsigned long index)
{
...
entry = xa_zero_to_null(xas_load(&xas));
...
return entry;
}
Best regards,
Srini
