From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:33984) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bLOP4-0002Yl-Vd for qemu-devel@nongnu.org; Fri, 08 Jul 2016 01:32:44 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bLOP2-0007S2-Dj for qemu-devel@nongnu.org; Fri, 08 Jul 2016 01:32:41 -0400 Date: Fri, 8 Jul 2016 15:24:13 +1000 From: David Gibson Message-ID: <20160708052413.GM14675@voom.fritz.box> References: <1467903025-13383-1-git-send-email-bharata@linux.vnet.ibm.com> <1467903025-13383-4-git-send-email-bharata@linux.vnet.ibm.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="KQSGybwJm5+gcNx6" Content-Disposition: inline In-Reply-To: <1467903025-13383-4-git-send-email-bharata@linux.vnet.ibm.com> Subject: Re: [Qemu-devel] [RFC PATCH v2 3/5] spapr: Set stable_cpu_id for threads of CPU cores 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, imammedo@redhat.com, groug@kaod.org, nikunj@linux.vnet.ibm.com, pbonzini@redhat.com --KQSGybwJm5+gcNx6 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Thu, Jul 07, 2016 at 08:20:23PM +0530, Bharata B Rao wrote: > Conditonally set stable_cpu_id for CPU threads that are created as part > of spapr CPU cores. The use of stable_cpu_id is enabled for pseries-2.7 > onwards. >=20 > Signed-off-by: Bharata B Rao > --- > hw/ppc/spapr_cpu_core.c | 7 +++++++ > 1 file changed, 7 insertions(+) >=20 > diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c > index b104778..0ec3513 100644 > --- a/hw/ppc/spapr_cpu_core.c > +++ b/hw/ppc/spapr_cpu_core.c > @@ -293,8 +293,15 @@ static void spapr_cpu_core_realize(DeviceState *dev,= Error **errp) > for (i =3D 0; i < cc->nr_threads; i++) { > char id[32]; > obj =3D sc->threads + i * size; > + CPUState *cs; > =20 > object_initialize(obj, size, typename); > + cs =3D CPU(obj); > + > + /* Use core_id (which is actually cpu_dt_id) as stable CPU id */ > + if (cs->has_stable_cpu_id) { > + cs->stable_cpu_id =3D cc->core_id + i; > + } Testing cs->has_stable_cpu_id here in machine type specific code seems really weird. It's the machine type that knows whether it has a stable ID to give to the CPU or not, rather than the other way around. Since we haven't yet had a release with cpu cores, I think the right thing is for cpu_core to unconditionally set the stable ID (and set has_stable_id to true). The backup path that does thread-based cpu init, can set has_stable_id to false (if that's not the default). > snprintf(id, sizeof(id), "thread[%d]", i); > object_property_add_child(OBJECT(sc), id, obj, &local_err); > if (local_err) { --=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 --KQSGybwJm5+gcNx6 Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJXfzj9AAoJEGw4ysog2bOSeUAP/ApxY1MdunkCrxUNUcd7OgTk dJAE8W0keWmF00/j+5xGzChBFwB4bSdsj7naXOheXOqCzm9r/qfSKc/Mf+nw225Z hX66Lxj5LnZaFasni/Ze9G9OfcbuFkL5XyN+ZyHyak2bw/qY//4Lesj5ks/3WvO8 V/JY1N3kJ1vBMXc1yUpFRQjWvBBf0qCBs4B6OweEtejZwOmhPoMyZWjmXbNc2tD+ zyRbAriuA/xonImVUPgHfTSxHZWWvHd32akODPN4ZAz9kfutbRnLory7K1jhTZZL OWSg1JsfKP/t+pOkGw0i7a5BhAGkkkIWG38WeAeJJf7A2ZQA7ACXlzsFgHvUNwpe n7WTG39vIbkcV/3dTIIgCvzw6elxYmLJEGM6lePT06L7mQ0DlZoc6EhT4QRoMte/ XSbkHsQMesHgz8HVA7Xo0mAJJpo8P+6IT2hmDXKeG/x/uvuAZw5e0cvDl2F7KiF/ HdgYcgEKA0ZDozRmB6jQcMxDO/ewwG66Xs65XV1h+F2vMYR1UAOyB9Xgy8rCGxCx GUNaZtBAl9N2bpuqKnxJNqLdsim8glOfAjIlL0eWjJN83TRnyBJLZ0RTe1wMJ+t1 Y3frnrd67bHs1kmCMzn6b/oHCP9aRG+Ph+ZagsL8epwIkWkXFljEL6x8YBtpipO5 rIQAKbUPBZUHvZ/xYvD7 =f5eq -----END PGP SIGNATURE----- --KQSGybwJm5+gcNx6--