On Tue, Mar 08, 2016 at 02:18:12PM +0100, Igor Mammedov wrote: > it's just a hack to get qiuck swith to numeric core id > should be split and merged in patches > introducing modified code. > > Signed-off-by: Igor Mammedov <[email protected]> > --- > hw/cpu/core.c | 32 +++++++++++++++++++++++--------- > hw/ppc/spapr.c | 39 ++------------------------------------- > hw/ppc/spapr_cpu_core.c | 25 ++----------------------- > include/hw/cpu/core.h | 4 ++-- > 4 files changed, 29 insertions(+), 71 deletions(-) > > diff --git a/hw/cpu/core.c b/hw/cpu/core.c > index d8caf37..90a9408 100644 > --- a/hw/cpu/core.c > +++ b/hw/cpu/core.c > @@ -7,25 +7,39 @@ > * See the COPYING file in the top-level directory. > */ > #include "hw/cpu/core.h" > +#include "qapi/visitor.h" > > -static char *core_prop_get_slot(Object *obj, Error **errp) > +static void core_prop_get_core(Object *obj, Visitor *v, > + const char *name, void *opaque, > + Error **errp) > { > - CPUCore *core = CPU_CORE(obj); > + CPUCore *cc = CPU_CORE(obj); > + int64_t value = cc->core; > > - return g_strdup(core->slot); > + visit_type_int(v, name, &value, errp); > } > > -static void core_prop_set_slot(Object *obj, const char *val, Error **errp) > +static void core_prop_set_core(Object *obj, Visitor *v, > + const char *name, void *opaque, > + Error **errp) > { > - CPUCore *core = CPU_CORE(obj); > - > - core->slot = g_strdup(val); > + CPUCore *cc = CPU_CORE(obj); > + Error *local_err = NULL; > + int64_t value; > + > + visit_type_int(v, name, &value, &local_err); > + if (local_err) { > + error_propagate(errp, local_err); > + return; > + } > + cc->core = value; > } > > static void cpu_core_instance_init(Object *obj) > { > - object_property_add_str(obj, "slot", core_prop_get_slot, > core_prop_set_slot, > - NULL); > + object_property_add(obj, CPU_CORE_ID_PROP, "int", > + core_prop_get_core, core_prop_set_core, > + NULL, NULL, NULL); > }
Something we should clarify at some point: is the core property
intended to be globally unique on its own, or just unique in
combination with other information (e.g. socket and/or node).
> static const TypeInfo cpu_core_type_info = {
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 6173c1b..6890a44 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -1755,28 +1755,6 @@ static void spapr_validate_node_memory(MachineState
> *machine, Error **errp)
> }
> }
>
> -/*
> - * Check to see if core is being hot-plugged into an already populated slot.
> - */
> -static void spapr_cpu_core_allow_set_link(Object *obj, const char *name,
> - Object *val, Error **errp)
> -{
> - Object *core = object_property_get_link(qdev_get_machine(), name, NULL);
> -
> - /*
> - * Allow the link to be unset when the core is unplugged.
> - */
> - if (!val) {
> - return;
> - }
> -
> - if (core) {
> - char *path = object_get_canonical_path(core);
> - error_setg(errp, "Slot %s already populated with %s", name, path);
> - g_free(path);
> - }
> -}
> -
> /* pSeries LPAR / sPAPR hardware init */
> static void ppc_spapr_init(MachineState *machine)
> {
> @@ -1884,21 +1862,8 @@ static void ppc_spapr_init(MachineState *machine)
> spapr->cores = g_new0(Object *, spapr_max_cores);
>
> for (i = 0; i < spapr_max_cores; i++) {
> - char name[32];
> -
> - /*
> - * Create links from machine objects to all possible cores.
> - */
> - snprintf(name, sizeof(name), "%s[%d]", SPAPR_MACHINE_CPU_CORE_PROP,
> i);
> - object_property_add_link(OBJECT(spapr), name, TYPE_SPAPR_CPU_CORE,
> - (Object **)&spapr->cores[i],
> - spapr_cpu_core_allow_set_link,
> - OBJ_PROP_LINK_UNREF_ON_RELEASE,
> - &error_fatal);
> -
> /*
> - * Create cores and set link from machine object to core object for
> - * boot time CPUs and realize them.
> + * Create cores for boot time CPUs and realize them.
> */
> if (i < spapr_cores) {
> Object *core = object_new(TYPE_SPAPR_CPU_CORE);
> @@ -1907,7 +1872,7 @@ static void ppc_spapr_init(MachineState *machine)
> &error_fatal);
> object_property_set_int(core, smp_threads, "nr_threads",
> &error_fatal);
> - object_property_set_str(core, name, CPU_CORE_SLOT_PROP,
> + object_property_set_int(core, i, CPU_CORE_ID_PROP,
> &error_fatal);
Not really important for this quick hack, but for spapr it might make
sense to have the "core" property be the dt_id, rather than just an
arbitrary index.
> object_property_set_bool(core, true, "realized", &error_fatal);
> }
> diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
> index 5156eb3..98af840 100644
> --- a/hw/ppc/spapr_cpu_core.c
> +++ b/hw/ppc/spapr_cpu_core.c
> @@ -117,19 +117,12 @@ static int spapr_cpu_release(Object *obj, void *opaque)
> static void spapr_core_release(DeviceState *dev, void *opaque)
> {
> struct sPAPRCPUUnplugList unplug_list;
> - sPAPRMachineState *spapr = SPAPR_MACHINE(qdev_get_machine());
> sPAPRCPUCore *core = SPAPR_CPU_CORE(OBJECT(dev));
> - char *slot = object_property_get_str(OBJECT(dev), CPU_CORE_SLOT_PROP,
> - &error_fatal);
>
> QLIST_INIT(&unplug_list);
> object_child_foreach(OBJECT(dev), spapr_cpu_release, &unplug_list);
> spapr_cpu_core_cleanup(&unplug_list);
>
> - /* Unset the link from machine object to this core */
> - object_property_set_link(OBJECT(spapr), NULL, slot, NULL);
> - g_free(slot);
> -
> g_free(core->threads);
> object_unparent(OBJECT(dev));
> }
> @@ -181,9 +174,6 @@ static int spapr_cpu_core_realize_child(Object *child,
> void *opaque)
> static void spapr_cpu_core_realize(DeviceState *dev, Error **errp)
> {
> sPAPRCPUCore *core = SPAPR_CPU_CORE(OBJECT(dev));
> - sPAPRMachineState *spapr = SPAPR_MACHINE(qdev_get_machine());
> - char *slot;
> - Error *local_err = NULL;
>
> if (!core->nr_threads) {
> error_setg(errp, "nr_threads property can't be 0");
> @@ -199,19 +189,8 @@ static void spapr_cpu_core_realize(DeviceState *dev,
> Error **errp)
> * TODO: If slot isn't specified, plug this core into
> * an existing empty slot.
> */
> - slot = object_property_get_str(OBJECT(dev), CPU_CORE_SLOT_PROP,
> &local_err);
> - if (!slot) {
> - error_setg(errp, "slot property isn't set");
> - return;
> - }
> -
> - object_property_set_link(OBJECT(spapr), OBJECT(core), slot, &local_err);
> - g_free(slot);
> - if (local_err) {
> - error_propagate(errp, local_err);
> - return;
> - }
> -
> + /* probably should error out as 'core' should be specified
> + either by board or by user */
> object_child_foreach(OBJECT(dev), spapr_cpu_core_realize_child, errp);
> }
>
> diff --git a/include/hw/cpu/core.h b/include/hw/cpu/core.h
> index 2daa724..a627969 100644
> --- a/include/hw/cpu/core.h
> +++ b/include/hw/cpu/core.h
> @@ -22,9 +22,9 @@ typedef struct CPUCore {
> DeviceState parent_obj;
>
> /*< public >*/
> - char *slot;
> + int core;
> } CPUCore;
>
> -#define CPU_CORE_SLOT_PROP "slot"
> +#define CPU_CORE_ID_PROP "core"
>
> #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
