On 7/3/25 3:21 PM, Charalampos Mitrodimas wrote:
According to the RISC-V Privileged Architecture specification, the low bit of MEPC/SEPC must always be zero. When IALIGN=32, the two low bits must be zero. This commit fixes the behavior of MEPC/SEPC CSR reads and writes, and the implicit reads by MRET/SRET instructions to properly mask the lowest bit(s) based on whether the C extension is enabled: - When C extension is enabled (IALIGN=16): mask bit 0 - When C extension is disabled (IALIGN=32): mask bits [1:0] Previously, when vectored mode bits from STVEC (which sets bit 0 for vectored mode) were written to MEPC, the bits would not be cleared correctly, causing incorrect behavior on MRET. Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2855 Signed-off-by: Charalampos Mitrodimas <charmi...@posteo.net> ---
Reviewed-by: Daniel Henrique Barboza <dbarb...@ventanamicro.com>
target/riscv/csr.c | 8 ++++---- target/riscv/internals.h | 11 +++++++++++ target/riscv/op_helper.c | 4 ++-- 3 files changed, 17 insertions(+), 6 deletions(-) diff --git a/target/riscv/csr.c b/target/riscv/csr.c index fb14972169..c33a6e86d2 100644 --- a/target/riscv/csr.c +++ b/target/riscv/csr.c @@ -3126,14 +3126,14 @@ static RISCVException write_mscratch(CPURISCVState *env, int csrno, static RISCVException read_mepc(CPURISCVState *env, int csrno, target_ulong *val) { - *val = env->mepc; + *val = env->mepc & get_xepc_mask(env); return RISCV_EXCP_NONE; }static RISCVException write_mepc(CPURISCVState *env, int csrno,target_ulong val, uintptr_t ra) { - env->mepc = val; + env->mepc = val & get_xepc_mask(env); return RISCV_EXCP_NONE; }@@ -4113,14 +4113,14 @@ static RISCVException write_sscratch(CPURISCVState *env, int csrno,static RISCVException read_sepc(CPURISCVState *env, int csrno, target_ulong *val) { - *val = env->sepc; + *val = env->sepc & get_xepc_mask(env); return RISCV_EXCP_NONE; }static RISCVException write_sepc(CPURISCVState *env, int csrno,target_ulong val, uintptr_t ra) { - env->sepc = val; + env->sepc = val & get_xepc_mask(env); return RISCV_EXCP_NONE; }diff --git a/target/riscv/internals.h b/target/riscv/internals.hindex 4570bd50be..89ac6a160f 100644 --- a/target/riscv/internals.h +++ b/target/riscv/internals.h @@ -142,6 +142,17 @@ static inline float16 check_nanbox_h(CPURISCVState *env, uint64_t f) } }+static inline target_ulong get_xepc_mask(CPURISCVState *env)+{ + /* When IALIGN=32, both low bits must be zero. + * When IALIGN=16 (has C extension), only bit 0 must be zero. */ + if (riscv_has_ext(env, RVC)) { + return ~(target_ulong)1; + } else { + return ~(target_ulong)3; + } +} + #ifndef CONFIG_USER_ONLY /* Our implementation of SysemuCPUOps::has_work */ bool riscv_cpu_has_work(CPUState *cs); diff --git a/target/riscv/op_helper.c b/target/riscv/op_helper.c index 557807ba4b..15460bf84b 100644 --- a/target/riscv/op_helper.c +++ b/target/riscv/op_helper.c @@ -280,7 +280,7 @@ target_ulong helper_sret(CPURISCVState *env) riscv_raise_exception(env, RISCV_EXCP_ILLEGAL_INST, GETPC()); }- target_ulong retpc = env->sepc;+ target_ulong retpc = env->sepc & get_xepc_mask(env); if (!riscv_cpu_allow_16bit_insn(&env_archcpu(env)->cfg, env->priv_ver, env->misa_ext) && (retpc & 0x3)) { @@ -391,7 +391,7 @@ static target_ulong ssdbltrp_mxret(CPURISCVState *env, target_ulong mstatus,target_ulong helper_mret(CPURISCVState *env){ - target_ulong retpc = env->mepc; + target_ulong retpc = env->mepc & get_xepc_mask(env); uint64_t mstatus = env->mstatus; target_ulong prev_priv = get_field(mstatus, MSTATUS_MPP);
