[U-Boot] p4080ds starts OS with MSR[EE]=1 and DEC armed?

[U-Boot] p4080ds starts OS with MSR[EE]=1 and DEC armed?

[Jimi Xenidis]

I was surprised to find myself at the decrement interrupt when running my new stuff.
That is against ePAPR, right?
Does u-boot at least make sure that the DEC is set to some large value before it leaps at me?
I don't mind forcing EE=0 but I'd like to make sure I make it that far into the code :)
-JX

[U-Boot] p4080ds starts OS with MSR[EE]=1 and DEC armed?

[Kumar Gala]

On Mar 3, 2011, at 8:59 AM, Jimi Xenidis wrote:

> I was surprised to find myself at the decrement interrupt when running my new stuff.
> That is against ePAPR, right?
> Does u-boot at least make sure that the DEC is set to some large value before it leaps at me?
> I don't mind forcing EE=0 but I'd like to make sure I make it that far into the code :)
> -JX

Jimi,

Not sure how or why you are seeing this, but u-boot should disable interrupts in common/cmd_bootm.c

Look for disable_interrupts() -> this should set MSR[EE] = 0.

- k

[U-Boot] p4080ds starts OS with MSR[EE]=1 and DEC armed?

[Jimi Xenidis]

On Mar 12, 2011, at 4:56 PM, Kumar Gala wrote:
> 
> 
> On Mar 3, 2011, at 8:59 AM, Jimi Xenidis wrote:
> 
>> I was surprised to find myself at the decrement interrupt when running my new stuff.
>> That is against ePAPR, right?
>> Does u-boot at least make sure that the DEC is set to some large value before it leaps at me?
>> I don't mind forcing EE=0 but I'd like to make sure I make it that far into the code :)
>> -JX
> 
> Jimi,
> 
> Not sure how or why you are seeing this, but u-boot should disable interrupts in common/cmd_bootm.c
> 
> Look for disable_interrupts() -> this should set MSR[EE] = 0.

Sorry, this is my brain fart.
I was running my unit tests (on SimiCS p4080ds) and when I set MSR[EE]=1 to get a doorbell, I got a DEC (as one should according to interrupt order).
Turns out that TSR[DIS] was on when my program took over, and this is ePAPR legal.
I need to reset the DEC in my code.

Sorry for the confusion.
-JX

> 
> - k
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] p4080ds starts OS with MSR[EE]=1 and DEC armed?

[Timur Tabi]

On Sun, Mar 13, 2011 at 10:46 AM, Jimi Xenidis <jimix@watson.ibm.com> wrote:

> Turns out that TSR[DIS] was on when my program took over, and this is ePAPR legal.
> I need to reset the DEC in my code.

I wonder if it should be legal.  What value does it have to leave a
pending decrementer interrupt when booting the OS?  That just forces
every OS to program the decrementer and clear DIS before enabling
interrupts.

-- 
Timur Tabi
Linux kernel developer at Freescale

[U-Boot] p4080ds starts OS with MSR[EE]=1 and DEC armed?

[Scott Wood]

On Tue, 15 Mar 2011 14:21:03 -0500
Timur Tabi <timur@freescale.com> wrote:

> On Sun, Mar 13, 2011 at 10:46 AM, Jimi Xenidis <jimix@watson.ibm.com> wrote:
> 
> > Turns out that TSR[DIS] was on when my program took over, and this is ePAPR legal.
> > I need to reset the DEC in my code.
> 
> I wonder if it should be legal.  What value does it have to leave a
> pending decrementer interrupt when booting the OS?  That just forces
> every OS to program the decrementer and clear DIS before enabling
> interrupts.

The OS should ensure that timer interrupts are functioning the way it wants
before it enables interrupts -- but in any case, once it enables interrupts
it should be able to handle any interrupt that is enabled.  There's no
general need to clear DIS.

There's actually no good way for boot software to ensure that TSR[DIS] is
clear, other than stopping the timebase (not allowed, or at least shouldn't
be), or assuming that the OS will take control of the decrementer within a
reasonable amount of time.  We could require that TCR[DIE] be clear, but
that just imposes an extra requirement on boot software for no good reason
-- and in general, we want to minimize the extent to which we trust boot
software to do something specific.  Especially if it's something that won't
show up as a problem with most client software, until someone decides to
rely on it.

