[PATCH v3 0/4] Smstateen FCSR

qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed

* [PATCH v3 0/4] Smstateen FCSR
@ 2023-04-28 16:52 Mayuresh Chitale
  2023-04-28 16:52 ` [PATCH v3 1/4] target/riscv: smstateen check for fcsr Mayuresh Chitale
                   ` (3 more replies)
  0 siblings, 4 replies; 9+ messages in thread
From: Mayuresh Chitale @ 2023-04-28 16:52 UTC (permalink / raw)
  To: qemu-devel, qemu-riscv, alistair.francis
  Cc: Mayuresh Chitale, Alistair Francis, Daniel Barboza, liweiwei,
	Richard Henderson

Patch 4 and 5 of the smstateen series need to be re-submitted with
changes described in the email below.
https://lists.nongnu.org/archive/html/qemu-riscv/2022-11/msg00155.html
Hence splitting the patch 4 of the original series into three and
re-submitting along with the original patch 5.

Changes in v3:
- Reuse TB_FLAGS.FS (instead of TB_FLAGS.HS_FS) for smstateen as HS_FS bits been removed.
- Remove fcsr check for zfh and zfhmin

Changes in v2:
 - Improve patch 1 description
 - Reuse TB_FLAGS.HS_FS for smstateen
 - Convert smstateen_fcsr_check to function
 - Add fcsr check for zdinx

Mayuresh Chitale (4):
  target/riscv: smstateen check for fcsr
  target/riscv: Reuse tb->flags.FS
  target/riscv: check smstateen fcsr flag
  target/riscv: smstateen knobs

 target/riscv/cpu.c                        |  3 ++-
 target/riscv/cpu_helper.c                 |  9 +++++++
 target/riscv/csr.c                        | 15 +++++++++++
 target/riscv/insn_trans/trans_rvd.c.inc   | 13 ++++++---
 target/riscv/insn_trans/trans_rvf.c.inc   | 24 ++++++++++++++---
 target/riscv/insn_trans/trans_rvzfh.c.inc | 32 +++++++++++++++--------
 target/riscv/translate.c                  | 12 ++++++++-
 7 files changed, 87 insertions(+), 21 deletions(-)

-- 
2.34.1



^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v3 1/4] target/riscv: smstateen check for fcsr
  2023-04-28 16:52 [PATCH v3 0/4] Smstateen FCSR Mayuresh Chitale
@ 2023-04-28 16:52 ` Mayuresh Chitale
  2023-04-28 16:52 ` [PATCH v3 2/4] target/riscv: Reuse tb->flags.FS Mayuresh Chitale
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 9+ messages in thread
From: Mayuresh Chitale @ 2023-04-28 16:52 UTC (permalink / raw)
  To: qemu-devel, qemu-riscv, alistair.francis
  Cc: Mayuresh Chitale, Alistair Francis, Daniel Barboza, liweiwei,
	Richard Henderson

If smstateen is implemented and smtateen0.fcsr is clear and misa.F
is off then the floating point operations must return illegal
instruction exception or virtual instruction trap, if relevant.

Signed-off-by: Mayuresh Chitale <mchitale@ventanamicro.com>
Reviewed-by: Weiwei Li <liweiwei@iscas.ac.cn>
---
 target/riscv/csr.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index 4451bd1263..3f6b824bd2 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -82,6 +82,10 @@ static RISCVException fs(CPURISCVState *env, int csrno)
         !riscv_cpu_cfg(env)->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;
 }
@@ -2100,6 +2104,9 @@ static RISCVException write_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);
 }
@@ -2173,6 +2180,10 @@ static RISCVException write_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);
 }
 
@@ -2259,6 +2270,10 @@ static RISCVException write_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);
 }
 
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v3 2/4] target/riscv: Reuse tb->flags.FS
  2023-04-28 16:52 [PATCH v3 0/4] Smstateen FCSR Mayuresh Chitale
  2023-04-28 16:52 ` [PATCH v3 1/4] target/riscv: smstateen check for fcsr Mayuresh Chitale
@ 2023-04-28 16:52 ` Mayuresh Chitale
  2023-04-29  1:17   ` Weiwei Li
  2023-04-29  8:54   ` Richard Henderson
  2023-04-28 16:52 ` [PATCH v3 3/4] target/riscv: check smstateen fcsr flag Mayuresh Chitale
  2023-04-28 16:52 ` [PATCH v3 4/4] target/riscv: smstateen knobs Mayuresh Chitale
  3 siblings, 2 replies; 9+ messages in thread
From: Mayuresh Chitale @ 2023-04-28 16:52 UTC (permalink / raw)
  To: qemu-devel, qemu-riscv, alistair.francis
  Cc: Mayuresh Chitale, Alistair Francis, Daniel Barboza, liweiwei,
	Richard Henderson

When misa.F is 0 tb->flags.FS field is unused and can be used to save
the current state of smstateen0.FCSR check which is needed by the
floating point translation routines.

Signed-off-by: Mayuresh Chitale <mchitale@ventanamicro.com>
---
 target/riscv/cpu_helper.c |  9 +++++++++
 target/riscv/translate.c  | 12 +++++++++++-
 2 files changed, 20 insertions(+), 1 deletion(-)

diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index b68dcfe7b6..126ac221a0 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -119,6 +119,15 @@ void cpu_get_tb_cpu_state(CPURISCVState *env, target_ulong *pc,
         vs = MIN(vs, get_field(env->mstatus_hs, MSTATUS_VS));
     }
 
+    /*
+     * If misa.F is 0 then the FS field of the tb->flags can be used to pass
+     * the current state of the smstateen.FCSR bit which must be checked for
+     * in the floating point translation routines.
+     */
+    if (!riscv_has_ext(env, RVF)) {
+        fs = (smstateen_acc_ok(env, 0, SMSTATEEN0_FCSR) == RISCV_EXCP_NONE);
+    }
+
     if (cpu->cfg.debug && !icount_enabled()) {
         flags = FIELD_DP32(flags, TB_FLAGS, ITRIGGER, env->itrigger_enabled);
     }
diff --git a/target/riscv/translate.c b/target/riscv/translate.c
index 928da0d3f0..74f624aa62 100644
--- a/target/riscv/translate.c
+++ b/target/riscv/translate.c
@@ -78,6 +78,7 @@ typedef struct DisasContext {
     int frm;
     RISCVMXL ol;
     bool virt_inst_excp;
+    bool smstateen_fcsr_ok;
     bool virt_enabled;
     const RISCVCPUConfig *cfg_ptr;
     /* vector extension */
@@ -1155,7 +1156,11 @@ static void riscv_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cs)
     ctx->pc_succ_insn = ctx->base.pc_first;
     ctx->priv = FIELD_EX32(tb_flags, TB_FLAGS, PRIV);
     ctx->mem_idx = FIELD_EX32(tb_flags, TB_FLAGS, MEM_IDX);
-    ctx->mstatus_fs = FIELD_EX32(tb_flags, TB_FLAGS, FS);
+    if (has_ext(ctx, RVF)) {
+        ctx->mstatus_fs = FIELD_EX32(tb_flags, TB_FLAGS, FS);
+    } else {
+        ctx->mstatus_fs = 0;
+    }
     ctx->mstatus_vs = FIELD_EX32(tb_flags, TB_FLAGS, VS);
     ctx->priv_ver = env->priv_ver;
     ctx->virt_enabled = FIELD_EX32(tb_flags, TB_FLAGS, VIRT_ENABLED);
@@ -1178,6 +1183,11 @@ static void riscv_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cs)
     ctx->itrigger = FIELD_EX32(tb_flags, TB_FLAGS, ITRIGGER);
     ctx->zero = tcg_constant_tl(0);
     ctx->virt_inst_excp = false;
+    if (has_ext(ctx, RVF) || !cpu->cfg.ext_smstateen) {
+        ctx->smstateen_fcsr_ok = 1;
+    } else {
+        ctx->smstateen_fcsr_ok = FIELD_EX32(tb_flags, TB_FLAGS, FS);
+    }
 }
 
 static void riscv_tr_tb_start(DisasContextBase *db, CPUState *cpu)
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH v3 2/4] target/riscv: Reuse tb->flags.FS
  2023-04-28 16:52 ` [PATCH v3 2/4] target/riscv: Reuse tb->flags.FS Mayuresh Chitale
@ 2023-04-29  1:17   ` Weiwei Li
  2023-04-29  8:54   ` Richard Henderson
  1 sibling, 0 replies; 9+ messages in thread
From: Weiwei Li @ 2023-04-29  1:17 UTC (permalink / raw)
  To: Mayuresh Chitale, qemu-devel, qemu-riscv, alistair.francis
  Cc: liweiwei, Alistair Francis, Daniel Barboza, Richard Henderson


On 2023/4/29 00:52, Mayuresh Chitale wrote:
> When misa.F is 0 tb->flags.FS field is unused and can be used to save
> the current state of smstateen0.FCSR check which is needed by the
> floating point translation routines.
>
> Signed-off-by: Mayuresh Chitale <mchitale@ventanamicro.com>
> ---

Reviewed-by: Weiwei Li <liweiwei@iscas.ac.cn>

Weiwei Li

>   target/riscv/cpu_helper.c |  9 +++++++++
>   target/riscv/translate.c  | 12 +++++++++++-
>   2 files changed, 20 insertions(+), 1 deletion(-)
>
> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> index b68dcfe7b6..126ac221a0 100644
> --- a/target/riscv/cpu_helper.c
> +++ b/target/riscv/cpu_helper.c
> @@ -119,6 +119,15 @@ void cpu_get_tb_cpu_state(CPURISCVState *env, target_ulong *pc,
>           vs = MIN(vs, get_field(env->mstatus_hs, MSTATUS_VS));
>       }
>   
> +    /*
> +     * If misa.F is 0 then the FS field of the tb->flags can be used to pass
> +     * the current state of the smstateen.FCSR bit which must be checked for
> +     * in the floating point translation routines.
> +     */
> +    if (!riscv_has_ext(env, RVF)) {
> +        fs = (smstateen_acc_ok(env, 0, SMSTATEEN0_FCSR) == RISCV_EXCP_NONE);
> +    }
> +
>       if (cpu->cfg.debug && !icount_enabled()) {
>           flags = FIELD_DP32(flags, TB_FLAGS, ITRIGGER, env->itrigger_enabled);
>       }
> diff --git a/target/riscv/translate.c b/target/riscv/translate.c
> index 928da0d3f0..74f624aa62 100644
> --- a/target/riscv/translate.c
> +++ b/target/riscv/translate.c
> @@ -78,6 +78,7 @@ typedef struct DisasContext {
>       int frm;
>       RISCVMXL ol;
>       bool virt_inst_excp;
> +    bool smstateen_fcsr_ok;
>       bool virt_enabled;
>       const RISCVCPUConfig *cfg_ptr;
>       /* vector extension */
> @@ -1155,7 +1156,11 @@ static void riscv_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cs)
>       ctx->pc_succ_insn = ctx->base.pc_first;
>       ctx->priv = FIELD_EX32(tb_flags, TB_FLAGS, PRIV);
>       ctx->mem_idx = FIELD_EX32(tb_flags, TB_FLAGS, MEM_IDX);
> -    ctx->mstatus_fs = FIELD_EX32(tb_flags, TB_FLAGS, FS);
> +    if (has_ext(ctx, RVF)) {
> +        ctx->mstatus_fs = FIELD_EX32(tb_flags, TB_FLAGS, FS);
> +    } else {
> +        ctx->mstatus_fs = 0;
> +    }
>       ctx->mstatus_vs = FIELD_EX32(tb_flags, TB_FLAGS, VS);
>       ctx->priv_ver = env->priv_ver;
>       ctx->virt_enabled = FIELD_EX32(tb_flags, TB_FLAGS, VIRT_ENABLED);
> @@ -1178,6 +1183,11 @@ static void riscv_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cs)
>       ctx->itrigger = FIELD_EX32(tb_flags, TB_FLAGS, ITRIGGER);
>       ctx->zero = tcg_constant_tl(0);
>       ctx->virt_inst_excp = false;
> +    if (has_ext(ctx, RVF) || !cpu->cfg.ext_smstateen) {
> +        ctx->smstateen_fcsr_ok = 1;
> +    } else {
> +        ctx->smstateen_fcsr_ok = FIELD_EX32(tb_flags, TB_FLAGS, FS);
> +    }
>   }
>   
>   static void riscv_tr_tb_start(DisasContextBase *db, CPUState *cpu)



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v3 2/4] target/riscv: Reuse tb->flags.FS
  2023-04-28 16:52 ` [PATCH v3 2/4] target/riscv: Reuse tb->flags.FS Mayuresh Chitale
  2023-04-29  1:17   ` Weiwei Li
@ 2023-04-29  8:54   ` Richard Henderson
  2023-04-29  9:20     ` Weiwei Li
  1 sibling, 1 reply; 9+ messages in thread
From: Richard Henderson @ 2023-04-29  8:54 UTC (permalink / raw)
  To: Mayuresh Chitale, qemu-devel, qemu-riscv, alistair.francis
  Cc: Alistair Francis, Daniel Barboza, liweiwei

On 4/28/23 17:52, Mayuresh Chitale wrote:
> When misa.F is 0 tb->flags.FS field is unused and can be used to save
> the current state of smstateen0.FCSR check which is needed by the
> floating point translation routines.
> 
> Signed-off-by: Mayuresh Chitale <mchitale@ventanamicro.com>
> ---
>   target/riscv/cpu_helper.c |  9 +++++++++
>   target/riscv/translate.c  | 12 +++++++++++-
>   2 files changed, 20 insertions(+), 1 deletion(-)
> 
> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> index b68dcfe7b6..126ac221a0 100644
> --- a/target/riscv/cpu_helper.c
> +++ b/target/riscv/cpu_helper.c
> @@ -119,6 +119,15 @@ void cpu_get_tb_cpu_state(CPURISCVState *env, target_ulong *pc,
>           vs = MIN(vs, get_field(env->mstatus_hs, MSTATUS_VS));
>       }
>   
> +    /*
> +     * If misa.F is 0 then the FS field of the tb->flags can be used to pass
> +     * the current state of the smstateen.FCSR bit which must be checked for
> +     * in the floating point translation routines.
> +     */
> +    if (!riscv_has_ext(env, RVF)) {
> +        fs = (smstateen_acc_ok(env, 0, SMSTATEEN0_FCSR) == RISCV_EXCP_NONE);
> +    }

You have misunderstood my suggestion:

     /* With Zfinx, floating point is enabled/disabled by Smstateen. */
     if (!riscv_has_ext(env, RVF)) {
         fs = (smstateen_acc_ok(env, 0, SMSTATEEN0_FCSR)
               ? EXT_STATUS_DIRTY : EXT_STATUS_DISABLED);
     }

> +    bool smstateen_fcsr_ok;

Not needed.

> -    ctx->mstatus_fs = FIELD_EX32(tb_flags, TB_FLAGS, FS);
> +    if (has_ext(ctx, RVF)) {
> +        ctx->mstatus_fs = FIELD_EX32(tb_flags, TB_FLAGS, FS);
> +    } else {
> +        ctx->mstatus_fs = 0;
> +    }

Not needed.

In patch 3, which should be merged with this, there are no changes to REQUIRE_ZFINX_OR_F, 
no additional smstateen_fcsr_check, and REQUIRE_FPU reduces to

#define REQUIRE_FPU do {                          \
     if (ctx->mstatus_fs == EXT_STATUS_DISABLED) { \
         return false;                             \
     }                                             \
} while (0)

This makes the DisasContext version of fs be the single gate for floating point.
No extra checks required.


r~


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v3 2/4] target/riscv: Reuse tb->flags.FS
  2023-04-29  8:54   ` Richard Henderson
