Re: [Qemu-devel] [RFC PATCH 01/10] configure: factor out list of supported Xen/KVM targets

[Peter Maydell]

On 17 September 2012 17:00, Paolo Bonzini <address@hidden> wrote:
> Signed-off-by: Paolo Bonzini <address@hidden>
> ---
>  configure | 63 
> +++++++++++++++++++++++++++++++++++++--------------------------
>  1 file modificato, 37 inserzioni(+), 26 rimozioni(-)
>
> diff --git a/configure b/configure
> index 7e23309..cea8db0 100755
> --- a/configure
> +++ b/configure
> @@ -3664,6 +3664,31 @@ if test "$linux" = "yes" ; then
>      fi
>  fi
>
> +supported_kvm_target() {
> +    test "$kvm" = "yes" || return 1
> +    test "$target_softmmu" = "yes" || return 1
> +    case "$target_arch2:$cpu" in
> +        i386:i386 | i386:x86_64 | x86_64:i386 | x86_64:x86_64 | \
> +        ppc:ppc | ppcemb:ppc | ppc64:ppc | \
> +        ppc:ppc64 | ppcemb:ppc64 | ppc64:ppc64 | \
> +        s390x:s390x)
> +            return 0
> +        ;;
> +    esac
> +    return 1
> +}
> +
> +supported_xen_target() {
> +    test "$xen" = "yes" || return 1
> +    test "$target_softmmu" = "yes" || return 1
> +    case "$target_arch2:$cpu" in
> +        i386:i386 | i386:x86_64 | x86_64:i386 | x86_64:x86_64)
> +            return 0
> +        ;;
> +    esac
> +    return 1
> +}

This is a change in behaviour, isn't it? The old configure code
enables CONFIG_XEN based only on $target_arch2, it doesn't try
to ensure that the host and target CPU are the same architecture.
Granted, that looks like a bug, but it should probably be fixed
in a separate patch to this kind of refactoring patch.

> +
>  for target in $target_list; do
>  target_dir="$target"
>  config_target_mak=$target_dir/config-target.mak
> @@ -3886,38 +3911,24 @@ if [ "$TARGET_ABI_DIR" = "" ]; then
>    TARGET_ABI_DIR=$TARGET_ARCH
>  fi
>  echo "TARGET_ABI_DIR=$TARGET_ABI_DIR" >> $config_target_mak
> -case "$target_arch2" in
> -  i386|x86_64)
> -    if test "$xen" = "yes" -a "$target_softmmu" = "yes" ; then
> -      target_phys_bits=64
> -      echo "CONFIG_XEN=y" >> $config_target_mak
> -      if test "$xen_pci_passthrough" = yes; then
> +
> +if supported_xen_target; then
> +    target_phys_bits=64

...and while I'm looking at this code, this looks kind of bogus.
If Xen requires 64 bit physaddrs it should probably just be asserting
this here, not randomly changing target_phys_bits. In fact all the
supported Xen target archs already have 64 bit physaddrs, so it's
harmless. But if there ever were a target Xen arch which didn't support
64 bit phys addrs then the right approach would be to convert it to
do so regardless of whether we were using Xen or not.

> +    echo "CONFIG_XEN=y" >> $config_target_mak
> +    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
> -    ;;
> -  *)
> +else
>      echo "CONFIG_NO_XEN=y" >> $config_target_mak
> -esac
> -case "$target_arch2" in
> -  i386|x86_64|ppcemb|ppc|ppc64|s390x)
> +fi
> +if supported_kvm_target; then
>      # Make sure the target and host cpus are compatible
> -    if test "$kvm" = "yes" -a "$target_softmmu" = "yes" -a \
> -      \( "$target_arch2" = "$cpu" -o \
> -      \( "$target_arch2" = "ppcemb" -a "$cpu" = "ppc" \) -o \
> -      \( "$target_arch2" = "ppc64"  -a "$cpu" = "ppc" \) -o \
> -      \( "$target_arch2" = "ppc"    -a "$cpu" = "ppc64" \) -o \
> -      \( "$target_arch2" = "ppcemb" -a "$cpu" = "ppc64" \) -o \
> -      \( "$target_arch2" = "x86_64" -a "$cpu" = "i386"   \) -o \
> -      \( "$target_arch2" = "i386"   -a "$cpu" = "x86_64" \) \) ; then
> -      echo "CONFIG_KVM=y" >> $config_target_mak
> -      if test "$vhost_net" = "yes" ; then
> +    echo "CONFIG_KVM=y" >> $config_target_mak
> +    if test "$vhost_net" = "yes" ; then
>          echo "CONFIG_VHOST_NET=y" >> $config_target_mak
> -      fi
>      fi
> -esac
> +fi
> +
>  case "$target_arch2" in
>    i386|x86_64)
>      echo "CONFIG_HAVE_GET_MEMORY_MAPPING=y" >> $config_target_mak
> --
> 1.7.12
>
>
>


-- PMM