Re: [Qemu-devel] [PATCH v2 1/5] elf-loader: Allow late loading of elf

[Cornelia Huck <cornelia.huck@de.ibm.com>]

On Thu, 23 Feb 2017 13:20:21 +0100
Cornelia Huck <cornelia.huck@de.ibm.com> wrote:

> From: Farhan Ali <alifm@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 <alifm@linux.vnet.ibm.com>
> Reviewed-by: Christian Borntraeger <borntraeger@de.ibm.com>
> Cc: Peter Maydell <peter.maydell@linaro.org>
> Signed-off-by: Cornelia Huck <cornelia.huck@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(-)
> 

Any further acks for this?

Or, put differently, any objections to me sending this in a pullreq for
2.9?

> 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];
> 
> @@ -473,11 +486,11 @@ int load_elf_as(const char *filename,
>      if (e_ident[EI_CLASS] == ELFCLASS64) {
>          ret = load_elf64(filename, fd, translate_fn, translate_opaque, must_swab,
>                           pentry, lowaddr, highaddr, elf_machine, clear_lsb,
> -                         data_swab, as);
> +                         data_swab, as, load_rom);
>      } else {
>          ret = load_elf32(filename, fd, translate_fn, translate_opaque, must_swab,
>                           pentry, lowaddr, highaddr, elf_machine, clear_lsb,
> -                         data_swab, as);
> +                         data_swab, as, load_rom);
>      }
> 
>   fail:
> diff --git a/include/hw/elf_ops.h b/include/hw/elf_ops.h
> index 25659b93be..a172a6068a 100644
> --- a/include/hw/elf_ops.h
> +++ b/include/hw/elf_ops.h
> @@ -264,7 +264,7 @@ static int glue(load_elf, SZ)(const char *name, int fd,
>                                int must_swab, uint64_t *pentry,
>                                uint64_t *lowaddr, uint64_t *highaddr,
>                                int elf_machine, int clear_lsb, int data_swab,
> -                              AddressSpace *as)
> +                              AddressSpace *as, bool load_rom)
>  {
>      struct elfhdr ehdr;
>      struct elf_phdr *phdr = NULL, *ph;
> @@ -403,10 +403,15 @@ static int glue(load_elf, SZ)(const char *name, int fd,
>                  *pentry = ehdr.e_entry - ph->p_vaddr + ph->p_paddr;
>              }
> 
> -            snprintf(label, sizeof(label), "phdr #%d: %s", i, name);
> +            if (load_rom) {
> +                snprintf(label, sizeof(label), "phdr #%d: %s", i, name);
> 
> -            /* rom_add_elf_program() seize the ownership of 'data' */
> -            rom_add_elf_program(label, data, file_size, mem_size, addr, as);
> +                /* rom_add_elf_program() seize the ownership of 'data' */
> +                rom_add_elf_program(label, data, file_size, mem_size, addr, as);
> +            } else {
> +                cpu_physical_memory_write(addr, data, file_size);
> +                g_free(data);
> +            }
> 
>              total_size += mem_size;
>              if (addr < low)
> diff --git a/include/hw/loader.h b/include/hw/loader.h
> index 0dbd8d6bf3..3df4d73398 100644
> --- a/include/hw/loader.h
> +++ b/include/hw/loader.h
> @@ -65,7 +65,7 @@ int load_image_gzipped(const char *filename, hwaddr addr, uint64_t max_sz);
>  #define ELF_LOAD_WRONG_ENDIAN -4
>  const char *load_elf_strerror(int error);
> 
> -/** load_elf_as:
> +/** load_elf_ram:
>   * @filename: Path of ELF file
>   * @translate_fn: optional function to translate load addresses
>   * @translate_opaque: opaque data passed to @translate_fn
> @@ -81,6 +81,7 @@ const char *load_elf_strerror(int error);
>   *             words and 3 for within doublewords.
>   * @as: The AddressSpace to load the ELF to. The value of address_space_memory
>   *      is used if nothing is supplied here.
> + * @load_rom : Load ELF binary as ROM
>   *
>   * Load an ELF file's contents to the emulated system's address space.
>   * Clients may optionally specify a callback to perform address
> @@ -93,6 +94,16 @@ const char *load_elf_strerror(int error);
>   * If @elf_machine is EM_NONE then the machine type will be read from the
>   * ELF header and no checks will be carried out against the machine type.
>   */
> +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);
> +
> +/** load_elf_as:
> + * Same as load_elf_ram(), but always loads the elf as ROM
> + */
>  int load_elf_as(const char *filename,
>                  uint64_t (*translate_fn)(void *, uint64_t),
>                  void *translate_opaque, uint64_t *pentry, uint64_t *lowaddr,