From 67a050170846b6cb751c7162c3a3bdb898261660 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Sat, 7 Jan 2023 14:07:15 -0700 Subject: [PATCH] binman: Support optional external blobs Some blobs are actually not necessary for the board to work correctly. Add a property to allow this to be indicated. Missing optional blobs do not cause a build failure. Signed-off-by: Simon Glass --- tools/binman/binman.rst | 9 +++++++++ tools/binman/control.py | 11 +++++++++++ tools/binman/entry.py | 25 +++++++++++++++++++++---- tools/binman/etype/blob.py | 2 +- tools/binman/etype/fit.py | 11 ++++++----- tools/binman/etype/section.py | 14 +++++++++++++- tools/binman/ftest.py | 10 ++++++++++ tools/binman/test/266_blob_ext_opt.dts | 21 +++++++++++++++++++++ 8 files changed, 92 insertions(+), 11 deletions(-) create mode 100644 tools/binman/test/266_blob_ext_opt.dts diff --git a/tools/binman/binman.rst b/tools/binman/binman.rst index 5e3961f..bfe300a 100644 --- a/tools/binman/binman.rst +++ b/tools/binman/binman.rst @@ -690,6 +690,15 @@ no-expanded: `no-expanded` property disables this just for a single entry. Put the `no-expanded` boolean property in the node to select this behaviour. +optional: + External blobs are normally required to be present for the image to be + built (but see `External blobs`_). This properly allows an entry to be + optional, so that when it is cannot be found, this problem is ignored and + an empty file is used for this blob. This should be used only when the blob + is entirely optional and is not needed for correct operation of the image. + 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. + The attributes supported for images and sections are described below. Several are similar to those for entries. diff --git a/tools/binman/control.py b/tools/binman/control.py index 0722538..e647400 100644 --- a/tools/binman/control.py +++ b/tools/binman/control.py @@ -594,12 +594,14 @@ def ProcessImage(image, update_fdt, write_map, get_contents=True, image.BuildImage() if write_map: image.WriteMap() + missing_list = [] image.CheckMissing(missing_list) if missing_list: tout.warning("Image '%s' is missing external blobs and is non-functional: %s" % (image.name, ' '.join([e.name for e in missing_list]))) _ShowHelpForMissingBlobs(missing_list) + faked_list = [] image.CheckFakedBlobs(faked_list) if faked_list: @@ -607,6 +609,15 @@ def ProcessImage(image, update_fdt, write_map, get_contents=True, "Image '%s' has faked external blobs and is non-functional: %s" % (image.name, ' '.join([os.path.basename(e.GetDefaultFilename()) for e in faked_list]))) + + optional_list = [] + image.CheckOptional(optional_list) + if optional_list: + tout.warning( + "Image '%s' is missing external blobs but is still functional: %s" % + (image.name, ' '.join([e.name for e in optional_list]))) + _ShowHelpForMissingBlobs(optional_list) + missing_bintool_list = [] image.check_missing_bintools(missing_bintool_list) if missing_bintool_list: diff --git a/tools/binman/entry.py b/tools/binman/entry.py index de51d29..d73f301 100644 --- a/tools/binman/entry.py +++ b/tools/binman/entry.py @@ -73,7 +73,9 @@ class Entry(object): compress: Compression algoithm used (e.g. 'lz4'), 'none' if none orig_offset: Original offset value read from node orig_size: Original size value read from node - missing: True if this entry is missing its contents + missing: True if this entry is missing its contents. Note that if it is + optional, this entry will not appear in the list generated by + entry.CheckMissing() since it is considered OK for it to be missing. allow_missing: Allow children of this entry to be missing (used by subclasses such as Entry_section) allow_fake: Allow creating a dummy fake file if the blob file is not @@ -95,6 +97,7 @@ class Entry(object): the entry itself, allowing it to vanish in certain circumstances. An absent entry is removed during processing so that it does not appear in the map + optional (bool): True if this entry contains an optional external blob """ fake_dir = None @@ -138,6 +141,7 @@ class Entry(object): self.elf_fname = None self.auto_write_symbols = auto_write_symbols self.absent = False + self.optional = False @staticmethod def FindEntryClass(etype, expanded): @@ -289,6 +293,7 @@ class Entry(object): self.offset_unset = fdt_util.GetBool(self._node, 'offset-unset') self.extend_size = fdt_util.GetBool(self._node, 'extend-size') self.missing_msg = fdt_util.GetString(self._node, 'missing-msg') + self.optional = fdt_util.GetBool(self._node, 'optional') # This is only supported by blobs and sections at present self.compress = fdt_util.GetString(self._node, 'compress', 'none') @@ -1039,14 +1044,15 @@ features to produce new behaviours. self.allow_fake = allow_fake def CheckMissing(self, missing_list): - """Check if any entries in this section have missing external blobs + """Check if the entry has missing external blobs - If there are missing blobs, the entries are added to the list + 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 """ - if self.missing: + if self.missing and not self.optional: missing_list.append(self) def check_fake_fname(self, fname, size=0): @@ -1085,6 +1091,17 @@ features to produce new behaviours. # This is meaningless for anything other than blobs pass + def CheckOptional(self, optional_list): + """Check if the entry has 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 + """ + if self.missing and self.optional: + optional_list.append(self) + def GetAllowMissing(self): """Get whether a section allows missing external blobs diff --git a/tools/binman/etype/blob.py b/tools/binman/etype/blob.py index a50a806..70dea71 100644 --- a/tools/binman/etype/blob.py +++ b/tools/binman/etype/blob.py @@ -39,7 +39,7 @@ class Entry_blob(Entry): def ObtainContents(self, fake_size=0): self._filename = self.GetDefaultFilename() self._pathname = tools.get_input_filename(self._filename, - self.external and self.section.GetAllowMissing()) + self.external and (self.optional or self.section.GetAllowMissing())) # Allow the file to be missing if not self._pathname: self._pathname, faked = self.check_fake_fname(self._filename, diff --git a/tools/binman/etype/fit.py b/tools/binman/etype/fit.py index fea3adc..f0e3fd1 100644 --- a/tools/binman/etype/fit.py +++ b/tools/binman/etype/fit.py @@ -392,8 +392,8 @@ class Entry_fit(Entry_section): _add_entries(self._node, 0, self._node) - # Keep a copy of all entries, including generator entries, since these - # removed from self._entries later. + # Keep a copy of all entries, including generator entries, since those + # are removed from self._entries later. self._priv_entries = dict(self._entries) def BuildSectionData(self, required): @@ -602,14 +602,15 @@ class Entry_fit(Entry_section): # Entry_section.ObtainContents() either returns True or # raises an exception. data = None - missing_list = [] + missing_opt_list = [] entry.ObtainContents() entry.Pack(0) - entry.CheckMissing(missing_list) + entry.CheckMissing(missing_opt_list) + entry.CheckOptional(missing_opt_list) # If any pieces are missing, skip this. The missing entries will # show an error - if not missing_list: + if not missing_opt_list: segs = entry.read_elf_segments() if segs: segments, entry_addr = segs diff --git a/tools/binman/etype/section.py b/tools/binman/etype/section.py index 57bfee0..44dafaf 100644 --- a/tools/binman/etype/section.py +++ b/tools/binman/etype/section.py @@ -863,7 +863,8 @@ class Entry_section(Entry): def CheckMissing(self, missing_list): """Check if any entries in this section have missing external blobs - If there are missing blobs, the entries are added to the list + 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 @@ -882,6 +883,17 @@ class Entry_section(Entry): for entry in self._entries.values(): entry.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._entries.values(): + entry.CheckOptional(optional_list) + def check_missing_bintools(self, missing_list): """Check if any entries in this section have missing bintools diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index f893050..330e8e1 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -6178,6 +6178,16 @@ fdt fdtmap Extract the devicetree blob from the fdtmap "Node '/binman/fit/images/@tee-SEQ/tee-os': Invalid OP-TEE file: size mismatch (expected 0x4, have 0xe)", str(exc.exception)) + def testExtblobOptional(self): + """Test an image with an external blob that is optional""" + with test_util.capture_sys_output() as (stdout, stderr): + data = self._DoReadFile('266_blob_ext_opt.dts') + self.assertEqual(REFCODE_DATA, data) + err = stderr.getvalue() + self.assertRegex( + err, + "Image '.*' is missing external blobs but is still functional: missing") + if __name__ == "__main__": unittest.main() diff --git a/tools/binman/test/266_blob_ext_opt.dts b/tools/binman/test/266_blob_ext_opt.dts new file mode 100644 index 0000000..7171531 --- /dev/null +++ b/tools/binman/test/266_blob_ext_opt.dts @@ -0,0 +1,21 @@ +// SPDX-License-Identifier: GPL-2.0+ + +/dts-v1/; + +/ { + #address-cells = <1>; + #size-cells = <1>; + + binman { + ok { + type = "blob-ext"; + filename = "refcode.bin"; + }; + + missing { + type = "blob-ext"; + filename = "missing.bin"; + optional; + }; + }; +}; -- 2.7.4