[Qemu-devel] [PATCH v2 00/10] TARGET_*_ALIGNMENT cleanup, and config-target.mak trimming

[Qemu-devel] [PATCH v2 00/10] TARGET_*_ALIGNMENT cleanup, and config-target.mak trimming

[Paolo Bonzini]

This series expands on the one I sent yesterday, but also fixes some
confusion between target (CPU) and ABI types in linux-user/elfload.c.

target_short/int/llong and the corresponding unsigned types are renamed
to abi_short/int/llong, and TARGET_*_ALIGNMENT is similarly renamed to
ABI_*_ALIGNMENT.  target_long/ulong, which identify the register width
in the CPU, do not have special alignment anymore.  Core dumps are
changed to use the new types, with fixes to include the full 64-bit
contents of mipsn32 registers.

Patches 1-3 are new.  Patches 4-7 are also new, and replace patches 1-2
in the previous submission.  Patches 8-10 are the same as patches 3-5
in the previous submission, with a fix for -lm link errors in patch 9.

Paolo

Paolo Bonzini (10):
  elfload: fix size of ABI-dependent fields in core dumps
  elfload: fix size of registers for N32
  elfload: use tswapreg consistently in elf_core_copy_regs
  elfload: use abi_short/ushort instead of target_short/ushort
  elfload: use abi_int/uint instead of target_int/uint
  elfload: only give abi_long/ulong the alignment specified by the target
  elfload: use abi_llong/ullong instead of target_llong/ullong
  configure: move CONFIG_QEMU_LDST_OPTIMIZATION to config-host.mak
  configure: move common libraries to config-host.mak
  configure: CONFIG_NO_XEN is duplicated

 Makefile.target              |  10 +--
 configure                    |  43 +++--------
 hw/xen.h                     |   2 +-
 include/exec/cpu-defs.h      |  14 +---
 include/exec/user/abitypes.h |  36 +++++++--
 linux-user/elfload.c         | 175 ++++++++++++++++++++++---------------------
 target-arm/cpu.h             |   1 +
 target-m68k/cpu.h            |   3 +
 8 files changed, 147 insertions(+), 137 deletions(-)

-- 
1.8.1.4

[Qemu-devel] [PATCH v2 01/10] elfload: fix size of ABI-dependent fields in core dumps

[Paolo Bonzini]

Some fields in core dumps are 32-bit in 32-or-64 environments (ppc64abi32,
sparc32plus).  Use abi_long/ulong for those.

