[PATCH] binman: Support updating section contents

Simon Glass sjg at chromium.org
Sat Feb 4 23:24:13 CET 2023


Implement this feature since it is useful for updating FITs within an
image.

Signed-off-by: Simon Glass <sjg at chromium.org>
---

 tools/binman/binman.rst       | 16 ++++++++++
 tools/binman/etype/section.py | 28 ++++++++++++-----
 tools/binman/ftest.py         | 58 ++++++++++++++++++++++++++++++++---
 3 files changed, 90 insertions(+), 12 deletions(-)

diff --git a/tools/binman/binman.rst b/tools/binman/binman.rst
index 03a99a19bc6..ece2c05128a 100644
--- a/tools/binman/binman.rst
+++ b/tools/binman/binman.rst
@@ -1327,6 +1327,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/etype/section.py b/tools/binman/etype/section.py
index 57b91ff726c..1c03d357201 100644
--- a/tools/binman/etype/section.py
+++ b/tools/binman/etype/section.py
@@ -168,6 +168,9 @@ class Entry_section(Entry):
         self._end_4gb = False
         self._ignore_missing = False
         self._filename = None
+        # Indicates that self.data should be used as is, without calling
+        # BuildSectionData()
+        self._build_done = False
 
     def IsSpecialSubnode(self, node):
         """Check if a node is a special one used by the section itself
@@ -397,10 +400,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 +433,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()
 
@@ -866,7 +875,12 @@ 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._build_done = True
+        return ok
 
     def WriteChildData(self, child):
         return True
diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
index 6b203dfb644..2ec4864290a 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):
-- 
2.39.1.519.gcb327c4b5f-goog



More information about the U-Boot mailing list