qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH 0/3] ARM: add query-gic-capability SMP command
@ 2016-02-23 10:52 Peter Xu
  2016-02-23 10:52 ` [Qemu-devel] [PATCH 1/3] arm: gic: add GICType Peter Xu
                   ` (2 more replies)
  0 siblings, 3 replies; 15+ messages in thread
From: Peter Xu @ 2016-02-23 10:52 UTC (permalink / raw)
  To: qemu-devel; +Cc: wei, peter.maydell, drjones, mdroth, armbru, peterx, abologna

This patch is to add ARM-specific command "query-gic-capability".

This command can report which kind of GIC device the host/QEMU
support. The returned result is in the form of array. One example
would be:

{"execute": "query-gic-capability"}
{"return": ["gicv2-kvm", "gicv2"]}

For more information on the interface, please refer to the RFC
thread:

https://lists.gnu.org/archive/html/qemu-devel/2016-02/msg02882.html

As discussed in previous RFC threads, this may not the best way to
implement it. Instead, a more generic interface/framework might be
prefered. However this is still needed possibly before the new
interface ready. As a tradeoff, I/we decided to first provide the
command, then we'd better make sure we can have a smooth transition
if there would be a better solution.

Peter Xu (3):
  arm: gic: add GICType
  arm: gic: add "query-gic-capability" interface
  arm: implement query-gic-capability

 monitor.c            |  8 ++++++++
 qapi-schema.json     | 28 ++++++++++++++++++++++++++++
 qmp-commands.hx      | 25 +++++++++++++++++++++++++
 scripts/qapi.py      |  1 +
 target-arm/machine.c | 37 +++++++++++++++++++++++++++++++++++++
 5 files changed, 99 insertions(+)

-- 
2.4.3

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [Qemu-devel] [PATCH 1/3] arm: gic: add GICType
  2016-02-23 10:52 [Qemu-devel] [PATCH 0/3] ARM: add query-gic-capability SMP command Peter Xu
@ 2016-02-23 10:52 ` Peter Xu
  2016-03-01 14:20   ` Andrea Bolognani
  2016-03-01 16:46   ` Eric Blake
  2016-02-23 10:52 ` [Qemu-devel] [PATCH 2/3] arm: gic: add "query-gic-capability" interface Peter Xu
  2016-02-23 10:52 ` [Qemu-devel] [PATCH 3/3] arm: implement query-gic-capability Peter Xu
  2 siblings, 2 replies; 15+ messages in thread
From: Peter Xu @ 2016-02-23 10:52 UTC (permalink / raw)
  To: qemu-devel; +Cc: wei, peter.maydell, drjones, mdroth, armbru, peterx, abologna

A new enum type is added to define ARM GIC types.

Signed-off-by: Peter Xu <peterx@redhat.com>
---
 qapi-schema.json | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/qapi-schema.json b/qapi-schema.json
index 8d04897..81654bd 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -4083,3 +4083,20 @@
 ##
 { 'enum': 'ReplayMode',
   'data': [ 'none', 'record', 'play' ] }
+
+##
+# @GICType:
+#
+# An enumeration of GIC types
+#
+# @gicv2:     GICv2 support without kernel irqchip
+#
+# @gicv3:     GICv3 support without kernel irqchip
+#
+# @gicv2-kvm: GICv3 support with kernel irqchip
+#
+# @gicv3-kvm: GICv3 support with kernel irqchip
+#
+# Since: 2.6
+##
+{ 'enum': 'GICType', 'data': [ 'gicv2', 'gicv3', 'gicv2-kvm', 'gicv3-kvm' ] }
-- 
2.4.3

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [Qemu-devel] [PATCH 2/3] arm: gic: add "query-gic-capability" interface
  2016-02-23 10:52 [Qemu-devel] [PATCH 0/3] ARM: add query-gic-capability SMP command Peter Xu
  2016-02-23 10:52 ` [Qemu-devel] [PATCH 1/3] arm: gic: add GICType Peter Xu
@ 2016-02-23 10:52 ` Peter Xu
  2016-02-23 10:52 ` [Qemu-devel] [PATCH 3/3] arm: implement query-gic-capability Peter Xu
  2 siblings, 0 replies; 15+ messages in thread
From: Peter Xu @ 2016-02-23 10:52 UTC (permalink / raw)
  To: qemu-devel; +Cc: wei, peter.maydell, drjones, mdroth, armbru, peterx, abologna

