[PATCH 2/2] sparc: Fix mistakenly cleared PSR[EF] bit.

Maksim Kozlov maksim.e.kozlov at gmail.com
Thu Jun 6 16:11:56 UTC 2019


Hello, Sebastian

I think that this patch is necessary in two reasons.

In general view, if we have a conditional branch, we want, obviously, to
invoke two instruction siquences with different results. Without this
patch we clear EF bit in conditional branch in any case and I think this
is not that we want regardless if this causes error or not.

More specific technical reason is that there is a real situation - I have
debugging info and instruction trace (which was got with hardware debugging
module DSU on LEON3) where I see that sometimes we return from interrupt
with cleared PSR[EF] flag in the floating point task (I described this a
bit in my first message) and this causes excessive lazy fp switch
(previously I had system crash before fixing lazy fp switch syscall).

Just now I can't give very detail description why and how this happens,
but bellow I put part of LEON3 DSU trace for mentioned above situation

This is part of code which I use to investigate problem which we talk about

...
4f102d8c:       95 a2 88 58     faddd  %f10, %f24, %f10
4f102d90:       80 a3 00 09     cmp  %o4, %o1
4f102d94:       91 a2 09 54     fmuld  %f8, %f20, %f8
4f102d98:       95 a0 05 4a     fsqrtd  %f10, %f10
4f102d9c:       d5 3b bf f8     std  %f10, [ %sp + -8 ]
4f102da0:       01 00 00 00     nop
4f102da4:       91 a2 08 4a     faddd  %f8, %f10, %f8
4f102da8:       95 a4 89 c8     fdivd  %f18, %f8, %f10
4f102dac:       d5 3b bf f8     std  %f10, [ %sp + -8 ]
4f102db0:       01 00 00 00     nop
4f102db4:       d1 38 7f 70     std  %f8, [ %g1 + -144 ]
4f102db8:       d1 18 80 00     ldd  [ %g2 ], %f8        <--- Interrupt
4f102dbc:       91 a2 08 ce     fsubd  %f8, %f14, %f8
4f102dc0:       91 a2 09 4a     fmuld  %f8, %f10, %f8
4f102dc4:       d1 38 7f 58     std  %f8, [ %g1 + -168 ]
4f102dc8:       d1 18 a0 08     ldd  [ %g2 + 8 ], %f8
4f102dcc:       91 a2 08 cc     fsubd  %f8, %f12, %f8
...

Part of trace


463493523  4F06D6A0  restore                       [00000000]
463493524  4F06D6A4  ldd  [%g1], %l0               [0000000C 4F57C348]
463493526  4F06D6A8  ldd  [%g1 + 0x8], %l2         [0406CE9C 0406CE78]
463493528  4F06D6AC  ldd  [%g1 + 0x10], %l4        [0406CE54 0406CE30]
463493530  4F06D6B0  ldd  [%g1 + 0x18], %l6        [4F57C528 0406CE84]
463493533  4F06D6B4  ldd  [%g1 + 0x20], %i0        [0406CF78 4F534BB0]
463493610  4F06D6B8  ldd  [%g1 + 0x28], %i2        [0408CDC0 0406CE24]
463493612  4F06D6BC  ldd  [%g1 + 0x30], %i4        [00000001 00000003]
463493616  4F06D6C0  ldd  [%g1 + 0x38], %fp        [4F534B40 4F103868]
463493620  4F06D6C4  save                          [00000000]
463493622  4F06D6C8  mov  %l0, %psr                [F34000C6] <-- (1)
463493625  4F06D6CC  nop                           [00000000]
463493702  4F06D6D0  nop                           [00000000]
463493703  4F06D6D4  nop                           [00000000]
463493717  4F06D6D8  ld  [%g1 + 0x6C], %g1         [0408D148]
463493718  4F06D6DC  jmp  %l1                      [4F06D6DC]
463493719  4F06D6E0  rett  %l2                     [4F102DC0]
463493722  4F102DBC  fsubd  %f8, %f14, %f8         [  TRAP  ] <-- (2)
463493728  04000040  mov  %psr, %l0                [F34000C6]
463493729  04000044  sethi  %hi(0x4F06D800), %l4   [4F06D800]
463493730  04000048  jmp  %l4 + 0x20               [04000048]
463493731  0400004C  sethi  %hi(0x1000), %l3       [00001000]


