feat(cart): Phase 5 cart I/O — PI domain regs, save backends, SI joybus, retail HLE boot - #117
Merged
Conversation
Phase 5 (cart boot + saves), Sprint 1 — the first of the PI-completion work. Implements PI_BSD_DOM1/DOM2 LAT/PWD/PGS/RLS (0x0460_0014..0x0460_0030): each stores the written value masked to its field width (LAT/PWD 8-bit, PGS 4-bit, RLS 2-bit) and reads it back, for DOM1 and DOM2 independently. IPL2 programs DOM1 from the ROM header (LAT 64 / PWD 18 / PGS 7 / RLS 3 for official ROMs), so a game that reads them back must see what it wrote. Deriving DMA duration from these (the non-instant completion) is the follow-up step that reads the stored fields; open-bus on unmapped PI reads and the retail HLE boot are the rest of Sprint 1. Unit-tested (store/read-back per field + width masking + DOM1/DOM2 independence). Gates: fmt, clippy (-D warnings), cargo test -p rustyn64-cart — green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…r Pak) Phase 5 (cart boot + saves), Sprint 1 — the save subsystem. - New `rustyn64-cart::save::SaveDevice`: SRAM (flat 32 KiB, PI DOM2), FlashRAM as its real erase/program/status state machine (CIR at 0x0801_0000; ReadArray / Status / SiliconID / LoadPage modes; program is a bit-clearing AND, so a page must be erased to all-ones first — a flat store would mis-save), EEPROM 4k/16k (joybus 8-byte blocks), and the Controller Pak (joybus 32-byte blocks). - Wired into `Cart`: SRAM/FlashRAM route through pi_read/pi_write + a word-level pi_write_word for the FlashRAM CIR; the joybus backends expose block accessors for the SI/PIF module (Sprint 2). `Cart::save()` returns the persistable bytes. - The Bus's direct-I/O PI write now persists to the save (via pi_write_word), not just latches for read-back timing — so a `SW` to SRAM/FlashRAM actually stores. Tests: SRAM/EEPROM/Controller-Pak round-trips, the FlashRAM erase→load→program sequence, and program-only-clears-bits (rustyn64-cart); a Bus-level SRAM direct-I/O round-trip through the CPU path (rustyn64-core). docs/cart.md + CHANGELOG. Also this sprint: the PI BSD domain-timing registers (prior commit). Gates: fmt, clippy (-D warnings), workspace test, no_std, markdownlint — all green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Phase 5 (cart boot + saves), Sprint 2 — the SI/PIF/joybus transport. - New `rustyn64-cart::pif`: the 64-byte PIF RAM + the joybus frame executor. It parses the per-channel handshakes (TX RX cmd.. resp..) and runs 0x00/0xFF info, 0x01 controller state, 0x02/0x03 Controller-Pak accessory access (with the data CRC8, seed 0x00 poly 0x85), and 0x04/0x05 EEPROM blocks. Channels 0-3 are controller ports (fed by the Bus's packed port words), channel 4 the cart bus (EEPROM). The `Pif` lives in the `Cart` alongside the save device. - Bus SI block at 0x0480_0000: SI_DRAM_ADDR, SI_PIF_AD_RD64B/WR64B, SI_STATUS; the 64-byte PIF↔RDRAM DMA (WR64B loads the frame, RD64B executes it + copies the replies out), and MI_INTR.si. The PIF-RAM window (0x1FC0_07C0) is CPU- addressable for direct access. Tests: joybus command decode + EEPROM/Controller-Pak round-trips (rustyn64-cart); a Bus end-to-end controller read through SI DMA → PIF execute → RDRAM, with the SI interrupt (rustyn64-core). docs/cart.md + CHANGELOG. Gates: fmt, clippy (-D warnings), workspace test, no_std, markdownlint — all green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 21 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (9)
📝 WalkthroughWalkthroughThe cartridge subsystem adds four save backends, FlashRAM command handling, PIF joybus execution, SI DMA and interrupt routing, PI BSD timing registers, retail HLE boot support, integration tests, and updated documentation. ChangesCartridge and peripheral emulation
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant CpuBus
participant Bus
participant Cart
participant Pif
participant SaveDevice
CpuBus->>Bus: write SI DMA command
Bus->>Cart: transfer PIF RAM
Cart->>Pif: execute joybus frame
Pif->>SaveDevice: access EEPROM or Controller Pak
SaveDevice-->>Pif: return save data
Pif-->>Cart: write reply bytes
Cart-->>Bus: transfer reply to RDRAM
Bus-->>CpuBus: assert MI_INTR.si
🚥 Pre-merge checks | ✅ 7 | ❌ 3❌ Failed checks (3 warnings)
✅ Passed checks (7 passed)
Comment |
Phase 5 (cart boot + saves), Sprint 1 — `rom::hle_boot`. Boots a retail ROM by seeding the machine state IPL3 leaves and running the cart's *real* IPL3 (rather than reimplementing the boot copy/jump): - Inject the per-CIC seed (6101/6102/6103/6105/6106, from cen64) into PIF RAM 0x24, whence the boot reads the s3-s7 GPRs. - COP0 Status = 0x3400_0000, Config = 0x7006_E463; s3-s7 = rom_type 0 / tv NTSC / reset cold / seed byte / 0; PI DOM1 timing from the ROM header's first word. - Copy the real IPL3 (ROM 0x40..0x1000) into RSP DMEM and set PC = 0xA400_0040; IPL3 then copies the game to RDRAM and jumps to the header entry. An ELF-payload ROM (n64-systemtest) is still handed to seed_ipl3_handoff. The algorithm follows simple64's bootrom_hle.c (GPLv3 — studied, not copied). A committable unit test asserts the boot seeds the DMEM copy / s6 seed / PIF-RAM seed / PI DOM1 / PC; running IPL3 to a game is the local commercial capstone. Gates: fmt, clippy (-D warnings), workspace test — all green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
doublegate
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/cart.md (1)
19-38: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Interfacespseudocode is stale against this PR's ownCartadditions.
Cart's synopsis here still shows onlyload/header/save/tick, but this PR addspif_ram,pif_load,pif_read,pif_write,pif_execute,save_device_mut, andpi_write_wordto the realCartincrates/rustyn64-cart/src/lib.rs. As per path instructions, "Docs are the SPEC here, not a changelog... A behaviour change with an untouched spec is a finding" — update the interface listing in the same commit that changes the state/behaviour sections below it.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/cart.md` around lines 19 - 38, Update the Cart interface synopsis in the docs to include the newly exposed pif_ram, pif_load, pif_read, pif_write, pif_execute, save_device_mut, and pi_write_word methods, matching their actual signatures and public behavior. Keep the existing load, header, save, and tick entries intact so the interface listing fully reflects Cart’s current API.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@CHANGELOG.md`:
- Around line 13-30: The cart subsystem changelog entry must state the measured
effect on n64-systemtest’s failing-assertion count. Update the “Added — cart
(Phase 5, Sprint 1)” section with the observed before/after delta, or explicitly
state that n64-systemtest was not measured.
In `@crates/rustyn64-cart/src/lib.rs`:
- Around line 26-27: Add rustdoc comments for the public module declarations pif
and save in the crate root, describing each module’s purpose. Keep the existing
module declarations and their contents unchanged.
In `@crates/rustyn64-cart/src/pi.rs`:
- Around line 207-210: Update the comment in the Pi register-handling branch
near addr to remove the claim that timing registers affect DMA duration via
transfer_cycles. Document only the implemented masked write/readback behavior,
and state that transfer-duration modelling remains a follow-up consistent with
the existing note near the initialization logic.
- Around line 158-160: Add provenance for both hardware assertions: at
crates/rustyn64-cart/src/pi.rs lines 158-160, cite authoritative evidence for
the 0x7F length-register readback or explicitly label the zero return as an
unverified simplification; at lines 363-367, cite the source or add an
accuracy-ledger entry for the 64/18/7/3 timing tuple with its evidence status.
If either claim is only a fixture or simplification, remove the hardware
attribution.
- Around line 52-55: Add the missing public PI_BSD_DOM2_PWD and PI_BSD_DOM2_PGS
constants at the decoder-supported addresses between PI_BSD_DOM2_LAT and
PI_BSD_DOM2_RLS, then update the DOM2 independence test to assert both registers
independently.
In `@crates/rustyn64-cart/src/pif.rs`:
- Around line 219-232: Update the channel-4 EEPROM handlers for commands 0x04
and 0x05 in the PIF command dispatch to verify that save is SaveDevice::Eeprom
before reading or writing. For non-EEPROM devices, call the existing
mark_no_device path and avoid returning data or success status; preserve the
current read/write behavior for present EEPROM devices.
In `@crates/rustyn64-cart/src/save.rs`:
- Around line 94-99: Validate the page-derived sector base in the 0x4B command
handling before storing it in erase_sector, ensuring only bases within the flash
array are accepted, or add an equivalent bounds guard in the 0x78 sector-erase
execution path before slicing. Preserve valid sector erases and prevent
out-of-range guest page indices from reaching self.array[base..end] in Execute.
- Around line 138-141: Update the FlashMode::Status branch in the status read
handling to return self.status for every odd byte offset, while returning 0 for
even offsets. Preserve the existing repeated 00/status pattern and extend flash
status coverage to verify offsets such as 1, 5, and 7 in addition to 3.
- Around line 199-206: Update SaveDevice::pi_read to return Some only for
addresses within the save device window: bound-check SRAM access and gate
FlashRAM access so out-of-window reads return None. Also update Cart::pi_read to
consult self.save only when the address is within the existing
SAVE_PI_BASE..ROM_PI_BASE range, matching Cart::pi_write; apply the
corresponding changes in crates/rustyn64-cart/src/save.rs:199-206 and
crates/rustyn64-cart/src/lib.rs:340-349.
---
Outside diff comments:
In `@docs/cart.md`:
- Around line 19-38: Update the Cart interface synopsis in the docs to include
the newly exposed pif_ram, pif_load, pif_read, pif_write, pif_execute,
save_device_mut, and pi_write_word methods, matching their actual signatures and
public behavior. Keep the existing load, header, save, and tick entries intact
so the interface listing fully reflects Cart’s current API.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 85f2295f-0a46-4d8c-9a94-6677d7733a69
📒 Files selected for processing (7)
CHANGELOG.mdcrates/rustyn64-cart/src/lib.rscrates/rustyn64-cart/src/pi.rscrates/rustyn64-cart/src/pif.rscrates/rustyn64-cart/src/save.rscrates/rustyn64-core/src/bus.rsdocs/cart.md
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/rustyn64-test-harness/src/rom.rs`:
- Around line 207-228: Document the CIC seed values and HLE retail-boot
assumptions used by cic_seed and the surrounding HLE boot flow in
docs/accuracy-ledger.md. Include measurement details, falsification criteria,
and the measured impact on the n64-systemtest assertion, replacing reliance on
the cen64 and local simple64 references as unsupported provenance.
- Around line 461-464: Extend the DOM1 header assertions in the ROM test to
validate the encoded RLS value of 3 using the PI_BSD_DOM1_RLS register symbol,
alongside the existing LAT, PWD, and PGS checks. Ensure the test fails if the
DOM1 RLS write is removed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 5ba2e22a-5c5f-40b7-811f-a9cf02bddc8f
📒 Files selected for processing (1)
crates/rustyn64-test-harness/src/rom.rs
Address all 11 CodeRabbit comments on PR #117 and add the Phase 5 commercial-boot capstone. Two were guest-reachable defects: - CRITICAL (save.rs): FlashRAM sector-erase page index was unbounded (`page & 0xFFFF`), so a page >= 1024 made `base > array.len()` and the 0x78 Execute panicked on `array[base..end]`. Clamp the sector to the last valid one, matching the 0xA5 Page-Program guard. - CRITICAL (save.rs/lib.rs): `SaveDevice::pi_read` returned `Some` for every address on an Sram/Flash cart, so `Cart::pi_read` never fell through to ROM — a game could not read its own cartridge. Gate the save window to `SAVE_PI_BASE..ROM_PI_BASE` before consulting it. Also adopted: - FlashRAM status now sits at every odd offset (1,3,5,7...), per the n64brew Flash burst pattern, not only offset 3. - Joybus EEPROM read/write (0x04/0x05) now gate on device presence, so a non-EEPROM cart replies RX_NO_DEVICE instead of 8 silent zero bytes. - Expose PI_BSD_DOM2_PWD/PGS and cover all DOM2 fields in the independence test; assert DOM1 RLS in the HLE-boot test. - rustdoc on `pub mod pif/save`; cite the PI 0x7F readback + official-ROM timing tuple (N64brew); drop the false DMA-duration comment. Docs/ledger: - C-32 records the HLE boot state + per-CIC seeds as cited constants (N64brew CIC-NUS/PIF-NUS, cen64), not tuned; simple64 studied only. - R-18 characterises the commercial-boot capstone honestly: a retail ROM boots and executes real code (~10^8 retired instructions) but does not yet reach video (VI/RI/F3DEX OS-boot runtime, out of Phase 5 scope). - CHANGELOG states the measured n64-systemtest effect: 93 -> 90 (-3). The capstone test (`commercial_boot.rs`, local-only, #[ignore]d) asserts the achievable claim — boots and retires >= 10^6 instructions without panicking — and reports the lit-pixel count rather than asserting video. Gates: fmt, clippy -D warnings, cart/core/harness tests, rustdoc -D warnings, markdownlint, no_std build all green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The retail boot strategy (HLE default, real-PIF staged behind an off-by-default flag) is a cross-cutting decision that ledger entry C-32 already referenced. Record it as a proper ADR in Nygard format and point C-32 and the CHANGELOG at ADR 0009, closing the dangling reference. The HLE path ships now; the real-PIF path is documented as the Sprint-2 deliverable (needs the copyrighted PIF ROM, never CI-gated). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Antigravity review (Gemini via Ultra)This PR implements Phase 5 cartridge I/O, including PI BSD domain-timing register storage, four save backends (SRAM, FlashRAM, EEPROM, Controller Pak), the SI joybus subsystem with PIF RAM frame execution, and a retail HLE boot sequence. Blocking issues
Suggestions
Nitpicks
Automated first-pass review by |
This was referenced Jul 24, 2026
doublegate
added a commit
that referenced
this pull request
Jul 24, 2026
This was referenced Jul 24, 2026
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
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.
What
Phase 5 (cart boot + saves) — the cartridge I/O subsystems: PI completion, the four save backends, and the SI/PIF/joybus transport. (The retail HLE boot + the commercial-ROM capstone are follow-ups.)
PI completion
PI_BSD_DOM1/2LAT/PWD/PGS/RLS,0x0460_0014–0x0460_0030) store and read back with field-width masking (LAT/PWD 8-bit, PGS 4-bit, RLS 2-bit), DOM1/DOM2 independent.The four save backends (
rustyn64-cart::save::SaveDevice)0x0801_0000; ReadArray/Status/SiliconID/LoadPage modes; program is a bit-clearing AND, so a page must be erased first — a flat store would mis-save), EEPROM 4k/16k (joybus 8-byte blocks), and the Controller Pak (joybus 32-byte blocks + the data CRC8).Cart::pi_read/pi_write(+ a word-level path for the FlashRAM CIR); the Bus's direct-I/O PI write now persists to the save instead of only latching.The SI joybus (
rustyn64-cart::pif+ the Bus SI block at0x0480_0000)SI_DRAM_ADDR,SI_PIF_AD_RD64B/WR64B), the 64-byte PIF↔RDRAM DMA, and the SI interrupt. The joybus frame executor runs0x00/0xFFinfo,0x01controller state (from the Bus's four packed port words),0x02/0x03Controller-Pak access, and0x04/0x05EEPROM.WR64BDMA to PIF →RD64Bexecutes the handshakes → replies DMA'd back — verified by a Bus integration test.Tests / gates
rustyn64-cart: PI domain registers, each save backend's round-trip (incl. the FlashRAM erase→load→program sequence + program-only-clears-bits), joybus command decode, EEPROM/Controller-Pak joybus round-trips.rustyn64-core: a direct-I/O SRAM round-trip through the CPU path, and the end-to-end SI joybus controller read.cargo fmt --all --check,cargo clippy --workspace --all-targets -- -D warnings,cargo test --workspace,RUSTDOCFLAGS="-D warnings" cargo doc, theno_stdcross-build,markdownlint— all green.Next (follow-ups)
The retail HLE boot (+ CIC seed), PI open-bus / non-instant DMA, and the local commercial-ROM boot capstone land in follow-up PRs toward the v0.6.0 release.
🤖 Generated with Claude Code
Retail HLE boot (added)
rom::hle_bootseeds the state IPL3 expects (COP0 Status/Config, the s3–s7 GPRs from the per-CIC seed injected into PIF RAM, PI DOM1 timing from the header) and copies the cart's real IPL3 into RSP DMEM, jumping to0xA400_0040— IPL3 then copies the game to RDRAM and jumps to the header entry. Algorithm from simple64'sbootrom_hle.c(GPLv3 — studied, not copied). Unit-tested for the seeded state; running IPL3 to a game is the local commercial capstone.