Reset the register file's ring pointers on an NCRA reset
An NCRA reset returns RXFrameAssembler's write pointer to page 1, but the
register file's COPY of RWP was never reset -- it kept whatever the last frame
left. So after any reset the GC reads a stale RWP pointing at a page the
assembler is not using, and its first RX after a reset reads the wrong page.
CLAUDE.md has stated the intent all along ("NCRA = 0x01 ... resets W5500 +
SPRAM ptrs"); the register file simply never did it.
This is NOT only a netboot problem. Any driver that resets and then trusts
RWP hits it, Swiss included. It stayed invisible because the suite's RX test
only checks that RWP CHANGES, never that it points at the right page, and
t_dma used to read a hardcoded 0x100 so it never consulted RWP either. It
surfaced through netboot, whose RRP = RWP sync landed on a stale page and
found garbage where a descriptor should be.
T1c covers it, and BOTH halves have teeth. My first version did not: RWP read
[1] with or without the fix because nothing in the testbench had moved it. It
now drives the assembler's RWP update path to page 9 and writes RRP to 7
first, so the negative control fails on both:
T1c RWP after NCRA reset: expected [1], got [9]
T1c RRP after NCRA reset: expected [1], got [7]
A test that passes whether or not the fix is present is worse than no test.
Gateware only -- the test app needs no change and the existing .dol is
compatible: after its reset it reads RWP, now gets 1, and writes RRP = 1.
--seeds 8: clk 28.5-32.2 MHz, EXI 51.3-63.4 MHz, 8/8 PASS. Best seed 2
(clk 32.19, EXI 63.23), flashed.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -451,6 +451,24 @@ class BBARegisterFile(Elaboratable):
|
|||||||
m.d.sync += r_ncra[0].eq(0) # RESET self-clears
|
m.d.sync += r_ncra[0].eq(0) # RESET self-clears
|
||||||
m.d.sync += ncra_rst_pulse.eq(1)
|
m.d.sync += ncra_rst_pulse.eq(1)
|
||||||
m.d.sync += id_phase.eq(0)
|
m.d.sync += id_phase.eq(0)
|
||||||
|
# An NCRA reset resets the RX ring, so the
|
||||||
|
# pointers the GC reads MUST follow -- the
|
||||||
|
# assembler's own write pointer goes back to
|
||||||
|
# page 1, and without this the register file
|
||||||
|
# keeps whatever the last frame left.
|
||||||
|
#
|
||||||
|
# The GC then reads a stale RWP pointing at a
|
||||||
|
# page the assembler is not using, so its
|
||||||
|
# first RX after a reset reads the wrong page.
|
||||||
|
# Found via netboot: after Swiss's own reset,
|
||||||
|
# syncing RRP to RWP landed on a stale page
|
||||||
|
# and the descriptor there was garbage. This
|
||||||
|
# affects any driver that resets and then
|
||||||
|
# trusts RWP, Swiss included.
|
||||||
|
m.d.sync += [
|
||||||
|
r_rwp_lo.eq(_RX_PAGE_FIRST),
|
||||||
|
r_rrp_lo.eq(_RX_PAGE_FIRST),
|
||||||
|
]
|
||||||
# Fallback trigger: TWD_HI normally queues the
|
# Fallback trigger: TWD_HI normally queues the
|
||||||
# frame (see _TWD_HI). Only fire here if it did
|
# frame (see _TWD_HI). Only fire here if it did
|
||||||
# not, so software that skips TWD still transmits
|
# not, so software that skips TWD still transmits
|
||||||
@@ -748,6 +766,38 @@ if __name__ == "__main__":
|
|||||||
f"[{_RX_PAGE_FIRST}], got {got}")
|
f"[{_RX_PAGE_FIRST}], got {got}")
|
||||||
print(f"T1b {name} at reset: {got} (want [{_RX_PAGE_FIRST}])")
|
print(f"T1b {name} at reset: {got} (want [{_RX_PAGE_FIRST}])")
|
||||||
|
|
||||||
|
# T1c: an NCRA reset must put BOTH pointers back to page 1, even after
|
||||||
|
# they have been moved. The assembler's own write pointer returns to
|
||||||
|
# page 1 on reset, so a register file that keeps the old value hands
|
||||||
|
# the GC a RWP pointing at a page the assembler is not using -- and
|
||||||
|
# its first RX after a reset then reads the wrong page. That is a
|
||||||
|
# real-driver bug, not just a test-app one: Swiss resets and then
|
||||||
|
# trusts RWP.
|
||||||
|
await exi_write(ctx, _RRP_LO, [7]) # move RRP off page 1
|
||||||
|
got = await exi_read(ctx, _RRP_LO, 1)
|
||||||
|
if got != [7]:
|
||||||
|
errors.append(f"T1c RRP should be writable, got {got}")
|
||||||
|
# Move RWP too, the way the assembler does -- otherwise this half of
|
||||||
|
# the test has no teeth, because RWP would read 1 whether or not the
|
||||||
|
# reset clears it.
|
||||||
|
ctx.set(reg.rx_wptr_w_data, 9)
|
||||||
|
ctx.set(reg.rx_wptr_w_en, 1)
|
||||||
|
await ctx.tick("sync").repeat(1)
|
||||||
|
ctx.set(reg.rx_wptr_w_en, 0)
|
||||||
|
await ctx.tick("sync").repeat(4)
|
||||||
|
got = await exi_read(ctx, _RWP_LO, 1)
|
||||||
|
if got != [9]:
|
||||||
|
errors.append(f"T1c RWP should track the assembler, got {got}")
|
||||||
|
await exi_write(ctx, _NCRA, [0x01]) # NCRA reset
|
||||||
|
await ctx.tick("sync").repeat(4)
|
||||||
|
for name, a in (("RWP", _RWP_LO), ("RRP", _RRP_LO)):
|
||||||
|
got = await exi_read(ctx, a, 1)
|
||||||
|
if got != [_RX_PAGE_FIRST]:
|
||||||
|
errors.append(f"T1c {name} after NCRA reset: expected "
|
||||||
|
f"[{_RX_PAGE_FIRST}], got {got}")
|
||||||
|
print(f"T1c {name} after NCRA reset: {got} "
|
||||||
|
f"(want [{_RX_PAGE_FIRST}])")
|
||||||
|
|
||||||
# ...and the constant must not drift from the assembler's, which owns
|
# ...and the constant must not drift from the assembler's, which owns
|
||||||
# the write pointer. Two copies of a page number is exactly how the
|
# the write pointer. Two copies of a page number is exactly how the
|
||||||
# off-by-one got in.
|
# off-by-one got in.
|
||||||
|
|||||||
Reference in New Issue
Block a user