From 230461addb38096d6d51422ee9d00a618cadf241 Mon Sep 17 00:00:00 2001 From: technomancer Date: Wed, 30 Sep 2026 08:26:57 -0700 Subject: [PATCH] fix crashes when running fillrate benchmark --- rules/gr2/fin2-wait-must-stall.md | 21 ++++++++++------- rules/gr2/gedma-read-port.md | 38 +++++++++++++++++++++++++++++++ src/dev/gr2/mod.rs | 31 +++++++++++++++++-------- src/gfifo.rs | 8 +++++++ 4 files changed, 80 insertions(+), 18 deletions(-) create mode 100644 rules/gr2/gedma-read-port.md diff --git a/rules/gr2/fin2-wait-must-stall.md b/rules/gr2/fin2-wait-must-stall.md index 0c35b3b..62a3bd4 100644 --- a/rules/gr2/fin2-wait-must-stall.md +++ b/rules/gr2/fin2-wait-must-stall.md @@ -18,11 +18,12 @@ Fix (mod.rs `fin2_wait`): the FIN2 ack arms a wait; while armed, a version read with FIN2 clear returns bus busy as long as the HQ2 has queued or in-progress work, so the kernel's first poll after the DMA sees FIN2. With the HQ2 idle and no FIN2 the read returns at once and the kernel times out, -as on hardware. The stall is capped at 2 s of host time, counted from the -first stalled poll (not from the ack: the DMA in between can take long on a -busy host; counting from the ack failed this test on a GitHub CI runner) -(FIN2_STALL_LIMIT), in case the pipeline can never finish (RE3 held by a -CPU-driven RWDATA readback). Sample "HQ2 working" BEFORE reading the register (the HQ2 +as on hardware. The stall gives up after 2 s of host time WITHOUT +PROGRESS (FIN2_STALL_LIMIT; progress = the HQ2 or RE3 FIFO consumer moved, +`GFifo::consumed`), in case the pipeline can never finish (RE3 held by a +CPU-driven RWDATA readback). Not counted from the ack (the DMA in between +can take long on a busy host; that failed this test on a GitHub CI runner), +and not a fixed cap from the first poll either: see gltest below. Sample "HQ2 working" BEFORE reading the register (the HQ2 raises FIN2 before it consumes the entry and drops hq_busy); sampling after lost the race in about one run in three. Test: `kernel_pixel_dma_fin2_poll_waits_for_hq`. @@ -32,9 +33,13 @@ FIN3 seen too early). The same wait covers the context switch (Gr2PcxSwap: GE_HQMSAV, 0x1E0 / 0x1E6, save / restore; 1,000,000 x us_delay(1)). With our deep FIFO a GL -client can queue enough work that the switch's FIN2 comes after the budget -(suspected cause of Xsgi dying under `gltest --bench 400`, which passes on -hardware at 1000). +client can queue far more work than hardware's 512 words allow: `gltest +--bench N` is N full-screen 800x600 quads (a few FIFO words, 480k pixels +each) then glFinish. N = 500 is ~2.4 s of RE3 work; a context switch behind +it outlived the old fixed 2 s cap, the kernel's FIN2 poll timed out and it +crashed in Gr2PcxSwap's 0x1E5 clip loop (PC 0x882adcac, IRIX 6.5.22 XZ); +N = 300..400 stayed under 2 s and passed. Hardware passes at 1000. Hence the +limit counts from the last progress, not from the first poll. Second cause of the same console message (IRIX 5.3 MRI software): the pixel DMA command itself was unimplemented. lrectwrite goes out as token 0x0B5 diff --git a/rules/gr2/gedma-read-port.md b/rules/gr2/gedma-read-port.md new file mode 100644 index 0000000..d1c7016 --- /dev/null +++ b/rules/gr2/gedma-read-port.md @@ -0,0 +1,38 @@ +# HQ2_GEDMA reads: a ring the HQ2 fills, busy while the HQ2 has work + +The read side of HQ2_GEDMA (0x6a068) is the HQ2's output to the host. The +kernel queues a request and starts the VDMA read at once, with no barrier: + +- 0x1E1 context save (Gr2PcxSwap / _Gr2CXSaveRestore); +- 0x152 pixel DMA read (Xsgi expReadImage*, XGetImage over 1024 pixels); +- 0x0AC pixel DMA read (IRIS GL lrectread, OpenGL glReadPixels KDMA). + +Our HQ2 is a thread behind a deep FIFO, so the first read often beats it. +`Gr2::gedma_out` is a single-producer ring (inline AtomicU32 array, head / +tail counters) the HQ2 thread fills when it executes the request; a read: + +- a word in the ring: return it; +- empty, and the HQ2 had work (FIFO not empty or `hq_busy`) sampled BEFORE + looking at the ring: bus busy (the VDMA worker and the CPU retry); +- empty and the HQ2 idle: overrun, log, return 0. + +Sampling order matters: the HQ2 pushes its words before it drops +`hq_busy`, so an idle HQ2 seen first means everything it produced is +visible. Checking the ring first races (empty, then the HQ2 pushes and goes +idle, then "idle" = false overrun), same as the FIN2 stall +(fin2-wait-must-stall.md). + +Each transfer starts with `gedma_begin`, which drops words a previous one +left unread (they would shift this one). The HQ2 waits while the ring is +full, up to 2 s without a reader, then drops the rest. + +What went wrong without it: +- save: a read that beat 0x1E1 returned the previous image's tail, the + image shifted one word, the restore was rejected and another context's + GL state stayed live (cx-save-must-wait-for-hq.md); +- 0x152 / 0x0AC unhandled: zeros, no FIN2, "Gr2PixelDma: TIMEOUT", the + kernel reset the board (and with a GL client, panicked). + +Tests: `gl_context_save_read_waits_for_hq`, +`ddx_dma_read_pixels_streams_gedma_and_sets_fin2`, +`gedma_read_with_hq_idle_is_an_overrun`, `gl_dma_read_*`. diff --git a/src/dev/gr2/mod.rs b/src/dev/gr2/mod.rs index 77132ed..a729337 100644 --- a/src/dev/gr2/mod.rs +++ b/src/dev/gr2/mod.rs @@ -171,13 +171,19 @@ pub struct Gr2 { /// kernel sees FIN2 as soon as the HQ2 gets there. See /// rules/gr2/fin2-wait-must-stall.md. fin2_wait: AtomicU32, - /// Host time (fin2_clock ns) of the first stalled FIN2 poll since the - /// wait was armed (0 = none yet). The stall gives up FIN2_STALL_LIMIT - /// after that, so a pipeline that can never finish (RE3 held by a - /// CPU-driven readback) cannot hang the machine. Counted from the first - /// poll, not from the ack: the DMA between ack and poll may take long on - /// a busy host (a 400-row pixel DMA on a GitHub CI runner did). + /// Host time (fin2_clock ns) of the last progress seen by a stalled FIN2 + /// poll since the wait was armed (0 = no stalled poll yet). The stall + /// gives up after FIN2_STALL_LIMIT without progress, so a pipeline that + /// can never finish (RE3 held by a CPU-driven readback) cannot hang the + /// machine, while a long but moving backlog is waited out: gltest + /// --bench 500 (500 full-screen quads, ~2.4 s of RE3 work) timed out a + /// context switch's FIN2 with a fixed 2 s cap and crashed the kernel in + /// Gr2PcxSwap. Not counted from the ack: the DMA between ack and poll + /// may take long on a busy host (a 400-row pixel DMA on a GitHub CI + /// runner did). fin2_armed_ns: AtomicU64, + /// HQ2 + RE3 FIFO consumer positions at the last stalled FIN2 poll. + fin2_progress: AtomicU64, /// HQ2_GEDMA read port: HQ2 -> host words (context saves 0x1E1, pixel /// DMA reads 0x152 / 0x0AC). A ring the HQ2 thread fills and the reader /// (the kernel's VDMA) drains; head / tail count words since reset. @@ -306,15 +312,20 @@ impl Gr2 { && !self.re3_busy.load(Ordering::Acquire) } - /// A FIN2 poll may stall: true until FIN2_STALL_LIMIT after the first - /// stalled poll of this wait (Gr2::fin2_armed_ns). + /// A FIN2 poll may stall: true until the HQ2 and RE3 have made no + /// progress for FIN2_STALL_LIMIT (Gr2::fin2_armed_ns). fn fin2_stall_ok(&self) -> bool { let now = fin2_clock().max(1); - let first = match self.fin2_armed_ns.compare_exchange(0, now, Ordering::AcqRel, Ordering::Acquire) { + let pos = (self.hq_fifo.consumed() as u64) ^ ((self.re3_fifo.consumed() as u64) << 32); + if self.fin2_progress.swap(pos, Ordering::AcqRel) != pos { + self.fin2_armed_ns.store(now, Ordering::Release); + return true; + } + let last = match self.fin2_armed_ns.compare_exchange(0, now, Ordering::AcqRel, Ordering::Acquire) { Ok(_) => now, Err(t) => t, }; - now.saturating_sub(first) < FIN2_STALL_LIMIT.as_nanos() as u64 + now.saturating_sub(last) < FIN2_STALL_LIMIT.as_nanos() as u64 } /// Spin until both engines are idle (tests and snapshots). diff --git a/src/gfifo.rs b/src/gfifo.rs index b00f434..f7ab0d9 100644 --- a/src/gfifo.rs +++ b/src/gfifo.rs @@ -60,6 +60,14 @@ impl GFifo { unsafe { std::mem::zeroed() } } + /// Entries the consumer has published as consumed since reset (wraps). + /// A change means the consumer made progress; the consumer publishes + /// in batches, so it lags by up to one batch. + #[inline] + pub fn consumed(&self) -> usize { + self.head.load(Ordering::Acquire) + } + /// Returns the approximate number of entries currently in the queue. #[inline] pub fn len(&self) -> usize {