From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([208.118.235.92]:58522)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <peter.maydell@linaro.org>) id 1ShMX1-0007ox-Sy
	for qemu-devel@nongnu.org; Wed, 20 Jun 2012 11:09:25 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <peter.maydell@linaro.org>) id 1ShMWw-0005Es-Uz
	for qemu-devel@nongnu.org; Wed, 20 Jun 2012 11:09:19 -0400
Received: from mail-bk0-f45.google.com ([209.85.214.45]:60606)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <peter.maydell@linaro.org>) id 1ShMWw-0005Eg-OH
	for qemu-devel@nongnu.org; Wed, 20 Jun 2012 11:09:14 -0400
Received: by bkwj10 with SMTP id j10so6676888bkw.4
	for <qemu-devel@nongnu.org>; Wed, 20 Jun 2012 08:09:12 -0700 (PDT)
MIME-Version: 1.0
In-Reply-To: <1340203444-20394-1-git-send-email-aliguori@us.ibm.com>
References: <1340203444-20394-1-git-send-email-aliguori@us.ibm.com>
Date: Wed, 20 Jun 2012 16:09:12 +0100
Message-ID: <CAFEAcA8s1gJ=1=Zx7dvbsiUzAt+5d+uC7UkCiLz0c9N0Crpxrg@mail.gmail.com>
From: Peter Maydell <peter.maydell@linaro.org>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable
Subject: Re: [Qemu-devel] [PATCH] build: introduce target CONFIG_ variables
 and use them for kvm
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Anthony Liguori <aliguori@us.ibm.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>, qemu-devel@nongnu.org, Andreas Faerber <afaerber@suse.de>

On 20 June 2012 15:44, Anthony Liguori <aliguori@us.ibm.com> wrote:
> This avoids the problem associated with having multiple target specific f=
iles
> in a single directory with the current build system.
>
> We can eventually get rid of the hw/$BASE_ARCH/Makefiles.obj files too
>
> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
> ---
> I tried to add a nice comment to the config-target.mak that described how=
 to
> use these macros but that upset the header generation script.
>
> So I left this out of this patch.
> ---
> =C2=A0configure =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 | =C2=A0 30 +++=
+++++++++++++++++----------
> =C2=A0hw/Makefile.objs =C2=A0 =C2=A0 =C2=A0| =C2=A0 =C2=A02 ++
> =C2=A0hw/i386/Makefile.objs | =C2=A0 =C2=A01 -
> =C2=A0hw/kvm/Makefile.objs =C2=A0| =C2=A0 =C2=A02 +-
> =C2=A04 files changed, 23 insertions(+), 12 deletions(-)
>
> diff --git a/configure b/configure
> index b68c0ca..f07c464 100755
> --- a/configure
> +++ b/configure
> @@ -3684,19 +3684,29 @@ case "$target_arch2" in
> =C2=A0 ;;
> =C2=A0esac
>
> -echo "TARGET_SHORT_ALIGNMENT=3D$target_short_alignment" >> $config_targe=
t_mak
> -echo "TARGET_INT_ALIGNMENT=3D$target_int_alignment" >> $config_target_ma=
k
> -echo "TARGET_LONG_ALIGNMENT=3D$target_long_alignment" >> $config_target_=
mak
> -echo "TARGET_LLONG_ALIGNMENT=3D$target_llong_alignment" >> $config_targe=
t_mak
> -echo "TARGET_ARCH=3D$TARGET_ARCH" >> $config_target_mak
> -target_arch_name=3D"`echo $TARGET_ARCH | LC_ALL=3DC tr '[a-z]' '[A-Z]'`"
> -echo "TARGET_$target_arch_name=3Dy" >> $config_target_mak
> -echo "TARGET_ARCH2=3D$target_arch2" >> $config_target_mak
> -echo "TARGET_BASE_ARCH=3D$TARGET_BASE_ARCH" >> $config_target_mak
> +upper() {
> + =C2=A0 =C2=A0echo "$@" | LC_ALL=3DC tr '[a-z]' '[A-Z]'
> +}
> +
> +target_arch_name=3D"`upper $TARGET_ARCH`"
> =C2=A0if [ "$TARGET_ABI_DIR" =3D "" ]; then
> =C2=A0 TARGET_ABI_DIR=3D$TARGET_ARCH
> =C2=A0fi
> -echo "TARGET_ABI_DIR=3D$TARGET_ABI_DIR" >> $config_target_mak
> +
> +cat <<EOF >> $config_target_mak
> +TARGET_SHORT_ALIGNMENT=3D$target_short_alignment
> +TARGET_INT_ALIGNMENT=3D$target_int_alignment
> +TARGET_LONG_ALIGNMENT=3D$target_long_alignment
> +TARGET_LLONG_ALIGNMENT=3D$target_llong_alignment
> +TARGET_ARCH=3D$TARGET_ARCH
> +TARGET_$target_arch_name=3Dy
> +TARGET_ARCH2=3D$target_arch2
> +TARGET_BASE_ARCH=3D$TARGET_BASE_ARCH
> +TARGET_ABI_DIR=3D$TARGET_ABI_DIR
> +CONFIG_`upper $TARGET_BASE_ARCH`=3Dy
> +CONFIG_`upper $TARGET_ARCH`=3Dy
> +EOF
> +

I'd prefer the rearrangement of the existing code to be in
a separate patch -- otherwise it's a bit hard to tell what's been
added here. A brief comment describing the difference between
TARGET_I386 and CONFIG_I386 would also be good.

> =C2=A0case "$target_arch2" in
> =C2=A0 i386|x86_64)
> =C2=A0 =C2=A0 if test "$xen" =3D "yes" -a "$target_softmmu" =3D "yes" ; t=
hen
> diff --git a/hw/Makefile.objs b/hw/Makefile.objs
> index 3d77259..cee0e06 100644
> --- a/hw/Makefile.objs
> +++ b/hw/Makefile.objs
> @@ -166,6 +166,8 @@ obj-$(CONFIG_VGA) +=3D vga.o
> =C2=A0obj-$(CONFIG_SOFTMMU) +=3D device-hotplug.o
> =C2=A0obj-$(CONFIG_XEN) +=3D xen_domainbuild.o xen_machine_pv.o
>

I think this blank line is superfluous.

> +obj-$(CONFIG_KVM) +=3D kvm/
> +
> =C2=A0# Inter-VM PCI shared memory
> =C2=A0ifeq ($(CONFIG_PCI), y)
> =C2=A0obj-$(CONFIG_KVM) +=3D ivshmem.o

Otherwise looks good, and I've given it a quick smoke test and
it seems to DTRT.

-- PMM