[Qemu-devel] [PATCH 0/5] trim down config-target.mak

[Qemu-devel] [PATCH 0/5] trim down config-target.mak

[Paolo Bonzini]

configure puts a bunch of things in config-target.mak that do not
really belong there; a better place is either config-host.mak or
target-*/cpu.h.  This series cleans it up.

Paolo Bonzini (5):
  configure: move TARGET_*_ALIGNMENT to target-*/cpu.h
  cpu: default TARGET_LONG_ALIGNMENT to TARGET_LONG_SIZE
  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         |  2 +-
 configure               | 43 ++++++++++++-------------------------------
 hw/xen.h                |  2 +-
 include/exec/cpu-defs.h | 13 +++++++++++++
 target-arm/cpu.h        |  1 +
 target-m68k/cpu.h       |  3 +++
 target-mips/mips-defs.h |  1 +
 7 files changed, 32 insertions(+), 33 deletions(-)

-- 
1.8.1.4

[Qemu-devel] [PATCH 1/5] configure: move TARGET_*_ALIGNMENT to target-*/cpu.h

[Paolo Bonzini]

This is no different from, for example, TARGET_LONG_BITS.  It does
not belong in configure.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 configure               | 18 ------------------
 include/exec/cpu-defs.h | 13 +++++++++++++
 target-alpha/cpu.h      |  1 +
 target-arm/cpu.h        |  1 +
 target-i386/cpu.h       |  1 +
 target-m68k/cpu.h       |  3 +++
 target-mips/mips-defs.h |  1 +
 target-ppc/cpu.h        |  1 +
 target-s390x/cpu.h      |  1 +
 target-sparc/cpu.h      |  1 +
 10 files changed, 23 insertions(+), 18 deletions(-)

diff --git a/configure b/configure
index 04c2618..5d9a87e 100755
--- a/configure
+++ b/configure
@@ -4013,10 +4013,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
 target_libs_softmmu=
 
 TARGET_ARCH="$target_arch2"
@@ -4028,10 +4024,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)
@@ -4039,7 +4033,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"
@@ -4050,9 +4043,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
   ;;
   microblaze|microblazeel)
     TARGET_ARCH=microblaze
@@ -4074,7 +4064,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)
   ;;
@@ -4096,7 +4085,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
@@ -4114,7 +4102,6 @@ case "$target_arch2" in
   ;;
   sparc64)
     TARGET_BASE_ARCH=sparc
-    target_long_alignment=8
   ;;
   sparc32plus)
     TARGET_ARCH=sparc64
@@ -4124,7 +4111,6 @@ case "$target_arch2" in
   ;;
   s390x)
     target_nptl="yes"
-    target_long_alignment=8
   ;;
   unicore32)
   ;;
@@ -4156,10 +4142,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
 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 0ae967a..32d11e8 100644
--- a/include/exec/cpu-defs.h
+++ b/include/exec/cpu-defs.h
@@ -36,6 +36,19 @@
 
 #define TARGET_LONG_SIZE (TARGET_LONG_BITS / 8)
 
+#ifndef TARGET_SHORT_ALIGNMENT
+#define TARGET_SHORT_ALIGNMENT 2
+#endif
+#ifndef TARGET_INT_ALIGNMENT
+#define TARGET_INT_ALIGNMENT 4
+#endif
+#ifndef TARGET_LONG_ALIGNMENT
+#define TARGET_LONG_ALIGNMENT 4
+#endif
+#ifndef TARGET_LLONG_ALIGNMENT
+#define TARGET_LLONG_ALIGNMENT 8
+#endif
+
 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)));
diff --git a/target-alpha/cpu.h b/target-alpha/cpu.h
index 2156a1e..b0338fb 100644
--- a/target-alpha/cpu.h
+++ b/target-alpha/cpu.h
@@ -24,6 +24,7 @@
 #include "qemu-common.h"
 
 #define TARGET_LONG_BITS 64
+#define TARGET_LONG_ALIGNMENT 8
 
 #define CPUArchState struct CPUAlphaState
 
diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index 2b97221..9a8bb03 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 TARGET_LLONG_ALIGNMENT 4
 
 #define ELF_MACHINE	EM_ARM
 
diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index 48f41ca..f1ef87c 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -24,6 +24,7 @@
 
 #ifdef TARGET_X86_64
 #define TARGET_LONG_BITS 64
+#define TARGET_LONG_ALIGNMENT 8
 #else
 #define TARGET_LONG_BITS 32
 #endif
diff --git a/target-m68k/cpu.h b/target-m68k/cpu.h
index c90c40c..74612a9 100644
--- a/target-m68k/cpu.h
+++ b/target-m68k/cpu.h
@@ -21,6 +21,9 @@
 #define CPU_M68K_H
 
 #define TARGET_LONG_BITS 32
+#define TARGET_INT_ALIGNMENT 2
+#define TARGET_LONG_ALIGNMENT 2
+#define TARGET_LLONG_ALIGNMENT 2
 
 #define CPUArchState struct CPUM68KState
 
diff --git a/target-mips/mips-defs.h b/target-mips/mips-defs.h
index bf094a3..624a015 100644
--- a/target-mips/mips-defs.h
+++ b/target-mips/mips-defs.h
@@ -10,6 +10,7 @@
 
 #if defined(TARGET_MIPS64)
 #define TARGET_LONG_BITS 64
+#define TARGET_LONG_ALIGNMENT 8
 #define TARGET_PHYS_ADDR_SPACE_BITS 36
 #define TARGET_VIRT_ADDR_SPACE_BITS 42
 #else
diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
index 42c36e2..19d4e4c 100644
--- a/target-ppc/cpu.h
+++ b/target-ppc/cpu.h
@@ -27,6 +27,7 @@
 #if defined (TARGET_PPC64)
 /* PowerPC 64 definitions */
 #define TARGET_LONG_BITS 64
+#define TARGET_LONG_ALIGNMENT 8
 #define TARGET_PAGE_BITS 12
 
 /* Note that the official physical address space bits is 62-M where M
diff --git a/target-s390x/cpu.h b/target-s390x/cpu.h
index e351005..8a241d3 100644
--- a/target-s390x/cpu.h
+++ b/target-s390x/cpu.h
@@ -26,6 +26,7 @@
 #include "qemu-common.h"
 
 #define TARGET_LONG_BITS 64
+#define TARGET_LONG_ALIGNMENT 8
 
 #define ELF_MACHINE	EM_S390
 
diff --git a/target-sparc/cpu.h b/target-sparc/cpu.h
index 6fa7778..9e5941a 100644
--- a/target-sparc/cpu.h
+++ b/target-sparc/cpu.h
@@ -13,6 +13,7 @@
 #define TARGET_VIRT_ADDR_SPACE_BITS 32
 #else
 #define TARGET_LONG_BITS 64
+#define TARGET_LONG_ALIGNMENT 8
 #define TARGET_DPREGS 32
 #define TARGET_PAGE_BITS 13 /* 8k */
 #define TARGET_PHYS_ADDR_SPACE_BITS 41
-- 
1.8.1.4

Re: [Qemu-devel] [PATCH 1/5] configure: move TARGET_*_ALIGNMENT to target-*/cpu.h

[Peter Maydell]

On 2 April 2013 15:44, Paolo Bonzini <pbonzini@redhat.com> wrote:
> This is no different from, for example, TARGET_LONG_BITS.  It does
> not belong in configure.

> diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
> index 42c36e2..19d4e4c 100644
> --- a/target-ppc/cpu.h
> +++ b/target-ppc/cpu.h
> @@ -27,6 +27,7 @@
>  #if defined (TARGET_PPC64)
>  /* PowerPC 64 definitions */
>  #define TARGET_LONG_BITS 64
> +#define TARGET_LONG_ALIGNMENT 8
>  #define TARGET_PAGE_BITS 12

Doesn't this incorrectly set the long alignment to 8
for ppc64abi32? (Probably similar problem for
sparc32plus and mipsn32. The underlying point here is that
alignment is an ABI decision and you can have more than one
ABI for a particular TARGET_FOO.)

-- PMM

Re: [Qemu-devel] [PATCH 1/5] configure: move TARGET_*_ALIGNMENT to target-*/cpu.h

[Paolo Bonzini]

Il 02/04/2013 18:43, Peter Maydell ha scritto:
> On 2 April 2013 15:44, Paolo Bonzini <pbonzini@redhat.com> wrote:
>> This is no different from, for example, TARGET_LONG_BITS.  It does
>> not belong in configure.
> 
>> diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
>> index 42c36e2..19d4e4c 100644
>> --- a/target-ppc/cpu.h
>> +++ b/target-ppc/cpu.h
>> @@ -27,6 +27,7 @@
>>  #if defined (TARGET_PPC64)
>>  /* PowerPC 64 definitions */
>>  #define TARGET_LONG_BITS 64
>> +#define TARGET_LONG_ALIGNMENT 8
>>  #define TARGET_PAGE_BITS 12
> 
> Doesn't this incorrectly set the long alignment to 8
> for ppc64abi32? (Probably similar problem for
> sparc32plus and mipsn32. The underlying point here is that
> alignment is an ABI decision and you can have more than one
> ABI for a particular TARGET_FOO.)

Hmm, seems like you're right _but_ I am not sure if the *current* code
is correct.  On real hardware, the CPUs are certainly not able to do
unaligned 32-bit accesses, and target_long/target_ulong pointers look
like they're never used for data that comes from target memory.  Rather,
they're used for by-reference passing into functions, and stuff like that.

What these targets want to have 32-bit alignment is really
abi_long/abi_ulong, and that's already okay.  Alex, Blue, Aurelien, can
you test the above three targets?

Paolo

Re: [Qemu-devel] [PATCH 1/5] configure: move TARGET_*_ALIGNMENT to target-*/cpu.h

[Peter Maydell]

On 2 April 2013 17:56, Paolo Bonzini <pbonzini@redhat.com> wrote:
> Il 02/04/2013 18:43, Peter Maydell ha scritto:
>> Doesn't this incorrectly set the long alignment to 8
>> for ppc64abi32? (Probably similar problem for
>> sparc32plus and mipsn32. The underlying point here is that
>> alignment is an ABI decision and you can have more than one
>> ABI for a particular TARGET_FOO.)
>
> Hmm, seems like you're right _but_ I am not sure if the *current* code
> is correct.  On real hardware, the CPUs are certainly not able to do
> unaligned 32-bit accesses, and target_long/target_ulong pointers look
> like they're never used for data that comes from target memory.

Did you check linux-user too? I'm pretty sure we have structs
and so on that mirror target memory and use target_ulong.

> What these targets want to have 32-bit alignment is really
> abi_long/abi_ulong, and that's already okay. Alex, Blue, Aurelien,
> can you test the above three targets?

Mmm, rather than speculating we should just confirm what gcc
thinks the alignment of void* should be on these targets
(since "a thing the size of a pointer" is what target_long/ulong
represent, I think.)


That said, we should keep bugfixes and cleanup patches separated,
so on approach for proceeding with these cleanup patches is just
to define TARGET_LONG_ALIGNMENT based on TARGET_ABI32 or whatever is
appropriate for each target CPU. Then we retain the same behaviour.

-- PMM

Re: [Qemu-devel] [PATCH 1/5] configure: move TARGET_*_ALIGNMENT to target-*/cpu.h

[Paolo Bonzini]

Il 02/04/2013 19:17, Peter Maydell ha scritto:
> On 2 April 2013 17:56, Paolo Bonzini <pbonzini@redhat.com> wrote:
>> Il 02/04/2013 18:43, Peter Maydell ha scritto:
>>> Doesn't this incorrectly set the long alignment to 8
>>> for ppc64abi32? (Probably similar problem for
>>> sparc32plus and mipsn32. The underlying point here is that
>>> alignment is an ABI decision and you can have more than one
>>> ABI for a particular TARGET_FOO.)
>>
>> Hmm, seems like you're right _but_ I am not sure if the *current* code
>> is correct.  On real hardware, the CPUs are certainly not able to do
>> unaligned 32-bit accesses, and target_long/target_ulong pointers look
>> like they're never used for data that comes from target memory.
> 
> Did you check linux-user too? I'm pretty sure we have structs
> and so on that mirror target memory and use target_ulong.
> 
>> What these targets want to have 32-bit alignment is really
>> abi_long/abi_ulong, and that's already okay. Alex, Blue, Aurelien,
>> can you test the above three targets?
> 
> Mmm, rather than speculating we should just confirm what gcc
> thinks the alignment of void* should be on these targets
> (since "a thing the size of a pointer" is what target_long/ulong
> represent, I think.)

I think "a thing the size of a pointer" should be abi_long/ulong.  The
pointer is not a CPU concept.

> That said, we should keep bugfixes and cleanup patches separated,

Indeed.  The change was unintended, and your comment is worth a respin.

> so on approach for proceeding with these cleanup patches is just
> to define TARGET_LONG_ALIGNMENT based on TARGET_ABI32 or whatever is
> appropriate for each target CPU. Then we retain the same behaviour.

Yes, TARGET_ABI32.  I'll have to squash patches 1 and 2, and the testing
RFH still holds because this alignment things sounds fishy...

Paolo

Re: [Qemu-devel] [PATCH 1/5] configure: move TARGET_*_ALIGNMENT to target-*/cpu.h

[Peter Maydell]

On 2 April 2013 18:26, Paolo Bonzini <pbonzini@redhat.com> wrote:
> I think "a thing the size of a pointer" should be abi_long/ulong.  The
> pointer is not a CPU concept.

Yeah. OTOH type alignment isn't a CPU concept either, so I'm
a little suspicious of these defines in general.

-- PMM

Re: [Qemu-devel] [PATCH 1/5] configure: move TARGET_*_ALIGNMENT to target-*/cpu.h

[Paolo Bonzini]

Il 02/04/2013 19:47, Peter Maydell ha scritto:
> On 2 April 2013 18:26, Paolo Bonzini <pbonzini@redhat.com> wrote:
>> I think "a thing the size of a pointer" should be abi_long/ulong.  The
>> pointer is not a CPU concept.
> 
> Yeah. OTOH type alignment isn't a CPU concept either, so I'm
> a little suspicious of these defines in general.

Ok, the main case where the target alignment matters is in 'struct
target_elf_prstatus' (linux-user/elfload.c).

Linux, in its n32 implementation, explicitly uses a different struct
that changes some longs to ints (pr_sigpend, pr_sighold, pr_flag) and
keeps longs for others (pr_reg).

---
typedef unsigned long elf_greg_t;
typedef elf_greg_t elf_gregset_t[ELF_NGREG];

#define elf_prstatus elf_prstatus32
struct elf_prstatus32
{
        struct elf_siginfo pr_info;
        short   pr_cursig;              /* Current signal */
        unsigned int pr_sigpend;        /* Set of pending signals */
        unsigned int pr_sighold;        /* Set of held signals */
        pid_t   pr_pid;
        pid_t   pr_ppid;
        pid_t   pr_pgrp;
        pid_t   pr_sid;
        struct compat_timeval pr_utime; /* User time */
        struct compat_timeval pr_stime; /* System time */
        struct compat_timeval pr_cutime;/* Cumulative user time */
        struct compat_timeval pr_cstime;/* Cumulative system time */
        elf_gregset_t pr_reg;   /* GP registers */
        int pr_fpvalid;
};
---

Instead, we use target_ulong for both (possibly via the
target_elf_greg_t typedef).

sparc32plus and ppc64abi32 instead use 32-bit for pr_reg too (see
include/linux/elfcore-compat.h and fs/compat_binfmt_elf.c).  This is
also wrong.

In any case, what we are doing is doubly wrong.  Things that have 4-byte
alignment should also have 4-byte size.  Things that have 8-byte
alignment should also have 8-byte size.

Paolo

Re: [Qemu-devel] [PATCH 1/5] configure: move TARGET_*_ALIGNMENT to target-*/cpu.h

[Aurelien Jarno]

On Tue, Apr 02, 2013 at 05:43:36PM +0100, Peter Maydell wrote:
> On 2 April 2013 15:44, Paolo Bonzini <pbonzini@redhat.com> wrote:
> > This is no different from, for example, TARGET_LONG_BITS.  It does
> > not belong in configure.
> 
> > diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
> > index 42c36e2..19d4e4c 100644
> > --- a/target-ppc/cpu.h
> > +++ b/target-ppc/cpu.h
> > @@ -27,6 +27,7 @@
> >  #if defined (TARGET_PPC64)
> >  /* PowerPC 64 definitions */
> >  #define TARGET_LONG_BITS 64
> > +#define TARGET_LONG_ALIGNMENT 8
> >  #define TARGET_PAGE_BITS 12
> 
> Doesn't this incorrectly set the long alignment to 8
> for ppc64abi32? (Probably similar problem for
> sparc32plus and mipsn32. The underlying point here is that
> alignment is an ABI decision and you can have more than one
> ABI for a particular TARGET_FOO.)
> 

I'll do a test for mipsn32, but first I'll try to look how/where it is
used.

Note that ppc64abi32 and sparc32plus are different than mipsn32. The
first two are basically executing a 32-bit binary (with the 32-bit
ABI) on a 64-bit emulated CPU, with for sparc access to more
instructions. mipsn32 is a different ABI using 64-bit registers, but
32-bit pointers, simlar to x32.

-- 
Aurelien Jarno	                        GPG: 1024D/F1BCDB73
aurelien@aurel32.net                 http://www.aurel32.net

[Qemu-devel] [PATCH 2/5] cpu: default TARGET_LONG_ALIGNMENT to TARGET_LONG_SIZE

[Paolo Bonzini]

This is the case for all supported 64-bits architectures.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 include/exec/cpu-defs.h | 2 +-
 target-alpha/cpu.h      | 1 -
 target-i386/cpu.h       | 1 -
 target-ppc/cpu.h        | 1 -
 target-s390x/cpu.h      | 1 -
 target-sparc/cpu.h      | 1 -
 6 files changed, 1 insertion(+), 6 deletions(-)

diff --git a/include/exec/cpu-defs.h b/include/exec/cpu-defs.h
index 32d11e8..7d23fc2 100644
--- a/include/exec/cpu-defs.h
+++ b/include/exec/cpu-defs.h
@@ -43,7 +43,7 @@
 #define TARGET_INT_ALIGNMENT 4
 #endif
 #ifndef TARGET_LONG_ALIGNMENT
-#define TARGET_LONG_ALIGNMENT 4
+#define TARGET_LONG_ALIGNMENT TARGET_LONG_SIZE
 #endif
 #ifndef TARGET_LLONG_ALIGNMENT
 #define TARGET_LLONG_ALIGNMENT 8
diff --git a/target-alpha/cpu.h b/target-alpha/cpu.h
index b0338fb..2156a1e 100644
--- a/target-alpha/cpu.h
+++ b/target-alpha/cpu.h
@@ -24,7 +24,6 @@
 #include "qemu-common.h"
 
 #define TARGET_LONG_BITS 64
-#define TARGET_LONG_ALIGNMENT 8
 
 #define CPUArchState struct CPUAlphaState
 
diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index f1ef87c..48f41ca 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -24,7 +24,6 @@
 
 #ifdef TARGET_X86_64
 #define TARGET_LONG_BITS 64
-#define TARGET_LONG_ALIGNMENT 8
 #else
 #define TARGET_LONG_BITS 32
 #endif
diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
index 19d4e4c..42c36e2 100644
--- a/target-ppc/cpu.h
+++ b/target-ppc/cpu.h
@@ -27,7 +27,6 @@
 #if defined (TARGET_PPC64)
 /* PowerPC 64 definitions */
 #define TARGET_LONG_BITS 64
-#define TARGET_LONG_ALIGNMENT 8
 #define TARGET_PAGE_BITS 12
 
 /* Note that the official physical address space bits is 62-M where M
diff --git a/target-s390x/cpu.h b/target-s390x/cpu.h
index 8a241d3..e351005 100644
--- a/target-s390x/cpu.h
+++ b/target-s390x/cpu.h
@@ -26,7 +26,6 @@
 #include "qemu-common.h"
 
 #define TARGET_LONG_BITS 64
-#define TARGET_LONG_ALIGNMENT 8
 
 #define ELF_MACHINE	EM_S390
 
diff --git a/target-sparc/cpu.h b/target-sparc/cpu.h
index 9e5941a..6fa7778 100644
--- a/target-sparc/cpu.h
+++ b/target-sparc/cpu.h
@@ -13,7 +13,6 @@
 #define TARGET_VIRT_ADDR_SPACE_BITS 32
 #else
 #define TARGET_LONG_BITS 64
-#define TARGET_LONG_ALIGNMENT 8
 #define TARGET_DPREGS 32
 #define TARGET_PAGE_BITS 13 /* 8k */
 #define TARGET_PHYS_ADDR_SPACE_BITS 41
-- 
1.8.1.4

Re: [Qemu-devel] [PATCH 2/5] cpu: default TARGET_LONG_ALIGNMENT to TARGET_LONG_SIZE

[Peter Maydell]

On 2 April 2013 15:44, Paolo Bonzini <pbonzini@redhat.com> wrote:
> This is the case for all supported 64-bits architectures.

Being sensible with the defaults is good, but I wonder if
we could structure cpu-defs.h (or somewhere else) so there
was a nice clear list of "here are all the things you need
to either define for your new target or check that the
defaults are sensible". Especially for things like the
type alignment it's easy to end up with something that
only mostly works if the default isn't correct for you.