@ 2023-04-29  9:20     ` Weiwei Li
  0 siblings, 0 replies; 9+ messages in thread
From: Weiwei Li @ 2023-04-29  9:20 UTC (permalink / raw)
  To: Richard Henderson, Mayuresh Chitale, qemu-devel, qemu-riscv,
	alistair.francis
  Cc: liweiwei, Alistair Francis, Daniel Barboza


On 2023/4/29 16:54, Richard Henderson wrote:
> On 4/28/23 17:52, Mayuresh Chitale wrote:
>> When misa.F is 0 tb->flags.FS field is unused and can be used to save
>> the current state of smstateen0.FCSR check which is needed by the
>> floating point translation routines.
>>
>> Signed-off-by: Mayuresh Chitale <mchitale@ventanamicro.com>
>> ---
>>   target/riscv/cpu_helper.c |  9 +++++++++
>>   target/riscv/translate.c  | 12 +++++++++++-
>>   2 files changed, 20 insertions(+), 1 deletion(-)
>>
>> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
>> index b68dcfe7b6..126ac221a0 100644
>> --- a/target/riscv/cpu_helper.c
>> +++ b/target/riscv/cpu_helper.c
>> @@ -119,6 +119,15 @@ void cpu_get_tb_cpu_state(CPURISCVState *env, 
>> target_ulong *pc,
>>           vs = MIN(vs, get_field(env->mstatus_hs, MSTATUS_VS));
>>       }
>>   +    /*
>> +     * If misa.F is 0 then the FS field of the tb->flags can be used 
>> to pass
>> +     * the current state of the smstateen.FCSR bit which must be 
>> checked for
>> +     * in the floating point translation routines.
>> +     */
>> +    if (!riscv_has_ext(env, RVF)) {
>> +        fs = (smstateen_acc_ok(env, 0, SMSTATEEN0_FCSR) == 
>> RISCV_EXCP_NONE);
>> +    }
>
> You have misunderstood my suggestion:
>
>     /* With Zfinx, floating point is enabled/disabled by Smstateen. */
>     if (!riscv_has_ext(env, RVF)) {
>         fs = (smstateen_acc_ok(env, 0, SMSTATEEN0_FCSR)
>               ? EXT_STATUS_DIRTY : EXT_STATUS_DISABLED);
>     }
>
>> +    bool smstateen_fcsr_ok;
>
> Not needed.
>
>> -    ctx->mstatus_fs = FIELD_EX32(tb_flags, TB_FLAGS, FS);
>> +    if (has_ext(ctx, RVF)) {
>> +        ctx->mstatus_fs = FIELD_EX32(tb_flags, TB_FLAGS, FS);
>> +    } else {
>> +        ctx->mstatus_fs = 0;
>> +    }
>
> Not needed.
>
> In patch 3, which should be merged with this, there are no changes to 
> REQUIRE_ZFINX_OR_F, no additional smstateen_fcsr_check, and 
> REQUIRE_FPU reduces to
>
> #define REQUIRE_FPU do {                          \
>     if (ctx->mstatus_fs == EXT_STATUS_DISABLED) { \
>         return false;                             \
>     }                                             \
> } while (0)
>
> This makes the DisasContext version of fs be the single gate for 
> floating point.
> No extra checks required.

Yeah. It's better to merge with REQUIRE_FPU.

However,  virtual instruction exception will be triggered in VS/VU mode 
if smstateen check fails, which is different from normal REQUIRE_FPU.

So, we may need to modify REQUIRE_FPU to distinguish them:

#define REQUIRE_FPU do {                          \
     if (ctx->mstatus_fs == EXT_STATUS_DISABLED) { \
           ctx->virt_inst_excp = ctx->virt_enabled && 
ctx->cfg_ptr->ext_zfinx;  \
           return false;                             \
     }                                             \
} while (0)

Regards,

Weiwei Li

>
>
> r~



^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v3 3/4] target/riscv: check smstateen fcsr flag
  2023-04-28 16:52 [PATCH v3 0/4] Smstateen FCSR Mayuresh Chitale
  2023-04-28 16:52 ` [PATCH v3 1/4] target/riscv: smstateen check for fcsr Mayuresh Chitale
  2023-04-28 16:52 ` [PATCH v3 2/4] target/riscv: Reuse tb->flags.FS Mayuresh Chitale
@ 2023-04-28 16:52 ` Mayuresh Chitale
  2023-04-29  1:20   ` Weiwei Li
  2023-04-28 16:52 ` [PATCH v3 4/4] target/riscv: smstateen knobs Mayuresh Chitale
  3 siblings, 1 reply; 9+ messages in thread
