From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:58097) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1adV7j-00047F-5d for qemu-devel@nongnu.org; Tue, 08 Mar 2016 22:49:24 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1adV7f-0002J8-LO for qemu-devel@nongnu.org; Tue, 08 Mar 2016 22:49:23 -0500 Date: Wed, 9 Mar 2016 14:19:26 +1100 From: David Gibson Message-ID: <20160309031926.GL22546@voom.fritz.box> References: <1457443095-213125-1-git-send-email-imammedo@redhat.com> <1457443095-213125-3-git-send-email-imammedo@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="Uus2/pBTyv0r7Av1" Content-Disposition: inline In-Reply-To: <1457443095-213125-3-git-send-email-imammedo@redhat.com> Subject: Re: [Qemu-devel] [PATCH v2 2/5] spapr: convert slot name property to numeric core and links List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Igor Mammedov Cc: mjrosato@linux.vnet.ibm.com, agraf@suse.de, thuth@redhat.com, pkrempa@redhat.com, ehabkost@redhat.com, aik@ozlabs.ru, qemu-devel@nongnu.org, armbru@redhat.com, borntraeger@de.ibm.com, qemu-ppc@nongnu.org, bharata@linux.vnet.ibm.com, pbonzini@redhat.com, mdroth@linux.vnet.ibm.com, afaerber@suse.de --Uus2/pBTyv0r7Av1 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable 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. >=20 > Signed-off-by: Igor Mammedov > --- > 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(-) >=20 > 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" > =20 > -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 =3D CPU_CORE(obj); > + CPUCore *cc =3D CPU_CORE(obj); > + int64_t value =3D cc->core; > =20 > - return g_strdup(core->slot); > + visit_type_int(v, name, &value, errp); > } > =20 > -static void core_prop_set_slot(Object *obj, const char *val, Error **err= p) > +static void core_prop_set_core(Object *obj, Visitor *v, > + const char *name, void *opaque, > + Error **errp) > { > - CPUCore *core =3D CPU_CORE(obj); > - > - core->slot =3D g_strdup(val); > + CPUCore *cc =3D CPU_CORE(obj); > + Error *local_err =3D NULL; > + int64_t value; > + > + visit_type_int(v, name, &value, &local_err); > + if (local_err) { > + error_propagate(errp, local_err); > + return; > + } > + cc->core =3D value; > } > =20 > static void cpu_core_instance_init(Object *obj) > { > - object_property_add_str(obj, "slot", core_prop_get_slot, core_prop_s= et_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 =3D { > 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(MachineStat= e *machine, Error **errp) > } > } > =20 > -/* > - * Check to see if core is being hot-plugged into an already populated s= lot. > - */ > -static void spapr_cpu_core_allow_set_link(Object *obj, const char *name, > - Object *val, Error **errp) > -{ > - Object *core =3D 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 =3D 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 =3D g_new0(Object *, spapr_max_cores); > =20 > for (i =3D 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_PR= OP, i); > - object_property_add_link(OBJECT(spapr), name, TYPE_SPAPR_CPU_COR= E, > - (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 =3D 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_fata= l); > } > 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 *opa= que) > static void spapr_core_release(DeviceState *dev, void *opaque) > { > struct sPAPRCPUUnplugList unplug_list; > - sPAPRMachineState *spapr =3D SPAPR_MACHINE(qdev_get_machine()); > sPAPRCPUCore *core =3D SPAPR_CPU_CORE(OBJECT(dev)); > - char *slot =3D object_property_get_str(OBJECT(dev), CPU_CORE_SLOT_PR= OP, > - &error_fatal); > =20 > QLIST_INIT(&unplug_list); > object_child_foreach(OBJECT(dev), spapr_cpu_release, &unplug_list); > spapr_cpu_core_cleanup(&unplug_list); > =20 > - /* 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 =3D SPAPR_CPU_CORE(OBJECT(dev)); > - sPAPRMachineState *spapr =3D SPAPR_MACHINE(qdev_get_machine()); > - char *slot; > - Error *local_err =3D NULL; > =20 > 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 =3D object_property_get_str(OBJECT(dev), CPU_CORE_SLOT_PROP, &l= ocal_err); > - if (!slot) { > - error_setg(errp, "slot property isn't set"); > - return; > - } > - > - object_property_set_link(OBJECT(spapr), OBJECT(core), slot, &local_e= rr); > - 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= ); > } > =20 > 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; > =20 > /*< public >*/ > - char *slot; > + int core; > } CPUCore; > =20 > -#define CPU_CORE_SLOT_PROP "slot" > +#define CPU_CORE_ID_PROP "core" > =20 > #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 --Uus2/pBTyv0r7Av1 Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJW35Y+AAoJEGw4ysog2bOSENwP/0QNkbo9aMdlR0lM58LQB5BY BX7jd1CrZCwUeOGnQFlsuqXqjf6vZywYZSf538rDChYSvaVvgPcaOXw0KpGEctfE /wP6ApTT3P3Zrly4ri8MCnsassMtPXcKue6qq5Pn+qICGIIjRYGZ+SVro/+vurB8 PeAiFYs10WCntnjgPt5/vL07qVSnAZMiO/8b4cJckmwau3EB65Yb5p8ErNyglyjA PhETDA9/gtyDx3ZrCZNucmY7sbguP8IPnxinK3kyx2/t9SG5qhHB5TBCF75fxJLq Yypa5/9xs7niDHclzO+B8GPI6fA4O1rPS4ErNVrAoHcTL4fe1WiJt6n74Lc4794N 1N6qpld7oIfu2pqbhZc0XvYnTXYwJHIKp+J7H44eSh0FjJZzwaQm8/dCsOgTmKIS 9OuprdhSRReALLd9LH0Z5OUa/9wV4oJBQ5G9qvR7qKop6Er/aq18JE0VPc++8zMJ CO8C+LtUG80yGurHRHy8jzvmskdia6BjYPFIwyekGqcRFXAMMm4xxrdXbm1NqVcY YIgSQIH+89IvvNrIJOKVXBsHkIlTyYyOnxzKx26Ksk3N20lB4uWRWKSOKvxH+yLm zTC6Iw/NFGlR+eYvmGf1KUXp0BgSiqshhnzhTyH+nt/VahiLwB4NEtDClxPF+eyd hqG0ECQ4gB1Wv1v8JcHZ =+CoQ -----END PGP SIGNATURE----- --Uus2/pBTyv0r7Av1--