From f069303852bdfa1303cd5787667bd7924f0482d9 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Fri, 14 Sep 2018 04:57:07 -0600 Subject: binman: Move 'special properties' docs to README.entries This information should be in the entry it relates to, not in the main README. Move it. Signed-off-by: Simon Glass --- 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(-) (limited to 'tools') diff --git a/tools/binman/README b/tools/binman/README index 9d9d1832ee..10dfe5766d 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 c6e7b22609..041e77771e 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 51e7ba48f5..c850b59a1e 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 -- cgit v1.2.1 From d178eab8f92cb2d67288e01d1ae5b0eb8d0f8db1 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Fri, 14 Sep 2018 04:57:08 -0600 Subject: binman: Allow 'fill' entry to have a size of 0 The check for this should be for None, not 0. Fix it and add a test. Signed-off-by: Simon Glass --- 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 (limited to 'tools') diff --git a/tools/binman/etype/fill.py b/tools/binman/etype/fill.py index 7210a8324a..dcfe978a5b 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 a8456c2615..7f82264f8a 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 0000000000..2b78d3ae88 --- /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]; + }; + }; +}; -- cgit v1.2.1 From 0b489364f90d5cd70b594268442b14e0143c89b5 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Fri, 14 Sep 2018 04:57:09 -0600 Subject: binman: Generate an error when text is not provided 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 --- tools/binman/etype/text.py | 3 +++ tools/binman/ftest.py | 7 +++++++ 2 files changed, 10 insertions(+) (limited to 'tools') diff --git a/tools/binman/etype/text.py b/tools/binman/etype/text.py index 7a1cddf4af..6e99819487 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 7f82264f8a..d956bd42e1 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() -- cgit v1.2.1 From 35b384cbe5d40e618391cc076409e89cedf9c863 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Fri, 14 Sep 2018 04:57:10 -0600 Subject: binman: Add x86 support for starting TPL 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 --- 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 (limited to 'tools') diff --git a/tools/binman/README.entries b/tools/binman/README.entries index 041e77771e..31bc725d57 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 0000000000..46ce169ae0 --- /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 +# +# 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 d956bd42e1..3f4f5f3a43 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 0000000000..68e6bbd68f --- /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 { + }; + }; +}; -- cgit v1.2.1 From a326b495cdcfd56507841e38158683e6e4d5894c Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Fri, 14 Sep 2018 04:57:11 -0600 Subject: binman: Tidy up the vblock entry 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 --- 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(-) (limited to 'tools') diff --git a/tools/binman/README.entries b/tools/binman/README.entries index 31bc725d57..5cb52a92ff 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. - input file passed to futility + - vblock. - 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 77cfab9c5d..e671a2ea09 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 6fa735ed59..4100bcc3d3 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 595af5456d..c4d970ed16 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. - input file passed to futility + - vblock. - 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 3f4f5f3a43..c4065551e7 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() -- cgit v1.2.1 From 0bfa7b09ba16f4ffaf0cfc9315336aaa708dcd26 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Fri, 14 Sep 2018 04:57:12 -0600 Subject: binman: Support building a selection of images 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 --- tools/binman/cmdline.py | 2 ++ tools/binman/control.py | 9 +++++++++ tools/binman/ftest.py | 19 ++++++++++++++++++- 3 files changed, 29 insertions(+), 1 deletion(-) (limited to 'tools') diff --git a/tools/binman/cmdline.py b/tools/binman/cmdline.py index f0de4ded44..4ce8bc6ab4 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 2de1c86ecf..8c48008fc7 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 c4065551e7..290e9aebf1 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() -- cgit v1.2.1 From fa80c25c09a6c59be4df9c42b4a02538d8a07382 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Fri, 14 Sep 2018 04:57:13 -0600 Subject: dtoc: Allow syncing of the device tree back to a file 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 --- tools/binman/control.py | 2 ++ tools/dtoc/fdt.py | 91 ++++++++++++++++++++++++++++++++++++++++++++----- tools/dtoc/test_fdt.py | 8 ++++- 3 files changed, 91 insertions(+), 10 deletions(-) (limited to 'tools') diff --git a/tools/binman/control.py b/tools/binman/control.py index 8c48008fc7..ded1b71109 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 55baa3857f..76cd66fbf9 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 e88d19f80e..4a67f8949d 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 -- cgit v1.2.1 From af53f5aafcbbe1ba97264a0262067657f3dc8c34 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Fri, 14 Sep 2018 04:57:14 -0600 Subject: dtoc: Fixed endianness in Prop.GetEmpty() This should be big endian, since that is what device tree uses. Fix it. Signed-off-by: Simon Glass --- tools/dtoc/fdt.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'tools') diff --git a/tools/dtoc/fdt.py b/tools/dtoc/fdt.py index 76cd66fbf9..ccf3b23ced 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); elif type == TYPE_STRING: return '' else: -- cgit v1.2.1 From e21c27af47183619c7ec7097abb1dec34410e775 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Fri, 14 Sep 2018 04:57:15 -0600 Subject: dtoc: Support adding new nodes Add a way to add new nodes and sync them back to the blob. Signed-off-by: Simon Glass --- tools/dtoc/fdt.py | 20 ++++++++++++++++++++ tools/dtoc/test_fdt.py | 8 ++++++++ 2 files changed, 28 insertions(+) (limited to 'tools') diff --git a/tools/dtoc/fdt.py b/tools/dtoc/fdt.py index ccf3b23ced..26f4a4ee56 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 4a67f8949d..c94e455d12 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): -- cgit v1.2.1 From 6434961b2b2099b458e7dc9dcced5d450b45cbb4 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Fri, 14 Sep 2018 04:57:16 -0600 Subject: dtoc: Add methods for adding and updating properties 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 --- tools/dtoc/fdt.py | 70 ++++++++++++++++++++++++++++++++++++++++++++++++++ tools/dtoc/test_fdt.py | 43 +++++++++++++++++++++++++++++++ 2 files changed, 113 insertions(+) (limited to 'tools') diff --git a/tools/dtoc/fdt.py b/tools/dtoc/fdt.py index 26f4a4ee56..c5054e8cc9 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 c94e455d12..22a075d6c5 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 -- cgit v1.2.1 From 746aee3f2f8c0c7534ad7ac7d438ccec35c6c99c Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Fri, 14 Sep 2018 04:57:17 -0600 Subject: dtoc: Add a way to create an Fdt object from a data block Support creating an Fdt object without having to write the data to a file first. Signed-off-by: Simon Glass --- tools/dtoc/fdt.py | 14 ++++++++++++++ tools/dtoc/test_fdt.py | 8 ++++++++ 2 files changed, 22 insertions(+) (limited to 'tools') diff --git a/tools/dtoc/fdt.py b/tools/dtoc/fdt.py index c5054e8cc9..2df2d4b0cc 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 22a075d6c5..d259702050 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 -- cgit v1.2.1 From 6c234bfbf7a9c5b33c3bea92e037c45d37e94f35 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Fri, 14 Sep 2018 04:57:18 -0600 Subject: binman: Add an entry method for getting the default filename 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 --- tools/binman/entry.py | 3 +++ tools/binman/entry_test.py | 5 +++++ 2 files changed, 8 insertions(+) (limited to 'tools') diff --git a/tools/binman/entry.py b/tools/binman/entry.py index e671a2ea09..ec3b22e9b3 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 4100bcc3d3..69d85b4ced 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() -- cgit v1.2.1 From c55a50f558f13c6c018c0e5cc0f0d765711a3828 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Fri, 14 Sep 2018 04:57:19 -0600 Subject: binman: Move state information into a new module 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 --- 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 (limited to 'tools') diff --git a/tools/binman/control.py b/tools/binman/control.py index ded1b71109..00dcb8ad6a 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 ec3b22e9b3..1d6299aefa 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 285a28dd1e..3fab766011 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 290e9aebf1..867179702d 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 0000000000..6bc24ddf8a --- /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 +# +# 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) -- cgit v1.2.1 From 2a72cc72ca29fb14a61dd50a60ffcd096a0be317 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Fri, 14 Sep 2018 04:57:20 -0600 Subject: binman: Move state logic into the state module Rather than reaching into this module from control, move the code that needs this info into state. Signed-off-by: Simon Glass --- tools/binman/control.py | 21 +++++++++++++-------- tools/binman/state.py | 43 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 56 insertions(+), 8 deletions(-) (limited to 'tools') diff --git a/tools/binman/control.py b/tools/binman/control.py index 00dcb8ad6a..fd8b779945 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 6bc24ddf8a..9583b3fa5f 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 + -- cgit v1.2.1 From f46621d255181bd8d1e8092945ffc66147b88531 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Fri, 14 Sep 2018 04:57:21 -0600 Subject: binman: Centralise device-tree updates within binman 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 --- tools/binman/bsection.py | 9 +++++---- tools/binman/entry.py | 8 ++++---- tools/binman/state.py | 35 +++++++++++++++++++++++++++++++++++ 3 files changed, 44 insertions(+), 8 deletions(-) (limited to 'tools') diff --git a/tools/binman/bsection.py b/tools/binman/bsection.py index a0bd1b6d34..4f5c33f048 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 1d6299aefa..4b87156ff8 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 9583b3fa5f..5f25b907b9 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) -- cgit v1.2.1 From 539aece516d87084437a38d879a1b91c661209f8 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Fri, 14 Sep 2018 04:57:22 -0600 Subject: binman: Obtain the list of device trees from the config 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 --- 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(-) (limited to 'tools') diff --git a/tools/binman/bsection.py b/tools/binman/bsection.py index 4f5c33f048..44adb82795 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 fd8b779945..49d49a001c 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 4b87156ff8..e5f557749f 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 68126bc3e6..1fb5eb65db 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 5f25b907b9..600eb86cfe 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 -- cgit v1.2.1 From 93d174135ac44cbbe81c87ea564488309949e6d4 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Fri, 14 Sep 2018 04:57:23 -0600 Subject: binman: Allow control of whether a fake DT is used 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 --- 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(-) (limited to 'tools') diff --git a/tools/binman/cmdline.py b/tools/binman/cmdline.py index 4ce8bc6ab4..f8caa7d284 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 49d49a001c..34ec74ba1b 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 867179702d..75e9a2143c 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 600eb86cfe..b27eb077a6 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() -- cgit v1.2.1 From 6ed45ba0a831393ab6c5d3355384238d2b4f47da Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Fri, 14 Sep 2018 04:57:24 -0600 Subject: binman: Support updating all device tree files 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 --- 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 (limited to 'tools') diff --git a/tools/binman/README.entries b/tools/binman/README.entries index 5cb52a92ff..091fb5ce2b 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 34ec74ba1b..e326456a4b 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 e5f557749f..f922107523 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 0000000000..cc5b4a3f76 --- /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 +# +# 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 f5b2ed67cf..a30cc91545 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 fb3dd1cf64..6263c4ebee 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 3fab766011..73f5fbf903 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 cb29ba3fd8..e7354646f1 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 9c4e668347..bdeb0f75a2 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 75e9a2143c..6bfef7b63a 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 1fb5eb65db..bfb2229779 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 b27eb077a6..09ead442a6 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 0000000000..284975cc28 --- /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 { + }; + }; +}; -- cgit v1.2.1 From 04187a845c86215da0e3ad680cdcf2fc7515b99a Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Fri, 14 Sep 2018 04:57:25 -0600 Subject: patman: Detect missing tools and report them 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 --- tools/patman/tools.py | 29 ++++++++++++++++++++++++++++- 1 file changed, 28 insertions(+), 1 deletion(-) (limited to 'tools') diff --git a/tools/patman/tools.py b/tools/patman/tools.py index e80481438b..0870bccca0 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. -- cgit v1.2.1 From 83d73c2f7c471c1a7e5a9a2bf0de287491408b2d Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Fri, 14 Sep 2018 04:57:26 -0600 Subject: binman: Support compressed entries 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 --- tools/binman/README | 16 +++++++++++++ tools/binman/README.entries | 7 ++++++ tools/binman/etype/blob.py | 49 ++++++++++++++++++++++++++++++++------- tools/binman/ftest.py | 31 +++++++++++++++++++++++++ tools/binman/test/83_compress.dts | 11 +++++++++ 5 files changed, 105 insertions(+), 9 deletions(-) create mode 100644 tools/binman/test/83_compress.dts (limited to 'tools') diff --git a/tools/binman/README b/tools/binman/README index 10dfe5766d..d6871946ab 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 091fb5ce2b..2cf7dc0338 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 3f46eecf30..642a0e482a 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 6bfef7b63a..1c3c46fc23 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -54,6 +54,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 +117,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 +1508,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 0000000000..07813bdeaa --- /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"; + }; + }; +}; -- cgit v1.2.1 From b4e1a38c294f56708d1a82717c850da359401d7e Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Fri, 14 Sep 2018 04:57:27 -0600 Subject: binman: Allow zero-size sections 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 --- tools/binman/bsection.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'tools') diff --git a/tools/binman/bsection.py b/tools/binman/bsection.py index 44adb82795..1c37d84e99 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, -- cgit v1.2.1 From 0a98b28b06800da48f006069fe14e47dd399d2ff Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Fri, 14 Sep 2018 04:57:28 -0600 Subject: binman: Support adding files 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 --- 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 + tools/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 (limited to 'tools') diff --git a/tools/binman/README.entries b/tools/binman/README.entries index 2cf7dc0338..3afc560052 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 1c37d84e99..4bf206878d 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 e326456a4b..caa194c899 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 f922107523..7316ad43b5 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 0000000000..99f2f2f67f --- /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 +# +# 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 1c3c46fc23..e919e702b5 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -54,6 +54,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' @@ -117,6 +119,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 @@ -1536,6 +1541,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 bfb2229779..4b922b51c4 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 09ead442a6..2f8c0863a8 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 0000000000..83ddb78f8e --- /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 0000000000..847b398bf2 --- /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 0000000000..34bd92f224 --- /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 0000000000..0cb5b469cb --- /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 0000000000..a952470617 --- /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 0000000000..687ea52730 --- /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 0000000000..e69de29bb2 diff --git a/tools/binman/test/files/not-this-one b/tools/binman/test/files/not-this-one new file mode 100644 index 0000000000..e71c2250f9 --- /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 0870bccca0..1c9bf4e810 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 sorted(files) + def Align(pos, align): if align: mask = align - 1 -- cgit v1.2.1 From ba64a0bbb7b7128479a97fdf58baa9ddfbfe4db6 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Fri, 14 Sep 2018 04:57:29 -0600 Subject: binman: Support expanding entries It is useful to have entries which can grow automatically to fill available space. Add support for this. Signed-off-by: Simon Glass --- 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 (limited to 'tools') diff --git a/tools/binman/README b/tools/binman/README index d6871946ab..6aa5b38419 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 4bf206878d..52ac31a467 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 7316ad43b5..0915b470b4 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 02c165c0c3..3e345bd952 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 a30cc91545..005a9f9cb2 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 e919e702b5..b156943323 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -1579,6 +1579,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 0000000000..c8a01308ec --- /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 0000000000..edc0e5cf68 --- /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>; + }; + }; +}; -- cgit v1.2.1 From 9c888cca5e87e28e9addcffae9810fee481428a8 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Fri, 14 Sep 2018 04:57:30 -0600 Subject: binman: Mention section attributes in docs 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 --- tools/binman/README | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'tools') diff --git a/tools/binman/README b/tools/binman/README index 6aa5b38419..cf1a06d164 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 -- cgit v1.2.1 From e0e5df9310d3a0e1fc0eda86ff43fd3e782e61f1 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Fri, 14 Sep 2018 04:57:31 -0600 Subject: binman: Support hashing entries 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 --- 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 (limited to 'tools') diff --git a/tools/binman/README b/tools/binman/README index cf1a06d164..088f3a63d9 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 52ac31a467..650e9ba353 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 0915b470b4..fd7223477e 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 b156943323..c46a065382 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -6,6 +6,7 @@ # # python -m unittest func_test.TestFunctional.testHelp +import hashlib from optparse import OptionParser import os import shutil @@ -1608,6 +1609,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 2f8c0863a8..d945e4bf65 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 0000000000..200304599d --- /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 0000000000..b64df20511 --- /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 0000000000..d2402000db --- /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 0000000000..dcd8683d64 --- /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"; + }; + }; + }; +}; -- cgit v1.2.1 From f025363543636191cfc6d277733317cb0198189f Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Fri, 14 Sep 2018 04:57:32 -0600 Subject: binman: Support x86 microcode in TPL 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 --- tools/binman/README.entries | 20 +++++++++++++++++ tools/binman/etype/u_boot_dtb_with_ucode.py | 15 ++++++++----- tools/binman/etype/u_boot_spl_with_ucode_ptr.py | 2 ++ tools/binman/etype/u_boot_tpl_dtb_with_ucode.py | 25 +++++++++++++++++++++ tools/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 (limited to 'tools') diff --git a/tools/binman/README.entries b/tools/binman/README.entries index 3afc560052..4dd67d64fa 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 73f5fbf903..444c51b8b7 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 0dfe268a56..b650cf0146 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 0000000000..71e04fcd44 --- /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 +# +# 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 0000000000..8d94dded69 --- /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 +# +# 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 6acf94d8cb..a00e530295 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 c850b59a1e..01f3513e7d 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 c46a065382..f8faef1893 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -44,6 +44,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' @@ -103,6 +104,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) @@ -1644,6 +1647,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 0000000000..d7ed9fc66b --- /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>; + }; + }; +}; -- cgit v1.2.1 From 08723a7abbc7e28b22d18684faf5142fc6f155e8 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Fri, 14 Sep 2018 04:57:33 -0600 Subject: binman: Record the parent section of each section 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 --- tools/binman/bsection.py | 5 ++++- tools/binman/etype/section.py | 3 ++- tools/binman/image.py | 5 +++-- 3 files changed, 9 insertions(+), 4 deletions(-) (limited to 'tools') diff --git a/tools/binman/bsection.py b/tools/binman/bsection.py index 650e9ba353..e4c1900b17 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 005a9f9cb2..7f1b413604 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 4b922b51c4..e113a60ac9 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""" -- cgit v1.2.1 From f8f8df6eb870b53e025aa447f8d40cd2ce2a77f6 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Fri, 14 Sep 2018 04:57:34 -0600 Subject: binman: Correct fmap output on x86 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 --- 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 (limited to 'tools') diff --git a/tools/binman/bsection.py b/tools/binman/bsection.py index e4c1900b17..c208029c25 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 fd7223477e..01be291b70 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 f1dd81ec49..bf35a5bbf4 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 01f3513e7d..da0e12417b 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 7d520e3391..be3cbee87b 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 f8faef1893..1abb768e64 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -1663,6 +1663,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 0000000000..613c5dab42 --- /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 0000000000..4cfce45670 --- /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>; + }; + }; + }; +}; -- cgit v1.2.1 From fe1ae3ecc3a2203babd7837bd2d5cf514a374c1f Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Fri, 14 Sep 2018 04:57:35 -0600 Subject: binman: Support ELF files for U-Boot and SPL 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 --- 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 (limited to 'tools') diff --git a/tools/binman/README.entries b/tools/binman/README.entries index 4dd67d64fa..69b435f96e 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 0000000000..134b6cc15b --- /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 +# +# 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 0000000000..da328ae15e --- /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 +# +# 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 1abb768e64..27dca3a2a7 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -1708,6 +1708,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 0000000000..df3440c319 --- /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 0000000000..6f3c66fd70 --- /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 { + }; + }; +}; -- cgit v1.2.1 From 163ed6c342cfd15b623a46f3755203c712374a9a Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Fri, 14 Sep 2018 04:57:36 -0600 Subject: binman: Allow writing a map file when something goes wrong 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 --- 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(-) (limited to 'tools') diff --git a/tools/binman/control.py b/tools/binman/control.py index caa194c899..3446e2e79c 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 01be291b70..648cfd241f 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 ' ' + 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 27dca3a2a7..abf02b62e8 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -1607,8 +1607,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)) @@ -1724,6 +1725,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 + 00000000 00000007 main-section + 00000000 00000004 u-boot + 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 e113a60ac9..f237ae302d 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 -- cgit v1.2.1 From e62a24ce27ab86efc1b37d14112c29d3f2010238 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Mon, 17 Sep 2018 23:55:42 -0600 Subject: buildman: Avoid hanging when the config changes Something has changed in the last several month such that when buildman builds U-Boot incrementally and a new CONFIG option has been added to the Kconfig, the build hanges waiting for input: Test new config (NEW_CONFIG) [N/y/?] (NEW) Since binamn does not connect the build's stdin to anything this waits on stdin to the build thread, which never comes. Eventually I suspect all the threads end up in this state and the build does not progress. Fix this by passing /dev/null as input to the build. That way, if there is a new CONFIG, the build will stop (and fail): Test new config (NEW_CONFIG) [N/y/?] (NEW) Error in reading or end of file. Signed-off-by: Simon Glass --- tools/buildman/builder.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'tools') diff --git a/tools/buildman/builder.py b/tools/buildman/builder.py index a5a2ffdfdf..05f8299541 100644 --- a/tools/buildman/builder.py +++ b/tools/buildman/builder.py @@ -408,7 +408,7 @@ class Builder: """ cmd = [self.gnu_make] + list(args) result = command.RunPipe([cmd], capture=True, capture_stderr=True, - cwd=cwd, raise_on_error=False, **kwargs) + cwd=cwd, raise_on_error=False, infile='/dev/null', **kwargs) if self.verbose_build: result.stdout = '%s\n' % (' '.join(cmd)) + result.stdout result.combined = '%s\n' % (' '.join(cmd)) + result.combined -- cgit v1.2.1