[PATCH v2 3/4] binman: Support updating section contents
Simon Glass
sjg at chromium.org
Fri Mar 3 01:02:44 CET 2023
Implement this feature since it is useful for updating FITs within an
image.
Signed-off-by: Simon Glass <sjg at chromium.org>
---
Changes in v2:
- Update to support more replacement cases
tools/binman/binman.rst | 16 ++
tools/binman/control.py | 2 +
tools/binman/entry.py | 22 +++
tools/binman/etype/atf_fip.py | 2 +-
tools/binman/etype/cbfs.py | 2 +-
tools/binman/etype/fit.py | 5 +
tools/binman/etype/section.py | 30 +++-
tools/binman/ftest.py | 137 +++++++++++++++++-
tools/binman/test/277_replace_fit_sibling.dts | 61 ++++++++
.../binman/test/278_replace_section_deep.dts | 25 ++++
10 files changed, 287 insertions(+), 15 deletions(-)
create mode 100644 tools/binman/test/277_replace_fit_sibling.dts
create mode 100644 tools/binman/test/278_replace_section_deep.dts
diff --git a/tools/binman/binman.rst b/tools/binman/binman.rst
index 55ef317a691..717780a39ef 100644
--- a/tools/binman/binman.rst
+++ b/tools/binman/binman.rst
@@ -1326,6 +1326,22 @@ You can also replace just a selection of entries::
$ binman replace -i image.bin "*u-boot*" -I indir
+It is possible to replace whole sections as well, but in that case any
+information about entries within the section may become outdated. This is
+because Binman cannot know whether things have moved around or resized within
+the section, once you have updated its data.
+
+Technical note: With 'allow-repack', Binman writes information about the
+original offset and size properties of each entry, if any were specified, in
+the 'orig-offset' and 'orig-size' properties. This allows Binman to distinguish
+between an entry which ended up being packed at an offset (or assigned a size)
+and an entry which had a particular offset / size requested in the Binman
+configuration. Where are particular offset / size was requested, this is treated
+as set in stone, so Binman will ensure it doesn't change. Without this feature,
+repacking an entry might cause it to disobey the original constraints provided
+when it was created.
+
+ Repacking an image involves
.. _`BinmanLogging`:
diff --git a/tools/binman/control.py b/tools/binman/control.py
index f4e3898b34c..803ed5f01d5 100644
--- a/tools/binman/control.py
+++ b/tools/binman/control.py
@@ -402,6 +402,8 @@ def ReplaceEntries(image_fname, input_fname, indir, entry_paths,
image_fname = os.path.abspath(image_fname)
image = Image.FromFile(image_fname)
+ image.mark_build_done()
+
# Replace an entry from a single file, as a special case
if input_fname:
if not entry_paths:
diff --git a/tools/binman/entry.py b/tools/binman/entry.py
index 5eacc5fa6c4..07209673f22 100644
--- a/tools/binman/entry.py
+++ b/tools/binman/entry.py
@@ -100,6 +100,10 @@ class Entry(object):
appear in the map
optional (bool): True if this entry contains an optional external blob
overlap (bool): True if this entry overlaps with others
+ build_done (bool): Indicates that the entry data has been built and does
+ not need to be done again. This is only used with 'binman replace',
+ to stop sections from being rebuilt if their entries have not been
+ replaced
"""
fake_dir = None
@@ -148,6 +152,7 @@ class Entry(object):
self.overlap = False
self.elf_base_sym = None
self.offset_from_elf = None
+ self.build_done = False
@staticmethod
def FindEntryClass(etype, expanded):
@@ -1006,6 +1011,7 @@ features to produce new behaviours.
else:
self.contents_size = self.pre_reset_size
ok = self.ProcessContentsUpdate(data)
+ self.build_done = False
self.Detail('WriteData: size=%x, ok=%s' % (len(data), ok))
section_ok = self.section.WriteChildData(self)
return ok and section_ok
@@ -1027,6 +1033,14 @@ features to produce new behaviours.
True if the section could be updated successfully, False if the
data is such that the section could not update
"""
+ self.build_done = False
+ entry = self.section
+
+ # Now we must rebuild all sections above this one
+ while entry and entry != entry.section:
+ self.build_done = False
+ entry = entry.section
+
return True
def GetSiblingOrder(self):
@@ -1349,3 +1363,11 @@ features to produce new behaviours.
val = elf.GetSymbolOffset(entry.elf_fname, sym_name,
entry.elf_base_sym)
return val + offset
+
+ def mark_build_done(self):
+ """Mark an entry as already built"""
+ self.build_done = True
+ entries = self.GetEntries()
+ if entries:
+ for entry in entries.values():
+ entry.mark_build_done()
diff --git a/tools/binman/etype/atf_fip.py b/tools/binman/etype/atf_fip.py
index 6ecd95b71f9..c44ee4aa53d 100644
--- a/tools/binman/etype/atf_fip.py
+++ b/tools/binman/etype/atf_fip.py
@@ -270,4 +270,4 @@ class Entry_atf_fip(Entry_section):
# Recreate the data structure, leaving the data for this child alone,
# so that child.data is used to pack into the FIP.
self.ObtainContents(skip_entry=child)
- return True
+ return super().WriteChildData(child)
diff --git a/tools/binman/etype/cbfs.py b/tools/binman/etype/cbfs.py
index 832f8d038f0..575aa624f6c 100644
--- a/tools/binman/etype/cbfs.py
+++ b/tools/binman/etype/cbfs.py
@@ -295,7 +295,7 @@ class Entry_cbfs(Entry):
# Recreate the data structure, leaving the data for this child alone,
# so that child.data is used to pack into the FIP.
self.ObtainContents(skip_entry=child)
- return True
+ return super().WriteChildData(child)
def AddBintools(self, btools):
super().AddBintools(btools)
diff --git a/tools/binman/etype/fit.py b/tools/binman/etype/fit.py
index be0cdcc900f..76687492f8a 100644
--- a/tools/binman/etype/fit.py
+++ b/tools/binman/etype/fit.py
@@ -777,6 +777,8 @@ class Entry_fit(Entry_section):
Args:
image_pos (int): Position of this entry in the image
"""
+ if self.build_done:
+ return
super().SetImagePos(image_pos)
# If mkimage is missing we'll have empty data,
@@ -830,3 +832,6 @@ class Entry_fit(Entry_section):
# missing
for entry in self._priv_entries.values():
entry.CheckMissing(missing_list)
+
+ def CheckEntries(self):
+ pass
diff --git a/tools/binman/etype/section.py b/tools/binman/etype/section.py
index 57b91ff726c..6565348e36e 100644
--- a/tools/binman/etype/section.py
+++ b/tools/binman/etype/section.py
@@ -397,10 +397,13 @@ class Entry_section(Entry):
This excludes any padding. If the section is compressed, the
compressed data is returned
"""
- data = self.BuildSectionData(required)
- if data is None:
- return None
- self.SetContents(data)
+ if not self.build_done:
+ data = self.BuildSectionData(required)
+ if data is None:
+ return None
+ self.SetContents(data)
+ else:
+ data = self.data
if self._filename:
tools.write_file(tools.get_output_filename(self._filename), data)
return data
@@ -427,8 +430,11 @@ class Entry_section(Entry):
self._SortEntries()
self._extend_entries()
- data = self.BuildSectionData(True)
- self.SetContents(data)
+ if self.build_done:
+ self.size = None
+ else:
+ data = self.BuildSectionData(True)
+ self.SetContents(data)
self.CheckSize()
@@ -810,6 +816,9 @@ class Entry_section(Entry):
def LoadData(self, decomp=True):
for entry in self._entries.values():
entry.LoadData(decomp)
+ data = self.ReadData(decomp)
+ self.contents_size = len(data)
+ self.ProcessContentsUpdate(data)
self.Detail('Loaded data')
def GetImage(self):
@@ -866,10 +875,15 @@ class Entry_section(Entry):
return data
def WriteData(self, data, decomp=True):
- self.Raise("Replacing sections is not implemented yet")
+ ok = super().WriteData(data, decomp)
+
+ # The section contents are now fixed and cannot be rebuilt from the
+ # containing entries.
+ self.mark_build_done()
+ return ok
def WriteChildData(self, child):
- return True
+ return super().WriteChildData(child)
def SetAllowMissing(self, allow_missing):
"""Set whether a section allows missing external blobs
diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
index b4b5019f90c..e732e07c781 100644
--- a/tools/binman/ftest.py
+++ b/tools/binman/ftest.py
@@ -5819,13 +5819,61 @@ fdt fdtmap Extract the devicetree blob from the fdtmap
self.assertEqual(expected_fdtmap, fdtmap)
def testReplaceSectionSimple(self):
- """Test replacing a simple section with arbitrary data"""
+ """Test replacing a simple section with same-sized data"""
new_data = b'w' * len(COMPRESS_DATA + U_BOOT_DATA)
- with self.assertRaises(ValueError) as exc:
- self._RunReplaceCmd('section', new_data,
- dts='241_replace_section_simple.dts')
+ data, expected_fdtmap, image = self._RunReplaceCmd('section',
+ new_data, dts='241_replace_section_simple.dts')
+ self.assertEqual(new_data, data)
+
+ entries = image.GetEntries()
+ self.assertIn('section', entries)
+ entry = entries['section']
+ self.assertEqual(len(new_data), entry.size)
+
+ def testReplaceSectionLarger(self):
+ """Test replacing a simple section with larger data"""
+ new_data = b'w' * (len(COMPRESS_DATA + U_BOOT_DATA) + 1)
+ data, expected_fdtmap, image = self._RunReplaceCmd('section',
+ new_data, dts='241_replace_section_simple.dts')
+ self.assertEqual(new_data, data)
+
+ entries = image.GetEntries()
+ self.assertIn('section', entries)
+ entry = entries['section']
+ self.assertEqual(len(new_data), entry.size)
+ fentry = entries['fdtmap']
+ self.assertEqual(entry.offset + entry.size, fentry.offset)
+
+ def testReplaceSectionSmaller(self):
+ """Test replacing a simple section with smaller data"""
+ new_data = b'w' * (len(COMPRESS_DATA + U_BOOT_DATA) - 1) + b'\0'
+ data, expected_fdtmap, image = self._RunReplaceCmd('section',
+ new_data, dts='241_replace_section_simple.dts')
+ self.assertEqual(new_data, data)
+
+ # The new size is the same as the old, just with a pad byte at the end
+ entries = image.GetEntries()
+ self.assertIn('section', entries)
+ entry = entries['section']
+ self.assertEqual(len(new_data), entry.size)
+
+ def testReplaceSectionSmallerAllow(self):
+ """Test failing to replace a simple section with smaller data"""
+ new_data = b'w' * (len(COMPRESS_DATA + U_BOOT_DATA) - 1)
+ try:
+ state.SetAllowEntryContraction(True)
+ with self.assertRaises(ValueError) as exc:
+ self._RunReplaceCmd('section', new_data,
+ dts='241_replace_section_simple.dts')
+ finally:
+ state.SetAllowEntryContraction(False)
+
+ # Since we have no information about the position of things within the
+ # section, we cannot adjust the position of /section-u-boot so it ends
+ # up outside the section
self.assertIn(
- "Node '/section': Replacing sections is not implemented yet",
+ "Node '/section/u-boot': Offset 0x24 (36) size 0x4 (4) is outside "
+ "the section '/section' starting at 0x0 (0) of size 0x27 (39)",
str(exc.exception))
def testMkimageImagename(self):
@@ -6394,6 +6442,85 @@ fdt fdtmap Extract the devicetree blob from the fdtmap
self.assertEqual(['u-boot', 'atf-2'],
fdt_util.GetStringList(node, 'loadables'))
+ def testReplaceSectionEntry(self):
+ """Test replacing an entry in a section"""
+ expect_data = b'w' * len(U_BOOT_DATA + COMPRESS_DATA)
+ entry_data, expected_fdtmap, image = self._RunReplaceCmd('section/blob',
+ expect_data, dts='241_replace_section_simple.dts')
+ self.assertEqual(expect_data, entry_data)
+
+ entries = image.GetEntries()
+ self.assertIn('section', entries)
+ section = entries['section']
+
+ sect_entries = section.GetEntries()
+ self.assertIn('blob', sect_entries)
+ entry = sect_entries['blob']
+ self.assertEqual(len(expect_data), entry.size)
+
+ fname = tools.get_output_filename('image-updated.bin')
+ data = tools.read_file(fname)
+
+ new_blob_data = data[entry.image_pos:entry.image_pos + len(expect_data)]
+ self.assertEqual(expect_data, new_blob_data)
+
+ self.assertEqual(U_BOOT_DATA,
+ data[entry.image_pos + len(expect_data):]
+ [:len(U_BOOT_DATA)])
+
+ def testReplaceSectionDeep(self):
+ """Test replacing an entry in two levels of sections"""
+ expect_data = b'w' * len(U_BOOT_DATA + COMPRESS_DATA)
+ entry_data, expected_fdtmap, image = self._RunReplaceCmd(
+ 'section/section/blob', expect_data,
+ dts='278_replace_section_deep.dts')
+ self.assertEqual(expect_data, entry_data)
+
+ entries = image.GetEntries()
+ self.assertIn('section', entries)
+ section = entries['section']
+
+ subentries = section.GetEntries()
+ self.assertIn('section', subentries)
+ section = subentries['section']
+
+ sect_entries = section.GetEntries()
+ self.assertIn('blob', sect_entries)
+ entry = sect_entries['blob']
+ self.assertEqual(len(expect_data), entry.size)
+
+ fname = tools.get_output_filename('image-updated.bin')
+ data = tools.read_file(fname)
+
+ new_blob_data = data[entry.image_pos:entry.image_pos + len(expect_data)]
+ self.assertEqual(expect_data, new_blob_data)
+
+ self.assertEqual(U_BOOT_DATA,
+ data[entry.image_pos + len(expect_data):]
+ [:len(U_BOOT_DATA)])
+
+ def testReplaceFitSibling(self):
+ """Test an image with a FIT inside where we replace its sibling"""
+ fname = TestFunctional._MakeInputFile('once', b'available once')
+ self._DoReadFileRealDtb('277_replace_fit_sibling.dts')
+ os.remove(fname)
+
+ try:
+ tmpdir, updated_fname = self._SetupImageInTmpdir()
+
+ fname = os.path.join(tmpdir, 'update-blob')
+ expected = b'w' * (len(COMPRESS_DATA + U_BOOT_DATA) + 1)
+ tools.write_file(fname, expected)
+
+ self._DoBinman('replace', '-i', updated_fname, 'blob', '-f', fname)
+ data = tools.read_file(updated_fname)
+ start = len(U_BOOT_DTB_DATA)
+ self.assertEqual(expected, data[start:start + len(expected)])
+ map_fname = os.path.join(tmpdir, 'image-updated.map')
+ self.assertFalse(os.path.exists(map_fname))
+ finally:
+ shutil.rmtree(tmpdir)
+
if __name__ == "__main__":
unittest.main()
diff --git a/tools/binman/test/277_replace_fit_sibling.dts b/tools/binman/test/277_replace_fit_sibling.dts
new file mode 100644
index 00000000000..fc941a80816
--- /dev/null
+++ b/tools/binman/test/277_replace_fit_sibling.dts
@@ -0,0 +1,61 @@
+// SPDX-License-Identifier: GPL-2.0+
+/dts-v1/;
+
+/ {
+ binman {
+ allow-repack;
+
+ u-boot {
+ };
+
+ blob {
+ filename = "compress";
+ };
+
+ fit {
+ description = "test-desc";
+ #address-cells = <1>;
+
+ images {
+ kernel {
+ description = "Vanilla Linux kernel";
+ type = "kernel";
+ arch = "ppc";
+ os = "linux";
+ compression = "gzip";
+ load = <00000000>;
+ entry = <00000000>;
+ hash-1 {
+ algo = "crc32";
+ };
+ blob-ext {
+ filename = "once";
+ };
+ };
+ fdt-1 {
+ description = "Flattened Device Tree blob";
+ type = "flat_dt";
+ arch = "ppc";
+ compression = "none";
+ hash-1 {
+ algo = "crc32";
+ };
+ u-boot-spl-dtb {
+ };
+ };
+ };
+
+ configurations {
+ default = "conf-1";
+ conf-1 {
+ description = "Boot Linux kernel with FDT blob";
+ kernel = "kernel";
+ fdt = "fdt-1";
+ };
+ };
+ };
+
+ fdtmap {
+ };
+ };
+};
diff --git a/tools/binman/test/278_replace_section_deep.dts b/tools/binman/test/278_replace_section_deep.dts
new file mode 100644
index 00000000000..fba2d7dcf28
--- /dev/null
+++ b/tools/binman/test/278_replace_section_deep.dts
@@ -0,0 +1,25 @@
+// SPDX-License-Identifier: GPL-2.0+
+/dts-v1/;
+
+/ {
+ binman {
+ allow-repack;
+
+ u-boot-dtb {
+ };
+
+ section {
+ section {
+ blob {
+ filename = "compress";
+ };
+ };
+
+ u-boot {
+ };
+ };
+
+ fdtmap {
+ };
+ };
+};
--
2.40.0.rc0.216.gc4246ad0f0-goog
More information about the U-Boot
mailing list