[PATCH] lds: align u-boot-nodtb with 8 bytes boundary

Simon Glass sjg at chromium.org
Fri Dec 30 18:51:05 CET 2022 

Hi Pali,

On Fri, 30 Dec 2022 at 09:32, Pali Rohár <pali at 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.hristev at 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.hristev at 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.

Regards,
Simon

[1] https://lore.kernel.org/u-boot/CAPnjgZ1Vho5-sNMmPeU_9hCsGHK9pSGbYYnrQVxkB5D5R=biXw@mail.gmail.com/


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

More information about the U-Boot mailing list