diff mbox

[U-Boot] arm: mxs: Fix vectoring table crafting

Message ID 1366943832-8991-1-git-send-email-marex@denx.de
State Awaiting Upstream
Delegated to: Stefano Babic
Headers show

Commit Message

Marek Vasut April 26, 2013, 2:37 a.m. UTC
The vectoring table has to be placed at 0x0, but U-Boot on MX23/MX28
starts from RAM, so the vectoring table at 0x0 is not present. Craft
code that will be placed at 0x0 and will redirect interrupt vectoring
to proper location of the U-Boot in RAM.

Signed-off-by: Marek Vasut <marex@denx.de>
CC: Stefano Babic <sbabic@denx.de>
CC: Fabio Estevam <fabio.estevam@freescale.com>
---
 arch/arm/cpu/arm926ejs/mxs/mxs.c |   25 ++++++++++++++++++++++---
 1 file changed, 22 insertions(+), 3 deletions(-)

Comments

Otavio Salvador April 26, 2013, 3:05 a.m. UTC | #1
On Thu, Apr 25, 2013 at 11:37 PM, Marek Vasut <marex@denx.de> wrote:
> The vectoring table has to be placed at 0x0, but U-Boot on MX23/MX28
> starts from RAM, so the vectoring table at 0x0 is not present. Craft
> code that will be placed at 0x0 and will redirect interrupt vectoring
> to proper location of the U-Boot in RAM.
>
> Signed-off-by: Marek Vasut <marex@denx.de>
> CC: Stefano Babic <sbabic@denx.de>
> CC: Fabio Estevam <fabio.estevam@freescale.com>

Please always Cc me in 'mxs' patches.

> +/*
> + * This function will craft a jumptable at 0x0 which will redirect interrupt
> + * vectoring to proper location of U-Boot in RAM.
> + *
> + * The structure of the jumptable will be as follows:
> + *  ldr pc, [pc, #0x18] ..... for each vector, thus repeated 8 times
> + *  <destination address> ... for each previous ldr, thus also repeated 8 times
> + *
> + * The "ldr pc, [pc, #0x18]" instruction above loads address from memory at
> + * offset 0x18 from current value of PC register. Note that PC is already
> + * incremented by 4 when computing the offset, so the effective offset is
> + * actually 0x20, this the associated <destination address>. Loading the PC
> + * register with an address performs a jump to that address.
> + */

I understood what you're doing but not the motivation for it. What
problem you're fixing or feature this will allow?

--
Otavio Salvador                             O.S. Systems
E-mail: otavio@ossystems.com.br  http://www.ossystems.com.br
Mobile: +55 53 9981-7854              http://projetos.ossystems.com.br
Marek Vasut April 26, 2013, 3:10 a.m. UTC | #2
Dear Otavio Salvador,

[...]

> > +/*
> > + * This function will craft a jumptable at 0x0 which will redirect
> > interrupt + * vectoring to proper location of U-Boot in RAM.
> > + *
> > + * The structure of the jumptable will be as follows:
> > + *  ldr pc, [pc, #0x18] ..... for each vector, thus repeated 8 times
> > + *  <destination address> ... for each previous ldr, thus also repeated
> > 8 times + *
> > + * The "ldr pc, [pc, #0x18]" instruction above loads address from memory
> > at + * offset 0x18 from current value of PC register. Note that PC is
> > already + * incremented by 4 when computing the offset, so the effective
> > offset is + * actually 0x20, this the associated <destination address>.
> > Loading the PC + * register with an address performs a jump to that
> > address.
> > + */
> 
> I understood what you're doing but not the motivation for it. What
> problem you're fixing or feature this will allow?

In case the CPU core gets and interrupt (data abort, invalid instruction...), 
the CPU will jump to 0x0. It is imperative for handling code to be present at 
0x0, but since U-Boot on MXS is started from RAM, only remnants of SPL are 
present at 0x0. Any core interrupt will then hit those remnants and cause 
undefined behavior. This patch puts redirection table there which in turn jumps 
to U-Boot vectoring entry points in RAM ... but this is all in the commit 
message already, so dunno if that helps you.

Best regards,
Marek Vasut
Otavio Salvador April 26, 2013, 3:20 a.m. UTC | #3
On Fri, Apr 26, 2013 at 12:10 AM, Marek Vasut <marex@denx.de> wrote:
> Dear Otavio Salvador,
>
> [...]
>
>> > +/*
>> > + * This function will craft a jumptable at 0x0 which will redirect
>> > interrupt + * vectoring to proper location of U-Boot in RAM.
>> > + *
>> > + * The structure of the jumptable will be as follows:
>> > + *  ldr pc, [pc, #0x18] ..... for each vector, thus repeated 8 times
>> > + *  <destination address> ... for each previous ldr, thus also repeated
>> > 8 times + *
>> > + * The "ldr pc, [pc, #0x18]" instruction above loads address from memory
>> > at + * offset 0x18 from current value of PC register. Note that PC is
>> > already + * incremented by 4 when computing the offset, so the effective
>> > offset is + * actually 0x20, this the associated <destination address>.
>> > Loading the PC + * register with an address performs a jump to that
>> > address.
>> > + */
>>
>> I understood what you're doing but not the motivation for it. What
>> problem you're fixing or feature this will allow?
>
> In case the CPU core gets and interrupt (data abort, invalid instruction...),
> the CPU will jump to 0x0. It is imperative for handling code to be present at
> 0x0, but since U-Boot on MXS is started from RAM, only remnants of SPL are
> present at 0x0. Any core interrupt will then hit those remnants and cause
> undefined behavior. This patch puts redirection table there which in turn jumps
> to U-Boot vectoring entry points in RAM ... but this is all in the commit
> message already, so dunno if that helps you.

