All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/4] drm/i915: Allow i915 to manage the vma offset nodes instead of drm core
@ 2019-11-19 11:37 ` Abdiel Janulgue
  0 siblings, 0 replies; 23+ messages in thread
From: Abdiel Janulgue @ 2019-11-19 11:37 UTC (permalink / raw)
  To: intel-gfx; +Cc: Matthew Auld

Have i915 replace the core drm_gem_mmap implementation to overcome its
limitation in having only a single mmap offset node per gem object.
This change allows us to have multiple mmap offsets per object and
enables a mmapping instance to use unique fault-handlers per user vma.

This allows i915 to store extra data within vma->vm_private_data and
assign the pagefault ops for each mmap instance allowing objects to use
multiple fault handlers depending on its backing storage.

v2:
 - Fix race condition exposed by gem_mmap_gtt@close-race. Simplify
   lifetime management of the mmap offset objects be ensuring it is
   owned by the parent gem object instead of refcounting.
 - Track mmo used by fencing to avoid locking when revoking mmaps
   during GPU reset.
 - Rebase.
v3:
 - Simplify mmo tracking
v4:
 - use vma->mmo in __i915_gem_object_release_mmap_gtt
v5:
 - Revoke CPU mmaps on i915_gem_object_unbind() since unlike GTT
   mmaps, they don't have bound i915_vma objects. Rebase.
v6: Minor tweaks, header re-org (Chris)
v7:
- Call drm_vma_node_revoke for the mmo with corresponding file when
  releasing the gem handle instead of in the mmo's teardown.
- Add flag I915_BO_READONLY instead of obj->readonly.

Signed-off-by: Abdiel Janulgue <abdiel.janulgue@linux.intel.com>
Cc: Matthew Auld <matthew.auld@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/gem/i915_gem_domain.c    |   3 +-
 drivers/gpu/drm/i915/gem/i915_gem_mman.c      | 241 +++++++++++++++---
 drivers/gpu/drm/i915/gem/i915_gem_mman.h      |  28 ++
 drivers/gpu/drm/i915/gem/i915_gem_object.c    |  18 ++
 drivers/gpu/drm/i915/gem/i915_gem_object.h    |   7 +-
 .../gpu/drm/i915/gem/i915_gem_object_types.h  |  19 ++
 drivers/gpu/drm/i915/gem/i915_gem_pages.c     |   3 +
 drivers/gpu/drm/i915/gem/i915_gem_tiling.c    |   1 +
 .../drm/i915/gem/selftests/i915_gem_mman.c    |  20 +-
 drivers/gpu/drm/i915/gt/intel_reset.c         |   7 +-
 drivers/gpu/drm/i915/i915_drv.c               |  11 +-
 drivers/gpu/drm/i915/i915_drv.h               |   3 -
 drivers/gpu/drm/i915/i915_gem.c               |   3 +-
 drivers/gpu/drm/i915/i915_getparam.c          |   1 +
 drivers/gpu/drm/i915/i915_vma.c               |   3 +-
 drivers/gpu/drm/i915/i915_vma.h               |   3 +
 16 files changed, 313 insertions(+), 58 deletions(-)
 create mode 100644 drivers/gpu/drm/i915/gem/i915_gem_mman.h

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_domain.c b/drivers/gpu/drm/i915/gem/i915_gem_domain.c
index e2af63af67ad..fd1c6f12b77a 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_domain.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_domain.c
@@ -13,6 +13,7 @@
 #include "i915_gem_object.h"
 #include "i915_vma.h"
 #include "i915_gem_lmem.h"
+#include "i915_gem_mman.h"
 
 static void __i915_gem_object_flush_for_display(struct drm_i915_gem_object *obj)
 {
@@ -255,7 +256,7 @@ int i915_gem_object_set_cache_level(struct drm_i915_gem_object *obj,
 			}
 
 			if (obj->userfault_count)
-				__i915_gem_object_release_mmap(obj);
+				__i915_gem_object_release_mmap_gtt(obj);
 
 			/*
 			 * As we no longer need a fence for GTT access,
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_mman.c b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
index d60973603cc1..36fffb671601 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_mman.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
@@ -14,6 +14,7 @@
 #include "i915_gem_gtt.h"
 #include "i915_gem_ioctls.h"
 #include "i915_gem_object.h"
+#include "i915_gem_mman.h"
 #include "i915_trace.h"
 #include "i915_vma.h"
 
@@ -219,7 +220,8 @@ vm_fault_t i915_gem_fault(struct vm_fault *vmf)
 {
 #define MIN_CHUNK_PAGES (SZ_1M >> PAGE_SHIFT)
 	struct vm_area_struct *area = vmf->vma;
-	struct drm_i915_gem_object *obj = to_intel_bo(area->vm_private_data);
+	struct i915_mmap_offset *priv = area->vm_private_data;
+	struct drm_i915_gem_object *obj = priv->obj;
 	struct drm_device *dev = obj->base.dev;
 	struct drm_i915_private *i915 = to_i915(dev);
 	struct intel_runtime_pm *rpm = &i915->runtime_pm;
@@ -312,6 +314,9 @@ vm_fault_t i915_gem_fault(struct vm_fault *vmf)
 		list_add(&obj->userfault_link, &i915->ggtt.userfault_list);
 	mutex_unlock(&i915->ggtt.vm.mutex);
 
+	/* Track the mmo associated with the fenced vma */
+	vma->mmo = priv;
+
 	if (IS_ACTIVE(CONFIG_DRM_I915_USERFAULT_AUTOSUSPEND))
 		intel_wakeref_auto(&i915->ggtt.userfault_wakeref,
 				   msecs_to_jiffies_timeout(CONFIG_DRM_I915_USERFAULT_AUTOSUSPEND));
@@ -358,28 +363,19 @@ vm_fault_t i915_gem_fault(struct vm_fault *vmf)
 	}
 }
 
-void __i915_gem_object_release_mmap(struct drm_i915_gem_object *obj)
+void __i915_gem_object_release_mmap_gtt(struct drm_i915_gem_object *obj)
 {
 	struct i915_vma *vma;
 
 	GEM_BUG_ON(!obj->userfault_count);
 
-	obj->userfault_count = 0;
-	list_del(&obj->userfault_link);
-	drm_vma_node_unmap(&obj->base.vma_node,
-			   obj->base.dev->anon_inode->i_mapping);
-
 	for_each_ggtt_vma(vma, obj)
-		i915_vma_unset_userfault(vma);
+		i915_vma_revoke_mmap(vma);
+
+	GEM_BUG_ON(obj->userfault_count);
 }
 
 /**
- * i915_gem_object_release_mmap - remove physical page mappings
- * @obj: obj in question
- *
- * Preserve the reservation of the mmapping with the DRM core code, but
- * relinquish ownership of the pages back to the system.
- *
  * It is vital that we remove the page mapping if we have mapped a tiled
  * object through the GTT and then lose the fence register due to
  * resource pressure. Similarly if the object has been moved out of the
@@ -387,7 +383,7 @@ void __i915_gem_object_release_mmap(struct drm_i915_gem_object *obj)
  * mapping will then trigger a page fault on the next user access, allowing
  * fixup by i915_gem_fault().
  */
-void i915_gem_object_release_mmap(struct drm_i915_gem_object *obj)
+static void i915_gem_object_release_mmap_gtt(struct drm_i915_gem_object *obj)
 {
 	struct drm_i915_private *i915 = to_i915(obj->base.dev);
 	intel_wakeref_t wakeref;
@@ -406,7 +402,7 @@ void i915_gem_object_release_mmap(struct drm_i915_gem_object *obj)
 	if (!obj->userfault_count)
 		goto out;
 
-	__i915_gem_object_release_mmap(obj);
+	__i915_gem_object_release_mmap_gtt(obj);
 
 	/* Ensure that the CPU's PTE are revoked and there are not outstanding
 	 * memory transactions from userspace before we return. The TLB
@@ -422,15 +418,63 @@ void i915_gem_object_release_mmap(struct drm_i915_gem_object *obj)
 	intel_runtime_pm_put(&i915->runtime_pm, wakeref);
 }
 
-static int create_mmap_offset(struct drm_i915_gem_object *obj)
+void i915_gem_object_release_mmap_offset(struct drm_i915_gem_object *obj)
+{
+	struct i915_mmap_offset *mmo;
+
+	mutex_lock(&obj->mmo_lock);
+	list_for_each_entry(mmo, &obj->mmap_offsets, offset) {
+		/* vma_node_unmap for GTT mmaps handled already in
+		 * __i915_gem_object_release_mmap_gtt
+		 */
+		if (mmo->mmap_type != I915_MMAP_TYPE_GTT)
+			drm_vma_node_unmap(&mmo->vma_node,
+					   obj->base.dev->anon_inode->i_mapping);
+	}
+	mutex_unlock(&obj->mmo_lock);
+}
+
+/**
+ * i915_gem_object_release_mmap - remove physical page mappings
+ * @obj: obj in question
+ *
+ * Preserve the reservation of the mmapping with the DRM core code, but
+ * relinquish ownership of the pages back to the system.
+ */
+void i915_gem_object_release_mmap(struct drm_i915_gem_object *obj)
+{
+	i915_gem_object_release_mmap_gtt(obj);
+	i915_gem_object_release_mmap_offset(obj);
+}
+
+static void init_mmap_offset(struct drm_i915_gem_object *obj,
+			     struct i915_mmap_offset *mmo)
+{
+	mutex_lock(&obj->mmo_lock);
+	list_add(&mmo->offset, &obj->mmap_offsets);
+	mutex_unlock(&obj->mmo_lock);
+
+	mmo->obj = obj;
+	mmo->dev = obj->base.dev;
+}
+
+static int create_mmap_offset(struct drm_i915_gem_object *obj,
+			      struct i915_mmap_offset *mmo)
 {
 	struct drm_i915_private *i915 = to_i915(obj->base.dev);
 	struct intel_gt *gt = &i915->gt;
+	struct drm_device *dev = obj->base.dev;
 	int err;
 
-	err = drm_gem_create_mmap_offset(&obj->base);
-	if (likely(!err))
+	drm_vma_node_reset(&mmo->vma_node);
+	if (mmo->file)
+		drm_vma_node_allow(&mmo->vma_node, mmo->file);
+	err = drm_vma_offset_add(dev->vma_offset_manager, &mmo->vma_node,
+				 obj->base.size / PAGE_SIZE);
+	if (likely(!err)) {
+		init_mmap_offset(obj, mmo);
 		return 0;
+	}
 
 	/* Attempt to reap some mmap space from dead objects */
 	err = intel_gt_retire_requests_timeout(gt, MAX_SCHEDULE_TIMEOUT);
@@ -438,34 +482,56 @@ static int create_mmap_offset(struct drm_i915_gem_object *obj)
 		return err;
 
 	i915_gem_drain_freed_objects(i915);
-	return drm_gem_create_mmap_offset(&obj->base);
+	err = drm_vma_offset_add(dev->vma_offset_manager, &mmo->vma_node,
+				 obj->base.size / PAGE_SIZE);
+	if (err)
+		return err;
+
+	init_mmap_offset(obj, mmo);
+	return 0;
 }
 
-int
-i915_gem_mmap_gtt(struct drm_file *file,
-		  struct drm_device *dev,
-		  u32 handle,
-		  u64 *offset)
+static int
+__assign_gem_object_mmap_data(struct drm_file *file,
+			      u32 handle,
+			      enum i915_mmap_type mmap_type,
+			      u64 *offset)
 {
 	struct drm_i915_gem_object *obj;
+	struct i915_mmap_offset *mmo;
 	int ret;
 
-	if (!i915_ggtt_has_aperture(&to_i915(dev)->ggtt))
-		return -ENODEV;
-
 	obj = i915_gem_object_lookup(file, handle);
 	if (!obj)
 		return -ENOENT;
 
-	if (i915_gem_object_never_bind_ggtt(obj)) {
+	if (mmap_type == I915_MMAP_TYPE_GTT &&
+	    i915_gem_object_never_bind_ggtt(obj)) {
 		ret = -ENODEV;
 		goto out;
 	}
 
-	ret = create_mmap_offset(obj);
-	if (ret == 0)
-		*offset = drm_vma_node_offset_addr(&obj->base.vma_node);
+	if (mmap_type != I915_MMAP_TYPE_GTT &&
+	    !i915_gem_object_has_struct_page(obj)) {
+		ret = -ENODEV;
+		goto out;
+	}
+
+	mmo = kzalloc(sizeof(*mmo), GFP_KERNEL);
+	if (!mmo) {
+		ret = -ENOMEM;
+		goto out;
+	}
+
+	mmo->file = file;
+	ret = create_mmap_offset(obj, mmo);
+	if (ret) {
+		kfree(mmo);
+		goto out;
+	}
 
+	mmo->mmap_type = mmap_type;
+	*offset = drm_vma_node_offset_addr(&mmo->vma_node);
 out:
 	i915_gem_object_put(obj);
 	return ret;
@@ -492,7 +558,116 @@ i915_gem_mmap_gtt_ioctl(struct drm_device *dev, void *data,
 {
 	struct drm_i915_gem_mmap_gtt *args = data;
 
-	return i915_gem_mmap_gtt(file, dev, args->handle, &args->offset);
+	return __assign_gem_object_mmap_data(file, args->handle,
+					     I915_MMAP_TYPE_GTT,
+					     &args->offset);
+}
+
+void i915_mmap_offset_destroy(struct i915_mmap_offset *mmo, struct mutex *mutex)
+{
+	drm_vma_offset_remove(mmo->dev->vma_offset_manager, &mmo->vma_node);
+
+	mutex_lock(mutex);
+	list_del(&mmo->offset);
+	mutex_unlock(mutex);
+
+	kfree(mmo);
+}
+
+static void i915_gem_vm_open(struct vm_area_struct *vma)
+{
+	struct i915_mmap_offset *mmo = vma->vm_private_data;
+	struct drm_i915_gem_object *obj = mmo->obj;
+
+	GEM_BUG_ON(!obj);
+	i915_gem_object_get(obj);
+}
+
+static void i915_gem_vm_close(struct vm_area_struct *vma)
+{
+	struct i915_mmap_offset *mmo = vma->vm_private_data;
+	struct drm_i915_gem_object *obj = mmo->obj;
+
+	GEM_BUG_ON(!obj);
+	i915_gem_object_put(obj);
+}
+
+static const struct vm_operations_struct i915_gem_gtt_vm_ops = {
+	.fault = i915_gem_fault,
+	.open = i915_gem_vm_open,
+	.close = i915_gem_vm_close,
+};
+
+/* This overcomes the limitation in drm_gem_mmap's assignment of a
+ * drm_gem_object as the vma->vm_private_data. Since we need to
+ * be able to resolve multiple mmap offsets which could be tied
+ * to a single gem object.
+ */
+int i915_gem_mmap(struct file *filp, struct vm_area_struct *vma)
+{
+	struct drm_vma_offset_node *node;
+	struct drm_file *priv = filp->private_data;
+	struct drm_device *dev = priv->minor->dev;
+	struct i915_mmap_offset *mmo = NULL;
+	struct drm_gem_object *obj = NULL;
+
+	if (drm_dev_is_unplugged(dev))
+		return -ENODEV;
+
+	drm_vma_offset_lock_lookup(dev->vma_offset_manager);
+	node = drm_vma_offset_exact_lookup_locked(dev->vma_offset_manager,
+						  vma->vm_pgoff,
+						  vma_pages(vma));
+	if (likely(node)) {
+		mmo = container_of(node, struct i915_mmap_offset,
+				   vma_node);
+		/*
+		 * In our dependency chain, the drm_vma_offset_node
+		 * depends on the validity of the mmo, which depends on
+		 * the gem object. However the only reference we have
+		 * at this point is the mmo (as the parent of the node).
+		 * Try to check if the gem object was at least cleared.
+		 */
+		if (!mmo || !mmo->obj) {
+			drm_vma_offset_unlock_lookup(dev->vma_offset_manager);
+			return -EINVAL;
+		}
+		/*
+		 * Skip 0-refcnted objects as it is in the process of being
+		 * destroyed and will be invalid when the vma manager lock
+		 * is released.
+		 */
+		obj = &mmo->obj->base;
+		if (!kref_get_unless_zero(&obj->refcount))
+			obj = NULL;
+
+	}
+	drm_vma_offset_unlock_lookup(dev->vma_offset_manager);
+
+	if (!obj)
+		return -EINVAL;
+
+	if (!drm_vma_node_is_allowed(node, priv)) {
+		drm_gem_object_put_unlocked(obj);
+		return -EACCES;
+	}
+
+	if (i915_gem_object_is_readonly(to_intel_bo(obj))) {
+		if (vma->vm_flags & VM_WRITE) {
+			drm_gem_object_put_unlocked(obj);
+			return -EINVAL;
+		}
+		vma->vm_flags &= ~VM_MAYWRITE;
+	}
+
+	vma->vm_flags |= VM_PFNMAP | VM_DONTEXPAND | VM_DONTDUMP;
+	vma->vm_page_prot = pgprot_writecombine(vm_get_page_prot(vma->vm_flags));
+	vma->vm_page_prot = pgprot_decrypted(vma->vm_page_prot);
+	vma->vm_private_data = mmo;
+
+	vma->vm_ops = &i915_gem_gtt_vm_ops;
+
+	return 0;
 }
 
 #if IS_ENABLED(CONFIG_DRM_I915_SELFTEST)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_mman.h b/drivers/gpu/drm/i915/gem/i915_gem_mman.h
new file mode 100644
index 000000000000..25a3c4d6cd65
--- /dev/null
+++ b/drivers/gpu/drm/i915/gem/i915_gem_mman.h
@@ -0,0 +1,28 @@
+/*
+ * SPDX-License-Identifier: MIT
+ *
+ * Copyright © 2019 Intel Corporation
+ */
+
+#ifndef __I915_GEM_MMAN_H__
+#define __I915_GEM_MMAN_H__
+
+#include <linux/mm_types.h>
+#include <linux/types.h>
+
+struct drm_device;
+struct drm_file;
+struct drm_i915_gem_object;
+struct file;
+struct i915_mmap_offset;
+struct mutex;
+
+int i915_gem_mmap(struct file *filp, struct vm_area_struct *vma);
+vm_fault_t i915_gem_fault(struct vm_fault *vmf);
+void i915_mmap_offset_destroy(struct i915_mmap_offset *mmo, struct mutex *mutex);
+
+void __i915_gem_object_release_mmap_gtt(struct drm_i915_gem_object *obj);
+void i915_gem_object_release_mmap(struct drm_i915_gem_object *obj);
+void i915_gem_object_release_mmap_offset(struct drm_i915_gem_object *obj);
+
+#endif
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.c b/drivers/gpu/drm/i915/gem/i915_gem_object.c
index db103d3c8760..44cb5a2b140d 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_object.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_object.c
@@ -29,6 +29,7 @@
 #include "i915_drv.h"
 #include "i915_gem_clflush.h"
 #include "i915_gem_context.h"
+#include "i915_gem_mman.h"
 #include "i915_gem_object.h"
 #include "i915_globals.h"
 #include "i915_trace.h"
@@ -61,6 +62,9 @@ void i915_gem_object_init(struct drm_i915_gem_object *obj,
 
 	INIT_LIST_HEAD(&obj->lut_list);
 
+	mutex_init(&obj->mmo_lock);
+	INIT_LIST_HEAD(&obj->mmap_offsets);
+
 	init_rcu_head(&obj->rcu);
 
 	obj->ops = ops;
@@ -97,6 +101,7 @@ void i915_gem_close_object(struct drm_gem_object *gem, struct drm_file *file)
 	struct drm_i915_gem_object *obj = to_intel_bo(gem);
 	struct drm_i915_file_private *fpriv = file->driver_priv;
 	struct i915_lut_handle *lut, *ln;
+	struct i915_mmap_offset *mmo, *on;
 	LIST_HEAD(close);
 
 	i915_gem_object_lock(obj);
@@ -111,6 +116,9 @@ void i915_gem_close_object(struct drm_gem_object *gem, struct drm_file *file)
 	}
 	i915_gem_object_unlock(obj);
 
+	list_for_each_entry_safe(mmo, on, &obj->mmap_offsets, offset)
+		drm_vma_node_revoke(&mmo->vma_node, file);
+
 	list_for_each_entry_safe(lut, ln, &close, obj_link) {
 		struct i915_gem_context *ctx = lut->ctx;
 		struct i915_vma *vma;
@@ -158,6 +166,8 @@ static void __i915_gem_free_objects(struct drm_i915_private *i915,
 
 	wakeref = intel_runtime_pm_get(&i915->runtime_pm);
 	llist_for_each_entry_safe(obj, on, freed, freed) {
+		struct i915_mmap_offset *mmo, *mn;
+
 		trace_i915_gem_object_destroy(obj);
 
 		if (!list_empty(&obj->vma.list)) {
@@ -183,6 +193,14 @@ static void __i915_gem_free_objects(struct drm_i915_private *i915,
 			spin_unlock(&obj->vma.lock);
 		}
 
+		i915_gem_object_release_mmap(obj);
+
+		list_for_each_entry_safe(mmo, mn, &obj->mmap_offsets, offset) {
+			mmo->obj = NULL;
+			i915_mmap_offset_destroy(mmo, &obj->mmo_lock);
+		}
+
+		GEM_BUG_ON(!list_empty(&obj->mmap_offsets));
 		GEM_BUG_ON(atomic_read(&obj->bind_count));
 		GEM_BUG_ON(obj->userfault_count);
 		GEM_BUG_ON(!list_empty(&obj->lut_list));
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.h b/drivers/gpu/drm/i915/gem/i915_gem_object.h
index e5750d506cc9..a1eb7c0b23ac 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_object.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_object.h
@@ -132,13 +132,13 @@ void i915_gem_object_unlock_fence(struct drm_i915_gem_object *obj,
 static inline void
 i915_gem_object_set_readonly(struct drm_i915_gem_object *obj)
 {
-	obj->base.vma_node.readonly = true;
+	obj->flags |= I915_BO_READONLY;
 }
 
 static inline bool
 i915_gem_object_is_readonly(const struct drm_i915_gem_object *obj)
 {
-	return obj->base.vma_node.readonly;
+	return obj->flags & I915_BO_READONLY;
 }
 
 static inline bool
@@ -387,9 +387,6 @@ static inline void i915_gem_object_unpin_map(struct drm_i915_gem_object *obj)
 	i915_gem_object_unpin_pages(obj);
 }
 
-void __i915_gem_object_release_mmap(struct drm_i915_gem_object *obj);
-void i915_gem_object_release_mmap(struct drm_i915_gem_object *obj);
-
 void
 i915_gem_object_flush_write_domain(struct drm_i915_gem_object *obj,
 				   unsigned int flush_domains);
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 15f8297dc34e..8ff0834a1d5c 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
@@ -63,6 +63,20 @@ struct drm_i915_gem_object_ops {
 	void (*release)(struct drm_i915_gem_object *obj);
 };
 
+enum i915_mmap_type {
+	I915_MMAP_TYPE_GTT = 0,
+};
+
+struct i915_mmap_offset {
+	struct drm_device *dev;
+	struct drm_vma_offset_node vma_node;
+	struct drm_i915_gem_object *obj;
+	struct drm_file *file;
+	enum i915_mmap_type mmap_type;
+
+	struct list_head offset;
+};
+
 struct drm_i915_gem_object {
 	struct drm_gem_object base;
 
@@ -118,12 +132,17 @@ struct drm_i915_gem_object {
 	unsigned int userfault_count;
 	struct list_head userfault_link;
 
+	/* Protects access to mmap offsets */
+	struct mutex mmo_lock;
+	struct list_head mmap_offsets;
+
 	I915_SELFTEST_DECLARE(struct list_head st_link);
 
 	unsigned long flags;
 #define I915_BO_ALLOC_CONTIGUOUS BIT(0)
 #define I915_BO_ALLOC_VOLATILE   BIT(1)
 #define I915_BO_ALLOC_FLAGS (I915_BO_ALLOC_CONTIGUOUS | I915_BO_ALLOC_VOLATILE)
+#define I915_BO_READONLY         BIT(2)
 
 	/*
 	 * Is the object to be mapped as read-only to the GPU
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_pages.c b/drivers/gpu/drm/i915/gem/i915_gem_pages.c
index f402c2c415c2..75197ca696a8 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_pages.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_pages.c
@@ -8,6 +8,7 @@
 #include "i915_gem_object.h"
 #include "i915_scatterlist.h"
 #include "i915_gem_lmem.h"
+#include "i915_gem_mman.h"
 
 void __i915_gem_object_set_pages(struct drm_i915_gem_object *obj,
 				 struct sg_table *pages,
@@ -207,6 +208,8 @@ int __i915_gem_object_put_pages(struct drm_i915_gem_object *obj)
 		goto unlock;
 	}
 
+	i915_gem_object_release_mmap_offset(obj);
+
 	/*
 	 * ->put_pages might need to allocate memory for the bit17 swizzle
 	 * array, hence protect them from being reaped by removing them from gtt
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_tiling.c b/drivers/gpu/drm/i915/gem/i915_gem_tiling.c
index 1fa592d82af5..6c7825a2dc2a 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_tiling.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_tiling.c
@@ -11,6 +11,7 @@
 #include "i915_drv.h"
 #include "i915_gem.h"
 #include "i915_gem_ioctls.h"
+#include "i915_gem_mman.h"
 #include "i915_gem_object.h"
 
 /**
diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c b/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c
index 9f1a69027a04..6c2333c61469 100644
--- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c
+++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c
@@ -564,15 +564,20 @@ static bool assert_mmap_offset(struct drm_i915_private *i915,
 			       int expected)
 {
 	struct drm_i915_gem_object *obj;
+	/* Ownership transferred to parent gem object in create_mmap_offset */
+	struct i915_mmap_offset *mmo = kzalloc(sizeof(*mmo), GFP_KERNEL);
 	int err;
 
 	obj = i915_gem_object_create_internal(i915, size);
 	if (IS_ERR(obj))
 		return PTR_ERR(obj);
 
-	err = create_mmap_offset(obj);
+	err = create_mmap_offset(obj, mmo);
+	if (err)
+		kfree(mmo);
 	i915_gem_object_put(obj);
 
+
 	return err == expected;
 }
 
@@ -608,6 +613,8 @@ static int igt_mmap_offset_exhaustion(void *arg)
 	struct drm_mm *mm = &i915->drm.vma_offset_manager->vm_addr_space_mm;
 	struct drm_i915_gem_object *obj;
 	struct drm_mm_node *hole, *next;
+	/* Ownership transferred to parent gem object in create_mmap_offset */
+	struct i915_mmap_offset *mmo = kzalloc(sizeof(*mmo), GFP_KERNEL);
 	int loop, err;
 
 	/* Disable background reaper */
@@ -672,9 +679,10 @@ static int igt_mmap_offset_exhaustion(void *arg)
 		goto out;
 	}
 
-	err = create_mmap_offset(obj);
+	err = create_mmap_offset(obj, mmo);
 	if (err) {
 		pr_err("Unable to insert object into reclaimed hole\n");
+		kfree(mmo);
 		goto err_obj;
 	}
 
@@ -728,6 +736,8 @@ static int igt_mmap_gtt(void *arg)
 	struct drm_i915_private *i915 = arg;
 	struct drm_i915_gem_object *obj;
 	struct vm_area_struct *area;
+	/* Ownership transferred to parent gem object in create_mmap_offset */
+	struct i915_mmap_offset *mmo = kzalloc(sizeof(*mmo), GFP_KERNEL);
 	unsigned long addr;
 	void *vaddr;
 	int err, i;
@@ -748,7 +758,7 @@ static int igt_mmap_gtt(void *arg)
 	i915_gem_object_flush_map(obj);
 	i915_gem_object_unpin_map(obj);
 
-	err = create_mmap_offset(obj);
+	err = create_mmap_offset(obj, mmo);
 	if (err)
 		goto out;
 
@@ -876,6 +886,8 @@ static int igt_mmap_gtt_revoke(void *arg)
 {
 	struct drm_i915_private *i915 = arg;
 	struct drm_i915_gem_object *obj;
+	/* Ownership transferred to parent gem object in create_mmap_offset */
+	struct i915_mmap_offset *mmo = kzalloc(sizeof(*mmo), GFP_KERNEL);
 	unsigned long addr;
 	int err;
 
@@ -886,7 +898,7 @@ static int igt_mmap_gtt_revoke(void *arg)
 	if (IS_ERR(obj))
 		return PTR_ERR(obj);
 
-	err = create_mmap_offset(obj);
+	err = create_mmap_offset(obj, mmo);
 	if (err)
 		goto out;
 
diff --git a/drivers/gpu/drm/i915/gt/intel_reset.c b/drivers/gpu/drm/i915/gt/intel_reset.c
index b7007cd78c6f..777eaa483676 100644
--- a/drivers/gpu/drm/i915/gt/intel_reset.c
+++ b/drivers/gpu/drm/i915/gt/intel_reset.c
@@ -678,8 +678,13 @@ static void revoke_mmaps(struct intel_gt *gt)
 			continue;
 
 		GEM_BUG_ON(vma->fence != &gt->ggtt->fence_regs[i]);
-		node = &vma->obj->base.vma_node;
+
+		if (!vma->mmo)
+			continue;
+
+		node = &vma->mmo->vma_node;
 		vma_offset = vma->ggtt_view.partial.offset << PAGE_SHIFT;
+
 		unmap_mapping_range(gt->i915->drm.anon_inode->i_mapping,
 				    drm_vma_node_offset_addr(node) + vma_offset,
 				    vma->size,
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 8d7049792d62..3e129bcf8a12 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -61,6 +61,7 @@
 
 #include "gem/i915_gem_context.h"
 #include "gem/i915_gem_ioctls.h"
+#include "gem/i915_gem_mman.h"
 #include "gt/intel_gt.h"
 #include "gt/intel_gt_pm.h"
 #include "gt/intel_rc6.h"
@@ -2662,18 +2663,12 @@ const struct dev_pm_ops i915_pm_ops = {
 	.runtime_resume = intel_runtime_resume,
 };
 
-static const struct vm_operations_struct i915_gem_vm_ops = {
-	.fault = i915_gem_fault,
-	.open = drm_gem_vm_open,
-	.close = drm_gem_vm_close,
-};
-
 static const struct file_operations i915_driver_fops = {
 	.owner = THIS_MODULE,
 	.open = drm_open,
 	.release = drm_release,
 	.unlocked_ioctl = drm_ioctl,
-	.mmap = drm_gem_mmap,
+	.mmap = i915_gem_mmap,
 	.poll = drm_poll,
 	.read = drm_read,
 	.compat_ioctl = i915_compat_ioctl,
@@ -2762,7 +2757,6 @@ static struct drm_driver driver = {
 
 	.gem_close_object = i915_gem_close_object,
 	.gem_free_object_unlocked = i915_gem_free_object,
-	.gem_vm_ops = &i915_gem_vm_ops,
 
 	.prime_handle_to_fd = drm_gem_prime_handle_to_fd,
 	.prime_fd_to_handle = drm_gem_prime_fd_to_handle,
@@ -2773,7 +2767,6 @@ static struct drm_driver driver = {
 	.get_scanout_position = i915_get_crtc_scanoutpos,
 
 	.dumb_create = i915_gem_dumb_create,
-	.dumb_map_offset = i915_gem_mmap_gtt,
 	.ioctls = i915_ioctls,
 	.num_ioctls = ARRAY_SIZE(i915_ioctls),
 	.fops = &i915_driver_fops,
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index bbf4dfdfa8ba..a36c2778c09c 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1853,8 +1853,6 @@ i915_mutex_lock_interruptible(struct drm_device *dev)
 int i915_gem_dumb_create(struct drm_file *file_priv,
 			 struct drm_device *dev,
 			 struct drm_mode_create_dumb *args);
-int i915_gem_mmap_gtt(struct drm_file *file_priv, struct drm_device *dev,
-		      u32 handle, u64 *offset);
 int i915_gem_mmap_gtt_version(void);
 
 int __must_check i915_gem_set_global_seqno(struct drm_device *dev, u32 seqno);
@@ -1878,7 +1876,6 @@ void i915_gem_driver_release(struct drm_i915_private *dev_priv);
 void i915_gem_suspend(struct drm_i915_private *dev_priv);
 void i915_gem_suspend_late(struct drm_i915_private *dev_priv);
 void i915_gem_resume(struct drm_i915_private *dev_priv);
-vm_fault_t i915_gem_fault(struct vm_fault *vmf);
 
 int i915_gem_open(struct drm_i915_private *i915, struct drm_file *file);
 void i915_gem_release(struct drm_device *dev, struct drm_file *file);
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 1ba5f26700b0..ca4d4e012a62 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -44,6 +44,7 @@
 #include "gem/i915_gem_clflush.h"
 #include "gem/i915_gem_context.h"
 #include "gem/i915_gem_ioctls.h"
+#include "gem/i915_gem_mman.h"
 #include "gem/i915_gem_pm.h"
 #include "gt/intel_engine_user.h"
 #include "gt/intel_gt.h"
@@ -854,7 +855,7 @@ void i915_gem_runtime_suspend(struct drm_i915_private *i915)
 
 	list_for_each_entry_safe(obj, on,
 				 &i915->ggtt.userfault_list, userfault_link)
-		__i915_gem_object_release_mmap(obj);
+		__i915_gem_object_release_mmap_gtt(obj);
 
 	/*
 	 * The fence will be lost when the device powers down. If any were
diff --git a/drivers/gpu/drm/i915/i915_getparam.c b/drivers/gpu/drm/i915/i915_getparam.c
index cf8a8c3ef047..54fce81d5724 100644
--- a/drivers/gpu/drm/i915/i915_getparam.c
+++ b/drivers/gpu/drm/i915/i915_getparam.c
@@ -2,6 +2,7 @@
  * SPDX-License-Identifier: MIT
  */
 
+#include "gem/i915_gem_mman.h"
 #include "gt/intel_engine_user.h"
 
 #include "i915_drv.h"
diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c
index e5512f26e20a..5108dcb2e5b1 100644
--- a/drivers/gpu/drm/i915/i915_vma.c
+++ b/drivers/gpu/drm/i915/i915_vma.c
@@ -1054,7 +1054,7 @@ static void __i915_vma_iounmap(struct i915_vma *vma)
 
 void i915_vma_revoke_mmap(struct i915_vma *vma)
 {
-	struct drm_vma_offset_node *node = &vma->obj->base.vma_node;
+	struct drm_vma_offset_node *node;
 	u64 vma_offset;
 
 	lockdep_assert_held(&vma->vm->mutex);
@@ -1065,6 +1065,7 @@ void i915_vma_revoke_mmap(struct i915_vma *vma)
 	GEM_BUG_ON(!i915_vma_is_map_and_fenceable(vma));
 	GEM_BUG_ON(!vma->obj->userfault_count);
 
+	node = &vma->mmo->vma_node;
 	vma_offset = vma->ggtt_view.partial.offset << PAGE_SHIFT;
 	unmap_mapping_range(vma->vm->i915->drm.anon_inode->i_mapping,
 			    drm_vma_node_offset_addr(node) + vma_offset,
diff --git a/drivers/gpu/drm/i915/i915_vma.h b/drivers/gpu/drm/i915/i915_vma.h
index 465932813bc5..f09f4f513c41 100644
--- a/drivers/gpu/drm/i915/i915_vma.h
+++ b/drivers/gpu/drm/i915/i915_vma.h
@@ -63,6 +63,9 @@ struct i915_vma {
 	u64 display_alignment;
 	struct i915_page_sizes page_sizes;
 
+	/* mmap-offset associated with fencing for this vma */
+	struct i915_mmap_offset	*mmo;
+
 	u32 fence_size;
 	u32 fence_alignment;
 
-- 
2.23.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [Intel-gfx] [PATCH 1/4] drm/i915: Allow i915 to manage the vma offset nodes instead of drm core
@ 2019-11-19 11:37 ` Abdiel Janulgue
  0 siblings, 0 replies; 23+ messages in thread
