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

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

[Philippe Mathieu-Daudé]

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.

As a bonus we remove CONFIG_TCG in config_host, to avoid
building unnecessary objects depending on TCG.

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.

Please review,

Phil.

Philippe Mathieu-Daudé (12):
  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/qmp-cmd-test: Make test build-independent from accelerator
  tests/qtest: Do not restrict bios-tables-test to Aarch64 hosts anymore
  tests/meson: Only build softfloat objects if TCG is selected

 meson.build                    |  1 -
 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 +-
 tests/qtest/qmp-cmd-test.c     | 18 +++++--
 MAINTAINERS                    |  1 +
 accel/meson.build              |  2 +-
 tests/qtest/meson.build        |  3 +-
 12 files changed, 221 insertions(+), 95 deletions(-)
 create mode 100644 accel/accel-qmp.c

-- 
2.26.3

[PATCH v4 01/12] 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.

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 MAINTAINERS | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 36055f14c59..d5df4ba7891 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

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

[Thomas Huth]

On 15/04/2021 18.32, Philippe Mathieu-Daudé wrote:
> 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.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>   MAINTAINERS | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 36055f14c59..d5df4ba7891 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
> 

Reviewed-by: Thomas Huth <thuth@redhat.com>

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

[Philippe Mathieu-Daudé]

On 4/15/21 6:32 PM, Philippe Mathieu-Daudé wrote:
> 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.
> 

Forgot to include:
Acked-by: Andrew Jones <drjones@redhat.com>

> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>  MAINTAINERS | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 36055f14c59..d5df4ba7891 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
> 

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

[Alex Bennée]

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

> 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.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

> ---
>  MAINTAINERS | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 36055f14c59..d5df4ba7891 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


-- 
Alex Bennée

[PATCH v4 02/12] 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>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
Since v3: Simplify over-engineered AcceleratorInfo (Markus, kept Eric R-b)
Since v2: @since 6.0 -> 6.1, added note (Eric)
Since v1: 'type' -> 'name' in comments
---
 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..1ef729f6f84 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': [ 'qtest', 'tcg', 'kvm', 'hax', 'hvf', '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

Re: [PATCH v4 02/12] accel: Introduce 'query-accels' QMP command

[Alex Bennée]

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.
>
> Reviewed-by: Eric Blake <eblake@redhat.com>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Tested-by: Alex Bennée <alex.bennee@linaro.org>

-- 
Alex Bennée

Re: [PATCH v4 02/12] accel: Introduce 'query-accels' QMP command

[Eric Blake]

On 4/15/21 11:32 AM, Philippe Mathieu-Daudé wrote:
> 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>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
> Since v3: Simplify over-engineered AcceleratorInfo (Markus, kept Eric R-b)
> Since v2: @since 6.0 -> 6.1, added note (Eric)
> Since v1: 'type' -> 'name' in comments

> +++ 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': [ 'qtest', 'tcg', 'kvm', 'hax', 'hvf', 'whpx', 'xen' ] }

There's no requirement for enums to be in any order, although if the
list is likely to get larger over time, lexicographic order makes it
easier to know where to insert new entries.  Up to you whether it is
worth sorting, and your decision does not invalidate my R-b.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

Re: [PATCH v4 02/12] accel: Introduce 'query-accels' QMP command

[Philippe Mathieu-Daudé]

On 4/30/21 9:03 PM, Eric Blake wrote:
> On 4/15/21 11:32 AM, Philippe Mathieu-Daudé wrote:
>> 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>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>> ---
>> Since v3: Simplify over-engineered AcceleratorInfo (Markus, kept Eric R-b)
>> Since v2: @since 6.0 -> 6.1, added note (Eric)
>> Since v1: 'type' -> 'name' in comments
> 
>> +++ 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': [ 'qtest', 'tcg', 'kvm', 'hax', 'hvf', 'whpx', 'xen' ] }
> 
> There's no requirement for enums to be in any order, although if the
> list is likely to get larger over time, lexicographic order makes it
> easier to know where to insert new entries.  Up to you whether it is
> worth sorting, and your decision does not invalidate my R-b.

OK will do, thanks!

[PATCH v4 03/12] 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>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
Since v2:
- reworded (Eric)
Since v1:
- rename qtest_probe_accel() -> qtest_has_accel()
- run with -machine none before creating QTestState
---
 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

Re: [PATCH v4 03/12] tests/qtest: Add qtest_has_accel() method

[Alex Bennée]

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

> 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>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

-- 
Alex Bennée

[PATCH v4 04/12] 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>
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

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

[Andrew Jones]

On Thu, Apr 15, 2021 at 06:32:56PM +0200, Philippe Mathieu-Daudé wrote:
> Use the recently added generic qtest_has_accel() method to
> check if KVM is available.
> 
> Suggested-by: Claudio Fontana <cfontana@suse.de>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>  tests/qtest/arm-cpu-features.c | 25 +------------------------
>  1 file changed, 1 insertion(+), 24 deletions(-)
>

Reviewed-by: Andrew Jones <drjones@redhat.com>

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

[Alex Bennée]

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

> Use the recently added generic qtest_has_accel() method to
> check if KVM is available.
>
> Suggested-by: Claudio Fontana <cfontana@suse.de>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

-- 
Alex Bennée

[PATCH v4 05/12] 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>
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

Re: [PATCH v4 05/12] qtest/arm-cpu-features: Restrict sve_tests_sve_off_kvm test to KVM

[Andrew Jones]

On Thu, Apr 15, 2021 at 06:32:57PM +0200, Philippe Mathieu-Daudé wrote:
> The sve_tests_sve_off_kvm() test is KVM specific.
> Only run it if KVM is available.
> 
> Suggested-by: Andrew Jones <drjones@redhat.com>
> 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
>

Reviewed-by: Andrew Jones <drjones@redhat.com>

Re: [PATCH v4 05/12] qtest/arm-cpu-features: Restrict sve_tests_sve_off_kvm test to KVM

[Alex Bennée]

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

> The sve_tests_sve_off_kvm() test is KVM specific.
> Only run it if KVM is available.
>
> Suggested-by: Andrew Jones <drjones@redhat.com>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

> ---
>  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();


-- 
Alex Bennée

[PATCH v4 06/12] 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>
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

Re: [PATCH v4 06/12] qtest/arm-cpu-features: Remove TCG fallback to KVM specific tests

[Andrew Jones]

On Thu, Apr 15, 2021 at 06:32:58PM +0200, Philippe Mathieu-Daudé wrote:
> 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>
> 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
>

Reviewed-by: Andrew Jones <drjones@redhat.com>

Re: [PATCH v4 06/12] qtest/arm-cpu-features: Remove TCG fallback to KVM specific tests

[Alex Bennée]

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

> 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>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

-- 
Alex Bennée

[PATCH v4 07/12] 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>
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

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

[Andrew Jones]

On Thu, Apr 15, 2021 at 06:32:59PM +0200, Philippe Mathieu-Daudé wrote:
> 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>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>  tests/qtest/arm-cpu-features.c | 16 +++++++++-------
>  1 file changed, 9 insertions(+), 7 deletions(-)
>

Reviewed-by: Andrew Jones <drjones@redhat.com>

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

[Alex Bennée]

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

> 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>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

-- 
Alex Bennée

[PATCH v4 08/12] 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>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
Since v1: use global tcg_accel_available, call qtest_has_accel() once
---
 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

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

[Alex Bennée]

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

> 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>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
> Since v1: use global tcg_accel_available, call qtest_has_accel() once
> ---
>  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);

I find it odd that we need TCG here (especially on an x86 system) to
test what are essentially device models we use in KVM. On the other hand
it maintains the current effect with less ifdef hacks so:

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>


-- 
Alex Bennée

[PATCH v4 09/12] 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.

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
Cc: David Gibson <david@gibson.dropbear.id.au>
Cc: Greg Kurz <groug@kaod.org>
Cc: Halil Pasic <pasic@linux.ibm.com>
Cc: Cornelia Huck <cohuck@redhat.com>
Cc: Christian Borntraeger <borntraeger@de.ibm.com>
Cc: qemu-ppc@nongnu.org
Cc: qemu-s390x@nongnu.org
---
 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

Re: [PATCH v4 09/12] qtest/migration-test: Skip tests if KVM not builtin on s390x/ppc64

[David Gibson]

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

On Thu, Apr 15, 2021 at 06:33:01PM +0200, Philippe Mathieu-Daudé wrote:
> 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.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

> ---
> Cc: David Gibson <david@gibson.dropbear.id.au>
> Cc: Greg Kurz <groug@kaod.org>
> Cc: Halil Pasic <pasic@linux.ibm.com>
> Cc: Cornelia Huck <cohuck@redhat.com>
> Cc: Christian Borntraeger <borntraeger@de.ibm.com>
> Cc: qemu-ppc@nongnu.org
> Cc: qemu-s390x@nongnu.org
> ---
>  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();
>          }

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v4 09/12] qtest/migration-test: Skip tests if KVM not builtin on s390x/ppc64

[Greg Kurz]

On Thu, 15 Apr 2021 18:33:01 +0200
Philippe Mathieu-Daudé <philmd@redhat.com> wrote:

> 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.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---

LGTM

Reviewed-by: Greg Kurz <groug@kaod.org>

> Cc: David Gibson <david@gibson.dropbear.id.au>
> Cc: Greg Kurz <groug@kaod.org>
> Cc: Halil Pasic <pasic@linux.ibm.com>
> Cc: Cornelia Huck <cohuck@redhat.com>
> Cc: Christian Borntraeger <borntraeger@de.ibm.com>
> Cc: qemu-ppc@nongnu.org
> Cc: qemu-s390x@nongnu.org
> ---
>  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();
>          }

Re: [PATCH v4 09/12] qtest/migration-test: Skip tests if KVM not builtin on s390x/ppc64

[Cornelia Huck]

On Thu, 15 Apr 2021 18:33:01 +0200
Philippe Mathieu-Daudé <philmd@redhat.com> wrote:

> 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.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
> Cc: David Gibson <david@gibson.dropbear.id.au>
> Cc: Greg Kurz <groug@kaod.org>
> Cc: Halil Pasic <pasic@linux.ibm.com>
> Cc: Cornelia Huck <cohuck@redhat.com>
> Cc: Christian Borntraeger <borntraeger@de.ibm.com>
> Cc: qemu-ppc@nongnu.org
> Cc: qemu-s390x@nongnu.org
> ---
>  tests/qtest/migration-test.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

Reviewed-by: Cornelia Huck <cohuck@redhat.com>

Re: [PATCH v4 09/12] qtest/migration-test: Skip tests if KVM not builtin on s390x/ppc64

[Alex Bennée]

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

> 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.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
> Cc: David Gibson <david@gibson.dropbear.id.au>
> Cc: Greg Kurz <groug@kaod.org>
> Cc: Halil Pasic <pasic@linux.ibm.com>
> Cc: Cornelia Huck <cohuck@redhat.com>
> Cc: Christian Borntraeger <borntraeger@de.ibm.com>
> Cc: qemu-ppc@nongnu.org
> Cc: qemu-s390x@nongnu.org
> ---
>  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();
>          }

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

-- 
Alex Bennée

[PATCH v4 10/12] qtest/qmp-cmd-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 a runtime check.

Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 tests/qtest/qmp-cmd-test.c | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/tests/qtest/qmp-cmd-test.c b/tests/qtest/qmp-cmd-test.c
index c98b78d0339..8902d2f169f 100644
--- a/tests/qtest/qmp-cmd-test.c
+++ b/tests/qtest/qmp-cmd-test.c
@@ -21,19 +21,24 @@ const char common_args[] = "-nodefaults -machine none";
 
 /* Query smoke tests */
 
