Repository navigation
crypto: RSA Montgomery multiplication with mulx and two carry chains (parity R5) - #1014
Merged
Merged
Conversation
Collaborator
Author
Review of #1014, round 1 (head 04e784c, R5)What I read and did not run
Architecture
The kernel, checked
Dispatch
FindingsBLOCKER
Before landing (orchestrator)
SUGGESTION
PR body (orchestrator)
Verdict: SEND BACK (items 1–2, wording). After those, this PR is fit to LAND once the metal run and an ADX host test pass. |
M2 on both benches put a dual-carry-chain Montgomery row at 0.40x (T14) and 0.43x (AMD bench) of the mulx-and-addcarry intrinsics form for a 2048-bit modulus, and RSA-2048 verify was 2.14x ring on the T14 at #1000, its exponentiation the remaining cost. The kernel is one asm! block per word of b: a product row and a reduction row over the modulus's words, each four words per loop step, with mulx for the products and the low and high halves added on two independent carry chains, CF through adcx and OF through adox (Intel SDM Vol. 2A). The loop counts with lea and jrcxz, which leave both flags alone. Rust keeps the outer loop and the final subtraction; all RSA data is public. CPUID leaf 7 EBX bit 19 now admits ADX beside BMI2, and the hosted x86-64 dispatch takes the kernel where both are present; elsewhere the portable CIOS multiplication runs, now `mul_portable`. Keys use whole groups of four words: the zero words above a modulus only enlarge R. A unit test holds the dispatched multiplication to the portable one for five modulus lengths and the largest residues; it compares two runs of the portable code on CPUs without ADX, and so checks the kernel only on x86-64 hosts that have it. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PXEk8BBMDT8tXu4AnENB9Q
- The kernel's and both dispatchers' contracts say what they return: out and the top word are a b / R mod m plus at most one m, below 2m and unreduced; the caller subtracts. - The equivalence test names the word counts it covers. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PXEk8BBMDT8tXu4AnENB9Q
Japabu
force-pushed
the
wt/toyos-crypto-perf-7
branch
from
October 22, 2026 14:09
04e784c to
9b76272
Compare
The equivalence test prints CPUID leaf 7's BMI2 and ADX bits and whether the instruction path ran, so a run under --nocapture on an x86-64 host shows that it compared the kernel and not the portable code with itself. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PXEk8BBMDT8tXu4AnENB9Q
Collaborator
Author
Review of #1014, round 2 (head 79fbb43, R5)What I read and did not run
ArchitectureUnchanged from round 1, where it was accepted. The one Round-1 blockers
The ADX evidence
FindingsBLOCKER
SUGGESTION
PR body (orchestrator)
Verdict: SEND BACK (finding 1, a deletion of eight lines; everything else is ready to land). |
- The test reads no CPUID: whether rsa_mul answers already says which path ran, and the reading belongs in an arch module. Its probe is zero operands, inside the kernel's contract. - rsa_mul's contract says when it answers None. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PXEk8BBMDT8tXu4AnENB9Q
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 free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Based on main 18a3873 (after #993 and #1015). Two commits: 652b3d0 (round 0, 04e784c rebased) and 9b76272 (round 1).
Problem
On the T14 at #1000, RSA verify was 2.0–2.4x ring: RSA-2048 PKCS#1 52.5 us against 24.5. M9 put key construction at 13.7 us and the exponentiation at 38.7 us; both are Montgomery multiplications, 6 and 18 of them.
M2 measured this kernel's shape,
mulxwith two carry chains, at 0.40x (T14) and 0.43x (AMD bench) of themulx+_addcarry_u64intrinsics form for a 2048-bit modulus. LLVM emits noadcx/adoxfrom Rust, so this is the one place the analysis named whereasm!is justified (R5).What changes
arch/x86_64/montgomery.rs. Oneasm!block per word of b: a product row and a reduction row over the modulus's words, four words per loop step.mulxforms the products. Their low and high halves add on two independent chains, CF throughadcxand OF throughadox(Intel SDM Vol. 2A).leaandjrcxz, which leave both flags alone.mul_portable.Metal
The combined T14 boot (ec01b15, which carries this kernel unchanged) passed with judge exit 0.
crypto_parityverified ring's RSA signatures through the kernel before timing them. Ours over ring:At #1000 these were 2.14 / 1.99 and 2.41 / 2.30.
Review round 1 (9b76272)
cargo run -- --ci hostexit 0, 93 green; crate clippy (host and--target x86_64-apple-darwin) and crate tests in the three forms, exit 0. Log: https://github.com/ToyOSOrg/evidence/tree/847d357932fe05252d2d4a39f8c507bcd3c5580d/pr-1014/r2Round 2 (79fbb43)
instruction_path_matches_the_portable_multiplicationprints whetherrsa_mulanswered, that is whether the instruction path ran. A--nocapturerun then shows that it compared the kernel, not the portable code with itself. Under Rosetta it prints the portable path. The test reads no CPUID; its probe uses zero operands, inside the kernel's contract.rustls-providertests. The library, the kernel and this test are unchanged.rsa::tests::instruction_path --nocapture.Checks (round 0)
All at 04e784c. Logs: https://github.com/ToyOSOrg/evidence/tree/ce88e7308dadb4e4909cdad09cf839d4832d2d3c/pr-1014/r1
cargo run -- --ci host: exit 0, 93 steps green. The development Mac is aarch64, so these runs test the portable path.-D warnings): exit 0 in the three CI forms for the host and for--target x86_64-apple-darwin, and forx86_64-unknown-uefi. Crate tests: exit 0 in the three forms.crypto_instrument_prints_its_table(QEMU x86_64): exit 0. QEMU'sqemu64model has no BMI2, so it runs the portable path.emulate.pyin the evidence; scratch, never lands). It interprets the exactasm!template from the source file, with the SDM's semantics formov,xor,mulx,adcx,adox,imul,lea,jrcxzandjmp. Over whole multiplications it checks a·b/R mod m, below 2m:adoxturned intoadcx, and the OF carry into t[n + 1] dropped. So does a misplaced store.x86_64-unknown-toyos,[profile.toyos]): 163 lines, 12mulx, no call. The register allocation preservesrbx, which it chose forlo.instruction_path_matches_the_portable_multiplication: five modulus lengths (2048 to 4096 bits, three of them not multiples of 256), 64 cases each, the largest residues included. It runs the kernel only on x86-64 hosts with ADX.Measurements
Readback staged at 04e784c (
crypto_instrument,cryptocase; exit 2 = Staged). The RSA rows run the kernel on both benches. Before timing,crypto_paritychecks that each row's verification succeeds on ring's signatures.Risks
adcx/adox. The T14 boot above is its first execution. On an ADX host,instruction_path_matches_the_portable_multiplicationpassed on Linux on an AMD Ryzen 7 4800U (run, binary hash and build recipe). It printedbmi2 1 adx 1and the kernel path. The binary is the musl unit-test build at 79fbb43, with three C-backed dev-dependencies removed so that it builds on the Mac (musl-devdeps.patch). The library and test source are unchanged. That test covers 64 cases per word count, including the largest residues; it is the check that would catch a carry error on rare inputs. The run predates 720756f, which removed the test's CPUID print and changed only its probe's operands.x86_64-apple-darwinrun takes the kernel path. Rosetta's CPUID reports no leaf-7 feature: no BMI2, ADX or AVX2 (cpuid-rosetta.log). So--target x86_64-apple-darwintests take the portable path, and no Rosetta run exercises this kernel. This corrects my earlier finding, recorded in crypto: a 5x51 field with typed bounds and erasure at the boundary (parity items 1 and 5) #992's evidence, that Rosetta reports ADX. That probe was built with-C target-feature=+bmi2,+adx, which makesis_x86_feature_detected!a compile-time constant.🤖 Generated with Claude Code
https://claude.ai/code/session_01PXEk8BBMDT8tXu4AnENB9Q