On Thu, Mar 17, 2022 at 1:13 AM Weiwei Li <liwei...@iscas.ac.cn> wrote: > > > 在 2022/3/16 下午9:07, Bin Meng 写道: > > On Fri, Mar 11, 2022 at 5:46 PM Weiwei Li <liwei...@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 <liwei...@iscas.ac.cn> > >> Signed-off-by: Junqiang Wang <wangjunqi...@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 > > > > >