This is ARM-only command.

Signed-off-by: Peter Xu <peterx@redhat.com>
---
 monitor.c            |  8 ++++++++
 qapi-schema.json     | 11 +++++++++++
 qmp-commands.hx      | 25 +++++++++++++++++++++++++
 scripts/qapi.py      |  1 +
 target-arm/machine.c |  7 +++++++
 5 files changed, 52 insertions(+)

diff --git a/monitor.c b/monitor.c
index 73eac17..1c7f127 100644
--- a/monitor.c
+++ b/monitor.c
@@ -4241,3 +4241,11 @@ void qmp_dump_skeys(const char *filename, Error **errp)
     error_setg(errp, QERR_FEATURE_DISABLED, "dump-skeys");
 }
 #endif
+
+#ifndef TARGET_ARM
+GICTypeList *qmp_query_gic_capability(Error **errp)
+{
+    error_setg(errp, QERR_FEATURE_DISABLED, "query-gic-capability");
+    return NULL;
+}
+#endif
diff --git a/qapi-schema.json b/qapi-schema.json
index 81654bd..7e3b8cd 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -4100,3 +4100,14 @@
 # Since: 2.6
 ##
 { 'enum': 'GICType', 'data': [ 'gicv2', 'gicv3', 'gicv2-kvm', 'gicv3-kvm' ] }
+
+##
+# @query-gic-capability:
+#
+# Return a list of supported GIC types
+#
+# Returns: a list of GICType
+#
+# Since: 2.6
+##
+{ 'command': 'query-gic-capability', 'returns': ['GICType'] }
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 020e5ee..55930d3 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -4818,3 +4818,28 @@ Example:
                  {"type": 0, "out-pport": 0, "pport": 0, "vlan-id": 3840,
                   "pop-vlan": 1, "id": 251658240}
    ]}
+
+EQMP
+
+#if defined TARGET_ARM
+    {
+        .name       = "query-gic-capability",
+        .args_type  = "",
+        .mhandler.cmd_new = qmp_marshal_query_gic_capability,
+    },
+#endif
+
+SQMP
+query-gic-capability
+---------------
+
+Return a list of supported ARM GIC types.
+
+Arguments: None
+
+Example:
+
+-> { "execute": "query-gic-capability" }
+<- { "return": [ "gicv2", "gicv2-kvm", "gicv3-kvm" ] }
+
+EQMP
diff --git a/scripts/qapi.py b/scripts/qapi.py
index 7dec611..f4900a1 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/machine.c b/target-arm/machine.c
index ed1925a..9e10232 100644
--- a/target-arm/machine.c
+++ b/target-arm/machine.c
@@ -347,3 +347,10 @@ const char *gicv3_class_name(void)
 
     exit(1);
 }
+
+GICTypeList *qmp_query_gic_capability(Error **errp);
+
+GICTypeList *qmp_query_gic_capability(Error **errp)
+{
+    return NULL;
+}
-- 
2.4.3

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [Qemu-devel] [PATCH 3/3] arm: implement query-gic-capability
  2016-02-23 10:52 [Qemu-devel] [PATCH 0/3] ARM: add query-gic-capability SMP command Peter Xu
  2016-02-23 10:52 ` [Qemu-devel] [PATCH 1/3] arm: gic: add GICType Peter Xu
  2016-02-23 10:52 ` [Qemu-devel] [PATCH 2/3] arm: gic: add "query-gic-capability" interface Peter Xu
