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

[Sascha Hauer <s.hauer@pengutronix.de>]

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://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |