qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [RFC PATCH 0/2] ARM: add QMP command to query GIC version
@ 2016-02-14  5:41 Peter Xu
  2016-02-14  5:41 ` [Qemu-devel] [RFC PATCH 1/2] arm: gic: add GICType Peter Xu
                   ` (4 more replies)
  0 siblings, 5 replies; 36+ messages in thread
From: Peter Xu @ 2016-02-14  5:41 UTC (permalink / raw)
  To: qemu-devel, libvir-list; +Cc: wei, drjones, abologna, peterx

For ARM platform, we still do not have any interface to query
whether current QEMU/host support specific GIC version. This
patchset is trying to add one QMP interface for that. By querying
the GIC capability using the new interface, one should know exactly
what GIC version(s) the platform will support. The capability bits
will be decided by both QEMU and host kernel.

The current patchset only provides interface for review. Its handler
is a fake one which returns empty always.

The command interface I am planning to add is something like this:

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

Currently, all the possible supported GIC versions are:

- 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

Since "gicv3" is still not supported (to use GICv3, kernel irqchip
support is required for now, which corresponds to "gicv3-kvm"),
currently the maximum superset of the result should be:

["gicv2", "gicv2-kvm", "gicv3-kvm"]

Please help review whether the interface suits our need, also please
point out any error I have made.

One question: how should I make this command "ARM only"? I see that
in qmp-commands.hx, I can use something like "#if defined
TARGET_ARM" to block out ARM specified commands, however how should
I do the similiar thing in qapi-schema.json?

Thanks!
Peter

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

 qapi-schema.json | 28 ++++++++++++++++++++++++++++
 qmp-commands.hx  | 25 +++++++++++++++++++++++++
 qmp.c            |  5 +++++
 scripts/qapi.py  |  1 +
 4 files changed, 59 insertions(+)

-- 
2.4.3

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

* [Qemu-devel] [RFC PATCH 1/2] arm: gic: add GICType
  2016-02-14  5:41 [Qemu-devel] [RFC PATCH 0/2] ARM: add QMP command to query GIC version Peter Xu
@ 2016-02-14  5:41 ` Peter Xu
  2016-02-14  5:41 ` [Qemu-devel] [RFC PATCH 2/2] arm: gic: add "query-gic-capability" interface Peter Xu
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 36+ messages in thread
From: Peter Xu @ 2016-02-14  5:41 UTC (permalink / raw)
  To: qemu-devel, libvir-list; +Cc: wei, drjones, abologna, peterx

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] 36+ messages in thread

* [Qemu-devel] [RFC PATCH 2/2] arm: gic: add "query-gic-capability" interface
  2016-02-14  5:41 [Qemu-devel] [RFC PATCH 0/2] ARM: add QMP command to query GIC version Peter Xu
  2016-02-14  5:41 ` [Qemu-devel] [RFC PATCH 1/2] arm: gic: add GICType Peter Xu
@ 2016-02-14  5:41 ` Peter Xu
  2016-02-15  6:54 ` [Qemu-devel] [RFC PATCH 0/2] ARM: add QMP command to query GIC version Wei Huang
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 36+ messages in thread
From: Peter Xu @ 2016-02-14  5:41 UTC (permalink / raw)
  To: qemu-devel, libvir-list; +Cc: wei, drjones, abologna, peterx

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

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/qmp.c b/qmp.c
index 6ae7230..9ad74c7 100644
--- a/qmp.c
+++ b/qmp.c
@@ -753,3 +753,8 @@ ACPIOSTInfoList *qmp_query_acpi_ospm_status(Error **errp)
 
     return head;
 }
+
+GICTypeList *qmp_query_gic_capability(Error **errp)
+{
+    return NULL;
+}
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',
-- 
2.4.3

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

* Re: [Qemu-devel] [RFC PATCH 0/2] ARM: add QMP command to query GIC version
  2016-02-14  5:41 [Qemu-devel] [RFC PATCH 0/2] ARM: add QMP command to query GIC version Peter Xu
  2016-02-14  5:41 ` [Qemu-devel] [RFC PATCH 1/2] arm: gic: add GICType Peter Xu
  2016-02-14  5:41 ` [Qemu-devel] [RFC PATCH 2/2] arm: gic: add "query-gic-capability" interface Peter Xu
@ 2016-02-15  6:54 ` Wei Huang
  2016-02-15  7:34   ` Peter Xu
  2016-02-15  9:35 ` [Qemu-devel] [libvirt] " Martin Kletzander
  2016-02-15  9:52 ` [Qemu-devel] " Markus Armbruster
  4 siblings, 1 reply; 36+ messages in thread
From: Wei Huang @ 2016-02-15  6:54 UTC (permalink / raw)
  To: Peter Xu, qemu-devel, libvir-list; +Cc: drjones, abologna



On 2/13/16 23:41, Peter Xu wrote:
> For ARM platform, we still do not have any interface to query
> whether current QEMU/host support specific GIC version. This
> patchset is trying to add one QMP interface for that. By querying
> the GIC capability using the new interface, one should know exactly
> what GIC version(s) the platform will support. The capability bits
> will be decided by both QEMU and host kernel.
> 
> The current patchset only provides interface for review. Its handler
> is a fake one which returns empty always.
> 
> The command interface I am planning to add is something like this:
> 
> -> { "execute": "query-gic-capability" }
> <- { "return": [ "gicv2", "gicv2-kvm", "gicv3-kvm" ] }
> 
> Currently, all the possible supported GIC versions are:
> 
> - 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
> 
> Since "gicv3" is still not supported (to use GICv3, kernel irqchip
> support is required for now, which corresponds to "gicv3-kvm"),
> currently the maximum superset of the result should be:
> 
> ["gicv2", "gicv2-kvm", "gicv3-kvm"]
> 
> Please help review whether the interface suits our need, also please
> point out any error I have made.

I tested QEMU with these patches and they were able to work on a native
ARM64 machine.

> 
> One question: how should I make this command "ARM only"? I see that
> in qmp-commands.hx, I can use something like "#if defined
> TARGET_ARM" to block out ARM specified commands, however how should
> I do the similiar thing in qapi-schema.json?

This situation is similar to "rtc-reset-reinjection", right? I think You
can disable this feature on other arch's by setting
QERR_FEATURE_DISABLED in file like monitor.c.

> 
> Thanks!
> Peter
> 
> Peter Xu (2):
>   arm: gic: add GICType
>   arm: gic: add "query-gic-capability" interface
> 
>  qapi-schema.json | 28 ++++++++++++++++++++++++++++
>  qmp-commands.hx  | 25 +++++++++++++++++++++++++
>  qmp.c            |  5 +++++
>  scripts/qapi.py  |  1 +
>  4 files changed, 59 insertions(+)
> 

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

* Re: [Qemu-devel] [RFC PATCH 0/2] ARM: add QMP command to query GIC version
  2016-02-15  6:54 ` [Qemu-devel] [RFC PATCH 0/2] ARM: add QMP command to query GIC version Wei Huang
@ 2016-02-15  7:34   ` Peter Xu
  2016-02-15  7:49     ` Fam Zheng
  0 siblings, 1 reply; 36+ messages in thread
From: Peter Xu @ 2016-02-15  7:34 UTC (permalink / raw)
  To: Wei Huang; +Cc: libvir-list, drjones, qemu-devel, abologna

On Mon, Feb 15, 2016 at 12:54:57AM -0600, Wei Huang wrote:
> On 2/13/16 23:41, Peter Xu wrote:
> > Please help review whether the interface suits our need, also please
> > point out any error I have made.
> 
> I tested QEMU with these patches and they were able to work on a native
> ARM64 machine.

Thanks for the verification. :)

> 
> > 
> > One question: how should I make this command "ARM only"? I see that
> > in qmp-commands.hx, I can use something like "#if defined
> > TARGET_ARM" to block out ARM specified commands, however how should
> > I do the similiar thing in qapi-schema.json?
> 
> This situation is similar to "rtc-reset-reinjection", right? I think You
> can disable this feature on other arch's by setting
> QERR_FEATURE_DISABLED in file like monitor.c.

Yes. Will include this when post the real patches.

Thanks!
Peter

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

* Re: [Qemu-devel] [RFC PATCH 0/2] ARM: add QMP command to query GIC version
  2016-02-15  7:34   ` Peter Xu
@ 2016-02-15  7:49     ` Fam Zheng
  0 siblings, 0 replies; 36+ messages in thread
From: Fam Zheng @ 2016-02-15  7:49 UTC (permalink / raw)
  To: Peter Xu; +Cc: Wei Huang, drjones, libvir-list, qemu-devel, armbru, abologna

On Mon, 02/15 15:34, Peter Xu wrote:
> On Mon, Feb 15, 2016 at 12:54:57AM -0600, Wei Huang wrote:
> > On 2/13/16 23:41, Peter Xu wrote:
> > > One question: how should I make this command "ARM only"? I see that
> > > in qmp-commands.hx, I can use something like "#if defined
> > > TARGET_ARM" to block out ARM specified commands, however how should
> > > I do the similiar thing in qapi-schema.json?
> > 
> > This situation is similar to "rtc-reset-reinjection", right? I think You
> > can disable this feature on other arch's by setting
> > QERR_FEATURE_DISABLED in file like monitor.c.
> 
> Yes. Will include this when post the real patches.

Yes, I think returning an error for non-ARM targets suffices.

Fam

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

* Re: [Qemu-devel] [libvirt] [RFC PATCH 0/2] ARM: add QMP command to query GIC version
  2016-02-14  5:41 [Qemu-devel] [RFC PATCH 0/2] ARM: add QMP command to query GIC version Peter Xu
                   ` (2 preceding siblings ...)
  2016-02-15  6:54 ` [Qemu-devel] [RFC PATCH 0/2] ARM: add QMP command to query GIC version Wei Huang
@ 2016-02-15  9:35 ` Martin Kletzander
  2016-02-15  9:41   ` Peter Maydell
  2016-02-15 10:09   ` Peter Xu
  2016-02-15  9:52 ` [Qemu-devel] " Markus Armbruster
  4 siblings, 2 replies; 36+ messages in thread
From: Martin Kletzander @ 2016-02-15  9:35 UTC (permalink / raw)
  To: Peter Xu; +Cc: libvir-list, wei, drjones, qemu-devel, abologna

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

On Sun, Feb 14, 2016 at 01:41:41PM +0800, Peter Xu wrote:
>For ARM platform, we still do not have any interface to query
>whether current QEMU/host support specific GIC version. This
>patchset is trying to add one QMP interface for that. By querying
>the GIC capability using the new interface, one should know exactly
>what GIC version(s) the platform will support. The capability bits
>will be decided by both QEMU and host kernel.
>
>The current patchset only provides interface for review. Its handler
>is a fake one which returns empty always.
>
>The command interface I am planning to add is something like this:
>
>-> { "execute": "query-gic-capability" }
><- { "return": [ "gicv2", "gicv2-kvm", "gicv3-kvm" ] }
>
>Currently, all the possible supported GIC versions are:
>
>- 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
>
>Since "gicv3" is still not supported (to use GICv3, kernel irqchip
>support is required for now, which corresponds to "gicv3-kvm"),
>currently the maximum superset of the result should be:
>
>["gicv2", "gicv2-kvm", "gicv3-kvm"]
>
>Please help review whether the interface suits our need, also please
>point out any error I have made.
>

This looks nice.  I have some questions, but I'm not an expert in this
area, so excuse me if they are stupid.

So hardware itself supports some GIC version, let's say 3 for our case.
Does that mean it can be triggered to do v2 as well?  I mean is it
possible that HW supports multiple versions?  If yes, then I suspect
there is (will be) HW that does *not* do it and that's where QEMU (or
KVM) must emulate that version.  If all I'm having in mind is true, then
you are trying to reply with two orthogonal types of information. A)
versions kernel/HW supports and B) qemu/kvm can emulate.  That is fine
if libvirt can choose all the versions specified (e.g. gicv2-kvm,
gicv2), but if we can only select a version, then it might be worth just
returning those two types of information separately, e.g.:

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

But as I said, I don't know the pre-requisites for this and mainly I'm
not dealing with arm support now in libvirt, I'm just curious about it.

>One question: how should I make this command "ARM only"? I see that
>in qmp-commands.hx, I can use something like "#if defined
>TARGET_ARM" to block out ARM specified commands, however how should
>I do the similiar thing in qapi-schema.json?
>

As mentioned in the other thread, making it available everywhere and
just returning a proper error message (so we can parse the class and not
the error message itself) is the best choice, IMHO.

>Thanks!
>Peter
>
>Peter Xu (2):
>  arm: gic: add GICType
>  arm: gic: add "query-gic-capability" interface
>
> qapi-schema.json | 28 ++++++++++++++++++++++++++++
> qmp-commands.hx  | 25 +++++++++++++++++++++++++
> qmp.c            |  5 +++++
> scripts/qapi.py  |  1 +
> 4 files changed, 59 insertions(+)
>
>--
>2.4.3
>
>--
>libvir-list mailing list
>libvir-list@redhat.com
>https://www.redhat.com/mailman/listinfo/libvir-list

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

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

* Re: [Qemu-devel] [libvirt] [RFC PATCH 0/2] ARM: add QMP command to query GIC version
  2016-02-15  9:35 ` [Qemu-devel] [libvirt] " Martin Kletzander
@ 2016-02-15  9:41   ` Peter Maydell
  2016-02-15 12:16     ` Andrew Jones
  2016-02-15 10:09   ` Peter Xu
  1 sibling, 1 reply; 36+ messages in thread
From: Peter Maydell @ 2016-02-15  9:41 UTC (permalink / raw)
  To: Martin Kletzander
  Cc: Wei Huang, Andrew Jones, Libvirt, QEMU Developers, Peter Xu,
	Andrea Bolognani

On 15 February 2016 at 09:35, Martin Kletzander <mkletzan@redhat.com> wrote:
> So hardware itself supports some GIC version, let's say 3 for our case.
> Does that mean it can be triggered to do v2 as well?  I mean is it
> possible that HW supports multiple versions?  If yes, then I suspect
> there is (will be) HW that does *not* do it

Hardware may be:
 GICv2 only
 GICv3 only
 GICv3 with v2 backwards-compatibility support

> and that's where QEMU (or
> KVM) must emulate that version.

If the hardware doesn't support the version that we are trying
to provide to the guest then we can't run KVM at all. (KVM
with an emulated interrupt controller is not a supported setup,
because you wouldn't get CPU timer interrupts in the guest.)

thanks
-- PMM

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

* Re: [Qemu-devel] [RFC PATCH 0/2] ARM: add QMP command to query GIC version
  2016-02-14  5:41 [Qemu-devel] [RFC PATCH 0/2] ARM: add QMP command to query GIC version Peter Xu
                   ` (3 preceding siblings ...)
  2016-02-15  9:35 ` [Qemu-devel] [libvirt] " Martin Kletzander
@ 2016-02-15  9:52 ` Markus Armbruster
  2016-02-15 10:34   ` Peter Xu
  4 siblings, 1 reply; 36+ messages in thread
From: Markus Armbruster @ 2016-02-15  9:52 UTC (permalink / raw)
  To: Peter Xu; +Cc: libvir-list, wei, drjones, qemu-devel, abologna

Peter Xu <peterx@redhat.com> writes:

> For ARM platform, we still do not have any interface to query
> whether current QEMU/host support specific GIC version. This
> patchset is trying to add one QMP interface for that. By querying
> the GIC capability using the new interface, one should know exactly
> what GIC version(s) the platform will support. The capability bits
> will be decided by both QEMU and host kernel.
>
> The current patchset only provides interface for review. Its handler
> is a fake one which returns empty always.
>
> The command interface I am planning to add is something like this:
>
> -> { "execute": "query-gic-capability" }
> <- { "return": [ "gicv2", "gicv2-kvm", "gicv3-kvm" ] }
>
> Currently, all the possible supported GIC versions are:
>
> - 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
>
> Since "gicv3" is still not supported (to use GICv3, kernel irqchip
> support is required for now, which corresponds to "gicv3-kvm"),
> currently the maximum superset of the result should be:
>
> ["gicv2", "gicv2-kvm", "gicv3-kvm"]
>
> Please help review whether the interface suits our need, also please
> point out any error I have made.

Adding ad hoc queries as we go won't scale.  Is there really no generic
way to get this information, e.g. with qom-get?

> One question: how should I make this command "ARM only"? I see that
> in qmp-commands.hx, I can use something like "#if defined
> TARGET_ARM" to block out ARM specified commands, however how should
> I do the similiar thing in qapi-schema.json?

Have a look at the #if in qmp-commands.hx.

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

* Re: [Qemu-devel] [libvirt] [RFC PATCH 0/2] ARM: add QMP command to query GIC version
  2016-02-15  9:35 ` [Qemu-devel] [libvirt] " Martin Kletzander
  2016-02-15  9:41   ` Peter Maydell
@ 2016-02-15 10:09   ` Peter Xu
  1 sibling, 0 replies; 36+ messages in thread
From: Peter Xu @ 2016-02-15 10:09 UTC (permalink / raw)
  To: Martin Kletzander; +Cc: libvir-list, wei, drjones, qemu-devel, abologna

On Mon, Feb 15, 2016 at 10:35:39AM +0100, Martin Kletzander wrote:
> On Sun, Feb 14, 2016 at 01:41:41PM +0800, Peter Xu wrote:
> >
> >["gicv2", "gicv2-kvm", "gicv3-kvm"]
> >
> >Please help review whether the interface suits our need, also please
> >point out any error I have made.
> >
> 
> This looks nice.  I have some questions, but I'm not an expert in this
> area, so excuse me if they are stupid.
> 
> So hardware itself supports some GIC version, let's say 3 for our case.
> Does that mean it can be triggered to do v2 as well?  I mean is it
> possible that HW supports multiple versions?  If yes, then I suspect
> there is (will be) HW that does *not* do it and that's where QEMU (or
> KVM) must emulate that version.  If all I'm having in mind is true, then
> you are trying to reply with two orthogonal types of information. A)
> versions kernel/HW supports and B) qemu/kvm can emulate.  That is fine
> if libvirt can choose all the versions specified (e.g. gicv2-kvm,
> gicv2), but if we can only select a version, then it might be worth just
> returning those two types of information separately, e.g.:
> 
> ["v2": {"emulated": true, "kvm":true}, "v3": {"emulated": false, "kvm": true}]

Yes, I'd say that this is more clear especially when the matrix is
very big. Luckily for GIC versions, there is only 2x2 (2x3 if there
is v4) and it keeps a small one. So IMHO this is a flavor issue and
both work for me. :)

Thanks.
Peter

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

* Re: [Qemu-devel] [RFC PATCH 0/2] ARM: add QMP command to query GIC version
  2016-02-15  9:52 ` [Qemu-devel] " Markus Armbruster
@ 2016-02-15 10:34   ` Peter Xu
  2016-02-15 15:08     ` Markus Armbruster
  0 siblings, 1 reply; 36+ messages in thread
From: Peter Xu @ 2016-02-15 10:34 UTC (permalink / raw)
  To: Markus Armbruster; +Cc: libvir-list, wei, drjones, qemu-devel, abologna

On Mon, Feb 15, 2016 at 10:52:01AM +0100, Markus Armbruster wrote:
> Peter Xu <peterx@redhat.com> writes:
> 
> > For ARM platform, we still do not have any interface to query
> > whether current QEMU/host support specific GIC version. This
> > patchset is trying to add one QMP interface for that. By querying
> > the GIC capability using the new interface, one should know exactly
> > what GIC version(s) the platform will support. The capability bits
> > will be decided by both QEMU and host kernel.
> >
> > The current patchset only provides interface for review. Its handler
> > is a fake one which returns empty always.
> >
> > The command interface I am planning to add is something like this:
> >
> > -> { "execute": "query-gic-capability" }
> > <- { "return": [ "gicv2", "gicv2-kvm", "gicv3-kvm" ] }
> >
> > Currently, all the possible supported GIC versions are:
> >
> > - 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
> >
> > Since "gicv3" is still not supported (to use GICv3, kernel irqchip
> > support is required for now, which corresponds to "gicv3-kvm"),
> > currently the maximum superset of the result should be:
> >
> > ["gicv2", "gicv2-kvm", "gicv3-kvm"]
> >
> > Please help review whether the interface suits our need, also please
> > point out any error I have made.
> 
> Adding ad hoc queries as we go won't scale.  Is there really no generic
> way to get this information, e.g. with qom-get?

Haven't used "qom-get" before, but it seems to fetch one property
for a specific object. If so, will it be strange to hide some
capability bits into every GIC objects (though there is possibly
one object)?

I agree that we should keep the interface as simple as
possible. I see that there are already commands that works just like
this one, which is to query some capabilities from QEMU, like:

- query-dump-guest-memory-capability
- query-migrate-capabilities

So... besides the original proposal, what about adding a generic QMP
command to query all kinds of capabilities (and let GIC be the first
item)? Or any other way to avoid adding a new command?

Thanks.
Peter

> 
> > One question: how should I make this command "ARM only"? I see that
> > in qmp-commands.hx, I can use something like "#if defined
> > TARGET_ARM" to block out ARM specified commands, however how should
> > I do the similiar thing in qapi-schema.json?
> 
> Have a look at the #if in qmp-commands.hx.

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

* Re: [Qemu-devel] [libvirt] [RFC PATCH 0/2] ARM: add QMP command to query GIC version
  2016-02-15  9:41   ` Peter Maydell
@ 2016-02-15 12:16     ` Andrew Jones
  2016-02-15 12:27       ` Pavel Fedin
  0 siblings, 1 reply; 36+ messages in thread
From: Andrew Jones @ 2016-02-15 12:16 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Wei Huang, Libvirt, p.fedin, QEMU Developers, Peter Xu,
	Andrea Bolognani, Martin Kletzander

On Mon, Feb 15, 2016 at 09:41:57AM +0000, Peter Maydell wrote:
> On 15 February 2016 at 09:35, Martin Kletzander <mkletzan@redhat.com> wrote:
> > So hardware itself supports some GIC version, let's say 3 for our case.
> > Does that mean it can be triggered to do v2 as well?  I mean is it
> > possible that HW supports multiple versions?  If yes, then I suspect
> > there is (will be) HW that does *not* do it
> 
> Hardware may be:
>  GICv2 only
>  GICv3 only
>  GICv3 with v2 backwards-compatibility support
> 
> > and that's where QEMU (or
> > KVM) must emulate that version.
> 
> If the hardware doesn't support the version that we are trying
> to provide to the guest then we can't run KVM at all. (KVM
> with an emulated interrupt controller is not a supported setup,
> because you wouldn't get CPU timer interrupts in the guest.)

I know Pavel Fedin was trying to revive kernel_irqchip=off once,
but I don't know if that effort was abandoned or not. I think it
could be a nice-to-have, in order to help isolate bugs with KVM,
but I agree running that way wouldn't be the norm.

Thanks,
drew

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

* Re: [Qemu-devel] [libvirt] [RFC PATCH 0/2] ARM: add QMP command to query GIC version
  2016-02-15 12:16     ` Andrew Jones
@ 2016-02-15 12:27       ` Pavel Fedin
  0 siblings, 0 replies; 36+ messages in thread
From: Pavel Fedin @ 2016-02-15 12:27 UTC (permalink / raw)
  To: 'Andrew Jones', 'Peter Maydell'
  Cc: 'Wei Huang', 'Libvirt', 'QEMU Developers',
	'Peter Xu', 'Andrea Bolognani',
	'Martin Kletzander'

 Hello!

> I know Pavel Fedin was trying to revive kernel_irqchip=off once,
> but I don't know if that effort was abandoned or not.

 It should work with the latest kernel, at least i posted patches and all of them were applied. If nothing got broken during later
rewrites.
 The only missing part is generic timer support. There were problems with it, however, after rewrite, they can be clearly addressed,
without need for any hacks. The following patchset implements this on kernel side, but it has never been reviewed:
http://www.spinics.net/lists/kvm/msg124539.html. I also have qemu support in my experimental tree and it works great, i can run
"virt" guest on a Samsung's proprietary board with FrankenGIC, but since there was no interest, i never polished it up and
published.

> I think it
> could be a nice-to-have, in order to help isolate bugs with KVM,
> but I agree running that way wouldn't be the norm.

 IMHO it depends on what you want to achieve. If you strive for performance, then yes, of course. But, if you want to emulate some
particular hardware on another hardware, then this can be the only way to do it if, for example, you have GICv3-only hardware. KVM
without irqchip is still much better than TCG.

 But yes, i never included it into Libvirt. Once i was thinking about something like <gic version=off>, but perhaps it's not good
idea because this option is not ARM-specific, it's architecture-agnostic and applicable to any KVM acceleration for IRQ controller.
Whether it works or not for the given platform, it is IMHO different story.

Kind regards,
Pavel Fedin
Senior Engineer
Samsung Electronics Research center Russia

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

* Re: [Qemu-devel] [RFC PATCH 0/2] ARM: add QMP command to query GIC version
  2016-02-15 10:34   ` Peter Xu
@ 2016-02-15 15:08     ` Markus Armbruster
  2016-02-15 15:21       ` Peter Maydell
  2016-02-15 15:22       ` [Qemu-devel] " Daniel P. Berrange
  0 siblings, 2 replies; 36+ messages in thread
From: Markus Armbruster @ 2016-02-15 15:08 UTC (permalink / raw)
  To: Peter Xu; +Cc: libvir-list, wei, drjones, qemu-devel, abologna

Peter Xu <peterx@redhat.com> writes:

> On Mon, Feb 15, 2016 at 10:52:01AM +0100, Markus Armbruster wrote:
>> Peter Xu <peterx@redhat.com> writes:
>> 
>> > For ARM platform, we still do not have any interface to query
>> > whether current QEMU/host support specific GIC version. This
>> > patchset is trying to add one QMP interface for that. By querying
>> > the GIC capability using the new interface, one should know exactly
>> > what GIC version(s) the platform will support. The capability bits
>> > will be decided by both QEMU and host kernel.
>> >
>> > The current patchset only provides interface for review. Its handler
>> > is a fake one which returns empty always.
>> >
>> > The command interface I am planning to add is something like this:
>> >
>> > -> { "execute": "query-gic-capability" }
>> > <- { "return": [ "gicv2", "gicv2-kvm", "gicv3-kvm" ] }
>> >
>> > Currently, all the possible supported GIC versions are:
>> >
>> > - 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
>> >
>> > Since "gicv3" is still not supported (to use GICv3, kernel irqchip
>> > support is required for now, which corresponds to "gicv3-kvm"),
>> > currently the maximum superset of the result should be:
>> >
>> > ["gicv2", "gicv2-kvm", "gicv3-kvm"]
>> >
>> > Please help review whether the interface suits our need, also please
>> > point out any error I have made.
>> 
>> Adding ad hoc queries as we go won't scale.  Is there really no generic
>> way to get this information, e.g. with qom-get?
>
> Haven't used "qom-get" before, but it seems to fetch one property
> for a specific object. If so, will it be strange to hide some
> capability bits into every GIC objects (though there is possibly
> one object)?

Pardon my ignorance...  what are these "GIC objects"?

What exactly is the "GIC type", and how would the result of
query-gic-capability be used?

> I agree that we should keep the interface as simple as
> possible. I see that there are already commands that works just like
> this one, which is to query some capabilities from QEMU, like:
>
> - query-dump-guest-memory-capability
> - query-migrate-capabilities

I'm not saying we must not add another ad hoc query.  I'm trying to
figure out whether existing generic infrastructure can serve, or be
adapted to serve.  Once we establish the answer is "no" or "badly", I'm
willing to consider the ad hoc query.

> So... besides the original proposal, what about adding a generic QMP
> command to query all kinds of capabilities (and let GIC be the first
> item)? Or any other way to avoid adding a new command?

[...]

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

* Re: [Qemu-devel] [RFC PATCH 0/2] ARM: add QMP command to query GIC version
  2016-02-15 15:08     ` Markus Armbruster
@ 2016-02-15 15:21       ` Peter Maydell
  2016-02-15 19:40         ` Markus Armbruster
  2016-02-15 15:22       ` [Qemu-devel] " Daniel P. Berrange
  1 sibling, 1 reply; 36+ messages in thread
From: Peter Maydell @ 2016-02-15 15:21 UTC (permalink / raw)
  To: Markus Armbruster
  Cc: Wei Huang, Andrew Jones, Libvirt, QEMU Developers, Peter Xu,
	Andrea Bolognani

On 15 February 2016 at 15:08, Markus Armbruster <armbru@redhat.com> wrote:
> Peter Xu <peterx@redhat.com> writes:
>> On Mon, Feb 15, 2016 at 10:52:01AM +0100, Markus Armbruster wrote:
>>> Peter Xu <peterx@redhat.com> writes:
>>> Adding ad hoc queries as we go won't scale.  Is there really no generic
>>> way to get this information, e.g. with qom-get?
>>
>> Haven't used "qom-get" before, but it seems to fetch one property
>> for a specific object. If so, will it be strange to hide some
>> capability bits into every GIC objects (though there is possibly
>> one object)?
>
> Pardon my ignorance...  what are these "GIC objects"?
>
> What exactly is the "GIC type", and how would the result of
> query-gic-capability be used?

The GIC type (for our purposes) is the revision of the interrupt
controller supported by the host, which comes in two versions
(v2 and v3). These are not compatible, unless your host has
the v3-with-v2-compat flavour. If a host is v3-only, it is not
possible for it to give the guest a v2 virtual interrupt
controller; if v2, it can't give the guest a v3 virtual interrupt
controller. (If you ask QEMU to do this via command line options
we will report an error at startup.)

The underlying aim is to allow libvirt to say "this VM config
won't work on this host", rather than ploughing blindly on
and creating a VM config that always errors on startup.

The "GIC object" presumably is the GIC QOM device object.
However we do the "does this host support this GIC version?"
check in QEMU before we ever create the GIC device object,
so trying to probe it for properties isn't going to work.

The only other QOM object potentially available to probe would be
the machine (board) object. However as I understand it libvirt
does all its probing with the "none" machine type, and it seems
a bit odd to put a bunch of properties on the "none" machine
type. It would be possible though I guess.

thanks
-- PMM

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

* Re: [Qemu-devel] [RFC PATCH 0/2] ARM: add QMP command to query GIC version
  2016-02-15 15:08     ` Markus Armbruster
  2016-02-15 15:21       ` Peter Maydell
@ 2016-02-15 15:22       ` Daniel P. Berrange
  2016-02-18  4:40         ` Peter Xu
  1 sibling, 1 reply; 36+ messages in thread
From: Daniel P. Berrange @ 2016-02-15 15:22 UTC (permalink / raw)
  To: Markus Armbruster
  Cc: wei, drjones, libvir-list, qemu-devel, Peter Xu, abologna

On Mon, Feb 15, 2016 at 04:08:33PM +0100, Markus Armbruster wrote:
> Peter Xu <peterx@redhat.com> writes:
> 
> > On Mon, Feb 15, 2016 at 10:52:01AM +0100, Markus Armbruster wrote:
> >> Peter Xu <peterx@redhat.com> writes:
> >> 
> >> > For ARM platform, we still do not have any interface to query
> >> > whether current QEMU/host support specific GIC version. This
> >> > patchset is trying to add one QMP interface for that. By querying
> >> > the GIC capability using the new interface, one should know exactly
> >> > what GIC version(s) the platform will support. The capability bits
> >> > will be decided by both QEMU and host kernel.
> >> >
> >> > The current patchset only provides interface for review. Its handler
> >> > is a fake one which returns empty always.
> >> >
> >> > The command interface I am planning to add is something like this:
> >> >
> >> > -> { "execute": "query-gic-capability" }
> >> > <- { "return": [ "gicv2", "gicv2-kvm", "gicv3-kvm" ] }
> >> >
> >> > Currently, all the possible supported GIC versions are:
> >> >
> >> > - 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
> >> >
> >> > Since "gicv3" is still not supported (to use GICv3, kernel irqchip
> >> > support is required for now, which corresponds to "gicv3-kvm"),
> >> > currently the maximum superset of the result should be:
> >> >
> >> > ["gicv2", "gicv2-kvm", "gicv3-kvm"]
> >> >
> >> > Please help review whether the interface suits our need, also please
> >> > point out any error I have made.
> >> 
> >> Adding ad hoc queries as we go won't scale.  Is there really no generic
> >> way to get this information, e.g. with qom-get?
> >
> > Haven't used "qom-get" before, but it seems to fetch one property
> > for a specific object. If so, will it be strange to hide some
> > capability bits into every GIC objects (though there is possibly
> > one object)?
> 
> Pardon my ignorance...  what are these "GIC objects"?
> 
> What exactly is the "GIC type", and how would the result of
> query-gic-capability be used?
> 
> > I agree that we should keep the interface as simple as
> > possible. I see that there are already commands that works just like
> > this one, which is to query some capabilities from QEMU, like:
> >
> > - query-dump-guest-memory-capability
> > - query-migrate-capabilities
> 
> I'm not saying we must not add another ad hoc query.  I'm trying to
> figure out whether existing generic infrastructure can serve, or be
> adapted to serve.  Once we establish the answer is "no" or "badly", I'm
> willing to consider the ad hoc query.

Looking at this from the POV of solving the generic problem, what we
have here is an object with an integer property, for which only certain
values are permitted based on what host kernel/hardware we're runing
on.

So to solve this generically, we would need a way to provide information
in QOM as to what permitted values are for any given property. This would
make sense for at least bool, int and enum properties, since they can all
potentially have scenarios where the possible range of values is greater
than the currently permissible range of values.

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

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

* Re: [Qemu-devel] [RFC PATCH 0/2] ARM: add QMP command to query GIC version
  2016-02-15 15:21       ` Peter Maydell
