[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [Qemu-ppc] [RFC PATCH v7 1/4] use image_file_reset to r
From: |
Alexander Graf |
Subject: |
Re: [Qemu-devel] [Qemu-ppc] [RFC PATCH v7 1/4] use image_file_reset to reload initrd |
Date: |
Sun, 2 Dec 2012 12:20:25 +0100 |
Missing patch description
On 29.11.2012, at 06:26, Olivia Yin wrote:
> Signed-off-by: Olivia Yin <address@hidden>
> ---
> hw/loader.c | 24 ++++++++++++++++++++++++
> hw/loader.h | 6 ++++++
> 2 files changed, 30 insertions(+), 0 deletions(-)
>
> diff --git a/hw/loader.c b/hw/loader.c
> index ba01ca6..f62aa7c 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->addr, (uint8_t *)content, size);
> + g_free(content);
> + }
If I compare this function to rom_add_file(), it seems like there's a lot of
logic missing.
> +}
> +
> /* read()-like version */
> ssize_t read_targphys(const char *name,
> int fd, hwaddr dst_addr, size_t nbytes)
> @@ -113,6 +131,12 @@ int load_image_targphys(const char *filename,
Up here is:
> int size;
>
> size = get_image_size(filename);
> if (size > max_sz) {
> return -1;
which could be easily replaced by the glib helper function. It passes size and
a proper return code already.
> }
> 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);
You now have 2 competing reset handlers: The rom based one and the ImageFile
based one.
Why bother with the rom based one? Traditionally, having the rom caller buys
you 2 things:
1) Reset restoration of the rom data
This one is obsolete with the new dynamic loader.
2) Listing of the rom region in "info roms"
You can replace the Rom struct in loader.c with a new struct that is more
clever:
struct RomImage {
union {
ImageFile *image;
} u;
QTAILQ_ENTRY(RomImage) next;
}
which means that "info roms" can loop through these RomImage types and actually
show things like
ELF image /foo/bar.uImage
ELF .text section hwaddr=0x... size=0x...
ELF .data section hwaddr=0x... size=0x...
Raw image /foo/initrd hwaddr=0x... size=0x...
Alex
> }
> return size;
> }
> diff --git a/hw/loader.h b/hw/loader.h
> index 26480ad..9e76ebd 100644
> --- a/hw/loader.h
> +++ b/hw/loader.h
> @@ -1,6 +1,12 @@
> #ifndef LOADER_H
> #define LOADER_H
>
> +typedef struct ImageFile ImageFile;
> +struct ImageFile {
> + char *name;
> + hwaddr addr;
> +};
> +
> /* loader.c */
> int get_image_size(const char *filename);
> int load_image(const char *filename, uint8_t *addr); /* deprecated */
> --
> 1.7.1
>
>
>
- Re: [Qemu-devel] [Qemu-ppc] [RFC PATCH v7 1/4] use image_file_reset to reload initrd,
Alexander Graf <=