diff --git a/tools/binman/README b/tools/binman/README index 1f9e13784fc496e58ca9b64c034d07a5aff75c79..af2a23147195c0658b35efcf77cabafc9de8e2a7 100644 --- a/tools/binman/README +++ b/tools/binman/README @@ -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 diff --git a/tools/binman/control.py b/tools/binman/control.py index f9680e3948d2c8465d6c3e3adeeeb749c619f373..c3f358d45c40a2e5afaa0047aeb8bfbce86f2e0a 100644 --- a/tools/binman/control.py +++ b/tools/binman/control.py @@ -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 diff --git a/tools/binman/etype/fdtmap.py b/tools/binman/etype/fdtmap.py index ff3f1ae812969f820a1564a532f7d03d387b2a86..b1810b9ddb1152a54fa91723bd712d7745fb27b4 100644 --- a/tools/binman/etype/fdtmap.py +++ b/tools/binman/etype/fdtmap.py @@ -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): diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index bc751893edbdc6df6efb18cec3d39f7f13f159c2..64c6c0abae1b9adefba69a7636390fe754886632 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -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() diff --git a/tools/binman/image.py b/tools/binman/image.py index fd4f50449297dd8cb1e264ace66fbc4aeccc3eb7..7b39a1ddcecc47396d313cf58519ffd227a07684 100644 --- a/tools/binman/image.py +++ b/tools/binman/image.py @@ -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 diff --git a/tools/binman/state.py b/tools/binman/state.py index 2379e24ef6dff79af57c89b4c95e6be44f8cb56f..65536151b44adf83891b585e0e6bda49600330ac 100644 --- a/tools/binman/state.py +++ b/tools/binman/state.py @@ -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): diff --git a/tools/binman/test/139_replace_repack.dts b/tools/binman/test/139_replace_repack.dts new file mode 100644 index 0000000000000000000000000000000000000000..a3daf6f9b4615930dc2492378c1a7f12cca333f7 --- /dev/null +++ b/tools/binman/test/139_replace_repack.dts @@ -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"; + }; + }; +};