@ 2016-02-15 19:40         ` Markus Armbruster
  2016-02-15 20:18           ` Andrew Jones
  0 siblings, 1 reply; 36+ messages in thread
From: Markus Armbruster @ 2016-02-15 19:40 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Wei Huang, Andrew Jones, Libvirt, QEMU Developers, Peter Xu,
	Andrea Bolognani, Paolo Bonzini

Peter Maydell <peter.maydell@linaro.org> writes:

> On 15 February 2016 at 15:08, Markus Armbruster <armbru@redhat.com> wrote:
>> Peter Xu <peterx@redhat.com> writes:
>>> On Mon, Feb 15, 2016 at 10:52:01AM +0100, Markus Armbruster wrote:
>>>> Peter Xu <peterx@redhat.com> writes:
>>>> Adding ad hoc queries as we go won't scale.  Is there really no generic
>>>> way to get this information, e.g. with qom-get?
>>>
>>> Haven't used "qom-get" before, but it seems to fetch one property
>>> for a specific object. If so, will it be strange to hide some
>>> capability bits into every GIC objects (though there is possibly
>>> one object)?
>>
>> Pardon my ignorance...  what are these "GIC objects"?
>>
>> What exactly is the "GIC type", and how would the result of
>> query-gic-capability be used?
>
> The GIC type (for our purposes) is the revision of the interrupt
> controller supported by the host, which comes in two versions
> (v2 and v3). These are not compatible, unless your host has
> the v3-with-v2-compat flavour. If a host is v3-only, it is not
> possible for it to give the guest a v2 virtual interrupt
> controller; if v2, it can't give the guest a v3 virtual interrupt
> controller. (If you ask QEMU to do this via command line options
> we will report an error at startup.)

How would the command line look like?

I'm asking because the answer will point us to the introspection problem
to solve.

> The underlying aim is to allow libvirt to say "this VM config
> won't work on this host", rather than ploughing blindly on
> and creating a VM config that always errors on startup.

Yes, finding out with a suitable introspection interface is easier and
more robust than "try and guess what the error message might mean".

> The "GIC object" presumably is the GIC QOM device object.
> However we do the "does this host support this GIC version?"
> check in QEMU before we ever create the GIC device object,
> so trying to probe it for properties isn't going to work.

QMP introspection would ideally cover introspecting device objects:
simply introspect device_add.  But it doesn't cover it now, because QMP
introspection is really a special case of QAPI schema introspection, and
the QAPI schema cannot express device_add for two reasons:

1. The QAPI schema is fixed at compile time, but the device models
aren't.  Instead, whatever device models are linked into QEMU register
themselves during startup.  Collecting them at compile time is a
solvable problem, but unlikely to be fun.

2. The QAPI schema is data, but general QOM properties are code.  As
always, code is much harder to work with than data.  You can't extract
properties from code at compile time, you have to run the code.

To introspect QOM properties, you have to instantiate an object, examine
it, then destroy it.  Aside: this requires instantiate + destroy to have
no lasting side effects, and experience tells that device model code
gets that wrong easily.

Paolo suggested that we special-case device introspection instead of
somehow hook it into QMP introsprection.  Even if we do that, developing
it will take time.

It'll always be easier and more expedient to add another ad hoc query
instead.  But it leaves the generic problem unsolved while the ad hoc
queries pile up.

> The only other QOM object potentially available to probe would be
> the machine (board) object. However as I understand it libvirt
> does all its probing with the "none" machine type, and it seems
> a bit odd to put a bunch of properties on the "none" machine
> type. It would be possible though I guess.

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

* Re: [Qemu-devel] [RFC PATCH 0/2] ARM: add QMP command to query GIC version
  2016-02-15 19:40         ` Markus Armbruster
@ 2016-02-15 20:18           ` Andrew Jones
  2016-02-15 20:32             ` Peter Maydell
  0 siblings, 1 reply; 36+ messages in thread
From: Andrew Jones @ 2016-02-15 20:18 UTC (permalink / raw)
  To: Markus Armbruster
  Cc: Wei Huang, Peter Maydell, Libvirt, QEMU Developers, Peter Xu,
	Andrea Bolognani, Paolo Bonzini

On Mon, Feb 15, 2016 at 08:40:54PM +0100, Markus Armbruster wrote:
> Peter Maydell <peter.maydell@linaro.org> writes:
> 
> > On 15 February 2016 at 15:08, Markus Armbruster <armbru@redhat.com> wrote:
> >> Peter Xu <peterx@redhat.com> writes:
> >>> On Mon, Feb 15, 2016 at 10:52:01AM +0100, Markus Armbruster wrote:
> >>>> Peter Xu <peterx@redhat.com> writes:
> >>>> Adding ad hoc queries as we go won't scale.  Is there really no generic
> >>>> way to get this information, e.g. with qom-get?
> >>>
> >>> Haven't used "qom-get" before, but it seems to fetch one property
> >>> for a specific object. If so, will it be strange to hide some
> >>> capability bits into every GIC objects (though there is possibly
> >>> one object)?
> >>
> >> Pardon my ignorance...  what are these "GIC objects"?
> >>
> >> What exactly is the "GIC type", and how would the result of
> >> query-gic-capability be used?
> >
> > The GIC type (for our purposes) is the revision of the interrupt
> > controller supported by the host, which comes in two versions
> > (v2 and v3). These are not compatible, unless your host has
> > the v3-with-v2-compat flavour. If a host is v3-only, it is not
> > possible for it to give the guest a v2 virtual interrupt
> > controller; if v2, it can't give the guest a v3 virtual interrupt
> > controller. (If you ask QEMU to do this via command line options
> > we will report an error at startup.)
> 
> How would the command line look like?
>

Here is what is available today

# select gicv2 (this work with and without KVM)
qemu-system-aarch64 -M virt                      # v2 is the default
qemu-system-aarch64 -M virt,gic-version=2 ...

# select gicv3 (only works with KVM)
qemu-system-aarch64 -M virt,gic-version=3 ...

# select whatever the host has
qemu-system-aarch64 -M virt,gic-version=host ...


Thanks,
drew

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

* Re: [Qemu-devel] [RFC PATCH 0/2] ARM: add QMP command to query GIC version
  2016-02-15 20:18           ` Andrew Jones
@ 2016-02-15 20:32             ` Peter Maydell
  2016-02-16 10:10               ` Markus Armbruster
  0 siblings, 1 reply; 36+ messages in thread
From: Peter Maydell @ 2016-02-15 20:32 UTC (permalink / raw)
  To: Andrew Jones
  Cc: Wei Huang, QEMU Developers, Libvirt, Andrea Bolognani, Peter Xu,
	Markus Armbruster, Paolo Bonzini

On 15 February 2016 at 20:18, Andrew Jones <drjones@redhat.com> wrote:
> On Mon, Feb 15, 2016 at 08:40:54PM +0100, Markus Armbruster wrote:
>> How would the command line look like?
>>
>
> Here is what is available today
>
> # select gicv2 (this work with and without KVM)
> qemu-system-aarch64 -M virt                      # v2 is the default
> qemu-system-aarch64 -M virt,gic-version=2 ...
>
> # select gicv3 (only works with KVM)
> qemu-system-aarch64 -M virt,gic-version=3 ...

This will work with TCG once we get the GICv3 emulation upstream.

> # select whatever the host has
> qemu-system-aarch64 -M virt,gic-version=host ...

This only works with KVM (like -cpu host).

thanks
-- PMM

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

