| Message ID | 1368647776-12940-1-git-send-email-robherring2@gmail.com |
|---|---|
| State | Superseded |
| Delegated to: | Albert ARIBAUD |
| Headers | show |
Hi Rob, On Wed, 15 May 2013 14:56:07 -0500, Rob Herring <robherring2@gmail.com> wrote: > From: Rob Herring <rob.herring@calxeda.com> > > interrupt_init also sets up the abort stack, but is not setup before > relocation. So any aborts during relocation will hang and not print out > any useful information. Fix this by moving the interrupt_init to after > the stack setup in board_init_f. > > Signed-off-by: Rob Herring <rob.herring@calxeda.com> > --- > arch/arm/lib/board.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/arch/arm/lib/board.c b/arch/arm/lib/board.c > index 09ab4ad..6dbe7e2 100644 > --- a/arch/arm/lib/board.c > +++ b/arch/arm/lib/board.c > @@ -447,6 +447,7 @@ void board_init_f(ulong bootflag) > addr_sp += 128; /* leave 32 words for abort-stack */ > gd->irq_sp = addr_sp; > #endif > + interrupt_init(); > > debug("New Stack Pointer is: %08lx\n", addr_sp); > I fail to understand how this is even supposed to work through relocation: exception vectors are not relocated, so if they work before relocation, then they won't work any more afterward unless some code is added to relocate them. Also: if this patch is moving interrupt_init(), then where is the line where a call to interrupt_init() is removed? Amicalement,
On Wed, May 15, 2013 at 3:26 PM, Albert ARIBAUD <albert.u.boot@aribaud.net> wrote: > Hi Rob, > > On Wed, 15 May 2013 14:56:07 -0500, Rob Herring <robherring2@gmail.com> > wrote: > >> From: Rob Herring <rob.herring@calxeda.com> >> >> interrupt_init also sets up the abort stack, but is not setup before >> relocation. So any aborts during relocation will hang and not print out >> any useful information. Fix this by moving the interrupt_init to after >> the stack setup in board_init_f. >> >> Signed-off-by: Rob Herring <rob.herring@calxeda.com> >> --- >> arch/arm/lib/board.c | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/arch/arm/lib/board.c b/arch/arm/lib/board.c >> index 09ab4ad..6dbe7e2 100644 >> --- a/arch/arm/lib/board.c >> +++ b/arch/arm/lib/board.c >> @@ -447,6 +447,7 @@ void board_init_f(ulong bootflag) >> addr_sp += 128; /* leave 32 words for abort-stack */ >> gd->irq_sp = addr_sp; >> #endif >> + interrupt_init(); >> >> debug("New Stack Pointer is: %08lx\n", addr_sp); >> > > I fail to understand how this is even supposed to work through > relocation: exception vectors are not relocated, so if they work before > relocation, then they won't work any more afterward unless some code is > added to relocate them. They work before and after in my testing. The vectors are relocated along with the rest of u-boot and the vector base is updated by c_runtime_cpu_setup. I'm simply setting up the abort stack earlier. An alternative would be to setup a different abort handler and stack before relocation. I'm open to suggestions, but as it stands now aborts before or during relocation will simply hang without this. > Also: if this patch is moving interrupt_init(), then where is the line > where a call to interrupt_init() is removed? Yes, the later call to interrupt_init should be removed assuming we keep this approach. Rob
Hi Rob, On Wed, 15 May 2013 16:29:41 -0500, Rob Herring <robherring2@gmail.com> wrote: > On Wed, May 15, 2013 at 3:26 PM, Albert ARIBAUD > <albert.u.boot@aribaud.net> wrote: > > Hi Rob, > > > > On Wed, 15 May 2013 14:56:07 -0500, Rob Herring <robherring2@gmail.com> > > wrote: > > > >> From: Rob Herring <rob.herring@calxeda.com> > >> > >> interrupt_init also sets up the abort stack, but is not setup before > >> relocation. So any aborts during relocation will hang and not print out > >> any useful information. Fix this by moving the interrupt_init to after > >> the stack setup in board_init_f. > >> > >> Signed-off-by: Rob Herring <rob.herring@calxeda.com> > >> --- > >> arch/arm/lib/board.c | 1 + > >> 1 file changed, 1 insertion(+) > >> > >> diff --git a/arch/arm/lib/board.c b/arch/arm/lib/board.c > >> index 09ab4ad..6dbe7e2 100644 > >> --- a/arch/arm/lib/board.c > >> +++ b/arch/arm/lib/board.c > >> @@ -447,6 +447,7 @@ void board_init_f(ulong bootflag) > >> addr_sp += 128; /* leave 32 words for abort-stack */ > >> gd->irq_sp = addr_sp; > >> #endif > >> + interrupt_init(); > >> > >> debug("New Stack Pointer is: %08lx\n", addr_sp); > >> > > > > I fail to understand how this is even supposed to work through > > relocation: exception vectors are not relocated, so if they work before > > relocation, then they won't work any more afterward unless some code is > > added to relocate them. > > They work before and after in my testing. The vectors are relocated > along with the rest of u-boot and the vector base is updated by > c_runtime_cpu_setup. I'm simply setting up the abort stack earlier. An > alternative would be to setup a different abort handler and stack > before relocation. I'm open to suggestions, but as it stands now > aborts before or during relocation will simply hang without this. Thanks for the clarification. This opens at least two quite interesting issues: 1. interrupts_init() does not initialize interrupts but stacks, and does not initialize only interrupt stacks but also exception stacks; it badly needs a rename (and so does the abort stack, BTW). 2. Much more worrying, interrupt vectors are only fixed for armv7, and only by setting VBAR. This seems to imply that for all other ARM targets, VBAR is not modified, and neither are the exception vectors at either 0x00000000 or 0xFFFF0000; and this means that exceptions are never pointed to the RAM-running U-Boot; they would work for NOR FLASH based U-Boots, but any other case I simply fail to see how it would work as intended. But issue 2 is so big that I probably simply missed something obvious. Anyone able to show me how dumb I am here, feel free. :) Regarding your change, Rob, it's good news anyway as I'm not going to ask you to overhaul the whole of ARM exception handling code in any case :) and I am fine with the change as it is, except: > > Also: if this patch is moving interrupt_init(), then where is the line > > where a call to interrupt_init() is removed? > > Yes, the later call to interrupt_init should be removed assuming we > keep this approach. Then I would like the removal to be paired with this addition in a single commit. > Rob Amicalement,
diff --git a/arch/arm/lib/board.c b/arch/arm/lib/board.c index 09ab4ad..6dbe7e2 100644 --- a/arch/arm/lib/board.c +++ b/arch/arm/lib/board.c @@ -447,6 +447,7 @@ void board_init_f(ulong bootflag) addr_sp += 128; /* leave 32 words for abort-stack */ gd->irq_sp = addr_sp; #endif + interrupt_init(); debug("New Stack Pointer is: %08lx\n", addr_sp);