All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] [PATCH 00/26] binman: Allow reading of images to list contents
@ 2019-07-02  0:24 Simon Glass
  2019-07-02  0:24 ` [U-Boot] [PATCH 01/26] binman: Simplify the entry test Simon Glass
                   ` (25 more replies)
  0 siblings, 26 replies; 27+ messages in thread
From: Simon Glass @ 2019-07-02  0:24 UTC (permalink / raw)
  To: u-boot

This series adds features to allow binman to read its own images. Using
an 'FDT map' inside the image (basically a cut-down copy of the input
config with some pieces added) it can reverse-engineer the image and
figure out what is where.

So far the only user of this feature is a new 'list' command, which allows
listing the entries in an image.

Other improvements along the way include:
- Support for FDT map
- Support for an image header, which points to the FDT map
- Ability to deal with entries which change size after packing
- Additional error checking and FDT features for CBFS
- Convert from OptionParser to ArgumentParser

Future work will allow reading files from the image, but I've decided to
put that in the next series.


Simon Glass (26):
  binman: Simplify the entry test
  binman: Update future features
  binman: Update help for new features
  binman: Add an FDT map
  binman: Add an image header
  binman: Allow cbfstool to be optional with tests
  binman: Convert to use ArgumentParser
  binman: Move compression into the Entry base class
  binman: Drop an unused arg in Entry.Lookup()
  binman: Allow easy importing of entry modules
  binman: Fix up ProcessUpdateContents error and comments
  binman: Call ProcessUpdateContents() consistently
  binman: Add a return value to ProcessContentsUpdate()
  binman: Add a control for post-pack entry expansion
  binman: Allow entries to expand after packing
  binman: Allow device-tree entries to be compressed
  binman: Provide the actual data address for cbfs files
  binman: Use the cbfs memlen field only for uncompressed length
  binman: Add a comment about CBFS test structure
  binman: Support FDT update for CBFS
  binman: Detect bad CBFS file types
  binman: Allow listing the entries in an image
  binman: Support reading in firmware images
  binman: Support locating an image header
  binman: Support reading an image into an Image object
  binman: Support listing an image

 .travis.yml                                   |   3 +-
 test/run                                      |   5 +-
 tools/binman/README                           |  62 ++-
 tools/binman/README.entries                   |  57 +++
 tools/binman/binman.py                        |  43 +-
 tools/binman/bsection.py                      |  27 +-
 tools/binman/cbfs_util.py                     |  42 +-
 tools/binman/cbfs_util_test.py                |  49 ++-
 tools/binman/cmdline.py                       |  90 ++--
 tools/binman/control.py                       | 159 +++++--
 tools/binman/entry.py                         | 115 ++++-
 tools/binman/entry_test.py                    |  32 +-
 tools/binman/etype/__init__.py                |   0
 tools/binman/etype/_testing.py                |  14 +-
 tools/binman/etype/blob.py                    |  40 +-
 tools/binman/etype/blob_dtb.py                |  10 +-
 tools/binman/etype/cbfs.py                    |  57 ++-
 tools/binman/etype/fdtmap.py                  | 129 ++++++
 tools/binman/etype/fmap.py                    |   2 +-
 tools/binman/etype/image_header.py            |  99 +++++
 tools/binman/etype/section.py                 |  14 +-
 tools/binman/etype/u_boot_with_ucode_ptr.py   |   8 +-
 tools/binman/ftest.py                         | 407 ++++++++++++++++--
 tools/binman/image.py                         |  59 ++-
 tools/binman/state.py                         |  26 +-
 tools/binman/test/115_fdtmap.dts              |  13 +
 tools/binman/test/116_fdtmap_hdr.dts          |  17 +
 tools/binman/test/117_fdtmap_hdr_start.dts    |  19 +
 tools/binman/test/118_fdtmap_hdr_pos.dts      |  19 +
 tools/binman/test/119_fdtmap_hdr_missing.dts  |  16 +
 tools/binman/test/120_hdr_no_location.dts     |  16 +
 tools/binman/test/121_entry_expand.dts        |  20 +
 tools/binman/test/122_entry_expand_twice.dts  |  21 +
 .../binman/test/123_entry_expand_section.dts  |  22 +
 tools/binman/test/124_compress_dtb.dts        |  14 +
 tools/binman/test/125_cbfs_update.dts         |  21 +
 tools/binman/test/126_cbfs_bad_type.dts       |  17 +
 tools/binman/test/127_list.dts                |  33 ++
 tools/binman/test/128_decode_image.dts        |  36 ++
 tools/binman/test/128_decode_image_no_hdr.dts |  33 ++
 tools/binman/test/129_list_fdtmap.dts         |  36 ++
 tools/patman/test_util.py                     |   6 +-
 42 files changed, 1672 insertions(+), 236 deletions(-)
 create mode 100644 tools/binman/etype/__init__.py
 create mode 100644 tools/binman/etype/fdtmap.py
 create mode 100644 tools/binman/etype/image_header.py
 create mode 100644 tools/binman/test/115_fdtmap.dts
 create mode 100644 tools/binman/test/116_fdtmap_hdr.dts
 create mode 100644 tools/binman/test/117_fdtmap_hdr_start.dts
 create mode 100644 tools/binman/test/118_fdtmap_hdr_pos.dts
 create mode 100644 tools/binman/test/119_fdtmap_hdr_missing.dts
 create mode 100644 tools/binman/test/120_hdr_no_location.dts
 create mode 100644 tools/binman/test/121_entry_expand.dts
 create mode 100644 tools/binman/test/122_entry_expand_twice.dts
 create mode 100644 tools/binman/test/123_entry_expand_section.dts
 create mode 100644 tools/binman/test/124_compress_dtb.dts
 create mode 100644 tools/binman/test/125_cbfs_update.dts
 create mode 100644 tools/binman/test/126_cbfs_bad_type.dts
 create mode 100644 tools/binman/test/127_list.dts
 create mode 100644 tools/binman/test/128_decode_image.dts
 create mode 100644 tools/binman/test/128_decode_image_no_hdr.dts
 create mode 100644 tools/binman/test/129_list_fdtmap.dts

-- 
2.22.0.410.gd8fdbe21b5-goog

^ permalink raw reply	[flat|nested] 27+ messages in thread

* [U-Boot] [PATCH 01/26] binman: Simplify the entry test
  2019-07-02  0:24 [U-Boot] [PATCH 00/26] binman: Allow reading of images to list contents Simon Glass
@ 2019-07-02  0:24 ` Simon Glass
  2019-07-02  0:24 ` [U-Boot] [PATCH 02/26] binman: Update future features Simon Glass
                   ` (24 subsequent siblings)
  25 siblings, 0 replies; 27+ messages in thread
From: Simon Glass @ 2019-07-02  0:24 UTC (permalink / raw)
  To: u-boot

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

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

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

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

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

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

diff --git a/tools/binman/entry_test.py b/tools/binman/entry_test.py
index b30a7beecc8..b6ad3edb8dc 100644
--- a/tools/binman/entry_test.py
+++ b/tools/binman/entry_test.py
@@ -9,12 +9,11 @@ import os
 import sys
 import unittest
 
+import entry
 import fdt
 import fdt_util
 import tools
 
-entry = None
-
 class TestEntry(unittest.TestCase):
     def setUp(self):
         tools.PrepareOutputDir(None)
@@ -29,16 +28,7 @@ class TestEntry(unittest.TestCase):
         dtb = fdt.FdtScan(fname)
         return dtb.GetNode('/binman/u-boot')
 
-    def test1EntryNoImportLib(self):
-        """Test that we can import Entry subclassess successfully"""
-
-        sys.modules['importlib'] = None
-        global entry
-        import entry
-        entry.Entry.Create(None, self.GetNode(), 'u-boot')
-
-    def test2EntryImportLib(self):
-        del sys.modules['importlib']
+    def _ReloadEntry(self):
         global entry
         if entry:
             if sys.version_info[0] >= 3:
@@ -48,8 +38,21 @@ class TestEntry(unittest.TestCase):
                 reload(entry)
         else:
             import entry
+
+    def test1EntryNoImportLib(self):
+        """Test that we can import Entry subclassess successfully"""
+        sys.modules['importlib'] = None
+        global entry
+        self._ReloadEntry()
+        entry.Entry.Create(None, self.GetNode(), 'u-boot')
+        self.assertFalse(entry.have_importlib)
+
+    def test2EntryImportLib(self):
+        del sys.modules['importlib']
+        global entry
+        self._ReloadEntry()
         entry.Entry.Create(None, self.GetNode(), 'u-boot-spl')
-        del entry
+        self.assertTrue(entry.have_importlib)
 
     def testEntryContents(self):
         """Test the Entry bass class"""
@@ -59,7 +62,6 @@ class TestEntry(unittest.TestCase):
 
     def testUnknownEntry(self):
         """Test that unknown entry types are detected"""
-        import entry
         Node = collections.namedtuple('Node', ['name', 'path'])
         node = Node('invalid-name', 'invalid-path')
         with self.assertRaises(ValueError) as e:
@@ -69,7 +71,6 @@ class TestEntry(unittest.TestCase):
 
     def testUniqueName(self):
         """Test Entry.GetUniqueName"""
-        import entry
         Node = collections.namedtuple('Node', ['name', 'parent'])
         base_node = Node('root', None)
         base_entry = entry.Entry(None, None, base_node, read_node=False)
@@ -80,7 +81,6 @@ class TestEntry(unittest.TestCase):
 
     def testGetDefaultFilename(self):
         """Trivial test for this base class function"""
-        import entry
         base_entry = entry.Entry(None, None, None, read_node=False)
         self.assertIsNone(base_entry.GetDefaultFilename())
 
-- 
2.22.0.410.gd8fdbe21b5-goog

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [U-Boot] [PATCH 02/26] binman: Update future features
  2019-07-02  0:24 [U-Boot] [PATCH 00/26] binman: Allow reading of images to list contents Simon Glass
  2019-07-02  0:24 ` [U-Boot] [PATCH 01/26] binman: Simplify the entry test Simon Glass
@ 2019-07-02  0:24 ` Simon Glass
  2019-07-02  0:24 ` [U-Boot] [PATCH 03/26] binman: Update help for new features Simon Glass
                   ` (23 subsequent siblings)
  25 siblings, 0 replies; 27+ messages in thread
From: Simon Glass @ 2019-07-02  0:24 UTC (permalink / raw)
  To: u-boot

A few features have been completed and a few items are added.

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

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

diff --git a/tools/binman/README b/tools/binman/README
index 2f4c7fec21e..92341a14b61 100644
--- a/tools/binman/README
+++ b/tools/binman/README
@@ -811,13 +811,13 @@ Some ideas:
 - Use of-platdata to make the information available to code that is unable
   to use device tree (such as a very small SPL image)
 - Allow easy building of images by specifying just the board name
-- Produce a full Python binding for libfdt (for upstream). This is nearing
-    completion but some work remains
 - Add an option to decode an image into the constituent binaries
 - Support building an image for a board (-b) more completely, with a
   configurable build directory
-- Consider making binman work with buildman, although if it is used in the
-  Makefile, this will be automatic
+- Support putting the FDT in an image with a suitable magic number
+- Support adding a pointer to the FDT map
+- Support listing files in images
+- Support logging of binman's operations, with different levels of verbosity
 
 --
 Simon Glass <sjg@chromium.org>
-- 
2.22.0.410.gd8fdbe21b5-goog

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [U-Boot] [PATCH 03/26] binman: Update help for new features
  2019-07-02  0:24 [U-Boot] [PATCH 00/26] binman: Allow reading of images to list contents Simon Glass
  2019-07-02  0:24 ` [U-Boot] [PATCH 01/26] binman: Simplify the entry test Simon Glass
  2019-07-02  0:24 ` [U-Boot] [PATCH 02/26] binman: Update future features Simon Glass
@ 2019-07-02  0:24 ` Simon Glass
  2019-07-02  0:24 ` [U-Boot] [PATCH 04/26] binman: Add an FDT map Simon Glass
                   ` (22 subsequent siblings)
  25 siblings, 0 replies; 27+ messages in thread
From: Simon Glass @ 2019-07-02  0:24 UTC (permalink / raw)
  To: u-boot

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

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

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

diff --git a/tools/binman/README b/tools/binman/README
index 92341a14b61..f07de350f60 100644
--- a/tools/binman/README
+++ b/tools/binman/README
@@ -36,10 +36,9 @@ suitable padding and alignment. It provides a way to process binaries before
 they are included, by adding a Python plug-in. The device tree is available
 to U-Boot at run-time so that the images can be interpreted.
 
-Binman does not yet update the device tree with the final location of
-everything when it is done. A simple C structure could be generated for
-constrained environments like SPL (using dtoc) but this is also not
-implemented.
+Binman can update the device tree with the final location of everything when it
+is done. Entry positions can be provided to U-Boot SPL as run-time symbols,
+avoiding device-tree code overhead.
 
 Binman can also support incorporating filesystems in the image if required.
 For example x86 platforms may use CBFS in some cases.
-- 
2.22.0.410.gd8fdbe21b5-goog

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [U-Boot] [PATCH 04/26] binman: Add an FDT map
  2019-07-02  0:24 [U-Boot] [PATCH 00/26] binman: Allow reading of images to list contents Simon Glass
                   ` (2 preceding siblings ...)
  2019-07-02  0:24 ` [U-Boot] [PATCH 03/26] binman: Update help for new features Simon Glass
@ 2019-07-02  0:24 ` Simon Glass
  2019-07-02  0:24 ` [U-Boot] [PATCH 05/26] binman: Add an image header Simon Glass
                   ` (21 subsequent siblings)
  25 siblings, 0 replies; 27+ messages in thread
From: Simon Glass @ 2019-07-02  0:24 UTC (permalink / raw)
  To: u-boot

An FDT map is an entry which holds a full description of the image
entries, in FDT format. It can be discovered using the magic string at
its start. Tools can locate and read this entry to find out what entries
are in the image and where each entry is located.

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

 tools/binman/README              |   5 +-
 tools/binman/README.entries      |  38 +++++++++++
 tools/binman/etype/fdtmap.py     | 109 +++++++++++++++++++++++++++++++
 tools/binman/ftest.py            |  54 +++++++++++++--
 tools/binman/state.py            |   2 +-
 tools/binman/test/115_fdtmap.dts |  13 ++++
 6 files changed, 213 insertions(+), 8 deletions(-)
 create mode 100644 tools/binman/etype/fdtmap.py
 create mode 100644 tools/binman/test/115_fdtmap.dts

diff --git a/tools/binman/README b/tools/binman/README
index f07de350f60..77f047bf6a3 100644
--- a/tools/binman/README
+++ b/tools/binman/README
@@ -638,6 +638,10 @@ the image definition, binman calculates the final values and writes these to
 the device tree. These can be used by U-Boot at run-time to find the location
 of each entry.
 
+Alternatively, an FDT map entry can be used to add a special FDT containing
+just the information about the image. This is preceeded by a magic string so can
+be located anywhere in the image.
+
 
 Compression
 -----------
@@ -814,7 +818,6 @@ Some ideas:
 - Support building an image for a board (-b) more completely, with a
   configurable build directory
 - Support putting the FDT in an image with a suitable magic number
-- Support adding a pointer to the FDT map
 - Support listing files in images
 - Support logging of binman's operations, with different levels of verbosity
 
diff --git a/tools/binman/README.entries b/tools/binman/README.entries
index 3241befc7f4..7014d36f5ff 100644
--- a/tools/binman/README.entries
+++ b/tools/binman/README.entries
@@ -223,6 +223,44 @@ updating the EC on startup via software sync.
 
 
 
+Entry: fdtmap: An entry which contains an FDT map
+-------------------------------------------------
+
+Properties / Entry arguments:
+    None
+
+An FDT map is just a header followed by an FDT containing a list of all the
+entries in the image.
+
+The header is the string _FDTMAP_ followed by 8 unused bytes.
+
+When used, this entry will be populated with an FDT map which reflects the
+entries in the current image. Hierarchy is preserved, and all offsets and
+sizes are included.
+
+Note that the -u option must be provided to ensure that binman updates the
+FDT with the position of each entry.
+
+Example output for a simple image with U-Boot and an FDT map:
+
+/ {
+    size = <0x00000112>;
+    image-pos = <0x00000000>;
+    offset = <0x00000000>;
+    u-boot {
+        size = <0x00000004>;
+        image-pos = <0x00000000>;
+        offset = <0x00000000>;
+    };
+    fdtmap {
+        size = <0x0000010e>;
+        image-pos = <0x00000004>;
+        offset = <0x00000004>;
+    };
+};
+
+
+
 Entry: files: Entry containing a set of files
 ---------------------------------------------
 
diff --git a/tools/binman/etype/fdtmap.py b/tools/binman/etype/fdtmap.py
new file mode 100644
index 00000000000..cdeb491ebdc
--- /dev/null
+++ b/tools/binman/etype/fdtmap.py
@@ -0,0 +1,109 @@
+# SPDX-License-Identifier: GPL-2.0+
+# Copyright (c) 2018 Google, Inc
+# Written by Simon Glass <sjg@chromium.org>
+
+"""# Entry-type module for a full map of the firmware image
+
+This handles putting an FDT into the image with just the information about the
+image.
+"""
+
+import libfdt
+
+from entry import Entry
+from fdt import Fdt
+import state
+import tools
+
+FDTMAP_MAGIC = b'_FDTMAP_'
+
+class Entry_fdtmap(Entry):
+    """An entry which contains an FDT map
+
+    Properties / Entry arguments:
+        None
+
+    An FDT map is just a header followed by an FDT containing a list of all the
+    entries in the image.
+
+    The header is the string _FDTMAP_ followed by 8 unused bytes.
+
+    When used, this entry will be populated with an FDT map which reflects the
+    entries in the current image. Hierarchy is preserved, and all offsets and
+    sizes are included.
+
+    Note that the -u option must be provided to ensure that binman updates the
+    FDT with the position of each entry.
+
+    Example output for a simple image with U-Boot and an FDT map:
+
+    / {
+        size = <0x00000112>;
+        image-pos = <0x00000000>;
+        offset = <0x00000000>;
+        u-boot {
+            size = <0x00000004>;
+            image-pos = <0x00000000>;
+            offset = <0x00000000>;
+        };
+        fdtmap {
+            size = <0x0000010e>;
+            image-pos = <0x00000004>;
+            offset = <0x00000004>;
+        };
+    };
+    """
+    def __init__(self, section, etype, node):
+        Entry.__init__(self, section, etype, node)
+
+    def _GetFdtmap(self):
+        """Build an FDT map from the entries in the current image
+
+        Returns:
+            FDT map binary data
+        """
+        def _AddNode(node):
+            """Add a node to the FDT map"""
+            for pname, prop in node.props.items():
+                fsw.property(pname, prop.bytes)
+            for subnode in node.subnodes:
+                with fsw.add_node(subnode.name):
+                    _AddNode(subnode)
+
+        # Get the FDT data into an Fdt object
+        data = state.GetFdtContents()[1]
+        infdt = Fdt.FromData(data)
+        infdt.Scan()
+
+        # Find the node for the image containing the Fdt-map entry
+        path = self.section.GetPath()
+        node = infdt.GetNode(path)
+        if not node:
+            self.Raise("Internal error: Cannot locate node for path '%s'" %
+                       path)
+
+        # Build a new tree with all nodes and properties starting from that node
+        fsw = libfdt.FdtSw()
+        fsw.finish_reservemap()
+        with fsw.add_node(''):
+            _AddNode(node)
+        fdt = fsw.as_fdt()
+
+        # Pack this new FDT and return its contents
+        fdt.pack()
+        outfdt = Fdt.FromData(fdt.as_bytearray())
+        data = FDTMAP_MAGIC + tools.GetBytes(0, 8) + outfdt.GetContents()
+        return data
+
+    def ObtainContents(self):
+        """Obtain a placeholder for the fdt-map contents"""
+        self.SetContents(self._GetFdtmap())
+        return True
+
+    def ProcessContents(self):
+        """Write an updated version of the FDT map to this entry
+
+        This is necessary since new data may have been written back to it during
+        processing, e.g. the image-pos properties.
+        """
+        self.SetContents(self._GetFdtmap())
diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
index 4c5fd920e10..9e61f785d92 100644
--- a/tools/binman/ftest.py
+++ b/tools/binman/ftest.py
@@ -464,16 +464,16 @@ class TestFunctional(unittest.TestCase):
         """
         return struct.unpack('>L', dtb[4:8])[0]
 
-    def _GetPropTree(self, dtb, prop_names):
+    def _GetPropTree(self, dtb, prop_names, prefix='/binman/'):
         def AddNode(node, path):
             if node.name != '/':
                 path += '/' + node.name
+            for prop in node.props.values():
+                if prop.name in prop_names:
+                    prop_path = path + ':' + prop.name
+                    tree[prop_path[len(prefix):]] = fdt_util.fdt32_to_cpu(
+                        prop.value)
             for subnode in node.subnodes:
-                for prop in subnode.props.values():
-                    if prop.name in prop_names:
-                        prop_path = path + '/' + subnode.name + ':' + prop.name
-                        tree[prop_path[len('/binman/'):]] = fdt_util.fdt32_to_cpu(
-                            prop.value)
                 AddNode(subnode, path)
 
         tree = {}
@@ -2015,6 +2015,48 @@ class TestFunctional(unittest.TestCase):
         self.assertEqual(U_BOOT_DTB_DATA, cfile2.data)
         self.assertEqual(0x140, cfile2.cbfs_offset)
 
+    def testFdtmap(self):
+        """Test an FDT map can be inserted in the image"""
+        data = self._DoReadFileDtb('115_fdtmap.dts', use_real_dtb=True,
+                                   update_dtb=True)[0]
+        fdtmap_data = data[len(U_BOOT_DATA):]
+        magic = fdtmap_data[:8]
+        self.assertEqual('_FDTMAP_', magic)
+        self.assertEqual(tools.GetBytes(0, 8), fdtmap_data[8:16])
+
+        fdt_data = fdtmap_data[16:]
+        dtb = fdt.Fdt.FromData(fdt_data)
+        dtb.Scan()
+        props = self._GetPropTree(dtb, ['offset', 'size', 'image-pos'],
+                                  prefix='/')
+        self.assertEqual({
+            'image-pos': 0,
+            'offset': 0,
+            'u-boot:offset': 0,
+            'u-boot:size': len(U_BOOT_DATA),
+            'u-boot:image-pos': 0,
+            'fdtmap:image-pos': 4,
+            'fdtmap:offset': 4,
+            'fdtmap:size': len(fdtmap_data),
+            'size': len(data),
+        }, props)
+
+    def testFdtmapNoMatch(self):
+        """Check handling of an FDT map when the section cannot be found"""
+        self._DoReadFileDtb('115_fdtmap.dts', use_real_dtb=True,
+                            update_dtb=True)
+
+        # Mangle the section name, which should cause a mismatch between the
+        # correct FDT path and the one expected by the section
+        image = control.images['image']
+        image._section._node.path += '-suffix'
+        entries = image.GetEntries()
+        fdtmap = entries['fdtmap']
+        with self.assertRaises(ValueError) as e:
+            fdtmap._GetFdtmap()
+        self.assertIn("Cannot locate node for path '/binman-suffix'",
+                      str(e.exception))
+
 
 if __name__ == "__main__":
     unittest.main()
diff --git a/tools/binman/state.py b/tools/binman/state.py
index af9678649cd..3ccd7855d47 100644
--- a/tools/binman/state.py
+++ b/tools/binman/state.py
@@ -59,7 +59,7 @@ def GetFdtPath(fname):
     """
     return fdt_files[fname]._fname
 
-def GetFdtContents(fname):
+def GetFdtContents(fname='u-boot.dtb'):
     """Looks up the FDT pathname and contents
 
     This is used to obtain the Fdt pathname and contents when needed by an
diff --git a/tools/binman/test/115_fdtmap.dts b/tools/binman/test/115_fdtmap.dts
new file mode 100644
index 00000000000..2450c41f200
--- /dev/null
+++ b/tools/binman/test/115_fdtmap.dts
@@ -0,0 +1,13 @@
+/dts-v1/;
+
+/ {
+	#address-cells = <1>;
+	#size-cells = <1>;
+
+	binman {
+		u-boot {
+		};
+		fdtmap {
+		};
+	};
+};
-- 
2.22.0.410.gd8fdbe21b5-goog

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [U-Boot] [PATCH 05/26] binman: Add an image header
  2019-07-02  0:24 [U-Boot] [PATCH 00/26] binman: Allow reading of images to list contents Simon Glass
                   ` (3 preceding siblings ...)
  2019-07-02  0:24 ` [U-Boot] [PATCH 04/26] binman: Add an FDT map Simon Glass
@ 2019-07-02  0:24 ` Simon Glass
  2019-07-02  0:24 ` [U-Boot] [PATCH 06/26] binman: Allow cbfstool to be optional with tests Simon Glass
                   ` (20 subsequent siblings)
  25 siblings, 0 replies; 27+ messages in thread
