diff mbox series

[3/5] drm/i915: Add cpu fault handler for mmap_offset

Message ID 20190826122102.32400-3-abdiel.janulgue@linux.intel.com (mailing list archive)
State New, archived
Headers show
Series [1/5] drm/i915: Allow i915 to manage the vma offset nodes instead of drm core | expand

Commit Message

Abdiel Janulgue Aug. 26, 2019, 12:21 p.m. UTC
Fault handler to handle missing pages for shmem-backed objects. Handle also
logic needed to refault pages depending on fault handler usage.

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_domain.c |   2 +-
 drivers/gpu/drm/i915/gem/i915_gem_mman.c   | 189 ++++++++++++++++-----
 drivers/gpu/drm/i915/gem/i915_gem_object.h |   2 +-
 drivers/gpu/drm/i915/i915_gem.c            |   2 +-
 4 files changed, 149 insertions(+), 46 deletions(-)

Comments

Chris Wilson Aug. 26, 2019, 12:44 p.m. UTC | #1
Quoting Abdiel Janulgue (2019-08-26 13:21:00)
> Fault handler to handle missing pages for shmem-backed objects. Handle also
> logic needed to refault pages depending on fault handler usage.
> 
> 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_domain.c |   2 +-
>  drivers/gpu/drm/i915/gem/i915_gem_mman.c   | 189 ++++++++++++++++-----
>  drivers/gpu/drm/i915/gem/i915_gem_object.h |   2 +-
>  drivers/gpu/drm/i915/i915_gem.c            |   2 +-
>  4 files changed, 149 insertions(+), 46 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_domain.c b/drivers/gpu/drm/i915/gem/i915_gem_domain.c
> index 9c58e8fac1d9..517e73c3aa31 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_domain.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_domain.c
> @@ -254,7 +254,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 87690aa64814..d0a653a9a4cc 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"
> @@ -199,6 +200,45 @@ compute_partial_view(const struct drm_i915_gem_object *obj,
>         return view;
>  }
>  
> +static vm_fault_t i915_error_to_vmf_fault(int err, struct intel_gt *gt)
> +{
> +       switch (err) {
> +       case -EIO:
> +               /*
> +                * We eat errors when the gpu is terminally wedged to avoid
> +                * userspace unduly crashing (gl has no provisions for mmaps to
> +                * fail). But any other -EIO isn't ours (e.g. swap in failure)
> +                * and so needs to be reported.
> +                */
> +               if (!intel_gt_is_wedged(gt))
> +                       return VM_FAULT_SIGBUS;
> +               /* else, fall through */
> +       case -EAGAIN:
> +               /*
> +                * EAGAIN means the gpu is hung and we'll wait for the error
> +                * handler to reset everything when re-faulting in
> +                * i915_mutex_lock_interruptible.
> +                */

These errors don't exist anymore and should not be cargo-culted onto
paths that could not possibly generate them.
-Chris
Chris Wilson Aug. 26, 2019, 12:46 p.m. UTC | #2
Quoting Abdiel Janulgue (2019-08-26 13:21:00)
> +       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_ERROR) {
> +                       vmf_ret = i915_error_to_vmf_fault(vm_fault_to_errno(vmf_ret, 0),
> +                                                         &dev_priv->gt);

Sigh. You take a mm/, run it through the internal error convertor that
*only* applies to i915 errors, and then return the original fault code.
-Chris
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_domain.c b/drivers/gpu/drm/i915/gem/i915_gem_domain.c
index 9c58e8fac1d9..517e73c3aa31 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_domain.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_domain.c
@@ -254,7 +254,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 87690aa64814..d0a653a9a4cc 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"
@@ -199,6 +200,45 @@  compute_partial_view(const struct drm_i915_gem_object *obj,
 	return view;
 }
 
+static vm_fault_t i915_error_to_vmf_fault(int err, struct intel_gt *gt)
+{
+	switch (err) {
+	case -EIO:
+		/*
+		 * We eat errors when the gpu is terminally wedged to avoid
+		 * userspace unduly crashing (gl has no provisions for mmaps to
+		 * fail). But any other -EIO isn't ours (e.g. swap in failure)
+		 * and so needs to be reported.
+		 */
+		if (!intel_gt_is_wedged(gt))
+			return VM_FAULT_SIGBUS;
+		/* else, fall through */
+	case -EAGAIN:
+		/*
+		 * EAGAIN means the gpu is hung and we'll wait for the error
+		 * handler to reset everything when re-faulting in
+		 * i915_mutex_lock_interruptible.
+		 */
+	case 0:
+	case -ERESTARTSYS:
+	case -EINTR:
+	case -EBUSY:
+		/*
+		 * EBUSY is ok: this just means that another thread
+		 * already did the job.
+		 */
+		return VM_FAULT_NOPAGE;
+	case -ENOMEM:
+		return VM_FAULT_OOM;
+	case -ENOSPC:
+	case -EFAULT:
+		return VM_FAULT_SIGBUS;
+	default:
+		WARN_ONCE(err, "unhandled error in %s: %i\n", __func__, err);
+		return VM_FAULT_SIGBUS;
+	}
+}
+
 /**
  * i915_gem_fault - fault a page into the GTT
  * @vmf: fault info
@@ -336,44 +376,48 @@  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) {
-	case -EIO:
-		/*
-		 * We eat errors when the gpu is terminally wedged to avoid
-		 * userspace unduly crashing (gl has no provisions for mmaps to
-		 * fail). But any other -EIO isn't ours (e.g. swap in failure)
-		 * and so needs to be reported.
-		 */
-		if (!intel_gt_is_wedged(ggtt->vm.gt))
-			return VM_FAULT_SIGBUS;
-		/* else, fall through */
-	case -EAGAIN:
-		/*
-		 * EAGAIN means the gpu is hung and we'll wait for the error
-		 * handler to reset everything when re-faulting in
-		 * i915_mutex_lock_interruptible.
-		 */
-	case 0:
-	case -ERESTARTSYS:
-	case -EINTR:
-	case -EBUSY:
-		/*
-		 * EBUSY is ok: this just means that another thread
-		 * already did the job.
-		 */
-		return VM_FAULT_NOPAGE;
-	case -ENOMEM:
-		return VM_FAULT_OOM;
-	case -ENOSPC:
-	case -EFAULT:
-		return VM_FAULT_SIGBUS;
-	default:
-		WARN_ONCE(ret, "unhandled error in %s: %i\n", __func__, ret);
+	return i915_error_to_vmf_fault(ret, ggtt->vm.gt);
+}
+
+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;
+	struct drm_device *dev = obj->base.dev;
+	struct drm_i915_private *dev_priv = to_i915(dev);
+	vm_fault_t vmf_ret;
+	unsigned long size = area->vm_end - area->vm_start;
+	bool write = area->vm_flags & VM_WRITE;
+	int i, ret;
+
+	/* 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, &dev_priv->gt);
+
+	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_ERROR) {
+			vmf_ret = i915_error_to_vmf_fault(vm_fault_to_errno(vmf_ret, 0),
+							  &dev_priv->gt);
+			break;
+		}
 	}
+
+	i915_gem_object_unpin_pages(obj);
+
+	return vmf_ret;
 }
 
-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;
 	struct i915_mmap_offset *mmo;
@@ -396,12 +440,6 @@  void __i915_gem_object_release_mmap(struct drm_i915_gem_object *obj)
 }
 
 /**
- * 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
@@ -409,7 +447,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;
@@ -428,7 +466,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
@@ -444,6 +482,35 @@  void i915_gem_object_release_mmap(struct drm_i915_gem_object *obj)
 	intel_runtime_pm_put(&i915->runtime_pm, wakeref);
 }
 
+static 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)
 {
@@ -627,6 +694,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_OFFSET_WC:
+		vma->vm_page_prot =
+			pgprot_writecombine(vm_get_page_prot(vma->vm_flags));
+		break;
+	case I915_MMAP_TYPE_OFFSET_WB:
+		vma->vm_page_prot = vm_get_page_prot(vma->vm_flags);
+		break;
+	case I915_MMAP_TYPE_OFFSET_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
@@ -683,7 +777,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_OFFSET_WC:
+	case I915_MMAP_TYPE_OFFSET_WB:
+	case I915_MMAP_TYPE_OFFSET_UC:
+		set_vmdata_mmap_offset(mmo, vma);
+		break;
+	case I915_MMAP_TYPE_GTT:
+		vma->vm_ops = &i915_gem_gtt_vm_ops;
+		break;
+	}
 
 	/*
 	 * Take a ref for our mmap_offset object. The reference is cleaned
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.h b/drivers/gpu/drm/i915/gem/i915_gem_object.h
index d667ed8bb0a6..012e5d1d99cc 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_object.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_object.h
@@ -338,7 +338,7 @@  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_gtt(struct drm_i915_gem_object *obj);
 void i915_gem_object_release_mmap(struct drm_i915_gem_object *obj);
 
 void
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 95e7c52cf8ed..363b35ba559d 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -857,7 +857,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