From: "Alex Bennée" <alex.bennee@linaro.org>
To: Aleksandar Markovic <aleksandar.m.mail@gmail.com>
Cc: peter.maydell@linaro.org, Riku Voipio <riku.voipio@iki.fi>,
qemu-devel@nongnu.org, qemu-arm@nongnu.org,
Laurent Vivier <laurent@vivier.eu>
Subject: Re: [Qemu-devel] [PATCH v1 4/4] elf: move ELF_ARCH definition to elf-arch.h
Date: Wed, 11 Sep 2019 10:26:37 +0100 [thread overview]
Message-ID: <874l1j18vm.fsf@linaro.org> (raw)
In-Reply-To: <CAL1e-=giNc1e+kN04KqHFQGkJbP4BKziQyxVd3-RTtL+UOfSoQ@mail.gmail.com>
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
next prev parent reply other threads:[~2019-09-11 9:27 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-09-10 19:34 [Qemu-devel] [PATCH v1 0/4] ELF and (macro) safety Alex Bennée
2019-09-10 19:34 ` [Qemu-devel] [PATCH v1 1/4] target/ppc: fix signal delivery for ppc64abi32 Alex Bennée
2019-09-10 19:45 ` Alex Bennée
2019-09-10 19:34 ` [Qemu-devel] [PATCH v1 2/4] elf: move elf.h to elf/elf.h and split out types Alex Bennée
2019-09-11 0:08 ` David Gibson
2019-09-11 8:29 ` BALATON Zoltan
2019-09-11 9:19 ` Alex Bennée
2019-09-14 18:15 ` Richard Henderson
2019-10-21 13:53 ` Laurent Vivier
2019-10-21 14:04 ` Peter Maydell
2019-09-10 19:34 ` [Qemu-devel] [PATCH v1 3/4] elf: move elf_ops.h into include/elf/ and rename Alex Bennée
2019-09-11 8:20 ` Alex Bennée
2019-09-14 18:16 ` Richard Henderson
2019-10-21 13:56 ` Laurent Vivier
2019-09-10 19:34 ` [Qemu-devel] [PATCH v1 4/4] elf: move ELF_ARCH definition to elf-arch.h Alex Bennée
2019-09-10 21:14 ` Aleksandar Markovic
2019-09-11 9:26 ` Alex Bennée [this message]
2019-09-13 14:45 ` Aleksandar Markovic
2019-09-14 15:52 ` Richard Henderson
2019-09-14 17:51 ` Alex Bennée
2019-09-14 18:19 ` Richard Henderson
2019-09-10 21:39 ` Aleksandar Markovic
2019-09-11 8:19 ` Alex Bennée
2019-10-21 14:03 ` Laurent Vivier
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=874l1j18vm.fsf@linaro.org \
--to=alex.bennee@linaro.org \
--cc=aleksandar.m.mail@gmail.com \
--cc=laurent@vivier.eu \
--cc=peter.maydell@linaro.org \
--cc=qemu-arm@nongnu.org \
--cc=qemu-devel@nongnu.org \
--cc=riku.voipio@iki.fi \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).