binman: Why is the first word the compressed size in compressed data
Hi Simon,
I want to compress a FPGA Image on the fly via binman but this doesn't work. I have add a bintool implementation for gzip, add gzip support to comp_util.py and set `compress` and `compression` property in the binman node of the u-boot dtsi:
fpga-2cg { compatible = "u-boot,fpga-legacy"; description = "FPGA"; type = "fpga"; compression = "gzip"; load = <(CONFIG_SYS_TEXT_BASE + 0x4000000)>;
blob-ext { compress = "gzip"; filename = "2cg.bit.bin"; }; };
It works if I remove the `compress` property and use a `2cg.bit.bin.gz` or remove the header for gzip [1].
Regarding the code binman add the compressed size as first word to the data [2]. The code in spl-fit.c doesn't remove the size [3]. Why is this size added?
[1] https://source.denx.de/u-boot/u-boot/-/blob/master/tools/binman/comp_util.py... [2] https://source.denx.de/u-boot/u-boot/-/blob/master/tools/binman/entry.py#L10... [3] https://source.denx.de/u-boot/u-boot/-/blob/master/common/spl/spl_fit.c#L334
Regards Stefan
On 01/06/2022 11:29, Stefan Herbrechtsmeier wrote:
Hi Simon,
I want to compress a FPGA Image on the fly via binman but this doesn't work. I have add a bintool implementation for gzip, add gzip support to comp_util.py and set `compress` and `compression` property in the binman node of the u-boot dtsi:
fpga-2cg { compatible = "u-boot,fpga-legacy"; description = "FPGA"; type = "fpga"; compression = "gzip"; load = <(CONFIG_SYS_TEXT_BASE + 0x4000000)>;
blob-ext { compress = "gzip"; filename = "2cg.bit.bin"; }; };
It works if I remove the `compress` property and use a `2cg.bit.bin.gz` or remove the header for gzip [1].
Regarding the code binman add the compressed size as first word to the data [2]. The code in spl-fit.c doesn't remove the size [3]. Why is this size added?
I looked into this a tiny bit, git blame eventually ends up at:
commit eb0f4a4cb40264a90a91e10e3ec00d1e0da7fb66 Author: Simon Glass sjg@chromium.org Date: 2019-07-20 12:24:06 -0600
binman: Support replacing data in a cbfs
At present binman cannot replace data within a CBFS since it does not allow rewriting of the files in that CBFS. Implement this by using the new WriteData() method to handle the case.
Add a header to compressed data so that the amount of compressed data can be determined without reference to the size of the containing entry. This allows the entry to be larger that the contents, without causing errors in decompression. 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). It is not used with CBFS since it has its own metadata for this. Increase the number of passes allowed to resolve the position of entries, to handle this case.
Add a test for this new logic.
Signed-off-by: Simon Glass sjg@chromium.org
For the record, I dislike the size header, since as you found out it turns the standard formats into custom formats that nothing else knows.
But I'm not familiar with binman's compression parts, I don't know what would go wrong if the header is removed and how. I'm also not sure what happens in the 'obscure case' the commit message mentions.
Am 02.06.2022 um 20:10 schrieb Alper Nebi Yasak:
On 01/06/2022 11:29, Stefan Herbrechtsmeier wrote:
Hi Simon,
I want to compress a FPGA Image on the fly via binman but this doesn't work. I have add a bintool implementation for gzip, add gzip support to comp_util.py and set `compress` and `compression` property in the binman node of the u-boot dtsi:
fpga-2cg { compatible = "u-boot,fpga-legacy"; description = "FPGA"; type = "fpga"; compression = "gzip"; load = <(CONFIG_SYS_TEXT_BASE + 0x4000000)>;
blob-ext { compress = "gzip"; filename = "2cg.bit.bin"; }; };
It works if I remove the `compress` property and use a `2cg.bit.bin.gz` or remove the header for gzip [1].
Regarding the code binman add the compressed size as first word to the data [2]. The code in spl-fit.c doesn't remove the size [3]. Why is this size added?
I looked into this a tiny bit, git blame eventually ends up at:
commit eb0f4a4cb40264a90a91e10e3ec00d1e0da7fb66 Author: Simon Glass sjg@chromium.org Date: 2019-07-20 12:24:06 -0600
binman: Support replacing data in a cbfs
At present binman cannot replace data within a CBFS since it does not allow rewriting of the files in that CBFS. Implement this by using the new WriteData() method to handle the case.
Add a header to compressed data so that the amount of compressed data can be determined without reference to the size of the containing entry. This allows the entry to be larger that the contents, without causing errors in decompression. 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). It is not used with CBFS since it has its own metadata for this. Increase the number of passes allowed to resolve the position of entries, to handle this case.
Add a test for this new logic.
Signed-off-by: Simon Glass sjg@chromium.org
Thanks. It looks like the header was added for internal compress and decompress. It shouldn't be part of the blob.
For the record, I dislike the size header, since as you found out it turns the standard formats into custom formats that nothing else knows.
Maybe I overlook something in the code but I don't find a support for the header inside the bootm or spl_fit code.
But I'm not familiar with binman's compression parts, I don't know what would go wrong if the header is removed and how. I'm also not sure what happens in the 'obscure case' the commit message mentions.
The header should be removed or the fit image should point to the offset behind the header. The compression test passed only because it supports the header.
On 03/06/2022 18:07, Stefan Herbrechtsmeier wrote:
Am 02.06.2022 um 20:10 schrieb Alper Nebi Yasak:
On 01/06/2022 11:29, Stefan Herbrechtsmeier wrote:
Hi Simon,
I want to compress a FPGA Image on the fly via binman but this doesn't work. I have add a bintool implementation for gzip, add gzip support to comp_util.py and set `compress` and `compression` property in the binman node of the u-boot dtsi:
fpga-2cg { compatible = "u-boot,fpga-legacy"; description = "FPGA"; type = "fpga"; compression = "gzip"; load = <(CONFIG_SYS_TEXT_BASE + 0x4000000)>;
blob-ext { compress = "gzip"; filename = "2cg.bit.bin"; }; };
It works if I remove the `compress` property and use a `2cg.bit.bin.gz` or remove the header for gzip [1].
Regarding the code binman add the compressed size as first word to the data [2]. The code in spl-fit.c doesn't remove the size [3]. Why is this size added?
I looked into this a tiny bit, git blame eventually ends up at:
commit eb0f4a4cb40264a90a91e10e3ec00d1e0da7fb66 Author: Simon Glass sjg@chromium.org Date: 2019-07-20 12:24:06 -0600
binman: Support replacing data in a cbfs
At present binman cannot replace data within a CBFS since it does not allow rewriting of the files in that CBFS. Implement this by using the new WriteData() method to handle the case.
Add a header to compressed data so that the amount of compressed data can be determined without reference to the size of the containing entry. This allows the entry to be larger that the contents, without causing errors in decompression. 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). It is not used with CBFS since it has its own metadata for this. Increase the number of passes allowed to resolve the position of entries, to handle this case.
Add a test for this new logic.
Signed-off-by: Simon Glass sjg@chromium.org
Thanks. It looks like the header was added for internal compress and decompress. It shouldn't be part of the blob.
For the record, I dislike the size header, since as you found out it turns the standard formats into custom formats that nothing else knows.
Maybe I overlook something in the code but I don't find a support for the header inside the bootm or spl_fit code.
But I'm not familiar with binman's compression parts, I don't know what would go wrong if the header is removed and how. I'm also not sure what happens in the 'obscure case' the commit message mentions.
The header should be removed or the fit image should point to the offset behind the header. The compression test passed only because it supports the header.
I agree that the header should be removed completely. I'll try to do it later on, but I don't know when I'll be able to.
participants (2)
-
Alper Nebi Yasak -
Stefan Herbrechtsmeier