[PATCH v5 09/20] binman: Convert mkimage to Entry_section

Simon Glass sjg at chromium.org
Tue Jul 18 15:23:58 CEST 2023


From: Marek Vasut <marex at denx.de>

This is needed to handle mkimage with inner section located itself in a
section.

Signed-off-by: Marek Vasut <marex at denx.de>
Use BuildSectionData() instead of ObtainContents(), add tests and a few
other minor fixes:
Signed-off-by: Simon Glass <sjg at chromium.org>
---

Changes in v5:
- Drop method implementations for which the entry_Section one is enough

Changes in v3:
- Fix up some tests which now need SPL and TPL
- Avoid calling ObtainContents() when not needed

Changes in v2:
- Drop now-unnecessary methods in mkimage etype

 tools/binman/entry.py                     |   6 +-
 tools/binman/etype/mkimage.py             | 113 ++++++++--------------
 tools/binman/ftest.py                     |  67 ++++++++++++-
 tools/binman/test/283_mkimage_special.dts |  24 +++++
 4 files changed, 130 insertions(+), 80 deletions(-)
 create mode 100644 tools/binman/test/283_mkimage_special.dts

diff --git a/tools/binman/entry.py b/tools/binman/entry.py
index f20f32213a9b..0d4cb94f7002 100644
--- a/tools/binman/entry.py
+++ b/tools/binman/entry.py
@@ -1314,10 +1314,8 @@ features to produce new behaviours.
         """
         data = b''
         for entry in entries:
-            # First get the input data and put it in a file. If not available,
-            # try later.
-            if not entry.ObtainContents(fake_size=fake_size):
-                return None, None, None
+            # First get the input data and put it in a file
+            entry.ObtainContents(fake_size=fake_size)
             data += entry.GetData()
         uniq = self.GetUniqueName()
         fname = tools.get_output_filename(f'{prefix}.{uniq}')
diff --git a/tools/binman/etype/mkimage.py b/tools/binman/etype/mkimage.py
index cb3e10672ad7..31ac05dd56d1 100644
--- a/tools/binman/etype/mkimage.py
+++ b/tools/binman/etype/mkimage.py
@@ -8,10 +8,11 @@
 from collections import OrderedDict
 
 from binman.entry import Entry
+from binman.etype.section import Entry_section
 from dtoc import fdt_util
 from u_boot_pylib import tools
 
-class Entry_mkimage(Entry):
+class Entry_mkimage(Entry_section):
     """Binary produced by mkimage
 
     Properties / Entry arguments:
@@ -121,10 +122,8 @@ class Entry_mkimage(Entry):
     """
     def __init__(self, section, etype, node):
         super().__init__(section, etype, node)
-        self._mkimage_entries = OrderedDict()
         self._imagename = None
-        self._filename = fdt_util.GetString(self._node, 'filename')
-        self.align_default = None
+        self._multiple_data_files = False
 
     def ReadNode(self):
         super().ReadNode()
@@ -135,41 +134,55 @@ class Entry_mkimage(Entry):
                                                    'data-to-imagename')
         if self._data_to_imagename and self._node.FindNode('imagename'):
             self.Raise('Cannot use both imagename node and data-to-imagename')
-        self.ReadEntries()
 
     def ReadEntries(self):
         """Read the subnodes to find out what should go in this image"""
         for node in self._node.subnodes:
-            entry = Entry.Create(self, node)
+            if self.IsSpecialSubnode(node):
+                continue
+            entry = Entry.Create(self, node,
+                                 expanded=self.GetImage().use_expanded,
+                                 missing_etype=self.GetImage().missing_etype)
             entry.ReadNode()
+            entry.SetPrefix(self._name_prefix)
             if entry.name == 'imagename':
                 self._imagename = entry
             else:
-                self._mkimage_entries[entry.name] = entry
+                self._entries[entry.name] = entry
 
-    def ObtainContents(self):
+    def BuildSectionData(self, required):
+        """Build mkimage entry contents
+
+        Runs mkimage to build the entry contents
+
+        Args:
+            required (bool): True if the data must be present, False if it is OK
+                to return None
+
+        Returns:
+            bytes: Contents of the section
+        """
         # Use a non-zero size for any fake files to keep mkimage happy
         # Note that testMkimageImagename() relies on this 'mkimage' parameter
         fake_size = 1024
         if self._multiple_data_files:
             fnames = []
             uniq = self.GetUniqueName()
-            for entry in self._mkimage_entries.values():
-                if not entry.ObtainContents(fake_size=fake_size):
-                    return False
-                if entry._pathname:
-                    fnames.append(entry._pathname)
+            for entry in self._entries.values():
+                # Put the contents in a temporary file
+                ename = f'mkimage-in-{uniq}-{entry.name}'
+                fname = tools.get_output_filename(ename)
+                data = entry.GetData(required)
+                tools.write_file(fname, data)
+                fnames.append(fname)
             input_fname = ":".join(fnames)
+            data = b''
         else:
             data, input_fname, uniq = self.collect_contents_to_file(
-                self._mkimage_entries.values(), 'mkimage', fake_size)
-            if data is None:
-                return False
+                self._entries.values(), 'mkimage', fake_size)
         if self._imagename:
             image_data, imagename_fname, _ = self.collect_contents_to_file(
                 [self._imagename], 'mkimage-n', 1024)
-            if image_data is None:
-                return False
         outfile = self._filename if self._filename else 'mkimage-out.%s' % uniq
         output_fname = tools.get_output_filename(outfile)
 
@@ -177,8 +190,7 @@ class Entry_mkimage(Entry):
         self.CheckMissing(missing_list)
         self.missing = bool(missing_list)
         if self.missing:
-            self.SetContents(b'')
-            return self.allow_missing
+            return b''
 
         args = ['-d', input_fname]
         if self._data_to_imagename:
@@ -187,71 +199,22 @@ class Entry_mkimage(Entry):
             args += ['-n', imagename_fname]
         args += self._args + [output_fname]
         if self.mkimage.run_cmd(*args) is not None:
-            self.SetContents(tools.read_file(output_fname))
+            return tools.read_file(output_fname)
         else:
             # Bintool is missing; just use the input data as the output
             self.record_missing_bintool(self.mkimage)
-            self.SetContents(data)
-
-        return True
+            return data
 
     def GetEntries(self):
         # Make a copy so we don't change the original
-        entries = OrderedDict(self._mkimage_entries)
+        entries = OrderedDict(self._entries)
         if self._imagename:
             entries['imagename'] = self._imagename
         return entries
 
-    def SetAllowMissing(self, allow_missing):
-        """Set whether a section allows missing external blobs
-
-        Args:
-            allow_missing: True if allowed, False if not allowed
-        """
-        self.allow_missing = allow_missing
-        for entry in self._mkimage_entries.values():
-            entry.SetAllowMissing(allow_missing)
-        if self._imagename:
-            self._imagename.SetAllowMissing(allow_missing)
-
-    def SetAllowFakeBlob(self, allow_fake):
-        """Set whether the sub nodes allows to create a fake blob
-
-        Args:
-            allow_fake: True if allowed, False if not allowed
-        """
-        for entry in self._mkimage_entries.values():
-            entry.SetAllowFakeBlob(allow_fake)
-        if self._imagename:
-            self._imagename.SetAllowFakeBlob(allow_fake)
-
-    def CheckMissing(self, missing_list):
-        """Check if any entries in this section have missing external blobs
-
-        If there are missing (non-optional) blobs, the entries are added to the
-        list
-
-        Args:
-            missing_list: List of Entry objects to be added to
-        """
-        for entry in self._mkimage_entries.values():
-            entry.CheckMissing(missing_list)
-        if self._imagename:
-            self._imagename.CheckMissing(missing_list)
-
-    def CheckFakedBlobs(self, faked_blobs_list):
-        """Check if any entries in this section have faked external blobs
-
-        If there are faked blobs, the entries are added to the list
-
-        Args:
-            faked_blobs_list: List of Entry objects to be added to
-        """
-        for entry in self._mkimage_entries.values():
-            entry.CheckFakedBlobs(faked_blobs_list)
-        if self._imagename:
-            self._imagename.CheckFakedBlobs(faked_blobs_list)
-
     def AddBintools(self, btools):
         super().AddBintools(btools)
         self.mkimage = self.AddBintool(btools, 'mkimage')
+
+    def CheckEntries(self):
+        pass
diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
index dabb3f689fdb..db348de72228 100644
--- a/tools/binman/ftest.py
+++ b/tools/binman/ftest.py
@@ -1103,6 +1103,7 @@ class TestFunctional(unittest.TestCase):
 
     def testPackZeroOffset(self):
         """Test that an entry at offset 0 is not given a new offset"""
+        self._SetupSplElf()
         with self.assertRaises(ValueError) as e:
             self._DoTestFile('025_pack_zero_size.dts')
         self.assertIn("Node '/binman/u-boot-spl': Offset 0x0 (0) overlaps "
@@ -1116,6 +1117,7 @@ class TestFunctional(unittest.TestCase):
 
     def testPackX86RomNoSize(self):
         """Test that the end-at-4gb property requires a size property"""
+        self._SetupSplElf()
         with self.assertRaises(ValueError) as e:
             self._DoTestFile('027_pack_4gb_no_size.dts')
         self.assertIn("Image '/binman': Section size must be provided when "
@@ -1124,6 +1126,7 @@ class TestFunctional(unittest.TestCase):
     def test4gbAndSkipAtStartTogether(self):
         """Test that the end-at-4gb and skip-at-size property can't be used
         together"""
+        self._SetupSplElf()
         with self.assertRaises(ValueError) as e:
             self._DoTestFile('098_4gb_and_skip_at_start_together.dts')
         self.assertIn("Image '/binman': Provide either 'end-at-4gb' or "
@@ -1131,6 +1134,7 @@ class TestFunctional(unittest.TestCase):
 
     def testPackX86RomOutside(self):
         """Test that the end-at-4gb property checks for offset boundaries"""
+        self._SetupSplElf()
         with self.assertRaises(ValueError) as e:
             self._DoTestFile('028_pack_4gb_outside.dts')
         self.assertIn("Node '/binman/u-boot': Offset 0x0 (0) size 0x4 (4) "
@@ -1423,6 +1427,7 @@ class TestFunctional(unittest.TestCase):
 
     def testPackUbootSplMicrocode(self):
         """Test that x86 microcode can be handled correctly in SPL"""
+        self._SetupSplElf()
         self._PackUbootSplMicrocode('049_x86_ucode_spl.dts')
 
     def testPackUbootSplMicrocodeReorder(self):
@@ -1442,6 +1447,7 @@ class TestFunctional(unittest.TestCase):
 
     def testSplDtb(self):
         """Test that an image with spl/u-boot-spl.dtb can be created"""
+        self._SetupSplElf()
         data = self._DoReadFile('051_u_boot_spl_dtb.dts')
         self.assertEqual(U_BOOT_SPL_DTB_DATA, data[:len(U_BOOT_SPL_DTB_DATA)])
 
@@ -1962,6 +1968,8 @@ class TestFunctional(unittest.TestCase):
 
     def testUpdateFdtAll(self):
         """Test that all device trees are updated with offset/size info"""
+        self._SetupSplElf()
+        self._SetupTplElf()
         data = self._DoReadFileRealDtb('082_fdt_update_all.dts')
 
         base_expected = {
@@ -3284,6 +3292,8 @@ class TestFunctional(unittest.TestCase):
 
     def testUpdateFdtAllRepack(self):
         """Test that all device trees are updated with offset/size info"""
+        self._SetupSplElf()
+        self._SetupTplElf()
         data = self._DoReadFileRealDtb('134_fdt_update_all_repack.dts')
         SECTION_SIZE = 0x300
         DTB_SIZE = 602
@@ -3737,6 +3747,7 @@ class TestFunctional(unittest.TestCase):
 
     def testMkimage(self):
         """Test using mkimage to build an image"""
+        self._SetupSplElf()
         data = self._DoReadFile('156_mkimage.dts')
 
         # Just check that the data appears in the file somewhere
@@ -3744,6 +3755,7 @@ class TestFunctional(unittest.TestCase):
 
     def testMkimageMissing(self):
         """Test that binman still produces an image if mkimage is missing"""
+        self._SetupSplElf()
         with test_util.capture_sys_output() as (_, stderr):
             self._DoTestFile('156_mkimage.dts',
                              force_missing_bintools='mkimage')
@@ -3856,6 +3868,7 @@ class TestFunctional(unittest.TestCase):
 
     def testSimpleFit(self):
         """Test an image with a FIT inside"""
+        self._SetupSplElf()
         data = self._DoReadFile('161_fit.dts')
         self.assertEqual(U_BOOT_DATA, data[:len(U_BOOT_DATA)])
         self.assertEqual(U_BOOT_NODTB_DATA, data[-len(U_BOOT_NODTB_DATA):])
@@ -5375,6 +5388,7 @@ fdt         fdtmap                Extract the devicetree blob from the fdtmap
 
     def testFitSubentryHashSubnode(self):
         """Test an image with a FIT inside"""
+        self._SetupSplElf()
         data, _, _, out_dtb_name = self._DoReadFileDtb(
             '221_fit_subentry_hash.dts', use_real_dtb=True, update_dtb=True)
 
@@ -5893,6 +5907,7 @@ fdt         fdtmap                Extract the devicetree blob from the fdtmap
 
     def testMkimageImagename(self):
         """Test using mkimage with -n holding the data too"""
+        self._SetupSplElf()
         data = self._DoReadFile('242_mkimage_name.dts')
 
         # Check that the data appears in the file somewhere
@@ -5910,6 +5925,7 @@ fdt         fdtmap                Extract the devicetree blob from the fdtmap
 
     def testMkimageImage(self):
         """Test using mkimage with -n holding the data too"""
+        self._SetupSplElf()
         data = self._DoReadFile('243_mkimage_image.dts')
 
         # Check that the data appears in the file somewhere
@@ -5930,6 +5946,7 @@ fdt         fdtmap                Extract the devicetree blob from the fdtmap
 
     def testMkimageImageNoContent(self):
         """Test using mkimage with -n and no data"""
+        self._SetupSplElf()
         with self.assertRaises(ValueError) as exc:
             self._DoReadFile('244_mkimage_image_no_content.dts')
         self.assertIn('Could not complete processing of contents',
@@ -5937,6 +5954,7 @@ fdt         fdtmap                Extract the devicetree blob from the fdtmap
 
     def testMkimageImageBad(self):
         """Test using mkimage with imagename node and data-to-imagename"""
+        self._SetupSplElf()
         with self.assertRaises(ValueError) as exc:
             self._DoReadFile('245_mkimage_image_bad.dts')
         self.assertIn('Cannot use both imagename node and data-to-imagename',
@@ -5952,6 +5970,7 @@ fdt         fdtmap                Extract the devicetree blob from the fdtmap
 
     def testMkimageCollection(self):
         """Test using a collection referring to an entry in a mkimage entry"""
+        self._SetupSplElf()
         data = self._DoReadFile('247_mkimage_coll.dts')
         expect = U_BOOT_SPL_DATA + U_BOOT_DATA
         self.assertEqual(expect, data[:len(expect)])
@@ -6037,6 +6056,8 @@ fdt         fdtmap                Extract the devicetree blob from the fdtmap
 
     def testMkimageMultipleDataFiles(self):
         """Test passing multiple files to mkimage in a mkimage entry"""
+        self._SetupSplElf()
+        self._SetupTplElf()
         data = self._DoReadFile('252_mkimage_mult_data.dts')
         # Size of files are packed in their 4B big-endian format
         expect = struct.pack('>I', len(U_BOOT_TPL_DATA))
@@ -6051,8 +6072,42 @@ fdt         fdtmap                Extract the devicetree blob from the fdtmap
         expect += U_BOOT_SPL_DATA
         self.assertEqual(expect, data[-len(expect):])
 
+    def testMkimageMultipleExpanded(self):
+        """Test passing multiple files to mkimage in a mkimage entry"""
+        self._SetupSplElf()
+        self._SetupTplElf()
+        entry_args = {
+            'spl-bss-pad': 'y',
+            'spl-dtb': 'y',
+        }
+        data = self._DoReadFileDtb('252_mkimage_mult_data.dts',
+                                   use_expanded=True, entry_args=entry_args)[0]
+        pad_len = 10
+        tpl_expect = U_BOOT_TPL_DATA
+        spl_expect = U_BOOT_SPL_NODTB_DATA + tools.get_bytes(0, pad_len)
+        spl_expect += U_BOOT_SPL_DTB_DATA
+
+        content = data[0x40:]
+        lens = struct.unpack('>III', content[:12])
+
+        # Size of files are packed in their 4B big-endian format
+        # Size info is always followed by a 4B zero value.
+        self.assertEqual(len(tpl_expect), lens[0])
+        self.assertEqual(len(spl_expect), lens[1])
+        self.assertEqual(0, lens[2])
+
+        rest = content[12:]
+        self.assertEqual(tpl_expect, rest[:len(tpl_expect)])
+
+        rest = rest[len(tpl_expect):]
+        align_pad = len(tpl_expect) % 4
+        self.assertEqual(tools.get_bytes(0, align_pad), rest[:align_pad])
+        rest = rest[align_pad:]
+        self.assertEqual(spl_expect, rest)
+
     def testMkimageMultipleNoContent(self):
         """Test passing multiple data files to mkimage with one data file having no content"""
+        self._SetupSplElf()
         with self.assertRaises(ValueError) as exc:
             self._DoReadFile('253_mkimage_mult_no_content.dts')
         self.assertIn('Could not complete processing of contents',
@@ -6060,6 +6115,7 @@ fdt         fdtmap                Extract the devicetree blob from the fdtmap
 
     def testMkimageFilename(self):
         """Test using mkimage to build a binary with a filename"""
+        self._SetupSplElf()
         retcode = self._DoTestFile('254_mkimage_filename.dts')
         self.assertEqual(0, retcode)
         fname = tools.get_output_filename('mkimage-test.bin')
@@ -6534,6 +6590,7 @@ fdt         fdtmap                Extract the devicetree blob from the fdtmap
 
     def testReplaceFitSibling(self):
         """Test an image with a FIT inside where we replace its sibling"""
+        self._SetupSplElf()
         fname = TestFunctional._MakeInputFile('once', b'available once')
         self._DoReadFileRealDtb('277_replace_fit_sibling.dts')
         os.remove(fname)
@@ -6608,7 +6665,7 @@ fdt         fdtmap                Extract the devicetree blob from the fdtmap
                 Private key
                 DTB
         """