+static bool tcg_accel_available;
+
 static int query_error_class(const char *cmd)
 {
-    static struct {
+    static const struct {
         const char *cmd;
         int err_class;
+        /*
+         * Pointer to boolean.
+         * If non-NULL and value is %true, the error class is skipped.
+         */
+        bool *skip_err_class;
     } fails[] = {
         /* Success depends on build configuration: */
 #ifndef CONFIG_SPICE
         { "query-spice", ERROR_CLASS_COMMAND_NOT_FOUND },
 #endif
-#ifndef CONFIG_TCG
-        { "query-replay", ERROR_CLASS_COMMAND_NOT_FOUND },
-#endif
+        { "query-replay", ERROR_CLASS_COMMAND_NOT_FOUND, &tcg_accel_available },
 #ifndef CONFIG_VNC
         { "query-vnc", ERROR_CLASS_GENERIC_ERROR },
         { "query-vnc-servers", ERROR_CLASS_GENERIC_ERROR },
@@ -51,6 +56,9 @@ static int query_error_class(const char *cmd)
     int i;
 
     for (i = 0; fails[i].cmd; i++) {
+        if (fails[i].skip_err_class && *fails[i].skip_err_class) {
+            continue;
+        }
         if (!strcmp(cmd, fails[i].cmd)) {
             return fails[i].err_class;
         }
@@ -334,6 +342,8 @@ int main(int argc, char *argv[])
     QmpSchema schema;
     int ret;
 
+    tcg_accel_available = qtest_has_accel("tcg");
+
     g_test_init(&argc, &argv, NULL);
 
     qmp_schema_init(&schema);
-- 
2.26.3

Re: [PATCH v4 10/12] qtest/qmp-cmd-test: Make test build-independent from accelerator

[Philippe Mathieu-Daudé]

Hi Thomas, Markus, do you mind reviewing this one please?

On 4/15/21 6:33 PM, Philippe Mathieu-Daudé 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 a runtime check.
> 
> Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>  tests/qtest/qmp-cmd-test.c | 18 ++++++++++++++----
>  1 file changed, 14 insertions(+), 4 deletions(-)
> 
> diff --git a/tests/qtest/qmp-cmd-test.c b/tests/qtest/qmp-cmd-test.c
> index c98b78d0339..8902d2f169f 100644
> --- a/tests/qtest/qmp-cmd-test.c
> +++ b/tests/qtest/qmp-cmd-test.c
> @@ -21,19 +21,24 @@ const char common_args[] = "-nodefaults -machine none";
>  
>  /* Query smoke tests */
>  
> +static bool tcg_accel_available;
> +
>  static int query_error_class(const char *cmd)
>  {
> -    static struct {
> +    static const struct {
>          const char *cmd;
>          int err_class;
> +        /*
> +         * Pointer to boolean.
> +         * If non-NULL and value is %true, the error class is skipped.
> +         */
> +        bool *skip_err_class;
>      } fails[] = {
>          /* Success depends on build configuration: */
>  #ifndef CONFIG_SPICE
>          { "query-spice", ERROR_CLASS_COMMAND_NOT_FOUND },
>  #endif
> -#ifndef CONFIG_TCG
> -        { "query-replay", ERROR_CLASS_COMMAND_NOT_FOUND },
> -#endif
> +        { "query-replay", ERROR_CLASS_COMMAND_NOT_FOUND, &tcg_accel_available },
>  #ifndef CONFIG_VNC
>          { "query-vnc", ERROR_CLASS_GENERIC_ERROR },
>          { "query-vnc-servers", ERROR_CLASS_GENERIC_ERROR },
> @@ -51,6 +56,9 @@ static int query_error_class(const char *cmd)
>      int i;
>  
>      for (i = 0; fails[i].cmd; i++) {
> +        if (fails[i].skip_err_class && *fails[i].skip_err_class) {
> +            continue;
> +        }
>          if (!strcmp(cmd, fails[i].cmd)) {
>              return fails[i].err_class;
>          }
> @@ -334,6 +342,8 @@ int main(int argc, char *argv[])
>      QmpSchema schema;
>      int ret;
>  
> +    tcg_accel_available = qtest_has_accel("tcg");
> +
>      g_test_init(&argc, &argv, NULL);
>  
>      qmp_schema_init(&schema);
> 

Re: [PATCH v4 10/12] qtest/qmp-cmd-test: Make test build-independent from accelerator

[Markus Armbruster]

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

> 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 a runtime check.
>
> Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Please read the last remark first.  The other ones are detail; feel free
to skip them until we're done with the last one.

> ---
>  tests/qtest/qmp-cmd-test.c | 18 ++++++++++++++----
>  1 file changed, 14 insertions(+), 4 deletions(-)
>
> diff --git a/tests/qtest/qmp-cmd-test.c b/tests/qtest/qmp-cmd-test.c
> index c98b78d0339..8902d2f169f 100644
> --- a/tests/qtest/qmp-cmd-test.c
> +++ b/tests/qtest/qmp-cmd-test.c
> @@ -21,19 +21,24 @@ const char common_args[] = "-nodefaults -machine none";
>  
>  /* Query smoke tests */
>  
> +static bool tcg_accel_available;
> +
>  static int query_error_class(const char *cmd)
>  {
> -    static struct {
> +    static const struct {
>          const char *cmd;
>          int err_class;
> +        /*
> +         * Pointer to boolean.

Let's not paraphrase code in comments.

> +         * If non-NULL and value is %true, the error class is skipped.

Suggest "When it points to true, the test case is skipped", and ...

> +         */
> +        bool *skip_err_class;

... name this just @skip.  Or maybe @skip_ptr, because fails[i].skip
reads as if the test case was to be skipped.

>      } fails[] = {
>          /* Success depends on build configuration: */

Note the headline ^^^

>  #ifndef CONFIG_SPICE
>          { "query-spice", ERROR_CLASS_COMMAND_NOT_FOUND },
>  #endif
> -#ifndef CONFIG_TCG
> -        { "query-replay", ERROR_CLASS_COMMAND_NOT_FOUND },
> -#endif
> +        { "query-replay", ERROR_CLASS_COMMAND_NOT_FOUND, &tcg_accel_available },

No longer fits under the headline.  Move it under its own headline,
perhaps

           /* Success depends on dynamic state as reported by other queries */

>  #ifndef CONFIG_VNC
>          { "query-vnc", ERROR_CLASS_GENERIC_ERROR },
>          { "query-vnc-servers", ERROR_CLASS_GENERIC_ERROR },
> @@ -51,6 +56,9 @@ static int query_error_class(const char *cmd)
>      int i;
>  
>      for (i = 0; fails[i].cmd; i++) {
> +        if (fails[i].skip_err_class && *fails[i].skip_err_class) {
> +            continue;
> +        }
>          if (!strcmp(cmd, fails[i].cmd)) {
>              return fails[i].err_class;
>          }
> @@ -334,6 +342,8 @@ int main(int argc, char *argv[])
>      QmpSchema schema;
>      int ret;
>  
> +    tcg_accel_available = qtest_has_accel("tcg");
> +

When does tcg_accel_available differ from defined(CONFIG_TCG)?

>      g_test_init(&argc, &argv, NULL);
>  
>      qmp_schema_init(&schema);

Re: [PATCH v4 10/12] qtest/qmp-cmd-test: Make test build-independent from accelerator

[Philippe Mathieu-Daudé]

On 4/29/21 7:43 AM, Markus Armbruster wrote:
> Philippe Mathieu-Daudé <philmd@redhat.com> writes:
> 
>> 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 a runtime check.
>>
>> Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> 
> Please read the last remark first.  The other ones are detail; feel free
> to skip them until we're done with the last one.
> 
>> ---
>>  tests/qtest/qmp-cmd-test.c | 18 ++++++++++++++----
>>  1 file changed, 14 insertions(+), 4 deletions(-)

>> +    tcg_accel_available = qtest_has_accel("tcg");
>> +
> 
> When does tcg_accel_available differ from defined(CONFIG_TCG)?

qtest_has_accel("tcg") is a runtime check, while defined(CONFIG_TCG)
is build-time.

Having tests doing runtime checking allow to:

- have easier reviews, having less #ifdef'ry
- build them once for all targets
- build them once for all ./configure options
  (thinking about CI, the a single job could build the tests, then
  we run them using the QEMU binaries from other jobs)
- use the same binaries to test the built binary and the distribution
  installed one
- remove the dependencies between tests and binaries

> 
>>      g_test_init(&argc, &argv, NULL);
>>  
>>      qmp_schema_init(&schema);
> 

Re: [PATCH v4 10/12] qtest/qmp-cmd-test: Make test build-independent from accelerator

[Markus Armbruster]

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

> On 4/29/21 7:43 AM, Markus Armbruster wrote:
>> Philippe Mathieu-Daudé <philmd@redhat.com> writes:
>> 
>>> 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 a runtime check.
>>>
>>> Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>> 
>> Please read the last remark first.  The other ones are detail; feel free
>> to skip them until we're done with the last one.
>> 
>>> ---
>>>  tests/qtest/qmp-cmd-test.c | 18 ++++++++++++++----
>>>  1 file changed, 14 insertions(+), 4 deletions(-)
>
>>> +    tcg_accel_available = qtest_has_accel("tcg");
>>> +
>> 
>> When does tcg_accel_available differ from defined(CONFIG_TCG)?
>
> qtest_has_accel("tcg") is a runtime check, while defined(CONFIG_TCG)
> is build-time.

Let me rephrase my question: under what conditions can the values of
qtest_has_accel("tcg") and defined(CONFIG_TCG) differ?

> Having tests doing runtime checking allow to:
>
> - have easier reviews, having less #ifdef'ry
> - build them once for all targets
> - build them once for all ./configure options
>   (thinking about CI, the a single job could build the tests, then
>   we run them using the QEMU binaries from other jobs)
> - use the same binaries to test the built binary and the distribution
>   installed one
> - remove the dependencies between tests and binaries
>
>> 
>>>      g_test_init(&argc, &argv, NULL);
>>>  
>>>      qmp_schema_init(&schema);
>> 

Re: [PATCH v4 10/12] qtest/qmp-cmd-test: Make test build-independent from accelerator

[Philippe Mathieu-Daudé]

On 4/29/21 3:22 PM, Markus Armbruster wrote:
> Philippe Mathieu-Daudé <philmd@redhat.com> writes:
>>>> 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 a runtime check.
>>>>
>>>> Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
>>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>>>
>>> Please read the last remark first.  The other ones are detail; feel free
>>> to skip them until we're done with the last one.
>>>
>>>> ---
>>>>  tests/qtest/qmp-cmd-test.c | 18 ++++++++++++++----
>>>>  1 file changed, 14 insertions(+), 4 deletions(-)
>>
>>>> +    tcg_accel_available = qtest_has_accel("tcg");
>>>> +
>>>
>>> When does tcg_accel_available differ from defined(CONFIG_TCG)?
>>
>> qtest_has_accel("tcg") is a runtime check, while defined(CONFIG_TCG)
>> is build-time.
> 
> Let me rephrase my question: under what conditions can the values of
> qtest_has_accel("tcg") and defined(CONFIG_TCG) differ?

They are currently the same, so this patch is a no-op. *But* it
allows us to remove the global dependence on CONFIG_TCG in the
Meson machinery (see the last commit in this series).

Is that missing part of the commit description?

"This will allow us to remove the global CONFIG_TCG dependency
in our Meson machinery in a pair of commits."?

>> Having tests doing runtime checking allow to:
>>
>> - have easier reviews, having less #ifdef'ry
>> - build them once for all targets
>> - build them once for all ./configure options
>>   (thinking about CI, the a single job could build the tests, then
>>   we run them using the QEMU binaries from other jobs)
>> - use the same binaries to test the built binary and the distribution
>>   installed one
>> - remove the dependencies between tests and binaries
>>
>>>
>>>>      g_test_init(&argc, &argv, NULL);
>>>>  
>>>>      qmp_schema_init(&schema);
>>>
> 

Re: [PATCH v4 10/12] qtest/qmp-cmd-test: Make test build-independent from accelerator

[Markus Armbruster]

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

> On 4/29/21 3:22 PM, Markus Armbruster wrote:
>> Philippe Mathieu-Daudé <philmd@redhat.com> writes:
>>>>> 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 a runtime check.
>>>>>
>>>>> Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
>>>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>>>>
>>>> Please read the last remark first.  The other ones are detail; feel free
>>>> to skip them until we're done with the last one.
>>>>
>>>>> ---
>>>>>  tests/qtest/qmp-cmd-test.c | 18 ++++++++++++++----
>>>>>  1 file changed, 14 insertions(+), 4 deletions(-)
>>>
>>>>> +    tcg_accel_available = qtest_has_accel("tcg");
>>>>> +
>>>>
>>>> When does tcg_accel_available differ from defined(CONFIG_TCG)?
>>>
>>> qtest_has_accel("tcg") is a runtime check, while defined(CONFIG_TCG)
>>> is build-time.
>> 
>> Let me rephrase my question: under what conditions can the values of
>> qtest_has_accel("tcg") and defined(CONFIG_TCG) differ?
>
> They are currently the same, so this patch is a no-op. *But* it
> allows us to remove the global dependence on CONFIG_TCG in the
> Meson machinery (see the last commit in this series).
>
> Is that missing part of the commit description?
>
> "This will allow us to remove the global CONFIG_TCG dependency
> in our Meson machinery in a pair of commits."?

Do you mean "in the next two commits"?

Please also note that the probing at run-time always gives the same
result as the compile-time check it replaces.

I don't understand what exactly the conversion to probing enables and
how, but I believe I don't have to.

[...]

Re: [PATCH v4 10/12] qtest/qmp-cmd-test: Make test build-independent from accelerator

[Philippe Mathieu-Daudé]

On 4/30/21 8:13 AM, Markus Armbruster wrote:
> Philippe Mathieu-Daudé <philmd@redhat.com> writes:
>> On 4/29/21 3:22 PM, Markus Armbruster wrote:
>>> Philippe Mathieu-Daudé <philmd@redhat.com> writes:
>>>>>> 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 a runtime check.
>>>>>>
>>>>>> Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
>>>>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>>>>>
>>>>> Please read the last remark first.  The other ones are detail; feel free
>>>>> to skip them until we're done with the last one.
>>>>>
>>>>>> ---
>>>>>>  tests/qtest/qmp-cmd-test.c | 18 ++++++++++++++----
>>>>>>  1 file changed, 14 insertions(+), 4 deletions(-)
>>>>
>>>>>> +    tcg_accel_available = qtest_has_accel("tcg");
>>>>>> +
>>>>>
>>>>> When does tcg_accel_available differ from defined(CONFIG_TCG)?
>>>>
>>>> qtest_has_accel("tcg") is a runtime check, while defined(CONFIG_TCG)
>>>> is build-time.
>>>
>>> Let me rephrase my question: under what conditions can the values of
>>> qtest_has_accel("tcg") and defined(CONFIG_TCG) differ?
>>
>> They are currently the same, so this patch is a no-op. *But* it
>> allows us to remove the global dependence on CONFIG_TCG in the
>> Meson machinery (see the last commit in this series).
>>
>> Is that missing part of the commit description?
>>
>> "This will allow us to remove the global CONFIG_TCG dependency
>> in our Meson machinery in a pair of commits."?
> 
> Do you mean "in the next two commits"?

Yes.

> Please also note that the probing at run-time always gives the same
> result as the compile-time check it replaces.
> 
> I don't understand what exactly the conversion to probing enables and
> how, but I believe I don't have to.

This series is removing some limitations in qtests to help Claudio work:

x86: https://www.mail-archive.com/qemu-devel@nongnu.org/msg793885.html
arm: https://www.mail-archive.com/qemu-devel@nongnu.org/msg799328.html
s390x: https://www.mail-archive.com/qemu-devel@nongnu.org/msg800254.html

which allow building with different sets of accelerators (and more).

Claudio/Paolo could better explain :)


What I like from feature tested at runtime is we can run qtests using
older binaries, binaries built with different configure flags, or the
binaries installed by the distribution.

Thanks,

Phil.

Re: [PATCH v4 10/12] qtest/qmp-cmd-test: Make test build-independent from accelerator

[Markus Armbruster]

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

> On 4/30/21 8:13 AM, Markus Armbruster wrote:
>> Philippe Mathieu-Daudé <philmd@redhat.com> writes:
>>> On 4/29/21 3:22 PM, Markus Armbruster wrote:
>>>> Philippe Mathieu-Daudé <philmd@redhat.com> writes:
>>>>>>> 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 a runtime check.
>>>>>>>
>>>>>>> Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
>>>>>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>>>>>>
>>>>>> Please read the last remark first.  The other ones are detail; feel free
>>>>>> to skip them until we're done with the last one.
>>>>>>
>>>>>>> ---
>>>>>>>  tests/qtest/qmp-cmd-test.c | 18 ++++++++++++++----
>>>>>>>  1 file changed, 14 insertions(+), 4 deletions(-)
>>>>>
>>>>>>> +    tcg_accel_available = qtest_has_accel("tcg");
>>>>>>> +
>>>>>>
>>>>>> When does tcg_accel_available differ from defined(CONFIG_TCG)?
>>>>>
>>>>> qtest_has_accel("tcg") is a runtime check, while defined(CONFIG_TCG)
>>>>> is build-time.
>>>>
>>>> Let me rephrase my question: under what conditions can the values of
>>>> qtest_has_accel("tcg") and defined(CONFIG_TCG) differ?
>>>
>>> They are currently the same, so this patch is a no-op. *But* it
>>> allows us to remove the global dependence on CONFIG_TCG in the
>>> Meson machinery (see the last commit in this series).
>>>
>>> Is that missing part of the commit description?
>>>
>>> "This will allow us to remove the global CONFIG_TCG dependency
>>> in our Meson machinery in a pair of commits."?
>> 
>> Do you mean "in the next two commits"?
>
> Yes.
>
>> Please also note that the probing at run-time always gives the same
>> result as the compile-time check it replaces.
>> 
>> I don't understand what exactly the conversion to probing enables and
>> how, but I believe I don't have to.
>
> This series is removing some limitations in qtests to help Claudio work:
>
> x86: https://www.mail-archive.com/qemu-devel@nongnu.org/msg793885.html
> arm: https://www.mail-archive.com/qemu-devel@nongnu.org/msg799328.html
> s390x: https://www.mail-archive.com/qemu-devel@nongnu.org/msg800254.html
>
> which allow building with different sets of accelerators (and more).
>
> Claudio/Paolo could better explain :)

Will this lead to different values of qtest_has_accel("tcg") and
defined(CONFIG_TCG) within a single build tree?

> What I like from feature tested at runtime is we can run qtests using
> older binaries, binaries built with different configure flags, or the
> binaries installed by the distribution.

Running with binaries build from a different HEAD seems unlikely to be
useful.  Any failures are just as likely to be caused bu the version
mismatch as by actual bugs.  Binaries for the same HEAD built for
another configuration can be made to work (this patch doesn't yet
suffice), but why should I care?

What I don't like is yet another moving part.

I'm not sure this is a good trade.  Quite possibly because I still don't
fully understand what we're trying to accomplish here :)

[PATCH v4 11/12] tests/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>
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 v4 11/12] tests/qtest: Do not restrict bios-tables-test to Aarch64 hosts anymore

[Alex Bennée]

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

> 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>
> 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

Yeah this one has been fixed for some time I reckon. Once I fixed my
cross compile detection scripts I was able to run them all with 32 bit
ARM binaries.

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Tested-by: Alex Bennée <alex.bennee@linaro.org>


>  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']


-- 
Alex Bennée

[PATCH v4 12/12] tests/meson: Only build softfloat objects if TCG is selected

[Philippe Mathieu-Daudé]

From: Philippe Mathieu-Daudé <f4bug@amsat.org>

The previous attempt (commit f77147cd4de) doesn't work as
expected, as we still have CONFIG_TCG=1 when using:

  configure --disable-system --disable-user

Now than we have removed the use of CONFIG_TCG from target-dependent
files in tests/qtest/, we can remove the unconditional definition of
CONFIG_TCG in config_host.

This avoid to build a bunch of unrequired objects when building with
--disable-tcg (in particular the softfloat tests):

Before:

  $ make
  [1/812] Generating trace-qom.h with a custom command
  ...

After:

  $ make
  [1/349] Generating trace-qom.h with a custom command
  ...

A difference of 463 objects...

Reported-by: Claudio Fontana <cfontana@suse.de>
Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
v3: Include Paolo's feedback:
https://www.mail-archive.com/qemu-devel@nongnu.org/msg793872.html
therefore o not include Alex's R-b tag.

Cc: Richard Henderson <richard.henderson@linaro.org>
Cc: Alex Bennée <alex.bennee@linaro.org>
Cc: Emilio G. Cota <cota@braap.org>
---
 meson.build | 1 -
 1 file changed, 1 deletion(-)

diff --git a/meson.build b/meson.build
index c6f4b0cf5e8..623cbe50685 100644
--- a/meson.build
+++ b/meson.build
@@ -262,7 +262,6 @@
                         language: ['c', 'cpp', 'objc'])
 
   accelerators += 'CONFIG_TCG'
-  config_host += { 'CONFIG_TCG': 'y' }
 endif
 
 if 'CONFIG_KVM' not in accelerators and get_option('kvm').enabled()
-- 
2.26.3

Re: [PATCH v4 12/12] tests/meson: Only build softfloat objects if TCG is selected

[Philippe Mathieu-Daudé]

Alex, Richard, do you mind reviewing this one please?

On 4/15/21 6:33 PM, Philippe Mathieu-Daudé wrote:
> From: Philippe Mathieu-Daudé <f4bug@amsat.org>
> 
> The previous attempt (commit f77147cd4de) doesn't work as
> expected, as we still have CONFIG_TCG=1 when using:
> 
>   configure --disable-system --disable-user
> 
> Now than we have removed the use of CONFIG_TCG from target-dependent
> files in tests/qtest/, we can remove the unconditional definition of
> CONFIG_TCG in config_host.
> 
> This avoid to build a bunch of unrequired objects when building with
> --disable-tcg (in particular the softfloat tests):
> 
> Before:
> 
>   $ make
>   [1/812] Generating trace-qom.h with a custom command
>   ...
> 
> After:
> 
>   $ make
>   [1/349] Generating trace-qom.h with a custom command
>   ...
> 
> A difference of 463 objects...
> 
> Reported-by: Claudio Fontana <cfontana@suse.de>
> Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> v3: Include Paolo's feedback:
> https://www.mail-archive.com/qemu-devel@nongnu.org/msg793872.html
> therefore o not include Alex's R-b tag.
> 
> Cc: Richard Henderson <richard.henderson@linaro.org>
> Cc: Alex Bennée <alex.bennee@linaro.org>
> Cc: Emilio G. Cota <cota@braap.org>
> ---
>  meson.build | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/meson.build b/meson.build
> index c6f4b0cf5e8..623cbe50685 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -262,7 +262,6 @@
>                          language: ['c', 'cpp', 'objc'])
>  
>    accelerators += 'CONFIG_TCG'
> -  config_host += { 'CONFIG_TCG': 'y' }
>  endif
>  
>  if 'CONFIG_KVM' not in accelerators and get_option('kvm').enabled()
> 

Re: [PATCH v4 12/12] tests/meson: Only build softfloat objects if TCG is selected

[Alex Bennée]

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

> Alex, Richard, do you mind reviewing this one please?

Isn't it already merged (with my r-b tag no less ;-)

  f77147cd4de8c726f89b2702f7a9d0c9711d8875
  Author:     Philippe Mathieu-Daudé <philmd@redhat.com>
  AuthorDate: Fri Jan 22 21:44:31 2021 +0100
  Commit:     Paolo Bonzini <pbonzini@redhat.com>
  CommitDate: Mon Feb 8 14:43:55 2021 +0100

>
> On 4/15/21 6:33 PM, Philippe Mathieu-Daudé wrote:
>> From: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> 
>> The previous attempt (commit f77147cd4de) doesn't work as
>> expected, as we still have CONFIG_TCG=1 when using:
>> 
>>   configure --disable-system --disable-user
>> 
>> Now than we have removed the use of CONFIG_TCG from target-dependent
>> files in tests/qtest/, we can remove the unconditional definition of
>> CONFIG_TCG in config_host.
>> 
>> This avoid to build a bunch of unrequired objects when building with
>> --disable-tcg (in particular the softfloat tests):
>> 
>> Before:
>> 
>>   $ make
>>   [1/812] Generating trace-qom.h with a custom command
>>   ...
>> 
>> After:
>> 
>>   $ make
>>   [1/349] Generating trace-qom.h with a custom command
>>   ...
>> 
>> A difference of 463 objects...
>> 
>> Reported-by: Claudio Fontana <cfontana@suse.de>
>> Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> ---
>> v3: Include Paolo's feedback:
>> https://www.mail-archive.com/qemu-devel@nongnu.org/msg793872.html
>> therefore o not include Alex's R-b tag.
>> 
>> Cc: Richard Henderson <richard.henderson@linaro.org>
>> Cc: Alex Bennée <alex.bennee@linaro.org>
>> Cc: Emilio G. Cota <cota@braap.org>
>> ---
>>  meson.build | 1 -
>>  1 file changed, 1 deletion(-)
>> 
>> diff --git a/meson.build b/meson.build
>> index c6f4b0cf5e8..623cbe50685 100644
>> --- a/meson.build
>> +++ b/meson.build
>> @@ -262,7 +262,6 @@
>>                          language: ['c', 'cpp', 'objc'])
>>  
>>    accelerators += 'CONFIG_TCG'
>> -  config_host += { 'CONFIG_TCG': 'y' }
>>  endif
>>  
>>  if 'CONFIG_KVM' not in accelerators and get_option('kvm').enabled()
>> 


-- 
Alex Bennée

Re: [PATCH v4 12/12] tests/meson: Only build softfloat objects if TCG is selected

[Philippe Mathieu-Daudé]

On 4/28/21 7:06 PM, Alex Bennée wrote:
> Philippe Mathieu-Daudé <philmd@redhat.com> writes:
> 
>> Alex, Richard, do you mind reviewing this one please?
> 
> Isn't it already merged (with my r-b tag no less ;-)
> 
>   f77147cd4de8c726f89b2702f7a9d0c9711d8875

See ...

>   Author:     Philippe Mathieu-Daudé <philmd@redhat.com>
>   AuthorDate: Fri Jan 22 21:44:31 2021 +0100
>   Commit:     Paolo Bonzini <pbonzini@redhat.com>
>   CommitDate: Mon Feb 8 14:43:55 2021 +0100
> 
>>
>> On 4/15/21 6:33 PM, Philippe Mathieu-Daudé wrote:
>>> From: Philippe Mathieu-Daudé <f4bug@amsat.org>
>>>
>>> The previous attempt (commit f77147cd4de) doesn't work as

... ^ this comment :(

>>> expected, as we still have CONFIG_TCG=1 when using:
>>>
>>>   configure --disable-system --disable-user
>>>
>>> Now than we have removed the use of CONFIG_TCG from target-dependent
>>> files in tests/qtest/, we can remove the unconditional definition of
>>> CONFIG_TCG in config_host.
>>>
>>> This avoid to build a bunch of unrequired objects when building with
>>> --disable-tcg (in particular the softfloat tests):
>>>
>>> Before:
>>>
>>>   $ make
>>>   [1/812] Generating trace-qom.h with a custom command
>>>   ...
>>>
>>> After:
>>>
>>>   $ make
>>>   [1/349] Generating trace-qom.h with a custom command
>>>   ...
>>>
>>> A difference of 463 objects...
>>>
>>> Reported-by: Claudio Fontana <cfontana@suse.de>
>>> Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
>>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>>> ---
>>> v3: Include Paolo's feedback:
>>> https://www.mail-archive.com/qemu-devel@nongnu.org/msg793872.html
>>> therefore o not include Alex's R-b tag.
>>>
>>> Cc: Richard Henderson <richard.henderson@linaro.org>
>>> Cc: Alex Bennée <alex.bennee@linaro.org>
>>> Cc: Emilio G. Cota <cota@braap.org>
>>> ---
>>>  meson.build | 1 -
>>>  1 file changed, 1 deletion(-)
>>>
>>> diff --git a/meson.build b/meson.build
>>> index c6f4b0cf5e8..623cbe50685 100644
>>> --- a/meson.build
>>> +++ b/meson.build
>>> @@ -262,7 +262,6 @@
>>>                          language: ['c', 'cpp', 'objc'])
>>>  
>>>    accelerators += 'CONFIG_TCG'
>>> -  config_host += { 'CONFIG_TCG': 'y' }
>>>  endif
>>>  
>>>  if 'CONFIG_KVM' not in accelerators and get_option('kvm').enabled()
>>>
> 
> 

Re: [PATCH v4 12/12] tests/meson: Only build softfloat objects if TCG is selected

[Alex Bennée]

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

> On 4/28/21 7:06 PM, Alex Bennée wrote:
>> Philippe Mathieu-Daudé <philmd@redhat.com> writes:
>> 
>>> Alex, Richard, do you mind reviewing this one please?
>> 
>> Isn't it already merged (with my r-b tag no less ;-)
>> 
>>   f77147cd4de8c726f89b2702f7a9d0c9711d8875
>
> See ...
>
>>   Author:     Philippe Mathieu-Daudé <philmd@redhat.com>
>>   AuthorDate: Fri Jan 22 21:44:31 2021 +0100
>>   Commit:     Paolo Bonzini <pbonzini@redhat.com>
>>   CommitDate: Mon Feb 8 14:43:55 2021 +0100
>> 
>>>
>>> On 4/15/21 6:33 PM, Philippe Mathieu-Daudé wrote:
>>>> From: Philippe Mathieu-Daudé <f4bug@amsat.org>
>>>>
>>>> The previous attempt (commit f77147cd4de) doesn't work as
>
> ... ^ this comment :(

Ahh - my tooling was confused having searched by the subject title ;-)

>
>>>> expected, as we still have CONFIG_TCG=1 when using:
>>>>
>>>>   configure --disable-system --disable-user
>>>>
>>>> Now than we have removed the use of CONFIG_TCG from target-dependent
>>>> files in tests/qtest/, we can remove the unconditional definition of
>>>> CONFIG_TCG in config_host.
>>>>
>>>> This avoid to build a bunch of unrequired objects when building with
>>>> --disable-tcg (in particular the softfloat tests):
>>>>
>>>> Before:
>>>>
>>>>   $ make
>>>>   [1/812] Generating trace-qom.h with a custom command
>>>>   ...
>>>>
>>>> After:
>>>>
>>>>   $ make
>>>>   [1/349] Generating trace-qom.h with a custom command
>>>>   ...
>>>>
>>>> A difference of 463 objects...
>>>>
>>>> Reported-by: Claudio Fontana <cfontana@suse.de>
>>>> Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
>>>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>>>> ---
>>>> v3: Include Paolo's feedback:
>>>> https://www.mail-archive.com/qemu-devel@nongnu.org/msg793872.html
>>>> therefore o not include Alex's R-b tag.
>>>>
>>>> Cc: Richard Henderson <richard.henderson@linaro.org>
>>>> Cc: Alex Bennée <alex.bennee@linaro.org>
>>>> Cc: Emilio G. Cota <cota@braap.org>
>>>> ---
>>>>  meson.build | 1 -
>>>>  1 file changed, 1 deletion(-)
>>>>
>>>> diff --git a/meson.build b/meson.build
>>>> index c6f4b0cf5e8..623cbe50685 100644
>>>> --- a/meson.build
>>>> +++ b/meson.build
>>>> @@ -262,7 +262,6 @@
>>>>                          language: ['c', 'cpp', 'objc'])
>>>>  
>>>>    accelerators += 'CONFIG_TCG'
>>>> -  config_host += { 'CONFIG_TCG': 'y' }
>>>>  endif
>>>>  
>>>>  if 'CONFIG_KVM' not in accelerators and get_option('kvm').enabled()
>>>>
>> 
>> 


-- 
Alex Bennée

Re: [PATCH v4 12/12] tests/meson: Only build softfloat objects if TCG is selected

[Philippe Mathieu-Daudé]

On 4/28/21 9:04 PM, Alex Bennée wrote:
> 
> Philippe Mathieu-Daudé <philmd@redhat.com> writes:
> 
>> On 4/28/21 7:06 PM, Alex Bennée wrote:
>>> Philippe Mathieu-Daudé <philmd@redhat.com> writes:
>>>
>>>> Alex, Richard, do you mind reviewing this one please?
>>>
>>> Isn't it already merged (with my r-b tag no less ;-)
>>>
>>>   f77147cd4de8c726f89b2702f7a9d0c9711d8875
>>
>> See ...
>>
>>>   Author:     Philippe Mathieu-Daudé <philmd@redhat.com>
>>>   AuthorDate: Fri Jan 22 21:44:31 2021 +0100
>>>   Commit:     Paolo Bonzini <pbonzini@redhat.com>
>>>   CommitDate: Mon Feb 8 14:43:55 2021 +0100
>>>
>>>>
>>>> On 4/15/21 6:33 PM, Philippe Mathieu-Daudé wrote:
>>>>> From: Philippe Mathieu-Daudé <f4bug@amsat.org>
>>>>>
>>>>> The previous attempt (commit f77147cd4de) doesn't work as
>>
>> ... ^ this comment :(
> 
> Ahh - my tooling was confused having searched by the subject title ;-)

Oh I see, I'll rename as:
"tests/meson: Only build softfloat objects if TCG is selected (again)".

Re: [PATCH v4 12/12] tests/meson: Only build softfloat objects if TCG is selected

[Philippe Mathieu-Daudé]

On 4/28/21 9:12 PM, Philippe Mathieu-Daudé wrote:
> On 4/28/21 9:04 PM, Alex Bennée wrote:
>>
>> Philippe Mathieu-Daudé <philmd@redhat.com> writes:
>>
>>> On 4/28/21 7:06 PM, Alex Bennée wrote:
>>>> Philippe Mathieu-Daudé <philmd@redhat.com> writes:
>>>>
>>>>> Alex, Richard, do you mind reviewing this one please?
>>>>
>>>> Isn't it already merged (with my r-b tag no less ;-)
>>>>
>>>>   f77147cd4de8c726f89b2702f7a9d0c9711d8875
>>>
>>> See ...
>>>
>>>>   Author:     Philippe Mathieu-Daudé <philmd@redhat.com>
>>>>   AuthorDate: Fri Jan 22 21:44:31 2021 +0100
>>>>   Commit:     Paolo Bonzini <pbonzini@redhat.com>
>>>>   CommitDate: Mon Feb 8 14:43:55 2021 +0100
>>>>
>>>>>
>>>>> On 4/15/21 6:33 PM, Philippe Mathieu-Daudé wrote:
>>>>>> From: Philippe Mathieu-Daudé <f4bug@amsat.org>
>>>>>>
>>>>>> The previous attempt (commit f77147cd4de) doesn't work as
>>>
>>> ... ^ this comment :(
>>
>> Ahh - my tooling was confused having searched by the subject title ;-)
> 
> Oh I see, I'll rename as:
> "tests/meson: Only build softfloat objects if TCG is selected (again)".

BTW you indeed reviewed/tested v1 of this patch:
https://www.mail-archive.com/qemu-devel@nongnu.org/msg793900.html
but after addressing Paolo's comments I preferred to remove your tags.

Re: [PATCH v4 12/12] tests/meson: Only build softfloat objects if TCG is selected

[Alex Bennée]

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

> From: Philippe Mathieu-Daudé <f4bug@amsat.org>
>
> The previous attempt (commit f77147cd4de) doesn't work as
> expected, as we still have CONFIG_TCG=1 when using:
>
>   configure --disable-system --disable-user
>
> Now than we have removed the use of CONFIG_TCG from target-dependent
> files in tests/qtest/, we can remove the unconditional definition of
> CONFIG_TCG in config_host.
>
> This avoid to build a bunch of unrequired objects when building with
> --disable-tcg (in particular the softfloat tests):

Hmm something else gets broken because I bisected a check-tcg failure to
this commit:

  timeout --foreground 15  /home/alex.bennee/lsrc/qemu.git/builds/bisect/qemu-system-aarch64 -monitor none -display none -chardev file,path=memory-record.out,id=output -icount shift=5,rr=record,rrfile=record.bin  -M virt -cpu max -display none -semihosting-config enable=on,target=native,chardev=output -kernel memory
  qemu-system-aarch64: Please enable icount to use record/replay
  /home/alex.bennee/lsrc/qemu.git/tests/tcg/aarch64/Makefile.softmmu-target:49: recipe for target 'run-memory-record' failed
  make[2]: *** [run-memory-record] Error 1
  make[2]: Leaving directory '/home/alex.bennee/lsrc/qemu.git/builds/bisect/tests/tcg/aarch64-softmmu'
  /home/alex.bennee/lsrc/qemu.git/tests/tcg/Makefile.qemu:102: recipe for target 'run-guest-tests' failed
  make[1]: *** [run-guest-tests] Error 2
  make[1]: Leaving directory '/home/alex.bennee/lsrc/qemu.git/builds/bisect'
  /home/alex.bennee/lsrc/qemu.git/tests/Makefile.include:63: recipe for target 'run-tcg-tests-aarch64-softmmu' failed
  make: *** [run-tcg-tests-aarch64-softmmu] Error 2

So it looks like it breaks icount somehow.

>
> Before:
>
>   $ make
>   [1/812] Generating trace-qom.h with a custom command
>   ...
>
> After:
>
>   $ make
>   [1/349] Generating trace-qom.h with a custom command
>   ...
>
> A difference of 463 objects...
>
> Reported-by: Claudio Fontana <cfontana@suse.de>
> Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> v3: Include Paolo's feedback:
> https://www.mail-archive.com/qemu-devel@nongnu.org/msg793872.html
> therefore o not include Alex's R-b tag.
>
> Cc: Richard Henderson <richard.henderson@linaro.org>
> Cc: Alex Bennée <alex.bennee@linaro.org>
> Cc: Emilio G. Cota <cota@braap.org>
> ---
>  meson.build | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/meson.build b/meson.build
> index c6f4b0cf5e8..623cbe50685 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -262,7 +262,6 @@
>                          language: ['c', 'cpp', 'objc'])
>  
>    accelerators += 'CONFIG_TCG'
> -  config_host += { 'CONFIG_TCG': 'y' }
>  endif
>  
>  if 'CONFIG_KVM' not in accelerators and get_option('kvm').enabled()


-- 
Alex Bennée