verify: cocotb datapath suite + 13 RTL fixes across gmii_cdc / eth_mac_rx / mii_tx_saf

.github/workflows/sim.yml

@@ -1,7 +1,7 @@
 # SPDX-License-Identifier: Apache-2.0
 # Copyright (c) 2026 Leonardo Capossio - bard0 design
 #
-# Lint + simulation regression. Mirrors `python build_and_test.py`.
+# Lint + simulation + cocotb regression. Mirrors `python build_and_test.py`.
 
 name: sim
 
@@ -32,5 +32,12 @@ jobs:
         with:
           python-version: '3.11'
 
-      - name: Run lint + simulation
+      - name: Install cocotb
+        # Icarus (installed above) is cocotb's simulator backend. Without this
+        # the cocotb phase (PHASE 2) would skip silently instead of running.
+        run: |
+          pip install "cocotb>=2.0"
+          python -c "import cocotb; print('cocotb', cocotb.__version__)"
+
+      - name: Run lint + simulation + cocotb
         run: python build_and_test.py

.gitignore

@@ -1,8 +1,13 @@
 # Sim artifacts
 *.vvp
 *.vcd
+*.fst
 a.out
 
+# cocotb
+sim/cocotb/**/sim_build/
+sim/cocotb/**/results*.xml
+
 # Vivado / synthesis
 *.log
 *.jou

CHANGELOG.md

@@ -77,6 +77,70 @@ This project does not yet maintain long-lived release branches.
 
 ### Fixed
 
