[PATCH v3 06/27] binman: Support symbols in u-boot-spl-nodtb

[Simon Glass <sjg@chromium.org>]

Since this is an execuable we should be able insert symbol values into it.
Add support for this.

Use common code for this test and the original testSymbols. Use hex
consistently for the values and add some more comments.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

Changes in v3:
- Add new patch to support symbols in u-boot-spl-nodtb

 tools/binman/README.entries             |  9 +++++
 tools/binman/etype/u_boot_spl_nodtb.py  | 14 +++++++
 tools/binman/ftest.py                   | 49 +++++++++++++++----------
 tools/binman/test/053_symbols.dts       |  2 +-
 tools/binman/test/192_symbols_nodtb.dts | 26 +++++++++++++
 tools/binman/test/u_boot_binman_syms.c  |  2 +-
 6 files changed, 81 insertions(+), 21 deletions(-)
 create mode 100644 tools/binman/test/192_symbols_nodtb.dts

diff --git a/tools/binman/README.entries b/tools/binman/README.entries
index 5c6663e2c7c..1dbdd0be03d 100644
--- a/tools/binman/README.entries
+++ b/tools/binman/README.entries
@@ -1011,6 +1011,15 @@ a device tree to operate on your platform. You can add a u-boot-spl-dtb
 entry after this one, or use a u-boot-spl entry instead (which contains
 both SPL and the device tree).
 
+SPL can access binman symbols at runtime. See:
+
+    'Access to binman entry offsets at run time (symbols)'
+
+in the binman README for more information.
+
+The ELF file 'spl/u-boot-spl' must also be available for this to work, since
+binman uses that to look up symbols to write into the SPL binary.
+
 
 
 Entry: u-boot-spl-with-ucode-ptr: U-Boot SPL with embedded microcode pointer
diff --git a/tools/binman/etype/u_boot_spl_nodtb.py b/tools/binman/etype/u_boot_spl_nodtb.py
index 41d75054910..dbf2f12743a 100644
--- a/tools/binman/etype/u_boot_spl_nodtb.py
+++ b/tools/binman/etype/u_boot_spl_nodtb.py
@@ -5,6 +5,7 @@
 # Entry-type module for 'u-boot-spl-nodtb.bin'
 #
 
+from binman import elf
 from binman.entry import Entry
 from binman.etype.blob import Entry_blob
 
@@ -19,9 +20,22 @@ class Entry_u_boot_spl_nodtb(Entry_blob):
     a device tree to operate on your platform. You can add a u-boot-spl-dtb
     entry after this one, or use a u-boot-spl entry instead (which contains
     both SPL and the device tree).
