When support for sections (and thus hierarchical images) was added to binman, the decision was made to create a new Section class which could be used by both Image and an Entry_section class. The decision between using inheritance and composition was tricky to make, but in the end it was decided that Image was different enough from Entry that it made sense to put the implementation of sections in an entirely separate class. It also has the advantage that core Image code does have to rely on an entry class in the etype directory.
This work was mostly completed in commit:
8f1da50ccc "binman: Refactor much of the image code into 'section'
As a result of this, the Section class has its own version of things like offset and size and these must be kept in sync with the parent Entry_section class in some cases.
In the last year it has become apparent that the cost of keeping things in sync is larger than expected, since more and more code wants to access these properties.
An alternative approach, previously considered and rejected, now seems better.
Adjust Image to be a subclass of Entry_section. Move the code from Section (in bsection.py) to Entry_section and delete Section. Update all tests accordingly.
This requires substantial changes to Image. Overall the changes reduce code size by about 240 lines. While much of that is just boilerplate from Section, there are quite a few functions in Entry_section which now do not need to be overiden from Entry. This suggests the change is beneficial even without further functionality being added.
A side benefit is that the properties of sections are now consistent with other entries. This fixes a problem in testListCmd() where some properties are missing for sections.
Unfortunately this is a very large commit since it is not feasible to do the migration piecemeal. Given the substantial tests available and the 100% code coverage of binman, we should be able to do this safely.
Signed-off-by: Simon Glass sjg@chromium.org ---
Changes in v2: None
tools/binman/README.entries | 21 +- tools/binman/bsection.py | 523 ---------------------------------- tools/binman/entry.py | 8 +- tools/binman/etype/files.py | 3 +- tools/binman/etype/fmap.py | 2 +- tools/binman/etype/section.py | 431 +++++++++++++++++++++++++--- tools/binman/ftest.py | 29 +- tools/binman/image.py | 129 +++------ tools/binman/image_test.py | 18 +- 9 files changed, 462 insertions(+), 702 deletions(-) delete mode 100644 tools/binman/bsection.py
diff --git a/tools/binman/README.entries b/tools/binman/README.entries index 598d8278a70..7ce88ee5da8 100644 --- a/tools/binman/README.entries +++ b/tools/binman/README.entries @@ -521,16 +521,21 @@ Entry: section: Entry that contains other entries -------------------------------------------------
Properties / Entry arguments: (see binman README for more information) - - size: Size of section in bytes - - align-size: Align size to a particular power of two - - pad-before: Add padding before the entry - - pad-after: Add padding after the entry - - pad-byte: Pad byte to use when padding - - sort-by-offset: Reorder the entries by offset - - end-at-4gb: Used to build an x86 ROM which ends at 4GB (2^32) - - name-prefix: Adds a prefix to the name of every entry in the section + pad-byte: Pad byte to use when padding + sort-by-offset: True if entries should be sorted by offset, False if + they must be in-order in the device tree description + end-at-4gb: Used to build an x86 ROM which ends at 4GB (2^32) + skip-at-start: Number of bytes before the first entry starts. These + effectively adjust the starting offset of entries. For example, + if this is 16, then the first entry would start at 16. An entry + with offset = 20 would in fact be written at offset 4 in the image + file, since the first 16 bytes are skipped when writing. + name-prefix: Adds a prefix to the name of every entry in the section when writing out the map
+Since a section is also an entry, it inherits all the properies of entries +too. + A section is an entry which can contain other entries, thus allowing hierarchical images to be created. See 'Sections and hierarchical images' in the binman README for more information. diff --git a/tools/binman/bsection.py b/tools/binman/bsection.py deleted file mode 100644 index 082f424241c..00000000000 --- a/tools/binman/bsection.py +++ /dev/null @@ -1,523 +0,0 @@ -# SPDX-License-Identifier: GPL-2.0+ -# Copyright (c) 2018 Google, Inc -# Written by Simon Glass sjg@chromium.org -# -# Base class for sections (collections of entries) -# - -from __future__ import print_function - -from collections import OrderedDict -import sys - -from entry import Entry -import fdt_util -import re -import state -import tools - -class Section(object): - """A section which contains multiple entries - - A section represents a collection of entries. There must be one or more - sections in an image. Sections are used to group entries together. - - Attributes: - _node: Node object that contains the section definition in device tree - _parent_section: Parent Section object which created this Section - _size: Section size in bytes, or None if not known yet - _align_size: Section size alignment, or None - _pad_before: Number of bytes before the first entry starts. This - effectively changes the place where entry offset 0 starts - _pad_after: Number of bytes after the last entry ends. The last - entry will finish on or before this boundary - _pad_byte: Byte to use to pad the section where there is no entry - _sort: True if entries should be sorted by offset, False if they - must be in-order in the device tree description - _skip_at_start: Number of bytes before the first entry starts. These - effectively adjust the starting offset of entries. For example, - if _pad_before is 16, then the first entry would start at 16. - An entry with offset = 20 would in fact be written at offset 4 - in the image file. - _end_4gb: Indicates that the section ends at the 4GB boundary. This is - used for x86 images, which want to use offsets such that a memory - address (like 0xff800000) is the first entry offset. This causes - _skip_at_start to be set to the starting memory address. - _name_prefix: Prefix to add to the name of all entries within this - section - _entries: OrderedDict() of entries - _orig_offset: Original offset value read from node - _orig_size: Original size value read from node - """ - def __init__(self, name, parent_section, node, image, test=False): - global entry - global Entry - import entry - from entry import Entry - - self._parent_section = parent_section - self._name = name - self._node = node - self._image = image - self._offset = None - self._size = None - self._align_size = None - self._pad_before = 0 - self._pad_after = 0 - self._pad_byte = 0 - self._sort = False - self._skip_at_start = None - self._end_4gb = False - self._name_prefix = '' - self._entries = OrderedDict() - self._image_pos = None - if not test: - self._ReadNode() - self._ReadEntries() - - def _ReadNode(self): - """Read properties from the section node""" - self._offset = fdt_util.GetInt(self._node, 'offset') - self._size = fdt_util.GetInt(self._node, 'size') - self._orig_offset = self._offset - self._orig_size = self._size - self._align_size = fdt_util.GetInt(self._node, 'align-size') - if tools.NotPowerOfTwo(self._align_size): - self._Raise("Alignment size %s must be a power of two" % - self._align_size) - self._pad_before = fdt_util.GetInt(self._node, 'pad-before', 0) - self._pad_after = fdt_util.GetInt(self._node, 'pad-after', 0) - self._pad_byte = fdt_util.GetInt(self._node, 'pad-byte', 0) - self._sort = fdt_util.GetBool(self._node, 'sort-by-offset') - self._end_4gb = fdt_util.GetBool(self._node, 'end-at-4gb') - self._skip_at_start = fdt_util.GetInt(self._node, 'skip-at-start') - if self._end_4gb: - if not self._size: - self._Raise("Section size must be provided when using end-at-4gb") - if self._skip_at_start is not None: - self._Raise("Provide either 'end-at-4gb' or 'skip-at-start'") - else: - self._skip_at_start = 0x100000000 - self._size - else: - if self._skip_at_start is None: - self._skip_at_start = 0 - self._name_prefix = fdt_util.GetString(self._node, 'name-prefix') - - def _ReadEntries(self): - for node in self._node.subnodes: - if node.name == 'hash': - continue - entry = Entry.Create(self, node) - entry.SetPrefix(self._name_prefix) - self._entries[node.name] = entry - - def GetFdtSet(self): - """Get the set of device tree files used by this image""" - fdt_set = set() - for entry in self._entries.values(): - fdt_set.update(entry.GetFdtSet()) - return fdt_set - - def SetOffset(self, offset): - self._offset = offset - - def ExpandEntries(self): - for entry in self._entries.values(): - entry.ExpandEntries() - - def AddMissingProperties(self): - """Add new properties to the device tree as needed for this entry""" - for prop in ['offset', 'size', 'image-pos']: - if not prop in self._node.props: - state.AddZeroProp(self._node, prop) - state.CheckAddHashProp(self._node) - for entry in self._entries.values(): - entry.AddMissingProperties() - - def SetCalculatedProperties(self): - state.SetInt(self._node, 'offset', self._offset or 0) - state.SetInt(self._node, 'size', self._size) - image_pos = self._image_pos - if self._parent_section: - image_pos -= self._parent_section.GetRootSkipAtStart() - state.SetInt(self._node, 'image-pos', image_pos) - for entry in self._entries.values(): - entry.SetCalculatedProperties() - - def ProcessFdt(self, fdt): - todo = self._entries.values() - for passnum in range(3): - next_todo = [] - for entry in todo: - if not entry.ProcessFdt(fdt): - next_todo.append(entry) - todo = next_todo - if not todo: - break - if todo: - self._Raise('Internal error: Could not complete processing of Fdt: ' - 'remaining %s' % todo) - return True - - def CheckSize(self): - """Check that the section contents does not exceed its size, etc.""" - contents_size = 0 - for entry in self._entries.values(): - contents_size = max(contents_size, entry.offset + entry.size) - - contents_size -= self._skip_at_start - - size = self._size - if not size: - size = self._pad_before + contents_size + self._pad_after - size = tools.Align(size, self._align_size) - - if self._size and contents_size > self._size: - self._Raise("contents size %#x (%d) exceeds section size %#x (%d)" % - (contents_size, contents_size, self._size, self._size)) - if not self._size: - self._size = size - if self._size != tools.Align(self._size, self._align_size): - self._Raise("Size %#x (%d) does not match align-size %#x (%d)" % - (self._size, self._size, self._align_size, self._align_size)) - return size - - def _Raise(self, msg): - """Raises an error for this section - - Args: - msg: Error message to use in the raise string - Raises: - ValueError() - """ - raise ValueError("Section '%s': %s" % (self._node.path, msg)) - - def GetPath(self): - """Get the path of an image (in the FDT) - - Returns: - Full path of the node for this image - """ - return self._node.path - - def FindEntryType(self, etype): - """Find an entry type in the section - - Args: - etype: Entry type to find - Returns: - entry matching that type, or None if not found - """ - for entry in self._entries.values(): - if entry.etype == etype: - return entry - return None - - def GetEntryContents(self): - """Call ObtainContents() for each entry - - This calls each entry's ObtainContents() a few times until they all - return True. We stop calling an entry's function once it returns - True. This allows the contents of one entry to depend on another. - - After 3 rounds we give up since it's likely an error. - """ - todo = self._entries.values() - for passnum in range(3): - next_todo = [] - for entry in todo: - if not entry.ObtainContents(): - next_todo.append(entry) - todo = next_todo - if not todo: - break - if todo: - self._Raise('Internal error: Could not complete processing of ' - 'contents: remaining %s' % todo) - return True - - def _SetEntryOffsetSize(self, name, offset, size): - """Set the offset and size of an entry - - Args: - name: Entry name to update - offset: New offset, or None to leave alone - size: New size, or None to leave alone - """ - entry = self._entries.get(name) - if not entry: - self._Raise("Unable to set offset/size for unknown entry '%s'" % - name) - entry.SetOffsetSize(self._skip_at_start + offset if offset else None, - size) - - def GetEntryOffsets(self): - """Handle entries that want to set the offset/size of other entries - - This calls each entry's GetOffsets() method. If it returns a list - of entries to update, it updates them. - """ - for entry in self._entries.values(): - offset_dict = entry.GetOffsets() - for name, info in offset_dict.items(): - self._SetEntryOffsetSize(name, *info) - - def ResetForPack(self): - """Reset offset/size fields so that packing can be done again""" - self._offset = self._orig_offset - self._size = self._orig_size - for entry in self._entries.values(): - entry.ResetForPack() - - def PackEntries(self): - """Pack all entries into the section""" - offset = self._skip_at_start - for entry in self._entries.values(): - offset = entry.Pack(offset) - self._size = self.CheckSize() - - def _SortEntries(self): - """Sort entries by offset""" - entries = sorted(self._entries.values(), key=lambda entry: entry.offset) - self._entries.clear() - for entry in entries: - self._entries[entry._node.name] = entry - - def _ExpandEntries(self): - """Expand any entries that are permitted to""" - exp_entry = None - for entry in self._entries.values(): - if exp_entry: - exp_entry.ExpandToLimit(entry.offset) - exp_entry = None - if entry.expand_size: - exp_entry = entry - if exp_entry: - exp_entry.ExpandToLimit(self._size) - - def CheckEntries(self): - """Check that entries do not overlap or extend outside the section - - This also sorts entries, if needed and expands - """ - if self._sort: - self._SortEntries() - self._ExpandEntries() - offset = 0 - prev_name = 'None' - for entry in self._entries.values(): - entry.CheckOffset() - if (entry.offset < self._skip_at_start or - entry.offset + entry.size > self._skip_at_start + self._size): - entry.Raise("Offset %#x (%d) is outside the section starting " - "at %#x (%d)" % - (entry.offset, entry.offset, self._skip_at_start, - self._skip_at_start)) - if entry.offset < offset: - entry.Raise("Offset %#x (%d) overlaps with previous entry '%s' " - "ending at %#x (%d)" % - (entry.offset, entry.offset, prev_name, offset, offset)) - offset = entry.offset + entry.size - prev_name = entry.GetPath() - - def SetImagePos(self, image_pos): - self._image_pos = image_pos - for entry in self._entries.values(): - entry.SetImagePos(image_pos) - - def ProcessEntryContents(self): - """Call the ProcessContents() method for each entry - - This is intended to adjust the contents as needed by the entry type. - - Returns: - True if no entries needed to change their size - """ - sizes_ok = True - for entry in self._entries.values(): - if not entry.ProcessContents(): - sizes_ok = False - print("Entry '%s' size change" % self._node.path) - return sizes_ok - - def WriteSymbols(self): - """Write symbol values into binary files for access at run time""" - for entry in self._entries.values(): - entry.WriteSymbols(self) - - def BuildSection(self, fd, base_offset): - """Write the section to a file""" - fd.seek(base_offset) - fd.write(self.GetData()) - - def GetData(self): - """Get the contents of the section""" - section_data = tools.GetBytes(self._pad_byte, self._size) - - for entry in self._entries.values(): - data = entry.GetData() - base = self._pad_before + entry.offset - self._skip_at_start - section_data = (section_data[:base] + data + - section_data[base + len(data):]) - return section_data - - def LookupSymbol(self, sym_name, optional, msg): - """Look up a symbol in an ELF file - - Looks up a symbol in an ELF file. Only entry types which come from an - ELF image can be used by this function. - - At present the only entry property supported is offset. - - Args: - sym_name: Symbol name in the ELF file 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 - _any to <entry> to have it search for any matching entry. E.g. - _binman_u_boot_any_prop_offset will match entries called u-boot, - u-boot-img and u-boot-nodtb) - 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 - - Returns: - Value that should be assigned to that symbol, or None if it was - optional and not found - - Raises: - ValueError if the symbol is invalid or not found, or references a - property which is not supported - """ - m = re.match(r'^_binman_(\w+)_prop_(\w+)$', sym_name) - if not m: - raise ValueError("%s: Symbol '%s' has invalid format" % - (msg, sym_name)) - entry_name, prop_name = m.groups() - entry_name = entry_name.replace('_', '-') - entry = self._entries.get(entry_name) - if not entry: - if entry_name.endswith('-any'): - root = entry_name[:-4] - for name in self._entries: - if name.startswith(root): - rest = name[len(root):] - if rest in ['', '-img', '-nodtb']: - entry = self._entries[name] - if not entry: - err = ("%s: Entry '%s' not found in list (%s)" % - (msg, entry_name, ','.join(self._entries.keys()))) - if optional: - print('Warning: %s' % err, file=sys.stderr) - return None - raise ValueError(err) - if prop_name == 'offset': - return entry.offset - elif prop_name == 'image_pos': - return entry.image_pos - else: - raise ValueError("%s: No such property '%s'" % (msg, prop_name)) - - def GetEntries(self): - """Get the dict of entries in a section - - Returns: - OrderedDict of entries in a section - """ - return self._entries - - def GetSize(self): - """Get the size of a section in bytes - - This is only meaningful if the section has a pre-defined size, or the - entries within it have been packed, so that the size has been - calculated. - - Returns: - Entry size in bytes - """ - return self._size - - def WriteMap(self, fd, indent): - """Write a map of the section to a .map file - - Args: - fd: File to write the map to - """ - Entry.WriteMapLine(fd, indent, self._name, self._offset or 0, - self._size, self._image_pos) - for entry in self._entries.values(): - entry.WriteMap(fd, indent + 1) - - def GetContentsByPhandle(self, phandle, source_entry): - """Get the data contents of an entry specified by a phandle - - This uses a phandle to look up a node and and find the entry - associated with it. Then it returnst he contents of that entry. - - Args: - phandle: Phandle to look up (integer) - source_entry: Entry containing that phandle (used for error - reporting) - - Returns: - data from associated entry (as a string), or None if not found - """ - node = self._node.GetFdt().LookupPhandle(phandle) - if not node: - source_entry.Raise("Cannot find node for phandle %d" % phandle) - for entry in self._entries.values(): - if entry._node == node: - return entry.GetData() - source_entry.Raise("Cannot find entry for node '%s'" % node.name) - - def ExpandSize(self, size): - """Change the size of an entry - - Args: - size: New size for entry - """ - if size != self._size: - self._size = size - - def GetRootSkipAtStart(self): - """Get the skip-at-start value for the top-level section - - This is used to find out the starting offset for root section that - contains this section. If this is a top-level section then it returns - the skip-at-start offset for this section. - - This is used to get the absolute position of section within the image. - - Returns: - Integer skip-at-start value for the root section containing this - section - """ - if self._parent_section: - return self._parent_section.GetRootSkipAtStart() - return self._skip_at_start - - def GetStartOffset(self): - """Get the start offset for this section - - Returns: - The first available offset in this section (typically 0) - """ - return self._skip_at_start - - def GetImageSize(self): - """Get the size of the image containing this section - - Returns: - Image size as an integer number of bytes, which may be None if the - image size is dynamic and its sections have not yet been packed - """ - return self._image._size - - def ListEntries(self, entries, indent): - """Override this method to list all files in the section""" - Entry.AddEntryInfo(entries, indent, self._name, 'section', self._size, - self._image_pos, None, self._offset, - self._parent_section) - for entry in self._entries.values(): - entry.ListEntries(entries, indent + 1) diff --git a/tools/binman/entry.py b/tools/binman/entry.py index 6c74f2a2175..c45a2fdb4b0 100644 --- a/tools/binman/entry.py +++ b/tools/binman/entry.py @@ -175,8 +175,8 @@ class Entry(object): self.pad_after = fdt_util.GetInt(self._node, 'pad-after', 0) self.align_size = fdt_util.GetInt(self._node, 'align-size') if tools.NotPowerOfTwo(self.align_size): - raise ValueError("Node '%s': Alignment size %s must be a power " - "of two" % (self._node.path, self.align_size)) + self.Raise("Alignment size %s must be a power of two" % + self.align_size) self.align_end = fdt_util.GetInt(self._node, 'align-end') self.offset_unset = fdt_util.GetBool(self._node, 'offset-unset') self.expand_size = fdt_util.GetBool(self._node, 'expand-size') @@ -216,8 +216,8 @@ class Entry(object): """Set the value of device-tree properties calculated by binman""" state.SetInt(self._node, 'offset', self.offset) state.SetInt(self._node, 'size', self.size) - state.SetInt(self._node, 'image-pos', - self.image_pos - self.section.GetRootSkipAtStart()) + base = self.section.GetRootSkipAtStart() if self.section else 0 + state.SetInt(self._node, 'image-pos', self.image_pos - base) if self.uncomp_size is not None: state.SetInt(self._node, 'uncomp-size', self.uncomp_size) state.CheckSetHashValue(self._node, self.GetData) diff --git a/tools/binman/etype/files.py b/tools/binman/etype/files.py index 99f2f2f67fe..0068b305f75 100644 --- a/tools/binman/etype/files.py +++ b/tools/binman/etype/files.py @@ -14,7 +14,6 @@ import fdt_util import state import tools
-import bsection
class Entry_files(Entry_section): """Entry containing a set of files @@ -54,4 +53,4 @@ class Entry_files(Entry_section): state.AddString(subnode, 'compress', self._compress)
# Read entries again, now that we have some - self._section._ReadEntries() + self._ReadEntries() diff --git a/tools/binman/etype/fmap.py b/tools/binman/etype/fmap.py index 3a809486098..f8d8d866f13 100644 --- a/tools/binman/etype/fmap.py +++ b/tools/binman/etype/fmap.py @@ -49,7 +49,7 @@ class Entry_fmap(Entry): areas.append(fmap_util.FmapArea(pos or 0, entry.size or 0, tools.FromUnicode(entry.name), 0))
- entries = self.section._image.GetEntries() + entries = self.section.image.GetEntries() areas = [] for entry in entries.values(): _AddEntries(areas, entry) diff --git a/tools/binman/etype/section.py b/tools/binman/etype/section.py index 178e89352e5..6db3c7a6f03 100644 --- a/tools/binman/etype/section.py +++ b/tools/binman/etype/section.py @@ -1,59 +1,155 @@ # SPDX-License-Identifier: GPL-2.0+ # Copyright (c) 2018 Google, Inc # Written by Simon Glass sjg@chromium.org -# -# Entry-type module for sections, which are entries which can contain other -# entries. -# + +"""Entry-type module for sections (groups of entries) + +Sections are entries which can contain other entries. This allows hierarchical +images to be created. +""" + +from __future__ import print_function + +from collections import OrderedDict +import re +import sys
from entry import Entry import fdt_util import tools
-import bsection
class Entry_section(Entry): """Entry that contains other entries
Properties / Entry arguments: (see binman README for more information) - - size: Size of section in bytes - - align-size: Align size to a particular power of two - - pad-before: Add padding before the entry - - pad-after: Add padding after the entry - - pad-byte: Pad byte to use when padding - - sort-by-offset: Reorder the entries by offset - - end-at-4gb: Used to build an x86 ROM which ends at 4GB (2^32) - - name-prefix: Adds a prefix to the name of every entry in the section + pad-byte: Pad byte to use when padding + sort-by-offset: True if entries should be sorted by offset, False if + they must be in-order in the device tree description + end-at-4gb: Used to build an x86 ROM which ends at 4GB (2^32) + skip-at-start: Number of bytes before the first entry starts. These + effectively adjust the starting offset of entries. For example, + if this is 16, then the first entry would start at 16. An entry + with offset = 20 would in fact be written at offset 4 in the image + file, since the first 16 bytes are skipped when writing. + name-prefix: Adds a prefix to the name of every entry in the section when writing out the map
+ Since a section is also an entry, it inherits all the properies of entries + too. + A section is an entry which can contain other entries, thus allowing hierarchical images to be created. See 'Sections and hierarchical images' in the binman README for more information. """ - def __init__(self, section, etype, node): - Entry.__init__(self, section, etype, node) - self._section = bsection.Section(node.name, section, node, - section._image) + def __init__(self, section, etype, node, test=False): + if not test: + Entry.__init__(self, section, etype, node) + if section: + self.image = section.image + self._entries = OrderedDict() + self._pad_byte = 0 + self._sort = False + self._skip_at_start = None + self._end_4gb = False + if not test: + self._ReadNode() + self._ReadEntries() + + def _Raise(self, msg): + """Raises an error for this section + + Args: + msg: Error message to use in the raise string + Raises: + ValueError() + """ + raise ValueError("Section '%s': %s" % (self._node.path, msg)) + + def _ReadNode(self): + """Read properties from the image node""" + self._pad_byte = fdt_util.GetInt(self._node, 'pad-byte', 0) + self._sort = fdt_util.GetBool(self._node, 'sort-by-offset') + self._end_4gb = fdt_util.GetBool(self._node, 'end-at-4gb') + self._skip_at_start = fdt_util.GetInt(self._node, 'skip-at-start') + if self._end_4gb: + if not self.size: + self.Raise("Section size must be provided when using end-at-4gb") + if self._skip_at_start is not None: + self.Raise("Provide either 'end-at-4gb' or 'skip-at-start'") + else: + self._skip_at_start = 0x100000000 - self.size + else: + if self._skip_at_start is None: + self._skip_at_start = 0 + self._name_prefix = fdt_util.GetString(self._node, 'name-prefix') + filename = fdt_util.GetString(self._node, 'filename') + if filename: + self._filename = filename + + def _ReadEntries(self): + for node in self._node.subnodes: + if node.name == 'hash': + continue + entry = Entry.Create(self, node) + entry.SetPrefix(self._name_prefix) + self._entries[node.name] = entry
def GetFdtSet(self): - return self._section.GetFdtSet() + fdt_set = set() + for entry in self._entries.values(): + fdt_set.update(entry.GetFdtSet()) + return fdt_set
def ProcessFdt(self, fdt): - return self._section.ProcessFdt(fdt) + """Allow entries to adjust the device tree + + Some entries need to adjust the device tree for their purposes. This + may involve adding or deleting properties. + """ + todo = self._entries.values() + for passnum in range(3): + next_todo = [] + for entry in todo: + if not entry.ProcessFdt(fdt): + next_todo.append(entry) + todo = next_todo + if not todo: + break + if todo: + self.Raise('Internal error: Could not complete processing of Fdt: remaining %s' % + todo) + return True
def ExpandEntries(self): + """Expand out any entries which have calculated sub-entries + + Some entries are expanded out at runtime, e.g. 'files', which produces + a section containing a list of files. Process these entries so that + this information is added to the device tree. + """ Entry.ExpandEntries(self) - self._section.ExpandEntries() + for entry in self._entries.values(): + entry.ExpandEntries()
def AddMissingProperties(self): + """Add new properties to the device tree as needed for this entry""" Entry.AddMissingProperties(self) - self._section.AddMissingProperties() + for entry in self._entries.values(): + entry.AddMissingProperties()
def ObtainContents(self): - return self._section.GetEntryContents() + return self.GetEntryContents()
def GetData(self): - return self._section.GetData() + section_data = tools.GetBytes(self._pad_byte, self.size) + + for entry in self._entries.values(): + data = entry.GetData() + base = self.pad_before + entry.offset - self._skip_at_start + section_data = (section_data[:base] + data + + section_data[base + len(data):]) + return section_data
def GetOffsets(self): """Handle entries that want to set the offset/size of other entries @@ -61,41 +157,94 @@ class Entry_section(Entry): This calls each entry's GetOffsets() method. If it returns a list of entries to update, it updates them. """ - self._section.GetEntryOffsets() + self.GetEntryOffsets() return {}
def ResetForPack(self): """Reset offset/size fields so that packing can be done again""" - self._section.ResetForPack() Entry.ResetForPack(self) + for entry in self._entries.values(): + entry.ResetForPack()
def Pack(self, offset): """Pack all entries into the section""" - self._section.PackEntries() - if self._section._offset is None: - self._section.SetOffset(offset) - self.size = self._section.GetSize() - return super(Entry_section, self).Pack(offset) + self._PackEntries() + return Entry.Pack(self, offset)
- def SetImagePos(self, image_pos): - Entry.SetImagePos(self, image_pos) - self._section.SetImagePos(image_pos + self.offset) + def _PackEntries(self): + """Pack all entries into the image""" + offset = self._skip_at_start + for entry in self._entries.values(): + offset = entry.Pack(offset) + self.size = self.CheckSize() + + def _ExpandEntries(self): + """Expand any entries that are permitted to""" + exp_entry = None + for entry in self._entries.values(): + if exp_entry: + exp_entry.ExpandToLimit(entry.offset) + exp_entry = None + if entry.expand_size: + exp_entry = entry + if exp_entry: + exp_entry.ExpandToLimit(self.size) + + def _SortEntries(self): + """Sort entries by offset""" + entries = sorted(self._entries.values(), key=lambda entry: entry.offset) + self._entries.clear() + for entry in entries: + self._entries[entry._node.name] = entry + + def CheckEntries(self): + """Check that entries do not overlap or extend outside the image""" + if self._sort: + self._SortEntries() + self._ExpandEntries() + offset = 0 + prev_name = 'None' + for entry in self._entries.values(): + entry.CheckOffset() + if (entry.offset < self._skip_at_start or + entry.offset + entry.size > self._skip_at_start + + self.size): + entry.Raise("Offset %#x (%d) is outside the section starting " + "at %#x (%d)" % + (entry.offset, entry.offset, self._skip_at_start, + self._skip_at_start)) + if entry.offset < offset: + entry.Raise("Offset %#x (%d) overlaps with previous entry '%s' " + "ending at %#x (%d)" % + (entry.offset, entry.offset, prev_name, offset, offset)) + offset = entry.offset + entry.size + prev_name = entry.GetPath()
def WriteSymbols(self, section): """Write symbol values into binary files for access at run time""" - self._section.WriteSymbols() + for entry in self._entries.values(): + entry.WriteSymbols(self)
def SetCalculatedProperties(self): Entry.SetCalculatedProperties(self) - self._section.SetCalculatedProperties() + for entry in self._entries.values(): + entry.SetCalculatedProperties() + + def SetImagePos(self, image_pos): + Entry.SetImagePos(self, image_pos) + for entry in self._entries.values(): + entry.SetImagePos(image_pos + self.offset)
def ProcessContents(self): - sizes_ok = self._section.ProcessEntryContents() sizes_ok_base = super(Entry_section, self).ProcessContents() + sizes_ok = True + for entry in self._entries.values(): + if not entry.ProcessContents(): + sizes_ok = False return sizes_ok and sizes_ok_base
def CheckOffset(self): - self._section.CheckEntries() + self.CheckEntries()
def WriteMap(self, fd, indent): """Write a map of the section to a .map file @@ -103,15 +252,211 @@ class Entry_section(Entry): Args: fd: File to write the map to """ - self._section.WriteMap(fd, indent) + Entry.WriteMapLine(fd, indent, self.name, self.offset or 0, + self.size, self.image_pos) + for entry in self._entries.values(): + entry.WriteMap(fd, indent + 1)
def GetEntries(self): - return self._section.GetEntries() + return self._entries + + def GetContentsByPhandle(self, phandle, source_entry): + """Get the data contents of an entry specified by a phandle + + This uses a phandle to look up a node and and find the entry + associated with it. Then it returnst he contents of that entry. + + Args: + phandle: Phandle to look up (integer) + source_entry: Entry containing that phandle (used for error + reporting) + + Returns: + data from associated entry (as a string), or None if not found + """ + node = self._node.GetFdt().LookupPhandle(phandle) + if not node: + source_entry.Raise("Cannot find node for phandle %d" % phandle) + for entry in self._entries.values(): + if entry._node == node: + return entry.GetData() + source_entry.Raise("Cannot find entry for node '%s'" % node.name) + + def LookupSymbol(self, sym_name, optional, msg): + """Look up a symbol in an ELF file + + Looks up a symbol in an ELF file. Only entry types which come from an + ELF image can be used by this function. + + At present the only entry property supported is offset. + + Args: + sym_name: Symbol name in the ELF file 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 + _any to <entry> to have it search for any matching entry. E.g. + _binman_u_boot_any_prop_offset will match entries called u-boot, + u-boot-img and u-boot-nodtb) + 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 + + Returns: + Value that should be assigned to that symbol, or None if it was + optional and not found + + Raises: + ValueError if the symbol is invalid or not found, or references a + property which is not supported + """ + m = re.match(r'^_binman_(\w+)_prop_(\w+)$', sym_name) + if not m: + raise ValueError("%s: Symbol '%s' has invalid format" % + (msg, sym_name)) + entry_name, prop_name = m.groups() + entry_name = entry_name.replace('_', '-') + entry = self._entries.get(entry_name) + if not entry: + if entry_name.endswith('-any'): + root = entry_name[:-4] + for name in self._entries: + if name.startswith(root): + rest = name[len(root):] + if rest in ['', '-img', '-nodtb']: + entry = self._entries[name] + if not entry: + err = ("%s: Entry '%s' not found in list (%s)" % + (msg, entry_name, ','.join(self._entries.keys()))) + if optional: + print('Warning: %s' % err, file=sys.stderr) + return None + raise ValueError(err) + if prop_name == 'offset': + return entry.offset + elif prop_name == 'image_pos': + return entry.image_pos + else: + raise ValueError("%s: No such property '%s'" % (msg, prop_name)) + + def GetRootSkipAtStart(self): + """Get the skip-at-start value for the top-level section + + This is used to find out the starting offset for root section that + contains this section. If this is a top-level section then it returns + the skip-at-start offset for this section. + + This is used to get the absolute position of section within the image. + + Returns: + Integer skip-at-start value for the root section containing this + section + """ + if self.section: + return self.section.GetRootSkipAtStart() + return self._skip_at_start + + def GetStartOffset(self): + """Get the start offset for this section + + Returns: + The first available offset in this section (typically 0) + """ + return self._skip_at_start + + def GetImageSize(self): + """Get the size of the image containing this section + + Returns: + Image size as an integer number of bytes, which may be None if the + image size is dynamic and its sections have not yet been packed + """ + return self.image.size + + def FindEntryType(self, etype): + """Find an entry type in the section + + Args: + etype: Entry type to find + Returns: + entry matching that type, or None if not found + """ + for entry in self._entries.values(): + if entry.etype == etype: + return entry + return None + + def GetEntryContents(self): + """Call ObtainContents() for the section + """ + todo = self._entries.values() + for passnum in range(3): + next_todo = [] + for entry in todo: + if not entry.ObtainContents(): + next_todo.append(entry) + todo = next_todo + if not todo: + break + if todo: + self.Raise('Internal error: Could not complete processing of contents: remaining %s' % + todo) + return True + + def _SetEntryOffsetSize(self, name, offset, size): + """Set the offset and size of an entry + + Args: + name: Entry name to update + offset: New offset, or None to leave alone + size: New size, or None to leave alone + """ + entry = self._entries.get(name) + if not entry: + self._Raise("Unable to set offset/size for unknown entry '%s'" % + name) + entry.SetOffsetSize(self._skip_at_start + offset if offset else None, + size) + + def GetEntryOffsets(self): + """Handle entries that want to set the offset/size of other entries + + This calls each entry's GetOffsets() method. If it returns a list + of entries to update, it updates them. + """ + for entry in self._entries.values(): + offset_dict = entry.GetOffsets() + for name, info in offset_dict.items(): + self._SetEntryOffsetSize(name, *info) + + + def CheckSize(self): + """Check that the image contents does not exceed its size, etc.""" + contents_size = 0 + for entry in self._entries.values(): + contents_size = max(contents_size, entry.offset + entry.size) + + contents_size -= self._skip_at_start + + size = self.size + if not size: + size = self.pad_before + contents_size + self.pad_after + size = tools.Align(size, self.align_size)
- def ExpandToLimit(self, limit): - super(Entry_section, self).ExpandToLimit(limit) - self._section.ExpandSize(self.size) + if self.size and contents_size > self.size: + self._Raise("contents size %#x (%d) exceeds section size %#x (%d)" % + (contents_size, contents_size, self.size, self.size)) + if not self.size: + self.size = size + if self.size != tools.Align(self.size, self.align_size): + self._Raise("Size %#x (%d) does not match align-size %#x (%d)" % + (self.size, self.size, self.align_size, + self.align_size)) + return size
def ListEntries(self, entries, indent): """List the files in the section""" - self._section.ListEntries(entries, indent) + Entry.AddEntryInfo(entries, indent, self.name, 'section', self.size, + self.image_pos, None, self.offset, self) + for entry in self._entries.values(): + entry.ListEntries(entries, indent + 1) diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index f3a8e64ad13..4f58ce3c859 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -595,7 +595,7 @@ class TestFunctional(unittest.TestCase): self.assertEqual(0, retcode)
image = control.images['image1'] - self.assertEqual(len(U_BOOT_DATA), image._size) + self.assertEqual(len(U_BOOT_DATA), image.size) fname = tools.GetOutputFilename('image1.bin') self.assertTrue(os.path.exists(fname)) with open(fname, 'rb') as fd: @@ -603,7 +603,7 @@ class TestFunctional(unittest.TestCase): self.assertEqual(U_BOOT_DATA, data)
image = control.images['image2'] - self.assertEqual(3 + len(U_BOOT_DATA) + 5, image._size) + self.assertEqual(3 + len(U_BOOT_DATA) + 5, image.size) fname = tools.GetOutputFilename('image2.bin') self.assertTrue(os.path.exists(fname)) with open(fname, 'rb') as fd: @@ -659,7 +659,7 @@ class TestFunctional(unittest.TestCase): self.assertEqual(61, entry.offset) self.assertEqual(len(U_BOOT_DATA), entry.size)
- self.assertEqual(65, image._size) + self.assertEqual(65, image.size)
def testPackExtra(self): """Test that extra packing feature works as expected""" @@ -703,7 +703,7 @@ class TestFunctional(unittest.TestCase): self.assertEqual(64, entry.size)
self.CheckNoGaps(entries) - self.assertEqual(128, image._size) + self.assertEqual(128, image.size)
def testPackAlignPowerOf2(self): """Test that invalid entry alignment is detected""" @@ -761,7 +761,7 @@ class TestFunctional(unittest.TestCase): self.assertEqual(0, retcode) self.assertIn('image', control.images) image = control.images['image'] - self.assertEqual(7, image._size) + self.assertEqual(7, image.size)
def testPackImageSizeAlign(self): """Test that image size alignemnt works as expected""" @@ -769,7 +769,7 @@ class TestFunctional(unittest.TestCase): self.assertEqual(0, retcode) self.assertIn('image', control.images) image = control.images['image'] - self.assertEqual(16, image._size) + self.assertEqual(16, image.size)
def testPackInvalidImageAlign(self): """Test that invalid image alignment is detected""" @@ -782,7 +782,7 @@ class TestFunctional(unittest.TestCase): """Test that invalid image alignment is detected""" with self.assertRaises(ValueError) as e: self._DoTestFile('020_pack_inv_image_align_power2.dts') - self.assertIn("Section '/binman': Alignment size 131 must be a power of " + self.assertIn("Image '/binman': Alignment size 131 must be a power of " "two", str(e.exception))
def testImagePadByte(self): @@ -833,7 +833,7 @@ class TestFunctional(unittest.TestCase): """Test that the end-at-4gb property requires a size property""" with self.assertRaises(ValueError) as e: self._DoTestFile('027_pack_4gb_no_size.dts') - self.assertIn("Section '/binman': Section size must be provided when " + self.assertIn("Image '/binman': Section size must be provided when " "using end-at-4gb", str(e.exception))
def test4gbAndSkipAtStartTogether(self): @@ -841,7 +841,7 @@ class TestFunctional(unittest.TestCase): together""" with self.assertRaises(ValueError) as e: self._DoTestFile('80_4gb_and_skip_at_start_together.dts') - self.assertIn("Section '/binman': Provide either 'end-at-4gb' or " + self.assertIn("Image '/binman': Provide either 'end-at-4gb' or " "'skip-at-start'", str(e.exception))
def testPackX86RomOutside(self): @@ -1217,7 +1217,7 @@ class TestFunctional(unittest.TestCase): """Test that obtaining the contents works as expected""" with self.assertRaises(ValueError) as e: self._DoReadFile('057_unknown_contents.dts', True) - self.assertIn("Section '/binman': Internal error: Could not complete " + self.assertIn("Image '/binman': Internal error: Could not complete " "processing of contents: remaining [<_testing.Entry__testing ", str(e.exception))
@@ -1657,7 +1657,7 @@ class TestFunctional(unittest.TestCase): image = control.images['image'] entries = image.GetEntries() files = entries['files'] - entries = files._section._entries + entries = files._entries
orig = b'' for i in range(1, 3): @@ -2209,7 +2209,7 @@ class TestFunctional(unittest.TestCase): self.assertEqual(len(data), ent.size) self.assertEqual(0, ent.image_pos) self.assertEqual(None, ent.uncomp_size) - self.assertEqual(None, ent.offset) + self.assertEqual(0, ent.offset)
ent = entries[1] self.assertEqual(1, ent.indent) @@ -2227,7 +2227,7 @@ class TestFunctional(unittest.TestCase): section_size = ent.size self.assertEqual(0x100, ent.image_pos) self.assertEqual(None, ent.uncomp_size) - self.assertEqual(len(U_BOOT_DATA), ent.offset) + self.assertEqual(0x100, ent.offset)
ent = entries[3] self.assertEqual(2, ent.indent) @@ -2331,7 +2331,7 @@ class TestFunctional(unittest.TestCase): image_fname = tools.GetOutputFilename('image.bin') image = Image.FromFile(image_fname) self.assertTrue(isinstance(image, Image)) - self.assertEqual('image', image._name) + self.assertEqual('image', image.image_name)
def testReadImageFail(self): """Test failing to read an image image's FDT map""" @@ -2341,5 +2341,6 @@ class TestFunctional(unittest.TestCase): image = Image.FromFile(image_fname) self.assertIn("Cannot find FDT map in image", str(e.exception))
+ if __name__ == "__main__": unittest.main() diff --git a/tools/binman/image.py b/tools/binman/image.py index f890350a8d0..487290b6c4e 100644 --- a/tools/binman/image.py +++ b/tools/binman/image.py @@ -12,14 +12,15 @@ from operator import attrgetter import re import sys
+from entry import Entry from etype import fdtmap from etype import image_header +from etype import section import fdt import fdt_util -import bsection import tools
-class Image: +class Image(section.Entry_section): """A Image, representing an output from binman
An image is comprised of a collection of entries each containing binary @@ -27,12 +28,8 @@ class Image:
This class implements the various operations needed for images.
- Atrtributes: - _node: Node object that contains the image definition in device tree - _name: Image name - _size: Image size in bytes, or None if not known yet - _filename: Output filename for image - _sections: Sections present in this image (may be one or more) + Attributes: + filename: Output filename for image
Args: test: True if this is being called from a test of Images. This this case @@ -40,15 +37,15 @@ class Image: we create a section manually. """ def __init__(self, name, node, test=False): - self._node = node - self._name = name - self._size = None - self._filename = '%s.bin' % self._name - if test: - self._section = bsection.Section('main-section', None, self._node, - self, True) - else: - self._ReadNode() + self.image = self + section.Entry_section.__init__(self, None, 'section', node, test) + self.name = 'main-section' + self.image_name = name + self._filename = '%s.bin' % self.image_name + if not test: + filename = fdt_util.GetString(self._node, 'filename') + if filename: + self._filename = filename
@classmethod def FromFile(cls, fname): @@ -85,84 +82,17 @@ class Image: # Return an Image with the associated nodes return Image('image', dtb.GetRoot())
- def _ReadNode(self): - """Read properties from the image node""" - self._size = fdt_util.GetInt(self._node, 'size') - filename = fdt_util.GetString(self._node, 'filename') - if filename: - self._filename = filename - self._section = bsection.Section('main-section', None, self._node, self) - def Raise(self, msg): """Convenience function to raise an error referencing an image""" raise ValueError("Image '%s': %s" % (self._node.path, msg))
- def GetFdtSet(self): - """Get the set of device tree files used by this image""" - return self._section.GetFdtSet() - - def ExpandEntries(self): - """Expand out any entries which have calculated sub-entries - - Some entries are expanded out at runtime, e.g. 'files', which produces - a section containing a list of files. Process these entries so that - this information is added to the device tree. - """ - self._section.ExpandEntries() - - def AddMissingProperties(self): - """Add properties that are not present in the device tree - - When binman has completed packing the entries the offset and size of - each entry are known. But before this the device tree may not specify - these. Add any missing properties, with a dummy value, so that the - size of the entry is correct. That way we can insert the correct values - later. - """ - self._section.AddMissingProperties() - - def ProcessFdt(self, fdt): - """Allow entries to adjust the device tree - - Some entries need to adjust the device tree for their purposes. This - may involve adding or deleting properties. - """ - return self._section.ProcessFdt(fdt) - - def GetEntryContents(self): - """Call ObtainContents() for the section - """ - self._section.GetEntryContents() - - def GetEntryOffsets(self): - """Handle entries that want to set the offset/size of other entries - - This calls each entry's GetOffsets() method. If it returns a list - of entries to update, it updates them. - """ - self._section.GetEntryOffsets() - - def ResetForPack(self): - """Reset offset/size fields so that packing can be done again""" - self._section.ResetForPack() - def PackEntries(self): """Pack all entries into the image""" - self._section.PackEntries() - - def CheckSize(self): - """Check that the image contents does not exceed its size, etc.""" - self._size = self._section.CheckSize() - - def CheckEntries(self): - """Check that entries do not overlap or extend outside the image""" - self._section.CheckEntries() - - def SetCalculatedProperties(self): - self._section.SetCalculatedProperties() + section.Entry_section.Pack(self, 0)
def SetImagePos(self): - self._section.SetImagePos(0) + # This first section in the image so it starts at 0 + section.Entry_section.SetImagePos(self, 0)
def ProcessEntryContents(self): """Call the ProcessContents() method for each entry @@ -172,20 +102,27 @@ class Image: Returns: True if the new data size is OK, False if expansion is needed """ - return self._section.ProcessEntryContents() + sizes_ok = True + for entry in self._entries.values(): + if not entry.ProcessContents(): + sizes_ok = False + print("Entry '%s' size change" % self._node.path) + return sizes_ok
def WriteSymbols(self): """Write symbol values into binary files for access at run time""" - self._section.WriteSymbols() + section.Entry_section.WriteSymbols(self, self) + + def BuildSection(self, fd, base_offset): + """Write the section to a file""" + fd.seek(base_offset) + fd.write(self.GetData())
def BuildImage(self): """Write the image to a file""" fname = tools.GetOutputFilename(self._filename) with open(fname, 'wb') as fd: - self._section.BuildSection(fd, 0) - - def GetEntries(self): - return self._section.GetEntries() + self.BuildSection(fd, 0)
def WriteMap(self): """Write a map of the image to a .map file @@ -193,12 +130,12 @@ class Image: Returns: Filename of map file written """ - filename = '%s.map' % self._name + filename = '%s.map' % self.image_name fname = tools.GetOutputFilename(filename) with open(fname, 'w') as fd: print('%8s %8s %8s %s' % ('ImagePos', 'Offset', 'Size', 'Name'), file=fd) - self._section.WriteMap(fd, 0) + section.Entry_section.WriteMap(self, fd, 0) return fname
def BuildEntryList(self): @@ -208,5 +145,5 @@ class Image: List of entry.EntryInfo objects describing all entries in the image """ entries = [] - self._section.ListEntries(entries, 0) + self.ListEntries(entries, 0) return entries diff --git a/tools/binman/image_test.py b/tools/binman/image_test.py index 3775e1afb07..4004f789b7c 100644 --- a/tools/binman/image_test.py +++ b/tools/binman/image_test.py @@ -12,28 +12,25 @@ from test_util import capture_sys_output class TestImage(unittest.TestCase): def testInvalidFormat(self): image = Image('name', 'node', test=True) - section = image._section with self.assertRaises(ValueError) as e: - section.LookupSymbol('_binman_something_prop_', False, 'msg') + image.LookupSymbol('_binman_something_prop_', False, 'msg') self.assertIn( "msg: Symbol '_binman_something_prop_' has invalid format", str(e.exception))
def testMissingSymbol(self): image = Image('name', 'node', test=True) - section = image._section - section._entries = {} + image._entries = {} with self.assertRaises(ValueError) as e: - section.LookupSymbol('_binman_type_prop_pname', False, 'msg') + image.LookupSymbol('_binman_type_prop_pname', False, 'msg') self.assertIn("msg: Entry 'type' not found in list ()", str(e.exception))
def testMissingSymbolOptional(self): image = Image('name', 'node', test=True) - section = image._section - section._entries = {} + image._entries = {} with capture_sys_output() as (stdout, stderr): - val = section.LookupSymbol('_binman_type_prop_pname', True, 'msg') + val = image.LookupSymbol('_binman_type_prop_pname', True, 'msg') self.assertEqual(val, None) self.assertEqual("Warning: msg: Entry 'type' not found in list ()\n", stderr.getvalue()) @@ -41,8 +38,7 @@ class TestImage(unittest.TestCase):
def testBadProperty(self): image = Image('name', 'node', test=True) - section = image._section - section._entries = {'u-boot': 1} + image._entries = {'u-boot': 1} with self.assertRaises(ValueError) as e: - section.LookupSymbol('_binman_u_boot_prop_bad', False, 'msg') + image.LookupSymbol('_binman_u_boot_prop_bad', False, 'msg') self.assertIn("msg: No such property 'bad", str(e.exception))