+- `eth_mac_rx` framing survives RX-FIFO overflow: the readout reserves headroom
+  so a frame's SOF and closing TLAST words are never the ones dropped when the
+  2 KB FIFO fills. Overrun data is dropped and the frame is flagged `terror`, but
+  it always starts and terminates cleanly - a dropped SOF used to leave the sink
+  unable to delimit and a dropped TLAST merged the frame into the next. New
+  `eth_mac_rx_robust` suite (`overflow_framing`).
+- `eth_mac_rx` runt handling: a frame shorter than 64 wire bytes is now delivered
+  with `terror` (undersize) instead of as a clean frame with a garbage FCS, so
+  the wrapper's error-drop stage discards it. Regression `runt_terror`.
+- `eth_mac_rx` `byte_cnt` no longer wraps: the 14-bit counter saturates at
+  0x3FFF, so a frame past 16383 wire bytes cannot re-enter the `byte_cnt==5`
+  decision, re-capture the dst MAC and inject a phantom SOF that corrupts the
+  following frame. Regression `bytecnt_no_wrap`.
+- `eth_mac_rx` reports `rx_er` asserted on a preamble/SFD byte (was only sampled
+  in `S_DATA`), so such a frame carries `terror` + `stat_err_align`. Regression
+  `preamble_rx_er`.
+- `mii_tx_saf` now fails elaboration if `MAX_FRAME >= FIFO_DEPTH` (an `initial`
+  `$finish`): the oversize cap relies on that invariant, and violating it silently
+  reintroduces the permanent TX wedge.
+- `eth_mac_sys` sizes the MII `mii_tx_saf` frame FIFO from `MAX_FRAME`
+  (`$clog2`-derived) instead of a fixed 4096 entries. The old fixed size wedged
+  the MII TX path on frames between 4096 and `MAX_FRAME` (9018) bytes; the FIFO
+  now holds one whole frame, and a standard build (`MAX_FRAME=1518`) pays only for
+  a 2048-deep FIFO - the jumbo cost is incurred only when jumbo is built.
+- `eth_mac_rx` multicast-hash filter gated on the wrong bit: the hash-admit term
+  tested `mac_chk[0]` (LSB of the last dst octet) instead of `mac_chk[40]` (the
+  I/G bit, dst byte 0 LSB) - so with `MCAST_HASH_FILTER=1` a genuine group address
+  whose last octet was even was rejected, and a unicast with an odd last octet and
+  a colliding hash bucket was admitted. Both admit sites now use `mac_chk[40]`,
+  matching the neighboring `is_mcast_r`. Regression suite `eth_mac_rx_mcast`.
+- `gmii_cdc` TX error input was dropped: `gmii_tx_er_in` was never captured and
+  `gmii_tx_er_out` was hard-wired 0, so a MAC-signalled transmit error never
+  reached the media side (the RX path already carried `rx_er`). The TX FIFO word
+  gained a per-byte error lane (9 -> 10 bits) that re-drives `gmii_tx_er_out`.
+  Regression `test_gmii_cdc.tx_error_flag`.
+- `gmii_cdc` paced-TX held the final (EOF) byte for only 1 media cycle instead of
+  the full pace interval at 100M/10M, so a paced downstream could mis-sample the
+  last byte. The frame now closes out at the next `pace_tick`, giving the last
+  byte its full `period`. Regression `test_gmii_cdc.paced_last_byte_hold_100m`.
+- `gmii_cdc` RX multi-frame byte-drop: the sys-side readout returned to idle at
+  each frame's EOF and re-ran its "align" pre-consume on the next frame - correct
+  on a cold start out of empty, but on a frame boundary (next frame already
+  buffered, first-word-fall-through FIFO) it consumed and dropped that frame's
+  first byte. Every frame after the first lost byte 0. The readout now stays in
+  its reading state across the EOF marker so the following frame's first byte is
+  taken by the normal data path. Regression `test_gmii_cdc.rx_directed`.
+- `gmii_cdc` RX committed-frame-counter wrap (same class as the TX/`mii_tx_saf`
+  fixes): a 4-bit `rx_frames_pending` counter aliased once 16 frames buffered, so
+  under a slow sys drain small frames piled up past 16 long before the 4K RX FIFO
+  filled - `rx_frame_ready` read false and the readout stalled. Widened to
+  `ADDR_WIDTH+1` (13) bits. Regression `test_gmii_cdc.rx_burst_wrap_probe`.
+- `gmii_cdc` paced-TX phantom-frame stall: in 100M/10M the read pointer was left
+  parked on a frame's EOF word (its per-byte prefetch is suppressed on EOF), so
+  the next paced frame emitted that stale EOF byte, ended after one cycle, and
+  orphaned the real frame in the FIFO - dropping every frame after the first.
+  The frame close-out now advances the read pointer past the EOF word in the
+  paced modes only (1G's every-cycle prefetch already realigns it). Regression
+  `test_gmii_cdc.directed_100m` / `directed_10m`.
+- `gmii_cdc` paced-TX committed-counter wrap (same class as the `mii_tx_saf` fix
+  below): a 4-bit committed-frame counter aliased once 16 whole frames backed up
+  in the 16 KB TX FIFO, deasserting the store-and-forward start gate and wedging
+  the media side long before the FIFO filled. Widened the counter and its gray
+  CDC to `ADDR_WIDTH+1` (15) bits so the byte FIFO fills first. Regression
+  `test_gmii_cdc.burst_small_frames_100m` (20 small frames).
 - `mii_tx_saf` TX deadlock (committed-frame counter wrap): a 4-bit committed
   frame counter aliased to a false "equal" once 16 frames backed up in the 4 KB
   FIFO, parking the framer in idle. Widened the counter to `FIFO_ADDR_WIDTH+1`

build_and_test.py

@@ -683,6 +683,39 @@ def run_simulation():
     return all_pass
 
 
+def run_cocotb():
+    header("PHASE 2: cocotb (directed + randomized)")
+    try:
+        import cocotb  # noqa: F401
+    except Exception:
+        print(f"  {C.YELLOW}SKIP{C.END} cocotb not installed "
+              "(pip install 'cocotb>=2.0') - randomized suite not run")
+        return True
+
+    runner = os.path.join(PROJECT_DIR, "sim", "cocotb", "run.py")
+    if not os.path.exists(runner):
+        fail("cocotb runner missing")
+        return False
+
+    rc, stdout, stderr = run_cmd(
+        f'"{sys.executable}" "{runner}"', cwd=PROJECT_DIR, timeout=1800
+    )
+    combined = stdout + stderr
+    for line in combined.splitlines():
+        if re.search(r"TESTS=\d", line):
+            print(f"    {line.strip()}")
+
+    if rc == 0:
+        ok("cocotb suite (mii_tx_saf: directed + randomized)")
+        return True
+
+    fail(f"cocotb suite (mii_tx_saf) rc={rc}")
+    for line in combined.splitlines():
+        if "failed" in line.lower() or "random seed =" in line.lower():
+            print(f"    {line.strip()[:160]}")  # seeds shown for deterministic replay
+    return False
+
+
 def main():
     parser = argparse.ArgumentParser(description="emacZero — Build & Test")
     parser.add_argument("--sim-only", action="store_true", help="Run simulation only")
@@ -698,8 +731,9 @@ def main():
     lint_ok = run_lint()
     verilator_ok = run_verilator_lint()
     sim_ok = run_simulation()
+    cocotb_ok = run_cocotb()
 
-    if version_ok and lint_ok and verilator_ok and sim_ok:
+    if version_ok and lint_ok and verilator_ok and sim_ok and cocotb_ok:
         print(f"\n{C.GREEN}{C.BOLD}All tests passed.{C.END}")
         sys.exit(0)
     else:

rtl/eth_mac_rx.v

@@ -85,6 +85,7 @@ module eth_mac_rx #(
 
     reg [47:0] dst_mac_captured;
     reg        mac_ok;
+    reg        frame_started;    // this frame's SOF word made it into the FIFO
     reg        rx_er_seen;
     reg        rx_overflow_seen;
     reg        is_bcast_r;
@@ -103,7 +104,7 @@ module eth_mac_rx #(
                          (mac_chk == 48'hFFFFFFFFFFFF) ||
                          promisc || passthrough ||
                          (MCAST_HASH_FILTER &&
-                          mac_chk[0] &&
+                          mac_chk[40] &&
                           mac_chk != 48'hFFFFFFFFFFFF &&
                           mcast_hash_table[mcast_hash_idx]);
 
@@ -115,6 +116,7 @@ module eth_mac_rx #(
     reg       push_last;
     reg       push_err;
     reg       push_sof;
+    reg       data_drop;        // a data byte was dropped for want of FIFO room
     reg       push_en_r;
     reg [7:0] push_data_r;
     reg       push_last_r;
@@ -125,6 +127,16 @@ module eth_mac_rx #(
     wire        fifo_overflow;
     wire [10:0] fifo_rd_data;
     wire        fifo_rd_valid;
+    wire [AXIS_FIFO_ADDR_WIDTH:0] fifo_count;
+
+    // Reserve a few slots so a frame's SOF and closing TLAST words are never the
+    // ones dropped on overflow: once occupancy passes the high-water mark we stop
+    // pushing DATA (dropped bytes just set the overflow/terror flag), but the SOF
+    // that starts a frame and the TLAST that ends it always find room. That keeps
+    // AXIS framing intact under backpressure - a dropped SOF would leave the sink
+    // unable to delimit, and a dropped TLAST would merge this frame into the next.
+    localparam [AXIS_FIFO_ADDR_WIDTH:0] FIFO_HWM = AXIS_FIFO_DEPTH - 4;
+    wire fifo_room = (fifo_count < FIFO_HWM);
 
     sync_fifo #(
         .DATA_WIDTH (11),
@@ -139,7 +151,7 @@ module eth_mac_rx #(
         .rd_valid    (fifo_rd_valid),
         .rd_en       (m_axis_tready),
         .rd_empty    (),
-        .count       (),
+        .count       (fifo_count),
         .wr_overflow (fifo_overflow)
     );
 
@@ -166,6 +178,11 @@ module eth_mac_rx #(
     wire err_overflow_now = rx_overflow_seen;
     wire err_oversize_now = (!jumbo_en && (byte_cnt > MAX_FRAME_STD)) ||
                             (jumbo_en  && (byte_cnt > MAX_FRAME_JUMBO));
+    // Runt: a valid 802.3 frame is >= 64 wire bytes (60 data/pad + 4 FCS).
+    // byte_cnt counts bytes after the SFD, so < 64 is undersized - a collision
+    // fragment or truncated frame. Deliver it with terror instead of as a clean
+    // frame with a garbage FCS, so the wrapper's error-drop stage discards it.
+    wire err_undersize_now = (byte_cnt < 14'd64);
 
     // Combinational push request from the receive FSM. This is registered
     // before sync_fifo so MAC filtering does not directly drive the FIFO CE
@@ -176,23 +193,41 @@ module eth_mac_rx #(
         push_last = 1'b0;
         push_err  = 1'b0;
         push_sof  = 1'b0;
+        data_drop = 1'b0;
         case (state)
             S_DATA: begin
-                if (gmii_rx_dv && (
-                        (byte_cnt == 14'd5 && mac_pass_now) ||
-                        (byte_cnt >= 14'd6 && mac_ok))) begin
-                    push_en   = 1'b1;
-                    push_data = delay_pipe1;
-                    push_sof  = (byte_cnt == 14'd5);
+                if (gmii_rx_dv && byte_cnt == 14'd5 && mac_pass_now) begin
+                    // SOF: start the frame only if it can be buffered. If not,
+                    // frame_started stays 0 and the whole frame is cleanly
+                    // dropped (no partial, so the sink's delimiting is unharmed).
+                    if (fifo_room) begin
+                        push_en   = 1'b1;
+                        push_data = delay_pipe1;
+                        push_sof  = 1'b1;
+                    end
+                end else if (gmii_rx_dv && byte_cnt >= 14'd6 &&
+                             mac_ok && frame_started) begin
+                    // Data byte: push while there is headroom; otherwise drop it
+                    // and flag overflow so the frame is terror'd (its reserved
+                    // SOF/TLAST still bound it correctly).
+                    if (fifo_room) begin
+                        push_en   = 1'b1;
+                        push_data = delay_pipe1;
+                    end else begin
+                        data_drop = 1'b1;
+                    end
                 end
             end
             S_CRC_CHECK: begin
-                if (byte_cnt >= 14'd6 && mac_ok) begin
+                // Always emit the closing word for a started frame - the reserved
+                // headroom guarantees room, so the frame is always terminated.
+                if (byte_cnt >= 14'd6 && mac_ok && frame_started) begin
                     push_en   = 1'b1;
                     push_data = delay_pipe1;
                     push_last = 1'b1;
                     push_err  = err_fcs_now || err_align_now ||
-                                err_overflow_now || err_oversize_now;
+                                err_overflow_now || err_oversize_now ||
+                                err_undersize_now;
                 end
             end
             default: ;
@@ -215,6 +250,7 @@ module eth_mac_rx #(
             delay_pipe5      <= 8'd0;
             dst_mac_captured <= 48'd0;
             mac_ok           <= 1'b0;
+            frame_started    <= 1'b0;
             rx_er_seen       <= 1'b0;
             rx_overflow_seen <= 1'b0;
             is_bcast_r       <= 1'b0;
@@ -242,6 +278,12 @@ module eth_mac_rx #(
             push_err_r        <= push_err;
             push_sof_r        <= push_sof;
 
+            // The frame is "started" once its SOF word is committed to the FIFO;
+            // gates data/TLAST pushes and stat_done so a frame whose SOF could not
+            // be buffered is dropped whole (no partial, no phantom stat).
+            if (push_sof)
+                frame_started <= 1'b1;
+
             case (state)
                 S_IDLE: begin
                     byte_cnt         <= 14'd0;
@@ -254,6 +296,7 @@ module eth_mac_rx #(
                     delay_pipe5      <= 8'd0;
                     dst_mac_captured <= 48'd0;
                     mac_ok           <= 1'b0;
+                    frame_started    <= 1'b0;
                     rx_er_seen       <= 1'b0;
                     rx_overflow_seen <= 1'b0;
                     is_bcast_r       <= 1'b0;
@@ -263,6 +306,11 @@ module eth_mac_rx #(
                 end
 
                 S_PREAMBLE: begin
+                    // A carrier/coding error on a preamble or SFD byte is a valid
+                    // 802.3 error indication; latch it so the frame is delivered
+                    // with terror + stat_err_align, not silently clean.
+                    if (gmii_rx_er)
+                        rx_er_seen <= 1'b1;
                     if (!gmii_rx_dv) begin
                         state <= S_IDLE;
                     end else if (gmii_rxd == 8'hD5) begin
@@ -292,7 +340,7 @@ module eth_mac_rx #(
                                 mac_chk == 48'hFFFFFFFFFFFF ||
                                 promisc || passthrough ||
                                 (MCAST_HASH_FILTER &&
-                                 mac_chk[0] &&
+                                 mac_chk[40] &&
                                  mac_chk != 48'hFFFFFFFFFFFF &&
                                  mcast_hash_table[mcast_hash_idx]))
                                 mac_ok <= 1'b1;
@@ -313,7 +361,14 @@ module eth_mac_rx #(
                         delay_pipe1 <= delay_pipe2;
                         delay_pipe0 <= delay_pipe1;
 
-                        byte_cnt <= byte_cnt + 14'd1;
+                        // Saturate instead of wrapping. byte_cnt gates dst
+                        // capture (<6), the MAC decision (==5) and oversize
+                        // (>MAX_FRAME); a 14-bit wrap at 16384 would re-enter
+                        // byte_cnt==5 mid-frame, re-capturing the dst and firing a
+                        // second SOF. Freezing at 0x3FFF keeps oversize asserted
+                        // (>= jumbo) and cannot re-trigger those decisions.
+                        if (byte_cnt != 14'h3FFF)
+                            byte_cnt <= byte_cnt + 14'd1;
                         if (first_byte)
                             first_byte <= 1'b0;
                     end
@@ -324,7 +379,7 @@ module eth_mac_rx #(
                     // End-of-frame classification pulse for stats.
                     // Only emit when MAC filter passed (i.e. frame was actually
                     // delivered to the AXIS sink), so counts match deliveries.
-                    if (mac_ok) begin
+                    if (mac_ok && frame_started) begin
                         stat_done         <= 1'b1;
                         stat_len          <= byte_cnt;
                         stat_err_fcs      <= err_fcs_now;
@@ -344,8 +399,10 @@ module eth_mac_rx #(
                 default: state <= S_IDLE;
             endcase
 
-            // Sticky overflow flag, latched on any dropped FIFO write.
-            if (fifo_overflow)
+            // Sticky overflow flag: latched on a data byte dropped for want of
+            // headroom (the normal path now - SOF/TLAST are reserved), or on any
+            // raw FIFO overflow as a backstop.
+            if (data_drop || fifo_overflow)
                 rx_overflow_seen <= 1'b1;
         end
     end

rtl/eth_mac_sys.v

@@ -436,9 +436,15 @@ module eth_mac_sys #(
 
             // Store-and-forward MII transmit: single frame FIFO + media-clk
             // framer. Drives the MII TX pins and the shared TX status/stats.
+            // Size the frame FIFO from MAX_FRAME so it always holds one whole
+            // frame (mii_tx_saf's oversize cap requires FIFO_DEPTH > MAX_FRAME,
+            // else a MAX_FRAME-sized frame wedges the TX path). A standard build
+            // (MAX_FRAME=1518) gets a 2048-deep FIFO; a jumbo build (9018) gets
+            // 16384 - the jumbo cost is paid only when jumbo is actually built.
             mii_tx_saf #(
                 .MAX_FRAME      (MAX_FRAME),
-                .FIFO_ADDR_WIDTH(12)
+                .FIFO_ADDR_WIDTH(($clog2(MAX_FRAME + 1) > 11)
+                                 ? $clog2(MAX_FRAME + 1) : 11)
             ) u_mii_tx (
                 .clk           (clk),
                 .rst_n         (rst_n),