qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH  v1 0/4] ELF and (macro) safety
@ 2019-09-10 19:34 Alex Bennée
  2019-09-10 19:34 ` [Qemu-devel] [PATCH v1 1/4] target/ppc: fix signal delivery for ppc64abi32 Alex Bennée
                   ` (3 more replies)
  0 siblings, 4 replies; 24+ messages in thread
From: Alex Bennée @ 2019-09-10 19:34 UTC (permalink / raw)
  To: peter.maydell; +Cc: qemu-arm, Alex Bennée, qemu-devel

Hi,

This is a small re-factoring series which I'll be needing for adding
guest architecture awareness to plugins. There is a little clean up of
concerns by removing the "template" type behaviour from elf.h into a
new elf-types.inc.h file. I then rationalise the ELF related headers
to all be in the same place. Finally the actual useful piece of moving
the definition of ELF_ARCH out of the two loader files and into an
stand alone header.

Alex Bennée (4):
  target/ppc: fix signal delivery for ppc64abi32
  elf: move elf.h to elf/elf.h and split out types
  elf: move elf_ops.h into include/elf/ and rename
  elf: move ELF_ARCH definition to elf-arch.h

 bsd-user/elfload.c                          |  15 +--
 contrib/elf2dmp/qemu_elf.h                  |   2 +-
 disas.c                                     |   2 +-
 dump/dump.c                                 |   2 +-
 dump/win_dump.c                             |   2 +-
 hw/alpha/dp264.c                            |   2 +-
 hw/arm/armv7m.c                             |   2 +-
 hw/arm/boot.c                               |   2 +-
 hw/core/loader.c                            |   7 +-
 hw/cris/axis_dev88.c                        |   2 +-
 hw/cris/boot.c                              |   2 +-
 hw/hppa/machine.c                           |   2 +-
 hw/i386/multiboot.c                         |   2 +-
 hw/i386/pc.c                                |   2 +-
 hw/lm32/lm32_boards.c                       |   2 +-
 hw/lm32/milkymist.c                         |   2 +-
 hw/m68k/an5206.c                            |   2 +-
 hw/m68k/mcf5208.c                           |   2 +-
 hw/microblaze/boot.c                        |   2 +-
 hw/mips/mips_fulong2e.c                     |   2 +-
 hw/mips/mips_malta.c                        |   2 +-
 hw/mips/mips_mipssim.c                      |   2 +-
 hw/mips/mips_r4k.c                          |   2 +-
 hw/moxie/moxiesim.c                         |   2 +-
 hw/nios2/boot.c                             |   2 +-
 hw/openrisc/openrisc_sim.c                  |   2 +-
 hw/pci-host/prep.c                          |   2 +-
 hw/ppc/e500.c                               |   2 +-
 hw/ppc/mac_newworld.c                       |   2 +-
 hw/ppc/mac_oldworld.c                       |   2 +-
 hw/ppc/ppc440_bamboo.c                      |   2 +-
 hw/ppc/prep.c                               |   2 +-
 hw/ppc/sam460ex.c                           |   2 +-
 hw/ppc/spapr.c                              |   2 +-
 hw/ppc/spapr_vio.c                          |   2 +-
 hw/ppc/virtex_ml507.c                       |   2 +-
 hw/riscv/boot.c                             |   2 +-
 hw/s390x/ipl.c                              |   2 +-
 hw/sparc/leon3.c                            |   2 +-
 hw/sparc/sun4m.c                            |   2 +-
 hw/sparc64/sun4u.c                          |   2 +-
 hw/tricore/tricore_testboard.c              |   2 +-
 hw/xtensa/sim.c                             |   2 +-
 hw/xtensa/xtfpga.c                          |   2 +-
 include/elf/elf-arch.h                      | 109 ++++++++++++++++++++
 include/elf/elf-types.inc.h                 |  63 +++++++++++
 include/{ => elf}/elf.h                     |  42 --------
 include/{hw/elf_ops.h => elf/elf_ops.inc.h} |   9 ++
 include/hw/core/generic-loader.h            |   2 +-
 linux-user/arm/cpu_loop.c                   |   2 +-
 linux-user/elfload.c                        |  32 ++----
 linux-user/main.c                           |   2 +-
 linux-user/mips/cpu_loop.c                  |   2 +-
 linux-user/ppc/signal.c                     |   4 +-
 linux-user/riscv/cpu_loop.c                 |   2 +-
 target/arm/arch_dump.c                      |   2 +-
 target/i386/arch_dump.c                     |   2 +-
 target/ppc/arch_dump.c                      |   2 +-
 target/ppc/kvm.c                            |   2 +-
 target/s390x/arch_dump.c                    |   2 +-
 tcg/arm/tcg-target.inc.c                    |   2 +-
 tcg/ppc/tcg-target.inc.c                    |   2 +-
 tcg/s390/tcg-target.inc.c                   |   2 +-
 tcg/tcg.c                                   |   5 +-
 tests/tcg/configure.sh                      |   1 +
 tests/tcg/multiarch/Makefile.target         |   5 -
 util/getauxval.c                            |   2 +-
 67 files changed, 258 insertions(+), 146 deletions(-)
 create mode 100644 include/elf/elf-arch.h
 create mode 100644 include/elf/elf-types.inc.h
 rename include/{ => elf}/elf.h (98%)
 rename include/{hw/elf_ops.h => elf/elf_ops.inc.h} (98%)

-- 
2.20.1



^ permalink raw reply	[flat|nested] 24+ messages in thread

* [Qemu-devel] [PATCH v1 1/4] target/ppc: fix signal delivery for ppc64abi32
  2019-09-10 19:34 [Qemu-devel] [PATCH v1 0/4] ELF and (macro) safety Alex Bennée
@ 2019-09-10 19:34 ` Alex Bennée
  2019-09-10 19:45   ` Alex Bennée
  2019-09-10 19:34 ` [Qemu-devel] [PATCH v1 2/4] elf: move elf.h to elf/elf.h and split out types Alex Bennée
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 24+ messages in thread
From: Alex Bennée @ 2019-09-10 19:34 UTC (permalink / raw)
  To: peter.maydell
  Cc: Riku Voipio, qemu-arm, Alex Bennée, qemu-devel, Laurent Vivier

We were incorrectly setting NIP resulting in a segfault. This fixes
linux-test for this ABI.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 linux-user/ppc/signal.c             | 4 +++-
 tests/tcg/configure.sh              | 1 +
 tests/tcg/multiarch/Makefile.target | 5 -----
 3 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/linux-user/ppc/signal.c b/linux-user/ppc/signal.c
index 619a56950df..5b82af6cb62 100644
--- a/linux-user/ppc/signal.c
+++ b/linux-user/ppc/signal.c
@@ -501,7 +501,9 @@ void setup_rt_frame(int sig, struct target_sigaction *ka,
     int i, err = 0;
 #if defined(TARGET_PPC64)
     struct target_sigcontext *sc = 0;
+#if !defined(TARGET_ABI32)
     struct image_info *image = ((TaskState *)thread_cpu->opaque)->info;
+#endif
 #endif
 
     rt_sf_addr = get_sigframe(ka, env, sizeof(*rt_sf));
@@ -557,7 +559,7 @@ void setup_rt_frame(int sig, struct target_sigaction *ka,
     env->gpr[5] = (target_ulong) h2g(&rt_sf->uc);
     env->gpr[6] = (target_ulong) h2g(rt_sf);
 
-#if defined(TARGET_PPC64)
+#if defined(TARGET_PPC64) && !defined(TARGET_ABI32)
     if (get_ppc64_abi(image) < 2) {
         /* ELFv1 PPC64 function pointers are pointers to OPD entries. */
         struct target_func_ptr *handler =
diff --git a/tests/tcg/configure.sh b/tests/tcg/configure.sh
index 6c4a471aeae..e8a1a1495fc 100755
--- a/tests/tcg/configure.sh
+++ b/tests/tcg/configure.sh
@@ -54,6 +54,7 @@ fi
 : ${cross_cc_cflags_ppc="-m32"}
 : ${cross_cc_ppc64="powerpc-linux-gnu-gcc"}
 : ${cross_cc_cflags_ppc64="-m64"}
+: ${cross_cc_cflags_ppc64abi32="-mcpu=power8"}
 : ${cross_cc_ppc64le="powerpc64le-linux-gnu-gcc"}
 : ${cross_cc_cflags_s390x="-m64"}
 : ${cross_cc_cflags_sparc="-m32 -mv8plus -mcpu=ultrasparc"}
diff --git a/tests/tcg/multiarch/Makefile.target b/tests/tcg/multiarch/Makefile.target
index 6b1e30e2fec..e6893b2e283 100644
--- a/tests/tcg/multiarch/Makefile.target
+++ b/tests/tcg/multiarch/Makefile.target
@@ -12,11 +12,6 @@ VPATH 		+= $(MULTIARCH_SRC)
 MULTIARCH_SRCS   =$(notdir $(wildcard $(MULTIARCH_SRC)/*.c))
 MULTIARCH_TESTS  =$(MULTIARCH_SRCS:.c=)
 
-# FIXME: ppc64abi32 linux-test seems to have issues but the other basic tests work
-ifeq ($(TARGET_NAME),ppc64abi32)
-BROKEN_TESTS = linux-test
-endif
-
 # Update TESTS
 TESTS		+= $(filter-out $(BROKEN_TESTS), $(MULTIARCH_TESTS))
 
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [Qemu-devel] [PATCH v1 2/4] elf: move elf.h to elf/elf.h and split out types
  2019-09-10 19:34 [Qemu-devel] [PATCH v1 0/4] ELF and (macro) safety Alex Bennée
  2019-09-10 19:34 ` [Qemu-devel] [PATCH v1 1/4] target/ppc: fix signal delivery for ppc64abi32 Alex Bennée
@ 2019-09-10 19:34 ` Alex Bennée
  2019-09-11  0:08   ` David Gibson
                     ` (3 more replies)
  2019-09-10 19:34 ` [Qemu-devel] [PATCH v1 3/4] elf: move elf_ops.h into include/elf/ and rename Alex Bennée
  2019-09-10 19:34 ` [Qemu-devel] [PATCH v1 4/4] elf: move ELF_ARCH definition to elf-arch.h Alex Bennée
  3 siblings, 4 replies; 24+ messages in thread
From: Alex Bennée @ 2019-09-10 19:34 UTC (permalink / raw)
  To: peter.maydell
  Cc: Cornelia Huck, Sagar Karandikar, Michael S. Tsirkin,
	Anthony Green, Palmer Dabbelt, Mark Cave-Ayland, qemu-devel,
	Max Filippov, KONRAD Frederic, Alistair Francis,
	Edgar E. Iglesias, Marek Vasut, Jia Liu, Aleksandar Rikalo,
	Helge Deller, David Hildenbrand, Halil Pasic,
	Christian Borntraeger, Hervé Poussineau,
	Marc-André Lureau, David Gibson, Artyom Tarasenko,
	Eduardo Habkost, Riku Voipio, Fabien Chouteau,
	open list:S390-ccw boot, qemu-arm, Stafford Horne,
	Alex Bennée, Richard Henderson, open list:RISC-V TCG CPUs,
	Viktor Prutyanov, Thomas Huth, Bastian Koppelmann, Chris Wulff,
	Laurent Vivier, Michael Walle, open list:PReP,
	Aleksandar Markovic, Paolo Bonzini, Aurelien Jarno

Most of the users of elf.h just want the standard Elf definitions. The
couple that want more than that want an expansion based on ELF_CLASS
which can be used for size agnostic code. The later is moved into
elf/elf-types.inc.h to make it clearer what it is for. While doing
that I also removed the whitespace damage.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 bsd-user/elfload.c               |  2 +-
 contrib/elf2dmp/qemu_elf.h       |  2 +-
 disas.c                          |  2 +-
 dump/dump.c                      |  2 +-
 dump/win_dump.c                  |  2 +-
 hw/alpha/dp264.c                 |  2 +-
 hw/arm/armv7m.c                  |  2 +-
 hw/arm/boot.c                    |  2 +-
 hw/core/loader.c                 |  3 +-
 hw/cris/axis_dev88.c             |  2 +-
 hw/cris/boot.c                   |  2 +-
 hw/hppa/machine.c                |  2 +-
 hw/i386/multiboot.c              |  2 +-
 hw/i386/pc.c                     |  2 +-
 hw/lm32/lm32_boards.c            |  2 +-
 hw/lm32/milkymist.c              |  2 +-
 hw/m68k/an5206.c                 |  2 +-
 hw/m68k/mcf5208.c                |  2 +-
 hw/microblaze/boot.c             |  2 +-
 hw/mips/mips_fulong2e.c          |  2 +-
 hw/mips/mips_malta.c             |  2 +-
 hw/mips/mips_mipssim.c           |  2 +-
 hw/mips/mips_r4k.c               |  2 +-
 hw/moxie/moxiesim.c              |  2 +-
 hw/nios2/boot.c                  |  2 +-
 hw/openrisc/openrisc_sim.c       |  2 +-
 hw/pci-host/prep.c               |  2 +-
 hw/ppc/e500.c                    |  2 +-
 hw/ppc/mac_newworld.c            |  2 +-
 hw/ppc/mac_oldworld.c            |  2 +-
 hw/ppc/ppc440_bamboo.c           |  2 +-
 hw/ppc/prep.c                    |  2 +-
 hw/ppc/sam460ex.c                |  2 +-
 hw/ppc/spapr.c                   |  2 +-
 hw/ppc/spapr_vio.c               |  2 +-
 hw/ppc/virtex_ml507.c            |  2 +-
 hw/riscv/boot.c                  |  2 +-
 hw/s390x/ipl.c                   |  2 +-
 hw/sparc/leon3.c                 |  2 +-
 hw/sparc/sun4m.c                 |  2 +-
 hw/sparc64/sun4u.c               |  2 +-
 hw/tricore/tricore_testboard.c   |  2 +-
 hw/xtensa/sim.c                  |  2 +-
 hw/xtensa/xtfpga.c               |  2 +-
 include/elf/elf-types.inc.h      | 63 ++++++++++++++++++++++++++++++++
 include/{ => elf}/elf.h          | 42 ---------------------
 include/hw/core/generic-loader.h |  2 +-
 linux-user/arm/cpu_loop.c        |  2 +-
 linux-user/elfload.c             |  5 +--
 linux-user/main.c                |  2 +-
 linux-user/mips/cpu_loop.c       |  2 +-
 linux-user/riscv/cpu_loop.c      |  2 +-
 target/arm/arch_dump.c           |  2 +-
 target/i386/arch_dump.c          |  2 +-
 target/ppc/arch_dump.c           |  2 +-
 target/ppc/kvm.c                 |  2 +-
 target/s390x/arch_dump.c         |  2 +-
 tcg/arm/tcg-target.inc.c         |  2 +-
 tcg/ppc/tcg-target.inc.c         |  2 +-
 tcg/s390/tcg-target.inc.c        |  2 +-
 tcg/tcg.c                        |  5 ++-
 util/getauxval.c                 |  2 +-
 62 files changed, 128 insertions(+), 104 deletions(-)
 create mode 100644 include/elf/elf-types.inc.h
 rename include/{ => elf}/elf.h (98%)

diff --git a/bsd-user/elfload.c b/bsd-user/elfload.c
index 32378af7b2e..321ee98b86b 100644
--- a/bsd-user/elfload.c
+++ b/bsd-user/elfload.c
@@ -509,7 +509,7 @@ static inline void init_thread(struct target_pt_regs *regs, struct image_info *i
 #define bswaptls(ptr) bswap32s(ptr)
 #endif
 
-#include "elf.h"
+#include "elf/elf.h"
 
 struct exec
 {
diff --git a/contrib/elf2dmp/qemu_elf.h b/contrib/elf2dmp/qemu_elf.h
index b2f0d9cbc9b..060d148d7f0 100644
--- a/contrib/elf2dmp/qemu_elf.h
+++ b/contrib/elf2dmp/qemu_elf.h
@@ -7,7 +7,7 @@
 #ifndef ELF2DMP_QEMU_ELF_H
 #define ELF2DMP_QEMU_ELF_H
 
-#include "elf.h"
+#include "elf/elf.h"
 
 typedef struct QEMUCPUSegment {
     uint32_t selector;
diff --git a/disas.c b/disas.c
index 3e2bfa572b1..6f2370cfda7 100644
--- a/disas.c
+++ b/disas.c
@@ -1,7 +1,7 @@
 /* General "disassemble this chunk" code.  Used for debugging. */
 #include "qemu/osdep.h"
 #include "disas/dis-asm.h"
-#include "elf.h"
+#include "elf/elf.h"
 #include "qemu/qemu-print.h"
 
 #include "cpu.h"
diff --git a/dump/dump.c b/dump/dump.c
index 6fb6e1245ad..6b084a21a2a 100644
--- a/dump/dump.c
+++ b/dump/dump.c
@@ -14,7 +14,7 @@
 #include "qemu/osdep.h"
 #include "qemu-common.h"
 #include "qemu/cutils.h"
-#include "elf.h"
+#include "elf/elf.h"
 #include "cpu.h"
 #include "exec/hwaddr.h"
 #include "monitor/monitor.h"
diff --git a/dump/win_dump.c b/dump/win_dump.c
index eda2a489742..8232c3cb6b3 100644
--- a/dump/win_dump.c
+++ b/dump/win_dump.c
@@ -11,7 +11,7 @@
 #include "qemu/osdep.h"
 #include "qemu-common.h"
 #include "qemu/cutils.h"
-#include "elf.h"
+#include "elf/elf.h"
 #include "cpu.h"
 #include "exec/hwaddr.h"
 #include "monitor/monitor.h"
diff --git a/hw/alpha/dp264.c b/hw/alpha/dp264.c
index 51feee85581..87e5c77c69c 100644
--- a/hw/alpha/dp264.c
+++ b/hw/alpha/dp264.c
@@ -9,7 +9,7 @@
 #include "qemu/osdep.h"
 #include "qemu-common.h"
 #include "cpu.h"
-#include "elf.h"
+#include "elf/elf.h"
 #include "hw/loader.h"
 #include "alpha_sys.h"
 #include "qemu/error-report.h"
diff --git a/hw/arm/armv7m.c b/hw/arm/armv7m.c
index 7a3c48f0026..559586fa527 100644
--- a/hw/arm/armv7m.c
+++ b/hw/arm/armv7m.c
@@ -15,7 +15,7 @@
 #include "hw/arm/boot.h"
 #include "hw/loader.h"
 #include "hw/qdev-properties.h"
-#include "elf.h"
+#include "elf/elf.h"
 #include "sysemu/qtest.h"
 #include "sysemu/reset.h"
 #include "qemu/error-report.h"
diff --git a/hw/arm/boot.c b/hw/arm/boot.c
index bf97ef3e339..7818e5b5518 100644
--- a/hw/arm/boot.c
+++ b/hw/arm/boot.c
@@ -20,7 +20,7 @@
 #include "hw/boards.h"
 #include "sysemu/reset.h"
 #include "hw/loader.h"
-#include "elf.h"
+#include "elf/elf.h"
 #include "sysemu/device_tree.h"
 #include "qemu/config-file.h"
 #include "qemu/option.h"
diff --git a/hw/core/loader.c b/hw/core/loader.c
index 32f7cc7c33c..e0c6563e643 100644
--- a/hw/core/loader.c
+++ b/hw/core/loader.c
@@ -59,6 +59,7 @@
 #include "hw/boards.h"
 #include "qemu/cutils.h"
 #include "sysemu/runstate.h"
+#include "elf/elf.h"
 
 #include <zlib.h>
 
@@ -295,7 +296,7 @@ static void *load_at(int fd, off_t offset, size_t size)
 #endif
 
 #define ELF_CLASS   ELFCLASS32
-#include "elf.h"
+#include "elf/elf-types.inc.h"
 
 #define SZ		32
 #define elf_word        uint32_t
diff --git a/hw/cris/axis_dev88.c b/hw/cris/axis_dev88.c
index 940c7dd1226..31dc391a637 100644
--- a/hw/cris/axis_dev88.c
+++ b/hw/cris/axis_dev88.c
@@ -32,7 +32,7 @@
 #include "hw/boards.h"
 #include "hw/cris/etraxfs.h"
 #include "hw/loader.h"
-#include "elf.h"
+#include "elf/elf.h"
 #include "boot.h"
 #include "exec/address-spaces.h"
 #include "sysemu/qtest.h"
diff --git a/hw/cris/boot.c b/hw/cris/boot.c
index 2d2cc0c7a53..0b8008ca0b2 100644
--- a/hw/cris/boot.c
+++ b/hw/cris/boot.c
@@ -25,7 +25,7 @@
 #include "qemu/osdep.h"
 #include "cpu.h"
 #include "hw/loader.h"
-#include "elf.h"
+#include "elf/elf.h"
 #include "boot.h"
 #include "qemu/cutils.h"
 #include "sysemu/reset.h"
diff --git a/hw/hppa/machine.c b/hw/hppa/machine.c
index 2736ce835ee..3c121f1a645 100644
--- a/hw/hppa/machine.c
+++ b/hw/hppa/machine.c
@@ -6,7 +6,7 @@
 #include "qemu/osdep.h"
 #include "qemu-common.h"
 #include "cpu.h"
-#include "elf.h"
+#include "elf/elf.h"
 #include "hw/loader.h"
 #include "hw/boards.h"
 #include "qemu/error-report.h"
diff --git a/hw/i386/multiboot.c b/hw/i386/multiboot.c
index 9a59f954972..54a7e5f048e 100644
--- a/hw/i386/multiboot.c
+++ b/hw/i386/multiboot.c
@@ -28,7 +28,7 @@
 #include "hw/nvram/fw_cfg.h"
 #include "multiboot.h"
 #include "hw/loader.h"
-#include "elf.h"
+#include "elf/elf.h"
 #include "sysemu/sysemu.h"
 #include "qemu/error-report.h"
 
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index bad866fe44f..e84710a944a 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -39,7 +39,7 @@
 #include "hw/timer/hpet.h"
 #include "hw/firmware/smbios.h"
 #include "hw/loader.h"
-#include "elf.h"
+#include "elf/elf.h"
 #include "migration/vmstate.h"
 #include "multiboot.h"
 #include "hw/timer/mc146818rtc.h"
diff --git a/hw/lm32/lm32_boards.c b/hw/lm32/lm32_boards.c
index 5ae308bfcfb..12c60ad9b74 100644
--- a/hw/lm32/lm32_boards.c
+++ b/hw/lm32/lm32_boards.c
@@ -26,7 +26,7 @@
 #include "hw/block/flash.h"
 #include "hw/boards.h"
 #include "hw/loader.h"
-#include "elf.h"
+#include "elf/elf.h"
 #include "lm32_hwsetup.h"
 #include "lm32.h"
 #include "exec/address-spaces.h"
diff --git a/hw/lm32/milkymist.c b/hw/lm32/milkymist.c
index 460d322de57..9f3a2f2ff5f 100644
--- a/hw/lm32/milkymist.c
+++ b/hw/lm32/milkymist.c
@@ -31,7 +31,7 @@
 #include "hw/boards.h"
 #include "hw/loader.h"
 #include "hw/qdev-properties.h"
-#include "elf.h"
+#include "elf/elf.h"
 #include "milkymist-hw.h"
 #include "hw/display/milkymist_tmu2.h"
 #include "lm32.h"
diff --git a/hw/m68k/an5206.c b/hw/m68k/an5206.c
index 54ccbe1a822..12664b872bc 100644
--- a/hw/m68k/an5206.c
+++ b/hw/m68k/an5206.c
@@ -12,7 +12,7 @@
 #include "hw/m68k/mcf.h"
 #include "hw/boards.h"
 #include "hw/loader.h"
-#include "elf.h"
+#include "elf/elf.h"
 #include "exec/address-spaces.h"
 #include "qemu/error-report.h"
 #include "sysemu/qtest.h"
diff --git a/hw/m68k/mcf5208.c b/hw/m68k/mcf5208.c
index 012710d057d..42d79bd2f03 100644
--- a/hw/m68k/mcf5208.c
+++ b/hw/m68k/mcf5208.c
@@ -25,7 +25,7 @@
 #include "hw/boards.h"
 #include "hw/loader.h"
 #include "hw/sysbus.h"
-#include "elf.h"
+#include "elf/elf.h"
 #include "exec/address-spaces.h"
 
 #define SYS_FREQ 166666666
diff --git a/hw/microblaze/boot.c b/hw/microblaze/boot.c
index bade4d22c00..0c1020cd373 100644
--- a/hw/microblaze/boot.c
+++ b/hw/microblaze/boot.c
@@ -34,7 +34,7 @@
 #include "sysemu/reset.h"
 #include "sysemu/sysemu.h"
 #include "hw/loader.h"
-#include "elf.h"
+#include "elf/elf.h"
 #include "qemu/cutils.h"
 
 #include "boot.h"
diff --git a/hw/mips/mips_fulong2e.c b/hw/mips/mips_fulong2e.c
index cf537dd7e63..4ba670cc909 100644
--- a/hw/mips/mips_fulong2e.c
+++ b/hw/mips/mips_fulong2e.c
@@ -37,7 +37,7 @@
 #include "qemu/log.h"
 #include "hw/loader.h"
 #include "hw/ide.h"
-#include "elf.h"
+#include "elf/elf.h"
 #include "hw/isa/vt82c686.h"
 #include "hw/timer/mc146818rtc.h"
 #include "hw/timer/i8254.h"
diff --git a/hw/mips/mips_malta.c b/hw/mips/mips_malta.c
index 4d9c64b36ab..1c841298363 100644
--- a/hw/mips/mips_malta.c
+++ b/hw/mips/mips_malta.c
@@ -44,7 +44,7 @@
 #include "hw/ide.h"
 #include "hw/irq.h"
 #include "hw/loader.h"
-#include "elf.h"
+#include "elf/elf.h"
 #include "hw/timer/mc146818rtc.h"
 #include "hw/timer/i8254.h"
 #include "exec/address-spaces.h"
diff --git a/hw/mips/mips_mipssim.c b/hw/mips/mips_mipssim.c
index 282bbecb24e..d6acd53b3e3 100644
--- a/hw/mips/mips_mipssim.c
+++ b/hw/mips/mips_mipssim.c
@@ -38,7 +38,7 @@
 #include "hw/boards.h"
 #include "hw/mips/bios.h"
 #include "hw/loader.h"
-#include "elf.h"
+#include "elf/elf.h"
 #include "hw/sysbus.h"
 #include "exec/address-spaces.h"
 #include "qemu/error-report.h"
diff --git a/hw/mips/mips_r4k.c b/hw/mips/mips_r4k.c
index bc0be265441..7fc2fc51fee 100644
--- a/hw/mips/mips_r4k.c
+++ b/hw/mips/mips_r4k.c
@@ -27,7 +27,7 @@
 #include "hw/mips/bios.h"
 #include "hw/ide.h"
 #include "hw/loader.h"
-#include "elf.h"
+#include "elf/elf.h"
 #include "hw/timer/mc146818rtc.h"
 #include "hw/input/i8042.h"
 #include "hw/timer/i8254.h"
diff --git a/hw/moxie/moxiesim.c b/hw/moxie/moxiesim.c
index 57af1b48912..bc1cc8b0bf8 100644
--- a/hw/moxie/moxiesim.c
+++ b/hw/moxie/moxiesim.c
@@ -37,7 +37,7 @@
 #include "hw/loader.h"
 #include "hw/char/serial.h"
 #include "exec/address-spaces.h"
-#include "elf.h"
+#include "elf/elf.h"
 
 #define PHYS_MEM_BASE 0x80000000
 #define FIRMWARE_BASE 0x1000
diff --git a/hw/nios2/boot.c b/hw/nios2/boot.c
index d78bc9ed0e2..ec1eb15bcaa 100644
--- a/hw/nios2/boot.c
+++ b/hw/nios2/boot.c
@@ -39,7 +39,7 @@
 #include "sysemu/reset.h"
 #include "sysemu/sysemu.h"
 #include "hw/loader.h"
-#include "elf.h"
+#include "elf/elf.h"
 
 #include "boot.h"
 
diff --git a/hw/openrisc/openrisc_sim.c b/hw/openrisc/openrisc_sim.c
index 79e70493fc7..f21a2962a90 100644
--- a/hw/openrisc/openrisc_sim.c
+++ b/hw/openrisc/openrisc_sim.c
@@ -24,7 +24,7 @@
 #include "cpu.h"
 #include "hw/irq.h"
 #include "hw/boards.h"
-#include "elf.h"
+#include "elf/elf.h"
 #include "hw/char/serial.h"
 #include "net/net.h"
 #include "hw/loader.h"
diff --git a/hw/pci-host/prep.c b/hw/pci-host/prep.c
index 85d7ba90374..9568746d8e2 100644
--- a/hw/pci-host/prep.c
+++ b/hw/pci-host/prep.c
@@ -37,7 +37,7 @@
 #include "hw/loader.h"
 #include "hw/or-irq.h"
 #include "exec/address-spaces.h"
-#include "elf.h"
+#include "elf/elf.h"
 
 #define TYPE_RAVEN_PCI_DEVICE "raven"
 #define TYPE_RAVEN_PCI_HOST_BRIDGE "raven-pcihost"
diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c
index 91cd4c26f91..add6277ad6c 100644
--- a/hw/ppc/e500.c
+++ b/hw/ppc/e500.c
@@ -36,7 +36,7 @@
 #include "hw/ppc/ppc.h"
 #include "hw/qdev-properties.h"
 #include "hw/loader.h"
-#include "elf.h"
+#include "elf/elf.h"
 #include "hw/sysbus.h"
 #include "exec/address-spaces.h"
 #include "qemu/host-utils.h"
diff --git a/hw/ppc/mac_newworld.c b/hw/ppc/mac_newworld.c
index c5bbcc74335..50babbb7a67 100644
--- a/hw/ppc/mac_newworld.c
+++ b/hw/ppc/mac_newworld.c
@@ -65,7 +65,7 @@
 #include "hw/ide.h"
 #include "hw/loader.h"
 #include "hw/fw-path-provider.h"
-#include "elf.h"
+#include "elf/elf.h"
 #include "qemu/error-report.h"
 #include "sysemu/kvm.h"
 #include "sysemu/reset.h"
diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c
index 0fa680b7499..64fe33cd1f3 100644
--- a/hw/ppc/mac_oldworld.c
+++ b/hw/ppc/mac_oldworld.c
@@ -44,7 +44,7 @@
 #include "hw/ide.h"
 #include "hw/loader.h"
 #include "hw/fw-path-provider.h"
-#include "elf.h"
+#include "elf/elf.h"
 #include "qemu/error-report.h"
 #include "sysemu/kvm.h"
 #include "sysemu/reset.h"
diff --git a/hw/ppc/ppc440_bamboo.c b/hw/ppc/ppc440_bamboo.c
index 4d95c0f8a88..15f39b332a8 100644
--- a/hw/ppc/ppc440_bamboo.c
+++ b/hw/ppc/ppc440_bamboo.c
@@ -23,7 +23,7 @@
 #include "kvm_ppc.h"
 #include "sysemu/device_tree.h"
 #include "hw/loader.h"
-#include "elf.h"
+#include "elf/elf.h"
 #include "exec/address-spaces.h"
 #include "hw/char/serial.h"
 #include "hw/ppc/ppc.h"
diff --git a/hw/ppc/prep.c b/hw/ppc/prep.c
index 4f3c6bf1901..8bd3209dec7 100644
--- a/hw/ppc/prep.c
+++ b/hw/ppc/prep.c
@@ -49,7 +49,7 @@
 #include "sysemu/reset.h"
 #include "exec/address-spaces.h"
 #include "trace.h"
-#include "elf.h"
+#include "elf/elf.h"
 #include "qemu/units.h"
 #include "kvm_ppc.h"
 
diff --git a/hw/ppc/sam460ex.c b/hw/ppc/sam460ex.c
index 437e214210b..982b80e5bad 100644
--- a/hw/ppc/sam460ex.c
+++ b/hw/ppc/sam460ex.c
@@ -22,7 +22,7 @@
 #include "sysemu/device_tree.h"
 #include "sysemu/block-backend.h"
 #include "hw/loader.h"
-#include "elf.h"
+#include "elf/elf.h"
 #include "exec/address-spaces.h"
 #include "exec/memory.h"
 #include "ppc440.h"
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 222a325056c..57f8041ec81 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -36,7 +36,7 @@
 #include "sysemu/runstate.h"
 #include "qemu/log.h"
 #include "hw/fw-path-provider.h"
-#include "elf.h"
+#include "elf/elf.h"
 #include "net/net.h"
 #include "sysemu/device_tree.h"
 #include "sysemu/cpus.h"
diff --git a/hw/ppc/spapr_vio.c b/hw/ppc/spapr_vio.c
index 0803649658f..6d90322db0b 100644
--- a/hw/ppc/spapr_vio.c
+++ b/hw/ppc/spapr_vio.c
@@ -26,7 +26,7 @@
 #include "hw/irq.h"
 #include "qemu/log.h"
 #include "hw/loader.h"
-#include "elf.h"
+#include "elf/elf.h"
 #include "hw/sysbus.h"
 #include "sysemu/kvm.h"
 #include "sysemu/device_tree.h"
diff --git a/hw/ppc/virtex_ml507.c b/hw/ppc/virtex_ml507.c
index 68625522d8a..ede7da4bbc2 100644
--- a/hw/ppc/virtex_ml507.c
+++ b/hw/ppc/virtex_ml507.c
@@ -35,7 +35,7 @@
 #include "hw/boards.h"
 #include "sysemu/device_tree.h"
 #include "hw/loader.h"
-#include "elf.h"
+#include "elf/elf.h"
 #include "qemu/error-report.h"
 #include "qemu/log.h"
 #include "qemu/option.h"
diff --git a/hw/riscv/boot.c b/hw/riscv/boot.c
index 4c63b5cf8a8..9a7b4a5ca87 100644
--- a/hw/riscv/boot.c
+++ b/hw/riscv/boot.c
@@ -25,7 +25,7 @@
 #include "hw/boards.h"
 #include "hw/loader.h"
 #include "hw/riscv/boot.h"
-#include "elf.h"
+#include "elf/elf.h"
 #include "sysemu/qtest.h"
 
 #if defined(TARGET_RISCV32)
diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c
index ca544d64c5e..cf4e06b633e 100644
--- a/hw/s390x/ipl.c
+++ b/hw/s390x/ipl.c
@@ -19,7 +19,7 @@
 #include "sysemu/sysemu.h"
 #include "sysemu/tcg.h"
 #include "cpu.h"
-#include "elf.h"
+#include "elf/elf.h"
 #include "hw/loader.h"
 #include "hw/qdev-properties.h"
 #include "hw/boards.h"
diff --git a/hw/sparc/leon3.c b/hw/sparc/leon3.c
index c5f1b1ee72e..735a823fd56 100644
--- a/hw/sparc/leon3.c
+++ b/hw/sparc/leon3.c
@@ -37,7 +37,7 @@
 #include "sysemu/reset.h"
 #include "hw/boards.h"
 #include "hw/loader.h"
-#include "elf.h"
+#include "elf/elf.h"
 #include "trace.h"
 #include "exec/address-spaces.h"
 
diff --git a/hw/sparc/sun4m.c b/hw/sparc/sun4m.c
index 6c5a17a0205..d542a6c203c 100644
--- a/hw/sparc/sun4m.c
+++ b/hw/sparc/sun4m.c
@@ -49,7 +49,7 @@
 #include "hw/empty_slot.h"
 #include "hw/irq.h"
 #include "hw/loader.h"
-#include "elf.h"
+#include "elf/elf.h"
 #include "trace.h"
 
 /*
diff --git a/hw/sparc64/sun4u.c b/hw/sparc64/sun4u.c
index 1ded2a4c9ab..79b15c8aec3 100644
--- a/hw/sparc64/sun4u.c
+++ b/hw/sparc64/sun4u.c
@@ -54,7 +54,7 @@
 #include "hw/ide/pci.h"
 #include "hw/loader.h"
 #include "hw/fw-path-provider.h"
-#include "elf.h"
+#include "elf/elf.h"
 #include "trace.h"
 
 #define KERNEL_LOAD_ADDR     0x00404000
diff --git a/hw/tricore/tricore_testboard.c b/hw/tricore/tricore_testboard.c
index aef3289f8c3..84e8d7b429f 100644
--- a/hw/tricore/tricore_testboard.c
+++ b/hw/tricore/tricore_testboard.c
@@ -26,7 +26,7 @@
 #include "hw/boards.h"
 #include "hw/loader.h"
 #include "exec/address-spaces.h"
-#include "elf.h"
+#include "elf/elf.h"
 #include "hw/tricore/tricore.h"
 #include "qemu/error-report.h"
 
diff --git a/hw/xtensa/sim.c b/hw/xtensa/sim.c
index 981dbb7bbeb..f8d96fc452c 100644
--- a/hw/xtensa/sim.c
+++ b/hw/xtensa/sim.c
@@ -32,7 +32,7 @@
 #include "sysemu/sysemu.h"
 #include "hw/boards.h"
 #include "hw/loader.h"
-#include "elf.h"
+#include "elf/elf.h"
 #include "exec/memory.h"
 #include "exec/address-spaces.h"
 #include "qemu/error-report.h"
diff --git a/hw/xtensa/xtfpga.c b/hw/xtensa/xtfpga.c
index 8220c7a3794..5e3ed738fa8 100644
--- a/hw/xtensa/xtfpga.c
+++ b/hw/xtensa/xtfpga.c
@@ -33,7 +33,7 @@
 #include "hw/boards.h"
 #include "hw/loader.h"
 #include "hw/qdev-properties.h"
-#include "elf.h"
+#include "elf/elf.h"
 #include "exec/memory.h"
 #include "exec/address-spaces.h"
 #include "hw/char/serial.h"
diff --git a/include/elf/elf-types.inc.h b/include/elf/elf-types.inc.h
new file mode 100644
index 00000000000..35163adb2b5
--- /dev/null
+++ b/include/elf/elf-types.inc.h
@@ -0,0 +1,63 @@
+/*
+ * Elf Type Specialisation
+ *
+ * Copyright (c) 2019
+ * Written by Alex Bennée <alex.bennee@linaro.org>
+ *
+ * This code is licensed under the GNU .
+ */
+
+#ifndef _ELF_TYPES_INC_H_
+#define _ELF_TYPES_INC_H_
+
+#ifndef ELF_CLASS
+#error you must define ELF_CLASS before including elf-types.inc.h
+#else
+
+#if ELF_CLASS == ELFCLASS32
+
+#define elfhdr      elf32_hdr
+#define elf_phdr    elf32_phdr
+#define elf_note    elf32_note
+#define elf_shdr    elf32_shdr
+#define elf_sym     elf32_sym
+#define elf_addr_t  Elf32_Off
+#define elf_rela    elf32_rela
+
+#ifdef ELF_USES_RELOCA
+# define ELF_RELOC  Elf32_Rela
+#else
+# define ELF_RELOC  Elf32_Rel
+#endif
+
+#ifndef ElfW
+#  define ElfW(x)   Elf32_ ## x
+#  define ELFW(x)   ELF32_ ## x
+#endif
+
+#else /* ELF_CLASS == ELFCLASS64 */
+
+#define elfhdr      elf64_hdr
+#define elf_phdr    elf64_phdr
+#define elf_note    elf64_note
+#define elf_shdr    elf64_shdr
+#define elf_sym     elf64_sym
+#define elf_addr_t  Elf64_Off
+#define elf_rela    elf64_rela
+
+#ifdef ELF_USES_RELOCA
+# define ELF_RELOC  Elf64_Rela
+#else
+# define ELF_RELOC  Elf64_Rel
+#endif
+
+#ifndef ElfW
+#  define ElfW(x)   Elf64_ ## x
+#  define ELFW(x)   ELF64_ ## x
+#endif
+
+#endif /* ELF_CLASS == ELFCLASS64 */
+#endif /* ELF_CLASS */
+#else
+#error elf-types.inc.h should not be included twice in one compilation unit
+#endif /* _ELF_TYPES_INC_H_ */
diff --git a/include/elf.h b/include/elf/elf.h
similarity index 98%
rename from include/elf.h
rename to include/elf/elf.h
index 3501e0c8d03..2e264c1a7a0 100644
--- a/include/elf.h
+++ b/include/elf/elf.h
@@ -1696,49 +1696,7 @@ struct elf32_fdpic_loadmap {
 };
 
 #ifdef ELF_CLASS
-#if ELF_CLASS == ELFCLASS32
-
-#define elfhdr		elf32_hdr
-#define elf_phdr	elf32_phdr
-#define elf_note	elf32_note
-#define elf_shdr	elf32_shdr
-#define elf_sym		elf32_sym
-#define elf_addr_t	Elf32_Off
-#define elf_rela  elf32_rela
-
-#ifdef ELF_USES_RELOCA
-# define ELF_RELOC      Elf32_Rela
-#else
-# define ELF_RELOC      Elf32_Rel
-#endif
-
-#else
-
-#define elfhdr		elf64_hdr
-#define elf_phdr	elf64_phdr
-#define elf_note	elf64_note
-#define elf_shdr	elf64_shdr
-#define elf_sym		elf64_sym
-#define elf_addr_t	Elf64_Off
-#define elf_rela  elf64_rela
-
-#ifdef ELF_USES_RELOCA
-# define ELF_RELOC      Elf64_Rela
-#else
-# define ELF_RELOC      Elf64_Rel
-#endif
-
-#endif /* ELF_CLASS */
 
-#ifndef ElfW
-# if ELF_CLASS == ELFCLASS32
-#  define ElfW(x)  Elf32_ ## x
-#  define ELFW(x)  ELF32_ ## x
-# else
-#  define ElfW(x)  Elf64_ ## x
-#  define ELFW(x)  ELF64_ ## x
-# endif
-#endif
 
 #endif /* ELF_CLASS */
 
diff --git a/include/hw/core/generic-loader.h b/include/hw/core/generic-loader.h
index 9ffce1c5a30..ca97affd8e1 100644
--- a/include/hw/core/generic-loader.h
+++ b/include/hw/core/generic-loader.h
@@ -18,7 +18,7 @@
 #ifndef GENERIC_LOADER_H
 #define GENERIC_LOADER_H
 
-#include "elf.h"
+#include "elf/elf.h"
 #include "hw/qdev-core.h"
 
 typedef struct GenericLoaderState {
diff --git a/linux-user/arm/cpu_loop.c b/linux-user/arm/cpu_loop.c
index 8d65de5b9f4..970fff8b1bc 100644
--- a/linux-user/arm/cpu_loop.c
+++ b/linux-user/arm/cpu_loop.c
@@ -20,7 +20,7 @@
 #include "qemu/osdep.h"
 #include "qemu-common.h"
 #include "qemu.h"
-#include "elf.h"
+#include "elf/elf.h"
 #include "cpu_loop-common.h"
 
 #define get_user_code_u32(x, gaddr, env)                \
diff --git a/linux-user/elfload.c b/linux-user/elfload.c
index 3365e192eb3..59a0d21c6f1 100644
--- a/linux-user/elfload.c
+++ b/linux-user/elfload.c
@@ -7,6 +7,7 @@
 
 #include "qemu.h"
 #include "disas/disas.h"
+#include "elf/elf.h"
 #include "qemu/path.h"
 #include "qemu/queue.h"
 #include "qemu/guest-random.h"
@@ -1317,8 +1318,6 @@ static inline void init_thread(struct target_pt_regs *regs,
 #define ELF_DATA	ELFDATA2MSB
 #define ELF_ARCH	EM_S390
 
-#include "elf.h"
-
 #define ELF_HWCAP get_elf_hwcap()
 
 #define GET_FEATURE(_feat, _hwcap) \
@@ -1512,7 +1511,7 @@ static void elf_core_copy_regs(target_elf_gregset_t *regs,
 #define bswaptls(ptr) bswap32s(ptr)
 #endif
 
-#include "elf.h"
+#include "elf/elf-types.inc.h"
 
 struct exec
 {
diff --git a/linux-user/main.c b/linux-user/main.c
index 47917bbb20f..c796a15700d 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -40,7 +40,7 @@
 #include "qemu/timer.h"
 #include "qemu/envlist.h"
 #include "qemu/guest-random.h"
-#include "elf.h"
+#include "elf/elf.h"
 #include "trace/control.h"
 #include "target_elf.h"
 #include "cpu_loop-common.h"
diff --git a/linux-user/mips/cpu_loop.c b/linux-user/mips/cpu_loop.c
index 39915b3fde2..0d3f0738b58 100644
--- a/linux-user/mips/cpu_loop.c
+++ b/linux-user/mips/cpu_loop.c
@@ -21,7 +21,7 @@
 #include "qemu-common.h"
 #include "qemu.h"
 #include "cpu_loop-common.h"
-#include "elf.h"
+#include "elf/elf.h"
 #include "internal.h"
 
 # ifdef TARGET_ABI_MIPSO32
diff --git a/linux-user/riscv/cpu_loop.c b/linux-user/riscv/cpu_loop.c
index 12aa3c0f16e..f9f5beef431 100644
--- a/linux-user/riscv/cpu_loop.c
+++ b/linux-user/riscv/cpu_loop.c
@@ -22,7 +22,7 @@
 #include "qemu/error-report.h"
 #include "qemu.h"
 #include "cpu_loop-common.h"
-#include "elf.h"
+#include "elf/elf.h"
 
 void cpu_loop(CPURISCVState *env)
 {
diff --git a/target/arm/arch_dump.c b/target/arm/arch_dump.c
index 26a2c098687..c05a2845883 100644
--- a/target/arm/arch_dump.c
+++ b/target/arm/arch_dump.c
@@ -20,7 +20,7 @@
 
 #include "qemu/osdep.h"
 #include "cpu.h"
-#include "elf.h"
+#include "elf/elf.h"
 #include "sysemu/dump.h"
 
 /* struct user_pt_regs from arch/arm64/include/uapi/asm/ptrace.h */
diff --git a/target/i386/arch_dump.c b/target/i386/arch_dump.c
index 004141fc042..9eb1e2a8bcf 100644
--- a/target/i386/arch_dump.c
+++ b/target/i386/arch_dump.c
@@ -14,7 +14,7 @@
 #include "qemu/osdep.h"
 #include "cpu.h"
 #include "sysemu/dump.h"
-#include "elf.h"
+#include "elf/elf.h"
 #include "sysemu/memory_mapping.h"
 
 #define ELF_NOTE_SIZE(hdr_size, name_size, desc_size)   \
diff --git a/target/ppc/arch_dump.c b/target/ppc/arch_dump.c
index 9ab04b2c38f..0e102be1ed5 100644
--- a/target/ppc/arch_dump.c
+++ b/target/ppc/arch_dump.c
@@ -14,7 +14,7 @@
 
 #include "qemu/osdep.h"
 #include "cpu.h"
-#include "elf.h"
+#include "elf/elf.h"
 #include "sysemu/dump.h"
 #include "sysemu/kvm.h"
 #include "exec/helper-proto.h"
diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
index 8c5b1f25cc9..c2e5f72ab0d 100644
--- a/target/ppc/kvm.c
+++ b/target/ppc/kvm.c
@@ -48,7 +48,7 @@
 #include "qemu/cutils.h"
 #include "qemu/main-loop.h"
 #include "qemu/mmap-alloc.h"
-#include "elf.h"
+#include "elf/elf.h"
 #include "sysemu/kvm_int.h"
 
 #define PROC_DEVTREE_CPU      "/proc/device-tree/cpus/"
diff --git a/target/s390x/arch_dump.c b/target/s390x/arch_dump.c
index 50fa0ae4b67..5fe9519d26c 100644
--- a/target/s390x/arch_dump.c
+++ b/target/s390x/arch_dump.c
@@ -14,7 +14,7 @@
 #include "qemu/osdep.h"
 #include "cpu.h"
 #include "internal.h"
-#include "elf.h"
+#include "elf/elf.h"
 #include "sysemu/dump.h"
 
 
diff --git a/tcg/arm/tcg-target.inc.c b/tcg/arm/tcg-target.inc.c
index 94d80d79d1f..e04f726e4a6 100644
--- a/tcg/arm/tcg-target.inc.c
+++ b/tcg/arm/tcg-target.inc.c
@@ -22,7 +22,7 @@
  * THE SOFTWARE.
  */
 
-#include "elf.h"
+#include "elf/elf.h"
 #include "tcg-pool.inc.c"
 
 int arm_arch = __ARM_ARCH;
diff --git a/tcg/ppc/tcg-target.inc.c b/tcg/ppc/tcg-target.inc.c
index 815edac077f..4886f8c0d39 100644
--- a/tcg/ppc/tcg-target.inc.c
+++ b/tcg/ppc/tcg-target.inc.c
@@ -22,7 +22,7 @@
  * THE SOFTWARE.
  */
 
-#include "elf.h"
+#include "elf/elf.h"
 #include "tcg-pool.inc.c"
 
 #if defined _CALL_DARWIN || defined __APPLE__
diff --git a/tcg/s390/tcg-target.inc.c b/tcg/s390/tcg-target.inc.c
index 8aaa4cebe8d..82a81d2d94d 100644
--- a/tcg/s390/tcg-target.inc.c
+++ b/tcg/s390/tcg-target.inc.c
@@ -30,7 +30,7 @@
 #endif
 
 #include "tcg-pool.inc.c"
-#include "elf.h"
+#include "elf/elf.h"
 
 /* ??? The translation blocks produced by TCG are generally small enough to
    be entirely reachable with a 16-bit displacement.  Leaving the option for
diff --git a/tcg/tcg.c b/tcg/tcg.c
index 16b2d0e0ece..b8e2c7956b7 100644
--- a/tcg/tcg.c
+++ b/tcg/tcg.c
@@ -50,6 +50,8 @@
 
 #include "tcg-op.h"
 
+#include "elf/elf.h"
+
 #if UINTPTR_MAX == UINT32_MAX
 # define ELF_CLASS  ELFCLASS32
 #else
@@ -61,7 +63,8 @@
 # define ELF_DATA   ELFDATA2LSB
 #endif
 
-#include "elf.h"
+#include "elf/elf-types.inc.h"
+
 #include "exec/log.h"
 #include "sysemu/sysemu.h"
 
diff --git a/util/getauxval.c b/util/getauxval.c
index 36afdfb9e62..ee216c81c0b 100644
--- a/util/getauxval.c
+++ b/util/getauxval.c
@@ -36,7 +36,7 @@ unsigned long qemu_getauxval(unsigned long key)
     return getauxval(key);
 }
 #elif defined(__linux__)
-#include "elf.h"
+#include "elf/elf.h"
 
 /* Our elf.h doesn't contain Elf32_auxv_t and Elf64_auxv_t, which is ok because
    that just makes it easier to define it properly for the host here.  */
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [Qemu-devel] [PATCH v1 3/4] elf: move elf_ops.h into include/elf/ and rename
  2019-09-10 19:34 [Qemu-devel] [PATCH v1 0/4] ELF and (macro) safety Alex Bennée
  2019-09-10 19:34 ` [Qemu-devel] [PATCH v1 1/4] target/ppc: fix signal delivery for ppc64abi32 Alex Bennée
  2019-09-10 19:34 ` [Qemu-devel] [PATCH v1 2/4] elf: move elf.h to elf/elf.h and split out types Alex Bennée
@ 2019-09-10 19:34 ` Alex Bennée
  2019-09-11  8:20   ` Alex Bennée
  2019-10-21 13:56   ` Laurent Vivier
  2019-09-10 19:34 ` [Qemu-devel] [PATCH v1 4/4] elf: move ELF_ARCH definition to elf-arch.h Alex Bennée
  3 siblings, 2 replies; 24+ messages in thread
From: Alex Bennée @ 2019-09-10 19:34 UTC (permalink / raw)
  To: peter.maydell; +Cc: qemu-arm, Alex Bennée, qemu-devel

Lets keep all the Elf manipulation bits together. Also rename the file
to better reflect how it is used and add a little header to the file.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 hw/core/loader.c                            | 4 ++--
 include/{hw/elf_ops.h => elf/elf_ops.inc.h} | 9 +++++++++
 2 files changed, 11 insertions(+), 2 deletions(-)
 rename include/{hw/elf_ops.h => elf/elf_ops.inc.h} (98%)

diff --git a/hw/core/loader.c b/hw/core/loader.c
index e0c6563e643..886179a4947 100644
--- a/hw/core/loader.c
+++ b/hw/core/loader.c
@@ -302,7 +302,7 @@ static void *load_at(int fd, off_t offset, size_t size)
 #define elf_word        uint32_t
 #define elf_sword        int32_t
 #define bswapSZs	bswap32s
-#include "hw/elf_ops.h"
+#include "elf/elf_ops.inc.h"
 
 #undef elfhdr
 #undef elf_phdr
@@ -324,7 +324,7 @@ static void *load_at(int fd, off_t offset, size_t size)
 #define elf_sword        int64_t
 #define bswapSZs	bswap64s
 #define SZ		64
-#include "hw/elf_ops.h"
+#include "elf/elf_ops.inc.h"
 
 const char *load_elf_strerror(int error)
 {
diff --git a/include/hw/elf_ops.h b/include/elf/elf_ops.inc.h
similarity index 98%
rename from include/hw/elf_ops.h
rename to include/elf/elf_ops.inc.h
index 1496d7e7536..a820bd821d5 100644
--- a/include/hw/elf_ops.h
+++ b/include/elf/elf_ops.inc.h
@@ -1,3 +1,12 @@
+/*
+ * Macro expansions for Elf operations. This is included in a
+ * compilation unit with appropriate definitions for SZ and elf
+ * headers to generate utility functions for reading 32 and 64 bit elf
+ * headers.
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
 static void glue(bswap_ehdr, SZ)(struct elfhdr *ehdr)
 {
     bswap16s(&ehdr->e_type);			/* Object file type */
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [Qemu-devel] [PATCH v1 4/4] elf: move ELF_ARCH definition to elf-arch.h
  2019-09-10 19:34 [Qemu-devel] [PATCH v1 0/4] ELF and (macro) safety Alex Bennée
                   ` (2 preceding siblings ...)
  2019-09-10 19:34 ` [Qemu-devel] [PATCH v1 3/4] elf: move elf_ops.h into include/elf/ and rename Alex Bennée
@ 2019-09-10 19:34 ` Alex Bennée
  2019-09-10 21:14   ` Aleksandar Markovic
                     ` (2 more replies)
  3 siblings, 3 replies; 24+ messages in thread
From: Alex Bennée @ 2019-09-10 19:34 UTC (permalink / raw)
  To: peter.maydell
  Cc: Riku Voipio, qemu-arm, Alex Bennée, qemu-devel, Laurent Vivier

This is preparatory for plugins which will want to report the
architecture to plugins. Move the ELF_ARCH definition out of the
loader and into its own header.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 bsd-user/elfload.c     |  13 +----
 include/elf/elf-arch.h | 109 +++++++++++++++++++++++++++++++++++++++++
 linux-user/elfload.c   |  27 ++--------
 3 files changed, 115 insertions(+), 34 deletions(-)
 create mode 100644 include/elf/elf-arch.h

diff --git a/bsd-user/elfload.c b/bsd-user/elfload.c
index 321ee98b86b..adaae0e0dca 100644
--- a/bsd-user/elfload.c
+++ b/bsd-user/elfload.c
@@ -5,6 +5,7 @@
 #include "qemu.h"
 #include "disas/disas.h"
 #include "qemu/path.h"
+#include "elf/elf-arch.h"
 
 #ifdef _ARCH_PPC64
 #undef ARCH_DLINFO
@@ -12,7 +13,6 @@
 #undef ELF_HWCAP
 #undef ELF_CLASS
 #undef ELF_DATA
-#undef ELF_ARCH
 #endif
 
 /* from personality.h */
@@ -115,7 +115,6 @@ static uint32_t get_elf_hwcap(void)
 
 #define ELF_CLASS      ELFCLASS64
 #define ELF_DATA       ELFDATA2LSB
