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

Jonas Karlman jonas at kwiboo.se
Thu Jun 29 00:35:50 CEST 2023


Hi Simon,
On 2023-06-28 13:41, Simon Glass wrote:
> From: Marek Vasut <marex at denx.de>
> 
> This is needed to handle mkimage with inner section located itself in a
> section.
> 
> Signed-off-by: Marek Vasut <marex at denx.de>
> Use BuildSectionData() instead of ObtainContents(), add tests and a few
> other minor fixes:
> Signed-off-by: Simon Glass <sjg at 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";
> +			};
> +		};
> +	};
> +};



More information about the U-Boot mailing list