From: Mayuresh Chitale @ 2023-04-28 16:52 UTC (permalink / raw)
  To: qemu-devel, qemu-riscv, alistair.francis
  Cc: Mayuresh Chitale, Alistair Francis, Daniel Barboza, liweiwei,
	Richard Henderson

If misa.F and smstateen_fcsr_ok flag are clear then all the floating
point instructions must generate an appropriate exception.

Signed-off-by: Mayuresh Chitale <mchitale@ventanamicro.com>
---
 target/riscv/insn_trans/trans_rvd.c.inc   | 13 ++++++---
 target/riscv/insn_trans/trans_rvf.c.inc   | 24 ++++++++++++++---
 target/riscv/insn_trans/trans_rvzfh.c.inc | 32 +++++++++++++++--------
 3 files changed, 50 insertions(+), 19 deletions(-)

diff --git a/target/riscv/insn_trans/trans_rvd.c.inc b/target/riscv/insn_trans/trans_rvd.c.inc
index 2c51e01c40..d9e0cf116f 100644
--- a/target/riscv/insn_trans/trans_rvd.c.inc
+++ b/target/riscv/insn_trans/trans_rvd.c.inc
@@ -18,10 +18,15 @@
  * this program.  If not, see <http://www.gnu.org/licenses/>.
  */
 
-#define REQUIRE_ZDINX_OR_D(ctx) do { \
-    if (!ctx->cfg_ptr->ext_zdinx) { \
-        REQUIRE_EXT(ctx, RVD); \
-    } \
+#define REQUIRE_ZDINX_OR_D(ctx) do {           \
+    if (!has_ext(ctx, RVD)) {                  \
+        if (!ctx->cfg_ptr->ext_zdinx) {        \
+            return false;                      \
+        }                                      \
+        if (!smstateen_fcsr_check(ctx)) {      \
+            return false;                      \
+        }                                      \
+    }                                          \
 } while (0)
 
 #define REQUIRE_EVEN(ctx, reg) do { \
