[U-Boot] [PATCH v2 2/2] RISCV: image: Parse Image.gz support in booti.

[Atish Patra]

On 4/25/19 10:33 PM, Marek Vasut wrote:
> On 4/25/19 9:56 PM, Atish Patra wrote:
>> Add gz parsing logic so that booti can parse both Image
>> and Image.gz.
>>
>> Signed-off-by: Atish Patra <atish.patra at wdc.com>
>> ---
>>   arch/riscv/lib/image.c | 28 +++++++++++++++++++++++++++-
>>   1 file changed, 27 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/riscv/lib/image.c b/arch/riscv/lib/image.c
>> index e8802007c446..73ebd0da3885 100644
>> --- a/arch/riscv/lib/image.c
>> +++ b/arch/riscv/lib/image.c
>> @@ -9,6 +9,8 @@
>>   #include <common.h>
>>   #include <mapmem.h>
>>   #include <errno.h>
>> +#include <bootm.h>
>> +#include <malloc.h>
>>   #include <linux/sizes.h>
>>   #include <linux/stddef.h>
>>   
>> @@ -16,6 +18,8 @@ DECLARE_GLOBAL_DATA_PTR;
>>   
>>   /* ASCII version of "RISCV" defined in Linux kernel */
>>   #define LINUX_RISCV_IMAGE_MAGIC 0x5643534952
>> +#define GZ_HEADER_0 0x1f
>> +#define GZ_HEADER_1 0x8b
>>   
>>   struct linux_image_h {
>>   	uint32_t	code0;		/* Executable code */
>> @@ -32,9 +36,31 @@ int booti_setup(ulong image, ulong *relocated_addr, ulong *size,
>>   		bool force_reloc)
>>   {
>>   	struct linux_image_h *lhdr;
>> +	uint8_t *temp;
>> +	void *dest;
>> +	ulong dest_end;
>> +	int ret;
>> +	/* TODO: Is there a way to figure out length of compressed Image.gz ?
>> +	 * Otherwise, set it to SYS_BOOTM_LEN which should be sufficient.
>> +	 */
> 
> Presumably this is a RFC patch then ?
> 
Yeah. I am not very sure if there is a better way to determine the size.
Hence the comment. I am hoping somebody here would suggest something ;).

> What happens if you have two (or more) gzip-ed files back-to-back ?
> Wouldn't you then decompress both ? That might lead to all kinds of
> problems.
> 

That will be catastrophic. But this was intended only for booti and the 
expectation was that only Image.gz must be loaded before this.

Having said that, if we can find a reliable way of figuring out the 
compressed file size here, we can get rid of this hack.

>> +	int len = CONFIG_SYS_BOOTM_LEN;
>> +
>> +	temp = (uint8_t *)map_sysmem(image, 0);
> 
> Is the type cast really needed ?
> 

Just wanted to be explicit. Will remove it.

>> -	lhdr = (struct linux_image_h *)map_sysmem(image, 0);
>> +	if (*(temp)  == GZ_HEADER_0 && *(temp+1) == GZ_HEADER_1) {
> 
> Wrap the image in some fitImage or so contained, mark the image as gzip
> compressed there and all this goes away.
> 

Yes. FIT image parsing can be done in that way. However, the idea was 
here to load Image.gz directly. Image.gz is default compressed Linux 
kernel image format in RISC-V.

Regards,
Atish
>> +		/* Need a temporary location to copy the uncompressed image */
>> +		dest = (void *)map_sysmem(image + 8 * CONFIG_SYS_BOOTM_LEN, 0);
>> +		ret = bootm_decomp_image(IH_COMP_GZIP, 0, image, IH_TYPE_KERNEL,
>> +					 dest, (void *)image, len,
>> +					 CONFIG_SYS_BOOTM_LEN, &dest_end);
>> +		if (ret)
>> +			return ret;
>> +		/* dest_end contains the uncompressed Image size */
>> +		memmove((void *) image, dest, dest_end);
>> +		unmap_sysmem(dest);
>> +	}
>>   
>> +	lhdr = (struct linux_image_h *)temp;
>>   	if (lhdr->magic != LINUX_RISCV_IMAGE_MAGIC) {
>>   		puts("Bad Linux RISCV Image magic!\n");
>>   		return -EINVAL;
>>
> 
>