Also, the fields of target_elf_siginfo are ints.  Use tswap32 to convert them.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 linux-user/elfload.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/linux-user/elfload.c b/linux-user/elfload.c
index 89db49c..d3589ff 100644
--- a/linux-user/elfload.c
+++ b/linux-user/elfload.c
@@ -2110,8 +2110,8 @@ struct target_elf_siginfo {
 struct target_elf_prstatus {
     struct target_elf_siginfo pr_info;      /* Info associated with signal */
     target_short       pr_cursig;    /* Current signal */
-    target_ulong       pr_sigpend;   /* XXX */
-    target_ulong       pr_sighold;   /* XXX */
+    abi_ulong          pr_sigpend;   /* XXX */
+    abi_ulong          pr_sighold;   /* XXX */
     target_pid_t       pr_pid;
     target_pid_t       pr_ppid;
     target_pid_t       pr_pgrp;
@@ -2131,7 +2131,7 @@ struct target_elf_prpsinfo {
     char         pr_sname;       /* char for pr_state */
     char         pr_zomb;        /* zombie */
     char         pr_nice;        /* nice val */
-    target_ulong pr_flag;        /* flags */
+    abi_ulong    pr_flag;        /* flags */
     target_uid_t pr_uid;
     target_gid_t pr_gid;
     target_pid_t pr_pid, pr_ppid, pr_pgrp, pr_sid;
@@ -2215,12 +2215,12 @@ static int write_note_info(struct elf_note_info *, int);
 #ifdef BSWAP_NEEDED
 static void bswap_prstatus(struct target_elf_prstatus *prstatus)
 {
-    prstatus->pr_info.si_signo = tswapl(prstatus->pr_info.si_signo);
-    prstatus->pr_info.si_code = tswapl(prstatus->pr_info.si_code);
-    prstatus->pr_info.si_errno = tswapl(prstatus->pr_info.si_errno);
+    prstatus->pr_info.si_signo = tswap32(prstatus->pr_info.si_signo);
+    prstatus->pr_info.si_code = tswap32(prstatus->pr_info.si_code);
+    prstatus->pr_info.si_errno = tswap32(prstatus->pr_info.si_errno);
     prstatus->pr_cursig = tswap16(prstatus->pr_cursig);
-    prstatus->pr_sigpend = tswapl(prstatus->pr_sigpend);
-    prstatus->pr_sighold = tswapl(prstatus->pr_sighold);
+    prstatus->pr_sigpend = tswapal(prstatus->pr_sigpend);
+    prstatus->pr_sighold = tswapal(prstatus->pr_sighold);
     prstatus->pr_pid = tswap32(prstatus->pr_pid);
     prstatus->pr_ppid = tswap32(prstatus->pr_ppid);
     prstatus->pr_pgrp = tswap32(prstatus->pr_pgrp);
@@ -2232,7 +2232,7 @@ static void bswap_prstatus(struct target_elf_prstatus *prstatus)
 
 static void bswap_psinfo(struct target_elf_prpsinfo *psinfo)
 {
-    psinfo->pr_flag = tswapl(psinfo->pr_flag);
+    psinfo->pr_flag = tswapal(psinfo->pr_flag);
     psinfo->pr_uid = tswap16(psinfo->pr_uid);
     psinfo->pr_gid = tswap16(psinfo->pr_gid);
     psinfo->pr_pid = tswap32(psinfo->pr_pid);
-- 
1.8.1.4

Re: [Qemu-devel] [PATCH v2 01/10] elfload: fix size of ABI-dependent fields in core dumps

[Peter Maydell]

On 3 April 2013 11:31, Paolo Bonzini <pbonzini@redhat.com> wrote:
> Some fields in core dumps are 32-bit in 32-or-64 environments (ppc64abi32,
> sparc32plus).  Use abi_long/ulong for those.
>
> Also, the fields of target_elf_siginfo are ints.  Use tswap32 to convert them.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

-- PMM

[Qemu-devel] [PATCH v2 02/10] elfload: fix size of registers for N32

[Paolo Bonzini]

Registers are 64-bit in size for the MIPS n32 ABI.  Define
target_elf_greg_t accordingly, and use the correct function
to do endian swaps.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 linux-user/elfload.c | 21 ++++++++++++++-------
 1 file changed, 14 insertions(+), 7 deletions(-)

diff --git a/linux-user/elfload.c b/linux-user/elfload.c
index d3589ff..9d5dbb8 100644
--- a/linux-user/elfload.c
+++ b/linux-user/elfload.c
@@ -101,7 +101,14 @@ enum {
 #define ELF_DATA        ELFDATA2LSB
 #endif
 
+#ifdef TARGET_ABI_MIPSN32
 typedef target_ulong    target_elf_greg_t;
+#define tswapreg(ptr)   tswapl(ptr)
+#else
+typedef abi_ulong       target_elf_greg_t;
+#define tswapreg(ptr)   tswapal(ptr)
+#endif
+
 #ifdef USE_UID16
 typedef target_ushort   target_uid_t;
 typedef target_ushort   target_gid_t;
@@ -747,17 +754,17 @@ static void elf_core_copy_regs(target_elf_gregset_t *regs, const CPUMIPSState *e
     (*regs)[TARGET_EF_R0] = 0;
 
     for (i = 1; i < ARRAY_SIZE(env->active_tc.gpr); i++) {
-        (*regs)[TARGET_EF_R0 + i] = tswapl(env->active_tc.gpr[i]);
+        (*regs)[TARGET_EF_R0 + i] = tswapreg(env->active_tc.gpr[i]);
     }
 
     (*regs)[TARGET_EF_R26] = 0;
     (*regs)[TARGET_EF_R27] = 0;
-    (*regs)[TARGET_EF_LO] = tswapl(env->active_tc.LO[0]);
-    (*regs)[TARGET_EF_HI] = tswapl(env->active_tc.HI[0]);
-    (*regs)[TARGET_EF_CP0_EPC] = tswapl(env->active_tc.PC);
-    (*regs)[TARGET_EF_CP0_BADVADDR] = tswapl(env->CP0_BadVAddr);
-    (*regs)[TARGET_EF_CP0_STATUS] = tswapl(env->CP0_Status);
-    (*regs)[TARGET_EF_CP0_CAUSE] = tswapl(env->CP0_Cause);
+    (*regs)[TARGET_EF_LO] = tswapreg(env->active_tc.LO[0]);
+    (*regs)[TARGET_EF_HI] = tswapreg(env->active_tc.HI[0]);
+    (*regs)[TARGET_EF_CP0_EPC] = tswapreg(env->active_tc.PC);
+    (*regs)[TARGET_EF_CP0_BADVADDR] = tswapreg(env->CP0_BadVAddr);
+    (*regs)[TARGET_EF_CP0_STATUS] = tswapreg(env->CP0_Status);
+    (*regs)[TARGET_EF_CP0_CAUSE] = tswapreg(env->CP0_Cause);
 }
 
 #define USE_ELF_CORE_DUMP
-- 
1.8.1.4

Re: [Qemu-devel] [PATCH v2 02/10] elfload: fix size of registers for N32

[Peter Maydell]

On 3 April 2013 11:32, Paolo Bonzini <pbonzini@redhat.com> wrote:
> Registers are 64-bit in size for the MIPS n32 ABI.  Define
> target_elf_greg_t accordingly, and use the correct function
> to do endian swaps.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  linux-user/elfload.c | 21 ++++++++++++++-------
>  1 file changed, 14 insertions(+), 7 deletions(-)
>
> diff --git a/linux-user/elfload.c b/linux-user/elfload.c
> index d3589ff..9d5dbb8 100644
> --- a/linux-user/elfload.c
> +++ b/linux-user/elfload.c
> @@ -101,7 +101,14 @@ enum {
>  #define ELF_DATA        ELFDATA2LSB
>  #endif
>
> +#ifdef TARGET_ABI_MIPSN32
>  typedef target_ulong    target_elf_greg_t;
> +#define tswapreg(ptr)   tswapl(ptr)
> +#else
> +typedef abi_ulong       target_elf_greg_t;
> +#define tswapreg(ptr)   tswapal(ptr)
> +#endif

This is kind of ugly but it looks like the kernel is kind
of ugly too (ie elf_greg_t as a type is not defined the
same way necessarily for all targets and ABIs). At some
point this type should probably live in a header file
in linux-user/$arch/ but for now I guess it can pass.

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

-- PMM

Re: [Qemu-devel] [PATCH v2 02/10] elfload: fix size of registers for N32

[Paolo Bonzini]

Il 04/04/2013 17:32, Peter Maydell ha scritto:
> This is kind of ugly but it looks like the kernel is kind
> of ugly too (ie elf_greg_t as a type is not defined the
> same way necessarily for all targets and ABIs). At some
> point this type should probably live in a header file
> in linux-user/$arch/ but for now I guess it can pass.
> 
> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

I'll add abi_reg to abitypes.h and move tswapreg there, so that the
signal handling structures could use it too.

Paolo

Re: [Qemu-devel] [PATCH v2 02/10] elfload: fix size of registers for N32

[Peter Maydell]

On 4 April 2013 16:41, Paolo Bonzini <pbonzini@redhat.com> wrote:
> Il 04/04/2013 17:32, Peter Maydell ha scritto:
>> This is kind of ugly but it looks like the kernel is kind
>> of ugly too (ie elf_greg_t as a type is not defined the
>> same way necessarily for all targets and ABIs). At some
>> point this type should probably live in a header file
>> in linux-user/$arch/ but for now I guess it can pass.
>>
>> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
>
> I'll add abi_reg to abitypes.h and move tswapreg there, so
> that the signal handling structures could use it too.

I started off thinking that was the right thing, but then
I realised that the signal handling structures vary in what
they use (eg the MIPS ones tend to really use u32 and u64),
so they aren't necessarily the same type as elf_greg_t).

I think the conceptual ideal is that we should have
abi types which match the kernel's user-facing types.
This is a bit messed up here because the kernel doesn't
actually expose elf_greg_t in most archs (and because
MIPS has different types for "what the kernel thinks
long is" vs "what userspace thinks long is", which is
just plain confusing when looking at non-user-facing
struct definitions).

-- PMM

Re: [Qemu-devel] [PATCH v2 02/10] elfload: fix size of registers for N32

[Paolo Bonzini]

Il 04/04/2013 17:46, Peter Maydell ha scritto:
> On 4 April 2013 16:41, Paolo Bonzini <pbonzini@redhat.com> wrote:
>> Il 04/04/2013 17:32, Peter Maydell ha scritto:
>>> This is kind of ugly but it looks like the kernel is kind
>>> of ugly too (ie elf_greg_t as a type is not defined the
>>> same way necessarily for all targets and ABIs). At some
>>> point this type should probably live in a header file
>>> in linux-user/$arch/ but for now I guess it can pass.
>>>
>>> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
>>
>> I'll add abi_reg to abitypes.h and move tswapreg there, so
>> that the signal handling structures could use it too.
> 
> I started off thinking that was the right thing, but then
> I realised that the signal handling structures vary in what
> they use (eg the MIPS ones tend to really use u32 and u64),
> so they aren't necessarily the same type as elf_greg_t).

Right, MIPS o32 uses uint64_t.  So I'll keep target_elf_greg_t as in v2.

> I think the conceptual ideal is that we should have
> abi types which match the kernel's user-facing types.
> This is a bit messed up here because the kernel doesn't
> actually expose elf_greg_t in most archs (and because
> MIPS has different types for "what the kernel thinks
> long is" vs "what userspace thinks long is", which is
> just plain confusing when looking at non-user-facing
> struct definitions).

Paolo

[Qemu-devel] [PATCH v2 03/10] elfload: use tswapreg consistently in elf_core_copy_regs

[Paolo Bonzini]

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 linux-user/elfload.c | 114 +++++++++++++++++++++++++--------------------------
 1 file changed, 57 insertions(+), 57 deletions(-)

diff --git a/linux-user/elfload.c b/linux-user/elfload.c
index 9d5dbb8..bc2e9f1 100644
--- a/linux-user/elfload.c
+++ b/linux-user/elfload.c
@@ -297,25 +297,25 @@ typedef target_elf_greg_t  target_elf_gregset_t[ELF_NREG];
 
 static void elf_core_copy_regs(target_elf_gregset_t *regs, const CPUARMState *env)
 {
-    (*regs)[0] = tswapl(env->regs[0]);
-    (*regs)[1] = tswapl(env->regs[1]);
-    (*regs)[2] = tswapl(env->regs[2]);
-    (*regs)[3] = tswapl(env->regs[3]);
-    (*regs)[4] = tswapl(env->regs[4]);
-    (*regs)[5] = tswapl(env->regs[5]);
-    (*regs)[6] = tswapl(env->regs[6]);
-    (*regs)[7] = tswapl(env->regs[7]);
-    (*regs)[8] = tswapl(env->regs[8]);
-    (*regs)[9] = tswapl(env->regs[9]);
-    (*regs)[10] = tswapl(env->regs[10]);
-    (*regs)[11] = tswapl(env->regs[11]);
-    (*regs)[12] = tswapl(env->regs[12]);
-    (*regs)[13] = tswapl(env->regs[13]);
-    (*regs)[14] = tswapl(env->regs[14]);
-    (*regs)[15] = tswapl(env->regs[15]);
-
-    (*regs)[16] = tswapl(cpsr_read((CPUARMState *)env));
-    (*regs)[17] = tswapl(env->regs[0]); /* XXX */
+    (*regs)[0] = tswapreg(env->regs[0]);
+    (*regs)[1] = tswapreg(env->regs[1]);
+    (*regs)[2] = tswapreg(env->regs[2]);
+    (*regs)[3] = tswapreg(env->regs[3]);
+    (*regs)[4] = tswapreg(env->regs[4]);
+    (*regs)[5] = tswapreg(env->regs[5]);
+    (*regs)[6] = tswapreg(env->regs[6]);
+    (*regs)[7] = tswapreg(env->regs[7]);
+    (*regs)[8] = tswapreg(env->regs[8]);
+    (*regs)[9] = tswapreg(env->regs[9]);
+    (*regs)[10] = tswapreg(env->regs[10]);
+    (*regs)[11] = tswapreg(env->regs[11]);
+    (*regs)[12] = tswapreg(env->regs[12]);
+    (*regs)[13] = tswapreg(env->regs[13]);
+    (*regs)[14] = tswapreg(env->regs[14]);
+    (*regs)[15] = tswapreg(env->regs[15]);
+
+    (*regs)[16] = tswapreg(cpsr_read((CPUARMState *)env));
+    (*regs)[17] = tswapreg(env->regs[0]); /* XXX */
 }
 
 #define USE_ELF_CORE_DUMP
@@ -681,19 +681,19 @@ static void elf_core_copy_regs(target_elf_gregset_t *regs, const CPUPPCState *en
     target_ulong ccr = 0;
 
     for (i = 0; i < ARRAY_SIZE(env->gpr); i++) {
-        (*regs)[i] = tswapl(env->gpr[i]);
+        (*regs)[i] = tswapreg(env->gpr[i]);
     }
 
-    (*regs)[32] = tswapl(env->nip);
-    (*regs)[33] = tswapl(env->msr);
-    (*regs)[35] = tswapl(env->ctr);
-    (*regs)[36] = tswapl(env->lr);
-    (*regs)[37] = tswapl(env->xer);
+    (*regs)[32] = tswapreg(env->nip);
+    (*regs)[33] = tswapreg(env->msr);
+    (*regs)[35] = tswapreg(env->ctr);
+    (*regs)[36] = tswapreg(env->lr);
+    (*regs)[37] = tswapreg(env->xer);
 
     for (i = 0; i < ARRAY_SIZE(env->crf); i++) {
         ccr |= env->crf[i] << (32 - ((i + 1) * 4));
     }
-    (*regs)[38] = tswapl(ccr);
+    (*regs)[38] = tswapreg(ccr);
 }
 
 #define USE_ELF_CORE_DUMP
@@ -801,11 +801,11 @@ static void elf_core_copy_regs(target_elf_gregset_t *regs, const CPUMBState *env
     int i, pos = 0;
 
     for (i = 0; i < 32; i++) {
-        (*regs)[pos++] = tswapl(env->regs[i]);
+        (*regs)[pos++] = tswapreg(env->regs[i]);
     }
 
     for (i = 0; i < 6; i++) {
-        (*regs)[pos++] = tswapl(env->sregs[i]);
+        (*regs)[pos++] = tswapreg(env->sregs[i]);
     }
 }
 
@@ -841,11 +841,11 @@ static void elf_core_copy_regs(target_elf_gregset_t *regs,
     int i;
 
     for (i = 0; i < 32; i++) {
-        (*regs)[i] = tswapl(env->gpr[i]);
+        (*regs)[i] = tswapreg(env->gpr[i]);
     }
 
-    (*regs)[32] = tswapl(env->pc);
-    (*regs)[33] = tswapl(env->sr);
+    (*regs)[32] = tswapreg(env->pc);
+    (*regs)[33] = tswapreg(env->sr);
 }
 #define ELF_HWCAP 0
 #define ELF_PLATFORM NULL
@@ -890,15 +890,15 @@ static inline void elf_core_copy_regs(target_elf_gregset_t *regs,
     int i;
 
     for (i = 0; i < 16; i++) {
-        (*regs[i]) = tswapl(env->gregs[i]);
+        (*regs[i]) = tswapreg(env->gregs[i]);
     }
 
-    (*regs)[TARGET_REG_PC] = tswapl(env->pc);
-    (*regs)[TARGET_REG_PR] = tswapl(env->pr);
-    (*regs)[TARGET_REG_SR] = tswapl(env->sr);
-    (*regs)[TARGET_REG_GBR] = tswapl(env->gbr);
-    (*regs)[TARGET_REG_MACH] = tswapl(env->mach);
-    (*regs)[TARGET_REG_MACL] = tswapl(env->macl);
+    (*regs)[TARGET_REG_PC] = tswapreg(env->pc);
+    (*regs)[TARGET_REG_PR] = tswapreg(env->pr);
+    (*regs)[TARGET_REG_SR] = tswapreg(env->sr);
+    (*regs)[TARGET_REG_GBR] = tswapreg(env->gbr);
+    (*regs)[TARGET_REG_MACH] = tswapreg(env->mach);
+    (*regs)[TARGET_REG_MACL] = tswapreg(env->macl);
     (*regs)[TARGET_REG_SYSCALL] = 0; /* FIXME */
 }
 
@@ -952,25 +952,25 @@ typedef target_elf_greg_t target_elf_gregset_t[ELF_NREG];
 
 static void elf_core_copy_regs(target_elf_gregset_t *regs, const CPUM68KState *env)
 {
-    (*regs)[0] = tswapl(env->dregs[1]);
-    (*regs)[1] = tswapl(env->dregs[2]);
-    (*regs)[2] = tswapl(env->dregs[3]);
-    (*regs)[3] = tswapl(env->dregs[4]);
-    (*regs)[4] = tswapl(env->dregs[5]);
-    (*regs)[5] = tswapl(env->dregs[6]);
-    (*regs)[6] = tswapl(env->dregs[7]);
-    (*regs)[7] = tswapl(env->aregs[0]);
-    (*regs)[8] = tswapl(env->aregs[1]);
-    (*regs)[9] = tswapl(env->aregs[2]);
-    (*regs)[10] = tswapl(env->aregs[3]);
-    (*regs)[11] = tswapl(env->aregs[4]);
-    (*regs)[12] = tswapl(env->aregs[5]);
-    (*regs)[13] = tswapl(env->aregs[6]);
-    (*regs)[14] = tswapl(env->dregs[0]);
-    (*regs)[15] = tswapl(env->aregs[7]);
-    (*regs)[16] = tswapl(env->dregs[0]); /* FIXME: orig_d0 */
-    (*regs)[17] = tswapl(env->sr);
-    (*regs)[18] = tswapl(env->pc);
+    (*regs)[0] = tswapreg(env->dregs[1]);
+    (*regs)[1] = tswapreg(env->dregs[2]);
+    (*regs)[2] = tswapreg(env->dregs[3]);
+    (*regs)[3] = tswapreg(env->dregs[4]);
+    (*regs)[4] = tswapreg(env->dregs[5]);
+    (*regs)[5] = tswapreg(env->dregs[6]);
+    (*regs)[6] = tswapreg(env->dregs[7]);
+    (*regs)[7] = tswapreg(env->aregs[0]);
+    (*regs)[8] = tswapreg(env->aregs[1]);
+    (*regs)[9] = tswapreg(env->aregs[2]);
+    (*regs)[10] = tswapreg(env->aregs[3]);
+    (*regs)[11] = tswapreg(env->aregs[4]);
+    (*regs)[12] = tswapreg(env->aregs[5]);
+    (*regs)[13] = tswapreg(env->aregs[6]);
+    (*regs)[14] = tswapreg(env->dregs[0]);
+    (*regs)[15] = tswapreg(env->aregs[7]);
+    (*regs)[16] = tswapreg(env->dregs[0]); /* FIXME: orig_d0 */
+    (*regs)[17] = tswapreg(env->sr);
+    (*regs)[18] = tswapreg(env->pc);
     (*regs)[19] = 0;  /* FIXME: regs->format | regs->vector */
 }
 
-- 
1.8.1.4

Re: [Qemu-devel] [PATCH v2 03/10] elfload: use tswapreg consistently in elf_core_copy_regs

[Peter Maydell]

On 3 April 2013 11:32, Paolo Bonzini <pbonzini@redhat.com> wrote:
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

-- PMM

[Qemu-devel] [PATCH v2 04/10] elfload: use abi_short/ushort instead of target_short/ushort

[Paolo Bonzini]

The alignment is a characteristic of the ABI, not the CPU.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 configure                    | 2 --
 include/exec/cpu-defs.h      | 2 --
 include/exec/user/abitypes.h | 7 +++++++
 linux-user/elfload.c         | 6 +++---
 4 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/configure b/configure
index 437baa8..68314d1 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_short_alignment=2
 target_int_alignment=4
 target_long_alignment=4
 target_llong_alignment=8
@@ -4154,7 +4153,6 @@ case "$cpu" in
   ;;
 esac
 
-echo "TARGET_SHORT_ALIGNMENT=$target_short_alignment" >> $config_target_mak
 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
diff --git a/include/exec/cpu-defs.h b/include/exec/cpu-defs.h
index 0ae967a..d376f0f 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 int16_t target_short __attribute__ ((aligned(TARGET_SHORT_ALIGNMENT)));
-typedef uint16_t target_ushort __attribute__((aligned(TARGET_SHORT_ALIGNMENT)));
 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)));
diff --git a/include/exec/user/abitypes.h b/include/exec/user/abitypes.h
index fe7f662..abaa028 100644
--- a/include/exec/user/abitypes.h
+++ b/include/exec/user/abitypes.h
@@ -2,6 +2,13 @@
 #define QEMU_TYPES_H
 #include "cpu.h"
 
+#ifndef ABI_SHORT_ALIGNMENT
+#define ABI_SHORT_ALIGNMENT 2
+#endif
+
+typedef int16_t abi_short __attribute__ ((aligned(ABI_SHORT_ALIGNMENT)));
+typedef uint16_t abi_ushort __attribute__((aligned(ABI_SHORT_ALIGNMENT)));
+
 #ifdef TARGET_ABI32
 typedef uint32_t abi_ulong;
 typedef int32_t abi_long;
diff --git a/linux-user/elfload.c b/linux-user/elfload.c
index bc2e9f1..5eca934 100644
--- a/linux-user/elfload.c
+++ b/linux-user/elfload.c
@@ -110,8 +110,8 @@ typedef abi_ulong       target_elf_greg_t;
 #endif
 
 #ifdef USE_UID16
-typedef target_ushort   target_uid_t;
-typedef target_ushort   target_gid_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;
@@ -2116,7 +2116,7 @@ struct target_elf_siginfo {
 
 struct target_elf_prstatus {
     struct target_elf_siginfo pr_info;      /* Info associated with signal */
-    target_short       pr_cursig;    /* Current signal */
+    abi_short          pr_cursig;    /* Current signal */
     abi_ulong          pr_sigpend;   /* XXX */
     abi_ulong          pr_sighold;   /* XXX */
     target_pid_t       pr_pid;
-- 
1.8.1.4

Re: [Qemu-devel] [PATCH v2 04/10] elfload: use abi_short/ushort instead of target_short/ushort

[Peter Maydell]

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>

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

-- PMM

[Qemu-devel] [PATCH v2 05/10] elfload: use abi_int/uint instead of target_int/uint

[Paolo Bonzini]

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(-)

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
 
-- 
1.8.1.4

Re: [Qemu-devel] [PATCH v2 05/10] elfload: use abi_int/uint instead of target_int/uint

[Peter Maydell]

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

Re: [Qemu-devel] [PATCH v2 05/10] elfload: use abi_int/uint instead of target_int/uint

[Paolo Bonzini]

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

Re: [Qemu-devel] [PATCH v2 05/10] elfload: use abi_int/uint instead of target_int/uint

[Peter Maydell]

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

Re: [Qemu-devel] [PATCH v2 05/10] elfload: use abi_int/uint instead of target_int/uint

[Paolo Bonzini]

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...

[Qemu-devel] [PATCH v2 06/10] elfload: only give abi_long/ulong the alignment specified by the target

[Paolo Bonzini]

Previously, this was done for target_long/ulong, and propagated to
abi_long/ulong via a typedef.  But target_long/ulong should not
have any specific alignment, it is never used to access guest
memory.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 configure                    |  9 ---------
 include/exec/cpu-defs.h      |  8 ++++----
 include/exec/user/abitypes.h | 19 +++++++++++++------
 target-m68k/cpu.h            |  1 +
 4 files changed, 18 insertions(+), 19 deletions(-)

diff --git a/configure b/configure
index 8276e79..0763047 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_long_alignment=4
 target_llong_alignment=8
 target_libs_softmmu=
 
@@ -4024,10 +4023,8 @@ case "$target_arch2" in
   ;;
   x86_64)
     TARGET_BASE_ARCH=i386
-    target_long_alignment=8
   ;;
   alpha)
-    target_long_alignment=8
     target_nptl="yes"
   ;;
   arm|armeb)
@@ -4046,7 +4043,6 @@ case "$target_arch2" in
   m68k)
     bflt="yes"
     gdb_xml_files="cf-core.xml cf-fp.xml"
-    target_long_alignment=2
     target_llong_alignment=2
   ;;
   microblaze|microblazeel)
@@ -4069,7 +4065,6 @@ case "$target_arch2" in
     TARGET_ARCH=mips64
     TARGET_BASE_ARCH=mips
     echo "TARGET_ABI_MIPSN64=y" >> $config_target_mak
-    target_long_alignment=8
   ;;
   moxie)
   ;;
@@ -4091,7 +4086,6 @@ case "$target_arch2" in
     TARGET_BASE_ARCH=ppc
     TARGET_ABI_DIR=ppc
     gdb_xml_files="power64-core.xml power-fpu.xml power-altivec.xml power-spe.xml"
-    target_long_alignment=8
   ;;
   ppc64abi32)
     TARGET_ARCH=ppc64
@@ -4109,7 +4103,6 @@ case "$target_arch2" in
   ;;
   sparc64)
     TARGET_BASE_ARCH=sparc
-    target_long_alignment=8
   ;;
   sparc32plus)
     TARGET_ARCH=sparc64
@@ -4119,7 +4112,6 @@ case "$target_arch2" in
   ;;
   s390x)
     target_nptl="yes"
-    target_long_alignment=8
   ;;
   unicore32)
   ;;
@@ -4151,7 +4143,6 @@ case "$cpu" in
   ;;
 esac
 
-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
 target_arch_name="`upper $TARGET_ARCH`"
diff --git a/include/exec/cpu-defs.h b/include/exec/cpu-defs.h
index 2aa9331..3cf1272 100644
--- a/include/exec/cpu-defs.h
+++ b/include/exec/cpu-defs.h
@@ -40,14 +40,14 @@ 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 */
 #if TARGET_LONG_SIZE == 4
