The current test for the 'entry' module is a bit convoluted since it has to import the module multiple times. It also relies on ordering, in that test1EntryNoImportLib() must run before test2EntryImportLib() if they are running in the same Python process.

This is unreliable since neither the ordering of tests nor the process that they run in is defined.

Fix this by always reloading the entry in these two tests. Also add a check that the expected value of have_importlib is obtained.

This corrects a code-coverage problem in the 'entry' module on some systems.

Signed-off-by: Simon Glass sjg@chromium.org ---

tools/binman/entry_test.py | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-)

diff --git a/tools/binman/entry_test.py b/tools/binman/entry_test.py index b30a7beecc8..b6ad3edb8dc 100644 --- a/tools/binman/entry_test.py +++ b/tools/binman/entry_test.py @@ -9,12 +9,11 @@ import os import sys import unittest

+import entry import fdt import fdt_util import tools

-entry = None - class TestEntry(unittest.TestCase): def setUp(self): tools.PrepareOutputDir(None) @@ -29,16 +28,7 @@ class TestEntry(unittest.TestCase): dtb = fdt.FdtScan(fname) return dtb.GetNode('/binman/u-boot')

- def test1EntryNoImportLib(self): - """Test that we can import Entry subclassess successfully""" - - sys.modules['importlib'] = None - global entry - import entry - entry.Entry.Create(None, self.GetNode(), 'u-boot') - - def test2EntryImportLib(self): - del sys.modules['importlib'] + def _ReloadEntry(self): global entry if entry: if sys.version_info[0] >= 3: @@ -48,8 +38,21 @@ class TestEntry(unittest.TestCase): reload(entry) else: import entry + + def test1EntryNoImportLib(self): + """Test that we can import Entry subclassess successfully""" + sys.modules['importlib'] = None + global entry + self._ReloadEntry() + entry.Entry.Create(None, self.GetNode(), 'u-boot') + self.assertFalse(entry.have_importlib) + + def test2EntryImportLib(self): + del sys.modules['importlib'] + global entry + self._ReloadEntry() entry.Entry.Create(None, self.GetNode(), 'u-boot-spl') - del entry + self.assertTrue(entry.have_importlib)

def testEntryContents(self): """Test the Entry bass class""" @@ -59,7 +62,6 @@ class TestEntry(unittest.TestCase):

def testUnknownEntry(self): """Test that unknown entry types are detected""" - import entry Node = collections.namedtuple('Node', ['name', 'path']) node = Node('invalid-name', 'invalid-path') with self.assertRaises(ValueError) as e: @@ -69,7 +71,6 @@ class TestEntry(unittest.TestCase):

def testUniqueName(self): """Test Entry.GetUniqueName""" - import entry Node = collections.namedtuple('Node', ['name', 'parent']) base_node = Node('root', None) base_entry = entry.Entry(None, None, base_node, read_node=False) @@ -80,7 +81,6 @@ class TestEntry(unittest.TestCase):

def testGetDefaultFilename(self): """Trivial test for this base class function""" - import entry base_entry = entry.Entry(None, None, None, read_node=False) self.assertIsNone(base_entry.GetDefaultFilename())

A few new features have been added. This has rendered part of the README obsolete. Update it.

Signed-off-by: Simon Glass sjg@chromium.org ---

tools/binman/README | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/tools/binman/README b/tools/binman/README index 92341a14b61..f07de350f60 100644 --- a/tools/binman/README +++ b/tools/binman/README @@ -36,10 +36,9 @@ suitable padding and alignment. It provides a way to process binaries before they are included, by adding a Python plug-in. The device tree is available to U-Boot at run-time so that the images can be interpreted.

-Binman does not yet update the device tree with the final location of -everything when it is done. A simple C structure could be generated for -constrained environments like SPL (using dtoc) but this is also not -implemented. +Binman can update the device tree with the final location of everything when it +is done. Entry positions can be provided to U-Boot SPL as run-time symbols, +avoiding device-tree code overhead.

Binman can also support incorporating filesystems in the image if required. For example x86 platforms may use CBFS in some cases.

It is useful to be able to quickly locate the FDT map in the image. An easy way to do this is with a pointer at the start or end of the image.

Add an 'image header' entry, which places a magic number followed by a pointer to the FDT map. This can be located at the start or end of the image, or at a chosen location.

As part of this, update GetSiblingImagePos() to detect missing siblings.

Signed-off-by: Simon Glass sjg@chromium.org ---

tools/binman/README | 5 +- tools/binman/README.entries | 19 +++++ tools/binman/entry.py | 11 +++ tools/binman/etype/image_header.py | 76 ++++++++++++++++++++ tools/binman/ftest.py | 50 +++++++++++++ tools/binman/test/116_fdtmap_hdr.dts | 17 +++++ tools/binman/test/117_fdtmap_hdr_start.dts | 19 +++++ tools/binman/test/118_fdtmap_hdr_pos.dts | 19 +++++ tools/binman/test/119_fdtmap_hdr_missing.dts | 16 +++++ tools/binman/test/120_hdr_no_location.dts | 16 +++++ 10 files changed, 246 insertions(+), 2 deletions(-) create mode 100644 tools/binman/etype/image_header.py create mode 100644 tools/binman/test/116_fdtmap_hdr.dts create mode 100644 tools/binman/test/117_fdtmap_hdr_start.dts create mode 100644 tools/binman/test/118_fdtmap_hdr_pos.dts create mode 100644 tools/binman/test/119_fdtmap_hdr_missing.dts create mode 100644 tools/binman/test/120_hdr_no_location.dts

diff --git a/tools/binman/README b/tools/binman/README index 77f047bf6a3..61a7a20f232 100644 --- a/tools/binman/README +++ b/tools/binman/README @@ -640,7 +640,9 @@ of each entry.

Alternatively, an FDT map entry can be used to add a special FDT containing just the information about the image. This is preceeded by a magic string so can -be located anywhere in the image. +be located anywhere in the image. An image header (typically at the start or end +of the image) can be used to point to the FDT map. See fdtmap and image-header +entries for more information.

Compression @@ -817,7 +819,6 @@ Some ideas: - Add an option to decode an image into the constituent binaries - Support building an image for a board (-b) more completely, with a configurable build directory -- Support putting the FDT in an image with a suitable magic number - Support listing files in images - Support logging of binman's operations, with different levels of verbosity

diff --git a/tools/binman/README.entries b/tools/binman/README.entries index 7014d36f5ff..598d8278a70 100644 --- a/tools/binman/README.entries +++ b/tools/binman/README.entries @@ -331,6 +331,25 @@ README.chromium for how to obtain the required keys and tools.

+Entry: image-header: An entry which contains a pointer to the FDT map +--------------------------------------------------------------------- + +Properties / Entry arguments: + location: Location of header ("start" or "end" of image). This is + optional. If omitted then the entry must have an offset property. + +This adds an 8-byte entry to the start or end of the image, pointing to the +location of the FDT map. The format is a magic number followed by an offset +from the start or end of the image, in twos-compliment format. + +This entry must be in the top-level part of the image. + +NOTE: If the location is at the start/end, you will probably need to specify +sort-by-offset for the image, unless you actually put the image header +first/last in the entry list. + + + Entry: intel-cmc: Entry containing an Intel Chipset Micro Code (CMC) file -------------------------------------------------------------------------

diff --git a/tools/binman/entry.py b/tools/binman/entry.py index 7356c49c626..e1cd0d3a882 100644 --- a/tools/binman/entry.py +++ b/tools/binman/entry.py @@ -561,3 +561,14 @@ features to produce new behaviours. else False """ return name in self.section.GetEntries() + + def GetSiblingImagePos(self, name): + """Return the image position of the given sibling + + Returns: + Image position of sibling, or None if the sibling has no position, + or False if there is no such sibling + """ + if not self.HasSibling(name): + return False + return self.section.GetEntries()[name].image_pos diff --git a/tools/binman/etype/image_header.py b/tools/binman/etype/image_header.py new file mode 100644 index 00000000000..9bc84ec01d4 --- /dev/null +++ b/tools/binman/etype/image_header.py @@ -0,0 +1,76 @@ +# SPDX-License-Identifier: GPL-2.0+ +# Copyright (c) 2018 Google, Inc +# Written by Simon Glass sjg@chromium.org + +"""Entry-type module for an image header which points to the FDT map + +This creates an 8-byte entry with a magic number and the offset of the FDT map +(which is another entry in the image), relative to the start or end of the +image. +""" + +import struct + +from entry import Entry +import fdt_util + +IMAGE_HEADER_MAGIC = b'BinM' + +class Entry_image_header(Entry): + """An entry which contains a pointer to the FDT map + + Properties / Entry arguments: + location: Location of header ("start" or "end" of image). This is + optional. If omitted then the entry must have an offset property. + + This adds an 8-byte entry to the start or end of the image, pointing to the + location of the FDT map. The format is a magic number followed by an offset + from the start or end of the image, in twos-compliment format. + + This entry must be in the top-level part of the image. + + NOTE: If the location is at the start/end, you will probably need to specify + sort-by-offset for the image, unless you actually put the image header + first/last in the entry list. + """ + def __init__(self, section, etype, node): + Entry.__init__(self, section, etype, node) + self.location = fdt_util.GetString(self._node, 'location') + + def _GetHeader(self): + image_pos = self.GetSiblingImagePos('fdtmap') + if image_pos == False: + self.Raise("'image_header' section must have an 'fdtmap' sibling") + elif image_pos is None: + # This will be available when called from ProcessContents(), but not + # when called from ObtainContents() + offset = 0xffffffff + else: + image_size = self.section.GetImageSize() or 0 + base = (0 if self.location != 'end' else image_size) + offset = (image_pos - base) & 0xffffffff + data = IMAGE_HEADER_MAGIC + struct.pack('<I', offset) + return data + + def ObtainContents(self): + """Obtain a placeholder for the header contents""" + self.SetContents(self._GetHeader()) + return True + + def Pack(self, offset): + """Special pack method to set the offset to start/end of image""" + if not self.offset: + if self.location not in ['start', 'end']: + self.Raise("Invalid location '%s', expected 'start' or 'end'" % + self.location) + image_size = self.section.GetImageSize() or 0 + self.offset = (0 if self.location != 'end' else image_size - 8) + return Entry.Pack(self, offset) + + def ProcessContents(self): + """Write an updated version of the FDT map to this entry + + This is necessary since image_pos is not available when ObtainContents() + is called, since by then the entries have not been packed in the image. + """ + self.SetContents(self._GetHeader()) diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index 9e61f785d92..46540e8f5dd 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -2057,6 +2057,56 @@ class TestFunctional(unittest.TestCase): self.assertIn("Cannot locate node for path '/binman-suffix'", str(e.exception))

+ def testFdtmapHeader(self): + """Test an FDT map and image header can be inserted in the image""" + data = self._DoReadFileDtb('116_fdtmap_hdr.dts', use_real_dtb=True, + update_dtb=True)[0] + fdtmap_pos = len(U_BOOT_DATA) + fdtmap_data = data[fdtmap_pos:] + fdt_data = fdtmap_data[16:] + dtb = fdt.Fdt.FromData(fdt_data) + fdt_size = dtb.GetFdtObj().totalsize() + hdr_data = data[-8:] + self.assertEqual('BinM', hdr_data[:4]) + offset = struct.unpack('<I', hdr_data[4:])[0] & 0xffffffff + self.assertEqual(fdtmap_pos - 0x400, offset - (1 << 32)) + + def testFdtmapHeaderStart(self): + """Test an image header can be inserted at the image start""" + data = self._DoReadFileDtb('117_fdtmap_hdr_start.dts', + use_real_dtb=True, update_dtb=True)[0] + fdtmap_pos = 0x100 + len(U_BOOT_DATA) + hdr_data = data[:8] + self.assertEqual('BinM', hdr_data[:4]) + offset = struct.unpack('<I', hdr_data[4:])[0] + self.assertEqual(fdtmap_pos, offset) + + def testFdtmapHeaderPos(self): + """Test an image header can be inserted at a chosen position""" + data = self._DoReadFileDtb('118_fdtmap_hdr_pos.dts', + use_real_dtb=True, update_dtb=True)[0] + fdtmap_pos = 0x100 + len(U_BOOT_DATA) + hdr_data = data[0x80:0x88] + self.assertEqual('BinM', hdr_data[:4]) + offset = struct.unpack('<I', hdr_data[4:])[0] + self.assertEqual(fdtmap_pos, offset) + + def testHeaderMissingFdtmap(self): + """Test an image header requires an fdtmap""" + with self.assertRaises(ValueError) as e: + self._DoReadFileDtb('119_fdtmap_hdr_missing.dts', + use_real_dtb=True, update_dtb=True) + self.assertIn("'image_header' section must have an 'fdtmap' sibling", + str(e.exception)) + + def testHeaderNoLocation(self): + """Test an image header with a no specified location is detected""" + with self.assertRaises(ValueError) as e: + self._DoReadFileDtb('120_hdr_no_location.dts', + use_real_dtb=True, update_dtb=True) + self.assertIn("Invalid location 'None', expected 'start' or 'end'", + str(e.exception)) +

if __name__ == "__main__": unittest.main() diff --git a/tools/binman/test/116_fdtmap_hdr.dts b/tools/binman/test/116_fdtmap_hdr.dts new file mode 100644 index 00000000000..77a2194b394 --- /dev/null +++ b/tools/binman/test/116_fdtmap_hdr.dts @@ -0,0 +1,17 @@ +/dts-v1/; + +/ { + #address-cells = <1>; + #size-cells = <1>; + + binman { + size = <0x400>; + u-boot { + }; + fdtmap { + }; + image-header { + location = "end"; + }; + }; +}; diff --git a/tools/binman/test/117_fdtmap_hdr_start.dts b/tools/binman/test/117_fdtmap_hdr_start.dts new file mode 100644 index 00000000000..17b6be00470 --- /dev/null +++ b/tools/binman/test/117_fdtmap_hdr_start.dts @@ -0,0 +1,19 @@ +/dts-v1/; + +/ { + #address-cells = <1>; + #size-cells = <1>; + + binman { + size = <0x400>; + sort-by-offset; + u-boot { + offset = <0x100>; + }; + fdtmap { + }; + image-header { + location = "start"; + }; + }; +}; diff --git a/tools/binman/test/118_fdtmap_hdr_pos.dts b/tools/binman/test/118_fdtmap_hdr_pos.dts new file mode 100644 index 00000000000..fd803f57fba --- /dev/null +++ b/tools/binman/test/118_fdtmap_hdr_pos.dts @@ -0,0 +1,19 @@ +/dts-v1/; + +/ { + #address-cells = <1>; + #size-cells = <1>; + + binman { + size = <0x400>; + sort-by-offset; + u-boot { + offset = <0x100>; + }; + fdtmap { + }; + image-header { + offset = <0x80>; + }; + }; +}; diff --git a/tools/binman/test/119_fdtmap_hdr_missing.dts b/tools/binman/test/119_fdtmap_hdr_missing.dts new file mode 100644 index 00000000000..41bb680f08f --- /dev/null +++ b/tools/binman/test/119_fdtmap_hdr_missing.dts @@ -0,0 +1,16 @@ +/dts-v1/; + +/ { + #address-cells = <1>; + #size-cells = <1>; + + binman { + sort-by-offset; + u-boot { + }; + image-header { + offset = <0x80>; + location = "start"; + }; + }; +}; diff --git a/tools/binman/test/120_hdr_no_location.dts b/tools/binman/test/120_hdr_no_location.dts new file mode 100644 index 00000000000..585e21f456b --- /dev/null +++ b/tools/binman/test/120_hdr_no_location.dts @@ -0,0 +1,16 @@ +/dts-v1/; + +/ { + #address-cells = <1>; + #size-cells = <1>; + + binman { + sort-by-offset; + u-boot { + }; + fdtmap { + }; + image-header { + }; + }; +};

This class is the new way to handle arguments in Python. Convert binman over to use it. At the same time, introduce commands so that we can separate out the different parts of binman functionality.

Signed-off-by: Simon Glass sjg@chromium.org ---

.travis.yml | 2 +- test/run | 5 ++- tools/binman/README | 6 +-- tools/binman/binman.py | 43 ++++++++++--------- tools/binman/cmdline.py | 90 +++++++++++++++++++++++---------------- tools/binman/control.py | 51 +++++++++++----------- tools/binman/ftest.py | 35 +++++++-------- tools/patman/test_util.py | 5 ++- 8 files changed, 128 insertions(+), 109 deletions(-)

diff --git a/.travis.yml b/.travis.yml index ae68a2d351c..4592c00c9ce 100644 --- a/.travis.yml +++ b/.travis.yml @@ -146,7 +146,7 @@ script: if [[ -n "${TEST_PY_TOOLS}" ]]; then PYTHONPATH="${UBOOT_TRAVIS_BUILD_DIR}/scripts/dtc/pylibfdt" PATH="${UBOOT_TRAVIS_BUILD_DIR}/scripts/dtc:${PATH}" - ./tools/binman/binman --toolpath ${UBOOT_TRAVIS_BUILD_DIR}/tools -t && + ./tools/binman/binman --toolpath ${UBOOT_TRAVIS_BUILD_DIR}/tools test && ./tools/patman/patman --test && ./tools/buildman/buildman -t && PYTHONPATH="${UBOOT_TRAVIS_BUILD_DIR}/scripts/dtc/pylibfdt" diff --git a/test/run b/test/run index b97647eba6f..fc7fe5c2bbc 100755 --- a/test/run +++ b/test/run @@ -40,7 +40,7 @@ export PYTHONPATH=${DTC_DIR}/pylibfdt export DTC=${DTC_DIR}/dtc TOOLS_DIR=build-sandbox_spl/tools

-run_test "binman" ./tools/binman/binman -t --toolpath ${TOOLS_DIR} +run_test "binman" ./tools/binman/binman test --toolpath ${TOOLS_DIR} run_test "patman" ./tools/patman/patman --test

[ "$1" == "quick" ] && skip=--skip-net-tests @@ -52,7 +52,8 @@ run_test "dtoc" ./tools/dtoc/dtoc -t # To enable Python test coverage on Debian-type distributions (e.g. Ubuntu): # $ sudo apt-get install python-pytest python-coverage export PATH=$PATH:${TOOLS_DIR} -run_test "binman code coverage" ./tools/binman/binman -T --toolpath ${TOOLS_DIR} +run_test "binman code coverage" ./tools/binman/binman test -T \ + --toolpath ${TOOLS_DIR} run_test "dtoc code coverage" ./tools/dtoc/dtoc -T run_test "fdt code coverage" ./tools/dtoc/test_fdt -T

diff --git a/tools/binman/README b/tools/binman/README index 61a7a20f232..99b4e5f4504 100644 --- a/tools/binman/README +++ b/tools/binman/README @@ -187,7 +187,7 @@ First install prerequisites, e.g.

Type:

- binman -b <board_name> + binman build -b <board_name>

to build an image for a board. The board name is the same name used when configuring U-Boot (e.g. for sandbox_defconfig the board name is 'sandbox'). @@ -195,7 +195,7 @@ Binman assumes that the input files for the build are in ../b/<board_name>.

Or you can specify this explicitly:

- binman -I <build_path> + binman build -I <build_path>

where <build_path> is the build directory containing the output of the U-Boot build. @@ -483,7 +483,7 @@ Entry Documentation For details on the various entry types supported by binman and how to use them, see README.entries. This is generated from the source code using:

- binman -E >tools/binman/README.entries + binman entry-docs >tools/binman/README.entries

Hashing Entries diff --git a/tools/binman/binman.py b/tools/binman/binman.py index 613aad5c451..d225fe86024 100755 --- a/tools/binman/binman.py +++ b/tools/binman/binman.py @@ -20,14 +20,15 @@ import sys import traceback import unittest

-# Bring in the patman and dtoc libraries +# Bring in the patman and dtoc libraries (but don't override the first path +# in PYTHONPATH) our_path = os.path.dirname(os.path.realpath(__file__)) for dirname in ['../patman', '../dtoc', '..', '../concurrencytest']: - sys.path.insert(0, os.path.join(our_path, dirname)) + sys.path.insert(2, os.path.join(our_path, dirname))

# Bring in the libfdt module -sys.path.insert(0, 'scripts/dtc/pylibfdt') -sys.path.insert(0, os.path.join(our_path, +sys.path.insert(2, 'scripts/dtc/pylibfdt') +sys.path.insert(2, os.path.join(our_path, '../../build-sandbox_spl/scripts/dtc/pylibfdt'))

# When running under python-coverage on Ubuntu 16.04, the dist-packages @@ -158,37 +159,36 @@ def RunTestCoverage(): for item in glob_list if '_testing' not in item]) test_util.RunTestCoverage('tools/binman/binman.py', None, ['*test*', '*binman.py', 'tools/patman/*', 'tools/dtoc/*'], - options.build_dir, all_set) + args.build_dir, all_set)

-def RunBinman(options, args): +def RunBinman(args): """Main entry point to binman once arguments are parsed

Args: - options: Command-line options - args: Non-option arguments + args: Command line arguments Namespace object """ ret_code = 0

- if not options.debug: + if not args.debug: sys.tracebacklimit = 0

- if options.test: - ret_code = RunTests(options.debug, options.verbosity, options.processes, - options.test_preserve_dirs, args[1:], - options.toolpath) - - elif options.test_coverage: - RunTestCoverage() + if args.cmd == 'test': + if args.test_coverage: + RunTestCoverage() + else: + ret_code = RunTests(args.debug, args.verbosity, args.processes, + args.test_preserve_dirs, args.tests, + args.toolpath)

- elif options.entry_docs: + elif args.cmd == 'entry-docs': control.WriteEntryDocs(GetEntryModules())

else: try: - ret_code = control.Binman(options, args) + ret_code = control.Binman(args) except Exception as e: print('binman: %s' % e) - if options.debug: + if args.debug: print() traceback.print_exc() ret_code = 1 @@ -196,6 +196,7 @@ def RunBinman(options, args):

if __name__ == "__main__": - (options, args) = cmdline.ParseArgs(sys.argv) - ret_code = RunBinman(options, args) + args = cmdline.ParseArgs(sys.argv[1:]) + + ret_code = RunBinman(args) sys.exit(ret_code) diff --git a/tools/binman/cmdline.py b/tools/binman/cmdline.py index 91e007e4e03..99ca6564654 100644 --- a/tools/binman/cmdline.py +++ b/tools/binman/cmdline.py @@ -5,7 +5,7 @@ # Command-line parser for binman #

-from optparse import OptionParser +from argparse import ArgumentParser

def ParseArgs(argv): """Parse the binman command-line arguments @@ -17,56 +17,72 @@ def ParseArgs(argv): options provides access to the options (e.g. option.debug) args is a list of string arguments """ - parser = OptionParser() - parser.add_option('-a', '--entry-arg', type='string', action='append', + if '-H' in argv: + argv.insert(0, 'build') + + usage = '''binman [-B <dir<] [-D] [-H] [-v<n>] [--toolpath <path>] <cmd> <options> + +Create and manipulate images for a board from a set of binaries. Binman is +controlled by a description in the board device tree.''' + + base_parser = ArgumentParser(add_help=False) + base_parser.add_argument('-B', '--build-dir', type=str, default='b', + help='Directory containing the build output') + base_parser.add_argument('-D', '--debug', action='store_true', + help='Enabling debugging (provides a full traceback on error)') + base_parser.add_argument('-H', '--full-help', action='store_true', + default=False, help='Display the README file') + base_parser.add_argument('--toolpath', type=str, action='append', + help='Add a path to the directories containing tools') + base_parser.add_argument('-v', '--verbosity', default=1, + type=int, help='Control verbosity: 0=silent, 1=progress, 3=full, ' + '4=debug') + + parser = ArgumentParser(usage=usage, parents=[base_parser]) + subparsers = parser.add_subparsers(dest='cmd') + + build_parser = subparsers.add_parser('build', help='Build firmware image', + parents=[base_parser]) + build_parser.add_argument('-a', '--entry-arg', type=str, action='append', help='Set argument value arg=value') - parser.add_option('-b', '--board', type='string', + build_parser.add_argument('-b', '--board', type=str, help='Board name to build') - parser.add_option('-B', '--build-dir', type='string', default='b', - help='Directory containing the build output') - parser.add_option('-d', '--dt', type='string', + build_parser.add_argument('-d', '--dt', type=str, help='Configuration file (.dtb) to use') - parser.add_option('-D', '--debug', action='store_true', - help='Enabling debugging (provides a full traceback on error)') - parser.add_option('-E', '--entry-docs', action='store_true', - help='Write out entry documentation (see README.entries)') - parser.add_option('--fake-dtb', action='store_true', + build_parser.add_argument('--fake-dtb', action='store_true', help='Use fake device tree contents (for testing only)') - parser.add_option('-i', '--image', type='string', action='append', + build_parser.add_argument('-i', '--image', type=str, action='append', help='Image filename to build (if not specified, build all)') - parser.add_option('-I', '--indir', action='append', + build_parser.add_argument('-I', '--indir', action='append', help='Add a path to the list of directories to use for input files') - parser.add_option('-H', '--full-help', action='store_true', - default=False, help='Display the README file') - parser.add_option('-m', '--map', action='store_true', + build_parser.add_argument('-m', '--map', action='store_true', default=False, help='Output a map file for each image') - parser.add_option('-O', '--outdir', type='string', + build_parser.add_argument('-O', '--outdir', type=str, action='store', help='Path to directory to use for intermediate and ' 'output files') - parser.add_option('-p', '--preserve', action='store_true',\ + build_parser.add_argument('-p', '--preserve', action='store_true',\ help='Preserve temporary output directory even if option -O is not ' 'given') - parser.add_option('-P', '--processes', type=int, - help='set number of processes to use for running tests') - parser.add_option('-t', '--test', action='store_true', - default=False, help='run tests') - parser.add_option('-T', '--test-coverage', action='store_true', - default=False, help='run tests and check for 100% coverage') - parser.add_option('--toolpath', type='string', action='append', - help='Add a path to the directories containing tools') - parser.add_option('-u', '--update-fdt', action='store_true', + build_parser.add_argument('-u', '--update-fdt', action='store_true', default=False, help='Update the binman node with offset/size info') - parser.add_option('-v', '--verbosity', default=1, - type='int', help='Control verbosity: 0=silent, 1=progress, 3=full, ' - '4=debug') - parser.add_option('-X', '--test-preserve-dirs', action='store_true', + + entry_parser = subparsers.add_parser('entry-docs', + help='Write out entry documentation (see README.entries)') + + list_parser = subparsers.add_parser('list', help='List files in an image') + list_parser.add_argument('fname', type=str, help='Image file to list') + + test_parser = subparsers.add_parser('test', help='Run tests', + parents=[base_parser]) + test_parser.add_argument('-P', '--processes', type=int, + help='set number of processes to use for running tests') + test_parser.add_argument('-T', '--test-coverage', action='store_true', + default=False, help='run tests and check for 100%% coverage') + test_parser.add_argument('-X', '--test-preserve-dirs', action='store_true', help='Preserve and display test-created input directories; also ' 'preserve the output directory if a single test is run (pass test ' 'name at the end of the command line') - - parser.usage += """ - -Create images for a board from a set of binaries. It is controlled by a -description in the board device tree.""" + test_parser.add_argument('tests', nargs='*', + help='Test names to run (omit for all)')

return parser.parse_args(argv) diff --git a/tools/binman/control.py b/tools/binman/control.py index 4a94afc8640..9022cf76e99 100644 --- a/tools/binman/control.py +++ b/tools/binman/control.py @@ -67,19 +67,18 @@ def WriteEntryDocs(modules, test_missing=None): from entry import Entry Entry.WriteDocs(modules, test_missing)

-def Binman(options, args): +def Binman(args): """The main control code for binman

This assumes that help and test options have already been dealt with. It deals with the core task of building images.

Args: - options: Command line options object - args: Command line arguments (list of strings) + args: Command line arguments Namespace object """ global images

- if options.full_help: + if args.full_help: pager = os.getenv('PAGER') if not pager: pager = 'more' @@ -89,17 +88,17 @@ def Binman(options, args): return 0

# Try to figure out which device tree contains our image description - if options.dt: - dtb_fname = options.dt + if args.dt: + dtb_fname = args.dt else: - board = options.board + board = args.board if not board: raise ValueError('Must provide a board to process (use -b <board>)') - board_pathname = os.path.join(options.build_dir, board) + board_pathname = os.path.join(args.build_dir, board) dtb_fname = os.path.join(board_pathname, 'u-boot.dtb') - if not options.indir: - options.indir = ['.'] - options.indir.append(board_pathname) + if not args.indir: + args.indir = ['.'] + args.indir.append(board_pathname)

try: # Import these here in case libfdt.py is not available, in which case @@ -107,15 +106,15 @@ def Binman(options, args): import fdt import fdt_util

- tout.Init(options.verbosity) - elf.debug = options.debug - cbfs_util.VERBOSE = options.verbosity > 2 - state.use_fake_dtb = options.fake_dtb + tout.Init(args.verbosity) + elf.debug = args.debug + cbfs_util.VERBOSE = args.verbosity > 2 + state.use_fake_dtb = args.fake_dtb try: - tools.SetInputDirs(options.indir) - tools.PrepareOutputDir(options.outdir, options.preserve) - tools.SetToolPaths(options.toolpath) - state.SetEntryArgs(options.entry_arg) + tools.SetInputDirs(args.indir) + tools.PrepareOutputDir(args.outdir, args.preserve) + tools.SetToolPaths(args.toolpath) + state.SetEntryArgs(args.entry_arg)

# Get the device tree ready by compiling it and copying the compiled # output into a file in our output directly. Then scan it for use @@ -132,16 +131,16 @@ def Binman(options, args):

images = _ReadImageDesc(node)

- if options.image: + if args.image: skip = [] new_images = OrderedDict() for name, image in images.items(): - if name in options.image: + if name in args.image: new_images[name] = image else: skip.append(name) images = new_images - if skip and options.verbosity >= 2: + if skip and args.verbosity >= 2: print('Skipping images: %s' % ', '.join(skip))

state.Prepare(images, dtb) @@ -155,7 +154,7 @@ def Binman(options, args): # entry offsets remain the same. for image in images.values(): image.ExpandEntries() - if options.update_fdt: + if args.update_fdt: image.AddMissingProperties() image.ProcessFdt(dtb)

@@ -176,19 +175,19 @@ def Binman(options, args): image.CheckSize() image.CheckEntries() except Exception as e: - if options.map: + if args.map: fname = image.WriteMap() print("Wrote map file '%s' to show errors" % fname) raise image.SetImagePos() - if options.update_fdt: + if args.update_fdt: image.SetCalculatedProperties() for dtb_item in state.GetFdts(): dtb_item.Sync() image.ProcessEntryContents() image.WriteSymbols() image.BuildImage() - if options.map: + if args.map: image.WriteMap()

# Write the updated FDTs to our output files diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index 46540e8f5dd..f1a79baa905 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -207,7 +207,7 @@ class TestFunctional(unittest.TestCase): result.stdout + result.stderr)) return result

- def _DoBinman(self, *args): + def _DoBinman(self, *argv): """Run binman using directly (in the same process)

Args: @@ -215,16 +215,16 @@ class TestFunctional(unittest.TestCase): Returns: Return value (0 for success) """ - args = list(args) + argv = list(argv) if '-D' in sys.argv: - args = args + ['-D'] - (options, args) = cmdline.ParseArgs(args) - options.pager = 'binman-invalid-pager' - options.build_dir = self._indir + argv = argv + ['-D'] + args = cmdline.ParseArgs(argv) + args.pager = 'binman-invalid-pager' + args.build_dir = self._indir

# For testing, you can force an increase in verbosity here - # options.verbosity = tout.DEBUG - return control.Binman(options, args) + # args.verbosity = tout.DEBUG + return control.Binman(args)

def _DoTestFile(self, fname, debug=False, map=False, update_dtb=False, entry_args=None, images=None, use_real_dtb=False, @@ -242,7 +242,7 @@ class TestFunctional(unittest.TestCase): value: value of that arg images: List of image names to build """ - args = ['-p', '-I', self._indir, '-d', self.TestFile(fname)] + args = ['build', '-p', '-I', self._indir, '-d', self.TestFile(fname)] if debug: args.append('-D') if map: @@ -515,20 +515,20 @@ class TestFunctional(unittest.TestCase): """Test that we can run it with a specific board""" self._SetupDtb('005_simple.dts', 'sandbox/u-boot.dtb') TestFunctional._MakeInputFile('sandbox/u-boot.bin', U_BOOT_DATA) - result = self._DoBinman('-b', 'sandbox') + result = self._DoBinman('build', '-b', 'sandbox') self.assertEqual(0, result)

def testNeedBoard(self): """Test that we get an error when no board ius supplied""" with self.assertRaises(ValueError) as e: - result = self._DoBinman() + result = self._DoBinman('build') self.assertIn("Must provide a board to process (use -b <board>)", str(e.exception))

def testMissingDt(self): """Test that an invalid device-tree file generates an error""" with self.assertRaises(Exception) as e: - self._RunBinman('-d', 'missing_file') + self._RunBinman('build', '-d', 'missing_file') # We get one error from libfdt, and a different one from fdtget. self.AssertInList(["Couldn't open blob from 'missing_file'", 'No such file or directory'], str(e.exception)) @@ -540,26 +540,26 @@ class TestFunctional(unittest.TestCase): will come from the device-tree compiler (dtc). """ with self.assertRaises(Exception) as e: - self._RunBinman('-d', self.TestFile('001_invalid.dts')) + self._RunBinman('build', '-d', self.TestFile('001_invalid.dts')) self.assertIn("FATAL ERROR: Unable to parse input tree", str(e.exception))

def testMissingNode(self): """Test that a device tree without a 'binman' node generates an error""" with self.assertRaises(Exception) as e: - self._DoBinman('-d', self.TestFile('002_missing_node.dts')) + self._DoBinman('build', '-d', self.TestFile('002_missing_node.dts')) self.assertIn("does not have a 'binman' node", str(e.exception))

def testEmpty(self): """Test that an empty binman node works OK (i.e. does nothing)""" - result = self._RunBinman('-d', self.TestFile('003_empty.dts')) + result = self._RunBinman('build', '-d', self.TestFile('003_empty.dts')) self.assertEqual(0, len(result.stderr)) self.assertEqual(0, result.return_code)

def testInvalidEntry(self): """Test that an invalid entry is flagged""" with self.assertRaises(Exception) as e: - result = self._RunBinman('-d', + result = self._RunBinman('build', '-d', self.TestFile('004_invalid_entry.dts')) self.assertIn("Unknown entry type 'not-a-valid-type' in node " "'/binman/not-a-valid-type'", str(e.exception)) @@ -1290,7 +1290,8 @@ class TestFunctional(unittest.TestCase):

def testEntryArgsInvalidFormat(self): """Test that an invalid entry-argument format is detected""" - args = ['-d', self.TestFile('064_entry_args_required.dts'), '-ano-value'] + args = ['build', '-d', self.TestFile('064_entry_args_required.dts'), + '-ano-value'] with self.assertRaises(ValueError) as e: self._DoBinman(*args) self.assertIn("Invalid entry arguemnt 'no-value'", str(e.exception)) diff --git a/tools/patman/test_util.py b/tools/patman/test_util.py index ea36cd16339..40098159c08 100644 --- a/tools/patman/test_util.py +++ b/tools/patman/test_util.py @@ -46,9 +46,10 @@ def RunTestCoverage(prog, filter_fname, exclude_list, build_dir, required=None): glob_list = [] glob_list += exclude_list glob_list += ['*libfdt.py', '*site-packages*', '*dist-packages*'] + test_cmd = 'test' if 'binman.py' in prog else '-t' cmd = ('PYTHONPATH=$PYTHONPATH:%s/sandbox_spl/tools %s-coverage run ' - '--omit "%s" %s -P1 -t' % (build_dir, PYTHON, ','.join(glob_list), - prog)) + '--omit "%s" %s %s -P1' % (build_dir, PYTHON, ','.join(glob_list), + prog, test_cmd)) os.system(cmd) stdout = command.Output('%s-coverage' % PYTHON, 'report') lines = stdout.splitlines()

The first argument is not used. Remove it.

Signed-off-by: Simon Glass sjg@chromium.org ---

tools/binman/entry.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/tools/binman/entry.py b/tools/binman/entry.py index 8cccc2ed5f0..00bb1d190a9 100644 --- a/tools/binman/entry.py +++ b/tools/binman/entry.py @@ -85,11 +85,10 @@ class Entry(object): self.ReadNode()

@staticmethod - def Lookup(section, node_path, etype): + def Lookup(node_path, etype): """Look up the entry class for a node.

Args: - section: Section object containing this node node_node: Path name of Node object containing information about the entry to create (used for errors) etype: Entry type to use @@ -140,7 +139,7 @@ class Entry(object): """ if not etype: etype = fdt_util.GetString(node, 'type', node.name) - obj = Entry.Lookup(section, node.path, etype) + obj = Entry.Lookup(node.path, etype)

# Call its constructor to get the object we want. return obj(section, etype, node) @@ -514,7 +513,7 @@ features to produce new behaviours. modules.remove('_testing') missing = [] for name in modules: - module = Entry.Lookup(name, name, name) + module = Entry.Lookup(name, name) docs = getattr(module, '__doc__') if test_missing == name: docs = None

This function raises an exception with its arguments around the wrong way so the message is incorrect. Fix this as well as a few minor comment problems.

Signed-off-by: Simon Glass sjg@chromium.org ---

tools/binman/entry.py | 8 ++++---- tools/binman/ftest.py | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/tools/binman/entry.py b/tools/binman/entry.py index a04e149d96c..b19a3b026f2 100644 --- a/tools/binman/entry.py +++ b/tools/binman/entry.py @@ -237,25 +237,25 @@ class Entry(object): This sets both the data and content_size properties

Args: - data: Data to set to the contents (string) + data: Data to set to the contents (bytes) """ self.data = data self.contents_size = len(self.data)

def ProcessContentsUpdate(self, data): - """Update the contens of an entry, after the size is fixed + """Update the contents of an entry, after the size is fixed

This checks that the new data is the same size as the old.

Args: - data: Data to set to the contents (string) + data: Data to set to the contents (bytes)

Raises: ValueError if the new data size is not the same as the old """ if len(data) != self.contents_size: self.Raise('Cannot update entry size from %d to %d' % - (len(data), self.contents_size)) + (self.contents_size, len(data))) self.SetContents(data)

def ObtainContents(self): diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index f1a79baa905..8c9942982ba 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -1213,7 +1213,7 @@ class TestFunctional(unittest.TestCase): with self.assertRaises(ValueError) as e: self._DoReadFile('059_change_size.dts', True) self.assertIn("Node '/binman/_testing': Cannot update entry size from " - '2 to 1', str(e.exception)) + '1 to 2', str(e.exception))

def testUpdateFdt(self): """Test that we can update the device tree with offset/size info"""

At present if this function tries to update the contents such that the size changes, it raises an error. We plan to add the ability to change the size of entries after packing is completed, since in some cases it is not possible to determine the size in advance.

An example of this is with a compressed device tree, where the values of the device tree change in SetCalculatedProperties() or ProcessEntryContents(). While the device tree itself does not change size, since placeholders for any new properties have already bee added by AddMissingProperties(), we cannot predict the size of the device tree after compression. If a value changes from 0 to 0x1234 (say), then the compressed device tree may expand.

As a first step towards supporting this, make ProcessContentsUpdate() return a value indicating whether the content size is OK. For now this is always True (since otherwise binman raises an error), but later patches will adjust this.

Signed-off-by: Simon Glass sjg@chromium.org ---

tools/binman/bsection.py | 8 +++++++- tools/binman/entry.py | 22 +++++++++++++++++++-- tools/binman/etype/_testing.py | 5 +++-- tools/binman/etype/blob_dtb.py | 2 +- tools/binman/etype/fdtmap.py | 2 +- tools/binman/etype/fmap.py | 2 +- tools/binman/etype/image_header.py | 2 +- tools/binman/etype/section.py | 5 +++-- tools/binman/etype/u_boot_with_ucode_ptr.py | 6 +++--- tools/binman/image.py | 5 ++++- 10 files changed, 44 insertions(+), 15 deletions(-)

diff --git a/tools/binman/bsection.py b/tools/binman/bsection.py index 3e3d369d5e4..f49a6e93bc7 100644 --- a/tools/binman/bsection.py +++ b/tools/binman/bsection.py @@ -317,9 +317,15 @@ class Section(object): """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(): - entry.ProcessContents() + if not entry.ProcessContents(): + sizes_ok = False + return sizes_ok

def WriteSymbols(self): """Write symbol values into binary files for access at run time""" diff --git a/tools/binman/entry.py b/tools/binman/entry.py index b19a3b026f2..7db1402b84f 100644 --- a/tools/binman/entry.py +++ b/tools/binman/entry.py @@ -245,7 +245,8 @@ class Entry(object): def ProcessContentsUpdate(self, data): """Update the contents of an entry, after the size is fixed

- This checks that the new data is the same size as the old. + This checks that the new data is the same size as the old. If the size + has changed, this triggers a re-run of the packing algorithm.

Args: data: Data to set to the contents (bytes) @@ -253,10 +254,12 @@ class Entry(object): Raises: ValueError if the new data size is not the same as the old """ + size_ok = True if len(data) != self.contents_size: self.Raise('Cannot update entry size from %d to %d' % (self.contents_size, len(data))) self.SetContents(data) + return size_ok

def ObtainContents(self): """Figure out the contents of an entry. @@ -401,7 +404,22 @@ class Entry(object): self.image_pos = image_pos + self.offset

def ProcessContents(self): - pass + """Do any post-packing updates of entry contents + + This function should call ProcessContentsUpdate() to update the entry + contents, if necessary, returning its return value here. + + Args: + data: Data to set to the contents (bytes) + + Returns: + True if the new data size is OK, False if expansion is needed + + Raises: + ValueError if the new data size is not the same as the old and + state.AllowEntryExpansion() is False + """ + return True

def WriteSymbols(self, section): """Write symbol values into binary files for access at run time diff --git a/tools/binman/etype/_testing.py b/tools/binman/etype/_testing.py index ac62d2e2005..2204362281c 100644 --- a/tools/binman/etype/_testing.py +++ b/tools/binman/etype/_testing.py @@ -88,9 +88,10 @@ class Entry__testing(Entry):

def ProcessContents(self): if self.bad_update_contents: - # Request to update the conents with something larger, to cause a + # Request to update the contents with something larger, to cause a # failure. - self.ProcessContentsUpdate('aa') + return self.ProcessContentsUpdate('aa') + return True

def ProcessFdt(self, fdt): """Force reprocessing the first time""" diff --git a/tools/binman/etype/blob_dtb.py b/tools/binman/etype/blob_dtb.py index d80c3d7e006..09d5d727138 100644 --- a/tools/binman/etype/blob_dtb.py +++ b/tools/binman/etype/blob_dtb.py @@ -30,4 +30,4 @@ class Entry_blob_dtb(Entry_blob): def ProcessContents(self): """Re-read the DTB contents so that we get any calculated properties""" _, data = state.GetFdtContents(self._filename) - self.ProcessContentsUpdate(data) + return self.ProcessContentsUpdate(data) diff --git a/tools/binman/etype/fdtmap.py b/tools/binman/etype/fdtmap.py index ddac148b9ba..bfd7962be3a 100644 --- a/tools/binman/etype/fdtmap.py +++ b/tools/binman/etype/fdtmap.py @@ -106,4 +106,4 @@ class Entry_fdtmap(Entry): This is necessary since new data may have been written back to it during processing, e.g. the image-pos properties. """ - self.ProcessContentsUpdate(self._GetFdtmap()) + return self.ProcessContentsUpdate(self._GetFdtmap()) diff --git a/tools/binman/etype/fmap.py b/tools/binman/etype/fmap.py index 45d6db18a31..3a809486098 100644 --- a/tools/binman/etype/fmap.py +++ b/tools/binman/etype/fmap.py @@ -62,4 +62,4 @@ class Entry_fmap(Entry): return True

def ProcessContents(self): - self.ProcessContentsUpdate(self._GetFmap()) + return self.ProcessContentsUpdate(self._GetFmap()) diff --git a/tools/binman/etype/image_header.py b/tools/binman/etype/image_header.py index d6de58ce4b7..b1c4f8a07e9 100644 --- a/tools/binman/etype/image_header.py +++ b/tools/binman/etype/image_header.py @@ -73,4 +73,4 @@ class Entry_image_header(Entry): This is necessary since image_pos is not available when ObtainContents() is called, since by then the entries have not been packed in the image. """ - self.ProcessContentsUpdate(self._GetHeader()) + return self.ProcessContentsUpdate(self._GetHeader()) diff --git a/tools/binman/etype/section.py b/tools/binman/etype/section.py index 3681a484689..51eddcd995a 100644 --- a/tools/binman/etype/section.py +++ b/tools/binman/etype/section.py @@ -85,8 +85,9 @@ class Entry_section(Entry): self._section.SetCalculatedProperties()

def ProcessContents(self): - self._section.ProcessEntryContents() - super(Entry_section, self).ProcessContents() + sizes_ok = self._section.ProcessEntryContents() + sizes_ok_base = super(Entry_section, self).ProcessContents() + return sizes_ok and sizes_ok_base

def CheckOffset(self): self._section.CheckEntries() diff --git a/tools/binman/etype/u_boot_with_ucode_ptr.py b/tools/binman/etype/u_boot_with_ucode_ptr.py index da0e12417b5..4104bf8bf13 100644 --- a/tools/binman/etype/u_boot_with_ucode_ptr.py +++ b/tools/binman/etype/u_boot_with_ucode_ptr.py @@ -91,6 +91,6 @@ class Entry_u_boot_with_ucode_ptr(Entry_blob): # Write the microcode offset and size into the entry offset_and_size = struct.pack('<2L', offset, size) self.target_offset -= self.image_pos - self.ProcessContentsUpdate(self.data[:self.target_offset] + - offset_and_size + - self.data[self.target_offset + 8:]) + return self.ProcessContentsUpdate(self.data[:self.target_offset] + + offset_and_size + + self.data[self.target_offset + 8:]) diff --git a/tools/binman/image.py b/tools/binman/image.py index f237ae302df..c8bce394aa1 100644 --- a/tools/binman/image.py +++ b/tools/binman/image.py @@ -122,8 +122,11 @@ class Image: """Call the ProcessContents() method for each entry

This is intended to adjust the contents as needed by the entry type. + + Returns: + True if the new data size is OK, False if expansion is needed """ - self._section.ProcessEntryContents() + return self._section.ProcessEntryContents()

def WriteSymbols(self): """Write symbol values into binary files for access at run time"""

Add support for detecting entries that change size after they have already been packed, and re-running packing when it happens.

This removes the limitation that entry size cannot change after PackEntries() is called.

Signed-off-by: Simon Glass sjg@chromium.org ---

tools/binman/README | 3 +- tools/binman/bsection.py | 11 ++++++ tools/binman/control.py | 39 ++++++++++++------- tools/binman/entry.py | 18 ++++++++- tools/binman/etype/_testing.py | 11 +++++- tools/binman/etype/section.py | 5 +++ tools/binman/etype/u_boot_with_ucode_ptr.py | 2 +- tools/binman/ftest.py | 33 ++++++++++++++-- tools/binman/image.py | 8 ++++ tools/binman/test/121_entry_expand.dts | 20 ++++++++++ tools/binman/test/122_entry_expand_twice.dts | 21 ++++++++++ .../binman/test/123_entry_expand_section.dts | 22 +++++++++++ 12 files changed, 168 insertions(+), 25 deletions(-) create mode 100644 tools/binman/test/121_entry_expand.dts create mode 100644 tools/binman/test/122_entry_expand_twice.dts create mode 100644 tools/binman/test/123_entry_expand_section.dts

diff --git a/tools/binman/README b/tools/binman/README index 150d94c65b8..7e745a2d466 100644 --- a/tools/binman/README +++ b/tools/binman/README @@ -566,7 +566,8 @@ tree. This sets the correct 'offset' and 'size' vaues, for example. The default implementatoin does nothing. This can be overriden to adjust the contents of an entry in some way. For example, it would be possible to create an entry containing a hash of the contents of some other entries. At this -stage the offset and size of entries should not be adjusted. +stage the offset and size of entries should not be adjusted unless absolutely +necessary, since it requires a repack (going back to PackEntries()).

10. WriteSymbols() - write the value of symbols into the U-Boot SPL binary. See 'Access to binman entry offsets at run time' below for a description of diff --git a/tools/binman/bsection.py b/tools/binman/bsection.py index f49a6e93bc7..d368e3f6baa 100644 --- a/tools/binman/bsection.py +++ b/tools/binman/bsection.py @@ -45,6 +45,8 @@ class Section(object): _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 @@ -76,6 +78,8 @@ class Section(object): """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" % @@ -257,6 +261,13 @@ class Section(object): 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 diff --git a/tools/binman/control.py b/tools/binman/control.py index 9022cf76e99..d48e7ac0070 100644 --- a/tools/binman/control.py +++ b/tools/binman/control.py @@ -170,21 +170,30 @@ def Binman(args): # completed and written, but that does not seem important. image.GetEntryContents() image.GetEntryOffsets() - try: - image.PackEntries() - image.CheckSize() - image.CheckEntries() - except Exception as e: - if args.map: - fname = image.WriteMap() - print("Wrote map file '%s' to show errors" % fname) - raise - image.SetImagePos() - if args.update_fdt: - image.SetCalculatedProperties() - for dtb_item in state.GetFdts(): - dtb_item.Sync() - image.ProcessEntryContents() + passes = 2 + for pack_pass in range(passes): + try: + image.PackEntries() + image.CheckSize() + image.CheckEntries() + except Exception as e: + if args.map: + fname = image.WriteMap() + print("Wrote map file '%s' to show errors" % fname) + raise + image.SetImagePos() + if args.update_fdt: + image.SetCalculatedProperties() + for dtb_item in state.GetFdts(): + dtb_item.Sync() + sizes_ok = image.ProcessEntryContents() + if sizes_ok: + break + image.ResetForPack() + if not sizes_ok: + image.Raise('Entries expanded after packing (tried %s passes' % + passes) + image.WriteSymbols() image.BuildImage() if args.map: diff --git a/tools/binman/entry.py b/tools/binman/entry.py index 7db1402b84f..a9a9d119e1d 100644 --- a/tools/binman/entry.py +++ b/tools/binman/entry.py @@ -61,6 +61,8 @@ class Entry(object): pad_after: Number of pad bytes after the contents, 0 if none data: Contents of entry (string of bytes) compress: Compression algoithm used (e.g. 'lz4'), 'none' if none + orig_offset: Original offset value read from node + orig_size: Original size value read from node """ def __init__(self, section, etype, node, read_node=True, name_prefix=''): self.section = section @@ -153,6 +155,7 @@ class Entry(object): self.Raise("Please use 'offset' instead of 'pos'") self.offset = fdt_util.GetInt(self._node, 'offset') self.size = fdt_util.GetInt(self._node, 'size') + self.orig_offset, self.orig_size = self.offset, self.size self.align = fdt_util.GetInt(self._node, 'align') if tools.NotPowerOfTwo(self.align): raise ValueError("Node '%s': Alignment %s must be a power of two" % @@ -255,9 +258,16 @@ class Entry(object): ValueError if the new data size is not the same as the old """ size_ok = True - if len(data) != self.contents_size: + new_size = len(data) + if state.AllowEntryExpansion(): + if new_size > self.contents_size: + print("Entry '%s' size change from %#x to %#x" % ( + self._node.path, self.contents_size, new_size)) + # self.data will indicate the new size needed + size_ok = False + elif new_size != self.contents_size: self.Raise('Cannot update entry size from %d to %d' % - (self.contents_size, len(data))) + (self.contents_size, new_size)) self.SetContents(data) return size_ok

@@ -271,6 +281,10 @@ class Entry(object): # No contents by default: subclasses can implement this return True

+ def ResetForPack(self): + """Reset offset/size fields so that packing can be done again""" + self.offset, self.size = self.orig_offset, self.orig_size + def Pack(self, offset): """Figure out how to pack the entry into the section

diff --git a/tools/binman/etype/_testing.py b/tools/binman/etype/_testing.py index 2204362281c..ae24fe8105a 100644 --- a/tools/binman/etype/_testing.py +++ b/tools/binman/etype/_testing.py @@ -50,6 +50,8 @@ class Entry__testing(Entry): 'bad-update-contents') self.return_contents_once = fdt_util.GetBool(self._node, 'return-contents-once') + self.bad_update_contents_twice = fdt_util.GetBool(self._node, + 'bad-update-contents-twice')

# Set to True when the entry is ready to process the FDT. self.process_fdt_ready = False @@ -71,11 +73,12 @@ class Entry__testing(Entry): if self.force_bad_datatype: self.GetEntryArgsOrProps([EntryArg('test-bad-datatype-arg', bool)]) self.return_contents = True + self.contents = b'a'

def ObtainContents(self): if self.return_unknown_contents or not self.return_contents: return False - self.data = b'a' + self.data = self.contents self.contents_size = len(self.data) if self.return_contents_once: self.return_contents = False @@ -90,7 +93,11 @@ class Entry__testing(Entry): if self.bad_update_contents: # Request to update the contents with something larger, to cause a # failure. - return self.ProcessContentsUpdate('aa') + if self.bad_update_contents_twice: + self.contents += b'a' + else: + self.contents = b'aa' + return self.ProcessContentsUpdate(self.contents) return True

def ProcessFdt(self, fdt): diff --git a/tools/binman/etype/section.py b/tools/binman/etype/section.py index 51eddcd995a..23bf22113d4 100644 --- a/tools/binman/etype/section.py +++ b/tools/binman/etype/section.py @@ -64,6 +64,11 @@ class Entry_section(Entry): self._section.GetEntryOffsets() return {}

+ def ResetForPack(self): + """Reset offset/size fields so that packing can be done again""" + self._section.ResetForPack() + Entry.ResetForPack(self) + def Pack(self, offset): """Pack all entries into the section""" self._section.PackEntries() diff --git a/tools/binman/etype/u_boot_with_ucode_ptr.py b/tools/binman/etype/u_boot_with_ucode_ptr.py index 4104bf8bf13..cb7dbc68dbb 100644 --- a/tools/binman/etype/u_boot_with_ucode_ptr.py +++ b/tools/binman/etype/u_boot_with_ucode_ptr.py @@ -49,7 +49,7 @@ class Entry_u_boot_with_ucode_ptr(Entry_blob): def ProcessContents(self): # If the image does not need microcode, there is nothing to do if not self.target_offset: - return + return True

# Get the offset of the microcode ucode_entry = self.section.FindEntryType('u-boot-ucode') diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index 8c9942982ba..0ff8b5e2de8 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -1210,10 +1210,14 @@ class TestFunctional(unittest.TestCase):

def testBadChangeSize(self): """Test that trying to change the size of an entry fails""" - with self.assertRaises(ValueError) as e: - self._DoReadFile('059_change_size.dts', True) - self.assertIn("Node '/binman/_testing': Cannot update entry size from " - '1 to 2', str(e.exception)) + try: + state.SetAllowEntryExpansion(False) + with self.assertRaises(ValueError) as e: + self._DoReadFile('059_change_size.dts', True) + self.assertIn("Node '/binman/_testing': Cannot update entry size from 1 to 2", + str(e.exception)) + finally: + state.SetAllowEntryExpansion(True)

def testUpdateFdt(self): """Test that we can update the device tree with offset/size info""" @@ -2108,6 +2112,27 @@ class TestFunctional(unittest.TestCase): self.assertIn("Invalid location 'None', expected 'start' or 'end'", str(e.exception))

+ def testEntryExpand(self): + """Test expanding an entry after it is packed""" + data = self._DoReadFile('121_entry_expand.dts') + self.assertEqual('aa', data[:2]) + self.assertEqual(U_BOOT_DATA, data[2:2 + len(U_BOOT_DATA)]) + self.assertEqual('aa', data[-2:]) + + def testEntryExpandBad(self): + """Test expanding an entry after it is packed, twice""" + with self.assertRaises(ValueError) as e: + self._DoReadFile('122_entry_expand_twice.dts') + self.assertIn("Image '/binman': Entries expanded after packing", + str(e.exception)) + + def testEntryExpandSection(self): + """Test expanding an entry within a section after it is packed""" + data = self._DoReadFile('123_entry_expand_section.dts') + self.assertEqual('aa', data[:2]) + self.assertEqual(U_BOOT_DATA, data[2:2 + len(U_BOOT_DATA)]) + self.assertEqual('aa', data[-2:]) +