-
+        self._SetupSplElf()
         data = self._DoReadFileRealDtb(dts)
         updated_fname = tools.get_output_filename('image-updated.bin')
         tools.write_file(updated_fname, data)
@@ -6683,6 +6740,7 @@ fdt         fdtmap                Extract the devicetree blob from the fdtmap
 
     def testSymbolNoWrite(self):
         """Test disabling of symbol writing"""
+        self._SetupSplElf()
         self.checkSymbols('282_symbols_disable.dts', U_BOOT_SPL_DATA, 0x1c,
                           no_write_symbols=True)
 
@@ -6696,6 +6754,13 @@ fdt         fdtmap                Extract the devicetree blob from the fdtmap
                           entry_args=entry_args, use_expanded=True,
                           no_write_symbols=True)
 
+    def testMkimageSpecial(self):
+        """Test mkimage ignores special hash-1 node"""
+        data = self._DoReadFile('283_mkimage_special.dts')
+
+        # Just check that the data appears in the file somewhere
+        self.assertIn(U_BOOT_DATA, data)
+
 
 if __name__ == "__main__":
     unittest.main()
diff --git a/tools/binman/test/283_mkimage_special.dts b/tools/binman/test/283_mkimage_special.dts
new file mode 100644
index 000000000000..c234093e6ece
--- /dev/null
+++ b/tools/binman/test/283_mkimage_special.dts
@@ -0,0 +1,24 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+/dts-v1/;
+
+/ {
+	#address-cells = <1>;
+	#size-cells = <1>;
+
+	binman {
+		mkimage {
+			args = "-T script";
+
+			u-boot {
+			};
+
+			hash {
+			};
+
+			imagename {
+				type = "u-boot";
+			};
+		};
+	};
+};
-- 
2.41.0.455.g037347b96a-goog



More information about the U-Boot mailing list