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

[Simon Goldschmidt]

On 19.10.2018 05:25, Simon Glass wrote:
> Hi Simon,
>
> On 17 October 2018 at 03:41, Simon Goldschmidt
> <simon.k.r.goldschmidt at gmail.com> wrote:
>> On Wed, Oct 17, 2018 at 8:54 AM Alexander Graf <agraf at suse.de> wrote:
>>>
>>>
>>> On 16.10.18 21:33, Simon Goldschmidt 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.

OK, we could embed this information into the FIT by extracting in 
'mkimage'? That way, we know the uncompressed size...

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

Why do we need an extra limit for uncompressed copy-only? Both load 
address and size are known from the FIT.

>
>>>> - 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.?
> See lmb
>
>>> So I can at least give input on the memory map part :).
>>>
>>> In efi_loader, we actually do maintain a full system memory map already,
>>> including allocation functions that give you "safe" allocation
>>> functionality (allocate somewhere in memory where you know nothing
>>> overlaps).
>>>
>>> Maybe we should move this into a more generic system and reuse it for
>>> big memory allocations that really don't need to be in the U-Boot
>>> preallocated regions?
>> Hmm, inspecting this further, it seems that there is such an allocator
>> for bootm (using lmb_*() functions and struct lmb). Maybe this should be
>> better integrated into the fit loading function. I don't know if the
>> lmb functions
>> correctly detect overlapping of regions allocated by known addresses though.
>>
>> Thanks for your thoughts!
> Yes lmb is the right mechanism and I think it checks for overlap.

That didn't work for all overlap checks for me. I'll dig into it to see 
what's missing for me.


Simon