From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:51041) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1amsvR-0005oZ-8g for qemu-devel@nongnu.org; Sun, 03 Apr 2016 21:03:30 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1amsvO-0004ja-1j for qemu-devel@nongnu.org; Sun, 03 Apr 2016 21:03:29 -0400 Date: Mon, 4 Apr 2016 10:16:10 +1000 From: David Gibson Message-ID: <20160404001610.GC16485@voom.fritz.box> References: <1459413561-30745-1-git-send-email-bharata@linux.vnet.ibm.com> <1459413561-30745-9-git-send-email-bharata@linux.vnet.ibm.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="IpbVkmxF4tDyP/Kb" Content-Disposition: inline In-Reply-To: <1459413561-30745-9-git-send-email-bharata@linux.vnet.ibm.com> Subject: Re: [Qemu-devel] [RFC PATCH v2.1 08/12] spapr: Add CPU type specific core devices List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Bharata B Rao Cc: mjrosato@linux.vnet.ibm.com, agraf@suse.de, thuth@redhat.com, pkrempa@redhat.com, ehabkost@redhat.com, aik@ozlabs.ru, qemu-devel@nongnu.org, armbru@redhat.com, borntraeger@de.ibm.com, qemu-ppc@nongnu.org, pbonzini@redhat.com, imammedo@redhat.com, afaerber@suse.de, mdroth@linux.vnet.ibm.com --IpbVkmxF4tDyP/Kb Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Thu, Mar 31, 2016 at 02:09:17PM +0530, Bharata B Rao wrote: > Introduce core devices for each CPU type supported by sPAPR. These > core devices are derived from the base spapr-cpu-core device type. >=20 > TODO: > - Add core types for other remaining CPU types > - Handle CPU model alias correctly >=20 > Signed-off-by: Bharata B Rao > --- > hw/ppc/spapr.c | 3 +- > hw/ppc/spapr_cpu_core.c | 118 ++++++++++++++++++++++++++++++++++= ++++++ > include/hw/ppc/spapr.h | 1 + > include/hw/ppc/spapr_cpu_core.h | 36 ++++++++++++ > 4 files changed, 156 insertions(+), 2 deletions(-) >=20 > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c > index 64c4acc..45ac5dc 100644 > --- a/hw/ppc/spapr.c > +++ b/hw/ppc/spapr.c > @@ -1614,8 +1614,7 @@ static void spapr_boot_set(void *opaque, const char= *boot_device, > machine->boot_order =3D g_strdup(boot_device); > } > =20 > -static void spapr_cpu_init(sPAPRMachineState *spapr, PowerPCCPU *cpu, > - Error **errp) > +void spapr_cpu_init(sPAPRMachineState *spapr, PowerPCCPU *cpu, Error **e= rrp) > { > CPUPPCState *env =3D &cpu->env; > =20 > diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c > index 8cbe2a5..3751a54 100644 > --- a/hw/ppc/spapr_cpu_core.c > +++ b/hw/ppc/spapr_cpu_core.c > @@ -22,9 +22,127 @@ static const TypeInfo spapr_cpu_core_type_info =3D { > .instance_size =3D sizeof(sPAPRCPUCore), > }; > =20 > +#define DEFINE_SPAPR_CPU_CORE(_name) = \ > +static void = \ > +glue(_name, _spapr_cpu_core_create_threads)(DeviceState *dev, int thread= s, \ > + Error **errp) = \ > +{ = \ > + int i; = \ > + Error *local_err =3D NULL; = \ > + sPAPRCPUCore *sc =3D SPAPR_CPU_CORE(OBJECT(dev)); = \ > + glue(_name, sPAPRCPUCore) * core =3D = \ > + glue(_name, _SPAPR_CPU_CORE)(OBJECT(dev)); = \ > + = \ > + for (i =3D 0; i < threads; i++) { = \ > + char id[32]; = \ > + = \ > + object_initialize(&sc->threads[i], sizeof(sc->threads[i]), = \ > + object_class_get_name(core->cpu)); = \ > + snprintf(id, sizeof(id), "thread[%d]", i); = \ > + object_property_add_child(OBJECT(core), id, OBJECT(&sc->threads[= i]), \ > + &local_err); = \ > + if (local_err) { = \ > + goto err; = \ > + } = \ > + } = \ > + return; = \ > + = \ > +err: = \ > + while (--i) { = \ > + object_unparent(OBJECT(&sc->threads[i])); = \ > + } = \ > + error_propagate(errp, local_err); = \ > +} = \ > + = \ > +static int = \ > +glue(_name, _spapr_cpu_core_realize_child)(Object *child, void *opaque) = \ > +{ = \ > + Error **errp =3D opaque; = \ > + sPAPRMachineState *spapr =3D SPAPR_MACHINE(qdev_get_machine()); = \ > + CPUState *cs =3D CPU(child); = \ > + PowerPCCPU *cpu =3D POWERPC_CPU(cs); = \ > + = \ > + object_property_set_bool(child, true, "realized", errp); = \ > + if (*errp) { = \ > + return 1; = \ > + } = \ > + = \ > + spapr_cpu_init(spapr, cpu, errp); = \ > + if (*errp) { = \ > + return 1; = \ > + } = \ > + return 0; = \ > +} = \ > + = \ > +static void = \ > +glue(_name, _spapr_cpu_core_realize)(DeviceState *dev, Error **errp) = \ > +{ = \ > + sPAPRCPUCore *sc =3D SPAPR_CPU_CORE(OBJECT(dev)); = \ > + CPUCore *cc =3D CPU_CORE(OBJECT(dev)); = \ > + Error *local_err =3D NULL; = \ > + = \ > + /* = \ > + * TODO: This is CPU model specific CPU core's realize routine. = \ > + * However I am initializing "threads" field of the parent type = \ > + * sPAPRCPUCore here. Is this ok ? If not I will have make "threads"= \ > + * part of CPU model specific CPU core type and have different plug(= ) \ > + * handlers for each type instead of having a common plug() handler = \ > + * for all core types. = \ > + */ > \ I think that should be fine - it's generally ok for methods to manipulate superclass data fields (technically this is what C++ calls a "protected" as opposed to "private" field). > + sc->threads =3D g_new0(PowerPCCPU, cc->threads); = \ > + glue(_name, _spapr_cpu_core_create_threads)(dev, cc->threads, &local= _err); \ > + if (local_err) { = \ > + goto out; = \ > + } = \ > + = \ > + object_child_foreach(OBJECT(dev), = \ > + glue(_name, _spapr_cpu_core_realize_child), = \ > + &local_err); = \ > + = \ > +out: = \ > + if (local_err) { = \ > + g_free(sc->threads); = \ > + error_propagate(errp, local_err); = \ > + } = \ > +} = \ > + = \ > +static void = \ > +glue(_name, _spapr_cpu_core_instance_init)(Object *obj) = \ > +{ = \ > + glue(_name, sPAPRCPUCore) * core =3D glue(_name, _SPAPR_CPU_CORE)(ob= j); \ > + const char *type =3D stringify(_name) "-" TYPE_POWERPC_CPU; = \ > + ObjectClass *oc =3D object_class_by_name(type); = \ > + = \ > + core->cpu =3D oc; = \ > +} = \ > + = \ > +static void = \ > +glue(_name, _spapr_cpu_core_class_init)(ObjectClass *oc, void *data) = \ > +{ = \ > + = \ > + DeviceClass *dc =3D DEVICE_CLASS(oc); = \ > + dc->realize =3D glue(_name, _spapr_cpu_core_realize); = \ > +} = \ > + = \ > +static const TypeInfo glue(_name, _spapr_cpu_core_type_info) =3D = \ > +{ = \ > + .name =3D stringify(_name) "-" TYPE_SPAPR_CPU_CORE, = \ > + .parent =3D TYPE_SPAPR_CPU_CORE, = \ > + .instance_size =3D sizeof(glue(_name, sPAPRCPUCore)), = \ > + .instance_init =3D glue(_name, _spapr_cpu_core_instance_init), = \ > + .class_init =3D glue(_name, _spapr_cpu_core_class_init), = \ > +}; > + > +DEFINE_SPAPR_CPU_CORE(host); > +DEFINE_SPAPR_CPU_CORE(POWER7); > +DEFINE_SPAPR_CPU_CORE(POWER8); > + > static void spapr_cpu_core_register_types(void) > { > type_register_static(&spapr_cpu_core_type_info); > + type_register_static(&host_spapr_cpu_core_type_info); > + type_register_static(&POWER7_spapr_cpu_core_type_info); > + type_register_static(&POWER8_spapr_cpu_core_type_info); > } > =20 > type_init(spapr_cpu_core_register_types) > diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h > index 098d85d..0fdf448 100644 > --- a/include/hw/ppc/spapr.h > +++ b/include/hw/ppc/spapr.h > @@ -585,6 +585,7 @@ void spapr_hotplug_req_add_by_count(sPAPRDRConnectorT= ype drc_type, > uint32_t count); > void spapr_hotplug_req_remove_by_count(sPAPRDRConnectorType drc_type, > uint32_t count); > +void spapr_cpu_init(sPAPRMachineState *spapr, PowerPCCPU *cpu, Error **e= rrp); > =20 > /* rtas-configure-connector state */ > struct sPAPRConfigureConnectorState { > diff --git a/include/hw/ppc/spapr_cpu_core.h b/include/hw/ppc/spapr_cpu_c= ore.h > index e3340ea..71e69c0 100644 > --- a/include/hw/ppc/spapr_cpu_core.h > +++ b/include/hw/ppc/spapr_cpu_core.h > @@ -24,4 +24,40 @@ typedef struct sPAPRCPUCore { > PowerPCCPU *threads; > } sPAPRCPUCore; > =20 > +#define TYPE_host_SPAPR_CPU_CORE "host-spapr-cpu-core" > +#define host_SPAPR_CPU_CORE(obj) \ > + OBJECT_CHECK(hostsPAPRCPUCore, (obj), TYPE_host_SPAPR_CPU_CORE) > + > +typedef struct hostsPAPRCPUCore { > + /*< private >*/ > + sPAPRCPUCore parent_obj; > + > + /*< public >*/ > + ObjectClass *cpu; > +} hostsPAPRCPUCore; > + > +#define TYPE_POWER7_SPAPR_CPU_CORE "POWER7-spapr-cpu-core" > +#define POWER7_SPAPR_CPU_CORE(obj) \ > + OBJECT_CHECK(POWER7sPAPRCPUCore, (obj), TYPE_POWER7_SPAPR_CPU_CORE) > + > +typedef struct POWER7sPAPRCPUCore { > + /*< private >*/ > + sPAPRCPUCore parent_obj; > + > + /*< public >*/ > + ObjectClass *cpu; > +} POWER7sPAPRCPUCore; > + > +#define TYPE_POWER8_SPAPR_CPU_CORE "POWER8-spapr-cpu-core" > +#define POWER8_SPAPR_CPU_CORE(obj) \ > + OBJECT_CHECK(POWER8sPAPRCPUCore, (obj), TYPE_POWER8_SPAPR_CPU_CORE) > + > +typedef struct POWER8sPAPRCPUCore { > + /*< private >*/ > + sPAPRCPUCore parent_obj; > + > + /*< public >*/ > + ObjectClass *cpu; > +} POWER8sPAPRCPUCore; > + > #endif --=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 --IpbVkmxF4tDyP/Kb Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJXAbJKAAoJEGw4ysog2bOSlpgQANnCH86czCzX+VLSilpQ/9ps TZ134yvoYFeY05So6V9vYy8OgdLpA2Z9zV5fkH/KXZ2pcvnxrhZwYv6liWLo4M7F S5lUh2zz3M1vTY4nfkMbKd5X+Ma3KRL6FZsnvKLuMHVDqfH72Sa2k+LCdAl1wQCH ebd3+RFFcr73XXBNEGjwqN0wYK2PZWqZ2EcWFVeD/YOTCZVTyHlJtU2bAEyPZoaW o5ZtA/7YIK/Sb2EMimZYdsKPAD4xK5Vga2dUrfadhRGvtYs39mDJ+tzx+KvRZ+/t /qqEttKV8uBAVhNjzD9zAvXWrmmlkPWrZTYgugCWuZminSjubL/nwFI9XX0zjOZt tX/lSxI5MkGcoJBRd2ElU727vP0agzLqPv8ydNlsMZXt+AgkeQ2zKbcLZESsYKhd SUApdjKDBXgl43P+smmNetXEe14RZifNy9cxCtTbIhTIGX0GS+HvcupqGrifEMjo NryvtFy9wUJ0RTTe5f7uM0pXyyNkevMFs2y3iQJlFLEEZ5YyRJSM0T2VhBfpK2sA xsRordZcTOpaopOJ6zshPF7n6CvVuiFoQCsfa1ydLkvCGAEokFWyrvBDKmyFj8h7 RtQWWgUnwYXZs8I4roc6aSgKBr6f9wfNeNej4D3deu8rJjrB4VAIK3R7HHskY2ng Guz0sYKK1nXliSEGwKwJ =NMH9 -----END PGP SIGNATURE----- --IpbVkmxF4tDyP/Kb--