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

[Stefan Roese]

On 15.04.20 14:52, Daniel Schwierzeck wrote:
> 
> 
> 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.

Good idea. I was a bit concerned, even about minimal code size increase
in SPL as well. I'll work on this in v7.

Thanks,
Stefan

>> +
>> +#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;
>>   }
>>
> 


Viele Grüße,
Stefan

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr at denx.de