From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:54054) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aYsAU-0006qq-33 for qemu-devel@nongnu.org; Thu, 25 Feb 2016 04:25:07 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aYsAQ-0007vn-LO for qemu-devel@nongnu.org; Thu, 25 Feb 2016 04:25:06 -0500 Date: Thu, 25 Feb 2016 17:41:14 +1100 From: David Gibson Message-ID: <20160225064114.GH22216@voom.redhat.com> References: <20160223052431.GS2808@voom.fritz.box> <20160223094026.GA21081@in.ibm.com> <20160223100504.GW2808@voom.fritz.box> <20160223121859.5e93bc68@nial.brq.redhat.com> <20160224020106.GH2808@voom.fritz.box> <20160224114833.21b177d3@nial.brq.redhat.com> <20160224112822.GP2808@voom.fritz.box> <20160224144117.52c27338@nial.brq.redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="LZFKeWUZP29EKQNE" Content-Disposition: inline In-Reply-To: <20160224144117.52c27338@nial.brq.redhat.com> Subject: Re: [Qemu-devel] CPU hotplug, again List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Igor Mammedov Cc: agraf@suse.de, thuth@redhat.com, ehabkost@redhat.com, qemu-devel@nongnu.org, armbru@redhat.com, qemu-ppc@nongnu.org, Bharata B Rao , pbonzini@redhat.com, Andreas =?iso-8859-1?Q?F=E4rber?= --LZFKeWUZP29EKQNE Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Wed, Feb 24, 2016 at 02:41:17PM +0100, Igor Mammedov wrote: > On Wed, 24 Feb 2016 22:28:22 +1100 > David Gibson wrote: >=20 > > On Wed, Feb 24, 2016 at 11:48:33AM +0100, Igor Mammedov wrote: > > > On Wed, 24 Feb 2016 13:01:06 +1100 > > > David Gibson wrote: > > > =20 > > > > On Tue, Feb 23, 2016 at 12:18:59PM +0100, Igor Mammedov wrote: =20 > > > > > On Tue, 23 Feb 2016 21:05:04 +1100 > > > > > David Gibson wrote: > > > > > =20 > > > > > > On Tue, Feb 23, 2016 at 03:10:26PM +0530, Bharata B Rao wrote: = =20 > > > > > > > On Tue, Feb 23, 2016 at 04:24:31PM +1100, David Gibson wrote:= =20 > > > > > > > > Hi Andreas, > > > > > > > >=20 > > > > > > > > I've now found (with Thomas' help) your RFC series for sock= et/core > > > > > > > > based cpu hotplug on x86 > > > > > > > > (https://github.com/afaerber/qemu-cpu/compare/qom-cpu-x86).= It seems > > > > > > > > sensible enough as far as it goes, but doesn't seem to addr= ess a bunch > > > > > > > > of the things that I was attempting to do with the cpu-pack= age > > > > > > > > proposal - and which we absolutely need for cpu hotplug on = Power. > > > > > > > >=20 > > > > > > > > 1) What interface do you envisage beyond cpu_add? > > > > > > > >=20 > > > > > > > > The patches I see just construct extra socket and core obje= cts, but > > > > > > > > still control hotplug (for x86) through the cpu_add interfa= ce. That > > > > > > > > interface is absolutely unusable on Power, since it operate= s on a > > > > > > > > per-thread basis, whereas the PAPR guest<->host interfaces = can only > > > > > > > > communicate information at a per-core granularity. > > > > > > > >=20 > > > > > > > > 2) When hotplugging at core or socket granularity, where wo= uld the > > > > > > > > code to construct the individual thread objects sit? > > > > > > > >=20 > > > > > > > > Your series has the construction done in both the machine i= nit path > > > > > > > > and the hotplug path. The latter works because hotplug occ= urs at > > > > > > > > thread granularity. If we're hotplugging at core or socket > > > > > > > > granularity what would do the construct? The core/socket o= bject > > > > > > > > itself (in instance_init? in realize?); the hotplug handle= r? > > > > > > > > something else? > > > > > > > >=20 > > > > > > > > 3) How does the management layer determine what is pluggabl= e? > > > > > > > >=20 > > > > > > > > Both the number of pluggable slots, and what it will need t= o do to > > > > > > > > populate them. > > > > > > > >=20 > > > > > > > > 4) How do we enforce that toplogies illegal for the platfor= m can't be > > > > > > > > constructed? =20 > > > > > > >=20 > > > > > > > 5) QOM-links > > > > > > >=20 > > > > > > > Andreas, You have often talked about setting up links from ma= chine object > > > > > > > to the CPU objects. Would the below code correctly capture th= at idea of > > > > > > > yours ? > > > > > > >=20 > > > > > > > #define SPAPR_MACHINE_CPU_CORE_PROP "core" > > > > > > >=20 > > > > > > > /* MachineClass.init for sPAPR */ > > > > > > > static void ppc_spapr_init(MachineState *machine) > > > > > > > { > > > > > > > sPAPRMachineState *spapr =3D SPAPR_MACHINE(machine); > > > > > > > int spapr_smp_cores =3D smp_cpus / smp_threads; > > > > > > > int spapr_max_cores =3D max_cpus / smp_threads; > > > > > > >=20 > > > > > > > ... > > > > > > > for (i =3D 0; i < spapr_max_cores; i++) { > > > > > > > Object *obj =3D object_new(TYPE_SPAPR_CPU_CORE); > > > > > > > sPAPRCPUCore *core =3D SPAPR_CPU_CORE(obj); > > > > > > > char name[32]; > > > > > > >=20 > > > > > > > snprintf(name, sizeof(name), "%s[%d]", SPAPR_MACHINE_= CPU_CORE_PROP, i); > > > > > > >=20 > > > > > > > /* > > > > > > > * Create links from machine objects to all possible = cores. > > > > > > > */ > > > > > > > object_property_add_link(OBJECT(spapr), name, TYPE_SP= APR_CPU_CORE, > > > > > > > (Object **)&spapr->core[i], > > > > > > > NULL, NULL, &error_abort);= =20 > > > > > > >=20 > > > > > > > /* > > > > > > > * Set the QOM link from machine object to core objec= t for all > > > > > > > * boot time CPUs specified with -smp. For rest of th= e hotpluggable > > > > > > > * cores this is done from the core hotplug path. > > > > > > > */ > > > > > > > if (i < spapr_smp_cores) { > > > > > > > object_property_set_link(OBJECT(spapr), OBJECT(co= re), > > > > > > > SPAPR_MACHINE_CPU_CORE_P= ROP, &error_abort); =20 > > > > > >=20 > > > > > > I hope we can at least have a helper function to both construct= the > > > > > > core and create the links, if we can't handle the link creation= in the > > > > > > core object itself. > > > > > >=20 > > > > > > Having to open-code it in each machine sounds like a recipe for= subtle > > > > > > differences in presentation between platforms, which is exactly= what > > > > > > we want to avoid. =20 > > > > > Creating links doesn't give us much, it's just adds means for mgmt > > > > > to check how many CPUs could be hotplugged without keeping that > > > > > state in mgmt like it's now, so links are mostly useless if one > > > > > care where CPU is being plugged in. > > > > > The rest like enumerating exiting CPUs could be done by > > > > > traversing QOM tree, links would just simplify finding > > > > > CPUs putting them at fixed namespace. =20 > > > >=20 > > > > Simplifying finding CPUs is pretty much all we intended the links f= or. =20 > > > Do mgmt really needs it? For machine it's easy to find CPUs under > > > /machine/[peripheral|unattached] by enumerating entries over there. > > > For human, one would need to implement a dedicated HMP command that > > > would do the same, so it doesn't really matter where links are > > > located. =20 > >=20 > > If we require management to go searching the whole device tree for > > cpus, I'm concerned they'll just assume they're in the x86 location > > instead, and we'll have to fix it over and over for every platform > > that puts them somewhere different. > CPUs are inherited from Device so inherited behaviour is that they > are pretty much at fixed location /machine/[peripheral|unattached] > regardless of platform QOM tree wise, like every other device. Hmm.. that's true now, but I can see reasons you might want to put CPUs on a different bus in future. In particular consider a machine type modelling real hardware for a modern multisocket machine - these are often built from several chips on a common fabric, each containing several CPU cores, but also other peripherals and bus bridges. --=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 --LZFKeWUZP29EKQNE Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJWzqIKAAoJEGw4ysog2bOSktsP/2pXoBkvzZFPJZ+gz0ZDGVj2 a9fR9zJ+p7qPpiITfK7saaPwvhorsLs7OwmSAtCJzLUE0KCbIuF8LmgfB93BYAeG 4ustS1Jt8ik6ho5+XyJiKpx4jkk4+o953CuuZo9NnR4UXBH5hldicJ3xCf1yP4xI oZKYfp/hbnR3zNAwPon44dMKvLPbOIYj2YDtq+zdrxf18SjOmRK3COEkBIlA+HUn m5autaG3GuOd4s2jMm+ojCxeJY9PUNxnF0ElhReqWv53QEeQ9SrMavkFOchLjKz/ q+zP8brb/z8kfgNsLbhxJxWKNGDURkF4TVrZB/pnQOMI49gD8nAScyKc2L05XSqY 0NwXrbEc10rbhkz98wCPee7QYM7DVDqlCAf2O8jxS4O0sMbkbeUv1uCpFAM7tIY9 VNem/a8Z7A4CXPxMqMMmQzb+ZphrOb88N5K/1eW+MPh3h2Y0Pz7WWAwakg2sba3C +3dHz0xLc2Sz7kjA80Q2muzl0KuQeDzaofQP6D2GOTTK3FL4dNtO9U5nizZp/L1T qRmw738pmLuBWovo4wEyb18B3zWS3J0ZjlSmhnqPu5H/r1T7wvxN0uaEfse0U3vX pEUh07k9vb6hB9p08zFpbVS/h7l1r7FxWmha5SgQRmv2Pd6onqw42nAr/6TpS5vA rxVdAxxld7N3kmLBKOrJ =Qd8y -----END PGP SIGNATURE----- --LZFKeWUZP29EKQNE--