On 7/8/26 17:27, Srinivasan Shanmugam wrote: > AMDGPU already has a global PASID xarray used for PASID allocation. > > Allow amdgpu_pasid_alloc() to optionally store the owning DRM > file-private object directly. > > Initial callers pass NULL. 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 > > Clear the stored owner from amdgpu_pasid_free_delayed() before waiting > for outstanding fences so PASID lookups cannot observe a stale fpriv > while the PASID itself is pending delayed release. > > v5: > - Store NULL instead of xa_mk_value(0) for ownerless PASIDs. > - Simplify owner clearing by unconditionally storing NULL. > > v4: > - 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]>
Reviewed-by: Christian König <[email protected]> > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c | 69 +++++++++++++++++++++++-- > drivers/gpu/drm/amd/amdgpu/amdgpu_ids.h | 6 ++- > drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 2 +- > 3 files changed, 71 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c > index 684f40fce73f..d41601aab3c7 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,7 +61,7 @@ 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; > @@ -68,9 +69,9 @@ int amdgpu_pasid_alloc(unsigned int bits) > 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); > + r = xa_alloc_cyclic_irq(&amdgpu_pasid_xa, &pasid, fpriv, > + XA_LIMIT(1, (1U << bits) - 1), > + &amdgpu_pasid_xa_next, GFP_KERNEL); > if (r < 0) > return r; > > @@ -106,6 +107,64 @@ static void amdgpu_pasid_free_cb(struct dma_fence *fence, > kfree(cb); > } > > +/** > + * amdgpu_pasid_clear_owner - Clear the owner associated with a PASID > + * @pasid: PASID whose owner should be cleared > + * > + * Replace the stored owner with NULL while keeping the PASID allocated. > + * > + * This is used by the delayed PASID free path so that future PASID > + * lookups cannot resolve a stale DRM file-private object while the PASID > + * is still waiting for outstanding fences before being released. > + */ > +static void amdgpu_pasid_clear_owner(u32 pasid) > +{ > + unsigned long flags; > + > + if (!pasid) > + return; > + > + xa_lock_irqsave(&amdgpu_pasid_xa, flags); > + __xa_store(&amdgpu_pasid_xa, pasid, NULL, GFP_ATOMIC); > + xa_unlock_irqrestore(&amdgpu_pasid_xa, flags); > +} > + > +/** > + * amdgpu_pasid_lock - acquire the global PASID xarray lock > + * @flags: storage for interrupt state > + * > + * Acquire the global PASID xarray lock with interrupts disabled. > + * The saved interrupt state must be passed to > + * amdgpu_pasid_unlock(). > + */ > +void amdgpu_pasid_lock(unsigned long *flags) > +{ > + xa_lock_irqsave(&amdgpu_pasid_xa, *flags); > +} > + > +/** > + * amdgpu_pasid_unlock - release the global PASID xarray lock > + * @flags: interrupt state returned by amdgpu_pasid_lock() > + * > + * Release the global PASID xarray lock and restore the previous > + * interrupt state. > + */ > +void amdgpu_pasid_unlock(unsigned long flags) > +{ > + xa_unlock_irqrestore(&amdgpu_pasid_xa, flags); > +} > + > +/** > + * amdgpu_pasid_get_fpriv_locked - get fpriv from PASID > + * @pasid: PASID to resolve > + * > + * Caller must hold the PASID XA lock. > + */ > +struct amdgpu_fpriv *amdgpu_pasid_get_fpriv_locked(u32 pasid) > +{ > + return xa_load(&amdgpu_pasid_xa, pasid); > +} > + > /** > * amdgpu_pasid_free_delayed - free pasid when fences signal > * > @@ -121,6 +180,8 @@ void amdgpu_pasid_free_delayed(struct dma_resv *resv, > struct dma_fence *fence; > int r; > > + amdgpu_pasid_clear_owner(pasid); > + > r = dma_resv_get_singleton(resv, DMA_RESV_USAGE_BOOKKEEP, &fence); > if (r) > goto fallback; > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.h > b/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.h > index a57919478d3b..4b55d0d9703a 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.h > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.h > @@ -34,6 +34,7 @@ > #define AMDGPU_NUM_VMID 16 > > struct amdgpu_device; > +struct amdgpu_fpriv; > struct amdgpu_vm; > struct amdgpu_ring; > struct amdgpu_sync; > @@ -70,7 +71,10 @@ struct amdgpu_vmid_mgr { > bool reserved_vmid; > }; > > -int amdgpu_pasid_alloc(unsigned int bits); > +int amdgpu_pasid_alloc(unsigned int bits, struct amdgpu_fpriv *fpriv); > +void amdgpu_pasid_lock(unsigned long *flags); > +void amdgpu_pasid_unlock(unsigned long flags); > +struct amdgpu_fpriv *amdgpu_pasid_get_fpriv_locked(u32 pasid); > void amdgpu_pasid_free(u32 pasid); > void amdgpu_pasid_free_delayed(struct dma_resv *resv, > u32 pasid); > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c > index 6b4de2a6c629..0c973a3c73fa 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c > @@ -1488,7 +1488,7 @@ int amdgpu_driver_open_kms(struct drm_device *dev, > struct drm_file *file_priv) > goto out_suspend; > } > > - pasid = amdgpu_pasid_alloc(16); > + pasid = amdgpu_pasid_alloc(16, NULL); > if (pasid < 0) { > dev_warn(adev->dev, "No more PASIDs available!"); > pasid = 0;
