| Message ID | 1508144254-3526-2-git-send-email-tina.zhang@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Quoting Tina Zhang (2017-10-16 09:57:33) > 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. > > V1: > - the patch is separated from the "Dma-buf support for Gvt-g" > patch-set. (Joonas) > > 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> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch> > Cc: Chris Wilson <chris@chris-wilson.co.uk> > --- > drivers/gpu/drm/i915/i915_gem.c | 22 ++++++++++++++++++++++ > drivers/gpu/drm/i915/i915_gem_object.h | 11 +++++++++-- > drivers/gpu/drm/i915/i915_gem_tiling.c | 8 ++++++++ > 3 files changed, 39 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c > index 82a1003..7d3116a 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. > + */ > + Then we probably want a GEM_BUG_ON(i915_gem_object_is_proxy()) inside pin_to_display and for this path, flush_if_display. Otherwise, looking very nice. The comments are very much appreciated and useful. I just need to go over the list of ioctls and check you have everything. -Chris
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 82a1003..7d3116a 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); @@ -3766,6 +3780,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;