binman: Support symbols in u-boot-spl-nodtb

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>

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

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

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

tools/binman/test/053_symbols.dts

@@ -10,7 +10,7 @@
 		};
 
 		u-boot {
-			offset = <24>;
+			offset = <0x18>;
 		};
 
 		u-boot-spl2 {

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";
+		};
+	};
+};

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