Re: [PATCH v2 6/6] RFC: binman: Improve allow missing for mkimage entry

Hi Jonas,

On Tue, 14 Feb 2023 at 03:34, Jonas Karlman jonas@kwiboo.se wrote:

Implement CheckMissing and CheckOptional methods that is adapted to Entry_mkimage in order to improve support for allow missing flag.

Use collect_contents_to_file in multiple-data-files handling to improve support for allow missing and fake blobs handling.

Signed-off-by: Jonas Karlman jonas@kwiboo.se

Building for RK3568 without ROCKCHIP_TPL will result in the following error message, regardless if BINMAN_ALLOW_MISSING is used or not.

binman: Filename 'rockchip-tpl' not found in input path (...)

With this patch and using BINMAN_ALLOW_MISSING=1 a new external blob with no content is created and then passed to mkimage, resulting in an error from the mkimage command.

binman: Error 255 running 'mkimage -d ./mkimage-0.simple-bin.mkimage:./mkimage-1.simple-bin.mkimage -n rk3568 -T rksd ./idbloader.img': mkimage: Can't read ./mkimage-0.simple-bin.mkimage: Invalid argument

If the --fake-ext-blobs argument is also used I get the message I was expecting to see from the beginning.

Image 'main-section' is missing external blobs and is non-functional: rockchip-tpl

/binman/simple-bin/mkimage/rockchip-tpl: An external TPL is required to initialize DRAM. Get the external TPL binary and build with ROCKCHIP_TPL=/path/to/ddr.bin. One possible source for the external TPL binary is https://github.com/rockchip-linux/rkbin. Image 'main-section' has faked external blobs and is non-functional: rockchip-tpl

Some images are invalid

Not sure how this should work, but I was expecting to see the message about the missing rockchip-tpl from the beginning instead of the generic "not found in input path" message. At leas with BINMAN_ALLOW_MISSING=1.

tools/binman/etype/mkimage.py | 37 +++++++++++++++++++++++++++++++---- 1 file changed, 33 insertions(+), 4 deletions(-)

diff --git a/tools/binman/etype/mkimage.py b/tools/binman/etype/mkimage.py index cb264c3cad0b..44510a8c40ba 100644 --- a/tools/binman/etype/mkimage.py +++ b/tools/binman/etype/mkimage.py @@ -153,10 +153,12 @@ class Entry_mkimage(Entry): if self._multiple_data_files: fnames = [] uniq = self.GetUniqueName()

•        for entry in self._mkimage_entries.values():
  

•            if not entry.ObtainContents(fake_size=fake_size):
  

•        for idx, entry in enumerate(self._mkimage_entries.values()):
  

•            entry_data, entry_fname, _ = self.collect_contents_to_file(
  

•                [entry], 'mkimage-%s' % idx, fake_size)
  

•            if entry_data is None:
  

This is OK, I suppose, but I'm not quite sure what actually changes here, other than writing each entry to a file?

Also, if you do this, please add / extend a test that checks that the output files are written, since there is otherwise no coverage here.

What test uses these optional mkimage pieces?

                 return False

•            fnames.append(tools.get_input_filename(entry.GetDefaultFilename()))
  

•            fnames.append(entry_fname)
         input_fname = ":".join(fnames)
     else:
         data, input_fname, uniq = self.collect_contents_to_file(
  

@@ -165,7 +167,7 @@ class Entry_mkimage(Entry): return False if self._imagename: image_data, imagename_fname, _ = self.collect_contents_to_file(

•            [self._imagename], 'mkimage-n', 1024)
  

•            [self._imagename], 'mkimage-n', fake_size)
         if image_data is None:
             return False
     outfile = self._filename if self._filename else 'mkimage-out.%s' % uniq
  

@@ -216,6 +218,20 @@ class Entry_mkimage(Entry): 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

@@ -229,6 +245,19 @@ class Entry_mkimage(Entry): if self._imagename: self._imagename.CheckFakedBlobs(faked_blobs_list)

• def CheckOptional(self, optional_list):

•    """Check the section for missing but optional external blobs
  

•    If there are missing (optional) blobs, the entries are added to the list
  

•    Args:
  

•        optional_list (list): List of Entry objects to be added to
  

•    """
  

•    for entry in self._mkimage_entries.values():
  

•        entry.CheckOptional(optional_list)
  

•    if self._imagename:
  

•        self._imagename.CheckOptional(optional_list)
  

• def AddBintools(self, btools): super().AddBintools(btools) self.mkimage = self.AddBintool(btools, 'mkimage')

-- 2.39.1

Regards, Simon