Hi Simon,
Am 05.08.2022 um 18:48 schrieb Simon Glass:
Hi Stefan,
On Fri, 5 Aug 2022 at 01:51, Stefan Herbrechtsmeier
<stefan.herbrechtsmeier-...@weidmueller.com> wrote:
Hi Simon,
Am 04.08.2022 um 15:57 schrieb Simon Glass:
Hi Stefan,
On Thu, 4 Aug 2022 at 01:50, Stefan Herbrechtsmeier
<stefan.herbrechtsmeier-...@weidmueller.com> wrote:
Hi Simon,
Am 03.08.2022 um 20:14 schrieb Simon Glass:
Hi Stefan,
On Tue, 2 Aug 2022 at 07:45, Stefan Herbrechtsmeier
<stefan.herbrechtsmeier-...@weidmueller.com> wrote:
Hi Simon,
Am 02.08.2022 um 14:41 schrieb Simon Glass:
Hi Stefan,
On Tue, 2 Aug 2022 at 06:29, Stefan Herbrechtsmeier
<stefan.herbrechtsmeier-...@weidmueller.com> wrote:
From: Stefan Herbrechtsmeier <stefan.herbrechtsme...@weidmueller.com>
Remove header from compressed data because this 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 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 the compressed data
by itself.
I was not able to find a way around this due to the chicken-and egg
problem. Compressed data has an unpredictable size and adding an extra
uncompressed byte may increase or decrease the compressed size.
Is it possible to use the `pad-after` attribute to record the unused
space. In this case it is possible to calculate the size of the
compressed data.
Well if you update that attribute it can change the size of the DTB
which is the chicken-and-egg problem again. As far as I know, if
people set the size of the region to something a bit larger than
needed, the problem is avoided. But the image generation does need to
be deterministic.
Does this means the size is only needed for the creation of the fitImage
and not for decompression in u-boot?
Possibly, but of course we cannot do that. As you say, U-Boot mainline
does not expect or support the header, at present.
Do you have a test for this use case?
There are various ones, e.g. testCompressDtb()
Thanks. Now I understand the problem and why you call it a
chicken-and-egg problem. It wasn't clear to me that the attributes are
inside the DTB.
OK good.
But I wonder how the decompression of the DTB works if the fitImage
implementation doesn't support the header.
It doesn't. Something needs to change here for compression to work.
So my solution was to add the header.
Is the header used outside of binman? I don't spot it in the decompress
fitImage implementation.
It is used in the Chromium OS verified boot implementation, but not elsewhere.
It is optional though, so can we perhaps have a property in the
description to enable it?
Is this header needed and supported outside of binman?
At the moment the header is incompatible and not well documented. It
took me some time to find out why my gzip compression via binman doesn't
work as expected because I assume a compatibility between binman
compress and fitImage decompress.
Yes I understand that, however you can pass a parameter to not include
the size value.
Do we need the header outside of the DTB? Otherwise we could handle this
special or we could add a special compression type.
It would also be possible to add a property to the image (top-level
section) that indicates whether this field is present, such property
to apply to the whole image. We could have it default to off, if you
like.
Do we really need the header outside of the DTB entry?
That's an interesting question. It is possible that we only need it if
DTB is present and is compressed. It should be possible to check this
by adjusting the tests and checking for failures.
But I am not sure it is a good idea, since it is wildly inconsistent.
I do prefer things to be deterministic - i.e. you specify what you
want and you get it. If binman starts adopting obscure conventions it
could be confusing.
I add tests for gzip, lz4 and lzma_alone and all support padding at the
end and don't need the header. Even the testCompressDtb works without
the header. Furthermore I add bzip2, lzop, xz and zstd support to
bintool and only zstd doesn't support padding. Do we really need the
header or could we add an error if DTB is used together with zstd?
OK great!
Yes, I tried pretty hard to avoid it, but could not make it work.
Would you like me to take a look at the situations that spark it? It
might be around replacing things, too.
I really tried to avoid banning things, since it is such a pain and
confusing for people. But since this is error (and a corner case) I
think it would be fine to require a particular property to enable the
advanced functionality.
I have send a new version. Would be nice if you could check if you need
the new attribute to append the length header.
Should I commit bzip2, lzop, xz and zstd support to bintool?
I have add the tools to the series. Maybe the test environment need an
update to include all compression tools.
Regards
Stefan
