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 ?
