Re: [Qemu-devel] [PATCH v2] register reset handler to write image into memory

["Andreas Färber" <afaerber@suse.de>]

Hi,

Am 23.08.2012 11:45, schrieb Yin Olivia-R63875:
> Dear All,
> 
> I can't find MAINTAINER of hw/loader.c.
> Who can help review and apply this patch?

This patch is not a small bugfix so it won't be applied during the v1.2
Hard Freeze. You based it onto ppc-next so the obvious answer is, Alex
needs to review it, whom you forgot to cc.

This patch does not answer the question why you try to avoid the ROM
blobs and what ROM blobs are still being used for after your patch. I
don't think it makes much sense to work around them for your use cases
and to leave them behind - if there's something fundamentally wrong with
them they should be ripped out completely or fixed. But maybe I'm
misunderstanding in the absence of explanations?

Regards,
Andreas

> 
> Best Regards,
> Olivia Yin
> 
>> -----Original Message-----
>> From: Yin Olivia-R63875
>> Sent: Friday, August 17, 2012 5:08 PM
>> To: qemu-ppc@nongnu.org; qemu-devel@nongnu.org
>> Cc: Yin Olivia-R63875
>> Subject: [PATCH v2] register reset handler to write image into memory
>>
>> Instead of add rom blobs, this patch just write them directly to memory.
>>
>> This patch registers reset handler uimage_reset() and image_file_reset()
>> which load images into RAM during initial bootup and VM reset.
>>
>> v2: use g_file_get_content() to load load image file.
>>
>> Signed-off-by: Olivia Yin <hong-hua.yin@freescale.com>
>> ---
>> This patch is based on branch 'ppc-next' of Alex's upstream QEMU repo:
>> http://repo.or.cz/r/qemu/agraf.git
>>
>>  hw/loader.c |   88 ++++++++++++++++++++++++++++++++++++++++++++---------
>> ------
>>  1 files changed, 66 insertions(+), 22 deletions(-)
>>
>> diff --git a/hw/loader.c b/hw/loader.c
>> index 33acc2f..0be8bf7 100644
>> --- a/hw/loader.c
>> +++ b/hw/loader.c
>> @@ -56,6 +56,12 @@
>>
>>  static int roms_loaded;
>>
>> +typedef struct ImageFile ImageFile;
>> +struct ImageFile {
>> +    char *name;
>> +    target_phys_addr_t addr;
>> +};
>> +
>>  /* return the size or -1 if error */
>>  int get_image_size(const char *filename)  { @@ -86,6 +92,24 @@ int
>> load_image(const char *filename, uint8_t *addr)
>>      return size;
>>  }
>>
>> +static void image_file_reset(void *opaque) {
>> +    ImageFile *image = opaque;
>> +    GError *err = NULL;
>> +    gboolean res;
>> +    gchar *content;
>> +    gsize size;
>> +
>> +    res = g_file_get_contents(image->name, &content, &size, &err);
>> +    if (res == FALSE) {
>> +       error_report("failed to read image file: %s\n", image->name);
>> +       g_error_free(err);
>> +    } else {
>> +       cpu_physical_memory_rw(image->addr, (uint8_t *)content, size, 1);
>> +       g_free(content);
>> +    }
>> +}
>> +
>>  /* read()-like version */
>>  ssize_t read_targphys(const char *name,
>>                        int fd, target_phys_addr_t dst_addr, size_t nbytes)
>> @@ -112,7 +136,12 @@ int load_image_targphys(const char *filename,
>>          return -1;
>>      }
>>      if (size > 0) {
>> -        rom_add_file_fixed(filename, addr, -1);
>> +        ImageFile *image;
>> +        image = g_malloc0(sizeof(*image));
>> +        image->name = g_strdup(filename);
>> +        image->addr = addr;
>> +
>> +        qemu_register_reset(image_file_reset, image);
>>      }
>>      return size;
>>  }
>> @@ -433,15 +462,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, target_phys_addr_t *ep,
>> -                target_phys_addr_t *loadaddr, int *is_linux)
>> +/* write uimage into memory */
>> +static int uimage_physical_loader(const char *filename, uint8_t **data,
>> +                                  target_phys_addr_t *loadaddr)
>>  {
>>      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); @@ -474,18 +502,9 @@ int
>> load_uimage(const char *filename, target_phys_addr_t *ep,
>>          goto out;
>>      }
>>
>> -    /* TODO: Check CPU type.  */
>> -    if (is_linux) {
>> -        if (hdr->ih_os == IH_OS_LINUX)
>> -            *is_linux = 1;
>> -        else
>> -            *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;
>>      }
>> @@ -495,11 +514,11 @@ int load_uimage(const char *filename,
>> target_phys_addr_t *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"); @@
>> -508,7 +527,6 @@ int load_uimage(const char *filename, target_phys_addr_t
>> *ep,
>>          hdr->ih_size = bytes;
>>      }
>>
>> -    rom_add_blob_fixed(filename, data, hdr->ih_size, hdr->ih_load);
>>
>>      if (loadaddr)
>>          *loadaddr = hdr->ih_load;
>> @@ -516,12 +534,38 @@ int load_uimage(const char *filename,
>> target_phys_addr_t *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);
>> +    cpu_physical_memory_rw(image->addr, data, size, 1);
>> +    g_free(data);
>> +}
>> +
>> +/* Load a U-Boot image.  */
>> +int load_uimage(const char *filename, target_phys_addr_t *ep,
>> +                target_phys_addr_t *loadaddr, int *is_linux) {
>> +    int size;
>> +    ImageFile *image;
>> +    uint8_t *data = NULL;
>> +
>> +    size= uimage_physical_loader(filename, &data, loadaddr);
>> +    g_free(data);
>> +    image = g_malloc0(sizeof(*image));
>> +    image->name = g_strdup(filename);
>> +    image->addr = *loadaddr;
>> +    qemu_register_reset(uimage_reset, image);
>> +    return size;
>> +}
>> +
>>  /*
>>   * Functions for reboot-persistent memory regions.
>>   *  - used for vga bios and option roms.
>> --
>> 1.7.1
> 
> 
> 


-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg