binman: Add info to allow safely repacking an image later

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>
This commit is contained in:
Simon Glass 2019-07-20 12:23:51 -06:00
parent 10f9d0066b
commit 12bb1a99c2
8 changed files with 165 additions and 14 deletions

View 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
-------------------

View 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

View 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)

View 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)

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

View 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

View 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):

View File

@ -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 {
};
};
};