On 23.02.2017 13:20, Cornelia Huck wrote:
> From: Farhan Ali <al...@linux.vnet.ibm.com>
>
> The current QEMU ROM infrastructure rejects late loading of ROMs.
> And ELFs are currently loaded as ROM, this prevents delayed loading
> of ELFs. So when loading ELF, allow the user to specify if ELF should
> be loaded as ROM or not.
>
> If an ELF is not loaded as ROM, then they are not restored on a
> guest reboot/reset and so its upto the user to handle the reloading.
>
> Signed-off-by: Farhan Ali <al...@linux.vnet.ibm.com>
> Reviewed-by: Christian Borntraeger <borntrae...@de.ibm.com>
> Cc: Peter Maydell <peter.mayd...@linaro.org>
> Signed-off-by: Cornelia Huck <cornelia.h...@de.ibm.com>
> ---
> hw/core/loader.c | 17 +++++++++++++++--
> include/hw/elf_ops.h | 13 +++++++++----
> include/hw/loader.h | 13 ++++++++++++-
> 3 files changed, 36 insertions(+), 7 deletions(-)
>
> diff --git a/hw/core/loader.c b/hw/core/loader.c
> index ee5abd6eb7..9d1af1f6f3 100644
> --- a/hw/core/loader.c
> +++ b/hw/core/loader.c
> @@ -435,6 +435,19 @@ int load_elf_as(const char *filename,
> uint64_t *highaddr, int big_endian, int elf_machine,
> int clear_lsb, int data_swab, AddressSpace *as)
> {
> + return load_elf_ram(filename, translate_fn, translate_opaque,
> + pentry, lowaddr, highaddr, big_endian, elf_machine,
> + clear_lsb, data_swab, as, true);
> +}
> +
> +/* return < 0 if error, otherwise the number of bytes loaded in memory */
> +int load_elf_ram(const char *filename,
> + uint64_t (*translate_fn)(void *, uint64_t),
> + void *translate_opaque, uint64_t *pentry, uint64_t *lowaddr,
> + uint64_t *highaddr, int big_endian, int elf_machine,
> + int clear_lsb, int data_swab, AddressSpace *as,
> + bool load_rom)
> +{
> int fd, data_order, target_data_order, must_swab, ret = ELF_LOAD_FAILED;
> uint8_t e_ident[EI_NIDENT];
<bikeshedpaintingsofeelfreetoignoreme>
The patch looks basically fine to me, but I think it's a little bit
confusing to have a function called load_elf_ram() which can also be
used to load ROMs with a load_rom=1 parameter. If I read
"load_elf_ram()", I'd expect a function that can only read ELFs to RAM.
So what about adding the "load_rom" parameter to load_elf_as() instead
and then making load_elf_ram() a wrapper function to that one with
load_rom=0 ? AFAICS there is only one additional caller to load_elf_as
(in the generic-loader), so the additional effort here should be OK, I
think.
</bikeshedpaintingsofeelfreetoignoreme>
Thomas
