search

openhwgroup / corev-gcc

GNU General Public License v2.0
22 stars 23 forks source link

Extensions state save and restore #36

Open pascalgouedo opened 1 year ago

pascalgouedo commented 1 year ago

When using corev-openhw-gcc-centos7-20230331 toolchain with F extensions, all interrupt handlers blindly save and restore f registers without looking at Floating-Point state in MSTATUS register. Moreover it does not save/restore full state as FCSR is missing.

In RISC-V Privilege specification, Machine Status Register (mstatus) section shows some fields for extension states:

When SD equals 1, this means that any of those 3 extensions has been used since they have been enabled or since the last context restore. So FS/XS/VS should be tested to understand which extension is "Dirty" and needs whole state save and restore when changing the execution context (interrupt route, context switch, etc,...).

When SD is 0, no state needs to be saved/restored and interrupt latency/context switch/... is improved because 33 registers write in memory (generally stack) are not done. Same for the state restore at the end of the interrupt routine/context switch/...

A full description of the FS/XS/VS state transitions is described in Table 3.4

pascalgouedo commented 1 year ago

Be careful that those save/restore action should be ISA/configuration dependent. If F is present in MISA and if MSTATUS.SD = 1 then save FPR + FFLAGS and set MSTATUS.FS to Clean at call step; if FS = Clean restore FPR + FFLAGS at return step. If F is not present in MISA but Zfinx CSR LSB is 1 then always save/restore only FFLAGS (no MSTATUS.FS/SD). else do nothing

simonpcook commented 1 year ago

I see some work has been done on this upstream - at least from a first glance https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=dbcbc858c71f69da76d1f36d6bb5d72f2db11eda looks like it may potentially help/solve this issue once a future rebase occurs?

MaryBennett commented 1 year ago

@pascalgouedo, the commit mentioned by @simonpcook is now in our codebase, and is in the latest toolchain release. Does this address the issue for you? Can we close this issue?

pascalgouedo commented 1 year ago

Hi @Mary Just had a look to the commit and this seems just to address the missing FCSR save/restore. But it doesn't seem to address the second problem of blindly saving/restoring F state without having a look to xstatus.sd

pascalgouedo commented 11 months ago

Hi I changed it again to bug because as of today, this save/restore is done even if Floating-Point extension is not enabled (MSTATUS.FS = OFF) causing illegal instruction detection and respective handler to be executed.

This finally causes deadlock because returning address from an interrupt (MEPC) has not been saved at the beginning of the save phase and is lost because of its override by the illegal instruction detection.

So this MSTATUS.FS value check should be added to all the checks described above.

MaryBennett commented 5 months ago

AP: Is there an upstream bug? No bug yet: RISC-V bugs