From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:43504) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bJywI-0005iv-Eu for qemu-devel@nongnu.org; Mon, 04 Jul 2016 04:09:11 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bJywD-0002yK-Vz for qemu-devel@nongnu.org; Mon, 04 Jul 2016 04:09:10 -0400 Received: from 9.mo69.mail-out.ovh.net ([46.105.56.78]:34321) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bJywD-0002yA-Kr for qemu-devel@nongnu.org; Mon, 04 Jul 2016 04:09:05 -0400 Received: from player789.ha.ovh.net (b9.ovh.net [213.186.33.59]) by mo69.mail-out.ovh.net (Postfix) with ESMTP id DC4DB1001034 for ; Mon, 4 Jul 2016 10:09:04 +0200 (CEST) Date: Mon, 4 Jul 2016 10:08:53 +0200 From: Greg Kurz Message-ID: <20160704100853.7933f809@bahia.lan> In-Reply-To: <20160704083204.60c8b9e6@bahia.lan> References: <146741287399.948.15988269239450224065.stgit@bahia.lan> <146741290890.948.9125710372347515263.stgit@bahia.lan> <20160702080622.GH21596@in.ibm.com> <20160702103333.55119e79@bahia.lan> <20160704035455.GE2919@voom.fritz.box> <20160704083204.60c8b9e6@bahia.lan> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; boundary="Sig_/0yWayNl1GU/Vnma8imoyGxD"; protocol="application/pgp-signature" Subject: Re: [Qemu-devel] [Qemu-ppc] [PATCH v2 4/7] ppc: open code cpu creation for machine types List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: David Gibson Cc: qemu-devel@nongnu.org, qemu-ppc@nongnu.org, Cedric Le Goater , Bharata B Rao , Scott Wood , Igor Mammedov --Sig_/0yWayNl1GU/Vnma8imoyGxD Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: quoted-printable On Mon, 4 Jul 2016 08:32:04 +0200 Greg Kurz wrote: > On Mon, 4 Jul 2016 13:54:55 +1000 > David Gibson wrote: >=20 > > On Sat, Jul 02, 2016 at 10:33:33AM +0200, Greg Kurz wrote: =20 > > > On Sat, 2 Jul 2016 13:36:22 +0530 > > > Bharata B Rao wrote: > > > =20 > > > > On Sat, Jul 02, 2016 at 12:41:48AM +0200, Greg Kurz wrote: =20 > > > > > If we want to generate cpu_dt_id in the machine code, this must o= ccur > > > > > before the cpu gets realized. We must open code the cpu creation = to be > > > > > able to do this. > > > > >=20 > > > > > This patch just does that. It borrows some lines from previous wo= rk > > > > > from Bharata to handle the feature parsing. > > > > >=20 > > > > > Signed-off-by: Greg Kurz > > > > > --- > > > > > hw/ppc/ppc.c | 39 ++++++++++++++++++++++++++++++++++++++- > > > > > 1 file changed, 38 insertions(+), 1 deletion(-) > > > > >=20 > > > > > diff --git a/hw/ppc/ppc.c b/hw/ppc/ppc.c > > > > > index dc3d214009c5..57f4ddd073d0 100644 > > > > > --- a/hw/ppc/ppc.c > > > > > +++ b/hw/ppc/ppc.c > > > > > @@ -32,6 +32,7 @@ > > > > > #include "sysemu/cpus.h" > > > > > #include "hw/timer/m48t59.h" > > > > > #include "qemu/log.h" > > > > > +#include "qapi/error.h" > > > > > #include "qemu/error-report.h" > > > > > #include "hw/loader.h" > > > > > #include "sysemu/kvm.h" > > > > > @@ -1353,5 +1354,41 @@ PowerPCCPU *ppc_get_vcpu_by_dt_id(int cpu_= dt_id) > > > > >=20 > > > > > PowerPCCPU *ppc_cpu_init(const char *cpu_model) > > > > > { > > > > > - return POWERPC_CPU(cpu_generic_init(TYPE_POWERPC_CPU, cpu_mo= del)); > > > > > + PowerPCCPU *cpu; > > > > > + CPUClass *cc; > > > > > + ObjectClass *oc; > > > > > + gchar **model_pieces; > > > > > + Error *err =3D NULL; > > > > > + > > > > > + model_pieces =3D g_strsplit(cpu_model, ",", 2); > > > > > + if (!model_pieces[0]) { > > > > > + error_report("Invalid/empty CPU model name"); > > > > > + return NULL; > > > > > + } > > > > > + > > > > > + oc =3D cpu_class_by_name(TYPE_POWERPC_CPU, model_pieces[0]); > > > > > + if (oc =3D=3D NULL) { > > > > > + error_report("Unable to find CPU definition: %s", model_= pieces[0]); > > > > > + return NULL; > > > > > + } > > > > > + > > > > > + cpu =3D POWERPC_CPU(object_new(object_class_get_name(oc))); > > > > > + > > > > > + cc =3D CPU_CLASS(oc); > > > > > + cc->parse_features(CPU(cpu), model_pieces[1], &err); =20 > > > >=20 > > > > Igor is working on a patchset to convert -cpu features into global = properties. > > > > IIUC, after that patchset, it is not recommended to parse the -cpu = features > > > > for every CPU but do it only once. > > > > =20 > > >=20 > > > cpu_generic_init() in the current code also does the parsing, and as = the title > > > says, this patch is just about open coding the creation. I don't want= to > > > change behavior yet. > > >=20 > > > But yes, I agree that we should only parse features once and I'll be = more than > > > happy to fix this in a followup patch, based on Igor's work. > > >=20 > > > In the meantime, maybe I can add a comment stating that the parsing s= hould go > > > away ? =20 > >=20 > > Right. But the thing is by open coding here, you're making two copies > > that need to be fixed instead of one, which increases the chances of > > error. > >=20 > > It seems like it would be safer to change the generic code so there's > > a new generic function which doesn't do the realize which we can use > > on ppc (and other platforms when/if they need it). > >=20 > > Doing the change just on ppc by making our own copy of > > cpu_generic_init() seems more like to lead to future mistakes. > > =20 >=20 > I had this in v1: >=20 > http://patchwork.ozlabs.org/patch/642216/ >=20 > I'll repost it in v3. >=20 I tend to agree with Igor's latest feeback: https://lists.nongnu.org/archive/html/qemu-devel/2016-07/msg00381.html I'll keep the open coded version in v3. > > > > That is what I attempted here in the context of supporting compat c= pu type > > > > for pseries-2.7: > > > >=20 > > > > https://www.mail-archive.com/qemu-devel@nongnu.org/msg381660.html > > > > =20 > > >=20 > > > Yeah and this is where I borrowed some lines. :) > > > =20 > > > > Regards, > > > > Bharata. > > > > =20 > > >=20 > > > Cheers. > > > =20 > > =20 >=20 --Sig_/0yWayNl1GU/Vnma8imoyGxD Content-Type: application/pgp-signature Content-Description: OpenPGP digital signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iEYEARECAAYFAld6GZUACgkQAvw66wEB28KbEQCfRlMfwUrFs3tlY6/M8Zmfi8pC vv8AnjMs2Ckjr4psJkuXy1d8Rzx80wmj =XbEL -----END PGP SIGNATURE----- --Sig_/0yWayNl1GU/Vnma8imoyGxD--