diff mbox

[v7,2/4] arm: enhance kvm_arm_create_scratch_host_vcpu

Message ID 1458788142-17509-3-git-send-email-peterx@redhat.com (mailing list archive)
State New, archived
Headers show

Commit Message

Peter Xu March 24, 2016, 2:55 a.m. UTC
Some more lines to make sure we allow NULL for 1st/3rd parameter.

Signed-off-by: Peter Xu <peterx@redhat.com>
---
 target-arm/kvm.c     | 14 +++++++++++++-
 target-arm/kvm_arm.h |  6 ++++--
 2 files changed, 17 insertions(+), 3 deletions(-)

Comments

Sergey Fedorov March 24, 2016, 9:13 a.m. UTC | #1
On 24/03/16 05:55, Peter Xu wrote:
> Some more lines to make sure we allow NULL for 1st/3rd parameter.
>
> Signed-off-by: Peter Xu <peterx@redhat.com>

Acked-by: Sergey Fedorov <serge.fdrv@gmail.com>

Kind regards,
Sergey

> ---
>  target-arm/kvm.c     | 14 +++++++++++++-
>  target-arm/kvm_arm.h |  6 ++++--
>  2 files changed, 17 insertions(+), 3 deletions(-)
>
> diff --git a/target-arm/kvm.c b/target-arm/kvm.c
> index 969ab0b..bb6935e 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,16 @@ bool kvm_arm_create_scratch_host_vcpu(const uint32_t *cpus_to_try,
>          if (ret < 0) {
>              goto err;
>          }
> +    } else {
> +        /*
> +         * Here, we got init != NULL but cpus_to_retry == NULL,
> +         * meanwhile, we failed to probe one preferred target type
> +         * (no matter what). So we fail.
> +         */
> +        goto err;
>      }
>  
> +finish:
>      fdarray[0] = kvmfd;
>      fdarray[1] = vmfd;
>      fdarray[2] = cpufd;
> diff --git a/target-arm/kvm_arm.h b/target-arm/kvm_arm.h
> index 07f0c72..4f01a99 100644
> --- a/target-arm/kvm_arm.h
> +++ b/target-arm/kvm_arm.h
> @@ -124,9 +124,11 @@ void kvm_arm_reset_vcpu(ARMCPU *cpu);
>   * kvm_arm_create_scratch_host_vcpu:
>   * @cpus_to_try: array of QEMU_KVM_ARM_TARGET_* values (terminated with
>   * QEMU_KVM_ARM_TARGET_NONE) to try as fallback if the kernel does not
> - * know the PREFERRED_TARGET ioctl
> + * know the PREFERRED_TARGET ioctl. If NULL is provided, will try
> + * to use perferred target, and fail if no preferred found.
>   * @fdarray: filled in with kvmfd, vmfd, cpufd file descriptors in that order
> - * @init: filled in with the necessary values for creating a host vcpu
> + * @init: filled in with the necessary values for creating a host
> + * vcpu. If NULL is provided, will not init the vCPU.
>   *
>   * Create a scratch vcpu in its own VM of the type preferred by the host
>   * kernel (as would be used for '-cpu host'), for purposes of probing it
Peter Maydell March 29, 2016, 8:04 p.m. UTC | #2
On 24 March 2016 at 02:55, Peter Xu <peterx@redhat.com> wrote:
> Some more lines to make sure we allow NULL for 1st/3rd parameter.
>
> Signed-off-by: Peter Xu <peterx@redhat.com>
> ---
>  target-arm/kvm.c     | 14 +++++++++++++-
>  target-arm/kvm_arm.h |  6 ++++--
>  2 files changed, 17 insertions(+), 3 deletions(-)
>
> diff --git a/target-arm/kvm.c b/target-arm/kvm.c
> index 969ab0b..bb6935e 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,16 @@ bool kvm_arm_create_scratch_host_vcpu(const uint32_t *cpus_to_try,
>          if (ret < 0) {
>              goto err;
>          }
> +    } else {
> +        /*
> +         * Here, we got init != NULL but cpus_to_retry == NULL,
> +         * meanwhile, we failed to probe one preferred target type
> +         * (no matter what). So we fail.
> +         */
> +        goto err;

aka "treat a NULL cpus_to_retry like an empty list".

>      }
>
> +finish:
>      fdarray[0] = kvmfd;
>      fdarray[1] = vmfd;
>      fdarray[2] = cpufd;
> diff --git a/target-arm/kvm_arm.h b/target-arm/kvm_arm.h
> index 07f0c72..4f01a99 100644
> --- a/target-arm/kvm_arm.h
> +++ b/target-arm/kvm_arm.h
> @@ -124,9 +124,11 @@ void kvm_arm_reset_vcpu(ARMCPU *cpu);
>   * kvm_arm_create_scratch_host_vcpu:
>   * @cpus_to_try: array of QEMU_KVM_ARM_TARGET_* values (terminated with
>   * QEMU_KVM_ARM_TARGET_NONE) to try as fallback if the kernel does not
> - * know the PREFERRED_TARGET ioctl
> + * know the PREFERRED_TARGET ioctl. If NULL is provided, will try
> + * to use perferred target, and fail if no preferred found.

"preferred". Better phrased as "a NULL pointer is treated like an
empty array", I think.

>   * @fdarray: filled in with kvmfd, vmfd, cpufd file descriptors in that order
> - * @init: filled in with the necessary values for creating a host vcpu
> + * @init: filled in with the necessary values for creating a host
> + * vcpu. If NULL is provided, will not init the vCPU.
>   *
>   * Create a scratch vcpu in its own VM of the type preferred by the host
>   * kernel (as would be used for '-cpu host'), for purposes of probing it
> --
> 2.4.3
>

(I'll fix this up in target-arm.next, it's a minor nit.)

thanks
-- PMM
diff mbox

Patch

diff --git a/target-arm/kvm.c b/target-arm/kvm.c
index 969ab0b..bb6935e 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,16 @@  bool kvm_arm_create_scratch_host_vcpu(const uint32_t *cpus_to_try,
         if (ret < 0) {
             goto err;
         }
+    } else {
+        /*
+         * Here, we got init != NULL but cpus_to_retry == NULL,
+         * meanwhile, we failed to probe one preferred target type
+         * (no matter what). So we fail.
+         */
+        goto err;
     }
 
+finish:
     fdarray[0] = kvmfd;
     fdarray[1] = vmfd;
     fdarray[2] = cpufd;
diff --git a/target-arm/kvm_arm.h b/target-arm/kvm_arm.h
index 07f0c72..4f01a99 100644
--- a/target-arm/kvm_arm.h
+++ b/target-arm/kvm_arm.h
@@ -124,9 +124,11 @@  void kvm_arm_reset_vcpu(ARMCPU *cpu);
  * kvm_arm_create_scratch_host_vcpu:
  * @cpus_to_try: array of QEMU_KVM_ARM_TARGET_* values (terminated with
  * QEMU_KVM_ARM_TARGET_NONE) to try as fallback if the kernel does not
- * know the PREFERRED_TARGET ioctl
+ * know the PREFERRED_TARGET ioctl. If NULL is provided, will try
+ * to use perferred target, and fail if no preferred found.
  * @fdarray: filled in with kvmfd, vmfd, cpufd file descriptors in that order
- * @init: filled in with the necessary values for creating a host vcpu
+ * @init: filled in with the necessary values for creating a host
+ * vcpu. If NULL is provided, will not init the vCPU.
  *
  * Create a scratch vcpu in its own VM of the type preferred by the host
  * kernel (as would be used for '-cpu host'), for purposes of probing it