Hi Marek, all, On Tue, 2026-02-10 at 14:41 +0100, Marek Vasut wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you > know the content is safe > > On 2/10/26 1:28 PM, Marek Vasut wrote: > > On 2/10/26 1:24 PM, [email protected] wrote: > > > Hi Marek, > > > > > > On Fri, 2026-02-06 at 20:06 +0100, Marek Vasut wrote: > > > > EXTERNAL EMAIL: Do not click links or open attachments unless > > > > you > > > > know the content is safe > > > > > > > > On 2/6/26 4:14 PM, Jamie Gibbons wrote: > > > > > Some platforms > > > > > > > > All platforms, DTs and DTOs have to be 8 Byte aligned, always, > > > > period. > > > Thanks for your feedback. I have realised I misinterpreted Tom's > > > instruction on my previous patch fix about passing a new flag. > > > > > > > > > require device tree overlays (DTOs) to be 8-byte aligned > > > > > when applying them with libfdt. Previously, the code did not > > > > > guarantee > > > > > this alignment, which could result in alignment errors during > > > > > overlay > > > > > application. > > > > > Introduce a new runtime parameter, always_relocate_overlay, > > > > > to > > > > > boot_get_fdt_fit(). When set, overlays are always copied to > > > > > an 8- > > > > > byte > > > > > aligned buffer before being passed to fdt_open_into(). > > > > > This resolves issues with FDT overlay application on > > > > > platforms that > > > > > require strict alignment. > > > > > > > > > > Fixes: 8fbcc0e0e839 ("boot: Assure FDT is always at 8-byte > > > > > aligned > > > > > address") > > > > > > > > > > Signed-off-by: Jamie Gibbons <[email protected]> > > > > > --- > > > > > boot/image-fdt.c | 2 +- > > > > > boot/image-fit.c | 23 +++++++++++++++++------ > > > > > include/image.h | 5 ++++- > > > > > 3 files changed, 22 insertions(+), 8 deletions(-) > > > > > > > > > > diff --git a/boot/image-fdt.c b/boot/image-fdt.c > > > > > index a3a4fb8b558..33fd4248acd 100644 > > > > > --- a/boot/image-fdt.c > > > > > +++ b/boot/image-fdt.c > > > > > @@ -431,7 +431,7 @@ static int select_fdt(struct > > > > > bootm_headers > > > > > *images, const char *select, u8 arch, > > > > > fdt_noffset = > > > > > boot_get_fdt_fit(images, fdt_addr, > > > > > &fit_uname_fdt, > > > > > &fit_uname_config, > > > > > - > > > > > arch, > > > > > &load, &len); > > > > > + > > > > > arch, > > > > > &load, &len, true); > > > > > > > > This flag is always true, do we actually need it ? > > > > > > > > > if (fdt_noffset < 0) > > > > > return -ENOENT; > > > > > diff --git a/boot/image-fit.c b/boot/image-fit.c > > > > > index d2b0a7daccc..a3fa2a01463 100644 > > > > > --- a/boot/image-fit.c > > > > > +++ b/boot/image-fit.c > > > > > @@ -2370,7 +2370,8 @@ int boot_get_setup_fit(struct > > > > > bootm_headers > > > > > *images, uint8_t arch, > > > > > #ifndef USE_HOSTCC > > > > > int boot_get_fdt_fit(struct bootm_headers *images, ulong > > > > > addr, > > > > > const char **fit_unamep, const char > > > > > **fit_uname_configp, > > > > > - int arch, ulong *datap, ulong *lenp) > > > > > + int arch, ulong *datap, ulong *lenp, > > > > > + bool always_relocate_overlay) > > > > > { > > > > > int fdt_noffset, cfg_noffset, count; > > > > > const void *fit; > > > > > @@ -2492,11 +2493,21 @@ int boot_get_fdt_fit(struct > > > > > bootm_headers > > > > > *images, ulong addr, > > > > > ov = map_sysmem(ovload, ovlen); > > > > > > > > > > ovcopylen = ALIGN(fdt_totalsize(ov), SZ_4K); > > > > > - ovcopy = malloc(ovcopylen); > > > > > - if (!ovcopy) { > > > > > - printf("failed to duplicate DTO before > > > > > application\n"); > > > > > - fdt_noffset = -ENOMEM; > > > > > - goto out; > > > > > + if (always_relocate_overlay) { > > > > > + ovcopy = aligned_alloc(8, ovcopylen); > > > > > + if (!ovcopy) { > > > > > + printf("failed to allocate > > > > > aligned > > > > > DTO buffer\n"); > > > > > + fdt_noffset = -ENOMEM; > > > > > + goto out; > > > > > + } > > > > > + memcpy(ovcopy, ov, ovlen); > > > > > + } else { > > > > > + ovcopy = malloc(ovcopylen); > > > > > + if (!ovcopy) { > > > > > + printf("failed to duplicate DTO > > > > > before application\n"); > > > > > + fdt_noffset = -ENOMEM; > > > > > + goto out; > > > > > + } > > > > > > > > Looking at this, if the problem is $ov aligned to 4 Bytes, then > > > > $base > > > > a > > > > bit below could also be aligned to 4 Bytes if there are two or > > > > more > > > > DTs > > > > in the fitImage without external data. Can you please try the > > > > attached > > > > patch and see if that still works ? > > > > > > > > diff --git a/boot/image-fit.c b/boot/image-fit.c > > > > index 3ed69b5f7bc..845e769a7e3 100644 > > > > --- a/boot/image-fit.c > > > > +++ b/boot/image-fit.c > > > > @@ -2368,6 +2368,48 @@ int boot_get_setup_fit(struct > > > > bootm_headers > > > > *images, uint8_t arch, > > > > } > > > > > > > > #ifndef USE_HOSTCC > > > > +#ifdef CONFIG_OF_LIBFDT_OVERLAY > > > > +static int boot_get_fdt_fit_into_buffer(ulong loadaddr, ulong > > > > loadlen, > > > > void **fdtdstbuf) > > > > +{ > > > > + void *fdtsrcbuf, *map; > > > > + ulong fdtdstlen; > > > > + int err; > > > > + > > > > + map = map_sysmem(loadaddr, loadlen); > > > > + > > > > + /* Set up source buffer, make sure DT in the buffer is > > > > 8- > > > > Bytes aligned */ > > > > + if (!IS_ALIGNED((uintptr_t)map, 8)) { > > > > + fdtsrcbuf = aligned_alloc(8, loadlen); > > > > + if (!fdtsrcbuf) > > > > + return -ENOMEM; > > > > + memcpy(fdtsrcbuf, map, loadlen); > > > > + } else { > > > > + fdtsrcbuf = map; > > > > + } > > > > + > > > > + /* Set up destination buffer, make sure DT in the > > > > buffer is > > > > 8-Bytes > > > > aligned */ > > > > + fdtdstlen = ALIGN(fdt_totalsize(fdtsrcbuf), SZ_4K); > > > > + fdtdstbuf = aligned_alloc(8, fdtdstlen); > > > > + if (!fdtdstbuf) { > > > > + printf("failed to duplicate DTO before > > > > application\n"); > > > > + if (!IS_ALIGNED((uintptr_t)map, 8)) > > > > + free(fdtsrcbuf); > > > > + return -ENOMEM; > > > > + } > > > > + > > > > + err = fdt_open_into(fdtsrcbuf, *fdtdstbuf, fdtdstlen); > > > > + if (!IS_ALIGNED((uintptr_t)map, 8)) > > > > + free(fdtsrcbuf); > > > > + if (err < 0) { > > > > + printf("failed on fdt_open_into for DTO: %s\n", > > > > + fdt_strerror(err)); > > > > + return err; > > > > + } > > > > + > > > > + return 0; > > > > +}; > > > > +#endif > > > > + > > > > int boot_get_fdt_fit(struct bootm_headers *images, ulong > > > > addr, > > > > const char **fit_unamep, const char > > > > **fit_uname_configp, > > > > int arch, ulong *datap, ulong *lenp) > > > > @@ -2380,7 +2422,7 @@ int boot_get_fdt_fit(struct bootm_headers > > > > *images, > > > > ulong addr, > > > > char *next_config = NULL; > > > > ulong load, len; > > > > #ifdef CONFIG_OF_LIBFDT_OVERLAY > > > > - ulong ovload, ovlen, ovcopylen; > > > > + ulong ovload, ovlen; > > > > const char *uconfig; > > > > const char *uname; > > > > /* > > > > @@ -2391,7 +2433,7 @@ int boot_get_fdt_fit(struct bootm_headers > > > > *images, > > > > ulong addr, > > > > * Instead, let's be lazy and use void *. > > > > */ > > > > char *of_flat_tree; > > > > - void *base, *ov, *ovcopy = NULL; > > > > + void *base, *ovcopy = NULL; > > > > int i, err, noffset, ov_noffset; > > > > #endif > > > > > > > > @@ -2489,17 +2531,7 @@ int boot_get_fdt_fit(struct > > > > bootm_headers > > > > *images, ulong addr, > > > > } > > > > debug("%s loaded at 0x%08lx len=0x%08lx\n", > > > > uname, ovload, ovlen); > > > > - ov = map_sysmem(ovload, ovlen); > > > > - > > > > - ovcopylen = ALIGN(fdt_totalsize(ov), SZ_4K); > > > > - ovcopy = malloc(ovcopylen); > > > > - if (!ovcopy) { > > > > - printf("failed to duplicate DTO before > > > > application\n"); > > > > - fdt_noffset = -ENOMEM; > > > > - goto out; > > > > - } > > > > - > > > > - err = fdt_open_into(ov, ovcopy, ovcopylen); > > > > + err = boot_get_fdt_fit_into_buffer(ovload, > > > > ovlen, > > > > &ovcopy); > > > > if (err < 0) { > > > > printf("failed on fdt_open_into for > > > > DTO: > > > > %s\n", > > > > fdt_strerror(err)); > > > > @@ -2507,8 +2539,7 @@ int boot_get_fdt_fit(struct bootm_headers > > > > *images, > > > > ulong addr, > > > > goto out; > > > > } > > > > > > > > - base = map_sysmem(load, len + ovlen); > > > > - err = fdt_open_into(base, base, len + ovlen); > > > > + err = boot_get_fdt_fit_into_buffer(load, len + > > > > ovlen, > > > > &base); > > > > if (err < 0) { > > > > printf("failed on fdt_open_into: > > > > %s\n", > > > > fdt_strerror(err)); > > > > -- > > > > 2.51.0 > > > Thanks for the suggestion. I tested your patch and found that, > > > although > > > overlays are applied to aligned buffers, the CPU OPP overlay does > > > not > > > appear in the final device tree in Linux. With my previous patch > > > above > > > (the alignment fix), the OPP overlay works. It looks like the new > > > buffer management breaks overlay chaining or the final DT pointer > > > isn’t > > > updated correctly, so overlays aren’t present in the running > > > device > > > tree. > > > Let me know if you have suggestions for tracking the buffer or > > > chaining > > > overlays differently. > > Maybe some memcpy is missing there somewhere ? Can you please have > > a > > look and debug this a bit ? > > Also, can you reproduce this in sandbox ? Can you share such a > reproducer ?
This patch is no longer needed as the issue this was working to fix has been fixed by James' patch: [PATCH v4 1/1] image: fit: Apply overlays using aligned writable FDT copies. Tested and confirmed.