diff --git a/target/riscv/insn_trans/trans_rvf.c.inc b/target/riscv/insn_trans/trans_rvf.c.inc
index b2de4fcf3f..e4d9834237 100644
--- a/target/riscv/insn_trans/trans_rvf.c.inc
+++ b/target/riscv/insn_trans/trans_rvf.c.inc
@@ -24,10 +24,26 @@
             return false; \
 } while (0)
 
-#define REQUIRE_ZFINX_OR_F(ctx) do {\
-    if (!ctx->cfg_ptr->ext_zfinx) { \
-        REQUIRE_EXT(ctx, RVF); \
-    } \
+static inline bool smstateen_fcsr_check(DisasContext *ctx)
+{
+#ifndef CONFIG_USER_ONLY
+    if (!ctx->smstateen_fcsr_ok) {
+        ctx->virt_inst_excp = ctx->virt_enabled;
+        return false;
+    }
+#endif
+    return true;
+}
+
+#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)) {      \
+            return false;                      \
+        }                                      \
+    }                                          \
 } while (0)
 
 #define REQUIRE_ZCF(ctx) do {                  \
diff --git a/target/riscv/insn_trans/trans_rvzfh.c.inc b/target/riscv/insn_trans/trans_rvzfh.c.inc
index 74dde37ff7..e228ae28a5 100644
--- a/target/riscv/insn_trans/trans_rvzfh.c.inc
+++ b/target/riscv/insn_trans/trans_rvzfh.c.inc
@@ -16,28 +16,38 @@
  * this program.  If not, see <http://www.gnu.org/licenses/>.
  */
 
-#define REQUIRE_ZFH(ctx) do { \
+#define REQUIRE_ZFH(ctx) do {          \
     if (!ctx->cfg_ptr->ext_zfh) {      \
-        return false;         \
-    }                         \
-} while (0)
-
-#define REQUIRE_ZHINX_OR_ZFH(ctx) do { \
-    if (!ctx->cfg_ptr->ext_zhinx && !ctx->cfg_ptr->ext_zfh) { \
         return false;                  \
     }                                  \
 } while (0)
 
+#define REQUIRE_ZHINX_OR_ZFH(ctx) do {                \
+    if (!ctx->cfg_ptr->ext_zfh) {                     \
+        if (!ctx->cfg_ptr->ext_zhinx) {               \
+            return false;                             \
+        }                                             \
+        if (!smstateen_fcsr_check(ctx)) {             \
+            return false;                             \
+        }                                             \
+    }                                                 \
+} while (0)
+
 #define REQUIRE_ZFHMIN(ctx) do {              \
     if (!ctx->cfg_ptr->ext_zfhmin) {          \
         return false;                         \
     }                                         \
 } while (0)
 
-#define REQUIRE_ZFHMIN_OR_ZHINXMIN(ctx) do {                 \
-    if (!(ctx->cfg_ptr->ext_zfhmin || ctx->cfg_ptr->ext_zhinxmin)) { \
-        return false;                                        \
-    }                                                        \
+#define REQUIRE_ZFHMIN_OR_ZHINXMIN(ctx) do {             \
+    if (!ctx->cfg_ptr->ext_zfhmin) {                     \
+        if (ctx->cfg_ptr->ext_zhinxmin) {                \
+            return false;                                \
+        }                                                \
+        if (!smstateen_fcsr_check(ctx)) {                \
+            return false;                                \
+        }                                                \
+    }                                                    \
 } while (0)
 
 static bool trans_flh(DisasContext *ctx, arg_flh *a)
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH v3 3/4] target/riscv: check smstateen fcsr flag
  2023-04-28 16:52 ` [PATCH v3 3/4] target/riscv: check smstateen fcsr flag Mayuresh Chitale
@ 2023-04-29  1:20   ` Weiwei Li
  0 siblings, 0 replies; 9+ messages in thread
From: Weiwei Li @ 2023-04-29  1:20 UTC (permalink / raw)
  To: Mayuresh Chitale, qemu-devel, qemu-riscv, alistair.francis
  Cc: liweiwei, Alistair Francis, Daniel Barboza, Richard Henderson


