Re: [PATCH 06/12] binman: Convert mkimage to Entry_section

Hi Simon, On 2023-06-28 13:41, Simon Glass wrote:

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

---

tools/binman/entry.py | 6 +- tools/binman/etype/mkimage.py | 76 ++++++++++++++--------- tools/binman/ftest.py | 45 +++++++++++++- tools/binman/test/283_mkimage_special.dts | 24 +++++++ 4 files changed, 117 insertions(+), 34 deletions(-) create mode 100644 tools/binman/test/283_mkimage_special.dts

diff --git a/tools/binman/entry.py b/tools/binman/entry.py index 328b5bc568a9..8f06fea51ad4 100644 --- a/tools/binman/entry.py +++ b/tools/binman/entry.py @@ -1311,10 +1311,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..8311fed59762 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,60 @@ 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():
  

•            entry.ObtainContents(fake_size=fake_size)
  

•            # If this is a section, put the contents in a temporary file.
  

•            # Otherwise, assume it is a blob and use the pathname
  

•            if isinstance(entry, Entry_section):
  

•                ename = f'mkimage-in-{uniq}-{entry.name}'
  

•                fname = tools.get_output_filename(ename)
  

•                tools.write_file(fname, entry.data)
  

•            elif entry._pathname:
  

•                fname = entry._pathname
  

•            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 +195,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 +204,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 +224,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():
  

With the changes to use GetEntries() in Entry_section for this and following Check and Set functions, I suspect these can be removed and use the base implementation from Entry_section.

GetEntries() returns self._entries + self._imagename

Regards, Jonas

         entry.SetAllowMissing(allow_missing)
     if self._imagename:
         self._imagename.SetAllowMissing(allow_missing)

@@ -220,7 +235,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 +249,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 +262,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 +270,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..6d0ffda2f432 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -3737,6 +3737,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 +3745,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')
  

@@ -5952,6 +5954,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)])
  

@@ -6051,6 +6054,39 @@ 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""" with self.assertRaises(ValueError) as exc:

@@ -6696,6 +6732,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";
  

• 	};
  

• };
  

• };

+};