Re: [Qemu-devel] [PATCH] build: introduce target CONFIG_ variables and use them for kvm

[Peter Maydell <peter.maydell@linaro.org>]

On 20 June 2012 15:44, Anthony Liguori <aliguori@us.ibm.com> wrote:
> This avoids the problem associated with having multiple target specific files
> 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.
> ---
>  configure             |   30 ++++++++++++++++++++----------
>  hw/Makefile.objs      |    2 ++
>  hw/i386/Makefile.objs |    1 -
>  hw/kvm/Makefile.objs  |    2 +-
>  4 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
>   ;;
>  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="`echo $TARGET_ARCH | LC_ALL=C tr '[a-z]' '[A-Z]'`"
> -echo "TARGET_$target_arch_name=y" >> $config_target_mak
> -echo "TARGET_ARCH2=$target_arch2" >> $config_target_mak
> -echo "TARGET_BASE_ARCH=$TARGET_BASE_ARCH" >> $config_target_mak
> +upper() {
> +    echo "$@" | LC_ALL=C tr '[a-z]' '[A-Z]'
> +}
> +
> +target_arch_name="`upper $TARGET_ARCH`"
>  if [ "$TARGET_ABI_DIR" = "" ]; then
>   TARGET_ABI_DIR=$TARGET_ARCH
>  fi
> -echo "TARGET_ABI_DIR=$TARGET_ABI_DIR" >> $config_target_mak
> +
> +cat <<EOF >> $config_target_mak
> +TARGET_SHORT_ALIGNMENT=$target_short_alignment
> +TARGET_INT_ALIGNMENT=$target_int_alignment
> +TARGET_LONG_ALIGNMENT=$target_long_alignment
> +TARGET_LLONG_ALIGNMENT=$target_llong_alignment
> +TARGET_ARCH=$TARGET_ARCH
> +TARGET_$target_arch_name=y
> +TARGET_ARCH2=$target_arch2
> +TARGET_BASE_ARCH=$TARGET_BASE_ARCH
> +TARGET_ABI_DIR=$TARGET_ABI_DIR
> +CONFIG_`upper $TARGET_BASE_ARCH`=y
> +CONFIG_`upper $TARGET_ARCH`=y
> +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.

>  case "$target_arch2" in
>   i386|x86_64)
>     if test "$xen" = "yes" -a "$target_softmmu" = "yes" ; then
> 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) += vga.o
>  obj-$(CONFIG_SOFTMMU) += device-hotplug.o
>  obj-$(CONFIG_XEN) += xen_domainbuild.o xen_machine_pv.o
>

I think this blank line is superfluous.

> +obj-$(CONFIG_KVM) += kvm/
> +
>  # Inter-VM PCI shared memory
>  ifeq ($(CONFIG_PCI), y)
>  obj-$(CONFIG_KVM) += ivshmem.o

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

-- PMM