Hi Eugen, On Thu, 15 Dec 2022 at 06:37, <eugen.hris...@microchip.com> wrote: > > On 12/15/22 16:24, Simon Glass wrote: > > Hi Eugen, > > > > On Thu, 15 Dec 2022 at 03:58, Eugen Hristev <eugen.hris...@microchip.com> > > wrote: > >> > >> Newer DTC require that the DTB start address is aligned at 8 bytes. > >> In the u-boot.bin case, the u-boot-nodtb.bin is concatenated with the > >> DTB, but there is no alignment/padding to the next 8byte aligned address. > >> This causes the board to fail booting, because the FDT will claim > >> that the DTB inside u-boot.bin is not a valid DTB, it will fail with > >> -FDT_ERR_ALIGNMENT. > >> To solve this, have the u-boot binary `_end` aligned with 8 bytes. > >> The objcopy in the Makefile will create the u-boot-nodtb.bin and it has to > >> be truncated to 8 bytes to correspond to the u-boot.map file which will > >> have the `_end` aligned to 8 bytes. > >> The lds files which use devicetrees have been changed to align the `_end` > >> tag with 8 bytes. > >> > >> This patch is also a prerequisite to have the possibility to update the > >> dtc inside u-boot to newer versions (1.6.1+) > >> > >> Signed-off-by: Eugen Hristev <eugen.hris...@microchip.com> > >> --- > >> Hi, > >> > >> I could not test all affected boards, it's an impossible task. > >> I tried this on at91 boards which I have, and ran the CI on denx. > >> I cannot guarantee that no other boards are affected, so this patch is a > >> bit > >> of an RFC. > >> If the u-boot-nodtb.bin does not have the size equal with the corresponding > >> one in u-boot.map, the binary_size_check will fail at build time with > >> something like this: > >> > >> u-boot.map shows a binary size of 502684 > >> but u-boot-nodtb.bin shows 502688 > >> > >> Thanks, > >> Eugen > >> > >> Makefile | 2 ++ > >> arch/arm/cpu/armv8/u-boot.lds | 4 ++-- > >> arch/arm/cpu/u-boot-spl.lds | 1 + > >> arch/arm/cpu/u-boot.lds | 1 + > >> arch/arm/lib/elf_arm_efi.lds | 5 +++++ > >> arch/arm/mach-at91/arm926ejs/u-boot-spl.lds | 2 +- > >> arch/arm/mach-at91/armv7/u-boot-spl.lds | 2 +- > >> arch/arm/mach-zynq/u-boot-spl.lds | 2 +- > >> arch/mips/cpu/u-boot.lds | 2 +- > >> arch/sandbox/cpu/u-boot.lds | 6 ++++++ > >> arch/sh/cpu/u-boot.lds | 2 ++ > >> board/ti/am335x/u-boot.lds | 1 + > >> tools/binman/test/u_boot_binman_embed.lds | 2 +- > >> 13 files changed, 25 insertions(+), 7 deletions(-) > >> > >> diff --git a/Makefile b/Makefile > >> index 9d84f96481..b4d387bcce 100644 > >> --- a/Makefile > >> +++ b/Makefile > >> @@ -1317,6 +1317,8 @@ endif > >> > >> u-boot-nodtb.bin: u-boot FORCE > >> $(call if_changed,objcopy_uboot) > >> +# Make sure the size is 8 byte-aligned. > >> + @truncate -s %8 $@ > >> $(BOARD_SIZE_CHECK) > > > > I agree this line is needed, since otherwise we will only get 4-byte > > alignment. But it would be better if we could have the linker scripts > > fill bytes out to the required alignment. Is that possible? > > > > Reviewed-by: Simon Glass <s...@chromium.org> > > > > Regards, > > Simon > > > Hi Simon, > > I tried to check the objcopy option --pad-to , to use it at the time of > objcopy, but this requires a real number to be passed to it. > And this number could only be found by inspecting the u-boot.map file, > since u-boot-nodtb.bin still does not exist. > And if we pad to the size specified in u-boot.map, then > binary_size_check does not make much sense anymore, as we will basically > use the same information to fit the file, and it will always pass with a > success. (even if we would pad many more bytes than 4 ) > Hence it would lose it's purpose ( binary_size_check ), which I think > was created to make sure no objects were lost when doing objcopy and > creating the u-boot-nodtb.bin file.
Yes, I was more thinking of something like: fill { . = ALIGN(8); QUAD(0) }; in the link script, or something that actually writes the padding bytes. > > On a side note, do you think I covered all the implied lds files ? I > would hate to break someone's boards. If CI passes you should be able to rely on the binary size check. > > And also, P.S. : I would require to have the same change when building a > FIT image with mkimage... all subimages inside a FIT must be aligned to > 8 bytes. However mkimage only aligns the start address and header of the > FIT (-B option). Out of your knowledge, is this possible and where could > I have a look to do this change ? I lean towards the view that this 8-byte alignment is a bad idea. Does libfdt itself support this sort of thing? We'll need to be able to tell it to align the value of a property to 8 bytes, both in the sw interface and the normal one. For binman to support it this is pretty easy once the sw interface is updated - see _add_node() in fit.py. For mkimage to support it, see fit_import_data(). Perhaps need a libfdt function like fdt_setprop_align()...? Regards, Simon