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@xxxxxxxxx> > Reviewed-by: Joonas Lahtinen <joonas.lahtinen@xxxxxxxxxxxxxxx> > Reviewed-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> > Cc: Daniel Vetter <daniel.vetter@xxxxxxxx> <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 -- Joonas Lahtinen Open Source Technology Center Intel Corporation _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
