hw/arm/virt: Improve CPU help and fix testing under KVM

[PATCH 8/9] hw/arm/virt: Restrict 32-bit CPUs to TCG

Posted by Philippe Mathieu-Daudé 5 years ago

Support for ARMv7 has been dropped in commit 82bf7ae84ce
("target/arm: Remove KVM support for 32-bit Arm hosts").
Restrict the 32-bit CPUs to --enable-tcg builds.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/arm/virt.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index f5e4a6ec914..ab6300650f9 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -197,8 +197,10 @@ static const int a15irqmap[] = {
 };
 
 static const char *valid_cpus[] = {
+#ifdef CONFIG_TCG
     ARM_CPU_TYPE_NAME("cortex-a7"),
     ARM_CPU_TYPE_NAME("cortex-a15"),
+#endif /* CONFIG_TCG */
 #ifdef TARGET_AARCH64
     ARM_CPU_TYPE_NAME("cortex-a53"),
     ARM_CPU_TYPE_NAME("cortex-a57"),
-- 
2.26.2

Re: [PATCH 8/9] hw/arm/virt: Restrict 32-bit CPUs to TCG

Posted by Andrew Jones 5 years ago

On Fri, Feb 05, 2021 at 03:43:44PM +0100, Philippe Mathieu-Daudé wrote:
> Support for ARMv7 has been dropped in commit 82bf7ae84ce
> ("target/arm: Remove KVM support for 32-bit Arm hosts").
> Restrict the 32-bit CPUs to --enable-tcg builds.
> 
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  hw/arm/virt.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index f5e4a6ec914..ab6300650f9 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -197,8 +197,10 @@ static const int a15irqmap[] = {
>  };
>  
>  static const char *valid_cpus[] = {
> +#ifdef CONFIG_TCG
>      ARM_CPU_TYPE_NAME("cortex-a7"),
>      ARM_CPU_TYPE_NAME("cortex-a15"),
> +#endif /* CONFIG_TCG */
>  #ifdef TARGET_AARCH64
>      ARM_CPU_TYPE_NAME("cortex-a53"),
>      ARM_CPU_TYPE_NAME("cortex-a57"),
> -- 
> 2.26.2
>

So this filters the cpus out of KVM only builds, which seems
reasonable to do. Of course, if the build is for both KVM and
TCG, then the cpus won't be filtered out and we'll have to rely
on the runtime checks to error out if one where to try a 32-bit
cpu with KVM. But that's fine too, so

Reviewed-by: Andrew Jones <drjones@redhat.com>

Thanks,
drew

Re: [PATCH 8/9] hw/arm/virt: Restrict 32-bit CPUs to TCG

Posted by Claudio Fontana 4 years, 11 months ago

Hi Peter,

what do you think of the following patch? We messaged yesterday about cortex-a15 being the default cpu for virt,

this patch would need also changing the default CPU for virt under KVM I would think.

Or, we could change the virt default cpu to "max"?

Thanks,

Claudio


On 2/5/21 4:19 PM, Andrew Jones wrote:
> On Fri, Feb 05, 2021 at 03:43:44PM +0100, Philippe Mathieu-Daudé wrote:
>> Support for ARMv7 has been dropped in commit 82bf7ae84ce
>> ("target/arm: Remove KVM support for 32-bit Arm hosts").
>> Restrict the 32-bit CPUs to --enable-tcg builds.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> ---
>>  hw/arm/virt.c | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
>> index f5e4a6ec914..ab6300650f9 100644
>> --- a/hw/arm/virt.c
>> +++ b/hw/arm/virt.c
>> @@ -197,8 +197,10 @@ static const int a15irqmap[] = {
>>  };
>>  
>>  static const char *valid_cpus[] = {
>> +#ifdef CONFIG_TCG
>>      ARM_CPU_TYPE_NAME("cortex-a7"),
>>      ARM_CPU_TYPE_NAME("cortex-a15"),
>> +#endif /* CONFIG_TCG */
>>  #ifdef TARGET_AARCH64
>>      ARM_CPU_TYPE_NAME("cortex-a53"),
>>      ARM_CPU_TYPE_NAME("cortex-a57"),
>> -- 
>> 2.26.2
>>
> 
> So this filters the cpus out of KVM only builds, which seems
> reasonable to do. Of course, if the build is for both KVM and
> TCG, then the cpus won't be filtered out and we'll have to rely
> on the runtime checks to error out if one where to try a 32-bit
> cpu with KVM. But that's fine too, so
> 
> Reviewed-by: Andrew Jones <drjones@redhat.com>
> 
> Thanks,
> drew
> 
>

Re: [PATCH 8/9] hw/arm/virt: Restrict 32-bit CPUs to TCG

Posted by Peter Maydell 4 years, 11 months ago

On Fri, 5 Feb 2021 at 14:44, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
> Support for ARMv7 has been dropped in commit 82bf7ae84ce
> ("target/arm: Remove KVM support for 32-bit Arm hosts").
> Restrict the 32-bit CPUs to --enable-tcg builds.
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  hw/arm/virt.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index f5e4a6ec914..ab6300650f9 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -197,8 +197,10 @@ static const int a15irqmap[] = {
>  };
>
>  static const char *valid_cpus[] = {
> +#ifdef CONFIG_TCG
>      ARM_CPU_TYPE_NAME("cortex-a7"),
>      ARM_CPU_TYPE_NAME("cortex-a15"),
> +#endif /* CONFIG_TCG */
>  #ifdef TARGET_AARCH64
>      ARM_CPU_TYPE_NAME("cortex-a53"),
>      ARM_CPU_TYPE_NAME("cortex-a57"),

How painful would it be to just have it check whether the
CPU type is present in the executable, rather than hard-coding an ifdef ?

I think that if you try to run the virt board with command line
arguments that (implicitly or explicitly) mean you've asked for
a CPU which isn't present in the QEMU executable, it should give
an error rather than silently selecting something else.

thanks
-- PMM