# Privileged Test Fixes Implementation Plan > **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. **Goal:** Make `rv64mi-p-instret_overflow`, `rv64mi-p-mcsr`, `rv64si-p-dirty`, and `rv64si-p-icache-alias` behave according to the privileged tests instead of failing or timing out. **Architecture:** Fix the small M-mode CSR gaps first, then make address translation privilege-aware for data accesses, then add an instruction-side translation path. Keep the implementation minimal for these tests: Sv39 only, no ASIDs, no speculative PTW sharing, and no broader cache redesign. **Tech Stack:** Scala 2.13, Chisel 7.7, ScalaTest, generated SystemVerilog, Verilator RISC-V testbench. --- ## File Structure - Modify `src/main/scala/common/Privileged.scala`: add missing machine CSR constants and mstatus bit constants used by translation logic. - Modify `src/main/scala/csr/CSRPermission.scala`: classify read-only ID CSRs and writable machine counters correctly. - Modify `src/main/scala/csr/CSRFile.scala`: implement `mvendorid/marchid/mimpid`, `minstret`, `mcountinhibit.IR`, and instret/minstret write semantics. - Modify `src/main/scala/common/Bundles.scala`: carry privilege and mstatus-derived access controls into translation requests. - Modify `src/main/scala/memory/MMU.scala`: enforce Sv39 permissions for S/U/MPRV, A/D, SUM, and superpage alignment. - Modify `src/main/scala/memory/LSU.scala`: decide effective privilege for loads/stores using `mstatus.MPRV/MPP`, and run translation only when effective privilege is S/U with non-Bare `satp`. - Modify `src/main/scala/OoOBackend.scala`: pass `currentPriv` and `mstatus` into LSU. - Modify `src/main/scala/frontend/Frontend.scala`: add `satp/currentPriv` inputs and an instruction PTW path. - Modify `src/main/scala/Core.scala`: wire frontend `satp/currentPriv` from backend/privilege state. - Test `src/test/scala/csr/CSRPermissionSpec.scala`: cover missing CSR permissions. - Test `src/test/scala/csr/CSRFileSpec.scala`: cover generated CSR read/write behavior. - Test `src/test/scala/memory/Sv39Spec.scala`: cover the generated MMU/LSU/Frontend ports and key permission expressions. - Verify with `sim/scripts/run_privileged_tests.sh`. ## Task 1: Fix Machine CSR Address Map **Files:** - Modify: `src/main/scala/common/Privileged.scala` - Modify: `src/main/scala/csr/CSRPermission.scala` - Test: `src/test/scala/csr/CSRPermissionSpec.scala` - [ ] **Step 1: Add constants for missing machine CSRs** In `src/main/scala/common/Privileged.scala`, add these constants near the existing machine CSR constants: ```scala val CSR_MVENDORID = "hf11".U(12.W) val CSR_MARCHID = "hf12".U(12.W) val CSR_MIMPID = "hf13".U(12.W) val CSR_MHARTID = "hf14".U(12.W) val CSR_MCOUNTINHIBIT = "h320".U(12.W) val CSR_MINSTRET = "hb02".U(12.W) ``` Keep `CSR_CYCLE`, `CSR_TIME`, and `CSR_INSTRET` as user-visible read-only counter aliases at `0xc00` through `0xc02`. - [ ] **Step 2: Update CSR permission sets** In `CSRPermission.scala`, include the missing machine CSRs in `machineCsrs`, and treat ID CSRs as read-only: ```scala private val machineCsrs = Set( 0x300, 0x301, 0x302, 0x303, 0x304, 0x305, 0x306, 0x320, 0x340, 0x341, 0x342, 0x343, 0x344, 0x3a0, 0x3b0, 0x744, 0x7a0, 0x7a1, 0x7a2, 0x7a5, 0xb02, 0xf11, 0xf12, 0xf13, 0xf14) private val counterCsrs = Set(0xc00, 0xc01, 0xc02) private val readOnlyCsrs = Set(0xf11, 0xf12, 0xf13, 0xf14) ++ counterCsrs ``` Update `isMachine` to include `CSR_MCOUNTINHIBIT`, `CSR_MINSTRET`, `CSR_MVENDORID`, `CSR_MARCHID`, and `CSR_MIMPID`. - [ ] **Step 3: Add permission regression tests** Append these cases to `CSRPermissionSpec.scala`: ```scala it should "allow machine-mode reads of implementation ID CSRs and reject writes" in { val ids = Seq(Privileged.CSR_MVENDORID, Privileged.CSR_MARCHID, Privileged.CSR_MIMPID, Privileged.CSR_MHARTID) ids.foreach { csr => CSRPermission.readAllowedLit(csr.litValue, Privileged.PRV_M.litValue) should be(true) CSRPermission.writeAllowedLit(csr.litValue, Privileged.PRV_M.litValue) should be(false) CSRPermission.readAllowedLit(csr.litValue, Privileged.PRV_S.litValue) should be(false) } } it should "allow machine-mode writes to minstret and mcountinhibit" in { CSRPermission.readAllowedLit(Privileged.CSR_MINSTRET.litValue, Privileged.PRV_M.litValue) should be(true) CSRPermission.writeAllowedLit(Privileged.CSR_MINSTRET.litValue, Privileged.PRV_M.litValue) should be(true) CSRPermission.readAllowedLit(Privileged.CSR_MCOUNTINHIBIT.litValue, Privileged.PRV_M.litValue) should be(true) CSRPermission.writeAllowedLit(Privileged.CSR_MCOUNTINHIBIT.litValue, Privileged.PRV_M.litValue) should be(true) } ``` - [ ] **Step 4: Run the targeted test** Run: ```bash sbt "testOnly CSRPermissionSpec" ``` Expected before implementation: failures for missing constants or denied CSRs. Expected after implementation: `CSRPermissionSpec` passes. ## Task 2: Implement `mcsr` and `instret_overflow` CSR Semantics **Files:** - Modify: `src/main/scala/csr/CSRFile.scala` - Test: `src/test/scala/csr/CSRFileSpec.scala` - [ ] **Step 1: Add CSR state** In `CSRFile.scala`, add: ```scala val mcountinhibit = RegInit(0.U(p.xlen.W)) val suppressInstretIncrement = WireDefault(false.B) ``` The existing `instret` register should serve both `minstret` and `instret`. - [ ] **Step 2: Increment `instret` unless inhibited or just written** Replace the unconditional counter update region with: ```scala cycle := cycle + 1.U when(!mcountinhibit(2) && !suppressInstretIncrement) { instret := instret + 1.U } ``` This matches the test expectation that a write to `minstret` suppresses the increment observed by the immediately following read. - [ ] **Step 3: Add CSR reads** Add to both the read switch and `writeOld` switch: ```scala is(Privileged.CSR_MCOUNTINHIBIT) { r := mcountinhibit } is(Privileged.CSR_MINSTRET) { r := instret } is(Privileged.CSR_MVENDORID) { r := 0.U } is(Privileged.CSR_MARCHID) { r := 0.U } is(Privileged.CSR_MIMPID) { r := 0.U } ``` For the `writeOld` switch, use `writeOld := ...` instead of `r := ...`. - [ ] **Step 4: Add CSR writes** Inside the CSR write switch: ```scala is(Privileged.CSR_MCOUNTINHIBIT) { mcountinhibit := next & 4.U } is(Privileged.CSR_MINSTRET) { instret := next suppressInstretIncrement := true.B } ``` Do not make `mvendorid/marchid/mimpid/mhartid` writable. - [ ] **Step 5: Add generated-Verilog smoke tests** Append to `CSRFileSpec.scala`: ```scala it should "contain machine counter and ID CSR decode paths" in { val verilog = ChiselStage.emitSystemVerilog(new CSRFile()) verilog should include("mcountinhibit") verilog should include("12'hB02") verilog should include("12'hF11") verilog should include("12'hF12") verilog should include("12'hF13") } ``` - [ ] **Step 6: Run targeted tests and two privileged binaries** Run: ```bash sbt "testOnly CSRPermissionSpec CSRFileSpec" make -C sim/verilator compile timeout 5s sim/verilator/obj_dir/VCore ../../riscv-tests/isa/rv64mi-p-mcsr timeout 5s sim/verilator/obj_dir/VCore ../../riscv-tests/isa/rv64mi-p-instret_overflow ``` Expected: both Verilator commands exit `0`. ## Task 3: Make Data Translation Privilege-Aware **Files:** - Modify: `src/main/scala/common/Privileged.scala` - Modify: `src/main/scala/common/Bundles.scala` - Modify: `src/main/scala/memory/MMU.scala` - Modify: `src/main/scala/memory/LSU.scala` - Modify: `src/main/scala/OoOBackend.scala` - Test: `src/test/scala/memory/Sv39Spec.scala` - [ ] **Step 1: Add mstatus bit constants** In `Privileged.scala`, add: ```scala val MSTATUS_MPRV_BIT = 17 val MSTATUS_SUM_BIT = 18 val MSTATUS_MXR_BIT = 19 val MSTATUS_MPP_HI = 12 val MSTATUS_MPP_LO = 11 ``` - [ ] **Step 2: Extend TLB request metadata** In `Bundles.scala`, extend `TlbReq`: ```scala class TlbReq(p: CoreParams = CoreParams()) extends Bundle { val valid = Bool() val vaddr = UInt(p.xlen.W) val isStore = Bool() val isFetch = Bool() val priv = UInt(2.W) val sum = Bool() val mxr = Bool() } ``` - [ ] **Step 3: Enforce Sv39 permissions in the page walker** In `MMU.scala`, register `priv/sum/mxr` alongside request state: ```scala val privReg = Reg(UInt(2.W)) val sumReg = Reg(Bool()) val mxrReg = Reg(Bool()) ``` Set them in `sIdle` when accepting a request: ```scala privReg := io.req.priv sumReg := io.req.sum mxrReg := io.req.mxr ``` Replace the existing `permFault` with: ```scala val userPage = pteU val supervisorAccessToUser = privReg === Privileged.PRV_S && userPage && !sumReg && !isFetchReg val supervisorFetchUser = privReg === Privileged.PRV_S && userPage && isFetchReg val userAccessSupervisor = privReg === Privileged.PRV_U && !userPage val readAllowed = pteR || (mxrReg && pteX) val accessPermFault = Mux(isFetchReg, !pteX, Mux(isStoreReg, !pteW || !pteD, !readAllowed)) val adFault = !pteA || (isStoreReg && !pteD) val permFault = accessPermFault || adFault || supervisorAccessToUser || supervisorFetchUser || userAccessSupervisor ``` Add superpage PPN alignment checks in the leaf case: ```scala val misalignedSuperpage = (level === 2.U && ptePpn(17, 0) =/= 0.U) || (level === 1.U && ptePpn(8, 0) =/= 0.U) when(permFault || misalignedSuperpage) { walkFault := true.B } ``` - [ ] **Step 4: Make LSU choose effective privilege** Add LSU IO inputs: ```scala val currentPriv = Input(UInt(2.W)) val mstatus = Input(UInt(p.xlen.W)) ``` In `LSU.scala`, replace `bare` with effective-mode logic: ```scala val mprv = io.mstatus(Privileged.MSTATUS_MPRV_BIT) val mpp = io.mstatus(Privileged.MSTATUS_MPP_HI, Privileged.MSTATUS_MPP_LO) val effectivePriv = Mux(io.currentPriv === Privileged.PRV_M && mprv, mpp, io.currentPriv) val translate = io.satp(63, 60) =/= 0.U && effectivePriv =/= Privileged.PRV_M val bare = !translate ``` Populate DTLB/MMU request metadata: ```scala dtlb.io.req.priv := effectivePriv dtlb.io.req.sum := io.mstatus(Privileged.MSTATUS_SUM_BIT) dtlb.io.req.mxr := io.mstatus(Privileged.MSTATUS_MXR_BIT) mmu.io.req.priv := effectivePriv mmu.io.req.sum := io.mstatus(Privileged.MSTATUS_SUM_BIT) mmu.io.req.mxr := io.mstatus(Privileged.MSTATUS_MXR_BIT) ``` - [ ] **Step 5: Wire backend to LSU** In `OoOBackend.scala`, add: ```scala lsu.io.currentPriv := io.currentPriv lsu.io.mstatus := csr.io.mstatus ``` - [ ] **Step 6: Update tests** Append to `Sv39Spec.scala`: ```scala it should "generate LSU effective privilege translation controls" in { val verilog = ChiselStage.emitSystemVerilog(new LSU()) verilog should include("io_currentPriv") verilog should include("io_mstatus") verilog should include("io_mstatus[17]") } ``` - [ ] **Step 7: Run tests and dirty binary** Run: ```bash sbt "testOnly Sv39Spec" make -C sim/verilator compile timeout 5s sim/verilator/obj_dir/VCore ../../riscv-tests/isa/rv64si-p-dirty ``` Expected: `rv64si-p-dirty` exits `0`. If it still fails, inspect whether the first `TESTNUM=2` store raises store page fault and whether the M-mode handler reads `page_table_1` physically. ## Task 4: Add Instruction-Side Sv39 Translation **Files:** - Modify: `src/main/scala/frontend/ITLB.scala` - Modify: `src/main/scala/frontend/Frontend.scala` - Modify: `src/main/scala/OoOBackend.scala` - Modify: `src/main/scala/Core.scala` - Test: `src/test/scala/memory/Sv39Spec.scala` - [ ] **Step 1: Extend Frontend IO** Add to `Frontend` IO: ```scala val satp = Input(UInt(p.xlen.W)) val currentPriv = Input(UInt(2.W)) ``` - [ ] **Step 2: Add an instruction MMU instance** In `Frontend.scala`, instantiate MMU: ```scala val immu = Module(new MMU(p)) val fetchTranslate = io.satp(63, 60) =/= 0.U && io.currentPriv =/= Privileged.PRV_M ``` Set ITLB request valid only when translation is active: ```scala itlb.io.req.valid := fetchTranslate itlb.io.req.vaddr := pc itlb.io.req.isStore := false.B itlb.io.req.isFetch := true.B itlb.io.req.priv := io.currentPriv itlb.io.req.sum := false.B itlb.io.req.mxr := false.B ``` Drive `immu` for ITLB misses: ```scala immu.io.satp := io.satp immu.io.req.valid := fetchTranslate && itlb.io.resp.miss immu.io.req.vaddr := pc immu.io.req.isStore := false.B immu.io.req.isFetch := true.B immu.io.req.priv := io.currentPriv immu.io.req.sum := false.B immu.io.req.mxr := false.B itlb.io.refill := immu.io.refill ``` - [ ] **Step 3: Add instruction PTW memory port** Add frontend IO: ```scala val ptwMemReqValid = Output(Bool()) val ptwMemReqAddr = Output(UInt(p.xlen.W)) val ptwMemRespValid = Input(Bool()) val ptwMemRespData = Input(UInt(p.xlen.W)) ``` Wire: ```scala io.ptwMemReqValid := immu.io.ptwMemReq.valid io.ptwMemReqAddr := immu.io.ptwMemReq.addr immu.io.ptwMemResp.valid := io.ptwMemRespValid immu.io.ptwMemResp.data := io.ptwMemRespData ``` In `Core.scala`, arbitrate instruction-side PTW reads onto the existing data memory read path. Add a one-bit outstanding register so the frontend only consumes the memory response that belongs to its PTW request: ```scala val frontendPtwOutstanding = RegInit(false.B) val frontendPtwFire = frontend.io.ptwMemReqValid when(frontendPtwFire) { frontendPtwOutstanding := true.B }.elsewhen(io.dmem_resp_valid && frontendPtwOutstanding) { frontendPtwOutstanding := false.B } io.dmem_req_valid := frontend.io.ptwMemReqValid || backend.io.dmemReqValid io.dmem_req_bits_addr := Mux(frontend.io.ptwMemReqValid, frontend.io.ptwMemReqAddr, backend.io.dmemReq.addr) io.dmem_req_bits_data := Mux(frontend.io.ptwMemReqValid, 0.U, backend.io.dmemReq.data) io.dmem_req_bits_isStore := Mux(frontend.io.ptwMemReqValid, false.B, backend.io.dmemReq.isStore) io.dmem_req_bits_size := Mux(frontend.io.ptwMemReqValid, 3.U, backend.io.dmemReq.size) frontend.io.ptwMemRespValid := io.dmem_resp_valid && frontendPtwOutstanding frontend.io.ptwMemRespData := io.dmem_resp_bits ``` - [ ] **Step 4: Gate fetch while instruction translation is pending** Use translated physical address only when translation is ready: ```scala val fetchTranslationReady = !fetchTranslate || itlb.io.resp.hit val fetchTranslationFault = fetchTranslate && (itlb.io.resp.pageFault || immu.io.resp.pageFault) val fetchPaddr = Mux(fetchTranslate, itlb.io.resp.paddr, pc) icache.io.reqValid := fetchTranslationReady && !fetchTranslationFault && !instMisaligned && !faultPending icache.io.reqAddr := fetchPaddr ``` Update `fetchFault` to include `fetchTranslationFault`. - [ ] **Step 5: Expose backend CSR `satp`** In `OoOBackend.scala`, add an output: ```scala val satpOut = Output(UInt(p.xlen.W)) ``` Drive it from the CSR file: ```scala io.satpOut := csr.io.satp ``` Keep the existing LSU connection: ```scala lsu.io.satp := csr.io.satp ``` - [ ] **Step 6: Wire frontend privilege inputs** In `Core.scala`: ```scala frontend.io.satp := backend.io.satpOut frontend.io.currentPriv := privCtrl.io.priv ``` - [ ] **Step 7: Add generated-Verilog frontend test** Append to `Sv39Spec.scala`: ```scala it should "generate frontend instruction-side translation ports" in { val verilog = ChiselStage.emitSystemVerilog(new Frontend()) verilog should include("io_satp") verilog should include("io_currentPriv") verilog should include("ptwMemReq") } ``` - [ ] **Step 8: Run icache-alias** Run: ```bash sbt "testOnly Sv39Spec" make -C sim/verilator compile timeout 5s sim/verilator/obj_dir/VCore ../../riscv-tests/isa/rv64si-p-icache-alias ``` Expected: no `Bad imem fetch pc=0x0`, no out-of-bounds fetch stream, exit `0`. ## Task 5: Full Privileged Regression **Files:** - No source edits. - Verify: `sim/results/privileged_test_results.txt` - [ ] **Step 1: Rebuild generated RTL/testbench** Run: ```bash sbt "runMain CoreOoO" make -C sim/verilator compile ``` Expected: both commands exit `0`. - [ ] **Step 2: Run privileged suite** Run: ```bash cd sim/scripts TIMEOUT_SECONDS=10 ./run_privileged_tests.sh ``` Expected: ```text rv64mi-p-instret_overflow: exit=0 PASS rv64mi-p-mcsr: exit=0 PASS rv64si-p-dirty: exit=0 PASS rv64si-p-icache-alias: exit=0 PASS ``` `rv64ui-p-ma_data` is not in this privileged script and remains acceptable as a known aligned-access limitation in the base UI suite. - [ ] **Step 3: Run base ISA suite** Run: ```bash cd sim/scripts ./run_tests.sh ``` Expected: existing PASS count is preserved, with only `rv64ui-p-ma_data` failing if the processor intentionally does not support misaligned data access. ## Self-Review - Spec coverage: `rv64mi-p-instret_overflow` is covered by Task 1 and Task 2; `rv64mi-p-mcsr` is covered by Task 1 and Task 2; `rv64si-p-dirty` is covered by Task 3; `rv64si-p-icache-alias` is covered by Task 4. - Placeholder scan: no deferred design items are left in the plan; every task has concrete files, code snippets, commands, and expected outcomes. - Type consistency: new `TlbReq` fields are populated in LSU and Frontend; MMU consumes the same field names; Core/OoOBackend wiring is explicitly called out where ownership of `satp` crosses module boundaries.