diff mbox

[v5,2/5] arm: qmp: add query-gic-capabilities interface

Message ID 1458271654-23706-3-git-send-email-peterx@redhat.com
State New, archived
Headers show

Commit Message

Peter Xu March 18, 2016, 3:27 a.m. UTC
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 <peterx@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

Comments

Markus Armbruster March 22, 2016, 6:28 p.m. UTC | #1
Copying Eric in case further review is needed in my absence.

Peter Xu <peterx@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 <peterx@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;
> +}
Eric Blake March 22, 2016, 6:42 p.m. UTC | #2
On 03/17/2016 09:27 PM, Peter Xu wrote:
> This patch adds the command "query-gic-capabilities" but not implemnet

s/not implemnet/does not implement/

> 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 <peterx@redhat.com>
> ---

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

On the surface, this seems okay.  As mentioned before, I would have
squashed 1 and 2 into a single patch.  The GICCapability type is
extensible, and introspection is sufficient at seeing what the type is
currently capable of exposing.

On the other hand...

> +++ b/scripts/qapi.py
> @@ -46,6 +46,7 @@ returns_whitelist = [
>      'query-tpm-models',
>      'query-tpm-types',
>      'ringbuf-read',
> +    'query-gic-capability',

...it required a whitelist, because you are violating the usual
convention of returning a dict.  If you DO need the whitelist, your
addition should have been kept sorted.  But you don't need it, if you
would modify your QAPI to return a dict:

{ 'struct': 'GICCapabilitiesReturn',
  'data': { 'capabilities': ['GICCapability'] } }
{ 'command': 'query-gic-capabilities',
  'returns': 'GICCapabilitiesReturn' }

Yes, the dict has only a single key, and that key points to the same
list; but now you have future extensibility: in the future, we could
return any future global data as a sibling to the array, without having
to modify every element of the array to repeat redundant information.
Markus Armbruster March 22, 2016, 7:09 p.m. UTC | #3
Eric Blake <eblake@redhat.com> writes:

> On 03/17/2016 09:27 PM, Peter Xu wrote:
>> This patch adds the command "query-gic-capabilities" but not implemnet
>
> s/not implemnet/does not implement/
>
>> 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 <peterx@redhat.com>
>> ---
>
>> +++ 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'] }
>
> On the surface, this seems okay.  As mentioned before, I would have
> squashed 1 and 2 into a single patch.  The GICCapability type is
> extensible, and introspection is sufficient at seeing what the type is
> currently capable of exposing.
>
> On the other hand...
>
>> +++ b/scripts/qapi.py
>> @@ -46,6 +46,7 @@ returns_whitelist = [
>>      'query-tpm-models',
>>      'query-tpm-types',
>>      'ringbuf-read',
>> +    'query-gic-capability',
>
> ...it required a whitelist, because you are violating the usual
> convention of returning a dict.  If you DO need the whitelist, your
> addition should have been kept sorted.  But you don't need it, if you
> would modify your QAPI to return a dict:
>
> { 'struct': 'GICCapabilitiesReturn',
>   'data': { 'capabilities': ['GICCapability'] } }
> { 'command': 'query-gic-capabilities',
>   'returns': 'GICCapabilitiesReturn' }
>
> Yes, the dict has only a single key, and that key points to the same
> list; but now you have future extensibility: in the future, we could
> return any future global data as a sibling to the array, without having
> to modify every element of the array to repeat redundant information.

I just tested it, the whitelist entry is superfluous, check_command()
accepts a list of dicts just fine.
Peter Xu March 23, 2016, 4:07 a.m. UTC | #4
On Tue, Mar 22, 2016 at 12:42:42PM -0600, Eric Blake wrote:
> On 03/17/2016 09:27 PM, Peter Xu wrote:
> > This patch adds the command "query-gic-capabilities" but not implemnet
> 
> s/not implemnet/does not implement/

Yep, again. Thanks.

> 
> > 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 <peterx@redhat.com>
> > ---
> 
> > +++ 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'] }
> 
> On the surface, this seems okay.  As mentioned before, I would have
> squashed 1 and 2 into a single patch.  The GICCapability type is
> extensible, and introspection is sufficient at seeing what the type is
> currently capable of exposing.
> 
> On the other hand...
> 
> > +++ b/scripts/qapi.py
> > @@ -46,6 +46,7 @@ returns_whitelist = [
> >      'query-tpm-models',
> >      'query-tpm-types',
> >      'ringbuf-read',
> > +    'query-gic-capability',
> 
> ...it required a whitelist, because you are violating the usual
> convention of returning a dict.  If you DO need the whitelist, your
> addition should have been kept sorted.  But you don't need it, if you
> would modify your QAPI to return a dict:
> 
> { 'struct': 'GICCapabilitiesReturn',
>   'data': { 'capabilities': ['GICCapability'] } }
> { 'command': 'query-gic-capabilities',
>   'returns': 'GICCapabilitiesReturn' }
> 
> Yes, the dict has only a single key, and that key points to the same
> list; but now you have future extensibility: in the future, we could
> return any future global data as a sibling to the array, without having
> to modify every element of the array to repeat redundant information.

Yes, I think this is better solution. Will adopt this in next version.

Thanks for the comments!

-- peterx
Peter Xu March 23, 2016, 4:14 a.m. UTC | #5
On Tue, Mar 22, 2016 at 07:28:13PM +0100, Markus Armbruster wrote:
> Peter Xu <peterx@redhat.com> writes:
> > 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?

This might not be required. Agree with you and Eric. Will use a hash
instead with single key.

> > +#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.

I just copied them from machine.c and dropped lots, seems still
redundant... Will keep it a minimum subset in next version. Thanks!

-- peterx
Markus Armbruster March 23, 2016, 9:52 a.m. UTC | #6
Peter Xu <peterx@redhat.com> writes:

> On Tue, Mar 22, 2016 at 07:28:13PM +0100, Markus Armbruster wrote:
>> Peter Xu <peterx@redhat.com> writes:
>> > 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?

I've since tested it and double-checked the code: you don't need this.

>> If yes, why should your command be permitted to violate the design
>> rules?
>
> This might not be required. Agree with you and Eric. Will use a hash
> instead with single key.

The rule against returning non-dictionaries exists to avoid interfaces
that cannot evolve.  With a dictionary, you can evolve by adding
members.

The rule does *not* forbid returning lists of dictionaries.  When a
command fundamentally returns a list of things, being able to evolve the
things suffices.

query-gic-capabilities looks like it fundamentally returns a list of
capabilities.  Returning ['GICCapability'] is just fine then.

Only if query-gic-capabilities doesn't have this list nature should you
complicate its return value by wrapping it in another object type.

In either case, drop the change to returns_whitelist.

[...]
Markus Armbruster March 23, 2016, 9:54 a.m. UTC | #7
Peter Xu <peterx@redhat.com> writes:

> On Tue, Mar 22, 2016 at 12:42:42PM -0600, Eric Blake wrote:
>> On 03/17/2016 09:27 PM, Peter Xu wrote:
>> > This patch adds the command "query-gic-capabilities" but not implemnet
>> 
>> s/not implemnet/does not implement/
>
> Yep, again. Thanks.
>
>> 
>> > 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 <peterx@redhat.com>
>> > ---
>> 
>> > +++ 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'] }
>> 
>> On the surface, this seems okay.  As mentioned before, I would have
>> squashed 1 and 2 into a single patch.  The GICCapability type is
>> extensible, and introspection is sufficient at seeing what the type is
>> currently capable of exposing.
>> 
>> On the other hand...
>> 
>> > +++ b/scripts/qapi.py
>> > @@ -46,6 +46,7 @@ returns_whitelist = [
>> >      'query-tpm-models',
>> >      'query-tpm-types',
>> >      'ringbuf-read',
>> > +    'query-gic-capability',
>> 
>> ...it required a whitelist, because you are violating the usual

It doesn't :)

>> convention of returning a dict.  If you DO need the whitelist, your
>> addition should have been kept sorted.  But you don't need it, if you
>> would modify your QAPI to return a dict:
>> 
>> { 'struct': 'GICCapabilitiesReturn',
>>   'data': { 'capabilities': ['GICCapability'] } }
>> { 'command': 'query-gic-capabilities',
>>   'returns': 'GICCapabilitiesReturn' }
>> 
>> Yes, the dict has only a single key, and that key points to the same
>> list; but now you have future extensibility: in the future, we could
>> return any future global data as a sibling to the array, without having
>> to modify every element of the array to repeat redundant information.
>
> Yes, I think this is better solution. Will adopt this in next version.

As explained in my other message, do this only if query-gic-capabilities
truly needs it.  There's plenty of precedence for returning a list of a
structured type.

> Thanks for the comments!
>
> -- peterx
Peter Xu March 23, 2016, 12:04 p.m. UTC | #8
On Wed, Mar 23, 2016 at 10:52:29AM +0100, Markus Armbruster wrote:
> The rule against returning non-dictionaries exists to avoid interfaces
> that cannot evolve.  With a dictionary, you can evolve by adding
> members.
> 
> The rule does *not* forbid returning lists of dictionaries.  When a
> command fundamentally returns a list of things, being able to evolve the
> things suffices.

Ok.

> 
> query-gic-capabilities looks like it fundamentally returns a list of
> capabilities.  Returning ['GICCapability'] is just fine then.

I have posted v6 just as Eric has suggested. At least one advantage
is that it is easier to be extended (if needed) in the future, also
to follow the more-generic format to use dicts rather than
arrays. If you would not mind, I'll keep the dict interface there.

[...]

> In either case, drop the change to returns_whitelist.

Yep. Dropped in v6.

Thanks.

-- peterx
Markus Armbruster March 23, 2016, 2:06 p.m. UTC | #9
Peter Xu <peterx@redhat.com> writes:

> On Wed, Mar 23, 2016 at 10:52:29AM +0100, Markus Armbruster wrote:
>> The rule against returning non-dictionaries exists to avoid interfaces
>> that cannot evolve.  With a dictionary, you can evolve by adding
>> members.
>> 
>> The rule does *not* forbid returning lists of dictionaries.  When a
>> command fundamentally returns a list of things, being able to evolve the
>> things suffices.
>
> Ok.
>
>> 
>> query-gic-capabilities looks like it fundamentally returns a list of
>> capabilities.  Returning ['GICCapability'] is just fine then.
>
> I have posted v6 just as Eric has suggested. At least one advantage
> is that it is easier to be extended (if needed) in the future, also
> to follow the more-generic format to use dicts rather than
> arrays. If you would not mind, I'll keep the dict interface there.

Returning such a list of a structured type is a well-established
convention by now --- qmp-introspect.c shows 28 commands doing it, most
of them named query-FOO.  I'd rather not start a different convention
now, just because we got temporarily confused about our own rules.  So,
unless there's something about query-gic-capabilities that makes it more
likely to need extension outside its list element type, let's stick to
returning the list.

Eric?

>
> [...]
>
>> In either case, drop the change to returns_whitelist.
>
> Yep. Dropped in v6.
>
> Thanks.
>
> -- peterx
Eric Blake March 23, 2016, 2:31 p.m. UTC | #10
On 03/23/2016 08:06 AM, Markus Armbruster wrote:

>>> query-gic-capabilities looks like it fundamentally returns a list of
>>> capabilities.  Returning ['GICCapability'] is just fine then.
>>
>> I have posted v6 just as Eric has suggested. At least one advantage
>> is that it is easier to be extended (if needed) in the future, also
>> to follow the more-generic format to use dicts rather than
>> arrays. If you would not mind, I'll keep the dict interface there.
> 
> Returning such a list of a structured type is a well-established
> convention by now --- qmp-introspect.c shows 28 commands doing it, most
> of them named query-FOO.  I'd rather not start a different convention
> now, just because we got temporarily confused about our own rules.  So,
> unless there's something about query-gic-capabilities that makes it more
> likely to need extension outside its list element type, let's stick to
> returning the list.
> 
> Eric?

Makes sense to me, and perhaps my own fault for getting confused by not
even trying to delete just the whitelist line and seeing what would
happen.  Having a consistent style of query-* returning an array type is
reasonable to continue.
diff mbox

Patch

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 = [
     'query-tpm-models',
     'query-tpm-types',
     'ringbuf-read',
+    'query-gic-capability',
 
     # From QGA:
     'guest-file-open',
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"
+
+GICCapabilityList *qmp_query_gic_capabilities(Error **errp)
+{
+    return NULL;
+}