[PATCH v2 0/3] drm/nouveau: nv04 FIFO cleanup + recovery for Tesla

[Marek Czernohous]

From: Marek Czernohous <marek@xxxxxxxxxxxxx>

v1 of this series is here:

  https://lore.kernel.org/nouveau/20260513175014.96599-1-marek@xxxxxxxxxxxxx/

Please do not apply v1. While running it on the reference machine I found
three defects in my own patch, one of which is a guaranteed kernel panic on
exactly the hardware the series targets. v2 fixes all three and adds one new
patch as a prerequisite.

What went wrong in v1, and how v2 addresses it:

1) NULL pointer dereference in interrupt context (fixed in 3/3)

   v1 called nvkm_chan_error(chan, true). nv50 and g84 channels have no
   .preempt callback, and the call in nvkm_chan_error() is guarded only by
   the preempt argument and not by a NULL check, so preempt=true dereferences
   a NULL function pointer under chan->lock in interrupt context.

   This is not theoretical. On 2026-06-02 it fired on the reference machine
   (then running 7.0.10-p1) during a deliberate VRAM stress test, captured
   over netconsole:

     fifo: CACHE_ERROR - ch 2 [labwc[3950]] subc 3 mthd 0f00 data 0000007b
     nv04_fifo_intr_cache_error+0x111
      -> nv04_fifo_recover+0x80
      -> nvkm_chan_error+0x99
     RIP: 0010:0x0

   The oops escalated to a full panic because nouveau's drm_panic scanout
   path ioremaps in panic context:

     nv50_wndw_get_scanout_buffer -> nouveau_bo_map -> ttm_bo_kmap
      -> __ioremap_caller -> __get_vm_area_node
     kernel BUG at mm/vmalloc.c:3212

   That is a separate, pre-existing problem which I have not yet reported;
   I will do so on its own. The hardware watchdog then reset the machine.
   v2 uses preempt=false, which is what the existing caller in
   nvkm_runl_rc() does as well.

2) Killing the channel on the first fault is wrong (fixed in 3/3)

   v1 killed the channel on every fault. But there is a single PFIFO cache
   puller, and on a fault it names the channel that is *resident*, not
   necessarily the one that caused the fault; nv04_fifo_pause() documents
   this ("incorrect instance offsets to PGRAPH"). On the reference machine
   this killed the Wayland compositor twice for somebody else's fault, on
   2026-06-02 and again on 2026-07-22.

   v2 keeps mainline behaviour for the first faults (skip the method or drop
   the push segment and resume) and only escalates to the kill once the same
   channel object has faulted NVKM_FIFO_KILL_COUNT times inside
   NVKM_FIFO_KILL_WINDOW_MS. The streak is keyed on the channel object
   pointer as an identity token, never dereferenced, and dropped in
   nvkm_chan_del() so a reused channel id cannot inherit it.

3) A killed channel is not survivable on Tesla (new patch 2/3)

   This is the reason for the new patch, and it is the part I would most
   like reviewed.

   nouveau_channel_init() only subscribes to the channel-killed event for
   FERMI_CHANNEL_GPFIFO and newer. On Tesla the ERRORED event is therefore
   delivered into an empty notifier list, nouveau_fence_context_kill() never
   runs, and the pending fences of the killed channel are never signalled.

   Today that is harmless upstream, because nothing kills a Tesla channel.
   Patch 3/3 introduces exactly such a caller, so without 2/3 the series
   would trade a recoverable fault for an unrecoverable hang. On the
   reference machine this presented twice as a frozen desktop on a machine
   that was otherwise alive and reachable over ssh. The stacks as observed:

     kworker/u8 events_unbound: dma_fence_default_wait
       <- drm_atomic_helper_wait_for_fences <- nv50_disp_atomic_commit_tail
     12x kworker/uN ttm: dma_fence_default_wait
       <- dma_resv_wait_timeout <- ttm_bo_fini

   The ttm frame is reported as ttm_bo_fini; the wait that actually holds
   those workers is the MAX_SCHEDULE_TIMEOUT one in ttm_bo_delayed_delete,
   which is static and in the same file, so the shortened frame is
   consistent with inlining. Either way both waits are uninterruptible.
   The desktop stayed frozen for minutes until the fences timed out, after
   which a compositor respawn brought the display back; a reboot cleared it
   immediately. Leaving a fence unsignalled also violates the dma-fence
   contract, which is why I think the subscription belongs on NV50+ even
   independently of this series.

