lkml.org 
[lkml]   [2012]   [Mar]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH] OpenRISC: Handle r0 with care
From
Date
Hi Richard,

On Mon, 2012-03-05 at 22:07 +0100, Richard Weinberger wrote:
> 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@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.

> 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.

>
> 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.

> /* ---[ 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?

> 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

[unhandled content-type:application/pgp-signature]
\
 
 \ /
  Last update: 2012-03-06 07:13    [W:0.035 / U:1.752 seconds]
©2003-2017 Jasper Spaans. hosted at Digital OceanAdvertise on this site