Merge patch series "Fix handling of optional blobs in binman"

Yannic Moog <y.moog@phytec.de> says:

This series solves a contradiction regarding ext blobs packaged in
binman. When they are marked as optional, by default they are faked, two
messages are emitted. One says the image is not functional the other
says the image is still functional. Both concern the same binman
entry/blob.

Binman is set up to have fake external blobs in case they are missing.
This is regardless on whether they are optional or not.
The implementation does not allow different types of entries to override
the faking decision; at least there wouldn't be much sense in doing so.

Here is an example build output of a phycore-imx8mp:

  BINMAN  .binman_stamp
Image 'image' is missing optional external blobs but is still functional: tee-os

/binman/section/fit/images/tee/tee-os (tee.bin):
   See the documentation for your board. You may need to build Open Portable
   Trusted Execution Environment (OP-TEE) and build with TEE=/path/to/tee.bin

Image 'image' has faked optional external blobs and is still functional: tee.bin

  OFCHK   .config

The output stays to inform/warn the user, but in this case the tee-os
entry will not be present in the final image.

Link: https://lore.kernel.org/r/20250613-binman_faked_optional-v3-0-1e23dd7c41a2@phytec.de
This commit is contained in:
Tom Rini
2025-06-26 09:54:24 -06:00
10 changed files with 102 additions and 42 deletions

View File

@@ -1143,6 +1143,13 @@ Optional entries
Some entries need to exist only if certain conditions are met. For example, an
entry may want to appear in the image only if a file has a particular format.
Also, the ``optional`` property may be used to mark entries as optional::
tee-os {
filename = "tee.bin";
optional;
};
Obviously the entry must exist in the image description for it to be processed
at all, so a way needs to be found to have the entry remove itself.

View File

