[PATCH] arch/ppc/qemu: Moved exception handlers from beginning of RAM

[BALATON Zoltan]

Do not put exception handler routines between 0x00-0xff as some OSes
use this area for their own purposes and can corrupt them. (In
particular MorphOS writes to 0x80 during boot). This patch frees up
this area by moving the routines that were there higher.

Signed-off-by: BALATON Zoltan 
---

This replaces my previous attempt (Use exception vectors from ROM
instead of copying them to page zero) as that relied on the MSR[IP]
bit that was found not to be generally available.

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)
@@ -280,21 +280,9 @@ GLOBL(__vectors):
 1:	nop			//
    b	1b

-call_dsi_exception:
-	LOAD_REG_FUNC(r3, dsi_exception)
-	mtctr	r3
-	bctrl
-	b	exception_return
+VECTOR( 0x100, "SRE" ):
+        b       _entry

-call_isi_exception:
-	LOAD_REG_FUNC(r3, isi_exception)
-	mtctr	r3
-	bctrl
-	b	exception_return
-
-exception_return:
-	EXCEPTION_EPILOGUE
-
 trap_error:
    lis	r1, 0x8000			/* r1=0x80000000 */
    add.	r1,r1,r1			/* r1=r1+r1 (high 32bit !0) */
@@ -309,9 +297,6 @@ trap_error:
    mtctr r4
    bctr

-VECTOR( 0x100, "SRE" ):
-        b       _entry
-
 ILLEGAL_VECTOR( 0x200 )

VECTOR( 0x300, "DSI" ):
@@ -373,11 +358,16 @@ VECTOR( 0x2200, "ISI_64" ):

real_dsi:
    EXCEPTION_PREAMBLE
-	b	call_dsi_exception
+	LOAD_REG_FUNC(r3, dsi_exception)
+	b call_exception_handler

real_isi:
    EXCEPTION_PREAMBLE
-	b	call_isi_exception
+	LOAD_REG_FUNC(r3, isi_exception)
+call_exception_handler:
+	mtctr	r3
+	bctrl
+	EXCEPTION_EPILOGUE

GLOBL(__vectors_end):

[PATCH] arch/ppc/qemu: Moved exception handlers from beginning of RAM

[Alexander Graf]

On 26.05.14 23:24, BALATON Zoltan wrote:
> Do not put exception handler routines between 0x00-0xff as some OSes
> use this area for their own purposes and can corrupt them. (In
> particular MorphOS writes to 0x80 during boot). This patch frees up
> this area by moving the routines that were there higher.
>
> Signed-off-by: BALATON Zoltan

So do you happen to have any idea why the code was written the way it 
was written? I don't feel very confident that I grasp why the original 
author did it that way.

Alex
> This replaces my previous attempt (Use exception vectors from ROM
> instead of copying them to page zero) as that relied on the MSR[IP]
> bit that was found not to be generally available.
> 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)
> @@ -280,21 +280,9 @@ GLOBL(__vectors):
>   1:	nop			//
>    b	1b
>
> -call_dsi_exception:
> LOAD_REG_FUNC(r3, dsi_exception)
> mtctr	r3
> bctrl
> b	exception_return
>
> +VECTOR( 0x100, "SRE" ):
>    b       _entry
>
> -call_isi_exception:
> LOAD_REG_FUNC(r3, isi_exception)
> mtctr	r3
> bctrl
> b	exception_return
>
> -exception_return:
> EXCEPTION_EPILOGUE
> trap_error:
>  lis	r1, 0x8000			/* r1=0x80000000 */
>  add.	r1,r1,r1			/* r1=r1+r1 (high 32bit !0) */
>
> @@ -309,9 +297,6 @@ trap_error:
>    mtctr r4
>    bctr
>
> -VECTOR( 0x100, "SRE" ):
>    b       _entry
>
> ILLEGAL_VECTOR( 0x200 )
>
> VECTOR( 0x300, "DSI" ):
>
> @@ -373,11 +358,16 @@ VECTOR( 0x2200, "ISI_64" ):
>
> real_dsi:
>    EXCEPTION_PREAMBLE
> b	call_dsi_exception
>
> LOAD_REG_FUNC(r3, dsi_exception)
>
> b call_exception_handler
>
> real_isi:
> EXCEPTION_PREAMBLE
> b	call_isi_exception
> LOAD_REG_FUNC(r3, isi_exception)
>
> +call_exception_handler:
>
> mtctr	r3
>
> bctrl
>
> EXCEPTION_EPILOGUE
>
> GLOBL(__vectors_end):

[PATCH] arch/ppc/qemu: Moved exception handlers from beginning of RAM

[Mark Cave-Ayland]

On 28/05/14 00:59, Alexander Graf wrote:
> On 26.05.14 23:24, BALATON Zoltan wrote:
> > Do not put exception handler routines between 0x00-0xff as some OSes
> > use this area for their own purposes and can corrupt them. (In
> > particular MorphOS writes to 0x80 during boot). This patch frees up
> > this area by moving the routines that were there higher.
> >
> > Signed-off-by: BALATON Zoltan
>
> So do you happen to have any idea why the code was written the way it
> was written? I don't feel very confident that I grasp why the original
> author did it that way.

+1.

ATB,

Mark.

[PATCH] arch/ppc/qemu: Moved exception handlers from beginning of RAM

[Laurent Vivier]

