* [PATCH v2 1/2] target/riscv: do not raise the no-PMP-rule access fault from mret
2026-10-28 20:01 [PATCH v2 0/2] target/riscv: report the no-PMP-rule fault on the S/U fetch rather than on mret tewaro
@ 2026-10-28 20:01 ` tewaro
2026-10-28 20:01 ` [PATCH v2 2/2] tests/tcg/riscv64: add test for mret to S-mode with no PMP entry tewaro
1 sibling, 0 replies; 3+ messages in thread
From: tewaro @ 2026-10-28 20:01 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-riscv, palmer, alistair.francis, liwei1518, daniel.barboza,
zhiwei_liu, chao.liu, disclosures, tewaro, tewaro
Consider a CPU that implements PMP but has no PMP entry configured.
If M-mode executes mret with MPP set to S or U, QEMU raises an
instruction access fault on the mret. The trap is taken in M-mode,
with mepc pointing at the mret, mtval zero and MPP equal to M.
The spec says the mret succeeds and the first S/U-mode fetch fails
[1][2]. The trap should therefore be taken from S/U-mode, with mepc
and mtval equal to the mret target [3][4] and MPP equal to S/U [5].
The early check in check_ret_from_m_mode() is what moves the fault
onto the mret. Without it, the fetch at the target misses the TB and
TLB caches, which are keyed by privilege level, and reaches
pmp_hart_has_privs_default() through riscv_cpu_tlb_fill(). That
function already denies S/U-mode when no rule matches. Nothing
executes between the privilege switch and that fetch, so no other
behaviour changes.
As it stands, S/U-mode can never be entered on such a configuration,
and a handler that steps mepc past the apparent fault resumes in
M-mode at the S-mode entry point (GitLab issue #2074).
The check was added by commit d102f19a208 alongside the pmp.c change
that makes the no-rule path M-mode-only, and I believe it was already
redundant then. Building v6.0.0 and v7.2.0 with only the check removed
confirms the fetch faults as expected. Later commits (0fbb5d2d3c9,
4c48aad1, 3157a553, 16aa7771) changed its gating, cause and location
but not the normal path. Remove it and the now unused prev_priv
parameter.
The bug was found and debugged with the help of Claude (Anthropic),
used as a research and debugging aid; the patch was written by hand.
[1] Privileged ISA v1.12, 3.7.1 "Physical Memory Protection CSRs",
Priority and Matching Logic
[2] Privileged ISA v1.12, 3.3.2 "Trap-Return Instructions"
[3] Privileged ISA v1.12, 3.1.14 "Machine Exception Program Counter"
[4] Privileged ISA v1.12, 3.1.16 "Machine Trap Value Register"
[5] Privileged ISA v1.12, 3.1.6.1 "Privilege and Global
Interrupt-Enable Stack in mstatus register"
Privileged ISA v1.12 (ratified, 2021-12-03):
https://github.com/riscv/riscv-isa-manual/releases/tag/Priv-v1.12
Current source, same wording:
https://github.com/riscv/riscv-isa-manual/blob/4877e41a/src/priv/machine.adoc
Fixes: d102f19a2085 ("target/riscv/pmp: Raise exception if no PMP entry is configured")
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2074
Signed-off-by: tewaro <tewaro@theorem.dev>
---
v2: rebased onto current master; no change.
target/riscv/tcg/op_helper.c | 10 ++--------
1 file changed, 2 insertions(+), 8 deletions(-)
diff --git a/target/riscv/tcg/op_helper.c b/target/riscv/tcg/op_helper.c
index f9c79fe6b9..e46190079a 100644
--- a/target/riscv/tcg/op_helper.c
+++ b/target/riscv/tcg/op_helper.c
@@ -388,7 +388,6 @@ target_ulong HELPER(sret)(CPURISCVState *env)
}
static void check_ret_from_m_mode(CPURISCVState *env, target_ulong retpc,
- privilege_mode_t prev_priv,
uintptr_t ra)
{
if (!(env->priv >= PRV_M)) {
@@ -400,11 +399,6 @@ static void check_ret_from_m_mode(CPURISCVState *env, target_ulong retpc,
env->misa_ext) && (retpc & 0x3)) {
riscv_raise_exception(env, RISCV_EXCP_INST_ADDR_MIS, ra);
}
-
- if (riscv_cpu_cfg(env)->pmp &&
- !pmp_get_num_rules(env) && (prev_priv != PRV_M)) {
- riscv_raise_exception(env, RISCV_EXCP_INST_ACCESS_FAULT, ra);
- }
}
static target_ulong ssdbltrp_mxret(CPURISCVState *env, target_ulong mstatus,
privilege_mode_t prev_priv,
@@ -430,7 +424,7 @@ target_ulong HELPER(mret)(CPURISCVState *env)
privilege_mode_t prev_priv = get_field(mstatus, MSTATUS_MPP);
uintptr_t ra = GETPC();
- check_ret_from_m_mode(env, retpc, prev_priv, ra);
+ check_ret_from_m_mode(env, retpc, ra);
bool prev_virt = !!(get_field(env->mstatus, MSTATUS_MPV) &&
(prev_priv != PRV_M));
@@ -548,7 +542,7 @@ target_ulong HELPER(mnret)(CPURISCVState *env)
bool prev_virt;
uintptr_t ra = GETPC();
- check_ret_from_m_mode(env, retpc, prev_priv, ra);
+ check_ret_from_m_mode(env, retpc, ra);
prev_virt = !!(get_field(env->mnstatus, MNSTATUS_MNPV) &&
(prev_priv != PRV_M));
--
2.54.0 (Apple Git-157)
^ permalink raw reply related [flat|nested] 3+ messages in thread* [PATCH v2 2/2] tests/tcg/riscv64: add test for mret to S-mode with no PMP entry
2026-10-28 20:01 [PATCH v2 0/2] target/riscv: report the no-PMP-rule fault on the S/U fetch rather than on mret tewaro
2026-10-28 20:01 ` [PATCH v2 1/2] target/riscv: do not raise the no-PMP-rule access fault from mret tewaro
@ 2026-10-28 20:01 ` tewaro
1 sibling, 0 replies; 3+ messages in thread
From: tewaro @ 2026-10-28 20:01 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-riscv, palmer, alistair.francis, liwei1518, daniel.barboza,
zhiwei_liu, chao.liu, disclosures, tewaro, tewaro
Regression test for the previous commit. With PMP implemented and no
entry configured, the test executes mret to S-mode and checks in the
M-mode trap handler that the fault is an instruction access fault
taken from S-mode on the mret target, with mcause 1, mepc and mtval
equal to the target and MPP equal to S. It exits with 0 on success or
the number of the failed check.
Signed-off-by: tewaro <tewaro@theorem.dev>
---
v2: rebased onto current master; the meson.build entry moved to sit
between the tests added since v1. No other change.
tests/tcg/riscv64/system/meson.build | 1 +
tests/tcg/riscv64/test-mret-nopmp.S | 100 +++++++++++++++++++++++++++
2 files changed, 101 insertions(+)
create mode 100644 tests/tcg/riscv64/test-mret-nopmp.S
diff --git a/tests/tcg/riscv64/system/meson.build b/tests/tcg/riscv64/system/meson.build
index 3c8b018d9e..f7410c6a65 100644
--- a/tests/tcg/riscv64/system/meson.build
+++ b/tests/tcg/riscv64/system/meson.build
@@ -25,6 +25,7 @@ tests += {
'test-minstret-ecall.S': setup + {'qemu_args': ['-icount', 'shift=1', qemu_args]},
'test-minstret-wfi.S': setup + {'qemu_args': ['-icount', 'shift=1', qemu_args]},
'test-misa-w.S': setup + {'qemu_args': ['-cpu', 'rv64,x-misa-w=true,c=true,v=true', qemu_args]},
+ 'test-mret-nopmp.S': setup + {'qemu_args': ['-cpu', 'rv64,pmp=true', qemu_args]},
'test-zvabd.S': setup + {'qemu_args': ['-cpu', 'rv64,v=true,vlen=256,x-zvabd=true', qemu_args]},
'test-vsepc-masking.S': setup + {
'test_name': 'test-vsepc-masking-c',
diff --git a/tests/tcg/riscv64/test-mret-nopmp.S b/tests/tcg/riscv64/test-mret-nopmp.S
new file mode 100644
index 0000000000..df20f35bc8
--- /dev/null
+++ b/tests/tcg/riscv64/test-mret-nopmp.S
@@ -0,0 +1,100 @@
+/*
+ * Test mret to S-mode when PMP is implemented but no PMP entry is
+ * configured.
+ *
+ * The mret must succeed and the first S-mode fetch must fail [1][2].
+ * The trap is therefore taken from S-mode, with mcause 1, mepc and
+ * mtval equal to the mret target [3][4] and mstatus.MPP equal to S
+ * [5].
+ *
+ * Exit code is 0 on success, otherwise the number of the failed check.
+ *
+ * [1] Privileged ISA v1.12, 3.7.1 "Physical Memory Protection CSRs",
+ * Priority and Matching Logic
+ * [2] Privileged ISA v1.12, 3.3.2 "Trap-Return Instructions"
+ * [3] Privileged ISA v1.12, 3.1.14 "Machine Exception Program Counter"
+ * [4] Privileged ISA v1.12, 3.1.16 "Machine Trap Value Register"
+ * [5] Privileged ISA v1.12, 3.1.6.1 "Privilege and Global
+ * Interrupt-Enable Stack in mstatus register"
+ *
+ * Privileged ISA v1.12 (ratified, 2021-12-03):
+ * https://github.com/riscv/riscv-isa-manual/releases/tag/Priv-v1.12
+ * Current source, same wording:
+ * https://github.com/riscv/riscv-isa-manual/blob/4877e41a/src/priv/machine.adoc
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+ .option norvc
+
+ .text
+ .global _start
+_start:
+ /* Set up machine trap vector, handle everything in M-mode */
+ lla t0, machine_trap_handler
+ csrw mtvec, t0
+ csrw medeleg, zero
+
+ /* mstatus.MPP = S */
+ li t0, 0x1800
+ csrc mstatus, t0
+ li t0, 0x0800
+ csrs mstatus, t0
+
+ /* Return to S-mode at `target` with no PMP entry configured */
+ lla t0, target
+ csrw mepc, t0
+ mret
+
+target:
+ /* Should be unreachable with no PMP */
+ li a0, 5
+ j _exit
+
+machine_trap_handler:
+ /* 1: mcause is instruction access fault */
+ li a0, 1
+ csrr t0, mcause
+ li t1, 1
+ bne t0, t1, _exit
+
+ /* 2: mepc == target, not mret in _start */
+ li a0, 2
+ csrr t0, mepc
+ lla t1, target
+ bne t0, t1, _exit
+
+ /* 3: mtval == target because aligned instructions */
+ li a0, 3
+ csrr t0, mtval
+ bne t0, t1, _exit
+
+ /* 4: trap from S-mode */
+ li a0, 4
+ csrr t0, mstatus
+ srli t0, t0, 11
+ andi t0, t0, 3
+ li t1, 1
+ bne t0, t1, _exit
+
+ li a0, 0
+ j _exit
+
+/* Exit with semihosting */
+_exit:
+ lla a1, semiargs
+ li t0, 0x20026 /* ADP_Stopped_ApplicationExit */
+ sd t0, 0(a1)
+ sd a0, 8(a1)
+ li a0, 0x20 /* TARGET_SYS_EXIT_EXTENDED */
+
+ /* Semihosting call sequence */
+ slli zero, zero, 0x1f
+ ebreak
+ srai zero, zero, 0x7
+ j .
+
+ .data
+ .balign 8
+semiargs:
+ .space 16
--
2.54.0 (Apple Git-157)
^ permalink raw reply related [flat|nested] 3+ messages in thread