Re: [Qemu-devel] [PATCH v1 4/4] elf: move ELF_ARCH definition to elf-arch.h
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 > --- > 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
Re: [Qemu-devel] [PATCH v1 4/4] elf: move ELF_ARCH definition to elf-arch.h
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~
Re: [Qemu-devel] [PATCH v1 4/4] elf: move ELF_ARCH definition to elf-arch.h
Richard Henderson writes: > On 9/11/19 5:26 AM, Alex Bennée wrote: >> >> Aleksandar Markovic writes: >> >>> 10.09.2019. 21.34, "Alex Bennée" је написао/ла: 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 -- >>> >>> 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
Re: [Qemu-devel] [PATCH v1 4/4] elf: move ELF_ARCH definition to elf-arch.h
On 9/11/19 5:26 AM, Alex Bennée wrote: > > Aleksandar Markovic writes: > >> 10.09.2019. 21.34, "Alex Bennée" је написао/ла: >>> >>> 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 >>> -- >> >> 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~
Re: [Qemu-devel] [PATCH v1 4/4] elf: move ELF_ARCH definition to elf-arch.h
On Wed, Sep 11, 2019 at 11:26 AM Alex Bennée wrote:
>
> Aleksandar Markovic writes:
>
> > 10.09.2019. 21.34, "Alex Bennée" је написао/ла:
> >>
> >> 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
> >> --
> >
> > 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_DATAELFDATA2LSB
> >> -#define ELF_ARCHEM_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_DATAELFDATA2LSB
> >> #endif
> >> -#define ELF_ARCHEM_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_DATAELFDATA2MSB
> >> -#define ELF_ARCHEM_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_DATAELFDATA2MSB
> >> -#define ELF_ARCHEM_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_DATAELFDATA2LSB
> >> #endif
> >> -#define ELF_ARCHEM_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
> >> #defin
Re: [Qemu-devel] [PATCH v1 4/4] elf: move ELF_ARCH definition to elf-arch.h
Aleksandar Markovic writes:
> 10.09.2019. 21.34, "Alex Bennée" је написао/ла:
>>
>> 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
>> --
>
> 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_DATAELFDATA2LSB
>> -#define ELF_ARCHEM_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_DATAELFDATA2LSB
>> #endif
>> -#define ELF_ARCHEM_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_DATAELFDATA2MSB
>> -#define ELF_ARCHEM_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_DATAELFDATA2MSB
>> -#define ELF_ARCHEM_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_DATAELFDATA2LSB
>> #endif
>> -#define ELF_ARCHEM_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_DATAELFDATA2LSB
>> #endif
>> -#define ELF_ARCHEM_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_DATAELFDATA2MSB
>> -#define ELF_ARCHEM_68K
>>
>> /* ??? Does this need to do anything?
>> #define ELF_P
Re: [Qemu-devel] [PATCH v1 4/4] elf: move ELF_ARCH definition to elf-arch.h
Aleksandar Markovic writes:
> 10.09.2019. 21.34, "Alex Bennée" је написао/ла:
>>
>> 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
>> ---
>
> 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.ben...@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_DATAELFDATA2LSB
>> -#define ELF_ARCHEM_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_DATAELFDATA2LSB
>> #endif
>> -#define ELF_ARCHEM_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_DATAELFDATA2MSB
>> -#define ELF_ARCHEM_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_DATAELFDATA2MSB
>> -#define ELF_ARCHEM_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_DATAELFDATA2LSB
>> #endif
>> -#define ELF_ARCHEM_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_DATAELFDATA2LSB
>> #endif
>> -#define ELF_ARCHEM_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 ELFDATA2
Re: [Qemu-devel] [PATCH v1 4/4] elf: move ELF_ARCH definition to elf-arch.h
10.09.2019. 21.34, "Alex Bennée" је написао/ла:
>
> 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
> ---
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_DATAELFDATA2LSB
> -#define ELF_ARCHEM_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_DATAELFDATA2LSB
> #endif
> -#define ELF_ARCHEM_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_DATAELFDATA2MSB
> -#define ELF_ARCHEM_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_DATAELFDATA2MSB
> -#define ELF_ARCHEM_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_DATAELFDATA2LSB
> #endif
> -#define ELF_ARCHEM_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_DATAELFDATA2LSB
> #endif
> -#define ELF_ARCHEM_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_DATAELFDATA2MSB
> -#define ELF_ARCHEM_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 000..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.
> +
Re: [Qemu-devel] [PATCH v1 4/4] elf: move ELF_ARCH definition to elf-arch.h
10.09.2019. 21.34, "Alex Bennée" је написао/ла:
>
> 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
> --
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_DATAELFDATA2LSB
> -#define ELF_ARCHEM_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_DATAELFDATA2LSB
> #endif
> -#define ELF_ARCHEM_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_DATAELFDATA2MSB
> -#define ELF_ARCHEM_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_DATAELFDATA2MSB
> -#define ELF_ARCHEM_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_DATAELFDATA2LSB
> #endif
> -#define ELF_ARCHEM_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_DATAELFDATA2LSB
> #endif
> -#define ELF_ARCHEM_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_DATAELFDATA2MSB
> -#define ELF_ARCHEM_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 000..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
