From ada937862888132a3af93ef2c938ff6cf1a0ba47 Mon Sep 17 00:00:00 2001 From: Dennis Brentjes Date: Fri, 9 Oct 2026 19:54:23 +0200 Subject: [PATCH] Restore the TX-done clears to the TX handler; shorten the ACK timeout TIMING MEASUREMENT, which is the useful part. Raising the host timeout 0.1 -> 0.3 barely moved the retry count (110 -> 95) but tripled the cost of each one: 16.9 s -> 40.1 s. If those were LATE ACKs, three times the deadline would have all but eliminated them. They are LOST ACKs, so a SHORT timeout is correct, not a long one -- the GC re-ACKs a duplicate block, so retrying early costs nothing. Host default is now 0.05. REGRESSION, mine, from 92f7d71. Inserting the RBFI handler split the TX-done block, leaving these two lines attached to rx_overflow -- an RX event: m.d.sync += r_ncra[1:3].eq(0) # clear ST bits m.d.sync += tx_queued.eq(0) # ready for the next frame So after the first transmit tx_queued stayed set forever. The NCRA ST fallback trigger is guarded on ~tx_queued, so software driving TX via ST rather than TWD would transmit exactly ONCE and then silently stop, and the ST bits were left asserted in NCRA. Our own path arms via TWD, which is why it kept working and why this was not noticed. T9 covers it and is mutation-tested: with the clears misplaced, NCRA reads 0x0A with ST stuck set. This does NOT explain the 1/3 ACK loss -- the TWD path was unaffected -- so that remains open. txto from the next run is the discriminator: near zero means the frames reach the W5100 and are lost after that, which points at the chip's TX handling rather than the EXI side. --seeds 8: clk 27.5-29.4 MHz, EXI 54.7-62.3 MHz, 8/8 PASS. Best seed 5 (clk 29.41, EXI 60.22), flashed. The .dol is unchanged; only send-dol.py's default timeout moved, and that is read from the repo. Co-Authored-By: Claude Opus 5 --- exi_bba/bba_register_file.py | 42 ++++++++++++++++++++++++++++++++---- scripts/send-dol.py | 13 ++++++----- 2 files changed, 46 insertions(+), 9 deletions(-) diff --git a/exi_bba/bba_register_file.py b/exi_bba/bba_register_file.py index d99b43c..5333b7c 100644 --- a/exi_bba/bba_register_file.py +++ b/exi_bba/bba_register_file.py @@ -687,8 +687,17 @@ class BBARegisterFile(Elaboratable): m.d.sync += twd_armed.eq(0) m.d.sync += tx_queued.eq(1) + # TX done. Clearing the ST bits and tx_queued belongs HERE, with the + # completion that they describe. Adding the RBFI handler below split + # this block and left those two lines attached to rx_overflow -- an RX + # event -- so after the first frame tx_queued stayed set forever, which + # disables the NCRA ST fallback trigger entirely (it is guarded on + # ~tx_queued) and leaves the ST bits asserted in NCRA. Software that + # drives TX via ST instead of TWD would transmit exactly once. with m.If(self.tx_irq_i): - m.d.sync += r_ir[2].eq(1) # TI bit + m.d.sync += r_ir[2].eq(1) # TI bit + m.d.sync += r_ncra[1:3].eq(0) # clear ST bits + m.d.sync += tx_queued.eq(0) # ready for the next frame # RBFI: the ring was full and a frame was discarded. Reported rather # than hidden -- the previous behaviour was to overwrite unread pages, @@ -696,8 +705,6 @@ class BBARegisterFile(Elaboratable): # gone wrong. with m.If(self.rx_overflow): m.d.sync += r_ir[7].eq(1) - m.d.sync += r_ncra[1:3].eq(0) # clear ST bits - m.d.sync += tx_queued.eq(0) # ready for the next frame return m @@ -996,7 +1003,34 @@ if __name__ == "__main__": print(f"T8 multi-byte TXDATA write: {len(got)}/{len(payload)} bytes " f"{'ok' if got == payload else 'MISMATCH'}") - # NO T9 FOR THE LATE-RESPONSE LEAK -- and that is deliberate. + # T9: TX completion must clear the ST bits and re-arm for the next + # frame. Those two actions used to sit in the rx_overflow handler by + # accident, so after ONE transmit tx_queued stayed set forever: the + # NCRA ST fallback trigger is guarded on ~tx_queued, so software that + # drives TX via ST rather than TWD transmitted exactly once and then + # silently stopped. + await exi_write(ctx, _NCRA, [0x08 | 0x02]) # SR + ST0 + await ctx.tick("sync").repeat(2) + ctx.set(reg.tx_irq_i, 1) # frame completed + await ctx.tick("sync").repeat(1) + ctx.set(reg.tx_irq_i, 0) + await ctx.tick("sync").repeat(2) + got = await exi_read(ctx, _NCRA, 1) + if got and (got[0] & 0x06): + errors.append(f"T9 ST bits not cleared on TX done: NCRA=0x{got[0]:02X}") + print(f"T9 NCRA after TX done: 0x{got[0]:02X} (ST bits must be clear)") + + # ...and a SECOND ST-triggered transmit must still queue, which it + # cannot if tx_queued was left set. + tx_ctrl_before = ctx.get(reg.tx_ctrl_r_rdy) + await exi_write(ctx, _NCRA, [0x08 | 0x02]) + await ctx.tick("sync").repeat(4) + if not ctx.get(reg.tx_ctrl_r_rdy): + errors.append("T9 second ST trigger did not queue a frame") + print(f"T9 second ST trigger queues: {bool(ctx.get(reg.tx_ctrl_r_rdy))} " + f"(was {bool(tx_ctrl_before)})") + + # NO T10 FOR THE LATE-RESPONSE LEAK -- and that is deliberate. # # The guard added to the response handler (outstanding != 0) and the # sp_pending clear in HEADER0 fix a real latent bug: a response that diff --git a/scripts/send-dol.py b/scripts/send-dol.py index bce698e..fe6f647 100755 --- a/scripts/send-dol.py +++ b/scripts/send-dol.py @@ -58,11 +58,14 @@ def main(): ap = argparse.ArgumentParser() ap.add_argument("iface", help="network interface facing the GameCube") ap.add_argument("dol", help=".dol to send") - ap.add_argument("--timeout", type=float, default=0.3, - help="per-block ACK timeout in seconds (default 0.3). " - "The GC needs ~20 ms per block and the variance is " - "what matters: at 0.1 roughly a third of blocks " - "overshot and each miss cost a full timeout.") + ap.add_argument("--timeout", type=float, default=0.05, + help="per-block ACK timeout in seconds (default 0.05). " + "Measured: raising it 0.1 -> 0.3 barely changed the " + "retry count (110 -> 95) but tripled the cost of " + "each one (16.9 s -> 40.1 s), so the misses are " + "LOST ACKs, not late ones -- which makes a SHORT " + "timeout the right answer. The GC re-ACKs a " + "duplicate block, so retrying early is harmless.") ap.add_argument("--retries", type=int, default=20, help="per-block retransmits before giving up (default 20)") ap.add_argument("--dest", default="/gc_bba_test.dol",