[PATCH] Fix data abort in startup for at91 machines based on ARM926EJS

Martin Townsend martin at rufilla.com
Sat Mar 20 12:07:05 CET 2021


Hi Eugen,

It didn't work I'm afraid.  I took a look at the code and I don't think the stack is available, in start.S we have the following for the reset vector:

reset:
/*
* set the cpu to SVC32 mode
*/
mrs r0,cpsr
bic r0,r0,#0x1f
orr r0,r0,#0xd3
msr cpsr,r0

/*
* we do sys-critical inits only at reboot,
* not when booting from ram!
*/
#ifndef CONFIG_SKIP_LOWLEVEL_INIT
bl cpu_init_crit
#endif

bl _main

cpu_init_crit calls lowlevel_init but the stack is not setup until _main is executed

ENTRY(_main)

/*
 * Set up initial C runtime environment and call board_init_f(0).
 */

#if defined(CONFIG_TPL_BUILD) && defined(CONFIG_TPL_NEEDS_SEPARATE_STACK)
ldr r0, =(CONFIG_TPL_STACK)
#elif defined(CONFIG_SPL_BUILD) && defined(CONFIG_SPL_STACK)
ldr r0, =(CONFIG_SPL_STACK)
#else
ldr r0, =(CONFIG_SYS_INIT_SP_ADDR)
#endif
bic r0, r0, #7 /* 8-byte alignment for ABI compliance */
mov sp, r0

which kind of makes sense as the lowlevel_init will initialise the SDRAM that the stack requires.




<http://www.rufilla.com/site/wp-content/uploads/2018/06/GDPR_Privacy_Notice_20180615-FINAL.pdf>


________________________________
From: Eugen.Hristev at microchip.com <Eugen.Hristev at microchip.com>
Sent: 19 March 2021 06:49
To: Martin Townsend <martin at rufilla.com>; u-boot at lists.denx.de <u-boot at lists.denx.de>
Cc: albert.u.boot at aribaud.net <albert.u.boot at aribaud.net>; Nicolas.Ferre at microchip.com <Nicolas.Ferre at microchip.com>
Subject: Re: [PATCH] Fix data abort in startup for at91 machines based on ARM926EJS

Yes, please, try like this, continue to use r4 as it was done initially,
but save r4 to stack and restore before returning. Let's see if this
solution works for you.
I will wait for your v2 of the patch.

Thanks !
Eugen

On 3/18/21 10:31 PM, Martin Townsend wrote:
> Hi Eugen,
>
> Pop and push gets my vote as it future proofs the code from usage of r6
> in lowlevel_init.  Let me know if you want me to respin the patch with
> this and test it out on my board here.
>
> Kind regards
>
> Martin
>
> *Martin Townsend**| *Embedded Software Engineer**
>
> _martin at rufilla.com <mailto:martin at rufilla.com>_
>
>
> **
>
> **
>
> *W***_http://www.rufilla.com <http://www.rufilla.com/>_
>
> *T*    +44 (0)1865 601201
>
> *A*    Building D5 | Culham Science Centre | Abingdon, UK | OX14 3DB
>
> rufilla_logo_transparent <http://www.rufilla.com/>
>
> Rufilla Ltd is a company registered in England and Wales, No. 7093478.
>   Registered address : 6a St Andrew’s Court, Wellington Street, Thame,
> Oxfordshire, OX9 3WT, United Kingdom.  Rufilla Ltd cannot guarantee
> e-mail transmission to be secure or error-free as information could be
> intercepted, corrupted, lost, destroyed, arrive late or incomplete, or
> contain viruses. The sender therefore does not accept any liability
> whatsoever for any errors or omissions in the contents of this message
> or that arises from any data corruption, interception or unauthorised
> amendment, whether or not these arise as a result of the e-mail
> transmission. The information on which this communication is based has
> been obtained from sources we believe to be reliable, but we do not
> guarantee its accuracy or completeness. All expressions of opinion are
> subject to change without notice.  This email may contain confidential
> information and as such, should be treated as confidential. It may also
> contain legally privileged information. It is intended solely for use by
> the normal or ordinary user of the e-mail address to which it has been
> addressed. If you are not the named addressee of this e-mail you are
> prohibited from disseminating, distributing, amending, copying or
> otherwise acting on the contents, including any attachments, of this
> e-mail. Please notify the sender immediately by e-mail if you have
> received this e-mail in error and immediately delete this e-mail from
> your system. You may be asked to confirm that the e-mail received in
> error has been deleted from your system and/or that you have not made
> any unauthorised use, disclosure, distribution or copy of the e-mail.
> Rufilla Ltd is a VAT registered company No.131363252.
>
> Any and all communications sent to us may be monitored and/or stored by
> us to ensure compliance with relevant legislation, rules, and policies.
> All communications are handled in full compliance with current data
> protection legislation including, but not limited to, EU Regulation
> 2016/679 General Data Protection Regulation (“GDPR”).  For further
> information, please refer to our _Privacy Policy
> <http://www.rufilla.com/s/wp-content/uploads/2018/06/GDPR_Privacy_Notice_20180615-FINAL.pdf>_
>
> <http://www.rufilla.com/site/wp-content/uploads/2018/06/GDPR_Privacy_Notice_20180615-FINAL.pdf>
>
>
> ------------------------------------------------------------------------
> *From:* Eugen.Hristev at microchip.com <Eugen.Hristev at microchip.com>
> *Sent:* 18 March 2021 16:53
> *To:* Martin Townsend <martin at rufilla.com>; u-boot at lists.denx.de
> <u-boot at lists.denx.de>
> *Cc:* albert.u.boot at aribaud.net <albert.u.boot at aribaud.net>;
> Nicolas.Ferre at microchip.com <Nicolas.Ferre at microchip.com>
> *Subject:* Re: [PATCH] Fix data abort in startup for at91 machines based
> on ARM926EJS
> Hello Martin,
>
> I reviewed things a bit, and, as it looks to me, and according to AAPCS,
> we can still use r4, as this is scratch space, but we need to save it
> and restore it before and after the code of lowlevel_init.
> Because r6 is also scratch, with your patch , we lose the data in r6, in
> case someone was using it before the call.
> So , do you agree to change this to : still use r4 for operations, but
> push r4 to stack and pop r4 before returning ?
>
> If anyone else has a different opinion, please share.
>
> Thanks,
> Eugen
>
>
> On 3/1/21 2:34 PM, Martin Townsend wrote:
>>
>> Kind regards
>>
>> Martin
>>
>> *Martin Townsend**| *Embedded Software Engineer**
>>
>> _martin at rufilla.com <mailto:martin at rufilla.com>_
>>
>>
>> **
>>
>> **
>>
>> *W***_http://www.rufilla.com <http://www.rufilla.com/>_
>>
>> *T*    +44 (0)1865 601201
>>
>> *A*    Building D5 | Culham Science Centre | Abingdon, UK | OX14 3DB
>>
>> rufilla_logo_transparent <http://www.rufilla.com/>
>>
>> Rufilla Ltd is a company registered in England and Wales, No. 7093478.
>>   Registered address : 6a St Andrew’s Court, Wellington Street, Thame,
>> Oxfordshire, OX9 3WT, United Kingdom.  Rufilla Ltd cannot guarantee
>> e-mail transmission to be secure or error-free as information could be
>> intercepted, corrupted, lost, destroyed, arrive late or incomplete, or
>> contain viruses. The sender therefore does not accept any liability
>> whatsoever for any errors or omissions in the contents of this message
>> or that arises from any data corruption, interception or unauthorised
>> amendment, whether or not these arise as a result of the e-mail
>> transmission. The information on which this communication is based has
>> been obtained from sources we believe to be reliable, but we do not
>> guarantee its accuracy or completeness. All expressions of opinion are
>> subject to change without notice.  This email may contain confidential
>> information and as such, should be treated as confidential. It may also
>> contain legally privileged information. It is intended solely for use by
>> the normal or ordinary user of the e-mail address to which it has been
>> addressed. If you are not the named addressee of this e-mail you are
>> prohibited from disseminating, distributing, amending, copying or
>> otherwise acting on the contents, including any attachments, of this
>> e-mail. Please notify the sender immediately by e-mail if you have
>> received this e-mail in error and immediately delete this e-mail from
>> your system. You may be asked to confirm that the e-mail received in
>> error has been deleted from your system and/or that you have not made
>> any unauthorised use, disclosure, distribution or copy of the e-mail.
>> Rufilla Ltd is a VAT registered company No.131363252.
>>
>> Any and all communications sent to us may be monitored and/or stored by
>> us to ensure compliance with relevant legislation, rules, and policies.
>> All communications are handled in full compliance with current data
>> protection legislation including, but not limited to, EU Regulation
>> 2016/679 General Data Protection Regulation (“GDPR”).  For further
>> information, please refer to our _Privacy Policy
>> <http://www.rufilla.com/s/wp-content/uploads/2018/06/GDPR_Privacy_Notice_20180615-FINAL.pdf>_
>>
>> <http://www.rufilla.com/site/wp-content/uploads/2018/06/GDPR_Privacy_Notice_20180615-FINAL.pdf>
>>
>>
>>
>> ------------------------------------------------------------------------
>> *From:* Eugen.Hristev at microchip.com <Eugen.Hristev at microchip.com>
>> *Sent:* 01 March 2021 12:17
>> *To:* Martin Townsend <martin at rufilla.com>; u-boot at lists.denx.de
>> <u-boot at lists.denx.de>
>> *Cc:* albert.u.boot at aribaud.net <albert.u.boot at aribaud.net>;
>> Nicolas.Ferre at microchip.com <Nicolas.Ferre at microchip.com>
>> *Subject:* Re: [PATCH] Fix data abort in startup for at91 machines based
>> on ARM926EJS
>> Hi,
>>
>> On 26.02.2021 10:44, Martin Townsend wrote:
>>> The startup code in arm/cpu/arm926ejs preserves the link register across
>>> the call to lowlevel_init by using r4:
>>>
>>> mov     r4, lr          /* perserve link reg across call */
>>> bl      lowlevel_init   /* go setup pll,mux,memory */
>>> mov     lr, r4          /* restore link */
>>>
>>> The lowlevel_init function for at91 machines based on the same CPU uses r4
>>> and hence corrupts it causing a data abort when it returns to the startup
>>> code. This patch fixes this by using r6 instead of r4 in the lowlevel_init
>>> function.
>>>
>>> Discovered and the fix was tested on a AT91SAM9261 based board.
>>
>> Very interesting find. I have a few questions:
>> How is this reproducible ?
>> I'm getting a custom Ronetix PM9261 board working with a 2020.01 version
>> of U-Boot and just noticed that it hung with no console output so I
>> attached a JTAG debugger and could see that it was stuck in the data
>> abort so I single stepped through the code and could see the problem was
>> when it returned from lowlevel_init and that the r4 register had been
>> corrupted.
>> Since when this happens ?
>> I have no idea I'm afraid this is the first board that I'm trying to
>> bring up that has this processor architecture.
>> Do you have a fixes tag for this ?
>> I don't I'm afraid
>> Does it affect any board using the arm926ejs ?
>> I only have this Ronetix PM9261 with has a AT91SAM9261 SoC.  I've had a
>> look around and can't see any other boards that I have that use the
>> arm926ejs architecture.
>> Currently we have sam9x60 which is an SoC based on arm 926ejs and we did
>> not see this behavior as of today.
>> Does it have the SKIP_LOWLEVEL_INIT defined? (Not sure of the exact name
>> but it's something like this)  The Ronetix board runs U-Boot from the
>> NOR flash using XIP so the startup code runs from NOR and must run the
>> lowlevel_init function. Maybe the board you have skips the low level
>> intiailisation, just a guess though.
>>
>> Could you also modify the subject, to adhere with the rules subsystem:
>> component: change
>>
>> In your case it's something like
>>
>>    ARM: at91: arm926esj: fix usage of r4 in lowlevel_init
>>
>> Sure will do if you think it's a valid bug.
>>
>> Thanks,
>> Eugen
>>
>>>
>>> Signed-off-by: Martin Townsend <martin at rufilla.com>
>>> ---
>>>   arch/arm/mach-at91/arm926ejs/lowlevel_init.S | 16 ++++++++--------
>>>   1 file changed, 8 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/arch/arm/mach-at91/arm926ejs/lowlevel_init.S b/arch/arm/mach-at91/arm926ejs/lowlevel_init.S
>>> index 71d7582ce0..994f42eb4a 100644
>>> --- a/arch/arm/mach-at91/arm926ejs/lowlevel_init.S
>>> +++ b/arch/arm/mach-at91/arm926ejs/lowlevel_init.S
>>> @@ -71,10 +71,10 @@ POS1:
>>>          str     r0, [r1]
>>>
>>>          /* Reading the PMC Status to detect when the Main Oscillator is enabled */
>>> -       mov     r4, #AT91_PMC_IXR_MOSCS
>>> +       mov     r6, #AT91_PMC_IXR_MOSCS
>>>   MOSCS_Loop:
>>>          ldr     r3, [r2]
>>> -       and     r3, r4, r3
>>> +       and     r3, r6, r3
>>>          cmp     r3, #AT91_PMC_IXR_MOSCS
>>>          bne     MOSCS_Loop
>>>
>>> @@ -89,10 +89,10 @@ MOSCS_Loop:
>>>          str     r0, [r1]
>>>
>>>          /* Reading the PMC Status register to detect when the PLLA is locked */
>>> -       mov     r4, #AT91_PMC_IXR_LOCKA
>>> +       mov     r6, #AT91_PMC_IXR_LOCKA
>>>   MOSCS_Loop1:
>>>          ldr     r3, [r2]
>>> -       and     r3, r4, r3
>>> +       and     r3, r6, r3
>>>          cmp     r3, #AT91_PMC_IXR_LOCKA
>>>          bne     MOSCS_Loop1
>>>
>>> @@ -109,10 +109,10 @@ MOSCS_Loop1:
>>>          str     r0, [r1]
>>>
>>>          /* Reading the PMC Status to detect when the Master clock is ready */
>>> -       mov     r4, #AT91_PMC_IXR_MCKRDY
>>> +       mov     r6, #AT91_PMC_IXR_MCKRDY
>>>   MCKRDY_Loop:
>>>          ldr     r3, [r2]
>>> -       and     r3, r4, r3
>>> +       and     r3, r6, r3
>>>          cmp     r3, #AT91_PMC_IXR_MCKRDY
>>>          bne     MCKRDY_Loop
>>>
>>> @@ -120,10 +120,10 @@ MCKRDY_Loop:
>>>          str     r0, [r1]
>>>
>>>          /* Reading the PMC Status to detect when the Master clock is ready */
>>> -       mov     r4, #AT91_PMC_IXR_MCKRDY
>>> +       mov     r6, #AT91_PMC_IXR_MCKRDY
>>>   MCKRDY_Loop1:
>>>          ldr     r3, [r2]
>>> -       and     r3, r4, r3
>>> +       and     r3, r6, r3
>>>          cmp     r3, #AT91_PMC_IXR_MCKRDY
>>>          bne     MCKRDY_Loop1
>>>   PLL_setup_end:
>>> --
>>> 2.25.1
>>>
>>
>



More information about the U-Boot mailing list