ARM: aarch64: lowlevel: potential bug in arm_cpu_lowlevel_init

[Lior Weintraub]

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`.

Cheers,
Lior.

> -----Original Message-----
> From: Sascha Hauer <s.hauer at pengutronix.de>
> Sent: Wednesday, August 16, 2023 1:02 PM
> To: Lior Weintraub <liorw at pliops.com>
> Cc: andrew.smirnov at gmail.com; barebox at lists.infradead.org; Ahmad
> Fatoum <a.fatoum at 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 |