Skip to content

Commit b65a948

Browse files
icklemlankhorst
authored andcommitted
drm/i915/userptr: Probe existence of backing struct pages upon creation
Jason Ekstrand requested a more efficient method than userptr+set-domain to determine if the userptr object was backed by a complete set of pages upon creation. To be more efficient than simply populating the userptr using get_user_pages() (as done by the call to set-domain or execbuf), we can walk the tree of vm_area_struct and check for gaps or vma not backed by struct page (VM_PFNMAP). The question is how to handle VM_MIXEDMAP which may be either struct page or pfn backed... With discrete we are going to drop support for set_domain(), so offering a way to probe the pages, without having to resort to dummy batches has been requested. v2: - add new query param for the PROBE flag, so userspace can easily check if the kernel supports it(Jason). - use mmap_read_{lock, unlock}. - add some kernel-doc. v3: - In the docs also mention that PROBE doesn't guarantee that the pages will remain valid by the time they are actually used(Tvrtko). - Add a small comment for the hole finding logic(Jason). - Move the param next to all the other params which just return true. Testcase: igt/gem_userptr_blits/probe Signed-off-by: Chris Wilson <[email protected]> Signed-off-by: Matthew Auld <[email protected]> Cc: Thomas Hellström <[email protected]> Cc: Maarten Lankhorst <[email protected]> Cc: Tvrtko Ursulin <[email protected]> Cc: Jordan Justen <[email protected]> Cc: Kenneth Graunke <[email protected]> Cc: Jason Ekstrand <[email protected]> Cc: Daniel Vetter <[email protected]> Cc: Ramalingam C <[email protected]> Reviewed-by: Tvrtko Ursulin <[email protected]> Acked-by: Kenneth Graunke <[email protected]> Reviewed-by: Jason Ekstrand <[email protected]> Acked-by: Maarten Lankhorst <[email protected]> Signed-off-by: Maarten Lankhorst <[email protected]> Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
1 parent 8e02cce commit b65a948

File tree

3 files changed

+61
-1
lines changed

3 files changed

+61
-1
lines changed

drivers/gpu/drm/i915/gem/i915_gem_userptr.c

Lines changed: 40 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -422,6 +422,34 @@ static const struct drm_i915_gem_object_ops i915_gem_userptr_ops = {
422422

423423
#endif
424424

425+
static int
426+
probe_range(struct mm_struct *mm, unsigned long addr, unsigned long len)
427+
{
428+
const unsigned long end = addr + len;
429+
struct vm_area_struct *vma;
430+
int ret = -EFAULT;
431+
432+
mmap_read_lock(mm);
433+
for (vma = find_vma(mm, addr); vma; vma = vma->vm_next) {
434+
/* Check for holes, note that we also update the addr below */
435+
if (vma->vm_start > addr)
436+
break;
437+
438+
if (vma->vm_flags & (VM_PFNMAP | VM_MIXEDMAP))
439+
break;
440+
441+
if (vma->vm_end >= end) {
442+
ret = 0;
443+
break;
444+
}
445+
446+
addr = vma->vm_end;
447+
}
448+
mmap_read_unlock(mm);
449+
450+
return ret;
451+
}
452+
425453
/*
426454
* Creates a new mm object that wraps some normal memory from the process
427455
* context - user memory.
@@ -477,7 +505,8 @@ i915_gem_userptr_ioctl(struct drm_device *dev,
477505
}
478506

479507
if (args->flags & ~(I915_USERPTR_READ_ONLY |
480-
I915_USERPTR_UNSYNCHRONIZED))
508+
I915_USERPTR_UNSYNCHRONIZED |
509+
I915_USERPTR_PROBE))
481510
return -EINVAL;
482511

483512
if (i915_gem_object_size_2big(args->user_size))
@@ -504,6 +533,16 @@ i915_gem_userptr_ioctl(struct drm_device *dev,
504533
return -ENODEV;
505534
}
506535

536+
if (args->flags & I915_USERPTR_PROBE) {
537+
/*
538+
* Check that the range pointed to represents real struct
539+
* pages and not iomappings (at this moment in time!)
540+
*/
541+
ret = probe_range(current->mm, args->user_ptr, args->user_size);
542+
if (ret)
543+
return ret;
544+
}
545+
507546
#ifdef CONFIG_MMU_NOTIFIER
508547
obj = i915_gem_object_alloc();
509548
if (obj == NULL)

drivers/gpu/drm/i915/i915_getparam.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,7 @@ int i915_getparam_ioctl(struct drm_device *dev, void *data,
134134
case I915_PARAM_HAS_EXEC_FENCE_ARRAY:
135135
case I915_PARAM_HAS_EXEC_SUBMIT_FENCE:
136136
case I915_PARAM_HAS_EXEC_TIMELINE_FENCES:
137+
case I915_PARAM_HAS_USERPTR_PROBE:
137138
/* For the time being all of these are always true;
138139
* if some supported hardware does not have one of these
139140
* features this value needs to be provided from

include/uapi/drm/i915_drm.h

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -683,6 +683,9 @@ typedef struct drm_i915_irq_wait {
683683
*/
684684
#define I915_PARAM_HAS_EXEC_TIMELINE_FENCES 55
685685

686+
/* Query if the kernel supports the I915_USERPTR_PROBE flag. */
687+
#define I915_PARAM_HAS_USERPTR_PROBE 56
688+
686689
/* Must be kept compact -- no holes and well documented */
687690

688691
typedef struct drm_i915_getparam {
@@ -2231,12 +2234,29 @@ struct drm_i915_gem_userptr {
22312234
* through the GTT. If the HW can't support readonly access, an error is
22322235
* returned.
22332236
*
2237+
* I915_USERPTR_PROBE:
2238+
*
2239+
* Probe the provided @user_ptr range and validate that the @user_ptr is
2240+
* indeed pointing to normal memory and that the range is also valid.
2241+
* For example if some garbage address is given to the kernel, then this
2242+
* should complain.
2243+
*
2244+
* Returns -EFAULT if the probe failed.
2245+
*
2246+
* Note that this doesn't populate the backing pages, and also doesn't
2247+
* guarantee that the object will remain valid when the object is
2248+
* eventually used.
2249+
*
2250+
* The kernel supports this feature if I915_PARAM_HAS_USERPTR_PROBE
2251+
* returns a non-zero value.
2252+
*
22342253
* I915_USERPTR_UNSYNCHRONIZED:
22352254
*
22362255
* NOT USED. Setting this flag will result in an error.
22372256
*/
22382257
__u32 flags;
22392258
#define I915_USERPTR_READ_ONLY 0x1
2259+
#define I915_USERPTR_PROBE 0x2
22402260
#define I915_USERPTR_UNSYNCHRONIZED 0x80000000
22412261
/**
22422262
* @handle: Returned handle for the object.

0 commit comments

Comments
 (0)