Or maybe I'm being overoptimistic about whether new target
authors would actually check through the list anyway :-)

-- PMM

[Qemu-devel] [PATCH 3/5] configure: move CONFIG_QEMU_LDST_OPTIMIZATION to config-host.mak

[Paolo Bonzini]

Its value is the same for all targets.

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

diff --git a/configure b/configure
index 5d9a87e..629f70b 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
@@ -4133,15 +4142,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

Re: [Qemu-devel] [PATCH 3/5] configure: move CONFIG_QEMU_LDST_OPTIMIZATION to config-host.mak

[Peter Maydell]

On 2 April 2013 15:44, Paolo Bonzini <pbonzini@redhat.com> wrote:
> Its value is the same for all targets.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

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

-- PMM

[Qemu-devel] [PATCH 4/5] configure: move common libraries to config-host.mak

[Paolo Bonzini]

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

diff --git a/Makefile.target b/Makefile.target
index 2bd6d14..c8513f1 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -119,7 +119,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
diff --git a/configure b/configure
index 629f70b..da0e8aa 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
 
@@ -4192,7 +4194,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 4/5] configure: move common libraries to config-host.mak

[Peter Maydell]

On 2 April 2013 15:44, Paolo Bonzini <pbonzini@redhat.com> wrote:
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

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

I think this may perturb the order of the link line slightly,
but hopefully not in any way that matters. (Did you test a
static build?)

-- PMM

Re: [Qemu-devel] [PATCH 4/5] configure: move common libraries to config-host.mak

[Paolo Bonzini]

Il 02/04/2013 19:01, Peter Maydell ha scritto:
>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
> 
> I think this may perturb the order of the link line slightly,
> but hopefully not in any way that matters. (Did you test a
> static build?)

I don't have static versions of half of the prerequisites of system
emulators...

Paolo

Re: [Qemu-devel] [PATCH 4/5] configure: move common libraries to config-host.mak

[Peter Maydell]

On 2 April 2013 18:26, Paolo Bonzini <pbonzini@redhat.com> wrote:
> Il 02/04/2013 19:01, Peter Maydell ha scritto:
>> I think this may perturb the order of the link line slightly,
>> but hopefully not in any way that matters. (Did you test a
>> static build?)
>
> I don't have static versions of half of the prerequisites of system
> emulators...

Really? Ubuntu provides them, maybe you should complain to your
distro vendor :-)

Anyway, I did a compile test of everything with --static and it
does turn out to be busted:

ccache gcc -Werror -m64 -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64
-D_LARGEFILE_SOURCE -Wstrict-prototypes -Wredundant-decls -Wall
-Wundef -Wwrite-strings -Wmissing-prototypes -fno-strict-aliasing
-fstack-protector-all -Wendif-labels -Wmissing-include-dirs
-Wempty-body -Wnested-externs -Wformat-security -Wformat-y2k
-Winit-self -Wignored-qualifiers -Wold-style-declaration
-Wold-style-definition -Wtype-limits -I/usr/include/libpng12
-I/usr/include/pixman-1   -DHAS_AUDIO -DHAS_AUDIO_CHOICE  -I. -I.
-I../linux-headers -I..
-I/home/petmay01/linaro/qemu-from-laptop/qemu/target-i386 -DNEED_CPU_H
-I/home/petmay01/linaro/qemu-from-laptop/qemu/include -pthread
-I/usr/include/glib-2.0 -I/usr/lib/x86_64-linux-gnu/glib-2.0/include
-g  -Wl,--warn-common -m64 -static -g   -o qemu-system-i386
../aio-posix.o [.o list elided] translate-all.o xen-stub.o
../libqemuutil.a ../libqemustub.a  -lrt -pthread -lgthread-2.0
-lglib-2.0 -lrt -lpcre     -lm -lutil -lbluetooth   -lncurses -ltinfo
  -lvdeplug -luuid -lpng12 -lz -lm    -lz -laio -lfdt -lpixman-1
