-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256
On Thu, Dec 24, 2020 at 02:24:59AM +0000, Jinoh Kang wrote:
> 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 disables graphics
> acceleration and may cripple other functionalities 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
> bliting.
>
> This behavior can be controlled by using a new modparam
> "gem_userptr_prefault", which is true by default.
>
> The only known use for this patch is Qubes OS's GUI virtualization.
> Userspace is expected to resort to DMA-BUF whenever possible.
>
> Signed-off-by: Jinoh Kang <jinoh.kang...@gmail.com>
Thanks for the path. I've tried this one on 5.10.5, and it needs a
couple fixes to build, see below.
But then, it seems to fix the issue with VT switch and disabling
acceleration.
Independently, starting with 5.9.x (without the patch) the system freezes
with specific monitor configuration (two monitors side by side, but one
scaled x1.5). It happens immediately after configuring the second
display (and I still got the message about it in Xorg.0.log). Not
setting scaling on the monitor workarounds the issue.
Switching to modesetting driver fixes it. This issue wasn't here on
5.8.x or earlier and looks to be unrelated to this patch. I'm not sure
whether it's purely kernel driver issue, or some kernel<->Xorg
incompatibility.
> ---
> drivers/gpu/drm/i915/gem/i915_gem_userptr.c | 36 +++++++++++++++++++++
> drivers/gpu/drm/i915/i915_params.c | 3 ++
> drivers/gpu/drm/i915/i915_params.h | 1 +
> 3 files changed, 40 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..65596d2b284f 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_userptr.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_userptr.c
> @@ -712,6 +712,34 @@ 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)
> +{
> + struct mm_struct *mm = current->mm;
> + unsigned int gup_flags = (readonly ? 0 : FOLL_WRITE) | FOLL_NOWAIT;
> + int err = 0;
> +
> + down_read(&mm->mmap_sem);
mmap_read_lock(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;
> + }
> + up_read(&mm->mmap_sem);
mmap_read_unlock(mm);
> +
> + return err;
> +}
> +
> /*
> * Creates a new mm object that wraps some normal memory from the process
> * context - user memory.
> @@ -796,6 +824,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 (READ_ONCE(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..c39766adeda2 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:true)");
> +
> 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..1169a610a73c 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, true) \
param(bool, gem_userptr_prefault, true, 0600)
> param(bool, enable_dp_mst, true, 0600) \
> param(bool, enable_gvt, false, 0400)
>
- --
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?
-----BEGIN PGP SIGNATURE-----
iQEzBAEBCAAdFiEEhrpukzGPukRmQqkK24/THMrX1ywFAl/7fkwACgkQ24/THMrX
1yyJ+Qf+Kp1NqR/RruBKW3pKbNEZy2Y92viOcKkMcOq96fjLEn/boRevpQHjBpjQ
bdBa5wZasgk0aHV6UTGB1GrLMQupbupMcI2kffmJnvo0/uleRdad1QgPqlcWhO+x
Y0CtPioefWLEwBNITIJGr1emtyM/pO7NpVkFJt3jjei7DfG/jFEkmD36oKb/ea+P
GVmug75CpJKPpmYZT39RzqfoI6ZwCq7Lq70I+/kjQBNiyo2N5/xTKiBkt7NDkQas
lmrtBXAQRn0UGps7SU2tfsdkU/vYJWohNGK7NzLTSN4jyBsH8CBTyJ0x6DxTvYox
W9BPiiLAjBSYJ6jeZ4x8Ly89s0rw8Q==
=EF0b
-----END PGP SIGNATURE-----
--
You received this message because you are subscribed to the Google Groups
"qubes-users" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to qubes-users+unsubscr...@googlegroups.com.
To view this discussion on the web visit
https://groups.google.com/d/msgid/qubes-users/20210110222307.GA1176%40mail-itl.
