Daniel, Sorry about that; I had some issues on my end using email APIs.
I've resent them here: https://lore.kernel.org/qemu-devel/fbeadd5b2fe5ccfd20b52dbccd4e5856f215ede0.1793217528.git.tew...@theorem.dev/T/#t Thanks, tewaro On Wed, Oct 28, 2026 at 6:31 AM Daniel Henrique Barboza < [email protected]> wrote: > Hi, > > I couldn't apply this patch. git claims that the file is corrupted: > > $ git am --reject \[PATCH\ * > Applying: target/riscv: do not raise the no-PMP-rule access fault from mret > error: corrupt patch at line 18 > Patch failed at 0001 target/riscv: do not raise the no-PMP-rule access > fault from mret > > > If I try to apply patch 2 standalone I get a different error: > > > $ git am \[PATCH\ 2_2\]\ tests_tcg_riscv64_\ add\ test\ for\ mret\ to\ > S-mode\ with\ no\ PMP\ entry\ -\ tewaro\ \([email protected]\)\ -\ > 2026-10-22\ 0157.eml > Applying: tests/tcg/riscv64: add test for mret to S-mode with no PMP entry > error: git diff header lacks filename information when removing 1 leading > pathname component (line 8) > Patch failed at 0001 tests/tcg/riscv64: add test for mret to S-mode with > no PMP entry > hint: Use 'git am --show-current-patch=diff' to see the failed patch > > > Can you please re-send both? Thanks, > > Daniel > > > On 10/22/2026 1:57 AM, 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 > > Signed-off-by: tewaro <[email protected]> > > --- > > 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 b67c7718db..a355d28f4b 100644 > > --- a/target/riscv/tcg/op_helper.c > > +++ b/target/riscv/tcg/op_helper.c > > @@ -423,7 +423,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)) { > > @@ -435,11 +434,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, > > @@ -465,7 +459,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)); > > @@ -515,7 +509,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)); > >
