From da4b1f574fad29a03934175f924d2caa00d4a8c4 Mon Sep 17 00:00:00 2001 From: murdoa Date: Tue, 6 Oct 2026 15:59:10 +0100 Subject: [PATCH 1/3] fix(privilege): implement non-H MPRV and clear it on xRET --- packages/river_hdl/lib/src/core.dart | 42 ++-- packages/river_hdl/lib/src/core/csr.dart | 34 ++- packages/river_hdl/lib/src/core/mmu.dart | 64 +++--- .../river_hdl/test/csr/mprv_csr_test.dart | 135 ++++++++++++ .../test/mmu/mprv_core_microcoded_test.dart | 3 + .../river_hdl/test/mmu/mprv_core_test.dart | 201 ++++++++++++++++++ .../test/mmu/mprv_selector_test.dart | 76 +++++++ .../river_hdl/test/mmu/mprv_walk_test.dart | 136 ++++++++++++ 8 files changed, 645 insertions(+), 46 deletions(-) create mode 100644 packages/river_hdl/test/csr/mprv_csr_test.dart create mode 100644 packages/river_hdl/test/mmu/mprv_core_microcoded_test.dart create mode 100644 packages/river_hdl/test/mmu/mprv_core_test.dart create mode 100644 packages/river_hdl/test/mmu/mprv_selector_test.dart create mode 100644 packages/river_hdl/test/mmu/mprv_walk_test.dart diff --git a/packages/river_hdl/lib/src/core.dart b/packages/river_hdl/lib/src/core.dart index 125a7b9..5e7e0d9 100644 --- a/packages/river_hdl/lib/src/core.dart +++ b/packages/river_hdl/lib/src/core.dart @@ -302,6 +302,7 @@ class RiverCore extends BridgeModule { final enableMxr = Logic(name: 'enableMxr'); final enableSum = Logic(name: 'enableSum'); + final effectiveDataPriv = Logic(name: 'effectiveDataPriv', width: 3); // DTLBFC (rpipelinectl[3]): drives the MMU's flush-data-TLB-on-priv-change. // Forward-declared here, driven from the CSR file below (mirrors enableMxr). final dtlbFlushOnPriv = Logic(name: 'dtlbFlushOnPriv'); @@ -426,12 +427,10 @@ class RiverCore extends BridgeModule { // does not flush the cache on every trap, which whole-cache invalidation on // a privilege change would. // - // Fetch permission depends on the privilege mode only. A load also depends - // on sstatus.SUM, so the D-cache carries that bit too. Two mode bits - // separate U (0), S (1) and M (3), which matters as well because machine - // mode is untranslated and its addresses are physical. sstatus.SUM only - // exists with the CSR file, so drop that bit when there is none: the - // privilege bits alone are still correct, just coarser. + // Fetch uses actual privilege; data uses effective privilege and, for + // non-H cores, SUM/MXR. Two privilege bits separate U (0), S (1) and M (3), + // including translated S/U addresses from physical M addresses. Without + // CSRs the permission controls are tied low. final privContext = (useICache || useDCache) && (config.hasSupervisor || config.hasUser); Logic? iCtx; @@ -439,15 +438,14 @@ class RiverCore extends BridgeModule { if (privContext) { final privTag = mode.slice(1, 0); iCtx = virt == null ? privTag : [virt, privTag].swizzle(); - // sstatus.SUM is deliberately left out. It only separates an S-mode access - // made with SUM set from one made with SUM clear, which is a narrow - // permission hole and not a data-corruption path for a correct kernel. The - // privilege bits alone catch the case that corrupts Linux, where a kernel - // virtual address and a user virtual address share a tag because the tag - // holds no address bit above 31. Each tag bit widens the comparator, and - // that comparator is this core's FPGA critical path, so the bit is not - // free: including SUM measurably hurt routing on xc7s50. - dCtx = [if (guestAccessWire != null) guestAccessWire, privTag].swizzle(); + // A data hit bypasses the MMU. A change to MPRV/MPP, SUM or MXR must + // therefore miss under the old authorization, without a software fence. + // H keeps its existing context until guest permission selection is wired. + dCtx = [ + if (guestAccessWire != null) guestAccessWire, + if (!config.hasHypervisor) ...[enableSum, enableMxr], + effectiveDataPriv.slice(1, 0), + ].swizzle(); } // Significant low address bits BOTH L1 caches must tag on. @@ -565,6 +563,7 @@ class RiverCore extends BridgeModule { gMode: config.hasHypervisor ? gModeWire : null, gRoot: config.hasHypervisor ? gRootWire : null, privMode: config.mmu.hasPaging ? mode : null, + dataPrivMode: config.mmu.hasPaging ? effectiveDataPriv : null, sum: config.mmu.hasPaging ? enableSum : null, mxr: config.mmu.hasPaging ? enableMxr : null, // Translate instruction fetches (below M-mode). The icache is VIRTUALLY @@ -1134,11 +1133,24 @@ class RiverCore extends BridgeModule { Const(config.mxlen.satpPpnMask, width: config.mxlen.size)); } + // MPRV is an access override, not an actual-M-mode-only permission bit. + // H deliberately retains read-only-zero MPRV pending MPV/VS integration. + effectiveDataPriv <= + (csrs == null || config.hasHypervisor + ? mode + : mux( + csrs.mstatus[17], + csrs.mstatus.slice(12, 11).zeroExtend(3), + mode, + )); if (csrs != null) { enableMxr <= ((csrs.mstatus >> 19) & Const(1, width: config.mxlen.size)).neq(0); enableSum <= ((csrs.mstatus >> 18) & Const(1, width: config.mxlen.size)).neq(0); + } else { + enableMxr <= Const(0); + enableSum <= Const(0); } // DTLBFC bit (rpipelinectl[3]) -> MMU data-TLB priv-change flush. dtlbFlushOnPriv <= (csrs == null ? Const(0) : csrs.rpipelinectl[3]); diff --git a/packages/river_hdl/lib/src/core/csr.dart b/packages/river_hdl/lib/src/core/csr.dart index abf5bf0..540e900 100644 --- a/packages/river_hdl/lib/src/core/csr.dart +++ b/packages/river_hdl/lib/src/core/csr.dart @@ -19,6 +19,7 @@ class RiscVMstatusCsr extends CsrConfig { // popped on trap/MRET. RiscVMstatusCsr({ bool sup = false, + bool mprv = false, bool sum = false, bool mxr = false, bool fp = false, @@ -81,6 +82,13 @@ class RiscVMstatusCsr extends CsrConfig { name: 'fs', access: CsrFieldAccess.readWrite, ), + if (mprv) + CsrFieldConfig( + start: 17, + width: 1, + name: 'mprv', + access: CsrFieldAccess.readWrite, + ), if (sum) CsrFieldConfig( start: 18, @@ -615,6 +623,9 @@ class RiscVCsrFile extends Module { CsrInstanceConfig( arch: RiscVMstatusCsr( sup: hasSupervisor, + // H effective-context selection is not implemented yet. Preserve its + // existing read-only-zero MPRV rather than advertise partial support. + mprv: hasUser && !hasHypervisor, sum: hasSum, mxr: hasMxr, fp: _hasFloat, @@ -622,7 +633,7 @@ class RiscVCsrFile extends Module { hyp: hasHypervisor, ), addr: CsrAddress.mstatus.address, - resetValue: 0, + resetValue: !hasHypervisor && !hasUser ? 3 << 11 : 0, width: mxlen.size, // Hardware-written on trap entry / xRET; wrEn driven in _wireTrapState. isBackdoorWritable: true, @@ -1257,6 +1268,20 @@ class RiscVCsrFile extends Module { Logic _maskWriteData(Logic addr12, Logic data) { Logic out = data; + if (!hasHypervisor) { + final mpp = data.slice(12, 11); + final legalMpp = + mpp.eq(3) | + (hasSupervisor ? mpp.eq(1) : Const(0)) | + (hasUser ? mpp.eq(0) : Const(0)); + // WARL policy: reserved or unimplemented privileges become M, never an + // unchecked privilege encoding at the data-port permission boundary. + out = mux( + addr12.eq(CsrAddress.mstatus.address), + data.withSet(11, mux(legalMpp, mpp, Const(3, width: 2))), + data, + ); + } // *tvec BASE is the full XLEN address (bits [xlen-1:2]); only the 2-bit MODE // field [1:0] is WARL (River implements direct=0). A 0xFFFFFFFC literal here @@ -1609,7 +1634,9 @@ class RiscVCsrFile extends Module { final mRet = mcur .withSet(3, mcur[7]) // MIE <- MPIE .withSet(7, Const(1, width: 1)) // MPIE <- 1 - .withSet(11, Const(0, width: 2)); // MPP <- U + .withSet(11, Const(!hasHypervisor && !hasUser ? 3 : 0, width: 2)) + // Use the OLD MPP: MRET to M preserves MPRV even as MPP is reset. + .withSet(17, mux(mcur.slice(12, 11).eq(3), mcur[17], Const(0))); mepcBd.wrEn! <= trapToM; mepcBd.wrData! <= _trapPc!; @@ -1646,7 +1673,8 @@ class RiscVCsrFile extends Module { final sRet = mcur .withSet(1, mcur[5]) // SIE <- SPIE .withSet(5, Const(1, width: 1)) // SPIE <- 1 - .withSet(8, Const(0, width: 1)); // SPP <- U + .withSet(8, Const(0, width: 1)) // SPP <- U + .withSet(17, Const(0)); // Every successful SRET returns below M. // One writer for the one register. A trap and an xRET never retire in the // same cycle, so the priority order here only breaks a tie that cannot diff --git a/packages/river_hdl/lib/src/core/mmu.dart b/packages/river_hdl/lib/src/core/mmu.dart index 746a776..7152c03 100644 --- a/packages/river_hdl/lib/src/core/mmu.dart +++ b/packages/river_hdl/lib/src/core/mmu.dart @@ -56,7 +56,8 @@ class RiverMmu extends Module { Logic? virtIn, Logic? gMode, Logic? gRoot, - Logic? privMode, // current effective privilege (for U-bit/SUM checks) + Logic? privMode, // actual instruction-fetch privilege + Logic? dataPrivMode, // effective data privilege; defaults to privMode Logic? sum, // mstatus.SUM (supervisor may access user pages) Logic? mxr, // mstatus.MXR (loads may read execute-only pages) // When true, instruction fetches below machine mode are translated through @@ -113,6 +114,9 @@ class RiverMmu extends Module { // Privilege/permission inputs for leaf checks (U-bit, SUM, MXR). When not // wired, the leaf check falls back to R/W only (the pre-existing behavior). final priv = privMode == null ? null : addInput('priv', privMode, width: 3); + final dataPriv = dataPrivMode == null + ? priv + : addInput('dataPriv', dataPrivMode, width: 3); final sumIn = sum == null ? null : addInput('sum', sum); final mxrIn = mxr == null ? null : addInput('mxr', mxr); final tlbFlushIn = tlbFlush == null @@ -236,9 +240,9 @@ class RiverMmu extends Module { final satpShadowRoot = Logic(name: 'satpShadowRoot', width: xlen); // Shadow of the privilege mode, to detect a context switch for DTLBFC. final privShadow = Logic(name: 'privShadow', width: 3); - final privChanged = priv == null + final privChanged = dataPriv == null ? Const(0) - : priv.neq(privShadow).named('privChanged'); + : dataPriv.neq(privShadow).named('privChanged'); // G-stage (hypervisor second-stage) walk state. Under two-stage, every // VS-stage bus address (`walkAddr`) is guest-physical and must be G-walked @@ -303,7 +307,15 @@ class RiverMmu extends Module { pte[1] | (mxrEff & pte[3]), // read -> R | (MXR & X) ); final permOk = mux(isFetch, pte[3], dataPerm); // fetch -> X - final privEff = atPriv ?? priv; + final privEff = + atPriv ?? + (priv == null && dataPriv == null + ? null + : mux( + isFetch, + priv ?? Const(3, width: 3), + dataPriv ?? Const(3, width: 3), + )); if (privEff == null) return ~permOk; final isUser = privEff.eq(Const(PrivilegeMode.user.id, width: 3)); final isSup = privEff.eq(Const(PrivilegeMode.supervisor.id, width: 3)); @@ -373,20 +385,12 @@ class RiverMmu extends Module { priv.neq(Const(PrivilegeMode.machine.id, width: 3)) & (virtIn == null ? Const(1) : ~virtIn)); - // Data (load/store) translation is ALSO off in machine mode. River does not - // implement mstatus.MPRV, so the effective data privilege is just the - // current privilege: an M-mode load/store is always physical. Without this - // gate, once supervisor enables paging (satp.MODE != 0) an M-mode access - // (e.g. the SBI firmware restoring its own stack in a trap handler) would be - // walked through the SUPERVISOR page tables - its physical address is not a - // valid supervisor VA, so the walk faults or the bus access never returns - // and the core hangs. The one M-mode exception is an explicit virtualized - // access (HLV/HSV): those carry [virtIn] and must translate through the - // guest tables regardless of the current privilege, so OR virtIn back in. - final dataPagingOn = priv == null + // MPRV can select S/U data translation while fetch remains physical in M. + // Retain the existing HLV/HSV override for callers using guest accesses. + final dataPagingOn = dataPriv == null ? pagingOn : (pagingOn & - (priv.neq(Const(PrivilegeMode.machine.id, width: 3)) | + (dataPriv.neq(Const(PrivilegeMode.machine.id, width: 3)) | (virtIn ?? Const(0)))); // Fetch-TLB lookup for the requested fetch address. @@ -534,7 +538,7 @@ class RiverMmu extends Module { dpFaultGuestR < 0, ifFaultR < 0, justCompleted < 0, - privShadow < (priv ?? Const(0, width: 3)), + privShadow < (dataPriv ?? Const(0, width: 3)), // Track satp so the fetch-TLB self-invalidates when it changes, and // flush it on sfence.vma (tlbFlushIn). if (hasPaging) ...[ @@ -657,7 +661,9 @@ class RiverMmu extends Module { wbDatMiso, isFetchWalk, reqWe, - atPriv: priv == null ? null : reqPriv, + atPriv: priv == null && dataPriv == null + ? null + : reqPriv, atSum: sumIn == null ? null : reqSum, atMxr: mxrIn == null ? null : reqMxr, ), @@ -941,7 +947,7 @@ class RiverMmu extends Module { // Freeze the privilege state this access was made at, // so the leaf check at the end of the walk judges the // requester and not whoever is running by then. - reqPriv < (priv ?? Const(0, width: 3)), + reqPriv < (dataPriv ?? Const(3, width: 3)), reqSum < (sumIn ?? Const(0)), reqMxr < (mxrIn ?? Const(0)), walkArmed < 1, @@ -1027,7 +1033,7 @@ class RiverMmu extends Module { reqSize < Const(3, width: 3), // See the data port above: the fetch is judged by the // privilege it was requested at. - reqPriv < (priv ?? Const(0, width: 3)), + reqPriv < (priv ?? Const(3, width: 3)), reqSum < (sumIn ?? Const(0)), reqMxr < (mxrIn ?? Const(0)), walkArmed < 1, @@ -1075,13 +1081,11 @@ class RiverMmu extends Module { // // bit 0 everUser the core was in user mode at all // bit 1 userWalk it was in user mode while a page-table walk ran - // bit 2 latchMismatch a walk accepted in user mode returned its leaf - // while the core was no longer in user mode. This - // is the EXACT condition under which the - // grant-time privilege latch judges an access - // differently from the live-privilege check it - // replaced. - // bit 3 userFault such a walk actually raised a page fault + // bit 2 latchMismatch a walk accepted at effective U privilege returned + // its leaf while the live access privilege was no + // longer U (including MPRV/MPP changes). + // bit 3 userFault an effective-U walk raised a page fault + // Bits 0/1 track actual execution mode, not MPRV data privilege. // // It is a SEPARATE sequential block on purpose: it adds no input to the // walk FSM's condition cone, so it cannot slow down the logic it measures. @@ -1106,7 +1110,11 @@ class RiverMmu extends Module { orElse: [ If(priv.eq(userId), then: [everUser < 1]), If(priv.eq(userId) & walking, then: [userWalk < 1]), - If(acceptedInUser & priv.neq(userId), then: [latchMismatch < 1]), + If( + acceptedInUser & + mux(isFetchWalk, priv, dataPriv ?? priv).neq(userId), + then: [latchMismatch < 1], + ), If( acceptedInUser & leafPermFault( diff --git a/packages/river_hdl/test/csr/mprv_csr_test.dart b/packages/river_hdl/test/csr/mprv_csr_test.dart new file mode 100644 index 0000000..1217f87 --- /dev/null +++ b/packages/river_hdl/test/csr/mprv_csr_test.dart @@ -0,0 +1,135 @@ +import 'dart:async'; + +import 'package:river/river.dart'; +import 'package:river_hdl/river_hdl.dart'; +import 'package:rohd/rohd.dart'; +import 'package:test/test.dart'; + +void main() { + tearDown(Simulator.reset); + for (final xlen in [RiscVMxlen.rv32, RiscVMxlen.rv64]) { + for (final modes in [ + 'M', + 'MU', + 'MSU', + if (xlen == RiscVMxlen.rv64) 'MSUH', + ]) { + final user = modes.contains('U'); + final sup = modes.contains('S'); + final hyp = modes.contains('H'); + for (final scenario in [ + 'availability', + if (!hyp) ...['MPP WARL', 'MRET', if (sup) 'SRET'], + ]) { + test('RV${xlen.size} $modes $scenario', () async { + final clk = SimpleClockGenerator(10).clk; + final reset = Logic()..inject(1); + final mode = Logic(width: 3)..inject(3); + final ret = Logic()..inject(0); + final retM = Logic()..inject(1); + final rd = DataPortInterface(xlen.size, 12); + final wr = DataPortInterface(xlen.size, 12); + rd.en.inject(0); + rd.addr.inject(0x300); + wr.en.inject(0); + wr.addr.inject(0x300); + wr.data.inject(0); + final csr = RiscVCsrFile( + clk, + reset, + mode, + mxlen: xlen, + misa: xlen.misa, + hasUser: user, + hasSupervisor: sup, + hasHypervisor: hyp, + csrRead: rd, + csrWrite: wr, + trapActive: Const(0), + trapTargetIsM: Const(1), + trapPc: Const(0, width: xlen.size), + trapCauseVal: Const(0, width: xlen.size), + trapTval: Const(0, width: xlen.size), + returnActive: ret, + returnFromM: retM, + ); + await csr.build(); + Simulator.setMaxSimTime(2000); + unawaited(Simulator.run()); + const mprv = 1 << 17; + int status() => csr.mstatus.value.toInt(); + Future write(int value, {int address = 0x300}) async { + wr.addr.inject(address); + wr.data.inject(value); + wr.en.inject(1); + await clk.nextNegedge; + expect(wr.valid.value.toBool(), isTrue); + wr.en.inject(0); + await clk.nextNegedge; + } + + try { + await clk.nextNegedge; + reset.inject(0); + await clk.nextNegedge; + switch (scenario) { + case 'availability': + await write(mprv | (3 << 11)); + expect(status() & mprv, user && !hyp ? mprv : 0); + if (sup) { + await write(0, address: 0x100); + expect( + status() & mprv, + hyp ? 0 : mprv, + reason: 'sstatus cannot clear MPRV', + ); + } + await write(3 << 11); + expect(status() & mprv, 0); + case 'MPP WARL': + expect((status() >> 11) & 3, user ? 0 : 3); + for (final requested in [0, 1, 2, 3]) { + await write(requested << 11); + final legal = + requested == 3 || + (requested == 0 && user) || + (requested == 1 && sup); + expect((status() >> 11) & 3, legal ? requested : 3); + } + case 'MRET': + for (final target in [if (user) 0, if (sup) 1, 3]) { + await write(mprv | (target << 11) | (1 << 7)); + ret.inject(1); + await clk.nextNegedge; + ret.inject(0); + await clk.nextNegedge; + expect(status() & mprv, user && target == 3 ? mprv : 0); + expect((status() >> 11) & 3, user ? 0 : 3); + expect(status() & 0x88, 0x88); + } + case 'SRET': + retM.inject(0); + for (final source in [1, 3]) { + for (final spp in [0, 1]) { + mode.inject(3); + await write(mprv | (3 << 11) | (spp << 8) | (1 << 5)); + mode.inject(source); + ret.inject(1); + await clk.nextNegedge; + ret.inject(0); + await clk.nextNegedge; + expect(status() & mprv, 0); + expect((status() >> 11) & 3, 3); + expect(status() & 0x122, 0x22); + } + } + } + } finally { + await Simulator.endSimulation(); + await Simulator.simulationEnded; + } + }); + } + } + } +} diff --git a/packages/river_hdl/test/mmu/mprv_core_microcoded_test.dart b/packages/river_hdl/test/mmu/mprv_core_microcoded_test.dart new file mode 100644 index 0000000..0b58c39 --- /dev/null +++ b/packages/river_hdl/test/mmu/mprv_core_microcoded_test.dart @@ -0,0 +1,3 @@ +import 'mprv_core_test.dart' as cases; + +void main() => cases.runMprvCoreTests(microcoded: true); diff --git a/packages/river_hdl/test/mmu/mprv_core_test.dart b/packages/river_hdl/test/mmu/mprv_core_test.dart new file mode 100644 index 0000000..acf6bbd --- /dev/null +++ b/packages/river_hdl/test/mmu/mprv_core_test.dart @@ -0,0 +1,201 @@ +import 'package:river/river.dart'; +import 'package:rohd/rohd.dart'; +import 'package:test/test.dart'; +import '../core_harness.dart'; + +int _csrw(int addr, int rs) => (addr << 20) | (rs << 15) | 0x1073; +int _csrr(int addr, int rd) => (addr << 20) | (rd << 7) | 0x2073; +int _ld(int rd) => (11 << 15) | (3 << 12) | (rd << 7) | 3; +String _bytes(int value, int count) => List.generate( + count, + (i) => ((value >> (i * 8)) & 255).toRadixString(16).padLeft(2, '0'), +).join(' '); +String _image(List program, int leaf) => + ''' +@0 +${program.map((w) => _bytes(w, 4)).join('\n')} +@100 +${[_csrr(0x342, 8), _csrr(0x343, 9), 0x6f].map((w) => _bytes(w, 4)).join('\n')} +@10000 +${_bytes(0x4401, 8)} +@11000 +${_bytes(0x4801, 8)} +@12100 +${_bytes(0xc000 | leaf, 8)} +@20000 +${_bytes(0x1111, 8)} +@30000 +${_bytes(0x2222, 8)} +'''; + +void main() => runMprvCoreTests(); + +void runMprvCoreTests({bool microcoded = false}) { + tearDown(Simulator.reset); + const mprv = 1 << 17, sum = 1 << 18, mxr = 1 << 19; + for (final cached in [false, true]) { + final config = RiverCoreConfig( + mxlen: RiscVMxlen.rv64, + extensions: kRva22S64Extensions, + type: RiverCoreType.general, + microcodeMode: microcoded ? MicrocodeMode.full : MicrocodeMode.none, + l1cache: cached + ? HarborL1CacheConfig.split( + iSize: 64, + dSize: 256, + ways: 1, + lineSize: 8, + ) + : null, + mmu: HarborMmuConfig( + mxlen: RiscVMxlen.rv64, + pagingModes: const [RiscVPagingMode.bare, RiscVPagingMode.sv39], + tlbLevels: const [], + pmp: HarborPmpConfig.none, + hasSupervisorUserMemory: true, + hasMakeExecutableReadable: true, + ), + interrupts: [], + clock: const HarborClockConfig( + name: 'test', + rate: HarborFixedClockRate(10000), + ), + ); + final seeds = { + Register.x10: 0x8000000000000010, + Register.x11: 0x20000, + Register.x14: 0x100, + }; + test('MPRV physical/translated warm loads cached=$cached', () async { + final program = [ + _csrw(0x180, 10), + _csrw(0x305, 14), + _ld(5), + _ld(6), + _csrw(0x300, 12), + _ld(7), + _ld(8), + _csrw(0x300, 13), + _ld(9), + _csrw(0x300, 0), + _ld(15), + 0x6f, + ]; + await coreTest( + _image(program, 0xcf), + { + Register.x5: 0x1111, + Register.x6: 0x1111, + Register.x7: 0x2222, + Register.x8: 0x2222, + Register.x9: 0x1111, + Register.x15: 0x1111, + }, + config, + initRegisters: { + ...seeds, + Register.x12: mprv | (1 << 11), + Register.x13: mprv | (3 << 11), + }, + nextPc: (program.length - 1) * 4, + maxCycles: 3000, + ); + }); + for (final c in [ + (name: 'MPP S to U', leaf: 0xcf, warm: mprv | (1 << 11), cold: mprv), + ( + name: 'SUM cleared', + leaf: 0xdf, + warm: mprv | (1 << 11) | sum, + cold: mprv | (1 << 11), + ), + ( + name: 'MXR cleared', + leaf: 0xc9, + warm: mprv | (1 << 11) | mxr, + cold: mprv | (1 << 11), + ), + ]) { + test('MPRV ${c.name} denies warm hit cached=$cached', () async { + final program = [ + _csrw(0x180, 10), + _csrw(0x305, 14), + _csrw(0x300, 12), + _ld(5), + _ld(6), + _csrw(0x300, 13), + _ld(7), + // If incorrectly allowed, still reach the checker instead of spinning. + // x7/cause/tval then expose the failure. jal x0, 0x100 from PC=0x1c. + 0x0e40006f, + ]; + await coreTest( + _image(program, c.leaf), + { + Register.x5: 0x2222, + Register.x6: 0x2222, + Register.x7: 0, + Register.x8: 13, + Register.x9: 0x20000, + }, + config, + initRegisters: {...seeds, Register.x12: c.warm, Register.x13: c.cold}, + nextPc: 0x108, + maxCycles: 3000, + ); + }); + } + test('MPRV store uses translated address cached=$cached', () async { + final program = [ + _csrw(0x180, 10), _csrw(0x305, 14), + _csrw(0x300, 12), _ld(5), + (15 << 20) | (11 << 15) | (3 << 12) | 0x23, // sd x15, 0(x11) + _ld(6), _csrw(0x300, 0), _ld(7), 0x6f, + ]; + await coreTest( + _image(program, 0xcf), + {Register.x5: 0x2222, Register.x6: 0x3333, Register.x7: 0x1111}, + config, + initRegisters: { + ...seeds, + Register.x12: mprv | (1 << 11), + Register.x15: 0x3333, + }, + memStates: {0x20000: 0x1111, 0x30000: 0x3333}, + nextPc: 32, + maxCycles: 3000, + ); + }); + test('MRET to M changes effective data privilege cached=$cached', () async { + final program = [ + _csrw(0x180, 10), + _csrw(0x305, 14), + _csrw(0x300, 12), + _ld(5), + _ld(6), + _csrw(0x341, 13), + 0x30200073, + _ld(7), + _ld(8), + 0x6f, + ]; + await coreTest( + _image(program, 0xdf), + { + Register.x5: 0x1111, + Register.x6: 0x1111, + Register.x7: 0x2222, + Register.x8: 0x2222, + }, + config, + initRegisters: { + ...seeds, + Register.x12: mprv | (3 << 11), + Register.x13: 28, + }, + nextPc: 36, + maxCycles: 3000, + ); + }); + } +} diff --git a/packages/river_hdl/test/mmu/mprv_selector_test.dart b/packages/river_hdl/test/mmu/mprv_selector_test.dart new file mode 100644 index 0000000..3363222 --- /dev/null +++ b/packages/river_hdl/test/mmu/mprv_selector_test.dart @@ -0,0 +1,76 @@ +import 'dart:async'; +import 'package:river/river.dart'; +import 'package:river_hdl/river_hdl.dart'; +import 'package:rohd/rohd.dart'; +import 'package:test/test.dart'; + +void main() { + tearDown(Simulator.reset); + for (final xlen in [RiscVMxlen.rv32, RiscVMxlen.rv64]) { + test('RV${xlen.size} white-box selector has no actual-mode gate', () async { + final clk = SimpleClockGenerator(10).clk; + final reset = Logic()..inject(1); + final core = RiverCore( + RiverCoreConfig( + clock: const HarborClockConfig( + name: 'test', + rate: HarborFixedClockRate(10000), + ), + mxlen: xlen, + extensions: [rv32i, if (xlen == RiscVMxlen.rv64) rv64i, rvZicsr], + interrupts: [], + mmu: HarborMmuConfig( + mxlen: xlen, + pagingModes: const [RiscVPagingMode.bare], + tlbLevels: const [], + pmp: HarborPmpConfig.none, + ), + type: RiverCoreType.general, + ), + busConfig: WishboneConfig( + addressWidth: xlen.size, + dataWidth: xlen.size, + selWidth: xlen.size ~/ 8, + ), + ); + core.input('clk').srcConnection! <= clk; + core.input('reset').srcConnection! <= reset; + // Keep the core quiescent: no instruction can complete its first fetch. + core.input('dataBus_ACK').srcConnection! <= Const(0); + core.input('dataBus_DAT_MISO').srcConnection! <= + Const(0, width: xlen.size); + await core.build(); + final csr = core.subModules.whereType().single; + final mode = core.internalSignals.firstWhere((s) => s.name == 'mode'); + final effective = core.internalSignals.firstWhere( + (s) => s.name == 'effectiveDataPriv', + ); + Simulator.setMaxSimTime(5000); + unawaited(Simulator.run()); + try { + await clk.nextNegedge; + reset.inject(0); + await clk.nextNegedge; + // Deliberately injected states. Ordinary xRET clears MPRV below M; + // this is not a claim that software can construct these lower-mode states. + for (final actual in [0, 1, 3]) { + for (final mprv in [0, 1]) { + for (final mpp in [0, 1, 3]) { + mode.inject(actual); + csr.setData( + LogicValue.ofInt(0x300, 12), + LogicValue.ofInt((mprv << 17) | (mpp << 11), xlen.size), + ); + await clk.nextNegedge; + expect(mode.value.toInt(), actual); + expect(effective.value.toInt(), mprv == 1 ? mpp : actual); + } + } + } + } finally { + await Simulator.endSimulation(); + await Simulator.simulationEnded; + } + }); + } +} diff --git a/packages/river_hdl/test/mmu/mprv_walk_test.dart b/packages/river_hdl/test/mmu/mprv_walk_test.dart new file mode 100644 index 0000000..6afdb23 --- /dev/null +++ b/packages/river_hdl/test/mmu/mprv_walk_test.dart @@ -0,0 +1,136 @@ +import 'dart:async'; +import 'package:river/river.dart'; +import 'package:river_hdl/river_hdl.dart'; +import 'package:rohd/rohd.dart'; +import 'package:test/test.dart'; + +void main() { + tearDown(Simulator.reset); + for (final control in ['privilege', 'SUM', 'MXR']) { + for (final tighten in [true, false]) { + test( + 'data walk retains $control on ${tighten ? 'tightening' : 'relaxation'}', + () async { + final clk = SimpleClockGenerator(10).clk; + final reset = Logic()..inject(1); + final en = Logic()..inject(0); + final priv = Logic(width: 3)..inject(1); + final sum = Logic()..inject(0); + final mxr = Logic()..inject(0); + final ack = Logic(); + final data = Logic(width: 64); + final mmu = RiverMmu( + clk, + reset, + Const(0), + Const(0, width: 64), + en, + Const(0x201000, width: 64), + Const(0), + Const(0, width: 64), + Const(3, width: 3), + ack, + data, + mmuConfig: HarborMmuConfig( + mxlen: RiscVMxlen.rv64, + pagingModes: const [RiscVPagingMode.bare, RiscVPagingMode.sv39], + tlbLevels: const [], + pmp: HarborPmpConfig.none, + hasSupervisorUserMemory: true, + hasMakeExecutableReadable: true, + ), + busConfig: WishboneConfig( + addressWidth: 64, + dataWidth: 64, + selWidth: 8, + ), + satpMode: Const(8, width: 4), + satpRoot: Const(0x10, width: 64), + privMode: Const(3, width: 3), + dataPrivMode: priv, + userProbe: true, + sum: sum, + mxr: mxr, + ); + await mmu.build(); + final leaf = control == 'SUM' + ? 0xdf + : control == 'MXR' + ? 0xc9 + : 0xcf; + ack <= mmu.wbCyc & mmu.wbStb; + data <= + mux( + mmu.wbAdr.eq(0x10000), + Const(0x4401, width: 64), + mux( + mmu.wbAdr.eq(0x11008), + Const(0x10000000 | leaf, width: 64), + mux( + mmu.wbAdr.eq(0x40001000), + Const(0x1234, width: 64), + Const(0, width: 64), + ), + ), + ); + void permissions(bool allow) { + priv.inject(control == 'privilege' && !allow ? 0 : 1); + sum.inject(control == 'SUM' && allow ? 1 : 0); + mxr.inject(control == 'MXR' && allow ? 1 : 0); + } + + permissions(tighten); + Simulator.setMaxSimTime(5000); + unawaited(Simulator.run()); + try { + await clk.nextNegedge; + reset.inject(0); + await clk.nextNegedge; + for (final first in [true, false]) { + en.inject(1); + var changed = false; + var completed = false; + final accesses = []; + for (var cycle = 0; cycle < 100; cycle++) { + await clk.nextNegedge; + if (mmu.wbCyc.value.toBool()) { + accesses.add(mmu.wbAdr.value.toInt()); + if (first && !changed) { + // The root request proves acceptance already latched context. + expect(mmu.wbAdr.value.toInt(), 0x10000); + permissions(!tighten); + changed = true; + } + } + if (mmu.dportDone.value.toBool()) { + final allow = first ? tighten : !tighten; + expect(mmu.dportFault.value.toBool(), !allow); + expect(mmu.dportValid.value.toBool(), allow); + expect(accesses.contains(0x40001000), allow); + if (allow) expect(mmu.dportRdata.value.toInt(), 0x1234); + completed = true; + break; + } + } + expect(completed, isTrue, reason: 'request did not complete'); + if (first) expect(changed, isTrue); + en.inject(0); + for (var n = 0; n < 3; n++) { + await clk.nextNegedge; + } + } + final probe = mmu.probe.value.toInt(); + expect(probe & 3, 0, reason: 'actual execution never left M'); + expect(probe & 4, control == 'privilege' && !tighten ? 4 : 0); + // On tightening the later U request faults in the warm DTLB, + // not in a walk; bit 3 deliberately counts only walked faults. + expect(probe & 8, control == 'privilege' && !tighten ? 8 : 0); + } finally { + await Simulator.endSimulation(); + await Simulator.simulationEnded; + } + }, + ); + } + } +} From 0b4c862a3953513d0a798ff1d5767848781c82a4 Mon Sep 17 00:00:00 2001 From: murdoa Date: Tue, 6 Oct 2026 16:09:06 +0100 Subject: [PATCH 2/3] fix(mmu): preserve actual-mode DTLB flush policy --- packages/river_hdl/lib/src/core/mmu.dart | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/packages/river_hdl/lib/src/core/mmu.dart b/packages/river_hdl/lib/src/core/mmu.dart index 7152c03..7cbcebd 100644 --- a/packages/river_hdl/lib/src/core/mmu.dart +++ b/packages/river_hdl/lib/src/core/mmu.dart @@ -238,11 +238,12 @@ class RiverMmu extends Module { final dtlbLevel = Logic(name: 'dtlbLevel', width: 3); final satpShadowMode = Logic(name: 'satpShadowMode', width: 4); final satpShadowRoot = Logic(name: 'satpShadowRoot', width: xlen); - // Shadow of the privilege mode, to detect a context switch for DTLBFC. + // Preserve DTLBFC's actual-mode switch policy. MPRV/MPP changes need no + // flush: each data-TLB hit rechecks permissions with effective privilege. final privShadow = Logic(name: 'privShadow', width: 3); - final privChanged = dataPriv == null + final privChanged = priv == null ? Const(0) - : dataPriv.neq(privShadow).named('privChanged'); + : priv.neq(privShadow).named('privChanged'); // G-stage (hypervisor second-stage) walk state. Under two-stage, every // VS-stage bus address (`walkAddr`) is guest-physical and must be G-walked @@ -538,7 +539,7 @@ class RiverMmu extends Module { dpFaultGuestR < 0, ifFaultR < 0, justCompleted < 0, - privShadow < (dataPriv ?? Const(0, width: 3)), + privShadow < (priv ?? Const(0, width: 3)), // Track satp so the fetch-TLB self-invalidates when it changes, and // flush it on sfence.vma (tlbFlushIn). if (hasPaging) ...[ From 1c118a591e9bf87f9d40812cdb752617626c633e Mon Sep 17 00:00:00 2001 From: murdoa Date: Tue, 6 Oct 2026 16:25:46 +0100 Subject: [PATCH 3/3] test(mmu): consolidate MPRV core test variants --- .../test/mmu/mprv_core_microcoded_test.dart | 3 --- packages/river_hdl/test/mmu/mprv_core_test.dart | 12 +++++++++--- 2 files changed, 9 insertions(+), 6 deletions(-) delete mode 100644 packages/river_hdl/test/mmu/mprv_core_microcoded_test.dart diff --git a/packages/river_hdl/test/mmu/mprv_core_microcoded_test.dart b/packages/river_hdl/test/mmu/mprv_core_microcoded_test.dart deleted file mode 100644 index 0b58c39..0000000 --- a/packages/river_hdl/test/mmu/mprv_core_microcoded_test.dart +++ /dev/null @@ -1,3 +0,0 @@ -import 'mprv_core_test.dart' as cases; - -void main() => cases.runMprvCoreTests(microcoded: true); diff --git a/packages/river_hdl/test/mmu/mprv_core_test.dart b/packages/river_hdl/test/mmu/mprv_core_test.dart index acf6bbd..7fef000 100644 --- a/packages/river_hdl/test/mmu/mprv_core_test.dart +++ b/packages/river_hdl/test/mmu/mprv_core_test.dart @@ -28,10 +28,16 @@ ${_bytes(0x1111, 8)} ${_bytes(0x2222, 8)} '''; -void main() => runMprvCoreTests(); - -void runMprvCoreTests({bool microcoded = false}) { +void main() { tearDown(Simulator.reset); + for (final microcoded in [false, true]) { + group(microcoded ? 'microcoded' : 'static', () { + _runMprvCoreTests(microcoded: microcoded); + }); + } +} + +void _runMprvCoreTests({required bool microcoded}) { const mprv = 1 << 17, sum = 1 << 18, mxr = 1 << 19; for (final cached in [false, true]) { final config = RiverCoreConfig(