Re: kernel panic on ibm4xx-based powerpc box with DDB

On 2016/12/28 5:59, Matt Thomas wrote:
> Why don't use you
>
> 	ACCESS_PROLOG(CI_DDBSAVE)
> 	bla	ddbtrap
>
> and just rid of ddbstk?  since ddbstk/ipkdbstk don't support nested traps.
>
> Ditto for ipkdbtrap.

Thank you very much for your reply. I revised the patch accordingly, and
it passed some stress tests on my OPENBLOCKS266.

However, sorry for bothering you, but I don't understand why this work.
The original DDB/IPKDB handlers use ddbstk/ipkdbstk, that clearly do not
support nested traps, as you pointed out. The patched version uses
CI_{DDB,IPKDB}SAVE, that are save areas in cpu_info. It seems to me that
they also do not support nested traps; a succeeding trap overwrites
save areas already used by a preceding trap, doesn't it? I'm a beginner
of assembler programming, and maybe I misunderstand something...

Thanks,
Rin
====
--- src/sys/arch/powerpc/ibm4xx/trap_subr.S.orig	2016-12-28 07:56:49.376180545 +0900
+++ src/sys/arch/powerpc/ibm4xx/trap_subr.S	2016-12-28 07:58:42.785516679 +0900
@@ -191,20 +191,9 @@
  /*
   * In case of DDB we want a separate trap catcher for it
   */
-	.lcomm	ddbstk,INTSTK,16	/* ddb stack */
-
  	.globl	_C_LABEL(ddblow),_C_LABEL(ddbsize)
  _C_LABEL(ddblow):
-	mtsprg1	%r1			/* save SP */
-	GET_CPUINFO(%r1)
-	stmw	%r28,CI_DDBSAVE(%r1)	/* free r28-r31 */
-	mflr	%r28			/* save LR */
-	mfcr	%r29			/* save CR */
-	mfsrr0	%r30
-	mfsrr1	%r31
-	stmw	%r30,(CI_DDBSAVE+CPUSAVE_SRR0)(%r1)	/* save srr0/srr1 */
-	lis	%r1,ddbstk+INTSTK-CALLFRAMELEN@ha	/* get new SP */
-	addi	%r1,%r1,ddbstk+INTSTK-CALLFRAMELEN@l
+	ACCESS_PROLOG(CI_DDBSAVE)
  	bla	ddbtrap
  _C_LABEL(ddbsize) = .-_C_LABEL(ddblow)
  #endif	/* DDB || KGDB */
@@ -213,21 +202,9 @@
  /*
   * In case of IPKDB we want a separate trap catcher for it
   */
-
-	.lcomm	ipkdbstk,INTSTK,16	/* ipkdb stack */
-
  	.globl	_C_LABEL(ipkdblow),_C_LABEL(ipkdbsize)
  _C_LABEL(ipkdblow):
-	mtsprg1	%r1			/* save SP */
-	GET_CPUINFO(%r1)
-	stmw	%r28,CI_IPKDBSAVE(%r1)	/* free r28-r31 */
-	mflr	%r28			/* save LR */
-	mfcr	%r29			/* save CR */
-	mfsrr0	%r30
-	mfsrr1	%r31
-	stmw	%r30,(CI_IPKDBSAVE+CPUSAVE_SRR0)(%r1)	/* save srr0/srr1 */
-	lis	%r1,ipkdbstk+INTSTK-CALLFRAMELEN@ha	/* get new SP */
-	addi	%r1,%r1,ipkdbstk+INTSTK-CALLFRAMELEN@l
+	ACCESS_PROLOG(CI_IPKDBSAVE)
  	bla	ipkdbtrap
  _C_LABEL(ipkdbsize) = .-_C_LABEL(ipkdblow)
  #endif	/* IPKDB */