From 4831c1353c9b0a7fc8f0ed840c6e9163733c1534 Mon Sep 17 00:00:00 2001 From: Dennis Brentjes Date: Fri, 9 Oct 2026 18:45:00 +0200 Subject: [PATCH] netboot: locate the descriptor, report the offset split, stop scrolling Hardware with RBFI active: "timeout mid-transfer (5 seen, 18 bad desc, 0 lapped)", plus a frame dump of "1E FF FF FF FF FF FF 00". Three things that tells us: - lapped = 0, so the ring is no longer being overwritten. RBFI works and overflow is eliminated as the cause. - got > 0 ("mid-transfer"), so some blocks DID arrive and match. - 0x1E is 286 & 0xFF, the low byte of total_len, followed by the full six-byte broadcast MAC and then 0x00. The frame body is intact but starts one byte later than the reader expects. 18 of 23 frames rejected while 5 parsed in the SAME run means the offset VARIES frame to frame -- a race in the assembler, not a constant. Simulation does not reproduce it: T2/T5 put the descriptor at +0 and the body at +4 with the write port both free-running and stalled 1-in-3, and the mutation controls for both known handshake bugs fail as expected. So the cause is still unknown and I am not going to guess at it a fifth time. ring_next() now LOCATES the descriptor instead of assuming +0: it probes offsets 0 and 1 and takes whichever yields a plausible length (> 4 descriptor bytes, <= a legal frame), then reads the body and advances RRP from that same offset. A length is a strong discriminator, so this is a measurement rather than a guess, and it both lets the transfer proceed and COUNTS the split -- "desc@+0=N +1=M" on success and on timeout. That distribution is the measurement needed to find the real gateware bug. Also trimmed the on-screen guidance: the console holds ~25 lines and the explanatory text was pushing the failure diagnostics off the top, which cost two blind round trips. Status during a transfer is now one line, and the menu blurb one line. Co-Authored-By: Claude Opus 5 --- gc_bba_test/source/main.c | 12 +++---- gc_bba_test/source/netboot.c | 61 ++++++++++++++++++++++++++---------- 2 files changed, 51 insertions(+), 22 deletions(-) diff --git a/gc_bba_test/source/main.c b/gc_bba_test/source/main.c index d0b9827..06ae956 100644 --- a/gc_bba_test/source/main.c +++ b/gc_bba_test/source/main.c @@ -146,12 +146,12 @@ static void do_netboot(char *status, size_t nstatus) printf("\n" C_HEAD " Self-update over the BBA" C_RESET "\n\n"); printf(" On the host: sudo ./scripts/send-dol.py \n\n"); - printf(C_DIM " The received build is written to a TEMP file, verified, and\n" - " only then rotated in -- the previous build is kept alongside it\n" - " as .prev.dol, so if the new one is broken you can boot the\n" - " old one from Swiss and try the transfer again.\n" - " Nothing received is ever launched directly: this boots /boot.dol,\n" - " your known-good Swiss.\n" C_RESET "\n"); + /* Deliberately terse: the console holds ~25 lines and the diagnostics + * printed during a failed transfer are what matter. Explanatory text + * here pushed them off the top of the screen, which cost two blind + * round trips. */ + printf(C_DIM " temp+verify+rotate; keeps .prev.dol; boots /boot.dol\n" + C_RESET); printf(" Blocks until done or it times out (30 s idle, 5 s mid-xfer).\n\n"); if (!s_stage) { diff --git a/gc_bba_test/source/netboot.c b/gc_bba_test/source/netboot.c index f59c223..3f970a3 100644 --- a/gc_bba_test/source/netboot.c +++ b/gc_bba_test/source/netboot.c @@ -29,6 +29,10 @@ static u8 s_peer_mac[6]; /* Last descriptor ring_next() looked at, kept for the error path: when a * length is rejected, the four raw bytes are the whole diagnosis. */ static u8 s_last_desc[4]; +/* How often the descriptor was found at each page offset. The assembler is + * supposed to put it at +0 always; hardware shows it sometimes landing at +1, + * varying frame to frame, so count both rather than assume either. */ +static u32 s_shift_hits[2]; static u8 s_last_dump[32]; static u32 s_last_page; @@ -52,8 +56,9 @@ static u32 s_rrp; static int ring_next(u8 *buf, u32 cap) { - u8 wp[2], hdr[4]; + u8 wp[2], hdr[8]; u32 rpage, wpage, dlen, flen, addr; + u32 shift; if (bba_read(REG_RWP_LO, wp, 2) < 0) return -1; rpage = s_rrp; @@ -76,11 +81,29 @@ static int ring_next(u8 *buf, u32 cap) if (ring_lag(rpage, wpage) > 10) return -4; addr = rpage << 8; - if (bba_stream((u16)addr, hdr, 4) < 0) return -1; + if (bba_stream((u16)addr, hdr, 8) < 0) return -1; memcpy(s_last_desc, hdr, 4); s_last_page = rpage; - dlen = ((u32)hdr[2] << 8) | hdr[3]; - if (dlen <= 4 || dlen > 1518 + 4) { + + /* LOCATE the descriptor rather than assume it is at +0. + * + * It should always be at +0: four bytes [00 00 len_hi len_lo] followed + * immediately by the frame. On hardware it sometimes sits at +1 instead, + * and which one varies frame to frame -- 18 of 23 frames were rejected + * for an implausible length while 5 parsed fine, in the same run. That + * is a race in the assembler, not a constant offset, and the gateware bug + * behind it is not yet found. + * + * Taking the length from whichever offset yields a plausible one lets the + * transfer proceed either way AND measures the split, which is the + * measurement needed to find the real cause. A length is a strong check: + * it must exceed the 4 descriptor bytes and fit a legal frame. */ + for (shift = 0; shift <= 1; shift++) { + dlen = ((u32)hdr[shift + 2] << 8) | hdr[shift + 3]; + if (dlen > 4 && dlen <= 1518 + 4) break; + } + if (shift > 1) { + dlen = ((u32)hdr[2] << 8) | hdr[3]; /* report the nominal one */ /* Only now is a hex dump worth an EXI transaction. Doing it for * EVERY frame (as an earlier version did) added a round trip to the * poll loop that is already losing the race against the writer -- @@ -88,6 +111,7 @@ static int ring_next(u8 *buf, u32 cap) bba_stream((u16)addr, s_last_dump, sizeof(s_last_dump)); return -3; } + s_shift_hits[shift]++; flen = dlen - 4; if (flen > cap) flen = cap; @@ -96,15 +120,15 @@ static int ring_next(u8 *buf, u32 cap) * one DMA read covers it. */ { static u8 tmp[1536 + 4]; - u32 want = dlen; + u32 want = shift + dlen; if (want > sizeof(tmp)) want = sizeof(tmp); if (bba_stream((u16)addr, tmp, want) < 0) return -1; - memcpy(buf, tmp + 4, flen); + memcpy(buf, tmp + shift + 4, flen); } /* Advance RRP by the pages this frame occupied, wrapping 1..15. */ { - u32 pages = (dlen + 255) >> 8; + u32 pages = (shift + dlen + 255) >> 8; u32 next = ((rpage - 1 + pages) % 15) + 1; if (bba_w8(REG_RRP_LO, (u8)next) < 0) return -1; s_rrp = next; @@ -210,11 +234,10 @@ int netboot_receive(void *dst, u32 maxlen, void (*status)(const char *msg)) * one the adapter is actually filtering on. */ if (bba_read(REG_PAR0, s_our_mac, 6) < 0) return -1; - if (status) { - status("waiting for host (send-dol.py)..."); - status("NOTE: quiet the link first, or other traffic laps the ring:"); - status(" sudo ./scripts/gc-testlink.sh up "); - } + /* Keep this to ONE line. The console holds about 25 lines and the + * failure diagnostics are what matter, so guidance text here just + * pushes them off the top of the screen. */ + if (status) status("waiting for host (send-dol.py)..."); t0 = gettime(); for (;;) { @@ -308,9 +331,11 @@ int netboot_receive(void *dst, u32 maxlen, void (*status)(const char *msg)) if (idle > (got ? 5000u : 30000u)) { if (status) { snprintf(line, sizeof(line), - "%s (%u seen, %u bad desc, %u lapped)", + "%s (%u seen, %u bad, %u lapped, desc@+0=%u +1=%u)", got ? "timeout mid-transfer" : "timeout: no match", - (unsigned)seen, (unsigned)bad, (unsigned)lapped); + (unsigned)seen, (unsigned)bad, (unsigned)lapped, + (unsigned)s_shift_hits[0], + (unsigned)s_shift_hits[1]); status(line); } return -3; @@ -348,8 +373,12 @@ int netboot_receive(void *dst, u32 maxlen, void (*status)(const char *msg)) s_dest[plen] = 0; } if (status) { snprintf(line, sizeof(line), - "received %u bytes in %u blocks", - (unsigned)got, (unsigned)nblocks); + "received %u bytes, %u blocks " + "(desc@+0=%u +1=%u, %u bad)", + (unsigned)got, (unsigned)nblocks, + (unsigned)s_shift_hits[0], + (unsigned)s_shift_hits[1], + (unsigned)bad); status(line); } return (int)got; }