[usual complaints about glibc functions needing the libraries
at runtime for NSS stuff elided]
/usr/lib/gcc/x86_64-linux-gnu/4.6/../../../x86_64-linux-gnu/libpixman-1.a(pixman-radial-gradient.o):
In function `radial_compute_color':
(.text+0x16f): undefined reference to `sqrt'
/usr/lib/gcc/x86_64-linux-gnu/4.6/../../../x86_64-linux-gnu/libpixman-1.a(pixman-combine32.o):
In function `combine_soft_light_ca':
(.text+0xa1e7): undefined reference to `sqrt'
/usr/lib/gcc/x86_64-linux-gnu/4.6/../../../x86_64-linux-gnu/libpixman-1.a(pixman-combine32.o):
In function `combine_soft_light_ca':
(.text+0xa25e): undefined reference to `sqrt'
/usr/lib/gcc/x86_64-linux-gnu/4.6/../../../x86_64-linux-gnu/libpixman-1.a(pixman-combine32.o):
In function `combine_soft_light_ca':
(.text+0xa2cc): undefined reference to `sqrt'
/usr/lib/gcc/x86_64-linux-gnu/4.6/../../../x86_64-linux-gnu/libpixman-1.a(pixman-combine32.o):
In function `combine_soft_light_u':
(.text+0xab46): undefined reference to `sqrt'
/usr/lib/gcc/x86_64-linux-gnu/4.6/../../../x86_64-linux-gnu/libpixman-1.a(pixman-combine32.o):(.text+0xabfc):
more undefined references to `sqrt' follow
collect2: ld returned 1 exit status
make[1]: *** [qemu-system-i386] Error 1
make[1]: Leaving directory
`/home/petmay01/linaro/qemu-from-laptop/qemu/i386-softmmu'
make: *** [subdir-i386-softmmu] Error 2

This is because we seem to have lost the trailing "-lm -lz"; here's
the tail end of the link command from a build on master for
comparison:

translate-all.o xen-stub.o ../libqemuutil.a ../libqemustub.a  -lrt
-pthread -lgthread-2.0 -lglib-2.0 -lrt -lpcre    -lutil -lbluetooth
-lncurses -ltinfo    -lvdeplug -luuid -lpng12 -lz -lm    -laio -lfdt
-lpixman-1    -lm -lz

thanks
-- PMM

Re: [Qemu-devel] [PATCH 4/5] configure: move common libraries to config-host.mak

[Paolo Bonzini]

Il 02/04/2013 20:17, Peter Maydell ha scritto:

> This is because we seem to have lost the trailing "-lm -lz"; here's
> the tail end of the link command from a build on master for
> comparison:
> 
> translate-all.o xen-stub.o ../libqemuutil.a ../libqemustub.a  -lrt
> -pthread -lgthread-2.0 -lglib-2.0 -lrt -lpcre    -lutil -lbluetooth
> -lncurses -ltinfo    -lvdeplug -luuid -lpng12 -lz -lm    -laio -lfdt
> -lpixman-1    -lm -lz

I can fix this, but apparently upstream pixman doesn't support static linking:

$ pkg-config pixman-1 --libs
-lpixman-1  
$ pkg-config pixman-1 --libs --static
-lpixman-1  

which is why my distro vendor does not provide static libraries. :)

Here is the fix:

diff --git a/Makefile.target b/Makefile.target
index c8513f1..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
 
@@ -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

Paolo

[Qemu-devel] [PATCH 5/5] 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.

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 da0e8aa..98716ae 100755
--- a/configure
+++ b/configure
@@ -4161,12 +4161,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

Re: [Qemu-devel] [PATCH 5/5] configure: CONFIG_NO_XEN is duplicated

[Stefano Stabellini]

On Tue, 2 Apr 2013, Paolo Bonzini wrote:
> We already define it in Makefile.target.  But we need to avoid a
> curious double negation in order to eliminate it.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

Tested-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>


>  configure | 3 ---
>  hw/xen.h  | 2 +-
>  2 files changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/configure b/configure
> index da0e8aa..98716ae 100755
> --- a/configure
> +++ b/configure
> @@ -4161,12 +4161,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
> 
> 

Re: [Qemu-devel] [PATCH 5/5] configure: CONFIG_NO_XEN is duplicated

[Peter Maydell]

On 2 April 2013 15:44, Paolo Bonzini <pbonzini@redhat.com> wrote:
> We already define it in Makefile.target.  But we need to avoid a
> curious double negation in order to eliminate it.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

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

CONFIG_NO_FOO should only be used in makefiles; I
checked, and the CONFIG_NO_XEN you eliminate here is
the only one that had strayed into source code.

(The real trick of course would be to be able to define
CONFIG_TEA and CONFIG_NO_TEA at the same time.)

-- PMM