* [PATCH RESEND v2] target/riscv: Set mstatus_hs.[SD|FS] bits if Clean and V=1 in mark_fs_dirty()
@ 2021-09-17 9:31 ` frank.chang
0 siblings, 0 replies; 8+ messages in thread
From: frank.chang @ 2021-09-17 9:31 UTC (permalink / raw)
To: qemu-devel, qemu-riscv
Cc: Frank Chang, Vincent Chen, Alistair Francis, Bin Meng, Palmer Dabbelt
From: Frank Chang <frank.chang@sifive.com>
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 <frank.chang@sifive.com>
Reviewed-by: Vincent Chen <vincent.chen@sifive.com>
Tested-by: Vincent Chen <vincent.chen@sifive.com>
---
target/riscv/cpu.h | 4 ++++
target/riscv/translate.c | 24 +++++++++++++++---------
2 files changed, 19 insertions(+), 9 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..2b48db6fd02 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,26 +281,30 @@ 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->virt_enabled && ctx->mstatus_hs_fs != MSTATUS_FS) {
+ /* Remember the stage change for the rest of the TB. */
+ ctx->mstatus_hs_fs = MSTATUS_FS;
+
+ 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);
+ }
if (ctx->mstatus_fs == MSTATUS_FS) {
return;
}
+
/* 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;
-
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) {
- 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);
}
#else
@@ -533,6 +538,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
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH RESEND v2] target/riscv: Set mstatus_hs.[SD|FS] bits if Clean and V=1 in mark_fs_dirty()
@ 2021-09-17 9:31 ` frank.chang
0 siblings, 0 replies; 8+ messages in thread
From: frank.chang @ 2021-09-17 9:31 UTC (permalink / raw)
To: qemu-devel, qemu-riscv
Cc: Frank Chang, Vincent Chen, Palmer Dabbelt, Alistair Francis, Bin Meng
From: Frank Chang <frank.chang@sifive.com>
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 <frank.chang@sifive.com>
Reviewed-by: Vincent Chen <vincent.chen@sifive.com>
Tested-by: Vincent Chen <vincent.chen@sifive.com>
---
target/riscv/cpu.h | 4 ++++
target/riscv/translate.c | 24 +++++++++++++++---------
2 files changed, 19 insertions(+), 9 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..2b48db6fd02 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,26 +281,30 @@ 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->virt_enabled && ctx->mstatus_hs_fs != MSTATUS_FS) {
+ /* Remember the stage change for the rest of the TB. */
+ ctx->mstatus_hs_fs = MSTATUS_FS;
+
+ 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);
+ }
if (ctx->mstatus_fs == MSTATUS_FS) {
return;
}
+
/* 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;
-
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) {
- 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);
}
#else
@@ -533,6 +538,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
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH RESEND v2] target/riscv: Set mstatus_hs.[SD|FS] bits if Clean and V=1 in mark_fs_dirty()
2021-09-17 9:31 ` frank.chang
@ 2021-09-18 18:46 ` Richard Henderson
-1 siblings, 0 replies; 8+ messages in thread
From: Richard Henderson @ 2021-09-18 18:46 UTC (permalink / raw)
To: frank.chang, qemu-devel, qemu-riscv
Cc: Vincent Chen, Bin Meng, Alistair Francis, Palmer Dabbelt
On 9/17/21 2:31 AM, frank.chang@sifive.com wrote:
> From: Frank Chang <frank.chang@sifive.com>
>
> 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 <frank.chang@sifive.com>
> Reviewed-by: Vincent Chen <vincent.chen@sifive.com>
> Tested-by: Vincent Chen <vincent.chen@sifive.com>
> ---
> target/riscv/cpu.h | 4 ++++
> target/riscv/translate.c | 24 +++++++++++++++---------
> 2 files changed, 19 insertions(+), 9 deletions(-)
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> static void mark_fs_dirty(DisasContext *ctx)
> {
> TCGv tmp;
> - target_ulong sd;
> + target_ulong sd = is_32bit(ctx) ? MSTATUS32_SD : MSTATUS64_SD;
> +
> + 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;
> +
> + 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);
> + }
>
> if (ctx->mstatus_fs == MSTATUS_FS) {
> return;
> }
> +
> /* 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;
> -
> 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) {
> - 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);
While it works, it would be nicer to keep these two cases as similar as possible.
r~
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH RESEND v2] target/riscv: Set mstatus_hs.[SD|FS] bits if Clean and V=1 in mark_fs_dirty()
@ 2021-09-18 18:46 ` Richard Henderson
0 siblings, 0 replies; 8+ messages in thread
From: Richard Henderson @ 2021-09-18 18:46 UTC (permalink / raw)
To: frank.chang, qemu-devel, qemu-riscv
Cc: Vincent Chen, Alistair Francis, Bin Meng, Palmer Dabbelt
On 9/17/21 2:31 AM, frank.chang@sifive.com wrote:
> From: Frank Chang <frank.chang@sifive.com>
>
> 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 <frank.chang@sifive.com>
> Reviewed-by: Vincent Chen <vincent.chen@sifive.com>
> Tested-by: Vincent Chen <vincent.chen@sifive.com>
> ---
> target/riscv/cpu.h | 4 ++++
> target/riscv/translate.c | 24 +++++++++++++++---------
> 2 files changed, 19 insertions(+), 9 deletions(-)
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> static void mark_fs_dirty(DisasContext *ctx)
> {
> TCGv tmp;
> - target_ulong sd;
> + target_ulong sd = is_32bit(ctx) ? MSTATUS32_SD : MSTATUS64_SD;
> +
> + 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;
> +
> + 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);
> + }
>
> if (ctx->mstatus_fs == MSTATUS_FS) {
> return;
> }
> +
> /* 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;
> -
> 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) {
> - 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);
While it works, it would be nicer to keep these two cases as similar as possible.
r~
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH RESEND v2] target/riscv: Set mstatus_hs.[SD|FS] bits if Clean and V=1 in mark_fs_dirty()
2021-09-18 18:46 ` Richard Henderson
@ 2021-09-19 2:54 ` Frank Chang
-1 siblings, 0 replies; 8+ messages in thread
From: Frank Chang @ 2021-09-19 2:54 UTC (permalink / raw)
To: Richard Henderson
Cc: open list:RISC-V, Bin Meng, qemu-devel@nongnu.org Developers,
Vincent Chen, Alistair Francis, Palmer Dabbelt
[-- Attachment #1: Type: text/plain, Size: 3193 bytes --]
On Sun, Sep 19, 2021 at 2:46 AM Richard Henderson <
richard.henderson@linaro.org> wrote:
> On 9/17/21 2:31 AM, frank.chang@sifive.com wrote:
> > From: Frank Chang <frank.chang@sifive.com>
> >
> > 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 <frank.chang@sifive.com>
> > Reviewed-by: Vincent Chen <vincent.chen@sifive.com>
> > Tested-by: Vincent Chen <vincent.chen@sifive.com>
> > ---
> > target/riscv/cpu.h | 4 ++++
> > target/riscv/translate.c | 24 +++++++++++++++---------
> > 2 files changed, 19 insertions(+), 9 deletions(-)
>
>
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
>
> > static void mark_fs_dirty(DisasContext *ctx)
> > {
> > TCGv tmp;
> > - target_ulong sd;
> > + target_ulong sd = is_32bit(ctx) ? MSTATUS32_SD : MSTATUS64_SD;
> > +
> > + 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;
> > +
> > + 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);
> > + }
> >
> > if (ctx->mstatus_fs == MSTATUS_FS) {
> > return;
> > }
> > +
> > /* 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;
> > -
> > 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) {
> > - 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);
>
> While it works, it would be nicer to keep these two cases as similar as
> possible.
>
>
Hi, Richard, thanks for the review.
Do you mean it's better to change to code sequence to something like:
static void mark_fs_dirty(DisasContext *ctx)
{
.....
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->mstatus_fs != MSTATUS_FS) {
/* Remember the state change for the rest of the TB. */
ctx->mstatus_fs = MSTATUS_FS;
.....
}
}
If so, I can update and send out the v3 patch.
Regards,
Frank Chang
>
> r~
>
[-- Attachment #2: Type: text/html, Size: 5010 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH RESEND v2] target/riscv: Set mstatus_hs.[SD|FS] bits if Clean and V=1 in mark_fs_dirty()
@ 2021-09-19 2:54 ` Frank Chang
0 siblings, 0 replies; 8+ messages in thread
From: Frank Chang @ 2021-09-19 2:54 UTC (permalink / raw)
To: Richard Henderson
Cc: qemu-devel@nongnu.org Developers, open list:RISC-V, Vincent Chen,
Alistair Francis, Bin Meng, Palmer Dabbelt
[-- Attachment #1: Type: text/plain, Size: 3193 bytes --]
On Sun, Sep 19, 2021 at 2:46 AM Richard Henderson <
richard.henderson@linaro.org> wrote:
> On 9/17/21 2:31 AM, frank.chang@sifive.com wrote:
> > From: Frank Chang <frank.chang@sifive.com>
> >
> > 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 <frank.chang@sifive.com>
> > Reviewed-by: Vincent Chen <vincent.chen@sifive.com>
> > Tested-by: Vincent Chen <vincent.chen@sifive.com>
> > ---
> > target/riscv/cpu.h | 4 ++++
> > target/riscv/translate.c | 24 +++++++++++++++---------
> > 2 files changed, 19 insertions(+), 9 deletions(-)
>
>
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
>
> > static void mark_fs_dirty(DisasContext *ctx)
> > {
> > TCGv tmp;
> > - target_ulong sd;
> > + target_ulong sd = is_32bit(ctx) ? MSTATUS32_SD : MSTATUS64_SD;
> > +
> > + 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;
> > +
> > + 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);
> > + }
> >
> > if (ctx->mstatus_fs == MSTATUS_FS) {
> > return;
> > }
> > +
> > /* 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;
> > -
> > 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) {
> > - 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);
>
> While it works, it would be nicer to keep these two cases as similar as
> possible.
>
>
Hi, Richard, thanks for the review.
Do you mean it's better to change to code sequence to something like:
static void mark_fs_dirty(DisasContext *ctx)
{
.....
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->mstatus_fs != MSTATUS_FS) {
/* Remember the state change for the rest of the TB. */
ctx->mstatus_fs = MSTATUS_FS;
.....
}
}
If so, I can update and send out the v3 patch.
Regards,
Frank Chang
>
> r~
>
[-- Attachment #2: Type: text/html, Size: 5010 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH RESEND v2] target/riscv: Set mstatus_hs.[SD|FS] bits if Clean and V=1 in mark_fs_dirty()
2021-09-19 2:54 ` Frank Chang
@ 2021-09-19 16:32 ` Richard Henderson
-1 siblings, 0 replies; 8+ messages in thread
From: Richard Henderson @ 2021-09-19 16:32 UTC (permalink / raw)
To: Frank Chang
Cc: open list:RISC-V, Bin Meng, qemu-devel@nongnu.org Developers,
Vincent Chen, Alistair Francis, Palmer Dabbelt
On 9/18/21 7:54 PM, Frank Chang wrote:
> Do you mean it's better to change to code sequence to something like:
>
> static void mark_fs_dirty(DisasContext *ctx)
> {
> .....
>
> 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->mstatus_fs != MSTATUS_FS) {
> /* Remember the state change for the rest of the TB. */
> ctx->mstatus_fs = MSTATUS_FS;
> .....
> }
> }
>
> If so, I can update and send out the v3 patch.
Yes, something like that.
r~
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH RESEND v2] target/riscv: Set mstatus_hs.[SD|FS] bits if Clean and V=1 in mark_fs_dirty()
@ 2021-09-19 16:32 ` Richard Henderson
0 siblings, 0 replies; 8+ messages in thread
From: Richard Henderson @ 2021-09-19 16:32 UTC (permalink / raw)
To: Frank Chang
Cc: qemu-devel@nongnu.org Developers, open list:RISC-V, Vincent Chen,
Alistair Francis, Bin Meng, Palmer Dabbelt
On 9/18/21 7:54 PM, Frank Chang wrote:
> Do you mean it's better to change to code sequence to something like:
>
> static void mark_fs_dirty(DisasContext *ctx)
> {
> .....
>
> 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->mstatus_fs != MSTATUS_FS) {
> /* Remember the state change for the rest of the TB. */
> ctx->mstatus_fs = MSTATUS_FS;
> .....
> }
> }
>
> If so, I can update and send out the v3 patch.
Yes, something like that.
r~
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2021-09-19 16:33 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-17 9:31 [PATCH RESEND v2] target/riscv: Set mstatus_hs.[SD|FS] bits if Clean and V=1 in mark_fs_dirty() frank.chang
2021-09-17 9:31 ` frank.chang
2021-09-18 18:46 ` Richard Henderson
2021-09-18 18:46 ` Richard Henderson
2021-09-19 2:54 ` Frank Chang
2021-09-19 2:54 ` Frank Chang
2021-09-19 16:32 ` Richard Henderson
2021-09-19 16:32 ` Richard Henderson
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.