Re: [Qemu-arm] [Qemu-devel] [PATCH v3 3/3] arm: implement query-gic-capability

[Andrew Jones <drjones@redhat.com>]

On Mon, Mar 07, 2016 at 12:23:28PM +0800, Peter Xu 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 | 105 ++++++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 104 insertions(+), 1 deletion(-)
> 
> diff --git a/target-arm/machine.c b/target-arm/machine.c
> index 60bd5c1..ff50411 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,108 @@ 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
> +/*
> + * This is merely the same as kvm_create_device(). The only
> + * difference is we are using raw fds rather than KVMState, so that
> + * we can use it even without kvm_state initialized.
> + */
> +static int kvm_create_device_fds(int kvm_fd, int vmfd,
> +                                 uint64_t type, bool test)

I don't think we need this helper function. Who else will call it?
Particularly without test==true? Anyway, I think three ioctls directly
called from qmp_query_gic_capability should be OK.

> +{
> +    int ret;
> +    struct kvm_create_device create_dev;
> +
> +    create_dev.type = type;
> +    create_dev.fd = -1;
> +    create_dev.flags = test ? KVM_CREATE_DEVICE_TEST : 0;
> +
> +    if (ioctl(kvm_fd, KVM_CHECK_EXTENSION, KVM_CAP_DEVICE_CTRL) <= 0) {
> +        return -ENOTSUP;
> +    }
> +
> +    ret = ioctl(vmfd, KVM_CREATE_DEVICE, &create_dev);
> +    if (ret) {
> +        return ret;
> +    }
> +
> +    return test ? 0 : create_dev.fd;
> +}
> +#endif
> +
>  GICCapabilityList *qmp_query_gic_capability(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
> +    {
> +        /*
> +         * HACK: here we create one temporary VM, do the probing,
> +         * then release it properly.
> +         */
> +        int kvm_fd = -1;
> +        int vmfd = -1;
> +
> +        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;
> +        }
> +
> +        /* Test KVM GICv2 */
> +        if (kvm_create_device_fds(kvm_fd, vmfd, KVM_DEV_TYPE_ARM_VGIC_V2,
> +                                  true) >= 0) {
> +            v2->kernel = true;
> +        }
> +
> +        /* Test KVM GICv3 */
> +        if (kvm_create_device_fds(kvm_fd, vmfd, KVM_DEV_TYPE_ARM_VGIC_V3,
> +                                  true) >= 0) {
> +            v3->kernel = true;
> +        }
> +
> +        close(vmfd);
> +kvm_fd_close:
> +        close(kvm_fd);
> +    }
> +#endif
> +
> +out:
> +    head = gic_cap_list_add(head, v2);
> +    head = gic_cap_list_add(head, v3);
> +
> +    return head;
>  }
> -- 
> 2.4.3
> 
>