linux-riscv.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* BUG: FP registers leak across execve
@ 2018-12-10 22:13 Aurelien Jarno
  2019-01-03 23:36 ` Palmer Dabbelt
  0 siblings, 1 reply; 5+ messages in thread
From: Aurelien Jarno @ 2018-12-10 22:13 UTC (permalink / raw)
  To: linux-riscv


[-- Attachment #1.1.1: Type: text/plain, Size: 1318 bytes --]

Hi all,

Debugging some glibc testsuite math failures, I have found out that most
of the time, the FP status register and the FP registers are not zeroed
as they should. This can be tested with the attached code. The best way
to reproduce it is to execute from Python (i guess Perl or another
interpreted language that support FP computation should work). When 
running an FP computation before calling the program, the result of the
computation can be seen in f10.

The zeroing of the FP status happens in kernel/process.c in the
flush_thread function. It seems that the kernel restore that state only
if a context switch happens between flush_thread and the first FP
instruction of the executed program.

A possible workaround is to restore of the FP registers in flush_thread,
but that's probably not the best way to do that:


--- a/arch/riscv/kernel/process.c
+++ b/arch/riscv/kernel/process.c
@@ -93,6 +93,7 @@ void flush_thread(void)
         *      fflags: accrued exceptions cleared
         */
        memset(&current->thread.fstate, 0, sizeof(current->thread.fstate));
+       fstate_restore(current, task_pt_regs(current));
 #endif
 }


Aurelien

-- 
Aurelien Jarno                          GPG: 4096R/1DDD8C9B
aurelien@aurel32.net                 http://www.aurel32.net

[-- Attachment #1.1.2: dump-fpu-state.c --]
[-- Type: text/x-csrc, Size: 1235 bytes --]

#include <stdio.h>
#include <stdint.h>

int
main ()
{
  uint32_t cw;
  uint64_t f[32];

  __asm__ volatile ("frsr %0":"=r" (cw));

  printf ("cw = %08x\n", cw);

  __asm__ volatile ("fsd  f0,   0(%0)\n\t"
		    "fsd  f1,   8(%0)\n\t"
		    "fsd  f2,  16(%0)\n\t"
		    "fsd  f3,  24(%0)\n\t"
		    "fsd  f4,  32(%0)\n\t"
		    "fsd  f5,  40(%0)\n\t"
		    "fsd  f6,  48(%0)\n\t"
		    "fsd  f7,  56(%0)\n\t"
		    "fsd  f8,  64(%0)\n\t"
		    "fsd  f9,  72(%0)\n\t"
		    "fsd f10,  80(%0)\n\t"
		    "fsd f11,  88(%0)\n\t"
		    "fsd f12,  96(%0)\n\t"
		    "fsd f13, 104(%0)\n\t"
		    "fsd f14, 112(%0)\n\t"
		    "fsd f15, 120(%0)\n\t"
		    "fsd f16, 128(%0)\n\t"
		    "fsd f17, 136(%0)\n\t"
		    "fsd f18, 144(%0)\n\t"
		    "fsd f19, 152(%0)\n\t"
		    "fsd f20, 160(%0)\n\t"
		    "fsd f21, 168(%0)\n\t"
		    "fsd f22, 176(%0)\n\t"
		    "fsd f23, 184(%0)\n\t"
		    "fsd f24, 192(%0)\n\t"
		    "fsd f25, 200(%0)\n\t"
		    "fsd f26, 208(%0)\n\t"
		    "fsd f27, 216(%0)\n\t"
		    "fsd f28, 224(%0)\n\t"
		    "fsd f29, 232(%0)\n\t"
		    "fsd f30, 240(%0)\n\t"
		    "fsd f31, 248(%0)\n\t"
		    :
		    :"r" (f)
		    :"memory");

  for (int i = 0; i < 31; i++)
    {
      printf ("f%02i = %016lx\n", i, f[i]);
    }
}

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 161 bytes --]

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: BUG: FP registers leak across execve
  2018-12-10 22:13 BUG: FP registers leak across execve Aurelien Jarno
@ 2019-01-03 23:36 ` Palmer Dabbelt
  2019-01-04  1:39   ` Aurelien Jarno
  0 siblings, 1 reply; 5+ messages in thread
From: Palmer Dabbelt @ 2019-01-03 23:36 UTC (permalink / raw)
  To: aurelien; +Cc: linux-riscv

On Mon, 10 Dec 2018 14:13:28 PST (-0800), aurelien@aurel32.net wrote:
> Hi all,
>
> Debugging some glibc testsuite math failures, I have found out that most
> of the time, the FP status register and the FP registers are not zeroed
> as they should. This can be tested with the attached code. The best way
> to reproduce it is to execute from Python (i guess Perl or another
> interpreted language that support FP computation should work). When
> running an FP computation before calling the program, the result of the
> computation can be seen in f10.
>
> The zeroing of the FP status happens in kernel/process.c in the
> flush_thread function. It seems that the kernel restore that state only
> if a context switch happens between flush_thread and the first FP
> instruction of the executed program.
>
> A possible workaround is to restore of the FP registers in flush_thread,
> but that's probably not the best way to do that:
>
>
> --- a/arch/riscv/kernel/process.c
> +++ b/arch/riscv/kernel/process.c
> @@ -93,6 +93,7 @@ void flush_thread(void)
>          *      fflags: accrued exceptions cleared
>          */
>         memset(&current->thread.fstate, 0, sizeof(current->thread.fstate));
> +       fstate_restore(current, task_pt_regs(current));
>  #endif
>  }

Are you running this in QEMU?  IIRC there was a bug here and we might not have 
the right fix upstream yet.

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: BUG: FP registers leak across execve
  2019-01-03 23:36 ` Palmer Dabbelt