-typedef int32_t target_long __attribute__((aligned(TARGET_LONG_ALIGNMENT)));
-typedef uint32_t target_ulong __attribute__((aligned(TARGET_LONG_ALIGNMENT)));
+typedef int32_t target_long;
+typedef uint32_t target_ulong;
 #define TARGET_FMT_lx "%08x"
 #define TARGET_FMT_ld "%d"
 #define TARGET_FMT_lu "%u"
 #elif TARGET_LONG_SIZE == 8
-typedef int64_t target_long __attribute__((aligned(TARGET_LONG_ALIGNMENT)));
-typedef uint64_t target_ulong __attribute__((aligned(TARGET_LONG_ALIGNMENT)));
+typedef int64_t target_long;
+typedef uint64_t target_ulong;
 #define TARGET_FMT_lx "%016" PRIx64
 #define TARGET_FMT_ld "%" PRId64
 #define TARGET_FMT_lu "%" PRIu64
diff --git a/include/exec/user/abitypes.h b/include/exec/user/abitypes.h
index 97a87a6..9bf916a 100644
--- a/include/exec/user/abitypes.h
+++ b/include/exec/user/abitypes.h
@@ -2,12 +2,21 @@
 #define QEMU_TYPES_H
 #include "cpu.h"
 
+#ifdef TARGET_ABI32
+#define TARGET_ABI_BITS 32
+#else
+#define TARGET_ABI_BITS TARGET_LONG_BITS
+#endif
+
 #ifndef ABI_SHORT_ALIGNMENT
 #define ABI_SHORT_ALIGNMENT 2
 #endif
 #ifndef ABI_INT_ALIGNMENT
 #define ABI_INT_ALIGNMENT 4
 #endif
+#ifndef ABI_LONG_ALIGNMENT
+#define ABI_LONG_ALIGNMENT (TARGET_ABI_BITS / 8)
+#endif
 
 typedef int16_t abi_short __attribute__ ((aligned(ABI_SHORT_ALIGNMENT)));
 typedef uint16_t abi_ushort __attribute__((aligned(ABI_SHORT_ALIGNMENT)));
@@ -15,12 +24,11 @@ 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;
-typedef int32_t abi_long;
+typedef uint32_t abi_ulong __attribute__((aligned(ABI_LONG_ALIGNMENT)));
+typedef int32_t abi_long __attribute__((aligned(ABI_LONG_ALIGNMENT)));
 #define TARGET_ABI_FMT_lx "%08x"
 #define TARGET_ABI_FMT_ld "%d"
 #define TARGET_ABI_FMT_lu "%u"
-#define TARGET_ABI_BITS 32
 
 static inline abi_ulong tswapal(abi_ulong v)
 {
@@ -28,12 +36,11 @@ static inline abi_ulong tswapal(abi_ulong v)
 }
 
 #else
-typedef target_ulong abi_ulong;
-typedef target_long abi_long;
+typedef target_ulong abi_ulong __attribute__((aligned(ABI_LONG_ALIGNMENT)));
+typedef target_long abi_long __attribute__((aligned(ABI_LONG_ALIGNMENT)));
 #define TARGET_ABI_FMT_lx TARGET_FMT_lx
 #define TARGET_ABI_FMT_ld TARGET_FMT_ld
 #define TARGET_ABI_FMT_lu TARGET_FMT_lu
