Re: [PATCH v3] target/riscv: Set mstatus_hs.[SD|FS] bits if Clean and V=1 in mark_fs_dirty()

2021-09-24 Thread Alistair Francis
On Tue, Sep 21, 2021 at 12:03 PM  wrote:
>
> From: Frank Chang 
>
> When V=1, both vsstauts.FS and HS-level sstatus.FS are in effect.
> Modifying the floating-point state when V=1 causes both fields to
> be set to 3 (Dirty).
>
> However, it's possible that HS-level sstatus.FS is Clean and VS-level
> vsstatus.FS is Dirty at the time mark_fs_dirty() is called when V=1.
> We can't early return for this case because we still need to set
> sstatus.FS to Dirty according to spec.
>
> Signed-off-by: Frank Chang 
> Reviewed-by: Vincent Chen 
> Tested-by: Vincent Chen 
> Reviewed-by: Richard Henderson 

Thanks!

Applied to riscv-to-apply.next

Alistair



Re: [PATCH v3] target/riscv: Set mstatus_hs.[SD|FS] bits if Clean and V=1 in mark_fs_dirty()

2021-09-24 Thread Alistair Francis
On Tue, Sep 21, 2021 at 12:03 PM  wrote:
>
> From: Frank Chang 
>
> When V=1, both vsstauts.FS and HS-level sstatus.FS are in effect.
> Modifying the floating-point state when V=1 causes both fields to
> be set to 3 (Dirty).
>
> However, it's possible that HS-level sstatus.FS is Clean and VS-level
> vsstatus.FS is Dirty at the time mark_fs_dirty() is called when V=1.
> We can't early return for this case because we still need to set
> sstatus.FS to Dirty according to spec.
>
> Signed-off-by: Frank Chang 
> Reviewed-by: Vincent Chen 
> Tested-by: Vincent Chen 
> Reviewed-by: Richard Henderson 

Reviewed-by: Alistair Francis 

Alistair