* Re: [Qemu-devel] [RFC PATCH 0/2] ARM: add QMP command to query GIC version
  2016-02-15 20:32             ` Peter Maydell
@ 2016-02-16 10:10               ` Markus Armbruster
  2016-02-16 10:15                 ` Daniel P. Berrange
  0 siblings, 1 reply; 36+ messages in thread
From: Markus Armbruster @ 2016-02-16 10:10 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Wei Huang, Andrew Jones, Libvirt, QEMU Developers, Peter Xu,
	Andrea Bolognani, Paolo Bonzini

Peter Maydell <peter.maydell@linaro.org> writes:

> On 15 February 2016 at 20:18, Andrew Jones <drjones@redhat.com> wrote:
>> On Mon, Feb 15, 2016 at 08:40:54PM +0100, Markus Armbruster wrote:
>>> How would the command line look like?
>>>
>>
>> Here is what is available today
>>
>> # select gicv2 (this work with and without KVM)
>> qemu-system-aarch64 -M virt                      # v2 is the default
>> qemu-system-aarch64 -M virt,gic-version=2 ...
>>
>> # select gicv3 (only works with KVM)
>> qemu-system-aarch64 -M virt,gic-version=3 ...
>
> This will work with TCG once we get the GICv3 emulation upstream.
>
>> # select whatever the host has
>> qemu-system-aarch64 -M virt,gic-version=host ...
>
> This only works with KVM (like -cpu host).

Aha, it's a machine option.  Therefore, the generic direct solution
would be command line introspection.  A couple of remarks.

We don't have comprehensive command line introspection.  There's only
query-command-line-options, but it's incomplete and insufficiently
expressive.  We usually sidestep command line introspection and
introspect the corresponding QMP command, or we "look for a witness" in
QMP, i.e. some introspectible indicator for the non-introspectible
feature we need to know.

The is no QMP command corresponding to --machine.  There's a long term
vision to start QEMU with a blank slate, then configure everything via
QMP.  With that, QMP introspection would cover machine options.  Of
course, visions aren't going to help you now.

Even if there was a QMP command, the way we do --machine options would
defeat QMP introspection: they're QOM properties, defined in code.

Defining things in code is the most flexible solution, but it makes
reasoning about them *much* harder: the only general way to learn what
code does is run it.  This is fundamentally incompatible with
introspection.  In other words, QOM's design sacrifices
introspectability for flexibility.  The flexibility isn't actually
needed most of the time, but it defeats introspection all of the time.
For me, this was an design mistake.  We made the same mistake before,
with migration.

I figure we'll need to crack the QOM introspection problem to at least
some useful degree.  But it's going to be a lot of work.

A less flexible, introspectible, data-driven interface could be layered
on top of the code-driven one.  With as many users as possible converted
to the data-driven interface, the flexibility then defeats introspection
only where we actually use the flexibility.

Back to GIV.  Recognized values of gic-version are fixed at compile
time: 2, 3, host.  Once again, QOM does things in code rather than data:
the set of values is defined in the setter function
virt_set_gic_version().

Some values are accepted only together with other configuration: 3
requires accel=kvm (for now), host requires -cpu host.  Static
introspection can't show such constraints.

Would the proposed query-gic-capability show them?  How?

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

* Re: [Qemu-devel] [RFC PATCH 0/2] ARM: add QMP command to query GIC version
  2016-02-16 10:10               ` Markus Armbruster
@ 2016-02-16 10:15                 ` Daniel P. Berrange
  2016-02-16 12:05                   ` [Qemu-devel] [libvirt] " Andrea Bolognani
  0 siblings, 1 reply; 36+ messages in thread
From: Daniel P. Berrange @ 2016-02-16 10:15 UTC (permalink / raw)
  To: Markus Armbruster
  Cc: Wei Huang, Peter Maydell, Andrew Jones, Libvirt, QEMU Developers,
	Peter Xu, Andrea Bolognani, Paolo Bonzini

On Tue, Feb 16, 2016 at 11:10:55AM +0100, Markus Armbruster wrote:
> Peter Maydell <peter.maydell@linaro.org> writes:
> 
> > On 15 February 2016 at 20:18, Andrew Jones <drjones@redhat.com> wrote:
> >> On Mon, Feb 15, 2016 at 08:40:54PM +0100, Markus Armbruster wrote:
> >>> How would the command line look like?
> >>>
> >>
> >> Here is what is available today
> >>
> >> # select gicv2 (this work with and without KVM)
> >> qemu-system-aarch64 -M virt                      # v2 is the default
> >> qemu-system-aarch64 -M virt,gic-version=2 ...
> >>
> >> # select gicv3 (only works with KVM)
> >> qemu-system-aarch64 -M virt,gic-version=3 ...
> >
> > This will work with TCG once we get the GICv3 emulation upstream.
> >
> >> # select whatever the host has
> >> qemu-system-aarch64 -M virt,gic-version=host ...
> >
> > This only works with KVM (like -cpu host).
> 
> Aha, it's a machine option.  Therefore, the generic direct solution
> would be command line introspection.  A couple of remarks.
> 
> We don't have comprehensive command line introspection.  There's only
> query-command-line-options, but it's incomplete and insufficiently
> expressive.  We usually sidestep command line introspection and
> introspect the corresponding QMP command, or we "look for a witness" in
> QMP, i.e. some introspectible indicator for the non-introspectible
> feature we need to know.
> 
> The is no QMP command corresponding to --machine.  There's a long term
> vision to start QEMU with a blank slate, then configure everything via
> QMP.  With that, QMP introspection would cover machine options.  Of
> course, visions aren't going to help you now.
> 
> Even if there was a QMP command, the way we do --machine options would
> defeat QMP introspection: they're QOM properties, defined in code.
> 
> Defining things in code is the most flexible solution, but it makes
> reasoning about them *much* harder: the only general way to learn what
> code does is run it.  This is fundamentally incompatible with
> introspection.  In other words, QOM's design sacrifices
> introspectability for flexibility.  The flexibility isn't actually
> needed most of the time, but it defeats introspection all of the time.
> For me, this was an design mistake.  We made the same mistake before,
> with migration.
> 
> I figure we'll need to crack the QOM introspection problem to at least
> some useful degree.  But it's going to be a lot of work.
> 
> A less flexible, introspectible, data-driven interface could be layered
> on top of the code-driven one.  With as many users as possible converted
> to the data-driven interface, the flexibility then defeats introspection
> only where we actually use the flexibility.
> 
> Back to GIV.  Recognized values of gic-version are fixed at compile
> time: 2, 3, host.  Once again, QOM does things in code rather than data:
> the set of values is defined in the setter function
> virt_set_gic_version().
> 
> Some values are accepted only together with other configuration: 3
> requires accel=kvm (for now), host requires -cpu host.  Static
> introspection can't show such constraints.
> 
> Would the proposed query-gic-capability show them?  How?

Also bear in mind that libvirt probes capabilities using '-m none' so
you're not going to have any 'virt' machine type instantiated when
probing is done.

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

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

* Re: [Qemu-devel] [libvirt] [RFC PATCH 0/2] ARM: add QMP command to query GIC version
  2016-02-16 10:15                 ` Daniel P. Berrange
@ 2016-02-16 12:05                   ` Andrea Bolognani
  2016-02-16 12:09                     ` Peter Maydell
  2016-02-16 12:15                     ` Daniel P. Berrange
  0 siblings, 2 replies; 36+ messages in thread
From: Andrea Bolognani @ 2016-02-16 12:05 UTC (permalink / raw)
  To: Daniel P. Berrange, Markus Armbruster
  Cc: Wei Huang, Peter Maydell, Andrew Jones, Libvirt, QEMU Developers,
	Peter Xu, Paolo Bonzini

On Tue, 2016-02-16 at 10:15 +0000, Daniel P. Berrange wrote:
> > Back to GIV.  Recognized values of gic-version are fixed at compile
> > time: 2, 3, host.  Once again, QOM does things in code rather than data:
> > the set of values is defined in the setter function
> > virt_set_gic_version().
> > 
> > Some values are accepted only together with other configuration: 3
> > requires accel=kvm (for now), host requires -cpu host.  Static
> > introspection can't show such constraints.
> > 
> > Would the proposed query-gic-capability show them?  How?
> 
> Also bear in mind that libvirt probes capabilities using '-m none' so
> you're not going to have any 'virt' machine type instantiated when
> probing is done.

The idea is to add this information to domain capabilities, which
already have virtualization type, architecture and machine type as
inputs.

Since GIC is only available for the virt machine type on ARM hosts,
and the GIC versions might be limited by using TCG instead of KVM,
it seems like it would fit nicely...

Cheers.

-- 
Andrea Bolognani
Software Engineer - Virtualization Team

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

* Re: [Qemu-devel] [libvirt] [RFC PATCH 0/2] ARM: add QMP command to query GIC version
  2016-02-16 12:05                   ` [Qemu-devel] [libvirt] " Andrea Bolognani
@ 2016-02-16 12:09                     ` Peter Maydell
  2016-02-16 12:20                       ` Andrea Bolognani
  2016-02-16 12:15                     ` Daniel P. Berrange
  1 sibling, 1 reply; 36+ messages in thread
From: Peter Maydell @ 2016-02-16 12:09 UTC (permalink / raw)
  To: Andrea Bolognani
  Cc: Wei Huang, Andrew Jones, Libvirt, Markus Armbruster, Peter Xu,
	QEMU Developers, Paolo Bonzini

On 16 February 2016 at 12:05, Andrea Bolognani <abologna@redhat.com> wrote:
> The idea is to add this information to domain capabilities, which
> already have virtualization type, architecture and machine type as
> inputs.
>
> Since GIC is only available for the virt machine type on ARM hosts,
> and the GIC versions might be limited by using TCG instead of KVM,
> it seems like it would fit nicely...

The GIC is available for more than just "virt". It's just that
at the moment "virt" is the only machine type which lets you
pick which GIC version you want, and so the only machine type
with that machine property. (For instance vexpress-a15
is always GICv2, since the hardware we're modelling there is
always GICv2.)

Other board models in future might support a choice of GIC
versions. (The obvious possibility would be the Android
'ranchu' board if that is ever upstreamed.)

thanks
-- PMM

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

* Re: [Qemu-devel] [libvirt] [RFC PATCH 0/2] ARM: add QMP command to query GIC version
  2016-02-16 12:05                   ` [Qemu-devel] [libvirt] " Andrea Bolognani
  2016-02-16 12:09                     ` Peter Maydell
@ 2016-02-16 12:15                     ` Daniel P. Berrange
  2016-02-16 12:27                       ` Andrea Bolognani
  1 sibling, 1 reply; 36+ messages in thread
From: Daniel P. Berrange @ 2016-02-16 12:15 UTC (permalink / raw)
  To: Andrea Bolognani
  Cc: Wei Huang, Peter Maydell, Andrew Jones, Libvirt,
	Markus Armbruster, Peter Xu, QEMU Developers, Paolo Bonzini

On Tue, Feb 16, 2016 at 01:05:45PM +0100, Andrea Bolognani wrote:
> On Tue, 2016-02-16 at 10:15 +0000, Daniel P. Berrange wrote:
> > > Back to GIV.  Recognized values of gic-version are fixed at compile
> > > time: 2, 3, host.  Once again, QOM does things in code rather than data:
> > > the set of values is defined in the setter function
> > > virt_set_gic_version().
> > > 
> > > Some values are accepted only together with other configuration: 3
> > > requires accel=kvm (for now), host requires -cpu host.  Static
> > > introspection can't show such constraints.
> > > 
> > > Would the proposed query-gic-capability show them?  How?
> > 
> > Also bear in mind that libvirt probes capabilities using '-m none' so
> > you're not going to have any 'virt' machine type instantiated when
> > probing is done.
> 
> The idea is to add this information to domain capabilities, which
> already have virtualization type, architecture and machine type as
> inputs.

Regardless of the way it is exposed in libvirt API, when libvirt probes
for capabilities it will *always* use '-m none'.

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

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

* Re: [Qemu-devel] [libvirt] [RFC PATCH 0/2] ARM: add QMP command to query GIC version
  2016-02-16 12:09                     ` Peter Maydell
@ 2016-02-16 12:20                       ` Andrea Bolognani
  0 siblings, 0 replies; 36+ messages in thread
From: Andrea Bolognani @ 2016-02-16 12:20 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Wei Huang, Andrew Jones, Libvirt, QEMU Developers, Peter Xu,
	Paolo Bonzini

On Tue, 2016-02-16 at 12:09 +0000, Peter Maydell wrote:
> On 16 February 2016 at 12:05, Andrea Bolognani <abologna@redhat.com> wrote:
> > The idea is to add this information to domain capabilities, which
> > already have virtualization type, architecture and machine type as
> > inputs.
> > 
> > Since GIC is only available for the virt machine type on ARM hosts,
> > and the GIC versions might be limited by using TCG instead of KVM,
> > it seems like it would fit nicely...
> 
> The GIC is available for more than just "virt". It's just that
> at the moment "virt" is the only machine type which lets you
> pick which GIC version you want, and so the only machine type
> with that machine property. (For instance vexpress-a15
> is always GICv2, since the hardware we're modelling there is
> always GICv2.)
> 
> Other board models in future might support a choice of GIC
> versions. (The obvious possibility would be the Android
> 'ranchu' board if that is ever upstreamed.)

Yeah, I didn't express myself properly: I meant to say that the
ability to pick a GIC version is limited to the virt machine
type so far. Sorry about that :)

Cheers.

-- 
Andrea Bolognani
Software Engineer - Virtualization Team

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

* Re: [Qemu-devel] [libvirt] [RFC PATCH 0/2] ARM: add QMP command to query GIC version
  2016-02-16 12:15                     ` Daniel P. Berrange
@ 2016-02-16 12:27                       ` Andrea Bolognani
  2016-02-16 12:38                         ` Daniel P. Berrange
  0 siblings, 1 reply; 36+ messages in thread
From: Andrea Bolognani @ 2016-02-16 12:27 UTC (permalink / raw)
  To: Daniel P. Berrange
  Cc: Wei Huang, Peter Maydell, Andrew Jones, Libvirt, QEMU Developers,
	Peter Xu, Paolo Bonzini

On Tue, 2016-02-16 at 12:15 +0000, Daniel P. Berrange wrote:
> On Tue, Feb 16, 2016 at 01:05:45PM +0100, Andrea Bolognani wrote:
> > On Tue, 2016-02-16 at 10:15 +0000, Daniel P. Berrange wrote:
> > > > Back to GIV.  Recognized values of gic-version are fixed at compile
> > > > time: 2, 3, host.  Once again, QOM does things in code rather than data:
> > > > the set of values is defined in the setter function
> > > > virt_set_gic_version().
> > > >  
> > > > Some values are accepted only together with other configuration: 3
> > > > requires accel=kvm (for now), host requires -cpu host.  Static
> > > > introspection can't show such constraints.
> > > >  
> > > > Would the proposed query-gic-capability show them?  How?
> > >  
> > > Also bear in mind that libvirt probes capabilities using '-m none' so
> > > you're not going to have any 'virt' machine type instantiated when
> > > probing is done.
> > 
> > The idea is to add this information to domain capabilities, which
> > already have virtualization type, architecture and machine type as
> > inputs.
> 
> Regardless of the way it is exposed in libvirt API, when libvirt probes
> for capabilities it will *always* use '-m none'.

Domain capabilities are currently derived almost entirely from data
taken from virQEMUCaps, but is there anything stopping us from
calling QEMU with the appropriate machine type from
qemuConnectGetDomainCapabilities() to query for machine type
dependent domain capabilities such as supported values for the
gic-version property?

Cheers.

-- 
Andrea Bolognani
Software Engineer - Virtualization Team

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

* Re: [Qemu-devel] [libvirt] [RFC PATCH 0/2] ARM: add QMP command to query GIC version
  2016-02-16 12:27                       ` Andrea Bolognani
@ 2016-02-16 12:38                         ` Daniel P. Berrange
  2016-02-16 13:14                           ` Andrea Bolognani
  0 siblings, 1 reply; 36+ messages in thread
From: Daniel P. Berrange @ 2016-02-16 12:38 UTC (permalink / raw)
  To: Andrea Bolognani
  Cc: Wei Huang, Peter Maydell, Andrew Jones, Libvirt, QEMU Developers,
	Peter Xu, Paolo Bonzini

On Tue, Feb 16, 2016 at 01:27:55PM +0100, Andrea Bolognani wrote:
> On Tue, 2016-02-16 at 12:15 +0000, Daniel P. Berrange wrote:
> > On Tue, Feb 16, 2016 at 01:05:45PM +0100, Andrea Bolognani wrote:
> > > On Tue, 2016-02-16 at 10:15 +0000, Daniel P. Berrange wrote:
> > > > > Back to GIV.  Recognized values of gic-version are fixed at compile
> > > > > time: 2, 3, host.  Once again, QOM does things in code rather than data:
> > > > > the set of values is defined in the setter function
> > > > > virt_set_gic_version().
> > > > >  
> > > > > Some values are accepted only together with other configuration: 3
> > > > > requires accel=kvm (for now), host requires -cpu host.  Static
> > > > > introspection can't show such constraints.
> > > > >  
> > > > > Would the proposed query-gic-capability show them?  How?
> > > >  
> > > > Also bear in mind that libvirt probes capabilities using '-m none' so
> > > > you're not going to have any 'virt' machine type instantiated when
> > > > probing is done.
> > > 
> > > The idea is to add this information to domain capabilities, which
> > > already have virtualization type, architecture and machine type as
> > > inputs.
> > 
> > Regardless of the way it is exposed in libvirt API, when libvirt probes
> > for capabilities it will *always* use '-m none'.
> 
> Domain capabilities are currently derived almost entirely from data
> taken from virQEMUCaps, but is there anything stopping us from
> calling QEMU with the appropriate machine type from
> qemuConnectGetDomainCapabilities() to query for machine type
> dependent domain capabilities such as supported values for the
> gic-version property?

The cost of invoking QEMU for each architecture and probing capabilities
is already higher than we would like. If we multiply that cost by the
number of machine types, it makes the problem orders of magnitude worse,
so we cannot go that route.

This is a key reason why we would like to be able to probe properties
against QEMU classses without instantiating them. ie so we can launch
QEMU once with "-m none" and still be able to query specific properties
we want from the 'virt' machine type. A small step towards this was
made with a patch we landed to let us register properties against
classes in QOM, instead of against objects. We've not done work to
convert code internally to use this facility yet though.

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

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

* Re: [Qemu-devel] [libvirt] [RFC PATCH 0/2] ARM: add QMP command to query GIC version
  2016-02-16 12:38                         ` Daniel P. Berrange
@ 2016-02-16 13:14                           ` Andrea Bolognani
  0 siblings, 0 replies; 36+ messages in thread
From: Andrea Bolognani @ 2016-02-16 13:14 UTC (permalink / raw)
  To: Daniel P. Berrange
  Cc: Wei Huang, Peter Maydell, Andrew Jones, Libvirt, QEMU Developers,
	Peter Xu, Paolo Bonzini

On Tue, 2016-02-16 at 12:38 +0000, Daniel P. Berrange wrote:
> > > Regardless of the way it is exposed in libvirt API, when libvirt probes
> > > for capabilities it will *always* use '-m none'.
> > 
> > Domain capabilities are currently derived almost entirely from data
> > taken from virQEMUCaps, but is there anything stopping us from
> > calling QEMU with the appropriate machine type from
> > qemuConnectGetDomainCapabilities() to query for machine type
> > dependent domain capabilities such as supported values for the
> > gic-version property?
> 
> The cost of invoking QEMU for each architecture and probing capabilities
> is already higher than we would like. If we multiply that cost by the
> number of machine types, it makes the problem orders of magnitude worse,
> so we cannot go that route.
> 
> This is a key reason why we would like to be able to probe properties
> against QEMU classses without instantiating them. ie so we can launch
> QEMU once with "-m none" and still be able to query specific properties
> we want from the 'virt' machine type. A small step towards this was
> made with a patch we landed to let us register properties against
> classes in QOM, instead of against objects. We've not done work to
> convert code internally to use this facility yet though.

So, until such support lands in QEMU, is there really anything we
can do in libvirt beside passing through whatever value the user
has specified in the domain XML and report QEMU's error on failure?

I had underestimated the performance implications of spawning more
QEMU processes, thanks for highlighting them.

Cheers.

-- 
Andrea Bolognani
Software Engineer - Virtualization Team

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

