[RFC PATCH 2/3] spl: spl_legacy: Add lzma decompression support for legacy image

[Daniel Schwierzeck]

Am 10.04.20 um 13:04 schrieb Stefan Roese:
> From: Weijie Gao <weijie.gao at mediatek.com>
> 
> This patch adds support for decompressing LZMA compressed u-boot payload
> in legacy uImage format.
> 
> Using this patch together with u-boot-lzma.img may be useful for some
> platforms as they can reduce the size and load time of u-boot payload.
> 
> Signed-off-by: Weijie Gao <weijie.gao at mediatek.com>
> Signed-off-by: Stefan Roese <sr at denx.de>
> Cc: Daniel Schwierzeck <daniel.schwierzeck at gmail.com>
> Cc: Simon Goldschmidt <simon.k.r.goldschmidt at gmail.com>
> ---
>  common/spl/spl_legacy.c | 50 +++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 48 insertions(+), 2 deletions(-)

Reviewed-by: Daniel Schwierzeck <daniel.schwierzeck at gmail.com>

nits below

> 
> diff --git a/common/spl/spl_legacy.c b/common/spl/spl_legacy.c
> index 7f00fc8885..41734c026f 100644
> --- a/common/spl/spl_legacy.c
> +++ b/common/spl/spl_legacy.c
> @@ -4,8 +4,15 @@
>   */
>  
>  #include <common.h>
> +#include <malloc.h>
>  #include <spl.h>
>  
> +#include <lzma/LzmaTypes.h>
> +#include <lzma/LzmaDec.h>
> +#include <lzma/LzmaTools.h>
> +
> +#define LZMA_LEN	(1 << 20)
> +
>  int spl_parse_legacy_header(struct spl_image_info *spl_image,
>  			    const struct image_header *header)
>  {
> @@ -55,7 +62,10 @@ int spl_parse_legacy_header(struct spl_image_info *spl_image,
>  int spl_load_legacy_img(struct spl_image_info *spl_image,
>  			struct spl_load_info *load, ulong header)
>  {
> +	__maybe_unused SizeT lzma_len;
> +	__maybe_unused void *src;
>  	struct image_header hdr;
> +	ulong dataptr;
>  	int ret;
>  
>  	/* Read header into local struct */
> @@ -65,9 +75,45 @@ int spl_load_legacy_img(struct spl_image_info *spl_image,
>  	if (ret)
>  		return ret;
>  
> +	dataptr = header + sizeof(hdr);
> +
>  	/* Read image */
> -	load->read(load, header + sizeof(hdr), spl_image->size,
> -		   (void *)(unsigned long)spl_image->load_addr);
> +	switch (image_get_comp(&hdr)) {
> +	case IH_COMP_NONE:
> +		load->read(load, dataptr, spl_image->size,
> +			   (void *)(unsigned long)spl_image->load_addr);
> +		break;

to avoid the little increase of binary footprint due to the extra check
maybe a little wrapper like this could help:

static inline int spl_image_get_comp(const struct image_header *hdr)
{
    if (IS_ENABLED(CONFIG_SPL_LZMA) /* ||
        IS_ENABLED(CONFIG_SPL_ANOTHER_FANCY_COMPRESSION) */)
        return image_get_comp(hdr);

    return IH_COMP_NONE;
}

switch (spl_image_get_comp(&hdr)) {
    ...
}

then the compiler should optimise the switch/case statement away due to
Dead Code Elimination.

> +
> +#if IS_ENABLED(CONFIG_SPL_LZMA)
> +	case IH_COMP_LZMA:
> +		lzma_len = LZMA_LEN;
> +
> +		debug("LZMA: Decompressing %08lx to %08lx\n",
> +		      dataptr, spl_image->load_addr);
> +		src = malloc(spl_image->size);
> +		if (!src) {
> +			printf("Unable to allocate %d bytes for LZMA\n",
> +			       spl_image->size);
> +			return -ENOMEM;
> +		}
> +
> +		load->read(load, dataptr, spl_image->size, src);
> +		ret = lzmaBuffToBuffDecompress((void *)spl_image->load_addr,
> +					       &lzma_len, src, spl_image->size);
> +		if (ret) {
> +			printf("LZMA decompression error: %d\n", ret);
> +			return ret;
> +		}
> +
> +		spl_image->size = lzma_len;
> +		break;
> +#endif
> +
> +	default:
> +		debug("Compression method %s is not supported\n",
> +		      genimg_get_comp_short_name(image_get_comp(&hdr)));
> +		return -EINVAL;
> +	}
>  
>  	return 0;
>  }
> 

-- 
- Daniel