netboot: hold the screen on failure, dump the descriptor, shrink the block
Two separate problems, one of which was hiding the other. 1. The diagnostics were unreadable. do_netboot() printed them and returned, and the caller's draw_main() redrew immediately and wiped the screen -- so two consecutive failures were reported only as "a short flash" with no detail, and the error code alone was not enough to act on. nb_pause() now holds the screen until a button is pressed, on every failure path. This cost two blind round trips, each of which needs a hand copy of the .dol to the SD card. Printing a diagnosis and then erasing it is worse than not printing one. 2. The default block was 1024 bytes, making ~1050-byte frames that span FIVE of the SPRAM ring's 15 pages. The only multi-page frame ever proven on this hardware is the 342-byte DHCP broadcast from bring-up, which spans two. Five-page frames are untested territory, and a transfer is the wrong place to discover a wrap bug. Default is now 256 bytes -> ~296-byte frame -> two pages, deliberately close to the proven case. --block raises it once small blocks work; the GC still caps at NB_BLOCK_MAX = 1024. ring_next() also keeps the four raw descriptor bytes and the page it read them from, and the error path prints them. When a length is rejected those bytes ARE the diagnosis: 00 00 xx xx tells us the assembler wrote a descriptor and the length is simply unexpected, whereas garbage tells us we are reading the wrong page. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -125,6 +125,21 @@ static void nb_status(const char *msg)
|
|||||||
printf(" %s\n", msg);
|
printf(" %s\n", msg);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/* Hold the screen until a button is pressed. Without this the caller's
|
||||||
|
* draw_main() redraws immediately and wipes every diagnostic line netboot
|
||||||
|
* just printed -- which is exactly why the first two failures were reported
|
||||||
|
* as "a short flash" with no detail. */
|
||||||
|
static void nb_pause(const char *why)
|
||||||
|
{
|
||||||
|
printf("\n" C_DIM " %s -- press any button to return to the menu\n" C_RESET,
|
||||||
|
why);
|
||||||
|
for (;;) {
|
||||||
|
VIDEO_WaitVSync();
|
||||||
|
PAD_ScanPads();
|
||||||
|
if (PAD_ButtonsDown(0)) return;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
static void do_netboot(char *status, size_t nstatus)
|
static void do_netboot(char *status, size_t nstatus)
|
||||||
{
|
{
|
||||||
int len, rc;
|
int len, rc;
|
||||||
@@ -147,11 +162,13 @@ static void do_netboot(char *status, size_t nstatus)
|
|||||||
len = netboot_receive(s_stage, s_stage_max, nb_status);
|
len = netboot_receive(s_stage, s_stage_max, nb_status);
|
||||||
if (len <= 0) {
|
if (len <= 0) {
|
||||||
snprintf(status, nstatus, "net-update failed (%d)", len);
|
snprintf(status, nstatus, "net-update failed (%d)", len);
|
||||||
|
nb_pause("transfer failed");
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
rc = netboot_sd_write(netboot_dest_path(), s_stage, (u32)len, nb_status);
|
rc = netboot_sd_write(netboot_dest_path(), s_stage, (u32)len, nb_status);
|
||||||
if (rc < 0) {
|
if (rc < 0) {
|
||||||
|
nb_pause("SD write failed");
|
||||||
/* Nothing has been launched and nothing is half-written that we
|
/* Nothing has been launched and nothing is half-written that we
|
||||||
* know of, so staying in the menu is the safe outcome. */
|
* know of, so staying in the menu is the safe outcome. */
|
||||||
snprintf(status, nstatus,
|
snprintf(status, nstatus,
|
||||||
@@ -166,6 +183,7 @@ static void do_netboot(char *status, size_t nstatus)
|
|||||||
/* Reuse the staging region: the new build is already on the card and
|
/* Reuse the staging region: the new build is already on the card and
|
||||||
* verified, so the copy in RAM is finished with. */
|
* verified, so the copy in RAM is finished with. */
|
||||||
rc = netboot_boot_file("/boot.dol", s_stage, s_stage_max, nb_status);
|
rc = netboot_boot_file("/boot.dol", s_stage, s_stage_max, nb_status);
|
||||||
|
nb_pause("boot refused");
|
||||||
|
|
||||||
/* Only reached on refusal. The new build IS on the card either way,
|
/* Only reached on refusal. The new build IS on the card either way,
|
||||||
* so this is recoverable by rebooting into Swiss by hand. */
|
* so this is recoverable by rebooting into Swiss by hand. */
|
||||||
|
|||||||
@@ -26,6 +26,10 @@
|
|||||||
|
|
||||||
static u8 s_our_mac[6];
|
static u8 s_our_mac[6];
|
||||||
static u8 s_peer_mac[6];
|
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];
|
||||||
|
static u32 s_last_page;
|
||||||
|
|
||||||
static u16 rd16(const u8 *p) { return ((u16)p[0] << 8) | p[1]; }
|
static u16 rd16(const u8 *p) { return ((u16)p[0] << 8) | p[1]; }
|
||||||
static void wr16(u8 *p, u16 v) { p[0] = (u8)(v >> 8); p[1] = (u8)v; }
|
static void wr16(u8 *p, u16 v) { p[0] = (u8)(v >> 8); p[1] = (u8)v; }
|
||||||
@@ -48,6 +52,8 @@ static int ring_next(u8 *buf, u32 cap)
|
|||||||
|
|
||||||
addr = rpage << 8;
|
addr = rpage << 8;
|
||||||
if (bba_stream((u16)addr, hdr, 4) < 0) return -1;
|
if (bba_stream((u16)addr, hdr, 4) < 0) return -1;
|
||||||
|
memcpy(s_last_desc, hdr, 4);
|
||||||
|
s_last_page = rpage;
|
||||||
dlen = ((u32)hdr[2] << 8) | hdr[3];
|
dlen = ((u32)hdr[2] << 8) | hdr[3];
|
||||||
if (dlen <= 4 || dlen > 1518 + 4) return -3; /* descriptor implausible */
|
if (dlen <= 4 || dlen > 1518 + 4) return -3; /* descriptor implausible */
|
||||||
flen = dlen - 4;
|
flen = dlen - 4;
|
||||||
@@ -186,6 +192,11 @@ int netboot_receive(void *dst, u32 maxlen, void (*status)(const char *msg))
|
|||||||
n == -2 ? "RRP outside pages 1-15" :
|
n == -2 ? "RRP outside pages 1-15" :
|
||||||
"descriptor length implausible");
|
"descriptor length implausible");
|
||||||
status(line);
|
status(line);
|
||||||
|
snprintf(line, sizeof(line),
|
||||||
|
" page %u descriptor = %02X %02X %02X %02X",
|
||||||
|
(unsigned)s_last_page, s_last_desc[0],
|
||||||
|
s_last_desc[1], s_last_desc[2], s_last_desc[3]);
|
||||||
|
status(line);
|
||||||
}
|
}
|
||||||
return -2;
|
return -2;
|
||||||
}
|
}
|
||||||
|
|||||||
+14
-3
@@ -25,7 +25,13 @@ import time
|
|||||||
ETHERTYPE = 0x88B5
|
ETHERTYPE = 0x88B5
|
||||||
MAGIC = b"GCUP"
|
MAGIC = b"GCUP"
|
||||||
OP_DATA, OP_ACK, OP_DONE = 1, 2, 3
|
OP_DATA, OP_ACK, OP_DONE = 1, 2, 3
|
||||||
BLOCK = 1024
|
# Default payload 256 bytes -> a ~296-byte frame, which with the 4-byte
|
||||||
|
# descriptor occupies TWO of the SPRAM ring's 15 pages. That is deliberately
|
||||||
|
# close to the only multi-page frame ever proven on this hardware (a 342-byte
|
||||||
|
# DHCP broadcast). 1024-byte payloads make ~1050-byte frames spanning FIVE
|
||||||
|
# pages, which has never been exercised -- raise --block once small blocks are
|
||||||
|
# known to work, not before.
|
||||||
|
BLOCK_DEFAULT = 256
|
||||||
HDR = 26
|
HDR = 26
|
||||||
# The GC's MAC comes from PAR0-5, which the gateware pre-populates with a
|
# The GC's MAC comes from PAR0-5, which the gateware pre-populates with a
|
||||||
# Nintendo OUI. Discovered from the first ACK, so this is only the initial
|
# Nintendo OUI. Discovered from the first ACK, so this is only the initial
|
||||||
@@ -54,10 +60,15 @@ def main():
|
|||||||
ap.add_argument("--dest", default="/gc_bba_test.dol",
|
ap.add_argument("--dest", default="/gc_bba_test.dol",
|
||||||
help="where to write it on the SD card "
|
help="where to write it on the SD card "
|
||||||
"(default /gc_bba_test.dol)")
|
"(default /gc_bba_test.dol)")
|
||||||
|
ap.add_argument("--block", type=int, default=BLOCK_DEFAULT,
|
||||||
|
help=f"payload bytes per frame (default {BLOCK_DEFAULT}; "
|
||||||
|
"the GC caps it at 1024)")
|
||||||
args = ap.parse_args()
|
args = ap.parse_args()
|
||||||
|
|
||||||
|
if not 1 <= args.block <= 1024:
|
||||||
|
sys.exit("--block must be 1..1024 (the GC's NB_BLOCK_MAX)")
|
||||||
img = open(args.dol, "rb").read()
|
img = open(args.dol, "rb").read()
|
||||||
blocks = [img[i:i + BLOCK] for i in range(0, len(img), BLOCK)]
|
blocks = [img[i:i + args.block] for i in range(0, len(img), args.block)]
|
||||||
if not blocks:
|
if not blocks:
|
||||||
sys.exit("empty image")
|
sys.exit("empty image")
|
||||||
if len(blocks) > 0xFFFF:
|
if len(blocks) > 0xFFFF:
|
||||||
@@ -68,7 +79,7 @@ def main():
|
|||||||
src = s.getsockname()[4]
|
src = s.getsockname()[4]
|
||||||
s.settimeout(args.timeout)
|
s.settimeout(args.timeout)
|
||||||
|
|
||||||
print(f"{args.dol}: {len(img)} bytes, {len(blocks)} blocks of {BLOCK}")
|
print(f"{args.dol}: {len(img)} bytes, {len(blocks)} blocks of {args.block}")
|
||||||
print(f"sending from {src.hex(':')} on {args.iface}")
|
print(f"sending from {src.hex(':')} on {args.iface}")
|
||||||
|
|
||||||
dst = BROADCAST
|
dst = BROADCAST
|
||||||
|
|||||||
Reference in New Issue
Block a user