From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:38598) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aYXbQ-0005UD-2e for qemu-devel@nongnu.org; Wed, 24 Feb 2016 06:27:33 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aYXbM-0005ma-1o for qemu-devel@nongnu.org; Wed, 24 Feb 2016 06:27:31 -0500 Date: Wed, 24 Feb 2016 22:28:22 +1100 From: David Gibson Message-ID: <20160224112822.GP2808@voom.fritz.box> 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> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="DCmRZ841GpRfzaEr" Content-Disposition: inline In-Reply-To: <20160224114833.21b177d3@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?= --DCmRZ841GpRfzaEr Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable 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: > > > 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 socket/c= ore > > > > > > 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 address = a bunch > > > > > > of the things that I was attempting to do with the cpu-package > > > > > > proposal - and which we absolutely need for cpu hotplug on Powe= r. > > > > > >=20 > > > > > > 1) What interface do you envisage beyond cpu_add? > > > > > >=20 > > > > > > The patches I see just construct extra socket and core objects,= but > > > > > > still control hotplug (for x86) through the cpu_add interface. = That > > > > > > interface is absolutely unusable on Power, since it operates 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 would = the > > > > > > code to construct the individual thread objects sit? > > > > > >=20 > > > > > > Your series has the construction done in both the machine init = path > > > > > > and the hotplug path. The latter works because hotplug occurs = at > > > > > > thread granularity. If we're hotplugging at core or socket > > > > > > granularity what would do the construct? The core/socket object > > > > > > itself (in instance_init? in realize?); the hotplug handler? > > > > > > something else? > > > > > >=20 > > > > > > 3) How does the management layer determine what is pluggable? > > > > > >=20 > > > > > > Both the number of pluggable slots, and what it will need to do= to > > > > > > populate them. > > > > > >=20 > > > > > > 4) How do we enforce that toplogies illegal for the platform ca= n't be > > > > > > constructed? =20 > > > > >=20 > > > > > 5) QOM-links > > > > >=20 > > > > > Andreas, You have often talked about setting up links from machin= e object > > > > > to the CPU objects. Would the below code correctly capture that i= dea 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 core= s. > > > > > */ > > > > > object_property_add_link(OBJECT(spapr), name, TYPE_SPAPR_= CPU_CORE, > > > > > (Object **)&spapr->core[i], > > > > > NULL, NULL, &error_abort);=20 > > > > >=20 > > > > > /* > > > > > * Set the QOM link from machine object to core object fo= r all > > > > > * boot time CPUs specified with -smp. For rest of the ho= tpluggable > > > > > * cores this is done from the core hotplug path. > > > > > */ > > > > > if (i < spapr_smp_cores) { > > > > > object_property_set_link(OBJECT(spapr), OBJECT(core), > > > > > SPAPR_MACHINE_CPU_CORE_PROP,= &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 sub= tle > > > > 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 for. > 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. 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. > > Well, and then I was expecting a different set of links to simplify > > enumerating all the threads in a cpu package/core/socket/whatever. > >=20 >=20 --=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 --DCmRZ841GpRfzaEr Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJWzZPWAAoJEGw4ysog2bOSv/UQAMCdXuPIKVXtHfbIunScqAcT CHy4t70P7s2a45puUsayv4mKNnuQKRQYvuMXM89YIOnq7z4XvvO0KFLuZYpqaHBF RzGBFHxKT0QIQ/rdNoUAfbqNyd39cwxaq+FBriVL6eEFbMFwF3bshc+Ds5RkubFT qB1q08Lj9cpR49plr9NFCUTaJrCOxUaQLF96+PQXbRiNiAWKN2NQswvg5E70aD+o bQxG6q1c9RIOwZtPCl82OYWESGb17MC+tWEaqHUdKRBvqbeekKJqZ44sLokbV17i 8zPVWIiT2kljcI2HDIjnrCbt7y83prKbswmWcsvkSyv9L0IsFmBDKY57sZFcSO36 Tdqr3edRqIvqN0q39yAO0fOcCz3M46sfVpmAE1T3yqiAPeGbjVW9CYPt6iN7cYPA P2rAJma5wgCzYYNoZoAQbIpswzfEhp+7aW1pgOdyY/YFPzQYRPPhCEEYUlBDc3/U G8jwjh1pBBThO2tW0oVhRVMmmEZY5KjakKxLn7+xh8FI36edyc8zJg5YWc588noo xis+nP6N6FXrkSeDF8XUJCyY9DXDfiDN7re6lP+QthWOSEIq0lvsOSFRm2NU4Hns VSx5ZP7koF6qenukL6zbp8VgQ8U+r/nMs0x44mq2u0Wk4nSoJxuTyNV9VCTB0K38 /0fm3RQdPsPieeKhdgLj =AR5K -----END PGP SIGNATURE----- --DCmRZ841GpRfzaEr--