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

[David Gibson <david@gibson.dropbear.id.au>]

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

On Thu, May 26, 2016 at 10:02:10AM +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>

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

> ---
>  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 f9f00e2e56cb..f357ccde9122 100644
> --- a/include/sysemu/kvm.h
> +++ b/include/sysemu/kvm.h
> @@ -345,6 +345,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 f9ae8f9bf809..e56f38527815 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 954195f5e494..a003c1029d31 100644
> --- a/target-ppc/translate_init.c
> +++ b/target-ppc/translate_init.c
> @@ -9231,6 +9231,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 --]