On Wed, May 17, 2023 at 8:42 AM Alistair Francis <alistai...@gmail.com> wrote: > > On Tue, May 2, 2023 at 12:00 AM Mayuresh Chitale > <mchit...@ventanamicro.com> wrote: > > > > If smstateen is implemented and smtateen0.fcsr is clear and misa.F > > is off then the floating point operations must return illegal > > instruction exception or virtual instruction trap, if relevant. > > Do you mind re-wording this commit message? I can't get my head around > it. You talk about returning an illegal instruction exception, but > most of this patch is just adding SMSTATEEN0_FCSR to the write mask if > floating point is disabled. > Sure. The generic code for access check was added in a previous patch. > It looks to me like you are returning an exception trying to access a > floating pointer register if FP is off and SMSTATEEN0_FCSR is not set > (which you describe) but also then only allow changing SMSTATEEN0_FCSR > if the RVF is not enabled, which is where I'm confused. The smstateen0.fcsr bit is writable only when misa.F == 0.
> > Your patch seems to be correct, I think the commit message and title > just needs a small tweak. Maybe something like this: > > ``` > target/riscv: smstateen add support for fcsr bit > > If smstateen is implemented and SMSTATEEN0.FCSR is zero floating point > CSR access should raise an illegal instruction exception or virtual > equivalent as required. > > We also allow the guest to set/unset the FCSR bit, but only if misa.F > == 0, as defined in the spec. How about: target/riscv: smstateen check for fcsr Implement the s/h/mstateen.fcsr bit as as defined in the smstateen spec and check for it when accessing the fcsr register and its fields. > ``` > > Alistair > > > > > Signed-off-by: Mayuresh Chitale <mchit...@ventanamicro.com> > > Reviewed-by: Weiwei Li <liwei...@iscas.ac.cn> > > --- > > target/riscv/csr.c | 15 +++++++++++++++ > > 1 file changed, 15 insertions(+) > > > > diff --git a/target/riscv/csr.c b/target/riscv/csr.c > > index 4451bd1263..3f6b824bd2 100644 > > --- a/target/riscv/csr.c > > +++ b/target/riscv/csr.c > > @@ -82,6 +82,10 @@ static RISCVException fs(CPURISCVState *env, int csrno) > > !riscv_cpu_cfg(env)->ext_zfinx) { > > return RISCV_EXCP_ILLEGAL_INST; > > } > > + > > + if (!env->debugger && !riscv_cpu_fp_enabled(env)) { > > + return smstateen_acc_ok(env, 0, SMSTATEEN0_FCSR); > > + } > > #endif > > return RISCV_EXCP_NONE; > > } > > @@ -2100,6 +2104,9 @@ static RISCVException write_mstateen0(CPURISCVState > > *env, int csrno, > > target_ulong new_val) > > { > > uint64_t wr_mask = SMSTATEEN_STATEEN | SMSTATEEN0_HSENVCFG; > > + if (!riscv_has_ext(env, RVF)) { > > + wr_mask |= SMSTATEEN0_FCSR; > > + } > > > > return write_mstateen(env, csrno, wr_mask, new_val); > > } > > @@ -2173,6 +2180,10 @@ static RISCVException write_hstateen0(CPURISCVState > > *env, int csrno, > > { > > uint64_t wr_mask = SMSTATEEN_STATEEN | SMSTATEEN0_HSENVCFG; > > > > + if (!riscv_has_ext(env, RVF)) { > > + wr_mask |= SMSTATEEN0_FCSR; > > + } > > + > > return write_hstateen(env, csrno, wr_mask, new_val); > > } > > > > @@ -2259,6 +2270,10 @@ static RISCVException write_sstateen0(CPURISCVState > > *env, int csrno, > > { > > uint64_t wr_mask = SMSTATEEN_STATEEN | SMSTATEEN0_HSENVCFG; > > > > + if (!riscv_has_ext(env, RVF)) { > > + wr_mask |= SMSTATEEN0_FCSR; > > + } > > + > > return write_sstateen(env, csrno, wr_mask, new_val); > > } > > > > -- > > 2.34.1 > >