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

Philippe Mathieu-Daudé posted 12 patches 4 years, 8 months ago
Maintainers: Thomas Huth <thuth@redhat.com>, Cornelia Huck <cohuck@redhat.com>
[PATCH v8 02/12] accel: Introduce 'query-accels' QMP command
Posted by Philippe Mathieu-Daudé 4 years, 8 months ago
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"
            }
        ]
    }

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>
---
v8:
- Include code snippet from Markus adding to machine-target.json
  to be able to use enum values or union branches conditional.
- Use accel_find() on enum to be sure the accelerator is enabled
  at runtime (chat with jsnow / eblake).

Cc: Eric Blake <eblake@redhat.com>
Cc: John Snow <jsnow@redhat.com>
Cc: Markus Armbruster <armbru@redhat.com>
---
 qapi/machine-target.json | 54 ++++++++++++++++++++++++++++++++++++++++
 accel/accel-qmp.c        | 32 ++++++++++++++++++++++++
 accel/meson.build        |  2 +-
 3 files changed, 87 insertions(+), 1 deletion(-)
 create mode 100644 accel/accel-qmp.c

diff --git a/qapi/machine-target.json b/qapi/machine-target.json
index e7811654b72..586a61b5d99 100644
--- a/qapi/machine-target.json
+++ b/qapi/machine-target.json
@@ -329,3 +329,57 @@
 ##
 { 'command': 'query-cpu-definitions', 'returns': ['CpuDefinitionInfo'],
   'if': 'defined(TARGET_PPC) || defined(TARGET_ARM) || defined(TARGET_I386) || defined(TARGET_S390X) || defined(TARGET_MIPS)' }
+
+##
+# @Accelerator:
+#
+# An enumeration of accelerator names.
+#
+# Since: 6.1
+##
+{ 'enum': 'Accelerator',
+  'data': [
+      { 'name': 'hax', 'if': 'defined(CONFIG_HAX)' },
+      { 'name': 'hvf', 'if': 'defined(CONFIG_HVF)' },
+      { 'name': 'kvm', 'if': 'defined(CONFIG_KVM)' },
+      { 'name': 'qtest' },
+      { 'name': 'tcg', 'if': 'defined(CONFIG_TCG)' },
+      { 'name': 'whpx', 'if': 'defined(CONFIG_WHPX)' },
+      { 'name': 'xen', 'if': 'defined(CONFIG_XEN_BACKEND)' } ] }
+
+##
+# @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..0098297caa5
--- /dev/null
+++ b/accel/accel-qmp.c
@@ -0,0 +1,32 @@
+/*
+ * QEMU accelerators, QMP commands
+ *
+ * Copyright (c) 2021 Red Hat Inc.
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/accel.h"
+#include "qapi/qapi-types-machine-target.h"
+#include "qapi/qapi-commands-machine-target.h"
+
+AcceleratorInfoList *qmp_query_accels(Error **errp)
+{
+    AcceleratorInfoList *list = NULL, **tail = &list;
+
+    for (Accelerator accel = 0; accel < ACCELERATOR__MAX; accel++) {
+        AcceleratorInfo *info;
+
+        if (!accel_find(Accelerator_str(accel))) {
+            /* Accelerator available at build time but not at runtime. */
+            continue;
+        }
+
+        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 v8 02/12] accel: Introduce 'query-accels' QMP command
Posted by John Snow 4 years, 8 months ago
On 5/26/21 1:04 PM, 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"
>              }
>          ]
>      }
> 
> 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>
> ---
> v8:
> - Include code snippet from Markus adding to machine-target.json
>    to be able to use enum values or union branches conditional.
> - Use accel_find() on enum to be sure the accelerator is enabled
>    at runtime (chat with jsnow / eblake).
> 

Hi Phil -- Unfortunately I think I am going to defer on this one until 
Markus is back. I need to chat with him about the right way to design 
this, since I'm also not entirely clear on it myself.

--js

> Cc: Eric Blake <eblake@redhat.com>
> Cc: John Snow <jsnow@redhat.com>
> Cc: Markus Armbruster <armbru@redhat.com>


Re: [PATCH v8 02/12] accel: Introduce 'query-accels' QMP command
Posted by Philippe Mathieu-Daudé 4 years, 8 months ago
On 6/3/21 1:26 AM, John Snow wrote:
> On 5/26/21 1:04 PM, 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"
>>              }
>>          ]
>>      }
>>
>> 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>
>> ---
>> v8:
>> - Include code snippet from Markus adding to machine-target.json
>>    to be able to use enum values or union branches conditional.
>> - Use accel_find() on enum to be sure the accelerator is enabled
>>    at runtime (chat with jsnow / eblake).
>>
> 
> Hi Phil -- Unfortunately I think I am going to defer on this one until
> Markus is back. I need to chat with him about the right way to design
> this, since I'm also not entirely clear on it myself.

OK, thanks for the update :)


Re: [PATCH v8 02/12] accel: Introduce 'query-accels' QMP command
Posted by Alex Bennée 4 years, 8 months ago
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"
>             }
>         ]
>     }
>
> 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>
> ---
> v8:
> - Include code snippet from Markus adding to machine-target.json
>   to be able to use enum values or union branches conditional.
> - Use accel_find() on enum to be sure the accelerator is enabled
>   at runtime (chat with jsnow / eblake).

Hmm something broke because now I get:

 /usr/lib/x86_64-linux-gnu/libpixman-1.so -lgthread-2.0 -lglib-2.0 -lstdc++ -Wl,--end-group
/usr/bin/ld: libqemu-aarch64_be-linux-user.fa.p/accel_accel-qmp.c.o: in function `qmp_query_accels':
/home/alex/lsrc/qemu.git/builds/arm.all/../../accel/accel-qmp.c:15: undefined reference to `Accelerator_lookup'
collect2: error: ld returned 1 exit status
[1327/1413] Linking target qemu-io

-- 
Alex Bennée

Re: [PATCH v8 02/12] accel: Introduce 'query-accels' QMP command
Posted by Philippe Mathieu-Daudé 4 years, 8 months ago
On 6/3/21 7:19 PM, Alex Bennée 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"
>>             }
>>         ]
>>     }
>>
>> 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>
>> ---
>> v8:
>> - Include code snippet from Markus adding to machine-target.json
>>   to be able to use enum values or union branches conditional.
>> - Use accel_find() on enum to be sure the accelerator is enabled
>>   at runtime (chat with jsnow / eblake).
> 
> Hmm something broke because now I get:
> 
>  /usr/lib/x86_64-linux-gnu/libpixman-1.so -lgthread-2.0 -lglib-2.0 -lstdc++ -Wl,--end-group
> /usr/bin/ld: libqemu-aarch64_be-linux-user.fa.p/accel_accel-qmp.c.o: in function `qmp_query_accels':
> /home/alex/lsrc/qemu.git/builds/arm.all/../../accel/accel-qmp.c:15: undefined reference to `Accelerator_lookup'
> collect2: error: ld returned 1 exit status
> [1327/1413] Linking target qemu-io

Sorry I missed that for user-mode, will be fixed in v9.


Re: [PATCH v8 02/12] accel: Introduce 'query-accels' QMP command
Posted by Thomas Huth 4 years, 8 months ago
On 26/05/2021 19.04, 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"
>              }
>          ]
>      }
> 
> 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>
> ---
> v8:
> - Include code snippet from Markus adding to machine-target.json
>    to be able to use enum values or union branches conditional.
> - Use accel_find() on enum to be sure the accelerator is enabled
>    at runtime (chat with jsnow / eblake).
> 
> Cc: Eric Blake <eblake@redhat.com>
> Cc: John Snow <jsnow@redhat.com>
> Cc: Markus Armbruster <armbru@redhat.com>
> ---
>   qapi/machine-target.json | 54 ++++++++++++++++++++++++++++++++++++++++
>   accel/accel-qmp.c        | 32 ++++++++++++++++++++++++
>   accel/meson.build        |  2 +-
>   3 files changed, 87 insertions(+), 1 deletion(-)
>   create mode 100644 accel/accel-qmp.c
> 
> diff --git a/qapi/machine-target.json b/qapi/machine-target.json
> index e7811654b72..586a61b5d99 100644
> --- a/qapi/machine-target.json
> +++ b/qapi/machine-target.json
> @@ -329,3 +329,57 @@
>   ##
>   { 'command': 'query-cpu-definitions', 'returns': ['CpuDefinitionInfo'],
>     'if': 'defined(TARGET_PPC) || defined(TARGET_ARM) || defined(TARGET_I386) || defined(TARGET_S390X) || defined(TARGET_MIPS)' }
> +
> +##
> +# @Accelerator:
> +#
> +# An enumeration of accelerator names.
> +#
> +# Since: 6.1
> +##
> +{ 'enum': 'Accelerator',
> +  'data': [
> +      { 'name': 'hax', 'if': 'defined(CONFIG_HAX)' },
> +      { 'name': 'hvf', 'if': 'defined(CONFIG_HVF)' },
> +      { 'name': 'kvm', 'if': 'defined(CONFIG_KVM)' },
> +      { 'name': 'qtest' },
> +      { 'name': 'tcg', 'if': 'defined(CONFIG_TCG)' },
> +      { 'name': 'whpx', 'if': 'defined(CONFIG_WHPX)' },
> +      { 'name': 'xen', 'if': 'defined(CONFIG_XEN_BACKEND)' } ] }
> +
> +##
> +# @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'] }

What about Markus' comment here:

  https://lore.kernel.org/qemu-devel/87mtsoieyz.fsf@dusky.pond.sub.org/

?

If I've got him right, you don't need the command at all, the Accelerator
enum should be sufficient?

  Thomas
  


Re: [PATCH v8 02/12] accel: Introduce 'query-accels' QMP command
Posted by Philippe Mathieu-Daudé 4 years, 8 months ago
On 6/8/21 3:27 PM, Thomas Huth wrote:
> On 26/05/2021 19.04, 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"
>>              }
>>          ]
>>      }
>>
>> 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>
>> ---
>> v8:
>> - Include code snippet from Markus adding to machine-target.json
>>    to be able to use enum values or union branches conditional.
>> - Use accel_find() on enum to be sure the accelerator is enabled
>>    at runtime (chat with jsnow / eblake).
>>
>> Cc: Eric Blake <eblake@redhat.com>
>> Cc: John Snow <jsnow@redhat.com>
>> Cc: Markus Armbruster <armbru@redhat.com>
>> ---
>>   qapi/machine-target.json | 54 ++++++++++++++++++++++++++++++++++++++++
>>   accel/accel-qmp.c        | 32 ++++++++++++++++++++++++
>>   accel/meson.build        |  2 +-
>>   3 files changed, 87 insertions(+), 1 deletion(-)
>>   create mode 100644 accel/accel-qmp.c
>>
>> diff --git a/qapi/machine-target.json b/qapi/machine-target.json
>> index e7811654b72..586a61b5d99 100644
>> --- a/qapi/machine-target.json
>> +++ b/qapi/machine-target.json
>> @@ -329,3 +329,57 @@
>>   ##
>>   { 'command': 'query-cpu-definitions', 'returns': ['CpuDefinitionInfo'],
>>     'if': 'defined(TARGET_PPC) || defined(TARGET_ARM) ||
>> defined(TARGET_I386) || defined(TARGET_S390X) || defined(TARGET_MIPS)' }
>> +
>> +##
>> +# @Accelerator:
>> +#
>> +# An enumeration of accelerator names.
>> +#
>> +# Since: 6.1
>> +##
>> +{ 'enum': 'Accelerator',
>> +  'data': [
>> +      { 'name': 'hax', 'if': 'defined(CONFIG_HAX)' },
>> +      { 'name': 'hvf', 'if': 'defined(CONFIG_HVF)' },
>> +      { 'name': 'kvm', 'if': 'defined(CONFIG_KVM)' },
>> +      { 'name': 'qtest' },
>> +      { 'name': 'tcg', 'if': 'defined(CONFIG_TCG)' },
>> +      { 'name': 'whpx', 'if': 'defined(CONFIG_WHPX)' },
>> +      { 'name': 'xen', 'if': 'defined(CONFIG_XEN_BACKEND)' } ] }
>> +
>> +##
>> +# @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'] }
> 
> What about Markus' comment here:
> 
>  https://lore.kernel.org/qemu-devel/87mtsoieyz.fsf@dusky.pond.sub.org/
> 
> ?
> 
> If I've got him right, you don't need the command at all, the Accelerator
> enum should be sufficient?

Yes, this is the part jsnow said "we are waiting for Markus to comment"
on the other thread ;) We'd like to only have enums, but QAPI doesn't
seem to allow a "leaf without branch", we need a command or struct to
use the enum else it is elided. Or maybe we didn't understood Markus
idea.


Re: [PATCH v8 02/12] accel: Introduce 'query-accels' QMP command
Posted by Markus Armbruster 4 years, 8 months ago
Philippe Mathieu-Daudé <philmd@redhat.com> writes:

> On 6/8/21 3:27 PM, Thomas Huth wrote:
>> On 26/05/2021 19.04, 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"
>>>              }
>>>          ]
>>>      }
>>>
>>> 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>
>>> ---
>>> v8:
>>> - Include code snippet from Markus adding to machine-target.json
>>>    to be able to use enum values or union branches conditional.
>>> - Use accel_find() on enum to be sure the accelerator is enabled
>>>    at runtime (chat with jsnow / eblake).
>>>
>>> Cc: Eric Blake <eblake@redhat.com>
>>> Cc: John Snow <jsnow@redhat.com>
>>> Cc: Markus Armbruster <armbru@redhat.com>
>>> ---
>>>   qapi/machine-target.json | 54 ++++++++++++++++++++++++++++++++++++++++
>>>   accel/accel-qmp.c        | 32 ++++++++++++++++++++++++
>>>   accel/meson.build        |  2 +-
>>>   3 files changed, 87 insertions(+), 1 deletion(-)
>>>   create mode 100644 accel/accel-qmp.c
>>>
>>> diff --git a/qapi/machine-target.json b/qapi/machine-target.json
>>> index e7811654b72..586a61b5d99 100644
>>> --- a/qapi/machine-target.json
>>> +++ b/qapi/machine-target.json
>>> @@ -329,3 +329,57 @@
>>>   ##
>>>   { 'command': 'query-cpu-definitions', 'returns': ['CpuDefinitionInfo'],
>>>     'if': 'defined(TARGET_PPC) || defined(TARGET_ARM) ||
>>> defined(TARGET_I386) || defined(TARGET_S390X) || defined(TARGET_MIPS)' }
>>> +
>>> +##
>>> +# @Accelerator:
>>> +#
>>> +# An enumeration of accelerator names.
>>> +#
>>> +# Since: 6.1
>>> +##
>>> +{ 'enum': 'Accelerator',
>>> +  'data': [
>>> +      { 'name': 'hax', 'if': 'defined(CONFIG_HAX)' },
>>> +      { 'name': 'hvf', 'if': 'defined(CONFIG_HVF)' },
>>> +      { 'name': 'kvm', 'if': 'defined(CONFIG_KVM)' },
>>> +      { 'name': 'qtest' },
>>> +      { 'name': 'tcg', 'if': 'defined(CONFIG_TCG)' },
>>> +      { 'name': 'whpx', 'if': 'defined(CONFIG_WHPX)' },
>>> +      { 'name': 'xen', 'if': 'defined(CONFIG_XEN_BACKEND)' } ] }
>>> +
>>> +##
>>> +# @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'] }
>> 
>> What about Markus' comment here:
>> 
>>  https://lore.kernel.org/qemu-devel/87mtsoieyz.fsf@dusky.pond.sub.org/
>> 
>> ?
>> 
>> If I've got him right, you don't need the command at all, the Accelerator
>> enum should be sufficient?
>
> Yes, this is the part jsnow said "we are waiting for Markus to comment"
> on the other thread ;) We'd like to only have enums, but QAPI doesn't
> seem to allow a "leaf without branch", we need a command or struct to
> use the enum else it is elided.

Correct; query-qmp-schema shows only the stuff used by commands and
events.

>                                 Or maybe we didn't understood Markus
> idea.


Re: [PATCH v8 02/12] accel: Introduce 'query-accels' QMP command
Posted by Markus Armbruster 4 years, 8 months ago
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.

