diff --git a/CLAUDE.md b/CLAUDE.md index 8ffcc2a..7f10ec0 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -139,6 +139,29 @@ is **$C9**. See `src/net/uci/uci_regs.inc` for the full equate list `uci_read_data`, etc. No zero-page usage — all absolute addressing and self-modifying code. +`uci_wait_idle` is wall-clock-bounded (5 s budget via CIA1 TOD) per +the design note below. On timeout it returns C=1 with `net_last_error += UCI_ERR_WAIT_TIMEOUT`. The four callers (`net_dhcp_acquire`, +`net_tcp_connect`, `net_tcp_send`, `net_tcp_close`) all `bcs` out to +surface the failure rather than letting the C64 hang indefinitely on +a wedged FPGA. `uci_push_wait` and `uci_end_cmd` are still unbounded +and should be converted to the same TOD pattern if a wedge there is +ever observed. + +### UCI error codes + +`src/net/uci/uci_errors.inc` enumerates the values surfaced via +`net_last_error` and `net_tcp_state`. The most load-bearing: + + - `UCI_ERR_NO_SOCKET` — `net_tcp_connect` got a short-read on the + OPEN_TCP response (no socket-id byte), so the firmware never + actually opened the TCP connection. `net_tcp_state` is set to + `UCI_TCP_CONNECT_FAIL` and C=1 is returned. Without this check + the C64 would commit to a phantom socket and push TLS bytes + into nowhere (see issue #36). + - `UCI_ERR_WAIT_TIMEOUT` — `uci_wait_idle` exhausted its 5 s budget + (see issue #37). + ### DNS UCI firmware resolves hostnames internally during `TCP_CONNECT`. There @@ -360,16 +383,24 @@ touching TLS call sites. ### Design note — bounded timeouts must use wall-clock time -Any future robustness work on the UCI adapter's spin-wait helpers -(`uci_wait_idle`, `uci_push_wait`, etc.) MUST use a wall-clock time -source — CIA timer on stock C64, TOD clock on U64E — rather than a -cycle-counted iteration budget. The fences around every UCI register -access make per-iteration cost scale with CPU speed: a budget that is -ample at 1 MHz collapses to far too short at 48 MHz because turbo -scales CPU cycles but not the FPGA's wire-level operation durations. -A prior attempt on branch `feat/net-drain-abi` split waits into -fast/long tiers with cycle-count budgets and broke DHCP at turbo for -exactly this reason; the branch was abandoned. +Robustness work on the UCI adapter's spin-wait helpers (`uci_wait_idle`, +`uci_push_wait`, etc.) MUST use a wall-clock time source — CIA timer +on stock C64, TOD clock on U64E — rather than a cycle-counted iteration +budget. The fences around every UCI register access make per-iteration +cost scale with CPU speed: a budget that is ample at 1 MHz collapses +to far too short at 48 MHz because turbo scales CPU cycles but not the +FPGA's wire-level operation durations. A prior attempt on branch +`feat/net-drain-abi` split waits into fast/long tiers with cycle-count +budgets and broke DHCP at turbo for exactly this reason; the branch +was abandoned. + +`uci_wait_idle` is the first helper to follow this pattern (issue #37). +At entry it samples CIA1 TOD ($DC08-$DC0B) — read order is HOUR +(latch) → MIN → SEC → TENTHS (unlatch) — and on each spin pass re-reads +TENTHS, bailing with C=1 + `net_last_error = UCI_ERR_WAIT_TIMEOUT` +after 50 transitions (~5 s wall-clock, independent of CPU turbo). State +lives in two SMC bytes inside the routine to match the file's no-ZP +convention. Use this as the template for any future bounded helper. ## Memory layout diff --git a/src/net/uci/net.s b/src/net/uci/net.s index e7cb1ec..c27964b 100644 --- a/src/net/uci/net.s +++ b/src/net/uci/net.s @@ -313,6 +313,7 @@ net_poll: ; ============================================================================= net_dhcp_acquire: jsr uci_wait_idle + bcs @dhcp_wait_to ; FPGA wedged — bail with C=1 lda #UCI_TARGET_NETWORK jsr uci_begin_cmd @@ -332,6 +333,7 @@ net_dhcp_acquire: lda #UCI_ERR_CMD_FAILED sta net_last_error +@dhcp_wait_to: sec rts @@ -399,6 +401,14 @@ net_tcp_connect: stx uci_connect_port_hi jsr uci_wait_idle + bcc :+ + ; FPGA wedged before we even queued anything — surface the timeout + ; (net_last_error already set) with the connect-fail tcp_state. + lda #UCI_TCP_CONNECT_FAIL + sta net_tcp_state + sec + rts +: lda #UCI_TARGET_NETWORK jsr uci_begin_cmd @@ -446,6 +456,10 @@ net_tcp_connect: @tc_no_err: ; Read 1-byte socket_id response. + ; Pre-zero uci_socket_id so a short-read leaves a known sentinel + ; (uci_read_resp_bytes only writes the bytes it actually receives). + lda #$00 + sta uci_socket_id lda #uci_socket_id @@ -458,11 +472,29 @@ net_tcp_connect: jsr uci_drain_status jsr uci_ack + ; Validate the response: firmware must have returned at least 1 + ; byte (uci_resp_count) AND a non-zero socket_id. Issue #36 — at + ; least one observed U64E firmware path returns no payload while + ; clearing the error bit, leaving uci_socket_id = 0 and us writing + ; into a phantom socket. Convert that into a clean failure. + lda uci_resp_count + beq @tc_no_socket + lda uci_socket_id + beq @tc_no_socket + lda #UCI_TCP_CONNECTED sta net_tcp_state clc rts +@tc_no_socket: + lda #UCI_ERR_NO_SOCKET + sta net_last_error + lda #UCI_TCP_CONNECT_FAIL + sta net_tcp_state + sec + rts + ; ============================================================================= ; net_tcp_send — push up to net_send_len bytes from (AX) through SOCKET_WRITE. ; @@ -519,6 +551,12 @@ net_tcp_send: @begin_chunk: jsr uci_wait_idle + bcc :+ + ; FPGA wedged mid-send — surface as send-fail (net_last_error already + ; set to UCI_ERR_WAIT_TIMEOUT inside uci_wait_idle). + sec + rts +: lda #UCI_TARGET_NETWORK jsr uci_begin_cmd @@ -646,6 +684,13 @@ net_tcp_send: ; ============================================================================= net_tcp_close: jsr uci_wait_idle + bcc :+ + ; FPGA wedged on close — force CLOSED state and bail. Best-effort + ; semantics already match the existing close path (no return code). + lda #UCI_TCP_CLOSED + sta net_tcp_state + rts +: lda #UCI_TARGET_NETWORK jsr uci_begin_cmd diff --git a/src/net/uci/uci_cmd.s b/src/net/uci/uci_cmd.s index 6ff81eb..b07cd70 100644 --- a/src/net/uci/uci_cmd.s +++ b/src/net/uci/uci_cmd.s @@ -26,6 +26,10 @@ ; one interface-index parameter). Later phases will extend as needed. .include "uci_regs.inc" +.include "uci_errors.inc" + +; net_last_error lives in net.s's BSS — we set it on wait timeout (#37). +.import net_last_error .export uci_abort .export uci_wait_idle @@ -61,19 +65,69 @@ uci_abort: rts ; ============================================================================= -; uci_wait_idle — spin until STATE==0 AND CMD_BUSY==0 +; uci_wait_idle — spin until STATE==0 AND CMD_BUSY==0, with wall-clock cap ; UCI_STAT_STATE ($30) covers the state field; CMD_BUSY ($01) is bit 0. ; ORing them (MASK $31) and looping while nonzero gives "fully idle". +; +; Issue #37 — the historical unbounded spin converts an FPGA wedge into a +; 600 s test sentinel timeout. The cap below uses CIA1 TOD (CIA_TOD_TENTHS, +; ticks at 10 Hz) — the only clock that runs at the same wall-clock rate +; regardless of CPU turbo speed. Cycle-counted budgets do NOT work here: +; the per-iteration cost scales with turbo (each fence is ~38 us of FPGA +; wall time but only a few CPU cycles at 48 MHz), so a budget tuned at +; 1 MHz collapses at 48 MHz (and vice versa). A prior attempt on +; feat/net-drain-abi shipped cycle-counted budgets and broke turbo DHCP +; for exactly this reason. +; +; CIA TOD read protocol: reading the HOUR register latches the four +; registers atomically; reading the TENTHS register unlatches them. +; We only need TENTHS for our 5-second budget, but we still latch+unlatch +; properly so we don't disturb other code that might be reading TOD. +; +; Budget: UCI_WAIT_IDLE_BUDGET_TENTHS (50 = 5 s). On expiry: set +; net_last_error = UCI_ERR_WAIT_TIMEOUT, return C=1. +; +; Output: C=0 on idle, C=1 on timeout. ; Clobbers: A ; ============================================================================= +CIA_TOD_TENTHS = $DC08 +CIA_TOD_HOUR = $DC0B +UCI_WAIT_IDLE_BUDGET_TENTHS = 50 ; 5 seconds at 10 Hz + uci_wait_idle: + ; Sample initial TENTHS for delta-tracking. Latch via HOUR, + ; release via TENTHS. We don't care about the HOUR value itself. + lda CIA_TOD_HOUR + lda CIA_TOD_TENTHS + sta @wi_last_tenths + lda #$00 + sta @wi_elapsed +@wi_loop: lda UCI_STATUS uci_fence ; settle read before testing bits and #(UCI_STAT_STATE | UCI_STAT_CMD_BUSY) ; $31 beq @idle_done - jmp uci_wait_idle ; long branch: fence too wide for BNE + + ; Check TOD for elapsed tenths. Latch (HOUR) then read TENTHS. + lda CIA_TOD_HOUR + lda CIA_TOD_TENTHS + cmp @wi_last_tenths + beq @wi_loop ; no change — keep spinning + sta @wi_last_tenths + inc @wi_elapsed + lda @wi_elapsed + cmp #UCI_WAIT_IDLE_BUDGET_TENTHS + bcc @wi_loop ; under budget — continue + ; Timeout + lda #UCI_ERR_WAIT_TIMEOUT + sta net_last_error + sec + rts @idle_done: + clc rts +@wi_last_tenths: .byte 0 +@wi_elapsed: .byte 0 ; ============================================================================= ; uci_wait_not_busy — spin until CMD_BUSY==0 (ignore STATE) diff --git a/src/net/uci/uci_errors.inc b/src/net/uci/uci_errors.inc index 4ac13f3..6525e6c 100644 --- a/src/net/uci/uci_errors.inc +++ b/src/net/uci/uci_errors.inc @@ -12,11 +12,14 @@ UCI_ERR_CONNECT_FAIL = $84 ; TCP_CONNECT returned an error bit UCI_ERR_SEND_FAIL = $85 ; SOCKET_WRITE returned an error bit UCI_ERR_READ_FAIL = $86 ; SOCKET_READ returned an error bit UCI_ERR_SHORT_WRITE = $87 ; SOCKET_WRITE wrote fewer bytes than requested +UCI_ERR_NO_SOCKET = $88 ; TCP_CONNECT response yielded no socket_id +UCI_ERR_WAIT_TIMEOUT = $89 ; uci_wait_idle exceeded its wall-clock budget ; TCP state values stored in net_tcp_state UCI_TCP_CLOSED = $00 ; no active socket UCI_TCP_CONNECTED = $01 ; connected, reads/writes valid UCI_TCP_ERROR = $02 ; saw an error on a read — stop polling +UCI_TCP_CONNECT_FAIL = $03 ; TCP_CONNECT did not yield a usable socket ; UCI firmware data queue max per SOCKET_WRITE push (see uci_network.py) UCI_DATA_QUEUE_MAX = 800 diff --git a/tools/uci/_analyze_ecdsa_trace.py b/tools/uci/_analyze_ecdsa_trace.py index ef2b132..666d31a 100644 --- a/tools/uci/_analyze_ecdsa_trace.py +++ b/tools/uci/_analyze_ecdsa_trace.py @@ -21,6 +21,8 @@ from collections import Counter from pathlib import Path +from c64_test_harness import Labels + REPO_ROOT = Path(__file__).resolve().parents[2] DEFAULT_BASE = Path("/tmp/ecdsa_debug") @@ -32,14 +34,9 @@ def _load_labels() -> dict[int, str]: if not labels_file.is_file(): return {} out: dict[int, str] = {} - for line in labels_file.read_text().splitlines(): - parts = line.split() - if len(parts) >= 3 and parts[0] == "al" and parts[2].startswith("."): - name = parts[2][1:] - _, hex_addr = parts[1].split(":", 1) - a = int(hex_addr, 16) - # Prefer the first (public) label if multiple map to same addr - out.setdefault(a, name) + for name, a in Labels.from_file(labels_file).items(): + # Prefer the first (public) label if multiple map to same addr + out.setdefault(a, name) return out diff --git a/tools/uci/phase2_check.py b/tools/uci/phase2_check.py index 0caf388..fb66e0e 100644 --- a/tools/uci/phase2_check.py +++ b/tools/uci/phase2_check.py @@ -31,6 +31,7 @@ import time from pathlib import Path +from c64_test_harness import Labels from c64_test_harness.backends.device_lock import DeviceLock from c64_test_harness.backends.ultimate64 import Ultimate64Transport from c64_test_harness.backends.ultimate64_client import Ultimate64Client @@ -74,14 +75,10 @@ def load_label(name: str) -> int: Entries look like: `al C:BC4B .net_local_ip` — the `.name` token is unambiguous across backend cfgs. """ - token = f".{name}" - for line in LABELS_PATH.read_text().splitlines(): - parts = line.split() - if len(parts) >= 3 and parts[0] == "al" and parts[2] == token: - addr_tok = parts[1] # "C:BC4B" - _, hex_addr = addr_tok.split(":", 1) - return int(hex_addr, 16) - raise KeyError(f"label {name!r} not found in {LABELS_PATH}") + labels = Labels.from_file(LABELS_PATH) + if name not in labels: + raise KeyError(f"label {name!r} not found in {LABELS_PATH}") + return labels[name] def is_plausible_private(ip: tuple[int, int, int, int]) -> bool: diff --git a/tools/uci/phase3_tcp_echo.py b/tools/uci/phase3_tcp_echo.py index b861d0f..30395b0 100644 --- a/tools/uci/phase3_tcp_echo.py +++ b/tools/uci/phase3_tcp_echo.py @@ -35,6 +35,7 @@ import time from pathlib import Path +from c64_test_harness import Labels from c64_test_harness.backends.device_lock import DeviceLock from c64_test_harness.backends.ultimate64 import Ultimate64Transport from c64_test_harness.backends.ultimate64_client import Ultimate64Client @@ -107,14 +108,7 @@ def _run_echo_server(bind_ip: str, port: int, result: dict) -> None: def _load_labels() -> dict[str, int]: - labels: dict[str, int] = {} - for line in LABELS_PATH.read_text().splitlines(): - parts = line.split() - if len(parts) >= 3 and parts[0] == "al" and parts[2].startswith("."): - name = parts[2][1:] - _, hex_addr = parts[1].split(":", 1) - labels[name] = int(hex_addr, 16) - return labels + return dict(Labels.from_file(LABELS_PATH)) def _build_test_routine(labels: dict[str, int], host_ip: str, port: int) -> bytes: diff --git a/tools/uci/test_http_live.py b/tools/uci/test_http_live.py index 792a4c8..2c0c9ae 100644 --- a/tools/uci/test_http_live.py +++ b/tools/uci/test_http_live.py @@ -18,6 +18,7 @@ import time from pathlib import Path +from c64_test_harness import Labels from c64_test_harness.backends.device_lock import DeviceLock from c64_test_harness.backends.ultimate64 import Ultimate64Transport from c64_test_harness.backends.ultimate64_client import Ultimate64Client @@ -44,14 +45,7 @@ def _load_labels() -> dict[str, int]: - labels: dict[str, int] = {} - for line in LABELS_PATH.read_text().splitlines(): - parts = line.split() - if len(parts) >= 3 and parts[0] == "al" and parts[2].startswith("."): - name = parts[2][1:] - _, hex_addr = parts[1].split(":", 1) - labels[name] = int(hex_addr, 16) - return labels + return dict(Labels.from_file(LABELS_PATH)) def _build_http_routine(labels: dict[str, int], hostname_len: int, port: int) -> bytes: diff --git a/tools/uci/test_http_local.py b/tools/uci/test_http_local.py index 3da097d..7ba9981 100644 --- a/tools/uci/test_http_local.py +++ b/tools/uci/test_http_local.py @@ -26,6 +26,7 @@ import time from pathlib import Path +from c64_test_harness import Labels from c64_test_harness.backends.device_lock import DeviceLock from c64_test_harness.backends.ultimate64 import Ultimate64Transport from c64_test_harness.backends.ultimate64_client import Ultimate64Client @@ -97,14 +98,7 @@ def _run_http_server(bind_ip: str, port: int, result: dict) -> None: def _load_labels() -> dict[str, int]: - labels: dict[str, int] = {} - for line in LABELS_PATH.read_text().splitlines(): - parts = line.split() - if len(parts) >= 3 and parts[0] == "al" and parts[2].startswith("."): - name = parts[2][1:] - _, hex_addr = parts[1].split(":", 1) - labels[name] = int(hex_addr, 16) - return labels + return dict(Labels.from_file(LABELS_PATH)) def _build_http_routine(labels: dict[str, int], port: int) -> bytes: