* [PATCH v3 1/3] imx8: ls1028a: Drop raw image support @ 2021-08-01 20:59 Simon Glass 2021-08-01 20:59 ` [PATCH v3 2/3] arm: Add an __image_copy_start symbol for ARMv8 Simon Glass 2021-08-01 20:59 ` [PATCH v3 3/3] binman: Show an error if __image_copy_start is missing Simon Glass 0 siblings, 2 replies; 8+ messages in thread From: Simon Glass @ 2021-08-01 20:59 UTC (permalink / raw) To: U-Boot Mailing List; +Cc: Tim Harvey, Scott Wood, Simon Glass, Michael Walle The CONFIG_SPL_RAW_IMAGE_SUPPORT option requires that binman provides an offset for the image (see spl_set_header_raw_uboot()), if binman is used. These boards use FIT to store U-Boot, so raw image support is not used. Drop this option to avoid errors once binman starts checking this. Signed-off-by: Simon Glass <sjg@chromium.org> Acked-by: Michael Walle <michael@walle.cc> # for kontron_sl28 --- Changes in v3: - Add ls1028a tag since kontron_sl28 is not imx8 Changes in v2: - Add a new patch to drop raw image support for some imx8 boards configs/imx8mm-cl-iot-gate_defconfig | 1 + configs/imx8mm_evk_defconfig | 1 + configs/imx8mn_ddr4_evk_defconfig | 1 + configs/imx8mp_evk_defconfig | 1 + configs/imx8mq_cm_defconfig | 1 + configs/kontron_sl28_defconfig | 1 + configs/phycore-imx8mp_defconfig | 1 + 7 files changed, 7 insertions(+) diff --git a/configs/imx8mm-cl-iot-gate_defconfig b/configs/imx8mm-cl-iot-gate_defconfig index 79e4bde0703..7a7f9413dbc 100644 --- a/configs/imx8mm-cl-iot-gate_defconfig +++ b/configs/imx8mm-cl-iot-gate_defconfig @@ -30,6 +30,7 @@ CONFIG_OF_SYSTEM_SETUP=y CONFIG_SYS_EXTRA_OPTIONS="IMX_CONFIG=board/compulab/imx8mm-cl-iot-gate/imximage-8mm-lpddr4.cfg" CONFIG_BOARD_LATE_INIT=y CONFIG_SPL_BOARD_INIT=y +# CONFIG_SPL_RAW_IMAGE_SUPPORT is not set CONFIG_SPL_SEPARATE_BSS=y CONFIG_SPL_I2C=y CONFIG_SPL_POWER=y diff --git a/configs/imx8mm_evk_defconfig b/configs/imx8mm_evk_defconfig index f7f39b8dc63..8862e10ea57 100644 --- a/configs/imx8mm_evk_defconfig +++ b/configs/imx8mm_evk_defconfig @@ -27,6 +27,7 @@ CONFIG_OF_SYSTEM_SETUP=y CONFIG_SYS_EXTRA_OPTIONS="IMX_CONFIG=board/freescale/imx8mm_evk/imximage-8mm-lpddr4.cfg" CONFIG_BOARD_LATE_INIT=y CONFIG_SPL_BOARD_INIT=y +# CONFIG_SPL_RAW_IMAGE_SUPPORT is not set CONFIG_SPL_SEPARATE_BSS=y CONFIG_SPL_I2C=y CONFIG_SPL_POWER=y diff --git a/configs/imx8mn_ddr4_evk_defconfig b/configs/imx8mn_ddr4_evk_defconfig index 78943dd91d3..ee9960e3cee 100644 --- a/configs/imx8mn_ddr4_evk_defconfig +++ b/configs/imx8mn_ddr4_evk_defconfig @@ -30,6 +30,7 @@ CONFIG_DEFAULT_FDT_FILE="imx8mn-ddr4-evk.dtb" CONFIG_BOARD_LATE_INIT=y CONFIG_SPL_BOARD_INIT=y CONFIG_SPL_BOOTROM_SUPPORT=y +# CONFIG_SPL_RAW_IMAGE_SUPPORT is not set CONFIG_SPL_SEPARATE_BSS=y CONFIG_SPL_I2C=y CONFIG_SPL_WATCHDOG=y diff --git a/configs/imx8mp_evk_defconfig b/configs/imx8mp_evk_defconfig index 2c6fc16cdf5..b39333eff7e 100644 --- a/configs/imx8mp_evk_defconfig +++ b/configs/imx8mp_evk_defconfig @@ -31,6 +31,7 @@ CONFIG_BOARD_EARLY_INIT_F=y CONFIG_BOARD_LATE_INIT=y CONFIG_SPL_BOARD_INIT=y CONFIG_SPL_BOOTROM_SUPPORT=y +# CONFIG_SPL_RAW_IMAGE_SUPPORT is not set CONFIG_SPL_SYS_MALLOC_SIMPLE=y CONFIG_SPL_SEPARATE_BSS=y CONFIG_SPL_I2C=y diff --git a/configs/imx8mq_cm_defconfig b/configs/imx8mq_cm_defconfig index e0a038b168c..ab02312c143 100644 --- a/configs/imx8mq_cm_defconfig +++ b/configs/imx8mq_cm_defconfig @@ -25,6 +25,7 @@ CONFIG_OF_SYSTEM_SETUP=y CONFIG_SYS_EXTRA_OPTIONS="IMX_CONFIG=board/ronetix/imx8mq-cm/imximage-8mq-lpddr4.cfg" CONFIG_BOARD_LATE_INIT=y CONFIG_SPL_BOARD_INIT=y +# CONFIG_SPL_RAW_IMAGE_SUPPORT is not set CONFIG_SPL_SEPARATE_BSS=y CONFIG_SPL_I2C=y CONFIG_SYS_PROMPT="u-boot=> " diff --git a/configs/kontron_sl28_defconfig b/configs/kontron_sl28_defconfig index 29a45ec54bc..3868d7c39b0 100644 --- a/configs/kontron_sl28_defconfig +++ b/configs/kontron_sl28_defconfig @@ -31,6 +31,7 @@ CONFIG_USE_BOOTARGS=y CONFIG_BOARD_LATE_INIT=y CONFIG_PCI_INIT_R=y CONFIG_SPL_BOARD_INIT=y +# CONFIG_SPL_RAW_IMAGE_SUPPORT is not set CONFIG_SPL_SEPARATE_BSS=y CONFIG_SPL_MPC8XXX_INIT_DDR_SUPPORT=y CONFIG_SPL_SPI_LOAD=y diff --git a/configs/phycore-imx8mp_defconfig b/configs/phycore-imx8mp_defconfig index 84a0a5cbaf2..e90c583c85c 100644 --- a/configs/phycore-imx8mp_defconfig +++ b/configs/phycore-imx8mp_defconfig @@ -27,6 +27,7 @@ CONFIG_DEFAULT_FDT_FILE="oftree" CONFIG_BOARD_LATE_INIT=y CONFIG_SPL_BOARD_INIT=y CONFIG_SPL_BOOTROM_SUPPORT=y +# CONFIG_SPL_RAW_IMAGE_SUPPORT is not set CONFIG_SPL_SYS_MALLOC_SIMPLE=y CONFIG_SPL_SEPARATE_BSS=y CONFIG_SPL_I2C=y -- 2.32.0.554.ge1b32706d8-goog ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v3 2/3] arm: Add an __image_copy_start symbol for ARMv8 2021-08-01 20:59 [PATCH v3 1/3] imx8: ls1028a: Drop raw image support Simon Glass @ 2021-08-01 20:59 ` Simon Glass 2021-08-04 19:52 ` Scott Wood 2021-08-01 20:59 ` [PATCH v3 3/3] binman: Show an error if __image_copy_start is missing Simon Glass 1 sibling, 1 reply; 8+ messages in thread From: Simon Glass @ 2021-08-01 20:59 UTC (permalink / raw) To: U-Boot Mailing List Cc: Tim Harvey, Scott Wood, Simon Glass, Albert Aribaud, Tom Rini This symbol is needed for binman to locate the start of the image. Add it. Note: the existing line to bring in the .__image_copy_start symbol does not appear to do anything. Signed-off-by: Simon Glass <sjg@chromium.org> --- I have copied Scott Wood who originally added the line about the __image_copy_start in the hope that he can decide if we should remove it. (no changes since v2) Changes in v2: - Add new patch to add an __image_copy_start symbol for ARMv8 arch/arm/cpu/armv8/u-boot-spl.lds | 1 + 1 file changed, 1 insertion(+) diff --git a/arch/arm/cpu/armv8/u-boot-spl.lds b/arch/arm/cpu/armv8/u-boot-spl.lds index 9edb662b094..2827a07590d 100644 --- a/arch/arm/cpu/armv8/u-boot-spl.lds +++ b/arch/arm/cpu/armv8/u-boot-spl.lds @@ -22,6 +22,7 @@ ENTRY(_start) SECTIONS { .text : { + __image_copy_start = .; . = ALIGN(8); *(.__image_copy_start) CPUDIR/start.o (.text*) -- 2.32.0.554.ge1b32706d8-goog ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v3 2/3] arm: Add an __image_copy_start symbol for ARMv8 2021-08-01 20:59 ` [PATCH v3 2/3] arm: Add an __image_copy_start symbol for ARMv8 Simon Glass @ 2021-08-04 19:52 ` Scott Wood 2021-08-05 19:20 ` Simon Glass 0 siblings, 1 reply; 8+ messages in thread From: Scott Wood @ 2021-08-04 19:52 UTC (permalink / raw) To: Simon Glass, U-Boot Mailing List; +Cc: Tim Harvey, Albert Aribaud, Tom Rini On Sun, 2021-08-01 at 14:59 -0600, Simon Glass wrote: > This symbol is needed for binman to locate the start of the image. Add it. > > Note: the existing line to bring in the .__image_copy_start symbol does > not appear to do anything. > > Signed-off-by: Simon Glass <sjg@chromium.org> > --- > I have copied Scott Wood who originally added the line about the > __image_copy_start in the hope that he can decide if we should remove it. It's been a long time since I looked at this stuff, but __image_copy_start is used for relocation and that code does not seem to be in the SPL, so the *(.__image_copy_start) was probably just a copy-and-paste leftover from the main SPL that can go away. Of course, that doesn't resolve the binman issue. :-) > diff --git a/arch/arm/cpu/armv8/u-boot-spl.lds b/arch/arm/cpu/armv8/u-boot- > spl.lds > index 9edb662b094..2827a07590d 100644 > --- a/arch/arm/cpu/armv8/u-boot-spl.lds > +++ b/arch/arm/cpu/armv8/u-boot-spl.lds > @@ -22,6 +22,7 @@ ENTRY(_start) > SECTIONS > { > .text : { > + __image_copy_start = .; > . = ALIGN(8); > *(.__image_copy_start) > CPUDIR/start.o (.text*) If for whatever reason you did need to define the symbol this way, shouldn't it be after the alignment? -Scott ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v3 2/3] arm: Add an __image_copy_start symbol for ARMv8 2021-08-04 19:52 ` Scott Wood @ 2021-08-05 19:20 ` Simon Glass 2021-09-25 15:46 ` Simon Glass 0 siblings, 1 reply; 8+ messages in thread From: Simon Glass @ 2021-08-05 19:20 UTC (permalink / raw) To: Scott Wood; +Cc: U-Boot Mailing List, Tim Harvey, Albert Aribaud, Tom Rini Hi Scott, On Wed, 4 Aug 2021 at 13:53, Scott Wood <oss@buserror.net> wrote: > > On Sun, 2021-08-01 at 14:59 -0600, Simon Glass wrote: > > This symbol is needed for binman to locate the start of the image. Add it. > > > > Note: the existing line to bring in the .__image_copy_start symbol does > > not appear to do anything. > > > > Signed-off-by: Simon Glass <sjg@chromium.org> > > --- > > I have copied Scott Wood who originally added the line about the > > __image_copy_start in the hope that he can decide if we should remove it. > > It's been a long time since I looked at this stuff, but __image_copy_start is > used for relocation and that code does not seem to be in the SPL, so the > *(.__image_copy_start) was probably just a copy-and-paste leftover from the > main SPL that can go away. > > Of course, that doesn't resolve the binman issue. :-) > > > diff --git a/arch/arm/cpu/armv8/u-boot-spl.lds b/arch/arm/cpu/armv8/u-boot- > > spl.lds > > index 9edb662b094..2827a07590d 100644 > > --- a/arch/arm/cpu/armv8/u-boot-spl.lds > > +++ b/arch/arm/cpu/armv8/u-boot-spl.lds > > @@ -22,6 +22,7 @@ ENTRY(_start) > > SECTIONS > > { > > .text : { > > + __image_copy_start = .; > > . = ALIGN(8); > > *(.__image_copy_start) > > CPUDIR/start.o (.text*) > > If for whatever reason you did need to define the symbol this way, shouldn't > it be after the alignment? Well I don't want to miss out any of the image, otherwise the offsets would be off. But perhaps that is another question. Since this is the first section, it should already be aligned (to 16 I suspect). So why do we need it? Regards, Simon ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v3 2/3] arm: Add an __image_copy_start symbol for ARMv8 2021-08-05 19:20 ` Simon Glass @ 2021-09-25 15:46 ` Simon Glass 2021-09-25 22:35 ` Scott Wood 0 siblings, 1 reply; 8+ messages in thread From: Simon Glass @ 2021-09-25 15:46 UTC (permalink / raw) To: Scott Wood; +Cc: U-Boot Mailing List, Tim Harvey, Albert Aribaud, Tom Rini Hi Scott, On Thu, 5 Aug 2021 at 13:20, Simon Glass <sjg@chromium.org> wrote: > > Hi Scott, > > On Wed, 4 Aug 2021 at 13:53, Scott Wood <oss@buserror.net> wrote: > > > > On Sun, 2021-08-01 at 14:59 -0600, Simon Glass wrote: > > > This symbol is needed for binman to locate the start of the image. Add it. > > > > > > Note: the existing line to bring in the .__image_copy_start symbol does > > > not appear to do anything. > > > > > > Signed-off-by: Simon Glass <sjg@chromium.org> > > > --- > > > I have copied Scott Wood who originally added the line about the > > > __image_copy_start in the hope that he can decide if we should remove it. > > > > It's been a long time since I looked at this stuff, but __image_copy_start is > > used for relocation and that code does not seem to be in the SPL, so the > > *(.__image_copy_start) was probably just a copy-and-paste leftover from the > > main SPL that can go away. > > > > Of course, that doesn't resolve the binman issue. :-) > > > > > diff --git a/arch/arm/cpu/armv8/u-boot-spl.lds b/arch/arm/cpu/armv8/u-boot- > > > spl.lds > > > index 9edb662b094..2827a07590d 100644 > > > --- a/arch/arm/cpu/armv8/u-boot-spl.lds > > > +++ b/arch/arm/cpu/armv8/u-boot-spl.lds > > > @@ -22,6 +22,7 @@ ENTRY(_start) > > > SECTIONS > > > { > > > .text : { > > > + __image_copy_start = .; > > > . = ALIGN(8); > > > *(.__image_copy_start) > > > CPUDIR/start.o (.text*) > > > > If for whatever reason you did need to define the symbol this way, shouldn't > > it be after the alignment? > > Well I don't want to miss out any of the image, otherwise the offsets > would be off. > > But perhaps that is another question. Since this is the first section, > it should already be aligned (to 16 I suspect). So why do we need it? I'd like to get this applied, assuming it is correct, because at present binman is silently ignoring problems where it cannot find symbols. The fix for that is: http://patchwork.ozlabs.org/project/uboot/patch/20210722210216.1.Id1246d1ff1cb5750f8c7ddde9665cf6f09615a7c@changeid/ which has been sitting there for a while. Regards, Simon ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v3 2/3] arm: Add an __image_copy_start symbol for ARMv8 2021-09-25 15:46 ` Simon Glass @ 2021-09-25 22:35 ` Scott Wood 2021-09-26 15:54 ` Simon Glass 0 siblings, 1 reply; 8+ messages in thread From: Scott Wood @ 2021-09-25 22:35 UTC (permalink / raw) To: Simon Glass; +Cc: U-Boot Mailing List, Tim Harvey, Albert Aribaud, Tom Rini On Sat, 2021-09-25 at 09:46 -0600, Simon Glass wrote: > Hi Scott, > > On Thu, 5 Aug 2021 at 13:20, Simon Glass <sjg@chromium.org> wrote: > > > > Hi Scott, > > > > On Wed, 4 Aug 2021 at 13:53, Scott Wood <oss@buserror.net> wrote: > > > > > > On Sun, 2021-08-01 at 14:59 -0600, Simon Glass wrote: > > > > This symbol is needed for binman to locate the start of the image. Add > > > > it. > > > > > > > > Note: the existing line to bring in the .__image_copy_start symbol > > > > does > > > > not appear to do anything. > > > > > > > > Signed-off-by: Simon Glass <sjg@chromium.org> > > > > --- > > > > I have copied Scott Wood who originally added the line about the > > > > __image_copy_start in the hope that he can decide if we should remove > > > > it. > > > > > > It's been a long time since I looked at this stuff, but > > > __image_copy_start is > > > used for relocation and that code does not seem to be in the SPL, so the > > > *(.__image_copy_start) was probably just a copy-and-paste leftover from > > > the > > > main SPL that can go away. > > > > > > Of course, that doesn't resolve the binman issue. :-) > > > > > > > diff --git a/arch/arm/cpu/armv8/u-boot-spl.lds b/arch/arm/cpu/armv8/u- > > > > boot- > > > > spl.lds > > > > index 9edb662b094..2827a07590d 100644 > > > > --- a/arch/arm/cpu/armv8/u-boot-spl.lds > > > > +++ b/arch/arm/cpu/armv8/u-boot-spl.lds > > > > @@ -22,6 +22,7 @@ ENTRY(_start) > > > > SECTIONS > > > > { > > > > .text : { > > > > + __image_copy_start = .; > > > > . = ALIGN(8); > > > > *(.__image_copy_start) > > > > CPUDIR/start.o (.text*) > > > > > > If for whatever reason you did need to define the symbol this way, > > > shouldn't > > > it be after the alignment? > > > > Well I don't want to miss out any of the image, otherwise the offsets > > would be off. > > > > But perhaps that is another question. Since this is the first section, > > it should already be aligned (to 16 I suspect). So why do we need it? > > I'd like to get this applied, assuming it is correct, because at > present binman is silently ignoring problems where it cannot find > symbols. The fix for that is: > > http://patchwork.ozlabs.org/project/uboot/patch/20210722210216.1.Id1246d1ff1cb5750f8c7ddde9665cf6f09615a7c@changeid/ > > which has been sitting there for a while. I'm not sure what you need from me... as I said before, as far as I can tell the __image_copy_start stuff should not be needed by the SPL itself. If you want to add it just for binman, there should probably be a comment to that effect, but I'm not the ARM maintainer (or involved in U-Boot development at all these days) so I can't help you get anything applied. As for why the alignment is there, it's probably just paranoia, but in any case, the SPL linker script probably shouldn't handle alignment in a gratuitously different way from the main linker script. -Scott ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v3 2/3] arm: Add an __image_copy_start symbol for ARMv8 2021-09-25 22:35 ` Scott Wood @ 2021-09-26 15:54 ` Simon Glass 0 siblings, 0 replies; 8+ messages in thread From: Simon Glass @ 2021-09-26 15:54 UTC (permalink / raw) To: Scott Wood, Stefano Babic Cc: U-Boot Mailing List, Tim Harvey, Albert Aribaud, Tom Rini +Stefano Babic who might know On Sat, 25 Sept 2021 at 16:36, Scott Wood <oss@buserror.net> wrote: > > On Sat, 2021-09-25 at 09:46 -0600, Simon Glass wrote: > > Hi Scott, > > > > On Thu, 5 Aug 2021 at 13:20, Simon Glass <sjg@chromium.org> wrote: > > > > > > Hi Scott, > > > > > > On Wed, 4 Aug 2021 at 13:53, Scott Wood <oss@buserror.net> wrote: > > > > > > > > On Sun, 2021-08-01 at 14:59 -0600, Simon Glass wrote: > > > > > This symbol is needed for binman to locate the start of the image. Add > > > > > it. > > > > > > > > > > Note: the existing line to bring in the .__image_copy_start symbol > > > > > does > > > > > not appear to do anything. > > > > > > > > > > Signed-off-by: Simon Glass <sjg@chromium.org> > > > > > --- > > > > > I have copied Scott Wood who originally added the line about the > > > > > __image_copy_start in the hope that he can decide if we should remove > > > > > it. > > > > > > > > It's been a long time since I looked at this stuff, but > > > > __image_copy_start is > > > > used for relocation and that code does not seem to be in the SPL, so the > > > > *(.__image_copy_start) was probably just a copy-and-paste leftover from > > > > the > > > > main SPL that can go away. > > > > > > > > Of course, that doesn't resolve the binman issue. :-) > > > > > > > > > diff --git a/arch/arm/cpu/armv8/u-boot-spl.lds b/arch/arm/cpu/armv8/u- > > > > > boot- > > > > > spl.lds > > > > > index 9edb662b094..2827a07590d 100644 > > > > > --- a/arch/arm/cpu/armv8/u-boot-spl.lds > > > > > +++ b/arch/arm/cpu/armv8/u-boot-spl.lds > > > > > @@ -22,6 +22,7 @@ ENTRY(_start) > > > > > SECTIONS > > > > > { > > > > > .text : { > > > > > + __image_copy_start = .; > > > > > . = ALIGN(8); > > > > > *(.__image_copy_start) > > > > > CPUDIR/start.o (.text*) > > > > > > > > If for whatever reason you did need to define the symbol this way, > > > > shouldn't > > > > it be after the alignment? > > > > > > Well I don't want to miss out any of the image, otherwise the offsets > > > would be off. > > > > > > But perhaps that is another question. Since this is the first section, > > > it should already be aligned (to 16 I suspect). So why do we need it? > > > > I'd like to get this applied, assuming it is correct, because at > > present binman is silently ignoring problems where it cannot find > > symbols. The fix for that is: > > > > http://patchwork.ozlabs.org/project/uboot/patch/20210722210216.1.Id1246d1ff1cb5750f8c7ddde9665cf6f09615a7c@changeid/ > > > > which has been sitting there for a while. > > I'm not sure what you need from me... as I said before, as far as I can tell > the __image_copy_start stuff should not be needed by the SPL itself. If you > want to add it just for binman, there should probably be a comment to that > effect, but I'm not the ARM maintainer (or involved in U-Boot development at > all these days) so I can't help you get anything applied. > > As for why the alignment is there, it's probably just paranoia, but in any > case, the SPL linker script probably shouldn't handle alignment in a > gratuitously different way from the main linker script. > > -Scott > > ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v3 3/3] binman: Show an error if __image_copy_start is missing 2021-08-01 20:59 [PATCH v3 1/3] imx8: ls1028a: Drop raw image support Simon Glass 2021-08-01 20:59 ` [PATCH v3 2/3] arm: Add an __image_copy_start symbol for ARMv8 Simon Glass @ 2021-08-01 20:59 ` Simon Glass 1 sibling, 0 replies; 8+ messages in thread From: Simon Glass @ 2021-08-01 20:59 UTC (permalink / raw) To: U-Boot Mailing List; +Cc: Tim Harvey, Scott Wood, Simon Glass, Tim Harvey Binman needs this symbol to be able to figure out the start of the image. Detect if it is missing and report an error if any symbols are needed. Add more documentation about possible binman warnings. Signed-off-by: Simon Glass <sjg@chromium.org> Reviewed-by: Tim Harvey <tharvey@gatewrorks.com> --- (no changes since v1) tools/binman/binman.rst | 109 +++++++++++++++++++++++++++++++++++++++ tools/binman/elf.py | 6 ++- tools/binman/elf_test.py | 7 ++- 3 files changed, 118 insertions(+), 4 deletions(-) diff --git a/tools/binman/binman.rst b/tools/binman/binman.rst index 09e7b571982..81e0a1364ff 100644 --- a/tools/binman/binman.rst +++ b/tools/binman/binman.rst @@ -1158,6 +1158,115 @@ development, since dealing with exceptions and problems in threads is more difficult. This avoids any use of ThreadPoolExecutor. +Dealing with warnings and errors +-------------------------------- + +__image_copy_start +~~~~~~~~~~~~~~~~~~ + +If you see:: + + Cannot process symbol 'xxx' since there is no __image_copy_start + +this means that your SPL image does not include an `__image_copy_start` symbol. +You can check this with:: + + nm spl/u-boot-spl |grep __image_copy_start + +If there is no output them you don't have that symbol. It is normally created +in a `u-boot-spl.lds` file, like this:: + + text : + { + __image_copy_start = .; + *(.vectors) + CPUDIR/start.o (.text*) + *(.text*) + *(.glue*) + } + +Check the appropriate file for your board, typically in the `arch/xxx/cpu` +or `arch/xxx/cpu/xxx` directory. + +Entry xx not found in list +~~~~~~~~~~~~~~~~~~~~~~~~~~ + +If you see something like:: + + output: 'binman: Section '/binman/u-boot-spl-ddr': + Symbol '_binman_u_boot_any_prop_image_pos' + in entry '/binman/u-boot-spl-ddr/u-boot-spl/u-boot-spl-nodtb': + Entry 'u-boot-any' not found in list (u-boot-spl-nodtb,u-boot-spl-dtb, + u-boot-spl,blob-ext@1,blob-ext@2,blob-ext@3,blob-ext@4,main-section) + +this means that binman knows it should set the value of a symbol called +`_binman_u_boot_any_prop_image_pos` but does not know how. That symbol name is +generated by the `binman_symname` macro (see `binman_sym.h`):: + + #define binman_symname(_entry_name, _prop_name) \ + _binman_ ## _entry_name ## _prop_ ## _prop_name + +so binman decodes it into: + +_binman_ + prefix for all symbols +u_boot_any + entry to find +_prop_ + prefix for property +image_pos + image_pos property + +It therefore looks for u-boot-any, which means any U-Boot symbol. Supported ones +are: + +- u-boot +- u-boot-img +- u-boot-nodtb + +You can see a list of the symbols it tried, in brackets. None of these matches +the above list. The source definition in this example is:: + + &binman { + u-boot-spl-ddr { + filename = "u-boot-spl-ddr.bin"; + pad-byte = <0xff>; + align-size = <4>; + align = <4>; + + u-boot-spl { + align-end = <4>; + }; + + blob-ext-1 { + filename = "lpddr4_pmu_train_1d_imem.bin"; + size = <0x8000>; + }; + + blob-ext-2 { + filename = "lpddr4_pmu_train_1d_dmem.bin"; + size = <0x4000>; + }; + + blob-ext-3 { + filename = "lpddr4_pmu_train_2d_imem.bin"; + size = <0x8000>; + }; + + blob-ext-4 { + filename = "lpddr4_pmu_train_2d_dmem.bin"; + size = <0x4000>; + }; + }; + +and you can see that, while `u-boot-spl` is present, `u-boot` is not. Binman +must find the required symbol somewhere in the same image. + +In this case the problem is that CONFIG_SPL_RAW_IMAGE_SUPPORT is enabled, even +though U-Boot is actually stored in a FIT. This means that +spl_set_header_raw_uboot() is called and it looks for a symbol for U-Boot. +Disabling that option fixes the error. + History / Credits ----------------- diff --git a/tools/binman/elf.py b/tools/binman/elf.py index 03b49d7163c..f14d07da157 100644 --- a/tools/binman/elf.py +++ b/tools/binman/elf.py @@ -112,12 +112,14 @@ def LookupAndWriteSymbols(elf_fname, entry, section): if not syms: return base = syms.get('__image_copy_start') - if not base: - return for name, sym in syms.items(): if name.startswith('_binman'): msg = ("Section '%s': Symbol '%s'\n in entry '%s'" % (section.GetPath(), name, entry.GetPath())) + if not base: + raise ValueError("Cannot process symbol '%s' since there is no __image_copy_start" % + name) + offset = sym.address - base.address if offset < 0 or offset + sym.size > entry.contents_size: raise ValueError('%s has offset %x (size %x) but the contents ' diff --git a/tools/binman/elf_test.py b/tools/binman/elf_test.py index 7a128018d9f..96630502b2f 100644 --- a/tools/binman/elf_test.py +++ b/tools/binman/elf_test.py @@ -134,8 +134,11 @@ class TestElf(unittest.TestCase): entry = FakeEntry(10) section = FakeSection() elf_fname = self.ElfTestFile('u_boot_binman_syms_bad') - self.assertEqual(elf.LookupAndWriteSymbols(elf_fname, entry, section), - None) + with self.assertRaises(ValueError) as e: + self.assertEqual(elf.LookupAndWriteSymbols(elf_fname, entry, section), + None) + self.assertIn("Cannot process symbol '_binman_u_boot_spl_any_prop_offset' since there is no __image_copy_start", + str(e.exception)) def testBadSymbolSize(self): """Test that an attempt to use an 8-bit symbol are detected -- 2.32.0.554.ge1b32706d8-goog ^ permalink raw reply related [flat|nested] 8+ messages in thread
end of thread, other threads:[~2021-09-26 15:54 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-08-01 20:59 [PATCH v3 1/3] imx8: ls1028a: Drop raw image support Simon Glass 2021-08-01 20:59 ` [PATCH v3 2/3] arm: Add an __image_copy_start symbol for ARMv8 Simon Glass 2021-08-04 19:52 ` Scott Wood 2021-08-05 19:20 ` Simon Glass 2021-09-25 15:46 ` Simon Glass 2021-09-25 22:35 ` Scott Wood 2021-09-26 15:54 ` Simon Glass 2021-08-01 20:59 ` [PATCH v3 3/3] binman: Show an error if __image_copy_start is missing Simon Glass
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).