From fe05701b058c2944c288b60f4839d9b552acf059 Mon Sep 17 00:00:00 2001 From: Alper Nebi Yasak Date: Mon, 31 Aug 2020 12:58:20 +0300 Subject: [PATCH] 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 Reviewed-by: Simon Glass Reinstate check in testPadInSections(), squash in "binman: Allow FIT binaries to have missing external blobs" Signed-off-by: Simon Glass Reviewed-by: Alper Nebi Yasak --- tools/binman/etype/fit.py | 56 ++++++++++++------ tools/binman/ftest.py | 32 +++++++++++ .../test/167_fit_image_subentry_alignment.dts | 57 +++++++++++++++++++ tools/binman/test/168_fit_missing_blob.dts | 41 +++++++++++++ 4 files changed, 169 insertions(+), 17 deletions(-) create mode 100644 tools/binman/test/167_fit_image_subentry_alignment.dts create mode 100644 tools/binman/test/168_fit_missing_blob.dts diff --git a/tools/binman/etype/fit.py b/tools/binman/etype/fit.py index 75712f4409..615b2fd877 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 53da709d51..e672967dba 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 0000000000..360cac5266 --- /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 0000000000..e007aa41d8 --- /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 { + }; + }; +};