[PATCH v5 00/10] qtests: Check accelerator available at runtime via QMP 'query-accels'

[PATCH v5 00/10] qtests: Check accelerator available at runtime via QMP 'query-accels'

[Philippe Mathieu-Daudé]

Series fully reviewed.

Hi,

This series aims at having accelerator-independent qtests
by querying a QEMU instance at runtime to check the list
of built-in accelerators.

First we add the 'query-accels' QMP command,
then we add the qtest_has_accel() method to libqtest,
finally we use this new method to allow running
bios-tables-test on KVM-only builds.

Since v5:
- Removed patch 10 (Markus, patch justification not clear)
  'qtest/qmp-cmd-test: Make test build-independent from accelerator'
- Removed patch 12 (Alex, icount / record/replay issue)
  'tests/meson: Only build softfloat objects if TCG is selected (again)'
- Sorted @Accelerator QAPI enum (Eric)
- Added R-b/T-b

Since v4:
- Added Markus review comments
- Added R-b/A-b tags

Since v3:
- Addressed Markus & Drew review comments
- Added qtest/migration-test patch

Since v2:
- Addressed Eric & Paolo review comments

Since v1:
- kept over-engineered union (I don't how to do simple enum)
- dropped arm-cpu-features patches for now
- fixed typos (Eric)
- rename qtest_has_accel (Thomas)
- probe accel with machine none previous qtest (Paolo)
- iterate over QAPI enum (Markus)

Eric's suggestion of conditional QAPI didn't worked out,
as accelerator definitions are poisoned.

Phil.

Philippe Mathieu-Daudé (10):
  MAINTAINERS: Add qtest/arm-cpu-features.c to ARM TCG CPUs section
  accel: Introduce 'query-accels' QMP command
  tests/qtest: Add qtest_has_accel() method
  qtest/arm-cpu-features: Use generic qtest_has_accel() to check for KVM
  qtest/arm-cpu-features: Restrict sve_tests_sve_off_kvm test to KVM
  qtest/arm-cpu-features: Remove TCG fallback to KVM specific tests
  qtest/arm-cpu-features: Use generic qtest_has_accel() to check for TCG
  qtest/bios-tables-test: Make test build-independent from accelerator
  qtest/migration-test: Skip tests if KVM not builtin on s390x/ppc64
  qtest: Do not restrict bios-tables-test to Aarch64 hosts anymore

 qapi/machine.json              | 47 ++++++++++++++++
 tests/qtest/libqos/libqtest.h  |  8 +++
 accel/accel-qmp.c              | 49 +++++++++++++++++
 tests/qtest/arm-cpu-features.c | 55 ++++++-------------
 tests/qtest/bios-tables-test.c | 99 ++++++++++++++++++----------------
 tests/qtest/libqtest.c         | 29 ++++++++++
 tests/qtest/migration-test.c   |  4 +-
 MAINTAINERS                    |  1 +
 accel/meson.build              |  2 +-
 tests/qtest/meson.build        |  3 +-
 10 files changed, 207 insertions(+), 90 deletions(-)
 create mode 100644 accel/accel-qmp.c

-- 
2.26.3

[PATCH v5 01/10] MAINTAINERS: Add qtest/arm-cpu-features.c to ARM TCG CPUs section

[Philippe Mathieu-Daudé]

We want the ARM maintainers and the qemu-arm@ list to be
notified when this file is modified. Add an entry to the
'ARM TCG CPUs' section in the MAINTAINERS file.

Acked-by: Andrew Jones <drjones@redhat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 MAINTAINERS | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 4c05ff8bbab..5f1f59f9b3c 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -156,6 +156,7 @@ S: Maintained
 F: target/arm/
 F: tests/tcg/arm/
 F: tests/tcg/aarch64/
+F: tests/qtest/arm-cpu-features.c
 F: hw/arm/
 F: hw/cpu/a*mpcore.c
 F: include/hw/cpu/a*mpcore.h
-- 
2.26.3

[PATCH v5 02/10] accel: Introduce 'query-accels' QMP command

[Philippe Mathieu-Daudé]

Introduce the 'query-accels' QMP command which returns a list
of built-in accelerator names.

- Accelerator is a QAPI enum of all existing accelerators,

- AcceleratorInfo is a QAPI structure providing accelerator
  specific information. Currently the common structure base
  provides the name of the accelerator, while the specific
  part is empty, but each accelerator can expand it.

- 'query-accels' QMP command returns a list of @AcceleratorInfo

For example on a KVM-only build we get:

    { "execute": "query-accels" }
    {
        "return": [
            {
                "name": "qtest"
            },
            {
                "name": "kvm"
            }
        ]
    }

Note that we can't make the enum values or union branches conditional
because of target-specific poisoning of accelerator definitions.

Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Tested-by: Alex Bennée <alex.bennee@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 qapi/machine.json | 47 +++++++++++++++++++++++++++++++++++++++++++++
 accel/accel-qmp.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++
 accel/meson.build |  2 +-
 3 files changed, 97 insertions(+), 1 deletion(-)
 create mode 100644 accel/accel-qmp.c

diff --git a/qapi/machine.json b/qapi/machine.json
index 6e90d463fc9..6dd3b765248 100644
--- a/qapi/machine.json
+++ b/qapi/machine.json
@@ -1274,3 +1274,50 @@
 ##
 { 'event': 'MEM_UNPLUG_ERROR',
   'data': { 'device': 'str', 'msg': 'str' } }
+
+##
+# @Accelerator:
+#
+# An enumeration of accelerator names.
+#
+# Since: 6.1
+##
+{ 'enum': 'Accelerator',
+  'data': [ 'hax', 'hvf', 'kvm', 'qtest', 'tcg', 'whpx', 'xen' ] }
+
+##
+# @AcceleratorInfo:
+#
+# Accelerator information.
+#
+# @name: The accelerator name.
+#
+# Since: 6.1
+##
+{ 'struct': 'AcceleratorInfo',
+  'data': { 'name': 'Accelerator' } }
+
+##
+# @query-accels:
+#
+# Get a list of AcceleratorInfo for all built-in accelerators.
+#
+# Returns: a list of @AcceleratorInfo describing each accelerator.
+#
+# Since: 6.1
+#
+# Example:
+#
+# -> { "execute": "query-accels" }
+# <- { "return": [
+#        {
+#            "name": "qtest"
+#        },
+#        {
+#            "name": "kvm"
+#        }
+#    ] }
+#
+##
+{ 'command': 'query-accels',
+  'returns': ['AcceleratorInfo'] }
diff --git a/accel/accel-qmp.c b/accel/accel-qmp.c
new file mode 100644
index 00000000000..426737b3f9a
--- /dev/null
+++ b/accel/accel-qmp.c
@@ -0,0 +1,49 @@
+/*
+ * QEMU accelerators, QMP commands
+ *
+ * Copyright (c) 2021 Red Hat Inc.
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include "qemu/osdep.h"
+#include "qapi/qapi-commands-machine.h"
+
+static const bool accel_builtin_list[ACCELERATOR__MAX] = {
+    [ACCELERATOR_QTEST] = true,
+#ifdef CONFIG_TCG
+    [ACCELERATOR_TCG] = true,
+#endif
+#ifdef CONFIG_KVM
+    [ACCELERATOR_KVM] = true,
+#endif
+#ifdef CONFIG_HAX
+    [ACCELERATOR_HAX] = true,
+#endif
+#ifdef CONFIG_HVF
+    [ACCELERATOR_HVF] = true,
+#endif
+#ifdef CONFIG_WHPX
+    [ACCELERATOR_WHPX] = true,
+#endif
+#ifdef CONFIG_XEN_BACKEND
+    [ACCELERATOR_XEN] = true,
+#endif
+};
+
+AcceleratorInfoList *qmp_query_accels(Error **errp)
+{
+    AcceleratorInfoList *list = NULL, **tail = &list;
+
+    for (Accelerator accel = 0; accel < ACCELERATOR__MAX; accel++) {
+        if (accel_builtin_list[accel]) {
+            AcceleratorInfo *info = g_new0(AcceleratorInfo, 1);
+
+            info->name = accel;
+
+            QAPI_LIST_APPEND(tail, info);
+        }
+    }
+
+    return list;
+}
diff --git a/accel/meson.build b/accel/meson.build
index b44ba30c864..7a48f6d568d 100644
--- a/accel/meson.build
+++ b/accel/meson.build
@@ -1,4 +1,4 @@
-specific_ss.add(files('accel-common.c'))
+specific_ss.add(files('accel-common.c', 'accel-qmp.c'))
 softmmu_ss.add(files('accel-softmmu.c'))
 user_ss.add(files('accel-user.c'))
 
-- 
2.26.3

[PATCH v5 03/10] tests/qtest: Add qtest_has_accel() method

[Philippe Mathieu-Daudé]

Introduce the qtest_has_accel() method which allows a runtime
query on whether a QEMU instance has an accelerator built-in.

Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 tests/qtest/libqos/libqtest.h |  8 ++++++++
 tests/qtest/libqtest.c        | 29 +++++++++++++++++++++++++++++
 2 files changed, 37 insertions(+)

diff --git a/tests/qtest/libqos/libqtest.h b/tests/qtest/libqos/libqtest.h
index a68dcd79d44..d80c618c18d 100644
--- a/tests/qtest/libqos/libqtest.h
+++ b/tests/qtest/libqos/libqtest.h
@@ -763,6 +763,14 @@ void qmp_expect_error_and_unref(QDict *rsp, const char *class);
  */
 bool qtest_probe_child(QTestState *s);
 
+/**
+ * qtest_has_accel:
+ * @accel_name: Accelerator name to check for.
+ *
+ * Returns: true if the accelerator is built in.
+ */
+bool qtest_has_accel(const char *accel_name);
+
 /**
  * qtest_set_expected_status:
  * @s: QTestState instance to operate on.
diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
index 71e359efcd3..2156b7e3972 100644
--- a/tests/qtest/libqtest.c
+++ b/tests/qtest/libqtest.c
@@ -393,6 +393,35 @@ QTestState *qtest_init_with_serial(const char *extra_args, int *sock_fd)
     return qts;
 }
 
+bool qtest_has_accel(const char *accel_name)
+{
+    bool has_accel = false;
+    QDict *response;
+    QList *accels;
+    QListEntry *accel;
+    QTestState *qts;
+
+    qts = qtest_initf("-accel qtest -machine none");
+    response = qtest_qmp(qts, "{'execute': 'query-accels'}");
+    accels = qdict_get_qlist(response, "return");
+
+    QLIST_FOREACH_ENTRY(accels, accel) {
+        QDict *accel_dict = qobject_to(QDict, qlist_entry_obj(accel));
+        const char *name = qdict_get_str(accel_dict, "name");
+
+        if (g_str_equal(name, accel_name)) {
+            has_accel = true;
+            break;
+        }
+    }
+    qobject_unref(response);
+
+    qtest_quit(qts);
+
+    return has_accel;
+}
+
+
 void qtest_quit(QTestState *s)
 {
     qtest_remove_abrt_handler(s);
-- 
2.26.3

[PATCH v5 04/10] qtest/arm-cpu-features: Use generic qtest_has_accel() to check for KVM

[Philippe Mathieu-Daudé]

Use the recently added generic qtest_has_accel() method to
check if KVM is available.

Suggested-by: Claudio Fontana <cfontana@suse.de>
Reviewed-by: Andrew Jones <drjones@redhat.com>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 tests/qtest/arm-cpu-features.c | 25 +------------------------
 1 file changed, 1 insertion(+), 24 deletions(-)

diff --git a/tests/qtest/arm-cpu-features.c b/tests/qtest/arm-cpu-features.c
index 8252b85bb85..7f4b2521277 100644
--- a/tests/qtest/arm-cpu-features.c
+++ b/tests/qtest/arm-cpu-features.c
@@ -26,21 +26,6 @@
                     "  'arguments': { 'type': 'full', "
 #define QUERY_TAIL  "}}"
 
-static bool kvm_enabled(QTestState *qts)
-{
-    QDict *resp, *qdict;
-    bool enabled;
-
-    resp = qtest_qmp(qts, "{ 'execute': 'query-kvm' }");
-    g_assert(qdict_haskey(resp, "return"));
-    qdict = qdict_get_qdict(resp, "return");
-    g_assert(qdict_haskey(qdict, "enabled"));
-    enabled = qdict_get_bool(qdict, "enabled");
-    qobject_unref(resp);
-
-    return enabled;
-}
-
 static QDict *do_query_no_props(QTestState *qts, const char *cpu_type)
 {
     return qtest_qmp(qts, QUERY_HEAD "'model': { 'name': %s }"
@@ -493,14 +478,6 @@ static void test_query_cpu_model_expansion_kvm(const void *data)
 
     qts = qtest_init(MACHINE_KVM "-cpu max");
 
-    /*
-     * These tests target the 'host' CPU type, so KVM must be enabled.
-     */
-    if (!kvm_enabled(qts)) {
-        qtest_quit(qts);
-        return;
-    }
-
     /* Enabling and disabling kvm-no-adjvtime should always work. */
     assert_has_feature_disabled(qts, "host", "kvm-no-adjvtime");
     assert_set_feature(qts, "host", "kvm-no-adjvtime", true);
@@ -624,7 +601,7 @@ int main(int argc, char **argv)
      * order avoid attempting to run an AArch32 QEMU with KVM on
      * AArch64 hosts. That won't work and isn't easy to detect.
      */
-    if (g_str_equal(qtest_get_arch(), "aarch64")) {
+    if (g_str_equal(qtest_get_arch(), "aarch64") && qtest_has_accel("kvm")) {
         qtest_add_data_func("/arm/kvm/query-cpu-model-expansion",
                             NULL, test_query_cpu_model_expansion_kvm);
     }
-- 
2.26.3

[PATCH v5 05/10] qtest/arm-cpu-features: Restrict sve_tests_sve_off_kvm test to KVM

[Philippe Mathieu-Daudé]

The sve_tests_sve_off_kvm() test is KVM specific.
Only run it if KVM is available.

Suggested-by: Andrew Jones <drjones@redhat.com>
Reviewed-by: Andrew Jones <drjones@redhat.com>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 tests/qtest/arm-cpu-features.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/qtest/arm-cpu-features.c b/tests/qtest/arm-cpu-features.c
index 7f4b2521277..66300c3bc20 100644
--- a/tests/qtest/arm-cpu-features.c
+++ b/tests/qtest/arm-cpu-features.c
@@ -604,6 +604,8 @@ int main(int argc, char **argv)
     if (g_str_equal(qtest_get_arch(), "aarch64") && qtest_has_accel("kvm")) {
         qtest_add_data_func("/arm/kvm/query-cpu-model-expansion",
                             NULL, test_query_cpu_model_expansion_kvm);
+        qtest_add_data_func("/arm/kvm/query-cpu-model-expansion/sve-off",
+                            NULL, sve_tests_sve_off_kvm);
     }
 
     if (g_str_equal(qtest_get_arch(), "aarch64")) {
@@ -611,8 +613,6 @@ int main(int argc, char **argv)
                             NULL, sve_tests_sve_max_vq_8);
         qtest_add_data_func("/arm/max/query-cpu-model-expansion/sve-off",
                             NULL, sve_tests_sve_off);
-        qtest_add_data_func("/arm/kvm/query-cpu-model-expansion/sve-off",
-                            NULL, sve_tests_sve_off_kvm);
     }
 
     return g_test_run();
-- 
2.26.3

[PATCH v5 06/10] qtest/arm-cpu-features: Remove TCG fallback to KVM specific tests

[Philippe Mathieu-Daudé]

sve_tests_sve_off_kvm() and test_query_cpu_model_expansion_kvm()
tests are now only being run if KVM is available. Drop the TCG
fallback.

Suggested-by: Andrew Jones <drjones@redhat.com>
Reviewed-by: Andrew Jones <drjones@redhat.com>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 tests/qtest/arm-cpu-features.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/qtest/arm-cpu-features.c b/tests/qtest/arm-cpu-features.c
index 66300c3bc20..b1d406542f7 100644
--- a/tests/qtest/arm-cpu-features.c
+++ b/tests/qtest/arm-cpu-features.c
@@ -21,7 +21,7 @@
 #define SVE_MAX_VQ 16
 
 #define MACHINE     "-machine virt,gic-version=max -accel tcg "
-#define MACHINE_KVM "-machine virt,gic-version=max -accel kvm -accel tcg "
+#define MACHINE_KVM "-machine virt,gic-version=max -accel kvm "
 #define QUERY_HEAD  "{ 'execute': 'query-cpu-model-expansion', " \
                     "  'arguments': { 'type': 'full', "
 #define QUERY_TAIL  "}}"
-- 
2.26.3

[PATCH v5 07/10] qtest/arm-cpu-features: Use generic qtest_has_accel() to check for TCG

[Philippe Mathieu-Daudé]

Now than we can probe if the TCG accelerator is available
at runtime with a QMP command, only run these tests if TCG
is built into the QEMU binary.

Suggested-by: Andrew Jones <drjones@redhat.com>
Reviewed-by: Andrew Jones <drjones@redhat.com>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 tests/qtest/arm-cpu-features.c | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/tests/qtest/arm-cpu-features.c b/tests/qtest/arm-cpu-features.c
index b1d406542f7..0d9145dd168 100644
--- a/tests/qtest/arm-cpu-features.c
+++ b/tests/qtest/arm-cpu-features.c
@@ -20,7 +20,7 @@
  */
 #define SVE_MAX_VQ 16
 
-#define MACHINE     "-machine virt,gic-version=max -accel tcg "
+#define MACHINE_TCG "-machine virt,gic-version=max -accel tcg "
 #define MACHINE_KVM "-machine virt,gic-version=max -accel kvm "
 #define QUERY_HEAD  "{ 'execute': 'query-cpu-model-expansion', " \
                     "  'arguments': { 'type': 'full', "
@@ -337,7 +337,7 @@ static void sve_tests_sve_max_vq_8(const void *data)
 {
     QTestState *qts;
 
-    qts = qtest_init(MACHINE "-cpu max,sve-max-vq=8");
+    qts = qtest_init(MACHINE_TCG "-cpu max,sve-max-vq=8");
 
     assert_sve_vls(qts, "max", BIT_ULL(8) - 1, NULL);
 
@@ -372,7 +372,7 @@ static void sve_tests_sve_off(const void *data)
 {
     QTestState *qts;
 
-    qts = qtest_init(MACHINE "-cpu max,sve=off");
+    qts = qtest_init(MACHINE_TCG "-cpu max,sve=off");
 
     /* SVE is off, so the map should be empty. */
     assert_sve_vls(qts, "max", 0, NULL);
@@ -428,7 +428,7 @@ static void test_query_cpu_model_expansion(const void *data)
 {
     QTestState *qts;
 
-    qts = qtest_init(MACHINE "-cpu max");
+    qts = qtest_init(MACHINE_TCG "-cpu max");
 
     /* Test common query-cpu-model-expansion input validation */
     assert_type_full(qts);
@@ -593,8 +593,10 @@ int main(int argc, char **argv)
 {
     g_test_init(&argc, &argv, NULL);
 
-    qtest_add_data_func("/arm/query-cpu-model-expansion",
-                        NULL, test_query_cpu_model_expansion);
+    if (qtest_has_accel("tcg")) {
+        qtest_add_data_func("/arm/query-cpu-model-expansion",
+                            NULL, test_query_cpu_model_expansion);
+    }
 
     /*
      * For now we only run KVM specific tests with AArch64 QEMU in
@@ -608,7 +610,7 @@ int main(int argc, char **argv)
                             NULL, sve_tests_sve_off_kvm);
     }
 
-    if (g_str_equal(qtest_get_arch(), "aarch64")) {
+    if (g_str_equal(qtest_get_arch(), "aarch64") && qtest_has_accel("tcg")) {
         qtest_add_data_func("/arm/max/query-cpu-model-expansion/sve-max-vq-8",
                             NULL, sve_tests_sve_max_vq_8);
         qtest_add_data_func("/arm/max/query-cpu-model-expansion/sve-off",
-- 
2.26.3

[PATCH v5 08/10] qtest/bios-tables-test: Make test build-independent from accelerator

[Philippe Mathieu-Daudé]

Now than we can probe if the TCG accelerator is available
at runtime with a QMP command, do it once at the beginning
and only register the tests we can run.
We can then replace the #ifdef'ry by an assertion.

Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 tests/qtest/bios-tables-test.c | 99 ++++++++++++++++++----------------
 1 file changed, 52 insertions(+), 47 deletions(-)

diff --git a/tests/qtest/bios-tables-test.c b/tests/qtest/bios-tables-test.c
index 156d4174aa3..a4c7bddf6f3 100644
--- a/tests/qtest/bios-tables-test.c
+++ b/tests/qtest/bios-tables-test.c
@@ -97,6 +97,7 @@ typedef struct {
     QTestState *qts;
 } test_data;
 
+static bool tcg_accel_available;
 static char disk[] = "tests/acpi-test-disk-XXXXXX";
 static const char *data_dir = "tests/data/acpi";
 #ifdef CONFIG_IASL
@@ -718,15 +719,11 @@ static void test_acpi_one(const char *params, test_data *data)
     char *args;
     bool use_uefi = data->uefi_fl1 && data->uefi_fl2;
 
-#ifndef CONFIG_TCG
-    if (data->tcg_only) {
-        g_test_skip("TCG disabled, skipping ACPI tcg_only test");
-        return;
-    }
-#endif /* CONFIG_TCG */
+    assert(!data->tcg_only || tcg_accel_available);
 
     args = test_acpi_create_args(data, params, use_uefi);
     data->qts = qtest_init(args);
+
     test_acpi_load_tables(data, use_uefi);
 
     if (getenv(ACPI_REBUILD_EXPECTED_AML)) {
@@ -1504,6 +1501,8 @@ int main(int argc, char *argv[])
     const char *arch = qtest_get_arch();
     int ret;
 
+    tcg_accel_available = qtest_has_accel("tcg");
+
     g_test_init(&argc, &argv, NULL);
 
     if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
@@ -1512,56 +1511,62 @@ int main(int argc, char *argv[])
             return ret;
         }
         qtest_add_func("acpi/q35/oem-fields", test_acpi_oem_fields_q35);
-        qtest_add_func("acpi/q35/tpm-tis", test_acpi_q35_tcg_tpm_tis);
-        qtest_add_func("acpi/piix4", test_acpi_piix4_tcg);
         qtest_add_func("acpi/oem-fields", test_acpi_oem_fields_pc);
-        qtest_add_func("acpi/piix4/bridge", test_acpi_piix4_tcg_bridge);
+        qtest_add_func("acpi/microvm/oem-fields", test_acpi_oem_fields_microvm);
         qtest_add_func("acpi/piix4/pci-hotplug/no_root_hotplug",
                        test_acpi_piix4_no_root_hotplug);
         qtest_add_func("acpi/piix4/pci-hotplug/no_bridge_hotplug",
                        test_acpi_piix4_no_bridge_hotplug);
         qtest_add_func("acpi/piix4/pci-hotplug/off",
                        test_acpi_piix4_no_acpi_pci_hotplug);
-        qtest_add_func("acpi/q35", test_acpi_q35_tcg);
-        qtest_add_func("acpi/q35/bridge", test_acpi_q35_tcg_bridge);
-        qtest_add_func("acpi/q35/mmio64", test_acpi_q35_tcg_mmio64);
-        qtest_add_func("acpi/piix4/ipmi", test_acpi_piix4_tcg_ipmi);
-        qtest_add_func("acpi/q35/ipmi", test_acpi_q35_tcg_ipmi);
-        qtest_add_func("acpi/piix4/cpuhp", test_acpi_piix4_tcg_cphp);
-        qtest_add_func("acpi/q35/cpuhp", test_acpi_q35_tcg_cphp);
-        qtest_add_func("acpi/piix4/memhp", test_acpi_piix4_tcg_memhp);
-        qtest_add_func("acpi/q35/memhp", test_acpi_q35_tcg_memhp);
-        qtest_add_func("acpi/piix4/numamem", test_acpi_piix4_tcg_numamem);
-        qtest_add_func("acpi/q35/numamem", test_acpi_q35_tcg_numamem);
-        qtest_add_func("acpi/piix4/nosmm", test_acpi_piix4_tcg_nosmm);
-        qtest_add_func("acpi/piix4/smm-compat",
-                       test_acpi_piix4_tcg_smm_compat);
-        qtest_add_func("acpi/piix4/smm-compat-nosmm",
-                       test_acpi_piix4_tcg_smm_compat_nosmm);
-        qtest_add_func("acpi/piix4/nohpet", test_acpi_piix4_tcg_nohpet);
-        qtest_add_func("acpi/q35/nosmm", test_acpi_q35_tcg_nosmm);
-        qtest_add_func("acpi/q35/smm-compat",
-                       test_acpi_q35_tcg_smm_compat);
-        qtest_add_func("acpi/q35/smm-compat-nosmm",
-                       test_acpi_q35_tcg_smm_compat_nosmm);
-        qtest_add_func("acpi/q35/nohpet", test_acpi_q35_tcg_nohpet);
-        qtest_add_func("acpi/piix4/dimmpxm", test_acpi_piix4_tcg_dimm_pxm);
-        qtest_add_func("acpi/q35/dimmpxm", test_acpi_q35_tcg_dimm_pxm);
-        qtest_add_func("acpi/piix4/acpihmat", test_acpi_piix4_tcg_acpi_hmat);
-        qtest_add_func("acpi/q35/acpihmat", test_acpi_q35_tcg_acpi_hmat);
-        qtest_add_func("acpi/microvm", test_acpi_microvm_tcg);
-        qtest_add_func("acpi/microvm/usb", test_acpi_microvm_usb_tcg);
-        qtest_add_func("acpi/microvm/rtc", test_acpi_microvm_rtc_tcg);
-        qtest_add_func("acpi/microvm/ioapic2", test_acpi_microvm_ioapic2_tcg);
-        qtest_add_func("acpi/microvm/oem-fields", test_acpi_oem_fields_microvm);
-        if (strcmp(arch, "x86_64") == 0) {
-            qtest_add_func("acpi/microvm/pcie", test_acpi_microvm_pcie_tcg);
+        if (tcg_accel_available) {
+            qtest_add_func("acpi/q35/tpm-tis", test_acpi_q35_tcg_tpm_tis);
+            qtest_add_func("acpi/piix4", test_acpi_piix4_tcg);
+            qtest_add_func("acpi/piix4/bridge", test_acpi_piix4_tcg_bridge);
+            qtest_add_func("acpi/q35", test_acpi_q35_tcg);
+            qtest_add_func("acpi/q35/bridge", test_acpi_q35_tcg_bridge);
+            qtest_add_func("acpi/q35/mmio64", test_acpi_q35_tcg_mmio64);
+            qtest_add_func("acpi/piix4/ipmi", test_acpi_piix4_tcg_ipmi);
+            qtest_add_func("acpi/q35/ipmi", test_acpi_q35_tcg_ipmi);
+            qtest_add_func("acpi/piix4/cpuhp", test_acpi_piix4_tcg_cphp);
+            qtest_add_func("acpi/q35/cpuhp", test_acpi_q35_tcg_cphp);
+            qtest_add_func("acpi/piix4/memhp", test_acpi_piix4_tcg_memhp);
+            qtest_add_func("acpi/q35/memhp", test_acpi_q35_tcg_memhp);
+            qtest_add_func("acpi/piix4/numamem", test_acpi_piix4_tcg_numamem);
+            qtest_add_func("acpi/q35/numamem", test_acpi_q35_tcg_numamem);
+            qtest_add_func("acpi/piix4/nosmm", test_acpi_piix4_tcg_nosmm);
+            qtest_add_func("acpi/piix4/smm-compat",
+                           test_acpi_piix4_tcg_smm_compat);
+            qtest_add_func("acpi/piix4/smm-compat-nosmm",
+                           test_acpi_piix4_tcg_smm_compat_nosmm);
+            qtest_add_func("acpi/piix4/nohpet", test_acpi_piix4_tcg_nohpet);
+            qtest_add_func("acpi/q35/nosmm", test_acpi_q35_tcg_nosmm);
+            qtest_add_func("acpi/q35/smm-compat",
+                           test_acpi_q35_tcg_smm_compat);
+            qtest_add_func("acpi/q35/smm-compat-nosmm",
+                           test_acpi_q35_tcg_smm_compat_nosmm);
+            qtest_add_func("acpi/q35/nohpet", test_acpi_q35_tcg_nohpet);
+            qtest_add_func("acpi/piix4/dimmpxm", test_acpi_piix4_tcg_dimm_pxm);
+            qtest_add_func("acpi/q35/dimmpxm", test_acpi_q35_tcg_dimm_pxm);
+            qtest_add_func("acpi/piix4/acpihmat",
+                           test_acpi_piix4_tcg_acpi_hmat);
+            qtest_add_func("acpi/q35/acpihmat", test_acpi_q35_tcg_acpi_hmat);
+            qtest_add_func("acpi/microvm", test_acpi_microvm_tcg);
+            qtest_add_func("acpi/microvm/usb", test_acpi_microvm_usb_tcg);
+            qtest_add_func("acpi/microvm/rtc", test_acpi_microvm_rtc_tcg);
+            qtest_add_func("acpi/microvm/ioapic2",
+                           test_acpi_microvm_ioapic2_tcg);
+            if (strcmp(arch, "x86_64") == 0) {
+                qtest_add_func("acpi/microvm/pcie", test_acpi_microvm_pcie_tcg);
+            }
         }
     } else if (strcmp(arch, "aarch64") == 0) {
-        qtest_add_func("acpi/virt", test_acpi_virt_tcg);
-        qtest_add_func("acpi/virt/numamem", test_acpi_virt_tcg_numamem);
-        qtest_add_func("acpi/virt/memhp", test_acpi_virt_tcg_memhp);
-        qtest_add_func("acpi/virt/pxb", test_acpi_virt_tcg_pxb);
+        if (tcg_accel_available) {
+            qtest_add_func("acpi/virt", test_acpi_virt_tcg);
+            qtest_add_func("acpi/virt/numamem", test_acpi_virt_tcg_numamem);
+            qtest_add_func("acpi/virt/memhp", test_acpi_virt_tcg_memhp);
+            qtest_add_func("acpi/virt/pxb", test_acpi_virt_tcg_pxb);
+        }
         qtest_add_func("acpi/virt/oem-fields", test_acpi_oem_fields_virt);
     }
     ret = g_test_run();
-- 
2.26.3

[PATCH v5 09/10] qtest/migration-test: Skip tests if KVM not builtin on s390x/ppc64

[Philippe Mathieu-Daudé]

We might have a s390x/ppc64 QEMU binary built without the KVM
accelerator (configured with --disable-kvm).
Checking for /dev/kvm accessibility isn't enough, also check for the
accelerator in the binary.

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
Reviewed-by: Greg Kurz <groug@kaod.org>
Reviewed-by: Cornelia Huck <cohuck@redhat.com>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 tests/qtest/migration-test.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
index 3a711bb4929..c32a2aa30a2 100644
--- a/tests/qtest/migration-test.c
+++ b/tests/qtest/migration-test.c
@@ -1408,7 +1408,7 @@ int main(int argc, char **argv)
      */
     if (g_str_equal(qtest_get_arch(), "ppc64") &&
         (access("/sys/module/kvm_hv", F_OK) ||
-         access("/dev/kvm", R_OK | W_OK))) {
+         access("/dev/kvm", R_OK | W_OK) || !qtest_has_accel("kvm"))) {
         g_test_message("Skipping test: kvm_hv not available");
         return g_test_run();
     }
@@ -1419,7 +1419,7 @@ int main(int argc, char **argv)
      */
     if (g_str_equal(qtest_get_arch(), "s390x")) {
 #if defined(HOST_S390X)
-        if (access("/dev/kvm", R_OK | W_OK)) {
+        if (access("/dev/kvm", R_OK | W_OK) || !qtest_has_accel("kvm")) {
             g_test_message("Skipping test: kvm not available");
             return g_test_run();
         }
-- 
2.26.3

[PATCH v5 10/10] qtest: Do not restrict bios-tables-test to Aarch64 hosts anymore

[Philippe Mathieu-Daudé]

Since commit 82bf7ae84ce ("target/arm: Remove KVM support for
32-bit Arm hosts") we can remove the comment / check added in
commit ab6b6a77774 and directly run the bios-tables-test.

Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Tested-by: Alex Bennée <alex.bennee@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 tests/qtest/meson.build | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
index 0c767389217..46de073d155 100644
--- a/tests/qtest/meson.build
+++ b/tests/qtest/meson.build
@@ -175,14 +175,13 @@
    'boot-serial-test',
    'hexloader-test']
 
-# TODO: once aarch64 TCG is fixed on ARM 32 bit host, make bios-tables-test unconditional
 qtests_aarch64 = \
-  (cpu != 'arm' ? ['bios-tables-test'] : []) +                                                  \
   (config_all_devices.has_key('CONFIG_TPM_TIS_SYSBUS') ? ['tpm-tis-device-test'] : []) +        \
   (config_all_devices.has_key('CONFIG_TPM_TIS_SYSBUS') ? ['tpm-tis-device-swtpm-test'] : []) +  \
   ['arm-cpu-features',
    'numa-test',
    'boot-serial-test',
+   'bios-tables-test',
    'xlnx-can-test',
    'migration-test']
 
-- 
2.26.3

Re: [PATCH v5 08/10] qtest/bios-tables-test: Make test build-independent from accelerator

[Igor Mammedov]

On Sun,  2 May 2021 00:36:36 +0200
Philippe Mathieu-Daudé <philmd@redhat.com> wrote:

> Now than we can probe if the TCG accelerator is available
> at runtime with a QMP command, do it once at the beginning
> and only register the tests we can run.
> We can then replace the #ifdef'ry by an assertion.
> 
> Reviewed-by: Eric Blake <eblake@redhat.com>
> Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>  tests/qtest/bios-tables-test.c | 99 ++++++++++++++++++----------------
>  1 file changed, 52 insertions(+), 47 deletions(-)
> 
> diff --git a/tests/qtest/bios-tables-test.c b/tests/qtest/bios-tables-test.c
> index 156d4174aa3..a4c7bddf6f3 100644
> --- a/tests/qtest/bios-tables-test.c
> +++ b/tests/qtest/bios-tables-test.c
> @@ -97,6 +97,7 @@ typedef struct {
>      QTestState *qts;
>  } test_data;
>  
> +static bool tcg_accel_available;
>  static char disk[] = "tests/acpi-test-disk-XXXXXX";
>  static const char *data_dir = "tests/data/acpi";
>  #ifdef CONFIG_IASL
> @@ -718,15 +719,11 @@ static void test_acpi_one(const char *params, test_data *data)
>      char *args;
>      bool use_uefi = data->uefi_fl1 && data->uefi_fl2;
>  
> -#ifndef CONFIG_TCG
> -    if (data->tcg_only) {
> -        g_test_skip("TCG disabled, skipping ACPI tcg_only test");
> -        return;
> -    }
> -#endif /* CONFIG_TCG */
> +    assert(!data->tcg_only || tcg_accel_available);
>  
>      args = test_acpi_create_args(data, params, use_uefi);
>      data->qts = qtest_init(args);
> +
stray new line?

>      test_acpi_load_tables(data, use_uefi);
>  
>      if (getenv(ACPI_REBUILD_EXPECTED_AML)) {
> @@ -1504,6 +1501,8 @@ int main(int argc, char *argv[])
>      const char *arch = qtest_get_arch();
>      int ret;
>  
> +    tcg_accel_available = qtest_has_accel("tcg");
> +
>      g_test_init(&argc, &argv, NULL);
>  
>      if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
> @@ -1512,56 +1511,62 @@ int main(int argc, char *argv[])
>              return ret;
>          }
>          qtest_add_func("acpi/q35/oem-fields", test_acpi_oem_fields_q35);
> -        qtest_add_func("acpi/q35/tpm-tis", test_acpi_q35_tcg_tpm_tis);
> -        qtest_add_func("acpi/piix4", test_acpi_piix4_tcg);
>          qtest_add_func("acpi/oem-fields", test_acpi_oem_fields_pc);
> -        qtest_add_func("acpi/piix4/bridge", test_acpi_piix4_tcg_bridge);
> +        qtest_add_func("acpi/microvm/oem-fields", test_acpi_oem_fields_microvm);
>          qtest_add_func("acpi/piix4/pci-hotplug/no_root_hotplug",
>                         test_acpi_piix4_no_root_hotplug);
>          qtest_add_func("acpi/piix4/pci-hotplug/no_bridge_hotplug",
>                         test_acpi_piix4_no_bridge_hotplug);
>          qtest_add_func("acpi/piix4/pci-hotplug/off",
>                         test_acpi_piix4_no_acpi_pci_hotplug);
> -        qtest_add_func("acpi/q35", test_acpi_q35_tcg);
> -        qtest_add_func("acpi/q35/bridge", test_acpi_q35_tcg_bridge);
> -        qtest_add_func("acpi/q35/mmio64", test_acpi_q35_tcg_mmio64);
> -        qtest_add_func("acpi/piix4/ipmi", test_acpi_piix4_tcg_ipmi);
> -        qtest_add_func("acpi/q35/ipmi", test_acpi_q35_tcg_ipmi);
> -        qtest_add_func("acpi/piix4/cpuhp", test_acpi_piix4_tcg_cphp);
> -        qtest_add_func("acpi/q35/cpuhp", test_acpi_q35_tcg_cphp);
> -        qtest_add_func("acpi/piix4/memhp", test_acpi_piix4_tcg_memhp);
> -        qtest_add_func("acpi/q35/memhp", test_acpi_q35_tcg_memhp);
> -        qtest_add_func("acpi/piix4/numamem", test_acpi_piix4_tcg_numamem);
> -        qtest_add_func("acpi/q35/numamem", test_acpi_q35_tcg_numamem);
> -        qtest_add_func("acpi/piix4/nosmm", test_acpi_piix4_tcg_nosmm);
> -        qtest_add_func("acpi/piix4/smm-compat",
> -                       test_acpi_piix4_tcg_smm_compat);
> -        qtest_add_func("acpi/piix4/smm-compat-nosmm",
> -                       test_acpi_piix4_tcg_smm_compat_nosmm);
> -        qtest_add_func("acpi/piix4/nohpet", test_acpi_piix4_tcg_nohpet);
> -        qtest_add_func("acpi/q35/nosmm", test_acpi_q35_tcg_nosmm);
> -        qtest_add_func("acpi/q35/smm-compat",
> -                       test_acpi_q35_tcg_smm_compat);
> -        qtest_add_func("acpi/q35/smm-compat-nosmm",
> -                       test_acpi_q35_tcg_smm_compat_nosmm);
> -        qtest_add_func("acpi/q35/nohpet", test_acpi_q35_tcg_nohpet);
> -        qtest_add_func("acpi/piix4/dimmpxm", test_acpi_piix4_tcg_dimm_pxm);
> -        qtest_add_func("acpi/q35/dimmpxm", test_acpi_q35_tcg_dimm_pxm);
> -        qtest_add_func("acpi/piix4/acpihmat", test_acpi_piix4_tcg_acpi_hmat);
> -        qtest_add_func("acpi/q35/acpihmat", test_acpi_q35_tcg_acpi_hmat);
> -        qtest_add_func("acpi/microvm", test_acpi_microvm_tcg);
> -        qtest_add_func("acpi/microvm/usb", test_acpi_microvm_usb_tcg);
> -        qtest_add_func("acpi/microvm/rtc", test_acpi_microvm_rtc_tcg);
> -        qtest_add_func("acpi/microvm/ioapic2", test_acpi_microvm_ioapic2_tcg);
> -        qtest_add_func("acpi/microvm/oem-fields", test_acpi_oem_fields_microvm);
> -        if (strcmp(arch, "x86_64") == 0) {
> -            qtest_add_func("acpi/microvm/pcie", test_acpi_microvm_pcie_tcg);
> +        if (tcg_accel_available) {

most of this can run without TCG if KVM is available, so why are you limiting it to TCG only
or am I missing something?

> +            qtest_add_func("acpi/q35/tpm-tis", test_acpi_q35_tcg_tpm_tis);
> +            qtest_add_func("acpi/piix4", test_acpi_piix4_tcg);
> +            qtest_add_func("acpi/piix4/bridge", test_acpi_piix4_tcg_bridge);
> +            qtest_add_func("acpi/q35", test_acpi_q35_tcg);
> +            qtest_add_func("acpi/q35/bridge", test_acpi_q35_tcg_bridge);
> +            qtest_add_func("acpi/q35/mmio64", test_acpi_q35_tcg_mmio64);
> +            qtest_add_func("acpi/piix4/ipmi", test_acpi_piix4_tcg_ipmi);
> +            qtest_add_func("acpi/q35/ipmi", test_acpi_q35_tcg_ipmi);
> +            qtest_add_func("acpi/piix4/cpuhp", test_acpi_piix4_tcg_cphp);
> +            qtest_add_func("acpi/q35/cpuhp", test_acpi_q35_tcg_cphp);
> +            qtest_add_func("acpi/piix4/memhp", test_acpi_piix4_tcg_memhp);
> +            qtest_add_func("acpi/q35/memhp", test_acpi_q35_tcg_memhp);
> +            qtest_add_func("acpi/piix4/numamem", test_acpi_piix4_tcg_numamem);
> +            qtest_add_func("acpi/q35/numamem", test_acpi_q35_tcg_numamem);
> +            qtest_add_func("acpi/piix4/nosmm", test_acpi_piix4_tcg_nosmm);
> +            qtest_add_func("acpi/piix4/smm-compat",
> +                           test_acpi_piix4_tcg_smm_compat);
> +            qtest_add_func("acpi/piix4/smm-compat-nosmm",
> +                           test_acpi_piix4_tcg_smm_compat_nosmm);
> +            qtest_add_func("acpi/piix4/nohpet", test_acpi_piix4_tcg_nohpet);
> +            qtest_add_func("acpi/q35/nosmm", test_acpi_q35_tcg_nosmm);
> +            qtest_add_func("acpi/q35/smm-compat",
> +                           test_acpi_q35_tcg_smm_compat);
> +            qtest_add_func("acpi/q35/smm-compat-nosmm",
> +                           test_acpi_q35_tcg_smm_compat_nosmm);
> +            qtest_add_func("acpi/q35/nohpet", test_acpi_q35_tcg_nohpet);
> +            qtest_add_func("acpi/piix4/dimmpxm", test_acpi_piix4_tcg_dimm_pxm);
> +            qtest_add_func("acpi/q35/dimmpxm", test_acpi_q35_tcg_dimm_pxm);
> +            qtest_add_func("acpi/piix4/acpihmat",
> +                           test_acpi_piix4_tcg_acpi_hmat);
> +            qtest_add_func("acpi/q35/acpihmat", test_acpi_q35_tcg_acpi_hmat);
> +            qtest_add_func("acpi/microvm", test_acpi_microvm_tcg);
> +            qtest_add_func("acpi/microvm/usb", test_acpi_microvm_usb_tcg);
> +            qtest_add_func("acpi/microvm/rtc", test_acpi_microvm_rtc_tcg);
> +            qtest_add_func("acpi/microvm/ioapic2",
> +                           test_acpi_microvm_ioapic2_tcg);
> +            if (strcmp(arch, "x86_64") == 0) {
> +                qtest_add_func("acpi/microvm/pcie", test_acpi_microvm_pcie_tcg);
> +            }
>          }
>      } else if (strcmp(arch, "aarch64") == 0) {
> -        qtest_add_func("acpi/virt", test_acpi_virt_tcg);
> -        qtest_add_func("acpi/virt/numamem", test_acpi_virt_tcg_numamem);
> -        qtest_add_func("acpi/virt/memhp", test_acpi_virt_tcg_memhp);
> -        qtest_add_func("acpi/virt/pxb", test_acpi_virt_tcg_pxb);
> +        if (tcg_accel_available) {
> +            qtest_add_func("acpi/virt", test_acpi_virt_tcg);
> +            qtest_add_func("acpi/virt/numamem", test_acpi_virt_tcg_numamem);
> +            qtest_add_func("acpi/virt/memhp", test_acpi_virt_tcg_memhp);
> +            qtest_add_func("acpi/virt/pxb", test_acpi_virt_tcg_pxb);
> +        }
>          qtest_add_func("acpi/virt/oem-fields", test_acpi_oem_fields_virt);
>      }
>      ret = g_test_run();

Re: [PATCH v5 08/10] qtest/bios-tables-test: Make test build-independent from accelerator

[Philippe Mathieu-Daudé]

Hi Igor,

On 5/3/21 2:36 PM, Igor Mammedov wrote:
> On Sun,  2 May 2021 00:36:36 +0200
> Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
> 
>> Now than we can probe if the TCG accelerator is available
>> at runtime with a QMP command, do it once at the beginning
>> and only register the tests we can run.
>> We can then replace the #ifdef'ry by an assertion.
>>
>> Reviewed-by: Eric Blake <eblake@redhat.com>
>> Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>> ---
>>  tests/qtest/bios-tables-test.c | 99 ++++++++++++++++++----------------
>>  1 file changed, 52 insertions(+), 47 deletions(-)
>>
>> diff --git a/tests/qtest/bios-tables-test.c b/tests/qtest/bios-tables-test.c
>> index 156d4174aa3..a4c7bddf6f3 100644
>> --- a/tests/qtest/bios-tables-test.c
>> +++ b/tests/qtest/bios-tables-test.c
>> @@ -97,6 +97,7 @@ typedef struct {
>>      QTestState *qts;
>>  } test_data;
>>  
>> +static bool tcg_accel_available;
>>  static char disk[] = "tests/acpi-test-disk-XXXXXX";
>>  static const char *data_dir = "tests/data/acpi";
>>  #ifdef CONFIG_IASL
>> @@ -718,15 +719,11 @@ static void test_acpi_one(const char *params, test_data *data)
>>      char *args;
>>      bool use_uefi = data->uefi_fl1 && data->uefi_fl2;
>>  
>> -#ifndef CONFIG_TCG
>> -    if (data->tcg_only) {
>> -        g_test_skip("TCG disabled, skipping ACPI tcg_only test");
>> -        return;
>> -    }
>> -#endif /* CONFIG_TCG */
>> +    assert(!data->tcg_only || tcg_accel_available);
>>  
>>      args = test_acpi_create_args(data, params, use_uefi);
>>      data->qts = qtest_init(args);
>> +
> stray new line?

Oops.

> 
>>      test_acpi_load_tables(data, use_uefi);
>>  
>>      if (getenv(ACPI_REBUILD_EXPECTED_AML)) {
>> @@ -1504,6 +1501,8 @@ int main(int argc, char *argv[])
>>      const char *arch = qtest_get_arch();
>>      int ret;
>>  
>> +    tcg_accel_available = qtest_has_accel("tcg");
>> +
>>      g_test_init(&argc, &argv, NULL);
>>  
>>      if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
>> @@ -1512,56 +1511,62 @@ int main(int argc, char *argv[])
>>              return ret;
>>          }
>>          qtest_add_func("acpi/q35/oem-fields", test_acpi_oem_fields_q35);
>> -        qtest_add_func("acpi/q35/tpm-tis", test_acpi_q35_tcg_tpm_tis);
>> -        qtest_add_func("acpi/piix4", test_acpi_piix4_tcg);
>>          qtest_add_func("acpi/oem-fields", test_acpi_oem_fields_pc);
>> -        qtest_add_func("acpi/piix4/bridge", test_acpi_piix4_tcg_bridge);
>> +        qtest_add_func("acpi/microvm/oem-fields", test_acpi_oem_fields_microvm);
>>          qtest_add_func("acpi/piix4/pci-hotplug/no_root_hotplug",
>>                         test_acpi_piix4_no_root_hotplug);
>>          qtest_add_func("acpi/piix4/pci-hotplug/no_bridge_hotplug",
>>                         test_acpi_piix4_no_bridge_hotplug);
>>          qtest_add_func("acpi/piix4/pci-hotplug/off",
>>                         test_acpi_piix4_no_acpi_pci_hotplug);
>> -        qtest_add_func("acpi/q35", test_acpi_q35_tcg);
>> -        qtest_add_func("acpi/q35/bridge", test_acpi_q35_tcg_bridge);
>> -        qtest_add_func("acpi/q35/mmio64", test_acpi_q35_tcg_mmio64);
>> -        qtest_add_func("acpi/piix4/ipmi", test_acpi_piix4_tcg_ipmi);
>> -        qtest_add_func("acpi/q35/ipmi", test_acpi_q35_tcg_ipmi);
>> -        qtest_add_func("acpi/piix4/cpuhp", test_acpi_piix4_tcg_cphp);
>> -        qtest_add_func("acpi/q35/cpuhp", test_acpi_q35_tcg_cphp);
>> -        qtest_add_func("acpi/piix4/memhp", test_acpi_piix4_tcg_memhp);
>> -        qtest_add_func("acpi/q35/memhp", test_acpi_q35_tcg_memhp);
>> -        qtest_add_func("acpi/piix4/numamem", test_acpi_piix4_tcg_numamem);
>> -        qtest_add_func("acpi/q35/numamem", test_acpi_q35_tcg_numamem);
>> -        qtest_add_func("acpi/piix4/nosmm", test_acpi_piix4_tcg_nosmm);
>> -        qtest_add_func("acpi/piix4/smm-compat",
>> -                       test_acpi_piix4_tcg_smm_compat);
>> -        qtest_add_func("acpi/piix4/smm-compat-nosmm",
>> -                       test_acpi_piix4_tcg_smm_compat_nosmm);
>> -        qtest_add_func("acpi/piix4/nohpet", test_acpi_piix4_tcg_nohpet);
>> -        qtest_add_func("acpi/q35/nosmm", test_acpi_q35_tcg_nosmm);
>> -        qtest_add_func("acpi/q35/smm-compat",
>> -                       test_acpi_q35_tcg_smm_compat);
>> -        qtest_add_func("acpi/q35/smm-compat-nosmm",
>> -                       test_acpi_q35_tcg_smm_compat_nosmm);
>> -        qtest_add_func("acpi/q35/nohpet", test_acpi_q35_tcg_nohpet);
>> -        qtest_add_func("acpi/piix4/dimmpxm", test_acpi_piix4_tcg_dimm_pxm);
>> -        qtest_add_func("acpi/q35/dimmpxm", test_acpi_q35_tcg_dimm_pxm);
>> -        qtest_add_func("acpi/piix4/acpihmat", test_acpi_piix4_tcg_acpi_hmat);
>> -        qtest_add_func("acpi/q35/acpihmat", test_acpi_q35_tcg_acpi_hmat);
>> -        qtest_add_func("acpi/microvm", test_acpi_microvm_tcg);
>> -        qtest_add_func("acpi/microvm/usb", test_acpi_microvm_usb_tcg);
>> -        qtest_add_func("acpi/microvm/rtc", test_acpi_microvm_rtc_tcg);
>> -        qtest_add_func("acpi/microvm/ioapic2", test_acpi_microvm_ioapic2_tcg);
>> -        qtest_add_func("acpi/microvm/oem-fields", test_acpi_oem_fields_microvm);
>> -        if (strcmp(arch, "x86_64") == 0) {
>> -            qtest_add_func("acpi/microvm/pcie", test_acpi_microvm_pcie_tcg);
>> +        if (tcg_accel_available) {
> 
> most of this can run without TCG if KVM is available, so why are you limiting it to TCG only
> or am I missing something?

This patch is a simple API change, these tests are already restricted by
the 'g_test_skip("TCG disabled, skipping ACPI tcg_only test");' call.

If someone is willing to send a patch to have them run without TCG, I'm
fine to rebase my series on top. Else it can also be done on top of my
series. Whichever you prefer, but I'd rather not delay Claudio's work
too long...

> 
>> +            qtest_add_func("acpi/q35/tpm-tis", test_acpi_q35_tcg_tpm_tis);
>> +            qtest_add_func("acpi/piix4", test_acpi_piix4_tcg);
>> +            qtest_add_func("acpi/piix4/bridge", test_acpi_piix4_tcg_bridge);
>> +            qtest_add_func("acpi/q35", test_acpi_q35_tcg);
>> +            qtest_add_func("acpi/q35/bridge", test_acpi_q35_tcg_bridge);
>> +            qtest_add_func("acpi/q35/mmio64", test_acpi_q35_tcg_mmio64);
>> +            qtest_add_func("acpi/piix4/ipmi", test_acpi_piix4_tcg_ipmi);
>> +            qtest_add_func("acpi/q35/ipmi", test_acpi_q35_tcg_ipmi);
>> +            qtest_add_func("acpi/piix4/cpuhp", test_acpi_piix4_tcg_cphp);
>> +            qtest_add_func("acpi/q35/cpuhp", test_acpi_q35_tcg_cphp);
>> +            qtest_add_func("acpi/piix4/memhp", test_acpi_piix4_tcg_memhp);
>> +            qtest_add_func("acpi/q35/memhp", test_acpi_q35_tcg_memhp);
>> +            qtest_add_func("acpi/piix4/numamem", test_acpi_piix4_tcg_numamem);
>> +            qtest_add_func("acpi/q35/numamem", test_acpi_q35_tcg_numamem);
>> +            qtest_add_func("acpi/piix4/nosmm", test_acpi_piix4_tcg_nosmm);
>> +            qtest_add_func("acpi/piix4/smm-compat",
>> +                           test_acpi_piix4_tcg_smm_compat);
>> +            qtest_add_func("acpi/piix4/smm-compat-nosmm",
>> +                           test_acpi_piix4_tcg_smm_compat_nosmm);
>> +            qtest_add_func("acpi/piix4/nohpet", test_acpi_piix4_tcg_nohpet);
>> +            qtest_add_func("acpi/q35/nosmm", test_acpi_q35_tcg_nosmm);
>> +            qtest_add_func("acpi/q35/smm-compat",
>> +                           test_acpi_q35_tcg_smm_compat);
>> +            qtest_add_func("acpi/q35/smm-compat-nosmm",
>> +                           test_acpi_q35_tcg_smm_compat_nosmm);
>> +            qtest_add_func("acpi/q35/nohpet", test_acpi_q35_tcg_nohpet);
>> +            qtest_add_func("acpi/piix4/dimmpxm", test_acpi_piix4_tcg_dimm_pxm);
>> +            qtest_add_func("acpi/q35/dimmpxm", test_acpi_q35_tcg_dimm_pxm);
>> +            qtest_add_func("acpi/piix4/acpihmat",
>> +                           test_acpi_piix4_tcg_acpi_hmat);
>> +            qtest_add_func("acpi/q35/acpihmat", test_acpi_q35_tcg_acpi_hmat);
>> +            qtest_add_func("acpi/microvm", test_acpi_microvm_tcg);
>> +            qtest_add_func("acpi/microvm/usb", test_acpi_microvm_usb_tcg);
>> +            qtest_add_func("acpi/microvm/rtc", test_acpi_microvm_rtc_tcg);
>> +            qtest_add_func("acpi/microvm/ioapic2",
>> +                           test_acpi_microvm_ioapic2_tcg);
>> +            if (strcmp(arch, "x86_64") == 0) {
>> +                qtest_add_func("acpi/microvm/pcie", test_acpi_microvm_pcie_tcg);
>> +            }
>>          }
>>      } else if (strcmp(arch, "aarch64") == 0) {
>> -        qtest_add_func("acpi/virt", test_acpi_virt_tcg);
>> -        qtest_add_func("acpi/virt/numamem", test_acpi_virt_tcg_numamem);
>> -        qtest_add_func("acpi/virt/memhp", test_acpi_virt_tcg_memhp);
>> -        qtest_add_func("acpi/virt/pxb", test_acpi_virt_tcg_pxb);
>> +        if (tcg_accel_available) {
>> +            qtest_add_func("acpi/virt", test_acpi_virt_tcg);
>> +            qtest_add_func("acpi/virt/numamem", test_acpi_virt_tcg_numamem);
>> +            qtest_add_func("acpi/virt/memhp", test_acpi_virt_tcg_memhp);
>> +            qtest_add_func("acpi/virt/pxb", test_acpi_virt_tcg_pxb);
>> +        }
>>          qtest_add_func("acpi/virt/oem-fields", test_acpi_oem_fields_virt);
>>      }
>>      ret = g_test_run();
> 

Re: [PATCH v5 08/10] qtest/bios-tables-test: Make test build-independent from accelerator

[Igor Mammedov]

On Mon, 3 May 2021 14:44:32 +0200
Philippe Mathieu-Daudé <philmd@redhat.com> wrote:

> Hi Igor,
> 
> On 5/3/21 2:36 PM, Igor Mammedov wrote:
> > On Sun,  2 May 2021 00:36:36 +0200
> > Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
> >   
> >> Now than we can probe if the TCG accelerator is available
> >> at runtime with a QMP command, do it once at the beginning
> >> and only register the tests we can run.
> >> We can then replace the #ifdef'ry by an assertion.
> >>
> >> Reviewed-by: Eric Blake <eblake@redhat.com>
> >> Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
> >> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> >> ---
> >>  tests/qtest/bios-tables-test.c | 99 ++++++++++++++++++----------------
> >>  1 file changed, 52 insertions(+), 47 deletions(-)
> >>
> >> diff --git a/tests/qtest/bios-tables-test.c b/tests/qtest/bios-tables-test.c
> >> index 156d4174aa3..a4c7bddf6f3 100644
> >> --- a/tests/qtest/bios-tables-test.c
> >> +++ b/tests/qtest/bios-tables-test.c
> >> @@ -97,6 +97,7 @@ typedef struct {
> >>      QTestState *qts;
> >>  } test_data;
> >>  
> >> +static bool tcg_accel_available;
> >>  static char disk[] = "tests/acpi-test-disk-XXXXXX";
> >>  static const char *data_dir = "tests/data/acpi";
> >>  #ifdef CONFIG_IASL
> >> @@ -718,15 +719,11 @@ static void test_acpi_one(const char *params, test_data *data)
> >>      char *args;
> >>      bool use_uefi = data->uefi_fl1 && data->uefi_fl2;
> >>  
> >> -#ifndef CONFIG_TCG
> >> -    if (data->tcg_only) {
> >> -        g_test_skip("TCG disabled, skipping ACPI tcg_only test");
> >> -        return;
> >> -    }
> >> -#endif /* CONFIG_TCG */
> >> +    assert(!data->tcg_only || tcg_accel_available);
> >>  
> >>      args = test_acpi_create_args(data, params, use_uefi);
> >>      data->qts = qtest_init(args);
> >> +  
> > stray new line?  
> 
> Oops.
> 
> >   
> >>      test_acpi_load_tables(data, use_uefi);
> >>  
> >>      if (getenv(ACPI_REBUILD_EXPECTED_AML)) {
> >> @@ -1504,6 +1501,8 @@ int main(int argc, char *argv[])
> >>      const char *arch = qtest_get_arch();
> >>      int ret;
> >>  
> >> +    tcg_accel_available = qtest_has_accel("tcg");
> >> +
> >>      g_test_init(&argc, &argv, NULL);
> >>  
> >>      if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
> >> @@ -1512,56 +1511,62 @@ int main(int argc, char *argv[])
> >>              return ret;
> >>          }
> >>          qtest_add_func("acpi/q35/oem-fields", test_acpi_oem_fields_q35);
> >> -        qtest_add_func("acpi/q35/tpm-tis", test_acpi_q35_tcg_tpm_tis);
> >> -        qtest_add_func("acpi/piix4", test_acpi_piix4_tcg);
> >>          qtest_add_func("acpi/oem-fields", test_acpi_oem_fields_pc);
> >> -        qtest_add_func("acpi/piix4/bridge", test_acpi_piix4_tcg_bridge);
> >> +        qtest_add_func("acpi/microvm/oem-fields", test_acpi_oem_fields_microvm);
> >>          qtest_add_func("acpi/piix4/pci-hotplug/no_root_hotplug",
> >>                         test_acpi_piix4_no_root_hotplug);
> >>          qtest_add_func("acpi/piix4/pci-hotplug/no_bridge_hotplug",
> >>                         test_acpi_piix4_no_bridge_hotplug);
> >>          qtest_add_func("acpi/piix4/pci-hotplug/off",
> >>                         test_acpi_piix4_no_acpi_pci_hotplug);
> >> -        qtest_add_func("acpi/q35", test_acpi_q35_tcg);
> >> -        qtest_add_func("acpi/q35/bridge", test_acpi_q35_tcg_bridge);
> >> -        qtest_add_func("acpi/q35/mmio64", test_acpi_q35_tcg_mmio64);
> >> -        qtest_add_func("acpi/piix4/ipmi", test_acpi_piix4_tcg_ipmi);
> >> -        qtest_add_func("acpi/q35/ipmi", test_acpi_q35_tcg_ipmi);
> >> -        qtest_add_func("acpi/piix4/cpuhp", test_acpi_piix4_tcg_cphp);
> >> -        qtest_add_func("acpi/q35/cpuhp", test_acpi_q35_tcg_cphp);
> >> -        qtest_add_func("acpi/piix4/memhp", test_acpi_piix4_tcg_memhp);
> >> -        qtest_add_func("acpi/q35/memhp", test_acpi_q35_tcg_memhp);
> >> -        qtest_add_func("acpi/piix4/numamem", test_acpi_piix4_tcg_numamem);
> >> -        qtest_add_func("acpi/q35/numamem", test_acpi_q35_tcg_numamem);
> >> -        qtest_add_func("acpi/piix4/nosmm", test_acpi_piix4_tcg_nosmm);
> >> -        qtest_add_func("acpi/piix4/smm-compat",
> >> -                       test_acpi_piix4_tcg_smm_compat);
> >> -        qtest_add_func("acpi/piix4/smm-compat-nosmm",
> >> -                       test_acpi_piix4_tcg_smm_compat_nosmm);
> >> -        qtest_add_func("acpi/piix4/nohpet", test_acpi_piix4_tcg_nohpet);
> >> -        qtest_add_func("acpi/q35/nosmm", test_acpi_q35_tcg_nosmm);
> >> -        qtest_add_func("acpi/q35/smm-compat",
> >> -                       test_acpi_q35_tcg_smm_compat);
> >> -        qtest_add_func("acpi/q35/smm-compat-nosmm",
> >> -                       test_acpi_q35_tcg_smm_compat_nosmm);
> >> -        qtest_add_func("acpi/q35/nohpet", test_acpi_q35_tcg_nohpet);
> >> -        qtest_add_func("acpi/piix4/dimmpxm", test_acpi_piix4_tcg_dimm_pxm);
> >> -        qtest_add_func("acpi/q35/dimmpxm", test_acpi_q35_tcg_dimm_pxm);
> >> -        qtest_add_func("acpi/piix4/acpihmat", test_acpi_piix4_tcg_acpi_hmat);
> >> -        qtest_add_func("acpi/q35/acpihmat", test_acpi_q35_tcg_acpi_hmat);
> >> -        qtest_add_func("acpi/microvm", test_acpi_microvm_tcg);
> >> -        qtest_add_func("acpi/microvm/usb", test_acpi_microvm_usb_tcg);
> >> -        qtest_add_func("acpi/microvm/rtc", test_acpi_microvm_rtc_tcg);
> >> -        qtest_add_func("acpi/microvm/ioapic2", test_acpi_microvm_ioapic2_tcg);
> >> -        qtest_add_func("acpi/microvm/oem-fields", test_acpi_oem_fields_microvm);
> >> -        if (strcmp(arch, "x86_64") == 0) {
> >> -            qtest_add_func("acpi/microvm/pcie", test_acpi_microvm_pcie_tcg);
> >> +        if (tcg_accel_available) {  
> > 
> > most of this can run without TCG if KVM is available, so why are you limiting it to TCG only
> > or am I missing something?  
> 
> This patch is a simple API change, these tests are already restricted by
> the 'g_test_skip("TCG disabled, skipping ACPI tcg_only test");' call.
> 


with current code, assume we have TCG compiled out:
 - test_acpi_one() will execute any test that is not marked as tcg_only

with this patch if tcg_accel_available == False,
it will not even register any test under "if (tcg_accel_available) {" branch
and in this patch that includes a bunch of _non_ tcg_only tests.
So tests won't be executed on KVM only build, where previously they were executed just fine.

> If someone is willing to send a patch to have them run without TCG, I'm
> fine to rebase my series on top. Else it can also be done on top of my
> series. Whichever you prefer, but I'd rather not delay Claudio's work
> too long...
> 
> >   
> >> +            qtest_add_func("acpi/q35/tpm-tis", test_acpi_q35_tcg_tpm_tis);
> >> +            qtest_add_func("acpi/piix4", test_acpi_piix4_tcg);
> >> +            qtest_add_func("acpi/piix4/bridge", test_acpi_piix4_tcg_bridge);
> >> +            qtest_add_func("acpi/q35", test_acpi_q35_tcg);
> >> +            qtest_add_func("acpi/q35/bridge", test_acpi_q35_tcg_bridge);
> >> +            qtest_add_func("acpi/q35/mmio64", test_acpi_q35_tcg_mmio64);
> >> +            qtest_add_func("acpi/piix4/ipmi", test_acpi_piix4_tcg_ipmi);
> >> +            qtest_add_func("acpi/q35/ipmi", test_acpi_q35_tcg_ipmi);
> >> +            qtest_add_func("acpi/piix4/cpuhp", test_acpi_piix4_tcg_cphp);
> >> +            qtest_add_func("acpi/q35/cpuhp", test_acpi_q35_tcg_cphp);
> >> +            qtest_add_func("acpi/piix4/memhp", test_acpi_piix4_tcg_memhp);
> >> +            qtest_add_func("acpi/q35/memhp", test_acpi_q35_tcg_memhp);
> >> +            qtest_add_func("acpi/piix4/numamem", test_acpi_piix4_tcg_numamem);
> >> +            qtest_add_func("acpi/q35/numamem", test_acpi_q35_tcg_numamem);
> >> +            qtest_add_func("acpi/piix4/nosmm", test_acpi_piix4_tcg_nosmm);
> >> +            qtest_add_func("acpi/piix4/smm-compat",
> >> +                           test_acpi_piix4_tcg_smm_compat);
> >> +            qtest_add_func("acpi/piix4/smm-compat-nosmm",
> >> +                           test_acpi_piix4_tcg_smm_compat_nosmm);
> >> +            qtest_add_func("acpi/piix4/nohpet", test_acpi_piix4_tcg_nohpet);
> >> +            qtest_add_func("acpi/q35/nosmm", test_acpi_q35_tcg_nosmm);
> >> +            qtest_add_func("acpi/q35/smm-compat",
> >> +                           test_acpi_q35_tcg_smm_compat);
> >> +            qtest_add_func("acpi/q35/smm-compat-nosmm",
> >> +                           test_acpi_q35_tcg_smm_compat_nosmm);
> >> +            qtest_add_func("acpi/q35/nohpet", test_acpi_q35_tcg_nohpet);
> >> +            qtest_add_func("acpi/piix4/dimmpxm", test_acpi_piix4_tcg_dimm_pxm);
> >> +            qtest_add_func("acpi/q35/dimmpxm", test_acpi_q35_tcg_dimm_pxm);
> >> +            qtest_add_func("acpi/piix4/acpihmat",
> >> +                           test_acpi_piix4_tcg_acpi_hmat);
> >> +            qtest_add_func("acpi/q35/acpihmat", test_acpi_q35_tcg_acpi_hmat);
> >> +            qtest_add_func("acpi/microvm", test_acpi_microvm_tcg);
> >> +            qtest_add_func("acpi/microvm/usb", test_acpi_microvm_usb_tcg);
> >> +            qtest_add_func("acpi/microvm/rtc", test_acpi_microvm_rtc_tcg);
> >> +            qtest_add_func("acpi/microvm/ioapic2",
> >> +                           test_acpi_microvm_ioapic2_tcg);
> >> +            if (strcmp(arch, "x86_64") == 0) {
> >> +                qtest_add_func("acpi/microvm/pcie", test_acpi_microvm_pcie_tcg);
> >> +            }
> >>          }
> >>      } else if (strcmp(arch, "aarch64") == 0) {
> >> -        qtest_add_func("acpi/virt", test_acpi_virt_tcg);
> >> -        qtest_add_func("acpi/virt/numamem", test_acpi_virt_tcg_numamem);
> >> -        qtest_add_func("acpi/virt/memhp", test_acpi_virt_tcg_memhp);
> >> -        qtest_add_func("acpi/virt/pxb", test_acpi_virt_tcg_pxb);
> >> +        if (tcg_accel_available) {
> >> +            qtest_add_func("acpi/virt", test_acpi_virt_tcg);
> >> +            qtest_add_func("acpi/virt/numamem", test_acpi_virt_tcg_numamem);
> >> +            qtest_add_func("acpi/virt/memhp", test_acpi_virt_tcg_memhp);
> >> +            qtest_add_func("acpi/virt/pxb", test_acpi_virt_tcg_pxb);
> >> +        }
> >>          qtest_add_func("acpi/virt/oem-fields", test_acpi_oem_fields_virt);
> >>      }
> >>      ret = g_test_run();  
> >   
> 
> 

Re: [PATCH v5 08/10] qtest/bios-tables-test: Make test build-independent from accelerator

[Philippe Mathieu-Daudé]

On 5/3/21 6:04 PM, Igor Mammedov wrote:
> On Mon, 3 May 2021 14:44:32 +0200
> Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
> 
>> Hi Igor,
>>
>> On 5/3/21 2:36 PM, Igor Mammedov wrote:
>>> On Sun,  2 May 2021 00:36:36 +0200
>>> Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
>>>   
>>>> Now than we can probe if the TCG accelerator is available
>>>> at runtime with a QMP command, do it once at the beginning
>>>> and only register the tests we can run.
>>>> We can then replace the #ifdef'ry by an assertion.
>>>>
>>>> Reviewed-by: Eric Blake <eblake@redhat.com>
>>>> Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
>>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>>>> ---
>>>>  tests/qtest/bios-tables-test.c | 99 ++++++++++++++++++----------------
>>>>  1 file changed, 52 insertions(+), 47 deletions(-)
>>>>
>>>> diff --git a/tests/qtest/bios-tables-test.c b/tests/qtest/bios-tables-test.c
>>>> index 156d4174aa3..a4c7bddf6f3 100644
>>>> --- a/tests/qtest/bios-tables-test.c
>>>> +++ b/tests/qtest/bios-tables-test.c
>>>> @@ -97,6 +97,7 @@ typedef struct {
>>>>      QTestState *qts;
>>>>  } test_data;
>>>>  
>>>> +static bool tcg_accel_available;
>>>>  static char disk[] = "tests/acpi-test-disk-XXXXXX";
>>>>  static const char *data_dir = "tests/data/acpi";
>>>>  #ifdef CONFIG_IASL
>>>> @@ -718,15 +719,11 @@ static void test_acpi_one(const char *params, test_data *data)
>>>>      char *args;
>>>>      bool use_uefi = data->uefi_fl1 && data->uefi_fl2;
>>>>  
>>>> -#ifndef CONFIG_TCG
>>>> -    if (data->tcg_only) {
>>>> -        g_test_skip("TCG disabled, skipping ACPI tcg_only test");
>>>> -        return;
>>>> -    }
>>>> -#endif /* CONFIG_TCG */
>>>> +    assert(!data->tcg_only || tcg_accel_available);
>>>>  
>>>>      args = test_acpi_create_args(data, params, use_uefi);
>>>>      data->qts = qtest_init(args);
>>>> +  
>>> stray new line?  
>>
>> Oops.
>>
>>>   
>>>>      test_acpi_load_tables(data, use_uefi);
>>>>  
>>>>      if (getenv(ACPI_REBUILD_EXPECTED_AML)) {
>>>> @@ -1504,6 +1501,8 @@ int main(int argc, char *argv[])
>>>>      const char *arch = qtest_get_arch();
>>>>      int ret;
>>>>  
>>>> +    tcg_accel_available = qtest_has_accel("tcg");
>>>> +
>>>>      g_test_init(&argc, &argv, NULL);
>>>>  
>>>>      if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
>>>> @@ -1512,56 +1511,62 @@ int main(int argc, char *argv[])
>>>>              return ret;
>>>>          }
>>>>          qtest_add_func("acpi/q35/oem-fields", test_acpi_oem_fields_q35);
>>>> -        qtest_add_func("acpi/q35/tpm-tis", test_acpi_q35_tcg_tpm_tis);
>>>> -        qtest_add_func("acpi/piix4", test_acpi_piix4_tcg);
>>>>          qtest_add_func("acpi/oem-fields", test_acpi_oem_fields_pc);
>>>> -        qtest_add_func("acpi/piix4/bridge", test_acpi_piix4_tcg_bridge);
>>>> +        qtest_add_func("acpi/microvm/oem-fields", test_acpi_oem_fields_microvm);
>>>>          qtest_add_func("acpi/piix4/pci-hotplug/no_root_hotplug",
>>>>                         test_acpi_piix4_no_root_hotplug);
>>>>          qtest_add_func("acpi/piix4/pci-hotplug/no_bridge_hotplug",
>>>>                         test_acpi_piix4_no_bridge_hotplug);
>>>>          qtest_add_func("acpi/piix4/pci-hotplug/off",
>>>>                         test_acpi_piix4_no_acpi_pci_hotplug);
>>>> -        qtest_add_func("acpi/q35", test_acpi_q35_tcg);
>>>> -        qtest_add_func("acpi/q35/bridge", test_acpi_q35_tcg_bridge);
>>>> -        qtest_add_func("acpi/q35/mmio64", test_acpi_q35_tcg_mmio64);
>>>> -        qtest_add_func("acpi/piix4/ipmi", test_acpi_piix4_tcg_ipmi);
>>>> -        qtest_add_func("acpi/q35/ipmi", test_acpi_q35_tcg_ipmi);
>>>> -        qtest_add_func("acpi/piix4/cpuhp", test_acpi_piix4_tcg_cphp);
>>>> -        qtest_add_func("acpi/q35/cpuhp", test_acpi_q35_tcg_cphp);
>>>> -        qtest_add_func("acpi/piix4/memhp", test_acpi_piix4_tcg_memhp);
>>>> -        qtest_add_func("acpi/q35/memhp", test_acpi_q35_tcg_memhp);
>>>> -        qtest_add_func("acpi/piix4/numamem", test_acpi_piix4_tcg_numamem);
>>>> -        qtest_add_func("acpi/q35/numamem", test_acpi_q35_tcg_numamem);
>>>> -        qtest_add_func("acpi/piix4/nosmm", test_acpi_piix4_tcg_nosmm);
>>>> -        qtest_add_func("acpi/piix4/smm-compat",
>>>> -                       test_acpi_piix4_tcg_smm_compat);
>>>> -        qtest_add_func("acpi/piix4/smm-compat-nosmm",
>>>> -                       test_acpi_piix4_tcg_smm_compat_nosmm);
>>>> -        qtest_add_func("acpi/piix4/nohpet", test_acpi_piix4_tcg_nohpet);
>>>> -        qtest_add_func("acpi/q35/nosmm", test_acpi_q35_tcg_nosmm);
>>>> -        qtest_add_func("acpi/q35/smm-compat",
>>>> -                       test_acpi_q35_tcg_smm_compat);
>>>> -        qtest_add_func("acpi/q35/smm-compat-nosmm",
>>>> -                       test_acpi_q35_tcg_smm_compat_nosmm);
>>>> -        qtest_add_func("acpi/q35/nohpet", test_acpi_q35_tcg_nohpet);
>>>> -        qtest_add_func("acpi/piix4/dimmpxm", test_acpi_piix4_tcg_dimm_pxm);
>>>> -        qtest_add_func("acpi/q35/dimmpxm", test_acpi_q35_tcg_dimm_pxm);
>>>> -        qtest_add_func("acpi/piix4/acpihmat", test_acpi_piix4_tcg_acpi_hmat);
>>>> -        qtest_add_func("acpi/q35/acpihmat", test_acpi_q35_tcg_acpi_hmat);
>>>> -        qtest_add_func("acpi/microvm", test_acpi_microvm_tcg);
>>>> -        qtest_add_func("acpi/microvm/usb", test_acpi_microvm_usb_tcg);
>>>> -        qtest_add_func("acpi/microvm/rtc", test_acpi_microvm_rtc_tcg);
>>>> -        qtest_add_func("acpi/microvm/ioapic2", test_acpi_microvm_ioapic2_tcg);
>>>> -        qtest_add_func("acpi/microvm/oem-fields", test_acpi_oem_fields_microvm);
>>>> -        if (strcmp(arch, "x86_64") == 0) {
>>>> -            qtest_add_func("acpi/microvm/pcie", test_acpi_microvm_pcie_tcg);
>>>> +        if (tcg_accel_available) {  
>>>
>>> most of this can run without TCG if KVM is available, so why are you limiting it to TCG only
>>> or am I missing something?  
>>
>> This patch is a simple API change, these tests are already restricted by
>> the 'g_test_skip("TCG disabled, skipping ACPI tcg_only test");' call.
>>
> 
> 
> with current code, assume we have TCG compiled out:
>  - test_acpi_one() will execute any test that is not marked as tcg_only
> 
> with this patch if tcg_accel_available == False,
> it will not even register any test under "if (tcg_accel_available) {" branch
> and in this patch that includes a bunch of _non_ tcg_only tests.
> So tests won't be executed on KVM only build, where previously they were executed just fine.

I guess you refer to the 'data->tcg_only' field, OK, now I understood.

Re: [PATCH v5 02/10] accel: Introduce 'query-accels' QMP command

[Markus Armbruster]

Philippe Mathieu-Daudé <philmd@redhat.com> writes:

> Introduce the 'query-accels' QMP command which returns a list
> of built-in accelerator names.
>
> - Accelerator is a QAPI enum of all existing accelerators,
>
> - AcceleratorInfo is a QAPI structure providing accelerator
>   specific information. Currently the common structure base
>   provides the name of the accelerator, while the specific
>   part is empty, but each accelerator can expand it.
>
> - 'query-accels' QMP command returns a list of @AcceleratorInfo
>
> For example on a KVM-only build we get:
>
>     { "execute": "query-accels" }
>     {
>         "return": [
>             {
>                 "name": "qtest"
>             },
>             {
>                 "name": "kvm"
>             }
>         ]
>     }
>
> Note that we can't make the enum values or union branches conditional
> because of target-specific poisoning of accelerator definitions.

Let me try to complete this argument;

  If we did, enum Accelerator could only be used in target-specific
  code.  But we want to use it in generic code, too.

Which generic code exactly?

> Reviewed-by: Eric Blake <eblake@redhat.com>
> Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
> Tested-by: Alex Bennée <alex.bennee@linaro.org>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Patch looks good to me.

Re: [PATCH v5 02/10] accel: Introduce 'query-accels' QMP command

[Philippe Mathieu-Daudé]

On 5/5/21 9:07 AM, Markus Armbruster wrote:
> Philippe Mathieu-Daudé <philmd@redhat.com> writes:
> 
>> Introduce the 'query-accels' QMP command which returns a list
>> of built-in accelerator names.
>>
>> - Accelerator is a QAPI enum of all existing accelerators,
>>
>> - AcceleratorInfo is a QAPI structure providing accelerator
>>   specific information. Currently the common structure base
>>   provides the name of the accelerator, while the specific
>>   part is empty, but each accelerator can expand it.
>>
>> - 'query-accels' QMP command returns a list of @AcceleratorInfo
>>
>> For example on a KVM-only build we get:
>>
>>     { "execute": "query-accels" }
>>     {
>>         "return": [
>>             {
>>                 "name": "qtest"
>>             },
>>             {
>>                 "name": "kvm"
>>             }
>>         ]
>>     }
>>
>> Note that we can't make the enum values or union branches conditional
>> because of target-specific poisoning of accelerator definitions.
> 
> Let me try to complete this argument;
> 
>   If we did, enum Accelerator could only be used in target-specific
>   code.  But we want to use it in generic code, too.
> 
> Which generic code exactly?


cpu.c:133:#ifdef CONFIG_TCG
hmp-commands-info.hx:271:#if defined(CONFIG_TCG)
monitor/misc.c:324:#ifdef CONFIG_TCG
softmmu/physmem.c:28:#ifdef CONFIG_TCG

and more importantly:

include/exec/cpu-all.h:430:#ifdef CONFIG_TCG
include/exec/cpu-defs.h:77:#if !defined(CONFIG_USER_ONLY) &&
defined(CONFIG_TCG)
include/exec/exec-all.h:25:#ifdef CONFIG_TCG
include/sysemu/cpu-timers.h:27:#ifdef CONFIG_TCG

Thomas might provide more cases, IIRC he audited this recently.

Re: [PATCH v5 02/10] accel: Introduce 'query-accels' QMP command

[Markus Armbruster]

Philippe Mathieu-Daudé <philmd@redhat.com> writes:

> On 5/5/21 9:07 AM, Markus Armbruster wrote:
>> Philippe Mathieu-Daudé <philmd@redhat.com> writes:
>> 
>>> Introduce the 'query-accels' QMP command which returns a list
>>> of built-in accelerator names.
>>>
>>> - Accelerator is a QAPI enum of all existing accelerators,
>>>
>>> - AcceleratorInfo is a QAPI structure providing accelerator
>>>   specific information. Currently the common structure base
>>>   provides the name of the accelerator, while the specific
>>>   part is empty, but each accelerator can expand it.
>>>
>>> - 'query-accels' QMP command returns a list of @AcceleratorInfo
>>>
>>> For example on a KVM-only build we get:
>>>
>>>     { "execute": "query-accels" }
>>>     {
>>>         "return": [
>>>             {
>>>                 "name": "qtest"
>>>             },
>>>             {
>>>                 "name": "kvm"
>>>             }
>>>         ]
>>>     }
>>>
>>> Note that we can't make the enum values or union branches conditional
>>> because of target-specific poisoning of accelerator definitions.
>> 
>> Let me try to complete this argument;
>> 
>>   If we did, enum Accelerator could only be used in target-specific
>>   code.  But we want to use it in generic code, too.
>> 
>> Which generic code exactly?
>
>
> cpu.c:133:#ifdef CONFIG_TCG
> hmp-commands-info.hx:271:#if defined(CONFIG_TCG)
> monitor/misc.c:324:#ifdef CONFIG_TCG
> softmmu/physmem.c:28:#ifdef CONFIG_TCG
>
> and more importantly:
>
> include/exec/cpu-all.h:430:#ifdef CONFIG_TCG
> include/exec/cpu-defs.h:77:#if !defined(CONFIG_USER_ONLY) &&
> defined(CONFIG_TCG)
> include/exec/exec-all.h:25:#ifdef CONFIG_TCG
> include/sysemu/cpu-timers.h:27:#ifdef CONFIG_TCG
>
> Thomas might provide more cases, IIRC he audited this recently.

No need, I'm interested in examples, not a complete list.

Please amend the commit message:

    Note that we can't make the enum values or union branches conditional
    because of target-specific poisoning of accelerator definitions.
    If we did, enum Accelerator could only be used in target-specific
    code.  But we want to also use it in generic code, such as ...

with ... replaced by one or more references to relevant code.

If we expect readers of the code to later wonder why we're not using
QAPI conditionals, then we should add a comment to the QAPI schema,
too.  Not a demand, since I'm not sure what to expect.

Re: [PATCH v5 02/10] accel: Introduce 'query-accels' QMP command

[Philippe Mathieu-Daudé]

On 5/5/21 5:01 PM, Markus Armbruster wrote:
> Philippe Mathieu-Daudé <philmd@redhat.com> writes:
> 
>> On 5/5/21 9:07 AM, Markus Armbruster wrote:
>>> Philippe Mathieu-Daudé <philmd@redhat.com> writes:
>>>
>>>> Introduce the 'query-accels' QMP command which returns a list
>>>> of built-in accelerator names.
>>>>
>>>> - Accelerator is a QAPI enum of all existing accelerators,
>>>>
>>>> - AcceleratorInfo is a QAPI structure providing accelerator
>>>>   specific information. Currently the common structure base
>>>>   provides the name of the accelerator, while the specific
>>>>   part is empty, but each accelerator can expand it.
>>>>
>>>> - 'query-accels' QMP command returns a list of @AcceleratorInfo
>>>>
>>>> For example on a KVM-only build we get:
>>>>
>>>>     { "execute": "query-accels" }
>>>>     {
>>>>         "return": [
>>>>             {
>>>>                 "name": "qtest"
>>>>             },
>>>>             {
>>>>                 "name": "kvm"
>>>>             }
>>>>         ]
>>>>     }
>>>>
>>>> Note that we can't make the enum values or union branches conditional
>>>> because of target-specific poisoning of accelerator definitions.
>>>
>>> Let me try to complete this argument;
>>>
>>>   If we did, enum Accelerator could only be used in target-specific
>>>   code.  But we want to use it in generic code, too.
>>>
>>> Which generic code exactly?
>>
>>
>> cpu.c:133:#ifdef CONFIG_TCG
>> hmp-commands-info.hx:271:#if defined(CONFIG_TCG)
>> monitor/misc.c:324:#ifdef CONFIG_TCG
>> softmmu/physmem.c:28:#ifdef CONFIG_TCG
>>
>> and more importantly:
>>
>> include/exec/cpu-all.h:430:#ifdef CONFIG_TCG
>> include/exec/cpu-defs.h:77:#if !defined(CONFIG_USER_ONLY) &&
>> defined(CONFIG_TCG)
>> include/exec/exec-all.h:25:#ifdef CONFIG_TCG
>> include/sysemu/cpu-timers.h:27:#ifdef CONFIG_TCG
>>
>> Thomas might provide more cases, IIRC he audited this recently.
> 
> No need, I'm interested in examples, not a complete list.
> 
> Please amend the commit message:
> 
>     Note that we can't make the enum values or union branches conditional
>     because of target-specific poisoning of accelerator definitions.
>     If we did, enum Accelerator could only be used in target-specific
>     code.  But we want to also use it in generic code, such as ...
> 
> with ... replaced by one or more references to relevant code.
> 
> If we expect readers of the code to later wonder why we're not using
> QAPI conditionals, then we should add a comment to the QAPI schema,
> too.  Not a demand, since I'm not sure what to expect.

OK, will do in v8.