Patchwork [U-Boot] arm: bugfix: Move vector table before jumping relocated code

login
register
mail settings
Submitter Tetsuyuki Kobayashi
Date June 25, 2012, 12:40 p.m.
Message ID <4FE85C59.1010400@kmckk.co.jp>
Download mbox | patch
Permalink /patch/167090/
State Accepted
Delegated to: Albert ARIBAUD
Headers show

Comments

Tetsuyuki Kobayashi - June 25, 2012, 12:40 p.m.
Interrupts and exceptions doesn't work in relocated code.
It badly use IRQ_STACK_START_IN in rom area as interrupt stack.
It is because the vecotr table is not moved to ram area.
This patch moves vector table before jumping relocated code.

Signed-off-by: Tetsuyuki Kobayashi <koba@kmckk.co.jp>
---
 arch/arm/cpu/armv7/start.S |   12 ++++++++++++
 1 file changed, 12 insertions(+)
Stephen Warren - June 25, 2012, 3:10 p.m.
On 06/25/2012 06:40 AM, Tetsuyuki Kobayashi wrote:
> Interrupts and exceptions doesn't work in relocated code.
> It badly use IRQ_STACK_START_IN in rom area as interrupt stack.
> It is because the vecotr table is not moved to ram area.
> This patch moves vector table before jumping relocated code.
> 
> Signed-off-by: Tetsuyuki Kobayashi <koba@kmckk.co.jp>

CC'ing in some Tegra people.

Tetsuyuki, you probably want to CC some OMAP people too.

> ---
>  arch/arm/cpu/armv7/start.S |   12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/arch/arm/cpu/armv7/start.S b/arch/arm/cpu/armv7/start.S
> index 52f7f6e..5098f7b 100644
> --- a/arch/arm/cpu/armv7/start.S
> +++ b/arch/arm/cpu/armv7/start.S
> @@ -277,6 +277,18 @@ jump_2_ram:
>  	mcr     p15, 0, r0, c7, c10, 4	@ DSB
>  	mcr     p15, 0, r0, c7, c5, 4	@ ISB
>  #endif
> +/*
> + * Move vector table
> + */
> +#if !defined(CONFIG_TEGRA2)
> +#if !(defined(CONFIG_OMAP44XX) && defined(CONFIG_SPL_BUILD))
> +	/* Set vector address in CP15 VBAR register */
> +	ldr     r0, =_start
> +	add     r0, r0, r9
> +	mcr     p15, 0, r0, c12, c0, 0  @Set VBAR
> +#endif
> +#endif /* !Tegra2 */
> +	
>  	ldr	r0, _board_init_r_ofs
>  	adr	r1, _start
>  	add	lr, r0, r1
Tetsuyuki Kobayashi - June 26, 2012, 1:03 a.m.
Hello,

(06/26/2012 12:10 AM), Stephen Warren wrote:
> On 06/25/2012 06:40 AM, Tetsuyuki Kobayashi wrote:
>> Interrupts and exceptions doesn't work in relocated code.
>> It badly use IRQ_STACK_START_IN in rom area as interrupt stack.
>> It is because the vecotr table is not moved to ram area.
>> This patch moves vector table before jumping relocated code.
>>
>> Signed-off-by: Tetsuyuki Kobayashi <koba@kmckk.co.jp>
> 
> CC'ing in some Tegra people.
> 
> Tetsuyuki, you probably want to CC some OMAP people too.

Thank you. 
I don't know proper person to CC because I am very new in this ML.
I need some help ..

'#if condition' in my patch is the same as the code setting VBAR right after reset.
(in file arch/arm/cpu/armv7/start.S)

> 
>> ---
>>  arch/arm/cpu/armv7/start.S |   12 ++++++++++++
>>  1 file changed, 12 insertions(+)
>>
>> diff --git a/arch/arm/cpu/armv7/start.S b/arch/arm/cpu/armv7/start.S
>> index 52f7f6e..5098f7b 100644
>> --- a/arch/arm/cpu/armv7/start.S
>> +++ b/arch/arm/cpu/armv7/start.S
>> @@ -277,6 +277,18 @@ jump_2_ram:
>>  	mcr     p15, 0, r0, c7, c10, 4	@ DSB
>>  	mcr     p15, 0, r0, c7, c5, 4	@ ISB
>>  #endif
>> +/*
>> + * Move vector table
>> + */
>> +#if !defined(CONFIG_TEGRA2)
>> +#if !(defined(CONFIG_OMAP44XX) && defined(CONFIG_SPL_BUILD))
>> +	/* Set vector address in CP15 VBAR register */
>> +	ldr     r0, =_start
>> +	add     r0, r0, r9
>> +	mcr     p15, 0, r0, c12, c0, 0  @Set VBAR
>> +#endif
>> +#endif /* !Tegra2 */
>> +	
>>  	ldr	r0, _board_init_r_ofs
>>  	adr	r1, _start
>>  	add	lr, r0, r1
> 	
>
Stephen Warren - June 26, 2012, 4:24 p.m.
On 06/25/2012 07:03 PM, Tetsuyuki Kobayashi wrote:
> Hello,
> 
> (06/26/2012 12:10 AM), Stephen Warren wrote:
>> On 06/25/2012 06:40 AM, Tetsuyuki Kobayashi wrote:
>>> Interrupts and exceptions doesn't work in relocated code.
>>> It badly use IRQ_STACK_START_IN in rom area as interrupt stack.
>>> It is because the vecotr table is not moved to ram area.
>>> This patch moves vector table before jumping relocated code.
>>>
>>> Signed-off-by: Tetsuyuki Kobayashi <koba@kmckk.co.jp>
>>
>> CC'ing in some Tegra people.
>>
>> Tetsuyuki, you probably want to CC some OMAP people too.
> 
> Thank you. 
> I don't know proper person to CC because I am very new in this ML.
> I need some help ..
> 
> '#if condition' in my patch is the same as the code setting VBAR right after reset.
> (in file arch/arm/cpu/armv7/start.S)

I imagine the primary ARM custodian and the TI ARM sub-arch custodian
(both now CC'd) would be a good place to start. You'd need to CC them
anyway in order to get this patch checked in.

>>> ---
>>>  arch/arm/cpu/armv7/start.S |   12 ++++++++++++
>>>  1 file changed, 12 insertions(+)
>>>
>>> diff --git a/arch/arm/cpu/armv7/start.S b/arch/arm/cpu/armv7/start.S
>>> index 52f7f6e..5098f7b 100644
>>> --- a/arch/arm/cpu/armv7/start.S
>>> +++ b/arch/arm/cpu/armv7/start.S
>>> @@ -277,6 +277,18 @@ jump_2_ram:
>>>  	mcr     p15, 0, r0, c7, c10, 4	@ DSB
>>>  	mcr     p15, 0, r0, c7, c5, 4	@ ISB
>>>  #endif
>>> +/*
>>> + * Move vector table
>>> + */
>>> +#if !defined(CONFIG_TEGRA2)
>>> +#if !(defined(CONFIG_OMAP44XX) && defined(CONFIG_SPL_BUILD))
>>> +	/* Set vector address in CP15 VBAR register */
>>> +	ldr     r0, =_start
>>> +	add     r0, r0, r9
>>> +	mcr     p15, 0, r0, c12, c0, 0  @Set VBAR
>>> +#endif
>>> +#endif /* !Tegra2 */
>>> +	
>>>  	ldr	r0, _board_init_r_ofs
>>>  	adr	r1, _start
>>>  	add	lr, r0, r1
>> 	
>>
> 
>
Tom Rini - June 27, 2012, 6:23 p.m.
On Mon, Jun 25, 2012 at 09:40:57PM +0900, Tetsuyuki Kobayashi wrote:

> Interrupts and exceptions doesn't work in relocated code.
> It badly use IRQ_STACK_START_IN in rom area as interrupt stack.
> It is because the vecotr table is not moved to ram area.
> This patch moves vector table before jumping relocated code.
> 
> Signed-off-by: Tetsuyuki Kobayashi <koba@kmckk.co.jp>
> ---
>  arch/arm/cpu/armv7/start.S |   12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/arch/arm/cpu/armv7/start.S b/arch/arm/cpu/armv7/start.S
> index 52f7f6e..5098f7b 100644
> --- a/arch/arm/cpu/armv7/start.S
> +++ b/arch/arm/cpu/armv7/start.S
> @@ -277,6 +277,18 @@ jump_2_ram:
>  	mcr     p15, 0, r0, c7, c10, 4	@ DSB
>  	mcr     p15, 0, r0, c7, c5, 4	@ ISB
>  #endif
> +/*
> + * Move vector table
> + */
> +#if !defined(CONFIG_TEGRA2)
> +#if !(defined(CONFIG_OMAP44XX) && defined(CONFIG_SPL_BUILD))
> +	/* Set vector address in CP15 VBAR register */
> +	ldr     r0, =_start
> +	add     r0, r0, r9
> +	mcr     p15, 0, r0, c12, c0, 0  @Set VBAR
> +#endif
> +#endif /* !Tegra2 */
> +	
>  	ldr	r0, _board_init_r_ofs
>  	adr	r1, _start
>  	add	lr, r0, r1

I think this code should get boot tested on a few platforms to make sure
it's OK.  As such, I've tried on am335x and omap3 and they're still
fine.

Tested-by: Tom Rini <trini@ti.com>
Tetsuyuki Kobayashi - June 28, 2012, 1:27 a.m.
Hi Tom and all,

(2012/06/28 3:23), Tom Rini wrote:
> On Mon, Jun 25, 2012 at 09:40:57PM +0900, Tetsuyuki Kobayashi wrote:
>
>> Interrupts and exceptions doesn't work in relocated code.
>> It badly use IRQ_STACK_START_IN in rom area as interrupt stack.
>> It is because the vecotr table is not moved to ram area.
>> This patch moves vector table before jumping relocated code.
>>
>> Signed-off-by: Tetsuyuki Kobayashi<koba@kmckk.co.jp>
>> ---
>>   arch/arm/cpu/armv7/start.S |   12 ++++++++++++
>>   1 file changed, 12 insertions(+)
>>
>> diff --git a/arch/arm/cpu/armv7/start.S b/arch/arm/cpu/armv7/start.S
>> index 52f7f6e..5098f7b 100644
>> --- a/arch/arm/cpu/armv7/start.S
>> +++ b/arch/arm/cpu/armv7/start.S
>> @@ -277,6 +277,18 @@ jump_2_ram:
>>   	mcr     p15, 0, r0, c7, c10, 4	@ DSB
>>   	mcr     p15, 0, r0, c7, c5, 4	@ ISB
>>   #endif
>> +/*
>> + * Move vector table
>> + */
>> +#if !defined(CONFIG_TEGRA2)
>> +#if !(defined(CONFIG_OMAP44XX)&&  defined(CONFIG_SPL_BUILD))
>> +	/* Set vector address in CP15 VBAR register */
>> +	ldr     r0, =_start
>> +	add     r0, r0, r9
>> +	mcr     p15, 0, r0, c12, c0, 0  @Set VBAR
>> +#endif
>> +#endif /* !Tegra2 */
>> +	
>>   	ldr	r0, _board_init_r_ofs
>>   	adr	r1, _start
>>   	add	lr, r0, r1
>
> I think this code should get boot tested on a few platforms to make sure
> it's OK.  As such, I've tried on am335x and omap3 and they're still
> fine.
>
> Tested-by: Tom Rini<trini@ti.com>
>
Tom, thank you for testing.

Easy way to test this patch.
Jump any bad address using go command on U-boot prompt.

Good restult:

 > go 0x0badc0de
## Starting application at 0x0BADC0DE ...
undefined instruction
pc : [<0badc0e0>]          lr : [<5ff91354>]
sp : 5feefb98  ip : 0000001c     fp : 5feefbdd
r10: 00000002  r9 : 5feefbdd     r8 : 5feeff68
r7 : 5feeff14  r6 : 0badc0de     r5 : 5feefed4  r4 : 00000002
r3 : 0badc0de  r2 : 5feefed4     r1 : 5feefed4  r0 : 00000001
Flags: nZCv  IRQs off  FIQs off  Mode SVC_32
Resetting CPU ...

resetting ...


Bad result:
 > go 0x0badc0de

(.. hang up without any crash dump.)
Albert ARIBAUD - July 5, 2012, 9:27 a.m.
Hi Tetsuyuki, Stephen,

On Tue, 26 Jun 2012 10:24:24 -0600, Stephen Warren
<swarren@wwwdotorg.org> wrote:
> On 06/25/2012 07:03 PM, Tetsuyuki Kobayashi wrote:
> > Hello,
> > 
> > (06/26/2012 12:10 AM), Stephen Warren wrote:
> >> On 06/25/2012 06:40 AM, Tetsuyuki Kobayashi wrote:
> >>> Interrupts and exceptions doesn't work in relocated code.
> >>> It badly use IRQ_STACK_START_IN in rom area as interrupt stack.
> >>> It is because the vecotr table is not moved to ram area.
> >>> This patch moves vector table before jumping relocated code.
> >>>
> >>> Signed-off-by: Tetsuyuki Kobayashi <koba@kmckk.co.jp>
> >>
> >> CC'ing in some Tegra people.
> >>
> >> Tetsuyuki, you probably want to CC some OMAP people too.
> > 
> > Thank you. 
> > I don't know proper person to CC because I am very new in this ML.
> > I need some help ..
> > 
> > '#if condition' in my patch is the same as the code setting VBAR
> > right after reset. (in file arch/arm/cpu/armv7/start.S)
> 
> I imagine the primary ARM custodian and the TI ARM sub-arch custodian
> (both now CC'd) would be a good place to start. You'd need to CC them
> anyway in order to get this patch checked in.

I'll pull this in as soon as atmel and marvell pull reqs are in.

Amicalement,
Albert ARIBAUD - July 5, 2012, noon
Hi Tetsuyuki,

On Mon, 25 Jun 2012 21:40:57 +0900, Tetsuyuki Kobayashi
<koba@kmckk.co.jp> wrote:
> Interrupts and exceptions doesn't work in relocated code.
> It badly use IRQ_STACK_START_IN in rom area as interrupt stack.
> It is because the vecotr table is not moved to ram area.
> This patch moves vector table before jumping relocated code.
> 
> Signed-off-by: Tetsuyuki Kobayashi <koba@kmckk.co.jp>
> ---
>  arch/arm/cpu/armv7/start.S |   12 ++++++++++++
>  1 file changed, 12 insertions(+)

Applied to u-boot-arm/master, thanks.

Amicalement,
Joel Fernandes - Dec. 21, 2012, 3:17 p.m.
Hi Tesuyuki and friends,

I had a question with this patch.

On Wed, Jun 27, 2012 at 8:27 PM, Tetsuyuki Kobayashi <koba@kmckk.co.jp> wrote:
>>> This patch moves vector table before jumping relocated code.
>>>
>>> Signed-off-by: Tetsuyuki Kobayashi<koba@kmckk.co.jp>
>>> ---
>>>   arch/arm/cpu/armv7/start.S |   12 ++++++++++++
>>>   1 file changed, 12 insertions(+)
>>>
>>> diff --git a/arch/arm/cpu/armv7/start.S b/arch/arm/cpu/armv7/start.S
>>> index 52f7f6e..5098f7b 100644
>>> --- a/arch/arm/cpu/armv7/start.S
>>> +++ b/arch/arm/cpu/armv7/start.S
>>> @@ -277,6 +277,18 @@ jump_2_ram:
>>>         mcr     p15, 0, r0, c7, c10, 4  @ DSB
>>>         mcr     p15, 0, r0, c7, c5, 4   @ ISB
>>>   #endif
>>> +/*
>>> + * Move vector table
>>> + */
>>> +#if !defined(CONFIG_TEGRA2)
>>> +#if !(defined(CONFIG_OMAP44XX)&&  defined(CONFIG_SPL_BUILD))
>>>
>>> +       /* Set vector address in CP15 VBAR register */
>>> +       ldr     r0, =_start
>>> +       add     r0, r0, r9
>>> +       mcr     p15, 0, r0, c12, c0, 0  @Set VBAR
>>> +#endif

Why is c12 (VBAR) setup only for SPL builds? Because main u-boot does
relocation too, shouldn't we setup c12 to point to new table addr
after relocation?

Also how do interrupts (irq/fiq) work at all in U-boot if c12 is  not
setup to point to the new vector table address after relocation?

Thanks,
Joel
Tetsuyuki Kobayashi - Dec. 25, 2012, 11:18 p.m.
Hello, Joel

(2012/12/22 0:17), Joel A Fernandes wrote:
> Hi Tesuyuki and friends,
>
> I had a question with this patch.
>
> On Wed, Jun 27, 2012 at 8:27 PM, Tetsuyuki Kobayashi <koba@kmckk.co.jp> wrote:
>>>> This patch moves vector table before jumping relocated code.
>>>>
>>>> Signed-off-by: Tetsuyuki Kobayashi<koba@kmckk.co.jp>
>>>> ---
>>>>    arch/arm/cpu/armv7/start.S |   12 ++++++++++++
>>>>    1 file changed, 12 insertions(+)
>>>>
>>>> diff --git a/arch/arm/cpu/armv7/start.S b/arch/arm/cpu/armv7/start.S
>>>> index 52f7f6e..5098f7b 100644
>>>> --- a/arch/arm/cpu/armv7/start.S
>>>> +++ b/arch/arm/cpu/armv7/start.S
>>>> @@ -277,6 +277,18 @@ jump_2_ram:
>>>>          mcr     p15, 0, r0, c7, c10, 4  @ DSB
>>>>          mcr     p15, 0, r0, c7, c5, 4   @ ISB
>>>>    #endif
>>>> +/*
>>>> + * Move vector table
>>>> + */
>>>> +#if !defined(CONFIG_TEGRA2)
>>>> +#if !(defined(CONFIG_OMAP44XX)&&  defined(CONFIG_SPL_BUILD))
>>>>
>>>> +       /* Set vector address in CP15 VBAR register */
>>>> +       ldr     r0, =_start
>>>> +       add     r0, r0, r9
>>>> +       mcr     p15, 0, r0, c12, c0, 0  @Set VBAR
>>>> +#endif
>
> Why is c12 (VBAR) setup only for SPL builds? Because main u-boot does
> relocation too, shouldn't we setup c12 to point to new table addr
> after relocation?

You may mis-understand. ! means NOT.
Joel Fernandes - Dec. 26, 2012, 12:08 a.m.
On Tue, Dec 25, 2012 at 5:18 PM, Tetsuyuki Kobayashi <koba@kmckk.co.jp> wrote:
> Hello, Joel
>
>
> (2012/12/22 0:17), Joel A Fernandes wrote:
>>
>> Hi Tesuyuki and friends,
>>
>> I had a question with this patch.
>>
>> On Wed, Jun 27, 2012 at 8:27 PM, Tetsuyuki Kobayashi <koba@kmckk.co.jp>
>> wrote:
>>>>>
>>>>> This patch moves vector table before jumping relocated code.
>>>>>
>>>>> Signed-off-by: Tetsuyuki Kobayashi<koba@kmckk.co.jp>
>>>>> ---
>>>>>    arch/arm/cpu/armv7/start.S |   12 ++++++++++++
>>>>>    1 file changed, 12 insertions(+)
>>>>>
>>>>> diff --git a/arch/arm/cpu/armv7/start.S b/arch/arm/cpu/armv7/start.S
>>>>> index 52f7f6e..5098f7b 100644
>>>>> --- a/arch/arm/cpu/armv7/start.S
>>>>> +++ b/arch/arm/cpu/armv7/start.S
>>>>> @@ -277,6 +277,18 @@ jump_2_ram:
>>>>>          mcr     p15, 0, r0, c7, c10, 4  @ DSB
>>>>>          mcr     p15, 0, r0, c7, c5, 4   @ ISB
>>>>>    #endif
>>>>> +/*
>>>>> + * Move vector table
>>>>> + */
>>>>> +#if !defined(CONFIG_TEGRA2)
>>>>> +#if !(defined(CONFIG_OMAP44XX)&&  defined(CONFIG_SPL_BUILD))
>>>>>
>>>>> +       /* Set vector address in CP15 VBAR register */
>>>>> +       ldr     r0, =_start
>>>>> +       add     r0, r0, r9
>>>>> +       mcr     p15, 0, r0, c12, c0, 0  @Set VBAR
>>>>> +#endif
>>
>>
>> Why is c12 (VBAR) setup only for SPL builds? Because main u-boot does
>> relocation too, shouldn't we setup c12 to point to new table addr
>> after relocation?
>
>
> You may mis-understand. ! means NOT.

Yes! Thanks for pointing it out.

Regards,
Joel

Patch

diff --git a/arch/arm/cpu/armv7/start.S b/arch/arm/cpu/armv7/start.S
index 52f7f6e..5098f7b 100644
--- a/arch/arm/cpu/armv7/start.S
+++ b/arch/arm/cpu/armv7/start.S
@@ -277,6 +277,18 @@  jump_2_ram:
 	mcr     p15, 0, r0, c7, c10, 4	@ DSB
 	mcr     p15, 0, r0, c7, c5, 4	@ ISB
 #endif
+/*
+ * Move vector table
+ */
+#if !defined(CONFIG_TEGRA2)
+#if !(defined(CONFIG_OMAP44XX) && defined(CONFIG_SPL_BUILD))
+	/* Set vector address in CP15 VBAR register */
+	ldr     r0, =_start
+	add     r0, r0, r9
+	mcr     p15, 0, r0, c12, c0, 0  @Set VBAR
+#endif
+#endif /* !Tegra2 */
+	
 	ldr	r0, _board_init_r_ofs
 	adr	r1, _start
 	add	lr, r0, r1