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

[Greg Kurz]

On Thu, 7 Jul 2016 11:12:42 +1000
David Gibson <address@hidden> wrote:

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

Yes, you're right. Ans since this isn't a prerequisite to compute cpu_dt_id,
it can stay in ppc_cpu_realizefn().

I'll drop this patch in v4.

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

pgpRvHISobtlh.pgp
Description: OpenPGP digital signature