Re: [PATCH v2 2/6] target/riscv: hardwire mstatus.FS to zero when enable zfinx

2022-01-01 Thread Weiwei Li 



在 2022/1/2 上午3:46, Richard Henderson 写道:

On 12/31/21 9:55 PM, Weiwei Li wrote:

This shouldn't be necessary because it should never have been set.
Yes, I think so. However, I have a question about MSTATUS_MIE and 
MSTATUS_MPRV, will they be set before cpu reset?


Yes, via warm reset.

Thanks. I'll fix this later.


I would think it would be more correct to have a positive test for 
RVF, rather than a negative test for ZFINX?
It may  deviate from the original value of mstatus_mask with a 
positive test for RVF.


Oh, you mean misa_ext?  Hmm.  Interesting point.

When F extension is not implemented, FS is either hardwired to zero 
(without S-mode) or optionally zero.  So this looks like an existing 
bug to be fixed.



OK.  There is similar logic in write_mstatus and .  I'll fix it too.

I'll also update write_frm,  write_fflags and write_fflags to have a 
positive test for RVF .




r~

Re: [PATCH v2 2/6] target/riscv: hardwire mstatus.FS to zero when enable zfinx

2022-01-01 Thread Richard Henderson 

On 12/31/21 9:55 PM, Weiwei Li wrote:

This shouldn't be necessary because it should never have been set.
Yes, I think so. However, I have a question about MSTATUS_MIE and MSTATUS_MPRV, will they 
be set before cpu reset?


Yes, via warm reset.

I would think it would be more correct to have a positive test for RVF, rather than a 
negative test for ZFINX?

It may  deviate from the original value of mstatus_mask with a positive test 
for RVF.


Oh, you mean misa_ext?  Hmm.  Interesting point.

When F extension is not implemented, FS is either hardwired to zero (without S-mode) or 
optionally zero.  So this looks like an existing bug to be fixed.



r~

Re: [PATCH v2 2/6] target/riscv: hardwire mstatus.FS to zero when enable zfinx

2021-12-31 Thread Weiwei Li 

Thanks for your comments.

在 2022/1/1 上午3:56, Richard Henderson 写道:

On 12/30/21 7:23 PM, Weiwei Li wrote:

@@ -363,6 +363,10 @@ static void riscv_cpu_reset(DeviceState *dev)
  env->misa_mxl = env->misa_mxl_max;
  env->priv = PRV_M;
  env->mstatus &= ~(MSTATUS_MIE | MSTATUS_MPRV);
+    /* hardwire mstatus.FS to zero when enable zfinx */
+    if (RISCV_CPU(env_cpu(env))->cfg.ext_zfinx) {
+    env->mstatus &= ~MSTATUS_FS;
+    }


This shouldn't be necessary because it should never have been set.
Yes, I think so. However, I have a question about MSTATUS_MIE and 
MSTATUS_MPRV, will they be set before cpu reset?



-    uint64_t mstatus_mask = MSTATUS_MXR | MSTATUS_SUM | MSTATUS_FS |
+    uint64_t mstatus_mask = MSTATUS_MXR | MSTATUS_SUM|
  MSTATUS_SPP | MSTATUS_SPIE | MSTATUS_SIE |
  MSTATUS64_UXL | MSTATUS_VS;
+    /* hardwire mstatus.FS to zero when enable zfinx */
+    if (!RISCV_CPU(env_cpu(env))->cfg.ext_zfinx) {
+    mstatus_mask |= MSTATUS_FS;
+    }


I would think it would be more correct to have a positive test for 
RVF, rather than a negative test for ZFINX?
It may  deviate from the original value of mstatus_mask with a positive 
test for RVF.



r~

Re: [PATCH v2 2/6] target/riscv: hardwire mstatus.FS to zero when enable zfinx

2021-12-31 Thread Richard Henderson 

On 12/30/21 7:23 PM, Weiwei Li wrote:

@@ -363,6 +363,10 @@ static void riscv_cpu_reset(DeviceState *dev)
  env->misa_mxl = env->misa_mxl_max;
  env->priv = PRV_M;
  env->mstatus &= ~(MSTATUS_MIE | MSTATUS_MPRV);
+/* hardwire mstatus.FS to zero when enable zfinx */
+if (RISCV_CPU(env_cpu(env))->cfg.ext_zfinx) {
+env->mstatus &= ~MSTATUS_FS;
+}


This shouldn't be necessary because it should never have been set.


-uint64_t mstatus_mask = MSTATUS_MXR | MSTATUS_SUM | MSTATUS_FS |
+uint64_t mstatus_mask = MSTATUS_MXR | MSTATUS_SUM|
  MSTATUS_SPP | MSTATUS_SPIE | MSTATUS_SIE |
  MSTATUS64_UXL | MSTATUS_VS;
+/* hardwire mstatus.FS to zero when enable zfinx */
+if (!RISCV_CPU(env_cpu(env))->cfg.ext_zfinx) {
+mstatus_mask |= MSTATUS_FS;
+}


I would think it would be more correct to have a positive test for RVF, rather than a 
negative test for ZFINX?



r~

[PATCH v2 2/6] target/riscv: hardwire mstatus.FS to zero when enable zfinx

2021-12-30 Thread Weiwei Li 
From: liweiwei 

Co-authored-by: ardxwe 
Signed-off-by: Weiwei Li 
Signed-off-by: Junqiang Wang 
---
 target/riscv/cpu.c|  4 
 target/riscv/cpu_helper.c |  6 +-
 target/riscv/csr.c| 24 +++-
 target/riscv/translate.c  |  5 +
 4 files changed, 33 insertions(+), 6 deletions(-)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index d9ea005724..cc7da446f1 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -363,6 +363,10 @@ static void riscv_cpu_reset(DeviceState *dev)
 env->misa_mxl = env->misa_mxl_max;
 env->priv = PRV_M;
 env->mstatus &= ~(MSTATUS_MIE | MSTATUS_MPRV);
