parisc64 kernel and ret1 (gr29) setup

Hi,
I tried calling ptrace() from a 32 bit app on a 64 bit kernel, and
the kernel crashed. sys_ptrace needs a 32 bit wrapper, but that is a seperate issue (I think).

It died at sys_ptrace+0x28, where it tried to use ret1. (ret1 = 00000000000517a1):

0000000000000000 <sys_ptrace>:

   0:   0f c2 12 c1     std  rp,-10(sr0,sp)
   4:   37 de 03 00     ldo 180(sp),sp
   8:   73 c5 3e 51     std r5,-d8(sp)
   c:   37 a5 3f 81     ldo -40(ret1),r5
  10:   73 c4 3e 61     std r4,-d0(sp)
  14:   08 1b 02 44     copy dp,r4
  18:   db 39 0f e0     extrd,s r25,63,32,r25
  1c:   73 c8 3e 31     std r8,-e8(sp)
  20:   73 c6 3e 41     std r6,-e0(sp)
  24:   73 c3 3e 71     std r3,-c8(sp)
  28:   73 b7 3f b1     std r23,-28(ret1)
  2c:   ef 40 2e 28     cmpib,*= 0,r26,748 <.L1098+0x7c>
  30:   34 08 3f ff     ldi -1,r8
  34:   d3 33 1e e8     extrw,s r25,23,24,r19
  38:   2b 60 00 00     addil 0,dp,%r1

38: R_PARISC_DLTIND21L pidhash

At the moment we set up sp with "ldo TASK_SZ_ALGN+64(%r1),%r30" on syscall entry. For 64 bit presumably we should do something like:

	ldo     TASK_SZ_ALGN+80(%r1),%r30
	ldo	-16(%r30),%r29

Which gives 64 bytes for parameter saves, plus 16 bytes for rp+sp, and initialises ret1.

Does that sound right?

Richard

does the 64-bit ABI relax the requirement for the stack to be 64-byte aligned? if not, it should be ldo TASK_SZ_ALIGN+128(%r1), %r30

Revolutions do not require corporate support.

Says "16 byte aligned", not 64.

Richard

So, I still havn't fixed this, because I added a syscall wrapper for sys_ptrace() and that masked the problem. The wrapper didn't try to use the incoming r29, and it initialised r29 before calling sys_ptrace().

64 bit functions expect r29 to point to a parameter save area. I've made changes in my tree to increase FRAME_SIZE from 64 to 128 bytes on 64 bit, and initialise r29 on syscall entry. I havn't committed it yet. I was just going to increase to 80 bytes - 64 bytes save area plus 16 for rp and sp - but Willy thought there was a requirement for the stack to be 64 byte aligned.
Presumably r29 needs initialising on every call from entry.S and syscall.S to C code, but I'm not over confident about that, so I thought I'd let others see my diff so far. Comments?

Richard

Index: arch/parisc/kernel/syscall.S
===================================================================
RCS file: /home/cvs/parisc/linux/arch/parisc/kernel/syscall.S,v retrieving revision 1.49

diff -u -r1.49 syscall.S
@@ -23,7 +23,13 @@
 	.level		1.1
 #endif
 	.text
-	
+
+#ifdef __LP64__
+#define FRAME_SIZE	128
+#else
+#define FRAME_SIZE	64
+#endif
+
 	.import syscall_exit,code
 	.import syscall_exit_rfi,code
 	.export linux_gateway_page

@@ -94,11 +100,12 @@
 	STREG	%r27, TASK_PT_SAR(%r1)
 
 	loadgp
-
-	ldo	TASK_SZ_ALGN+64(%r1),%r30	/* set up kernel stack */
 
-#ifndef __LP64__
-	/* no need to save these on stack because in wide mode the first 8
+	ldo	TASK_SZ_ALGN+FRAME_SIZE(%r1),%r30	/* set up kernel stack */
+#ifdef __LP64__
+	ldo	-16(%r30),%r29			/* Reference param save area */
+#else
+	/* no need to save these on stack in wide mode because the first 8
 	 * args are passed in registers */
 	stw     %r22, -52(%r30)                 /* 5th argument */
 	stw     %r21, -56(%r30)                 /* 6th argument */

@@ -170,7 +177,7 @@
 	 * C bit set, a non-straced syscall entry results in C and D clear
 	 * in the saved PSW.
 	 */
-	ldo     -TASK_SZ_ALGN-64(%r30),%r1      /* get task ptr */
+	ldo     -TASK_SZ_ALGN-FRAME_SIZE(%r30),%r1      /* get task ptr */
 	ssm	0,%r2
 	STREG	%r2,TASK_PT_PSW(%r1)		/* Lower 8 bits only!! */
 	STREG	%r1,TASK_PT_CR30(%r1)

@@ -224,7 +231,7 @@
 	LDIL_FIXUP(%r1)
 	ldo     R%sys_call_table(%r1), %r19
 
-	ldo     -TASK_SZ_ALGN-64(%r30),%r1      /* get task ptr */
+	ldo     -TASK_SZ_ALGN-FRAME_SIZE(%r30),%r1      /* get task ptr */
 	LDREG   TASK_PT_GR20(%r1), %r20
 	LDREG   TASK_PT_GR26(%r1), %r26		/* Restore the users args */
 	LDREG   TASK_PT_GR25(%r1), %r25

@@ -260,10 +267,10 @@
 	makes a direct call to syscall_trace. */
 	
 tracesys_exit:
-	ldo     -TASK_SZ_ALGN-64(%r30),%r1      /* get task ptr */
+	ldo     -TASK_SZ_ALGN-FRAME_SIZE(%r30),%r1      /* get task ptr */
 	bl	syscall_trace, %r2
 	STREG   %r28,TASK_PT_GR28(%r1)          /* save return value now */
-	ldo     -TASK_SZ_ALGN-64(%r30),%r1      /* get task ptr */
+	ldo     -TASK_SZ_ALGN-FRAME_SIZE(%r30),%r1      /* get task ptr */
 	LDREG   TASK_PT_GR28(%r1), %r28		/* Restore return val. */
 
 	ldil	L%syscall_exit,%r1

@@ -278,7 +285,7 @@
 	ldo	R%tracesys_sigexit(%r2),%r2
 
 tracesys_sigexit:
-	ldo     -TASK_SZ_ALGN-64(%r30),%r1      /* get task ptr */
+	ldo     -TASK_SZ_ALGN-FRAME_SIZE(%r30),%r1      /* get task ptr */
 	bl	syscall_trace, %r2
 	nop
 

Index: arch/parisc/kernel/entry.S
===================================================================
RCS file: /home/cvs/parisc/linux/arch/parisc/kernel/entry.S,v retrieving revision 1.58

diff -u -r1.58 entry.S
@@ -48,7 +48,7 @@
 #include <asm/signal.h>
 
 #ifdef __LP64__
-#define FRAME_SIZE	64
+#define FRAME_SIZE	128
 #else
 #define FRAME_SIZE	64
 #endif

@@ -1787,7 +1787,7 @@
 
 	/* Set the return value for the child */
 child_return:
-	LDREG	TASK_PT_GR19-TASK_SZ_ALGN-128(%r30),%r2
+	LDREG	TASK_PT_GR19-TASK_SZ_ALGN-FRAME_SIZE-FRAME_SIZE(%r30),%r2
 	b	wrapper_exit
 	copy	%r0,%r28
 

Yup.

Yes, that too.

Is there any code elsewhere that "knows" the stack frame is 64 bytes (except for gdb and signal handler frames)?

Alan

Linuxcare.  Support for the Revolution.

i'm not sure that's the point... knowing the stack frame is 64 byte aligned allows the compiler to know the alignment of all types being placed on the stack.

Revolutions do not require corporate support.

I don't know of any; the kernel still boots ok with my changes, so it looks promising. I'll play a bit more before I commit anything.

Thanks,
Richard

So, I committed that diff, and have been looking at what happens w.r.t. stack frame setup on interrupts. All we seem to do is to set sp to the top of task_struct (if in a user context), or move sp up by a struct pt_regs if in kernel space. In both cases, those values are rounded up (TASK_SZ_ALGN and PT_SZ_ALIGN), so there would probably be some space below sp, but should get_stack be explicity allocating a stack frame really?

Richard

This is a bit interesting. A called procedure can write into certain parts of the caller's stack frame, so we should certainly be allocating space for these areas.

A 64 bit stack frame looks like:

	|----------------------------------|
psp ->	|       Register Spill Area        | variable size
	|----------------------------------|
	|       Local Storage Area         | variable size
	|----------------------------------|
	|      Dynamic Storage Area        | variable size
	|----------------------------------|
	| Outgoing Register Parameter Area | 64 bytes
        | arg word 0                       |
        | arg word 1                       |
        | arg word 2                       |
        | arg word 3                       |
	|----------------------------------|
ap ->	|  Outgoing Stack Parameter Area   | variable size
        | arg word 4                       |
        | .                                |
        | arg word n-1                     |
	|----------------------------------|
	|          Frame Marker            | 16 bytes
	| rp                               |
	| psp                              |
        |----------------------------------|

sp ->

Any of the variable size areas may be zero size. ap (%r29) points to first byte of outgoing stack param area, and sp (%r30) points to first byte of free stack area. Stack grows up to increasing memory addresses. Actual arg locations are as shown for 64 bit and smaller args. The mapping from args to arg words gets a little tricky with larger argument sizes.

The areas written by the called function are the frame marker (rp save), and the outgoing register parameter area. This means we ought to be allocating 80 bytes past the end of struct pt_regs, although it's likely the register parameter area is never written.

32-bit stack frames look like:

	|----------------------------------|
psp ->	|       Register Spill Area        | variable size
	|----------------------------------|
	|       Local Storage Area         | variable size
	|----------------------------------|
	|      Dynamic Storage Area        | variable size
	|----------------------------------|
	|  Outgoing Stack Parameter Area   | variable size
        | arg word n-1                     |
        | .                                |
        | arg word 4                       |
	|----------------------------------|
	| Outgoing Register Parameter Area | 32 bytes
        | arg word 3                       |
        | arg word 2                       |
        | arg word 1                       |
        | arg word 0                       |
	|----------------------------------|
	|          Frame Marker            | 32 bytes
	| External Data Pointer (DP)       |
	| External sr4                     |
	| External/stub RP (RP')           |
	| Current RP                       |
	| Static Link                      |
	| Clean up                         |
	| Calling Stub RP (RP'')           |
	| Previous SP                      |
        |----------------------------------|

sp ->

Again, it is the frame marker and outgoing register param area that may be written by the called function, so here we should allocate 64 bytes past the end of struct pt_regs.

Alan Modra

Linuxcare.  Support for the Revolution.

We now allocate 64 bytes for 32 bit kernels and 128 bytes for 64 bit kernels. In fact, (as jsm pointed out to me) the align macro in arch/parisc/tools/offset.c allowed for a stack frame in its rounding up already. It used to always allow only 64 bytes, but I've changed that to allow 64 or 128 bytes as required.

Richard