-#define TARGET_ABI_BITS TARGET_LONG_BITS
 /* for consistency, define ABI32 too */
 #if TARGET_ABI_BITS == 32
 #define TARGET_ABI32 1
diff --git a/target-m68k/cpu.h b/target-m68k/cpu.h
index 29cc887..6825e7c 100644
--- a/target-m68k/cpu.h
+++ b/target-m68k/cpu.h
@@ -22,6 +22,7 @@
 
 #define TARGET_LONG_BITS 32
 #define ABI_INT_ALIGNMENT 2
+#define ABI_LONG_ALIGNMENT 2
 
 #define CPUArchState struct CPUM68KState
 
-- 
1.8.1.4

Re: [Qemu-devel] [PATCH v2 06/10] elfload: only give abi_long/ulong the alignment specified by the target

[Peter Maydell]

On 3 April 2013 11:32, Paolo Bonzini <pbonzini@redhat.com> wrote:
> Previously, this was done for target_long/ulong, and propagated to
> abi_long/ulong via a typedef.  But target_long/ulong should not
> have any specific alignment, it is never used to access guest
> memory.

Agreed in principle, but you seem to have missed some structs
which use target_ulong currently and which presumably should
use abi_ulong instead, eg all the target_ucontext etc structs
in linux-user/signal.c

Also linux-user/elfload.c:symfind() is casting a pointer to
target_ulong* and dereferencing it, and that might now cause
an alignment fault on some host CPUs if the host CPU alignment
requirements are stricter than the guest's.

thanks
-- PMM

Re: [Qemu-devel] [PATCH v2 06/10] elfload: only give abi_long/ulong the alignment specified by the target

[Paolo Bonzini]

Il 04/04/2013 16:09, Peter Maydell ha scritto:
> Agreed in principle, but you seem to have missed some structs
> which use target_ulong currently and which presumably should
> use abi_ulong instead, eg all the target_ucontext etc structs
> in linux-user/signal.c

Right.

> Also linux-user/elfload.c:symfind() is casting a pointer to
> target_ulong* and dereferencing it, and that might now cause
> an alignment fault on some host CPUs if the host CPU alignment
> requirements are stricter than the guest's.

I had seen this, but it is only used with bsearch and safe:

static const char *lookup_symbolxx(struct syminfo *s, target_ulong orig_addr)
{
#if ELF_CLASS == ELFCLASS32
    struct elf_sym *syms = s->disas_symtab.elf32;
#else
    struct elf_sym *syms = s->disas_symtab.elf64;
#endif

    // binary search
    struct elf_sym *sym;

    sym = bsearch(&orig_addr, syms, s->disas_num_syms, sizeof(*syms), symfind);
    ...
}

Paolo

Re: [Qemu-devel] [PATCH v2 06/10] elfload: only give abi_long/ulong the alignment specified by the target

[Peter Maydell]

On 4 April 2013 15:11, Paolo Bonzini <pbonzini@redhat.com> wrote:
> Il 04/04/2013 16:09, Peter Maydell ha scritto:
>> Also linux-user/elfload.c:symfind() is casting a pointer to
>> target_ulong* and dereferencing it, and that might now cause
>> an alignment fault on some host CPUs if the host CPU alignment
>> requirements are stricter than the guest's.
>
> I had seen this, but it is only used with bsearch and safe

Ah yes, you're right. Sorry.

-- PMM

Re: [Qemu-devel] [PATCH v2 06/10] elfload: only give abi_long/ulong the alignment specified by the target

[Paolo Bonzini]

Il 04/04/2013 16:18, Peter Maydell ha scritto:
>>> Also linux-user/elfload.c:symfind() is casting a pointer to
>>> >> target_ulong* and dereferencing it, and that might now cause
>>> >> an alignment fault on some host CPUs if the host CPU alignment
>>> >> requirements are stricter than the guest's.
>> >
>> > I had seen this, but it is only used with bsearch and safe
> Ah yes, you're right. Sorry.

Regarding the others, none of them are in target-generic places, and
none of them affect m68k (ARM only uses non-standard alignment for llong):

- linux-user/mips64/syscall.h is correct with target_ulong, and in
general MIPS is best left as it is (it often uses uint32_t/uint64_t or
target_long/ulong explicitly so that n32 is handled correctly).

- linux-user/openrisc/syscall.h could use abi_ulong instead of
target_ulong, and abi_uint instead of uint32_t, but it doesn't change
anything so it is more of a cleanup

- linux-user/syscall_defs.h's use of target_ulong for st_ino would be a
bug, but on these architectures target_ulong==abi_ulong.  In general the
whole struct should be using abi_* types, but again it is more of a cleanup.

So the patch is okay as is, I think.

Paolo

Re: [Qemu-devel] [PATCH v2 06/10] elfload: only give abi_long/ulong the alignment specified by the target

[Peter Maydell]

On 4 April 2013 15:26, Paolo Bonzini <pbonzini@redhat.com> wrote:
> Regarding the others, none of them are in target-generic places, and
> none of them affect m68k (ARM only uses non-standard alignment for llong):
>
> - linux-user/mips64/syscall.h is correct with target_ulong, and in
> general MIPS is best left as it is (it often uses uint32_t/uint64_t or
> target_long/ulong explicitly so that n32 is handled correctly).

Hmm, is this really right? target_ulong before this patch would
have had an explicit alignment attribute, and it no longer does.
So if you're running a mips64 guest on an m68k host then you'll
now get structs with the natural m68k alignment rather than the
desired mips64 alignment...

(I can entirely believe that we get this wrong in a lot of
places, and that in theory just about anything in a target_
struct needs an alignment specifier.)

I'm running your patchset through an LTP test run for ARM
guest, by the way.

