Re: [Qemu-devel] [PATCH v3 2/7] ppc: move smp_threads sanity checks to spapr

[David Gibson]

On Wed, Jul 06, 2016 at 02:13:30PM +0200, Greg Kurz wrote:
> Only POWER5 and newer PowerPC cpus from IBM have SMT capabilities.
> Since they are only supported by pseries, let's move the checks to
> ppc_spapr_init().
> 
> Signed-off-by: Greg Kurz <address@hidden>

I'm not comfortable with this.  SMT may only be used for pseries at
the moment, but the SMT possibilities are absolutely a constraint of
the CPU itself, not the machine type.  It seems more logical to me to
check this in the CPU code.

> ---
>  hw/ppc/spapr.c              |   12 ++++++++++++
>  target-ppc/translate_init.c |   14 --------------
>  2 files changed, 12 insertions(+), 14 deletions(-)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index d134eb2f338e..09600fee19b2 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -1739,6 +1739,18 @@ static void ppc_spapr_init(MachineState *machine)
>          }
>      }
>  
> +    if (smp_threads > smt) {
> +        error_report("Cannot support more than %d threads on PPC with %s",
> +                     smt, kvm_enabled() ? "KVM" : "TCG");
> +        exit(1);
> +    }
> +    if (!is_power_of_2(smp_threads)) {
> +        error_report("Cannot support %d threads on PPC with %s, "
> +                     "threads count must be a power of 2.",
> +                     smp_threads, kvm_enabled() ? "KVM" : "TCG");
> +        exit(1);
> +    }
> +
>      msi_nonbroken = true;
>  
>      QLIST_INIT(&spapr->phbs);
> diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
> index 31120a5aaf33..775df72cf6c2 100644
> --- a/target-ppc/translate_init.c
> +++ b/target-ppc/translate_init.c
> @@ -9531,20 +9531,6 @@ static void ppc_cpu_realizefn(DeviceState *dev, Error 
> **errp)
>      int max_smt = kvmppc_smt_threads();
>  #endif
>  
> -#if !defined(CONFIG_USER_ONLY)
> -    if (smp_threads > max_smt) {
> -        error_setg(errp, "Cannot support more than %d threads on PPC with 
> %s",
> -                   max_smt, kvm_enabled() ? "KVM" : "TCG");
> -        return;
> -    }
> -    if (!is_power_of_2(smp_threads)) {
> -        error_setg(errp, "Cannot support %d threads on PPC with %s, "
> -                   "threads count must be a power of 2.",
> -                   smp_threads, kvm_enabled() ? "KVM" : "TCG");
> -        return;
> -    }
> -#endif
> -
>      cpu_exec_init(cs, &local_err);
>      if (local_err != NULL) {
>          error_propagate(errp, local_err);
> 

-- 
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

signature.asc
Description: PGP signature