From: Abdiel Janulgue @ 2019-11-19 11:37 UTC (permalink / raw)
  To: intel-gfx; +Cc: Matthew Auld

Have i915 replace the core drm_gem_mmap implementation to overcome its
limitation in having only a single mmap offset node per gem object.
This change allows us to have multiple mmap offsets per object and
enables a mmapping instance to use unique fault-handlers per user vma.

This allows i915 to store extra data within vma->vm_private_data and
assign the pagefault ops for each mmap instance allowing objects to use
multiple fault handlers depending on its backing storage.

v2:
 - Fix race condition exposed by gem_mmap_gtt@close-race. Simplify
   lifetime management of the mmap offset objects be ensuring it is
   owned by the parent gem object instead of refcounting.
 - Track mmo used by fencing to avoid locking when revoking mmaps
   during GPU reset.
 - Rebase.
v3:
 - Simplify mmo tracking
v4:
 - use vma->mmo in __i915_gem_object_release_mmap_gtt
v5:
 - Revoke CPU mmaps on i915_gem_object_unbind() since unlike GTT
   mmaps, they don't have bound i915_vma objects. Rebase.
v6: Minor tweaks, header re-org (Chris)
v7:
- Call drm_vma_node_revoke for the mmo with corresponding file when
  releasing the gem handle instead of in the mmo's teardown.
- Add flag I915_BO_READONLY instead of obj->readonly.

Signed-off-by: Abdiel Janulgue <abdiel.janulgue@linux.intel.com>
Cc: Matthew Auld <matthew.auld@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/gem/i915_gem_domain.c    |   3 +-
 drivers/gpu/drm/i915/gem/i915_gem_mman.c      | 241 +++++++++++++++---
 drivers/gpu/drm/i915/gem/i915_gem_mman.h      |  28 ++
 drivers/gpu/drm/i915/gem/i915_gem_object.c    |  18 ++
 drivers/gpu/drm/i915/gem/i915_gem_object.h    |   7 +-
 .../gpu/drm/i915/gem/i915_gem_object_types.h  |  19 ++
 drivers/gpu/drm/i915/gem/i915_gem_pages.c     |   3 +
 drivers/gpu/drm/i915/gem/i915_gem_tiling.c    |   1 +
 .../drm/i915/gem/selftests/i915_gem_mman.c    |  20 +-
 drivers/gpu/drm/i915/gt/intel_reset.c         |   7 +-
 drivers/gpu/drm/i915/i915_drv.c               |  11 +-
 drivers/gpu/drm/i915/i915_drv.h               |   3 -
 drivers/gpu/drm/i915/i915_gem.c               |   3 +-
 drivers/gpu/drm/i915/i915_getparam.c          |   1 +
 drivers/gpu/drm/i915/i915_vma.c               |   3 +-
 drivers/gpu/drm/i915/i915_vma.h               |   3 +
 16 files changed, 313 insertions(+), 58 deletions(-)
 create mode 100644 drivers/gpu/drm/i915/gem/i915_gem_mman.h

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_domain.c b/drivers/gpu/drm/i915/gem/i915_gem_domain.c
index e2af63af67ad..fd1c6f12b77a 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_domain.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_domain.c
@@ -13,6 +13,7 @@
 #include "i915_gem_object.h"
 #include "i915_vma.h"
 #include "i915_gem_lmem.h"
+#include "i915_gem_mman.h"
 
 static void __i915_gem_object_flush_for_display(struct drm_i915_gem_object *obj)
 {
@@ -255,7 +256,7 @@ int i915_gem_object_set_cache_level(struct drm_i915_gem_object *obj,
 			}
 
 			if (obj->userfault_count)
-				__i915_gem_object_release_mmap(obj);
+				__i915_gem_object_release_mmap_gtt(obj);
 
 			/*
 			 * As we no longer need a fence for GTT access,
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_mman.c b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
index d60973603cc1..36fffb671601 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_mman.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
@@ -14,6 +14,7 @@
 #include "i915_gem_gtt.h"
 #include "i915_gem_ioctls.h"
 #include "i915_gem_object.h"
+#include "i915_gem_mman.h"
 #include "i915_trace.h"
 #include "i915_vma.h"
 
@@ -219,7 +220,8 @@ vm_fault_t i915_gem_fault(struct vm_fault *vmf)
 {
 #define MIN_CHUNK_PAGES (SZ_1M >> PAGE_SHIFT)
 	struct vm_area_struct *area = vmf->vma;
-	struct drm_i915_gem_object *obj = to_intel_bo(area->vm_private_data);
+	struct i915_mmap_offset *priv = area->vm_private_data;
+	struct drm_i915_gem_object *obj = priv->obj;
 	struct drm_device *dev = obj->base.dev;
 	struct drm_i915_private *i915 = to_i915(dev);
 	struct intel_runtime_pm *rpm = &i915->runtime_pm;
@@ -312,6 +314,9 @@ vm_fault_t i915_gem_fault(struct vm_fault *vmf)
 		list_add(&obj->userfault_link, &i915->ggtt.userfault_list);
 	mutex_unlock(&i915->ggtt.vm.mutex);
 
+	/* Track the mmo associated with the fenced vma */
+	vma->mmo = priv;
+
 	if (IS_ACTIVE(CONFIG_DRM_I915_USERFAULT_AUTOSUSPEND))
 		intel_wakeref_auto(&i915->ggtt.userfault_wakeref,
 				   msecs_to_jiffies_timeout(CONFIG_DRM_I915_USERFAULT_AUTOSUSPEND));
@@ -358,28 +363,19 @@ vm_fault_t i915_gem_fault(struct vm_fault *vmf)
 	}
 }
 
-void __i915_gem_object_release_mmap(struct drm_i915_gem_object *obj)
+void __i915_gem_object_release_mmap_gtt(struct drm_i915_gem_object *obj)
 {
 	struct i915_vma *vma;
 
 	GEM_BUG_ON(!obj->userfault_count);
 
-	obj->userfault_count = 0;
-	list_del(&obj->userfault_link);
-	drm_vma_node_unmap(&obj->base.vma_node,
-			   obj->base.dev->anon_inode->i_mapping);
-
 	for_each_ggtt_vma(vma, obj)
-		i915_vma_unset_userfault(vma);
+		i915_vma_revoke_mmap(vma);
+
+	GEM_BUG_ON(obj->userfault_count);
 }
 
 /**
- * i915_gem_object_release_mmap - remove physical page mappings
- * @obj: obj in question
- *
- * Preserve the reservation of the mmapping with the DRM core code, but
- * relinquish ownership of the pages back to the system.
- *
  * It is vital that we remove the page mapping if we have mapped a tiled
  * object through the GTT and then lose the fence register due to
  * resource pressure. Similarly if the object has been moved out of the
@@ -387,7 +383,7 @@ void __i915_gem_object_release_mmap(struct drm_i915_gem_object *obj)
  * mapping will then trigger a page fault on the next user access, allowing
  * fixup by i915_gem_fault().
  */
-void i915_gem_object_release_mmap(struct drm_i915_gem_object *obj)
+static void i915_gem_object_release_mmap_gtt(struct drm_i915_gem_object *obj)
 {
 	struct drm_i915_private *i915 = to_i915(obj->base.dev);
 	intel_wakeref_t wakeref;
@@ -406,7 +402,7 @@ void i915_gem_object_release_mmap(struct drm_i915_gem_object *obj)
 	if (!obj->userfault_count)
 		goto out;
 
-	__i915_gem_object_release_mmap(obj);
+	__i915_gem_object_release_mmap_gtt(obj);
 
 	/* Ensure that the CPU's PTE are revoked and there are not outstanding
 	 * memory transactions from userspace before we return. The TLB
@@ -422,15 +418,63 @@ void i915_gem_object_release_mmap(struct drm_i915_gem_object *obj)
 	intel_runtime_pm_put(&i915->runtime_pm, wakeref);
 }
 
-static int create_mmap_offset(struct drm_i915_gem_object *obj)
+void i915_gem_object_release_mmap_offset(struct drm_i915_gem_object *obj)
+{
+	struct i915_mmap_offset *mmo;
+
+	mutex_lock(&obj->mmo_lock);
+	list_for_each_entry(mmo, &obj->mmap_offsets, offset) {
+		/* vma_node_unmap for GTT mmaps handled already in
+		 * __i915_gem_object_release_mmap_gtt
+		 */
+		if (mmo->mmap_type != I915_MMAP_TYPE_GTT)
+			drm_vma_node_unmap(&mmo->vma_node,
+					   obj->base.dev->anon_inode->i_mapping);
+	}
+	mutex_unlock(&obj->mmo_lock);
+}
+
+/**
+ * i915_gem_object_release_mmap - remove physical page mappings
+ * @obj: obj in question
+ *
+ * Preserve the reservation of the mmapping with the DRM core code, but
+ * relinquish ownership of the pages back to the system.
+ */
+void i915_gem_object_release_mmap(struct drm_i915_gem_object *obj)
+{
+	i915_gem_object_release_mmap_gtt(obj);
+	i915_gem_object_release_mmap_offset(obj);
+}
+
+static void init_mmap_offset(struct drm_i915_gem_object *obj,
+			     struct i915_mmap_offset *mmo)
+{
+	mutex_lock(&obj->mmo_lock);
+	list_add(&mmo->offset, &obj->mmap_offsets);
+	mutex_unlock(&obj->mmo_lock);
+
+	mmo->obj = obj;
+	mmo->dev = obj->base.dev;
+}
+
+static int create_mmap_offset(struct drm_i915_gem_object *obj,
+			      struct i915_mmap_offset *mmo)
 {
 	struct drm_i915_private *i915 = to_i915(obj->base.dev);
 	struct intel_gt *gt = &i915->gt;
+	struct drm_device *dev = obj->base.dev;
 	int err;
 
-	err = drm_gem_create_mmap_offset(&obj->base);
-	if (likely(!err))
+	drm_vma_node_reset(&mmo->vma_node);
+	if (mmo->file)
+		drm_vma_node_allow(&mmo->vma_node, mmo->file);
+	err = drm_vma_offset_add(dev->vma_offset_manager, &mmo->vma_node,
+				 obj->base.size / PAGE_SIZE);
+	if (likely(!err)) {
+		init_mmap_offset(obj, mmo);
 		return 0;
+	}
 
 	/* Attempt to reap some mmap space from dead objects */
 	err = intel_gt_retire_requests_timeout(gt, MAX_SCHEDULE_TIMEOUT);
@@ -438,34 +482,56 @@ static int create_mmap_offset(struct drm_i915_gem_object *obj)
 		return err;
 
 	i915_gem_drain_freed_objects(i915);
-	return drm_gem_create_mmap_offset(&obj->base);
+	err = drm_vma_offset_add(dev->vma_offset_manager, &mmo->vma_node,
+				 obj->base.size / PAGE_SIZE);
+	if (err)
+		return err;
+
+	init_mmap_offset(obj, mmo);
+	return 0;
 }
 
