[PATCH v12 2/3] drm/amdgpu: Add mapping info option for GEM_OP ioctl
Christian König
christian.koenig at amd.com
Tue Aug 12 09:25:52 UTC 2025
On 12.08.25 11:20, Tvrtko Ursulin wrote:
>
> On 11/08/2025 16:05, David Francis wrote:
>> Add new GEM_OP_IOCTL option GET_MAPPING_INFO, which
>> returns a list of mappings associated with a given bo, along with
>> their positions and offsets.
>>
>> Signed-off-by: David Francis <David.Francis at amd.com>
>> ---
>> drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 86 ++++++++++++++++++++++++-
>> drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h | 5 ++
>> include/uapi/drm/amdgpu_drm.h | 18 ++++++
>> 3 files changed, 108 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
>> index 3873d2c19b4b..1632460623b2 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
>> @@ -885,7 +885,7 @@ int amdgpu_gem_va_ioctl(struct drm_device *dev, void *data,
>> goto error;
>> }
>> - r = amdgpu_vm_lock_pd(&fpriv->vm, &exec, 2);
>> + r = amdgpu_vm_lock_pd(&fpriv->vm, &exec, 0);
>
> Why is this new hunk related? Is it becase I asked what are the two reserved slots in amdgpu_gem_list_mappings() for? But that one remained so it is all unclear to me.
Yeah that looks like a bug to me. The change was most likely applied to the wrong code.
>
>> drm_exec_retry_on_contention(&exec);
>> if (unlikely(r))
>> goto error;
>> @@ -956,6 +956,86 @@ int amdgpu_gem_va_ioctl(struct drm_device *dev, void *data,
>> return r;
>> }
>> +/**
>> + * amdgpu_gem_list_mappings - get information about a buffer's mappings
>> + *
>> + * @gobj: gem object
>> + * @args: gem_op arguments
>> + * @fpriv: drm file pointer
>> + *
>> + * num_entries is set as an input to the size of the user-allocated array of
>> + * drm_amdgpu_gem_vm_bucket stored at args->value.
>> + * num_entries is sent back as output as the number of mappings the bo has.
>> + * If that number is larger than the size of the array, the ioctl must
>> + * be retried.
>> + *
>> + * Returns:
>> + * 0 for success, -errno for errors.
>> + */
>> +static int amdgpu_gem_list_mappings(struct drm_gem_object *gobj, struct amdgpu_fpriv *fpriv,
>> + struct drm_amdgpu_gem_op *args)
>> +{
>> + struct amdgpu_vm *avm = &fpriv->vm;
>> + struct amdgpu_bo *bo = gem_to_amdgpu_bo(gobj);
>> + struct amdgpu_bo_va *bo_va = amdgpu_vm_bo_find(avm, bo);
>> + struct drm_amdgpu_gem_vm_bucket *vm_buckets;
>> + struct amdgpu_bo_va_mapping *mapping;
>> + struct drm_exec exec;
>> + int num_mappings = 0;
>> + int ret;
>> +
>> + vm_buckets = kvcalloc(args->num_entries, sizeof(*vm_buckets), GFP_KERNEL);
>> + if (!vm_buckets)
>> + return -ENOMEM;
>> +
>> + drm_exec_init(&exec, DRM_EXEC_INTERRUPTIBLE_WAIT |
>> + DRM_EXEC_IGNORE_DUPLICATES, 0);
>> + drm_exec_until_all_locked(&exec) {
>> + if (gobj) {
>> + ret = drm_exec_lock_obj(&exec, gobj);
>> + drm_exec_retry_on_contention(&exec);
>> + if (ret)
>> + goto unlock_exec;
>> + }
>> +
>> + ret = amdgpu_vm_lock_pd(&fpriv->vm, &exec, 2);
>> + drm_exec_retry_on_contention(&exec);
>> + if (ret)
>> + goto unlock_exec;
>> + }
>> +
>> + amdgpu_vm_bo_va_for_each_valid_mapping(bo_va, mapping) {
>> + if (num_mappings < args->num_entries) {
>> + vm_buckets[num_mappings].start = mapping->start;
>> + vm_buckets[num_mappings].last = mapping->last;
>> + vm_buckets[num_mappings].offset = mapping->offset;
>> + vm_buckets[num_mappings].flags = mapping->flags;
>> + }
>> + num_mappings += 1;
>> + }
>> +
>> + amdgpu_vm_bo_va_for_each_invalid_mapping(bo_va, mapping) {
>> + if (num_mappings < args->num_entries) {
>> + vm_buckets[num_mappings].start = mapping->start;
>> + vm_buckets[num_mappings].last = mapping->last;
>> + vm_buckets[num_mappings].offset = mapping->offset;
>> + vm_buckets[num_mappings].flags = mapping->flags;
>> + }
>> + num_mappings += 1;
>> + }
>> +
>> + if (num_mappings > 0 && num_mappings <= args->num_entries)
>> + ret = copy_to_user(u64_to_user_ptr(args->value), vm_buckets, num_mappings * sizeof(*vm_buckets));
>> +
>> + args->num_entries = num_mappings;
>> +
>> +unlock_exec:
>> + drm_exec_fini(&exec);
>> + kvfree(vm_buckets);
>> +
>> + return ret;
>> +}
>> +
>> int amdgpu_gem_op_ioctl(struct drm_device *dev, void *data,
>> struct drm_file *filp)
>> {
>> @@ -1022,6 +1102,10 @@ int amdgpu_gem_op_ioctl(struct drm_device *dev, void *data,
>> amdgpu_bo_unreserve(robj);
>> break;
>> + case AMDGPU_GEM_OP_GET_MAPPING_INFO:
>> + amdgpu_bo_unreserve(robj);
>> + r = amdgpu_gem_list_mappings(gobj, filp->driver_priv, args);
>> + break;
>> default:
>> amdgpu_bo_unreserve(robj);
>> r = -EINVAL;
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
>> index f9549f6b3d1f..5a63ae490b0e 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
>> @@ -668,4 +668,9 @@ void amdgpu_vm_tlb_fence_create(struct amdgpu_device *adev,
>> struct amdgpu_vm *vm,
>> struct dma_fence **fence);
>> +#define amdgpu_vm_bo_va_for_each_valid_mapping(bo_va, mapping) \
>> + list_for_each_entry(mapping, &bo_va->valids, list)
>> +#define amdgpu_vm_bo_va_for_each_invalid_mapping(bo_va, mapping) \
>> + list_for_each_entry(mapping, &bo_va->invalids, list)
>> +
>> #endif
>> diff --git a/include/uapi/drm/amdgpu_drm.h b/include/uapi/drm/amdgpu_drm.h
>> index 59b423883e91..7072959103e6 100644
>> --- a/include/uapi/drm/amdgpu_drm.h
>> +++ b/include/uapi/drm/amdgpu_drm.h
>> @@ -802,6 +802,21 @@ union drm_amdgpu_wait_fences {
>> #define AMDGPU_GEM_OP_GET_GEM_CREATE_INFO 0
>> #define AMDGPU_GEM_OP_SET_PLACEMENT 1
>> +#define AMDGPU_GEM_OP_GET_MAPPING_INFO 2
>> +
>> +struct drm_amdgpu_gem_vm_bucket {
>> + /* Start of mapping (in number of pages) */
>> + __u64 start;
>> +
>> + /* End of mapping (in number of pages) */
>> + __u64 last;
>> +
>> + /* Mapping offset */
>> + __u64 offset;
>> +
>> + /* flags needed to recreate mapping */
>> + __u64 flags;
>> +};
>> /* Sets or returns a value associated with a buffer. */
>> struct drm_amdgpu_gem_op {
>> @@ -811,6 +826,9 @@ struct drm_amdgpu_gem_op {
>> __u32 op;
>> /** Input or return value */
>> __u64 value;
>> + /** For MAPPING_INFO op: number of mappings (in/out) */
>> + __u32 num_entries;
>> + __u32 padding;
>
> I suppose uapi kerneldoc should explain the value field is also used for AMDGPU_GEM_OP_GET_MAPPING_INFO and for what.
>
> Alternatively union could be used to self document it but not sure if that is acceptable for amdgpu_drm.h? Like:
>
> struct drm_amdgpu_gem_op {
> /** GEM object handle */
> __u32 handle;
> /** AMDGPU_GEM_OP_* */
> __u32 op;
> union {
> /** Input or return value */
> __u64 value;
>
> /** AMDGPU_GEM_OP_GET_MAPPING_INFO */
> struct {
> __u64 entries; /** Pointer to struct drm_amdgpu_gem_vm_bucket array (out) */
> __u64 num_entries; /** Number of mappings (in/out) */
> };
> };
> };
>
> Just a thought.
Na, that looks quite ugly I think.
Extending the structure at the end is perfectly supported, but with the union you need to be really careful not to mess things up.
Regards,
Christian.
>
> Regards,
>
> Tvrtko
>
>> };
>> #define AMDGPU_GEM_LIST_HANDLES_FLAG_IS_IMPORT (1 << 0)
>
More information about the amd-gfx
mailing list