| Message ID | 20210505125806.1263441-3-philmd@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | qtests: Check accelerator available at runtime via QMP 'query-accels' | expand |
On Wed, May 05, 2021 at 02:57:56PM +0200, 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. I assume this will be clarified in v8, based on your reply to v5. I don't understand what "target-specific poisoning of accelerator definitions" means. > > 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> Sorry for not even reviewing this before, but still: Acked-by: Eduardo Habkost <ehabkost@redhat.com>
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. I second Eduardo's plea to explain this more clearly. It's important, because if a properly conditionalized enum is feasible, then query-accel isn't needed. > Reviewed-by: Eric Blake <eblake@redhat.com> > Reviewed-by: Alex Bennée <alex.bennee@linaro.org> > Tested-by: Alex Bennée <alex.bennee@linaro.org> > Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> > --- > qapi/machine.json | 47 +++++++++++++++++++++++++++++++++++++++++++++ > accel/accel-qmp.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++ > accel/meson.build | 2 +- > 3 files changed, 97 insertions(+), 1 deletion(-) > create mode 100644 accel/accel-qmp.c > > diff --git a/qapi/machine.json b/qapi/machine.json > index 6e90d463fc9..6dd3b765248 100644 > --- a/qapi/machine.json > +++ b/qapi/machine.json > @@ -1274,3 +1274,50 @@ > ## > { 'event': 'MEM_UNPLUG_ERROR', > 'data': { 'device': 'str', 'msg': 'str' } } > + > +## > +# @Accelerator: > +# > +# An enumeration of accelerator names. > +# > +# Since: 6.1 > +## > +{ 'enum': 'Accelerator', > + 'data': [ 'hax', 'hvf', 'kvm', 'qtest', 'tcg', 'whpx', 'xen' ] } > + > +## > +# @AcceleratorInfo: > +# > +# Accelerator information. > +# > +# @name: The accelerator name. > +# > +# Since: 6.1 > +## > +{ 'struct': 'AcceleratorInfo', > + 'data': { 'name': 'Accelerator' } } > + > +## > +# @query-accels: > +# > +# Get a list of AcceleratorInfo for all built-in accelerators. "built-in" means compiled in. See accel_builtin_list[] below. > +# > +# 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; > +} CLI -accel help also lists the available accelerators, but finds them differently. Whereas query-accel relies on hard-coded accel_builtin_list[], that one instead finds the concrete subtypes of TYPE_ACCEL. Any particular reason for the difference? > 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'))
On 5/5/21 9:41 PM, Eduardo Habkost wrote: > On Wed, May 05, 2021 at 02:57:56PM +0200, 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. > > I assume this will be clarified in v8, based on your reply to v5. > I don't understand what "target-specific poisoning of accelerator > definitions" means. $ git grep poison ... include/exec/poison.h:88:#pragma GCC poison CONFIG_HAX include/exec/poison.h:89:#pragma GCC poison CONFIG_HVF include/exec/poison.h:90:#pragma GCC poison CONFIG_LINUX_USER include/exec/poison.h:91:#pragma GCC poison CONFIG_KVM include/exec/poison.h:92:#pragma GCC poison CONFIG_SOFTMMU include/exec/poison.h:93:#pragma GCC poison CONFIG_WHPX include/exec/poison.h:94:#pragma GCC poison CONFIG_XEN I thought QAPI was target agnostic, but I just found: if module.endswith('-target') qapi_specific_outputs += qapi_module_outputs specific_ss.add(when: 'CONFIG_SOFTMMU', if_true: qapi_files[i]) So instead of adding this to qapi/machine.json I'll see if I can add it to qapi/machine-target.json and use conditionals.
Markus Armbruster <armbru@redhat.com> writes: > 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 "union branches" existed in a previous iteration, but no more. >> because of target-specific poisoning of accelerator definitions. > > I second Eduardo's plea to explain this more clearly. It's important, > because if a properly conditionalized enum is feasible, then query-accel > isn't needed. The appended incremental patch conditionalizes the enum. It applies on top of the series, and passes "make check" for me. Seems to contradict "we can't make the enum values conditional". diff --git a/qapi/machine-target.json b/qapi/machine-target.json index e7811654b7..586a61b5d9 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/qapi/machine.json b/qapi/machine.json index 79a0891793..58a9c86b36 100644 --- a/qapi/machine.json +++ b/qapi/machine.json @@ -1274,50 +1274,3 @@ ## { 'event': 'MEM_UNPLUG_ERROR', 'data': { 'device': 'str', 'msg': 'str' } } - -## -# @Accelerator: -# -# An enumeration of accelerator names. -# -# Since: 6.1 -## -{ 'enum': 'Accelerator', - 'data': [ 'hax', 'hvf', 'kvm', 'qtest', 'tcg', 'whpx', 'xen' ] } - -## -# @AcceleratorInfo: -# -# Accelerator information. -# -# @name: The accelerator name. -# -# Since: 6.1 -## -{ 'struct': 'AcceleratorInfo', - 'data': { 'name': 'Accelerator' } } - -## -# @query-accels: -# -# Get a list of AcceleratorInfo for all built-in accelerators. -# -# Returns: a list of @AcceleratorInfo describing each accelerator. -# -# Since: 6.1 -# -# Example: -# -# -> { "execute": "query-accels" } -# <- { "return": [ -# { -# "name": "qtest" -# }, -# { -# "name": "kvm" -# } -# ] } -# -## -{ 'command': 'query-accels', - 'returns': ['AcceleratorInfo'] } diff --git a/accel/accel-qmp.c b/accel/accel-qmp.c index 426737b3f9..aca90f8682 100644 --- a/accel/accel-qmp.c +++ b/accel/accel-qmp.c @@ -7,42 +7,18 @@ */ #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 -}; +#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++) { - if (accel_builtin_list[accel]) { - AcceleratorInfo *info = g_new0(AcceleratorInfo, 1); + AcceleratorInfo *info = g_new0(AcceleratorInfo, 1); - info->name = accel; + info->name = accel; - QAPI_LIST_APPEND(tail, info); - } + QAPI_LIST_APPEND(tail, info); } return list;
Markus Armbruster <armbru@redhat.com> writes: > The appended incremental patch conditionalizes the enum. It applies on > top of the series, and passes "make check" for me. Seems to contradict > "we can't make the enum values conditional". Neglected to mention that query-accel becomes almost useless in this version: we need it just to pull enum Accelerator into the QMP interface, so query-qmp-schema shows it. It may become a useful command if we add more members to AcceleratorInfo later.
diff --git a/qapi/machine.json b/qapi/machine.json index 6e90d463fc9..6dd3b765248 100644 --- a/qapi/machine.json +++ b/qapi/machine.json @@ -1274,3 +1274,50 @@ ## { 'event': 'MEM_UNPLUG_ERROR', 'data': { 'device': 'str', 'msg': 'str' } } + +## +# @Accelerator: +# +# An enumeration of accelerator names. +# +# Since: 6.1 +## +{ 'enum': 'Accelerator', + 'data': [ 'hax', 'hvf', 'kvm', 'qtest', 'tcg', 'whpx', 'xen' ] } + +## +# @AcceleratorInfo: +# +# Accelerator information. +# +# @name: The accelerator name. +# +# Since: 6.1 +## +{ 'struct': 'AcceleratorInfo', + 'data': { 'name': 'Accelerator' } } + +## +# @query-accels: +# +# Get a list of AcceleratorInfo for all built-in accelerators. +# +# Returns: a list of @AcceleratorInfo describing each accelerator. +# +# Since: 6.1 +# +# Example: +# +# -> { "execute": "query-accels" } +# <- { "return": [ +# { +# "name": "qtest" +# }, +# { +# "name": "kvm" +# } +# ] } +# +## +{ 'command': 'query-accels', + 'returns': ['AcceleratorInfo'] } diff --git a/accel/accel-qmp.c b/accel/accel-qmp.c new file mode 100644 index 00000000000..426737b3f9a --- /dev/null +++ b/accel/accel-qmp.c @@ -0,0 +1,49 @@ +/* + * QEMU accelerators, QMP commands + * + * Copyright (c) 2021 Red Hat Inc. + * + * SPDX-License-Identifier: GPL-2.0-or-later + */ + +#include "qemu/osdep.h" +#include "qapi/qapi-commands-machine.h" + +static const bool accel_builtin_list[ACCELERATOR__MAX] = { + [ACCELERATOR_QTEST] = true, +#ifdef CONFIG_TCG + [ACCELERATOR_TCG] = true, +#endif +#ifdef CONFIG_KVM + [ACCELERATOR_KVM] = true, +#endif +#ifdef CONFIG_HAX + [ACCELERATOR_HAX] = true, +#endif +#ifdef CONFIG_HVF + [ACCELERATOR_HVF] = true, +#endif +#ifdef CONFIG_WHPX + [ACCELERATOR_WHPX] = true, +#endif +#ifdef CONFIG_XEN_BACKEND + [ACCELERATOR_XEN] = true, +#endif +}; + +AcceleratorInfoList *qmp_query_accels(Error **errp) +{ + AcceleratorInfoList *list = NULL, **tail = &list; + + for (Accelerator accel = 0; accel < ACCELERATOR__MAX; accel++) { + if (accel_builtin_list[accel]) { + AcceleratorInfo *info = g_new0(AcceleratorInfo, 1); + + info->name = accel; + + QAPI_LIST_APPEND(tail, info); + } + } + + return list; +} diff --git a/accel/meson.build b/accel/meson.build index b44ba30c864..7a48f6d568d 100644 --- a/accel/meson.build +++ b/accel/meson.build @@ -1,4 +1,4 @@ -specific_ss.add(files('accel-common.c')) +specific_ss.add(files('accel-common.c', 'accel-qmp.c')) softmmu_ss.add(files('accel-softmmu.c')) user_ss.add(files('accel-user.c'))
Introduce the 'query-accels' QMP command which returns a list of built-in accelerator names. - Accelerator is a QAPI enum of all existing accelerators, - AcceleratorInfo is a QAPI structure providing accelerator specific information. Currently the common structure base provides the name of the accelerator, while the specific part is empty, but each accelerator can expand it. - 'query-accels' QMP command returns a list of @AcceleratorInfo For example on a KVM-only build we get: { "execute": "query-accels" } { "return": [ { "name": "qtest" }, { "name": "kvm" } ] } Note that we can't make the enum values or union branches conditional because of target-specific poisoning of accelerator definitions. Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Alex Bennée <alex.bennee@linaro.org> Tested-by: Alex Bennée <alex.bennee@linaro.org> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> --- qapi/machine.json | 47 +++++++++++++++++++++++++++++++++++++++++++++ accel/accel-qmp.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++ accel/meson.build | 2 +- 3 files changed, 97 insertions(+), 1 deletion(-) create mode 100644 accel/accel-qmp.c