[U-Boot] ARM: interrupt_init before relocation, write fails

[U-Boot] ARM: interrupt_init before relocation, write fails

[Joe Kulikauskas]

Hi,

v2013.10-rc1 (and after) has a patch (
http://lists.denx.de/pipermail/u-boot/2013-June/156298.html) which sets up
the abort stack before relocation. However, what I am seeing:
IRQ_STACK_START_IN
is in flash at that time, so this write fails.

interrupt_init:
FF0A0FA8 e59f3010   LDR       R3,ff0a0fc0 (ff0a0050=IRQ_STACK_START_IN)

Before this patch, abort stack setup was done after relocation, so target
location is in RAM and writeable.

interrupt_init:
9FFB4020 e59f3010   LDR       R3,9ffb4038 (9ffb3054=IRQ_STACK_START_IN)

If I revert that patch, I don't see that problem.

Joe Kulikauskas

[U-Boot] ARM: interrupt_init before relocation, write fails

[Andrew Ruder]

On Wed, Oct 23, 2013 at 11:41:45AM -0600, Joe Kulikauskas wrote:
> If I revert that patch, I don't see that problem.

FWIW, I am working on a PXA270 target, and have had to revert this patch
as well.  I hadn't gotten around to tracking down where and why I was
crashing though so hadn't emailed in a bug report yet.  Now seeing as
there's someone else now seeing it too I thought I would chime in with
a "me too".

Rob: CC'd you since you were the author and might have some insight.
Full email in entirety below.

- Andy

On Wed, Oct 23, 2013 at 11:41:45AM -0600, Joe Kulikauskas wrote:
> v2013.10-rc1 (and after) has a patch (
> http://lists.denx.de/pipermail/u-boot/2013-June/156298.html) which sets up
> the abort stack before relocation. However, what I am seeing:
> IRQ_STACK_START_IN
> is in flash at that time, so this write fails.
> 
> interrupt_init:
> FF0A0FA8 e59f3010   LDR       R3,ff0a0fc0 (ff0a0050=IRQ_STACK_START_IN)
> 
> Before this patch, abort stack setup was done after relocation, so target
> location is in RAM and writeable.
> 
> interrupt_init:
> 9FFB4020 e59f3010   LDR       R3,9ffb4038 (9ffb3054=IRQ_STACK_START_IN)
> 
> If I revert that patch, I don't see that problem.
> 
> Joe Kulikauskas

[U-Boot] ARM: interrupt_init before relocation, write fails

[Rob Herring]

On Thu, Oct 24, 2013 at 4:37 PM, Andrew Ruder <andy@aeruder.net> wrote:
> On Wed, Oct 23, 2013 at 11:41:45AM -0600, Joe Kulikauskas wrote:
>> If I revert that patch, I don't see that problem.
>
> FWIW, I am working on a PXA270 target, and have had to revert this patch
> as well.  I hadn't gotten around to tracking down where and why I was
> crashing though so hadn't emailed in a bug report yet.  Now seeing as
> there's someone else now seeing it too I thought I would chime in with
> a "me too".
>
> Rob: CC'd you since you were the author and might have some insight.
> Full email in entirety below.

Other than doing stack setup in a completely different way by setting
the mode in the CPSR and setting up SP_irq and SP_abt directly, I
don't see an easy fix. So we should revert this change. It works for
me because I run from RAM before relocation.

Albert or Tom, can you please revert commit 0f5141e9c57e96de11642a.

Rob

>
> - Andy
>
> On Wed, Oct 23, 2013 at 11:41:45AM -0600, Joe Kulikauskas wrote:
>> v2013.10-rc1 (and after) has a patch (
>> http://lists.denx.de/pipermail/u-boot/2013-June/156298.html) which sets up
>> the abort stack before relocation. However, what I am seeing:
>> IRQ_STACK_START_IN
>> is in flash at that time, so this write fails.
>>
>> interrupt_init:
>> FF0A0FA8 e59f3010   LDR       R3,ff0a0fc0 (ff0a0050=IRQ_STACK_START_IN)
>>
>> Before this patch, abort stack setup was done after relocation, so target
>> location is in RAM and writeable.
>>
>> interrupt_init:
>> 9FFB4020 e59f3010   LDR       R3,9ffb4038 (9ffb3054=IRQ_STACK_START_IN)
>>
>> If I revert that patch, I don't see that problem.
>>
>> Joe Kulikauskas
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] ARM: interrupt_init before relocation, write fails

[Albert ARIBAUD]

Hi Rob,

On Tue, 5 Nov 2013 10:22:24 -0600, Rob Herring <robherring2@gmail.com>
wrote:

> On Thu, Oct 24, 2013 at 4:37 PM, Andrew Ruder <andy@aeruder.net> wrote:
> > On Wed, Oct 23, 2013 at 11:41:45AM -0600, Joe Kulikauskas wrote:

(putting back more context)

> >> v2013.10-rc1 (and after) has a patch (
> >> http://lists.denx.de/pipermail/u-boot/2013-June/156298.html)
> >> which sets up the abort stack before relocation. However, what
> >> I am seeing: IRQ_STACK_START_IN
> >> is in flash at that time, so this write fails.

Hmm... Which one exactly is "this write"? The instructions below are
reads, not writes.
 
> >> interrupt_init:
> >> FF0A0FA8 e59f3010   LDR       R3,ff0a0fc0
> >> (ff0a0050=IRQ_STACK_START_IN)
> > 
> >> Before this patch, abort stack setup was done after relocation, so
> >> target location is in RAM and writeable.
> > 
> >> interrupt_init:
> >> 9FFB4020 e59f3010   LDR       R3,9ffb4038
> >> (9ffb3054=IRQ_STACK_START_IN)
> > 
> >> If I revert that patch, I don't see that problem.

That problem only happens when there actually is an abort before
relocation, right?

> > FWIW, I am working on a PXA270 target, and have had to revert this patch
> > as well.  I hadn't gotten around to tracking down where and why I was
> > crashing though so hadn't emailed in a bug report yet.  Now seeing as
> > there's someone else now seeing it too I thought I would chime in with
> > a "me too".
> >
> > Rob: CC'd you since you were the author and might have some insight.
> > Full email in entirety below.
> 
> Other than doing stack setup in a completely different way by setting
> the mode in the CPSR and setting up SP_irq and SP_abt directly, I
> don't see an easy fix. So we should revert this change. It works for
> me because I run from RAM before relocation.
>
> Albert or Tom, can you please revert commit 0f5141e9c57e96de11642a.

I'm ok with reverting in the ARM repo (I'll PR to mainline after I get
Atmel and IMX in), as soon as I get a clear understanding of the exact
issue.

> Rob

Amicalement,
-- 
Albert.

[U-Boot] ARM: interrupt_init before relocation, write fails

[Rob Herring]

On 11/05/2013 01:17 PM, Albert ARIBAUD wrote:
> Hi Rob,
> 
> On Tue, 5 Nov 2013 10:22:24 -0600, Rob Herring <robherring2@gmail.com>
> wrote:
> 
>> On Thu, Oct 24, 2013 at 4:37 PM, Andrew Ruder <andy@aeruder.net> wrote:
>>> On Wed, Oct 23, 2013 at 11:41:45AM -0600, Joe Kulikauskas wrote:
> 
> (putting back more context)
> 
>>>> v2013.10-rc1 (and after) has a patch (
>>>> http://lists.denx.de/pipermail/u-boot/2013-June/156298.html)
>>>> which sets up the abort stack before relocation. However, what
>>>> I am seeing: IRQ_STACK_START_IN
>>>> is in flash at that time, so this write fails.
> 
> Hmm... Which one exactly is "this write"? The instructions below are
> reads, not writes.
>  
>>>> interrupt_init:
>>>> FF0A0FA8 e59f3010   LDR       R3,ff0a0fc0
>>>> (ff0a0050=IRQ_STACK_START_IN)
>>>
>>>> Before this patch, abort stack setup was done after relocation, so
>>>> target location is in RAM and writeable.
>>>
>>>> interrupt_init:
>>>> 9FFB4020 e59f3010   LDR       R3,9ffb4038
>>>> (9ffb3054=IRQ_STACK_START_IN)
>>>
>>>> If I revert that patch, I don't see that problem.
> 
> That problem only happens when there actually is an abort before
> relocation, right?

Not as I understand it. The address here is the address of the variable
holding the stack address. So we are trying to write the stack address
to flash which I guess does more that just get ignored.

Rob

> 
>>> FWIW, I am working on a PXA270 target, and have had to revert this patch
>>> as well.  I hadn't gotten around to tracking down where and why I was
>>> crashing though so hadn't emailed in a bug report yet.  Now seeing as
>>> there's someone else now seeing it too I thought I would chime in with
>>> a "me too".
>>>
>>> Rob: CC'd you since you were the author and might have some insight.
>>> Full email in entirety below.
>>
>> Other than doing stack setup in a completely different way by setting
>> the mode in the CPSR and setting up SP_irq and SP_abt directly, I
>> don't see an easy fix. So we should revert this change. It works for
>> me because I run from RAM before relocation.
>>
>> Albert or Tom, can you please revert commit 0f5141e9c57e96de11642a.
> 
> I'm ok with reverting in the ARM repo (I'll PR to mainline after I get
> Atmel and IMX in), as soon as I get a clear understanding of the exact
> issue.
> 
>> Rob
> 
> Amicalement,
> 

[U-Boot] ARM: interrupt_init before relocation, write fails

[Joe Kulikauskas]

Hi,

Rob, yes that's correct.

To Albert's question: the disassembled instruction I had showed
LDR       R3,ff0a0fc0 is load of r3 with address of the variable holding
the stack address;  "this write" is the str which I've now copied in below.
IRQ_STACK_START_IN = gd->irq_sp + 8;
ff0a0fb0: e5992044 ldr r2, [r9, #68] ; 0x44
ff0a0fb4: e2822008 add r2, r2, #8
ff0a0fb8: e5832000 str r2, [r3]


For my target, IIRC the write to flash caused problem in the flash
controller hardware, breaking further instruction fetches.  On other
platforms the write to flash may fail silently.  But the issue is that
interrupt_init() moving into board_init_f (i.e. before relocation)
generally just doesn't work right.

Joe


On Tue, Nov 5, 2013 at 12:46 PM, Rob Herring <robherring2@gmail.com> wrote:

> On 11/05/2013 01:17 PM, Albert ARIBAUD wrote:
> > Hi Rob,
> >
> > On Tue, 5 Nov 2013 10:22:24 -0600, Rob Herring <robherring2@gmail.com>
> > wrote:
> >
> >> On Thu, Oct 24, 2013 at 4:37 PM, Andrew Ruder <andy@aeruder.net> wrote:
> >>> On Wed, Oct 23, 2013 at 11:41:45AM -0600, Joe Kulikauskas wrote:
> >
> > (putting back more context)
> >
> >>>> v2013.10-rc1 (and after) has a patch (
> >>>> http://lists.denx.de/pipermail/u-boot/2013-June/156298.html)
> >>>> which sets up the abort stack before relocation. However, what
> >>>> I am seeing: IRQ_STACK_START_IN
> >>>> is in flash at that time, so this write fails.
> >
> > Hmm... Which one exactly is "this write"? The instructions below are
> > reads, not writes.
> >
> >>>> interrupt_init:
> >>>> FF0A0FA8 e59f3010   LDR       R3,ff0a0fc0
> >>>> (ff0a0050=IRQ_STACK_START_IN)
> >>>
> >>>> Before this patch, abort stack setup was done after relocation, so
> >>>> target location is in RAM and writeable.
> >>>
> >>>> interrupt_init:
> >>>> 9FFB4020 e59f3010   LDR       R3,9ffb4038
> >>>> (9ffb3054=IRQ_STACK_START_IN)
> >>>
> >>>> If I revert that patch, I don't see that problem.
> >
> > That problem only happens when there actually is an abort before
> > relocation, right?
>
> Not as I understand it. The address here is the address of the variable
> holding the stack address. So we are trying to write the stack address
> to flash which I guess does more that just get ignored.
>
> Rob
>
> >
> >>> FWIW, I am working on a PXA270 target, and have had to revert this
> patch
> >>> as well.  I hadn't gotten around to tracking down where and why I was
> >>> crashing though so hadn't emailed in a bug report yet.  Now seeing as
> >>> there's someone else now seeing it too I thought I would chime in with
> >>> a "me too".
> >>>
> >>> Rob: CC'd you since you were the author and might have some insight.
> >>> Full email in entirety below.
> >>
> >> Other than doing stack setup in a completely different way by setting
> >> the mode in the CPSR and setting up SP_irq and SP_abt directly, I
> >> don't see an easy fix. So we should revert this change. It works for
> >> me because I run from RAM before relocation.
> >>
> >> Albert or Tom, can you please revert commit 0f5141e9c57e96de11642a.
> >
> > I'm ok with reverting in the ARM repo (I'll PR to mainline after I get
> > Atmel and IMX in), as soon as I get a clear understanding of the exact
> > issue.
> >
> >> Rob
> >
> > Amicalement,
> >
>
>

[U-Boot] ARM: interrupt_init before relocation, write fails

[Albert ARIBAUD]

Hi Joe,

On Wed, 6 Nov 2013 09:18:53 -0700, Joe Kulikauskas
<jkulikauskas@cardinalpeak.com> wrote:

> Hi,
> 
> Rob, yes that's correct.
> 
> To Albert's question: the disassembled instruction I had showed
> LDR       R3,ff0a0fc0 is load of r3 with address of the variable holding
> the stack address;  "this write" is the str which I've now copied in below.
> IRQ_STACK_START_IN = gd->irq_sp + 8;
> ff0a0fb0: e5992044 ldr r2, [r9, #68] ; 0x44
> ff0a0fb4: e2822008 add r2, r2, #8
> ff0a0fb8: e5832000 str r2, [r3]
> 
> For my target, IIRC the write to flash caused problem in the flash
> controller hardware, breaking further instruction fetches.  On other
> platforms the write to flash may fail silently.  But the issue is that
> interrupt_init() moving into board_init_f (i.e. before relocation)
> generally just doesn't work right.
> 
> Joe

Thanks Rob and Joe for the clarification.

Indeed interrupt_init() cannot execute before relocation as it sets
globals.

Further, interrupt_init() uses globals to communicate with start.S.
This should be changed. I understand that is because the interrupt
handlers actually set the interrupt stack when they are invoked -- and
that is unnecessary; stacks should be set up as soon as their addresses
are known.

I'll revert the patch as soon as I finish getting the current PRs in.

I have a start.S rewrite effort underway whcuh I should post soon.
I'll add to it a change to the way stacks are initialized.

Amicalement,
-- 
Albert.