On 2023/4/29 00:52, Mayuresh Chitale wrote:
> If misa.F and smstateen_fcsr_ok flag are clear then all the floating
> point instructions must generate an appropriate exception.
>
> Signed-off-by: Mayuresh Chitale <mchitale@ventanamicro.com>
> ---
>   target/riscv/insn_trans/trans_rvd.c.inc   | 13 ++++++---
>   target/riscv/insn_trans/trans_rvf.c.inc   | 24 ++++++++++++++---
>   target/riscv/insn_trans/trans_rvzfh.c.inc | 32 +++++++++++++++--------
>   3 files changed, 50 insertions(+), 19 deletions(-)
>
> diff --git a/target/riscv/insn_trans/trans_rvd.c.inc b/target/riscv/insn_trans/trans_rvd.c.inc
> index 2c51e01c40..d9e0cf116f 100644
> --- a/target/riscv/insn_trans/trans_rvd.c.inc
> +++ b/target/riscv/insn_trans/trans_rvd.c.inc
> @@ -18,10 +18,15 @@
>    * this program.  If not, see <http://www.gnu.org/licenses/>.
>    */
>   
> -#define REQUIRE_ZDINX_OR_D(ctx) do { \
> -    if (!ctx->cfg_ptr->ext_zdinx) { \
> -        REQUIRE_EXT(ctx, RVD); \
> -    } \
> +#define REQUIRE_ZDINX_OR_D(ctx) do {           \
> +    if (!has_ext(ctx, RVD)) {                  \
> +        if (!ctx->cfg_ptr->ext_zdinx) {        \
> +            return false;                      \
> +        }                                      \
> +        if (!smstateen_fcsr_check(ctx)) {      \
> +            return false;                      \
> +        }                                      \
> +    }                                          \
>   } while (0)
>   
>   #define REQUIRE_EVEN(ctx, reg) do { \
> diff --git a/target/riscv/insn_trans/trans_rvf.c.inc b/target/riscv/insn_trans/trans_rvf.c.inc
> index b2de4fcf3f..e4d9834237 100644
> --- a/target/riscv/insn_trans/trans_rvf.c.inc
> +++ b/target/riscv/insn_trans/trans_rvf.c.inc
> @@ -24,10 +24,26 @@
>               return false; \
>   } while (0)
>   
> -#define REQUIRE_ZFINX_OR_F(ctx) do {\
> -    if (!ctx->cfg_ptr->ext_zfinx) { \
> -        REQUIRE_EXT(ctx, RVF); \
> -    } \
> +static inline bool smstateen_fcsr_check(DisasContext *ctx)
> +{
> +#ifndef CONFIG_USER_ONLY
> +    if (!ctx->smstateen_fcsr_ok) {
> +        ctx->virt_inst_excp = ctx->virt_enabled;
> +        return false;
> +    }
> +#endif
> +    return true;
> +}
> +
> +#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)) {      \
> +            return false;                      \
> +        }                                      \
> +    }                                          \
>   } while (0)
>   
>   #define REQUIRE_ZCF(ctx) do {                  \
> diff --git a/target/riscv/insn_trans/trans_rvzfh.c.inc b/target/riscv/insn_trans/trans_rvzfh.c.inc
> index 74dde37ff7..e228ae28a5 100644
> --- a/target/riscv/insn_trans/trans_rvzfh.c.inc
> +++ b/target/riscv/insn_trans/trans_rvzfh.c.inc
> @@ -16,28 +16,38 @@
>    * this program.  If not, see <http://www.gnu.org/licenses/>.
>    */
>   
> -#define REQUIRE_ZFH(ctx) do { \
> +#define REQUIRE_ZFH(ctx) do {          \

This modification seems unnecessary.

Otherwise, Reviewed-by: Weiwei Li <liweiwei@iscas.ac.cn>

Weiwei Li

>       if (!ctx->cfg_ptr->ext_zfh) {      \
> -        return false;         \
> -    }                         \
> -} while (0)
> -
> -#define REQUIRE_ZHINX_OR_ZFH(ctx) do { \
> -    if (!ctx->cfg_ptr->ext_zhinx && !ctx->cfg_ptr->ext_zfh) { \
>           return false;                  \
>       }                                  \
>   } while (0)
>   
> +#define REQUIRE_ZHINX_OR_ZFH(ctx) do {                \
> +    if (!ctx->cfg_ptr->ext_zfh) {                     \
> +        if (!ctx->cfg_ptr->ext_zhinx) {               \
> +            return false;                             \
> +        }                                             \
> +        if (!smstateen_fcsr_check(ctx)) {             \
> +            return false;                             \
> +        }                                             \
> +    }                                                 \
> +} while (0)
> +
>   #define REQUIRE_ZFHMIN(ctx) do {              \
>       if (!ctx->cfg_ptr->ext_zfhmin) {          \
>           return false;                         \
>       }                                         \
>   } while (0)
>   
> -#define REQUIRE_ZFHMIN_OR_ZHINXMIN(ctx) do {                 \
> -    if (!(ctx->cfg_ptr->ext_zfhmin || ctx->cfg_ptr->ext_zhinxmin)) { \
> -        return false;                                        \
> -    }                                                        \
> +#define REQUIRE_ZFHMIN_OR_ZHINXMIN(ctx) do {             \
> +    if (!ctx->cfg_ptr->ext_zfhmin) {                     \
> +        if (ctx->cfg_ptr->ext_zhinxmin) {                \
> +            return false;                                \
> +        }                                                \
> +        if (!smstateen_fcsr_check(ctx)) {                \
> +            return false;                                \
> +        }                                                \
> +    }                                                    \
>   } while (0)
>   
>   static bool trans_flh(DisasContext *ctx, arg_flh *a)



