Merge branch '2022-04-23-binman-updates'

- Assorted binman updates, and add Alper as a maintainer, after talking
  with Simon.

MAINTAINERS

@@ -690,6 +690,7 @@ F:	arch/arm/dts/phytium-durian.dts
 
 BINMAN
 M:	Simon Glass <sjg@chromium.org>
+M:	Alper Nebi Yasak <alpernebiyasak@gmail.com>
 S:	Maintained
 F:	tools/binman/
 

tools/binman/control.py

@@ -299,10 +299,11 @@ def BeforeReplace(image, allow_resize):
     """
     state.PrepareFromLoadedData(image)
     image.LoadData()
+    image.CollectBintools()
 
     # If repacking, drop the old offset/size values except for the original
     # ones, so we are only left with the constraints.
-    if allow_resize:
+    if image.allow_repack and allow_resize:
         image.ResetForPack()
 
 

tools/binman/entry.py

@@ -775,7 +775,7 @@ features to produce new behaviours.
         node = self._node
         while node.parent:
             node = node.parent
-            if node.name == 'binman':
+            if node.name in ('binman', '/'):
                 break
             name = '%s.%s' % (node.name, name)
         return name

tools/binman/etype/_testing.py

@@ -39,6 +39,10 @@ class Entry__testing(Entry):
             error if not)
         force-bad-datatype: Force a call to GetEntryArgsOrProps() with a bad
             data type (generating an error)
+        require-bintool-for-contents: Raise an error if the specified
+            bintool isn't usable in ObtainContents()
+        require-bintool-for-pack: Raise an error if the specified
+            bintool isn't usable in Pack()
     """
     def __init__(self, section, etype, node):
         super().__init__(section, etype, node)
@@ -82,6 +86,26 @@ class Entry__testing(Entry):
         self.return_contents = True
         self.contents = b'aa'
 
+        # Set to the required bintool when collecting bintools.
+        self.bintool_for_contents = None
+        self.require_bintool_for_contents = fdt_util.GetString(self._node,
+                                              'require-bintool-for-contents')
+        if self.require_bintool_for_contents == '':
+            self.require_bintool_for_contents = '_testing'
+
+        self.bintool_for_pack = None
+        self.require_bintool_for_pack = fdt_util.GetString(self._node,
+                                                  'require-bintool-for-pack')
+        if self.require_bintool_for_pack == '':
+            self.require_bintool_for_pack = '_testing'
+
+    def Pack(self, offset):
+        """Figure out how to pack the entry into the section"""
+        if self.require_bintool_for_pack:
+            if self.bintool_for_pack is None:
+                self.Raise("Required bintool unusable in Pack()")
+        return super().Pack(offset)
+
     def ObtainContents(self, fake_size=0):
         if self.return_unknown_contents or not self.return_contents:
             return False
@@ -92,6 +116,9 @@ class Entry__testing(Entry):
         self.contents_size = len(self.data)
         if self.return_contents_once:
             self.return_contents = False
+        if self.require_bintool_for_contents:
+            if self.bintool_for_contents is None:
+                self.Raise("Required bintool unusable in ObtainContents()")
         return True
 
     def GetOffsets(self):
@@ -127,3 +154,12 @@ class Entry__testing(Entry):
         if not self.never_complete_process_fdt:
             self.process_fdt_ready = True
         return ready
+
+    def AddBintools(self, btools):
+        """Add the bintools used by this entry type"""
+        if self.require_bintool_for_contents is not None:
+            self.bintool_for_contents = self.AddBintool(btools,
+                    self.require_bintool_for_contents)
+        if self.require_bintool_for_pack is not None:
+            self.bintool_for_pack = self.AddBintool(btools,
+                    self.require_bintool_for_pack)

tools/binman/etype/fit.py

@@ -380,11 +380,12 @@ class Entry_fit(Entry_section):
                 # section entries for them here to merge the content subnodes
                 # together and put the merged contents in the subimage node's
                 # 'data' property later.