> ---
>  target/riscv/cpu.h   |  4 
>  target/riscv/translate.c | 30 +-
>  2 files changed, 21 insertions(+), 13 deletions(-)
>
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index e735e53e26c..bef7c551646 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -394,6 +394,7 @@ FIELD(TB_FLAGS, SEW, 5, 3)
>  FIELD(TB_FLAGS, VILL, 8, 1)
>  /* Is a Hypervisor instruction load/store allowed? */
>  FIELD(TB_FLAGS, HLSX, 9, 1)
> +FIELD(TB_FLAGS, MSTATUS_HS_FS, 10, 2)
>
>  bool riscv_cpu_is_32bit(CPURISCVState *env);
>
> @@ -450,6 +451,9 @@ static inline void cpu_get_tb_cpu_state(CPURISCVState 
> *env, target_ulong *pc,
>  get_field(env->hstatus, HSTATUS_HU))) {
>  flags = FIELD_DP32(flags, TB_FLAGS, HLSX, 1);
>  }
> +
> +flags = FIELD_DP32(flags, TB_FLAGS, MSTATUS_HS_FS,
> +   get_field(env->mstatus_hs, MSTATUS_FS));
>  }
>  #endif
>
> diff --git a/target/riscv/translate.c b/target/riscv/translate.c
> index 74b33fa3c90..6be22347426 100644
> --- a/target/riscv/translate.c
> +++ b/target/riscv/translate.c
> @@ -58,6 +58,7 @@ typedef struct DisasContext {
>  target_ulong misa;
>  uint32_t opcode;
>  uint32_t mstatus_fs;
> +uint32_t mstatus_hs_fs;
>  uint32_t mem_idx;
>  /* Remember the rounding mode encoded in the previous fp instruction,
> which we have already installed into env->fp_status.  Or -1 for
> @@ -280,27 +281,29 @@ static void gen_jal(DisasContext *ctx, int rd, 
> target_ulong imm)
>  static void mark_fs_dirty(DisasContext *ctx)
>  {
>  TCGv tmp;
> -target_ulong sd;
> +target_ulong sd = is_32bit(ctx) ? MSTATUS32_SD : MSTATUS64_SD;
>
> -if (ctx->mstatus_fs == MSTATUS_FS) {
> -return;
> -}
> -/* Remember the state change for the rest of the TB.  */
> -ctx->mstatus_fs = MSTATUS_FS;
> +if (ctx->mstatus_fs != MSTATUS_FS) {
> +/* Remember the state change for the rest of the TB. */
> +ctx->mstatus_fs = MSTATUS_FS;
>
> -tmp = tcg_temp_new();
> -sd = is_32bit(ctx) ? MSTATUS32_SD : MSTATUS64_SD;
> +tmp = tcg_temp_new();
> +tcg_gen_ld_tl(tmp, cpu_env, offsetof(CPURISCVState, mstatus));
> +tcg_gen_ori_tl(tmp, tmp, MSTATUS_FS | sd);
> +tcg_gen_st_tl(tmp, cpu_env, offsetof(CPURISCVState, mstatus));
> +tcg_temp_free(tmp);
> +}
>
> -tcg_gen_ld_tl(tmp, cpu_env, offsetof(CPURISCVState, mstatus));
> -tcg_gen_ori_tl(tmp, tmp, MSTATUS_FS | sd);
> -tcg_gen_st_tl(tmp, cpu_env, offsetof(CPURISCVState, mstatus));
> +if (ctx->virt_enabled && ctx->mstatus_hs_fs != MSTATUS_FS) {
> +/* Remember the stage change for the rest of the TB. */
> +ctx->mstatus_hs_fs = MSTATUS_FS;
>
> -if (ctx->virt_enabled) {
> +tmp = tcg_temp_new();
>  tcg_gen_ld_tl(tmp, cpu_env, offsetof(CPURISCVState, mstatus_hs));
>  tcg_gen_ori_tl(tmp, tmp, MSTATUS_FS | sd);
>  tcg_gen_st_tl(tmp, cpu_env, offsetof(CPURISCVState, mstatus_hs));
> +tcg_temp_free(tmp);
>  }
> -tcg_temp_free(tmp);
>  }
>  #else
>  static inline void mark_fs_dirty(DisasContext *ctx) { }
> @@ -533,6 +536,7 @@ static void riscv_tr_init_disas_context(DisasContextBase 
> *dcbase, CPUState *cs)
>  ctx->frm = -1;  /* unknown rounding mode */
>  ctx->ext_ifencei = cpu->cfg.ext_ifencei;
>  ctx->vlen = cpu->cfg.vlen;
> +ctx->mstatus_hs_fs = FIELD_EX32(tb_flags, TB_FLAGS, MSTATUS_HS_FS);
>  ctx->hlsx = FIELD_EX32(tb_flags, TB_FLAGS, HLSX);
>  ctx->vill = FIELD_EX32(tb_flags, TB_FLAGS, VILL);
>  ctx->sew = FIELD_EX32(tb_flags, TB_FLAGS, SEW);
> --
> 2.25.1
>
>



[PATCH v3] target/riscv: Set mstatus_hs.[SD|FS] bits if Clean and V=1 in mark_fs_dirty()

2021-09-20 Thread frank . chang
From: Frank Chang 

When V=1, both vsstauts.FS and HS-level sstatus.FS are in effect.
Modifying the floating-point state when V=1 causes both fields to
be set to 3 (Dirty).

However, it's possible that HS-level sstatus.FS is Clean and VS-level
vsstatus.FS is Dirty at the time mark_fs_dirty() is called when V=1.
We can't early return for this case because we still need to set
sstatus.FS to Dirty according to spec.

Signed-off-by: Frank Chang 
Reviewed-by: Vincent Chen 
Tested-by: Vincent Chen 
Reviewed-by: Richard Henderson 
---
 target/riscv/cpu.h   |  4 
 target/riscv/translate.c | 30 +-
 2 files changed, 21 insertions(+), 13 deletions(-)

diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index e735e53e26c..bef7c551646 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -394,6 +394,7 @@ FIELD(TB_FLAGS, SEW, 5, 3)
 FIELD(TB_FLAGS, VILL, 8, 1)
 /* Is a Hypervisor instruction load/store allowed? */
 FIELD(TB_FLAGS, HLSX, 9, 1)
+FIELD(TB_FLAGS, MSTATUS_HS_FS, 10, 2)
 
 bool riscv_cpu_is_32bit(CPURISCVState *env);
 
@@ -450,6 +451,9 @@ static inline void cpu_get_tb_cpu_state(CPURISCVState *env, 
target_ulong *pc,
 get_field(env->hstatus, HSTATUS_HU))) {
 flags = FIELD_DP32(flags, TB_FLAGS, HLSX, 1);
 }