@ 2016-02-23 10:52 ` Peter Xu
  2 siblings, 0 replies; 15+ messages in thread
From: Peter Xu @ 2016-02-23 10:52 UTC (permalink / raw)
  To: qemu-devel; +Cc: wei, peter.maydell, drjones, mdroth, armbru, peterx, abologna

For emulated ARM VM, only gicv2 is supported. We need to add gicv3 in
when emulated gicv3 ready. For KVM accelerated ARM VM, we detect the
capability bits using ioctls.

Signed-off-by: Peter Xu <peterx@redhat.com>
---
 target-arm/machine.c | 32 +++++++++++++++++++++++++++++++-
 1 file changed, 31 insertions(+), 1 deletion(-)

diff --git a/target-arm/machine.c b/target-arm/machine.c
index 9e10232..7915096 100644
--- a/target-arm/machine.c
+++ b/target-arm/machine.c
@@ -1,3 +1,4 @@
+#include <linux/kvm.h>
 #include "qemu/osdep.h"
 #include "hw/hw.h"
 #include "hw/boards.h"
@@ -348,9 +349,38 @@ const char *gicv3_class_name(void)
     exit(1);
 }
 
+static GICTypeList *gic_capability_add(GICTypeList *head, GICType type)
+{
+    GICTypeList *item = g_new0(GICTypeList, 1);
+    item->value = type;
+    item->next = head;
+    return item;
+}
+
 GICTypeList *qmp_query_gic_capability(Error **errp);
 
 GICTypeList *qmp_query_gic_capability(Error **errp)
 {
-    return NULL;
+    /* We by default support emulated gicv2 */
+    GICTypeList *head = gic_capability_add(NULL, GIC_TYPE_GICV2);
+
+    /* TODO: add emulated gicv3 type when ready */
+
+#ifdef CONFIG_KVM
+    if (kvm_enabled()) {
+        /* Test KVM GICv2 */
+        if (kvm_create_device(kvm_state, KVM_DEV_TYPE_ARM_VGIC_V2,
+                              true) >= 0) {
+            head = gic_capability_add(head, GIC_TYPE_GICV2_KVM);
+        }
+
+        /* Test KVM GICv3 */
+        if (kvm_create_device(kvm_state, KVM_DEV_TYPE_ARM_VGIC_V3,
+                              true) >= 0) {
+            head = gic_capability_add(head, GIC_TYPE_GICV3_KVM);
+        }
+    }
+#endif
+
+    return head;
 }
-- 
2.4.3

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [Qemu-devel] [PATCH 1/3] arm: gic: add GICType
  2016-02-23 10:52 ` [Qemu-devel] [PATCH 1/3] arm: gic: add GICType Peter Xu
@ 2016-03-01 14:20   ` Andrea Bolognani
  2016-03-02  3:34     ` Peter Xu
  2016-03-01 16:46   ` Eric Blake
  1 sibling, 1 reply; 15+ messages in thread
From: Andrea Bolognani @ 2016-03-01 14:20 UTC (permalink / raw)
  To: Peter Xu, qemu-devel; +Cc: wei, peter.maydell, drjones, armbru, mdroth

On Tue, 2016-02-23 at 18:52 +0800, Peter Xu wrote:
> A new enum type is added to define ARM GIC types.
> 
> Signed-off-by: Peter Xu <peterx@redhat.com>
> ---
>  qapi-schema.json | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
> 
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 8d04897..81654bd 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -4083,3 +4083,20 @@
>  ##
>  { 'enum': 'ReplayMode',
>    'data': [ 'none', 'record', 'play' ] }
> +
> +##
> +# @GICType:
> +#
> +# An enumeration of GIC types
> +#
> +# @gicv2:     GICv2 support without kernel irqchip
> +#
> +# @gicv3:     GICv3 support without kernel irqchip
> +#
> +# @gicv2-kvm: GICv3 support with kernel irqchip
> +#
> +# @gicv3-kvm: GICv3 support with kernel irqchip
> +#
> +# Since: 2.6
> +##
> +{ 'enum': 'GICType', 'data': [ 'gicv2', 'gicv3', 'gicv2-kvm', 'gicv3-kvm' ] }

Wouldn't this conflate the use of accel= and kernel_irqchip= options?

IIUC, depending on the hardware, you might find yourself in the
following situation:

  accel=tcg,gic-version=3                     unavailable
  accel=kvm,kernel_irqchip=off,gic-version=3  unavailable
  accel=kvm,gic-version=3                     available

so I'd expect the output to be something like

  [ "v2": { "tcg": true,
            "kvm-without-kernel-irqchip": true,
            "kvm": true },
    "v3": { "tcg": false,
            "kvm-without-kernel-irqchip": false,
            "kvm": true } ]

Since libvirt currently doesn't have support for the
kernel_irqchip= option, it would only take the "tcg" and "kvm"
values into account; on the other hand, if at some point
libvirt will grow support for that option it would be able to
retrieve all the required information.

Cheers.

-- 
Andrea Bolognani
Software Engineer - Virtualization Team

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Qemu-devel] [PATCH 1/3] arm: gic: add GICType
  2016-02-23 10:52 ` [Qemu-devel] [PATCH 1/3] arm: gic: add GICType Peter Xu
  2016-03-01 14:20   ` Andrea Bolognani
@ 2016-03-01 16:46   ` Eric Blake
  2016-03-02  4:55     ` Peter Xu
  1 sibling, 1 reply; 15+ messages in thread
From: Eric Blake @ 2016-03-01 16:46 UTC (permalink / raw)
  To: Peter Xu, qemu-devel
  Cc: wei, peter.maydell, drjones, mdroth, armbru, abologna

[-- Attachment #1: Type: text/plain, Size: 1081 bytes --]

On 02/23/2016 03:52 AM, Peter Xu wrote:
> A new enum type is added to define ARM GIC types.
> 
> Signed-off-by: Peter Xu <peterx@redhat.com>
> ---
>  qapi-schema.json | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
> 
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 8d04897..81654bd 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -4083,3 +4083,20 @@
>  ##
>  { 'enum': 'ReplayMode',
>    'data': [ 'none', 'record', 'play' ] }
> +
> +##
> +# @GICType:
> +#
> +# An enumeration of GIC types

Worth spelling out the acronym?

> +#
> +# @gicv2:     GICv2 support without kernel irqchip
> +#
> +# @gicv3:     GICv3 support without kernel irqchip
> +#
> +# @gicv2-kvm: GICv3 support with kernel irqchip

Doc typo, should be GICv2

> +#
> +# @gicv3-kvm: GICv3 support with kernel irqchip
> +#
> +# Since: 2.6
> +##
> +{ 'enum': 'GICType', 'data': [ 'gicv2', 'gicv3', 'gicv2-kvm', 'gicv3-kvm' ] }
> 

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Qemu-devel] [PATCH 1/3] arm: gic: add GICType
  2016-03-01 14:20   ` Andrea Bolognani
@ 2016-03-02  3:34     ` Peter Xu
  2016-03-02  7:15       ` Peter Xu
  0 siblings, 1 reply; 15+ messages in thread
From: Peter Xu @ 2016-03-02  3:34 UTC (permalink / raw)
  To: Andrea Bolognani; +Cc: wei, peter.maydell, drjones, armbru, mdroth, qemu-devel

On Tue, Mar 01, 2016 at 03:20:59PM +0100, Andrea Bolognani wrote:
> On Tue, 2016-02-23 at 18:52 +0800, Peter Xu wrote:
> > +{ 'enum': 'GICType', 'data': [ 'gicv2', 'gicv3', 'gicv2-kvm', 'gicv3-kvm' ] }
> 
> Wouldn't this conflate the use of accel= and kernel_irqchip= options?

AFAIU, it's not a problem. Let me paste some lines from the original
RFC thread which explains the definition of the entries:

- gicv2:      GIC version 2 without kernel IRQ chip
- gicv2-kvm:  GIC version 2 with kernel IRQ chip
- gicv3:      GIC version 3 without kernel IRQ chip (not supported)
- gicv3-kvm:  GIC version 3 with kernel IRQ chip

(from https://lists.gnu.org/archive/html/qemu-devel/2016-02/msg02882.html)

So... what I understand is that, we are not talking about "accel="
at all. Instead, we are talking about "kernel_irqchip=" only. Or
say, all these GIC version information we provide from QEMU does not
tell whether KVM is supported or not (for "accel=", it is provided
by another QMP message named "query-kvm"). We are only talking about
which kind of GIC we support. In our case, for each version, it
could be supported either in userspace, or in kernel.

> 
> IIUC, depending on the hardware, you might find yourself in the
> following situation:
> 
>   accel=tcg,gic-version=3                     unavailable
>   accel=kvm,kernel_irqchip=off,gic-version=3  unavailable

As explained above, IIUC, both of these two "unavailable" ones
correspond to "gicv3" entry of the results.

>   accel=kvm,gic-version=3                     available

And this one corresponds to "gicv3-kvm" entry.

> 
> so I'd expect the output to be something like
> 
>   [ "v2": { "tcg": true,
>             "kvm-without-kernel-irqchip": true,
>             "kvm": true },
>     "v3": { "tcg": false,
>             "kvm-without-kernel-irqchip": false,
>             "kvm": true } ]

Actually, this reminded me about the "kernel_irqchip=split" case. Do
we need to consider that one? AFAIK, splitted irqchip is only used
for x86 currently. Whether ARM will possibly support splitted kernel
irqchip one day just like x86? If so, I would prefer to change the
query result layout from array to dict, like:

[ "v2": { "emulated": true,
          "split": false,
          "kernel": true },
  "v3": { "emulated": false,
          "split": false,
          "kernel": true } ]

Since the matrix is big enough (2x3) to consider drop the array
format (I'd admit maybe dict is always the best one...).

Peter

> 
> Since libvirt currently doesn't have support for the
> kernel_irqchip= option, it would only take the "tcg" and "kvm"
> values into account; on the other hand, if at some point
> libvirt will grow support for that option it would be able to
> retrieve all the required information.
> 
> Cheers.
> 
> -- 
> Andrea Bolognani
> Software Engineer - Virtualization Team

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Qemu-devel] [PATCH 1/3] arm: gic: add GICType
  2016-03-01 16:46   ` Eric Blake
@ 2016-03-02  4:55     ` Peter Xu
  0 siblings, 0 replies; 15+ messages in thread
From: Peter Xu @ 2016-03-02  4:55 UTC (permalink / raw)
  To: Eric Blake
  Cc: wei, peter.maydell, drjones, mdroth, qemu-devel, armbru, abologna

On Tue, Mar 01, 2016 at 09:46:07AM -0700, Eric Blake wrote:
> On 02/23/2016 03:52 AM, Peter Xu wrote:
> > A new enum type is added to define ARM GIC types.
> > 
> > Signed-off-by: Peter Xu <peterx@redhat.com>
> > ---
> >  qapi-schema.json | 17 +++++++++++++++++
> >  1 file changed, 17 insertions(+)
> > 
> > diff --git a/qapi-schema.json b/qapi-schema.json
> > index 8d04897..81654bd 100644
> > --- a/qapi-schema.json
> > +++ b/qapi-schema.json
> > @@ -4083,3 +4083,20 @@
> >  ##
> >  { 'enum': 'ReplayMode',
> >    'data': [ 'none', 'record', 'play' ] }
> > +
> > +##
> > +# @GICType:
> > +#
> > +# An enumeration of GIC types
> 
> Worth spelling out the acronym?

Sure. I can do it in next spin. 

> 
> > +#
> > +# @gicv2:     GICv2 support without kernel irqchip
> > +#
> > +# @gicv3:     GICv3 support without kernel irqchip
> > +#
> > +# @gicv2-kvm: GICv3 support with kernel irqchip
> 
> Doc typo, should be GICv2

Will fix. If dict to be used finally, will rework as a whole.

Thanks!
Peter

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Qemu-devel] [PATCH 1/3] arm: gic: add GICType
  2016-03-02  3:34     ` Peter Xu
@ 2016-03-02  7:15       ` Peter Xu
  2016-03-02  9:47         ` Markus Armbruster
  0 siblings, 1 reply; 15+ messages in thread
From: Peter Xu @ 2016-03-02  7:15 UTC (permalink / raw)
  To: Andrea Bolognani; +Cc: wei, peter.maydell, drjones, armbru, mdroth, qemu-devel

On Wed, Mar 02, 2016 at 11:34:44AM +0800, Peter Xu wrote:
> [ "v2": { "emulated": true,
>           "split": false,
>           "kernel": true },
>   "v3": { "emulated": false,
>           "split": false,
>           "kernel": true } ]

Or something like this:

[{
    "version": 2,
    "emulated": true,
    "split": false,
    "kernel": true
},
{
    "version": 3,
    "emulated": false,
    "split": false,
    "kernel": true
}]

If temporarily not considering kernel_irqchip=split case:

[{
    "version": 2,
    "emulated": true,
    "kernel": true
},
{
    "version": 3,
    "emulated": false,
    "kernel": true
}]

To use array rather than dict so that we do not need to change qapi
schema again when GICv4 comes.

Peter

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Qemu-devel] [PATCH 1/3] arm: gic: add GICType
  2016-03-02  7:15       ` Peter Xu
@ 2016-03-02  9:47         ` Markus Armbruster
  2016-03-02 10:55           ` Peter Xu
  0 siblings, 1 reply; 15+ messages in thread
From: Markus Armbruster @ 2016-03-02  9:47 UTC (permalink / raw)
  To: Peter Xu
  Cc: wei, peter.maydell, drjones, mdroth, qemu-devel, Andrea Bolognani

Peter Xu <peterx@redhat.com> writes:

> On Wed, Mar 02, 2016 at 11:34:44AM +0800, Peter Xu wrote:
>> [ "v2": { "emulated": true,
>>           "split": false,
>>           "kernel": true },
>>   "v3": { "emulated": false,
>>           "split": false,
>>           "kernel": true } ]
>
> Or something like this:
>
> [{
>     "version": 2,
>     "emulated": true,
>     "split": false,
>     "kernel": true
> },
> {
>     "version": 3,
>     "emulated": false,
>     "split": false,
>     "kernel": true
> }]
>
> If temporarily not considering kernel_irqchip=split case:
>
> [{
>     "version": 2,
>     "emulated": true,
>     "kernel": true
> },
> {
>     "version": 3,
>     "emulated": false,
>     "kernel": true
> }]
>
> To use array rather than dict so that we do not need to change qapi
> schema again when GICv4 comes.

Drive-by shooting without sufficient context: we may *want* to change
the QAPI schema, because that makes the change introspectable with
query-schema.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Qemu-devel] [PATCH 1/3] arm: gic: add GICType
  2016-03-02  9:47         ` Markus Armbruster
@ 2016-03-02 10:55           ` Peter Xu
  2016-03-02 13:59             ` Markus Armbruster
  0 siblings, 1 reply; 15+ messages in thread
From: Peter Xu @ 2016-03-02 10:55 UTC (permalink / raw)
  To: Markus Armbruster
  Cc: wei, peter.maydell, drjones, mdroth, qemu-devel, Andrea Bolognani

On Wed, Mar 02, 2016 at 10:47:39AM +0100, Markus Armbruster wrote:
> Peter Xu <peterx@redhat.com> writes:
> > If temporarily not considering kernel_irqchip=split case:
> >
> > [{
> >     "version": 2,
> >     "emulated": true,
> >     "kernel": true
> > },
> > {
> >     "version": 3,
> >     "emulated": false,
> >     "kernel": true
> > }]
> >
> > To use array rather than dict so that we do not need to change qapi
> > schema again when GICv4 comes.
> 
> Drive-by shooting without sufficient context: we may *want* to change
> the QAPI schema, because that makes the change introspectable with
> query-schema.

Failed to catch the point. :(

What's "query-schema"? Is that a QMP command?

What I meant is that, we can define the following (for example):

{ 'struct': 'GICCapInfo',
  'data': [
    'version': 'int',
    'emulated': 'bool',
    'kernel': 'bool'] }

And:

{ 'command': 'query-gic-capability',
  'returns': ['GICCapInfo'] }

So we can keep this schema as it is when new versions arrive. We
can just push another element in.

Peter

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Qemu-devel] [PATCH 1/3] arm: gic: add GICType
  2016-03-02 10:55           ` Peter Xu
@ 2016-03-02 13:59             ` Markus Armbruster
  2016-03-03  4:27               ` Peter Xu
  0 siblings, 1 reply; 15+ messages in thread
From: Markus Armbruster @ 2016-03-02 13:59 UTC (permalink / raw)
  To: Peter Xu
  Cc: wei, peter.maydell, drjones, qemu-devel, mdroth, Andrea Bolognani

Peter Xu <peterx@redhat.com> writes:

> On Wed, Mar 02, 2016 at 10:47:39AM +0100, Markus Armbruster wrote:
>> Peter Xu <peterx@redhat.com> writes:
>> > If temporarily not considering kernel_irqchip=split case:
>> >
>> > [{
>> >     "version": 2,
>> >     "emulated": true,
>> >     "kernel": true
>> > },
>> > {
>> >     "version": 3,
>> >     "emulated": false,
>> >     "kernel": true
>> > }]
>> >
>> > To use array rather than dict so that we do not need to change qapi
>> > schema again when GICv4 comes.
>> 
>> Drive-by shooting without sufficient context: we may *want* to change
>> the QAPI schema, because that makes the change introspectable with
>> query-schema.
>
> Failed to catch the point. :(
>
> What's "query-schema"? Is that a QMP command?

Yes.

More than you ever wanted to know:
http://events.linuxfoundation.org/sites/events/files/slides/armbru-qemu-introspection.pdf
https://www.youtube.com/watch?v=IEa8Ao8_B9o&list=PLW3ep1uCIRfyLNSu708gWG7uvqlolk0ep&index=28

> What I meant is that, we can define the following (for example):
>
> { 'struct': 'GICCapInfo',
>   'data': [
>     'version': 'int',
>     'emulated': 'bool',
>     'kernel': 'bool'] }
>
> And:
>
> { 'command': 'query-gic-capability',
>   'returns': ['GICCapInfo'] }
>
> So we can keep this schema as it is when new versions arrive. We
> can just push another element in.

To answer questions of the sort "can this QEMU version do X?", it's
often useful to tie X to a schema change that is visible in the result
of query-schema.

Often != always.  I'd rather not mess with the schema in unnatural ways
just to make something visible in query-schema.

Note that "can this *board* do X" is a different question.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Qemu-devel] [PATCH 1/3] arm: gic: add GICType
  2016-03-02 13:59             ` Markus Armbruster
@ 2016-03-03  4:27               ` Peter Xu
  2016-03-03  6:34                 ` Markus Armbruster
  0 siblings, 1 reply; 15+ messages in thread
From: Peter Xu @ 2016-03-03  4:27 UTC (permalink / raw)
  To: Markus Armbruster
  Cc: wei, peter.maydell, drjones, qemu-devel, mdroth, Andrea Bolognani

On Wed, Mar 02, 2016 at 02:59:57PM +0100, Markus Armbruster wrote:
> Peter Xu <peterx@redhat.com> writes:
> > What's "query-schema"? Is that a QMP command?
> 
> Yes.
> 
> More than you ever wanted to know:
> http://events.linuxfoundation.org/sites/events/files/slides/armbru-qemu-introspection.pdf
> https://www.youtube.com/watch?v=IEa8Ao8_B9o&list=PLW3ep1uCIRfyLNSu708gWG7uvqlolk0ep&index=28

Thanks for the pointers and cool slides! It's a good thing to pick
up. :-)

It'll be cool we treat data as codes, and codes as data.

I see that qapi-introspect branch is not there now. Is it merged to
some other branch already? When will it be there in QEMU master
(still not in, right?)? Just curious about it.

> 
> > What I meant is that, we can define the following (for example):
> >
> > { 'struct': 'GICCapInfo',
> >   'data': [
> >     'version': 'int',
> >     'emulated': 'bool',
> >     'kernel': 'bool'] }
> >
> > And:
> >
> > { 'command': 'query-gic-capability',
> >   'returns': ['GICCapInfo'] }
> >
> > So we can keep this schema as it is when new versions arrive. We
> > can just push another element in.
> 
> To answer questions of the sort "can this QEMU version do X?", it's
> often useful to tie X to a schema change that is visible in the result
> of query-schema.

Now I can understand. For this case, I guess both ways work, right?
Considering that if "query-schema" is still not there, I'd still
prefer the "array" solution. At least, it can keep the schema
several lines shorter (as you have mentioned already, it's *big*
enough :). Also, even we would have "query-schema", I would still
prefer not change schema unless necessary. What do you think?

Thanks!
Peter

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Qemu-devel] [PATCH 1/3] arm: gic: add GICType
  2016-03-03  4:27               ` Peter Xu
@ 2016-03-03  6:34                 ` Markus Armbruster
  2016-03-03  6:58                   ` Peter Xu
  0 siblings, 1 reply; 15+ messages in thread
From: Markus Armbruster @ 2016-03-03  6:34 UTC (permalink / raw)
  To: Peter Xu
  Cc: wei, peter.maydell, drjones, mdroth, qemu-devel, Andrea Bolognani

Peter Xu <peterx@redhat.com> writes:

> On Wed, Mar 02, 2016 at 02:59:57PM +0100, Markus Armbruster wrote:
>> Peter Xu <peterx@redhat.com> writes:
>> > What's "query-schema"? Is that a QMP command?
>> 
>> Yes.
>> 
>> More than you ever wanted to know:
>> http://events.linuxfoundation.org/sites/events/files/slides/armbru-qemu-introspection.pdf
>> https://www.youtube.com/watch?v=IEa8Ao8_B9o&list=PLW3ep1uCIRfyLNSu708gWG7uvqlolk0ep&index=28
>
> Thanks for the pointers and cool slides! It's a good thing to pick
> up. :-)
>
> It'll be cool we treat data as codes, and codes as data.
>
> I see that qapi-introspect branch is not there now. Is it merged to
> some other branch already? When will it be there in QEMU master
> (still not in, right?)? Just curious about it.

