From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:49193) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bGH4U-0007bj-95 for qemu-devel@nongnu.org; Thu, 23 Jun 2016 22:42:19 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bGH4R-0002Do-N9 for qemu-devel@nongnu.org; Thu, 23 Jun 2016 22:42:17 -0400 Date: Fri, 24 Jun 2016 12:37:03 +1000 From: David Gibson Message-ID: <20160624023703.GB15625@voom.fritz.box> References: <1466238846-21365-1-git-send-email-bharata@linux.vnet.ibm.com> <20160621090957.20152960@igors-macbook-pro.local> <20160622023027.GB5613@in.ibm.com> <20160622090946.5dbcde8a@igors-macbook-pro.local> <20160623155536.GB2048@thinpad.lan.raisama.net> <20160624014006.GC27296@in.ibm.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="rS8CxjVDS/+yyDmU" Content-Disposition: inline In-Reply-To: <20160624014006.GC27296@in.ibm.com> Subject: Re: [Qemu-devel] [RFC PATCH v0 1/1] spapr: Support setting of compat CPU type for CPU cores List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Bharata B Rao Cc: Eduardo Habkost , Igor Mammedov , thuth@redhat.com, qemu-ppc@nongnu.org, qemu-devel@nongnu.org --rS8CxjVDS/+yyDmU Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Fri, Jun 24, 2016 at 07:10:06AM +0530, Bharata B Rao wrote: > On Thu, Jun 23, 2016 at 12:55:36PM -0300, Eduardo Habkost wrote: > > On Wed, Jun 22, 2016 at 09:09:46AM +0200, Igor Mammedov wrote: > > > On Wed, 22 Jun 2016 08:00:28 +0530 > > > Bharata B Rao wrote: > > >=20 > > > > On Tue, Jun 21, 2016 at 09:09:57AM +0200, Igor Mammedov wrote: > > > > > On Sat, 18 Jun 2016 14:04:06 +0530 > > > > > Bharata B Rao wrote: > > > > >=20 > > > > > > Compat CPU type is typically specified on -cpu cmdline option > > > > > > like: -cpu host,compat=3Dpower7 or -cpu POWER8E,compat=3Dpower7= etc. > > > > > > With the introduction of sPAPR CPU core devices, we need to > > > > > > support the same for core devices too. > > > > > >=20 > > > > > > Support the specification of CPU compat type on device_add > > > > > > command for sPAPRCPUCore devices like: > > > > > > (qemu) device_add > > > > > > POWER8E-spapr-cpu-core,id=3Dcore3,compat=3Dpower7,core-id=3D24 > > > > > >=20 > > > > > > Signed-off-by: Bharata B Rao > > > > > > --- > > > > > > Applies on ppc-for-2.7 branch of David Gibson's tree. > > > > > >=20 > > > > > > hw/ppc/spapr.c | 8 +++++ > > > > > > hw/ppc/spapr_cpu_core.c | 73 > > > > > > +++++++++++++++++++++++++++++++++++++++++ > > > > > > include/hw/ppc/spapr_cpu_core.h | 2 ++ 3 files changed, 83 > > > > > > insertions(+) > > > > > >=20 > > > > > > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c > > > > > > index 778fa25..2049d7d 100644 > > > > > > --- a/hw/ppc/spapr.c > > > > > > +++ b/hw/ppc/spapr.c > > > > > > @@ -1807,6 +1807,7 @@ static void ppc_spapr_init(MachineState > > > > > > *machine) if (i < spapr_cores) { > > > > > > char *type =3D > > > > > > spapr_get_cpu_core_type(machine->cpu_model); Object *core; > > > > > > + char *compat; > > > > > > =20 > > > > > > if (!object_class_by_name(type)) { > > > > > > error_report("Unable to find sPAPR CPU Core > > > > > > definition"); @@ -1818,6 +1819,13 @@ static void > > > > > > ppc_spapr_init(MachineState *machine) &error_fatal); > > > > > > object_property_set_int(core, core_dt_id, > > > > > > CPU_CORE_PROP_CORE_ID, &error_fatal); > > > > > > + compat =3D > > > > > > spapr_get_cpu_compat_type(machine->cpu_model); > > > > > > + if (compat) { > > > > > > + object_property_set_str(core, compat, > > > > > > "compat", > > > > > > + &error_fatal); > > > > > > + g_free(compat); > > > > > > + } > > > > > > + > > > > > > object_property_set_bool(core, true, "realized= ", > > > > > > &error_fatal); } > > > > > > } > > > > > > diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c > > > > > > index 3a5da09..9eb63cc 100644 > > > > > > --- a/hw/ppc/spapr_cpu_core.c > > > > > > +++ b/hw/ppc/spapr_cpu_core.c > > > > > > @@ -96,6 +96,24 @@ char *spapr_get_cpu_core_type(const char > > > > > > *model) return core_type; > > > > > > } > > > > > > =20 > > > > > > +/* > > > > > > + * Returns the CPU compat type specified in -cpu @model. > > > > > > + */ > > > > > > +char *spapr_get_cpu_compat_type(const char *model) > > > > > CPUClass already has such parser and hook to override it in case = of > > > > > target does legacy parsing, see CPUClass->parse_features. > > > > > Maybe extending generic core in similar way and reusing generic > > > > > parser will do the job. > > > >=20 > > > > In fact, I am already using CPUClass->parse_features() to set the > > > > feature properties (which right now is only compat=3D for us) for c= ore > > > > device. > > > >=20 > > > > What I need in the above routine spapr_get_cpu_compat_type() is to > > > > extract "compat=3D", so that I can verify that the compat type spec= ified > > > > with -device matches with what was specified with -cpu. > > > >=20 > > > > >=20 > > > > > However we are in progress [1] of converting legacy > > > > > -cpu cpuname,feat1=3Dx,feat2=3Dy,... > > > > > into a set of global properties > > > > > -global cputype.feat1=3Dx ... > > > > >=20 > > > > > it would be better if you wouldn't start using -cpu for features > > > > > but use directly -global mechanism, > > > > > for that you need only have a corresponding property in spapr_core > > > > > type. > > > >=20 > > > > Will this work be part of 2.7 ? If not, we will have to use -cpu to > > > > extract the compat type since powerpc core hotplug is already > > > > upstream. > > > You can use -global without that work but so far it looks like it will > > > make into 2.7. > >=20 > > I expect the -cpu =3D> globals series to be included soon, unless > > there are objections by others. > >=20 > > But as Igor said, globals are already supported. If you want to > > set a property in the CPU core objects, you just need to use: > >=20 > > -global spapr-cpu-core.compat=3DXXX > >=20 > > The conflict here seems to be that you want the "-cpu" option to > > affect the cpu-core objects, not just the VCPU/thread objects. > > But we have been assuming that "-cpu" would only affect the > > VCPU/thread objects. >=20 > -cpu property affects the CPU thread objects in our case too. Since those > CPU thread objects are aggregated under a core object, I was attempting > to add compat=3D with core device and validate that with -cpu type,compat= =3D > from cmdline. >=20 > What we really need is this: >=20 > If -global cputype.compat=3D or -cpu cpuname,compat=3D is set, we need to > apply the same to all the CPU thread objects of cpu core object. Hence > I am thinking we don't need a separate property for this in the core obje= ct. Right, I was just coming to the same conclusion. Since we always need to set the compat property uniformly, I don't think we actually need to bounce this information via the core object. Instead, the user can either use -global or -cpu to set it directly on all thread objects. In the unlikely event we ever have a setup where we need different cores in different compat modes we can add a property to the core object then. --=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 --rS8CxjVDS/+yyDmU Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJXbJzPAAoJEGw4ysog2bOSyocP/299nsTMRKsqmNxdQabfmmns tVf032V4juoRkicXFNc0aWaRm1kPVS6amCyPMoiKjGYoP0UI7qLZyS+wvFtuS2cl KnJYkQxj8gJ2FgEQalLbLW1Q3F0kF+rQ0A6g2xKrKZoRZ30KnkAAycZrwHG81EGz 3BI79s9hZ8B63J07G+37v27M0Ximgmw9n4suyTTSr5Gm5AaGlHZ/npcbHe/WjJG4 b2UfjP519/m/R7aFqyQ0xHN45ZwhwQ/lL8JdvTVTHmsaUbkSZB+zuIim6XTf5+mi yKMgQvF4Av32jSkBW7SSjtVIRrob7Oq0BPBtnownyU/kwINzMvNGkzgfRSn63fOk jK6ennJ9gCssEkRAtqwL4K4MKLEf4gFjwk5qgXhGObE3EctvCZIGXmKe6J7cD269 FV1oW2RyBFiSSkbj7Pkla31EgVp6OUn4/wUMS/4IgwjUSfLKe4zQF422OHpIrj0A LDob962pk+b/lGDa31bNcEYP5XcJblUDAmTNIrerKtL4jsc/dbTapx3UBvR+8nBT D+T4lCggJ+uNuF/XqSI9Hbtw+Wi1Y9Zx7EOsMgMkol7JoseyH20uKig+ZVNMCs8D g753jYns/U5Ythr+WiOc9pcng/6P6DiZVNE9aTw828a0bwZKnv32gSTh1VkpcRPA xVp3pLaTH6nTQgTMXPli =1vMz -----END PGP SIGNATURE----- --rS8CxjVDS/+yyDmU--