Fabiano Rosas <faro...@suse.de> writes: > Peter Xu <pet...@redhat.com> writes: > >> On Fri, Jan 26, 2024 at 11:54:32AM -0300, Fabiano Rosas wrote: >>> Peter Maydell <peter.mayd...@linaro.org> writes: >>> >>> > On Fri, 26 Jan 2024 at 14:36, Fabiano Rosas <faro...@suse.de> wrote: >>> >> >>> >> pet...@redhat.com writes: >>> >> >>> >> > From: Fabiano Rosas <faro...@suse.de> >>> >> > >>> >> > The 'max' cpu is not expected to be stable in terms of features across >>> >> > QEMU versions, so it should not be expected to migrate. >>> >> > >>> >> > While the tests currently all pass with -cpu max, that is only because >>> >> > we're not testing across QEMU versions, which is the more common >>> >> > use-case for migration. >>> >> > >>> >> > We've recently introduced compatibility tests that use two different >>> >> > QEMU versions and the tests are now failing for aarch64. The next >>> >> > patch adds those tests to CI, so we cannot use the 'max' cpu >>> >> > anymore. Replace it with the 'neoverse-n1', which has a fixed set of >>> >> > features. >>> >> > >>> >> > Suggested-by: Peter Maydell <peter.mayd...@linaro.org> >>> >> > Signed-off-by: Fabiano Rosas <faro...@suse.de> >>> >> > Link: https://lore.kernel.org/r/20240118164951.30350-2-faro...@suse.de >>> >> > Signed-off-by: Peter Xu <pet...@redhat.com> >>> >> > --- >>> >> > tests/qtest/migration-test.c | 2 +- >>> >> > 1 file changed, 1 insertion(+), 1 deletion(-) >>> >> > >>> >> > diff --git a/tests/qtest/migration-test.c >>> >> > b/tests/qtest/migration-test.c >>> >> > index 7675519cfa..15713f3666 100644 >>> >> > --- a/tests/qtest/migration-test.c >>> >> > +++ b/tests/qtest/migration-test.c >>> >> > @@ -820,7 +820,7 @@ static int test_migrate_start(QTestState **from, >>> >> > QTestState **to, >>> >> > memory_size = "150M"; >>> >> > machine_alias = "virt"; >>> >> > machine_opts = "gic-version=max"; >>> >> > - arch_opts = g_strdup_printf("-cpu max -kernel %s", bootpath); >>> >> > + arch_opts = g_strdup_printf("-cpu neoverse-n1 -kernel %s", >>> >> > bootpath); >>> >> > start_address = ARM_TEST_MEM_START; >>> >> > end_address = ARM_TEST_MEM_END; >>> >> > } else { >>> >> >>> >> This breaks the tests on an arm host with KVM support. We could drop >>> >> this patch from the PR, it doesn't affect anything else. >>> >> >>> >> Or squash in: >>> >> >>> >> -->8-- >>> >> From b8aa5d8a2b33dcc28e4cd4ce2c4f4eacc3a3b845 Mon Sep 17 00:00:00 2001 >>> >> From: Fabiano Rosas <faro...@suse.de> >>> >> Date: Fri, 26 Jan 2024 11:33:15 -0300 >>> >> Subject: [PATCH] fixup! tests/qtest/migration: Don't use -cpu max for >>> >> aarch64 >>> >> >>> >> Signed-off-by: Fabiano Rosas <faro...@suse.de> >>> >> --- >>> >> tests/qtest/migration-test.c | 4 +++- >>> >> 1 file changed, 3 insertions(+), 1 deletion(-) >>> >> >>> >> diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c >>> >> index 15713f3666..2ba9cab684 100644 >>> >> --- a/tests/qtest/migration-test.c >>> >> +++ b/tests/qtest/migration-test.c >>> >> @@ -820,7 +820,9 @@ static int test_migrate_start(QTestState **from, >>> >> QTestState **to, >>> >> memory_size = "150M"; >>> >> machine_alias = "virt"; >>> >> machine_opts = "gic-version=max"; >>> >> - arch_opts = g_strdup_printf("-cpu neoverse-n1 -kernel %s", >>> >> bootpath); >>> >> + arch_opts = g_strdup_printf("-cpu %s -kernel %s", >>> >> + qtest_has_accel("kvm") ? >>> >> + "host" : "neoverse-n1", bootpath); >>> >> start_address = ARM_TEST_MEM_START; >>> >> end_address = ARM_TEST_MEM_END; >>> >> } else { >>> > >>> > If you want to do that then a comment explaining why would be >>> > helpful for future readers, I think. >>> >>> Ok, let's drop this one then, I'll resend. >> >> I'll drop this one for now then, thanks. >> >> Just to double check: Fabiano, you meant that "-cpu host" won't hit the >> same issue as what "-cpu max" would have for the new "n-1" CI test, right? > > Well, no. What we need here is a cpu that works with KVM. Currently > that's 'host'. If that breaks the n-1 test, then it's a regression. > > We also need a cpu that works with TCG. Any of them would do. Except max > which changes in incompatible ways (that was the original patch's > purpose). > > The issue that occurs to me now is that 'cpu host' will not work with > TCG. We might actually need to go poking /dev/kvm for this to work.
Nevermind this last part. There's not going to be a scenario where we build with CONFIG_KVM, but run in an environment that does not support KVM.
