Re: [PATCH v2 30/32] drm/xe/uapi: Add uapi for vma count and mem attributes

["Ghimiray, Himal Prasad" <himal.prasad.ghimiray@intel.com>]

On 21-05-2025 14:24, Ghimiray, Himal Prasad wrote:
> 
> 
> On 15-05-2025 02:38, Matthew Brost wrote:
>> On Mon, Apr 07, 2025 at 03:47:17PM +0530, Himal Prasad Ghimiray wrote:
>>> -DRM_IOCTL_XE_VM_QUERY_VMAS: Return number of VMAs in user-specified 
>>> range.
>>> -DRM_IOCTL_XE_VM_QUERY_VMAS_ATTRS: Fill VMA attributes in user-provided
>>>   buffer.
>>>
>>
>> Replied to wrong version eariler...
>>
>>
>> I can't remember if we landed on if this is needed? I thought the answer
>> was - no not needed.
> 
> Will hold on this, till UMD confirms whether they need it or not.

UMD confirmed, they need this. Will add changes to support this via same 
ioctl as suggested.

> 
>>
>> If it is needed could be make this a single IOCTL. e.g. Call in once
>> with num_vmas == 0 + NULL vector, IOCTL returns num_vmas, then called
>> again with num_vmas != 0 + non-NULL vector. Generally we try not burn
>> IOCTL numbers, rather overload functionality.
>>
>> Matt
>>
>>
>>> Signed-off-by: Himal Prasad Ghimiray <himal.prasad.ghimiray@intel.com>
>>> ---
>>>   drivers/gpu/drm/xe/xe_device.c |   2 +
>>>   drivers/gpu/drm/xe/xe_vm.c     |  94 +++++++++++++++++++++++++++
>>>   drivers/gpu/drm/xe/xe_vm.h     |   3 +-
>>>   include/uapi/drm/xe_drm.h      | 115 +++++++++++++++++++++++++++++++++
>>>   4 files changed, 213 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/xe/xe_device.c b/drivers/gpu/drm/xe/ 
>>> xe_device.c
>>> index 3e57300014bf..968c24c77241 100644
>>> --- a/drivers/gpu/drm/xe/xe_device.c
>>> +++ b/drivers/gpu/drm/xe/xe_device.c
>>> @@ -198,6 +198,8 @@ static const struct drm_ioctl_desc xe_ioctls[] = {
>>>                 DRM_RENDER_ALLOW),
>>>       DRM_IOCTL_DEF_DRV(XE_OBSERVATION, xe_observation_ioctl, 
>>> DRM_RENDER_ALLOW),
>>>       DRM_IOCTL_DEF_DRV(XE_MADVISE, xe_vm_madvise_ioctl, 
>>> DRM_RENDER_ALLOW),
>>> +    DRM_IOCTL_DEF_DRV(XE_VM_QUERY_VMAS, xe_vm_query_vmas_ioctl, 
>>> DRM_RENDER_ALLOW),
>>> +    DRM_IOCTL_DEF_DRV(XE_VM_QUERY_VMAS_ATTRS, 
>>> xe_vm_query_vmas_attrs_ioctl, DRM_RENDER_ALLOW),
>>>   };
>>>   static long xe_drm_ioctl(struct file *file, unsigned int cmd, 
>>> unsigned long arg)
>>> diff --git a/drivers/gpu/drm/xe/xe_vm.c b/drivers/gpu/drm/xe/xe_vm.c
>>> index e5246c633e62..f1d4daf90efe 100644
>>> --- a/drivers/gpu/drm/xe/xe_vm.c
>>> +++ b/drivers/gpu/drm/xe/xe_vm.c
>>> @@ -2165,6 +2165,100 @@ int xe_vm_destroy_ioctl(struct drm_device 
>>> *dev, void *data,
>>>       return err;
>>>   }
>>> +int xe_vm_query_vmas_ioctl(struct drm_device *dev, void *data,
>>> +               struct drm_file *file)
>>> +{
>>> +    struct xe_device *xe = to_xe_device(dev);
>>> +    struct xe_file *xef = to_xe_file(file);
>>> +    struct drm_xe_vm_query_num_vmas *args = data;
>>> +    struct drm_gpuva *gpuva;
>>> +    struct xe_vm *vm;
>>> +
>>> +    vm = xe_vm_lookup(xef, args->vm_id);
>>> +    if (XE_IOCTL_DBG(xe, !vm))
>>> +        return -EINVAL;
>>> +
>>> +    args->num_vmas = 0;
>>> +    down_write(&vm->lock);
>>> +
>>> +    drm_gpuvm_for_each_va_range(gpuva, &vm->gpuvm, args->start, 
>>> args->start + args->range)
>>> +        args->num_vmas++;
>>> +
>>> +    up_write(&vm->lock);
>>> +    return 0;
>>> +}
>>> +
>>> +static int get_mem_attrs(struct xe_vm *vm, u32 *num_vmas, u64 start,
>>> +             u64 end, struct drm_xe_vma_mem_attr *mem_attrs)
>>> +{
>>> +    struct drm_gpuva *gpuva;
>>> +    int i = 0;
>>> +
>>> +    lockdep_assert_held(&vm->lock);
>>> +
>>> +    drm_gpuvm_for_each_va_range(gpuva, &vm->gpuvm, start, end) {
>>> +        struct xe_vma *vma = gpuva_to_vma(gpuva);
>>> +
>>> +        if (i == *num_vmas)
>>> +            return -EINVAL;
>>> +
>>> +        mem_attrs[i].start = xe_vma_start(vma);
>>> +        mem_attrs[i].end = xe_vma_end(vma);
>>> +        mem_attrs[i].atomic.val = vma->attr.atomic_access;
>>> +        mem_attrs[i].pat_index.val = vma->attr.pat_index;
>>> +        mem_attrs[i].preferred_mem_loc.devmem_fd = vma- 
>>> >attr.preferred_loc.devmem_fd;
>>> +        mem_attrs[i].preferred_mem_loc.migration_policy = vma- 
>>> >attr.preferred_loc.migration_policy;
>>> +
>>> +        i++;
>>> +    }
>>> +
>>> +    if (i <  (*num_vmas - 1))
>>> +        *num_vmas = i;
>>> +    return 0;
>>> +}
>>> +
>>> +int xe_vm_query_vmas_attrs_ioctl(struct drm_device *dev, void *data, 
>>> struct drm_file *file)
>>> +{
>>> +    struct xe_device *xe = to_xe_device(dev);
>>> +    struct xe_file *xef = to_xe_file(file);
>>> +    struct drm_xe_vma_mem_attr *mem_attrs;
>>> +    struct drm_xe_vm_query_vmas_attr *args = data;
>>> +    u64 __user *attrs_user = NULL;
>>> +    struct xe_vm *vm;
>>> +    int err;
>>> +
>>> +    if (XE_IOCTL_DBG(xe, args->num_vmas < 1))
>>> +        return -EINVAL;
>>> +
>>> +    vm = xe_vm_lookup(xef, args->vm_id);
>>> +    if (XE_IOCTL_DBG(xe, !vm))
>>> +        return -EINVAL;
>>> +
>>> +    down_write(&vm->lock);
>>> +
>>> +    attrs_user = u64_to_user_ptr(args->vector_of_vma_mem_attr);
>>> +    mem_attrs = kvmalloc_array(args->num_vmas, sizeof(struct 
>>> drm_xe_vma_mem_attr),
>>> +                   GFP_KERNEL | __GFP_ACCOUNT |
>>> +                   __GFP_RETRY_MAYFAIL | __GFP_NOWARN);
>>> +    if (!mem_attrs)
>>> +        return args->num_vmas > 1 ? -ENOBUFS : -ENOMEM;
>>> +
>>> +    err = get_mem_attrs(vm, &args->num_vmas, args->start,
>>> +                args->start + args->range, mem_attrs);
>>> +    if (err)
>>> +        goto free_mem_attrs;
>>> +
>>> +    err = __copy_to_user(attrs_user, mem_attrs,
>>> +                 sizeof(struct drm_xe_vma_mem_attr) * args->num_vmas);
>>> +
>>> +free_mem_attrs:
>>> +    kvfree(mem_attrs);
>>> +
>>> +    up_write(&vm->lock);
>>> +
>>> +    return err;
>>> +}
>>> +
>>>   static bool vma_matches(struct xe_vma *vma, u64 page_addr)
>>>   {
>>>       if (page_addr > xe_vma_end(vma) - 1 ||
>>> diff --git a/drivers/gpu/drm/xe/xe_vm.h b/drivers/gpu/drm/xe/xe_vm.h
>>> index 377f62f859b7..0b2d6e9f77ef 100644
>>> --- a/drivers/gpu/drm/xe/xe_vm.h
>>> +++ b/drivers/gpu/drm/xe/xe_vm.h
>>> @@ -193,7 +193,8 @@ int xe_vm_destroy_ioctl(struct drm_device *dev, 
>>> void *data,
>>>               struct drm_file *file);
>>>   int xe_vm_bind_ioctl(struct drm_device *dev, void *data,
>>>                struct drm_file *file);
>>> -
>>> +int xe_vm_query_vmas_ioctl(struct drm_device *dev, void *data, 
>>> struct drm_file *file);
>>> +int xe_vm_query_vmas_attrs_ioctl(struct drm_device *dev, void *data, 
>>> struct drm_file *file);
>>>   void xe_vm_close_and_put(struct xe_vm *vm);
>>>   static inline bool xe_vm_in_fault_mode(struct xe_vm *vm)
>>> diff --git a/include/uapi/drm/xe_drm.h b/include/uapi/drm/xe_drm.h
>>> index ab96dee25f6c..177ee3a1c20d 100644
>>> --- a/include/uapi/drm/xe_drm.h
>>> +++ b/include/uapi/drm/xe_drm.h
>>> @@ -82,6 +82,8 @@ extern "C" {
>>>    *  - &DRM_IOCTL_XE_WAIT_USER_FENCE
>>>    *  - &DRM_IOCTL_XE_OBSERVATION
>>>    *  - &DRM_IOCTL_XE_MADVISE
>>> + *  - &DRM_IOCTL_XE_VM_QUERY_VMAS
>>> + *  - &DRM_IOCTL_XE_VM_QUERY_VMAS_ATTRS
>>>    */
>>>   /*
>>> @@ -104,6 +106,8 @@ extern "C" {
>>>   #define DRM_XE_WAIT_USER_FENCE        0x0a
>>>   #define DRM_XE_OBSERVATION        0x0b
>>>   #define DRM_XE_MADVISE            0x0c
>>> +#define DRM_XE_VM_QUERY_VMAS        0x0d
>>> +#define DRM_XE_VM_QUERY_VMAS_ATTRS    0x0e
>>>   /* Must be kept compact -- no holes */
>>> @@ -120,6 +124,8 @@ extern "C" {
>>>   #define DRM_IOCTL_XE_WAIT_USER_FENCE        
>>> DRM_IOWR(DRM_COMMAND_BASE + DRM_XE_WAIT_USER_FENCE, struct 
>>> drm_xe_wait_user_fence)
>>>   #define DRM_IOCTL_XE_OBSERVATION        DRM_IOW(DRM_COMMAND_BASE + 
>>> DRM_XE_OBSERVATION, struct drm_xe_observation_param)
>>>   #define DRM_IOCTL_XE_MADVISE            DRM_IOWR(DRM_COMMAND_BASE + 
>>> DRM_XE_MADVISE, struct drm_xe_madvise)
>>> +#define DRM_IOCTL_XE_VM_QUERY_VMAS        DRM_IOWR(DRM_COMMAND_BASE 
>>> + DRM_XE_VM_QUERY_VMAS, struct drm_xe_vm_query_num_vmas)
>>> +#define DRM_IOCTL_XE_VM_QUERY_VMAS_ATTRS    
>>> DRM_IOWR(DRM_COMMAND_BASE + DRM_XE_VM_QUERY_VMAS_ATTRS, struct 
>>> drm_xe_vm_query_vmas_attr)
>>>   /**
>>>    * DOC: Xe IOCTL Extensions
>>> @@ -2063,6 +2069,115 @@ struct drm_xe_madvise {
>>>   };
>>> +/**
>>> + * struct drm_xe_vm_query_num_vmas - Input of 
>>> &DRM_IOCTL_XE_VM_QUERY_VMAS
>>> + *
>>> + * Get number of vmas in virtual range of vm_id
>>> + */
>>> +struct drm_xe_vm_query_num_vmas {
>>> +     /** @extensions: Pointer to the first extension struct, if any */
>>> +    __u64 extensions;
>>> +
>>> +    /** @vm_id: vm_id of the virtual range */
>>> +    __u32 vm_id;
>>> +
>>> +    /** @num_vmas: number of vmas in range returned in @num_vmas */
>>> +    __u32 num_vmas;
>>> +
>>> +    /** @start: start of the virtual address range */
>>> +    __u64 start;
>>> +
>>> +    /** @size: size of the virtual address range */
>>> +    __u64 range;
>>> +
>>> +     /** @reserved: Reserved */
>>> +    __u64 reserved[2];
>>> +};
>>> +
>>> +struct drm_xe_vma_mem_attr {
>>> +     /** @extensions: Pointer to the first extension struct, if any */
>>> +    __u64 extensions;
>>> +
>>> +    /** @start: start of the vma */
>>> +    __u64 start;
>>> +
>>> +    /** @size: end of the vma */
>>> +    __u64 end;
>>> +
>>> +    struct {
>>> +        struct {
>>> +        /** @val: value of atomic operation*/
>>> +            __u32 val;
>>> +
>>> +        /** @reserved: Reserved */
>>> +            __u32 reserved;
>>> +        } atomic;
>>> +
>>> +        struct {
>>> +        /** @val: value for DRM_XE_VMA_ATTR_PURGEABLE_STATE */
>>> +            __u32 val;
>>> +
>>> +        /** @reserved: Reserved */
>>> +            __u32 reserved;
>>> +        } purge_state_val;
>>> +
>>> +        struct {
>>> +            /** @pat_index */
>>> +            __u32 val;
>>> +
>>> +            /** @reserved: Reserved */
>>> +            __u32 reserved;
>>> +        } pat_index;
>>> +
>>> +        /** @preferred_mem_loc: preferred memory location */
>>> +        struct {
>>> +            __u32 devmem_fd;
>>> +
>>> +            __u32 migration_policy;
>>> +        } preferred_mem_loc;
>>> +    };
>>> +
>>> +     /** @reserved: Reserved */
>>> +    __u64 reserved[2];
>>> +};
>>> +
>>> +/**
>>> + * struct drm_xe_vm_query_vmas_attr - Input of 
>>> &DRM_IOCTL_XE_VM_QUERY_MEM_ATTRIBUTES
>>> + *
>>> + * Get memory attributes to a virtual address range
>>> + */
>>> +struct drm_xe_vm_query_vmas_attr {
>>> +    /** @extensions: Pointer to the first extension struct, if any */
>>> +    __u64 extensions;
>>> +
>>> +    /** @vm_id: vm_id of the virtual range */
>>> +    __u32 vm_id;
>>> +
>>> +    /** @num_vmas: number of vmas in range returned in @num_vmas */
>>> +    __u32 num_vmas;
>>> +
>>> +    /** @start: start of the virtual address range */
>>> +    __u64 start;
>>> +
>>> +    /** @size: size of the virtual address range */
>>> +    __u64 range;
>>> +
>>> +    union {
>>> +        /** @num_vmas: used if num_vmas == 1 */
>>> +        struct drm_xe_vma_mem_attr attr;
>>> +
>>> +        /**
>>> +         * @vector_of_ops: userptr to array of struct
>>> +         * drm_xe_vma_mem_attr if num_vmas > 1
>>> +         */
>>> +        __u64 vector_of_vma_mem_attr;
>>> +    };
>>> +
>>> +    /** @reserved: Reserved */
>>> +    __u64 reserved[2];
>>> +
>>> +};
>>> +
>>>   #if defined(__cplusplus)
>>>   }
>>>   #endif
>>> -- 
>>> 2.34.1
>>>
>