-- PMM

Re: [Qemu-devel] [PATCH v2 06/10] elfload: only give abi_long/ulong the alignment specified by the target

[Paolo Bonzini]

Il 04/04/2013 16:37, Peter Maydell ha scritto:
>> Regarding the others, none of them are in target-generic places, and
>> > none of them affect m68k (ARM only uses non-standard alignment for llong):
>> >
>> > - linux-user/mips64/syscall.h is correct with target_ulong, and in
>> > general MIPS is best left as it is (it often uses uint32_t/uint64_t or
>> > target_long/ulong explicitly so that n32 is handled correctly).
> Hmm, is this really right? target_ulong before this patch would
> have had an explicit alignment attribute, and it no longer does.
> So if you're running a mips64 guest on an m68k host then you'll
> now get structs with the natural m68k alignment rather than the
> desired mips64 alignment...

Note that target_ulong and abi_ulong would have different sizes.  Better
to get alignment wrong on m68k, than size wrong on all platforms. :)

Also I think this should not be a problem.  These structures have no
holes in them, the padding is always written down explicitly and they
are accessed (or should be) via copy_from/to_user.

> (I can entirely believe that we get this wrong in a lot of
> places, and that in theory just about anything in a target_
> struct needs an alignment specifier.)

Yes, or more simply just use abi_* types.  It doesn't help that
potential problems would only show up on m68k.

Paolo

Re: [Qemu-devel] [PATCH v2 06/10] elfload: only give abi_long/ulong the alignment specified by the target

[Peter Maydell]

On 4 April 2013 15:45, Paolo Bonzini <pbonzini@redhat.com> wrote:
> Il 04/04/2013 16:37, Peter Maydell ha scritto:
>> (I can entirely believe that we get this wrong in a lot of
>> places, and that in theory just about anything in a target_
>> struct needs an alignment specifier.)
>
> Yes, or more simply just use abi_* types.  It doesn't help that
> potential problems would only show up on m68k.

Mmm. Maybe we could use an addition to HACKING which at
least defines what the "right thing" is for new target
structs (and when you should use abi_* vs target_*).

PS: LTP test run says no new failures, so that's good.

-- PMM

[Qemu-devel] [PATCH v2 07/10] elfload: use abi_llong/ullong instead of target_llong/ullong

[Paolo Bonzini]

The alignment is a characteristic of the ABI, not the CPU.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 configure                    | 4 ----
 include/exec/cpu-defs.h      | 2 --
 include/exec/user/abitypes.h | 5 +++++
 linux-user/elfload.c         | 4 ++--
 target-arm/cpu.h             | 1 +
 target-m68k/cpu.h            | 1 +
 6 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/configure b/configure
index 0763047..10ea70e 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_llong_alignment=8
 target_libs_softmmu=
 
 TARGET_ARCH="$target_arch2"
@@ -4032,7 +4031,6 @@ case "$target_arch2" in
     bflt="yes"
     target_nptl="yes"
     gdb_xml_files="arm-core.xml arm-vfp.xml arm-vfp3.xml arm-neon.xml"
-    target_llong_alignment=4
   ;;
   cris)
     target_nptl="yes"
@@ -4043,7 +4041,6 @@ case "$target_arch2" in
   m68k)
     bflt="yes"
     gdb_xml_files="cf-core.xml cf-fp.xml"
-    target_llong_alignment=2
   ;;
   microblaze|microblazeel)
     TARGET_ARCH=microblaze
@@ -4143,7 +4140,6 @@ case "$cpu" in
   ;;
 esac
 
-echo "TARGET_LLONG_ALIGNMENT=$target_llong_alignment" >> $config_target_mak
 echo "TARGET_ARCH=$TARGET_ARCH" >> $config_target_mak
 target_arch_name="`upper $TARGET_ARCH`"
 echo "TARGET_$target_arch_name=y" >> $config_target_mak
diff --git a/include/exec/cpu-defs.h b/include/exec/cpu-defs.h
index 3cf1272..d8c64e9 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 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 */
 #if TARGET_LONG_SIZE == 4
 typedef int32_t target_long;
diff --git a/include/exec/user/abitypes.h b/include/exec/user/abitypes.h
index 9bf916a..0cec7b0 100644
--- a/include/exec/user/abitypes.h
+++ b/include/exec/user/abitypes.h
@@ -17,11 +17,16 @@
 #ifndef ABI_LONG_ALIGNMENT
 #define ABI_LONG_ALIGNMENT (TARGET_ABI_BITS / 8)
 #endif
+#ifndef ABI_LLONG_ALIGNMENT
+#define ABI_LLONG_ALIGNMENT 8
+#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)));
+typedef int64_t abi_llong __attribute__((aligned(ABI_LLONG_ALIGNMENT)));
+typedef uint64_t abi_ullong __attribute__((aligned(ABI_LLONG_ALIGNMENT)));
 
 #ifdef TARGET_ABI32
 typedef uint32_t abi_ulong __attribute__((aligned(ABI_LONG_ALIGNMENT)));
diff --git a/linux-user/elfload.c b/linux-user/elfload.c
index 14a8ecf..979b57c 100644
--- a/linux-user/elfload.c
+++ b/linux-user/elfload.c
@@ -102,8 +102,8 @@ enum {
 #endif
 
 #ifdef TARGET_ABI_MIPSN32
-typedef target_ulong    target_elf_greg_t;
-#define tswapreg(ptr)   tswapl(ptr)
+typedef abi_ullong      target_elf_greg_t;
+#define tswapreg(ptr)   tswap64(ptr)
 #else
 typedef abi_ulong       target_elf_greg_t;
 #define tswapreg(ptr)   tswapal(ptr)
diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index 2b97221..6418199 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -20,6 +20,7 @@
 #define CPU_ARM_H
 
 #define TARGET_LONG_BITS 32
+#define ABI_LLONG_ALIGNMENT 4
 
 #define ELF_MACHINE	EM_ARM
 
diff --git a/target-m68k/cpu.h b/target-m68k/cpu.h
index 6825e7c..09ee998 100644
--- a/target-m68k/cpu.h
+++ b/target-m68k/cpu.h
@@ -23,6 +23,7 @@
 #define TARGET_LONG_BITS 32
 #define ABI_INT_ALIGNMENT 2
 #define ABI_LONG_ALIGNMENT 2
+#define ABI_LLONG_ALIGNMENT 2
 
 #define CPUArchState struct CPUM68KState
 
-- 
1.8.1.4

[Qemu-devel] [PATCH v2 08/10] configure: move CONFIG_QEMU_LDST_OPTIMIZATION to config-host.mak

[Paolo Bonzini]

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 configure | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/configure b/configure
index 10ea70e..7d83600 100755
--- a/configure
+++ b/configure
@@ -3481,6 +3481,15 @@ echo "extra_ldflags=$EXTRA_LDFLAGS" >> $config_host_mak
 echo "qemu_localedir=$qemu_localedir" >> $config_host_mak
 
 echo "ARCH=$ARCH" >> $config_host_mak
+
+case "$cpu" in
+  i386|x86_64|ppc)
+    # The TCG interpreter currently does not support ld/st optimization.
+    if test "$tcg_interpreter" = "no" ; then
+        echo "CONFIG_QEMU_LDST_OPTIMIZATION=y" >> $config_host_mak
+    fi
+  ;;
+esac
 if test "$debug_tcg" = "yes" ; then
   echo "CONFIG_DEBUG_TCG=y" >> $config_host_mak
 fi
