[Qemu-devel] [PATCH v4 0/3] ARM: add query-gic-capabilities SMP command

[Qemu-devel] [PATCH v4 0/3] ARM: add query-gic-capabilities SMP command

[Peter Xu]

v4 changes:
- all: rename query-gic-capability to query-gic-capabilities [Andrea]
- patch 3: rename helper function to kvm_support_device, make it
  inline and lighter. [Drew]

v3 changes:
- patch 2: remove func declaration, add qmp header [Drew]
- patch 3: being able to detect KVM GIC capabilities even without
  kvm enabled [Andrea]: this is a little bit hacky, need some more
  review on this.

v2 changes:
- result layout change: use array and dict for the capability bits
  rather than a single array of strings [Andrea/Markus]
- spelling out what GIC is in doc [Eric]

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

The new command can report which kind of GIC device the host/QEMU
support. The returned result is in the form of array.

Sample command and output:

{"execute": "query-gic-capability"}
{"return": [{"emulated": false, "version": 3, "kernel": false},
            {"emulated": true, "version": 2, "kernel": true}]}

Testing:

Smoke tests on both x86 (emulated) and another moonshot ARM server.

Peter Xu (3):
  arm: qmp: add GICCapability struct
  arm: qmp: add query-gic-capabilities interface
  arm: implement query-gic-capabilities

 monitor.c            |  8 +++++
 qapi-schema.json     | 33 ++++++++++++++++++
 qmp-commands.hx      | 26 ++++++++++++++
 scripts/qapi.py      |  1 +
 target-arm/machine.c | 98 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 166 insertions(+)

-- 
2.4.3

[Qemu-devel] [PATCH v4 1/3] arm: qmp: add GICCapability struct

[Peter Xu]

Define new struct to describe whether we support specific GIC version.

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

diff --git a/qapi-schema.json b/qapi-schema.json
index 7b8f2a1..0b2de6c 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -4135,3 +4135,25 @@
 ##
 { 'enum': 'ReplayMode',
   'data': [ 'none', 'record', 'play' ] }
+
+##
+# @GICCapability:
+#
+# This struct describes capability for a specific GIC version. These
+# bits are not only decided by QEMU/KVM software version, but also
+# decided by the hardware that the program is running upon.
+#
+# @version:  version of GIC to be described.
+#
+# @emulated: whether current QEMU/hardware supports emulated GIC
+#            device in user space.
+#
+# @kernel:   whether current QEMU/hardware supports hardware
+#            accelerated GIC device in kernel.
+#
+# Since: 2.6
+##
+{ 'struct': 'GICCapability',
+  'data': { 'version': 'int',
+            'emulated': 'bool',
+            'kernel': 'bool' } }
-- 
2.4.3

[Qemu-devel] [PATCH v4 2/3] arm: qmp: add query-gic-capabilities interface

[Peter Xu]

This patch adds the command "query-gic-capabilities" but not implemnet
it. The command is ARM-only. Return of the command is a list of
GICCapability struct that describes all GIC versions that current QEMU
and system support.

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

diff --git a/monitor.c b/monitor.c
index 73eac17..9e8cbdb 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
+GICCapabilityList *qmp_query_gic_capabilities(Error **errp)
+{
+    error_setg(errp, QERR_FEATURE_DISABLED, "query-gic-capabilities");
+    return NULL;
+}
+#endif
diff --git a/qapi-schema.json b/qapi-schema.json
index 0b2de6c..9c0a503 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -4157,3 +4157,14 @@
   'data': { 'version': 'int',
             'emulated': 'bool',
             'kernel': 'bool' } }
+
+##
+# @query-gic-capabilities:
+#
+# Return a list of supported GIC version capabilities.
+#
+# Returns: a list of GICCapability.
+#
+# Since: 2.6
+##
+{ 'command': 'query-gic-capabilities', 'returns': ['GICCapability'] }
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 13f158d..c003838 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -4852,3 +4852,29 @@ Example:
                  {"type": 0, "out-pport": 0, "pport": 0, "vlan-id": 3840,
                   "pop-vlan": 1, "id": 251658240}
    ]}
+
+EQMP
+
+#if defined TARGET_ARM
+    {
+        .name       = "query-gic-capabilities",
+        .args_type  = "",
+        .mhandler.cmd_new = qmp_marshal_query_gic_capabilities,
+    },
+#endif
+
+SQMP
+query-gic-capabilities
+---------------
+
+Return a list of supported ARM GIC versions and their capabilities.
+
+Arguments: None
+
+Example:
+
+-> { "execute": "query-gic-capabilities" }
+<- { "return": [{ "version": 2, "emulated": true, "kernel": false },
+                { "version": 3, "emulated": false, "kernel": true } ] }
+
+EQMP
diff --git a/scripts/qapi.py b/scripts/qapi.py
index 8497777..9dc8f73 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 03a73d9..813909e 100644
--- a/target-arm/machine.c
+++ b/target-arm/machine.c
@@ -5,6 +5,7 @@
 #include "sysemu/kvm.h"
 #include "kvm_arm.h"
 #include "internals.h"
+#include "qmp-commands.h"
 
 static bool vfp_needed(void *opaque)
 {
@@ -345,3 +346,8 @@ const char *gicv3_class_name(void)
 
     exit(1);
 }
+
+GICCapabilityList *qmp_query_gic_capabilities(Error **errp)
+{
+    return NULL;
+}
-- 
2.4.3

Re: [Qemu-devel] [PATCH v4 2/3] arm: qmp: add query-gic-capabilities interface

[Peter Maydell]

On 8 March 2016 at 07:36, Peter Xu <peterx@redhat.com> wrote:
> This patch adds the command "query-gic-capabilities" but not implemnet
> it. The command is ARM-only. Return of the command is a list of
> GICCapability struct that describes all GIC versions that current QEMU
> and system support.
>
> Signed-off-by: Peter Xu <peterx@redhat.com>
> ---
>  monitor.c            |  8 ++++++++
>  qapi-schema.json     | 11 +++++++++++
>  qmp-commands.hx      | 26 ++++++++++++++++++++++++++
>  scripts/qapi.py      |  1 +
>  target-arm/machine.c |  6 ++++++
>  5 files changed, 52 insertions(+)
>
> diff --git a/monitor.c b/monitor.c
> index 73eac17..9e8cbdb 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
> +GICCapabilityList *qmp_query_gic_capabilities(Error **errp)
> +{
> +    error_setg(errp, QERR_FEATURE_DISABLED, "query-gic-capabilities");
> +    return NULL;
> +}
> +#endif

Given where we are in the release cycle I guess we need to do this,
but longer term we should sort out a structure so we can add
target-specific qmp and hmp commands without having to add more
TARGET_* ifdefs to common files...

> diff --git a/target-arm/machine.c b/target-arm/machine.c
> index 03a73d9..813909e 100644
> --- a/target-arm/machine.c
> +++ b/target-arm/machine.c
> @@ -5,6 +5,7 @@
>  #include "sysemu/kvm.h"
>  #include "kvm_arm.h"
>  #include "internals.h"
> +#include "qmp-commands.h"
>
>  static bool vfp_needed(void *opaque)
>  {
> @@ -345,3 +346,8 @@ const char *gicv3_class_name(void)
>
>      exit(1);
>  }
> +
> +GICCapabilityList *qmp_query_gic_capabilities(Error **errp)
> +{
> +    return NULL;
> +}

Why is this here? machine.c is for migration code.

thanks
-- PMM

Re: [Qemu-devel] [PATCH v4 2/3] arm: qmp: add query-gic-capabilities interface

[Peter Xu]

On Wed, Mar 16, 2016 at 10:24:39AM +0000, Peter Maydell wrote:
> On 8 March 2016 at 07:36, Peter Xu <peterx@redhat.com> wrote:
> > diff --git a/target-arm/machine.c b/target-arm/machine.c
> > index 03a73d9..813909e 100644
> > --- a/target-arm/machine.c
> > +++ b/target-arm/machine.c
> > @@ -5,6 +5,7 @@
> >  #include "sysemu/kvm.h"
> >  #include "kvm_arm.h"
> >  #include "internals.h"
> > +#include "qmp-commands.h"
> >
> >  static bool vfp_needed(void *opaque)
> >  {
> > @@ -345,3 +346,8 @@ const char *gicv3_class_name(void)
> >
> >      exit(1);
> >  }
> > +
> > +GICCapabilityList *qmp_query_gic_capabilities(Error **errp)
> > +{
> > +    return NULL;
> > +}
> 
> Why is this here? machine.c is for migration code.

Hi, Peter,

Thanks for the review comments. Do you have any suggestion on which
file should I add this in (or create a new one)? I failed to figure
it out myself. :(

TIA.

-- peterx

Re: [Qemu-devel] [PATCH v4 2/3] arm: qmp: add query-gic-capabilities interface

[Peter Maydell]

On 17 March 2016 at 04:09, Peter Xu <peterx@redhat.com> wrote:
> Thanks for the review comments. Do you have any suggestion on which
> file should I add this in (or create a new one)? I failed to figure
> it out myself. :(

I suggest target-arm/monitor.c (we have a monitor.c for other
target-* already for monitor related functions).

thanks
-- PMM

Re: [Qemu-devel] [PATCH v4 2/3] arm: qmp: add query-gic-capabilities interface

[Peter Xu]

On Thu, Mar 17, 2016 at 08:50:38AM +0000, Peter Maydell wrote:
> On 17 March 2016 at 04:09, Peter Xu <peterx@redhat.com> wrote:
> > Thanks for the review comments. Do you have any suggestion on which
> > file should I add this in (or create a new one)? I failed to figure
> > it out myself. :(
> 
> I suggest target-arm/monitor.c (we have a monitor.c for other
> target-* already for monitor related functions).

Will take your advice and respin. Thanks!

-- peterx

[Qemu-devel] [PATCH v4 3/3] arm: implement query-gic-capabilities

[Peter Xu]

For emulated GIC capabilities, currently 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.

When probing the KVM capabilities, we cannot leverage existing helper
functions like kvm_create_device() since QEMU might be using TCG while
probing (actually this is the case for libvirt probing). So, one
temporary VM is created to do the probing.

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

diff --git a/target-arm/machine.c b/target-arm/machine.c
index 813909e..8f52f74 100644
--- a/target-arm/machine.c
+++ b/target-arm/machine.c
@@ -1,3 +1,5 @@
+#include <linux/kvm.h>
+#include <sys/ioctl.h>
 #include "qemu/osdep.h"
 #include "hw/hw.h"
 #include "hw/boards.h"
@@ -347,7 +349,97 @@ const char *gicv3_class_name(void)
     exit(1);
 }
 
+static GICCapability *gic_cap_new(int version)
+{
+    GICCapability *cap = g_new0(GICCapability, 1);
+    cap->version = version;
+    /* by default, support none */
+    cap->emulated = false;
+    cap->kernel = false;
+    return cap;
+}
+
+static GICCapabilityList *gic_cap_list_add(GICCapabilityList *head,
+                                           GICCapability *cap)
+{
+    GICCapabilityList *item = g_new0(GICCapabilityList, 1);
+    item->value = cap;
+    item->next = head;
+    return item;
+}
+
+#ifdef CONFIG_KVM
+/* Test whether KVM support specific device. */
+static inline int kvm_support_device(int vmfd, uint64_t type)
+{
+    struct kvm_create_device create_dev = {
+        .type = type,
+        .fd = -1,
+        .flags = KVM_CREATE_DEVICE_TEST,
+    };
+    return ioctl(vmfd, KVM_CREATE_DEVICE, &create_dev);
+}
+#endif
+
 GICCapabilityList *qmp_query_gic_capabilities(Error **errp)
 {
-    return NULL;
+    GICCapabilityList *head = NULL;
+    GICCapability *v2 = gic_cap_new(2), *v3 = gic_cap_new(3);
+
+    v2->emulated = true;
+    /* FIXME: we'd change to true after we get emulated GICv3. */
+    v3->emulated = false;
+
+#ifdef CONFIG_KVM
+    {
+        int kvm_fd = -1;
+        int vmfd = -1;
+        /*
+         * HACK: here we create one temporary VM, do the probing,
+         * then release it properly.
+         */
+        kvm_fd = qemu_open("/dev/kvm", O_RDWR);
+        if (kvm_fd == -1) {
+            /* KVM may not enabled on host, which is fine. */
+            goto out;
+        }
+
+        do {
+            /* For ARM, VM type could only be zero now. */
+            vmfd = ioctl(kvm_fd, KVM_CREATE_VM, 0);
+        } while (vmfd == -EINTR);
+
+        if (vmfd < 0) {
+            goto kvm_fd_close;
+        }
+
+        if (ioctl(kvm_fd, KVM_CHECK_EXTENSION,
+                  KVM_CAP_DEVICE_CTRL) <= 0) {
+            /* older version of KVM possibly */
+            goto kvm_vmfd_close;
+        }
+
+        /* Test KVM GICv2 */
+        if (kvm_support_device(vmfd, KVM_DEV_TYPE_ARM_VGIC_V2) >= 0) {
+            v2->kernel = true;
+        }
+
+        /* Test KVM GICv3 */
+        if (kvm_support_device(vmfd, KVM_DEV_TYPE_ARM_VGIC_V3) >= 0) {
+            v3->kernel = true;
+        }
+
+kvm_vmfd_close:
+        close(vmfd);
+kvm_fd_close:
+        close(kvm_fd);
+out:
+        ;
+    }
+#endif
+
+    head = gic_cap_list_add(head, v2);
+    head = gic_cap_list_add(head, v3);
+
+    return head;
 }
-- 
2.4.3

Re: [Qemu-devel] [PATCH v4 3/3] arm: implement query-gic-capabilities

[Peter Maydell]

On 8 March 2016 at 07:36, Peter Xu <peterx@redhat.com> wrote:
> For emulated GIC capabilities, currently 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.
>
> When probing the KVM capabilities, we cannot leverage existing helper
> functions like kvm_create_device() since QEMU might be using TCG while
> probing (actually this is the case for libvirt probing). So, one
> temporary VM is created to do the probing.
>
> Signed-off-by: Peter Xu <peterx@redhat.com>
> ---
>  target-arm/machine.c | 94 +++++++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 93 insertions(+), 1 deletion(-)
>
> diff --git a/target-arm/machine.c b/target-arm/machine.c
> index 813909e..8f52f74 100644
> --- a/target-arm/machine.c
> +++ b/target-arm/machine.c
> @@ -1,3 +1,5 @@
> +#include <linux/kvm.h>

This will break compilation on non-Linux hosts; you can't include
linux headers like this.

(This is all in the wrong file anyway, machine.c is for migration.)

> +#include <sys/ioctl.h>
>  #include "qemu/osdep.h"
>  #include "hw/hw.h"
>  #include "hw/boards.h"
> @@ -347,7 +349,97 @@ const char *gicv3_class_name(void)
>      exit(1);
>  }
>
> +static GICCapability *gic_cap_new(int version)
> +{
> +    GICCapability *cap = g_new0(GICCapability, 1);
> +    cap->version = version;
> +    /* by default, support none */
> +    cap->emulated = false;
> +    cap->kernel = false;
> +    return cap;
> +}
> +
> +static GICCapabilityList *gic_cap_list_add(GICCapabilityList *head,
> +                                           GICCapability *cap)
> +{
> +    GICCapabilityList *item = g_new0(GICCapabilityList, 1);
> +    item->value = cap;
> +    item->next = head;
> +    return item;
> +}
> +
> +#ifdef CONFIG_KVM
> +/* Test whether KVM support specific device. */
> +static inline int kvm_support_device(int vmfd, uint64_t type)
> +{
> +    struct kvm_create_device create_dev = {
> +        .type = type,
> +        .fd = -1,
> +        .flags = KVM_CREATE_DEVICE_TEST,
> +    };
> +    return ioctl(vmfd, KVM_CREATE_DEVICE, &create_dev);
> +}
> +#endif

This is not ARM specific so it should go in kvm-all.c.

> +
>  GICCapabilityList *qmp_query_gic_capabilities(Error **errp)
>  {
> -    return NULL;
> +    GICCapabilityList *head = NULL;
> +    GICCapability *v2 = gic_cap_new(2), *v3 = gic_cap_new(3);
> +
> +    v2->emulated = true;
> +    /* FIXME: we'd change to true after we get emulated GICv3. */
> +    v3->emulated = false;
> +
> +#ifdef CONFIG_KVM

KVM specific code should be factored out and live in one of
the target-arm/kvm*.c files.

> +    {
> +        int kvm_fd = -1;
> +        int vmfd = -1;
> +        /*
> +         * HACK: here we create one temporary VM, do the probing,
> +         * then release it properly.
> +         */
> +        kvm_fd = qemu_open("/dev/kvm", O_RDWR);
> +        if (kvm_fd == -1) {
> +            /* KVM may not enabled on host, which is fine. */
> +            goto out;
> +        }
> +
> +        do {
> +            /* For ARM, VM type could only be zero now. */
> +            vmfd = ioctl(kvm_fd, KVM_CREATE_VM, 0);
> +        } while (vmfd == -EINTR);
> +
> +        if (vmfd < 0) {
> +            goto kvm_fd_close;
> +        }

Rather than open-coding this you might as well use
kvm_arm_creat_scratch_host_vcpu() (you don't need the vcpu fd but
it's pretty harmless to create it.)


> +
> +        if (ioctl(kvm_fd, KVM_CHECK_EXTENSION,
> +                  KVM_CAP_DEVICE_CTRL) <= 0) {
> +            /* older version of KVM possibly */
> +            goto kvm_vmfd_close;
> +        }

Do this in kvm_support_device() [mostly just to parallel how
kvm_create_device() does it.]

> +
> +        /* Test KVM GICv2 */
> +        if (kvm_support_device(vmfd, KVM_DEV_TYPE_ARM_VGIC_V2) >= 0) {
> +            v2->kernel = true;
> +        }
> +
> +        /* Test KVM GICv3 */
> +        if (kvm_support_device(vmfd, KVM_DEV_TYPE_ARM_VGIC_V3) >= 0) {
> +            v3->kernel = true;
> +        }
> +
> +kvm_vmfd_close:
> +        close(vmfd);
> +kvm_fd_close:
> +        close(kvm_fd);
> +out:
> +        ;
> +    }
> +#endif
> +
> +    head = gic_cap_list_add(head, v2);
> +    head = gic_cap_list_add(head, v3);
> +
> +    return head;
>  }
> --
> 2.4.3

thanks
-- PMM

Re: [Qemu-devel] [PATCH v4 3/3] arm: implement query-gic-capabilities

[Peter Xu]

On Wed, Mar 16, 2016 at 10:30:36AM +0000, Peter Maydell wrote:
> On 8 March 2016 at 07:36, Peter Xu <peterx@redhat.com> wrote:
> > For emulated GIC capabilities, currently 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.
> >
> > When probing the KVM capabilities, we cannot leverage existing helper
> > functions like kvm_create_device() since QEMU might be using TCG while
> > probing (actually this is the case for libvirt probing). So, one
> > temporary VM is created to do the probing.
> >
> > Signed-off-by: Peter Xu <peterx@redhat.com>
> > ---
> >  target-arm/machine.c | 94 +++++++++++++++++++++++++++++++++++++++++++++++++++-
> >  1 file changed, 93 insertions(+), 1 deletion(-)
> >
> > diff --git a/target-arm/machine.c b/target-arm/machine.c
> > index 813909e..8f52f74 100644
> > --- a/target-arm/machine.c
> > +++ b/target-arm/machine.c
> > @@ -1,3 +1,5 @@
> > +#include <linux/kvm.h>
> 
> This will break compilation on non-Linux hosts; you can't include
> linux headers like this.
> 
> (This is all in the wrong file anyway, machine.c is for migration.)

Okay. Will move all kvm lines into kvm*.c.

> 
> > +#include <sys/ioctl.h>
> >  #include "qemu/osdep.h"
> >  #include "hw/hw.h"
> >  #include "hw/boards.h"
> > @@ -347,7 +349,97 @@ const char *gicv3_class_name(void)
> >      exit(1);
> >  }
> >
> > +static GICCapability *gic_cap_new(int version)
> > +{
> > +    GICCapability *cap = g_new0(GICCapability, 1);
> > +    cap->version = version;
> > +    /* by default, support none */
> > +    cap->emulated = false;
> > +    cap->kernel = false;
> > +    return cap;
> > +}
> > +
> > +static GICCapabilityList *gic_cap_list_add(GICCapabilityList *head,
> > +                                           GICCapability *cap)
> > +{
> > +    GICCapabilityList *item = g_new0(GICCapabilityList, 1);
> > +    item->value = cap;
> > +    item->next = head;
> > +    return item;
> > +}
> > +
> > +#ifdef CONFIG_KVM
> > +/* Test whether KVM support specific device. */
> > +static inline int kvm_support_device(int vmfd, uint64_t type)
> > +{
> > +    struct kvm_create_device create_dev = {
> > +        .type = type,
> > +        .fd = -1,
> > +        .flags = KVM_CREATE_DEVICE_TEST,
> > +    };
> > +    return ioctl(vmfd, KVM_CREATE_DEVICE, &create_dev);
> > +}
> > +#endif
> 
> This is not ARM specific so it should go in kvm-all.c.

Will do.

> 
> > +
> >  GICCapabilityList *qmp_query_gic_capabilities(Error **errp)
> >  {
> > -    return NULL;
> > +    GICCapabilityList *head = NULL;
> > +    GICCapability *v2 = gic_cap_new(2), *v3 = gic_cap_new(3);
> > +
> > +    v2->emulated = true;
> > +    /* FIXME: we'd change to true after we get emulated GICv3. */
> > +    v3->emulated = false;
> > +
> > +#ifdef CONFIG_KVM
> 
> KVM specific code should be factored out and live in one of
> the target-arm/kvm*.c files.

Ok, will fix.

> 
> > +    {
> > +        int kvm_fd = -1;
> > +        int vmfd = -1;
> > +        /*
> > +         * HACK: here we create one temporary VM, do the probing,
> > +         * then release it properly.
> > +         */
> > +        kvm_fd = qemu_open("/dev/kvm", O_RDWR);
> > +        if (kvm_fd == -1) {
> > +            /* KVM may not enabled on host, which is fine. */
> > +            goto out;
> > +        }
> > +
> > +        do {
> > +            /* For ARM, VM type could only be zero now. */
> > +            vmfd = ioctl(kvm_fd, KVM_CREATE_VM, 0);
> > +        } while (vmfd == -EINTR);
> > +
> > +        if (vmfd < 0) {
> > +            goto kvm_fd_close;
> > +        }
> 
> Rather than open-coding this you might as well use
> kvm_arm_creat_scratch_host_vcpu() (you don't need the vcpu fd but
> it's pretty harmless to create it.)

Thanks for the func pointer! That's what I was looking for.

> 
> 
> > +
> > +        if (ioctl(kvm_fd, KVM_CHECK_EXTENSION,
> > +                  KVM_CAP_DEVICE_CTRL) <= 0) {
> > +            /* older version of KVM possibly */
> > +            goto kvm_vmfd_close;
> > +        }
> 
> Do this in kvm_support_device() [mostly just to parallel how
> kvm_create_device() does it.]

Will fix.

Thanks!

-- peterx

Re: [Qemu-devel] [PATCH v4 0/3] ARM: add query-gic-capabilities SMP command

[Peter Maydell]

On 8 March 2016 at 07:36, Peter Xu <peterx@redhat.com> wrote:
> v4 changes:
> - all: rename query-gic-capability to query-gic-capabilities [Andrea]
> - patch 3: rename helper function to kvm_support_device, make it
>   inline and lighter. [Drew]
>
> v3 changes:
> - patch 2: remove func declaration, add qmp header [Drew]
> - patch 3: being able to detect KVM GIC capabilities even without
>   kvm enabled [Andrea]: this is a little bit hacky, need some more
>   review on this.
>
> v2 changes:
> - result layout change: use array and dict for the capability bits
>   rather than a single array of strings [Andrea/Markus]
> - spelling out what GIC is in doc [Eric]
>
> This patch is to add ARM-specific command "query-gic-capability".
>
> The new command can report which kind of GIC device the host/QEMU
> support. The returned result is in the form of array.

Hi. I've made some code review comments on the specifics of the
implementation, but really what I'd like to see is:
 * an ack from the libvirt folks that this API meets their
   requirements
 * an ack/review frem Eric or somebody else familiar with qmp
   that it's OK from a protocol perspective

since I'm not really able to judge those aspects.

thanks
-- PMM

Re: [Qemu-devel] [PATCH v4 0/3] ARM: add query-gic-capabilities SMP command

[Andrea Bolognani]

On Wed, 2016-03-16 at 10:32 +0000, Peter Maydell wrote:
> On 8 March 2016 at 07:36, Peter Xu <peterx@redhat.com> wrote:
> > 
> > v4 changes:
> > - all: rename query-gic-capability to query-gic-capabilities [Andrea]
> > - patch 3: rename helper function to kvm_support_device, make it
> >   inline and lighter. [Drew]
> > 
> > v3 changes:
> > - patch 2: remove func declaration, add qmp header [Drew]
> > - patch 3: being able to detect KVM GIC capabilities even without
> >   kvm enabled [Andrea]: this is a little bit hacky, need some more
> >   review on this.
> > 
> > v2 changes:
> > - result layout change: use array and dict for the capability bits
> >   rather than a single array of strings [Andrea/Markus]
> > - spelling out what GIC is in doc [Eric]
> > 
> > This patch is to add ARM-specific command "query-gic-capability".
> > 
> > The new command can report which kind of GIC device the host/QEMU
> > support. The returned result is in the form of array.
> 
> Hi. I've made some code review comments on the specifics of the
> implementation, but really what I'd like to see is:
>  * an ack from the libvirt folks that this API meets their
>    requirements

I have a working implementation of the libvirt part, but I need
to polish it up before it can be posted on the list; then other
libvirt folks will be able to provide feedback.

Cheers.

-- 
Andrea Bolognani
Software Engineer - Virtualization Team