[PATCH 0/7] tests/qtest/migration: Update framework to allow using HVF accelerator

[PATCH 0/7] tests/qtest/migration: Update framework to allow using HVF accelerator

[Philippe Mathieu-Daudé]

Hi,

This series modify few bits of the migration QTest framework
to allow running the tests using the HVF framework (also
leaving the possibilty for other ones, removing the KVM/TCG
restriction).

Philippe Mathieu-Daudé (7):
  migration/dirtyrate: Do not unlock cpu_list lock twice
  tests/qtest/migration: Make 'has_dirty_ring' generic
  tests/qtest/migration: Initialize MigrationTestEnv::arch early
  tests/qtest/migration: Pass accelerator arguments as machine option
  tests/qtest/migration: Add MigrationTestEnv::has_hvf field
  tests/qtest/migration: Run aarch64/HVF tests using GICv2
  tests/qtest/migration: Allow using accelerators different of TCG / KVM

 tests/qtest/migration/framework.h     |  1 +
 migration/dirtyrate.c                 |  1 -
 tests/qtest/migration/framework.c     | 43 ++++++++++++++-------------
 tests/qtest/migration/precopy-tests.c |  6 ++--
 4 files changed, 26 insertions(+), 25 deletions(-)

-- 
2.47.1

[PATCH 1/7] migration/dirtyrate: Do not unlock cpu_list lock twice

[Philippe Mathieu-Daudé]

&qemu_cpu_list_lock is locked within the WITH_QEMU_LOCK_GUARD()
context, then unlocked. No need to manually unlock it.

Fixes: 370ed600296 ("cpu: expose qemu_cpu_list_lock for lock-guard use")
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 migration/dirtyrate.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/migration/dirtyrate.c b/migration/dirtyrate.c
index 7c955894e47..4b94dd7c500 100644
--- a/migration/dirtyrate.c
+++ b/migration/dirtyrate.c
@@ -174,7 +174,6 @@ retry:
         if (gen_id != cpu_list_generation_id_get()) {
             g_free(records);
             g_free(stat->rates);
-            cpu_list_unlock();
             goto retry;
         }
         vcpu_dirty_stat_collect(records, false);
-- 
2.47.1

Re: [PATCH 1/7] migration/dirtyrate: Do not unlock cpu_list lock twice

[Fabiano Rosas]

Philippe Mathieu-Daudé <philmd@linaro.org> writes:

> &qemu_cpu_list_lock is locked within the WITH_QEMU_LOCK_GUARD()
> context, then unlocked. No need to manually unlock it.
>
> Fixes: 370ed600296 ("cpu: expose qemu_cpu_list_lock for lock-guard use")
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  migration/dirtyrate.c | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/migration/dirtyrate.c b/migration/dirtyrate.c
> index 7c955894e47..4b94dd7c500 100644
> --- a/migration/dirtyrate.c
> +++ b/migration/dirtyrate.c
> @@ -174,7 +174,6 @@ retry:
>          if (gen_id != cpu_list_generation_id_get()) {
>              g_free(records);
>              g_free(stat->rates);
> -            cpu_list_unlock();
>              goto retry;
>          }
>          vcpu_dirty_stat_collect(records, false);

Reviewed-by: Fabiano Rosas <farosas@suse.de>

Re: [PATCH 1/7] migration/dirtyrate: Do not unlock cpu_list lock twice

[Richard Henderson]

On 1/28/25 05:54, Philippe Mathieu-Daudé wrote:
> &qemu_cpu_list_lock is locked within the WITH_QEMU_LOCK_GUARD()
> context, then unlocked. No need to manually unlock it.
> 
> Fixes: 370ed600296 ("cpu: expose qemu_cpu_list_lock for lock-guard use")
> Signed-off-by: Philippe Mathieu-Daudé<philmd@linaro.org>
> ---
>   migration/dirtyrate.c | 1 -
>   1 file changed, 1 deletion(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

[PATCH 2/7] tests/qtest/migration: Make 'has_dirty_ring' generic

[Philippe Mathieu-Daudé]

Keep accelerator knowledge limited within MigrationTestEnv,
expose a generic %has_dirty_ring value, only checking for
KVM when initializing it in migration_get_env().

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 tests/qtest/migration/framework.c     | 2 +-
 tests/qtest/migration/precopy-tests.c | 6 ++----
 2 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/tests/qtest/migration/framework.c b/tests/qtest/migration/framework.c
index 4550cda129c..a3bd92a9519 100644
--- a/tests/qtest/migration/framework.c
+++ b/tests/qtest/migration/framework.c
@@ -946,7 +946,7 @@ MigrationTestEnv *migration_get_env(void)
         return env;
     }
 
-    env->has_dirty_ring = kvm_dirty_ring_supported();
+    env->has_dirty_ring = env->has_kvm && kvm_dirty_ring_supported();
     env->has_uffd = ufd_version_check(&env->uffd_feature_thread_id);
     env->arch = qtest_get_arch();
     env->is_x86 = !strcmp(env->arch, "i386") || !strcmp(env->arch, "x86_64");
diff --git a/tests/qtest/migration/precopy-tests.c b/tests/qtest/migration/precopy-tests.c
index 23599b29ee2..8dcd0ad2660 100644
--- a/tests/qtest/migration/precopy-tests.c
+++ b/tests/qtest/migration/precopy-tests.c
@@ -983,8 +983,7 @@ void migration_test_add_precopy(MigrationTestEnv *env)
     if (g_test_slow()) {
         migration_test_add("/migration/auto_converge",
                            test_auto_converge);
-        if (g_str_equal(env->arch, "x86_64") &&
-            env->has_kvm && env->has_dirty_ring) {
+        if (g_str_equal(env->arch, "x86_64") && env->has_dirty_ring) {
             migration_test_add("/dirty_limit",
                                test_dirty_limit);
         }
@@ -999,8 +998,7 @@ void migration_test_add_precopy(MigrationTestEnv *env)
                        test_multifd_tcp_no_zero_page);
     migration_test_add("/migration/multifd/tcp/plain/cancel",
                        test_multifd_tcp_cancel);
-    if (g_str_equal(env->arch, "x86_64")
-        && env->has_kvm && env->has_dirty_ring) {
+    if (g_str_equal(env->arch, "x86_64") && env->has_dirty_ring) {
 
         migration_test_add("/migration/dirty_ring",
                            test_precopy_unix_dirty_ring);
-- 
2.47.1

Re: [PATCH 2/7] tests/qtest/migration: Make 'has_dirty_ring' generic

[Fabiano Rosas]

Philippe Mathieu-Daudé <philmd@linaro.org> writes:

> Keep accelerator knowledge limited within MigrationTestEnv,
> expose a generic %has_dirty_ring value, only checking for
> KVM when initializing it in migration_get_env().
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Reviewed-by: Fabiano Rosas <farosas@suse.de>

[PATCH 3/7] tests/qtest/migration: Initialize MigrationTestEnv::arch early

[Philippe Mathieu-Daudé]

Some tests expect MigrationTestEnv::arch to be set. Initialize
it early enough to avoid SIGSEGV, for example like the following
g_str_equal() call in migration/precopy-tests.c:

   954 void migration_test_add_precopy(MigrationTestEnv *env)
   955 {
   ...
  1001     if (g_str_equal(env->arch, "x86_64") && env->has_dirty_ring) {
  1002
  1003         migration_test_add("/migration/dirty_ring",
  1004                            test_precopy_unix_dirty_ring);

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 tests/qtest/migration/framework.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tests/qtest/migration/framework.c b/tests/qtest/migration/framework.c
index a3bd92a9519..38a0a1a5264 100644
--- a/tests/qtest/migration/framework.c
+++ b/tests/qtest/migration/framework.c
@@ -938,6 +938,8 @@ MigrationTestEnv *migration_get_env(void)
         exit(1);
     }
 
+    env->arch = qtest_get_arch();
+
     env->has_kvm = qtest_has_accel("kvm");
     env->has_tcg = qtest_has_accel("tcg");
 
@@ -948,7 +950,6 @@ MigrationTestEnv *migration_get_env(void)
 
     env->has_dirty_ring = env->has_kvm && kvm_dirty_ring_supported();
     env->has_uffd = ufd_version_check(&env->uffd_feature_thread_id);
-    env->arch = qtest_get_arch();
     env->is_x86 = !strcmp(env->arch, "i386") || !strcmp(env->arch, "x86_64");
 
     env->tmpfs = g_dir_make_tmp("migration-test-XXXXXX", &err);
-- 
2.47.1

Re: [PATCH 3/7] tests/qtest/migration: Initialize MigrationTestEnv::arch early

[Fabiano Rosas]

Philippe Mathieu-Daudé <philmd@linaro.org> writes:

> Some tests expect MigrationTestEnv::arch to be set. Initialize
> it early enough to avoid SIGSEGV, for example like the following
> g_str_equal() call in migration/precopy-tests.c:
>
>    954 void migration_test_add_precopy(MigrationTestEnv *env)
>    955 {
>    ...
>   1001     if (g_str_equal(env->arch, "x86_64") && env->has_dirty_ring) {
>   1002
>   1003         migration_test_add("/migration/dirty_ring",
>   1004                            test_precopy_unix_dirty_ring);
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  tests/qtest/migration/framework.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/tests/qtest/migration/framework.c b/tests/qtest/migration/framework.c
> index a3bd92a9519..38a0a1a5264 100644
> --- a/tests/qtest/migration/framework.c
> +++ b/tests/qtest/migration/framework.c
> @@ -938,6 +938,8 @@ MigrationTestEnv *migration_get_env(void)
>          exit(1);
>      }
>  
> +    env->arch = qtest_get_arch();
> +
>      env->has_kvm = qtest_has_accel("kvm");
>      env->has_tcg = qtest_has_accel("tcg");
>  
> @@ -948,7 +950,6 @@ MigrationTestEnv *migration_get_env(void)
>  
>      env->has_dirty_ring = env->has_kvm && kvm_dirty_ring_supported();
>      env->has_uffd = ufd_version_check(&env->uffd_feature_thread_id);
> -    env->arch = qtest_get_arch();
>      env->is_x86 = !strcmp(env->arch, "i386") || !strcmp(env->arch, "x86_64");
>  
>      env->tmpfs = g_dir_make_tmp("migration-test-XXXXXX", &err);

Reviewed-by: Fabiano Rosas <farosas@suse.de>

The change itself is fine, but I think the actual issue is that we
shouldn't be calling g_test_skip() from migration_get_env(). There's no
point adding a bunch of tests if none of them will run because there's
no supported accel present. So:

    if (!env->has_tcg && !env->has_kvm) {
-        g_test_skip("No KVM or TCG accelerator available");
-        return env;
+        g_test_message("No KVM or TCG accelerator available");
+        exit(0);
    }

Re: [PATCH 3/7] tests/qtest/migration: Initialize MigrationTestEnv::arch early

[Richard Henderson]

On 1/28/25 05:54, Philippe Mathieu-Daudé wrote:
> Some tests expectMigrationTestEnv::arch to be set. Initialize
> it early enough to avoid SIGSEGV, for example like the following
> g_str_equal() call in migration/precopy-tests.c:
> 
>     954 void migration_test_add_precopy(MigrationTestEnv *env)
>     955 {
>     ...
>    1001     if (g_str_equal(env->arch, "x86_64") && env->has_dirty_ring) {
>    1002
>    1003         migration_test_add("/migration/dirty_ring",
>    1004                            test_precopy_unix_dirty_ring);
> 
> Signed-off-by: Philippe Mathieu-Daudé<philmd@linaro.org>
> ---
>   tests/qtest/migration/framework.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

[PATCH 4/7] tests/qtest/migration: Pass accelerator arguments as machine option

[Philippe Mathieu-Daudé]

The '-accel' CLI option is handler as sugar property as
'-machine,accel='. Replace the migration tests command
line, only using the best accelerator available (first
hardware, then software).

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 tests/qtest/migration/framework.c | 28 ++++++++++++++++------------
 1 file changed, 16 insertions(+), 12 deletions(-)

diff --git a/tests/qtest/migration/framework.c b/tests/qtest/migration/framework.c
index 38a0a1a5264..e567296b014 100644
--- a/tests/qtest/migration/framework.c
+++ b/tests/qtest/migration/framework.c
@@ -214,8 +214,9 @@ int migrate_start(QTestState **from, QTestState **to, const char *uri,
     const gchar *ignore_stderr;
     g_autofree char *shmem_opts = NULL;
     g_autofree char *shmem_path = NULL;
-    const char *kvm_opts = NULL;
-    const char *arch = qtest_get_arch();
+    const char *accel_args = NULL;
+    const MigrationTestEnv *env = migration_get_env();
+    const char *arch = env->arch;
     const char *memory_size;
     const char *machine_alias, *machine_opts = "";
     g_autofree char *machine = NULL;
@@ -296,8 +297,15 @@ int migrate_start(QTestState **from, QTestState **to, const char *uri,
             memory_size, shmem_path);
     }
 
-    if (args->use_dirty_ring) {
-        kvm_opts = ",dirty-ring-size=4096";
+    if (env->has_kvm) {
+        if (args->use_dirty_ring) {
+            accel_args = "kvm,dirty-ring-size=4096";
+        } else {
+            accel_args = "kvm";
+        }
+    } else {
+        assert(env->has_tcg);
+        accel_args = "tcg";
     }
 
     if (!qtest_has_machine(machine_alias)) {
@@ -311,14 +319,12 @@ int migrate_start(QTestState **from, QTestState **to, const char *uri,
 
     g_test_message("Using machine type: %s", machine);
 
-    cmd_source = g_strdup_printf("-accel kvm%s -accel tcg "
-                                 "-machine %s,%s "
+    cmd_source = g_strdup_printf("-machine %s,%s,accel=%s "
                                  "-name source,debug-threads=on "
                                  "-m %s "
                                  "-serial file:%s/src_serial "
                                  "%s %s %s %s",
-                                 kvm_opts ? kvm_opts : "",
-                                 machine, machine_opts,
+                                 machine, machine_opts, accel_args,
                                  memory_size, tmpfs,
                                  arch_opts ? arch_opts : "",
                                  shmem_opts ? shmem_opts : "",
@@ -332,15 +338,13 @@ int migrate_start(QTestState **from, QTestState **to, const char *uri,
                                      &src_state);
     }
 
-    cmd_target = g_strdup_printf("-accel kvm%s -accel tcg "
-                                 "-machine %s,%s "
+    cmd_target = g_strdup_printf("-machine %s,%s,accel=%s "
                                  "-name target,debug-threads=on "
                                  "-m %s "
                                  "-serial file:%s/dest_serial "
                                  "-incoming %s "
                                  "%s %s %s %s",
-                                 kvm_opts ? kvm_opts : "",
-                                 machine, machine_opts,
+                                 machine, machine_opts, accel_args,
                                  memory_size, tmpfs, uri,
                                  arch_opts ? arch_opts : "",
                                  shmem_opts ? shmem_opts : "",
-- 
2.47.1

Re: [PATCH 4/7] tests/qtest/migration: Pass accelerator arguments as machine option

[Fabiano Rosas]

Philippe Mathieu-Daudé <philmd@linaro.org> writes:

> The '-accel' CLI option is handler as sugar property as
> '-machine,accel='. Replace the migration tests command
> line, only using the best accelerator available (first
> hardware, then software).
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  tests/qtest/migration/framework.c | 28 ++++++++++++++++------------
>  1 file changed, 16 insertions(+), 12 deletions(-)
>
> diff --git a/tests/qtest/migration/framework.c b/tests/qtest/migration/framework.c
> index 38a0a1a5264..e567296b014 100644
> --- a/tests/qtest/migration/framework.c
> +++ b/tests/qtest/migration/framework.c
> @@ -214,8 +214,9 @@ int migrate_start(QTestState **from, QTestState **to, const char *uri,
>      const gchar *ignore_stderr;
>      g_autofree char *shmem_opts = NULL;
>      g_autofree char *shmem_path = NULL;
> -    const char *kvm_opts = NULL;
> -    const char *arch = qtest_get_arch();
> +    const char *accel_args = NULL;
> +    const MigrationTestEnv *env = migration_get_env();
> +    const char *arch = env->arch;
>      const char *memory_size;
>      const char *machine_alias, *machine_opts = "";
>      g_autofree char *machine = NULL;
> @@ -296,8 +297,15 @@ int migrate_start(QTestState **from, QTestState **to, const char *uri,
>              memory_size, shmem_path);
>      }
>  
> -    if (args->use_dirty_ring) {
> -        kvm_opts = ",dirty-ring-size=4096";
> +    if (env->has_kvm) {
> +        if (args->use_dirty_ring) {
> +            accel_args = "kvm,dirty-ring-size=4096";
> +        } else {
> +            accel_args = "kvm";
> +        }
> +    } else {
> +        assert(env->has_tcg);
> +        accel_args = "tcg";
>      }

I don't think this approach works when testing across
architectures. IIUC has_kvm will be true whenever the *test* can access
/dev/kvm. Which means if we run on a x86 while testing
qemu-system-s390x, then has_kvm==true but we actually need to use TCG,
so the fallback in the QEMU binary (-accel kvm -accel tcg) will be
automatically used.

>  
>      if (!qtest_has_machine(machine_alias)) {
> @@ -311,14 +319,12 @@ int migrate_start(QTestState **from, QTestState **to, const char *uri,
>  
>      g_test_message("Using machine type: %s", machine);
>  
> -    cmd_source = g_strdup_printf("-accel kvm%s -accel tcg "
> -                                 "-machine %s,%s "
> +    cmd_source = g_strdup_printf("-machine %s,%s,accel=%s "
>                                   "-name source,debug-threads=on "
>                                   "-m %s "
>                                   "-serial file:%s/src_serial "
>                                   "%s %s %s %s",
> -                                 kvm_opts ? kvm_opts : "",
> -                                 machine, machine_opts,
> +                                 machine, machine_opts, accel_args,
>                                   memory_size, tmpfs,
>                                   arch_opts ? arch_opts : "",
>                                   shmem_opts ? shmem_opts : "",
> @@ -332,15 +338,13 @@ int migrate_start(QTestState **from, QTestState **to, const char *uri,
>                                       &src_state);
>      }
>  
> -    cmd_target = g_strdup_printf("-accel kvm%s -accel tcg "
> -                                 "-machine %s,%s "
> +    cmd_target = g_strdup_printf("-machine %s,%s,accel=%s "
>                                   "-name target,debug-threads=on "
>                                   "-m %s "
>                                   "-serial file:%s/dest_serial "
>                                   "-incoming %s "
>                                   "%s %s %s %s",
> -                                 kvm_opts ? kvm_opts : "",
> -                                 machine, machine_opts,
> +                                 machine, machine_opts, accel_args,
>                                   memory_size, tmpfs, uri,
>                                   arch_opts ? arch_opts : "",
>                                   shmem_opts ? shmem_opts : "",

Re: [PATCH 4/7] tests/qtest/migration: Pass accelerator arguments as machine option

[Richard Henderson]

On 1/28/25 05:54, Philippe Mathieu-Daudé wrote:
> The '-accel' CLI option is handler as sugar property as
> '-machine,accel='. Replace the migration tests command
> line, only using the best accelerator available (first
> hardware, then software).

Is that really true?  I thought -accel was split from -machine explicitly because of the 
introduction of accelerator parameters, just like dirty-ring-size.  Otherwise you risk 
confusing accelerator parameters and machine parameters.


r~

[PATCH 5/7] tests/qtest/migration: Add MigrationTestEnv::has_hvf field

[Philippe Mathieu-Daudé]

Allow tests to tune their parameters when running on HVF.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 tests/qtest/migration/framework.h | 1 +
 tests/qtest/migration/framework.c | 1 +
 2 files changed, 2 insertions(+)

diff --git a/tests/qtest/migration/framework.h b/tests/qtest/migration/framework.h
index 7991ee56b6f..76bd4dc1a95 100644
--- a/tests/qtest/migration/framework.h
+++ b/tests/qtest/migration/framework.h
@@ -19,6 +19,7 @@
 
 typedef struct MigrationTestEnv {
     bool has_kvm;
+    bool has_hvf;
     bool has_tcg;
     bool has_uffd;
     bool uffd_feature_thread_id;
diff --git a/tests/qtest/migration/framework.c b/tests/qtest/migration/framework.c
index e567296b014..5629b8ba4e3 100644
--- a/tests/qtest/migration/framework.c
+++ b/tests/qtest/migration/framework.c
@@ -945,6 +945,7 @@ MigrationTestEnv *migration_get_env(void)
     env->arch = qtest_get_arch();
 
     env->has_kvm = qtest_has_accel("kvm");
+    env->has_hvf = qtest_has_accel("hvf");
     env->has_tcg = qtest_has_accel("tcg");
 
     if (!env->has_tcg && !env->has_kvm) {
-- 
2.47.1

Re: [PATCH 5/7] tests/qtest/migration: Add MigrationTestEnv::has_hvf field

[Fabiano Rosas]

Philippe Mathieu-Daudé <philmd@linaro.org> writes:

> Allow tests to tune their parameters when running on HVF.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Reviewed-by: Fabiano Rosas <farosas@suse.de>

Re: [PATCH 5/7] tests/qtest/migration: Add MigrationTestEnv::has_hvf field

[Richard Henderson]

On 1/28/25 05:54, Philippe Mathieu-Daudé wrote:
> Allow tests to tune their parameters when running on HVF.
> 
> Signed-off-by: Philippe Mathieu-Daudé<philmd@linaro.org>
> ---
>   tests/qtest/migration/framework.h | 1 +
>   tests/qtest/migration/framework.c | 1 +
>   2 files changed, 2 insertions(+)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

[RFC PATCH 6/7] tests/qtest/migration: Run aarch64/HVF tests using GICv2

[Philippe Mathieu-Daudé]

GICv3 isn't supported on aarch64/HVF, but GICv2 is.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
RFC: Test eventually timeouts :(

 tests/qtest/migration/framework.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tests/qtest/migration/framework.c b/tests/qtest/migration/framework.c
index 5629b8ba4e3..30808de14e0 100644
--- a/tests/qtest/migration/framework.c
+++ b/tests/qtest/migration/framework.c
@@ -266,7 +266,7 @@ int migrate_start(QTestState **from, QTestState **to, const char *uri,
     } else if (strcmp(arch, "aarch64") == 0) {
         memory_size = "150M";
         machine_alias = "virt";
-        machine_opts = "gic-version=3";
+        machine_opts = env->has_hvf ? "gic-version=2" : "gic-version=3";
         arch_opts = g_strdup_printf("-cpu max -kernel %s", bootpath);
         start_address = ARM_TEST_MEM_START;
         end_address = ARM_TEST_MEM_END;
@@ -303,6 +303,8 @@ int migrate_start(QTestState **from, QTestState **to, const char *uri,
         } else {
             accel_args = "kvm";
         }
+    } else if (env->has_hvf) {
+        accel_args = "hvf";
     } else {
         assert(env->has_tcg);
         accel_args = "tcg";
-- 
2.47.1

Re: [RFC PATCH 6/7] tests/qtest/migration: Run aarch64/HVF tests using GICv2

[Akihiko Odaki]

On 2025/01/28 22:54, Philippe Mathieu-Daudé wrote:
> GICv3 isn't supported on aarch64/HVF, but GICv2 is.

Commit bdb0ade663c7 ("tests/migration-test: Stick with gicv3 in aarch64 
test"), which set gic-version=3, says:
 > Switch to a static gic version "3" rather than using version "max",
 > so that GIC should be stable now across any future QEMU binaries for
 > migration-test.

 > Here the version can actually be anything as long as the ABI is
 > stable.

So I think we should always set gic-version=2 so that gic-version will 
not vary with hosts.

> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
> RFC: Test eventually timeouts :(
> 
>   tests/qtest/migration/framework.c | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/qtest/migration/framework.c b/tests/qtest/migration/framework.c
> index 5629b8ba4e3..30808de14e0 100644
> --- a/tests/qtest/migration/framework.c
> +++ b/tests/qtest/migration/framework.c
> @@ -266,7 +266,7 @@ int migrate_start(QTestState **from, QTestState **to, const char *uri,
>       } else if (strcmp(arch, "aarch64") == 0) {
>           memory_size = "150M";
>           machine_alias = "virt";
> -        machine_opts = "gic-version=3";
> +        machine_opts = env->has_hvf ? "gic-version=2" : "gic-version=3";
>           arch_opts = g_strdup_printf("-cpu max -kernel %s", bootpath);
>           start_address = ARM_TEST_MEM_START;
>           end_address = ARM_TEST_MEM_END;
> @@ -303,6 +303,8 @@ int migrate_start(QTestState **from, QTestState **to, const char *uri,
>           } else {
>               accel_args = "kvm";
>           }
> +    } else if (env->has_hvf) {
> +        accel_args = "hvf";
>       } else {
>           assert(env->has_tcg);
>           accel_args = "tcg";

Re: [RFC PATCH 6/7] tests/qtest/migration: Run aarch64/HVF tests using GICv2

[Phil Dennis-Jordan]

[-- Attachment #1: Type: text/plain, Size: 1646 bytes --]

On Tue, 28 Jan 2025 at 14:55, Philippe Mathieu-Daudé <philmd@linaro.org>
wrote:

> GICv3 isn't supported on aarch64/HVF, but GICv2 is.


Hmm. I'm still trying to get my head around this patch series and QTest in
general, but in my experience GICv3 works fine with HVF? (Device name
"arm-gicv3") Can you clarify what you mean or why this difference is needed?


> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
> RFC: Test eventually timeouts :(
>
>  tests/qtest/migration/framework.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/tests/qtest/migration/framework.c
> b/tests/qtest/migration/framework.c
> index 5629b8ba4e3..30808de14e0 100644
> --- a/tests/qtest/migration/framework.c
> +++ b/tests/qtest/migration/framework.c
> @@ -266,7 +266,7 @@ int migrate_start(QTestState **from, QTestState **to,
> const char *uri,
>      } else if (strcmp(arch, "aarch64") == 0) {
>          memory_size = "150M";
>          machine_alias = "virt";
> -        machine_opts = "gic-version=3";
> +        machine_opts = env->has_hvf ? "gic-version=2" : "gic-version=3";
>          arch_opts = g_strdup_printf("-cpu max -kernel %s", bootpath);
>          start_address = ARM_TEST_MEM_START;
>          end_address = ARM_TEST_MEM_END;
> @@ -303,6 +303,8 @@ int migrate_start(QTestState **from, QTestState **to,
> const char *uri,
>          } else {
>              accel_args = "kvm";
>          }
> +    } else if (env->has_hvf) {
> +        accel_args = "hvf";
>      } else {
>          assert(env->has_tcg);
>          accel_args = "tcg";
> --
> 2.47.1
>
>
>

[-- Attachment #2: Type: text/html, Size: 2449 bytes --]

[PATCH 7/7] tests/qtest/migration: Allow using accelerators different of TCG / KVM

[Philippe Mathieu-Daudé]

There is no particular reason to restrict all the framework
to TCG or KVM, since we can check on a per-test basis which
accelerator is available (via MigrationTestEnv::has_$ACCEL).

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 tests/qtest/migration/framework.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/tests/qtest/migration/framework.c b/tests/qtest/migration/framework.c
index 30808de14e0..e5f0712c266 100644
--- a/tests/qtest/migration/framework.c
+++ b/tests/qtest/migration/framework.c
@@ -950,11 +950,6 @@ MigrationTestEnv *migration_get_env(void)
     env->has_hvf = qtest_has_accel("hvf");
     env->has_tcg = qtest_has_accel("tcg");
 
-    if (!env->has_tcg && !env->has_kvm) {
-        g_test_skip("No KVM or TCG accelerator available");
-        return env;
-    }
-
     env->has_dirty_ring = env->has_kvm && kvm_dirty_ring_supported();
     env->has_uffd = ufd_version_check(&env->uffd_feature_thread_id);
     env->is_x86 = !strcmp(env->arch, "i386") || !strcmp(env->arch, "x86_64");
-- 
2.47.1

Re: [PATCH 7/7] tests/qtest/migration: Allow using accelerators different of TCG / KVM

[Fabiano Rosas]

Philippe Mathieu-Daudé <philmd@linaro.org> writes:

> There is no particular reason to restrict all the framework
> to TCG or KVM, since we can check on a per-test basis which
> accelerator is available (via MigrationTestEnv::has_$ACCEL).

The reason is:

CONFIG_KVM=n
CONFIG_TCG=n

The check is about "there is no accelerator at all".

>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  tests/qtest/migration/framework.c | 5 -----
>  1 file changed, 5 deletions(-)
>
> diff --git a/tests/qtest/migration/framework.c b/tests/qtest/migration/framework.c
> index 30808de14e0..e5f0712c266 100644
> --- a/tests/qtest/migration/framework.c
> +++ b/tests/qtest/migration/framework.c
> @@ -950,11 +950,6 @@ MigrationTestEnv *migration_get_env(void)
>      env->has_hvf = qtest_has_accel("hvf");
>      env->has_tcg = qtest_has_accel("tcg");
>  
> -    if (!env->has_tcg && !env->has_kvm) {
> -        g_test_skip("No KVM or TCG accelerator available");
> -        return env;
> -    }
> -
>      env->has_dirty_ring = env->has_kvm && kvm_dirty_ring_supported();
>      env->has_uffd = ufd_version_check(&env->uffd_feature_thread_id);
>      env->is_x86 = !strcmp(env->arch, "i386") || !strcmp(env->arch, "x86_64");

Re: [PATCH 7/7] tests/qtest/migration: Allow using accelerators different of TCG / KVM

[Richard Henderson]

On 1/28/25 07:04, Fabiano Rosas wrote:
> Philippe Mathieu-Daudé <philmd@linaro.org> writes:
> 
>> There is no particular reason to restrict all the framework
>> to TCG or KVM, since we can check on a per-test basis which
>> accelerator is available (via MigrationTestEnv::has_$ACCEL).
> 
> The reason is:
> 
> CONFIG_KVM=n
> CONFIG_TCG=n
> 
> The check is about "there is no accelerator at all".

We perform the no accelerator check in meson.build on a per-target basis.
So you'll never have a qemu-system-foo binary with zero accelerators.


r~

Re: [PATCH 7/7] tests/qtest/migration: Allow using accelerators different of TCG / KVM

[Fabiano Rosas]

Richard Henderson <richard.henderson@linaro.org> writes:

> On 1/28/25 07:04, Fabiano Rosas wrote:
>> Philippe Mathieu-Daudé <philmd@linaro.org> writes:
>> 
>>> There is no particular reason to restrict all the framework
>>> to TCG or KVM, since we can check on a per-test basis which
>>> accelerator is available (via MigrationTestEnv::has_$ACCEL).
>> 
>> The reason is:
>> 
>> CONFIG_KVM=n
>> CONFIG_TCG=n
>> 
>> The check is about "there is no accelerator at all".
>
> We perform the no accelerator check in meson.build on a per-target basis.
> So you'll never have a qemu-system-foo binary with zero accelerators.
>

Ok, the issue was not zero accelerators, but Xen. On aarch64, this
produces a binary with no TCG nor KVM:

--target-list=x86_64-softmmu --disable-tcg --enable-xen 

Re: [PATCH 7/7] tests/qtest/migration: Allow using accelerators different of TCG / KVM

[Richard Henderson]

On 1/28/25 11:50, Fabiano Rosas wrote:
> Richard Henderson <richard.henderson@linaro.org> writes:
> 
>> On 1/28/25 07:04, Fabiano Rosas wrote:
>>> Philippe Mathieu-Daudé <philmd@linaro.org> writes:
>>>
>>>> There is no particular reason to restrict all the framework
>>>> to TCG or KVM, since we can check on a per-test basis which
>>>> accelerator is available (via MigrationTestEnv::has_$ACCEL).
>>>
>>> The reason is:
>>>
>>> CONFIG_KVM=n
>>> CONFIG_TCG=n
>>>
>>> The check is about "there is no accelerator at all".
>>
>> We perform the no accelerator check in meson.build on a per-target basis.
>> So you'll never have a qemu-system-foo binary with zero accelerators.
>>
> 
> Ok, the issue was not zero accelerators, but Xen. On aarch64, this
> produces a binary with no TCG nor KVM:
> 
> --target-list=x86_64-softmmu --disable-tcg --enable-xen

Right.  Phil's patch set specifically mentions HVF, but there's no reason Xen can't be 
better handled too.  Unless there's something explicit about Xen that cannot be handled 
here?  But even then, the if condition needs updating.


r~

Re: [PATCH 7/7] tests/qtest/migration: Allow using accelerators different of TCG / KVM

[Richard Henderson]

On 1/28/25 05:54, Philippe Mathieu-Daudé wrote:
> There is no particular reason to restrict all the framework
> to TCG or KVM, since we can check on a per-test basis which
> accelerator is available (viaMigrationTestEnv::has_$ACCEL).
> 
> Signed-off-by: Philippe Mathieu-Daudé<philmd@linaro.org>
> ---
>   tests/qtest/migration/framework.c | 5 -----
>   1 file changed, 5 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Re: [PATCH 0/7] tests/qtest/migration: Update framework to allow using HVF accelerator

[Philippe Mathieu-Daudé]

On 28/1/25 14:54, Philippe Mathieu-Daudé wrote:
> Hi,
> 
> This series modify few bits of the migration QTest framework
> to allow running the tests using the HVF framework (also
> leaving the possibilty for other ones, removing the KVM/TCG
> restriction).

Forgot to mention:
Based-on: <20250128111821.93767-1-philmd@linaro.org>
           "tests/qtest: Make qtest_has_accel() generic"

> Philippe Mathieu-Daudé (7):
>    migration/dirtyrate: Do not unlock cpu_list lock twice
>    tests/qtest/migration: Make 'has_dirty_ring' generic
>    tests/qtest/migration: Initialize MigrationTestEnv::arch early
>    tests/qtest/migration: Pass accelerator arguments as machine option
>    tests/qtest/migration: Add MigrationTestEnv::has_hvf field
>    tests/qtest/migration: Run aarch64/HVF tests using GICv2
>    tests/qtest/migration: Allow using accelerators different of TCG / KVM