Message ID | 1472808998-3030-1-git-send-email-bharata@linux.vnet.ibm.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
On 09/02/2016 11:36 AM, 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. > > 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. > > 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. > > In an unrelated cleanup, fix a typo in .get_hotplug_handler routine. > > Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com> yes. it is nicer without the SPAPR_CPU_CORE_INITFN() routine. Reviewed-by: Cédric Le Goater <clg@kaod.org> C. > --- > hw/ppc/spapr.c | 6 +-- > hw/ppc/spapr_cpu_core.c | 103 +++++++++++++++------------------------- > include/hw/ppc/spapr_cpu_core.h | 11 ++++- > target-ppc/kvm.c | 22 +++------ > 4 files changed, 56 insertions(+), 86 deletions(-) > > 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(HotplugHandler *hotplug_dev, > } > } > > -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 *oc, void *data) > mc->kvm_type = spapr_kvm_type; > mc->has_dynamic_sysbus = true; > mc->pci_allow_0_address = true; > - mc->get_hotplug_handler = spapr_get_hotpug_handler; > + mc->get_hotplug_handler = spapr_get_hotplug_handler; > hc->pre_plug = spapr_machine_device_pre_plug; > hc->plug = spapr_machine_device_plug; > hc->unplug = spapr_machine_device_unplug; > diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c > index bcb483d..1710908 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 = SPAPR_CPU_CORE(OBJECT(dev)); > - const char *typename = object_class_get_name(sc->cpu_class); > + sPAPRCPUCoreClass *scc = SPAPR_CPU_CORE_GET_CLASS(OBJECT(dev)); > + const char *typename = object_class_get_name(scc->cpu_class); > size_t size = object_type_get_instance_size(typename); > sPAPRMachineState *spapr = SPAPR_MACHINE(qdev_get_machine()); > CPUCore *cc = CPU_CORE(dev); > @@ -287,8 +288,9 @@ static void spapr_cpu_core_realize_child(Object *child, Error **errp) > static void spapr_cpu_core_realize(DeviceState *dev, Error **errp) > { > sPAPRCPUCore *sc = SPAPR_CPU_CORE(OBJECT(dev)); > + sPAPRCPUCoreClass *scc = SPAPR_CPU_CORE_GET_CLASS(OBJECT(dev)); > CPUCore *cc = CPU_CORE(OBJECT(dev)); > - const char *typename = object_class_get_name(sc->cpu_class); > + const char *typename = object_class_get_name(scc->cpu_class); > size_t size = object_type_get_instance_size(typename); > Error *local_err = NULL; > void *obj; > @@ -331,83 +333,43 @@ err: > error_propagate(errp, local_err); > } > > -static void spapr_cpu_core_class_init(ObjectClass *oc, void *data) > -{ > - DeviceClass *dc = DEVICE_CLASS(oc); > - dc->realize = 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 = SPAPR_CPU_CORE(obj); \ > - char *name = g_strdup_printf("%s-" TYPE_POWERPC_CPU, stringify(_type)); \ > - ObjectClass *oc = object_class_by_name(name); \ > - g_assert(oc); \ > - g_free((void *)name); \ > - core->cpu_class = 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[] = { > +static const char *spapr_core_models[] = { > /* 970 */ > - { .name = "970_v2.2", .initfn = spapr_cpu_core_970_initfn }, > + "970_v2.2", > > /* 970MP variants */ > - { .name = "970MP_v1.0", .initfn = spapr_cpu_core_970MP_v10_initfn }, > - { .name = "970mp_v1.0", .initfn = spapr_cpu_core_970MP_v10_initfn }, > - { .name = "970MP_v1.1", .initfn = spapr_cpu_core_970MP_v11_initfn }, > - { .name = "970mp_v1.1", .initfn = spapr_cpu_core_970MP_v11_initfn }, > + "970MP_v1.0", > + "970mp_v1.0", > + "970MP_v1.1", > + "970mp_v1.1", > > /* POWER5+ */ > - { .name = "POWER5+_v2.1", .initfn = spapr_cpu_core_POWER5plus_initfn }, > + "POWER5+_v2.1", > > /* POWER7 */ > - { .name = "POWER7_v2.3", .initfn = spapr_cpu_core_POWER7_initfn }, > + "POWER7_v2.3", > > /* POWER7+ */ > - { .name = "POWER7+_v2.1", .initfn = spapr_cpu_core_POWER7plus_initfn }, > + "POWER7+_v2.1", > > /* POWER8 */ > - { .name = "POWER8_v2.0", .initfn = spapr_cpu_core_POWER8_initfn }, > + "POWER8_v2.0", > > /* POWER8E */ > - { .name = "POWER8E_v2.1", .initfn = spapr_cpu_core_POWER8E_initfn }, > + "POWER8E_v2.1", > > /* POWER8NVL */ > - { .name = "POWER8NVL_v1.0", .initfn = spapr_cpu_core_POWER8NVL_initfn }, > - > - { .name = NULL } > + "POWER8NVL_v1.0", > }; > > -static void spapr_cpu_core_register(const SPAPRCoreInfo *info) > +void spapr_cpu_core_class_init(ObjectClass *oc, void *data) > { > - TypeInfo type_info = { > - .parent = TYPE_SPAPR_CPU_CORE, > - .instance_size = sizeof(sPAPRCPUCore), > - .instance_init = info->initfn, > - }; > - > - type_info.name = g_strdup_printf("%s-" TYPE_SPAPR_CPU_CORE, info->name); > - type_register(&type_info); > - g_free((void *)type_info.name); > + DeviceClass *dc = DEVICE_CLASS(oc); > + sPAPRCPUCoreClass *scc = SPAPR_CPU_CORE_CLASS(oc); > + > + dc->realize = spapr_cpu_core_realize; > + scc->cpu_class = cpu_class_by_name(TYPE_POWERPC_CPU, data); > + g_assert(scc->cpu_class); > } > > static const TypeInfo spapr_cpu_core_type_info = { > @@ -415,17 +377,26 @@ static const TypeInfo spapr_cpu_core_type_info = { > .parent = TYPE_CPU_CORE, > .abstract = true, > .instance_size = sizeof(sPAPRCPUCore), > - .class_init = spapr_cpu_core_class_init, > }; > > static void spapr_cpu_core_register_types(void) > { > - const SPAPRCoreInfo *info = spapr_cores; > + int i; > > type_register_static(&spapr_cpu_core_type_info); > - while (info->name) { > - spapr_cpu_core_register(info); > - info++; > + > + for (i = 0; i < ARRAY_SIZE(spapr_core_models); i++) { > + TypeInfo type_info = { > + .parent = TYPE_SPAPR_CPU_CORE, > + .instance_size = sizeof(sPAPRCPUCore), > + .class_init = spapr_cpu_core_class_init, > + .class_data = (void *) spapr_core_models[i], > + }; > + > + type_info.name = g_strdup_printf("%s-" TYPE_SPAPR_CPU_CORE, > + spapr_core_models[i]); > + type_register(&type_info); > + g_free((void *)type_info.name); > } > } > > diff --git a/include/hw/ppc/spapr_cpu_core.h b/include/hw/ppc/spapr_cpu_core.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) > > typedef struct sPAPRCPUCore { > /*< private >*/ > @@ -23,9 +27,13 @@ typedef struct sPAPRCPUCore { > > /*< public >*/ > void *threads; > - ObjectClass *cpu_class; > } sPAPRCPUCore; > > +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, DeviceState *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; > } > > -#if defined(TARGET_PPC64) > -static void spapr_cpu_core_host_initfn(Object *obj) > -{ > - sPAPRCPUCore *core = SPAPR_CPU_CORE(obj); > - char *name = g_strdup_printf("%s-" TYPE_POWERPC_CPU, "host"); > - ObjectClass *oc = object_class_by_name(name); > - > - g_assert(oc); > - g_free((void *)name); > - core->cpu_class = oc; > -} > -#endif > - > static int kvm_ppc_register_host_cpu_type(void) > { > TypeInfo type_info = { > @@ -2404,14 +2392,16 @@ static int kvm_ppc_register_host_cpu_type(void) > #if defined(TARGET_PPC64) > type_info.name = g_strdup_printf("%s-"TYPE_SPAPR_CPU_CORE, "host"); > type_info.parent = TYPE_SPAPR_CPU_CORE, > - type_info.instance_size = sizeof(sPAPRCPUCore), > - type_info.instance_init = spapr_cpu_core_host_initfn, > - type_info.class_init = NULL; > + type_info.instance_size = sizeof(sPAPRCPUCore); > + type_info.instance_init = NULL; > + type_info.class_init = spapr_cpu_core_class_init; > + type_info.class_data = (void *) "host"; > type_register(&type_info); > g_free((void *)type_info.name); > > /* Register generic spapr CPU family class for current host CPU type */ > type_info.name = g_strdup_printf("%s-"TYPE_SPAPR_CPU_CORE, dc->desc); > + type_info.class_data = (void *) dc->desc; > type_register(&type_info); > g_free((void *)type_info.name); > #endif >
On Fri, Sep 02, 2016 at 03:06:38PM +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. > > 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. > > 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. > > In an unrelated cleanup, fix a typo in .get_hotplug_handler routine. > > Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com> I like the concept, but... [snip] > static const TypeInfo spapr_cpu_core_type_info = { > @@ -415,17 +377,26 @@ static const TypeInfo spapr_cpu_core_type_info = { > .parent = TYPE_CPU_CORE, > .abstract = true, > .instance_size = sizeof(sPAPRCPUCore), > - .class_init = spapr_cpu_core_class_init, .. I'm pretty sure you need .class_size = sizeof(sPAPRCPUCoreClass) here, or initializing the cpu_class field will corrupt memory.
On Thu, Sep 08, 2016 at 11:51:32AM +1000, David Gibson wrote: > On Fri, Sep 02, 2016 at 03:06:38PM +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. > > > > 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. > > > > 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. > > > > In an unrelated cleanup, fix a typo in .get_hotplug_handler routine. > > > > Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com> > > I like the concept, but... > > [snip] > > static const TypeInfo spapr_cpu_core_type_info = { > > @@ -415,17 +377,26 @@ static const TypeInfo spapr_cpu_core_type_info = { > > .parent = TYPE_CPU_CORE, > > .abstract = true, > > .instance_size = sizeof(sPAPRCPUCore), > > - .class_init = spapr_cpu_core_class_init, > > .. I'm pretty sure you need .class_size = sizeof(sPAPRCPUCoreClass) > here, or initializing the cpu_class field will corrupt memory. Ok. The other thing I am not comfortable with is exposing .class_init routine outside of spapr_cpu_core.c so that cpu cores can be initialized from target-ppc/kvm.c. Any opinion on that ? Regards, Bharata.
On Thu, Sep 08, 2016 at 09:56:31AM +0530, Bharata B Rao wrote: > On Thu, Sep 08, 2016 at 11:51:32AM +1000, David Gibson wrote: > > On Fri, Sep 02, 2016 at 03:06:38PM +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. > > > > > > 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. > > > > > > 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. > > > > > > In an unrelated cleanup, fix a typo in .get_hotplug_handler routine. > > > > > > Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com> > > > > I like the concept, but... > > > > [snip] > > > static const TypeInfo spapr_cpu_core_type_info = { > > > @@ -415,17 +377,26 @@ static const TypeInfo spapr_cpu_core_type_info = { > > > .parent = TYPE_CPU_CORE, > > > .abstract = true, > > > .instance_size = sizeof(sPAPRCPUCore), > > > - .class_init = spapr_cpu_core_class_init, > > > > .. I'm pretty sure you need .class_size = sizeof(sPAPRCPUCoreClass) > > here, or initializing the cpu_class field will corrupt memory. > > Ok. > > The other thing I am not comfortable with is exposing > .class_init routine outside of spapr_cpu_core.c so that cpu cores > can be initialized from target-ppc/kvm.c. Any opinion on that ? Not a strong one. I guess my feeling is that it's a bit ugly, but worth it for the other advantages of this approach.
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(HotplugHandler *hotplug_dev, } } -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 *oc, void *data) mc->kvm_type = spapr_kvm_type; mc->has_dynamic_sysbus = true; mc->pci_allow_0_address = true; - mc->get_hotplug_handler = spapr_get_hotpug_handler; + mc->get_hotplug_handler = spapr_get_hotplug_handler; hc->pre_plug = spapr_machine_device_pre_plug; hc->plug = spapr_machine_device_plug; hc->unplug = spapr_machine_device_unplug; diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c index bcb483d..1710908 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 = SPAPR_CPU_CORE(OBJECT(dev)); - const char *typename = object_class_get_name(sc->cpu_class); + sPAPRCPUCoreClass *scc = SPAPR_CPU_CORE_GET_CLASS(OBJECT(dev)); + const char *typename = object_class_get_name(scc->cpu_class); size_t size = object_type_get_instance_size(typename); sPAPRMachineState *spapr = SPAPR_MACHINE(qdev_get_machine()); CPUCore *cc = CPU_CORE(dev); @@ -287,8 +288,9 @@ static void spapr_cpu_core_realize_child(Object *child, Error **errp) static void spapr_cpu_core_realize(DeviceState *dev, Error **errp) { sPAPRCPUCore *sc = SPAPR_CPU_CORE(OBJECT(dev)); + sPAPRCPUCoreClass *scc = SPAPR_CPU_CORE_GET_CLASS(OBJECT(dev)); CPUCore *cc = CPU_CORE(OBJECT(dev)); - const char *typename = object_class_get_name(sc->cpu_class); + const char *typename = object_class_get_name(scc->cpu_class); size_t size = object_type_get_instance_size(typename); Error *local_err = NULL; void *obj; @@ -331,83 +333,43 @@ err: error_propagate(errp, local_err); } -static void spapr_cpu_core_class_init(ObjectClass *oc, void *data) -{ - DeviceClass *dc = DEVICE_CLASS(oc); - dc->realize = 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 = SPAPR_CPU_CORE(obj); \ - char *name = g_strdup_printf("%s-" TYPE_POWERPC_CPU, stringify(_type)); \ - ObjectClass *oc = object_class_by_name(name); \ - g_assert(oc); \ - g_free((void *)name); \ - core->cpu_class = 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[] = { +static const char *spapr_core_models[] = { /* 970 */ - { .name = "970_v2.2", .initfn = spapr_cpu_core_970_initfn }, + "970_v2.2", /* 970MP variants */ - { .name = "970MP_v1.0", .initfn = spapr_cpu_core_970MP_v10_initfn }, - { .name = "970mp_v1.0", .initfn = spapr_cpu_core_970MP_v10_initfn }, - { .name = "970MP_v1.1", .initfn = spapr_cpu_core_970MP_v11_initfn }, - { .name = "970mp_v1.1", .initfn = spapr_cpu_core_970MP_v11_initfn }, + "970MP_v1.0", + "970mp_v1.0", + "970MP_v1.1", + "970mp_v1.1", /* POWER5+ */ - { .name = "POWER5+_v2.1", .initfn = spapr_cpu_core_POWER5plus_initfn }, + "POWER5+_v2.1", /* POWER7 */ - { .name = "POWER7_v2.3", .initfn = spapr_cpu_core_POWER7_initfn }, + "POWER7_v2.3", /* POWER7+ */ - { .name = "POWER7+_v2.1", .initfn = spapr_cpu_core_POWER7plus_initfn }, + "POWER7+_v2.1", /* POWER8 */ - { .name = "POWER8_v2.0", .initfn = spapr_cpu_core_POWER8_initfn }, + "POWER8_v2.0", /* POWER8E */ - { .name = "POWER8E_v2.1", .initfn = spapr_cpu_core_POWER8E_initfn }, + "POWER8E_v2.1", /* POWER8NVL */ - { .name = "POWER8NVL_v1.0", .initfn = spapr_cpu_core_POWER8NVL_initfn }, - - { .name = NULL } + "POWER8NVL_v1.0", }; -static void spapr_cpu_core_register(const SPAPRCoreInfo *info) +void spapr_cpu_core_class_init(ObjectClass *oc, void *data) { - TypeInfo type_info = { - .parent = TYPE_SPAPR_CPU_CORE, - .instance_size = sizeof(sPAPRCPUCore), - .instance_init = info->initfn, - }; - - type_info.name = g_strdup_printf("%s-" TYPE_SPAPR_CPU_CORE, info->name); - type_register(&type_info); - g_free((void *)type_info.name); + DeviceClass *dc = DEVICE_CLASS(oc); + sPAPRCPUCoreClass *scc = SPAPR_CPU_CORE_CLASS(oc); + + dc->realize = spapr_cpu_core_realize; + scc->cpu_class = cpu_class_by_name(TYPE_POWERPC_CPU, data); + g_assert(scc->cpu_class); } static const TypeInfo spapr_cpu_core_type_info = { @@ -415,17 +377,26 @@ static const TypeInfo spapr_cpu_core_type_info = { .parent = TYPE_CPU_CORE, .abstract = true, .instance_size = sizeof(sPAPRCPUCore), - .class_init = spapr_cpu_core_class_init, }; static void spapr_cpu_core_register_types(void) { - const SPAPRCoreInfo *info = spapr_cores; + int i; type_register_static(&spapr_cpu_core_type_info); - while (info->name) { - spapr_cpu_core_register(info); - info++; + + for (i = 0; i < ARRAY_SIZE(spapr_core_models); i++) { + TypeInfo type_info = { + .parent = TYPE_SPAPR_CPU_CORE, + .instance_size = sizeof(sPAPRCPUCore), + .class_init = spapr_cpu_core_class_init, + .class_data = (void *) spapr_core_models[i], + }; + + type_info.name = g_strdup_printf("%s-" TYPE_SPAPR_CPU_CORE, + spapr_core_models[i]); + type_register(&type_info); + g_free((void *)type_info.name); } } diff --git a/include/hw/ppc/spapr_cpu_core.h b/include/hw/ppc/spapr_cpu_core.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) typedef struct sPAPRCPUCore { /*< private >*/ @@ -23,9 +27,13 @@ typedef struct sPAPRCPUCore { /*< public >*/ void *threads; - ObjectClass *cpu_class; } sPAPRCPUCore; +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, DeviceState *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; } -#if defined(TARGET_PPC64) -static void spapr_cpu_core_host_initfn(Object *obj) -{ - sPAPRCPUCore *core = SPAPR_CPU_CORE(obj); - char *name = g_strdup_printf("%s-" TYPE_POWERPC_CPU, "host"); - ObjectClass *oc = object_class_by_name(name); - - g_assert(oc); - g_free((void *)name); - core->cpu_class = oc; -} -#endif - static int kvm_ppc_register_host_cpu_type(void) { TypeInfo type_info = { @@ -2404,14 +2392,16 @@ static int kvm_ppc_register_host_cpu_type(void) #if defined(TARGET_PPC64) type_info.name = g_strdup_printf("%s-"TYPE_SPAPR_CPU_CORE, "host"); type_info.parent = TYPE_SPAPR_CPU_CORE, - type_info.instance_size = sizeof(sPAPRCPUCore), - type_info.instance_init = spapr_cpu_core_host_initfn, - type_info.class_init = NULL; + type_info.instance_size = sizeof(sPAPRCPUCore); + type_info.instance_init = NULL; + type_info.class_init = spapr_cpu_core_class_init; + type_info.class_data = (void *) "host"; type_register(&type_info); g_free((void *)type_info.name); /* Register generic spapr CPU family class for current host CPU type */ type_info.name = g_strdup_printf("%s-"TYPE_SPAPR_CPU_CORE, dc->desc); + type_info.class_data = (void *) dc->desc; type_register(&type_info); g_free((void *)type_info.name); #endif
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. 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. 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. In an unrelated cleanup, fix a typo in .get_hotplug_handler routine. Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com> --- hw/ppc/spapr.c | 6 +-- hw/ppc/spapr_cpu_core.c | 103 +++++++++++++++------------------------- include/hw/ppc/spapr_cpu_core.h | 11 ++++- target-ppc/kvm.c | 22 +++------ 4 files changed, 56 insertions(+), 86 deletions(-)