[RFC PATCH v2 0/4] Smstateen FCSR

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

* [RFC PATCH v2 0/4] Smstateen FCSR
@ 2023-04-14 16:01 Mayuresh Chitale
  2023-04-14 16:01 ` [RFC PATCH v2 1/4] target/riscv: smstateen check for fcsr Mayuresh Chitale
                   ` (3 more replies)
  0 siblings, 4 replies; 13+ messages in thread
From: Mayuresh Chitale @ 2023-04-14 16:01 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 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.MSTATUS_HFS_FS
  target/riscv: check smstateen fcsr flag
  target/riscv: smstateen knobs

 target/riscv/cpu.c                        |  3 ++-
 target/riscv/cpu_helper.c                 | 12 ++++++++++++
 target/riscv/csr.c                        | 23 ++++++++++++++++++++++
 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 | 18 ++++++++++++++---
 target/riscv/translate.c                  |  7 +++++++
 7 files changed, 88 insertions(+), 12 deletions(-)

-- 
2.34.1



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

* [RFC PATCH v2 1/4] target/riscv: smstateen check for fcsr
  2023-04-14 16:01 [RFC PATCH v2 0/4] Smstateen FCSR Mayuresh Chitale
@ 2023-04-14 16:01 ` Mayuresh Chitale
  2023-04-15  1:01   ` Weiwei Li
  2023-04-14 16:02 ` [RFC PATCH v2 2/4] target/riscv: Reuse TB_FLAGS.MSTATUS_HFS_FS Mayuresh Chitale
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 13+ messages in thread
From: Mayuresh Chitale @ 2023-04-14 16:01 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>
---
 target/riscv/csr.c | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index f4d2dcfdc8..8ae9e95f9f 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;
 }
@@ -2081,6 +2085,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);
 }
@@ -2117,6 +2124,10 @@ static RISCVException write_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);
 }
 
@@ -2154,6 +2165,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);
 }
 
@@ -2193,6 +2208,10 @@ static RISCVException write_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);
 }
 
@@ -2240,6 +2259,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] 13+ messages in thread

* Re: [RFC PATCH v2 1/4] target/riscv: smstateen check for fcsr
  2023-04-14 16:01 ` [RFC PATCH v2 1/4] target/riscv: smstateen check for fcsr Mayuresh Chitale
@ 2023-04-15  1:01   ` Weiwei Li
  2023-04-26 17:12     ` Mayuresh Chitale
  0 siblings, 1 reply; 13+ messages in thread
From: Weiwei Li @ 2023-04-15  1:01 UTC (permalink / raw)
  To: Mayuresh Chitale, qemu-devel, qemu-riscv, alistair.francis
  Cc: Alistair Francis, Daniel Barboza, liweiwei, Richard Henderson


On 2023/4/15 00:01, Mayuresh Chitale wrote:
> 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>
> ---
>   target/riscv/csr.c | 23 +++++++++++++++++++++++
>   1 file changed, 23 insertions(+)
>
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index f4d2dcfdc8..8ae9e95f9f 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;
>   }
> @@ -2081,6 +2085,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);
>   }
> @@ -2117,6 +2124,10 @@ static RISCVException write_mstateen0h(CPURISCVState *env, int csrno,
>   {
>       uint64_t wr_mask = SMSTATEEN_STATEEN | SMSTATEEN0_HSENVCFG;
>   
> +    if (!riscv_has_ext(env, RVF)) {
> +        wr_mask |= SMSTATEEN0_FCSR;
> +    }
> +

FCSR is bit 1 of mstateen0.  So it seems unnecessary to be added to 
wr_mask for mstateen0h.

Similar to hstateen0h.

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

Weiwei Li
>       return write_mstateenh(env, csrno, wr_mask, new_val);
>   }
>   
> @@ -2154,6 +2165,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);
>   }
>   
> @@ -2193,6 +2208,10 @@ static RISCVException write_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);
>   }
>   
> @@ -2240,6 +2259,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);
>   }
>   



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

* Re: [RFC PATCH v2 1/4] target/riscv: smstateen check for fcsr
  2023-04-15  1:01   ` Weiwei Li
@ 2023-04-26 17:12     ` Mayuresh Chitale
  0 siblings, 0 replies; 13+ messages in thread
From: Mayuresh Chitale @ 2023-04-26 17:12 UTC (permalink / raw)
  To: Weiwei Li
  Cc: qemu-devel, qemu-riscv, alistair.francis, Alistair Francis,
	Daniel Barboza, Richard Henderson

On Sat, Apr 15, 2023 at 6:32 AM Weiwei Li <liweiwei@iscas.ac.cn> wrote:
>
>
> On 2023/4/15 00:01, Mayuresh Chitale wrote:
> > 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>
> > ---
> >   target/riscv/csr.c | 23 +++++++++++++++++++++++
> >   1 file changed, 23 insertions(+)
> >
> > diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> > index f4d2dcfdc8..8ae9e95f9f 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;
> >   }
> > @@ -2081,6 +2085,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);
> >   }
> > @@ -2117,6 +2124,10 @@ static RISCVException write_mstateen0h(CPURISCVState *env, int csrno,
> >   {
> >       uint64_t wr_mask = SMSTATEEN_STATEEN | SMSTATEEN0_HSENVCFG;
> >
> > +    if (!riscv_has_ext(env, RVF)) {
> > +        wr_mask |= SMSTATEEN0_FCSR;
> > +    }
> > +
>
> FCSR is bit 1 of mstateen0.  So it seems unnecessary to be added to
> wr_mask for mstateen0h.
>
> Similar to hstateen0h.
>
> Otherwise,  Reviewed-by: Weiwei Li <liweiwei@iscas.ac.cn>
>
Thanks, I will make the change above in the next version.
> Weiwei Li
> >       return write_mstateenh(env, csrno, wr_mask, new_val);
> >   }
> >
> > @@ -2154,6 +2165,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);
> >   }
> >
> > @@ -2193,6 +2208,10 @@ static RISCVException write_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);
> >   }
> >
> > @@ -2240,6 +2259,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);
> >   }
> >
>


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

* [RFC PATCH v2 2/4] target/riscv: Reuse TB_FLAGS.MSTATUS_HFS_FS
  2023-04-14 16:01 [RFC PATCH v2 0/4] Smstateen FCSR Mayuresh Chitale
  2023-04-14 16:01 ` [RFC PATCH v2 1/4] target/riscv: smstateen check for fcsr Mayuresh Chitale
@ 2023-04-14 16:02 ` Mayuresh Chitale
  2023-04-15  1:15   ` Weiwei Li
  2023-04-15  1:45   ` Weiwei Li
  2023-04-14 16:02 ` [RFC PATCH v2 3/4] target/riscv: check smstateen fcsr flag Mayuresh Chitale
  2023-04-14 16:02 ` [RFC PATCH v2 4/4] target/riscv: smstateen knobs Mayuresh Chitale
  3 siblings, 2 replies; 13+ messages in thread
From: Mayuresh Chitale @ 2023-04-14 16:02 UTC (permalink / raw)
  To: qemu-devel, qemu-riscv, alistair.francis
  Cc: Mayuresh Chitale, Alistair Francis, Daniel Barboza, liweiwei,
	Richard Henderson

When misa.F is clear, TB_FLAGS.MSTATUS_HS_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 | 12 ++++++++++++
 target/riscv/translate.c  |  7 +++++++
 2 files changed, 19 insertions(+)

diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index 433ea529b0..fd1731cc39 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -105,6 +105,18 @@ void cpu_get_tb_cpu_state(CPURISCVState *env, target_ulong *pc,
         flags = FIELD_DP32(flags, TB_FLAGS, MSTATUS_HS_VS,
                            get_field(env->mstatus_hs, MSTATUS_VS));
     }
