Re: riscv: How to check floating point support is currently enabled?

[Alistair Francis]

On Wed, Jan 22, 2020 at 8:22 AM Ian Jiang <address@hidden> wrote:
>
> Alistair Francis <address@hidden> 于2020年1月21日周二 下午6:50写道：
> >
> > On Tue, Jan 21, 2020 at 11:12 AM Ian Jiang <address@hidden> wrote:
> > >
> > > The function riscv_cpu_fp_enabled() is used for checking whether floating 
> > > point support is currently enabled. In fact it checks the FS field in the 
> > > mstatus MSR.
> > >
> > > target/riscv/cpu_helper.c
> > >  76 bool riscv_cpu_fp_enabled(CPURISCVState *env)
> > >  77 {
> > >  78     if (env->mstatus & MSTATUS_FS) {
> > >  79         return true;
> > >  80     }
> > >  81
> > >  82     return false;
> > >  83 }
> > >
> > > This will cause a problem that the SD bit in mstatus is not set to 1 when 
> > >  FS in mstatus is modified from '00'b to '11'b with write_mstatus().
> >
> > Thanks for looking into this.
> >
> > There are patches on list fixing floating point errors. Can you check
> > if this branch fixes any issues you have:
> > https://github.com/palmer-dabbelt/qemu/commits/for-master
>
> Yes, there is:
> https://github.com/palmer-dabbelt/qemu/commit/82f014671cf057de51c4a577c9e2ad637dcec6f9

Great! This should make it into master soon.

Alistair

> Thanks!
>
> >
> > >
> > > file target/riscv/csr.c, func write_mstatus():
> > > 350     dirty = (riscv_cpu_fp_enabled(env) &&
> > > 351              ((mstatus & MSTATUS_FS) == MSTATUS_FS)) |
> > > 352             ((mstatus & MSTATUS_XS) == MSTATUS_XS);
> > > 353     mstatus = set_field(mstatus, MSTATUS_SD, dirty);
> > > 354     env->mstatus = mstatus;
> > >
> > > So checking fields D and F in the misa MSR (bit 3 and bit 5) may be an 
> > > better way. That is
> > > bool riscv_cpu_fp_enabled(CPURISCVState *env)
> > >     if (env->misa & (MISA_F | MISA_F) {
> > >         return true;
> > >     }
> > >     return false;
> > > }
> >
> > This doesn't seem right, just because the HW supports it doesn't mean
> > it's enabled.
>
> OK.
>
> >
> > Alistair
> >
> > >
> > >
> > > --
> > > Ian Jiang