[Intel-gfx] [PATCH 1/2] drm/i915: Use CPU mapping for userspace dma-buf mmap()

[Chris Wilson]

On Fri, Jul 31, 2015 at 05:42:23PM -0300, Tiago Vignatti wrote:
> For now we're opting out devices that don't have the LLC CPU cache (mostly
> "Atom" devices). Alternatively, we could build up a path to mmap them through
> GTT WC (and ignore the fact that will be dead-slow for reading). Or, an even
> more complex work I believe, would involve on setting up dma-buf ioctls to
> allow userspace flush, controlling manually the synchronization via
> begin{,end}_cpu_access.
> 
> Signed-off-by: Tiago Vignatti <tiago.vignatti at intel.com>
> ---
>  drivers/gpu/drm/i915/i915_gem_dmabuf.c | 21 ++++++++++++++++++++-
>  1 file changed, 20 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_dmabuf.c b/drivers/gpu/drm/i915/i915_gem_dmabuf.c
> index e9c2bfd..e6cb402 100644
> --- a/drivers/gpu/drm/i915/i915_gem_dmabuf.c
> +++ b/drivers/gpu/drm/i915/i915_gem_dmabuf.c
> @@ -193,7 +193,26 @@ static void i915_gem_dmabuf_kunmap(struct dma_buf *dma_buf, unsigned long page_n
>  
>  static int i915_gem_dmabuf_mmap(struct dma_buf *dma_buf, struct vm_area_struct *vma)
>  {
> -	return -EINVAL;
> +	struct drm_i915_gem_object *obj = dma_buf_to_obj(dma_buf);
> +	struct drm_device *dev = obj->base.dev;
> +	int ret;
> +
> +	if (obj->base.size < vma->vm_end - vma->vm_start)
> +		return -EINVAL;
> +
> +	/* On non-LLC machines we'd need to be careful cause CPU and GPU don't
> +	 * share the CPU's L3 cache and coherency may hurt when CPU mapping. */
> +	if (!HAS_LLC(dev))
> +		return -EINVAL;

The first problem is that llc does not guarrantee that the buffer is
cache coherent with all aspects of the GPU. For scanout and similar
writes need to be WC.

if (obj->has_framebuffer_references) would at least catch where the fb
is made before the mmap.

Equally this buffer could then be shared with other devices and exposing
a CPU mmap to userspace (and no flush/set-domain protocol) will result in
corruption.

> +	if (!obj->base.filp)
> +		return -EINVAL;
> +
> +	ret = obj->base.filp->f_op->mmap(obj->base.filp, vma);
> +	fput(vma->vm_file);
> +	vma->vm_file = get_file(obj->base.filp);

Transfer owenership even if the ->mmap() fails?
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre