diff options
25 files changed, 460 insertions, 115 deletions
diff --git a/tools/binman/binman.rst b/tools/binman/binman.rst index 0cafc36bdcb..f9a3a42183b 100644 --- a/tools/binman/binman.rst +++ b/tools/binman/binman.rst @@ -494,12 +494,18 @@ point into the image. For example, say SPL is at the start of the image and linked to start at address 80108000. If U-Boot's image-pos is 0x8000 then binman will write an image-pos for U-Boot of 80110000 into the SPL binary, since it assumes the image is loaded -to 80108000, with SPL at 80108000 and U-Boot at 80110000. +to 80108000, with SPL at 80108000 and U-Boot at 80110000. In other words, the +positions are calculated relative to the start address of the image to which +they are being written. For x86 devices (with the end-at-4gb property) this base address is not added since it is assumed that images are XIP and the offsets already include the address. +For non-x86 cases where the symbol is used as a flash offset, the symbols-base +property can be set to that offset (e.g. 0), so that the unadjusted image-pos +is written into the image. + While U-Boot's symbol updating is handled automatically by the u-boot-spl entry type (and others), it is possible to use this feature with any blob. To do this, add a `write-symbols` (boolean) property to the node, set the ELF @@ -741,6 +747,17 @@ insert-template: properties are brought into the target node. See Templates_ below for more information. +symbols-base: + When writing symbols into a binary, the value of that symbol is assumed to + be relative to the base address of the binary. This allow the binary to be + loaded in memory at its base address, so that symbols point into the binary + correctly. In some cases the binary is in fact not yet in memory, but must + be read from storage. In this case there is no base address for the symbols. + This property can be set to 0 to indicate this. Other values for + symbols-base are allowed, but care must be taken that the code which uses + the symbol is aware of the base being used. If omitted, the binary's base + address is used. + The attributes supported for images and sections are described below. Several are similar to those for entries. diff --git a/tools/binman/btool/fdtgrep.py b/tools/binman/btool/fdtgrep.py index da1f8c7bf4e..446b2f4144b 100644 --- a/tools/binman/btool/fdtgrep.py +++ b/tools/binman/btool/fdtgrep.py @@ -74,8 +74,7 @@ class Bintoolfdtgrep(bintool.Bintool): (with only neceesary nodes and properties) Returns: - CommandResult: Resulting output from the bintool, or None if the - tool is not present + str or bytes: Resulting stdout from the bintool """ if phase == 'tpl': tag = 'bootph-pre-sram' diff --git a/tools/binman/elf.py b/tools/binman/elf.py index a4694056391..c75f4478813 100644 --- a/tools/binman/elf.py +++ b/tools/binman/elf.py @@ -234,7 +234,7 @@ def GetSymbolOffset(elf_fname, sym_name, base_sym=None): return val - base def LookupAndWriteSymbols(elf_fname, entry, section, is_elf=False, - base_sym=None): + base_sym=None, base_addr=None): """Replace all symbols in an entry with their correct values The entry contents is updated so that values for referenced symbols will be @@ -247,7 +247,10 @@ def LookupAndWriteSymbols(elf_fname, entry, section, is_elf=False, entry entry: Entry to process section: Section which can be used to lookup symbol values - base_sym: Base symbol marking the start of the image + base_sym: Base symbol marking the start of the image (__image_copy_start + by default) + base_addr (int): Base address to use for the entry being written. If + None then the value of base_sym is used Returns: int: Number of symbols written @@ -277,7 +280,8 @@ def LookupAndWriteSymbols(elf_fname, entry, section, is_elf=False, if not base and not is_elf: tout.debug(f'LookupAndWriteSymbols: no base: elf_fname={elf_fname}, base_sym={base_sym}, is_elf={is_elf}') return 0 - base_addr = 0 if is_elf else base.address + if base_addr is None: + base_addr = 0 if is_elf else base.address count = 0 for name, sym in syms.items(): if name.startswith('_binman'): @@ -301,8 +305,8 @@ def LookupAndWriteSymbols(elf_fname, entry, section, is_elf=False, value = BINMAN_SYM_MAGIC_VALUE else: # Look up the symbol in our entry tables. - value = section.GetImage().LookupImageSymbol(name, sym.weak, - msg, base_addr) + value = section.GetImage().GetImageSymbolValue(name, sym.weak, + msg, base_addr) if value is None: value = -1 pack_string = pack_string.lower() diff --git a/tools/binman/elf_test.py b/tools/binman/elf_test.py index b64134123c1..2f22639dffc 100644 --- a/tools/binman/elf_test.py +++ b/tools/binman/elf_test.py @@ -37,7 +37,7 @@ class FakeSection: """A fake Section object, used for testing This has the minimum feature set needed to support testing elf functions. - A LookupSymbol() function is provided which returns a fake value for amu + A GetSymbolValue() function is provided which returns a fake value for any symbol requested. """ def __init__(self, sym_value=1): @@ -46,7 +46,7 @@ class FakeSection: def GetPath(self): return 'section_path' - def LookupImageSymbol(self, name, weak, msg, base_addr): + def GetImageSymbolValue(self, name, weak, msg, base_addr): """Fake implementation which returns the same value for all symbols""" return self.sym_value diff --git a/tools/binman/entry.py b/tools/binman/entry.py index 6d2f3789940..68f8d62bba9 100644 --- a/tools/binman/entry.py +++ b/tools/binman/entry.py @@ -108,6 +108,9 @@ class Entry(object): not need to be done again. This is only used with 'binman replace', to stop sections from being rebuilt if their entries have not been replaced + symbols_base (int): Use this value as the assumed load address of the + target entry, when calculating the symbol value. If None, this is + 0 for blobs and the image-start address for ELF files """ fake_dir = None @@ -159,6 +162,7 @@ class Entry(object): self.preserve = False self.build_done = False self.no_write_symbols = False + self.symbols_base = None @staticmethod def FindEntryClass(etype, expanded): @@ -324,6 +328,7 @@ class Entry(object): self.preserve = fdt_util.GetBool(self._node, 'preserve') self.no_write_symbols = fdt_util.GetBool(self._node, 'no-write-symbols') + self.symbols_base = fdt_util.GetInt(self._node, 'symbols-base') def GetDefaultFilename(self): return None @@ -576,8 +581,16 @@ class Entry(object): def GetEntryArgsOrProps(self, props, required=False): """Return the values of a set of properties + Looks up the named entryargs and returns the value for each. If any + required ones are missing, the error is reported to the user. + Args: - props: List of EntryArg objects + props (list of EntryArg): List of entry arguments to look up + required (bool): True if these entry arguments are required + + Returns: + list of values: one for each item in props, the type is determined + by the EntryArg's 'datatype' property (str or int) Raises: ValueError if a property is not found @@ -698,14 +711,22 @@ class Entry(object): def WriteSymbols(self, section): """Write symbol values into binary files for access at run time + As a special case, if symbols_base is not specified and this is an + end-at-4gb image, a symbols_base of 0 is used + Args: section: Section containing the entry """ if self.auto_write_symbols and not self.no_write_symbols: # Check if we are writing symbols into an ELF file is_elf = self.GetDefaultFilename() == self.elf_fname + + symbols_base = self.symbols_base + if symbols_base is None and self.GetImage()._end_4gb: + symbols_base = 0 + elf.LookupAndWriteSymbols(self.elf_fname, self, section.GetImage(), - is_elf, self.elf_base_sym) + is_elf, self.elf_base_sym, symbols_base) def CheckEntries(self): """Check that the entry offsets are correct diff --git a/tools/binman/etype/atf_fip.py b/tools/binman/etype/atf_fip.py index 3da0dfcfc12..636e073afc8 100644 --- a/tools/binman/etype/atf_fip.py +++ b/tools/binman/etype/atf_fip.py @@ -248,7 +248,7 @@ class Entry_atf_fip(Entry_section): fent = entry._fip_entry entry.size = fent.size entry.offset = fent.offset - entry.image_pos = self.image_pos + entry.offset + entry.SetImagePos(image_pos + self.offset) def ReadChildData(self, child, decomp=True, alt_format=None): if not self.reader: diff --git a/tools/binman/etype/blob_phase.py b/tools/binman/etype/blob_phase.py index 951d9934050..09bb89b3b78 100644 --- a/tools/binman/etype/blob_phase.py +++ b/tools/binman/etype/blob_phase.py @@ -57,3 +57,8 @@ class Entry_blob_phase(Entry_section): if self.no_write_symbols: for entry in self._entries.values(): entry.no_write_symbols = True + + # Propagate the symbols-base property + if self.symbols_base is not None: + for entry in self._entries.values(): + entry.symbols_base = self.symbols_base diff --git a/tools/binman/etype/cbfs.py b/tools/binman/etype/cbfs.py index 575aa624f6c..124fa1e4ffc 100644 --- a/tools/binman/etype/cbfs.py +++ b/tools/binman/etype/cbfs.py @@ -245,7 +245,7 @@ class Entry_cbfs(Entry): cfile = entry._cbfs_file entry.size = cfile.data_len entry.offset = cfile.calced_cbfs_offset - entry.image_pos = self.image_pos + entry.offset + entry.SetImagePos(image_pos + self.offset) if entry._cbfs_compress: entry.uncomp_size = cfile.memlen diff --git a/tools/binman/etype/efi_capsule.py b/tools/binman/etype/efi_capsule.py index 768e006dc50..9f06cc88e6e 100644 --- a/tools/binman/etype/efi_capsule.py +++ b/tools/binman/etype/efi_capsule.py @@ -151,6 +151,8 @@ class Entry_efi_capsule(Entry_section): return tools.read_file(capsule_fname) else: # Bintool is missing; just use the input data as the output + if not self.GetAllowMissing(): + self.Raise("Missing tool: 'mkeficapsule'") self.record_missing_bintool(self.mkeficapsule) return data diff --git a/tools/binman/etype/fit.py b/tools/binman/etype/fit.py index ee44e5a1cd6..0abe1c78c43 100644 --- a/tools/binman/etype/fit.py +++ b/tools/binman/etype/fit.py @@ -6,9 +6,10 @@ """Entry-type module for producing a FIT""" import glob -import libfdt import os +import libfdt + from binman.entry import Entry, EntryArg from binman.etype.section import Entry_section from binman import elf @@ -23,6 +24,7 @@ OPERATIONS = { 'split-elf': OP_SPLIT_ELF, } +# pylint: disable=invalid-name class Entry_fit(Entry_section): """Flat Image Tree (FIT) @@ -94,7 +96,10 @@ class Entry_fit(Entry_section): can be provided as a directory. Each .dtb file in the directory is processed, , e.g.:: - fit,fdt-list-dir = "arch/arm/dts + fit,fdt-list-dir = "arch/arm/dts"; + + In this case the input directories are ignored and all devicetree + files must be in that directory. Substitutions ~~~~~~~~~~~~~ @@ -381,31 +386,46 @@ class Entry_fit(Entry_section): def __init__(self, section, etype, node): """ Members: - _fit: FIT file being built - _entries: dict from Entry_section: + _fit (str): FIT file being built + _fit_props (list of str): 'fit,...' properties found in the + top-level node + _fdts (list of str): Filenames of .dtb files to process + _fdt_dir (str): Directory to scan to find .dtb files, or None + _fit_list_prop (str): Name of the EntryArg containing a list of .dtb + files + _fit_default_dt (str): Name of the EntryArg containing the default + .dtb file + _entries (dict of entries): from Entry_section: key: relative path to entry Node (from the base of the FIT) value: Entry_section object comprising the contents of this node - _priv_entries: Internal copy of _entries which includes 'generator' - entries which are used to create the FIT, but should not be - processed as real entries. This is set up once we have the - entries - _loadables: List of generated split-elf nodes, each a node name + _priv_entries (dict of entries): Internal copy of _entries which + includes 'generator' entries which are used to create the FIT, + but should not be processed as real entries. This is set up once + we have the entries + _loadables (list of str): List of generated split-elf nodes, each + a node name + _remove_props (list of str): Value of of-spl-remove-props EntryArg, + the list of properties to remove with fdtgrep + mkimage (Bintool): mkimage tool + fdtgrep (Bintool): fdtgrep tool """ super().__init__(section, etype, node) self._fit = None self._fit_props = {} self._fdts = None self._fdt_dir = None - self.mkimage = None - self.fdtgrep = None + self._fit_list_prop = None + self._fit_default_dt = None self._priv_entries = {} self._loadables = [] self._remove_props = [] - props, = self.GetEntryArgsOrProps( - [EntryArg('of-spl-remove-props', str)], required=False) + props = self.GetEntryArgsOrProps( + [EntryArg('of-spl-remove-props', str)], required=False)[0] if props: self._remove_props = props.split() + self.mkimage = None + self.fdtgrep = None def ReadNode(self): super().ReadNode() @@ -414,8 +434,8 @@ class Entry_fit(Entry_section): self._fit_props[pname] = prop self._fit_list_prop = self._fit_props.get('fit,fdt-list') if self._fit_list_prop: - fdts, = self.GetEntryArgsOrProps( - [EntryArg(self._fit_list_prop.value, str)]) + fdts = self.GetEntryArgsOrProps( + [EntryArg(self._fit_list_prop.value, str)])[0] if fdts is not None: self._fdts = fdts.split() else: @@ -431,7 +451,7 @@ class Entry_fit(Entry_section): self._fit_default_dt = self.GetEntryArgsOrProps([EntryArg('default-dt', str)])[0] - def _get_operation(self, base_node, node): + def _get_operation(self, node): """Get the operation referenced by a subnode Args: @@ -550,6 +570,9 @@ class Entry_fit(Entry_section): phase (str): Phase to generate for ('tpl', 'vpl', 'spl') outfile (str): Output filename to write the grepped FDT contents to (with only neceesary nodes and properties) + + Returns: + str or bytes: Resulting stdout from fdtgrep """ return self.fdtgrep.create_for_phase(infile, phase, outfile, self._remove_props) @@ -557,9 +580,6 @@ class Entry_fit(Entry_section): def _build_input(self): """Finish the FIT by adding the 'data' properties to it - Arguments: - fdt: FIT to update - Returns: bytes: New fdt contents """ @@ -580,13 +600,17 @@ class Entry_fit(Entry_section): if val.startswith('@'): if not self._fdts: return - if not self._fit_default_dt: + default_dt = self._fit_default_dt + if not default_dt: self.Raise("Generated 'default' node requires default-dt entry argument") - if self._fit_default_dt not in self._fdts: - self.Raise( - f"default-dt entry argument '{self._fit_default_dt}' " - f"not found in fdt list: {', '.join(self._fdts)}") - seq = self._fdts.index(self._fit_default_dt) + if default_dt not in self._fdts: + if self._fdt_dir: + default_dt = os.path.basename(default_dt) + if default_dt not in self._fdts: + self.Raise( + f"default-dt entry argument '{self._fit_default_dt}' " + f"not found in fdt list: {', '.join(self._fdts)}") + seq = self._fdts.index(default_dt) val = val[1:].replace('DEFAULT-SEQ', str(seq + 1)) fsw.property_string(pname, val) return @@ -634,7 +658,7 @@ class Entry_fit(Entry_section): result.append(name) return firmware, result - def _gen_fdt_nodes(base_node, node, depth, in_images): + def _gen_fdt_nodes(node, depth, in_images): """Generate FDT nodes This creates one node for each member of self._fdts using the @@ -654,7 +678,10 @@ class Entry_fit(Entry_section): # Generate nodes for each FDT for seq, fdt_fname in enumerate(self._fdts): node_name = node.name[1:].replace('SEQ', str(seq + 1)) - fname = tools.get_input_filename(fdt_fname + '.dtb') + if self._fdt_dir: + fname = os.path.join(self._fdt_dir, fdt_fname + '.dtb') + else: + fname = tools.get_input_filename(fdt_fname + '.dtb') fdt_phase = None with fsw.add_node(node_name): for pname, prop in node.props.items(): @@ -688,8 +715,9 @@ class Entry_fit(Entry_section): # Add data for 'images' nodes (but not 'config') if depth == 1 and in_images: if fdt_phase: + leaf = os.path.basename(fdt_fname) phase_fname = tools.get_output_filename( - f'{fdt_fname}-{fdt_phase}.dtb') + f'{leaf}-{fdt_phase}.dtb') self._run_fdtgrep(fname, fdt_phase, phase_fname) data = tools.read_file(phase_fname) else: @@ -707,11 +735,10 @@ class Entry_fit(Entry_section): else: self.Raise("Generator node requires 'fit,fdt-list' property") - def _gen_split_elf(base_node, node, depth, segments, entry_addr): + def _gen_split_elf(node, depth, segments, entry_addr): """Add nodes for the ELF file, one per group of contiguous segments Args: - base_node (Node): Template node from the binman definition node (Node): Node to replace (in the FIT being built) depth: Current node depth (0 is the base 'fit' node) segments (list): list of segments, each: @@ -742,7 +769,7 @@ class Entry_fit(Entry_section): with fsw.add_node(subnode.name): _add_node(node, depth + 1, subnode) - def _gen_node(base_node, node, depth, in_images, entry): + def _gen_node(node, depth, in_images, entry): """Generate nodes from a template This creates one or more nodes depending on the fit,operation being @@ -758,8 +785,6 @@ class Entry_fit(Entry_section): If the file is missing, nothing is generated. Args: - base_node (Node): Base Node of the FIT (with 'description' - property) node (Node): Generator node to process depth (int): Current node depth (0 is the base 'fit' node) in_images (bool): True if this is inside the 'images' node, so @@ -767,13 +792,12 @@ class Entry_fit(Entry_section): entry (entry_Section): Entry for the section containing the contents of this node """ - oper = self._get_operation(base_node, node) + oper = self._get_operation(node) if oper == OP_GEN_FDT_NODES: - _gen_fdt_nodes(base_node, node, depth, in_images) + _gen_fdt_nodes(node, depth, in_images) elif oper == OP_SPLIT_ELF: # Entry_section.ObtainContents() either returns True or # raises an exception. - data = None missing_opt_list = [] entry.ObtainContents() entry.Pack(0) @@ -795,7 +819,7 @@ class Entry_fit(Entry_section): self._raise_subnode( node, f'Failed to read ELF file: {str(exc)}') - _gen_split_elf(base_node, node, depth, segments, entry_addr) + _gen_split_elf(node, depth, segments, entry_addr) def _add_node(base_node, depth, node): """Add nodes to the output FIT @@ -826,7 +850,6 @@ class Entry_fit(Entry_section): fsw.property('data', bytes(data)) for subnode in node.subnodes: - subnode_path = f'{rel_path}/{subnode.name}' if has_images and not self.IsSpecialSubnode(subnode): # This subnode is a content node not meant to appear in # the FIT (e.g. "/images/kernel/u-boot"), so don't call @@ -834,7 +857,7 @@ class Entry_fit(Entry_section): pass elif self.GetImage().generate and subnode.name.startswith('@'): entry = self._priv_entries.get(subnode.name) - _gen_node(base_node, subnode, depth, in_images, entry) + _gen_node(subnode, depth, in_images, entry) # This is a generator (template) entry, so remove it from # the list of entries used by PackEntries(), etc. Otherwise # it will appear in the binman output @@ -876,7 +899,10 @@ class Entry_fit(Entry_section): """ if self.build_done: return - super().SetImagePos(image_pos) + + # Skip the section processing, since we do that below. Just call the + # entry method + Entry.SetImagePos(self, image_pos) # If mkimage is missing we'll have empty data, # which will cause a FDT_ERR_BADMAGIC error @@ -886,7 +912,7 @@ class Entry_fit(Entry_section): fdt = Fdt.FromData(self.GetData()) fdt.Scan() - for image_name, section in self._entries.items(): + for image_name, entry in self._entries.items(): path = f"/images/{image_name}" node = fdt.GetNode(path) @@ -914,10 +940,12 @@ class Entry_fit(Entry_section): # This should never happen else: # pragma: no cover + offset = None + size = None self.Raise(f'{path}: missing data properties') - section.SetOffsetSize(offset, size) - section.SetImagePos(self.image_pos) + entry.SetOffsetSize(offset, size) + entry.SetImagePos(image_pos + self.offset) def AddBintools(self, btools): super().AddBintools(btools) @@ -947,7 +975,7 @@ class Entry_fit(Entry_section): if input_fname: fname = input_fname else: - fname = tools.get_output_filename('%s.fit' % uniq) + fname = tools.get_output_filename(f'{uniq}.fit') tools.write_file(fname, self.GetData()) args.append(fname) diff --git a/tools/binman/etype/nxp_imx8mimage.py b/tools/binman/etype/nxp_imx8mimage.py index 3585120b79b..8ad177b3b65 100644 --- a/tools/binman/etype/nxp_imx8mimage.py +++ b/tools/binman/etype/nxp_imx8mimage.py @@ -27,7 +27,8 @@ class Entry_nxp_imx8mimage(Entry_mkimage): def __init__(self, section, etype, node): super().__init__(section, etype, node) - self.required_props = ['nxp,boot-from', 'nxp,rom-version', 'nxp,loader-address'] + self.required_props = ['nxp,boot-from', 'nxp,rom-version', + 'nxp,loader-address'] def ReadNode(self): super().ReadNode() diff --git a/tools/binman/etype/section.py b/tools/binman/etype/section.py index 30c1041c7e8..f4f48c00e87 100644 --- a/tools/binman/etype/section.py +++ b/tools/binman/etype/section.py @@ -563,13 +563,13 @@ class Entry_section(Entry): return entry.GetData(required) def LookupEntry(self, entries, sym_name, msg): - """Look up the entry for an ENF symbol + """Look up the entry for a binman symbol Args: entries (dict): entries to search: key: entry name value: Entry object - sym_name: Symbol name in the ELF file to look up in the format + sym_name: Symbol name to look up in the format _binman_<entry>_prop_<property> where <entry> is the name of the entry and <property> is the property to find (e.g. _binman_u_boot_prop_offset). As a special case, you can append @@ -606,11 +606,10 @@ class Entry_section(Entry): entry = entries[name] return entry, entry_name, prop_name - def LookupSymbol(self, sym_name, optional, msg, base_addr, entries=None): - """Look up a symbol in an ELF file + def GetSymbolValue(self, sym_name, optional, msg, base_addr, entries=None): + """Get the value of a Binman symbol - Looks up a symbol in an ELF file. Only entry types which come from an - ELF image can be used by this function. + Look up a Binman symbol and obtain its value. At present the only entry properties supported are: offset @@ -618,7 +617,7 @@ class Entry_section(Entry): size Args: - sym_name: Symbol name in the ELF file to look up in the format + sym_name: Symbol name to look up in the format _binman_<entry>_prop_<property> where <entry> is the name of the entry and <property> is the property to find (e.g. _binman_u_boot_prop_offset). As a special case, you can append @@ -628,12 +627,10 @@ class Entry_section(Entry): optional: True if the symbol is optional. If False this function will raise if the symbol is not found msg: Message to display if an error occurs - base_addr: Base address of image. This is added to the returned - image_pos in most cases so that the returned position indicates - where the targetted entry/binary has actually been loaded. But - if end-at-4gb is used, this is not done, since the binary is - already assumed to be linked to the ROM position and using - execute-in-place (XIP). + base_addr (int): Base address of image. This is added to the + returned value of image-pos so that the returned position + indicates where the targeted entry/binary has actually been + loaded Returns: Value that should be assigned to that symbol, or None if it was @@ -656,10 +653,10 @@ class Entry_section(Entry): if prop_name == 'offset': return entry.offset elif prop_name == 'image_pos': - value = entry.image_pos - if not self.GetImage()._end_4gb: - value += base_addr - return value + if not entry.image_pos: + tout.info(f'Symbol-writing: no value for {entry._node.path}') + return None + return base_addr + entry.image_pos if prop_name == 'size': return entry.size else: diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index 2577c0016c0..e3f231e4bcc 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -403,8 +403,10 @@ class TestFunctional(unittest.TestCase): test_section_timeout: True to force the first time to timeout, as used in testThreadTimeout() update_fdt_in_elf: Value to pass with --update-fdt-in-elf=xxx - force_missing_tools (str): comma-separated list of bintools to + force_missing_bintools (str): comma-separated list of bintools to regard as missing + ignore_missing (bool): True to return success even if there are + missing blobs or bintools output_dir: Specific output directory to use for image using -O Returns: @@ -503,8 +505,9 @@ class TestFunctional(unittest.TestCase): return dtb.GetContents() def _DoReadFileDtb(self, fname, use_real_dtb=False, use_expanded=False, - map=False, update_dtb=False, entry_args=None, - reset_dtbs=True, extra_indirs=None, threads=None): + verbosity=None, 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 @@ -521,6 +524,7 @@ class TestFunctional(unittest.TestCase): But in some test we need the real contents. 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) 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 @@ -557,7 +561,8 @@ class TestFunctional(unittest.TestCase): try: retcode = self._DoTestFile(fname, map=map, update_dtb=update_dtb, entry_args=entry_args, use_real_dtb=use_real_dtb, - use_expanded=use_expanded, extra_indirs=extra_indirs, + use_expanded=use_expanded, verbosity=verbosity, + extra_indirs=extra_indirs, threads=threads) self.assertEqual(0, retcode) out_dtb_fname = tools.get_output_filename('u-boot.dtb.out') @@ -1498,18 +1503,22 @@ class TestFunctional(unittest.TestCase): self.assertEqual(U_BOOT_SPL_NODTB_DATA, data[:len(U_BOOT_SPL_NODTB_DATA)]) def checkSymbols(self, dts, base_data, u_boot_offset, entry_args=None, - use_expanded=False, no_write_symbols=False): + use_expanded=False, no_write_symbols=False, + symbols_base=None): """Check the image contains the expected symbol values Args: dts: Device tree file to use for test base_data: Data before and after 'u-boot' section - u_boot_offset: Offset of 'u-boot' section in image + u_boot_offset (int): Offset of 'u-boot' section in image, or None if + the offset not available due to it being in a compressed section entry_args: Dict of entry args to supply to binman key: arg name value: value of that arg use_expanded: True to use expanded entries where available, e.g. 'u-boot-expanded' instead of 'u-boot' + symbols_base (int): Value to expect for symbols-base in u-boot-spl, + None if none """ elf_fname = self.ElfTestFile('u_boot_binman_syms') syms = elf.GetSymbols(elf_fname, ['binman', 'image']) @@ -1520,22 +1529,64 @@ class TestFunctional(unittest.TestCase): self._SetupSplElf('u_boot_binman_syms') data = self._DoReadFileDtb(dts, entry_args=entry_args, - use_expanded=use_expanded)[0] + use_expanded=use_expanded, + verbosity=None if u_boot_offset else 3)[0] + + # The lz4-compressed version of the U-Boot data is 19 bytes long + comp_uboot_len = 19 + # The image should contain the symbols from u_boot_binman_syms.c # Note that image_pos is adjusted by the base address of the image, # which is 0x10 in our test image - sym_values = struct.pack('<LLQLL', elf.BINMAN_SYM_MAGIC_VALUE, - 0x00, u_boot_offset + len(U_BOOT_DATA), - 0x10 + u_boot_offset, 0x04) + # If u_boot_offset is None, Binman should write -1U into the image + vals2 = (elf.BINMAN_SYM_MAGIC_VALUE, 0x00, + u_boot_offset + len(U_BOOT_DATA) if u_boot_offset else + len(U_BOOT_SPL_DATA) + 1 + comp_uboot_len, + 0x10 + u_boot_offset if u_boot_offset else 0xffffffff, 0x04) + + # u-boot-spl has a symbols-base property, so take that into account if + # required. The caller must supply the value + vals = list(vals2) + if symbols_base is not None: + vals[3] = symbols_base + u_boot_offset + vals = tuple(vals) + + sym_values = struct.pack('<LLQLL', *vals) + sym_values2 = struct.pack('<LLQLL', *vals2) if no_write_symbols: - expected = (base_data + - tools.get_bytes(0xff, 0x38 - len(base_data)) + - U_BOOT_DATA + base_data) + self.assertEqual( + base_data + + tools.get_bytes(0xff, 0x38 - len(base_data)) + + U_BOOT_DATA + base_data, data) else: - expected = (sym_values + base_data[24:] + - tools.get_bytes(0xff, 1) + U_BOOT_DATA + sym_values + - base_data[24:]) - self.assertEqual(expected, data) + got_vals = struct.unpack('<LLQLL', data[:24]) + + # For debugging: + #print('expect:', list(f'{v:x}' for v in vals)) + #print(' got:', list(f'{v:x}' for v in got_vals)) + + self.assertEqual(vals, got_vals) + self.assertEqual(sym_values, data[:24]) + + blen = len(base_data) + self.assertEqual(base_data[24:], data[24:blen]) + self.assertEqual(0xff, data[blen]) + + if u_boot_offset: + ofs = blen + 1 + len(U_BOOT_DATA) + self.assertEqual(U_BOOT_DATA, data[blen + 1:ofs]) + else: + ofs = blen + 1 + comp_uboot_len + + self.assertEqual(sym_values2, data[ofs:ofs + 24]) + self.assertEqual(base_data[24:], data[ofs + 24:]) + + # Just repeating the above asserts all at once, for clarity + if u_boot_offset: + expected = (sym_values + base_data[24:] + + tools.get_bytes(0xff, 1) + U_BOOT_DATA + + sym_values2 + base_data[24:]) + self.assertEqual(expected, data) def testSymbols(self): """Test binman can assign symbols embedded in U-Boot""" @@ -4181,7 +4232,8 @@ class TestFunctional(unittest.TestCase): data = self._DoReadFile('172_scp.dts') self.assertEqual(SCP_DATA, data[:len(SCP_DATA)]) - def CheckFitFdt(self, dts='170_fit_fdt.dts', use_fdt_list=True): + def CheckFitFdt(self, dts='170_fit_fdt.dts', use_fdt_list=True, + default_dt=None): """Check an image with an FIT with multiple FDT images""" def _CheckFdt(seq, expected_data): """Check the FDT nodes @@ -4225,6 +4277,8 @@ class TestFunctional(unittest.TestCase): } if use_fdt_list: entry_args['of-list'] = 'test-fdt1 test-fdt2' + if default_dt: + entry_args['default-dt'] = default_dt data = self._DoReadFileDtb( dts, entry_args=entry_args, @@ -7624,7 +7678,22 @@ fdt fdtmap Extract the devicetree blob from the fdtmap def testFitFdtListDir(self): """Test an image with an FIT with FDT images using fit,fdt-list-dir""" - self.CheckFitFdt('333_fit_fdt_dir.dts', False) + old_dir = os.getcwd() + try: + os.chdir(self._indir) + self.CheckFitFdt('333_fit_fdt_dir.dts', False) + finally: + os.chdir(old_dir) + + def testFitFdtListDirDefault(self): + """Test an FIT fit,fdt-list-dir where the default DT in is a subdir""" + old_dir = os.getcwd() + try: + os.chdir(self._indir) + self.CheckFitFdt('333_fit_fdt_dir.dts', False, + default_dt='rockchip/test-fdt2') + finally: + os.chdir(old_dir) def testFitFdtCompat(self): """Test an image with an FIT with compatible in the config nodes""" @@ -7690,6 +7759,51 @@ fdt fdtmap Extract the devicetree blob from the fdtmap # Make sure the other node is gone self.assertIsNone(dtb.GetNode('/node/other-node')) + def testMkeficapsuleMissing(self): + """Test that binman complains if mkeficapsule is missing""" + with self.assertRaises(ValueError) as e: + self._DoTestFile('311_capsule.dts', + force_missing_bintools='mkeficapsule') + self.assertIn("Node '/binman/efi-capsule': Missing tool: 'mkeficapsule'", + str(e.exception)) + + def testMkeficapsuleMissingOk(self): + """Test that binman deals with mkeficapsule being missing""" + with test_util.capture_sys_output() as (stdout, stderr): + ret = self._DoTestFile('311_capsule.dts', + force_missing_bintools='mkeficapsule', + allow_missing=True) + self.assertEqual(103, ret) + err = stderr.getvalue() + self.assertRegex(err, "Image 'image'.*missing bintools.*: mkeficapsule") + + def testSymbolsBase(self): + """Test handling of symbols-base""" + self.checkSymbols('336_symbols_base.dts', U_BOOT_SPL_DATA, 0x1c, + symbols_base=0) + + def testSymbolsBaseExpanded(self): + """Test handling of symbols-base with expanded entries""" + entry_args = { + 'spl-dtb': '1', + } + self.checkSymbols('337_symbols_base_expand.dts', U_BOOT_SPL_NODTB_DATA + + U_BOOT_SPL_DTB_DATA, 0x38, + entry_args=entry_args, use_expanded=True, + symbols_base=0) + + def testSymbolsCompressed(self): + """Test binman complains about symbols from a compressed section""" + with test_util.capture_sys_output() as (stdout, stderr): + self.checkSymbols('338_symbols_comp.dts', U_BOOT_SPL_DATA, None) + out = stdout.getvalue() + self.assertIn('Symbol-writing: no value for /binman/section/u-boot', + out) + + def testNxpImx8Image(self): + """Test that binman can produce an iMX8 image""" + self._DoTestFile('339_nxp_imx8.dts') + if __name__ == "__main__": unittest.main() diff --git a/tools/binman/image.py b/tools/binman/image.py index 702c9055585..24ce0af7c72 100644 --- a/tools/binman/image.py +++ b/tools/binman/image.py @@ -381,11 +381,10 @@ class Image(section.Entry_section): selected_entries.append(entry) return selected_entries, lines, widths - def LookupImageSymbol(self, sym_name, optional, msg, base_addr): - """Look up a symbol in an ELF file + def GetImageSymbolValue(self, sym_name, optional, msg, base_addr): + """Get the value of a Binman symbol - Looks up a symbol in an ELF file. Only entry types which come from an - ELF image can be used by this function. + Look up a Binman symbol and obtain its value. This searches through this image including all of its subsections. @@ -405,12 +404,10 @@ class Image(section.Entry_section): optional: True if the symbol is optional. If False this function will raise if the symbol is not found msg: Message to display if an error occurs - base_addr: Base address of image. This is added to the returned - image_pos in most cases so that the returned position indicates - where the targeted entry/binary has actually been loaded. But - if end-at-4gb is used, this is not done, since the binary is - already assumed to be linked to the ROM position and using - execute-in-place (XIP). + base_addr (int): Base address of image. This is added to the + returned value of image-pos so that the returned position + indicates where the targeted entry/binary has actually been + loaded Returns: Value that should be assigned to that symbol, or None if it was @@ -423,8 +420,8 @@ class Image(section.Entry_section): entries = OrderedDict() entries_by_name = {} self._CollectEntries(entries, entries_by_name, self) - return self.LookupSymbol(sym_name, optional, msg, base_addr, - entries_by_name) + return self.GetSymbolValue(sym_name, optional, msg, base_addr, + entries_by_name) def CollectBintools(self): """Collect all the bintools used by this image diff --git a/tools/binman/image_test.py b/tools/binman/image_test.py index bd51c1e55d1..7d65e2d589a 100644 --- a/tools/binman/image_test.py +++ b/tools/binman/image_test.py @@ -13,7 +13,7 @@ class TestImage(unittest.TestCase): def testInvalidFormat(self): image = Image('name', 'node', test=True) with self.assertRaises(ValueError) as e: - image.LookupSymbol('_binman_something_prop_', False, 'msg', 0) + image.GetSymbolValue('_binman_something_prop_', False, 'msg', 0) self.assertIn( "msg: Symbol '_binman_something_prop_' has invalid format", str(e.exception)) @@ -22,7 +22,7 @@ class TestImage(unittest.TestCase): image = Image('name', 'node', test=True) image._entries = {} with self.assertRaises(ValueError) as e: - image.LookupSymbol('_binman_type_prop_pname', False, 'msg', 0) + image.GetSymbolValue('_binman_type_prop_pname', False, 'msg', 0) self.assertIn("msg: Entry 'type' not found in list ()", str(e.exception)) @@ -30,7 +30,7 @@ class TestImage(unittest.TestCase): image = Image('name', 'node', test=True) image._entries = {} with capture_sys_output() as (stdout, stderr): - val = image.LookupSymbol('_binman_type_prop_pname', True, 'msg', 0) + val = image.GetSymbolValue('_binman_type_prop_pname', True, 'msg', 0) self.assertEqual(val, None) self.assertEqual("Warning: msg: Entry 'type' not found in list ()\n", stderr.getvalue()) @@ -40,5 +40,5 @@ class TestImage(unittest.TestCase): image = Image('name', 'node', test=True) image._entries = {'u-boot': 1} with self.assertRaises(ValueError) as e: - image.LookupSymbol('_binman_u_boot_prop_bad', False, 'msg', 0) + image.GetSymbolValue('_binman_u_boot_prop_bad', False, 'msg', 0) self.assertIn("msg: No such property 'bad", str(e.exception)) diff --git a/tools/binman/test/336_symbols_base.dts b/tools/binman/test/336_symbols_base.dts new file mode 100644 index 00000000000..e4dccd38c22 --- /dev/null +++ b/tools/binman/test/336_symbols_base.dts @@ -0,0 +1,23 @@ +// SPDX-License-Identifier: GPL-2.0+ + +/dts-v1/; + +/ { + #address-cells = <1>; + #size-cells = <1>; + + binman { + pad-byte = <0xff>; + u-boot-spl { + symbols-base = <0>; + }; + + u-boot { + offset = <0x1c>; + }; + + u-boot-spl2 { + type = "u-boot-spl"; + }; + }; +}; diff --git a/tools/binman/test/337_symbols_base_expand.dts b/tools/binman/test/337_symbols_base_expand.dts new file mode 100644 index 00000000000..5a777ae63b8 --- /dev/null +++ b/tools/binman/test/337_symbols_base_expand.dts @@ -0,0 +1,24 @@ +// SPDX-License-Identifier: GPL-2.0+ + +/dts-v1/; + +/ { + #address-cells = <1>; + #size-cells = <1>; + + binman { + pad-byte = <0xff>; + u-boot-spl { + symbols-base = <0>; + }; + + u-boot { + offset = <0x38>; + no-expanded; + }; + + u-boot-spl2 { + type = "u-boot-spl"; + }; + }; +}; diff --git a/tools/binman/test/338_symbols_comp.dts b/tools/binman/test/338_symbols_comp.dts new file mode 100644 index 00000000000..15008507cfd --- /dev/null +++ b/tools/binman/test/338_symbols_comp.dts @@ -0,0 +1,26 @@ +// SPDX-License-Identifier: GPL-2.0+ + +/dts-v1/; + +/ { + #address-cells = <1>; + #size-cells = <1>; + + binman { + pad-byte = <0xff>; + u-boot-spl { + }; + + section { + offset = <0x1c>; + compress = "lz4"; + + u-boot { + }; + }; + + u-boot-spl2 { + type = "u-boot-spl"; + }; + }; +}; diff --git a/tools/binman/test/339_nxp_imx8.dts b/tools/binman/test/339_nxp_imx8.dts new file mode 100644 index 00000000000..cb512ae9aa2 --- /dev/null +++ b/tools/binman/test/339_nxp_imx8.dts @@ -0,0 +1,17 @@ +// SPDX-License-Identifier: GPL-2.0+ + +/dts-v1/; + +/ { + #address-cells = <1>; + #size-cells = <1>; + + binman { + nxp-imx8mimage { + args; /* TODO: Needed by mkimage etype superclass */ + nxp,boot-from = "sd"; + nxp,rom-version = <1>; + nxp,loader-address = <0x10>; + }; + }; +}; diff --git a/tools/buildman/builder.py b/tools/buildman/builder.py index c4384f53e8d..4090d328b30 100644 --- a/tools/buildman/builder.py +++ b/tools/buildman/builder.py @@ -22,6 +22,7 @@ from buildman import toolchain from patman import gitutil from u_boot_pylib import command from u_boot_pylib import terminal +from u_boot_pylib import tools from u_boot_pylib.terminal import tprint # This indicates an new int or hex Kconfig property with no default @@ -263,7 +264,8 @@ class Builder: adjust_cfg=None, allow_missing=False, no_lto=False, reproducible_builds=False, force_build=False, force_build_failures=False, force_reconfig=False, - in_tree=False, force_config_on_failure=False, make_func=None): + in_tree=False, force_config_on_failure=False, make_func=None, + dtc_skip=False): """Create a new Builder object Args: @@ -312,6 +314,7 @@ class Builder: force_config_on_failure (bool): Reconfigure the build before retrying a failed build make_func (function): Function to call to run 'make' + dtc_skip (bool): True to skip building dtc and use the system one """ self.toolchains = toolchains self.base_dir = base_dir @@ -354,6 +357,12 @@ class Builder: self.in_tree = in_tree self.force_config_on_failure = force_config_on_failure self.fallback_mrproper = fallback_mrproper + if dtc_skip: + self.dtc = shutil.which('dtc') + if not self.dtc: + raise ValueError('Cannot find dtc') + else: + self.dtc = None if not self.squash_config_y: self.config_filenames += EXTRA_CONFIG_FILENAMES @@ -407,6 +416,22 @@ class Builder: def signal_handler(self, signal, frame): sys.exit(1) + def make_environment(self, toolchain): + """Create the environment to use for building + + Args: + toolchain (Toolchain): Toolchain to use for building + + Returns: + dict: + key (str): Variable name + value (str): Variable value + """ + env = toolchain.MakeEnvironment(self.full_path) + if self.dtc: + env[b'DTC'] = tools.to_bytes(self.dtc) + return env + def set_display_options(self, show_errors=False, show_sizes=False, show_detail=False, show_bloat=False, list_error_boards=False, show_config=False, diff --git a/tools/buildman/builderthread.py b/tools/buildman/builderthread.py index bbe2f6f0d24..b5afee61aff 100644 --- a/tools/buildman/builderthread.py +++ b/tools/buildman/builderthread.py @@ -406,7 +406,7 @@ class BuilderThread(threading.Thread): the next incremental build """ # Set up the environment and command line - env = self.toolchain.MakeEnvironment(self.builder.full_path) + env = self.builder.make_environment(self.toolchain) mkdir(out_dir) args, cwd, src_dir = self._build_args(brd, out_dir, out_rel_dir, @@ -574,7 +574,7 @@ class BuilderThread(threading.Thread): outf.write(f'{result.return_code}') # Write out the image and function size information and an objdump - env = result.toolchain.MakeEnvironment(self.builder.full_path) + env = self.builder.make_environment(self.toolchain) with open(os.path.join(build_dir, 'out-env'), 'wb') as outf: for var in sorted(env.keys()): outf.write(b'%s="%s"' % (var, env[var])) @@ -755,6 +755,14 @@ class BuilderThread(threading.Thread): self.mrproper, self.builder.config_only, True, self.builder.force_build_failures, job.work_in_output, job.adjust_cfg) + failed = result.return_code or result.stderr + if failed and not self.mrproper: + result, request_config = self.run_commit(None, brd, work_dir, + True, self.builder.fallback_mrproper, + self.builder.config_only, True, + self.builder.force_build_failures, + job.work_in_output, job.adjust_cfg) + result.commit_upto = 0 self._write_result(result, job.keep_outputs, job.work_in_output) self._send_result(result) diff --git a/tools/buildman/buildman.rst b/tools/buildman/buildman.rst index b8ff3bf1ab2..e873611e596 100644 --- a/tools/buildman/buildman.rst +++ b/tools/buildman/buildman.rst @@ -1030,6 +1030,9 @@ of the source tree, thus allowing rapid tested evolution of the code:: ./tools/buildman/buildman -Pr tegra +Note also the `--dtc-skip` option which uses the system device-tree compiler to +avoid needing to build it for each board. This can save 10-20% of build time. +An alternative is to set DTC=/path/to/dtc when running buildman. Checking configuration ---------------------- diff --git a/tools/buildman/cmdline.py b/tools/buildman/cmdline.py index 544a391a464..7573e5bdfe8 100644 --- a/tools/buildman/cmdline.py +++ b/tools/buildman/cmdline.py @@ -46,6 +46,8 @@ def add_upto_m(parser): help='Show detailed size delta for each board in the -S summary') parser.add_argument('-D', '--debug', action='store_true', help='Enabling debugging (provides a full traceback on error)') + parser.add_argument('--dtc-skip', action='store_true', default=False, + help='Skip building of dtc and use the system version') parser.add_argument('-e', '--show_errors', action='store_true', default=False, help='Show errors and warnings') parser.add_argument('-E', '--warnings-as-errors', action='store_true', diff --git a/tools/buildman/control.py b/tools/buildman/control.py index d3d027f02ab..55d4d770c5c 100644 --- a/tools/buildman/control.py +++ b/tools/buildman/control.py @@ -809,7 +809,8 @@ def do_buildman(args, toolchains=None, make_func=None, brds=None, force_build = args.force_build, force_build_failures = args.force_build_failures, force_reconfig = args.force_reconfig, in_tree = args.in_tree, - force_config_on_failure=not args.quick, make_func=make_func) + force_config_on_failure=not args.quick, make_func=make_func, + dtc_skip=args.dtc_skip) TEST_BUILDER = builder diff --git a/tools/buildman/test.py b/tools/buildman/test.py index 46aa2a17916..15801f6097f 100644 --- a/tools/buildman/test.py +++ b/tools/buildman/test.py @@ -999,6 +999,37 @@ class TestBuild(unittest.TestCase): self.assertEqual( {b'CROSS_COMPILE': b'fred aarch64-linux-', b'LC_ALL': b'C'}, diff) + def test_skip_dtc(self): + """Test skipping building the dtc tool""" + old_path = os.getenv('PATH') + try: + os.environ['PATH'] = self.base_dir + + # Check a missing tool + with self.assertRaises(ValueError) as exc: + builder.Builder(self.toolchains, self.base_dir, None, 0, 2, + dtc_skip=True) + self.assertIn('Cannot find dtc', str(exc.exception)) + + # Create a fake tool to use + dtc = os.path.join(self.base_dir, 'dtc') + tools.write_file(dtc, b'xx') + os.chmod(dtc, 0o777) + + build = builder.Builder(self.toolchains, self.base_dir, None, 0, 2, + dtc_skip=True) + toolchain = self.toolchains.Select('arm') + env = build.make_environment(toolchain) + self.assertIn(b'DTC', env) + + # Try the normal case, i.e. not skipping the dtc build + build = builder.Builder(self.toolchains, self.base_dir, None, 0, 2) + toolchain = self.toolchains.Select('arm') + env = build.make_environment(toolchain) + self.assertNotIn(b'DTC', env) + finally: + os.environ['PATH'] = old_path + if __name__ == "__main__": unittest.main() |