+    /*
+     * If misa.F is 0 then the MSTATUS_HS_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)) {
+        if (smstateen_acc_ok(env, 0, SMSTATEEN0_FCSR) == RISCV_EXCP_NONE) {
+            flags = FIELD_DP32(flags, TB_FLAGS, MSTATUS_HS_FS, 1);
+        } else {
+            flags = FIELD_DP32(flags, TB_FLAGS, MSTATUS_HS_FS, 0);
+        }
+    }
     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 d0094922b6..e29bbb8b70 100644
--- a/target/riscv/translate.c
+++ b/target/riscv/translate.c
@@ -79,6 +79,7 @@ typedef struct DisasContext {
     int frm;
     RISCVMXL ol;
     bool virt_inst_excp;
+    bool smstateen_fcsr_ok;
     bool virt_enabled;
     const RISCVCPUConfig *cfg_ptr;
     bool hlsx;
@@ -1202,6 +1203,12 @@ 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)) {
+        ctx->smstateen_fcsr_ok = 1;
+    } else {
+        ctx->smstateen_fcsr_ok = FIELD_EX32(tb_flags, TB_FLAGS,
+                                             MSTATUS_HS_FS);
+    }
 }
 
 static void riscv_tr_tb_start(DisasContextBase *db, CPUState *cpu)
-- 
2.34.1



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

* Re: [RFC PATCH v2 2/4] target/riscv: Reuse TB_FLAGS.MSTATUS_HFS_FS
  2023-04-14 16:02 ` [RFC PATCH v2 2/4] target/riscv: Reuse TB_FLAGS.MSTATUS_HFS_FS Mayuresh Chitale
@ 2023-04-15  1:15   ` Weiwei Li
  2023-04-15  1:45   ` Weiwei Li
  1 sibling, 0 replies; 13+ messages in thread
From: Weiwei Li @ 2023-04-15  1:15 UTC (permalink / raw)
  To: Mayuresh Chitale, qemu-devel, qemu-riscv, alistair.francis
  Cc: liweiwei, Alistair Francis, Daniel Barboza, Richard Henderson


On 2023/4/15 00:02, Mayuresh Chitale wrote:
> When misa.F is clear, TB_FLAGS.MSTATUS_HS_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 | 12 ++++++++++++
>   target/riscv/translate.c  |  7 +++++++
>   2 files changed, 19 insertions(+)
>
> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> index 433ea529b0..fd1731cc39 100644
> --- a/target/riscv/cpu_helper.c
> +++ b/target/riscv/cpu_helper.c
> @@ -105,6 +105,18 @@ void cpu_get_tb_cpu_state(CPURISCVState *env, target_ulong *pc,
>           flags = FIELD_DP32(flags, TB_FLAGS, MSTATUS_HS_VS,
>                              get_field(env->mstatus_hs, MSTATUS_VS));
>       }
> +    /*
> +     * If misa.F is 0 then the MSTATUS_HS_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)) {
> +        if (smstateen_acc_ok(env, 0, SMSTATEEN0_FCSR) == RISCV_EXCP_NONE) {
> +            flags = FIELD_DP32(flags, TB_FLAGS, MSTATUS_HS_FS, 1);
> +        } else {
> +            flags = FIELD_DP32(flags, TB_FLAGS, MSTATUS_HS_FS, 0);
> +        }
> +    }
>       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 d0094922b6..e29bbb8b70 100644
> --- a/target/riscv/translate.c
> +++ b/target/riscv/translate.c
> @@ -79,6 +79,7 @@ typedef struct DisasContext {
>       int frm;
>       RISCVMXL ol;
>       bool virt_inst_excp;
> +    bool smstateen_fcsr_ok;
>       bool virt_enabled;
>       const RISCVCPUConfig *cfg_ptr;
>       bool hlsx;
> @@ -1202,6 +1203,12 @@ 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)) {
> +        ctx->smstateen_fcsr_ok = 1;
> +    } else {
> +        ctx->smstateen_fcsr_ok = FIELD_EX32(tb_flags, TB_FLAGS,
> +                                             MSTATUS_HS_FS);

Seems unaligned with tb_flags.

Otherwise, LGTM.

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

Weiwei Li

> +    }
>   }
>   
>   static void riscv_tr_tb_start(DisasContextBase *db, CPUState *cpu)



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

* Re: [RFC PATCH v2 2/4] target/riscv: Reuse TB_FLAGS.MSTATUS_HFS_FS
  2023-04-14 16:02 ` [RFC PATCH v2 2/4] target/riscv: Reuse TB_FLAGS.MSTATUS_HFS_FS Mayuresh Chitale
  2023-04-15  1:15   ` Weiwei Li
@ 2023-04-15  1:45   ` Weiwei Li
  2023-04-26 17:13     ` Mayuresh Chitale
  1 sibling, 1 reply; 13+ messages in thread
From: Weiwei Li @ 2023-04-15  1:45 UTC (permalink / raw)
  To: Mayuresh Chitale, qemu-devel, qemu-riscv, alistair.francis
  Cc: Alistair Francis, Daniel Barboza, liweiwei, Richard Henderson


On 2023/4/15 00:02, Mayuresh Chitale wrote:
> When misa.F is clear, TB_FLAGS.MSTATUS_HS_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 | 12 ++++++++++++
>   target/riscv/translate.c  |  7 +++++++
>   2 files changed, 19 insertions(+)
>
> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> index 433ea529b0..fd1731cc39 100644
> --- a/target/riscv/cpu_helper.c
> +++ b/target/riscv/cpu_helper.c
> @@ -105,6 +105,18 @@ void cpu_get_tb_cpu_state(CPURISCVState *env, target_ulong *pc,
>           flags = FIELD_DP32(flags, TB_FLAGS, MSTATUS_HS_VS,
>                              get_field(env->mstatus_hs, MSTATUS_VS));
>       }
> +    /*
> +     * If misa.F is 0 then the MSTATUS_HS_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)) {
> +        if (smstateen_acc_ok(env, 0, SMSTATEEN0_FCSR) == RISCV_EXCP_NONE) {
> +            flags = FIELD_DP32(flags, TB_FLAGS, MSTATUS_HS_FS, 1);
> +        } else {
> +            flags = FIELD_DP32(flags, TB_FLAGS, MSTATUS_HS_FS, 0);
> +        }
> +    }
>       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 d0094922b6..e29bbb8b70 100644
> --- a/target/riscv/translate.c
> +++ b/target/riscv/translate.c
> @@ -79,6 +79,7 @@ typedef struct DisasContext {
>       int frm;
>       RISCVMXL ol;
>       bool virt_inst_excp;
> +    bool smstateen_fcsr_ok;
>       bool virt_enabled;
>       const RISCVCPUConfig *cfg_ptr;
>       bool hlsx;
> @@ -1202,6 +1203,12 @@ 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)) {
> +        ctx->smstateen_fcsr_ok = 1;
> +    } else {
> +        ctx->smstateen_fcsr_ok = FIELD_EX32(tb_flags, TB_FLAGS,
> +                                             MSTATUS_HS_FS);

By the way, it may introduce new question when MSTATUS_FS and 
MSTATUS_HS_FS is merged to save bits in tb_flag

by Richerd's patchset: 20230412114333.118895-5-richard.henderson@linaro.org

such as: the check "s->mstatus_fs == 0" in require_rvf() will be false 
if smstateen_fcsr_ok is true.

However, this should be true in this case to indicate F is diabled.

So we may need to set ctx->mstatus_fs = 0 here once merged with 
Richerd's patchset.

Regards,

Weiwei Li

> +    }
>   }
>   
>   static void riscv_tr_tb_start(DisasContextBase *db, CPUState *cpu)



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

* Re: [RFC PATCH v2 2/4] target/riscv: Reuse TB_FLAGS.MSTATUS_HFS_FS
  2023-04-15  1:45   ` Weiwei Li
@ 2023-04-26 17:13     ` Mayuresh Chitale
  0 siblings, 0 replies; 13+ messages in thread
From: Mayuresh Chitale @ 2023-04-26 17:13 UTC (permalink / raw)
  To: Weiwei Li
  Cc: qemu-devel, qemu-riscv, alistair.francis, Alistair Francis,
	Daniel Barboza, Richard Henderson

On Sat, Apr 15, 2023 at 7:15 AM Weiwei Li <liweiwei@iscas.ac.cn> wrote:
>
>
> On 2023/4/15 00:02, Mayuresh Chitale wrote:
> > When misa.F is clear, TB_FLAGS.MSTATUS_HS_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 | 12 ++++++++++++
> >   target/riscv/translate.c  |  7 +++++++
> >   2 files changed, 19 insertions(+)
> >
> > diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> > index 433ea529b0..fd1731cc39 100644
> > --- a/target/riscv/cpu_helper.c
> > +++ b/target/riscv/cpu_helper.c
> > @@ -105,6 +105,18 @@ void cpu_get_tb_cpu_state(CPURISCVState *env, target_ulong *pc,
> >           flags = FIELD_DP32(flags, TB_FLAGS, MSTATUS_HS_VS,
> >                              get_field(env->mstatus_hs, MSTATUS_VS));
> >       }
> > +    /*
> > +     * If misa.F is 0 then the MSTATUS_HS_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)) {
> > +        if (smstateen_acc_ok(env, 0, SMSTATEEN0_FCSR) == RISCV_EXCP_NONE) {
> > +            flags = FIELD_DP32(flags, TB_FLAGS, MSTATUS_HS_FS, 1);
> > +        } else {
> > +            flags = FIELD_DP32(flags, TB_FLAGS, MSTATUS_HS_FS, 0);
> > +        }
> > +    }
> >       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 d0094922b6..e29bbb8b70 100644
> > --- a/target/riscv/translate.c
> > +++ b/target/riscv/translate.c
> > @@ -79,6 +79,7 @@ typedef struct DisasContext {
> >       int frm;
> >       RISCVMXL ol;
> >       bool virt_inst_excp;
> > +    bool smstateen_fcsr_ok;
> >       bool virt_enabled;
> >       const RISCVCPUConfig *cfg_ptr;
> >       bool hlsx;
> > @@ -1202,6 +1203,12 @@ 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)) {
> > +        ctx->smstateen_fcsr_ok = 1;
> > +    } else {
> > +        ctx->smstateen_fcsr_ok = FIELD_EX32(tb_flags, TB_FLAGS,
> > +                                             MSTATUS_HS_FS);
>
> By the way, it may introduce new question when MSTATUS_FS and
> MSTATUS_HS_FS is merged to save bits in tb_flag
>
> by Richerd's patchset: 20230412114333.118895-5-richard.henderson@linaro.org
>
> such as: the check "s->mstatus_fs == 0" in require_rvf() will be false
> if smstateen_fcsr_ok is true.
>
> However, this should be true in this case to indicate F is diabled.
>
> So we may need to set ctx->mstatus_fs = 0 here once merged with
> Richerd's patchset.

Yes, that is correct.
>
> Regards,
>
> Weiwei Li
>
> > +    }
> >   }
> >
> >   static void riscv_tr_tb_start(DisasContextBase *db, CPUState *cpu)
>


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

* [RFC PATCH v2 3/4] target/riscv: check smstateen fcsr flag
  2023-04-14 16:01 [RFC PATCH v2 0/4] Smstateen FCSR Mayuresh Chitale
  2023-04-14 16:01 ` [RFC PATCH v2 1/4] target/riscv: smstateen check for fcsr Mayuresh Chitale
  2023-04-14 16:02 ` [RFC PATCH v2 2/4] target/riscv: Reuse TB_FLAGS.MSTATUS_HFS_FS Mayuresh Chitale
@ 2023-04-14 16:02 ` Mayuresh Chitale
  2023-04-15  1:25   ` Weiwei Li
  2023-04-14 16:02 ` [RFC PATCH v2 4/4] target/riscv: smstateen knobs Mayuresh Chitale
  3 siblings, 1 reply; 13+ messages in thread
From: Mayuresh Chitale @ 2023-04-14 16:02 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 | 18 ++++++++++++++---
 3 files changed, 44 insertions(+), 11 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 9e9fa2087a..6bf6fe16be 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 (!has_ext(ctx, RVF) && !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..74a125e4c0 100644
--- a/target/riscv/insn_trans/trans_rvzfh.c.inc
+++ b/target/riscv/insn_trans/trans_rvzfh.c.inc
@@ -16,28 +16,40 @@
  * 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;         \
-    }                         \
+        return false;                  \
+    }                                  \
+    if (!smstateen_fcsr_check(ctx)) {  \
+        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)) {  \
+        return false;                  \
+    }                                  \
 } while (0)
 
 #define REQUIRE_ZFHMIN(ctx) do {              \
     if (!ctx->cfg_ptr->ext_zfhmin) {          \
         return false;                         \
     }                                         \
+    if (!smstateen_fcsr_check(ctx)) {         \
+        return false;                         \
+    }                                         \
 } while (0)
 
 #define REQUIRE_ZFHMIN_OR_ZHINXMIN(ctx) do {                 \
     if (!(ctx->cfg_ptr->ext_zfhmin || 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] 13+ messages in thread

* Re: [RFC PATCH v2 3/4] target/riscv: check smstateen fcsr flag
  2023-04-14 16:02 ` [RFC PATCH v2 3/4] target/riscv: check smstateen fcsr flag Mayuresh Chitale
@ 2023-04-15  1:25   ` Weiwei Li
  2023-04-26 17:18     ` Mayuresh Chitale
  0 siblings, 1 reply; 13+ messages in thread
From: Weiwei Li @ 2023-04-15  1:25 UTC (permalink / raw)
  To: Mayuresh Chitale, qemu-devel, qemu-riscv, alistair.francis
  Cc: liweiwei, Alistair Francis, Daniel Barboza, Richard Henderson


On 2023/4/15 00:02, 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 | 18 ++++++++++++++---
>   3 files changed, 44 insertions(+), 11 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 9e9fa2087a..6bf6fe16be 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 (!has_ext(ctx, RVF) && !ctx->smstateen_fcsr_ok) {

We needn't check RVF here. Two reasons:

1. This check only be done when RVF is diabled.

2. ctx->smstateen_fcsr_ok is always be true (set in last patch) when RVF 
is enabled

> +        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..74a125e4c0 100644
> --- a/target/riscv/insn_trans/trans_rvzfh.c.inc
> +++ b/target/riscv/insn_trans/trans_rvzfh.c.inc
> @@ -16,28 +16,40 @@
>    * 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;         \
> -    }                         \
> +        return false;                  \
> +    }                                  \
> +    if (!smstateen_fcsr_check(ctx)) {  \
> +        return false;                  \
> +    }                                  \

This is unnecessary here. This check is only for Zhinx.

Similar to REQUIRE_ZFHMIN.

>   } 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)) {  \
> +        return false;                  \
> +    }                                  \

This check is only for Zhinx here. So it's better to take the similar 
way as REQUIRE_ZFINX_OR_F.

Similar to REQUIRE_ZFHMIN_OR_ZHINXMIN.

Regards,

Weiwei Li

>   } while (0)
>   
>   #define REQUIRE_ZFHMIN(ctx) do {              \
>       if (!ctx->cfg_ptr->ext_zfhmin) {          \
>           return false;                         \
>       }                                         \
> +    if (!smstateen_fcsr_check(ctx)) {         \
> +        return false;                         \
> +    }                                         \
>   } while (0)
>   
>   #define REQUIRE_ZFHMIN_OR_ZHINXMIN(ctx) do {                 \
>       if (!(ctx->cfg_ptr->ext_zfhmin || 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] 13+ messages in thread

* Re: [RFC PATCH v2 3/4] target/riscv: check smstateen fcsr flag
  2023-04-15  1:25   ` Weiwei Li
@ 2023-04-26 17:18     ` Mayuresh Chitale
  2023-04-27  1:04       ` Weiwei Li
  0 siblings, 1 reply; 13+ messages in thread
From: Mayuresh Chitale @ 2023-04-26 17:18 UTC (permalink / raw)
  To: Weiwei Li
  Cc: qemu-devel, qemu-riscv, alistair.francis, Alistair Francis,
	Daniel Barboza, Richard Henderson

On Sat, Apr 15, 2023 at 6:55 AM Weiwei Li <liweiwei@iscas.ac.cn> wrote:
>
>
> On 2023/4/15 00:02, 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 | 18 ++++++++++++++---
> >   3 files changed, 44 insertions(+), 11 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 9e9fa2087a..6bf6fe16be 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 (!has_ext(ctx, RVF) && !ctx->smstateen_fcsr_ok) {
>
> We needn't check RVF here. Two reasons:
>
> 1. This check only be done when RVF is diabled.
>
> 2. ctx->smstateen_fcsr_ok is always be true (set in last patch) when RVF
> is enabled

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..74a125e4c0 100644
> > --- a/target/riscv/insn_trans/trans_rvzfh.c.inc
> > +++ b/target/riscv/insn_trans/trans_rvzfh.c.inc
> > @@ -16,28 +16,40 @@
> >    * 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;         \
> > -    }                         \
> > +        return false;                  \
> > +    }                                  \
> > +    if (!smstateen_fcsr_check(ctx)) {  \
> > +        return false;                  \
> > +    }                                  \
>
> This is unnecessary here. This check is only for Zhinx.
>
> Similar to REQUIRE_ZFHMIN.
>
> >   } 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)) {  \
> > +        return false;                  \
> > +    }                                  \
>
> This check is only for Zhinx here. So it's better to take the similar
> way as REQUIRE_ZFINX_OR_F.
>
> Similar to REQUIRE_ZFHMIN_OR_ZHINXMIN.

I am not sure I follow the comments above.
The FCSR check is applicable to all the extensions ie zfinx, zdinx,
zhinx and zhinxmin.
>
> Regards,
>
> Weiwei Li
>
> >   } while (0)
> >
> >   #define REQUIRE_ZFHMIN(ctx) do {              \
> >       if (!ctx->cfg_ptr->ext_zfhmin) {          \
> >           return false;                         \
> >       }                                         \
> > +    if (!smstateen_fcsr_check(ctx)) {         \
> > +        return false;                         \
> > +    }                                         \
> >   } while (0)
> >
> >   #define REQUIRE_ZFHMIN_OR_ZHINXMIN(ctx) do {                 \
> >       if (!(ctx->cfg_ptr->ext_zfhmin || 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] 13+ messages in thread

* Re: [RFC PATCH v2 3/4] target/riscv: check smstateen fcsr flag
  2023-04-26 17:18     ` Mayuresh Chitale
@ 2023-04-27  1:04       ` Weiwei Li
  0 siblings, 0 replies; 13+ messages in thread
From: Weiwei Li @ 2023-04-27  1:04 UTC (permalink / raw)
  To: Mayuresh Chitale
  Cc: liweiwei, qemu-devel, qemu-riscv, alistair.francis,
	Alistair Francis, Daniel Barboza, Richard Henderson


On 2023/4/27 01:18, Mayuresh Chitale wrote:
> On Sat, Apr 15, 2023 at 6:55 AM Weiwei Li <liweiwei@iscas.ac.cn> wrote:
>>
>> On 2023/4/15 00:02, 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 | 18 ++++++++++++++---
>>>    3 files changed, 44 insertions(+), 11 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 9e9fa2087a..6bf6fe16be 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 (!has_ext(ctx, RVF) && !ctx->smstateen_fcsr_ok) {
>> We needn't check RVF here. Two reasons:
>>
>> 1. This check only be done when RVF is diabled.
>>
>> 2. ctx->smstateen_fcsr_ok is always be true (set in last patch) when RVF
>> is enabled
> 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..74a125e4c0 100644
>>> --- a/target/riscv/insn_trans/trans_rvzfh.c.inc
>>> +++ b/target/riscv/insn_trans/trans_rvzfh.c.inc
>>> @@ -16,28 +16,40 @@
>>>     * 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;         \
>>> -    }                         \
>>> +        return false;                  \
>>> +    }                                  \
>>> +    if (!smstateen_fcsr_check(ctx)) {  \
>>> +        return false;                  \
>>> +    }                                  \
>> This is unnecessary here. This check is only for Zhinx.
>>
>> Similar to REQUIRE_ZFHMIN.
>>
>>>    } 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)) {  \
>>> +        return false;                  \
>>> +    }                                  \
>> This check is only for Zhinx here. So it's better to take the similar
>> way as REQUIRE_ZFINX_OR_F.
>>
>> Similar to REQUIRE_ZFHMIN_OR_ZHINXMIN.
> I am not sure I follow the comments above.
> The FCSR check is applicable to all the extensions ie zfinx, zdinx,
> zhinx and zhinxmin.

Yeah. FCSR check is applicable to all Z*inx extensions, but unnecessary 
for Zfh.  So I think it's better to be:

       if (!ctx->cfg_ptr->ext_zfh) {                                \
           if (!ctx->cfg_ptr->ext_zhinx)) {                         \
               return false;                                        \
           }                                                        \
           if (!smstateen_fcsr_check(ctx)) {                        \
               return false;                                        \
           }                                                        \
       }

Regards,

Weiwei Li

>> Regards,
>>
>> Weiwei Li
>>
>>>    } while (0)
>>>
>>>    #define REQUIRE_ZFHMIN(ctx) do {              \
>>>        if (!ctx->cfg_ptr->ext_zfhmin) {          \
>>>            return false;                         \
>>>        }                                         \
>>> +    if (!smstateen_fcsr_check(ctx)) {         \
>>> +        return false;                         \
>>> +    }                                         \
>>>    } while (0)
>>>
>>>    #define REQUIRE_ZFHMIN_OR_ZHINXMIN(ctx) do {                 \
>>>        if (!(ctx->cfg_ptr->ext_zfhmin || 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] 13+ messages in thread

* [RFC PATCH v2 4/4] target/riscv: smstateen knobs
  2023-04-14 16:01 [RFC PATCH v2 0/4] Smstateen FCSR Mayuresh Chitale
                   ` (2 preceding siblings ...)
  2023-04-14 16:02 ` [RFC PATCH v2 3/4] target/riscv: check smstateen fcsr flag Mayuresh Chitale
@ 2023-04-14 16:02 ` Mayuresh Chitale
  3 siblings, 0 replies; 13+ messages in thread
From: Mayuresh Chitale @ 2023-04-14 16:02 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 fab38859ec..5c00106899 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] 13+ messages in thread

end of thread, other threads:[~2023-04-27  1:06 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-04-14 16:01 [RFC PATCH v2 0/4] Smstateen FCSR Mayuresh Chitale
2023-04-14 16:01 ` [RFC PATCH v2 1/4] target/riscv: smstateen check for fcsr Mayuresh Chitale
2023-04-15  1:01   ` Weiwei Li
2023-04-26 17:12     ` Mayuresh Chitale
2023-04-14 16:02 ` [RFC PATCH v2 2/4] target/riscv: Reuse TB_FLAGS.MSTATUS_HFS_FS Mayuresh Chitale
2023-04-15  1:15   ` Weiwei Li
2023-04-15  1:45   ` Weiwei Li
2023-04-26 17:13     ` Mayuresh Chitale
2023-04-14 16:02 ` [RFC PATCH v2 3/4] target/riscv: check smstateen fcsr flag Mayuresh Chitale
2023-04-15  1:25   ` Weiwei Li
2023-04-26 17:18     ` Mayuresh Chitale
2023-04-27  1:04       ` Weiwei Li
2023-04-14 16:02 ` [RFC PATCH v2 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).