[PATCH] Revert "Fix data abort caused by mis-aligning FIT data"

[Marek Vasut]

On 10/20/20 12:17 AM, Reuben Dowle wrote:

[...]

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

The patch you posted aligns the load address to 8 bytes, not to 4 bytes.
So why didn't the ALIGN use 4 byte alignment ?

> Maybe an alternative way to address this would be to force alignment in the image generation process (either dtc or mkimage).

mkimage -E / -B already can do that for you.

> 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?

No. If the DT which is supposed to be at the end of the
spl_image->load_addr + spl_image->size is suddenly a few bytes off (due
to the ALIGN()), then it is inaccessible and the boot fails. The
spl_image->load_addr + spl_image->size should already be 4 byte aligned.

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

So, are you triggering the problem with custom-patched U-Boot, not with
mainline ? Could it be that some of the custom patches triggered the
problem instead ?