Much clear now. Did you actually tested this in MX23 as well? How did
you simulated the exception to test it?

--
Otavio Salvador                             O.S. Systems
E-mail: otavio@ossystems.com.br  http://www.ossystems.com.br
Mobile: +55 53 9981-7854              http://projetos.ossystems.com.br
Marek Vasut April 26, 2013, 12:08 p.m. UTC | #4
Dear Otavio Salvador,

> On Fri, Apr 26, 2013 at 12:10 AM, Marek Vasut <marex@denx.de> wrote:
> > Dear Otavio Salvador,
> > 
> > [...]
> > 
> >> > +/*
> >> > + * This function will craft a jumptable at 0x0 which will redirect
> >> > interrupt + * vectoring to proper location of U-Boot in RAM.
> >> > + *
> >> > + * The structure of the jumptable will be as follows:
> >> > + *  ldr pc, [pc, #0x18] ..... for each vector, thus repeated 8 times
> >> > + *  <destination address> ... for each previous ldr, thus also
> >> > repeated 8 times + *
> >> > + * The "ldr pc, [pc, #0x18]" instruction above loads address from
> >> > memory at + * offset 0x18 from current value of PC register. Note
> >> > that PC is already + * incremented by 4 when computing the offset, so
> >> > the effective offset is + * actually 0x20, this the associated
> >> > <destination address>. Loading the PC + * register with an address
> >> > performs a jump to that address.
> >> > + */
> >> 
> >> I understood what you're doing but not the motivation for it. What
> >> problem you're fixing or feature this will allow?
> > 
> > In case the CPU core gets and interrupt (data abort, invalid
> > instruction...), the CPU will jump to 0x0. It is imperative for handling
> > code to be present at 0x0, but since U-Boot on MXS is started from RAM,
> > only remnants of SPL are present at 0x0. Any core interrupt will then
> > hit those remnants and cause undefined behavior. This patch puts
> > redirection table there which in turn jumps to U-Boot vectoring entry
> > points in RAM ... but this is all in the commit message already, so
> > dunno if that helps you.
> 
> Much clear now. Did you actually tested this in MX23 as well?

No, it is not needed, but you can test it if it'd make you feel better. The ARM 
core is the same in both, so is the memory map for DRAM and SRAM.

> How did
> you simulated the exception to test it?

Try "md.l 0x1" or "go 0x20", they'll cause either dabt or invalid instruction.


Best regards,
Marek Vasut
Fabio Estevam April 26, 2013, 8:32 p.m. UTC | #5
On Thu, Apr 25, 2013 at 11:37 PM, Marek Vasut <marex@denx.de> wrote:
> The vectoring table has to be placed at 0x0, but U-Boot on MX23/MX28
> starts from RAM, so the vectoring table at 0x0 is not present. Craft
> code that will be placed at 0x0 and will redirect interrupt vectoring
> to proper location of the U-Boot in RAM.
>
> Signed-off-by: Marek Vasut <marex@denx.de>
> CC: Stefano Babic <sbabic@denx.de>
> CC: Fabio Estevam <fabio.estevam@freescale.com>

On a mx23evk:

Tested-by: Fabio Estevam <fabio.estevam@freescale.com>
Stefano Babic June 3, 2013, 10:50 a.m. UTC | #6
On 26/04/2013 04:37, Marek Vasut wrote:
> The vectoring table has to be placed at 0x0, but U-Boot on MX23/MX28
> starts from RAM, so the vectoring table at 0x0 is not present. Craft
> code that will be placed at 0x0 and will redirect interrupt vectoring
> to proper location of the U-Boot in RAM.
> 
> Signed-off-by: Marek Vasut <marex@denx.de>
> CC: Stefano Babic <sbabic@denx.de>
> CC: Fabio Estevam <fabio.estevam@freescale.com>
> ---


Applied to u-boot-imx, thanks.

Best regards,
Stefano Babic
diff mbox

Patch

diff --git a/arch/arm/cpu/arm926ejs/mxs/mxs.c b/arch/arm/cpu/arm926ejs/mxs/mxs.c
index e2b4196..600e2a8 100644
--- a/arch/arm/cpu/arm926ejs/mxs/mxs.c
+++ b/arch/arm/cpu/arm926ejs/mxs/mxs.c
@@ -139,13 +139,32 @@  int mxs_reset_block(struct mxs_register_32 *reg)
 	return 0;
 }
 
+/*
+ * This function will craft a jumptable at 0x0 which will redirect interrupt
+ * vectoring to proper location of U-Boot in RAM.
+ *
+ * The structure of the jumptable will be as follows:
+ *  ldr pc, [pc, #0x18] ..... for each vector, thus repeated 8 times
+ *  <destination address> ... for each previous ldr, thus also repeated 8 times
+ *
+ * The "ldr pc, [pc, #0x18]" instruction above loads address from memory at
+ * offset 0x18 from current value of PC register. Note that PC is already
+ * incremented by 4 when computing the offset, so the effective offset is
+ * actually 0x20, this the associated <destination address>. Loading the PC
+ * register with an address performs a jump to that address.
+ */
 void mx28_fixup_vt(uint32_t start_addr)
 {
-	uint32_t *vt = (uint32_t *)0x20;
+	/* ldr pc, [pc, #0x18] */
+	const uint32_t ldr_pc = 0xe59ff018;
+	/* Jumptable location is 0x0 */
+	uint32_t *vt = (uint32_t *)0x0;
 	int i;
 
-	for (i = 0; i < 8; i++)
-		vt[i] = start_addr + (4 * i);
+	for (i = 0; i < 8; i++) {
+		vt[i] = ldr_pc;
+		vt[i + 8] = start_addr + (4 * i);
+	}
 }
 
 #ifdef	CONFIG_ARCH_MISC_INIT