if __name__ == "__main__": unittest.main() diff --git a/tools/binman/image.py b/tools/binman/image.py index c8bce394aa1..6339d020e76 100644 --- a/tools/binman/image.py +++ b/tools/binman/image.py @@ -55,6 +55,10 @@ class Image: 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() @@ -100,6 +104,10 @@ class Image: """ 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() diff --git a/tools/binman/test/121_entry_expand.dts b/tools/binman/test/121_entry_expand.dts new file mode 100644 index 00000000000..ebb7816db90 --- /dev/null +++ b/tools/binman/test/121_entry_expand.dts @@ -0,0 +1,20 @@ +/dts-v1/; + +/ { + #address-cells = <1>; + #size-cells = <1>; + + binman { + _testing { + bad-update-contents; + }; + + u-boot { + }; + + _testing2 { + type = "_testing"; + bad-update-contents; + }; + }; +}; diff --git a/tools/binman/test/122_entry_expand_twice.dts b/tools/binman/test/122_entry_expand_twice.dts new file mode 100644 index 00000000000..258cf859f4b --- /dev/null +++ b/tools/binman/test/122_entry_expand_twice.dts @@ -0,0 +1,21 @@ +/dts-v1/; + +/ { + #address-cells = <1>; + #size-cells = <1>; + + binman { + _testing { + bad-update-contents; + bad-update-contents-twice; + }; + + u-boot { + }; + + _testing2 { + type = "_testing"; + bad-update-contents; + }; + }; +}; diff --git a/tools/binman/test/123_entry_expand_section.dts b/tools/binman/test/123_entry_expand_section.dts new file mode 100644 index 00000000000..046f7234348 --- /dev/null +++ b/tools/binman/test/123_entry_expand_section.dts @@ -0,0 +1,22 @@ +/dts-v1/; + +/ { + #address-cells = <1>; + #size-cells = <1>; + + binman { + _testing { + bad-update-contents; + }; + + u-boot { + }; + + section { + _testing2 { + type = "_testing"; + bad-update-contents; + }; + }; + }; +};

At present a file with no explicit CBFS offset is placed in the next available location but there is no way to find out where it ended up. Update and rename the get_data() function to provide this information.

Signed-off-by: Simon Glass sjg@chromium.org ---

tools/binman/cbfs_util.py | 31 ++++++++++++++++++++----------- tools/binman/cbfs_util_test.py | 12 ++++++++++++ 2 files changed, 32 insertions(+), 11 deletions(-)

diff --git a/tools/binman/cbfs_util.py b/tools/binman/cbfs_util.py index 1cdbcb2339e..530629a5c96 100644 --- a/tools/binman/cbfs_util.py +++ b/tools/binman/cbfs_util.py @@ -185,7 +185,8 @@ class CbfsFile(object): """Class to represent a single CBFS file

This is used to hold the information about a file, including its contents. - Use the get_data() method to obtain the raw output for writing to CBFS. + Use the get_data_and_offset() method to obtain the raw output for writing to + CBFS.

Properties: name: Name of file @@ -319,12 +320,15 @@ class CbfsFile(object): raise ValueError('Unknown file type %#x\n' % self.ftype) return hdr_len

- def get_data(self, offset=None, pad_byte=None): - """Obtain the contents of the file, in CBFS format + def get_data_and_offset(self, offset=None, pad_byte=None): + """Obtain the contents of the file, in CBFS format and the offset of + the data within the file

Returns: - bytes representing the contents of this file, packed and aligned - for directly inserting into the final CBFS output + tuple: + bytes representing the contents of this file, packed and aligned + for directly inserting into the final CBFS output + offset to the file data from the start of the returned data. """ name = _pack_string(self.name) hdr_len = len(name) + FILE_HEADER_LEN @@ -368,8 +372,10 @@ class CbfsFile(object): (self.name, self.cbfs_offset, offset)) pad = tools.GetBytes(pad_byte, pad_len) hdr_len += pad_len - self.offset = len(content) + len(data) - hdr = struct.pack(FILE_HEADER_FORMAT, FILE_MAGIC, self.offset, + + # This is the offset of the start of the file's data, + size = len(content) + len(data) + hdr = struct.pack(FILE_HEADER_FORMAT, FILE_MAGIC, size, self.ftype, attr_pos, hdr_len)

# Do a sanity check of the get_header_len() function, to ensure that it @@ -381,7 +387,7 @@ class CbfsFile(object): # happen. It probably indicates that get_header_len() is broken. raise ValueError("Internal error: CBFS file '%s': Expected headers of %#x bytes, got %#d" % (self.name, expected_len, actual_len)) - return hdr + name + attr + pad + content + data + return hdr + name + attr + pad + content + data, hdr_len

class CbfsWriter(object): @@ -392,7 +398,7 @@ class CbfsWriter(object): cbw = CbfsWriter(size) cbw.add_file_raw('u-boot', tools.ReadFile('u-boot.bin')) ... - data = cbw.get_data() + data, cbfs_offset = cbw.get_data_and_offset()

Attributes: _master_name: Name of the file containing the master header @@ -475,7 +481,7 @@ class CbfsWriter(object): todo = align_int_down(offset - upto, self._align) if todo: cbf = CbfsFile.empty(todo, self._erase_byte) - fd.write(cbf.get_data()) + fd.write(cbf.get_data_and_offset()[0]) self._skip_to(fd, offset)

def _align_to(self, fd, align): @@ -579,8 +585,11 @@ class CbfsWriter(object): offset = cbf.calc_start_offset() if offset is not None: self._pad_to(fd, align_int_down(offset, self._align)) - fd.write(cbf.get_data(fd.tell(), self._erase_byte)) + pos = fd.tell() + data, data_offset = cbf.get_data_and_offset(pos, self._erase_byte) + fd.write(data) self._align_to(fd, self._align) + cbf.calced_cbfs_offset = pos + data_offset if not self._hdr_at_start: self._write_header(fd, add_fileheader=self._add_fileheader)

diff --git a/tools/binman/cbfs_util_test.py b/tools/binman/cbfs_util_test.py index e77f5c51c98..894c0c383b1 100755 --- a/tools/binman/cbfs_util_test.py +++ b/tools/binman/cbfs_util_test.py @@ -595,6 +595,18 @@ class TestCbfs(unittest.TestCase): cbw.get_data() self.assertIn('No space for data before pad offset', str(e.exception))

+ def test_cbfs_check_offset(self): + """Test that we can discover the offset of a file after writing it""" + size = 0xb0 + cbw = CbfsWriter(size) + cbw.add_file_raw('u-boot', U_BOOT_DATA) + cbw.add_file_raw('u-boot-dtb', U_BOOT_DTB_DATA) + data = cbw.get_data() + + cbfs = cbfs_util.CbfsReader(data) + self.assertEqual(0x38, cbfs.files['u-boot'].cbfs_offset) + self.assertEqual(0x78, cbfs.files['u-boot-dtb'].cbfs_offset) +

if __name__ == '__main__': unittest.main()

Most of the CBFS functionality is tested by cbfs_util rather than the binman functional tests. Add a comment to that effect.

Signed-off-by: Simon Glass sjg@chromium.org ---

tools/binman/ftest.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index b1dc1643011..bee4feb1c3a 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -2003,7 +2003,11 @@ class TestFunctional(unittest.TestCase): str(e.exception))

def testCbfsOffset(self): - """Test a CBFS with files at particular offsets""" + """Test a CBFS with files at particular offsets + + Like all CFBS tests, this is just checking the logic that calls + cbfs_util. See cbfs_util_test for fully tests (e.g. test_cbfs_offset()). + """ data = self._DoReadFile('114_cbfs_offset.dts') size = 0x200

Detect when an unknown or unsupported file type is specified and report an error.

Signed-off-by: Simon Glass sjg@chromium.org ---

tools/binman/etype/cbfs.py | 3 +++ tools/binman/ftest.py | 6 ++++++ tools/binman/test/126_cbfs_bad_type.dts | 17 +++++++++++++++++ 3 files changed, 26 insertions(+) create mode 100644 tools/binman/test/126_cbfs_bad_type.dts

diff --git a/tools/binman/etype/cbfs.py b/tools/binman/etype/cbfs.py index a46bb98a033..175ecae1584 100644 --- a/tools/binman/etype/cbfs.py +++ b/tools/binman/etype/cbfs.py @@ -190,6 +190,9 @@ class Entry_cbfs(Entry): elif entry._type == 'stage': cfile = cbfs.add_file_stage(entry._cbfs_name, data, entry._cbfs_offset) + else: + entry.Raise("Unknown cbfs-type '%s' (use 'raw', 'stage')" % + entry._type) if cfile: entry._cbfs_file = cfile entry.size = cfile.data_len diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index 9cdbf798b87..4f29a294e61 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -2179,6 +2179,12 @@ class TestFunctional(unittest.TestCase): 'cbfs/u-boot-dtb:image-pos': 0xb8, }, props)

+ def testCbfsBadType(self): + """Test an image header with a no specified location is detected""" + with self.assertRaises(ValueError) as e: + self._DoReadFile('126_cbfs_bad_type.dts') + self.assertIn("Unknown cbfs-type 'badtype'", str(e.exception)) +

if __name__ == "__main__": unittest.main() diff --git a/tools/binman/test/126_cbfs_bad_type.dts b/tools/binman/test/126_cbfs_bad_type.dts new file mode 100644 index 00000000000..2cd6fc6d52d --- /dev/null +++ b/tools/binman/test/126_cbfs_bad_type.dts @@ -0,0 +1,17 @@ +// SPDX-License-Identifier: GPL-2.0+ + +/dts-v1/; + +/ { + #address-cells = <1>; + #size-cells = <1>; + + binman { + cbfs { + size = <0x100>; + u-boot { + cbfs-type = "badtype"; + }; + }; + }; +};

Add support for reading in an image and locating its Fdt map.

Signed-off-by: Simon Glass sjg@chromium.org ---

tools/binman/etype/fdtmap.py | 22 +++++++++++++++- tools/binman/ftest.py | 17 ++++++++++++ tools/binman/test/128_decode_image.dts | 36 ++++++++++++++++++++++++++ 3 files changed, 74 insertions(+), 1 deletion(-) create mode 100644 tools/binman/test/128_decode_image.dts

diff --git a/tools/binman/etype/fdtmap.py b/tools/binman/etype/fdtmap.py index bfd7962be3a..4fe5f8d9e10 100644 --- a/tools/binman/etype/fdtmap.py +++ b/tools/binman/etype/fdtmap.py @@ -15,7 +15,27 @@ from fdt import Fdt import state import tools

-FDTMAP_MAGIC = b'_FDTMAP_' +FDTMAP_MAGIC = b'_FDTMAP_' +FDTMAP_HDR_LEN = 16 + +def LocateFdtmap(data): + """Search an image for an fdt map + + Args: + data: Data to search + + Returns: + Position of fdt map in data, or None if not found. Note that the + position returned is of the FDT map data itself, i.e. after the + header + """ + hdr_pos = data.find(FDTMAP_MAGIC) + size = len(data) + if hdr_pos: + hdr = data[hdr_pos:hdr_pos + FDTMAP_HDR_LEN] + if len(hdr) == FDTMAP_HDR_LEN: + return hdr_pos + FDTMAP_HDR_LEN + return None

class Entry_fdtmap(Entry): """An entry which contains an FDT map diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index 9552c3b2761..2536d03c374 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -24,6 +24,7 @@ import command import control import elf import fdt +from etype import fdtmap import fdt_util import fmap_util import test_util @@ -2260,5 +2261,21 @@ class TestFunctional(unittest.TestCase): self.assertEqual(len(data), 0x100 + section_size) self.assertEqual(section_size, 0x400 + dtb_size)

