All of lore.kernel.org
 help / color / mirror / Atom feed
From: Reuben Dowle <reuben.dowle@4rf.com>
To: u-boot@lists.denx.de
Subject: [PATCH] Revert "Fix data abort caused by mis-aligning FIT data"
Date: Mon, 19 Oct 2020 22:17:44 +0000	[thread overview]
Message-ID: <0d8797cb314a46bfbf0cfa6e465dd63e@4rf.com> (raw)
In-Reply-To: <38fc6118-b7d7-8455-2c05-3cbe368c4f12@denx.de>

> -----Original Message-----
> From: Marek Vasut <marex@denx.de>
> Sent: Tuesday, 20 October 2020 10:59 am
> To: Reuben Dowle <reuben.dowle@4rf.com>; u-boot at lists.denx.de
> Cc: Tom Rini <trini@konsulko.com>
> Subject: Re: [PATCH] Revert "Fix data abort caused by mis-aligning FIT data"
> 
> On 10/19/20 11:50 PM, Reuben Dowle wrote:
> > The alignment of 8 bytes would also work if code was expecting 4 byte
> alignment. So the explanation you give for reverting this does not make
> sense to me.
> 
> Well, since U-Boot 2020.10-rc5, any STM32MP1 board does no longer boot
> and if I revert this patch, it works again (per git bisect). But this also applies to
> any other arm32 boards which load fitImage in SPL, all of those boards are
> broken in U-Boot 2020.10.

Well if it breaks these boards then we need to do something to fix this. Maybe reverting this patch is a good idea to fix this breakage, especially since others were not running into the same issue as me. But I would like to address the issue I was facing, so we need to figure out how to do something that works for my situation and supports those other boards.

> It seems that the end of the U-Boot image is at 4-byte aligned offset on
> arm32, and that is where the DT is also loaded ; but your patch forces the
> alignment to 8-bytes, so suddenly the DT location is 4-bytes off.

The issue I was facing is that spl_image->size was not a multiple of 4 bytes (alignment depended on the contents of the DT, which got misaligned when it was updated with secure boot information). Maybe an alternative way to address this would be to force alignment in the image generation process (either dtc or mkimage).

Moving image_info.load_addr by a few bytes to ensure alignment should not cause any following code to fail. Maybe the SPL is so close to size limit that wasting the 4 bytes pushes it over the limit?

> > The version I use in production uses 4 byte alignment, but on advice of Tom
> Rini I extended to 8 bytes. Maybe we could switch to just forcing 4 bytes,
> although I can't see why 8 byte would not work?
> >
> > Note also that I was getting SPL data abort crashes on my arm32 target
> when image size was not 4 byte aligned. So reverting this patch will break my
> platform.
> 
> Which platform is that ?

I am using a custom platform which is based off of the clearfog. I have changes to uboot to support our custom hardware, secure boot, etc.

  reply	other threads:[~2020-10-19 22:17 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-10-19 21:40 [PATCH] Revert "Fix data abort caused by mis-aligning FIT data" Marek Vasut
2020-10-19 21:50 ` Reuben Dowle
2020-10-19 21:59   ` Marek Vasut
2020-10-19 22:17     ` Reuben Dowle [this message]
2020-10-19 22:46       ` Marek Vasut
2020-10-19 22:45     ` Tom Rini
2020-10-19 22:54       ` Marek Vasut
2020-10-19 22:58         ` Tom Rini
2020-10-19 23:02           ` Marek Vasut
2020-10-19 23:09             ` Alex G.
2020-10-19 23:13               ` Reuben Dowle
2020-10-20  0:31                 ` Alex G.
2020-10-19 23:02         ` Reuben Dowle
2020-10-19 23:11           ` Marek Vasut
2020-10-20  0:27             ` Reuben Dowle
2020-10-20  9:05               ` Marek Vasut
2020-10-20 14:07                 ` Tom Rini
2020-10-20 14:29                   ` Marek Vasut
2020-10-20 14:32                     ` Tom Rini
2020-10-20 14:38                       ` Alex G.
2020-10-20 15:54                         ` Tom Rini
2020-10-20 17:01                           ` Alex G.
2020-10-20 18:10                             ` Tom Rini
2020-10-21 17:11                               ` Alex G.
2020-10-20 14:42                       ` Marek Vasut
2020-10-21 23:12 ` Tom Rini

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=0d8797cb314a46bfbf0cfa6e465dd63e@4rf.com \
    --to=reuben.dowle@4rf.com \
    --cc=u-boot@lists.denx.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.