| Message ID | 1364985128-23772-6-git-send-email-pbonzini@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 3 April 2013 11:32, Paolo Bonzini <pbonzini@redhat.com> wrote: > The alignment is a characteristic of the ABI, not the CPU. > > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > --- a/target-m68k/cpu.h > +++ b/target-m68k/cpu.h > @@ -21,6 +21,7 @@ > #define CPU_M68K_H > > #define TARGET_LONG_BITS 32 > +#define ABI_INT_ALIGNMENT 2 > > #define CPUArchState struct CPUM68KState I think this define should go in an include file in linux-user/m68k/ -- it's ABI specific and we should be aiming to isolate ABI specific info in linux-user/ rather than having it leaking into target-* and thus into the system emulation code. Otherwise patch looks good. thanks -- PMM
Il 04/04/2013 15:56, Peter Maydell ha scritto: >> > #define TARGET_LONG_BITS 32 >> > +#define ABI_INT_ALIGNMENT 2 >> > >> > #define CPUArchState struct CPUM68KState > I think this define should go in an include file in > linux-user/m68k/ -- it's ABI specific and we should > be aiming to isolate ABI specific info in linux-user/ > rather than having it leaking into target-* and thus > into the system emulation code. > > Otherwise patch looks good. That would create 10 empty files, and two files between one and three lines of code. I would just put those in include/exec/user/abitypes.h. WDYT? Paolo
On 4 April 2013 15:04, Paolo Bonzini <pbonzini@redhat.com> wrote: > Il 04/04/2013 15:56, Peter Maydell ha scritto: >> I think this define should go in an include file in >> linux-user/m68k/ -- it's ABI specific and we should >> be aiming to isolate ABI specific info in linux-user/ >> rather than having it leaking into target-* and thus >> into the system emulation code. >> >> Otherwise patch looks good. > > That would create 10 empty files, and two files between one and three > lines of code. I would just put those in include/exec/user/abitypes.h. I would prefer not to add more code with TARGET ifdefs. (I have a vague plan to split up some of the current linux-user code which is #ifdefs in source files in linux-user into per target source files which make just pulls in as appropriate.) Also, separate files in linux-user/foo encourage authors of new targets to think "do I need to put something in my new target's version of this?" whereas #ifdef TARGET_FOO in common source files don't. Riku's the linux-user maintainer, though... thanks -- PMM
Il 04/04/2013 16:22, Peter Maydell ha scritto: >> > >> > That would create 10 empty files, and two files between one and three >> > lines of code. I would just put those in include/exec/user/abitypes.h. > I would prefer not to add more code with TARGET ifdefs. > (I have a vague plan to split up some of the current linux-user > code which is #ifdefs in source files in linux-user into per > target source files which make just pulls in as appropriate.) > > Also, separate files in linux-user/foo encourage authors of > new targets to think "do I need to put something in my new > target's version of this?" whereas #ifdef TARGET_FOO in > common source files don't. I can't deny these are all good arguments. I'm not sure if abitypes would be the best or the worst place to start doing this conversion. Paolo > Riku's the linux-user maintainer, though...
diff --git a/configure b/configure index 68314d1..8276e79 100755 --- a/configure +++ b/configure @@ -4011,7 +4011,6 @@ bflt="no" target_nptl="no" interp_prefix1=`echo "$interp_prefix" | sed "s/%M/$target_arch2/g"` gdb_xml_files="" -target_int_alignment=4 target_long_alignment=4 target_llong_alignment=8 target_libs_softmmu= @@ -4047,7 +4046,6 @@ case "$target_arch2" in m68k) bflt="yes" gdb_xml_files="cf-core.xml cf-fp.xml" - target_int_alignment=2 target_long_alignment=2 target_llong_alignment=2 ;; @@ -4153,7 +4151,6 @@ case "$cpu" in ;; esac -echo "TARGET_INT_ALIGNMENT=$target_int_alignment" >> $config_target_mak echo "TARGET_LONG_ALIGNMENT=$target_long_alignment" >> $config_target_mak echo "TARGET_LLONG_ALIGNMENT=$target_llong_alignment" >> $config_target_mak echo "TARGET_ARCH=$TARGET_ARCH" >> $config_target_mak diff --git a/include/exec/cpu-defs.h b/include/exec/cpu-defs.h index d376f0f..2aa9331 100644 --- a/include/exec/cpu-defs.h +++ b/include/exec/cpu-defs.h @@ -36,8 +36,6 @@ #define TARGET_LONG_SIZE (TARGET_LONG_BITS / 8) -typedef int32_t target_int __attribute__((aligned(TARGET_INT_ALIGNMENT))); -typedef uint32_t target_uint __attribute__((aligned(TARGET_INT_ALIGNMENT))); typedef int64_t target_llong __attribute__((aligned(TARGET_LLONG_ALIGNMENT))); typedef uint64_t target_ullong __attribute__((aligned(TARGET_LLONG_ALIGNMENT))); /* target_ulong is the type of a virtual address */ diff --git a/include/exec/user/abitypes.h b/include/exec/user/abitypes.h index abaa028..97a87a6 100644 --- a/include/exec/user/abitypes.h +++ b/include/exec/user/abitypes.h @@ -5,9 +5,14 @@ #ifndef ABI_SHORT_ALIGNMENT #define ABI_SHORT_ALIGNMENT 2 #endif +#ifndef ABI_INT_ALIGNMENT +#define ABI_INT_ALIGNMENT 4 +#endif typedef int16_t abi_short __attribute__ ((aligned(ABI_SHORT_ALIGNMENT))); typedef uint16_t abi_ushort __attribute__((aligned(ABI_SHORT_ALIGNMENT))); +typedef int32_t abi_int __attribute__((aligned(ABI_INT_ALIGNMENT))); +typedef uint32_t abi_uint __attribute__((aligned(ABI_INT_ALIGNMENT))); #ifdef TARGET_ABI32 typedef uint32_t abi_ulong; diff --git a/linux-user/elfload.c b/linux-user/elfload.c index 5eca934..14a8ecf 100644 --- a/linux-user/elfload.c +++ b/linux-user/elfload.c @@ -113,10 +113,10 @@ typedef abi_ulong target_elf_greg_t; typedef abi_ushort target_uid_t; typedef abi_ushort target_gid_t; #else -typedef target_uint target_uid_t; -typedef target_uint target_gid_t; +typedef abi_uint target_uid_t; +typedef abi_uint target_gid_t; #endif -typedef target_int target_pid_t; +typedef abi_int target_pid_t; #ifdef TARGET_I386 @@ -2109,9 +2109,9 @@ struct memelfnote { }; struct target_elf_siginfo { - target_int si_signo; /* signal number */ - target_int si_code; /* extra code */ - target_int si_errno; /* errno */ + abi_int si_signo; /* signal number */ + abi_int si_code; /* extra code */ + abi_int si_errno; /* errno */ }; struct target_elf_prstatus { @@ -2128,7 +2128,7 @@ struct target_elf_prstatus { struct target_timeval pr_cutime; /* XXX Cumulative user time */ struct target_timeval pr_cstime; /* XXX Cumulative system time */ target_elf_gregset_t pr_reg; /* GP registers */ - target_int pr_fpvalid; /* XXX */ + abi_int pr_fpvalid; /* XXX */ }; #define ELF_PRARGSZ (80) /* Number of chars for args */ diff --git a/target-m68k/cpu.h b/target-m68k/cpu.h index c90c40c..29cc887 100644 --- a/target-m68k/cpu.h +++ b/target-m68k/cpu.h @@ -21,6 +21,7 @@ #define CPU_M68K_H #define TARGET_LONG_BITS 32 +#define ABI_INT_ALIGNMENT 2 #define CPUArchState struct CPUM68KState
The alignment is a characteristic of the ABI, not the CPU. Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> --- configure | 3 --- include/exec/cpu-defs.h | 2 -- include/exec/user/abitypes.h | 5 +++++ linux-user/elfload.c | 14 +++++++------- target-m68k/cpu.h | 1 + 5 files changed, 13 insertions(+), 12 deletions(-)