All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v4 01/13] binman: Skip elf tests if python elftools is not available
@ 2022-08-16  8:41 Stefan Herbrechtsmeier
  2022-08-16  8:41 ` [PATCH v4 02/13] binman: Add length header attribute to dtb entry Stefan Herbrechtsmeier
                   ` (11 more replies)
  0 siblings, 12 replies; 25+ messages in thread
From: Stefan Herbrechtsmeier @ 2022-08-16  8:41 UTC (permalink / raw)
  To: u-boot; +Cc: Stefan Herbrechtsmeier, Simon Glass, Alper Nebi Yasak

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:
- Added

 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,
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH v4 02/13] binman: Add length header attribute to dtb entry
  2022-08-16  8:41 [PATCH v4 01/13] binman: Skip elf tests if python elftools is not available Stefan Herbrechtsmeier
@ 2022-08-16  8:41 ` Stefan Herbrechtsmeier
  2022-08-16 11:48   ` Simon Glass
  2022-08-16  8:42 ` [PATCH v4 03/13] binman: Disable compressed data header Stefan Herbrechtsmeier
                   ` (10 subsequent siblings)
  11 siblings, 1 reply; 25+ messages in thread
From: Stefan Herbrechtsmeier @ 2022-08-16  8:41 UTC (permalink / raw)
  To: u-boot; +Cc: Stefan Herbrechtsmeier, Alper Nebi Yasak, Simon Glass

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 v3)

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                | 24 +++++++++++++++++++
 tools/binman/ftest.py                         | 22 +++++++++++++++++
 .../test/235_compress_prepend_length_dtb.dts  | 17 +++++++++++++
 4 files changed, 66 insertions(+)
 create mode 100644 tools/binman/test/235_compress_prepend_length_dtb.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..4fd2ecda83 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
@@ -25,6 +30,15 @@ class Entry_blob_dtb(Entry_blob):
 
         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"""
         self._filename = self.GetDefaultFilename()
@@ -35,6 +49,9 @@ class Entry_blob_dtb(Entry_blob):
         """Re-read the DTB contents so that we get any calculated properties"""
         _, indata = state.GetFdtContents(self.GetFdtEtype())
         data = self.CompressData(indata)
+        if self.prepend == 'length':
+            hdr = struct.pack('<I', len(data))
+            data = hdr + data
         return self.ProcessContentsUpdate(data)
 
     def GetFdtEtype(self):
@@ -50,6 +67,13 @@ class Entry_blob_dtb(Entry_blob):
         fname = self.GetDefaultFilename()
         return {self.GetFdtEtype(): [self, fname]}
 
+    def ReadData(self, decomp=True, alt_format=None):
+        data = super().ReadData(decomp, alt_format)
+        if self.prepend == 'length':
+            data_len = struct.unpack('<I', data[:4])[0]
+            data = data[4:4 + data_len]
+        return data
+
     def WriteData(self, data, decomp=True):
         ok = super().WriteData(data, decomp)
 
diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
index da9aa9e679..1b468d8e6d 100644
--- a/tools/binman/ftest.py
+++ b/tools/binman/ftest.py
@@ -2536,6 +2536,28 @@ class TestFunctional(unittest.TestCase):
             }
         self.assertEqual(expected, props)
 
+    def testCompressPrependLengthDtb(self):
+        """Test that compress of device-tree files with length header is
+        supported
+        """
+        data = self.data = self._DoReadFileRealDtb('235_compress_prepend_length_dtb.dts')
+        self.assertEqual(U_BOOT_DATA, data[:len(U_BOOT_DATA)])
+        dtb_data = data[len(U_BOOT_DATA):]
+        comp_data_len = struct.unpack('<I', dtb_data[:4])[0]
+        comp_data = dtb_data[4:4 + comp_data_len]
+        orig = self._decompress(comp_data)
+        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': len(dtb_data),
+            'size': len(data),
+            }
+        self.assertEqual(expected, props)
+
+
     def testCbfsUpdateFdt(self):
         """Test that we can update the device tree with CBFS offset/size info"""
         self._CheckLz4()
diff --git a/tools/binman/test/235_compress_prepend_length_dtb.dts b/tools/binman/test/235_compress_prepend_length_dtb.dts
new file mode 100644
index 0000000000..a5abc60477
--- /dev/null
+++ b/tools/binman/test/235_compress_prepend_length_dtb.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 = "length";
+		};
+	};
+};
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH v4 03/13] binman: Disable compressed data header
  2022-08-16  8:41 [PATCH v4 01/13] binman: Skip elf tests if python elftools is not available Stefan Herbrechtsmeier
  2022-08-16  8:41 ` [PATCH v4 02/13] binman: Add length header attribute to dtb entry Stefan Herbrechtsmeier
@ 2022-08-16  8:42 ` Stefan Herbrechtsmeier
  2022-08-16 11:48   ` Simon Glass
  2022-08-16  8:42 ` [PATCH v4 04/13] binman: Remove obsolete compressed data header handling Stefan Herbrechtsmeier
                   ` (9 subsequent siblings)
  11 siblings, 1 reply; 25+ messages in thread
From: Stefan Herbrechtsmeier @ 2022-08-16  8:42 UTC (permalink / raw)
  To: u-boot
  Cc: Stefan Herbrechtsmeier, Alper Nebi Yasak, Heiko Thiery, Simon Glass

From: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com>

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

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.

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

---

(no changes since v3)

Changes in v3:
- Added

 tools/binman/entry.py         |  2 +-
 tools/binman/etype/section.py |  3 ++-
 tools/binman/ftest.py         | 18 +++++++++++-------
 3 files changed, 14 insertions(+), 9 deletions(-)

diff --git a/tools/binman/entry.py b/tools/binman/entry.py
index e3767aefa7..a45aeeaa59 100644
--- a/tools/binman/entry.py
+++ b/tools/binman/entry.py
@@ -1079,7 +1079,7 @@ features to produce new behaviours.
         self.uncomp_data = indata
         if self.compress != 'none':
             self.uncomp_size = len(indata)
-        data = comp_util.compress(indata, self.compress)
+        data = comp_util.compress(indata, self.compress, with_header=False)
         return data
 
     @classmethod
diff --git a/tools/binman/etype/section.py b/tools/binman/etype/section.py
index bd67238b91..0a92bf2386 100644
--- a/tools/binman/etype/section.py
+++ b/tools/binman/etype/section.py
@@ -777,7 +777,8 @@ class Entry_section(Entry):
         data = parent_data[offset:offset + child.size]
         if decomp:
             indata = data
-            data = comp_util.decompress(indata, child.compress)
+            data = comp_util.decompress(indata, child.compress,
+                                        with_header=False)
             if child.uncomp_size:
                 tout.info("%s: Decompressing data size %#x with algo '%s' to data size %#x" %
                             (child.GetPath(), len(indata), child.compress,
diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
index 1b468d8e6d..d082442bec 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')
+        return comp_util.decompress(data, 'lz4', with_header=False)
 
     def testCompress(self):
         """Test compression of blobs"""
@@ -4449,15 +4449,19 @@ 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)])
+        expect1 = comp_util.compress(COMPRESS_DATA + U_BOOT_DATA, 'lz4',
+                                     with_header=False)
+        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)])
+        expect2 = comp_util.compress(COMPRESS_DATA + COMPRESS_DATA, 'lz4',
+                                     with_header=False)
+        data2 = rest1[:len(expect2)]
+        section2 = self._decompress(data2)
+        self.assertEquals(expect2, data2)
         self.assertEquals(COMPRESS_DATA + COMPRESS_DATA, section2)
         rest2 = rest1[len(expect2):]
 
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH v4 04/13] binman: Remove obsolete compressed data header handling
  2022-08-16  8:41 [PATCH v4 01/13] binman: Skip elf tests if python elftools is not available Stefan Herbrechtsmeier
  2022-08-16  8:41 ` [PATCH v4 02/13] binman: Add length header attribute to dtb entry Stefan Herbrechtsmeier
  2022-08-16  8:42 ` [PATCH v4 03/13] binman: Disable compressed data header Stefan Herbrechtsmeier
@ 2022-08-16  8:42 ` Stefan Herbrechtsmeier
  2022-08-16  8:42 ` [PATCH v4 05/13] binman: Simplify comp_util class Stefan Herbrechtsmeier
                   ` (8 subsequent siblings)
  11 siblings, 0 replies; 25+ messages in thread
From: Stefan Herbrechtsmeier @ 2022-08-16  8:42 UTC (permalink / raw)
  To: u-boot
  Cc: Stefan Herbrechtsmeier, Alper Nebi Yasak, Heiko Thiery, Simon Glass

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 v3)

Changes in v3:
- Added

 tools/binman/cbfs_util.py     |  8 ++++----
 tools/binman/comp_util.py     | 11 ++---------
 tools/binman/entry.py         |  4 ++--
 tools/binman/etype/section.py |  3 +--
 tools/binman/ftest.py         | 10 ++++------
 5 files changed, 13 insertions(+), 23 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 a45aeeaa59..9ec5811b46 100644
--- a/tools/binman/entry.py
+++ b/tools/binman/entry.py
@@ -1074,12 +1074,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
 
     @classmethod
diff --git a/tools/binman/etype/section.py b/tools/binman/etype/section.py
index 0a92bf2386..bd67238b91 100644
--- a/tools/binman/etype/section.py
+++ b/tools/binman/etype/section.py
@@ -777,8 +777,7 @@ class Entry_section(Entry):
         data = parent_data[offset:offset + child.size]
         if decomp:
             indata = data
-            data = comp_util.decompress(indata, child.compress,
-                                        with_header=False)
+            data = comp_util.decompress(indata, child.compress)
             if child.uncomp_size:
                 tout.info("%s: Decompressing data size %#x with algo '%s' to data size %#x" %
                             (child.GetPath(), len(indata), child.compress,
diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
index d082442bec..057b4e28b7 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"""
@@ -2878,7 +2878,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):
@@ -4449,16 +4449,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)
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH v4 05/13] binman: Simplify comp_util class
  2022-08-16  8:41 [PATCH v4 01/13] binman: Skip elf tests if python elftools is not available Stefan Herbrechtsmeier
                   ` (2 preceding siblings ...)
  2022-08-16  8:42 ` [PATCH v4 04/13] binman: Remove obsolete compressed data header handling Stefan Herbrechtsmeier
@ 2022-08-16  8:42 ` Stefan Herbrechtsmeier
  2022-08-16 11:48   ` Simon Glass
  2022-08-16  8:42 ` [PATCH v4 06/13] binman: Add compression tests Stefan Herbrechtsmeier
                   ` (7 subsequent siblings)
  11 siblings, 1 reply; 25+ messages in thread
From: Stefan Herbrechtsmeier @ 2022-08-16  8:42 UTC (permalink / raw)
  To: u-boot; +Cc: Stefan Herbrechtsmeier, Alper Nebi Yasak, Simon Glass

From: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com>

Simplify the comp_util class by remove duplicate code as preparation for
additional compress algorithms.

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

---

(no changes since v3)

Changes in v3:
- Rename COMPRESSIONS to ALGORITHMS
- Rework existing comments
- Add _get_tool_name function comment
- Add _get_tool function comment

 tools/binman/cbfs_util_test.py |   2 +-
 tools/binman/comp_util.py      | 101 +++++++++++++++++++++++----------
 tools/binman/ftest.py          |   2 +-
 3 files changed, 72 insertions(+), 33 deletions(-)

diff --git a/tools/binman/cbfs_util_test.py b/tools/binman/cbfs_util_test.py
index f86b295149..44ebd04278 100755
--- a/tools/binman/cbfs_util_test.py
+++ b/tools/binman/cbfs_util_test.py
@@ -50,7 +50,7 @@ class TestCbfs(unittest.TestCase):
         cls.cbfstool = bintool.Bintool.create('cbfstool')
         cls.have_cbfstool = cls.cbfstool.is_present()
 
-        cls.have_lz4 = comp_util.HAVE_LZ4
+        cls.have_lz4 = comp_util.is_present('lz4')
 
     @classmethod
     def tearDownClass(cls):
diff --git a/tools/binman/comp_util.py b/tools/binman/comp_util.py
index 269bbf7975..00761d44cc 100644
--- a/tools/binman/comp_util.py
+++ b/tools/binman/comp_util.py
@@ -1,69 +1,108 @@
 # 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>
 #
-"""Utilities to compress and decompress data"""
+"""Utilities to compress and decompress data
+
+This supports the following compression algorithm:
+  none
+  lz4
+  lzma
+
+Note that for lzma this uses an old version of the algorithm, not that
+provided by xz.
+
+This requires the following tools:
+  lz4
+  lzma_alone
+
+It also requires an output directory to be previously set up, by calling
+PrepareOutputDir().
+"""
 
 import tempfile
 
 from binman import bintool
 from patman import tools
 
-LZ4 = bintool.Bintool.create('lz4')
-HAVE_LZ4 = LZ4.is_present()
+# Supported compression algorithms
+ALGORITHMS = ['lz4', 'lzma']
 
-LZMA_ALONE = bintool.Bintool.create('lzma_alone')
-HAVE_LZMA_ALONE = LZMA_ALONE.is_present()
+bintools = {}
 
+def _get_tool_name(algo):
+    """Get the tool name of a compression algorithm
 
-def compress(indata, algo):
-    """Compress some data using a given algorithm
+    Args:
+        algo (str): Algorithm to use
 
-    Note that for lzma this uses an old version of the algorithm, not that
-    provided by xz.
+    Returns:
+        str: Tool name
+    """
+    names = {'lzma': 'lzma_alone'}
+    return names.get(algo, algo)
+
+def _get_tool(algo):
+    """Get the bintool object of a compression algorithm
 
-    This requires 'lz4' and 'lzma_alone' tools. It also requires an output
-    directory to be previously set up, by calling PrepareOutputDir().
+    The function creates new bintool object on demand per compression algorithm
+    and save it in a global bintools dictionary.
+
+    Args:
+        algo (str): Algorithm to use
+
+    Returns:
+        A bintool object for the compression algorithm
+    """
+    global bintools
+    name = _get_tool_name(algo)
+    tool = bintools.get(name)
+    if not tool:
+        tool = bintool.Bintool.create(name)
+        bintools[name] = tool
+    return tool
+
+def compress(indata, algo):
+    """Compress some data using a given algorithm
 
     Args:
         indata (bytes): Input data to compress
-        algo (str): Algorithm to use ('none', 'lz4' or 'lzma')
+        algo (str): Algorithm to use
 
     Returns:
         bytes: Compressed data
     """
     if algo == 'none':
         return indata
-    if algo == 'lz4':
-        data = LZ4.compress(indata)
-    # cbfstool uses a very old version of lzma
-    elif algo == 'lzma':
-        data = LZMA_ALONE.compress(indata)
-    else:
+    if algo not in ALGORITHMS:
         raise ValueError("Unknown algorithm '%s'" % algo)
+
+    tool = _get_tool(algo)
+    data = tool.compress(indata)
+
     return data
 
 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
-    provided by xz.
-
-    This requires 'lz4' and 'lzma_alone' tools. It also requires an output
-    directory to be previously set up, by calling PrepareOutputDir().
-
     Args:
         indata (bytes): Input data to decompress
-        algo (str): Algorithm to use ('none', 'lz4' or 'lzma')
+        algo (str): Algorithm to use
 
     Returns:
-        (bytes) Compressed data
+        bytes: Decompressed data
     """
     if algo == 'none':
         return indata
-    if algo == 'lz4':
-        data = LZ4.decompress(indata)
-    elif algo == 'lzma':
-        data = LZMA_ALONE.decompress(indata)
-    else:
+    if algo not in ALGORITHMS:
         raise ValueError("Unknown algorithm '%s'" % algo)
+
+    tool = _get_tool(algo)
+    data = tool.decompress(indata)
+
     return data
+
+def is_present(algo):
+     tool = _get_tool(algo)
+     return tool.is_present()
diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
index 057b4e28b7..96c15cff77 100644
--- a/tools/binman/ftest.py
+++ b/tools/binman/ftest.py
@@ -212,7 +212,7 @@ class TestFunctional(unittest.TestCase):
         TestFunctional._MakeInputFile('tee.elf',
             tools.read_file(cls.ElfTestFile('elf_sections')))
 
-        cls.have_lz4 = comp_util.HAVE_LZ4
+        cls.have_lz4 = comp_util.is_present('lz4')
 
     @classmethod
     def tearDownClass(cls):
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH v4 06/13] binman: Add compression tests
  2022-08-16  8:41 [PATCH v4 01/13] binman: Skip elf tests if python elftools is not available Stefan Herbrechtsmeier
                   ` (3 preceding siblings ...)
  2022-08-16  8:42 ` [PATCH v4 05/13] binman: Simplify comp_util class Stefan Herbrechtsmeier
@ 2022-08-16  8:42 ` Stefan Herbrechtsmeier
  2022-08-16  8:42 ` [PATCH v4 07/13] binman: Add BintoolPacker class to bintool Stefan Herbrechtsmeier
                   ` (6 subsequent siblings)
  11 siblings, 0 replies; 25+ messages in thread
From: Stefan Herbrechtsmeier @ 2022-08-16  8:42 UTC (permalink / raw)
  To: u-boot; +Cc: Stefan Herbrechtsmeier, Simon Glass, Alper Nebi Yasak

From: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com>

Add common test functions to test all supported compressions.

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

---
Instead of the for loop it is possible to use Parameterized [1] testing.

[1] https://github.com/wolever/parameterized

(no changes since v3)

Changes in v3:
- Use 'tools.get_bytes(0, 64)' instead of 'bytes([0]) * 64'
- Check if tool is present
- Rename tests

Changes in v2:
- Added

 tools/binman/ftest.py | 31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
index 96c15cff77..bc17107735 100644
--- a/tools/binman/ftest.py
+++ b/tools/binman/ftest.py
@@ -5248,6 +5248,37 @@ fdt         fdtmap                Extract the devicetree blob from the fdtmap
             comp_util.decompress(b'1234', 'invalid')
         self.assertIn("Unknown algorithm 'invalid'", str(e.exception))
 
+    def _checkCompUtil(self, algo):
+        if not comp_util.is_present(algo):
+            self.skipTest('%s not available' % comp_util._get_tool_name(algo))
+
+    def testCompUtilCompressions(self):
+        """Test compression algorithms"""
+        for algo in comp_util.ALGORITHMS:
+            self._checkCompUtil(algo)
+            data = comp_util.compress(COMPRESS_DATA, algo)
+            self.assertNotEqual(COMPRESS_DATA, data)
+            orig = comp_util.decompress(data, algo)
+            self.assertEquals(COMPRESS_DATA, orig)
+
+    def testCompUtilVersions(self):
+        """Test tool version of compression algorithms"""
+        for algo in comp_util.ALGORITHMS:
+            self._checkCompUtil(algo)
+            tool = comp_util._get_tool(algo)
+            version = tool.version()
+            print('%s - %s' % (algo, version))
+            self.assertRegex(version, '^v?[0-9]+[0-9.]*')
+
+    def testCompUtilPadding(self):
+        """Test padding of compression algorithms"""
+        for algo in comp_util.ALGORITHMS:
+            self._checkCompUtil(algo)
+            data = comp_util.compress(COMPRESS_DATA, algo)
+            data = data + tools.get_bytes(0, 64)
+            orig = comp_util.decompress(data, algo)
+            self.assertEquals(COMPRESS_DATA, orig)
+
     def testBintoolDocs(self):
         """Test for creation of bintool documentation"""
         with test_util.capture_sys_output() as (stdout, stderr):
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH v4 07/13] binman: Add BintoolPacker class to bintool
  2022-08-16  8:41 [PATCH v4 01/13] binman: Skip elf tests if python elftools is not available Stefan Herbrechtsmeier
                   ` (4 preceding siblings ...)
  2022-08-16  8:42 ` [PATCH v4 06/13] binman: Add compression tests Stefan Herbrechtsmeier
@ 2022-08-16  8:42 ` Stefan Herbrechtsmeier
  2022-08-17 18:53   ` Simon Glass
  2022-08-16  8:42 ` [PATCH v4 08/13] binman: Add bzip2 bintool Stefan Herbrechtsmeier
                   ` (5 subsequent siblings)
  11 siblings, 1 reply; 25+ messages in thread
From: Stefan Herbrechtsmeier @ 2022-08-16  8:42 UTC (permalink / raw)
  To: u-boot; +Cc: Stefan Herbrechtsmeier, Alper Nebi Yasak, Simon Glass

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 v3)

Changes in v3:
- Document class properties

Changes in v2:
- Added

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

diff --git a/tools/binman/bintool.py b/tools/binman/bintool.py
index 8435b29749..2eaad418f2 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,108 @@ 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
+        pkg = self.fetch_package if self.fetch_package else self.name
+        return self.apt_install(pkg)
+
+    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
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH v4 08/13] binman: Add bzip2 bintool
  2022-08-16  8:41 [PATCH v4 01/13] binman: Skip elf tests if python elftools is not available Stefan Herbrechtsmeier
                   ` (5 preceding siblings ...)
  2022-08-16  8:42 ` [PATCH v4 07/13] binman: Add BintoolPacker class to bintool Stefan Herbrechtsmeier
@ 2022-08-16  8:42 ` Stefan Herbrechtsmeier
  2022-08-16 11:48   ` Simon Glass
  2022-08-16  8:42 ` [PATCH v4 09/13] binman: Add gzip bintool Stefan Herbrechtsmeier
                   ` (4 subsequent siblings)
  11 siblings, 1 reply; 25+ messages in thread
From: Stefan Herbrechtsmeier @ 2022-08-16  8:42 UTC (permalink / raw)
  To: u-boot; +Cc: Stefan Herbrechtsmeier, Alper Nebi Yasak, Simon Glass

From: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com>

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

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

---

(no changes since v3)

Changes in v3:
- Rebase

Changes in v2:
- Added

 tools/binman/btool/bzip2.py | 30 ++++++++++++++++++++++++++++++
 tools/binman/comp_util.py   |  4 +++-
 2 files changed, 33 insertions(+), 1 deletion(-)
 create mode 100644 tools/binman/btool/bzip2.py

diff --git a/tools/binman/btool/bzip2.py b/tools/binman/btool/bzip2.py
new file mode 100644
index 0000000000..9be87a621f
--- /dev/null
+++ b/tools/binman/btool/bzip2.py
@@ -0,0 +1,30 @@
+# SPDX-License-Identifier: GPL-2.0+
+# Copyright (C) 2022 Weidmüller Interface GmbH & Co. KG
+# Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com>
+#
+"""Bintool implementation for bzip2
+
+bzip2 allows compression and decompression of files.
+
+Documentation is available via::
+
+   man bzip2
+"""
+
+from binman import bintool
+
+# pylint: disable=C0103
+class Bintoolbzip2(bintool.BintoolPacker):
+    """Compression/decompression using the bzip2 algorithm
+
+    This bintool supports running `bzip2` 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 bzip2
+    """
+    def __init__(self, name):
+        super().__init__(name, version_regex=r'bzip2.*Version ([0-9.]+)')
diff --git a/tools/binman/comp_util.py b/tools/binman/comp_util.py
index 00761d44cc..6ec371b145 100644
--- a/tools/binman/comp_util.py
+++ b/tools/binman/comp_util.py
@@ -7,6 +7,7 @@
 
 This supports the following compression algorithm:
   none
+  bzip2
   lz4
   lzma
 
@@ -14,6 +15,7 @@ Note that for lzma this uses an old version of the algorithm, not that
 provided by xz.
 
 This requires the following tools:
+  bzip2
   lz4
   lzma_alone
 
@@ -27,7 +29,7 @@ from binman import bintool
 from patman import tools
 
 # Supported compression algorithms
-ALGORITHMS = ['lz4', 'lzma']
+ALGORITHMS = ['bzip2', 'lz4', 'lzma']
 
 bintools = {}
 
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH v4 09/13] binman: Add gzip bintool
  2022-08-16  8:41 [PATCH v4 01/13] binman: Skip elf tests if python elftools is not available Stefan Herbrechtsmeier
                   ` (6 preceding siblings ...)
  2022-08-16  8:42 ` [PATCH v4 08/13] binman: Add bzip2 bintool Stefan Herbrechtsmeier
@ 2022-08-16  8:42 ` Stefan Herbrechtsmeier
  2022-08-16 11:48   ` Simon Glass
  2022-08-16  8:42 ` [PATCH v4 10/13] binman: Add lzop bintool Stefan Herbrechtsmeier
                   ` (3 subsequent siblings)
  11 siblings, 1 reply; 25+ messages in thread
From: Stefan Herbrechtsmeier @ 2022-08-16  8:42 UTC (permalink / raw)
  To: u-boot; +Cc: Stefan Herbrechtsmeier, Alper Nebi Yasak, Simon Glass

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 v3)

Changes in v3:
- Rebase

Changes in v2:
- Added

 tools/binman/btool/gzip.py | 31 +++++++++++++++++++++++++++++++
 tools/binman/comp_util.py  |  4 +++-
 2 files changed, 34 insertions(+), 1 deletion(-)
 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/comp_util.py b/tools/binman/comp_util.py
index 6ec371b145..19627e490c 100644
--- a/tools/binman/comp_util.py
+++ b/tools/binman/comp_util.py
@@ -8,6 +8,7 @@
 This supports the following compression algorithm:
   none
   bzip2
+  gzip
   lz4
   lzma
 
@@ -16,6 +17,7 @@ provided by xz.
 
 This requires the following tools:
   bzip2
+  gzip
   lz4
   lzma_alone
 
@@ -29,7 +31,7 @@ from binman import bintool
 from patman import tools
 
 # Supported compression algorithms
-ALGORITHMS = ['bzip2', 'lz4', 'lzma']
+ALGORITHMS = ['bzip2', 'gzip', 'lz4', 'lzma']
 
 bintools = {}
 
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH v4 10/13] binman: Add lzop bintool
  2022-08-16  8:41 [PATCH v4 01/13] binman: Skip elf tests if python elftools is not available Stefan Herbrechtsmeier
                   ` (7 preceding siblings ...)
  2022-08-16  8:42 ` [PATCH v4 09/13] binman: Add gzip bintool Stefan Herbrechtsmeier
@ 2022-08-16  8:42 ` Stefan Herbrechtsmeier
  2022-08-16 11:48   ` Simon Glass
  2022-08-16  8:42 ` [PATCH v4 11/13] binman: Add xz bintool Stefan Herbrechtsmeier
                   ` (2 subsequent siblings)
  11 siblings, 1 reply; 25+ messages in thread
From: Stefan Herbrechtsmeier @ 2022-08-16  8:42 UTC (permalink / raw)
  To: u-boot; +Cc: Stefan Herbrechtsmeier, Alper Nebi Yasak, Simon Glass

From: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com>

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

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

---

(no changes since v3)

Changes in v3:
- Rebase

Changes in v2:
- Added

 tools/binman/btool/lzop.py | 30 ++++++++++++++++++++++++++++++
 tools/binman/comp_util.py  |  6 ++++--
 2 files changed, 34 insertions(+), 2 deletions(-)
 create mode 100644 tools/binman/btool/lzop.py

diff --git a/tools/binman/btool/lzop.py b/tools/binman/btool/lzop.py
new file mode 100644
index 0000000000..f6903b4db7
--- /dev/null
+++ b/tools/binman/btool/lzop.py
@@ -0,0 +1,30 @@
+# SPDX-License-Identifier: GPL-2.0+
+# Copyright (C) 2022 Weidmüller Interface GmbH & Co. KG
+# Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com>
+#
+"""Bintool implementation for lzop
+
+lzop allows compression and decompression of files.
+
+Documentation is available via::
+
+   man lzop
+"""
+
+from binman import bintool
+
+# pylint: disable=C0103
+class Bintoollzop(bintool.BintoolPacker):
+    """Compression/decompression using the lzop algorithm
+
+    This bintool supports running `lzop` 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 lzop
+    """
+    def __init__(self, name):
+        super().__init__(name, 'lzo', compress_args=[])
diff --git a/tools/binman/comp_util.py b/tools/binman/comp_util.py
index 19627e490c..d8fdd5c7a2 100644
--- a/tools/binman/comp_util.py
+++ b/tools/binman/comp_util.py
@@ -11,6 +11,7 @@ This supports the following compression algorithm:
   gzip
   lz4
   lzma
+  lzo
 
 Note that for lzma this uses an old version of the algorithm, not that
 provided by xz.
@@ -20,6 +21,7 @@ This requires the following tools:
   gzip
   lz4
   lzma_alone
+  lzop
 
 It also requires an output directory to be previously set up, by calling
 PrepareOutputDir().
@@ -31,7 +33,7 @@ from binman import bintool
 from patman import tools
 
 # Supported compression algorithms
-ALGORITHMS = ['bzip2', 'gzip', 'lz4', 'lzma']
+ALGORITHMS = ['bzip2', 'gzip', 'lz4', 'lzma', 'lzo']
 
 bintools = {}
 
@@ -44,7 +46,7 @@ def _get_tool_name(algo):
     Returns:
         str: Tool name
     """
-    names = {'lzma': 'lzma_alone'}
+    names = {'lzma': 'lzma_alone', 'lzo': 'lzop'}
     return names.get(algo, algo)
 
 def _get_tool(algo):
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH v4 11/13] binman: Add xz bintool
  2022-08-16  8:41 [PATCH v4 01/13] binman: Skip elf tests if python elftools is not available Stefan Herbrechtsmeier
                   ` (8 preceding siblings ...)
  2022-08-16  8:42 ` [PATCH v4 10/13] binman: Add lzop bintool Stefan Herbrechtsmeier
@ 2022-08-16  8:42 ` Stefan Herbrechtsmeier
  2022-08-16 11:48   ` Simon Glass
  2022-08-16  8:42 ` [PATCH v4 12/13] binman: Add zstd bintool Stefan Herbrechtsmeier
  2022-08-16  8:42 ` [PATCH v4 13/13] binman: Support missing compression tools Stefan Herbrechtsmeier
  11 siblings, 1 reply; 25+ messages in thread
From: Stefan Herbrechtsmeier @ 2022-08-16  8:42 UTC (permalink / raw)
  To: u-boot; +Cc: Stefan Herbrechtsmeier, Alper Nebi Yasak, Simon Glass

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 v3)

Changes in v3:
- Rebase

Changes in v2:
- Added

 tools/binman/btool/xz.py  | 31 +++++++++++++++++++++++++++++++
 tools/binman/comp_util.py |  4 +++-
 2 files changed, 34 insertions(+), 1 deletion(-)
 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/comp_util.py b/tools/binman/comp_util.py
index d8fdd5c7a2..8c0fe5078f 100644
--- a/tools/binman/comp_util.py
+++ b/tools/binman/comp_util.py
@@ -12,6 +12,7 @@ This supports the following compression algorithm:
   lz4
   lzma
   lzo
+  xz
 
 Note that for lzma this uses an old version of the algorithm, not that
 provided by xz.
@@ -22,6 +23,7 @@ This requires the following tools:
   lz4
   lzma_alone
   lzop
+  xz
 
 It also requires an output directory to be previously set up, by calling
 PrepareOutputDir().
@@ -33,7 +35,7 @@ from binman import bintool
 from patman import tools
 
 # Supported compression algorithms
-ALGORITHMS = ['bzip2', 'gzip', 'lz4', 'lzma', 'lzo']
+ALGORITHMS = ['bzip2', 'gzip', 'lz4', 'lzma', 'lzo', 'xz']
 
 bintools = {}
 
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH v4 12/13] binman: Add zstd bintool
  2022-08-16  8:41 [PATCH v4 01/13] binman: Skip elf tests if python elftools is not available Stefan Herbrechtsmeier
                   ` (9 preceding siblings ...)
  2022-08-16  8:42 ` [PATCH v4 11/13] binman: Add xz bintool Stefan Herbrechtsmeier
@ 2022-08-16  8:42 ` Stefan Herbrechtsmeier
  2022-08-16 11:48   ` Simon Glass
  2022-08-16  8:42 ` [PATCH v4 13/13] binman: Support missing compression tools Stefan Herbrechtsmeier
  11 siblings, 1 reply; 25+ messages in thread
