Pull this support from these upstream commits:

bfbfab0 pylibfdt: Add a means to add and delete notes 9005f41 pylibfdt: Allow delprop() to return errors

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

scripts/dtc/pylibfdt/libfdt.i_shipped | 34 ++++++++++++++++++++++----- 1 file changed, 28 insertions(+), 6 deletions(-)

diff --git a/scripts/dtc/pylibfdt/libfdt.i_shipped b/scripts/dtc/pylibfdt/libfdt.i_shipped index 0f17c5879e7..76e61e98bdf 100644 --- a/scripts/dtc/pylibfdt/libfdt.i_shipped +++ b/scripts/dtc/pylibfdt/libfdt.i_shipped @@ -628,28 +628,50 @@ class Fdt(FdtRo): return check_err(fdt_setprop(self._fdt, nodeoffset, prop_name, val, len(val)), quiet)

- def delprop(self, nodeoffset, prop_name): + def delprop(self, nodeoffset, prop_name, quiet=()): """Delete a property from a node

Args: nodeoffset: Node offset containing property to delete prop_name: Name of property to delete + quiet: Errors to ignore (empty to raise on all errors) + + Returns: + Error code, or 0 if OK

Raises: FdtError if the property does not exist, or another error occurs """ - return check_err(fdt_delprop(self._fdt, nodeoffset, prop_name)) + return check_err(fdt_delprop(self._fdt, nodeoffset, prop_name), quiet) + + def add_subnode(self, parentoffset, name, quiet=()): + """Add a new subnode to a node

- def del_node(self, nodeoffset): + Args: + parentoffset: Parent offset to add the subnode to + name: Name of node to add + + Returns: + offset of the node created, or negative error code on failure + + Raises: + FdtError if there is not enough space, or another error occurs + """ + return check_err(fdt_add_subnode(self._fdt, parentoffset, name), quiet) + + def del_node(self, nodeoffset, quiet=()): """Delete a node

Args: - nodeoffset: Node offset containing property to delete + nodeoffset: Offset of node to delete + + Returns: + Error code, or 0 if OK

Raises: - FdtError if the node does not exist, or another error occurs + FdtError if an error occurs """ - return check_err(fdt_del_node(self._fdt, nodeoffset)) + return check_err(fdt_del_node(self._fdt, nodeoffset), quiet)

class Property(bytearray):

This information should be in the entry it relates to, not in the main README. Move it.

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

tools/binman/README | 11 ----------- tools/binman/README.entries | 5 ++++- tools/binman/etype/u_boot_with_ucode_ptr.py | 3 +++ 3 files changed, 7 insertions(+), 12 deletions(-)

diff --git a/tools/binman/README b/tools/binman/README index cb34171e5fc..3dc8b9b80a8 100644 --- a/tools/binman/README +++ b/tools/binman/README @@ -462,17 +462,6 @@ see README.entries. This is generated from the source code using: binman -E >tools/binman/README.entries

-Special properties ------------------- - -Some entries support special properties, documented here: - -u-boot-with-ucode-ptr: - optional-ucode: boolean property to make microcode optional. If the - u-boot.bin image does not include microcode, no error will - be generated. - - Order of image creation -----------------------

diff --git a/tools/binman/README.entries b/tools/binman/README.entries index c6e7b226090..041e77771ed 100644 --- a/tools/binman/README.entries +++ b/tools/binman/README.entries @@ -524,6 +524,9 @@ Entry: u-boot-with-ucode-ptr: U-Boot with embedded microcode pointer

Properties / Entry arguments: - filename: Filename of u-boot-nodtb.dtb (default 'u-boot-nodtb.dtb') + - optional-ucode: boolean property to make microcode optional. If the + u-boot.bin image does not include microcode, no error will + be generated.

See Entry_u_boot_ucode for full details of the three entries involved in this process. This entry updates U-Boot with the offset and size of the @@ -543,7 +546,7 @@ Properties / Entry arguments: - kernelkey: Name of the kernel key to use (inside keydir) - preamble-flags: Value of the vboot preamble flags (typically 0)

-Chromium OS signs the read-write firmware and kernel, writing the signature +Chromium OS signs the read-write firmware and kernel, writing the signature in this block. This allows U-Boot to verify that the next firmware stage and kernel are genuine.

diff --git a/tools/binman/etype/u_boot_with_ucode_ptr.py b/tools/binman/etype/u_boot_with_ucode_ptr.py index 51e7ba48f59..c850b59a1e1 100644 --- a/tools/binman/etype/u_boot_with_ucode_ptr.py +++ b/tools/binman/etype/u_boot_with_ucode_ptr.py @@ -19,6 +19,9 @@ class Entry_u_boot_with_ucode_ptr(Entry_blob):

Properties / Entry arguments: - filename: Filename of u-boot-nodtb.dtb (default 'u-boot-nodtb.dtb') + - optional-ucode: boolean property to make microcode optional. If the + u-boot.bin image does not include microcode, no error will + be generated.

See Entry_u_boot_ucode for full details of the three entries involved in this process. This entry updates U-Boot with the offset and size of the

The check for this should be for None, not 0. Fix it and add a test.

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

tools/binman/etype/fill.py | 2 +- tools/binman/ftest.py | 5 +++++ tools/binman/test/80_fill_empty.dts | 15 +++++++++++++++ 3 files changed, 21 insertions(+), 1 deletion(-) create mode 100644 tools/binman/test/80_fill_empty.dts

diff --git a/tools/binman/etype/fill.py b/tools/binman/etype/fill.py index 7210a8324a0..dcfe978a5bf 100644 --- a/tools/binman/etype/fill.py +++ b/tools/binman/etype/fill.py @@ -23,7 +23,7 @@ class Entry_fill(Entry): """ def __init__(self, section, etype, node): Entry.__init__(self, section, etype, node) - if not self.size: + if self.size is None: self.Raise("'fill' entry must have a size property") self.fill_value = fdt_util.GetByte(self._node, 'fill-byte', 0)

diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index a8456c26157..7f82264f8ad 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -1364,6 +1364,11 @@ class TestFunctional(unittest.TestCase): self.assertIn("Node '/binman/u-boot': Please use 'offset' instead of " "'pos'", str(e.exception))

+ def testFillZero(self): + """Test for an fill entry type with a size of 0""" + data = self._DoReadFile('80_fill_empty.dts') + self.assertEqual(chr(0) * 16, data) +

if __name__ == "__main__": unittest.main() diff --git a/tools/binman/test/80_fill_empty.dts b/tools/binman/test/80_fill_empty.dts new file mode 100644 index 00000000000..2b78d3ae88d --- /dev/null +++ b/tools/binman/test/80_fill_empty.dts @@ -0,0 +1,15 @@ +// SPDX-License-Identifier: GPL-2.0+ +/dts-v1/; + +/ { + #address-cells = <1>; + #size-cells = <1>; + + binman { + size = <16>; + fill { + size = <0>; + fill-byte = [ff]; + }; + }; +};

When the value of a text entry is not provided an execption is generated talking about a None type. This is confusing. Add a more explanatory error and a test for this case.

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

tools/binman/etype/text.py | 3 +++ tools/binman/ftest.py | 7 +++++++ 2 files changed, 10 insertions(+)

diff --git a/tools/binman/etype/text.py b/tools/binman/etype/text.py index 7a1cddf4af8..6e99819487f 100644 --- a/tools/binman/etype/text.py +++ b/tools/binman/etype/text.py @@ -51,6 +51,9 @@ class Entry_text(Entry): self.text_label, = self.GetEntryArgsOrProps( [EntryArg('text-label', str)]) self.value, = self.GetEntryArgsOrProps([EntryArg(self.text_label, str)]) + if not self.value: + self.Raise("No value provided for text label '%s'" % + self.text_label)

def ObtainContents(self): self.SetContents(self.value) diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index 7f82264f8ad..d956bd42e1b 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -1369,6 +1369,13 @@ class TestFunctional(unittest.TestCase): data = self._DoReadFile('80_fill_empty.dts') self.assertEqual(chr(0) * 16, data)

+ def testTextMissing(self): + """Test for a text entry type where there is no text""" + with self.assertRaises(ValueError) as e: + self._DoReadFileDtb('66_text.dts',) + self.assertIn("Node '/binman/text': No value provided for text label " + "'test-id'", str(e.exception)) +

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

Sometimes we want to include TPL for x86 platforms, such as when we want to select between different SPL images (e.g. for Chrome OS verified boot). Add support for this.

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

tools/binman/README.entries | 17 ++++++++++++++ tools/binman/etype/x86_start16_tpl.py | 30 ++++++++++++++++++++++++ tools/binman/ftest.py | 10 +++++++- tools/binman/test/81_x86-start16-tpl.dts | 14 +++++++++++ 4 files changed, 70 insertions(+), 1 deletion(-) create mode 100644 tools/binman/etype/x86_start16_tpl.py create mode 100644 tools/binman/test/81_x86-start16-tpl.dts

diff --git a/tools/binman/README.entries b/tools/binman/README.entries index 041e77771ed..31bc725d577 100644 --- a/tools/binman/README.entries +++ b/tools/binman/README.entries @@ -586,3 +586,20 @@ For 32-bit U-Boot, the 'x86_start16' entry type is used instead.

+Entry: x86-start16-tpl: x86 16-bit start-up code for TPL +-------------------------------------------------------- + +Properties / Entry arguments: + - filename: Filename of tpl/u-boot-x86-16bit-tpl.bin (default + 'tpl/u-boot-x86-16bit-tpl.bin') + +x86 CPUs start up in 16-bit mode, even if they are 64-bit CPUs. This code +must be placed at a particular address. This entry holds that code. It is +typically placed at offset CONFIG_SYS_X86_START16. The code is responsible +for changing to 32-bit mode and starting TPL, which in turn jumps to SPL. + +If TPL is not being used, the 'x86_start16_spl or 'x86_start16' entry types +may be used instead. + + + diff --git a/tools/binman/etype/x86_start16_tpl.py b/tools/binman/etype/x86_start16_tpl.py new file mode 100644 index 00000000000..46ce169ae0a --- /dev/null +++ b/tools/binman/etype/x86_start16_tpl.py @@ -0,0 +1,30 @@ +# SPDX-License-Identifier: GPL-2.0+ +# Copyright (c) 2018 Google, Inc +# Written by Simon Glass sjg@chromium.org +# +# Entry-type module for the 16-bit x86 start-up code for U-Boot TPL +# + +from entry import Entry +from blob import Entry_blob + +class Entry_x86_start16_tpl(Entry_blob): + """x86 16-bit start-up code for TPL + + Properties / Entry arguments: + - filename: Filename of tpl/u-boot-x86-16bit-tpl.bin (default + 'tpl/u-boot-x86-16bit-tpl.bin') + + x86 CPUs start up in 16-bit mode, even if they are 64-bit CPUs. This code + must be placed at a particular address. This entry holds that code. It is + typically placed at offset CONFIG_SYS_X86_START16. The code is responsible + for changing to 32-bit mode and starting TPL, which in turn jumps to SPL. + + If TPL is not being used, the 'x86_start16_spl or 'x86_start16' entry types + may be used instead. + """ + def __init__(self, section, etype, node): + Entry_blob.__init__(self, section, etype, node) + + def GetDefaultFilename(self): + return 'tpl/u-boot-x86-16bit-tpl.bin' diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index d956bd42e1b..3f4f5f3a43a 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -39,6 +39,7 @@ U_BOOT_SPL_DTB_DATA = 'spldtb' U_BOOT_TPL_DTB_DATA = 'tpldtb' X86_START16_DATA = 'start16' X86_START16_SPL_DATA = 'start16spl' +X86_START16_TPL_DATA = 'start16tpl' U_BOOT_NODTB_DATA = 'nodtb with microcode pointer somewhere in here' U_BOOT_SPL_NODTB_DATA = 'splnodtb with microcode pointer somewhere in here' FSP_DATA = 'fsp' @@ -92,6 +93,8 @@ class TestFunctional(unittest.TestCase): TestFunctional._MakeInputFile('u-boot-x86-16bit.bin', X86_START16_DATA) TestFunctional._MakeInputFile('spl/u-boot-x86-16bit-spl.bin', X86_START16_SPL_DATA) + TestFunctional._MakeInputFile('tpl/u-boot-x86-16bit-tpl.bin', + X86_START16_TPL_DATA) TestFunctional._MakeInputFile('u-boot-nodtb.bin', U_BOOT_NODTB_DATA) TestFunctional._MakeInputFile('spl/u-boot-spl-nodtb.bin', U_BOOT_SPL_NODTB_DATA) @@ -964,7 +967,7 @@ class TestFunctional(unittest.TestCase): str(e.exception))

def testPackStart16Spl(self): - """Test that an image with an x86 start16 region can be created""" + """Test that an image with an x86 start16 SPL region can be created""" data = self._DoReadFile('48_x86-start16-spl.dts') self.assertEqual(X86_START16_SPL_DATA, data[:len(X86_START16_SPL_DATA)])

@@ -1376,6 +1379,11 @@ class TestFunctional(unittest.TestCase): self.assertIn("Node '/binman/text': No value provided for text label " "'test-id'", str(e.exception))

+ def testPackStart16Tpl(self): + """Test that an image with an x86 start16 TPL region can be created""" + data = self._DoReadFile('81_x86-start16-tpl.dts') + self.assertEqual(X86_START16_TPL_DATA, data[:len(X86_START16_TPL_DATA)]) +

if __name__ == "__main__": unittest.main() diff --git a/tools/binman/test/81_x86-start16-tpl.dts b/tools/binman/test/81_x86-start16-tpl.dts new file mode 100644 index 00000000000..68e6bbd68f0 --- /dev/null +++ b/tools/binman/test/81_x86-start16-tpl.dts @@ -0,0 +1,14 @@ +// SPDX-License-Identifier: GPL-2.0+ +/dts-v1/; + +/ { + #address-cells = <1>; + #size-cells = <1>; + + binman { + size = <16>; + + x86-start16-tpl { + }; + }; +};

At present if there are two vblock entries an image their contents are written to the same file in the output directory. This prevents checking the contents of each separately.

Fix this by adding part of the entry path to the filename, and add some missing comments.

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

tools/binman/README.entries | 5 +++++ tools/binman/entry.py | 18 ++++++++++++++++++ tools/binman/entry_test.py | 11 +++++++++++ tools/binman/etype/vblock.py | 11 ++++++++--- tools/binman/ftest.py | 2 +- 5 files changed, 43 insertions(+), 4 deletions(-)

diff --git a/tools/binman/README.entries b/tools/binman/README.entries index 31bc725d577..5cb52a92ff9 100644 --- a/tools/binman/README.entries +++ b/tools/binman/README.entries @@ -546,6 +546,11 @@ Properties / Entry arguments: - kernelkey: Name of the kernel key to use (inside keydir) - preamble-flags: Value of the vboot preamble flags (typically 0)

+Output files: + - input.<unique_name> - input file passed to futility + - vblock.<unique_name> - output file generated by futility (which is + used as the entry contents) + Chromium OS signs the read-write firmware and kernel, writing the signature in this block. This allows U-Boot to verify that the next firmware stage and kernel are genuine. diff --git a/tools/binman/entry.py b/tools/binman/entry.py index 77cfab9c5de..e671a2ea094 100644 --- a/tools/binman/entry.py +++ b/tools/binman/entry.py @@ -456,3 +456,21 @@ features to produce new behaviours. if missing: raise ValueError('Documentation is missing for modules: %s' % ', '.join(missing)) + + def GetUniqueName(self): + """Get a unique name for a node + + Returns: + String containing a unique name for a node, consisting of the name + of all ancestors (starting from within the 'binman' node) separated + by a dot ('.'). This can be useful for generating unique filesnames + in the output directory. + """ + name = self.name + node = self._node + while node.parent: + node = node.parent + if node.name == 'binman': + break + name = '%s.%s' % (node.name, name) + return name diff --git a/tools/binman/entry_test.py b/tools/binman/entry_test.py index 6fa735ed596..4100bcc3d32 100644 --- a/tools/binman/entry_test.py +++ b/tools/binman/entry_test.py @@ -54,6 +54,17 @@ class TestEntry(unittest.TestCase): self.assertIn("Unknown entry type 'invalid-name' in node " "'invalid-path'", str(e.exception))

+ 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) + self.assertEqual('root', base_entry.GetUniqueName()) + sub_node = Node('subnode', base_node) + sub_entry = entry.Entry(None, None, sub_node, read_node=False) + self.assertEqual('root.subnode', sub_entry.GetUniqueName()) +

if __name__ == "__main__": unittest.main() diff --git a/tools/binman/etype/vblock.py b/tools/binman/etype/vblock.py index 595af5456d1..c4d970ed160 100644 --- a/tools/binman/etype/vblock.py +++ b/tools/binman/etype/vblock.py @@ -25,6 +25,11 @@ class Entry_vblock(Entry): - kernelkey: Name of the kernel key to use (inside keydir) - preamble-flags: Value of the vboot preamble flags (typically 0)

+ Output files: + - input.<unique_name> - input file passed to futility + - vblock.<unique_name> - output file generated by futility (which is + used as the entry contents) + Chromium OS signs the read-write firmware and kernel, writing the signature in this block. This allows U-Boot to verify that the next firmware stage and kernel are genuine. @@ -53,8 +58,9 @@ class Entry_vblock(Entry): return False input_data += data

- output_fname = tools.GetOutputFilename('vblock.%s' % self.name) - input_fname = tools.GetOutputFilename('input.%s' % self.name) + uniq = self.GetUniqueName() + output_fname = tools.GetOutputFilename('vblock.%s' % uniq) + input_fname = tools.GetOutputFilename('input.%s' % uniq) tools.WriteFile(input_fname, input_data) prefix = self.keydir + '/' args = [ @@ -69,6 +75,5 @@ class Entry_vblock(Entry): ] #out.Notice("Sign '%s' into %s" % (', '.join(self.value), self.label)) stdout = tools.Run('futility', *args) - #out.Debug(stdout) self.SetContents(tools.ReadFile(output_fname)) return True diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index 3f4f5f3a43a..c4065551e79 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -1316,7 +1316,7 @@ class TestFunctional(unittest.TestCase): """Fake calls to the futility utility""" if pipe_list[0][0] == 'futility': fname = pipe_list[0][3] - with open(fname, 'w') as fd: + with open(fname, 'wb') as fd: fd.write(VBLOCK_DATA) return command.CommandResult()

Sometimes it is useful to build only a subset of the images provided by the binman configuration. Add a -i option for this. It can be given multiple times to build several images. If the option is not given, all images are built.

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

tools/binman/cmdline.py | 2 ++ tools/binman/control.py | 9 +++++++++ tools/binman/ftest.py | 19 ++++++++++++++++++- 3 files changed, 29 insertions(+), 1 deletion(-)

diff --git a/tools/binman/cmdline.py b/tools/binman/cmdline.py index f0de4ded443..4ce8bc6ab43 100644 --- a/tools/binman/cmdline.py +++ b/tools/binman/cmdline.py @@ -30,6 +30,8 @@ def ParseArgs(argv): 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('-i', '--image', type='string', action='append', + help='Image filename to build (if not specified, build all)') parser.add_option('-I', '--indir', action='append', help='Add a path to a directory to use for input files') parser.add_option('-H', '--full-help', action='store_true', diff --git a/tools/binman/control.py b/tools/binman/control.py index 2de1c86ecfe..8c48008fc75 100644 --- a/tools/binman/control.py +++ b/tools/binman/control.py @@ -162,6 +162,15 @@ def Binman(options, args):

images = _ReadImageDesc(node)

+ if options.image: + skip = [] + for name, image in images.iteritems(): + if name not in options.image: + del images[name] + skip.append(name) + if skip: + print 'Skipping images: %s\n' % ', '.join(skip) + # Prepare the device tree by making sure that any missing # properties are added (e.g. 'pos' and 'size'). The values of these # may not be correct yet, but we add placeholders so that the diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index c4065551e79..290e9aebf16 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -171,7 +171,7 @@ class TestFunctional(unittest.TestCase): return control.Binman(options, args)

def _DoTestFile(self, fname, debug=False, map=False, update_dtb=False, - entry_args=None): + entry_args=None, images=None): """Run binman with a given test file

Args: @@ -180,6 +180,10 @@ class TestFunctional(unittest.TestCase): map: True to output map files for the images update_dtb: Update the offset and size of each entry in the device tree before packing it into the image + entry_args: Dict of entry args to supply to binman + key: arg name + value: value of that arg + images: List of image names to build """ args = ['-p', '-I', self._indir, '-d', self.TestFile(fname)] if debug: @@ -191,6 +195,9 @@ class TestFunctional(unittest.TestCase): if entry_args: for arg, value in entry_args.iteritems(): args.append('-a%s=%s' % (arg, value)) + if images: + for image in images: + args += ['-i', image] return self._DoBinman(*args)

def _SetupDtb(self, fname, outfile='u-boot.dtb'): @@ -1384,6 +1391,16 @@ class TestFunctional(unittest.TestCase): data = self._DoReadFile('81_x86-start16-tpl.dts') self.assertEqual(X86_START16_TPL_DATA, data[:len(X86_START16_TPL_DATA)])

+ def testSelectImage(self): + """Test that we can select which images to build""" + with test_util.capture_sys_output() as (stdout, stderr): + retcode = self._DoTestFile('06_dual_image.dts', images=['image2']) + self.assertEqual(0, retcode) + self.assertIn('Skipping images: image1', stdout.getvalue()) + + self.assertFalse(os.path.exists(tools.GetOutputFilename('image1.bin'))) + self.assertTrue(os.path.exists(tools.GetOutputFilename('image2.bin'))) +

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

At present we require the caller to manually update the device tree using individual calls to libfdt functions. This is not ideal. It would be better if we could make changes using the Python structure and then call a Sync() function to write them back.

Add this feature to the Fdt class. Update binman and the tests to match.

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

tools/binman/control.py | 2 + tools/dtoc/fdt.py | 91 +++++++++++++++++++++++++++++++++++++---- tools/dtoc/test_fdt.py | 8 +++- 3 files changed, 91 insertions(+), 10 deletions(-)

diff --git a/tools/binman/control.py b/tools/binman/control.py index 8c48008fc75..ded1b71109b 100644 --- a/tools/binman/control.py +++ b/tools/binman/control.py @@ -183,6 +183,7 @@ def Binman(options, args): image.AddMissingProperties() image.ProcessFdt(dtb)

+ dtb.Sync(auto_resize=True) dtb.Pack() dtb.Flush()

@@ -199,6 +200,7 @@ def Binman(options, args): image.SetImagePos() if options.update_fdt: image.SetCalculatedProperties() + dtb.Sync() image.ProcessEntryContents() image.WriteSymbols() image.BuildImage() diff --git a/tools/dtoc/fdt.py b/tools/dtoc/fdt.py index 55baa3857f7..76cd66fbf95 100644 --- a/tools/dtoc/fdt.py +++ b/tools/dtoc/fdt.py @@ -43,6 +43,7 @@ class Prop: self.name = name self.value = None self.bytes = str(bytes) + self.dirty = False if not bytes: self.type = TYPE_BOOL self.value = True @@ -160,6 +161,45 @@ class Prop: self._node._fdt.CheckCache() return self._node._fdt.GetStructOffset(self._offset)

+ def SetInt(self, val): + """Set the integer value of the property + + The device tree is marked dirty so that the value will be written to + the block on the next sync. + + Args: + val: Integer value (32-bit, single cell) + """ + self.bytes = struct.pack('>I', val); + self.value = val + self.type = TYPE_INT + self.dirty = True + + def Sync(self, auto_resize=False): + """Sync property changes back to the device tree + + This updates the device tree blob with any changes to this property + since the last sync. + + Args: + auto_resize: Resize the device tree automatically if it does not + have enough space for the update + + Raises: + FdtException if auto_resize is False and there is not enough space + """ + if self._offset is None or self.dirty: + node = self._node + fdt_obj = node._fdt._fdt_obj + if auto_resize: + while fdt_obj.setprop(node.Offset(), self.name, self.bytes, + (libfdt.NOSPACE,)) == -libfdt.NOSPACE: + fdt_obj.resize(fdt_obj.totalsize() + 1024) + fdt_obj.setprop(node.Offset(), self.name, self.bytes) + else: + fdt_obj.setprop(node.Offset(), self.name, self.bytes) + + class Node: """A device tree node

@@ -284,13 +324,7 @@ class Node: Args: prop_name: Name of property """ - fdt_obj = self._fdt._fdt_obj - if fdt_obj.setprop_u32(self.Offset(), prop_name, 0, - (libfdt.NOSPACE,)) == -libfdt.NOSPACE: - fdt_obj.resize(fdt_obj.totalsize() + 1024) - fdt_obj.setprop_u32(self.Offset(), prop_name, 0) - self.props[prop_name] = Prop(self, -1, prop_name, '\0' * 4) - self._fdt.Invalidate() + self.props[prop_name] = Prop(self, None, prop_name, '\0' * 4)

def SetInt(self, prop_name, val): """Update an integer property int the device tree. @@ -301,8 +335,34 @@ class Node: prop_name: Name of property val: Value to set """ - fdt_obj = self._fdt._fdt_obj - fdt_obj.setprop_u32(self.Offset(), prop_name, val) + self.props[prop_name].SetInt(val) + + def Sync(self, auto_resize=False): + """Sync node changes back to the device tree + + This updates the device tree blob with any changes to this node and its + subnodes since the last sync. + + Args: + auto_resize: Resize the device tree automatically if it does not + have enough space for the update + + Raises: + FdtException if auto_resize is False and there is not enough space + """ + # Sync subnodes in reverse so that we don't disturb node offsets for + # nodes that are earlier in the DT. This avoids an O(n^2) rescan of + # node offsets. + for node in reversed(self.subnodes): + node.Sync(auto_resize) + + # Sync properties now, whose offsets should not have been disturbed. + # We do this after subnodes, since this disturbs the offsets of these + # properties. + prop_list = sorted(self.props.values(), key=lambda prop: prop._offset, + reverse=True) + for prop in prop_list: + prop.Sync(auto_resize)

class Fdt: @@ -381,6 +441,19 @@ class Fdt: with open(self._fname, 'wb') as fd: fd.write(self._fdt_obj.as_bytearray())

+ def Sync(self, auto_resize=False): + """Make sure any DT changes are written to the blob + + Args: + auto_resize: Resize the device tree automatically if it does not + have enough space for the update + + Raises: + FdtException if auto_resize is False and there is not enough space + """ + self._root.Sync(auto_resize) + self.Invalidate() + def Pack(self): """Pack the device tree down to its minimum size

diff --git a/tools/dtoc/test_fdt.py b/tools/dtoc/test_fdt.py index e88d19f80ef..4a67f8949df 100755 --- a/tools/dtoc/test_fdt.py +++ b/tools/dtoc/test_fdt.py @@ -337,6 +337,7 @@ class TestProp(unittest.TestCase): self.node.AddZeroProp('one') self.node.AddZeroProp('two') self.node.AddZeroProp('three') + self.dtb.Sync(auto_resize=True)

# Updating existing properties should be OK, since the device-tree size # does not change @@ -344,12 +345,17 @@ class TestProp(unittest.TestCase): self.node.SetInt('one', 1) self.node.SetInt('two', 2) self.node.SetInt('three', 3) + self.dtb.Sync(auto_resize=False)

# This should fail since it would need to increase the device-tree size + self.node.AddZeroProp('four') with self.assertRaises(libfdt.FdtException) as e: - self.node.SetInt('four', 4) + self.dtb.Sync(auto_resize=False) self.assertIn('FDT_ERR_NOSPACE', str(e.exception))

+ def testAddNode(self): + self.fdt.pack() +

class TestFdtUtil(unittest.TestCase): """Tests for the fdt_util module

This should be big endian, since that is what device tree uses. Fix it.

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

tools/dtoc/fdt.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/dtoc/fdt.py b/tools/dtoc/fdt.py index 76cd66fbf95..ccf3b23ced4 100644 --- a/tools/dtoc/fdt.py +++ b/tools/dtoc/fdt.py @@ -146,7 +146,7 @@ class Prop: if type == TYPE_BYTE: return chr(0) elif type == TYPE_INT: - return struct.pack('<I', 0); + return struct.pack('>I', 0); elif type == TYPE_STRING: return '' else:

Add a way to add new nodes and sync them back to the blob.

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

tools/dtoc/fdt.py | 20 ++++++++++++++++++++ tools/dtoc/test_fdt.py | 8 ++++++++ 2 files changed, 28 insertions(+)

diff --git a/tools/dtoc/fdt.py b/tools/dtoc/fdt.py index ccf3b23ced4..26f4a4ee562 100644 --- a/tools/dtoc/fdt.py +++ b/tools/dtoc/fdt.py @@ -337,6 +337,12 @@ class Node: """ self.props[prop_name].SetInt(val)

+ def AddSubnode(self, name): + path = self.path + '/' + name + subnode = Node(self._fdt, self, None, name, path) + self.subnodes.append(subnode) + return subnode + def Sync(self, auto_resize=False): """Sync node changes back to the device tree

@@ -350,6 +356,20 @@ class Node: Raises: FdtException if auto_resize is False and there is not enough space """ + if self._offset is None: + # The subnode doesn't exist yet, so add it + fdt_obj = self._fdt._fdt_obj + if auto_resize: + while True: + offset = fdt_obj.add_subnode(self.parent._offset, self.name, + (libfdt.NOSPACE,)) + if offset != -libfdt.NOSPACE: + break + fdt_obj.resize(fdt_obj.totalsize() + 1024) + else: + offset = fdt_obj.add_subnode(self.parent._offset, self.name) + self._offset = offset + # Sync subnodes in reverse so that we don't disturb node offsets for # nodes that are earlier in the DT. This avoids an O(n^2) rescan of # node offsets. diff --git a/tools/dtoc/test_fdt.py b/tools/dtoc/test_fdt.py index 4a67f8949df..c94e455d121 100755 --- a/tools/dtoc/test_fdt.py +++ b/tools/dtoc/test_fdt.py @@ -355,6 +355,14 @@ class TestProp(unittest.TestCase):

def testAddNode(self): self.fdt.pack() + self.node.AddSubnode('subnode') + with self.assertRaises(libfdt.FdtException) as e: + self.dtb.Sync(auto_resize=False) + self.assertIn('FDT_ERR_NOSPACE', str(e.exception)) + + self.dtb.Sync(auto_resize=True) + offset = self.fdt.path_offset('/spl-test/subnode') + self.assertTrue(offset > 0)

class TestFdtUtil(unittest.TestCase):

Add a few more functions which allow creating and modifying property values. If only we could do this so easily in the real world.

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

tools/dtoc/fdt.py | 70 ++++++++++++++++++++++++++++++++++++++++++ tools/dtoc/test_fdt.py | 43 ++++++++++++++++++++++++++ 2 files changed, 113 insertions(+)

diff --git a/tools/dtoc/fdt.py b/tools/dtoc/fdt.py index 26f4a4ee562..c5054e8cc91 100644 --- a/tools/dtoc/fdt.py +++ b/tools/dtoc/fdt.py @@ -175,6 +175,16 @@ class Prop: self.type = TYPE_INT self.dirty = True

+ def SetData(self, bytes): + """Set the value of a property as bytes + + Args: + bytes: New property value to set + """ + self.bytes = str(bytes) + self.type, self.value = self.BytesToValue(bytes) + self.dirty = True + def Sync(self, auto_resize=False): """Sync property changes back to the device tree

@@ -326,18 +336,78 @@ class Node: """ self.props[prop_name] = Prop(self, None, prop_name, '\0' * 4)

+ def AddEmptyProp(self, prop_name, len): + """Add a property with a fixed data size, for filling in later + + The device tree is marked dirty so that the value will be written to + the blob on the next sync. + + Args: + prop_name: Name of property + len: Length of data in property + """ + value = chr(0) * len + self.props[prop_name] = Prop(self, None, prop_name, value) + def SetInt(self, prop_name, val): """Update an integer property int the device tree.

This is not allowed to change the size of the FDT.

+ The device tree is marked dirty so that the value will be written to + the blob on the next sync. + Args: prop_name: Name of property val: Value to set """ self.props[prop_name].SetInt(val)

+ def SetData(self, prop_name, val): + """Set the data value of a property + + The device tree is marked dirty so that the value will be written to + the blob on the next sync. + + Args: + prop_name: Name of property to set + val: Data value to set + """ + self.props[prop_name].SetData(val) + + def SetString(self, prop_name, val): + """Set the string value of a property + + The device tree is marked dirty so that the value will be written to + the blob on the next sync. + + Args: + prop_name: Name of property to set + val: String value to set (will be \0-terminated in DT) + """ + self.props[prop_name].SetData(val + chr(0)) + + def AddString(self, prop_name, val): + """Add a new string property to a node + + The device tree is marked dirty so that the value will be written to + the blob on the next sync. + + Args: + prop_name: Name of property to add + val: String value of property + """ + self.props[prop_name] = Prop(self, None, prop_name, val + chr(0)) + def AddSubnode(self, name): + """Add a new subnode to the node + + Args: + name: name of node to add + + Returns: + New subnode that was created + """ path = self.path + '/' + name subnode = Node(self._fdt, self, None, name, path) self.subnodes.append(subnode) diff --git a/tools/dtoc/test_fdt.py b/tools/dtoc/test_fdt.py index c94e455d121..22a075d6c57 100755 --- a/tools/dtoc/test_fdt.py +++ b/tools/dtoc/test_fdt.py @@ -352,6 +352,7 @@ class TestProp(unittest.TestCase): with self.assertRaises(libfdt.FdtException) as e: self.dtb.Sync(auto_resize=False) self.assertIn('FDT_ERR_NOSPACE', str(e.exception)) + self.dtb.Sync(auto_resize=True)

def testAddNode(self): self.fdt.pack() @@ -364,6 +365,48 @@ class TestProp(unittest.TestCase): offset = self.fdt.path_offset('/spl-test/subnode') self.assertTrue(offset > 0)

+ def testAddMore(self): + """Test various other methods for adding and setting properties""" + self.node.AddZeroProp('one') + self.dtb.Sync(auto_resize=True) + data = self.fdt.getprop(self.node.Offset(), 'one') + self.assertEqual(0, fdt32_to_cpu(data)) + + self.node.SetInt('one', 1) + self.dtb.Sync(auto_resize=False) + data = self.fdt.getprop(self.node.Offset(), 'one') + self.assertEqual(1, fdt32_to_cpu(data)) + + val = '123' + chr(0) + '456' + self.node.AddString('string', val) + self.dtb.Sync(auto_resize=True) + data = self.fdt.getprop(self.node.Offset(), 'string') + self.assertEqual(val + '\0', data) + + self.fdt.pack() + self.node.SetString('string', val + 'x') + with self.assertRaises(libfdt.FdtException) as e: + self.dtb.Sync(auto_resize=False) + self.assertIn('FDT_ERR_NOSPACE', str(e.exception)) + self.node.SetString('string', val[:-1]) + + prop = self.node.props['string'] + prop.SetData(val) + self.dtb.Sync(auto_resize=False) + data = self.fdt.getprop(self.node.Offset(), 'string') + self.assertEqual(val, data) + + self.node.AddEmptyProp('empty', 5) + self.dtb.Sync(auto_resize=True) + prop = self.node.props['empty'] + prop.SetData(val) + self.dtb.Sync(auto_resize=False) + data = self.fdt.getprop(self.node.Offset(), 'empty') + self.assertEqual(val, data) + + self.node.SetData('empty', '123') + self.assertEqual('123', prop.bytes) +

class TestFdtUtil(unittest.TestCase): """Tests for the fdt_util module

Support creating an Fdt object without having to write the data to a file first.

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

tools/dtoc/fdt.py | 14 ++++++++++++++ tools/dtoc/test_fdt.py | 8 ++++++++ 2 files changed, 22 insertions(+)

diff --git a/tools/dtoc/fdt.py b/tools/dtoc/fdt.py index c5054e8cc91..2df2d4b0cc7 100644 --- a/tools/dtoc/fdt.py +++ b/tools/dtoc/fdt.py @@ -472,6 +472,20 @@ class Fdt: with open(self._fname) as fd: self._fdt_obj = libfdt.Fdt(fd.read())

+ @staticmethod + def FromData(data): + """Create a new Fdt object from the given data + + Args: + data: Device-tree data blob + + Returns: + Fdt object containing the data + """ + fdt = Fdt(None) + fdt._fdt_obj = libfdt.Fdt(bytearray(data)) + return fdt + def LookupPhandle(self, phandle): """Look up a phandle

diff --git a/tools/dtoc/test_fdt.py b/tools/dtoc/test_fdt.py index 22a075d6c57..d2597020500 100755 --- a/tools/dtoc/test_fdt.py +++ b/tools/dtoc/test_fdt.py @@ -407,6 +407,14 @@ class TestProp(unittest.TestCase): self.node.SetData('empty', '123') self.assertEqual('123', prop.bytes)

+ def testFromData(self): + dtb2 = fdt.Fdt.FromData(self.dtb.GetContents()) + self.assertEqual(dtb2.GetContents(), self.dtb.GetContents()) + + self.node.AddEmptyProp('empty', 5) + self.dtb.Sync(auto_resize=True) + self.assertTrue(dtb2.GetContents() != self.dtb.GetContents()) +

class TestFdtUtil(unittest.TestCase): """Tests for the fdt_util module

Various entry implementations provide a way to obtain the default filename for an entry. But at present there is no base-class implementation for this function. Add one so that the API is defined.

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

tools/binman/entry.py | 3 +++ tools/binman/entry_test.py | 5 +++++ 2 files changed, 8 insertions(+)

diff --git a/tools/binman/entry.py b/tools/binman/entry.py index e671a2ea094..ec3b22e9b31 100644 --- a/tools/binman/entry.py +++ b/tools/binman/entry.py @@ -160,6 +160,9 @@ class Entry(object): self.align_end = fdt_util.GetInt(self._node, 'align-end') self.offset_unset = fdt_util.GetBool(self._node, 'offset-unset')

+ def GetDefaultFilename(self): + return None + def AddMissingProperties(self): """Add new properties to the device tree as needed for this entry""" for prop in ['offset', 'size', 'image-pos']: diff --git a/tools/binman/entry_test.py b/tools/binman/entry_test.py index 4100bcc3d32..69d85b4cedb 100644 --- a/tools/binman/entry_test.py +++ b/tools/binman/entry_test.py @@ -65,6 +65,11 @@ class TestEntry(unittest.TestCase): sub_entry = entry.Entry(None, None, sub_node, read_node=False) self.assertEqual('root.subnode', sub_entry.GetUniqueName())

+ 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())

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

At present the control module has state information in it, since it is the primary user of this. But it is a bit odd to have entries and other modules importing control to obtain this information.

It seems better to have a dedicated state module, which control can use as well. Create a new module using code from control and update other modules to use it.

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

tools/binman/control.py | 50 +++---------- tools/binman/entry.py | 7 +- tools/binman/etype/u_boot_dtb_with_ucode.py | 6 +- tools/binman/ftest.py | 3 +- tools/binman/state.py | 77 +++++++++++++++++++++ 5 files changed, 95 insertions(+), 48 deletions(-) create mode 100644 tools/binman/state.py

diff --git a/tools/binman/control.py b/tools/binman/control.py index ded1b71109b..00dcb8ad6a5 100644 --- a/tools/binman/control.py +++ b/tools/binman/control.py @@ -7,27 +7,19 @@

from collections import OrderedDict import os -import re import sys import tools

import command import elf from image import Image +import state import tout

# List of images we plan to create # Make this global so that it can be referenced from tests images = OrderedDict()

-# Records the device-tree files known to binman, keyed by filename (e.g. -# 'u-boot-spl.dtb') -fdt_files = {} - -# Arguments passed to binman to provide arguments to entries -entry_args = {} - - def _ReadImageDesc(binman_node): """Read the image descriptions from the /binman node

@@ -60,39 +52,15 @@ def _FindBinmanNode(dtb): return node return None

-def GetFdt(fname): - """Get the Fdt object for a particular device-tree filename - - Binman keeps track of at least one device-tree file called u-boot.dtb but - can also have others (e.g. for SPL). This function looks up the given - filename and returns the associated Fdt object. +def WriteEntryDocs(modules, test_missing=None): + """Write out documentation for all entries

Args: - fname: Filename to look up (e.g. 'u-boot.dtb'). - - Returns: - Fdt object associated with the filename + modules: List of Module objects to get docs for + test_missing: Used for testing only, to force an entry's documeentation + to show as missing even if it is present. Should be set to None in + normal use. """ - return fdt_files[fname] - -def GetFdtPath(fname): - return fdt_files[fname]._fname - -def SetEntryArgs(args): - global entry_args - - entry_args = {} - if args: - for arg in args: - m = re.match('([^=]*)=(.*)', arg) - if not m: - raise ValueError("Invalid entry arguemnt '%s'" % arg) - entry_args[m.group(1)] = m.group(2) - -def GetEntryArg(name): - return entry_args.get(name) - -def WriteEntryDocs(modules, test_missing=None): from entry import Entry Entry.WriteDocs(modules, test_missing)

@@ -141,7 +109,7 @@ def Binman(options, args): try: tools.SetInputDirs(options.indir) tools.PrepareOutputDir(options.outdir, options.preserve) - SetEntryArgs(options.entry_arg) + state.SetEntryArgs(options.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 @@ -154,7 +122,7 @@ def Binman(options, args): dtb = fdt.FdtScan(fname)

# Note the file so that GetFdt() can find it - fdt_files['u-boot.dtb'] = dtb + state.fdt_files['u-boot.dtb'] = dtb node = _FindBinmanNode(dtb) if not node: raise ValueError("Device tree '%s' does not have a 'binman' " diff --git a/tools/binman/entry.py b/tools/binman/entry.py index ec3b22e9b31..1d6299aefa5 100644 --- a/tools/binman/entry.py +++ b/tools/binman/entry.py @@ -17,10 +17,11 @@ try: except: have_importlib = False

-import fdt_util -import control import os import sys + +import fdt_util +import state import tools

modules = {} @@ -393,7 +394,7 @@ class Entry(object): Raises: ValueError if the argument cannot be converted to in """ - value = control.GetEntryArg(name) + value = state.GetEntryArg(name) if value is not None: if datatype == int: try: diff --git a/tools/binman/etype/u_boot_dtb_with_ucode.py b/tools/binman/etype/u_boot_dtb_with_ucode.py index 285a28dd1e7..3fab766011e 100644 --- a/tools/binman/etype/u_boot_dtb_with_ucode.py +++ b/tools/binman/etype/u_boot_dtb_with_ucode.py @@ -5,9 +5,9 @@ # Entry-type module for U-Boot device tree with the microcode removed #

-import control from entry import Entry from blob import Entry_blob +import state import tools

class Entry_u_boot_dtb_with_ucode(Entry_blob): @@ -51,7 +51,7 @@ class Entry_u_boot_dtb_with_ucode(Entry_blob):

# Remove the microcode fname = self.GetDefaultFilename() - fdt = control.GetFdt(fname) + fdt = state.GetFdt(fname) self.ucode = fdt.GetNode('/microcode') if not self.ucode: raise self.Raise("No /microcode node found in '%s'" % fname) @@ -70,7 +70,7 @@ class Entry_u_boot_dtb_with_ucode(Entry_blob): def ObtainContents(self): # Call the base class just in case it does something important. Entry_blob.ObtainContents(self) - self._pathname = control.GetFdtPath(self._filename) + self._pathname = state.GetFdtPath(self._filename) self.ReadBlobContents() if self.ucode: for node in self.ucode.subnodes: diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index 290e9aebf16..867179702d9 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -23,6 +23,7 @@ import fdt import fdt_util import fmap_util import test_util +import state import tools import tout

@@ -258,7 +259,7 @@ class TestFunctional(unittest.TestCase): retcode = self._DoTestFile(fname, map=map, update_dtb=update_dtb, entry_args=entry_args) self.assertEqual(0, retcode) - out_dtb_fname = control.GetFdtPath('u-boot.dtb') + out_dtb_fname = state.GetFdtPath('u-boot.dtb')

# Find the (only) image, read it and return its contents image = control.images['image'] diff --git a/tools/binman/state.py b/tools/binman/state.py new file mode 100644 index 00000000000..6bc24ddf8ac --- /dev/null +++ b/tools/binman/state.py @@ -0,0 +1,77 @@ +# SPDX-License-Identifier: GPL-2.0+ +# Copyright 2018 Google, Inc +# Written by Simon Glass sjg@chromium.org +# +# Holds and modifies the state information held by binman +# + +import re +from sets import Set + +import os +import tools + +# Records the device-tree files known to binman, keyed by filename (e.g. +# 'u-boot-spl.dtb') +fdt_files = {} + +# Arguments passed to binman to provide arguments to entries +entry_args = {} + +def GetFdt(fname): + """Get the Fdt object for a particular device-tree filename + + Binman keeps track of at least one device-tree file called u-boot.dtb but + can also have others (e.g. for SPL). This function looks up the given + filename and returns the associated Fdt object. + + Args: + fname: Filename to look up (e.g. 'u-boot.dtb'). + + Returns: + Fdt object associated with the filename + """ + return fdt_files[fname] + +def GetFdtPath(fname): + """Get the full pathname of a particular Fdt object + + Similar to GetFdt() but returns the pathname associated with the Fdt. + + Args: + fname: Filename to look up (e.g. 'u-boot.dtb'). + + Returns: + Full path name to the associated Fdt + """ + return fdt_files[fname]._fname + +def SetEntryArgs(args): + """Set the value of the entry args + + This sets up the entry_args dict which is used to supply entry arguments to + entries. + + Args: + args: List of entry arguments, each in the format "name=value" + """ + global entry_args + + entry_args = {} + if args: + for arg in args: + m = re.match('([^=]*)=(.*)', arg) + if not m: + raise ValueError("Invalid entry arguemnt '%s'" % arg) + entry_args[m.group(1)] = m.group(2) + +def GetEntryArg(name): + """Get the value of an entry argument + + Args: + name: Name of argument to retrieve + + Returns: + String value of argument + """ + return entry_args.get(name)

Rather than reaching into this module from control, move the code that needs this info into state.

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

tools/binman/control.py | 21 ++++++++++++-------- tools/binman/state.py | 43 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 56 insertions(+), 8 deletions(-)

diff --git a/tools/binman/control.py b/tools/binman/control.py index 00dcb8ad6a5..fd8b779945d 100644 --- a/tools/binman/control.py +++ b/tools/binman/control.py @@ -121,8 +121,6 @@ def Binman(options, args): outfd.write(infd.read()) dtb = fdt.FdtScan(fname)

- # Note the file so that GetFdt() can find it - state.fdt_files['u-boot.dtb'] = dtb node = _FindBinmanNode(dtb) if not node: raise ValueError("Device tree '%s' does not have a 'binman' " @@ -139,6 +137,8 @@ def Binman(options, args): if skip: print 'Skipping images: %s\n' % ', '.join(skip)

+ state.Prepare(dtb) + # Prepare the device tree by making sure that any missing # properties are added (e.g. 'pos' and 'size'). The values of these # may not be correct yet, but we add placeholders so that the @@ -151,9 +151,10 @@ def Binman(options, args): image.AddMissingProperties() image.ProcessFdt(dtb)

- dtb.Sync(auto_resize=True) - dtb.Pack() - dtb.Flush() + for dtb_item in state.GetFdts(): + dtb_item.Sync(auto_resize=True) + dtb_item.Pack() + dtb_item.Flush()

for image in images.values(): # Perform all steps for this image, including checking and @@ -168,14 +169,18 @@ def Binman(options, args): image.SetImagePos() if options.update_fdt: image.SetCalculatedProperties() - dtb.Sync() + for dtb_item in state.GetFdts(): + dtb_item.Sync() image.ProcessEntryContents() image.WriteSymbols() image.BuildImage() if options.map: image.WriteMap() - with open(fname, 'wb') as outfd: - outfd.write(dtb.GetContents()) + + # Write the updated FDTs to our output files + for dtb_item in state.GetFdts(): + tools.WriteFile(dtb_item._fname, dtb_item.GetContents()) + finally: tools.FinaliseOutputDir() finally: diff --git a/tools/binman/state.py b/tools/binman/state.py index 6bc24ddf8ac..9583b3fa5f6 100644 --- a/tools/binman/state.py +++ b/tools/binman/state.py @@ -18,6 +18,15 @@ fdt_files = {} # Arguments passed to binman to provide arguments to entries entry_args = {}

+# Set of all device tree files references by images +fdt_set = Set() + +# Same as above, but excluding the main one +fdt_subset = Set() + +# The DTB which contains the full image information +main_dtb = None + def GetFdt(fname): """Get the Fdt object for a particular device-tree filename

@@ -75,3 +84,37 @@ def GetEntryArg(name): String value of argument """ return entry_args.get(name) + +def Prepare(dtb): + """Get device tree files ready for use + + This sets up a set of device tree files that can be retrieved by GetFdts(). + At present there is only one, that for U-Boot proper. + + Args: + dtb: Main dtb + """ + global fdt_set, fdt_subset, fdt_files, main_dtb + # Import these here in case libfdt.py is not available, in which case + # the above help option still works. + import fdt + import fdt_util + + # If we are updating the DTBs we need to put these updated versions + # where Entry_blob_dtb can find them. We can ignore 'u-boot.dtb' + # since it is assumed to be the one passed in with options.dt, and + # was handled just above. + main_dtb = dtb + fdt_files.clear() + fdt_files['u-boot.dtb'] = dtb + fdt_set = Set() + fdt_subset = Set() + +def GetFdts(): + """Yield all device tree files being used by binman + + Yields: + Device trees being used (U-Boot proper, SPL, TPL) + """ + yield main_dtb +

At present we have a few calls to device-tree functions in binman and plan to add more as we add new entry types which need to report their results.

It makes sense to put this code in a central place so that we can make sure all device trees are updated. At present we only have U-Boot proper, but plan to add SPL and TPL too.

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

tools/binman/bsection.py | 9 +++++---- tools/binman/entry.py | 8 ++++---- tools/binman/state.py | 35 +++++++++++++++++++++++++++++++++++ 3 files changed, 44 insertions(+), 8 deletions(-)

diff --git a/tools/binman/bsection.py b/tools/binman/bsection.py index a0bd1b6d34e..4f5c33f048b 100644 --- a/tools/binman/bsection.py +++ b/tools/binman/bsection.py @@ -12,6 +12,7 @@ import sys

import fdt_util import re +import state import tools

class Section(object): @@ -98,14 +99,14 @@ class Section(object): """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: - self._node.AddZeroProp(prop) + state.AddZeroProp(self._node, prop) for entry in self._entries.values(): entry.AddMissingProperties()

def SetCalculatedProperties(self): - self._node.SetInt('offset', self._offset) - self._node.SetInt('size', self._size) - self._node.SetInt('image-pos', self._image_pos) + state.SetInt(self._node, 'offset', self._offset) + state.SetInt(self._node, 'size', self._size) + state.SetInt(self._node, 'image-pos', self._image_pos) for entry in self._entries.values(): entry.SetCalculatedProperties()

diff --git a/tools/binman/entry.py b/tools/binman/entry.py index 1d6299aefa5..4b87156ff80 100644 --- a/tools/binman/entry.py +++ b/tools/binman/entry.py @@ -168,13 +168,13 @@ class Entry(object): """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: - self._node.AddZeroProp(prop) + state.AddZeroProp(self._node, prop)

def SetCalculatedProperties(self): """Set the value of device-tree properties calculated by binman""" - self._node.SetInt('offset', self.offset) - self._node.SetInt('size', self.size) - self._node.SetInt('image-pos', self.image_pos) + state.SetInt(self._node, 'offset', self.offset) + state.SetInt(self._node, 'size', self.size) + state.SetInt(self._node, 'image-pos', self.image_pos)

def ProcessFdt(self, fdt): return True diff --git a/tools/binman/state.py b/tools/binman/state.py index 9583b3fa5f6..5f25b907b9e 100644 --- a/tools/binman/state.py +++ b/tools/binman/state.py @@ -118,3 +118,38 @@ def GetFdts(): """ yield main_dtb

+def GetUpdateNodes(node): + """Yield all the nodes that need to be updated in all device trees + + The property referenced by this node is added to any device trees which + have the given node. Due to removable of unwanted notes, SPL and TPL may + not have this node. + + Args: + node: Node object in the main device tree to look up + + Yields: + Node objects in each device tree that is in use (U-Boot proper, which + is node, SPL and TPL) + """ + yield node + +def AddZeroProp(node, prop): + """Add a new property to affected device trees with an integer value of 0. + + Args: + prop_name: Name of property + """ + for n in GetUpdateNodes(node): + n.AddZeroProp(prop) + +def SetInt(node, prop, value): + """Update an integer property in affected device trees with an integer value + + This is not allowed to change the size of the FDT. + + Args: + prop_name: Name of property + """ + for n in GetUpdateNodes(node): + n.SetInt(prop, value)

We always have a device tree for U-Boot proper. But we may also have one for SPL and TPL. Add a new Entry method to find out what DTs an entry has, and use that list when updating DTs.

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

tools/binman/bsection.py | 8 ++++++++ tools/binman/control.py | 2 +- tools/binman/entry.py | 15 +++++++++++++++ tools/binman/image.py | 4 ++++ tools/binman/state.py | 20 ++++++++++++++++++-- 5 files changed, 46 insertions(+), 3 deletions(-)

diff --git a/tools/binman/bsection.py b/tools/binman/bsection.py index 4f5c33f048b..44adb82795b 100644 --- a/tools/binman/bsection.py +++ b/tools/binman/bsection.py @@ -8,6 +8,7 @@ from __future__ import print_function

from collections import OrderedDict +from sets import Set import sys

import fdt_util @@ -92,6 +93,13 @@ class Section(object): 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

diff --git a/tools/binman/control.py b/tools/binman/control.py index fd8b779945d..49d49a001c7 100644 --- a/tools/binman/control.py +++ b/tools/binman/control.py @@ -137,7 +137,7 @@ def Binman(options, args): if skip: print 'Skipping images: %s\n' % ', '.join(skip)

- state.Prepare(dtb) + state.Prepare(images, dtb)

# Prepare the device tree by making sure that any missing # properties are added (e.g. 'pos' and 'size'). The values of these diff --git a/tools/binman/entry.py b/tools/binman/entry.py index 4b87156ff80..e5f557749f6 100644 --- a/tools/binman/entry.py +++ b/tools/binman/entry.py @@ -18,6 +18,7 @@ except: have_importlib = False

import os +from sets import Set import sys

import fdt_util @@ -164,6 +165,20 @@ class Entry(object): def GetDefaultFilename(self): return None

+ def GetFdtSet(self): + """Get the set of device trees used by this entry + + Returns: + Set containing the filename from this entry, if it is a .dtb, else + an empty set + """ + fname = self.GetDefaultFilename() + # It would be better to use isinstance(self, Entry_blob_dtb) here but + # we cannot access Entry_blob_dtb + if fname and fname.endswith('.dtb'): + return Set([fname]) + return Set() + def AddMissingProperties(self): """Add new properties to the device tree as needed for this entry""" for prop in ['offset', 'size', 'image-pos']: diff --git a/tools/binman/image.py b/tools/binman/image.py index 68126bc3e69..1fb5eb65db3 100644 --- a/tools/binman/image.py +++ b/tools/binman/image.py @@ -54,6 +54,10 @@ class Image: self._filename = filename self._section = bsection.Section('main-section', self._node)

+ def GetFdtSet(self): + """Get the set of device tree files used by this image""" + return self._section.GetFdtSet() + def AddMissingProperties(self): """Add properties that are not present in the device tree

diff --git a/tools/binman/state.py b/tools/binman/state.py index 5f25b907b9e..600eb86cfe2 100644 --- a/tools/binman/state.py +++ b/tools/binman/state.py @@ -18,6 +18,10 @@ fdt_files = {} # Arguments passed to binman to provide arguments to entries entry_args = {}

+# True to use fake device-tree files for testing (see U_BOOT_DTB_DATA in +# ftest.py) +use_fake_dtb = True + # Set of all device tree files references by images fdt_set = Set()

@@ -85,13 +89,14 @@ def GetEntryArg(name): """ return entry_args.get(name)

-def Prepare(dtb): +def Prepare(images, dtb): """Get device tree files ready for use

This sets up a set of device tree files that can be retrieved by GetFdts(). At present there is only one, that for U-Boot proper.

Args: + images: List of images being used dtb: Main dtb """ global fdt_set, fdt_subset, fdt_files, main_dtb @@ -107,8 +112,19 @@ def Prepare(dtb): main_dtb = dtb fdt_files.clear() fdt_files['u-boot.dtb'] = dtb - fdt_set = Set() fdt_subset = Set() + if not use_fake_dtb: + for image in images.values(): + fdt_subset.update(image.GetFdtSet()) + fdt_subset.discard('u-boot.dtb') + for other_fname in fdt_subset: + infile = tools.GetInputFilename(other_fname) + other_fname_dtb = fdt_util.EnsureCompiled(infile) + out_fname = tools.GetOutputFilename('%s.out' % + os.path.split(other_fname)[1]) + tools.WriteFile(out_fname, tools.ReadFile(other_fname_dtb)) + other_dtb = fdt.FdtScan(out_fname) + fdt_files[other_fname] = other_dtb

def GetFdts(): """Yield all device tree files being used by binman

We use a fake device tree in tests most of the time since tests don't normally care about the actual data. For example, for U-Boot proper we use U_BOOT_DTB_DATA which is just a four-character string. This makes testing the image output against an expected value very easy.

However in some cases, such as when the test wants to check that the DT output containing particular nodes, we do actually need the real DT. Add support for this, along with a command-line option to select 'test mode'.

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

tools/binman/cmdline.py | 2 ++ tools/binman/control.py | 1 + tools/binman/ftest.py | 4 +++- tools/binman/state.py | 2 +- 4 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/tools/binman/cmdline.py b/tools/binman/cmdline.py index 4ce8bc6ab43..f8caa7d2841 100644 --- a/tools/binman/cmdline.py +++ b/tools/binman/cmdline.py @@ -30,6 +30,8 @@ def ParseArgs(argv): 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', + help='Use fake device tree contents (for testing only)') parser.add_option('-i', '--image', type='string', action='append', help='Image filename to build (if not specified, build all)') parser.add_option('-I', '--indir', action='append', diff --git a/tools/binman/control.py b/tools/binman/control.py index 49d49a001c7..34ec74ba1b3 100644 --- a/tools/binman/control.py +++ b/tools/binman/control.py @@ -106,6 +106,7 @@ def Binman(options, args):

tout.Init(options.verbosity) elf.debug = options.debug + state.use_fake_dtb = options.fake_dtb try: tools.SetInputDirs(options.indir) tools.PrepareOutputDir(options.outdir, options.preserve) diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index 867179702d9..75e9a2143ca 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -172,7 +172,7 @@ class TestFunctional(unittest.TestCase): return control.Binman(options, args)

def _DoTestFile(self, fname, debug=False, map=False, update_dtb=False, - entry_args=None, images=None): + entry_args=None, images=None, use_real_dtb=False): """Run binman with a given test file

Args: @@ -193,6 +193,8 @@ class TestFunctional(unittest.TestCase): args.append('-m') if update_dtb: args.append('-up') + if not use_real_dtb: + args.append('--fake-dtb') if entry_args: for arg, value in entry_args.iteritems(): args.append('-a%s=%s' % (arg, value)) diff --git a/tools/binman/state.py b/tools/binman/state.py index 600eb86cfe2..b27eb077a61 100644 --- a/tools/binman/state.py +++ b/tools/binman/state.py @@ -20,7 +20,7 @@ entry_args = {}

# True to use fake device-tree files for testing (see U_BOOT_DTB_DATA in # ftest.py) -use_fake_dtb = True +use_fake_dtb = False

# Set of all device tree files references by images fdt_set = Set()

Binman currently supports updating the main device tree with things like the position of each entry. Extend this support to SPL and TPL as well, since they may need (a subset of) this information.

Also adjust DTB output files to have a .out extension since this seems clearer than having a .dtb extension with 'out' in the name somwhere.

Also add a few missing comments and update the DT setup code to use ReadFile and WriteFile().

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

tools/binman/README.entries | 12 +++ tools/binman/control.py | 6 +- tools/binman/entry.py | 11 ++ tools/binman/etype/blob_dtb.py | 33 ++++++ tools/binman/etype/section.py | 3 + tools/binman/etype/u_boot_dtb.py | 9 +- tools/binman/etype/u_boot_dtb_with_ucode.py | 8 +- tools/binman/etype/u_boot_spl_dtb.py | 6 +- tools/binman/etype/u_boot_tpl_dtb.py | 6 +- tools/binman/ftest.py | 109 +++++++++++++++++++- tools/binman/image.py | 5 + tools/binman/state.py | 30 ++++++ tools/binman/test/82_fdt_update_all.dts | 18 ++++ 13 files changed, 235 insertions(+), 21 deletions(-) create mode 100644 tools/binman/etype/blob_dtb.py create mode 100644 tools/binman/test/82_fdt_update_all.dts

diff --git a/tools/binman/README.entries b/tools/binman/README.entries index 5cb52a92ff9..091fb5ce2b6 100644 --- a/tools/binman/README.entries +++ b/tools/binman/README.entries @@ -26,6 +26,15 @@ example the 'u_boot' entry which provides the filename 'u-boot.bin'.

+Entry: blob-dtb: A blob that holds a device tree +------------------------------------------------ + +This is a blob containing a device tree. The contents of the blob are +obtained from the list of available device-tree files, managed by the +'state' module. + + + Entry: blob-named-by-arg: A blob entry which gets its filename property from its subclass -----------------------------------------------------------------------------------------

@@ -309,6 +318,9 @@ This is the U-Boot device tree, containing configuration information for U-Boot. U-Boot needs this to know what devices are present and which drivers to activate.

+Note: This is mostly an internal entry type, used by others. This allows +binman to know which entries contain a device tree. +

Entry: u-boot-dtb-with-ucode: A U-Boot device tree file, with the microcode removed diff --git a/tools/binman/control.py b/tools/binman/control.py index 34ec74ba1b3..e326456a4ba 100644 --- a/tools/binman/control.py +++ b/tools/binman/control.py @@ -116,10 +116,8 @@ def Binman(options, args): # output into a file in our output directly. Then scan it for use # in binman. dtb_fname = fdt_util.EnsureCompiled(dtb_fname) - fname = tools.GetOutputFilename('u-boot-out.dtb') - with open(dtb_fname) as infd: - with open(fname, 'wb') as outfd: - outfd.write(infd.read()) + fname = tools.GetOutputFilename('u-boot.dtb.out') + tools.WriteFile(fname, tools.ReadFile(dtb_fname)) dtb = fdt.FdtScan(fname)

node = _FindBinmanNode(dtb) diff --git a/tools/binman/entry.py b/tools/binman/entry.py index e5f557749f6..f922107523e 100644 --- a/tools/binman/entry.py +++ b/tools/binman/entry.py @@ -192,6 +192,17 @@ class Entry(object): state.SetInt(self._node, 'image-pos', self.image_pos)

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. + + Returns: + True if processing is complete + False if processing could not be completed due to a dependency. + This will cause the entry to be retried after others have been + called + """ return True

def SetPrefix(self, prefix): diff --git a/tools/binman/etype/blob_dtb.py b/tools/binman/etype/blob_dtb.py new file mode 100644 index 00000000000..cc5b4a3f760 --- /dev/null +++ b/tools/binman/etype/blob_dtb.py @@ -0,0 +1,33 @@ +# SPDX-License-Identifier: GPL-2.0+ +# Copyright (c) 2018 Google, Inc +# Written by Simon Glass sjg@chromium.org +# +# Entry-type module for U-Boot device tree files +# + +import state + +from entry import Entry +from blob import Entry_blob + +class Entry_blob_dtb(Entry_blob): + """A blob that holds a device tree + + This is a blob containing a device tree. The contents of the blob are + obtained from the list of available device-tree files, managed by the + 'state' module. + """ + def __init__(self, section, etype, node): + Entry_blob.__init__(self, section, etype, node) + + def ObtainContents(self): + """Get the device-tree from the list held by the 'state' module""" + self._filename = self.GetDefaultFilename() + self._pathname, data = state.GetFdtContents(self._filename) + self.SetContents(data) + return True + + def ProcessContents(self): + """Re-read the DTB contents so that we get any calculated properties""" + _, data = state.GetFdtContents(self._filename) + self.SetContents(data) diff --git a/tools/binman/etype/section.py b/tools/binman/etype/section.py index f5b2ed67cf8..a30cc915457 100644 --- a/tools/binman/etype/section.py +++ b/tools/binman/etype/section.py @@ -34,6 +34,9 @@ class Entry_section(Entry): Entry.__init__(self, section, etype, node) self._section = bsection.Section(node.name, node)

+ def GetFdtSet(self): + return self._section.GetFdtSet() + def ProcessFdt(self, fdt): return self._section.ProcessFdt(fdt)

diff --git a/tools/binman/etype/u_boot_dtb.py b/tools/binman/etype/u_boot_dtb.py index fb3dd1cf642..6263c4ebee3 100644 --- a/tools/binman/etype/u_boot_dtb.py +++ b/tools/binman/etype/u_boot_dtb.py @@ -6,9 +6,9 @@ #

from entry import Entry -from blob import Entry_blob +from blob_dtb import Entry_blob_dtb

-class Entry_u_boot_dtb(Entry_blob): +class Entry_u_boot_dtb(Entry_blob_dtb): """U-Boot device tree

Properties / Entry arguments: @@ -17,9 +17,12 @@ class Entry_u_boot_dtb(Entry_blob): This is the U-Boot device tree, containing configuration information for U-Boot. U-Boot needs this to know what devices are present and which drivers to activate. + + Note: This is mostly an internal entry type, used by others. This allows + binman to know which entries contain a device tree. """ def __init__(self, section, etype, node): - Entry_blob.__init__(self, section, etype, node) + Entry_blob_dtb.__init__(self, section, etype, node)

def GetDefaultFilename(self): return 'u-boot.dtb' diff --git a/tools/binman/etype/u_boot_dtb_with_ucode.py b/tools/binman/etype/u_boot_dtb_with_ucode.py index 3fab766011e..73f5fbf9033 100644 --- a/tools/binman/etype/u_boot_dtb_with_ucode.py +++ b/tools/binman/etype/u_boot_dtb_with_ucode.py @@ -6,11 +6,11 @@ #

from entry import Entry -from blob import Entry_blob +from blob_dtb import Entry_blob_dtb import state import tools

-class Entry_u_boot_dtb_with_ucode(Entry_blob): +class Entry_u_boot_dtb_with_ucode(Entry_blob_dtb): """A U-Boot device tree file, with the microcode removed

Properties / Entry arguments: @@ -25,7 +25,7 @@ class Entry_u_boot_dtb_with_ucode(Entry_blob): it available to u_boot_ucode. """ def __init__(self, section, etype, node): - Entry_blob.__init__(self, section, etype, node) + Entry_blob_dtb.__init__(self, section, etype, node) self.ucode_data = '' self.collate = False self.ucode_offset = None @@ -69,7 +69,7 @@ class Entry_u_boot_dtb_with_ucode(Entry_blob):

def ObtainContents(self): # Call the base class just in case it does something important. - Entry_blob.ObtainContents(self) + Entry_blob_dtb.ObtainContents(self) self._pathname = state.GetFdtPath(self._filename) self.ReadBlobContents() if self.ucode: diff --git a/tools/binman/etype/u_boot_spl_dtb.py b/tools/binman/etype/u_boot_spl_dtb.py index cb29ba3fd87..e7354646f13 100644 --- a/tools/binman/etype/u_boot_spl_dtb.py +++ b/tools/binman/etype/u_boot_spl_dtb.py @@ -6,9 +6,9 @@ #

from entry import Entry -from blob import Entry_blob +from blob_dtb import Entry_blob_dtb

-class Entry_u_boot_spl_dtb(Entry_blob): +class Entry_u_boot_spl_dtb(Entry_blob_dtb): """U-Boot SPL device tree

Properties / Entry arguments: @@ -19,7 +19,7 @@ class Entry_u_boot_spl_dtb(Entry_blob): to activate. """ def __init__(self, section, etype, node): - Entry_blob.__init__(self, section, etype, node) + Entry_blob_dtb.__init__(self, section, etype, node)

def GetDefaultFilename(self): return 'spl/u-boot-spl.dtb' diff --git a/tools/binman/etype/u_boot_tpl_dtb.py b/tools/binman/etype/u_boot_tpl_dtb.py index 9c4e668347e..bdeb0f75a24 100644 --- a/tools/binman/etype/u_boot_tpl_dtb.py +++ b/tools/binman/etype/u_boot_tpl_dtb.py @@ -6,9 +6,9 @@ #

from entry import Entry -from blob import Entry_blob +from blob_dtb import Entry_blob_dtb

-class Entry_u_boot_tpl_dtb(Entry_blob): +class Entry_u_boot_tpl_dtb(Entry_blob_dtb): """U-Boot TPL device tree

Properties / Entry arguments: @@ -19,7 +19,7 @@ class Entry_u_boot_tpl_dtb(Entry_blob): to activate. """ def __init__(self, section, etype, node): - Entry_blob.__init__(self, section, etype, node) + Entry_blob_dtb.__init__(self, section, etype, node)

def GetDefaultFilename(self): return 'tpl/u-boot-tpl.dtb' diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index 75e9a2143ca..6bfef7b63a6 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -225,8 +225,26 @@ class TestFunctional(unittest.TestCase): TestFunctional._MakeInputFile(outfile, data) return data

+ def _GetDtbContentsForSplTpl(self, dtb_data, name): + """Create a version of the main DTB for SPL or SPL + + For testing we don't actually have different versions of the DTB. With + U-Boot we normally run fdtgrep to remove unwanted nodes, but for tests + we don't normally have any unwanted nodes. + + We still want the DTBs for SPL and TPL to be different though, since + otherwise it is confusing to know which one we are looking at. So add + an 'spl' or 'tpl' property to the top-level node. + """ + dtb = fdt.Fdt.FromData(dtb_data) + dtb.Scan() + dtb.GetNode('/binman').AddZeroProp(name) + dtb.Sync(auto_resize=True) + dtb.Pack() + return dtb.GetContents() + def _DoReadFileDtb(self, fname, use_real_dtb=False, map=False, - update_dtb=False, entry_args=None): + update_dtb=False, entry_args=None, reset_dtbs=True): """Run binman and return the resulting image

This runs binman with a given test file and then reads the resulting @@ -256,12 +274,21 @@ class TestFunctional(unittest.TestCase): # Use the compiled test file as the u-boot-dtb input if use_real_dtb: dtb_data = self._SetupDtb(fname) + infile = os.path.join(self._indir, 'u-boot.dtb') + + # For testing purposes, make a copy of the DT for SPL and TPL. Add + # a node indicating which it is, so aid verification. + for name in ['spl', 'tpl']: + dtb_fname = '%s/u-boot-%s.dtb' % (name, name) + outfile = os.path.join(self._indir, dtb_fname) + TestFunctional._MakeInputFile(dtb_fname, + self._GetDtbContentsForSplTpl(dtb_data, name))

try: retcode = self._DoTestFile(fname, map=map, update_dtb=update_dtb, - entry_args=entry_args) + entry_args=entry_args, use_real_dtb=use_real_dtb) self.assertEqual(0, retcode) - out_dtb_fname = state.GetFdtPath('u-boot.dtb') + out_dtb_fname = tools.GetOutputFilename('u-boot.dtb.out')

