diff --git a/exi_bba/bba_register_file.py b/exi_bba/bba_register_file.py index 967bc81..544999a 100644 --- a/exi_bba/bba_register_file.py +++ b/exi_bba/bba_register_file.py @@ -343,7 +343,12 @@ class BBARegisterFile(Elaboratable): with m.If(self.rx_rdy): m.d.comb += self.rx_en.eq(1) m.d.sync += [is_write.eq(self.rx_data[6]), prefetched.eq(0), - byte_ctr.eq(0), sp_req.eq(0), outstanding.eq(0)] + byte_ctr.eq(0), sp_req.eq(0), outstanding.eq(0), + # sp_pending too: a byte still held from the + # PREVIOUS transaction would be pushed as this + # one's first response and shift the whole + # read by one. + sp_pending.eq(0)] with m.If(self.rx_data[7]): # register space m.d.sync += cmd_space.eq(0) m.next = "RHDR_HI" @@ -589,10 +594,26 @@ class BBARegisterFile(Elaboratable): # it outside the FSM: push straight to tx_fifo if it has room, else # hold it one cycle. SPRAM reads and register reads never overlap, so # driving tx here cannot collide with the FSM's own pushes. - with m.If(self.spram_rsp_valid & self.tx_rdy & ~sp_pending): + # ⚠️ A response is only OURS if a request is actually in flight. + # + # ETH writes preempt EXI reads in SPRAMArbiter (ETH has absolute + # priority and asserts eth_wr_ready combinationally), so a read + # response can arrive later than expected -- including after the + # transaction that asked for it has ended. Pushing it then INSERTS A + # BYTE at the start of the NEXT DMA read and shifts everything the GC + # reads by one. It also underflowed `outstanding` (4-bit: 0 - 1 = 15), + # which broke the prefetch bound for the rest of that transaction. + # + # Hardware symptom (2026-10-09): the SPRAM ring descriptor appearing at + # page offset +1 instead of +0 for 17 of 22 frames, VARYING frame to + # frame, while the assembler was writing it correctly all along. It + # only happens when the GC reads the ring while frames are being + # written into it, which no simulation did. + with m.If(self.spram_rsp_valid & (outstanding != 0) + & self.tx_rdy & ~sp_pending): m.d.comb += [self.tx_data.eq(self.spram_rsp_data), self.tx_en.eq(1)] m.d.sync += outstanding.eq(outstanding - 1) - with m.Elif(self.spram_rsp_valid): + with m.Elif(self.spram_rsp_valid & (outstanding != 0)): m.d.sync += [sp_hold.eq(self.spram_rsp_data), sp_pending.eq(1), outstanding.eq(outstanding - 1)] with m.Elif(sp_pending & self.tx_rdy): @@ -899,6 +920,29 @@ if __name__ == "__main__": errors.append(f"T7 NWAYS after DMA: got {result}") print(f"T7 NWAYS after DMA read: 0x{result[0]:02X}") + # NO T8 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 + # arrives after its transaction ended used to be pushed anyway, and + # `outstanding - 1` from zero underflowed a 4-bit counter to 15, + # breaking the prefetch bound for the rest of the transaction. + # + # But this testbench CANNOT demonstrate the consequence. It models the + # TX side as tx_rdy/tx_en with a fresh capture list per transaction -- + # there is no persistent tx FIFO and no header drain pops, which is + # exactly the mechanism by which a leaked byte shifts the NEXT read. A + # test written here passes whether or not the guard is present (checked + # both ways), and a test that cannot fail is worse than no test. + # + # WHAT IS ACTUALLY MISSING: a bba_top-level test that performs an EXI + # DMA read from the SPRAM ring WHILE the ethernet side writes frames + # into it. ETH has absolute priority in SPRAMArbiter, so only then can + # a read response be delayed past the end of its transaction. No + # simulation does this today, which is why the +1 descriptor offset + # (17 of 22 frames on hardware, 2026-10-09) is still unexplained. + + sim = Simulator(reg) sim.add_clock(Period(MHz=24), domain="sync") sim.add_testbench(testbench)