Re: [PULL 06/15] tests/qtest/migration: Don't use -cpu max for aarch64

[Fabiano Rosas <farosas@suse.de>]

Fabiano Rosas <farosas@suse.de> writes:

> Peter Xu <peterx@redhat.com> writes:
>
>> On Fri, Jan 26, 2024 at 11:54:32AM -0300, Fabiano Rosas wrote:
>>> Peter Maydell <peter.maydell@linaro.org> writes:
>>> 
>>> > On Fri, 26 Jan 2024 at 14:36, Fabiano Rosas <farosas@suse.de> wrote:
>>> >>
>>> >> peterx@redhat.com writes:
>>> >>
>>> >> > From: Fabiano Rosas <farosas@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.maydell@linaro.org>
>>> >> > Signed-off-by: Fabiano Rosas <farosas@suse.de>
>>> >> > Link: https://lore.kernel.org/r/20240118164951.30350-2-farosas@suse.de
>>> >> > Signed-off-by: Peter Xu <peterx@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 <farosas@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 <farosas@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.