From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:43484) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bJtNL-00085d-G4 for qemu-devel@nongnu.org; Sun, 03 Jul 2016 22:12:44 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bJtNI-0008U5-7p for qemu-devel@nongnu.org; Sun, 03 Jul 2016 22:12:43 -0400 Date: Mon, 4 Jul 2016 12:14:01 +1000 From: David Gibson Message-ID: <20160704021401.GB2919@voom.fritz.box> References: <1467350079-2597-1-git-send-email-bharata@linux.vnet.ibm.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="tsOsTdHNUZQcU9Ye" Content-Disposition: inline In-Reply-To: <1467350079-2597-1-git-send-email-bharata@linux.vnet.ibm.com> Subject: Re: [Qemu-devel] [PATCH v0] spapr: Ensure thread0 of CPU core is always realized first List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Bharata B Rao Cc: qemu-devel@nongnu.org, qemu-ppc@nongnu.org --tsOsTdHNUZQcU9Ye Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Fri, Jul 01, 2016 at 10:44:39AM +0530, Bharata B Rao 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. >=20 > 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. >=20 > 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. >=20 > Signed-off-by: Bharata B Rao Applied to ppc-for-2.7, thanks. > --- > hw/ppc/spapr_cpu_core.c | 29 ++++++++++++++++------------- > 1 file changed, 16 insertions(+), 13 deletions(-) >=20 > 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); > } > =20 > -static int spapr_cpu_core_realize_child(Object *child, void *opaque) > +static void spapr_cpu_core_realize_child(Object *child, Error **errp) > { > - Error **errp =3D opaque, *local_err =3D NULL; > + Error *local_err =3D NULL; > sPAPRMachineState *spapr =3D SPAPR_MACHINE(qdev_get_machine()); > CPUState *cs =3D CPU(child); > PowerPCCPU *cpu =3D POWERPC_CPU(cs); > @@ -269,15 +269,14 @@ static int spapr_cpu_core_realize_child(Object *chi= ld, void *opaque) > object_property_set_bool(child, true, "realized", &local_err); > if (local_err) { > error_propagate(errp, local_err); > - return 1; > + return; > } > =20 > spapr_cpu_init(spapr, cpu, &local_err); > if (local_err) { > error_propagate(errp, local_err); > - return 1; > + return; > } > - return 0; > } > =20 > 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 =3D object_class_get_name(sc->cpu_class); > size_t size =3D object_type_get_instance_size(typename); > Error *local_err =3D NULL; > - Object *obj; > - int i; > + void *obj; > + int i, j; > =20 > sc->threads =3D g_malloc0(size * cc->nr_threads); > for (i =3D 0; i < cc->nr_threads; i++) { > char id[32]; > - void *obj =3D sc->threads + i * size; > + obj =3D sc->threads + i * size; > =20 > 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, &loc= al_err); > - if (local_err) { > - goto err; > - } else { > - return; > + > + for (j =3D 0; j < cc->nr_threads; j++) { > + obj =3D sc->threads + j * size; > + > + spapr_cpu_core_realize_child(obj, &local_err); > + if (local_err) { > + goto err; > + } > } > + return; > =20 > err: > while (--i >=3D 0) { --=20 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 --tsOsTdHNUZQcU9Ye Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJXecZpAAoJEGw4ysog2bOSUx0QAMwZSrWshY3TfxicnQYzlMS3 SPKSsjcD2rsXSWNwvy5WiXp8seTX1EPa9N5MzZGu7utusLNJ104T+aOHlmZjoTI/ 8UKE0TQFXcUvicIsRAZqxdmG/mLJoJ2rPqW1Wn3h1cG1eJyq/ptwbDdbrOEj+3pE bPm1CBORyKDlwzuszjObZeLvozlOsjtv9QRCKTPPdg9wns008KIWSoRRbXI+siMh m/GC92uQ8Wf1y8wpd7iQHVQeUbYJ1XfeL6FpHS05Oa7JfS88fZBr7L7NR9ILoYYQ vjaUXmHrA+/Oul1l8to8M847KXjpqFzztDtJeAS4loAOWSH42sMwsf8cZVAT5+xW oBTDgX16+BA8Yd6Kv9Lo1oA8fpQ7JpPdamyLQBJ6a76M1I+GAp5ZFBJFiV4JPBPO cpdtefmFhgYyo00tKPdZYo8hNMnL7rGvyx7JDFR/LFtFM0y7mU5vOOnLCJuPCOLA JZYZvpWLs23vpfoLn7xPEic+sGKAe4VI9vrkOgZwZ8Szono3vzkDTrZ2qrKktve2 D5Czzc6SNEF9snYm0RvU297lZ35kcsyZI+ogO+DCsQro/TkTz9LA8Ds5zuO4ObV4 nKT0RSYcxVMjKC2o2wUdsv42ASAinWfO3PEvA3xjrFNoyTV/6EcRJSVk38fWgZ7b xV2WRUmGhKB2bV82aCjb =nJDB -----END PGP SIGNATURE----- --tsOsTdHNUZQcU9Ye--