Re: [PATCH v6 3/5] target/riscv: smstateen check for fcsr

[Ben Dooks]

On 28/07/2022 07:15, Mayuresh Chitale wrote:

On Mon, 2022-07-25 at 15:23 +0800, Weiwei Li wrote:

在 2022/7/24 下午11:49, Mayuresh Chitale 写道:

On Fri, 2022-07-22 at 09:42 +0800, Weiwei Li wrote:

在 2022/7/21 下午11:31, Mayuresh Chitale 写道:

If smstateen is implemented and sstateen0.fcsr is clear then
the
floating point operations must return illegal instruction
exception.

Signed-off-by: Mayuresh Chitale <mchit...@ventanamicro.com>
---
   target/riscv/csr.c                        | 23 ++++++++++++++
   target/riscv/insn_trans/trans_rvf.c.inc   | 38
+++++++++++++++++++++--
   target/riscv/insn_trans/trans_rvzfh.c.inc |  4 +++
   3 files changed, 63 insertions(+), 2 deletions(-)

diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index ab06b117f9..a597b6cbc7 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -96,6 +96,10 @@ static RISCVException fs(CPURISCVState *env,
int
csrno)
           !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);
+    }
   #endif
       return RISCV_EXCP_NONE;
   }
@@ -1876,6 +1880,9 @@ static RISCVException
write_mstateen0(CPURISCVState *env, int csrno,
                                         target_ulong new_val)
   {
       uint64_t wr_mask = SMSTATEEN_STATEN |
SMSTATEEN0_HSENVCFG;
+    if (!riscv_has_ext(env, RVF)) {
+        wr_mask |= SMSTATEEN0_FCSR;
+    }

return write_mstateen(env, csrno, wr_mask, new_val);

   }
@@ -1924,6 +1931,10 @@ static RISCVException
write_mstateen0h(CPURISCVState *env, int csrno,
   {
       uint64_t wr_mask = SMSTATEEN_STATEN |
SMSTATEEN0_HSENVCFG;

+ if (!riscv_has_ext(env, RVF)) {

+        wr_mask |= SMSTATEEN0_FCSR;
+    }
+
       return write_mstateenh(env, csrno, wr_mask, new_val);
   }

@@ -1973,6 +1984,10 @@ static RISCVException

write_hstateen0(CPURISCVState *env, int csrno,
   {
       uint64_t wr_mask = SMSTATEEN_STATEN |
SMSTATEEN0_HSENVCFG;

+ if (!riscv_has_ext(env, RVF)) {

+        wr_mask |= SMSTATEEN0_FCSR;
+    }
+
       return write_hstateen(env, csrno, wr_mask, new_val);
   }

@@ -2024,6 +2039,10 @@ static RISCVException

write_hstateen0h(CPURISCVState *env, int csrno,
   {
       uint64_t wr_mask = SMSTATEEN_STATEN |
SMSTATEEN0_HSENVCFG;

+ if (!riscv_has_ext(env, RVF)) {

+        wr_mask |= SMSTATEEN0_FCSR;
+    }
+
       return write_hstateenh(env, csrno, wr_mask, new_val);
   }

@@ -2083,6 +2102,10 @@ static RISCVException

write_sstateen0(CPURISCVState *env, int csrno,
   {
       uint64_t wr_mask = SMSTATEEN_STATEN |
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.inc

b/target/riscv/insn_trans/trans_rvf.c.inc
index a1d3eb52ad..c43c48336b 100644
--- a/target/riscv/insn_trans/trans_rvf.c.inc
+++ b/target/riscv/insn_trans/trans_rvf.c.inc
@@ -24,9 +24,43 @@
               return false; \
   } while (0)

+#ifndef CONFIG_USER_ONLY

+#define SMSTATEEN_CHECK(ctx) do {\
+    CPUState *cpu = ctx->cs; \
+    CPURISCVState *env = cpu->env_ptr; \
+    if (ctx->cfg_ptr->ext_smstateen && \
+        (env->priv < PRV_M)) { \
+        uint64_t stateen = env->mstateen[0]; \
+        uint64_t hstateen = env->hstateen[0]; \
+        uint64_t sstateen = env->sstateen[0]; \
+        if (!(stateen & SMSTATEEN_STATEN)) {\
+            hstateen = 0; \
+            sstateen = 0; \
+        } \
+        if (ctx->virt_enabled) { \
+            stateen &= hstateen; \
+            if (!(hstateen & SMSTATEEN_STATEN)) {\
+                sstateen = 0; \
+            } \
+        } \
+        if (env->priv == PRV_U && has_ext(ctx, RVS))
{\eventually
meaning
+            stateen &= sstateen; \
+        } \
+        if (!(stateen & SMSTATEEN0_FCSR)) { \
+            return false; \
+        } \
+    } \

given the size of that I would have thought an "static inline"
function would be easier to write and maintain for SMSTATEEN_CHECK


--
Ben Dooks                               http://www.codethink.co.uk/
Senior Engineer                         Codethink - Providing Genius

https://www.codethink.co.uk/privacy.html