From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:45736) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aaHyn-0005OS-IG for qemu-devel@nongnu.org; Mon, 29 Feb 2016 02:10:54 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aaHyj-0007ce-CN for qemu-devel@nongnu.org; Mon, 29 Feb 2016 02:10:53 -0500 Date: Mon, 29 Feb 2016 18:11:39 +1100 From: David Gibson Message-ID: <20160229071139.GH5427@voom.redhat.com> References: <1456417362-20652-1-git-send-email-bharata@linux.vnet.ibm.com> <1456417362-20652-4-git-send-email-bharata@linux.vnet.ibm.com> <20160226161857.2c6b7a3a@nial.brq.redhat.com> <20160229053531.GB5756@in.ibm.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="kHRd/tpU31Zn62xO" Content-Disposition: inline In-Reply-To: <20160229053531.GB5756@in.ibm.com> Subject: Re: [Qemu-devel] [RFC PATCH v0 3/6] spapr: Represent boot CPUs as spapr-cpu-core devices List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Bharata B Rao Cc: mjrosato@linux.vnet.ibm.com, agraf@suse.de, thuth@redhat.com, pkrempa@redhat.com, ehabkost@redhat.com, aik@ozlabs.ru, qemu-devel@nongnu.org, armbru@redhat.com, borntraeger@de.ibm.com, qemu-ppc@nongnu.org, pbonzini@redhat.com, Igor Mammedov , afaerber@suse.de, mdroth@linux.vnet.ibm.com --kHRd/tpU31Zn62xO Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Mon, Feb 29, 2016 at 11:05:32AM +0530, Bharata B Rao wrote: > On Fri, Feb 26, 2016 at 04:18:57PM +0100, Igor Mammedov wrote: > > On Thu, 25 Feb 2016 21:52:39 +0530 > > Bharata B Rao wrote: [snip] > > > @@ -2209,6 +2251,7 @@ static void spapr_machine_device_plug(HotplugHa= ndler *hotplug_dev, > > > DeviceState *dev, Error **errp) > > > { > > > sPAPRMachineClass *smc =3D SPAPR_MACHINE_GET_CLASS(qdev_get_mach= ine()); > > > + sPAPRMachineState *ms =3D SPAPR_MACHINE(hotplug_dev); > > > =20 > > > if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) { > > > int node; > > > @@ -2245,6 +2288,11 @@ static void spapr_machine_device_plug(HotplugH= andler *hotplug_dev, > > > } > > > =20 > > > spapr_memory_plug(hotplug_dev, dev, node, errp); > > > + } else if (object_dynamic_cast(OBJECT(dev), TYPE_CPU)) { > > here probably should be CORE and not TYPE_CPU, > > then if board needs to set some state for child threads it > > could ennumerate child of core here and do the required job. >=20 > Hmm, we have things to set for CPUs and cores as well from hotplug > handler. So the above code is for CPUs and I have spapr_core_plug() > which is called conditionally when device is of type TYPE_SPAPR_CPU_CORE. >=20 > Given that ->plug() is called during realization of both individual CPU > threads as well as their parent core, I thought handling the setup for > them separately like the above is simpler, no ? So, this bothered me a bit as well. I think having the hotplug handler for the threads is misleading, since it suggests you can individually hotplug them, when in fact the intention is just that this is part of the hotplug code path for a whole core. My suggestion earlier was to have the core realize code itself perform this configuration on the sub-threads. However, Igor's suggestion of, essentially, an explicit loop over the threads in the core hotplug handler would be fine by me also. --=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 --kHRd/tpU31Zn62xO Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJW0+8rAAoJEGw4ysog2bOSDYMP/ReNAMF/5r1Y91hlwAAkKa/D Y5FpYEVs3jMB8fEguOhZNMFB6xqZXSE6MvBQb65VqvEgGb1bafXqYGaW/rWEs4oJ cg/UsojUQsWRj/Yam1bwjh2mP5j/0oKvicm7nmgXFg2kma0bidG1/EVDjtLTcJoD e2KmNsVl5c4T/+pXqxpzgWkXIJk/+lvS1evlihD7KBQLJKh9OifCvQMHe6HfQs9R TmmBzr1finos8j31TgwGtExiy3AcF/SLbKp5cyp16/lVo2RAwdwiVLXDl8yj+cSl ULnUXHencuMhVCXAVBCViZG+r1+iZAzlzqWsgfUjRFDaL/1UyZy/SNC9iHR2+Waq LRg0WsANXGshq+QfvXfzwENUEqr8KHjD/CcaZUwX5a1tFpou327zll6v2IBJdCly Da/MXb+J+8Ut/T6ME09pCyc1H8wOkXjVJQguv/uzAyaJ7WXIKtutie9GDLebXn7s IyICCOJFf+izrb2l/Gav0YJ1D8y9VpzzQPdp8KjSr9stTXLOd/di7mwEBcwTwZeh mc8D1/XyyYengfWAEyhmWnypN1G2Y80gKZwOiGV+AeB3bsKGHfX911BhWKcrpWAF XIzk/KrzqJkrdHYBfwEXX5olMwfnx/J7kVCiLxHSt57lQNZWaWoJ7edrhDTcR6YZ wTpSc1vSA80ghaMcXBpz =OQIf -----END PGP SIGNATURE----- --kHRd/tpU31Zn62xO--