Re: [PATCH v3 6/6] drm/xe/xe_vm: Implement xe_vm_get_property_ioctl

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 






From: Cavitt, Jonathan <jonathan.cavitt@xxxxxxxxx>
Sent: Friday, February 28, 2025 10:21 AM
To: intel-xe@xxxxxxxxxxxxxxxxxxxxx <intel-xe@xxxxxxxxxxxxxxxxxxxxx>
Cc: Gupta, saurabhg <saurabhg.gupta@xxxxxxxxx>; Zuo, Alex <alex.zuo@xxxxxxxxx>; Cavitt, Jonathan <jonathan.cavitt@xxxxxxxxx>; joonas.lahtinen@xxxxxxxxxxxxxxx <joonas.lahtinen@xxxxxxxxxxxxxxx>; Brost, Matthew <matthew.brost@xxxxxxxxx>; Zhang, Jianxun <jianxun.zhang@xxxxxxxxx>; dri-devel@xxxxxxxxxxxxxxxxxxxxx <dri-devel@xxxxxxxxxxxxxxxxxxxxx>
Subject: [PATCH v3 6/6] drm/xe/xe_vm: Implement xe_vm_get_property_ioctl
 
Add support for userspace to request a list of observed failed
pagefaults from a specified VM.

v2:
- Only allow querying of failed pagefaults (Matt Brost)

Signed-off-by: Jonathan Cavitt <jonathan.cavitt@xxxxxxxxx>
Suggested-by: Matthew Brost <matthew.brost@xxxxxxxxx>
---
 drivers/gpu/drm/xe/xe_device.c |  3 ++
 drivers/gpu/drm/xe/xe_vm.c     | 79 ++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/xe/xe_vm.h     |  2 +
 3 files changed, 84 insertions(+)

diff --git a/drivers/gpu/drm/xe/xe_device.c b/drivers/gpu/drm/xe/xe_device.c
index 9454b51f7ad8..43accae152ff 100644
--- a/drivers/gpu/drm/xe/xe_device.c
+++ b/drivers/gpu/drm/xe/xe_device.c
@@ -193,6 +193,9 @@ static const struct drm_ioctl_desc xe_ioctls[] = {
         DRM_IOCTL_DEF_DRV(XE_WAIT_USER_FENCE, xe_wait_user_fence_ioctl,
                           DRM_RENDER_ALLOW),
         DRM_IOCTL_DEF_DRV(XE_OBSERVATION, xe_observation_ioctl, DRM_RENDER_ALLOW),
+       DRM_IOCTL_DEF_DRV(XE_VM_GET_PROPERTY, xe_vm_get_property_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 6211b971bbbd..00d2c62ccf53 100644
--- a/drivers/gpu/drm/xe/xe_vm.c
+++ b/drivers/gpu/drm/xe/xe_vm.c
@@ -3234,6 +3234,85 @@ int xe_vm_bind_ioctl(struct drm_device *dev, void *data, struct drm_file *file)
         return err;
 }
 
+static u32 xe_vm_get_property_size(struct xe_vm *vm, u32 property)
+{
+       u32 size = 0;
+
+       switch (property) {
+       case DRM_XE_VM_GET_PROPERTY_FAULTS:
+               spin_lock(&vm->pfs.lock);
+               size = vm->pfs.len * sizeof(struct drm_xe_pf);
+               spin_unlock(&vm->pfs.lock);
+               return size;
+       default:
+               return -EINVAL;
+       }
+}
+
+static int fill_property_pfs(struct xe_vm *vm,
+                            struct drm_xe_vm_get_property *args,
+                            u32 size)
+{
+       struct drm_xe_pf __user *usr_ptr = u64_to_user_ptr(args->ptr);
+       struct drm_xe_pf *fault_list;
+       struct drm_xe_pf *fault;
+       struct xe_vm_pf_entry *entry;
+       int i = 0;
+
+       if (copy_from_user(&fault_list, usr_ptr, size))
+               return -EFAULT;
Why is copying from user space memory needed in this query ioctl?

Does copy_from_user() automatically allocate kernel memory for fault_list?

+
+       spin_lock(&vm->pfs.lock);
+       list_for_each_entry(entry, &vm->pfs.list, list) {
+               struct xe_pagefault *pf = entry->pf;
+
+               fault = &fault_list[i++];
+               fault->address = pf->page_addr;
+               fault->address_type = pf->address_type;
+               fault->address_precision = SZ_4K;
If we can get the exact address, the precision should be 1, right? (https://registry.khronos.org/vulkan/specs/latest/man/html/VkDeviceFaultAddressInfoEXT.html)

+       }
+       spin_unlock(&vm->pfs.lock);
+
+       if (copy_to_user(usr_ptr, &fault_list, size))
+               return -EFAULT;
+
+       return 0;
+}
+
+int xe_vm_get_property_ioctl(struct drm_device *drm, void *data,
+                            struct drm_file *file)
+{
+       struct xe_device *xe = to_xe_device(drm);
+       struct xe_file *xef = to_xe_file(file);
+       struct drm_xe_vm_get_property *args = data;
+       struct xe_vm *vm;
+       u32 size;
+
+       if (XE_IOCTL_DBG(xe, args->reserved[0] || args->reserved[1]))
+               return -EINVAL;
+
+       vm = xe_vm_lookup(xef, args->vm_id);
+       if (XE_IOCTL_DBG(xe, !vm))
+               return -ENOENT;
+
+       size = xe_vm_get_property_size(vm, args->property);
+       if (size < 0) {
+               return size;
+       } else if (args->size != size) {
+               if (args->size)
+                       return -EINVAL;
If there is a change in the size of property between the first and the second calls, say, more faults added in this case, the 2nd call will fail unintendedly. 
+               args->size = size;
​+               return 0;
+       }
+
+       switch (args->property) {
+       case DRM_XE_VM_GET_PROPERTY_FAULTS:
+               return fill_property_pfs(vm, args, size);
+       default:
+               return -EINVAL;
+       }
+}
+
 /**
  * xe_vm_bind_kernel_bo - bind a kernel BO to a VM
  * @vm: VM to bind the BO to
diff --git a/drivers/gpu/drm/xe/xe_vm.h b/drivers/gpu/drm/xe/xe_vm.h
index 4d94ab5c8ea4..bf6604465aa3 100644
--- a/drivers/gpu/drm/xe/xe_vm.h
+++ b/drivers/gpu/drm/xe/xe_vm.h
@@ -184,6 +184,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_get_property_ioctl(struct drm_device *dev, void *data,
+                            struct drm_file *file);
 
 void xe_vm_close_and_put(struct xe_vm *vm);
 
--
2.43.0


[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux