On 18 January 2016 at 07:12, Peter Crosthwaite
<crosthwaitepe...@gmail.com> wrote:
> Support ARM big-endian ELF files in system-mode emulation. When loading
> an elf, determine the endianness mode expected by the elf, and set the
> relevant CPU state accordingly.
>
> With this, big-endian modes are now fully supported via system-mode LE,
> so there is no need to restrict the elf loading to the TARGET
> endianness so the ifdeffery on TARGET_WORDS_BIGENDIAN goes away.
>
> Signed-off-by: Peter Crosthwaite <crosthwaite.pe...@gmail.com>
> ---
>
> hw/arm/boot.c | 96
> ++++++++++++++++++++++++++++++++++++++++++----------
> include/hw/arm/arm.h | 9 +++++
> 2 files changed, 88 insertions(+), 17 deletions(-)
>
> diff --git a/hw/arm/boot.c b/hw/arm/boot.c
> index 0de4269..053c9e8 100644
> --- a/hw/arm/boot.c
> +++ b/hw/arm/boot.c
> @@ -465,9 +465,34 @@ static void do_cpu_reset(void *opaque)
> cpu_reset(cs);
> if (info) {
> if (!info->is_linux) {
> + int i;
> /* Jump to the entry point. */
> uint64_t entry = info->entry;
>
> + switch (info->endianness) {
> + case ARM_ENDIANNESS_LE:
> + env->cp15.sctlr_el[1] &= ~SCTLR_E0E;
> + for (i = 1; i < 4; ++i) {
> + env->cp15.sctlr_el[i] &= ~SCTLR_EE;
> + }
> + env->uncached_cpsr &= ~CPSR_E;
> + break;
> + case ARM_ENDIANNESS_BE8:
> + env->cp15.sctlr_el[1] |= SCTLR_E0E;
> + for (i = 1; i < 4; ++i) {
> + env->cp15.sctlr_el[i] |= SCTLR_EE;
> + }
> + env->uncached_cpsr |= CPSR_E;
> + break;
> + case ARM_ENDIANNESS_BE32:
> + env->cp15.sctlr_el[1] |= SCTLR_B;
> + break;
> + case ARM_ENDIANNESS_UNKNOWN:
> + break; /* Board's decision */
> + default:
> + g_assert_not_reached();
> + }
Do we really want this much magic for non-linux images? I would
expect that the image would be intended to run with whatever the
state the board puts the CPU in from reset (ie the CPU has suitable
QOM properties for its initial endianness state, corresponding to
real hardware reset-config signals like the A15's CFGEND/CFGTE).
> +
> if (!env->aarch64) {
> env->thumb = info->entry & 1;
> entry &= 0xfffffffe;
> @@ -589,16 +614,23 @@ static void arm_load_kernel_notify(Notifier *notifier,
> void *data)
> int kernel_size;
> int initrd_size;
> int is_linux = 0;
> +
> uint64_t elf_entry, elf_low_addr, elf_high_addr;
> int elf_machine;
> + bool elf_is64;
> + union {
> + Elf32_Ehdr h32;
> + Elf64_Ehdr h64;
> + } elf_header;
> +
> hwaddr entry, kernel_load_offset;
> - int big_endian;
> static const ARMInsnFixup *primary_loader;
> ArmLoadKernelNotifier *n = DO_UPCAST(ArmLoadKernelNotifier,
> notifier, notifier);
> ARMCPU *cpu = n->cpu;
> struct arm_boot_info *info =
> container_of(n, struct arm_boot_info, load_kernel_notifier);
> + Error *err = NULL;
>
> /* The board code is not supposed to set secure_board_setup unless
> * running its code in secure mode is actually possible, and KVM
> @@ -678,12 +710,6 @@ static void arm_load_kernel_notify(Notifier *notifier,
> void *data)
> if (info->nb_cpus == 0)
> info->nb_cpus = 1;
>
> -#ifdef TARGET_WORDS_BIGENDIAN
> - big_endian = 1;
> -#else
> - big_endian = 0;
> -#endif
Was this code ever built with TARGET_WORDS_BIGENDIAN defined?
> -
> /* We want to put the initrd far enough into RAM that when the
> * kernel is uncompressed it will not clobber the initrd. However
> * on boards without much RAM we must ensure that we still leave
> @@ -698,16 +724,52 @@ static void arm_load_kernel_notify(Notifier *notifier,
> void *data)
> MIN(info->ram_size / 2, 128 * 1024 * 1024);
>
> /* Assume that raw images are linux kernels, and ELF images are not. */
> - kernel_size = load_elf(info->kernel_filename, NULL, NULL, &elf_entry,
> - &elf_low_addr, &elf_high_addr, big_endian,
> - elf_machine, 1, 0);
> - if (kernel_size > 0 && have_dtb(info)) {
> - /* If there is still some room left at the base of RAM, try and put
> - * the DTB there like we do for images loaded with -bios or -pflash.
> - */
> - if (elf_low_addr > info->loader_start
> - || elf_high_addr < info->loader_start) {
> - /* Pass elf_low_addr as address limit to load_dtb if it may be
> +
> + load_elf_hdr(info->kernel_filename, &elf_header, &elf_is64, &err);
> +
> + if (!err) {
> + int data_swab = 0;
> + bool big_endian;
> +
> + if (elf_is64) {
> + big_endian = elf_header.h64.e_ident[EI_DATA] == ELFDATA2MSB;
> + info->endianness = big_endian ? ARM_ENDIANNESS_BE8
> + : ARM_ENDIANNESS_LE;
> + } else {
> + big_endian = elf_header.h32.e_ident[EI_DATA] == ELFDATA2MSB;
> + if (big_endian) {
> + if (bswap32(elf_header.h32.e_flags) & EF_ARM_BE8) {
> + info->endianness = ARM_ENDIANNESS_BE8;
> + } else {
> + info->endianness = ARM_ENDIANNESS_BE32;
> + /* In BE32, the CPU has a different view of the per-byte
> + * address map than the rest of the system. BE32 elfs are
> + * organised such that they can be programmed through the
> + * CPUs per-word byte-reversed view of the world. QEMU
> + * however loads elfs independently of the CPU. So tell
> + * the elf loader to byte reverse the data for us.
> + */
> + data_swab = 2;
> + }
> + } else {
> + info->endianness = ARM_ENDIANNESS_LE;
> + }
> + }
> +
> + kernel_size = load_elf(info->kernel_filename, NULL, NULL, &elf_entry,
> + &elf_low_addr, &elf_high_addr, big_endian,
> + elf_machine, 1, data_swab);
> + if (kernel_size <= 0) {
> + exit(1);
> + }
> +
> + if (have_dtb(info) && (elf_low_addr > info->loader_start ||
> + elf_high_addr < info->loader_start)) {
> + /* If there is still some room left at the base of RAM, try and
> + * put the DTB there like we do for images loaded with -bios or
> + * -pflash.
> + *
> + * Pass elf_low_addr as address limit to load_dtb if it may be
> * pointing into RAM, otherwise pass '0' (no limit)
> */
> if (elf_low_addr < info->loader_start) {
I think this change would be easier to read if we had a new
(static) function in this file load_arm_elf() which did the
"load header; figure out data_swab; load full elf file with
corresponding swab value". Then the change to this function would
just be a one-liner turning the load_elf() call into load_arm_elf().
thanks
-- PMM
