|
| From: | weiwei |
| Subject: | Re: [PATCH v10 4/5] target/riscv: smstateen check for fcsr |
| Date: | Mon, 10 Oct 2022 22:41:25 +0800 |
| User-agent: | Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.2.2 |
On Tue, 2022-10-04 at 21:23 +0800, weiwei wrote:
On 2022/10/4 14:51, mchitale@ventanamicro.com wrote:
On Mon, 2022-10-03 at 21:02 +0800, weiwei wrote:On 2022/10/3 19:47, Mayuresh Chitale wrote:If smstateen is implemented and sstateen0.fcsr is clear then thefloating pointoperations must return illegal instruction exception or virtualinstructiontrap, if relevant.Signed-off-by: Mayuresh Chitale<mchitale@ventanamicro.com>---target/riscv/csr.c | 23 ++++++++++++target/riscv/insn_trans/trans_rvf.c.inc | 43+++++++++++++++++++++--target/riscv/insn_trans/trans_rvzfh.c.inc | 12 +++++++3 files changed, 75 insertions(+), 3 deletions(-)diff --git a/target/riscv/csr.c b/target/riscv/csr.cindex 71236f2b5d..8b25f885ec 100644--- a/target/riscv/csr.c+++ b/target/riscv/csr.c@@ -84,6 +84,10 @@ static RISCVException fs(CPURISCVState *env, intcsrno)!RISCV_CPU(env_cpu(env))->cfg.ext_zfinx) {return RISCV_EXCP_ILLEGAL_INST;}++ if (!env->debugger && !riscv_cpu_fp_enabled(env)) {+ return smstateen_acc_ok(env, 0, SMSTATEEN0_FCSR);+ }#endifreturn RISCV_EXCP_NONE;}@@ -2023,6 +2027,9 @@ static RISCVExceptionwrite_mstateen0(CPURISCVState *env, int csrno,target_ulong new_val){uint64_t wr_mask = SMSTATEEN_STATEEN | SMSTATEEN0_HSENVCFG;+ if (!riscv_has_ext(env, RVF)) {+ wr_mask |= SMSTATEEN0_FCSR;+ }return write_mstateen(env, csrno, wr_mask, new_val);}@@ -2059,6 +2066,10 @@ static RISCVExceptionwrite_mstateen0h(CPURISCVState *env, int csrno,{uint64_t wr_mask = SMSTATEEN_STATEEN | SMSTATEEN0_HSENVCFG;+ if (!riscv_has_ext(env, RVF)) {+ wr_mask |= SMSTATEEN0_FCSR;+ }+return write_mstateenh(env, csrno, wr_mask, new_val);}@@ -2096,6 +2107,10 @@ static RISCVExceptionwrite_hstateen0(CPURISCVState *env, int csrno,{uint64_t wr_mask = SMSTATEEN_STATEEN | SMSTATEEN0_HSENVCFG;+ if (!riscv_has_ext(env, RVF)) {+ wr_mask |= SMSTATEEN0_FCSR;+ }+return write_hstateen(env, csrno, wr_mask, new_val);}@@ -2135,6 +2150,10 @@ static RISCVExceptionwrite_hstateen0h(CPURISCVState *env, int csrno,{uint64_t wr_mask = SMSTATEEN_STATEEN | SMSTATEEN0_HSENVCFG;+ if (!riscv_has_ext(env, RVF)) {+ wr_mask |= SMSTATEEN0_FCSR;+ }+return write_hstateenh(env, csrno, wr_mask, new_val);}@@ -2182,6 +2201,10 @@ static RISCVExceptionwrite_sstateen0(CPURISCVState *env, int csrno,{uint64_t wr_mask = SMSTATEEN_STATEEN | SMSTATEEN0_HSENVCFG;+ if (!riscv_has_ext(env, RVF)) {+ wr_mask |= SMSTATEEN0_FCSR;+ }+return write_sstateen(env, csrno, wr_mask, new_val);}diff --git a/target/riscv/insn_trans/trans_rvf.c.incb/target/riscv/insn_trans/trans_rvf.c.incindex a1d3eb52ad..93657680c6 100644--- a/target/riscv/insn_trans/trans_rvf.c.inc+++ b/target/riscv/insn_trans/trans_rvf.c.inc@@ -24,9 +24,46 @@return false; \} while (0)-#define REQUIRE_ZFINX_OR_F(ctx) do {\- if (!ctx->cfg_ptr->ext_zfinx) { \- REQUIRE_EXT(ctx, RVF); \+#ifndef CONFIG_USER_ONLY+static inline bool smstateen_fcsr_check(DisasContext *ctx, intindex)+{+ CPUState *cpu = ctx->cs;+ CPURISCVState *env = cpu->env_ptr;+ uint64_t stateen = env->mstateen[index];++ if (!ctx->cfg_ptr->ext_smstateen || env->priv == PRV_M) {+ return true;+ }++ if (ctx->virt_enabled) {+ stateen &= env->hstateen[index];+ }++ if (env->priv == PRV_U && has_ext(ctx, RVS)) {+ stateen &= env->sstateen[index];+ }++ if (!(stateen & SMSTATEEN0_FCSR)) {+ if (ctx->virt_enabled) {+ ctx->virt_inst_excp = true;+ }Are there any considerations for adding virt_inst_excp instead ofdirectly"generate_exception(ctx, RISCV_EXCP_VIRT_INSTRUCTION_FAULT)" here?Regards,Weiwei LiI had considered it but I think this is a simpler approach as the restof the code path stays the same as generating an illegal instructionexception, for e.g setting the bins value (tval).OK, we did need to set bins value for virt instruction exception. However I prefer directly call a
new gen_exception_virt function(similar to gen_exception_illegal) here.
Also we need toreturn true from all the caller trans_* functions even if the smstateencheck has failed.False is returned when smstateen check fails currently.
Yes, however if we make this change then should return true if the check fails so that the decode_opc doesnt fall through and generate another exception. It can be done but it would be contrary to the general convention.
OK. Acceptable to me.
Reviewed-by: Weiwei Li <liweiwei@iscas.ac.cn>
Regards,
Weiwei Li
Regards,
Weiwei Li
+ return false;+ }++ return true;+}+#else+#define smstateen_fcsr_check(ctx, index) (true)+#endif++#define REQUIRE_ZFINX_OR_F(ctx) do { \+ if (!has_ext(ctx, RVF)) { \+ if (!ctx->cfg_ptr->ext_zfinx) { \+ return false; \+ } \+ if (!smstateen_fcsr_check(ctx, 0)) { \+ return false; \+ } \} \} while (0)diff --git a/target/riscv/insn_trans/trans_rvzfh.c.incb/target/riscv/insn_trans/trans_rvzfh.c.incindex 5d07150cd0..6c2e338c0a 100644--- a/target/riscv/insn_trans/trans_rvzfh.c.inc+++ b/target/riscv/insn_trans/trans_rvzfh.c.inc@@ -20,18 +20,27 @@if (!ctx->cfg_ptr->ext_zfh) { \return false; \} \+ if (!smstateen_fcsr_check(ctx, 0)) { \+ return false; \+ } \} while (0)#define REQUIRE_ZHINX_OR_ZFH(ctx) do { \if (!ctx->cfg_ptr->ext_zhinx && !ctx->cfg_ptr->ext_zfh) { \return false; \} \+ if (!smstateen_fcsr_check(ctx, 0)) { \+ return false; \+ } \} while (0)#define REQUIRE_ZFH_OR_ZFHMIN(ctx) do { \if (!(ctx->cfg_ptr->ext_zfh || ctx->cfg_ptr->ext_zfhmin)) { \return false; \} \+ if (!smstateen_fcsr_check(ctx, 0)) { \+ return false; \+ } \} while (0)#define REQUIRE_ZFH_OR_ZFHMIN_OR_ZHINX_OR_ZHINXMIN(ctx) do { \@@ -39,6 +48,9 @@ctx->cfg_ptr->ext_zhinx || ctx->cfg_ptr->ext_zhinxmin)){ \return false; \} \+ if (!smstateen_fcsr_check(ctx, 0)) { \+ return false; \+ } \} while (0)static bool trans_flh(DisasContext *ctx, arg_flh *a)
| [Prev in Thread] | Current Thread | [Next in Thread] |