Add a pragma to deal with the code-coverage gap which drops binman down to 90% coverage.

Fixes: de65b122a25 (tools: Fall back to importlib_resources on Python 3.6)

Signed-off-by: Simon Glass sjg@chromium.org ---

(no changes since v1)

tools/binman/control.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/binman/control.py b/tools/binman/control.py index 68597c4e7792..7e2dd3541b96 100644 --- a/tools/binman/control.py +++ b/tools/binman/control.py @@ -9,7 +9,7 @@ from collections import OrderedDict import glob try: import importlib.resources -except ImportError: +except ImportError: # pragma: no cover # for Python 3.6 import importlib_resources import os

Some section types don't have a simple _entries list. Use the GetEntries() method in GetEntryContents() and other places to handle this.

This makes the behaviour more consistent.

Signed-off-by: Simon Glass sjg@chromium.org ---

(no changes since v1)

tools/binman/etype/section.py | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/tools/binman/etype/section.py b/tools/binman/etype/section.py index 77250a7525c6..d56cc11d1023 100644 --- a/tools/binman/etype/section.py +++ b/tools/binman/etype/section.py @@ -720,7 +720,7 @@ class Entry_section(Entry): next_todo.append(entry) return entry

- todo = self._entries.values() + todo = self.GetEntries().values() for passnum in range(3): threads = state.GetThreads() next_todo = [] @@ -893,7 +893,7 @@ class Entry_section(Entry): allow_missing: True if allowed, False if not allowed """ self.allow_missing = allow_missing - for entry in self._entries.values(): + for entry in self.GetEntries().values(): entry.SetAllowMissing(allow_missing)

def SetAllowFakeBlob(self, allow_fake): @@ -903,7 +903,7 @@ class Entry_section(Entry): allow_fake: True if allowed, False if not allowed """ super().SetAllowFakeBlob(allow_fake) - for entry in self._entries.values(): + for entry in self.GetEntries().values(): entry.SetAllowFakeBlob(allow_fake)

def CheckMissing(self, missing_list): @@ -915,7 +915,7 @@ class Entry_section(Entry): Args: missing_list: List of Entry objects to be added to """ - for entry in self._entries.values(): + for entry in self.GetEntries().values(): entry.CheckMissing(missing_list)

def CheckFakedBlobs(self, faked_blobs_list): @@ -926,7 +926,7 @@ class Entry_section(Entry): Args: faked_blobs_list: List of Entry objects to be added to """ - for entry in self._entries.values(): + for entry in self.GetEntries().values(): entry.CheckFakedBlobs(faked_blobs_list)

def CheckOptional(self, optional_list): @@ -937,7 +937,7 @@ class Entry_section(Entry): Args: optional_list (list): List of Entry objects to be added to """ - for entry in self._entries.values(): + for entry in self.GetEntries().values(): entry.CheckOptional(optional_list)

def check_missing_bintools(self, missing_list): @@ -949,7 +949,7 @@ class Entry_section(Entry): missing_list: List of Bintool objects to be added to """ super().check_missing_bintools(missing_list) - for entry in self._entries.values(): + for entry in self.GetEntries().values(): entry.check_missing_bintools(missing_list)

def _CollectEntries(self, entries, entries_by_name, add_entry): @@ -999,12 +999,12 @@ class Entry_section(Entry): entry.Raise(f'Missing required properties/entry args: {missing}')

def CheckAltFormats(self, alt_formats): - for entry in self._entries.values(): + for entry in self.GetEntries().values(): entry.CheckAltFormats(alt_formats)

def AddBintools(self, btools): super().AddBintools(btools) - for entry in self._entries.values(): + for entry in self.GetEntries().values(): entry.AddBintools(btools)

def read_elf_segments(self):

Some boards don't use symbol writing but do access the symbols in SPL. Provide an option to work around this.

Signed-off-by: Simon Glass sjg@chromium.org ---

(no changes since v1)

tools/binman/binman.rst | 7 ++++++ tools/binman/entry.py | 4 +++- tools/binman/etype/blob_phase.py | 5 ++++ tools/binman/ftest.py | 28 +++++++++++++++++++---- tools/binman/test/282_symbols_disable.dts | 25 ++++++++++++++++++++ 5 files changed, 64 insertions(+), 5 deletions(-) create mode 100644 tools/binman/test/282_symbols_disable.dts

diff --git a/tools/binman/binman.rst b/tools/binman/binman.rst index 23cbb99b4b0b..a4b31fe5397b 100644 --- a/tools/binman/binman.rst +++ b/tools/binman/binman.rst @@ -831,6 +831,13 @@ write-symbols: binman. This is automatic for certain entry types, e.g. `u-boot-spl`. See binman_syms_ for more information.

