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.

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.

Reply via email to