diff mbox

[v10,1/4] use image_file_reset to reload initrd

Message ID 1358990783-23048-2-git-send-email-hong-hua.yin@freescale.com
State New
Headers show

Commit Message

Olivia Yin Jan. 24, 2013, 1:26 a.m. UTC 
Signed-off-by: Olivia Yin <hong-hua.yin@freescale.com>
---
 hw/loader.c |   25 +++++++++++++++++++++++++
 hw/loader.h |    6 ++++++
 2 files changed, 31 insertions(+), 0 deletions(-)

Comments

Alexander Graf Jan. 24, 2013, 4:57 p.m. UTC | #1 
On 24.01.2013, at 02:26, Olivia Yin wrote:

> Signed-off-by: Olivia Yin <hong-hua.yin@freescale.com>
> ---
> hw/loader.c |   25 +++++++++++++++++++++++++
> hw/loader.h |    6 ++++++
> 2 files changed, 31 insertions(+), 0 deletions(-)
> 
> diff --git a/hw/loader.c b/hw/loader.c
> index ba01ca6..4fa9965 100644
> --- a/hw/loader.c
> +++ b/hw/loader.c
> @@ -86,6 +86,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_write(image->loadaddr, (uint8_t *)content, size);
> +       g_free(content);
> +    }
> +}
> +
> /* read()-like version */
> ssize_t read_targphys(const char *name,
>                       int fd, hwaddr dst_addr, size_t nbytes)
> @@ -113,6 +131,13 @@ int load_image_targphys(const char *filename,

What happens before here? Why doesn't the normal loading process use the above code? Also, reset gets invoked even on initial machine start, so this function only needs to

  * read the size of the file
  * register the reset handler
  * return the size of the file

>     }
>     if (size > 0) {
>         rom_add_file_fixed(filename, addr, -1);

Why keep this? According to 0/4 you want to change this to "info images". Get rid of any rom infrastructure in here then!


Also, patch 4/4 never made it to the mailing list.

Alex

> +
> +        ImageFile *image;
> +        image = g_malloc0(sizeof(*image));
> +        image->name = g_strdup(filename);
> +        image->loadaddr = addr;
> +        image->size = size;
> +        qemu_register_reset(image_file_reset, image);
>     }
>     return size;
> }
> diff --git a/hw/loader.h b/hw/loader.h
> index 26480ad..29f52ab 100644
> --- a/hw/loader.h
> +++ b/hw/loader.h
> @@ -1,6 +1,12 @@
> #ifndef LOADER_H
> #define LOADER_H
> 
> +typedef struct ImageFile {
> +    char *name;
> +    hwaddr loadaddr;
> +    ssize_t size;
> +} ImageFile;
> +
> /* loader.c */
> int get_image_size(const char *filename);
> int load_image(const char *filename, uint8_t *addr); /* deprecated */
> -- 
> 1.7.1
> 
> 
>
Yin Olivia-R63875 Jan. 25, 2013, 2:51 a.m. UTC | #2 
> -----Original Message-----
> From: Alexander Graf [mailto:agraf@suse.de]
> Sent: Friday, January 25, 2013 12:58 AM
> To: Yin Olivia-R63875
> Cc: qemu-devel@nongnu.org; qemu-ppc@nongnu.org
> Subject: Re: [Qemu-ppc] [PATCH v10 1/4] use image_file_reset to reload
> initrd
> 
> 
> On 24.01.2013, at 02:26, Olivia Yin wrote:
> 
> > Signed-off-by: Olivia Yin <hong-hua.yin@freescale.com>
> > ---
> > hw/loader.c |   25 +++++++++++++++++++++++++
> > hw/loader.h |    6 ++++++
> > 2 files changed, 31 insertions(+), 0 deletions(-)
> >
> > diff --git a/hw/loader.c b/hw/loader.c index ba01ca6..4fa9965 100644
> > --- a/hw/loader.c
> > +++ b/hw/loader.c
> > @@ -86,6 +86,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_write(image->loadaddr, (uint8_t *)content,
> size);
> > +       g_free(content);
> > +    }
> > +}
> > +
> > /* read()-like version */
> > ssize_t read_targphys(const char *name,
> >                       int fd, hwaddr dst_addr, size_t nbytes) @@
> > -113,6 +131,13 @@ int load_image_targphys(const char *filename,
> 
> What happens before here? Why doesn't the normal loading process use the
> above code? 
load_image_targphys() is called in board initialization. For example, ppce500_init()
       initrd_size = load_image_targphys(params->initrd_filename, initrd_base,
                                          ram_size - initrd_base);

> Also, reset gets invoked even on initial machine start, so this
> function only needs to
> 
>   * read the size of the file
>   * register the reset handler
>   * return the size of the file

Sure, this function complete the above steps.
int load_image_targphys(const char *filename,
                        hwaddr addr, uint64_t max_sz)
{
    int size;

    size = get_image_size(filename);
    if (size > max_sz) {
        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->loadaddr = addr;
        image->size = size;
        qemu_register_reset(image_file_reset, image);
    }
    return size;
}

> >     }
> >     if (size > 0) {
> >         rom_add_file_fixed(filename, addr, -1);
> 
> Why keep this? According to 0/4 you want to change this to "info images".
> Get rid of any rom infrastructure in here then!

I kept rom infrastructure because some other platforms need the KERNEL/INITRD rom information called by rom_ptr().
hw/sun4m.c:             ptr = rom_ptr(KERNEL_LOAD_ADDR + i);
hw/s390-virtio.c:       stq_p(rom_ptr(INITRD_PARM_START), initrd_offset);
hw/s390-virtio.c:       stq_p(rom_ptr(INITRD_PARM_SIZE), initrd_size);
hw/s390-virtio.c:    	if (rom_ptr(KERN_PARM_AREA)) {
hw/s390-virtio.c:       memcpy(rom_ptr(KERN_PARM_AREA), kernel_cmdline,
hw/sun4u.c:             ptr = rom_ptr(*kernel_addr + i);

I created a new image infrastructure for reloadable images and display the information with new API do_info_images() in Patch 4/4.
> 
> Also, patch 4/4 never made it to the mailing list.
> 
> Alex
> 
> > +
> > +        ImageFile *image;
> > +        image = g_malloc0(sizeof(*image));
> > +        image->name = g_strdup(filename);
> > +        image->loadaddr = addr;
> > +        image->size = size;
> > +        qemu_register_reset(image_file_reset, image);
> >     }
> >     return size;
> > }
> > diff --git a/hw/loader.h b/hw/loader.h index 26480ad..29f52ab 100644
> > --- a/hw/loader.h
> > +++ b/hw/loader.h
> > @@ -1,6 +1,12 @@
> > #ifndef LOADER_H
> > #define LOADER_H
> >
> > +typedef struct ImageFile {
> > +    char *name;
> > +    hwaddr loadaddr;
> > +    ssize_t size;
> > +} ImageFile;
> > +
> > /* loader.c */
> > int get_image_size(const char *filename); int load_image(const char
> > *filename, uint8_t *addr); /* deprecated */
> > --
> > 1.7.1
> >
> >
> >
>
diff mbox

Patch

diff --git a/hw/loader.c b/hw/loader.c
index ba01ca6..4fa9965 100644
--- a/hw/loader.c
+++ b/hw/loader.c
@@ -86,6 +86,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_write(image->loadaddr, (uint8_t *)content, size);
+       g_free(content);
+    }
+}
+
 /* read()-like version */
 ssize_t read_targphys(const char *name,
                       int fd, hwaddr dst_addr, size_t nbytes)
@@ -113,6 +131,13 @@  int load_image_targphys(const char *filename,
     }
     if (size > 0) {
         rom_add_file_fixed(filename, addr, -1);
+
+        ImageFile *image;
+        image = g_malloc0(sizeof(*image));
+        image->name = g_strdup(filename);
+        image->loadaddr = addr;
+        image->size = size;
+        qemu_register_reset(image_file_reset, image);
     }
     return size;
 }
diff --git a/hw/loader.h b/hw/loader.h
index 26480ad..29f52ab 100644
--- a/hw/loader.h
+++ b/hw/loader.h
@@ -1,6 +1,12 @@ 
 #ifndef LOADER_H
 #define LOADER_H
 
+typedef struct ImageFile {
+    char *name;
+    hwaddr loadaddr;
+    ssize_t size;
+} ImageFile;
+
 /* loader.c */
 int get_image_size(const char *filename);
 int load_image(const char *filename, uint8_t *addr); /* deprecated */