| Message ID | 1507629007-3183-7-git-send-email-tina.zhang@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, 2017-10-10 at 17:50 +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. > > 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> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch> > Cc: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> <SNIP> > +++ b/drivers/gpu/drm/i915/i915_gem_object.h > @@ -39,6 +39,7 @@ 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_IS_PROXY BIT(2) Please fix the indent to match. Do convert the above two lines to use TAB character too. <SNIP> > @@ -1690,7 +1704,7 @@ i915_gem_mmap_ioctl(struct drm_device *dev, void *data, > */ > if (!obj->base.filp) { > i915_gem_object_put(obj); > - return -EINVAL; > + return -ENXIO; > } This still needs to be a separate patch. With those fixes, this is; Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Regards, Joonas
On Tue, 2017-10-10 at 13:58 +0300, Joonas Lahtinen wrote: > On Tue, 2017-10-10 at 17:50 +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. > > > > 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> > > Cc: Daniel Vetter <daniel.vetter@ffwll.ch> > > Cc: Chris Wilson <chris@chris-wilson.co.uk> > > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> > > <SNIP> > > > +++ b/drivers/gpu/drm/i915/i915_gem_object.h > > @@ -39,6 +39,7 @@ 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_IS_PROXY BIT(2) > > Please fix the indent to match. Do convert the above two lines to use > TAB character too. > > <SNIP> > > > @@ -1690,7 +1704,7 @@ i915_gem_mmap_ioctl(struct drm_device *dev, void *data, > > */ > > if (!obj->base.filp) { > > i915_gem_object_put(obj); > > - return -EINVAL; > > + return -ENXIO; > > } > > This still needs to be a separate patch. > > With those fixes, this is; > > Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Also, send the produced two patches (this and the split patch) as a standalone series for easier testing and merging. CI seems to have hard time applying the whole series. Regards, Joonas
> -----Original Message----- > From: Joonas Lahtinen [mailto:joonas.lahtinen@linux.intel.com] > Sent: Tuesday, October 10, 2017 9:02 PM > To: Zhang, Tina <tina.zhang@intel.com>; alex.williamson@redhat.com; > kraxel@redhat.com; chris@chris-wilson.co.uk; zhenyuw@linux.intel.com; Lv, > Zhiyuan <zhiyuan.lv@intel.com>; Wang, Zhi A <zhi.a.wang@intel.com>; Tian, > Kevin <kevin.tian@intel.com>; daniel@ffwll.ch; kwankhede@nvidia.com > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>; intel-gfx@lists.freedesktop.org; > intel-gvt-dev@lists.freedesktop.org; linux-kernel@vger.kernel.org > Subject: Re: [Intel-gfx] [PATCH v15 6/7] drm/i915: Introduce GEM proxy > > On Tue, 2017-10-10 at 13:58 +0300, Joonas Lahtinen wrote: > > On Tue, 2017-10-10 at 17:50 +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. > > > > > > 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> > > > Cc: Daniel Vetter <daniel.vetter@ffwll.ch> > > > Cc: Chris Wilson <chris@chris-wilson.co.uk> > > > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> > > > > <SNIP> > > > > > +++ b/drivers/gpu/drm/i915/i915_gem_object.h > > > @@ -39,6 +39,7 @@ 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_IS_PROXY BIT(2) > > > > Please fix the indent to match. Do convert the above two lines to use > > TAB character too. > > > > <SNIP> > > > > > @@ -1690,7 +1704,7 @@ i915_gem_mmap_ioctl(struct drm_device *dev, > void *data, > > > */ > > > if (!obj->base.filp) { > > > i915_gem_object_put(obj); > > > - return -EINVAL; > > > + return -ENXIO; > > > } > > > > This still needs to be a separate patch. > > > > With those fixes, this is; > > > > Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> > > Also, send the produced two patches (this and the split patch) as a standalone > series for easier testing and merging. Got it. Thanks. BR, Tina > > CI seems to have hard time applying the whole series. > > Regards, Joonas > -- > Joonas Lahtinen > Open Source Technology Center > Intel Corporation
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 000a764..7f1e7ab 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -1584,6 +1584,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. @@ -1640,6 +1650,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); @@ -1690,7 +1704,7 @@ i915_gem_mmap_ioctl(struct drm_device *dev, void *data, */ if (!obj->base.filp) { i915_gem_object_put(obj); - return -EINVAL; + return -ENXIO; } addr = vm_mmap(obj->base.filp, 0, args->size, @@ -3749,6 +3763,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 5b19a49..f3b382a 100644 --- a/drivers/gpu/drm/i915/i915_gem_object.h +++ b/drivers/gpu/drm/i915/i915_gem_object.h @@ -39,6 +39,7 @@ 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_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 @@ -300,6 +301,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;
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. 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> Cc: Daniel Vetter <daniel.vetter@ffwll.ch> Cc: Chris Wilson <chris@chris-wilson.co.uk> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> --- drivers/gpu/drm/i915/i915_gem.c | 24 +++++++++++++++++++++++- drivers/gpu/drm/i915/i915_gem_object.h | 7 +++++++ drivers/gpu/drm/i915/i915_gem_tiling.c | 8 ++++++++ 3 files changed, 38 insertions(+), 1 deletion(-)