From 8cf97637ff8891be040bac37b96dd97e5996ca93 Mon Sep 17 00:00:00 2001

From: Daniel Vetter <daniel.vetter@ffwll.ch>

Date: Thu, 2 Sep 2021 16:20:49 +0200

Subject: drm/i915: Keep gem ctx->vm alive until the final put

MIME-Version: 1.0

Content-Type: text/plain; charset=UTF-8

Content-Transfer-Encoding: 8bit

Git-commit: 8cf97637ff8891be040bac37b96dd97e5996ca93

Patch-mainline: v5.16-rc1

References: bsc#1152489

The comment added in

    commit b81dde719439c8f09bb61e742ed95bfc4b33946b

    Author: Chris Wilson <chris@chris-wilson.co.uk>

__    Date:   Tue May 21 22:11:29 2019 +0100

        drm/i915: Allow userspace to clone contexts on creation

and moved in

    commit 27dbae8f36c1c25008b7885fc07c57054b7dfba3

    Author: Chris Wilson <chris@chris-wilson.co.uk>

__    Date:   Wed Nov 6 09:13:12 2019 +0000

        drm/i915/gem: Safely acquire the ctx->vm when copying

suggested that i915_address_space were at least intended to be managed

through SLAB_TYPESAFE_BY_RCU:

                * This ppgtt may have be reallocated between

                * the read and the kref, and reassigned to a third

                * context. In order to avoid inadvertent sharing

                * of this ppgtt with that third context (and not

                * src), we have to confirm that we have the same

                * ppgtt after passing through the strong memory

                * barrier implied by a successful

                * kref_get_unless_zero().

But extensive git history search has not brough any such reuse to

light.

What has come to light though is that ever since

commit 2850748ef8763ab46958e43a4d1c445f29eeb37d

Author: Chris Wilson <chris@chris-wilson.co.uk>

__Date:   Fri Oct 4 14:39:58 2019 +0100

    drm/i915: Pull i915_vma_pin under the vm->mutex

(yes this commit is earlier) the final i915_vma_put call has been

moved from i915_gem_context_free (now called _release) to

context_close, which means it's not actually safe anymore to access

the ctx->vm pointer without lock helds, because it might disappear at

any moment. Note that superficially things all still work, because the

i915_address_space is RCU protected since

    commit b32fa811156328aea5a3c2ff05cc096490382456

    Author: Chris Wilson <chris@chris-wilson.co.uk>

__    Date:   Thu Jun 20 19:37:05 2019 +0100

        drm/i915/gtt: Defer address space cleanup to an RCU worker

except the very clever macro above (which is designed to protected

against object reuse due to SLAB_TYPESAFE_BY_RCU or similar tricks)

results in an endless loop if the refcount of the ctx->vm ever

permanently drops to 0. Which it totally now can.

Fix that by moving the final i915_vm_put to where it should be.

Note that i915_gem_context is rcu protected, but _only_ the final

kfree. This means anyone who chases a pointer to a gem ctx solely

under the protection can pretty only call kref_get_unless_zero(). This

seems to be pretty much the case, aside from a bunch of cases that

consult the scheduling information without any further protection.

Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>

Cc: Jason Ekstrand <jason@jlekstrand.net>

Cc: Chris Wilson <chris@chris-wilson.co.uk>

Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

Cc: Matthew Brost <matthew.brost@intel.com>

Cc: Matthew Auld <matthew.auld@intel.com>

Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>

Cc: "Thomas Hellström" <thomas.hellstrom@intel.com>

Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com>

Cc: Dave Airlie <airlied@redhat.com>

Fixes: 2850748ef876 ("drm/i915: Pull i915_vma_pin under the vm->mutex")

Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>

Link: https://patchwork.freedesktop.org/patch/msgid/20210902142057.929669-3-daniel.vetter@ffwll.ch

Acked-by: Thomas Zimmermann <tzimmermann@suse.de>

---

A message from ksd-import-patch:

    Hello dear friend,

    I'm ksd-import-patch, the tool that imported this patch

    file.

    I had to prefix at least one line in the commit message

    with __ to work around a bug in the commit hook of

    kernel-sources, which thinks the line contains a patch

    tag.

    If you think the patch should have been merged as it is

    in upstream, please send this file via email to

            kernel-cvs@suse.de

    and request the problem to be fixed.

    Yours truely,

            ksd-import-patch

---

 drivers/gpu/drm/i915/gem/i915_gem_context.c |   15 ++++++++++++++-

 1 file changed, 14 insertions(+), 1 deletion(-)

--- a/drivers/gpu/drm/i915/gem/i915_gem_context.c

+++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c

@@ -335,12 +335,18 @@ static struct i915_gem_engines *default_

 static void i915_gem_context_free(struct i915_gem_context *ctx)

 {

+	struct i915_address_space *vm;

+

 	GEM_BUG_ON(!i915_gem_context_is_closed(ctx));

 	spin_lock(&ctx->i915->gem.contexts.lock);

 	list_del(&ctx->link);

 	spin_unlock(&ctx->i915->gem.contexts.lock);

+	vm = i915_gem_context_vm(ctx);

+	if (vm)

+		i915_vm_put(vm);

+

 	mutex_destroy(&ctx->engines_mutex);

 	mutex_destroy(&ctx->lut_mutex);

@@ -607,8 +613,15 @@ static void context_close(struct i915_ge

 	set_closed_name(ctx);

 	vm = i915_gem_context_vm(ctx);

-	if (vm)

+	if (vm) {

+		/* i915_vm_close drops the final reference, which is a bit too

+		 * early and could result in surprises with concurrent

+		 * operations racing with thist ctx close. Keep a full reference

+		 * until the end.

+		 */

+		i915_vm_get(vm);

 		i915_vm_close(vm);

+	}

 	ctx->file_priv = ERR_PTR(-EBADF);