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