@@ -4131,15 +4140,6 @@ upper() {
     echo "$@"| LC_ALL=C tr '[a-z]' '[A-Z]'
 }
 
-case "$cpu" in
-  i386|x86_64|ppc)
-    # The TCG interpreter currently does not support ld/st optimization.
-    if test "$tcg_interpreter" = "no" ; then
-        echo "CONFIG_QEMU_LDST_OPTIMIZATION=y" >> $config_target_mak
-    fi
-  ;;
-esac
-
 echo "TARGET_ARCH=$TARGET_ARCH" >> $config_target_mak
 target_arch_name="`upper $TARGET_ARCH`"
 echo "TARGET_$target_arch_name=y" >> $config_target_mak
-- 
1.8.1.4

[Qemu-devel] [PATCH v2 09/10] configure: move common libraries to config-host.mak

[Paolo Bonzini]

Move -lm to the end of the line, so that it can be picked up as a
dependency by pixman.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 Makefile.target | 10 +++++-----
 configure       |  4 +++-
 2 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/Makefile.target b/Makefile.target
index 2bd6d14..2636103 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -31,10 +31,6 @@ PROGS+=$(QEMU_PROGW)
 endif
 STPFILES=
 
-ifndef CONFIG_HAIKU
-LIBS+=-lm
-endif
-
 config-target.h: config-target.h-timestamp
 config-target.h-timestamp: config-target.mak
 
@@ -119,7 +115,7 @@ obj-$(CONFIG_HAVE_GET_MEMORY_MAPPING) += memory_mapping.o
 obj-$(CONFIG_HAVE_CORE_DUMP) += dump.o
 obj-$(CONFIG_NO_GET_MEMORY_MAPPING) += memory_mapping-stub.o
 obj-$(CONFIG_NO_CORE_DUMP) += dump-stub.o
-LIBS+=-lz
+LIBS+=$(libs_softmmu)
 
 # xen support
 obj-$(CONFIG_XEN) += xen-all.o xen-mapcache.o
@@ -149,6 +145,10 @@ include $(SRC_PATH)/Makefile.objs
 all-obj-y = $(obj-y)
 all-obj-y += $(addprefix ../, $(common-obj-y))
 
+ifndef CONFIG_HAIKU
+LIBS+=-lm
+endif
+
 ifdef QEMU_PROGW
 # The linker builds a windows executable. Make also a console executable.
 $(QEMU_PROGW): $(all-obj-y) ../libqemuutil.a ../libqemustub.a
diff --git a/configure b/configure
index 7d83600..c8d47fe 100755
--- a/configure
+++ b/configure
@@ -1455,6 +1455,7 @@ EOF
         exit 1
     fi
 fi
+libs_softmmu="$libs_softmmu -lz"
 
 ##########################################
 # libseccomp check
@@ -3479,6 +3480,7 @@ echo "qemu_helperdir=$libexecdir" >> $config_host_mak
 echo "extra_cflags=$EXTRA_CFLAGS" >> $config_host_mak
 echo "extra_ldflags=$EXTRA_LDFLAGS" >> $config_host_mak
 echo "qemu_localedir=$qemu_localedir" >> $config_host_mak
+echo "libs_softmmu=$libs_softmmu" >> $config_host_mak
 
 echo "ARCH=$ARCH" >> $config_host_mak
 
@@ -4190,7 +4192,7 @@ if test "$target_bigendian" = "yes" ; then
 fi
 if test "$target_softmmu" = "yes" ; then
   echo "CONFIG_SOFTMMU=y" >> $config_target_mak
-  echo "LIBS+=$libs_softmmu $target_libs_softmmu" >> $config_target_mak
+  echo "LIBS+=$target_libs_softmmu" >> $config_target_mak
   case "$target_arch2" in
     i386|x86_64)
       echo "CONFIG_HAVE_CORE_DUMP=y" >> $config_target_mak
-- 
1.8.1.4

Re: [Qemu-devel] [PATCH v2 09/10] configure: move common libraries to config-host.mak

[Peter Maydell]

On 3 April 2013 11:32, Paolo Bonzini <pbonzini@redhat.com> wrote:
> Move -lm to the end of the line, so that it can be picked up as a
> dependency by pixman.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

(you might note in the commit message that the pixman
issue only makes a difference for static builds).

-- PMM

[Qemu-devel] [PATCH v2 10/10] configure: CONFIG_NO_XEN is duplicated

[Paolo Bonzini]

We already define it in Makefile.target.  But we need to avoid a
curious double negation in order to eliminate it.

Tested-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 configure | 3 ---
 hw/xen.h  | 2 +-
 2 files changed, 1 insertion(+), 4 deletions(-)

diff --git a/configure b/configure
index c8d47fe..52539a9 100755
--- a/configure
+++ b/configure
@@ -4159,12 +4159,9 @@ case "$target_arch2" in
       if test "$xen_pci_passthrough" = yes; then
         echo "CONFIG_XEN_PCI_PASSTHROUGH=y" >> "$config_target_mak"
       fi
-    else
-      echo "CONFIG_NO_XEN=y" >> $config_target_mak
     fi
     ;;
   *)
-    echo "CONFIG_NO_XEN=y" >> $config_target_mak
 esac
 case "$target_arch2" in
   arm|i386|x86_64|ppcemb|ppc|ppc64|s390x)
diff --git a/hw/xen.h b/hw/xen.h
index 6235f91..7451c5a 100644
--- a/hw/xen.h
+++ b/hw/xen.h
@@ -25,7 +25,7 @@ extern bool xen_allowed;
 
 static inline bool xen_enabled(void)
 {
-#if defined(CONFIG_XEN_BACKEND) && !defined(CONFIG_NO_XEN)
+#if defined(CONFIG_XEN_BACKEND) && defined(CONFIG_XEN)
     return xen_allowed;
 #else
     return 0;
-- 
1.8.1.4