Re: [Qemu-devel] [RFC PATCH v4 2/3] use uimage_reset to reload uimage

[Stuart Yoder]

On Wed, Nov 14, 2012 at 2:28 PM, Olivia Yin <address@hidden> wrote:
> Signed-off-by: Olivia Yin <address@hidden>
> ---
>  hw/loader.c |   64 ++++++++++++++++++++++++++++++++++++++++++----------------
>  1 files changed, 46 insertions(+), 18 deletions(-)
>
> diff --git a/hw/loader.c b/hw/loader.c
> index a8a0a09..1a909d0 100644
> --- a/hw/loader.c
> +++ b/hw/loader.c
> @@ -55,6 +55,8 @@
>  #include <zlib.h>
>
>  static int roms_loaded;
> +static int kernel_loaded;
> +static int *is_linux;

Why do we need these 2 new variables?

>  /* return the size or -1 if error */
>  int get_image_size(const char *filename)
> @@ -472,15 +474,14 @@ static ssize_t gunzip(void *dst, size_t dstlen, uint8_t 
> *src,
>      return dstbytes;
>  }
>
> -/* Load a U-Boot image.  */
> -int load_uimage(const char *filename, hwaddr *ep,
> -                hwaddr *loadaddr, int *is_linux)
> +/* write uimage into memory */
> +static int uimage_physical_loader(const char *filename, uint8_t **data,
> +                                  hwaddr *loadaddr, int *is_linux)
>  {
>      int fd;
>      int size;
>      uboot_image_header_t h;
>      uboot_image_header_t *hdr = &h;
> -    uint8_t *data = NULL;
>      int ret = -1;
>
>      fd = open(filename, O_RDONLY | O_BINARY);
> @@ -521,10 +522,9 @@ int load_uimage(const char *filename, hwaddr *ep,
>              *is_linux = 0;
>      }
>
> -    *ep = hdr->ih_ep;
> -    data = g_malloc(hdr->ih_size);
> +    *data = g_malloc(hdr->ih_size);
>
> -    if (read(fd, data, hdr->ih_size) != hdr->ih_size) {
> +    if (read(fd, *data, hdr->ih_size) != hdr->ih_size) {
>          fprintf(stderr, "Error reading file\n");
>          goto out;
>      }
> @@ -534,11 +534,11 @@ int load_uimage(const char *filename, hwaddr *ep,
>          size_t max_bytes;
>          ssize_t bytes;
>
> -        compressed_data = data;
> +        compressed_data = *data;
>          max_bytes = UBOOT_MAX_GUNZIP_BYTES;
> -        data = g_malloc(max_bytes);
> +        *data = g_malloc(max_bytes);
>
> -        bytes = gunzip(data, max_bytes, compressed_data, hdr->ih_size);
> +        bytes = gunzip(*data, max_bytes, compressed_data, hdr->ih_size);
>          g_free(compressed_data);
>          if (bytes < 0) {
>              fprintf(stderr, "Unable to decompress gzipped image!\n");
> @@ -547,7 +547,9 @@ int load_uimage(const char *filename, hwaddr *ep,
>          hdr->ih_size = bytes;
>      }
>
> -    rom_add_blob_fixed(filename, data, hdr->ih_size, hdr->ih_load);
> +    if (!kernel_loaded) {
> +        rom_add_blob_fixed(filename, *data, hdr->ih_size, hdr->ih_load);
> +    }

Why do we need to keep the rom_add_blob_fixed() call?  I thought the
point of this was to remove adding roms.

>      if (loadaddr)
>          *loadaddr = hdr->ih_load;
> @@ -555,12 +557,41 @@ int load_uimage(const char *filename, hwaddr *ep,
>      ret = hdr->ih_size;
>
>  out:
> -    if (data)
> -        g_free(data);
>      close(fd);
>      return ret;
>  }
>
> +static void uimage_reset(void *opaque)
> +{
> +    ImageFile *image = opaque;
> +    uint8_t *data = NULL;
> +    int size;
> +
> +    size = uimage_physical_loader(image->name, &data, &image->addr, 
> is_linux);

Not sure why we need is_linux here.  I think you should just set that
parameter to NULL.
Nothing will look at is_linux.

Stuart