mail archive of the barebox mailing list
 help / color / mirror / Atom feed
From: Lior Weintraub <liorw@pliops.com>
To: Ahmad Fatoum <a.fatoum@pengutronix.de>,
	Sascha Hauer <s.hauer@pengutronix.de>
Cc: "andrew.smirnov@gmail.com" <andrew.smirnov@gmail.com>,
	"barebox@lists.infradead.org" <barebox@lists.infradead.org>
Subject: RE: ARM: aarch64: lowlevel: potential bug in arm_cpu_lowlevel_init
Date: Thu, 7 Sep 2023 08:30:34 +0000	[thread overview]
Message-ID: <PR3P195MB0555BADA6D384623482DC8D1C3EEA@PR3P195MB0555.EURP195.PROD.OUTLOOK.COM> (raw)
In-Reply-To: <1051bfae-680e-b4d9-3036-87802801710a@pengutronix.de>

Thanks Ahmad,
Cheers,
Lior.

> -----Original Message-----
> From: Ahmad Fatoum <a.fatoum@pengutronix.de>
> Sent: Thursday, September 7, 2023 11:27 AM
> To: Lior Weintraub <liorw@pliops.com>; Sascha Hauer
> <s.hauer@pengutronix.de>
> Cc: andrew.smirnov@gmail.com; barebox@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@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://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 |
> 


      reply	other threads:[~2023-09-07  8:31 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <PR3P195MB0555C0CAAE5F70743B7DC534C317A@PR3P195MB0555.EURP195.PROD.OUTLOOK.COM>
2023-08-14 11:41 ` Lior Weintraub
2023-08-16 10:01 ` Sascha Hauer
2023-08-16 10:14   ` Lior Weintraub
2023-09-07  8:26     ` Ahmad Fatoum
2023-09-07  8:30       ` Lior Weintraub [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=PR3P195MB0555BADA6D384623482DC8D1C3EEA@PR3P195MB0555.EURP195.PROD.OUTLOOK.COM \
    --to=liorw@pliops.com \
    --cc=a.fatoum@pengutronix.de \
    --cc=andrew.smirnov@gmail.com \
    --cc=barebox@lists.infradead.org \
    --cc=s.hauer@pengutronix.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox