[PATCH v2 02/10] binman: Make compressed data header optional
Stefan Herbrechtsmeier
stefan.herbrechtsmeier-oss at weidmueller.com
Mon Aug 8 12:51:17 CEST 2022
From: Stefan Herbrechtsmeier <stefan.herbrechtsmeier at weidmueller.com>
Move the compressed data header handling into the dtb blob entry class
and make it optional. The header is uncommon, not supported by U-Boot
and incompatible with external compressed artifacts.
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 that 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 at weidmueller.com>
---
Changes in v2:
- Reworked to make the feature optional.
tools/binman/cbfs_util.py | 8 ++---
tools/binman/comp_util.py | 11 ++----
tools/binman/entries.rst | 4 +++
tools/binman/entry.py | 2 +-
tools/binman/etype/blob_dtb.py | 21 ++++++++++++
tools/binman/ftest.py | 34 ++++++++++++++++---
.../test/235_compress_prepend_length_dtb.dts | 17 ++++++++++
7 files changed, 78 insertions(+), 19 deletions(-)
create mode 100644 tools/binman/test/235_compress_prepend_length_dtb.dts
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/entries.rst b/tools/binman/entries.rst
index ae4305c99e..8e722426d3 100644
--- a/tools/binman/entries.rst
+++ b/tools/binman/entries.rst
@@ -208,6 +208,10 @@ 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:
+ none: No header
+ length: 32-bit length header
Entry: blob-ext: Externally built binary blob
diff --git a/tools/binman/entry.py b/tools/binman/entry.py
index a07a588864..8cbfd43af9 100644
--- a/tools/binman/entry.py
+++ b/tools/binman/entry.py
@@ -1069,7 +1069,7 @@ 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':
diff --git a/tools/binman/etype/blob_dtb.py b/tools/binman/etype/blob_dtb.py
index 4159e3032a..652b8abd8f 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'), 'none' if none
"""
def __init__(self, section, etype, node):
# Put this here to allow entry-docs and help to work without libfdt
@@ -25,6 +30,12 @@ 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', 'none')
+
def ObtainContents(self):
"""Get the device-tree from the list held by the 'state' module"""
self._filename = self.GetDefaultFilename()
@@ -35,6 +46,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 +64,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..057b4e28b7 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()
@@ -2856,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):
@@ -4427,15 +4449,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):]
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
More information about the U-Boot
mailing list