-Scot

[U-Boot] p4080ds starts OS with MSR[EE]=1 and DEC armed?

[Jimi Xenidis]

On Mar 15, 2011, at 3:24 PM, Scott Wood wrote:
> 
> On Tue, 15 Mar 2011 14:21:03 -0500
> Timur Tabi <timur@freescale.com> wrote:
> 
>> On Sun, Mar 13, 2011 at 10:46 AM, Jimi Xenidis <jimix@watson.ibm.com> wrote:
>> 
>>> Turns out that TSR[DIS] was on when my program took over, and this is ePAPR legal.
>>> I need to reset the DEC in my code.
>> 
>> I wonder if it should be legal.  What value does it have to leave a
>> pending decrementer interrupt when booting the OS?  That just forces
>> every OS to program the decrementer and clear DIS before enabling
>> interrupts.
> 
> The OS should ensure that timer interrupts are functioning the way it wants
> before it enables interrupts -- but in any case, once it enables interrupts
> it should be able to handle any interrupt that is enabled.  There's no
> general need to clear DIS.

I guess I could go either way on this.

> 
> There's actually no good way for boot software to ensure that TSR[DIS] is
> clear, other than stopping the timebase (not allowed, or at least shouldn't
> be), or assuming that the OS will take control of the decrementer within a
> reasonable amount of time.  We could require that TCR[DIE] be clear, but
> that just imposes an extra requirement on boot software for no good reason

That is true for "server" and "classic" because they do not necessarily stop at 0.
for embedded you can just:
	/* u-boot leaves decrementer enabled */
	mtspr(SPRN_DECAR, 0); /* paranoia */
	mtspr(SPRN_DEC, 0);
	mtspr(SPRN_TCR, 0);
	mtspr(SPRN_TSR, 0);

And there is no way in hell any timer will go off, right?
-JX

[U-Boot] p4080ds starts OS with MSR[EE]=1 and DEC armed?

[Timur Tabi]

Jimi Xenidis wrote:
> That is true for "server" and "classic" because they do not necessarily stop at 0.
> for embedded you can just:
> 	/* u-boot leaves decrementer enabled */
> 	mtspr(SPRN_DECAR, 0); /* paranoia */
> 	mtspr(SPRN_DEC, 0);
> 	mtspr(SPRN_TCR, 0);
> 	mtspr(SPRN_TSR, 0);

I think you mean

	mtspr(SPRN_TSR, 0xFFFFFFFF);

since it's write-1-to-clear.  Other than that, this is exactly what I was
talking about.  I just think it makes sense for the boot loader to try to
disable as many interrupt sources as possible, and force the OS to enable the
ones it wants.

On the other hand, Scott's points are completely valid.

-- 
Timur Tabi
Linux kernel developer at Freescale

[U-Boot] p4080ds starts OS with MSR[EE]=1 and DEC armed?

[Jimi Xenidis]

On Mar 15, 2011, at 4:21 PM, Timur Tabi wrote:
> 
> Jimi Xenidis wrote:
>> That is true for "server" and "classic" because they do not necessarily stop at 0.
>> for embedded you can just:
>> 	/* u-boot leaves decrementer enabled */
>> 	mtspr(SPRN_DECAR, 0); /* paranoia */
>> 	mtspr(SPRN_DEC, 0);
>> 	mtspr(SPRN_TCR, 0);
>> 	mtspr(SPRN_TSR, 0);
> 
> I think you mean
> 
> 	mtspr(SPRN_TSR, 0xFFFFFFFF);
> 

Ahh yes.. thank you :)
-JX

> since it's write-1-to-clear.  Other than that, this is exactly what I was
> talking about.  I just think it makes sense for the boot loader to try to
> disable as many interrupt sources as possible, and force the OS to enable the
> ones it wants.
> 
> On the other hand, Scott's points are completely valid.

[U-Boot] p4080ds starts OS with MSR[EE]=1 and DEC armed?

[Scott Wood]

On Tue, 15 Mar 2011 16:16:39 -0500
Jimi Xenidis <jimix@pobox.com> wrote:

> That is true for "server" and "classic" because they do not necessarily stop at 0.
> for embedded you can just:
> 	/* u-boot leaves decrementer enabled */
> 	mtspr(SPRN_DECAR, 0); /* paranoia */
> 	mtspr(SPRN_DEC, 0);
> 	mtspr(SPRN_TCR, 0);
> 	mtspr(SPRN_TSR, 0);
> 
> And there is no way in hell any timer will go off, right?

Ah, right.  Forgot it stopped at zero.

-Scott

[U-Boot] p4080ds starts OS with MSR[EE]=1 and DEC armed?

[Scott Wood]

On Tue, 15 Mar 2011 16:22:50 -0500
Scott Wood <scottwood@freescale.com> wrote:

> On Tue, 15 Mar 2011 16:16:39 -0500
> Jimi Xenidis <jimix@pobox.com> wrote:
> 
> > That is true for "server" and "classic" because they do not necessarily stop at 0.
> > for embedded you can just:
> > 	/* u-boot leaves decrementer enabled */
> > 	mtspr(SPRN_DECAR, 0); /* paranoia */
> > 	mtspr(SPRN_DEC, 0);
> > 	mtspr(SPRN_TCR, 0);
> > 	mtspr(SPRN_TSR, 0);
> > 
> > And there is no way in hell any timer will go off, right?
> 
> Ah, right.  Forgot it stopped at zero.

Though that last line should be something like mtspr(SPRN_TSR, 0xffffffff);

-Scott

[U-Boot] p4080ds starts OS with MSR[EE]=1 and DEC armed?

[Scott Wood]

On Sat, 12 Mar 2011 16:56:09 -0600
Kumar Gala <galak@kernel.crashing.org> wrote:

> 
> On Mar 3, 2011, at 8:59 AM, Jimi Xenidis wrote:
> 
> > I was surprised to find myself at the decrement interrupt when running my new stuff.
> > That is against ePAPR, right?
> > Does u-boot at least make sure that the DEC is set to some large value before it leaps at me?
> > I don't mind forcing EE=0 but I'd like to make sure I make it that far into the code :)
> > -JX
> 
> Jimi,
> 
> Not sure how or why you are seeing this, but u-boot should disable interrupts in common/cmd_bootm.c
> 
> Look for disable_interrupts() -> this should set MSR[EE] = 0.

What about the rest of MSR -- ME/CE/DE?

-Scott

[U-Boot] p4080ds starts OS with MSR[EE]=1 and DEC armed?

[Jimi Xenidis]

On Mar 14, 2011, at 1:09 PM, Scott Wood wrote:
> 
> On Sat, 12 Mar 2011 16:56:09 -0600
> Kumar Gala <galak@kernel.crashing.org> wrote:
> 
>> 
>> On Mar 3, 2011, at 8:59 AM, Jimi Xenidis wrote:
>> 
>>> I was surprised to find myself at the decrement interrupt when running my new stuff.
>>> That is against ePAPR, right?
>>> Does u-boot at least make sure that the DEC is set to some large value before it leaps at me?
>>> I don't mind forcing EE=0 but I'd like to make sure I make it that far into the code :)
>>> -JX
>> 
>> Jimi,
>> 
>> Not sure how or why you are seeing this, but u-boot should disable interrupts in common/cmd_bootm.c
>> 
>> Look for disable_interrupts() -> this should set MSR[EE] = 0.
> 
> What about the rest of MSR -- ME/CE/DE?

I can attest that MSR=0 when my program takes over.
BTW: secondary CPUs have MSR[IS|DS] == 0b11

-JX

> 
> -Scott

[U-Boot] p4080ds starts OS with MSR[EE]=1 and DEC armed?

[Kumar Gala]

On Mar 14, 2011, at 2:39 PM, Jimi Xenidis wrote:

> On Mar 14, 2011, at 1:09 PM, Scott Wood wrote:
>> 
>> On Sat, 12 Mar 2011 16:56:09 -0600
>> Kumar Gala <galak@kernel.crashing.org> wrote:
>> 
>>> 
>>> On Mar 3, 2011, at 8:59 AM, Jimi Xenidis wrote:
>>> 
>>>> I was surprised to find myself at the decrement interrupt when running my new stuff.
>>>> That is against ePAPR, right?
>>>> Does u-boot at least make sure that the DEC is set to some large value before it leaps at me?
>>>> I don't mind forcing EE=0 but I'd like to make sure I make it that far into the code :)
>>>> -JX
>>> 
>>> Jimi,
>>> 
>>> Not sure how or why you are seeing this, but u-boot should disable interrupts in common/cmd_bootm.c
>>> 
>>> Look for disable_interrupts() -> this should set MSR[EE] = 0.
>> 
>> What about the rest of MSR -- ME/CE/DE?
> 
> I can attest that MSR=0 when my program takes over.
> BTW: secondary CPUs have MSR[IS|DS] == 0b11

The spin in such a mapping, but when released out of spin they should be in a MSR[IS|DS] = 0b00 mapping.

- k

[U-Boot] p4080ds starts OS with MSR[EE]=1 and DEC armed?

[Kumar Gala]

On Mar 14, 2011, at 1:09 PM, Scott Wood wrote:

> On Sat, 12 Mar 2011 16:56:09 -0600
> Kumar Gala <galak@kernel.crashing.org> wrote:
> 
>> 
>> On Mar 3, 2011, at 8:59 AM, Jimi Xenidis wrote:
>> 
>>> I was surprised to find myself at the decrement interrupt when running my new stuff.
>>> That is against ePAPR, right?
>>> Does u-boot at least make sure that the DEC is set to some large value before it leaps at me?
>>> I don't mind forcing EE=0 but I'd like to make sure I make it that far into the code :)
>>> -JX
>> 
>> Jimi,
>> 
>> Not sure how or why you are seeing this, but u-boot should disable interrupts in common/cmd_bootm.c
>> 
>> Look for disable_interrupts() -> this should set MSR[EE] = 0.
> 
> What about the rest of MSR -- ME/CE/DE?

I dont think we ever turn on CE in u-boot, DE would only get set in some weird external debugger build.

- k

[U-Boot] p4080ds starts OS with MSR[EE]=1 and DEC armed?

[Scott Wood]

On Mon, 14 Mar 2011 16:25:54 -0500
Kumar Gala <galak@kernel.crashing.org> wrote:

> 
> On Mar 14, 2011, at 1:09 PM, Scott Wood wrote:
> 
> > On Sat, 12 Mar 2011 16:56:09 -0600
> > Kumar Gala <galak@kernel.crashing.org> wrote:
> > 
> >> 
> >> On Mar 3, 2011, at 8:59 AM, Jimi Xenidis wrote:
> >> 
> >>> I was surprised to find myself at the decrement interrupt when running my new stuff.
> >>> That is against ePAPR, right?
> >>> Does u-boot at least make sure that the DEC is set to some large value before it leaps at me?
> >>> I don't mind forcing EE=0 but I'd like to make sure I make it that far into the code :)
> >>> -JX
> >> 
> >> Jimi,
> >> 
> >> Not sure how or why you are seeing this, but u-boot should disable interrupts in common/cmd_bootm.c
> >> 
> >> Look for disable_interrupts() -> this should set MSR[EE] = 0.
> > 
> > What about the rest of MSR -- ME/CE/DE?
> 
> I dont think we ever turn on CE in u-boot, DE would only get set in some weird external debugger build.

From arch/powerpc/cpu/mpc85xx/start.S:

        /* switch back to AS = 0 */
        lis     r3,(MSR_CE|MSR_ME|MSR_DE)@h
        ori     r3,r3,(MSR_CE|MSR_ME|MSR_DE)@l
        mtmsr   r3
        isync

        bl      cpu_init_f
        bl      board_init_f
        isync

-Scott

[U-Boot] p4080ds starts OS with MSR[EE]=1 and DEC armed?

[Kumar Gala]

On Mar 14, 2011, at 4:37 PM, Scott Wood wrote:

> On Mon, 14 Mar 2011 16:25:54 -0500
> Kumar Gala <galak@kernel.crashing.org> wrote:
> 
>> 
>> On Mar 14, 2011, at 1:09 PM, Scott Wood wrote:
>> 
>>> On Sat, 12 Mar 2011 16:56:09 -0600
>>> Kumar Gala <galak@kernel.crashing.org> wrote:
>>> 
>>>> 
>>>> On Mar 3, 2011, at 8:59 AM, Jimi Xenidis wrote:
>>>> 
>>>>> I was surprised to find myself at the decrement interrupt when running my new stuff.
>>>>> That is against ePAPR, right?
>>>>> Does u-boot at least make sure that the DEC is set to some large value before it leaps at me?
>>>>> I don't mind forcing EE=0 but I'd like to make sure I make it that far into the code :)
>>>>> -JX
>>>> 
>>>> Jimi,
>>>> 
>>>> Not sure how or why you are seeing this, but u-boot should disable interrupts in common/cmd_bootm.c
>>>> 
>>>> Look for disable_interrupts() -> this should set MSR[EE] = 0.
>>> 
>>> What about the rest of MSR -- ME/CE/DE?
>> 
>> I dont think we ever turn on CE in u-boot, DE would only get set in some weird external debugger build.
> 
> From arch/powerpc/cpu/mpc85xx/start.S:
> 
>        /* switch back to AS = 0 */
>        lis     r3,(MSR_CE|MSR_ME|MSR_DE)@h
>        ori     r3,r3,(MSR_CE|MSR_ME|MSR_DE)@l
>        mtmsr   r3
>        isync
> 
>        bl      cpu_init_f
>        bl      board_init_f
>        isync
> 
> -Scott

commit 15fba3279b56333bdb65ead366f82c945ed320d1
Author: Kumar Gala <galak@kernel.crashing.org>
Date:   Fri Sep 11 15:28:41 2009 -0500

    ppc/85xx: Disable all async interrupt sources when we boot
    
    We should make sure to clear MSR[ME, CE, DE] when we boot an OS image
    since we have changed the exception vectors and the OSes vectors might
    not be setup we should avoid async interrupts at all costs.
    
    Signed-off-by: Kumar Gala <galak@kernel.crashing.org>

diff --git a/cpu/mpc85xx/cpu_init.c b/cpu/mpc85xx/cpu_init.c
index 48a82ed..a6d1e99 100644
--- a/cpu/mpc85xx/cpu_init.c
+++ b/cpu/mpc85xx/cpu_init.c
@@ -364,5 +364,16 @@ extern void setup_ivors(void);
 
 void arch_preboot_os(void)
 {
+       u32 msr;
+
+       /*
+        * We are changing interrupt offsets and are about to boot the OS so
+        * we need to make sure we disable all async interrupts. EE is already
+        * disabled by the time we get called.
+        */
+       msr = mfmsr();
+       msr &= ~(MSR_ME|MSR_CE|MSR_DE);
+       mtmsr(msr);
+
        setup_ivors();
 }

-----

:)

- k

[U-Boot] p4080ds starts OS with MSR[EE]=1 and DEC armed?

[Scott Wood]

On Tue, 15 Mar 2011 00:39:17 -0500
Kumar Gala <galak@kernel.crashing.org> wrote:

> diff --git a/cpu/mpc85xx/cpu_init.c b/cpu/mpc85xx/cpu_init.c
> index 48a82ed..a6d1e99 100644
> --- a/cpu/mpc85xx/cpu_init.c
> +++ b/cpu/mpc85xx/cpu_init.c
> @@ -364,5 +364,16 @@ extern void setup_ivors(void);
>  
>  void arch_preboot_os(void)
>  {
> +       u32 msr;
> +
> +       /*
> +        * We are changing interrupt offsets and are about to boot the OS so
> +        * we need to make sure we disable all async interrupts. EE is already
> +        * disabled by the time we get called.
> +        */
> +       msr = mfmsr();
> +       msr &= ~(MSR_ME|MSR_CE|MSR_DE);
> +       mtmsr(msr);
> +
>         setup_ivors();
>  }
> 
> -----
> 
> :)

Ah, thanks.  Not sure why my previous grep found the other one but not
this. :-)

-Scott