Re: ARM: aarch64: lowlevel: potential bug in arm_cpu_lowlevel_init

[Ahmad Fatoum <a.fatoum@pengutronix.de>]

Hello Lior,

On 16.08.23 12:14, Lior Weintraub wrote:
> Thanks Sascha!
> 
> Actually the bootloader is still under development so there is no problem to change and work without MMU.
> (or at lease disable it before jumping to Barebox).
> I totally agree that if virtual mapping was used it would be impossible to disable the MMU.
> Just thought that it is totally harmless to add `dsb sy`.

as Sascha mentioned, we expect MMU to be off, but some BootROMs fail to
do that in some error cases. I thus think if a barrier would save us
in such a case, we should have it in arm_cpu_lowlevel_init().

Cheers,
Ahmad

> 
> Cheers,
> Lior.
> 
>> -----Original Message-----
>> From: Sascha Hauer <s.hauer@pengutronix.de>
>> Sent: Wednesday, August 16, 2023 1:02 PM
>> To: Lior Weintraub <liorw@pliops.com>
>> Cc: andrew.smirnov@gmail.com; barebox@lists.infradead.org; Ahmad
>> Fatoum <a.fatoum@pengutronix.de>
>> Subject: Re: ARM: aarch64: lowlevel: potential bug in arm_cpu_lowlevel_init
>>
>> CAUTION: External Sender
>>
>> Hi Lior,
>>
>> On Mon, Aug 14, 2023 at 11:35:05AM +0000, Lior Weintraub wrote:
>>>    Link: [1]File-List
>>>    Hi Andrew,
>>>
>>>    I am asking about a patch you've introduced about 4 years ago:
>>>    Commit: cd6e1857a6a824d562bd27379d191602c074f6b7
>>>
>>>    ENTRY(arm_cpu_lowlevel_init)
>>>                    switch_el x1, 3f, 2f, 1f
>>>
>>>    3:
>>>                    mov       x0, #1                                    /*
>>>    Non-Secure EL0/1 */
>>>                    orr          x0, x0, #(1 << 10)              /* 64-bit EL2
>>>    */
>>>                    msr        scr_el3, x0
>>>                    msr        cptr_el3, xzr
>>>
>>>                    mrs        x0, sctlr_el3
>>>                    ldr           x1, =SCTLR_ELx_FLAGS
>>>                    bic          x0, x0, x1
>>>                    msr        sctlr_el3, x0
>>>                    isb
>>>
>>>                    b             done
>>>
>>>    This code has introduced a bug in our barebox porting.
>>>    It could be our mistake but then again we couldn't find any prerequisites
>>>    conditions that bootloaders need to meet before passing control to
>> barebox
>>>    pbl.
>>>    There are 2 bugs that can happen here:
>>>     1. The bootloader enabled MMU and set the SRAM (given to barebox) as
>>>        non-secure – This issue can be resolved with adding "dsb sy" command
>>>        before the "isb"
>>>     2. The bootloader enabled MMU and dcache on SRAM (given to barebox)
>> as
>>>        non-secure – This is a bit harder to solve because it needs to call
>>>        cache invalidate on the stack.
>>
>> Eventhough it might not be explicitly documented, barebox normally
>> expects to be called with MMU disabled.
>>
>> When the MMU is enabled it could have virtually any mapping and there's
>> no sane way to disable the MMU then.
>>
>> If your bootloader enables the MMU and there is no way to change that,
>> then it's up to your board entry code to disable the MMU before calling into
>> barebox. That said, when there's something can change in
>> arm_cpu_lowlevel_init()
>> to help you with your case, then we can do that, but I wouldn't consider
>> this a bug in barebox.
>>
>> Sascha
>>
>> --
>> Pengutronix e.K.                           |                             |
>> Steuerwalder Str. 21                       | http://secure-
>> web.cisco.com/17RdScP_TBoXqc1IerwhZX8kAHhqutjAnByPiUM5lNZJyN6OD
>> jkdTnyYCzrqpRZ9mdLg4wp-dZiElAak3iS7lf-YQwNFSattw-4zxIt6ro_qHh-
>> 8ovK33TBdMnr5qL5qPKFUriYYQ8bLMAnMHa4bKtTIfhZI2eSXUr6hqjDcnoKZ
>> GKUO2dbBDXceX3NT-ql9GBlwdH-mmkhd9fgYV2r0UtR4ot5d58hg45JTJ-
>> LsuvNVMsKrFZd1Iu3AiMSjmi4E5BDc6KpQx_MgAD410FQWC0HpDavhSc1DC
>> BgdMO2RlGrZk0L-
>> St3n_yH_QZvH4UfjY/http%3A%2F%2Fwww.pengutronix.de%2F  |
>> 31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
>> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |