Re: [Qemu-devel] [Qemu-arm] [PATCH v6 2/4] arm: enhance kvm_arm_create_scratch_host_vcpu

[Sergey Fedorov <serge.fdrv@gmail.com>]

On 23/03/16 08:32, Peter Xu wrote:
> diff --git a/target-arm/kvm.c b/target-arm/kvm.c
> index 969ab0b..0a7f9a6 100644
> --- a/target-arm/kvm.c
> +++ b/target-arm/kvm.c
> @@ -62,13 +62,17 @@ bool kvm_arm_create_scratch_host_vcpu(const uint32_t *cpus_to_try,
>          goto err;
>      }
>  
> +    if (!init) {
> +        goto finish;
> +    }
> +
>      ret = ioctl(vmfd, KVM_ARM_PREFERRED_TARGET, init);
>      if (ret >= 0) {
>          ret = ioctl(cpufd, KVM_ARM_VCPU_INIT, init);
>          if (ret < 0) {
>              goto err;
>          }
> -    } else {
> +    } else if (cpus_to_try) {
>          /* Old kernel which doesn't know about the
>           * PREFERRED_TARGET ioctl: we know it will only support
>           * creating one kind of guest CPU which is its preferred
> @@ -85,8 +89,12 @@ bool kvm_arm_create_scratch_host_vcpu(const uint32_t *cpus_to_try,
>          if (ret < 0) {
>              goto err;
>          }
> +    } else {
> +        /* Not providing cpus_to_try, do nothing. */
> +        ;

I think it's probably not the best idea to skip CPU initialization here.
I'd rather raise an error in such case. If we supplied non-NULL init
argument then we need VCPU been initialized, don't we? If we pass NULL
as init then we actually skip this code.

Kind regards,
Sergey

>      }
>  
> +finish:
>      fdarray[0] = kvmfd;
>      fdarray[1] = vmfd;
>      fdarray[2] = cpufd;