| Message ID | 1509528141-4189-2-git-send-email-tina.zhang@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, 2017-11-01 at 17:22 +0800, Tina Zhang wrote: > GEM proxy is a kind of GEM, whose backing physical memory is pinned > and produced by guest VM and is used by host as read only. With GEM > proxy, host is able to access guest physical memory through GEM object > interface. As GEM proxy is such a special kind of GEM, a new flag > I915_GEM_OBJECT_IS_PROXY is introduced to ban host from changing the > backing storage of GEM proxy. > > v2: > - return -ENXIO when pin and map pages of GEM proxy to kernel space. > (Chris) > > Here are the histories of this patch in "Dma-buf support for Gvt-g" > patch-set: > > v14: > - return -ENXIO when gem proxy object is banned by ioctl. > (Chris) (Daniel) > > v13: > - add comments to GEM proxy. (Chris) > - don't ban GEM proxy in i915_gem_sw_finish_ioctl. (Chris) > - check GEM proxy bar after finishing i915_gem_object_wait. (Chris) > - remove GEM proxy bar in i915_gem_madvise_ioctl. > > v6: > - add gem proxy barrier in the following ioctls. (Chris) > i915_gem_set_caching_ioctl > i915_gem_set_domain_ioctl > i915_gem_sw_finish_ioctl > i915_gem_set_tiling_ioctl > i915_gem_madvise_ioctl > > Signed-off-by: Tina Zhang <tina.zhang@intel.com> > Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> > Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch> <SNIP> > @@ -1649,6 +1659,10 @@ i915_gem_sw_finish_ioctl(struct drm_device *dev, void *data, > if (!obj) > return -ENOENT; > > + /* Proxy objects are barred from CPU access, so there is no > + * need to ban sw_finish as it is a nop. > + */ > + > /* Pinned buffers may be scanout, so flush the cache */ > i915_gem_object_flush_if_display(obj); > i915_gem_object_put(obj); > @@ -2614,7 +2628,8 @@ void *i915_gem_object_pin_map(struct drm_i915_gem_object *obj, > void *ptr; > int ret; > > - GEM_BUG_ON(!i915_gem_object_has_struct_page(obj)); > + if (unlikely(!i915_gem_object_has_struct_page(obj))) > + return ERR_PTR(-ENODEV); You should have marked this change in the changelog and then marked the Reviewed-by tags to be valid only to the previous version of this patch. It's not a fair game to claim a patch to be "Reviewed-by" at the current version, when you've made changes that were not agreed upon. So that's some meta-review. Back to the actual review; Which codepath was hitting the GEM_BUG_ON? Wondering if it would be cleaner to avoid the call to this function on that single codepath. Regards, Joonas
> -----Original Message----- > From: intel-gvt-dev [mailto:intel-gvt-dev-bounces@lists.freedesktop.org] On > Behalf Of Joonas Lahtinen > Sent: Monday, November 6, 2017 7:24 PM > To: Zhang, Tina <tina.zhang@intel.com>; zhenyuw@linux.intel.com; Wang, Zhi > A <zhi.a.wang@intel.com>; daniel@ffwll.ch; chris@chris-wilson.co.uk > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>; intel-gfx@lists.freedesktop.org; > intel-gvt-dev@lists.freedesktop.org > Subject: Re: [PATCH v2 1/2] drm/i915: Introduce GEM proxy > > On Wed, 2017-11-01 at 17:22 +0800, Tina Zhang wrote: > > GEM proxy is a kind of GEM, whose backing physical memory is pinned > > and produced by guest VM and is used by host as read only. With GEM > > proxy, host is able to access guest physical memory through GEM object > > interface. As GEM proxy is such a special kind of GEM, a new flag > > I915_GEM_OBJECT_IS_PROXY is introduced to ban host from changing the > > backing storage of GEM proxy. > > > > v2: > > - return -ENXIO when pin and map pages of GEM proxy to kernel space. > > (Chris) > > > > Here are the histories of this patch in "Dma-buf support for Gvt-g" > > patch-set: > > > > v14: > > - return -ENXIO when gem proxy object is banned by ioctl. > > (Chris) (Daniel) > > > > v13: > > - add comments to GEM proxy. (Chris) > > - don't ban GEM proxy in i915_gem_sw_finish_ioctl. (Chris) > > - check GEM proxy bar after finishing i915_gem_object_wait. (Chris) > > - remove GEM proxy bar in i915_gem_madvise_ioctl. > > > > v6: > > - add gem proxy barrier in the following ioctls. (Chris) > > i915_gem_set_caching_ioctl > > i915_gem_set_domain_ioctl > > i915_gem_sw_finish_ioctl > > i915_gem_set_tiling_ioctl > > i915_gem_madvise_ioctl > > > > Signed-off-by: Tina Zhang <tina.zhang@intel.com> > > Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> > > Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> > > Cc: Daniel Vetter <daniel.vetter@ffwll.ch> > > <SNIP> > > > @@ -1649,6 +1659,10 @@ i915_gem_sw_finish_ioctl(struct drm_device > *dev, void *data, > > if (!obj) > > return -ENOENT; > > > > + /* Proxy objects are barred from CPU access, so there is no > > + * need to ban sw_finish as it is a nop. > > + */ > > + > > /* Pinned buffers may be scanout, so flush the cache */ > > i915_gem_object_flush_if_display(obj); > > i915_gem_object_put(obj); > > @@ -2614,7 +2628,8 @@ void *i915_gem_object_pin_map(struct > drm_i915_gem_object *obj, > > void *ptr; > > int ret; > > > > - GEM_BUG_ON(!i915_gem_object_has_struct_page(obj)); > > + if (unlikely(!i915_gem_object_has_struct_page(obj))) > > + return ERR_PTR(-ENODEV); > > You should have marked this change in the changelog and then marked the > Reviewed-by tags to be valid only to the previous version of this patch. > > It's not a fair game to claim a patch to be "Reviewed-by" at the current version, > when you've made changes that were not agreed upon. I thought we were agreed on this :) > > So that's some meta-review. Back to the actual review; > > Which codepath was hitting the GEM_BUG_ON? Wondering if it would be > cleaner to avoid the call to this function on that single codepath. Here is the previously comments: https://lists.freedesktop.org/archives/intel-gvt-dev/2017-October/002278.html Thanks. BR, Tina > > Regards, Joonas > -- > Joonas Lahtinen > Open Source Technology Center > Intel Corporation > _______________________________________________ > intel-gvt-dev mailing list > intel-gvt-dev@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gvt-dev
On Tue, 2017-11-07 at 04:53 +0000, Zhang, Tina wrote: > > -----Original Message----- > > From: intel-gvt-dev [mailto:intel-gvt-dev-bounces@lists.freedesktop.org] On > > Behalf Of Joonas Lahtinen > > Sent: Monday, November 6, 2017 7:24 PM > > To: Zhang, Tina <tina.zhang@intel.com>; zhenyuw@linux.intel.com; Wang, Zhi > > A <zhi.a.wang@intel.com>; daniel@ffwll.ch; chris@chris-wilson.co.uk > > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>; intel-gfx@lists.freedesktop.org; > > intel-gvt-dev@lists.freedesktop.org > > Subject: Re: [PATCH v2 1/2] drm/i915: Introduce GEM proxy > > > > On Wed, 2017-11-01 at 17:22 +0800, Tina Zhang wrote: > > > GEM proxy is a kind of GEM, whose backing physical memory is pinned > > > and produced by guest VM and is used by host as read only. With GEM > > > proxy, host is able to access guest physical memory through GEM object > > > interface. As GEM proxy is such a special kind of GEM, a new flag > > > I915_GEM_OBJECT_IS_PROXY is introduced to ban host from changing the > > > backing storage of GEM proxy. > > > > > > v2: > > > - return -ENXIO when pin and map pages of GEM proxy to kernel space. > > > (Chris) > > > > > > Here are the histories of this patch in "Dma-buf support for Gvt-g" > > > patch-set: > > > > > > v14: > > > - return -ENXIO when gem proxy object is banned by ioctl. > > > (Chris) (Daniel) > > > > > > v13: > > > - add comments to GEM proxy. (Chris) > > > - don't ban GEM proxy in i915_gem_sw_finish_ioctl. (Chris) > > > - check GEM proxy bar after finishing i915_gem_object_wait. (Chris) > > > - remove GEM proxy bar in i915_gem_madvise_ioctl. > > > > > > v6: > > > - add gem proxy barrier in the following ioctls. (Chris) > > > i915_gem_set_caching_ioctl > > > i915_gem_set_domain_ioctl > > > i915_gem_sw_finish_ioctl > > > i915_gem_set_tiling_ioctl > > > i915_gem_madvise_ioctl > > > > > > Signed-off-by: Tina Zhang <tina.zhang@intel.com> > > > Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> > > > Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> > > > Cc: Daniel Vetter <daniel.vetter@ffwll.ch> > > > > <SNIP> > > > > > @@ -1649,6 +1659,10 @@ i915_gem_sw_finish_ioctl(struct drm_device > > > > *dev, void *data, > > > if (!obj) > > > return -ENOENT; > > > > > > + /* Proxy objects are barred from CPU access, so there is no > > > + * need to ban sw_finish as it is a nop. > > > + */ > > > + > > > /* Pinned buffers may be scanout, so flush the cache */ > > > i915_gem_object_flush_if_display(obj); > > > i915_gem_object_put(obj); > > > @@ -2614,7 +2628,8 @@ void *i915_gem_object_pin_map(struct > > > > drm_i915_gem_object *obj, > > > void *ptr; > > > int ret; > > > > > > - GEM_BUG_ON(!i915_gem_object_has_struct_page(obj)); > > > + if (unlikely(!i915_gem_object_has_struct_page(obj))) > > > + return ERR_PTR(-ENODEV); > > > > You should have marked this change in the changelog and then marked the > > Reviewed-by tags to be valid only to the previous version of this patch. > > > > It's not a fair game to claim a patch to be "Reviewed-by" at the current version, > > when you've made changes that were not agreed upon. > > I thought we were agreed on this :) > > > > > So that's some meta-review. Back to the actual review; > > > > Which codepath was hitting the GEM_BUG_ON? Wondering if it would be > > cleaner to avoid the call to this function on that single codepath. > > Here is the previously comments: > https://lists.freedesktop.org/archives/intel-gvt-dev/2017-October/002278.html > Thanks. I never even noticed such an e-mail, so the correct response would've been; Reviewed-by: Joonas #vX Reviewed-by: Chris Where #vX is the version I actually agreed to. Reviewed-by tags are are ones you need to be especially careful about in addition to the Signed-off-bys because they carry special meaning: https://www.kernel.org/doc/html/v4.13/process/submitting-patches.html#r eviewer-s-statement-of-oversight Regards, Joonas
> -----Original Message----- > From: intel-gvt-dev [mailto:intel-gvt-dev-bounces@lists.freedesktop.org] On > Behalf Of Joonas Lahtinen > Sent: Tuesday, November 7, 2017 9:06 PM > To: Zhang, Tina <tina.zhang@intel.com>; zhenyuw@linux.intel.com; Wang, Zhi > A <zhi.a.wang@intel.com>; daniel@ffwll.ch; chris@chris-wilson.co.uk > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>; intel-gfx@lists.freedesktop.org; > intel-gvt-dev@lists.freedesktop.org > Subject: Re: [PATCH v2 1/2] drm/i915: Introduce GEM proxy > > On Tue, 2017-11-07 at 04:53 +0000, Zhang, Tina wrote: > > > -----Original Message----- > > > From: intel-gvt-dev > > > [mailto:intel-gvt-dev-bounces@lists.freedesktop.org] On Behalf Of > > > Joonas Lahtinen > > > Sent: Monday, November 6, 2017 7:24 PM > > > To: Zhang, Tina <tina.zhang@intel.com>; zhenyuw@linux.intel.com; > > > Wang, Zhi A <zhi.a.wang@intel.com>; daniel@ffwll.ch; > > > chris@chris-wilson.co.uk > > > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>; > > > intel-gfx@lists.freedesktop.org; intel-gvt-dev@lists.freedesktop.org > > > Subject: Re: [PATCH v2 1/2] drm/i915: Introduce GEM proxy > > > > > > On Wed, 2017-11-01 at 17:22 +0800, Tina Zhang wrote: > > > > GEM proxy is a kind of GEM, whose backing physical memory is > > > > pinned and produced by guest VM and is used by host as read only. > > > > With GEM proxy, host is able to access guest physical memory > > > > through GEM object interface. As GEM proxy is such a special kind > > > > of GEM, a new flag I915_GEM_OBJECT_IS_PROXY is introduced to ban > > > > host from changing the backing storage of GEM proxy. > > > > > > > > v2: > > > > - return -ENXIO when pin and map pages of GEM proxy to kernel space. > > > > (Chris) > > > > > > > > Here are the histories of this patch in "Dma-buf support for Gvt-g" > > > > patch-set: > > > > > > > > v14: > > > > - return -ENXIO when gem proxy object is banned by ioctl. > > > > (Chris) (Daniel) > > > > > > > > v13: > > > > - add comments to GEM proxy. (Chris) > > > > - don't ban GEM proxy in i915_gem_sw_finish_ioctl. (Chris) > > > > - check GEM proxy bar after finishing i915_gem_object_wait. > > > > (Chris) > > > > - remove GEM proxy bar in i915_gem_madvise_ioctl. > > > > > > > > v6: > > > > - add gem proxy barrier in the following ioctls. (Chris) > > > > i915_gem_set_caching_ioctl > > > > i915_gem_set_domain_ioctl > > > > i915_gem_sw_finish_ioctl > > > > i915_gem_set_tiling_ioctl > > > > i915_gem_madvise_ioctl > > > > > > > > Signed-off-by: Tina Zhang <tina.zhang@intel.com> > > > > Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> > > > > Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> > > > > Cc: Daniel Vetter <daniel.vetter@ffwll.ch> > > > > > > <SNIP> > > > > > > > @@ -1649,6 +1659,10 @@ i915_gem_sw_finish_ioctl(struct drm_device > > > > > > *dev, void *data, > > > > if (!obj) > > > > return -ENOENT; > > > > > > > > + /* Proxy objects are barred from CPU access, so there is no > > > > + * need to ban sw_finish as it is a nop. > > > > + */ > > > > + > > > > /* Pinned buffers may be scanout, so flush the cache */ > > > > i915_gem_object_flush_if_display(obj); > > > > i915_gem_object_put(obj); > > > > @@ -2614,7 +2628,8 @@ void *i915_gem_object_pin_map(struct > > > > > > drm_i915_gem_object *obj, > > > > void *ptr; > > > > int ret; > > > > > > > > - GEM_BUG_ON(!i915_gem_object_has_struct_page(obj)); > > > > + if (unlikely(!i915_gem_object_has_struct_page(obj))) > > > > + return ERR_PTR(-ENODEV); > > > > > > You should have marked this change in the changelog and then marked > > > the Reviewed-by tags to be valid only to the previous version of this patch. > > > > > > It's not a fair game to claim a patch to be "Reviewed-by" at the > > > current version, when you've made changes that were not agreed upon. > > > > I thought we were agreed on this :) > > > > > > > > So that's some meta-review. Back to the actual review; > > > > > > Which codepath was hitting the GEM_BUG_ON? Wondering if it would be > > > cleaner to avoid the call to this function on that single codepath. > > > > Here is the previously comments: > > https://lists.freedesktop.org/archives/intel-gvt-dev/2017-October/0022 > > 78.html > > Thanks. > > I never even noticed such an e-mail, so the correct response would've been; > > Reviewed-by: Joonas #vX > Reviewed-by: Chris > > Where #vX is the version I actually agreed to. > > Reviewed-by tags are are ones you need to be especially careful about in > addition to the Signed-off-bys because they carry special meaning: > > https://www.kernel.org/doc/html/v4.13/process/submitting-patches.html#r > eviewer-s-statement-of-oversight Indeed, thank you :) BR, Tina > > Regards, Joonas > -- > Joonas Lahtinen > Open Source Technology Center > Intel Corporation > _______________________________________________ > intel-gvt-dev mailing list > intel-gvt-dev@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gvt-dev
Hi, As the "Dma-buf support for GVT-g" patch-set relys on this patch, can I collect the comments for this version? Do we all agree on it? Thanks. Comments of the previous version: https://lists.freedesktop.org/archives/intel-gvt-dev/2017-October/002278.html BR, Tina > -----Original Message----- > From: Zhang, Tina > Sent: Wednesday, November 1, 2017 5:22 PM > To: zhenyuw@linux.intel.com; Wang, Zhi A <zhi.a.wang@intel.com>; > daniel@ffwll.ch; joonas.lahtinen@linux.intel.com; chris@chris-wilson.co.uk > Cc: Zhang, Tina <tina.zhang@intel.com>; intel-gfx@lists.freedesktop.org; intel- > gvt-dev@lists.freedesktop.org; Daniel Vetter <daniel.vetter@ffwll.ch> > Subject: [PATCH v2 1/2] drm/i915: Introduce GEM proxy > > GEM proxy is a kind of GEM, whose backing physical memory is pinned and > produced by guest VM and is used by host as read only. With GEM proxy, host is > able to access guest physical memory through GEM object interface. As GEM > proxy is such a special kind of GEM, a new flag I915_GEM_OBJECT_IS_PROXY is > introduced to ban host from changing the backing storage of GEM proxy. > > v2: > - return -ENXIO when pin and map pages of GEM proxy to kernel space. > (Chris) > > Here are the histories of this patch in "Dma-buf support for Gvt-g" > patch-set: > > v14: > - return -ENXIO when gem proxy object is banned by ioctl. > (Chris) (Daniel) > > v13: > - add comments to GEM proxy. (Chris) > - don't ban GEM proxy in i915_gem_sw_finish_ioctl. (Chris) > - check GEM proxy bar after finishing i915_gem_object_wait. (Chris) > - remove GEM proxy bar in i915_gem_madvise_ioctl. > > v6: > - add gem proxy barrier in the following ioctls. (Chris) > i915_gem_set_caching_ioctl > i915_gem_set_domain_ioctl > i915_gem_sw_finish_ioctl > i915_gem_set_tiling_ioctl > i915_gem_madvise_ioctl > > Signed-off-by: Tina Zhang <tina.zhang@intel.com> > Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> > Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch> > --- > drivers/gpu/drm/i915/i915_gem.c | 25 ++++++++++++++++++++++++- > drivers/gpu/drm/i915/i915_gem_object.h | 11 +++++++++-- > drivers/gpu/drm/i915/i915_gem_tiling.c | 8 ++++++++ > 3 files changed, 41 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_gem.c > b/drivers/gpu/drm/i915/i915_gem.c index 82a1003..13bc18d 100644 > --- a/drivers/gpu/drm/i915/i915_gem.c > +++ b/drivers/gpu/drm/i915/i915_gem.c > @@ -1593,6 +1593,16 @@ i915_gem_set_domain_ioctl(struct drm_device > *dev, void *data, > if (err) > goto out; > > + /* Proxy objects do not control access to the backing storage, ergo > + * they cannot be used as a means to manipulate the cache domain > + * tracking for that backing storage. The proxy object is always > + * considered to be outside of any cache domain. > + */ > + if (i915_gem_object_is_proxy(obj)) { > + err = -ENXIO; > + goto out; > + } > + > /* Flush and acquire obj->pages so that we are coherent through > * direct access in memory with previous cached writes through > * shmemfs and that our cache domain tracking remains valid. > @@ -1649,6 +1659,10 @@ i915_gem_sw_finish_ioctl(struct drm_device *dev, > void *data, > if (!obj) > return -ENOENT; > > + /* Proxy objects are barred from CPU access, so there is no > + * need to ban sw_finish as it is a nop. > + */ > + > /* Pinned buffers may be scanout, so flush the cache */ > i915_gem_object_flush_if_display(obj); > i915_gem_object_put(obj); > @@ -2614,7 +2628,8 @@ void *i915_gem_object_pin_map(struct > drm_i915_gem_object *obj, > void *ptr; > int ret; > > - GEM_BUG_ON(!i915_gem_object_has_struct_page(obj)); > + if (unlikely(!i915_gem_object_has_struct_page(obj))) > + return ERR_PTR(-ENODEV); > > ret = mutex_lock_interruptible(&obj->mm.lock); > if (ret) > @@ -3766,6 +3781,14 @@ int i915_gem_set_caching_ioctl(struct drm_device > *dev, void *data, > if (!obj) > return -ENOENT; > > + /* The caching mode of proxy object is handled by its generator, and > not > + * expected to be changed by user mode. > + */ > + if (i915_gem_object_is_proxy(obj)) { > + ret = -ENXIO; > + goto out; > + } > + > if (obj->cache_level == level) > goto out; > > diff --git a/drivers/gpu/drm/i915/i915_gem_object.h > b/drivers/gpu/drm/i915/i915_gem_object.h > index 956c911..78d65db 100644 > --- a/drivers/gpu/drm/i915/i915_gem_object.h > +++ b/drivers/gpu/drm/i915/i915_gem_object.h > @@ -53,8 +53,9 @@ struct i915_lut_handle { > > struct drm_i915_gem_object_ops { > unsigned int flags; > -#define I915_GEM_OBJECT_HAS_STRUCT_PAGE BIT(0) > -#define I915_GEM_OBJECT_IS_SHRINKABLE BIT(1) > +#define I915_GEM_OBJECT_HAS_STRUCT_PAGE BIT(0) > +#define I915_GEM_OBJECT_IS_SHRINKABLE BIT(1) > +#define I915_GEM_OBJECT_IS_PROXY BIT(2) > > /* Interface between the GEM object and its backing storage. > * get_pages() is called once prior to the use of the associated set @@ - > 355,6 +356,12 @@ i915_gem_object_is_shrinkable(const struct > drm_i915_gem_object *obj) } > > static inline bool > +i915_gem_object_is_proxy(const struct drm_i915_gem_object *obj) { > + return obj->ops->flags & I915_GEM_OBJECT_IS_PROXY; } > + > +static inline bool > i915_gem_object_is_active(const struct drm_i915_gem_object *obj) { > return obj->active_count; > diff --git a/drivers/gpu/drm/i915/i915_gem_tiling.c > b/drivers/gpu/drm/i915/i915_gem_tiling.c > index fb5231f..f617012 100644 > --- a/drivers/gpu/drm/i915/i915_gem_tiling.c > +++ b/drivers/gpu/drm/i915/i915_gem_tiling.c > @@ -345,6 +345,14 @@ i915_gem_set_tiling_ioctl(struct drm_device *dev, > void *data, > if (!obj) > return -ENOENT; > > + /* The tiling mode of proxy objects is handled by its generator, and not > + * expected to be changed by user mode. > + */ > + if (i915_gem_object_is_proxy(obj)) { > + err = -ENXIO; > + goto err; > + } > + > if (!i915_tiling_ok(obj, args->tiling_mode, args->stride)) { > err = -EINVAL; > goto err; > -- > 2.7.4
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 82a1003..13bc18d 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -1593,6 +1593,16 @@ i915_gem_set_domain_ioctl(struct drm_device *dev, void *data, if (err) goto out; + /* Proxy objects do not control access to the backing storage, ergo + * they cannot be used as a means to manipulate the cache domain + * tracking for that backing storage. The proxy object is always + * considered to be outside of any cache domain. + */ + if (i915_gem_object_is_proxy(obj)) { + err = -ENXIO; + goto out; + } + /* Flush and acquire obj->pages so that we are coherent through * direct access in memory with previous cached writes through * shmemfs and that our cache domain tracking remains valid. @@ -1649,6 +1659,10 @@ i915_gem_sw_finish_ioctl(struct drm_device *dev, void *data, if (!obj) return -ENOENT; + /* Proxy objects are barred from CPU access, so there is no + * need to ban sw_finish as it is a nop. + */ + /* Pinned buffers may be scanout, so flush the cache */ i915_gem_object_flush_if_display(obj); i915_gem_object_put(obj); @@ -2614,7 +2628,8 @@ void *i915_gem_object_pin_map(struct drm_i915_gem_object *obj, void *ptr; int ret; - GEM_BUG_ON(!i915_gem_object_has_struct_page(obj)); + if (unlikely(!i915_gem_object_has_struct_page(obj))) + return ERR_PTR(-ENODEV); ret = mutex_lock_interruptible(&obj->mm.lock); if (ret) @@ -3766,6 +3781,14 @@ int i915_gem_set_caching_ioctl(struct drm_device *dev, void *data, if (!obj) return -ENOENT; + /* The caching mode of proxy object is handled by its generator, and not + * expected to be changed by user mode. + */ + if (i915_gem_object_is_proxy(obj)) { + ret = -ENXIO; + goto out; + } + if (obj->cache_level == level) goto out; diff --git a/drivers/gpu/drm/i915/i915_gem_object.h b/drivers/gpu/drm/i915/i915_gem_object.h index 956c911..78d65db 100644 --- a/drivers/gpu/drm/i915/i915_gem_object.h +++ b/drivers/gpu/drm/i915/i915_gem_object.h @@ -53,8 +53,9 @@ struct i915_lut_handle { struct drm_i915_gem_object_ops { unsigned int flags; -#define I915_GEM_OBJECT_HAS_STRUCT_PAGE BIT(0) -#define I915_GEM_OBJECT_IS_SHRINKABLE BIT(1) +#define I915_GEM_OBJECT_HAS_STRUCT_PAGE BIT(0) +#define I915_GEM_OBJECT_IS_SHRINKABLE BIT(1) +#define I915_GEM_OBJECT_IS_PROXY BIT(2) /* Interface between the GEM object and its backing storage. * get_pages() is called once prior to the use of the associated set @@ -355,6 +356,12 @@ i915_gem_object_is_shrinkable(const struct drm_i915_gem_object *obj) } static inline bool +i915_gem_object_is_proxy(const struct drm_i915_gem_object *obj) +{ + return obj->ops->flags & I915_GEM_OBJECT_IS_PROXY; +} + +static inline bool i915_gem_object_is_active(const struct drm_i915_gem_object *obj) { return obj->active_count; diff --git a/drivers/gpu/drm/i915/i915_gem_tiling.c b/drivers/gpu/drm/i915/i915_gem_tiling.c index fb5231f..f617012 100644 --- a/drivers/gpu/drm/i915/i915_gem_tiling.c +++ b/drivers/gpu/drm/i915/i915_gem_tiling.c @@ -345,6 +345,14 @@ i915_gem_set_tiling_ioctl(struct drm_device *dev, void *data, if (!obj) return -ENOENT; + /* The tiling mode of proxy objects is handled by its generator, and not + * expected to be changed by user mode. + */ + if (i915_gem_object_is_proxy(obj)) { + err = -ENXIO; + goto err; + } + if (!i915_tiling_ok(obj, args->tiling_mode, args->stride)) { err = -EINVAL; goto err;