Merged in commit 9e72681.

Between the talk and the merge, query-schema got renamed to
query-qmp-schema.  Sometimes I relapse.  Sorry for the confusion!

>> > What I meant is that, we can define the following (for example):
>> >
>> > { 'struct': 'GICCapInfo',
>> >   'data': [
>> >     'version': 'int',
>> >     'emulated': 'bool',
>> >     'kernel': 'bool'] }
>> >
>> > And:
>> >
>> > { 'command': 'query-gic-capability',
>> >   'returns': ['GICCapInfo'] }
>> >
>> > So we can keep this schema as it is when new versions arrive. We
>> > can just push another element in.
>> 
>> To answer questions of the sort "can this QEMU version do X?", it's
>> often useful to tie X to a schema change that is visible in the result
>> of query-schema.
>
> Now I can understand. For this case, I guess both ways work, right?
> Considering that if "query-schema" is still not there, I'd still
> prefer the "array" solution. At least, it can keep the schema
> several lines shorter (as you have mentioned already, it's *big*
> enough :). Also, even we would have "query-schema", I would still
> prefer not change schema unless necessary. What do you think?

I can't say without understanding what the introspection question would
be.  That needs actual thought, which is in short supply, especially
before breakfast ;)

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [Qemu-devel] [PATCH 1/3] arm: gic: add GICType
  2016-03-03  6:34                 ` Markus Armbruster
@ 2016-03-03  6:58                   ` Peter Xu
  0 siblings, 0 replies; 15+ messages in thread
From: Peter Xu @ 2016-03-03  6:58 UTC (permalink / raw)
  To: Markus Armbruster
  Cc: wei, peter.maydell, drjones, mdroth, qemu-devel, Andrea Bolognani

On Thu, Mar 03, 2016 at 07:34:21AM +0100, Markus Armbruster wrote:
> Peter Xu <peterx@redhat.com> writes:
> > I see that qapi-introspect branch is not there now. Is it merged to
> > some other branch already? When will it be there in QEMU master
> > (still not in, right?)? Just curious about it.
> 
> Merged in commit 9e72681.
> 
> Between the talk and the merge, query-schema got renamed to
> query-qmp-schema.  Sometimes I relapse.  Sorry for the confusion!

Got it!

> > Now I can understand. For this case, I guess both ways work, right?
> > Considering that if "query-schema" is still not there, I'd still
> > prefer the "array" solution. At least, it can keep the schema
> > several lines shorter (as you have mentioned already, it's *big*
> > enough :). Also, even we would have "query-schema", I would still
> > prefer not change schema unless necessary. What do you think?
> 
> I can't say without understanding what the introspection question would
> be.  That needs actual thought, which is in short supply, especially
> before breakfast ;)

Yah, anyway, looking forward to further review comments! For now,
maybe I can start to work on v2 if no big problem. If there is
better way, v3 is ready to go. :)

Peter

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2016-03-03  6:58 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-02-23 10:52 [Qemu-devel] [PATCH 0/3] ARM: add query-gic-capability SMP command Peter Xu
2016-02-23 10:52 ` [Qemu-devel] [PATCH 1/3] arm: gic: add GICType Peter Xu
2016-03-01 14:20   ` Andrea Bolognani
2016-03-02  3:34     ` Peter Xu
2016-03-02  7:15       ` Peter Xu
2016-03-02  9:47         ` Markus Armbruster
2016-03-02 10:55           ` Peter Xu
2016-03-02 13:59             ` Markus Armbruster
2016-03-03  4:27               ` Peter Xu
2016-03-03  6:34                 ` Markus Armbruster
2016-03-03  6:58                   ` Peter Xu
2016-03-01 16:46   ` Eric Blake
2016-03-02  4:55     ` Peter Xu
2016-02-23 10:52 ` [Qemu-devel] [PATCH 2/3] arm: gic: add "query-gic-capability" interface Peter Xu
2016-02-23 10:52 ` [Qemu-devel] [PATCH 3/3] arm: implement query-gic-capability Peter Xu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).