Skip to content

fix(privilege): implement non-H MPRV and xRET handling #7

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
RossComputerGuy merged 3 commits into from
Oct 6, 2026
Merged

fix(privilege): implement non-H MPRV and xRET handling #7

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
da4b1f5
fix(privilege): implement non-H MPRV and clear it on xRET
murdoa Oct 6, 2026
0b4c862
fix(mmu): preserve actual-mode DTLB flush policy
murdoa Oct 6, 2026
1c118a5
test(mmu): consolidate MPRV core test variants
murdoa Oct 6, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 27 additions & 15 deletions packages/river_hdl/lib/src/core.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down Expand Up @@ -426,28 +427,25 @@ 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;
Logic? dCtx;
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.
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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]);
Expand Down
34 changes: 31 additions & 3 deletions packages/river_hdl/lib/src/core/csr.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -615,14 +623,17 @@ 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,
vec: _hasVector,
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,
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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!;
Expand Down Expand Up @@ -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
Expand Down
61 changes: 35 additions & 26 deletions packages/river_hdl/lib/src/core/mmu.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -234,7 +238,8 @@ 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 = priv == null
? Const(0)
Expand Down Expand Up @@ -303,7 +308,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));
Expand Down Expand Up @@ -373,20 +386,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.
Expand Down Expand Up @@ -657,7 +662,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,
),
Expand Down Expand Up @@ -941,7 +948,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,
Expand Down Expand Up @@ -1027,7 +1034,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,
Expand Down Expand Up @@ -1075,13 +1082,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.
Expand All @@ -1106,7 +1111,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(
Expand Down
Loading
Loading