Re: [Qemu-devel] [PATCH 3/4] kvm: Common device control API functions

[Peter Maydell <peter.maydell@linaro.org>]

On 23 August 2013 20:41, Christoffer Dall <christoffer.dall@linaro.org> wrote:
> Introduces two simple functions:
>     int kvm_device_ioctl(int fd, int type, ...);
>     int kvm_create_device(KVMState *s, uint64_t type, bool test);
>
> These functions wrap the basic ioctl-based interactions with KVM in a
> way similar to other KVM ioctl wrappers.
>
> Signed-off-by: Christoffer Dall <christoffer.dall@linaro.org>
> ---
>  include/sysemu/kvm.h |    5 +++++
>  kvm-all.c            |   39 +++++++++++++++++++++++++++++++++++++++
>  trace-events         |    1 +
>  3 files changed, 45 insertions(+)
>
> diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h
> index 1e5847e..84ca5ef 100644
> --- a/include/sysemu/kvm.h
> +++ b/include/sysemu/kvm.h
> @@ -190,6 +190,11 @@ int kvm_vm_ioctl(KVMState *s, int type, ...);
>
>  int kvm_vcpu_ioctl(CPUState *cpu, int type, ...);
>
> +int kvm_device_ioctl(int fd, int type, ...);
> +
> +int kvm_create_device(KVMState *s, uint64_t type, bool test);

Could we have doc comments for these, please?

> +
> +
>  /* Arch specific hooks */
>
>  extern const KVMCapabilityInfo kvm_arch_required_capabilities[];
> diff --git a/kvm-all.c b/kvm-all.c
> index fe64f3b..957b961 100644
> --- a/kvm-all.c
> +++ b/kvm-all.c
> @@ -1770,6 +1770,24 @@ int kvm_vcpu_ioctl(CPUState *cpu, int type, ...)
>      return ret;
>  }
>
> +int kvm_device_ioctl(int fd, int type, ...)
> +{
> +    int ret;
> +    void *arg;
> +    va_list ap;
> +
> +    va_start(ap, type);
> +    arg = va_arg(ap, void *);
> +    va_end(ap);
> +
> +    trace_kvm_device_ioctl(fd, type, arg);
> +    ret = ioctl(fd, type, arg);
> +    if (ret == -1) {
> +        ret = -errno;
> +    }
> +    return ret;
> +}
> +
>  int kvm_has_sync_mmu(void)
>  {
>      return kvm_check_extension(kvm_state, KVM_CAP_SYNC_MMU);
> @@ -2064,3 +2082,24 @@ int kvm_on_sigbus(int code, void *addr)
>  {
>      return kvm_arch_on_sigbus(code, addr);
>  }
> +
> +int kvm_create_device(KVMState *s, uint64_t type, bool test)
> +{
> +    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;

Why the brackets round 'test' ?

> +
> +    if (!kvm_check_extension(s, KVM_CAP_DEVICE_CTRL)) {
> +        return -1;

We should probably return -$some_errno here, since we
pass through a -errno return from kvm_vm_ioctl below.

> +    }
> +
> +    ret = kvm_vm_ioctl(s, KVM_CREATE_DEVICE, &create_dev);
> +    if (ret) {
> +        return ret;
> +    }
> +
> +    return (test) ? 0 : create_dev.fd;
> +}

thanks
-- PMM