Hi Marek, On Thu, 13 Nov 2025 at 14:57, Marek Vasut <[email protected]> wrote: > > On 11/13/25 8:33 PM, Simon Glass wrote: > > Hello Simon, > > >> The fitImage may contain FDT at 4-byte aligned address, because alignment > >> of DT tags is 4 bytes. However, libfdt and also Linux expects DT to be at > >> 8-byte aligned address. Make sure that the DTs embedded in fitImages are > >> always used from 8-byte aligned addresses. In case the DT is decompressed, > >> make sure the target buffer is 8-byte aligned. In case the DT is only > >> loaded, make sure the target buffer is 8-byte aligned too. > >> > >> Signed-off-by: Marek Vasut <[email protected]> > >> --- > >> Cc: Heinrich Schuchardt <[email protected]> > >> Cc: Quentin Schulz <[email protected]> > >> Cc: Simon Glass <[email protected]> > >> Cc: Tom Rini <[email protected]> > >> Cc: Wolfgang Wallner <[email protected]> > >> Cc: [email protected] > >> --- > >> boot/image-fit.c | 9 +++++++-- > >> 1 file changed, 7 insertions(+), 2 deletions(-) > >> > >> diff --git a/boot/image-fit.c b/boot/image-fit.c > >> index 2f2d3e9304d..cccaa48f683 100644 > >> --- a/boot/image-fit.c > >> +++ b/boot/image-fit.c > >> @@ -23,7 +23,6 @@ > >> #include <log.h> > >> #include <mapmem.h> > >> #include <asm/io.h> > >> -#include <malloc.h> > >> #include <memalign.h> > >> #include <asm/global_data.h> > >> #ifdef CONFIG_DM_HASH > >> @@ -36,6 +35,7 @@ DECLARE_GLOBAL_DATA_PTR; > >> #include <bootm.h> > >> #include <image.h> > >> #include <bootstage.h> > >> +#include <malloc.h> > >> #include <upl.h> > >> #include <u-boot/crc.h> > >> > >> @@ -2279,7 +2279,7 @@ int fit_image_load(struct bootm_headers *images, > >> ulong addr, > >> > >> log_debug("decompressing image\n"); > >> if (load == data) { > >> - loadbuf = malloc(max_decomp_len); > >> + loadbuf = memalign(8, max_decomp_len); > >> load = map_to_sysmem(loadbuf); > >> } else { > >> loadbuf = map_sysmem(load, max_decomp_len); > >> @@ -2291,6 +2291,11 @@ int fit_image_load(struct bootm_headers *images, > >> ulong addr, > >> return -ENOEXEC; > >> } > >> len = load_end - load; > >> + } else if (load_op != FIT_LOAD_IGNORED && image_type == > >> IH_TYPE_FLATDT && > >> + ((uintptr_t)buf & 7)) { > >> + loadbuf = memalign(8, len); > >> + load = map_to_sysmem(loadbuf); > >> + memcpy(loadbuf, buf, len); > >> } else if (load != data) { > >> log_debug("copying\n"); > >> loadbuf = map_sysmem(load, len); > >> -- > >> 2.51.0 > >> > > > > We really can't do a memory allocation in fit_image_load() ! > > We already do, notice the malloc() when "decompressing image" part. > > > The problem should be fixed higher up, in its callers, etc. For one > > thing, the caller knows whether it is a DT or not, so putting this > > logic here is messy. See boot_get_fdt_fit() > > This is triggered by a fitImage with DT embedded in that fitImage as > non-external-data. That DT is at 4-byte aligned address, so how can the > caller deal with it ? The caller gets a fitImage, which itself is at > 8-byte aligned address, but the DT in it is not at 8-byte aligned > address, so that DT has to be relocated somehow and that happens here.
OK, but don't put the code in here...see boot_get_fdt_fit(). But even then, why does it matter where the FDT is? We are going to use fdt_openinto() at some point and put it elsewhere, right, so we can do pre-boot fixups? Perhaps we should deprecate FITs with internal data, too? Regards, Simon
