Copying Eric in case further review is needed in my absence.
Peter Xu <pet...@redhat.com> writes:
> This patch adds the command "query-gic-capabilities" but not implemnet
> it. The command is ARM-only. Return of the command is a list of
> GICCapability struct that describes all GIC versions that current QEMU
> and system support.
>
> Signed-off-by: Peter Xu <pet...@redhat.com>
> ---
> monitor.c | 8 ++++++++
> qapi-schema.json | 11 +++++++++++
> qmp-commands.hx | 26 ++++++++++++++++++++++++++
> scripts/qapi.py | 1 +
> target-arm/Makefile.objs | 2 +-
> target-arm/monitor.c | 31 +++++++++++++++++++++++++++++++
> 6 files changed, 78 insertions(+), 1 deletion(-)
> create mode 100644 target-arm/monitor.c
>
> diff --git a/monitor.c b/monitor.c
> index 894f862..d463dc4 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -4257,3 +4257,11 @@ void qmp_dump_skeys(const char *filename, Error **errp)
> error_setg(errp, QERR_FEATURE_DISABLED, "dump-skeys");
> }
> #endif
> +
> +#ifndef TARGET_ARM
> +GICCapabilityList *qmp_query_gic_capabilities(Error **errp)
> +{
> + error_setg(errp, QERR_FEATURE_DISABLED, "query-gic-capabilities");
> + return NULL;
> +}
> +#endif
> diff --git a/qapi-schema.json b/qapi-schema.json
> index da9671a..b2ef149 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -4156,3 +4156,14 @@
> 'data': { 'version': 'int',
> 'emulated': 'bool',
> 'kernel': 'bool' } }
> +
> +##
> +# @query-gic-capabilities:
> +#
> +# Return a list of supported GIC version capabilities.
> +#
> +# Returns: a list of GICCapability.
> +#
> +# Since: 2.6
> +##
> +{ 'command': 'query-gic-capabilities', 'returns': ['GICCapability'] }
> diff --git a/qmp-commands.hx b/qmp-commands.hx
> index 9e05365..a124ea8 100644
> --- a/qmp-commands.hx
> +++ b/qmp-commands.hx
> @@ -4853,3 +4853,29 @@ Example:
> {"type": 0, "out-pport": 0, "pport": 0, "vlan-id": 3840,
> "pop-vlan": 1, "id": 251658240}
> ]}
> +
> +EQMP
> +
> +#if defined TARGET_ARM
> + {
> + .name = "query-gic-capabilities",
> + .args_type = "",
> + .mhandler.cmd_new = qmp_marshal_query_gic_capabilities,
> + },
> +#endif
> +
> +SQMP
> +query-gic-capabilities
> +---------------
> +
> +Return a list of supported ARM GIC versions and their capabilities.
> +
> +Arguments: None
> +
> +Example:
> +
> +-> { "execute": "query-gic-capabilities" }
> +<- { "return": [{ "version": 2, "emulated": true, "kernel": false },
> + { "version": 3, "emulated": false, "kernel": true } ] }
> +
> +EQMP
> diff --git a/scripts/qapi.py b/scripts/qapi.py
> index 6b2aa6e..716474e 100644
> --- a/scripts/qapi.py
> +++ b/scripts/qapi.py
> @@ -46,6 +46,7 @@ returns_whitelist = [
# Whitelist of commands allowed to return a non-dictionary
returns_whitelist = [
'human-monitor-command',
'qom-get',
'query-migrate-cache-size',
> 'query-tpm-models',
> 'query-tpm-types',
> 'ringbuf-read',
> + 'query-gic-capability',
>
> # From QGA:
> 'guest-file-open',
The whitelist exists to except existing commands from design rules on
return types. New commands don't get to violate the rules without a
really, really compelling reason.
Do you actually need this?
If yes, why should your command be permitted to violate the design
rules?
> diff --git a/target-arm/Makefile.objs b/target-arm/Makefile.objs
> index a80eb39..334074c 100644
> --- a/target-arm/Makefile.objs
> +++ b/target-arm/Makefile.objs
> @@ -8,4 +8,4 @@ obj-y += translate.o op_helper.o helper.o cpu.o
> obj-y += neon_helper.o iwmmxt_helper.o
> obj-y += gdbstub.o
> obj-$(TARGET_AARCH64) += cpu64.o translate-a64.o helper-a64.o gdbstub64.o
> -obj-y += crypto_helper.o
> +obj-y += crypto_helper.o monitor.o
> diff --git a/target-arm/monitor.c b/target-arm/monitor.c
> new file mode 100644
> index 0000000..5678eb8
> --- /dev/null
> +++ b/target-arm/monitor.c
> @@ -0,0 +1,31 @@
> +/*
> + * QEMU monitor.c for ARM.
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> copy
> + * of this software and associated documentation files (the "Software"), to
> deal
> + * in the Software without restriction, including without limitation the
> rights
> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
> + * copies of the Software, and to permit persons to whom the Software is
> + * furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> FROM,
> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
> + * THE SOFTWARE.
> + */
> +#include "qemu/osdep.h"
> +#include "hw/boards.h"
> +#include "qemu/error-report.h"
> +#include "sysemu/kvm.h"
> +#include "qmp-commands.h"
I very much doubt you need all these includes. Try dropping all but the
first and the last one.
> +
> +GICCapabilityList *qmp_query_gic_capabilities(Error **errp)
> +{
> + return NULL;
> +}