Testing

  Reference hardware: Apple Mac mini Late 2009, MCP79 / GeForce 9400M
  (NVAC), Core 2 Duo, Wayland (labwc) on Mesa 26.0.8.

  Fault injection. I restored the debugfs-style fault injector I used for the
  original validation (as a pair of write-only module parameters this time)
  and drove nv04_fifo_recover() directly on a throwaway GL process, so that
  the compositor was never the target:

    ch 5 fault 1/3 in 10000ms window, skipping method and resuming (Tier-0)
    ch 5 fault 2/3 in 10000ms window, skipping method and resuming (Tier-0)
    fifo:000000:0005:0005:[fbo-stress[13247]] errored - disabling channel
    Xwayland[4931]: channel 5 killed!

  (The last line is prefixed with the DRM client rather than the faulting
  process because the test program is an X11 GL client; NV_PRINTK names the
  client that owns the channel.)

  That covers all three defects:

  - The channel survives the first two faults and only the third one inside
    the window escalates, so an unrelated process is no longer killed for a
    fault the puller misattributed.
  - A fault 18 seconds after the previous one counted as 1/3 again, so the
    window expiry works.
  - The "channel N killed!" line is the one that never appeared before patch
    2/3. It proves the event is delivered and the fences of the killed
    channel are signalled: no kworker ended up in dma_fence_default_wait,
    no TTM worker was left in D state (there were twelve during the freeze),
    the compositor kept running, and the display commit never had to fall
    back to any timeout.
  - The victim process died with a SIGSEGV inside libgallium. That is the
    known Mesa nv50 limitation (no GL robustness on this generation), not a
    consequence of these patches.

  What the injection does not cover, to be explicit: it enters the recovery
  path from process context, where the real fault arrives in hard interrupt
  context. The locking is the same, but the context is not, so this is
  functional coverage of the recovery path rather than of the interrupt path.

  Soak. The v1 patches ran on that machine from 2026-05 onwards, which is how
  defects 1 and 2 were found. The v2 code, including the escalation ladder and
  the widened event subscription, has been running since 2026-07-25, twelve
  days as of this posting, across a kernel bump from 7.1.5 to 7.1.6 with the patch
  series unchanged. Since every channel now subscribes to the kill event, the
  interesting long-run question is channel churn, so the soak is ordinary
  daily driving with a browser and GL applications rather than a synthetic
  loop.

  One difference between the soaked code and the diff below, stated so it is
  not a surprise: downstream I carry the widened subscription behind a
  module parameter (default on) so I can fall back to the Fermi+ behaviour
  without rebuilding. Patch 2/3 as posted does it unconditionally. At the
  default the two are behaviourally identical, and that default is what ran
  for those twelve days; the parameter itself is a local debugging aid and I did
  not think it belonged upstream.

  Compile coverage, precisely: the changed translation units build clean in a
  downstream 7.1.6 tree that carries equivalent code. The series is rebased
  onto the mainline base commit below, but I have not built it there, because
  my mainline checkout is sparse. The full module, in the downstream form, is
  built and booted on the reference machine.

  A note on MAINTAINERS. Patch 3/3 adds a new file
  drivers/gpu/drm/nouveau/nvkm/engine/fifo/recover.c. The change is covered
  by the existing nouveau MAINTAINERS section (drivers/gpu/drm/nouveau/),
  so no MAINTAINERS update is included. checkpatch.pl flags this as a hint;
  it is not load-bearing.

AI assistance

  Lyude asked on the v1 thread whether these patches were written by a human
  and pointed at Documentation/process/coding-assistants.rst. The answer,
  repeated here for the archive: this work is AI assisted. I use Claude
  (claude-opus-4-7 for v1, claude-opus-5 for v2) as a coding and analysis
  assistant. Every patch carries an Assisted-by trailer accordingly, and no
  Signed-off-by is added by the tool.

  Nature of the assistance, so you can calibrate your review: the assistant
  did most of the code archaeology and drafting. I described symptoms
  ("the desktop freezes but the machine is alive", "the compositor gets
  killed for a fault it did not cause"), asked for the mechanism to be
  traced in the source rather than guessed, and asked for each claim to be
  backed by a file and line. The register dumps, kernel stacks and the
  netconsole trace quoted above are measurements from the machine, not
  model output. The assistant also reviewed its own earlier work
  adversarially, which is how defects 2 and 3 were found. I reviewed the
  result, I understand the code, and I take responsibility for it.

  One consequence worth stating: a first draft of this cover letter
  described the fence-context teardown ordering in nouveau_channel_del() as
  a use-after-free worth its own patch. On closer inspection most of that
  window is harmless, because nouveau_fence_context_del() empties the
  pending list before the event is unsubscribed. I dropped that patch rather
  than send an overstated claim. If you would still like the reordering as a
  hardening change, I can send it separately.

Changes since v1

  - new patch 2/3 (subscribe to channel-kill events on NV50+)
  - 3/3: preempt=false instead of preempt=true
  - 3/3: escalation ladder instead of kill on first fault
  - 3/3: per-channel streak dropped in nvkm_chan_del()
  - 3/3: chan_killed tracepoint moved to the actual kill
  - 3/3: comments that had slipped through in German are now in English
  - rebased onto current mainline
  - Assisted-by trailers added, as offered on the v1 thread
  - defects 2 and 3 reproduced and their fixes verified with fault
    injection; defect 1 was reproduced organically before the fix and is
    covered here by the absence of the panic under injection

Marek Czernohous (3):
  drm/nouveau/fifo/nv04: filter benign CACHE_ERROR from Mesa NV50 bind
    probe
  drm/nouveau: subscribe to channel-kill events on NV50 and newer
  drm/nouveau/fifo: add recovery path for Tesla cache_error/dma_pusher

 .../drm/nouveau/include/nvkm/engine/fifo.h    |  31 +++
 .../include/trace/events/nouveau_fifo.h       |  58 ++++++
 drivers/gpu/drm/nouveau/nouveau_chan.c        |   2 +-
 drivers/gpu/drm/nouveau/nouveau_drm.c         |  29 +++
 .../gpu/drm/nouveau/nvkm/engine/fifo/Kbuild   |   1 +
 .../gpu/drm/nouveau/nvkm/engine/fifo/base.c   |   3 +
 .../gpu/drm/nouveau/nvkm/engine/fifo/chan.c   |  14 ++
 .../gpu/drm/nouveau/nvkm/engine/fifo/nv04.c   |  29 ++-
 .../gpu/drm/nouveau/nvkm/engine/fifo/priv.h   |  10 +
 .../drm/nouveau/nvkm/engine/fifo/recover.c    | 176 ++++++++++++++++++
 10 files changed, 346 insertions(+), 7 deletions(-)
 create mode 100644 drivers/gpu/drm/nouveau/include/trace/events/nouveau_fifo.h
 create mode 100644 drivers/gpu/drm/nouveau/nvkm/engine/fifo/recover.c


base-commit: c21bb4193868a8de71fc4693fa741e195fdf5d86
-- 
2.54.0