| Message ID | 20250512161033.146396-1-marek.vasut@mailbox.org |
|---|---|
| State | Superseded |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | ARM: Align image end to 8 bytes to fit DT alignment | expand |
Hi Marek, On 5/12/25 6:10 PM, Marek Vasut wrote: > Align U-Boot image end to 8 bytes to make sure DT alignment requirement > is fulfilled. This fixes a possible failure in fdt_find_separate() in > case the U-Boot image is aligned to 4 Bytes and DT is appended at the > end at already 8 Byte aligned offset. > > Signed-off-by: Marek Vasut <marek.vasut@mailbox.org> > --- > Cc: Ilias Apalodimas <ilias.apalodimas@linaro.org> > Cc: Sam Edwards <cfsworks@gmail.com> > Cc: Tom Rini <trini@konsulko.com> > Cc: u-boot@lists.denx.de > --- > arch/arm/cpu/u-boot.lds | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/arch/arm/cpu/u-boot.lds b/arch/arm/cpu/u-boot.lds > index 817e7a983ae..b2109e03ce3 100644 > --- a/arch/arm/cpu/u-boot.lds > +++ b/arch/arm/cpu/u-boot.lds > @@ -153,14 +153,14 @@ SECTIONS > __efi_runtime_rel_stop = .; > } > > - . = ALIGN(4); > + . = ALIGN(8); > __image_copy_end = .; > > /* > * if CONFIG_USE_ARCH_MEMSET is not selected __bss_end - __bss_start > * needs to be a multiple of 4 and we overlay .bss with .rel.dyn > */ While 8 is a multiple of 4, it's now a bit confusing to have this comment just above the following line. Maybe you can update it? Cheers, Quentin
Hi Marek, On Mon, 12 May 2025 at 18:11, Marek Vasut <marek.vasut@mailbox.org> wrote: > > Align U-Boot image end to 8 bytes to make sure DT alignment requirement > is fulfilled. This fixes a possible failure in fdt_find_separate() in > case the U-Boot image is aligned to 4 Bytes and DT is appended at the > end at already 8 Byte aligned offset. > > Signed-off-by: Marek Vasut <marek.vasut@mailbox.org> > --- > Cc: Ilias Apalodimas <ilias.apalodimas@linaro.org> > Cc: Sam Edwards <cfsworks@gmail.com> > Cc: Tom Rini <trini@konsulko.com> > Cc: u-boot@lists.denx.de > --- > arch/arm/cpu/u-boot.lds | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/arch/arm/cpu/u-boot.lds b/arch/arm/cpu/u-boot.lds > index 817e7a983ae..b2109e03ce3 100644 > --- a/arch/arm/cpu/u-boot.lds > +++ b/arch/arm/cpu/u-boot.lds > @@ -153,14 +153,14 @@ SECTIONS > __efi_runtime_rel_stop = .; > } > > - . = ALIGN(4); > + . = ALIGN(8); > __image_copy_end = .; > > /* > * if CONFIG_USE_ARCH_MEMSET is not selected __bss_end - __bss_start > * needs to be a multiple of 4 and we overlay .bss with .rel.dyn > */ > - .rel.dyn ALIGN(4) : { > + .rel.dyn ALIGN(8) : { > __rel_dyn_start = .; > *(.rel*) > __rel_dyn_end = .; > -- > 2.47.2 > Reviewed-by: Simon Glass <sjg@chromium.org> Does the same need apply to SPL? Regards, Simon
On Mon, May 12, 2025 at 06:10:28PM +0200, Marek Vasut wrote: > Align U-Boot image end to 8 bytes to make sure DT alignment requirement > is fulfilled. This fixes a possible failure in fdt_find_separate() in > case the U-Boot image is aligned to 4 Bytes and DT is appended at the > end at already 8 Byte aligned offset. > > Signed-off-by: Marek Vasut <marek.vasut@mailbox.org> > --- > Cc: Ilias Apalodimas <ilias.apalodimas@linaro.org> > Cc: Sam Edwards <cfsworks@gmail.com> > Cc: Tom Rini <trini@konsulko.com> > Cc: u-boot@lists.denx.de Link: https://source.denx.de/u-boot/u-boot/-/issues/30 That mentioned, we should stop waiting for the solution that covers all cases and at least get more cases right. Reviewed-by: Tom Rini <trini@konsulko.com>
On 5/12/25 6:50 PM, Quentin Schulz wrote: > Hi Marek, > > On 5/12/25 6:10 PM, Marek Vasut wrote: >> Align U-Boot image end to 8 bytes to make sure DT alignment requirement >> is fulfilled. This fixes a possible failure in fdt_find_separate() in >> case the U-Boot image is aligned to 4 Bytes and DT is appended at the >> end at already 8 Byte aligned offset. >> >> Signed-off-by: Marek Vasut <marek.vasut@mailbox.org> >> --- >> Cc: Ilias Apalodimas <ilias.apalodimas@linaro.org> >> Cc: Sam Edwards <cfsworks@gmail.com> >> Cc: Tom Rini <trini@konsulko.com> >> Cc: u-boot@lists.denx.de >> --- >> arch/arm/cpu/u-boot.lds | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/arch/arm/cpu/u-boot.lds b/arch/arm/cpu/u-boot.lds >> index 817e7a983ae..b2109e03ce3 100644 >> --- a/arch/arm/cpu/u-boot.lds >> +++ b/arch/arm/cpu/u-boot.lds >> @@ -153,14 +153,14 @@ SECTIONS >> __efi_runtime_rel_stop = .; >> } >> - . = ALIGN(4); >> + . = ALIGN(8); >> __image_copy_end = .; >> /* >> * if CONFIG_USE_ARCH_MEMSET is not selected __bss_end - >> __bss_start >> * needs to be a multiple of 4 and we overlay .bss with .rel.dyn >> */ > > While 8 is a multiple of 4, it's now a bit confusing to have this > comment just above the following line. Maybe you can update it? Fixed in V2, thanks for spotting this.
On 5/15/25 9:27 PM, Simon Glass wrote: > Hi Marek, > > On Mon, 12 May 2025 at 18:11, Marek Vasut <marek.vasut@mailbox.org> wrote: >> >> Align U-Boot image end to 8 bytes to make sure DT alignment requirement >> is fulfilled. This fixes a possible failure in fdt_find_separate() in >> case the U-Boot image is aligned to 4 Bytes and DT is appended at the >> end at already 8 Byte aligned offset. >> >> Signed-off-by: Marek Vasut <marek.vasut@mailbox.org> >> --- >> Cc: Ilias Apalodimas <ilias.apalodimas@linaro.org> >> Cc: Sam Edwards <cfsworks@gmail.com> >> Cc: Tom Rini <trini@konsulko.com> >> Cc: u-boot@lists.denx.de >> --- >> arch/arm/cpu/u-boot.lds | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/arch/arm/cpu/u-boot.lds b/arch/arm/cpu/u-boot.lds >> index 817e7a983ae..b2109e03ce3 100644 >> --- a/arch/arm/cpu/u-boot.lds >> +++ b/arch/arm/cpu/u-boot.lds >> @@ -153,14 +153,14 @@ SECTIONS >> __efi_runtime_rel_stop = .; >> } >> >> - . = ALIGN(4); >> + . = ALIGN(8); >> __image_copy_end = .; >> >> /* >> * if CONFIG_USE_ARCH_MEMSET is not selected __bss_end - __bss_start >> * needs to be a multiple of 4 and we overlay .bss with .rel.dyn >> */ >> - .rel.dyn ALIGN(4) : { >> + .rel.dyn ALIGN(8) : { >> __rel_dyn_start = .; >> *(.rel*) >> __rel_dyn_end = .; >> -- >> 2.47.2 >> > > Reviewed-by: Simon Glass <sjg@chromium.org> > > Does the same need apply to SPL? Yes, although in a slightly different manner. For SPL, the alignment has to be applied to __image_binary_end and __bss_end , see lib/fdtdec.c : 1243 static void *fdt_find_separate(void) 1244 { 1245 void *fdt_blob = NULL; 1246 1247 if (IS_ENABLED(CONFIG_SANDBOX)) 1248 return NULL; 1249 1250 #ifdef CONFIG_XPL_BUILD 1251 /* FDT is at end of BSS unless it is in a different memory region */ 1252 if (CONFIG_IS_ENABLED(SEPARATE_BSS)) 1253 fdt_blob = (ulong *)_image_binary_end; 1254 else 1255 fdt_blob = (ulong *)__bss_end; 1256 #else Fixed in V2.
diff --git a/arch/arm/cpu/u-boot.lds b/arch/arm/cpu/u-boot.lds index 817e7a983ae..b2109e03ce3 100644 --- a/arch/arm/cpu/u-boot.lds +++ b/arch/arm/cpu/u-boot.lds @@ -153,14 +153,14 @@ SECTIONS __efi_runtime_rel_stop = .; } - . = ALIGN(4); + . = ALIGN(8); __image_copy_end = .; /* * if CONFIG_USE_ARCH_MEMSET is not selected __bss_end - __bss_start * needs to be a multiple of 4 and we overlay .bss with .rel.dyn */ - .rel.dyn ALIGN(4) : { + .rel.dyn ALIGN(8) : { __rel_dyn_start = .; *(.rel*) __rel_dyn_end = .;
Align U-Boot image end to 8 bytes to make sure DT alignment requirement is fulfilled. This fixes a possible failure in fdt_find_separate() in case the U-Boot image is aligned to 4 Bytes and DT is appended at the end at already 8 Byte aligned offset. Signed-off-by: Marek Vasut <marek.vasut@mailbox.org> --- Cc: Ilias Apalodimas <ilias.apalodimas@linaro.org> Cc: Sam Edwards <cfsworks@gmail.com> Cc: Tom Rini <trini@konsulko.com> Cc: u-boot@lists.denx.de --- arch/arm/cpu/u-boot.lds | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)