[PATCH v3 3/3] binman: Show an error if __image_copy_start is missing

[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 at chromium.org>
Reviewed-by: Tim Harvey <tharvey at 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 at 1,blob-ext at 2,blob-ext at 3,blob-ext at 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