Roflin
6b2e6724a2
The sweep after honouring BP/RHBP came back clk 16.5-17.6 MHz, 0/8 seeds
passing against the 24 MHz target -- from ~29 MHz before. The build was
unusable.
Cause: the sanitised bounds were combinational, so
bp/rhbp -> comparators -> Mux -> subtract -> pg_total -> add -> raw
-> compare -> free -> compare -> have_room
became ONE path where it had been constant-folded. Two fixes were needed, and
the first alone was not enough:
1. Register pg_first/pg_last/pg_total and rrp_s. They change only when the GC
writes BP/RHBP/RRP, so a cycle of staleness costs nothing, and `free`
already reaches the FSM through the registered have_room. This got it to
~20 MHz, still failing.
2. Narrow them from 8 bits to 4. Ring pages are 1-15. An 8-bit variable-vs-
variable comparison is a carry chain where variable-vs-CONSTANT folds into
a couple of LUTs -- precisely what making the bounds dynamic had thrown
away. geom_ok guarantees bp < rhbp <= 0x0F, which makes the narrow slice
safe.
Now 26.90 and 28.50 MHz on seeds 1-2, against 28.00/29.21 for the build
without the geometry change.
Attribution needed an A/B: the register-space decode (item 4) landed in the
same stretch and had never been swept on its own. Measured separately it
closes 28.0-29.2 MHz and costs nothing, so the whole regression was the
assembler. The critical-path report named rx_frame_assembler.py:215 -- the
rrp clamp -- which is what pointed at the comparator width rather than at the
arithmetic everyone would suspect first.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
845 lines
40 KiB
Python
845 lines
40 KiB
Python
"""RX frame assembler — sync domain (24 MHz).
|
||
|
||
Receives raw ethernet frames from W5500SPIMaster and writes them into the SPRAM
|
||
ring buffer in MX98730EC format.
|
||
|
||
Ring buffer layout (SPRAM byte addresses)
|
||
------------------------------------------
|
||
0x0100–0x0FFF 15 pages × 256 bytes = 3840 bytes
|
||
Pages 0x01–0x0F; page 0x00 is reserved.
|
||
Page wrap: after 0x0F → 0x01 (skip 0x00).
|
||
|
||
Frame descriptor (4 bytes at page start)
|
||
-----------------------------------------------
|
||
A PACKED 32-BIT LITTLE-ENDIAN WORD, not a byte-wise layout:
|
||
|
||
bits 0–11 : next_packet_ptr — the PAGE at which the NEXT frame starts
|
||
bits 12–23 : packet_len — total bytes, INCLUDING these 4
|
||
bits 24–31 : status — errors are 0x80 and 0x04; 0x00 means good
|
||
|
||
so the four bytes at the page start are
|
||
|
||
Byte 0: next_packet_ptr[7:0]
|
||
Byte 1: packet_len[3:0] << 4 | next_packet_ptr[11:8]
|
||
Byte 2: packet_len[11:4]
|
||
Byte 3: status
|
||
Bytes 4+: raw ethernet frame
|
||
|
||
⚠️ This is NOT a guess and must not be "simplified" back to a plain
|
||
big-endian length. It was read out of libogc's own BBA driver
|
||
(`/opt/devkitpro/libogc/lib/cube/libbba.a`, `gcif.o`, `bba_interrupt`), which
|
||
is the driver GC software actually runs:
|
||
|
||
lwbrx r9,0,r9 # load the 4 bytes as a LITTLE-ENDIAN word
|
||
rlwinm r4,r9,20,20,31 # packet_len = (word >> 12) & 0xFFF
|
||
addi r4,r4,-4 # payload = packet_len - 4
|
||
cmpwi r4,1540 # ... which must be <= 1540
|
||
srwi r9,r9,24 # status = word >> 24
|
||
and. r9,r9,r18 # r18 = 0x...84, so errors are 0x80 | 0x04
|
||
|
||
and it writes the low 12 bits straight back to RRP as the next read pointer:
|
||
|
||
lwz r9,cur_descr # byte 0 -> register 0x0018 (RRP_LO)
|
||
rlwinm r9,r31,24,28,31 # bits 11:8 -> register 0x0019 (RRP_HI)
|
||
|
||
The length semantics and addressing already matched (descriptor at page<<8,
|
||
body at +4, length including the descriptor); the PACKING did not, and
|
||
`next_packet_ptr` was never written at all. Our previous
|
||
`00 00 len[11:8] len[7:0]` decodes under that driver as packet_len = 16 and
|
||
next_packet_ptr = 0 for a 342-byte frame — a 12-byte frame and a read pointer
|
||
into the reserved page.
|
||
|
||
Write sequence
|
||
--------------
|
||
1. Issue 4 SPRAM writes of 0x00 (placeholder descriptor).
|
||
2. For each byte received from W5500, issue one SPRAM write.
|
||
3. After EOF: compute the next page, then rewrite descriptor bytes 0, 1 and 2
|
||
with the packed next_packet_ptr + packet_len. Byte 3 (status) stays 0x00.
|
||
4. Advance RWP, push to rx_wptr FIFO, pulse rx_irq.
|
||
"""
|
||
|
||
from amaranth import *
|
||
|
||
__all__ = ["RXFrameAssembler"]
|
||
|
||
_RX_PAGE_FIRST = 0x01
|
||
_RX_PAGE_LAST = 0x0F
|
||
_PAGES_TOTAL = _RX_PAGE_LAST - _RX_PAGE_FIRST + 1 # 15
|
||
# Worst-case pages one frame can occupy: 1518 + 4 descriptor bytes -> 6 pages.
|
||
_MAX_FRAME_PAGES = 6
|
||
|
||
|
||
class RXFrameAssembler(Elaboratable):
|
||
"""Writes incoming ethernet frames into the SPRAM ring buffer.
|
||
|
||
W5500 streaming interface (sync domain)
|
||
----------------------------------------
|
||
rx_data / rx_valid / rx_ready : byte stream
|
||
rx_sof / rx_eof : frame delimiters (same cycle as rx_valid)
|
||
|
||
SPRAM write interface (to SPRAMArbiter, sync domain)
|
||
-----------------------------------------------------
|
||
eth_wr_addr / eth_wr_data / eth_wr_valid / eth_wr_ready
|
||
|
||
CDC outputs (wired by BBATop)
|
||
-----------------------------
|
||
rx_wptr_w_data / rx_wptr_w_en / rx_wptr_w_rdy
|
||
rx_irq : 1-cycle pulse → PulseSynchronizer input
|
||
rx_enabled : controlled by NCRA SR bit (from BBARegisterFile)
|
||
"""
|
||
|
||
def __init__(self):
|
||
# W5500 stream in
|
||
self.rx_data = Signal(8)
|
||
self.rx_valid = Signal()
|
||
self.rx_ready = Signal()
|
||
self.rx_sof = Signal()
|
||
self.rx_eof = Signal()
|
||
|
||
# SPRAM write out
|
||
self.eth_wr_addr = Signal(16)
|
||
self.eth_wr_data = Signal(8)
|
||
self.eth_wr_valid = Signal()
|
||
self.eth_wr_ready = Signal()
|
||
|
||
# RWP FIFO write-side (sync→exi)
|
||
self.rx_wptr_w_data = Signal(8)
|
||
self.rx_wptr_w_en = Signal()
|
||
self.rx_wptr_w_rdy = Signal()
|
||
|
||
# rx_irq pulse (→ PulseSynchronizer)
|
||
self.rx_irq = Signal()
|
||
|
||
# RX gate from NCRA SR bit
|
||
self.rx_enabled = Signal()
|
||
|
||
# ── Ring-overflow protection (RBFI) ───────────────────────────────
|
||
# The GC's read pointer, so we can tell how much of the ring is still
|
||
# unread. Without it this module overwrote pages the GC had not read
|
||
# yet: a traffic burst corrupted whatever frame the driver was midway
|
||
# through, with no indication that it had happened.
|
||
self.rrp = Signal(8, init=_RX_PAGE_FIRST)
|
||
# ── Ring geometry, from the GC's own BP / RHBP registers ──────────
|
||
# compat TODO item 2. These were hardcoded, so a driver configuring a
|
||
# SMALLER RX ring was ignored and we kept writing pages it would never
|
||
# read -- silent corruption with nothing reporting an error. libogc2
|
||
# writes BP = 0x0001 and RHBP = 0x000F, which happens to match the old
|
||
# hardcoded layout exactly, which is why it never showed up.
|
||
self.bp = Signal(12, init=_RX_PAGE_FIRST) # first RX page
|
||
self.rhbp = Signal(12, init=_RX_PAGE_LAST) # last RX page
|
||
# Pulses when a frame had to be DROPPED because the ring was full.
|
||
# Drives IR[7] (RBFI), which the GC's IMR enables (0x86).
|
||
self.rx_overflow = Signal()
|
||
# High when the driver's BP/RHBP were usable and are being honoured;
|
||
# low means they were rejected and the hardcoded layout is in use, so
|
||
# a mismatch is observable rather than silent.
|
||
self.geom_honoured = Signal()
|
||
|
||
# The GC's NCRA reset. MUST reset `rwp` here as well as the register
|
||
# file's copy: they are two copies of the same pointer, and resetting
|
||
# only one leaves them disagreeing about where the ring is. A driver
|
||
# that then syncs RRP to the register file's value is pointing
|
||
# somewhere the writer is not, the free-space calculation shrinks with
|
||
# every frame, and once it falls below _MAX_FRAME_PAGES EVERY frame is
|
||
# dropped -- silently, because a dropped frame never reaches the ring
|
||
# to be counted. Hardware symptom: "0 seen, 0 bad desc, 0 lapped".
|
||
self.ncra_rst = Signal()
|
||
|
||
def elaborate(self, platform):
|
||
m = Module()
|
||
|
||
# ── Ring-buffer state ─────────────────────────────────────────────
|
||
rwp = Signal(8, init=_RX_PAGE_FIRST) # current RX write page (1–15)
|
||
|
||
# Write address within current frame
|
||
wr_addr = Signal(16)
|
||
# Number of frame data bytes received
|
||
data_ctr = Signal(12)
|
||
# Total length = data_ctr + 4
|
||
total_len = Signal(12)
|
||
# The page the NEXT frame will start at. Computed before the
|
||
# descriptor is written, because the descriptor carries it as
|
||
# next_packet_ptr -- libogc writes those 12 bits straight to RRP.
|
||
next_page = Signal(12)
|
||
|
||
# Descriptor base (rwp*256) — saved when frame starts
|
||
desc_base = Signal(16)
|
||
|
||
# Placeholder descriptor byte counter (0..3)
|
||
desc_ctr = Signal(2)
|
||
|
||
# Number of pages consumed by this frame (rounded up)
|
||
pages_used = Signal(5)
|
||
|
||
# Default: no pulses
|
||
m.d.sync += self.rx_irq.eq(0)
|
||
m.d.sync += self.rx_wptr_w_en.eq(0)
|
||
m.d.sync += self.rx_overflow.eq(0)
|
||
|
||
# ── Sanitised ring bounds ─────────────────────────────────────────
|
||
# A driver's BP/RHBP are used only when they describe a usable ring
|
||
# inside SPRAM; otherwise the hardcoded layout stands. Without this a
|
||
# garbage write (BP = 0, or RHBP below BP) would give a zero or
|
||
# negative ring size and the assembler would write outside it. Page 0
|
||
# is reserved for the TX buffer, so BP must be at least 1.
|
||
# ⚠️ REGISTERED, not combinational. Computing these every cycle put
|
||
# bp/rhbp -> comparators -> Mux -> subtract -> pg_total -> add -> raw
|
||
# -> compare -> free -> compare -> have_room all in ONE combinational
|
||
# path, where it used to be constant-folded. `clk` fmax collapsed
|
||
# from ~29 MHz to ~17 MHz and ALL EIGHT SEEDS FAILED the 24 MHz
|
||
# target. bp/rhbp only change when the GC writes BP/RHBP, so one
|
||
# cycle of staleness costs nothing and splits the path in two.
|
||
# ⚠️ REGISTERED AND 4 BITS WIDE, both deliberately.
|
||
#
|
||
# Computing these combinationally put bp/rhbp -> comparators -> Mux ->
|
||
# subtract -> pg_total -> add -> raw -> compare -> free -> compare ->
|
||
# have_room in ONE path, where it used to be constant-folded. `clk`
|
||
# fmax collapsed from ~29 MHz to ~17 MHz and ALL EIGHT SEEDS FAILED
|
||
# the 24 MHz target. Registering split it, but 8-bit variable-vs-
|
||
# variable comparators are carry chains where variable-vs-CONSTANT
|
||
# folds into a couple of LUTs, so it was still only ~20 MHz.
|
||
#
|
||
# Ring pages are 1-15, so 4 bits is the natural width; geom_ok
|
||
# guarantees bp < rhbp <= 0x0F, which is what makes the narrow slice
|
||
# safe. bp/rhbp change only when the GC writes BP/RHBP, so a cycle of
|
||
# staleness costs nothing.
|
||
geom_ok = Signal()
|
||
geom_ok_c = Signal()
|
||
pg_first = Signal(4, init=_RX_PAGE_FIRST)
|
||
pg_last = Signal(4, init=_RX_PAGE_LAST)
|
||
pg_total = Signal(5, init=_PAGES_TOTAL)
|
||
m.d.comb += geom_ok_c.eq((self.bp >= _RX_PAGE_FIRST)
|
||
& (self.rhbp <= _RX_PAGE_LAST)
|
||
& (self.rhbp > self.bp))
|
||
m.d.sync += [
|
||
geom_ok .eq(geom_ok_c),
|
||
pg_first.eq(Mux(geom_ok_c, self.bp[0:4], _RX_PAGE_FIRST)),
|
||
pg_last .eq(Mux(geom_ok_c, self.rhbp[0:4], _RX_PAGE_LAST)),
|
||
pg_total.eq(Mux(geom_ok_c, self.rhbp[0:4] - self.bp[0:4] + 1,
|
||
_PAGES_TOTAL)),
|
||
]
|
||
m.d.comb += self.geom_honoured.eq(geom_ok)
|
||
|
||
# ── How many pages may we still consume? ──────────────────────────
|
||
# rwp == rrp means EMPTY, so writing the whole ring would wrap back to
|
||
# "empty" and lose the lot. Usable = (rrp - rwp - 1) mod pg_total,
|
||
# which is pg_total-1 when the ring is empty.
|
||
# rrp_s is REGISTERED too: the clamp is two comparisons against the
|
||
# (now variable) bounds, and the GC writes RRP about as often as it
|
||
# writes BP/RHBP. `free` already reaches the FSM through a register
|
||
# (`have_room`), so this adds no behavioural latency that matters.
|
||
rrp_s = Signal(4, init=_RX_PAGE_FIRST)
|
||
m.d.sync += rrp_s.eq(Mux((self.rrp >= pg_first)
|
||
& (self.rrp <= pg_last),
|
||
self.rrp[0:4], pg_first))
|
||
raw = Signal(5)
|
||
free = Signal(5)
|
||
with m.If(rrp_s >= rwp):
|
||
m.d.comb += raw.eq(rrp_s - rwp)
|
||
with m.Else():
|
||
m.d.comb += raw.eq(rrp_s + pg_total - rwp)
|
||
m.d.comb += free.eq(Mux(raw == 0, pg_total - 1, raw - 1))
|
||
|
||
# A frame's length is not known until EOF, so reserve the worst case:
|
||
# 1518 bytes + a 4-byte descriptor is 6 pages. Starting a frame with
|
||
# less room than that risks running into unread data partway through,
|
||
# which is worse than refusing it outright.
|
||
# REGISTERED, not combinational. The free-space calculation is a
|
||
# mod-15 subtract plus a comparator, and feeding that into the FSM's
|
||
# branch condition cost real depth in `sync` -- the domain with the
|
||
# least margin in this design (clk fell 27.8-32.2 -> 27.0-28.0 MHz
|
||
# when this was comb). One cycle of staleness is harmless: rwp only
|
||
# moves at end-of-frame and rrp only when the GC writes it, so both
|
||
# are many cycles old by the time the next SOF arrives.
|
||
have_room = Signal()
|
||
m.d.sync += have_room.eq(free >= _MAX_FRAME_PAGES)
|
||
|
||
# Combinatorial outputs
|
||
m.d.comb += total_len.eq(data_ctr + 4)
|
||
|
||
with m.FSM(domain="sync", name="rx_fsm"):
|
||
|
||
with m.State("IDLE"):
|
||
m.d.comb += self.rx_ready.eq(0)
|
||
m.d.sync += self.eth_wr_valid.eq(0)
|
||
with m.If(self.rx_valid & self.rx_sof & self.rx_enabled
|
||
& ~have_room):
|
||
# Ring full: discard this frame rather than overwrite data
|
||
# the GC has not read. This is what the real BBA's RBFI
|
||
# reports, and what this module previously lacked.
|
||
m.next = "DROP_FRAME"
|
||
with m.Elif(self.rx_valid & self.rx_sof & self.rx_enabled):
|
||
frame_base = Signal(16)
|
||
m.d.comb += frame_base.eq(Cat(Const(0, 8), rwp))
|
||
m.d.sync += desc_base.eq(frame_base)
|
||
m.d.sync += wr_addr.eq(frame_base)
|
||
m.d.sync += data_ctr.eq(0)
|
||
m.d.sync += desc_ctr.eq(0)
|
||
m.next = "WRITE_PLACEHOLDER"
|
||
|
||
with m.State("DROP_FRAME"):
|
||
# Consume and discard to EOF. rx_ready MUST stay high here:
|
||
# stalling the ethernet master instead would back pressure into
|
||
# the W5100's own buffer and turn a dropped frame into a wedged
|
||
# receiver.
|
||
m.d.comb += self.rx_ready.eq(1)
|
||
with m.If(self.rx_valid & self.rx_eof):
|
||
m.d.sync += self.rx_overflow.eq(1)
|
||
m.next = "IDLE"
|
||
|
||
with m.State("WRITE_PLACEHOLDER"):
|
||
# Write 4 bytes of 0x00 as placeholder descriptor.
|
||
#
|
||
# ⚠️ Advance on `valid & ready`, NOT on `ready` alone.
|
||
# `eth_wr_valid` is scheduled with m.d.sync and so is LOW for
|
||
# the whole first cycle in this state, while `ready` is tested
|
||
# immediately -- and `ready` is usually already high here.
|
||
# Testing `ready` alone therefore counted a write that was
|
||
# never issued: only THREE placeholder bytes went out and
|
||
# wr_addr ended at desc_base+3, so the frame body started at
|
||
# descriptor byte 3 and EVERY RECEIVED FRAME WAS SHIFTED ONE
|
||
# BYTE EARLY.
|
||
#
|
||
# Hardware symptom (2026-10-09): the ring filled normally and
|
||
# RWP advanced, but the GC's ethertype test read the wrong
|
||
# offset and silently dropped every frame -- "no ACK" from the
|
||
# host with no other sign of trouble. The matching
|
||
# descriptor bug (same cause, in the length write) showed up as a
|
||
# descriptor of 00 00 00 01.
|
||
#
|
||
# The first byte is written twice as a consequence of
|
||
# eth_wr_addr trailing wr_addr by a cycle, and desc_base+3 is
|
||
# left unwritten -- both harmless, because every descriptor
|
||
# byte is either zero anyway or overwritten by WRITE_LEN_*.
|
||
# What matters is that wr_addr ends at desc_base+4.
|
||
m.d.sync += self.eth_wr_addr .eq(wr_addr)
|
||
m.d.sync += self.eth_wr_data .eq(0x00)
|
||
m.d.sync += self.eth_wr_valid.eq(1)
|
||
with m.If(self.eth_wr_valid & self.eth_wr_ready):
|
||
m.d.sync += wr_addr.eq(wr_addr + 1)
|
||
with m.If(desc_ctr == 3):
|
||
m.d.sync += self.eth_wr_valid.eq(0)
|
||
m.next = "RECV_AND_WRITE"
|
||
with m.Else():
|
||
m.d.sync += desc_ctr.eq(desc_ctr + 1)
|
||
|
||
with m.State("RECV_AND_WRITE"):
|
||
# Accept bytes from W5500 and write each to SPRAM immediately
|
||
m.d.comb += self.rx_ready.eq(~self.eth_wr_valid | self.eth_wr_ready)
|
||
with m.If(self.rx_valid & (~self.eth_wr_valid | self.eth_wr_ready)):
|
||
m.d.sync += self.eth_wr_addr.eq(wr_addr)
|
||
m.d.sync += self.eth_wr_data.eq(self.rx_data)
|
||
m.d.sync += self.eth_wr_valid.eq(1)
|
||
m.d.sync += wr_addr.eq(wr_addr + 1)
|
||
m.d.sync += data_ctr.eq(data_ctr + 1)
|
||
with m.If(self.rx_eof):
|
||
m.next = "WAIT_LAST_WRITE"
|
||
with m.Elif(self.eth_wr_valid & self.eth_wr_ready):
|
||
m.d.sync += self.eth_wr_valid.eq(0)
|
||
|
||
with m.State("WAIT_LAST_WRITE"):
|
||
# Wait for the last data byte write to be accepted
|
||
with m.If(~self.eth_wr_valid | self.eth_wr_ready):
|
||
m.d.sync += self.eth_wr_valid.eq(0)
|
||
# Compute pages used: ceil((data_ctr + 4) / 256)
|
||
# = (total_len + 255) >> 8 = total_len[11:8] + (total_len[7:0] != 0)
|
||
m.d.sync += pages_used.eq(total_len[8:12] + (total_len[:8] != 0))
|
||
m.next = "CALC_NEXT"
|
||
|
||
# ⚠️ All three descriptor writes must advance on `valid & ready`,
|
||
# NOT on `ready` alone. `valid` is scheduled with m.d.sync and so is low
|
||
# for the whole first cycle in the state, while `ready` is tested
|
||
# immediately -- and coming out of WAIT_LAST_WRITE `ready` is
|
||
# usually already high. Testing `ready` alone therefore fired both
|
||
# valid.eq(1) and valid.eq(0) in the same cycle, the later won, the
|
||
# write was never issued, and the length byte was SILENTLY DROPPED.
|
||
#
|
||
# Hardware symptom (2026-10-09): a descriptor of 00 00 00 01 --
|
||
# byte 2 still at its placeholder 0x00 because the HI write was
|
||
# dropped -- so the GC read a 1-byte frame length and rejected the
|
||
# ring. Intermittent by nature: it depends on whether the arbiter
|
||
# happens to hold `ready` high on the entry cycle, which is why
|
||
# bring-up's 342-byte DHCP frame read a correct 00 00 01 5A and
|
||
# this went unnoticed.
|
||
with m.State("CALC_NEXT"):
|
||
# The descriptor carries next_packet_ptr, so the next page must
|
||
# be known BEFORE the descriptor is written -- this state used
|
||
# to come after it (as ADVANCE_RWP).
|
||
# next_rwp = ((rwp - 1 + pages_used) % 15) + 1
|
||
next_rwp_raw = Signal(8)
|
||
m.d.comb += next_rwp_raw.eq(rwp + pages_used)
|
||
with m.If(next_rwp_raw > pg_last):
|
||
m.d.sync += next_page.eq(next_rwp_raw - pg_total)
|
||
with m.Else():
|
||
m.d.sync += next_page.eq(next_rwp_raw)
|
||
m.next = "WRITE_D0"
|
||
|
||
with m.State("WRITE_D0"):
|
||
# Descriptor byte 0 = next_packet_ptr[7:0]
|
||
m.d.sync += self.eth_wr_addr.eq(desc_base + 0)
|
||
m.d.sync += self.eth_wr_data.eq(next_page[0:8])
|
||
m.d.sync += self.eth_wr_valid.eq(1)
|
||
with m.If(self.eth_wr_valid & self.eth_wr_ready):
|
||
m.d.sync += self.eth_wr_valid.eq(0)
|
||
m.next = "WRITE_D1"
|
||
|
||
with m.State("WRITE_D1"):
|
||
# Descriptor byte 1 = packet_len[3:0] << 4 | next_ptr[11:8].
|
||
# Cat puts its FIRST argument in the low bits.
|
||
m.d.sync += self.eth_wr_addr.eq(desc_base + 1)
|
||
m.d.sync += self.eth_wr_data.eq(
|
||
Cat(next_page[8:12], total_len[0:4]))
|
||
m.d.sync += self.eth_wr_valid.eq(1)
|
||
with m.If(self.eth_wr_valid & self.eth_wr_ready):
|
||
m.d.sync += self.eth_wr_valid.eq(0)
|
||
m.next = "WRITE_D2"
|
||
|
||
with m.State("WRITE_D2"):
|
||
# Descriptor byte 2 = packet_len[11:4]. Byte 3 (status) keeps
|
||
# the placeholder 0x00, which libogc reads as "no error".
|
||
m.d.sync += self.eth_wr_addr.eq(desc_base + 2)
|
||
m.d.sync += self.eth_wr_data.eq(total_len[4:12])
|
||
m.d.sync += self.eth_wr_valid.eq(1)
|
||
with m.If(self.eth_wr_valid & self.eth_wr_ready):
|
||
m.d.sync += self.eth_wr_valid.eq(0)
|
||
m.next = "WRITE_D3"
|
||
|
||
with m.State("WRITE_D3"):
|
||
# Descriptor byte 3 = status = 0x00 ("no error": libogc tests
|
||
# it against 0x84).
|
||
#
|
||
# ⚠️ This MUST be written explicitly and cannot rely on the
|
||
# placeholder. WRITE_PLACEHOLDER issues four writes but the
|
||
# first lands twice (eth_wr_addr trails wr_addr by a cycle),
|
||
# so desc_base+3 is NEVER written. That was harmless while
|
||
# the length write covered byte 3; now byte 3 is the status
|
||
# and leaving it would hand libogc whatever the previous
|
||
# frame -- or power-on -- left in that SPRAM cell, and any
|
||
# frame landing on a stale 0x80 or 0x04 would be discarded as
|
||
# an error. The simulation model reports it as an unwritten
|
||
# cell; real SPRAM has no such tell.
|
||
m.d.sync += self.eth_wr_addr.eq(desc_base + 3)
|
||
m.d.sync += self.eth_wr_data.eq(0x00)
|
||
m.d.sync += self.eth_wr_valid.eq(1)
|
||
with m.If(self.eth_wr_valid & self.eth_wr_ready):
|
||
m.d.sync += self.eth_wr_valid.eq(0)
|
||
m.next = "ADVANCE_RWP"
|
||
|
||
with m.State("ADVANCE_RWP"):
|
||
m.d.sync += rwp.eq(next_page)
|
||
m.next = "PUSH_WPT"
|
||
|
||
with m.State("PUSH_WPT"):
|
||
with m.If(self.rx_wptr_w_rdy):
|
||
m.d.sync += self.rx_wptr_w_data.eq(rwp)
|
||
m.d.sync += self.rx_wptr_w_en.eq(1)
|
||
m.d.sync += self.rx_irq.eq(1)
|
||
m.next = "IDLE"
|
||
|
||
# ── NCRA reset: the write pointer goes back to the ring start ──────
|
||
# Placed AFTER the FSM so it takes priority over ADVANCE_RWP in the
|
||
# same cycle.
|
||
#
|
||
# This MUST happen here as well as in the register file: they are two
|
||
# copies of the same pointer, and resetting only one leaves them
|
||
# disagreeing about where the ring is. A driver that then syncs RRP to
|
||
# the register file's value points somewhere the writer is not, the
|
||
# free-space calculation shrinks with every frame, and once it drops
|
||
# below _MAX_FRAME_PAGES EVERY frame is dropped -- silently, because a
|
||
# dropped frame never reaches the ring to be counted. Hardware
|
||
# symptom: "timeout: no match (0 seen, 0 bad desc, 0 lapped)".
|
||
#
|
||
# No in-flight frame needs abandoning: the same NCRA write that pulses
|
||
# this also clears the SR bit, so rx_enabled is low and the FSM is in
|
||
# IDLE.
|
||
with m.If(self.ncra_rst):
|
||
m.d.sync += rwp.eq(pg_first)
|
||
|
||
return m
|
||
|
||
|
||
# ── Testbench ─────────────────────────────────────────────────────────────
|
||
|
||
if __name__ == "__main__":
|
||
import sys
|
||
from amaranth.sim import Simulator, Period
|
||
|
||
dut = RXFrameAssembler()
|
||
errors = []
|
||
|
||
def brief(v):
|
||
"""Long byte lists are unreadable in a pass/fail line; show the ends
|
||
and the length, and keep the full value only for a failure."""
|
||
if isinstance(v, list) and len(v) > 8:
|
||
head = ", ".join(str(x) for x in v[:4])
|
||
tail = ", ".join(str(x) for x in v[-2:])
|
||
return f"[{head}, ... {tail}] ({len(v)} bytes)"
|
||
return str(v)
|
||
|
||
def check(name, got, want):
|
||
ok = got == want
|
||
print(f" {'ok ' if ok else 'FAIL'} {name}: "
|
||
f"got {brief(got)}, want {brief(want)}")
|
||
if not ok:
|
||
if isinstance(got, list) and isinstance(want, list):
|
||
first = next((i for i, (g, w) in enumerate(zip(got, want))
|
||
if g != w), None)
|
||
if first is not None:
|
||
print(f" first difference at index {first}: "
|
||
f"got {got[first]}, want {want[first]}")
|
||
errors.append(name)
|
||
|
||
async def testbench(ctx):
|
||
# Final contents of every SPRAM address the DUT wrote.
|
||
#
|
||
# The old testbench declared a writes list and never filled it, and
|
||
# drove the RX stream OPEN-LOOP -- one byte per tick, never checking
|
||
# rx_ready. Both of the 2026-10-09 handshake bugs were therefore
|
||
# invisible to it: it only ever asserted on rwp and rx_irq, so a
|
||
# dropped descriptor byte or a frame body written one byte early
|
||
# changed nothing it looked at.
|
||
mem = {}
|
||
order = []
|
||
|
||
# rx_irq and rx_overflow are 1-CYCLE pulses, and rx_overflow fires on
|
||
# the very tick that consumes the EOF byte -- i.e. inside send_frame.
|
||
# Polling for it afterwards misses it entirely, which first showed up
|
||
# as a dropped frame looking like a stall. Latch both in tick().
|
||
pulse = {"irq": None, "ovf": False}
|
||
# eth_wr_ready stall pattern. Held constantly HIGH by every test this
|
||
# file ever had, which is a serious blind spot: on hardware the SPRAM
|
||
# arbiter stalls the write port constantly, because the GC is reading
|
||
# the ring over EXI while frames are being written into it. Any
|
||
# address bookkeeping that depends on ready timing is invisible until
|
||
# ready actually goes low. stall[0] = assert ready 1 cycle in N.
|
||
stall = [1]
|
||
cyc = [0]
|
||
|
||
def apply_ready():
|
||
"""Apply eth_wr_ready for the cycle about to be clocked. It must
|
||
be applied BEFORE rx_ready is read: rx_ready is combinational on
|
||
it, so reading first and applying second puts the harness a cycle
|
||
out of phase with the accept the DUT actually makes."""
|
||
cyc[0] += 1
|
||
ctx.set(dut.eth_wr_ready, 1 if cyc[0] % stall[0] == 0 else 0)
|
||
|
||
async def tick_only(n=1):
|
||
for _ in range(n):
|
||
# A write is issued on a cycle where valid AND ready are both
|
||
# high, so sample before the edge that completes it.
|
||
if ctx.get(dut.eth_wr_valid) and ctx.get(dut.eth_wr_ready):
|
||
a, d = ctx.get(dut.eth_wr_addr), ctx.get(dut.eth_wr_data)
|
||
mem[a] = d
|
||
order.append((a, d))
|
||
await ctx.tick("sync")
|
||
if ctx.get(dut.rx_overflow):
|
||
pulse["ovf"] = True
|
||
if ctx.get(dut.rx_irq):
|
||
pulse["irq"] = ctx.get(dut.rx_wptr_w_data)
|
||
|
||
async def tick(n=1):
|
||
for _ in range(n):
|
||
apply_ready()
|
||
await tick_only()
|
||
|
||
tag = ["?"]
|
||
|
||
async def send_frame(frame):
|
||
pulse["irq"] = None
|
||
pulse["ovf"] = False
|
||
"""Push a frame RESPECTING BACK-PRESSURE, the way the real
|
||
ethernet master does. rx_ready is LOW through IDLE and the whole
|
||
placeholder-descriptor write, so a master that advances anyway
|
||
simply loses those bytes -- which is what the old testbench did,
|
||
and why it could not see the frame body land at the wrong
|
||
offset."""
|
||
for i, b in enumerate(frame):
|
||
ctx.set(dut.rx_data, b)
|
||
ctx.set(dut.rx_valid, 1)
|
||
ctx.set(dut.rx_sof, 1 if i == 0 else 0)
|
||
ctx.set(dut.rx_eof, 1 if i == len(frame) - 1 else 0)
|
||
# Apply ready, THEN read rx_ready, THEN clock -- so the
|
||
# value the DUT decides on is the one this loop checked.
|
||
# Polling before applying had the loop waiting on a byte the
|
||
# DUT had already accepted: the frame stored correctly while
|
||
# send_frame reported rx_ready never asserted.
|
||
guard = 0
|
||
while True:
|
||
apply_ready()
|
||
if ctx.get(dut.rx_ready):
|
||
break
|
||
await tick_only()
|
||
guard += 1
|
||
if guard > 5000:
|
||
errors.append(f"send_frame[{tag[0]}] byte {i}: "
|
||
f"rx_ready never asserted")
|
||
return
|
||
await tick_only()
|
||
ctx.set(dut.rx_valid, 0)
|
||
ctx.set(dut.rx_sof, 0)
|
||
ctx.set(dut.rx_eof, 0)
|
||
|
||
async def wait_irq(limit=200):
|
||
for _ in range(limit):
|
||
if pulse["irq"] is not None:
|
||
return pulse["irq"]
|
||
await tick()
|
||
return None
|
||
|
||
async def wait_done(limit=400):
|
||
"""Wait for a frame to be either stored (rx_irq) or discarded
|
||
(rx_overflow), reading the latches so a pulse that fired during
|
||
send_frame is not lost."""
|
||
for _ in range(limit):
|
||
if pulse["ovf"]:
|
||
return "dropped"
|
||
if pulse["irq"] is not None:
|
||
return "stored"
|
||
await tick()
|
||
return "timeout"
|
||
|
||
def descriptor(page):
|
||
base = page << 8
|
||
return [mem.get(base + i) for i in range(4)]
|
||
|
||
def pack_desc(total, next_ptr, status=0):
|
||
"""The 4 descriptor bytes libogc's driver expects."""
|
||
word = (status << 24) | ((total & 0xFFF) << 12) | (next_ptr & 0xFFF)
|
||
return [(word >> (8 * i)) & 0xFF for i in range(4)]
|
||
|
||
def libogc_decode(d):
|
||
"""Decode exactly as libogc's bba_interrupt does, so these tests
|
||
check the driver's ALGORITHM rather than our own packing:
|
||
|
||
lwbrx -> little-endian word from the 4 bytes
|
||
rlwinm r4,r9,20,20,31 -> packet_len = (word >> 12) & 0xFFF
|
||
srwi r9,r9,24 -> status = word >> 24
|
||
word & 0xFFF -> next_packet_ptr
|
||
|
||
Returns (payload_len, next_ptr, status_is_error)."""
|
||
if any(b is None for b in d):
|
||
return (None, None, None)
|
||
word = d[0] | (d[1] << 8) | (d[2] << 16) | (d[3] << 24)
|
||
return (((word >> 12) & 0xFFF) - 4, word & 0xFFF,
|
||
bool((word >> 24) & 0x84))
|
||
|
||
def body(page, n):
|
||
base = (page << 8) + 4
|
||
return [mem.get(base + i) for i in range(n)]
|
||
|
||
ctx.set(dut.rx_wptr_w_rdy, 1)
|
||
ctx.set(dut.rx_enabled, 1)
|
||
await tick(2)
|
||
|
||
# ── T1: short frame at page 1 ────────────────────────────────────
|
||
f1 = [0xAA, 0xBB, 0xCC, 0xDD, 0x08, 0x00, 0x45, 0x00, 0x00, 0x01]
|
||
tag[0] = "T1"
|
||
await send_frame(f1)
|
||
check("T1 rwp advance", await wait_irq(), 2)
|
||
total1 = len(f1) + 4
|
||
check("T1 descriptor", descriptor(1), pack_desc(total1, 2))
|
||
check("T1 descriptor via libogc decode", libogc_decode(descriptor(1)),
|
||
(len(f1), 2, False))
|
||
# The body MUST start at base+4. This is the check that catches a
|
||
# placeholder write being dropped: with only three placeholder bytes
|
||
# written the body landed at base+3 and every frame was shifted one
|
||
# byte early, which on hardware made the GC drop every frame.
|
||
check("T1 body at base+4", body(1, len(f1)), f1)
|
||
|
||
await tick(4)
|
||
|
||
# ── T2: frame LONGER THAN 255 BYTES, so the descriptor's HIGH length
|
||
# byte is non-zero. This is the case that catches a dropped
|
||
# WRITE_LEN_HI: with the high byte left at its placeholder 0x00 the GC
|
||
# read an impossible length (hardware saw 00 00 00 01) and rejected
|
||
# the ring. Every earlier test used a short frame, where a dropped
|
||
# high byte is indistinguishable from a correct zero. ───────────────
|
||
f2 = [(i * 7 + 3) & 0xFF for i in range(300)]
|
||
tag[0] = "T2"
|
||
await send_frame(f2)
|
||
rwp2 = await wait_irq()
|
||
total2 = len(f2) + 4
|
||
check("T2 descriptor (len > 255)", descriptor(2), pack_desc(total2, 4))
|
||
check("T2 descriptor via libogc decode", libogc_decode(descriptor(2)),
|
||
(len(f2), 4, False))
|
||
check("T2 body at base+4", body(2, len(f2)), f2)
|
||
# 304 bytes spans two pages, so page 2 -> page 4.
|
||
check("T2 rwp advance (2 pages)", rwp2, 4)
|
||
|
||
await tick(4)
|
||
|
||
# ── T4: ring overflow must DROP, not overwrite ───────────────────
|
||
# rrp is held still, as if the GC had stopped reading. The writer
|
||
# must refuse new frames once the ring is nearly full and leave the
|
||
# unread ones intact. Before this existed the writer simply lapped
|
||
# the reader, so a traffic burst corrupted whatever frame the driver
|
||
# was midway through -- with no indication it had happened. That is
|
||
# what produced a "descriptor" of 00 00 00 01 on hardware: frame data
|
||
# from the frame that had replaced the one being read.
|
||
ctx.set(dut.rrp, 4) # GC has consumed up to page 4
|
||
await tick(2)
|
||
f3 = [(i * 3 + 1) & 0xFF for i in range(300)] # 2 pages each
|
||
tag[0] = "T4-first"
|
||
await send_frame(f3)
|
||
check("T4 first frame stored", await wait_done(), "stored")
|
||
page4 = descriptor(4) + body(4, 32)
|
||
|
||
outcomes = []
|
||
for _ in range(12):
|
||
tag[0] = "T4-fill"
|
||
await send_frame(f3)
|
||
outcomes.append(await wait_done())
|
||
if outcomes[-1] == "dropped":
|
||
break
|
||
check("T4 ring eventually refuses a frame",
|
||
outcomes[-1] if outcomes else "none", "dropped")
|
||
# THE property: the frame the GC has not read yet is untouched.
|
||
check("T4 unread page preserved", descriptor(4) + body(4, 32), page4)
|
||
|
||
# Once the GC catches up, frames must be accepted again -- a full ring
|
||
# must not latch the receiver off. The writer is at page 14 here, so
|
||
# rrp must move well past it to free the 6 pages a worst-case frame
|
||
# needs: free = (rrp - rwp - 1) mod 15, so rrp = 8 gives 8 free.
|
||
ctx.set(dut.rrp, 8)
|
||
await tick(4)
|
||
tag[0] = "T4-again"
|
||
await send_frame(f3)
|
||
check("T4 accepts again after RRP advances", await wait_done(), "stored")
|
||
|
||
await tick(4)
|
||
|
||
# ── T5: the same frame WITH THE WRITE PORT STALLING ──────────────
|
||
# Models arbiter contention: ready asserted only 1 cycle in 3. The
|
||
# descriptor and body offset must be identical to the unstalled case.
|
||
# This is the case hardware runs in and no previous test ever did --
|
||
# hardware reported a frame whose body was one byte late while
|
||
# simulation with ready held high was byte-perfect.
|
||
ctx.set(dut.rrp, 1)
|
||
await tick(2)
|
||
stall[0] = 3
|
||
f5 = [(i * 5 + 9) & 0xFF for i in range(300)]
|
||
tag[0] = "T5"
|
||
await send_frame(f5)
|
||
rwp5 = await wait_done()
|
||
check("T5 stored despite stalls", rwp5, "stored")
|
||
pg5 = (ctx.get(dut.rx_wptr_w_data) - 2) & 0xFF # page it was written to
|
||
total5 = len(f5) + 4
|
||
next5 = ctx.get(dut.rx_wptr_w_data)
|
||
check("T5 descriptor under stall", descriptor(pg5),
|
||
pack_desc(total5, next5))
|
||
check("T5 descriptor via libogc decode", libogc_decode(descriptor(pg5)),
|
||
(len(f5), next5, False))
|
||
check("T5 body at base+4 under stall", body(pg5, len(f5)), f5)
|
||
stall[0] = 1
|
||
await tick(4)
|
||
|
||
# ── T6: NCRA reset must return the WRITE pointer to page 1 ───────
|
||
# The register file resets its own copy of RWP on an NCRA reset; if
|
||
# this module does not reset its own, the two disagree and the GC ends
|
||
# up pointing somewhere the writer is not. The free-space sum then
|
||
# shrinks with every frame until every frame is dropped -- silently,
|
||
# which on hardware read as "0 seen, 0 bad desc, 0 lapped".
|
||
ctx.set(dut.rrp, 1)
|
||
await tick(2)
|
||
ctx.set(dut.ncra_rst, 1)
|
||
await tick(1)
|
||
ctx.set(dut.ncra_rst, 0)
|
||
await tick(2)
|
||
f6 = [0xC0 | (i & 0x0F) for i in range(40)]
|
||
await send_frame(f6)
|
||
check("T6 rwp back to page 1 after NCRA reset", await wait_irq(), 2)
|
||
total6 = len(f6) + 4
|
||
check("T6 frame lands at page 1", descriptor(1), pack_desc(total6, 2))
|
||
check("T6 descriptor via libogc decode", libogc_decode(descriptor(1)),
|
||
(len(f6), 2, False))
|
||
check("T6 body at page 1 base+4", body(1, len(f6)), f6)
|
||
|
||
await tick(4)
|
||
|
||
# ── T3: RX disabled — SOF must be ignored ────────────────────────
|
||
before = len(mem)
|
||
ctx.set(dut.rx_enabled, 0)
|
||
ctx.set(dut.rx_data, 0xDE)
|
||
ctx.set(dut.rx_valid, 1)
|
||
ctx.set(dut.rx_sof, 1)
|
||
await tick(8)
|
||
ctx.set(dut.rx_valid, 0)
|
||
ctx.set(dut.rx_sof, 0)
|
||
check("T3 no writes while rx disabled", len(mem), before)
|
||
|
||
# ── T7: the GC's BP/RHBP are HONOURED (compat TODO item 2) ───────
|
||
# These were hardcoded to pages 1-15, so a driver configuring a
|
||
# SMALLER ring was ignored and we kept writing pages it would never
|
||
# read -- silent corruption. libogc2 writes BP=1/RHBP=15, exactly the
|
||
# old hardcode, which is why it never showed up.
|
||
ctx.set(dut.rx_enabled, 1)
|
||
ctx.set(dut.rrp, 1)
|
||
ctx.set(dut.bp, 1)
|
||
ctx.set(dut.rhbp, 8) # an 8-page ring: pages 1..8
|
||
await tick(4)
|
||
check("T7 small ring honoured", ctx.get(dut.geom_honoured), 1)
|
||
|
||
# ⚠️ The ring must be LARGER than _MAX_FRAME_PAGES or nothing is ever
|
||
# stored: `free` can never reach the worst-case reservation, so RBFI
|
||
# drops every frame. A 4-page ring was tried here first and stored
|
||
# NOTHING -- the test passed vacuously. 8 pages is the smallest that
|
||
# both exercises the wrap and leaves room.
|
||
pages_seen = set()
|
||
for n in range(10):
|
||
mem.clear()
|
||
# Keep the reader moving so the ring never fills.
|
||
ctx.set(dut.rrp, (n % 8) + 1)
|
||
await tick(4)
|
||
await send_frame([0x90 + n] * 32)
|
||
if await wait_done() == "stored":
|
||
pages_seen |= {a >> 8 for a in mem}
|
||
await tick(4)
|
||
print(f" T7 pages written with RHBP=8: {sorted(pages_seen)}")
|
||
check("T7 stored at least one frame in the small ring",
|
||
len(pages_seen) > 0, True)
|
||
outside = {p for p in pages_seen if p < 1 or p > 8}
|
||
check("T7 never writes outside pages 1-8", sorted(outside), [])
|
||
|
||
# ── T7b: garbage geometry is REJECTED, not obeyed ────────────────
|
||
# Without the sanity check, BP=0 (page 0 is the TX buffer) or an RHBP
|
||
# below BP gives a zero or negative ring size and the assembler writes
|
||
# outside it.
|
||
for bp, rhbp, why in ((0, 15, "BP=0 is the TX buffer"),
|
||
(8, 4, "RHBP below BP"),
|
||
(1, 0x20, "RHBP beyond SPRAM ring")):
|
||
ctx.set(dut.bp, bp)
|
||
ctx.set(dut.rhbp, rhbp)
|
||
await tick(4)
|
||
if ctx.get(dut.geom_honoured):
|
||
check(f"T7b reject {why}", "honoured", "rejected")
|
||
check("T7b garbage geometry rejected", ctx.get(dut.geom_honoured), 0)
|
||
|
||
# ... and with it rejected, the hardcoded layout still works.
|
||
ctx.set(dut.rrp, 1)
|
||
await tick(4)
|
||
mem.clear()
|
||
await send_frame([0x5A] * 32)
|
||
check("T7b still stores with fallback layout", await wait_done(),
|
||
"stored")
|
||
fb = {a >> 8 for a in mem}
|
||
print(f" T7b fallback pages: {sorted(fb)}")
|
||
check("T7b fallback wrote something", len(fb) > 0, True)
|
||
check("T7b fallback writes inside pages 1-15",
|
||
sorted(p for p in fb if p < 1 or p > 15), [])
|
||
ctx.set(dut.bp, 1)
|
||
ctx.set(dut.rhbp, 15)
|
||
|
||
sim = Simulator(dut)
|
||
sim.add_clock(Period(MHz=24), domain="sync")
|
||
sim.add_testbench(testbench)
|
||
|
||
with sim.write_vcd("RXFrameAssembler.vcd"):
|
||
sim.run()
|
||
|
||
if errors:
|
||
print("\nFAILURES:")
|
||
for e in errors:
|
||
print(" -", e)
|
||
sys.exit(1)
|
||
print("\nAll tests passed.")
|