On 10/23/26 07:14, Alistair Francis wrote:
On Thu, 2026-10-22 at 13:21 +0200,
[email protected] wrote:
From: Frédéric Pétrot <[email protected]>
UndefinedBehaviorSanitizer reports an undefined behavior when setting
the Interrupt flag (msb) in the [ms]cause registers due to an out of
bounds shift.
We specifically correct these cases, leaving the ones depending on
specific extensions untouched.
Reported-by: Peter Maydell <[email protected]>
Signed-off-by: Frédéric Pétrot
<[email protected]>
---
target/riscv/cpu.h | 2 ++
target/riscv/tcg/cpu_helper.c | 14 ++++++++++++--
2 files changed, 14 insertions(+), 2 deletions(-)
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index b65d631994..17bfcf38c0 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -354,10 +354,12 @@ struct CPUArchState {
uint64_t stvec;
uint64_t sepc;
uint64_t scause;
+ uint64_t scauseh;
uint64_t mtvec;
uint64_t mepc;
uint64_t mcause;
+ uint64_t mcauseh;
This isn't correct though, there is no mcauseh CSR in RISC-V
I do think this is getting hard to justify as we are adding CSRs and
CPU state that doesn't actually exist and there appears to be no effort
towards making it exist (from a RIV perspective).
It's not clear if these should just be 64-bit CSRs
Fair enough.
Right now the 32 and 64 bits RV specs assume (with a few
exceptions) XLEN csrs.
Some have information in their MSb, e.g. XLEN in misa,
SD in mstatus, Interrupt in mcause.
In the end, I would expect rv128 to align with that (since
it is at zero hardware cost).
uint64_t mtval; /* since: priv-1.10.0 */
uint64_t mctrctl;
diff --git a/target/riscv/tcg/cpu_helper.c
b/target/riscv/tcg/cpu_helper.c
index 58e98e975c..b35b61dc01 100644
--- a/target/riscv/tcg/cpu_helper.c
+++ b/target/riscv/tcg/cpu_helper.c
@@ -2375,7 +2375,12 @@ void riscv_cpu_do_interrupt(CPUState *cs)
}
env->mstatus = s;
sxlen = 16 << riscv_cpu_sxl(env);
- env->scause = cause | ((target_ulong)async << (sxlen - 1));
+ if (riscv_cpu_sxl(env) != MXL_RV128) {
+ env->scause = cause | ((target_ulong)async << (sxlen -
1));
+ } else {
+ env->scause = cause;
+ env->scauseh = (target_ulong)async << 63;
+ }
env->sepc = env->pc;
env->stval = tval;
env->htval = htval;
@@ -2447,7 +2452,12 @@ void riscv_cpu_do_interrupt(CPUState *cs)
s = set_field(s, MSTATUS_MDT, 1);
}
env->mstatus = s;
- env->mcause = cause | ((target_ulong)async << (mxlen - 1));
+ if (riscv_cpu_mxl(env) != MXL_RV128) {
+ env->mcause = cause | ((target_ulong)async << (mxlen -
1));
+ } else {
+ env->mcause = cause;
+ env->mcauseh = (target_ulong)async << 63;
+ }
It's great you set these, but they are never read or used anywhere
I'll do a v3 with the proper "i128" functions to access [ms]cause
(and their dependents) properly.
I wanted at first to answer Peter's concerns quickly.
Frédéric
Alistair
if (smode_double_trap) {
env->mtval2 = env->mcause;
env->mcause = RISCV_EXCP_DOUBLE_TRAP;
--
+---------------------------------------------------------------------------+
| Frédéric Pétrot, Pr. Grenoble INP-UGA@Ensimag/TIMA, MADMAX UGA/CNRS/Inria |
| Mob/Pho: +33 6 74 57 99 65/+33 4 76 57 48 70 Ad augusta per angusta |
| http://tima.univ-grenoble-alpes.fr [email protected] |
+---------------------------------------------------------------------------+
