diff mbox series

[v1,4/4] elf: move ELF_ARCH definition to elf-arch.h

Message ID 20190910193408.28917-5-alex.bennee@linaro.org
State New
Headers show
Series ELF and (macro) safety | expand

Commit Message

Alex Bennée Sept. 10, 2019, 7:34 p.m. UTC
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

-- 
2.20.1

Comments

Aleksandar Markovic Sept. 10, 2019, 9:14 p.m. UTC | #1
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

>

>
Aleksandar Markovic Sept. 10, 2019, 9:39 p.m. UTC | #2
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

>

>
Alex Bennée Sept. 11, 2019, 8:19 a.m. UTC | #3
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
Alex Bennée Sept. 11, 2019, 9:26 a.m. UTC | #4
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
Aleksandar Markovic Sept. 13, 2019, 2:45 p.m. UTC | #5
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

>
Richard Henderson Sept. 14, 2019, 3:52 p.m. UTC | #6
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~
Alex Bennée Sept. 14, 2019, 5:51 p.m. UTC | #7
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
Richard Henderson Sept. 14, 2019, 6:19 p.m. UTC | #8
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~
Laurent Vivier Oct. 21, 2019, 2:03 p.m. UTC | #9
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
diff mbox series

Patch

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)