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

[Peter Xu <peterx@redhat.com>]

On Tue, Jan 30, 2024 at 06:23:10PM -0300, Fabiano Rosas wrote:
> Peter Xu <peterx@redhat.com> writes:
> 
> > On Tue, Jan 30, 2024 at 10:18:07AM +0000, Peter Maydell wrote:
> >> On Mon, 29 Jan 2024 at 23:31, Fabiano Rosas <farosas@suse.de> wrote:
> >> >
> >> > 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:
> >> > > 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.
> >> 
> >> Yes, there is. We'll build with CONFIG_KVM on any aarch64 host,
> >> but that doesn't imply that the user running the build and
> >> test has permissions for /dev/kvm.
> >
> > I'm actually pretty confused on why this would be a problem even for
> > neoverse-n1: can we just try to use KVM, if it fails then use TCG?
> > Something like:
> >
> >   (construct qemu cmdline)
> >   ..
> > #ifdef CONFIG_KVM
> 
> >   "-accel kvm "
> > #endif
> >   "-accel tcg "
> >   ..
> >
> > ?
> > IIUC if we specify two "-accel", we'll try the first, then if failed then
> > the 2nd?
> 
> Aside from '-cpu max', there's no -accel and -cpu combination that works
> on all of:
> 
> x86_64 host - TCG-only
> aarch64 host - KVM & TCG
> aarch64 host with --disable-tcg - KVM-only
> aarch64 host without access to /dev/kvm - TCG-only
> 
> And the cpus are:
> host - KVM-only
> neoverse-n1 - TCG-only
> 
> We'll need something like:
> 
> /* covers aarch64 host with --disable-tcg */
> if (qtest_has_accel("kvm") && !qtest_has_accel("tcg")) {
>    if (open("/dev/kvm", O_RDONLY) < 0) {
>        g_test_skip()
>    } else {
>        "-accel kvm -cpu host"
>    }
> }
> 
> /* covers x86_64 host */
> if (!qtest_has_accel("kvm") && qtest_has_accel("tcg")) {
>    "-accel tcg -cpu neoverse-n1"
> }
> 
> /* covers aarch64 host */
> if (qtest_has_accel("kvm") && qtest_has_accel("tcg")) {
>    if (open("/dev/kvm", O_RDONLY) < 0) {
>       "-accel tcg -cpu neoverse-n1"
>    } else {
>       "-accel kvm -cpu host"
>    }
> }

The open("/dev/kvm") logic more or less duplicates what QEMU already does
when init accelerators:

    if (!qemu_opts_foreach(qemu_find_opts("accel"),
                           do_configure_accelerator, &init_failed, &error_fatal)) {
        if (!init_failed) {
            error_report("no accelerator found");
        }
        exit(1);
    }

If /dev/kvm not accessible I think it'll already fallback to tcg here, as
do_configure_accelerator() for kvm will just silently fail for qtest.  I
hope we can still rely on that for /dev/kvm access issues.

Hmm, I just notice that test_migrate_start() already has this later:

        "-accel kvm%s -accel tcg "

So we're actually good from that regard, AFAIU.

Then did I understand it right that in the failure case KVM is properly
initialized, however it crashed later in neoverse-n1 asking for TCG?  So
the logic in the accel code above didn't really work to do a real fallback?
A backtrace of such crash would help, maybe; I tried to find it in the
pipeline log but I can only see:

  ----------------------------------- stderr -----------------------------------
  Broken pipe
  ../tests/qtest/libqtest.c:195: kill_qemu() tried to terminate QEMU process but encountered exit status 1 (expected 0)

Or, is there some aarch64 cpu that will have a stable CPU ABI (not like
-max, which is unstable), meanwhile supports both TCG + KVM?

Another thing I noticed that we may need to be caution is that currently
gic is also using max version:

        machine_opts = "gic-version=max";

We may want to choose a sane version too, probably altogether with the
patch?

-- 
Peter Xu