Re: [Qemu-stable] [PATCH v2 1/2] hw/core/loader: implement address translation in uimage loader

[Max Filippov]

Hi,

On Sun, Oct 19, 2014 at 8:04 AM, Max Filippov <address@hidden> wrote:
> Such address translation is needed when load address recorded in uImage
> is a virtual address. When the actual load address is requested, return
> untranslated address: user that needs the translated address can always
> apply translation function to it and those that need it untranslated
> don't need to do the inverse translation.
>
> Add translation function pointer and its parameter to uimage_load
> prototype. Update all existing users.
>
> No user-visible functional changes.

Alexander: thanks for the review.
Peter M., Edgar, Jia, Scott, Peter C., Paolo, I wouldn't mind your
reviewed/acked-bys or complaints.

> Cc: address@hidden
> Signed-off-by: Max Filippov <address@hidden>
> ---
> Changes v1->v2:
> - drop custom uImage loader interface and add generic translation function
>   instead (suggested by Alexander Graf).
>
>  hw/arm/boot.c              |  2 +-
>  hw/core/loader.c           | 17 +++++++++++++----
>  hw/m68k/an5206.c           |  3 ++-
>  hw/m68k/dummy_m68k.c       |  3 ++-
>  hw/m68k/mcf5208.c          |  3 ++-
>  hw/microblaze/boot.c       |  3 ++-
>  hw/openrisc/openrisc_sim.c |  2 +-
>  hw/ppc/e500.c              |  3 ++-
>  hw/ppc/ppc440_bamboo.c     |  3 ++-
>  hw/xtensa/xtfpga.c         |  3 ++-
>  include/hw/loader.h        |  4 +++-
>  11 files changed, 32 insertions(+), 14 deletions(-)
>
> diff --git a/hw/arm/boot.c b/hw/arm/boot.c
> index c8dc34f..e5627cc 100644
> --- a/hw/arm/boot.c
> +++ b/hw/arm/boot.c
> @@ -571,7 +571,7 @@ void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info 
> *info)
>      entry = elf_entry;
>      if (kernel_size < 0) {
>          kernel_size = load_uimage(info->kernel_filename, &entry, NULL,
> -                                  &is_linux);
> +                                  &is_linux, NULL, NULL);
>      }
>      /* On aarch64, it's the bootloader's job to uncompress the kernel. */
>      if (arm_feature(&cpu->env, ARM_FEATURE_AARCH64) && kernel_size < 0) {
> diff --git a/hw/core/loader.c b/hw/core/loader.c
> index 5f3a859..bbe6eb3 100644
> --- a/hw/core/loader.c
> +++ b/hw/core/loader.c
> @@ -477,7 +477,9 @@ static ssize_t gunzip(void *dst, size_t dstlen, uint8_t 
> *src,
>
>  /* Load a U-Boot image.  */
>  static int load_uboot_image(const char *filename, hwaddr *ep, hwaddr 
> *loadaddr,
> -                            int *is_linux, uint8_t image_type)
> +                            int *is_linux, uint8_t image_type,
> +                            uint64_t (*translate_fn)(void *, uint64_t),
> +                            void *translate_opaque)
>  {
>      int fd;
>      int size;
> @@ -511,6 +513,9 @@ static int load_uboot_image(const char *filename, hwaddr 
> *ep, hwaddr *loadaddr,
>      switch (hdr->ih_type) {
>      case IH_TYPE_KERNEL:
>          address = hdr->ih_load;
> +        if (translate_fn) {
> +            address = translate_fn(translate_opaque, address);
> +        }
>          if (loadaddr) {
>              *loadaddr = hdr->ih_load;
>          }
> @@ -587,15 +592,19 @@ out:
>  }
>
>  int load_uimage(const char *filename, hwaddr *ep, hwaddr *loadaddr,
> -                int *is_linux)
> +                int *is_linux,
> +                uint64_t (*translate_fn)(void *, uint64_t),
> +                void *translate_opaque)
>  {
> -    return load_uboot_image(filename, ep, loadaddr, is_linux, 
> IH_TYPE_KERNEL);
> +    return load_uboot_image(filename, ep, loadaddr, is_linux, IH_TYPE_KERNEL,
> +                            translate_fn, translate_opaque);
>  }
>
>  /* 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);
> +    return load_uboot_image(filename, NULL, &addr, NULL, IH_TYPE_RAMDISK,
> +                            NULL, NULL);
>  }
>
>  /* This simply prevents g_malloc in the function below from allocating
> diff --git a/hw/m68k/an5206.c b/hw/m68k/an5206.c
> index a9ac270..f1f1350 100644
> --- a/hw/m68k/an5206.c
> +++ b/hw/m68k/an5206.c
> @@ -74,7 +74,8 @@ static void an5206_init(MachineState *machine)
>                             NULL, NULL, 1, ELF_MACHINE, 0);
>      entry = elf_entry;
>      if (kernel_size < 0) {
> -        kernel_size = load_uimage(kernel_filename, &entry, NULL, NULL);
> +        kernel_size = load_uimage(kernel_filename, &entry, NULL, NULL,
> +                                  NULL, NULL);
>      }
>      if (kernel_size < 0) {
>          kernel_size = load_image_targphys(kernel_filename, KERNEL_LOAD_ADDR,
> diff --git a/hw/m68k/dummy_m68k.c b/hw/m68k/dummy_m68k.c
> index 957ef82..facd561 100644
> --- a/hw/m68k/dummy_m68k.c
> +++ b/hw/m68k/dummy_m68k.c
> @@ -50,7 +50,8 @@ static void dummy_m68k_init(MachineState *machine)
>                                 NULL, NULL, 1, ELF_MACHINE, 0);
>          entry = elf_entry;
>          if (kernel_size < 0) {
> -            kernel_size = load_uimage(kernel_filename, &entry, NULL, NULL);
> +            kernel_size = load_uimage(kernel_filename, &entry, NULL, NULL,
> +                                      NULL, NULL);
>          }
>          if (kernel_size < 0) {
>              kernel_size = load_image_targphys(kernel_filename,
> diff --git a/hw/m68k/mcf5208.c b/hw/m68k/mcf5208.c
> index 188230f..a01a445 100644
> --- a/hw/m68k/mcf5208.c
> +++ b/hw/m68k/mcf5208.c
> @@ -279,7 +279,8 @@ static void mcf5208evb_init(MachineState *machine)
>                             NULL, NULL, 1, ELF_MACHINE, 0);
>      entry = elf_entry;
>      if (kernel_size < 0) {
> -        kernel_size = load_uimage(kernel_filename, &entry, NULL, NULL);
> +        kernel_size = load_uimage(kernel_filename, &entry, NULL, NULL,
> +                                  NULL, NULL);
>      }
>      if (kernel_size < 0) {
>          kernel_size = load_image_targphys(kernel_filename, 0x40000000,
> diff --git a/hw/microblaze/boot.c b/hw/microblaze/boot.c
> index 6bf36d0..a2843cd 100644
> --- a/hw/microblaze/boot.c
> +++ b/hw/microblaze/boot.c
> @@ -154,7 +154,8 @@ void microblaze_load_kernel(MicroBlazeCPU *cpu, hwaddr 
> ddr_base,
>          if (kernel_size < 0) {
>              hwaddr uentry, loadaddr;
>
> -            kernel_size = load_uimage(kernel_filename, &uentry, &loadaddr, 
> 0);
> +            kernel_size = load_uimage(kernel_filename, &uentry, &loadaddr, 0,
> +                                      NULL, NULL);
>              boot_info.bootstrap_pc = uentry;
>              high = (loadaddr + kernel_size + 3) & ~3;
>          }
> diff --git a/hw/openrisc/openrisc_sim.c b/hw/openrisc/openrisc_sim.c
> index c110033..1da0657 100644
> --- a/hw/openrisc/openrisc_sim.c
> +++ b/hw/openrisc/openrisc_sim.c
> @@ -72,7 +72,7 @@ static void cpu_openrisc_load_kernel(ram_addr_t ram_size,
>          entry = elf_entry;
>          if (kernel_size < 0) {
>              kernel_size = load_uimage(kernel_filename,
> -                                      &entry, NULL, NULL);
> +                                      &entry, NULL, NULL, NULL, NULL);
>          }
>          if (kernel_size < 0) {
>              kernel_size = load_image_targphys(kernel_filename,
> diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c
> index 16c85ef..2157d87 100644
> --- a/hw/ppc/e500.c
> +++ b/hw/ppc/e500.c
> @@ -830,7 +830,8 @@ void ppce500_init(MachineState *machine, PPCE500Params 
> *params)
>           * Hrm. No ELF image? Try a uImage, maybe someone is giving us an
>           * ePAPR compliant kernel
>           */
> -        kernel_size = load_uimage(filename, &bios_entry, &loadaddr, NULL);
> +        kernel_size = load_uimage(filename, &bios_entry, &loadaddr, NULL,
> +                                  NULL, NULL);
>          if (kernel_size < 0) {
>              fprintf(stderr, "qemu: could not load firmware '%s'\n", 
> filename);
>              exit(1);
> diff --git a/hw/ppc/ppc440_bamboo.c b/hw/ppc/ppc440_bamboo.c
> index 81a06d3..778970a 100644
> --- a/hw/ppc/ppc440_bamboo.c
> +++ b/hw/ppc/ppc440_bamboo.c
> @@ -253,7 +253,8 @@ static void bamboo_init(MachineState *machine)
>
>      /* Load kernel. */
>      if (kernel_filename) {
> -        success = load_uimage(kernel_filename, &entry, &loadaddr, NULL);
> +        success = load_uimage(kernel_filename, &entry, &loadaddr, NULL,
> +                              NULL, NULL);
>          if (success < 0) {
>              success = load_elf(kernel_filename, NULL, NULL, &elf_entry,
>                                 &elf_lowaddr, NULL, 1, ELF_MACHINE, 0);
> diff --git a/hw/xtensa/xtfpga.c b/hw/xtensa/xtfpga.c
> index ed06ff7..3c2f409 100644
> --- a/hw/xtensa/xtfpga.c
> +++ b/hw/xtensa/xtfpga.c
> @@ -327,7 +327,8 @@ static void lx_init(const LxBoardDesc *board, 
> MachineState *machine)
>          } else {
>              hwaddr ep;
>              int is_linux;
> -            success = load_uimage(kernel_filename, &ep, NULL, &is_linux);
> +            success = load_uimage(kernel_filename, &ep, NULL, &is_linux,
> +                                  NULL, NULL);
>              if (success > 0 && is_linux) {
>                  entry_point = ep;
>              } else {
> diff --git a/include/hw/loader.h b/include/hw/loader.h
> index 9190387..054c6a2 100644
> --- a/include/hw/loader.h
> +++ b/include/hw/loader.h
> @@ -30,7 +30,9 @@ int load_elf(const char *filename, uint64_t 
> (*translate_fn)(void *, uint64_t),
>  int load_aout(const char *filename, hwaddr addr, int max_sz,
>                int bswap_needed, hwaddr target_page_size);
>  int load_uimage(const char *filename, hwaddr *ep,
> -                hwaddr *loadaddr, int *is_linux);
> +                hwaddr *loadaddr, int *is_linux,
> +                uint64_t (*translate_fn)(void *, uint64_t),
> +                void *translate_opaque);
>
>  /**
>   * load_ramdisk:
> --
> 1.8.1.4
>

-- 
Thanks.
-- Max