# Find the (only) image, read it and return its contents image = control.images['image'] @@ -277,7 +304,7 @@ class TestFunctional(unittest.TestCase): return fd.read(), dtb_data, map_data, out_dtb_fname finally: # Put the test file back - if use_real_dtb: + if reset_dtbs and use_real_dtb: self._ResetDtbs()

def _DoReadFile(self, fname, use_real_dtb=False): @@ -1404,6 +1431,80 @@ class TestFunctional(unittest.TestCase): self.assertFalse(os.path.exists(tools.GetOutputFilename('image1.bin'))) self.assertTrue(os.path.exists(tools.GetOutputFilename('image2.bin')))

+ def testUpdateFdtAll(self): + """Test that all device trees are updated with offset/size info""" + data, _, _, _ = self._DoReadFileDtb('82_fdt_update_all.dts', + use_real_dtb=True, update_dtb=True) + + base_expected = { + 'section:image-pos': 0, + 'u-boot-tpl-dtb:size': 513, + 'u-boot-spl-dtb:size': 513, + 'u-boot-spl-dtb:offset': 493, + 'image-pos': 0, + 'section/u-boot-dtb:image-pos': 0, + 'u-boot-spl-dtb:image-pos': 493, + 'section/u-boot-dtb:size': 493, + 'u-boot-tpl-dtb:image-pos': 1006, + 'section/u-boot-dtb:offset': 0, + 'section:size': 493, + 'offset': 0, + 'section:offset': 0, + 'u-boot-tpl-dtb:offset': 1006, + 'size': 1519 + } + + # We expect three device-tree files in the output, one after the other. + # Read them in sequence. We look for an 'spl' property in the SPL tree, + # and 'tpl' in the TPL tree, to make sure they are distinct from the + # main U-Boot tree. All three should have the same postions and offset. + start = 0 + for item in ['', 'spl', 'tpl']: + dtb = fdt.Fdt.FromData(data[start:]) + dtb.Scan() + props = self._GetPropTree(dtb, ['offset', 'size', 'image-pos', + 'spl', 'tpl']) + expected = dict(base_expected) + if item: + expected[item] = 0 + self.assertEqual(expected, props) + start += dtb._fdt_obj.totalsize() + + def testUpdateFdtOutput(self): + """Test that output DTB files are updated""" + try: + data, dtb_data, _, _ = self._DoReadFileDtb('82_fdt_update_all.dts', + use_real_dtb=True, update_dtb=True, reset_dtbs=False) + + # Unfortunately, compiling a source file always results in a file + # called source.dtb (see fdt_util.EnsureCompiled()). The test + # source file (e.g. test/75_fdt_update_all.dts) thus does not enter + # binman as a file called u-boot.dtb. To fix this, copy the file + # over to the expected place. + #tools.WriteFile(os.path.join(self._indir, 'u-boot.dtb'), + #tools.ReadFile(tools.GetOutputFilename('source.dtb'))) + start = 0 + for fname in ['u-boot.dtb.out', 'spl/u-boot-spl.dtb.out', + 'tpl/u-boot-tpl.dtb.out']: + dtb = fdt.Fdt.FromData(data[start:]) + size = dtb._fdt_obj.totalsize() + pathname = tools.GetOutputFilename(os.path.split(fname)[1]) + outdata = tools.ReadFile(pathname) + name = os.path.split(fname)[0] + + if name: + orig_indata = self._GetDtbContentsForSplTpl(dtb_data, name) + else: + orig_indata = dtb_data + self.assertNotEqual(outdata, orig_indata, + "Expected output file '%s' be updated" % pathname) + self.assertEqual(outdata, data[start:start + size], + "Expected output file '%s' to match output image" % + pathname) + start += size + finally: + self._ResetDtbs() +

if __name__ == "__main__": unittest.main() diff --git a/tools/binman/image.py b/tools/binman/image.py index 1fb5eb65db3..bfb22297797 100644 --- a/tools/binman/image.py +++ b/tools/binman/image.py @@ -70,6 +70,11 @@ class Image: 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): diff --git a/tools/binman/state.py b/tools/binman/state.py index b27eb077a61..09ead442a60 100644 --- a/tools/binman/state.py +++ b/tools/binman/state.py @@ -59,6 +59,29 @@ def GetFdtPath(fname): """ return fdt_files[fname]._fname

+def GetFdtContents(fname): + """Looks up the FDT pathname and contents + + This is used to obtain the Fdt pathname and contents when needed by an + entry. It supports a 'fake' dtb, allowing tests to substitute test data for + the real dtb. + + Args: + fname: Filename to look up (e.g. 'u-boot.dtb'). + + Returns: + tuple: + pathname to Fdt + Fdt data (as bytes) + """ + if fname in fdt_files and not use_fake_dtb: + pathname = GetFdtPath(fname) + data = GetFdt(fname).GetContents() + else: + pathname = tools.GetInputFilename(fname) + data = tools.ReadFile(pathname) + return pathname, data + def SetEntryArgs(args): """Set the value of the entry args

@@ -133,6 +156,8 @@ def GetFdts(): Device trees being used (U-Boot proper, SPL, TPL) """ yield main_dtb + for other_fname in fdt_subset: + yield fdt_files[other_fname]

def GetUpdateNodes(node): """Yield all the nodes that need to be updated in all device trees @@ -149,6 +174,11 @@ def GetUpdateNodes(node): is node, SPL and TPL) """ yield node + for dtb in fdt_files.values(): + if dtb != node.GetFdt(): + other_node = dtb.GetNode(node.path) + if other_node: + yield other_node

def AddZeroProp(node, prop): """Add a new property to affected device trees with an integer value of 0. diff --git a/tools/binman/test/82_fdt_update_all.dts b/tools/binman/test/82_fdt_update_all.dts new file mode 100644 index 00000000000..284975cc289 --- /dev/null +++ b/tools/binman/test/82_fdt_update_all.dts @@ -0,0 +1,18 @@ +// SPDX-License-Identifier: GPL-2.0+ +/dts-v1/; + +/ { + #address-cells = <1>; + #size-cells = <1>; + + binman { + section { + u-boot-dtb { + }; + }; + u-boot-spl-dtb { + }; + u-boot-tpl-dtb { + }; + }; +};

When tools are needed but not present, at present we just get an error which can be confusing for the user. Try to be helpful by reporting the tool as missing and suggesting a possible remedy.

Also update the Run() method to support this.

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

tools/patman/tools.py | 29 ++++++++++++++++++++++++++++- 1 file changed, 28 insertions(+), 1 deletion(-)

diff --git a/tools/patman/tools.py b/tools/patman/tools.py index e80481438b5..0870bccca0e 100644 --- a/tools/patman/tools.py +++ b/tools/patman/tools.py @@ -22,6 +22,10 @@ chroot_path = None # Search paths to use for Filename(), used to find files search_paths = []

+# Tools and the packages that contain them, on debian +packages = { + 'lz4': 'liblz4-tool', + }

def PrepareOutputDir(dirname, preserve=False): """Select an output directory, ensuring it exists. @@ -128,8 +132,31 @@ def Align(pos, align): def NotPowerOfTwo(num): return num and (num & (num - 1))

+def PathHasFile(fname): + """Check if a given filename is in the PATH + + Args: + fname: Filename to check + + Returns: + True if found, False if not + """ + for dir in os.environ['PATH'].split(':'): + if os.path.exists(os.path.join(dir, fname)): + return True + return False + def Run(name, *args): - command.Run(name, *args, cwd=outdir) + try: + return command.Run(name, *args, cwd=outdir, capture=True) + except: + if not PathHasFile(name): + msg = "Plesae install tool '%s'" % name + package = packages.get(name) + if package: + msg += " (e.g. from package '%s')" % package + raise ValueError(msg) + raise

def Filename(fname): """Resolve a file path to an absolute path.

Add support for compressing blob entries. This can help reduce image sizes for many types of data. It requires that the firmware be able to decompress the data at run-time.

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

tools/binman/README | 16 ++++++++++ tools/binman/README.entries | 7 +++++ tools/binman/etype/blob.py | 49 +++++++++++++++++++++++++------ tools/binman/ftest.py | 32 ++++++++++++++++++++ tools/binman/test/83_compress.dts | 11 +++++++ 5 files changed, 106 insertions(+), 9 deletions(-) create mode 100644 tools/binman/test/83_compress.dts

diff --git a/tools/binman/README b/tools/binman/README index 3dc8b9b80a8..34b83110f64 100644 --- a/tools/binman/README +++ b/tools/binman/README @@ -593,6 +593,22 @@ the device tree. These can be used by U-Boot at run-time to find the location of each entry.

+Compression +----------- + +Binman support compression for 'blob' entries (those of type 'blob' and +derivatives). To enable this for an entry, add a 'compression' property: + + blob { + filename = "datafile"; + compression = "lz4"; + }; + +The entry will then contain the compressed data, using the 'lz4' compression +algorithm. Currently this is the only one that is supported. + + + Map files ---------

diff --git a/tools/binman/README.entries b/tools/binman/README.entries index 091fb5ce2b6..2cf7dc0338d 100644 --- a/tools/binman/README.entries +++ b/tools/binman/README.entries @@ -19,11 +19,18 @@ class by other entry types.

Properties / Entry arguments: - filename: Filename of file to read into entry + - compress: Compression algorithm to use: + none: No compression + lz4: Use lz4 compression (via 'lz4' command-line utility)

This entry reads data from a file and places it in the entry. The default filename is often specified specified by the subclass. See for example the 'u_boot' entry which provides the filename 'u-boot.bin'.

+If compression is enabled, an extra 'uncomp-size' property is written to +the node (if enabled with -u) which provides the uncompressed size of the +data. +

Entry: blob-dtb: A blob that holds a device tree diff --git a/tools/binman/etype/blob.py b/tools/binman/etype/blob.py index 3f46eecf308..642a0e482a7 100644 --- a/tools/binman/etype/blob.py +++ b/tools/binman/etype/blob.py @@ -7,6 +7,7 @@

from entry import Entry import fdt_util +import state import tools

class Entry_blob(Entry): @@ -17,14 +18,23 @@ class Entry_blob(Entry):

Properties / Entry arguments: - filename: Filename of file to read into entry + - compress: Compression algorithm to use: + none: No compression + lz4: Use lz4 compression (via 'lz4' command-line utility)

This entry reads data from a file and places it in the entry. The default filename is often specified specified by the subclass. See for example the 'u_boot' entry which provides the filename 'u-boot.bin'. + + If compression is enabled, an extra 'uncomp-size' property is written to + the node (if enabled with -u) which provides the uncompressed size of the + data. """ def __init__(self, section, etype, node): Entry.__init__(self, section, etype, node) - self._filename = fdt_util.GetString(self._node, "filename", self.etype) + self._filename = fdt_util.GetString(self._node, 'filename', self.etype) + self._compress = fdt_util.GetString(self._node, 'compress', 'none') + self._uncompressed_size = None

def ObtainContents(self): self._filename = self.GetDefaultFilename() @@ -33,15 +43,36 @@ class Entry_blob(Entry): return True

def ReadBlobContents(self): - with open(self._pathname) as fd: - # We assume the data is small enough to fit into memory. If this - # is used for large filesystem image that might not be true. - # In that case, Image.BuildImage() could be adjusted to use a - # new Entry method which can read in chunks. Then we could copy - # the data in chunks and avoid reading it all at once. For now - # this seems like an unnecessary complication. - self.SetContents(fd.read()) + # We assume the data is small enough to fit into memory. If this + # is used for large filesystem image that might not be true. + # In that case, Image.BuildImage() could be adjusted to use a + # new Entry method which can read in chunks. Then we could copy + # the data in chunks and avoid reading it all at once. For now + # this seems like an unnecessary complication. + data = tools.ReadFile(self._pathname) + if self._compress == 'lz4': + self._uncompressed_size = len(data) + ''' + import lz4 # Import this only if needed (python-lz4 dependency) + + try: + data = lz4.frame.compress(data) + except AttributeError: + data = lz4.compress(data) + ''' + data = tools.Run('lz4', '-c', self._pathname, ) + self.SetContents(data) return True

def GetDefaultFilename(self): return self._filename + + def AddMissingProperties(self): + Entry.AddMissingProperties(self) + if self._compress != 'none': + state.AddZeroProp(self._node, 'uncomp-size') + + def SetCalculatedProperties(self): + Entry.SetCalculatedProperties(self) + if self._uncompressed_size is not None: + state.SetInt(self._node, 'uncomp-size', self._uncompressed_size) diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index 6bfef7b63a6..c5e7236c424 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -6,6 +6,7 @@ # # python -m unittest func_test.TestFunctional.testHelp

+import lz4 from optparse import OptionParser import os import shutil @@ -54,6 +55,7 @@ CROS_EC_RW_DATA = 'ecrw' GBB_DATA = 'gbbd' BMPBLK_DATA = 'bmp' VBLOCK_DATA = 'vblk' +COMPRESS_DATA = 'data to compress'

class TestFunctional(unittest.TestCase): @@ -116,6 +118,8 @@ class TestFunctional(unittest.TestCase): with open(self.TestFile('descriptor.bin')) as fd: TestFunctional._MakeInputFile('descriptor.bin', fd.read())

+ TestFunctional._MakeInputFile('compress', COMPRESS_DATA) + @classmethod def tearDownClass(self): """Remove the temporary input directory and its contents""" @@ -1505,6 +1509,34 @@ class TestFunctional(unittest.TestCase): finally: self._ResetDtbs()

+ def _decompress(self, data): + out = os.path.join(self._indir, 'lz4.tmp') + with open(out, 'wb') as fd: + fd.write(data) + return tools.Run('lz4', '-dc', out) + ''' + try: + orig = lz4.frame.decompress(data) + except AttributeError: + orig = lz4.decompress(data) + ''' + + def testCompress(self): + """Test compression of blobs""" + data, _, _, out_dtb_fname = self._DoReadFileDtb('83_compress.dts', + use_real_dtb=True, update_dtb=True) + dtb = fdt.Fdt(out_dtb_fname) + dtb.Scan() + props = self._GetPropTree(dtb, ['size', 'uncomp-size']) + orig = self._decompress(data) + self.assertEquals(COMPRESS_DATA, orig) + expected = { + 'blob:uncomp-size': len(COMPRESS_DATA), + 'blob:size': len(data), + 'size': len(data), + } + self.assertEqual(expected, props) +

if __name__ == "__main__": unittest.main() diff --git a/tools/binman/test/83_compress.dts b/tools/binman/test/83_compress.dts new file mode 100644 index 00000000000..07813bdeaa3 --- /dev/null +++ b/tools/binman/test/83_compress.dts @@ -0,0 +1,11 @@ +// SPDX-License-Identifier: GPL-2.0+ +/dts-v1/; + +/ { + binman { + blob { + filename = "compress"; + compress = "lz4"; + }; + }; +};

At present if there is only a zero-size entry in a section this is reported as an error, e.g.:

Offset 0x0 (0) is outside the section starting at 0x0 (0)

Adjust the logic in CheckEntries() to avoid this.

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

tools/binman/bsection.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/binman/bsection.py b/tools/binman/bsection.py index 44adb82795b..1c37d84e99a 100644 --- a/tools/binman/bsection.py +++ b/tools/binman/bsection.py @@ -258,7 +258,7 @@ class Section(object): for entry in self._entries.values(): entry.CheckOffset() if (entry.offset < self._skip_at_start or - entry.offset >= self._skip_at_start + self._size): + 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,

In some cases it is useful to add a group of files to the image and be able to access them at run-time. Of course it is possible to generate the binman config file with a set of blobs each with a filename. But for convenience, add an entry type which can do this.

Add required support (for adding nodes and string properties) into the state module.

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

tools/binman/README.entries | 15 +++++ tools/binman/bsection.py | 4 ++ tools/binman/control.py | 1 + tools/binman/entry.py | 3 + tools/binman/etype/files.py | 57 +++++++++++++++++++ tools/binman/ftest.py | 43 ++++++++++++++ tools/binman/image.py | 9 +++ tools/binman/state.py | 27 +++++++++ tools/binman/test/84_files.dts | 11 ++++ tools/binman/test/85_files_compress.dts | 11 ++++ tools/binman/test/86_files_none.dts | 12 ++++ tools/binman/test/87_files_no_pattern.dts | 11 ++++ tools/binman/test/files/1.dat | 1 + tools/binman/test/files/2.dat | 1 + .../binman/test/files/ignored_dir.dat/ignore | 0 tools/binman/test/files/not-this-one | 1 + tools/patman/tools.py | 18 ++++++ 17 files changed, 225 insertions(+) create mode 100644 tools/binman/etype/files.py create mode 100644 tools/binman/test/84_files.dts create mode 100644 tools/binman/test/85_files_compress.dts create mode 100644 tools/binman/test/86_files_none.dts create mode 100644 tools/binman/test/87_files_no_pattern.dts create mode 100644 tools/binman/test/files/1.dat create mode 100644 tools/binman/test/files/2.dat create mode 100644 tools/binman/test/files/ignored_dir.dat/ignore create mode 100644 tools/binman/test/files/not-this-one

diff --git a/tools/binman/README.entries b/tools/binman/README.entries index 2cf7dc0338d..3afc560052a 100644 --- a/tools/binman/README.entries +++ b/tools/binman/README.entries @@ -71,6 +71,21 @@ updating the EC on startup via software sync.

+Entry: files: Entry containing a set of files +--------------------------------------------- + +Properties / Entry arguments: + - pattern: Filename pattern to match the files to include + - compress: Compression algorithm to use: + none: No compression + lz4: Use lz4 compression (via 'lz4' command-line utility) + +This entry reads a number of files and places each in a separate sub-entry +within this entry. To access these you need to enable device-tree updates +at run-time so you can obtain the file positions. + + + Entry: fill: An entry which is filled to a particular byte value ----------------------------------------------------------------

diff --git a/tools/binman/bsection.py b/tools/binman/bsection.py index 1c37d84e99a..4bf206878dc 100644 --- a/tools/binman/bsection.py +++ b/tools/binman/bsection.py @@ -103,6 +103,10 @@ class Section(object): 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']: diff --git a/tools/binman/control.py b/tools/binman/control.py index e326456a4ba..caa194c8990 100644 --- a/tools/binman/control.py +++ b/tools/binman/control.py @@ -146,6 +146,7 @@ def Binman(options, args): # without changing the device-tree size, thus ensuring that our # entry offsets remain the same. for image in images.values(): + image.ExpandEntries() if options.update_fdt: image.AddMissingProperties() image.ProcessFdt(dtb) diff --git a/tools/binman/entry.py b/tools/binman/entry.py index f922107523e..7316ad43b5e 100644 --- a/tools/binman/entry.py +++ b/tools/binman/entry.py @@ -179,6 +179,9 @@ class Entry(object): return Set([fname]) return Set()

+ def ExpandEntries(self): + pass + def AddMissingProperties(self): """Add new properties to the device tree as needed for this entry""" for prop in ['offset', 'size', 'image-pos']: diff --git a/tools/binman/etype/files.py b/tools/binman/etype/files.py new file mode 100644 index 00000000000..99f2f2f67fe --- /dev/null +++ b/tools/binman/etype/files.py @@ -0,0 +1,57 @@ +# SPDX-License-Identifier: GPL-2.0+ +# Copyright (c) 2018 Google, Inc +# Written by Simon Glass sjg@chromium.org +# +# Entry-type module for a set of files which are placed in individual +# sub-entries +# + +import glob +import os + +from section import Entry_section +import fdt_util +import state +import tools + +import bsection + +class Entry_files(Entry_section): + """Entry containing a set of files + + Properties / Entry arguments: + - pattern: Filename pattern to match the files to include + - compress: Compression algorithm to use: + none: No compression + lz4: Use lz4 compression (via 'lz4' command-line utility) + + This entry reads a number of files and places each in a separate sub-entry + within this entry. To access these you need to enable device-tree updates + at run-time so you can obtain the file positions. + """ + def __init__(self, section, etype, node): + Entry_section.__init__(self, section, etype, node) + self._pattern = fdt_util.GetString(self._node, 'pattern') + if not self._pattern: + self.Raise("Missing 'pattern' property") + self._compress = fdt_util.GetString(self._node, 'compress', 'none') + self._require_matches = fdt_util.GetBool(self._node, + 'require-matches') + + def ExpandEntries(self): + files = tools.GetInputFilenameGlob(self._pattern) + if self._require_matches and not files: + self.Raise("Pattern '%s' matched no files" % self._pattern) + for fname in files: + if not os.path.isfile(fname): + continue + name = os.path.basename(fname) + subnode = self._node.FindNode(name) + if not subnode: + subnode = state.AddSubnode(self._node, name) + state.AddString(subnode, 'type', 'blob') + state.AddString(subnode, 'filename', fname) + state.AddString(subnode, 'compress', self._compress) + + # Read entries again, now that we have some + self._section._ReadEntries() diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index c5e7236c424..36eee398cc6 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -55,6 +55,8 @@ CROS_EC_RW_DATA = 'ecrw' GBB_DATA = 'gbbd' BMPBLK_DATA = 'bmp' VBLOCK_DATA = 'vblk' +FILES_DATA = ("sorry I'm late\nOh, don't bother apologising, I'm " + + "sorry you're alive\n") COMPRESS_DATA = 'data to compress'

@@ -118,6 +120,9 @@ class TestFunctional(unittest.TestCase): with open(self.TestFile('descriptor.bin')) as fd: TestFunctional._MakeInputFile('descriptor.bin', fd.read())

+ shutil.copytree(self.TestFile('files'), + os.path.join(self._indir, 'files')) + TestFunctional._MakeInputFile('compress', COMPRESS_DATA)

@classmethod @@ -1537,6 +1542,44 @@ class TestFunctional(unittest.TestCase): } self.assertEqual(expected, props)

+ def testFiles(self): + """Test bringing in multiple files""" + data = self._DoReadFile('84_files.dts') + self.assertEqual(FILES_DATA, data) + + def testFilesCompress(self): + """Test bringing in multiple files and compressing them""" + data = self._DoReadFile('85_files_compress.dts') + + image = control.images['image'] + entries = image.GetEntries() + files = entries['files'] + entries = files._section._entries + + orig = '' + for i in range(1, 3): + key = '%d.dat' % i + start = entries[key].image_pos + len = entries[key].size + chunk = data[start:start + len] + orig += self._decompress(chunk) + + self.assertEqual(FILES_DATA, orig) + + def testFilesMissing(self): + """Test missing files""" + with self.assertRaises(ValueError) as e: + data = self._DoReadFile('86_files_none.dts') + self.assertIn("Node '/binman/files': Pattern 'files/*.none' matched " + 'no files', str(e.exception)) + + def testFilesNoPattern(self): + """Test missing files""" + with self.assertRaises(ValueError) as e: + data = self._DoReadFile('87_files_no_pattern.dts') + self.assertIn("Node '/binman/files': Missing 'pattern' property", + str(e.exception)) +

if __name__ == "__main__": unittest.main() diff --git a/tools/binman/image.py b/tools/binman/image.py index bfb22297797..4b922b51c42 100644 --- a/tools/binman/image.py +++ b/tools/binman/image.py @@ -58,6 +58,15 @@ class Image: """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

diff --git a/tools/binman/state.py b/tools/binman/state.py index 09ead442a60..2f8c0863a8f 100644 --- a/tools/binman/state.py +++ b/tools/binman/state.py @@ -189,6 +189,33 @@ def AddZeroProp(node, prop): for n in GetUpdateNodes(node): n.AddZeroProp(prop)

+def AddSubnode(node, name): + """Add a new subnode to a node in affected device trees + + Args: + node: Node to add to + name: name of node to add + + Returns: + New subnode that was created in main tree + """ + first = None + for n in GetUpdateNodes(node): + subnode = n.AddSubnode(name) + if not first: + first = subnode + return first + +def AddString(node, prop, value): + """Add a new string property to affected device trees + + Args: + prop_name: Name of property + value: String value (which will be \0-terminated in the DT) + """ + for n in GetUpdateNodes(node): + n.AddString(prop, value) + def SetInt(node, prop, value): """Update an integer property in affected device trees with an integer value

diff --git a/tools/binman/test/84_files.dts b/tools/binman/test/84_files.dts new file mode 100644 index 00000000000..83ddb78f8e7 --- /dev/null +++ b/tools/binman/test/84_files.dts @@ -0,0 +1,11 @@ +// SPDX-License-Identifier: GPL-2.0+ +/dts-v1/; + +/ { + binman { + files { + pattern = "files/*.dat"; + compress = "none"; + }; + }; +}; diff --git a/tools/binman/test/85_files_compress.dts b/tools/binman/test/85_files_compress.dts new file mode 100644 index 00000000000..847b398bf2b --- /dev/null +++ b/tools/binman/test/85_files_compress.dts @@ -0,0 +1,11 @@ +// SPDX-License-Identifier: GPL-2.0+ +/dts-v1/; + +/ { + binman { + files { + pattern = "files/*.dat"; + compress = "lz4"; + }; + }; +}; diff --git a/tools/binman/test/86_files_none.dts b/tools/binman/test/86_files_none.dts new file mode 100644 index 00000000000..34bd92f224a --- /dev/null +++ b/tools/binman/test/86_files_none.dts @@ -0,0 +1,12 @@ +// SPDX-License-Identifier: GPL-2.0+ +/dts-v1/; + +/ { + binman { + files { + pattern = "files/*.none"; + compress = "none"; + require-matches; + }; + }; +}; diff --git a/tools/binman/test/87_files_no_pattern.dts b/tools/binman/test/87_files_no_pattern.dts new file mode 100644 index 00000000000..0cb5b469cb0 --- /dev/null +++ b/tools/binman/test/87_files_no_pattern.dts @@ -0,0 +1,11 @@ +// SPDX-License-Identifier: GPL-2.0+ +/dts-v1/; + +/ { + binman { + files { + compress = "none"; + require-matches; + }; + }; +}; diff --git a/tools/binman/test/files/1.dat b/tools/binman/test/files/1.dat new file mode 100644 index 00000000000..a9524706171 --- /dev/null +++ b/tools/binman/test/files/1.dat @@ -0,0 +1 @@ +sorry I'm late diff --git a/tools/binman/test/files/2.dat b/tools/binman/test/files/2.dat new file mode 100644 index 00000000000..687ea52730d --- /dev/null +++ b/tools/binman/test/files/2.dat @@ -0,0 +1 @@ +Oh, don't bother apologising, I'm sorry you're alive diff --git a/tools/binman/test/files/ignored_dir.dat/ignore b/tools/binman/test/files/ignored_dir.dat/ignore new file mode 100644 index 00000000000..e69de29bb2d diff --git a/tools/binman/test/files/not-this-one b/tools/binman/test/files/not-this-one new file mode 100644 index 00000000000..e71c2250f96 --- /dev/null +++ b/tools/binman/test/files/not-this-one @@ -0,0 +1 @@ +this does not have a .dat extenion diff --git a/tools/patman/tools.py b/tools/patman/tools.py index 0870bccca0e..6e694433475 100644 --- a/tools/patman/tools.py +++ b/tools/patman/tools.py @@ -4,6 +4,7 @@ #

import command +import glob import os import shutil import tempfile @@ -123,6 +124,23 @@ def GetInputFilename(fname): raise ValueError("Filename '%s' not found in input path (%s) (cwd='%s')" % (fname, ','.join(indir), os.getcwd()))

+def GetInputFilenameGlob(pattern): + """Return a list of filenames for use as input. + + Args: + pattern: Filename pattern to search for + + Returns: + A list of matching files in all input directories + """ + if not indir: + return glob.glob(fname) + files = [] + for dirname in indir: + pathname = os.path.join(dirname, pattern) + files += glob.glob(pathname) + return files + def Align(pos, align): if align: mask = align - 1

It is useful to have entries which can grow automatically to fill available space. Add support for this.

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

tools/binman/README | 4 +++ tools/binman/bsection.py | 22 +++++++++++- tools/binman/entry.py | 11 ++++++ tools/binman/etype/_testing.py | 7 +++- tools/binman/etype/section.py | 8 +++++ tools/binman/ftest.py | 29 ++++++++++++++++ tools/binman/test/88_expand_size.dts | 43 ++++++++++++++++++++++++ tools/binman/test/89_expand_size_bad.dts | 14 ++++++++ 8 files changed, 136 insertions(+), 2 deletions(-) create mode 100644 tools/binman/test/88_expand_size.dts create mode 100644 tools/binman/test/89_expand_size_bad.dts

diff --git a/tools/binman/README b/tools/binman/README index 34b83110f64..cd1ad2fb0fe 100644 --- a/tools/binman/README +++ b/tools/binman/README @@ -330,6 +330,10 @@ image-pos: for each entry. This makes it easy to find out exactly where the entry ended up in the image, regardless of parent sections, etc.

+expand-size: + Expand the size of this entry to fit available space. This space is only + limited by the size of the image/section and the position of the next + entry.

The attributes supported for images are described below. Several are similar to those for entries. diff --git a/tools/binman/bsection.py b/tools/binman/bsection.py index 4bf206878dc..52ac31a4672 100644 --- a/tools/binman/bsection.py +++ b/tools/binman/bsection.py @@ -253,10 +253,26 @@ class Section(object): 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""" + """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(): @@ -419,3 +435,7 @@ class Section(object): return None return entry.data source_entry.Raise("Cannot find entry for node '%s'" % node.name) + + def ExpandSize(self, size): + if size != self._size: + self._size = size diff --git a/tools/binman/entry.py b/tools/binman/entry.py index 7316ad43b5e..0915b470b4e 100644 --- a/tools/binman/entry.py +++ b/tools/binman/entry.py @@ -76,6 +76,7 @@ class Entry(object): self.pad_after = 0 self.offset_unset = False self.image_pos = None + self._expand_size = False if read_node: self.ReadNode()

@@ -161,6 +162,7 @@ class Entry(object): "of two" % (self._node.path, 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')

def GetDefaultFilename(self): return None @@ -507,3 +509,12 @@ features to produce new behaviours. break name = '%s.%s' % (node.name, name) return name + + def ExpandToLimit(self, limit): + """Expand an entry so that it ends at the given offset limit""" + if self.offset + self.size < limit: + self.size = limit - self.offset + # Request the contents again, since changing the size requires that + # the data grows. This should not fail, but check it to be sure. + if not self.ObtainContents(): + self.Raise('Cannot obtain contents when expanding entry') diff --git a/tools/binman/etype/_testing.py b/tools/binman/etype/_testing.py index 02c165c0c3e..3e345bd9526 100644 --- a/tools/binman/etype/_testing.py +++ b/tools/binman/etype/_testing.py @@ -48,6 +48,8 @@ class Entry__testing(Entry): 'return-unknown-contents') self.bad_update_contents = fdt_util.GetBool(self._node, 'bad-update-contents') + self.return_contents_once = fdt_util.GetBool(self._node, + 'return-contents-once')

# Set to True when the entry is ready to process the FDT. self.process_fdt_ready = False @@ -68,12 +70,15 @@ class Entry__testing(Entry): EntryArg('test-existing-prop', str)], self.require_args) if self.force_bad_datatype: self.GetEntryArgsOrProps([EntryArg('test-bad-datatype-arg', bool)]) + self.return_contents = True

def ObtainContents(self): - if self.return_unknown_contents: + if self.return_unknown_contents or not self.return_contents: return False self.data = 'a' self.contents_size = len(self.data) + if self.return_contents_once: + self.return_contents = False return True

def GetOffsets(self): diff --git a/tools/binman/etype/section.py b/tools/binman/etype/section.py index a30cc915457..005a9f9cb2e 100644 --- a/tools/binman/etype/section.py +++ b/tools/binman/etype/section.py @@ -40,6 +40,10 @@ class Entry_section(Entry): def ProcessFdt(self, fdt): return self._section.ProcessFdt(fdt)

+ def ExpandEntries(self): + Entry.ExpandEntries(self) + self._section.ExpandEntries() + def AddMissingProperties(self): Entry.AddMissingProperties(self) self._section.AddMissingProperties() @@ -95,3 +99,7 @@ class Entry_section(Entry):

def GetEntries(self): return self._section.GetEntries() + + def ExpandToLimit(self, limit): + super(Entry_section, self).ExpandToLimit(limit) + self._section.ExpandSize(self.size) diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index 36eee398cc6..faacb6fa8f6 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -1580,6 +1580,35 @@ class TestFunctional(unittest.TestCase): self.assertIn("Node '/binman/files': Missing 'pattern' property", str(e.exception))

+ def testExpandSize(self): + """Test an expanding entry""" + data, _, map_data, _ = self._DoReadFileDtb('88_expand_size.dts', + map=True) + expect = ('a' * 8 + U_BOOT_DATA + + MRC_DATA + 'b' * 1 + U_BOOT_DATA + + 'c' * 8 + U_BOOT_DATA + + 'd' * 8) + self.assertEqual(expect, data) + self.assertEqual('''ImagePos Offset Size Name +00000000 00000000 00000028 main-section +00000000 00000000 00000008 fill +00000008 00000008 00000004 u-boot +0000000c 0000000c 00000004 section +0000000c 00000000 00000003 intel-mrc +00000010 00000010 00000004 u-boot2 +00000014 00000014 0000000c section2 +00000014 00000000 00000008 fill +0000001c 00000008 00000004 u-boot +00000020 00000020 00000008 fill2 +''', map_data) + + def testExpandSizeBad(self): + """Test an expanding entry which fails to provide contents""" + with self.assertRaises(ValueError) as e: + self._DoReadFileDtb('89_expand_size_bad.dts', map=True) + self.assertIn("Node '/binman/_testing': Cannot obtain contents when " + 'expanding entry', str(e.exception)) +

if __name__ == "__main__": unittest.main() diff --git a/tools/binman/test/88_expand_size.dts b/tools/binman/test/88_expand_size.dts new file mode 100644 index 00000000000..c8a01308ec5 --- /dev/null +++ b/tools/binman/test/88_expand_size.dts @@ -0,0 +1,43 @@ +// SPDX-License-Identifier: GPL-2.0+ +/dts-v1/; + +/ { + binman { + size = <40>; + fill { + expand-size; + fill-byte = [61]; + size = <0>; + }; + u-boot { + offset = <8>; + }; + section { + expand-size; + pad-byte = <0x62>; + intel-mrc { + }; + }; + u-boot2 { + type = "u-boot"; + offset = <16>; + }; + section2 { + type = "section"; + fill { + expand-size; + fill-byte = [63]; + size = <0>; + }; + u-boot { + offset = <8>; + }; + }; + fill2 { + type = "fill"; + expand-size; + fill-byte = [64]; + size = <0>; + }; + }; +}; diff --git a/tools/binman/test/89_expand_size_bad.dts b/tools/binman/test/89_expand_size_bad.dts new file mode 100644 index 00000000000..edc0e5cf681 --- /dev/null +++ b/tools/binman/test/89_expand_size_bad.dts @@ -0,0 +1,14 @@ +// SPDX-License-Identifier: GPL-2.0+ +/dts-v1/; + +/ { + binman { + _testing { + expand-size; + return-contents-once; + }; + u-boot { + offset = <8>; + }; + }; +};

Images and sections have the same attributes, since an image is mostly just a top-level section. Update the docs to explain this.

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

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

diff --git a/tools/binman/README b/tools/binman/README index cd1ad2fb0fe..7e566ffe05b 100644 --- a/tools/binman/README +++ b/tools/binman/README @@ -335,8 +335,8 @@ expand-size: limited by the size of the image/section and the position of the next entry.

-The attributes supported for images are described below. Several are similar -to those for entries. +The attributes supported for images and sections are described below. Several +are similar to those for entries.

size: Sets the image size in bytes, for example 'size = <0x100000>' for a

Sometimesi it us useful to be able to verify the content of entries with a hash. Add an easy way to do this in binman. The hash information can be retrieved from the device tree at run time.

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

tools/binman/README | 22 ++++++++++++++++ tools/binman/bsection.py | 3 +++ tools/binman/entry.py | 4 +++ tools/binman/ftest.py | 36 ++++++++++++++++++++++++++ tools/binman/state.py | 25 ++++++++++++++++++ tools/binman/test/90_hash.dts | 12 +++++++++ tools/binman/test/91_hash_no_algo.dts | 11 ++++++++ tools/binman/test/92_hash_bad_algo.dts | 12 +++++++++ tools/binman/test/99_hash_section.dts | 18 +++++++++++++ 9 files changed, 143 insertions(+) create mode 100644 tools/binman/test/90_hash.dts create mode 100644 tools/binman/test/91_hash_no_algo.dts create mode 100644 tools/binman/test/92_hash_bad_algo.dts create mode 100644 tools/binman/test/99_hash_section.dts

diff --git a/tools/binman/README b/tools/binman/README index 7e566ffe05b..6bdb7e4c66e 100644 --- a/tools/binman/README +++ b/tools/binman/README @@ -466,6 +466,28 @@ see README.entries. This is generated from the source code using: binman -E >tools/binman/README.entries

+Hashing Entries +--------------- + +It is possible to ask binman to hash the contents of an entry and write that +value back to the device-tree node. For example: + + binman { + u-boot { + hash { + algo = "sha256"; + }; + }; + }; + +Here, a new 'value' property will be written to the 'hash' node containing +the hash of the 'u-boot' entry. Only SHA256 is supported at present. Whole +sections can be hased if desired, by adding the 'hash' node to the section. + +The has value can be chcked at runtime by hashing the data actually read and +comparing this has to the value in the device tree. + + Order of image creation -----------------------

diff --git a/tools/binman/bsection.py b/tools/binman/bsection.py index 52ac31a4672..650e9ba353f 100644 --- a/tools/binman/bsection.py +++ b/tools/binman/bsection.py @@ -89,6 +89,8 @@ class Section(object):

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 @@ -112,6 +114,7 @@ class Section(object): 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()

diff --git a/tools/binman/entry.py b/tools/binman/entry.py index 0915b470b4e..fd7223477eb 100644 --- a/tools/binman/entry.py +++ b/tools/binman/entry.py @@ -189,12 +189,16 @@ class Entry(object): for prop in ['offset', 'size', 'image-pos']: if not prop in self._node.props: state.AddZeroProp(self._node, prop) + err = state.CheckAddHashProp(self._node) + if err: + self.Raise(err)

def SetCalculatedProperties(self): """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) + state.CheckSetHashValue(self._node, self.GetData)

def ProcessFdt(self, fdt): """Allow entries to adjust the device tree diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index faacb6fa8f6..6c86f4a742c 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -6,6 +6,7 @@ # # python -m unittest func_test.TestFunctional.testHelp

+import hashlib import lz4 from optparse import OptionParser import os @@ -1609,6 +1610,41 @@ class TestFunctional(unittest.TestCase): self.assertIn("Node '/binman/_testing': Cannot obtain contents when " 'expanding entry', str(e.exception))

+ def testHash(self): + """Test hashing of the contents of an entry""" + _, _, _, out_dtb_fname = self._DoReadFileDtb('90_hash.dts', + use_real_dtb=True, update_dtb=True) + dtb = fdt.Fdt(out_dtb_fname) + dtb.Scan() + hash_node = dtb.GetNode('/binman/u-boot/hash').props['value'] + m = hashlib.sha256() + m.update(U_BOOT_DATA) + self.assertEqual(m.digest(), ''.join(hash_node.value)) + + def testHashNoAlgo(self): + with self.assertRaises(ValueError) as e: + self._DoReadFileDtb('91_hash_no_algo.dts', update_dtb=True) + self.assertIn("Node '/binman/u-boot': Missing 'algo' property for " + 'hash node', str(e.exception)) + + def testHashBadAlgo(self): + with self.assertRaises(ValueError) as e: + self._DoReadFileDtb('92_hash_bad_algo.dts', update_dtb=True) + self.assertIn("Node '/binman/u-boot': Unknown hash algorithm", + str(e.exception)) + + def testHashSection(self): + """Test hashing of the contents of an entry""" + _, _, _, out_dtb_fname = self._DoReadFileDtb('99_hash_section.dts', + use_real_dtb=True, update_dtb=True) + dtb = fdt.Fdt(out_dtb_fname) + dtb.Scan() + hash_node = dtb.GetNode('/binman/section/hash').props['value'] + m = hashlib.sha256() + m.update(U_BOOT_DATA) + m.update(16 * 'a') + self.assertEqual(m.digest(), ''.join(hash_node.value)) +

if __name__ == "__main__": unittest.main() diff --git a/tools/binman/state.py b/tools/binman/state.py index 2f8c0863a8f..d945e4bf657 100644 --- a/tools/binman/state.py +++ b/tools/binman/state.py @@ -5,6 +5,7 @@ # Holds and modifies the state information held by binman #

+import hashlib import re from sets import Set

@@ -226,3 +227,27 @@ def SetInt(node, prop, value): """ for n in GetUpdateNodes(node): n.SetInt(prop, value) + +def CheckAddHashProp(node): + hash_node = node.FindNode('hash') + if hash_node: + algo = hash_node.props.get('algo') + if not algo: + return "Missing 'algo' property for hash node" + if algo.value == 'sha256': + size = 32 + else: + return "Unknown hash algorithm '%s'" % algo + for n in GetUpdateNodes(hash_node): + n.AddEmptyProp('value', size) + +def CheckSetHashValue(node, get_data_func): + hash_node = node.FindNode('hash') + if hash_node: + algo = hash_node.props.get('algo').value + if algo == 'sha256': + m = hashlib.sha256() + m.update(get_data_func()) + data = m.digest() + for n in GetUpdateNodes(hash_node): + n.SetData('value', data) diff --git a/tools/binman/test/90_hash.dts b/tools/binman/test/90_hash.dts new file mode 100644 index 00000000000..200304599dc --- /dev/null +++ b/tools/binman/test/90_hash.dts @@ -0,0 +1,12 @@ +// SPDX-License-Identifier: GPL-2.0+ +/dts-v1/; + +/ { + binman { + u-boot { + hash { + algo = "sha256"; + }; + }; + }; +}; diff --git a/tools/binman/test/91_hash_no_algo.dts b/tools/binman/test/91_hash_no_algo.dts new file mode 100644 index 00000000000..b64df205117 --- /dev/null +++ b/tools/binman/test/91_hash_no_algo.dts @@ -0,0 +1,11 @@ +// SPDX-License-Identifier: GPL-2.0+ +/dts-v1/; + +/ { + binman { + u-boot { + hash { + }; + }; + }; +}; diff --git a/tools/binman/test/92_hash_bad_algo.dts b/tools/binman/test/92_hash_bad_algo.dts new file mode 100644 index 00000000000..d2402000db6 --- /dev/null +++ b/tools/binman/test/92_hash_bad_algo.dts @@ -0,0 +1,12 @@ +// SPDX-License-Identifier: GPL-2.0+ +/dts-v1/; + +/ { + binman { + u-boot { + hash { + algo = "invalid"; + }; + }; + }; +}; diff --git a/tools/binman/test/99_hash_section.dts b/tools/binman/test/99_hash_section.dts new file mode 100644 index 00000000000..dcd8683d642 --- /dev/null +++ b/tools/binman/test/99_hash_section.dts @@ -0,0 +1,18 @@ +// SPDX-License-Identifier: GPL-2.0+ +/dts-v1/; + +/ { + binman { + section { + u-boot { + }; + fill { + size = <0x10>; + fill-byte = [61]; + }; + hash { + algo = "sha256"; + }; + }; + }; +};

When TPL is used on x86 we may want to program the microcode (at least for the first CPU) early in boot. Add support for this by refactoring the existing code to be more generic.

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

tools/binman/README.entries | 20 +++++++++++++ tools/binman/etype/u_boot_dtb_with_ucode.py | 15 ++++++---- .../binman/etype/u_boot_spl_with_ucode_ptr.py | 2 ++ .../binman/etype/u_boot_tpl_dtb_with_ucode.py | 25 ++++++++++++++++ .../binman/etype/u_boot_tpl_with_ucode_ptr.py | 27 +++++++++++++++++ tools/binman/etype/u_boot_ucode.py | 26 +++++++++-------- tools/binman/etype/u_boot_with_ucode_ptr.py | 9 ++++-- tools/binman/ftest.py | 19 ++++++++++++ tools/binman/test/93_x86_tpl_ucode.dts | 29 +++++++++++++++++++ 9 files changed, 151 insertions(+), 21 deletions(-) create mode 100644 tools/binman/etype/u_boot_tpl_dtb_with_ucode.py create mode 100644 tools/binman/etype/u_boot_tpl_with_ucode_ptr.py create mode 100644 tools/binman/test/93_x86_tpl_ucode.dts

diff --git a/tools/binman/README.entries b/tools/binman/README.entries index 3afc560052a..4dd67d64fa7 100644 --- a/tools/binman/README.entries +++ b/tools/binman/README.entries @@ -462,6 +462,8 @@ both SPL and the device tree). Entry: u-boot-spl-with-ucode-ptr: U-Boot SPL with embedded microcode pointer ----------------------------------------------------------------------------

+This is used when SPL must set up the microcode for U-Boot. + See Entry_u_boot_ucode for full details of the entries involved in this process.

@@ -503,6 +505,24 @@ to activate.

+Entry: u-boot-tpl-dtb-with-ucode: U-Boot TPL with embedded microcode pointer +---------------------------------------------------------------------------- + +This is used when TPL must set up the microcode for U-Boot. + +See Entry_u_boot_ucode for full details of the entries involved in this +process. + + + +Entry: u-boot-tpl-with-ucode-ptr: U-Boot TPL with embedded microcode pointer +---------------------------------------------------------------------------- + +See Entry_u_boot_ucode for full details of the entries involved in this +process. + + + Entry: u-boot-ucode: U-Boot microcode block -------------------------------------------

diff --git a/tools/binman/etype/u_boot_dtb_with_ucode.py b/tools/binman/etype/u_boot_dtb_with_ucode.py index 73f5fbf9033..444c51b8b72 100644 --- a/tools/binman/etype/u_boot_dtb_with_ucode.py +++ b/tools/binman/etype/u_boot_dtb_with_ucode.py @@ -43,6 +43,9 @@ class Entry_u_boot_dtb_with_ucode(Entry_blob_dtb): # If the section does not need microcode, there is nothing to do ucode_dest_entry = self.section.FindEntryType( 'u-boot-spl-with-ucode-ptr') + if not ucode_dest_entry or not ucode_dest_entry.target_offset: + ucode_dest_entry = self.section.FindEntryType( + 'u-boot-tpl-with-ucode-ptr') if not ucode_dest_entry or not ucode_dest_entry.target_offset: ucode_dest_entry = self.section.FindEntryType( 'u-boot-with-ucode-ptr') @@ -70,14 +73,14 @@ class Entry_u_boot_dtb_with_ucode(Entry_blob_dtb): def ObtainContents(self): # Call the base class just in case it does something important. Entry_blob_dtb.ObtainContents(self) - self._pathname = state.GetFdtPath(self._filename) - self.ReadBlobContents() - if self.ucode: + if self.ucode and not self.collate: for node in self.ucode.subnodes: data_prop = node.props.get('data') - if data_prop and not self.collate: + if data_prop: # Find the offset in the device tree of the ucode data self.ucode_offset = data_prop.GetOffset() + 12 self.ucode_size = len(data_prop.bytes) - self.ready = True - return True + self.ready = True + else: + self.ready = True + return self.ready diff --git a/tools/binman/etype/u_boot_spl_with_ucode_ptr.py b/tools/binman/etype/u_boot_spl_with_ucode_ptr.py index 0dfe268a568..b650cf0146c 100644 --- a/tools/binman/etype/u_boot_spl_with_ucode_ptr.py +++ b/tools/binman/etype/u_boot_spl_with_ucode_ptr.py @@ -16,6 +16,8 @@ import tools class Entry_u_boot_spl_with_ucode_ptr(Entry_u_boot_with_ucode_ptr): """U-Boot SPL with embedded microcode pointer

+ This is used when SPL must set up the microcode for U-Boot. + See Entry_u_boot_ucode for full details of the entries involved in this process. """ diff --git a/tools/binman/etype/u_boot_tpl_dtb_with_ucode.py b/tools/binman/etype/u_boot_tpl_dtb_with_ucode.py new file mode 100644 index 00000000000..71e04fcd44f --- /dev/null +++ b/tools/binman/etype/u_boot_tpl_dtb_with_ucode.py @@ -0,0 +1,25 @@ +# SPDX-License-Identifier: GPL-2.0+ +# Copyright (c) 2016 Google, Inc +# Written by Simon Glass sjg@chromium.org +# +# Entry-type module for U-Boot device tree with the microcode removed +# + +import control +from entry import Entry +from u_boot_dtb_with_ucode import Entry_u_boot_dtb_with_ucode +import tools + +class Entry_u_boot_tpl_dtb_with_ucode(Entry_u_boot_dtb_with_ucode): + """U-Boot TPL with embedded microcode pointer + + This is used when TPL must set up the microcode for U-Boot. + + See Entry_u_boot_ucode for full details of the entries involved in this + process. + """ + def __init__(self, section, etype, node): + Entry_u_boot_dtb_with_ucode.__init__(self, section, etype, node) + + def GetDefaultFilename(self): + return 'tpl/u-boot-tpl.dtb' diff --git a/tools/binman/etype/u_boot_tpl_with_ucode_ptr.py b/tools/binman/etype/u_boot_tpl_with_ucode_ptr.py new file mode 100644 index 00000000000..8d94dded694 --- /dev/null +++ b/tools/binman/etype/u_boot_tpl_with_ucode_ptr.py @@ -0,0 +1,27 @@ +# SPDX-License-Identifier: GPL-2.0+ +# Copyright (c) 2016 Google, Inc +# Written by Simon Glass sjg@chromium.org +# +# Entry-type module for an TPL binary with an embedded microcode pointer +# + +import struct + +import command +from entry import Entry +from blob import Entry_blob +from u_boot_with_ucode_ptr import Entry_u_boot_with_ucode_ptr +import tools + +class Entry_u_boot_tpl_with_ucode_ptr(Entry_u_boot_with_ucode_ptr): + """U-Boot TPL with embedded microcode pointer + + See Entry_u_boot_ucode for full details of the entries involved in this + process. + """ + def __init__(self, section, etype, node): + Entry_u_boot_with_ucode_ptr.__init__(self, section, etype, node) + self.elf_fname = 'tpl/u-boot-tpl' + + def GetDefaultFilename(self): + return 'tpl/u-boot-tpl-nodtb.bin' diff --git a/tools/binman/etype/u_boot_ucode.py b/tools/binman/etype/u_boot_ucode.py index 6acf94d8cbc..a00e530295b 100644 --- a/tools/binman/etype/u_boot_ucode.py +++ b/tools/binman/etype/u_boot_ucode.py @@ -62,19 +62,24 @@ class Entry_u_boot_ucode(Entry_blob):

def ObtainContents(self): # If the section does not need microcode, there is nothing to do - ucode_dest_entry = self.section.FindEntryType('u-boot-with-ucode-ptr') - ucode_dest_entry_spl = self.section.FindEntryType( - 'u-boot-spl-with-ucode-ptr') - if ((not ucode_dest_entry or not ucode_dest_entry.target_offset) and - (not ucode_dest_entry_spl or not ucode_dest_entry_spl.target_offset)): + found = False + for suffix in ['', '-spl', '-tpl']: + name = 'u-boot%s-with-ucode-ptr' % suffix + entry = self.section.FindEntryType(name) + if entry and entry.target_offset: + found = True + if not found: self.data = '' return True - # Get the microcode from the device tree entry. If it is not available # yet, return False so we will be called later. If the section simply # doesn't exist, then we may as well return True, since we are going to # get an error anyway. - fdt_entry = self.section.FindEntryType('u-boot-dtb-with-ucode') + for suffix in ['', '-spl', '-tpl']: + name = 'u-boot%s-dtb-with-ucode' % suffix + fdt_entry = self.section.FindEntryType(name) + if fdt_entry: + break if not fdt_entry: return True if not fdt_entry.ready: @@ -86,12 +91,9 @@ class Entry_u_boot_ucode(Entry_blob): return True

# Write it out to a file - dtb_name = 'u-boot-ucode.bin' - fname = tools.GetOutputFilename(dtb_name) - with open(fname, 'wb') as fd: - fd.write(fdt_entry.ucode_data) + self._pathname = tools.GetOutputFilename('u-boot-ucode.bin') + tools.WriteFile(self._pathname, fdt_entry.ucode_data)

- self._pathname = fname self.ReadBlobContents()

return True diff --git a/tools/binman/etype/u_boot_with_ucode_ptr.py b/tools/binman/etype/u_boot_with_ucode_ptr.py index c850b59a1e1..01f3513e7d3 100644 --- a/tools/binman/etype/u_boot_with_ucode_ptr.py +++ b/tools/binman/etype/u_boot_with_ucode_ptr.py @@ -74,13 +74,16 @@ class Entry_u_boot_with_ucode_ptr(Entry_blob):

# Get the microcode, either from u-boot-ucode or u-boot-dtb-with-ucode. # If we have left the microcode in the device tree, then it will be - # in the former. If we extracted the microcode from the device tree - # and collated it in one place, it will be in the latter. + # in the latter. If we extracted the microcode from the device tree + # and collated it in one place, it will be in the former. if ucode_entry.size: offset, size = ucode_entry.offset, ucode_entry.size else: dtb_entry = self.section.FindEntryType('u-boot-dtb-with-ucode') - if not dtb_entry or not dtb_entry.ready: + if not dtb_entry: + dtb_entry = self.section.FindEntryType( + 'u-boot-tpl-dtb-with-ucode') + if not dtb_entry: self.Raise('Cannot find microcode region u-boot-dtb-with-ucode') offset = dtb_entry.offset + dtb_entry.ucode_offset size = dtb_entry.ucode_size diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index 6c86f4a742c..2a9eea2a499 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -45,6 +45,7 @@ X86_START16_SPL_DATA = 'start16spl' X86_START16_TPL_DATA = 'start16tpl' U_BOOT_NODTB_DATA = 'nodtb with microcode pointer somewhere in here' U_BOOT_SPL_NODTB_DATA = 'splnodtb with microcode pointer somewhere in here' +U_BOOT_TPL_NODTB_DATA = 'tplnodtb with microcode pointer somewhere in here' FSP_DATA = 'fsp' CMC_DATA = 'cmc' VBT_DATA = 'vbt' @@ -104,6 +105,8 @@ class TestFunctional(unittest.TestCase): TestFunctional._MakeInputFile('u-boot-nodtb.bin', U_BOOT_NODTB_DATA) TestFunctional._MakeInputFile('spl/u-boot-spl-nodtb.bin', U_BOOT_SPL_NODTB_DATA) + TestFunctional._MakeInputFile('tpl/u-boot-tpl-nodtb.bin', + U_BOOT_TPL_NODTB_DATA) TestFunctional._MakeInputFile('fsp.bin', FSP_DATA) TestFunctional._MakeInputFile('cmc.bin', CMC_DATA) TestFunctional._MakeInputFile('vbt.bin', VBT_DATA) @@ -1645,6 +1648,22 @@ class TestFunctional(unittest.TestCase): m.update(16 * 'a') self.assertEqual(m.digest(), ''.join(hash_node.value))

+ def testPackUBootTplMicrocode(self): + """Test that x86 microcode can be handled correctly in TPL + + We expect to see the following in the image, in order: + u-boot-tpl-nodtb.bin with a microcode pointer inserted at the correct + place + u-boot-tpl.dtb with the microcode removed + the microcode + """ + with open(self.TestFile('u_boot_ucode_ptr')) as fd: + TestFunctional._MakeInputFile('tpl/u-boot-tpl', fd.read()) + first, pos_and_size = self._RunMicrocodeTest('93_x86_tpl_ucode.dts', + U_BOOT_TPL_NODTB_DATA) + self.assertEqual('tplnodtb with microc' + pos_and_size + + 'ter somewhere in here', first) +

if __name__ == "__main__": unittest.main() diff --git a/tools/binman/test/93_x86_tpl_ucode.dts b/tools/binman/test/93_x86_tpl_ucode.dts new file mode 100644 index 00000000000..d7ed9fc66b8 --- /dev/null +++ b/tools/binman/test/93_x86_tpl_ucode.dts @@ -0,0 +1,29 @@ +/dts-v1/; + +/ { + #address-cells = <1>; + #size-cells = <1>; + + binman { + sort-by-offset; + end-at-4gb; + size = <0x200>; + u-boot-tpl-with-ucode-ptr { + }; + + u-boot-tpl-dtb-with-ucode { + }; + + u-boot-ucode { + }; + }; + + microcode { + update@0 { + data = <0x12345678 0x12345679>; + }; + update@1 { + data = <0xabcd0000 0x78235609>; + }; + }; +};

At present sections have no record of their parent so it is not possible to traverse up the tree to the root and figure out the position of a section within the image.

Change the constructor to record this information.

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

tools/binman/bsection.py | 5 ++++- tools/binman/etype/section.py | 3 ++- tools/binman/image.py | 5 +++-- 3 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/tools/binman/bsection.py b/tools/binman/bsection.py index 650e9ba353f..e4c1900b17f 100644 --- a/tools/binman/bsection.py +++ b/tools/binman/bsection.py @@ -24,6 +24,7 @@ class Section(object):

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 @@ -46,14 +47,16 @@ class Section(object): section _entries: OrderedDict() of entries """ - def __init__(self, name, node, test=False): + 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 = 0 self._size = None self._align_size = None diff --git a/tools/binman/etype/section.py b/tools/binman/etype/section.py index 005a9f9cb2e..7f1b4136049 100644 --- a/tools/binman/etype/section.py +++ b/tools/binman/etype/section.py @@ -32,7 +32,8 @@ class Entry_section(Entry): """ def __init__(self, section, etype, node): Entry.__init__(self, section, etype, node) - self._section = bsection.Section(node.name, node) + self._section = bsection.Section(node.name, section, node, + section._image)

def GetFdtSet(self): return self._section.GetFdtSet() diff --git a/tools/binman/image.py b/tools/binman/image.py index 4b922b51c42..e113a60ac9a 100644 --- a/tools/binman/image.py +++ b/tools/binman/image.py @@ -42,7 +42,8 @@ class Image: self._size = None self._filename = '%s.bin' % self._name if test: - self._section = bsection.Section('main-section', self._node, True) + self._section = bsection.Section('main-section', None, self._node, + self, True) else: self._ReadNode()

@@ -52,7 +53,7 @@ class Image: filename = fdt_util.GetString(self._node, 'filename') if filename: self._filename = filename - self._section = bsection.Section('main-section', self._node) + self._section = bsection.Section('main-section', None, self._node, self)

def GetFdtSet(self): """Get the set of device tree files used by this image"""

Normally x86 platforms use the end-at-4gb option. This currently produces an FMAP with positions which have a large offset. The use of end-at-4gb is a useful convenience within binman, but we don't really want to export a map with these offsets.

Fix this by subtracting the 'skip at start' parameter.

Also put the code which convers names to fmap format, for clarity.

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

tools/binman/bsection.py | 18 +++++++-- tools/binman/entry.py | 3 +- tools/binman/etype/fmap.py | 11 +++-- tools/binman/etype/u_boot_with_ucode_ptr.py | 12 +++--- tools/binman/fmap_util.py | 6 ++- tools/binman/ftest.py | 45 +++++++++++++++++++++ tools/binman/test/94_fmap_x86.dts | 20 +++++++++ tools/binman/test/95_fmap_x86_section.dts | 22 ++++++++++ 8 files changed, 121 insertions(+), 16 deletions(-) create mode 100644 tools/binman/test/94_fmap_x86.dts create mode 100644 tools/binman/test/95_fmap_x86_section.dts

diff --git a/tools/binman/bsection.py b/tools/binman/bsection.py index e4c1900b17f..c208029c25b 100644 --- a/tools/binman/bsection.py +++ b/tools/binman/bsection.py @@ -68,6 +68,7 @@ class Section(object): self._end_4gb = False self._name_prefix = '' self._entries = OrderedDict() + self._image_pos = None if not test: self._ReadNode() self._ReadEntries() @@ -124,7 +125,10 @@ class Section(object): def SetCalculatedProperties(self): state.SetInt(self._node, 'offset', self._offset) state.SetInt(self._node, 'size', self._size) - state.SetInt(self._node, 'image-pos', self._image_pos) + 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()

@@ -437,11 +441,17 @@ class Section(object): source_entry.Raise("Cannot find node for phandle %d" % phandle) for entry in self._entries.values(): if entry._node == node: - if entry.data is None: - return None - return entry.data + return entry.GetData() source_entry.Raise("Cannot find entry for node '%s'" % node.name)

def ExpandSize(self, size): if size != self._size: self._size = size + + def GetRootSkipAtStart(self): + if self._parent_section: + return self._parent_section.GetRootSkipAtStart() + return self._skip_at_start + + def GetImageSize(self): + return self._image._size diff --git a/tools/binman/entry.py b/tools/binman/entry.py index fd7223477eb..01be291b709 100644 --- a/tools/binman/entry.py +++ b/tools/binman/entry.py @@ -197,7 +197,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) + state.SetInt(self._node, 'image-pos', + self.image_pos - self.section.GetRootSkipAtStart()) state.CheckSetHashValue(self._node, self.GetData)

def ProcessFdt(self, fdt): diff --git a/tools/binman/etype/fmap.py b/tools/binman/etype/fmap.py index f1dd81ec493..bf35a5bbf4e 100644 --- a/tools/binman/etype/fmap.py +++ b/tools/binman/etype/fmap.py @@ -42,14 +42,17 @@ class Entry_fmap(Entry): for subentry in entries.values(): _AddEntries(areas, subentry) else: - areas.append(fmap_util.FmapArea(entry.image_pos or 0, - entry.size or 0, entry.name, 0)) + pos = entry.image_pos + if pos is not None: + pos -= entry.section.GetRootSkipAtStart() + areas.append(fmap_util.FmapArea(pos or 0, entry.size or 0, + entry.name, 0))

- entries = self.section.GetEntries() + entries = self.section._image.GetEntries() areas = [] for entry in entries.values(): _AddEntries(areas, entry) - return fmap_util.EncodeFmap(self.section.GetSize() or 0, self.name, + return fmap_util.EncodeFmap(self.section.GetImageSize() or 0, self.name, areas)

def ObtainContents(self): diff --git a/tools/binman/etype/u_boot_with_ucode_ptr.py b/tools/binman/etype/u_boot_with_ucode_ptr.py index 01f3513e7d3..da0e12417b5 100644 --- a/tools/binman/etype/u_boot_with_ucode_ptr.py +++ b/tools/binman/etype/u_boot_with_ucode_ptr.py @@ -66,11 +66,11 @@ class Entry_u_boot_with_ucode_ptr(Entry_blob): # the U-Boot region must start at offset 7MB in the section. In this # case the ROM starts at 0xff800000, so the offset of the first # entry in the section corresponds to that. - if (self.target_offset < self.offset or - self.target_offset >= self.offset + self.size): - self.Raise('Microcode pointer _dt_ucode_base_size at %08x is ' - 'outside the section ranging from %08x to %08x' % - (self.target_offset, self.offset, self.offset + self.size)) + if (self.target_offset < self.image_pos or + self.target_offset >= self.image_pos + self.size): + self.Raise('Microcode pointer _dt_ucode_base_size at %08x is outside the section ranging from %08x to %08x' % + (self.target_offset, self.image_pos, + self.image_pos + self.size))

# Get the microcode, either from u-boot-ucode or u-boot-dtb-with-ucode. # If we have left the microcode in the device tree, then it will be @@ -90,7 +90,7 @@ 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.offset + self.target_offset -= self.image_pos self.ProcessContentsUpdate(self.data[:self.target_offset] + offset_and_size + self.data[self.target_offset + 8:]) diff --git a/tools/binman/fmap_util.py b/tools/binman/fmap_util.py index 7d520e33919..be3cbee87bd 100644 --- a/tools/binman/fmap_util.py +++ b/tools/binman/fmap_util.py @@ -49,6 +49,9 @@ FmapHeader = collections.namedtuple('FmapHeader', FMAP_HEADER_NAMES) FmapArea = collections.namedtuple('FmapArea', FMAP_AREA_NAMES)

+def NameToFmap(name): + return name.replace('\0', '').replace('-', '_').upper() + def ConvertName(field_names, fields): """Convert a name to something flashrom likes

@@ -62,7 +65,7 @@ def ConvertName(field_names, fields): value: value of that field (string for the ones we support) """ name_index = field_names.index('name') - fields[name_index] = fields[name_index].replace('\0', '').replace('-', '_').upper() + fields[name_index] = NameToFmap(fields[name_index])

def DecodeFmap(data): """Decode a flashmap into a header and list of areas @@ -100,6 +103,7 @@ def EncodeFmap(image_size, name, areas): """ def _FormatBlob(fmt, names, obj): params = [getattr(obj, name) for name in names] + ConvertName(names, params) return struct.pack(fmt, *params)

values = FmapHeader(FMAP_SIGNATURE, 1, 0, 0, image_size, name, len(areas)) diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index 2a9eea2a499..ff934d02222 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -1664,6 +1664,51 @@ class TestFunctional(unittest.TestCase): self.assertEqual('tplnodtb with microc' + pos_and_size + 'ter somewhere in here', first)

+ def testFmapX86(self): + """Basic test of generation of a flashrom fmap""" + data = self._DoReadFile('94_fmap_x86.dts') + fhdr, fentries = fmap_util.DecodeFmap(data[32:]) + expected = U_BOOT_DATA + MRC_DATA + 'a' * (32 - 7) + self.assertEqual(expected, data[:32]) + fhdr, fentries = fmap_util.DecodeFmap(data[32:]) + + self.assertEqual(0x100, fhdr.image_size) + + self.assertEqual(0, fentries[0].offset) + self.assertEqual(4, fentries[0].size) + self.assertEqual('U_BOOT', fentries[0].name) + + self.assertEqual(4, fentries[1].offset) + self.assertEqual(3, fentries[1].size) + self.assertEqual('INTEL_MRC', fentries[1].name) + + self.assertEqual(32, fentries[2].offset) + self.assertEqual(fmap_util.FMAP_HEADER_LEN + + fmap_util.FMAP_AREA_LEN * 3, fentries[2].size) + self.assertEqual('FMAP', fentries[2].name) + + def testFmapX86Section(self): + """Basic test of generation of a flashrom fmap""" + data = self._DoReadFile('95_fmap_x86_section.dts') + expected = U_BOOT_DATA + MRC_DATA + 'b' * (32 - 7) + self.assertEqual(expected, data[:32]) + fhdr, fentries = fmap_util.DecodeFmap(data[36:]) + + self.assertEqual(0x100, fhdr.image_size) + + self.assertEqual(0, fentries[0].offset) + self.assertEqual(4, fentries[0].size) + self.assertEqual('U_BOOT', fentries[0].name) + + self.assertEqual(4, fentries[1].offset) + self.assertEqual(3, fentries[1].size) + self.assertEqual('INTEL_MRC', fentries[1].name) + + self.assertEqual(36, fentries[2].offset) + self.assertEqual(fmap_util.FMAP_HEADER_LEN + + fmap_util.FMAP_AREA_LEN * 3, fentries[2].size) + self.assertEqual('FMAP', fentries[2].name) +

if __name__ == "__main__": unittest.main() diff --git a/tools/binman/test/94_fmap_x86.dts b/tools/binman/test/94_fmap_x86.dts new file mode 100644 index 00000000000..613c5dab425 --- /dev/null +++ b/tools/binman/test/94_fmap_x86.dts @@ -0,0 +1,20 @@ +// SPDX-License-Identifier: GPL-2.0+ +/dts-v1/; + +/ { + #address-cells = <1>; + #size-cells = <1>; + + binman { + end-at-4gb; + size = <0x100>; + pad-byte = <0x61>; + u-boot { + }; + intel-mrc { + }; + fmap { + offset = <0xffffff20>; + }; + }; +}; diff --git a/tools/binman/test/95_fmap_x86_section.dts b/tools/binman/test/95_fmap_x86_section.dts new file mode 100644 index 00000000000..4cfce456705 --- /dev/null +++ b/tools/binman/test/95_fmap_x86_section.dts @@ -0,0 +1,22 @@ +// SPDX-License-Identifier: GPL-2.0+ +/dts-v1/; + +/ { + #address-cells = <1>; + #size-cells = <1>; + + binman { + end-at-4gb; + size = <0x100>; + u-boot { + }; + section { + pad-byte = <0x62>; + intel-mrc { + }; + fmap { + offset = <0x20>; + }; + }; + }; +};

For sandbox we want to put ELF files in the image since that is what we need to execute. Add support for this.

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

tools/binman/README.entries | 22 ++++++++++++++++ tools/binman/etype/u_boot_elf.py | 39 ++++++++++++++++++++++++++++ tools/binman/etype/u_boot_spl_elf.py | 24 +++++++++++++++++ tools/binman/ftest.py | 16 ++++++++++++ tools/binman/test/96_elf.dts | 14 ++++++++++ tools/binman/test/97_elf_strip.dts | 15 +++++++++++ 6 files changed, 130 insertions(+) create mode 100644 tools/binman/etype/u_boot_elf.py create mode 100644 tools/binman/etype/u_boot_spl_elf.py create mode 100644 tools/binman/test/96_elf.dts create mode 100644 tools/binman/test/97_elf_strip.dts

diff --git a/tools/binman/README.entries b/tools/binman/README.entries index 4dd67d64fa7..69b435f96eb 100644 --- a/tools/binman/README.entries +++ b/tools/binman/README.entries @@ -361,6 +361,17 @@ it available to u_boot_ucode.

+Entry: u-boot-elf: U-Boot ELF image +----------------------------------- + +Properties / Entry arguments: + - filename: Filename of u-boot (default 'u-boot') + +This is the U-Boot ELF image. It does not include a device tree but can be +relocated to any address for execution. + + + Entry: u-boot-img: U-Boot legacy image --------------------------------------

@@ -444,6 +455,17 @@ to activate.

+Entry: u-boot-spl-elf: U-Boot SPL ELF image +------------------------------------------- + +Properties / Entry arguments: + - filename: Filename of SPL u-boot (default 'spl/u-boot') + +This is the U-Boot SPL ELF image. It does not include a device tree but can +be relocated to any address for execution. + + + Entry: u-boot-spl-nodtb: SPL binary without device tree appended ----------------------------------------------------------------

diff --git a/tools/binman/etype/u_boot_elf.py b/tools/binman/etype/u_boot_elf.py new file mode 100644 index 00000000000..134b6cc15b4 --- /dev/null +++ b/tools/binman/etype/u_boot_elf.py @@ -0,0 +1,39 @@ +# SPDX-License-Identifier: GPL-2.0+ +# Copyright (c) 2018 Google, Inc +# Written by Simon Glass sjg@chromium.org +# +# Entry-type module for U-Boot ELF image +# + +from entry import Entry +from blob import Entry_blob + +import fdt_util +import tools + +class Entry_u_boot_elf(Entry_blob): + """U-Boot ELF image + + Properties / Entry arguments: + - filename: Filename of u-boot (default 'u-boot') + + This is the U-Boot ELF image. It does not include a device tree but can be + relocated to any address for execution. + """ + def __init__(self, section, etype, node): + Entry_blob.__init__(self, section, etype, node) + self._strip = fdt_util.GetBool(self._node, 'strip') + + def ReadBlobContents(self): + if self._strip: + uniq = self.GetUniqueName() + out_fname = tools.GetOutputFilename('%s.stripped' % uniq) + tools.WriteFile(out_fname, tools.ReadFile(self._pathname)) + tools.Run('strip', out_fname) + self.SetContents(tools.ReadFile(out_fname)) + else: + self.SetContents(tools.ReadFile(self._pathname)) + return True + + def GetDefaultFilename(self): + return 'u-boot' diff --git a/tools/binman/etype/u_boot_spl_elf.py b/tools/binman/etype/u_boot_spl_elf.py new file mode 100644 index 00000000000..da328ae15e1 --- /dev/null +++ b/tools/binman/etype/u_boot_spl_elf.py @@ -0,0 +1,24 @@ +# SPDX-License-Identifier: GPL-2.0+ +# Copyright (c) 2018 Google, Inc +# Written by Simon Glass sjg@chromium.org +# +# Entry-type module for U-Boot SPL ELF image +# + +from entry import Entry +from blob import Entry_blob + +class Entry_u_boot_spl_elf(Entry_blob): + """U-Boot SPL ELF image + + Properties / Entry arguments: + - filename: Filename of SPL u-boot (default 'spl/u-boot') + + This is the U-Boot SPL ELF image. It does not include a device tree but can + be relocated to any address for execution. + """ + def __init__(self, section, etype, node): + Entry_blob.__init__(self, section, etype, node) + + def GetDefaultFilename(self): + return 'spl/u-boot-spl' diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index ff934d02222..0fe16d90661 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -1709,6 +1709,22 @@ class TestFunctional(unittest.TestCase): fmap_util.FMAP_AREA_LEN * 3, fentries[2].size) self.assertEqual('FMAP', fentries[2].name)

+ def testElf(self): + """Basic test of ELF entries""" + with open(self.TestFile('bss_data')) as fd: + TestFunctional._MakeInputFile('spl/u-boot-spl', fd.read()) + with open(self.TestFile('bss_data')) as fd: + TestFunctional._MakeInputFile('-boot', fd.read()) + data = self._DoReadFile('96_elf.dts') + + def testElfStripg(self): + """Basic test of ELF entries""" + with open(self.TestFile('bss_data')) as fd: + TestFunctional._MakeInputFile('spl/u-boot-spl', fd.read()) + with open(self.TestFile('bss_data')) as fd: + TestFunctional._MakeInputFile('-boot', fd.read()) + data = self._DoReadFile('97_elf_strip.dts') +

if __name__ == "__main__": unittest.main() diff --git a/tools/binman/test/96_elf.dts b/tools/binman/test/96_elf.dts new file mode 100644 index 00000000000..df3440c3194 --- /dev/null +++ b/tools/binman/test/96_elf.dts @@ -0,0 +1,14 @@ +// SPDX-License-Identifier: GPL-2.0+ +/dts-v1/; + +/ { + #address-cells = <1>; + #size-cells = <1>; + + binman { + u-boot-elf { + }; + u-boot-spl-elf { + }; + }; +}; diff --git a/tools/binman/test/97_elf_strip.dts b/tools/binman/test/97_elf_strip.dts new file mode 100644 index 00000000000..6f3c66fd705 --- /dev/null +++ b/tools/binman/test/97_elf_strip.dts @@ -0,0 +1,15 @@ +// SPDX-License-Identifier: GPL-2.0+ +/dts-v1/; + +/ { + #address-cells = <1>; + #size-cells = <1>; + + binman { + u-boot-elf { + strip; + }; + u-boot-spl-elf { + }; + }; +};

When we get a problem like overlapping regions it is sometimes hard to figure what what is going on. At present we don't write the map file in this case. However the file does provide useful information.

Catch any packing errors and write a map file (if enabled with -m) to aid debugging.

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

tools/binman/control.py | 12 +++++++++--- tools/binman/entry.py | 11 +++++++++-- tools/binman/ftest.py | 24 ++++++++++++++++++++++-- tools/binman/image.py | 7 ++++++- 4 files changed, 46 insertions(+), 8 deletions(-)

diff --git a/tools/binman/control.py b/tools/binman/control.py index caa194c8990..3446e2e79c5 100644 --- a/tools/binman/control.py +++ b/tools/binman/control.py @@ -163,9 +163,15 @@ def Binman(options, args): # completed and written, but that does not seem important. image.GetEntryContents() image.GetEntryOffsets() - image.PackEntries() - image.CheckSize() - image.CheckEntries() + try: + image.PackEntries() + image.CheckSize() + image.CheckEntries() + except Exception as e: + if options.map: + fname = image.WriteMap() + print "Wrote map file '%s' to show errors" % fname + raise image.SetImagePos() if options.update_fdt: image.SetCalculatedProperties() diff --git a/tools/binman/entry.py b/tools/binman/entry.py index 01be291b709..648cfd241f1 100644 --- a/tools/binman/entry.py +++ b/tools/binman/entry.py @@ -390,10 +390,17 @@ class Entry(object): """ pass

+ @staticmethod + def GetStr(value): + if value is None: + return '<none> ' + return '%08x' % value + @staticmethod def WriteMapLine(fd, indent, name, offset, size, image_pos): - print('%08x %s%08x %08x %s' % (image_pos, ' ' * indent, offset, - size, name), file=fd) + print('%s %s%s %s %s' % (Entry.GetStr(image_pos), ' ' * indent, + Entry.GetStr(offset), Entry.GetStr(size), + name), file=fd)

def WriteMap(self, fd, indent): """Write a map of the entry to a .map file diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index 0fe16d90661..a32e423baf5 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -1608,8 +1608,9 @@ class TestFunctional(unittest.TestCase):

def testExpandSizeBad(self): """Test an expanding entry which fails to provide contents""" - with self.assertRaises(ValueError) as e: - self._DoReadFileDtb('89_expand_size_bad.dts', map=True) + with test_util.capture_sys_output() as (stdout, stderr): + with self.assertRaises(ValueError) as e: + self._DoReadFileDtb('89_expand_size_bad.dts', map=True) self.assertIn("Node '/binman/_testing': Cannot obtain contents when " 'expanding entry', str(e.exception))

@@ -1725,6 +1726,25 @@ class TestFunctional(unittest.TestCase): TestFunctional._MakeInputFile('-boot', fd.read()) data = self._DoReadFile('97_elf_strip.dts')

+ def testPackOverlapMap(self): + """Test that overlapping regions are detected""" + with test_util.capture_sys_output() as (stdout, stderr): + with self.assertRaises(ValueError) as e: + self._DoTestFile('14_pack_overlap.dts', map=True) + map_fname = tools.GetOutputFilename('image.map') + self.assertEqual("Wrote map file '%s' to show errors\n" % map_fname, + stdout.getvalue()) + + # We should not get an inmage, but there should be a map file + self.assertFalse(os.path.exists(tools.GetOutputFilename('image.bin'))) + self.assertTrue(os.path.exists(map_fname)) + map_data = tools.ReadFile(map_fname) + self.assertEqual('''ImagePos Offset Size Name +<none> 00000000 00000007 main-section +<none> 00000000 00000004 u-boot +<none> 00000003 00000004 u-boot-align +''', map_data) +

if __name__ == "__main__": unittest.main() diff --git a/tools/binman/image.py b/tools/binman/image.py index e113a60ac9a..f237ae302df 100644 --- a/tools/binman/image.py +++ b/tools/binman/image.py @@ -139,10 +139,15 @@ class Image: return self._section.GetEntries()

def WriteMap(self): - """Write a map of the image to a .map file""" + """Write a map of the image to a .map file + + Returns: + Filename of map file written + """ filename = '%s.map' % self._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) + return fname