+/* hardwire mstatus.FS to zero when enable zfinx */
+if (RISCV_CPU(env_cpu(env))->cfg.ext_zfinx) {
+env->mstatus &= ~MSTATUS_FS;
+}
 if (env->misa_mxl > MXL_RV32) {
 /*
  * The reset status of SXL/UXL is undefined, but mstatus is WARL
diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index 10f3baba53..a71edee44c 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -222,9 +222,13 @@ bool riscv_cpu_vector_enabled(CPURISCVState *env)
 
 void riscv_cpu_swap_hypervisor_regs(CPURISCVState *env)
 {
-uint64_t mstatus_mask = MSTATUS_MXR | MSTATUS_SUM | MSTATUS_FS |
+uint64_t mstatus_mask = MSTATUS_MXR | MSTATUS_SUM|
 MSTATUS_SPP | MSTATUS_SPIE | MSTATUS_SIE |
 MSTATUS64_UXL | MSTATUS_VS;
+/* hardwire mstatus.FS to zero when enable zfinx */
+if (!RISCV_CPU(env_cpu(env))->cfg.ext_zfinx) {
+mstatus_mask |= MSTATUS_FS;
+}
 bool current_virt = riscv_cpu_virt_enabled(env);
 
 g_assert(riscv_has_ext(env, RVH));
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index 146447eac5..de20206b73 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -38,7 +38,8 @@ void riscv_set_csr_ops(int csrno, riscv_csr_operations *ops)
 static RISCVException fs(CPURISCVState *env, int csrno)
 {
 #if !defined(CONFIG_USER_ONLY)
-if (!env->debugger && !riscv_cpu_fp_enabled(env)) {
+if (!env->debugger && !riscv_cpu_fp_enabled(env) &&
+!RISCV_CPU(env_cpu(env))->cfg.ext_zfinx) {
 return RISCV_EXCP_ILLEGAL_INST;
 }
 #endif
@@ -234,7 +235,10 @@ static RISCVException write_fflags(CPURISCVState *env, int 
csrno,
target_ulong val)
 {
 #if !defined(CONFIG_USER_ONLY)
-env->mstatus |= MSTATUS_FS;
+/* hardwire mstatus.FS to zero when enable zfinx */
+if (!RISCV_CPU(env_cpu(env))->cfg.ext_zfinx) {
+env->mstatus |= MSTATUS_FS;
+}
 #endif
 riscv_cpu_set_fflags(env, val & (FSR_AEXC >> FSR_AEXC_SHIFT));
 return RISCV_EXCP_NONE;
@@ -251,7 +255,10 @@ static RISCVException write_frm(CPURISCVState *env, int 
csrno,
 target_ulong val)
 {
 #if !defined(CONFIG_USER_ONLY)
-env->mstatus |= MSTATUS_FS;
+/* hardwire mstatus.FS to zero when enable zfinx */
+if (!RISCV_CPU(env_cpu(env))->cfg.ext_zfinx) {
+env->mstatus |= MSTATUS_FS;
+}
 #endif
 env->frm = val & (FSR_RD >> FSR_RD_SHIFT);
 return RISCV_EXCP_NONE;
@@ -269,7 +276,10 @@ static RISCVException write_fcsr(CPURISCVState *env, int 
csrno,
  target_ulong val)
 {
 #if !defined(CONFIG_USER_ONLY)
-env->mstatus |= MSTATUS_FS;
+/* hardwire mstatus.FS to zero when enable zfinx */
+if (!RISCV_CPU(env_cpu(env))->cfg.ext_zfinx) {
+env->mstatus |= MSTATUS_FS;
+}
 #endif
 env->frm = (val & FSR_RD) >> FSR_RD_SHIFT;
 riscv_cpu_set_fflags(env, (val & FSR_AEXC) >> FSR_AEXC_SHIFT);
@@ -562,9 +572,13 @@ static RISCVException write_mstatus(CPURISCVState *env, 
int csrno,
 tlb_flush(env_cpu(env));
 }
 mask = MSTATUS_SIE | MSTATUS_SPIE | MSTATUS_MIE | MSTATUS_MPIE |
-MSTATUS_SPP | MSTATUS_FS | MSTATUS_MPRV | MSTATUS_SUM |
+MSTATUS_SPP | MSTATUS_MPRV | MSTATUS_SUM |
 MSTATUS_MPP | MSTATUS_MXR | MSTATUS_TVM | MSTATUS_TSR |
 MSTATUS_TW | MSTATUS_VS;
+/* hardwire mstatus.FS to zero when enable zfinx */
+if (!RISCV_CPU(env_cpu(env))->cfg.ext_zfinx) {
+mask |= MSTATUS_FS;
+}
 
 if (riscv_cpu_mxl(env) != MXL_RV32) {
 /*
diff --git a/target/riscv/translate.c b/target/riscv/translate.c
index 8b1cdacf50..17bf20a799 100644
--- a/target/riscv/translate.c
+++ b/target/riscv/translate.c
@@ -342,6 +342,11 @@ static void mark_fs_dirty(DisasContext *ctx)
 {
 TCGv tmp;
 
+/* hardwire mstatus.FS to zero when enable zfinx */
+if (ctx->ext_zfinx) {
+return;
+}
+
 if (ctx->mstatus_fs != MSTATUS_FS) {
 /* Remember the state change for the rest of the TB. */
 ctx->mstatus_fs = MSTATUS_FS;
-- 
2.17.1