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 <noreply@anthropic.com>
This commit is contained in:
2026-10-09 18:45:00 +02:00
parent 2aa50a7cc3
commit 4831c1353c
2 changed files with 51 additions and 22 deletions
+6 -6
View File
@@ -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("\n" C_HEAD " Self-update over the BBA" C_RESET "\n\n");
printf(" On the host: sudo ./scripts/send-dol.py <iface> <file.dol>\n\n"); printf(" On the host: sudo ./scripts/send-dol.py <iface> <file.dol>\n\n");
printf(C_DIM " The received build is written to a TEMP file, verified, and\n" /* Deliberately terse: the console holds ~25 lines and the diagnostics
" only then rotated in -- the previous build is kept alongside it\n" * printed during a failed transfer are what matter. Explanatory text
" as <name>.prev.dol, so if the new one is broken you can boot the\n" * here pushed them off the top of the screen, which cost two blind
" old one from Swiss and try the transfer again.\n" * round trips. */
" Nothing received is ever launched directly: this boots /boot.dol,\n" printf(C_DIM " temp+verify+rotate; keeps <name>.prev.dol; boots /boot.dol\n"
" your known-good Swiss.\n" C_RESET "\n"); C_RESET);
printf(" Blocks until done or it times out (30 s idle, 5 s mid-xfer).\n\n"); printf(" Blocks until done or it times out (30 s idle, 5 s mid-xfer).\n\n");
if (!s_stage) { if (!s_stage) {
+45 -16
View File
@@ -29,6 +29,10 @@ static u8 s_peer_mac[6];
/* Last descriptor ring_next() looked at, kept for the error path: when a /* Last descriptor ring_next() looked at, kept for the error path: when a
* length is rejected, the four raw bytes are the whole diagnosis. */ * length is rejected, the four raw bytes are the whole diagnosis. */
static u8 s_last_desc[4]; 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 u8 s_last_dump[32];
static u32 s_last_page; static u32 s_last_page;
@@ -52,8 +56,9 @@ static u32 s_rrp;
static int ring_next(u8 *buf, u32 cap) 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 rpage, wpage, dlen, flen, addr;
u32 shift;
if (bba_read(REG_RWP_LO, wp, 2) < 0) return -1; if (bba_read(REG_RWP_LO, wp, 2) < 0) return -1;
rpage = s_rrp; rpage = s_rrp;
@@ -76,11 +81,29 @@ static int ring_next(u8 *buf, u32 cap)
if (ring_lag(rpage, wpage) > 10) return -4; if (ring_lag(rpage, wpage) > 10) return -4;
addr = rpage << 8; 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); memcpy(s_last_desc, hdr, 4);
s_last_page = rpage; 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 /* 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 * EVERY frame (as an earlier version did) added a round trip to the
* poll loop that is already losing the race against the writer -- * 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)); bba_stream((u16)addr, s_last_dump, sizeof(s_last_dump));
return -3; return -3;
} }
s_shift_hits[shift]++;
flen = dlen - 4; flen = dlen - 4;
if (flen > cap) flen = cap; if (flen > cap) flen = cap;
@@ -96,15 +120,15 @@ static int ring_next(u8 *buf, u32 cap)
* one DMA read covers it. */ * one DMA read covers it. */
{ {
static u8 tmp[1536 + 4]; static u8 tmp[1536 + 4];
u32 want = dlen; u32 want = shift + dlen;
if (want > sizeof(tmp)) want = sizeof(tmp); if (want > sizeof(tmp)) want = sizeof(tmp);
if (bba_stream((u16)addr, tmp, want) < 0) return -1; 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. */ /* 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; u32 next = ((rpage - 1 + pages) % 15) + 1;
if (bba_w8(REG_RRP_LO, (u8)next) < 0) return -1; if (bba_w8(REG_RRP_LO, (u8)next) < 0) return -1;
s_rrp = next; 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. */ * one the adapter is actually filtering on. */
if (bba_read(REG_PAR0, s_our_mac, 6) < 0) return -1; if (bba_read(REG_PAR0, s_our_mac, 6) < 0) return -1;
if (status) { /* Keep this to ONE line. The console holds about 25 lines and the
status("waiting for host (send-dol.py)..."); * failure diagnostics are what matter, so guidance text here just
status("NOTE: quiet the link first, or other traffic laps the ring:"); * pushes them off the top of the screen. */
status(" sudo ./scripts/gc-testlink.sh up <iface>"); if (status) status("waiting for host (send-dol.py)...");
}
t0 = gettime(); t0 = gettime();
for (;;) { for (;;) {
@@ -308,9 +331,11 @@ int netboot_receive(void *dst, u32 maxlen, void (*status)(const char *msg))
if (idle > (got ? 5000u : 30000u)) { if (idle > (got ? 5000u : 30000u)) {
if (status) { if (status) {
snprintf(line, sizeof(line), 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", 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); status(line);
} }
return -3; return -3;
@@ -348,8 +373,12 @@ int netboot_receive(void *dst, u32 maxlen, void (*status)(const char *msg))
s_dest[plen] = 0; s_dest[plen] = 0;
} }
if (status) { snprintf(line, sizeof(line), if (status) { snprintf(line, sizeof(line),
"received %u bytes in %u blocks", "received %u bytes, %u blocks "
(unsigned)got, (unsigned)nblocks); "(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); } status(line); }
return (int)got; return (int)got;
} }