On Supporting no-FPU machines

On Supporting no-FPU machines

[Alan Kao]

Hi Palmer,

We would like to run Linux on a RISC-V machine that has no F and D
extension, but we found some problem.  We would like to have your
feedback on this.

When context-switching, fstate_restore is always called, and then
regs->sstatus.FS([13:14]) is checked; if the value is anything other
than SR_FS_OFF, the kernel restores the floating-point state for the
next process to be switched to.

However, when a process is loaded, its regs->sstatus.FS is set to
SR_FS_INITIAL (in start_thread), so it is going to lauch __fstate_restore 
(in __switch_to_aux) anyway.  On a board that doesn't have FPU, this causes
a following problem because flds are viewed as illegal instructions.

A quick work-around is something like this:

diff --git a/arch/riscv/kernel/process.c b/arch/riscv/kernel/process.c
index cb20913..96159e1 100644
--- a/arch/riscv/kernel/process.c
+++ b/arch/riscv/kernel/process.c
@@ -83,7 +83,11 @@ void show_regs(struct pt_regs *regs)
 void start_thread(struct pt_regs *regs, unsigned long pc,
        unsigned long sp)
 {
+#ifdef __riscv_flen
        regs->sstatus = SR_SPIE /* User mode, irqs on */ | SR_FS_INITIAL;
+#else
+       regs->sstatus = SR_SPIE | SR_FS_OFF;
+#endif
        regs->sepc = pc;
        regs->sp = sp;
        set_fs(USER_DS);

This prevents the flds in __fstate_restore if choosing an appropriate toolchain.
The condition

(regs->sstatus & SR_FS) != SR_FS_OFF

in fstate_restore will never be taken.  
Actually, we don't even need those floating-point routines for no-FPU boards.  
We would need a larger patch to make those FP code inside some __riscv_flen
pre-processor sections.

Do you think this is going to be a right direction to support no-FPU machines?

Many thanks,
Alan

On Supporting no-FPU machines

[Christoph Hellwig]

On Wed, Jun 13, 2018 at 02:26:53PM +0800, Alan Kao wrote:
> A quick work-around is something like this:

I think we need to detect this at run time, to support a single
kernel image on systems with or without FPU.

> in fstate_restore will never be taken.  
> Actually, we don't even need those floating-point routines for no-FPU boards.  
> We would need a larger patch to make those FP code inside some __riscv_flen
> pre-processor sections.

A separate CONFIG_FPU to not build the FPU code sounds fine to me
as long as it defaults to on.

On Supporting no-FPU machines

[Arnd Bergmann]

On Wed, Jun 13, 2018 at 8:32 AM, Christoph Hellwig <hch@infradead.org> wrote:
> On Wed, Jun 13, 2018 at 02:26:53PM +0800, Alan Kao wrote:
>> A quick work-around is something like this:
>
> I think we need to detect this at run time, to support a single
> kernel image on systems with or without FPU.

Right.

>> in fstate_restore will never be taken.
>> Actually, we don't even need those floating-point routines for no-FPU boards.
>> We would need a larger patch to make those FP code inside some __riscv_flen
>> pre-processor sections.
>
> A separate CONFIG_FPU to not build the FPU code sounds fine to me
> as long as it defaults to on.

If we do this, that option must also take care to disable the FPU hardware
if it exists. Otherwise you might run into the situation of having a system
intended to run without FPU access but a task uses FPU registers anyway
(e.g. because the compiler decides it is faster that way on the
microarchitecture it is optimizing for) and we fail to save/restore the FPU
state between task switches.

        Arnd

On Supporting no-FPU machines

[Christoph Hellwig]

On Wed, Jun 13, 2018 at 08:47:00AM +0200, Arnd Bergmann wrote:
> > A separate CONFIG_FPU to not build the FPU code sounds fine to me
> > as long as it defaults to on.
> 
> If we do this, that option must also take care to disable the FPU hardware
> if it exists. Otherwise you might run into the situation of having a system
> intended to run without FPU access but a task uses FPU registers anyway
> (e.g. because the compiler decides it is faster that way on the
> microarchitecture it is optimizing for) and we fail to save/restore the FPU
> state between task switches.

I'm not sure we can force this.  It would require a write to misa,
which requires M-mode privileges.

On Supporting no-FPU machines

[Christoph Hellwig]

On Wed, Jun 13, 2018 at 12:20:12AM -0700, Christoph Hellwig wrote:
> On Wed, Jun 13, 2018 at 08:47:00AM +0200, Arnd Bergmann wrote:
> > > A separate CONFIG_FPU to not build the FPU code sounds fine to me
> > > as long as it defaults to on.
> > 
> > If we do this, that option must also take care to disable the FPU hardware
> > if it exists. Otherwise you might run into the situation of having a system
> > intended to run without FPU access but a task uses FPU registers anyway
> > (e.g. because the compiler decides it is faster that way on the
> > microarchitecture it is optimizing for) and we fail to save/restore the FPU
> > state between task switches.
> 
> I'm not sure we can force this.  It would require a write to misa,
> which requires M-mode privileges.

[send too early]

Instead we should just refuse to boot a !CONFIG_FPU kernel on a system
with a FPU.

On Supporting no-FPU machines

[Arnd Bergmann]

On Wed, Jun 13, 2018 at 9:20 AM, Christoph Hellwig <hch@infradead.org> wrote:
> On Wed, Jun 13, 2018 at 12:20:12AM -0700, Christoph Hellwig wrote:
>> On Wed, Jun 13, 2018 at 08:47:00AM +0200, Arnd Bergmann wrote:
>> > > A separate CONFIG_FPU to not build the FPU code sounds fine to me
>> > > as long as it defaults to on.
>> >
>> > If we do this, that option must also take care to disable the FPU hardware
>> > if it exists. Otherwise you might run into the situation of having a system
>> > intended to run without FPU access but a task uses FPU registers anyway
>> > (e.g. because the compiler decides it is faster that way on the
>> > microarchitecture it is optimizing for) and we fail to save/restore the FPU
>> > state between task switches.
>>
>> I'm not sure we can force this.  It would require a write to misa,
>> which requires M-mode privileges.

That also means we can't have lazy save/restore of the FPU context,
trapping on the first access to an FPU register from a user space
task before swapping out the previous task's state, right?

> [send too early]
>
> Instead we should just refuse to boot a !CONFIG_FPU kernel on a system
> with a FPU.

Sure, that would work.

          Arnd

On Supporting no-FPU machines

[Darius Rad]

On 06/13/2018 08:29 AM, Arnd Bergmann wrote:
> On Wed, Jun 13, 2018 at 9:20 AM, Christoph Hellwig <hch@infradead.org> wrote:
>> On Wed, Jun 13, 2018 at 12:20:12AM -0700, Christoph Hellwig wrote:
>>> On Wed, Jun 13, 2018 at 08:47:00AM +0200, Arnd Bergmann wrote:
>>>>> A separate CONFIG_FPU to not build the FPU code sounds fine to me
>>>>> as long as it defaults to on.
>>>>
>>>> If we do this, that option must also take care to disable the FPU hardware
>>>> if it exists. Otherwise you might run into the situation of having a system
>>>> intended to run without FPU access but a task uses FPU registers anyway
>>>> (e.g. because the compiler decides it is faster that way on the
>>>> microarchitecture it is optimizing for) and we fail to save/restore the FPU
>>>> state between task switches.
>>>
>>> I'm not sure we can force this.  It would require a write to misa,
>>> which requires M-mode privileges.
> 
> That also means we can't have lazy save/restore of the FPU context,
> trapping on the first access to an FPU register from a user space
> task before swapping out the previous task's state, right?
> 

The FS field of the sstatus register, available in supervisor mode,
should be sufficient for lazy context save/restore.  The FS field could
also serve to prevent use of the FPU with a kernel compiled without
support for one.

>> [send too early]
>>
>> Instead we should just refuse to boot a !CONFIG_FPU kernel on a system
>> with a FPU.
> 
> Sure, that would work.
> 
>           Arnd
> 

On Supporting no-FPU machines

[Alan Kao]

On Wed, Jun 13, 2018 at 02:29:45PM +0200, Arnd Bergmann wrote:
> On Wed, Jun 13, 2018 at 9:20 AM, Christoph Hellwig <hch@infradead.org> wrote:
> > On Wed, Jun 13, 2018 at 12:20:12AM -0700, Christoph Hellwig wrote:
> >> On Wed, Jun 13, 2018 at 08:47:00AM +0200, Arnd Bergmann wrote:
> >> > > A separate CONFIG_FPU to not build the FPU code sounds fine to me
> >> > > as long as it defaults to on.
> >> >
> >> > If we do this, that option must also take care to disable the FPU hardware
> >> > if it exists. Otherwise you might run into the situation of having a system
> >> > intended to run without FPU access but a task uses FPU registers anyway
> >> > (e.g. because the compiler decides it is faster that way on the
> >> > microarchitecture it is optimizing for) and we fail to save/restore the FPU
> >> > state between task switches.
> >>
> >> I'm not sure we can force this.  It would require a write to misa,
> >> which requires M-mode privileges.
> 
> That also means we can't have lazy save/restore of the FPU context,
> trapping on the first access to an FPU register from a user space
> task before swapping out the previous task's state, right?
> 

The lazy FP context feature trades initial performance for flexibility, and
requires changes to both M- and S-mode software.

> > [send too early]
> >
> > Instead we should just refuse to boot a !CONFIG_FPU kernel on a system
> > with a FPU.
> 
> Sure, that would work.
> 
>           Arnd

It seems to me that a CONFIG_FPU option to compile/ignore all the floating-point
relative routines is acceptable from this discussion.  Besides, there should be
a mechanism that refuses to boot a !CONFIG_FPU kernel on a system with F or D,
and also refuses to boot a CONFIG_FPU kernel on a system lacking both.

In this way, we can eliminate unnecessary codes to the maximum and keep the
modifications small for RISC-V machines without FPU.

Thanks for all your opinions.  If there is no more suggestions, we will be ready
soon to send a patch for this.

Alan

On Supporting no-FPU machines

[Palmer Dabbelt]

On Tue, 12 Jun 2018 23:26:53 PDT (-0700), alankao at andestech.com wrote:
> Hi Palmer,
>
> We would like to run Linux on a RISC-V machine that has no F and D
> extension, but we found some problem.  We would like to have your
> feedback on this.
>
> When context-switching, fstate_restore is always called, and then
> regs->sstatus.FS([13:14]) is checked; if the value is anything other
> than SR_FS_OFF, the kernel restores the floating-point state for the
> next process to be switched to.
>
> However, when a process is loaded, its regs->sstatus.FS is set to
> SR_FS_INITIAL (in start_thread), so it is going to lauch __fstate_restore
> (in __switch_to_aux) anyway.  On a board that doesn't have FPU, this causes
> a following problem because flds are viewed as illegal instructions.
>
> A quick work-around is something like this:
>
> diff --git a/arch/riscv/kernel/process.c b/arch/riscv/kernel/process.c
> index cb20913..96159e1 100644
> --- a/arch/riscv/kernel/process.c
> +++ b/arch/riscv/kernel/process.c
> @@ -83,7 +83,11 @@ void show_regs(struct pt_regs *regs)
>  void start_thread(struct pt_regs *regs, unsigned long pc,
>         unsigned long sp)
>  {
> +#ifdef __riscv_flen
>         regs->sstatus = SR_SPIE /* User mode, irqs on */ | SR_FS_INITIAL;
> +#else
> +       regs->sstatus = SR_SPIE | SR_FS_OFF;
> +#endif
>         regs->sepc = pc;
>         regs->sp = sp;
>         set_fs(USER_DS);
>
> This prevents the flds in __fstate_restore if choosing an appropriate toolchain.
> The condition
>
> (regs->sstatus & SR_FS) != SR_FS_OFF
>
> in fstate_restore will never be taken.
> Actually, we don't even need those floating-point routines for no-FPU boards.
> We would need a larger patch to make those FP code inside some __riscv_flen
> pre-processor sections.
>
> Do you think this is going to be a right direction to support no-FPU machines?

Ooh, I think this might be an Andrew question.  I'd been operating under the 
assumption that sstatus.fs was a WARL field, with the result being that writing 
sstatus.fs to initial would have no effect on a machine with no floating point 
support.  This is the behavior that Rocket has, but I can't actually find this 
definition anywhere in the ISA spec, though.  If the spec is the way I've 
interpreted it, then we can fix up start_thread by just round tripping the 
value through sstatus to see what the machine actually supports.

Otherwise we'll have to have a kernel config option to enable/disable the FPU,
and while that's not a big deal it's slightly nicer if the same kernel works on
FPU and no-FPU systems.

Andrew: am I missing something?

 Alan

On Supporting no-FPU machines

[Andrew Waterman]

On Fri, Jun 15, 2018 at 1:38 PM, Palmer Dabbelt <palmer@sifive.com> wrote:
> On Tue, 12 Jun 2018 23:26:53 PDT (-0700), alankao at andestech.com wrote:
>>
>> Hi Palmer,
>>
>> We would like to run Linux on a RISC-V machine that has no F and D
>> extension, but we found some problem.  We would like to have your
>> feedback on this.
>>
>> When context-switching, fstate_restore is always called, and then
>> regs->sstatus.FS([13:14]) is checked; if the value is anything other
>> than SR_FS_OFF, the kernel restores the floating-point state for the
>> next process to be switched to.
>>
>> However, when a process is loaded, its regs->sstatus.FS is set to
>> SR_FS_INITIAL (in start_thread), so it is going to lauch __fstate_restore
>> (in __switch_to_aux) anyway.  On a board that doesn't have FPU, this
>> causes
>> a following problem because flds are viewed as illegal instructions.
>>
>> A quick work-around is something like this:
>>
>> diff --git a/arch/riscv/kernel/process.c b/arch/riscv/kernel/process.c
>> index cb20913..96159e1 100644
>> --- a/arch/riscv/kernel/process.c
>> +++ b/arch/riscv/kernel/process.c
>> @@ -83,7 +83,11 @@ void show_regs(struct pt_regs *regs)
>>  void start_thread(struct pt_regs *regs, unsigned long pc,
>>         unsigned long sp)
>>  {
>> +#ifdef __riscv_flen
>>         regs->sstatus = SR_SPIE /* User mode, irqs on */ | SR_FS_INITIAL;
>> +#else
>> +       regs->sstatus = SR_SPIE | SR_FS_OFF;
>> +#endif
>>         regs->sepc = pc;
>>         regs->sp = sp;
>>         set_fs(USER_DS);
>>
>> This prevents the flds in __fstate_restore if choosing an appropriate
>> toolchain.
>> The condition
>>
>> (regs->sstatus & SR_FS) != SR_FS_OFF
>>
>> in fstate_restore will never be taken.
>> Actually, we don't even need those floating-point routines for no-FPU
>> boards.
>> We would need a larger patch to make those FP code inside some
>> __riscv_flen
>> pre-processor sections.
>>
>> Do you think this is going to be a right direction to support no-FPU
>> machines?
>
>
> Ooh, I think this might be an Andrew question.  I'd been operating under the
> assumption that sstatus.fs was a WARL field, with the result being that
> writing sstatus.fs to initial would have no effect on a machine with no
> floating point support.  This is the behavior that Rocket has, but I can't
> actually find this definition anywhere in the ISA spec, though.  If the spec
> is the way I've interpreted it, then we can fix up start_thread by just
> round tripping the value through sstatus to see what the machine actually
> supports.

"In systems that do not implement S-mode and do not have a
floating-point unit, the FS field is hardwired to zero."

Since the system has S-mode, FS should be writable, even though there
is no FPU.  We defined it this way to support M-mode emulation of the
FPU.

>
> Otherwise we'll have to have a kernel config option to enable/disable the
> FPU,
> and while that's not a big deal it's slightly nicer if the same kernel works
> on
> FPU and no-FPU systems.

I think this makes sense.

>
> Andrew: am I missing something?
>
> Alan