[Qemu-devel] improvements in uImage loading

[Qemu-devel] improvements in uImage loading

[Hollis Blanchard]

These are fixes and enhancements for loading uImages, a kernel format defined
by the u-boot bootloader. These are needed in order to merge PowerPC 440 KVM
support.

There are a few existing users of the uImage loader in qemu, but while I cannot
test these patches, they shouldn't cause any impact (other than adding gzip
loading capability).

-Hollis

[Qemu-devel] [PATCH 1/4] uImage: free file data after copying into guest memory

[Hollis Blanchard]

Signed-off-by: Hollis Blanchard <hollisb@us.ibm.com>
---
 loader.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/loader.c b/loader.c
index 8722909..7f4e69c 100644
--- a/loader.c
+++ b/loader.c
@@ -399,6 +399,7 @@ int load_uboot(const char *filename, target_ulong *ep, int *is_linux)
     }
 
     cpu_physical_memory_write_rom(hdr->ih_load, data, hdr->ih_size);
+    qemu_free(data);
 
     return hdr->ih_size;
 
-- 
1.5.6.5

[Qemu-devel] [PATCH 2/4] uImage: implement gzip support

[Hollis Blanchard]

Based on gzip uImage loading code from u-boot.

Signed-off-by: Jerone Young <jyoung5@us.ibm.com>
Signed-off-by: Hollis Blanchard <hollisb@us.ibm.com>
---
 loader.c |  110 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++--
 1 files changed, 106 insertions(+), 4 deletions(-)

diff --git a/loader.c b/loader.c
index 7f4e69c..c9cb906 100644
--- a/loader.c
+++ b/loader.c
@@ -26,6 +26,8 @@
 #include "sysemu.h"
 #include "uboot_image.h"
 
+#include <zlib.h>
+
 /* return the size or -1 if error */
 int get_image_size(const char *filename)
 {
@@ -345,10 +347,93 @@ static void bswap_uboot_header(uboot_image_header_t *hdr)
 #endif
 }
 
+/* gunzip functionality is derived from gunzip function
+ * in uboot source code
+ */
+
+#define	ZALLOC_ALIGNMENT	16
+
+static void *zalloc(void *x, unsigned items, unsigned size)
+{
+	void *p;
+
+	size *= items;
+	size = (size + ZALLOC_ALIGNMENT - 1) & ~(ZALLOC_ALIGNMENT - 1);
+
+	p = qemu_malloc(size);
+
+	return (p);
+}
+
+static void zfree(void *x, void *addr, unsigned nb)
+{
+	qemu_free(addr);
+}
+
+
+#define HEAD_CRC	2
+#define EXTRA_FIELD	4
+#define ORIG_NAME	8
+#define COMMENT		0x10
+#define RESERVED	0xe0
+
+#define DEFLATED	8
+
+static ssize_t gunzip(void *dst, size_t dstlen, uint8_t *src,
+                      size_t srclen)
+{
+	z_stream s;
+	ssize_t dstbytes;
+	int r, i, flags;
+
+	/* skip header */
+	i = 10;
+	flags = src[3];
+	if (src[2] != DEFLATED || (flags & RESERVED) != 0) {
+		puts ("Error: Bad gzipped data\n");
+		return -1;
+	}
+	if ((flags & EXTRA_FIELD) != 0)
+		i = 12 + src[10] + (src[11] << 8);
+	if ((flags & ORIG_NAME) != 0)
+		while (src[i++] != 0)
+			;
+	if ((flags & COMMENT) != 0)
+		while (src[i++] != 0)
+			;
+	if ((flags & HEAD_CRC) != 0)
+		i += 2;
+	if (i >= srclen) {
+		puts ("Error: gunzip out of data in header\n");
+		return -1;
+	}
+
+	s.zalloc = zalloc;
+	s.zfree = (free_func)zfree;
+
+	r = inflateInit2(&s, -MAX_WBITS);
+	if (r != Z_OK) {
+		printf ("Error: inflateInit2() returned %d\n", r);
+		return (-1);
+	}
+	s.next_in = src + i;
+	s.avail_in = srclen - i;
+	s.next_out = dst;
+	s.avail_out = dstlen;
+	r = inflate(&s, Z_FINISH);
+	if (r != Z_OK && r != Z_STREAM_END) {
+		printf ("Error: inflate() returned %d\n", r);
+		return -1;
+	}
+	dstbytes = s.next_out - (unsigned char *) dst;
+	inflateEnd(&s);
+
+	return dstbytes;
+}
+
 /* Load a U-Boot image.  */
 int load_uboot(const char *filename, target_ulong *ep, int *is_linux)
 {
-
     int fd;
     int size;
     uboot_image_header_t h;
@@ -374,9 +459,9 @@ int load_uboot(const char *filename, target_ulong *ep, int *is_linux)
         goto fail;
     }
 
