| Message ID | 20220311094601.30440-1-liweiwei@iscas.ac.cn (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] target/riscv: write back unmodified value for csrrc/csrrs with rs1 is not x0 but holding zero | expand |
On Fri, Mar 11, 2022 at 5:46 PM Weiwei Li <liweiwei@iscas.ac.cn> wrote: > > For csrrs and csrrc, if rs1 specifies a register other than x0, holding > a zero value, the instruction will still attempt to write the unmodified > value back to the csr and will cause side effects > > v2: > * change to explictly pass "bool write_op" argument in riscv_csrrw*, do > write permission check and write operation depend on it > * extend riscv_csr_predicate_fn to pass "write_op" argument which will > be checked by seed csr or other future "write-only" csr The changelog should be put below --- > > Signed-off-by: Weiwei Li <liweiwei@iscas.ac.cn> > Signed-off-by: Junqiang Wang <wangjunqiang@iscas.ac.cn> > --- > target/riscv/cpu.c | 3 +- > target/riscv/cpu.h | 15 +++--- > target/riscv/csr.c | 101 +++++++++++++++++++++------------------ > target/riscv/gdbstub.c | 15 +++--- > target/riscv/op_helper.c | 12 ++--- > 5 files changed, 79 insertions(+), 67 deletions(-) > > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c > index ddda4906ff..76ad9bffac 100644 > --- a/target/riscv/cpu.c > +++ b/target/riscv/cpu.c > @@ -314,7 +314,8 @@ static void riscv_cpu_dump_state(CPUState *cs, FILE *f, int flags) > for (int i = 0; i < ARRAY_SIZE(dump_csrs); ++i) { > int csrno = dump_csrs[i]; > target_ulong val = 0; > - RISCVException res = riscv_csrrw_debug(env, csrno, &val, 0, 0); > + RISCVException res = riscv_csrrw_debug(env, csrno, &val, 0, 0, > + false); > > /* > * Rely on the smode, hmode, etc, predicates within csr.c > diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h > index 9ba05042ed..971d193d61 100644 > --- a/target/riscv/cpu.h > +++ b/target/riscv/cpu.h > @@ -610,27 +610,29 @@ void riscv_cpu_update_mask(CPURISCVState *env); > > RISCVException riscv_csrrw(CPURISCVState *env, int csrno, > target_ulong *ret_value, > - target_ulong new_value, target_ulong write_mask); > + target_ulong new_value, target_ulong write_mask, > + bool write_op); > RISCVException riscv_csrrw_debug(CPURISCVState *env, int csrno, > target_ulong *ret_value, > target_ulong new_value, > - target_ulong write_mask); > + target_ulong write_mask, bool write_op); > > static inline void riscv_csr_write(CPURISCVState *env, int csrno, > target_ulong val) > { > - riscv_csrrw(env, csrno, NULL, val, MAKE_64BIT_MASK(0, TARGET_LONG_BITS)); > + riscv_csrrw(env, csrno, NULL, val, MAKE_64BIT_MASK(0, TARGET_LONG_BITS), > + true); > } > > static inline target_ulong riscv_csr_read(CPURISCVState *env, int csrno) > { > target_ulong val = 0; > - riscv_csrrw(env, csrno, &val, 0, 0); > + riscv_csrrw(env, csrno, &val, 0, 0, false); > return val; > } > > typedef RISCVException (*riscv_csr_predicate_fn)(CPURISCVState *env, > - int csrno); > + int csrno, bool write_op); > typedef RISCVException (*riscv_csr_read_fn)(CPURISCVState *env, int csrno, > target_ulong *ret_value); > typedef RISCVException (*riscv_csr_write_fn)(CPURISCVState *env, int csrno, > @@ -642,7 +644,8 @@ typedef RISCVException (*riscv_csr_op_fn)(CPURISCVState *env, int csrno, > > RISCVException riscv_csrrw_i128(CPURISCVState *env, int csrno, > Int128 *ret_value, > - Int128 new_value, Int128 write_mask); > + Int128 new_value, Int128 write_mask, > + bool write_op); > > typedef RISCVException (*riscv_csr_read128_fn)(CPURISCVState *env, int csrno, > Int128 *ret_value); > diff --git a/target/riscv/csr.c b/target/riscv/csr.c > index aea82dff4a..1907481fb1 100644 > --- a/target/riscv/csr.c > +++ b/target/riscv/csr.c > @@ -36,7 +36,7 @@ void riscv_set_csr_ops(int csrno, riscv_csr_operations *ops) > } > > /* Predicates */ > -static RISCVException fs(CPURISCVState *env, int csrno) > +static RISCVException fs(CPURISCVState *env, int csrno, bool write_op) > { > #if !defined(CONFIG_USER_ONLY) > if (!env->debugger && !riscv_cpu_fp_enabled(env) && > @@ -47,7 +47,7 @@ static RISCVException fs(CPURISCVState *env, int csrno) > return RISCV_EXCP_NONE; > } > > -static RISCVException vs(CPURISCVState *env, int csrno) > +static RISCVException vs(CPURISCVState *env, int csrno, bool write_op) > { > CPUState *cs = env_cpu(env); > RISCVCPU *cpu = RISCV_CPU(cs); > @@ -64,7 +64,7 @@ static RISCVException vs(CPURISCVState *env, int csrno) > return RISCV_EXCP_ILLEGAL_INST; > } > > -static RISCVException ctr(CPURISCVState *env, int csrno) > +static RISCVException ctr(CPURISCVState *env, int csrno, bool write_op) > { > #if !defined(CONFIG_USER_ONLY) > CPUState *cs = env_cpu(env); > @@ -135,50 +135,50 @@ static RISCVException ctr(CPURISCVState *env, int csrno) > return RISCV_EXCP_NONE; > } > > -static RISCVException ctr32(CPURISCVState *env, int csrno) > +static RISCVException ctr32(CPURISCVState *env, int csrno, bool write_op) > { > if (riscv_cpu_mxl(env) != MXL_RV32) { > return RISCV_EXCP_ILLEGAL_INST; > } > > - return ctr(env, csrno); > + return ctr(env, csrno, write_op); > } > > #if !defined(CONFIG_USER_ONLY) > -static RISCVException any(CPURISCVState *env, int csrno) > +static RISCVException any(CPURISCVState *env, int csrno, bool write_op) > { > return RISCV_EXCP_NONE; > } > > -static RISCVException any32(CPURISCVState *env, int csrno) > +static RISCVException any32(CPURISCVState *env, int csrno, bool write_op) > { > if (riscv_cpu_mxl(env) != MXL_RV32) { > return RISCV_EXCP_ILLEGAL_INST; > } > > - return any(env, csrno); > + return any(env, csrno, write_op); > > } > > -static int aia_any(CPURISCVState *env, int csrno) > +static int aia_any(CPURISCVState *env, int csrno, bool write_op) > { > if (!riscv_feature(env, RISCV_FEATURE_AIA)) { > return RISCV_EXCP_ILLEGAL_INST; > } > > - return any(env, csrno); > + return any(env, csrno, write_op); > } > > -static int aia_any32(CPURISCVState *env, int csrno) > +static int aia_any32(CPURISCVState *env, int csrno, bool write_op) > { > if (!riscv_feature(env, RISCV_FEATURE_AIA)) { > return RISCV_EXCP_ILLEGAL_INST; > } > > - return any32(env, csrno); > + return any32(env, csrno, write_op); > } > > -static RISCVException smode(CPURISCVState *env, int csrno) > +static RISCVException smode(CPURISCVState *env, int csrno, bool write_op) > { > if (riscv_has_ext(env, RVS)) { > return RISCV_EXCP_NONE; > @@ -187,34 +187,34 @@ static RISCVException smode(CPURISCVState *env, int csrno) > return RISCV_EXCP_ILLEGAL_INST; > } > > -static int smode32(CPURISCVState *env, int csrno) > +static int smode32(CPURISCVState *env, int csrno, bool write_op) > { > if (riscv_cpu_mxl(env) != MXL_RV32) { > return RISCV_EXCP_ILLEGAL_INST; > } > > - return smode(env, csrno); > + return smode(env, csrno, write_op); > } > > -static int aia_smode(CPURISCVState *env, int csrno) > +static int aia_smode(CPURISCVState *env, int csrno, bool write_op) > { > if (!riscv_feature(env, RISCV_FEATURE_AIA)) { > return RISCV_EXCP_ILLEGAL_INST; > } > > - return smode(env, csrno); > + return smode(env, csrno, write_op); > } > > -static int aia_smode32(CPURISCVState *env, int csrno) > +static int aia_smode32(CPURISCVState *env, int csrno, bool write_op) > { > if (!riscv_feature(env, RISCV_FEATURE_AIA)) { > return RISCV_EXCP_ILLEGAL_INST; > } > > - return smode32(env, csrno); > + return smode32(env, csrno, write_op); > } > > -static RISCVException hmode(CPURISCVState *env, int csrno) > +static RISCVException hmode(CPURISCVState *env, int csrno, bool write_op) > { > if (riscv_has_ext(env, RVS) && > riscv_has_ext(env, RVH)) { > @@ -230,7 +230,7 @@ static RISCVException hmode(CPURISCVState *env, int csrno) > return RISCV_EXCP_ILLEGAL_INST; > } > > -static RISCVException hmode32(CPURISCVState *env, int csrno) > +static RISCVException hmode32(CPURISCVState *env, int csrno, bool write_op) > { > if (riscv_cpu_mxl(env) != MXL_RV32) { > if (!riscv_cpu_virt_enabled(env)) { > @@ -240,12 +240,13 @@ static RISCVException hmode32(CPURISCVState *env, int csrno) > } > } > > - return hmode(env, csrno); > + return hmode(env, csrno, write_op); > > } > > /* Checks if PointerMasking registers could be accessed */ > -static RISCVException pointer_masking(CPURISCVState *env, int csrno) > +static RISCVException pointer_masking(CPURISCVState *env, int csrno, > + bool write_op) > { > /* Check if j-ext is present */ > if (riscv_has_ext(env, RVJ)) { > @@ -254,25 +255,25 @@ static RISCVException pointer_masking(CPURISCVState *env, int csrno) > return RISCV_EXCP_ILLEGAL_INST; > } > > -static int aia_hmode(CPURISCVState *env, int csrno) > +static int aia_hmode(CPURISCVState *env, int csrno, bool write_op) > { > if (!riscv_feature(env, RISCV_FEATURE_AIA)) { > return RISCV_EXCP_ILLEGAL_INST; > } > > - return hmode(env, csrno); > + return hmode(env, csrno, write_op); > } > > -static int aia_hmode32(CPURISCVState *env, int csrno) > +static int aia_hmode32(CPURISCVState *env, int csrno, bool write_op) > { > if (!riscv_feature(env, RISCV_FEATURE_AIA)) { > return RISCV_EXCP_ILLEGAL_INST; > } > > - return hmode32(env, csrno); > + return hmode32(env, csrno, write_op); > } > > -static RISCVException pmp(CPURISCVState *env, int csrno) > +static RISCVException pmp(CPURISCVState *env, int csrno, bool write_op) > { > if (riscv_feature(env, RISCV_FEATURE_PMP)) { > return RISCV_EXCP_NONE; > @@ -281,7 +282,7 @@ static RISCVException pmp(CPURISCVState *env, int csrno) > return RISCV_EXCP_ILLEGAL_INST; > } > > -static RISCVException epmp(CPURISCVState *env, int csrno) > +static RISCVException epmp(CPURISCVState *env, int csrno, bool write_op) > { > if (env->priv == PRV_M && riscv_feature(env, RISCV_FEATURE_EPMP)) { > return RISCV_EXCP_NONE; > @@ -2873,7 +2874,8 @@ static RISCVException write_upmbase(CPURISCVState *env, int csrno, > static inline RISCVException riscv_csrrw_check(CPURISCVState *env, > int csrno, > bool write_mask, > - RISCVCPU *cpu) > + RISCVCPU *cpu, > + bool write_op) > { > /* check privileges and return RISCV_EXCP_ILLEGAL_INST if check fails */ > int read_only = get_field(csrno, 0xC00) == 3; > @@ -2895,7 +2897,7 @@ static inline RISCVException riscv_csrrw_check(CPURISCVState *env, > return RISCV_EXCP_ILLEGAL_INST; > } > #endif > - if (write_mask && read_only) { > + if (write_op && read_only) { > return RISCV_EXCP_ILLEGAL_INST; > } > > @@ -2909,13 +2911,13 @@ static inline RISCVException riscv_csrrw_check(CPURISCVState *env, > return RISCV_EXCP_ILLEGAL_INST; > } > > - return csr_ops[csrno].predicate(env, csrno); > + return csr_ops[csrno].predicate(env, csrno, write_op); > } > > static RISCVException riscv_csrrw_do64(CPURISCVState *env, int csrno, > target_ulong *ret_value, > target_ulong new_value, > - target_ulong write_mask) > + target_ulong write_mask, bool write_op) > { > RISCVException ret; > target_ulong old_value; > @@ -2935,8 +2937,8 @@ static RISCVException riscv_csrrw_do64(CPURISCVState *env, int csrno, > return ret; > } > > - /* write value if writable and write mask set, otherwise drop writes */ > - if (write_mask) { > + /* write value if writable and write_op set, otherwise drop writes */ > + if (write_op) { > new_value = (old_value & ~write_mask) | (new_value & write_mask); > if (csr_ops[csrno].write) { > ret = csr_ops[csrno].write(env, csrno, new_value); > @@ -2956,22 +2958,25 @@ static RISCVException riscv_csrrw_do64(CPURISCVState *env, int csrno, > > RISCVException riscv_csrrw(CPURISCVState *env, int csrno, > target_ulong *ret_value, > - target_ulong new_value, target_ulong write_mask) > + target_ulong new_value, target_ulong write_mask, > + bool write_op) > { > RISCVCPU *cpu = env_archcpu(env); > > - RISCVException ret = riscv_csrrw_check(env, csrno, write_mask, cpu); > + RISCVException ret = riscv_csrrw_check(env, csrno, write_mask, cpu, > + write_op); > if (ret != RISCV_EXCP_NONE) { > return ret; > } > > - return riscv_csrrw_do64(env, csrno, ret_value, new_value, write_mask); > + return riscv_csrrw_do64(env, csrno, ret_value, new_value, write_mask, > + write_op); > } > > static RISCVException riscv_csrrw_do128(CPURISCVState *env, int csrno, > Int128 *ret_value, > Int128 new_value, > - Int128 write_mask) > + Int128 write_mask, bool write_op) > { > RISCVException ret; > Int128 old_value; > @@ -2982,8 +2987,8 @@ static RISCVException riscv_csrrw_do128(CPURISCVState *env, int csrno, > return ret; > } > > - /* write value if writable and write mask set, otherwise drop writes */ > - if (int128_nz(write_mask)) { > + /* write value if writable and write_op set, otherwise drop writes */ > + if (write_op) { > new_value = int128_or(int128_and(old_value, int128_not(write_mask)), > int128_and(new_value, write_mask)); > if (csr_ops[csrno].write128) { > @@ -3010,18 +3015,20 @@ static RISCVException riscv_csrrw_do128(CPURISCVState *env, int csrno, > > RISCVException riscv_csrrw_i128(CPURISCVState *env, int csrno, > Int128 *ret_value, > - Int128 new_value, Int128 write_mask) > + Int128 new_value, Int128 write_mask, > + bool write_op) > { > RISCVException ret; > RISCVCPU *cpu = env_archcpu(env); > > - ret = riscv_csrrw_check(env, csrno, int128_nz(write_mask), cpu); > + ret = riscv_csrrw_check(env, csrno, int128_nz(write_mask), cpu, write_op); > if (ret != RISCV_EXCP_NONE) { > return ret; > } > > if (csr_ops[csrno].read128) { > - return riscv_csrrw_do128(env, csrno, ret_value, new_value, write_mask); > + return riscv_csrrw_do128(env, csrno, ret_value, new_value, write_mask, > + write_op); > } > > /* > @@ -3033,7 +3040,7 @@ RISCVException riscv_csrrw_i128(CPURISCVState *env, int csrno, > target_ulong old_value; > ret = riscv_csrrw_do64(env, csrno, &old_value, > int128_getlo(new_value), > - int128_getlo(write_mask)); > + int128_getlo(write_mask), write_op); > if (ret == RISCV_EXCP_NONE && ret_value) { > *ret_value = int128_make64(old_value); > } > @@ -3047,13 +3054,13 @@ RISCVException riscv_csrrw_i128(CPURISCVState *env, int csrno, > RISCVException riscv_csrrw_debug(CPURISCVState *env, int csrno, > target_ulong *ret_value, > target_ulong new_value, > - target_ulong write_mask) > + target_ulong write_mask, bool write_op) > { > RISCVException ret; > #if !defined(CONFIG_USER_ONLY) > env->debugger = true; > #endif > - ret = riscv_csrrw(env, csrno, ret_value, new_value, write_mask); > + ret = riscv_csrrw(env, csrno, ret_value, new_value, write_mask, write_op); > #if !defined(CONFIG_USER_ONLY) > env->debugger = false; > #endif > diff --git a/target/riscv/gdbstub.c b/target/riscv/gdbstub.c > index 9ed049c29e..5e37ea12cb 100644 > --- a/target/riscv/gdbstub.c > +++ b/target/riscv/gdbstub.c > @@ -124,7 +124,7 @@ static int riscv_gdb_get_fpu(CPURISCVState *env, GByteArray *buf, int n) > * This also works for CSR_FRM and CSR_FCSR. > */ > result = riscv_csrrw_debug(env, n - 32, &val, > - 0, 0); > + 0, 0, false); > if (result == RISCV_EXCP_NONE) { > return gdb_get_regl(buf, val); > } > @@ -147,7 +147,7 @@ static int riscv_gdb_set_fpu(CPURISCVState *env, uint8_t *mem_buf, int n) > * This also works for CSR_FRM and CSR_FCSR. > */ > result = riscv_csrrw_debug(env, n - 32, NULL, > - val, -1); > + val, -1, true); > if (result == RISCV_EXCP_NONE) { > return sizeof(target_ulong); > } > @@ -209,7 +209,7 @@ static int riscv_gdb_get_vector(CPURISCVState *env, GByteArray *buf, int n) > } > > target_ulong val = 0; > - int result = riscv_csrrw_debug(env, csrno, &val, 0, 0); > + int result = riscv_csrrw_debug(env, csrno, &val, 0, 0, false); > > if (result == 0) { > return gdb_get_regl(buf, val); > @@ -236,7 +236,7 @@ static int riscv_gdb_set_vector(CPURISCVState *env, uint8_t *mem_buf, int n) > } > > target_ulong val = ldtul_p(mem_buf); > - int result = riscv_csrrw_debug(env, csrno, NULL, val, -1); > + int result = riscv_csrrw_debug(env, csrno, NULL, val, -1, true); > > if (result == 0) { > return sizeof(target_ulong); > @@ -251,7 +251,7 @@ static int riscv_gdb_get_csr(CPURISCVState *env, GByteArray *buf, int n) > target_ulong val = 0; > int result; > > - result = riscv_csrrw_debug(env, n, &val, 0, 0); > + result = riscv_csrrw_debug(env, n, &val, 0, 0, false); > if (result == RISCV_EXCP_NONE) { > return gdb_get_regl(buf, val); > } > @@ -265,7 +265,7 @@ static int riscv_gdb_set_csr(CPURISCVState *env, uint8_t *mem_buf, int n) > target_ulong val = ldtul_p(mem_buf); > int result; > > - result = riscv_csrrw_debug(env, n, NULL, val, -1); > + result = riscv_csrrw_debug(env, n, NULL, val, -1, true); > if (result == RISCV_EXCP_NONE) { > return sizeof(target_ulong); > } > @@ -319,7 +319,8 @@ static int riscv_gen_dynamic_csr_xml(CPUState *cs, int base_reg) > > for (i = 0; i < CSR_TABLE_SIZE; i++) { > predicate = csr_ops[i].predicate; > - if (predicate && (predicate(env, i) == RISCV_EXCP_NONE)) { > + if (predicate && ((predicate(env, i, false) == RISCV_EXCP_NONE) || > + (predicate(env, i, true) == RISCV_EXCP_NONE))) { > if (csr_ops[i].name) { > g_string_append_printf(s, "<reg name=\"%s\"", csr_ops[i].name); > } else { > diff --git a/target/riscv/op_helper.c b/target/riscv/op_helper.c > index 1a75ba11e6..e0d708091e 100644 > --- a/target/riscv/op_helper.c > +++ b/target/riscv/op_helper.c > @@ -40,7 +40,7 @@ void helper_raise_exception(CPURISCVState *env, uint32_t exception) > target_ulong helper_csrr(CPURISCVState *env, int csr) > { > target_ulong val = 0; > - RISCVException ret = riscv_csrrw(env, csr, &val, 0, 0); > + RISCVException ret = riscv_csrrw(env, csr, &val, 0, 0, false); > > if (ret != RISCV_EXCP_NONE) { > riscv_raise_exception(env, ret, GETPC()); > @@ -51,7 +51,7 @@ target_ulong helper_csrr(CPURISCVState *env, int csr) > void helper_csrw(CPURISCVState *env, int csr, target_ulong src) > { > target_ulong mask = env->xl == MXL_RV32 ? UINT32_MAX : (target_ulong)-1; > - RISCVException ret = riscv_csrrw(env, csr, NULL, src, mask); > + RISCVException ret = riscv_csrrw(env, csr, NULL, src, mask, true); > > if (ret != RISCV_EXCP_NONE) { > riscv_raise_exception(env, ret, GETPC()); > @@ -62,7 +62,7 @@ target_ulong helper_csrrw(CPURISCVState *env, int csr, > target_ulong src, target_ulong write_mask) > { > target_ulong val = 0; > - RISCVException ret = riscv_csrrw(env, csr, &val, src, write_mask); > + RISCVException ret = riscv_csrrw(env, csr, &val, src, write_mask, true); > > if (ret != RISCV_EXCP_NONE) { > riscv_raise_exception(env, ret, GETPC()); > @@ -75,7 +75,7 @@ target_ulong helper_csrr_i128(CPURISCVState *env, int csr) > Int128 rv = int128_zero(); > RISCVException ret = riscv_csrrw_i128(env, csr, &rv, > int128_zero(), > - int128_zero()); > + int128_zero(), false); > > if (ret != RISCV_EXCP_NONE) { > riscv_raise_exception(env, ret, GETPC()); > @@ -90,7 +90,7 @@ void helper_csrw_i128(CPURISCVState *env, int csr, > { > RISCVException ret = riscv_csrrw_i128(env, csr, NULL, > int128_make128(srcl, srch), > - UINT128_MAX); > + UINT128_MAX, true); > > if (ret != RISCV_EXCP_NONE) { > riscv_raise_exception(env, ret, GETPC()); > @@ -104,7 +104,7 @@ target_ulong helper_csrrw_i128(CPURISCVState *env, int csr, > Int128 rv = int128_zero(); > RISCVException ret = riscv_csrrw_i128(env, csr, &rv, > int128_make128(srcl, srch), > - int128_make128(maskl, maskh)); > + int128_make128(maskl, maskh), true); > > if (ret != RISCV_EXCP_NONE) { > riscv_raise_exception(env, ret, GETPC()); I am afraid the adding of "bool write_op" argument was done on many functions, some of which do not look good to me, e.g.: predicate funcs. v1 is much better. Or maybe, is that possible we do something in trans_csrrs() instead? Regards, Bin
在 2022/3/16 下午9:07, Bin Meng 写道: > On Fri, Mar 11, 2022 at 5:46 PM Weiwei Li <liweiwei@iscas.ac.cn> wrote: >> For csrrs and csrrc, if rs1 specifies a register other than x0, holding >> a zero value, the instruction will still attempt to write the unmodified >> value back to the csr and will cause side effects >> >> v2: >> * change to explictly pass "bool write_op" argument in riscv_csrrw*, do >> write permission check and write operation depend on it >> * extend riscv_csr_predicate_fn to pass "write_op" argument which will >> be checked by seed csr or other future "write-only" csr > The changelog should be put below --- > >> Signed-off-by: Weiwei Li <liweiwei@iscas.ac.cn> >> Signed-off-by: Junqiang Wang <wangjunqiang@iscas.ac.cn> >> --- >> target/riscv/cpu.c | 3 +- >> target/riscv/cpu.h | 15 +++--- >> target/riscv/csr.c | 101 +++++++++++++++++++++------------------ >> target/riscv/gdbstub.c | 15 +++--- >> target/riscv/op_helper.c | 12 ++--- >> 5 files changed, 79 insertions(+), 67 deletions(-) >> >> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c >> index ddda4906ff..76ad9bffac 100644 >> --- a/target/riscv/cpu.c >> +++ b/target/riscv/cpu.c >> @@ -314,7 +314,8 @@ static void riscv_cpu_dump_state(CPUState *cs, FILE *f, int flags) >> for (int i = 0; i < ARRAY_SIZE(dump_csrs); ++i) { >> int csrno = dump_csrs[i]; >> target_ulong val = 0; >> - RISCVException res = riscv_csrrw_debug(env, csrno, &val, 0, 0); >> + RISCVException res = riscv_csrrw_debug(env, csrno, &val, 0, 0, >> + false); >> >> /* >> * Rely on the smode, hmode, etc, predicates within csr.c >> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h >> index 9ba05042ed..971d193d61 100644 >> --- a/target/riscv/cpu.h >> +++ b/target/riscv/cpu.h >> @@ -610,27 +610,29 @@ void riscv_cpu_update_mask(CPURISCVState *env); >> >> RISCVException riscv_csrrw(CPURISCVState *env, int csrno, >> target_ulong *ret_value, >> - target_ulong new_value, target_ulong write_mask); >> + target_ulong new_value, target_ulong write_mask, >> + bool write_op); >> RISCVException riscv_csrrw_debug(CPURISCVState *env, int csrno, >> target_ulong *ret_value, >> target_ulong new_value, >> - target_ulong write_mask); >> + target_ulong write_mask, bool write_op); >> >> static inline void riscv_csr_write(CPURISCVState *env, int csrno, >> target_ulong val) >> { >> - riscv_csrrw(env, csrno, NULL, val, MAKE_64BIT_MASK(0, TARGET_LONG_BITS)); >> + riscv_csrrw(env, csrno, NULL, val, MAKE_64BIT_MASK(0, TARGET_LONG_BITS), >> + true); >> } >> >> static inline target_ulong riscv_csr_read(CPURISCVState *env, int csrno) >> { >> target_ulong val = 0; >> - riscv_csrrw(env, csrno, &val, 0, 0); >> + riscv_csrrw(env, csrno, &val, 0, 0, false); >> return val; >> } >> >> typedef RISCVException (*riscv_csr_predicate_fn)(CPURISCVState *env, >> - int csrno); >> + int csrno, bool write_op); >> typedef RISCVException (*riscv_csr_read_fn)(CPURISCVState *env, int csrno, >> target_ulong *ret_value); >> typedef RISCVException (*riscv_csr_write_fn)(CPURISCVState *env, int csrno, >> @@ -642,7 +644,8 @@ typedef RISCVException (*riscv_csr_op_fn)(CPURISCVState *env, int csrno, >> >> RISCVException riscv_csrrw_i128(CPURISCVState *env, int csrno, >> Int128 *ret_value, >> - Int128 new_value, Int128 write_mask); >> + Int128 new_value, Int128 write_mask, >> + bool write_op); >> >> typedef RISCVException (*riscv_csr_read128_fn)(CPURISCVState *env, int csrno, >> Int128 *ret_value); >> diff --git a/target/riscv/csr.c b/target/riscv/csr.c >> index aea82dff4a..1907481fb1 100644 >> --- a/target/riscv/csr.c >> +++ b/target/riscv/csr.c >> @@ -36,7 +36,7 @@ void riscv_set_csr_ops(int csrno, riscv_csr_operations *ops) >> } >> >> /* Predicates */ >> -static RISCVException fs(CPURISCVState *env, int csrno) >> +static RISCVException fs(CPURISCVState *env, int csrno, bool write_op) >> { >> #if !defined(CONFIG_USER_ONLY) >> if (!env->debugger && !riscv_cpu_fp_enabled(env) && >> @@ -47,7 +47,7 @@ static RISCVException fs(CPURISCVState *env, int csrno) >> return RISCV_EXCP_NONE; >> } >> >> -static RISCVException vs(CPURISCVState *env, int csrno) >> +static RISCVException vs(CPURISCVState *env, int csrno, bool write_op) >> { >> CPUState *cs = env_cpu(env); >> RISCVCPU *cpu = RISCV_CPU(cs); >> @@ -64,7 +64,7 @@ static RISCVException vs(CPURISCVState *env, int csrno) >> return RISCV_EXCP_ILLEGAL_INST; >> } >> >> -static RISCVException ctr(CPURISCVState *env, int csrno) >> +static RISCVException ctr(CPURISCVState *env, int csrno, bool write_op) >> { >> #if !defined(CONFIG_USER_ONLY) >> CPUState *cs = env_cpu(env); >> @@ -135,50 +135,50 @@ static RISCVException ctr(CPURISCVState *env, int csrno) >> return RISCV_EXCP_NONE; >> } >> >> -static RISCVException ctr32(CPURISCVState *env, int csrno) >> +static RISCVException ctr32(CPURISCVState *env, int csrno, bool write_op) >> { >> if (riscv_cpu_mxl(env) != MXL_RV32) { >> return RISCV_EXCP_ILLEGAL_INST; >> } >> >> - return ctr(env, csrno); >> + return ctr(env, csrno, write_op); >> } >> >> #if !defined(CONFIG_USER_ONLY) >> -static RISCVException any(CPURISCVState *env, int csrno) >> +static RISCVException any(CPURISCVState *env, int csrno, bool write_op) >> { >> return RISCV_EXCP_NONE; >> } >> >> -static RISCVException any32(CPURISCVState *env, int csrno) >> +static RISCVException any32(CPURISCVState *env, int csrno, bool write_op) >> { >> if (riscv_cpu_mxl(env) != MXL_RV32) { >> return RISCV_EXCP_ILLEGAL_INST; >> } >> >> - return any(env, csrno); >> + return any(env, csrno, write_op); >> >> } >> >> -static int aia_any(CPURISCVState *env, int csrno) >> +static int aia_any(CPURISCVState *env, int csrno, bool write_op) >> { >> if (!riscv_feature(env, RISCV_FEATURE_AIA)) { >> return RISCV_EXCP_ILLEGAL_INST; >> } >> >> - return any(env, csrno); >> + return any(env, csrno, write_op); >> } >> >> -static int aia_any32(CPURISCVState *env, int csrno) >> +static int aia_any32(CPURISCVState *env, int csrno, bool write_op) >> { >> if (!riscv_feature(env, RISCV_FEATURE_AIA)) { >> return RISCV_EXCP_ILLEGAL_INST; >> } >> >> - return any32(env, csrno); >> + return any32(env, csrno, write_op); >> } >> >> -static RISCVException smode(CPURISCVState *env, int csrno) >> +static RISCVException smode(CPURISCVState *env, int csrno, bool write_op) >> { >> if (riscv_has_ext(env, RVS)) { >> return RISCV_EXCP_NONE; >> @@ -187,34 +187,34 @@ static RISCVException smode(CPURISCVState *env, int csrno) >> return RISCV_EXCP_ILLEGAL_INST; >> } >> >> -static int smode32(CPURISCVState *env, int csrno) >> +static int smode32(CPURISCVState *env, int csrno, bool write_op) >> { >> if (riscv_cpu_mxl(env) != MXL_RV32) { >> return RISCV_EXCP_ILLEGAL_INST; >> } >> >> - return smode(env, csrno); >> + return smode(env, csrno, write_op); >> } >> >> -static int aia_smode(CPURISCVState *env, int csrno) >> +static int aia_smode(CPURISCVState *env, int csrno, bool write_op) >> { >> if (!riscv_feature(env, RISCV_FEATURE_AIA)) { >> return RISCV_EXCP_ILLEGAL_INST; >> } >> >> - return smode(env, csrno); >> + return smode(env, csrno, write_op); >> } >> >> -static int aia_smode32(CPURISCVState *env, int csrno) >> +static int aia_smode32(CPURISCVState *env, int csrno, bool write_op) >> { >> if (!riscv_feature(env, RISCV_FEATURE_AIA)) { >> return RISCV_EXCP_ILLEGAL_INST; >> } >> >> - return smode32(env, csrno); >> + return smode32(env, csrno, write_op); >> } >> >> -static RISCVException hmode(CPURISCVState *env, int csrno) >> +static RISCVException hmode(CPURISCVState *env, int csrno, bool write_op) >> { >> if (riscv_has_ext(env, RVS) && >> riscv_has_ext(env, RVH)) { >> @@ -230,7 +230,7 @@ static RISCVException hmode(CPURISCVState *env, int csrno) >> return RISCV_EXCP_ILLEGAL_INST; >> } >> >> -static RISCVException hmode32(CPURISCVState *env, int csrno) >> +static RISCVException hmode32(CPURISCVState *env, int csrno, bool write_op) >> { >> if (riscv_cpu_mxl(env) != MXL_RV32) { >> if (!riscv_cpu_virt_enabled(env)) { >> @@ -240,12 +240,13 @@ static RISCVException hmode32(CPURISCVState *env, int csrno) >> } >> } >> >> - return hmode(env, csrno); >> + return hmode(env, csrno, write_op); >> >> } >> >> /* Checks if PointerMasking registers could be accessed */ >> -static RISCVException pointer_masking(CPURISCVState *env, int csrno) >> +static RISCVException pointer_masking(CPURISCVState *env, int csrno, >> + bool write_op) >> { >> /* Check if j-ext is present */ >> if (riscv_has_ext(env, RVJ)) { >> @@ -254,25 +255,25 @@ static RISCVException pointer_masking(CPURISCVState *env, int csrno) >> return RISCV_EXCP_ILLEGAL_INST; >> } >> >> -static int aia_hmode(CPURISCVState *env, int csrno) >> +static int aia_hmode(CPURISCVState *env, int csrno, bool write_op) >> { >> if (!riscv_feature(env, RISCV_FEATURE_AIA)) { >> return RISCV_EXCP_ILLEGAL_INST; >> } >> >> - return hmode(env, csrno); >> + return hmode(env, csrno, write_op); >> } >> >> -static int aia_hmode32(CPURISCVState *env, int csrno) >> +static int aia_hmode32(CPURISCVState *env, int csrno, bool write_op) >> { >> if (!riscv_feature(env, RISCV_FEATURE_AIA)) { >> return RISCV_EXCP_ILLEGAL_INST; >> } >> >> - return hmode32(env, csrno); >> + return hmode32(env, csrno, write_op); >> } >> >> -static RISCVException pmp(CPURISCVState *env, int csrno) >> +static RISCVException pmp(CPURISCVState *env, int csrno, bool write_op) >> { >> if (riscv_feature(env, RISCV_FEATURE_PMP)) { >> return RISCV_EXCP_NONE; >> @@ -281,7 +282,7 @@ static RISCVException pmp(CPURISCVState *env, int csrno) >> return RISCV_EXCP_ILLEGAL_INST; >> } >> >> -static RISCVException epmp(CPURISCVState *env, int csrno) >> +static RISCVException epmp(CPURISCVState *env, int csrno, bool write_op) >> { >> if (env->priv == PRV_M && riscv_feature(env, RISCV_FEATURE_EPMP)) { >> return RISCV_EXCP_NONE; >> @@ -2873,7 +2874,8 @@ static RISCVException write_upmbase(CPURISCVState *env, int csrno, >> static inline RISCVException riscv_csrrw_check(CPURISCVState *env, >> int csrno, >> bool write_mask, >> - RISCVCPU *cpu) >> + RISCVCPU *cpu, >> + bool write_op) >> { >> /* check privileges and return RISCV_EXCP_ILLEGAL_INST if check fails */ >> int read_only = get_field(csrno, 0xC00) == 3; >> @@ -2895,7 +2897,7 @@ static inline RISCVException riscv_csrrw_check(CPURISCVState *env, >> return RISCV_EXCP_ILLEGAL_INST; >> } >> #endif >> - if (write_mask && read_only) { >> + if (write_op && read_only) { >> return RISCV_EXCP_ILLEGAL_INST; >> } >> >> @@ -2909,13 +2911,13 @@ static inline RISCVException riscv_csrrw_check(CPURISCVState *env, >> return RISCV_EXCP_ILLEGAL_INST; >> } >> >> - return csr_ops[csrno].predicate(env, csrno); >> + return csr_ops[csrno].predicate(env, csrno, write_op); >> } >> >> static RISCVException riscv_csrrw_do64(CPURISCVState *env, int csrno, >> target_ulong *ret_value, >> target_ulong new_value, >> - target_ulong write_mask) >> + target_ulong write_mask, bool write_op) >> { >> RISCVException ret; >> target_ulong old_value; >> @@ -2935,8 +2937,8 @@ static RISCVException riscv_csrrw_do64(CPURISCVState *env, int csrno, >> return ret; >> } >> >> - /* write value if writable and write mask set, otherwise drop writes */ >> - if (write_mask) { >> + /* write value if writable and write_op set, otherwise drop writes */ >> + if (write_op) { >> new_value = (old_value & ~write_mask) | (new_value & write_mask); >> if (csr_ops[csrno].write) { >> ret = csr_ops[csrno].write(env, csrno, new_value); >> @@ -2956,22 +2958,25 @@ static RISCVException riscv_csrrw_do64(CPURISCVState *env, int csrno, >> >> RISCVException riscv_csrrw(CPURISCVState *env, int csrno, >> target_ulong *ret_value, >> - target_ulong new_value, target_ulong write_mask) >> + target_ulong new_value, target_ulong write_mask, >> + bool write_op) >> { >> RISCVCPU *cpu = env_archcpu(env); >> >> - RISCVException ret = riscv_csrrw_check(env, csrno, write_mask, cpu); >> + RISCVException ret = riscv_csrrw_check(env, csrno, write_mask, cpu, >> + write_op); >> if (ret != RISCV_EXCP_NONE) { >> return ret; >> } >> >> - return riscv_csrrw_do64(env, csrno, ret_value, new_value, write_mask); >> + return riscv_csrrw_do64(env, csrno, ret_value, new_value, write_mask, >> + write_op); >> } >> >> static RISCVException riscv_csrrw_do128(CPURISCVState *env, int csrno, >> Int128 *ret_value, >> Int128 new_value, >> - Int128 write_mask) >> + Int128 write_mask, bool write_op) >> { >> RISCVException ret; >> Int128 old_value; >> @@ -2982,8 +2987,8 @@ static RISCVException riscv_csrrw_do128(CPURISCVState *env, int csrno, >> return ret; >> } >> >> - /* write value if writable and write mask set, otherwise drop writes */ >> - if (int128_nz(write_mask)) { >> + /* write value if writable and write_op set, otherwise drop writes */ >> + if (write_op) { >> new_value = int128_or(int128_and(old_value, int128_not(write_mask)), >> int128_and(new_value, write_mask)); >> if (csr_ops[csrno].write128) { >> @@ -3010,18 +3015,20 @@ static RISCVException riscv_csrrw_do128(CPURISCVState *env, int csrno, >> >> RISCVException riscv_csrrw_i128(CPURISCVState *env, int csrno, >> Int128 *ret_value, >> - Int128 new_value, Int128 write_mask) >> + Int128 new_value, Int128 write_mask, >> + bool write_op) >> { >> RISCVException ret; >> RISCVCPU *cpu = env_archcpu(env); >> >> - ret = riscv_csrrw_check(env, csrno, int128_nz(write_mask), cpu); >> + ret = riscv_csrrw_check(env, csrno, int128_nz(write_mask), cpu, write_op); >> if (ret != RISCV_EXCP_NONE) { >> return ret; >> } >> >> if (csr_ops[csrno].read128) { >> - return riscv_csrrw_do128(env, csrno, ret_value, new_value, write_mask); >> + return riscv_csrrw_do128(env, csrno, ret_value, new_value, write_mask, >> + write_op); >> } >> >> /* >> @@ -3033,7 +3040,7 @@ RISCVException riscv_csrrw_i128(CPURISCVState *env, int csrno, >> target_ulong old_value; >> ret = riscv_csrrw_do64(env, csrno, &old_value, >> int128_getlo(new_value), >> - int128_getlo(write_mask)); >> + int128_getlo(write_mask), write_op); >> if (ret == RISCV_EXCP_NONE && ret_value) { >> *ret_value = int128_make64(old_value); >> } >> @@ -3047,13 +3054,13 @@ RISCVException riscv_csrrw_i128(CPURISCVState *env, int csrno, >> RISCVException riscv_csrrw_debug(CPURISCVState *env, int csrno, >> target_ulong *ret_value, >> target_ulong new_value, >> - target_ulong write_mask) >> + target_ulong write_mask, bool write_op) >> { >> RISCVException ret; >> #if !defined(CONFIG_USER_ONLY) >> env->debugger = true; >> #endif >> - ret = riscv_csrrw(env, csrno, ret_value, new_value, write_mask); >> + ret = riscv_csrrw(env, csrno, ret_value, new_value, write_mask, write_op); >> #if !defined(CONFIG_USER_ONLY) >> env->debugger = false; >> #endif >> diff --git a/target/riscv/gdbstub.c b/target/riscv/gdbstub.c >> index 9ed049c29e..5e37ea12cb 100644 >> --- a/target/riscv/gdbstub.c >> +++ b/target/riscv/gdbstub.c >> @@ -124,7 +124,7 @@ static int riscv_gdb_get_fpu(CPURISCVState *env, GByteArray *buf, int n) >> * This also works for CSR_FRM and CSR_FCSR. >> */ >> result = riscv_csrrw_debug(env, n - 32, &val, >> - 0, 0); >> + 0, 0, false); >> if (result == RISCV_EXCP_NONE) { >> return gdb_get_regl(buf, val); >> } >> @@ -147,7 +147,7 @@ static int riscv_gdb_set_fpu(CPURISCVState *env, uint8_t *mem_buf, int n) >> * This also works for CSR_FRM and CSR_FCSR. >> */ >> result = riscv_csrrw_debug(env, n - 32, NULL, >> - val, -1); >> + val, -1, true); >> if (result == RISCV_EXCP_NONE) { >> return sizeof(target_ulong); >> } >> @@ -209,7 +209,7 @@ static int riscv_gdb_get_vector(CPURISCVState *env, GByteArray *buf, int n) >> } >> >> target_ulong val = 0; >> - int result = riscv_csrrw_debug(env, csrno, &val, 0, 0); >> + int result = riscv_csrrw_debug(env, csrno, &val, 0, 0, false); >> >> if (result == 0) { >> return gdb_get_regl(buf, val); >> @@ -236,7 +236,7 @@ static int riscv_gdb_set_vector(CPURISCVState *env, uint8_t *mem_buf, int n) >> } >> >> target_ulong val = ldtul_p(mem_buf); >> - int result = riscv_csrrw_debug(env, csrno, NULL, val, -1); >> + int result = riscv_csrrw_debug(env, csrno, NULL, val, -1, true); >> >> if (result == 0) { >> return sizeof(target_ulong); >> @@ -251,7 +251,7 @@ static int riscv_gdb_get_csr(CPURISCVState *env, GByteArray *buf, int n) >> target_ulong val = 0; >> int result; >> >> - result = riscv_csrrw_debug(env, n, &val, 0, 0); >> + result = riscv_csrrw_debug(env, n, &val, 0, 0, false); >> if (result == RISCV_EXCP_NONE) { >> return gdb_get_regl(buf, val); >> } >> @@ -265,7 +265,7 @@ static int riscv_gdb_set_csr(CPURISCVState *env, uint8_t *mem_buf, int n) >> target_ulong val = ldtul_p(mem_buf); >> int result; >> >> - result = riscv_csrrw_debug(env, n, NULL, val, -1); >> + result = riscv_csrrw_debug(env, n, NULL, val, -1, true); >> if (result == RISCV_EXCP_NONE) { >> return sizeof(target_ulong); >> } >> @@ -319,7 +319,8 @@ static int riscv_gen_dynamic_csr_xml(CPUState *cs, int base_reg) >> >> for (i = 0; i < CSR_TABLE_SIZE; i++) { >> predicate = csr_ops[i].predicate; >> - if (predicate && (predicate(env, i) == RISCV_EXCP_NONE)) { >> + if (predicate && ((predicate(env, i, false) == RISCV_EXCP_NONE) || >> + (predicate(env, i, true) == RISCV_EXCP_NONE))) { >> if (csr_ops[i].name) { >> g_string_append_printf(s, "<reg name=\"%s\"", csr_ops[i].name); >> } else { >> diff --git a/target/riscv/op_helper.c b/target/riscv/op_helper.c >> index 1a75ba11e6..e0d708091e 100644 >> --- a/target/riscv/op_helper.c >> +++ b/target/riscv/op_helper.c >> @@ -40,7 +40,7 @@ void helper_raise_exception(CPURISCVState *env, uint32_t exception) >> target_ulong helper_csrr(CPURISCVState *env, int csr) >> { >> target_ulong val = 0; >> - RISCVException ret = riscv_csrrw(env, csr, &val, 0, 0); >> + RISCVException ret = riscv_csrrw(env, csr, &val, 0, 0, false); >> >> if (ret != RISCV_EXCP_NONE) { >> riscv_raise_exception(env, ret, GETPC()); >> @@ -51,7 +51,7 @@ target_ulong helper_csrr(CPURISCVState *env, int csr) >> void helper_csrw(CPURISCVState *env, int csr, target_ulong src) >> { >> target_ulong mask = env->xl == MXL_RV32 ? UINT32_MAX : (target_ulong)-1; >> - RISCVException ret = riscv_csrrw(env, csr, NULL, src, mask); >> + RISCVException ret = riscv_csrrw(env, csr, NULL, src, mask, true); >> >> if (ret != RISCV_EXCP_NONE) { >> riscv_raise_exception(env, ret, GETPC()); >> @@ -62,7 +62,7 @@ target_ulong helper_csrrw(CPURISCVState *env, int csr, >> target_ulong src, target_ulong write_mask) >> { >> target_ulong val = 0; >> - RISCVException ret = riscv_csrrw(env, csr, &val, src, write_mask); >> + RISCVException ret = riscv_csrrw(env, csr, &val, src, write_mask, true); >> >> if (ret != RISCV_EXCP_NONE) { >> riscv_raise_exception(env, ret, GETPC()); >> @@ -75,7 +75,7 @@ target_ulong helper_csrr_i128(CPURISCVState *env, int csr) >> Int128 rv = int128_zero(); >> RISCVException ret = riscv_csrrw_i128(env, csr, &rv, >> int128_zero(), >> - int128_zero()); >> + int128_zero(), false); >> >> if (ret != RISCV_EXCP_NONE) { >> riscv_raise_exception(env, ret, GETPC()); >> @@ -90,7 +90,7 @@ void helper_csrw_i128(CPURISCVState *env, int csr, >> { >> RISCVException ret = riscv_csrrw_i128(env, csr, NULL, >> int128_make128(srcl, srch), >> - UINT128_MAX); >> + UINT128_MAX, true); >> >> if (ret != RISCV_EXCP_NONE) { >> riscv_raise_exception(env, ret, GETPC()); >> @@ -104,7 +104,7 @@ target_ulong helper_csrrw_i128(CPURISCVState *env, int csr, >> Int128 rv = int128_zero(); >> RISCVException ret = riscv_csrrw_i128(env, csr, &rv, >> int128_make128(srcl, srch), >> - int128_make128(maskl, maskh)); >> + int128_make128(maskl, maskh), true); >> >> if (ret != RISCV_EXCP_NONE) { >> riscv_raise_exception(env, ret, GETPC()); > I am afraid the adding of "bool write_op" argument was done on many > functions, some of which do not look good to me, e.g.: predicate > funcs. v1 is much better. Yeah, I agree adding argument to predicate is not a good idea. However it seems that the csr(like seed) itself cannot distinguish whether it's to be written or not except these two ways(in v1 and v2). Or we can take seed CSR as a special case in riscv_csrrw_check since no other csr will limit itself to be accessed only with read-write instruction currently. > > Or maybe, is that possible we do something in trans_csrrs() instead? The read and write operation in trans_csr* have been truely distinguished in original code: all the read operation will go to the helper_csrr, write to helper_csrrw, read/write to helper_csrrw. However, they all go to the same progress riscv_csrrw* in the helper with different arguments. Regards, Weiwei Li > > Regards, > Bin
On Thu, Mar 17, 2022 at 1:13 AM Weiwei Li <liweiwei@iscas.ac.cn> wrote: > > > 在 2022/3/16 下午9:07, Bin Meng 写道: > > On Fri, Mar 11, 2022 at 5:46 PM Weiwei Li <liweiwei@iscas.ac.cn> wrote: > >> For csrrs and csrrc, if rs1 specifies a register other than x0, holding > >> a zero value, the instruction will still attempt to write the unmodified > >> value back to the csr and will cause side effects > >> > >> v2: > >> * change to explictly pass "bool write_op" argument in riscv_csrrw*, do > >> write permission check and write operation depend on it > >> * extend riscv_csr_predicate_fn to pass "write_op" argument which will > >> be checked by seed csr or other future "write-only" csr > > The changelog should be put below --- > > > >> Signed-off-by: Weiwei Li <liweiwei@iscas.ac.cn> > >> Signed-off-by: Junqiang Wang <wangjunqiang@iscas.ac.cn> > >> --- > >> target/riscv/cpu.c | 3 +- > >> target/riscv/cpu.h | 15 +++--- > >> target/riscv/csr.c | 101 +++++++++++++++++++++------------------ > >> target/riscv/gdbstub.c | 15 +++--- > >> target/riscv/op_helper.c | 12 ++--- > >> 5 files changed, 79 insertions(+), 67 deletions(-) > >> > >> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c > >> index ddda4906ff..76ad9bffac 100644 > >> --- a/target/riscv/cpu.c > >> +++ b/target/riscv/cpu.c > >> @@ -314,7 +314,8 @@ static void riscv_cpu_dump_state(CPUState *cs, FILE *f, int flags) > >> for (int i = 0; i < ARRAY_SIZE(dump_csrs); ++i) { > >> int csrno = dump_csrs[i]; > >> target_ulong val = 0; > >> - RISCVException res = riscv_csrrw_debug(env, csrno, &val, 0, 0); > >> + RISCVException res = riscv_csrrw_debug(env, csrno, &val, 0, 0, > >> + false); > >> > >> /* > >> * Rely on the smode, hmode, etc, predicates within csr.c > >> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h > >> index 9ba05042ed..971d193d61 100644 > >> --- a/target/riscv/cpu.h > >> +++ b/target/riscv/cpu.h > >> @@ -610,27 +610,29 @@ void riscv_cpu_update_mask(CPURISCVState *env); > >> > >> RISCVException riscv_csrrw(CPURISCVState *env, int csrno, > >> target_ulong *ret_value, > >> - target_ulong new_value, target_ulong write_mask); > >> + target_ulong new_value, target_ulong write_mask, > >> + bool write_op); > >> RISCVException riscv_csrrw_debug(CPURISCVState *env, int csrno, > >> target_ulong *ret_value, > >> target_ulong new_value, > >> - target_ulong write_mask); > >> + target_ulong write_mask, bool write_op); > >> > >> static inline void riscv_csr_write(CPURISCVState *env, int csrno, > >> target_ulong val) > >> { > >> - riscv_csrrw(env, csrno, NULL, val, MAKE_64BIT_MASK(0, TARGET_LONG_BITS)); > >> + riscv_csrrw(env, csrno, NULL, val, MAKE_64BIT_MASK(0, TARGET_LONG_BITS), > >> + true); > >> } > >> > >> static inline target_ulong riscv_csr_read(CPURISCVState *env, int csrno) > >> { > >> target_ulong val = 0; > >> - riscv_csrrw(env, csrno, &val, 0, 0); > >> + riscv_csrrw(env, csrno, &val, 0, 0, false); > >> return val; > >> } > >> > >> typedef RISCVException (*riscv_csr_predicate_fn)(CPURISCVState *env, > >> - int csrno); > >> + int csrno, bool write_op); > >> typedef RISCVException (*riscv_csr_read_fn)(CPURISCVState *env, int csrno, > >> target_ulong *ret_value); > >> typedef RISCVException (*riscv_csr_write_fn)(CPURISCVState *env, int csrno, > >> @@ -642,7 +644,8 @@ typedef RISCVException (*riscv_csr_op_fn)(CPURISCVState *env, int csrno, > >> > >> RISCVException riscv_csrrw_i128(CPURISCVState *env, int csrno, > >> Int128 *ret_value, > >> - Int128 new_value, Int128 write_mask); > >> + Int128 new_value, Int128 write_mask, > >> + bool write_op); > >> > >> typedef RISCVException (*riscv_csr_read128_fn)(CPURISCVState *env, int csrno, > >> Int128 *ret_value); > >> diff --git a/target/riscv/csr.c b/target/riscv/csr.c > >> index aea82dff4a..1907481fb1 100644 > >> --- a/target/riscv/csr.c > >> +++ b/target/riscv/csr.c > >> @@ -36,7 +36,7 @@ void riscv_set_csr_ops(int csrno, riscv_csr_operations *ops) > >> } > >> > >> /* Predicates */ > >> -static RISCVException fs(CPURISCVState *env, int csrno) > >> +static RISCVException fs(CPURISCVState *env, int csrno, bool write_op) > >> { > >> #if !defined(CONFIG_USER_ONLY) > >> if (!env->debugger && !riscv_cpu_fp_enabled(env) && > >> @@ -47,7 +47,7 @@ static RISCVException fs(CPURISCVState *env, int csrno) > >> return RISCV_EXCP_NONE; > >> } > >> > >> -static RISCVException vs(CPURISCVState *env, int csrno) > >> +static RISCVException vs(CPURISCVState *env, int csrno, bool write_op) > >> { > >> CPUState *cs = env_cpu(env); > >> RISCVCPU *cpu = RISCV_CPU(cs); > >> @@ -64,7 +64,7 @@ static RISCVException vs(CPURISCVState *env, int csrno) > >> return RISCV_EXCP_ILLEGAL_INST; > >> } > >> > >> -static RISCVException ctr(CPURISCVState *env, int csrno) > >> +static RISCVException ctr(CPURISCVState *env, int csrno, bool write_op) > >> { > >> #if !defined(CONFIG_USER_ONLY) > >> CPUState *cs = env_cpu(env); > >> @@ -135,50 +135,50 @@ static RISCVException ctr(CPURISCVState *env, int csrno) > >> return RISCV_EXCP_NONE; > >> } > >> > >> -static RISCVException ctr32(CPURISCVState *env, int csrno) > >> +static RISCVException ctr32(CPURISCVState *env, int csrno, bool write_op) > >> { > >> if (riscv_cpu_mxl(env) != MXL_RV32) { > >> return RISCV_EXCP_ILLEGAL_INST; > >> } > >> > >> - return ctr(env, csrno); > >> + return ctr(env, csrno, write_op); > >> } > >> > >> #if !defined(CONFIG_USER_ONLY) > >> -static RISCVException any(CPURISCVState *env, int csrno) > >> +static RISCVException any(CPURISCVState *env, int csrno, bool write_op) > >> { > >> return RISCV_EXCP_NONE; > >> } > >> > >> -static RISCVException any32(CPURISCVState *env, int csrno) > >> +static RISCVException any32(CPURISCVState *env, int csrno, bool write_op) > >> { > >> if (riscv_cpu_mxl(env) != MXL_RV32) { > >> return RISCV_EXCP_ILLEGAL_INST; > >> } > >> > >> - return any(env, csrno); > >> + return any(env, csrno, write_op); > >> > >> } > >> > >> -static int aia_any(CPURISCVState *env, int csrno) > >> +static int aia_any(CPURISCVState *env, int csrno, bool write_op) > >> { > >> if (!riscv_feature(env, RISCV_FEATURE_AIA)) { > >> return RISCV_EXCP_ILLEGAL_INST; > >> } > >> > >> - return any(env, csrno); > >> + return any(env, csrno, write_op); > >> } > >> > >> -static int aia_any32(CPURISCVState *env, int csrno) > >> +static int aia_any32(CPURISCVState *env, int csrno, bool write_op) > >> { > >> if (!riscv_feature(env, RISCV_FEATURE_AIA)) { > >> return RISCV_EXCP_ILLEGAL_INST; > >> } > >> > >> - return any32(env, csrno); > >> + return any32(env, csrno, write_op); > >> } > >> > >> -static RISCVException smode(CPURISCVState *env, int csrno) > >> +static RISCVException smode(CPURISCVState *env, int csrno, bool write_op) > >> { > >> if (riscv_has_ext(env, RVS)) { > >> return RISCV_EXCP_NONE; > >> @@ -187,34 +187,34 @@ static RISCVException smode(CPURISCVState *env, int csrno) > >> return RISCV_EXCP_ILLEGAL_INST; > >> } > >> > >> -static int smode32(CPURISCVState *env, int csrno) > >> +static int smode32(CPURISCVState *env, int csrno, bool write_op) > >> { > >> if (riscv_cpu_mxl(env) != MXL_RV32) { > >> return RISCV_EXCP_ILLEGAL_INST; > >> } > >> > >> - return smode(env, csrno); > >> + return smode(env, csrno, write_op); > >> } > >> > >> -static int aia_smode(CPURISCVState *env, int csrno) > >> +static int aia_smode(CPURISCVState *env, int csrno, bool write_op) > >> { > >> if (!riscv_feature(env, RISCV_FEATURE_AIA)) { > >> return RISCV_EXCP_ILLEGAL_INST; > >> } > >> > >> - return smode(env, csrno); > >> + return smode(env, csrno, write_op); > >> } > >> > >> -static int aia_smode32(CPURISCVState *env, int csrno) > >> +static int aia_smode32(CPURISCVState *env, int csrno, bool write_op) > >> { > >> if (!riscv_feature(env, RISCV_FEATURE_AIA)) { > >> return RISCV_EXCP_ILLEGAL_INST; > >> } > >> > >> - return smode32(env, csrno); > >> + return smode32(env, csrno, write_op); > >> } > >> > >> -static RISCVException hmode(CPURISCVState *env, int csrno) > >> +static RISCVException hmode(CPURISCVState *env, int csrno, bool write_op) > >> { > >> if (riscv_has_ext(env, RVS) && > >> riscv_has_ext(env, RVH)) { > >> @@ -230,7 +230,7 @@ static RISCVException hmode(CPURISCVState *env, int csrno) > >> return RISCV_EXCP_ILLEGAL_INST; > >> } > >> > >> -static RISCVException hmode32(CPURISCVState *env, int csrno) > >> +static RISCVException hmode32(CPURISCVState *env, int csrno, bool write_op) > >> { > >> if (riscv_cpu_mxl(env) != MXL_RV32) { > >> if (!riscv_cpu_virt_enabled(env)) { > >> @@ -240,12 +240,13 @@ static RISCVException hmode32(CPURISCVState *env, int csrno) > >> } > >> } > >> > >> - return hmode(env, csrno); > >> + return hmode(env, csrno, write_op); > >> > >> } > >> > >> /* Checks if PointerMasking registers could be accessed */ > >> -static RISCVException pointer_masking(CPURISCVState *env, int csrno) > >> +static RISCVException pointer_masking(CPURISCVState *env, int csrno, > >> + bool write_op) > >> { > >> /* Check if j-ext is present */ > >> if (riscv_has_ext(env, RVJ)) { > >> @@ -254,25 +255,25 @@ static RISCVException pointer_masking(CPURISCVState *env, int csrno) > >> return RISCV_EXCP_ILLEGAL_INST; > >> } > >> > >> -static int aia_hmode(CPURISCVState *env, int csrno) > >> +static int aia_hmode(CPURISCVState *env, int csrno, bool write_op) > >> { > >> if (!riscv_feature(env, RISCV_FEATURE_AIA)) { > >> return RISCV_EXCP_ILLEGAL_INST; > >> } > >> > >> - return hmode(env, csrno); > >> + return hmode(env, csrno, write_op); > >> } > >> > >> -static int aia_hmode32(CPURISCVState *env, int csrno) > >> +static int aia_hmode32(CPURISCVState *env, int csrno, bool write_op) > >> { > >> if (!riscv_feature(env, RISCV_FEATURE_AIA)) { > >> return RISCV_EXCP_ILLEGAL_INST; > >> } > >> > >> - return hmode32(env, csrno); > >> + return hmode32(env, csrno, write_op); > >> } > >> > >> -static RISCVException pmp(CPURISCVState *env, int csrno) > >> +static RISCVException pmp(CPURISCVState *env, int csrno, bool write_op) > >> { > >> if (riscv_feature(env, RISCV_FEATURE_PMP)) { > >> return RISCV_EXCP_NONE; > >> @@ -281,7 +282,7 @@ static RISCVException pmp(CPURISCVState *env, int csrno) > >> return RISCV_EXCP_ILLEGAL_INST; > >> } > >> > >> -static RISCVException epmp(CPURISCVState *env, int csrno) > >> +static RISCVException epmp(CPURISCVState *env, int csrno, bool write_op) > >> { > >> if (env->priv == PRV_M && riscv_feature(env, RISCV_FEATURE_EPMP)) { > >> return RISCV_EXCP_NONE; > >> @@ -2873,7 +2874,8 @@ static RISCVException write_upmbase(CPURISCVState *env, int csrno, > >> static inline RISCVException riscv_csrrw_check(CPURISCVState *env, > >> int csrno, > >> bool write_mask, > >> - RISCVCPU *cpu) > >> + RISCVCPU *cpu, > >> + bool write_op) > >> { > >> /* check privileges and return RISCV_EXCP_ILLEGAL_INST if check fails */ > >> int read_only = get_field(csrno, 0xC00) == 3; > >> @@ -2895,7 +2897,7 @@ static inline RISCVException riscv_csrrw_check(CPURISCVState *env, > >> return RISCV_EXCP_ILLEGAL_INST; > >> } > >> #endif > >> - if (write_mask && read_only) { > >> + if (write_op && read_only) { > >> return RISCV_EXCP_ILLEGAL_INST; > >> } > >> > >> @@ -2909,13 +2911,13 @@ static inline RISCVException riscv_csrrw_check(CPURISCVState *env, > >> return RISCV_EXCP_ILLEGAL_INST; > >> } > >> > >> - return csr_ops[csrno].predicate(env, csrno); > >> + return csr_ops[csrno].predicate(env, csrno, write_op); > >> } > >> > >> static RISCVException riscv_csrrw_do64(CPURISCVState *env, int csrno, > >> target_ulong *ret_value, > >> target_ulong new_value, > >> - target_ulong write_mask) > >> + target_ulong write_mask, bool write_op) > >> { > >> RISCVException ret; > >> target_ulong old_value; > >> @@ -2935,8 +2937,8 @@ static RISCVException riscv_csrrw_do64(CPURISCVState *env, int csrno, > >> return ret; > >> } > >> > >> - /* write value if writable and write mask set, otherwise drop writes */ > >> - if (write_mask) { > >> + /* write value if writable and write_op set, otherwise drop writes */ > >> + if (write_op) { > >> new_value = (old_value & ~write_mask) | (new_value & write_mask); > >> if (csr_ops[csrno].write) { > >> ret = csr_ops[csrno].write(env, csrno, new_value); > >> @@ -2956,22 +2958,25 @@ static RISCVException riscv_csrrw_do64(CPURISCVState *env, int csrno, > >> > >> RISCVException riscv_csrrw(CPURISCVState *env, int csrno, > >> target_ulong *ret_value, > >> - target_ulong new_value, target_ulong write_mask) > >> + target_ulong new_value, target_ulong write_mask, > >> + bool write_op) > >> { > >> RISCVCPU *cpu = env_archcpu(env); > >> > >> - RISCVException ret = riscv_csrrw_check(env, csrno, write_mask, cpu); > >> + RISCVException ret = riscv_csrrw_check(env, csrno, write_mask, cpu, > >> + write_op); > >> if (ret != RISCV_EXCP_NONE) { > >> return ret; > >> } > >> > >> - return riscv_csrrw_do64(env, csrno, ret_value, new_value, write_mask); > >> + return riscv_csrrw_do64(env, csrno, ret_value, new_value, write_mask, > >> + write_op); > >> } > >> > >> static RISCVException riscv_csrrw_do128(CPURISCVState *env, int csrno, > >> Int128 *ret_value, > >> Int128 new_value, > >> - Int128 write_mask) > >> + Int128 write_mask, bool write_op) > >> { > >> RISCVException ret; > >> Int128 old_value; > >> @@ -2982,8 +2987,8 @@ static RISCVException riscv_csrrw_do128(CPURISCVState *env, int csrno, > >> return ret; > >> } > >> > >> - /* write value if writable and write mask set, otherwise drop writes */ > >> - if (int128_nz(write_mask)) { > >> + /* write value if writable and write_op set, otherwise drop writes */ > >> + if (write_op) { > >> new_value = int128_or(int128_and(old_value, int128_not(write_mask)), > >> int128_and(new_value, write_mask)); > >> if (csr_ops[csrno].write128) { > >> @@ -3010,18 +3015,20 @@ static RISCVException riscv_csrrw_do128(CPURISCVState *env, int csrno, > >> > >> RISCVException riscv_csrrw_i128(CPURISCVState *env, int csrno, > >> Int128 *ret_value, > >> - Int128 new_value, Int128 write_mask) > >> + Int128 new_value, Int128 write_mask, > >> + bool write_op) > >> { > >> RISCVException ret; > >> RISCVCPU *cpu = env_archcpu(env); > >> > >> - ret = riscv_csrrw_check(env, csrno, int128_nz(write_mask), cpu); > >> + ret = riscv_csrrw_check(env, csrno, int128_nz(write_mask), cpu, write_op); > >> if (ret != RISCV_EXCP_NONE) { > >> return ret; > >> } > >> > >> if (csr_ops[csrno].read128) { > >> - return riscv_csrrw_do128(env, csrno, ret_value, new_value, write_mask); > >> + return riscv_csrrw_do128(env, csrno, ret_value, new_value, write_mask, > >> + write_op); > >> } > >> > >> /* > >> @@ -3033,7 +3040,7 @@ RISCVException riscv_csrrw_i128(CPURISCVState *env, int csrno, > >> target_ulong old_value; > >> ret = riscv_csrrw_do64(env, csrno, &old_value, > >> int128_getlo(new_value), > >> - int128_getlo(write_mask)); > >> + int128_getlo(write_mask), write_op); > >> if (ret == RISCV_EXCP_NONE && ret_value) { > >> *ret_value = int128_make64(old_value); > >> } > >> @@ -3047,13 +3054,13 @@ RISCVException riscv_csrrw_i128(CPURISCVState *env, int csrno, > >> RISCVException riscv_csrrw_debug(CPURISCVState *env, int csrno, > >> target_ulong *ret_value, > >> target_ulong new_value, > >> - target_ulong write_mask) > >> + target_ulong write_mask, bool write_op) > >> { > >> RISCVException ret; > >> #if !defined(CONFIG_USER_ONLY) > >> env->debugger = true; > >> #endif > >> - ret = riscv_csrrw(env, csrno, ret_value, new_value, write_mask); > >> + ret = riscv_csrrw(env, csrno, ret_value, new_value, write_mask, write_op); > >> #if !defined(CONFIG_USER_ONLY) > >> env->debugger = false; > >> #endif > >> diff --git a/target/riscv/gdbstub.c b/target/riscv/gdbstub.c > >> index 9ed049c29e..5e37ea12cb 100644 > >> --- a/target/riscv/gdbstub.c > >> +++ b/target/riscv/gdbstub.c > >> @@ -124,7 +124,7 @@ static int riscv_gdb_get_fpu(CPURISCVState *env, GByteArray *buf, int n) > >> * This also works for CSR_FRM and CSR_FCSR. > >> */ > >> result = riscv_csrrw_debug(env, n - 32, &val, > >> - 0, 0); > >> + 0, 0, false); > >> if (result == RISCV_EXCP_NONE) { > >> return gdb_get_regl(buf, val); > >> } > >> @@ -147,7 +147,7 @@ static int riscv_gdb_set_fpu(CPURISCVState *env, uint8_t *mem_buf, int n) > >> * This also works for CSR_FRM and CSR_FCSR. > >> */ > >> result = riscv_csrrw_debug(env, n - 32, NULL, > >> - val, -1); > >> + val, -1, true); > >> if (result == RISCV_EXCP_NONE) { > >> return sizeof(target_ulong); > >> } > >> @@ -209,7 +209,7 @@ static int riscv_gdb_get_vector(CPURISCVState *env, GByteArray *buf, int n) > >> } > >> > >> target_ulong val = 0; > >> - int result = riscv_csrrw_debug(env, csrno, &val, 0, 0); > >> + int result = riscv_csrrw_debug(env, csrno, &val, 0, 0, false); > >> > >> if (result == 0) { > >> return gdb_get_regl(buf, val); > >> @@ -236,7 +236,7 @@ static int riscv_gdb_set_vector(CPURISCVState *env, uint8_t *mem_buf, int n) > >> } > >> > >> target_ulong val = ldtul_p(mem_buf); > >> - int result = riscv_csrrw_debug(env, csrno, NULL, val, -1); > >> + int result = riscv_csrrw_debug(env, csrno, NULL, val, -1, true); > >> > >> if (result == 0) { > >> return sizeof(target_ulong); > >> @@ -251,7 +251,7 @@ static int riscv_gdb_get_csr(CPURISCVState *env, GByteArray *buf, int n) > >> target_ulong val = 0; > >> int result; > >> > >> - result = riscv_csrrw_debug(env, n, &val, 0, 0); > >> + result = riscv_csrrw_debug(env, n, &val, 0, 0, false); > >> if (result == RISCV_EXCP_NONE) { > >> return gdb_get_regl(buf, val); > >> } > >> @@ -265,7 +265,7 @@ static int riscv_gdb_set_csr(CPURISCVState *env, uint8_t *mem_buf, int n) > >> target_ulong val = ldtul_p(mem_buf); > >> int result; > >> > >> - result = riscv_csrrw_debug(env, n, NULL, val, -1); > >> + result = riscv_csrrw_debug(env, n, NULL, val, -1, true); > >> if (result == RISCV_EXCP_NONE) { > >> return sizeof(target_ulong); > >> } > >> @@ -319,7 +319,8 @@ static int riscv_gen_dynamic_csr_xml(CPUState *cs, int base_reg) > >> > >> for (i = 0; i < CSR_TABLE_SIZE; i++) { > >> predicate = csr_ops[i].predicate; > >> - if (predicate && (predicate(env, i) == RISCV_EXCP_NONE)) { > >> + if (predicate && ((predicate(env, i, false) == RISCV_EXCP_NONE) || > >> + (predicate(env, i, true) == RISCV_EXCP_NONE))) { > >> if (csr_ops[i].name) { > >> g_string_append_printf(s, "<reg name=\"%s\"", csr_ops[i].name); > >> } else { > >> diff --git a/target/riscv/op_helper.c b/target/riscv/op_helper.c > >> index 1a75ba11e6..e0d708091e 100644 > >> --- a/target/riscv/op_helper.c > >> +++ b/target/riscv/op_helper.c > >> @@ -40,7 +40,7 @@ void helper_raise_exception(CPURISCVState *env, uint32_t exception) > >> target_ulong helper_csrr(CPURISCVState *env, int csr) > >> { > >> target_ulong val = 0; > >> - RISCVException ret = riscv_csrrw(env, csr, &val, 0, 0); > >> + RISCVException ret = riscv_csrrw(env, csr, &val, 0, 0, false); > >> > >> if (ret != RISCV_EXCP_NONE) { > >> riscv_raise_exception(env, ret, GETPC()); > >> @@ -51,7 +51,7 @@ target_ulong helper_csrr(CPURISCVState *env, int csr) > >> void helper_csrw(CPURISCVState *env, int csr, target_ulong src) > >> { > >> target_ulong mask = env->xl == MXL_RV32 ? UINT32_MAX : (target_ulong)-1; > >> - RISCVException ret = riscv_csrrw(env, csr, NULL, src, mask); > >> + RISCVException ret = riscv_csrrw(env, csr, NULL, src, mask, true); > >> > >> if (ret != RISCV_EXCP_NONE) { > >> riscv_raise_exception(env, ret, GETPC()); > >> @@ -62,7 +62,7 @@ target_ulong helper_csrrw(CPURISCVState *env, int csr, > >> target_ulong src, target_ulong write_mask) > >> { > >> target_ulong val = 0; > >> - RISCVException ret = riscv_csrrw(env, csr, &val, src, write_mask); > >> + RISCVException ret = riscv_csrrw(env, csr, &val, src, write_mask, true); > >> > >> if (ret != RISCV_EXCP_NONE) { > >> riscv_raise_exception(env, ret, GETPC()); > >> @@ -75,7 +75,7 @@ target_ulong helper_csrr_i128(CPURISCVState *env, int csr) > >> Int128 rv = int128_zero(); > >> RISCVException ret = riscv_csrrw_i128(env, csr, &rv, > >> int128_zero(), > >> - int128_zero()); > >> + int128_zero(), false); > >> > >> if (ret != RISCV_EXCP_NONE) { > >> riscv_raise_exception(env, ret, GETPC()); > >> @@ -90,7 +90,7 @@ void helper_csrw_i128(CPURISCVState *env, int csr, > >> { > >> RISCVException ret = riscv_csrrw_i128(env, csr, NULL, > >> int128_make128(srcl, srch), > >> - UINT128_MAX); > >> + UINT128_MAX, true); > >> > >> if (ret != RISCV_EXCP_NONE) { > >> riscv_raise_exception(env, ret, GETPC()); > >> @@ -104,7 +104,7 @@ target_ulong helper_csrrw_i128(CPURISCVState *env, int csr, > >> Int128 rv = int128_zero(); > >> RISCVException ret = riscv_csrrw_i128(env, csr, &rv, > >> int128_make128(srcl, srch), > >> - int128_make128(maskl, maskh)); > >> + int128_make128(maskl, maskh), true); > >> > >> if (ret != RISCV_EXCP_NONE) { > >> riscv_raise_exception(env, ret, GETPC()); > > I am afraid the adding of "bool write_op" argument was done on many > > functions, some of which do not look good to me, e.g.: predicate > > funcs. v1 is much better.*> > Yeah, I agree adding argument to predicate is not a good idea. However > it seems that the csr(like seed) > > itself cannot distinguish whether it's to be written or not except > these two ways(in v1 and v2). > > Or we can take seed CSR as a special case in riscv_csrrw_check since no > other csr will limit itself > > to be accessed only with read-write instruction currently. > > > > > Or maybe, is that possible we do something in trans_csrrs() instead? That might be a better option. Unless there are other CSRs we expect to do this I think trans_csrr*() is probably the place to have this, similar to the `rd == 0` case. It could also go in helper_csrr() which would just be a simple if check. Alistair > > The read and write operation in trans_csr* have been truely > distinguished in original code: > > all the read operation will go to the helper_csrr, write to > helper_csrrw, read/write to helper_csrrw. > > However, they all go to the same progress riscv_csrrw* in the helper > with different arguments. > > Regards, > > Weiwei Li > > > > > Regards, > > Bin > > > > >
在 2022/3/17 上午6:35, Alistair Francis 写道: > On Thu, Mar 17, 2022 at 1:13 AM Weiwei Li <liweiwei@iscas.ac.cn> wrote: >> >>>> riscv_raise_exception(env, ret, GETPC()); >>>> @@ -90,7 +90,7 @@ void helper_csrw_i128(CPURISCVState *env, int csr, >>>> { >>>> RISCVException ret = riscv_csrrw_i128(env, csr, NULL, >>>> int128_make128(srcl, srch), >>>> - UINT128_MAX); >>>> + UINT128_MAX, true); >>>> >>>> if (ret != RISCV_EXCP_NONE) { >>>> riscv_raise_exception(env, ret, GETPC()); >>>> @@ -104,7 +104,7 @@ target_ulong helper_csrrw_i128(CPURISCVState *env, int csr, >>>> Int128 rv = int128_zero(); >>>> RISCVException ret = riscv_csrrw_i128(env, csr, &rv, >>>> int128_make128(srcl, srch), >>>> - int128_make128(maskl, maskh)); >>>> + int128_make128(maskl, maskh), true); >>>> >>>> if (ret != RISCV_EXCP_NONE) { >>>> riscv_raise_exception(env, ret, GETPC()); >>> I am afraid the adding of "bool write_op" argument was done on many >>> functions, some of which do not look good to me, e.g.: predicate >>> funcs. v1 is much better.*> >> Yeah, I agree adding argument to predicate is not a good idea. However >> it seems that the csr(like seed) >> >> itself cannot distinguish whether it's to be written or not except >> these two ways(in v1 and v2). >> >> Or we can take seed CSR as a special case in riscv_csrrw_check since no >> other csr will limit itself >> >> to be accessed only with read-write instruction currently. >> >>> Or maybe, is that possible we do something in trans_csrrs() instead? > That might be a better option. Unless there are other CSRs we expect > to do this I think trans_csrr*() is probably the place to have this, > similar to the `rd == 0` case. > > It could also go in helper_csrr() which would just be a simple if check. > > Alistair > Sorry. I don't understand what can be done in trans_csrr*(). As I said in last email: trans_csr* have made all the read operation will go to the helper_csrr. However helper_csrr share the progress of riscv_csrrw* with helper_csrrw/helper_csrw to implement its function. The truely question is that helper_csrr will call riscv_csrrw*with write_mask = zero, new_value=zero, which cannot distinguished from the call from helper_csrrw of which its write_mask all can be zero (origin from trans_csrrs/trans_csrrc when rs1 is not x0 and the value of rs1 reg is zero). Regards, Weiwei Li >> The read and write operation in trans_csr* have been truely >> distinguished in original code: >> >> all the read operation will go to the helper_csrr, write to >> helper_csrrw, read/write to helper_csrrw. >> >> However, they all go to the same progress riscv_csrrw* in the helper >> with different arguments. >> >> Regards, >> >> Weiwei Li >> >>> Regards, >>> Bin >> >> >> >>
On Thu, Mar 17, 2022 at 12:10 PM Weiwei Li <liweiwei@iscas.ac.cn> wrote: > > > 在 2022/3/17 上午6:35, Alistair Francis 写道: > > On Thu, Mar 17, 2022 at 1:13 AM Weiwei Li <liweiwei@iscas.ac.cn> wrote: > >> > >>>> riscv_raise_exception(env, ret, GETPC()); > >>>> @@ -90,7 +90,7 @@ void helper_csrw_i128(CPURISCVState *env, int csr, > >>>> { > >>>> RISCVException ret = riscv_csrrw_i128(env, csr, NULL, > >>>> int128_make128(srcl, srch), > >>>> - UINT128_MAX); > >>>> + UINT128_MAX, true); > >>>> > >>>> if (ret != RISCV_EXCP_NONE) { > >>>> riscv_raise_exception(env, ret, GETPC()); > >>>> @@ -104,7 +104,7 @@ target_ulong helper_csrrw_i128(CPURISCVState *env, int csr, > >>>> Int128 rv = int128_zero(); > >>>> RISCVException ret = riscv_csrrw_i128(env, csr, &rv, > >>>> int128_make128(srcl, srch), > >>>> - int128_make128(maskl, maskh)); > >>>> + int128_make128(maskl, maskh), true); > >>>> > >>>> if (ret != RISCV_EXCP_NONE) { > >>>> riscv_raise_exception(env, ret, GETPC()); > >>> I am afraid the adding of "bool write_op" argument was done on many > >>> functions, some of which do not look good to me, e.g.: predicate > >>> funcs. v1 is much better.*> > >> Yeah, I agree adding argument to predicate is not a good idea. However > >> it seems that the csr(like seed) > >> > >> itself cannot distinguish whether it's to be written or not except > >> these two ways(in v1 and v2). > >> > >> Or we can take seed CSR as a special case in riscv_csrrw_check since no > >> other csr will limit itself > >> > >> to be accessed only with read-write instruction currently. > >> > >>> Or maybe, is that possible we do something in trans_csrrs() instead? > > That might be a better option. Unless there are other CSRs we expect > > to do this I think trans_csrr*() is probably the place to have this, > > similar to the `rd == 0` case. > > > > It could also go in helper_csrr() which would just be a simple if check. > > > > Alistair > > > Sorry. I don't understand what can be done in trans_csrr*(). As I said > in last email: trans_csr* > > have made all the read operation will go to the helper_csrr. However > helper_csrr share the progress > > of riscv_csrrw* with helper_csrrw/helper_csrw to implement its function. Yep, so the problem you are trying to solve is the seed CSR access right? "The seed CSR must be accessed with a read-write instruction. A read-only instruction such as CSRRS/CSRRC with rs1=x0 or CSRRSI/CSRRCI with uimm=0 will raise an illegal instruction exception. The write value (in rs1 or uimm) must be ignored by implementations. The purpose of the write is to signal polling and flushing." So trans_csr*() probably isn't the right place, but you could do the check in helper_csr*() helper_csrr() is only called from do_csrr(), which should only be called on an invalid access for the seed CSR. > > The truely question is that helper_csrr will call riscv_csrrw*with > write_mask = zero, new_value=zero, > > which cannot distinguished from the call from helper_csrrw of which its > write_mask all can be zero Agreed. But helper_csrrw() isn't called if rs1=x0 or uimm=0. So if helper_csrrw() is called no matter the write mask then you should be fine. So a simple: if (csr == CSR_SEED) { riscv_raise_exception() } in helper_csrr() and helper_csrw() should be fine. Or am I missing something? Alistair > > (origin from trans_csrrs/trans_csrrc when rs1 is not x0 and the value of > rs1 reg is zero). > > Regards, > > Weiwei Li > > > >> The read and write operation in trans_csr* have been truely > >> distinguished in original code: > >> > >> all the read operation will go to the helper_csrr, write to > >> helper_csrrw, read/write to helper_csrrw. > >> > >> However, they all go to the same progress riscv_csrrw* in the helper > >> with different arguments. > >> > >> Regards, > >> > >> Weiwei Li > >> > >>> Regards, > >>> Bin > >> > >> > >> > >> >
在 2022/3/17 下午12:39, Alistair Francis 写道: > On Thu, Mar 17, 2022 at 12:10 PM Weiwei Li <liweiwei@iscas.ac.cn> wrote: >> >> 在 2022/3/17 上午6:35, Alistair Francis 写道: >>> On Thu, Mar 17, 2022 at 1:13 AM Weiwei Li <liweiwei@iscas.ac.cn> wrote: >>>>>> riscv_raise_exception(env, ret, GETPC()); >>>>>> @@ -90,7 +90,7 @@ void helper_csrw_i128(CPURISCVState *env, int csr, >>>>>> { >>>>>> RISCVException ret = riscv_csrrw_i128(env, csr, NULL, >>>>>> int128_make128(srcl, srch), >>>>>> - UINT128_MAX); >>>>>> + UINT128_MAX, true); >>>>>> >>>>>> if (ret != RISCV_EXCP_NONE) { >>>>>> riscv_raise_exception(env, ret, GETPC()); >>>>>> @@ -104,7 +104,7 @@ target_ulong helper_csrrw_i128(CPURISCVState *env, int csr, >>>>>> Int128 rv = int128_zero(); >>>>>> RISCVException ret = riscv_csrrw_i128(env, csr, &rv, >>>>>> int128_make128(srcl, srch), >>>>>> - int128_make128(maskl, maskh)); >>>>>> + int128_make128(maskl, maskh), true); >>>>>> >>>>>> if (ret != RISCV_EXCP_NONE) { >>>>>> riscv_raise_exception(env, ret, GETPC()); >>>>> I am afraid the adding of "bool write_op" argument was done on many >>>>> functions, some of which do not look good to me, e.g.: predicate >>>>> funcs. v1 is much better.*> >>>> Yeah, I agree adding argument to predicate is not a good idea. However >>>> it seems that the csr(like seed) >>>> >>>> itself cannot distinguish whether it's to be written or not except >>>> these two ways(in v1 and v2). >>>> >>>> Or we can take seed CSR as a special case in riscv_csrrw_check since no >>>> other csr will limit itself >>>> >>>> to be accessed only with read-write instruction currently. >>>> >>>>> Or maybe, is that possible we do something in trans_csrrs() instead? >>> That might be a better option. Unless there are other CSRs we expect >>> to do this I think trans_csrr*() is probably the place to have this, >>> similar to the `rd == 0` case. >>> >>> It could also go in helper_csrr() which would just be a simple if check. >>> >>> Alistair >>> >> Sorry. I don't understand what can be done in trans_csrr*(). As I said >> in last email: trans_csr* >> >> have made all the read operation will go to the helper_csrr. However >> helper_csrr share the progress >> >> of riscv_csrrw* with helper_csrrw/helper_csrw to implement its function. > Yep, so the problem you are trying to solve is the seed CSR access right? To solve the seed CSR access problem is my initial purpose. The other purpose is to write back the unmodified value back as the coments in trans_csrrs/trans_csrrc: " * Note that if rs1 specifies a register other than x0, holding * a zero value, the instruction will still attempt to write the * unmodified value back to the csr and will cause side effects. */ " For this purpose, we still should distinguish the two cases in riscv_csrrw, since whether write_mask is zero is used to distinguish read and write operation currently. > > "The seed CSR must be accessed with a read-write instruction. A > read-only instruction such as CSRRS/CSRRC with rs1=x0 or CSRRSI/CSRRCI > with uimm=0 will raise an illegal instruction exception. The write > value (in rs1 or uimm) must be ignored by implementations. The purpose > of the write is to signal polling and flushing." > > So trans_csr*() probably isn't the right place, but you could do the > check in helper_csr*() > > helper_csrr() is only called from do_csrr(), which should only be > called on an invalid access for the seed CSR. > >> The truely question is that helper_csrr will call riscv_csrrw*with >> write_mask = zero, new_value=zero, >> >> which cannot distinguished from the call from helper_csrrw of which its >> write_mask all can be zero > Agreed. But helper_csrrw() isn't called if rs1=x0 or uimm=0. So if > helper_csrrw() is called no matter the write mask then you should be > fine. > > So a simple: > if (csr == CSR_SEED) { > riscv_raise_exception() > } > > in helper_csrr() and helper_csrw() should be fine. Or am I missing something? It's OK to do this to solve the seed csr problem. Maybe it's unnecessary to do this in helper_csrw since the spec seems not specify the case that rd = x0 to raise an illegal instruction exception. Regards, Weiwei Li > > Alistair > >> (origin from trans_csrrs/trans_csrrc when rs1 is not x0 and the value of >> rs1 reg is zero). >> >> Regards, >> >> Weiwei Li >> >> >>>> The read and write operation in trans_csr* have been truely >>>> distinguished in original code: >>>> >>>> all the read operation will go to the helper_csrr, write to >>>> helper_csrrw, read/write to helper_csrrw. >>>> >>>> However, they all go to the same progress riscv_csrrw* in the helper >>>> with different arguments. >>>> >>>> Regards, >>>> >>>> Weiwei Li >>>> >>>>> Regards, >>>>> Bin >>>> >>>> >>>>
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c index ddda4906ff..76ad9bffac 100644 --- a/target/riscv/cpu.c +++ b/target/riscv/cpu.c @@ -314,7 +314,8 @@ static void riscv_cpu_dump_state(CPUState *cs, FILE *f, int flags) for (int i = 0; i < ARRAY_SIZE(dump_csrs); ++i) { int csrno = dump_csrs[i]; target_ulong val = 0; - RISCVException res = riscv_csrrw_debug(env, csrno, &val, 0, 0); + RISCVException res = riscv_csrrw_debug(env, csrno, &val, 0, 0, + false); /* * Rely on the smode, hmode, etc, predicates within csr.c diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h index 9ba05042ed..971d193d61 100644 --- a/target/riscv/cpu.h +++ b/target/riscv/cpu.h @@ -610,27 +610,29 @@ void riscv_cpu_update_mask(CPURISCVState *env); RISCVException riscv_csrrw(CPURISCVState *env, int csrno, target_ulong *ret_value, - target_ulong new_value, target_ulong write_mask); + target_ulong new_value, target_ulong write_mask, + bool write_op); RISCVException riscv_csrrw_debug(CPURISCVState *env, int csrno, target_ulong *ret_value, target_ulong new_value, - target_ulong write_mask); + target_ulong write_mask, bool write_op); static inline void riscv_csr_write(CPURISCVState *env, int csrno, target_ulong val) { - riscv_csrrw(env, csrno, NULL, val, MAKE_64BIT_MASK(0, TARGET_LONG_BITS)); + riscv_csrrw(env, csrno, NULL, val, MAKE_64BIT_MASK(0, TARGET_LONG_BITS), + true); } static inline target_ulong riscv_csr_read(CPURISCVState *env, int csrno) { target_ulong val = 0; - riscv_csrrw(env, csrno, &val, 0, 0); + riscv_csrrw(env, csrno, &val, 0, 0, false); return val; } typedef RISCVException (*riscv_csr_predicate_fn)(CPURISCVState *env, - int csrno); + int csrno, bool write_op); typedef RISCVException (*riscv_csr_read_fn)(CPURISCVState *env, int csrno, target_ulong *ret_value); typedef RISCVException (*riscv_csr_write_fn)(CPURISCVState *env, int csrno, @@ -642,7 +644,8 @@ typedef RISCVException (*riscv_csr_op_fn)(CPURISCVState *env, int csrno, RISCVException riscv_csrrw_i128(CPURISCVState *env, int csrno, Int128 *ret_value, - Int128 new_value, Int128 write_mask); + Int128 new_value, Int128 write_mask, + bool write_op); typedef RISCVException (*riscv_csr_read128_fn)(CPURISCVState *env, int csrno, Int128 *ret_value); diff --git a/target/riscv/csr.c b/target/riscv/csr.c index aea82dff4a..1907481fb1 100644 --- a/target/riscv/csr.c +++ b/target/riscv/csr.c @@ -36,7 +36,7 @@ void riscv_set_csr_ops(int csrno, riscv_csr_operations *ops) } /* Predicates */ -static RISCVException fs(CPURISCVState *env, int csrno) +static RISCVException fs(CPURISCVState *env, int csrno, bool write_op) { #if !defined(CONFIG_USER_ONLY) if (!env->debugger && !riscv_cpu_fp_enabled(env) && @@ -47,7 +47,7 @@ static RISCVException fs(CPURISCVState *env, int csrno) return RISCV_EXCP_NONE; } -static RISCVException vs(CPURISCVState *env, int csrno) +static RISCVException vs(CPURISCVState *env, int csrno, bool write_op) { CPUState *cs = env_cpu(env); RISCVCPU *cpu = RISCV_CPU(cs); @@ -64,7 +64,7 @@ static RISCVException vs(CPURISCVState *env, int csrno) return RISCV_EXCP_ILLEGAL_INST; } -static RISCVException ctr(CPURISCVState *env, int csrno) +static RISCVException ctr(CPURISCVState *env, int csrno, bool write_op) { #if !defined(CONFIG_USER_ONLY) CPUState *cs = env_cpu(env); @@ -135,50 +135,50 @@ static RISCVException ctr(CPURISCVState *env, int csrno) return RISCV_EXCP_NONE; } -static RISCVException ctr32(CPURISCVState *env, int csrno) +static RISCVException ctr32(CPURISCVState *env, int csrno, bool write_op) { if (riscv_cpu_mxl(env) != MXL_RV32) { return RISCV_EXCP_ILLEGAL_INST; } - return ctr(env, csrno); + return ctr(env, csrno, write_op); } #if !defined(CONFIG_USER_ONLY) -static RISCVException any(CPURISCVState *env, int csrno) +static RISCVException any(CPURISCVState *env, int csrno, bool write_op) { return RISCV_EXCP_NONE; } -static RISCVException any32(CPURISCVState *env, int csrno) +static RISCVException any32(CPURISCVState *env, int csrno, bool write_op) { if (riscv_cpu_mxl(env) != MXL_RV32) { return RISCV_EXCP_ILLEGAL_INST; } - return any(env, csrno); + return any(env, csrno, write_op); } -static int aia_any(CPURISCVState *env, int csrno) +static int aia_any(CPURISCVState *env, int csrno, bool write_op) { if (!riscv_feature(env, RISCV_FEATURE_AIA)) { return RISCV_EXCP_ILLEGAL_INST; } - return any(env, csrno); + return any(env, csrno, write_op); } -static int aia_any32(CPURISCVState *env, int csrno) +static int aia_any32(CPURISCVState *env, int csrno, bool write_op) { if (!riscv_feature(env, RISCV_FEATURE_AIA)) { return RISCV_EXCP_ILLEGAL_INST; } - return any32(env, csrno); + return any32(env, csrno, write_op); } -static RISCVException smode(CPURISCVState *env, int csrno) +static RISCVException smode(CPURISCVState *env, int csrno, bool write_op) { if (riscv_has_ext(env, RVS)) { return RISCV_EXCP_NONE; @@ -187,34 +187,34 @@ static RISCVException smode(CPURISCVState *env, int csrno) return RISCV_EXCP_ILLEGAL_INST; } -static int smode32(CPURISCVState *env, int csrno) +static int smode32(CPURISCVState *env, int csrno, bool write_op) { if (riscv_cpu_mxl(env) != MXL_RV32) { return RISCV_EXCP_ILLEGAL_INST; } - return smode(env, csrno); + return smode(env, csrno, write_op); } -static int aia_smode(CPURISCVState *env, int csrno) +static int aia_smode(CPURISCVState *env, int csrno, bool write_op) { if (!riscv_feature(env, RISCV_FEATURE_AIA)) { return RISCV_EXCP_ILLEGAL_INST; } - return smode(env, csrno); + return smode(env, csrno, write_op); } -static int aia_smode32(CPURISCVState *env, int csrno) +static int aia_smode32(CPURISCVState *env, int csrno, bool write_op) { if (!riscv_feature(env, RISCV_FEATURE_AIA)) { return RISCV_EXCP_ILLEGAL_INST; } - return smode32(env, csrno); + return smode32(env, csrno, write_op); } -static RISCVException hmode(CPURISCVState *env, int csrno) +static RISCVException hmode(CPURISCVState *env, int csrno, bool write_op) { if (riscv_has_ext(env, RVS) && riscv_has_ext(env, RVH)) { @@ -230,7 +230,7 @@ static RISCVException hmode(CPURISCVState *env, int csrno) return RISCV_EXCP_ILLEGAL_INST; } -static RISCVException hmode32(CPURISCVState *env, int csrno) +static RISCVException hmode32(CPURISCVState *env, int csrno, bool write_op) { if (riscv_cpu_mxl(env) != MXL_RV32) { if (!riscv_cpu_virt_enabled(env)) { @@ -240,12 +240,13 @@ static RISCVException hmode32(CPURISCVState *env, int csrno) } } - return hmode(env, csrno); + return hmode(env, csrno, write_op); } /* Checks if PointerMasking registers could be accessed */ -static RISCVException pointer_masking(CPURISCVState *env, int csrno) +static RISCVException pointer_masking(CPURISCVState *env, int csrno, + bool write_op) { /* Check if j-ext is present */ if (riscv_has_ext(env, RVJ)) { @@ -254,25 +255,25 @@ static RISCVException pointer_masking(CPURISCVState *env, int csrno) return RISCV_EXCP_ILLEGAL_INST; } -static int aia_hmode(CPURISCVState *env, int csrno) +static int aia_hmode(CPURISCVState *env, int csrno, bool write_op) { if (!riscv_feature(env, RISCV_FEATURE_AIA)) { return RISCV_EXCP_ILLEGAL_INST; } - return hmode(env, csrno); + return hmode(env, csrno, write_op); } -static int aia_hmode32(CPURISCVState *env, int csrno) +static int aia_hmode32(CPURISCVState *env, int csrno, bool write_op) { if (!riscv_feature(env, RISCV_FEATURE_AIA)) { return RISCV_EXCP_ILLEGAL_INST; } - return hmode32(env, csrno); + return hmode32(env, csrno, write_op); } -static RISCVException pmp(CPURISCVState *env, int csrno) +static RISCVException pmp(CPURISCVState *env, int csrno, bool write_op) { if (riscv_feature(env, RISCV_FEATURE_PMP)) { return RISCV_EXCP_NONE; @@ -281,7 +282,7 @@ static RISCVException pmp(CPURISCVState *env, int csrno) return RISCV_EXCP_ILLEGAL_INST; } -static RISCVException epmp(CPURISCVState *env, int csrno) +static RISCVException epmp(CPURISCVState *env, int csrno, bool write_op) { if (env->priv == PRV_M && riscv_feature(env, RISCV_FEATURE_EPMP)) { return RISCV_EXCP_NONE; @@ -2873,7 +2874,8 @@ static RISCVException write_upmbase(CPURISCVState *env, int csrno, static inline RISCVException riscv_csrrw_check(CPURISCVState *env, int csrno, bool write_mask, - RISCVCPU *cpu) + RISCVCPU *cpu, + bool write_op) { /* check privileges and return RISCV_EXCP_ILLEGAL_INST if check fails */ int read_only = get_field(csrno, 0xC00) == 3; @@ -2895,7 +2897,7 @@ static inline RISCVException riscv_csrrw_check(CPURISCVState *env, return RISCV_EXCP_ILLEGAL_INST; } #endif - if (write_mask && read_only) { + if (write_op && read_only) { return RISCV_EXCP_ILLEGAL_INST; } @@ -2909,13 +2911,13 @@ static inline RISCVException riscv_csrrw_check(CPURISCVState *env, return RISCV_EXCP_ILLEGAL_INST; } - return csr_ops[csrno].predicate(env, csrno); + return csr_ops[csrno].predicate(env, csrno, write_op); } static RISCVException riscv_csrrw_do64(CPURISCVState *env, int csrno, target_ulong *ret_value, target_ulong new_value, - target_ulong write_mask) + target_ulong write_mask, bool write_op) { RISCVException ret; target_ulong old_value; @@ -2935,8 +2937,8 @@ static RISCVException riscv_csrrw_do64(CPURISCVState *env, int csrno, return ret; } - /* write value if writable and write mask set, otherwise drop writes */ - if (write_mask) { + /* write value if writable and write_op set, otherwise drop writes */ + if (write_op) { new_value = (old_value & ~write_mask) | (new_value & write_mask); if (csr_ops[csrno].write) { ret = csr_ops[csrno].write(env, csrno, new_value); @@ -2956,22 +2958,25 @@ static RISCVException riscv_csrrw_do64(CPURISCVState *env, int csrno, RISCVException riscv_csrrw(CPURISCVState *env, int csrno, target_ulong *ret_value, - target_ulong new_value, target_ulong write_mask) + target_ulong new_value, target_ulong write_mask, + bool write_op) { RISCVCPU *cpu = env_archcpu(env); - RISCVException ret = riscv_csrrw_check(env, csrno, write_mask, cpu); + RISCVException ret = riscv_csrrw_check(env, csrno, write_mask, cpu, + write_op); if (ret != RISCV_EXCP_NONE) { return ret; } - return riscv_csrrw_do64(env, csrno, ret_value, new_value, write_mask); + return riscv_csrrw_do64(env, csrno, ret_value, new_value, write_mask, + write_op); } static RISCVException riscv_csrrw_do128(CPURISCVState *env, int csrno, Int128 *ret_value, Int128 new_value, - Int128 write_mask) + Int128 write_mask, bool write_op) { RISCVException ret; Int128 old_value; @@ -2982,8 +2987,8 @@ static RISCVException riscv_csrrw_do128(CPURISCVState *env, int csrno, return ret; } - /* write value if writable and write mask set, otherwise drop writes */ - if (int128_nz(write_mask)) { + /* write value if writable and write_op set, otherwise drop writes */ + if (write_op) { new_value = int128_or(int128_and(old_value, int128_not(write_mask)), int128_and(new_value, write_mask)); if (csr_ops[csrno].write128) { @@ -3010,18 +3015,20 @@ static RISCVException riscv_csrrw_do128(CPURISCVState *env, int csrno, RISCVException riscv_csrrw_i128(CPURISCVState *env, int csrno, Int128 *ret_value, - Int128 new_value, Int128 write_mask) + Int128 new_value, Int128 write_mask, + bool write_op) { RISCVException ret; RISCVCPU *cpu = env_archcpu(env); - ret = riscv_csrrw_check(env, csrno, int128_nz(write_mask), cpu); + ret = riscv_csrrw_check(env, csrno, int128_nz(write_mask), cpu, write_op); if (ret != RISCV_EXCP_NONE) { return ret; } if (csr_ops[csrno].read128) { - return riscv_csrrw_do128(env, csrno, ret_value, new_value, write_mask); + return riscv_csrrw_do128(env, csrno, ret_value, new_value, write_mask, + write_op); } /* @@ -3033,7 +3040,7 @@ RISCVException riscv_csrrw_i128(CPURISCVState *env, int csrno, target_ulong old_value; ret = riscv_csrrw_do64(env, csrno, &old_value, int128_getlo(new_value), - int128_getlo(write_mask)); + int128_getlo(write_mask), write_op); if (ret == RISCV_EXCP_NONE && ret_value) { *ret_value = int128_make64(old_value); } @@ -3047,13 +3054,13 @@ RISCVException riscv_csrrw_i128(CPURISCVState *env, int csrno, RISCVException riscv_csrrw_debug(CPURISCVState *env, int csrno, target_ulong *ret_value, target_ulong new_value, - target_ulong write_mask) + target_ulong write_mask, bool write_op) { RISCVException ret; #if !defined(CONFIG_USER_ONLY) env->debugger = true; #endif - ret = riscv_csrrw(env, csrno, ret_value, new_value, write_mask); + ret = riscv_csrrw(env, csrno, ret_value, new_value, write_mask, write_op); #if !defined(CONFIG_USER_ONLY) env->debugger = false; #endif diff --git a/target/riscv/gdbstub.c b/target/riscv/gdbstub.c index 9ed049c29e..5e37ea12cb 100644 --- a/target/riscv/gdbstub.c +++ b/target/riscv/gdbstub.c @@ -124,7 +124,7 @@ static int riscv_gdb_get_fpu(CPURISCVState *env, GByteArray *buf, int n) * This also works for CSR_FRM and CSR_FCSR. */ result = riscv_csrrw_debug(env, n - 32, &val, - 0, 0); + 0, 0, false); if (result == RISCV_EXCP_NONE) { return gdb_get_regl(buf, val); } @@ -147,7 +147,7 @@ static int riscv_gdb_set_fpu(CPURISCVState *env, uint8_t *mem_buf, int n) * This also works for CSR_FRM and CSR_FCSR. */ result = riscv_csrrw_debug(env, n - 32, NULL, - val, -1); + val, -1, true); if (result == RISCV_EXCP_NONE) { return sizeof(target_ulong); } @@ -209,7 +209,7 @@ static int riscv_gdb_get_vector(CPURISCVState *env, GByteArray *buf, int n) } target_ulong val = 0; - int result = riscv_csrrw_debug(env, csrno, &val, 0, 0); + int result = riscv_csrrw_debug(env, csrno, &val, 0, 0, false); if (result == 0) { return gdb_get_regl(buf, val); @@ -236,7 +236,7 @@ static int riscv_gdb_set_vector(CPURISCVState *env, uint8_t *mem_buf, int n) } target_ulong val = ldtul_p(mem_buf); - int result = riscv_csrrw_debug(env, csrno, NULL, val, -1); + int result = riscv_csrrw_debug(env, csrno, NULL, val, -1, true); if (result == 0) { return sizeof(target_ulong); @@ -251,7 +251,7 @@ static int riscv_gdb_get_csr(CPURISCVState *env, GByteArray *buf, int n) target_ulong val = 0; int result; - result = riscv_csrrw_debug(env, n, &val, 0, 0); + result = riscv_csrrw_debug(env, n, &val, 0, 0, false); if (result == RISCV_EXCP_NONE) { return gdb_get_regl(buf, val); } @@ -265,7 +265,7 @@ static int riscv_gdb_set_csr(CPURISCVState *env, uint8_t *mem_buf, int n) target_ulong val = ldtul_p(mem_buf); int result; - result = riscv_csrrw_debug(env, n, NULL, val, -1); + result = riscv_csrrw_debug(env, n, NULL, val, -1, true); if (result == RISCV_EXCP_NONE) { return sizeof(target_ulong); } @@ -319,7 +319,8 @@ static int riscv_gen_dynamic_csr_xml(CPUState *cs, int base_reg) for (i = 0; i < CSR_TABLE_SIZE; i++) { predicate = csr_ops[i].predicate; - if (predicate && (predicate(env, i) == RISCV_EXCP_NONE)) { + if (predicate && ((predicate(env, i, false) == RISCV_EXCP_NONE) || + (predicate(env, i, true) == RISCV_EXCP_NONE))) { if (csr_ops[i].name) { g_string_append_printf(s, "<reg name=\"%s\"", csr_ops[i].name); } else { diff --git a/target/riscv/op_helper.c b/target/riscv/op_helper.c index 1a75ba11e6..e0d708091e 100644 --- a/target/riscv/op_helper.c +++ b/target/riscv/op_helper.c @@ -40,7 +40,7 @@ void helper_raise_exception(CPURISCVState *env, uint32_t exception) target_ulong helper_csrr(CPURISCVState *env, int csr) { target_ulong val = 0; - RISCVException ret = riscv_csrrw(env, csr, &val, 0, 0); + RISCVException ret = riscv_csrrw(env, csr, &val, 0, 0, false); if (ret != RISCV_EXCP_NONE) { riscv_raise_exception(env, ret, GETPC()); @@ -51,7 +51,7 @@ target_ulong helper_csrr(CPURISCVState *env, int csr) void helper_csrw(CPURISCVState *env, int csr, target_ulong src) { target_ulong mask = env->xl == MXL_RV32 ? UINT32_MAX : (target_ulong)-1; - RISCVException ret = riscv_csrrw(env, csr, NULL, src, mask); + RISCVException ret = riscv_csrrw(env, csr, NULL, src, mask, true); if (ret != RISCV_EXCP_NONE) { riscv_raise_exception(env, ret, GETPC()); @@ -62,7 +62,7 @@ target_ulong helper_csrrw(CPURISCVState *env, int csr, target_ulong src, target_ulong write_mask) { target_ulong val = 0; - RISCVException ret = riscv_csrrw(env, csr, &val, src, write_mask); + RISCVException ret = riscv_csrrw(env, csr, &val, src, write_mask, true); if (ret != RISCV_EXCP_NONE) { riscv_raise_exception(env, ret, GETPC()); @@ -75,7 +75,7 @@ target_ulong helper_csrr_i128(CPURISCVState *env, int csr) Int128 rv = int128_zero(); RISCVException ret = riscv_csrrw_i128(env, csr, &rv, int128_zero(), - int128_zero()); + int128_zero(), false); if (ret != RISCV_EXCP_NONE) { riscv_raise_exception(env, ret, GETPC()); @@ -90,7 +90,7 @@ void helper_csrw_i128(CPURISCVState *env, int csr, { RISCVException ret = riscv_csrrw_i128(env, csr, NULL, int128_make128(srcl, srch), - UINT128_MAX); + UINT128_MAX, true); if (ret != RISCV_EXCP_NONE) { riscv_raise_exception(env, ret, GETPC()); @@ -104,7 +104,7 @@ target_ulong helper_csrrw_i128(CPURISCVState *env, int csr, Int128 rv = int128_zero(); RISCVException ret = riscv_csrrw_i128(env, csr, &rv, int128_make128(srcl, srch), - int128_make128(maskl, maskh)); + int128_make128(maskl, maskh), true); if (ret != RISCV_EXCP_NONE) { riscv_raise_exception(env, ret, GETPC());