From: Simon Glass @ 2019-07-02  0:24 UTC (permalink / raw)
  To: u-boot

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

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

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

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

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

diff --git a/tools/binman/README b/tools/binman/README
index 77f047bf6a3..61a7a20f232 100644
--- a/tools/binman/README
+++ b/tools/binman/README
@@ -640,7 +640,9 @@ of each entry.
 
 Alternatively, an FDT map entry can be used to add a special FDT containing
 just the information about the image. This is preceeded by a magic string so can
-be located anywhere in the image.
+be located anywhere in the image. An image header (typically at the start or end
+of the image) can be used to point to the FDT map. See fdtmap and image-header
+entries for more information.
 
 
 Compression
@@ -817,7 +819,6 @@ Some ideas:
 - Add an option to decode an image into the constituent binaries
 - Support building an image for a board (-b) more completely, with a
   configurable build directory
-- Support putting the FDT in an image with a suitable magic number
 - Support listing files in images
 - Support logging of binman's operations, with different levels of verbosity
 
diff --git a/tools/binman/README.entries b/tools/binman/README.entries
index 7014d36f5ff..598d8278a70 100644
--- a/tools/binman/README.entries
+++ b/tools/binman/README.entries
@@ -331,6 +331,25 @@ README.chromium for how to obtain the required keys and tools.
 
 
 
+Entry: image-header: An entry which contains a pointer to the FDT map
+---------------------------------------------------------------------
+
+Properties / Entry arguments:
+    location: Location of header ("start" or "end" of image). This is
+        optional. If omitted then the entry must have an offset property.
+
+This adds an 8-byte entry to the start or end of the image, pointing to the
+location of the FDT map. The format is a magic number followed by an offset
+from the start or end of the image, in twos-compliment format.
+
+This entry must be in the top-level part of the image.
+
+NOTE: If the location is at the start/end, you will probably need to specify
+sort-by-offset for the image, unless you actually put the image header
+first/last in the entry list.
+
+
+
 Entry: intel-cmc: Entry containing an Intel Chipset Micro Code (CMC) file
 -------------------------------------------------------------------------
 
diff --git a/tools/binman/entry.py b/tools/binman/entry.py
index 7356c49c626..e1cd0d3a882 100644
--- a/tools/binman/entry.py
+++ b/tools/binman/entry.py
@@ -561,3 +561,14 @@ features to produce new behaviours.
                 else False
         """
         return name in self.section.GetEntries()
+
+    def GetSiblingImagePos(self, name):
+        """Return the image position of the given sibling
+
+        Returns:
+            Image position of sibling, or None if the sibling has no position,
+                or False if there is no such sibling
+        """
+        if not self.HasSibling(name):
+            return False
+        return self.section.GetEntries()[name].image_pos
diff --git a/tools/binman/etype/image_header.py b/tools/binman/etype/image_header.py
new file mode 100644
index 00000000000..9bc84ec01d4
--- /dev/null
+++ b/tools/binman/etype/image_header.py
@@ -0,0 +1,76 @@
+# SPDX-License-Identifier: GPL-2.0+
+# Copyright (c) 2018 Google, Inc
+# Written by Simon Glass <sjg@chromium.org>
+
+"""Entry-type module for an image header which points to the FDT map
+
+This creates an 8-byte entry with a magic number and the offset of the FDT map
+(which is another entry in the image), relative to the start or end of the
+image.
+"""
+
+import struct
+
+from entry import Entry
+import fdt_util
+
+IMAGE_HEADER_MAGIC = b'BinM'
+
+class Entry_image_header(Entry):
+    """An entry which contains a pointer to the FDT map
+
+    Properties / Entry arguments:
+        location: Location of header ("start" or "end" of image). This is
+            optional. If omitted then the entry must have an offset property.
+
+    This adds an 8-byte entry to the start or end of the image, pointing to the
+    location of the FDT map. The format is a magic number followed by an offset
+    from the start or end of the image, in twos-compliment format.
+
+    This entry must be in the top-level part of the image.
+
+    NOTE: If the location is at the start/end, you will probably need to specify
+    sort-by-offset for the image, unless you actually put the image header
+    first/last in the entry list.
+    """
+    def __init__(self, section, etype, node):
+        Entry.__init__(self, section, etype, node)
+        self.location = fdt_util.GetString(self._node, 'location')
+
+    def _GetHeader(self):
+        image_pos = self.GetSiblingImagePos('fdtmap')
+        if image_pos == False:
+            self.Raise("'image_header' section must have an 'fdtmap' sibling")
+        elif image_pos is None:
+            # This will be available when called from ProcessContents(), but not
+            # when called from ObtainContents()
+            offset = 0xffffffff
+        else:
+            image_size = self.section.GetImageSize() or 0
+            base = (0 if self.location != 'end' else image_size)
+            offset = (image_pos - base) & 0xffffffff
+        data = IMAGE_HEADER_MAGIC + struct.pack('<I', offset)
+        return data
+
+    def ObtainContents(self):
+        """Obtain a placeholder for the header contents"""
+        self.SetContents(self._GetHeader())
+        return True
+
+    def Pack(self, offset):
+        """Special pack method to set the offset to start/end of image"""
+        if not self.offset:
+            if self.location not in ['start', 'end']:
+                self.Raise("Invalid location '%s', expected 'start' or 'end'" %
+                           self.location)
+            image_size = self.section.GetImageSize() or 0
+            self.offset = (0 if self.location != 'end' else image_size - 8)
+        return Entry.Pack(self, offset)
+
+    def ProcessContents(self):
+        """Write an updated version of the FDT map to this entry
+
+        This is necessary since image_pos is not available when ObtainContents()
+        is called, since by then the entries have not been packed in the image.
+        """
+        self.SetContents(self._GetHeader())
diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
index 9e61f785d92..46540e8f5dd 100644
--- a/tools/binman/ftest.py
+++ b/tools/binman/ftest.py
@@ -2057,6 +2057,56 @@ class TestFunctional(unittest.TestCase):
         self.assertIn("Cannot locate node for path '/binman-suffix'",
                       str(e.exception))
 
+    def testFdtmapHeader(self):
+        """Test an FDT map and image header can be inserted in the image"""
+        data = self._DoReadFileDtb('116_fdtmap_hdr.dts', use_real_dtb=True,
+                                   update_dtb=True)[0]
+        fdtmap_pos = len(U_BOOT_DATA)
+        fdtmap_data = data[fdtmap_pos:]
+        fdt_data = fdtmap_data[16:]
+        dtb = fdt.Fdt.FromData(fdt_data)
+        fdt_size = dtb.GetFdtObj().totalsize()
+        hdr_data = data[-8:]
+        self.assertEqual('BinM', hdr_data[:4])
+        offset = struct.unpack('<I', hdr_data[4:])[0] & 0xffffffff
+        self.assertEqual(fdtmap_pos - 0x400, offset - (1 << 32))
+
+    def testFdtmapHeaderStart(self):
+        """Test an image header can be inserted at the image start"""
+        data = self._DoReadFileDtb('117_fdtmap_hdr_start.dts',
+                                   use_real_dtb=True, update_dtb=True)[0]
+        fdtmap_pos = 0x100 + len(U_BOOT_DATA)
+        hdr_data = data[:8]
+        self.assertEqual('BinM', hdr_data[:4])
+        offset = struct.unpack('<I', hdr_data[4:])[0]
+        self.assertEqual(fdtmap_pos, offset)
+
+    def testFdtmapHeaderPos(self):
+        """Test an image header can be inserted@a chosen position"""
+        data = self._DoReadFileDtb('118_fdtmap_hdr_pos.dts',
+                                   use_real_dtb=True, update_dtb=True)[0]
+        fdtmap_pos = 0x100 + len(U_BOOT_DATA)
+        hdr_data = data[0x80:0x88]
+        self.assertEqual('BinM', hdr_data[:4])
+        offset = struct.unpack('<I', hdr_data[4:])[0]
+        self.assertEqual(fdtmap_pos, offset)
+
+    def testHeaderMissingFdtmap(self):
+        """Test an image header requires an fdtmap"""
+        with self.assertRaises(ValueError) as e:
+            self._DoReadFileDtb('119_fdtmap_hdr_missing.dts',
+                                use_real_dtb=True, update_dtb=True)
+        self.assertIn("'image_header' section must have an 'fdtmap' sibling",
+                      str(e.exception))
+
+    def testHeaderNoLocation(self):
+        """Test an image header with a no specified location is detected"""
+        with self.assertRaises(ValueError) as e:
+            self._DoReadFileDtb('120_hdr_no_location.dts',
+                                use_real_dtb=True, update_dtb=True)
+        self.assertIn("Invalid location 'None', expected 'start' or 'end'",
+                      str(e.exception))
+
 
 if __name__ == "__main__":
     unittest.main()
diff --git a/tools/binman/test/116_fdtmap_hdr.dts b/tools/binman/test/116_fdtmap_hdr.dts
new file mode 100644
index 00000000000..77a2194b394
--- /dev/null
+++ b/tools/binman/test/116_fdtmap_hdr.dts
@@ -0,0 +1,17 @@
+/dts-v1/;
+
+/ {
+	#address-cells = <1>;
+	#size-cells = <1>;
+
+	binman {
+		size = <0x400>;
+		u-boot {
+		};
+		fdtmap {
+		};
+		image-header {
+			location = "end";
+		};
+	};
+};
diff --git a/tools/binman/test/117_fdtmap_hdr_start.dts b/tools/binman/test/117_fdtmap_hdr_start.dts
new file mode 100644
index 00000000000..17b6be00470
--- /dev/null
+++ b/tools/binman/test/117_fdtmap_hdr_start.dts
@@ -0,0 +1,19 @@
+/dts-v1/;
+
+/ {
+	#address-cells = <1>;
+	#size-cells = <1>;
+
+	binman {
+		size = <0x400>;
+		sort-by-offset;
+		u-boot {
+			offset = <0x100>;
+		};
+		fdtmap {
+		};
+		image-header {
+			location = "start";
+		};
+	};
+};
diff --git a/tools/binman/test/118_fdtmap_hdr_pos.dts b/tools/binman/test/118_fdtmap_hdr_pos.dts
new file mode 100644
index 00000000000..fd803f57fba
--- /dev/null
+++ b/tools/binman/test/118_fdtmap_hdr_pos.dts
@@ -0,0 +1,19 @@
+/dts-v1/;
+
+/ {
+	#address-cells = <1>;
+	#size-cells = <1>;
+
+	binman {
+		size = <0x400>;
+		sort-by-offset;
+		u-boot {
+			offset = <0x100>;
+		};
+		fdtmap {
+		};
+		image-header {
+			offset = <0x80>;
+		};
+	};
+};
diff --git a/tools/binman/test/119_fdtmap_hdr_missing.dts b/tools/binman/test/119_fdtmap_hdr_missing.dts
new file mode 100644
index 00000000000..41bb680f08f
--- /dev/null
+++ b/tools/binman/test/119_fdtmap_hdr_missing.dts
@@ -0,0 +1,16 @@
+/dts-v1/;
+
+/ {
+	#address-cells = <1>;
+	#size-cells = <1>;
+
+	binman {
+		sort-by-offset;
+		u-boot {
+		};
+		image-header {
+			offset = <0x80>;
+			location = "start";
+		};
+	};
+};
diff --git a/tools/binman/test/120_hdr_no_location.dts b/tools/binman/test/120_hdr_no_location.dts
new file mode 100644
index 00000000000..585e21f456b
--- /dev/null
+++ b/tools/binman/test/120_hdr_no_location.dts
@@ -0,0 +1,16 @@
+/dts-v1/;
+
+/ {
+	#address-cells = <1>;
+	#size-cells = <1>;
+
+	binman {
+		sort-by-offset;
+		u-boot {
+		};
+		fdtmap {
+		};
+		image-header {
+		};
+	};
+};
-- 
2.22.0.410.gd8fdbe21b5-goog

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [U-Boot] [PATCH 06/26] binman: Allow cbfstool to be optional with tests
  2019-07-02  0:24 [U-Boot] [PATCH 00/26] binman: Allow reading of images to list contents Simon Glass
                   ` (4 preceding siblings ...)
  2019-07-02  0:24 ` [U-Boot] [PATCH 05/26] binman: Add an image header Simon Glass
@ 2019-07-02  0:24 ` Simon Glass
  2019-07-02  0:24 ` [U-Boot] [PATCH 07/26] binman: Convert to use ArgumentParser Simon Glass
                   ` (19 subsequent siblings)
  25 siblings, 0 replies; 27+ messages in thread
From: Simon Glass @ 2019-07-02  0:24 UTC (permalink / raw)
  To: u-boot

If cbfstool is available, it is useful to compare the output of binman's
CBFS generation with the output from that tool. However this is not
absolutely necessary for tests, and cbfstool is not widely available, so
make it optional. This allows tests to run on travis, for example.

Also update travis to install the lzma tool required for CBFS.

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

 .travis.yml                    |  1 +
 tools/binman/cbfs_util_test.py | 37 +++++++++++++++++++++++++++-------
 2 files changed, 31 insertions(+), 7 deletions(-)

diff --git a/.travis.yml b/.travis.yml
index 941f032cd38..ae68a2d351c 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -32,6 +32,7 @@ addons:
     - device-tree-compiler
     - lzop
     - liblz4-tool
+    - lzma-alone
     - libisl15
     - clang-7
     - srecord
diff --git a/tools/binman/cbfs_util_test.py b/tools/binman/cbfs_util_test.py
index a0eb68ffce4..e77f5c51c98 100755
--- a/tools/binman/cbfs_util_test.py
+++ b/tools/binman/cbfs_util_test.py
@@ -47,6 +47,12 @@ class TestCbfs(unittest.TestCase):
         # compressing files
         tools.PrepareOutputDir(None)
 
+        cls.have_cbfstool = True
+        try:
+            tools.Run('which', 'cbfstool')
+        except:
+            cls.have_cbfstool = False
+
     @classmethod
     def tearDownClass(cls):
         """Remove the temporary input directory and its contents"""
@@ -152,6 +158,19 @@ class TestCbfs(unittest.TestCase):
         self._check_dtb(cbfs)
 
     def _get_expected_cbfs(self, size, arch='x86', compress=None, base=None):
+        """Get the file created by cbfstool for a particular scenario
+
+        Args:
+            size: Size of the CBFS in bytes
+            arch: Architecture of the CBFS, as a string
+            compress: Compression to use, e.g. cbfs_util.COMPRESS_LZMA
+            base: Base address of file, or None to put it anywhere
+
+        Returns:
+            Resulting CBFS file, or None if cbfstool is not available
+        """
+        if not self.have_cbfstool:
+            return None
         cbfs_fname = os.path.join(self._indir, 'test.cbfs')
         cbfs_util.cbfstool(cbfs_fname, 'create', '-m', arch, '-s', '%#x' % size)
         if base:
@@ -178,6 +197,8 @@ class TestCbfs(unittest.TestCase):
             data: CBFS created by binman
             cbfstool_fname: CBFS created by cbfstool
         """
+        if not self.have_cbfstool:
+            return
         expect = tools.ReadFile(cbfstool_fname)
         if expect != data:
             tools.WriteFile('/tmp/expect', expect)
@@ -195,6 +216,8 @@ class TestCbfs(unittest.TestCase):
 
     def test_cbfstool_failure(self):
         """Test failure to run cbfstool"""
+        if not self.have_cbfstool:
+            self.skipTest('No cbfstool available')
         try:
             # In verbose mode this test fails since stderr is not captured. Fix
             # this by turning off verbosity.
@@ -467,12 +490,6 @@ class TestCbfs(unittest.TestCase):
         cbw = CbfsWriter(size)
         cbw.add_file_stage('u-boot', tools.ReadFile(elf_fname))
 
-        cbfs_fname = os.path.join(self._indir, 'test.cbfs')
-        cbfs_util.cbfstool(cbfs_fname, 'create', '-m', 'x86', '-s',
-                           '%#x' % size)
-        cbfs_util.cbfstool(cbfs_fname, 'add-stage', '-n', 'u-boot',
-                           '-f', elf_fname)
-
         data = cbw.get_data()
         cbfs = self._check_hdr(data, size)
         load = 0xfef20000
@@ -487,7 +504,13 @@ class TestCbfs(unittest.TestCase):
                          cfile.data_len)
 
         # Compare against what cbfstool creates
-        self._compare_expected_cbfs(data, cbfs_fname)
+        if self.have_cbfstool:
+            cbfs_fname = os.path.join(self._indir, 'test.cbfs')
+            cbfs_util.cbfstool(cbfs_fname, 'create', '-m', 'x86', '-s',
+                               '%#x' % size)
+            cbfs_util.cbfstool(cbfs_fname, 'add-stage', '-n', 'u-boot',
+                               '-f', elf_fname)
+            self._compare_expected_cbfs(data, cbfs_fname)
 
     def test_cbfs_raw_compress(self):
         """Test base handling of compressing raw files"""
-- 
2.22.0.410.gd8fdbe21b5-goog

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [U-Boot] [PATCH 07/26] binman: Convert to use ArgumentParser
  2019-07-02  0:24 [U-Boot] [PATCH 00/26] binman: Allow reading of images to list contents Simon Glass
                   ` (5 preceding siblings ...)
  2019-07-02  0:24 ` [U-Boot] [PATCH 06/26] binman: Allow cbfstool to be optional with tests Simon Glass
@ 2019-07-02  0:24 ` Simon Glass
  2019-07-02  0:24 ` [U-Boot] [PATCH 08/26] binman: Move compression into the Entry base class Simon Glass
                   ` (18 subsequent siblings)
  25 siblings, 0 replies; 27+ messages in thread
From: Simon Glass @ 2019-07-02  0:24 UTC (permalink / raw)
  To: u-boot

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

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

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

diff --git a/.travis.yml b/.travis.yml
index ae68a2d351c..4592c00c9ce 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -146,7 +146,7 @@ script:
    if [[ -n "${TEST_PY_TOOLS}" ]]; then
      PYTHONPATH="${UBOOT_TRAVIS_BUILD_DIR}/scripts/dtc/pylibfdt"
      PATH="${UBOOT_TRAVIS_BUILD_DIR}/scripts/dtc:${PATH}"
-     ./tools/binman/binman --toolpath ${UBOOT_TRAVIS_BUILD_DIR}/tools -t &&
+     ./tools/binman/binman --toolpath ${UBOOT_TRAVIS_BUILD_DIR}/tools test &&
      ./tools/patman/patman --test &&
      ./tools/buildman/buildman -t &&
      PYTHONPATH="${UBOOT_TRAVIS_BUILD_DIR}/scripts/dtc/pylibfdt"
diff --git a/test/run b/test/run
index b97647eba6f..fc7fe5c2bbc 100755
--- a/test/run
+++ b/test/run
@@ -40,7 +40,7 @@ export PYTHONPATH=${DTC_DIR}/pylibfdt
 export DTC=${DTC_DIR}/dtc
 TOOLS_DIR=build-sandbox_spl/tools
 
-run_test "binman" ./tools/binman/binman -t --toolpath ${TOOLS_DIR}
+run_test "binman" ./tools/binman/binman test --toolpath ${TOOLS_DIR}
 run_test "patman" ./tools/patman/patman --test
 
 [ "$1" == "quick" ] && skip=--skip-net-tests
@@ -52,7 +52,8 @@ run_test "dtoc" ./tools/dtoc/dtoc -t
 # To enable Python test coverage on Debian-type distributions (e.g. Ubuntu):
 #   $ sudo apt-get install python-pytest python-coverage
 export PATH=$PATH:${TOOLS_DIR}
-run_test "binman code coverage" ./tools/binman/binman -T --toolpath ${TOOLS_DIR}
+run_test "binman code coverage" ./tools/binman/binman test -T \
+    --toolpath ${TOOLS_DIR}
 run_test "dtoc code coverage" ./tools/dtoc/dtoc -T
 run_test "fdt code coverage" ./tools/dtoc/test_fdt -T
 
diff --git a/tools/binman/README b/tools/binman/README
index 61a7a20f232..99b4e5f4504 100644
--- a/tools/binman/README
+++ b/tools/binman/README
@@ -187,7 +187,7 @@ First install prerequisites, e.g.
 
 Type:
 
-	binman -b <board_name>
+	binman build -b <board_name>
 
 to build an image for a board. The board name is the same name used when
 configuring U-Boot (e.g. for sandbox_defconfig the board name is 'sandbox').
@@ -195,7 +195,7 @@ Binman assumes that the input files for the build are in ../b/<board_name>.
 
 Or you can specify this explicitly:
 
-	binman -I <build_path>
+	binman build -I <build_path>
 
 where <build_path> is the build directory containing the output of the U-Boot
 build.
@@ -483,7 +483,7 @@ Entry Documentation
 For details on the various entry types supported by binman and how to use them,
 see README.entries. This is generated from the source code using:
 
-	binman -E >tools/binman/README.entries
+	binman entry-docs >tools/binman/README.entries
 
 
 Hashing Entries
diff --git a/tools/binman/binman.py b/tools/binman/binman.py
index 613aad5c451..d225fe86024 100755
--- a/tools/binman/binman.py
+++ b/tools/binman/binman.py
@@ -20,14 +20,15 @@ import sys
 import traceback
 import unittest
 
-# Bring in the patman and dtoc libraries
+# Bring in the patman and dtoc libraries (but don't override the first path
+# in PYTHONPATH)
 our_path = os.path.dirname(os.path.realpath(__file__))
 for dirname in ['../patman', '../dtoc', '..', '../concurrencytest']:
-    sys.path.insert(0, os.path.join(our_path, dirname))
+    sys.path.insert(2, os.path.join(our_path, dirname))
 
 # Bring in the libfdt module
-sys.path.insert(0, 'scripts/dtc/pylibfdt')
-sys.path.insert(0, os.path.join(our_path,
+sys.path.insert(2, 'scripts/dtc/pylibfdt')
+sys.path.insert(2, os.path.join(our_path,
                 '../../build-sandbox_spl/scripts/dtc/pylibfdt'))
 
 # When running under python-coverage on Ubuntu 16.04, the dist-packages
@@ -158,37 +159,36 @@ def RunTestCoverage():
                    for item in glob_list if '_testing' not in item])
     test_util.RunTestCoverage('tools/binman/binman.py', None,
             ['*test*', '*binman.py', 'tools/patman/*', 'tools/dtoc/*'],
-            options.build_dir, all_set)
+            args.build_dir, all_set)
 
-def RunBinman(options, args):
+def RunBinman(args):
     """Main entry point to binman once arguments are parsed
 
     Args:
-        options: Command-line options
-        args: Non-option arguments
+        args: Command line arguments Namespace object
     """
     ret_code = 0
 
-    if not options.debug:
+    if not args.debug:
         sys.tracebacklimit = 0
 
-    if options.test:
-        ret_code = RunTests(options.debug, options.verbosity, options.processes,
-                            options.test_preserve_dirs, args[1:],
-                            options.toolpath)
-
-    elif options.test_coverage:
-        RunTestCoverage()
+    if args.cmd == 'test':
+        if args.test_coverage:
+            RunTestCoverage()
+        else:
+            ret_code = RunTests(args.debug, args.verbosity, args.processes,
+                                args.test_preserve_dirs, args.tests,
+                                args.toolpath)
 
-    elif options.entry_docs:
+    elif args.cmd == 'entry-docs':
         control.WriteEntryDocs(GetEntryModules())
 
     else:
         try:
-            ret_code = control.Binman(options, args)
+            ret_code = control.Binman(args)
         except Exception as e:
             print('binman: %s' % e)
-            if options.debug:
+            if args.debug:
                 print()
                 traceback.print_exc()
             ret_code = 1
@@ -196,6 +196,7 @@ def RunBinman(options, args):
 
 
 if __name__ == "__main__":
-    (options, args) = cmdline.ParseArgs(sys.argv)
-    ret_code = RunBinman(options, args)
+    args = cmdline.ParseArgs(sys.argv[1:])
+
+    ret_code = RunBinman(args)
     sys.exit(ret_code)
diff --git a/tools/binman/cmdline.py b/tools/binman/cmdline.py
index 91e007e4e03..99ca6564654 100644
--- a/tools/binman/cmdline.py
+++ b/tools/binman/cmdline.py
@@ -5,7 +5,7 @@
 # Command-line parser for binman
 #
 
-from optparse import OptionParser
+from argparse import ArgumentParser
 
 def ParseArgs(argv):
     """Parse the binman command-line arguments
@@ -17,56 +17,72 @@ def ParseArgs(argv):
             options provides access to the options (e.g. option.debug)
             args is a list of string arguments
     """
-    parser = OptionParser()
-    parser.add_option('-a', '--entry-arg', type='string', action='append',
+    if '-H' in argv:
+        argv.insert(0, 'build')
+
+    usage = '''binman [-B <dir<] [-D] [-H] [-v<n>] [--toolpath <path>] <cmd> <options>
+
+Create and manipulate images for a board from a set of binaries. Binman is
+controlled by a description in the board device tree.'''
+
+    base_parser = ArgumentParser(add_help=False)
+    base_parser.add_argument('-B', '--build-dir', type=str, default='b',
+        help='Directory containing the build output')
+    base_parser.add_argument('-D', '--debug', action='store_true',
+        help='Enabling debugging (provides a full traceback on error)')
+    base_parser.add_argument('-H', '--full-help', action='store_true',
+        default=False, help='Display the README file')
+    base_parser.add_argument('--toolpath', type=str, action='append',
+        help='Add a path to the directories containing tools')
+    base_parser.add_argument('-v', '--verbosity', default=1,
+        type=int, help='Control verbosity: 0=silent, 1=progress, 3=full, '
+        '4=debug')
+
+    parser = ArgumentParser(usage=usage, parents=[base_parser])
+    subparsers = parser.add_subparsers(dest='cmd')
+
+    build_parser = subparsers.add_parser('build', help='Build firmware image',
+                                         parents=[base_parser])
+    build_parser.add_argument('-a', '--entry-arg', type=str, action='append',
             help='Set argument value arg=value')
-    parser.add_option('-b', '--board', type='string',
+    build_parser.add_argument('-b', '--board', type=str,
             help='Board name to build')
-    parser.add_option('-B', '--build-dir', type='string', default='b',
-            help='Directory containing the build output')
-    parser.add_option('-d', '--dt', type='string',
+    build_parser.add_argument('-d', '--dt', type=str,
             help='Configuration file (.dtb) to use')
-    parser.add_option('-D', '--debug', action='store_true',
-            help='Enabling debugging (provides a full traceback on error)')
-    parser.add_option('-E', '--entry-docs', action='store_true',
-            help='Write out entry documentation (see README.entries)')
-    parser.add_option('--fake-dtb', action='store_true',
+    build_parser.add_argument('--fake-dtb', action='store_true',
             help='Use fake device tree contents (for testing only)')
-    parser.add_option('-i', '--image', type='string', action='append',
+    build_parser.add_argument('-i', '--image', type=str, action='append',
             help='Image filename to build (if not specified, build all)')
-    parser.add_option('-I', '--indir', action='append',
+    build_parser.add_argument('-I', '--indir', action='append',
             help='Add a path to the list of directories to use for input files')
-    parser.add_option('-H', '--full-help', action='store_true',
-        default=False, help='Display the README file')
-    parser.add_option('-m', '--map', action='store_true',
+    build_parser.add_argument('-m', '--map', action='store_true',
         default=False, help='Output a map file for each image')
-    parser.add_option('-O', '--outdir', type='string',
+    build_parser.add_argument('-O', '--outdir', type=str,
         action='store', help='Path to directory to use for intermediate and '
         'output files')
-    parser.add_option('-p', '--preserve', action='store_true',\
+    build_parser.add_argument('-p', '--preserve', action='store_true',\
         help='Preserve temporary output directory even if option -O is not '
              'given')
-    parser.add_option('-P', '--processes', type=int,
-                      help='set number of processes to use for running tests')
-    parser.add_option('-t', '--test', action='store_true',
-                    default=False, help='run tests')
-    parser.add_option('-T', '--test-coverage', action='store_true',
-                    default=False, help='run tests and check for 100% coverage')
-    parser.add_option('--toolpath', type='string', action='append',
-            help='Add a path to the directories containing tools')
-    parser.add_option('-u', '--update-fdt', action='store_true',
+    build_parser.add_argument('-u', '--update-fdt', action='store_true',
         default=False, help='Update the binman node with offset/size info')
-    parser.add_option('-v', '--verbosity', default=1,
-        type='int', help='Control verbosity: 0=silent, 1=progress, 3=full, '
-        '4=debug')
-    parser.add_option('-X', '--test-preserve-dirs', action='store_true',
+
+    entry_parser = subparsers.add_parser('entry-docs',
+        help='Write out entry documentation (see README.entries)')
+
+    list_parser = subparsers.add_parser('list', help='List files in an image')
+    list_parser.add_argument('fname', type=str, help='Image file to list')
+
+    test_parser = subparsers.add_parser('test', help='Run tests',
+                                        parents=[base_parser])
+    test_parser.add_argument('-P', '--processes', type=int,
+        help='set number of processes to use for running tests')
+    test_parser.add_argument('-T', '--test-coverage', action='store_true',
+        default=False, help='run tests and check for 100%% coverage')
+    test_parser.add_argument('-X', '--test-preserve-dirs', action='store_true',
         help='Preserve and display test-created input directories; also '
              'preserve the output directory if a single test is run (pass test '
              'name at the end of the command line')
-
-    parser.usage += """
-
-Create images for a board from a set of binaries. It is controlled by a
-description in the board device tree."""
+    test_parser.add_argument('tests', nargs='*',
+                             help='Test names to run (omit for all)')
 
     return parser.parse_args(argv)
diff --git a/tools/binman/control.py b/tools/binman/control.py
index 4a94afc8640..9022cf76e99 100644
--- a/tools/binman/control.py
+++ b/tools/binman/control.py
@@ -67,19 +67,18 @@ def WriteEntryDocs(modules, test_missing=None):
     from entry import Entry
     Entry.WriteDocs(modules, test_missing)
 
-def Binman(options, args):
+def Binman(args):
     """The main control code for binman
 
     This assumes that help and test options have already been dealt with. It
     deals with the core task of building images.
 
     Args:
-        options: Command line options object
-        args: Command line arguments (list of strings)
+        args: Command line arguments Namespace object
     """
     global images
 
-    if options.full_help:
+    if args.full_help:
         pager = os.getenv('PAGER')
         if not pager:
             pager = 'more'
@@ -89,17 +88,17 @@ def Binman(options, args):
         return 0
 
     # Try to figure out which device tree contains our image description
-    if options.dt:
-        dtb_fname = options.dt
+    if args.dt:
+        dtb_fname = args.dt
     else:
-        board = options.board
+        board = args.board
         if not board:
             raise ValueError('Must provide a board to process (use -b <board>)')
-        board_pathname = os.path.join(options.build_dir, board)
+        board_pathname = os.path.join(args.build_dir, board)
         dtb_fname = os.path.join(board_pathname, 'u-boot.dtb')
-        if not options.indir:
-            options.indir = ['.']
-        options.indir.append(board_pathname)
+        if not args.indir:
+            args.indir = ['.']
+        args.indir.append(board_pathname)
 
     try:
         # Import these here in case libfdt.py is not available, in which case
@@ -107,15 +106,15 @@ def Binman(options, args):
         import fdt
         import fdt_util
 
-        tout.Init(options.verbosity)
-        elf.debug = options.debug
-        cbfs_util.VERBOSE = options.verbosity > 2
-        state.use_fake_dtb = options.fake_dtb
+        tout.Init(args.verbosity)
+        elf.debug = args.debug
+        cbfs_util.VERBOSE = args.verbosity > 2
+        state.use_fake_dtb = args.fake_dtb
         try:
-            tools.SetInputDirs(options.indir)
-            tools.PrepareOutputDir(options.outdir, options.preserve)
-            tools.SetToolPaths(options.toolpath)
-            state.SetEntryArgs(options.entry_arg)
+            tools.SetInputDirs(args.indir)
+            tools.PrepareOutputDir(args.outdir, args.preserve)
+            tools.SetToolPaths(args.toolpath)
+            state.SetEntryArgs(args.entry_arg)
 
             # Get the device tree ready by compiling it and copying the compiled
             # output into a file in our output directly. Then scan it for use
@@ -132,16 +131,16 @@ def Binman(options, args):
 
             images = _ReadImageDesc(node)
 
-            if options.image:
+            if args.image:
                 skip = []
                 new_images = OrderedDict()
                 for name, image in images.items():
-                    if name in options.image:
+                    if name in args.image:
                         new_images[name] = image
                     else:
                         skip.append(name)
                 images = new_images
-                if skip and options.verbosity >= 2:
+                if skip and args.verbosity >= 2:
                     print('Skipping images: %s' % ', '.join(skip))
 
             state.Prepare(images, dtb)
@@ -155,7 +154,7 @@ def Binman(options, args):
             # entry offsets remain the same.
             for image in images.values():
                 image.ExpandEntries()
-                if options.update_fdt:
+                if args.update_fdt:
                     image.AddMissingProperties()
                 image.ProcessFdt(dtb)
 
@@ -176,19 +175,19 @@ def Binman(options, args):
                     image.CheckSize()
                     image.CheckEntries()
                 except Exception as e:
-                    if options.map:
+                    if args.map:
                         fname = image.WriteMap()
                         print("Wrote map file '%s' to show errors"  % fname)
                     raise
                 image.SetImagePos()
-                if options.update_fdt:
+                if args.update_fdt:
                     image.SetCalculatedProperties()
                     for dtb_item in state.GetFdts():
                         dtb_item.Sync()
                 image.ProcessEntryContents()
                 image.WriteSymbols()
                 image.BuildImage()
-                if options.map:
+                if args.map:
                     image.WriteMap()
 
             # Write the updated FDTs to our output files
diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
index 46540e8f5dd..f1a79baa905 100644
--- a/tools/binman/ftest.py
+++ b/tools/binman/ftest.py
@@ -207,7 +207,7 @@ class TestFunctional(unittest.TestCase):
                             result.stdout + result.stderr))
         return result
 
-    def _DoBinman(self, *args):
+    def _DoBinman(self, *argv):
         """Run binman using directly (in the same process)
 
         Args:
@@ -215,16 +215,16 @@ class TestFunctional(unittest.TestCase):
         Returns:
             Return value (0 for success)
         """
-        args = list(args)
+        argv = list(argv)
         if '-D' in sys.argv:
-            args = args + ['-D']
-        (options, args) = cmdline.ParseArgs(args)
-        options.pager = 'binman-invalid-pager'
-        options.build_dir = self._indir
+            argv = argv + ['-D']
+        args = cmdline.ParseArgs(argv)
+        args.pager = 'binman-invalid-pager'
+        args.build_dir = self._indir
 
         # For testing, you can force an increase in verbosity here
-        # options.verbosity = tout.DEBUG
-        return control.Binman(options, args)
+        # args.verbosity = tout.DEBUG
+        return control.Binman(args)
 
     def _DoTestFile(self, fname, debug=False, map=False, update_dtb=False,
                     entry_args=None, images=None, use_real_dtb=False,
@@ -242,7 +242,7 @@ class TestFunctional(unittest.TestCase):
                 value: value of that arg
             images: List of image names to build
         """
-        args = ['-p', '-I', self._indir, '-d', self.TestFile(fname)]
+        args = ['build', '-p', '-I', self._indir, '-d', self.TestFile(fname)]
         if debug:
             args.append('-D')
         if map:
@@ -515,20 +515,20 @@ class TestFunctional(unittest.TestCase):
         """Test that we can run it with a specific board"""
         self._SetupDtb('005_simple.dts', 'sandbox/u-boot.dtb')
         TestFunctional._MakeInputFile('sandbox/u-boot.bin', U_BOOT_DATA)
-        result = self._DoBinman('-b', 'sandbox')
+        result = self._DoBinman('build', '-b', 'sandbox')
         self.assertEqual(0, result)
 
     def testNeedBoard(self):
         """Test that we get an error when no board ius supplied"""
         with self.assertRaises(ValueError) as e:
-            result = self._DoBinman()
+            result = self._DoBinman('build')
         self.assertIn("Must provide a board to process (use -b <board>)",
                 str(e.exception))
 
     def testMissingDt(self):
         """Test that an invalid device-tree file generates an error"""
         with self.assertRaises(Exception) as e:
-            self._RunBinman('-d', 'missing_file')
+            self._RunBinman('build', '-d', 'missing_file')
         # We get one error from libfdt, and a different one from fdtget.
         self.AssertInList(["Couldn't open blob from 'missing_file'",
                            'No such file or directory'], str(e.exception))
@@ -540,26 +540,26 @@ class TestFunctional(unittest.TestCase):
         will come from the device-tree compiler (dtc).
         """
         with self.assertRaises(Exception) as e:
-            self._RunBinman('-d', self.TestFile('001_invalid.dts'))
+            self._RunBinman('build', '-d', self.TestFile('001_invalid.dts'))
         self.assertIn("FATAL ERROR: Unable to parse input tree",
                 str(e.exception))
 
     def testMissingNode(self):
         """Test that a device tree without a 'binman' node generates an error"""
         with self.assertRaises(Exception) as e:
-            self._DoBinman('-d', self.TestFile('002_missing_node.dts'))
+            self._DoBinman('build', '-d', self.TestFile('002_missing_node.dts'))
         self.assertIn("does not have a 'binman' node", str(e.exception))
 
     def testEmpty(self):
         """Test that an empty binman node works OK (i.e. does nothing)"""
-        result = self._RunBinman('-d', self.TestFile('003_empty.dts'))
+        result = self._RunBinman('build', '-d', self.TestFile('003_empty.dts'))
         self.assertEqual(0, len(result.stderr))
         self.assertEqual(0, result.return_code)
 
     def testInvalidEntry(self):
         """Test that an invalid entry is flagged"""
         with self.assertRaises(Exception) as e:
-            result = self._RunBinman('-d',
+            result = self._RunBinman('build', '-d',
                                      self.TestFile('004_invalid_entry.dts'))
         self.assertIn("Unknown entry type 'not-a-valid-type' in node "
                 "'/binman/not-a-valid-type'", str(e.exception))
@@ -1290,7 +1290,8 @@ class TestFunctional(unittest.TestCase):
 
     def testEntryArgsInvalidFormat(self):
         """Test that an invalid entry-argument format is detected"""
-        args = ['-d', self.TestFile('064_entry_args_required.dts'), '-ano-value']
+        args = ['build', '-d', self.TestFile('064_entry_args_required.dts'),
+                '-ano-value']
         with self.assertRaises(ValueError) as e:
             self._DoBinman(*args)
         self.assertIn("Invalid entry arguemnt 'no-value'", str(e.exception))
diff --git a/tools/patman/test_util.py b/tools/patman/test_util.py
index ea36cd16339..40098159c08 100644
--- a/tools/patman/test_util.py
+++ b/tools/patman/test_util.py
@@ -46,9 +46,10 @@ def RunTestCoverage(prog, filter_fname, exclude_list, build_dir, required=None):
         glob_list = []
     glob_list += exclude_list
     glob_list += ['*libfdt.py', '*site-packages*', '*dist-packages*']
+    test_cmd = 'test' if 'binman.py' in prog else '-t'
     cmd = ('PYTHONPATH=$PYTHONPATH:%s/sandbox_spl/tools %s-coverage run '
-           '--omit "%s" %s -P1 -t' % (build_dir, PYTHON, ','.join(glob_list),
-                                      prog))
+           '--omit "%s" %s %s -P1' % (build_dir, PYTHON, ','.join(glob_list),
+                                      prog, test_cmd))
     os.system(cmd)
     stdout = command.Output('%s-coverage' % PYTHON, 'report')
     lines = stdout.splitlines()
-- 
2.22.0.410.gd8fdbe21b5-goog

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [U-Boot] [PATCH 08/26] binman: Move compression into the Entry base class
  2019-07-02  0:24 [U-Boot] [PATCH 00/26] binman: Allow reading of images to list contents Simon Glass
                   ` (6 preceding siblings ...)
  2019-07-02  0:24 ` [U-Boot] [PATCH 07/26] binman: Convert to use ArgumentParser Simon Glass
@ 2019-07-02  0:24 ` Simon Glass
  2019-07-02  0:24 ` [U-Boot] [PATCH 09/26] binman: Drop an unused arg in Entry.Lookup() Simon Glass
                   ` (17 subsequent siblings)
  25 siblings, 0 replies; 27+ messages in thread
From: Simon Glass @ 2019-07-02  0:24 UTC (permalink / raw)
  To: u-boot

Compression is currently available only with blobs. However we want to
report the compression algorithm and uncompressed size for all entries,
so that other entry types can support compression. This will help with
the forthcoming 'list' feature which lists entries in the image.

Move the compression properties into the base class. Also fix up the docs
which had the wrong property name.

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

 tools/binman/README        | 11 ++++++++---
 tools/binman/entry.py      |  9 +++++++++
 tools/binman/etype/blob.py | 19 ++++---------------
 3 files changed, 21 insertions(+), 18 deletions(-)

diff --git a/tools/binman/README b/tools/binman/README
index 99b4e5f4504..150d94c65b8 100644
--- a/tools/binman/README
+++ b/tools/binman/README
@@ -339,6 +339,10 @@ expand-size:
 	limited by the size of the image/section and the position of the next
 	entry.
 
+compress:
+	Sets the compression algortihm to use (for blobs only). See the entry
+	documentation for details.
+
 The attributes supported for images and sections are described below. Several
 are similar to those for entries.
 
@@ -649,15 +653,16 @@ Compression
 -----------
 
 Binman support compression for 'blob' entries (those of type 'blob' and
-derivatives). To enable this for an entry, add a 'compression' property:
+derivatives). To enable this for an entry, add a 'compress' property:
 
     blob {
         filename = "datafile";
-        compression = "lz4";
+        compress = "lz4";
     };
 
 The entry will then contain the compressed data, using the 'lz4' compression
-algorithm. Currently this is the only one that is supported.
+algorithm. Currently this is the only one that is supported. The uncompressed
+size is written to the node in an 'uncomp-size' property, if -u is used.
 
 
 
diff --git a/tools/binman/entry.py b/tools/binman/entry.py
index e1cd0d3a882..8cccc2ed5f0 100644
--- a/tools/binman/entry.py
+++ b/tools/binman/entry.py
@@ -51,6 +51,8 @@ class Entry(object):
         offset: Offset of entry within the section, None if not known yet (in
             which case it will be calculated by Pack())
         size: Entry size in bytes, None if not known
+        uncomp_size: Size of uncompressed data in bytes, if the entry is
+            compressed, else None
         contents_size: Size of contents in bytes, 0 by default
         align: Entry start offset alignment, or None
         align_size: Entry size alignment, or None
@@ -58,6 +60,7 @@ class Entry(object):
         pad_before: Number of pad bytes before the contents, 0 if none
         pad_after: Number of pad bytes after the contents, 0 if none
         data: Contents of entry (string of bytes)
+        compress: Compression algoithm used (e.g. 'lz4'), 'none' if none
     """
     def __init__(self, section, etype, node, read_node=True, name_prefix=''):
         self.section = section
@@ -66,6 +69,7 @@ class Entry(object):
         self.name = node and (name_prefix + node.name) or 'none'
         self.offset = None
         self.size = None
+        self.uncomp_size = None
         self.data = None
         self.contents_size = 0
         self.align = None
@@ -76,6 +80,7 @@ class Entry(object):
         self.offset_unset = False
         self.image_pos = None
         self._expand_size = False
+        self.compress = 'none'
         if read_node:
             self.ReadNode()
 
@@ -188,6 +193,8 @@ class Entry(object):
         for prop in ['offset', 'size', 'image-pos']:
             if not prop in self._node.props:
                 state.AddZeroProp(self._node, prop)
+        if self.compress != 'none':
+            state.AddZeroProp(self._node, 'uncomp-size')
         err = state.CheckAddHashProp(self._node)
         if err:
             self.Raise(err)
@@ -198,6 +205,8 @@ class Entry(object):
         state.SetInt(self._node, 'size', self.size)
         state.SetInt(self._node, 'image-pos',
                        self.image_pos - self.section.GetRootSkipAtStart())
+        if self.uncomp_size is not None:
+            state.SetInt(self._node, 'uncomp-size', self.uncomp_size)
         state.CheckSetHashValue(self._node, self.GetData)
 
     def ProcessFdt(self, fdt):
diff --git a/tools/binman/etype/blob.py b/tools/binman/etype/blob.py
index a91e7847009..ec94568ac0a 100644
--- a/tools/binman/etype/blob.py
+++ b/tools/binman/etype/blob.py
@@ -33,8 +33,7 @@ class Entry_blob(Entry):
     def __init__(self, section, etype, node):
         Entry.__init__(self, section, etype, node)
         self._filename = fdt_util.GetString(self._node, 'filename', self.etype)
-        self._compress = fdt_util.GetString(self._node, 'compress', 'none')
-        self._uncompressed_size = None
+        self.compress = fdt_util.GetString(self._node, 'compress', 'none')
 
     def ObtainContents(self):
         self._filename = self.GetDefaultFilename()
@@ -50,21 +49,11 @@ class Entry_blob(Entry):
         # the data in chunks and avoid reading it all at once. For now
         # this seems like an unnecessary complication.
         indata = tools.ReadFile(self._pathname)
-        if self._compress != 'none':
-            self._uncompressed_size = len(indata)
-        data = tools.Compress(indata, self._compress)
+        if self.compress != 'none':
+            self.uncomp_size = len(indata)
+        data = tools.Compress(indata, self.compress)
         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)
-- 
2.22.0.410.gd8fdbe21b5-goog

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [U-Boot] [PATCH 09/26] binman: Drop an unused arg in Entry.Lookup()
  2019-07-02  0:24 [U-Boot] [PATCH 00/26] binman: Allow reading of images to list contents Simon Glass
                   ` (7 preceding siblings ...)
  2019-07-02  0:24 ` [U-Boot] [PATCH 08/26] binman: Move compression into the Entry base class Simon Glass
@ 2019-07-02  0:24 ` Simon Glass
  2019-07-02  0:24 ` [U-Boot] [PATCH 10/26] binman: Allow easy importing of entry modules Simon Glass
                   ` (16 subsequent siblings)
  25 siblings, 0 replies; 27+ messages in thread
From: Simon Glass @ 2019-07-02  0:24 UTC (permalink / raw)
  To: u-boot

The first argument is not used. Remove it.

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

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

diff --git a/tools/binman/entry.py b/tools/binman/entry.py
index 8cccc2ed5f0..00bb1d190a9 100644
--- a/tools/binman/entry.py
+++ b/tools/binman/entry.py
@@ -85,11 +85,10 @@ class Entry(object):
             self.ReadNode()
 
     @staticmethod
-    def Lookup(section, node_path, etype):
+    def Lookup(node_path, etype):
         """Look up the entry class for a node.
 
         Args:
-            section:   Section object containing this node
             node_node: Path name of Node object containing information about
                        the entry to create (used for errors)
             etype:   Entry type to use
@@ -140,7 +139,7 @@ class Entry(object):
         """
         if not etype:
             etype = fdt_util.GetString(node, 'type', node.name)
-        obj = Entry.Lookup(section, node.path, etype)
+        obj = Entry.Lookup(node.path, etype)
 
         # Call its constructor to get the object we want.
         return obj(section, etype, node)
@@ -514,7 +513,7 @@ features to produce new behaviours.
             modules.remove('_testing')
         missing = []
         for name in modules:
-            module = Entry.Lookup(name, name, name)
+            module = Entry.Lookup(name, name)
             docs = getattr(module, '__doc__')
             if test_missing == name:
                 docs = None
-- 
2.22.0.410.gd8fdbe21b5-goog

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [U-Boot] [PATCH 10/26] binman: Allow easy importing of entry modules
  2019-07-02  0:24 [U-Boot] [PATCH 00/26] binman: Allow reading of images to list contents Simon Glass
                   ` (8 preceding siblings ...)
  2019-07-02  0:24 ` [U-Boot] [PATCH 09/26] binman: Drop an unused arg in Entry.Lookup() Simon Glass
@ 2019-07-02  0:24 ` Simon Glass
  2019-07-02  0:24 ` [U-Boot] [PATCH 11/26] binman: Fix up ProcessUpdateContents error and comments Simon Glass
                   ` (15 subsequent siblings)
  25 siblings, 0 replies; 27+ messages in thread
From: Simon Glass @ 2019-07-02  0:24 UTC (permalink / raw)
  To: u-boot

At present entry modules can only be accessed using Entry.Lookup() or
Entry.Create(). Most of the time this is fine, but sometimes a module
needs to provide constants or helper functions useful to other modules.
It is easier in this case to use 'import'.

Add an __init__ file to permit this.

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

 tools/binman/entry.py          | 2 ++
 tools/binman/etype/__init__.py | 0
 tools/patman/test_util.py      | 1 +
 3 files changed, 3 insertions(+)
 create mode 100644 tools/binman/etype/__init__.py

diff --git a/tools/binman/entry.py b/tools/binman/entry.py
index 00bb1d190a9..a04e149d96c 100644
--- a/tools/binman/entry.py
+++ b/tools/binman/entry.py
@@ -513,6 +513,8 @@ features to produce new behaviours.
             modules.remove('_testing')
         missing = []
         for name in modules:
+            if name.startswith('__'):
+                continue
             module = Entry.Lookup(name, name)
             docs = getattr(module, '__doc__')
             if test_missing == name:
diff --git a/tools/binman/etype/__init__.py b/tools/binman/etype/__init__.py
new file mode 100644
index 00000000000..e69de29bb2d
diff --git a/tools/patman/test_util.py b/tools/patman/test_util.py
index 40098159c08..09f258c26b4 100644
--- a/tools/patman/test_util.py
+++ b/tools/patman/test_util.py
@@ -58,6 +58,7 @@ def RunTestCoverage(prog, filter_fname, exclude_list, build_dir, required=None):
         test_set = set([os.path.splitext(os.path.basename(line.split()[0]))[0]
                         for line in lines if '/etype/' in line])
         missing_list = required
+        missing_list.discard('__init__')
         missing_list.difference_update(test_set)
         if missing_list:
             print('Missing tests for %s' % (', '.join(missing_list)))
-- 
2.22.0.410.gd8fdbe21b5-goog

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [U-Boot] [PATCH 11/26] binman: Fix up ProcessUpdateContents error and comments
  2019-07-02  0:24 [U-Boot] [PATCH 00/26] binman: Allow reading of images to list contents Simon Glass
                   ` (9 preceding siblings ...)
  2019-07-02  0:24 ` [U-Boot] [PATCH 10/26] binman: Allow easy importing of entry modules Simon Glass
@ 2019-07-02  0:24 ` Simon Glass
  2019-07-02  0:24 ` [U-Boot] [PATCH 12/26] binman: Call ProcessUpdateContents() consistently Simon Glass
                   ` (14 subsequent siblings)
  25 siblings, 0 replies; 27+ messages in thread
From: Simon Glass @ 2019-07-02  0:24 UTC (permalink / raw)
  To: u-boot

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

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

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

diff --git a/tools/binman/entry.py b/tools/binman/entry.py
index a04e149d96c..b19a3b026f2 100644
--- a/tools/binman/entry.py
+++ b/tools/binman/entry.py
@@ -237,25 +237,25 @@ class Entry(object):
         This sets both the data and content_size properties
 
         Args:
-            data: Data to set to the contents (string)
+            data: Data to set to the contents (bytes)
         """
         self.data = data
         self.contents_size = len(self.data)
 
     def ProcessContentsUpdate(self, data):
-        """Update the contens of an entry, after the size is fixed
+        """Update the contents of an entry, after the size is fixed
 
         This checks that the new data is the same size as the old.
 
         Args:
-            data: Data to set to the contents (string)
+            data: Data to set to the contents (bytes)
 
         Raises:
             ValueError if the new data size is not the same as the old
         """
         if len(data) != self.contents_size:
             self.Raise('Cannot update entry size from %d to %d' %
-                       (len(data), self.contents_size))
+                       (self.contents_size, len(data)))
         self.SetContents(data)
 
     def ObtainContents(self):
diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
index f1a79baa905..8c9942982ba 100644
--- a/tools/binman/ftest.py
+++ b/tools/binman/ftest.py
@@ -1213,7 +1213,7 @@ class TestFunctional(unittest.TestCase):
         with self.assertRaises(ValueError) as e:
             self._DoReadFile('059_change_size.dts', True)
         self.assertIn("Node '/binman/_testing': Cannot update entry size from "
-                      '2 to 1', str(e.exception))
+                      '1 to 2', str(e.exception))
 
     def testUpdateFdt(self):
         """Test that we can update the device tree with offset/size info"""
-- 
2.22.0.410.gd8fdbe21b5-goog

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [U-Boot] [PATCH 12/26] binman: Call ProcessUpdateContents() consistently
  2019-07-02  0:24 [U-Boot] [PATCH 00/26] binman: Allow reading of images to list contents Simon Glass
                   ` (10 preceding siblings ...)
  2019-07-02  0:24 ` [U-Boot] [PATCH 11/26] binman: Fix up ProcessUpdateContents error and comments Simon Glass
@ 2019-07-02  0:24 ` Simon Glass
  2019-07-02  0:24 ` [U-Boot] [PATCH 13/26] binman: Add a return value to ProcessContentsUpdate() Simon Glass
                   ` (13 subsequent siblings)
  25 siblings, 0 replies; 27+ messages in thread
From: Simon Glass @ 2019-07-02  0:24 UTC (permalink / raw)
  To: u-boot

SetContents() should only be called to set the contents of an entry from
within the ObtainContents() call, since it has no guard against increasing
the size of the contents, thus triggering incorrect operation.

Change all such calls to use ProcessUpdateContents() instead.

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

 tools/binman/etype/blob_dtb.py     | 2 +-
 tools/binman/etype/fdtmap.py       | 2 +-
 tools/binman/etype/fmap.py         | 2 +-
 tools/binman/etype/image_header.py | 2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/tools/binman/etype/blob_dtb.py b/tools/binman/etype/blob_dtb.py
index cc5b4a3f760..d80c3d7e006 100644
--- a/tools/binman/etype/blob_dtb.py
+++ b/tools/binman/etype/blob_dtb.py
@@ -30,4 +30,4 @@ class Entry_blob_dtb(Entry_blob):
     def ProcessContents(self):
         """Re-read the DTB contents so that we get any calculated properties"""
         _, data = state.GetFdtContents(self._filename)
-        self.SetContents(data)
+        self.ProcessContentsUpdate(data)
diff --git a/tools/binman/etype/fdtmap.py b/tools/binman/etype/fdtmap.py
index cdeb491ebdc..ddac148b9ba 100644
--- a/tools/binman/etype/fdtmap.py
+++ b/tools/binman/etype/fdtmap.py
@@ -106,4 +106,4 @@ class Entry_fdtmap(Entry):
         This is necessary since new data may have been written back to it during
         processing, e.g. the image-pos properties.
         """
-        self.SetContents(self._GetFdtmap())
+        self.ProcessContentsUpdate(self._GetFdtmap())
diff --git a/tools/binman/etype/fmap.py b/tools/binman/etype/fmap.py
index e6b5c5c74c0..45d6db18a31 100644
--- a/tools/binman/etype/fmap.py
+++ b/tools/binman/etype/fmap.py
@@ -62,4 +62,4 @@ class Entry_fmap(Entry):
         return True
 
     def ProcessContents(self):
-        self.SetContents(self._GetFmap())
+        self.ProcessContentsUpdate(self._GetFmap())
diff --git a/tools/binman/etype/image_header.py b/tools/binman/etype/image_header.py
index 9bc84ec01d4..d6de58ce4b7 100644
--- a/tools/binman/etype/image_header.py
+++ b/tools/binman/etype/image_header.py
@@ -73,4 +73,4 @@ class Entry_image_header(Entry):
         This is necessary since image_pos is not available when ObtainContents()
         is called, since by then the entries have not been packed in the image.
         """
-        self.SetContents(self._GetHeader())
+        self.ProcessContentsUpdate(self._GetHeader())
-- 
2.22.0.410.gd8fdbe21b5-goog

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [U-Boot] [PATCH 13/26] binman: Add a return value to ProcessContentsUpdate()
  2019-07-02  0:24 [U-Boot] [PATCH 00/26] binman: Allow reading of images to list contents Simon Glass
                   ` (11 preceding siblings ...)
  2019-07-02  0:24 ` [U-Boot] [PATCH 12/26] binman: Call ProcessUpdateContents() consistently Simon Glass
@ 2019-07-02  0:24 ` Simon Glass
  2019-07-02  0:24 ` [U-Boot] [PATCH 14/26] binman: Add a control for post-pack entry expansion Simon Glass
                   ` (12 subsequent siblings)
  25 siblings, 0 replies; 27+ messages in thread
From: Simon Glass @ 2019-07-02  0:24 UTC (permalink / raw)
  To: u-boot

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

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

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

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

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

diff --git a/tools/binman/bsection.py b/tools/binman/bsection.py
index 3e3d369d5e4..f49a6e93bc7 100644
--- a/tools/binman/bsection.py
+++ b/tools/binman/bsection.py
@@ -317,9 +317,15 @@ class Section(object):
         """Call the ProcessContents() method for each entry
 
         This is intended to adjust the contents as needed by the entry type.
+
+        Returns:
+            True if no entries needed to change their size
         """
+        sizes_ok = True
         for entry in self._entries.values():
-            entry.ProcessContents()
+            if not entry.ProcessContents():
+                sizes_ok = False
+        return sizes_ok
 
     def WriteSymbols(self):
         """Write symbol values into binary files for access at run time"""
diff --git a/tools/binman/entry.py b/tools/binman/entry.py
index b19a3b026f2..7db1402b84f 100644
--- a/tools/binman/entry.py
+++ b/tools/binman/entry.py
@@ -245,7 +245,8 @@ class Entry(object):
     def ProcessContentsUpdate(self, data):
         """Update the contents of an entry, after the size is fixed
 
-        This checks that the new data is the same size as the old.
+        This checks that the new data is the same size as the old. If the size
+        has changed, this triggers a re-run of the packing algorithm.
 
         Args:
             data: Data to set to the contents (bytes)
@@ -253,10 +254,12 @@ class Entry(object):
         Raises:
             ValueError if the new data size is not the same as the old
         """
+        size_ok = True
         if len(data) != self.contents_size:
             self.Raise('Cannot update entry size from %d to %d' %
                        (self.contents_size, len(data)))
         self.SetContents(data)
+        return size_ok
 
     def ObtainContents(self):
         """Figure out the contents of an entry.
@@ -401,7 +404,22 @@ class Entry(object):
         self.image_pos = image_pos + self.offset
 
     def ProcessContents(self):
-        pass
+        """Do any post-packing updates of entry contents
+
+        This function should call ProcessContentsUpdate() to update the entry
+        contents, if necessary, returning its return value here.
+
+        Args:
+            data: Data to set to the contents (bytes)
+
+        Returns:
+            True if the new data size is OK, False if expansion is needed
+
+        Raises:
+            ValueError if the new data size is not the same as the old and
+                state.AllowEntryExpansion() is False
+        """
+        return True
 
     def WriteSymbols(self, section):
         """Write symbol values into binary files for access at run time
diff --git a/tools/binman/etype/_testing.py b/tools/binman/etype/_testing.py
index ac62d2e2005..2204362281c 100644
--- a/tools/binman/etype/_testing.py
+++ b/tools/binman/etype/_testing.py
@@ -88,9 +88,10 @@ class Entry__testing(Entry):
 
     def ProcessContents(self):
         if self.bad_update_contents:
-            # Request to update the conents with something larger, to cause a
+            # Request to update the contents with something larger, to cause a
             # failure.
-            self.ProcessContentsUpdate('aa')
+            return self.ProcessContentsUpdate('aa')
+        return True
 
     def ProcessFdt(self, fdt):
         """Force reprocessing the first time"""
diff --git a/tools/binman/etype/blob_dtb.py b/tools/binman/etype/blob_dtb.py
index d80c3d7e006..09d5d727138 100644
--- a/tools/binman/etype/blob_dtb.py
+++ b/tools/binman/etype/blob_dtb.py
@@ -30,4 +30,4 @@ class Entry_blob_dtb(Entry_blob):
     def ProcessContents(self):
         """Re-read the DTB contents so that we get any calculated properties"""
         _, data = state.GetFdtContents(self._filename)
-        self.ProcessContentsUpdate(data)
+        return self.ProcessContentsUpdate(data)
diff --git a/tools/binman/etype/fdtmap.py b/tools/binman/etype/fdtmap.py
index ddac148b9ba..bfd7962be3a 100644
--- a/tools/binman/etype/fdtmap.py
+++ b/tools/binman/etype/fdtmap.py
@@ -106,4 +106,4 @@ class Entry_fdtmap(Entry):
         This is necessary since new data may have been written back to it during
         processing, e.g. the image-pos properties.
         """
-        self.ProcessContentsUpdate(self._GetFdtmap())
+        return self.ProcessContentsUpdate(self._GetFdtmap())
diff --git a/tools/binman/etype/fmap.py b/tools/binman/etype/fmap.py
index 45d6db18a31..3a809486098 100644
--- a/tools/binman/etype/fmap.py
+++ b/tools/binman/etype/fmap.py
@@ -62,4 +62,4 @@ class Entry_fmap(Entry):
         return True
 
     def ProcessContents(self):
-        self.ProcessContentsUpdate(self._GetFmap())
+        return self.ProcessContentsUpdate(self._GetFmap())
diff --git a/tools/binman/etype/image_header.py b/tools/binman/etype/image_header.py
index d6de58ce4b7..b1c4f8a07e9 100644
--- a/tools/binman/etype/image_header.py
+++ b/tools/binman/etype/image_header.py
@@ -73,4 +73,4 @@ class Entry_image_header(Entry):
         This is necessary since image_pos is not available when ObtainContents()
         is called, since by then the entries have not been packed in the image.
         """
-        self.ProcessContentsUpdate(self._GetHeader())
+        return self.ProcessContentsUpdate(self._GetHeader())
diff --git a/tools/binman/etype/section.py b/tools/binman/etype/section.py
index 3681a484689..51eddcd995a 100644
--- a/tools/binman/etype/section.py
+++ b/tools/binman/etype/section.py
@@ -85,8 +85,9 @@ class Entry_section(Entry):
         self._section.SetCalculatedProperties()
 
     def ProcessContents(self):
-        self._section.ProcessEntryContents()
-        super(Entry_section, self).ProcessContents()
+        sizes_ok = self._section.ProcessEntryContents()
+        sizes_ok_base = super(Entry_section, self).ProcessContents()
+        return sizes_ok and sizes_ok_base
 
     def CheckOffset(self):
         self._section.CheckEntries()
diff --git a/tools/binman/etype/u_boot_with_ucode_ptr.py b/tools/binman/etype/u_boot_with_ucode_ptr.py
index da0e12417b5..4104bf8bf13 100644
--- a/tools/binman/etype/u_boot_with_ucode_ptr.py
+++ b/tools/binman/etype/u_boot_with_ucode_ptr.py
@@ -91,6 +91,6 @@ class Entry_u_boot_with_ucode_ptr(Entry_blob):
         # Write the microcode offset and size into the entry
         offset_and_size = struct.pack('<2L', offset, size)
         self.target_offset -= self.image_pos
-        self.ProcessContentsUpdate(self.data[:self.target_offset] +
-                                   offset_and_size +
-                                   self.data[self.target_offset + 8:])
+        return self.ProcessContentsUpdate(self.data[:self.target_offset] +
+                                          offset_and_size +
+                                          self.data[self.target_offset + 8:])
diff --git a/tools/binman/image.py b/tools/binman/image.py
index f237ae302df..c8bce394aa1 100644
--- a/tools/binman/image.py
+++ b/tools/binman/image.py
@@ -122,8 +122,11 @@ class Image:
         """Call the ProcessContents() method for each entry
 
         This is intended to adjust the contents as needed by the entry type.
+
+        Returns:
+            True if the new data size is OK, False if expansion is needed
         """
-        self._section.ProcessEntryContents()
+        return self._section.ProcessEntryContents()
 
     def WriteSymbols(self):
         """Write symbol values into binary files for access@run time"""
-- 
2.22.0.410.gd8fdbe21b5-goog

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [U-Boot] [PATCH 14/26] binman: Add a control for post-pack entry expansion
  2019-07-02  0:24 [U-Boot] [PATCH 00/26] binman: Allow reading of images to list contents Simon Glass
                   ` (12 preceding siblings ...)
  2019-07-02  0:24 ` [U-Boot] [PATCH 13/26] binman: Add a return value to ProcessContentsUpdate() Simon Glass
@ 2019-07-02  0:24 ` Simon Glass
  2019-07-02  0:24 ` [U-Boot] [PATCH 15/26] binman: Allow entries to expand after packing Simon Glass
                   ` (11 subsequent siblings)
  25 siblings, 0 replies; 27+ messages in thread
From: Simon Glass @ 2019-07-02  0:24 UTC (permalink / raw)
  To: u-boot

We plan to support changing the size of entries after they have been
packed. For now it will always be enabled. But to aid testing of both
cases (in the event that we want to add a command-line flag, for example),
add a setting to control it.

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

 tools/binman/state.py | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/tools/binman/state.py b/tools/binman/state.py
index 3ccd7855d47..382bda32219 100644
--- a/tools/binman/state.py
+++ b/tools/binman/state.py
@@ -31,6 +31,11 @@ fdt_subset = set()
 # The DTB which contains the full image information
 main_dtb = None
 
+# Allow entries to expand after they have been packed. This is detected and
+# forces a re-pack. If not allowed, any attempted expansion causes an error in
+# Entry.ProcessContentsUpdate()
+allow_entry_expansion = True
+
 def GetFdt(fname):
     """Get the Fdt object for a particular device-tree filename
 
@@ -250,3 +255,22 @@ def CheckSetHashValue(node, get_data_func):
             data = m.digest()
         for n in GetUpdateNodes(hash_node):
             n.SetData('value', data)
+
+def SetAllowEntryExpansion(allow):
+    """Set whether post-pack expansion of entries is allowed
+
+    Args:
+       allow: True to allow expansion, False to raise an exception
+    """
+    global allow_entry_expansion
+
+    allow_entry_expansion = allow
+
+def AllowEntryExpansion():
+    """Check whether post-pack expansion of entries is allowed
+
+    Returns:
+        True if expansion should be allowed, False if an exception should be
+            raised
+    """
+    return allow_entry_expansion
-- 
2.22.0.410.gd8fdbe21b5-goog

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [U-Boot] [PATCH 15/26] binman: Allow entries to expand after packing
  2019-07-02  0:24 [U-Boot] [PATCH 00/26] binman: Allow reading of images to list contents Simon Glass
                   ` (13 preceding siblings ...)
  2019-07-02  0:24 ` [U-Boot] [PATCH 14/26] binman: Add a control for post-pack entry expansion Simon Glass
@ 2019-07-02  0:24 ` Simon Glass
  2019-07-02  0:24 ` [U-Boot] [PATCH 16/26] binman: Allow device-tree entries to be compressed Simon Glass
                   ` (10 subsequent siblings)
  25 siblings, 0 replies; 27+ messages in thread
From: Simon Glass @ 2019-07-02  0:24 UTC (permalink / raw)
  To: u-boot

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

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

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

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

diff --git a/tools/binman/README b/tools/binman/README
index 150d94c65b8..7e745a2d466 100644
--- a/tools/binman/README
+++ b/tools/binman/README
@@ -566,7 +566,8 @@ tree. This sets the correct 'offset' and 'size' vaues, for example.
 The default implementatoin does nothing. This can be overriden to adjust the
 contents of an entry in some way. For example, it would be possible to create
 an entry containing a hash of the contents of some other entries. At this
-stage the offset and size of entries should not be adjusted.
+stage the offset and size of entries should not be adjusted unless absolutely
+necessary, since it requires a repack (going back to PackEntries()).
 
 10. WriteSymbols() - write the value of symbols into the U-Boot SPL binary.
 See 'Access to binman entry offsets at run time' below for a description of
diff --git a/tools/binman/bsection.py b/tools/binman/bsection.py
index f49a6e93bc7..d368e3f6baa 100644
--- a/tools/binman/bsection.py
+++ b/tools/binman/bsection.py
@@ -45,6 +45,8 @@ class Section(object):
         _name_prefix: Prefix to add to the name of all entries within this
             section
         _entries: OrderedDict() of entries
+        _orig_offset: Original offset value read from node
+        _orig_size: Original size value read from node
     """
     def __init__(self, name, parent_section, node, image, test=False):
         global entry
@@ -76,6 +78,8 @@ class Section(object):
         """Read properties from the section node"""
         self._offset = fdt_util.GetInt(self._node, 'offset')
         self._size = fdt_util.GetInt(self._node, 'size')
+        self._orig_offset = self._offset
+        self._orig_size = self._size
         self._align_size = fdt_util.GetInt(self._node, 'align-size')
         if tools.NotPowerOfTwo(self._align_size):
             self._Raise("Alignment size %s must be a power of two" %
@@ -257,6 +261,13 @@ class Section(object):
             for name, info in offset_dict.items():
                 self._SetEntryOffsetSize(name, *info)
 
+    def ResetForPack(self):
+        """Reset offset/size fields so that packing can be done again"""
+        self._offset = self._orig_offset
+        self._size = self._orig_size
+        for entry in self._entries.values():
+            entry.ResetForPack()
+
     def PackEntries(self):
         """Pack all entries into the section"""
         offset = self._skip_at_start
diff --git a/tools/binman/control.py b/tools/binman/control.py
index 9022cf76e99..d48e7ac0070 100644
--- a/tools/binman/control.py
+++ b/tools/binman/control.py
@@ -170,21 +170,30 @@ def Binman(args):
                 # completed and written, but that does not seem important.
                 image.GetEntryContents()
                 image.GetEntryOffsets()
-                try:
-                    image.PackEntries()
-                    image.CheckSize()
-                    image.CheckEntries()
-                except Exception as e:
-                    if args.map:
-                        fname = image.WriteMap()
-                        print("Wrote map file '%s' to show errors"  % fname)
-                    raise
-                image.SetImagePos()
-                if args.update_fdt:
-                    image.SetCalculatedProperties()
-                    for dtb_item in state.GetFdts():
-                        dtb_item.Sync()
-                image.ProcessEntryContents()
+                passes = 2
+                for pack_pass in range(passes):
+                    try:
+                        image.PackEntries()
+                        image.CheckSize()
+                        image.CheckEntries()
+                    except Exception as e:
+                        if args.map:
+                            fname = image.WriteMap()
+                            print("Wrote map file '%s' to show errors"  % fname)
+                        raise
+                    image.SetImagePos()
+                    if args.update_fdt:
+                        image.SetCalculatedProperties()
+                        for dtb_item in state.GetFdts():
+                            dtb_item.Sync()
+                    sizes_ok = image.ProcessEntryContents()
+                    if sizes_ok:
+                        break
+                    image.ResetForPack()
+                if not sizes_ok:
+                    image.Raise('Entries expanded after packing (tried %s passes' %
+                                passes)
+
                 image.WriteSymbols()
                 image.BuildImage()
                 if args.map:
diff --git a/tools/binman/entry.py b/tools/binman/entry.py
index 7db1402b84f..a9a9d119e1d 100644
--- a/tools/binman/entry.py
+++ b/tools/binman/entry.py
@@ -61,6 +61,8 @@ class Entry(object):
         pad_after: Number of pad bytes after the contents, 0 if none
         data: Contents of entry (string of bytes)
         compress: Compression algoithm used (e.g. 'lz4'), 'none' if none
+        orig_offset: Original offset value read from node
+        orig_size: Original size value read from node
     """
     def __init__(self, section, etype, node, read_node=True, name_prefix=''):
         self.section = section
@@ -153,6 +155,7 @@ class Entry(object):
             self.Raise("Please use 'offset' instead of 'pos'")
         self.offset = fdt_util.GetInt(self._node, 'offset')
         self.size = fdt_util.GetInt(self._node, 'size')
+        self.orig_offset, self.orig_size = self.offset, self.size
         self.align = fdt_util.GetInt(self._node, 'align')
         if tools.NotPowerOfTwo(self.align):
             raise ValueError("Node '%s': Alignment %s must be a power of two" %
@@ -255,9 +258,16 @@ class Entry(object):
             ValueError if the new data size is not the same as the old
         """
         size_ok = True
-        if len(data) != self.contents_size:
+        new_size = len(data)
+        if state.AllowEntryExpansion():
+            if new_size > self.contents_size:
+                print("Entry '%s' size change from %#x to %#x" % (
+                    self._node.path, self.contents_size, new_size))
+                # self.data will indicate the new size needed
+                size_ok = False
+        elif new_size != self.contents_size:
             self.Raise('Cannot update entry size from %d to %d' %
-                       (self.contents_size, len(data)))
+                       (self.contents_size, new_size))
         self.SetContents(data)
         return size_ok
 
@@ -271,6 +281,10 @@ class Entry(object):
         # No contents by default: subclasses can implement this
         return True
 
+    def ResetForPack(self):
+        """Reset offset/size fields so that packing can be done again"""
+        self.offset, self.size = self.orig_offset, self.orig_size
+
     def Pack(self, offset):
         """Figure out how to pack the entry into the section
 
diff --git a/tools/binman/etype/_testing.py b/tools/binman/etype/_testing.py
index 2204362281c..ae24fe8105a 100644
--- a/tools/binman/etype/_testing.py
+++ b/tools/binman/etype/_testing.py
@@ -50,6 +50,8 @@ class Entry__testing(Entry):
                                                     'bad-update-contents')
         self.return_contents_once = fdt_util.GetBool(self._node,
                                                      'return-contents-once')
+        self.bad_update_contents_twice = fdt_util.GetBool(self._node,
+                                                    'bad-update-contents-twice')
 
         # Set to True when the entry is ready to process the FDT.
         self.process_fdt_ready = False
@@ -71,11 +73,12 @@ class Entry__testing(Entry):
         if self.force_bad_datatype:
             self.GetEntryArgsOrProps([EntryArg('test-bad-datatype-arg', bool)])
         self.return_contents = True
+        self.contents = b'a'
 
     def ObtainContents(self):
         if self.return_unknown_contents or not self.return_contents:
             return False
-        self.data = b'a'
+        self.data = self.contents
         self.contents_size = len(self.data)
         if self.return_contents_once:
             self.return_contents = False
@@ -90,7 +93,11 @@ class Entry__testing(Entry):
         if self.bad_update_contents:
             # Request to update the contents with something larger, to cause a
             # failure.
-            return self.ProcessContentsUpdate('aa')
+            if self.bad_update_contents_twice:
+                self.contents += b'a'
+            else:
+                self.contents = b'aa'
+            return self.ProcessContentsUpdate(self.contents)
         return True
 
     def ProcessFdt(self, fdt):
diff --git a/tools/binman/etype/section.py b/tools/binman/etype/section.py
index 51eddcd995a..23bf22113d4 100644
--- a/tools/binman/etype/section.py
+++ b/tools/binman/etype/section.py
@@ -64,6 +64,11 @@ class Entry_section(Entry):
         self._section.GetEntryOffsets()
         return {}
 
+    def ResetForPack(self):
+        """Reset offset/size fields so that packing can be done again"""
+        self._section.ResetForPack()
+        Entry.ResetForPack(self)
+
     def Pack(self, offset):
         """Pack all entries into the section"""
         self._section.PackEntries()
diff --git a/tools/binman/etype/u_boot_with_ucode_ptr.py b/tools/binman/etype/u_boot_with_ucode_ptr.py
index 4104bf8bf13..cb7dbc68dbb 100644
--- a/tools/binman/etype/u_boot_with_ucode_ptr.py
+++ b/tools/binman/etype/u_boot_with_ucode_ptr.py
@@ -49,7 +49,7 @@ class Entry_u_boot_with_ucode_ptr(Entry_blob):
     def ProcessContents(self):
         # If the image does not need microcode, there is nothing to do
         if not self.target_offset:
-            return
+            return True
 
         # Get the offset of the microcode
         ucode_entry = self.section.FindEntryType('u-boot-ucode')
diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
index 8c9942982ba..0ff8b5e2de8 100644
--- a/tools/binman/ftest.py
+++ b/tools/binman/ftest.py
@@ -1210,10 +1210,14 @@ class TestFunctional(unittest.TestCase):
 
     def testBadChangeSize(self):
         """Test that trying to change the size of an entry fails"""
-        with self.assertRaises(ValueError) as e:
-            self._DoReadFile('059_change_size.dts', True)
-        self.assertIn("Node '/binman/_testing': Cannot update entry size from "
-                      '1 to 2', str(e.exception))
+        try:
+            state.SetAllowEntryExpansion(False)
+            with self.assertRaises(ValueError) as e:
+                self._DoReadFile('059_change_size.dts', True)
+            self.assertIn("Node '/binman/_testing': Cannot update entry size from 1 to 2",
+                          str(e.exception))
+        finally:
+            state.SetAllowEntryExpansion(True)
 
     def testUpdateFdt(self):
         """Test that we can update the device tree with offset/size info"""
@@ -2108,6 +2112,27 @@ class TestFunctional(unittest.TestCase):
         self.assertIn("Invalid location 'None', expected 'start' or 'end'",
                       str(e.exception))
 
+    def testEntryExpand(self):
+        """Test expanding an entry after it is packed"""
+        data = self._DoReadFile('121_entry_expand.dts')
+        self.assertEqual('aa', data[:2])
+        self.assertEqual(U_BOOT_DATA, data[2:2 + len(U_BOOT_DATA)])
+        self.assertEqual('aa', data[-2:])
+
+    def testEntryExpandBad(self):
+        """Test expanding an entry after it is packed, twice"""
+        with self.assertRaises(ValueError) as e:
+            self._DoReadFile('122_entry_expand_twice.dts')
+        self.assertIn("Image '/binman': Entries expanded after packing",
+                      str(e.exception))
+
+    def testEntryExpandSection(self):
+        """Test expanding an entry within a section after it is packed"""
+        data = self._DoReadFile('123_entry_expand_section.dts')
+        self.assertEqual('aa', data[:2])
+        self.assertEqual(U_BOOT_DATA, data[2:2 + len(U_BOOT_DATA)])
+        self.assertEqual('aa', data[-2:])
+
 
 if __name__ == "__main__":
     unittest.main()
diff --git a/tools/binman/image.py b/tools/binman/image.py
index c8bce394aa1..6339d020e76 100644
--- a/tools/binman/image.py
+++ b/tools/binman/image.py
@@ -55,6 +55,10 @@ class Image:
             self._filename = filename
         self._section = bsection.Section('main-section', None, self._node, self)
 
+    def Raise(self, msg):
+        """Convenience function to raise an error referencing an image"""
+        raise ValueError("Image '%s': %s" % (self._node.path, msg))
+
     def GetFdtSet(self):
         """Get the set of device tree files used by this image"""
         return self._section.GetFdtSet()
@@ -100,6 +104,10 @@ class Image:
         """
         self._section.GetEntryOffsets()
 
+    def ResetForPack(self):
+        """Reset offset/size fields so that packing can be done again"""
+        self._section.ResetForPack()
+
     def PackEntries(self):
         """Pack all entries into the image"""
         self._section.PackEntries()
diff --git a/tools/binman/test/121_entry_expand.dts b/tools/binman/test/121_entry_expand.dts
new file mode 100644
index 00000000000..ebb7816db90
--- /dev/null
+++ b/tools/binman/test/121_entry_expand.dts
@@ -0,0 +1,20 @@
+/dts-v1/;
+
+/ {
+	#address-cells = <1>;
+	#size-cells = <1>;
+
+	binman {
+		_testing {
+			bad-update-contents;
+		};
+
+		u-boot {
+		};
+
+		_testing2 {
+			type = "_testing";
+			bad-update-contents;
+		};
+	};
+};
diff --git a/tools/binman/test/122_entry_expand_twice.dts b/tools/binman/test/122_entry_expand_twice.dts
new file mode 100644
index 00000000000..258cf859f4b
--- /dev/null
+++ b/tools/binman/test/122_entry_expand_twice.dts
@@ -0,0 +1,21 @@
+/dts-v1/;
+
+/ {
+	#address-cells = <1>;
+	#size-cells = <1>;
+
+	binman {
+		_testing {
+			bad-update-contents;
+			bad-update-contents-twice;
+		};
+
+		u-boot {
+		};
+
+		_testing2 {
+			type = "_testing";
+			bad-update-contents;
+		};
+	};
+};
diff --git a/tools/binman/test/123_entry_expand_section.dts b/tools/binman/test/123_entry_expand_section.dts
new file mode 100644
index 00000000000..046f7234348
--- /dev/null
+++ b/tools/binman/test/123_entry_expand_section.dts
@@ -0,0 +1,22 @@
+/dts-v1/;
+
+/ {
+	#address-cells = <1>;
+	#size-cells = <1>;
+
+	binman {
+		_testing {
+			bad-update-contents;
+		};
+
+		u-boot {
+		};
+
+		section {
+			_testing2 {
+				type = "_testing";
+				bad-update-contents;
+			};
+		};
+	};
+};
-- 
2.22.0.410.gd8fdbe21b5-goog

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [U-Boot] [PATCH 16/26] binman: Allow device-tree entries to be compressed
  2019-07-02  0:24 [U-Boot] [PATCH 00/26] binman: Allow reading of images to list contents Simon Glass
                   ` (14 preceding siblings ...)
  2019-07-02  0:24 ` [U-Boot] [PATCH 15/26] binman: Allow entries to expand after packing Simon Glass
@ 2019-07-02  0:24 ` Simon Glass
  2019-07-02  0:24 ` [U-Boot] [PATCH 17/26] binman: Provide the actual data address for cbfs files Simon Glass
                   ` (9 subsequent siblings)
  25 siblings, 0 replies; 27+ messages in thread
From: Simon Glass @ 2019-07-02  0:24 UTC (permalink / raw)
  To: u-boot

At present the logic skips the blob class' handling of compression, so
this is not supported with device tree entries. Fix this.

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

 tools/binman/etype/blob.py             | 25 +++++++++++++++++--------
 tools/binman/etype/blob_dtb.py         |  8 ++++----
 tools/binman/ftest.py                  | 18 ++++++++++++++++++
 tools/binman/test/124_compress_dtb.dts | 14 ++++++++++++++
 4 files changed, 53 insertions(+), 12 deletions(-)
 create mode 100644 tools/binman/test/124_compress_dtb.dts

diff --git a/tools/binman/etype/blob.py b/tools/binman/etype/blob.py
index ec94568ac0a..a4ff0efcebc 100644
--- a/tools/binman/etype/blob.py
+++ b/tools/binman/etype/blob.py
@@ -41,17 +41,26 @@ class Entry_blob(Entry):
         self.ReadBlobContents()
         return True
 
-    def ReadBlobContents(self):
-        # 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.
-        indata = tools.ReadFile(self._pathname)
+    def CompressData(self, indata):
         if self.compress != 'none':
             self.uncomp_size = len(indata)
         data = tools.Compress(indata, self.compress)
+        return data
+
+    def ReadBlobContents(self):
+        """Read blob contents into memory
+
+        This function compresses the data before storing if needed.
+
+        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.
+        """
+        indata = tools.ReadFile(self._pathname)
+        data = self.CompressData(indata)
         self.SetContents(data)
         return True
 
diff --git a/tools/binman/etype/blob_dtb.py b/tools/binman/etype/blob_dtb.py
index 09d5d727138..88ed55d8865 100644
--- a/tools/binman/etype/blob_dtb.py
+++ b/tools/binman/etype/blob_dtb.py
@@ -23,11 +23,11 @@ class Entry_blob_dtb(Entry_blob):
     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
+        self._pathname, _ = state.GetFdtContents(self._filename)
+        return Entry_blob.ReadBlobContents(self)
 
     def ProcessContents(self):
         """Re-read the DTB contents so that we get any calculated properties"""
-        _, data = state.GetFdtContents(self._filename)
+        _, indata = state.GetFdtContents(self._filename)
+        data = self.CompressData(indata)
         return self.ProcessContentsUpdate(data)
diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
index 0ff8b5e2de8..b1dc1643011 100644
--- a/tools/binman/ftest.py
+++ b/tools/binman/ftest.py
@@ -2133,6 +2133,24 @@ class TestFunctional(unittest.TestCase):
         self.assertEqual(U_BOOT_DATA, data[2:2 + len(U_BOOT_DATA)])
         self.assertEqual('aa', data[-2:])
 
+    def testCompressDtb(self):
+        """Test that compress of device-tree files is supported"""
+        data = self._DoReadFileDtb('124_compress_dtb.dts', use_real_dtb=True,
+                                   update_dtb=True)[0]
+        self.assertEqual(U_BOOT_DATA, data[:len(U_BOOT_DATA)])
+        comp_data = data[len(U_BOOT_DATA):]
+        orig = self._decompress(comp_data)
+        dtb = fdt.Fdt.FromData(orig)
+        dtb.Scan()
+        props = self._GetPropTree(dtb, ['size', 'uncomp-size'])
+        expected = {
+            'u-boot:size': len(U_BOOT_DATA),
+            'u-boot-dtb:uncomp-size': len(orig),
+            'u-boot-dtb:size': len(comp_data),
+            'size': len(data),
+            }
+        self.assertEqual(expected, props)
+
 
 if __name__ == "__main__":
     unittest.main()
diff --git a/tools/binman/test/124_compress_dtb.dts b/tools/binman/test/124_compress_dtb.dts
new file mode 100644
index 00000000000..46bfd8b265f
--- /dev/null
+++ b/tools/binman/test/124_compress_dtb.dts
@@ -0,0 +1,14 @@
+/dts-v1/;
+
+/ {
+	#address-cells = <1>;
+	#size-cells = <1>;
+
+	binman {
+		u-boot {
+		};
+		u-boot-dtb {
+			compress = "lz4";
+		};
+	};
+};
-- 
2.22.0.410.gd8fdbe21b5-goog

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [U-Boot] [PATCH 17/26] binman: Provide the actual data address for cbfs files
  2019-07-02  0:24 [U-Boot] [PATCH 00/26] binman: Allow reading of images to list contents Simon Glass
                   ` (15 preceding siblings ...)
  2019-07-02  0:24 ` [U-Boot] [PATCH 16/26] binman: Allow device-tree entries to be compressed Simon Glass
@ 2019-07-02  0:24 ` Simon Glass
  2019-07-02  0:24 ` [U-Boot] [PATCH 18/26] binman: Use the cbfs memlen field only for uncompressed length Simon Glass
                   ` (8 subsequent siblings)
  25 siblings, 0 replies; 27+ messages in thread
From: Simon Glass @ 2019-07-02  0:24 UTC (permalink / raw)
  To: u-boot

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

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

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

diff --git a/tools/binman/cbfs_util.py b/tools/binman/cbfs_util.py
index 1cdbcb2339e..530629a5c96 100644
--- a/tools/binman/cbfs_util.py
+++ b/tools/binman/cbfs_util.py
@@ -185,7 +185,8 @@ class CbfsFile(object):
     """Class to represent a single CBFS file
 
     This is used to hold the information about a file, including its contents.
-    Use the get_data() method to obtain the raw output for writing to CBFS.
+    Use the get_data_and_offset() method to obtain the raw output for writing to
+    CBFS.
 
     Properties:
         name: Name of file
@@ -319,12 +320,15 @@ class CbfsFile(object):
             raise ValueError('Unknown file type %#x\n' % self.ftype)
         return hdr_len
 
-    def get_data(self, offset=None, pad_byte=None):
-        """Obtain the contents of the file, in CBFS format
+    def get_data_and_offset(self, offset=None, pad_byte=None):
+        """Obtain the contents of the file, in CBFS format and the offset of
+        the data within the file
 
         Returns:
-            bytes representing the contents of this file, packed and aligned
-                for directly inserting into the final CBFS output
+            tuple:
+                bytes representing the contents of this file, packed and aligned
+                    for directly inserting into the final CBFS output
+                offset to the file data from the start of the returned data.
         """
         name = _pack_string(self.name)
         hdr_len = len(name) + FILE_HEADER_LEN
@@ -368,8 +372,10 @@ class CbfsFile(object):
                                  (self.name, self.cbfs_offset, offset))
             pad = tools.GetBytes(pad_byte, pad_len)
             hdr_len += pad_len
-        self.offset = len(content) + len(data)
-        hdr = struct.pack(FILE_HEADER_FORMAT, FILE_MAGIC, self.offset,
+
+        # This is the offset of the start of the file's data,
+        size = len(content) + len(data)
+        hdr = struct.pack(FILE_HEADER_FORMAT, FILE_MAGIC, size,
                           self.ftype, attr_pos, hdr_len)
 
         # Do a sanity check of the get_header_len() function, to ensure that it
@@ -381,7 +387,7 @@ class CbfsFile(object):
             # happen. It probably indicates that get_header_len() is broken.
             raise ValueError("Internal error: CBFS file '%s': Expected headers of %#x bytes, got %#d" %
                              (self.name, expected_len, actual_len))
-        return hdr + name + attr + pad + content + data
+        return hdr + name + attr + pad + content + data, hdr_len
 
 
 class CbfsWriter(object):
@@ -392,7 +398,7 @@ class CbfsWriter(object):
         cbw = CbfsWriter(size)
         cbw.add_file_raw('u-boot', tools.ReadFile('u-boot.bin'))
         ...
-        data = cbw.get_data()
+        data, cbfs_offset = cbw.get_data_and_offset()
 
     Attributes:
         _master_name: Name of the file containing the master header
@@ -475,7 +481,7 @@ class CbfsWriter(object):
         todo = align_int_down(offset - upto, self._align)
         if todo:
             cbf = CbfsFile.empty(todo, self._erase_byte)
-            fd.write(cbf.get_data())
+            fd.write(cbf.get_data_and_offset()[0])
         self._skip_to(fd, offset)
 
     def _align_to(self, fd, align):
@@ -579,8 +585,11 @@ class CbfsWriter(object):
             offset = cbf.calc_start_offset()
             if offset is not None:
                 self._pad_to(fd, align_int_down(offset, self._align))
-            fd.write(cbf.get_data(fd.tell(), self._erase_byte))
+            pos = fd.tell()
+            data, data_offset = cbf.get_data_and_offset(pos, self._erase_byte)
+            fd.write(data)
             self._align_to(fd, self._align)
+            cbf.calced_cbfs_offset = pos + data_offset
         if not self._hdr_at_start:
             self._write_header(fd, add_fileheader=self._add_fileheader)
 
diff --git a/tools/binman/cbfs_util_test.py b/tools/binman/cbfs_util_test.py
index e77f5c51c98..894c0c383b1 100755
--- a/tools/binman/cbfs_util_test.py
+++ b/tools/binman/cbfs_util_test.py
@@ -595,6 +595,18 @@ class TestCbfs(unittest.TestCase):
             cbw.get_data()
         self.assertIn('No space for data before pad offset', str(e.exception))
 
+    def test_cbfs_check_offset(self):
+        """Test that we can discover the offset of a file after writing it"""
+        size = 0xb0
+        cbw = CbfsWriter(size)
+        cbw.add_file_raw('u-boot', U_BOOT_DATA)
+        cbw.add_file_raw('u-boot-dtb', U_BOOT_DTB_DATA)
+        data = cbw.get_data()
+
+        cbfs = cbfs_util.CbfsReader(data)
+        self.assertEqual(0x38, cbfs.files['u-boot'].cbfs_offset)
+        self.assertEqual(0x78, cbfs.files['u-boot-dtb'].cbfs_offset)
+
 
 if __name__ == '__main__':
     unittest.main()
-- 
2.22.0.410.gd8fdbe21b5-goog

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [U-Boot] [PATCH 18/26] binman: Use the cbfs memlen field only for uncompressed length
  2019-07-02  0:24 [U-Boot] [PATCH 00/26] binman: Allow reading of images to list contents Simon Glass
                   ` (16 preceding siblings ...)
  2019-07-02  0:24 ` [U-Boot] [PATCH 17/26] binman: Provide the actual data address for cbfs files Simon Glass
@ 2019-07-02  0:24 ` Simon Glass
  2019-07-02  0:24 ` [U-Boot] [PATCH 19/26] binman: Add a comment about CBFS test structure Simon Glass
                   ` (7 subsequent siblings)
  25 siblings, 0 replies; 27+ messages in thread
From: Simon Glass @ 2019-07-02  0:24 UTC (permalink / raw)
  To: u-boot

The purpose of this badly named field is a bit ambiguous. Adjust the code
to use it only to store the uncompressed length of a file, leaving it set
to None if there is no compression used. This makes it easy to see if the
value in this field is relevant / useful.

Also set data_len for compressed fields, since it should be the length of
the compressed data, not the uncompressed data.

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

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

diff --git a/tools/binman/cbfs_util.py b/tools/binman/cbfs_util.py
index 530629a5c96..4691be4aee2 100644
--- a/tools/binman/cbfs_util.py
+++ b/tools/binman/cbfs_util.py
@@ -197,7 +197,8 @@ class CbfsFile(object):
         data_len: Length of (possibly compressed) data in bytes
         ftype: File type (TYPE_...)
         compression: Compression type (COMPRESS_...)
-        memlen: Length of data in memory (typically the uncompressed length)
+        memlen: Length of data in memory, i.e. the uncompressed length, None if
+            no compression algortihm is selected
         load: Load address in memory if known, else None
         entry: Entry address in memory if known, else None. This is where
             execution starts after the file is loaded
@@ -213,11 +214,11 @@ class CbfsFile(object):
         self.data = data
         self.ftype = ftype
         self.compress = compress
-        self.memlen = len(data)
+        self.memlen = None
         self.load = None
         self.entry = None
         self.base_address = None
-        self.data_len = 0
+        self.data_len = len(data)
         self.erase_byte = None
         self.size = None
 
@@ -349,9 +350,11 @@ class CbfsFile(object):
                 data = tools.Compress(orig_data, 'lz4')
             elif self.compress == COMPRESS_LZMA:
                 data = tools.Compress(orig_data, 'lzma')
+            self.memlen = len(orig_data)
+            self.data_len = len(data)
             attr = struct.pack(ATTR_COMPRESSION_FORMAT,
                                FILE_ATTR_TAG_COMPRESSION, ATTR_COMPRESSION_LEN,
-                               self.compress, len(orig_data))
+                               self.compress, self.memlen)
         elif self.ftype == TYPE_EMPTY:
             data = tools.GetBytes(self.erase_byte, self.size)
         else:
-- 
2.22.0.410.gd8fdbe21b5-goog

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [U-Boot] [PATCH 19/26] binman: Add a comment about CBFS test structure
  2019-07-02  0:24 [U-Boot] [PATCH 00/26] binman: Allow reading of images to list contents Simon Glass
                   ` (17 preceding siblings ...)
  2019-07-02  0:24 ` [U-Boot] [PATCH 18/26] binman: Use the cbfs memlen field only for uncompressed length Simon Glass
@ 2019-07-02  0:24 ` Simon Glass
  2019-07-02  0:24 ` [U-Boot] [PATCH 20/26] binman: Support FDT update for CBFS Simon Glass
                   ` (6 subsequent siblings)
  25 siblings, 0 replies; 27+ messages in thread
From: Simon Glass @ 2019-07-02  0:24 UTC (permalink / raw)
  To: u-boot

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

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

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

diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
index b1dc1643011..bee4feb1c3a 100644
--- a/tools/binman/ftest.py
+++ b/tools/binman/ftest.py
@@ -2003,7 +2003,11 @@ class TestFunctional(unittest.TestCase):
                       str(e.exception))
 
     def testCbfsOffset(self):
-        """Test a CBFS with files at particular offsets"""
+        """Test a CBFS with files at particular offsets
+
+        Like all CFBS tests, this is just checking the logic that calls
+        cbfs_util. See cbfs_util_test for fully tests (e.g. test_cbfs_offset()).
+        """
         data = self._DoReadFile('114_cbfs_offset.dts')
         size = 0x200
 
-- 
2.22.0.410.gd8fdbe21b5-goog

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [U-Boot] [PATCH 20/26] binman: Support FDT update for CBFS
  2019-07-02  0:24 [U-Boot] [PATCH 00/26] binman: Allow reading of images to list contents Simon Glass
                   ` (18 preceding siblings ...)
  2019-07-02  0:24 ` [U-Boot] [PATCH 19/26] binman: Add a comment about CBFS test structure Simon Glass
@ 2019-07-02  0:24 ` Simon Glass
  2019-07-02  0:24 ` [U-Boot] [PATCH 21/26] binman: Detect bad CBFS file types Simon Glass
                   ` (5 subsequent siblings)
  25 siblings, 0 replies; 27+ messages in thread
From: Simon Glass @ 2019-07-02  0:24 UTC (permalink / raw)
  To: u-boot

It is useful to add the CBFS file information (offset, size, etc.) into
the FDT so that the layout is complete. Add support for this.

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

 tools/binman/etype/cbfs.py            | 49 +++++++++++++++++++++++++--
 tools/binman/ftest.py                 | 24 +++++++++++++
 tools/binman/test/125_cbfs_update.dts | 21 ++++++++++++
 3 files changed, 91 insertions(+), 3 deletions(-)
 create mode 100644 tools/binman/test/125_cbfs_update.dts

diff --git a/tools/binman/etype/cbfs.py b/tools/binman/etype/cbfs.py
index 49baa6a4f63..a46bb98a033 100644
--- a/tools/binman/etype/cbfs.py
+++ b/tools/binman/etype/cbfs.py
@@ -11,6 +11,7 @@ import cbfs_util
 from cbfs_util import CbfsWriter
 from entry import Entry
 import fdt_util
+import state
 
 class Entry_cbfs(Entry):
     """Entry containing a Coreboot Filesystem (CBFS)
@@ -181,11 +182,17 @@ class Entry_cbfs(Entry):
             if not entry.ObtainContents():
                 return False
             data = entry.GetData()
+            cfile = None
             if entry._type == 'raw':
-                cbfs.add_file_raw(entry._cbfs_name, data, entry._cbfs_offset,
-                                  entry._cbfs_compress)
+                cfile = cbfs.add_file_raw(entry._cbfs_name, data,
+                                          entry._cbfs_offset,
+                                          entry._cbfs_compress)
             elif entry._type == 'stage':
-                cbfs.add_file_stage(entry._cbfs_name, data, entry._cbfs_offset)
+                cfile = cbfs.add_file_stage(entry._cbfs_name, data,
+                                            entry._cbfs_offset)
+            if cfile:
+                entry._cbfs_file = cfile
+                entry.size = cfile.data_len
         data = cbfs.get_data()
         self.SetContents(data)
         return True
@@ -203,3 +210,39 @@ class Entry_cbfs(Entry):
                 self.Raise("Invalid compression in '%s': '%s'" %
                            (node.name, compress))
             self._cbfs_entries[entry._cbfs_name] = entry
+
+    def SetImagePos(self, image_pos):
+        """Override this function to set all the entry properties from CBFS
+
+        We can only do this once image_pos is known
+
+        Args:
+            image_pos: Position of this entry in the image
+        """
+        Entry.SetImagePos(self, image_pos)
+
+        # Now update the entries with info from the CBFS entries
+        for entry in self._cbfs_entries.values():
+            cfile = entry._cbfs_file
+            entry.size = cfile.data_len
+            entry.offset = cfile.calced_cbfs_offset
+            entry.image_pos = self.image_pos + entry.offset
+            if entry._cbfs_compress:
+                entry.uncomp_size = cfile.memlen
+
+    def AddMissingProperties(self):
+        Entry.AddMissingProperties(self)
+        for entry in self._cbfs_entries.values():
+            entry.AddMissingProperties()
+            if entry._cbfs_compress:
+                state.AddZeroProp(entry._node, 'uncomp-size')
+
+    def SetCalculatedProperties(self):
+        """Set the value of device-tree properties calculated by binman"""
+        Entry.SetCalculatedProperties(self)
+        for entry in self._cbfs_entries.values():
+            state.SetInt(entry._node, 'offset', entry.offset)
+            state.SetInt(entry._node, 'size', entry.size)
+            state.SetInt(entry._node, 'image-pos', entry.image_pos)
+            if entry.uncomp_size is not None:
+                state.SetInt(entry._node, 'uncomp-size', entry.uncomp_size)
diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
index bee4feb1c3a..9cdbf798b87 100644
--- a/tools/binman/ftest.py
+++ b/tools/binman/ftest.py
@@ -2155,6 +2155,30 @@ class TestFunctional(unittest.TestCase):
             }
         self.assertEqual(expected, props)
 
+    def testCbfsUpdateFdt(self):
+        """Test that we can update the device tree with CBFS offset/size info"""
+        data, _, _, out_dtb_fname = self._DoReadFileDtb('125_cbfs_update.dts',
+                                                        update_dtb=True)
+        dtb = fdt.Fdt(out_dtb_fname)
+        dtb.Scan()
+        props = self._GetPropTree(dtb, ['offset', 'size', 'image-pos',
+                                        'uncomp-size'])
+        del props['cbfs/u-boot:size']
+        self.assertEqual({
+            'offset': 0,
+            'size': len(data),
+            'image-pos': 0,
+            'cbfs:offset': 0,
+            'cbfs:size': len(data),
+            'cbfs:image-pos': 0,
+            'cbfs/u-boot:offset': 0x38,
+            'cbfs/u-boot:uncomp-size': len(U_BOOT_DATA),
+            'cbfs/u-boot:image-pos': 0x38,
+            'cbfs/u-boot-dtb:offset': 0xb8,
+            'cbfs/u-boot-dtb:size': len(U_BOOT_DATA),
+            'cbfs/u-boot-dtb:image-pos': 0xb8,
+            }, props)
+
 
 if __name__ == "__main__":
     unittest.main()
diff --git a/tools/binman/test/125_cbfs_update.dts b/tools/binman/test/125_cbfs_update.dts
new file mode 100644
index 00000000000..6d2e8a0b8ff
--- /dev/null
+++ b/tools/binman/test/125_cbfs_update.dts
@@ -0,0 +1,21 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+/dts-v1/;
+
+/ {
+	#address-cells = <1>;
+	#size-cells = <1>;
+
+	binman {
+		cbfs {
+			size = <0x100>;
+			u-boot {
+				cbfs-type = "raw";
+				cbfs-compress = "lz4";
+			};
+			u-boot-dtb {
+				cbfs-type = "raw";
+			};
+		};
+	};
+};
-- 
2.22.0.410.gd8fdbe21b5-goog

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [U-Boot] [PATCH 21/26] binman: Detect bad CBFS file types
  2019-07-02  0:24 [U-Boot] [PATCH 00/26] binman: Allow reading of images to list contents Simon Glass
                   ` (19 preceding siblings ...)
  2019-07-02  0:24 ` [U-Boot] [PATCH 20/26] binman: Support FDT update for CBFS Simon Glass
@ 2019-07-02  0:24 ` Simon Glass
  2019-07-02  0:24 ` [U-Boot] [PATCH 22/26] binman: Allow listing the entries in an image Simon Glass
                   ` (4 subsequent siblings)
  25 siblings, 0 replies; 27+ messages in thread
From: Simon Glass @ 2019-07-02  0:24 UTC (permalink / raw)
  To: u-boot

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

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

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

diff --git a/tools/binman/etype/cbfs.py b/tools/binman/etype/cbfs.py
index a46bb98a033..175ecae1584 100644
--- a/tools/binman/etype/cbfs.py
+++ b/tools/binman/etype/cbfs.py
@@ -190,6 +190,9 @@ class Entry_cbfs(Entry):
             elif entry._type == 'stage':
                 cfile = cbfs.add_file_stage(entry._cbfs_name, data,
                                             entry._cbfs_offset)
+            else:
+                entry.Raise("Unknown cbfs-type '%s' (use 'raw', 'stage')" %
+                            entry._type)
             if cfile:
                 entry._cbfs_file = cfile
                 entry.size = cfile.data_len
diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
index 9cdbf798b87..4f29a294e61 100644
--- a/tools/binman/ftest.py
+++ b/tools/binman/ftest.py
@@ -2179,6 +2179,12 @@ class TestFunctional(unittest.TestCase):
             'cbfs/u-boot-dtb:image-pos': 0xb8,
             }, props)
 
+    def testCbfsBadType(self):
+        """Test an image header with a no specified location is detected"""
+        with self.assertRaises(ValueError) as e:
+            self._DoReadFile('126_cbfs_bad_type.dts')
+        self.assertIn("Unknown cbfs-type 'badtype'", str(e.exception))
+
 
 if __name__ == "__main__":
     unittest.main()
diff --git a/tools/binman/test/126_cbfs_bad_type.dts b/tools/binman/test/126_cbfs_bad_type.dts
new file mode 100644
index 00000000000..2cd6fc6d52d
--- /dev/null
+++ b/tools/binman/test/126_cbfs_bad_type.dts
@@ -0,0 +1,17 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+/dts-v1/;
+
+/ {
+	#address-cells = <1>;
+	#size-cells = <1>;
+
+	binman {
+		cbfs {
+			size = <0x100>;
+			u-boot {
+				cbfs-type = "badtype";
+			};
+		};
+	};
+};
-- 
2.22.0.410.gd8fdbe21b5-goog

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [U-Boot] [PATCH 22/26] binman: Allow listing the entries in an image
  2019-07-02  0:24 [U-Boot] [PATCH 00/26] binman: Allow reading of images to list contents Simon Glass
                   ` (20 preceding siblings ...)
  2019-07-02  0:24 ` [U-Boot] [PATCH 21/26] binman: Detect bad CBFS file types Simon Glass
@ 2019-07-02  0:24 ` Simon Glass
  2019-07-02  0:24 ` [U-Boot] [PATCH 23/26] binman: Support reading in firmware images Simon Glass
                   ` (3 subsequent siblings)
  25 siblings, 0 replies; 27+ messages in thread
From: Simon Glass @ 2019-07-02  0:24 UTC (permalink / raw)
  To: u-boot

It is useful to be able to summarise all the entries in an image, e.g. to
display this to this user. Add a new ListEntries() method to Entry, and
set up a way to call it through the Image class.

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

 tools/binman/bsection.py       |  8 ++++
 tools/binman/entry.py          | 34 ++++++++++++++++
 tools/binman/etype/cbfs.py     |  7 +++-
 tools/binman/etype/section.py  |  4 ++
 tools/binman/ftest.py          | 74 ++++++++++++++++++++++++++++++++++
 tools/binman/image.py          | 10 +++++
 tools/binman/test/127_list.dts | 33 +++++++++++++++
 7 files changed, 169 insertions(+), 1 deletion(-)
 create mode 100644 tools/binman/test/127_list.dts

diff --git a/tools/binman/bsection.py b/tools/binman/bsection.py
index d368e3f6baa..79639f28cfa 100644
--- a/tools/binman/bsection.py
+++ b/tools/binman/bsection.py
@@ -10,6 +10,7 @@ from __future__ import print_function
 from collections import OrderedDict
 import sys
 
+from entry import Entry
 import fdt_util
 import re
 import state
@@ -511,3 +512,10 @@ class Section(object):
                 image size is dynamic and its sections have not yet been packed
         """
         return self._image._size
+
+    def ListEntries(self, entries, indent):
+        """Override this method to list all files in the section"""
+        Entry.AddEntryInfo(entries, indent, self._name, 'section', self._size,
+                           self._image_pos, None, self._offset)
+        for entry in self._entries.values():
+            entry.ListEntries(entries, indent + 1)
diff --git a/tools/binman/entry.py b/tools/binman/entry.py
index a9a9d119e1d..a39b316b3eb 100644
--- a/tools/binman/entry.py
+++ b/tools/binman/entry.py
@@ -33,6 +33,9 @@ our_path = os.path.dirname(os.path.realpath(__file__))
 # device-tree properties.
 EntryArg = namedtuple('EntryArg', ['name', 'datatype'])
 
+# Information about an entry for use when displaying summaries
+EntryInfo = namedtuple('EntryInfo', ['indent', 'name', 'etype', 'size',
+                                     'image_pos', 'uncomp_size', 'offset'])
 
 class Entry(object):
     """An Entry in the section
@@ -614,3 +617,34 @@ features to produce new behaviours.
         if not self.HasSibling(name):
             return False
         return self.section.GetEntries()[name].image_pos
+
+    @staticmethod
+    def AddEntryInfo(entries, indent, name, etype, size, image_pos,
+                     uncomp_size, offset):
+        """Add a new entry to the entries list
+
+        Args:
+            entries: List (of EntryInfo objects) to add to
+            indent: Current indent level to add to list
+            name: Entry name (string)
+            etype: Entry type (string)
+            size: Entry size in bytes (int)
+            image_pos: Position within image in bytes (int)
+            uncomp_size: Uncompressed size if the entry uses compression, else
+                None
+            offset: Entry offset within parent in bytes (int)
+        """
+        entries.append(EntryInfo(indent, name, etype, size, image_pos,
+                                 uncomp_size, offset))
+
+    def ListEntries(self, entries, indent):
+        """Add files in this entry to the list of entries
+
+        This can be overridden by subclasses which need different behaviour.
+
+        Args:
+            entries: List (of EntryInfo objects) to add to
+            indent: Current indent level to add to list
+        """
+        self.AddEntryInfo(entries, indent, self.name, self.etype, self.size,
+                          self.image_pos, self.uncomp_size, self.offset)
diff --git a/tools/binman/etype/cbfs.py b/tools/binman/etype/cbfs.py
index 175ecae1584..953d6f4868d 100644
--- a/tools/binman/etype/cbfs.py
+++ b/tools/binman/etype/cbfs.py
@@ -195,7 +195,6 @@ class Entry_cbfs(Entry):
                             entry._type)
             if cfile:
                 entry._cbfs_file = cfile
-                entry.size = cfile.data_len
         data = cbfs.get_data()
         self.SetContents(data)
         return True
@@ -249,3 +248,9 @@ class Entry_cbfs(Entry):
             state.SetInt(entry._node, 'image-pos', entry.image_pos)
             if entry.uncomp_size is not None:
                 state.SetInt(entry._node, 'uncomp-size', entry.uncomp_size)
+
+    def ListEntries(self, entries, indent):
+        """Override this method to list all files in the section"""
+        Entry.ListEntries(self, entries, indent)
+        for entry in self._cbfs_entries.values():
+            entry.ListEntries(entries, indent + 1)
diff --git a/tools/binman/etype/section.py b/tools/binman/etype/section.py
index 23bf22113d4..178e89352e5 100644
--- a/tools/binman/etype/section.py
+++ b/tools/binman/etype/section.py
@@ -111,3 +111,7 @@ class Entry_section(Entry):
     def ExpandToLimit(self, limit):
         super(Entry_section, self).ExpandToLimit(limit)
         self._section.ExpandSize(self.size)
+
+    def ListEntries(self, entries, indent):
+        """List the files in the section"""
+        self._section.ListEntries(entries, indent)
diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
index 4f29a294e61..9552c3b2761 100644
--- a/tools/binman/ftest.py
+++ b/tools/binman/ftest.py
@@ -2185,6 +2185,80 @@ class TestFunctional(unittest.TestCase):
             self._DoReadFile('126_cbfs_bad_type.dts')
         self.assertIn("Unknown cbfs-type 'badtype'", str(e.exception))
 
+    def testList(self):
+        """Test listing the files in an image"""
+        data = self._DoReadFile('127_list.dts')
+        image = control.images['image']
+        entries = image.ListEntries()
+        self.assertEqual(7, len(entries))
+
+        ent = entries[0]
+        self.assertEqual(0, ent.indent)
+        self.assertEqual('main-section', ent.name)
+        self.assertEqual('section', ent.etype)
+        self.assertEqual(len(data), ent.size)
+        self.assertEqual(0, ent.image_pos)
+        self.assertEqual(None, ent.uncomp_size)
+        self.assertEqual(None, ent.offset)
+
+        ent = entries[1]
+        self.assertEqual(1, ent.indent)
+        self.assertEqual('u-boot', ent.name)
+        self.assertEqual('u-boot', ent.etype)
+        self.assertEqual(len(U_BOOT_DATA), ent.size)
+        self.assertEqual(0, ent.image_pos)
+        self.assertEqual(None, ent.uncomp_size)
+        self.assertEqual(0, ent.offset)
+
+        ent = entries[2]
+        self.assertEqual(1, ent.indent)
+        self.assertEqual('section', ent.name)
+        self.assertEqual('section', ent.etype)
+        section_size = ent.size
+        self.assertEqual(0x100, ent.image_pos)
+        self.assertEqual(None, ent.uncomp_size)
+        self.assertEqual(len(U_BOOT_DATA), ent.offset)
+
+        ent = entries[3]
+        self.assertEqual(2, ent.indent)
+        self.assertEqual('cbfs', ent.name)
+        self.assertEqual('cbfs', ent.etype)
+        self.assertEqual(0x400, ent.size)
+        self.assertEqual(0x100, ent.image_pos)
+        self.assertEqual(None, ent.uncomp_size)
+        self.assertEqual(0, ent.offset)
+
+        ent = entries[4]
+        self.assertEqual(3, ent.indent)
+        self.assertEqual('u-boot', ent.name)
+        self.assertEqual('u-boot', ent.etype)
+        self.assertEqual(len(U_BOOT_DATA), ent.size)
+        self.assertEqual(0x138, ent.image_pos)
+        self.assertEqual(None, ent.uncomp_size)
+        self.assertEqual(0x38, ent.offset)
+
+        ent = entries[5]
+        self.assertEqual(3, ent.indent)
+        self.assertEqual('u-boot-dtb', ent.name)
+        self.assertEqual('text', ent.etype)
+        self.assertGreater(len(COMPRESS_DATA), ent.size)
+        self.assertEqual(0x178, ent.image_pos)
+        self.assertEqual(len(COMPRESS_DATA), ent.uncomp_size)
+        self.assertEqual(0x78, ent.offset)
+
+        ent = entries[6]
+        self.assertEqual(2, ent.indent)
+        self.assertEqual('u-boot-dtb', ent.name)
+        self.assertEqual('u-boot-dtb', ent.etype)
+        self.assertEqual(0x500, ent.image_pos)
+        self.assertEqual(len(U_BOOT_DTB_DATA), ent.uncomp_size)
+        dtb_size = ent.size
+        # Compressing this data expands it since headers are added
+        self.assertGreater(dtb_size, len(U_BOOT_DTB_DATA))
+        self.assertEqual(0x400, ent.offset)
+
+        self.assertEqual(len(data), 0x100 + section_size)
+        self.assertEqual(section_size, 0x400 + dtb_size)
 
 if __name__ == "__main__":
     unittest.main()
diff --git a/tools/binman/image.py b/tools/binman/image.py
index 6339d020e76..d4145685972 100644
--- a/tools/binman/image.py
+++ b/tools/binman/image.py
@@ -162,3 +162,13 @@ class Image:
                   file=fd)
             self._section.WriteMap(fd, 0)
         return fname
+
+    def ListEntries(self):
+        """List the files in an image
+
+        Returns:
+            List of entry.EntryInfo objects describing all entries in the image
+        """
+        entries = []
+        self._section.ListEntries(entries, 0)
+        return entries
diff --git a/tools/binman/test/127_list.dts b/tools/binman/test/127_list.dts
new file mode 100644
index 00000000000..c1d6fce3f9e
--- /dev/null
+++ b/tools/binman/test/127_list.dts
@@ -0,0 +1,33 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+/dts-v1/;
+
+/ {
+	#address-cells = <1>;
+	#size-cells = <1>;
+
+	binman {
+		u-boot {
+		};
+		section {
+			align = <0x100>;
+			cbfs {
+				size = <0x400>;
+				u-boot {
+					cbfs-type = "raw";
+					cbfs-offset = <0x38>;
+				};
+				u-boot-dtb {
+					type = "text";
+					text = "compress xxxxxxxxxxxxxxxxxxxxxx data";
+					cbfs-type = "raw";
+					cbfs-compress = "lzma";
+					cbfs-offset = <0x78>;
+				};
+			};
+			u-boot-dtb {
+				compress = "lz4";
+			};
+		};
+	};
+};
-- 
2.22.0.410.gd8fdbe21b5-goog

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [U-Boot] [PATCH 23/26] binman: Support reading in firmware images
  2019-07-02  0:24 [U-Boot] [PATCH 00/26] binman: Allow reading of images to list contents Simon Glass
                   ` (21 preceding siblings ...)
  2019-07-02  0:24 ` [U-Boot] [PATCH 22/26] binman: Allow listing the entries in an image Simon Glass
@ 2019-07-02  0:24 ` Simon Glass
  2019-07-02  0:24 ` [U-Boot] [PATCH 24/26] binman: Support locating an image header Simon Glass
                   ` (2 subsequent siblings)
  25 siblings, 0 replies; 27+ messages in thread
From: Simon Glass @ 2019-07-02  0:24 UTC (permalink / raw)
  To: u-boot

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

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

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

diff --git a/tools/binman/etype/fdtmap.py b/tools/binman/etype/fdtmap.py
index bfd7962be3a..4fe5f8d9e10 100644
--- a/tools/binman/etype/fdtmap.py
+++ b/tools/binman/etype/fdtmap.py
@@ -15,7 +15,27 @@ from fdt import Fdt
 import state
 import tools
 
-FDTMAP_MAGIC = b'_FDTMAP_'
+FDTMAP_MAGIC   = b'_FDTMAP_'
+FDTMAP_HDR_LEN = 16
+
+def LocateFdtmap(data):
+    """Search an image for an fdt map
+
+    Args:
+        data: Data to search
+
+    Returns:
+        Position of fdt map in data, or None if not found. Note that the
+            position returned is of the FDT map data itself, i.e. after the
+            header
+    """
+    hdr_pos = data.find(FDTMAP_MAGIC)
+    size = len(data)
+    if hdr_pos:
+        hdr = data[hdr_pos:hdr_pos + FDTMAP_HDR_LEN]
+        if len(hdr) == FDTMAP_HDR_LEN:
+            return hdr_pos + FDTMAP_HDR_LEN
+    return None
 
 class Entry_fdtmap(Entry):
     """An entry which contains an FDT map
diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
index 9552c3b2761..2536d03c374 100644
--- a/tools/binman/ftest.py
+++ b/tools/binman/ftest.py
@@ -24,6 +24,7 @@ import command
 import control
 import elf
 import fdt
+from etype import fdtmap
 import fdt_util
 import fmap_util
 import test_util
@@ -2260,5 +2261,21 @@ class TestFunctional(unittest.TestCase):
         self.assertEqual(len(data), 0x100 + section_size)
         self.assertEqual(section_size, 0x400 + dtb_size)
 
+    def testFindFdtmap(self):
+        """Test locating an FDT map in an image"""
+        data = self._DoReadFileDtb('128_decode_image.dts', use_real_dtb=True,
+                                   update_dtb=True)[0]
+        image = control.images['image']
+        entries = image.GetEntries()
+        entry = entries['fdtmap']
+        self.assertEqual(entry.image_pos + fdtmap.FDTMAP_HDR_LEN,
+                         fdtmap.LocateFdtmap(data))
+
+    def testFindFdtmapMissing(self):
+        """Test failing to locate an FDP map"""
+        data = self._DoReadFile('005_simple.dts')
+        self.assertEqual(None, fdtmap.LocateFdtmap(data))
+
+
 if __name__ == "__main__":
     unittest.main()
diff --git a/tools/binman/test/128_decode_image.dts b/tools/binman/test/128_decode_image.dts
new file mode 100644
index 00000000000..449fccc41df
--- /dev/null
+++ b/tools/binman/test/128_decode_image.dts
@@ -0,0 +1,36 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+/dts-v1/;
+
+/ {
+	#address-cells = <1>;
+	#size-cells = <1>;
+
+	binman {
+		size = <0xc00>;
+		u-boot {
+		};
+		section {
+			align = <0x100>;
+			cbfs {
+				size = <0x400>;
+				u-boot {
+					cbfs-type = "raw";
+				};
+				u-boot-dtb {
+					cbfs-type = "raw";
+					cbfs-compress = "lzma";
+					cbfs-offset = <0x80>;
+				};
+			};
+			u-boot-dtb {
+				compress = "lz4";
+			};
+		};
+		fdtmap {
+		};
+		image-header {
+			location = "end";
+		};
+	};
+};
-- 
2.22.0.410.gd8fdbe21b5-goog

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [U-Boot] [PATCH 24/26] binman: Support locating an image header
  2019-07-02  0:24 [U-Boot] [PATCH 00/26] binman: Allow reading of images to list contents Simon Glass
                   ` (22 preceding siblings ...)
  2019-07-02  0:24 ` [U-Boot] [PATCH 23/26] binman: Support reading in firmware images Simon Glass
@ 2019-07-02  0:24 ` Simon Glass
  2019-07-02  0:24 ` [U-Boot] [PATCH 25/26] binman: Support reading an image into an Image object Simon Glass
  2019-07-02  0:24 ` [U-Boot] [PATCH 26/26] binman: Support listing an image Simon Glass
  25 siblings, 0 replies; 27+ messages in thread
From: Simon Glass @ 2019-07-02  0:24 UTC (permalink / raw)
  To: u-boot

Add support for locating an image header in an image.

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

 tools/binman/etype/image_header.py | 23 +++++++++++++++++++++++
 tools/binman/ftest.py              | 26 ++++++++++++++++++++++++++
 2 files changed, 49 insertions(+)

diff --git a/tools/binman/etype/image_header.py b/tools/binman/etype/image_header.py
index b1c4f8a07e9..8f9c5aa5d9e 100644
--- a/tools/binman/etype/image_header.py
+++ b/tools/binman/etype/image_header.py
@@ -15,6 +15,29 @@ from entry import Entry
 import fdt_util
 
 IMAGE_HEADER_MAGIC = b'BinM'
+IMAGE_HEADER_LEN   = 8
+
+def LocateHeaderOffset(data):
+    """Search an image for an image header
+
+    Args:
+        data: Data to search
+
+    Returns:
+        Offset of image header in the image, or None if not found
+    """
+    hdr_pos = data.find(IMAGE_HEADER_MAGIC)
+    if hdr_pos != -1:
+        size = len(data)
+        hdr = data[hdr_pos:hdr_pos + IMAGE_HEADER_LEN]
+        if len(hdr) == IMAGE_HEADER_LEN:
+            offset = struct.unpack('<I', hdr[4:])[0]
+            if hdr_pos == len(data) - IMAGE_HEADER_LEN:
+                pos = size + offset - (1 << 32)
+            else:
+                pos = offset
+            return pos
+    return None
 
 class Entry_image_header(Entry):
     """An entry which contains a pointer to the FDT map
diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
index 2536d03c374..15cf5cea457 100644
--- a/tools/binman/ftest.py
+++ b/tools/binman/ftest.py
@@ -25,6 +25,7 @@ import control
 import elf
 import fdt
 from etype import fdtmap
+from etype import image_header
 import fdt_util
 import fmap_util
 import test_util
@@ -2276,6 +2277,31 @@ class TestFunctional(unittest.TestCase):
         data = self._DoReadFile('005_simple.dts')
         self.assertEqual(None, fdtmap.LocateFdtmap(data))
 
+    def testFindImageHeader(self):
+        """Test locating a image header"""
+        data = self._DoReadFileDtb('128_decode_image.dts', use_real_dtb=True,
+                                   update_dtb=True)[0]
+        image = control.images['image']
+        entries = image.GetEntries()
+        entry = entries['fdtmap']
+        # The header should point to the FDT map
+        self.assertEqual(entry.image_pos, image_header.LocateHeaderOffset(data))
+
+    def testFindImageHeaderStart(self):
+        """Test locating a image header located@the start of an image"""
+        data = self._DoReadFileDtb('117_fdtmap_hdr_start.dts',
+                                   use_real_dtb=True, update_dtb=True)[0]
+        image = control.images['image']
+        entries = image.GetEntries()
+        entry = entries['fdtmap']
+        # The header should point to the FDT map
+        self.assertEqual(entry.image_pos, image_header.LocateHeaderOffset(data))
+
+    def testFindImageHeaderMissing(self):
+        """Test failing to locate an image header"""
+        data = self._DoReadFile('005_simple.dts')
+        self.assertEqual(None, image_header.LocateHeaderOffset(data))
+
 
 if __name__ == "__main__":
     unittest.main()
-- 
2.22.0.410.gd8fdbe21b5-goog

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [U-Boot] [PATCH 25/26] binman: Support reading an image into an Image object
  2019-07-02  0:24 [U-Boot] [PATCH 00/26] binman: Allow reading of images to list contents Simon Glass
                   ` (23 preceding siblings ...)
  2019-07-02  0:24 ` [U-Boot] [PATCH 24/26] binman: Support locating an image header Simon Glass
@ 2019-07-02  0:24 ` Simon Glass
  2019-07-02  0:24 ` [U-Boot] [PATCH 26/26] binman: Support listing an image Simon Glass
  25 siblings, 0 replies; 27+ messages in thread
From: Simon Glass @ 2019-07-02  0:24 UTC (permalink / raw)
  To: u-boot

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

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

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

diff --git a/tools/binman/entry.py b/tools/binman/entry.py
index a39b316b3eb..1fe905ae904 100644
--- a/tools/binman/entry.py
+++ b/tools/binman/entry.py
@@ -159,6 +159,12 @@ class Entry(object):
         self.offset = fdt_util.GetInt(self._node, 'offset')
         self.size = fdt_util.GetInt(self._node, 'size')
         self.orig_offset, self.orig_size = self.offset, self.size
+
+        # These should not be set in input files, but are set in an FDT map,
+        # which is also read by this code.
+        self.image_pos = fdt_util.GetInt(self._node, 'image-pos')
+        self.uncomp_size = fdt_util.GetInt(self._node, 'uncomp-size')
+
         self.align = fdt_util.GetInt(self._node, 'align')
         if tools.NotPowerOfTwo(self.align):
             raise ValueError("Node '%s': Alignment %s must be a power of two" %
diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
index 15cf5cea457..88c3d847f82 100644
--- a/tools/binman/ftest.py
+++ b/tools/binman/ftest.py
@@ -30,6 +30,7 @@ import fdt_util
 import fmap_util
 import test_util
 import gzip
+from image import Image
 import state
 import tools
 import tout
@@ -2302,6 +2303,38 @@ class TestFunctional(unittest.TestCase):
         data = self._DoReadFile('005_simple.dts')
         self.assertEqual(None, image_header.LocateHeaderOffset(data))
 
+    def testReadImage(self):
+        """Test reading an image and accessing its FDT map"""
+        data = self._DoReadFileDtb('128_decode_image.dts',
+                                   use_real_dtb=True, update_dtb=True)[0]
+        image_fname = tools.GetOutputFilename('image.bin')
+        orig_image = control.images['image']
+        image = Image.FromFile(image_fname)
+        self.assertEqual(orig_image.GetEntries().keys(),
+                         image.GetEntries().keys())
+
+        orig_entry = orig_image.GetEntries()['fdtmap']
+        entry = image.GetEntries()['fdtmap']
+        self.assertEquals(orig_entry.offset, entry.offset)
+        self.assertEquals(orig_entry.size, entry.size)
+        self.assertEquals(orig_entry.image_pos, entry.image_pos)
+
+    def testReadImageNoHeader(self):
+        """Test accessing an image's FDT map without an image header"""
+        data = self._DoReadFileDtb('128_decode_image_no_hdr.dts',
+                                   use_real_dtb=True, update_dtb=True)[0]
+        image_fname = tools.GetOutputFilename('image.bin')
+        image = Image.FromFile(image_fname)
+        self.assertTrue(isinstance(image, Image))
+        self.assertEqual('image', image._name)
+
+    def testReadImageFail(self):
+        """Test failing to read an image image's FDT map"""
+        self._DoReadFile('005_simple.dts')
+        image_fname = tools.GetOutputFilename('image.bin')
+        image = Image.FromFile(image_fname)
+        self.assertTrue(isinstance(image, str))
+        self.assertEqual('Cannot find FDT map in image', image)
 
 if __name__ == "__main__":
     unittest.main()
diff --git a/tools/binman/image.py b/tools/binman/image.py
index d4145685972..d918b5a7194 100644
--- a/tools/binman/image.py
+++ b/tools/binman/image.py
@@ -12,6 +12,9 @@ from operator import attrgetter
 import re
 import sys
 
+from etype import fdtmap
+from etype import image_header
+import fdt
 import fdt_util
 import bsection
 import tools
@@ -47,6 +50,39 @@ class Image:
         else:
             self._ReadNode()
 
+    @classmethod
+    def FromFile(cls, fname):
+        """Convert an image file into an Image for use in binman
+
+        Args:
+            fname: Filename of image file to read
+
+        Returns:
+            Image object on success, or string error message on failure
+        """
+        data = tools.ReadFile(fname)
+        size = len(data)
+
+        # First look for an image header
+        pos = image_header.LocateHeaderOffset(data)
+        if pos is None:
+            # Look for the FDT map
+            pos = fdtmap.LocateFdtmap(data)
+        else:
+            # Move past the header to the FDT
+            pos += fdtmap.FDTMAP_HDR_LEN
+        if pos is None:
+            return 'Cannot find FDT map in image'
+
+        # We don't knowe the FDT size, so check its header first
+        probe_dtb = fdt.Fdt.FromData(data[pos:pos + 256])
+        dtb_size = probe_dtb.GetFdtObj().totalsize()
+        dtb = fdt.Fdt.FromData(data[pos:pos + dtb_size])
+        dtb.Scan()
+
+        # Return an Image with the associated nodes
+        return Image('image', dtb.GetRoot())
+
     def _ReadNode(self):
         """Read properties from the image node"""
         self._size = fdt_util.GetInt(self._node, 'size')
diff --git a/tools/binman/test/128_decode_image_no_hdr.dts b/tools/binman/test/128_decode_image_no_hdr.dts
new file mode 100644
index 00000000000..90fdd8820ca
--- /dev/null
+++ b/tools/binman/test/128_decode_image_no_hdr.dts
@@ -0,0 +1,33 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+/dts-v1/;
+
+/ {
+	#address-cells = <1>;
+	#size-cells = <1>;
+
+	binman {
+		size = <0xc00>;
+		u-boot {
+		};
+		section {
+			align = <0x100>;
+			cbfs {
+				size = <0x400>;
+				u-boot {
+					cbfs-type = "raw";
+				};
+				u-boot-dtb {
+					cbfs-type = "raw";
+					cbfs-compress = "lzma";
+					cbfs-offset = <0x80>;
+				};
+			};
+			u-boot-dtb {
+				compress = "lz4";
+			};
+		};
+		fdtmap {
+		};
+	};
+};
diff --git a/tools/binman/test/129_list_fdtmap.dts b/tools/binman/test/129_list_fdtmap.dts
new file mode 100644
index 00000000000..449fccc41df
--- /dev/null
+++ b/tools/binman/test/129_list_fdtmap.dts
@@ -0,0 +1,36 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+/dts-v1/;
+
+/ {
+	#address-cells = <1>;
+	#size-cells = <1>;
+
+	binman {
+		size = <0xc00>;
+		u-boot {
+		};
+		section {
+			align = <0x100>;
+			cbfs {
+				size = <0x400>;
+				u-boot {
+					cbfs-type = "raw";
+				};
+				u-boot-dtb {
+					cbfs-type = "raw";
+					cbfs-compress = "lzma";
+					cbfs-offset = <0x80>;
+				};
+			};
+			u-boot-dtb {
+				compress = "lz4";
+			};
+		};
+		fdtmap {
+		};
+		image-header {
+			location = "end";
+		};
+	};
+};
-- 
2.22.0.410.gd8fdbe21b5-goog

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [U-Boot] [PATCH 26/26] binman: Support listing an image
  2019-07-02  0:24 [U-Boot] [PATCH 00/26] binman: Allow reading of images to list contents Simon Glass
                   ` (24 preceding siblings ...)
  2019-07-02  0:24 ` [U-Boot] [PATCH 25/26] binman: Support reading an image into an Image object Simon Glass
@ 2019-07-02  0:24 ` Simon Glass
  25 siblings, 0 replies; 27+ messages in thread
From: Simon Glass @ 2019-07-02  0:24 UTC (permalink / raw)
  To: u-boot

Add support for listing the entries in an image. This relies on the image
having an FDT map.

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

 tools/binman/README     | 25 +++++++++++++-
 tools/binman/control.py | 73 +++++++++++++++++++++++++++++++++++++++++
 tools/binman/ftest.py   | 31 +++++++++++++++++
 3 files changed, 128 insertions(+), 1 deletion(-)

diff --git a/tools/binman/README b/tools/binman/README
index 7e745a2d466..2f3ded58c91 100644
--- a/tools/binman/README
+++ b/tools/binman/README
@@ -490,6 +490,30 @@ see README.entries. This is generated from the source code using:
 	binman entry-docs >tools/binman/README.entries
 
 
+Listing images
+--------------
+
+It is possible to list the entries in an existing firmware image created by
+binman. For example:
+
+    $ binman list image.bin
+    Name              Image-pos  Size  Entry-type    Offset  Uncomp-size
+    ----------------------------------------------------------------------
+    main-section                  c00  section            0
+      u-boot                  0     4  u-boot             0
+      section                     5ff  section            4
+        cbfs                100   400  cbfs               0
+          u-boot            138     4  u-boot            38
+          u-boot-dtb        180   108  u-boot-dtb        80          3b5
+        u-boot-dtb          500   1ff  u-boot-dtb       400          3b5
+      fdtmap                6ff   381  fdtmap           6ff
+      image-header          bf8     8  image-header     bf8
+
+This shows the hierarchy of the image, the position, size and type of each
+entry, the offset of each entry within its parent and the uncompressed size if
+the entry is compressed.
+
+    
 Hashing Entries
 ---------------
 
@@ -825,7 +849,6 @@ Some ideas:
 - Add an option to decode an image into the constituent binaries
 - Support building an image for a board (-b) more completely, with a
   configurable build directory
-- Support listing files in images
 - Support logging of binman's operations, with different levels of verbosity
 
 --
diff --git a/tools/binman/control.py b/tools/binman/control.py
index d48e7ac0070..3ac3f0c3e71 100644
--- a/tools/binman/control.py
+++ b/tools/binman/control.py
@@ -67,6 +67,75 @@ def WriteEntryDocs(modules, test_missing=None):
     from entry import Entry
     Entry.WriteDocs(modules, test_missing)
 
+def EntryToStrings(entry):
+    """Convert an entry to a list of strings, one for each column
+
+    Args:
+        entry: EntryInfo object containing information to output
+
+    Returns:
+        List of strings, one for each field in entry
+    """
+    def _AppendHex(val):
+        """Append a hex value, or an empty string if val is None
+
+        Args:
+            val: Integer value, or None if none
+        """
+        args.append('' if val is None else '>%x' % val)
+
+    args = ['  ' * entry.indent + entry.name]
+    _AppendHex(entry.image_pos)
+    _AppendHex(entry.size)
+    args.append(entry.etype)
+    _AppendHex(entry.offset)
+    _AppendHex(entry.uncomp_size)
+    return args
+
+def ListEntries(fname):
+    """List the entries in an image
+
+    This decodes the supplied image and displays a table of entries from that
+    image, preceded by a header.
+
+    Args:
+        fname: Filename to process
+    """
+    def _DoLine(line):
+        out = []
+        for i, item in enumerate(line):
+            lengths[i] = max(lengths[i], len(item))
+            out.append(item)
+        return out
+
+    image = Image.FromFile(fname)
+
+    # Check for error
+    if isinstance(image, str):
+        print("Unable to read image '%s' (%s)" % (fname, image))
+        return
+    entries = image.ListEntries()
+
+    num_columns = 6
+    lines = []
+    lengths = [0] * num_columns
+    lines.append(_DoLine(['Name', 'Image-pos', 'Size', 'Entry-type',
+                          'Offset', 'Uncomp-size']))
+    for entry in entries:
+        lines.append(_DoLine(EntryToStrings(entry)))
+    for linenum, line in enumerate(lines):
+        if linenum == 1:
+            print('-' * (sum(lengths) + num_columns * 2))
+        out = ''
+        for i, item in enumerate(line):
+            width = -lengths[i]
+            if item.startswith('>'):
+                width = -width
+                item = item[1:]
+            txt = '%*s  ' % (width, item)
+            out += txt
+        print(out.rstrip())
+
 def Binman(args):
     """The main control code for binman
 
@@ -87,6 +156,10 @@ def Binman(args):
         command.Run(pager, fname)
         return 0
 
+    if args.cmd == 'list':
+        ListEntries(args.fname)
+        return 0
+
     # Try to figure out which device tree contains our image description
     if args.dt:
         dtb_fname = args.dt
diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
index 88c3d847f82..f5353911705 100644
--- a/tools/binman/ftest.py
+++ b/tools/binman/ftest.py
@@ -2336,5 +2336,36 @@ class TestFunctional(unittest.TestCase):
         self.assertTrue(isinstance(image, str))
         self.assertEqual('Cannot find FDT map in image', image)
 
+    def testListCmd(self):
+        """Test listing the files in an image using an Fdtmap"""
+        data = self._DoReadFileDtb('129_list_fdtmap.dts', use_real_dtb=True,
+                                   update_dtb=True)[0]
+        image_fname = tools.GetOutputFilename('image.bin')
+        with test_util.capture_sys_output() as (stdout, stderr):
+            self._DoBinman('list', image_fname)
+        lines = stdout.getvalue().splitlines()
+        expected = [
+            'Name              Image-pos  Size  Entry-type    Offset  Uncomp-size',
+'----------------------------------------------------------------------',
+'main-section                  c00  section            0',
+'  u-boot                  0     4  u-boot             0',
+'  section                     5ff  section            4',
+'    cbfs                100   400  cbfs               0',
+'      u-boot            138     4  u-boot            38',
+'      u-boot-dtb        180   108  u-boot-dtb        80          3b5',
+'    u-boot-dtb          500   1ff  u-boot-dtb       400          3b5',
+'  fdtmap                6ff   381  fdtmap           6ff',
+'  image-header          bf8     8  image-header     bf8',
+            ]
+        self.assertEqual(expected, lines)
+
+    def testListCmdFail(self):
+        """Test failing to list an image"""
+        self._DoReadFile('005_simple.dts')
+        image_fname = tools.GetOutputFilename('image.bin')
+        with test_util.capture_sys_output() as (stdout, stderr):
+            self._DoBinman('list', image_fname)
+
+
 if __name__ == "__main__":
     unittest.main()
-- 
2.22.0.410.gd8fdbe21b5-goog

^ permalink raw reply related	[flat|nested] 27+ messages in thread

end of thread, other threads:[~2019-07-02  0:24 UTC | newest]

Thread overview: 27+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-07-02  0:24 [U-Boot] [PATCH 00/26] binman: Allow reading of images to list contents Simon Glass
2019-07-02  0:24 ` [U-Boot] [PATCH 01/26] binman: Simplify the entry test Simon Glass
2019-07-02  0:24 ` [U-Boot] [PATCH 02/26] binman: Update future features Simon Glass
2019-07-02  0:24 ` [U-Boot] [PATCH 03/26] binman: Update help for new features Simon Glass
2019-07-02  0:24 ` [U-Boot] [PATCH 04/26] binman: Add an FDT map Simon Glass
2019-07-02  0:24 ` [U-Boot] [PATCH 05/26] binman: Add an image header Simon Glass
2019-07-02  0:24 ` [U-Boot] [PATCH 06/26] binman: Allow cbfstool to be optional with tests Simon Glass
2019-07-02  0:24 ` [U-Boot] [PATCH 07/26] binman: Convert to use ArgumentParser Simon Glass
2019-07-02  0:24 ` [U-Boot] [PATCH 08/26] binman: Move compression into the Entry base class Simon Glass
2019-07-02  0:24 ` [U-Boot] [PATCH 09/26] binman: Drop an unused arg in Entry.Lookup() Simon Glass
2019-07-02  0:24 ` [U-Boot] [PATCH 10/26] binman: Allow easy importing of entry modules Simon Glass
2019-07-02  0:24 ` [U-Boot] [PATCH 11/26] binman: Fix up ProcessUpdateContents error and comments Simon Glass
2019-07-02  0:24 ` [U-Boot] [PATCH 12/26] binman: Call ProcessUpdateContents() consistently Simon Glass
2019-07-02  0:24 ` [U-Boot] [PATCH 13/26] binman: Add a return value to ProcessContentsUpdate() Simon Glass
2019-07-02  0:24 ` [U-Boot] [PATCH 14/26] binman: Add a control for post-pack entry expansion Simon Glass
2019-07-02  0:24 ` [U-Boot] [PATCH 15/26] binman: Allow entries to expand after packing Simon Glass
2019-07-02  0:24 ` [U-Boot] [PATCH 16/26] binman: Allow device-tree entries to be compressed Simon Glass
2019-07-02  0:24 ` [U-Boot] [PATCH 17/26] binman: Provide the actual data address for cbfs files Simon Glass
2019-07-02  0:24 ` [U-Boot] [PATCH 18/26] binman: Use the cbfs memlen field only for uncompressed length Simon Glass
2019-07-02  0:24 ` [U-Boot] [PATCH 19/26] binman: Add a comment about CBFS test structure Simon Glass
2019-07-02  0:24 ` [U-Boot] [PATCH 20/26] binman: Support FDT update for CBFS Simon Glass
2019-07-02  0:24 ` [U-Boot] [PATCH 21/26] binman: Detect bad CBFS file types Simon Glass
2019-07-02  0:24 ` [U-Boot] [PATCH 22/26] binman: Allow listing the entries in an image Simon Glass
2019-07-02  0:24 ` [U-Boot] [PATCH 23/26] binman: Support reading in firmware images Simon Glass
2019-07-02  0:24 ` [U-Boot] [PATCH 24/26] binman: Support locating an image header Simon Glass
2019-07-02  0:24 ` [U-Boot] [PATCH 25/26] binman: Support reading an image into an Image object Simon Glass
2019-07-02  0:24 ` [U-Boot] [PATCH 26/26] binman: Support listing an image Simon Glass

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.