fix(crypto): REU-less X25519 under USE_NISTCURVES_ONCHIP — fe_mul rows via og_common - #69
Merged
Conversation
…s via og_common fe_mul's row fetch bound to the sibling's REU-DMA reu_fetch_mul_row even under the onchip profile, so the 'no REU required' onchip build computed garbage X25519 (KAT-confirmed: fe_mul 3/3 WRONG in VICE without -reu). Under USE_NISTCURVES_ONCHIP fe_mul now generates each row on-chip via a gen_mul_row-style stub (og_src_ld SMC-patch to mul_src2_buf, X=31, jmp og_common) — the exact fp256 pattern. Design decision: og_common reuse (non-CT) over a CT fixed-count generator. In-tree fe_mul is already data-dependent on secret bytes (src1[i]==0 / src2[j]==0 zero-skips, carry-propagation branches; fe_sqr's mult66 sign branch likewise), so a CT row generator would not restore constant time — it would only cost ~2x. og_common's staged-src contract (entries for nonzero staged bytes + diagonal, zero-skip) matches fe_mul's read set exactly; mul_src2_buf is already the absolute staging buffer. fe_sqr needs no change: it reads sqtab/sqtab2 quarter-square tables directly (mult66 inline), no REU rows. fe_mul_a24 uses mul38 tables. Cost estimate at 1 MHz: ~3.2K cy/row on-chip vs ~600 cy DMA → +~85K cy per fe_mul; X25519 scalarmult ≈ 1276 fe_mul → +~1.8 min per scalarmult at stock clock (fe_sqr/fe_inv dominated by sqr, unaffected). Turbo hosts scale it away — same shape as the verify-path onchip tradeoff. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The --slow RFC 7748 vectors failed on EVERY build/config (default REU profile included, VICE -reu on) with deterministic wrong outputs. x25519_scalarmult does not clamp; production clamps via x25519_clamp in tls_ecdh.s before the ladder. The test driver wrote raw RFC scalars, so the C64 computed the correct product for the UNCLAMPED scalar — verified byte-exact against a Python reference ladder run with the raw scalar. Test-only fix: jsr x25519_clamp before x25519_scalarmult, mirroring the production call order. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This was referenced Jul 29, 2026
JC-000 added a commit
that referenced
this pull request
Aug 12, 2026
…ndshake wall-clock (#74) * fix(tls): per-backend post-ServerHello drain budget — UCI 125.4 s -> expect ~56 s Fixes#73. Regression introduced by #71 (merged), measured on C64 Ultimate hardware: the shipped UCI onchip handshake+GET went 51.0 s -> 125.4 s at 48 MHz, a ~2.5x wall-clock regression. Correctness was never affected. The drain compensates for a property only ip65 has: it ACKs inbound TCP data only when the consumer pumps net_poll, so without draining, the server's post-SH flight tail sits unACKed through the multi-minute ECDHE/verify stalls and impatient peers drop the connection. UCI firmware ACKs autonomously — which is exactly why that bug was never observable on Ultimate hardware — so on UCI the drain has nothing to buy. Its cost, though, is anything but backend-neutral. An ip65 net_poll is a cheap NIC pump; a UCI net_poll is a full firmware command round-trip (SOCKET_READ: uci_wait_not_busy, uci_begin_cmd, 4x uci_put_byte, uci_push_wait, uci_check_err, header read, uci_drain_resp + uci_drain_status + uci_ack — ~25 fenced register accesses plus FPGA turnaround). Measured ~37 ms/poll at 48 MHz, of which only ~2.8 ms is fence time; the rest is clock-invariant firmware turnaround, so turbo does not amortize it. 2000 polls = ~70 s. Move the budget into a per-backend net_tuning.inc, resolved through the existing `-I src/net/$(BACKEND)` include path so tls13.s stays backend-agnostic: ip65: 8 x 250 = 2000 polls (UNCHANGED — the validated figure) uci: 1 x 16 = 16 polls (~0.6 s; a deliberate small hedge rather than 0, so anything already queued still lands in the ring before the long stalls without relying on firmware autonomy being absolute. Non-zero matters: the loop's dex/bne shape turns an INNER of 0 into 256 iterations.) Verified in the assembled listing: ip65 emits A0 08 / A2 FA, uci emits A0 01 / A2 10. All five profiles link with unchanged sizes (47,105 B ip65, 62,977 B uci). The ip65 images are byte-identical to master, so the ip65 e2e evidence from #71 carries over untouched; the UCI side needs a hardware re-measure (expected ~56 s = 51.0 s baseline + ~4.5 s for #69's on-chip X25519 rows at this clock). Process note for the record: I approved #71's unconditional drain with an "~1 s on UCI turbo" estimate that counted fence time only and ignored firmware turnaround — off by ~70x. An iteration-count budget calibrated on one backend's poll cost is precisely the failure mode that c64-lib-contract SPEC §13.4 (bounded waits must be wall-clock-based) exists to prevent. A TOD-bounded idle drain (poll until the ring stops growing) would be the principled backend-agnostic variant; it needs care around CIA1 TOD latch interaction with the UCI adapter's own TOD waits, so it is left as a documented refinement rather than bundled here. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * tools/uci: skip redundant turbo write — unblocks C64U 64 MHz measurement The C64 Ultimate speed-switch quirk is sharper than CLAUDE.md records, and in a way that made 64 MHz unmeasurable: the bridge glitch is caused by the REST config WRITE itself, it SURVIVES the following reset, and it fires even when the written value equals the current one. Evidence (C64U 10.53.21.158, onchip UCI build, 2026-07-29): three 64 MHz attempts each wrote "64" while the device was ALREADY at 64 MHz, and each lost its first TCP_CONNECT — UCI_ERR_NO_SOCKET, net_tcp_state=CONNECT_FAIL, all TLS/HTTP state zero, ring head==tail==0, no SYN on the wire. The same PRG at the same 64 MHz setting passes under tools/uci/test_http_local.py, whose only material difference is that it performs no config write before its reset. At 48 MHz the identical pattern costs only the first attempt (fail, pass on retry), which is why this hid for so long. Fix: probe Turbo Control + CPU Speed first and skip the write entirely when they already match (the common case for repeat runs at one speed), and give a genuine change a 3.0 s settle instead of 0.5 s (TURBO_SETTLE overrides). The probe is best-effort — if it raises, we write as before. Both sides of the speed comparison are str()-normalised so a firmware type change cannot silently restore always-write. Not the wedge signature (that reaches ENC1 RX and stalls); this never opens a socket. Found by the hardware worker while validating #72/#73 — it blocked the 64 MHz headline number for PR #74. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(tools/uci): turbo probe read the wrong JSON shape — skip was unreachable The redundant-write skip added in e044f51 never fired on the C64U: the probe read `get_config_item(...).get("value")`, but the REST config responses wrap items in a `<Category>` key and put each value directly under the ITEM name — there is no per-item "value" key. The probe therefore returned None/None on every device, `str(None) != "48"`, and the code always took the write path (log line: "Setting turbo to 48 MHz (from None/None)..."). Fix: mirror the harness's own get_reu_config — fetch the category, unwrap, index by item name — using public API only (no reliance on the private _unwrap). One request now covers both items instead of two. `.get(CAT, cat)` tolerates a response with or without the wrapper. Verified without hardware by parsing all three plausible shapes (wrapped, flat, int-typed CPU Speed): the skip decision comes out True for each, so a firmware shape or type change degrades to "write anyway" rather than to a silent wrong answer. Caught because the write path logs the probed values ("from None/None") — keeping the observed state in that message is what made an unreachable branch visible in a passing run. Worth remembering. Note the 3.0 s settle from e044f51 is doing real work independently: the first genuine speed change under it (64->48) connected on the first attempt, where every pre-fix genuine change cost a NO_SOCKET retry. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
JC-000 added a commit
that referenced
this pull request
Aug 12, 2026
The benchmark tables predated three merged changes and one open fix, and several status claims had gone stale. Measured numbers only — no extrapolated figures. **e2e wall-clock.** The 2026-07-20 campaign rows are kept as history and labelled as such; a new "Post-#74 e2e numbers" block records HEAD: device profile clock pre-#71 post-#71 post-#74 C64U onchip 48 MHz 51.0 s 125.4 s 44.6 s C64U onchip 64 MHz 39.7 s (unmeas.) 33.7 s U64E REU 48 MHz 82.1 s 161.0 s 82.1 s Both onchip rows land BELOW their pre-regression baselines and the REU row lands exactly AT it — a REU build cannot contain #69 (its change is inside .ifdef USE_NISTCURVES_ONCHIP), so the pair is a clean control showing #69 is a SPEEDUP at turbo, not a cost. The doc now states that explicitly, including why the sign is easy to get wrong: the profile's 1 MHz penalty exists only because REU DMA is cheap relative to the CPU down there, and inverts above the crossover. (I got this backwards during the campaign; recording the reasoning so the next reader doesn't.) **New: ip65 / stock-C64 wall-clock**, the first ip65 e2e figures we have — 36.0 min honest 1 MHz REU-less, with the phase breakdown, plus the accelerated runs. Notes that the verify stretch came in 1.4% off the T(f)=D+C/f prediction three orders of magnitude from where that model was fit, and that ip65's drain budget is byte-identically unchanged by #74 so the numbers stand at HEAD. **Corrected stale claims:** - "ip65 is NOT packaged: does not link" — it links (#68). Explains the SCRATCH_UNION lifetime argument and its guards, notes packaging it is now a live option since a stock C64 + RR-Net has no shipped PRG today, and demotes c64-nist-curves#54 from blocker to optional headroom. - The CRYPTO_COLD_SHADOW "1,662 B overflow, cfg relief exhausted" entry, same fix. - The X25519-sibling entry claimed the old BSS overflow. Re-measured 2026-07-29: USE_X25519_SIBLING=1 under ip65 still fails, but on a DIFFERENT problem — X25519_RODATA over CRYPTO_OVERLAY by 2,048 B and LIB_NISTCURVES_P256_CODE over CRYPTO_RESIDENT by 103 B, i.e. code/rodata placement (ip65's overlay slot is 4,212 B vs UCI's 7.5 KB), not BSS. Better to state the measured failure than leave a fixed one on the page. **New design note** for the post-ServerHello drain: the ip65 property that motivates it (no MSS in SYN + ACK-only-when-polled), the offline-verify failure signature it prevents, why the budget must be per-backend (~40 ms per UCI net_poll vs a cheap ip65 pump — the #73 regression), current values, and the two open follow-ups (in-crypto polling for large flights; a wall-clock/idle bound instead of an iteration count, which is what the section's own rule actually demands). **New Smoke-tests subsection** for the hardware-free VICE ip65 rig, with the two prerequisites that are easiest to lose: the patched ethernet-capable VICE (stock macOS builds gate pcap on geteuid()==0) and the /dev/bpf permissions that reset every reboot. Also replaces the "blocked on an upstream ip65 bug (see lost memory note)" line with what is actually known now. Stacked on fix/drain-backend-budget: the post-#74 rows describe that PR's tree, not master's. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This was referenced Aug 13, 2026
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes#67. Under
USE_NISTCURVES_ONCHIP,fe_mul's multiply-row fetch nowgoes through a new
fe_gen_mul_rowstub — the exact fp256gen_mul_rowpattern: SMC-patch the sibling
og_common'sog_src_ldoperand tomul_src2_buf(already fe_mul's absolute staging buffer),X=31, delegate.The
reu_fetch_mul_rowimport is compiled out under onchip so the X25519path cannot silently regress to the REU-DMA fetch.
Scope note:
fe_sqrhas no REU dependency (quarter-square tables only) —exactly one call site changes.
Design decision (og_common, not a CT generator): the in-tree fe25519
path is already non-CT on secret data (zero-skip on
src1[i]/src2[j]bytevalues, data-dependent carry branches, mult66 sign branch), so a CT row
generator would not restore constant time — it would only cost ~2x.
og_common's staged-entry contract (nonzero staged bytes + diagonal) matches
fe_mul's
beqread set exactly. Cost: ~+1.8 min per scalarmult at stock1 MHz (analytical; ~3.2K cy/row on-chip vs ~600 DMA).
Rides along:
tools/test_x25519.py--slowdriver fix — the RFC 7748vectors have been failing at HEAD on every build (zero coverage): the driver
fed raw scalars but
x25519_scalarmultdoesn't clamp (production clamps viax25519_clampintls_ecdh.sfirst). Proven byte-exact that the ladder wascomputing the correct product for the unclamped scalar; the driver now
clamps, mirroring production order.
Validation
With #68 (ip65 refit) this completes the path to a genuinely REU-less
stock-C64 PRG.
🤖 Generated with Claude Code