1. Patchew
  2. QEMU
  3. [RFC PATCH v2 0/4] Smstateen FCSR
  4. View patch

[RFC PATCH v2 2/4] target/riscv: Reuse TB_FLAGS.MSTATUS_HFS_FS

Mayuresh Chitale posted 4 patches 2 years, 10 months ago
There is a newer version of this series
[RFC PATCH v2 2/4] target/riscv: Reuse TB_FLAGS.MSTATUS_HFS_FS
Posted by Mayuresh Chitale 2 years, 10 months ago 
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
Re: [RFC PATCH v2 2/4] target/riscv: Reuse TB_FLAGS.MSTATUS_HFS_FS
Posted by Weiwei Li 2 years, 10 months ago 
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)
Re: [RFC PATCH v2 2/4] target/riscv: Reuse TB_FLAGS.MSTATUS_HFS_FS
Posted by Mayuresh Chitale 2 years, 9 months ago 
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)
>
Re: [RFC PATCH v2 2/4] target/riscv: Reuse TB_FLAGS.MSTATUS_HFS_FS
Posted by Weiwei Li 2 years, 10 months ago 
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)

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.