From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:58418) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aiA50-0003b5-KI for qemu-devel@nongnu.org; Mon, 21 Mar 2016 20:21:51 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aiA4z-0000xB-7o for qemu-devel@nongnu.org; Mon, 21 Mar 2016 20:21:50 -0400 Date: Tue, 22 Mar 2016 11:22:56 +1100 From: David Gibson Message-ID: <20160322002256.GQ23586@voom.redhat.com> References: <1457672078-17307-1-git-send-email-bharata@linux.vnet.ibm.com> <20160314104728.5ff09f86@nial.brq.redhat.com> <20160316034803.GC13176@in.ibm.com> <20160316164850.2c3420db@nial.brq.redhat.com> <20160317100343.GT9032@voom> <20160318032932.GA20937@in.ibm.com> <20160321035753.GE23586@voom.redhat.com> <20160321114334.1d5031ee@nial.brq.redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="+hz2tM55CCA8Ej21" Content-Disposition: inline In-Reply-To: <20160321114334.1d5031ee@nial.brq.redhat.com> Subject: Re: [Qemu-devel] [RFC PATCH v2 0/9] Core based CPU hotplug for PowerPC sPAPR List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Igor Mammedov Cc: mjrosato@linux.vnet.ibm.com, thuth@redhat.com, pkrempa@redhat.com, ehabkost@redhat.com, aik@ozlabs.ru, armbru@redhat.com, agraf@suse.de, qemu-devel@nongnu.org, borntraeger@de.ibm.com, qemu-ppc@nongnu.org, Bharata B Rao , pbonzini@redhat.com, afaerber@suse.de, mdroth@linux.vnet.ibm.com --+hz2tM55CCA8Ej21 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Mon, Mar 21, 2016 at 11:43:34AM +0100, Igor Mammedov wrote: > On Mon, 21 Mar 2016 14:57:53 +1100 > David Gibson wrote: >=20 > > On Fri, Mar 18, 2016 at 08:59:32AM +0530, Bharata B Rao wrote: > > > On Thu, Mar 17, 2016 at 09:03:43PM +1100, David Gibson wrote: =20 > > > > On Wed, Mar 16, 2016 at 04:48:50PM +0100, Igor Mammedov wrote: =20 > > > > > On Wed, 16 Mar 2016 09:18:03 +0530 > > > > > Bharata B Rao wrote: > > > > > =20 > > > > > > On Mon, Mar 14, 2016 at 10:47:28AM +0100, Igor Mammedov wrote: = =20 > > > > > > > On Fri, 11 Mar 2016 10:24:29 +0530 > > > > > > > Bharata B Rao wrote: > > > > > > > =20 > > > > > > > > Hi, > > > > > > > >=20 > > > > > > > > This is the next version of "Core based CPU hotplug for Pow= erPC sPAPR" that > > > > > > > > was posted at > > > > > > > > https://lists.gnu.org/archive/html/qemu-ppc/2016-03/msg0008= 1.html > > > > > > > >=20 > > > > > > > > device_add semantics > > > > > > > > -------------------- > > > > > > > > For -smp 16,sockets=3D1,cores=3D2,threads=3D8,maxcpus=3D32 > > > > > > > > (qemu) device_add spapr-cpu-core,id=3Dcore2,core=3D16,cpu_m= odel=3Dhost[,threads=3D8] =20 > > > > > > > do you plan to allow user to hotplug different cpu_models? > > > > > > > If not it would be better to hide cpu_model from user > > > > > > > and set it from machine pre_plug handler. =20 > > > > > >=20 > > > > > > In my earlier implementations I derived cpu model from -cpu and= threads from > > > > > > -smp,threads=3D commandline options and never exposed them to d= evice_add > > > > > > command. > > > > > >=20 > > > > > > Though we don't support heterogenous systems (different cpu mod= els and/or > > > > > > threads) now, it was felt that it should be easy enough to supp= ort such > > > > > > systems if required in future, that's how cpu_model and threads= became > > > > > > options for device_add. > > > > > >=20 > > > > > > One of the things that David felt was missing from my earlier Q= MP query > > > > > > command (and which is true in your QMP query implementation als= o) is that > > > > > > we aren't exporting cpu_model at all, at least for not-yet-plug= ged cores. > > > > > > So should we include that or let management figure that out sin= ce it > > > > > > would already know about the CPU model. =20 > > > > > 1. > > > > > so since you are not planning supporting heterogeneous setup yet, > > > > > I'd suggest to refrain from making user to provide cpu_model at > > > > > device_add time. Instead make machine code to set it for cores it > > > > > creates before core.realize() (yet another use for pre_plug()). > > > > >=20 > > > > > That way mgmt doesn't have to figure out what cpu_model to set at > > > > > device_add time and doesn't have find out what property to use fo= r it. =20 > > > >=20 > > > > Yes.. of course you could also do the same thing for nr_threads, so > > > > I'm wondering whether there's a good argument to keep one in > > > > pre_plug() and one in query-hotpluggable-cpus. =20 > > >=20 > > > Right, so what should be the way forward ? Should we keep cpu_model= =3D and > > > threads=3D options with device_add or just threads=3D or neither ? = =20 > >=20 > > I'm inclined to keep them both in device_add - I like the idea of > > having an example on day 0 of advertising extra properties (beyond > > nr_threads and location) to set from query-hotpluggable-cpus. > >=20 > > But, I'd probably change my mind if Igor or someone has a stronger > > opinion. > I don't have a strong opinion on this, but you have to keep in mind > that one you make it ABI you probably would have to maintain it forever. >=20 > So far 'threads' and 'cpu_model' look like a constant values, > fixed at start-up time for every core. > Taking in account that user is not supposed to change them during > hotplug time and that they are the same for every core, > I'd go for conservative route and hide them in pre_plug() for now. > You always can expose them later if needed. Hm, yes, I see your point. Hiding them in pre-plug does also make life more convenient for someone (not libvirt) manually experimenting with this - less to type on their device-add command. > > If we advertise cpu_model, however, it should probably be changed to > > cpu thread class name, since IIUC that's an existing advertised part > > of the QOM interface, but cpu_model isn't. > I still think that spapr-cpu-core should be an abstract type > with a concrete set of derived cores types per each thread type. > But this question is not related to hotplug, but rather to > start-up of QEMU from scratch with -device and supported types > discovery. So I'd postpone question for later and that's yet another > reason why I'd like to hide cpu_model from user for now. Ah.. yes, I think you're probably right, and we should have derived types. It's a little bit awkward for spapr, but we can probably do some macro magic to make it not too bad. --=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 --+hz2tM55CCA8Ej21 Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJW8JBfAAoJEGw4ysog2bOSCGQP/jTcGABZkHWLP8DP0OwmZyNF UO0SYCa7tKL54jbo/47vhRH2GLDTxmBRRrY6URQ6Vn74uqrFUU4g8K/6y5Jk01b3 s5BIKsA9XBdLG59I3QLih0RXgvYAiG2SfmFrsZF1BxGgvTBALAufTyRP9A3RgAoi dXml9rBDhsiJAJd3GuK0Gw2IEyS708BW4Y5AYME66kP7lL6BXFCXhvGBqRnC+Jtw g77mwO5cpzUq8rLiMqYk7mq+1eGLNFjulN3BvRiJDaFQhc4/Kqjk9LFZAT05XKra tY9Xe4YqZzhdAk+W3EoAmD4iwomzrLdIKVreVB7nZFKhz/DlJ3RMQlyiVZRQFDH7 Zru/BxSHqQDdnHyMr0u1o3vajhib73eOl1u8V41OwKiomTzJUimtV6s5fSdaVFPd eTxhUnRx7UgzNkSsWU7VbTfhhKjmnZgmh7iHyL5c5EkRhG3MKFZtCaD9qI67X+3o J72Cv+pjdwwxqLpuiLHRNP/2iwhdfcY6/bKu+B6KcntYlZxUjsb5VJ8I3Tr8U6uR hgKBlpSl511VVW64e/h+TYqoBTcr07WldCjj8l+q2iIzC/2ADz21KyyBlHIR+TBT Z9sVqtN2sjFOD2Za+iKC82sVwRw70WAw4zPBIM22+TP9qXBMt2c4AzG+L3PwQ2xz /SuzIB8XjeZgmmlYSKOC =0udA -----END PGP SIGNATURE----- --+hz2tM55CCA8Ej21--