[PATCH] Use exception vectors from ROM instead of copying them to page zero

[BALATON Zoltan]

Instead of copying the exception vectors from ROM to page zero use the
Interrupt Prefix bit of the CPU to use the vectors at the ROM address.
This simplifies the startup routine a little bit and avoids using
addresses that can be overwritten by clients. In particular MorphOS
writes to address 0x80 during boot which corrupted the exception
return code causing a crash.

Signed-off-by: BALATON Zoltan <balaton(a)eik.bme.hu>
---

Index: openbios-devel/arch/ppc/qemu/start.S
===================================================================
--- openbios-devel/arch/ppc/qemu/start.S	(revision 1304)
+++ openbios-devel/arch/ppc/qemu/start.S	(working copy)
@@ -388,7 +388,7 @@ GLOBL(__vectors_end):
 GLOBL(_entry):
 
 #ifdef CONFIG_PPC_64BITSUPPORT
-	li	r0,0
+	li	r0, MSR_IP
 
 	lis	r3, 0x8000			/* r1=0x80000000 */
 	add.	r3,r3,r3			/* r1=r1+r1 (high 32bit !0) */
@@ -404,32 +404,6 @@ no_64bit:
 
 real_entry:
 #endif
-
-	/* copy exception vectors */
-
-	LOAD_REG_IMMEDIATE(r3, __vectors)
-	li	r4,0
-	li	r5,__vectors_end - __vectors + 16
-	rlwinm	r5,r5,0,0,28
-1:	lwz	r6,0(r3)
-	lwz	r7,4(r3)
-	lwz	r8,8(r3)
-	lwz	r9,12(r3)
-	stw	r6,0(r4)
-	stw	r7,4(r4)
-	stw	r8,8(r4)
-	stw	r9,12(r4)
-	dcbst	0,r4
-	sync
-	icbi	0,r4
-	sync
-	addi	r5,r5,-16
-	addi	r3,r3,16
-	addi	r4,r4,16
-	cmpwi	r5,0
-	bgt	1b
-	isync
-
 	bl compute_ramsize
 
 	/* Memory map:
@@ -521,13 +495,13 @@ _GLOBAL(call_elf):
 	LOAD_REG_IMMEDIATE(r5, of_client_callback)	// r5 = callback
 	li	r6,0			// r6 = address of client program arguments (unused)
 	li	r7,0			// r7 = length of client program arguments (unused)
-	li	r0,MSR_FP | MSR_ME | MSR_DR | MSR_IR
+	li	r0,MSR_FP | MSR_ME | MSR_IP | MSR_DR | MSR_IR
 	MTMSRD(r0)
 	blrl
 
 #ifdef CONFIG_PPC64
     /* Restore SF bit */
-    LOAD_REG_IMMEDIATE(r0, MSR_SF | MSR_FP | MSR_ME | MSR_DR | MSR_IR)
+    LOAD_REG_IMMEDIATE(r0, MSR_SF | MSR_FP | MSR_ME | MSR_IP | MSR_DR | MSR_IR)
     MTMSRD(r0)
 #endif
 	LOAD_REG_IMMEDIATE(r8, saved_stack)		// restore stack pointer

[PATCH] Use exception vectors from ROM instead of copying them to page zero

[Mark Cave-Ayland]

On 26/05/14 13:20, BALATON Zoltan wrote:

  Instead of copying the exception vectors from ROM to
page zero use the
 Interrupt Prefix bit of the CPU to use the vectors at the ROM address.
 This simplifies the startup routine a little bit and avoids using
 addresses that can be overwritten by clients. In particular MorphOS
 writes to address 0x80 during boot which corrupted the exception
 return code causing a crash.

 Signed-off-by: BALATON Zoltan <balaton(a)eik.bme.hu>
 ---

 Index: openbios-devel/arch/ppc/qemu/start.S
 ===================================================================
 --- openbios-devel/arch/ppc/qemu/start.S	(revision 1304)
 +++ openbios-devel/arch/ppc/qemu/start.S	(working copy)
 @@ -388,7 +388,7 @@ GLOBL(__vectors_end):
   GLOBL(_entry):

   #ifdef CONFIG_PPC_64BITSUPPORT
 -	li	r0,0
 +	li	r0, MSR_IP

   	lis	r3, 0x8000			/* r1=0x80000000 */
   	add.	r3,r3,r3			/* r1=r1+r1 (high 32bit !0) */
 @@ -404,32 +404,6 @@ no_64bit:

   real_entry:
   #endif
 -
 -	/* copy exception vectors */
 -
 -	LOAD_REG_IMMEDIATE(r3, __vectors)
 -	li	r4,0
 -	li	r5,__vectors_end - __vectors + 16
 -	rlwinm	r5,r5,0,0,28
 -1:	lwz	r6,0(r3)
 -	lwz	r7,4(r3)
 -	lwz	r8,8(r3)
 -	lwz	r9,12(r3)
 -	stw	r6,0(r4)
 -	stw	r7,4(r4)
 -	stw	r8,8(r4)
 -	stw	r9,12(r4)
 -	dcbst	0,r4
 -	sync
 -	icbi	0,r4
 -	sync
 -	addi	r5,r5,-16
 -	addi	r3,r3,16
 -	addi	r4,r4,16
 -	cmpwi	r5,0
 -	bgt	1b
 -	isync
 -
   	bl compute_ramsize

   	/* Memory map:
 @@ -521,13 +495,13 @@ _GLOBAL(call_elf):
   	LOAD_REG_IMMEDIATE(r5, of_client_callback)	// r5 = callback
   	li	r6,0			// r6 = address of client program arguments (unused)
   	li	r7,0			// r7 = length of client program arguments (unused)
 -	li	r0,MSR_FP | MSR_ME | MSR_DR | MSR_IR
 +	li	r0,MSR_FP | MSR_ME | MSR_IP | MSR_DR | MSR_IR
   	MTMSRD(r0)
   	blrl

   #ifdef CONFIG_PPC64
       /* Restore SF bit */
 -    LOAD_REG_IMMEDIATE(r0, MSR_SF | MSR_FP | MSR_ME | MSR_DR | MSR_IR)
 +    LOAD_REG_IMMEDIATE(r0, MSR_SF | MSR_FP | MSR_ME | MSR_IP | MSR_DR | MSR_IR)
       MTMSRD(r0)
   #endif
   	LOAD_REG_IMMEDIATE(r8, saved_stack)		// restore stack pointer