+no-write-symbols: + Disables symbol writing for this entry. This can be used in entry types + where symbol writing is automatic. For example, if `u-boot-spl` refers to + the `u_boot_any_image_pos` symbol but U-Boot is not available in the image + containing SPL, this can be used to disable the writing. Quite likely this + indicates a bug in your setup. + elf-filename: Sets the file name of a blob's associated ELF file. For example, if the blob is `zephyr.bin` then the ELF file may be `zephyr.elf`. This allows diff --git a/tools/binman/entry.py b/tools/binman/entry.py index 39456906a477..328b5bc568a9 100644 --- a/tools/binman/entry.py +++ b/tools/binman/entry.py @@ -158,6 +158,7 @@ class Entry(object): self.offset_from_elf = None self.preserve = False self.build_done = False + self.no_write_symbols = False

@staticmethod def FindEntryClass(etype, expanded): @@ -321,6 +322,7 @@ class Entry(object): 'offset-from-elf')

self.preserve = fdt_util.GetBool(self._node, 'preserve') + self.no_write_symbols = fdt_util.GetBool(self._node, 'no-write-symbols')

def GetDefaultFilename(self): return None @@ -695,7 +697,7 @@ class Entry(object): Args: section: Section containing the entry """ - if self.auto_write_symbols: + if self.auto_write_symbols and not self.no_write_symbols: # Check if we are writing symbols into an ELF file is_elf = self.GetDefaultFilename() == self.elf_fname elf.LookupAndWriteSymbols(self.elf_fname, self, section.GetImage(), diff --git a/tools/binman/etype/blob_phase.py b/tools/binman/etype/blob_phase.py index b937158756fd..951d9934050e 100644 --- a/tools/binman/etype/blob_phase.py +++ b/tools/binman/etype/blob_phase.py @@ -52,3 +52,8 @@ class Entry_blob_phase(Entry_section):

# Read entries again, now that we have some self.ReadEntries() + + # Propagate the no-write-symbols property + if self.no_write_symbols: + for entry in self._entries.values(): + entry.no_write_symbols = True diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index 43b4f850a691..dabb3f689fdb 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -1452,7 +1452,7 @@ class TestFunctional(unittest.TestCase): self.assertEqual(U_BOOT_SPL_NODTB_DATA, data[:len(U_BOOT_SPL_NODTB_DATA)])

def checkSymbols(self, dts, base_data, u_boot_offset, entry_args=None, - use_expanded=False): + use_expanded=False, no_write_symbols=False): """Check the image contains the expected symbol values

Args: @@ -1481,9 +1481,14 @@ class TestFunctional(unittest.TestCase): sym_values = struct.pack('<LLQLL', elf.BINMAN_SYM_MAGIC_VALUE, 0x00, u_boot_offset + len(U_BOOT_DATA), 0x10 + u_boot_offset, 0x04) - expected = (sym_values + base_data[24:] + - tools.get_bytes(0xff, 1) + U_BOOT_DATA + sym_values + - base_data[24:]) + if no_write_symbols: + expected = (base_data + + tools.get_bytes(0xff, 0x38 - len(base_data)) + + U_BOOT_DATA + base_data) + else: + expected = (sym_values + base_data[24:] + + tools.get_bytes(0xff, 1) + U_BOOT_DATA + sym_values + + base_data[24:]) self.assertEqual(expected, data)

def testSymbols(self): @@ -6676,6 +6681,21 @@ fdt fdtmap Extract the devicetree blob from the fdtmap ['fit']) self.assertIn("Node '/fit': Missing tool: 'mkimage'", str(e.exception))

+ def testSymbolNoWrite(self): + """Test disabling of symbol writing""" + self.checkSymbols('282_symbols_disable.dts', U_BOOT_SPL_DATA, 0x1c, + no_write_symbols=True) + + def testSymbolNoWriteExpanded(self): + """Test disabling of symbol writing in expanded entries""" + entry_args = { + 'spl-dtb': '1', + } + self.checkSymbols('282_symbols_disable.dts', U_BOOT_SPL_NODTB_DATA + + U_BOOT_SPL_DTB_DATA, 0x38, + entry_args=entry_args, use_expanded=True, + no_write_symbols=True) +

if __name__ == "__main__": unittest.main() diff --git a/tools/binman/test/282_symbols_disable.dts b/tools/binman/test/282_symbols_disable.dts new file mode 100644 index 000000000000..6efa9335041b --- /dev/null +++ b/tools/binman/test/282_symbols_disable.dts @@ -0,0 +1,25 @@ +// SPDX-License-Identifier: GPL-2.0+ + +/dts-v1/; + +/ { + #address-cells = <1>; + #size-cells = <1>; + + binman { + pad-byte = <0xff>; + u-boot-spl { + no-write-symbols; + }; + + u-boot { + offset = <0x38>; + no-expanded; + }; + + u-boot-spl2 { + type = "u-boot-spl"; + no-write-symbols; + }; + }; +};

Update the LookupAndWriteSymbols() function to return the number of symbols written. Also add some logging for when debugging is not enabled.

Signed-off-by: Simon Glass sjg@chromium.org ---

(no changes since v3)

Changes in v3: - Add new patch for elf to return number of written symbols

tools/binman/elf.py | 13 +++++++++++-- tools/binman/elf_test.py | 8 +++++--- 2 files changed, 16 insertions(+), 5 deletions(-)

