From fadad3a64a12cf279d31088aebc45806ec86fb4e Mon Sep 17 00:00:00 2001 From: Marek Vasut Date: Tue, 18 Jul 2023 07:23:58 -0600 Subject: [PATCH] binman: Convert mkimage to Entry_section This is needed to handle mkimage with inner section located itself in a section. Signed-off-by: Marek Vasut Use BuildSectionData() instead of ObtainContents(), add tests and a few other minor fixes: Signed-off-by: Simon Glass --- 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 f20f322..0d4cb94 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 cb3e106..31ac05d 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 dabb3f6..db348de 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 0000000..c234093 --- /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.7.4