[Qemu-devel] [PATCH] target-arm: Load ELF images with the correct machine type for CPU

[Qemu-devel] [PATCH] target-arm: Load ELF images with the correct machine type for CPU

[Peter Maydell]

When trying to load an ELF file specified via -kernel, we need to
pass load_elf() the ELF machine type corresponding to the CPU we're
booting with, not the one corresponding to the softmmu binary
we happen to be running. (The two are different in the case of
loading a 32-bit ARM ELF file into a 32 bit CPU being emulated
by qemu-system aarch64.) This was causing us to incorrectly fail
to load ELF images in this situation.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
This isn't really a big deal since we can just say "use the
qemu-system-arm binary instead". However maybe we should put
this into 2.0. Opinions?

Incidentally I suspect hw/i386/multiboot.c has a similar
problem where it calls load_elf() passing ELF_MACHINE.
---
 hw/arm/boot.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/hw/arm/boot.c b/hw/arm/boot.c
index dc62918..3d1f4a2 100644
--- a/hw/arm/boot.c
+++ b/hw/arm/boot.c
@@ -448,6 +448,7 @@ void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info *info)
     int initrd_size;
     int is_linux = 0;
     uint64_t elf_entry;
+    int elf_machine;
     hwaddr entry, kernel_load_offset;
     int big_endian;
     static const ARMInsnFixup *primary_loader;
@@ -463,9 +464,11 @@ void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info *info)
     if (arm_feature(&cpu->env, ARM_FEATURE_AARCH64)) {
         primary_loader = bootloader_aarch64;
         kernel_load_offset = KERNEL64_LOAD_ADDR;
+        elf_machine = EM_AARCH64;
     } else {
         primary_loader = bootloader;
         kernel_load_offset = KERNEL_LOAD_ADDR;
+        elf_machine = EM_ARM;
     }
 
     info->dtb_filename = qemu_opt_get(qemu_get_machine_opts(), "dtb");
@@ -501,7 +504,7 @@ void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info *info)
 
     /* Assume that raw images are linux kernels, and ELF images are not.  */
     kernel_size = load_elf(info->kernel_filename, NULL, NULL, &elf_entry,
-                           NULL, NULL, big_endian, ELF_MACHINE, 1);
+                           NULL, NULL, big_endian, elf_machine, 1);
     entry = elf_entry;
     if (kernel_size < 0) {
         kernel_size = load_uimage(info->kernel_filename, &entry, NULL,
-- 
1.9.0

Re: [Qemu-devel] [PATCH] target-arm: Load ELF images with the correct machine type for CPU

[Andreas Färber]

Am 21.03.2014 19:44, schrieb Peter Maydell:
> When trying to load an ELF file specified via -kernel, we need to
> pass load_elf() the ELF machine type corresponding to the CPU we're
> booting with, not the one corresponding to the softmmu binary
> we happen to be running. (The two are different in the case of
> loading a 32-bit ARM ELF file into a 32 bit CPU being emulated
> by qemu-system aarch64.) This was causing us to incorrectly fail
> to load ELF images in this situation.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Andreas Färber <afaerber@suse.de>

> ---
> This isn't really a big deal since we can just say "use the
> qemu-system-arm binary instead". However maybe we should put
> this into 2.0. Opinions?

Looks un-intrusive, so +1.

Cheers,
Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH] target-arm: Load ELF images with the correct machine type for CPU

[Peter Crosthwaite]

On Sat, Mar 22, 2014 at 4:44 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
> When trying to load an ELF file specified via -kernel, we need to
> pass load_elf() the ELF machine type corresponding to the CPU we're
> booting with, not the one corresponding to the softmmu binary
> we happen to be running. (The two are different in the case of
> loading a 32-bit ARM ELF file into a 32 bit CPU being emulated
> by qemu-system aarch64.) This was causing us to incorrectly fail
> to load ELF images in this situation.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>

If we wanted to be tricky, we could reverse this relationship, and
based on the elf header, switch the CPU execution state. That would
allow for backwards compatible boots to AArch32 guests without having
to BYO bootloader.

Regards,
Peter

> ---
> This isn't really a big deal since we can just say "use the
> qemu-system-arm binary instead". However maybe we should put
> this into 2.0. Opinions?
>
> Incidentally I suspect hw/i386/multiboot.c has a similar
> problem where it calls load_elf() passing ELF_MACHINE.
> ---
>  hw/arm/boot.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/hw/arm/boot.c b/hw/arm/boot.c
> index dc62918..3d1f4a2 100644
> --- a/hw/arm/boot.c
> +++ b/hw/arm/boot.c
> @@ -448,6 +448,7 @@ void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info *info)
>      int initrd_size;
>      int is_linux = 0;
>      uint64_t elf_entry;
> +    int elf_machine;
>      hwaddr entry, kernel_load_offset;
>      int big_endian;
>      static const ARMInsnFixup *primary_loader;
> @@ -463,9 +464,11 @@ void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info *info)
>      if (arm_feature(&cpu->env, ARM_FEATURE_AARCH64)) {
>          primary_loader = bootloader_aarch64;
>          kernel_load_offset = KERNEL64_LOAD_ADDR;
> +        elf_machine = EM_AARCH64;
>      } else {
>          primary_loader = bootloader;
>          kernel_load_offset = KERNEL_LOAD_ADDR;
> +        elf_machine = EM_ARM;
>      }
>
>      info->dtb_filename = qemu_opt_get(qemu_get_machine_opts(), "dtb");
> @@ -501,7 +504,7 @@ void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info *info)
>
>      /* Assume that raw images are linux kernels, and ELF images are not.  */
>      kernel_size = load_elf(info->kernel_filename, NULL, NULL, &elf_entry,
> -                           NULL, NULL, big_endian, ELF_MACHINE, 1);
> +                           NULL, NULL, big_endian, elf_machine, 1);
>      entry = elf_entry;
>      if (kernel_size < 0) {
>          kernel_size = load_uimage(info->kernel_filename, &entry, NULL,
> --
> 1.9.0
>
>

Re: [Qemu-devel] [PATCH] target-arm: Load ELF images with the correct machine type for CPU

[Alexander Graf]

On 21.03.14 19:44, Peter Maydell wrote:
> When trying to load an ELF file specified via -kernel, we need to
> pass load_elf() the ELF machine type corresponding to the CPU we're
> booting with, not the one corresponding to the softmmu binary
> we happen to be running. (The two are different in the case of
> loading a 32-bit ARM ELF file into a 32 bit CPU being emulated
> by qemu-system aarch64.) This was causing us to incorrectly fail
> to load ELF images in this situation.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> This isn't really a big deal since we can just say "use the
> qemu-system-arm binary instead". However maybe we should put
> this into 2.0. Opinions?
>
> Incidentally I suspect hw/i386/multiboot.c has a similar
> problem where it calls load_elf() passing ELF_MACHINE.

We have some compatibility code in the elf loader that says "if 
elf_machine == ppc64, then allow loading of ppc32 binaries too":

http://git.qemu.org/?p=qemu.git;a=blob;f=include/hw/elf_ops.h;h=c6b5129bab394704cf2197fe079ab195ec84ec2a;hb=HEAD#l213

which we need because our mac99 firmware is 32bit, but does know how to 
drive a ppc64 CPU. I suppose your case is slightly different for AArch64 
which is not compatible with 32bit binaries on firmware level.


Alex