On Fri, 1 Jul 2016 10:44:39 +0530 Bharata B Rao <bhar...@linux.vnet.ibm.com> wrote:
> During CPU core realization, we create all the thread objects and parent > them to the core object in a loop. However, the realization of thread > objects is done separately by walking the threads of a core using > object_child_foreach(). With this, there is no guarantee on the order > in which the child thread objects get realized. Since CPU device tree > properties are currently derived from the CPU thread object, we assume > thread0 of the core to be the representative thread of the core when > creating device tree properties for the core. If thread0 is not the > first thread that gets realized, then we would end up having an > incorrect dt_id for the core and this causes hotplug failures from > the guest. > > Fix this by realizing each thread object by walking the core's thread > object list thereby ensuring that thread0 and other threads are always > realized in the correct order. > > Future TODO: CPU DT nodes are per-core properties and we should > ideally base the creation of CPU DT nodes on core objects rather than > the thread objects. > > Signed-off-by: Bharata B Rao <bhar...@linux.vnet.ibm.com> > --- This will also help to compute a new index to be used in place of cs->cpu_index in my series to move cpu_dt_id generation from target to machine code. Reviewed-by: Greg Kurz <gr...@kaod.org> > hw/ppc/spapr_cpu_core.c | 29 ++++++++++++++++------------- > 1 file changed, 16 insertions(+), 13 deletions(-) > > diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c > index a384db5..70b6b0b 100644 > --- a/hw/ppc/spapr_cpu_core.c > +++ b/hw/ppc/spapr_cpu_core.c > @@ -259,9 +259,9 @@ out: > error_propagate(errp, local_err); > } > > -static int spapr_cpu_core_realize_child(Object *child, void *opaque) > +static void spapr_cpu_core_realize_child(Object *child, Error **errp) > { > - Error **errp = opaque, *local_err = NULL; > + Error *local_err = NULL; > sPAPRMachineState *spapr = SPAPR_MACHINE(qdev_get_machine()); > CPUState *cs = CPU(child); > PowerPCCPU *cpu = POWERPC_CPU(cs); > @@ -269,15 +269,14 @@ static int spapr_cpu_core_realize_child(Object *child, > void *opaque) > object_property_set_bool(child, true, "realized", &local_err); > if (local_err) { > error_propagate(errp, local_err); > - return 1; > + return; > } > > spapr_cpu_init(spapr, cpu, &local_err); > if (local_err) { > error_propagate(errp, local_err); > - return 1; > + return; > } > - return 0; > } > > static void spapr_cpu_core_realize(DeviceState *dev, Error **errp) > @@ -287,13 +286,13 @@ static void spapr_cpu_core_realize(DeviceState *dev, > Error **errp) > const char *typename = object_class_get_name(sc->cpu_class); > size_t size = object_type_get_instance_size(typename); > Error *local_err = NULL; > - Object *obj; > - int i; > + void *obj; > + int i, j; > > sc->threads = g_malloc0(size * cc->nr_threads); > for (i = 0; i < cc->nr_threads; i++) { > char id[32]; > - void *obj = sc->threads + i * size; > + obj = sc->threads + i * size; > > object_initialize(obj, size, typename); > snprintf(id, sizeof(id), "thread[%d]", i); > @@ -303,12 +302,16 @@ static void spapr_cpu_core_realize(DeviceState *dev, > Error **errp) > } > object_unref(obj); > } > - object_child_foreach(OBJECT(dev), spapr_cpu_core_realize_child, > &local_err); > - if (local_err) { > - goto err; > - } else { > - return; > + > + for (j = 0; j < cc->nr_threads; j++) { > + obj = sc->threads + j * size; > + > + spapr_cpu_core_realize_child(obj, &local_err); > + if (local_err) { > + goto err; > + } > } > + return; > > err: > while (--i >= 0) {