[PATCH 16/25] binman: Avoid reporting image-pos with compression

[Simon Glass]

Hi Alper,

On Mon, 26 Oct 2020 at 17:20, Alper Nebi Yasak <alpernebiyasak at gmail.com> wrote:
>
> On 26/10/2020 22:22, Simon Glass wrote:
> > Hi Alper,
> >
> > On Mon, 19 Oct 2020 at 15:29, Alper Nebi Yasak <alpernebiyasak at gmail.com> wrote:
> >>
> >> On 19/10/2020 05:41, Simon Glass wrote:
> >>> When a section is compressed, all entries within it are grouped together
> >>> into a compressed block of data. This obscures the start of each
> >>> individual child entry.
> >>>
> >>> Avoid reporting bogus 'image-pos' properties in this case, since it is
> >>> not possible to access the entry at the location provided. The entire
> >>> section must be decompressed first.
> >>>
> >>> CBFS does not support compressing whole sections, only individual files,
> >>> so needs no special handling here.
> >>>
> >>> Signed-off-by: Simon Glass <sjg at chromium.org>
> >>> ---
> >>
> >> Maybe instead of this, 'image-pos' could be overloaded to a list of
> >> recursive relative positions within compressed archives? Something like:
> >>
> >>     section {
> >>         offset = <1000>;
> >>         compress = "lz4";
> >>         /* image-pos = <1000>; */
> >>
> >>         blob {
> >>             filename = "foo";
> >>             offset = <100>;
> >>             /* image-pos = <1000>, <100>; */
> >>         };
> >>     };
> >>
> >>     cbfs {
> >>         offset = <2000>;
> >>         /* image-pos = <2000>; */
> >>
> >>         blob {
> >>             filename = "bar";
> >>             cbfs-compress = "lz4";
> >>             cbfs-offset = <200>;
> >>             /* image-pos = <2200>, <0>; */
> >>         };
> >>
> >>         blob2 {
> >>             filename = "baz";
> >>             cbfs-offset = <800>;
> >>             /* image-pos = <2800>; */
> >>         };
> >>     };
> >>
> >
> > What do these values actually mean though? What would we use them for?
>
> What I meant is using pairs of <position-of-compressed-archive>,
> <position-in-uncompressed-data> to avoid losing position information of
> compressed entries, but honestly I'm not sure if any of this will be
> necessary. I think the single use would be to avoid parsing uncompressed
> data containing multiple entries to identify what is where.
>
> To get to an entry in a compressed section we could get "size" bytes of
> data starting from "image-pos[0]", decompress that to somewhere, then
> get "uncomp-size" bytes of data starting from decompression address +
> "image-pos[1]".  I think it can even be extended to multiple layers of
> compression (<pos-of-x>, <pos-of-y-in-d(x)>, <pos-of-z-in-d(y)>, ...).
>
> > Note that CBFS compresses its data on a per-entry basis so the
> > image-pos is actually supported.
>
> On the compressed cbfs example, I assume the data we currently get at
> image-pos = <2200> is compressed data and still needs to be decompressed
> for it to be useable (technically bogus?). If so, I'd go a level deeper
> by adding a <0> indicating the start of the uncompressed data is the
> actual position of the entry contents.

For the CBFS case there isn't anything needed I think.

We already have an offset field which tells you where something is in
the compressed data. For the case where a compressed section has just
other entries in it, the offset is enough.

For the case where the compressed section has other uncompressed
sections, we need to add together the offsets (uncompressed section +
its entry) to get to the compressed location. It certainly has some
benefits.

But I wonder if instead we should have a properly like comp-pos that
tells you the absolute offset within the compressed section? I'm not a
big fan of making image-pos a multi-cell value.

Regards,
Simon