| Message ID | 146780720136.26232.7902445403479806617.stgit@bahia.lab.toulouse-stg.fr.ibm.com |
|---|---|
| State | New |
| Headers | show |
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 <groug@kaod.org> 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); >
On Thu, 2016-07-07 at 11:12 +1000, David Gibson wrote: > 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. Also powernv uses them too no ? Ben.
On Thu, Jul 07, 2016 at 11:57:32AM +1000, Benjamin Herrenschmidt wrote: > On Thu, 2016-07-07 at 11:12 +1000, David Gibson wrote: > > 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. > > Also powernv uses them too no ? Well, powernv isn't in yet, but yes it will.
On Thu, 7 Jul 2016 11:12:42 +1000 David Gibson <david@gibson.dropbear.id.au> 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 <groug@kaod.org> > > 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); > > >
On Thu, 07 Jul 2016 11:57:32 +1000 Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote: > On Thu, 2016-07-07 at 11:12 +1000, David Gibson wrote: > > 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. > > Also powernv uses them too no ? > > Ben. > Hi Ben ! I've started to play with Cedric's tree on github. I'll work on adapting powernv to this series. Cheers. -- Greg
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);
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 <groug@kaod.org> --- hw/ppc/spapr.c | 12 ++++++++++++ target-ppc/translate_init.c | 14 -------------- 2 files changed, 12 insertions(+), 14 deletions(-)