| Message ID | 20240701033722.954-4-zhiwei_liu@linux.alibaba.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | target/riscv: Expose RV32 cpu to RV64 QEMU | expand |
Hi Tiancheng, Zhiwei, On 1/7/24 05:37, LIU Zhiwei wrote: > From: TANG Tiancheng <tangtiancheng.ttc@alibaba-inc.com> > > Ensure that riscv_cpu_sxl returns MXL_RV32 when runningRV32 in an > RV64 QEMU. > > Signed-off-by: TANG Tiancheng <tangtiancheng.ttc@alibaba-inc.com> > Fixes: 05e6ca5e156 ("target/riscv: Ignore reserved bits in PTE for RV64") > Reviewed-by: Liu Zhiwei <zhiwei_liu@linux.alibaba.com> > --- > target/riscv/cpu.h | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h > index 6fe0d712b4..36a712044a 100644 > --- a/target/riscv/cpu.h > +++ b/target/riscv/cpu.h > @@ -668,8 +668,11 @@ static inline RISCVMXL riscv_cpu_sxl(CPURISCVState *env) > #ifdef CONFIG_USER_ONLY > return env->misa_mxl; > #else > - return get_field(env->mstatus, MSTATUS64_SXL); > + if (env->misa_mxl != MXL_RV32) { > + return get_field(env->mstatus, MSTATUS64_SXL); > + } > #endif > + return MXL_RV32; Can we simplify the previous TARGET_RISCV32 ifdef'ry? > } > #endif >
On 2024/7/1 23:10, Philippe Mathieu-Daudé wrote: > Hi Tiancheng, Zhiwei, > > On 1/7/24 05:37, LIU Zhiwei wrote: >> From: TANG Tiancheng <tangtiancheng.ttc@alibaba-inc.com> >> >> Ensure that riscv_cpu_sxl returns MXL_RV32 when runningRV32 in an >> RV64 QEMU. >> >> Signed-off-by: TANG Tiancheng <tangtiancheng.ttc@alibaba-inc.com> >> Fixes: 05e6ca5e156 ("target/riscv: Ignore reserved bits in PTE for >> RV64") >> Reviewed-by: Liu Zhiwei <zhiwei_liu@linux.alibaba.com> >> --- >> target/riscv/cpu.h | 5 ++++- >> 1 file changed, 4 insertions(+), 1 deletion(-) >> >> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h >> index 6fe0d712b4..36a712044a 100644 >> --- a/target/riscv/cpu.h >> +++ b/target/riscv/cpu.h >> @@ -668,8 +668,11 @@ static inline RISCVMXL >> riscv_cpu_sxl(CPURISCVState *env) >> #ifdef CONFIG_USER_ONLY >> return env->misa_mxl; >> #else >> - return get_field(env->mstatus, MSTATUS64_SXL); >> + if (env->misa_mxl != MXL_RV32) { >> + return get_field(env->mstatus, MSTATUS64_SXL); >> + } >> #endif >> + return MXL_RV32; > > Can we simplify the previous TARGET_RISCV32 ifdef'ry? I think you mean the TARGET_RISCV32 macro here: #ifdef TARGET_RISCV32 #define riscv_cpu_sxl(env) ((void)(env), MXL_RV32) #else static inline RISCVMXL riscv_cpu_sxl(CPURISCVState *env) { #ifdef CONFIG_USER_ONLY return env->misa_mxl; #else return get_field(env->mstatus, MSTATUS64_SXL); #endif } #endif I think it is better to keep it here for better performance(as a constant). If you mean whether we can simplify all the ifdef TARGET_RISCV32 in target/riscv or hw/riscv, IMHO, it depends. I git grep the TARGET_RISCV32 from target/riscv: target/riscv/cpu-param.h:#elif defined(TARGET_RISCV32) target/riscv/cpu.c:#ifdef TARGET_RISCV32 target/riscv/cpu.c:#if defined(TARGET_RISCV32) target/riscv/cpu.h:#if defined(TARGET_RISCV32) target/riscv/cpu.h:#ifdef TARGET_RISCV32 target/riscv/cpu.h:#if defined(TARGET_RISCV32) target/riscv/cpu.h:#if defined(TARGET_RISCV32) target/riscv/cpu.h:#ifdef TARGET_RISCV32 target/riscv/insn_trans/trans_rvzacas.c.inc:#ifdef TARGET_RISCV32 target/riscv/kvm/kvm-cpu.c:#if defined(TARGET_RISCV32) target/riscv/translate.c:#ifdef TARGET_RISCV32 target/riscv/translate.c:#ifdef TARGET_RISCV32 target/riscv/translate.c:#ifdef TARGET_RISCV32 target/riscv/translate.c:#ifdef TARGET_RISCV32 target/riscv/translate.c:#ifdef TARGET_RISCV32 target/riscv/translate.c:#ifdef TARGET_RISCV32 target/riscv/translate.c:#ifdef TARGET_RISCV32 One case we can remove the TARGET_RISCV32 is in translate.c. static void gen_set_fpr_hs(DisasContext *ctx, int reg_num, TCGv_i64 t) { if (!ctx->cfg_ptr->ext_zfinx) { tcg_gen_mov_i64(cpu_fpr[reg_num], t); return; } if (reg_num != 0) { switch (get_xl(ctx)) { case MXL_RV32: #ifdef TARGET_RISCV32 tcg_gen_extrl_i64_i32(cpu_gpr[reg_num], t); break; #else /* fall through */ case MXL_RV64: tcg_gen_mov_i64(cpu_gpr[reg_num], t); break; #endif default: g_assert_not_reached(); } } } We can simplify this code by tcg_gen_trunc_i64_tl. One case we can't remove the TARGET_RISCV32 is in cpu.c, where we define any or max cpu with the width depending on this macro. I don't analyze all TARGET_RISCV32 using here. We will upstream a standalone patch for validating all its using later. Thanks, Zhiwei > >> } >> #endif
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h index 6fe0d712b4..36a712044a 100644 --- a/target/riscv/cpu.h +++ b/target/riscv/cpu.h @@ -668,8 +668,11 @@ static inline RISCVMXL riscv_cpu_sxl(CPURISCVState *env) #ifdef CONFIG_USER_ONLY return env->misa_mxl; #else - return get_field(env->mstatus, MSTATUS64_SXL); + if (env->misa_mxl != MXL_RV32) { + return get_field(env->mstatus, MSTATUS64_SXL); + } #endif + return MXL_RV32; } #endif