[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
> 
> Signed-off-by: Tina Zhang <tina.zhang at intel.com>
> Reviewed-by: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>
> Cc: Daniel Vetter <daniel.vetter at ffwll.ch>
> Cc: Chris Wilson <chris at 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