| Message ID | 20230109224232.11661-14-farosas@suse.de |
|---|---|
| State | New |
| Headers | show |
| Series | target/arm: Allow CONFIG_TCG=n builds | expand |
On 09/01/2023 23.42, Fabiano Rosas wrote: > From: Claudio Fontana <cfontana@suse.de> > > on ARM we currently list and build all machines, even when > building KVM-only, without TCG. > > Until we fix this (and we only list and build machines that are > compatible with KVM), only test specifically using the "virt" > machine in this case. Why don't you fix it immediately? ... it shouldn't be too hard to add some "depends on TCG" statements to the machine entries in hw/arm/Kconfig, should it? Anyway, if that's not possible (yet), I suggest to add your hack to qtest_cb_for_every_machine() instead, so you could change this in one central place instead of adding a hack to each and every test that uses this function. Thomas
Thomas Huth <thuth@redhat.com> writes: > On 09/01/2023 23.42, Fabiano Rosas wrote: >> From: Claudio Fontana <cfontana@suse.de> >> >> on ARM we currently list and build all machines, even when >> building KVM-only, without TCG. >> >> Until we fix this (and we only list and build machines that are >> compatible with KVM), only test specifically using the "virt" >> machine in this case. > > Why don't you fix it immediately? ... My idea was to have in this series the minimum to unbreak the --disable-tcg build and later bring the rest of the changes incrementally. (plus the cpregs code movement which conflicts with everything, so I'd rather merge it sooner) > it shouldn't be too hard to add some > "depends on TCG" statements to the machine entries in hw/arm/Kconfig, should it? I havent't looked into it yet. If it turns out to be simple I can do it now. > Anyway, if that's not possible (yet), I suggest to add your hack to > qtest_cb_for_every_machine() instead, so you could change this in one > central place instead of adding a hack to each and every test that uses this > function. Good idea. > > Thomas
On Tue, 10 Jan 2023 at 13:00, Fabiano Rosas <farosas@suse.de> wrote: > > Thomas Huth <thuth@redhat.com> writes: > > > On 09/01/2023 23.42, Fabiano Rosas wrote: > >> From: Claudio Fontana <cfontana@suse.de> > >> > >> on ARM we currently list and build all machines, even when > >> building KVM-only, without TCG. > >> > >> Until we fix this (and we only list and build machines that are > >> compatible with KVM), only test specifically using the "virt" > >> machine in this case. > > > > Why don't you fix it immediately? ... > > My idea was to have in this series the minimum to unbreak the > --disable-tcg build and later bring the rest of the changes > incrementally. This test is basically checking "all the machines should work". That's an important invariant to maintain, so I don't think we should just skip it for Arm targets. -- PMM
Peter Maydell <peter.maydell@linaro.org> writes: > On Tue, 10 Jan 2023 at 13:00, Fabiano Rosas <farosas@suse.de> wrote: >> >> Thomas Huth <thuth@redhat.com> writes: >> >> > On 09/01/2023 23.42, Fabiano Rosas wrote: >> >> From: Claudio Fontana <cfontana@suse.de> >> >> >> >> on ARM we currently list and build all machines, even when >> >> building KVM-only, without TCG. >> >> >> >> Until we fix this (and we only list and build machines that are >> >> compatible with KVM), only test specifically using the "virt" >> >> machine in this case. >> > >> > Why don't you fix it immediately? ... >> >> My idea was to have in this series the minimum to unbreak the >> --disable-tcg build and later bring the rest of the changes >> incrementally. > > This test is basically checking "all the machines should > work". That's an important invariant to maintain, so > I don't think we should just skip it for Arm targets. But what does "all machines" mean in a no-TCG build? The original intent of the patch was that only 'virt' should be present and therefore the only one tested. > > -- PMM
On Tue, 10 Jan 2023 at 13:36, Fabiano Rosas <farosas@suse.de> wrote: > > Peter Maydell <peter.maydell@linaro.org> writes: > > > On Tue, 10 Jan 2023 at 13:00, Fabiano Rosas <farosas@suse.de> wrote: > >> > >> Thomas Huth <thuth@redhat.com> writes: > >> > >> > On 09/01/2023 23.42, Fabiano Rosas wrote: > >> >> From: Claudio Fontana <cfontana@suse.de> > >> >> > >> >> on ARM we currently list and build all machines, even when > >> >> building KVM-only, without TCG. > >> >> > >> >> Until we fix this (and we only list and build machines that are > >> >> compatible with KVM), only test specifically using the "virt" > >> >> machine in this case. > >> > > >> > Why don't you fix it immediately? ... > >> > >> My idea was to have in this series the minimum to unbreak the > >> --disable-tcg build and later bring the rest of the changes > >> incrementally. > > > > This test is basically checking "all the machines should > > work". That's an important invariant to maintain, so > > I don't think we should just skip it for Arm targets. > > But what does "all machines" mean in a no-TCG build? The original intent > of the patch was that only 'virt' should be present and therefore the > only one tested. It means "every machine the user can create". If the machine can't run then either we shouldn't compile it in, or else we should be compiling in enough extra stuff to allow it to work. -- PMM
On 1/10/23 15:02, Peter Maydell wrote: > On Tue, 10 Jan 2023 at 13:36, Fabiano Rosas <farosas@suse.de> wrote: >> >> Peter Maydell <peter.maydell@linaro.org> writes: >> >>> On Tue, 10 Jan 2023 at 13:00, Fabiano Rosas <farosas@suse.de> wrote: >>>> >>>> Thomas Huth <thuth@redhat.com> writes: >>>> >>>>> On 09/01/2023 23.42, Fabiano Rosas wrote: >>>>>> From: Claudio Fontana <cfontana@suse.de> >>>>>> >>>>>> on ARM we currently list and build all machines, even when >>>>>> building KVM-only, without TCG. >>>>>> >>>>>> Until we fix this (and we only list and build machines that are >>>>>> compatible with KVM), only test specifically using the "virt" >>>>>> machine in this case. >>>>> >>>>> Why don't you fix it immediately? ... >>>> >>>> My idea was to have in this series the minimum to unbreak the >>>> --disable-tcg build and later bring the rest of the changes >>>> incrementally. >>> >>> This test is basically checking "all the machines should >>> work". That's an important invariant to maintain, so >>> I don't think we should just skip it for Arm targets. >> >> But what does "all machines" mean in a no-TCG build? The original intent >> of the patch was that only 'virt' should be present and therefore the >> only one tested. > > It means "every machine the user can create". If the > machine can't run then either we shouldn't compile it > in, or else we should be compiling in enough extra stuff > to allow it to work. > > -- PMM Hi, once upon a time there was a series by Philippe to accomplish that (KConfig) right? Ciao, Claudio
On 11/01/2023 13.08, Claudio Fontana wrote: > On 1/10/23 15:02, Peter Maydell wrote: >> On Tue, 10 Jan 2023 at 13:36, Fabiano Rosas <farosas@suse.de> wrote: >>> >>> Peter Maydell <peter.maydell@linaro.org> writes: >>> >>>> On Tue, 10 Jan 2023 at 13:00, Fabiano Rosas <farosas@suse.de> wrote: >>>>> >>>>> Thomas Huth <thuth@redhat.com> writes: >>>>> >>>>>> On 09/01/2023 23.42, Fabiano Rosas wrote: >>>>>>> From: Claudio Fontana <cfontana@suse.de> >>>>>>> >>>>>>> on ARM we currently list and build all machines, even when >>>>>>> building KVM-only, without TCG. >>>>>>> >>>>>>> Until we fix this (and we only list and build machines that are >>>>>>> compatible with KVM), only test specifically using the "virt" >>>>>>> machine in this case. >>>>>> >>>>>> Why don't you fix it immediately? ... >>>>> >>>>> My idea was to have in this series the minimum to unbreak the >>>>> --disable-tcg build and later bring the rest of the changes >>>>> incrementally. >>>> >>>> This test is basically checking "all the machines should >>>> work". That's an important invariant to maintain, so >>>> I don't think we should just skip it for Arm targets. >>> >>> But what does "all machines" mean in a no-TCG build? The original intent >>> of the patch was that only 'virt' should be present and therefore the >>> only one tested. >> >> It means "every machine the user can create". If the >> machine can't run then either we shouldn't compile it >> in, or else we should be compiling in enough extra stuff >> to allow it to work. >> >> -- PMM > > Hi, > > once upon a time there was a series by Philippe to accomplish that (KConfig) right? Maybe that one: https://lore.kernel.org/qemu-devel/20190903114729.3400-1-philmd@redhat.com/ ? Thomas
Claudio Fontana <cfontana@suse.de> writes: > On 1/10/23 15:02, Peter Maydell wrote: >> On Tue, 10 Jan 2023 at 13:36, Fabiano Rosas <farosas@suse.de> wrote: >>> >>> Peter Maydell <peter.maydell@linaro.org> writes: >>> >>>> On Tue, 10 Jan 2023 at 13:00, Fabiano Rosas <farosas@suse.de> wrote: >>>>> >>>>> Thomas Huth <thuth@redhat.com> writes: >>>>> >>>>>> On 09/01/2023 23.42, Fabiano Rosas wrote: >>>>>>> From: Claudio Fontana <cfontana@suse.de> >>>>>>> >>>>>>> on ARM we currently list and build all machines, even when >>>>>>> building KVM-only, without TCG. >>>>>>> >>>>>>> Until we fix this (and we only list and build machines that are >>>>>>> compatible with KVM), only test specifically using the "virt" >>>>>>> machine in this case. >>>>>> >>>>>> Why don't you fix it immediately? ... >>>>> >>>>> My idea was to have in this series the minimum to unbreak the >>>>> --disable-tcg build and later bring the rest of the changes >>>>> incrementally. >>>> >>>> This test is basically checking "all the machines should >>>> work". That's an important invariant to maintain, so >>>> I don't think we should just skip it for Arm targets. >>> >>> But what does "all machines" mean in a no-TCG build? The original intent >>> of the patch was that only 'virt' should be present and therefore the >>> only one tested. >> >> It means "every machine the user can create". If the >> machine can't run then either we shouldn't compile it >> in, or else we should be compiling in enough extra stuff >> to allow it to work. >> >> -- PMM > > Hi, > > once upon a time there was a series by Philippe to accomplish that (KConfig) right? Is this it? https://www.mail-archive.com/qemu-devel@nongnu.org/msg777719.html I have now moved the cpus into the tcg directory: # ./qemu-system-aarch64 -cpu ? Available CPUs: a64fx cortex-a35 cortex-a53 cortex-a55 cortex-a57 cortex-a72 cortex-a76 host max neoverse-n1 What remains is changing the Kconfig to not bring all the machines. Nowadays for KVM is the 'virt' machine the only one we use? I did some tests yesterday by keeping only CONFIG_ARM_VIRT and was left with this: # ./qemu-system-aarch64 -machine ? Supported machines are: none empty machine virt-2.10 QEMU 2.10 ARM Virtual Machine virt-2.11 QEMU 2.11 ARM Virtual Machine virt-2.12 QEMU 2.12 ARM Virtual Machine virt-2.6 QEMU 2.6 ARM Virtual Machine virt-2.7 QEMU 2.7 ARM Virtual Machine virt-2.8 QEMU 2.8 ARM Virtual Machine virt-2.9 QEMU 2.9 ARM Virtual Machine virt-3.0 QEMU 3.0 ARM Virtual Machine virt-3.1 QEMU 3.1 ARM Virtual Machine virt-4.0 QEMU 4.0 ARM Virtual Machine virt-4.1 QEMU 4.1 ARM Virtual Machine virt-4.2 QEMU 4.2 ARM Virtual Machine virt-5.0 QEMU 5.0 ARM Virtual Machine virt-5.1 QEMU 5.1 ARM Virtual Machine virt-5.2 QEMU 5.2 ARM Virtual Machine virt-6.0 QEMU 6.0 ARM Virtual Machine virt-6.1 QEMU 6.1 ARM Virtual Machine virt-6.2 QEMU 6.2 ARM Virtual Machine virt-7.0 QEMU 7.0 ARM Virtual Machine virt-7.1 QEMU 7.1 ARM Virtual Machine virt-7.2 QEMU 7.2 ARM Virtual Machine virt QEMU 8.0 ARM Virtual Machine (alias of virt-8.0) virt-8.0 QEMU 8.0 ARM Virtual Machine x-remote Experimental remote machine But qtest dislikes that even more. I need to figure out why. The approach of that series seems interesting, moving CONFIG_FOO=y from default.mak into Kconfig as 'default y if TCG'. Maybe that will yield better results, I'll try to follow that route.
On 1/11/23 04:36, Fabiano Rosas wrote:
> Nowadays for KVM is the 'virt' machine the only one we use?
Also sbsa-ref.
r~
Richard Henderson <richard.henderson@linaro.org> writes: > On 1/11/23 04:36, Fabiano Rosas wrote: >> Nowadays for KVM is the 'virt' machine the only one we use? > > Also sbsa-ref. It seems not, sbsa_ref_init has this: if (kvm_enabled()) { error_report("sbsa-ref: KVM is not supported for this machine"); exit(1); }
On Wed, 11 Jan 2023 at 12:36, Fabiano Rosas <farosas@suse.de> wrote: > What remains is changing the Kconfig to not bring all the > machines. Nowadays for KVM is the 'virt' machine the only one we use? I think perhaps also xlnx-versal-virt -- at any rate, the source files include some KVM related headers... Alistair, Edgar -- does xlnx-versal-virt work with KVM ? thanks -- PMM
diff --git a/tests/qtest/test-hmp.c b/tests/qtest/test-hmp.c index f8b22abe4c..daa1e76a06 100644 --- a/tests/qtest/test-hmp.c +++ b/tests/qtest/test-hmp.c @@ -157,8 +157,29 @@ int main(int argc, char **argv) g_test_init(&argc, &argv, NULL); + /* + * XXX currently we build also boards for ARM that are + * incompatible with KVM. We therefore need to check this + * explicitly, and only test virt for kvm-only arm builds. After + * we do the work of Kconfig etc to ensure that only + * KVM-compatible boards are built for the kvm-only build, we + * could remove this. + */ +#ifndef CONFIG_TCG + { + const char *arch = qtest_get_arch(); + + if (strcmp(arch, "arm") == 0 || strcmp(arch, "aarch64") == 0) { + add_machine_test_case("virt"); + goto add_machine_test_done; + } + } +#endif /* !CONFIG_TCG */ + qtest_cb_for_every_machine(add_machine_test_case, g_test_quick()); + goto add_machine_test_done; + add_machine_test_done: /* as none machine has no memory by default, add a test case with memory */ qtest_add_data_func("hmp/none+2MB", g_strdup("none -m 2"), test_machine);