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

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