@ 2019-01-04  1:39   ` Aurelien Jarno
  2019-01-04  3:12     ` Benjamin Herrenschmidt
  2019-01-22 20:31     ` Palmer Dabbelt
  0 siblings, 2 replies; 5+ messages in thread
From: Aurelien Jarno @ 2019-01-04  1:39 UTC (permalink / raw)
  To: Palmer Dabbelt; +Cc: linux-riscv

On 2019-01-03 15:36, Palmer Dabbelt wrote:
> On Mon, 10 Dec 2018 14:13:28 PST (-0800), aurelien@aurel32.net wrote:
> > Hi all,
> > 
> > Debugging some glibc testsuite math failures, I have found out that most
> > of the time, the FP status register and the FP registers are not zeroed
> > as they should. This can be tested with the attached code. The best way
> > to reproduce it is to execute from Python (i guess Perl or another
> > interpreted language that support FP computation should work). When
> > running an FP computation before calling the program, the result of the
> > computation can be seen in f10.
> > 
> > The zeroing of the FP status happens in kernel/process.c in the
> > flush_thread function. It seems that the kernel restore that state only
> > if a context switch happens between flush_thread and the first FP
> > instruction of the executed program.
> > 
> > A possible workaround is to restore of the FP registers in flush_thread,
> > but that's probably not the best way to do that:
> > 
> > 
> > --- a/arch/riscv/kernel/process.c
> > +++ b/arch/riscv/kernel/process.c
> > @@ -93,6 +93,7 @@ void flush_thread(void)
> >          *      fflags: accrued exceptions cleared
> >          */
> >         memset(&current->thread.fstate, 0, sizeof(current->thread.fstate));
> > +       fstate_restore(current, task_pt_regs(current));
> >  #endif
> >  }
> 
> Are you running this in QEMU?  IIRC there was a bug here and we might not
> have the right fix upstream yet.

I can reproduce the issue in a QEMU 3.1 VM running a 4.20 kernel, but
also on an HFU board running the original kernel.

-- 
Aurelien Jarno                          GPG: 4096R/1DDD8C9B
aurelien@aurel32.net                 http://www.aurel32.net

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: BUG: FP registers leak across execve
  2019-01-04  1:39   ` Aurelien Jarno
@ 2019-01-04  3:12     ` Benjamin Herrenschmidt
  2019-01-22 20:31     ` Palmer Dabbelt
  1 sibling, 0 replies; 5+ messages in thread
From: Benjamin Herrenschmidt @ 2019-01-04  3:12 UTC (permalink / raw)
  To: Aurelien Jarno, Palmer Dabbelt; +Cc: linux-riscv