+
+    SPL can access binman symbols at runtime. See:
+
+        'Access to binman entry offsets at run time (symbols)'
+
+    in the binman README for more information.
+
+    The ELF file 'spl/u-boot-spl' must also be available for this to work, since
+    binman uses that to look up symbols to write into the SPL binary.
     """
     def __init__(self, section, etype, node):
         super().__init__(section, etype, node)
+        self.elf_fname = 'spl/u-boot-spl'
 
     def GetDefaultFilename(self):
         return 'spl/u-boot-spl-nodtb.bin'
+
+    def WriteSymbols(self, section):
+        elf.LookupAndWriteSymbols(self.elf_fname, self, section.GetImage())
diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
index 814e91d42e9..e056601b9a4 100644
--- a/tools/binman/ftest.py
+++ b/tools/binman/ftest.py
@@ -1337,21 +1337,43 @@ class TestFunctional(unittest.TestCase):
         data = self._DoReadFile('052_u_boot_spl_nodtb.dts')
         self.assertEqual(U_BOOT_SPL_NODTB_DATA, data[:len(U_BOOT_SPL_NODTB_DATA)])
 
-    def testSymbols(self):
-        """Test binman can assign symbols embedded in U-Boot"""
+    def checkSymbols(self, dts, base_data, u_boot_offset):
+        """Check the image contains the expected symbol values
+
+        Args:
+            dts: Device tree file to use for test
+            base_data: Data before and after 'u-boot' section
+            u_boot_offset: Offset of 'u-boot' section in image
+        """
         elf_fname = self.ElfTestFile('u_boot_binman_syms')
         syms = elf.GetSymbols(elf_fname, ['binman', 'image'])
         addr = elf.GetSymbolAddress(elf_fname, '__image_copy_start')
-        self.assertEqual(syms['_binman_u_boot_spl_prop_offset'].address, addr)
+        self.assertEqual(syms['_binman_u_boot_spl_any_prop_offset'].address,
+                         addr)
 
         self._SetupSplElf('u_boot_binman_syms')
-        data = self._DoReadFile('053_symbols.dts')
-        sym_values = struct.pack('<LQLL', 0x00, 0x1c, 0x28, 0x04)
-        expected = (sym_values + U_BOOT_SPL_DATA[20:] +
+        data = self._DoReadFile(dts)
+        # The image should contain the symbols from u_boot_binman_syms.c
+        # Note that image_pos is adjusted by the base address of the image,
+        # which is 0x10 in our test image
+        sym_values = struct.pack('<LQLL', 0x00,
+                                 u_boot_offset + len(U_BOOT_DATA),
+                                 0x10 + u_boot_offset, 0x04)
+        expected = (sym_values + base_data[20:] +
                     tools.GetBytes(0xff, 1) + U_BOOT_DATA + sym_values +
-                    U_BOOT_SPL_DATA[20:])
+                    base_data[20:])
         self.assertEqual(expected, data)
 
+    def testSymbols(self):
+        """Test binman can assign symbols embedded in U-Boot"""
+        self.checkSymbols('053_symbols.dts', U_BOOT_SPL_DATA, 0x18)
+
+    def testSymbolsNoDtb(self):
+        """Test binman can assign symbols embedded in U-Boot SPL"""
+        self.checkSymbols('192_symbols_nodtb.dts',
+                          U_BOOT_SPL_NODTB_DATA + U_BOOT_SPL_DTB_DATA,
+                          0x38)
+
     def testPackUnitAddress(self):
         """Test that we support multiple binaries with the same name"""
         data = self._DoReadFile('054_unit_address.dts')
@@ -4186,18 +4208,7 @@ class TestFunctional(unittest.TestCase):
 
     def testSymbolsSubsection(self):
         """Test binman can assign symbols from a subsection"""
-        elf_fname = self.ElfTestFile('u_boot_binman_syms')
-        syms = elf.GetSymbols(elf_fname, ['binman', 'image'])
-        addr = elf.GetSymbolAddress(elf_fname, '__image_copy_start')
-        self.assertEqual(syms['_binman_u_boot_spl_prop_offset'].address, addr)
-
-        self._SetupSplElf('u_boot_binman_syms')
-        data = self._DoReadFile('187_symbols_sub.dts')
-        sym_values = struct.pack('<LQLL', 0x00, 0x1c, 0x28, 0x04)
-        expected = (sym_values + U_BOOT_SPL_DATA[20:] +
-                    tools.GetBytes(0xff, 1) + U_BOOT_DATA + sym_values +
-                    U_BOOT_SPL_DATA[20:])
-        self.assertEqual(expected, data)
+        self.checkSymbols('187_symbols_sub.dts', U_BOOT_SPL_DATA, 0x18)
 
     def testReadImageEntryArg(self):
         """Test reading an image that would need an entry arg to generate"""
diff --git a/tools/binman/test/053_symbols.dts b/tools/binman/test/053_symbols.dts
index 8af575158f5..29658092764 100644
--- a/tools/binman/test/053_symbols.dts
+++ b/tools/binman/test/053_symbols.dts
@@ -10,7 +10,7 @@
 		};
 
 		u-boot {
-			offset = <24>;
+			offset = <0x18>;
 		};
 
 		u-boot-spl2 {
diff --git a/tools/binman/test/192_symbols_nodtb.dts b/tools/binman/test/192_symbols_nodtb.dts
new file mode 100644
index 00000000000..5c900d60709
--- /dev/null
+++ b/tools/binman/test/192_symbols_nodtb.dts
@@ -0,0 +1,26 @@
+// SPDX-License-Identifier: GPL-2.0+
+/dts-v1/;
+
+/ {
+	#address-cells = <1>;
+	#size-cells = <1>;
+
+	binman {
+		pad-byte = <0xff>;
+		u-boot-spl-nodtb {
+		};
+		u-boot-spl-dtb {
+		};
+
+		u-boot {
+			offset = <0x38>;
+		};
+
+		u-boot-spl2 {
+			type = "u-boot-spl-nodtb";
+		};
+		u-boot-spl-dtb2 {
+			type = "u-boot-spl-dtb";
+		};
+	};
+};
diff --git a/tools/binman/test/u_boot_binman_syms.c b/tools/binman/test/u_boot_binman_syms.c
index 4520b319f16..37fc339ce84 100644
--- a/tools/binman/test/u_boot_binman_syms.c
+++ b/tools/binman/test/u_boot_binman_syms.c
@@ -8,7 +8,7 @@
 #define CONFIG_BINMAN
 #include <binman_sym.h>
 
-binman_sym_declare(unsigned long, u_boot_spl, offset);
+binman_sym_declare(unsigned long, u_boot_spl_any, offset);
 binman_sym_declare(unsigned long long, u_boot_spl2, offset);
 binman_sym_declare(unsigned long, u_boot_any, image_pos);
 binman_sym_declare(unsigned long, u_boot_any, size);
-- 
2.31.0.rc2.261.g7f71774620-goog