diff --git a/tools/binman/elf.py b/tools/binman/elf.py index 5816284c32a2..4219001feac3 100644 --- a/tools/binman/elf.py +++ b/tools/binman/elf.py @@ -248,6 +248,9 @@ def LookupAndWriteSymbols(elf_fname, entry, section, is_elf=False, entry: Entry to process section: Section which can be used to lookup symbol values base_sym: Base symbol marking the start of the image + + Returns: + int: Number of symbols written """ if not base_sym: base_sym = '__image_copy_start' @@ -269,12 +272,13 @@ def LookupAndWriteSymbols(elf_fname, entry, section, is_elf=False,

if not syms: tout.debug('LookupAndWriteSymbols: no syms') - return + return 0 base = syms.get(base_sym) if not base and not is_elf: tout.debug('LookupAndWriteSymbols: no base') - return + return 0 base_addr = 0 if is_elf else base.address + count = 0 for name, sym in syms.items(): if name.startswith('_binman'): msg = ("Section '%s': Symbol '%s'\n in entry '%s'" % @@ -307,6 +311,11 @@ def LookupAndWriteSymbols(elf_fname, entry, section, is_elf=False, (msg, name, offset, value, len(value_bytes))) entry.data = (entry.data[:offset] + value_bytes + entry.data[offset + sym.size:]) + count += 1 + if count: + tout.detail( + f"Section '{section.GetPath()}': entry '{entry.GetPath()}' : {count} symbols") + return count

def GetSymbolValue(sym, data, msg): """Get the value of a symbol diff --git a/tools/binman/elf_test.py b/tools/binman/elf_test.py index c98083961b53..2fb3f6f28ff1 100644 --- a/tools/binman/elf_test.py +++ b/tools/binman/elf_test.py @@ -141,7 +141,8 @@ class TestElf(unittest.TestCase): entry = FakeEntry(10) section = FakeSection() elf_fname = self.ElfTestFile('u_boot_binman_syms_bad') - elf.LookupAndWriteSymbols(elf_fname, entry, section) + count = elf.LookupAndWriteSymbols(elf_fname, entry, section) + self.assertEqual(0, count)

def testBadSymbolSize(self): """Test that an attempt to use an 8-bit symbol are detected @@ -162,7 +163,7 @@ class TestElf(unittest.TestCase): def testNoValue(self): """Test the case where we have no value for the symbol

- This should produce -1 values for all thress symbols, taking up the + This should produce -1 values for all three symbols, taking up the first 16 bytes of the image. """ if not elf.ELF_TOOLS: @@ -170,7 +171,8 @@ class TestElf(unittest.TestCase): entry = FakeEntry(28) section = FakeSection(sym_value=None) elf_fname = self.ElfTestFile('u_boot_binman_syms') - elf.LookupAndWriteSymbols(elf_fname, entry, section) + count = elf.LookupAndWriteSymbols(elf_fname, entry, section) + self.assertEqual(5, count) expected = (struct.pack('<L', elf.BINMAN_SYM_MAGIC_VALUE) + tools.get_bytes(255, 20) + tools.get_bytes(ord('a'), 4))

From: Marek Vasut marex@denx.de

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

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

(no changes since v3)

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 | 71 ++++++++++++++--------- tools/binman/ftest.py | 69 +++++++++++++++++++++- tools/binman/test/283_mkimage_special.dts | 24 ++++++++ 4 files changed, 135 insertions(+), 35 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..493761da59f5 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,17 +199,15 @@ 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 @@ -209,7 +219,7 @@ class Entry_mkimage(Entry): allow_missing: True if allowed, False if not allowed """ self.allow_missing = allow_missing - for entry in self._mkimage_entries.values(): + for entry in self._entries.values(): entry.SetAllowMissing(allow_missing) if self._imagename: self._imagename.SetAllowMissing(allow_missing) @@ -220,7 +230,7 @@ class Entry_mkimage(Entry): Args: allow_fake: True if allowed, False if not allowed """ - for entry in self._mkimage_entries.values(): + for entry in self._entries.values(): entry.SetAllowFakeBlob(allow_fake) if self._imagename: self._imagename.SetAllowFakeBlob(allow_fake) @@ -234,7 +244,7 @@ class Entry_mkimage(Entry): Args: missing_list: List of Entry objects to be added to """ - for entry in self._mkimage_entries.values(): + for entry in self._entries.values(): entry.CheckMissing(missing_list) if self._imagename: self._imagename.CheckMissing(missing_list) @@ -247,7 +257,7 @@ class Entry_mkimage(Entry): Args: faked_blobs_list: List of Entry objects to be added to """ - for entry in self._mkimage_entries.values(): + for entry in self._entries.values(): entry.CheckFakedBlobs(faked_blobs_list) if self._imagename: self._imagename.CheckFakedBlobs(faked_blobs_list) @@ -255,3 +265,6 @@ class Entry_mkimage(Entry): 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..60e69443c400 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__": +if __name__ == "_s_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"; + }; + }; + }; +};

This is not needed since the linker script sets it up. Drop the variable to avoid confusion.

Fix the prototype for main() while we are here.

Signed-off-by: Simon Glass sjg@chromium.org ---

(no changes since v1)

tools/binman/test/bss_data.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/tools/binman/test/bss_data.c b/tools/binman/test/bss_data.c index 4f9b64cef9ef..7047a3bb014d 100644 --- a/tools/binman/test/bss_data.c +++ b/tools/binman/test/bss_data.c @@ -7,9 +7,8 @@ */

int bss_data[10]; -int __bss_size = sizeof(bss_data);

-int main() +int main(void) { bss_data[2] = 2;

This permits implementation of a simple templating system, where a node can be reused as a base for others.

For now this adds new subnodes after any existing ones.

Signed-off-by: Simon Glass sjg@chromium.org ---

Changes in v4: - Avoid copying phandle nodes

Changes in v3: - Add a new devicetree file especially for node copying - Correct logic for merging nodes in order - Tidy up some comments

tools/dtoc/fdt.py | 119 ++++++++++++++++++++++++++++- tools/dtoc/test/dtoc_test_copy.dts | 76 ++++++++++++++++++ tools/dtoc/test_fdt.py | 74 ++++++++++++++++++ 3 files changed, 266 insertions(+), 3 deletions(-) create mode 100644 tools/dtoc/test/dtoc_test_copy.dts

diff --git a/tools/dtoc/fdt.py b/tools/dtoc/fdt.py index a8e05349a720..f4d84083e491 100644 --- a/tools/dtoc/fdt.py +++ b/tools/dtoc/fdt.py @@ -13,6 +13,7 @@ from dtoc import fdt_util import libfdt from libfdt import QUIET_NOTFOUND from u_boot_pylib import tools +from u_boot_pylib import tout

# This deals with a device tree, presenting it as an assortment of Node and # Prop objects, representing nodes and properties, respectively. This file @@ -264,6 +265,13 @@ class Prop: fdt_obj.setprop(node.Offset(), self.name, self.bytes) self.dirty = False

+ def purge(self): + """Set a property offset to None + + The property remains in the tree structure and will be recreated when + the FDT is synced + """ + self._offset = None

class Node: """A device tree node @@ -534,8 +542,8 @@ class Node: """ return self.AddData(prop_name, struct.pack('>I', val))

- def AddSubnode(self, name): - """Add a new subnode to the node + def Subnode(self, name): + """Create new subnode for the node

Args: name: name of node to add @@ -544,10 +552,72 @@ class Node: New subnode that was created """ path = self.path + '/' + name - subnode = Node(self._fdt, self, None, name, path) + return Node(self._fdt, self, None, name, path) + + def AddSubnode(self, name): + """Add a new subnode to the node, after all other subnodes + + Args: + name: name of node to add + + Returns: + New subnode that was created + """ + subnode = self.Subnode(name) self.subnodes.append(subnode) return subnode

+ def insert_subnode(self, name): + """Add a new subnode to the node, before all other subnodes + + This deletes other subnodes and sets their offset to None, so that they + will be recreated after this one. + + Args: + name: name of node to add + + Returns: + New subnode that was created + """ + # Deleting a node invalidates the offsets of all following nodes, so + # process in reverse order so that the offset of each node remains valid + # until deletion. + for subnode in reversed(self.subnodes): + subnode.purge(True) + subnode = self.Subnode(name) + self.subnodes.insert(0, subnode) + return subnode + + def purge(self, delete_it=False): + """Purge this node, setting offset to None and deleting from FDT""" + if self._offset is not None: + if delete_it: + CheckErr(self._fdt._fdt_obj.del_node(self.Offset()), + "Node '%s': delete" % self.path) + self._offset = None + self._fdt.Invalidate() + + for prop in self.props.values(): + prop.purge() + + for subnode in self.subnodes: + subnode.purge(False) + + def move_to_first(self): + """Move the current node to first in its parent's node list""" + parent = self.parent + if parent.subnodes and parent.subnodes[0] == self: + return + for subnode in reversed(parent.subnodes): + subnode.purge(True) + + new_subnodes = [self] + for subnode in parent.subnodes: + #subnode.purge(False) + if subnode != self: + new_subnodes.append(subnode) + parent.subnodes = new_subnodes + def Delete(self): """Delete a node

@@ -635,6 +705,49 @@ class Node: prop.Sync(auto_resize) return added

+ def merge_props(self, src): + """Copy missing properties (except 'phandle') from another node + + Args: + src (Node): Node containing properties to copy + + Adds properties which are present in src but not in this node. Any + 'phandle' property is not copied since this might result in two nodes + with the same phandle, thus making phandle references ambiguous. + """ + for name, src_prop in src.props.items(): + if name != 'phandle' and name not in self.props: + self.props[name] = Prop(self, None, name, src_prop.bytes) + + def copy_node(self, src): + """Copy a node and all its subnodes into this node + + Args: + src (Node): Node to copy + + Returns: + Node: Resulting destination node + + This works recursively. + + The new node is put before all other nodes. If the node already + exists, just its subnodes and properties are copied, placing them before + any existing subnodes. Properties which exist in the destination node + already are not copied. + """ + dst = self.FindNode(src.name) + if dst: + dst.move_to_first() + else: + dst = self.insert_subnode(src.name) + dst.merge_props(src) + + # Process in reverse order so that they appear correctly in the result, + # since copy_node() puts the node first in the list + for node in reversed(src.subnodes): + dst.copy_node(node) + return dst +

class Fdt: """Provides simple access to a flat device tree blob using libfdts. diff --git a/tools/dtoc/test/dtoc_test_copy.dts b/tools/dtoc/test/dtoc_test_copy.dts new file mode 100644 index 000000000000..85e2c342a7ed --- /dev/null +++ b/tools/dtoc/test/dtoc_test_copy.dts @@ -0,0 +1,76 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Test device tree file for dtoc + * + * Copyright 2017 Google, Inc + */ + +/dts-v1/; + +/ { + #address-cells = <1>; + #size-cells = <1>; + reference = <&over>; /* nake sure that the 'over' phandle exists */ + + dest { + bootph-all; + compatible = "sandbox,spl-test"; + stringarray = "one"; + longbytearray = [09 0a 0b 0c 0d 0e 0f 10]; + maybe-empty-int = <1>; + + first@0 { + a-prop = <456>; + b-prop = <1>; + }; + + existing { + }; + + base { + second { + second3 { + }; + + second2 { + new-prop; + }; + + second1 { + new-prop; + }; + + second4 { + }; + }; + }; + }; + + base { + compatible = "sandbox,i2c"; + bootph-all; + #address-cells = <1>; + #size-cells = <0>; + over: over { + compatible = "sandbox,pmic"; + bootph-all; + reg = <9>; + low-power; + }; + + first@0 { + reg = <0>; + a-prop = <123>; + }; + + second: second { + second1 { + some-prop; + }; + + second2 { + some-prop; + }; + }; + }; +}; diff --git a/tools/dtoc/test_fdt.py b/tools/dtoc/test_fdt.py index 4fe8d12c403a..ebc5297b5067 100755 --- a/tools/dtoc/test_fdt.py +++ b/tools/dtoc/test_fdt.py @@ -306,6 +306,80 @@ class TestNode(unittest.TestCase): self.assertIn("Internal error, node '/spl-test' name mismatch 'i2c@0'", str(exc.exception))

+ def test_copy_node(self): + """Test copy_node() function""" + def do_copy_checks(dtb, dst, expect_none): + self.assertEqual( + ['/dest/base', '/dest/first@0', '/dest/existing'], + [n.path for n in dst.subnodes]) + + chk = dtb.GetNode('/dest/base') + self.assertTrue(chk) + self.assertEqual( + {'compatible', 'bootph-all', '#address-cells', '#size-cells'}, + chk.props.keys()) + + # Check the first property + prop = chk.props['bootph-all'] + self.assertEqual('bootph-all', prop.name) + self.assertEqual(True, prop.value) + self.assertEqual(chk.path, prop._node.path) + + # Check the second property + prop2 = chk.props['compatible'] + self.assertEqual('compatible', prop2.name) + self.assertEqual('sandbox,i2c', prop2.value) + self.assertEqual(chk.path, prop2._node.path) + + base = chk.FindNode('base') + self.assertTrue(chk) + + first = dtb.GetNode('/dest/base/first@0') + self.assertTrue(first) + over = dtb.GetNode('/dest/base/over') + self.assertTrue(over) + + # Make sure that the phandle for 'over' is not copied + self.assertNotIn('phandle', over.props.keys()) + + second = dtb.GetNode('/dest/base/second') + self.assertTrue(second) + self.assertEqual([over.name, first.name, second.name], + [n.name for n in chk.subnodes]) + self.assertEqual(chk, over.parent) + self.assertEqual( + {'bootph-all', 'compatible', 'reg', 'low-power'}, + over.props.keys()) + + if expect_none: + self.assertIsNone(prop._offset) + self.assertIsNone(prop2._offset) + self.assertIsNone(over._offset) + else: + self.assertTrue(prop._offset) + self.assertTrue(prop2._offset) + self.assertTrue(over._offset) + + # Now check ordering of the subnodes + self.assertEqual( + ['second1', 'second2', 'second3', 'second4'], + [n.name for n in second.subnodes]) + + dtb = fdt.FdtScan(find_dtb_file('dtoc_test_copy.dts')) + tmpl = dtb.GetNode('/base') + dst = dtb.GetNode('/dest') + dst.copy_node(tmpl) + + do_copy_checks(dtb, dst, expect_none=True) + + dtb.Sync(auto_resize=True) + + # Now check that the FDT looks correct + new_dtb = fdt.Fdt.FromData(dtb.GetContents()) + new_dtb.Scan() + dst = new_dtb.GetNode('/dest') + do_copy_checks(new_dtb, dst, expect_none=False) +

class TestProp(unittest.TestCase): """Test operation of the Prop class"""

Collections can used to collect the contents of other entries into a single entry, but they result in a single entry, with the original entries 'left behind' in their old place.

It is useful to be able to specific a set of entries ones and have it used in multiple images, or parts of an image.

Implement this mechanism.

Signed-off-by: Simon Glass sjg@chromium.org ---

Changes in v4: - Copy over properties from the top-level template node

Changes in v2: - Correct ordering of template nodes - Fix 'preseverd' and 'inserter' typos

tools/binman/binman.rst | 87 ++++++++++++++++++++++++++++++ tools/binman/control.py | 26 +++++++++ tools/binman/etype/section.py | 3 +- tools/binman/ftest.py | 8 +++ tools/binman/test/286_template.dts | 42 +++++++++++++++ 5 files changed, 165 insertions(+), 1 deletion(-) create mode 100644 tools/binman/test/286_template.dts

diff --git a/tools/binman/binman.rst b/tools/binman/binman.rst index a4b31fe5397b..8f57b6cfc76f 100644 --- a/tools/binman/binman.rst +++ b/tools/binman/binman.rst @@ -727,6 +727,13 @@ optional: Note that missing, optional blobs do not produce a non-zero exit code from binman, although it does show a warning about the missing external blob.

+insert-template: + This is not strictly speaking an entry property, since it is processed early + in Binman before the entries are read. It is a list of phandles of nodes to + include in the current (target) node. For each node, its subnodes and their + properties are brought into the target node. See Templates_ below for + more information. + The attributes supported for images and sections are described below. Several are similar to those for entries.

@@ -1172,6 +1179,86 @@ If you are having trouble figuring out what is going on, you can use arch/arm/dts/u-boot.dtsi ... found: "arch/arm/dts/juno-r2-u-boot.dtsi"

+Templates +========= + +Sometimes multiple images need to be created which have all have a common +part. For example, a board may generate SPI and eMMC images which both include +a FIT. Since the FIT includes many entries, it is tedious to repeat them twice +in the image description. + +Templates provide a simple way to handle this:: + + binman { + multiple-images; + common_part: template-1 { + some-property; + fit { + ... lots of entries in here + }; + + text { + text = "base image"; + }; + }; + + spi-image { + filename = "image-spi.bin"; + insert-template = <&fit>; + + /* things specific to SPI follow */ + footer { + ]; + + text { + text = "SPI image"; + }; + }; + + mmc-image { + filename = "image-mmc.bin"; + insert-template = <&fit>; + + /* things specific to MMC follow */ + footer { + ]; + + text { + text = "MMC image"; + }; + }; + }; + +The template node name must start with 'template', so it is not considered to be +an image itself. + +The mechanism is very simple. For each phandle in the 'insert-templates' +property, the source node is looked up. Then the subnodes of that source node +are copied into the target node, i.e. the one containing the `insert-template` +property. + +If the target node has a node with the same name as a template, its properties +override corresponding properties in the template. This allows the template to +be uses as a base, with the node providing updates to the properties as needed. +The overriding happens recursively. + +Template nodes appear first in each node that they are inserted into and +ordering of template nodes is preserved. Other nodes come afterwards. If a +template node also appears in the target node, then the template node sets the +order. Thus the template can be used to set the ordering, even if the target +node provides all the properties. In the above example, `fit` and `text` appear +first in the `spi-image` and `mmc-image` images, followed by `footer`. + +Where there are multiple template nodes, they are inserted in that order. so +the first template node appears first, then the second. + +Properties in the template node are inserted into the destination node if they +do not exist there. In the example above, `some-property` is added to each of +`spi-image` and `mmc-image`. + +Note that template nodes are not removed from the binman description at present. + + Updating an ELF file ====================

diff --git a/tools/binman/control.py b/tools/binman/control.py index 7e2dd3541b96..e9c4a65a75a1 100644 --- a/tools/binman/control.py +++ b/tools/binman/control.py @@ -22,6 +22,7 @@ from binman import bintool from binman import cbfs_util from binman import elf from binman import entry +from dtoc import fdt_util from u_boot_pylib import command from u_boot_pylib import tools from u_boot_pylib import tout @@ -478,6 +479,29 @@ def SignEntries(image_fname, input_fname, privatekey_fname, algo, entry_paths,

AfterReplace(image, allow_resize=True, write_map=write_map)

+def _ProcessTemplates(parent): + """Handle any templates in the binman description + + Args: + parent: Binman node to process (typically /binman) + + Search though each target node looking for those with an 'insert-template' + property. Use that as a list of references to template nodes to use to + adjust the target node. + + Processing involves copying each subnode of the template node into the + target node. + + For now this is not done recursively, so templates must be at the top level + of the binman image. + + See 'Templates' in the Binman documnentation for details. + """ + for node in parent.subnodes: + tmpl = fdt_util.GetPhandleList(node, 'insert-template') + if tmpl: + node.copy_subnodes_from_phandles(tmpl) + def PrepareImagesAndDtbs(dtb_fname, select_images, update_fdt, use_expanded): """Prepare the images to be processed and select the device tree

@@ -520,6 +544,8 @@ def PrepareImagesAndDtbs(dtb_fname, select_images, update_fdt, use_expanded): raise ValueError("Device tree '%s' does not have a 'binman' " "node" % dtb_fname)

+ _ProcessTemplates(node) + images = _ReadImageDesc(node, use_expanded)

if select_images: diff --git a/tools/binman/etype/section.py b/tools/binman/etype/section.py index d9b9e428024a..7c4d312c16c0 100644 --- a/tools/binman/etype/section.py +++ b/tools/binman/etype/section.py @@ -179,7 +179,8 @@ class Entry_section(Entry): Returns: bool: True if the node is a special one, else False """ - return node.name.startswith('hash') or node.name.startswith('signature') + start_list = ('hash', 'signature', 'template') + return any(node.name.startswith(name) for name in start_list)

def ReadNode(self): """Read properties from the section node""" diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index dc9a95d341e5..fc5d8a839e6b 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -6785,6 +6785,14 @@ fdt fdtmap Extract the devicetree blob from the fdtmap data = self._DoReadFileDtb('285_spl_expand.dts', use_expanded=True, entry_args=entry_args)[0]

+ def testTemplate(self): + """Test using a template""" + TestFunctional._MakeInputFile('vga2.bin', b'#' + VGA_DATA) + data = self._DoReadFile('286_template.dts') + first = U_BOOT_DATA + VGA_DATA + U_BOOT_DTB_DATA + second = U_BOOT_DATA + b'#' + VGA_DATA + U_BOOT_DTB_DATA + self.assertEqual(U_BOOT_IMG_DATA + first + second, data) +

if __name__ == "__main__": unittest.main() diff --git a/tools/binman/test/286_template.dts b/tools/binman/test/286_template.dts new file mode 100644 index 000000000000..6980dbfafcc6 --- /dev/null +++ b/tools/binman/test/286_template.dts @@ -0,0 +1,42 @@ +// SPDX-License-Identifier: GPL-2.0+ + +/dts-v1/; + +/ { + #address-cells = <1>; + #size-cells = <1>; + + binman { + u-boot-img { + }; + + common_part: template { + u-boot { + }; + + intel-vga { + filename = "vga.bin"; + }; + }; + + first { + type = "section"; + insert-template = <&common_part>; + + u-boot-dtb { + }; + }; + + second { + type = "section"; + insert-template = <&common_part>; + + u-boot-dtb { + }; + + intel-vga { + filename = "vga2.bin"; + }; + }; + }; +};

Add this as a separate test case.

Signed-off-by: Simon Glass sjg@chromium.org ---

(no changes since v3)

Changes in v3: - Add new test case for templating in a FIT

tools/binman/ftest.py | 7 +++++ tools/binman/test/288_template_fit.dts | 37 ++++++++++++++++++++++++++ 2 files changed, 44 insertions(+) create mode 100644 tools/binman/test/288_template_fit.dts

diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index dd6075b871d4..dfca6316624c 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -6805,6 +6805,13 @@ fdt fdtmap Extract the devicetree blob from the fdtmap data = tools.read_file(image_fname) self.assertEqual(b'blob@@@@other', data)

+ def testTemplateFit(self): + """Test using a template in a FIT""" + fit_data = self._DoReadFile('288_template_fit.dts') + fname = os.path.join(self._indir, 'fit_data.fit') + tools.write_file(fname, fit_data) + out = tools.run('dumpimage', '-l', fname) +

if __name__ == "__main__": unittest.main() diff --git a/tools/binman/test/288_template_fit.dts b/tools/binman/test/288_template_fit.dts new file mode 100644 index 000000000000..d84dca4ea41e --- /dev/null +++ b/tools/binman/test/288_template_fit.dts @@ -0,0 +1,37 @@ +// SPDX-License-Identifier: GPL-2.0+ + +/dts-v1/; + +/ { + binman: binman { + multiple-images; + + my_template: template { + fit@0 { + images { + kernel-1 { + }; + kernel-2 { + }; + }; + }; + }; + + image { + filename = "image.bin"; + insert-template = <&my_template>; + + fit@0 { + description = "desc"; + configurations { + }; + images { + kernel-3 { + }; + kernel-4 { + }; + }; + }; + }; + }; +};

Add support for writing symbols and determining the assumed position of binaries inside a mkimage image. This is useful as an example for other entry types which might want to do the same thing.

Signed-off-by: Simon Glass sjg@chromium.org ---

(no changes since v3)

Changes in v3: - Add new patch to support writing symbols inside a mkimage image

tools/binman/entry.py | 2 - tools/binman/etype/mkimage.py | 36 +++++++++++++++ tools/binman/ftest.py | 64 +++++++++++++++++++++++++++ tools/binman/test/290_mkimage_sym.dts | 27 +++++++++++ 4 files changed, 127 insertions(+), 2 deletions(-) create mode 100644 tools/binman/test/290_mkimage_sym.dts

diff --git a/tools/binman/entry.py b/tools/binman/entry.py index 0d4cb94f7002..42e0b7b91450 100644 --- a/tools/binman/entry.py +++ b/tools/binman/entry.py @@ -1314,8 +1314,6 @@ features to produce new behaviours. """ data = b'' for entry in entries: - # 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 493761da59f5..9d60c85722b3 100644 --- a/tools/binman/etype/mkimage.py +++ b/tools/binman/etype/mkimage.py @@ -268,3 +268,39 @@ class Entry_mkimage(Entry_section):

def CheckEntries(self): pass + + def ProcessContents(self): + # The blob may have changed due to WriteSymbols() + ok = super().ProcessContents() + data = self.BuildSectionData(True) + ok2 = self.ProcessContentsUpdate(data) + return ok and ok2 + + def SetImagePos(self, image_pos): + """Set the position in the image + + This sets each subentry's offsets, sizes and positions-in-image + according to where they ended up in the packed mkimage file. + + NOTE: This assumes a legacy mkimage and assumes that the images are + written to the output in order. SoC-specific mkimage handling may not + conform to this, in which case these values may be wrong. + + Args: + image_pos (int): Position of this entry in the image + """ + # The mkimage header consists of 0x40 bytes, following by a table of + # offsets for each file + upto = 0x40 + + # Skip the 0-terminated list of offsets (assume a single image) + upto += 4 + 4 + for entry in self.GetEntries().values(): + entry.SetOffsetSize(upto, None) + + # Give up if any entries lack a size + if entry.size is None: + return + upto += entry.size + + super().SetImagePos(image_pos) diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index e96223cbd892..e53181afb78a 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -6820,6 +6820,70 @@ fdt fdtmap Extract the devicetree blob from the fdtmap second = U_BOOT_DATA + b'#' + VGA_DATA + U_BOOT_DTB_DATA self.assertEqual(U_BOOT_IMG_DATA + first + second + first, data)

+ def testMkimageSymbols(self): + """Test using mkimage to build an image with symbols in it""" + self._SetupSplElf('u_boot_binman_syms') + data = self._DoReadFile('290_mkimage_sym.dts') + + image = control.images['image'] + entries = image.GetEntries() + self.assertIn('u-boot', entries) + u_boot = entries['u-boot'] + + mkim = entries['mkimage'] + mkim_entries = mkim.GetEntries() + self.assertIn('u-boot-spl', mkim_entries) + spl = mkim_entries['u-boot-spl'] + self.assertIn('u-boot-spl2', mkim_entries) + spl2 = mkim_entries['u-boot-spl2'] + + # skip the mkimage header and the area sizes + mk_data = data[mkim.offset + 0x40:] + size, term = struct.unpack('>LL', mk_data[:8]) + + # There should be only one image, so check that the zero terminator is + # present + self.assertEqual(0, term) + + content = mk_data[8:8 + size] + + # The image should contain the symbols from u_boot_binman_syms.c + # Note that image_pos is adjusted by the base address of the image, + # which is 0x10 in our test image + spl_data = content[:0x18] + content = content[0x1b:] + + # After the header is a table of offsets for each image. There should + # only be one image, then a 0 terminator, so figure out the real start + # of the image data + base = 0x40 + 8 + + # Check symbols in both u-boot-spl and u-boot-spl2 + for i in range(2): + vals = struct.unpack('<LLQLL', spl_data) + + # The image should contain the symbols from u_boot_binman_syms.c + # Note that image_pos is adjusted by the base address of the image, + # which is 0x10 in our 'u_boot_binman_syms' test image + self.assertEqual(elf.BINMAN_SYM_MAGIC_VALUE, vals[0]) + self.assertEqual(base, vals[1]) + self.assertEqual(spl2.offset, vals[2]) + # figure out the internal positions of its components + self.assertEqual(0x10 + u_boot.image_pos, vals[3]) + + # Check that spl and spl2 are actually at the indicated positions + self.assertEqual( + elf.BINMAN_SYM_MAGIC_VALUE, + struct.unpack('<I', data[spl.image_pos:spl.image_pos + 4])[0]) + self.assertEqual( + elf.BINMAN_SYM_MAGIC_VALUE, + struct.unpack('<I', data[spl2.image_pos:spl2.image_pos + 4])[0]) + + self.assertEqual(len(U_BOOT_DATA), vals[4]) + + # Move to next + spl_data = content[:0x18] +

if __name__ == "__main__": unittest.main() diff --git a/tools/binman/test/290_mkimage_sym.dts b/tools/binman/test/290_mkimage_sym.dts new file mode 100644 index 000000000000..2dfd286ad44f --- /dev/null +++ b/tools/binman/test/290_mkimage_sym.dts @@ -0,0 +1,27 @@ +// SPDX-License-Identifier: GPL-2.0+ + +/dts-v1/; + +/ { + #address-cells = <1>; + #size-cells = <1>; + + binman { + u-boot-dtb { + }; + + mkimage { + args = "-n test -T script"; + + u-boot-spl { + }; + + u-boot-spl2 { + type = "u-boot-spl"; + }; + }; + + u-boot { + }; + }; +};