All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 1/3] imx8: Drop raw image support
@ 2021-07-25 16:54 Simon Glass
  2021-07-25 16:54 ` [PATCH v2 2/3] arm: Add an __image_copy_start symbol for ARMv8 Simon Glass
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Simon Glass @ 2021-07-25 16:54 UTC (permalink / raw)
  To: U-Boot Mailing List
  Cc: Tim Harvey, Simon Glass, Adam Ford, Alex Marginean, Alice Guo,
	Andrey Zhizhikin, Claudiu Manoil, Fabio Estevam, Heiko Schocher,
	Igor Opaniuk, Ilko Iliev, Kirill Kapranov, Michael Walle,
	Peng Fan, Priyanka Jain, Teresa Remmet, Uri Mashiach,
	Valentin Raevsky, Ye Li, Ying-Chun Liu (PaulLiu)

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>
---

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 f46f45bda91..a1ded18f7e8 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_SUPPORT=y
 CONFIG_SPL_POWER_SUPPORT=y
diff --git a/configs/imx8mm_evk_defconfig b/configs/imx8mm_evk_defconfig
index a06c6f9794a..9dc0be6b09e 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_SUPPORT=y
 CONFIG_SPL_POWER_SUPPORT=y
diff --git a/configs/imx8mn_ddr4_evk_defconfig b/configs/imx8mn_ddr4_evk_defconfig
index 205757da229..a96f729bde9 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_SUPPORT=y
 CONFIG_SPL_WATCHDOG_SUPPORT=y
diff --git a/configs/imx8mp_evk_defconfig b/configs/imx8mp_evk_defconfig
index d0f390ed776..df4463ca2b6 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_SUPPORT=y
diff --git a/configs/imx8mq_cm_defconfig b/configs/imx8mq_cm_defconfig
index e11122e645f..72d9ec8b258 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_SUPPORT=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 32d538c8bbb..d63cba70108 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_SUPPORT=y
-- 
2.32.0.432.gabb21c7263-goog


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v2 2/3] arm: Add an __image_copy_start symbol for ARMv8
  2021-07-25 16:54 [PATCH v2 1/3] imx8: Drop raw image support Simon Glass
@ 2021-07-25 16:54 ` Simon Glass
  2021-07-26 18:20   ` Tim Harvey
  2021-07-25 16:54 ` [PATCH v2 3/3] binman: Show an error if __image_copy_start is missing Simon Glass
  2021-07-26  7:33 ` [PATCH v2 1/3] imx8: Drop raw image support Michael Walle
  2 siblings, 1 reply; 7+ messages in thread
From: Simon Glass @ 2021-07-25 16:54 UTC (permalink / raw)
  To: U-Boot Mailing List
  Cc: Tim Harvey, Simon Glass, Albert Aribaud, Marek Behún, 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>
---

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.432.gabb21c7263-goog


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v2 3/3] binman: Show an error if __image_copy_start is missing
  2021-07-25 16:54 [PATCH v2 1/3] imx8: Drop raw image support Simon Glass
  2021-07-25 16:54 ` [PATCH v2 2/3] arm: Add an __image_copy_start symbol for ARMv8 Simon Glass
@ 2021-07-25 16:54 ` Simon Glass
  2021-07-26 18:36   ` Tim Harvey
  2021-07-26  7:33 ` [PATCH v2 1/3] imx8: Drop raw image support Michael Walle
  2 siblings, 1 reply; 7+ messages in thread
From: Simon Glass @ 2021-07-25 16:54 UTC (permalink / raw)
  To: U-Boot Mailing List; +Cc: Tim Harvey, Simon Glass

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>
---

(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.432.gabb21c7263-goog


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH v2 1/3] imx8: Drop raw image support
  2021-07-25 16:54 [PATCH v2 1/3] imx8: Drop raw image support Simon Glass
  2021-07-25 16:54 ` [PATCH v2 2/3] arm: Add an __image_copy_start symbol for ARMv8 Simon Glass
  2021-07-25 16:54 ` [PATCH v2 3/3] binman: Show an error if __image_copy_start is missing Simon Glass
@ 2021-07-26  7:33 ` Michael Walle
  2 siblings, 0 replies; 7+ messages in thread
