Bug 225321 - dtrace/powerpc64: System crash
Summary: dtrace/powerpc64: System crash
Status: New
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: CURRENT
Hardware: powerpc Any
: --- Affects Only Me
Assignee: freebsd-ppc mailing list
URL:
Keywords: patch
Depends on:
Blocks:
 
Reported: 2018-01-19 17:51 UTC by Breno Leitao
Modified: 2018-02-28 17:59 UTC (History)
4 users (show)

See Also:


Attachments
Create a trapstack to avoid tmpstack corruption (3.11 KB, patch)
2018-02-21 21:57 UTC, Breno Leitao
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Breno Leitao 2018-01-19 17:51:02 UTC
When running dtrace function on powerpc64 it crashes:


Dtrace line:
# dtrace -n 'sched:::off-cpu { @[stack(8)] = count(); }'


# Meanwhile you can run some workload as compiling the kernel. You are going to hit:


panic: acquiring blockable sleep lock with spinlock or critical section
held (sleep mutex) vm map (system) @
/root/kernel/freebsd/sys/vm/vm_map.c:4086
cpuid = 2
time = 1516383953
KDB: stack backtrace:
0xc000000042f7c160: at .kdb_backtrace+0x5c
0xc000000042f7c290: at .vpanic+0x1a4
0xc000000042f7c350: at .kassert_panic+0x8c
0xc000000042f7c3f0: at .witness_checkorder+0xf8
0xc000000042f7c4e0: at .__mtx_lock_flags+0xfc
0xc000000042f7c590: at ._vm_map_lock_read+0x34
0xc000000042f7c610: at .vm_map_lookup+0x94
0xc000000042f7c740: at .vm_fault_hold+0x158
0xc000000042f7c950: at .vm_fault+0x9c
0xc000000042f7ca00: at .trap_pfault+0xd8
0xc000000042f7caa0: at .trap+0xec0
0xc000000042f7cc60: at .powerpc_interrupt+0x1e0
0xc000000042f7cd00: kernel DSI read trap @ 0xffffffffffff9b90 by
.dtrace_getpcstack+0x138: srr1=0x8000000000001032
            r1=0xc000000042f7cfb0 cr=0x28202024 xer=0
ctr=0xc000000043290788 r2=0xc0000000432d66e8 sr=0x40000000
0xc000000042f7cfb0: at 0x1dffe3fc
0xc000000042f7d050: at .dtrace_probe+0xd70
0xc000000042f7d300: at .sched_switch+0x530
0xc000000042f7d3b0: at .mi_switch+0x2c4
0xc000000042f7d440: at .critical_exit+0xb4
0xc000000042f7d4c0: at .powerpc_interrupt+0xb4
0xc000000042f7d560: kernel EXI trap by .powerpc_interrupt+0x1f0:                                                                                                                                                   
srr1=0x8000000000009032
            r1=0xc000000042f7d810 cr=0x2000f032 xer=0 ctr=0x1
r2=0x1015e38
0xc000000042f7d810: user DSI write trap @ 0x810a59008 by 0x104a29a8:
srr1=0x800000000000f032
            r1=0xffffffffffff9b80 cr=0x22044028 xer=0 ctr=0x104b357c
r2=0x1071b970 sr=0x42000000
KDB: enter: panic
[ thread pid 8219 tid 100089 ]
Stopped at      .kdb_enter+0x60:        ld      r2, r1, 0x28
Comment 1 Justin Hibbits freebsd_committer 2018-01-19 19:50:02 UTC
This looks like it's trying to read a user address (0xffffffffffff9b90).  This shouldn't happen, so there must be an incorrect check in the dtrace arch code (sys/cddl/dev/dtrace/powerpc/dtrace_isa.c)
Comment 2 Breno Leitao 2018-02-19 12:27:18 UTC
Hi Justin,

We are debugging this one in background, and the problem seems to be a wrong stack  pointer (r1). Mainly after restoring it at sys/powerpc/aim/trap_subr64.S:

ASENTRY_NOPROF(breakpoint)
        ....

dbtrap:
        /* Write the trap vector to SPRG3 by computing LR & 0xff00 */
        mflr    %r1
        andi.   %r1,%r1,0xff00
        mtsprg3 %r1

        ld      %r1,TRAP_TOCBASE(0)             /* get new SP */
        ld      %r1,TOC_REF(tmpstk)(%r1)
        addi    %r1,%r1,(TMPSTKSZ-48)


We have a working patch already, and I would suggest the authors to share it.
Comment 3 Breno Leitao 2018-02-21 00:28:25 UTC
I did some further debugs, and this is what I found now:

1) The very first problem is that we are not able to leave kdb, if we just get in into kdb, and 'c' continue, we get:

db> c   

fatal kernel trap:

   exception       = 0x400 (instruction storage interrupt)
   virtual address = 0x426f6f7420666c60
   srr0            = 0x426f6f7420666c60 (0x426f6f7420666c60)
   srr1            = 0x8000000040001032
   lr              = 0x426f6f7420666c61 (0x426f6f7420666c61)
   curthread       = 0x1441460
          pid = 0, comm = 


We are going to an address that means "boot fla", which is clearly wrong.

Debugging the code, I found exactly what is happening.

On dbtrap->dbleave, we call FRAME_LEAVE(), which does leaves SRR0 = 0x6616r8 and SRR1 = 0x8000000000001032 and then call rfid. SRR0 and SRR1 are sane.

