| Message ID | 20211231032337.15579-3-liweiwei@iscas.ac.cn (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | support subsets of Float-Point in Integer Registers extensions | expand |
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~
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~
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~
在 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~
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;