-                entry = Entry.Create(self.section, node, etype='section')
+                entry = Entry.Create(self, node, etype='section')
                 entry.ReadNode()
                 # The hash subnodes here are for mkimage, not binman.
                 entry.SetUpdateHash(False)
-                self._entries[rel_path] = entry
+                image_name = rel_path[len('/images/'):]
+                self._entries[image_name] = entry
 
             for subnode in node.subnodes:
                 _add_entries(base_node, depth + 1, subnode)
@@ -630,7 +631,8 @@ class Entry_fit(Entry_section):
 
             has_images = depth == 2 and in_images
             if has_images:
-                entry = self._priv_entries[rel_path]
+                image_name = rel_path[len('/images/'):]
+                entry = self._priv_entries[image_name]
                 data = entry.GetData()
                 fsw.property('data', bytes(data))
 
@@ -643,12 +645,12 @@ class Entry_fit(Entry_section):
                     # fsw.add_node() or _add_node() for it.
                     pass
                 elif self.GetImage().generate and subnode.name.startswith('@'):
-                    entry = self._priv_entries.get(subnode_path)
+                    entry = self._priv_entries.get(subnode.name)
                     _gen_node(base_node, subnode, depth, in_images, entry)
                     # This is a generator (template) entry, so remove it from
                     # the list of entries used by PackEntries(), etc. Otherwise
                     # it will appear in the binman output
-                    to_remove.append(subnode_path)
+                    to_remove.append(subnode.name)
                 else:
                     with fsw.add_node(subnode.name):
                         _add_node(base_node, depth + 1, subnode)
@@ -693,7 +695,8 @@ class Entry_fit(Entry_section):
         fdt = Fdt.FromData(self.GetData())
         fdt.Scan()
 
-        for path, section in self._entries.items():
+        for image_name, section in self._entries.items():
+            path = f"/images/{image_name}"
             node = fdt.GetNode(path)
 
             data_prop = node.props.get("data")

tools/binman/etype/section.py

@@ -788,6 +788,9 @@ class Entry_section(Entry):
                 data = new_data
         return data
 
+    def WriteData(self, data, decomp=True):
+        self.Raise("Replacing sections is not implemented yet")
+
     def WriteChildData(self, child):
         return True
 

tools/binman/ftest.py

@@ -3764,6 +3764,13 @@ class TestFunctional(unittest.TestCase):
         self.assertEqual(len(kernel_data), int(data_sizes[0].split()[0]))
         self.assertEqual(len(fdt1_data), int(data_sizes[1].split()[0]))
 
+        # Check if entry listing correctly omits /images/
+        image = control.images['image']
+        fit_entry = image.GetEntries()['fit']
+        subentries = list(fit_entry.GetEntries().keys())
+        expected = ['kernel', 'fdt-1']
+        self.assertEqual(expected, subentries)
+
     def testSimpleFit(self):
         """Test an image with a FIT inside"""
         data = self._DoReadFile('161_fit.dts')
@@ -5523,5 +5530,178 @@ fdt         fdtmap                Extract the devicetree blob from the fdtmap
         with self.assertRaises(ValueError) as e:
             data = self._DoReadFile('231_pre_load_invalid_key.dts')
 
+    def _CheckSafeUniqueNames(self, *images):
+        """Check all entries of given images for unsafe unique names"""
+        for image in images:
+            entries = {}
+            image._CollectEntries(entries, {}, image)
+            for entry in entries.values():
+                uniq = entry.GetUniqueName()
+
+                # Used as part of a filename, so must not be absolute paths.
+                self.assertFalse(os.path.isabs(uniq))
+
+    def testSafeUniqueNames(self):
+        """Test entry unique names are safe in single image configuration"""
+        data = self._DoReadFileRealDtb('230_unique_names.dts')
+
+        orig_image = control.images['image']
+        image_fname = tools.get_output_filename('image.bin')
+        image = Image.FromFile(image_fname)
+
+        self._CheckSafeUniqueNames(orig_image, image)
+
+    def testSafeUniqueNamesMulti(self):
+        """Test entry unique names are safe with multiple images"""
+        data = self._DoReadFileRealDtb('231_unique_names_multi.dts')
+
+        orig_image = control.images['image']
+        image_fname = tools.get_output_filename('image.bin')
+        image = Image.FromFile(image_fname)
+
+        self._CheckSafeUniqueNames(orig_image, image)
+
+    def testReplaceCmdWithBintool(self):
+        """Test replacing an entry that needs a bintool to pack"""
+        data = self._DoReadFileRealDtb('232_replace_with_bintool.dts')
+        expected = U_BOOT_DATA + b'aa'
+        self.assertEqual(expected, data[:len(expected)])
+
+        try:
+            tmpdir, updated_fname = self._SetupImageInTmpdir()
+            fname = os.path.join(tmpdir, 'update-testing.bin')
+            tools.write_file(fname, b'zz')
+            self._DoBinman('replace', '-i', updated_fname,
+                           '_testing', '-f', fname)
+
+            data = tools.read_file(updated_fname)
+            expected = U_BOOT_DATA + b'zz'
+            self.assertEqual(expected, data[:len(expected)])
+        finally:
+            shutil.rmtree(tmpdir)
+
+    def testReplaceCmdOtherWithBintool(self):
+        """Test replacing an entry when another needs a bintool to pack"""
+        data = self._DoReadFileRealDtb('232_replace_with_bintool.dts')
+        expected = U_BOOT_DATA + b'aa'
+        self.assertEqual(expected, data[:len(expected)])
+
+        try:
+            tmpdir, updated_fname = self._SetupImageInTmpdir()
+            fname = os.path.join(tmpdir, 'update-u-boot.bin')
+            tools.write_file(fname, b'x' * len(U_BOOT_DATA))
+            self._DoBinman('replace', '-i', updated_fname,
+                           'u-boot', '-f', fname)
+
+            data = tools.read_file(updated_fname)
+            expected = b'x' * len(U_BOOT_DATA) + b'aa'
+            self.assertEqual(expected, data[:len(expected)])
+        finally:
+            shutil.rmtree(tmpdir)
+
+    def testReplaceResizeNoRepackSameSize(self):
+        """Test replacing entries with same-size data without repacking"""
+        expected = b'x' * len(U_BOOT_DATA)
+        data, expected_fdtmap, _ = self._RunReplaceCmd('u-boot', expected)
+        self.assertEqual(expected, data)
+
+        path, fdtmap = state.GetFdtContents('fdtmap')
+        self.assertIsNotNone(path)
+        self.assertEqual(expected_fdtmap, fdtmap)
+
+    def testReplaceResizeNoRepackSmallerSize(self):
+        """Test replacing entries with smaller-size data without repacking"""
+        new_data = b'x'
+        data, expected_fdtmap, _ = self._RunReplaceCmd('u-boot', new_data)
+        expected = new_data.ljust(len(U_BOOT_DATA), b'\0')
+        self.assertEqual(expected, data)
+
+        path, fdtmap = state.GetFdtContents('fdtmap')
+        self.assertIsNotNone(path)
+        self.assertEqual(expected_fdtmap, fdtmap)
+
+    def testExtractFit(self):
+        """Test extracting a FIT section"""
+        self._DoReadFileRealDtb('233_fit_extract_replace.dts')
+        image_fname = tools.get_output_filename('image.bin')
+
+        fit_data = control.ReadEntry(image_fname, 'fit')
+        fit = fdt.Fdt.FromData(fit_data)
+        fit.Scan()
+
+        # Check subentry data inside the extracted fit
+        for node_path, expected in [
+            ('/images/kernel', U_BOOT_DATA),
+            ('/images/fdt-1', U_BOOT_NODTB_DATA),
+            ('/images/scr-1', COMPRESS_DATA),
+        ]:
+            node = fit.GetNode(node_path)
+            data = fit.GetProps(node)['data'].bytes
+            self.assertEqual(expected, data)
+
+    def testExtractFitSubentries(self):
+        """Test extracting FIT section subentries"""
+        self._DoReadFileRealDtb('233_fit_extract_replace.dts')
+        image_fname = tools.get_output_filename('image.bin')
+
+        for entry_path, expected in [
+            ('fit/kernel', U_BOOT_DATA),
+            ('fit/kernel/u-boot', U_BOOT_DATA),
+            ('fit/fdt-1', U_BOOT_NODTB_DATA),
+            ('fit/fdt-1/u-boot-nodtb', U_BOOT_NODTB_DATA),
+            ('fit/scr-1', COMPRESS_DATA),
+            ('fit/scr-1/blob', COMPRESS_DATA),
+        ]:
+            data = control.ReadEntry(image_fname, entry_path)
+            self.assertEqual(expected, data)
+
+    def testReplaceFitSubentryLeafSameSize(self):
+        """Test replacing a FIT leaf subentry with same-size data"""
+        new_data = b'x' * len(U_BOOT_DATA)
+        data, expected_fdtmap, _ = self._RunReplaceCmd(
+            'fit/kernel/u-boot', new_data,
+            dts='233_fit_extract_replace.dts')
+        self.assertEqual(new_data, data)
+
+        path, fdtmap = state.GetFdtContents('fdtmap')
+        self.assertIsNotNone(path)
+        self.assertEqual(expected_fdtmap, fdtmap)
+
+    def testReplaceFitSubentryLeafBiggerSize(self):
+        """Test replacing a FIT leaf subentry with bigger-size data"""
+        new_data = b'ub' * len(U_BOOT_NODTB_DATA)
+        data, expected_fdtmap, _ = self._RunReplaceCmd(
+            'fit/fdt-1/u-boot-nodtb', new_data,
+            dts='233_fit_extract_replace.dts')
+        self.assertEqual(new_data, data)
+
+        # Will be repacked, so fdtmap must change
+        path, fdtmap = state.GetFdtContents('fdtmap')
+        self.assertIsNotNone(path)
+        self.assertNotEqual(expected_fdtmap, fdtmap)
+
+    def testReplaceFitSubentryLeafSmallerSize(self):
+        """Test replacing a FIT leaf subentry with smaller-size data"""
+        new_data = b'x'
+        expected = new_data.ljust(len(U_BOOT_NODTB_DATA), b'\0')
+        data, expected_fdtmap, _ = self._RunReplaceCmd(
+            'fit/fdt-1/u-boot-nodtb', new_data,
+            dts='233_fit_extract_replace.dts')
+        self.assertEqual(expected, data)
+
+        path, fdtmap = state.GetFdtContents('fdtmap')
+        self.assertIsNotNone(path)
+        self.assertEqual(expected_fdtmap, fdtmap)
+
+    @unittest.expectedFailure
+    def testReplaceSectionSimple(self):
+        """Test replacing a simple section with arbitrary data"""
+        new_data = b'w' * len(COMPRESS_DATA + U_BOOT_DATA)
+        data, expected_fdtmap, _ = self._RunReplaceCmd(
+            'section', new_data,
+            dts='234_replace_section_simple.dts')
+        self.assertEqual(new_data, data)
+
+
 if __name__ == "__main__":
     unittest.main()

tools/binman/main.py

@@ -9,7 +9,6 @@
 
 """See README for more information"""
 
-from distutils.sysconfig import get_python_lib
 import os
 import site
 import sys
@@ -44,12 +43,6 @@ sys.path.insert(2, os.path.join(srctree, 'scripts/dtc/pylibfdt'))
 sys.path.insert(2, os.path.join(srctree, 'build-sandbox/scripts/dtc/pylibfdt'))
 sys.path.insert(2, os.path.join(srctree, 'build-sandbox_spl/scripts/dtc/pylibfdt'))
 
-# When running under python-coverage on Ubuntu 16.04, the dist-packages
-# directories are dropped from the python path. Add them in so that we can find
-# the elffile module. We could use site.getsitepackages() here but unfortunately
-# that is not available in a virtualenv.
-sys.path.append(get_python_lib())
-
 from binman import cmdline
 from binman import control
 from patman import test_util

tools/binman/test/230_unique_names.dts

@@ -0,0 +1,34 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+/dts-v1/;
+
+/ {
+	#address-cells = <1>;
+	#size-cells = <1>;
+
+	binman {
+		size = <0xc00>;
+		allow-repack;
+
+		u-boot {
+		};
+
+		fdtmap {
+		};
+
+		u-boot2 {
+			type = "u-boot";
+		};
+
+		text {
+			text = "some text";
+		};
+
+		u-boot-dtb {
+		};
+
+		image-header {
+			location = "end";
+		};
+	};
+};

