Hi Mark, On Fri, 30 Dec 2022 at 12:11, Mark Kettenis <mark.kette...@xs4all.nl> wrote: > > > From: Simon Glass <s...@chromium.org> > > Date: Fri, 30 Dec 2022 11:51:05 -0600 > > > > Hi Pali, > > > > On Fri, 30 Dec 2022 at 09:32, Pali Rohár <p...@kernel.org> wrote: > > > > > > On Thursday 22 December 2022 10:49:58 Tom Rini wrote: > > > > On Fri, Dec 16, 2022 at 01:13:07AM +0100, Pali Rohár wrote: > > > > > On Thursday 15 December 2022 06:24:16 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. > > > > > > > > > > Hello! I do not fully agree that this line is needed. > > > > > > > > > > The whole issue is about DTB binary and its offset. So code/Makefile > > > > > which construct u-boot.bin should be fixed and not u-boot-nodtb.bin. > > > > > > > > We should indeed not force u-boot-nodtb.bin itself to be an 8 byte > > > > aligned size, yes. > > > > > > > > > > But it would be better if we could have the linker scripts > > > > > > fill bytes out to the required alignment. Is that possible? > > > > > > > > > > I already investigated this. LD linker and objcopy (at least older > > > > > version in Debian 10) drops trailing zero bytes in raw binary output. > > > > > > > > > > You can specify zero bytes in the linker script and they are filled in > > > > > ELF or COFF output. But not in raw binary, which u-boot.bin is. > > > > > > > > > > So it could be possible to extract "correct" size from ELF binary and > > > > > call truncate on raw binary generated from objcopy. > > > > > > > > > > > > > > > > > > > > I already hit this trailing-zeros issue for powerpc and I fixed it in: > > > > > 7696b80ec5e9 powerpc: mpc85xx: Fix loading U-Boot proper from SD card > > > > > in SPL > > > > > b898f6a6db76 powerpc: mpc85xx: Fix CONFIG_OF_SEPARATE support for NOR > > > > > booting > > > > > e8c0e0064c8a powerpc: mpc85xx: Fix CONFIG_OF_SEPARATE support > > > > > > > > > > All those commits re-align output to just 4 bytes, not more. > > > > > > > > This however needs a follow-up. By spec, device tree binaries must > > > > start at an 8 byte aligned address, not 4. Yes, 4 is clearly functional > > > > in this case, but when we update to a newer libdtc that finally enforces > > > > this check we'll have a problem. > > > > > > In https://lore.kernel.org/u-boot/20221217235913.w7ihsktbplbp2j7z@pali/ > > > I proposed how to extend Makefile rule to generate u-boot-nodtb.bin with > > > all required paddings. However this (enforcing u-boot-nodtb.bin to be 8 > > > byte aligned) is not we think that is the correct way. But I do not have > > > any other option yet how to solve this problem. > > > > See the other thread [1] on this > > > > > > > > I think that the source of this issue is that we are not putting DTB > > > binary into ELF binary (so objcopy would be able to easily generate > > > final u-boot.bin). And instead trying to simulate joining more binary > > > files (u-boot-nodtb.bin and DTB binary) into one _without_ information > > > how to do it correctly. But we already have this information in linker > > > script and also in ELF binary. > > > > The DTB can come later, during packaging, or may be updated in that > > step. Or there may multiple DTBs and SPL can decide which to use. > > Overall the DTB is considered separate from the code by design, so > > that it is possible to build one U-Boot that supports multiple boards. > > > > That is why we don't allow DTB in the ELF. > > Repeating that statement doesn't make it true. We have an option for > this (OF_EMBED) and that option is legitimately used by several > "boards" in cases where U-Boot is used as in combination with > "firmware" that expects a valid ELF binary as its payload.
We have this problem with EFI, and the approach is to embed an ELF 'stub' with a u-boot.bin payload. But a better way would be to update that 'firmware' to pass the DTB to U-Boot using standard passage. > > That said, I do think that OF_SEPARATE should be used whenever > possible. And in that case it should be the tool that combines the > various components into a single binary that makes sure the DTB is > properly aligned. OK good. That seems to be where we are heading. Regards, Simon