Re: [Qemu-devel] [Qemu-ppc] [RFC PATCH v7 2/4] use uimage_reset to reload uimage

[Alexander Graf]

On 29.11.2012, at 06:26, Olivia Yin wrote:

> Signed-off-by: Olivia Yin <address@hidden>
> ---
> hw/loader.c |   57 +++++++++++++++++++++++++++++++++++++++++++++------------
> hw/loader.h |    3 +++
> 2 files changed, 48 insertions(+), 12 deletions(-)
> 
> diff --git a/hw/loader.c b/hw/loader.c
> index f62aa7c..151ef20 100644
> --- a/hw/loader.c
> +++ b/hw/loader.c
> @@ -457,15 +457,15 @@ 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, hwaddr *ep, 
> +                                  uint8_t **data, hwaddr *loadaddr,
> +                                  int *is_linux, int reset)
> {
>     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);
> @@ -507,9 +507,9 @@ int load_uimage(const char *filename, hwaddr *ep,
>     }
> 
>     *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;
>     }
> @@ -519,11 +519,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");
> @@ -532,7 +532,11 @@ 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 (!reset) {
> +        rom_add_blob_fixed(filename, *data, hdr->ih_size, hdr->ih_load);
> +    } else {
> +        cpu_physical_memory_write(hdr->ih_load, *data, hdr->ih_size);
> +    }
> 
>     if (loadaddr)
>         *loadaddr = hdr->ih_load;
> @@ -540,12 +544,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;
> +
> +    uimage_physical_loader(image->name, image->ep, &data, &image->addr, 
> +                           image->is_linux, 1);
> +    g_free(data);
> +}
> +
> +/* Load a U-Boot image.  */
> +int load_uimage(const char *filename, hwaddr *ep,
> +                hwaddr *loadaddr, int *is_linux)
> +{
> +    int size;
> +    ImageFile *image;
> +    uint8_t *data = NULL;
> +
> +    size = uimage_physical_loader(filename, ep, &data, loadaddr, is_linux, 
> 0);
> +    if (size > 0) {
> +        g_free(data);
> +        image = g_malloc0(sizeof(*image));
> +        image->name = g_strdup(filename);
> +        image->addr = *loadaddr;
> +        image->ep = ep;

This variable could reside on the stack and be gone by the time you use it.

> +        image->is_linux = is_linux;

Same as above.

> +        qemu_register_reset(uimage_reset, image);
> +    }
> +    return size;
> +}
> +
> /*
>  * Functions for reboot-persistent memory regions.
>  *  - used for vga bios and option roms.
> diff --git a/hw/loader.h b/hw/loader.h
> index 9e76ebd..97b3aab 100644
> --- a/hw/loader.h
> +++ b/hw/loader.h
> @@ -5,6 +5,9 @@ typedef struct ImageFile ImageFile;
> struct ImageFile {
>     char *name;
>     hwaddr addr;
> +    /* uimage */

Why not create a new struct for uImage style images? You could add that to the 
union I was laying out in my previous mail and have all fields that are special 
to uImage be in there. If there is no special field, sharing it with the raw 
image struct works too of course.


Alex

> +    hwaddr *ep;
> +    int *is_linux; 
> };
> 
> /* loader.c */
> -- 
> 1.7.1
> 
> 
>