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 <bhar...@linux.vnet.ibm.com>
yes. it is nicer without the SPAPR_CPU_CORE_INITFN() routine.
Reviewed-by: Cédric Le Goater <c...@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
>