@@ -645,14 +645,27 @@ def CheckForProblems(image):
_ShowHelpForMissingBlobs(tout.ERROR, missing_list)
faked_list = []
faked_optional_list = []
faked_required_list = []
image.CheckFakedBlobs(faked_list)
if faked_list:
for e in faked_list:
if e.optional:
faked_optional_list.append(e)
else:
faked_required_list.append(e)
if faked_required_list:
tout.warning(
"Image '%s' has faked external blobs and is non-functional: %s\n" %
(image.name, ' '.join([os.path.basename(e.GetDefaultFilename())
for e in faked_list])))
for e in faked_required_list])))
optional_list = []
# For optional blobs, we should inform the user when the blob is not present. This will come as
# a warning since it may not be immediately apparent that something is missing otherwise.
# E.g. user thinks they supplied a blob, but there is no info of the contrary if they made an
# error.
# Faked optional blobs are not relevant for final images (as they are dropped anyway) so we
# will omit the message with default verbosity.
image.CheckOptional(optional_list)
if optional_list:
tout.warning(
@@ -660,6 +673,12 @@ def CheckForProblems(image):
(image.name, ' '.join([e.name for e in optional_list])))
_ShowHelpForMissingBlobs(tout.WARNING, optional_list)
if faked_optional_list:
tout.info(
"Image '%s' has faked optional external blobs but is still functional: %s\n" %
(image.name, ' '.join([os.path.basename(e.GetDefaultFilename())
for e in faked_optional_list])))
missing_bintool_list = []
image.check_missing_bintools(missing_bintool_list)
if missing_bintool_list:
@@ -667,7 +686,7 @@ def CheckForProblems(image):
"Image '%s' has missing bintools and is non-functional: %s\n" %
(image.name, ' '.join([os.path.basename(bintool.name)
for bintool in missing_bintool_list])))
return any([missing_list, faked_list, missing_bintool_list])
return any([missing_list, faked_required_list, missing_bintool_list])
def ProcessImage(image, update_fdt, write_map, get_contents=True,
allow_resize=True, allow_missing=False,
@@ -697,7 +716,6 @@ def ProcessImage(image, update_fdt, write_map, get_contents=True,
image.SetAllowMissing(allow_missing)
image.SetAllowFakeBlob(allow_fake_blobs)
image.GetEntryContents()
image.drop_absent()
image.GetEntryOffsets()
# We need to pack the entries to figure out where everything
@@ -736,12 +754,12 @@ def ProcessImage(image, update_fdt, write_map, get_contents=True,
image.Raise('Entries changed size after packing (tried %s passes)' %
passes)
has_problems = CheckForProblems(image)
image.BuildImage()
if write_map:
image.WriteMap()
has_problems = CheckForProblems(image)
image.WriteAlternates()
return has_problems

View File

@@ -88,6 +88,7 @@ class Entry(object):
updated with a hash of the entry contents
comp_bintool: Bintools used for compress and decompress data
fake_fname: Fake filename, if one was created, else None
faked (bool): True if the entry is absent and faked
required_props (dict of str): Properties which must be present. This can
be added to by subclasses
elf_fname (str): Filename of the ELF file, if this entry holds an ELF
@@ -759,7 +760,7 @@ class Entry(object):
self.image_pos)
# pylint: disable=assignment-from-none
def GetEntries(self):
def GetEntries(self) -> None:
"""Return a list of entries contained by this entry
Returns:
@@ -1120,7 +1121,7 @@ features to produce new behaviours.
if self.missing and not self.optional:
missing_list.append(self)
def check_fake_fname(self, fname, size=0):
def check_fake_fname(self, fname: str, size: int = 0) -> str:
"""If the file is missing and the entry allows fake blobs, fake it
Sets self.faked to True if faked
@@ -1130,9 +1131,7 @@ features to produce new behaviours.
size (int): Size of fake file to create
Returns:
tuple:
fname (str): Filename of faked file
bool: True if the blob was faked, False if not
fname (str): Filename of faked file
"""
if self.allow_fake and not pathlib.Path(fname).is_file():
if not self.fake_fname:
@@ -1142,8 +1141,8 @@ features to produce new behaviours.
tout.info(f"Entry '{self._node.path}': Faked blob '{outfname}'")
self.fake_fname = outfname
self.faked = True
return self.fake_fname, True
return fname, False
return self.fake_fname
return fname
def CheckFakedBlobs(self, faked_blobs_list):
"""Check if any entries in this section have faked external blobs
@@ -1352,6 +1351,10 @@ features to produce new behaviours.
os.mkdir(cls.fake_dir)
tout.notice(f"Fake-blob dir is '{cls.fake_dir}'")
def drop_absent_optional(self) -> None:
"""Entries don't have any entries, do nothing"""
pass
def ensure_props(self):
"""Raise an exception if properties are missing

View File

@@ -42,7 +42,7 @@ class Entry_blob(Entry):
if fdt_util.GetBool(self._node, 'write-symbols'):
self.auto_write_symbols = True
def ObtainContents(self, fake_size=0):
def ObtainContents(self, fake_size: int = 0) -> bool:
self._filename = self.GetDefaultFilename()
self._pathname = tools.get_input_filename(self._filename,
self.external and (self.optional or self.section.GetAllowMissing()))
@@ -50,10 +50,11 @@ class Entry_blob(Entry):
if not self._pathname:
if not fake_size and self.assume_size:
fake_size = self.assume_size
self._pathname, faked = self.check_fake_fname(self._filename,
fake_size)
self._pathname = self.check_fake_fname(self._filename, fake_size)
self.missing = True
if not faked:
if self.optional:
self.mark_absent("missing but optional")
if not self.faked:
content_size = 0
if self.assume_size: # Ensure we get test coverage on next line
content_size = self.assume_size

View File

@@ -33,11 +33,11 @@ class Entry_blob_ext_list(Entry_blob):
self._filenames = fdt_util.GetStringList(self._node, 'filenames')
self._pathnames = []
def ObtainContents(self):
def ObtainContents(self) -> bool:
missing = False
pathnames = []
for fname in self._filenames:
fname, _ = self.check_fake_fname(fname)
fname = self.check_fake_fname(fname)
pathname = tools.get_input_filename(
fname, self.external and self.section.GetAllowMissing())
# Allow the file to be missing

View File

@@ -276,7 +276,8 @@ class Entry_cbfs(Entry):
for entry in self._entries.values():
entry.ListEntries(entries, indent + 1)
def GetEntries(self):
def GetEntries(self) -> dict[str, Entry]:
"""Returns the entries (tree children) of this section"""
return self._entries
def ReadData(self, decomp=True, alt_format=None):

View File

@@ -205,7 +205,7 @@ class Entry_mkimage(Entry_section):
self.record_missing_bintool(self.mkimage)
return data
def GetEntries(self):
def GetEntries(self) -> dict[str, Entry]:
# Make a copy so we don't change the original
entries = OrderedDict(self._entries)
if self._imagename:

View File

@@ -537,7 +537,7 @@ class Entry_section(Entry):
for entry in self._entries.values():
entry.WriteMap(fd, indent + 1)
def GetEntries(self):
def GetEntries(self) -> dict[str, Entry]:
return self._entries
def GetContentsByPhandle(self, phandle, source_entry, required):
@@ -772,9 +772,17 @@ class Entry_section(Entry):
todo)
return True
def drop_absent(self):
"""Drop entries which are absent"""
self._entries = {n: e for n, e in self._entries.items() if not e.absent}
def drop_absent_optional(self) -> None:
"""Drop entries which are absent.
Call for all nodes in the tree. Leaf nodes will do nothing per
definition. Sections however have _entries and should drop all children
which are absent.
"""
self._entries = {n: e for n, e in self._entries.items() if not (e.absent and e.optional)}
# Drop nodes first before traversing children to avoid superfluous calls
# to children of absent nodes.
for e in self.GetEntries().values():
e.drop_absent_optional()
def _SetEntryOffsetSize(self, name, offset, size):
"""Set the offset and size of an entry

View File

@@ -84,6 +84,7 @@ FILES_DATA = (b"sorry I'm late\nOh, don't bother apologising, I'm " +
b"sorry you're alive\n")
COMPRESS_DATA = b'compress xxxxxxxxxxxxxxxxxxxxxx data'
COMPRESS_DATA_BIG = COMPRESS_DATA * 2
MISSING_DATA = b'missing'
REFCODE_DATA = b'refcode'
FSP_M_DATA = b'fsp_m'
FSP_S_DATA = b'fsp_s'
@@ -250,7 +251,7 @@ class TestFunctional(unittest.TestCase):
# ATF and OP_TEE
TestFunctional._MakeInputFile('bl31.elf',
tools.read_file(cls.ElfTestFile('elf_sections')))
TestFunctional._MakeInputFile('tee.elf',
TestFunctional.tee_elf_path = TestFunctional._MakeInputFile('tee.elf',
tools.read_file(cls.ElfTestFile('elf_sections')))
# Newer OP_TEE file in v1 binary format
@@ -514,9 +515,9 @@ class TestFunctional(unittest.TestCase):
return dtb.GetContents()
def _DoReadFileDtb(self, fname, use_real_dtb=False, use_expanded=False,
verbosity=None, map=False, update_dtb=False,
entry_args=None, reset_dtbs=True, extra_indirs=None,
threads=None):
verbosity=None, allow_fake_blobs=True, map=False,
update_dtb=False, entry_args=None, reset_dtbs=True,
extra_indirs=None, threads=None):
"""Run binman and return the resulting image
This runs binman with a given test file and then reads the resulting
@@ -534,6 +535,7 @@ class TestFunctional(unittest.TestCase):
use_expanded: True to use expanded entries where available, e.g.
'u-boot-expanded' instead of 'u-boot'
verbosity: Verbosity level to use (0-3, None=don't set it)
allow_fake_blobs: whether binman should fake missing ext blobs
map: True to output map files for the images
update_dtb: Update the offset and size of each entry in the device
tree before packing it into the image
@@ -571,7 +573,7 @@ class TestFunctional(unittest.TestCase):
retcode = self._DoTestFile(fname, map=map, update_dtb=update_dtb,
entry_args=entry_args, use_real_dtb=use_real_dtb,
use_expanded=use_expanded, verbosity=verbosity,
extra_indirs=extra_indirs,
allow_fake_blobs=allow_fake_blobs, extra_indirs=extra_indirs,
threads=threads)
self.assertEqual(0, retcode)
out_dtb_fname = tools.get_output_filename('u-boot.dtb.out')
@@ -5210,13 +5212,15 @@ fdt fdtmap Extract the devicetree blob from the fdtmap
def testExtblobList(self):
"""Test an image with an external blob list"""
data = self._DoReadFile('215_blob_ext_list.dts')
self.assertEqual(REFCODE_DATA + FSP_M_DATA, data)
data = self._DoReadFileDtb('215_blob_ext_list.dts',
allow_fake_blobs=False)
self.assertEqual(REFCODE_DATA + FSP_M_DATA, data[0])
def testExtblobListMissing(self):
"""Test an image with a missing external blob"""
with self.assertRaises(ValueError) as e:
self._DoReadFile('216_blob_ext_list_missing.dts')
self._DoReadFileDtb('216_blob_ext_list_missing.dts',
allow_fake_blobs=False)
self.assertIn("Filename 'missing-file' not found in input path",
str(e.exception))
@@ -5224,7 +5228,7 @@ fdt fdtmap Extract the devicetree blob from the fdtmap
"""Test an image with an missing external blob that is allowed"""
with terminal.capture() as (stdout, stderr):
self._DoTestFile('216_blob_ext_list_missing.dts',
allow_missing=True)
allow_missing=True, allow_fake_blobs=False)
err = stderr.getvalue()
self.assertRegex(err, "Image 'image'.*missing.*: blob-ext")
@@ -5766,10 +5770,10 @@ fdt fdtmap Extract the devicetree blob from the fdtmap
def testFitSplitElfMissing(self):
"""Test an split-elf FIT with a missing ELF file"""
"""Test an split-elf FIT with a missing ELF file. Don't fake the file."""
if not elf.ELF_TOOLS:
self.skipTest('Python elftools not available')
out, err = self.checkFitSplitElf(allow_missing=True)
out, err = self.checkFitSplitElf(allow_missing=True, allow_fake_blobs=False)
self.assertRegex(
err,
"Image '.*' is missing external blobs and is non-functional: .*")
@@ -6458,16 +6462,18 @@ fdt fdtmap Extract the devicetree blob from the fdtmap
def testAbsent(self):
"""Check handling of absent entries"""
data = self._DoReadFile('262_absent.dts')
self.assertEqual(U_BOOT_DATA + U_BOOT_IMG_DATA, data)
self.assertEqual(U_BOOT_DATA + b'aa' + U_BOOT_IMG_DATA, data)
def testPackTeeOsOptional(self):
"""Test that an image with an optional TEE binary can be created"""
def testPackTeeOsElf(self):
"""Test that an image with a TEE elf binary can be created"""
entry_args = {
'tee-os-path': 'tee.elf',
}
tee_path = self.tee_elf_path
data = self._DoReadFileDtb('263_tee_os_opt.dts',
entry_args=entry_args)[0]
self.assertEqual(U_BOOT_DATA + U_BOOT_IMG_DATA, data)
self.assertEqual(U_BOOT_DATA + tools.read_file(tee_path) +
U_BOOT_IMG_DATA, data)
def checkFitTee(self, dts, tee_fname):
"""Check that a tee-os entry works and returns data
@@ -6512,6 +6518,9 @@ fdt fdtmap Extract the devicetree blob from the fdtmap
self.assertRegex(
err,
"Image '.*' is missing optional external blobs but is still functional: tee-os")
self.assertNotRegex(
err,
"Image '.*' has faked external blobs and is non-functional: tee-os")
def testFitTeeOsOptionalFitBad(self):
"""Test an image with a FIT with an optional OP-TEE binary"""
@@ -6537,7 +6546,15 @@ 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):
def testExtblobMissingOptional(self):
"""Test an image with an external blob that is optional"""
with terminal.capture() as (stdout, stderr):
data = self._DoReadFileDtb('266_blob_ext_opt.dts',
allow_fake_blobs=False)[0]
self.assertEqual(REFCODE_DATA, data)
self.assertNotIn(MISSING_DATA, data)
def testExtblobFakedOptional(self):
"""Test an image with an external blob that is optional"""
with terminal.capture() as (stdout, stderr):
data = self._DoReadFile('266_blob_ext_opt.dts')
@@ -6546,6 +6563,9 @@ fdt fdtmap Extract the devicetree blob from the fdtmap
self.assertRegex(
err,
"Image '.*' is missing optional external blobs but is still functional: missing")
self.assertNotRegex(
err,
"Image '.*' has faked external blobs and is non-functional: missing")
def testSectionInner(self):
"""Test an inner section with a size"""
@@ -6726,7 +6746,7 @@ fdt fdtmap Extract the devicetree blob from the fdtmap
node = dtb.GetNode('/configurations/conf-missing-tee-1')
self.assertEqual('atf-1', node.props['firmware'].value)
self.assertEqual(['u-boot', 'atf-2'],
self.assertEqual(['u-boot', 'tee', 'atf-2'],
fdt_util.GetStringList(node, 'loadables'))
def testTooldir(self):

View File

@@ -183,6 +183,8 @@ class Image(section.Entry_section):
fname = tools.get_output_filename(self._filename)
tout.info("Writing image to '%s'" % fname)
with open(fname, 'wb') as fd:
# For final image, don't write absent blobs to file
self.drop_absent_optional()
data = self.GetPaddedData()
fd.write(data)
tout.info("Wrote %#x bytes" % len(data))