From: Stefan Herbrechtsmeier @ 2022-08-16  8:42 UTC (permalink / raw)
  To: u-boot; +Cc: Stefan Herbrechtsmeier, Alper Nebi Yasak, Simon Glass

From: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com>

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

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

---

(no changes since v3)

Changes in v3:
- Rebase

Changes in v2:
- Added

 tools/binman/btool/zstd.py     | 30 ++++++++++++++++++++++++++++++
 tools/binman/comp_util.py      |  4 +++-
 tools/binman/etype/blob_dtb.py |  4 ++++
 tools/binman/ftest.py          |  3 ++-
 4 files changed, 39 insertions(+), 2 deletions(-)
 create mode 100644 tools/binman/btool/zstd.py

diff --git a/tools/binman/btool/zstd.py b/tools/binman/btool/zstd.py
new file mode 100644
index 0000000000..299bd37126
--- /dev/null
+++ b/tools/binman/btool/zstd.py
@@ -0,0 +1,30 @@
+# SPDX-License-Identifier: GPL-2.0+
+# Copyright (C) 2022 Weidmüller Interface GmbH & Co. KG
+# Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com>
+#
+"""Bintool implementation for zstd
+
+zstd allows compression and decompression of files.
+
+Documentation is available via::
+
+   man zstd
+"""
+
+from binman import bintool
+
+# pylint: disable=C0103
+class Bintoolzstd(bintool.BintoolPacker):
+    """Compression/decompression using the zstd algorithm
+
+    This bintool supports running `zstd` 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 zstd
+    """
+    def __init__(self, name):
+        super().__init__(name)
diff --git a/tools/binman/comp_util.py b/tools/binman/comp_util.py
index 8c0fe5078f..6b4ab646e0 100644
--- a/tools/binman/comp_util.py
+++ b/tools/binman/comp_util.py
@@ -13,6 +13,7 @@ This supports the following compression algorithm:
   lzma
   lzo
   xz
+  zstd
 
 Note that for lzma this uses an old version of the algorithm, not that
 provided by xz.
@@ -24,6 +25,7 @@ This requires the following tools:
   lzma_alone
   lzop
   xz
+  zstd
 
 It also requires an output directory to be previously set up, by calling
 PrepareOutputDir().
@@ -35,7 +37,7 @@ from binman import bintool
 from patman import tools
 
 # Supported compression algorithms
-ALGORITHMS = ['bzip2', 'gzip', 'lz4', 'lzma', 'lzo', 'xz']
+ALGORITHMS = ['bzip2', 'gzip', 'lz4', 'lzma', 'lzo', 'xz', 'zstd']
 
 bintools = {}
 
diff --git a/tools/binman/etype/blob_dtb.py b/tools/binman/etype/blob_dtb.py
index 4fd2ecda83..fab79e43cc 100644
--- a/tools/binman/etype/blob_dtb.py
+++ b/tools/binman/etype/blob_dtb.py
@@ -48,6 +48,10 @@ class Entry_blob_dtb(Entry_blob):
     def ProcessContents(self):
         """Re-read the DTB contents so that we get any calculated properties"""
         _, indata = state.GetFdtContents(self.GetFdtEtype())
+
+        if self.compress == 'zstd' and self.prepend != 'length':
+            self.Raise('The zstd compression requires a length header')
+
         data = self.CompressData(indata)
         if self.prepend == 'length':
             hdr = struct.pack('<I', len(data))
diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
index bc17107735..a360ebeef5 100644
--- a/tools/binman/ftest.py
+++ b/tools/binman/ftest.py
@@ -5272,7 +5272,8 @@ fdt         fdtmap                Extract the devicetree blob from the fdtmap
 
     def testCompUtilPadding(self):
         """Test padding of compression algorithms"""
-        for algo in comp_util.ALGORITHMS:
+        # Skip zstd because it doesn't support padding
+        for algo in [a for a in comp_util.ALGORITHMS if a != 'zstd']:
             self._checkCompUtil(algo)
             data = comp_util.compress(COMPRESS_DATA, algo)
             data = data + tools.get_bytes(0, 64)
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH v4 13/13] binman: Support missing compression tools
  2022-08-16  8:41 [PATCH v4 01/13] binman: Skip elf tests if python elftools is not available Stefan Herbrechtsmeier
                   ` (10 preceding siblings ...)
  2022-08-16  8:42 ` [PATCH v4 12/13] binman: Add zstd bintool Stefan Herbrechtsmeier
@ 2022-08-16  8:42 ` Stefan Herbrechtsmeier
  2022-08-16 11:48   ` Simon Glass
  11 siblings, 1 reply; 25+ messages in thread
From: Stefan Herbrechtsmeier @ 2022-08-16  8:42 UTC (permalink / raw)
  To: u-boot; +Cc: Stefan Herbrechtsmeier, Alper Nebi Yasak, Simon Glass

From: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com>

Handle missing compression tools by returning empty data and marking the
entry as 'missing'.

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

---

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

Changes in v3:
- Added

 tools/binman/entry.py                            |  4 ++++
 tools/binman/ftest.py                            |  8 ++++++++
 .../test/236_compress_dtb_missing_bintool.dts    | 16 ++++++++++++++++
 3 files changed, 28 insertions(+)
 create mode 100644 tools/binman/test/236_compress_dtb_missing_bintool.dts

diff --git a/tools/binman/entry.py b/tools/binman/entry.py
index 9ec5811b46..c86b757a4e 100644
--- a/tools/binman/entry.py
+++ b/tools/binman/entry.py
@@ -1078,7 +1078,11 @@ features to produce new behaviours.
         """
         self.uncomp_data = indata
         if self.compress != 'none':
+            if not comp_util.is_present(self.compress):
+                self.missing = True
+                return b''
             self.uncomp_size = len(indata)
+
         data = comp_util.compress(indata, self.compress)
         return data
 
diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
index a360ebeef5..eac7ccb087 100644
--- a/tools/binman/ftest.py
+++ b/tools/binman/ftest.py
@@ -2557,6 +2557,14 @@ class TestFunctional(unittest.TestCase):
             }
         self.assertEqual(expected, props)
 
+    def testCompressMissingBintool(self):
+        """Test that compress of device-tree files with missing bintool is
+        supported
+        """
+        data = self.data = self._DoReadFileRealDtb('236_compress_dtb_missing_bintool.dts')
+        self.assertEqual(U_BOOT_DATA, data[:len(U_BOOT_DATA)])
+        dtb_data = data[len(U_BOOT_DATA):]
+        self.assertEqual(0, len(dtb_data))
 
     def testCbfsUpdateFdt(self):
         """Test that we can update the device tree with CBFS offset/size info"""
diff --git a/tools/binman/test/236_compress_dtb_missing_bintool.dts b/tools/binman/test/236_compress_dtb_missing_bintool.dts
new file mode 100644
index 0000000000..e7ce1b893d
--- /dev/null
+++ b/tools/binman/test/236_compress_dtb_missing_bintool.dts
@@ -0,0 +1,16 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+/dts-v1/;
+
+/ {
+	#address-cells = <1>;
+	#size-cells = <1>;
+
+	binman {
+		u-boot {
+		};
+		u-boot-dtb {
+			compress = "_testing";
+		};
+	};
+};
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* Re: [PATCH v4 02/13] binman: Add length header attribute to dtb entry
  2022-08-16  8:41 ` [PATCH v4 02/13] binman: Add length header attribute to dtb entry Stefan Herbrechtsmeier
@ 2022-08-16 11:48   ` Simon Glass
  0 siblings, 0 replies; 25+ messages in thread
From: Simon Glass @ 2022-08-16 11:48 UTC (permalink / raw)
  To: Stefan Herbrechtsmeier
  Cc: U-Boot Mailing List, Stefan Herbrechtsmeier, Alper Nebi Yasak

Hi Stefan,

On Tue, 16 Aug 2022 at 02:42, Stefan Herbrechtsmeier
<stefan.herbrechtsmeier-oss@weidmueller.com> wrote:
>
> 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 v3)

This looks fine except that it drops test coverage (binman test -T
should be 100%). Please can you check it?

Regards,
Simon

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v4 03/13] binman: Disable compressed data header
  2022-08-16  8:42 ` [PATCH v4 03/13] binman: Disable compressed data header Stefan Herbrechtsmeier
@ 2022-08-16 11:48   ` Simon Glass
  2022-08-16 12:02     ` Stefan Herbrechtsmeier
  0 siblings, 1 reply; 25+ messages in thread
From: Simon Glass @ 2022-08-16 11:48 UTC (permalink / raw)
  To: Stefan Herbrechtsmeier
  Cc: U-Boot Mailing List, Stefan Herbrechtsmeier, Alper Nebi Yasak,
	Heiko Thiery

Hi Stefan,

On Tue, 16 Aug 2022 at 02:42, Stefan Herbrechtsmeier
<stefan.herbrechtsmeier-oss@weidmueller.com> wrote:
>
> From: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com>
>
> Disable the compressed data header of the utilities to compress and
> decompress data. The header is uncommon, not supported by U-Boot and
> incompatible with external compressed artifacts.
>
> 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.
>
> Signed-off-by: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com>
>
> ---
>
> (no changes since v3)
>
> Changes in v3:
> - Added
>
>  tools/binman/entry.py         |  2 +-
>  tools/binman/etype/section.py |  3 ++-
>  tools/binman/ftest.py         | 18 +++++++++++-------
>  3 files changed, 14 insertions(+), 9 deletions(-)

This seems strange to me.

I would expect this patch to make every call set with_header to False,
then the next patch remove the parameters. That would allow later
bisecting to see where a problem occurs.

But at present this patch and the next seem to be mixed together.

>
> diff --git a/tools/binman/entry.py b/tools/binman/entry.py
> index e3767aefa7..a45aeeaa59 100644
> --- a/tools/binman/entry.py
> +++ b/tools/binman/entry.py
> @@ -1079,7 +1079,7 @@ features to produce new behaviours.
>          self.uncomp_data = indata
>          if self.compress != 'none':
>              self.uncomp_size = len(indata)
> -        data = comp_util.compress(indata, self.compress)
> +        data = comp_util.compress(indata, self.compress, with_header=False)
>          return data
>
>      @classmethod
> diff --git a/tools/binman/etype/section.py b/tools/binman/etype/section.py
> index bd67238b91..0a92bf2386 100644
> --- a/tools/binman/etype/section.py
> +++ b/tools/binman/etype/section.py
> @@ -777,7 +777,8 @@ class Entry_section(Entry):
>          data = parent_data[offset:offset + child.size]
>          if decomp:
>              indata = data
> -            data = comp_util.decompress(indata, child.compress)
> +            data = comp_util.decompress(indata, child.compress,
> +                                        with_header=False)
>              if child.uncomp_size:
>                  tout.info("%s: Decompressing data size %#x with algo '%s' to data size %#x" %
>                              (child.GetPath(), len(indata), child.compress,
> diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
> index 1b468d8e6d..d082442bec 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')
> +        return comp_util.decompress(data, 'lz4', with_header=False)
>
>      def testCompress(self):
>          """Test compression of blobs"""
> @@ -4449,15 +4449,19 @@ 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)])
> +        expect1 = comp_util.compress(COMPRESS_DATA + U_BOOT_DATA, 'lz4',
> +                                     with_header=False)
> +        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)])
> +        expect2 = comp_util.compress(COMPRESS_DATA + COMPRESS_DATA, 'lz4',
> +                                     with_header=False)
> +        data2 = rest1[:len(expect2)]
> +        section2 = self._decompress(data2)
> +        self.assertEquals(expect2, data2)
>          self.assertEquals(COMPRESS_DATA + COMPRESS_DATA, section2)
>          rest2 = rest1[len(expect2):]
>
> --
> 2.30.2
>

Regards,
Simon

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v4 05/13] binman: Simplify comp_util class
  2022-08-16  8:42 ` [PATCH v4 05/13] binman: Simplify comp_util class Stefan Herbrechtsmeier
@ 2022-08-16 11:48   ` Simon Glass
  0 siblings, 0 replies; 25+ messages in thread
From: Simon Glass @ 2022-08-16 11:48 UTC (permalink / raw)
  To: Stefan Herbrechtsmeier
  Cc: U-Boot Mailing List, Stefan Herbrechtsmeier, Alper Nebi Yasak

On Tue, 16 Aug 2022 at 02:42, Stefan Herbrechtsmeier
<stefan.herbrechtsmeier-oss@weidmueller.com> wrote:
>
> From: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com>
>
> Simplify the comp_util class by remove duplicate code as preparation for
> additional compress algorithms.
>
> Signed-off-by: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com>
>
> ---
>
> (no changes since v3)
>
> Changes in v3:
> - Rename COMPRESSIONS to ALGORITHMS
> - Rework existing comments
> - Add _get_tool_name function comment
> - Add _get_tool function comment
>
>  tools/binman/cbfs_util_test.py |   2 +-
>  tools/binman/comp_util.py      | 101 +++++++++++++++++++++++----------
>  tools/binman/ftest.py          |   2 +-
>  3 files changed, 72 insertions(+), 33 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v4 08/13] binman: Add bzip2 bintool
  2022-08-16  8:42 ` [PATCH v4 08/13] binman: Add bzip2 bintool Stefan Herbrechtsmeier
@ 2022-08-16 11:48   ` Simon Glass
  0 siblings, 0 replies; 25+ messages in thread
From: Simon Glass @ 2022-08-16 11:48 UTC (permalink / raw)
  To: Stefan Herbrechtsmeier
  Cc: U-Boot Mailing List, Stefan Herbrechtsmeier, Alper Nebi Yasak

On Tue, 16 Aug 2022 at 02:42, Stefan Herbrechtsmeier
<stefan.herbrechtsmeier-oss@weidmueller.com> wrote:
>
> From: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com>
>
> Add bzip2 bintool to binman to support on-the-fly compression.
>
> Signed-off-by: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com>
>
> ---
>
> (no changes since v3)
>
> Changes in v3:
> - Rebase
>
> Changes in v2:
> - Added
>
>  tools/binman/btool/bzip2.py | 30 ++++++++++++++++++++++++++++++
>  tools/binman/comp_util.py   |  4 +++-
>  2 files changed, 33 insertions(+), 1 deletion(-)
>  create mode 100644 tools/binman/btool/bzip2.py

Reviewed-by: Simon Glass <sjg@chromium.org>

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v4 09/13] binman: Add gzip bintool
  2022-08-16  8:42 ` [PATCH v4 09/13] binman: Add gzip bintool Stefan Herbrechtsmeier
@ 2022-08-16 11:48   ` Simon Glass
  0 siblings, 0 replies; 25+ messages in thread
From: Simon Glass @ 2022-08-16 11:48 UTC (permalink / raw)
  To: Stefan Herbrechtsmeier
  Cc: U-Boot Mailing List, Stefan Herbrechtsmeier, Alper Nebi Yasak

On Tue, 16 Aug 2022 at 02:42, Stefan Herbrechtsmeier
<stefan.herbrechtsmeier-oss@weidmueller.com> wrote:
>
> 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 v3)
>
> Changes in v3:
> - Rebase
>
> Changes in v2:
> - Added
>
>  tools/binman/btool/gzip.py | 31 +++++++++++++++++++++++++++++++
>  tools/binman/comp_util.py  |  4 +++-
>  2 files changed, 34 insertions(+), 1 deletion(-)
>  create mode 100644 tools/binman/btool/gzip.py

Reviewed-by: Simon Glass <sjg@chromium.org>

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v4 10/13] binman: Add lzop bintool
  2022-08-16  8:42 ` [PATCH v4 10/13] binman: Add lzop bintool Stefan Herbrechtsmeier
@ 2022-08-16 11:48   ` Simon Glass
  0 siblings, 0 replies; 25+ messages in thread
From: Simon Glass @ 2022-08-16 11:48 UTC (permalink / raw)
  To: Stefan Herbrechtsmeier
  Cc: U-Boot Mailing List, Stefan Herbrechtsmeier, Alper Nebi Yasak

On Tue, 16 Aug 2022 at 02:42, Stefan Herbrechtsmeier
<stefan.herbrechtsmeier-oss@weidmueller.com> wrote:
>
> From: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com>
>
> Add lzop bintool to binman to support on-the-fly compression.
>
> Signed-off-by: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com>
>
> ---
>
> (no changes since v3)
>
> Changes in v3:
> - Rebase
>
> Changes in v2:
> - Added
>
>  tools/binman/btool/lzop.py | 30 ++++++++++++++++++++++++++++++
>  tools/binman/comp_util.py  |  6 ++++--
>  2 files changed, 34 insertions(+), 2 deletions(-)
>  create mode 100644 tools/binman/btool/lzop.py

Reviewed-by: Simon Glass <sjg@chromium.org>

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v4 11/13] binman: Add xz bintool
  2022-08-16  8:42 ` [PATCH v4 11/13] binman: Add xz bintool Stefan Herbrechtsmeier
@ 2022-08-16 11:48   ` Simon Glass
  0 siblings, 0 replies; 25+ messages in thread
From: Simon Glass @ 2022-08-16 11:48 UTC (permalink / raw)
  To: Stefan Herbrechtsmeier
  Cc: U-Boot Mailing List, Stefan Herbrechtsmeier, Alper Nebi Yasak

On Tue, 16 Aug 2022 at 02:42, Stefan Herbrechtsmeier
<stefan.herbrechtsmeier-oss@weidmueller.com> wrote:
>
> 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 v3)
>
> Changes in v3:
> - Rebase
>
> Changes in v2:
> - Added
>
>  tools/binman/btool/xz.py  | 31 +++++++++++++++++++++++++++++++
>  tools/binman/comp_util.py |  4 +++-
>  2 files changed, 34 insertions(+), 1 deletion(-)
>  create mode 100644 tools/binman/btool/xz.py

Reviewed-by: Simon Glass <sjg@chromium.org>

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v4 12/13] binman: Add zstd bintool
  2022-08-16  8:42 ` [PATCH v4 12/13] binman: Add zstd bintool Stefan Herbrechtsmeier
@ 2022-08-16 11:48   ` Simon Glass
  0 siblings, 0 replies; 25+ messages in thread
From: Simon Glass @ 2022-08-16 11:48 UTC (permalink / raw)
  To: Stefan Herbrechtsmeier
  Cc: U-Boot Mailing List, Stefan Herbrechtsmeier, Alper Nebi Yasak

On Tue, 16 Aug 2022 at 02:42, Stefan Herbrechtsmeier
<stefan.herbrechtsmeier-oss@weidmueller.com> wrote:
>
> From: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com>
>
> Add zstd bintool to binman to support on-the-fly compression.
>
> Signed-off-by: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com>
>
> ---
>
> (no changes since v3)
>
> Changes in v3:
> - Rebase
>
> Changes in v2:
> - Added
>
>  tools/binman/btool/zstd.py     | 30 ++++++++++++++++++++++++++++++
>  tools/binman/comp_util.py      |  4 +++-
>  tools/binman/etype/blob_dtb.py |  4 ++++
>  tools/binman/ftest.py          |  3 ++-
>  4 files changed, 39 insertions(+), 2 deletions(-)
>  create mode 100644 tools/binman/btool/zstd.py

Reviewed-by: Simon Glass <sjg@chromium.org>

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v4 13/13] binman: Support missing compression tools
  2022-08-16  8:42 ` [PATCH v4 13/13] binman: Support missing compression tools Stefan Herbrechtsmeier
@ 2022-08-16 11:48   ` Simon Glass
  0 siblings, 0 replies; 25+ messages in thread
From: Simon Glass @ 2022-08-16 11:48 UTC (permalink / raw)
  To: Stefan Herbrechtsmeier
  Cc: U-Boot Mailing List, Stefan Herbrechtsmeier, Alper Nebi Yasak

Hi Stefan,

On Tue, 16 Aug 2022 at 02:42, Stefan Herbrechtsmeier
<stefan.herbrechtsmeier-oss@weidmueller.com> wrote:
>
> From: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com>
>
> Handle missing compression tools by returning empty data and marking the
> entry as 'missing'.

Great to see this.

This is actually a bit more subtle, see below.

>
> Signed-off-by: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com>
>
> ---
>
> Changes in v4:
> - Add missing 236_compress_dtb_missing_bintool.dts file
>
> Changes in v3:
> - Added
>
>  tools/binman/entry.py                            |  4 ++++
>  tools/binman/ftest.py                            |  8 ++++++++
>  .../test/236_compress_dtb_missing_bintool.dts    | 16 ++++++++++++++++
>  3 files changed, 28 insertions(+)
>  create mode 100644 tools/binman/test/236_compress_dtb_missing_bintool.dts
>
> diff --git a/tools/binman/entry.py b/tools/binman/entry.py
> index 9ec5811b46..c86b757a4e 100644
> --- a/tools/binman/entry.py
> +++ b/tools/binman/entry.py
> @@ -1078,7 +1078,11 @@ features to produce new behaviours.
>          """
>          self.uncomp_data = indata
>          if self.compress != 'none':
> +            if not comp_util.is_present(self.compress):
> +                self.missing = True

We must check self.GetAllowMissing(). If that is True then we can do
this. If False then we cannot. Hmm binman needs to fail if a bintool
is missing, unless --allow-missing is given.

Also you should call self.record_missing_bintool() here, instead of
setting self.missing (which refers to a missing blob).

BTW you also need to record the binbool somewhere with
self.AddBintools() in Entry:

def AddBintools(self, btools):
   self.comp_bintool = self.AddBintool(btools, self.compress)

That way you will get the right message, which is 'has missing bintools'

You then need to check for that stdout in your test, e.g. as is done
in testGbbMissing()

Finally, be careful to keep code coverage at 100%.

> +                return b''
>              self.uncomp_size = len(indata)
> +
>          data = comp_util.compress(indata, self.compress)
>          return data
>
> diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
> index a360ebeef5..eac7ccb087 100644
> --- a/tools/binman/ftest.py
> +++ b/tools/binman/ftest.py
> @@ -2557,6 +2557,14 @@ class TestFunctional(unittest.TestCase):
>              }
>          self.assertEqual(expected, props)
>
> +    def testCompressMissingBintool(self):
> +        """Test that compress of device-tree files with missing bintool is
> +        supported

Please add new tests at the end (so things are roughly in order of
test-file number). Also the test desc should fit on one like (e.g.
drop the 'Test that' text.

> +        """
> +        data = self.data = self._DoReadFileRealDtb('236_compress_dtb_missing_bintool.dts')

Can you drop self.data ?

> +        self.assertEqual(U_BOOT_DATA, data[:len(U_BOOT_DATA)])
> +        dtb_data = data[len(U_BOOT_DATA):]
> +        self.assertEqual(0, len(dtb_data))


