From: Stefan Herbrechtsmeier stefan.herbrechtsmeier@weidmueller.com

Skip tests which requires python elftools if the tool is not available.

Signed-off-by: Stefan Herbrechtsmeier stefan.herbrechtsmeier@weidmueller.com Reviewed-by: Simon Glass sjg@chromium.org

---

(no changes since v2)

Changes in v2: - Add commit to skip elf tests

tools/binman/elf_test.py | 14 ++++++++++++++ tools/binman/ftest.py | 18 ++++++++++++++++++ 2 files changed, 32 insertions(+)

diff --git a/tools/binman/elf_test.py b/tools/binman/elf_test.py index 5a51c64cfe..75b867c2be 100644 --- a/tools/binman/elf_test.py +++ b/tools/binman/elf_test.py @@ -122,6 +122,8 @@ class TestElf(unittest.TestCase):

def testOutsideFile(self): """Test a symbol which extends outside the entry area is detected""" + if not elf.ELF_TOOLS: + self.skipTest('Python elftools not available') entry = FakeEntry(10) section = FakeSection() elf_fname = self.ElfTestFile('u_boot_binman_syms') @@ -147,6 +149,8 @@ class TestElf(unittest.TestCase): Only 32 and 64 bits are supported, since we need to store an offset into the image. """ + if not elf.ELF_TOOLS: + self.skipTest('Python elftools not available') entry = FakeEntry(10) section = FakeSection() elf_fname =self.ElfTestFile('u_boot_binman_syms_size') @@ -161,6 +165,8 @@ class TestElf(unittest.TestCase): This should produce -1 values for all thress symbols, taking up the first 16 bytes of the image. """ + if not elf.ELF_TOOLS: + self.skipTest('Python elftools not available') entry = FakeEntry(28) section = FakeSection(sym_value=None) elf_fname = self.ElfTestFile('u_boot_binman_syms') @@ -172,6 +178,8 @@ class TestElf(unittest.TestCase):

def testDebug(self): """Check that enabling debug in the elf module produced debug output""" + if not elf.ELF_TOOLS: + self.skipTest('Python elftools not available') try: tout.init(tout.DEBUG) entry = FakeEntry(24) @@ -281,6 +289,8 @@ class TestElf(unittest.TestCase):

def test_read_segments_bad_data(self): """Test for read_loadable_segments() with an invalid ELF file""" + if not elf.ELF_TOOLS: + self.skipTest('Python elftools not available') fname = self.ElfTestFile('embed_data') with self.assertRaises(ValueError) as e: elf.read_loadable_segments(tools.get_bytes(100, 100)) @@ -288,6 +298,8 @@ class TestElf(unittest.TestCase):

def test_get_file_offset(self): """Test GetFileOffset() gives the correct file offset for a symbol""" + if not elf.ELF_TOOLS: + self.skipTest('Python elftools not available') fname = self.ElfTestFile('embed_data') syms = elf.GetSymbols(fname, ['embed']) addr = syms['embed'].address @@ -314,6 +326,8 @@ class TestElf(unittest.TestCase):

def test_get_symbol_from_address(self): """Test GetSymbolFromAddress()""" + if not elf.ELF_TOOLS: + self.skipTest('Python elftools not available') fname = self.ElfTestFile('elf_sections') sym_name = 'calculate' syms = elf.GetSymbols(fname, [sym_name]) diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index fa1f421c05..da9aa9e679 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -4807,6 +4807,8 @@ class TestFunctional(unittest.TestCase):

def testUpdateFdtInElf(self): """Test that we can update the devicetree in an ELF file""" + if not elf.ELF_TOOLS: + self.skipTest('Python elftools not available') infile = elf_fname = self.ElfTestFile('u_boot_binman_embed') outfile = os.path.join(self._indir, 'u-boot.out') begin_sym = 'dtb_embed_begin' @@ -4858,6 +4860,8 @@ class TestFunctional(unittest.TestCase):

def testUpdateFdtInElfNoSyms(self): """Test that missing symbols are detected with --update-fdt-in-elf""" + if not elf.ELF_TOOLS: + self.skipTest('Python elftools not available') infile = elf_fname = self.ElfTestFile('u_boot_binman_embed') outfile = '' begin_sym = 'wrong_begin' @@ -4871,6 +4875,8 @@ class TestFunctional(unittest.TestCase):

def testUpdateFdtInElfTooSmall(self): """Test that an over-large dtb is detected with --update-fdt-in-elf""" + if not elf.ELF_TOOLS: + self.skipTest('Python elftools not available') infile = elf_fname = self.ElfTestFile('u_boot_binman_embed_sm') outfile = os.path.join(self._indir, 'u-boot.out') begin_sym = 'dtb_embed_begin' @@ -5344,6 +5350,8 @@ fdt fdtmap Extract the devicetree blob from the fdtmap