+
+flags = FIELD_DP32(flags, TB_FLAGS, MSTATUS_HS_FS,
+   get_field(env->mstatus_hs, MSTATUS_FS));
 }
 #endif
 
diff --git a/target/riscv/translate.c b/target/riscv/translate.c
index 74b33fa3c90..6be22347426 100644
--- a/target/riscv/translate.c
+++ b/target/riscv/translate.c
@@ -58,6 +58,7 @@ typedef struct DisasContext {
 target_ulong misa;
 uint32_t opcode;
 uint32_t mstatus_fs;
+uint32_t mstatus_hs_fs;
 uint32_t mem_idx;
 /* Remember the rounding mode encoded in the previous fp instruction,
which we have already installed into env->fp_status.  Or -1 for
@@ -280,27 +281,29 @@ static void gen_jal(DisasContext *ctx, int rd, 
target_ulong imm)
 static void mark_fs_dirty(DisasContext *ctx)
 {
 TCGv tmp;
-target_ulong sd;
+target_ulong sd = is_32bit(ctx) ? MSTATUS32_SD : MSTATUS64_SD;
 
-if (ctx->mstatus_fs == MSTATUS_FS) {
-return;
-}
-/* Remember the state change for the rest of the TB.  */
-ctx->mstatus_fs = MSTATUS_FS;
+if (ctx->mstatus_fs != MSTATUS_FS) {
+/* Remember the state change for the rest of the TB. */
+ctx->mstatus_fs = MSTATUS_FS;
 
-tmp = tcg_temp_new();
-sd = is_32bit(ctx) ? MSTATUS32_SD : MSTATUS64_SD;
+tmp = tcg_temp_new();
+tcg_gen_ld_tl(tmp, cpu_env, offsetof(CPURISCVState, mstatus));
+tcg_gen_ori_tl(tmp, tmp, MSTATUS_FS | sd);
+tcg_gen_st_tl(tmp, cpu_env, offsetof(CPURISCVState, mstatus));
+tcg_temp_free(tmp);
+}
 
-tcg_gen_ld_tl(tmp, cpu_env, offsetof(CPURISCVState, mstatus));
-tcg_gen_ori_tl(tmp, tmp, MSTATUS_FS | sd);
-tcg_gen_st_tl(tmp, cpu_env, offsetof(CPURISCVState, mstatus));
+if (ctx->virt_enabled && ctx->mstatus_hs_fs != MSTATUS_FS) {
+/* Remember the stage change for the rest of the TB. */
+ctx->mstatus_hs_fs = MSTATUS_FS;
 
-if (ctx->virt_enabled) {
+tmp = tcg_temp_new();
 tcg_gen_ld_tl(tmp, cpu_env, offsetof(CPURISCVState, mstatus_hs));
 tcg_gen_ori_tl(tmp, tmp, MSTATUS_FS | sd);
 tcg_gen_st_tl(tmp, cpu_env, offsetof(CPURISCVState, mstatus_hs));
+tcg_temp_free(tmp);
 }
-tcg_temp_free(tmp);
 }
 #else
 static inline void mark_fs_dirty(DisasContext *ctx) { }
@@ -533,6 +536,7 @@ static void riscv_tr_init_disas_context(DisasContextBase 
*dcbase, CPUState *cs)
 ctx->frm = -1;  /* unknown rounding mode */
 ctx->ext_ifencei = cpu->cfg.ext_ifencei;
 ctx->vlen = cpu->cfg.vlen;
+ctx->mstatus_hs_fs = FIELD_EX32(tb_flags, TB_FLAGS, MSTATUS_HS_FS);
 ctx->hlsx = FIELD_EX32(tb_flags, TB_FLAGS, HLSX);
 ctx->vill = FIELD_EX32(tb_flags, TB_FLAGS, VILL);
 ctx->sew = FIELD_EX32(tb_flags, TB_FLAGS, SEW);
-- 
2.25.1