Patchwork [v2,05/10] elfload: use abi_int/uint instead of target_int/uint

login
register
mail settings
Submitter Paolo Bonzini
Date April 3, 2013, 10:32 a.m.
Message ID <1364985128-23772-6-git-send-email-pbonzini@redhat.com>
Download mbox | patch
Permalink /patch/233352/
State New
Headers show

Comments

Paolo Bonzini - April 3, 2013, 10:32 a.m.
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(-)
Peter Maydell - April 4, 2013, 1:56 p.m.
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
Paolo Bonzini - April 4, 2013, 2:04 p.m.
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
Peter Maydell - April 4, 2013, 2:22 p.m.
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
Paolo Bonzini - April 4, 2013, 2:28 p.m.
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...

Patch

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