[ORLinux] [PATCH] OpenRISC: Handle r0 with care

[Richard Weinberger]

Depending on the OpenRISC implementation a rough task may able
to change r0 and corrupt other taks.
Handle this case by setting r0 to zero on each entry point.
Also ensure that r0 is really zero before jumping into _start.

Signed-off-by: Richard Weinberger <richard at nod.at>

diff --git a/arch/openrisc/kernel/entry.S b/arch/openrisc/kernel/entry.S
index d5f9c35..7c9c4f6 100644
--- a/arch/openrisc/kernel/entry.S
+++ b/arch/openrisc/kernel/entry.S
@@ -130,6 +130,7 @@ handler: ;\
#define UNHANDLED_EXCEPTION(handler,vector) \
.global handler ;\
handler: ;\
+ l.andi r0,r0,0 ;\
/* r1, EPCR, ESR already saved */ ;\
l.sw PT_GPR2(r1),r2 ;\
l.sw PT_GPR3(r1),r3 ;\
@@ -185,8 +186,8 @@ handler: ;\
/* ---[ 0x100: RESET exception ]----------------------------------------- */

EXCEPTION_ENTRY(_tng_kernel_start)
+ l.andi r0,r0,0
l.jal _start
- l.andi r0,r0,0

/* ---[ 0x200: BUS exception ]------------------------------------------- */

@@ -976,6 +977,7 @@ ENTRY(_kernel_thread_helper)

.align 0x400
ENTRY(_switch)
+ l.andi r0,r0,0
/* We don't store SR as _switch only gets called in a context where
* the SR will be the same going in and coming out... */

diff --git a/arch/openrisc/kernel/head.S b/arch/openrisc/kernel/head.S
index c75018d..c439324 100644
--- a/arch/openrisc/kernel/head.S
+++ b/arch/openrisc/kernel/head.S
@@ -152,6 +152,7 @@
*/

#define EXCEPTION_HANDLE(handler) \
+ l.andi r0,r0,0 ;\
EXCEPTION_T_STORE_GPR30 ;\
l.mfspr r30,r0,SPR_ESR_BASE ;\
l.andi r30,r30,SPR_SR_SM ;\

[Stefan Kristiansson]

Post by Richard Weinberger
@@ -185,8 +186,8 @@ handler: ;\
/* ---[ 0x100: RESET exception ]----------------------------------------- */
EXCEPTION_ENTRY(_tng_kernel_start)
+ l.andi r0,r0,0
l.jal _start
- l.andi r0,r0,0
/* ---[ 0x200: BUS exception ]------------------------------------------- */

The OpenRISC architecture features delay slots, so you probably don't want
to do that.

Stefan

[Jonas Bonn]

Hi Richard,

Post by Richard Weinberger
Depending on the OpenRISC implementation a rough task may able
to change r0 and corrupt other taks.
Handle this case by setting r0 to zero on each entry point.
Also ensure that r0 is really zero before jumping into _start.
Signed-off-by: Richard Weinberger <richard at nod.at>

Given the difficulty that was expressed on IRC to understand that this
was a real problem, I think a longer explanation is in order here. In
particular, the "hardware" people should be able to read this and get a
feeling for the implications of having a writable r0.

Post by Richard Weinberger
diff --git a/arch/openrisc/kernel/entry.S b/arch/openrisc/kernel/entry.S
index d5f9c35..7c9c4f6 100644
--- a/arch/openrisc/kernel/entry.S
+++ b/arch/openrisc/kernel/entry.S
@@ -130,6 +130,7 @@ handler: ;\
#define UNHANDLED_EXCEPTION(handler,vector) \
.global handler ;\
handler: ;\
+ l.andi r0,r0,0 ;\
/* r1, EPCR, ESR already saved */ ;\
l.sw PT_GPR2(r1),r2 ;\
l.sw PT_GPR3(r1),r3 ;\
@@ -185,8 +186,8 @@ handler: ;\
/* ---[ 0x100: RESET exception ]----------------------------------------- */

If you're clearing r0 in EXCEPTION_HANDLE in head.S, then you probably
don't need to clear it again here... this should be in the same
execution path, I think.

Post by Richard Weinberger
EXCEPTION_ENTRY(_tng_kernel_start)
+ l.andi r0,r0,0
l.jal _start
- l.andi r0,r0,0

No, that was already correct. The delay slot (indented one space for
clarity) is executed before the jump instruction.

Post by Richard Weinberger
/* ---[ 0x200: BUS exception ]------------------------------------------- */
@@ -976,6 +977,7 @@ ENTRY(_kernel_thread_helper)
.align 0x400
ENTRY(_switch)
+ l.andi r0,r0,0
/* We don't store SR as _switch only gets called in a context where
* the SR will be the same going in and coming out... */

I'm scratching my head a bit on this one... why do we need to clear r0
here?

Post by Richard Weinberger
diff --git a/arch/openrisc/kernel/head.S b/arch/openrisc/kernel/head.S
index c75018d..c439324 100644
--- a/arch/openrisc/kernel/head.S
+++ b/arch/openrisc/kernel/head.S
@@ -152,6 +152,7 @@
*/
#define EXCEPTION_HANDLE(handler) \
+ l.andi r0,r0,0 ;\
EXCEPTION_T_STORE_GPR30 ;\
l.mfspr r30,r0,SPR_ESR_BASE ;\
l.andi r30,r30,SPR_SR_SM ;\

Doing the same thing to UNHANDLED_EXCEPTION in head.S seems to me like
the right to do... it's moot, as it's unhandled, but it would be nice to
have that path be 'correct', too.

Thanks,
Jonas

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: This is a digitally signed message part
URL: <http://lists.openrisc.net/pipermail/linux/attachments/20120306/7ad97103/attachment.pgp>