> -----Original Message-----
> From: Alexander Graf [mailto:ag...@suse.de]
> Sent: Sunday, December 02, 2012 7:20 PM
> To: Yin Olivia-R63875
> Cc: qemu-devel@nongnu.org; qemu-...@nongnu.org
> Subject: Re: [Qemu-ppc] [RFC PATCH v7 1/4] use image_file_reset to reload
> initrd
>
> Missing patch description
>
> On 29.11.2012, at 06:26, Olivia Yin wrote:
>
> > Signed-off-by: Olivia Yin <hong-hua....@freescale.com>
> > ---
> > 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.
get_image_size() is a public function in QEMU.
hw/palm.c: rom_size = get_image_size(option_rom[0].name);
hw/mips_fulong2e.c: initrd_size = get_image_size
(loaderparams.initrd_filename);
hw/loader.c:int get_image_size(const char *filename)
hw/loader.c: size = get_image_size(filename);
hw/multiboot.c: mb_mod_length =
get_image_size(initrd_filename);
hw/pc.c: initrd_size = get_image_size(initrd_filename);
hw/mips_mipssim.c: initrd_size = get_image_size
(loaderparams.initrd_filename);
hw/pc_sysfw.c: bios_size = get_image_size(filename);
hw/smbios.c: int size = get_image_size(buf);
hw/leon3.c: bios_size = get_image_size(filename);
hw/pci.c: size = get_image_size(path);
hw/ppc_prep.c: bios_size = get_image_size(filename);
hw/mips_r4k.c: initrd_size = get_image_size
(loaderparams.initrd_filename);
hw/mips_r4k.c: bios_size = get_image_size(filename);
hw/alpha_dp264.c: initrd_size =
get_image_size(initrd_filename);
hw/loader.h:int get_image_size(const char *filename);
hw/mips_malta.c: initrd_size = get_image_size
(loaderparams.initrd_filename);
hw/highbank.c: uint32_t filesize =
get_image_size(sysboot_filename);
device_tree.c: dt_size = get_image_size(filename_path);
Do you think I should replace get_image_fize() with g_file_get_contents()?
Or just revise get_image_size() function as below?
gsize get_image_size(const char *filename)
{
gchar *content;
gsize size;
g_file_get_contents(image->name, &content, &size, &err);
return size;
}
> > }
> > 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.
OK. I'll remove rom_reset() since the rom->data could be written when
rom_load_all().
> 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...
Exactly ehdr.e_phnum = 3 and the only first one is PT_LOAD type and will be
added into roms and written into memory.
for(i = 0; i < ehdr.e_phnum; i++) {
ph = &phdr[i];
if (ph->p_type == PT_LOAD) {
>
> 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
> >
> >
> >
>
