binman: Build FIT image subentries with the section etype

When reading subentries of each image, the FIT entry type directly
concatenates their contents without padding them according to their
offset, size, align, align-size, align-end, pad-before, pad-after
properties.

This patch makes sure these properties are respected by offloading this
image-data building to the section etype, where each subnode of the
"images" node is processed as a section. Alignments and offsets are
respective to the beginning of each image. For example, the following
fragment can end up having "u-boot-spl" start at 0x88 within the final
FIT binary, while "u-boot" would then end up starting at e.g. 0x20088.

	fit {
		description = "example";

		images {
			kernel-1 {
				description = "U-Boot with SPL";
				type = "kernel";
				arch = "arm64";
				os = "linux";
				compression = "none";

				u-boot-spl {
				};
				u-boot {
					align = <0x10000>;
				};
			};
		};
	}

Signed-off-by: Alper Nebi Yasak <alpernebiyasak@gmail.com>
Reviewed-by: Simon Glass <sjg@chromium.org>
Reinstate check in testPadInSections(), squash in
   "binman: Allow FIT binaries to have missing external blobs"
Signed-off-by: Simon Glass <sjg@chromium.org>
Reviewed-by: Alper Nebi Yasak <alpernebiyasak@gmail.com>

4 files changed, 169 insertions, 17 deletions

diff --git a/tools/binman/etype/fit.py b/tools/binman/etype/fit.py
index 75712f4..615b2fd 100644
--- a/tools/binman/etype/fit.py
+++ b/tools/binman/etype/fit.py
@@ -55,14 +55,14 @@ class Entry_fit(Entry):
         """
         Members:
             _fit: FIT file being built
-            _fit_content: dict:
+            _fit_sections: dict:
                 key: relative path to entry Node (from the base of the FIT)
-                value: List of Entry objects comprising the contents of this
+                value: Entry_section object comprising the contents of this
                     node
         """
         super().__init__(section, etype, node)
         self._fit = None
-        self._fit_content = defaultdict(list)
+        self._fit_sections = {}
         self._fit_props = {}
 
     def ReadNode(self):
@@ -91,15 +91,23 @@ class Entry_fit(Entry):
 
             rel_path = node.path[len(base_node.path):]
             has_images = depth == 2 and rel_path.startswith('/images/')
+            if has_images:
+                # This node is a FIT subimage node (e.g. "/images/kernel")
+                # containing content nodes. We collect the subimage nodes and
+                # 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.ReadNode()
+                self._fit_sections[rel_path] = entry
+
             for subnode in node.subnodes:
                 if has_images and not (subnode.name.startswith('hash') or
                                        subnode.name.startswith('signature')):
-                    # This is a content node. We collect all of these together
-                    # and put them in the 'data' property. They do not appear
-                    # in the FIT.
-                    entry = Entry.Create(self.section, subnode)
-                    entry.ReadNode()
-                    self._fit_content[rel_path].append(entry)
+                    # This subnode is a content node not meant to appear in
+                    # the FIT (e.g. "/images/kernel/u-boot"), so don't call
+                    # fsw.add_node() or _AddNode() for it.
+                    pass
                 else:
                     with fsw.add_node(subnode.name):
                         _AddNode(base_node, depth + 1, subnode)
@@ -123,9 +131,8 @@ class Entry_fit(Entry):
         This adds the 'data' properties to the input ITB (Image-tree Binary)
         then runs mkimage to process it.
         """
+        # self._BuildInput() either returns bytes or raises an exception.
         data = self._BuildInput(self._fdt)
-        if data == False:
-            return False
         uniq = self.GetUniqueName()
         input_fname = tools.GetOutputFilename('%s.itb' % uniq)
         output_fname = tools.GetOutputFilename('%s.fit' % uniq)
@@ -150,15 +157,30 @@ class Entry_fit(Entry):
         Returns:
             New fdt contents (bytes)
         """
-        for path, entries in self._fit_content.items():
+        for path, section in self._fit_sections.items():
             node = fdt.GetNode(path)
-            data = b''
-            for entry in entries:
-                if not entry.ObtainContents():
-                    return False
-                data += entry.GetData()
+            # Entry_section.ObtainContents() either returns True or
+            # raises an exception.
+            section.ObtainContents()
+            section.Pack(0)
+            data = section.GetData()
             node.AddData('data', data)
 
         fdt.Sync(auto_resize=True)
         data = fdt.GetContents()
         return data
+
+    def CheckMissing(self, missing_list):
+        """Check if any entries in this FIT have missing external blobs
+
+        If there are missing blobs, the entries are added to the list
+
+        Args:
+            missing_list: List of Entry objects to be added to
+        """
+        for path, section in self._fit_sections.items():
+            section.CheckMissing(missing_list)
+
+    def SetAllowMissing(self, allow_missing):
+        for section in self._fit_sections.values():
+            section.SetAllowMissing(allow_missing)
diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
index 53da709..e672967 100644
--- a/tools/binman/ftest.py
+++ b/tools/binman/ftest.py
@@ -3491,5 +3491,37 @@ class TestFunctional(unittest.TestCase):
                     U_BOOT_DATA)
         self.assertEqual(expected, data)
 
+    def testFitImageSubentryAlignment(self):
+        """Test relative alignability of FIT image subentries"""
+        entry_args = {
+            'test-id': TEXT_DATA,
+        }
+        data, _, _, _ = self._DoReadFileDtb('167_fit_image_subentry_alignment.dts',
+                                            entry_args=entry_args)
+        dtb = fdt.Fdt.FromData(data)
+        dtb.Scan()
+
+        node = dtb.GetNode('/images/kernel')
+        data = dtb.GetProps(node)["data"].bytes
+        align_pad = 0x10 - (len(U_BOOT_SPL_DATA) % 0x10)
+        expected = (tools.GetBytes(0, 0x20) + U_BOOT_SPL_DATA +
+                    tools.GetBytes(0, align_pad) + U_BOOT_DATA)
+        self.assertEqual(expected, data)
+
+        node = dtb.GetNode('/images/fdt-1')
+        data = dtb.GetProps(node)["data"].bytes
+        expected = (U_BOOT_SPL_DTB_DATA + tools.GetBytes(0, 20) +
+                    tools.ToBytes(TEXT_DATA) + tools.GetBytes(0, 30) +
+                    U_BOOT_DTB_DATA)
+        self.assertEqual(expected, data)
+
+    def testFitExtblobMissingOk(self):
+        """Test a FIT with a missing external blob that is allowed"""
+        with test_util.capture_sys_output() as (stdout, stderr):
+            self._DoTestFile('168_fit_missing_blob.dts',
+                             allow_missing=True)
+        err = stderr.getvalue()
+        self.assertRegex(err, "Image 'main-section'.*missing.*: blob-ext")
+
 if __name__ == "__main__":
     unittest.main()
diff --git a/tools/binman/test/167_fit_image_subentry_alignment.dts b/tools/binman/test/167_fit_image_subentry_alignment.dts
new file mode 100644
index 0000000..360cac5
--- /dev/null
+++ b/tools/binman/test/167_fit_image_subentry_alignment.dts
@@ -0,0 +1,57 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+/dts-v1/;
+
+/ {
+	#address-cells = <1>;
+	#size-cells = <1>;
+
+	binman {
+		fit {
+			description = "test-desc";
+			#address-cells = <1>;
+
+			images {
+				kernel {
+					description = "Offset-Align Test";
+					type = "kernel";
+					arch = "arm64";
+					os = "linux";
+					compression = "none";
+					load = <00000000>;
+					entry = <00000000>;
+					u-boot-spl {
+						offset = <0x20>;
+					};
+					u-boot {
+						align = <0x10>;
+					};
+				};
+				fdt-1 {
+					description = "Pad-Before-After Test";
+					type = "flat_dt";
+					arch = "arm64";
+					compression = "none";
+					u-boot-spl-dtb {
+					};
+					text {
+						text-label = "test-id";
+						pad-before = <20>;
+						pad-after = <30>;
+					};
+					u-boot-dtb {
+					};
+				};
+			};
+
+			configurations {
+				default = "conf-1";
+				conf-1 {
+					description = "Kernel with FDT blob";
+					kernel = "kernel";
+					fdt = "fdt-1";
+				};
+			};
+		};
+	};
+};
diff --git a/tools/binman/test/168_fit_missing_blob.dts b/tools/binman/test/168_fit_missing_blob.dts
new file mode 100644
index 0000000..e007aa4
--- /dev/null
+++ b/tools/binman/test/168_fit_missing_blob.dts
@@ -0,0 +1,41 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+/dts-v1/;
+
+/ {
+	#address-cells = <1>;
+	#size-cells = <1>;
+
+	binman {
+		u-boot {
+		};
+		fit {
+			description = "test-desc";
+			#address-cells = <1>;
+			fit,fdt-list = "of-list";
+
+			images {
+				kernel {
+					description = "ATF BL31";
+					type = "kernel";
+					arch = "ppc";
+					os = "linux";
+					compression = "gzip";
+					load = <00000000>;
+					entry = <00000000>;
+					hash-1 {
+						algo = "crc32";
+					};
+					hash-2 {
+						algo = "sha1";
+					};
+					blob-ext {
+						filename = "missing";
+					};
+				};
+			};
+		};
+		u-boot-nodtb {
+		};
+	};
+};