[Qemu-devel] [PATCH] PPC/KVM: early validation of vcpu id

[Qemu-devel] [PATCH] PPC/KVM: early validation of vcpu id

[Greg Kurz]

The KVM API restricts vcpu ids to be < KVM_CAP_MAX_VCPUS. On PowerPC
targets, depending on the number of threads per core in the host and
in the guest, some topologies do generate higher vcpu ids actually.
When this happens, QEMU bails out with the following error:

kvm_init_vcpu failed: Invalid argument

The KVM_CREATE_VCPU ioctl has several EINVAL return paths, so it is
not possible to fully disambiguate.

This patch adds a check in the code that computes vcpu ids, so that
we can detect the error earlier, and print a friendlier message instead
of calling KVM_CREATE_VCPU with an obviously bogus vcpu id.

Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com>
---
 include/sysemu/kvm.h        |    2 ++
 kvm-all.c                   |    6 ++++++
 target-ppc/translate_init.c |    8 ++++++++
 3 files changed, 16 insertions(+)

diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h
index 0e18f15c9493..27bf50ef379e 100644
--- a/include/sysemu/kvm.h
+++ b/include/sysemu/kvm.h
@@ -344,6 +344,8 @@ int kvm_arch_init(MachineState *ms, KVMState *s);
 
 int kvm_arch_init_vcpu(CPUState *cpu);
 
+bool kvm_vcpu_id_is_valid(int vcpu_id);
+
 /* Returns VCPU ID to be used on KVM_CREATE_VCPU ioctl() */
 unsigned long kvm_arch_vcpu_id(CPUState *cpu);
 
diff --git a/kvm-all.c b/kvm-all.c
index e7b66df197fd..3625a3e07457 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -1459,6 +1459,12 @@ static int kvm_max_vcpus(KVMState *s)
     return (ret) ? ret : kvm_recommended_vcpus(s);
 }
 
+bool kvm_vcpu_id_is_valid(int vcpu_id)
+{
+    KVMState *s = KVM_STATE(current_machine->accelerator);
+    return vcpu_id >= 0 && vcpu_id < kvm_max_vcpus(s);
+}
+
 static int kvm_init(MachineState *ms)
 {
     MachineClass *mc = MACHINE_GET_CLASS(ms);
diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
index f51572552bc2..6c89b18a05f9 100644
--- a/target-ppc/translate_init.c
+++ b/target-ppc/translate_init.c
@@ -9247,6 +9247,14 @@ static void ppc_cpu_realizefn(DeviceState *dev, Error **errp)
 #if !defined(CONFIG_USER_ONLY)
     cpu->cpu_dt_id = (cs->cpu_index / smp_threads) * max_smt
         + (cs->cpu_index % smp_threads);
+
+    if (kvm_enabled() && !kvm_vcpu_id_is_valid(cpu->cpu_dt_id)) {
+        error_setg(errp, "Can't create CPU with id %d in KVM", cpu->cpu_dt_id);
+        error_append_hint(errp, "Adjust the number of cpus to %d "
+                          "or try to raise the number of threads per core\n",
+                          cpu->cpu_dt_id * smp_threads / max_smt);
+        return;
+    }
 #endif
 
     if (tcg_enabled()) {

Re: [Qemu-devel] [PATCH] PPC/KVM: early validation of vcpu id

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 3248 bytes --]

On Tue, Apr 26, 2016 at 03:41:04PM +0200, Greg Kurz wrote:
> The KVM API restricts vcpu ids to be < KVM_CAP_MAX_VCPUS. On PowerPC
> targets, depending on the number of threads per core in the host and
> in the guest, some topologies do generate higher vcpu ids actually.
> When this happens, QEMU bails out with the following error:
> 
> kvm_init_vcpu failed: Invalid argument
> 
> The KVM_CREATE_VCPU ioctl has several EINVAL return paths, so it is
> not possible to fully disambiguate.
> 
> This patch adds a check in the code that computes vcpu ids, so that
> we can detect the error earlier, and print a friendlier message instead
> of calling KVM_CREATE_VCPU with an obviously bogus vcpu id.
> 
> Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com>

Applied to ppc-for-2.7, thanks.

I'm still kicking myself for doing the SMT / cpu ids that way way back
when.  Should have just had a "SET_SMT" ioctl() and allocated the cpu
ids sequentially.  Too clever by half :(

> ---
>  include/sysemu/kvm.h        |    2 ++
>  kvm-all.c                   |    6 ++++++
>  target-ppc/translate_init.c |    8 ++++++++
>  3 files changed, 16 insertions(+)
> 
> diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h
> index 0e18f15c9493..27bf50ef379e 100644
> --- a/include/sysemu/kvm.h
> +++ b/include/sysemu/kvm.h
> @@ -344,6 +344,8 @@ int kvm_arch_init(MachineState *ms, KVMState *s);
>  
>  int kvm_arch_init_vcpu(CPUState *cpu);
>  
> +bool kvm_vcpu_id_is_valid(int vcpu_id);
> +
>  /* Returns VCPU ID to be used on KVM_CREATE_VCPU ioctl() */
>  unsigned long kvm_arch_vcpu_id(CPUState *cpu);
>  
> diff --git a/kvm-all.c b/kvm-all.c
> index e7b66df197fd..3625a3e07457 100644
> --- a/kvm-all.c
> +++ b/kvm-all.c
> @@ -1459,6 +1459,12 @@ static int kvm_max_vcpus(KVMState *s)
>      return (ret) ? ret : kvm_recommended_vcpus(s);
>  }
>  
> +bool kvm_vcpu_id_is_valid(int vcpu_id)
> +{
> +    KVMState *s = KVM_STATE(current_machine->accelerator);
> +    return vcpu_id >= 0 && vcpu_id < kvm_max_vcpus(s);
> +}
> +
>  static int kvm_init(MachineState *ms)
>  {
>      MachineClass *mc = MACHINE_GET_CLASS(ms);
> diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
> index f51572552bc2..6c89b18a05f9 100644
> --- a/target-ppc/translate_init.c
> +++ b/target-ppc/translate_init.c
> @@ -9247,6 +9247,14 @@ static void ppc_cpu_realizefn(DeviceState *dev, Error **errp)
>  #if !defined(CONFIG_USER_ONLY)
>      cpu->cpu_dt_id = (cs->cpu_index / smp_threads) * max_smt
>          + (cs->cpu_index % smp_threads);
> +
> +    if (kvm_enabled() && !kvm_vcpu_id_is_valid(cpu->cpu_dt_id)) {
> +        error_setg(errp, "Can't create CPU with id %d in KVM", cpu->cpu_dt_id);
> +        error_append_hint(errp, "Adjust the number of cpus to %d "
> +                          "or try to raise the number of threads per core\n",
> +                          cpu->cpu_dt_id * smp_threads / max_smt);
> +        return;
> +    }
>  #endif
>  
>      if (tcg_enabled()) {
> 

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]