tools/binman/test/231_unique_names_multi.dts

@@ -0,0 +1,38 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+/dts-v1/;
+
+/ {
+	#address-cells = <1>;
+	#size-cells = <1>;
+
+	binman {
+		multiple-images;
+
+		image {
+			size = <0xc00>;
+			allow-repack;
+
+			u-boot {
+			};
+
+			fdtmap {
+			};
+
+			u-boot2 {
+				type = "u-boot";
+			};
+
+			text {
+				text = "some text";
+			};
+
+			u-boot-dtb {
+			};
+
+			image-header {
+				location = "end";
+			};
+		};
+	};
+};

tools/binman/test/232_replace_with_bintool.dts

@@ -0,0 +1,39 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+/dts-v1/;
+
+/ {
+	#address-cells = <1>;
+	#size-cells = <1>;
+
+	binman {
+		size = <0xc00>;
+		allow-repack;
+
+		u-boot {
+		};
+
+		_testing {
+			require-bintool-for-contents;
+			require-bintool-for-pack;
+		};
+
+		fdtmap {
+		};
+
+		u-boot2 {
+			type = "u-boot";
+		};
+
+		text {
+			text = "some text";
+		};
+
+		u-boot-dtb {
+		};
+
+		image-header {
+			location = "end";
+		};
+	};
+};

tools/binman/test/233_fit_extract_replace.dts

@@ -0,0 +1,74 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+/dts-v1/;
+
+/ {
+	#address-cells = <1>;
+	#size-cells = <1>;
+
+	binman {
+		allow-repack;
+
+		fill {
+			size = <0x1000>;
+			fill-byte = [77];
+		};
+
+		fit {
+			description = "test-desc";
+			#address-cells = <1>;
+
+			images {
+				kernel {
+					description = "test u-boot";
+					type = "kernel";
+					arch = "arm64";
+					os = "linux";
+					compression = "none";
+					load = <00000000>;
+					entry = <00000000>;
+
+					u-boot {
+					};
+				};
+
+				fdt-1 {
+					description = "test u-boot-nodtb";
+					type = "flat_dt";
+					arch = "arm64";
+					compression = "none";
+
+					u-boot-nodtb {
+					};
+				};
+
+				scr-1 {
+					description = "test blob";
+					type = "script";
+					arch = "arm64";
+					compression = "none";
+
+					blob {
+						filename = "compress";
+					};
+				};
+			};
+
+			configurations {
+				default = "conf-1";
+
+				conf-1 {
+					description = "Kernel with FDT blob";
+					kernel = "kernel";
+					fdt = "fdt-1";
+				};
+			};
+		};
+
+		u-boot-dtb {
+		};
+
+		fdtmap {
+		};
+	};
+};

tools/binman/test/234_replace_section_simple.dts

@@ -0,0 +1,23 @@
+// SPDX-License-Identifier: GPL-2.0+
+/dts-v1/;
+
+/ {
+	binman {
+		allow-repack;
+
+		u-boot-dtb {
+		};
+
+		section {
+			blob {
+				filename = "compress";
+			};
+
+			u-boot {
+			};
+		};
+
+		fdtmap {
+		};
+	};
+};