-    /* TODO: Implement compressed images.  */
-    if (hdr->ih_comp != IH_COMP_NONE) {
-        fprintf(stderr, "Unable to load compressed u-boot images\n");
+    /* TODO bzip2 support */
+    if (hdr->ih_comp == IH_COMP_BZIP2) {
+        fprintf(stderr, "Unable to load bzip2 compressed u-boot images\n");
         goto fail;
     }
 
@@ -398,6 +483,23 @@ int load_uboot(const char *filename, target_ulong *ep, int *is_linux)
         goto fail;
     }
 
+    if (hdr->ih_comp == IH_COMP_GZIP) {
+        uint8_t *compressed_data;
+        size_t max_bytes = 2 * hdr->ih_size;
+        ssize_t bytes;
+
+        compressed_data = data;
+        data = qemu_malloc(max_bytes);
+
+        bytes = gunzip(data, max_bytes, compressed_data, hdr->ih_size);
+        qemu_free(compressed_data);
+        if (bytes < 0) {
+            fprintf(stderr, "Unable to decompress gzipped image!\n");
+            goto fail;
+        }
+        hdr->ih_size = bytes;
+    }
+
     cpu_physical_memory_write_rom(hdr->ih_load, data, hdr->ih_size);
     qemu_free(data);
 
-- 
1.5.6.5

[Qemu-devel] [PATCH 3/4] uImage: return base load address

[Hollis Blanchard]

Return the base address at which the image was loaded so that callers may keep
track of currently occupied guest memory.

Signed-off-by: Hollis Blanchard <hollisb@us.ibm.com>
---
 hw/an5206.c     |    2 +-
 hw/arm_boot.c   |    3 ++-
 hw/dummy_m68k.c |    2 +-
 hw/mcf5208.c    |    2 +-
 loader.c        |    6 +++++-
 sysemu.h        |    3 ++-
 6 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/hw/an5206.c b/hw/an5206.c
index 9d315f3..bb9cde1 100644
--- a/hw/an5206.c
+++ b/hw/an5206.c
@@ -68,7 +68,7 @@ static void an5206_init(ram_addr_t ram_size, int vga_ram_size,
     kernel_size = load_elf(kernel_filename, 0, &elf_entry, NULL, NULL);
     entry = elf_entry;
     if (kernel_size < 0) {
-        kernel_size = load_uboot(kernel_filename, &entry, NULL);
+        kernel_size = load_uboot(kernel_filename, &entry, NULL, NULL);
     }
     if (kernel_size < 0) {
         kernel_size = load_image(kernel_filename,
diff --git a/hw/arm_boot.c b/hw/arm_boot.c
index 5990961..765075d 100644
--- a/hw/arm_boot.c
+++ b/hw/arm_boot.c
@@ -205,7 +205,8 @@ void arm_load_kernel(CPUState *env, struct arm_boot_info *info)
     kernel_size = load_elf(info->kernel_filename, 0, &elf_entry, NULL, NULL);
     entry = elf_entry;
     if (kernel_size < 0) {
-        kernel_size = load_uboot(info->kernel_filename, &entry, &is_linux);
+        kernel_size = load_uboot(info->kernel_filename, &entry, NULL,
+                                 &is_linux);
     }
     if (kernel_size < 0) {
         kernel_size = load_image(info->kernel_filename,
diff --git a/hw/dummy_m68k.c b/hw/dummy_m68k.c
index fc06e9c..5cd5358 100644
--- a/hw/dummy_m68k.c
+++ b/hw/dummy_m68k.c
@@ -44,7 +44,7 @@ static void dummy_m68k_init(ram_addr_t ram_size, int vga_ram_size,
         kernel_size = load_elf(kernel_filename, 0, &elf_entry, NULL, NULL);
         entry = elf_entry;
         if (kernel_size < 0) {
-            kernel_size = load_uboot(kernel_filename, &entry, NULL);
+            kernel_size = load_uboot(kernel_filename, &entry, NULL, NULL);
         }
         if (kernel_size < 0) {
             kernel_size = load_image(kernel_filename,
diff --git a/hw/mcf5208.c b/hw/mcf5208.c
index 3e0a811..82db38d 100644
--- a/hw/mcf5208.c
+++ b/hw/mcf5208.c
@@ -290,7 +290,7 @@ static void mcf5208evb_init(ram_addr_t ram_size, int vga_ram_size,
     kernel_size = load_elf(kernel_filename, 0, &elf_entry, NULL, NULL);
     entry = elf_entry;
     if (kernel_size < 0) {
-        kernel_size = load_uboot(kernel_filename, &entry, NULL);
+        kernel_size = load_uboot(kernel_filename, &entry, NULL, NULL);
     }
     if (kernel_size < 0) {
         kernel_size = load_image(kernel_filename, phys_ram_base);
diff --git a/loader.c b/loader.c
index c9cb906..9ad3c96 100644
--- a/loader.c
+++ b/loader.c
@@ -432,7 +432,8 @@ static ssize_t gunzip(void *dst, size_t dstlen, uint8_t *src,
 }
 
 /* Load a U-Boot image.  */
-int load_uboot(const char *filename, target_ulong *ep, int *is_linux)
+int load_uboot(const char *filename, target_ulong *ep, target_ulong *loadaddr,
+               int *is_linux)
 {
     int fd;
     int size;
@@ -503,6 +504,9 @@ int load_uboot(const char *filename, target_ulong *ep, int *is_linux)
     cpu_physical_memory_write_rom(hdr->ih_load, data, hdr->ih_size);
     qemu_free(data);
 
+    if (loadaddr)
+        *loadaddr = hdr->ih_load;
+
     return hdr->ih_size;
 
 fail:
diff --git a/sysemu.h b/sysemu.h
index ef0fe50..d3d3203 100644
--- a/sysemu.h
+++ b/sysemu.h
@@ -165,7 +165,8 @@ int load_image_targphys(const char *filename, target_phys_addr_t, int max_sz);
 int load_elf(const char *filename, int64_t address_offset,
              uint64_t *pentry, uint64_t *lowaddr, uint64_t *highaddr);
 int load_aout(const char *filename, target_phys_addr_t addr, int max_sz);
-int load_uboot(const char *filename, target_ulong *ep, int *is_linux);
+int load_uboot(const char *filename, target_ulong *ep, target_ulong *loadaddr,
+               int *is_linux);
 
 int fread_targphys(target_phys_addr_t dst_addr, size_t nbytes, FILE *f);
 int fread_targphys_ok(target_phys_addr_t dst_addr, size_t nbytes, FILE *f);
-- 
1.5.6.5

[Qemu-devel] [PATCH 4/4] uImage: rename load_uboot() to load_uimage()

[Hollis Blanchard]

u-boot is a bootloader. uImage is an executable file format.

Signed-off-by: Hollis Blanchard <hollisb@us.ibm.com>
---
 hw/an5206.c     |    2 +-
 hw/arm_boot.c   |    4 ++--
 hw/dummy_m68k.c |    2 +-
 hw/mcf5208.c    |    2 +-
 loader.c        |    4 ++--
 patches         |    1 +
 sysemu.h        |    4 ++--
 7 files changed, 10 insertions(+), 9 deletions(-)
 create mode 120000 patches

diff --git a/hw/an5206.c b/hw/an5206.c
index bb9cde1..98f35e3 100644
--- a/hw/an5206.c
+++ b/hw/an5206.c
@@ -68,7 +68,7 @@ static void an5206_init(ram_addr_t ram_size, int vga_ram_size,
     kernel_size = load_elf(kernel_filename, 0, &elf_entry, NULL, NULL);
     entry = elf_entry;
     if (kernel_size < 0) {
-        kernel_size = load_uboot(kernel_filename, &entry, NULL, NULL);
+        kernel_size = load_uimage(kernel_filename, &entry, NULL, NULL);
     }
     if (kernel_size < 0) {
         kernel_size = load_image(kernel_filename,
diff --git a/hw/arm_boot.c b/hw/arm_boot.c
index 765075d..cf9616a 100644
--- a/hw/arm_boot.c
+++ b/hw/arm_boot.c
@@ -205,8 +205,8 @@ void arm_load_kernel(CPUState *env, struct arm_boot_info *info)
     kernel_size = load_elf(info->kernel_filename, 0, &elf_entry, NULL, NULL);
     entry = elf_entry;
     if (kernel_size < 0) {
-        kernel_size = load_uboot(info->kernel_filename, &entry, NULL,
-                                 &is_linux);
+        kernel_size = load_uimage(info->kernel_filename, &entry, NULL,
+                                  &is_linux);
     }
     if (kernel_size < 0) {
         kernel_size = load_image(info->kernel_filename,
diff --git a/hw/dummy_m68k.c b/hw/dummy_m68k.c
index 5cd5358..7931b6d 100644
--- a/hw/dummy_m68k.c
+++ b/hw/dummy_m68k.c
@@ -44,7 +44,7 @@ static void dummy_m68k_init(ram_addr_t ram_size, int vga_ram_size,
         kernel_size = load_elf(kernel_filename, 0, &elf_entry, NULL, NULL);
         entry = elf_entry;
         if (kernel_size < 0) {
-            kernel_size = load_uboot(kernel_filename, &entry, NULL, NULL);
+            kernel_size = load_uimage(kernel_filename, &entry, NULL, NULL);
         }
         if (kernel_size < 0) {
             kernel_size = load_image(kernel_filename,
diff --git a/hw/mcf5208.c b/hw/mcf5208.c
index 82db38d..f9ae588 100644
--- a/hw/mcf5208.c
+++ b/hw/mcf5208.c
@@ -290,7 +290,7 @@ static void mcf5208evb_init(ram_addr_t ram_size, int vga_ram_size,
     kernel_size = load_elf(kernel_filename, 0, &elf_entry, NULL, NULL);
     entry = elf_entry;
     if (kernel_size < 0) {
-        kernel_size = load_uboot(kernel_filename, &entry, NULL, NULL);
+        kernel_size = load_uimage(kernel_filename, &entry, NULL, NULL);
     }
     if (kernel_size < 0) {
         kernel_size = load_image(kernel_filename, phys_ram_base);
diff --git a/loader.c b/loader.c
index 9ad3c96..dd8f92d 100644
--- a/loader.c
+++ b/loader.c
@@ -432,8 +432,8 @@ static ssize_t gunzip(void *dst, size_t dstlen, uint8_t *src,
 }
 
 /* Load a U-Boot image.  */
-int load_uboot(const char *filename, target_ulong *ep, target_ulong *loadaddr,
-               int *is_linux)
+int load_uimage(const char *filename, target_ulong *ep, target_ulong *loadaddr,
+                int *is_linux)
 {
     int fd;
     int size;
diff --git a/patches b/patches
new file mode 120000
index 0000000..848fc19
--- /dev/null
+++ b/patches
@@ -0,0 +1 @@
+.git/patches/master
\ No newline at end of file
diff --git a/sysemu.h b/sysemu.h
index d3d3203..f72ec96 100644
--- a/sysemu.h
+++ b/sysemu.h
@@ -165,8 +165,8 @@ int load_image_targphys(const char *filename, target_phys_addr_t, int max_sz);
 int load_elf(const char *filename, int64_t address_offset,
              uint64_t *pentry, uint64_t *lowaddr, uint64_t *highaddr);
 int load_aout(const char *filename, target_phys_addr_t addr, int max_sz);
-int load_uboot(const char *filename, target_ulong *ep, target_ulong *loadaddr,
-               int *is_linux);
+int load_uimage(const char *filename, target_ulong *ep, target_ulong *loadaddr,
+                int *is_linux);
 
 int fread_targphys(target_phys_addr_t dst_addr, size_t nbytes, FILE *f);
 int fread_targphys_ok(target_phys_addr_t dst_addr, size_t nbytes, FILE *f);
-- 
1.5.6.5

Re: [Qemu-devel] [PATCH 2/4] uImage: implement gzip support

[François Revol]

> -    /* TODO: Implement compressed images.  */
> -    if (hdr->ih_comp != IH_COMP_NONE) {
> -        fprintf(stderr, "Unable to load compressed u-boot images\
> n");
> +    /* TODO bzip2 support */
> +    if (hdr->ih_comp == IH_COMP_BZIP2) {
> +        fprintf(stderr, "Unable to load bzip2 compressed u-boot
> images\n");

Shouldn't it still be testing for !=NONE && !=GZIP ?
In case the header is incorrect or new compressions are added ?

François.

Re: [Qemu-devel] [PATCH 2/4] uImage: implement gzip support

[Hollis Blanchard]

On Mon, 2008-11-17 at 22:30 +0100, François Revol wrote:
> > -    /* TODO: Implement compressed images.  */
> > -    if (hdr->ih_comp != IH_COMP_NONE) {
> > -        fprintf(stderr, "Unable to load compressed u-boot images\
> > n");
> > +    /* TODO bzip2 support */
> > +    if (hdr->ih_comp == IH_COMP_BZIP2) {
> > +        fprintf(stderr, "Unable to load bzip2 compressed u-boot 
> > images\n");
> 
> Shouldn't it still be testing for !=NONE && !=GZIP ?
> In case the header is incorrect or new compressions are added ?

I don't see what you mean about the header being incorrect, but I agree
on the second point. I'll make this change.

-- 
Hollis Blanchard
IBM Linux Technology Center

Re: [Qemu-devel] [PATCH 2/4] uImage: implement gzip support

[Jean-Christophe PLAGNIOL-VILLARD]

On 15:18 Mon 17 Nov     , Hollis Blanchard wrote:
> Based on gzip uImage loading code from u-boot.
> 
> Signed-off-by: Jerone Young <jyoung5@us.ibm.com>
> Signed-off-by: Hollis Blanchard <hollisb@us.ibm.com>
> ---
>  loader.c |  110 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++--
>  1 files changed, 106 insertions(+), 4 deletions(-)
> 
> diff --git a/loader.c b/loader.c
> index 7f4e69c..c9cb906 100644
> --- a/loader.c
> +++ b/loader.c
> @@ -26,6 +26,8 @@
>  #include "sysemu.h"
>  #include "uboot_image.h"
>  
> +#include <zlib.h>
> +
>  /* return the size or -1 if error */
>  int get_image_size(const char *filename)
>  {
> @@ -345,10 +347,93 @@ static void bswap_uboot_header(uboot_image_header_t *hdr)
>  #endif
>  }
>  
> +/* gunzip functionality is derived from gunzip function
> + * in uboot source code
> + */
> +
> +#define	ZALLOC_ALIGNMENT	16
> +
> +static void *zalloc(void *x, unsigned items, unsigned size)
> +{
> +	void *p;
> +
> +	size *= items;
> +	size = (size + ZALLOC_ALIGNMENT - 1) & ~(ZALLOC_ALIGNMENT - 1);
> +
> +	p = qemu_malloc(size);
> +
> +	return (p);
> +}
> +
> +static void zfree(void *x, void *addr, unsigned nb)
> +{
> +	qemu_free(addr);
> +}
> +
> +
> +#define HEAD_CRC	2
> +#define EXTRA_FIELD	4
> +#define ORIG_NAME	8
> +#define COMMENT		0x10
> +#define RESERVED	0xe0
> +
> +#define DEFLATED	8
> +
> +static ssize_t gunzip(void *dst, size_t dstlen, uint8_t *src,
> +                      size_t srclen)
> +{
> +	z_stream s;
> +	ssize_t dstbytes;
> +	int r, i, flags;
> +
> +	/* skip header */
> +	i = 10;
> +	flags = src[3];
> +	if (src[2] != DEFLATED || (flags & RESERVED) != 0) {
> +		puts ("Error: Bad gzipped data\n");
> +		return -1;
> +	}
> +	if ((flags & EXTRA_FIELD) != 0)
> +		i = 12 + src[10] + (src[11] << 8);
> +	if ((flags & ORIG_NAME) != 0)
> +		while (src[i++] != 0)
> +			;
> +	if ((flags & COMMENT) != 0)
> +		while (src[i++] != 0)
> +			;
> +	if ((flags & HEAD_CRC) != 0)
> +		i += 2;
> +	if (i >= srclen) {
> +		puts ("Error: gunzip out of data in header\n");
> +		return -1;
> +	}
> +
> +	s.zalloc = zalloc;
> +	s.zfree = (free_func)zfree;
> +
> +	r = inflateInit2(&s, -MAX_WBITS);
> +	if (r != Z_OK) {
> +		printf ("Error: inflateInit2() returned %d\n", r);
> +		return (-1);
> +	}
> +	s.next_in = src + i;
> +	s.avail_in = srclen - i;
> +	s.next_out = dst;
> +	s.avail_out = dstlen;
> +	r = inflate(&s, Z_FINISH);
> +	if (r != Z_OK && r != Z_STREAM_END) {
> +		printf ("Error: inflate() returned %d\n", r);
> +		return -1;
> +	}
> +	dstbytes = s.next_out - (unsigned char *) dst;
> +	inflateEnd(&s);
> +
> +	return dstbytes;
> +}
> +
>  /* Load a U-Boot image.  */
>  int load_uboot(const char *filename, target_ulong *ep, int *is_linux)
>  {
> -
>      int fd;
>      int size;
>      uboot_image_header_t h;
> @@ -374,9 +459,9 @@ int load_uboot(const char *filename, target_ulong *ep, int *is_linux)
>          goto fail;
>      }
>  
> -    /* TODO: Implement compressed images.  */
> -    if (hdr->ih_comp != IH_COMP_NONE) {
> -        fprintf(stderr, "Unable to load compressed u-boot images\n");
> +    /* TODO bzip2 support */
> +    if (hdr->ih_comp == IH_COMP_BZIP2) {
> +        fprintf(stderr, "Unable to load bzip2 compressed u-boot images\n");
>          goto fail;
>      }
why do you remove the non compress mode?

BTW Please note 2 thinks
First this format of uImage is deprecated
we have switch to a fdt uImage format

Secondly we aslo support now LZMA compression

Best Regards,
J.

Re: [Qemu-devel] [PATCH 2/4] uImage: implement gzip support

[François Revol]

> > -    /* TODO: Implement compressed images.  */
> > -    if (hdr->ih_comp != IH_COMP_NONE) {
> > -        fprintf(stderr, "Unable to load compressed u-boot images\
> > n");
> > +    /* TODO bzip2 support */
> > +    if (hdr->ih_comp == IH_COMP_BZIP2) {
> > +        fprintf(stderr, "Unable to load bzip2 compressed u-boot
> > images\n");
> >          goto fail;
> >      }
> why do you remove the non compress mode?

He didn't remove it, he changed the check to only reject BZIP2, not
uncompressed (which is wrong though as it'd accept any other compession
including unknown).

> BTW Please note 2 thinks
> First this format of uImage is deprecated
> we have switch to a fdt uImage format

fdt ? as in flat device tree ?
Is that documented somewhere ?
I'll need to add U-Boot support to Haiku soon...

François.

Re: [Qemu-devel] [PATCH 2/4] uImage: implement gzip support

[Hollis Blanchard]

On Mon, 2008-11-17 at 23:07 +0100, Jean-Christophe PLAGNIOL-VILLARD
wrote:
> On 15:18 Mon 17 Nov     , Hollis Blanchard wrote:
> >  /* Load a U-Boot image.  */
> >  int load_uboot(const char *filename, target_ulong *ep, int *is_linux)
> >  {
> > -
> >      int fd;
> >      int size;
> >      uboot_image_header_t h;
> > @@ -374,9 +459,9 @@ int load_uboot(const char *filename, target_ulong *ep, int *is_linux)
> >          goto fail;
> >      }
> >  
> > -    /* TODO: Implement compressed images.  */
> > -    if (hdr->ih_comp != IH_COMP_NONE) {
> > -        fprintf(stderr, "Unable to load compressed u-boot images\n");
> > +    /* TODO bzip2 support */
> > +    if (hdr->ih_comp == IH_COMP_BZIP2) {
> > +        fprintf(stderr, "Unable to load bzip2 compressed u-boot images\n");
> >          goto fail;
> >      }
> why do you remove the non compress mode?

I do not.

> BTW Please note 2 thinks
> First this format of uImage is deprecated
> we have switch to a fdt uImage format

I see a new "fit" format; I assume this is what you're referring to. Who
is "we"? It looks like Linux does not yet use this format, neither for
PowerPC (my personal interest) nor ARM.

> Secondly we aslo support now LZMA compression

That's good to know, but PowerPC Linux has traditionally and continues
to use gzip uImages, so that's what this patch implements.

-- 
Hollis Blanchard
IBM Linux Technology Center

Re: [Qemu-devel] [PATCH 2/4] uImage: implement gzip support

[Jean-Christophe PLAGNIOL-VILLARD]

On 16:35 Mon 17 Nov     , Hollis Blanchard wrote:
> On Mon, 2008-11-17 at 23:07 +0100, Jean-Christophe PLAGNIOL-VILLARD
> wrote:
> > On 15:18 Mon 17 Nov     , Hollis Blanchard wrote:
> > >  /* Load a U-Boot image.  */
> > >  int load_uboot(const char *filename, target_ulong *ep, int *is_linux)
> > >  {
> > > -
> > >      int fd;
> > >      int size;
> > >      uboot_image_header_t h;
> > > @@ -374,9 +459,9 @@ int load_uboot(const char *filename, target_ulong *ep, int *is_linux)
> > >          goto fail;
> > >      }
> > >  
> > > -    /* TODO: Implement compressed images.  */
> > > -    if (hdr->ih_comp != IH_COMP_NONE) {
> > > -        fprintf(stderr, "Unable to load compressed u-boot images\n");
> > > +    /* TODO bzip2 support */
> > > +    if (hdr->ih_comp == IH_COMP_BZIP2) {
> > > +        fprintf(stderr, "Unable to load bzip2 compressed u-boot images\n");
> > >          goto fail;
> > >      }
> > why do you remove the non compress mode?
> 
> I do not.
Sorry I miss
> 
> > BTW Please note 2 thinks
> > First this format of uImage is deprecated
> > we have switch to a fdt uImage format
> 
> I see a new "fit" format; I assume this is what you're referring to. Who
> is "we"?
U-Boot dev & Maintainer
> It looks like Linux does not yet use this format, neither for
> PowerPC (my personal interest) nor ARM.

No the PowerPC has switch to it
And some ARM board will do as AT91

> 
> > Secondly we aslo support now LZMA compression
> 
> That's good to know, but PowerPC Linux has traditionally and continues
> to use gzip uImages, so that's what this patch implements.

Some boards as qemu_mips and some other ARM board start to use it

Best Regards,
J.

Re: [Qemu-devel] [PATCH 2/4] uImage: implement gzip support

[Jean-Christophe PLAGNIOL-VILLARD]

On 23:30 Mon 17 Nov     , François Revol wrote:
> > > -    /* TODO: Implement compressed images.  */
> > > -    if (hdr->ih_comp != IH_COMP_NONE) {
> > > -        fprintf(stderr, "Unable to load compressed u-boot images\
> > > n");
> > > +    /* TODO bzip2 support */
> > > +    if (hdr->ih_comp == IH_COMP_BZIP2) {
> > > +        fprintf(stderr, "Unable to load bzip2 compressed u-boot
> > > images\n");
> > >          goto fail;
> > >      }
> > why do you remove the non compress mode?
> 
> He didn't remove it, he changed the check to only reject BZIP2, not
> uncompressed (which is wrong though as it'd accept any other compession
> including unknown).

I've missed it but as we also support LZMA now you must add the check too
> 
> > BTW Please note 2 thinks
> > First this format of uImage is deprecated
> > we have switch to a fdt uImage format
> 
> fdt ? as in flat device tree ?
> Is that documented somewhere ?
> I'll need to add U-Boot support to Haiku soon...
in the source code doc/uImage.FIT/

I'm preparing the DHT-Walnut

I've add support and doc how to use it for the qemu_mips
and the ARM soon

Best Regards,
J.

Re: [Qemu-devel] [PATCH 2/4] uImage: implement gzip support

[François Revol]

> > > BTW Please note 2 thinks
> > > First this format of uImage is deprecated
> > > we have switch to a fdt uImage format
> >
> > fdt ? as in flat device tree ?
> > Is that documented somewhere ?
> > I'll need to add U-Boot support to Haiku soon...
> in the source code doc/uImage.FIT/

Ah yes, thanks!
I'll probably support the first format first though, as this one seems
a bit more complex. I'll need the old one anyway for hardware with old
U-Boot.

François.

Re: [Qemu-devel] [PATCH 2/4] uImage: implement gzip support

[Hollis Blanchard]

On Mon, 2008-11-17 at 23:39 +0100, Jean-Christophe PLAGNIOL-VILLARD
wrote:
> > It looks like Linux does not yet use this format, neither for
> > PowerPC (my personal interest) nor ARM.
> 
> No the PowerPC has switch to it
> And some ARM board will do as AT91

AFAICS the new format is only used with mkimage -f, correct? Since the
-f switch is *not* used in the Linux makefiles (neither PowerPC nor ARM,
in Linus's tree right now), I really don't understand what you mean.

> > > Secondly we aslo support now LZMA compression
> > 
> > That's good to know, but PowerPC Linux has traditionally and
> continues
> > to use gzip uImages, so that's what this patch implements.
> 
> Some boards as qemu_mips and some other ARM board start to use it

Again, that's good to know, but let's not confuse the issue. Qemu today
doesn't support gzipped uImages, Linux uses gzipped uImages, and my
patch adds support for that.

-- 
Hollis Blanchard
IBM Linux Technology Center

Re: [Qemu-devel] [PATCH 2/4] uImage: implement gzip support

[Anthony Liguori]

Hollis Blanchard wrote:
> Based on gzip uImage loading code from u-boot.
>
> Signed-off-by: Jerone Young <jyoung5@us.ibm.com>
> Signed-off-by: Hollis Blanchard <hollisb@us.ibm.com>
>   

So you pulled some code out of gunzip?  Presumably that's GPL'd and 
carries some sort of attribution?  This file is licensed under the X11 
license so this sort of change would be important.

Also note that there are multiple implementations of unzip in QEMU right 
now so you could refactor one of those into common code.

Regards,

Anthony Liguori

Re: [Qemu-devel] [PATCH 1/4] uImage: free file data after copying into guest memory

[Anthony Liguori]

Hollis Blanchard wrote:
> Signed-off-by: Hollis Blanchard <hollisb@us.ibm.com>
> ---
>  loader.c |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
>
> diff --git a/loader.c b/loader.c
> index 8722909..7f4e69c 100644
> --- a/loader.c
> +++ b/loader.c
> @@ -399,6 +399,7 @@ int load_uboot(const char *filename, target_ulong *ep, int *is_linux)
>      }
>  
>      cpu_physical_memory_write_rom(hdr->ih_load, data, hdr->ih_size);
> +    qemu_free(data);
>   

I notice that fd is also leaked in this path.  I think a better fix is 
to eliminate the separate error path and introduce a return variable.  
The success path should just set that to hdr->ih_size.

Regards,

Anthony Liguori

>  
>      return hdr->ih_size;
>  
>   

Re: [Qemu-devel] [PATCH 2/4] uImage: implement gzip support

[Hollis Blanchard]

On Tue, 2008-11-18 at 16:04 -0600, Anthony Liguori wrote:
> Hollis Blanchard wrote:
> > Based on gzip uImage loading code from u-boot.
> >
> > Signed-off-by: Jerone Young <jyoung5@us.ibm.com>
> > Signed-off-by: Hollis Blanchard <hollisb@us.ibm.com>
> >   
> 
> So you pulled some code out of gunzip?  Presumably that's GPL'd and 
> carries some sort of attribution?  This file is licensed under the X11 
> license so this sort of change would be important.

Actually from u-boot, which is also GPL. Apparently Jerone didn't notice
the issue, and I'm sorry that I didn't double-check it myself.

> Also note that there are multiple implementations of unzip in QEMU right 
> now so you could refactor one of those into common code.

Are there? grep didn't find them; can you point me at some?

-- 
Hollis Blanchard
IBM Linux Technology Center

Re: [Qemu-devel] [PATCH 2/4] uImage: implement gzip support

[Anthony Liguori]

Hollis Blanchard wrote:
>> Also note that there are multiple implementations of unzip in QEMU right 
>> now so you could refactor one of those into common code.
>>     
>
> Are there? grep didn't find them; can you point me at some?
>   

$ grep inflateInit *.c
block-cloop.c:    if(inflateInit(&s->zstream) != Z_OK)
block-dmg.c:    if(inflateInit(&s->zstream) != Z_OK)
block-qcow2.c:    ret = inflateInit2(strm, -12);
block-qcow.c:    ret = inflateInit2(strm, -12);
vl.c:    ret = inflateInit(&s->zstream);

The closest to what you're looking for is probably decompress_buffer() 
in block-qcow2.c

Regards,

Anthony Liguori