Re: [PATCH v2 2/6] target/riscv: hardwire mstatus.FS to zero when enable zfinx
在 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
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
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
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~