[Intel-gfx] [PATCH v1 1/2] drm/i915: Introduce GEM proxy

[Chris Wilson]

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

gem_busy: sees the local object, not the proxy activity; that's ok
get_caching: returns the constant value
pread: works by forcing GTT access
pwrite: works by forcing GTT access
mmap: disallowed by !filp, errno fixup in next patch
mmap_gtt: works
set-domain: fixed
sw-finish: noted as impossible (please add checks)
set-tiling: fixed
get_tiling: returns constant value
madvise: works, since it's a discard of the local page array
overlay-put-image? not supported on any relevant platforms!
gem_wait: works on local bo (as expected)

execbuf: works (or should at least work fine on a proxy object as either
a batch or one of the auxiliaries)

Hmm, use as a batch + cmdparser is broken; should result in ENODEV to
userspace. An admittedly odd result, but not an oops.

display: pin-to-display and fencing will work, I think, and
flush_for_display is irrelevant as no direct cpu access.

dmabuf: works partially, but what doesn't work is optional. Userspace
mmap is barred, most inter-driver interaction is barred. On paper a
nuisance should it want to be used, but we should not oops. Hmm, no,
we need to make the GEM_BUG_ON(!struct_page(obj)) in
i915_gem_object_pin_map() a real return.

Please add

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index d699ea3ab80b..6a7ca9a502ba 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2650,7 +2650,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)

and my
Reviewed-by: Chris Wilson <chris at chris-wilson.co.uk>
-Chris