On Fri, Mar 31, 2017 at 11:04:55AM +0530, Bharata B Rao wrote: > On Fri, Mar 31, 2017 at 10:21 AM, David Gibson <da...@gibson.dropbear.id.au> > wrote: > > > For reasons that may be useful in future, CPU core objects, as used on the > > pseries machine type have their own nr-threads property, potentially > > allowing cores with different numbers of threads in the same system. > > > > I remember we retained this flexibility to support heterogenous systems in > future ? I think we can go with this enforcement now and relax it later if > we ever reach there. > > > > > > If the user/management uses the values specified in query-hotpluggable-cpus > > as they're expected to do, this will never matter in pratice. But that's > > not actually enforced - it's possible to manually specify a core with > > a different number of threads from that in -smp. That will confuse the > > platform - most immediately, this can be used to create a CPU thread with > > index above max_cpus which leads to an assertion failure in > > spapr_cpu_core_realize(). > > > > For now, enforce that all cores must have the same, standard, number of > > threads. While we're at it, also enforce that the core ids are correctly > > aligned based on that number of threads. > > > > Signed-off-by: David Gibson <da...@gibson.dropbear.id.au> > > --- > > hw/ppc/spapr_cpu_core.c | 12 ++++++++++++ > > 1 file changed, 12 insertions(+) > > > > diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c > > index 6883f09..935ee62 100644 > > --- a/hw/ppc/spapr_cpu_core.c > > +++ b/hw/ppc/spapr_cpu_core.c > > @@ -167,6 +167,18 @@ static void spapr_cpu_core_realize(DeviceState *dev, > > Error **errp) > > void *obj; > > int i, j; > > > > + if (cc->nr_threads != smp_threads) { > > + error_setg(errp, > > + "Invalid nr-threads=%d of CPU[core-id: %d], must be > > %d", > > + cc->nr_threads, cc->core_id, smp_threads); > > + return; > > + } > > > > The above check should move to pre_plug handler.
Ah, good point. > > + if ((cc->core_id % smp_threads) != 0) { > > + error_setg(errp, "Invalid CPU core-id=%d, must be a multiple of > > %d", > > + cc->core_id, smp_threads); > > + return; > > + } > > > > Not sure when you will hit this as the same check is already present in > pre_plug handler. Oops, yes, didn't spot that. -- 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