SRR0 is part of the kdb_enter function, as showed:

0x0000000000661658 in kdb_enter (why=0xf68968 "bootflags", msg=0x80 "")

The kdb_enter() function follows:

void
kdb_enter(const char *why, const char *msg)
{

        if (kdb_dbbe != NULL && kdb_active == 0) {
                if (msg != NULL)
                        printf("KDB: enter: %s\n", msg);
                kdb_why = why;
                breakpoint();
                kdb_why = KDB_WHY_UNSET;
        }
}

After the rfid we get into 0xf68968 that executes:

   0x0000000000661668 <+112>:	ld      r0,16(r1)
   0x000000000066166c <+116>:	mtlr    r0
   ...
   0x000000000066167c <+132>:	blr

In this case, lr has 0x426f6f7420666c61, which is clearly wrong and causes the trap.

It appears that r1 does not point to the proper stack, since it contains:

r1 = 0xf68958

0xf68958:	0x6b65726e	0x656c6e61	0x6d650000	0x00000000
0xf68968:	0x626f6f74	0x666c6167	0x73000000	0x00000000
0xf68978:	0x426f6f74	0x20666c61

Anyway, clearly the kdb_enter() has a wrong stack pointer (r1) after calling breakpoint.
Comment 4 Breno Leitao 2018-02-21 15:56:31 UTC
Hi,

I created a patch that I think it makes sense.

I think that the problem we are not using the temporary stack properly, since it is defined on the bottom of the stack, as showed:

  #define TMPSTKSZ        16384           /* 16K temporary stack */
  GLOBAL(tmpstk)
          .space  TMPSTKSZ

  TOC_ENTRY(tmpstk)

So, the tmpstk is pointing to the bottom of the stack (higher address) in the TOC.

Later, in the dbtrap: section, we get tmpstk and increase TMPSTKSZ, which is going to get in a different area.

If the above is correct, we have two solutions:

1) Move the TOC_ENTRY to the TOP of the stack, something as (not validated):

  #define TMPSTKSZ        16384           /* 16K temporary stack */
  GLOBAL(tmpstk)
  TOC_ENTRY(tmpstk)
          .space  TMPSTKSZ


2) Does not increment the tmpstk pointer, since we are on the bottom of the stack already:

 index 7ef41d90ffe..2307ec4307c 100644
 --- a/sys/powerpc/aim/trap_subr64.S
 +++ b/sys/powerpc/aim/trap_subr64.S
 @@ -805,9 +805,9 @@ dbtrap:
         andi.   %r1,%r1,0xff00
         mtsprg3 %r1
 
         ld      %r1,TRAP_TOCBASE(0)             /* get new SP */
         ld      %r1,TOC_REF(tmpstk)(%r1)
 -       addi    %r1,%r1,(TMPSTKSZ-48)
 +       addi    %r1,%r1,-48
 

Does it make any sense?
Comment 5 Breno Leitao 2018-02-21 19:41:10 UTC
Looking further, it and the stack is properly set, i.e, the tmpstk is set at the correct place, and the patch above, although not crashing, is far to be correct.

This is my new discovery:

1) The tmpstk is being used by powerpc_init().
2) Powerpc_init calls kdb_enter(), which calls breakpoint().
3) Breakpoint goes to "dbtrap" label, which reset the stack to tmpstk, overwriting it since the beginning.
4) when dbtrap/breakpoint exits, the stack is a mess, causing this problem.

To prove that, I created a stack just for the trap, and this seems to fix the problem.

diff --git a/sys/powerpc/aim/locore64.S b/sys/powerpc/aim/locore64.S
index f283ba6489b..3dd1ca4092d 100644
--- a/sys/powerpc/aim/locore64.S
+++ b/sys/powerpc/aim/locore64.S
@@ -58,9 +58,13 @@ GLOBAL(__endkernel)
 #define        TMPSTKSZ        16384           /* 16K temporary stack */
 GLOBAL(tmpstk)
        .space  TMPSTKSZ
-
 TOC_ENTRY(tmpstk)
 
+#define TRAPSTKSZ      4096
+GLOBAL(trapstk)
+       .space  TRAPSTKSZ
+TOC_ENTRY(trapstk)
+
 /*
  * Entry point for bootloaders that do not fully implement ELF and start
  * at the beginning of the image (kexec, notably). In its own section so
diff --git a/sys/powerpc/aim/trap_subr64.S b/sys/powerpc/aim/trap_subr64.S
index 7ef41d90ffe..ffae46fa3f6 100644
--- a/sys/powerpc/aim/trap_subr64.S
+++ b/sys/powerpc/aim/trap_subr64.S
@@ -805,9 +805,9 @@ dbtrap:
        andi.   %r1,%r1,0xff00
        mtsprg3 %r1
 
-       ld      %r1,TRAP_TOCBASE(0)             /* get new SP */
-       ld      %r1,TOC_REF(tmpstk)(%r1)
-       addi    %r1,%r1,(TMPSTKSZ-48)
+       GET_TOCBASE(%r1)
+       ld      %r1,TOC_REF(trapstk)(%r1)
+       addi    %r1,%r1,(TRAPSTKSZ-48)
 
        FRAME_SETUP(PC_DBSAVE)
 /* Call C trap code: */
Comment 6 Breno Leitao 2018-02-21 21:57:47 UTC
Created attachment 190876 [details]
Create a trapstack to avoid tmpstack corruption

This is not tested on powerpc32 bits platform yet.