* [Qemu-devel] [PATCH 2/2] register reset handlers to write images into memory
2012-08-14 7:49 [Qemu-devel] [PATCH 1/2] extract file_load() function from rom_add_file() for reusing Olivia Yin
@ 2012-08-14 7:49 ` Olivia Yin
2012-08-16 11:13 ` [Qemu-devel] [PATCH 1/2] extract file_load() function from rom_add_file() for reusing Yin Olivia-R63875
2012-08-16 11:36 ` Peter Maydell
2 siblings, 0 replies; 7+ messages in thread
From: Olivia Yin @ 2012-08-14 7:49 UTC (permalink / raw)
To: qemu-ppc, qemu-devel; +Cc: Olivia Yin
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.
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 | 81 +++++++++++++++++++++++++++++++++++++++++++----------------
1 files changed, 59 insertions(+), 22 deletions(-)
diff --git a/hw/loader.c b/hw/loader.c
index f2099b6..3c97724 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)
{
@@ -115,6 +121,17 @@ err:
g_free(*data);
return -1;
}
+
+static void image_file_reset(void *opaque)
+{
+ ImageFile *image = opaque;
+ int size;
+ uint8_t *data = NULL;
+
+ size = file_load(image->name, &data);
+ cpu_physical_memory_rw(image->addr, data, size, 1);
+ g_free(data);
+}
/* read()-like version */
ssize_t read_targphys(const char *name,
@@ -142,7 +159,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;
}
@@ -463,15 +485,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);
@@ -504,18 +525,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;
}
@@ -525,11 +537,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");
@@ -538,7 +550,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;
@@ -546,12 +557,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
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [Qemu-devel] [PATCH 1/2] extract file_load() function from rom_add_file() for reusing
2012-08-14 7:49 [Qemu-devel] [PATCH 1/2] extract file_load() function from rom_add_file() for reusing Olivia Yin
2012-08-14 7:49 ` [Qemu-devel] [PATCH 2/2] register reset handlers to write images into memory Olivia Yin
@ 2012-08-16 11:13 ` Yin Olivia-R63875
2012-08-16 11:22 ` Avi Kivity
2012-08-16 11:36 ` Peter Maydell
2 siblings, 1 reply; 7+ messages in thread
From: Yin Olivia-R63875 @ 2012-08-16 11:13 UTC (permalink / raw)
To: Yin Olivia-R63875, qemu-ppc@nongnu.org, qemu-devel@nongnu.org,
Avi Kivity, blauwirbel@gmail.com, benh@kernel.crashing.org
Hi Avi & Ben,
I've got feedback from qemu-ppc list and revised this patch according to Blue's comments.
Could you please give any comments?
Best Regards,
Olivia
> -----Original Message-----
> From: Yin Olivia-R63875
> Sent: Tuesday, August 14, 2012 3:50 PM
> To: qemu-ppc@nongnu.org; qemu-devel@nongnu.org
> Cc: Yin Olivia-R63875
> Subject: [PATCH 1/2] extract file_load() function from rom_add_file() for
> reusing
>
> Sanity check in rom_add_file() could be reused by other image loaders.
>
> 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 | 61 +++++++++++++++++++++++++++++++----------------------
> -----
> 1 files changed, 33 insertions(+), 28 deletions(-)
>
> diff --git a/hw/loader.c b/hw/loader.c
> index 33acc2f..f2099b6 100644
> --- a/hw/loader.c
> +++ b/hw/loader.c
> @@ -86,6 +86,36 @@ int load_image(const char *filename, uint8_t *addr)
> return size;
> }
>
> +static int file_load(const char *file, uint8_t **data) {
> + int fd = -1;
> + ssize_t rc, size;
> +
> + fd = open(file, O_RDONLY | O_BINARY);
> + if (fd == -1) {
> + fprintf(stderr, "Could not open file '%s': %s\n",
> + file, strerror(errno));
> + return -1;
> + }
> +
> + size = lseek(fd, 0, SEEK_END);
> + *data = g_malloc0(size);
> + lseek(fd, 0, SEEK_SET);
> + rc = read(fd, *data, size);
> + if (rc != size) {
> + fprintf(stderr, "file %-20s: read error: rc=%zd
> (expected %zd)\n",
> + file, rc, size);
> + goto err;
> + }
> + close(fd);
> + return size;
> +err:
> + if (fd != -1)
> + close(fd);
> + g_free(*data);
> + return -1;
> +}
> +
> /* read()-like version */
> ssize_t read_targphys(const char *name,
> int fd, target_phys_addr_t dst_addr, size_t nbytes)
> @@ -568,38 +598,22 @@ int rom_add_file(const char *file, const char
> *fw_dir,
> target_phys_addr_t addr, int32_t bootindex) {
> Rom *rom;
> - int rc, fd = -1;
> char devpath[100];
>
> rom = g_malloc0(sizeof(*rom));
> rom->name = g_strdup(file);
> + rom->addr = addr;
> rom->path = qemu_find_file(QEMU_FILE_TYPE_BIOS, rom->name);
> if (rom->path == NULL) {
> rom->path = g_strdup(file);
> }
>
> - fd = open(rom->path, O_RDONLY | O_BINARY);
> - if (fd == -1) {
> - fprintf(stderr, "Could not open option rom '%s': %s\n",
> - rom->path, strerror(errno));
> - goto err;
> - }
> -
> if (fw_dir) {
> rom->fw_dir = g_strdup(fw_dir);
> rom->fw_file = g_strdup(file);
> }
> - rom->addr = addr;
> - rom->romsize = lseek(fd, 0, SEEK_END);
> - rom->data = g_malloc0(rom->romsize);
> - lseek(fd, 0, SEEK_SET);
> - rc = read(fd, rom->data, rom->romsize);
> - if (rc != rom->romsize) {
> - fprintf(stderr, "rom: file %-20s: read error: rc=%d
> (expected %zd)\n",
> - rom->name, rc, rom->romsize);
> - goto err;
> - }
> - close(fd);
> +
> + rom->romsize = file_load(rom->path, &rom->data);
> rom_insert(rom);
> if (rom->fw_file && fw_cfg) {
> const char *basename;
> @@ -621,15 +635,6 @@ int rom_add_file(const char *file, const char
> *fw_dir,
>
> add_boot_device_path(bootindex, NULL, devpath);
> return 0;
> -
> -err:
> - if (fd != -1)
> - close(fd);
> - g_free(rom->data);
> - g_free(rom->path);
> - g_free(rom->name);
> - g_free(rom);
> - return -1;
> }
>
> int rom_add_blob(const char *name, const void *blob, size_t len,
> --
> 1.7.1
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [Qemu-devel] [PATCH 1/2] extract file_load() function from rom_add_file() for reusing
2012-08-16 11:13 ` [Qemu-devel] [PATCH 1/2] extract file_load() function from rom_add_file() for reusing Yin Olivia-R63875
@ 2012-08-16 11:22 ` Avi Kivity
2012-08-17 9:42 ` Yin Olivia-R63875
0 siblings, 1 reply; 7+ messages in thread
From: Avi Kivity @ 2012-08-16 11:22 UTC (permalink / raw)
To: Yin Olivia-R63875
Cc: blauwirbel@gmail.com, qemu-ppc@nongnu.org, qemu-devel@nongnu.org
On 08/16/2012 02:13 PM, Yin Olivia-R63875 wrote:
> Hi Avi & Ben,
>
> I've got feedback from qemu-ppc list and revised this patch according to Blue's comments.
> Could you please give any comments?
I don't really have any comments, those files are outside my area of
expertise. They look okay to me, but that doesn't mean much as I'm not
familiar with ppc.
--
error compiling committee.c: too many arguments to function
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH 1/2] extract file_load() function from rom_add_file() for reusing
2012-08-16 11:22 ` Avi Kivity
@ 2012-08-17 9:42 ` Yin Olivia-R63875
0 siblings, 0 replies; 7+ messages in thread
From: Yin Olivia-R63875 @ 2012-08-17 9:42 UTC (permalink / raw)
To: Avi Kivity
Cc: blauwirbel@gmail.com, qemu-ppc@nongnu.org, qemu-devel@nongnu.org
Hi Avi,
Thanks. Exactly the second patch is more important which saves the QEMU memory.
Best Regards,
Olivia
> -----Original Message-----
> From: Avi Kivity [mailto:avi@redhat.com]
> Sent: Thursday, August 16, 2012 7:23 PM
> To: Yin Olivia-R63875
> Cc: qemu-ppc@nongnu.org; qemu-devel@nongnu.org; blauwirbel@gmail.com;
> benh@kernel.crashing.org
> Subject: Re: [PATCH 1/2] extract file_load() function from rom_add_file()
> for reusing
>
> On 08/16/2012 02:13 PM, Yin Olivia-R63875 wrote:
> > Hi Avi & Ben,
> >
> > I've got feedback from qemu-ppc list and revised this patch according
> to Blue's comments.
> > Could you please give any comments?
>
>
> I don't really have any comments, those files are outside my area of
> expertise. They look okay to me, but that doesn't mean much as I'm not
> familiar with ppc.
>
> --
> error compiling committee.c: too many arguments to function
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH 1/2] extract file_load() function from rom_add_file() for reusing
2012-08-14 7:49 [Qemu-devel] [PATCH 1/2] extract file_load() function from rom_add_file() for reusing Olivia Yin
2012-08-14 7:49 ` [Qemu-devel] [PATCH 2/2] register reset handlers to write images into memory Olivia Yin
2012-08-16 11:13 ` [Qemu-devel] [PATCH 1/2] extract file_load() function from rom_add_file() for reusing Yin Olivia-R63875
@ 2012-08-16 11:36 ` Peter Maydell
2012-08-17 9:40 ` Yin Olivia-R63875
2 siblings, 1 reply; 7+ messages in thread
From: Peter Maydell @ 2012-08-16 11:36 UTC (permalink / raw)
To: Olivia Yin; +Cc: qemu-ppc, qemu-devel
On 14 August 2012 08:49, Olivia Yin <hong-hua.yin@freescale.com> wrote:
> Sanity check in rom_add_file() could be reused by other image loaders.
>
> 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 | 61 +++++++++++++++++++++++++++++++---------------------------
> 1 files changed, 33 insertions(+), 28 deletions(-)
>
> diff --git a/hw/loader.c b/hw/loader.c
> index 33acc2f..f2099b6 100644
> --- a/hw/loader.c
> +++ b/hw/loader.c
> @@ -86,6 +86,36 @@ int load_image(const char *filename, uint8_t *addr)
> return size;
> }
>
> +static int file_load(const char *file, uint8_t **data)
> +{
> + int fd = -1;
> + ssize_t rc, size;
> +
> + fd = open(file, O_RDONLY | O_BINARY);
> + if (fd == -1) {
> + fprintf(stderr, "Could not open file '%s': %s\n",
> + file, strerror(errno));
> + return -1;
> + }
> +
> + size = lseek(fd, 0, SEEK_END);
> + *data = g_malloc0(size);
> + lseek(fd, 0, SEEK_SET);
> + rc = read(fd, *data, size);
> + if (rc != size) {
> + fprintf(stderr, "file %-20s: read error: rc=%zd (expected %zd)\n",
> + file, rc, size);
> + goto err;
> + }
> + close(fd);
> + return size;
> +err:
> + if (fd != -1)
> + close(fd);
> + g_free(*data);
> + return -1;
> +}
Isn't this function effectively a reimplementation of the glib
g_file_get_contents() function? It would probably be better to
just make the callers use that instead.
-- PMM
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [Qemu-devel] [PATCH 1/2] extract file_load() function from rom_add_file() for reusing
2012-08-16 11:36 ` Peter Maydell
@ 2012-08-17 9:40 ` Yin Olivia-R63875
0 siblings, 0 replies; 7+ messages in thread
From: Yin Olivia-R63875 @ 2012-08-17 9:40 UTC (permalink / raw)
To: Peter Maydell; +Cc: qemu-ppc@nongnu.org, qemu-devel@nongnu.org
Hi Peter,
Thanks for the reminder. I'll update the second patch to use g_file_get_contents().
Best Regards,
Olivia
> -----Original Message-----
> From: Peter Maydell [mailto:peter.maydell@linaro.org]
> Sent: Thursday, August 16, 2012 7:36 PM
> To: Yin Olivia-R63875
> Cc: qemu-ppc@nongnu.org; qemu-devel@nongnu.org
> Subject: Re: [Qemu-devel] [PATCH 1/2] extract file_load() function from
> rom_add_file() for reusing
>
> On 14 August 2012 08:49, Olivia Yin <hong-hua.yin@freescale.com> wrote:
> > Sanity check in rom_add_file() could be reused by other image loaders.
> >
> > 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 | 61 +++++++++++++++++++++++++++++++--------------------
> -------
> > 1 files changed, 33 insertions(+), 28 deletions(-)
> >
> > diff --git a/hw/loader.c b/hw/loader.c index 33acc2f..f2099b6 100644
> > --- a/hw/loader.c
> > +++ b/hw/loader.c
> > @@ -86,6 +86,36 @@ int load_image(const char *filename, uint8_t *addr)
> > return size;
> > }
> >
> > +static int file_load(const char *file, uint8_t **data) {
> > + int fd = -1;
> > + ssize_t rc, size;
> > +
> > + fd = open(file, O_RDONLY | O_BINARY);
> > + if (fd == -1) {
> > + fprintf(stderr, "Could not open file '%s': %s\n",
> > + file, strerror(errno));
> > + return -1;
> > + }
> > +
> > + size = lseek(fd, 0, SEEK_END);
> > + *data = g_malloc0(size);
> > + lseek(fd, 0, SEEK_SET);
> > + rc = read(fd, *data, size);
> > + if (rc != size) {
> > + fprintf(stderr, "file %-20s: read error: rc=%zd
> (expected %zd)\n",
> > + file, rc, size);
> > + goto err;
> > + }
> > + close(fd);
> > + return size;
> > +err:
> > + if (fd != -1)
> > + close(fd);
> > + g_free(*data);
> > + return -1;
> > +}
>
> Isn't this function effectively a reimplementation of the glib
> g_file_get_contents() function? It would probably be better to just make
> the callers use that instead.
>
> -- PMM
^ permalink raw reply [flat|nested] 7+ messages in thread