This has become somewhat misleading, I'm afraid.  If memory serves,
earlier versions had union AcceleratorInfo with a common base struct.
This patch has just a struct, which we can grow into a union when we
actually have accelerator-specific information to report.  Perhaps

  - AcceleratorInfo provides information on a specific accelerator.  It
    contains just the accelerator name so far.

>
> - '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"
>             }
>         ]
>     }
>
> 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>
> ---
> v8:
> - Include code snippet from Markus adding to machine-target.json
>   to be able to use enum values or union branches conditional.
> - Use accel_find() on enum to be sure the accelerator is enabled
>   at runtime (chat with jsnow / eblake).
>
> Cc: Eric Blake <eblake@redhat.com>
> Cc: John Snow <jsnow@redhat.com>
> Cc: Markus Armbruster <armbru@redhat.com>
> ---
>  qapi/machine-target.json | 54 ++++++++++++++++++++++++++++++++++++++++
>  accel/accel-qmp.c        | 32 ++++++++++++++++++++++++
>  accel/meson.build        |  2 +-
>  3 files changed, 87 insertions(+), 1 deletion(-)
>  create mode 100644 accel/accel-qmp.c
>
> diff --git a/qapi/machine-target.json b/qapi/machine-target.json
> index e7811654b72..586a61b5d99 100644
> --- a/qapi/machine-target.json
> +++ b/qapi/machine-target.json
> @@ -329,3 +329,57 @@
>  ##
>  { 'command': 'query-cpu-definitions', 'returns': ['CpuDefinitionInfo'],
>    'if': 'defined(TARGET_PPC) || defined(TARGET_ARM) || defined(TARGET_I386) || defined(TARGET_S390X) || defined(TARGET_MIPS)' }
> +
> +##
> +# @Accelerator:
> +#
> +# An enumeration of accelerator names.
> +#
> +# Since: 6.1
> +##
> +{ 'enum': 'Accelerator',
> +  'data': [
> +      { 'name': 'hax', 'if': 'defined(CONFIG_HAX)' },
> +      { 'name': 'hvf', 'if': 'defined(CONFIG_HVF)' },
> +      { 'name': 'kvm', 'if': 'defined(CONFIG_KVM)' },
> +      { 'name': 'qtest' },
> +      { 'name': 'tcg', 'if': 'defined(CONFIG_TCG)' },
> +      { 'name': 'whpx', 'if': 'defined(CONFIG_WHPX)' },
> +      { 'name': 'xen', 'if': 'defined(CONFIG_XEN_BACKEND)' } ] }
> +
> +##
> +# @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..0098297caa5
> --- /dev/null
> +++ b/accel/accel-qmp.c
> @@ -0,0 +1,32 @@
> +/*
> + * QEMU accelerators, QMP commands
> + *
> + * Copyright (c) 2021 Red Hat Inc.
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +
> +#include "qemu/osdep.h"
> +#include "qemu/accel.h"
> +#include "qapi/qapi-types-machine-target.h"
> +#include "qapi/qapi-commands-machine-target.h"
> +
> +AcceleratorInfoList *qmp_query_accels(Error **errp)
> +{
> +    AcceleratorInfoList *list = NULL, **tail = &list;
> +
> +    for (Accelerator accel = 0; accel < ACCELERATOR__MAX; accel++) {
> +        AcceleratorInfo *info;
> +
> +        if (!accel_find(Accelerator_str(accel))) {
> +            /* Accelerator available at build time but not at runtime. */
> +            continue;
> +        }
> +
> +        info = g_new0(AcceleratorInfo, 1);
> +        info->name = accel;
> +        QAPI_LIST_APPEND(tail, info);
> +    }
> +
> +    return list;
> +}

If I read this correctly, there's a subtle difference between the
information returned by query-accels and the information you can get
from introspecting query-accels with query-qmp-schema: the latter gives
you the accelerators compiled into this build of QEMU, the former gives
you the ones that are actually available at run time.  Suggest to
mention that in the commit message.

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

Preferably with the commit message tweaked to address my remarks:
Acked-by: Markus Armbruster <armbru@redhat.com>