>
>      def testCbfsUpdateFdt(self):
>          """Test that we can update the device tree with CBFS offset/size info"""
> diff --git a/tools/binman/test/236_compress_dtb_missing_bintool.dts b/tools/binman/test/236_compress_dtb_missing_bintool.dts
> new file mode 100644
> index 0000000000..e7ce1b893d
> --- /dev/null
> +++ b/tools/binman/test/236_compress_dtb_missing_bintool.dts
> @@ -0,0 +1,16 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +
> +/dts-v1/;
> +
> +/ {
> +       #address-cells = <1>;
> +       #size-cells = <1>;
> +
> +       binman {
> +               u-boot {
> +               };
> +               u-boot-dtb {
> +                       compress = "_testing";
> +               };
> +       };
> +};
> --
> 2.30.2
>

Regards,
Simon

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v4 03/13] binman: Disable compressed data header
  2022-08-16 11:48   ` Simon Glass
@ 2022-08-16 12:02     ` Stefan Herbrechtsmeier
  2022-08-16 20:42       ` Simon Glass
  0 siblings, 1 reply; 25+ messages in thread
From: Stefan Herbrechtsmeier @ 2022-08-16 12:02 UTC (permalink / raw)
  To: Simon Glass
  Cc: U-Boot Mailing List, Stefan Herbrechtsmeier, Alper Nebi Yasak,
	Heiko Thiery

Hi Simon,

Am 16.08.2022 um 13:48 schrieb Simon Glass:
> Hi Stefan,
> 
> On Tue, 16 Aug 2022 at 02:42, Stefan Herbrechtsmeier
> <stefan.herbrechtsmeier-oss@weidmueller.com> wrote:
>>
>> From: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com>
>>
>> Disable the compressed data header of the utilities to compress and
>> decompress data. The header is uncommon, not supported by U-Boot and
>> incompatible with external compressed artifacts.
>>
>> 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.
>>
>> Signed-off-by: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com>
>>
>> ---
>>
>> (no changes since v3)
>>
>> Changes in v3:
>> - Added
>>
>>   tools/binman/entry.py         |  2 +-
>>   tools/binman/etype/section.py |  3 ++-
>>   tools/binman/ftest.py         | 18 +++++++++++-------
>>   3 files changed, 14 insertions(+), 9 deletions(-)
> 
> This seems strange to me.
> 
> I would expect this patch to make every call set with_header to False,
> then the next patch remove the parameters. That would allow later
> bisecting to see where a problem occurs.
> 
> But at present this patch and the next seem to be mixed together.

I skipped the following calls because it doesn't matter:
comp_util.compress(b'',  'invalid')
comp_util.decompress(b'1234', 'invalid')

Do I miss a call?

The cbfs_util.py file doesn't use the header.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v4 03/13] binman: Disable compressed data header
  2022-08-16 12:02     ` Stefan Herbrechtsmeier
@ 2022-08-16 20:42       ` Simon Glass
  0 siblings, 0 replies; 25+ messages in thread
From: Simon Glass @ 2022-08-16 20:42 UTC (permalink / raw)
  To: Stefan Herbrechtsmeier
  Cc: U-Boot Mailing List, Stefan Herbrechtsmeier, Alper Nebi Yasak,
	Heiko Thiery

Hi Stefan,

On Tue, 16 Aug 2022 at 06:03, Stefan Herbrechtsmeier
<stefan.herbrechtsmeier-oss@weidmueller.com> wrote:
>
> Hi Simon,
>
> Am 16.08.2022 um 13:48 schrieb Simon Glass:
> > Hi Stefan,
> >
> > On Tue, 16 Aug 2022 at 02:42, Stefan Herbrechtsmeier
> > <stefan.herbrechtsmeier-oss@weidmueller.com> wrote:
> >>
> >> From: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com>
> >>
> >> Disable the compressed data header of the utilities to compress and
> >> decompress data. The header is uncommon, not supported by U-Boot and
> >> incompatible with external compressed artifacts.
> >>
> >> 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.
> >>
> >> Signed-off-by: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com>
> >>
> >> ---
> >>
> >> (no changes since v3)
> >>
> >> Changes in v3:
> >> - Added
> >>
> >>   tools/binman/entry.py         |  2 +-
> >>   tools/binman/etype/section.py |  3 ++-
> >>   tools/binman/ftest.py         | 18 +++++++++++-------
> >>   3 files changed, 14 insertions(+), 9 deletions(-)
> >
> > This seems strange to me.
> >
> > I would expect this patch to make every call set with_header to False,
> > then the next patch remove the parameters. That would allow later
> > bisecting to see where a problem occurs.
> >
> > But at present this patch and the next seem to be mixed together.
>
> I skipped the following calls because it doesn't matter:
> comp_util.compress(b'',  'invalid')
> comp_util.decompress(b'1234', 'invalid')

I still think it is worth it, because the default is True.

>
> Do I miss a call?

Everything should pass False in this patch I think.

>
> The cbfs_util.py file doesn't use the header.
OK

Regards,
SImon

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v4 07/13] binman: Add BintoolPacker class to bintool
  2022-08-16  8:42 ` [PATCH v4 07/13] binman: Add BintoolPacker class to bintool Stefan Herbrechtsmeier
@ 2022-08-17 18:53   ` Simon Glass
  0 siblings, 0 replies; 25+ messages in thread
From: Simon Glass @ 2022-08-17 18:53 UTC (permalink / raw)
  To: Stefan Herbrechtsmeier
  Cc: U-Boot Mailing List, Stefan Herbrechtsmeier, Alper Nebi Yasak

On Tue, 16 Aug 2022 at 02:42, Stefan Herbrechtsmeier
<stefan.herbrechtsmeier-oss@weidmueller.com> wrote:
>
> 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 v3)
>
> Changes in v3:
> - Document class properties
>
> Changes in v2:
> - Added
>
>  tools/binman/bintool.py | 107 ++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 107 insertions(+)

Reviewed-by: Simon Glass <sjg@chromium.org>

^ permalink raw reply	[flat|nested] 25+ messages in thread

end of thread, other threads:[~2022-08-17 18:54 UTC | newest]

Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-08-16  8:41 [PATCH v4 01/13] binman: Skip elf tests if python elftools is not available Stefan Herbrechtsmeier
2022-08-16  8:41 ` [PATCH v4 02/13] binman: Add length header attribute to dtb entry Stefan Herbrechtsmeier
2022-08-16 11:48   ` Simon Glass
2022-08-16  8:42 ` [PATCH v4 03/13] binman: Disable compressed data header Stefan Herbrechtsmeier
2022-08-16 11:48   ` Simon Glass
2022-08-16 12:02     ` Stefan Herbrechtsmeier
2022-08-16 20:42       ` Simon Glass
2022-08-16  8:42 ` [PATCH v4 04/13] binman: Remove obsolete compressed data header handling Stefan Herbrechtsmeier
2022-08-16  8:42 ` [PATCH v4 05/13] binman: Simplify comp_util class Stefan Herbrechtsmeier
2022-08-16 11:48   ` Simon Glass
2022-08-16  8:42 ` [PATCH v4 06/13] binman: Add compression tests Stefan Herbrechtsmeier
2022-08-16  8:42 ` [PATCH v4 07/13] binman: Add BintoolPacker class to bintool Stefan Herbrechtsmeier
2022-08-17 18:53   ` Simon Glass
2022-08-16  8:42 ` [PATCH v4 08/13] binman: Add bzip2 bintool Stefan Herbrechtsmeier
2022-08-16 11:48   ` Simon Glass
2022-08-16  8:42 ` [PATCH v4 09/13] binman: Add gzip bintool Stefan Herbrechtsmeier
2022-08-16 11:48   ` Simon Glass
2022-08-16  8:42 ` [PATCH v4 10/13] binman: Add lzop bintool Stefan Herbrechtsmeier
2022-08-16 11:48   ` Simon Glass
2022-08-16  8:42 ` [PATCH v4 11/13] binman: Add xz bintool Stefan Herbrechtsmeier
2022-08-16 11:48   ` Simon Glass
2022-08-16  8:42 ` [PATCH v4 12/13] binman: Add zstd bintool Stefan Herbrechtsmeier
2022-08-16 11:48   ` Simon Glass
2022-08-16  8:42 ` [PATCH v4 13/13] binman: Support missing compression tools Stefan Herbrechtsmeier
2022-08-16 11:48   ` Simon Glass

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.