def testFitSplitElf(self): """Test an image with an FIT with an split-elf operation""" + if not elf.ELF_TOOLS: + self.skipTest('Python elftools not available') entry_args = { 'of-list': 'test-fdt1 test-fdt2', 'default-dt': 'test-fdt2', @@ -5421,6 +5429,8 @@ fdt fdtmap Extract the devicetree blob from the fdtmap

def testFitSplitElfBadElf(self): """Test a FIT split-elf operation with an invalid ELF file""" + if not elf.ELF_TOOLS: + self.skipTest('Python elftools not available') TestFunctional._MakeInputFile('bad.elf', tools.get_bytes(100, 100)) entry_args = { 'of-list': 'test-fdt1 test-fdt2', @@ -5440,6 +5450,8 @@ fdt fdtmap Extract the devicetree blob from the fdtmap

def testFitSplitElfBadDirective(self): """Test a FIT split-elf invalid fit,xxx directive in an image node""" + if not elf.ELF_TOOLS: + self.skipTest('Python elftools not available') err = self._check_bad_fit('227_fit_bad_dir.dts') self.assertIn( "Node '/binman/fit': subnode 'images/@atf-SEQ': Unknown directive 'fit,something'", @@ -5447,6 +5459,8 @@ fdt fdtmap Extract the devicetree blob from the fdtmap

def testFitSplitElfBadDirectiveConfig(self): """Test a FIT split-elf with invalid fit,xxx directive in config""" + if not elf.ELF_TOOLS: + self.skipTest('Python elftools not available') err = self._check_bad_fit('228_fit_bad_dir_config.dts') self.assertEqual( "Node '/binman/fit': subnode 'configurations/@config-SEQ': Unknown directive 'fit,config'", @@ -5470,6 +5484,8 @@ fdt fdtmap Extract the devicetree blob from the fdtmap

def testFitSplitElfMissing(self): """Test an split-elf FIT with a missing ELF file""" + if not elf.ELF_TOOLS: + self.skipTest('Python elftools not available') err = self.checkFitSplitElf(allow_missing=True) self.assertRegex( err, @@ -5477,6 +5493,8 @@ fdt fdtmap Extract the devicetree blob from the fdtmap

def testFitSplitElfFaked(self): """Test an split-elf FIT with faked ELF file""" + if not elf.ELF_TOOLS: + self.skipTest('Python elftools not available') err = self.checkFitSplitElf(allow_missing=True, allow_fake_blobs=True) self.assertRegex( err,

From: Stefan Herbrechtsmeier stefan.herbrechtsmeier@weidmueller.com

Avoid duplicate entries in the list of bintools used by the image and the list of missing bintools.

Signed-off-by: Stefan Herbrechtsmeier stefan.herbrechtsmeier@weidmueller.com

---

(no changes since v5)

Changes in v5: - Add commit to avoid duplicates in bintool lists

tools/binman/entry.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/tools/binman/entry.py b/tools/binman/entry.py index e3767aefa7..8f846e55b6 100644 --- a/tools/binman/entry.py +++ b/tools/binman/entry.py @@ -1047,7 +1047,8 @@ features to produce new behaviours. Args: bintool (Bintool): Bintool that was missing """ - self.missing_bintools.append(bintool) + if bintool not in self.missing_bintools: + self.missing_bintools.append(bintool)

def check_missing_bintools(self, missing_list): """Check if any entries in this section have missing bintools @@ -1057,7 +1058,10 @@ features to produce new behaviours. Args: missing_list: List of Bintool objects to be added to """ - missing_list += self.missing_bintools + for bintool in self.missing_bintools: + if bintool not in missing_list: + missing_list.append(bintool) +

def GetHelpTags(self): """Get the tags use for missing-blob help

From: Stefan Herbrechtsmeier stefan.herbrechtsmeier@weidmueller.com

Forward AddBintools calls to base class to collect bintools of base class.

Signed-off-by: Stefan Herbrechtsmeier stefan.herbrechtsmeier@weidmueller.com

---

(no changes since v5)

Changes in v5: - Add commit to forward AddBintools calls to base class

tools/binman/etype/gbb.py | 1 + tools/binman/etype/intel_ifwi.py | 1 + tools/binman/etype/mkimage.py | 1 + tools/binman/etype/section.py | 1 + tools/binman/etype/vblock.py | 1 + 5 files changed, 5 insertions(+)

diff --git a/tools/binman/etype/gbb.py b/tools/binman/etype/gbb.py index 7394e4e5d3..ba2a362bb5 100644 --- a/tools/binman/etype/gbb.py +++ b/tools/binman/etype/gbb.py @@ -100,4 +100,5 @@ class Entry_gbb(Entry): return True

def AddBintools(self, btools): + super().AddBintools(btools) self.futility = self.AddBintool(btools, 'futility') diff --git a/tools/binman/etype/intel_ifwi.py b/tools/binman/etype/intel_ifwi.py index 4fa7d636fe..04fad401ee 100644 --- a/tools/binman/etype/intel_ifwi.py +++ b/tools/binman/etype/intel_ifwi.py @@ -144,4 +144,5 @@ class Entry_intel_ifwi(Entry_blob_ext): entry.WriteSymbols(self)

def AddBintools(self, btools): + super().AddBintools(btools) self.ifwitool = self.AddBintool(btools, 'ifwitool') diff --git a/tools/binman/etype/mkimage.py b/tools/binman/etype/mkimage.py index 5f6def2287..16a4eb4744 100644 --- a/tools/binman/etype/mkimage.py +++ b/tools/binman/etype/mkimage.py @@ -105,4 +105,5 @@ class Entry_mkimage(Entry): entry.CheckFakedBlobs(faked_blobs_list)

def AddBintools(self, btools): + super().AddBintools(btools) self.mkimage = self.AddBintool(btools, 'mkimage') diff --git a/tools/binman/etype/section.py b/tools/binman/etype/section.py index bd67238b91..48165abf6a 100644 --- a/tools/binman/etype/section.py +++ b/tools/binman/etype/section.py @@ -899,5 +899,6 @@ class Entry_section(Entry): entry.CheckAltFormats(alt_formats)

def AddBintools(self, btools): + super().AddBintools(btools) for entry in self._entries.values(): entry.AddBintools(btools) diff --git a/tools/binman/etype/vblock.py b/tools/binman/etype/vblock.py index c3ef08bbb2..04cb7228aa 100644 --- a/tools/binman/etype/vblock.py +++ b/tools/binman/etype/vblock.py @@ -98,4 +98,5 @@ class Entry_vblock(Entry_collection): return self.ProcessContentsUpdate(data)

def AddBintools(self, btools): + super().AddBintools(btools) self.futility = self.AddBintool(btools, 'futility')

From: Stefan Herbrechtsmeier stefan.herbrechtsmeier@weidmueller.com

Check only section data instead of the rest of the image in multi section test.

Signed-off-by: Stefan Herbrechtsmeier stefan.herbrechtsmeier@weidmueller.com

---

(no changes since v5)

Changes in v5: - Add commit to check only section data in multi section test

tools/binman/ftest.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index da9aa9e679..293108d738 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -4427,15 +4427,17 @@ class TestFunctional(unittest.TestCase): rest = base[len(U_BOOT_DATA):]

# Check compressed data - section1 = self._decompress(rest) expect1 = comp_util.compress(COMPRESS_DATA + U_BOOT_DATA, 'lz4') - self.assertEquals(expect1, rest[:len(expect1)]) + data1 = rest[:len(expect1)] + section1 = self._decompress(data1) + self.assertEquals(expect1, data1) self.assertEquals(COMPRESS_DATA + U_BOOT_DATA, section1) rest1 = rest[len(expect1):]

- section2 = self._decompress(rest1) expect2 = comp_util.compress(COMPRESS_DATA + COMPRESS_DATA, 'lz4') - self.assertEquals(expect2, rest1[:len(expect2)]) + data2 = rest1[:len(expect2)] + section2 = self._decompress(data2) + self.assertEquals(expect2, data2) self.assertEquals(COMPRESS_DATA + COMPRESS_DATA, section2) rest2 = rest1[len(expect2):]

From: Stefan Herbrechtsmeier stefan.herbrechtsmeier@weidmueller.com

Add an optional length header attribute to the device tree blob entry class based on the compressed data header from the utilities to compress and decompress data.

If needed the header could be enabled with the following attribute beside the compress attribute: prepend = "length";

The header was introduced as part of commit eb0f4a4cb402 ("binman: Support replacing data in a cbfs") to allow device tree entries to be larger than the compressed contents. Regarding the commit "this is necessary to cope with a compressed device tree being updated in such a way that it shrinks after the entry size is already set (an obscure case)". This case need to be fixed without influence any compressed data by itself.

Signed-off-by: Stefan Herbrechtsmeier stefan.herbrechtsmeier@weidmueller.com

---

(no changes since v5)

Changes in v5: - Fix decompress

Changes in v3: - Move comp_util.py changes (drop with_header) into separate commits.

Changes in v2: - Reworked to make the feature optional.

tools/binman/entries.rst | 3 ++ tools/binman/etype/blob_dtb.py | 27 +++++++++++++ tools/binman/ftest.py | 39 +++++++++++++++++++ .../test/235_compress_dtb_prepend_invalid.dts | 17 ++++++++ .../test/236_compress_dtb_prepend_length.dts | 19 +++++++++ 5 files changed, 105 insertions(+) create mode 100644 tools/binman/test/235_compress_dtb_prepend_invalid.dts create mode 100644 tools/binman/test/236_compress_dtb_prepend_length.dts

diff --git a/tools/binman/entries.rst b/tools/binman/entries.rst index 782bff1f8d..c9336d1bb4 100644 --- a/tools/binman/entries.rst +++ b/tools/binman/entries.rst @@ -216,6 +216,9 @@ This is a blob containing a device tree. The contents of the blob are obtained from the list of available device-tree files, managed by the 'state' module.

+Additional Properties / Entry arguments: + - prepend: Header type to use: + length: 32-bit length header

.. _etype_blob_ext: diff --git a/tools/binman/etype/blob_dtb.py b/tools/binman/etype/blob_dtb.py index 4159e3032a..5a6a454748 100644 --- a/tools/binman/etype/blob_dtb.py +++ b/tools/binman/etype/blob_dtb.py @@ -7,6 +7,8 @@

from binman.entry import Entry from binman.etype.blob import Entry_blob +from dtoc import fdt_util +import struct

# This is imported if needed state = None @@ -17,6 +19,9 @@ class Entry_blob_dtb(Entry_blob): This is a blob containing a device tree. The contents of the blob are obtained from the list of available device-tree files, managed by the 'state' module. + + Additional attributes: + prepend: Header used (e.g. 'length') """ def __init__(self, section, etype, node): # Put this here to allow entry-docs and help to work without libfdt @@ -24,6 +29,14 @@ class Entry_blob_dtb(Entry_blob): from binman import state

super().__init__(section, etype, node) + self.prepend = None + + def ReadNode(self): + super().ReadNode() + self.prepend = fdt_util.GetString(self._node, 'prepend') + if self.prepend and self.prepend not in ['length']: + self.Raise("Invalid prepend in '%s': '%s'" % + (self._node.name, self.prepend))

def ObtainContents(self): """Get the device-tree from the list held by the 'state' module""" @@ -58,3 +71,17 @@ class Entry_blob_dtb(Entry_blob): # will still return the old contents state.UpdateFdtContents(self.GetFdtEtype(), data) return ok + + def CompressData(self, indata): + data = super().CompressData(indata) + if self.prepend == 'length': + hdr = struct.pack('<I', len(data)) + data = hdr + data + return data + + def DecompressData(self, indata): + if self.prepend == 'length': + data_len = struct.unpack('<I', indata[:4])[0] + indata = indata[4:4 + data_len] + data = super().DecompressData(indata) + return data diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index 293108d738..6e63b7c27a 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -5723,6 +5723,45 @@ fdt fdtmap Extract the devicetree blob from the fdtmap dts='234_replace_section_simple.dts') self.assertEqual(new_data, data)

+ def testCompressDtbPrependInvalid(self): + """Test that invalid header is detected""" + with self.assertRaises(ValueError) as e: + self._DoReadFileDtb('235_compress_dtb_prepend_invalid.dts') + self.assertIn("Node '/binman/u-boot-dtb': Invalid prepend in " + "'u-boot-dtb': 'invalid'", str(e.exception)) + + def testCompressDtbPrependLength(self): + """Test that compress with length header works as expected""" + data = self._DoReadFileRealDtb('236_compress_dtb_prepend_length.dts') + image = control.images['image'] + entries = image.GetEntries() + self.assertIn('u-boot-dtb', entries) + u_boot_dtb = entries['u-boot-dtb'] + self.assertIn('fdtmap', entries) + fdtmap = entries['fdtmap'] + + image_fname = tools.get_output_filename('image.bin') + orig = control.ReadEntry(image_fname, 'u-boot-dtb') + dtb = fdt.Fdt.FromData(orig) + dtb.Scan() + props = self._GetPropTree(dtb, ['size', 'uncomp-size']) + expected = { + 'u-boot:size': len(U_BOOT_DATA), + 'u-boot-dtb:uncomp-size': len(orig), + 'u-boot-dtb:size': u_boot_dtb.size, + 'fdtmap:size': fdtmap.size, + 'size': len(data), + } + self.assertEqual(expected, props) + + # Check implementation + self.assertEqual(U_BOOT_DATA, data[:len(U_BOOT_DATA)]) + rest = data[len(U_BOOT_DATA):] + comp_data_len = struct.unpack('<I', rest[:4])[0] + comp_data = rest[4:4 + comp_data_len] + orig2 = self._decompress(comp_data) + self.assertEqual(orig, orig2) +

if __name__ == "__main__": unittest.main() diff --git a/tools/binman/test/235_compress_dtb_prepend_invalid.dts b/tools/binman/test/235_compress_dtb_prepend_invalid.dts new file mode 100644 index 0000000000..ee32670a91 --- /dev/null +++ b/tools/binman/test/235_compress_dtb_prepend_invalid.dts @@ -0,0 +1,17 @@ +// SPDX-License-Identifier: GPL-2.0+ + +/dts-v1/; + +/ { + #address-cells = <1>; + #size-cells = <1>; + + binman { + u-boot { + }; + u-boot-dtb { + compress = "lz4"; + prepend = "invalid"; + }; + }; +}; diff --git a/tools/binman/test/236_compress_dtb_prepend_length.dts b/tools/binman/test/236_compress_dtb_prepend_length.dts new file mode 100644 index 0000000000..1570233637 --- /dev/null +++ b/tools/binman/test/236_compress_dtb_prepend_length.dts @@ -0,0 +1,19 @@ +// SPDX-License-Identifier: GPL-2.0+ + +/dts-v1/; + +/ { + #address-cells = <1>; + #size-cells = <1>; + + binman { + u-boot { + }; + u-boot-dtb { + compress = "lz4"; + prepend = "length"; + }; + fdtmap { + }; + }; +};

From: Stefan Herbrechtsmeier stefan.herbrechtsmeier@weidmueller.com

Remove the obsolete compressed data header handling from the utilities to compress and decompress data. The header is uncommon, not supported by U-Boot and incompatible with external compressed artifacts.

Signed-off-by: Stefan Herbrechtsmeier stefan.herbrechtsmeier@weidmueller.com ---

(no changes since v5)

Changes in v5: - Remove header value in testInvalidCompress

Changes in v3: - Add commit to remove obsolete compressed data header handling

tools/binman/cbfs_util.py | 8 ++++---- tools/binman/comp_util.py | 11 ++--------- tools/binman/entry.py | 6 +++--- tools/binman/ftest.py | 14 ++++++-------- 4 files changed, 15 insertions(+), 24 deletions(-)

diff --git a/tools/binman/cbfs_util.py b/tools/binman/cbfs_util.py index 9cad03886f..a1836f4ad3 100644 --- a/tools/binman/cbfs_util.py +++ b/tools/binman/cbfs_util.py @@ -241,9 +241,9 @@ class CbfsFile(object): """Handle decompressing data if necessary""" indata = self.data if self.compress == COMPRESS_LZ4: - data = comp_util.decompress(indata, 'lz4', with_header=False) + data = comp_util.decompress(indata, 'lz4') elif self.compress == COMPRESS_LZMA: - data = comp_util.decompress(indata, 'lzma', with_header=False) + data = comp_util.decompress(indata, 'lzma') else: data = indata self.memlen = len(data) @@ -362,9 +362,9 @@ class CbfsFile(object): elif self.ftype == TYPE_RAW: orig_data = data if self.compress == COMPRESS_LZ4: - data = comp_util.compress(orig_data, 'lz4', with_header=False) + data = comp_util.compress(orig_data, 'lz4') elif self.compress == COMPRESS_LZMA: - data = comp_util.compress(orig_data, 'lzma', with_header=False) + data = comp_util.compress(orig_data, 'lzma') self.memlen = len(orig_data) self.data_len = len(data) attr = struct.pack(ATTR_COMPRESSION_FORMAT, diff --git a/tools/binman/comp_util.py b/tools/binman/comp_util.py index dc76adab35..269bbf7975 100644 --- a/tools/binman/comp_util.py +++ b/tools/binman/comp_util.py @@ -3,7 +3,6 @@ # """Utilities to compress and decompress data"""

-import struct import tempfile

from binman import bintool @@ -16,7 +15,7 @@ LZMA_ALONE = bintool.Bintool.create('lzma_alone') HAVE_LZMA_ALONE = LZMA_ALONE.is_present()

-def compress(indata, algo, with_header=True): +def compress(indata, algo): """Compress some data using a given algorithm

Note that for lzma this uses an old version of the algorithm, not that @@ -41,12 +40,9 @@ def compress(indata, algo, with_header=True): data = LZMA_ALONE.compress(indata) else: raise ValueError("Unknown algorithm '%s'" % algo) - if with_header: - hdr = struct.pack('<I', len(data)) - data = hdr + data return data

-def decompress(indata, algo, with_header=True): +def decompress(indata, algo): """Decompress some data using a given algorithm

Note that for lzma this uses an old version of the algorithm, not that @@ -64,9 +60,6 @@ def decompress(indata, algo, with_header=True): """ if algo == 'none': return indata - if with_header: - data_len = struct.unpack('<I', indata[:4])[0] - indata = indata[4:4 + data_len] if algo == 'lz4': data = LZ4.decompress(indata) elif algo == 'lzma': diff --git a/tools/binman/entry.py b/tools/binman/entry.py index ecce721ac6..398d5caa39 100644 --- a/tools/binman/entry.py +++ b/tools/binman/entry.py @@ -1078,12 +1078,12 @@ features to produce new behaviours. indata: Data to compress

Returns: - Compressed data (first word is the compressed size) + Compressed data """ self.uncomp_data = indata if self.compress != 'none': self.uncomp_size = len(indata) - data = comp_util.compress(indata, self.compress, with_header=False) + data = comp_util.compress(indata, self.compress) return data

def DecompressData(self, indata): @@ -1095,7 +1095,7 @@ features to produce new behaviours. Returns: Decompressed data """ - data = comp_util.decompress(indata, self.compress, with_header=False) + data = comp_util.decompress(indata, self.compress) if self.compress != 'none': self.uncomp_size = len(data) self.uncomp_data = data diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index edee1f3984..1538ca3c4e 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -1967,7 +1967,7 @@ class TestFunctional(unittest.TestCase): self._ResetDtbs()

def _decompress(self, data): - return comp_util.decompress(data, 'lz4', with_header=False) + return comp_util.decompress(data, 'lz4')

def testCompress(self): """Test compression of blobs""" @@ -2856,7 +2856,7 @@ class TestFunctional(unittest.TestCase): def testExtractCbfsRaw(self): """Test extracting CBFS compressed data without decompressing it""" data = self._RunExtractCmd('section/cbfs/u-boot-dtb', decomp=False) - dtb = comp_util.decompress(data, 'lzma', with_header=False) + dtb = comp_util.decompress(data, 'lzma') self.assertEqual(EXTRACT_DTB_SIZE, len(dtb))

def testExtractBadEntry(self): @@ -4427,16 +4427,14 @@ class TestFunctional(unittest.TestCase): rest = base[len(U_BOOT_DATA):]

# Check compressed data - expect1 = comp_util.compress(COMPRESS_DATA + U_BOOT_DATA, 'lz4', - with_header=False) + expect1 = comp_util.compress(COMPRESS_DATA + U_BOOT_DATA, 'lz4') data1 = rest[:len(expect1)] section1 = self._decompress(data1) self.assertEquals(expect1, data1) self.assertEquals(COMPRESS_DATA + U_BOOT_DATA, section1) rest1 = rest[len(expect1):]

- expect2 = comp_util.compress(COMPRESS_DATA + COMPRESS_DATA, 'lz4', - with_header=False) + expect2 = comp_util.compress(COMPRESS_DATA + COMPRESS_DATA, 'lz4') data2 = rest1[:len(expect2)] section2 = self._decompress(data2) self.assertEquals(expect2, data2) @@ -5221,11 +5219,11 @@ fdt fdtmap Extract the devicetree blob from the fdtmap

def testInvalidCompress(self): with self.assertRaises(ValueError) as e: - comp_util.compress(b'', 'invalid', with_header=False) + comp_util.compress(b'', 'invalid') self.assertIn("Unknown algorithm 'invalid'", str(e.exception))

with self.assertRaises(ValueError) as e: - comp_util.decompress(b'1234', 'invalid', with_header=False) + comp_util.decompress(b'1234', 'invalid') self.assertIn("Unknown algorithm 'invalid'", str(e.exception))

def testBintoolDocs(self):

From: Stefan Herbrechtsmeier stefan.herbrechtsmeier@weidmueller.com

Select the lz4 and lzma_alone bintools in cbfs_util class to centralize the supported compression algorithm evaluation inside the class and over multiple classes.

Signed-off-by: Stefan Herbrechtsmeier stefan.herbrechtsmeier@weidmueller.com

---

(no changes since v5)

Changes in v5: - Add commit to select compression bintools in cbfs_util class

tools/binman/cbfs_util.py | 20 +++++++++++--------- tools/binman/cbfs_util_test.py | 4 ++-- 2 files changed, 13 insertions(+), 11 deletions(-)

diff --git a/tools/binman/cbfs_util.py b/tools/binman/cbfs_util.py index a1836f4ad3..7bd3d89798 100644 --- a/tools/binman/cbfs_util.py +++ b/tools/binman/cbfs_util.py @@ -20,7 +20,7 @@ import io import struct import sys

-from binman import comp_util +from binman import bintool from binman import elf from patman import command from patman import tools @@ -236,14 +236,18 @@ class CbfsFile(object): self.data_len = len(data) self.erase_byte = None self.size = None + if self.compress == COMPRESS_LZ4: + self.comp_bintool = bintool.Bintool.create('lz4') + elif self.compress == COMPRESS_LZMA: + self.comp_bintool = bintool.Bintool.create('lzma_alone') + else: + self.comp_bintool = None

def decompress(self): """Handle decompressing data if necessary""" indata = self.data - if self.compress == COMPRESS_LZ4: - data = comp_util.decompress(indata, 'lz4') - elif self.compress == COMPRESS_LZMA: - data = comp_util.decompress(indata, 'lzma') + if self.comp_bintool: + data = self.comp_bintool.decompress(indata) else: data = indata self.memlen = len(data) @@ -361,10 +365,8 @@ class CbfsFile(object): data = elf_data.data elif self.ftype == TYPE_RAW: orig_data = data - if self.compress == COMPRESS_LZ4: - data = comp_util.compress(orig_data, 'lz4') - elif self.compress == COMPRESS_LZMA: - data = comp_util.compress(orig_data, 'lzma') + if self.comp_bintool: + data = self.comp_bintool.compress(orig_data) self.memlen = len(orig_data) self.data_len = len(data) attr = struct.pack(ATTR_COMPRESSION_FORMAT, diff --git a/tools/binman/cbfs_util_test.py b/tools/binman/cbfs_util_test.py index f86b295149..e0f792fd34 100755 --- a/tools/binman/cbfs_util_test.py +++ b/tools/binman/cbfs_util_test.py @@ -19,7 +19,6 @@ import unittest from binman import bintool from binman import cbfs_util from binman.cbfs_util import CbfsWriter -from binman import comp_util from binman import elf from patman import test_util from patman import tools @@ -50,7 +49,8 @@ class TestCbfs(unittest.TestCase): cls.cbfstool = bintool.Bintool.create('cbfstool') cls.have_cbfstool = cls.cbfstool.is_present()

- cls.have_lz4 = comp_util.HAVE_LZ4 + lz4 = bintool.Bintool.create('lz4') + cls.have_lz4 = lz4.is_present()

@classmethod def tearDownClass(cls):

From: Stefan Herbrechtsmeier stefan.herbrechtsmeier@weidmueller.com

Handle missing compression tools by returning empty data and record missing bintool.

Signed-off-by: Stefan Herbrechtsmeier stefan.herbrechtsmeier@weidmueller.com ---

Changes in v6: - Set data with tools.get_bytes(0, 1024) instead returning

Changes in v5: - Use record_missing_bintool function - Use tools.get_bytes function - Add support to DecompressData function - Reuse 85_compress_section.dts file - Remove 236_compress_dtb_missing_bintool.dts file

Changes in v4: - Add missing 236_compress_dtb_missing_bintool.dts file

Changes in v3: - Add commit to support missing compression tools

tools/binman/entry.py | 14 +++++++++++--- tools/binman/entry_test.py | 9 +++++++++ tools/binman/ftest.py | 9 +++++++++ 3 files changed, 29 insertions(+), 3 deletions(-)

diff --git a/tools/binman/entry.py b/tools/binman/entry.py index 5fa826aa71..48883bac3a 100644 --- a/tools/binman/entry.py +++ b/tools/binman/entry.py @@ -1084,7 +1084,11 @@ features to produce new behaviours. self.uncomp_data = indata if self.compress != 'none': self.uncomp_size = len(indata) - data = self.comp_bintool.compress(indata) + if self.comp_bintool.is_present(): + data = self.comp_bintool.compress(indata) + else: + self.record_missing_bintool(self.comp_bintool) + data = tools.get_bytes(0, 1024) else: data = indata return data @@ -1099,8 +1103,12 @@ features to produce new behaviours. Decompressed data """ if self.compress != 'none': - data = self.comp_bintool.decompress(indata) - self.uncomp_size = len(data) + if self.comp_bintool.is_present(): + data = self.comp_bintool.decompress(indata) + self.uncomp_size = len(data) + else: + self.record_missing_bintool(self.comp_bintool) + data = tools.get_bytes(0, 1024) else: data = indata self.uncomp_data = data diff --git a/tools/binman/entry_test.py b/tools/binman/entry_test.py index 1d60076be1..aa470c5816 100644 --- a/tools/binman/entry_test.py +++ b/tools/binman/entry_test.py @@ -105,6 +105,15 @@ class TestEntry(unittest.TestCase): self.assertTrue(isinstance(ent, Entry_blob)) self.assertEquals('missing', ent.etype)

+ def testDecompressData(self): + """Test the DecompressData() method of the base class""" + base = entry.Entry.Create(None, self.GetNode(), 'blob-dtb') + base.compress = 'lz4' + bintools = {} + base.comp_bintool = base.AddBintool(bintools, '_testing') + self.assertEquals(tools.get_bytes(0, 1024), base.CompressData(b'abc')) + self.assertEquals(tools.get_bytes(0, 1024), base.DecompressData(b'abc')) +

if __name__ == "__main__": unittest.main() diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index 939b265d7c..ffcd7d2567 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -4422,6 +4422,15 @@ class TestFunctional(unittest.TestCase): } self.assertEqual(expected, props)

+ def testLz4Missing(self): + """Test that binman still produces an image if lz4 is missing""" + with test_util.capture_sys_output() as (_, stderr): + self._DoTestFile('185_compress_section.dts', + force_missing_bintools='lz4') + err = stderr.getvalue() + self.assertRegex(err, + "Image 'main-section'.*missing bintools.*: lz4") + def testCompressExtra(self): """Test compression of a section with no fixed size""" self._CheckLz4()

From: Stefan Herbrechtsmeier stefan.herbrechtsmeier@weidmueller.com

Add a bintools base class for packers which compression / decompression entry contents.

Signed-off-by: Stefan Herbrechtsmeier stefan.herbrechtsmeier@weidmueller.com

---

(no changes since v5)

Changes in v5: - Remove self.fetch_package not None check in fetch function

Changes in v3: - Document class properties

Changes in v2: - Add commit to add BintoolPacker class to bintool

tools/binman/bintool.py | 106 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 106 insertions(+)

diff --git a/tools/binman/bintool.py b/tools/binman/bintool.py index 8435b29749..ccc1904d62 100644 --- a/tools/binman/bintool.py +++ b/tools/binman/bintool.py @@ -1,5 +1,7 @@ # SPDX-License-Identifier: GPL-2.0+ # Copyright 2022 Google LLC +# Copyright (C) 2022 Weidmüller Interface GmbH & Co. KG +# Stefan Herbrechtsmeier stefan.herbrechtsmeier@weidmueller.com # """Base class for all bintools

@@ -464,3 +466,107 @@ binaries. It is fairly easy to create new bintools. Just add a new file to the str: Version string for this bintool """ return 'unknown' + +class BintoolPacker(Bintool): + """Tool which compression / decompression entry contents + + This is a bintools base class for compression / decompression packer + + Properties: + name: Name of packer tool + compression: Compression type (COMPRESS_...), value of 'name' property + if none + compress_args: List of positional args provided to tool for compress, + ['--compress'] if none + decompress_args: List of positional args provided to tool for + decompress, ['--decompress'] if none + fetch_package: Name of the tool installed using the apt, value of 'name' + property if none + version_regex: Regular expressions to extract the version from tool + version output, '(v[0-9.]+)' if none + """ + def __init__(self, name, compression=None, compress_args=None, + decompress_args=None, fetch_package=None, + version_regex=r'(v[0-9.]+)'): + desc = '%s compression' % (compression if compression else name) + super().__init__(name, desc) + if compress_args is None: + compress_args = ['--compress'] + self.compress_args = compress_args + if decompress_args is None: + decompress_args = ['--decompress'] + self.decompress_args = decompress_args + if fetch_package is None: + fetch_package = name + self.fetch_package = fetch_package + self.version_regex = version_regex + + def compress(self, indata): + """Compress data + + Args: + indata (bytes): Data to compress + + Returns: + bytes: Compressed data + """ + with tempfile.NamedTemporaryFile(prefix='comp.tmp', + dir=tools.get_output_dir()) as tmp: + tools.write_file(tmp.name, indata) + args = self.compress_args + ['--stdout', tmp.name] + return self.run_cmd(*args, binary=True) + + def decompress(self, indata): + """Decompress data + + Args: + indata (bytes): Data to decompress + + Returns: + bytes: Decompressed data + """ + with tempfile.NamedTemporaryFile(prefix='decomp.tmp', + dir=tools.get_output_dir()) as inf: + tools.write_file(inf.name, indata) + args = self.decompress_args + ['--stdout', inf.name] + return self.run_cmd(*args, binary=True) + + def fetch(self, method): + """Fetch handler + + This installs the gzip package using the apt utility. + + Args: + method (FETCH_...): Method to use + + Returns: + True if the file was fetched and now installed, None if a method + other than FETCH_BIN was requested + + Raises: + Valuerror: Fetching could not be completed + """ + if method != FETCH_BIN: + return None + return self.apt_install(self.fetch_package) + + def version(self): + """Version handler + + Returns: + str: Version number + """ + import re + + result = self.run_cmd_result('-V') + if not result: + return super().version() + + out = result.stdout.strip() + if not out: + out = result.stderr.strip() + if not out: + return super().version() + + m_version = re.search(self.version_regex, out) + return m_version.group(1) if m_version else out

From: Stefan Herbrechtsmeier stefan.herbrechtsmeier@weidmueller.com

Add gzip bintool to binman to support on-the-fly compression of Linux kernel images and FPGA bitstreams. The SPL basic fitImage implementation supports only gzip decompression.

Signed-off-by: Stefan Herbrechtsmeier stefan.herbrechtsmeier@weidmueller.com

---

(no changes since v5)

Changes in v5: - Add algorithm name to entry.py - Add tool name to ftest.py

Changes in v2: - Add commit to add gzip bintool

tools/binman/btool/gzip.py | 31 +++++++++++++++++++++++++++++++ tools/binman/entry.py | 2 +- tools/binman/ftest.py | 2 +- 3 files changed, 33 insertions(+), 2 deletions(-) create mode 100644 tools/binman/btool/gzip.py

diff --git a/tools/binman/btool/gzip.py b/tools/binman/btool/gzip.py new file mode 100644 index 0000000000..0d75028120 --- /dev/null +++ b/tools/binman/btool/gzip.py @@ -0,0 +1,31 @@ +# SPDX-License-Identifier: GPL-2.0+ +# Copyright (C) 2022 Weidmüller Interface GmbH & Co. KG +# Stefan Herbrechtsmeier stefan.herbrechtsmeier@weidmueller.com +# +"""Bintool implementation for gzip + +gzip allows compression and decompression of files. + +Documentation is available via:: + + man gzip +""" + +from binman import bintool + +# pylint: disable=C0103 +class Bintoolgzip(bintool.BintoolPacker): + """Compression/decompression using the gzip algorithm + + This bintool supports running `gzip` to compress and decompress data, as + used by binman. + + It is also possible to fetch the tool, which uses `apt` to install it. + + Documentation is available via:: + + man gzip + """ + def __init__(self, name): + super().__init__(name, compress_args=[], + version_regex=r'gzip ([0-9.]+)') diff --git a/tools/binman/entry.py b/tools/binman/entry.py index a0c0a02f5d..930c7910d2 100644 --- a/tools/binman/entry.py +++ b/tools/binman/entry.py @@ -1157,7 +1157,7 @@ features to produce new behaviours. """ algo = self.compress if algo != 'none': - algos = ['bzip2', 'lz4', 'lzma'] + algos = ['bzip2', 'gzip', 'lz4', 'lzma'] if algo not in algos: raise ValueError("Unknown algorithm '%s'" % algo) names = {'lzma': 'lzma_alone'} diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index 9fa8e9e049..911c955cba 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -107,7 +107,7 @@ BASE_DTB_PROPS = ['offset', 'size', 'image-pos'] REPACK_DTB_PROPS = ['orig-offset', 'orig-size']

# Supported compression bintools -COMP_BINTOOLS = ['bzip2', 'lz4', 'lzma_alone'] +COMP_BINTOOLS = ['bzip2', 'gzip', 'lz4', 'lzma_alone']

class TestFunctional(unittest.TestCase): """Functional tests for binman

From: Stefan Herbrechtsmeier stefan.herbrechtsmeier@weidmueller.com

Add xz bintool to binman to support on-the-fly compression.

Signed-off-by: Stefan Herbrechtsmeier stefan.herbrechtsmeier@weidmueller.com

---

(no changes since v5)

Changes in v5: - Add algorithm name to entry.py - Add tool name to ftest.py

Changes in v2: - Add commit to add xz bintool

tools/binman/btool/xz.py | 31 +++++++++++++++++++++++++++++++ tools/binman/entry.py | 2 +- tools/binman/ftest.py | 2 +- 3 files changed, 33 insertions(+), 2 deletions(-) create mode 100644 tools/binman/btool/xz.py

diff --git a/tools/binman/btool/xz.py b/tools/binman/btool/xz.py new file mode 100644 index 0000000000..e2b413d18b --- /dev/null +++ b/tools/binman/btool/xz.py @@ -0,0 +1,31 @@ +# SPDX-License-Identifier: GPL-2.0+ +# Copyright (C) 2022 Weidmüller Interface GmbH & Co. KG +# Stefan Herbrechtsmeier stefan.herbrechtsmeier@weidmueller.com +# +"""Bintool implementation for xz + +xz allows compression and decompression of files. + +Documentation is available via:: + + man xz +""" + +from binman import bintool + +# pylint: disable=C0103 +class Bintoolxz(bintool.BintoolPacker): + """Compression/decompression using the xz algorithm + + This bintool supports running `xz` to compress and decompress data, as + used by binman. + + It is also possible to fetch the tool, which uses `apt` to install it. + + Documentation is available via:: + + man xz + """ + def __init__(self, name): + super().__init__(name, fetch_package='xz-utils', + version_regex=r'xz (XZ Utils) ([0-9.]+)') diff --git a/tools/binman/entry.py b/tools/binman/entry.py index 0a410971d8..0796fbe0c1 100644 --- a/tools/binman/entry.py +++ b/tools/binman/entry.py @@ -1157,7 +1157,7 @@ features to produce new behaviours. """ algo = self.compress if algo != 'none': - algos = ['bzip2', 'gzip', 'lz4', 'lzma', 'lzo'] + algos = ['bzip2', 'gzip', 'lz4', 'lzma', 'lzo', 'xz'] if algo not in algos: raise ValueError("Unknown algorithm '%s'" % algo) names = {'lzma': 'lzma_alone', 'lzo': 'lzop'} diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index 2cac505d8b..b337e2168c 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -107,7 +107,7 @@ BASE_DTB_PROPS = ['offset', 'size', 'image-pos'] REPACK_DTB_PROPS = ['orig-offset', 'orig-size']

# Supported compression bintools -COMP_BINTOOLS = ['bzip2', 'gzip', 'lz4', 'lzma_alone', 'lzop'] +COMP_BINTOOLS = ['bzip2', 'gzip', 'lz4', 'lzma_alone', 'lzop', 'xz']

class TestFunctional(unittest.TestCase): """Functional tests for binman