* Re: [Qemu-devel] [RFC PATCH 0/2] ARM: add QMP command to query GIC version
  2016-02-15 15:22       ` [Qemu-devel] " Daniel P. Berrange
@ 2016-02-18  4:40         ` Peter Xu
  2016-02-18 16:52           ` Andrew Jones
  0 siblings, 1 reply; 36+ messages in thread
From: Peter Xu @ 2016-02-18  4:40 UTC (permalink / raw)
  To: Daniel P. Berrange
  Cc: wei, drjones, qemu-devel, libvir-list, Markus Armbruster, abologna

On Mon, Feb 15, 2016 at 03:22:05PM +0000, Daniel P. Berrange wrote:
> On Mon, Feb 15, 2016 at 04:08:33PM +0100, Markus Armbruster wrote:
> > Peter Xu <peterx@redhat.com> writes:
> > 
> > > On Mon, Feb 15, 2016 at 10:52:01AM +0100, Markus Armbruster wrote:
> > >> Peter Xu <peterx@redhat.com> writes:
> > >> 
> > >> > For ARM platform, we still do not have any interface to query
> > >> > whether current QEMU/host support specific GIC version. This
> > >> > patchset is trying to add one QMP interface for that. By querying
> > >> > the GIC capability using the new interface, one should know exactly
> > >> > what GIC version(s) the platform will support. The capability bits
> > >> > will be decided by both QEMU and host kernel.
> > >> >
> > >> > The current patchset only provides interface for review. Its handler
> > >> > is a fake one which returns empty always.
> > >> >
> > >> > The command interface I am planning to add is something like this:
> > >> >
> > >> > -> { "execute": "query-gic-capability" }
> > >> > <- { "return": [ "gicv2", "gicv2-kvm", "gicv3-kvm" ] }
> > >> >
> > >> > Currently, all the possible supported GIC versions are:
> > >> >
> > >> > - 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
> > >> >
> > >> > Since "gicv3" is still not supported (to use GICv3, kernel irqchip
> > >> > support is required for now, which corresponds to "gicv3-kvm"),
> > >> > currently the maximum superset of the result should be:
> > >> >
> > >> > ["gicv2", "gicv2-kvm", "gicv3-kvm"]
> > >> >
> > >> > Please help review whether the interface suits our need, also please
> > >> > point out any error I have made.
> > >> 
> > >> Adding ad hoc queries as we go won't scale.  Is there really no generic
> > >> way to get this information, e.g. with qom-get?
> > >
> > > Haven't used "qom-get" before, but it seems to fetch one property
> > > for a specific object. If so, will it be strange to hide some
> > > capability bits into every GIC objects (though there is possibly
> > > one object)?
> > 
> > Pardon my ignorance...  what are these "GIC objects"?
> > 
> > What exactly is the "GIC type", and how would the result of
> > query-gic-capability be used?
> > 
> > > I agree that we should keep the interface as simple as
> > > possible. I see that there are already commands that works just like
> > > this one, which is to query some capabilities from QEMU, like:
> > >
> > > - query-dump-guest-memory-capability
> > > - query-migrate-capabilities
> > 
> > I'm not saying we must not add another ad hoc query.  I'm trying to
> > figure out whether existing generic infrastructure can serve, or be
> > adapted to serve.  Once we establish the answer is "no" or "badly", I'm
> > willing to consider the ad hoc query.
> 
> Looking at this from the POV of solving the generic problem, what we
> have here is an object with an integer property, for which only certain
> values are permitted based on what host kernel/hardware we're runing
> on.
> 
> So to solve this generically, we would need a way to provide information
> in QOM as to what permitted values are for any given property. This would
> make sense for at least bool, int and enum properties, since they can all
> potentially have scenarios where the possible range of values is greater
> than the currently permissible range of values.

Is this work on any of our todo list (or anyone has started the
prototyping)?

It seems reasonable to provide such a generic interface, rather than
adding a "query-gic-capability" for GIC versions only. The problem
is that, I am not sure how eagerly we are wanting this GIC
interface, and when will this framework be there in QOM.

Thanks.
Peter

> 
> Regards,
> Daniel
> -- 
> |: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
> |: http://libvirt.org              -o-             http://virt-manager.org :|
> |: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
> |: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

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

* Re: [Qemu-devel] [RFC PATCH 0/2] ARM: add QMP command to query GIC version
  2016-02-18  4:40         ` Peter Xu
@ 2016-02-18 16:52           ` Andrew Jones
  2016-02-18 17:10             ` Andrea Bolognani
  0 siblings, 1 reply; 36+ messages in thread
From: Andrew Jones @ 2016-02-18 16:52 UTC (permalink / raw)
  To: Peter Xu; +Cc: wei, libvir-list, Markus Armbruster, qemu-devel, abologna

On Thu, Feb 18, 2016 at 12:40:56PM +0800, Peter Xu wrote:
> On Mon, Feb 15, 2016 at 03:22:05PM +0000, Daniel P. Berrange wrote:
> > On Mon, Feb 15, 2016 at 04:08:33PM +0100, Markus Armbruster wrote:
> > > Peter Xu <peterx@redhat.com> writes:
> > > 
> > > > On Mon, Feb 15, 2016 at 10:52:01AM +0100, Markus Armbruster wrote:
> > > >> Peter Xu <peterx@redhat.com> writes:
> > > >> 
> > > >> > For ARM platform, we still do not have any interface to query
> > > >> > whether current QEMU/host support specific GIC version. This
> > > >> > patchset is trying to add one QMP interface for that. By querying
> > > >> > the GIC capability using the new interface, one should know exactly
> > > >> > what GIC version(s) the platform will support. The capability bits
> > > >> > will be decided by both QEMU and host kernel.
> > > >> >
> > > >> > The current patchset only provides interface for review. Its handler
> > > >> > is a fake one which returns empty always.
> > > >> >
> > > >> > The command interface I am planning to add is something like this:
> > > >> >
> > > >> > -> { "execute": "query-gic-capability" }
> > > >> > <- { "return": [ "gicv2", "gicv2-kvm", "gicv3-kvm" ] }
> > > >> >
> > > >> > Currently, all the possible supported GIC versions are:
> > > >> >
> > > >> > - 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
> > > >> >
> > > >> > Since "gicv3" is still not supported (to use GICv3, kernel irqchip
> > > >> > support is required for now, which corresponds to "gicv3-kvm"),
> > > >> > currently the maximum superset of the result should be:
> > > >> >
> > > >> > ["gicv2", "gicv2-kvm", "gicv3-kvm"]
> > > >> >
> > > >> > Please help review whether the interface suits our need, also please
> > > >> > point out any error I have made.
> > > >> 
> > > >> Adding ad hoc queries as we go won't scale.  Is there really no generic
> > > >> way to get this information, e.g. with qom-get?
> > > >
> > > > Haven't used "qom-get" before, but it seems to fetch one property
> > > > for a specific object. If so, will it be strange to hide some
> > > > capability bits into every GIC objects (though there is possibly
> > > > one object)?
> > > 
> > > Pardon my ignorance...  what are these "GIC objects"?
> > > 
> > > What exactly is the "GIC type", and how would the result of
> > > query-gic-capability be used?
> > > 
> > > > I agree that we should keep the interface as simple as
> > > > possible. I see that there are already commands that works just like
> > > > this one, which is to query some capabilities from QEMU, like:
> > > >
> > > > - query-dump-guest-memory-capability
> > > > - query-migrate-capabilities
> > > 
> > > I'm not saying we must not add another ad hoc query.  I'm trying to
> > > figure out whether existing generic infrastructure can serve, or be
> > > adapted to serve.  Once we establish the answer is "no" or "badly", I'm
> > > willing to consider the ad hoc query.
> > 
> > Looking at this from the POV of solving the generic problem, what we
> > have here is an object with an integer property, for which only certain
> > values are permitted based on what host kernel/hardware we're runing
> > on.
> > 
> > So to solve this generically, we would need a way to provide information
> > in QOM as to what permitted values are for any given property. This would
> > make sense for at least bool, int and enum properties, since they can all
> > potentially have scenarios where the possible range of values is greater
> > than the currently permissible range of values.
> 
> Is this work on any of our todo list (or anyone has started the
> prototyping)?
> 
> It seems reasonable to provide such a generic interface, rather than
> adding a "query-gic-capability" for GIC versions only. The problem
> is that, I am not sure how eagerly we are wanting this GIC
> interface, and when will this framework be there in QOM.

We want it eagerly :-) This type of a rabbit hole is likely why Daniel
was suggesting we do more in libvirt. I'm still not sure we want to
probe both kvm and qemu from libvirt though, so I'm still in favor of
an improved qemu probing method being worked out.

I don't know what the policy is for deprecating QMP commands, but I
wonder if we can't introduce a QMP command now, and then, after working
out the QOM extensions, we could shift to it, deprecating this QMP
command and any others that would no longer be needed.

Thanks,
drew


> 
> Thanks.
> Peter
> 
> > 
> > Regards,
> > Daniel
> > -- 
> > |: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
> > |: http://libvirt.org              -o-             http://virt-manager.org :|
> > |: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
> > |: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|
> 

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

* Re: [Qemu-devel] [RFC PATCH 0/2] ARM: add QMP command to query GIC version
  2016-02-18 16:52           ` Andrew Jones
@ 2016-02-18 17:10             ` Andrea Bolognani
  2016-02-19  1:55               ` Peter Xu
  0 siblings, 1 reply; 36+ messages in thread
From: Andrea Bolognani @ 2016-02-18 17:10 UTC (permalink / raw)
  To: Andrew Jones, Peter Xu; +Cc: wei, libvir-list, qemu-devel, Markus Armbruster

On Thu, 2016-02-18 at 17:52 +0100, Andrew Jones wrote:
> > Is this work on any of our todo list (or anyone has started the
> > prototyping)?
> > 
> > It seems reasonable to provide such a generic interface, rather than
> > adding a "query-gic-capability" for GIC versions only. The problem
> > is that, I am not sure how eagerly we are wanting this GIC
> > interface, and when will this framework be there in QOM.
> 
> We want it eagerly :-) This type of a rabbit hole is likely why Daniel
> was suggesting we do more in libvirt. I'm still not sure we want to
> probe both kvm and qemu from libvirt though, so I'm still in favor of
> an improved qemu probing method being worked out.
> 
> I don't know what the policy is for deprecating QMP commands, but I
> wonder if we can't introduce a QMP command now, and then, after working
> out the QOM extensions, we could shift to it, deprecating this QMP
> command and any others that would no longer be needed.

AFAIK, the current situation of libvirt passing the GIC version to
QEMU and simply reporting in case of failure is not unprecedented
and there are a few cases where probing in advance would simply not
be feasible.

Any probing code added to libvirt would have to be kept around
forever to ensure compatibility with current QEMU versions, so it
should IMHO be seen as a last resort in case we can't live without
GIC version probing while it's being implemented, properly, in QEMU.

Cheers.

-- 
Andrea Bolognani
Software Engineer - Virtualization Team

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

* Re: [Qemu-devel] [RFC PATCH 0/2] ARM: add QMP command to query GIC version
  2016-02-18 17:10             ` Andrea Bolognani
@ 2016-02-19  1:55               ` Peter Xu
  2016-02-19 12:33                 ` Andrea Bolognani
  0 siblings, 1 reply; 36+ messages in thread
From: Peter Xu @ 2016-02-19  1:55 UTC (permalink / raw)
  To: Andrea Bolognani
  Cc: wei, Andrew Jones, libvir-list, qemu-devel, Markus Armbruster

On Thu, Feb 18, 2016 at 06:10:21PM +0100, Andrea Bolognani wrote:
> On Thu, 2016-02-18 at 17:52 +0100, Andrew Jones wrote:
> > > Is this work on any of our todo list (or anyone has started the
> > > prototyping)?
> > > 
> > > It seems reasonable to provide such a generic interface, rather than
> > > adding a "query-gic-capability" for GIC versions only. The problem
> > > is that, I am not sure how eagerly we are wanting this GIC
> > > interface, and when will this framework be there in QOM.
> > 
> > We want it eagerly :-) This type of a rabbit hole is likely why Daniel
> > was suggesting we do more in libvirt. I'm still not sure we want to
> > probe both kvm and qemu from libvirt though, so I'm still in favor of
> > an improved qemu probing method being worked out.
> > 
> > I don't know what the policy is for deprecating QMP commands, but I
> > wonder if we can't introduce a QMP command now, and then, after working
> > out the QOM extensions, we could shift to it, deprecating this QMP
> > command and any others that would no longer be needed.
> 
> AFAIK, the current situation of libvirt passing the GIC version to
> QEMU and simply reporting in case of failure is not unprecedented
> and there are a few cases where probing in advance would simply not
> be feasible.
> 
> Any probing code added to libvirt would have to be kept around
> forever to ensure compatibility with current QEMU versions, so it
> should IMHO be seen as a last resort in case we can't live without
> GIC version probing while it's being implemented, properly, in QEMU.

If libvirt is the most possible consumer for the new command, I
think it might not be too hard to keep the compatibility of all
possible versions of QEMU. E.g., after we have got a better way to
query GIC version other than query-gic-capability, we can do
something like this in libvirt:

- try query-gic-capability
  - if supported -> [got GIC version]
  - if not supported -> try the new method
    - if supported -> [got GIC version]
    - if not supported -> [not support]

During the time when QEMU has both methods working (before
obsoleting the query-gic-capability QMP command), QEMU will make
sure querying in both way will get exactly the same results.

Does this work?

Thanks.
Peter

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

* Re: [Qemu-devel] [RFC PATCH 0/2] ARM: add QMP command to query GIC version
  2016-02-19  1:55               ` Peter Xu
@ 2016-02-19 12:33                 ` Andrea Bolognani
  2016-02-22  1:35                   ` Peter Xu
  0 siblings, 1 reply; 36+ messages in thread
From: Andrea Bolognani @ 2016-02-19 12:33 UTC (permalink / raw)
  To: Peter Xu; +Cc: wei, Andrew Jones, libvir-list, qemu-devel, Markus Armbruster

On Fri, 2016-02-19 at 09:55 +0800, Peter Xu wrote:
> > AFAIK, the current situation of libvirt passing the GIC version to
> > QEMU and simply reporting in case of failure is not unprecedented
> > and there are a few cases where probing in advance would simply not
> > be feasible.
> > 
> > Any probing code added to libvirt would have to be kept around
> > forever to ensure compatibility with current QEMU versions, so it
> > should IMHO be seen as a last resort in case we can't live without
> > GIC version probing while it's being implemented, properly, in QEMU.
> 
> If libvirt is the most possible consumer for the new command, I
> think it might not be too hard to keep the compatibility of all
> possible versions of QEMU. E.g., after we have got a better way to
> query GIC version other than query-gic-capability, we can do
> something like this in libvirt:
> 
> - try query-gic-capability
>   - if supported -> [got GIC version]
>   - if not supported -> try the new method
>     - if supported -> [got GIC version]
>     - if not supported -> [not support]
> 
> During the time when QEMU has both methods working (before
> obsoleting the query-gic-capability QMP command), QEMU will make
> sure querying in both way will get exactly the same results.

I didn't say it would be hard :)

I just said that such compatibility code would have to be kept
around forever. We already support lots and lots of similar cases
in libvirt, the difference being that in this case we would add
support for a new command *knowing in advance* that it will become
obsolete as soon as a proper implementation is available.

It might still be the right thing to do! I just want to make sure
everything's been properly considered and discussed beforehand.

Cheers.

-- 
Andrea Bolognani
Software Engineer - Virtualization Team

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

* Re: [Qemu-devel] [RFC PATCH 0/2] ARM: add QMP command to query GIC version
  2016-02-19 12:33                 ` Andrea Bolognani
@ 2016-02-22  1:35                   ` Peter Xu
  2016-02-29 16:30                     ` Andrea Bolognani
  0 siblings, 1 reply; 36+ messages in thread
From: Peter Xu @ 2016-02-22  1:35 UTC (permalink / raw)
  To: Andrea Bolognani
  Cc: wei, Andrew Jones, libvir-list, qemu-devel, Markus Armbruster

On Fri, Feb 19, 2016 at 01:33:09PM +0100, Andrea Bolognani wrote:
> I didn't say it would be hard :)
> 
> I just said that such compatibility code would have to be kept
> around forever. We already support lots and lots of similar cases
> in libvirt, the difference being that in this case we would add
> support for a new command *knowing in advance* that it will become
> obsolete as soon as a proper implementation is available.
> 
> It might still be the right thing to do! I just want to make sure
> everything's been properly considered and discussed beforehand.

I totally agree with you to think more before doing. :)

Then I will try to move on. Appreciate for all the review comments!

Peter

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

* Re: [Qemu-devel] [RFC PATCH 0/2] ARM: add QMP command to query GIC version
  2016-02-22  1:35                   ` Peter Xu
@ 2016-02-29 16:30                     ` Andrea Bolognani
  2016-03-01  2:19                       ` Peter Xu
  0 siblings, 1 reply; 36+ messages in thread
From: Andrea Bolognani @ 2016-02-29 16:30 UTC (permalink / raw)
  To: Peter Xu; +Cc: wei, libvir-list, Andrew Jones, qemu-devel, Markus Armbruster

On Mon, 2016-02-22 at 09:35 +0800, Peter Xu wrote:
> On Fri, Feb 19, 2016 at 01:33:09PM +0100, Andrea Bolognani wrote:
> > 
> > I didn't say it would be hard :)
> > 
> > I just said that such compatibility code would have to be kept
> > around forever. We already support lots and lots of similar cases
> > in libvirt, the difference being that in this case we would add
> > support for a new command *knowing in advance* that it will become
> > obsolete as soon as a proper implementation is available.
> > 
> > It might still be the right thing to do! I just want to make sure
> > everything's been properly considered and discussed beforehand.
> 
> I totally agree with you to think more before doing. :)

So, anyone else willing to give their $0.2 on how to implement
this The Right Way™?

I just skimmed the whole thread again and it doesn't look to me
like any consensus has been reached.

Cheers.

-- 
Andrea Bolognani
Software Engineer - Virtualization Team

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

* Re: [Qemu-devel] [RFC PATCH 0/2] ARM: add QMP command to query GIC version
  2016-02-29 16:30                     ` Andrea Bolognani
@ 2016-03-01  2:19                       ` Peter Xu
  0 siblings, 0 replies; 36+ messages in thread
From: Peter Xu @ 2016-03-01  2:19 UTC (permalink / raw)
  To: Andrea Bolognani
  Cc: wei, libvir-list, Andrew Jones, qemu-devel, Markus Armbruster

On Mon, Feb 29, 2016 at 05:30:36PM +0100, Andrea Bolognani wrote:
> On Mon, 2016-02-22 at 09:35 +0800, Peter Xu wrote:
> > On Fri, Feb 19, 2016 at 01:33:09PM +0100, Andrea Bolognani wrote:
> > > 
> > > I didn't say it would be hard :)
> > > 
> > > I just said that such compatibility code would have to be kept
> > > around forever. We already support lots and lots of similar cases
> > > in libvirt, the difference being that in this case we would add
> > > support for a new command *knowing in advance* that it will become
> > > obsolete as soon as a proper implementation is available.
> > > 
> > > It might still be the right thing to do! I just want to make sure
> > > everything's been properly considered and discussed beforehand.
> > 
> > I totally agree with you to think more before doing. :)
> 
> So, anyone else willing to give their $0.2 on how to implement
> this The Right Way™?
> 
> I just skimmed the whole thread again and it doesn't look to me
> like any consensus has been reached.

Hi, Andrea,

I have sent another non-rfc patchset about this already:

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

I am not sure whether you have received it or not (you should be in
the CC list :), just pasting it again here. Do you think this would
work for us?

Btw, the thread is receiving none reviews till now. Looking forward
to have any of your further comments!

Thanks!
Peter

> 
> Cheers.
> 
> -- 
> Andrea Bolognani
> Software Engineer - Virtualization Team

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

end of thread, other threads:[~2016-03-01  2:21 UTC | newest]

Thread overview: 36+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-02-14  5:41 [Qemu-devel] [RFC PATCH 0/2] ARM: add QMP command to query GIC version Peter Xu
2016-02-14  5:41 ` [Qemu-devel] [RFC PATCH 1/2] arm: gic: add GICType Peter Xu
2016-02-14  5:41 ` [Qemu-devel] [RFC PATCH 2/2] arm: gic: add "query-gic-capability" interface Peter Xu
2016-02-15  6:54 ` [Qemu-devel] [RFC PATCH 0/2] ARM: add QMP command to query GIC version Wei Huang
2016-02-15  7:34   ` Peter Xu
2016-02-15  7:49     ` Fam Zheng
2016-02-15  9:35 ` [Qemu-devel] [libvirt] " Martin Kletzander
2016-02-15  9:41   ` Peter Maydell
2016-02-15 12:16     ` Andrew Jones
2016-02-15 12:27       ` Pavel Fedin
2016-02-15 10:09   ` Peter Xu
2016-02-15  9:52 ` [Qemu-devel] " Markus Armbruster
2016-02-15 10:34   ` Peter Xu
2016-02-15 15:08     ` Markus Armbruster
2016-02-15 15:21       ` Peter Maydell
2016-02-15 19:40         ` Markus Armbruster
2016-02-15 20:18           ` Andrew Jones
2016-02-15 20:32             ` Peter Maydell
2016-02-16 10:10               ` Markus Armbruster
2016-02-16 10:15                 ` Daniel P. Berrange
2016-02-16 12:05                   ` [Qemu-devel] [libvirt] " Andrea Bolognani
2016-02-16 12:09                     ` Peter Maydell
2016-02-16 12:20                       ` Andrea Bolognani
2016-02-16 12:15                     ` Daniel P. Berrange
2016-02-16 12:27                       ` Andrea Bolognani
2016-02-16 12:38                         ` Daniel P. Berrange
2016-02-16 13:14                           ` Andrea Bolognani
2016-02-15 15:22       ` [Qemu-devel] " Daniel P. Berrange
2016-02-18  4:40         ` Peter Xu
2016-02-18 16:52           ` Andrew Jones
2016-02-18 17:10             ` Andrea Bolognani
2016-02-19  1:55               ` Peter Xu
2016-02-19 12:33                 ` Andrea Bolognani
2016-02-22  1:35                   ` Peter Xu
2016-02-29 16:30                     ` Andrea Bolognani
2016-03-01  2:19                       ` 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).