On 5/5/20 8:06 PM, Tom Rini wrote:
On Tue, May 05, 2020 at 07:59:24PM +0200, Marek Vasut wrote:
On 5/5/20 7:55 PM, Tom Rini wrote:
On Tue, May 05, 2020 at 07:53:42PM +0200, Marek Vasut wrote:
On 5/5/20 7:50 PM, Tom Rini wrote:
On Tue, May 05, 2020 at 06:39:58PM +0200, Marek Vasut wrote:
On 5/5/20 6:37 PM, Alex Kiernan wrote: > On Tue, May 5, 2020 at 2:28 PM Marek Vasut marex@denx.de wrote: >> >> On 5/5/20 3:22 PM, Alex Kiernan wrote: >>> On Mon, May 4, 2020 at 12:28 PM Tom Rini trini@konsulko.com wrote: >>>> >>>> On Fri, May 01, 2020 at 05:40:25PM +0200, Marek Vasut wrote: >>>> >>>>> There is no reason to tail-pad fitImage with external data to 4-bytes, >>>>> while fitImage without external data does not have any such padding and >>>>> is often unaligned. DT spec also does not mandate any such padding. >>>>> >>>>> Moreover, the tail-pad fills the last few bytes with uninitialized data, >>>>> which could lead to a potential information leak. >>>>> >>>>> $ echo -n xy > /tmp/data ; \ >>>>> ./tools/mkimage -E -f auto -d /tmp/data /tmp/fitImage ; \ >>>>> hexdump -vC /tmp/fitImage | tail -n 3 >>>>> >>>>> before: >>>>> 00000260 61 2d 6f 66 66 73 65 74 00 64 61 74 61 2d 73 69 |a-offset.data-si| >>>>> 00000270 7a 65 00 00 78 79 64 64 |ze..xydd| >>>>> ^^ ^^ ^^ >>>>> after: >>>>> 00000260 61 2d 6f 66 66 73 65 74 00 64 61 74 61 2d 73 69 |a-offset.data-si| >>>>> 00000270 7a 65 00 78 79 |ze.xy| >>>>> >>>>> Signed-off-by: Marek Vasut marex@denx.de >>>>> Reviewed-by: Simon Glass sjg@chromium.org >>>>> Cc: Heinrich Schuchardt xypron.glpk@gmx.de >>>>> Cc: Tom Rini trini@konsulko.com >>>> >>>> Applied to u-boot/master, thanks! >>>> >>> >>> This breaks booting on my board (am3352, eMMC boot, FIT u-boot, >>> CONFIG_SPL_LOAD_FIT). Not got any useful diagnostics - if I boot it >>> from eMMC I get nothing at all on the console, if I boot over ymodem >>> it stalls at 420k, before continuing to 460k. My guess is there's some >>> error going to the console at the 420k mark, but obviously it's lost >>> in the ymodem... I have two DTBs in the FIT image, 420k would about >>> align to the point between them. >> >> My bet would be on some padding / unaligned access problem that this >> patch uncovered. Can you take a look ? > > Seems plausible. With this change my external data starts at 0x483 and > everything after it is non-aligned:
Should the beginning of external data be aligned ?
If in U-Boot we revert e8c2d25845c72c7202a628a97d45e31beea40668 does the problem go away? If so, that's not a fix outright, it means we need to dig back in to the libfdt thread and find the "make this work without killing performance everywhere all the time" option.
Still, my question is, should the beginning of external data be aligned ? And if so, to what, 4 bytes like FDT entries OR 8 bytes to cater for arm64/rv64 ?
Is "external data" the kernel in this case? If so, I swear Linux mandates 8 byte alignment for arm32 as well.
External data can be anything, and if it is supposed to be 8 bytes, we already failed at that since forever.
I would be entirely unsurprised at things working through a combination of luck and co-incidence in our previous padding working out. So, what typically is "external data" in this context?
Anything you can bundle into the fitImage, -E just puts the content at the end of the fitImage and places a reference into the fitImage itself.