^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v3 4/4] target/riscv: smstateen knobs
  2023-04-28 16:52 [PATCH v3 0/4] Smstateen FCSR Mayuresh Chitale
                   ` (2 preceding siblings ...)
  2023-04-28 16:52 ` [PATCH v3 3/4] target/riscv: check smstateen fcsr flag Mayuresh Chitale
@ 2023-04-28 16:52 ` Mayuresh Chitale
  3 siblings, 0 replies; 9+ messages in thread
From: Mayuresh Chitale @ 2023-04-28 16:52 UTC (permalink / raw)
  To: qemu-devel, qemu-riscv, alistair.francis
  Cc: Mayuresh Chitale, Alistair Francis, Daniel Barboza, liweiwei,
	Richard Henderson

Add knobs to allow users to enable smstateen and also export it via the
ISA extension string.

Signed-off-by: Mayuresh Chitale <mchitale@ventanamicro.com>
Reviewed-by: Weiwei Li<liweiwei@iscas.ac.cn>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
---
 target/riscv/cpu.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index befa64528f..9420cd670e 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -119,6 +119,7 @@ static const struct isa_ext_data isa_edata_arr[] = {
     ISA_EXT_DATA_ENTRY(zhinx, PRIV_VERSION_1_12_0, ext_zhinx),
     ISA_EXT_DATA_ENTRY(zhinxmin, PRIV_VERSION_1_12_0, ext_zhinxmin),
     ISA_EXT_DATA_ENTRY(smaia, PRIV_VERSION_1_12_0, ext_smaia),
+    ISA_EXT_DATA_ENTRY(smstateen, PRIV_VERSION_1_12_0, ext_smstateen),
     ISA_EXT_DATA_ENTRY(ssaia, PRIV_VERSION_1_12_0, ext_ssaia),
     ISA_EXT_DATA_ENTRY(sscofpmf, PRIV_VERSION_1_12_0, ext_sscofpmf),
     ISA_EXT_DATA_ENTRY(sstc, PRIV_VERSION_1_12_0, ext_sstc),
@@ -1498,8 +1499,8 @@ static Property riscv_cpu_extensions[] = {
     DEFINE_PROP_UINT16("vlen", RISCVCPU, cfg.vlen, 128),
     DEFINE_PROP_UINT16("elen", RISCVCPU, cfg.elen, 64),
 
+    DEFINE_PROP_BOOL("smstateen", RISCVCPU, cfg.ext_smstateen, false),
     DEFINE_PROP_BOOL("svadu", RISCVCPU, cfg.ext_svadu, true),
-
     DEFINE_PROP_BOOL("svinval", RISCVCPU, cfg.ext_svinval, false),
     DEFINE_PROP_BOOL("svnapot", RISCVCPU, cfg.ext_svnapot, false),
     DEFINE_PROP_BOOL("svpbmt", RISCVCPU, cfg.ext_svpbmt, false),
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2023-04-29  9:21 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-04-28 16:52 [PATCH v3 0/4] Smstateen FCSR Mayuresh Chitale
2023-04-28 16:52 ` [PATCH v3 1/4] target/riscv: smstateen check for fcsr Mayuresh Chitale
2023-04-28 16:52 ` [PATCH v3 2/4] target/riscv: Reuse tb->flags.FS Mayuresh Chitale
2023-04-29  1:17   ` Weiwei Li
2023-04-29  8:54   ` Richard Henderson
2023-04-29  9:20     ` Weiwei Li
2023-04-28 16:52 ` [PATCH v3 3/4] target/riscv: check smstateen fcsr flag Mayuresh Chitale
2023-04-29  1:20   ` Weiwei Li
2023-04-28 16:52 ` [PATCH v3 4/4] target/riscv: smstateen knobs Mayuresh Chitale

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).