-int
-i915_gem_mmap_gtt(struct drm_file *file,
-		  struct drm_device *dev,
-		  u32 handle,
-		  u64 *offset)
+static int
+__assign_gem_object_mmap_data(struct drm_file *file,
+			      u32 handle,
+			      enum i915_mmap_type mmap_type,
+			      u64 *offset)
 {
 	struct drm_i915_gem_object *obj;
+	struct i915_mmap_offset *mmo;
 	int ret;
 
-	if (!i915_ggtt_has_aperture(&to_i915(dev)->ggtt))
-		return -ENODEV;
-
 	obj = i915_gem_object_lookup(file, handle);
 	if (!obj)
 		return -ENOENT;
 
-	if (i915_gem_object_never_bind_ggtt(obj)) {
+	if (mmap_type == I915_MMAP_TYPE_GTT &&
+	    i915_gem_object_never_bind_ggtt(obj)) {
 		ret = -ENODEV;
 		goto out;
 	}
 
-	ret = create_mmap_offset(obj);
-	if (ret == 0)
-		*offset = drm_vma_node_offset_addr(&obj->base.vma_node);
+	if (mmap_type != I915_MMAP_TYPE_GTT &&
+	    !i915_gem_object_has_struct_page(obj)) {
+		ret = -ENODEV;
+		goto out;
+	}
+
+	mmo = kzalloc(sizeof(*mmo), GFP_KERNEL);
+	if (!mmo) {
+		ret = -ENOMEM;
+		goto out;
+	}
+
+	mmo->file = file;
+	ret = create_mmap_offset(obj, mmo);
+	if (ret) {
+		kfree(mmo);
+		goto out;
+	}
 
+	mmo->mmap_type = mmap_type;
+	*offset = drm_vma_node_offset_addr(&mmo->vma_node);
 out:
 	i915_gem_object_put(obj);
 	return ret;
@@ -492,7 +558,116 @@ i915_gem_mmap_gtt_ioctl(struct drm_device *dev, void *data,
 {
 	struct drm_i915_gem_mmap_gtt *args = data;
 
-	return i915_gem_mmap_gtt(file, dev, args->handle, &args->offset);
+	return __assign_gem_object_mmap_data(file, args->handle,
+					     I915_MMAP_TYPE_GTT,
+					     &args->offset);
+}
+
+void i915_mmap_offset_destroy(struct i915_mmap_offset *mmo, struct mutex *mutex)
+{
+	drm_vma_offset_remove(mmo->dev->vma_offset_manager, &mmo->vma_node);
+
+	mutex_lock(mutex);
+	list_del(&mmo->offset);
+	mutex_unlock(mutex);
+
+	kfree(mmo);
+}
+
+static void i915_gem_vm_open(struct vm_area_struct *vma)
+{
+	struct i915_mmap_offset *mmo = vma->vm_private_data;
+	struct drm_i915_gem_object *obj = mmo->obj;
+
+	GEM_BUG_ON(!obj);
+	i915_gem_object_get(obj);
+}
+
+static void i915_gem_vm_close(struct vm_area_struct *vma)
+{
+	struct i915_mmap_offset *mmo = vma->vm_private_data;
+	struct drm_i915_gem_object *obj = mmo->obj;
+
+	GEM_BUG_ON(!obj);
+	i915_gem_object_put(obj);
+}
+
+static const struct vm_operations_struct i915_gem_gtt_vm_ops = {
+	.fault = i915_gem_fault,
+	.open = i915_gem_vm_open,
+	.close = i915_gem_vm_close,
+};
+
+/* This overcomes the limitation in drm_gem_mmap's assignment of a
+ * drm_gem_object as the vma->vm_private_data. Since we need to
+ * be able to resolve multiple mmap offsets which could be tied
+ * to a single gem object.
+ */
+int i915_gem_mmap(struct file *filp, struct vm_area_struct *vma)
+{
+	struct drm_vma_offset_node *node;
+	struct drm_file *priv = filp->private_data;
+	struct drm_device *dev = priv->minor->dev;
+	struct i915_mmap_offset *mmo = NULL;
+	struct drm_gem_object *obj = NULL;
+
+	if (drm_dev_is_unplugged(dev))
+		return -ENODEV;
+
+	drm_vma_offset_lock_lookup(dev->vma_offset_manager);
+	node = drm_vma_offset_exact_lookup_locked(dev->vma_offset_manager,
+						  vma->vm_pgoff,
+						  vma_pages(vma));
+	if (likely(node)) {
+		mmo = container_of(node, struct i915_mmap_offset,
+				   vma_node);
+		/*
+		 * In our dependency chain, the drm_vma_offset_node
+		 * depends on the validity of the mmo, which depends on
+		 * the gem object. However the only reference we have
+		 * at this point is the mmo (as the parent of the node).
+		 * Try to check if the gem object was at least cleared.
+		 */
+		if (!mmo || !mmo->obj) {
+			drm_vma_offset_unlock_lookup(dev->vma_offset_manager);
+			return -EINVAL;
+		}
+		/*
+		 * Skip 0-refcnted objects as it is in the process of being
+		 * destroyed and will be invalid when the vma manager lock
+		 * is released.
+		 */
+		obj = &mmo->obj->base;
+		if (!kref_get_unless_zero(&obj->refcount))
+			obj = NULL;
+
+	}
+	drm_vma_offset_unlock_lookup(dev->vma_offset_manager);
+
+	if (!obj)
+		return -EINVAL;
+
+	if (!drm_vma_node_is_allowed(node, priv)) {
+		drm_gem_object_put_unlocked(obj);
+		return -EACCES;
+	}
+
+	if (i915_gem_object_is_readonly(to_intel_bo(obj))) {
+		if (vma->vm_flags & VM_WRITE) {
+			drm_gem_object_put_unlocked(obj);
+			return -EINVAL;
+		}
+		vma->vm_flags &= ~VM_MAYWRITE;
+	}
+
+	vma->vm_flags |= VM_PFNMAP | VM_DONTEXPAND | VM_DONTDUMP;
+	vma->vm_page_prot = pgprot_writecombine(vm_get_page_prot(vma->vm_flags));
+	vma->vm_page_prot = pgprot_decrypted(vma->vm_page_prot);
+	vma->vm_private_data = mmo;
+
+	vma->vm_ops = &i915_gem_gtt_vm_ops;
+
+	return 0;
 }
 
 #if IS_ENABLED(CONFIG_DRM_I915_SELFTEST)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_mman.h b/drivers/gpu/drm/i915/gem/i915_gem_mman.h
new file mode 100644
index 000000000000..25a3c4d6cd65
--- /dev/null
+++ b/drivers/gpu/drm/i915/gem/i915_gem_mman.h
@@ -0,0 +1,28 @@
+/*
+ * SPDX-License-Identifier: MIT
+ *
+ * Copyright © 2019 Intel Corporation
+ */
+
+#ifndef __I915_GEM_MMAN_H__
+#define __I915_GEM_MMAN_H__
+
+#include <linux/mm_types.h>
+#include <linux/types.h>
+
+struct drm_device;
+struct drm_file;
+struct drm_i915_gem_object;
+struct file;
+struct i915_mmap_offset;
+struct mutex;
+
+int i915_gem_mmap(struct file *filp, struct vm_area_struct *vma);
+vm_fault_t i915_gem_fault(struct vm_fault *vmf);
+void i915_mmap_offset_destroy(struct i915_mmap_offset *mmo, struct mutex *mutex);
+
+void __i915_gem_object_release_mmap_gtt(struct drm_i915_gem_object *obj);
+void i915_gem_object_release_mmap(struct drm_i915_gem_object *obj);
+void i915_gem_object_release_mmap_offset(struct drm_i915_gem_object *obj);
+
+#endif
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.c b/drivers/gpu/drm/i915/gem/i915_gem_object.c
index db103d3c8760..44cb5a2b140d 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_object.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_object.c
@@ -29,6 +29,7 @@
 #include "i915_drv.h"
 #include "i915_gem_clflush.h"
 #include "i915_gem_context.h"
+#include "i915_gem_mman.h"
 #include "i915_gem_object.h"
 #include "i915_globals.h"
 #include "i915_trace.h"
@@ -61,6 +62,9 @@ void i915_gem_object_init(struct drm_i915_gem_object *obj,
 
 	INIT_LIST_HEAD(&obj->lut_list);
 
+	mutex_init(&obj->mmo_lock);
+	INIT_LIST_HEAD(&obj->mmap_offsets);
+
 	init_rcu_head(&obj->rcu);
 
 	obj->ops = ops;
@@ -97,6 +101,7 @@ void i915_gem_close_object(struct drm_gem_object *gem, struct drm_file *file)
 	struct drm_i915_gem_object *obj = to_intel_bo(gem);
 	struct drm_i915_file_private *fpriv = file->driver_priv;
 	struct i915_lut_handle *lut, *ln;
+	struct i915_mmap_offset *mmo, *on;
 	LIST_HEAD(close);
 
 	i915_gem_object_lock(obj);
@@ -111,6 +116,9 @@ void i915_gem_close_object(struct drm_gem_object *gem, struct drm_file *file)
 	}
 	i915_gem_object_unlock(obj);
 
+	list_for_each_entry_safe(mmo, on, &obj->mmap_offsets, offset)
+		drm_vma_node_revoke(&mmo->vma_node, file);
+
 	list_for_each_entry_safe(lut, ln, &close, obj_link) {
 		struct i915_gem_context *ctx = lut->ctx;
 		struct i915_vma *vma;
@@ -158,6 +166,8 @@ static void __i915_gem_free_objects(struct drm_i915_private *i915,
 
 	wakeref = intel_runtime_pm_get(&i915->runtime_pm);
 	llist_for_each_entry_safe(obj, on, freed, freed) {
+		struct i915_mmap_offset *mmo, *mn;
+
 		trace_i915_gem_object_destroy(obj);
 
 		if (!list_empty(&obj->vma.list)) {
@@ -183,6 +193,14 @@ static void __i915_gem_free_objects(struct drm_i915_private *i915,
 			spin_unlock(&obj->vma.lock);
 		}
 
+		i915_gem_object_release_mmap(obj);
+
+		list_for_each_entry_safe(mmo, mn, &obj->mmap_offsets, offset) {
+			mmo->obj = NULL;
+			i915_mmap_offset_destroy(mmo, &obj->mmo_lock);
+		}
+
+		GEM_BUG_ON(!list_empty(&obj->mmap_offsets));
 		GEM_BUG_ON(atomic_read(&obj->bind_count));
 		GEM_BUG_ON(obj->userfault_count);
 		GEM_BUG_ON(!list_empty(&obj->lut_list));
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.h b/drivers/gpu/drm/i915/gem/i915_gem_object.h
index e5750d506cc9..a1eb7c0b23ac 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_object.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_object.h
@@ -132,13 +132,13 @@ void i915_gem_object_unlock_fence(struct drm_i915_gem_object *obj,
 static inline void
 i915_gem_object_set_readonly(struct drm_i915_gem_object *obj)
 {
-	obj->base.vma_node.readonly = true;
+	obj->flags |= I915_BO_READONLY;
 }
 
 static inline bool
 i915_gem_object_is_readonly(const struct drm_i915_gem_object *obj)
 {
-	return obj->base.vma_node.readonly;
+	return obj->flags & I915_BO_READONLY;
 }
 
 static inline bool
@@ -387,9 +387,6 @@ static inline void i915_gem_object_unpin_map(struct drm_i915_gem_object *obj)
 	i915_gem_object_unpin_pages(obj);
 }
 
-void __i915_gem_object_release_mmap(struct drm_i915_gem_object *obj);
-void i915_gem_object_release_mmap(struct drm_i915_gem_object *obj);
-
 void
 i915_gem_object_flush_write_domain(struct drm_i915_gem_object *obj,
 				   unsigned int flush_domains);
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 15f8297dc34e..8ff0834a1d5c 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
@@ -63,6 +63,20 @@ struct drm_i915_gem_object_ops {
 	void (*release)(struct drm_i915_gem_object *obj);
 };
 
+enum i915_mmap_type {
+	I915_MMAP_TYPE_GTT = 0,
+};
+
+struct i915_mmap_offset {
+	struct drm_device *dev;
+	struct drm_vma_offset_node vma_node;
+	struct drm_i915_gem_object *obj;
+	struct drm_file *file;
+	enum i915_mmap_type mmap_type;
+
+	struct list_head offset;
+};
+
 struct drm_i915_gem_object {
 	struct drm_gem_object base;
 
@@ -118,12 +132,17 @@ struct drm_i915_gem_object {
 	unsigned int userfault_count;
 	struct list_head userfault_link;
 
+	/* Protects access to mmap offsets */
+	struct mutex mmo_lock;
+	struct list_head mmap_offsets;
+
 	I915_SELFTEST_DECLARE(struct list_head st_link);
 
 	unsigned long flags;
 #define I915_BO_ALLOC_CONTIGUOUS BIT(0)
 #define I915_BO_ALLOC_VOLATILE   BIT(1)
 #define I915_BO_ALLOC_FLAGS (I915_BO_ALLOC_CONTIGUOUS | I915_BO_ALLOC_VOLATILE)
+#define I915_BO_READONLY         BIT(2)
 
 	/*
 	 * Is the object to be mapped as read-only to the GPU
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_pages.c b/drivers/gpu/drm/i915/gem/i915_gem_pages.c
index f402c2c415c2..75197ca696a8 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_pages.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_pages.c
@@ -8,6 +8,7 @@
 #include "i915_gem_object.h"
 #include "i915_scatterlist.h"
 #include "i915_gem_lmem.h"
+#include "i915_gem_mman.h"
 
 void __i915_gem_object_set_pages(struct drm_i915_gem_object *obj,
 				 struct sg_table *pages,
@@ -207,6 +208,8 @@ int __i915_gem_object_put_pages(struct drm_i915_gem_object *obj)
 		goto unlock;
 	}
 
+	i915_gem_object_release_mmap_offset(obj);
+
 	/*
 	 * ->put_pages might need to allocate memory for the bit17 swizzle
 	 * array, hence protect them from being reaped by removing them from gtt
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_tiling.c b/drivers/gpu/drm/i915/gem/i915_gem_tiling.c
index 1fa592d82af5..6c7825a2dc2a 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_tiling.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_tiling.c
@@ -11,6 +11,7 @@
 #include "i915_drv.h"
 #include "i915_gem.h"
 #include "i915_gem_ioctls.h"
+#include "i915_gem_mman.h"
 #include "i915_gem_object.h"
 
 /**
diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c b/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c
index 9f1a69027a04..6c2333c61469 100644
--- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c
+++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c
@@ -564,15 +564,20 @@ static bool assert_mmap_offset(struct drm_i915_private *i915,
 			       int expected)
 {
 	struct drm_i915_gem_object *obj;
+	/* Ownership transferred to parent gem object in create_mmap_offset */
+	struct i915_mmap_offset *mmo = kzalloc(sizeof(*mmo), GFP_KERNEL);
 	int err;
 
 	obj = i915_gem_object_create_internal(i915, size);
 	if (IS_ERR(obj))
 		return PTR_ERR(obj);
 
-	err = create_mmap_offset(obj);
+	err = create_mmap_offset(obj, mmo);
+	if (err)
+		kfree(mmo);
 	i915_gem_object_put(obj);
 
+
 	return err == expected;
 }
 
@@ -608,6 +613,8 @@ static int igt_mmap_offset_exhaustion(void *arg)
 	struct drm_mm *mm = &i915->drm.vma_offset_manager->vm_addr_space_mm;
 	struct drm_i915_gem_object *obj;
 	struct drm_mm_node *hole, *next;
+	/* Ownership transferred to parent gem object in create_mmap_offset */
+	struct i915_mmap_offset *mmo = kzalloc(sizeof(*mmo), GFP_KERNEL);
 	int loop, err;
 
 	/* Disable background reaper */
@@ -672,9 +679,10 @@ static int igt_mmap_offset_exhaustion(void *arg)
 		goto out;
 	}
 
-	err = create_mmap_offset(obj);
+	err = create_mmap_offset(obj, mmo);
 	if (err) {
 		pr_err("Unable to insert object into reclaimed hole\n");
+		kfree(mmo);
 		goto err_obj;
 	}
 
@@ -728,6 +736,8 @@ static int igt_mmap_gtt(void *arg)
 	struct drm_i915_private *i915 = arg;
 	struct drm_i915_gem_object *obj;
 	struct vm_area_struct *area;
+	/* Ownership transferred to parent gem object in create_mmap_offset */
+	struct i915_mmap_offset *mmo = kzalloc(sizeof(*mmo), GFP_KERNEL);
 	unsigned long addr;
 	void *vaddr;
 	int err, i;
@@ -748,7 +758,7 @@ static int igt_mmap_gtt(void *arg)
 	i915_gem_object_flush_map(obj);
 	i915_gem_object_unpin_map(obj);
 
-	err = create_mmap_offset(obj);
+	err = create_mmap_offset(obj, mmo);
 	if (err)
 		goto out;
 
@@ -876,6 +886,8 @@ static int igt_mmap_gtt_revoke(void *arg)
 {
 	struct drm_i915_private *i915 = arg;
 	struct drm_i915_gem_object *obj;
+	/* Ownership transferred to parent gem object in create_mmap_offset */
+	struct i915_mmap_offset *mmo = kzalloc(sizeof(*mmo), GFP_KERNEL);
 	unsigned long addr;
 	int err;
 
@@ -886,7 +898,7 @@ static int igt_mmap_gtt_revoke(void *arg)
 	if (IS_ERR(obj))
 		return PTR_ERR(obj);
 
-	err = create_mmap_offset(obj);
+	err = create_mmap_offset(obj, mmo);
 	if (err)
 		goto out;
 
diff --git a/drivers/gpu/drm/i915/gt/intel_reset.c b/drivers/gpu/drm/i915/gt/intel_reset.c
index b7007cd78c6f..777eaa483676 100644
--- a/drivers/gpu/drm/i915/gt/intel_reset.c
+++ b/drivers/gpu/drm/i915/gt/intel_reset.c
@@ -678,8 +678,13 @@ static void revoke_mmaps(struct intel_gt *gt)
 			continue;
 
 		GEM_BUG_ON(vma->fence != &gt->ggtt->fence_regs[i]);
-		node = &vma->obj->base.vma_node;
+
+		if (!vma->mmo)
+			continue;
+
+		node = &vma->mmo->vma_node;
 		vma_offset = vma->ggtt_view.partial.offset << PAGE_SHIFT;
+
 		unmap_mapping_range(gt->i915->drm.anon_inode->i_mapping,
 				    drm_vma_node_offset_addr(node) + vma_offset,
 				    vma->size,
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 8d7049792d62..3e129bcf8a12 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -61,6 +61,7 @@
 
 #include "gem/i915_gem_context.h"
 #include "gem/i915_gem_ioctls.h"
+#include "gem/i915_gem_mman.h"
 #include "gt/intel_gt.h"
 #include "gt/intel_gt_pm.h"
 #include "gt/intel_rc6.h"
@@ -2662,18 +2663,12 @@ const struct dev_pm_ops i915_pm_ops = {
 	.runtime_resume = intel_runtime_resume,
 };
 
-static const struct vm_operations_struct i915_gem_vm_ops = {
-	.fault = i915_gem_fault,
-	.open = drm_gem_vm_open,
-	.close = drm_gem_vm_close,
-};
-
 static const struct file_operations i915_driver_fops = {
 	.owner = THIS_MODULE,
 	.open = drm_open,
 	.release = drm_release,
 	.unlocked_ioctl = drm_ioctl,
-	.mmap = drm_gem_mmap,
+	.mmap = i915_gem_mmap,
 	.poll = drm_poll,
 	.read = drm_read,
 	.compat_ioctl = i915_compat_ioctl,
@@ -2762,7 +2757,6 @@ static struct drm_driver driver = {
 
 	.gem_close_object = i915_gem_close_object,
 	.gem_free_object_unlocked = i915_gem_free_object,
-	.gem_vm_ops = &i915_gem_vm_ops,
 
 	.prime_handle_to_fd = drm_gem_prime_handle_to_fd,
 	.prime_fd_to_handle = drm_gem_prime_fd_to_handle,
@@ -2773,7 +2767,6 @@ static struct drm_driver driver = {
 	.get_scanout_position = i915_get_crtc_scanoutpos,
 
 	.dumb_create = i915_gem_dumb_create,
-	.dumb_map_offset = i915_gem_mmap_gtt,
 	.ioctls = i915_ioctls,
 	.num_ioctls = ARRAY_SIZE(i915_ioctls),
 	.fops = &i915_driver_fops,
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index bbf4dfdfa8ba..a36c2778c09c 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1853,8 +1853,6 @@ i915_mutex_lock_interruptible(struct drm_device *dev)
 int i915_gem_dumb_create(struct drm_file *file_priv,
 			 struct drm_device *dev,
 			 struct drm_mode_create_dumb *args);
-int i915_gem_mmap_gtt(struct drm_file *file_priv, struct drm_device *dev,
-		      u32 handle, u64 *offset);
 int i915_gem_mmap_gtt_version(void);
 
 int __must_check i915_gem_set_global_seqno(struct drm_device *dev, u32 seqno);
@@ -1878,7 +1876,6 @@ void i915_gem_driver_release(struct drm_i915_private *dev_priv);
 void i915_gem_suspend(struct drm_i915_private *dev_priv);
 void i915_gem_suspend_late(struct drm_i915_private *dev_priv);
 void i915_gem_resume(struct drm_i915_private *dev_priv);
-vm_fault_t i915_gem_fault(struct vm_fault *vmf);
 
 int i915_gem_open(struct drm_i915_private *i915, struct drm_file *file);
 void i915_gem_release(struct drm_device *dev, struct drm_file *file);
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 1ba5f26700b0..ca4d4e012a62 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -44,6 +44,7 @@
 #include "gem/i915_gem_clflush.h"
 #include "gem/i915_gem_context.h"
 #include "gem/i915_gem_ioctls.h"
+#include "gem/i915_gem_mman.h"
 #include "gem/i915_gem_pm.h"
 #include "gt/intel_engine_user.h"
 #include "gt/intel_gt.h"
@@ -854,7 +855,7 @@ void i915_gem_runtime_suspend(struct drm_i915_private *i915)
 
 	list_for_each_entry_safe(obj, on,
 				 &i915->ggtt.userfault_list, userfault_link)
-		__i915_gem_object_release_mmap(obj);
+		__i915_gem_object_release_mmap_gtt(obj);
 
 	/*
 	 * The fence will be lost when the device powers down. If any were
diff --git a/drivers/gpu/drm/i915/i915_getparam.c b/drivers/gpu/drm/i915/i915_getparam.c
index cf8a8c3ef047..54fce81d5724 100644
--- a/drivers/gpu/drm/i915/i915_getparam.c
+++ b/drivers/gpu/drm/i915/i915_getparam.c
@@ -2,6 +2,7 @@
  * SPDX-License-Identifier: MIT
  */
 
+#include "gem/i915_gem_mman.h"
 #include "gt/intel_engine_user.h"
 
 #include "i915_drv.h"
diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c
index e5512f26e20a..5108dcb2e5b1 100644
--- a/drivers/gpu/drm/i915/i915_vma.c
+++ b/drivers/gpu/drm/i915/i915_vma.c
@@ -1054,7 +1054,7 @@ static void __i915_vma_iounmap(struct i915_vma *vma)
 
 void i915_vma_revoke_mmap(struct i915_vma *vma)
 {
-	struct drm_vma_offset_node *node = &vma->obj->base.vma_node;
+	struct drm_vma_offset_node *node;
 	u64 vma_offset;
 
 	lockdep_assert_held(&vma->vm->mutex);
@@ -1065,6 +1065,7 @@ void i915_vma_revoke_mmap(struct i915_vma *vma)
 	GEM_BUG_ON(!i915_vma_is_map_and_fenceable(vma));
 	GEM_BUG_ON(!vma->obj->userfault_count);
 
+	node = &vma->mmo->vma_node;
 	vma_offset = vma->ggtt_view.partial.offset << PAGE_SHIFT;
 	unmap_mapping_range(vma->vm->i915->drm.anon_inode->i_mapping,
 			    drm_vma_node_offset_addr(node) + vma_offset,
diff --git a/drivers/gpu/drm/i915/i915_vma.h b/drivers/gpu/drm/i915/i915_vma.h
index 465932813bc5..f09f4f513c41 100644
--- a/drivers/gpu/drm/i915/i915_vma.h
+++ b/drivers/gpu/drm/i915/i915_vma.h
@@ -63,6 +63,9 @@ struct i915_vma {
 	u64 display_alignment;
 	struct i915_page_sizes page_sizes;
 
+	/* mmap-offset associated with fencing for this vma */
+	struct i915_mmap_offset	*mmo;
+
 	u32 fence_size;
 	u32 fence_alignment;
 
-- 
2.23.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 2/4] drm/i915: Introduce DRM_I915_GEM_MMAP_OFFSET
@ 2019-11-19 11:37   ` Abdiel Janulgue
  0 siblings, 0 replies; 23+ messages in thread
From: Abdiel Janulgue @ 2019-11-19 11:37 UTC (permalink / raw)
  To: intel-gfx; +Cc: Matthew Auld

This is really just an alias of mmap_gtt. The 'mmap offset' nomenclature
comes from the value returned by this ioctl which is the offset into the
device fd which userpace uses with mmap(2).

mmap_gtt was our initial mmap_offset implementation, this extends
our CPU mmap support to allow additional fault handlers that depends on
the object's backing pages.

Note that we multiplex mmap_gtt and mmap_offset through the same ioctl,
and use the zero extending behaviour of drm to differentiate between
them, when we inspect the flags.

v2:
- Drop the alias, just rename the struct (Chris)
- Don't bail out on no PAT when doing WB mmaps
- Prepare uAPI for further extensions
v3:
- drop MMAP_OFFSET_FLAGS
v4:
- Tweaks, header re-org

Signed-off-by: Abdiel Janulgue <abdiel.janulgue@linux.intel.com>
Signed-off-by: Matthew Auld <matthew.auld@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
 drivers/gpu/drm/i915/gem/i915_gem_ioctls.h    |  4 +-
 drivers/gpu/drm/i915/gem/i915_gem_mman.c      | 45 ++++++++++++++++---
 drivers/gpu/drm/i915/gem/i915_gem_mman.h      |  1 +
 .../gpu/drm/i915/gem/i915_gem_object_types.h  |  3 ++
 drivers/gpu/drm/i915/i915_drv.c               |  2 +-
 drivers/gpu/drm/i915/i915_drv.h               |  1 -
 include/uapi/drm/i915_drm.h                   | 27 +++++++++++
 7 files changed, 72 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ioctls.h b/drivers/gpu/drm/i915/gem/i915_gem_ioctls.h
index ddc7f2a52b3e..87d8b27f426d 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_ioctls.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_ioctls.h
@@ -28,8 +28,8 @@ int i915_gem_madvise_ioctl(struct drm_device *dev, void *data,
 			   struct drm_file *file);
 int i915_gem_mmap_ioctl(struct drm_device *dev, void *data,
 			struct drm_file *file);
-int i915_gem_mmap_gtt_ioctl(struct drm_device *dev, void *data,
-			    struct drm_file *file);
+int i915_gem_mmap_offset_ioctl(struct drm_device *dev, void *data,
+			       struct drm_file *file);
 int i915_gem_pread_ioctl(struct drm_device *dev, void *data,
 			 struct drm_file *file);
 int i915_gem_pwrite_ioctl(struct drm_device *dev, void *data,
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_mman.c b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
index 36fffb671601..bb05c53c03c8 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_mman.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
@@ -145,6 +145,9 @@ static unsigned int tile_row_pages(const struct drm_i915_gem_object *obj)
  * 3 - Remove implicit set-domain(GTT) and synchronisation on initial
  *     pagefault; swapin remains transparent.
  *
+ * 4 - Support multiple fault handlers per object depending on object's
+ *     backing storage (a.k.a. MMAP_OFFSET).
+ *
  * Restrictions:
  *
  *  * snoopable objects cannot be accessed via the GTT. It can cause machine
@@ -172,7 +175,7 @@ static unsigned int tile_row_pages(const struct drm_i915_gem_object *obj)
  */
 int i915_gem_mmap_gtt_version(void)
 {
-	return 3;
+	return 4;
 }
 
 static inline struct i915_ggtt_view
@@ -538,7 +541,7 @@ __assign_gem_object_mmap_data(struct drm_file *file,
 }
 
 /**
- * i915_gem_mmap_gtt_ioctl - prepare an object for GTT mmap'ing
+ * i915_gem_mmap_offset_ioctl - prepare an object for GTT mmap'ing
  * @dev: DRM device
  * @data: GTT mapping ioctl data
  * @file: GEM object info
@@ -553,13 +556,41 @@ __assign_gem_object_mmap_data(struct drm_file *file,
  * userspace.
  */
 int
-i915_gem_mmap_gtt_ioctl(struct drm_device *dev, void *data,
-			struct drm_file *file)
+i915_gem_mmap_offset_ioctl(struct drm_device *dev, void *data,
+			   struct drm_file *file)
 {
-	struct drm_i915_gem_mmap_gtt *args = data;
+	struct drm_i915_private *i915 = to_i915(dev);
+	struct drm_i915_gem_mmap_offset *args = data;
+	enum i915_mmap_type type;
+
+	switch (args->flags) {
+	case I915_MMAP_OFFSET_GTT:
+		if (!i915_ggtt_has_aperture(&i915->ggtt))
+			return -ENODEV;
+		type = I915_MMAP_TYPE_GTT;
+		break;
+
+	case I915_MMAP_OFFSET_WC:
+		if (!boot_cpu_has(X86_FEATURE_PAT))
+			return -ENODEV;
+		type = I915_MMAP_TYPE_WC;
+		break;
+
+	case I915_MMAP_OFFSET_WB:
+		type = I915_MMAP_TYPE_WB;
+		break;
+
+	case I915_MMAP_OFFSET_UC:
+		if (!boot_cpu_has(X86_FEATURE_PAT))
+			return -ENODEV;
+		type = I915_MMAP_TYPE_UC;
+		break;
+
+	default:
+		return -EINVAL;
+	}
 
-	return __assign_gem_object_mmap_data(file, args->handle,
-					     I915_MMAP_TYPE_GTT,
+	return __assign_gem_object_mmap_data(file, args->handle, type,
 					     &args->offset);
 }
 
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_mman.h b/drivers/gpu/drm/i915/gem/i915_gem_mman.h
index 25a3c4d6cd65..4d3b493e853a 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_mman.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_mman.h
@@ -24,5 +24,6 @@ void i915_mmap_offset_destroy(struct i915_mmap_offset *mmo, struct mutex *mutex)
 void __i915_gem_object_release_mmap_gtt(struct drm_i915_gem_object *obj);
 void i915_gem_object_release_mmap(struct drm_i915_gem_object *obj);
 void i915_gem_object_release_mmap_offset(struct drm_i915_gem_object *obj);
+int i915_gem_mmap_gtt_version(void);
 
 #endif
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 8ff0834a1d5c..c0beb97b56d1 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
@@ -65,6 +65,9 @@ struct drm_i915_gem_object_ops {
 
 enum i915_mmap_type {
 	I915_MMAP_TYPE_GTT = 0,
+	I915_MMAP_TYPE_WC,
+	I915_MMAP_TYPE_WB,
+	I915_MMAP_TYPE_UC,
 };
 
 struct i915_mmap_offset {
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 3e129bcf8a12..ac6d4470ce75 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -2715,7 +2715,7 @@ static const struct drm_ioctl_desc i915_ioctls[] = {
 	DRM_IOCTL_DEF_DRV(I915_GEM_PREAD, i915_gem_pread_ioctl, DRM_RENDER_ALLOW),
 	DRM_IOCTL_DEF_DRV(I915_GEM_PWRITE, i915_gem_pwrite_ioctl, DRM_RENDER_ALLOW),
 	DRM_IOCTL_DEF_DRV(I915_GEM_MMAP, i915_gem_mmap_ioctl, DRM_RENDER_ALLOW),
-	DRM_IOCTL_DEF_DRV(I915_GEM_MMAP_GTT, i915_gem_mmap_gtt_ioctl, DRM_RENDER_ALLOW),
+	DRM_IOCTL_DEF_DRV(I915_GEM_MMAP_OFFSET, i915_gem_mmap_offset_ioctl, DRM_RENDER_ALLOW),
 	DRM_IOCTL_DEF_DRV(I915_GEM_SET_DOMAIN, i915_gem_set_domain_ioctl, DRM_RENDER_ALLOW),
 	DRM_IOCTL_DEF_DRV(I915_GEM_SW_FINISH, i915_gem_sw_finish_ioctl, DRM_RENDER_ALLOW),
 	DRM_IOCTL_DEF_DRV(I915_GEM_SET_TILING, i915_gem_set_tiling_ioctl, DRM_RENDER_ALLOW),
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index a36c2778c09c..b2af6df2eae0 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1853,7 +1853,6 @@ i915_mutex_lock_interruptible(struct drm_device *dev)
 int i915_gem_dumb_create(struct drm_file *file_priv,
 			 struct drm_device *dev,
 			 struct drm_mode_create_dumb *args);
-int i915_gem_mmap_gtt_version(void);
 
 int __must_check i915_gem_set_global_seqno(struct drm_device *dev, u32 seqno);
 
diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index 5400d7e057f1..e844c3a8d197 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -395,6 +395,7 @@ typedef struct _drm_i915_sarea {
 #define DRM_IOCTL_I915_GEM_PWRITE	DRM_IOW (DRM_COMMAND_BASE + DRM_I915_GEM_PWRITE, struct drm_i915_gem_pwrite)
 #define DRM_IOCTL_I915_GEM_MMAP		DRM_IOWR(DRM_COMMAND_BASE + DRM_I915_GEM_MMAP, struct drm_i915_gem_mmap)
 #define DRM_IOCTL_I915_GEM_MMAP_GTT	DRM_IOWR(DRM_COMMAND_BASE + DRM_I915_GEM_MMAP_GTT, struct drm_i915_gem_mmap_gtt)
+#define DRM_IOCTL_I915_GEM_MMAP_OFFSET	DRM_IOWR(DRM_COMMAND_BASE + DRM_I915_GEM_MMAP_GTT, struct drm_i915_gem_mmap_offset)
 #define DRM_IOCTL_I915_GEM_SET_DOMAIN	DRM_IOW (DRM_COMMAND_BASE + DRM_I915_GEM_SET_DOMAIN, struct drm_i915_gem_set_domain)
 #define DRM_IOCTL_I915_GEM_SW_FINISH	DRM_IOW (DRM_COMMAND_BASE + DRM_I915_GEM_SW_FINISH, struct drm_i915_gem_sw_finish)
 #define DRM_IOCTL_I915_GEM_SET_TILING	DRM_IOWR (DRM_COMMAND_BASE + DRM_I915_GEM_SET_TILING, struct drm_i915_gem_set_tiling)
@@ -793,6 +794,32 @@ struct drm_i915_gem_mmap_gtt {
 	__u64 offset;
 };
 
+struct drm_i915_gem_mmap_offset {
+	/** Handle for the object being mapped. */
+	__u32 handle;
+	__u32 pad;
+	/**
+	 * Fake offset to use for subsequent mmap call
+	 *
+	 * This is a fixed-size type for 32/64 compatibility.
+	 */
+	__u64 offset;
+
+	/**
+	 * Flags for extended behaviour.
+	 *
+	 * It is mandatory that either one of the MMAP_OFFSET flags
+	 * should be passed here.
+	 */
+	__u64 flags;
+#define I915_MMAP_OFFSET_GTT 0
+#define I915_MMAP_OFFSET_WC  1
+#define I915_MMAP_OFFSET_WB  2
+#define I915_MMAP_OFFSET_UC  3
+
+	__u64 extensions;
+};
+
 struct drm_i915_gem_set_domain {
 	/** Handle for the object */
 	__u32 handle;
-- 
2.23.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [Intel-gfx] [PATCH 2/4] drm/i915: Introduce DRM_I915_GEM_MMAP_OFFSET
@ 2019-11-19 11:37   ` Abdiel Janulgue
  0 siblings, 0 replies; 23+ messages in thread
From: Abdiel Janulgue @ 2019-11-19 11:37 UTC (permalink / raw)
  To: intel-gfx; +Cc: Matthew Auld

This is really just an alias of mmap_gtt. The 'mmap offset' nomenclature
comes from the value returned by this ioctl which is the offset into the
device fd which userpace uses with mmap(2).

mmap_gtt was our initial mmap_offset implementation, this extends
our CPU mmap support to allow additional fault handlers that depends on
the object's backing pages.

Note that we multiplex mmap_gtt and mmap_offset through the same ioctl,
and use the zero extending behaviour of drm to differentiate between
them, when we inspect the flags.

v2:
- Drop the alias, just rename the struct (Chris)
- Don't bail out on no PAT when doing WB mmaps
- Prepare uAPI for further extensions
v3:
- drop MMAP_OFFSET_FLAGS
v4:
- Tweaks, header re-org

Signed-off-by: Abdiel Janulgue <abdiel.janulgue@linux.intel.com>
Signed-off-by: Matthew Auld <matthew.auld@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
 drivers/gpu/drm/i915/gem/i915_gem_ioctls.h    |  4 +-
 drivers/gpu/drm/i915/gem/i915_gem_mman.c      | 45 ++++++++++++++++---
 drivers/gpu/drm/i915/gem/i915_gem_mman.h      |  1 +
 .../gpu/drm/i915/gem/i915_gem_object_types.h  |  3 ++
 drivers/gpu/drm/i915/i915_drv.c               |  2 +-
 drivers/gpu/drm/i915/i915_drv.h               |  1 -
 include/uapi/drm/i915_drm.h                   | 27 +++++++++++
 7 files changed, 72 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ioctls.h b/drivers/gpu/drm/i915/gem/i915_gem_ioctls.h
index ddc7f2a52b3e..87d8b27f426d 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_ioctls.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_ioctls.h
@@ -28,8 +28,8 @@ int i915_gem_madvise_ioctl(struct drm_device *dev, void *data,
 			   struct drm_file *file);
 int i915_gem_mmap_ioctl(struct drm_device *dev, void *data,
 			struct drm_file *file);
-int i915_gem_mmap_gtt_ioctl(struct drm_device *dev, void *data,
-			    struct drm_file *file);
+int i915_gem_mmap_offset_ioctl(struct drm_device *dev, void *data,
+			       struct drm_file *file);
 int i915_gem_pread_ioctl(struct drm_device *dev, void *data,
 			 struct drm_file *file);
 int i915_gem_pwrite_ioctl(struct drm_device *dev, void *data,
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_mman.c b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
index 36fffb671601..bb05c53c03c8 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_mman.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
@@ -145,6 +145,9 @@ static unsigned int tile_row_pages(const struct drm_i915_gem_object *obj)
  * 3 - Remove implicit set-domain(GTT) and synchronisation on initial
  *     pagefault; swapin remains transparent.
  *
+ * 4 - Support multiple fault handlers per object depending on object's
+ *     backing storage (a.k.a. MMAP_OFFSET).
+ *
  * Restrictions:
  *
  *  * snoopable objects cannot be accessed via the GTT. It can cause machine
@@ -172,7 +175,7 @@ static unsigned int tile_row_pages(const struct drm_i915_gem_object *obj)
  */
 int i915_gem_mmap_gtt_version(void)
 {
-	return 3;
+	return 4;
 }
 
 static inline struct i915_ggtt_view
@@ -538,7 +541,7 @@ __assign_gem_object_mmap_data(struct drm_file *file,
 }
 
 /**
- * i915_gem_mmap_gtt_ioctl - prepare an object for GTT mmap'ing
+ * i915_gem_mmap_offset_ioctl - prepare an object for GTT mmap'ing
  * @dev: DRM device
  * @data: GTT mapping ioctl data
  * @file: GEM object info
@@ -553,13 +556,41 @@ __assign_gem_object_mmap_data(struct drm_file *file,
  * userspace.
  */
 int
-i915_gem_mmap_gtt_ioctl(struct drm_device *dev, void *data,
-			struct drm_file *file)
+i915_gem_mmap_offset_ioctl(struct drm_device *dev, void *data,
+			   struct drm_file *file)
 {
-	struct drm_i915_gem_mmap_gtt *args = data;
+	struct drm_i915_private *i915 = to_i915(dev);
+	struct drm_i915_gem_mmap_offset *args = data;
+	enum i915_mmap_type type;
+
+	switch (args->flags) {
+	case I915_MMAP_OFFSET_GTT:
+		if (!i915_ggtt_has_aperture(&i915->ggtt))
+			return -ENODEV;
+		type = I915_MMAP_TYPE_GTT;
+		break;
+
+	case I915_MMAP_OFFSET_WC:
+		if (!boot_cpu_has(X86_FEATURE_PAT))
+			return -ENODEV;
+		type = I915_MMAP_TYPE_WC;
+		break;
+
+	case I915_MMAP_OFFSET_WB:
+		type = I915_MMAP_TYPE_WB;
+		break;
+
+	case I915_MMAP_OFFSET_UC:
+		if (!boot_cpu_has(X86_FEATURE_PAT))
+			return -ENODEV;
+		type = I915_MMAP_TYPE_UC;
+		break;
+
+	default:
+		return -EINVAL;
+	}
 
-	return __assign_gem_object_mmap_data(file, args->handle,
-					     I915_MMAP_TYPE_GTT,
+	return __assign_gem_object_mmap_data(file, args->handle, type,
 					     &args->offset);
 }
 
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_mman.h b/drivers/gpu/drm/i915/gem/i915_gem_mman.h
index 25a3c4d6cd65..4d3b493e853a 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_mman.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_mman.h
@@ -24,5 +24,6 @@ void i915_mmap_offset_destroy(struct i915_mmap_offset *mmo, struct mutex *mutex)
 void __i915_gem_object_release_mmap_gtt(struct drm_i915_gem_object *obj);
 void i915_gem_object_release_mmap(struct drm_i915_gem_object *obj);
 void i915_gem_object_release_mmap_offset(struct drm_i915_gem_object *obj);
+int i915_gem_mmap_gtt_version(void);
 
 #endif
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 8ff0834a1d5c..c0beb97b56d1 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
@@ -65,6 +65,9 @@ struct drm_i915_gem_object_ops {
 
 enum i915_mmap_type {
 	I915_MMAP_TYPE_GTT = 0,
+	I915_MMAP_TYPE_WC,
+	I915_MMAP_TYPE_WB,
+	I915_MMAP_TYPE_UC,
 };
 
 struct i915_mmap_offset {
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 3e129bcf8a12..ac6d4470ce75 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -2715,7 +2715,7 @@ static const struct drm_ioctl_desc i915_ioctls[] = {
 	DRM_IOCTL_DEF_DRV(I915_GEM_PREAD, i915_gem_pread_ioctl, DRM_RENDER_ALLOW),
 	DRM_IOCTL_DEF_DRV(I915_GEM_PWRITE, i915_gem_pwrite_ioctl, DRM_RENDER_ALLOW),
 	DRM_IOCTL_DEF_DRV(I915_GEM_MMAP, i915_gem_mmap_ioctl, DRM_RENDER_ALLOW),
-	DRM_IOCTL_DEF_DRV(I915_GEM_MMAP_GTT, i915_gem_mmap_gtt_ioctl, DRM_RENDER_ALLOW),
+	DRM_IOCTL_DEF_DRV(I915_GEM_MMAP_OFFSET, i915_gem_mmap_offset_ioctl, DRM_RENDER_ALLOW),
 	DRM_IOCTL_DEF_DRV(I915_GEM_SET_DOMAIN, i915_gem_set_domain_ioctl, DRM_RENDER_ALLOW),
 	DRM_IOCTL_DEF_DRV(I915_GEM_SW_FINISH, i915_gem_sw_finish_ioctl, DRM_RENDER_ALLOW),
 	DRM_IOCTL_DEF_DRV(I915_GEM_SET_TILING, i915_gem_set_tiling_ioctl, DRM_RENDER_ALLOW),
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index a36c2778c09c..b2af6df2eae0 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1853,7 +1853,6 @@ i915_mutex_lock_interruptible(struct drm_device *dev)
 int i915_gem_dumb_create(struct drm_file *file_priv,
 			 struct drm_device *dev,
 			 struct drm_mode_create_dumb *args);
-int i915_gem_mmap_gtt_version(void);
 
 int __must_check i915_gem_set_global_seqno(struct drm_device *dev, u32 seqno);
 
diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index 5400d7e057f1..e844c3a8d197 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -395,6 +395,7 @@ typedef struct _drm_i915_sarea {
 #define DRM_IOCTL_I915_GEM_PWRITE	DRM_IOW (DRM_COMMAND_BASE + DRM_I915_GEM_PWRITE, struct drm_i915_gem_pwrite)
 #define DRM_IOCTL_I915_GEM_MMAP		DRM_IOWR(DRM_COMMAND_BASE + DRM_I915_GEM_MMAP, struct drm_i915_gem_mmap)
 #define DRM_IOCTL_I915_GEM_MMAP_GTT	DRM_IOWR(DRM_COMMAND_BASE + DRM_I915_GEM_MMAP_GTT, struct drm_i915_gem_mmap_gtt)
+#define DRM_IOCTL_I915_GEM_MMAP_OFFSET	DRM_IOWR(DRM_COMMAND_BASE + DRM_I915_GEM_MMAP_GTT, struct drm_i915_gem_mmap_offset)
 #define DRM_IOCTL_I915_GEM_SET_DOMAIN	DRM_IOW (DRM_COMMAND_BASE + DRM_I915_GEM_SET_DOMAIN, struct drm_i915_gem_set_domain)
 #define DRM_IOCTL_I915_GEM_SW_FINISH	DRM_IOW (DRM_COMMAND_BASE + DRM_I915_GEM_SW_FINISH, struct drm_i915_gem_sw_finish)
 #define DRM_IOCTL_I915_GEM_SET_TILING	DRM_IOWR (DRM_COMMAND_BASE + DRM_I915_GEM_SET_TILING, struct drm_i915_gem_set_tiling)
@@ -793,6 +794,32 @@ struct drm_i915_gem_mmap_gtt {
 	__u64 offset;
 };
 
+struct drm_i915_gem_mmap_offset {
+	/** Handle for the object being mapped. */
+	__u32 handle;
+	__u32 pad;
+	/**
+	 * Fake offset to use for subsequent mmap call
+	 *
+	 * This is a fixed-size type for 32/64 compatibility.
+	 */
+	__u64 offset;
+
+	/**
+	 * Flags for extended behaviour.
+	 *
+	 * It is mandatory that either one of the MMAP_OFFSET flags
+	 * should be passed here.
+	 */
+	__u64 flags;
+#define I915_MMAP_OFFSET_GTT 0
+#define I915_MMAP_OFFSET_WC  1
+#define I915_MMAP_OFFSET_WB  2
+#define I915_MMAP_OFFSET_UC  3
+
+	__u64 extensions;
+};
+
 struct drm_i915_gem_set_domain {
 	/** Handle for the object */
 	__u32 handle;
-- 
2.23.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 3/4] drm/i915: cpu-map based dumb buffers
@ 2019-11-19 11:37   ` Abdiel Janulgue
  0 siblings, 0 replies; 23+ messages in thread
From: Abdiel Janulgue @ 2019-11-19 11:37 UTC (permalink / raw)
  To: intel-gfx; +Cc: Matthew Auld

Prefer CPU WC mmaps via our new mmap offset plumbing otherwise fall-
back to GTT mmaps when hw doesn't support PAT

Signed-off-by: Abdiel Janulgue <abdiel.janulgue@linux.intel.com>
Cc: Matthew Auld <matthew.auld@intel.com>
Acked-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/gem/i915_gem_mman.c | 18 ++++++++++++++++++
 drivers/gpu/drm/i915/gem/i915_gem_mman.h |  2 ++
 drivers/gpu/drm/i915/i915_drv.c          |  1 +
 3 files changed, 21 insertions(+)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_mman.c b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
index bb05c53c03c8..3913634ab717 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_mman.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
@@ -540,6 +540,24 @@ __assign_gem_object_mmap_data(struct drm_file *file,
 	return ret;
 }
 
+int
+i915_gem_mmap_dumb(struct drm_file *file,
+		  struct drm_device *dev,
+		  u32 handle,
+		  u64 *offset)
+{
+	enum i915_mmap_type mmap_type;
+
+	if (boot_cpu_has(X86_FEATURE_PAT))
+		mmap_type = I915_MMAP_TYPE_WC;
+	else if (!i915_ggtt_has_aperture(&to_i915(dev)->ggtt))
+		return -ENODEV;
+	else
+		mmap_type = I915_MMAP_TYPE_GTT;
+
+	return __assign_gem_object_mmap_data(file, handle, mmap_type, offset);
+}
+
 /**
  * i915_gem_mmap_offset_ioctl - prepare an object for GTT mmap'ing
  * @dev: DRM device
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_mman.h b/drivers/gpu/drm/i915/gem/i915_gem_mman.h
index 4d3b493e853a..6e70b91dabc4 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_mman.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_mman.h
@@ -24,6 +24,8 @@ void i915_mmap_offset_destroy(struct i915_mmap_offset *mmo, struct mutex *mutex)
 void __i915_gem_object_release_mmap_gtt(struct drm_i915_gem_object *obj);
 void i915_gem_object_release_mmap(struct drm_i915_gem_object *obj);
 void i915_gem_object_release_mmap_offset(struct drm_i915_gem_object *obj);
+int i915_gem_mmap_dumb(struct drm_file *file_priv, struct drm_device *dev,
+		      u32 handle, u64 *offset);
 int i915_gem_mmap_gtt_version(void);
 
 #endif
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index ac6d4470ce75..f7db0bbbe302 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -2767,6 +2767,7 @@ static struct drm_driver driver = {
 	.get_scanout_position = i915_get_crtc_scanoutpos,
 
 	.dumb_create = i915_gem_dumb_create,
+	.dumb_map_offset = i915_gem_mmap_dumb,
 	.ioctls = i915_ioctls,
 	.num_ioctls = ARRAY_SIZE(i915_ioctls),
 	.fops = &i915_driver_fops,
-- 
2.23.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [Intel-gfx] [PATCH 3/4] drm/i915: cpu-map based dumb buffers
@ 2019-11-19 11:37   ` Abdiel Janulgue
  0 siblings, 0 replies; 23+ messages in thread
From: Abdiel Janulgue @ 2019-11-19 11:37 UTC (permalink / raw)
  To: intel-gfx; +Cc: Matthew Auld

Prefer CPU WC mmaps via our new mmap offset plumbing otherwise fall-
back to GTT mmaps when hw doesn't support PAT

Signed-off-by: Abdiel Janulgue <abdiel.janulgue@linux.intel.com>
Cc: Matthew Auld <matthew.auld@intel.com>
Acked-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/gem/i915_gem_mman.c | 18 ++++++++++++++++++
 drivers/gpu/drm/i915/gem/i915_gem_mman.h |  2 ++
 drivers/gpu/drm/i915/i915_drv.c          |  1 +
 3 files changed, 21 insertions(+)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_mman.c b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
index bb05c53c03c8..3913634ab717 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_mman.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
@@ -540,6 +540,24 @@ __assign_gem_object_mmap_data(struct drm_file *file,
 	return ret;
 }
 
+int
+i915_gem_mmap_dumb(struct drm_file *file,
+		  struct drm_device *dev,
+		  u32 handle,
+		  u64 *offset)
+{
+	enum i915_mmap_type mmap_type;
+
+	if (boot_cpu_has(X86_FEATURE_PAT))
+		mmap_type = I915_MMAP_TYPE_WC;
+	else if (!i915_ggtt_has_aperture(&to_i915(dev)->ggtt))
+		return -ENODEV;
+	else
+		mmap_type = I915_MMAP_TYPE_GTT;
+
+	return __assign_gem_object_mmap_data(file, handle, mmap_type, offset);
+}
+
 /**
  * i915_gem_mmap_offset_ioctl - prepare an object for GTT mmap'ing
  * @dev: DRM device
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_mman.h b/drivers/gpu/drm/i915/gem/i915_gem_mman.h
index 4d3b493e853a..6e70b91dabc4 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_mman.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_mman.h
@@ -24,6 +24,8 @@ void i915_mmap_offset_destroy(struct i915_mmap_offset *mmo, struct mutex *mutex)
 void __i915_gem_object_release_mmap_gtt(struct drm_i915_gem_object *obj);
 void i915_gem_object_release_mmap(struct drm_i915_gem_object *obj);
 void i915_gem_object_release_mmap_offset(struct drm_i915_gem_object *obj);
+int i915_gem_mmap_dumb(struct drm_file *file_priv, struct drm_device *dev,
+		      u32 handle, u64 *offset);
 int i915_gem_mmap_gtt_version(void);
 
 #endif
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index ac6d4470ce75..f7db0bbbe302 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -2767,6 +2767,7 @@ static struct drm_driver driver = {
 	.get_scanout_position = i915_get_crtc_scanoutpos,
 
 	.dumb_create = i915_gem_dumb_create,
+	.dumb_map_offset = i915_gem_mmap_dumb,
 	.ioctls = i915_ioctls,
 	.num_ioctls = ARRAY_SIZE(i915_ioctls),
 	.fops = &i915_driver_fops,
-- 
2.23.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 4/4] drm/i915: Add cpu fault handler for mmap_offset
@ 2019-11-19 11:37   ` Abdiel Janulgue
  0 siblings, 0 replies; 23+ messages in thread
From: Abdiel Janulgue @ 2019-11-19 11:37 UTC (permalink / raw)
  To: intel-gfx; +Cc: Matthew Auld

Fault handler to handle missing pages for shmem-backed objects.

v2: bail out of inserting PTEs when failing to insert the
    fault address
v3: has struct page check
v4: Add self-test for validating CPU fault handler to ensure PTEs
    are revoked when an object is unbound.
v5: Add comment where PTEs are revoked (Chris)

Signed-off-by: Abdiel Janulgue <abdiel.janulgue@linux.intel.com>
Signed-off-by: Matthew Auld <matthew.auld@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/gem/i915_gem_mman.c      | 129 ++++++++++++++----
 .../drm/i915/gem/selftests/i915_gem_mman.c    |  48 ++++++-
 2 files changed, 145 insertions(+), 32 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_mman.c b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
index 3913634ab717..b892f645ca2d 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/mman.h>
+#include <linux/pfn_t.h>
 #include <linux/sizes.h>
 
 #include "gt/intel_gt.h"
@@ -201,6 +202,71 @@ compute_partial_view(const struct drm_i915_gem_object *obj,
 	return view;
 }
 
+static vm_fault_t i915_error_to_vmf_fault(int err)
+{
+	switch (err) {
+	default:
+		WARN_ONCE(err, "unhandled error in %s: %i\n", __func__, err);
+		/* fallthrough */
+	case -EIO: /* shmemfs failure from swap device */
+	case -EFAULT: /* purged object */
+	case -ENODEV: /* bad object, how did you get here! */
+		return VM_FAULT_SIGBUS;
+
+	case -ENOSPC: /* shmemfs allocation failure */
+	case -ENOMEM: /* our allocation failure */
+		return VM_FAULT_OOM;
+
+	case 0:
+	case -EAGAIN:
+	case -ERESTARTSYS:
+	case -EINTR:
+	case -EBUSY:
+		/*
+		 * EBUSY is ok: this just means that another thread
+		 * already did the job.
+		 */
+		return VM_FAULT_NOPAGE;
+	}
+}
+
+static vm_fault_t i915_gem_fault_cpu(struct vm_fault *vmf)
+{
+	struct vm_area_struct *area = vmf->vma;
+	struct i915_mmap_offset *priv = area->vm_private_data;
+	struct drm_i915_gem_object *obj = priv->obj;
+	vm_fault_t vmf_ret;
+	unsigned long i, size = area->vm_end - area->vm_start;
+	bool write = area->vm_flags & VM_WRITE;
+	int ret;
+
+	if (!i915_gem_object_has_struct_page(obj))
+		return VM_FAULT_SIGBUS;
+
+	/* Sanity check that we allow writing into this object */
+	if (i915_gem_object_is_readonly(obj) && write)
+		return VM_FAULT_SIGBUS;
+
+	ret = i915_gem_object_pin_pages(obj);
+	if (ret)
+		return i915_error_to_vmf_fault(ret);
+
+	/* PTEs are revoked in obj->ops->put_pages() */
+	for (i = 0; i < size >> PAGE_SHIFT; i++) {
+		struct page *page = i915_gem_object_get_page(obj, i);
+
+		vmf_ret = vmf_insert_pfn(area,
+					 (unsigned long)area->vm_start + i * PAGE_SIZE,
+					 page_to_pfn(page));
+		if (vmf_ret != VM_FAULT_NOPAGE)
+			break;
+	}
+
+	i915_gem_object_unpin_pages(obj);
+
+	return vmf_ret;
+}
+
 /**
  * i915_gem_fault - fault a page into the GTT
  * @vmf: fault info
@@ -340,30 +406,7 @@ vm_fault_t i915_gem_fault(struct vm_fault *vmf)
 	intel_runtime_pm_put(rpm, wakeref);
 	i915_gem_object_unpin_pages(obj);
 err:
-	switch (ret) {
-	default:
-		WARN_ONCE(ret, "unhandled error in %s: %i\n", __func__, ret);
-		/* fallthrough */
-	case -EIO: /* shmemfs failure from swap device */
-	case -EFAULT: /* purged object */
-	case -ENODEV: /* bad object, how did you get here! */
-		return VM_FAULT_SIGBUS;
-
-	case -ENOSPC: /* shmemfs allocation failure */
-	case -ENOMEM: /* our allocation failure */
-		return VM_FAULT_OOM;
-
-	case 0:
-	case -EAGAIN:
-	case -ERESTARTSYS:
-	case -EINTR:
-	case -EBUSY:
-		/*
-		 * EBUSY is ok: this just means that another thread
-		 * already did the job.
-		 */
-		return VM_FAULT_NOPAGE;
-	}
+	return i915_error_to_vmf_fault(ret);
 }
 
 void __i915_gem_object_release_mmap_gtt(struct drm_i915_gem_object *obj)
@@ -647,6 +690,33 @@ static const struct vm_operations_struct i915_gem_gtt_vm_ops = {
 	.close = i915_gem_vm_close,
 };
 
+static const struct vm_operations_struct i915_gem_cpu_vm_ops = {
+	.fault = i915_gem_fault_cpu,
+	.open = i915_gem_vm_open,
+	.close = i915_gem_vm_close,
+};
+
+static void set_vmdata_mmap_offset(struct i915_mmap_offset *mmo, struct vm_area_struct *vma)
+{
+	switch (mmo->mmap_type) {
+	case I915_MMAP_TYPE_WC:
+		vma->vm_page_prot =
+			pgprot_writecombine(vm_get_page_prot(vma->vm_flags));
+		break;
+	case I915_MMAP_TYPE_WB:
+		vma->vm_page_prot = vm_get_page_prot(vma->vm_flags);
+		break;
+	case I915_MMAP_TYPE_UC:
+		vma->vm_page_prot =
+			pgprot_noncached(vm_get_page_prot(vma->vm_flags));
+		break;
+	default:
+		break;
+	}
+
+	vma->vm_ops = &i915_gem_cpu_vm_ops;
+}
+
 /* This overcomes the limitation in drm_gem_mmap's assignment of a
  * drm_gem_object as the vma->vm_private_data. Since we need to
  * be able to resolve multiple mmap offsets which could be tied
@@ -714,7 +784,16 @@ int i915_gem_mmap(struct file *filp, struct vm_area_struct *vma)
 	vma->vm_page_prot = pgprot_decrypted(vma->vm_page_prot);
 	vma->vm_private_data = mmo;
 
-	vma->vm_ops = &i915_gem_gtt_vm_ops;
+	switch (mmo->mmap_type) {
+	case I915_MMAP_TYPE_WC:
+	case I915_MMAP_TYPE_WB:
+	case I915_MMAP_TYPE_UC:
+		set_vmdata_mmap_offset(mmo, vma);
+		break;
+	case I915_MMAP_TYPE_GTT:
+		vma->vm_ops = &i915_gem_gtt_vm_ops;
+		break;
+	}
 
 	return 0;
 }
diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c b/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c
index 6c2333c61469..1bedc9d115c5 100644
--- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c
+++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c
@@ -731,7 +731,7 @@ static int igt_mmap_offset_exhaustion(void *arg)
 }
 
 #define expand32(x) (((x) << 0) | ((x) << 8) | ((x) << 16) | ((x) << 24))
-static int igt_mmap_gtt(void *arg)
+static int igt_mmap(void *arg, enum i915_mmap_type type)
 {
 	struct drm_i915_private *i915 = arg;
 	struct drm_i915_gem_object *obj;
@@ -762,7 +762,8 @@ static int igt_mmap_gtt(void *arg)
 	if (err)
 		goto out;
 
-	addr = igt_mmap_node(i915, &obj->base.vma_node,
+	mmo->mmap_type = type;
+	addr = igt_mmap_node(i915, &mmo->vma_node,
 			     0, PROT_WRITE, MAP_SHARED);
 	if (IS_ERR_VALUE(addr)) {
 		err = addr;
@@ -778,8 +779,8 @@ static int igt_mmap_gtt(void *arg)
 		goto out_unmap;
 	}
 
-	if (area->vm_private_data != obj) {
-		pr_err("vm_area_struct did not point back to our object!\n");
+	if (area->vm_private_data != mmo) {
+		pr_err("vm_area_struct did not point back to our mmap_offset object!\n");
 		err = -EINVAL;
 		goto out_unmap;
 	}
@@ -830,6 +831,16 @@ static int igt_mmap_gtt(void *arg)
 	return err;
 }
 
+static int igt_mmap_gtt(void *arg)
+{
+	return igt_mmap(arg, I915_MMAP_TYPE_GTT);
+}
+
+static int igt_mmap_cpu(void *arg)
+{
+	return igt_mmap(arg, I915_MMAP_TYPE_WC);
+}
+
 static int check_present_pte(pte_t *pte, unsigned long addr, void *data)
 {
 	if (!pte_present(*pte) || pte_none(*pte)) {
@@ -882,7 +893,7 @@ static int prefault_range(u64 start, u64 len)
 	return __get_user(c, end - 1);
 }
 
-static int igt_mmap_gtt_revoke(void *arg)
+static int igt_mmap_revoke(void *arg, enum i915_mmap_type type)
 {
 	struct drm_i915_private *i915 = arg;
 	struct drm_i915_gem_object *obj;
@@ -902,7 +913,8 @@ static int igt_mmap_gtt_revoke(void *arg)
 	if (err)
 		goto out;
 
-	addr = igt_mmap_node(i915, &obj->base.vma_node,
+	mmo->mmap_type = type;
+	addr = igt_mmap_node(i915, &mmo->vma_node,
 			     0, PROT_WRITE, MAP_SHARED);
 	if (IS_ERR_VALUE(addr)) {
 		err = addr;
@@ -913,7 +925,8 @@ static int igt_mmap_gtt_revoke(void *arg)
 	if (err)
 		goto out_unmap;
 
-	GEM_BUG_ON(!atomic_read(&obj->bind_count));
+	GEM_BUG_ON(mmo->mmap_type == I915_MMAP_TYPE_GTT &&
+		   !atomic_read(&obj->bind_count));
 
 	err = check_present(addr, obj->base.size);
 	if (err)
@@ -931,6 +944,15 @@ static int igt_mmap_gtt_revoke(void *arg)
 	}
 	GEM_BUG_ON(atomic_read(&obj->bind_count));
 
+	if (type != I915_MMAP_TYPE_GTT) {
+		__i915_gem_object_put_pages(obj);
+		if (i915_gem_object_has_pages(obj)) {
+			pr_err("Failed to put-pages object!\n");
+			err = -EINVAL;
+			goto out_unmap;
+		}
+	}
+
 	err = check_absent(addr, obj->base.size);
 	if (err)
 		goto out_unmap;
@@ -942,6 +964,16 @@ static int igt_mmap_gtt_revoke(void *arg)
 	return err;
 }
 
+static int igt_mmap_gtt_revoke(void *arg)
+{
+	return igt_mmap_revoke(arg, I915_MMAP_TYPE_GTT);
+}
+
+static int igt_mmap_cpu_revoke(void *arg)
+{
+	return igt_mmap_revoke(arg, I915_MMAP_TYPE_WC);
+}
+
 int i915_gem_mman_live_selftests(struct drm_i915_private *i915)
 {
 	static const struct i915_subtest tests[] = {
@@ -949,7 +981,9 @@ int i915_gem_mman_live_selftests(struct drm_i915_private *i915)
 		SUBTEST(igt_smoke_tiling),
 		SUBTEST(igt_mmap_offset_exhaustion),
 		SUBTEST(igt_mmap_gtt),
+		SUBTEST(igt_mmap_cpu),
 		SUBTEST(igt_mmap_gtt_revoke),
+		SUBTEST(igt_mmap_cpu_revoke),
 	};
 
 	return i915_subtests(tests, i915);
-- 
2.23.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [Intel-gfx] [PATCH 4/4] drm/i915: Add cpu fault handler for mmap_offset
@ 2019-11-19 11:37   ` Abdiel Janulgue
  0 siblings, 0 replies; 23+ messages in thread
From: Abdiel Janulgue @ 2019-11-19 11:37 UTC (permalink / raw)
  To: intel-gfx; +Cc: Matthew Auld

Fault handler to handle missing pages for shmem-backed objects.

v2: bail out of inserting PTEs when failing to insert the
    fault address
v3: has struct page check
v4: Add self-test for validating CPU fault handler to ensure PTEs
    are revoked when an object is unbound.
v5: Add comment where PTEs are revoked (Chris)

Signed-off-by: Abdiel Janulgue <abdiel.janulgue@linux.intel.com>
Signed-off-by: Matthew Auld <matthew.auld@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/gem/i915_gem_mman.c      | 129 ++++++++++++++----
 .../drm/i915/gem/selftests/i915_gem_mman.c    |  48 ++++++-
 2 files changed, 145 insertions(+), 32 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_mman.c b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
index 3913634ab717..b892f645ca2d 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/mman.h>
+#include <linux/pfn_t.h>
 #include <linux/sizes.h>
 
 #include "gt/intel_gt.h"
@@ -201,6 +202,71 @@ compute_partial_view(const struct drm_i915_gem_object *obj,
 	return view;
 }
 
+static vm_fault_t i915_error_to_vmf_fault(int err)
+{
+	switch (err) {
+	default:
+		WARN_ONCE(err, "unhandled error in %s: %i\n", __func__, err);
+		/* fallthrough */
+	case -EIO: /* shmemfs failure from swap device */
+	case -EFAULT: /* purged object */
+	case -ENODEV: /* bad object, how did you get here! */
+		return VM_FAULT_SIGBUS;
+
+	case -ENOSPC: /* shmemfs allocation failure */
+	case -ENOMEM: /* our allocation failure */
+		return VM_FAULT_OOM;
+
+	case 0:
+	case -EAGAIN:
+	case -ERESTARTSYS:
+	case -EINTR:
+	case -EBUSY:
+		/*
+		 * EBUSY is ok: this just means that another thread
+		 * already did the job.
+		 */
+		return VM_FAULT_NOPAGE;
+	}
+}
+
+static vm_fault_t i915_gem_fault_cpu(struct vm_fault *vmf)
+{
+	struct vm_area_struct *area = vmf->vma;
+	struct i915_mmap_offset *priv = area->vm_private_data;
+	struct drm_i915_gem_object *obj = priv->obj;
+	vm_fault_t vmf_ret;
+	unsigned long i, size = area->vm_end - area->vm_start;
+	bool write = area->vm_flags & VM_WRITE;
+	int ret;
+
+	if (!i915_gem_object_has_struct_page(obj))
+		return VM_FAULT_SIGBUS;
+
+	/* Sanity check that we allow writing into this object */
+	if (i915_gem_object_is_readonly(obj) && write)
+		return VM_FAULT_SIGBUS;
+
+	ret = i915_gem_object_pin_pages(obj);
+	if (ret)
+		return i915_error_to_vmf_fault(ret);
+
+	/* PTEs are revoked in obj->ops->put_pages() */
+	for (i = 0; i < size >> PAGE_SHIFT; i++) {
+		struct page *page = i915_gem_object_get_page(obj, i);
+
+		vmf_ret = vmf_insert_pfn(area,
+					 (unsigned long)area->vm_start + i * PAGE_SIZE,
+					 page_to_pfn(page));
+		if (vmf_ret != VM_FAULT_NOPAGE)
+			break;
+	}
+
+	i915_gem_object_unpin_pages(obj);
+
+	return vmf_ret;
+}
+
 /**
  * i915_gem_fault - fault a page into the GTT
  * @vmf: fault info
@@ -340,30 +406,7 @@ vm_fault_t i915_gem_fault(struct vm_fault *vmf)
 	intel_runtime_pm_put(rpm, wakeref);
 	i915_gem_object_unpin_pages(obj);
 err:
-	switch (ret) {
-	default:
-		WARN_ONCE(ret, "unhandled error in %s: %i\n", __func__, ret);
-		/* fallthrough */
-	case -EIO: /* shmemfs failure from swap device */
-	case -EFAULT: /* purged object */
-	case -ENODEV: /* bad object, how did you get here! */
-		return VM_FAULT_SIGBUS;
-
-	case -ENOSPC: /* shmemfs allocation failure */
-	case -ENOMEM: /* our allocation failure */
-		return VM_FAULT_OOM;
-
-	case 0:
-	case -EAGAIN:
-	case -ERESTARTSYS:
-	case -EINTR:
-	case -EBUSY:
-		/*
-		 * EBUSY is ok: this just means that another thread
-		 * already did the job.
-		 */
-		return VM_FAULT_NOPAGE;
-	}
+	return i915_error_to_vmf_fault(ret);
 }
 
 void __i915_gem_object_release_mmap_gtt(struct drm_i915_gem_object *obj)
@@ -647,6 +690,33 @@ static const struct vm_operations_struct i915_gem_gtt_vm_ops = {
 	.close = i915_gem_vm_close,
 };
 
+static const struct vm_operations_struct i915_gem_cpu_vm_ops = {
+	.fault = i915_gem_fault_cpu,
+	.open = i915_gem_vm_open,
+	.close = i915_gem_vm_close,
+};
+
+static void set_vmdata_mmap_offset(struct i915_mmap_offset *mmo, struct vm_area_struct *vma)
+{
+	switch (mmo->mmap_type) {
+	case I915_MMAP_TYPE_WC:
+		vma->vm_page_prot =
+			pgprot_writecombine(vm_get_page_prot(vma->vm_flags));
+		break;
+	case I915_MMAP_TYPE_WB:
+		vma->vm_page_prot = vm_get_page_prot(vma->vm_flags);
+		break;
+	case I915_MMAP_TYPE_UC:
+		vma->vm_page_prot =
+			pgprot_noncached(vm_get_page_prot(vma->vm_flags));
+		break;
+	default:
+		break;
+	}
+
+	vma->vm_ops = &i915_gem_cpu_vm_ops;
+}
+
 /* This overcomes the limitation in drm_gem_mmap's assignment of a
  * drm_gem_object as the vma->vm_private_data. Since we need to
  * be able to resolve multiple mmap offsets which could be tied
@@ -714,7 +784,16 @@ int i915_gem_mmap(struct file *filp, struct vm_area_struct *vma)
 	vma->vm_page_prot = pgprot_decrypted(vma->vm_page_prot);
 	vma->vm_private_data = mmo;
 
-	vma->vm_ops = &i915_gem_gtt_vm_ops;
+	switch (mmo->mmap_type) {
+	case I915_MMAP_TYPE_WC:
+	case I915_MMAP_TYPE_WB:
+	case I915_MMAP_TYPE_UC:
+		set_vmdata_mmap_offset(mmo, vma);
+		break;
+	case I915_MMAP_TYPE_GTT:
+		vma->vm_ops = &i915_gem_gtt_vm_ops;
+		break;
+	}
 
 	return 0;
 }
diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c b/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c
index 6c2333c61469..1bedc9d115c5 100644
--- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c
+++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c
@@ -731,7 +731,7 @@ static int igt_mmap_offset_exhaustion(void *arg)
 }
 
 #define expand32(x) (((x) << 0) | ((x) << 8) | ((x) << 16) | ((x) << 24))
-static int igt_mmap_gtt(void *arg)
+static int igt_mmap(void *arg, enum i915_mmap_type type)
 {
 	struct drm_i915_private *i915 = arg;
 	struct drm_i915_gem_object *obj;
@@ -762,7 +762,8 @@ static int igt_mmap_gtt(void *arg)
 	if (err)
 		goto out;
 
-	addr = igt_mmap_node(i915, &obj->base.vma_node,
+	mmo->mmap_type = type;
+	addr = igt_mmap_node(i915, &mmo->vma_node,
 			     0, PROT_WRITE, MAP_SHARED);
 	if (IS_ERR_VALUE(addr)) {
 		err = addr;
@@ -778,8 +779,8 @@ static int igt_mmap_gtt(void *arg)
 		goto out_unmap;
 	}
 
-	if (area->vm_private_data != obj) {
-		pr_err("vm_area_struct did not point back to our object!\n");
+	if (area->vm_private_data != mmo) {
+		pr_err("vm_area_struct did not point back to our mmap_offset object!\n");
 		err = -EINVAL;
 		goto out_unmap;
 	}
@@ -830,6 +831,16 @@ static int igt_mmap_gtt(void *arg)
 	return err;
 }
 
+static int igt_mmap_gtt(void *arg)
+{
+	return igt_mmap(arg, I915_MMAP_TYPE_GTT);
+}
+
+static int igt_mmap_cpu(void *arg)
+{
+	return igt_mmap(arg, I915_MMAP_TYPE_WC);
+}
+
 static int check_present_pte(pte_t *pte, unsigned long addr, void *data)
 {
 	if (!pte_present(*pte) || pte_none(*pte)) {
@@ -882,7 +893,7 @@ static int prefault_range(u64 start, u64 len)
 	return __get_user(c, end - 1);
 }
 
-static int igt_mmap_gtt_revoke(void *arg)
+static int igt_mmap_revoke(void *arg, enum i915_mmap_type type)
 {
 	struct drm_i915_private *i915 = arg;
 	struct drm_i915_gem_object *obj;
@@ -902,7 +913,8 @@ static int igt_mmap_gtt_revoke(void *arg)
 	if (err)
 		goto out;
 
-	addr = igt_mmap_node(i915, &obj->base.vma_node,
+	mmo->mmap_type = type;
+	addr = igt_mmap_node(i915, &mmo->vma_node,
 			     0, PROT_WRITE, MAP_SHARED);
 	if (IS_ERR_VALUE(addr)) {
 		err = addr;
@@ -913,7 +925,8 @@ static int igt_mmap_gtt_revoke(void *arg)
 	if (err)
 		goto out_unmap;
 
-	GEM_BUG_ON(!atomic_read(&obj->bind_count));
+	GEM_BUG_ON(mmo->mmap_type == I915_MMAP_TYPE_GTT &&
+		   !atomic_read(&obj->bind_count));
 
 	err = check_present(addr, obj->base.size);
 	if (err)
@@ -931,6 +944,15 @@ static int igt_mmap_gtt_revoke(void *arg)
 	}
 	GEM_BUG_ON(atomic_read(&obj->bind_count));
 
+	if (type != I915_MMAP_TYPE_GTT) {
+		__i915_gem_object_put_pages(obj);
+		if (i915_gem_object_has_pages(obj)) {
+			pr_err("Failed to put-pages object!\n");
+			err = -EINVAL;
+			goto out_unmap;
+		}
+	}
+
 	err = check_absent(addr, obj->base.size);
 	if (err)
 		goto out_unmap;
@@ -942,6 +964,16 @@ static int igt_mmap_gtt_revoke(void *arg)
 	return err;
 }
 
+static int igt_mmap_gtt_revoke(void *arg)
+{
+	return igt_mmap_revoke(arg, I915_MMAP_TYPE_GTT);
+}
+
+static int igt_mmap_cpu_revoke(void *arg)
+{
+	return igt_mmap_revoke(arg, I915_MMAP_TYPE_WC);
+}
+
 int i915_gem_mman_live_selftests(struct drm_i915_private *i915)
 {
 	static const struct i915_subtest tests[] = {
@@ -949,7 +981,9 @@ int i915_gem_mman_live_selftests(struct drm_i915_private *i915)
 		SUBTEST(igt_smoke_tiling),
 		SUBTEST(igt_mmap_offset_exhaustion),
 		SUBTEST(igt_mmap_gtt),
+		SUBTEST(igt_mmap_cpu),
 		SUBTEST(igt_mmap_gtt_revoke),
+		SUBTEST(igt_mmap_cpu_revoke),
 	};
 
 	return i915_subtests(tests, i915);
-- 
2.23.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* Re: [PATCH 2/4] drm/i915: Introduce DRM_I915_GEM_MMAP_OFFSET
@ 2019-11-19 11:40     ` Abdiel Janulgue
  0 siblings, 0 replies; 23+ messages in thread
From: Abdiel Janulgue @ 2019-11-19 11:40 UTC (permalink / raw)
  To: intel-gfx


On 19/11/2019 13.37, Abdiel Janulgue wrote:

> +struct drm_i915_gem_mmap_offset {
> +	/** Handle for the object being mapped. */
> +	__u32 handle;
> +	__u32 pad;
> +	/**
> +	 * Fake offset to use for subsequent mmap call
> +	 *
> +	 * This is a fixed-size type for 32/64 compatibility.
> +	 */
> +	__u64 offset;
> +
> +	/**
> +	 * Flags for extended behaviour.
> +	 *
> +	 * It is mandatory that either one of the MMAP_OFFSET flags
> +	 * should be passed here.
> +	 */
> +	__u64 flags;
> +#define I915_MMAP_OFFSET_GTT 0
> +#define I915_MMAP_OFFSET_WC  1
> +#define I915_MMAP_OFFSET_WB  2
> +#define I915_MMAP_OFFSET_UC  3
> +
> +	__u64 extensions;
> +};

The simple memset IGT portion of this, coming up soon.

-Abdiel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [Intel-gfx] [PATCH 2/4] drm/i915: Introduce DRM_I915_GEM_MMAP_OFFSET
@ 2019-11-19 11:40     ` Abdiel Janulgue
  0 siblings, 0 replies; 23+ messages in thread
From: Abdiel Janulgue @ 2019-11-19 11:40 UTC (permalink / raw)
  To: intel-gfx


On 19/11/2019 13.37, Abdiel Janulgue wrote:

> +struct drm_i915_gem_mmap_offset {
> +	/** Handle for the object being mapped. */
> +	__u32 handle;
> +	__u32 pad;
> +	/**
> +	 * Fake offset to use for subsequent mmap call
> +	 *
> +	 * This is a fixed-size type for 32/64 compatibility.
> +	 */
> +	__u64 offset;
> +
> +	/**
> +	 * Flags for extended behaviour.
> +	 *
> +	 * It is mandatory that either one of the MMAP_OFFSET flags
> +	 * should be passed here.
> +	 */
> +	__u64 flags;
> +#define I915_MMAP_OFFSET_GTT 0
> +#define I915_MMAP_OFFSET_WC  1
> +#define I915_MMAP_OFFSET_WB  2
> +#define I915_MMAP_OFFSET_UC  3
> +
> +	__u64 extensions;
> +};

The simple memset IGT portion of this, coming up soon.

-Abdiel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 23+ messages in thread

* ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/4] drm/i915: Allow i915 to manage the vma offset nodes instead of drm core
@ 2019-11-19 12:46   ` Patchwork
  0 siblings, 0 replies; 23+ messages in thread
From: Patchwork @ 2019-11-19 12:46 UTC (permalink / raw)
  To: Abdiel Janulgue; +Cc: intel-gfx

== Series Details ==

Series: series starting with [1/4] drm/i915: Allow i915 to manage the vma offset nodes instead of drm core
URL   : https://patchwork.freedesktop.org/series/69674/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
badcc07981f3 drm/i915: Allow i915 to manage the vma offset nodes instead of drm core
-:370: CHECK:BRACES: Blank lines aren't necessary before a close brace '}'
#370: FILE: drivers/gpu/drm/i915/gem/i915_gem_mman.c:644:
+
+	}

-:401: WARNING:FILE_PATH_CHANGES: added, moved or deleted file(s), does MAINTAINERS need updating?
#401: 
new file mode 100644

-:406: WARNING:SPDX_LICENSE_TAG: Missing or malformed SPDX-License-Identifier tag in line 1
#406: FILE: drivers/gpu/drm/i915/gem/i915_gem_mman.h:1:
+/*

-:407: WARNING:SPDX_LICENSE_TAG: Misplaced SPDX-License-Identifier tag - use line 1 instead
#407: FILE: drivers/gpu/drm/i915/gem/i915_gem_mman.h:2:
+ * SPDX-License-Identifier: MIT

-:626: CHECK:LINE_SPACING: Please don't use multiple blank lines
#626: FILE: drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c:580:
 
+

-:801: WARNING:SPDX_LICENSE_TAG: Misplaced SPDX-License-Identifier tag - use line 1 instead
#801: FILE: drivers/gpu/drm/i915/i915_getparam.c:2:
  * SPDX-License-Identifier: MIT

total: 0 errors, 4 warnings, 2 checks, 692 lines checked
614f0a31f6bf drm/i915: Introduce DRM_I915_GEM_MMAP_OFFSET
-:183: WARNING:LONG_LINE: line over 100 characters
#183: FILE: include/uapi/drm/i915_drm.h:398:
+#define DRM_IOCTL_I915_GEM_MMAP_OFFSET	DRM_IOWR(DRM_COMMAND_BASE + DRM_I915_GEM_MMAP_GTT, struct drm_i915_gem_mmap_offset)

total: 0 errors, 1 warnings, 0 checks, 150 lines checked
f4981991370d drm/i915: cpu-map based dumb buffers
-:23: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#23: FILE: drivers/gpu/drm/i915/gem/i915_gem_mman.c:545:
+i915_gem_mmap_dumb(struct drm_file *file,
+		  struct drm_device *dev,

-:33: WARNING:UNNECESSARY_ELSE: else is not generally useful after a break or return
#33: FILE: drivers/gpu/drm/i915/gem/i915_gem_mman.c:555:
+		return -ENODEV;
+	else

-:51: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#51: FILE: drivers/gpu/drm/i915/gem/i915_gem_mman.h:28:
+int i915_gem_mmap_dumb(struct drm_file *file_priv, struct drm_device *dev,
+		      u32 handle, u64 *offset);

total: 0 errors, 1 warnings, 2 checks, 39 lines checked
84e8b7b05885 drm/i915: Add cpu fault handler for mmap_offset

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/4] drm/i915: Allow i915 to manage the vma offset nodes instead of drm core
@ 2019-11-19 12:46   ` Patchwork
  0 siblings, 0 replies; 23+ messages in thread
From: Patchwork @ 2019-11-19 12:46 UTC (permalink / raw)
  To: Abdiel Janulgue; +Cc: intel-gfx

== Series Details ==

Series: series starting with [1/4] drm/i915: Allow i915 to manage the vma offset nodes instead of drm core
URL   : https://patchwork.freedesktop.org/series/69674/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
badcc07981f3 drm/i915: Allow i915 to manage the vma offset nodes instead of drm core
-:370: CHECK:BRACES: Blank lines aren't necessary before a close brace '}'
#370: FILE: drivers/gpu/drm/i915/gem/i915_gem_mman.c:644:
+
+	}

-:401: WARNING:FILE_PATH_CHANGES: added, moved or deleted file(s), does MAINTAINERS need updating?
#401: 
new file mode 100644

-:406: WARNING:SPDX_LICENSE_TAG: Missing or malformed SPDX-License-Identifier tag in line 1
#406: FILE: drivers/gpu/drm/i915/gem/i915_gem_mman.h:1:
+/*

-:407: WARNING:SPDX_LICENSE_TAG: Misplaced SPDX-License-Identifier tag - use line 1 instead
#407: FILE: drivers/gpu/drm/i915/gem/i915_gem_mman.h:2:
+ * SPDX-License-Identifier: MIT

-:626: CHECK:LINE_SPACING: Please don't use multiple blank lines
#626: FILE: drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c:580:
 
+

-:801: WARNING:SPDX_LICENSE_TAG: Misplaced SPDX-License-Identifier tag - use line 1 instead
#801: FILE: drivers/gpu/drm/i915/i915_getparam.c:2:
  * SPDX-License-Identifier: MIT

total: 0 errors, 4 warnings, 2 checks, 692 lines checked
614f0a31f6bf drm/i915: Introduce DRM_I915_GEM_MMAP_OFFSET
-:183: WARNING:LONG_LINE: line over 100 characters
#183: FILE: include/uapi/drm/i915_drm.h:398:
+#define DRM_IOCTL_I915_GEM_MMAP_OFFSET	DRM_IOWR(DRM_COMMAND_BASE + DRM_I915_GEM_MMAP_GTT, struct drm_i915_gem_mmap_offset)

total: 0 errors, 1 warnings, 0 checks, 150 lines checked
f4981991370d drm/i915: cpu-map based dumb buffers
-:23: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#23: FILE: drivers/gpu/drm/i915/gem/i915_gem_mman.c:545:
+i915_gem_mmap_dumb(struct drm_file *file,
+		  struct drm_device *dev,

-:33: WARNING:UNNECESSARY_ELSE: else is not generally useful after a break or return
#33: FILE: drivers/gpu/drm/i915/gem/i915_gem_mman.c:555:
+		return -ENODEV;
+	else

-:51: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#51: FILE: drivers/gpu/drm/i915/gem/i915_gem_mman.h:28:
+int i915_gem_mmap_dumb(struct drm_file *file_priv, struct drm_device *dev,
+		      u32 handle, u64 *offset);

total: 0 errors, 1 warnings, 2 checks, 39 lines checked
84e8b7b05885 drm/i915: Add cpu fault handler for mmap_offset

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 23+ messages in thread

* ✗ Fi.CI.BAT: failure for series starting with [1/4] drm/i915: Allow i915 to manage the vma offset nodes instead of drm core
@ 2019-11-19 13:25   ` Patchwork
  0 siblings, 0 replies; 23+ messages in thread
From: Patchwork @ 2019-11-19 13:25 UTC (permalink / raw)
  To: Abdiel Janulgue; +Cc: intel-gfx

== Series Details ==

Series: series starting with [1/4] drm/i915: Allow i915 to manage the vma offset nodes instead of drm core
URL   : https://patchwork.freedesktop.org/series/69674/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_7370 -> Patchwork_15327
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_15327 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_15327, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15327/index.html

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in Patchwork_15327:

### IGT changes ###

#### Possible regressions ####

  * igt@kms_pipe_crc_basic@hang-read-crc-pipe-a:
    - fi-gdg-551:         [PASS][1] -> [FAIL][2] +1 similar issue
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7370/fi-gdg-551/igt@kms_pipe_crc_basic@hang-read-crc-pipe-a.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15327/fi-gdg-551/igt@kms_pipe_crc_basic@hang-read-crc-pipe-a.html

  * igt@kms_pipe_crc_basic@nonblocking-crc-pipe-a-frame-sequence:
    - fi-ilk-650:         [PASS][3] -> [FAIL][4] +6 similar issues
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7370/fi-ilk-650/igt@kms_pipe_crc_basic@nonblocking-crc-pipe-a-frame-sequence.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15327/fi-ilk-650/igt@kms_pipe_crc_basic@nonblocking-crc-pipe-a-frame-sequence.html

  * igt@kms_pipe_crc_basic@read-crc-pipe-a:
    - fi-bwr-2160:        [PASS][5] -> [FAIL][6] +5 similar issues
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7370/fi-bwr-2160/igt@kms_pipe_crc_basic@read-crc-pipe-a.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15327/fi-bwr-2160/igt@kms_pipe_crc_basic@read-crc-pipe-a.html

  * igt@kms_pipe_crc_basic@read-crc-pipe-b:
    - fi-blb-e6850:       [PASS][7] -> [FAIL][8] +6 similar issues
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7370/fi-blb-e6850/igt@kms_pipe_crc_basic@read-crc-pipe-b.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15327/fi-blb-e6850/igt@kms_pipe_crc_basic@read-crc-pipe-b.html

  * igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a:
    - fi-bsw-kefka:       [PASS][9] -> [FAIL][10] +3 similar issues
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7370/fi-bsw-kefka/igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15327/fi-bsw-kefka/igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a.html

  
Known issues
------------

  Here are the changes found in Patchwork_15327 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@i915_pm_rpm@module-reload:
    - fi-skl-6770hq:      [PASS][11] -> [DMESG-WARN][12] ([fdo#112261])
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7370/fi-skl-6770hq/igt@i915_pm_rpm@module-reload.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15327/fi-skl-6770hq/igt@i915_pm_rpm@module-reload.html

  * igt@kms_pipe_crc_basic@nonblocking-crc-pipe-a-frame-sequence:
    - fi-byt-n2820:       [PASS][13] -> [FAIL][14] ([fdo#103191]) +4 similar issues
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7370/fi-byt-n2820/igt@kms_pipe_crc_basic@nonblocking-crc-pipe-a-frame-sequence.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15327/fi-byt-n2820/igt@kms_pipe_crc_basic@nonblocking-crc-pipe-a-frame-sequence.html

  * igt@kms_pipe_crc_basic@read-crc-pipe-a-frame-sequence:
    - fi-byt-j1900:       [PASS][15] -> [FAIL][16] ([fdo#103191]) +5 similar issues
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7370/fi-byt-j1900/igt@kms_pipe_crc_basic@read-crc-pipe-a-frame-sequence.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15327/fi-byt-j1900/igt@kms_pipe_crc_basic@read-crc-pipe-a-frame-sequence.html

  * igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a:
    - fi-glk-dsi:         [PASS][17] -> [FAIL][18] ([fdo#103191]) +3 similar issues
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7370/fi-glk-dsi/igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15327/fi-glk-dsi/igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a.html

  
#### Possible fixes ####

  * igt@gem_cpu_reloc@basic:
    - fi-icl-dsi:         [DMESG-WARN][19] ([fdo#106107]) -> [PASS][20]
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7370/fi-icl-dsi/igt@gem_cpu_reloc@basic.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15327/fi-icl-dsi/igt@gem_cpu_reloc@basic.html

  * igt@i915_pm_rpm@module-reload:
    - {fi-kbl-7560u}:     [FAIL][21] ([fdo#112269]) -> [PASS][22]
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7370/fi-kbl-7560u/igt@i915_pm_rpm@module-reload.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15327/fi-kbl-7560u/igt@i915_pm_rpm@module-reload.html

  * igt@i915_selftest@live_gem_contexts:
    - fi-skl-lmem:        [DMESG-FAIL][23] -> [PASS][24]
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7370/fi-skl-lmem/igt@i915_selftest@live_gem_contexts.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15327/fi-skl-lmem/igt@i915_selftest@live_gem_contexts.html

  * igt@kms_chamelium@hdmi-hpd-fast:
    - fi-kbl-7500u:       [FAIL][25] ([fdo#111407]) -> [PASS][26]
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7370/fi-kbl-7500u/igt@kms_chamelium@hdmi-hpd-fast.html
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15327/fi-kbl-7500u/igt@kms_chamelium@hdmi-hpd-fast.html

  * igt@kms_flip@basic-flip-vs-dpms:
    - fi-skl-6770hq:      [SKIP][27] ([fdo#109271]) -> [PASS][28] +27 similar issues
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7370/fi-skl-6770hq/igt@kms_flip@basic-flip-vs-dpms.html
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15327/fi-skl-6770hq/igt@kms_flip@basic-flip-vs-dpms.html

  * igt@kms_frontbuffer_tracking@basic:
    - fi-hsw-peppy:       [DMESG-WARN][29] ([fdo#102614]) -> [PASS][30]
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7370/fi-hsw-peppy/igt@kms_frontbuffer_tracking@basic.html
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15327/fi-hsw-peppy/igt@kms_frontbuffer_tracking@basic.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#102614]: https://bugs.freedesktop.org/show_bug.cgi?id=102614
  [fdo#103191]: https://bugs.freedesktop.org/show_bug.cgi?id=103191
  [fdo#106107]: https://bugs.freedesktop.org/show_bug.cgi?id=106107
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#111407]: https://bugs.freedesktop.org/show_bug.cgi?id=111407
  [fdo#112261]: https://bugs.freedesktop.org/show_bug.cgi?id=112261
  [fdo#112269]: https://bugs.freedesktop.org/show_bug.cgi?id=112269


Participating hosts (49 -> 44)
------------------------------

  Additional (1): fi-hsw-4770r 
  Missing    (6): fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 fi-byt-clapper fi-bdw-samus 


Build changes
-------------

  * CI: CI-20190529 -> None
  * Linux: CI_DRM_7370 -> Patchwork_15327

  CI-20190529: 20190529
  CI_DRM_7370: d2db945edccfe34bc3cc1d0accafac2036ad4e66 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5293: 4bb46f08f7cb6485642c4351cecdad93072d27a0 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_15327: 84e8b7b058852566875db9ede43e8d1d99cf400f @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

84e8b7b05885 drm/i915: Add cpu fault handler for mmap_offset
f4981991370d drm/i915: cpu-map based dumb buffers
614f0a31f6bf drm/i915: Introduce DRM_I915_GEM_MMAP_OFFSET
badcc07981f3 drm/i915: Allow i915 to manage the vma offset nodes instead of drm core

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15327/index.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [Intel-gfx] ✗ Fi.CI.BAT: failure for series starting with [1/4] drm/i915: Allow i915 to manage the vma offset nodes instead of drm core
@ 2019-11-19 13:25   ` Patchwork
  0 siblings, 0 replies; 23+ messages in thread
From: Patchwork @ 2019-11-19 13:25 UTC (permalink / raw)
  To: Abdiel Janulgue; +Cc: intel-gfx

== Series Details ==

Series: series starting with [1/4] drm/i915: Allow i915 to manage the vma offset nodes instead of drm core
URL   : https://patchwork.freedesktop.org/series/69674/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_7370 -> Patchwork_15327
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_15327 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_15327, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15327/index.html

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in Patchwork_15327:

### IGT changes ###

#### Possible regressions ####

  * igt@kms_pipe_crc_basic@hang-read-crc-pipe-a:
    - fi-gdg-551:         [PASS][1] -> [FAIL][2] +1 similar issue
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7370/fi-gdg-551/igt@kms_pipe_crc_basic@hang-read-crc-pipe-a.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15327/fi-gdg-551/igt@kms_pipe_crc_basic@hang-read-crc-pipe-a.html

  * igt@kms_pipe_crc_basic@nonblocking-crc-pipe-a-frame-sequence:
    - fi-ilk-650:         [PASS][3] -> [FAIL][4] +6 similar issues
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7370/fi-ilk-650/igt@kms_pipe_crc_basic@nonblocking-crc-pipe-a-frame-sequence.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15327/fi-ilk-650/igt@kms_pipe_crc_basic@nonblocking-crc-pipe-a-frame-sequence.html

  * igt@kms_pipe_crc_basic@read-crc-pipe-a:
    - fi-bwr-2160:        [PASS][5] -> [FAIL][6] +5 similar issues
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7370/fi-bwr-2160/igt@kms_pipe_crc_basic@read-crc-pipe-a.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15327/fi-bwr-2160/igt@kms_pipe_crc_basic@read-crc-pipe-a.html

  * igt@kms_pipe_crc_basic@read-crc-pipe-b:
    - fi-blb-e6850:       [PASS][7] -> [FAIL][8] +6 similar issues
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7370/fi-blb-e6850/igt@kms_pipe_crc_basic@read-crc-pipe-b.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15327/fi-blb-e6850/igt@kms_pipe_crc_basic@read-crc-pipe-b.html

  * igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a:
    - fi-bsw-kefka:       [PASS][9] -> [FAIL][10] +3 similar issues
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7370/fi-bsw-kefka/igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15327/fi-bsw-kefka/igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a.html

  
Known issues
------------

  Here are the changes found in Patchwork_15327 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@i915_pm_rpm@module-reload:
    - fi-skl-6770hq:      [PASS][11] -> [DMESG-WARN][12] ([fdo#112261])
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7370/fi-skl-6770hq/igt@i915_pm_rpm@module-reload.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15327/fi-skl-6770hq/igt@i915_pm_rpm@module-reload.html

  * igt@kms_pipe_crc_basic@nonblocking-crc-pipe-a-frame-sequence:
    - fi-byt-n2820:       [PASS][13] -> [FAIL][14] ([fdo#103191]) +4 similar issues
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7370/fi-byt-n2820/igt@kms_pipe_crc_basic@nonblocking-crc-pipe-a-frame-sequence.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15327/fi-byt-n2820/igt@kms_pipe_crc_basic@nonblocking-crc-pipe-a-frame-sequence.html

  * igt@kms_pipe_crc_basic@read-crc-pipe-a-frame-sequence:
    - fi-byt-j1900:       [PASS][15] -> [FAIL][16] ([fdo#103191]) +5 similar issues
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7370/fi-byt-j1900/igt@kms_pipe_crc_basic@read-crc-pipe-a-frame-sequence.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15327/fi-byt-j1900/igt@kms_pipe_crc_basic@read-crc-pipe-a-frame-sequence.html

  * igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a:
    - fi-glk-dsi:         [PASS][17] -> [FAIL][18] ([fdo#103191]) +3 similar issues
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7370/fi-glk-dsi/igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15327/fi-glk-dsi/igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a.html

  
#### Possible fixes ####

  * igt@gem_cpu_reloc@basic:
    - fi-icl-dsi:         [DMESG-WARN][19] ([fdo#106107]) -> [PASS][20]
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7370/fi-icl-dsi/igt@gem_cpu_reloc@basic.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15327/fi-icl-dsi/igt@gem_cpu_reloc@basic.html

  * igt@i915_pm_rpm@module-reload:
    - {fi-kbl-7560u}:     [FAIL][21] ([fdo#112269]) -> [PASS][22]
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7370/fi-kbl-7560u/igt@i915_pm_rpm@module-reload.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15327/fi-kbl-7560u/igt@i915_pm_rpm@module-reload.html

  * igt@i915_selftest@live_gem_contexts:
    - fi-skl-lmem:        [DMESG-FAIL][23] -> [PASS][24]
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7370/fi-skl-lmem/igt@i915_selftest@live_gem_contexts.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15327/fi-skl-lmem/igt@i915_selftest@live_gem_contexts.html

  * igt@kms_chamelium@hdmi-hpd-fast:
    - fi-kbl-7500u:       [FAIL][25] ([fdo#111407]) -> [PASS][26]
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7370/fi-kbl-7500u/igt@kms_chamelium@hdmi-hpd-fast.html
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15327/fi-kbl-7500u/igt@kms_chamelium@hdmi-hpd-fast.html

  * igt@kms_flip@basic-flip-vs-dpms:
    - fi-skl-6770hq:      [SKIP][27] ([fdo#109271]) -> [PASS][28] +27 similar issues
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7370/fi-skl-6770hq/igt@kms_flip@basic-flip-vs-dpms.html
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15327/fi-skl-6770hq/igt@kms_flip@basic-flip-vs-dpms.html

  * igt@kms_frontbuffer_tracking@basic:
    - fi-hsw-peppy:       [DMESG-WARN][29] ([fdo#102614]) -> [PASS][30]
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7370/fi-hsw-peppy/igt@kms_frontbuffer_tracking@basic.html
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15327/fi-hsw-peppy/igt@kms_frontbuffer_tracking@basic.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#102614]: https://bugs.freedesktop.org/show_bug.cgi?id=102614
  [fdo#103191]: https://bugs.freedesktop.org/show_bug.cgi?id=103191
  [fdo#106107]: https://bugs.freedesktop.org/show_bug.cgi?id=106107
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#111407]: https://bugs.freedesktop.org/show_bug.cgi?id=111407
  [fdo#112261]: https://bugs.freedesktop.org/show_bug.cgi?id=112261
  [fdo#112269]: https://bugs.freedesktop.org/show_bug.cgi?id=112269


Participating hosts (49 -> 44)
------------------------------

  Additional (1): fi-hsw-4770r 
  Missing    (6): fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 fi-byt-clapper fi-bdw-samus 


Build changes
-------------

  * CI: CI-20190529 -> None
  * Linux: CI_DRM_7370 -> Patchwork_15327

  CI-20190529: 20190529
  CI_DRM_7370: d2db945edccfe34bc3cc1d0accafac2036ad4e66 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5293: 4bb46f08f7cb6485642c4351cecdad93072d27a0 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_15327: 84e8b7b058852566875db9ede43e8d1d99cf400f @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

84e8b7b05885 drm/i915: Add cpu fault handler for mmap_offset
f4981991370d drm/i915: cpu-map based dumb buffers
614f0a31f6bf drm/i915: Introduce DRM_I915_GEM_MMAP_OFFSET
badcc07981f3 drm/i915: Allow i915 to manage the vma offset nodes instead of drm core

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15327/index.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 2/4] drm/i915: Introduce DRM_I915_GEM_MMAP_OFFSET
@ 2019-11-22 18:49     ` Ville Syrjälä
  0 siblings, 0 replies; 23+ messages in thread
From: Ville Syrjälä @ 2019-11-22 18:49 UTC (permalink / raw)
  To: Abdiel Janulgue; +Cc: intel-gfx, Matthew Auld

On Tue, Nov 19, 2019 at 01:37:08PM +0200, Abdiel Janulgue wrote:
> This is really just an alias of mmap_gtt. The 'mmap offset' nomenclature
> comes from the value returned by this ioctl which is the offset into the
> device fd which userpace uses with mmap(2).
> 
> mmap_gtt was our initial mmap_offset implementation, this extends
> our CPU mmap support to allow additional fault handlers that depends on
> the object's backing pages.
> 
> Note that we multiplex mmap_gtt and mmap_offset through the same ioctl,
> and use the zero extending behaviour of drm to differentiate between
> them, when we inspect the flags.
> 
> v2:
> - Drop the alias, just rename the struct (Chris)
> - Don't bail out on no PAT when doing WB mmaps
> - Prepare uAPI for further extensions
> v3:
> - drop MMAP_OFFSET_FLAGS
> v4:
> - Tweaks, header re-org
> 
> Signed-off-by: Abdiel Janulgue <abdiel.janulgue@linux.intel.com>
> Signed-off-by: Matthew Auld <matthew.auld@intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/gem/i915_gem_ioctls.h    |  4 +-
>  drivers/gpu/drm/i915/gem/i915_gem_mman.c      | 45 ++++++++++++++++---
>  drivers/gpu/drm/i915/gem/i915_gem_mman.h      |  1 +
>  .../gpu/drm/i915/gem/i915_gem_object_types.h  |  3 ++
>  drivers/gpu/drm/i915/i915_drv.c               |  2 +-
>  drivers/gpu/drm/i915/i915_drv.h               |  1 -
>  include/uapi/drm/i915_drm.h                   | 27 +++++++++++
>  7 files changed, 72 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ioctls.h b/drivers/gpu/drm/i915/gem/i915_gem_ioctls.h
> index ddc7f2a52b3e..87d8b27f426d 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_ioctls.h
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_ioctls.h
> @@ -28,8 +28,8 @@ int i915_gem_madvise_ioctl(struct drm_device *dev, void *data,
>  			   struct drm_file *file);
>  int i915_gem_mmap_ioctl(struct drm_device *dev, void *data,
>  			struct drm_file *file);
> -int i915_gem_mmap_gtt_ioctl(struct drm_device *dev, void *data,
> -			    struct drm_file *file);
> +int i915_gem_mmap_offset_ioctl(struct drm_device *dev, void *data,
> +			       struct drm_file *file);
>  int i915_gem_pread_ioctl(struct drm_device *dev, void *data,
>  			 struct drm_file *file);
>  int i915_gem_pwrite_ioctl(struct drm_device *dev, void *data,
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_mman.c b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
> index 36fffb671601..bb05c53c03c8 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_mman.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
> @@ -145,6 +145,9 @@ static unsigned int tile_row_pages(const struct drm_i915_gem_object *obj)
>   * 3 - Remove implicit set-domain(GTT) and synchronisation on initial
>   *     pagefault; swapin remains transparent.
>   *
> + * 4 - Support multiple fault handlers per object depending on object's
> + *     backing storage (a.k.a. MMAP_OFFSET).
> + *
>   * Restrictions:
>   *
>   *  * snoopable objects cannot be accessed via the GTT. It can cause machine
> @@ -172,7 +175,7 @@ static unsigned int tile_row_pages(const struct drm_i915_gem_object *obj)
>   */
>  int i915_gem_mmap_gtt_version(void)
>  {
> -	return 3;
> +	return 4;
>  }
>  
>  static inline struct i915_ggtt_view
> @@ -538,7 +541,7 @@ __assign_gem_object_mmap_data(struct drm_file *file,
>  }
>  
>  /**
> - * i915_gem_mmap_gtt_ioctl - prepare an object for GTT mmap'ing
> + * i915_gem_mmap_offset_ioctl - prepare an object for GTT mmap'ing
>   * @dev: DRM device
>   * @data: GTT mapping ioctl data
>   * @file: GEM object info
> @@ -553,13 +556,41 @@ __assign_gem_object_mmap_data(struct drm_file *file,
>   * userspace.
>   */
>  int
> -i915_gem_mmap_gtt_ioctl(struct drm_device *dev, void *data,
> -			struct drm_file *file)
> +i915_gem_mmap_offset_ioctl(struct drm_device *dev, void *data,
> +			   struct drm_file *file)
>  {
> -	struct drm_i915_gem_mmap_gtt *args = data;
> +	struct drm_i915_private *i915 = to_i915(dev);
> +	struct drm_i915_gem_mmap_offset *args = data;
> +	enum i915_mmap_type type;
> +
> +	switch (args->flags) {
> +	case I915_MMAP_OFFSET_GTT:
> +		if (!i915_ggtt_has_aperture(&i915->ggtt))
> +			return -ENODEV;
> +		type = I915_MMAP_TYPE_GTT;
> +		break;
> +
> +	case I915_MMAP_OFFSET_WC:
> +		if (!boot_cpu_has(X86_FEATURE_PAT))
> +			return -ENODEV;
> +		type = I915_MMAP_TYPE_WC;
> +		break;
> +
> +	case I915_MMAP_OFFSET_WB:
> +		type = I915_MMAP_TYPE_WB;
> +		break;
> +
> +	case I915_MMAP_OFFSET_UC:
> +		if (!boot_cpu_has(X86_FEATURE_PAT))
> +			return -ENODEV;
> +		type = I915_MMAP_TYPE_UC;
> +		break;
> +
> +	default:
> +		return -EINVAL;
> +	}
>  
> -	return __assign_gem_object_mmap_data(file, args->handle,
> -					     I915_MMAP_TYPE_GTT,
> +	return __assign_gem_object_mmap_data(file, args->handle, type,
>  					     &args->offset);
>  }
>  
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_mman.h b/drivers/gpu/drm/i915/gem/i915_gem_mman.h
> index 25a3c4d6cd65..4d3b493e853a 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_mman.h
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_mman.h
> @@ -24,5 +24,6 @@ void i915_mmap_offset_destroy(struct i915_mmap_offset *mmo, struct mutex *mutex)
>  void __i915_gem_object_release_mmap_gtt(struct drm_i915_gem_object *obj);
>  void i915_gem_object_release_mmap(struct drm_i915_gem_object *obj);
>  void i915_gem_object_release_mmap_offset(struct drm_i915_gem_object *obj);
> +int i915_gem_mmap_gtt_version(void);
>  
>  #endif
> 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 8ff0834a1d5c..c0beb97b56d1 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
> @@ -65,6 +65,9 @@ struct drm_i915_gem_object_ops {
>  
>  enum i915_mmap_type {
>  	I915_MMAP_TYPE_GTT = 0,
> +	I915_MMAP_TYPE_WC,
> +	I915_MMAP_TYPE_WB,
> +	I915_MMAP_TYPE_UC,
>  };
>  
>  struct i915_mmap_offset {
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 3e129bcf8a12..ac6d4470ce75 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -2715,7 +2715,7 @@ static const struct drm_ioctl_desc i915_ioctls[] = {
>  	DRM_IOCTL_DEF_DRV(I915_GEM_PREAD, i915_gem_pread_ioctl, DRM_RENDER_ALLOW),
>  	DRM_IOCTL_DEF_DRV(I915_GEM_PWRITE, i915_gem_pwrite_ioctl, DRM_RENDER_ALLOW),
>  	DRM_IOCTL_DEF_DRV(I915_GEM_MMAP, i915_gem_mmap_ioctl, DRM_RENDER_ALLOW),
> -	DRM_IOCTL_DEF_DRV(I915_GEM_MMAP_GTT, i915_gem_mmap_gtt_ioctl, DRM_RENDER_ALLOW),
> +	DRM_IOCTL_DEF_DRV(I915_GEM_MMAP_OFFSET, i915_gem_mmap_offset_ioctl, DRM_RENDER_ALLOW),
>  	DRM_IOCTL_DEF_DRV(I915_GEM_SET_DOMAIN, i915_gem_set_domain_ioctl, DRM_RENDER_ALLOW),
>  	DRM_IOCTL_DEF_DRV(I915_GEM_SW_FINISH, i915_gem_sw_finish_ioctl, DRM_RENDER_ALLOW),
>  	DRM_IOCTL_DEF_DRV(I915_GEM_SET_TILING, i915_gem_set_tiling_ioctl, DRM_RENDER_ALLOW),
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index a36c2778c09c..b2af6df2eae0 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1853,7 +1853,6 @@ i915_mutex_lock_interruptible(struct drm_device *dev)
>  int i915_gem_dumb_create(struct drm_file *file_priv,
>  			 struct drm_device *dev,
>  			 struct drm_mode_create_dumb *args);
> -int i915_gem_mmap_gtt_version(void);
>  
>  int __must_check i915_gem_set_global_seqno(struct drm_device *dev, u32 seqno);
>  
> diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
> index 5400d7e057f1..e844c3a8d197 100644
> --- a/include/uapi/drm/i915_drm.h
> +++ b/include/uapi/drm/i915_drm.h
> @@ -395,6 +395,7 @@ typedef struct _drm_i915_sarea {
>  #define DRM_IOCTL_I915_GEM_PWRITE	DRM_IOW (DRM_COMMAND_BASE + DRM_I915_GEM_PWRITE, struct drm_i915_gem_pwrite)
>  #define DRM_IOCTL_I915_GEM_MMAP		DRM_IOWR(DRM_COMMAND_BASE + DRM_I915_GEM_MMAP, struct drm_i915_gem_mmap)
>  #define DRM_IOCTL_I915_GEM_MMAP_GTT	DRM_IOWR(DRM_COMMAND_BASE + DRM_I915_GEM_MMAP_GTT, struct drm_i915_gem_mmap_gtt)
> +#define DRM_IOCTL_I915_GEM_MMAP_OFFSET	DRM_IOWR(DRM_COMMAND_BASE + DRM_I915_GEM_MMAP_GTT, struct drm_i915_gem_mmap_offset)
>  #define DRM_IOCTL_I915_GEM_SET_DOMAIN	DRM_IOW (DRM_COMMAND_BASE + DRM_I915_GEM_SET_DOMAIN, struct drm_i915_gem_set_domain)
>  #define DRM_IOCTL_I915_GEM_SW_FINISH	DRM_IOW (DRM_COMMAND_BASE + DRM_I915_GEM_SW_FINISH, struct drm_i915_gem_sw_finish)
>  #define DRM_IOCTL_I915_GEM_SET_TILING	DRM_IOWR (DRM_COMMAND_BASE + DRM_I915_GEM_SET_TILING, struct drm_i915_gem_set_tiling)
> @@ -793,6 +794,32 @@ struct drm_i915_gem_mmap_gtt {
>  	__u64 offset;
>  };
>  
> +struct drm_i915_gem_mmap_offset {
> +	/** Handle for the object being mapped. */
> +	__u32 handle;
> +	__u32 pad;
> +	/**
> +	 * Fake offset to use for subsequent mmap call
> +	 *
> +	 * This is a fixed-size type for 32/64 compatibility.
> +	 */
> +	__u64 offset;
> +
> +	/**
> +	 * Flags for extended behaviour.
> +	 *
> +	 * It is mandatory that either one of the MMAP_OFFSET flags
> +	 * should be passed here.

What does "either one" mean? "at least one", or "one and only one"?

> +	 */
> +	__u64 flags;
> +#define I915_MMAP_OFFSET_GTT 0
> +#define I915_MMAP_OFFSET_WC  1
> +#define I915_MMAP_OFFSET_WB  2
> +#define I915_MMAP_OFFSET_UC  3

These are mean to be used as I915_MMAP_OFFSET_FOO << 0
or 1 << I915_MMAP_OFFSET_FOO ?

/me looks at the code. Seems like the <<0 version is correct,
and that also explains the earlier question since you can't
even pass more than one, nor can you pass none since
I915_MMAP_OFFSET_GTT is 0.

Maybe these defines should inclue <<0 in them to make the
usage 100% obvious?

> +
> +	__u64 extensions;
> +};
> +
>  struct drm_i915_gem_set_domain {
>  	/** Handle for the object */
>  	__u32 handle;
> -- 
> 2.23.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [Intel-gfx] [PATCH 2/4] drm/i915: Introduce DRM_I915_GEM_MMAP_OFFSET
@ 2019-11-22 18:49     ` Ville Syrjälä
  0 siblings, 0 replies; 23+ messages in thread
From: Ville Syrjälä @ 2019-11-22 18:49 UTC (permalink / raw)
  To: Abdiel Janulgue; +Cc: intel-gfx, Matthew Auld

On Tue, Nov 19, 2019 at 01:37:08PM +0200, Abdiel Janulgue wrote:
> This is really just an alias of mmap_gtt. The 'mmap offset' nomenclature
> comes from the value returned by this ioctl which is the offset into the
> device fd which userpace uses with mmap(2).
> 
> mmap_gtt was our initial mmap_offset implementation, this extends
> our CPU mmap support to allow additional fault handlers that depends on
> the object's backing pages.
> 
> Note that we multiplex mmap_gtt and mmap_offset through the same ioctl,
> and use the zero extending behaviour of drm to differentiate between
> them, when we inspect the flags.
> 
> v2:
> - Drop the alias, just rename the struct (Chris)
> - Don't bail out on no PAT when doing WB mmaps
> - Prepare uAPI for further extensions
> v3:
> - drop MMAP_OFFSET_FLAGS
> v4:
> - Tweaks, header re-org
> 
> Signed-off-by: Abdiel Janulgue <abdiel.janulgue@linux.intel.com>
> Signed-off-by: Matthew Auld <matthew.auld@intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/gem/i915_gem_ioctls.h    |  4 +-
>  drivers/gpu/drm/i915/gem/i915_gem_mman.c      | 45 ++++++++++++++++---
>  drivers/gpu/drm/i915/gem/i915_gem_mman.h      |  1 +
>  .../gpu/drm/i915/gem/i915_gem_object_types.h  |  3 ++
>  drivers/gpu/drm/i915/i915_drv.c               |  2 +-
>  drivers/gpu/drm/i915/i915_drv.h               |  1 -
>  include/uapi/drm/i915_drm.h                   | 27 +++++++++++
>  7 files changed, 72 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ioctls.h b/drivers/gpu/drm/i915/gem/i915_gem_ioctls.h
> index ddc7f2a52b3e..87d8b27f426d 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_ioctls.h
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_ioctls.h
> @@ -28,8 +28,8 @@ int i915_gem_madvise_ioctl(struct drm_device *dev, void *data,
>  			   struct drm_file *file);
>  int i915_gem_mmap_ioctl(struct drm_device *dev, void *data,
>  			struct drm_file *file);
> -int i915_gem_mmap_gtt_ioctl(struct drm_device *dev, void *data,
> -			    struct drm_file *file);
> +int i915_gem_mmap_offset_ioctl(struct drm_device *dev, void *data,
> +			       struct drm_file *file);
>  int i915_gem_pread_ioctl(struct drm_device *dev, void *data,
>  			 struct drm_file *file);
>  int i915_gem_pwrite_ioctl(struct drm_device *dev, void *data,
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_mman.c b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
> index 36fffb671601..bb05c53c03c8 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_mman.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
> @@ -145,6 +145,9 @@ static unsigned int tile_row_pages(const struct drm_i915_gem_object *obj)
>   * 3 - Remove implicit set-domain(GTT) and synchronisation on initial
>   *     pagefault; swapin remains transparent.
>   *
> + * 4 - Support multiple fault handlers per object depending on object's
> + *     backing storage (a.k.a. MMAP_OFFSET).
> + *
>   * Restrictions:
>   *
>   *  * snoopable objects cannot be accessed via the GTT. It can cause machine
> @@ -172,7 +175,7 @@ static unsigned int tile_row_pages(const struct drm_i915_gem_object *obj)
>   */
>  int i915_gem_mmap_gtt_version(void)
>  {
> -	return 3;
> +	return 4;
>  }
>  
>  static inline struct i915_ggtt_view
> @@ -538,7 +541,7 @@ __assign_gem_object_mmap_data(struct drm_file *file,
>  }
>  
>  /**
> - * i915_gem_mmap_gtt_ioctl - prepare an object for GTT mmap'ing
> + * i915_gem_mmap_offset_ioctl - prepare an object for GTT mmap'ing
>   * @dev: DRM device
>   * @data: GTT mapping ioctl data
>   * @file: GEM object info
> @@ -553,13 +556,41 @@ __assign_gem_object_mmap_data(struct drm_file *file,
>   * userspace.
>   */
>  int
> -i915_gem_mmap_gtt_ioctl(struct drm_device *dev, void *data,
> -			struct drm_file *file)
> +i915_gem_mmap_offset_ioctl(struct drm_device *dev, void *data,
> +			   struct drm_file *file)
>  {
> -	struct drm_i915_gem_mmap_gtt *args = data;
> +	struct drm_i915_private *i915 = to_i915(dev);
> +	struct drm_i915_gem_mmap_offset *args = data;
> +	enum i915_mmap_type type;
> +
> +	switch (args->flags) {
> +	case I915_MMAP_OFFSET_GTT:
> +		if (!i915_ggtt_has_aperture(&i915->ggtt))
> +			return -ENODEV;
> +		type = I915_MMAP_TYPE_GTT;
> +		break;
> +
> +	case I915_MMAP_OFFSET_WC:
> +		if (!boot_cpu_has(X86_FEATURE_PAT))
> +			return -ENODEV;
> +		type = I915_MMAP_TYPE_WC;
> +		break;
> +
> +	case I915_MMAP_OFFSET_WB:
> +		type = I915_MMAP_TYPE_WB;
> +		break;
> +
> +	case I915_MMAP_OFFSET_UC:
> +		if (!boot_cpu_has(X86_FEATURE_PAT))
> +			return -ENODEV;
> +		type = I915_MMAP_TYPE_UC;
> +		break;
> +
> +	default:
> +		return -EINVAL;
> +	}
>  
> -	return __assign_gem_object_mmap_data(file, args->handle,
> -					     I915_MMAP_TYPE_GTT,
> +	return __assign_gem_object_mmap_data(file, args->handle, type,
>  					     &args->offset);
>  }
>  
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_mman.h b/drivers/gpu/drm/i915/gem/i915_gem_mman.h
> index 25a3c4d6cd65..4d3b493e853a 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_mman.h
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_mman.h
> @@ -24,5 +24,6 @@ void i915_mmap_offset_destroy(struct i915_mmap_offset *mmo, struct mutex *mutex)
>  void __i915_gem_object_release_mmap_gtt(struct drm_i915_gem_object *obj);
>  void i915_gem_object_release_mmap(struct drm_i915_gem_object *obj);
>  void i915_gem_object_release_mmap_offset(struct drm_i915_gem_object *obj);
> +int i915_gem_mmap_gtt_version(void);
>  
>  #endif
> 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 8ff0834a1d5c..c0beb97b56d1 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
> @@ -65,6 +65,9 @@ struct drm_i915_gem_object_ops {
>  
>  enum i915_mmap_type {
>  	I915_MMAP_TYPE_GTT = 0,
> +	I915_MMAP_TYPE_WC,
> +	I915_MMAP_TYPE_WB,
> +	I915_MMAP_TYPE_UC,
>  };
>  
>  struct i915_mmap_offset {
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 3e129bcf8a12..ac6d4470ce75 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -2715,7 +2715,7 @@ static const struct drm_ioctl_desc i915_ioctls[] = {
>  	DRM_IOCTL_DEF_DRV(I915_GEM_PREAD, i915_gem_pread_ioctl, DRM_RENDER_ALLOW),
>  	DRM_IOCTL_DEF_DRV(I915_GEM_PWRITE, i915_gem_pwrite_ioctl, DRM_RENDER_ALLOW),
>  	DRM_IOCTL_DEF_DRV(I915_GEM_MMAP, i915_gem_mmap_ioctl, DRM_RENDER_ALLOW),
> -	DRM_IOCTL_DEF_DRV(I915_GEM_MMAP_GTT, i915_gem_mmap_gtt_ioctl, DRM_RENDER_ALLOW),
> +	DRM_IOCTL_DEF_DRV(I915_GEM_MMAP_OFFSET, i915_gem_mmap_offset_ioctl, DRM_RENDER_ALLOW),
>  	DRM_IOCTL_DEF_DRV(I915_GEM_SET_DOMAIN, i915_gem_set_domain_ioctl, DRM_RENDER_ALLOW),
>  	DRM_IOCTL_DEF_DRV(I915_GEM_SW_FINISH, i915_gem_sw_finish_ioctl, DRM_RENDER_ALLOW),
>  	DRM_IOCTL_DEF_DRV(I915_GEM_SET_TILING, i915_gem_set_tiling_ioctl, DRM_RENDER_ALLOW),
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index a36c2778c09c..b2af6df2eae0 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1853,7 +1853,6 @@ i915_mutex_lock_interruptible(struct drm_device *dev)
>  int i915_gem_dumb_create(struct drm_file *file_priv,
>  			 struct drm_device *dev,
>  			 struct drm_mode_create_dumb *args);
> -int i915_gem_mmap_gtt_version(void);
>  
>  int __must_check i915_gem_set_global_seqno(struct drm_device *dev, u32 seqno);
>  
> diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
> index 5400d7e057f1..e844c3a8d197 100644
> --- a/include/uapi/drm/i915_drm.h
> +++ b/include/uapi/drm/i915_drm.h
> @@ -395,6 +395,7 @@ typedef struct _drm_i915_sarea {
>  #define DRM_IOCTL_I915_GEM_PWRITE	DRM_IOW (DRM_COMMAND_BASE + DRM_I915_GEM_PWRITE, struct drm_i915_gem_pwrite)
>  #define DRM_IOCTL_I915_GEM_MMAP		DRM_IOWR(DRM_COMMAND_BASE + DRM_I915_GEM_MMAP, struct drm_i915_gem_mmap)
>  #define DRM_IOCTL_I915_GEM_MMAP_GTT	DRM_IOWR(DRM_COMMAND_BASE + DRM_I915_GEM_MMAP_GTT, struct drm_i915_gem_mmap_gtt)
> +#define DRM_IOCTL_I915_GEM_MMAP_OFFSET	DRM_IOWR(DRM_COMMAND_BASE + DRM_I915_GEM_MMAP_GTT, struct drm_i915_gem_mmap_offset)
>  #define DRM_IOCTL_I915_GEM_SET_DOMAIN	DRM_IOW (DRM_COMMAND_BASE + DRM_I915_GEM_SET_DOMAIN, struct drm_i915_gem_set_domain)
>  #define DRM_IOCTL_I915_GEM_SW_FINISH	DRM_IOW (DRM_COMMAND_BASE + DRM_I915_GEM_SW_FINISH, struct drm_i915_gem_sw_finish)
>  #define DRM_IOCTL_I915_GEM_SET_TILING	DRM_IOWR (DRM_COMMAND_BASE + DRM_I915_GEM_SET_TILING, struct drm_i915_gem_set_tiling)
> @@ -793,6 +794,32 @@ struct drm_i915_gem_mmap_gtt {
>  	__u64 offset;
>  };
>  
> +struct drm_i915_gem_mmap_offset {
> +	/** Handle for the object being mapped. */
> +	__u32 handle;
> +	__u32 pad;
> +	/**
> +	 * Fake offset to use for subsequent mmap call
> +	 *
> +	 * This is a fixed-size type for 32/64 compatibility.
> +	 */
> +	__u64 offset;
> +
> +	/**
> +	 * Flags for extended behaviour.
> +	 *
> +	 * It is mandatory that either one of the MMAP_OFFSET flags
> +	 * should be passed here.

What does "either one" mean? "at least one", or "one and only one"?

> +	 */
> +	__u64 flags;
> +#define I915_MMAP_OFFSET_GTT 0
> +#define I915_MMAP_OFFSET_WC  1
> +#define I915_MMAP_OFFSET_WB  2
> +#define I915_MMAP_OFFSET_UC  3

These are mean to be used as I915_MMAP_OFFSET_FOO << 0
or 1 << I915_MMAP_OFFSET_FOO ?

/me looks at the code. Seems like the <<0 version is correct,
and that also explains the earlier question since you can't
even pass more than one, nor can you pass none since
I915_MMAP_OFFSET_GTT is 0.

Maybe these defines should inclue <<0 in them to make the
usage 100% obvious?

> +
> +	__u64 extensions;
> +};
> +
>  struct drm_i915_gem_set_domain {
>  	/** Handle for the object */
>  	__u32 handle;
> -- 
> 2.23.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 4/4] drm/i915: Add cpu fault handler for mmap_offset
@ 2019-11-28 11:08     ` Chris Wilson
  0 siblings, 0 replies; 23+ messages in thread
From: Chris Wilson @ 2019-11-28 11:08 UTC (permalink / raw)
  To: Abdiel Janulgue, intel-gfx; +Cc: Matthew Auld

Quoting Abdiel Janulgue (2019-11-19 11:37:10)
> Fault handler to handle missing pages for shmem-backed objects.
> 
> v2: bail out of inserting PTEs when failing to insert the
>     fault address
> v3: has struct page check
> v4: Add self-test for validating CPU fault handler to ensure PTEs
>     are revoked when an object is unbound.
> v5: Add comment where PTEs are revoked (Chris)
> 
> Signed-off-by: Abdiel Janulgue <abdiel.janulgue@linux.intel.com>
> Signed-off-by: Matthew Auld <matthew.auld@intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>

Uh oh, the mmap() does remain valid past a gem_close() -- the
vm_area_struct is lacking a reference to the object?
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [Intel-gfx] [PATCH 4/4] drm/i915: Add cpu fault handler for mmap_offset
@ 2019-11-28 11:08     ` Chris Wilson
  0 siblings, 0 replies; 23+ messages in thread
From: Chris Wilson @ 2019-11-28 11:08 UTC (permalink / raw)
  To: Abdiel Janulgue, intel-gfx; +Cc: Matthew Auld

Quoting Abdiel Janulgue (2019-11-19 11:37:10)
> Fault handler to handle missing pages for shmem-backed objects.
> 
> v2: bail out of inserting PTEs when failing to insert the
>     fault address
> v3: has struct page check
> v4: Add self-test for validating CPU fault handler to ensure PTEs
>     are revoked when an object is unbound.
> v5: Add comment where PTEs are revoked (Chris)
> 
> Signed-off-by: Abdiel Janulgue <abdiel.janulgue@linux.intel.com>
> Signed-off-by: Matthew Auld <matthew.auld@intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>

Uh oh, the mmap() does remain valid past a gem_close() -- the
vm_area_struct is lacking a reference to the object?
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 4/4] drm/i915: Add cpu fault handler for mmap_offset
@ 2019-11-28 11:22       ` Chris Wilson
  0 siblings, 0 replies; 23+ messages in thread
From: Chris Wilson @ 2019-11-28 11:22 UTC (permalink / raw)
  To: Abdiel Janulgue, intel-gfx; +Cc: Matthew Auld

Quoting Chris Wilson (2019-11-28 11:08:35)
> Quoting Abdiel Janulgue (2019-11-19 11:37:10)
> > Fault handler to handle missing pages for shmem-backed objects.
> > 
> > v2: bail out of inserting PTEs when failing to insert the
> >     fault address
> > v3: has struct page check
> > v4: Add self-test for validating CPU fault handler to ensure PTEs
> >     are revoked when an object is unbound.
> > v5: Add comment where PTEs are revoked (Chris)
> > 
> > Signed-off-by: Abdiel Janulgue <abdiel.janulgue@linux.intel.com>
> > Signed-off-by: Matthew Auld <matthew.auld@intel.com>
> > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> > Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> 
> Uh oh, the mmap() does remain valid past a gem_close() -- the
> vm_area_struct is lacking a reference to the object?

Hmm, test is misleading -- may just be a coherency bug.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [Intel-gfx] [PATCH 4/4] drm/i915: Add cpu fault handler for mmap_offset
@ 2019-11-28 11:22       ` Chris Wilson
  0 siblings, 0 replies; 23+ messages in thread
From: Chris Wilson @ 2019-11-28 11:22 UTC (permalink / raw)
  To: Abdiel Janulgue, intel-gfx; +Cc: Matthew Auld

Quoting Chris Wilson (2019-11-28 11:08:35)
> Quoting Abdiel Janulgue (2019-11-19 11:37:10)
> > Fault handler to handle missing pages for shmem-backed objects.
> > 
> > v2: bail out of inserting PTEs when failing to insert the
> >     fault address
> > v3: has struct page check
> > v4: Add self-test for validating CPU fault handler to ensure PTEs
> >     are revoked when an object is unbound.
> > v5: Add comment where PTEs are revoked (Chris)
> > 
> > Signed-off-by: Abdiel Janulgue <abdiel.janulgue@linux.intel.com>
> > Signed-off-by: Matthew Auld <matthew.auld@intel.com>
> > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> > Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> 
> Uh oh, the mmap() does remain valid past a gem_close() -- the
> vm_area_struct is lacking a reference to the object?

Hmm, test is misleading -- may just be a coherency bug.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 4/4] drm/i915: Add cpu fault handler for mmap_offset
  2019-11-15 11:45 ` [PATCH 4/4] drm/i915: Add cpu fault handler for mmap_offset Abdiel Janulgue
@ 2019-11-15 13:58   ` Chris Wilson
  0 siblings, 0 replies; 23+ messages in thread
From: Chris Wilson @ 2019-11-15 13:58 UTC (permalink / raw)
  To: Abdiel Janulgue, intel-gfx; +Cc: Matthew Auld

Quoting Abdiel Janulgue (2019-11-15 11:45:49)
> +static vm_fault_t i915_gem_fault_cpu(struct vm_fault *vmf)
> +{
> +       struct vm_area_struct *area = vmf->vma;
> +       struct i915_mmap_offset *priv = area->vm_private_data;
> +       struct drm_i915_gem_object *obj = priv->obj;
> +       vm_fault_t vmf_ret;
> +       unsigned long i, size = area->vm_end - area->vm_start;
> +       bool write = area->vm_flags & VM_WRITE;
> +       int ret;
> +
> +       if (!i915_gem_object_has_struct_page(obj))
> +               return VM_FAULT_SIGBUS;
> +
> +       /* Sanity check that we allow writing into this object */
> +       if (i915_gem_object_is_readonly(obj) && write)
> +               return VM_FAULT_SIGBUS;
> +
> +       ret = i915_gem_object_pin_pages(obj);
> +       if (ret)
> +               return i915_error_to_vmf_fault(ret);
> +

It is probably a good idea to mention here how we revoke the PTE.

/* PTEs are revoked in obj->ops->put_pages() */

> +       for (i = 0; i < size >> PAGE_SHIFT; i++) {
> +               struct page *page = i915_gem_object_get_page(obj, i);
> +
> +               vmf_ret = vmf_insert_pfn(area,
> +                                        (unsigned long)area->vm_start + i * PAGE_SIZE,
> +                                        page_to_pfn(page));
> +               if (vmf_ret != VM_FAULT_NOPAGE)
> +                       break;
> +       }
> +
> +       i915_gem_object_unpin_pages(obj);
> +
> +       return vmf_ret;

With the revoke + selftests in place, this looks correct.
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>

There might be room for fine tuning (not pin/mapping the whole object),
but that has been a concern for a long time and remains a concern :)
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [PATCH 4/4] drm/i915: Add cpu fault handler for mmap_offset
  2019-11-15 11:45 [PATCH 1/4] " Abdiel Janulgue
@ 2019-11-15 11:45 ` Abdiel Janulgue
  2019-11-15 13:58   ` Chris Wilson
  0 siblings, 1 reply; 23+ messages in thread
From: Abdiel Janulgue @ 2019-11-15 11:45 UTC (permalink / raw)
  To: intel-gfx; +Cc: Matthew Auld

Fault handler to handle missing pages for shmem-backed objects.

v2: bail out of inserting PTEs when failing to insert the
    fault address
v3: has struct page check
v4: Add self-test for validating CPU fault handler to ensure PTEs
    are revoked when an object is unbound.

Signed-off-by: Abdiel Janulgue <abdiel.janulgue@linux.intel.com>
Signed-off-by: Matthew Auld <matthew.auld@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
 drivers/gpu/drm/i915/gem/i915_gem_mman.c      | 128 ++++++++++++++----
 .../drm/i915/gem/selftests/i915_gem_mman.c    |  47 +++++--
 2 files changed, 141 insertions(+), 34 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_mman.c b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
index c1756e4f46b9..2f7ed2e6dfa8 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/mman.h>
+#include <linux/pfn_t.h>
 #include <linux/sizes.h>
 
 #include "gt/intel_gt.h"
@@ -201,6 +202,70 @@ compute_partial_view(const struct drm_i915_gem_object *obj,
 	return view;
 }
 
+static vm_fault_t i915_error_to_vmf_fault(int err)
+{
+	switch (err) {
+	default:
+		WARN_ONCE(err, "unhandled error in %s: %i\n", __func__, err);
+		/* fallthrough */
+	case -EIO: /* shmemfs failure from swap device */
+	case -EFAULT: /* purged object */
+	case -ENODEV: /* bad object, how did you get here! */
+		return VM_FAULT_SIGBUS;
+
+	case -ENOSPC: /* shmemfs allocation failure */
+	case -ENOMEM: /* our allocation failure */
+		return VM_FAULT_OOM;
+
+	case 0:
+	case -EAGAIN:
+	case -ERESTARTSYS:
+	case -EINTR:
+	case -EBUSY:
+		/*
+		 * EBUSY is ok: this just means that another thread
+		 * already did the job.
+		 */
+		return VM_FAULT_NOPAGE;
+	}
+}
+
+static vm_fault_t i915_gem_fault_cpu(struct vm_fault *vmf)
+{
+	struct vm_area_struct *area = vmf->vma;
+	struct i915_mmap_offset *priv = area->vm_private_data;
+	struct drm_i915_gem_object *obj = priv->obj;
+	vm_fault_t vmf_ret;
+	unsigned long i, size = area->vm_end - area->vm_start;
+	bool write = area->vm_flags & VM_WRITE;
+	int ret;
+
+	if (!i915_gem_object_has_struct_page(obj))
+		return VM_FAULT_SIGBUS;
+
+	/* Sanity check that we allow writing into this object */
+	if (i915_gem_object_is_readonly(obj) && write)
+		return VM_FAULT_SIGBUS;
+
+	ret = i915_gem_object_pin_pages(obj);
+	if (ret)
+		return i915_error_to_vmf_fault(ret);
+
+	for (i = 0; i < size >> PAGE_SHIFT; i++) {
+		struct page *page = i915_gem_object_get_page(obj, i);
+
+		vmf_ret = vmf_insert_pfn(area,
+					 (unsigned long)area->vm_start + i * PAGE_SIZE,
+					 page_to_pfn(page));
+		if (vmf_ret != VM_FAULT_NOPAGE)
+			break;
+	}
+
+	i915_gem_object_unpin_pages(obj);
+
+	return vmf_ret;
+}
+
 /**
  * i915_gem_fault - fault a page into the GTT
  * @vmf: fault info
@@ -340,30 +405,7 @@ vm_fault_t i915_gem_fault(struct vm_fault *vmf)
 	intel_runtime_pm_put(rpm, wakeref);
 	i915_gem_object_unpin_pages(obj);
 err:
-	switch (ret) {
-	default:
-		WARN_ONCE(ret, "unhandled error in %s: %i\n", __func__, ret);
-		/* fallthrough */
-	case -EIO: /* shmemfs failure from swap device */
-	case -EFAULT: /* purged object */
-	case -ENODEV: /* bad object, how did you get here! */
-		return VM_FAULT_SIGBUS;
-
-	case -ENOSPC: /* shmemfs allocation failure */
-	case -ENOMEM: /* our allocation failure */
-		return VM_FAULT_OOM;
-
-	case 0:
-	case -EAGAIN:
-	case -ERESTARTSYS:
-	case -EINTR:
-	case -EBUSY:
-		/*
-		 * EBUSY is ok: this just means that another thread
-		 * already did the job.
-		 */
-		return VM_FAULT_NOPAGE;
-	}
+	return i915_error_to_vmf_fault(ret);
 }
 
 void __i915_gem_object_release_mmap_gtt(struct drm_i915_gem_object *obj)
@@ -650,6 +692,33 @@ static const struct vm_operations_struct i915_gem_gtt_vm_ops = {
 	.close = i915_gem_vm_close,
 };
 
+static const struct vm_operations_struct i915_gem_cpu_vm_ops = {
+	.fault = i915_gem_fault_cpu,
+	.open = i915_gem_vm_open,
+	.close = i915_gem_vm_close,
+};
+
+static void set_vmdata_mmap_offset(struct i915_mmap_offset *mmo, struct vm_area_struct *vma)
+{
+	switch (mmo->mmap_type) {
+	case I915_MMAP_TYPE_WC:
+		vma->vm_page_prot =
+			pgprot_writecombine(vm_get_page_prot(vma->vm_flags));
+		break;
+	case I915_MMAP_TYPE_WB:
+		vma->vm_page_prot = vm_get_page_prot(vma->vm_flags);
+		break;
+	case I915_MMAP_TYPE_UC:
+		vma->vm_page_prot =
+			pgprot_noncached(vm_get_page_prot(vma->vm_flags));
+		break;
+	default:
+		break;
+	}
+
+	vma->vm_ops = &i915_gem_cpu_vm_ops;
+}
+
 /* This overcomes the limitation in drm_gem_mmap's assignment of a
  * drm_gem_object as the vma->vm_private_data. Since we need to
  * be able to resolve multiple mmap offsets which could be tied
@@ -717,7 +786,16 @@ int i915_gem_mmap(struct file *filp, struct vm_area_struct *vma)
 	vma->vm_page_prot = pgprot_decrypted(vma->vm_page_prot);
 	vma->vm_private_data = mmo;
 
-	vma->vm_ops = &i915_gem_gtt_vm_ops;
+	switch (mmo->mmap_type) {
+	case I915_MMAP_TYPE_WC:
+	case I915_MMAP_TYPE_WB:
+	case I915_MMAP_TYPE_UC:
+		set_vmdata_mmap_offset(mmo, vma);
+		break;
+	case I915_MMAP_TYPE_GTT:
+		vma->vm_ops = &i915_gem_gtt_vm_ops;
+		break;
+	}
 
 	return 0;
 }
diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c b/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c
index 63e24f6b0d40..1bedc9d115c5 100644
--- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c
+++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c
@@ -731,11 +731,13 @@ static int igt_mmap_offset_exhaustion(void *arg)
 }
 
 #define expand32(x) (((x) << 0) | ((x) << 8) | ((x) << 16) | ((x) << 24))
-static int igt_mmap_gtt(void *arg)
+static int igt_mmap(void *arg, enum i915_mmap_type type)
 {
 	struct drm_i915_private *i915 = arg;
 	struct drm_i915_gem_object *obj;
 	struct vm_area_struct *area;
+	/* Ownership transferred to parent gem object in create_mmap_offset */
+	struct i915_mmap_offset *mmo = kzalloc(sizeof(*mmo), GFP_KERNEL);
 	unsigned long addr;
 	void *vaddr;
 	int err, i;
@@ -756,11 +758,12 @@ static int igt_mmap_gtt(void *arg)
 	i915_gem_object_flush_map(obj);
 	i915_gem_object_unpin_map(obj);
 
-	err = create_mmap_offset(obj);
+	err = create_mmap_offset(obj, mmo);
 	if (err)
 		goto out;
 
-	addr = igt_mmap_node(i915, &obj->base.vma_node,
+	mmo->mmap_type = type;
+	addr = igt_mmap_node(i915, &mmo->vma_node,
 			     0, PROT_WRITE, MAP_SHARED);
 	if (IS_ERR_VALUE(addr)) {
 		err = addr;
@@ -776,8 +779,8 @@ static int igt_mmap_gtt(void *arg)
 		goto out_unmap;
 	}
 
-	if (area->vm_private_data != obj) {
-		pr_err("vm_area_struct did not point back to our object!\n");
+	if (area->vm_private_data != mmo) {
+		pr_err("vm_area_struct did not point back to our mmap_offset object!\n");
 		err = -EINVAL;
 		goto out_unmap;
 	}
@@ -828,6 +831,16 @@ static int igt_mmap_gtt(void *arg)
 	return err;
 }
 
+static int igt_mmap_gtt(void *arg)
+{
+	return igt_mmap(arg, I915_MMAP_TYPE_GTT);
+}
+
+static int igt_mmap_cpu(void *arg)
+{
+	return igt_mmap(arg, I915_MMAP_TYPE_WC);
+}
+
 static int check_present_pte(pte_t *pte, unsigned long addr, void *data)
 {
 	if (!pte_present(*pte) || pte_none(*pte)) {
@@ -880,10 +893,12 @@ static int prefault_range(u64 start, u64 len)
 	return __get_user(c, end - 1);
 }
 
-static int igt_mmap_gtt_revoke(void *arg)
+static int igt_mmap_revoke(void *arg, enum i915_mmap_type type)
 {
 	struct drm_i915_private *i915 = arg;
 	struct drm_i915_gem_object *obj;
+	/* Ownership transferred to parent gem object in create_mmap_offset */
+	struct i915_mmap_offset *mmo = kzalloc(sizeof(*mmo), GFP_KERNEL);
 	unsigned long addr;
 	int err;
 
@@ -894,11 +909,12 @@ static int igt_mmap_gtt_revoke(void *arg)
 	if (IS_ERR(obj))
 		return PTR_ERR(obj);
 
-	err = create_mmap_offset(obj);
+	err = create_mmap_offset(obj, mmo);
 	if (err)
 		goto out;
 
-	addr = igt_mmap_node(i915, &obj->base.vma_node,
+	mmo->mmap_type = type;
+	addr = igt_mmap_node(i915, &mmo->vma_node,
 			     0, PROT_WRITE, MAP_SHARED);
 	if (IS_ERR_VALUE(addr)) {
 		err = addr;
@@ -909,7 +925,8 @@ static int igt_mmap_gtt_revoke(void *arg)
 	if (err)
 		goto out_unmap;
 
-	GEM_BUG_ON(!atomic_read(&obj->bind_count));
+	GEM_BUG_ON(mmo->mmap_type == I915_MMAP_TYPE_GTT &&
+		   !atomic_read(&obj->bind_count));
 
 	err = check_present(addr, obj->base.size);
 	if (err)
@@ -947,6 +964,16 @@ static int igt_mmap_gtt_revoke(void *arg)
 	return err;
 }
 
+static int igt_mmap_gtt_revoke(void *arg)
+{
+	return igt_mmap_revoke(arg, I915_MMAP_TYPE_GTT);
+}
+
+static int igt_mmap_cpu_revoke(void *arg)
+{
+	return igt_mmap_revoke(arg, I915_MMAP_TYPE_WC);
+}
+
 int i915_gem_mman_live_selftests(struct drm_i915_private *i915)
 {
 	static const struct i915_subtest tests[] = {
@@ -954,7 +981,9 @@ int i915_gem_mman_live_selftests(struct drm_i915_private *i915)
 		SUBTEST(igt_smoke_tiling),
 		SUBTEST(igt_mmap_offset_exhaustion),
 		SUBTEST(igt_mmap_gtt),
+		SUBTEST(igt_mmap_cpu),
 		SUBTEST(igt_mmap_gtt_revoke),
+		SUBTEST(igt_mmap_cpu_revoke),
 	};
 
 	return i915_subtests(tests, i915);
-- 
2.23.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 4/4] drm/i915: Add cpu fault handler for mmap_offset
  2019-11-14 19:09 [PATCH 1/4] drm/i915: Allow i915 to manage the vma offset nodes instead of drm core Abdiel Janulgue
@ 2019-11-14 19:09 ` Abdiel Janulgue
  0 siblings, 0 replies; 23+ messages in thread
From: Abdiel Janulgue @ 2019-11-14 19:09 UTC (permalink / raw)
  To: intel-gfx; +Cc: Matthew Auld

Fault handler to handle missing pages for shmem-backed objects.

v2: bail out of inserting PTEs when failing to insert the
    fault address
v3: has struct page check
v4: Add self-test for validating CPU fault handler to ensure PTEs
    are revoked when an object is unbound.

Signed-off-by: Abdiel Janulgue <abdiel.janulgue@linux.intel.com>
Signed-off-by: Matthew Auld <matthew.auld@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
 drivers/gpu/drm/i915/gem/i915_gem_mman.c      | 128 ++++++++++++++----
 .../drm/i915/gem/selftests/i915_gem_mman.c    |  47 +++++--
 2 files changed, 141 insertions(+), 34 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_mman.c b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
index d625f0f5d395..62ebc24b219f 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/mman.h>
+#include <linux/pfn_t.h>
 #include <linux/sizes.h>
 
 #include "gt/intel_gt.h"
@@ -200,6 +201,70 @@ compute_partial_view(const struct drm_i915_gem_object *obj,
 	return view;
 }
 
+static vm_fault_t i915_error_to_vmf_fault(int err)
+{
+	switch (err) {
+	default:
+		WARN_ONCE(err, "unhandled error in %s: %i\n", __func__, err);
+		/* fallthrough */
+	case -EIO: /* shmemfs failure from swap device */
+	case -EFAULT: /* purged object */
+	case -ENODEV: /* bad object, how did you get here! */
+		return VM_FAULT_SIGBUS;
+
+	case -ENOSPC: /* shmemfs allocation failure */
+	case -ENOMEM: /* our allocation failure */
+		return VM_FAULT_OOM;
+
+	case 0:
+	case -EAGAIN:
+	case -ERESTARTSYS:
+	case -EINTR:
+	case -EBUSY:
+		/*
+		 * EBUSY is ok: this just means that another thread
+		 * already did the job.
+		 */
+		return VM_FAULT_NOPAGE;
+	}
+}
+
+static vm_fault_t i915_gem_fault_cpu(struct vm_fault *vmf)
+{
+	struct vm_area_struct *area = vmf->vma;
+	struct i915_mmap_offset *priv = area->vm_private_data;
+	struct drm_i915_gem_object *obj = priv->obj;
+	vm_fault_t vmf_ret;
+	unsigned long i, size = area->vm_end - area->vm_start;
+	bool write = area->vm_flags & VM_WRITE;
+	int ret;
+
+	if (!i915_gem_object_has_struct_page(obj))
+		return VM_FAULT_SIGBUS;
+
+	/* Sanity check that we allow writing into this object */
+	if (i915_gem_object_is_readonly(obj) && write)
+		return VM_FAULT_SIGBUS;
+
+	ret = i915_gem_object_pin_pages(obj);
+	if (ret)
+		return i915_error_to_vmf_fault(ret);
+
+	for (i = 0; i < size >> PAGE_SHIFT; i++) {
+		struct page *page = i915_gem_object_get_page(obj, i);
+
+		vmf_ret = vmf_insert_pfn(area,
+					 (unsigned long)area->vm_start + i * PAGE_SIZE,
+					 page_to_pfn(page));
+		if (vmf_ret != VM_FAULT_NOPAGE)
+			break;
+	}
+
+	i915_gem_object_unpin_pages(obj);
+
+	return vmf_ret;
+}
+
 /**
  * i915_gem_fault - fault a page into the GTT
  * @vmf: fault info
@@ -339,30 +404,7 @@ vm_fault_t i915_gem_fault(struct vm_fault *vmf)
 	intel_runtime_pm_put(rpm, wakeref);
 	i915_gem_object_unpin_pages(obj);
 err:
-	switch (ret) {
-	default:
-		WARN_ONCE(ret, "unhandled error in %s: %i\n", __func__, ret);
-		/* fallthrough */
-	case -EIO: /* shmemfs failure from swap device */
-	case -EFAULT: /* purged object */
-	case -ENODEV: /* bad object, how did you get here! */
-		return VM_FAULT_SIGBUS;
-
-	case -ENOSPC: /* shmemfs allocation failure */
-	case -ENOMEM: /* our allocation failure */
-		return VM_FAULT_OOM;
-
-	case 0:
-	case -EAGAIN:
-	case -ERESTARTSYS:
-	case -EINTR:
-	case -EBUSY:
-		/*
-		 * EBUSY is ok: this just means that another thread
-		 * already did the job.
-		 */
-		return VM_FAULT_NOPAGE;
-	}
+	return i915_error_to_vmf_fault(ret);
 }
 
 void __i915_gem_object_release_mmap_gtt(struct drm_i915_gem_object *obj)
@@ -651,6 +693,33 @@ static const struct vm_operations_struct i915_gem_gtt_vm_ops = {
 	.close = i915_gem_vm_close,
 };
 
+static const struct vm_operations_struct i915_gem_cpu_vm_ops = {
+	.fault = i915_gem_fault_cpu,
+	.open = i915_gem_vm_open,
+	.close = i915_gem_vm_close,
+};
+
+static void set_vmdata_mmap_offset(struct i915_mmap_offset *mmo, struct vm_area_struct *vma)
+{
+	switch (mmo->mmap_type) {
+	case I915_MMAP_TYPE_WC:
+		vma->vm_page_prot =
+			pgprot_writecombine(vm_get_page_prot(vma->vm_flags));
+		break;
+	case I915_MMAP_TYPE_WB:
+		vma->vm_page_prot = vm_get_page_prot(vma->vm_flags);
+		break;
+	case I915_MMAP_TYPE_UC:
+		vma->vm_page_prot =
+			pgprot_noncached(vm_get_page_prot(vma->vm_flags));
+		break;
+	default:
+		break;
+	}
+
+	vma->vm_ops = &i915_gem_cpu_vm_ops;
+}
+
 /* This overcomes the limitation in drm_gem_mmap's assignment of a
  * drm_gem_object as the vma->vm_private_data. Since we need to
  * be able to resolve multiple mmap offsets which could be tied
@@ -718,7 +787,16 @@ int i915_gem_mmap(struct file *filp, struct vm_area_struct *vma)
 	vma->vm_page_prot = pgprot_decrypted(vma->vm_page_prot);
 	vma->vm_private_data = mmo;
 
-	vma->vm_ops = &i915_gem_gtt_vm_ops;
+	switch (mmo->mmap_type) {
+	case I915_MMAP_TYPE_WC:
+	case I915_MMAP_TYPE_WB:
+	case I915_MMAP_TYPE_UC:
+		set_vmdata_mmap_offset(mmo, vma);
+		break;
+	case I915_MMAP_TYPE_GTT:
+		vma->vm_ops = &i915_gem_gtt_vm_ops;
+		break;
+	}
 
 	return 0;
 }
diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c b/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c
index 7761d2e0971d..63bc8864a38e 100644
--- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c
+++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c
@@ -731,11 +731,13 @@ static int igt_mmap_offset_exhaustion(void *arg)
 }
 
 #define expand32(x) (((x) << 0) | ((x) << 8) | ((x) << 16) | ((x) << 24))
-static int igt_mmap_gtt(void *arg)
+static int igt_mmap(void *arg, enum i915_mmap_type type)
 {
 	struct drm_i915_private *i915 = arg;
 	struct drm_i915_gem_object *obj;
 	struct vm_area_struct *area;
+	/* Ownership transferred to parent gem object in create_mmap_offset */
+	struct i915_mmap_offset *mmo = kzalloc(sizeof(*mmo), GFP_KERNEL);
 	unsigned long addr;
 	void *vaddr;
 	int err, i;
@@ -756,11 +758,12 @@ static int igt_mmap_gtt(void *arg)
 	i915_gem_object_flush_map(obj);
 	i915_gem_object_unpin_map(obj);
 
-	err = create_mmap_offset(obj);
+	err = create_mmap_offset(obj, mmo);
 	if (err)
 		goto out;
 
-	addr = igt_mmap_node(i915, &obj->base.vma_node,
+	mmo->mmap_type = type;
+	addr = igt_mmap_node(i915, &mmo->vma_node,
 			     0, PROT_WRITE, MAP_SHARED);
 	if (IS_ERR_VALUE(addr)) {
 		err = addr;
@@ -776,8 +779,8 @@ static int igt_mmap_gtt(void *arg)
 		goto out_unmap;
 	}
 
-	if (area->vm_private_data != obj) {
-		pr_err("vm_area_struct did not point back to our object!\n");
+	if (area->vm_private_data != mmo) {
+		pr_err("vm_area_struct did not point back to our mmap_offset object!\n");
 		err = -EINVAL;
 		goto out_unmap;
 	}
@@ -828,6 +831,16 @@ static int igt_mmap_gtt(void *arg)
 	return err;
 }
 
+static int igt_mmap_gtt(void *arg)
+{
+	return igt_mmap(arg, I915_MMAP_TYPE_GTT);
+}
+
+static int igt_mmap_cpu(void *arg)
+{
+	return igt_mmap(arg, I915_MMAP_TYPE_WC);
+}
+
 static int check_present_pte(pte_t *pte, unsigned long addr, void *data)
 {
 	if (!pte_present(*pte) || pte_none(*pte)) {
@@ -880,10 +893,12 @@ static int prefault_range(u64 start, u64 len)
 	return __get_user(c, end - 1);
 }
 
-static int igt_mmap_gtt_revoke(void *arg)
+static int igt_mmap_revoke(void *arg, enum i915_mmap_type type)
 {
 	struct drm_i915_private *i915 = arg;
 	struct drm_i915_gem_object *obj;
+	/* Ownership transferred to parent gem object in create_mmap_offset */
+	struct i915_mmap_offset *mmo = kzalloc(sizeof(*mmo), GFP_KERNEL);
 	unsigned long addr;
 	int err;
 
@@ -894,11 +909,12 @@ static int igt_mmap_gtt_revoke(void *arg)
 	if (IS_ERR(obj))
 		return PTR_ERR(obj);
 
-	err = create_mmap_offset(obj);
+	err = create_mmap_offset(obj, mmo);
 	if (err)
 		goto out;
 
-	addr = igt_mmap_node(i915, &obj->base.vma_node,
+	mmo->mmap_type = type;
+	addr = igt_mmap_node(i915, &mmo->vma_node,
 			     0, PROT_WRITE, MAP_SHARED);
 	if (IS_ERR_VALUE(addr)) {
 		err = addr;
@@ -909,7 +925,8 @@ static int igt_mmap_gtt_revoke(void *arg)
 	if (err)
 		goto out_unmap;
 
-	GEM_BUG_ON(!atomic_read(&obj->bind_count));
+	GEM_BUG_ON(mmo->mmap_type == I915_MMAP_TYPE_GTT &&
+		   !atomic_read(&obj->bind_count));
 
 	err = check_present(addr, obj->base.size);
 	if (err)
@@ -938,6 +955,16 @@ static int igt_mmap_gtt_revoke(void *arg)
 	return err;
 }
 
+static int igt_mmap_gtt_revoke(void *arg)
+{
+	return igt_mmap_revoke(arg, I915_MMAP_TYPE_GTT);
+}
+
+static int igt_mmap_cpu_revoke(void *arg)
+{
+	return igt_mmap_revoke(arg, I915_MMAP_TYPE_WC);
+}
+
 int i915_gem_mman_live_selftests(struct drm_i915_private *i915)
 {
 	static const struct i915_subtest tests[] = {
@@ -945,7 +972,9 @@ int i915_gem_mman_live_selftests(struct drm_i915_private *i915)
 		SUBTEST(igt_smoke_tiling),
 		SUBTEST(igt_mmap_offset_exhaustion),
 		SUBTEST(igt_mmap_gtt),
+		SUBTEST(igt_mmap_cpu),
 		SUBTEST(igt_mmap_gtt_revoke),
+		SUBTEST(igt_mmap_cpu_revoke),
 	};
 
 	return i915_subtests(tests, i915);
-- 
2.23.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 23+ messages in thread

end of thread, other threads:[~2019-11-28 11:23 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-19 11:37 [PATCH 1/4] drm/i915: Allow i915 to manage the vma offset nodes instead of drm core Abdiel Janulgue
2019-11-19 11:37 ` [Intel-gfx] " Abdiel Janulgue
2019-11-19 11:37 ` [PATCH 2/4] drm/i915: Introduce DRM_I915_GEM_MMAP_OFFSET Abdiel Janulgue
2019-11-19 11:37   ` [Intel-gfx] " Abdiel Janulgue
2019-11-19 11:40   ` Abdiel Janulgue
2019-11-19 11:40     ` [Intel-gfx] " Abdiel Janulgue
2019-11-22 18:49   ` Ville Syrjälä
2019-11-22 18:49     ` [Intel-gfx] " Ville Syrjälä
2019-11-19 11:37 ` [PATCH 3/4] drm/i915: cpu-map based dumb buffers Abdiel Janulgue
2019-11-19 11:37   ` [Intel-gfx] " Abdiel Janulgue
2019-11-19 11:37 ` [PATCH 4/4] drm/i915: Add cpu fault handler for mmap_offset Abdiel Janulgue
2019-11-19 11:37   ` [Intel-gfx] " Abdiel Janulgue
2019-11-28 11:08   ` Chris Wilson
2019-11-28 11:08     ` [Intel-gfx] " Chris Wilson
2019-11-28 11:22     ` Chris Wilson
2019-11-28 11:22       ` [Intel-gfx] " Chris Wilson
2019-11-19 12:46 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/4] drm/i915: Allow i915 to manage the vma offset nodes instead of drm core Patchwork
2019-11-19 12:46   ` [Intel-gfx] " Patchwork
2019-11-19 13:25 ` ✗ Fi.CI.BAT: failure " Patchwork
2019-11-19 13:25   ` [Intel-gfx] " Patchwork
  -- strict thread matches above, loose matches on Subject: below --
2019-11-15 11:45 [PATCH 1/4] " Abdiel Janulgue
2019-11-15 11:45 ` [PATCH 4/4] drm/i915: Add cpu fault handler for mmap_offset Abdiel Janulgue
2019-11-15 13:58   ` Chris Wilson
2019-11-14 19:09 [PATCH 1/4] drm/i915: Allow i915 to manage the vma offset nodes instead of drm core Abdiel Janulgue
2019-11-14 19:09 ` [PATCH 4/4] drm/i915: Add cpu fault handler for mmap_offset Abdiel Janulgue

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.