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. > > TODO: > - Add core types for other remaining CPU types > - Handle CPU model alias correctly > > Signed-off-by: Bharata B Rao <bhar...@linux.vnet.ibm.com> > --- > 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(-) > > 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 = g_strdup(boot_device); > } > > -static void spapr_cpu_init(sPAPRMachineState *spapr, PowerPCCPU *cpu, > - Error **errp) > +void spapr_cpu_init(sPAPRMachineState *spapr, PowerPCCPU *cpu, Error **errp) > { > CPUPPCState *env = &cpu->env; > > 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 = { > .instance_size = sizeof(sPAPRCPUCore), > }; > > +#define DEFINE_SPAPR_CPU_CORE(_name) > \ > +static void > \ > +glue(_name, _spapr_cpu_core_create_threads)(DeviceState *dev, int threads, > \ > + Error **errp) > \ > +{ > \ > + int i; > \ > + Error *local_err = NULL; > \ > + sPAPRCPUCore *sc = SPAPR_CPU_CORE(OBJECT(dev)); > \ > + glue(_name, sPAPRCPUCore) * core = > \ > + glue(_name, _SPAPR_CPU_CORE)(OBJECT(dev)); > \ > + > \ > + for (i = 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 = opaque; > \ > + sPAPRMachineState *spapr = SPAPR_MACHINE(qdev_get_machine()); > \ > + CPUState *cs = CPU(child); > \ > + PowerPCCPU *cpu = 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; > \ > +} > \
If you put the ObjectClass * for the threads in the base abstract class's class structure, then you can move most of this logic to the abstract class as well and make the macro-ized stuff much smaller. The realize_child stuff doesn't even need the ObjectClass* in the base class to factor out. > +static void > \ > +glue(_name, _spapr_cpu_core_realize)(DeviceState *dev, Error **errp) > \ > +{ > \ > + sPAPRCPUCore *sc = SPAPR_CPU_CORE(OBJECT(dev)); > \ > + CPUCore *cc = CPU_CORE(OBJECT(dev)); > \ > + Error *local_err = 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. > \ > + */ > \ > + sc->threads = 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 = glue(_name, _SPAPR_CPU_CORE)(obj); > \ > + const char *type = stringify(_name) "-" TYPE_POWERPC_CPU; > \ > + ObjectClass *oc = object_class_by_name(type); > \ > + > \ > + core->cpu = oc; > \ > +} > \ > + > \ > +static void > \ > +glue(_name, _spapr_cpu_core_class_init)(ObjectClass *oc, void *data) > \ > +{ > \ > + > \ > + DeviceClass *dc = DEVICE_CLASS(oc); > \ > + dc->realize = glue(_name, _spapr_cpu_core_realize); > \ I think the only callback you should need to construct in the macro is class_init to initialize the ObjectClass* field. > +} > \ > + > \ > +static const TypeInfo glue(_name, _spapr_cpu_core_type_info) = > \ > +{ > \ > + .name = stringify(_name) "-" TYPE_SPAPR_CPU_CORE, > \ > + .parent = TYPE_SPAPR_CPU_CORE, > \ > + .instance_size = sizeof(glue(_name, sPAPRCPUCore)), > \ > + .instance_init = glue(_name, _spapr_cpu_core_instance_init), > \ > + .class_init = 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); > } > > 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(sPAPRDRConnectorType > 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 **errp); > > /* rtas-configure-connector state */ > struct sPAPRConfigureConnectorState { > diff --git a/include/hw/ppc/spapr_cpu_core.h b/include/hw/ppc/spapr_cpu_core.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; > > +#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; These are all identical so should also be macro constructed as well. I don't think there's actually any need for the structures to be exposed in a header file either, so you should be able to do it in the same macro that constructs the implementation. Uh.. except if you move the ObjectClass* to the base class you won't even need these. > #endif -- 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
signature.asc
Description: PGP signature