From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:33314) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bmJZH-0004YS-QO for qemu-devel@nongnu.org; Tue, 20 Sep 2016 07:50:33 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bmJZC-0001qC-RR for qemu-devel@nongnu.org; Tue, 20 Sep 2016 07:50:30 -0400 Date: Tue, 20 Sep 2016 16:22:02 +1000 From: David Gibson Message-ID: <20160920062202.GZ20488@umbus> References: <1473667040-2778-1-git-send-email-bharata@linux.vnet.ibm.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="kUmJ0kIGcKOYePjT" Content-Disposition: inline In-Reply-To: <1473667040-2778-1-git-send-email-bharata@linux.vnet.ibm.com> Subject: Re: [Qemu-devel] [PATCH v1] spapr: Introduce sPAPRCPUCoreClass List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Bharata B Rao Cc: qemu-devel@nongnu.org, qemu-ppc@nongnu.org, thuth@redhat.com --kUmJ0kIGcKOYePjT Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Mon, Sep 12, 2016 at 01:27:20PM +0530, Bharata B Rao wrote: > Each spapr cpu core type defines an instance_init routine which just > populates the CPU class name. This can be done in the class_init > commonly for all core types which simplifies the registration. > This is inspired by how PowerNV core types are registered. >=20 > Certain types of spapr cpu cores ('host' and generic type based on host > CPU) are initialized in target-ppc/kvm.c. To convert these type > registrations to use class_init, we need to expose > spapr_cpu_core_class_init() outside of spapr_cpu_core.c. >=20 > Commit d11b268e1765 added a generic sPAPR CPU core family > type to support cases like POWER8 CPU type on POWER8E host CPU. > Switching to class_init would fix such scenarios to use the right > CPU thread type instead of defaulting to host-powerpc64-cpu. >=20 > In an unrelated cleanup, fix a typo in .get_hotplug_handler routine. >=20 > Signed-off-by: Bharata B Rao > --- > Applies on ppc-for-2.8 branch of David Gibson's tree. >=20 > Changes in v1: > - Explicitly populate class_size. Looks good, applied to ppc-for-2.8. Sorry I forgot about this one for a while. >=20 > v0: https://lists.gnu.org/archive/html/qemu-devel/2016-09/msg00208.html >=20 > hw/ppc/spapr.c | 6 +-- > hw/ppc/spapr_cpu_core.c | 104 +++++++++++++++-------------------= ------ > include/hw/ppc/spapr_cpu_core.h | 11 ++++- > target-ppc/kvm.c | 22 +++------ > 4 files changed, 57 insertions(+), 86 deletions(-) >=20 > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c > index ca77bb0..79d36b3 100644 > --- a/hw/ppc/spapr.c > +++ b/hw/ppc/spapr.c > @@ -2312,8 +2312,8 @@ static void spapr_machine_device_pre_plug(HotplugHa= ndler *hotplug_dev, > } > } > =20 > -static HotplugHandler *spapr_get_hotpug_handler(MachineState *machine, > - DeviceState *dev) > +static HotplugHandler *spapr_get_hotplug_handler(MachineState *machine, > + DeviceState *dev) > { > if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM) || > object_dynamic_cast(OBJECT(dev), TYPE_SPAPR_CPU_CORE)) { > @@ -2385,7 +2385,7 @@ static void spapr_machine_class_init(ObjectClass *o= c, void *data) > mc->kvm_type =3D spapr_kvm_type; > mc->has_dynamic_sysbus =3D true; > mc->pci_allow_0_address =3D true; > - mc->get_hotplug_handler =3D spapr_get_hotpug_handler; > + mc->get_hotplug_handler =3D spapr_get_hotplug_handler; > hc->pre_plug =3D spapr_machine_device_pre_plug; > hc->plug =3D spapr_machine_device_plug; > hc->unplug =3D spapr_machine_device_unplug; > diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c > index bcb483d..6f0533c 100644 > --- a/hw/ppc/spapr_cpu_core.c > +++ b/hw/ppc/spapr_cpu_core.c > @@ -112,7 +112,8 @@ char *spapr_get_cpu_core_type(const char *model) > static void spapr_core_release(DeviceState *dev, void *opaque) > { > sPAPRCPUCore *sc =3D SPAPR_CPU_CORE(OBJECT(dev)); > - const char *typename =3D object_class_get_name(sc->cpu_class); > + sPAPRCPUCoreClass *scc =3D SPAPR_CPU_CORE_GET_CLASS(OBJECT(dev)); > + const char *typename =3D object_class_get_name(scc->cpu_class); > size_t size =3D object_type_get_instance_size(typename); > sPAPRMachineState *spapr =3D SPAPR_MACHINE(qdev_get_machine()); > CPUCore *cc =3D CPU_CORE(dev); > @@ -287,8 +288,9 @@ static void spapr_cpu_core_realize_child(Object *chil= d, Error **errp) > static void spapr_cpu_core_realize(DeviceState *dev, Error **errp) > { > sPAPRCPUCore *sc =3D SPAPR_CPU_CORE(OBJECT(dev)); > + sPAPRCPUCoreClass *scc =3D SPAPR_CPU_CORE_GET_CLASS(OBJECT(dev)); > CPUCore *cc =3D CPU_CORE(OBJECT(dev)); > - const char *typename =3D object_class_get_name(sc->cpu_class); > + const char *typename =3D object_class_get_name(scc->cpu_class); > size_t size =3D object_type_get_instance_size(typename); > Error *local_err =3D NULL; > void *obj; > @@ -331,83 +333,43 @@ err: > error_propagate(errp, local_err); > } > =20 > -static void spapr_cpu_core_class_init(ObjectClass *oc, void *data) > -{ > - DeviceClass *dc =3D DEVICE_CLASS(oc); > - dc->realize =3D spapr_cpu_core_realize; > -} > - > -/* > - * instance_init routines from different flavours of sPAPR CPU cores. > - */ > -#define SPAPR_CPU_CORE_INITFN(_type, _fname) \ > -static void glue(glue(spapr_cpu_core_, _fname), _initfn(Object *obj)) \ > -{ \ > - sPAPRCPUCore *core =3D SPAPR_CPU_CORE(obj); \ > - char *name =3D g_strdup_printf("%s-" TYPE_POWERPC_CPU, stringify(_ty= pe)); \ > - ObjectClass *oc =3D object_class_by_name(name); \ > - g_assert(oc); \ > - g_free((void *)name); \ > - core->cpu_class =3D oc; \ > -} > - > -SPAPR_CPU_CORE_INITFN(970mp_v1.0, 970MP_v10); > -SPAPR_CPU_CORE_INITFN(970mp_v1.1, 970MP_v11); > -SPAPR_CPU_CORE_INITFN(970_v2.2, 970); > -SPAPR_CPU_CORE_INITFN(POWER5+_v2.1, POWER5plus); > -SPAPR_CPU_CORE_INITFN(POWER7_v2.3, POWER7); > -SPAPR_CPU_CORE_INITFN(POWER7+_v2.1, POWER7plus); > -SPAPR_CPU_CORE_INITFN(POWER8_v2.0, POWER8); > -SPAPR_CPU_CORE_INITFN(POWER8E_v2.1, POWER8E); > -SPAPR_CPU_CORE_INITFN(POWER8NVL_v1.0, POWER8NVL); > - > -typedef struct SPAPRCoreInfo { > - const char *name; > - void (*initfn)(Object *obj); > -} SPAPRCoreInfo; > - > -static const SPAPRCoreInfo spapr_cores[] =3D { > +static const char *spapr_core_models[] =3D { > /* 970 */ > - { .name =3D "970_v2.2", .initfn =3D spapr_cpu_core_970_initfn }, > + "970_v2.2", > =20 > /* 970MP variants */ > - { .name =3D "970MP_v1.0", .initfn =3D spapr_cpu_core_970MP_v10_initf= n }, > - { .name =3D "970mp_v1.0", .initfn =3D spapr_cpu_core_970MP_v10_initf= n }, > - { .name =3D "970MP_v1.1", .initfn =3D spapr_cpu_core_970MP_v11_initf= n }, > - { .name =3D "970mp_v1.1", .initfn =3D spapr_cpu_core_970MP_v11_initf= n }, > + "970MP_v1.0", > + "970mp_v1.0", > + "970MP_v1.1", > + "970mp_v1.1", > =20 > /* POWER5+ */ > - { .name =3D "POWER5+_v2.1", .initfn =3D spapr_cpu_core_POWER5plus_in= itfn }, > + "POWER5+_v2.1", > =20 > /* POWER7 */ > - { .name =3D "POWER7_v2.3", .initfn =3D spapr_cpu_core_POWER7_initfn = }, > + "POWER7_v2.3", > =20 > /* POWER7+ */ > - { .name =3D "POWER7+_v2.1", .initfn =3D spapr_cpu_core_POWER7plus_in= itfn }, > + "POWER7+_v2.1", > =20 > /* POWER8 */ > - { .name =3D "POWER8_v2.0", .initfn =3D spapr_cpu_core_POWER8_initfn = }, > + "POWER8_v2.0", > =20 > /* POWER8E */ > - { .name =3D "POWER8E_v2.1", .initfn =3D spapr_cpu_core_POWER8E_initf= n }, > + "POWER8E_v2.1", > =20 > /* POWER8NVL */ > - { .name =3D "POWER8NVL_v1.0", .initfn =3D spapr_cpu_core_POWER8NVL_i= nitfn }, > - > - { .name =3D NULL } > + "POWER8NVL_v1.0", > }; > =20 > -static void spapr_cpu_core_register(const SPAPRCoreInfo *info) > +void spapr_cpu_core_class_init(ObjectClass *oc, void *data) > { > - TypeInfo type_info =3D { > - .parent =3D TYPE_SPAPR_CPU_CORE, > - .instance_size =3D sizeof(sPAPRCPUCore), > - .instance_init =3D info->initfn, > - }; > - > - type_info.name =3D g_strdup_printf("%s-" TYPE_SPAPR_CPU_CORE, info->= name); > - type_register(&type_info); > - g_free((void *)type_info.name); > + DeviceClass *dc =3D DEVICE_CLASS(oc); > + sPAPRCPUCoreClass *scc =3D SPAPR_CPU_CORE_CLASS(oc); > + > + dc->realize =3D spapr_cpu_core_realize; > + scc->cpu_class =3D cpu_class_by_name(TYPE_POWERPC_CPU, data); > + g_assert(scc->cpu_class); > } > =20 > static const TypeInfo spapr_cpu_core_type_info =3D { > @@ -415,17 +377,27 @@ static const TypeInfo spapr_cpu_core_type_info =3D { > .parent =3D TYPE_CPU_CORE, > .abstract =3D true, > .instance_size =3D sizeof(sPAPRCPUCore), > - .class_init =3D spapr_cpu_core_class_init, > + .class_size =3D sizeof(sPAPRCPUCoreClass), > }; > =20 > static void spapr_cpu_core_register_types(void) > { > - const SPAPRCoreInfo *info =3D spapr_cores; > + int i; > =20 > type_register_static(&spapr_cpu_core_type_info); > - while (info->name) { > - spapr_cpu_core_register(info); > - info++; > + > + for (i =3D 0; i < ARRAY_SIZE(spapr_core_models); i++) { > + TypeInfo type_info =3D { > + .parent =3D TYPE_SPAPR_CPU_CORE, > + .instance_size =3D sizeof(sPAPRCPUCore), > + .class_init =3D spapr_cpu_core_class_init, > + .class_data =3D (void *) spapr_core_models[i], > + }; > + > + type_info.name =3D g_strdup_printf("%s-" TYPE_SPAPR_CPU_CORE, > + spapr_core_models[i]); > + type_register(&type_info); > + g_free((void *)type_info.name); > } > } > =20 > diff --git a/include/hw/ppc/spapr_cpu_core.h b/include/hw/ppc/spapr_cpu_c= ore.h > index 1c9b319..283969b 100644 > --- a/include/hw/ppc/spapr_cpu_core.h > +++ b/include/hw/ppc/spapr_cpu_core.h > @@ -16,6 +16,10 @@ > #define TYPE_SPAPR_CPU_CORE "spapr-cpu-core" > #define SPAPR_CPU_CORE(obj) \ > OBJECT_CHECK(sPAPRCPUCore, (obj), TYPE_SPAPR_CPU_CORE) > +#define SPAPR_CPU_CORE_CLASS(klass) \ > + OBJECT_CLASS_CHECK(sPAPRCPUCoreClass, (klass), TYPE_SPAPR_CPU_CORE) > +#define SPAPR_CPU_CORE_GET_CLASS(obj) \ > + OBJECT_GET_CLASS(sPAPRCPUCoreClass, (obj), TYPE_SPAPR_CPU_CORE) > =20 > typedef struct sPAPRCPUCore { > /*< private >*/ > @@ -23,9 +27,13 @@ typedef struct sPAPRCPUCore { > =20 > /*< public >*/ > void *threads; > - ObjectClass *cpu_class; > } sPAPRCPUCore; > =20 > +typedef struct sPAPRCPUCoreClass { > + DeviceClass parent_class; > + ObjectClass *cpu_class; > +} sPAPRCPUCoreClass; > + > void spapr_core_pre_plug(HotplugHandler *hotplug_dev, DeviceState *dev, > Error **errp); > char *spapr_get_cpu_core_type(const char *model); > @@ -33,4 +41,5 @@ void spapr_core_plug(HotplugHandler *hotplug_dev, Devic= eState *dev, > Error **errp); > void spapr_core_unplug(HotplugHandler *hotplug_dev, DeviceState *dev, > Error **errp); > +void spapr_cpu_core_class_init(ObjectClass *oc, void *data); > #endif > diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c > index dcb68b9..4457337 100644 > --- a/target-ppc/kvm.c > +++ b/target-ppc/kvm.c > @@ -36,6 +36,7 @@ > #include "hw/sysbus.h" > #include "hw/ppc/spapr.h" > #include "hw/ppc/spapr_vio.h" > +#include "hw/ppc/spapr_cpu_core.h" > #include "hw/ppc/ppc.h" > #include "sysemu/watchdog.h" > #include "trace.h" > @@ -2364,19 +2365,6 @@ PowerPCCPUClass *kvm_ppc_get_host_cpu_class(void) > return pvr_pcc; > } > =20 > -#if defined(TARGET_PPC64) > -static void spapr_cpu_core_host_initfn(Object *obj) > -{ > - sPAPRCPUCore *core =3D SPAPR_CPU_CORE(obj); > - char *name =3D g_strdup_printf("%s-" TYPE_POWERPC_CPU, "host"); > - ObjectClass *oc =3D object_class_by_name(name); > - > - g_assert(oc); > - g_free((void *)name); > - core->cpu_class =3D oc; > -} > -#endif > - > static int kvm_ppc_register_host_cpu_type(void) > { > TypeInfo type_info =3D { > @@ -2404,14 +2392,16 @@ static int kvm_ppc_register_host_cpu_type(void) > #if defined(TARGET_PPC64) > type_info.name =3D g_strdup_printf("%s-"TYPE_SPAPR_CPU_CORE, "host"); > type_info.parent =3D TYPE_SPAPR_CPU_CORE, > - type_info.instance_size =3D sizeof(sPAPRCPUCore), > - type_info.instance_init =3D spapr_cpu_core_host_initfn, > - type_info.class_init =3D NULL; > + type_info.instance_size =3D sizeof(sPAPRCPUCore); > + type_info.instance_init =3D NULL; > + type_info.class_init =3D spapr_cpu_core_class_init; > + type_info.class_data =3D (void *) "host"; > type_register(&type_info); > g_free((void *)type_info.name); > =20 > /* Register generic spapr CPU family class for current host CPU type= */ > type_info.name =3D g_strdup_printf("%s-"TYPE_SPAPR_CPU_CORE, dc->des= c); > + type_info.class_data =3D (void *) dc->desc; > type_register(&type_info); > g_free((void *)type_info.name); > #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 --kUmJ0kIGcKOYePjT Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQIcBAEBCAAGBQJX4NWKAAoJEGw4ysog2bOS2mgQALrmj5D9tPvgFuqj7ZoZJxhB AgjdUC2fdydILFeq3gITytYF6ExyIWL00jjxZMW52RxPx26E1nIClCoL3TH3RS6P +b+tyGil6Ve/69UPjW8iL8VSrf+cD+LOViSskCItDKA0e/4pVcOEheR9bLrym0Id 8XnXXZqaaSMmC8lIP00JKBlT4W72cnEVCd32v+beX6QSoQ4KHqlS4HQg/466cCE2 fsWD9QVn9GH6yS53u0lFK+kbgBlUXqQ8D5ugwcYENml+kTSx8n4g5cmv+6V8o70R yAJr9d9WbhzR2AlcAOkLe3/hKecjSqtXhMKJNx9zpHqaRbDspkELicrXLCoL313w 89kwhqpq7YUlA9yIImq0VcqCTC2oEdMWTT/fwMpQ98aQYpTINvD5kCsqkWPlwbYK uN7OsSu9tDOEXwVfjQM4RqiP916N8F0q6SjoB4HbrPYvvfHKuUZ9EpprB6cxeIgf DyVHVpZQt5YEH/DARz4c5L50E2fffCGJ4xWW3IZhZKTIcsvxrrH0iha6LGzbMryo F+4bQ+Ta8leKTSXeb/mc80Vevzie5lJ0Ww/HRbcUUTgsRhr4KodbDrnlYqoVa0lT ZGQTvhS6hNTeFA9+4112qPLiqSmGZN77Y3jmcjce91SRpqTmGd6q+aSMqwD9YZ+c o56wDA5lfXLW2D1AmhxV =oYp8 -----END PGP SIGNATURE----- --kUmJ0kIGcKOYePjT--