On 7/2/26 10:40, Srinivasan Shanmugam wrote:
> AMDGPU already has a global PASID xarray used by the PASID allocator.
>
> Currently allocated PASIDs store a dummy value in that xarray.
>
> Add helper functions so DRM-owned PASIDs can store and retrieve their
> owning DRM file-private object.
>
> This prepares for using:
>
> PASID -> fpriv -> vm
>
> instead of the separate per-device:
>
> PASID -> vm
>
> mapping.
>
> v3: (per Christian)
> - Document that PASID allocation is intentionally kept separate
> from fpriv registration because fpriv is not fully initialized
> when the PASID is allocated.
>
> 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 | 126 ++++++++++++++++++++++++
> drivers/gpu/drm/amd/amdgpu/amdgpu_ids.h | 13 +++
> 2 files changed, 139 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c
> index 684f40fce73f..5333937f0d04 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c
> @@ -78,6 +78,132 @@ int amdgpu_pasid_alloc(unsigned int bits)
> return pasid;
> }
>
> +/**
> + * amdgpu_pasid_set_fpriv - register the DRM owner of a PASID
> + * @pasid: PASID allocated for the DRM client
> + * @fpriv: owning DRM file-private object
> + *
> + * PASID allocation is intentionally kept separate from owner
> + * registration because the DRM file-private object is not fully
> + * initialized when the PASID is allocated.
> + *
> + * This helper replaces the temporary allocation marker with the
> + * initialized DRM owner.
Taking a closer look at I think we should rather re-structure the fprif init
code to allocate the pasid later.
> + */
> +int amdgpu_pasid_set_fpriv(u32 pasid,
> + struct amdgpu_fpriv *fpriv)
> +{
> + unsigned long flags;
> + void *entry;
> + int r = 0;
> +
> + if (!pasid || !fpriv)
> + return -EINVAL;
> +
> + xa_lock_irqsave(&amdgpu_pasid_xa, flags);
> +
> + entry = xa_load(&amdgpu_pasid_xa, pasid);
> +
> + if (!entry) {
> + r = -ENOENT;
> + goto unlock;
> + }
> +
> + if (!xa_is_value(entry)) {
> + r = -EBUSY;
> + goto unlock;
> + }
> +
> + entry = __xa_store(&amdgpu_pasid_xa, pasid,
> + fpriv, GFP_ATOMIC);
> + r = xa_err(entry);
> +
> +unlock:
> + xa_unlock_irqrestore(&amdgpu_pasid_xa, flags);
> +
> + return r;
> +}
> +
> +/**
> + * amdgpu_pasid_clear_fpriv - unregister PASID ownership
> + * @pasid: PASID whose owner is being removed
> + * @fpriv: DRM file-private object owning the PASID
> + *
> + * Restore the PASID entry back to the allocation marker before the
> + * DRM file-private object is destroyed.
That should clearly be merged into
amdgpu_pasid_free()/amdgpu_pasid_free_delayed() instead.
Regards,
Christian.
> + */
> +void amdgpu_pasid_clear_fpriv(u32 pasid,
> + struct amdgpu_fpriv *fpriv)
> +{
> + unsigned long flags;
> + void *entry;
> +
> + if (!pasid || !fpriv)
> + return;
> +
> + xa_lock_irqsave(&amdgpu_pasid_xa, flags);
> +
> + entry = xa_load(&amdgpu_pasid_xa, pasid);
> +
> + if (entry == fpriv)
> + __xa_store(&amdgpu_pasid_xa,
> + pasid,
> + xa_mk_value(0),
> + 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.
> + *
> + * The returned pointer is only protected by the PASID XA lock.
> + * Callers must not continue using it after dropping the lock
> + * unless additional lifetime protection exists.
> + *
> + * This intentionally does not add kref/RCU lifetime handling.
> + */
> + struct amdgpu_fpriv *
> +amdgpu_pasid_get_fpriv_locked(u32 pasid)
> +{
> + void *entry;
> +
> + entry = xa_load(&amdgpu_pasid_xa, pasid);
> +
> + if (!entry || xa_is_value(entry))
> + return NULL;
> +
> + return entry;
> +}
> +
> /**
> * amdgpu_pasid_free - Free a PASID
> * @pasid: PASID to free
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.h
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.h
> index a57919478d3b..220a0ba0cfb6 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.h
> @@ -38,6 +38,7 @@ struct amdgpu_vm;
> struct amdgpu_ring;
> struct amdgpu_sync;
> struct amdgpu_job;
> +struct amdgpu_fpriv;
>
> struct amdgpu_vmid {
> struct list_head list;
> @@ -92,4 +93,16 @@ void amdgpu_vmid_reset_all(struct amdgpu_device *adev);
> void amdgpu_vmid_mgr_init(struct amdgpu_device *adev);
> void amdgpu_vmid_mgr_fini(struct amdgpu_device *adev);
>
> +int amdgpu_pasid_set_fpriv(u32 pasid,
> + struct amdgpu_fpriv *fpriv);
> +
> +void amdgpu_pasid_clear_fpriv(u32 pasid,
> + 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);
> +
> #endif
