[PATCH 2/3] binman: Remove header from compressed data

[Simon Glass]

Hi Stefan,

On Tue, 2 Aug 2022 at 07:45, Stefan Herbrechtsmeier
<stefan.herbrechtsmeier-oss at 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-oss at weidmueller.com> wrote:
> >>
> >> From: Stefan Herbrechtsmeier <stefan.herbrechtsmeier at 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.

>
> Do you have a test for this use case?

There are various ones, e.g. testCompressDtb()

>
> > 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.

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.

Regards,
Simon