binman: Allow updating entries that change size
So far we don't allow entries to change size when repacking. But this is not very useful since it is common for entries to change size after an updated binary is built, etc. Add support for this, respecting the original offset/size/alignment constraints of the image layout. For this to work the original image must have been created with the 'allow-repack' property. This does not support entry types with sub-entries such as files and CBFS, but it does support sections. Signed-off-by: Simon Glass <sjg@chromium.org>
This commit is contained in:
parent
eba1f0cc94
commit
51014aabc2
@ -589,7 +589,9 @@ that there is an 'fdtmap' entry in the image. For example:
|
||||
$ binman replace -i image.bin section/cbfs/u-boot
|
||||
|
||||
which will write the contents of the file 'u-boot' from the current directory
|
||||
to the that entry. The file must be the same size as the entry being replaced.
|
||||
to the that entry. If the entry size changes, you must add the 'allow-repack'
|
||||
property to the original image before generating it (see above), otherwise you
|
||||
will get an error.
|
||||
|
||||
|
||||
Logging
|
||||
|
@ -118,11 +118,11 @@ def ReadEntry(image_fname, entry_path, decomp=True):
|
||||
return entry.ReadData(decomp)
|
||||
|
||||
|
||||
def WriteEntry(image_fname, entry_path, data, decomp=True):
|
||||
def WriteEntry(image_fname, entry_path, data, decomp=True, allow_resize=True):
|
||||
"""Replace an entry in an image
|
||||
|
||||
This replaces the data in a particular entry in an image. This size of the
|
||||
new data must match the size of the old data
|
||||
new data must match the size of the old data unless allow_resize is True.
|
||||
|
||||
Args:
|
||||
image_fname: Image filename to process
|
||||
@ -130,18 +130,33 @@ def WriteEntry(image_fname, entry_path, data, decomp=True):
|
||||
data: Data to replace with
|
||||
decomp: True to compress the data if needed, False if data is
|
||||
already compressed so should be used as is
|
||||
allow_resize: True to allow entries to change size (this does a re-pack
|
||||
of the entries), False to raise an exception
|
||||
|
||||
Returns:
|
||||
Image object that was updated
|
||||
"""
|
||||
tout.Info("WriteEntry '%s', file '%s'" % (entry_path, image_fname))
|
||||
image = Image.FromFile(image_fname)
|
||||
entry = image.FindEntryPath(entry_path)
|
||||
state.PrepareFromLoadedData(image)
|
||||
image.LoadData()
|
||||
|
||||
# If repacking, drop the old offset/size values except for the original
|
||||
# ones, so we are only left with the constraints.
|
||||
if allow_resize:
|
||||
image.ResetForPack()
|
||||
tout.Info('Writing data to %s' % entry.GetPath())
|
||||
if not entry.WriteData(data, decomp):
|
||||
entry.Raise('Entry data size does not match, but resize is disabled')
|
||||
if not image.allow_repack:
|
||||
entry.Raise('Entry data size does not match, but allow-repack is not present for this image')
|
||||
if not allow_resize:
|
||||
entry.Raise('Entry data size does not match, but resize is disabled')
|
||||
tout.Info('Processing image')
|
||||
ProcessImage(image, update_fdt=True, write_map=False, get_contents=False)
|
||||
ProcessImage(image, update_fdt=True, write_map=False, get_contents=False,
|
||||
allow_resize=allow_resize)
|
||||
tout.Info('WriteEntry done')
|
||||
return image
|
||||
|
||||
|
||||
def ExtractEntries(image_fname, output_fname, outdir, entry_paths,
|
||||
@ -264,7 +279,8 @@ def PrepareImagesAndDtbs(dtb_fname, select_images, update_fdt):
|
||||
return images
|
||||
|
||||
|
||||
def ProcessImage(image, update_fdt, write_map, get_contents=True):
|
||||
def ProcessImage(image, update_fdt, write_map, get_contents=True,
|
||||
allow_resize=True):
|
||||
"""Perform all steps for this image, including checking and # writing it.
|
||||
|
||||
This means that errors found with a later image will be reported after
|
||||
@ -277,6 +293,8 @@ def ProcessImage(image, update_fdt, write_map, get_contents=True):
|
||||
write_map: True to write a map file
|
||||
get_contents: True to get the image contents from files, etc., False if
|
||||
the contents is already present
|
||||
allow_resize: True to allow entries to change size (this does a re-pack
|
||||
of the entries), False to raise an exception
|
||||
"""
|
||||
if get_contents:
|
||||
image.GetEntryContents()
|
||||
@ -309,6 +327,7 @@ def ProcessImage(image, update_fdt, write_map, get_contents=True):
|
||||
image.SetCalculatedProperties()
|
||||
for dtb_item in state.GetAllFdts():
|
||||
dtb_item.Sync()
|
||||
dtb_item.Flush()
|
||||
sizes_ok = image.ProcessEntryContents()
|
||||
if sizes_ok:
|
||||
break
|
||||
|
@ -96,10 +96,10 @@ class Entry_fdtmap(Entry):
|
||||
with fsw.add_node(subnode.name):
|
||||
_AddNode(subnode)
|
||||
|
||||
outfdt = self.GetImage().fdtmap_dtb
|
||||
data = state.GetFdtContents('fdtmap')[1]
|
||||
# If we have an fdtmap it means that we are using this as the
|
||||
# read-only fdtmap for this image.
|
||||
if not outfdt:
|
||||
# fdtmap for this image.
|
||||
if data is None:
|
||||
# Get the FDT data into an Fdt object
|
||||
data = state.GetFdtContents()[1]
|
||||
infdt = Fdt.FromData(data)
|
||||
@ -126,7 +126,8 @@ class Entry_fdtmap(Entry):
|
||||
# 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()
|
||||
data = outfdt.GetContents()
|
||||
data = FDTMAP_MAGIC + tools.GetBytes(0, 8) + data
|
||||
return data
|
||||
|
||||
def ObtainContents(self):
|
||||
|
@ -2724,7 +2724,8 @@ class TestFunctional(unittest.TestCase):
|
||||
self.assertEqual(len(U_BOOT_DATA), entry.contents_size)
|
||||
self.assertEqual(len(U_BOOT_DATA), entry.size)
|
||||
|
||||
def _RunReplaceCmd(self, entry_name, data, decomp=True):
|
||||
def _RunReplaceCmd(self, entry_name, data, decomp=True, allow_resize=True,
|
||||
dts='132_replace.dts'):
|
||||
"""Replace an entry in an image
|
||||
|
||||
This writes the entry data to update it, then opens the updated file and
|
||||
@ -2735,13 +2736,16 @@ class TestFunctional(unittest.TestCase):
|
||||
data: Data to replace it with
|
||||
decomp: True to compress the data if needed, False if data is
|
||||
already compressed so should be used as is
|
||||
allow_resize: True to allow entries to change size, False to raise
|
||||
an exception
|
||||
|
||||
Returns:
|
||||
Tuple:
|
||||
data from entry
|
||||
data from fdtmap (excluding header)
|
||||
Image object that was modified
|
||||
"""
|
||||
dtb_data = self._DoReadFileDtb('132_replace.dts', use_real_dtb=True,
|
||||
dtb_data = self._DoReadFileDtb(dts, use_real_dtb=True,
|
||||
update_dtb=True)[1]
|
||||
|
||||
self.assertIn('image', control.images)
|
||||
@ -2753,21 +2757,24 @@ class TestFunctional(unittest.TestCase):
|
||||
image_fname = tools.GetOutputFilename('image.bin')
|
||||
updated_fname = tools.GetOutputFilename('image-updated.bin')
|
||||
tools.WriteFile(updated_fname, tools.ReadFile(image_fname))
|
||||
control.WriteEntry(updated_fname, entry_name, data, decomp)
|
||||
image = control.WriteEntry(updated_fname, entry_name, data, decomp,
|
||||
allow_resize)
|
||||
data = control.ReadEntry(updated_fname, entry_name, decomp)
|
||||
|
||||
# The DT data should not change
|
||||
new_dtb_data = entries['u-boot-dtb'].data
|
||||
self.assertEqual(new_dtb_data, orig_dtb_data)
|
||||
new_fdtmap_data = entries['fdtmap'].data
|
||||
self.assertEqual(new_fdtmap_data, orig_fdtmap_data)
|
||||
# The DT data should not change unless resized:
|
||||
if not allow_resize:
|
||||
new_dtb_data = entries['u-boot-dtb'].data
|
||||
self.assertEqual(new_dtb_data, orig_dtb_data)
|
||||
new_fdtmap_data = entries['fdtmap'].data
|
||||
self.assertEqual(new_fdtmap_data, orig_fdtmap_data)
|
||||
|
||||
return data, orig_fdtmap_data[fdtmap.FDTMAP_HDR_LEN:]
|
||||
return data, orig_fdtmap_data[fdtmap.FDTMAP_HDR_LEN:], image
|
||||
|
||||
def testReplaceSimple(self):
|
||||
"""Test replacing a single file"""
|
||||
expected = b'x' * len(U_BOOT_DATA)
|
||||
data, expected_fdtmap = self._RunReplaceCmd('u-boot', expected)
|
||||
data, expected_fdtmap, _ = self._RunReplaceCmd('u-boot', expected,
|
||||
allow_resize=False)
|
||||
self.assertEqual(expected, data)
|
||||
|
||||
# Test that the state looks right. There should be an FDT for the fdtmap
|
||||
@ -2775,7 +2782,7 @@ class TestFunctional(unittest.TestCase):
|
||||
# 'control' tables. Checking for an FDT that does not exist should
|
||||
# return None.
|
||||
path, fdtmap = state.GetFdtContents('fdtmap')
|
||||
self.assertIsNone(path)
|
||||
self.assertIsNotNone(path)
|
||||
self.assertEqual(expected_fdtmap, fdtmap)
|
||||
|
||||
dtb = state.GetFdtForEtype('fdtmap')
|
||||
@ -2794,7 +2801,8 @@ class TestFunctional(unittest.TestCase):
|
||||
"""Test replacing a file by something larger"""
|
||||
expected = U_BOOT_DATA + b'x'
|
||||
with self.assertRaises(ValueError) as e:
|
||||
self._RunReplaceCmd('u-boot', expected)
|
||||
self._RunReplaceCmd('u-boot', expected, allow_resize=False,
|
||||
dts='139_replace_repack.dts')
|
||||
self.assertIn("Node '/u-boot': Entry data size does not match, but resize is disabled",
|
||||
str(e.exception))
|
||||
|
||||
@ -2806,7 +2814,8 @@ class TestFunctional(unittest.TestCase):
|
||||
updated_fname = tools.GetOutputFilename('image-updated.bin')
|
||||
tools.WriteFile(updated_fname, data)
|
||||
entry_name = 'u-boot'
|
||||
control.WriteEntry(updated_fname, entry_name, expected)
|
||||
control.WriteEntry(updated_fname, entry_name, expected,
|
||||
allow_resize=False)
|
||||
data = control.ReadEntry(updated_fname, entry_name)
|
||||
self.assertEqual(expected, data)
|
||||
|
||||
@ -2818,7 +2827,8 @@ class TestFunctional(unittest.TestCase):
|
||||
updated_fname = tools.GetOutputFilename('first-updated.bin')
|
||||
tools.WriteFile(updated_fname, tools.ReadFile(image_fname))
|
||||
entry_name = 'u-boot'
|
||||
control.WriteEntry(updated_fname, entry_name, expected)
|
||||
control.WriteEntry(updated_fname, entry_name, expected,
|
||||
allow_resize=False)
|
||||
data = control.ReadEntry(updated_fname, entry_name)
|
||||
self.assertEqual(expected, data)
|
||||
|
||||
@ -2911,6 +2921,37 @@ class TestFunctional(unittest.TestCase):
|
||||
"""Test an image header at the end of an image with undefined size"""
|
||||
self._DoReadFileRealDtb('138_fdtmap_hdr_nosize.dts')
|
||||
|
||||
def testReplaceResize(self):
|
||||
"""Test replacing a single file in an entry with a larger file"""
|
||||
expected = U_BOOT_DATA + b'x'
|
||||
data, _, image = self._RunReplaceCmd('u-boot', expected,
|
||||
dts='139_replace_repack.dts')
|
||||
self.assertEqual(expected, data)
|
||||
|
||||
entries = image.GetEntries()
|
||||
dtb_data = entries['u-boot-dtb'].data
|
||||
dtb = fdt.Fdt.FromData(dtb_data)
|
||||
dtb.Scan()
|
||||
|
||||
# The u-boot section should now be larger in the dtb
|
||||
node = dtb.GetNode('/binman/u-boot')
|
||||
self.assertEqual(len(expected), fdt_util.GetInt(node, 'size'))
|
||||
|
||||
# Same for the fdtmap
|
||||
fdata = entries['fdtmap'].data
|
||||
fdtb = fdt.Fdt.FromData(fdata[fdtmap.FDTMAP_HDR_LEN:])
|
||||
fdtb.Scan()
|
||||
fnode = fdtb.GetNode('/u-boot')
|
||||
self.assertEqual(len(expected), fdt_util.GetInt(fnode, 'size'))
|
||||
|
||||
def testReplaceResizeNoRepack(self):
|
||||
"""Test replacing an entry with a larger file when not allowed"""
|
||||
expected = U_BOOT_DATA + b'x'
|
||||
with self.assertRaises(ValueError) as e:
|
||||
self._RunReplaceCmd('u-boot', expected)
|
||||
self.assertIn('Entry data size does not match, but allow-repack is not present for this image',
|
||||
str(e.exception))
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
||||
|
@ -97,12 +97,13 @@ class Image(section.Entry_section):
|
||||
fdt_data = fdtmap_data[fdtmap.FDTMAP_HDR_LEN:]
|
||||
out_fname = tools.GetOutputFilename('fdtmap.in.dtb')
|
||||
tools.WriteFile(out_fname, fdt_data)
|
||||
dtb = fdt.Fdt.FromData(fdt_data, out_fname)
|
||||
dtb = fdt.Fdt(out_fname)
|
||||
dtb.Scan()
|
||||
|
||||
# Return an Image with the associated nodes
|
||||
root = dtb.GetRoot()
|
||||
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
|
||||
|
@ -249,6 +249,7 @@ def GetUpdateNodes(node, for_repack=False):
|
||||
if for_repack and entry.etype != 'u-boot-dtb':
|
||||
continue
|
||||
other_node = dtb.GetNode(fdt_path_prefix + node.path)
|
||||
#print(' try', fdt_path_prefix + node.path, other_node)
|
||||
if other_node:
|
||||
yield other_node
|
||||
|
||||
@ -300,7 +301,7 @@ def SetInt(node, prop, value, for_repack=False):
|
||||
"""
|
||||
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.GetFdt().name, n.path, prop, value))
|
||||
n.SetInt(prop, value)
|
||||
|
||||
def CheckAddHashProp(node):
|
||||
|
22
tools/binman/test/139_replace_repack.dts
Normal file
22
tools/binman/test/139_replace_repack.dts
Normal file
@ -0,0 +1,22 @@
|
||||
// SPDX-License-Identifier: GPL-2.0+
|
||||
|
||||
/dts-v1/;
|
||||
|
||||
/ {
|
||||
#address-cells = <1>;
|
||||
#size-cells = <1>;
|
||||
|
||||
binman {
|
||||
size = <0xc00>;
|
||||
allow-repack;
|
||||
u-boot {
|
||||
};
|
||||
fdtmap {
|
||||
};
|
||||
u-boot-dtb {
|
||||
};
|
||||
image-header {
|
||||
location = "end";
|
||||
};
|
||||
};
|
||||
};
|
Loading…
Reference in New Issue
Block a user