ARM: aarch64: lowlevel: potential bug in arm_cpu_lowlevel_init

[Lior Weintraub]

Thanks Ahmad,
Cheers,
Lior.

> -----Original Message-----
> From: Ahmad Fatoum <a.fatoum at pengutronix.de>
> Sent: Thursday, September 7, 2023 11:27 AM
> To: Lior Weintraub <liorw at pliops.com>; Sascha Hauer
> <s.hauer at pengutronix.de>
> Cc: andrew.smirnov at gmail.com; barebox at lists.infradead.org
> Subject: Re: ARM: aarch64: lowlevel: potential bug in arm_cpu_lowlevel_init
> 
> CAUTION: External Sender
> 
> 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 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 |
> >
> 
> --
> Pengutronix e.K.                           |                             |
> Steuerwalder Str. 21                       | http://secure-
> web.cisco.com/1SHUS_p9EYTjEgUvZXKrq9NvnxBMmGacitjdSEASh4bUAik3yk
> i5yL5y5nluvQb7DkrLuqG_IPCzZqH8TSaWdOvJIyrVbvcGJ4pOtQ0NV4imoJZUX
> r37E4-w5SymjRxps76WVat_7azm-
> Ozm6ceDhQblgMTonfl7Vvi8ZBw8FQsuRKCkGr8t5RreU9oexo-
> qsvzXPANGzFzvv9Il3mmmRE3R8M0p_QxJV8xvS4AkYz7IR64Xv7Olh6z5I8Vgy
> QBxVyB3dEXXmhpY6Uoje_ksuei3vMtNQ0aPgYssmwVWolyIh1NKPorpFykcw
> VhjL9z1k/http%3A%2F%2Fwww.pengutronix.de%2F  |
> 31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
>