mirror of
https://source.denx.de/u-boot/u-boot.git
synced 2026-06-05 19:26:40 +03:00
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:
@@ -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.
|
||||
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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):
|
||||
|
||||
@@ -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:
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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):
|
||||
|
||||
@@ -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))
|
||||
|
||||
Reference in New Issue
Block a user