Re: [PATCH v3 4/4] drm/nouveau: subscribe to channel-kill events on NV50 and newer

[lyude@redhat.com]

This one is going to need some input from others I believe, mostly in
that I'd want to really make sure we know what the implications of
enabling an event like this on Tesla are and what kind of testing we
would need to know this isn't going to break things.

If it was a slightly newer generation I wouldn't be as worried. What
kind of testing have you done with this so far in terms of workload in
addition to what you mentioned on the cover letter? Tried running any
games, anything graphics intensive, etc.?

On Thu, 2026-08-13 at 01:13 +0200, Marek Czernohous wrote:
> From: Marek Czernohous <marek@czernohous.de>
> 
> nouveau_channel_init() only subscribes to the channel-killed event
> for
> FERMI_CHANNEL_GPFIFO and newer. On NV50/Tesla the subscription
> therefore
> never happens, and nvkm_chan_error()'s NVKM_CHAN_EVENT_ERRORED is
> delivered into an empty notifier list.
> 
> Today that is harmless, because nothing kills a channel on Tesla: the
> only nvkm_chan_error() callers are the Fermi and newer recovery
> paths.
> So this patch changes no observable behaviour on its own, and that is
> deliberate: it removes a latent trap before anything can fall into
> it.
> I am carrying a Tesla recovery path that does add such a caller and
> will
> send it separately once it is ready. Without a subscriber in place
> the
> consequences there are severe: nouveau_channel_killed() never runs,
> so
> nouveau_fence_context_kill() never runs either, and the pending
> fences
> of the killed channel are never signalled. Everything waiting on them
> waits forever: drm_atomic_helper_wait_for_fences() in the display
> commit
> tail waits uninterruptibly and without a timeout, and the TTM delayed
> delete workers wait in TASK_UNINTERRUPTIBLE. The user sees a frozen
> desktop on a machine that is otherwise alive, and nothing in the
> kernel
> ends that state: both waits pass MAX_SCHEDULE_TIMEOUT, so the fences
> cannot time out. They are signalled only when the fence context is
> torn
> down, that is when the DRM client owning the channel closes its fd
> and
> nouveau_fence_context_del() runs. Killing the client, or rebooting,
> clears it; waiting does not.
> That is also a dma-fence contract violation: a fence must always be
> signalled, with an error if necessary.
> 
> Lower the class gate to NV50_CHANNEL_GPFIFO. The nvkm side is already
> class neutral: the KILLED case hangs the notifier on runl->chid-
> >event,
> which every fifo owns since the runlist rework, and
> nvkm_uchan_uevent()
> does not discriminate by class. Pre-NV50 chips keep the old
> behaviour,
> so NV04 to NV40 are unaffected.
> 
> Assisted-by: Claude:claude-opus-5
> Signed-off-by: Marek Czernohous <marek@czernohous.de>
> ---
>  drivers/gpu/drm/nouveau/nouveau_chan.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/nouveau/nouveau_chan.c
> b/drivers/gpu/drm/nouveau/nouveau_chan.c
> index 07b0bd1bc519..5c2f4b9342b7 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_chan.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_chan.c
> @@ -485,7 +485,7 @@ nouveau_channel_init(struct nouveau_channel
> *chan, u32 vram, u32 gart)
>  	 * 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) {
> +	if (chan->user.oclass >= NV50_CHANNEL_GPFIFO) {
>  		DEFINE_RAW_FLEX(struct nvif_event_v0, args, data,
>  				sizeof(struct nvif_chan_event_v0));
>  		struct nvif_chan_event_v0 *host =