[U-Boot] [RFC] fit: include uncompression into fit_image_load

[Simon Glass]

Hi Simon,

On 16 October 2018 at 13:33, Simon Goldschmidt
<simon.k.r.goldschmidt at gmail.com> wrote:
> Currently, only the kernel can be compressed in a FIT image.
> By moving the uncompression logic to 'fit_image_load()', all types
> of images can be compressed.
>
> This works perfectly for me when using a gzip'ed FPGA image in
> a FIT image on a cyclone5 board (socrates). Also, a gzip'ed initrd
> being unzipped by U-Boot (not the kernel) worked.
>
> To clean this up, the uncompression function would have to be moved
> from bootm.c ('bootm_decomp_image()') to a more generic location,
> but I decided to keep it for now to make the patch easier to read.
> Because of this setup, the kernel is currently uncompressed twice.
> which doesn't work...
>
> There are, however, some more things to discuss:
> - The max. size passed to gunzip() (etc.) is not known before, so
>   we currently configure this to 8 MByte in U-Boot (via
>   CONFIG_SYS_BOOTM_LEN), which proved too small for my initrd...


We need the uncompressed size. If the legacy header doesn't have, stop
using it and use FIT?

Some compression formats include that in a header I think. But we
should record it in the U-Boot header.

> - CONFIG_SYS_BOOTM_LEN is set to 64 MByte default in SPL, so it's
>   a different default for SPL than for U-Boot !?!

Ick

> - CONFIG_SYS_BOOTM_LEN seemed to initially be used for kernel
>   uncompression but is now used as a copy-only limit, too (no unzip)

Yes

> - Uncompression only works if a load address is given, what should
>   happen if the FIT image does not contain a load address?

Fail.

> - The whole memory management around FIT images is a bit messed up
>   in that memory allocation is a mix of where U-Boot relocates itself
>   (and which address ranges it used), the load addresses of the FIT
>   image and the load addresses of the FIT image contents (and sizes).
>   What's the point here to check CONFIG_SYS_BOOTM_LEN? Maybe it would
>   be better to keep a memory map of allowed and already used data to
>   check if we're overwriting things or to get the maximum size passed
>   to gunzip etc.?

lmb is the mechanism for this and I think it checks for overlap.

> - Some code paths (like the command 'fpga loadmk') directly use
>   'fit_image_get_data()'. These will have to be converted to use
>   'fit_image_load()' instead (which will do nothing if the subimage
>   does not contain a load address.

OK

> - 'fit_image_load()' should probably check for subimages that have
>   a compression set but no load address... Or should we try to malloc()
>   here?

malloc() is generally small. Best to use lmb.

>
> A long list of questions, hopefully someone will join me in discussing
> them :-)

I think FIT provides a better format. We should be able to do the
right thing in all cases I believe.

>
> Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt at gmail.com>
> ---
>
>  common/bootm.c     |  8 ++++----
>  common/image-fit.c | 17 +++++++++++++++--
>  2 files changed, 19 insertions(+), 6 deletions(-)

Regards,
Simon