-#define ELF_ARCH       EM_X86_64
 
 static inline void init_thread(struct target_pt_regs *regs, struct image_info *infop)
 {
@@ -141,7 +140,6 @@ static inline void init_thread(struct target_pt_regs *regs, struct image_info *i
  */
 #define ELF_CLASS       ELFCLASS32
 #define ELF_DATA        ELFDATA2LSB
-#define ELF_ARCH        EM_386
 
 static inline void init_thread(struct target_pt_regs *regs, struct image_info *infop)
 {
@@ -176,7 +174,6 @@ static inline void init_thread(struct target_pt_regs *regs, struct image_info *i
 #else
 #define ELF_DATA        ELFDATA2LSB
 #endif
-#define ELF_ARCH        EM_ARM
 
 static inline void init_thread(struct target_pt_regs *regs, struct image_info *infop)
 {
@@ -231,7 +228,6 @@ enum
 
 #define ELF_CLASS   ELFCLASS64
 #define ELF_DATA    ELFDATA2MSB
-#define ELF_ARCH    EM_SPARCV9
 
 #define STACK_BIAS              2047
 
@@ -265,7 +261,6 @@ static inline void init_thread(struct target_pt_regs *regs, struct image_info *i
 
 #define ELF_CLASS   ELFCLASS32
 #define ELF_DATA    ELFDATA2MSB
-#define ELF_ARCH    EM_SPARC
 
 static inline void init_thread(struct target_pt_regs *regs, struct image_info *infop)
 {
@@ -302,7 +297,6 @@ static inline void init_thread(struct target_pt_regs *regs, struct image_info *i
 #else
 #define ELF_DATA        ELFDATA2LSB
 #endif
-#define ELF_ARCH        EM_PPC
 
 /*
  * We need to put in some extra aux table entries to tell glibc what
@@ -388,7 +382,6 @@ static inline void init_thread(struct target_pt_regs *_regs, struct image_info *
 #else
 #define ELF_DATA        ELFDATA2LSB
 #endif
-#define ELF_ARCH    EM_MIPS
 
 static inline void init_thread(struct target_pt_regs *regs, struct image_info *infop)
 {
@@ -410,7 +403,6 @@ static inline void init_thread(struct target_pt_regs *regs, struct image_info *i
 
 #define ELF_CLASS ELFCLASS32
 #define ELF_DATA  ELFDATA2LSB
-#define ELF_ARCH  EM_SH
 
 static inline void init_thread(struct target_pt_regs *regs, struct image_info *infop)
 {
@@ -432,7 +424,6 @@ static inline void init_thread(struct target_pt_regs *regs, struct image_info *i
 
 #define ELF_CLASS ELFCLASS32
 #define ELF_DATA  ELFDATA2LSB
-#define ELF_ARCH  EM_CRIS
 
 static inline void init_thread(struct target_pt_regs *regs, struct image_info *infop)
 {
@@ -452,7 +443,6 @@ static inline void init_thread(struct target_pt_regs *regs, struct image_info *i
 
 #define ELF_CLASS       ELFCLASS32
 #define ELF_DATA        ELFDATA2MSB
-#define ELF_ARCH        EM_68K
 
 /* ??? Does this need to do anything?
 #define ELF_PLAT_INIT(_r) */
@@ -477,7 +467,6 @@ static inline void init_thread(struct target_pt_regs *regs, struct image_info *i
 
 #define ELF_CLASS      ELFCLASS64
 #define ELF_DATA       ELFDATA2MSB
-#define ELF_ARCH       EM_ALPHA
 
 static inline void init_thread(struct target_pt_regs *regs, struct image_info *infop)
 {
diff --git a/include/elf/elf-arch.h b/include/elf/elf-arch.h
new file mode 100644
index 00000000000..9e052543c51
--- /dev/null
+++ b/include/elf/elf-arch.h
@@ -0,0 +1,109 @@
+/*
+ * Elf Architecture Definition
+ *
+ * This is a simple expansion to define common Elf types for the
+ * various machines for the various places it's needed in the source
+ * tree.
+ *
+ * Copyright (c) 2019 Alex Bennée <alex.bennee@linaro.org>
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#ifndef _ELF_ARCH_H_
+#define _ELF_ARCH_H_
+
+#include "elf/elf.h"
+
+#ifndef NEED_CPU_H
+#error Needs an target definition
+#endif
+
+#ifdef ELF_ARCH
+#error ELF_ARCH should only be defined once in this file
+#endif
+
+#ifdef TARGET_I386
+#ifdef TARGET_X86_64
+#define ELF_ARCH EM_X86_64
+#else
+#define ELF_ARCH EM_386
+#endif
+#endif
+
+#ifdef TARGET_ARM
+#ifndef TARGET_AARCH64
+#define ELF_ARCH EM_ARM
+#else
+#define ELF_ARCH EM_AARCH64
+#endif
+#endif
+
+#ifdef TARGET_SPARC
+#ifdef TARGET_SPARC64
+#define ELF_ARCH EM_SPARCV9
+#else
+#define ELF_ARCH EM_SPARC
+#endif
+#endif
+
+#ifdef TARGET_PPC
+#define ELF_ARCH EM_PPC
+#endif
+
+#ifdef TARGET_MIPS
+#define ELF_ARCH EM_MIPS
+#endif
+
+#ifdef TARGET_MICROBLAZE
+#define ELF_ARCH EM_MICROBLAZE
+#endif
+
+#ifdef TARGET_NIOS2
+#define ELF_ARCH EM_ALTERA_NIOS2
+#endif
+
+#ifdef TARGET_OPENRISC
+#define ELF_ARCH EM_OPENRISC
+#endif
+
+#ifdef TARGET_SH4
+#define ELF_ARCH EM_SH
+#endif
+
+#ifdef TARGET_CRIS
+#define ELF_ARCH EM_CRIS
+#endif
+
+#ifdef TARGET_M68K
+#define ELF_ARCH EM_68K
+#endif
+
+#ifdef TARGET_ALPHA
+#define ELF_ARCH EM_ALPHA
+#endif
+
+#ifdef TARGET_S390X
+#define ELF_ARCH EM_S390
+#endif
+
+#ifdef TARGET_TILEGX
+#define ELF_ARCH EM_TILEGX
+#endif
+
+#ifdef TARGET_RISCV
+#define ELF_ARCH EM_RISCV
+#endif
+
+#ifdef TARGET_HPPA
+#define ELF_ARCH EM_PARISC
+#endif
+
+#ifdef TARGET_XTENSA
+#define ELF_ARCH EM_XTENSA
+#endif
+
+#endif /* _ELF_ARCH_H_ */
diff --git a/linux-user/elfload.c b/linux-user/elfload.c
index 59a0d21c6f1..3ac7016a7e3 100644
--- a/linux-user/elfload.c
+++ b/linux-user/elfload.c
@@ -8,10 +8,15 @@
 #include "qemu.h"
 #include "disas/disas.h"
 #include "elf/elf.h"
+#include "elf/elf-arch.h"
 #include "qemu/path.h"
 #include "qemu/queue.h"
 #include "qemu/guest-random.h"
 
+#ifndef ELF_ARCH
+#error something got missed
+#endif
+
 #ifdef _ARCH_PPC64
 #undef ARCH_DLINFO
 #undef ELF_PLATFORM
@@ -19,7 +24,6 @@
 #undef ELF_HWCAP2
 #undef ELF_CLASS
 #undef ELF_DATA
-#undef ELF_ARCH
 #endif
 
 #define ELF_OSABI   ELFOSABI_SYSV
@@ -148,7 +152,6 @@ static uint32_t get_elf_hwcap(void)
 #define ELF_START_MMAP 0x2aaaaab000ULL
 
 #define ELF_CLASS      ELFCLASS64
-#define ELF_ARCH       EM_X86_64
 
 static inline void init_thread(struct target_pt_regs *regs, struct image_info *infop)
 {
@@ -211,7 +214,6 @@ static void elf_core_copy_regs(target_elf_gregset_t *regs, const CPUX86State *en
  * These are used to set parameters in the core dumps.
  */
 #define ELF_CLASS       ELFCLASS32
-#define ELF_ARCH        EM_386
 
 static inline void init_thread(struct target_pt_regs *regs,
                                struct image_info *infop)
@@ -273,7 +275,6 @@ static void elf_core_copy_regs(target_elf_gregset_t *regs, const CPUX86State *en
 
 #define ELF_START_MMAP 0x80000000
 
-#define ELF_ARCH        EM_ARM
 #define ELF_CLASS       ELFCLASS32
 
 static inline void init_thread(struct target_pt_regs *regs,
@@ -539,7 +540,6 @@ static const char *get_elf_platform(void)
 /* 64 bit ARM definitions */
 #define ELF_START_MMAP 0x80000000
 
-#define ELF_ARCH        EM_AARCH64
 #define ELF_CLASS       ELFCLASS64
 #ifdef TARGET_WORDS_BIGENDIAN
 # define ELF_PLATFORM    "aarch64_be"
@@ -667,7 +667,6 @@ static uint32_t get_elf_hwcap(void)
 #endif
 
 #define ELF_CLASS   ELFCLASS64
-#define ELF_ARCH    EM_SPARCV9
 
 #define STACK_BIAS              2047
 
@@ -696,7 +695,6 @@ static inline void init_thread(struct target_pt_regs *regs,
                     | HWCAP_SPARC_MULDIV)
 
 #define ELF_CLASS   ELFCLASS32
-#define ELF_ARCH    EM_SPARC
 
 static inline void init_thread(struct target_pt_regs *regs,
                                struct image_info *infop)
@@ -728,8 +726,6 @@ static inline void init_thread(struct target_pt_regs *regs,
 
 #endif
 
-#define ELF_ARCH        EM_PPC
-
 /* Feature masks for the Aux Vector Hardware Capabilities (AT_HWCAP).
    See arch/powerpc/include/asm/cputable.h.  */
 enum {
@@ -921,7 +917,6 @@ static void elf_core_copy_regs(target_elf_gregset_t *regs, const CPUPPCState *en
 #else
 #define ELF_CLASS   ELFCLASS32
 #endif
-#define ELF_ARCH    EM_MIPS
 
 #define elf_check_arch(x) ((x) == EM_MIPS || (x) == EM_NANOMIPS)
 
@@ -1014,7 +1009,6 @@ static uint32_t get_elf_hwcap(void)
 #define elf_check_arch(x) ( (x) == EM_MICROBLAZE || (x) == EM_MICROBLAZE_OLD)
 
 #define ELF_CLASS   ELFCLASS32
-#define ELF_ARCH    EM_MICROBLAZE
 
 static inline void init_thread(struct target_pt_regs *regs,
                                struct image_info *infop)
@@ -1053,7 +1047,6 @@ static void elf_core_copy_regs(target_elf_gregset_t *regs, const CPUMBState *env
 #define elf_check_arch(x) ((x) == EM_ALTERA_NIOS2)
 
 #define ELF_CLASS   ELFCLASS32
-#define ELF_ARCH    EM_ALTERA_NIOS2
 
 static void init_thread(struct target_pt_regs *regs, struct image_info *infop)
 {
@@ -1107,7 +1100,6 @@ static void elf_core_copy_regs(target_elf_gregset_t *regs,
 
 #define ELF_START_MMAP 0x08000000
 
-#define ELF_ARCH EM_OPENRISC
 #define ELF_CLASS ELFCLASS32
 #define ELF_DATA  ELFDATA2MSB
 
@@ -1146,7 +1138,6 @@ static void elf_core_copy_regs(target_elf_gregset_t *regs,
 #define ELF_START_MMAP 0x80000000
 
 #define ELF_CLASS ELFCLASS32
-#define ELF_ARCH  EM_SH
 
 static inline void init_thread(struct target_pt_regs *regs,
                                struct image_info *infop)
@@ -1228,7 +1219,6 @@ static uint32_t get_elf_hwcap(void)
 #define ELF_START_MMAP 0x80000000
 
 #define ELF_CLASS ELFCLASS32
-#define ELF_ARCH  EM_CRIS
 
 static inline void init_thread(struct target_pt_regs *regs,
                                struct image_info *infop)
@@ -1245,7 +1235,6 @@ static inline void init_thread(struct target_pt_regs *regs,
 #define ELF_START_MMAP 0x80000000
 
 #define ELF_CLASS       ELFCLASS32
-#define ELF_ARCH        EM_68K
 
 /* ??? Does this need to do anything?
    #define ELF_PLAT_INIT(_r) */
@@ -1296,7 +1285,6 @@ static void elf_core_copy_regs(target_elf_gregset_t *regs, const CPUM68KState *e
 #define ELF_START_MMAP (0x30000000000ULL)
 
 #define ELF_CLASS      ELFCLASS64
-#define ELF_ARCH       EM_ALPHA
 
 static inline void init_thread(struct target_pt_regs *regs,
                                struct image_info *infop)
@@ -1316,7 +1304,6 @@ static inline void init_thread(struct target_pt_regs *regs,
 
 #define ELF_CLASS	ELFCLASS64
 #define ELF_DATA	ELFDATA2MSB
-#define ELF_ARCH	EM_S390
 
 #define ELF_HWCAP get_elf_hwcap()
 
@@ -1362,7 +1349,6 @@ static inline void init_thread(struct target_pt_regs *regs, struct image_info *i
 
 #define ELF_CLASS   ELFCLASS64
 #define ELF_DATA    ELFDATA2LSB
-#define ELF_ARCH    EM_TILEGX
 
 static inline void init_thread(struct target_pt_regs *regs,
                                struct image_info *infop)
@@ -1379,7 +1365,6 @@ static inline void init_thread(struct target_pt_regs *regs,
 #ifdef TARGET_RISCV
 
 #define ELF_START_MMAP 0x80000000
-#define ELF_ARCH  EM_RISCV
 
 #ifdef TARGET_RISCV32
 #define ELF_CLASS ELFCLASS32
@@ -1402,7 +1387,6 @@ static inline void init_thread(struct target_pt_regs *regs,
 
 #define ELF_START_MMAP  0x80000000
 #define ELF_CLASS       ELFCLASS32
-#define ELF_ARCH        EM_PARISC
 #define ELF_PLATFORM    "PARISC"
 #define STACK_GROWS_DOWN 0
 #define STACK_ALIGNMENT  64
@@ -1427,7 +1411,6 @@ static inline void init_thread(struct target_pt_regs *regs,
 #define ELF_START_MMAP 0x20000000
 
 #define ELF_CLASS       ELFCLASS32
-#define ELF_ARCH        EM_XTENSA
 
 static inline void init_thread(struct target_pt_regs *regs,
                                struct image_info *infop)
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 24+ messages in thread

* Re: [Qemu-devel] [PATCH v1 1/4] target/ppc: fix signal delivery for ppc64abi32
  2019-09-10 19:34 ` [Qemu-devel] [PATCH v1 1/4] target/ppc: fix signal delivery for ppc64abi32 Alex Bennée
@ 2019-09-10 19:45   ` Alex Bennée
  0 siblings, 0 replies; 24+ messages in thread
From: Alex Bennée @ 2019-09-10 19:45 UTC (permalink / raw)
  To: peter.maydell; +Cc: Riku Voipio, qemu-arm, qemu-devel, Laurent Vivier


Alex Bennée <alex.bennee@linaro.org> writes:

> We were incorrectly setting NIP resulting in a segfault. This fixes
> linux-test for this ABI.

Oops, that was at the bottom of my tree for fixing ppc64abi32 which
showed up broken when testing/next enabled the TCG tests for it.

>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>  linux-user/ppc/signal.c             | 4 +++-
>  tests/tcg/configure.sh              | 1 +
>  tests/tcg/multiarch/Makefile.target | 5 -----
>  3 files changed, 4 insertions(+), 6 deletions(-)
>
> diff --git a/linux-user/ppc/signal.c b/linux-user/ppc/signal.c
> index 619a56950df..5b82af6cb62 100644
> --- a/linux-user/ppc/signal.c
> +++ b/linux-user/ppc/signal.c
> @@ -501,7 +501,9 @@ void setup_rt_frame(int sig, struct target_sigaction *ka,
>      int i, err = 0;
>  #if defined(TARGET_PPC64)
>      struct target_sigcontext *sc = 0;
> +#if !defined(TARGET_ABI32)
>      struct image_info *image = ((TaskState *)thread_cpu->opaque)->info;
> +#endif
>  #endif
>
>      rt_sf_addr = get_sigframe(ka, env, sizeof(*rt_sf));
> @@ -557,7 +559,7 @@ void setup_rt_frame(int sig, struct target_sigaction *ka,
>      env->gpr[5] = (target_ulong) h2g(&rt_sf->uc);
>      env->gpr[6] = (target_ulong) h2g(rt_sf);
>
> -#if defined(TARGET_PPC64)
> +#if defined(TARGET_PPC64) && !defined(TARGET_ABI32)
>      if (get_ppc64_abi(image) < 2) {
>          /* ELFv1 PPC64 function pointers are pointers to OPD entries. */
>          struct target_func_ptr *handler =
> diff --git a/tests/tcg/configure.sh b/tests/tcg/configure.sh
> index 6c4a471aeae..e8a1a1495fc 100755
> --- a/tests/tcg/configure.sh
> +++ b/tests/tcg/configure.sh
> @@ -54,6 +54,7 @@ fi
>  : ${cross_cc_cflags_ppc="-m32"}
>  : ${cross_cc_ppc64="powerpc-linux-gnu-gcc"}
>  : ${cross_cc_cflags_ppc64="-m64"}
> +: ${cross_cc_cflags_ppc64abi32="-mcpu=power8"}
>  : ${cross_cc_ppc64le="powerpc64le-linux-gnu-gcc"}
>  : ${cross_cc_cflags_s390x="-m64"}
>  : ${cross_cc_cflags_sparc="-m32 -mv8plus -mcpu=ultrasparc"}
> diff --git a/tests/tcg/multiarch/Makefile.target b/tests/tcg/multiarch/Makefile.target
> index 6b1e30e2fec..e6893b2e283 100644
> --- a/tests/tcg/multiarch/Makefile.target
> +++ b/tests/tcg/multiarch/Makefile.target
> @@ -12,11 +12,6 @@ VPATH 		+= $(MULTIARCH_SRC)
>  MULTIARCH_SRCS   =$(notdir $(wildcard $(MULTIARCH_SRC)/*.c))
>  MULTIARCH_TESTS  =$(MULTIARCH_SRCS:.c=)
>
> -# FIXME: ppc64abi32 linux-test seems to have issues but the other basic tests work
> -ifeq ($(TARGET_NAME),ppc64abi32)
> -BROKEN_TESTS = linux-test
> -endif
> -
>  # Update TESTS
>  TESTS		+= $(filter-out $(BROKEN_TESTS), $(MULTIARCH_TESTS))


--
Alex Bennée


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [Qemu-devel] [PATCH v1 4/4] elf: move ELF_ARCH definition to elf-arch.h
  2019-09-10 19:34 ` [Qemu-devel] [PATCH v1 4/4] elf: move ELF_ARCH definition to elf-arch.h Alex Bennée
@ 2019-09-10 21:14   ` Aleksandar Markovic
  2019-09-11  9:26     ` Alex Bennée
  2019-09-10 21:39   ` Aleksandar Markovic
  2019-10-21 14:03   ` Laurent Vivier
  2 siblings, 1 reply; 24+ messages in thread
From: Aleksandar Markovic @ 2019-09-10 21:14 UTC (permalink / raw)
  To: Alex Bennée
  Cc: peter.maydell, Riku Voipio, qemu-devel, qemu-arm, Laurent Vivier

10.09.2019. 21.34, "Alex Bennée" <alex.bennee@linaro.org> је написао/ла:
>
> This is preparatory for plugins which will want to report the
> architecture to plugins. Move the ELF_ARCH definition out of the
> loader and into its own header.
>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> --

Hi, Alex.

I advice some caution here
. For example, EM_NANOMIPS, and some other EM_xxx constants are not
included in the new header. I am not sure what exactly you want to achieve
with this refactoring. But you may miss your goal, since some EM_xxx will
be missing from your new header. Is this the right way to achieve what you
want, and could you unintentionally introduce bugs? Can you outline in more
details your intentions around the new header? Is this refactoring the only
way?

Thanks, Aleksandar

>  bsd-user/elfload.c     |  13 +----
>  include/elf/elf-arch.h | 109 +++++++++++++++++++++++++++++++++++++++++
>  linux-user/elfload.c   |  27 ++--------
>  3 files changed, 115 insertions(+), 34 deletions(-)
>  create mode 100644 include/elf/elf-arch.h
>
> diff --git a/bsd-user/elfload.c b/bsd-user/elfload.c
> index 321ee98b86b..adaae0e0dca 100644
> --- a/bsd-user/elfload.c
> +++ b/bsd-user/elfload.c
> @@ -5,6 +5,7 @@
>  #include "qemu.h"
>  #include "disas/disas.h"
>  #include "qemu/path.h"
> +#include "elf/elf-arch.h"
>
>  #ifdef _ARCH_PPC64
>  #undef ARCH_DLINFO
> @@ -12,7 +13,6 @@
>  #undef ELF_HWCAP
>  #undef ELF_CLASS
>  #undef ELF_DATA
> -#undef ELF_ARCH
>  #endif
>
>  /* from personality.h */
> @@ -115,7 +115,6 @@ static uint32_t get_elf_hwcap(void)
>
>  #define ELF_CLASS      ELFCLASS64
>  #define ELF_DATA       ELFDATA2LSB
> -#define ELF_ARCH       EM_X86_64
>
>  static inline void init_thread(struct target_pt_regs *regs, struct
image_info *infop)
>  {
> @@ -141,7 +140,6 @@ static inline void init_thread(struct target_pt_regs
*regs, struct image_info *i
>   */
>  #define ELF_CLASS       ELFCLASS32
>  #define ELF_DATA        ELFDATA2LSB
> -#define ELF_ARCH        EM_386
>
>  static inline void init_thread(struct target_pt_regs *regs, struct
image_info *infop)
>  {
> @@ -176,7 +174,6 @@ static inline void init_thread(struct target_pt_regs
*regs, struct image_info *i
>  #else
>  #define ELF_DATA        ELFDATA2LSB
>  #endif
> -#define ELF_ARCH        EM_ARM
>
>  static inline void init_thread(struct target_pt_regs *regs, struct
image_info *infop)
>  {
> @@ -231,7 +228,6 @@ enum
>
>  #define ELF_CLASS   ELFCLASS64
>  #define ELF_DATA    ELFDATA2MSB
> -#define ELF_ARCH    EM_SPARCV9
>
>  #define STACK_BIAS              2047
>
> @@ -265,7 +261,6 @@ static inline void init_thread(struct target_pt_regs
*regs, struct image_info *i
>
>  #define ELF_CLASS   ELFCLASS32
>  #define ELF_DATA    ELFDATA2MSB
> -#define ELF_ARCH    EM_SPARC
>
>  static inline void init_thread(struct target_pt_regs *regs, struct
image_info *infop)
>  {
> @@ -302,7 +297,6 @@ static inline void init_thread(struct target_pt_regs
*regs, struct image_info *i
>  #else
>  #define ELF_DATA        ELFDATA2LSB
>  #endif
> -#define ELF_ARCH        EM_PPC
>
>  /*
>   * We need to put in some extra aux table entries to tell glibc what
> @@ -388,7 +382,6 @@ static inline void init_thread(struct target_pt_regs
*_regs, struct image_info *
>  #else
>  #define ELF_DATA        ELFDATA2LSB
>  #endif
> -#define ELF_ARCH    EM_MIPS
>
>  static inline void init_thread(struct target_pt_regs *regs, struct
image_info *infop)
>  {
> @@ -410,7 +403,6 @@ static inline void init_thread(struct target_pt_regs
*regs, struct image_info *i
>
>  #define ELF_CLASS ELFCLASS32
>  #define ELF_DATA  ELFDATA2LSB
> -#define ELF_ARCH  EM_SH
>
>  static inline void init_thread(struct target_pt_regs *regs, struct
image_info *infop)
>  {
> @@ -432,7 +424,6 @@ static inline void init_thread(struct target_pt_regs
*regs, struct image_info *i
>
>  #define ELF_CLASS ELFCLASS32
>  #define ELF_DATA  ELFDATA2LSB
> -#define ELF_ARCH  EM_CRIS
>
>  static inline void init_thread(struct target_pt_regs *regs, struct
image_info *infop)
>  {
> @@ -452,7 +443,6 @@ static inline void init_thread(struct target_pt_regs
*regs, struct image_info *i
>
>  #define ELF_CLASS       ELFCLASS32
>  #define ELF_DATA        ELFDATA2MSB
> -#define ELF_ARCH        EM_68K
>
>  /* ??? Does this need to do anything?
>  #define ELF_PLAT_INIT(_r) */
> @@ -477,7 +467,6 @@ static inline void init_thread(struct target_pt_regs
*regs, struct image_info *i
>
>  #define ELF_CLASS      ELFCLASS64
>  #define ELF_DATA       ELFDATA2MSB
> -#define ELF_ARCH       EM_ALPHA
>
>  static inline void init_thread(struct target_pt_regs *regs, struct
image_info *infop)
>  {
> diff --git a/include/elf/elf-arch.h b/include/elf/elf-arch.h
> new file mode 100644
> index 00000000000..9e052543c51
> --- /dev/null
> +++ b/include/elf/elf-arch.h
> @@ -0,0 +1,109 @@
> +/*
> + * Elf Architecture Definition
> + *
> + * This is a simple expansion to define common Elf types for the
> + * various machines for the various places it's needed in the source
> + * tree.
> + *
> + * Copyright (c) 2019 Alex Bennée <alex.bennee@linaro.org>
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or
later.
> + * See the COPYING file in the top-level directory.
> + */
> +
> +#ifndef _ELF_ARCH_H_
> +#define _ELF_ARCH_H_
> +
> +#include "elf/elf.h"
> +
> +#ifndef NEED_CPU_H
> +#error Needs an target definition
> +#endif
> +
> +#ifdef ELF_ARCH
> +#error ELF_ARCH should only be defined once in this file
> +#endif
> +
> +#ifdef TARGET_I386
> +#ifdef TARGET_X86_64
> +#define ELF_ARCH EM_X86_64
> +#else
> +#define ELF_ARCH EM_386
> +#endif
> +#endif
> +
> +#ifdef TARGET_ARM
> +#ifndef TARGET_AARCH64
> +#define ELF_ARCH EM_ARM
> +#else
> +#define ELF_ARCH EM_AARCH64
> +#endif
> +#endif
> +
> +#ifdef TARGET_SPARC
> +#ifdef TARGET_SPARC64
> +#define ELF_ARCH EM_SPARCV9
> +#else
> +#define ELF_ARCH EM_SPARC
> +#endif
> +#endif
> +
> +#ifdef TARGET_PPC
> +#define ELF_ARCH EM_PPC
> +#endif
> +
> +#ifdef TARGET_MIPS
> +#define ELF_ARCH EM_MIPS
> +#endif
> +
> +#ifdef TARGET_MICROBLAZE
> +#define ELF_ARCH EM_MICROBLAZE
> +#endif
> +
> +#ifdef TARGET_NIOS2
> +#define ELF_ARCH EM_ALTERA_NIOS2
> +#endif
> +
> +#ifdef TARGET_OPENRISC
> +#define ELF_ARCH EM_OPENRISC
> +#endif
> +
> +#ifdef TARGET_SH4
> +#define ELF_ARCH EM_SH
> +#endif
> +
> +#ifdef TARGET_CRIS
> +#define ELF_ARCH EM_CRIS
> +#endif
> +
> +#ifdef TARGET_M68K
> +#define ELF_ARCH EM_68K
> +#endif
> +
> +#ifdef TARGET_ALPHA
> +#define ELF_ARCH EM_ALPHA
> +#endif
> +
> +#ifdef TARGET_S390X
> +#define ELF_ARCH EM_S390
> +#endif
> +
> +#ifdef TARGET_TILEGX
> +#define ELF_ARCH EM_TILEGX
> +#endif
> +
> +#ifdef TARGET_RISCV
> +#define ELF_ARCH EM_RISCV
> +#endif
> +
> +#ifdef TARGET_HPPA
> +#define ELF_ARCH EM_PARISC
> +#endif
> +
> +#ifdef TARGET_XTENSA
> +#define ELF_ARCH EM_XTENSA
> +#endif
> +
> +#endif /* _ELF_ARCH_H_ */
> diff --git a/linux-user/elfload.c b/linux-user/elfload.c
> index 59a0d21c6f1..3ac7016a7e3 100644
> --- a/linux-user/elfload.c
> +++ b/linux-user/elfload.c
> @@ -8,10 +8,15 @@
>  #include "qemu.h"
>  #include "disas/disas.h"
>  #include "elf/elf.h"
> +#include "elf/elf-arch.h"
>  #include "qemu/path.h"
>  #include "qemu/queue.h"
>  #include "qemu/guest-random.h"
>
> +#ifndef ELF_ARCH
> +#error something got missed
> +#endif
> +
>  #ifdef _ARCH_PPC64
>  #undef ARCH_DLINFO
>  #undef ELF_PLATFORM
> @@ -19,7 +24,6 @@
>  #undef ELF_HWCAP2
>  #undef ELF_CLASS
>  #undef ELF_DATA
> -#undef ELF_ARCH
>  #endif
>
>  #define ELF_OSABI   ELFOSABI_SYSV
> @@ -148,7 +152,6 @@ static uint32_t get_elf_hwcap(void)
>  #define ELF_START_MMAP 0x2aaaaab000ULL
>
>  #define ELF_CLASS      ELFCLASS64
> -#define ELF_ARCH       EM_X86_64
>
>  static inline void init_thread(struct target_pt_regs *regs, struct
image_info *infop)
>  {
> @@ -211,7 +214,6 @@ static void elf_core_copy_regs(target_elf_gregset_t
*regs, const CPUX86State *en
>   * These are used to set parameters in the core dumps.
>   */
>  #define ELF_CLASS       ELFCLASS32
> -#define ELF_ARCH        EM_386
>
>  static inline void init_thread(struct target_pt_regs *regs,
>                                 struct image_info *infop)
> @@ -273,7 +275,6 @@ static void elf_core_copy_regs(target_elf_gregset_t
*regs, const CPUX86State *en
>
>  #define ELF_START_MMAP 0x80000000
>
> -#define ELF_ARCH        EM_ARM
>  #define ELF_CLASS       ELFCLASS32
>
>  static inline void init_thread(struct target_pt_regs *regs,
> @@ -539,7 +540,6 @@ static const char *get_elf_platform(void)
>  /* 64 bit ARM definitions */
>  #define ELF_START_MMAP 0x80000000
>
> -#define ELF_ARCH        EM_AARCH64
>  #define ELF_CLASS       ELFCLASS64
>  #ifdef TARGET_WORDS_BIGENDIAN
>  # define ELF_PLATFORM    "aarch64_be"
> @@ -667,7 +667,6 @@ static uint32_t get_elf_hwcap(void)
>  #endif
>
>  #define ELF_CLASS   ELFCLASS64
> -#define ELF_ARCH    EM_SPARCV9
>
>  #define STACK_BIAS              2047
>
> @@ -696,7 +695,6 @@ static inline void init_thread(struct target_pt_regs
*regs,
>                      | HWCAP_SPARC_MULDIV)
>
>  #define ELF_CLASS   ELFCLASS32
> -#define ELF_ARCH    EM_SPARC
>
>  static inline void init_thread(struct target_pt_regs *regs,
>                                 struct image_info *infop)
> @@ -728,8 +726,6 @@ static inline void init_thread(struct target_pt_regs
*regs,
>
>  #endif
>
> -#define ELF_ARCH        EM_PPC
> -
>  /* Feature masks for the Aux Vector Hardware Capabilities (AT_HWCAP).
>     See arch/powerpc/include/asm/cputable.h.  */
>  enum {
> @@ -921,7 +917,6 @@ static void elf_core_copy_regs(target_elf_gregset_t
*regs, const CPUPPCState *en
>  #else
>  #define ELF_CLASS   ELFCLASS32
>  #endif
> -#define ELF_ARCH    EM_MIPS
>
>  #define elf_check_arch(x) ((x) == EM_MIPS || (x) == EM_NANOMIPS)
>
> @@ -1014,7 +1009,6 @@ static uint32_t get_elf_hwcap(void)
>  #define elf_check_arch(x) ( (x) == EM_MICROBLAZE || (x) ==
EM_MICROBLAZE_OLD)
>
>  #define ELF_CLASS   ELFCLASS32
> -#define ELF_ARCH    EM_MICROBLAZE
>
>  static inline void init_thread(struct target_pt_regs *regs,
>                                 struct image_info *infop)
> @@ -1053,7 +1047,6 @@ static void elf_core_copy_regs(target_elf_gregset_t
*regs, const CPUMBState *env
>  #define elf_check_arch(x) ((x) == EM_ALTERA_NIOS2)
>
>  #define ELF_CLASS   ELFCLASS32
> -#define ELF_ARCH    EM_ALTERA_NIOS2
>
>  static void init_thread(struct target_pt_regs *regs, struct image_info
*infop)
>  {
> @@ -1107,7 +1100,6 @@ static void elf_core_copy_regs(target_elf_gregset_t
*regs,
>
>  #define ELF_START_MMAP 0x08000000
>
> -#define ELF_ARCH EM_OPENRISC
>  #define ELF_CLASS ELFCLASS32
>  #define ELF_DATA  ELFDATA2MSB
>
> @@ -1146,7 +1138,6 @@ static void elf_core_copy_regs(target_elf_gregset_t
*regs,
>  #define ELF_START_MMAP 0x80000000
>
>  #define ELF_CLASS ELFCLASS32
> -#define ELF_ARCH  EM_SH
>
>  static inline void init_thread(struct target_pt_regs *regs,
>                                 struct image_info *infop)
> @@ -1228,7 +1219,6 @@ static uint32_t get_elf_hwcap(void)
>  #define ELF_START_MMAP 0x80000000
>
>  #define ELF_CLASS ELFCLASS32
> -#define ELF_ARCH  EM_CRIS
>
>  static inline void init_thread(struct target_pt_regs *regs,
>                                 struct image_info *infop)
> @@ -1245,7 +1235,6 @@ static inline void init_thread(struct
target_pt_regs *regs,
>  #define ELF_START_MMAP 0x80000000
>
>  #define ELF_CLASS       ELFCLASS32
> -#define ELF_ARCH        EM_68K
>
>  /* ??? Does this need to do anything?
>     #define ELF_PLAT_INIT(_r) */
> @@ -1296,7 +1285,6 @@ static void elf_core_copy_regs(target_elf_gregset_t
*regs, const CPUM68KState *e
>  #define ELF_START_MMAP (0x30000000000ULL)
>
>  #define ELF_CLASS      ELFCLASS64
> -#define ELF_ARCH       EM_ALPHA
>
>  static inline void init_thread(struct target_pt_regs *regs,
>                                 struct image_info *infop)
> @@ -1316,7 +1304,6 @@ static inline void init_thread(struct
target_pt_regs *regs,
>
>  #define ELF_CLASS      ELFCLASS64
>  #define ELF_DATA       ELFDATA2MSB
> -#define ELF_ARCH       EM_S390
>
>  #define ELF_HWCAP get_elf_hwcap()
>
> @@ -1362,7 +1349,6 @@ static inline void init_thread(struct
target_pt_regs *regs, struct image_info *i
>
>  #define ELF_CLASS   ELFCLASS64
>  #define ELF_DATA    ELFDATA2LSB
> -#define ELF_ARCH    EM_TILEGX
>
>  static inline void init_thread(struct target_pt_regs *regs,
>                                 struct image_info *infop)
> @@ -1379,7 +1365,6 @@ static inline void init_thread(struct
target_pt_regs *regs,
>  #ifdef TARGET_RISCV
>
>  #define ELF_START_MMAP 0x80000000
> -#define ELF_ARCH  EM_RISCV
>
>  #ifdef TARGET_RISCV32
>  #define ELF_CLASS ELFCLASS32
> @@ -1402,7 +1387,6 @@ static inline void init_thread(struct
target_pt_regs *regs,
>
>  #define ELF_START_MMAP  0x80000000
>  #define ELF_CLASS       ELFCLASS32
> -#define ELF_ARCH        EM_PARISC
>  #define ELF_PLATFORM    "PARISC"
>  #define STACK_GROWS_DOWN 0
>  #define STACK_ALIGNMENT  64
> @@ -1427,7 +1411,6 @@ static inline void init_thread(struct
target_pt_regs *regs,
>  #define ELF_START_MMAP 0x20000000
>
>  #define ELF_CLASS       ELFCLASS32
> -#define ELF_ARCH        EM_XTENSA
>
>  static inline void init_thread(struct target_pt_regs *regs,
>                                 struct image_info *infop)
> --
> 2.20.1
>
>

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [Qemu-devel] [PATCH v1 4/4] elf: move ELF_ARCH definition to elf-arch.h
  2019-09-10 19:34 ` [Qemu-devel] [PATCH v1 4/4] elf: move ELF_ARCH definition to elf-arch.h Alex Bennée
  2019-09-10 21:14   ` Aleksandar Markovic
@ 2019-09-10 21:39   ` Aleksandar Markovic
  2019-09-11  8:19     ` Alex Bennée
  2019-10-21 14:03   ` Laurent Vivier
  2 siblings, 1 reply; 24+ messages in thread
From: Aleksandar Markovic @ 2019-09-10 21:39 UTC (permalink / raw)
  To: Alex Bennée
  Cc: peter.maydell, Riku Voipio, qemu-devel, qemu-arm, Laurent Vivier

10.09.2019. 21.34, "Alex Bennée" <alex.bennee@linaro.org> је написао/ла:
>
> This is preparatory for plugins which will want to report the
> architecture to plugins. Move the ELF_ARCH definition out of the
> loader and into its own header.
>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---

ELF_ARCH is and has been used exclusively locally within elfload.c, and
some architectures use it in a specific way, which is perfectly legal in
the current code organization, and I have certain reservations about this
attempt to suddenly attach additional responsibility to these constants -
"reporting" to some unspecified plugin. In simpler words, it seems to me
that you are trying to use a thing for something it was not meant to.

Also, it would be better if you cc-ed corresponding architecture
submaintainers.

Yours, Aleksandar

>  bsd-user/elfload.c     |  13 +----
>  include/elf/elf-arch.h | 109 +++++++++++++++++++++++++++++++++++++++++
>  linux-user/elfload.c   |  27 ++--------
>  3 files changed, 115 insertions(+), 34 deletions(-)
>  create mode 100644 include/elf/elf-arch.h
>
> diff --git a/bsd-user/elfload.c b/bsd-user/elfload.c
> index 321ee98b86b..adaae0e0dca 100644
> --- a/bsd-user/elfload.c
> +++ b/bsd-user/elfload.c
> @@ -5,6 +5,7 @@
>  #include "qemu.h"
>  #include "disas/disas.h"
>  #include "qemu/path.h"
> +#include "elf/elf-arch.h"
>
>  #ifdef _ARCH_PPC64
>  #undef ARCH_DLINFO
> @@ -12,7 +13,6 @@
>  #undef ELF_HWCAP
>  #undef ELF_CLASS
>  #undef ELF_DATA
> -#undef ELF_ARCH
>  #endif
>
>  /* from personality.h */
> @@ -115,7 +115,6 @@ static uint32_t get_elf_hwcap(void)
>
>  #define ELF_CLASS      ELFCLASS64
>  #define ELF_DATA       ELFDATA2LSB
> -#define ELF_ARCH       EM_X86_64
>
>  static inline void init_thread(struct target_pt_regs *regs, struct
image_info *infop)
>  {
> @@ -141,7 +140,6 @@ static inline void init_thread(struct target_pt_regs
*regs, struct image_info *i
>   */
>  #define ELF_CLASS       ELFCLASS32
>  #define ELF_DATA        ELFDATA2LSB
> -#define ELF_ARCH        EM_386
>
>  static inline void init_thread(struct target_pt_regs *regs, struct
image_info *infop)
>  {
> @@ -176,7 +174,6 @@ static inline void init_thread(struct target_pt_regs
*regs, struct image_info *i
>  #else
>  #define ELF_DATA        ELFDATA2LSB
>  #endif
> -#define ELF_ARCH        EM_ARM
>
>  static inline void init_thread(struct target_pt_regs *regs, struct
image_info *infop)
>  {
> @@ -231,7 +228,6 @@ enum
>
>  #define ELF_CLASS   ELFCLASS64
>  #define ELF_DATA    ELFDATA2MSB
> -#define ELF_ARCH    EM_SPARCV9
>
>  #define STACK_BIAS              2047
>
> @@ -265,7 +261,6 @@ static inline void init_thread(struct target_pt_regs
*regs, struct image_info *i
>
>  #define ELF_CLASS   ELFCLASS32
>  #define ELF_DATA    ELFDATA2MSB
> -#define ELF_ARCH    EM_SPARC
>
>  static inline void init_thread(struct target_pt_regs *regs, struct
image_info *infop)
>  {
> @@ -302,7 +297,6 @@ static inline void init_thread(struct target_pt_regs
*regs, struct image_info *i
>  #else
>  #define ELF_DATA        ELFDATA2LSB
>  #endif
> -#define ELF_ARCH        EM_PPC
>
>  /*
>   * We need to put in some extra aux table entries to tell glibc what
> @@ -388,7 +382,6 @@ static inline void init_thread(struct target_pt_regs
*_regs, struct image_info *
>  #else
>  #define ELF_DATA        ELFDATA2LSB
>  #endif
> -#define ELF_ARCH    EM_MIPS
>
>  static inline void init_thread(struct target_pt_regs *regs, struct
image_info *infop)
>  {
> @@ -410,7 +403,6 @@ static inline void init_thread(struct target_pt_regs
*regs, struct image_info *i
>
>  #define ELF_CLASS ELFCLASS32
>  #define ELF_DATA  ELFDATA2LSB
> -#define ELF_ARCH  EM_SH
>
>  static inline void init_thread(struct target_pt_regs *regs, struct
image_info *infop)
>  {
> @@ -432,7 +424,6 @@ static inline void init_thread(struct target_pt_regs
*regs, struct image_info *i
>
>  #define ELF_CLASS ELFCLASS32
>  #define ELF_DATA  ELFDATA2LSB
> -#define ELF_ARCH  EM_CRIS
>
>  static inline void init_thread(struct target_pt_regs *regs, struct
image_info *infop)
>  {
> @@ -452,7 +443,6 @@ static inline void init_thread(struct target_pt_regs
*regs, struct image_info *i
>
>  #define ELF_CLASS       ELFCLASS32
>  #define ELF_DATA        ELFDATA2MSB
> -#define ELF_ARCH        EM_68K
>
>  /* ??? Does this need to do anything?
>  #define ELF_PLAT_INIT(_r) */
> @@ -477,7 +467,6 @@ static inline void init_thread(struct target_pt_regs
*regs, struct image_info *i
>
>  #define ELF_CLASS      ELFCLASS64
>  #define ELF_DATA       ELFDATA2MSB
> -#define ELF_ARCH       EM_ALPHA
>
>  static inline void init_thread(struct target_pt_regs *regs, struct
image_info *infop)
>  {
> diff --git a/include/elf/elf-arch.h b/include/elf/elf-arch.h
> new file mode 100644
> index 00000000000..9e052543c51
> --- /dev/null
> +++ b/include/elf/elf-arch.h
> @@ -0,0 +1,109 @@
> +/*
> + * Elf Architecture Definition
> + *
> + * This is a simple expansion to define common Elf types for the
> + * various machines for the various places it's needed in the source
> + * tree.
> + *
> + * Copyright (c) 2019 Alex Bennée <alex.bennee@linaro.org>
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or
later.
> + * See the COPYING file in the top-level directory.
> + */
> +
> +#ifndef _ELF_ARCH_H_
> +#define _ELF_ARCH_H_
> +
> +#include "elf/elf.h"
> +
> +#ifndef NEED_CPU_H
> +#error Needs an target definition
> +#endif
> +
> +#ifdef ELF_ARCH
> +#error ELF_ARCH should only be defined once in this file
> +#endif
> +
> +#ifdef TARGET_I386
> +#ifdef TARGET_X86_64
> +#define ELF_ARCH EM_X86_64
> +#else
> +#define ELF_ARCH EM_386
> +#endif
> +#endif
> +
> +#ifdef TARGET_ARM
> +#ifndef TARGET_AARCH64
> +#define ELF_ARCH EM_ARM
> +#else
> +#define ELF_ARCH EM_AARCH64
> +#endif
> +#endif
> +
> +#ifdef TARGET_SPARC
> +#ifdef TARGET_SPARC64
> +#define ELF_ARCH EM_SPARCV9
> +#else
> +#define ELF_ARCH EM_SPARC
> +#endif
> +#endif
> +
> +#ifdef TARGET_PPC
> +#define ELF_ARCH EM_PPC
> +#endif
> +
> +#ifdef TARGET_MIPS
> +#define ELF_ARCH EM_MIPS
> +#endif
> +
> +#ifdef TARGET_MICROBLAZE
> +#define ELF_ARCH EM_MICROBLAZE
> +#endif
> +
> +#ifdef TARGET_NIOS2
> +#define ELF_ARCH EM_ALTERA_NIOS2
> +#endif
> +
> +#ifdef TARGET_OPENRISC
> +#define ELF_ARCH EM_OPENRISC
> +#endif
> +
> +#ifdef TARGET_SH4
> +#define ELF_ARCH EM_SH
> +#endif
> +
> +#ifdef TARGET_CRIS
> +#define ELF_ARCH EM_CRIS
> +#endif
> +
> +#ifdef TARGET_M68K
> +#define ELF_ARCH EM_68K
> +#endif
> +
> +#ifdef TARGET_ALPHA
> +#define ELF_ARCH EM_ALPHA
> +#endif
> +
> +#ifdef TARGET_S390X
> +#define ELF_ARCH EM_S390
> +#endif
> +
> +#ifdef TARGET_TILEGX
> +#define ELF_ARCH EM_TILEGX
> +#endif
> +
> +#ifdef TARGET_RISCV
> +#define ELF_ARCH EM_RISCV
> +#endif
> +
> +#ifdef TARGET_HPPA
> +#define ELF_ARCH EM_PARISC
> +#endif
> +
> +#ifdef TARGET_XTENSA
> +#define ELF_ARCH EM_XTENSA
> +#endif
> +
> +#endif /* _ELF_ARCH_H_ */
> diff --git a/linux-user/elfload.c b/linux-user/elfload.c
> index 59a0d21c6f1..3ac7016a7e3 100644
> --- a/linux-user/elfload.c
> +++ b/linux-user/elfload.c
> @@ -8,10 +8,15 @@
>  #include "qemu.h"
>  #include "disas/disas.h"
>  #include "elf/elf.h"
> +#include "elf/elf-arch.h"
>  #include "qemu/path.h"
>  #include "qemu/queue.h"
>  #include "qemu/guest-random.h"
>
> +#ifndef ELF_ARCH
> +#error something got missed
> +#endif
> +
>  #ifdef _ARCH_PPC64
>  #undef ARCH_DLINFO
>  #undef ELF_PLATFORM
> @@ -19,7 +24,6 @@
>  #undef ELF_HWCAP2
>  #undef ELF_CLASS
>  #undef ELF_DATA
> -#undef ELF_ARCH
>  #endif
>
>  #define ELF_OSABI   ELFOSABI_SYSV
> @@ -148,7 +152,6 @@ static uint32_t get_elf_hwcap(void)
>  #define ELF_START_MMAP 0x2aaaaab000ULL
>
>  #define ELF_CLASS      ELFCLASS64
> -#define ELF_ARCH       EM_X86_64
>
>  static inline void init_thread(struct target_pt_regs *regs, struct
image_info *infop)
>  {
> @@ -211,7 +214,6 @@ static void elf_core_copy_regs(target_elf_gregset_t
*regs, const CPUX86State *en
>   * These are used to set parameters in the core dumps.
>   */
>  #define ELF_CLASS       ELFCLASS32
> -#define ELF_ARCH        EM_386
>
>  static inline void init_thread(struct target_pt_regs *regs,
>                                 struct image_info *infop)
> @@ -273,7 +275,6 @@ static void elf_core_copy_regs(target_elf_gregset_t
*regs, const CPUX86State *en
>
>  #define ELF_START_MMAP 0x80000000
>
> -#define ELF_ARCH        EM_ARM
>  #define ELF_CLASS       ELFCLASS32
>
>  static inline void init_thread(struct target_pt_regs *regs,
> @@ -539,7 +540,6 @@ static const char *get_elf_platform(void)
>  /* 64 bit ARM definitions */
>  #define ELF_START_MMAP 0x80000000
>
> -#define ELF_ARCH        EM_AARCH64
>  #define ELF_CLASS       ELFCLASS64
>  #ifdef TARGET_WORDS_BIGENDIAN
>  # define ELF_PLATFORM    "aarch64_be"
> @@ -667,7 +667,6 @@ static uint32_t get_elf_hwcap(void)
>  #endif
>
>  #define ELF_CLASS   ELFCLASS64
> -#define ELF_ARCH    EM_SPARCV9
>
>  #define STACK_BIAS              2047
>
> @@ -696,7 +695,6 @@ static inline void init_thread(struct target_pt_regs
*regs,
>                      | HWCAP_SPARC_MULDIV)
>
>  #define ELF_CLASS   ELFCLASS32
> -#define ELF_ARCH    EM_SPARC
>
>  static inline void init_thread(struct target_pt_regs *regs,
>                                 struct image_info *infop)
> @@ -728,8 +726,6 @@ static inline void init_thread(struct target_pt_regs
*regs,
>
>  #endif
>
> -#define ELF_ARCH        EM_PPC
> -
>  /* Feature masks for the Aux Vector Hardware Capabilities (AT_HWCAP).
>     See arch/powerpc/include/asm/cputable.h.  */
>  enum {
> @@ -921,7 +917,6 @@ static void elf_core_copy_regs(target_elf_gregset_t
*regs, const CPUPPCState *en
>  #else
>  #define ELF_CLASS   ELFCLASS32
>  #endif
> -#define ELF_ARCH    EM_MIPS
>
>  #define elf_check_arch(x) ((x) == EM_MIPS || (x) == EM_NANOMIPS)
>
> @@ -1014,7 +1009,6 @@ static uint32_t get_elf_hwcap(void)
>  #define elf_check_arch(x) ( (x) == EM_MICROBLAZE || (x) ==
EM_MICROBLAZE_OLD)
>
>  #define ELF_CLASS   ELFCLASS32
> -#define ELF_ARCH    EM_MICROBLAZE
>
>  static inline void init_thread(struct target_pt_regs *regs,
>                                 struct image_info *infop)
> @@ -1053,7 +1047,6 @@ static void elf_core_copy_regs(target_elf_gregset_t
*regs, const CPUMBState *env
>  #define elf_check_arch(x) ((x) == EM_ALTERA_NIOS2)
>
>  #define ELF_CLASS   ELFCLASS32
> -#define ELF_ARCH    EM_ALTERA_NIOS2
>
>  static void init_thread(struct target_pt_regs *regs, struct image_info
*infop)
>  {
> @@ -1107,7 +1100,6 @@ static void elf_core_copy_regs(target_elf_gregset_t
*regs,
>
>  #define ELF_START_MMAP 0x08000000
>
> -#define ELF_ARCH EM_OPENRISC
>  #define ELF_CLASS ELFCLASS32
>  #define ELF_DATA  ELFDATA2MSB
>
> @@ -1146,7 +1138,6 @@ static void elf_core_copy_regs(target_elf_gregset_t
*regs,
>  #define ELF_START_MMAP 0x80000000
>
>  #define ELF_CLASS ELFCLASS32
> -#define ELF_ARCH  EM_SH
>
>  static inline void init_thread(struct target_pt_regs *regs,
>                                 struct image_info *infop)
> @@ -1228,7 +1219,6 @@ static uint32_t get_elf_hwcap(void)
>  #define ELF_START_MMAP 0x80000000
>
>  #define ELF_CLASS ELFCLASS32
> -#define ELF_ARCH  EM_CRIS
>
>  static inline void init_thread(struct target_pt_regs *regs,
>                                 struct image_info *infop)
> @@ -1245,7 +1235,6 @@ static inline void init_thread(struct
target_pt_regs *regs,
>  #define ELF_START_MMAP 0x80000000
>
>  #define ELF_CLASS       ELFCLASS32
> -#define ELF_ARCH        EM_68K
>
>  /* ??? Does this need to do anything?
>     #define ELF_PLAT_INIT(_r) */
> @@ -1296,7 +1285,6 @@ static void elf_core_copy_regs(target_elf_gregset_t
*regs, const CPUM68KState *e
>  #define ELF_START_MMAP (0x30000000000ULL)
>
>  #define ELF_CLASS      ELFCLASS64
> -#define ELF_ARCH       EM_ALPHA
>
>  static inline void init_thread(struct target_pt_regs *regs,
>                                 struct image_info *infop)
> @@ -1316,7 +1304,6 @@ static inline void init_thread(struct
target_pt_regs *regs,
>
>  #define ELF_CLASS      ELFCLASS64
>  #define ELF_DATA       ELFDATA2MSB
> -#define ELF_ARCH       EM_S390
>
>  #define ELF_HWCAP get_elf_hwcap()
>
> @@ -1362,7 +1349,6 @@ static inline void init_thread(struct
target_pt_regs *regs, struct image_info *i
>
>  #define ELF_CLASS   ELFCLASS64
>  #define ELF_DATA    ELFDATA2LSB
> -#define ELF_ARCH    EM_TILEGX
>
>  static inline void init_thread(struct target_pt_regs *regs,
>                                 struct image_info *infop)
> @@ -1379,7 +1365,6 @@ static inline void init_thread(struct
target_pt_regs *regs,
>  #ifdef TARGET_RISCV
>
>  #define ELF_START_MMAP 0x80000000
> -#define ELF_ARCH  EM_RISCV
>
>  #ifdef TARGET_RISCV32
>  #define ELF_CLASS ELFCLASS32
> @@ -1402,7 +1387,6 @@ static inline void init_thread(struct
target_pt_regs *regs,
>
>  #define ELF_START_MMAP  0x80000000
>  #define ELF_CLASS       ELFCLASS32
> -#define ELF_ARCH        EM_PARISC
>  #define ELF_PLATFORM    "PARISC"
>  #define STACK_GROWS_DOWN 0
>  #define STACK_ALIGNMENT  64
> @@ -1427,7 +1411,6 @@ static inline void init_thread(struct
target_pt_regs *regs,
>  #define ELF_START_MMAP 0x20000000
>
>  #define ELF_CLASS       ELFCLASS32
> -#define ELF_ARCH        EM_XTENSA
>
>  static inline void init_thread(struct target_pt_regs *regs,
>                                 struct image_info *infop)
> --
> 2.20.1
>
>

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [Qemu-devel] [PATCH v1 2/4] elf: move elf.h to elf/elf.h and split out types
  2019-09-10 19:34 ` [Qemu-devel] [PATCH v1 2/4] elf: move elf.h to elf/elf.h and split out types Alex Bennée
@ 2019-09-11  0:08   ` David Gibson
  2019-09-11  8:29   ` BALATON Zoltan
                     ` (2 subsequent siblings)
  3 siblings, 0 replies; 24+ messages in thread
From: David Gibson @ 2019-09-11  0:08 UTC (permalink / raw)
  To: Alex Bennée
  Cc: peter.maydell, Cornelia Huck, Sagar Karandikar,
	Michael S. Tsirkin, Anthony Green, Palmer Dabbelt,
	Mark Cave-Ayland, qemu-devel, Max Filippov, KONRAD Frederic,
	Alistair Francis, Edgar E. Iglesias, Marek Vasut, Jia Liu,
	Aleksandar Rikalo, Helge Deller, David Hildenbrand, Halil Pasic,
	Christian Borntraeger, Hervé Poussineau,
	Marc-André Lureau, Artyom Tarasenko, Eduardo Habkost,
	Riku Voipio, Fabien Chouteau, open list:S390-ccw boot, qemu-arm,
	Stafford Horne, Richard Henderson, open list:RISC-V TCG CPUs,
	Viktor Prutyanov, Thomas Huth, Bastian Koppelmann, Chris Wulff,
	Laurent Vivier, Michael Walle, open list:PReP,
	Aleksandar Markovic, Paolo Bonzini, Aurelien Jarno

[-- Attachment #1: Type: text/plain, Size: 31135 bytes --]

On Tue, Sep 10, 2019 at 08:34:06PM +0100, Alex Bennée wrote:
> Most of the users of elf.h just want the standard Elf definitions. The
> couple that want more than that want an expansion based on ELF_CLASS
> which can be used for size agnostic code. The later is moved into
> elf/elf-types.inc.h to make it clearer what it is for. While doing
> that I also removed the whitespace damage.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>

ppc parts
Acked-by: David Gibson <david@gibson.dropbear.id.au>

> ---
>  bsd-user/elfload.c               |  2 +-
>  contrib/elf2dmp/qemu_elf.h       |  2 +-
>  disas.c                          |  2 +-
>  dump/dump.c                      |  2 +-
>  dump/win_dump.c                  |  2 +-
>  hw/alpha/dp264.c                 |  2 +-
>  hw/arm/armv7m.c                  |  2 +-
>  hw/arm/boot.c                    |  2 +-
>  hw/core/loader.c                 |  3 +-
>  hw/cris/axis_dev88.c             |  2 +-
>  hw/cris/boot.c                   |  2 +-
>  hw/hppa/machine.c                |  2 +-
>  hw/i386/multiboot.c              |  2 +-
>  hw/i386/pc.c                     |  2 +-
>  hw/lm32/lm32_boards.c            |  2 +-
>  hw/lm32/milkymist.c              |  2 +-
>  hw/m68k/an5206.c                 |  2 +-
>  hw/m68k/mcf5208.c                |  2 +-
>  hw/microblaze/boot.c             |  2 +-
>  hw/mips/mips_fulong2e.c          |  2 +-
>  hw/mips/mips_malta.c             |  2 +-
>  hw/mips/mips_mipssim.c           |  2 +-
>  hw/mips/mips_r4k.c               |  2 +-
>  hw/moxie/moxiesim.c              |  2 +-
>  hw/nios2/boot.c                  |  2 +-
>  hw/openrisc/openrisc_sim.c       |  2 +-
>  hw/pci-host/prep.c               |  2 +-
>  hw/ppc/e500.c                    |  2 +-
>  hw/ppc/mac_newworld.c            |  2 +-
>  hw/ppc/mac_oldworld.c            |  2 +-
>  hw/ppc/ppc440_bamboo.c           |  2 +-
>  hw/ppc/prep.c                    |  2 +-
>  hw/ppc/sam460ex.c                |  2 +-
>  hw/ppc/spapr.c                   |  2 +-
>  hw/ppc/spapr_vio.c               |  2 +-
>  hw/ppc/virtex_ml507.c            |  2 +-
>  hw/riscv/boot.c                  |  2 +-
>  hw/s390x/ipl.c                   |  2 +-
>  hw/sparc/leon3.c                 |  2 +-
>  hw/sparc/sun4m.c                 |  2 +-
>  hw/sparc64/sun4u.c               |  2 +-
>  hw/tricore/tricore_testboard.c   |  2 +-
>  hw/xtensa/sim.c                  |  2 +-
>  hw/xtensa/xtfpga.c               |  2 +-
>  include/elf/elf-types.inc.h      | 63 ++++++++++++++++++++++++++++++++
>  include/{ => elf}/elf.h          | 42 ---------------------
>  include/hw/core/generic-loader.h |  2 +-
>  linux-user/arm/cpu_loop.c        |  2 +-
>  linux-user/elfload.c             |  5 +--
>  linux-user/main.c                |  2 +-
>  linux-user/mips/cpu_loop.c       |  2 +-
>  linux-user/riscv/cpu_loop.c      |  2 +-
>  target/arm/arch_dump.c           |  2 +-
>  target/i386/arch_dump.c          |  2 +-
>  target/ppc/arch_dump.c           |  2 +-
>  target/ppc/kvm.c                 |  2 +-
>  target/s390x/arch_dump.c         |  2 +-
>  tcg/arm/tcg-target.inc.c         |  2 +-
>  tcg/ppc/tcg-target.inc.c         |  2 +-
>  tcg/s390/tcg-target.inc.c        |  2 +-
>  tcg/tcg.c                        |  5 ++-
>  util/getauxval.c                 |  2 +-
>  62 files changed, 128 insertions(+), 104 deletions(-)
>  create mode 100644 include/elf/elf-types.inc.h
>  rename include/{ => elf}/elf.h (98%)
> 
> diff --git a/bsd-user/elfload.c b/bsd-user/elfload.c
> index 32378af7b2e..321ee98b86b 100644
> --- a/bsd-user/elfload.c
> +++ b/bsd-user/elfload.c
> @@ -509,7 +509,7 @@ static inline void init_thread(struct target_pt_regs *regs, struct image_info *i
>  #define bswaptls(ptr) bswap32s(ptr)
>  #endif
>  
> -#include "elf.h"
> +#include "elf/elf.h"
>  
>  struct exec
>  {
> diff --git a/contrib/elf2dmp/qemu_elf.h b/contrib/elf2dmp/qemu_elf.h
> index b2f0d9cbc9b..060d148d7f0 100644
> --- a/contrib/elf2dmp/qemu_elf.h
> +++ b/contrib/elf2dmp/qemu_elf.h
> @@ -7,7 +7,7 @@
>  #ifndef ELF2DMP_QEMU_ELF_H
>  #define ELF2DMP_QEMU_ELF_H
>  
> -#include "elf.h"
> +#include "elf/elf.h"
>  
>  typedef struct QEMUCPUSegment {
>      uint32_t selector;
> diff --git a/disas.c b/disas.c
> index 3e2bfa572b1..6f2370cfda7 100644
> --- a/disas.c
> +++ b/disas.c
> @@ -1,7 +1,7 @@
>  /* General "disassemble this chunk" code.  Used for debugging. */
>  #include "qemu/osdep.h"
>  #include "disas/dis-asm.h"
> -#include "elf.h"
> +#include "elf/elf.h"
>  #include "qemu/qemu-print.h"
>  
>  #include "cpu.h"
> diff --git a/dump/dump.c b/dump/dump.c
> index 6fb6e1245ad..6b084a21a2a 100644
> --- a/dump/dump.c
> +++ b/dump/dump.c
> @@ -14,7 +14,7 @@
>  #include "qemu/osdep.h"
>  #include "qemu-common.h"
>  #include "qemu/cutils.h"
> -#include "elf.h"
> +#include "elf/elf.h"
>  #include "cpu.h"
>  #include "exec/hwaddr.h"
>  #include "monitor/monitor.h"
> diff --git a/dump/win_dump.c b/dump/win_dump.c
> index eda2a489742..8232c3cb6b3 100644
> --- a/dump/win_dump.c
> +++ b/dump/win_dump.c
> @@ -11,7 +11,7 @@
>  #include "qemu/osdep.h"
>  #include "qemu-common.h"
>  #include "qemu/cutils.h"
> -#include "elf.h"
> +#include "elf/elf.h"
>  #include "cpu.h"
>  #include "exec/hwaddr.h"
>  #include "monitor/monitor.h"
> diff --git a/hw/alpha/dp264.c b/hw/alpha/dp264.c
> index 51feee85581..87e5c77c69c 100644
> --- a/hw/alpha/dp264.c
> +++ b/hw/alpha/dp264.c
> @@ -9,7 +9,7 @@
>  #include "qemu/osdep.h"
>  #include "qemu-common.h"
>  #include "cpu.h"
> -#include "elf.h"
> +#include "elf/elf.h"
>  #include "hw/loader.h"
>  #include "alpha_sys.h"
>  #include "qemu/error-report.h"
> diff --git a/hw/arm/armv7m.c b/hw/arm/armv7m.c
> index 7a3c48f0026..559586fa527 100644
> --- a/hw/arm/armv7m.c
> +++ b/hw/arm/armv7m.c
> @@ -15,7 +15,7 @@
>  #include "hw/arm/boot.h"
>  #include "hw/loader.h"
>  #include "hw/qdev-properties.h"
> -#include "elf.h"
> +#include "elf/elf.h"
>  #include "sysemu/qtest.h"
>  #include "sysemu/reset.h"
>  #include "qemu/error-report.h"
> diff --git a/hw/arm/boot.c b/hw/arm/boot.c
> index bf97ef3e339..7818e5b5518 100644
> --- a/hw/arm/boot.c
> +++ b/hw/arm/boot.c
> @@ -20,7 +20,7 @@
>  #include "hw/boards.h"
>  #include "sysemu/reset.h"
>  #include "hw/loader.h"
> -#include "elf.h"
> +#include "elf/elf.h"
>  #include "sysemu/device_tree.h"
>  #include "qemu/config-file.h"
>  #include "qemu/option.h"
> diff --git a/hw/core/loader.c b/hw/core/loader.c
> index 32f7cc7c33c..e0c6563e643 100644
> --- a/hw/core/loader.c
> +++ b/hw/core/loader.c
> @@ -59,6 +59,7 @@
>  #include "hw/boards.h"
>  #include "qemu/cutils.h"
>  #include "sysemu/runstate.h"
> +#include "elf/elf.h"
>  
>  #include <zlib.h>
>  
> @@ -295,7 +296,7 @@ static void *load_at(int fd, off_t offset, size_t size)
>  #endif
>  
>  #define ELF_CLASS   ELFCLASS32
> -#include "elf.h"
> +#include "elf/elf-types.inc.h"
>  
>  #define SZ		32
>  #define elf_word        uint32_t
> diff --git a/hw/cris/axis_dev88.c b/hw/cris/axis_dev88.c
> index 940c7dd1226..31dc391a637 100644
> --- a/hw/cris/axis_dev88.c
> +++ b/hw/cris/axis_dev88.c
> @@ -32,7 +32,7 @@
>  #include "hw/boards.h"
>  #include "hw/cris/etraxfs.h"
>  #include "hw/loader.h"
> -#include "elf.h"
> +#include "elf/elf.h"
>  #include "boot.h"
>  #include "exec/address-spaces.h"
>  #include "sysemu/qtest.h"
> diff --git a/hw/cris/boot.c b/hw/cris/boot.c
> index 2d2cc0c7a53..0b8008ca0b2 100644
> --- a/hw/cris/boot.c
> +++ b/hw/cris/boot.c
> @@ -25,7 +25,7 @@
>  #include "qemu/osdep.h"
>  #include "cpu.h"
>  #include "hw/loader.h"
> -#include "elf.h"
> +#include "elf/elf.h"
>  #include "boot.h"
>  #include "qemu/cutils.h"
>  #include "sysemu/reset.h"
> diff --git a/hw/hppa/machine.c b/hw/hppa/machine.c
> index 2736ce835ee..3c121f1a645 100644
> --- a/hw/hppa/machine.c
> +++ b/hw/hppa/machine.c
> @@ -6,7 +6,7 @@
>  #include "qemu/osdep.h"
>  #include "qemu-common.h"
>  #include "cpu.h"
> -#include "elf.h"
> +#include "elf/elf.h"
>  #include "hw/loader.h"
>  #include "hw/boards.h"
>  #include "qemu/error-report.h"
> diff --git a/hw/i386/multiboot.c b/hw/i386/multiboot.c
> index 9a59f954972..54a7e5f048e 100644
> --- a/hw/i386/multiboot.c
> +++ b/hw/i386/multiboot.c
> @@ -28,7 +28,7 @@
>  #include "hw/nvram/fw_cfg.h"
>  #include "multiboot.h"
>  #include "hw/loader.h"
> -#include "elf.h"
> +#include "elf/elf.h"
>  #include "sysemu/sysemu.h"
>  #include "qemu/error-report.h"
>  
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index bad866fe44f..e84710a944a 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -39,7 +39,7 @@
>  #include "hw/timer/hpet.h"
>  #include "hw/firmware/smbios.h"
>  #include "hw/loader.h"
> -#include "elf.h"
> +#include "elf/elf.h"
>  #include "migration/vmstate.h"
>  #include "multiboot.h"
>  #include "hw/timer/mc146818rtc.h"
> diff --git a/hw/lm32/lm32_boards.c b/hw/lm32/lm32_boards.c
> index 5ae308bfcfb..12c60ad9b74 100644
> --- a/hw/lm32/lm32_boards.c
> +++ b/hw/lm32/lm32_boards.c
> @@ -26,7 +26,7 @@
>  #include "hw/block/flash.h"
>  #include "hw/boards.h"
>  #include "hw/loader.h"
> -#include "elf.h"
> +#include "elf/elf.h"
>  #include "lm32_hwsetup.h"
>  #include "lm32.h"
>  #include "exec/address-spaces.h"
> diff --git a/hw/lm32/milkymist.c b/hw/lm32/milkymist.c
> index 460d322de57..9f3a2f2ff5f 100644
> --- a/hw/lm32/milkymist.c
> +++ b/hw/lm32/milkymist.c
> @@ -31,7 +31,7 @@
>  #include "hw/boards.h"
>  #include "hw/loader.h"
>  #include "hw/qdev-properties.h"
> -#include "elf.h"
> +#include "elf/elf.h"
>  #include "milkymist-hw.h"
>  #include "hw/display/milkymist_tmu2.h"
>  #include "lm32.h"
> diff --git a/hw/m68k/an5206.c b/hw/m68k/an5206.c
> index 54ccbe1a822..12664b872bc 100644
> --- a/hw/m68k/an5206.c
> +++ b/hw/m68k/an5206.c
> @@ -12,7 +12,7 @@
>  #include "hw/m68k/mcf.h"
>  #include "hw/boards.h"
>  #include "hw/loader.h"
> -#include "elf.h"
> +#include "elf/elf.h"
>  #include "exec/address-spaces.h"
>  #include "qemu/error-report.h"
>  #include "sysemu/qtest.h"
> diff --git a/hw/m68k/mcf5208.c b/hw/m68k/mcf5208.c
> index 012710d057d..42d79bd2f03 100644
> --- a/hw/m68k/mcf5208.c
> +++ b/hw/m68k/mcf5208.c
> @@ -25,7 +25,7 @@
>  #include "hw/boards.h"
>  #include "hw/loader.h"
>  #include "hw/sysbus.h"
> -#include "elf.h"
> +#include "elf/elf.h"
>  #include "exec/address-spaces.h"
>  
>  #define SYS_FREQ 166666666
> diff --git a/hw/microblaze/boot.c b/hw/microblaze/boot.c
> index bade4d22c00..0c1020cd373 100644
> --- a/hw/microblaze/boot.c
> +++ b/hw/microblaze/boot.c
> @@ -34,7 +34,7 @@
>  #include "sysemu/reset.h"
>  #include "sysemu/sysemu.h"
>  #include "hw/loader.h"
> -#include "elf.h"
> +#include "elf/elf.h"
>  #include "qemu/cutils.h"
>  
>  #include "boot.h"
> diff --git a/hw/mips/mips_fulong2e.c b/hw/mips/mips_fulong2e.c
> index cf537dd7e63..4ba670cc909 100644
> --- a/hw/mips/mips_fulong2e.c
> +++ b/hw/mips/mips_fulong2e.c
> @@ -37,7 +37,7 @@
>  #include "qemu/log.h"
>  #include "hw/loader.h"
>  #include "hw/ide.h"
> -#include "elf.h"
> +#include "elf/elf.h"
>  #include "hw/isa/vt82c686.h"
>  #include "hw/timer/mc146818rtc.h"
>  #include "hw/timer/i8254.h"
> diff --git a/hw/mips/mips_malta.c b/hw/mips/mips_malta.c
> index 4d9c64b36ab..1c841298363 100644
> --- a/hw/mips/mips_malta.c
> +++ b/hw/mips/mips_malta.c
> @@ -44,7 +44,7 @@
>  #include "hw/ide.h"
>  #include "hw/irq.h"
>  #include "hw/loader.h"
> -#include "elf.h"
> +#include "elf/elf.h"
>  #include "hw/timer/mc146818rtc.h"
>  #include "hw/timer/i8254.h"
>  #include "exec/address-spaces.h"
> diff --git a/hw/mips/mips_mipssim.c b/hw/mips/mips_mipssim.c
> index 282bbecb24e..d6acd53b3e3 100644
> --- a/hw/mips/mips_mipssim.c
> +++ b/hw/mips/mips_mipssim.c
> @@ -38,7 +38,7 @@
>  #include "hw/boards.h"
>  #include "hw/mips/bios.h"
>  #include "hw/loader.h"
> -#include "elf.h"
> +#include "elf/elf.h"
>  #include "hw/sysbus.h"
>  #include "exec/address-spaces.h"
>  #include "qemu/error-report.h"
> diff --git a/hw/mips/mips_r4k.c b/hw/mips/mips_r4k.c
> index bc0be265441..7fc2fc51fee 100644
> --- a/hw/mips/mips_r4k.c
> +++ b/hw/mips/mips_r4k.c
> @@ -27,7 +27,7 @@
>  #include "hw/mips/bios.h"
>  #include "hw/ide.h"
>  #include "hw/loader.h"
> -#include "elf.h"
> +#include "elf/elf.h"
>  #include "hw/timer/mc146818rtc.h"
>  #include "hw/input/i8042.h"
>  #include "hw/timer/i8254.h"
> diff --git a/hw/moxie/moxiesim.c b/hw/moxie/moxiesim.c
> index 57af1b48912..bc1cc8b0bf8 100644
> --- a/hw/moxie/moxiesim.c
> +++ b/hw/moxie/moxiesim.c
> @@ -37,7 +37,7 @@
>  #include "hw/loader.h"
>  #include "hw/char/serial.h"
>  #include "exec/address-spaces.h"
> -#include "elf.h"
> +#include "elf/elf.h"
>  
>  #define PHYS_MEM_BASE 0x80000000
>  #define FIRMWARE_BASE 0x1000
> diff --git a/hw/nios2/boot.c b/hw/nios2/boot.c
> index d78bc9ed0e2..ec1eb15bcaa 100644
> --- a/hw/nios2/boot.c
> +++ b/hw/nios2/boot.c
> @@ -39,7 +39,7 @@
>  #include "sysemu/reset.h"
>  #include "sysemu/sysemu.h"
>  #include "hw/loader.h"
> -#include "elf.h"
> +#include "elf/elf.h"
>  
>  #include "boot.h"
>  
> diff --git a/hw/openrisc/openrisc_sim.c b/hw/openrisc/openrisc_sim.c
> index 79e70493fc7..f21a2962a90 100644
> --- a/hw/openrisc/openrisc_sim.c
> +++ b/hw/openrisc/openrisc_sim.c
> @@ -24,7 +24,7 @@
>  #include "cpu.h"
>  #include "hw/irq.h"
>  #include "hw/boards.h"
> -#include "elf.h"
> +#include "elf/elf.h"
>  #include "hw/char/serial.h"
>  #include "net/net.h"
>  #include "hw/loader.h"
> diff --git a/hw/pci-host/prep.c b/hw/pci-host/prep.c
> index 85d7ba90374..9568746d8e2 100644
> --- a/hw/pci-host/prep.c
> +++ b/hw/pci-host/prep.c
> @@ -37,7 +37,7 @@
>  #include "hw/loader.h"
>  #include "hw/or-irq.h"
>  #include "exec/address-spaces.h"
> -#include "elf.h"
> +#include "elf/elf.h"
>  
>  #define TYPE_RAVEN_PCI_DEVICE "raven"
>  #define TYPE_RAVEN_PCI_HOST_BRIDGE "raven-pcihost"
> diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c
> index 91cd4c26f91..add6277ad6c 100644
> --- a/hw/ppc/e500.c
> +++ b/hw/ppc/e500.c
> @@ -36,7 +36,7 @@
>  #include "hw/ppc/ppc.h"
>  #include "hw/qdev-properties.h"
>  #include "hw/loader.h"
> -#include "elf.h"
> +#include "elf/elf.h"
>  #include "hw/sysbus.h"
>  #include "exec/address-spaces.h"
>  #include "qemu/host-utils.h"
> diff --git a/hw/ppc/mac_newworld.c b/hw/ppc/mac_newworld.c
> index c5bbcc74335..50babbb7a67 100644
> --- a/hw/ppc/mac_newworld.c
> +++ b/hw/ppc/mac_newworld.c
> @@ -65,7 +65,7 @@
>  #include "hw/ide.h"
>  #include "hw/loader.h"
>  #include "hw/fw-path-provider.h"
> -#include "elf.h"
> +#include "elf/elf.h"
>  #include "qemu/error-report.h"
>  #include "sysemu/kvm.h"
>  #include "sysemu/reset.h"
> diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c
> index 0fa680b7499..64fe33cd1f3 100644
> --- a/hw/ppc/mac_oldworld.c
> +++ b/hw/ppc/mac_oldworld.c
> @@ -44,7 +44,7 @@
>  #include "hw/ide.h"
>  #include "hw/loader.h"
>  #include "hw/fw-path-provider.h"
> -#include "elf.h"
> +#include "elf/elf.h"
>  #include "qemu/error-report.h"
>  #include "sysemu/kvm.h"
>  #include "sysemu/reset.h"
> diff --git a/hw/ppc/ppc440_bamboo.c b/hw/ppc/ppc440_bamboo.c
> index 4d95c0f8a88..15f39b332a8 100644
> --- a/hw/ppc/ppc440_bamboo.c
> +++ b/hw/ppc/ppc440_bamboo.c
> @@ -23,7 +23,7 @@
>  #include "kvm_ppc.h"
>  #include "sysemu/device_tree.h"
>  #include "hw/loader.h"
> -#include "elf.h"
> +#include "elf/elf.h"
>  #include "exec/address-spaces.h"
>  #include "hw/char/serial.h"
>  #include "hw/ppc/ppc.h"
> diff --git a/hw/ppc/prep.c b/hw/ppc/prep.c
> index 4f3c6bf1901..8bd3209dec7 100644
> --- a/hw/ppc/prep.c
> +++ b/hw/ppc/prep.c
> @@ -49,7 +49,7 @@
>  #include "sysemu/reset.h"
>  #include "exec/address-spaces.h"
>  #include "trace.h"
> -#include "elf.h"
> +#include "elf/elf.h"
>  #include "qemu/units.h"
>  #include "kvm_ppc.h"
>  
> diff --git a/hw/ppc/sam460ex.c b/hw/ppc/sam460ex.c
> index 437e214210b..982b80e5bad 100644
> --- a/hw/ppc/sam460ex.c
> +++ b/hw/ppc/sam460ex.c
> @@ -22,7 +22,7 @@
>  #include "sysemu/device_tree.h"
>  #include "sysemu/block-backend.h"
>  #include "hw/loader.h"
> -#include "elf.h"
> +#include "elf/elf.h"
>  #include "exec/address-spaces.h"
>  #include "exec/memory.h"
>  #include "ppc440.h"
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 222a325056c..57f8041ec81 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -36,7 +36,7 @@
>  #include "sysemu/runstate.h"
>  #include "qemu/log.h"
>  #include "hw/fw-path-provider.h"
> -#include "elf.h"
> +#include "elf/elf.h"
>  #include "net/net.h"
>  #include "sysemu/device_tree.h"
>  #include "sysemu/cpus.h"
> diff --git a/hw/ppc/spapr_vio.c b/hw/ppc/spapr_vio.c
> index 0803649658f..6d90322db0b 100644
> --- a/hw/ppc/spapr_vio.c
> +++ b/hw/ppc/spapr_vio.c
> @@ -26,7 +26,7 @@
>  #include "hw/irq.h"
>  #include "qemu/log.h"
>  #include "hw/loader.h"
> -#include "elf.h"
> +#include "elf/elf.h"
>  #include "hw/sysbus.h"
>  #include "sysemu/kvm.h"
>  #include "sysemu/device_tree.h"
> diff --git a/hw/ppc/virtex_ml507.c b/hw/ppc/virtex_ml507.c
> index 68625522d8a..ede7da4bbc2 100644
> --- a/hw/ppc/virtex_ml507.c
> +++ b/hw/ppc/virtex_ml507.c
> @@ -35,7 +35,7 @@
>  #include "hw/boards.h"
>  #include "sysemu/device_tree.h"
>  #include "hw/loader.h"
> -#include "elf.h"
> +#include "elf/elf.h"
>  #include "qemu/error-report.h"
>  #include "qemu/log.h"
>  #include "qemu/option.h"
> diff --git a/hw/riscv/boot.c b/hw/riscv/boot.c
> index 4c63b5cf8a8..9a7b4a5ca87 100644
> --- a/hw/riscv/boot.c
> +++ b/hw/riscv/boot.c
> @@ -25,7 +25,7 @@
>  #include "hw/boards.h"
>  #include "hw/loader.h"
>  #include "hw/riscv/boot.h"
> -#include "elf.h"
> +#include "elf/elf.h"
>  #include "sysemu/qtest.h"
>  
>  #if defined(TARGET_RISCV32)
> diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c
> index ca544d64c5e..cf4e06b633e 100644
> --- a/hw/s390x/ipl.c
> +++ b/hw/s390x/ipl.c
> @@ -19,7 +19,7 @@
>  #include "sysemu/sysemu.h"
>  #include "sysemu/tcg.h"
>  #include "cpu.h"
> -#include "elf.h"
> +#include "elf/elf.h"
>  #include "hw/loader.h"
>  #include "hw/qdev-properties.h"
>  #include "hw/boards.h"
> diff --git a/hw/sparc/leon3.c b/hw/sparc/leon3.c
> index c5f1b1ee72e..735a823fd56 100644
> --- a/hw/sparc/leon3.c
> +++ b/hw/sparc/leon3.c
> @@ -37,7 +37,7 @@
>  #include "sysemu/reset.h"
>  #include "hw/boards.h"
>  #include "hw/loader.h"
> -#include "elf.h"
> +#include "elf/elf.h"
>  #include "trace.h"
>  #include "exec/address-spaces.h"
>  
> diff --git a/hw/sparc/sun4m.c b/hw/sparc/sun4m.c
> index 6c5a17a0205..d542a6c203c 100644
> --- a/hw/sparc/sun4m.c
> +++ b/hw/sparc/sun4m.c
> @@ -49,7 +49,7 @@
>  #include "hw/empty_slot.h"
>  #include "hw/irq.h"
>  #include "hw/loader.h"
> -#include "elf.h"
> +#include "elf/elf.h"
>  #include "trace.h"
>  
>  /*
> diff --git a/hw/sparc64/sun4u.c b/hw/sparc64/sun4u.c
> index 1ded2a4c9ab..79b15c8aec3 100644
> --- a/hw/sparc64/sun4u.c
> +++ b/hw/sparc64/sun4u.c
> @@ -54,7 +54,7 @@
>  #include "hw/ide/pci.h"
>  #include "hw/loader.h"
>  #include "hw/fw-path-provider.h"
> -#include "elf.h"
> +#include "elf/elf.h"
>  #include "trace.h"
>  
>  #define KERNEL_LOAD_ADDR     0x00404000
> diff --git a/hw/tricore/tricore_testboard.c b/hw/tricore/tricore_testboard.c
> index aef3289f8c3..84e8d7b429f 100644
> --- a/hw/tricore/tricore_testboard.c
> +++ b/hw/tricore/tricore_testboard.c
> @@ -26,7 +26,7 @@
>  #include "hw/boards.h"
>  #include "hw/loader.h"
>  #include "exec/address-spaces.h"
> -#include "elf.h"
> +#include "elf/elf.h"
>  #include "hw/tricore/tricore.h"
>  #include "qemu/error-report.h"
>  
> diff --git a/hw/xtensa/sim.c b/hw/xtensa/sim.c
> index 981dbb7bbeb..f8d96fc452c 100644
> --- a/hw/xtensa/sim.c
> +++ b/hw/xtensa/sim.c
> @@ -32,7 +32,7 @@
>  #include "sysemu/sysemu.h"
>  #include "hw/boards.h"
>  #include "hw/loader.h"
> -#include "elf.h"
> +#include "elf/elf.h"
>  #include "exec/memory.h"
>  #include "exec/address-spaces.h"
>  #include "qemu/error-report.h"
> diff --git a/hw/xtensa/xtfpga.c b/hw/xtensa/xtfpga.c
> index 8220c7a3794..5e3ed738fa8 100644
> --- a/hw/xtensa/xtfpga.c
> +++ b/hw/xtensa/xtfpga.c
> @@ -33,7 +33,7 @@
>  #include "hw/boards.h"
>  #include "hw/loader.h"
>  #include "hw/qdev-properties.h"
> -#include "elf.h"
> +#include "elf/elf.h"
>  #include "exec/memory.h"
>  #include "exec/address-spaces.h"
>  #include "hw/char/serial.h"
> diff --git a/include/elf/elf-types.inc.h b/include/elf/elf-types.inc.h
> new file mode 100644
> index 00000000000..35163adb2b5
> --- /dev/null
> +++ b/include/elf/elf-types.inc.h
> @@ -0,0 +1,63 @@
> +/*
> + * Elf Type Specialisation
> + *
> + * Copyright (c) 2019
> + * Written by Alex Bennée <alex.bennee@linaro.org>
> + *
> + * This code is licensed under the GNU .
> + */
> +
> +#ifndef _ELF_TYPES_INC_H_
> +#define _ELF_TYPES_INC_H_
> +
> +#ifndef ELF_CLASS
> +#error you must define ELF_CLASS before including elf-types.inc.h
> +#else
> +
> +#if ELF_CLASS == ELFCLASS32
> +
> +#define elfhdr      elf32_hdr
> +#define elf_phdr    elf32_phdr
> +#define elf_note    elf32_note
> +#define elf_shdr    elf32_shdr
> +#define elf_sym     elf32_sym
> +#define elf_addr_t  Elf32_Off
> +#define elf_rela    elf32_rela
> +
> +#ifdef ELF_USES_RELOCA
> +# define ELF_RELOC  Elf32_Rela
> +#else
> +# define ELF_RELOC  Elf32_Rel
> +#endif
> +
> +#ifndef ElfW
> +#  define ElfW(x)   Elf32_ ## x
> +#  define ELFW(x)   ELF32_ ## x
> +#endif
> +
> +#else /* ELF_CLASS == ELFCLASS64 */
> +
> +#define elfhdr      elf64_hdr
> +#define elf_phdr    elf64_phdr
> +#define elf_note    elf64_note
> +#define elf_shdr    elf64_shdr
> +#define elf_sym     elf64_sym
> +#define elf_addr_t  Elf64_Off
> +#define elf_rela    elf64_rela
> +
> +#ifdef ELF_USES_RELOCA
> +# define ELF_RELOC  Elf64_Rela
> +#else
> +# define ELF_RELOC  Elf64_Rel
> +#endif
> +
> +#ifndef ElfW
> +#  define ElfW(x)   Elf64_ ## x
> +#  define ELFW(x)   ELF64_ ## x
> +#endif
> +
> +#endif /* ELF_CLASS == ELFCLASS64 */
> +#endif /* ELF_CLASS */
> +#else
> +#error elf-types.inc.h should not be included twice in one compilation unit
> +#endif /* _ELF_TYPES_INC_H_ */
> diff --git a/include/elf.h b/include/elf/elf.h
> similarity index 98%
> rename from include/elf.h
> rename to include/elf/elf.h
> index 3501e0c8d03..2e264c1a7a0 100644
> --- a/include/elf.h
> +++ b/include/elf/elf.h
> @@ -1696,49 +1696,7 @@ struct elf32_fdpic_loadmap {
>  };
>  
>  #ifdef ELF_CLASS
> -#if ELF_CLASS == ELFCLASS32
> -
> -#define elfhdr		elf32_hdr
> -#define elf_phdr	elf32_phdr
> -#define elf_note	elf32_note
> -#define elf_shdr	elf32_shdr
> -#define elf_sym		elf32_sym
> -#define elf_addr_t	Elf32_Off
> -#define elf_rela  elf32_rela
> -
> -#ifdef ELF_USES_RELOCA
> -# define ELF_RELOC      Elf32_Rela
> -#else
> -# define ELF_RELOC      Elf32_Rel
> -#endif
> -
> -#else
> -
> -#define elfhdr		elf64_hdr
> -#define elf_phdr	elf64_phdr
> -#define elf_note	elf64_note
> -#define elf_shdr	elf64_shdr
> -#define elf_sym		elf64_sym
> -#define elf_addr_t	Elf64_Off
> -#define elf_rela  elf64_rela
> -
> -#ifdef ELF_USES_RELOCA
> -# define ELF_RELOC      Elf64_Rela
> -#else
> -# define ELF_RELOC      Elf64_Rel
> -#endif
> -
> -#endif /* ELF_CLASS */
>  
> -#ifndef ElfW
> -# if ELF_CLASS == ELFCLASS32
> -#  define ElfW(x)  Elf32_ ## x
> -#  define ELFW(x)  ELF32_ ## x
> -# else
> -#  define ElfW(x)  Elf64_ ## x
> -#  define ELFW(x)  ELF64_ ## x
> -# endif
> -#endif
>  
>  #endif /* ELF_CLASS */
>  
> diff --git a/include/hw/core/generic-loader.h b/include/hw/core/generic-loader.h
> index 9ffce1c5a30..ca97affd8e1 100644
> --- a/include/hw/core/generic-loader.h
> +++ b/include/hw/core/generic-loader.h
> @@ -18,7 +18,7 @@
>  #ifndef GENERIC_LOADER_H
>  #define GENERIC_LOADER_H
>  
> -#include "elf.h"
> +#include "elf/elf.h"
>  #include "hw/qdev-core.h"
>  
>  typedef struct GenericLoaderState {
> diff --git a/linux-user/arm/cpu_loop.c b/linux-user/arm/cpu_loop.c
> index 8d65de5b9f4..970fff8b1bc 100644
> --- a/linux-user/arm/cpu_loop.c
> +++ b/linux-user/arm/cpu_loop.c
> @@ -20,7 +20,7 @@
>  #include "qemu/osdep.h"
>  #include "qemu-common.h"
>  #include "qemu.h"
> -#include "elf.h"
> +#include "elf/elf.h"
>  #include "cpu_loop-common.h"
>  
>  #define get_user_code_u32(x, gaddr, env)                \
> diff --git a/linux-user/elfload.c b/linux-user/elfload.c
> index 3365e192eb3..59a0d21c6f1 100644
> --- a/linux-user/elfload.c
> +++ b/linux-user/elfload.c
> @@ -7,6 +7,7 @@
>  
>  #include "qemu.h"
>  #include "disas/disas.h"
> +#include "elf/elf.h"
>  #include "qemu/path.h"
>  #include "qemu/queue.h"
>  #include "qemu/guest-random.h"
> @@ -1317,8 +1318,6 @@ static inline void init_thread(struct target_pt_regs *regs,
>  #define ELF_DATA	ELFDATA2MSB
>  #define ELF_ARCH	EM_S390
>  
> -#include "elf.h"
> -
>  #define ELF_HWCAP get_elf_hwcap()
>  
>  #define GET_FEATURE(_feat, _hwcap) \
> @@ -1512,7 +1511,7 @@ static void elf_core_copy_regs(target_elf_gregset_t *regs,
>  #define bswaptls(ptr) bswap32s(ptr)
>  #endif
>  
> -#include "elf.h"
> +#include "elf/elf-types.inc.h"
>  
>  struct exec
>  {
> diff --git a/linux-user/main.c b/linux-user/main.c
> index 47917bbb20f..c796a15700d 100644
> --- a/linux-user/main.c
> +++ b/linux-user/main.c
> @@ -40,7 +40,7 @@
>  #include "qemu/timer.h"
>  #include "qemu/envlist.h"
>  #include "qemu/guest-random.h"
> -#include "elf.h"
> +#include "elf/elf.h"
>  #include "trace/control.h"
>  #include "target_elf.h"
>  #include "cpu_loop-common.h"
> diff --git a/linux-user/mips/cpu_loop.c b/linux-user/mips/cpu_loop.c
> index 39915b3fde2..0d3f0738b58 100644
> --- a/linux-user/mips/cpu_loop.c
> +++ b/linux-user/mips/cpu_loop.c
> @@ -21,7 +21,7 @@
>  #include "qemu-common.h"
>  #include "qemu.h"
>  #include "cpu_loop-common.h"
> -#include "elf.h"
> +#include "elf/elf.h"
>  #include "internal.h"
>  
>  # ifdef TARGET_ABI_MIPSO32
> diff --git a/linux-user/riscv/cpu_loop.c b/linux-user/riscv/cpu_loop.c
> index 12aa3c0f16e..f9f5beef431 100644
> --- a/linux-user/riscv/cpu_loop.c
> +++ b/linux-user/riscv/cpu_loop.c
> @@ -22,7 +22,7 @@
>  #include "qemu/error-report.h"
>  #include "qemu.h"
>  #include "cpu_loop-common.h"
> -#include "elf.h"
> +#include "elf/elf.h"
>  
>  void cpu_loop(CPURISCVState *env)
>  {
> diff --git a/target/arm/arch_dump.c b/target/arm/arch_dump.c
> index 26a2c098687..c05a2845883 100644
> --- a/target/arm/arch_dump.c
> +++ b/target/arm/arch_dump.c
> @@ -20,7 +20,7 @@
>  
>  #include "qemu/osdep.h"
>  #include "cpu.h"
> -#include "elf.h"
> +#include "elf/elf.h"
>  #include "sysemu/dump.h"
>  
>  /* struct user_pt_regs from arch/arm64/include/uapi/asm/ptrace.h */
> diff --git a/target/i386/arch_dump.c b/target/i386/arch_dump.c
> index 004141fc042..9eb1e2a8bcf 100644
> --- a/target/i386/arch_dump.c
> +++ b/target/i386/arch_dump.c
> @@ -14,7 +14,7 @@
>  #include "qemu/osdep.h"
>  #include "cpu.h"
>  #include "sysemu/dump.h"
> -#include "elf.h"
> +#include "elf/elf.h"
>  #include "sysemu/memory_mapping.h"
>  
>  #define ELF_NOTE_SIZE(hdr_size, name_size, desc_size)   \
> diff --git a/target/ppc/arch_dump.c b/target/ppc/arch_dump.c
> index 9ab04b2c38f..0e102be1ed5 100644
> --- a/target/ppc/arch_dump.c
> +++ b/target/ppc/arch_dump.c
> @@ -14,7 +14,7 @@
>  
>  #include "qemu/osdep.h"
>  #include "cpu.h"
> -#include "elf.h"
> +#include "elf/elf.h"
>  #include "sysemu/dump.h"
>  #include "sysemu/kvm.h"
>  #include "exec/helper-proto.h"
> diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
> index 8c5b1f25cc9..c2e5f72ab0d 100644
> --- a/target/ppc/kvm.c
> +++ b/target/ppc/kvm.c
> @@ -48,7 +48,7 @@
>  #include "qemu/cutils.h"
>  #include "qemu/main-loop.h"
>  #include "qemu/mmap-alloc.h"
> -#include "elf.h"
> +#include "elf/elf.h"
>  #include "sysemu/kvm_int.h"
>  
>  #define PROC_DEVTREE_CPU      "/proc/device-tree/cpus/"
> diff --git a/target/s390x/arch_dump.c b/target/s390x/arch_dump.c
> index 50fa0ae4b67..5fe9519d26c 100644
> --- a/target/s390x/arch_dump.c
> +++ b/target/s390x/arch_dump.c
> @@ -14,7 +14,7 @@
>  #include "qemu/osdep.h"
>  #include "cpu.h"
>  #include "internal.h"
> -#include "elf.h"
> +#include "elf/elf.h"
>  #include "sysemu/dump.h"
>  
>  
> diff --git a/tcg/arm/tcg-target.inc.c b/tcg/arm/tcg-target.inc.c
> index 94d80d79d1f..e04f726e4a6 100644
> --- a/tcg/arm/tcg-target.inc.c
> +++ b/tcg/arm/tcg-target.inc.c
> @@ -22,7 +22,7 @@
>   * THE SOFTWARE.
>   */
>  
> -#include "elf.h"
> +#include "elf/elf.h"
>  #include "tcg-pool.inc.c"
>  
>  int arm_arch = __ARM_ARCH;
> diff --git a/tcg/ppc/tcg-target.inc.c b/tcg/ppc/tcg-target.inc.c
> index 815edac077f..4886f8c0d39 100644
> --- a/tcg/ppc/tcg-target.inc.c
> +++ b/tcg/ppc/tcg-target.inc.c
> @@ -22,7 +22,7 @@
>   * THE SOFTWARE.
>   */
>  
> -#include "elf.h"
> +#include "elf/elf.h"
>  #include "tcg-pool.inc.c"
>  
>  #if defined _CALL_DARWIN || defined __APPLE__
> diff --git a/tcg/s390/tcg-target.inc.c b/tcg/s390/tcg-target.inc.c
> index 8aaa4cebe8d..82a81d2d94d 100644
> --- a/tcg/s390/tcg-target.inc.c
> +++ b/tcg/s390/tcg-target.inc.c
> @@ -30,7 +30,7 @@
>  #endif
>  
>  #include "tcg-pool.inc.c"
> -#include "elf.h"
> +#include "elf/elf.h"
>  
>  /* ??? The translation blocks produced by TCG are generally small enough to
>     be entirely reachable with a 16-bit displacement.  Leaving the option for
> diff --git a/tcg/tcg.c b/tcg/tcg.c
> index 16b2d0e0ece..b8e2c7956b7 100644
> --- a/tcg/tcg.c
> +++ b/tcg/tcg.c
> @@ -50,6 +50,8 @@
>  
>  #include "tcg-op.h"
>  
> +#include "elf/elf.h"
> +
>  #if UINTPTR_MAX == UINT32_MAX
>  # define ELF_CLASS  ELFCLASS32
>  #else
> @@ -61,7 +63,8 @@
>  # define ELF_DATA   ELFDATA2LSB
>  #endif
>  
> -#include "elf.h"
> +#include "elf/elf-types.inc.h"
> +
>  #include "exec/log.h"
>  #include "sysemu/sysemu.h"
>  
> diff --git a/util/getauxval.c b/util/getauxval.c
> index 36afdfb9e62..ee216c81c0b 100644
> --- a/util/getauxval.c
> +++ b/util/getauxval.c
> @@ -36,7 +36,7 @@ unsigned long qemu_getauxval(unsigned long key)
>      return getauxval(key);
>  }
>  #elif defined(__linux__)
> -#include "elf.h"
> +#include "elf/elf.h"
>  
>  /* Our elf.h doesn't contain Elf32_auxv_t and Elf64_auxv_t, which is ok because
>     that just makes it easier to define it properly for the host here.  */

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [Qemu-devel] [PATCH v1 4/4] elf: move ELF_ARCH definition to elf-arch.h
  2019-09-10 21:39   ` Aleksandar Markovic
@ 2019-09-11  8:19     ` Alex Bennée
  0 siblings, 0 replies; 24+ messages in thread
From: Alex Bennée @ 2019-09-11  8:19 UTC (permalink / raw)
  To: Aleksandar Markovic
  Cc: peter.maydell, Riku Voipio, qemu-devel, qemu-arm, Laurent Vivier


Aleksandar Markovic <aleksandar.m.mail@gmail.com> writes:

> 10.09.2019. 21.34, "Alex Bennée" <alex.bennee@linaro.org> је написао/ла:
>>
>> This is preparatory for plugins which will want to report the
>> architecture to plugins. Move the ELF_ARCH definition out of the
>> loader and into its own header.
>>
>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>> ---
>
> ELF_ARCH is and has been used exclusively locally within elfload.c, and
> some architectures use it in a specific way, which is perfectly legal in
> the current code organization, and I have certain reservations about this
> attempt to suddenly attach additional responsibility to these
> constants -

It is used locally for elfload (and was duplicated at that - as there is
a bsd and linux version). All it really does is translate the Elf
standard code for a guest architecture into a common symbol for the
benefit of common code. There is perhaps an argument this would be
better set in config-target.mak/h as it is something we could even know
at configure time.

> "reporting" to some unspecified plugin. In simpler words, it seems to me
> that you are trying to use a thing for something it was not meant to.

The discussion around the plugin is in the thread:

  Subject: [PATCH  v4 00/54] plugins for TCG
  Date: Wed, 31 Jul 2019 17:06:25 +0100
  Message-Id: <20190731160719.11396-1-alex.bennee@linaro.org>

but essentially it would be nice if the plugin could be told what the
guest architecture is so it could either not attempt to initialise or
maybe change how it sets itself up. We could either come up with a QEMU
specific enumeration or perhaps report the TARGET_NAME string but given
the Elf spec defines a bunch of constants why not re-use them?

>
> Also, it would be better if you cc-ed corresponding architecture
> submaintainers.

I was relying on get_maintainer.pl but it doesn't really deal with these
common code cases that well. However they should all be CC'd on the main
movement patch as it touches a lot of subdirs:

  [PATCH  v1 2/4] elf: move elf.h to elf/elf.h and split out types

which should hopefully give them visibility of the thread.

>
> Yours, Aleksandar
>
>>  bsd-user/elfload.c     |  13 +----
>>  include/elf/elf-arch.h | 109 +++++++++++++++++++++++++++++++++++++++++
>>  linux-user/elfload.c   |  27 ++--------
>>  3 files changed, 115 insertions(+), 34 deletions(-)
>>  create mode 100644 include/elf/elf-arch.h
>>
>> diff --git a/bsd-user/elfload.c b/bsd-user/elfload.c
>> index 321ee98b86b..adaae0e0dca 100644
>> --- a/bsd-user/elfload.c
>> +++ b/bsd-user/elfload.c
>> @@ -5,6 +5,7 @@
>>  #include "qemu.h"
>>  #include "disas/disas.h"
>>  #include "qemu/path.h"
>> +#include "elf/elf-arch.h"
>>
>>  #ifdef _ARCH_PPC64
>>  #undef ARCH_DLINFO
>> @@ -12,7 +13,6 @@
>>  #undef ELF_HWCAP
>>  #undef ELF_CLASS
>>  #undef ELF_DATA
>> -#undef ELF_ARCH
>>  #endif
>>
>>  /* from personality.h */
>> @@ -115,7 +115,6 @@ static uint32_t get_elf_hwcap(void)
>>
>>  #define ELF_CLASS      ELFCLASS64
>>  #define ELF_DATA       ELFDATA2LSB
>> -#define ELF_ARCH       EM_X86_64
>>
>>  static inline void init_thread(struct target_pt_regs *regs, struct
> image_info *infop)
>>  {
>> @@ -141,7 +140,6 @@ static inline void init_thread(struct target_pt_regs
> *regs, struct image_info *i
>>   */
>>  #define ELF_CLASS       ELFCLASS32
>>  #define ELF_DATA        ELFDATA2LSB
>> -#define ELF_ARCH        EM_386
>>
>>  static inline void init_thread(struct target_pt_regs *regs, struct
> image_info *infop)
>>  {
>> @@ -176,7 +174,6 @@ static inline void init_thread(struct target_pt_regs
> *regs, struct image_info *i
>>  #else
>>  #define ELF_DATA        ELFDATA2LSB
>>  #endif
>> -#define ELF_ARCH        EM_ARM
>>
>>  static inline void init_thread(struct target_pt_regs *regs, struct
> image_info *infop)
>>  {
>> @@ -231,7 +228,6 @@ enum
>>
>>  #define ELF_CLASS   ELFCLASS64
>>  #define ELF_DATA    ELFDATA2MSB
>> -#define ELF_ARCH    EM_SPARCV9
>>
>>  #define STACK_BIAS              2047
>>
>> @@ -265,7 +261,6 @@ static inline void init_thread(struct target_pt_regs
> *regs, struct image_info *i
>>
>>  #define ELF_CLASS   ELFCLASS32
>>  #define ELF_DATA    ELFDATA2MSB
>> -#define ELF_ARCH    EM_SPARC
>>
>>  static inline void init_thread(struct target_pt_regs *regs, struct
> image_info *infop)
>>  {
>> @@ -302,7 +297,6 @@ static inline void init_thread(struct target_pt_regs
> *regs, struct image_info *i
>>  #else
>>  #define ELF_DATA        ELFDATA2LSB
>>  #endif
>> -#define ELF_ARCH        EM_PPC
>>
>>  /*
>>   * We need to put in some extra aux table entries to tell glibc what
>> @@ -388,7 +382,6 @@ static inline void init_thread(struct target_pt_regs
> *_regs, struct image_info *
>>  #else
>>  #define ELF_DATA        ELFDATA2LSB
>>  #endif
>> -#define ELF_ARCH    EM_MIPS
>>
>>  static inline void init_thread(struct target_pt_regs *regs, struct
> image_info *infop)
>>  {
>> @@ -410,7 +403,6 @@ static inline void init_thread(struct target_pt_regs
> *regs, struct image_info *i
>>
>>  #define ELF_CLASS ELFCLASS32
>>  #define ELF_DATA  ELFDATA2LSB
>> -#define ELF_ARCH  EM_SH
>>
>>  static inline void init_thread(struct target_pt_regs *regs, struct
> image_info *infop)
>>  {
>> @@ -432,7 +424,6 @@ static inline void init_thread(struct target_pt_regs
> *regs, struct image_info *i
>>
>>  #define ELF_CLASS ELFCLASS32
>>  #define ELF_DATA  ELFDATA2LSB
>> -#define ELF_ARCH  EM_CRIS
>>
>>  static inline void init_thread(struct target_pt_regs *regs, struct
> image_info *infop)
>>  {
>> @@ -452,7 +443,6 @@ static inline void init_thread(struct target_pt_regs
> *regs, struct image_info *i
>>
>>  #define ELF_CLASS       ELFCLASS32
>>  #define ELF_DATA        ELFDATA2MSB
>> -#define ELF_ARCH        EM_68K
>>
>>  /* ??? Does this need to do anything?
>>  #define ELF_PLAT_INIT(_r) */
>> @@ -477,7 +467,6 @@ static inline void init_thread(struct target_pt_regs
> *regs, struct image_info *i
>>
>>  #define ELF_CLASS      ELFCLASS64
>>  #define ELF_DATA       ELFDATA2MSB
>> -#define ELF_ARCH       EM_ALPHA
>>
>>  static inline void init_thread(struct target_pt_regs *regs, struct
> image_info *infop)
>>  {
>> diff --git a/include/elf/elf-arch.h b/include/elf/elf-arch.h
>> new file mode 100644
>> index 00000000000..9e052543c51
>> --- /dev/null
>> +++ b/include/elf/elf-arch.h
>> @@ -0,0 +1,109 @@
>> +/*
>> + * Elf Architecture Definition
>> + *
>> + * This is a simple expansion to define common Elf types for the
>> + * various machines for the various places it's needed in the source
>> + * tree.
>> + *
>> + * Copyright (c) 2019 Alex Bennée <alex.bennee@linaro.org>
>> + *
>> + * SPDX-License-Identifier: GPL-2.0-or-later
>> + *
>> + * This work is licensed under the terms of the GNU GPL, version 2 or
> later.
>> + * See the COPYING file in the top-level directory.
>> + */
>> +
>> +#ifndef _ELF_ARCH_H_
>> +#define _ELF_ARCH_H_
>> +
>> +#include "elf/elf.h"
>> +
>> +#ifndef NEED_CPU_H
>> +#error Needs an target definition
>> +#endif
>> +
>> +#ifdef ELF_ARCH
>> +#error ELF_ARCH should only be defined once in this file
>> +#endif
>> +
>> +#ifdef TARGET_I386
>> +#ifdef TARGET_X86_64
>> +#define ELF_ARCH EM_X86_64
>> +#else
>> +#define ELF_ARCH EM_386
>> +#endif
>> +#endif
>> +
>> +#ifdef TARGET_ARM
>> +#ifndef TARGET_AARCH64
>> +#define ELF_ARCH EM_ARM
>> +#else
>> +#define ELF_ARCH EM_AARCH64
>> +#endif
>> +#endif
>> +
>> +#ifdef TARGET_SPARC
>> +#ifdef TARGET_SPARC64
>> +#define ELF_ARCH EM_SPARCV9
>> +#else
>> +#define ELF_ARCH EM_SPARC
>> +#endif
>> +#endif
>> +
>> +#ifdef TARGET_PPC
>> +#define ELF_ARCH EM_PPC
>> +#endif
>> +
>> +#ifdef TARGET_MIPS
>> +#define ELF_ARCH EM_MIPS
>> +#endif
>> +
>> +#ifdef TARGET_MICROBLAZE
>> +#define ELF_ARCH EM_MICROBLAZE
>> +#endif
>> +
>> +#ifdef TARGET_NIOS2
>> +#define ELF_ARCH EM_ALTERA_NIOS2
>> +#endif
>> +
>> +#ifdef TARGET_OPENRISC
>> +#define ELF_ARCH EM_OPENRISC
>> +#endif
>> +
>> +#ifdef TARGET_SH4
>> +#define ELF_ARCH EM_SH
>> +#endif
>> +
>> +#ifdef TARGET_CRIS
>> +#define ELF_ARCH EM_CRIS
>> +#endif
>> +
>> +#ifdef TARGET_M68K
>> +#define ELF_ARCH EM_68K
>> +#endif
>> +
>> +#ifdef TARGET_ALPHA
>> +#define ELF_ARCH EM_ALPHA
>> +#endif
>> +
>> +#ifdef TARGET_S390X
>> +#define ELF_ARCH EM_S390
>> +#endif
>> +
>> +#ifdef TARGET_TILEGX
>> +#define ELF_ARCH EM_TILEGX
>> +#endif
>> +
>> +#ifdef TARGET_RISCV
>> +#define ELF_ARCH EM_RISCV
>> +#endif
>> +
>> +#ifdef TARGET_HPPA
>> +#define ELF_ARCH EM_PARISC
>> +#endif
>> +
>> +#ifdef TARGET_XTENSA
>> +#define ELF_ARCH EM_XTENSA
>> +#endif
>> +
>> +#endif /* _ELF_ARCH_H_ */
>> diff --git a/linux-user/elfload.c b/linux-user/elfload.c
>> index 59a0d21c6f1..3ac7016a7e3 100644
>> --- a/linux-user/elfload.c
>> +++ b/linux-user/elfload.c
>> @@ -8,10 +8,15 @@
>>  #include "qemu.h"
>>  #include "disas/disas.h"
>>  #include "elf/elf.h"
>> +#include "elf/elf-arch.h"
>>  #include "qemu/path.h"
>>  #include "qemu/queue.h"
>>  #include "qemu/guest-random.h"
>>
>> +#ifndef ELF_ARCH
>> +#error something got missed
>> +#endif
>> +
>>  #ifdef _ARCH_PPC64
>>  #undef ARCH_DLINFO
>>  #undef ELF_PLATFORM
>> @@ -19,7 +24,6 @@
>>  #undef ELF_HWCAP2
>>  #undef ELF_CLASS
>>  #undef ELF_DATA
>> -#undef ELF_ARCH
>>  #endif
>>
>>  #define ELF_OSABI   ELFOSABI_SYSV
>> @@ -148,7 +152,6 @@ static uint32_t get_elf_hwcap(void)
>>  #define ELF_START_MMAP 0x2aaaaab000ULL
>>
>>  #define ELF_CLASS      ELFCLASS64
>> -#define ELF_ARCH       EM_X86_64
>>
>>  static inline void init_thread(struct target_pt_regs *regs, struct
> image_info *infop)
>>  {
>> @@ -211,7 +214,6 @@ static void elf_core_copy_regs(target_elf_gregset_t
> *regs, const CPUX86State *en
>>   * These are used to set parameters in the core dumps.
>>   */
>>  #define ELF_CLASS       ELFCLASS32
>> -#define ELF_ARCH        EM_386
>>
>>  static inline void init_thread(struct target_pt_regs *regs,
>>                                 struct image_info *infop)
>> @@ -273,7 +275,6 @@ static void elf_core_copy_regs(target_elf_gregset_t
> *regs, const CPUX86State *en
>>
>>  #define ELF_START_MMAP 0x80000000
>>
>> -#define ELF_ARCH        EM_ARM
>>  #define ELF_CLASS       ELFCLASS32
>>
>>  static inline void init_thread(struct target_pt_regs *regs,
>> @@ -539,7 +540,6 @@ static const char *get_elf_platform(void)
>>  /* 64 bit ARM definitions */
>>  #define ELF_START_MMAP 0x80000000
>>
>> -#define ELF_ARCH        EM_AARCH64
>>  #define ELF_CLASS       ELFCLASS64
>>  #ifdef TARGET_WORDS_BIGENDIAN
>>  # define ELF_PLATFORM    "aarch64_be"
>> @@ -667,7 +667,6 @@ static uint32_t get_elf_hwcap(void)
>>  #endif
>>
>>  #define ELF_CLASS   ELFCLASS64
>> -#define ELF_ARCH    EM_SPARCV9
>>
>>  #define STACK_BIAS              2047
>>
>> @@ -696,7 +695,6 @@ static inline void init_thread(struct target_pt_regs
> *regs,
>>                      | HWCAP_SPARC_MULDIV)
>>
>>  #define ELF_CLASS   ELFCLASS32
>> -#define ELF_ARCH    EM_SPARC
>>
>>  static inline void init_thread(struct target_pt_regs *regs,
>>                                 struct image_info *infop)
>> @@ -728,8 +726,6 @@ static inline void init_thread(struct target_pt_regs
> *regs,
>>
>>  #endif
>>
>> -#define ELF_ARCH        EM_PPC
>> -
>>  /* Feature masks for the Aux Vector Hardware Capabilities (AT_HWCAP).
>>     See arch/powerpc/include/asm/cputable.h.  */
>>  enum {
>> @@ -921,7 +917,6 @@ static void elf_core_copy_regs(target_elf_gregset_t
> *regs, const CPUPPCState *en
>>  #else
>>  #define ELF_CLASS   ELFCLASS32
>>  #endif
>> -#define ELF_ARCH    EM_MIPS
>>
>>  #define elf_check_arch(x) ((x) == EM_MIPS || (x) == EM_NANOMIPS)
>>
>> @@ -1014,7 +1009,6 @@ static uint32_t get_elf_hwcap(void)
>>  #define elf_check_arch(x) ( (x) == EM_MICROBLAZE || (x) ==
> EM_MICROBLAZE_OLD)
>>
>>  #define ELF_CLASS   ELFCLASS32
>> -#define ELF_ARCH    EM_MICROBLAZE
>>
>>  static inline void init_thread(struct target_pt_regs *regs,
>>                                 struct image_info *infop)
>> @@ -1053,7 +1047,6 @@ static void elf_core_copy_regs(target_elf_gregset_t
> *regs, const CPUMBState *env
>>  #define elf_check_arch(x) ((x) == EM_ALTERA_NIOS2)
>>
>>  #define ELF_CLASS   ELFCLASS32
>> -#define ELF_ARCH    EM_ALTERA_NIOS2
>>
>>  static void init_thread(struct target_pt_regs *regs, struct image_info
> *infop)
>>  {
>> @@ -1107,7 +1100,6 @@ static void elf_core_copy_regs(target_elf_gregset_t
> *regs,
>>
>>  #define ELF_START_MMAP 0x08000000
>>
>> -#define ELF_ARCH EM_OPENRISC
>>  #define ELF_CLASS ELFCLASS32
>>  #define ELF_DATA  ELFDATA2MSB
>>
>> @@ -1146,7 +1138,6 @@ static void elf_core_copy_regs(target_elf_gregset_t
> *regs,
>>  #define ELF_START_MMAP 0x80000000
>>
>>  #define ELF_CLASS ELFCLASS32
>> -#define ELF_ARCH  EM_SH
>>
>>  static inline void init_thread(struct target_pt_regs *regs,
>>                                 struct image_info *infop)
>> @@ -1228,7 +1219,6 @@ static uint32_t get_elf_hwcap(void)
>>  #define ELF_START_MMAP 0x80000000
>>
>>  #define ELF_CLASS ELFCLASS32
>> -#define ELF_ARCH  EM_CRIS
>>
>>  static inline void init_thread(struct target_pt_regs *regs,
>>                                 struct image_info *infop)
>> @@ -1245,7 +1235,6 @@ static inline void init_thread(struct
> target_pt_regs *regs,
>>  #define ELF_START_MMAP 0x80000000
>>
>>  #define ELF_CLASS       ELFCLASS32
>> -#define ELF_ARCH        EM_68K
>>
>>  /* ??? Does this need to do anything?
>>     #define ELF_PLAT_INIT(_r) */
>> @@ -1296,7 +1285,6 @@ static void elf_core_copy_regs(target_elf_gregset_t
> *regs, const CPUM68KState *e
>>  #define ELF_START_MMAP (0x30000000000ULL)
>>
>>  #define ELF_CLASS      ELFCLASS64
>> -#define ELF_ARCH       EM_ALPHA
>>
>>  static inline void init_thread(struct target_pt_regs *regs,
>>                                 struct image_info *infop)
>> @@ -1316,7 +1304,6 @@ static inline void init_thread(struct
> target_pt_regs *regs,
>>
>>  #define ELF_CLASS      ELFCLASS64
>>  #define ELF_DATA       ELFDATA2MSB
>> -#define ELF_ARCH       EM_S390
>>
>>  #define ELF_HWCAP get_elf_hwcap()
>>
>> @@ -1362,7 +1349,6 @@ static inline void init_thread(struct
> target_pt_regs *regs, struct image_info *i
>>
>>  #define ELF_CLASS   ELFCLASS64
>>  #define ELF_DATA    ELFDATA2LSB
>> -#define ELF_ARCH    EM_TILEGX
>>
>>  static inline void init_thread(struct target_pt_regs *regs,
>>                                 struct image_info *infop)
>> @@ -1379,7 +1365,6 @@ static inline void init_thread(struct
> target_pt_regs *regs,
>>  #ifdef TARGET_RISCV
>>
>>  #define ELF_START_MMAP 0x80000000
>> -#define ELF_ARCH  EM_RISCV
>>
>>  #ifdef TARGET_RISCV32
>>  #define ELF_CLASS ELFCLASS32
>> @@ -1402,7 +1387,6 @@ static inline void init_thread(struct
> target_pt_regs *regs,
>>
>>  #define ELF_START_MMAP  0x80000000
>>  #define ELF_CLASS       ELFCLASS32
>> -#define ELF_ARCH        EM_PARISC
>>  #define ELF_PLATFORM    "PARISC"
>>  #define STACK_GROWS_DOWN 0
>>  #define STACK_ALIGNMENT  64
>> @@ -1427,7 +1411,6 @@ static inline void init_thread(struct
> target_pt_regs *regs,
>>  #define ELF_START_MMAP 0x20000000
>>
>>  #define ELF_CLASS       ELFCLASS32
>> -#define ELF_ARCH        EM_XTENSA
>>
>>  static inline void init_thread(struct target_pt_regs *regs,
>>                                 struct image_info *infop)
>> --
>> 2.20.1
>>
>>


--
Alex Bennée


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [Qemu-devel] [PATCH v1 3/4] elf: move elf_ops.h into include/elf/ and rename
  2019-09-10 19:34 ` [Qemu-devel] [PATCH v1 3/4] elf: move elf_ops.h into include/elf/ and rename Alex Bennée
@ 2019-09-11  8:20   ` Alex Bennée
  2019-09-14 18:16     ` Richard Henderson
  2019-10-21 13:56   ` Laurent Vivier
  1 sibling, 1 reply; 24+ messages in thread
From: Alex Bennée @ 2019-09-11  8:20 UTC (permalink / raw)
  To: peter.maydell; +Cc: qemu-arm, qemu-devel


Alex Bennée <alex.bennee@linaro.org> writes:

> Lets keep all the Elf manipulation bits together. Also rename the file
> to better reflect how it is used and add a little header to the file.
>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>  hw/core/loader.c                            | 4 ++--

It is arguable this could be a private header in hw/core as it is only
included in one place.

>  include/{hw/elf_ops.h => elf/elf_ops.inc.h} | 9 +++++++++
>  2 files changed, 11 insertions(+), 2 deletions(-)
>  rename include/{hw/elf_ops.h => elf/elf_ops.inc.h} (98%)
>
> diff --git a/hw/core/loader.c b/hw/core/loader.c
> index e0c6563e643..886179a4947 100644
> --- a/hw/core/loader.c
> +++ b/hw/core/loader.c
> @@ -302,7 +302,7 @@ static void *load_at(int fd, off_t offset, size_t size)
>  #define elf_word        uint32_t
>  #define elf_sword        int32_t
>  #define bswapSZs	bswap32s
> -#include "hw/elf_ops.h"
> +#include "elf/elf_ops.inc.h"
>
>  #undef elfhdr
>  #undef elf_phdr
> @@ -324,7 +324,7 @@ static void *load_at(int fd, off_t offset, size_t size)
>  #define elf_sword        int64_t
>  #define bswapSZs	bswap64s
>  #define SZ		64
> -#include "hw/elf_ops.h"
> +#include "elf/elf_ops.inc.h"
>
>  const char *load_elf_strerror(int error)
>  {
> diff --git a/include/hw/elf_ops.h b/include/elf/elf_ops.inc.h
> similarity index 98%
> rename from include/hw/elf_ops.h
> rename to include/elf/elf_ops.inc.h
> index 1496d7e7536..a820bd821d5 100644
> --- a/include/hw/elf_ops.h
> +++ b/include/elf/elf_ops.inc.h
> @@ -1,3 +1,12 @@
> +/*
> + * Macro expansions for Elf operations. This is included in a
> + * compilation unit with appropriate definitions for SZ and elf
> + * headers to generate utility functions for reading 32 and 64 bit elf
> + * headers.
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +
>  static void glue(bswap_ehdr, SZ)(struct elfhdr *ehdr)
>  {
>      bswap16s(&ehdr->e_type);			/* Object file type */


--
Alex Bennée


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [Qemu-devel] [PATCH v1 2/4] elf: move elf.h to elf/elf.h and split out types
  2019-09-10 19:34 ` [Qemu-devel] [PATCH v1 2/4] elf: move elf.h to elf/elf.h and split out types Alex Bennée
  2019-09-11  0:08   ` David Gibson
@ 2019-09-11  8:29   ` BALATON Zoltan
  2019-09-11  9:19     ` Alex Bennée
  2019-09-14 18:15   ` Richard Henderson
  2019-10-21 13:53   ` Laurent Vivier
  3 siblings, 1 reply; 24+ messages in thread
From: BALATON Zoltan @ 2019-09-11  8:29 UTC (permalink / raw)
  To: Alex Bennée
  Cc: peter.maydell, open list:RISC-V TCG CPUs, qemu-devel,
	open list:S390-ccw boot, qemu-arm, open list:PReP

On Tue, 10 Sep 2019, Alex Bennée wrote:
> diff --git a/include/elf/elf-types.inc.h b/include/elf/elf-types.inc.h
> new file mode 100644
> index 00000000000..35163adb2b5
> --- /dev/null
> +++ b/include/elf/elf-types.inc.h
> @@ -0,0 +1,63 @@
> +/*
> + * Elf Type Specialisation
> + *
> + * Copyright (c) 2019
> + * Written by Alex Bennée <alex.bennee@linaro.org>
> + *
> + * This code is licensed under the GNU .

You're missing end of licence sentence here. Also original file did not 
have copyright and licence header so you may want to fix that too or leave 
it out here as well for consistency,

> + */
> +
> +#ifndef _ELF_TYPES_INC_H_
> +#define _ELF_TYPES_INC_H_
> +
> +#ifndef ELF_CLASS
> +#error you must define ELF_CLASS before including elf-types.inc.h
> +#else
> +
> +#if ELF_CLASS == ELFCLASS32
> +
> +#define elfhdr      elf32_hdr
> +#define elf_phdr    elf32_phdr
> +#define elf_note    elf32_note
> +#define elf_shdr    elf32_shdr
> +#define elf_sym     elf32_sym
> +#define elf_addr_t  Elf32_Off
> +#define elf_rela    elf32_rela
> +
> +#ifdef ELF_USES_RELOCA
> +# define ELF_RELOC  Elf32_Rela
> +#else
> +# define ELF_RELOC  Elf32_Rel
> +#endif
> +
> +#ifndef ElfW
> +#  define ElfW(x)   Elf32_ ## x
> +#  define ELFW(x)   ELF32_ ## x
> +#endif
> +
> +#else /* ELF_CLASS == ELFCLASS64 */
> +
> +#define elfhdr      elf64_hdr
> +#define elf_phdr    elf64_phdr
> +#define elf_note    elf64_note
> +#define elf_shdr    elf64_shdr
> +#define elf_sym     elf64_sym
> +#define elf_addr_t  Elf64_Off
> +#define elf_rela    elf64_rela
> +
> +#ifdef ELF_USES_RELOCA
> +# define ELF_RELOC  Elf64_Rela
> +#else
> +# define ELF_RELOC  Elf64_Rel
> +#endif
> +
> +#ifndef ElfW
> +#  define ElfW(x)   Elf64_ ## x
> +#  define ELFW(x)   ELF64_ ## x
> +#endif
> +
> +#endif /* ELF_CLASS == ELFCLASS64 */
> +#endif /* ELF_CLASS */
> +#else
> +#error elf-types.inc.h should not be included twice in one compilation unit
> +#endif /* _ELF_TYPES_INC_H_ */
> diff --git a/include/elf.h b/include/elf/elf.h
> similarity index 98%
> rename from include/elf.h
> rename to include/elf/elf.h
> index 3501e0c8d03..2e264c1a7a0 100644
> --- a/include/elf.h
> +++ b/include/elf/elf.h
> @@ -1696,49 +1696,7 @@ struct elf32_fdpic_loadmap {
> };
>
> #ifdef ELF_CLASS
> -#if ELF_CLASS == ELFCLASS32
> -
> -#define elfhdr		elf32_hdr
> -#define elf_phdr	elf32_phdr
> -#define elf_note	elf32_note
> -#define elf_shdr	elf32_shdr
> -#define elf_sym		elf32_sym
> -#define elf_addr_t	Elf32_Off
> -#define elf_rela  elf32_rela
> -
> -#ifdef ELF_USES_RELOCA
> -# define ELF_RELOC      Elf32_Rela
> -#else
> -# define ELF_RELOC      Elf32_Rel
> -#endif
> -
> -#else
> -
> -#define elfhdr		elf64_hdr
> -#define elf_phdr	elf64_phdr
> -#define elf_note	elf64_note
> -#define elf_shdr	elf64_shdr
> -#define elf_sym		elf64_sym
> -#define elf_addr_t	Elf64_Off
> -#define elf_rela  elf64_rela
> -
> -#ifdef ELF_USES_RELOCA
> -# define ELF_RELOC      Elf64_Rela
> -#else
> -# define ELF_RELOC      Elf64_Rel
> -#endif
> -
> -#endif /* ELF_CLASS */
>
> -#ifndef ElfW
> -# if ELF_CLASS == ELFCLASS32
> -#  define ElfW(x)  Elf32_ ## x
> -#  define ELFW(x)  ELF32_ ## x
> -# else
> -#  define ElfW(x)  Elf64_ ## x
> -#  define ELFW(x)  ELF64_ ## x
> -# endif
> -#endif
>
> #endif /* ELF_CLASS */

Is there anything remaining in this #ifdef ELF_CLASS after this patch? If 
not why do you keep it?

Regards,
BALATON Zoltan

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [Qemu-devel] [PATCH v1 2/4] elf: move elf.h to elf/elf.h and split out types
  2019-09-11  8:29   ` BALATON Zoltan
@ 2019-09-11  9:19     ` Alex Bennée
  0 siblings, 0 replies; 24+ messages in thread
From: Alex Bennée @ 2019-09-11  9:19 UTC (permalink / raw)
  To: BALATON Zoltan
  Cc: peter.maydell, open list:RISC-V TCG CPUs, qemu-devel,
	open list:S390-ccw boot, qemu-arm, open list:PReP


BALATON Zoltan <balaton@eik.bme.hu> writes:

> On Tue, 10 Sep 2019, Alex Bennée wrote:
>> diff --git a/include/elf/elf-types.inc.h b/include/elf/elf-types.inc.h
>> new file mode 100644
>> index 00000000000..35163adb2b5
>> --- /dev/null
>> +++ b/include/elf/elf-types.inc.h
>> @@ -0,0 +1,63 @@
>> +/*
>> + * Elf Type Specialisation
>> + *
>> + * Copyright (c) 2019
>> + * Written by Alex Bennée <alex.bennee@linaro.org>
>> + *
>> + * This code is licensed under the GNU .
>
> You're missing end of licence sentence here. Also original file did
> not have copyright and licence header so you may want to fix that too
> or leave it out here as well for consistency,

The fault of my header macro - I'll try and fix it up when it's
expanding on the QEMU tree.

I'm going to assume that is should be the whole project license (GPL
v2). Most of the original file dates from 2003 and is Frabrice's commit
with the occasional commit mentioning be copied from Linux.

>
>> + */
>> +
>> +#ifndef _ELF_TYPES_INC_H_
>> +#define _ELF_TYPES_INC_H_
>> +
>> +#ifndef ELF_CLASS
>> +#error you must define ELF_CLASS before including elf-types.inc.h
>> +#else
>> +
>> +#if ELF_CLASS == ELFCLASS32
>> +
>> +#define elfhdr      elf32_hdr
>> +#define elf_phdr    elf32_phdr
>> +#define elf_note    elf32_note
>> +#define elf_shdr    elf32_shdr
>> +#define elf_sym     elf32_sym
>> +#define elf_addr_t  Elf32_Off
>> +#define elf_rela    elf32_rela
>> +
>> +#ifdef ELF_USES_RELOCA
>> +# define ELF_RELOC  Elf32_Rela
>> +#else
>> +# define ELF_RELOC  Elf32_Rel
>> +#endif
>> +
>> +#ifndef ElfW
>> +#  define ElfW(x)   Elf32_ ## x
>> +#  define ELFW(x)   ELF32_ ## x
>> +#endif
>> +
>> +#else /* ELF_CLASS == ELFCLASS64 */
>> +
>> +#define elfhdr      elf64_hdr
>> +#define elf_phdr    elf64_phdr
>> +#define elf_note    elf64_note
>> +#define elf_shdr    elf64_shdr
>> +#define elf_sym     elf64_sym
>> +#define elf_addr_t  Elf64_Off
>> +#define elf_rela    elf64_rela
>> +
>> +#ifdef ELF_USES_RELOCA
>> +# define ELF_RELOC  Elf64_Rela
>> +#else
>> +# define ELF_RELOC  Elf64_Rel
>> +#endif
>> +
>> +#ifndef ElfW
>> +#  define ElfW(x)   Elf64_ ## x
>> +#  define ELFW(x)   ELF64_ ## x
>> +#endif
>> +
>> +#endif /* ELF_CLASS == ELFCLASS64 */
>> +#endif /* ELF_CLASS */
>> +#else
>> +#error elf-types.inc.h should not be included twice in one compilation unit
>> +#endif /* _ELF_TYPES_INC_H_ */
>> diff --git a/include/elf.h b/include/elf/elf.h
>> similarity index 98%
>> rename from include/elf.h
>> rename to include/elf/elf.h
>> index 3501e0c8d03..2e264c1a7a0 100644
>> --- a/include/elf.h
>> +++ b/include/elf/elf.h
>> @@ -1696,49 +1696,7 @@ struct elf32_fdpic_loadmap {
>> };
>>
>> #ifdef ELF_CLASS
>> -#if ELF_CLASS == ELFCLASS32
>> -
>> -#define elfhdr		elf32_hdr
>> -#define elf_phdr	elf32_phdr
>> -#define elf_note	elf32_note
>> -#define elf_shdr	elf32_shdr
>> -#define elf_sym		elf32_sym
>> -#define elf_addr_t	Elf32_Off
>> -#define elf_rela  elf32_rela
>> -
>> -#ifdef ELF_USES_RELOCA
>> -# define ELF_RELOC      Elf32_Rela
>> -#else
>> -# define ELF_RELOC      Elf32_Rel
>> -#endif
>> -
>> -#else
>> -
>> -#define elfhdr		elf64_hdr
>> -#define elf_phdr	elf64_phdr
>> -#define elf_note	elf64_note
>> -#define elf_shdr	elf64_shdr
>> -#define elf_sym		elf64_sym
>> -#define elf_addr_t	Elf64_Off
>> -#define elf_rela  elf64_rela
>> -
>> -#ifdef ELF_USES_RELOCA
>> -# define ELF_RELOC      Elf64_Rela
>> -#else
>> -# define ELF_RELOC      Elf64_Rel
>> -#endif
>> -
>> -#endif /* ELF_CLASS */
>>
>> -#ifndef ElfW
>> -# if ELF_CLASS == ELFCLASS32
>> -#  define ElfW(x)  Elf32_ ## x
>> -#  define ELFW(x)  ELF32_ ## x
>> -# else
>> -#  define ElfW(x)  Elf64_ ## x
>> -#  define ELFW(x)  ELF64_ ## x
>> -# endif
>> -#endif
>>
>> #endif /* ELF_CLASS */
>
> Is there anything remaining in this #ifdef ELF_CLASS after this patch?
> If not why do you keep it?
>
> Regards,
> BALATON Zoltan


--
Alex Bennée


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [Qemu-devel] [PATCH v1 4/4] elf: move ELF_ARCH definition to elf-arch.h
  2019-09-10 21:14   ` Aleksandar Markovic
@ 2019-09-11  9:26     ` Alex Bennée
  2019-09-13 14:45       ` Aleksandar Markovic
  2019-09-14 15:52       ` Richard Henderson
  0 siblings, 2 replies; 24+ messages in thread
From: Alex Bennée @ 2019-09-11  9:26 UTC (permalink / raw)
  To: Aleksandar Markovic
  Cc: peter.maydell, Riku Voipio, qemu-devel, qemu-arm, Laurent Vivier


Aleksandar Markovic <aleksandar.m.mail@gmail.com> writes:

> 10.09.2019. 21.34, "Alex Bennée" <alex.bennee@linaro.org> је написао/ла:
>>
>> This is preparatory for plugins which will want to report the
>> architecture to plugins. Move the ELF_ARCH definition out of the
>> loader and into its own header.
>>
>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>> --
>
> Hi, Alex.
>
> I advice some caution here
> . For example, EM_NANOMIPS, and some other EM_xxx constants are not
> included in the new header.

EM_ARCH was never set to EM_NANOMIPS. In fact I can see e_machine is
checked against it so I guess it is only ever used to checking the
loading elf directly.

In fact EM_ARCH is only really the default fallback case for checking
the elf type if there is not a more "forgiving" check done by arch
specific code (elf_check_arch). The only other cace is the fallback for
EM_MACHINE unless PPC does something with it.

> I am not sure what exactly you want to achieve
> with this refactoring. But you may miss your goal, since some EM_xxx will
> be missing from your new header. Is this the right way to achieve what you
> want, and could you unintentionally introduce bugs? Can you outline in more
> details your intentions around the new header? Is this refactoring the only
> way?

As mentioned in the other reply maybe exposing a value from configure
into config-target.[mak|h] would be a better approach?

>
> Thanks, Aleksandar
>
>>  bsd-user/elfload.c     |  13 +----
>>  include/elf/elf-arch.h | 109 +++++++++++++++++++++++++++++++++++++++++
>>  linux-user/elfload.c   |  27 ++--------
>>  3 files changed, 115 insertions(+), 34 deletions(-)
>>  create mode 100644 include/elf/elf-arch.h
>>
>> diff --git a/bsd-user/elfload.c b/bsd-user/elfload.c
>> index 321ee98b86b..adaae0e0dca 100644
>> --- a/bsd-user/elfload.c
>> +++ b/bsd-user/elfload.c
>> @@ -5,6 +5,7 @@
>>  #include "qemu.h"
>>  #include "disas/disas.h"
>>  #include "qemu/path.h"
>> +#include "elf/elf-arch.h"
>>
>>  #ifdef _ARCH_PPC64
>>  #undef ARCH_DLINFO
>> @@ -12,7 +13,6 @@
>>  #undef ELF_HWCAP
>>  #undef ELF_CLASS
>>  #undef ELF_DATA
>> -#undef ELF_ARCH
>>  #endif
>>
>>  /* from personality.h */
>> @@ -115,7 +115,6 @@ static uint32_t get_elf_hwcap(void)
>>
>>  #define ELF_CLASS      ELFCLASS64
>>  #define ELF_DATA       ELFDATA2LSB
>> -#define ELF_ARCH       EM_X86_64
>>
>>  static inline void init_thread(struct target_pt_regs *regs, struct
> image_info *infop)
>>  {
>> @@ -141,7 +140,6 @@ static inline void init_thread(struct target_pt_regs
> *regs, struct image_info *i
>>   */
>>  #define ELF_CLASS       ELFCLASS32
>>  #define ELF_DATA        ELFDATA2LSB
>> -#define ELF_ARCH        EM_386
>>
>>  static inline void init_thread(struct target_pt_regs *regs, struct
> image_info *infop)
>>  {
>> @@ -176,7 +174,6 @@ static inline void init_thread(struct target_pt_regs
> *regs, struct image_info *i
>>  #else
>>  #define ELF_DATA        ELFDATA2LSB
>>  #endif
>> -#define ELF_ARCH        EM_ARM
>>
>>  static inline void init_thread(struct target_pt_regs *regs, struct
> image_info *infop)
>>  {
>> @@ -231,7 +228,6 @@ enum
>>
>>  #define ELF_CLASS   ELFCLASS64
>>  #define ELF_DATA    ELFDATA2MSB
>> -#define ELF_ARCH    EM_SPARCV9
>>
>>  #define STACK_BIAS              2047
>>
>> @@ -265,7 +261,6 @@ static inline void init_thread(struct target_pt_regs
> *regs, struct image_info *i
>>
>>  #define ELF_CLASS   ELFCLASS32
>>  #define ELF_DATA    ELFDATA2MSB
>> -#define ELF_ARCH    EM_SPARC
>>
>>  static inline void init_thread(struct target_pt_regs *regs, struct
> image_info *infop)
>>  {
>> @@ -302,7 +297,6 @@ static inline void init_thread(struct target_pt_regs
> *regs, struct image_info *i
>>  #else
>>  #define ELF_DATA        ELFDATA2LSB
>>  #endif
>> -#define ELF_ARCH        EM_PPC
>>
>>  /*
>>   * We need to put in some extra aux table entries to tell glibc what
>> @@ -388,7 +382,6 @@ static inline void init_thread(struct target_pt_regs
> *_regs, struct image_info *
>>  #else
>>  #define ELF_DATA        ELFDATA2LSB
>>  #endif
>> -#define ELF_ARCH    EM_MIPS
>>
>>  static inline void init_thread(struct target_pt_regs *regs, struct
> image_info *infop)
>>  {
>> @@ -410,7 +403,6 @@ static inline void init_thread(struct target_pt_regs
> *regs, struct image_info *i
>>
>>  #define ELF_CLASS ELFCLASS32
>>  #define ELF_DATA  ELFDATA2LSB
>> -#define ELF_ARCH  EM_SH
>>
>>  static inline void init_thread(struct target_pt_regs *regs, struct
> image_info *infop)
>>  {
>> @@ -432,7 +424,6 @@ static inline void init_thread(struct target_pt_regs
> *regs, struct image_info *i
>>
>>  #define ELF_CLASS ELFCLASS32
>>  #define ELF_DATA  ELFDATA2LSB
>> -#define ELF_ARCH  EM_CRIS
>>
>>  static inline void init_thread(struct target_pt_regs *regs, struct
> image_info *infop)
>>  {
>> @@ -452,7 +443,6 @@ static inline void init_thread(struct target_pt_regs
> *regs, struct image_info *i
>>
>>  #define ELF_CLASS       ELFCLASS32
>>  #define ELF_DATA        ELFDATA2MSB
>> -#define ELF_ARCH        EM_68K
>>
>>  /* ??? Does this need to do anything?
>>  #define ELF_PLAT_INIT(_r) */
>> @@ -477,7 +467,6 @@ static inline void init_thread(struct target_pt_regs
> *regs, struct image_info *i
>>
>>  #define ELF_CLASS      ELFCLASS64
>>  #define ELF_DATA       ELFDATA2MSB
>> -#define ELF_ARCH       EM_ALPHA
>>
>>  static inline void init_thread(struct target_pt_regs *regs, struct
> image_info *infop)
>>  {
>> diff --git a/include/elf/elf-arch.h b/include/elf/elf-arch.h
>> new file mode 100644
>> index 00000000000..9e052543c51
>> --- /dev/null
>> +++ b/include/elf/elf-arch.h
>> @@ -0,0 +1,109 @@
>> +/*
>> + * Elf Architecture Definition
>> + *
>> + * This is a simple expansion to define common Elf types for the
>> + * various machines for the various places it's needed in the source
>> + * tree.
>> + *
>> + * Copyright (c) 2019 Alex Bennée <alex.bennee@linaro.org>
>> + *
>> + * SPDX-License-Identifier: GPL-2.0-or-later
>> + *
>> + * This work is licensed under the terms of the GNU GPL, version 2 or
> later.
>> + * See the COPYING file in the top-level directory.
>> + */
>> +
>> +#ifndef _ELF_ARCH_H_
>> +#define _ELF_ARCH_H_
>> +
>> +#include "elf/elf.h"
>> +
>> +#ifndef NEED_CPU_H
>> +#error Needs an target definition
>> +#endif
>> +
>> +#ifdef ELF_ARCH
>> +#error ELF_ARCH should only be defined once in this file
>> +#endif
>> +
>> +#ifdef TARGET_I386
>> +#ifdef TARGET_X86_64
>> +#define ELF_ARCH EM_X86_64
>> +#else
>> +#define ELF_ARCH EM_386
>> +#endif
>> +#endif
>> +
>> +#ifdef TARGET_ARM
>> +#ifndef TARGET_AARCH64
>> +#define ELF_ARCH EM_ARM
>> +#else
>> +#define ELF_ARCH EM_AARCH64
>> +#endif
>> +#endif
>> +
>> +#ifdef TARGET_SPARC
>> +#ifdef TARGET_SPARC64
>> +#define ELF_ARCH EM_SPARCV9
>> +#else
>> +#define ELF_ARCH EM_SPARC
>> +#endif
>> +#endif
>> +
>> +#ifdef TARGET_PPC
>> +#define ELF_ARCH EM_PPC
>> +#endif
>> +
>> +#ifdef TARGET_MIPS
>> +#define ELF_ARCH EM_MIPS
>> +#endif
>> +
>> +#ifdef TARGET_MICROBLAZE
>> +#define ELF_ARCH EM_MICROBLAZE
>> +#endif
>> +
>> +#ifdef TARGET_NIOS2
>> +#define ELF_ARCH EM_ALTERA_NIOS2
>> +#endif
>> +
>> +#ifdef TARGET_OPENRISC
>> +#define ELF_ARCH EM_OPENRISC
>> +#endif
>> +
>> +#ifdef TARGET_SH4
>> +#define ELF_ARCH EM_SH
>> +#endif
>> +
>> +#ifdef TARGET_CRIS
>> +#define ELF_ARCH EM_CRIS
>> +#endif
>> +
>> +#ifdef TARGET_M68K
>> +#define ELF_ARCH EM_68K
>> +#endif
>> +
>> +#ifdef TARGET_ALPHA
>> +#define ELF_ARCH EM_ALPHA
>> +#endif
>> +
>> +#ifdef TARGET_S390X
>> +#define ELF_ARCH EM_S390
>> +#endif
>> +
>> +#ifdef TARGET_TILEGX
>> +#define ELF_ARCH EM_TILEGX
>> +#endif
>> +
>> +#ifdef TARGET_RISCV
>> +#define ELF_ARCH EM_RISCV
>> +#endif
>> +
>> +#ifdef TARGET_HPPA
>> +#define ELF_ARCH EM_PARISC
>> +#endif
>> +
>> +#ifdef TARGET_XTENSA
>> +#define ELF_ARCH EM_XTENSA
>> +#endif
>> +
>> +#endif /* _ELF_ARCH_H_ */
>> diff --git a/linux-user/elfload.c b/linux-user/elfload.c
>> index 59a0d21c6f1..3ac7016a7e3 100644
>> --- a/linux-user/elfload.c
>> +++ b/linux-user/elfload.c
>> @@ -8,10 +8,15 @@
>>  #include "qemu.h"
>>  #include "disas/disas.h"
>>  #include "elf/elf.h"
>> +#include "elf/elf-arch.h"
>>  #include "qemu/path.h"
>>  #include "qemu/queue.h"
>>  #include "qemu/guest-random.h"
>>
>> +#ifndef ELF_ARCH
>> +#error something got missed
>> +#endif
>> +
>>  #ifdef _ARCH_PPC64
>>  #undef ARCH_DLINFO
>>  #undef ELF_PLATFORM
>> @@ -19,7 +24,6 @@
>>  #undef ELF_HWCAP2
>>  #undef ELF_CLASS
>>  #undef ELF_DATA
>> -#undef ELF_ARCH
>>  #endif
>>
>>  #define ELF_OSABI   ELFOSABI_SYSV
>> @@ -148,7 +152,6 @@ static uint32_t get_elf_hwcap(void)
>>  #define ELF_START_MMAP 0x2aaaaab000ULL
>>
>>  #define ELF_CLASS      ELFCLASS64
>> -#define ELF_ARCH       EM_X86_64
>>
>>  static inline void init_thread(struct target_pt_regs *regs, struct
> image_info *infop)
>>  {
>> @@ -211,7 +214,6 @@ static void elf_core_copy_regs(target_elf_gregset_t
> *regs, const CPUX86State *en
>>   * These are used to set parameters in the core dumps.
>>   */
>>  #define ELF_CLASS       ELFCLASS32
>> -#define ELF_ARCH        EM_386
>>
>>  static inline void init_thread(struct target_pt_regs *regs,
>>                                 struct image_info *infop)
>> @@ -273,7 +275,6 @@ static void elf_core_copy_regs(target_elf_gregset_t
> *regs, const CPUX86State *en
>>
>>  #define ELF_START_MMAP 0x80000000
>>
>> -#define ELF_ARCH        EM_ARM
>>  #define ELF_CLASS       ELFCLASS32
>>
>>  static inline void init_thread(struct target_pt_regs *regs,
>> @@ -539,7 +540,6 @@ static const char *get_elf_platform(void)
>>  /* 64 bit ARM definitions */
>>  #define ELF_START_MMAP 0x80000000
>>
>> -#define ELF_ARCH        EM_AARCH64
>>  #define ELF_CLASS       ELFCLASS64
>>  #ifdef TARGET_WORDS_BIGENDIAN
>>  # define ELF_PLATFORM    "aarch64_be"
>> @@ -667,7 +667,6 @@ static uint32_t get_elf_hwcap(void)
>>  #endif
>>
>>  #define ELF_CLASS   ELFCLASS64
>> -#define ELF_ARCH    EM_SPARCV9
>>
>>  #define STACK_BIAS              2047
>>
>> @@ -696,7 +695,6 @@ static inline void init_thread(struct target_pt_regs
> *regs,
>>                      | HWCAP_SPARC_MULDIV)
>>
>>  #define ELF_CLASS   ELFCLASS32
>> -#define ELF_ARCH    EM_SPARC
>>
>>  static inline void init_thread(struct target_pt_regs *regs,
>>                                 struct image_info *infop)
>> @@ -728,8 +726,6 @@ static inline void init_thread(struct target_pt_regs
> *regs,
>>
>>  #endif
>>
>> -#define ELF_ARCH        EM_PPC
>> -
>>  /* Feature masks for the Aux Vector Hardware Capabilities (AT_HWCAP).
>>     See arch/powerpc/include/asm/cputable.h.  */
>>  enum {
>> @@ -921,7 +917,6 @@ static void elf_core_copy_regs(target_elf_gregset_t
> *regs, const CPUPPCState *en
>>  #else
>>  #define ELF_CLASS   ELFCLASS32
>>  #endif
>> -#define ELF_ARCH    EM_MIPS
>>
>>  #define elf_check_arch(x) ((x) == EM_MIPS || (x) == EM_NANOMIPS)
>>
>> @@ -1014,7 +1009,6 @@ static uint32_t get_elf_hwcap(void)
>>  #define elf_check_arch(x) ( (x) == EM_MICROBLAZE || (x) ==
> EM_MICROBLAZE_OLD)
>>
>>  #define ELF_CLASS   ELFCLASS32
>> -#define ELF_ARCH    EM_MICROBLAZE
>>
>>  static inline void init_thread(struct target_pt_regs *regs,
>>                                 struct image_info *infop)
>> @@ -1053,7 +1047,6 @@ static void elf_core_copy_regs(target_elf_gregset_t
> *regs, const CPUMBState *env
>>  #define elf_check_arch(x) ((x) == EM_ALTERA_NIOS2)
>>
>>  #define ELF_CLASS   ELFCLASS32
>> -#define ELF_ARCH    EM_ALTERA_NIOS2
>>
>>  static void init_thread(struct target_pt_regs *regs, struct image_info
> *infop)
>>  {
>> @@ -1107,7 +1100,6 @@ static void elf_core_copy_regs(target_elf_gregset_t
> *regs,
>>
>>  #define ELF_START_MMAP 0x08000000
>>
>> -#define ELF_ARCH EM_OPENRISC
>>  #define ELF_CLASS ELFCLASS32
>>  #define ELF_DATA  ELFDATA2MSB
>>
>> @@ -1146,7 +1138,6 @@ static void elf_core_copy_regs(target_elf_gregset_t
> *regs,
>>  #define ELF_START_MMAP 0x80000000
>>
>>  #define ELF_CLASS ELFCLASS32
>> -#define ELF_ARCH  EM_SH
>>
>>  static inline void init_thread(struct target_pt_regs *regs,
>>                                 struct image_info *infop)
>> @@ -1228,7 +1219,6 @@ static uint32_t get_elf_hwcap(void)
>>  #define ELF_START_MMAP 0x80000000
>>
>>  #define ELF_CLASS ELFCLASS32
>> -#define ELF_ARCH  EM_CRIS
>>
>>  static inline void init_thread(struct target_pt_regs *regs,
>>                                 struct image_info *infop)
>> @@ -1245,7 +1235,6 @@ static inline void init_thread(struct
> target_pt_regs *regs,
>>  #define ELF_START_MMAP 0x80000000
>>
>>  #define ELF_CLASS       ELFCLASS32
>> -#define ELF_ARCH        EM_68K
>>
>>  /* ??? Does this need to do anything?
>>     #define ELF_PLAT_INIT(_r) */
>> @@ -1296,7 +1285,6 @@ static void elf_core_copy_regs(target_elf_gregset_t
> *regs, const CPUM68KState *e
>>  #define ELF_START_MMAP (0x30000000000ULL)
>>
>>  #define ELF_CLASS      ELFCLASS64
>> -#define ELF_ARCH       EM_ALPHA
>>
>>  static inline void init_thread(struct target_pt_regs *regs,
>>                                 struct image_info *infop)
>> @@ -1316,7 +1304,6 @@ static inline void init_thread(struct
> target_pt_regs *regs,
>>
>>  #define ELF_CLASS      ELFCLASS64
>>  #define ELF_DATA       ELFDATA2MSB
>> -#define ELF_ARCH       EM_S390
>>
>>  #define ELF_HWCAP get_elf_hwcap()
>>
>> @@ -1362,7 +1349,6 @@ static inline void init_thread(struct
> target_pt_regs *regs, struct image_info *i
>>
>>  #define ELF_CLASS   ELFCLASS64
>>  #define ELF_DATA    ELFDATA2LSB
>> -#define ELF_ARCH    EM_TILEGX
>>
>>  static inline void init_thread(struct target_pt_regs *regs,
>>                                 struct image_info *infop)
>> @@ -1379,7 +1365,6 @@ static inline void init_thread(struct
> target_pt_regs *regs,
>>  #ifdef TARGET_RISCV
>>
>>  #define ELF_START_MMAP 0x80000000
>> -#define ELF_ARCH  EM_RISCV
>>
>>  #ifdef TARGET_RISCV32
>>  #define ELF_CLASS ELFCLASS32
>> @@ -1402,7 +1387,6 @@ static inline void init_thread(struct
> target_pt_regs *regs,
>>
>>  #define ELF_START_MMAP  0x80000000
>>  #define ELF_CLASS       ELFCLASS32
>> -#define ELF_ARCH        EM_PARISC
>>  #define ELF_PLATFORM    "PARISC"
>>  #define STACK_GROWS_DOWN 0
>>  #define STACK_ALIGNMENT  64
>> @@ -1427,7 +1411,6 @@ static inline void init_thread(struct
> target_pt_regs *regs,
>>  #define ELF_START_MMAP 0x20000000
>>
>>  #define ELF_CLASS       ELFCLASS32
>> -#define ELF_ARCH        EM_XTENSA
>>
>>  static inline void init_thread(struct target_pt_regs *regs,
>>                                 struct image_info *infop)
>> --
>> 2.20.1
>>
>>


--
Alex Bennée


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [Qemu-devel] [PATCH v1 4/4] elf: move ELF_ARCH definition to elf-arch.h
  2019-09-11  9:26     ` Alex Bennée
@ 2019-09-13 14:45       ` Aleksandar Markovic
  2019-09-14 15:52       ` Richard Henderson
  1 sibling, 0 replies; 24+ messages in thread
From: Aleksandar Markovic @ 2019-09-13 14:45 UTC (permalink / raw)
  To: Alex Bennée
  Cc: Peter Maydell, Riku Voipio, QEMU Developers, open list:Stellaris,
	Laurent Vivier

On Wed, Sep 11, 2019 at 11:26 AM Alex Bennée <alex.bennee@linaro.org> wrote:

>
> Aleksandar Markovic <aleksandar.m.mail@gmail.com> writes:
>
> > 10.09.2019. 21.34, "Alex Bennée" <alex.bennee@linaro.org> је написао/ла:
> >>
> >> This is preparatory for plugins which will want to report the
> >> architecture to plugins. Move the ELF_ARCH definition out of the
> >> loader and into its own header.
> >>
> >> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> >> --
> >
> > Hi, Alex.
> >
> > I advice some caution here
> > . For example, EM_NANOMIPS, and some other EM_xxx constants are not
> > included in the new header.
>
> EM_ARCH was never set to EM_NANOMIPS. In fact I can see e_machine is
> checked against it so I guess it is only ever used to checking the
> loading elf directly.
>
> In fact EM_ARCH is only really the default fallback case for checking
> the elf type if there is not a more "forgiving" check done by arch
> specific code (elf_check_arch). The only other cace is the fallback for
> EM_MACHINE unless PPC does something with it.
>
> > I am not sure what exactly you want to achieve
> > with this refactoring. But you may miss your goal, since some EM_xxx will
> > be missing from your new header. Is this the right way to achieve what
> you
> > want, and could you unintentionally introduce bugs? Can you outline in
> more
> > details your intentions around the new header? Is this refactoring the
> only
> > way?
>
> As mentioned in the other reply maybe exposing a value from configure
> into config-target.[mak|h] would be a better approach?
>
>
Alex,

I am not sure about that either - and I still don't know the details of
what info
the new plugin(s) need. An alternative solution may be better (and needed).

For MIPS, we are in a situation (that is probably hard to comprehend by
other platforms' people) that QEMU compiled with TARGET_MIPS must
accept both EM_MIPS and EM_NANOMIPS as valid in corresponding
field of elf header (other checks, like compatibility of the CPU with such
elf file, are performed later on, separately). Needless to say, all this is
not known in compile time.

Perhaps we need to record EM_xxx value of accepted (loaded) elf file,
and expose that value through a callback, or a similar mechanism, to
the plugins in question?

Sorry for a little late response. Yours,
Aleksandar



> >
> > Thanks, Aleksandar
> >
> >>  bsd-user/elfload.c     |  13 +----
> >>  include/elf/elf-arch.h | 109 +++++++++++++++++++++++++++++++++++++++++
> >>  linux-user/elfload.c   |  27 ++--------
> >>  3 files changed, 115 insertions(+), 34 deletions(-)
> >>  create mode 100644 include/elf/elf-arch.h
> >>
> >> diff --git a/bsd-user/elfload.c b/bsd-user/elfload.c
> >> index 321ee98b86b..adaae0e0dca 100644
> >> --- a/bsd-user/elfload.c
> >> +++ b/bsd-user/elfload.c
> >> @@ -5,6 +5,7 @@
> >>  #include "qemu.h"
> >>  #include "disas/disas.h"
> >>  #include "qemu/path.h"
> >> +#include "elf/elf-arch.h"
> >>
> >>  #ifdef _ARCH_PPC64
> >>  #undef ARCH_DLINFO
> >> @@ -12,7 +13,6 @@
> >>  #undef ELF_HWCAP
> >>  #undef ELF_CLASS
> >>  #undef ELF_DATA
> >> -#undef ELF_ARCH
> >>  #endif
> >>
> >>  /* from personality.h */
> >> @@ -115,7 +115,6 @@ static uint32_t get_elf_hwcap(void)
> >>
> >>  #define ELF_CLASS      ELFCLASS64
> >>  #define ELF_DATA       ELFDATA2LSB
> >> -#define ELF_ARCH       EM_X86_64
> >>
> >>  static inline void init_thread(struct target_pt_regs *regs, struct
> > image_info *infop)
> >>  {
> >> @@ -141,7 +140,6 @@ static inline void init_thread(struct target_pt_regs
> > *regs, struct image_info *i
> >>   */
> >>  #define ELF_CLASS       ELFCLASS32
> >>  #define ELF_DATA        ELFDATA2LSB
> >> -#define ELF_ARCH        EM_386
> >>
> >>  static inline void init_thread(struct target_pt_regs *regs, struct
> > image_info *infop)
> >>  {
> >> @@ -176,7 +174,6 @@ static inline void init_thread(struct target_pt_regs
> > *regs, struct image_info *i
> >>  #else
> >>  #define ELF_DATA        ELFDATA2LSB
> >>  #endif
> >> -#define ELF_ARCH        EM_ARM
> >>
> >>  static inline void init_thread(struct target_pt_regs *regs, struct
> > image_info *infop)
> >>  {
> >> @@ -231,7 +228,6 @@ enum
> >>
> >>  #define ELF_CLASS   ELFCLASS64
> >>  #define ELF_DATA    ELFDATA2MSB
> >> -#define ELF_ARCH    EM_SPARCV9
> >>
> >>  #define STACK_BIAS              2047
> >>
> >> @@ -265,7 +261,6 @@ static inline void init_thread(struct target_pt_regs
> > *regs, struct image_info *i
> >>
> >>  #define ELF_CLASS   ELFCLASS32
> >>  #define ELF_DATA    ELFDATA2MSB
> >> -#define ELF_ARCH    EM_SPARC
> >>
> >>  static inline void init_thread(struct target_pt_regs *regs, struct
> > image_info *infop)
> >>  {
> >> @@ -302,7 +297,6 @@ static inline void init_thread(struct target_pt_regs
> > *regs, struct image_info *i
> >>  #else
> >>  #define ELF_DATA        ELFDATA2LSB
> >>  #endif
> >> -#define ELF_ARCH        EM_PPC
> >>
> >>  /*
> >>   * We need to put in some extra aux table entries to tell glibc what
> >> @@ -388,7 +382,6 @@ static inline void init_thread(struct target_pt_regs
> > *_regs, struct image_info *
> >>  #else
> >>  #define ELF_DATA        ELFDATA2LSB
> >>  #endif
> >> -#define ELF_ARCH    EM_MIPS
> >>
> >>  static inline void init_thread(struct target_pt_regs *regs, struct
> > image_info *infop)
> >>  {
> >> @@ -410,7 +403,6 @@ static inline void init_thread(struct target_pt_regs
> > *regs, struct image_info *i
> >>
> >>  #define ELF_CLASS ELFCLASS32
> >>  #define ELF_DATA  ELFDATA2LSB
> >> -#define ELF_ARCH  EM_SH
> >>
> >>  static inline void init_thread(struct target_pt_regs *regs, struct
> > image_info *infop)
> >>  {
> >> @@ -432,7 +424,6 @@ static inline void init_thread(struct target_pt_regs
> > *regs, struct image_info *i
> >>
> >>  #define ELF_CLASS ELFCLASS32
> >>  #define ELF_DATA  ELFDATA2LSB
> >> -#define ELF_ARCH  EM_CRIS
> >>
> >>  static inline void init_thread(struct target_pt_regs *regs, struct
> > image_info *infop)
> >>  {
> >> @@ -452,7 +443,6 @@ static inline void init_thread(struct target_pt_regs
> > *regs, struct image_info *i
> >>
> >>  #define ELF_CLASS       ELFCLASS32
> >>  #define ELF_DATA        ELFDATA2MSB
> >> -#define ELF_ARCH        EM_68K
> >>
> >>  /* ??? Does this need to do anything?
> >>  #define ELF_PLAT_INIT(_r) */
> >> @@ -477,7 +467,6 @@ static inline void init_thread(struct target_pt_regs
> > *regs, struct image_info *i
> >>
> >>  #define ELF_CLASS      ELFCLASS64
> >>  #define ELF_DATA       ELFDATA2MSB
> >> -#define ELF_ARCH       EM_ALPHA
> >>
> >>  static inline void init_thread(struct target_pt_regs *regs, struct
> > image_info *infop)
> >>  {
> >> diff --git a/include/elf/elf-arch.h b/include/elf/elf-arch.h
> >> new file mode 100644
> >> index 00000000000..9e052543c51
> >> --- /dev/null
> >> +++ b/include/elf/elf-arch.h
> >> @@ -0,0 +1,109 @@
> >> +/*
> >> + * Elf Architecture Definition
> >> + *
> >> + * This is a simple expansion to define common Elf types for the
> >> + * various machines for the various places it's needed in the source
> >> + * tree.
> >> + *
> >> + * Copyright (c) 2019 Alex Bennée <alex.bennee@linaro.org>
> >> + *
> >> + * SPDX-License-Identifier: GPL-2.0-or-later
> >> + *
> >> + * This work is licensed under the terms of the GNU GPL, version 2 or
> > later.
> >> + * See the COPYING file in the top-level directory.
> >> + */
> >> +
> >> +#ifndef _ELF_ARCH_H_
> >> +#define _ELF_ARCH_H_
> >> +
> >> +#include "elf/elf.h"
> >> +
> >> +#ifndef NEED_CPU_H
> >> +#error Needs an target definition
> >> +#endif
> >> +
> >> +#ifdef ELF_ARCH
> >> +#error ELF_ARCH should only be defined once in this file
> >> +#endif
> >> +
> >> +#ifdef TARGET_I386
> >> +#ifdef TARGET_X86_64
> >> +#define ELF_ARCH EM_X86_64
> >> +#else
> >> +#define ELF_ARCH EM_386
> >> +#endif
> >> +#endif
> >> +
> >> +#ifdef TARGET_ARM
> >> +#ifndef TARGET_AARCH64
> >> +#define ELF_ARCH EM_ARM
> >> +#else
> >> +#define ELF_ARCH EM_AARCH64
> >> +#endif
> >> +#endif
> >> +
> >> +#ifdef TARGET_SPARC
> >> +#ifdef TARGET_SPARC64
> >> +#define ELF_ARCH EM_SPARCV9
> >> +#else
> >> +#define ELF_ARCH EM_SPARC
> >> +#endif
> >> +#endif
> >> +
> >> +#ifdef TARGET_PPC
> >> +#define ELF_ARCH EM_PPC
> >> +#endif
> >> +
> >> +#ifdef TARGET_MIPS
> >> +#define ELF_ARCH EM_MIPS
> >> +#endif
> >> +
> >> +#ifdef TARGET_MICROBLAZE
> >> +#define ELF_ARCH EM_MICROBLAZE
> >> +#endif
> >> +
> >> +#ifdef TARGET_NIOS2
> >> +#define ELF_ARCH EM_ALTERA_NIOS2
> >> +#endif
> >> +
> >> +#ifdef TARGET_OPENRISC
> >> +#define ELF_ARCH EM_OPENRISC
> >> +#endif
> >> +
> >> +#ifdef TARGET_SH4
> >> +#define ELF_ARCH EM_SH
> >> +#endif
> >> +
> >> +#ifdef TARGET_CRIS
> >> +#define ELF_ARCH EM_CRIS
> >> +#endif
> >> +
> >> +#ifdef TARGET_M68K
> >> +#define ELF_ARCH EM_68K
> >> +#endif
> >> +
> >> +#ifdef TARGET_ALPHA
> >> +#define ELF_ARCH EM_ALPHA
> >> +#endif
> >> +
> >> +#ifdef TARGET_S390X
> >> +#define ELF_ARCH EM_S390
> >> +#endif
> >> +
> >> +#ifdef TARGET_TILEGX
> >> +#define ELF_ARCH EM_TILEGX
> >> +#endif
> >> +
> >> +#ifdef TARGET_RISCV
> >> +#define ELF_ARCH EM_RISCV
> >> +#endif
> >> +
> >> +#ifdef TARGET_HPPA
> >> +#define ELF_ARCH EM_PARISC
> >> +#endif
> >> +
> >> +#ifdef TARGET_XTENSA
> >> +#define ELF_ARCH EM_XTENSA
> >> +#endif
> >> +
> >> +#endif /* _ELF_ARCH_H_ */
> >> diff --git a/linux-user/elfload.c b/linux-user/elfload.c
> >> index 59a0d21c6f1..3ac7016a7e3 100644
> >> --- a/linux-user/elfload.c
> >> +++ b/linux-user/elfload.c
> >> @@ -8,10 +8,15 @@
> >>  #include "qemu.h"
> >>  #include "disas/disas.h"
> >>  #include "elf/elf.h"
> >> +#include "elf/elf-arch.h"
> >>  #include "qemu/path.h"
> >>  #include "qemu/queue.h"
> >>  #include "qemu/guest-random.h"
> >>
> >> +#ifndef ELF_ARCH
> >> +#error something got missed
> >> +#endif
> >> +
> >>  #ifdef _ARCH_PPC64
> >>  #undef ARCH_DLINFO
> >>  #undef ELF_PLATFORM
> >> @@ -19,7 +24,6 @@
> >>  #undef ELF_HWCAP2
> >>  #undef ELF_CLASS
> >>  #undef ELF_DATA
> >> -#undef ELF_ARCH
> >>  #endif
> >>
> >>  #define ELF_OSABI   ELFOSABI_SYSV
> >> @@ -148,7 +152,6 @@ static uint32_t get_elf_hwcap(void)
> >>  #define ELF_START_MMAP 0x2aaaaab000ULL
> >>
> >>  #define ELF_CLASS      ELFCLASS64
> >> -#define ELF_ARCH       EM_X86_64
> >>
> >>  static inline void init_thread(struct target_pt_regs *regs, struct
> > image_info *infop)
> >>  {
> >> @@ -211,7 +214,6 @@ static void elf_core_copy_regs(target_elf_gregset_t
> > *regs, const CPUX86State *en
> >>   * These are used to set parameters in the core dumps.
> >>   */
> >>  #define ELF_CLASS       ELFCLASS32
> >> -#define ELF_ARCH        EM_386
> >>
> >>  static inline void init_thread(struct target_pt_regs *regs,
> >>                                 struct image_info *infop)
> >> @@ -273,7 +275,6 @@ static void elf_core_copy_regs(target_elf_gregset_t
> > *regs, const CPUX86State *en
> >>
> >>  #define ELF_START_MMAP 0x80000000
> >>
> >> -#define ELF_ARCH        EM_ARM
> >>  #define ELF_CLASS       ELFCLASS32
> >>
> >>  static inline void init_thread(struct target_pt_regs *regs,
> >> @@ -539,7 +540,6 @@ static const char *get_elf_platform(void)
> >>  /* 64 bit ARM definitions */
> >>  #define ELF_START_MMAP 0x80000000
> >>
> >> -#define ELF_ARCH        EM_AARCH64
> >>  #define ELF_CLASS       ELFCLASS64
> >>  #ifdef TARGET_WORDS_BIGENDIAN
> >>  # define ELF_PLATFORM    "aarch64_be"
> >> @@ -667,7 +667,6 @@ static uint32_t get_elf_hwcap(void)
> >>  #endif
> >>
> >>  #define ELF_CLASS   ELFCLASS64
> >> -#define ELF_ARCH    EM_SPARCV9
> >>
> >>  #define STACK_BIAS              2047
> >>
> >> @@ -696,7 +695,6 @@ static inline void init_thread(struct target_pt_regs
> > *regs,
> >>                      | HWCAP_SPARC_MULDIV)
> >>
> >>  #define ELF_CLASS   ELFCLASS32
> >> -#define ELF_ARCH    EM_SPARC
> >>
> >>  static inline void init_thread(struct target_pt_regs *regs,
> >>                                 struct image_info *infop)
> >> @@ -728,8 +726,6 @@ static inline void init_thread(struct target_pt_regs
> > *regs,
> >>
> >>  #endif
> >>
> >> -#define ELF_ARCH        EM_PPC
> >> -
> >>  /* Feature masks for the Aux Vector Hardware Capabilities (AT_HWCAP).
> >>     See arch/powerpc/include/asm/cputable.h.  */
> >>  enum {
> >> @@ -921,7 +917,6 @@ static void elf_core_copy_regs(target_elf_gregset_t
> > *regs, const CPUPPCState *en
> >>  #else
> >>  #define ELF_CLASS   ELFCLASS32
> >>  #endif
> >> -#define ELF_ARCH    EM_MIPS
> >>
> >>  #define elf_check_arch(x) ((x) == EM_MIPS || (x) == EM_NANOMIPS)
> >>
> >> @@ -1014,7 +1009,6 @@ static uint32_t get_elf_hwcap(void)
> >>  #define elf_check_arch(x) ( (x) == EM_MICROBLAZE || (x) ==
> > EM_MICROBLAZE_OLD)
> >>
> >>  #define ELF_CLASS   ELFCLASS32
> >> -#define ELF_ARCH    EM_MICROBLAZE
> >>
> >>  static inline void init_thread(struct target_pt_regs *regs,
> >>                                 struct image_info *infop)
> >> @@ -1053,7 +1047,6 @@ static void
> elf_core_copy_regs(target_elf_gregset_t
> > *regs, const CPUMBState *env
> >>  #define elf_check_arch(x) ((x) == EM_ALTERA_NIOS2)
> >>
> >>  #define ELF_CLASS   ELFCLASS32
> >> -#define ELF_ARCH    EM_ALTERA_NIOS2
> >>
> >>  static void init_thread(struct target_pt_regs *regs, struct image_info
> > *infop)
> >>  {
> >> @@ -1107,7 +1100,6 @@ static void
> elf_core_copy_regs(target_elf_gregset_t
> > *regs,
> >>
> >>  #define ELF_START_MMAP 0x08000000
> >>
> >> -#define ELF_ARCH EM_OPENRISC
> >>  #define ELF_CLASS ELFCLASS32
> >>  #define ELF_DATA  ELFDATA2MSB
> >>
> >> @@ -1146,7 +1138,6 @@ static void
> elf_core_copy_regs(target_elf_gregset_t
> > *regs,
> >>  #define ELF_START_MMAP 0x80000000
> >>
> >>  #define ELF_CLASS ELFCLASS32
> >> -#define ELF_ARCH  EM_SH
> >>
> >>  static inline void init_thread(struct target_pt_regs *regs,
> >>                                 struct image_info *infop)
> >> @@ -1228,7 +1219,6 @@ static uint32_t get_elf_hwcap(void)
> >>  #define ELF_START_MMAP 0x80000000
> >>
> >>  #define ELF_CLASS ELFCLASS32
> >> -#define ELF_ARCH  EM_CRIS
> >>
> >>  static inline void init_thread(struct target_pt_regs *regs,
> >>                                 struct image_info *infop)
> >> @@ -1245,7 +1235,6 @@ static inline void init_thread(struct
> > target_pt_regs *regs,
> >>  #define ELF_START_MMAP 0x80000000
> >>
> >>  #define ELF_CLASS       ELFCLASS32
> >> -#define ELF_ARCH        EM_68K
> >>
> >>  /* ??? Does this need to do anything?
> >>     #define ELF_PLAT_INIT(_r) */
> >> @@ -1296,7 +1285,6 @@ static void
> elf_core_copy_regs(target_elf_gregset_t
> > *regs, const CPUM68KState *e
> >>  #define ELF_START_MMAP (0x30000000000ULL)
> >>
> >>  #define ELF_CLASS      ELFCLASS64
> >> -#define ELF_ARCH       EM_ALPHA
> >>
> >>  static inline void init_thread(struct target_pt_regs *regs,
> >>                                 struct image_info *infop)
> >> @@ -1316,7 +1304,6 @@ static inline void init_thread(struct
> > target_pt_regs *regs,
> >>
> >>  #define ELF_CLASS      ELFCLASS64
> >>  #define ELF_DATA       ELFDATA2MSB
> >> -#define ELF_ARCH       EM_S390
> >>
> >>  #define ELF_HWCAP get_elf_hwcap()
> >>
> >> @@ -1362,7 +1349,6 @@ static inline void init_thread(struct
> > target_pt_regs *regs, struct image_info *i
> >>
> >>  #define ELF_CLASS   ELFCLASS64
> >>  #define ELF_DATA    ELFDATA2LSB
> >> -#define ELF_ARCH    EM_TILEGX
> >>
> >>  static inline void init_thread(struct target_pt_regs *regs,
> >>                                 struct image_info *infop)
> >> @@ -1379,7 +1365,6 @@ static inline void init_thread(struct
> > target_pt_regs *regs,
> >>  #ifdef TARGET_RISCV
> >>
> >>  #define ELF_START_MMAP 0x80000000
> >> -#define ELF_ARCH  EM_RISCV
> >>
> >>  #ifdef TARGET_RISCV32
> >>  #define ELF_CLASS ELFCLASS32
> >> @@ -1402,7 +1387,6 @@ static inline void init_thread(struct
> > target_pt_regs *regs,
> >>
> >>  #define ELF_START_MMAP  0x80000000
> >>  #define ELF_CLASS       ELFCLASS32
> >> -#define ELF_ARCH        EM_PARISC
> >>  #define ELF_PLATFORM    "PARISC"
> >>  #define STACK_GROWS_DOWN 0
> >>  #define STACK_ALIGNMENT  64
> >> @@ -1427,7 +1411,6 @@ static inline void init_thread(struct
> > target_pt_regs *regs,
> >>  #define ELF_START_MMAP 0x20000000
> >>
> >>  #define ELF_CLASS       ELFCLASS32
> >> -#define ELF_ARCH        EM_XTENSA
> >>
> >>  static inline void init_thread(struct target_pt_regs *regs,
> >>                                 struct image_info *infop)
> >> --
> >> 2.20.1
> >>
> >>
>
>
> --
> Alex Bennée
>

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [Qemu-devel] [PATCH v1 4/4] elf: move ELF_ARCH definition to elf-arch.h
  2019-09-11  9:26     ` Alex Bennée
  2019-09-13 14:45       ` Aleksandar Markovic
@ 2019-09-14 15:52       ` Richard Henderson
  2019-09-14 17:51         ` Alex Bennée
  1 sibling, 1 reply; 24+ messages in thread
From: Richard Henderson @ 2019-09-14 15:52 UTC (permalink / raw)
  To: Alex Bennée, Aleksandar Markovic
  Cc: peter.maydell, Riku Voipio, qemu-devel, qemu-arm, Laurent Vivier

On 9/11/19 5:26 AM, Alex Bennée wrote:
> 
> Aleksandar Markovic <aleksandar.m.mail@gmail.com> writes:
> 
>> 10.09.2019. 21.34, "Alex Bennée" <alex.bennee@linaro.org> је написао/ла:
>>>
>>> This is preparatory for plugins which will want to report the
>>> architecture to plugins. Move the ELF_ARCH definition out of the
>>> loader and into its own header.
>>>
>>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>>> --
>>
>> Hi, Alex.
>>
>> I advice some caution here
>> . For example, EM_NANOMIPS, and some other EM_xxx constants are not
>> included in the new header.
> 
> EM_ARCH was never set to EM_NANOMIPS. In fact I can see e_machine is
> checked against it so I guess it is only ever used to checking the
> loading elf directly.
> 
> In fact EM_ARCH is only really the default fallback case for checking
> the elf type if there is not a more "forgiving" check done by arch
> specific code (elf_check_arch). The only other cace is the fallback for
> EM_MACHINE unless PPC does something with it.
> 
>> I am not sure what exactly you want to achieve
>> with this refactoring. But you may miss your goal, since some EM_xxx will
>> be missing from your new header. Is this the right way to achieve what you
>> want, and could you unintentionally introduce bugs? Can you outline in more
>> details your intentions around the new header? Is this refactoring the only
>> way?
> 
> As mentioned in the other reply maybe exposing a value from configure
> into config-target.[mak|h] would be a better approach?

I think using EM_FOO to tell a plugin about the architecture is just going to
be confusing.  As seen here wrt EM_NANOMIPS, but arguably elsewhere with
EM_SPARC vs EM_SPARC64.

You need to decide what it is that you want the plugin to know.  It is just be
gross architecture?  In which case QEMU_ARCH_FOO is probably enough.  Is it the
instruction set currently executing?  In which case neither EM_FOO nor
QEMU_ARCH_FOO is sufficient -- e.g. arm thumb -- and you'll have to invent
something new, because no two architectures handle this in the same way.  The
closest you might be able to get without invention from whole cloth is the
capstone cap_arch+cap_mode values from cc->disas_set_info().  But that only
handles the most popular of targets.

In any case, a single header that every target must touch is the wrong
approach.  If you want to do some cleanup, move these defines into e.g.
linux-user/$TARGET/target_elf.h.  (And I wouldn't bother touching bsd-user in
its current condition.)


r~


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [Qemu-devel] [PATCH v1 4/4] elf: move ELF_ARCH definition to elf-arch.h
  2019-09-14 15:52       ` Richard Henderson
@ 2019-09-14 17:51         ` Alex Bennée
  2019-09-14 18:19           ` Richard Henderson
  0 siblings, 1 reply; 24+ messages in thread
From: Alex Bennée @ 2019-09-14 17:51 UTC (permalink / raw)
  To: Richard Henderson
  Cc: peter.maydell, Riku Voipio, qemu-devel, Laurent Vivier, qemu-arm,
	Aleksandar Markovic


Richard Henderson <richard.henderson@linaro.org> writes:

> On 9/11/19 5:26 AM, Alex Bennée wrote:
>>
>> Aleksandar Markovic <aleksandar.m.mail@gmail.com> writes:
>>
>>> 10.09.2019. 21.34, "Alex Bennée" <alex.bennee@linaro.org> је написао/ла:
>>>>
>>>> This is preparatory for plugins which will want to report the
>>>> architecture to plugins. Move the ELF_ARCH definition out of the
>>>> loader and into its own header.
>>>>
>>>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>>>> --
>>>
>>> Hi, Alex.
>>>
>>> I advice some caution here
>>> . For example, EM_NANOMIPS, and some other EM_xxx constants are not
>>> included in the new header.
>>
>> EM_ARCH was never set to EM_NANOMIPS. In fact I can see e_machine is
>> checked against it so I guess it is only ever used to checking the
>> loading elf directly.
>>
>> In fact EM_ARCH is only really the default fallback case for checking
>> the elf type if there is not a more "forgiving" check done by arch
>> specific code (elf_check_arch). The only other cace is the fallback for
>> EM_MACHINE unless PPC does something with it.
>>
>>> I am not sure what exactly you want to achieve
>>> with this refactoring. But you may miss your goal, since some EM_xxx will
>>> be missing from your new header. Is this the right way to achieve what you
>>> want, and could you unintentionally introduce bugs? Can you outline in more
>>> details your intentions around the new header? Is this refactoring the only
>>> way?
>>
>> As mentioned in the other reply maybe exposing a value from configure
>> into config-target.[mak|h] would be a better approach?
>
> I think using EM_FOO to tell a plugin about the architecture is just going to
> be confusing.  As seen here wrt EM_NANOMIPS, but arguably elsewhere with
> EM_SPARC vs EM_SPARC64.
>
> You need to decide what it is that you want the plugin to know.  It is just be
> gross architecture?  In which case QEMU_ARCH_FOO is probably enough.  Is it the
> instruction set currently executing?  In which case neither EM_FOO nor
> QEMU_ARCH_FOO is sufficient -- e.g. arm thumb -- and you'll have to invent
> something new, because no two architectures handle this in the same way.  The
> closest you might be able to get without invention from whole cloth is the
> capstone cap_arch+cap_mode values from cc->disas_set_info().  But that only
> handles the most popular of targets.

I think I'm going to stick with the gross TARGET for now. It mostly
seems a way of avoiding building per-arch plugins. I assume most out of
tree plugins will be targeted at a specific machine anyway.

Anyway I think that means I'll drop this series unless 1-3 are worth
keeping as simple clean-ups leaving out 4?

>
> In any case, a single header that every target must touch is the wrong
> approach.  If you want to do some cleanup, move these defines into e.g.
> linux-user/$TARGET/target_elf.h.  (And I wouldn't bother touching bsd-user in
> its current condition.)
>
>
> r~


--
Alex Bennée


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [Qemu-devel] [PATCH v1 2/4] elf: move elf.h to elf/elf.h and split out types
  2019-09-10 19:34 ` [Qemu-devel] [PATCH v1 2/4] elf: move elf.h to elf/elf.h and split out types Alex Bennée
  2019-09-11  0:08   ` David Gibson
  2019-09-11  8:29   ` BALATON Zoltan
@ 2019-09-14 18:15   ` Richard Henderson
  2019-10-21 13:53   ` Laurent Vivier
  3 siblings, 0 replies; 24+ messages in thread
From: Richard Henderson @ 2019-09-14 18:15 UTC (permalink / raw)
  To: Alex Bennée, peter.maydell
  Cc: Chris Wulff, Sagar Karandikar, Michael S. Tsirkin, Anthony Green,
	Palmer Dabbelt, Mark Cave-Ayland, qemu-devel, Max Filippov,
	KONRAD Frederic, Alistair Francis, Edgar E. Iglesias,
	Marek Vasut, Jia Liu, Aleksandar Rikalo, Helge Deller,
	David Hildenbrand, Halil Pasic, Christian Borntraeger,
	Hervé Poussineau, Marc-André Lureau, Richard Henderson,
	Artyom Tarasenko, Eduardo Habkost, Riku Voipio, Fabien Chouteau,
	open list:S390-ccw boot, qemu-arm, Stafford Horne, David Gibson,
	open list:RISC-V TCG CPUs, Viktor Prutyanov, Thomas Huth,
	Bastian Koppelmann, Cornelia Huck, Laurent Vivier, Michael Walle,
	open list:PReP, Aleksandar Markovic, Paolo Bonzini,
	Aurelien Jarno

On 9/10/19 3:34 PM, Alex Bennée wrote:
> Most of the users of elf.h just want the standard Elf definitions. The
> couple that want more than that want an expansion based on ELF_CLASS
> which can be used for size agnostic code. The later is moved into
> elf/elf-types.inc.h to make it clearer what it is for. While doing
> that I also removed the whitespace damage.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---

This patch is hard to follow because it moves and splits at the same time.

With this patch split into two,
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [Qemu-devel] [PATCH v1 3/4] elf: move elf_ops.h into include/elf/ and rename
  2019-09-11  8:20   ` Alex Bennée
@ 2019-09-14 18:16     ` Richard Henderson
  0 siblings, 0 replies; 24+ messages in thread
From: Richard Henderson @ 2019-09-14 18:16 UTC (permalink / raw)
  To: Alex Bennée, peter.maydell; +Cc: qemu-arm, qemu-devel

On 9/11/19 4:20 AM, Alex Bennée wrote:
> Alex Bennée <alex.bennee@linaro.org> writes:
> 
>> Lets keep all the Elf manipulation bits together. Also rename the file
>> to better reflect how it is used and add a little header to the file.
>>
>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>> ---
>>  hw/core/loader.c                            | 4 ++--
> It is arguable this could be a private header in hw/core as it is only
> included in one place.
> 

Let's leave it here.


r~


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [Qemu-devel] [PATCH v1 4/4] elf: move ELF_ARCH definition to elf-arch.h
  2019-09-14 17:51         ` Alex Bennée
@ 2019-09-14 18:19           ` Richard Henderson
  0 siblings, 0 replies; 24+ messages in thread
From: Richard Henderson @ 2019-09-14 18:19 UTC (permalink / raw)
  To: Alex Bennée
  Cc: peter.maydell, Riku Voipio, qemu-devel, Laurent Vivier, qemu-arm,
	Aleksandar Markovic

On 9/14/19 1:51 PM, Alex Bennée wrote:
> I think I'm going to stick with the gross TARGET for now. It mostly
> seems a way of avoiding building per-arch plugins. I assume most out of
> tree plugins will be targeted at a specific machine anyway.

Ok.

> Anyway I think that means I'll drop this series unless 1-3 are worth
> keeping as simple clean-ups leaving out 4?

Patch 1 was supposed to be in another patch set, right?
Patch 2, split, is still a good cleanup, I think.


r~


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [Qemu-devel] [PATCH v1 2/4] elf: move elf.h to elf/elf.h and split out types
  2019-09-10 19:34 ` [Qemu-devel] [PATCH v1 2/4] elf: move elf.h to elf/elf.h and split out types Alex Bennée
                     ` (2 preceding siblings ...)
  2019-09-14 18:15   ` Richard Henderson
@ 2019-10-21 13:53   ` Laurent Vivier
  2019-10-21 14:04     ` Peter Maydell
  3 siblings, 1 reply; 24+ messages in thread
From: Laurent Vivier @ 2019-10-21 13:53 UTC (permalink / raw)
  To: Alex Bennée, peter.maydell
  Cc: Chris Wulff, Sagar Karandikar, Michael S. Tsirkin, Anthony Green,
	Palmer Dabbelt, Mark Cave-Ayland, qemu-devel, Max Filippov,
	KONRAD Frederic, Alistair Francis, Edgar E. Iglesias,
	Marek Vasut, Jia Liu, Aleksandar Rikalo, Helge Deller,
	David Hildenbrand, Halil Pasic, Christian Borntraeger,
	Hervé Poussineau, Marc-André Lureau, Richard Henderson,
	Artyom Tarasenko, Eduardo Habkost, Riku Voipio, Fabien Chouteau,
	open list:S390-ccw boot, qemu-arm, Stafford Horne, David Gibson,
	open list:RISC-V TCG CPUs, Viktor Prutyanov, Thomas Huth,
	Bastian Koppelmann, Cornelia Huck, Michael Walle, open list:PReP,
	Aleksandar Markovic, Paolo Bonzini, Aurelien Jarno

Le 10/09/2019 à 21:34, Alex Bennée a écrit :
> Most of the users of elf.h just want the standard Elf definitions. The
> couple that want more than that want an expansion based on ELF_CLASS
> which can be used for size agnostic code. The later is moved into
> elf/elf-types.inc.h to make it clearer what it is for. While doing
> that I also removed the whitespace damage.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>  bsd-user/elfload.c               |  2 +-
>  contrib/elf2dmp/qemu_elf.h       |  2 +-
>  disas.c                          |  2 +-
>  dump/dump.c                      |  2 +-
>  dump/win_dump.c                  |  2 +-
>  hw/alpha/dp264.c                 |  2 +-
>  hw/arm/armv7m.c                  |  2 +-
>  hw/arm/boot.c                    |  2 +-
>  hw/core/loader.c                 |  3 +-
>  hw/cris/axis_dev88.c             |  2 +-
>  hw/cris/boot.c                   |  2 +-
>  hw/hppa/machine.c                |  2 +-
>  hw/i386/multiboot.c              |  2 +-
>  hw/i386/pc.c                     |  2 +-
>  hw/lm32/lm32_boards.c            |  2 +-
>  hw/lm32/milkymist.c              |  2 +-
>  hw/m68k/an5206.c                 |  2 +-
>  hw/m68k/mcf5208.c                |  2 +-
>  hw/microblaze/boot.c             |  2 +-
>  hw/mips/mips_fulong2e.c          |  2 +-
>  hw/mips/mips_malta.c             |  2 +-
>  hw/mips/mips_mipssim.c           |  2 +-
>  hw/mips/mips_r4k.c               |  2 +-
>  hw/moxie/moxiesim.c              |  2 +-
>  hw/nios2/boot.c                  |  2 +-
>  hw/openrisc/openrisc_sim.c       |  2 +-
>  hw/pci-host/prep.c               |  2 +-
>  hw/ppc/e500.c                    |  2 +-
>  hw/ppc/mac_newworld.c            |  2 +-
>  hw/ppc/mac_oldworld.c            |  2 +-
>  hw/ppc/ppc440_bamboo.c           |  2 +-
>  hw/ppc/prep.c                    |  2 +-
>  hw/ppc/sam460ex.c                |  2 +-
>  hw/ppc/spapr.c                   |  2 +-
>  hw/ppc/spapr_vio.c               |  2 +-
>  hw/ppc/virtex_ml507.c            |  2 +-
>  hw/riscv/boot.c                  |  2 +-
>  hw/s390x/ipl.c                   |  2 +-
>  hw/sparc/leon3.c                 |  2 +-
>  hw/sparc/sun4m.c                 |  2 +-
>  hw/sparc64/sun4u.c               |  2 +-
>  hw/tricore/tricore_testboard.c   |  2 +-
>  hw/xtensa/sim.c                  |  2 +-
>  hw/xtensa/xtfpga.c               |  2 +-
>  include/elf/elf-types.inc.h      | 63 ++++++++++++++++++++++++++++++++
>  include/{ => elf}/elf.h          | 42 ---------------------
>  include/hw/core/generic-loader.h |  2 +-
>  linux-user/arm/cpu_loop.c        |  2 +-
>  linux-user/elfload.c             |  5 +--
>  linux-user/main.c                |  2 +-
>  linux-user/mips/cpu_loop.c       |  2 +-
>  linux-user/riscv/cpu_loop.c      |  2 +-
>  target/arm/arch_dump.c           |  2 +-
>  target/i386/arch_dump.c          |  2 +-
>  target/ppc/arch_dump.c           |  2 +-
>  target/ppc/kvm.c                 |  2 +-
>  target/s390x/arch_dump.c         |  2 +-
>  tcg/arm/tcg-target.inc.c         |  2 +-
>  tcg/ppc/tcg-target.inc.c         |  2 +-
>  tcg/s390/tcg-target.inc.c        |  2 +-
>  tcg/tcg.c                        |  5 ++-
>  util/getauxval.c                 |  2 +-
>  62 files changed, 128 insertions(+), 104 deletions(-)
>  create mode 100644 include/elf/elf-types.inc.h

The patch looks good, but why did you call the file "elf-types.inc.h"
and not "elf-types.h"?

Reviewed-by: Laurent Vivier <laurent@vivier.eu>

Thanks,
LAurent


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [Qemu-devel] [PATCH v1 3/4] elf: move elf_ops.h into include/elf/ and rename
  2019-09-10 19:34 ` [Qemu-devel] [PATCH v1 3/4] elf: move elf_ops.h into include/elf/ and rename Alex Bennée
  2019-09-11  8:20   ` Alex Bennée
@ 2019-10-21 13:56   ` Laurent Vivier
  1 sibling, 0 replies; 24+ messages in thread
From: Laurent Vivier @ 2019-10-21 13:56 UTC (permalink / raw)
  To: Alex Bennée, peter.maydell; +Cc: qemu-arm, qemu-devel

Le 10/09/2019 à 21:34, Alex Bennée a écrit :
> Lets keep all the Elf manipulation bits together. Also rename the file
> to better reflect how it is used and add a little header to the file.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>  hw/core/loader.c                            | 4 ++--
>  include/{hw/elf_ops.h => elf/elf_ops.inc.h} | 9 +++++++++
>  2 files changed, 11 insertions(+), 2 deletions(-)
>  rename include/{hw/elf_ops.h => elf/elf_ops.inc.h} (98%)

So the question is: what the ".inc" reflects for a ".h" file?

Thanks,
Laurent



^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [Qemu-devel] [PATCH v1 4/4] elf: move ELF_ARCH definition to elf-arch.h
  2019-09-10 19:34 ` [Qemu-devel] [PATCH v1 4/4] elf: move ELF_ARCH definition to elf-arch.h Alex Bennée
  2019-09-10 21:14   ` Aleksandar Markovic
  2019-09-10 21:39   ` Aleksandar Markovic
@ 2019-10-21 14:03   ` Laurent Vivier
  2 siblings, 0 replies; 24+ messages in thread
From: Laurent Vivier @ 2019-10-21 14:03 UTC (permalink / raw)
  To: Alex Bennée, peter.maydell; +Cc: Riku Voipio, qemu-devel, qemu-arm

Le 10/09/2019 à 21:34, Alex Bennée a écrit :
> This is preparatory for plugins which will want to report the
> architecture to plugins. Move the ELF_ARCH definition out of the
> loader and into its own header.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>  bsd-user/elfload.c     |  13 +----
>  include/elf/elf-arch.h | 109 +++++++++++++++++++++++++++++++++++++++++

Your file should be split across "linux-user/$(TARGET_ABI_DIR)/elf-arch.h"

Thanks,
Laurent


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [Qemu-devel] [PATCH v1 2/4] elf: move elf.h to elf/elf.h and split out types
  2019-10-21 13:53   ` Laurent Vivier
@ 2019-10-21 14:04     ` Peter Maydell
  0 siblings, 0 replies; 24+ messages in thread
From: Peter Maydell @ 2019-10-21 14:04 UTC (permalink / raw)
  To: Laurent Vivier
  Cc: Cornelia Huck, Sagar Karandikar, Michael S. Tsirkin,
	Anthony Green, Palmer Dabbelt, Mark Cave-Ayland, QEMU Developers,
	Max Filippov, KONRAD Frederic, Alistair Francis, Marek Vasut,
	Jia Liu, Aleksandar Rikalo, Helge Deller, David Hildenbrand,
	Halil Pasic, Christian Borntraeger, Hervé Poussineau,
	Marc-André Lureau, David Gibson, Artyom Tarasenko,
	Eduardo Habkost, Riku Voipio, Fabien Chouteau,
	open list:S390-ccw boot, qemu-arm, Stafford Horne,
	Alex Bennée, Richard Henderson, open list:RISC-V TCG CPUs,
	Viktor Prutyanov, Thomas Huth, Bastian Koppelmann, Chris Wulff,
	Michael Walle, open list:PReP, Aleksandar Markovic,
	Paolo Bonzini, Aurelien Jarno

On Mon, 21 Oct 2019 at 14:54, Laurent Vivier <laurent@vivier.eu> wrote:
>
> Le 10/09/2019 à 21:34, Alex Bennée a écrit :
> > Most of the users of elf.h just want the standard Elf definitions. The
> > couple that want more than that want an expansion based on ELF_CLASS
> > which can be used for size agnostic code. The later is moved into
> > elf/elf-types.inc.h to make it clearer what it is for. While doing
> > that I also removed the whitespace damage.
> >

> The patch looks good, but why did you call the file "elf-types.inc.h"
> and not "elf-types.h"?

This is our usual convention for files (well, that or .inc.c)
which get included multiple times with different preprocessor
defines to specify how they behave, i.e. which aren't standard
"simple header with multiple-inclusion protection guards".

thanks
-- PMM


^ permalink raw reply	[flat|nested] 24+ messages in thread

end of thread, other threads:[~2019-10-21 14:06 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-10 19:34 [Qemu-devel] [PATCH v1 0/4] ELF and (macro) safety Alex Bennée
2019-09-10 19:34 ` [Qemu-devel] [PATCH v1 1/4] target/ppc: fix signal delivery for ppc64abi32 Alex Bennée
2019-09-10 19:45   ` Alex Bennée
2019-09-10 19:34 ` [Qemu-devel] [PATCH v1 2/4] elf: move elf.h to elf/elf.h and split out types Alex Bennée
2019-09-11  0:08   ` David Gibson
2019-09-11  8:29   ` BALATON Zoltan
2019-09-11  9:19     ` Alex Bennée
2019-09-14 18:15   ` Richard Henderson
2019-10-21 13:53   ` Laurent Vivier
2019-10-21 14:04     ` Peter Maydell
2019-09-10 19:34 ` [Qemu-devel] [PATCH v1 3/4] elf: move elf_ops.h into include/elf/ and rename Alex Bennée
2019-09-11  8:20   ` Alex Bennée
2019-09-14 18:16     ` Richard Henderson
2019-10-21 13:56   ` Laurent Vivier
2019-09-10 19:34 ` [Qemu-devel] [PATCH v1 4/4] elf: move ELF_ARCH definition to elf-arch.h Alex Bennée
2019-09-10 21:14   ` Aleksandar Markovic
2019-09-11  9:26     ` Alex Bennée
2019-09-13 14:45       ` Aleksandar Markovic
2019-09-14 15:52       ` Richard Henderson
2019-09-14 17:51         ` Alex Bennée
2019-09-14 18:19           ` Richard Henderson
2019-09-10 21:39   ` Aleksandar Markovic
2019-09-11  8:19     ` Alex Bennée
2019-10-21 14:03   ` Laurent Vivier

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).