Re: [Qemu-devel] [PATCH v9 5/8] loader: Add AddressSpace loading support to uImages

[Peter Maydell <peter.maydell@linaro.org>]

On 14 July 2016 at 01:03, Alistair Francis <alistair.francis@xilinx.com> wrote:
> Add a new function load_uimage_as() that allows the caller to
> specify an AddressSpace to use when loading the uImage. The
> original load_uimage() function doesn't have any change in
> functionality.
>
> Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
> ---
>
>  hw/core/loader.c    | 17 +++++++++++++----
>  include/hw/loader.h |  6 ++++++
>  2 files changed, 19 insertions(+), 4 deletions(-)
>
> diff --git a/hw/core/loader.c b/hw/core/loader.c
> index 0f69894..861dbc2 100644
> --- a/hw/core/loader.c
> +++ b/hw/core/loader.c
> @@ -581,7 +581,7 @@ static ssize_t gunzip(void *dst, size_t dstlen, uint8_t *src,
>  static int load_uboot_image(const char *filename, hwaddr *ep, hwaddr *loadaddr,
>                              int *is_linux, uint8_t image_type,
>                              uint64_t (*translate_fn)(void *, uint64_t),
> -                            void *translate_opaque)
> +                            void *translate_opaque, AddressSpace *as)
>  {
>      int fd;
>      int size;
> @@ -682,7 +682,7 @@ static int load_uboot_image(const char *filename, hwaddr *ep, hwaddr *loadaddr,
>          hdr->ih_size = bytes;
>      }
>
> -    rom_add_blob_fixed(filename, data, hdr->ih_size, address);
> +    rom_add_blob_fixed_as(filename, data, hdr->ih_size, address, as);
>
>      ret = hdr->ih_size;
>
> @@ -698,14 +698,23 @@ int load_uimage(const char *filename, hwaddr *ep, hwaddr *loadaddr,
>                  void *translate_opaque)
>  {
>      return load_uboot_image(filename, ep, loadaddr, is_linux, IH_TYPE_KERNEL,
> -                            translate_fn, translate_opaque);
> +                            translate_fn, translate_opaque, NULL);
> +}
> +
> +int load_uimage_as(const char *filename, hwaddr *ep, hwaddr *loadaddr,
> +                   int *is_linux,
> +                   uint64_t (*translate_fn)(void *, uint64_t),
> +                   void *translate_opaque, AddressSpace *as)
> +{
> +    return load_uboot_image(filename, ep, loadaddr, is_linux, IH_TYPE_KERNEL,
> +                            translate_fn, translate_opaque, as);
>  }
>
>  /* Load a ramdisk.  */
>  int load_ramdisk(const char *filename, hwaddr addr, uint64_t max_sz)
>  {
>      return load_uboot_image(filename, NULL, &addr, NULL, IH_TYPE_RAMDISK,
> -                            NULL, NULL);
> +                            NULL, NULL, NULL);
>  }
>
>  /* Load a gzip-compressed kernel to a dynamically allocated buffer. */
> diff --git a/include/hw/loader.h b/include/hw/loader.h
> index 36a16cc..ede98f6 100644
> --- a/include/hw/loader.h
> +++ b/include/hw/loader.h
> @@ -108,6 +108,10 @@ int load_uimage(const char *filename, hwaddr *ep,
>                  hwaddr *loadaddr, int *is_linux,
>                  uint64_t (*translate_fn)(void *, uint64_t),
>                  void *translate_opaque);
> +int load_uimage_as(const char *filename, hwaddr *ep,
> +                   hwaddr *loadaddr, int *is_linux,
> +                   uint64_t (*translate_fn)(void *, uint64_t),
> +                   void *translate_opaque, AddressSpace *as);

The code changes are good, but could we have a doc comment for
this new function, please?

thanks
-- PMM