On Fri, Jul 4, 2025 at 4:25 AM Charalampos Mitrodimas
<charmi...@posteo.net> 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: Alistair Francis <alistair.fran...@wdc.com>
Alistair
> ---
> 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.h
> index 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);
>
> --
> 2.47.2
>
>
