| Message ID | 1352979151-9934-17-git-send-email-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Nov 15, 2012 at 11:32:31AM +0000, Chris Wilson wrote: > By exporting the ability to map user address and inserting PTEs > representing their backing pages into the GTT, we can exploit UMA in order > to utilize normal application data as a texture source or even as a > render target (depending upon the capabilities of the chipset). This has > a number of uses, with zero-copy downloads to the GPU and efficient > readback making the intermixed streaming of CPU and GPU operations > fairly efficient. This ability has many widespread implications from > faster rendering of client-side software rasterisers (chromium), > mitigation of stalls due to read back (firefox) and to faster pipelining > of texture data (such as pixel buffer objects in GL). > > v2: Compile with CONFIG_MMU_NOTIFIER > v3: We can sleep while performing invalidate-range, which we can utilise > to drop our page references prior to the kernel manipulating the vma > (for either discard or cloning) and so protect normal users. > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Quick review: - I don't like the internal byte offset we add/substract for relocations. mmap works on page boundaries, imo gpu mmap should do the same. - The mmu_notifier_register stuff looks ridiculously expensive - does it matter or do we need some sort of per-mm rbtree to keep track of our own gtt windows into that mm through the different bos? - select CONFIG_MMU_NOTIFIER? Especially since the mm handling differs quite a bit, I don't really like to keep the other code around. - I admit I haven't checked the refcounting/lifetime rules around holding pointers to random mms and how the mmu_notifier affects that. Since we're not core mm hackers I think a quick comment in the code is in order how that works. - gem api review in the view of these strange new objects - e.g. I think disallowing flink and dma_buf export would make sense here. I also think that tiling doesn't make much sense, at least if userspace doesn't want it's memory to get swizzled once a while ... - api tests in i-g-t both for anything the above api review brings up, but also for resource lifetime things (e.g. killing a process while a userptr is still busy on the gpu). Also some tests whether evil/buggy userspace gets the proper sigbus/-EINVAL if it unmaps a section of it's mm which is used by a userptr. - I think the locking needs some clarification ... For lifting the root-only flag I think only some form of memory/swap test would be required. Actually this would be good anyway, since then lockdep can do the deadlock potential review for me ;-) Cheers, Daniel > --- > drivers/gpu/drm/i915/Makefile | 1 + > drivers/gpu/drm/i915/i915_dma.c | 1 + > drivers/gpu/drm/i915/i915_drv.h | 22 +++ > drivers/gpu/drm/i915/i915_gem.c | 11 +- > drivers/gpu/drm/i915/i915_gem_userptr.c | 320 +++++++++++++++++++++++++++++++ > include/uapi/drm/i915_drm.h | 15 ++ > 6 files changed, 366 insertions(+), 4 deletions(-) > create mode 100644 drivers/gpu/drm/i915/i915_gem_userptr.c > > diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile > index 0f2c549..754d665 100644 > --- a/drivers/gpu/drm/i915/Makefile > +++ b/drivers/gpu/drm/i915/Makefile > @@ -14,6 +14,7 @@ i915-y := i915_drv.o i915_dma.o i915_irq.o \ > i915_gem_gtt.o \ > i915_gem_stolen.o \ > i915_gem_tiling.o \ > + i915_gem_userptr.o \ > i915_sysfs.o \ > i915_trace_points.o \ > intel_display.o \ > diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c > index e347a20..7c025ae 100644 > --- a/drivers/gpu/drm/i915/i915_dma.c > +++ b/drivers/gpu/drm/i915/i915_dma.c > @@ -1892,6 +1892,7 @@ struct drm_ioctl_desc i915_ioctls[] = { > DRM_IOCTL_DEF_DRV(I915_GEM_CONTEXT_CREATE, i915_gem_context_create_ioctl, DRM_UNLOCKED), > DRM_IOCTL_DEF_DRV(I915_GEM_CONTEXT_DESTROY, i915_gem_context_destroy_ioctl, DRM_UNLOCKED), > DRM_IOCTL_DEF_DRV(I915_REG_READ, i915_reg_read_ioctl, DRM_UNLOCKED), > + DRM_IOCTL_DEF_DRV(I915_GEM_USERPTR, i915_gem_userptr_ioctl, DRM_UNLOCKED), > }; > > int i915_max_ioctl = DRM_ARRAY_SIZE(i915_ioctls); > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index 930270c..c86ba1d 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -40,6 +40,7 @@ > #include <linux/backlight.h> > #include <linux/intel-iommu.h> > #include <linux/kref.h> > +#include <linux/mmu_notifier.h> > > /* General customization: > */ > @@ -954,6 +955,7 @@ struct drm_i915_gem_object_ops { > */ > int (*get_pages)(struct drm_i915_gem_object *); > void (*put_pages)(struct drm_i915_gem_object *); > + void (*release)(struct drm_i915_gem_object *); > }; > > struct drm_i915_gem_object { > @@ -1099,6 +1101,23 @@ struct drm_i915_gem_object { > atomic_t pending_flip; > }; > > +struct i915_gem_userptr_object { > + struct drm_i915_gem_object gem; > + uintptr_t user_ptr; > + size_t user_size; > + int read_only; > + > + struct mm_struct *mm; > +#if defined(CONFIG_MMU_NOTIFIER) > + struct mmu_notifier mn; > +#endif > +}; > + > +union drm_i915_gem_objects { > + struct drm_i915_gem_object base; > + struct i915_gem_userptr_object userptr; > +}; > + > inline static bool i915_gem_object_is_prime(struct drm_i915_gem_object *obj) > { > return obj->base.import_attach != NULL; > @@ -1364,6 +1383,8 @@ int i915_gem_entervt_ioctl(struct drm_device *dev, void *data, > struct drm_file *file_priv); > int i915_gem_leavevt_ioctl(struct drm_device *dev, void *data, > struct drm_file *file_priv); > +int i915_gem_userptr_ioctl(struct drm_device *dev, void *data, > + struct drm_file *file); > int i915_gem_set_tiling(struct drm_device *dev, void *data, > struct drm_file *file_priv); > int i915_gem_get_tiling(struct drm_device *dev, void *data, > @@ -1416,6 +1437,7 @@ static inline void i915_gem_object_unpin_pages(struct drm_i915_gem_object *obj) > BUG_ON(obj->pages_pin_count == 0); > obj->pages_pin_count--; > } > +int i915_gem_object_put_pages(struct drm_i915_gem_object *obj); > > int __must_check i915_mutex_lock_interruptible(struct drm_device *dev); > int i915_gem_object_sync(struct drm_i915_gem_object *obj, > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c > index 0bb5f86..c4731e2 100644 > --- a/drivers/gpu/drm/i915/i915_gem.c > +++ b/drivers/gpu/drm/i915/i915_gem.c > @@ -1802,7 +1802,7 @@ i915_gem_object_put_pages_gtt(struct drm_i915_gem_object *obj) > kfree(obj->pages); > } > > -static int > +int > i915_gem_object_put_pages(struct drm_i915_gem_object *obj) > { > const struct drm_i915_gem_object_ops *ops = obj->ops; > @@ -2571,9 +2571,9 @@ i915_gem_object_unbind(struct drm_i915_gem_object *obj) > /* Avoid an unnecessary call to unbind on rebind. */ > obj->map_and_fenceable = true; > > + obj->gtt_offset -= obj->gtt_space->start; > drm_mm_put_block(obj->gtt_space); > obj->gtt_space = NULL; > - obj->gtt_offset = 0; > > return 0; > } > @@ -3089,7 +3089,7 @@ i915_gem_object_bind_to_gtt(struct drm_i915_gem_object *obj, > list_move_tail(&obj->gtt_list, &dev_priv->mm.bound_list); > list_add_tail(&obj->mm_list, &dev_priv->mm.inactive_list); > > - obj->gtt_offset = obj->gtt_space->start; > + obj->gtt_offset += obj->gtt_space->start; > > fenceable = > obj->gtt_space->size == fence_size && > @@ -3882,6 +3882,9 @@ void i915_gem_free_object(struct drm_gem_object *gem_obj) > if (obj->base.import_attach) > drm_prime_gem_destroy(&obj->base, NULL); > > + if (obj->ops->release) > + obj->ops->release(obj); > + > drm_gem_object_release(&obj->base); > i915_gem_info_remove_obj(dev_priv, obj->base.size); > > @@ -4209,7 +4212,7 @@ i915_gem_load(struct drm_device *dev) > > dev_priv->slab = > kmem_cache_create("i915_gem_object", > - sizeof(struct drm_i915_gem_object), 0, > + sizeof(union drm_i915_gem_objects), 0, > SLAB_HWCACHE_ALIGN, > NULL); > > diff --git a/drivers/gpu/drm/i915/i915_gem_userptr.c b/drivers/gpu/drm/i915/i915_gem_userptr.c > new file mode 100644 > index 0000000..5545181 > --- /dev/null > +++ b/drivers/gpu/drm/i915/i915_gem_userptr.c > @@ -0,0 +1,320 @@ > +/* > + * Copyright © 2012 Intel Corporation > + * > + * Permission is hereby granted, free of charge, to any person obtaining a > + * copy of this software and associated documentation files (the "Software"), > + * to deal in the Software without restriction, including without limitation > + * the rights to use, copy, modify, merge, publish, distribute, sublicense, > + * and/or sell copies of the Software, and to permit persons to whom the > + * Software is furnished to do so, subject to the following conditions: > + * > + * The above copyright notice and this permission notice (including the next > + * paragraph) shall be included in all copies or substantial portions of the > + * Software. > + * > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL > + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING > + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS > + * IN THE SOFTWARE. > + * > + */ > + > +#include "drmP.h" > +#include "i915_drm.h" > +#include "i915_drv.h" > +#include "i915_trace.h" > +#include "intel_drv.h" > +#include <linux/mmu_notifier.h> > +#include <linux/swap.h> > + > +static struct i915_gem_userptr_object *to_userptr_object(struct drm_i915_gem_object *obj) > +{ > + return container_of(obj, struct i915_gem_userptr_object, gem); > +} > + > +#if defined(CONFIG_MMU_NOTIFIER) > +static void i915_gem_userptr_mn_invalidate_range_start(struct mmu_notifier *mn, > + struct mm_struct *mm, > + unsigned long start, > + unsigned long end) > +{ > + struct i915_gem_userptr_object *vmap; > + struct drm_device *dev; > + > + /* XXX race between obj unref and mmu notifier? */ > + vmap = container_of(mn, struct i915_gem_userptr_object, mn); > + BUG_ON(vmap->mm != mm); > + > + dev = vmap->gem.base.dev; > + mutex_lock(&dev->struct_mutex); > + if (vmap->gem.gtt_space) { > + struct drm_i915_private *dev_priv = dev->dev_private; > + bool was_interruptible; > + int ret; > + > + was_interruptible = dev_priv->mm.interruptible; > + dev_priv->mm.interruptible = false; > + > + ret = i915_gem_object_unbind(&vmap->gem); > + BUG_ON(ret != -EIO); > + > + dev_priv->mm.interruptible = was_interruptible; > + } > + > + BUG_ON(i915_gem_object_put_pages(&vmap->gem)); > + mutex_unlock(&dev->struct_mutex); > +} > + > +static void i915_gem_userptr_mn_release(struct mmu_notifier *mn, > + struct mm_struct *mm) > +{ > + struct i915_gem_userptr_object *vmap; > + > + vmap = container_of(mn, struct i915_gem_userptr_object, mn); > + BUG_ON(vmap->mm != mm); > + vmap->mm = NULL; > + > + /* XXX Schedule an eventual unbind? E.g. hook into require request? > + * However, locking will be complicated. > + */ > +} > + > +static const struct mmu_notifier_ops i915_gem_userptr_notifier = { > + .invalidate_range_start = i915_gem_userptr_mn_invalidate_range_start, > + .release = i915_gem_userptr_mn_release, > +}; > + > +static void > +i915_gem_userptr_release__mmu_notifier(struct i915_gem_userptr_object *vmap) > +{ > + if (vmap->mm) { > + mmu_notifier_unregister(&vmap->mn, vmap->mm); > + BUG_ON(vmap->mm); > + } > +} > + > +static int > +i915_gem_userptr_init__mmu_notifier(struct i915_gem_userptr_object *vmap) > +{ > + vmap->mn.ops = &i915_gem_userptr_notifier; > + return mmu_notifier_register(&vmap->mn, vmap->mm); > +} > + > +#else > + > +static void > +i915_gem_userptr_release__mmu_notifier(struct i915_gem_userptr_object *vmap) > +{ > +} > + > +static int > +i915_gem_userptr_init__mmu_notifier(struct i915_gem_userptr_object *vmap) > +{ > + if (!capable(CAP_SYS_ADMIN)) > + return -EPERM; > + > + return 0; > +} > +#endif > + > +static int > +i915_gem_userptr_get_pages(struct drm_i915_gem_object *obj) > +{ > + struct i915_gem_userptr_object *vmap = to_userptr_object(obj); > + int num_pages = obj->base.size >> PAGE_SHIFT; > + struct sg_table *st; > + struct scatterlist *sg; > + struct page **pvec; > + int n, pinned, ret; > + > + if (vmap->mm == NULL) > + return -EFAULT; > + > + if (!access_ok(vmap->read_only ? VERIFY_READ : VERIFY_WRITE, > + (char __user *)vmap->user_ptr, vmap->user_size)) > + return -EFAULT; > + > + /* If userspace should engineer that these pages are replaced in > + * the vma between us binding this page into the GTT and completion > + * of rendering... Their loss. If they change the mapping of their > + * pages they need to create a new bo to point to the new vma. > + * > + * However, that still leaves open the possibility of the vma > + * being copied upon fork. Which falls under the same userspace > + * synchronisation issue as a regular bo, except that this time > + * the process may not be expecting that a particular piece of > + * memory is tied to the GPU. > + * > + * Fortunately, we can hook into the mmu_notifier in order to > + * discard the page references prior to anything nasty happening > + * to the vma (discard or cloning) which should prevent the more > + * egregious cases from causing harm. > + */ > + > + pvec = kmalloc(num_pages*sizeof(struct page *), > + GFP_KERNEL | __GFP_NOWARN | __GFP_NORETRY); > + if (pvec == NULL) { > + pvec = drm_malloc_ab(num_pages, sizeof(struct page *)); > + if (pvec == NULL) > + return -ENOMEM; > + } > + > + pinned = 0; > + if (vmap->mm == current->mm) > + pinned = __get_user_pages_fast(vmap->user_ptr, num_pages, > + !vmap->read_only, pvec); > + if (pinned < num_pages) { > + struct mm_struct *mm = vmap->mm; > + ret = 0; > + mutex_unlock(&obj->base.dev->struct_mutex); > + down_read(&mm->mmap_sem); > + if (vmap->mm != NULL) > + ret = get_user_pages(current, mm, > + vmap->user_ptr + (pinned << PAGE_SHIFT), > + num_pages - pinned, > + !vmap->read_only, 0, > + pvec + pinned, > + NULL); > + up_read(&mm->mmap_sem); > + mutex_lock(&obj->base.dev->struct_mutex); > + if (ret > 0) > + pinned += ret; > + > + if (obj->pages || pinned < num_pages) { > + ret = obj->pages ? 0 : -EFAULT; > + goto cleanup_pinned; > + } > + } > + > + st = kmalloc(sizeof(*st), GFP_KERNEL); > + if (st == NULL) { > + ret = -ENOMEM; > + goto cleanup_pinned; > + } > + > + if (sg_alloc_table(st, num_pages, GFP_KERNEL)) { > + ret = -ENOMEM; > + goto cleanup_st; > + } > + > + for_each_sg(st->sgl, sg, num_pages, n) > + sg_set_page(sg, pvec[n], PAGE_SIZE, 0); > + drm_free_large(pvec); > + > + obj->pages = st; > + return 0; > + > +cleanup_st: > + kfree(st); > +cleanup_pinned: > + release_pages(pvec, pinned, 0); > + drm_free_large(pvec); > + return ret; > +} > + > +static void > +i915_gem_userptr_put_pages(struct drm_i915_gem_object *obj) > +{ > + struct scatterlist *sg; > + int i; > + > + if (obj->madv != I915_MADV_WILLNEED) > + obj->dirty = 0; > + > + for_each_sg(obj->pages->sgl, sg, obj->pages->nents, i) { > + struct page *page = sg_page(sg); > + > + if (obj->dirty) > + set_page_dirty(page); > + > + mark_page_accessed(page); > + page_cache_release(page); > + } > + obj->dirty = 0; > + > + sg_free_table(obj->pages); > + kfree(obj->pages); > +} > + > +static void > +i915_gem_userptr_release(struct drm_i915_gem_object *obj) > +{ > + struct i915_gem_userptr_object *vmap = to_userptr_object(obj); > + > + i915_gem_userptr_release__mmu_notifier(vmap); > +} > + > +static const struct drm_i915_gem_object_ops i915_gem_userptr_ops = { > + .get_pages = i915_gem_userptr_get_pages, > + .put_pages = i915_gem_userptr_put_pages, > + .release = i915_gem_userptr_release, > +}; > + > +/** > + * Creates a new mm object that wraps some user memory. > + */ > +int > +i915_gem_userptr_ioctl(struct drm_device *dev, void *data, struct drm_file *file) > +{ > + struct drm_i915_private *dev_priv = dev->dev_private; > + struct drm_i915_gem_userptr *args = data; > + struct i915_gem_userptr_object *obj; > + loff_t first_data_page, last_data_page; > + int num_pages; > + int ret; > + u32 handle; > + > + if (args->flags & ~(I915_USERPTR_READ_ONLY)) > + return -EINVAL; > + > + first_data_page = args->user_ptr / PAGE_SIZE; > + last_data_page = (args->user_ptr + args->user_size - 1) / PAGE_SIZE; > + num_pages = last_data_page - first_data_page + 1; > + if (num_pages * PAGE_SIZE > dev_priv->mm.gtt_total) > + return -E2BIG; > + > + ret = fault_in_multipages_readable((char __user *)(uintptr_t)args->user_ptr, > + args->user_size); > + if (ret) > + return ret; > + > + /* Allocate the new object */ > + obj = i915_gem_object_alloc(dev); > + if (obj == NULL) > + return -ENOMEM; > + > + if (drm_gem_private_object_init(dev, &obj->gem.base, > + num_pages * PAGE_SIZE)) { > + i915_gem_object_free(&obj->gem); > + return -ENOMEM; > + } > + > + i915_gem_object_init(&obj->gem, &i915_gem_userptr_ops); > + obj->gem.cache_level = I915_CACHE_LLC_MLC; > + > + obj->gem.gtt_offset = offset_in_page(args->user_ptr); > + obj->user_ptr = args->user_ptr; > + obj->user_size = args->user_size; > + obj->read_only = args->flags & I915_USERPTR_READ_ONLY; > + > + /* And keep a pointer to the current->mm for resolving the user pages > + * at binding. This means that we need to hook into the mmu_notifier > + * in order to detect if the mmu is destroyed. > + */ > + obj->mm = current->mm; > + ret = i915_gem_userptr_init__mmu_notifier(obj); > + if (ret) > + return ret; > + > + ret = drm_gem_handle_create(file, &obj->gem.base, &handle); > + /* drop reference from allocate - handle holds it now */ > + drm_gem_object_unreference(&obj->gem.base); > + if (ret) > + return ret; > + > + args->handle = handle; > + return 0; > +} > diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h > index b746a3c..067b98e 100644 > --- a/include/uapi/drm/i915_drm.h > +++ b/include/uapi/drm/i915_drm.h > @@ -198,6 +198,7 @@ typedef struct _drm_i915_sarea { > #define DRM_I915_GEM_SET_CACHING 0x2f > #define DRM_I915_GEM_GET_CACHING 0x30 > #define DRM_I915_REG_READ 0x31 > +#define DRM_I915_GEM_USERPTR 0x32 > > #define DRM_IOCTL_I915_INIT DRM_IOW( DRM_COMMAND_BASE + DRM_I915_INIT, drm_i915_init_t) > #define DRM_IOCTL_I915_FLUSH DRM_IO ( DRM_COMMAND_BASE + DRM_I915_FLUSH) > @@ -247,6 +248,7 @@ typedef struct _drm_i915_sarea { > #define DRM_IOCTL_I915_GEM_CONTEXT_CREATE DRM_IOWR (DRM_COMMAND_BASE + DRM_I915_GEM_CONTEXT_CREATE, struct drm_i915_gem_context_create) > #define DRM_IOCTL_I915_GEM_CONTEXT_DESTROY DRM_IOW (DRM_COMMAND_BASE + DRM_I915_GEM_CONTEXT_DESTROY, struct drm_i915_gem_context_destroy) > #define DRM_IOCTL_I915_REG_READ DRM_IOWR (DRM_COMMAND_BASE + DRM_I915_REG_READ, struct drm_i915_reg_read) > +#define DRM_IOCTL_I915_GEM_USERPTR DRM_IOWR (DRM_COMMAND_BASE + DRM_I915_GEM_USERPTR, struct drm_i915_gem_userptr) > > /* Allow drivers to submit batchbuffers directly to hardware, relying > * on the security mechanisms provided by hardware. > @@ -950,4 +952,17 @@ struct drm_i915_reg_read { > __u64 offset; > __u64 val; /* Return value */ > }; > + > +struct drm_i915_gem_userptr { > + __u64 user_ptr; > + __u32 user_size; > + __u32 flags; > +#define I915_USERPTR_READ_ONLY 0x1 > + /** > + * Returned handle for the object. > + * > + * Object handles are nonzero. > + */ > + __u32 handle; > +}; > #endif /* _UAPI_I915_DRM_H_ */ > -- > 1.7.10.4 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Sat, 1 Dec 2012 00:06:43 +0100, Daniel Vetter <daniel@ffwll.ch> wrote: > On Thu, Nov 15, 2012 at 11:32:31AM +0000, Chris Wilson wrote: > > By exporting the ability to map user address and inserting PTEs > > representing their backing pages into the GTT, we can exploit UMA in order > > to utilize normal application data as a texture source or even as a > > render target (depending upon the capabilities of the chipset). This has > > a number of uses, with zero-copy downloads to the GPU and efficient > > readback making the intermixed streaming of CPU and GPU operations > > fairly efficient. This ability has many widespread implications from > > faster rendering of client-side software rasterisers (chromium), > > mitigation of stalls due to read back (firefox) and to faster pipelining > > of texture data (such as pixel buffer objects in GL). > - gem api review in the view of these strange new objects - e.g. I think > disallowing flink and dma_buf export would make sense here. I also > think that tiling doesn't make much sense, at least if userspace doesn't > want it's memory to get swizzled once a while ... Over the course of the years this has been sitting around, I've changed my mind. Originally, I thought it would be wise to prevent userspace from trying to do things like pwrite, mmap-gtt, or set tiling. However, there is nothing fundamental in the hardware that prevents userspace from doing so, so long as it is fully cogniscient of what it is doing (which it has to be anyway). Similarly for flink/dma-buf, though I have not thought it through and mmu-notifier should be sufficient to teardown cross-process attachments - but it would be useful (cf SHM). The handwaving here is why it originally was root-only, and why we are looking at the mmu-notifiers to shootdown the vma as it is copied between processes. mmu_notifier is seemingly expensive, and I note is only compiled in for one of my distro kernels - it is not a burden I want to impose on my machines, but zero-copy uploads is too important to give up. -Chris
On Sat, 1 Dec 2012 00:06:43 +0100, Daniel Vetter <daniel@ffwll.ch> wrote: > Quick review: > - I don't like the internal byte offset we add/substract for relocations. > mmap works on page boundaries, imo gpu mmap should do the same. However, the necessity is that I work on unaligned boundaries since I have no control over how the clients allocate the memory to be mapped, or which bytes within that memory are associated with a particular pixmap. The XShm API is such that not even the ShmSegment is passed down to the driver. That vies with the lack of explicit synchronisation points as being the worst elements of its design. -Chris
On Sat, Dec 1, 2012 at 12:25 PM, Chris Wilson <chris@chris-wilson.co.uk> wrote: > On Sat, 1 Dec 2012 00:06:43 +0100, Daniel Vetter <daniel@ffwll.ch> wrote: >> Quick review: >> - I don't like the internal byte offset we add/substract for relocations. >> mmap works on page boundaries, imo gpu mmap should do the same. > > However, the necessity is that I work on unaligned boundaries since I > have no control over how the clients allocate the memory to be mapped, > or which bytes within that memory are associated with a particular > pixmap. The XShm API is such that not even the ShmSegment is passed down > to the driver. That vies with the lack of explicit synchronisation > points as being the worst elements of its design. Well, I just want to move the page_offset computation from the kernel to userspace. I know, this requires that you store that offset in userspace somewhere, which requires a new field right next to the existing u32 handle. -Daniel
diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile index 0f2c549..754d665 100644 --- a/drivers/gpu/drm/i915/Makefile +++ b/drivers/gpu/drm/i915/Makefile @@ -14,6 +14,7 @@ i915-y := i915_drv.o i915_dma.o i915_irq.o \ i915_gem_gtt.o \ i915_gem_stolen.o \ i915_gem_tiling.o \ + i915_gem_userptr.o \ i915_sysfs.o \ i915_trace_points.o \ intel_display.o \ diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c index e347a20..7c025ae 100644 --- a/drivers/gpu/drm/i915/i915_dma.c +++ b/drivers/gpu/drm/i915/i915_dma.c @@ -1892,6 +1892,7 @@ struct drm_ioctl_desc i915_ioctls[] = { DRM_IOCTL_DEF_DRV(I915_GEM_CONTEXT_CREATE, i915_gem_context_create_ioctl, DRM_UNLOCKED), DRM_IOCTL_DEF_DRV(I915_GEM_CONTEXT_DESTROY, i915_gem_context_destroy_ioctl, DRM_UNLOCKED), DRM_IOCTL_DEF_DRV(I915_REG_READ, i915_reg_read_ioctl, DRM_UNLOCKED), + DRM_IOCTL_DEF_DRV(I915_GEM_USERPTR, i915_gem_userptr_ioctl, DRM_UNLOCKED), }; int i915_max_ioctl = DRM_ARRAY_SIZE(i915_ioctls); diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 930270c..c86ba1d 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -40,6 +40,7 @@ #include <linux/backlight.h> #include <linux/intel-iommu.h> #include <linux/kref.h> +#include <linux/mmu_notifier.h> /* General customization: */ @@ -954,6 +955,7 @@ struct drm_i915_gem_object_ops { */ int (*get_pages)(struct drm_i915_gem_object *); void (*put_pages)(struct drm_i915_gem_object *); + void (*release)(struct drm_i915_gem_object *); }; struct drm_i915_gem_object { @@ -1099,6 +1101,23 @@ struct drm_i915_gem_object { atomic_t pending_flip; }; +struct i915_gem_userptr_object { + struct drm_i915_gem_object gem; + uintptr_t user_ptr; + size_t user_size; + int read_only; + + struct mm_struct *mm; +#if defined(CONFIG_MMU_NOTIFIER) + struct mmu_notifier mn; +#endif +}; + +union drm_i915_gem_objects { + struct drm_i915_gem_object base; + struct i915_gem_userptr_object userptr; +}; + inline static bool i915_gem_object_is_prime(struct drm_i915_gem_object *obj) { return obj->base.import_attach != NULL; @@ -1364,6 +1383,8 @@ int i915_gem_entervt_ioctl(struct drm_device *dev, void *data, struct drm_file *file_priv); int i915_gem_leavevt_ioctl(struct drm_device *dev, void *data, struct drm_file *file_priv); +int i915_gem_userptr_ioctl(struct drm_device *dev, void *data, + struct drm_file *file); int i915_gem_set_tiling(struct drm_device *dev, void *data, struct drm_file *file_priv); int i915_gem_get_tiling(struct drm_device *dev, void *data, @@ -1416,6 +1437,7 @@ static inline void i915_gem_object_unpin_pages(struct drm_i915_gem_object *obj) BUG_ON(obj->pages_pin_count == 0); obj->pages_pin_count--; } +int i915_gem_object_put_pages(struct drm_i915_gem_object *obj); int __must_check i915_mutex_lock_interruptible(struct drm_device *dev); int i915_gem_object_sync(struct drm_i915_gem_object *obj, diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 0bb5f86..c4731e2 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -1802,7 +1802,7 @@ i915_gem_object_put_pages_gtt(struct drm_i915_gem_object *obj) kfree(obj->pages); } -static int +int i915_gem_object_put_pages(struct drm_i915_gem_object *obj) { const struct drm_i915_gem_object_ops *ops = obj->ops; @@ -2571,9 +2571,9 @@ i915_gem_object_unbind(struct drm_i915_gem_object *obj) /* Avoid an unnecessary call to unbind on rebind. */ obj->map_and_fenceable = true; + obj->gtt_offset -= obj->gtt_space->start; drm_mm_put_block(obj->gtt_space); obj->gtt_space = NULL; - obj->gtt_offset = 0; return 0; } @@ -3089,7 +3089,7 @@ i915_gem_object_bind_to_gtt(struct drm_i915_gem_object *obj, list_move_tail(&obj->gtt_list, &dev_priv->mm.bound_list); list_add_tail(&obj->mm_list, &dev_priv->mm.inactive_list); - obj->gtt_offset = obj->gtt_space->start; + obj->gtt_offset += obj->gtt_space->start; fenceable = obj->gtt_space->size == fence_size && @@ -3882,6 +3882,9 @@ void i915_gem_free_object(struct drm_gem_object *gem_obj) if (obj->base.import_attach) drm_prime_gem_destroy(&obj->base, NULL); + if (obj->ops->release) + obj->ops->release(obj); + drm_gem_object_release(&obj->base); i915_gem_info_remove_obj(dev_priv, obj->base.size); @@ -4209,7 +4212,7 @@ i915_gem_load(struct drm_device *dev) dev_priv->slab = kmem_cache_create("i915_gem_object", - sizeof(struct drm_i915_gem_object), 0, + sizeof(union drm_i915_gem_objects), 0, SLAB_HWCACHE_ALIGN, NULL); diff --git a/drivers/gpu/drm/i915/i915_gem_userptr.c b/drivers/gpu/drm/i915/i915_gem_userptr.c new file mode 100644 index 0000000..5545181 --- /dev/null +++ b/drivers/gpu/drm/i915/i915_gem_userptr.c @@ -0,0 +1,320 @@ +/* + * Copyright © 2012 Intel Corporation + * + * Permission is hereby granted, free of charge, to any person obtaining a + * copy of this software and associated documentation files (the "Software"), + * to deal in the Software without restriction, including without limitation + * the rights to use, copy, modify, merge, publish, distribute, sublicense, + * and/or sell copies of the Software, and to permit persons to whom the + * Software is furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice (including the next + * paragraph) shall be included in all copies or substantial portions of the + * Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS + * IN THE SOFTWARE. + * + */ + +#include "drmP.h" +#include "i915_drm.h" +#include "i915_drv.h" +#include "i915_trace.h" +#include "intel_drv.h" +#include <linux/mmu_notifier.h> +#include <linux/swap.h> + +static struct i915_gem_userptr_object *to_userptr_object(struct drm_i915_gem_object *obj) +{ + return container_of(obj, struct i915_gem_userptr_object, gem); +} + +#if defined(CONFIG_MMU_NOTIFIER) +static void i915_gem_userptr_mn_invalidate_range_start(struct mmu_notifier *mn, + struct mm_struct *mm, + unsigned long start, + unsigned long end) +{ + struct i915_gem_userptr_object *vmap; + struct drm_device *dev; + + /* XXX race between obj unref and mmu notifier? */ + vmap = container_of(mn, struct i915_gem_userptr_object, mn); + BUG_ON(vmap->mm != mm); + + dev = vmap->gem.base.dev; + mutex_lock(&dev->struct_mutex); + if (vmap->gem.gtt_space) { + struct drm_i915_private *dev_priv = dev->dev_private; + bool was_interruptible; + int ret; + + was_interruptible = dev_priv->mm.interruptible; + dev_priv->mm.interruptible = false; + + ret = i915_gem_object_unbind(&vmap->gem); + BUG_ON(ret != -EIO); + + dev_priv->mm.interruptible = was_interruptible; + } + + BUG_ON(i915_gem_object_put_pages(&vmap->gem)); + mutex_unlock(&dev->struct_mutex); +} + +static void i915_gem_userptr_mn_release(struct mmu_notifier *mn, + struct mm_struct *mm) +{ + struct i915_gem_userptr_object *vmap; + + vmap = container_of(mn, struct i915_gem_userptr_object, mn); + BUG_ON(vmap->mm != mm); + vmap->mm = NULL; + + /* XXX Schedule an eventual unbind? E.g. hook into require request? + * However, locking will be complicated. + */ +} + +static const struct mmu_notifier_ops i915_gem_userptr_notifier = { + .invalidate_range_start = i915_gem_userptr_mn_invalidate_range_start, + .release = i915_gem_userptr_mn_release, +}; + +static void +i915_gem_userptr_release__mmu_notifier(struct i915_gem_userptr_object *vmap) +{ + if (vmap->mm) { + mmu_notifier_unregister(&vmap->mn, vmap->mm); + BUG_ON(vmap->mm); + } +} + +static int +i915_gem_userptr_init__mmu_notifier(struct i915_gem_userptr_object *vmap) +{ + vmap->mn.ops = &i915_gem_userptr_notifier; + return mmu_notifier_register(&vmap->mn, vmap->mm); +} + +#else + +static void +i915_gem_userptr_release__mmu_notifier(struct i915_gem_userptr_object *vmap) +{ +} + +static int +i915_gem_userptr_init__mmu_notifier(struct i915_gem_userptr_object *vmap) +{ + if (!capable(CAP_SYS_ADMIN)) + return -EPERM; + + return 0; +} +#endif + +static int +i915_gem_userptr_get_pages(struct drm_i915_gem_object *obj) +{ + struct i915_gem_userptr_object *vmap = to_userptr_object(obj); + int num_pages = obj->base.size >> PAGE_SHIFT; + struct sg_table *st; + struct scatterlist *sg; + struct page **pvec; + int n, pinned, ret; + + if (vmap->mm == NULL) + return -EFAULT; + + if (!access_ok(vmap->read_only ? VERIFY_READ : VERIFY_WRITE, + (char __user *)vmap->user_ptr, vmap->user_size)) + return -EFAULT; + + /* If userspace should engineer that these pages are replaced in + * the vma between us binding this page into the GTT and completion + * of rendering... Their loss. If they change the mapping of their + * pages they need to create a new bo to point to the new vma. + * + * However, that still leaves open the possibility of the vma + * being copied upon fork. Which falls under the same userspace + * synchronisation issue as a regular bo, except that this time + * the process may not be expecting that a particular piece of + * memory is tied to the GPU. + * + * Fortunately, we can hook into the mmu_notifier in order to + * discard the page references prior to anything nasty happening + * to the vma (discard or cloning) which should prevent the more + * egregious cases from causing harm. + */ + + pvec = kmalloc(num_pages*sizeof(struct page *), + GFP_KERNEL | __GFP_NOWARN | __GFP_NORETRY); + if (pvec == NULL) { + pvec = drm_malloc_ab(num_pages, sizeof(struct page *)); + if (pvec == NULL) + return -ENOMEM; + } + + pinned = 0; + if (vmap->mm == current->mm) + pinned = __get_user_pages_fast(vmap->user_ptr, num_pages, + !vmap->read_only, pvec); + if (pinned < num_pages) { + struct mm_struct *mm = vmap->mm; + ret = 0; + mutex_unlock(&obj->base.dev->struct_mutex); + down_read(&mm->mmap_sem); + if (vmap->mm != NULL) + ret = get_user_pages(current, mm, + vmap->user_ptr + (pinned << PAGE_SHIFT), + num_pages - pinned, + !vmap->read_only, 0, + pvec + pinned, + NULL); + up_read(&mm->mmap_sem); + mutex_lock(&obj->base.dev->struct_mutex); + if (ret > 0) + pinned += ret; + + if (obj->pages || pinned < num_pages) { + ret = obj->pages ? 0 : -EFAULT; + goto cleanup_pinned; + } + } + + st = kmalloc(sizeof(*st), GFP_KERNEL); + if (st == NULL) { + ret = -ENOMEM; + goto cleanup_pinned; + } + + if (sg_alloc_table(st, num_pages, GFP_KERNEL)) { + ret = -ENOMEM; + goto cleanup_st; + } + + for_each_sg(st->sgl, sg, num_pages, n) + sg_set_page(sg, pvec[n], PAGE_SIZE, 0); + drm_free_large(pvec); + + obj->pages = st; + return 0; + +cleanup_st: + kfree(st); +cleanup_pinned: + release_pages(pvec, pinned, 0); + drm_free_large(pvec); + return ret; +} + +static void +i915_gem_userptr_put_pages(struct drm_i915_gem_object *obj) +{ + struct scatterlist *sg; + int i; + + if (obj->madv != I915_MADV_WILLNEED) + obj->dirty = 0; + + for_each_sg(obj->pages->sgl, sg, obj->pages->nents, i) { + struct page *page = sg_page(sg); + + if (obj->dirty) + set_page_dirty(page); + + mark_page_accessed(page); + page_cache_release(page); + } + obj->dirty = 0; + + sg_free_table(obj->pages); + kfree(obj->pages); +} + +static void +i915_gem_userptr_release(struct drm_i915_gem_object *obj) +{ + struct i915_gem_userptr_object *vmap = to_userptr_object(obj); + + i915_gem_userptr_release__mmu_notifier(vmap); +} + +static const struct drm_i915_gem_object_ops i915_gem_userptr_ops = { + .get_pages = i915_gem_userptr_get_pages, + .put_pages = i915_gem_userptr_put_pages, + .release = i915_gem_userptr_release, +}; + +/** + * Creates a new mm object that wraps some user memory. + */ +int +i915_gem_userptr_ioctl(struct drm_device *dev, void *data, struct drm_file *file) +{ + struct drm_i915_private *dev_priv = dev->dev_private; + struct drm_i915_gem_userptr *args = data; + struct i915_gem_userptr_object *obj; + loff_t first_data_page, last_data_page; + int num_pages; + int ret; + u32 handle; + + if (args->flags & ~(I915_USERPTR_READ_ONLY)) + return -EINVAL; + + first_data_page = args->user_ptr / PAGE_SIZE; + last_data_page = (args->user_ptr + args->user_size - 1) / PAGE_SIZE; + num_pages = last_data_page - first_data_page + 1; + if (num_pages * PAGE_SIZE > dev_priv->mm.gtt_total) + return -E2BIG; + + ret = fault_in_multipages_readable((char __user *)(uintptr_t)args->user_ptr, + args->user_size); + if (ret) + return ret; + + /* Allocate the new object */ + obj = i915_gem_object_alloc(dev); + if (obj == NULL) + return -ENOMEM; + + if (drm_gem_private_object_init(dev, &obj->gem.base, + num_pages * PAGE_SIZE)) { + i915_gem_object_free(&obj->gem); + return -ENOMEM; + } + + i915_gem_object_init(&obj->gem, &i915_gem_userptr_ops); + obj->gem.cache_level = I915_CACHE_LLC_MLC; + + obj->gem.gtt_offset = offset_in_page(args->user_ptr); + obj->user_ptr = args->user_ptr; + obj->user_size = args->user_size; + obj->read_only = args->flags & I915_USERPTR_READ_ONLY; + + /* And keep a pointer to the current->mm for resolving the user pages + * at binding. This means that we need to hook into the mmu_notifier + * in order to detect if the mmu is destroyed. + */ + obj->mm = current->mm; + ret = i915_gem_userptr_init__mmu_notifier(obj); + if (ret) + return ret; + + ret = drm_gem_handle_create(file, &obj->gem.base, &handle); + /* drop reference from allocate - handle holds it now */ + drm_gem_object_unreference(&obj->gem.base); + if (ret) + return ret; + + args->handle = handle; + return 0; +} diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h index b746a3c..067b98e 100644 --- a/include/uapi/drm/i915_drm.h +++ b/include/uapi/drm/i915_drm.h @@ -198,6 +198,7 @@ typedef struct _drm_i915_sarea { #define DRM_I915_GEM_SET_CACHING 0x2f #define DRM_I915_GEM_GET_CACHING 0x30 #define DRM_I915_REG_READ 0x31 +#define DRM_I915_GEM_USERPTR 0x32 #define DRM_IOCTL_I915_INIT DRM_IOW( DRM_COMMAND_BASE + DRM_I915_INIT, drm_i915_init_t) #define DRM_IOCTL_I915_FLUSH DRM_IO ( DRM_COMMAND_BASE + DRM_I915_FLUSH) @@ -247,6 +248,7 @@ typedef struct _drm_i915_sarea { #define DRM_IOCTL_I915_GEM_CONTEXT_CREATE DRM_IOWR (DRM_COMMAND_BASE + DRM_I915_GEM_CONTEXT_CREATE, struct drm_i915_gem_context_create) #define DRM_IOCTL_I915_GEM_CONTEXT_DESTROY DRM_IOW (DRM_COMMAND_BASE + DRM_I915_GEM_CONTEXT_DESTROY, struct drm_i915_gem_context_destroy) #define DRM_IOCTL_I915_REG_READ DRM_IOWR (DRM_COMMAND_BASE + DRM_I915_REG_READ, struct drm_i915_reg_read) +#define DRM_IOCTL_I915_GEM_USERPTR DRM_IOWR (DRM_COMMAND_BASE + DRM_I915_GEM_USERPTR, struct drm_i915_gem_userptr) /* Allow drivers to submit batchbuffers directly to hardware, relying * on the security mechanisms provided by hardware. @@ -950,4 +952,17 @@ struct drm_i915_reg_read { __u64 offset; __u64 val; /* Return value */ }; + +struct drm_i915_gem_userptr { + __u64 user_ptr; + __u32 user_size; + __u32 flags; +#define I915_USERPTR_READ_ONLY 0x1 + /** + * Returned handle for the object. + * + * Object handles are nonzero. + */ + __u32 handle; +}; #endif /* _UAPI_I915_DRM_H_ */
By exporting the ability to map user address and inserting PTEs representing their backing pages into the GTT, we can exploit UMA in order to utilize normal application data as a texture source or even as a render target (depending upon the capabilities of the chipset). This has a number of uses, with zero-copy downloads to the GPU and efficient readback making the intermixed streaming of CPU and GPU operations fairly efficient. This ability has many widespread implications from faster rendering of client-side software rasterisers (chromium), mitigation of stalls due to read back (firefox) and to faster pipelining of texture data (such as pixel buffer objects in GL). v2: Compile with CONFIG_MMU_NOTIFIER v3: We can sleep while performing invalidate-range, which we can utilise to drop our page references prior to the kernel manipulating the vma (for either discard or cloning) and so protect normal users. Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> --- drivers/gpu/drm/i915/Makefile | 1 + drivers/gpu/drm/i915/i915_dma.c | 1 + drivers/gpu/drm/i915/i915_drv.h | 22 +++ drivers/gpu/drm/i915/i915_gem.c | 11 +- drivers/gpu/drm/i915/i915_gem_userptr.c | 320 +++++++++++++++++++++++++++++++ include/uapi/drm/i915_drm.h | 15 ++ 6 files changed, 366 insertions(+), 4 deletions(-) create mode 100644 drivers/gpu/drm/i915/i915_gem_userptr.c