Re: [PATCH v3] system: Convert qemu_arch_available() to TargetInfo API

["Alex Bennée" <alex.bennee@linaro.org>]

Philippe Mathieu-Daudé <philmd@linaro.org> writes:

> Get the base arch_mask from the current SysEmuTarget,
> making qemu_arch_available() target-agnostic.
>
> We don't need the per-target QEMU_ARCH definition anymore,
> remove it.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
> v3: Return directly within switch
> v2: Prefer switch over array (pbo)
> ---
>  meson.build        |  2 --
>  system/arch_init.c | 30 -----------------------
>  target-info.c      | 60 ++++++++++++++++++++++++++++++++++++++++++++++
>  system/meson.build |  1 -
>  4 files changed, 60 insertions(+), 33 deletions(-)
>  delete mode 100644 system/arch_init.c
>
> diff --git a/meson.build b/meson.build
> index 734c801cc77..435dc6e3c8e 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -3419,8 +3419,6 @@ foreach target : target_dirs
>        config_target_data.set(k, v)
>      endif
>    endforeach
> -  config_target_data.set('QEMU_ARCH',
> -                         'QEMU_ARCH_' + config_target['TARGET_BASE_ARCH'].to_upper())
>    config_target_h += {target: configure_file(output: target + '-config-target.h',
>                                                 configuration: config_target_data)}
>  
> diff --git a/system/arch_init.c b/system/arch_init.c
> deleted file mode 100644
> index e85736884c9..00000000000
> --- a/system/arch_init.c
> +++ /dev/null
> @@ -1,30 +0,0 @@
> -/*
> - * QEMU System Emulator
> - *
> - * Copyright (c) 2003-2008 Fabrice Bellard
> - *
> - * Permission is hereby granted, free of charge, to any person obtaining a copy
> - * of this software and associated documentation files (the "Software"), to deal
> - * in the Software without restriction, including without limitation the rights
> - * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
> - * copies of the Software, and to permit persons to whom the Software is
> - * furnished to do so, subject to the following conditions:
> - *
> - * The above copyright notice and this permission notice shall be included in
> - * all copies or substantial portions of the Software.
> - *
> - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> - * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> - * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> - * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> - * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
> - * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
> - * THE SOFTWARE.
> - */
> -#include "qemu/osdep.h"
> -#include "system/arch_init.h"
> -
> -bool qemu_arch_available(unsigned qemu_arch_mask)
> -{
> -    return qemu_arch_mask & QEMU_ARCH;
> -}
> diff --git a/target-info.c b/target-info.c
> index 24696ff4111..774fdcd2c46 100644
> --- a/target-info.c
> +++ b/target-info.c
> @@ -11,6 +11,7 @@
>  #include "qemu/target-info-qapi.h"
>  #include "qemu/target-info-impl.h"
>  #include "qapi/error.h"
> +#include "system/arch_init.h"
>  
>  const char *target_name(void)
>  {
> @@ -33,6 +34,65 @@ SysEmuTarget target_arch(void)
>      return arch;
>  }
>

This looks fine to me but I note the API takes a mask which implies you
could potentially pass multiple arches in the mask. Not an issue now but
we should probably add a kdoc to the declaration to make it clear you
can only test one arch at a time for now.

> +bool qemu_arch_available(unsigned qemu_arch_mask)
> +{
> +    switch (target_arch()) {
> +    case SYS_EMU_TARGET_ALPHA:
> +        return qemu_arch_mask & QEMU_ARCH_ALPHA;
> +    case SYS_EMU_TARGET_ARM:
> +    case SYS_EMU_TARGET_AARCH64:
> +        return qemu_arch_mask & QEMU_ARCH_ARM;
> +    case SYS_EMU_TARGET_I386:
> +    case SYS_EMU_TARGET_X86_64:
> +        return qemu_arch_mask & QEMU_ARCH_I386;
> +    case SYS_EMU_TARGET_M68K:
> +        return qemu_arch_mask & QEMU_ARCH_M68K;
> +    case SYS_EMU_TARGET_MICROBLAZE:
> +    case SYS_EMU_TARGET_MICROBLAZEEL:
> +        return qemu_arch_mask & QEMU_ARCH_MICROBLAZE;
> +    case SYS_EMU_TARGET_MIPS:
> +    case SYS_EMU_TARGET_MIPSEL:
> +    case SYS_EMU_TARGET_MIPS64:
> +    case SYS_EMU_TARGET_MIPS64EL:
> +        return qemu_arch_mask & QEMU_ARCH_MIPS;
> +    case SYS_EMU_TARGET_PPC:
> +    case SYS_EMU_TARGET_PPC64:
> +        return qemu_arch_mask & QEMU_ARCH_PPC;
> +    case SYS_EMU_TARGET_S390X:
> +        return qemu_arch_mask & QEMU_ARCH_S390X;
> +    case SYS_EMU_TARGET_SH4:
> +    case SYS_EMU_TARGET_SH4EB:
> +        return qemu_arch_mask & QEMU_ARCH_SH4;
> +    case SYS_EMU_TARGET_SPARC:
> +    case SYS_EMU_TARGET_SPARC64:
> +        return qemu_arch_mask & QEMU_ARCH_SPARC;
> +    case SYS_EMU_TARGET_XTENSA:
> +    case SYS_EMU_TARGET_XTENSAEB:
> +        return qemu_arch_mask & QEMU_ARCH_XTENSA;
> +    case SYS_EMU_TARGET_OR1K:
> +        return qemu_arch_mask & QEMU_ARCH_OPENRISC;
> +    case SYS_EMU_TARGET_TRICORE:
> +        return qemu_arch_mask & QEMU_ARCH_TRICORE;
> +    case SYS_EMU_TARGET_HPPA:
> +        return qemu_arch_mask & QEMU_ARCH_HPPA;
> +    case SYS_EMU_TARGET_RISCV32:
> +    case SYS_EMU_TARGET_RISCV64:
> +        return qemu_arch_mask & QEMU_ARCH_RISCV;
> +    case SYS_EMU_TARGET_RX:
> +        return qemu_arch_mask & QEMU_ARCH_RX;
> +    case SYS_EMU_TARGET_AVR:
> +        return qemu_arch_mask & QEMU_ARCH_AVR;
> +    /*
> +    case SYS_EMU_TARGET_HEXAGON:
> +        return qemu_arch_mask & QEMU_ARCH_HEXAGON;
> +    */
> +    case SYS_EMU_TARGET_LOONGARCH64:
> +        return qemu_arch_mask & QEMU_ARCH_LOONGARCH;
> +    default:
> +        g_assert_not_reached();
> +    };
> +}
> +
>  const char *target_cpu_type(void)
>  {
>      return target_info()->cpu_type;
> diff --git a/system/meson.build b/system/meson.build
> index 4b69ef0f5fb..66e16db55ce 100644
> --- a/system/meson.build
> +++ b/system/meson.build
> @@ -1,5 +1,4 @@
>  specific_ss.add(when: 'CONFIG_SYSTEM_ONLY', if_true: [files(
> -  'arch_init.c',
>    'globals-target.c',
>  )])

Otherwise:

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>


-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro