The cortex-a15 is not present anymore when CONFIG_TCG=n, so use the
cortex-a57 as default cpu for KVM.

Signed-off-by: Fabiano Rosas <farosas@suse.de>
---
 hw/arm/virt.c                  | 6 ++++++
 tests/qtest/arm-cpu-features.c | 3 +--
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index ea2413a0ba..19854f4137 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -203,8 +203,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
     ARM_CPU_TYPE_NAME("cortex-a35"),
     ARM_CPU_TYPE_NAME("cortex-a53"),
     ARM_CPU_TYPE_NAME("cortex-a55"),
@@ -3003,7 +3005,11 @@ static void virt_machine_class_init(ObjectClass *oc, void *data)
     mc->minimum_page_bits = 12;
     mc->possible_cpu_arch_ids = virt_possible_cpu_arch_ids;
     mc->cpu_index_to_instance_props = virt_cpu_index_to_props;
+#ifdef CONFIG_TCG
     mc->default_cpu_type = ARM_CPU_TYPE_NAME("cortex-a15");
+#else
+    mc->default_cpu_type = ARM_CPU_TYPE_NAME("cortex-a57");
+#endif
     mc->get_default_cpu_node_id = virt_get_default_cpu_node_id;
     mc->kvm_type = virt_kvm_type;
     assert(!mc->get_hotplug_handler);
diff --git a/tests/qtest/arm-cpu-features.c b/tests/qtest/arm-cpu-features.c
index 8691802950..4be1415823 100644
--- a/tests/qtest/arm-cpu-features.c
+++ b/tests/qtest/arm-cpu-features.c
@@ -507,8 +507,7 @@ static void test_query_cpu_model_expansion_kvm(const void *data)
         char *error;
 
         assert_error(qts, "cortex-a15",
-            "We cannot guarantee the CPU type 'cortex-a15' works "
-            "with KVM on this host", NULL);
+            "The CPU type 'cortex-a15' is not a recognized ARM CPU type", NULL);
 
         assert_has_feature_enabled(qts, "host", "aarch64");
 
-- 
2.35.3

On 1/13/23 06:04, Fabiano Rosas wrote:
> The cortex-a15 is not present anymore when CONFIG_TCG=n, so use the
> cortex-a57 as default cpu for KVM.
> 
> Signed-off-by: Fabiano Rosas <farosas@suse.de>

Ideally there would not be a default at all, requiring the command-line option to be used.

Second choice would be "host", since that's the only value that's actually usable (except 
for the off-chance that you're actually running on an a57, which is less and less likely 
as time moves on).


r~

On Fri, 13 Jan 2023 at 22:05, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> On 1/13/23 06:04, Fabiano Rosas wrote:
> > The cortex-a15 is not present anymore when CONFIG_TCG=n, so use the
> > cortex-a57 as default cpu for KVM.
> >
> > Signed-off-by: Fabiano Rosas <farosas@suse.de>
>
> Ideally there would not be a default at all, requiring the command-line option to be used.

Effectively, you pretty much do always need to use the command line
option, because almost 0% of people wanted the cortex-a15 at this point,
even when using TCG...

-- PMM

Richard Henderson <richard.henderson@linaro.org> writes:

> On 1/13/23 06:04, Fabiano Rosas wrote:
>> The cortex-a15 is not present anymore when CONFIG_TCG=n, so use the
>> cortex-a57 as default cpu for KVM.
>> 
>> Signed-off-by: Fabiano Rosas <farosas@suse.de>
>
> Ideally there would not be a default at all, requiring the command-line option to be used.

We could probably do that now without impact to users, since KVM would
always require a -cpu option due to the current default being
cortex-a15.

>
> Second choice would be "host", since that's the only value that's actually usable (except 
> for the off-chance that you're actually running on an a57, which is less and less likely 
> as time moves on).
>

I'll have to go around fixing qtest first, either to add -cpu or to add
-accel kvm, otherwise we get:

The 'host' CPU type can only be used with KVM or HVF

On Mon, 16 Jan 2023 at 13:45, Fabiano Rosas <farosas@suse.de> wrote:
>
> Richard Henderson <richard.henderson@linaro.org> writes:
>
> > On 1/13/23 06:04, Fabiano Rosas wrote:
> >> The cortex-a15 is not present anymore when CONFIG_TCG=n, so use the
> >> cortex-a57 as default cpu for KVM.
> >>
> >> Signed-off-by: Fabiano Rosas <farosas@suse.de>
> >
> > Ideally there would not be a default at all, requiring the command-line option to be used.
>
> We could probably do that now without impact to users, since KVM would
> always require a -cpu option due to the current default being
> cortex-a15.
>
> >
> > Second choice would be "host", since that's the only value that's actually usable (except
> > for the off-chance that you're actually running on an a57, which is less and less likely
> > as time moves on).
> >
>
> I'll have to go around fixing qtest first, either to add -cpu or to add
> -accel kvm, otherwise we get:
>
> The 'host' CPU type can only be used with KVM or HVF

For a CPU type that will work with either KVM or TCG, that would
be "max".

thanks
-- PMM

Peter Maydell <peter.maydell@linaro.org> writes:

> On Mon, 16 Jan 2023 at 13:45, Fabiano Rosas <farosas@suse.de> wrote:
>>
>> Richard Henderson <richard.henderson@linaro.org> writes:
>>
>> > On 1/13/23 06:04, Fabiano Rosas wrote:
>> >> The cortex-a15 is not present anymore when CONFIG_TCG=n, so use the
>> >> cortex-a57 as default cpu for KVM.
>> >>
>> >> Signed-off-by: Fabiano Rosas <farosas@suse.de>
>> >
>> > Ideally there would not be a default at all, requiring the command-line option to be used.
>>
>> We could probably do that now without impact to users, since KVM would
>> always require a -cpu option due to the current default being
>> cortex-a15.
>>
>> >
>> > Second choice would be "host", since that's the only value that's actually usable (except
>> > for the off-chance that you're actually running on an a57, which is less and less likely
>> > as time moves on).
>> >
>>
>> I'll have to go around fixing qtest first, either to add -cpu or to add
>> -accel kvm, otherwise we get:
>>
>> The 'host' CPU type can only be used with KVM or HVF
>
> For a CPU type that will work with either KVM or TCG, that would
> be "max".

Yes, although the issue here is more that there are tests running with
!kvm_enabled (no -accel kvm given) and !tcg_enabled (--disable-tcg).

The "max" cpu does in fact work with qtest because even when
CONFIG_TCG=n, it ends up configuring a "cortex-a57 + extra things" in
aarch64_max_initfn. But that seems a bit too implicit to me, it would be
better for the tests to explicitly set the accel and cpu options.

On 1/13/23 15:04, Fabiano Rosas wrote:
> The cortex-a15 is not present anymore when CONFIG_TCG=n, so use the
> cortex-a57 as default cpu for KVM.
> 
> Signed-off-by: Fabiano Rosas <farosas@suse.de>

Our recommendation currently for KVM on ARM is to always use CPU="host", as named cpu models generally don't work (yet?) with KVM.

https://qemu-project.gitlab.io/qemu/system/arm/cpu-features.html

Should then "host" be the default for KVM if CONFIG_TCG=N or CONFIG_TCG=M and the TCG .so is not loaded?

Thanks,

Claudio

> ---
>  hw/arm/virt.c                  | 6 ++++++
>  tests/qtest/arm-cpu-features.c | 3 +--
>  2 files changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index ea2413a0ba..19854f4137 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -203,8 +203,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
>      ARM_CPU_TYPE_NAME("cortex-a35"),
>      ARM_CPU_TYPE_NAME("cortex-a53"),
>      ARM_CPU_TYPE_NAME("cortex-a55"),
> @@ -3003,7 +3005,11 @@ static void virt_machine_class_init(ObjectClass *oc, void *data)
>      mc->minimum_page_bits = 12;
>      mc->possible_cpu_arch_ids = virt_possible_cpu_arch_ids;
>      mc->cpu_index_to_instance_props = virt_cpu_index_to_props;
> +#ifdef CONFIG_TCG
>      mc->default_cpu_type = ARM_CPU_TYPE_NAME("cortex-a15");
> +#else
> +    mc->default_cpu_type = ARM_CPU_TYPE_NAME("cortex-a57");
> +#endif
>      mc->get_default_cpu_node_id = virt_get_default_cpu_node_id;
>      mc->kvm_type = virt_kvm_type;
>      assert(!mc->get_hotplug_handler);
> diff --git a/tests/qtest/arm-cpu-features.c b/tests/qtest/arm-cpu-features.c
> index 8691802950..4be1415823 100644
> --- a/tests/qtest/arm-cpu-features.c
> +++ b/tests/qtest/arm-cpu-features.c
> @@ -507,8 +507,7 @@ static void test_query_cpu_model_expansion_kvm(const void *data)
>          char *error;
>  
>          assert_error(qts, "cortex-a15",
> -            "We cannot guarantee the CPU type 'cortex-a15' works "
> -            "with KVM on this host", NULL);
> +            "The CPU type 'cortex-a15' is not a recognized ARM CPU type", NULL);
>  
>          assert_has_feature_enabled(qts, "host", "aarch64");
>

Claudio Fontana <cfontana@suse.de> writes:

> On 1/13/23 15:04, Fabiano Rosas wrote:
>> The cortex-a15 is not present anymore when CONFIG_TCG=n, so use the
>> cortex-a57 as default cpu for KVM.
>> 
>> Signed-off-by: Fabiano Rosas <farosas@suse.de>
>
> Our recommendation currently for KVM on ARM is to always use CPU="host", as named cpu models generally don't work (yet?) with KVM.
>
> https://qemu-project.gitlab.io/qemu/system/arm/cpu-features.html
>
> Should then "host" be the default for KVM if CONFIG_TCG=N or CONFIG_TCG=M and the TCG .so is not loaded?

Yes, "host" seems to make more sense. I was under the impression that
there was a way to use cortex-a57 and cortex-a53 since they have
cpu->kvm_target set.

On 1/13/23 08:22, Fabiano Rosas wrote:
> Claudio Fontana <cfontana@suse.de> writes:
> 
>> On 1/13/23 15:04, Fabiano Rosas wrote:
>>> The cortex-a15 is not present anymore when CONFIG_TCG=n, so use the
>>> cortex-a57 as default cpu for KVM.
>>>
>>> Signed-off-by: Fabiano Rosas <farosas@suse.de>
>>
>> Our recommendation currently for KVM on ARM is to always use CPU="host", as named cpu models generally don't work (yet?) with KVM.
>>
>> https://qemu-project.gitlab.io/qemu/system/arm/cpu-features.html
>>
>> Should then "host" be the default for KVM if CONFIG_TCG=N or CONFIG_TCG=M and the TCG .so is not loaded?
> 
> Yes, "host" seems to make more sense. I was under the impression that
> there was a way to use cortex-a57 and cortex-a53 since they have
> cpu->kvm_target set.

One may use a -cpu other than 'host' if and only if it exactly matches the host.

r~