> Le 28 mai 2014 à 10:02, Mark Cave-Ayland  a
> écrit :
>
> On 28/05/14 00:59, Alexander Graf wrote:
> > On 26.05.14 23:24, BALATON Zoltan wrote:
> > > Do not put exception handler routines between 0x00-0xff as some OSes
> > > use this area for their own purposes and can corrupt them. (In
> > > particular MorphOS writes to 0x80 during boot). This patch frees up
> > > this area by moving the routines that were there higher.
> > >
> > > Signed-off-by: BALATON Zoltan
> >
> > So do you happen to have any idea why the code was written the way it
> > was written? I don't feel very confident that I grasp why the original
> > author did it that way.
>
> +1.

I think arch/ppc/qemu/start.S is coming from Mac-On-Linux. The MOL module is
patching the host exception table to manage the virtual CPU. So I think this is
the reason why it is written as it is.
But Alexander I think you know better than me how MOL is working.

Regards,
Laurent

[PATCH] arch/ppc/qemu: Moved exception handlers from beginning of RAM

[BALATON Zoltan]

On Wed, 28 May 2014, Alexander Graf wrote:
> So do you happen to have any idea why the code was written the way it was 
> written? I don't feel very confident that I grasp why the original author did 
> it that way.

No I don't have any idea why it looks like this but maybe only because 
previous changes wanted to preserve as much as possible (or change as 
little as possible) and they have added up like this. Looking at the 
commit logs I've found the following:

- The exception_return part has been there from the beginning.

- In r945 afaerber added call_[di]si_exception to free up 0x380 and 0x480 
for illegal vectors.

- The real_[di]si jumps has been added in r1043 by agraf probably because 
adding more instructions to the EXCEPTION_PREAMBLE made it not fit in 0x80 
bytes any more.

So I think you should now better as it seems you two are the original 
authors.

Regards,
BALATON Zoltan

[PATCH] arch/ppc/qemu: Moved exception handlers from beginning of RAM

[Andreas Färber]

Am 28.05.2014 12:05, schrieb BALATON Zoltan:
> On Wed, 28 May 2014, Alexander Graf wrote:
> > So do you happen to have any idea why the code was written the way it
> > was written? I don't feel very confident that I grasp why the original
> > author did it that way.
>
> No I don't have any idea why it looks like this but maybe only because
> previous changes wanted to preserve as much as possible (or change as
> little as possible) and they have added up like this. Looking at the
> commit logs I've found the following:
>
> The exception_return part has been there from the beginning.
>
> In r945 afaerber added call_[di]si_exception to free up 0x380 and
>
> 0x480 for illegal vectors.

AFAIR the issue was that the space in the exception table was
insufficient so I needed to branch out somewhere.

Another issue to keep in mind was that depending on QEMU's -cpu,
OpenBIOS testing revealed that sometimes the exception code was called
in place at fffffxxx and sometimes at the relocated addresses, so there
were absolute vs. relative branches to consider. From the code relocated
to 0x0 (or wherever that was) you need to do absolute jumps into the
main OpenBIOS code, which require more instructions.

Regards,
Andreas
> The real_[di]si jumps has been added in r1043 by agraf probably
>
> because adding more instructions to the EXCEPTION_PREAMBLE made it not
> fit in 0x80 bytes any more.
>
> So I think you should now better as it seems you two are the original
> authors.
>
> Regards,
> BALATON Zoltan

[PATCH] arch/ppc/qemu: Moved exception handlers from beginning of RAM

[Alexander Graf]

On 26.05.14 23:24, BALATON Zoltan wrote:
> Do not put exception handler routines between 0x00-0xff as some OSes
> use this area for their own purposes and can corrupt them. (In
> particular MorphOS writes to 0x80 during boot). This patch frees up
> this area by moving the routines that were there higher.
>
> Signed-off-by: BALATON Zoltan

Please split this into 2 patches. One that only moves code from the 
call_xsi_exception into the real_xsi parts of the code. Then if you like 
do another patch on top that changes the calls to use 
call_exception_handler.

The reason I'm asking for this is that there's quite some potential for 
breakage, and I would like to make sure we can easily find it through a 
bisect.

Alex
> This replaces my previous attempt (Use exception vectors from ROM
> instead of copying them to page zero) as that relied on the MSR[IP]
> bit that was found not to be generally available.
> 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)
> @@ -280,21 +280,9 @@ GLOBL(__vectors):
>   1:	nop			//
>    b	1b
>
> -call_dsi_exception:
> LOAD_REG_FUNC(r3, dsi_exception)
> mtctr	r3
> bctrl
> b	exception_return
>
> +VECTOR( 0x100, "SRE" ):
>    b       _entry
>
> -call_isi_exception:
> LOAD_REG_FUNC(r3, isi_exception)
> mtctr	r3
> bctrl
> b	exception_return
>
> -exception_return:
> EXCEPTION_EPILOGUE
> trap_error:
>  lis	r1, 0x8000			/* r1=0x80000000 */
>  add.	r1,r1,r1			/* r1=r1+r1 (high 32bit !0) */
>
> @@ -309,9 +297,6 @@ trap_error:
>    mtctr r4
>    bctr
>
> -VECTOR( 0x100, "SRE" ):
>    b       _entry
>
> ILLEGAL_VECTOR( 0x200 )
>
> VECTOR( 0x300, "DSI" ):
>
> @@ -373,11 +358,16 @@ VECTOR( 0x2200, "ISI_64" ):
>
> real_dsi:
>    EXCEPTION_PREAMBLE
> b	call_dsi_exception
>
> LOAD_REG_FUNC(r3, dsi_exception)
>
> b call_exception_handler
>
> real_isi:
> EXCEPTION_PREAMBLE
> b	call_isi_exception
> LOAD_REG_FUNC(r3, isi_exception)
>
> +call_exception_handler:
>
> mtctr	r3
>
> bctrl
>
> EXCEPTION_EPILOGUE
>
> GLOBL(__vectors_end):