Here (1) we see that restored PSR[EF] == 0 and directly after return from
interrupt (2) we fall in lazy fp switch syscall

To be honest, I cant say that I fully understand how _ISR_Handler procedure
works, therefore your help would be very desirable.

When I get more debugging information, I'll provide it. But it takes some
time.
--
Best regards,
Maksim Kozlov


On Thu, 6 Jun 2019 at 16:13, Sebastian Huber <
sebastian.huber at embedded-brains.de> wrote:

> Hello Maksim,
>
> On 06/06/2019 14:43, Maksim Kozlov wrote:
> > On Thu, 6 Jun 2019 at 09:57, Sebastian Huber
> > <sebastian.huber at embedded-brains.de
> > <mailto:sebastian.huber at embedded-brains.de>> wrote:
> >
> >
> >
> >     On 05/06/2019 21:22, Maksim E. Kozlov wrote:
> >     > ---
> >     >   cpukit/score/cpu/sparc/cpu_asm.S | 3 ++-
> >     >   1 file changed, 2 insertions(+), 1 deletion(-)
> >     >
> >     > diff --git a/cpukit/score/cpu/sparc/cpu_asm.S
> >     b/cpukit/score/cpu/sparc/cpu_asm.S
> >     > index 2f3ad24510..69bbfb2748 100644
> >     > --- a/cpukit/score/cpu/sparc/cpu_asm.S
> >     > +++ b/cpukit/score/cpu/sparc/cpu_asm.S
> >     > @@ -682,10 +682,11 @@ isr_dispatch:
> >     >           ld      [%g6 + SPARC_PER_CPU_FP_OWNER_OFFSET], %l7
> >     >           cmp     %l6, %l7
> >     >           bne     .Ldisable_fp
> >     > -         andn   %l0, %l5, %l0
> >     > +         nop
> >
> >     Wouldn't it be sufficient to change this to
> >
> >     bne,a .Ldisable_fp
> >       andn %l0, %l5, %l0
> >
> >     ?
> >
> >
> > Yes, you are right, this is my inattentiveness, and it is enough just
> > to set
> > an annul flag.
> > If it is needed, I attached modified patch, but if you prefer, you can
> > just add
> > this flag yourself.
>
> I still have problems to understand why this patch is necessary. The
> current code is like this:
>
>          /*
>           * If we are still the floating point unit owner, then reset the
>           * floating point unit owner to NULL, otherwise clear PSR[EF]
> in the
>           * interrupt frame and let the FP disabled system call do the
> floating
>           * point context save/restore.
>           */
>          ld      [%g6 + SPARC_PER_CPU_FP_OWNER_OFFSET], %l7
>          cmp     %l6, %l7
>          bne     .Ldisable_fp
>           andn   %l0, %l5, %l0
>
> ---> No matter what happens, the PSR[FP] is now cleared in %l0.
> ---> In this path we are the FPU owner and direct jump to ...
>
>          ba      .Lthread_dispatch_done
>           st     %g0, [%g6 + SPARC_PER_CPU_FP_OWNER_OFFSET]
>
> .Ldisable_fp:
>          ba      .Lthread_dispatch_done
>           st      %l0, [%fp + ISF_PSR_OFFSET]
> .Lnon_fp_thread_dispatch:
>
>          call    SYM(_Thread_Do_dispatch)
>           mov    %g6, %o0
>
> ---> ... here. From now on %l0 is no longer used and ...
>
> #if SPARC_HAS_FPU == 1
> .Lthread_dispatch_done:
> #endif
>
>          [... %l0 not used ]
>
> ---> ... here it is loaded:
>
>          ldd     [%fp + ISF_PSR_OFFSET], %l0    ! restore psr, PC
>
> --
> Sebastian Huber, embedded brains GmbH
>
> Address : Dornierstr. 4, D-82178 Puchheim, Germany
> Phone   : +49 89 189 47 41-16
> Fax     : +49 89 189 47 41-09
> E-Mail  : sebastian.huber at embedded-brains.de
> PGP     : Public key available on request.
>
> Diese Nachricht ist keine geschäftliche Mitteilung im Sinne des EHUG.
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.rtems.org/pipermail/devel/attachments/20190606/f159e23e/attachment-0002.html>


More information about the devel mailing list