Just as a general comment: I know that some OSs using Sun's PROM expect 
to be able to write to the trap table (which is why it is copied to RAM) 
so you'd need to check this thoroughly. Or, if MSR_IP uses virtual 
addresses you could do some MMU twiddling so that the virtual address 
actually points into a safe area of RAM rather than ROM.

As a starting point, have you tried booting any other OS with this 
change? For my OpenBIOS tests I have a collection of various NetBSD, 
FreeBSD, Linux, Darwin and HelenOS that I use to check for regressions.

This is certainly out of my comfort zone though, so will need a 
review/ack from Alex/Andreas.


ATB,

Mark.

[Segher Boessenkool]

  >Instead of copying the exception vectors from ROM
to page zero use the
 >Interrupt Prefix bit of the CPU to use the vectors at the ROM address.
 >This simplifies the startup routine a little bit and avoids using
 >addresses that can be overwritten by clients. In particular MorphOS
 >writes to address 0x80 during boot which corrupted the exception
 >return code causing a crash.
Many OSes use 0000..00ff for their own purposes.  OpenBIOS should
not use that area for anything, for its own good.

MSR[IP] is optional.  OpenBIOS should not use it if it wants to run
on more than just 970.

  Just as a general comment: I know that some OSs using
Sun's PROM expect 
 to be able to write to the trap table (which is why it is copied to RAM) 
 so you'd need to check this thoroughly. Or, if MSR_IP uses virtual 
 addresses you could do some MMU twiddling so that the virtual address 
 actually points into a safe area of RAM rather than ROM.
All exceptions are taken with translation off ("real mode").

Cheers,


Segher

[Segher Boessenkool]

  MSR[IP] is optional.  OpenBIOS should not use it if it
wants to run
 on more than just 970.
Ugh, how could I forget...  970 does not implement MSR[IP] either.
So how was this patch tested?


Segher

[PATCH] Use exception vectors from ROM instead of copying them to page zero

[Mark Cave-Ayland]

On 26/05/14 17:48, Segher Boessenkool wrote:

 > Instead of
copying the exception vectors from ROM to page zero use the
> Interrupt Prefix bit of the CPU to use the vectors at the ROM address.
> This simplifies the startup routine a little bit and avoids using
> addresses that can be overwritten by clients. In particular MorphOS
> writes to address 0x80 during boot which corrupted the exception
> return code causing a crash.
 Many OSes use 0000..00ff for their own purposes.  OpenBIOS should
 not use that area for anything, for its own good.
Really? That's really useful to know - is this documented anywhere so we 
can find more information about it? While the patch works, like Alex I'm 
slightly nervous as to why it was written this way in the first place. 
Are there any other assumptions that we're not aware of?


ATB,

Mark.

[Segher Boessenkool]

 Many OSes use
0000..00ff for their own purposes.  OpenBIOS should
not use that area for anything, for its own good.
 
 Really?
Yes, really.  I'm sorry.

The elephant in the room is Linux of course (although it kills OF
really early so things might just work).

  That's really useful to know - is this documented
anywhere so we 
 can find more information about it?
0000..00ff is documented as "reserved".  Many people read that as
"reserved for me to abuse" :-/

  While the patch works, like Alex I'm 
 slightly nervous as to why it was written this way in the first place. 
 Are there any other assumptions that we're not aware of?
Don't use anything below 3000 for anything else than exception vectors.
Don't forget that sometimes new exception vectors are added, sometimes
not at multiples of 100 or even 80.

I expect this code was written this way to save some code space; the
big empty holes in the exception table just beg to be filled up.


Segher