On Fri, 2019-01-04 at 02:39 +0100, Aurelien Jarno wrote:
> On 2019-01-03 15:36, Palmer Dabbelt wrote:
> > On Mon, 10 Dec 2018 14:13:28 PST (-0800), aurelien@aurel32.net wrote:
> > > Hi all,
> > > 
> > > Debugging some glibc testsuite math failures, I have found out that most
> > > of the time, the FP status register and the FP registers are not zeroed
> > > as they should. This can be tested with the attached code. The best way
> > > to reproduce it is to execute from Python (i guess Perl or another
> > > interpreted language that support FP computation should work). When
> > > running an FP computation before calling the program, the result of the
> > > computation can be seen in f10.
> > > 
> > > The zeroing of the FP status happens in kernel/process.c in the
> > > flush_thread function. It seems that the kernel restore that state only
> > > if a context switch happens between flush_thread and the first FP
> > > instruction of the executed program.
> > > 
> > > A possible workaround is to restore of the FP registers in flush_thread,
> > > but that's probably not the best way to do that:
> > > 
> > > 
> > > --- a/arch/riscv/kernel/process.c
> > > +++ b/arch/riscv/kernel/process.c
> > > @@ -93,6 +93,7 @@ void flush_thread(void)
> > >          *      fflags: accrued exceptions cleared
> > >          */
> > >         memset(&current->thread.fstate, 0, sizeof(current->thread.fstate));
> > > +       fstate_restore(current, task_pt_regs(current));
> > >  #endif
> > >  }
> > 
> > Are you running this in QEMU?  IIRC there was a bug here and we might not
> > have the right fix upstream yet.
> 
> I can reproduce the issue in a QEMU 3.1 VM running a 4.20 kernel, but
> also on an HFU board running the original kernel.

You can do that or force a restore in start_thread I think. (powerpc
does all that stuff in start_thread rather than flush_thread but that's
probably not that great an idea).

Cheers,
Ben.


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: BUG: FP registers leak across execve
  2019-01-04  1:39   ` Aurelien Jarno
  2019-01-04  3:12     ` Benjamin Herrenschmidt
@ 2019-01-22 20:31     ` Palmer Dabbelt
  1 sibling, 0 replies; 5+ messages in thread
From: Palmer Dabbelt @ 2019-01-22 20:31 UTC (permalink / raw)
  To: aurelien; +Cc: linux-riscv

On Thu, 03 Jan 2019 17:39:32 PST (-0800), aurelien@aurel32.net wrote:
> On 2019-01-03 15:36, Palmer Dabbelt wrote:
>> On Mon, 10 Dec 2018 14:13:28 PST (-0800), aurelien@aurel32.net wrote:
>> > Hi all,
>> >
>> > Debugging some glibc testsuite math failures, I have found out that most
>> > of the time, the FP status register and the FP registers are not zeroed
>> > as they should. This can be tested with the attached code. The best way
>> > to reproduce it is to execute from Python (i guess Perl or another
>> > interpreted language that support FP computation should work). When
>> > running an FP computation before calling the program, the result of the
>> > computation can be seen in f10.
>> >
>> > The zeroing of the FP status happens in kernel/process.c in the
>> > flush_thread function. It seems that the kernel restore that state only
>> > if a context switch happens between flush_thread and the first FP
>> > instruction of the executed program.
>> >
>> > A possible workaround is to restore of the FP registers in flush_thread,
>> > but that's probably not the best way to do that:
>> >
>> >
>> > --- a/arch/riscv/kernel/process.c
>> > +++ b/arch/riscv/kernel/process.c
>> > @@ -93,6 +93,7 @@ void flush_thread(void)
>> >          *      fflags: accrued exceptions cleared
>> >          */
>> >         memset(&current->thread.fstate, 0, sizeof(current->thread.fstate));
>> > +       fstate_restore(current, task_pt_regs(current));
>> >  #endif
>> >  }
>>
>> Are you running this in QEMU?  IIRC there was a bug here and we might not
>> have the right fix upstream yet.
>
> I can reproduce the issue in a QEMU 3.1 VM running a 4.20 kernel, but
> also on an HFU board running the original kernel.

OK, so at least it's not a QEMU bug.

I think this is the right place to put it, but we should also add

    regs->sstatus &= ~SR_FS_INITIAL;
    regs->sstatus |= SR_FS_INITIAL;

so the FPU's status register corresponds to the contents of the FPU.  I'm not 
sure if this is strictly necessary, but it'll certainly help FP save/restore 
performance.  I'm trying to construct a scenario where not setting setting 
SR.FS=initial actually manifests in incorrect behavior and can't think of one, 
but it does feel incorrect.

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2019-01-22 20:31 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-12-10 22:13 BUG: FP registers leak across execve Aurelien Jarno
2019-01-03 23:36 ` Palmer Dabbelt
2019-01-04  1:39   ` Aurelien Jarno
2019-01-04  3:12     ` Benjamin Herrenschmidt
2019-01-22 20:31     ` Palmer Dabbelt

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).