On 10/28/2026 5:01 PM, tewaro wrote:
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
The code LGTM but I don't think this is the right bug. This points to:
"riscv64 cannot use the mret instruction to jump to the address corresponding to s
mode"
A bug that is currently close as non-reproducible since Jan 2024.
Thanks,
Daniel
Signed-off-by: tewaro <[email protected]>
---
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));