+ def testFindFdtmap(self): + """Test locating an FDT map in an image""" + data = self._DoReadFileDtb('128_decode_image.dts', use_real_dtb=True, + update_dtb=True)[0] + image = control.images['image'] + entries = image.GetEntries() + entry = entries['fdtmap'] + self.assertEqual(entry.image_pos + fdtmap.FDTMAP_HDR_LEN, + fdtmap.LocateFdtmap(data)) + + def testFindFdtmapMissing(self): + """Test failing to locate an FDP map""" + data = self._DoReadFile('005_simple.dts') + self.assertEqual(None, fdtmap.LocateFdtmap(data)) + + if __name__ == "__main__": unittest.main() diff --git a/tools/binman/test/128_decode_image.dts b/tools/binman/test/128_decode_image.dts new file mode 100644 index 00000000000..449fccc41df --- /dev/null +++ b/tools/binman/test/128_decode_image.dts @@ -0,0 +1,36 @@ +// SPDX-License-Identifier: GPL-2.0+ + +/dts-v1/; + +/ { + #address-cells = <1>; + #size-cells = <1>; + + binman { + size = <0xc00>; + u-boot { + }; + section { + align = <0x100>; + cbfs { + size = <0x400>; + u-boot { + cbfs-type = "raw"; + }; + u-boot-dtb { + cbfs-type = "raw"; + cbfs-compress = "lzma"; + cbfs-offset = <0x80>; + }; + }; + u-boot-dtb { + compress = "lz4"; + }; + }; + fdtmap { + }; + image-header { + location = "end"; + }; + }; +};

It is possible to read an Image, locate its FDT map and then read it into the binman data structures. This allows full access to the entries that were written to the image. Add support for this.

Signed-off-by: Simon Glass sjg@chromium.org ---

tools/binman/entry.py | 6 ++++ tools/binman/ftest.py | 33 +++++++++++++++++ tools/binman/image.py | 36 +++++++++++++++++++ tools/binman/test/128_decode_image_no_hdr.dts | 33 +++++++++++++++++ tools/binman/test/129_list_fdtmap.dts | 36 +++++++++++++++++++ 5 files changed, 144 insertions(+) create mode 100644 tools/binman/test/128_decode_image_no_hdr.dts create mode 100644 tools/binman/test/129_list_fdtmap.dts

diff --git a/tools/binman/entry.py b/tools/binman/entry.py index a39b316b3eb..1fe905ae904 100644 --- a/tools/binman/entry.py +++ b/tools/binman/entry.py @@ -159,6 +159,12 @@ class Entry(object): self.offset = fdt_util.GetInt(self._node, 'offset') self.size = fdt_util.GetInt(self._node, 'size') self.orig_offset, self.orig_size = self.offset, self.size + + # These should not be set in input files, but are set in an FDT map, + # which is also read by this code. + self.image_pos = fdt_util.GetInt(self._node, 'image-pos') + self.uncomp_size = fdt_util.GetInt(self._node, 'uncomp-size') + self.align = fdt_util.GetInt(self._node, 'align') if tools.NotPowerOfTwo(self.align): raise ValueError("Node '%s': Alignment %s must be a power of two" % diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index 15cf5cea457..88c3d847f82 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -30,6 +30,7 @@ import fdt_util import fmap_util import test_util import gzip +from image import Image import state import tools import tout @@ -2302,6 +2303,38 @@ class TestFunctional(unittest.TestCase): data = self._DoReadFile('005_simple.dts') self.assertEqual(None, image_header.LocateHeaderOffset(data))

+ def testReadImage(self): + """Test reading an image and accessing its FDT map""" + data = self._DoReadFileDtb('128_decode_image.dts', + use_real_dtb=True, update_dtb=True)[0] + image_fname = tools.GetOutputFilename('image.bin') + orig_image = control.images['image'] + image = Image.FromFile(image_fname) + self.assertEqual(orig_image.GetEntries().keys(), + image.GetEntries().keys()) + + orig_entry = orig_image.GetEntries()['fdtmap'] + entry = image.GetEntries()['fdtmap'] + self.assertEquals(orig_entry.offset, entry.offset) + self.assertEquals(orig_entry.size, entry.size) + self.assertEquals(orig_entry.image_pos, entry.image_pos) + + def testReadImageNoHeader(self): + """Test accessing an image's FDT map without an image header""" + data = self._DoReadFileDtb('128_decode_image_no_hdr.dts', + use_real_dtb=True, update_dtb=True)[0] + image_fname = tools.GetOutputFilename('image.bin') + image = Image.FromFile(image_fname) + self.assertTrue(isinstance(image, Image)) + self.assertEqual('image', image._name) + + def testReadImageFail(self): + """Test failing to read an image image's FDT map""" + self._DoReadFile('005_simple.dts') + image_fname = tools.GetOutputFilename('image.bin') + image = Image.FromFile(image_fname) + self.assertTrue(isinstance(image, str)) + self.assertEqual('Cannot find FDT map in image', image)

if __name__ == "__main__": unittest.main() diff --git a/tools/binman/image.py b/tools/binman/image.py index d4145685972..d918b5a7194 100644 --- a/tools/binman/image.py +++ b/tools/binman/image.py @@ -12,6 +12,9 @@ from operator import attrgetter import re import sys

+from etype import fdtmap +from etype import image_header +import fdt import fdt_util import bsection import tools @@ -47,6 +50,39 @@ class Image: else: self._ReadNode()

+ @classmethod + def FromFile(cls, fname): + """Convert an image file into an Image for use in binman + + Args: + fname: Filename of image file to read + + Returns: + Image object on success, or string error message on failure + """ + data = tools.ReadFile(fname) + size = len(data) + + # First look for an image header + pos = image_header.LocateHeaderOffset(data) + if pos is None: + # Look for the FDT map + pos = fdtmap.LocateFdtmap(data) + else: + # Move past the header to the FDT + pos += fdtmap.FDTMAP_HDR_LEN + if pos is None: + return 'Cannot find FDT map in image' + + # We don't knowe the FDT size, so check its header first + probe_dtb = fdt.Fdt.FromData(data[pos:pos + 256]) + dtb_size = probe_dtb.GetFdtObj().totalsize() + dtb = fdt.Fdt.FromData(data[pos:pos + dtb_size]) + dtb.Scan() + + # 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') diff --git a/tools/binman/test/128_decode_image_no_hdr.dts b/tools/binman/test/128_decode_image_no_hdr.dts new file mode 100644 index 00000000000..90fdd8820ca --- /dev/null +++ b/tools/binman/test/128_decode_image_no_hdr.dts @@ -0,0 +1,33 @@ +// SPDX-License-Identifier: GPL-2.0+ + +/dts-v1/; + +/ { + #address-cells = <1>; + #size-cells = <1>; + + binman { + size = <0xc00>; + u-boot { + }; + section { + align = <0x100>; + cbfs { + size = <0x400>; + u-boot { + cbfs-type = "raw"; + }; + u-boot-dtb { + cbfs-type = "raw"; + cbfs-compress = "lzma"; + cbfs-offset = <0x80>; + }; + }; + u-boot-dtb { + compress = "lz4"; + }; + }; + fdtmap { + }; + }; +}; diff --git a/tools/binman/test/129_list_fdtmap.dts b/tools/binman/test/129_list_fdtmap.dts new file mode 100644 index 00000000000..449fccc41df --- /dev/null +++ b/tools/binman/test/129_list_fdtmap.dts @@ -0,0 +1,36 @@ +// SPDX-License-Identifier: GPL-2.0+ + +/dts-v1/; + +/ { + #address-cells = <1>; + #size-cells = <1>; + + binman { + size = <0xc00>; + u-boot { + }; + section { + align = <0x100>; + cbfs { + size = <0x400>; + u-boot { + cbfs-type = "raw"; + }; + u-boot-dtb { + cbfs-type = "raw"; + cbfs-compress = "lzma"; + cbfs-offset = <0x80>; + }; + }; + u-boot-dtb { + compress = "lz4"; + }; + }; + fdtmap { + }; + image-header { + location = "end"; + }; + }; +};