| Message ID | 20230113140419.4013-21-farosas@suse.de |
|---|---|
| State | New |
| Headers | show |
| Series | target/arm: Allow CONFIG_TCG=n builds | expand |
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 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 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~
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 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
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.
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");
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(-)