]> git.dujemihanovic.xyz Git - u-boot.git/commitdiff
binman: Add info to allow safely repacking an image later
authorSimon Glass <sjg@chromium.org>
Sat, 20 Jul 2019 18:23:51 +0000 (12:23 -0600)
committerSimon Glass <sjg@chromium.org>
Mon, 29 Jul 2019 15:38:06 +0000 (09:38 -0600)
At present it is not possible to discover the contraints to repacking an
image (e.g. maximum section size) since this information is not preserved
from the original image description.

Add new 'orig-offset' and 'orig-size' properties to hold this. Add them to
the main device tree in the image.

Signed-off-by: Simon Glass <sjg@chromium.org>
tools/binman/README
tools/binman/README.entries
tools/binman/entry.py
tools/binman/etype/fdtmap.py
tools/binman/ftest.py
tools/binman/image.py
tools/binman/state.py
tools/binman/test/134_fdt_update_all_repack.dts [new file with mode: 0644]

index 3522354519412ff57b3bfeaee205fd50a46fa82d..6a1cd110a4c6e377c212cb76eea9d2f136f491f6 100644 (file)
@@ -481,6 +481,29 @@ name-prefix:
        distinguish binaries with otherwise identical names.
 
 
+Image Properties
+----------------
+
+Image nodes act like sections but also have a few extra properties:
+
+filename:
+       Output filename for the image. This defaults to image.bin (or in the
+       case of multiple images <nodename>.bin where <nodename> is the name of
+       the image node.
+
+allow-repack:
+       Create an image that can be repacked. With this option it is possible
+       to change anything in the image after it is created, including updating
+       the position and size of image components. By default this is not
+       permitted since it is not possibly to know whether this might violate a
+       constraint in the image description. For example, if a section has to
+       increase in size to hold a larger binary, that might cause the section
+       to fall out of its allow region (e.g. read-only portion of flash).
+
+       Adding this property causes the original offset and size values in the
+       image description to be stored in the FDT and fdtmap.
+
+
 Entry Documentation
 -------------------
 
index 7ce88ee5da8569e7b5a8a43ab79512c1e4cb4e06..37b8b4c4f9840df5ac4f879062c3fcf5fac980f2 100644 (file)
@@ -230,7 +230,9 @@ 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.
+entries in the image. The root node corresponds to the image node in the
+original FDT, and an image-name property indicates the image name in that
+original tree.
 
 The header is the string _FDTMAP_ followed by 8 unused bytes.
 
@@ -244,6 +246,7 @@ FDT with the position of each entry.
 Example output for a simple image with U-Boot and an FDT map:
 
 / {
+    image-name = "binman";
     size = <0x00000112>;
     image-pos = <0x00000000>;
     offset = <0x00000000>;
@@ -259,6 +262,9 @@ Example output for a simple image with U-Boot and an FDT map:
     };
 };
 
+If allow-repack is used then 'orig-offset' and 'orig-size' properties are
+added as necessary. See the binman README.
+
 
 
 Entry: files: Entry containing a set of files
index 07d5838c1a2cd28b88eee2839c06f2bf0567c0d2..74e280849c4d76f2f19917ec6bce68e5af4e3608 100644 (file)
@@ -161,8 +161,11 @@ 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.offset
-        self.orig_size = self.size
+        self.orig_offset = fdt_util.GetInt(self._node, 'orig-offset')
+        self.orig_size = fdt_util.GetInt(self._node, 'orig-size')
+        if self.GetImage().copy_to_orig:
+            self.orig_offset = self.offset
+            self.orig_size = self.size
 
         # These should not be set in input files, but are set in an FDT map,
         # which is also read by this code.
@@ -207,6 +210,12 @@ class Entry(object):
         for prop in ['offset', 'size', 'image-pos']:
             if not prop in self._node.props:
                 state.AddZeroProp(self._node, prop)
+        if self.GetImage().allow_repack:
+            if self.orig_offset is not None:
+                state.AddZeroProp(self._node, 'orig-offset', True)
+            if self.orig_size is not None:
+                state.AddZeroProp(self._node, 'orig-size', True)
+
         if self.compress != 'none':
             state.AddZeroProp(self._node, 'uncomp-size')
         err = state.CheckAddHashProp(self._node)
@@ -219,6 +228,11 @@ class Entry(object):
         state.SetInt(self._node, 'size', self.size)
         base = self.section.GetRootSkipAtStart() if self.section else 0
         state.SetInt(self._node, 'image-pos', self.image_pos - base)
+        if self.GetImage().allow_repack:
+            if self.orig_offset is not None:
+                state.SetInt(self._node, 'orig-offset', self.orig_offset, True)
+            if self.orig_size is not None:
+                state.SetInt(self._node, 'orig-size', self.orig_size, True)
         if self.uncomp_size is not None:
             state.SetInt(self._node, 'uncomp-size', self.uncomp_size)
         state.CheckSetHashValue(self._node, self.GetData)
index 1271b50036a4deac93e56b50e39f2a07dc2184af..ff3f1ae812969f820a1564a532f7d03d387b2a86 100644 (file)
@@ -75,6 +75,9 @@ class Entry_fdtmap(Entry):
             offset = <0x00000004>;
         };
     };
+
+    If allow-repack is used then 'orig-offset' and 'orig-size' properties are
+    added as necessary. See the binman README.
     """
     def __init__(self, section, etype, node):
         Entry.__init__(self, section, etype, node)
index e201b741c6fcf127ec8b7600fa9767eab4182244..b67e8a15086b370ca02cb8ef7327f8b7288504bc 100644 (file)
@@ -75,6 +75,9 @@ EXTRACT_DTB_SIZE = 0x3c9
 # Properties expected to be in the device tree when update_dtb is used
 BASE_DTB_PROPS = ['offset', 'size', 'image-pos']
 
+# Extra properties expected to be in the device tree when allow-repack is used
+REPACK_DTB_PROPS = ['orig-offset', 'orig-size']
+
 
 class TestFunctional(unittest.TestCase):
     """Functional tests for binman
@@ -1244,7 +1247,7 @@ class TestFunctional(unittest.TestCase):
                                                      update_dtb=True)
         dtb = fdt.Fdt(out_dtb_fname)
         dtb.Scan()
-        props = self._GetPropTree(dtb, BASE_DTB_PROPS)
+        props = self._GetPropTree(dtb, BASE_DTB_PROPS + REPACK_DTB_PROPS)
         self.assertEqual({
             'image-pos': 0,
             'offset': 0,
@@ -1587,7 +1590,8 @@ class TestFunctional(unittest.TestCase):
         for item in ['', 'spl', 'tpl']:
             dtb = fdt.Fdt.FromData(data[start:])
             dtb.Scan()
-            props = self._GetPropTree(dtb, BASE_DTB_PROPS + ['spl', 'tpl'])
+            props = self._GetPropTree(dtb, BASE_DTB_PROPS + REPACK_DTB_PROPS +
+                                      ['spl', 'tpl'])
             expected = dict(base_expected)
             if item:
                 expected[item] = 0
@@ -2821,5 +2825,67 @@ class TestFunctional(unittest.TestCase):
         # Check the state looks right.
         self.assertEqual('/binman/first-image', state.fdt_path_prefix)
 
+    def testUpdateFdtAllRepack(self):
+        """Test that all device trees are updated with offset/size info"""
+        data = self._DoReadFileRealDtb('134_fdt_update_all_repack.dts')
+        SECTION_SIZE = 0x300
+        DTB_SIZE = 602
+        FDTMAP_SIZE = 608
+        base_expected = {
+            'offset': 0,
+            'size': SECTION_SIZE + DTB_SIZE * 2 + FDTMAP_SIZE,
+            'image-pos': 0,
+            'section:offset': 0,
+            'section:size': SECTION_SIZE,
+            'section:image-pos': 0,
+            'section/u-boot-dtb:offset': 4,
+            'section/u-boot-dtb:size': 636,
+            'section/u-boot-dtb:image-pos': 4,
+            'u-boot-spl-dtb:offset': SECTION_SIZE,
+            'u-boot-spl-dtb:size': DTB_SIZE,
+            'u-boot-spl-dtb:image-pos': SECTION_SIZE,
+            'u-boot-tpl-dtb:offset': SECTION_SIZE + DTB_SIZE,
+            'u-boot-tpl-dtb:image-pos': SECTION_SIZE + DTB_SIZE,
+            'u-boot-tpl-dtb:size': DTB_SIZE,
+            'fdtmap:offset': SECTION_SIZE + DTB_SIZE * 2,
+            'fdtmap:size': FDTMAP_SIZE,
+            'fdtmap:image-pos': SECTION_SIZE + DTB_SIZE * 2,
+        }
+        main_expected = {
+            'section:orig-size': SECTION_SIZE,
+            'section/u-boot-dtb:orig-offset': 4,
+        }
+
+        # We expect three device-tree files in the output, with the first one
+        # within a fixed-size section.
+        # Read them in sequence. We look for an 'spl' property in the SPL tree,
+        # and 'tpl' in the TPL tree, to make sure they are distinct from the
+        # main U-Boot tree. All three should have the same positions and offset
+        # except that the main tree should include the main_expected properties
+        start = 4
+        for item in ['', 'spl', 'tpl', None]:
+            if item is None:
+                start += 16  # Move past fdtmap header
+            dtb = fdt.Fdt.FromData(data[start:])
+            dtb.Scan()
+            props = self._GetPropTree(dtb,
+                BASE_DTB_PROPS + REPACK_DTB_PROPS + ['spl', 'tpl'],
+                prefix='/' if item is None else '/binman/')
+            expected = dict(base_expected)
+            if item:
+                expected[item] = 0
+            else:
+                # Main DTB and fdtdec should include the 'orig-' properties
+                expected.update(main_expected)
+            # Helpful for debugging:
+            #for prop in sorted(props):
+                #print('prop %s %s %s' % (prop, props[prop], expected[prop]))
+            self.assertEqual(expected, props)
+            if item == '':
+                start = SECTION_SIZE
+            else:
+                start += dtb._fdt_obj.totalsize()
+
+
 if __name__ == "__main__":
     unittest.main()
index 5185b68990a49cf0c86e9537cdd1691b25edef24..c81f7e3172ec9382b954299342392720784ca586 100644 (file)
@@ -36,19 +36,25 @@ class Image(section.Entry_section):
         image_node: Name of node containing the description for this image
         fdtmap_dtb: Fdt object for the fdtmap when loading from a file
         fdtmap_data: Contents of the fdtmap when loading from a file
+        allow_repack: True to add properties to allow the image to be safely
+            repacked later
 
     Args:
+        copy_to_orig: Copy offset/size to orig_offset/orig_size after reading
+            from the device tree
         test: True if this is being called from a test of Images. This this case
             there is no device tree defining the structure of the section, so
             we create a section manually.
     """
-    def __init__(self, name, node, test=False):
-        section.Entry_section.__init__(self, None, 'section', node, test)
+    def __init__(self, name, node, copy_to_orig=True, test=False):
+        section.Entry_section.__init__(self, None, 'section', node, test=test)
+        self.copy_to_orig = copy_to_orig
         self.name = 'main-section'
         self.image_name = name
         self._filename = '%s.bin' % self.image_name
         self.fdtmap_dtb = None
         self.fdtmap_data = None
+        self.allow_repack = False
         if not test:
             self.ReadNode()
 
@@ -57,6 +63,7 @@ class Image(section.Entry_section):
         filename = fdt_util.GetString(self._node, 'filename')
         if filename:
             self._filename = filename
+        self.allow_repack = fdt_util.GetBool(self._node, 'allow-repack')
 
     @classmethod
     def FromFile(cls, fname):
@@ -92,7 +99,7 @@ class Image(section.Entry_section):
 
         # Return an Image with the associated nodes
         root = dtb.GetRoot()
-        image = Image('image', root)
+        image = Image('image', root, copy_to_orig=False)
         image.image_node = fdt_util.GetString(root, 'image-node', 'image')
         image.fdtmap_dtb = dtb
         image.fdtmap_data = fdtmap_data
index 08e627985de399d832283ac8effb4955b83fedf7..2379e24ef6dff79af57c89b4c95e6be44f8cb56f 100644 (file)
@@ -226,7 +226,7 @@ def GetAllFdts():
         if dtb != main_dtb:
             yield dtb
 
-def GetUpdateNodes(node):
+def GetUpdateNodes(node, for_repack=False):
     """Yield all the nodes that need to be updated in all device trees
 
     The property referenced by this node is added to any device trees which
@@ -235,25 +235,31 @@ def GetUpdateNodes(node):
 
     Args:
         node: Node object in the main device tree to look up
+        for_repack: True if we want only nodes which need 'repack' properties
+            added to them (e.g. 'orig-offset'), False to return all nodes. We
+            don't add repack properties to SPL/TPL device trees.
 
     Yields:
         Node objects in each device tree that is in use (U-Boot proper, which
             is node, SPL and TPL)
     """
     yield node
-    for dtb, fname, _ in output_fdt_info.values():
+    for dtb, fname, entry in output_fdt_info.values():
         if dtb != node.GetFdt():
+            if for_repack and entry.etype != 'u-boot-dtb':
+                continue
             other_node = dtb.GetNode(fdt_path_prefix + node.path)
             if other_node:
                 yield other_node
 
-def AddZeroProp(node, prop):
+def AddZeroProp(node, prop, for_repack=False):
     """Add a new property to affected device trees with an integer value of 0.
 
     Args:
         prop_name: Name of property
+        for_repack: True is this property is only needed for repacking
     """
-    for n in GetUpdateNodes(node):
+    for n in GetUpdateNodes(node, for_repack):
         n.AddZeroProp(prop)
 
 def AddSubnode(node, name):
@@ -283,15 +289,18 @@ def AddString(node, prop, value):
     for n in GetUpdateNodes(node):
         n.AddString(prop, value)
 
-def SetInt(node, prop, value):
+def SetInt(node, prop, value, for_repack=False):
     """Update an integer property in affected device trees with an integer value
 
     This is not allowed to change the size of the FDT.
 
     Args:
         prop_name: Name of property
+        for_repack: True is this property is only needed for repacking
     """
-    for n in GetUpdateNodes(node):
+    for n in GetUpdateNodes(node, for_repack):
+        tout.Detail("File %s: Update node '%s' prop '%s' to %#x" %
+                    (node.GetFdt().name, node.path, prop, value))
         n.SetInt(prop, value)
 
 def CheckAddHashProp(node):
diff --git a/tools/binman/test/134_fdt_update_all_repack.dts b/tools/binman/test/134_fdt_update_all_repack.dts
new file mode 100644 (file)
index 0000000..625d376
--- /dev/null
@@ -0,0 +1,23 @@
+// SPDX-License-Identifier: GPL-2.0+
+/dts-v1/;
+
+/ {
+       #address-cells = <1>;
+       #size-cells = <1>;
+
+       binman {
+               allow-repack;
+               section {
+                       size = <0x300>;
+                       u-boot-dtb {
+                               offset = <4>;
+                       };
+               };
+               u-boot-spl-dtb {
+               };
+               u-boot-tpl-dtb {
+               };
+               fdtmap {
+               };
+       };
+};