Dev - #8
Merged
Merged
Conversation
Root-caused via direct sail_riscv_sim tracing (--trace-reg): our config declared REPORT_VA_IN_MTVAL/STVAL_ON_LOAD/STORE_AMO_MISALIGNED as false, so the Sail reference model expected mtval/stval=0 on a misaligned load/store trap. core.sv has always reported the real faulting address there instead (mem_paddr, per its own trap_val mux), so every one of ExceptionsZc-00's ~40 misaligned-access checkpoints failed identically on that one mismatch -- invisible in a signature diff since mtval isn't part of the RVTEST_SIGUPD region at all. Flipped both config files to declare true, matching what the RTL has always actually done. Regenerated the full ACT4 suite and confirmed: ExceptionsZc-00 no longer fails -- it now hits the same, already- documented EBREAK-sim-halt gap U-00 already shows (the test's one c.ebreak checkpoint halts sim before it reaches its own pass marker), not a new problem. Full regression clean.
Milestone 1 of the combined bus-error+CLINT-interrupts+Zifencei plan, done first since it's a real correctness fix to commit_now/the FSM itself, not just a missing feature: icache.sv/dcache.sv pair ack_o with err_o on a downstream error (a compromise from when core.sv had no wb_err_i consumer at all), so through the real cache path an AMO whose read phase errors would have satisfied wb_ack_i && is_amo_rmw and incorrectly proceeded into S_AMO_WRITE, issuing a bogus second bus write. Introduces wb_done (cycle terminated, ack or err) and wb_ok (terminated cleanly) to replace every bare wb_ack_i use in the FSM/ wb_master_drive, fetch_fault_q to carry a faulted fetch through to S_EXEC as an inert placeholder instruction (reusing c_expand_illegal's same trick), and mem_load_access_fault/mem_store_access_fault mirroring mem_load_misaligned/mem_store_misaligned's exact is_load/is_lr/ is_amo_rmw split. An AMO write-phase fault must read amo_addr_q rather than the by-then-repurposed mem_paddr for trap_val -- proven by a dedicated mock-slave test with operands chosen to make that exact bug class (mtval=105 instead of the real address) unmistakable. Flips the matching 6 UDB REPORT_VA_ON_*_ACCESS_FAULT config flags in the same change (both quantiumv-rv64im.yaml and sail.json), avoiding a second round of the ExceptionsZc-00 config/RTL mismatch from earlier today. No ACT4 test vectors exercise access faults yet, confirmed by a full regen+rerun landing byte-identical to the pre-change baseline. 35 new checks across 3 new testbenches, full regression (45/45) and verilator lint clean.
Code review of yesterday's bus-error-trapping commit (3-agent pass: cross-file tracer, reuse/simplification/efficiency, altitude) found a real bug: reservation_valid_q/reservation_addr_q were set on `commit_now && is_lr` with no !trap_taken guard, unlike reg_write/ csr_we which are both correctly gated. is_lr is purely decode-based and doesn't care whether the bus transaction actually succeeded, so a faulted LR (misaligned, or a real bus-access fault) still created a valid reservation for a load that never happened -- a subsequent SC to that address would then spuriously report success. The bus-access-fault trigger path is newly reachable specifically because of the bus-error-trapping commit: previously a faulting LR through a direct slave just hung forever on the then-unconsumed wb_err_i, never reaching commit_now at all. Fixed by adding && !trap_taken to the set condition, matching the existing reg_write/csr_we precedent. New test (core_reservation_fault_tb.sv) proves it: a faulted LR immediately followed by an SC to the same address must report failure (rd=1), not the untouched sentinel a spurious reservation would produce -- verified in both directions by reverting the fix locally and confirming the test fails with exactly the predicted value before restoring it. Also addresses two lower-severity findings from the same review: the amo_write_entered monitor duplicated across two new testbenches is now a shared module (state_reached_monitor.sv, distinct from the existing pc_trigger_sample_monitor.sv since the trigger shape genuinely differs), and trap_val's ternary chain is flattened back to its established single-level style via a named mem_access_fault_addr wire. Full regression (46/46) and verilator lint clean.
Milestone 2 of the combined bus-error+CLINT-interrupts+Zifencei plan. FENCE.I encoding/decode (design/defaults/instructions_and_masks.sv, instruction_codes.sv, decoder.sv, same zero-operand shape as ECALL/ EBREAK/MRET/SRET/WFI), a new core.sv output port icache_flush_o (commit_now && is_fence_i, timing provably clean since FENCE.I retires purely within bus-idle S_EXEC on this single-issue core), a new flush_i input on icache.sv (bare valid_q<='0, deliberately not folded into the reset arm since nothing is ever in flight to unwind when it fires), and cache_complex.sv passing flush_i through to icache0 UNCONDITIONALLY -- gating it on ifetch_i (low throughout S_EXEC) would have silently made FENCE.I a permanent no-op. soc.sv wires icache_flush_o -> cache0.flush_i and marks its own "KNOWN, DELIBERATE LIMITATION: no I$/D$ coherence" header comment closed. design/icache_tb.sv extended with a direct flush_i proof (install a line, flush, confirm a genuine re-miss). New testbench/core_fence_i_tb.sv: a plain retire/no-op check, plus the real proof -- a genuine self-modifying-code sequence through the real cached path (core_cache_harness.sv): call a target once (populates I$ with old bytes), D$-overwrite its first instruction, fence.i, call again -- the destination register's final value can only be the new instruction's result if the fetch genuinely bypassed the stale I$ line. Verified in both directions: broke the flush_i wiring locally and confirmed the test fails with exactly the predicted stale value before restoring it. Full regression (47/47, up from 46) and verilator lint clean.
Milestone 3 of the combined bus-error+CLINT-interrupts+Zifencei plan. Zero core.sv/soc.sv changes -- bus wiring and interrupt consumption are later milestones. mtime free-runs from 0 (same precedent as csr_file.sv's mcycle_q); mtimecmp resets to all-ones, not zero, so mip.MTIP doesn't read pending before software programs a real deadline; byte-lane writes and registered 1-wait-state ack match uart_tx.sv's own house style exactly. Built and adversarially reviewed via a Workflow (3 independent reviewers: Wishbone protocol/timing, RISC-V CLINT spec-conformance, test-coverage). Zero RTL correctness bugs found this time -- the first milestone this session where independent review came back clean on the actual design, likely because the RTL sticks close to the plan's own pre-validated pseudocode and existing uart_tx.sv/ wb4_sram.sv precedent rather than inventing new shapes. Real findings were in the new testbench: an unbounded mtip_o wait loop that would have hung forever against a genuinely broken comparator instead of failing cleanly (fixed with a bounded loop, verified by temporarily forcing mtip_o stuck low and confirming a clean failure instead of a hang before restoring the real RTL); a missing NBA-settle delay matching wb_driver.sv's own documented fix for the same race class; a handful of coverage gaps (full 8-byte write, an already-passed deadline, mtime-write-is-a-no-op); and a second-order timing bug in the pre-existing ack/err test that one of those new tests happened to expose (an implicit ordering dependency on ack having already cleared, now made explicit). 14/14 clint_tb.sv checks, full regression (48/48) and verilator lint clean.
wb_addr_decoder.sv generalizes from a 1-bit sel_uart (RAM/UART) split to
a 2-bit {addr_i[16], addr_i[15]} test routing to RAM/UART/CLINT, with the
old 1-bit sel_uart_q latch generalized to a registered 2-bit target_t enum
(TARGET_RAM/TARGET_UART/TARGET_CLINT) muxing ack_o/err_o/dat_o. soc.sv
instantiates clint0 off the decoder's new clint_* port group, sitting
outside cache_complex for the same reason uart0 does: mtime free-runs
every cycle and the CPU never writes it, so a cached read would freeze
forever with nothing to invalidate it. clint0.mtip_o has no consumer yet
(Milestone 6 wires it to core0.i_mtip) -- wrapped in a lint_off/on
UNUSEDSIGNAL pragma.
New testbench/decoder_clint_harness.sv wires a real decoder + real
wb4_sram/uart_tx/clint (no core.sv) for a bus-level firmware test,
independent of any interrupt-taking logic. design/wb_addr_decoder_clint_tb.sv
drives it: real mtime monotonic-increase and mtimecmp write/read-back
checks, plus RAM/UART-unaffected regression checks.
Built and adversarially reviewed via a Workflow (implementation +
3 independent reviewers). Zero RTL/wiring bugs found this time -- all
findings were in documentation and test coverage: added symmetric
cyc_o-stays-low checks that a couple of the new fake-slave tests were
missing, closed the two untested ordered target-pair transitions
(RAM->CLINT, CLINT->UART) in the round-trip sequence, added top-of-window
boundary checks for all three targets, corrected decoder_clint_harness.sv's
header (it omits cache_complex, so its RAM leg only proves decoder<->SRAM,
not decoder<->cache<->SRAM), and documented CLINT's address-aliasing gap
above its 2-register footprint as an accepted limitation, same class as
wb4_sram.sv's own bounds check being RAM's only backstop.
wb_addr_decoder_tb.sv: 25/25 (up from 17). wb_addr_decoder_clint_tb.sv:
8/8. Full regression 49/49, full-SoC verilator lint clean.
New i_mtip input (ANSI default = 1'b0, load-bearing: core.sv's own
instantiation doesn't connect it yet, and an unconnected input feeding
data rather than a condition would otherwise float to X). mip's bit 7
(MTIP) becomes derived, not stored: mip_effective = {mip_q[63:8], i_mtip,
mip_q[6:0]} feeds the CSR_ADDR_MIP/CSR_ADDR_SIP read-mux arms and a new
o_mip export, never raw mip_q. Both mip_q write arms (direct mip write,
and the sip-delegation-derived write) mask bit 7 out of storage so a
stale value there can never resurface. Four more control-plane exports
(o_mie, o_mideleg, o_mstatus_mie, o_mstatus_sie) mirror the existing
o_mtvec/o_medeleg idiom, for Milestone 6's interrupt-taking logic.
Built and adversarially reviewed via a Workflow (implementation + 3
independent reviewers). RTL correctness review came back clean. The
other two reviews were novel this session in actually proving their
findings via mutation testing, not just static reasoning:
- Test-coverage review found the "all-1s write to mip doesn't stick at
bit 7" test proved nothing about write-arm masking -- mip_effective
overrides bit 7 on every read path regardless of what's in storage, so
removing the write mask entirely still passed the full suite
unchanged (verified by literally stripping the mask and rerunning).
Same gap existed for all 4 non-mip export ports, which were wired into
the testbench but never actually asserted against (verified by
corrupting one export's bit index and getting an identical clean
pass). Fixed: corrected the misleading test comment, added two
narrowly-scoped dut.mip_q hierarchical peeks (the only way to actually
observe this one piece of RTL -- a deliberate, one-time exception to
this testbench's otherwise-strict black-box discipline), and added
checks for all 4 previously-dangling export wires at points in the
existing sequence where their expected values are already known.
Re-verified via the same mutation: reran against a deliberately
unmasked copy of csr_file.sv and got exactly 2 new failures, both the
newly-added checks, nothing else.
- Regression/lint review found a real regression against this project's
standing verilator lint gate (confirmed via git-stash bisection: clean
on the pre-Milestone-5 commit, 5 fatal PINMISSING warnings after) --
core.sv's untouched csr_file0 instantiation left the 5 new output
ports unconnected. Fixed with a narrow, deliberate exception to this
milestone's "zero core.sv changes" scope: 5 explicit empty port
connections (not Milestone 6's real wiring), wrapped in a
lint_off/on PINCONNECTEMPTY pragma (empty connections trade
PINMISSING for PINCONNECTEMPTY, same underlying fact) -- same
precedent as soc.sv's clint_mtip.
csr_file_tb.sv: 81/81 (up from 74). Full regression 49/49 unchanged,
full-SoC verilator lint clean.
…tone 6 The final milestone of the combined bus-error-trapping + CLINT-timer- interrupts + Zifencei plan. QuantiumV now delivers real machine-timer interrupts, not just synchronous traps. Interrupt-taking is a one-cycle-DEFERRED echo of commit_now (new commit_now_q register), checked at the moment a fresh S_FETCH begins -- deliberately not folded into commit_now's own trap_taken/next_pc mux. The deferral exists because checking interrupt-visibility at commit_now itself would read mstatus/mie/current_priv values that are one-edge- stale whenever the retiring instruction is what just changed them -- every MRET would otherwise fail to let an already-pending interrupt preempt the very first instruction it returns to, since M-mode trap-entry unconditionally clears mstatus.MIE and that clear is only visible starting the cycle after the trap-taking edge. A second register, fetch_redirect_q, holds the redirect stable across a multi-cycle I$ refill of the trap vector itself. mepc correctness needed zero new muxing: csr_file0's i_trap_pc(pc) connection stayed unchanged -- by the time interrupt_taken fires, pc has already been updated by the just-retired instruction's own commit to the architecturally-correct resume address, for every resume-address shape. New core.sv i_mtip input (ANSI default, keeps ~16 other core- instantiating files compiling unchanged), 5 new interrupt-eligibility wires (mti_pending/mti_to_s/mti_enabled/int_pending_and_enabled), interrupt_taken/interrupt_to_s, fetch_redirect_q/fetch_from_trap_vector, new arms on pc's and current_priv's always_ff, trap_vector's selector generalized to route synchronous exceptions and interrupts through the same mtvec/stvec mux. soc.sv wires clint0.mtip_o -> core0.i_mtip for real, closing out the CLINT integration started in Milestone 4. Built and adversarially reviewed via a Workflow (implementation + 4 independent reviewers: FSM-timing correctness, RISC-V spec correctness, test coverage via mutation testing, regression/lint). FSM-timing and regression/lint reviews came back clean. Two real, mutation-confirmed test-coverage gaps found by the other two reviews, both closed: - mideleg[7]=1 (S-mode interrupt delegation) was never exercised by any test -- confirmed by mutation (swapping mstatus_sie_w for mstatus_mie_w in mti_enabled's delegated branch passed the full suite unchanged). Fixed by adding a new Case 7 to core_interrupt_tb.sv: mideleg delegated, mstatus.SIE armed before MRET (MRET leaves S-side mstatus fields untouched, so SIE must already be set going in), confirming scause/sepc capture the interrupt and current_priv ends S. - i_trap_val's mux-to-0-on-a-pure-interrupt (a design decision resolved during planning, guarding against is_illegal_instr aliasing stale decode state on the interrupt_taken cycle) was never verified -- no test read mtval anywhere. Fixed with mtval_q/stval_q checks on the M-routed and new S-routed cases. Verified both new checks actually catch their target mutations: the mideleg mutation now fails exactly the 4 new Case-7 checks and nothing else; removing the i_trap_val mask makes Case 7's stval_q check fail with the literal MRET instruction encoding leaking through as stval -- empirically confirming the exact aliasing failure mode the design decision was meant to prevent. core_interrupt_tb.sv: 55/55 (up from 45). core_interrupt_icache_miss_tb.sv 7/7, soc_interrupt_tb.sv 5/5 unchanged. Full regression 52/52, full-SoC verilator lint clean. This closes the entire 6-milestone plan.
iverilog cannot compile any of taxi's AXI4/AXI4-Lite IP -- its entire product line (59 of 63 files) is built on SystemVerilog interface+ modport ports, confirmed unparseable by this project's Icarus build via an isolated, taxi-independent 15-line repro (fails identically with or without a modport qualifier). Verilator handles it natively and was already a project dependency (used for the --lint-only gate every milestone) -- confirmed via a genuine taxi_axi_ram write (0xDEADBEEF) + read-back round trip, real AXI handshaking exercised, not just elaborated. Vendored as a git submodule at third_party/taxi (shallow=true in .gitmodules for fast future clones), not just documented as an external clone step the way riscv-arch-test/riscv-formal are -- deliberately different, since taxi's AXI IP is meant to become real synthesizable RTL inside this SoC eventually, not stay a pure verification tool, and the already-accepted CERN-OHL-S-2.0 reciprocal license wants the exact source traveling with the repo. verification/taxi/ holds this project's own layer on top: - README.md: setup, the toolchain-fork rationale, the working `verilator --binary --timing` recipe, and a real gotcha worth keeping (taxi_axi_ram's DATA_W is derived from the connected interface, not its own instantiation parameter -- passing .DATA_W(...) is a hard elaboration error, not a no-op). - run_taxi_tests.sh: builds+runs every tb/*_tb.sv via Verilator's --binary mode, same PASS/FAIL/ERR summary shape as verification/riscv-arch-test/run_act_tests.sh. Per-testbench .f files list which taxi AXI sources each test needs. - tb/taxi_axi_ram_smoke_tb.sv: first real committed test, re-verified passing against the actual vendored submodule + committed script (not just the throwaway scratch clone this was originally validated against). No WB->AXI4 bridge or DRAM model exists yet -- this is the prerequisite toolchain/vendoring step only.
…taxi_axi_ram verification/taxi/rtl/dram_model.sv is a Wishbone-slave-shaped peripheral (same port contract as design/wb4_sram.sv) that bridges each WB request to a real, single-beat AXI4 transaction against a genuinely-vendored taxi_axi_ram instance -- exercises real taxi functional IP as the actual backing store, not a reimplementation. Hand-written DRAM-realism timing sits on top of that otherwise-fixed-latency RAM: a configurable ACCESS_LATENCY_CYCLES (extra wait after the AXI response) and a configurable REFRESH_INTERVAL_CYCLES/REFRESH_BUSY_CYCLES pair (periodically blocks starting a new transaction, never interrupts one already in flight). Two real protocol points found by tracing taxi_axi_ram's actual FSM, not assumed: it accepts AW and W on different cycles in general (handled via independent sticky "accepted" bits per channel, not a naive combined assumption), and it hardwires bresp/rresp to OKAY unconditionally, so err_o is sourced exclusively from a WB-side address-range check instead, bypassing the AXI FSM and refresh gating entirely for an out-of-range address. Latency/refresh timing is proven via cycle-count deltas against a zero-latency/refresh-disabled baseline instance, not a hardcoded absolute cycle count -- hand-tracing a third-party FSM's exact registered timing by inspection is error-prone (an earlier derivation for this same feature got the base latency wrong by one cycle before a recheck caught it); the delta approach is immune to that class of error. Confirmed empirically: the baseline landed at exactly the corrected hand-trace's predicted 3 cycles, and both the +5 and +6 deltas matched exactly. Verified via targeted mutation testing (remove the latency wait, remove the refresh gate, invert the sel_i->wstrb mapping, remove the out-of-range check) -- each caught cleanly, failing only the checks meant to guard that specific piece of logic. .f-file convention changed from taxi-rtl-relative flat filenames to repo-root-relative paths (run_taxi_tests.sh, taxi_axi_ram_smoke_tb.f updated to match), since a .f file now needs to reference a project-owned RTL file living outside third_party/taxi/src/axi/rtl/. Standalone module + testbench only, mirroring how clint.sv was a standalone module before its own bus-wiring became a separate step -- not wired into wb_addr_decoder.sv or soc.sv this round. Zero changes under design/ or testbench/. dram_model_tb.sv: 8/8. taxi_axi_ram_smoke_tb.sv: still 2/2 after the .f convention change. Full iverilog regression unaffected (52/52, unchanged).
…/DRAM) Generalizes wb_addr_decoder.sv from 3 slaves to 4, mirroring how CLINT's own bus-wiring worked before its firmware-through-the-real-SoC proof came as a separate step. CLINT narrowed from its old 64KB window (which deliberately claimed both addr_i[15] sub-ranges since "nothing needed the split further") down to 32KB -- still vastly more than its real 2-register footprint needs -- freeing 0x0001_8000-0x0001_FFFF for DRAM. RAM promoted from an implicit "everything else" default to a real named sel_ram wire, since 4 real targets now fully saturate the 2-bit field. A real bug was caught during planning, before any RTL was written: dram_model.sv does its own zero-based bounds check, but the decoder broadcasts the raw, untranslated system address to every slave -- safe today only by coincidence (RAM sits at address 0; UART/CLINT do no bounds check of their own). Feeding DRAM the raw address would have failed its bounds check on every legitimately-routed access, permanently and silently breaking it with err_o instead of ack_o. Fixed by rebasing dram_addr_o specifically -- the one deliberate exception among the four *_addr_o broadcast assigns. design/soc.sv itself gets no new module instantiation -- can't, structurally, forever: dram_model.sv is Verilator-only by construction (the interface-based AXI bridge is fundamental to its design), and soc.sv is compiled by many existing iverilog testbenches that must keep working. New dram_* decoder ports left explicitly unconnected (empty parens + PINCONNECTEMPTY pragma, same precedent as core.sv's own decoder0.o_instruction_address()). testbench/decoder_clint_harness.sv needed the identical treatment -- a second direct-instantiation call site easy to miss. New verification/taxi/rtl/decoder_dram_harness.sv + verification/taxi/tb/decoder_dram_tb.sv prove real decoder<->real dram_model routing end-to-end (mirrors decoder_clint_harness.sv's own precedent, all four real slaves this time), using dram_model's real default timing parameters rather than testbench-friendly toy values. design/wb_addr_decoder_tb.sv gained a 4th fake DRAM stub, two retargeted tests (the old CLINT-window tests' premise no longer holds), a new CLINT-narrowed-boundary test, and exactly 6 new round-trip pairs (not 12) -- target_q's flat overwrite has no pair-specific logic, so only proving DRAM as source/destination in each direction was needed for full 12/12 ordered-pair coverage. Verified via mutation testing: removing the dram_addr_o rebase fails exactly the 4 DRAM-specific checks in decoder_dram_tb.sv, nothing else disturbed. Reverting CLINT's narrowing is also caught, not by the round-trip data checks (target-selection priority happens to still route DRAM's data correctly) but by the "clint_cyc_o stays low" symmetry checks, correctly flagging that CLINT would spuriously also activate for DRAM addresses. decoder_dram_tb.sv: 15/15. wb_addr_decoder_tb.sv: 47/47 (up from 32). wb_addr_decoder_clint_tb.sv, soc_tb.sv unaffected. Full iverilog regression 52/52, full-SoC verilator lint clean. dram_model.sv is still not wired into design/soc.sv directly -- unlike CLINT, this isn't a staging choice deferred to later. soc.sv must stay iverilog-parseable forever and dram_model.sv is Verilator-only by construction, so a future firmware-through-DRAM step needs a different approach than CLINT's own playbook. verification/taxi/README.md's Status section documents this explicitly.
rvfi_intr had been hardwired 1'b0 since the interrupt-taking milestone. Per riscv-formal's own spec, it must flag the first instruction of a trap handler -- implemented mechanically (compare each retirement's pc against the prior retirement's own next_pc) rather than semantically (trap_taken/interrupt_taken tracking), since next_pc's own mux already keeps synchronous-exception PC chains consistent; only interrupt_taken's separate PC-register arm produces a genuine discontinuity, which the mechanical definition catches automatically and robustly against future redirect mechanisms. wrapper.sv never connected i_mtip to the core instantiation at all, so interrupt_taken could never fire in any generated check -- the rvfi_intr logic would have been dead code from the formal model's perspective. Fixed with a free rvformal_rand_reg, matching the existing wb_ack/wb_err convention and riscv-formal's own nerv wrapper precedent. Toolchain (sby/bitwuzla/yosys-slang) had bit-rotted since the last riscv-formal run -- all three referenced nix-store paths garbage-collected in this environment. Rebuilt all three from source in user-space (no root available), including working around a missing-GMP/MPFR-dev-headers gap via .deb extraction and a bitwuzla CLI version-skew fix for yosys-smtbmc. Verified directly: pc_fwd_ch0 and pc_bwd_ch0 (the only two check families that consume rvfi_intr) both PASS at full depth. Spot-checked insn_add_ch0, ill_ch0, and reg_ch0 (via boolector) for regressions from the newly-free i_mtip -- all still PASS. Full iverilog regression (52/52) and full-SoC verilator lint confirm zero effect on the real, non-formal build.
EBREAK now takes an ordinary synchronous trap (cause 3, Breakpoint) through the same mepc/mcause/mtval/mtvec machinery every other exception already uses, instead of latching a one-way halted register that froze the FSM forever. mtval falls through to the existing default-0 arm (no explicit EBREAK arm needed), matching common real-core convention for a plain EBREAK with no hardware trigger. The permanent halted latch and every !halted guard (commit_now, interrupt_taken, both wb_master_drive S_FETCH arms) are deleted outright, not left as dead placeholders -- a future Debug Module milestone reintroduces real halt/resume state under a new name. The real scope here was that `halted` was every testbench's own completion signal: 35 of 40 real testbench files polled it to know when a program finished. Fixed by redesigning testbench/halt_wait.sv's contract so callers declare a local sticky latch on the one-shot `trap_taken && is_ebreak` pulse instead of aliasing the now-deleted signal. No firmware or hand-encoded instruction streams needed changes -- testbenches only need to observe the trap firing once and stop watching, not stay frozen. Found and fixed a real, recurring bug class along the way: a "terminal" ebreak placed inside a trap handler, while mtvec is still armed pointing at that handler, no longer terminates anything -- it bounces back into the handler (a real trap now) and can corrupt mcause/mepc/GPR state before a testbench's own checks run. Fixed across 6 files via point-in-time snapshots taken at the correct instant, not live reads after the fact. testbench/act_runner_tb.sv (the generic ACT4 runner) needed a different fix for the same underlying issue: it now polls tohost directly for a real pass/fail marker instead of relying on any ebreak-based signal, since ACT4 tests are generic ELFs that may deliberately exercise ebreak mid-test. New testbench/core_ebreak_trap_tb.sv directly proves both M-mode EBREAK trapping and S-mode delegation via medeleg bit 3. ACT4's ExceptionsZc-00 flips UNKNOWN->PASS; U-00 now genuinely runs to completion (previously froze at its first subcase) and hits a real, previously-hidden failure elsewhere in the test, unrelated to EBREAK itself -- flagged as a separate follow-up, not chased down here. riscv-formal's pc_fwd_ch0/ pc_bwd_ch0 (the checks that originally motivated the now-deleted !halted guard) re-verified clean against the new RTL. Full iverilog regression 53/53, full-SoC verilator lint clean.
ACT4's priv/U/U-00 regressed from UNKNOWN to a real FAIL once EBREAK became a real, resumable trap (previous commit) -- U-00 ran far enough for the first time to reach its own boot-time csrrw x0, cycle, a0, a write-attempt to a genuinely read-only CSR (bits[11:10]=='11'). Root- caused via sail_riscv_sim tracing against the real ELF, diffed against this core's own RVFI retirement trace: Sail traps (illegal-instruction, cause 2), this core silently no-op'd the write and fell through. Per spec, "Attempts to write a read-only CSR... raise illegal instruction exceptions" unconditionally, regardless of privilege level. This was a known, deliberate gap -- deferred because core_zicsr_tb.sv's own csrrwi-to-mhartid case depended on the old silent-ignore behavior. That subtest is retired now that the real trap exists, replaced by a dedicated core_csr_readonly_trap_tb.sv covering the trap/resume path. ACT4: 108 -> 109 passed, U-00 now genuinely PASS. Full regression and verilator lint clean.
New design/uart_rx.sv: a Wishbone-slave receive peripheral, sibling to uart_tx.sv, matching its house style (no baud timing, registered 1-wait-state ack, err_o never asserted). A testbench-only push_byte backdoor task enqueues bytes into a 256-entry FIFO as if they'd arrived over the wire; real bus reads to RX_DATA (0x8010) pop the queue, RX_STATUS (0x8018) reports non-empty. design/soc.sv splits the decoder's single opaque uart_* target between uart0 (TX) and the new uart_rx0 (RX) via a new addr_i[4] sub-decode -- the first address-decode logic soc.sv itself has ever contained, since wb_addr_decoder.sv still only ever sees one "uart" target. An adversarial review pass (4 dimensions, each finding independently re-verified against the real files) caught one real correctness bug before it shipped: RX_DATA's destructive FIFO pop didn't check sel_i, so a sub-word read that didn't include byte lane 0 (e.g. a byte load at a nonzero offset within the register's own bus window) would silently discard a real received byte while returning 0. Fixed by gating the pop on sel_i[0], mirroring uart_tx.sv's own write gate. The review also found real test-coverage gaps, now closed: FIFO overflow/wraparound at the 256-entry boundary was never exercised; the sel_i[0] gate itself had no test; and soc.sv's new TX/RX response mux was only ever proven for the RX arm -- no test read TX_DATA or TX_STATUS through the real bus path, so a broken mux would have shipped silently for the TX side. soc_uart_rx_tb.sv now reads TX_STATUS through the real soc.sv mux as part of its firmware. Full regression (56/56) and verilator lint clean.
The README still described the RV64I+Zicsr, two-peripheral state from early in the project -- badly stale against the real current feature set (RV64IMAC+Zicsr, full M/S/U privilege, real timer interrupts, an L1 I$/D$ cache hierarchy, bus-error trapping, Zifencei, UART TX+RX, a standalone Wishbone-to-AXI4/DRAM model). Rewrites the Current state, Building/simulating, and Roadmap sections to match what's actually in the repo today, including the in-progress Debug Module (JTAG/DMI) staged plan.
New CSR block in design/csr_file.sv at the real spec addresses (0x7B0-0x7B3). dpc mirrors mepc's reset -> hardware-entry -> software-write priority chain, including its bit-0 WARL mask. dcsr's hardware-entry arm only touches cause[8:6]/prv[1:0] (mirroring mstatus's own partial-bit-range trap-entry idiom); everything else (ebreakm/stepie/etc.) persists across entry, software-writable only. xdebugver[31:28] is WARL-fixed at 4 via the same no-storage-bit, OR'd-at-read-time technique as MSTATUS_UXL_FIXED. dscratch0/dscratch1 are plain read/write. design/core.sv gains a dedicated debug_csr_violation check, separate from the existing csr_priv_violation: the four debug addresses encode imm_2[9:8]=='11', the same bit pattern as an M-mode-only CSR, so the existing magnitude comparison would let M-mode straight through -- the RISC-V Debug spec requires these be inaccessible outside Debug Mode specifically, not just gated by privilege level. in_debug_mode is a forward reference, tied 0 until Milestone 4 builds the real halt/resume FSM -- so every access traps from anywhere for now, which is spec-correct (no Debug Mode exists yet to legally be in). An adversarial review pass (3 dimensions, all 7 findings independently re-verified against the real files) found no live RTL bugs but several real test-coverage gaps, all closed: no test proved a genuine WRITE (not just a suppressed read) to a debug CSR also traps; the trap was only ever exercised from M-mode, never S or U (where csr_priv_violation is independently true too -- the one place both violation checks fire together); the address range-check's exact boundary (0x7AF/0x7B4) was untested; the xdebugver write-mask test only checked bits [31:28] themselves, not that adjacent bits survive; and dcsr's hardware-entry capture was only ever exercised once, unable to distinguish a correct overwrite from an accidental OR-accumulate bug. testbench/core_debug_csr_violation_tb.sv now drops privilege M->S->U and exercises a real write attempt in the same run; design/csr_file_tb.sv gained a second debug-entry pulse and a same-cycle entry-vs-software-write race case. Full regression (57/57) and verilator lint clean.
Milestone 3 of the EBREAK/JTAG staged plan landed since the last README refresh -- update the Architecture and Roadmap sections to reflect it.
The file had grown to 175 lines with no navigation, and covered neither how to contribute nor the project's actual license -- there's a real MIT LICENSE at the repo root that was never referenced anywhere in the README.
Keeps the generated internal engineering reference PDF (module port lists, purpose, verification environment) out of the public repo.
bitglitcher
approved these changes
Aug 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
2 participants
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.
No description provided.