[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-ppc] [Qemu-devel] [RFC PATCH v4 2/3] use uimage_reset to reloa
From: |
Alexander Graf |
Subject: |
Re: [Qemu-ppc] [Qemu-devel] [RFC PATCH v4 2/3] use uimage_reset to reload uimage |
Date: |
Mon, 19 Nov 2012 12:26:22 +0100 |
On 16.11.2012, at 07:11, Yin Olivia-R63875 wrote:
> Hi Stuart,
>
> load_uimage() is a public function which is called by below files:
> hw/dummy_m68k.c: kernel_size = load_uimage(kernel_filename,
> &entry, NULL, NULL);
> hw/an5206.c: kernel_size = load_uimage(kernel_filename, &entry,
> NULL, NULL);
> hw/ppc440_bamboo.c: success = load_uimage(kernel_filename,
> &entry, &loadaddr, NULL);
> hw/openrisc_sim.c: kernel_size =
> load_uimage(kernel_filename, entry, NULL, NULL);
> hw/ppc/e500.c: kernel_size =
> load_uimage(params->kernel_filename, &entry, &loadaddr, NULL);
> hw/arm_boot.c: kernel_size = load_uimage(info->kernel_filename,
> &entry, NULL, &is_linux);
> hw/microblaze_boot.c: kernel_size =
> load_uimage(kernel_filename, &uentry, &loadaddr, 0);
> hw/mcf5208.c: kernel_size = load_uimage(kernel_filename, &entry,
> NULL, NULL);
>
> Most value of *is_linux is NULL, only arm_boot.c will check the value of
> is_linux.
> I just define an static variable is_linux other than include it into
> structure ImageFile.
Just load the uImage once on load_uimage, so that you can populate all the
passed down return variables (entry, loadaddr, is_linux). No need for a global
here.
> To define static variable kernel_loaded because function
> uimage_physical_loader() will
> be called twice.
> 1) load_uimage() which will run only one time when startup.
> 2) uimage_reset() which will run once reset, so it maybe run many times.
> Since "ROM images must be loaded at startup", it means rom_add_*() should not
> be called by any reset handlers.
> So I use variable kernel_loaded to decide the booting is startup or reset.
Pass it as a function argument then?
Alex
>
>
> Best Regards,
> Olivia
>
>
>> -----Original Message-----
>> From: Stuart Yoder [mailto:address@hidden
>> Sent: Friday, November 16, 2012 4:46 AM
>> To: Yin Olivia-R63875
>> Cc: address@hidden; address@hidden
>> Subject: Re: [Qemu-devel] [RFC PATCH v4 2/3] use uimage_reset to reload
>> uimage
>>
>> On Wed, Nov 14, 2012 at 2:28 PM, Olivia Yin <address@hidden>
>> wrote:
>>> Signed-off-by: Olivia Yin <address@hidden>
>>> ---
>>> hw/loader.c | 64 ++++++++++++++++++++++++++++++++++++++++++---------
>> -------
>>> 1 files changed, 46 insertions(+), 18 deletions(-)
>>>
>>> diff --git a/hw/loader.c b/hw/loader.c index a8a0a09..1a909d0 100644
>>> --- a/hw/loader.c
>>> +++ b/hw/loader.c
>>> @@ -55,6 +55,8 @@
>>> #include <zlib.h>
>>>
>>> static int roms_loaded;
>>> +static int kernel_loaded;
>>> +static int *is_linux;
>>
>> Why do we need these 2 new variables?
>>
>>> /* return the size or -1 if error */
>>> int get_image_size(const char *filename) @@ -472,15 +474,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, hwaddr *ep,
>>> - hwaddr *loadaddr, int *is_linux)
>>> +/* write uimage into memory */
>>> +static int uimage_physical_loader(const char *filename, uint8_t **data,
>>> + hwaddr *loadaddr, int *is_linux)
>>> {
>>> 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); @@ -521,10 +522,9 @@
>>> int load_uimage(const char *filename, hwaddr *ep,
>>> *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;
>>> }
>>> @@ -534,11 +534,11 @@ int load_uimage(const char *filename, hwaddr *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");
>>> @@ -547,7 +547,9 @@ int load_uimage(const char *filename, hwaddr *ep,
>>> hdr->ih_size = bytes;
>>> }
>>>
>>> - rom_add_blob_fixed(filename, data, hdr->ih_size, hdr->ih_load);
>>> + if (!kernel_loaded) {
>>> + rom_add_blob_fixed(filename, *data, hdr->ih_size, hdr-
>>> ih_load);
>>> + }
>>
>> Why do we need to keep the rom_add_blob_fixed() call? I thought the
>> point of this was to remove adding roms.
>>
>>> if (loadaddr)
>>> *loadaddr = hdr->ih_load;
>>> @@ -555,12 +557,41 @@ int load_uimage(const char *filename, hwaddr *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,
>>> + is_linux);
>>
>> Not sure why we need is_linux here. I think you should just set that
>> parameter to NULL.
>> Nothing will look at is_linux.
>>
>> Stuart
>
>
>
Re: [Qemu-ppc] [Qemu-devel] [RFC PATCH v4 1/3] use image_file_reset to reload initrd image, Stuart Yoder, 2012/11/15
Re: [Qemu-ppc] [RFC PATCH v4 0/3] reload kernel/initrd/elf images when reset, Yin Olivia-R63875, 2012/11/15