Re: [PATCH] kbuild: correct size calculation of bzImgae / fix x86 boot

[Michal Marek <mmarek@suse.cz>]

Sam Ravnborg wrote:
> The whole business using shell scripts to append the size seems broken.
> How about moving this functionality to mkpiggy where we are
> less shell script dependent.
> 
> Something like the following.
> I have only tested it lightly (vmlinux.bin did not differ
> before/after the patch.


I think that it's a good idea to move this out of the makefiles. But for
2.6.33, I think I prefer the patch from Andreas, provided he signs it
off and Michael G confirms that it fixes his issue.


> It includes the length also in the gzip case - I dunno if that matters.

I don't know either, but this is trivially fixable by an option to
mkpiggy that is added only if CONFIG_KERNEL_BZIP2=y or CONFIG_KERNEL_LZMA=y.


> Also I dunno if ".long" is the same on 32 and 64 bit.

I think we can work this out until the .34 merge window :).

Michal
> 
> 	Sam
> 
> diff --git a/arch/x86/boot/compressed/Makefile b/arch/x86/boot/compressed/Makefile
> index f8ed065..d39fe2e 100644
> --- a/arch/x86/boot/compressed/Makefile
> +++ b/arch/x86/boot/compressed/Makefile
> @@ -54,7 +54,8 @@ suffix-$(CONFIG_KERNEL_BZIP2)	:= bz2
>  suffix-$(CONFIG_KERNEL_LZMA)	:= lzma
>  
>  quiet_cmd_mkpiggy = MKPIGGY $@
> -      cmd_mkpiggy = $(obj)/mkpiggy $< > $@ || ( rm -f $@ ; false )
> +      cmd_mkpiggy = $(obj)/mkpiggy $< $(vmlinux.bin.all-y) > $@ || \
> +                   ( rm -f $@ ; false )
>  
>  targets += piggy.S
>  $(obj)/piggy.S: $(obj)/vmlinux.bin.$(suffix-y) $(obj)/mkpiggy FORCE
> diff --git a/arch/x86/boot/compressed/mkpiggy.c b/arch/x86/boot/compressed/mkpiggy.c
> index bcbd36c..f4a1a3f 100644
> --- a/arch/x86/boot/compressed/mkpiggy.c
> +++ b/arch/x86/boot/compressed/mkpiggy.c
> @@ -29,6 +29,7 @@
>  #include <stdio.h>
>  #include <string.h>
>  #include <inttypes.h>
> +#include <sys/stat.h>
>  
>  static uint32_t getle32(const void *p)
>  {
> @@ -38,12 +39,27 @@ static uint32_t getle32(const void *p)
>  		((uint32_t)cp[2] << 16) + ((uint32_t)cp[3] << 24);
>  }
>  
> +static file_size(const char *filename)
> +{
> +	struct stat statbuf;
> +	int res;
> +
> +	res = stat(filename, &statbuf);
> +	if (res < 0) {
> +		perror(filename);
> +		exit(1);
> +	}
> +	return (size_t)statbuf.st_size;
> +}
> +
>  int main(int argc, char *argv[])
>  {
>  	uint32_t olen;
>  	long ilen;
>  	unsigned long offs;
> +	size_t size;
>  	FILE *f;
> +	int i;
>  
>  	if (argc < 2) {
>  		fprintf(stderr, "Usage: %s compressed_file\n", argv[0]);
> @@ -67,6 +83,11 @@ int main(int argc, char *argv[])
>  	olen = getle32(&olen);
>  	fclose(f);
>  
> +	i = 2;
> +	size = 0;
> +	while (i < argc)
> +		size += file_size(argv[i++]);
> +
>  	/*
>  	 * Now we have the input (compressed) and output (uncompressed)
>  	 * sizes, compute the necessary decompression offset...
> @@ -91,6 +112,14 @@ int main(int argc, char *argv[])
>  	printf(".globl input_data, input_data_end\n");
>  	printf("input_data:\n");
>  	printf(".incbin \"%s\"\n", argv[1]);
> +
> +        /*
> +         * Bzip2 and LZMA do not include size in file... so we have to fake that;
> +         * append the size as a 32-bit littleendian number as gzip does.
> +         */
> +	if (size > 0)
> +		printf(".long %d\n", size);
> +
>  	printf("input_data_end:\n");
>  
>  	return 0;
> diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
> index cd815ac..10bef1c 100644
> --- a/scripts/Makefile.lib
> +++ b/scripts/Makefile.lib
> @@ -211,27 +211,14 @@ cmd_gzip = (cat $(filter-out FORCE,$^) | gzip -f -9 > $@) || \
>  # Bzip2
>  # ---------------------------------------------------------------------------
>  
> -# Bzip2 and LZMA do not include size in file... so we have to fake that;
> -# append the size as a 32-bit littleendian number as gzip does.
> -size_append = printf $(shell						\
> -dec_size=0;								\
> -for F in $1; do								\
> -	fsize=$$(stat -c "%s" $$F);					\
> -	dec_size=$$(expr $$dec_size + $$fsize);				\
> -done;									\
> -printf "%08x" $$dec_size |						\
> -	sed 's/\(..\)\(..\)\(..\)\(..\)/\\\\x\4\\\\x\3\\\\x\2\\\\x\1/g'	\
> -)
> -
>  quiet_cmd_bzip2 = BZIP2   $@
> -cmd_bzip2 = (cat $(filter-out FORCE,$^) | \
> -	bzip2 -9 && $(call size_append, $(filter-out FORCE,$^))) > $@ || \
> -	(rm -f $@ ; false)
> +cmd_bzip2 = cat $(filter-out FORCE,$^) | \
> +	bzip2 -9 > $@ || (rm -f $@ ; false)
>  
>  # Lzma
>  # ---------------------------------------------------------------------------
>  
>  quiet_cmd_lzma = LZMA    $@
> -cmd_lzma = (cat $(filter-out FORCE,$^) | \
> -	lzma -9 && $(call size_append, $(filter-out FORCE,$^))) > $@ || \
> -	(rm -f $@ ; false)
> +cmd_lzma = cat $(filter-out FORCE,$^) | \
> +	lzma -9 > $@ || (rm -f $@ ; false)
> +