[PATCH v3 2/4] drm/nouveau: subscribe to the channel-kill event after the fence context

From: Marek Czernohous <[email protected]>

nouveau_channel_init() arms the channel-kill subscription early, right
after mapping userd, and only creates the fence context at the very end
of the same function. The handler it installs, nouveau_channel_killed(),
reaches nouveau_fence_context_kill(chan->fence).

The NULL check in nouveau_channel_kill() does not cover the window in
between. The backends publish the pointer before the context is usable:

	fctx = chan->fence = kzalloc_obj(*fctx);
	if (!fctx)
		return -ENOMEM;

	nouveau_fence_context_new(chan, &fctx->base);

and nouveau_fence_context_new() is what runs spin_lock_init(&fctx->lock)
and INIT_LIST_HEAD(&fctx->pending). An event arriving after the
assignment but before that call finds chan->fence non-NULL and unusable:
nouveau_fence_context_kill() takes a lock that was never initialised and
walks a list head whose next pointer is still the NULL left by kzalloc().

Move the subscription behind context_new() so the handler cannot observe
a half-built fence context. The failure path is unchanged in effect: the
caller drops the channel with nouveau_channel_del() either way, which
since the previous patch unsubscribes before freeing the context.

One behavioural change worth naming, and it is not free: a kill
delivered while the channel is still initialising is no longer observed,
because the subscription is not armed yet. That window does not close
here, it moves, and on Fermi and newer it grows by the span between the
old subscription point and context_new(). What changes is what the
window costs. Before, a kill landing in it reached a half-built fence
context; now it is missed, and the channel is left blocked with
chan->killed still 0, so nouveau_channel_idle() and the checks in
nouveau_gem_ioctl_pushbuf() and nouveau_exec_ioctl_exec() keep treating
it as alive. The missed-kill window is not introduced by this patch either:
nvkm_uchan_init() already calls nvkm_chan_allow() and nvkm_chan_insert(),
so the channel is schedulable before nouveau_channel_init() subscribes at
all. Closing it properly means subscribing before the channel becomes
schedulable, which is a bigger change than this fix.

As with the previous patch this is unreachable below Fermi today, and
the last patch in this series lowers the gate to NV50.

Fixes: ea13e5abf807 ("drm/nouveau: signal pending fences when channel has been killed")
Cc: [email protected]
Assisted-by: Claude:claude-opus-5
Signed-off-by: Marek Czernohous <[email protected]>
---
 drivers/gpu/drm/nouveau/nouveau_chan.c | 55 +++++++++++++++-----------
 1 file changed, 33 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_chan.c b/drivers/gpu/drm/nouveau/nouveau_chan.c
index f142f6310596..07b0bd1bc519 100644
--- a/drivers/gpu/drm/nouveau/nouveau_chan.c
+++ b/drivers/gpu/drm/nouveau/nouveau_chan.c
@@ -370,27 +370,6 @@ nouveau_channel_init(struct nouveau_channel *chan, u32 vram, u32 gart)
 	if (ret)
 		return ret;
 
-	if (chan->user.oclass >= FERMI_CHANNEL_GPFIFO) {
-		DEFINE_RAW_FLEX(struct nvif_event_v0, args, data,
-				sizeof(struct nvif_chan_event_v0));
-		struct nvif_chan_event_v0 *host =
-				(struct nvif_chan_event_v0 *)args->data;
-
-		host->version = 0;
-		host->type = NVIF_CHAN_EVENT_V0_KILLED;
-
-		ret = nvif_event_ctor(&chan->user, "abi16ChanKilled", chan->chid,
-				      nouveau_channel_killed, false,
-				      args, __struct_size(args), &chan->kill);
-		if (ret == 0)
-			ret = nvif_event_allow(&chan->kill);
-		if (ret) {
-			NV_ERROR(drm, "Failed to request channel kill "
-				      "notification: %d\n", ret);
-			return ret;
-		}
-	}
-
 	/* allocate dma objects to cover all allowed vram, and gart */
 	if (device->info.family < NV_DEVICE_INFO_V0_FERMI) {
 		if (device->info.family >= NV_DEVICE_INFO_V0_TESLA) {
@@ -494,7 +473,39 @@ nouveau_channel_init(struct nouveau_channel *chan, u32 vram, u32 gart)
 	}
 
 	/* initialise synchronisation */
-	return nouveau_fence(drm)->context_new(chan);
+	ret = nouveau_fence(drm)->context_new(chan);
+	if (ret)
+		return ret;
+
+	/*
+	 * Subscribe to the channel-kill event last.  The handler
+	 * dereferences chan->fence, and the fence context is only complete
+	 * once context_new() has returned: the backends assign chan->fence
+	 * from kzalloc() before nouveau_fence_context_new() initialises the
+	 * lock and the pending list, so an event arriving in between would
+	 * find a non-NULL but unusable context and walk a NULL list head.
+	 */
+	if (chan->user.oclass >= FERMI_CHANNEL_GPFIFO) {
+		DEFINE_RAW_FLEX(struct nvif_event_v0, args, data,
+				sizeof(struct nvif_chan_event_v0));
+		struct nvif_chan_event_v0 *host =
+				(struct nvif_chan_event_v0 *)args->data;
+
+		host->version = 0;
+		host->type = NVIF_CHAN_EVENT_V0_KILLED;
+
+		ret = nvif_event_ctor(&chan->user, "abi16ChanKilled", chan->chid,
+				      nouveau_channel_killed, false,
+				      args, __struct_size(args), &chan->kill);
+		if (ret == 0)
+			ret = nvif_event_allow(&chan->kill);
+		if (ret) {
+			NV_ERROR(drm, "Failed to request channel kill notification: %d\n", ret);
+			return ret;
+		}
+	}
+
+	return 0;
 }
 
 int
-- 
2.54.0