Re: [PATCH v6 05/11] target/arm: Restrict ARMv6 cpus to TCG accel

* Re: [PATCH v6 05/11] target/arm: Restrict ARMv6 cpus to TCG accel
       [not found] ` <20210131115022.242570-6-f4bug@amsat.org>
@ 2021-01-31 14:29   ` Claudio Fontana
  2021-02-01 17:18   ` Alex Bennée
  1 sibling, 0 replies; 11+ messages in thread
From: Claudio Fontana @ 2021-01-31 14:29 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, qemu-devel
  Cc: Thomas Huth, Philippe Mathieu-Daudé,
	Richard Henderson, Fam Zheng, Paolo Bonzini, qemu-block,
	Alex Bennée, kvm, Laurent Vivier, qemu-arm,
	Richard Henderson, John Snow, Peter Maydell, Eduardo Habkost

On 1/31/21 12:50 PM, Philippe Mathieu-Daudé wrote:
> KVM requires the target cpu to be at least ARMv8 architecture
> (support on ARMv7 has been dropped in commit 82bf7ae84ce:
> "target/arm: Remove KVM support for 32-bit Arm hosts").
> 
> Only enable the following ARMv6 CPUs when TCG is available:
> 
>   - ARM1136
>   - ARM1176
>   - ARM11MPCore
>   - Cortex-M0
> 
> The following machines are no more built when TCG is disabled:
> 
>   - kzm                  ARM KZM Emulation Baseboard (ARM1136)
>   - microbit             BBC micro:bit (Cortex-M0)
>   - n800                 Nokia N800 tablet aka. RX-34 (OMAP2420)
>   - n810                 Nokia N810 tablet aka. RX-44 (OMAP2420)
>   - realview-eb-mpcore   ARM RealView Emulation Baseboard (ARM11MPCore)
> 
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  default-configs/devices/arm-softmmu.mak | 2 --
>  hw/arm/realview.c                       | 2 +-
>  tests/qtest/cdrom-test.c                | 2 +-
>  hw/arm/Kconfig                          | 6 ++++++
>  target/arm/Kconfig                      | 4 ++++
>  5 files changed, 12 insertions(+), 4 deletions(-)
> 
> diff --git a/default-configs/devices/arm-softmmu.mak b/default-configs/devices/arm-softmmu.mak
> index 0aad35da0c4..175530595ce 100644
> --- a/default-configs/devices/arm-softmmu.mak
> +++ b/default-configs/devices/arm-softmmu.mak
> @@ -10,9 +10,7 @@ CONFIG_ARM_VIRT=y
>  CONFIG_CUBIEBOARD=y
>  CONFIG_EXYNOS4=y
>  CONFIG_HIGHBANK=y
> -CONFIG_FSL_IMX31=y
>  CONFIG_MUSCA=y
> -CONFIG_NSERIES=y
>  CONFIG_STELLARIS=y
>  CONFIG_REALVIEW=y
>  CONFIG_VEXPRESS=y
> diff --git a/hw/arm/realview.c b/hw/arm/realview.c
> index 2dcf0a4c23e..0606d22da14 100644
> --- a/hw/arm/realview.c
> +++ b/hw/arm/realview.c
> @@ -463,8 +463,8 @@ static void realview_machine_init(void)
>  {
>      if (tcg_builtin()) {
>          type_register_static(&realview_eb_type);
> +        type_register_static(&realview_eb_mpcore_type);
>      }
> -    type_register_static(&realview_eb_mpcore_type);
>      type_register_static(&realview_pb_a8_type);
>      type_register_static(&realview_pbx_a9_type);
>  }
> diff --git a/tests/qtest/cdrom-test.c b/tests/qtest/cdrom-test.c
> index 1f1bc26fa7a..cb0409c5a11 100644
> --- a/tests/qtest/cdrom-test.c
> +++ b/tests/qtest/cdrom-test.c
> @@ -224,8 +224,8 @@ int main(int argc, char **argv)
>          const char *armmachines[] = {
>  #ifdef CONFIG_TCG
>              "realview-eb",
> -#endif /* CONFIG_TCG */
>              "realview-eb-mpcore",
> +#endif /* CONFIG_TCG */
>              "realview-pb-a8",
>              "realview-pbx-a9", "versatileab", "versatilepb", "vexpress-a15",
>              "vexpress-a9", "virt", NULL
> diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
> index 560442bfc5c..6c4bce4d637 100644
> --- a/hw/arm/Kconfig
> +++ b/hw/arm/Kconfig
> @@ -123,6 +123,8 @@ config NETDUINOPLUS2
>  
>  config NSERIES
>      bool
> +    default y if TCG && ARM
> +    select ARM_V6
>      select OMAP
>      select TMP105   # tempature sensor
>      select BLIZZARD # LCD/TV controller
> @@ -401,6 +403,8 @@ config FSL_IMX25
>  
>  config FSL_IMX31
>      bool
> +    default y if TCG && ARM
> +    select ARM_V6
>      select SERIAL
>      select IMX
>      select IMX_I2C
> @@ -478,11 +482,13 @@ config FSL_IMX6UL
>  
>  config MICROBIT
>      bool
> +    default y if TCG && ARM
>      select NRF51_SOC
>  
>  config NRF51_SOC
>      bool
>      select I2C
> +    select ARM_V6
>      select ARM_V7M
>      select UNIMP
>  
> diff --git a/target/arm/Kconfig b/target/arm/Kconfig
> index 9b3635617dc..fbb7bba9018 100644
> --- a/target/arm/Kconfig
> +++ b/target/arm/Kconfig
> @@ -14,6 +14,10 @@ config ARM_V5
>      bool
>      depends on TCG && ARM
>  
> +config ARM_V6
> +    bool
> +    depends on TCG && ARM
> +
>  config ARM_V7M
>      bool
>      select PTIMER
> 

Added Cc: Eduardo,

Looks good to me in general,

Acked-by: Claudio Fontana <cfontana@suse.de>

I am just wondering about that if (tcg_builtin()) / (or _available() as I suggest elsewhere), 
should we instead use the build system already at this stage, so no such check is necessary?

It could be a successive change, but then tcg_builtin() would be introduced, only to become useless after the proper refactoring is done,
and the build system is used to select the right modules to compile, which would do the registration.

Ciao,

Claudio

^ permalink raw reply	[flat|nested] 11+ messages in thread