[RFC] drm/i915: Allow dmabuf mmap forwarding

Tvrtko Ursulin tvrtko.ursulin at linux.intel.com
Mon Sep 25 13:16:38 UTC 2023


From: Tvrtko Ursulin <tvrtko.ursulin at intel.com>

Allow mmap forwarding for imported buffers in order to allow minigbm mmap
to work on aperture-less platforms such as Meteorlake.

So far i915 did not allow mmap on imported buffers but from minigbm
perspective that worked because of the DRM_IOCTL_I915_GEM_MMAP_GTT fall-
back would then be attempted, and would be successful.

This stops working on Meteorlake since there is no aperture.

Allow i915 to mmap imported buffers using forwarding via dma_buf_mmap(),
which allows the primary minigbm path of DRM_IOCTL_I915_GEM_MMAP_OFFSET /
I915_MMAP_OFFSET_WB to work.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
Cc: Daniel Vetter <daniel.vetter at ffwll.ch>
Cc: Christian König <christian.koenig at amd.com>
Cc: Matthew Auld <matthew.auld at intel.com>
Cc: Nirmoy Das <nirmoy.das at intel.com>
---
1)
It is unclear to me if any real userspace depends on this, but there are
certainly compliance suites which fail.

2)
It is also a bit unclear to me if dma_buf_mmap() is exactly intended for
this kind of use. It seems that it is, but I also found some old mailing
list discussions suggesting there might be some unresolved questions
around VMA revocation.

1 + 2 = RFC for now.

Daniel and Christian were involved in 2) in the past so comments would
be appreciated.

Test-with: 20230925131539.32743-1-tvrtko.ursulin at linux.intel.com

---
 drivers/gpu/drm/i915/gem/i915_gem_mman.c      | 78 +++++++++++++++----
 .../gpu/drm/i915/gem/i915_gem_object_types.h  |  1 +
 2 files changed, 65 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_mman.c b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
index aa4d842d4c5a..78c84c0a8b08 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_mman.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
@@ -5,6 +5,7 @@
  */
 
 #include <linux/anon_inodes.h>
+#include <linux/dma-buf.h>
 #include <linux/mman.h>
 #include <linux/pfn_t.h>
 #include <linux/sizes.h>
@@ -664,6 +665,7 @@ insert_mmo(struct drm_i915_gem_object *obj, struct i915_mmap_offset *mmo)
 static struct i915_mmap_offset *
 mmap_offset_attach(struct drm_i915_gem_object *obj,
 		   enum i915_mmap_type mmap_type,
+		   bool forward_mmap,
 		   struct drm_file *file)
 {
 	struct drm_i915_private *i915 = to_i915(obj->base.dev);
@@ -682,6 +684,7 @@ mmap_offset_attach(struct drm_i915_gem_object *obj,
 
 	mmo->obj = obj;
 	mmo->mmap_type = mmap_type;
+	mmo->forward_mmap = forward_mmap;
 	drm_vma_node_reset(&mmo->vma_node);
 
 	err = drm_vma_offset_add(obj->base.dev->vma_offset_manager,
@@ -714,12 +717,25 @@ mmap_offset_attach(struct drm_i915_gem_object *obj,
 	return ERR_PTR(err);
 }
 
+static bool
+should_forward_mmap(struct drm_i915_gem_object *obj,
+		    enum i915_mmap_type mmap_type)
+{
+	if (!obj->base.import_attach)
+		return false;
+
+	return mmap_type == I915_MMAP_TYPE_WB ||
+	       mmap_type == I915_MMAP_TYPE_WC ||
+	       mmap_type == I915_MMAP_TYPE_UC;
+}
+
 static int
 __assign_mmap_offset(struct drm_i915_gem_object *obj,
 		     enum i915_mmap_type mmap_type,
 		     u64 *offset, struct drm_file *file)
 {
 	struct i915_mmap_offset *mmo;
+	bool should_forward;
 
 	if (i915_gem_object_never_mmap(obj))
 		return -ENODEV;
@@ -735,12 +751,15 @@ __assign_mmap_offset(struct drm_i915_gem_object *obj,
 	if (mmap_type == I915_MMAP_TYPE_FIXED)
 		return -ENODEV;
 
+	should_forward = should_forward_mmap(obj, mmap_type);
+
 	if (mmap_type != I915_MMAP_TYPE_GTT &&
 	    !i915_gem_object_has_struct_page(obj) &&
-	    !i915_gem_object_has_iomem(obj))
+	    !i915_gem_object_has_iomem(obj) &&
+	    !should_forward)
 		return -ENODEV;
 
-	mmo = mmap_offset_attach(obj, mmap_type, file);
+	mmo = mmap_offset_attach(obj, mmap_type, should_forward, file);
 	if (IS_ERR(mmo))
 		return PTR_ERR(mmo);
 
@@ -936,6 +955,32 @@ static struct file *mmap_singleton(struct drm_i915_private *i915)
 	return file;
 }
 
+static void
+__vma_mmap_pgprot(struct vm_area_struct *vma, enum i915_mmap_type mmap_type)
+{
+	const pgprot_t pgprot =vm_get_page_prot(vma->vm_flags);
+
+	switch (mmap_type) {
+	case I915_MMAP_TYPE_WC:
+		vma->vm_page_prot = pgprot_writecombine(pgprot);
+		break;
+	case I915_MMAP_TYPE_FIXED:
+		GEM_WARN_ON(1);
+		fallthrough;
+	case I915_MMAP_TYPE_WB:
+		vma->vm_page_prot = pgprot;
+		break;
+	case I915_MMAP_TYPE_UC:
+		vma->vm_page_prot = pgprot_noncached(pgprot);
+		break;
+	case I915_MMAP_TYPE_GTT:
+		vma->vm_page_prot = pgprot_writecombine(pgprot);
+		break;
+	}
+
+	vma->vm_page_prot = pgprot_decrypted(vma->vm_page_prot);
+}
+
 static int
 i915_gem_object_mmap(struct drm_i915_gem_object *obj,
 		     struct i915_mmap_offset *mmo,
@@ -953,6 +998,20 @@ i915_gem_object_mmap(struct drm_i915_gem_object *obj,
 		vm_flags_clear(vma, VM_MAYWRITE);
 	}
 
+	/* dma-buf import */
+	if (mmo && mmo->forward_mmap) {
+		__vma_mmap_pgprot(vma, mmo->mmap_type);
+		vm_flags_set(vma, VM_PFNMAP | VM_DONTEXPAND | VM_DONTDUMP | VM_IO);
+
+		/*
+		 * Don't have our vm_ops to drop the reference in this case so
+		 * drop it now and if object goes away userspace will fault.
+		 */
+		i915_gem_object_put(mmo->obj);
+
+		return dma_buf_mmap(obj->base.dma_buf, vma, 0);
+	}
+
 	anon = mmap_singleton(to_i915(dev));
 	if (IS_ERR(anon)) {
 		i915_gem_object_put(obj);
@@ -982,34 +1041,25 @@ i915_gem_object_mmap(struct drm_i915_gem_object *obj,
 
 	vma->vm_private_data = mmo;
 
+	__vma_mmap_pgprot(vma, mmo->mmap_type);
+
 	switch (mmo->mmap_type) {
 	case I915_MMAP_TYPE_WC:
-		vma->vm_page_prot =
-			pgprot_writecombine(vm_get_page_prot(vma->vm_flags));
 		vma->vm_ops = &vm_ops_cpu;
 		break;
-
 	case I915_MMAP_TYPE_FIXED:
 		GEM_WARN_ON(1);
 		fallthrough;
 	case I915_MMAP_TYPE_WB:
-		vma->vm_page_prot = vm_get_page_prot(vma->vm_flags);
 		vma->vm_ops = &vm_ops_cpu;
 		break;
-
 	case I915_MMAP_TYPE_UC:
-		vma->vm_page_prot =
-			pgprot_noncached(vm_get_page_prot(vma->vm_flags));
 		vma->vm_ops = &vm_ops_cpu;
 		break;
-
 	case I915_MMAP_TYPE_GTT:
-		vma->vm_page_prot =
-			pgprot_writecombine(vm_get_page_prot(vma->vm_flags));
 		vma->vm_ops = &vm_ops_gtt;
 		break;
 	}
-	vma->vm_page_prot = pgprot_decrypted(vma->vm_page_prot);
 
 	return 0;
 }
@@ -1084,7 +1134,7 @@ int i915_gem_fb_mmap(struct drm_i915_gem_object *obj, struct vm_area_struct *vma
 	} else {
 		/* handle stolen and smem objects */
 		mmap_type = i915_ggtt_has_aperture(ggtt) ? I915_MMAP_TYPE_GTT : I915_MMAP_TYPE_WC;
-		mmo = mmap_offset_attach(obj, mmap_type, NULL);
+		mmo = mmap_offset_attach(obj, mmap_type, false, NULL);
 		if (IS_ERR(mmo))
 			return PTR_ERR(mmo);
 	}
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object_types.h b/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
index 0c5cdab278b6..b4f86fa020aa 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
@@ -225,6 +225,7 @@ struct i915_mmap_offset {
 	struct drm_vma_offset_node vma_node;
 	struct drm_i915_gem_object *obj;
 	enum i915_mmap_type mmap_type;
+	bool forward_mmap;
 
 	struct rb_node offset;
 };
-- 
2.39.2



More information about the dri-devel mailing list