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

Signed-off-by: Simon Glass sjg@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):