linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jinoh Kang <jinoh.kang.kr@gmail.com>
To: "Marek Marczykowski-Górecki" <marmarek@invisiblethingslab.com>
Cc: Jani Nikula <jani.nikula@linux.intel.com>,
	Joonas Lahtinen <joonas.lahtinen@linux.intel.com>,
	Rodrigo Vivi <rodrigo.vivi@intel.com>,
	David Airlie <airlied@linux.ie>, Daniel Vetter <daniel@ffwll.ch>,
	Chris Wilson <chris@chris-wilson.co.uk>,
	Matthew Auld <matthew.auld@intel.com>,
	intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org,
	linux-kernel@vger.kernel.org
Subject: [PATCH] drm/i915/userptr: detect un-GUP-able pages early
Date: Fri, 15 Jan 2021 16:23:31 +0000	[thread overview]
Message-ID: <c742707e-eb6d-6a22-3006-52dc3bf458d8@gmail.com> (raw)

If GUP-ineligible pages are passed to a GEM userptr object, -EFAULT is
returned only when the object is actually bound.

The xf86-video-intel userspace driver cannot differentiate this
condition, and marks the GPU as wedged.  This not only disables graphics
acceleration but may also cripple other functions such as VT switch.

Solve this by "prefaulting" user pages on GEM object creation, testing
whether all pages are eligible for get_user_pages() in the process.
On failure, return -EFAULT so that userspace can fallback to software
blitting.

This behavior can be enabled via a new modparam "gem_userptr_prefault",
which is false by default.

Known use cases:

- As a debugging aid, invalid pointers and/or wrong pages passed to
  userptr could be caught much earlier.
- Qubes OS R4.0 uses VM_PFNMAP pages from drivers/xen/privcmd.c, in
  order to map framebuffers from Xen guest to dom0.  These pages are not
  GUP-able, but they cannot be exposed via DMA-BUF either.  Previously
  this issue had gone somehow undetected, until some patch between
  v4.14 and v4.19 triggered it.

Signed-off-by: Jinoh Kang <jinoh.kang.kr@gmail.com>
Cc: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Cc: Jani Nikula <jani.nikula@linux.intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: David Airlie <airlied@linux.ie>
Cc: Daniel Vetter <daniel@ffwll.ch>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Matthew Auld <matthew.auld@intel.com>
---
 drivers/gpu/drm/i915/gem/i915_gem_userptr.c | 35 +++++++++++++++++++++
 drivers/gpu/drm/i915/i915_params.c          |  3 ++
 drivers/gpu/drm/i915/i915_params.h          |  1 +
 3 files changed, 39 insertions(+)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_userptr.c b/drivers/gpu/drm/i915/gem/i915_gem_userptr.c
index f2eaed6aca3d..5d653df2f759 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_userptr.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_userptr.c
@@ -712,6 +712,33 @@ static const struct drm_i915_gem_object_ops i915_gem_userptr_ops = {
 	.release = i915_gem_userptr_release,
 };
 
+static int i915_gem_userptr_prefault(unsigned long start,
+				     unsigned long nr_pages,
+				     bool readonly)
+{
+	unsigned int gup_flags = (readonly ? 0 : FOLL_WRITE) | FOLL_NOWAIT;
+	int err = 0;
+
+	mmap_read_lock(current->mm);
+	while (nr_pages) {
+		long ret;
+
+		ret = get_user_pages(start, nr_pages, gup_flags, NULL, NULL);
+		if (ret < 0) {
+			err = (int)ret;
+			break;
+		}
+		if (ret == 0)
+			ret = 1;  /* skip this page */
+
+		start += ret << PAGE_SHIFT;
+		nr_pages -= ret;
+	}
+	mmap_read_unlock(current->mm);
+
+	return err;
+}
+
 /*
  * Creates a new mm object that wraps some normal memory from the process
  * context - user memory.
@@ -796,6 +823,14 @@ i915_gem_userptr_ioctl(struct drm_device *dev,
 	if (!access_ok((char __user *)(unsigned long)args->user_ptr, args->user_size))
 		return -EFAULT;
 
+	if (i915_modparams.gem_userptr_prefault) {
+		ret = i915_gem_userptr_prefault((unsigned long)args->user_ptr,
+						args->user_size >> PAGE_SHIFT,
+						args->flags & I915_USERPTR_READ_ONLY);
+		if (ret)
+			return ret;
+	}
+
 	if (args->flags & I915_USERPTR_READ_ONLY) {
 		/*
 		 * On almost all of the older hw, we cannot tell the GPU that
diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c
index 7f139ea4a90b..b5e0a88c059f 100644
--- a/drivers/gpu/drm/i915/i915_params.c
+++ b/drivers/gpu/drm/i915/i915_params.c
@@ -197,6 +197,9 @@ i915_param_named_unsafe(fake_lmem_start, ulong, 0400,
 	"Fake LMEM start offset (default: 0)");
 #endif
 
+i915_param_named(gem_userptr_prefault, bool, 0600,
+	"Prefault pages when userptr GEM object is created (default: false)");
+
 static __always_inline void _print_param(struct drm_printer *p,
 					 const char *name,
 					 const char *type,
diff --git a/drivers/gpu/drm/i915/i915_params.h b/drivers/gpu/drm/i915/i915_params.h
index 330c03e2b4f7..323f60298b05 100644
--- a/drivers/gpu/drm/i915/i915_params.h
+++ b/drivers/gpu/drm/i915/i915_params.h
@@ -79,6 +79,7 @@ struct drm_printer;
 	param(bool, disable_display, false, 0400) \
 	param(bool, verbose_state_checks, true, 0) \
 	param(bool, nuclear_pageflip, false, 0400) \
+	param(bool, gem_userptr_prefault, false, 0600) \
 	param(bool, enable_dp_mst, true, 0600) \
 	param(bool, enable_gvt, false, 0400)
 
-- 
2.26.2


             reply	other threads:[~2021-01-15 16:24 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-01-15 16:23 Jinoh Kang [this message]
     [not found] ` <161072980241.18103.11713889922046524226@build.alporthouse.com>
2021-01-15 17:03   ` [PATCH] drm/i915/userptr: detect un-GUP-able pages early Jinoh Kang
     [not found]   ` <161073047349.18103.16410031184146072179@build.alporthouse.com>
2021-01-16  5:11     ` Jinoh Kang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=c742707e-eb6d-6a22-3006-52dc3bf458d8@gmail.com \
    --to=jinoh.kang.kr@gmail.com \
    --cc=airlied@linux.ie \
    --cc=chris@chris-wilson.co.uk \
    --cc=daniel@ffwll.ch \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=jani.nikula@linux.intel.com \
    --cc=joonas.lahtinen@linux.intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=marmarek@invisiblethingslab.com \
    --cc=matthew.auld@intel.com \
    --cc=rodrigo.vivi@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).