From: Michael Walle @ 2021-07-26  7:33 UTC (permalink / raw)
  To: Simon Glass
  Cc: U-Boot Mailing List, Tim Harvey, Adam Ford, Alex Marginean,
	Alice Guo, Andrey Zhizhikin, Claudiu Manoil, Fabio Estevam,
	Heiko Schocher, Igor Opaniuk, Ilko Iliev, Kirill Kapranov,
	Peng Fan, Priyanka Jain, Teresa Remmet, Uri Mashiach,
	Valentin Raevsky, Ye Li, Ying-Chun Liu (PaulLiu)

Am 2021-07-25 18:54, schrieb Simon Glass:
> 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

small nit: this isn't an imx8 board but an ls1028a soc based one.

-michael

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2 2/3] arm: Add an __image_copy_start symbol for ARMv8
  2021-07-25 16:54 ` [PATCH v2 2/3] arm: Add an __image_copy_start symbol for ARMv8 Simon Glass
@ 2021-07-26 18:20   ` Tim Harvey
  2021-07-28  2:46     ` Simon Glass
  0 siblings, 1 reply; 7+ messages in thread
From: Tim Harvey @ 2021-07-26 18:20 UTC (permalink / raw)
  To: Simon Glass
  Cc: U-Boot Mailing List, Albert Aribaud, Marek Behún, Tom Rini

On Sun, Jul 25, 2021 at 9:54 AM Simon Glass <sjg@chromium.org> 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>
> ---
>
> 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.432.gabb21c7263-goog
>

Sould the '*(.__image_copy_start)' be removed?

I'll admit that I'm not very knowledgable when it comes to linker
files. I did verify removing it boots fine.

Tim

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2 3/3] binman: Show an error if __image_copy_start is missing
  2021-07-25 16:54 ` [PATCH v2 3/3] binman: Show an error if __image_copy_start is missing Simon Glass
@ 2021-07-26 18:36   ` Tim Harvey
  0 siblings, 0 replies; 7+ messages in thread
From: Tim Harvey @ 2021-07-26 18:36 UTC (permalink / raw)
  To: Simon Glass; +Cc: U-Boot Mailing List

On Sun, Jul 25, 2021 at 9:54 AM Simon Glass <sjg@chromium.org> wrote:
>
> 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>
> ---
>
> (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.432.gabb21c7263-goog
>

Simon,

Thanks - this is very helpful.

Reviewed-by: Tim Harvey <tharvey@gatewrorks.com>

Tim

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2 2/3] arm: Add an __image_copy_start symbol for ARMv8
  2021-07-26 18:20   ` Tim Harvey
@ 2021-07-28  2:46     ` Simon Glass
  0 siblings, 0 replies; 7+ messages in thread
From: Simon Glass @ 2021-07-28  2:46 UTC (permalink / raw)
  To: Tim Harvey
  Cc: U-Boot Mailing List, Albert Aribaud, Marek Behún, Tom Rini

Hi Tim,

On Mon, 26 Jul 2021 at 12:20, Tim Harvey <tharvey@gateworks.com> wrote:
>
> On Sun, Jul 25, 2021 at 9:54 AM Simon Glass <sjg@chromium.org> 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>
> > ---
> >
> > 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.432.gabb21c7263-goog
> >
>
> Sould the '*(.__image_copy_start)' be removed?
>
> I'll admit that I'm not very knowledgable when it comes to linker
> files. I did verify removing it boots fine.

I did look around for symbols in that section and could not find any,
but I'm not 100% sure.

Regards,
SImon

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2021-07-28  2:46 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-25 16:54 [PATCH v2 1/3] imx8: Drop raw image support Simon Glass
2021-07-25 16:54 ` [PATCH v2 2/3] arm: Add an __image_copy_start symbol for ARMv8 Simon Glass
2021-07-26 18:20   ` Tim Harvey
2021-07-28  2:46     ` Simon Glass
2021-07-25 16:54 ` [PATCH v2 3/3] binman: Show an error if __image_copy_start is missing Simon Glass
2021-07-26 18:36   ` Tim Harvey
2021-07-26  7:33 ` [PATCH v2 1/3] imx8: Drop raw image support Michael Walle

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.