[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[microblaze-uclinux] [patch test] syscall interface cleanup



Hi folks,

Attached are test patches against the kernel and uClibc to fix the 
abomination that is currently the system call entry mechanism.

For a variety of reasons, historical and technical, the system call 
entry interface is currently implemented with a standard branch 
instruction - "bralid r17, 0x8" - from userspace into the kernel.  This 
works, but it's not very clean.

Microblaze has a dedicated "break" instruction - brki - which is much 
preferred over branch, because it is the closest thing we have to a real 
"kernel trap" instruction.  In particular, it sets a bit in the flags 
register (BIP - break in progress) that masks interrupts.  This supports 
a cleaner approach in the kernel system call entry point.

The attached patches migrate the system call entry over to the brki 
instruction.  Any interested parties please try these patches and report 
your experiences back here.  They should apply cleanly to either the 
current CVS or the recent uclinux-dist-test from Snapgear.

The tricky part concerns when and how to apply these patches in the 
uclinux.org CVS repository.  Both patches (uClibc and kernel) are 
required for proper operation, but only the kernel CVS can be updated on 
demand.  This means the uClibc patch would have to be applied manually 
until the new uClinux-dist comes out, and it would, in effect, be 
mandatory for anyone tracking CVS on the kernel.

The other approach would be to get the Snapgear guys to apply the 
patches to the new uClinux-dist, then we apply the kernel patches to CVS 
at the same time as the uClinux-dist at cvs.uclinux.org is updated. 
This way anyone could do a CVS update over both the dist and kernel, and 
it should all pull down cleanly... (everybody synchronise your watches! :)

Finally, it is possible to make this new brki support conditional in the 
kernel with a config option, but it's ugly and seems like a lot of work 
to provide back-compatibility to something so broken.

Any comments or suggestions on this matter, and the patches themselves, 
are welcome...

Regards,

John
diff -Naur -x '.*' -x '*CVS*' -x '*.o' -x '*.a' /opt/src/uClinux-2.4.x/arch/microblaze/kernel/entry.S linux-2.4.x/arch/microblaze/kernel/entry.S
--- /opt/src/uClinux-2.4.x/arch/microblaze/kernel/entry.S	2004-10-14 04:27:47.000000000 +1000
+++ linux-2.4.x/arch/microblaze/kernel/entry.S	2004-10-15 10:11:03.000000000 +1000
@@ -466,9 +466,10 @@
 	nop;			/* Delay slot */		      
 
 /* Instructions to return from a trap */
-/* Note we use rtid to force interrupts back on */
+/* Note we use rtbd to clear BIP bit on way out */
+/* Return offset is 4, to execute instruction after syscall brki insn*/
 #define TRAP_RETURN_INST						      \
-	rtid	r17, 8;		/* r17 used as trap link register */	      \
+	rtbd	r17, 4;		/* r17 used as trap link register */	      \
 	nop;
 
 /* Instructions to return from a debug trap */
@@ -567,7 +568,7 @@
 	/* Note Microblaze barrel shift is optional, so don't rely on it */   \
 	add	r12, r12, r12;			/* convert num -> ptr */      \
 	add	r12, r12, r12;						      \
-	lwi	r12, r12, CSYM(sys_call_table);	/* Get function pointer */		      \
+	lwi	r12, r12, CSYM(sys_call_table);	/* Get function pointer */    \
 	/* Make the system call.  */					      \
 	bra	r12;							      \
 	/* The syscall number is invalid, return an error.  */		      \
@@ -588,28 +589,24 @@
  *
  * System calls are handled here.
  *
- * The stack-pointer (r1) should have already been saved to the memory
- * location ENTRY_SP (the reason for this is that the interrupt vectors may be
- * beyond a 22-bit signed offset jump from the actual interrupt handler, and
- * this allows them to save the stack-pointer and use that register to do an
- * indirect jump).
- *	
  * Syscall protocol:
  *   Syscall number in r12, args in r5-r10
  *   Return value in r3
+ *
+ * Trap entered via brki instruction, so BIP bit is set, and interrupts
+ * are masked.  This is nice, means we don't have to CLI before state save
+ * however we do have to do a special trick (rtbd) to clear BIP bit
+ * we also re-enable BIP bit prior to exit state save
  */
 G_ENTRY(trap):
-	ENTRY_CLI(r11);
 	swi	r1, r0, ENTRY_SP;	// save stack (emulate v850)
 	SAVE_STATE (TRAP, r12, ENTRY_SP) // Save registers. 
-	// No need to enable intrs here because microblaze
-	// traps just implemented as a branch, not a hardware trap
-	ENTRY_EI(r11);
+	rtbd	r0, trap_o;		// Clear BIP bit
+	nop;
+trap_o:					// BIP now cleared, interrupts enabled
 	la	r15, r0, ret_from_trap-8// where the trap should return
 					// need -8 to adjust for rtsd r15, 8
-
 	MAKE_SYS_CALL			// Jump to the syscall function. 
-
 END(trap)
 
 /* This is just like ret_from_trap, but first restores extra registers
@@ -620,7 +617,10 @@
 END(restore_extra_regs_and_ret_from_trap)
 
 /* Entry point used to return from a syscall/trap.  */
+/* We re-enable BIP bit before state restore */
 L_ENTRY(ret_from_trap):
+	brki	r0, ret_from_trap_o;	// Force set BIP bit
+ret_from_trap_o:
 	RETURN(TRAP)
 END(ret_from_trap)
 
diff -Naur -x '.*' -x '*CVS*' -x '*.o' -x '*.a' /opt/src/uClinux-2.4.x/arch/microblaze/kernel/process.c linux-2.4.x/arch/microblaze/kernel/process.c
--- /opt/src/uClinux-2.4.x/arch/microblaze/kernel/process.c	2004-07-08 16:07:01.000000000 +1000
+++ linux-2.4.x/arch/microblaze/kernel/process.c	2004-10-15 10:11:03.000000000 +1000
@@ -135,7 +135,7 @@
 	/* Clone this thread.  */
 	arg0 = flags | CLONE_VM;
 	syscall = __NR_clone;
-	asm volatile ("	bralid r17, 0x8; nop;"
+	asm volatile ("	brki	r17, 0x8;"
 		        : "=r" (ret) 
 			: "r" (syscall), "r" (arg0)
 		        : SYSCALL_CLOBBERS);
@@ -145,7 +145,7 @@
 		/* In child thread, call FN and exit.  */
 		arg0 = (*fn) (arg);
 		syscall = __NR_exit;
-		asm volatile ("bralid r17, 0x8; nop;"
+		asm volatile ("brki	r17, 0x8"
 			        : "=r" (ret) 
 				: "r" (syscall), "r" (arg0)
 			        : SYSCALL_CLOBBERS);
diff -Naur -x '.*' -x '*CVS*' -x '*.o' -x '*.a' /opt/src/uClinux-2.4.x/arch/microblaze/kernel/signal.c linux-2.4.x/arch/microblaze/kernel/signal.c
--- /opt/src/uClinux-2.4.x/arch/microblaze/kernel/signal.c	2004-07-08 16:07:01.000000000 +1000
+++ linux-2.4.x/arch/microblaze/kernel/signal.c	2004-10-15 10:11:03.000000000 +1000
@@ -1,6 +1,7 @@
 /*
- * arch/v850/kernel/signal.c -- Signal handling
+ * arch/microblaze/kernel/signal.c -- Signal handling
  *
+ *  Copyright (C) 2003,2004  John Williams <jwilliams@itee.uq.edu.au>
  *  Copyright (C) 2001  NEC Corporation
  *  Copyright (C) 2001  Miles Bader <miles@gnu.org>
  *  Copyright (C) 1999,2000  Niibe Yutaka & Kaz Kojima
@@ -349,10 +350,8 @@
 		/* addi  r12, r0, __NR_sigreturn  */
 		err |= __put_user(0x31800000 | __NR_sigreturn ,
 				  frame->tramp + 0);
-		/* bralid r17, 0x8 */
-		err |= __put_user(0xba3c0008, frame->tramp + 1);
-		/* nop; */
-		err |= __put_user(0x80000000, frame->tramp + 2);
+		/* brki r17, 0x8 */
+		err |= __put_user(0xba2c0008, frame->tramp + 1);
 
 		regs->gpr[GPR_LP] = (unsigned long)frame->tramp-8;
 
@@ -365,8 +364,8 @@
 	/* Set up registers for signal handler */
 	regs->gpr[GPR_SP] = (unsigned long) frame;
 	regs->gpr[GPR_ARG0] = signal; /* Arg for signal handler */
-	/* Offset of 8 to handle microblaze return wierdness */
-	regs->pc = (unsigned long) ka->sa.sa_handler-8;
+	/* Offset of 4 to handle microblaze rtbd r17, 4 */
+	regs->pc = (unsigned long) ka->sa.sa_handler-4;
 
 	set_fs(USER_DS);
 
@@ -423,15 +422,11 @@
 	if (ka->sa.sa_flags & SA_RESTORER) {
 		regs->gpr[GPR_LP] = (unsigned long) ka->sa.sa_restorer-8;
 	} else {
-		/* Note, these encodings are _big endian_!  */
-
 		/* addi  r12, r0, __NR_sigreturn  */
 		err |= __put_user(0x31800000 | __NR_sigreturn ,
 				  frame->tramp + 0);
-		/* bralid r17, 0x8 */
-		err |= __put_user(0xba3c0008, frame->tramp + 1);
-		/* nop; */
-		err |= __put_user(0x80000000, frame->tramp + 2);
+		/* brki r17, 0x8 */
+		err |= __put_user(0xba2c0008, frame->tramp + 1);
 
 		regs->gpr[GPR_LP] = (unsigned long)frame->tramp-8;
 
@@ -444,8 +439,8 @@
 	/* Set up registers for signal handler */
 	regs->gpr[GPR_SP] = (unsigned long) frame;
 	regs->gpr[GPR_ARG0] = signal; /* Arg for signal handler */
-	/* Offset to handle microblaze return offset wierdness */
-	regs->pc = (unsigned long) ka->sa.sa_handler-8;
+	/* Offset to handle microblaze rtbd r17, 4 */
+	regs->pc = (unsigned long) ka->sa.sa_handler-4;
 
 	set_fs(USER_DS);
 
@@ -486,11 +481,11 @@
 			/* fallthrough */
 			case -ERESTARTNOINTR:
 				regs->gpr[12] = PT_REGS_SYSCALL (regs);
-				/* offset of 12 bytes required = 8 for rtsd
+				/* offset of 8 bytes required = 4 for rtbd
 				   offset, plus 4 for size of 
-					"bralid r17,8"
+					"brki r17,8"
 				   instruction. */
-				regs->pc -= 12; 
+				regs->pc -= 8; 
 		}
 
 		PT_REGS_SET_SYSCALL (regs, 0);
@@ -647,7 +642,7 @@
 		    regs->gpr[GPR_RVAL] == -ERESTARTSYS ||
 		    regs->gpr[GPR_RVAL] == -ERESTARTNOINTR) {
 			regs->gpr[12] = PT_REGS_SYSCALL (regs);
-			regs->pc -= 12; /* 4 + 8 for microblaze return offset wierdness */
+			regs->pc -= 8; /* 4 + 8 for microblaze return offset wierdness */
 		}
 	}
 	return 0;
diff -Naur -x '.*' -x '*CVS*' -x '*.o' -x '*.a' /opt/src/uClinux-2.4.x/include/asm-microblaze/unistd.h linux-2.4.x/include/asm-microblaze/unistd.h
--- /opt/src/uClinux-2.4.x/include/asm-microblaze/unistd.h	2004-07-08 16:08:28.000000000 +1000
+++ linux-2.4.x/include/asm-microblaze/unistd.h	2004-10-15 10:11:03.000000000 +1000
@@ -254,6 +254,15 @@
 #define SYSCALL_ARG5	"r10"
 #define SYSCALL_RET	"r3"
 
+/* Backwards compatability for old dodgy way of doing syscalls with bralid.
+   As soon as possible, all targets must migrate to using brki instruction */
+
+#ifndef CONFIG_MICROBLAZE_BRKI_SYSCALL
+#define MB_SYSCALL "bralid r17, 0x8; nop;\n\t"
+#else
+#define MB_SYSCALL "brki r17, 0x8;\n\t"
+#endif
+
 #define SYSCALL_ENTRY_POINT C_SYMBOL_NAME(syscall_entry)
 
 /*
@@ -279,8 +288,8 @@
 type name (void)							\
 {									\
 	long __ret;							\
-	__asm__ __volatile__ ("bralid	r17, 0x8	\n\t"		\
-			      "addik	r12, r0, %1	\n\t"		\
+	__asm__ __volatile__ ("addik	r12, r0, %1	\n\t"		\
+			      "brki	r17, 0x8	\n\t"		\
 			      "addk	%0, r3, r0	\n\t"		\
 			      : "=r" (__ret)				\
 			      : "i" (__NR_##name)			\
@@ -293,8 +302,8 @@
 {									\
 	long __ret;							\
 	__asm__ __volatile__ ("addk	r5, r0, %2	\n\t"		\
-			      "bralid	r17, 0x8	\n\t"		\
 			      "addik	r12, r0, %1	\n\t"		\
+			      "brki	r17, 0x8	\n\t"		\
 			      "addk	%0, r3, r0	\n\t"		\
 			      : "=r" (__ret)				\
 			      : "i" (__NR_##name),			\
@@ -309,8 +318,8 @@
 	long __ret;							\
 	__asm__ __volatile__ ("addk	r5, r0, %2	\n\t"		\
 			      "addk	r6, r0, %3	\n\t"	   	\
-			      "bralid	r17, 0x8	\n\t"	   	\
 			      "addik	r12, r0, %1	\n\t"	   	\
+			      "brki	r17, 0x8	\n\t"	   	\
 			      "addk	%0, r3, r0	\n\t"	   	\
 			      : "=r" (__ret)			    	\
 			      : "i" (__NR_##name),		      	\
@@ -327,8 +336,8 @@
 	__asm__ __volatile__ ("addk	r5, r0, %2	\n\t"	   	\
 			      "addk	r6, r0, %3	\n\t"	   	\
 			      "addk	r7, r0, %4	\n\t"	   	\
-			      "bralid	r17, 0x8	\n\t"	   	\
 			      "addik	r12, r0, %1	\n\t"	   	\
+			      "brki	r17, 0x8	\n\t"	   	\
 			      "addk	%0, r3, r0	\n\t"	   	\
 			      : "=r" (__ret)			    	\
 			      : "i" (__NR_##name),		      	\
@@ -347,8 +356,8 @@
 			      "addk	r6, r0, %3	\n\t"	   	\
 			      "addk	r7, r0, %4	\n\t"	   	\
 			      "addk	r8, r0, %5	\n\t"	   	\
-			      "bralid	r17, 0x8	\n\t"	   	\
 			      "addik	r12, r0, %1	\n\t"	   	\
+			      "brki	r17, 0x8	\n\t"	   	\
 			      "addk	%0, r3, r0	\n\t"	   	\
 			      : "=r" (__ret)			    	\
 			      : "i" (__NR_##name),		      	\
@@ -369,8 +378,8 @@
 			      "addk	r7, r0, %4	\n\t"	   	\
 			      "addk	r8, r0, %5	\n\t"	   	\
 			      "addk	r9, r0, %6	\n\t"	   	\
-			      "bralid	r17, 0x8	\n\t"	   	\
 			      "addik	r12, r0, %1	\n\t"	   	\
+			      "brki	r17, 0x8	\n\t"	   	\
 			      "addk	%0, r3, r0	\n\t"	   	\
 			      : "=r" (__ret)			    	\
 			      : "i" (__NR_##name),		      	\
@@ -393,8 +402,8 @@
 			      "addk	r8, r0, %5	\n\t"	   	\
 			      "addk	r9, r0, %6	\n\t"	   	\
 			      "addk	r10, r0, %7	\n\t"	   	\
-			      "bralid	r17, 0x8	\n\t"	   	\
 			      "addik	r12, r0, %1	\n\t"	   	\
+			      "brki	r17, 0x8	\n\t"	   	\
 			      "addk	%0, r3, r0	\n\t"	   	\
 			      : "=r" (__ret)			    	\
 			      : "i" (__NR_##name),		      	\
diff -Naur -x '.*' -x '*CVS*' -x '*.o' -x '*.a' /opt/src/uClinux-dist/uClibc/libc/sysdeps/linux/microblaze/clone.c uClibc/libc/sysdeps/linux/microblaze/clone.c
--- /opt/src/uClinux-dist/uClibc/libc/sysdeps/linux/microblaze/clone.c	2004-07-08 16:10:54.000000000 +1000
+++ uClibc/libc/sysdeps/linux/microblaze/clone.c	2004-10-14 09:56:23.000000000 +1000
@@ -31,7 +31,7 @@
       arg0 = flags;
       arg1 = (unsigned long)child_stack;
       syscall = __NR_clone;
-      asm volatile ("bralid r17, 0x08;nop;" 
+      asm volatile ("brki	r17, 0x08;" 
 		    : "=r" (rval), "=r" (syscall)
 		    : "1" (syscall), "r" (arg0), "r" (arg1)
 		    : SYSCALL_CLOBBERS);
@@ -41,7 +41,7 @@
 	{
 	  arg0 = (*fn) (arg);
 	  syscall = __NR_exit;
-	  asm volatile ("bralid r17, 0x08;nop;" 
+	  asm volatile ("brki	r17, 0x08;" 
 			: "=r" (rval), "=r" (syscall)
 			: "1" (syscall), "r" (arg0)
 			: SYSCALL_CLOBBERS);
diff -Naur -x '.*' -x '*CVS*' -x '*.o' -x '*.a' /opt/src/uClinux-dist/uClibc/libc/sysdeps/linux/microblaze/syscall.c uClibc/libc/sysdeps/linux/microblaze/syscall.c
--- /opt/src/uClinux-dist/uClibc/libc/sysdeps/linux/microblaze/syscall.c	2004-07-08 16:10:54.000000000 +1000
+++ uClibc/libc/sysdeps/linux/microblaze/syscall.c	2004-10-14 09:56:42.000000000 +1000
@@ -36,7 +36,7 @@
   register unsigned long ret asm (SYSCALL_RET);
 	unsigned long ret_sav;
 
-  asm ("bralid r17, 0x08; nop;" 
+  asm ("brki	r17, 0x08" 
        : "=r" (ret)
        : "r" (syscall), "r" (a), "r" (b), "r" (c), "r" (d), "r" (e), "r" (f)
        : SYSCALL_CLOBBERS);
diff -Naur -x '.*' -x '*CVS*' -x '*.o' -x '*.a' /opt/src/uClinux-dist/uClibc/libc/sysdeps/linux/microblaze/vfork.S uClibc/libc/sysdeps/linux/microblaze/vfork.S
--- /opt/src/uClinux-dist/uClibc/libc/sysdeps/linux/microblaze/vfork.S	2004-07-08 16:10:54.000000000 +1000
+++ uClibc/libc/sysdeps/linux/microblaze/vfork.S	2004-10-14 09:57:05.000000000 +1000
@@ -29,8 +29,7 @@
 
 C_ENTRY (__vfork):
 	addi	r12, r0, SYS_vfork
-	bralid	r17, 0x08;
-	nop
+	brki	r17, 0x08;
 	addi	r4, r3, 125		// minimum err value
 	blti	r4, 1f			// is r3 < -125?
 	rtsd	r15, 8			// normal return