From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:57790) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bL4vi-0000k4-La for qemu-devel@nongnu.org; Thu, 07 Jul 2016 04:45:08 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bL4vd-0003XL-TL for qemu-devel@nongnu.org; Thu, 07 Jul 2016 04:45:06 -0400 Received: from mo179.mail-out.ovh.net ([178.32.228.179]:59149) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bL4vd-0003X8-H7 for qemu-devel@nongnu.org; Thu, 07 Jul 2016 04:45:01 -0400 Received: from player715.ha.ovh.net (b9.ovh.net [213.186.33.59]) by mo179.mail-out.ovh.net (Postfix) with ESMTP id 7D20D10085BB for ; Thu, 7 Jul 2016 10:44:55 +0200 (CEST) Date: Thu, 7 Jul 2016 10:44:39 +0200 From: Greg Kurz Message-ID: <20160707104439.098264e6@bahia.lan> In-Reply-To: <20160707020502.GL14675@voom.fritz.box> References: <146780713904.26232.12351310667258006783.stgit@bahia.lab.toulouse-stg.fr.ibm.com> <146780729482.26232.15157160444569956720.stgit@bahia.lab.toulouse-stg.fr.ibm.com> <20160707020502.GL14675@voom.fritz.box> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; boundary="Sig_/Dkn/NqlqVaJ73O_DQLC7VcT"; protocol="application/pgp-signature" Subject: Re: [Qemu-devel] [PATCH v3 7/7] spapr: consolidate the logic of core cpu_dt_id List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: David Gibson Cc: Eduardo Habkost , Benjamin Herrenschmidt , qemu-devel@nongnu.org, Alexander Graf , qemu-ppc@nongnu.org, Cedric Le Goater , Bharata B Rao , Scott Wood , Igor Mammedov --Sig_/Dkn/NqlqVaJ73O_DQLC7VcT Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: quoted-printable On Thu, 7 Jul 2016 12:05:02 +1000 David Gibson wrote: > On Wed, Jul 06, 2016 at 02:14:59PM +0200, Greg Kurz wrote: > > POWER5 and newer cpus from IBM have a specific numbering scheme for > > DT ids. This is currently open coded in several places. > >=20 > > This patch consolidates the logic in helpers. > >=20 > > Suggested-by: Bharata B Rao > > Signed-off-by: Greg Kurz =20 >=20 > This seems backwards to me. I think we should be constructing the > vcpu ids from the core id, not the other way around. Doing things Just to be sure: you're talking about the spapr_dt_id_to_*_index() helpers ? > that way will take a bit longer, but this seems like execessive work > as an interim cleanup until it's obsoleted by core based numbering. >=20 I agree this patch isn't needed anyway, especially if the numbering is abou= t to evolve. I'll drop this patch in v4. > > --- > > hw/ppc/spapr.c | 11 ++++------- > > hw/ppc/spapr_cpu_core.c | 26 +++++++++++++++++++------- > > include/hw/ppc/spapr_cpu_core.h | 9 +++++++++ > > 3 files changed, 32 insertions(+), 14 deletions(-) > >=20 > > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c > > index baefc7bd279c..89e61b976c60 100644 > > --- a/hw/ppc/spapr.c > > +++ b/hw/ppc/spapr.c > > @@ -199,7 +199,6 @@ static int spapr_fixup_cpu_dt(void *fdt, sPAPRMachi= neState *spapr) > > int ret =3D 0, offset, cpus_offset; > > CPUState *cs; > > char cpu_model[32]; > > - int smt =3D kvmppc_smt_threads(); > > uint32_t pft_size_prop[] =3D {0, cpu_to_be32(spapr->htab_shift)}; > > =20 > > CPU_FOREACH(cs) { > > @@ -207,7 +206,7 @@ static int spapr_fixup_cpu_dt(void *fdt, sPAPRMachi= neState *spapr) > > DeviceClass *dc =3D DEVICE_GET_CLASS(cs); > > int index =3D ppc_get_vcpu_dt_id(cpu); > > =20 > > - if ((index % smt) !=3D 0) { > > + if (!spapr_core_dt_id_is_valid(index)) { > > continue; > > } > > =20 > > @@ -735,7 +734,6 @@ static void spapr_populate_cpus_dt_node(void *fdt, = sPAPRMachineState *spapr) > > CPUState *cs; > > int cpus_offset; > > char *nodename; > > - int smt =3D kvmppc_smt_threads(); > > =20 > > cpus_offset =3D fdt_add_subnode(fdt, 0, "cpus"); > > _FDT(cpus_offset); > > @@ -753,7 +751,7 @@ static void spapr_populate_cpus_dt_node(void *fdt, = sPAPRMachineState *spapr) > > DeviceClass *dc =3D DEVICE_GET_CLASS(cs); > > int offset; > > =20 > > - if ((index % smt) !=3D 0) { > > + if (!spapr_core_dt_id_is_valid(index)) { > > continue; > > } > > =20 > > @@ -1822,7 +1820,7 @@ static void ppc_spapr_init(MachineState *machine) > > =20 > > spapr->cores =3D g_new0(Object *, spapr_max_cores); > > for (i =3D 0; i < spapr_max_cores; i++) { > > - int core_dt_id =3D i * smt; > > + int core_dt_id =3D spapr_core_index_to_dt_id(i); > > sPAPRDRConnector *drc =3D > > spapr_dr_connector_new(OBJECT(spapr), > > SPAPR_DR_CONNECTOR_TYPE_CPU, co= re_dt_id); > > @@ -2386,7 +2384,6 @@ static HotpluggableCPUList *spapr_query_hotplugga= ble_cpus(MachineState *machine) > > HotpluggableCPUList *head =3D NULL; > > sPAPRMachineState *spapr =3D SPAPR_MACHINE(machine); > > int spapr_max_cores =3D max_cpus / smp_threads; > > - int smt =3D kvmppc_smt_threads(); > > =20 > > for (i =3D 0; i < spapr_max_cores; i++) { > > HotpluggableCPUList *list_item =3D g_new0(typeof(*list_item), = 1); > > @@ -2396,7 +2393,7 @@ static HotpluggableCPUList *spapr_query_hotplugga= ble_cpus(MachineState *machine) > > cpu_item->type =3D spapr_get_cpu_core_type(machine->cpu_model); > > cpu_item->vcpus_count =3D smp_threads; > > cpu_props->has_core_id =3D true; > > - cpu_props->core_id =3D i * smt; > > + cpu_props->core_id =3D spapr_core_index_to_dt_id(i); > > /* TODO: add 'has_node/node' here to describe > > to which node core belongs */ > > =20 > > diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c > > index e3a3024baf32..b104778350df 100644 > > --- a/hw/ppc/spapr_cpu_core.c > > +++ b/hw/ppc/spapr_cpu_core.c > > @@ -103,7 +103,6 @@ static void spapr_core_release(DeviceState *dev, vo= id *opaque) > > size_t size =3D object_type_get_instance_size(typename); > > sPAPRMachineState *spapr =3D SPAPR_MACHINE(qdev_get_machine()); > > CPUCore *cc =3D CPU_CORE(dev); > > - int smt =3D kvmppc_smt_threads(); > > int i; > > =20 > > for (i =3D 0; i < cc->nr_threads; i++) { > > @@ -117,7 +116,7 @@ static void spapr_core_release(DeviceState *dev, vo= id *opaque) > > object_unparent(obj); > > } > > =20 > > - spapr->cores[cc->core_id / smt] =3D NULL; > > + spapr->cores[spapr_dt_id_to_core_index(cc->core_id)] =3D NULL; > > =20 > > g_free(sc->threads); > > object_unparent(OBJECT(dev)); > > @@ -160,10 +159,9 @@ void spapr_core_plug(HotplugHandler *hotplug_dev, = DeviceState *dev, > > void *fdt =3D NULL; > > int fdt_offset =3D 0; > > int index; > > - int smt =3D kvmppc_smt_threads(); > > =20 > > drc =3D spapr_dr_connector_by_id(SPAPR_DR_CONNECTOR_TYPE_CPU, cc->= core_id); > > - index =3D cc->core_id / smt; > > + index =3D spapr_dt_id_to_core_index(cc->core_id); > > spapr->cores[index] =3D OBJECT(dev); > > =20 > > if (!smc->dr_cpu_enabled) { > > @@ -217,7 +215,6 @@ void spapr_core_pre_plug(HotplugHandler *hotplug_de= v, DeviceState *dev, > > sPAPRMachineState *spapr =3D SPAPR_MACHINE(OBJECT(hotplug_dev)); > > int spapr_max_cores =3D max_cpus / smp_threads; > > int index; > > - int smt =3D kvmppc_smt_threads(); > > Error *local_err =3D NULL; > > CPUCore *cc =3D CPU_CORE(dev); > > char *base_core_type =3D spapr_get_cpu_core_type(machine->cpu_mode= l); > > @@ -238,12 +235,12 @@ void spapr_core_pre_plug(HotplugHandler *hotplug_= dev, DeviceState *dev, > > goto out; > > } > > =20 > > - if (cc->core_id % smt) { > > + if (!spapr_core_dt_id_is_valid(cc->core_id)) { > > error_setg(&local_err, "invalid core id %d\n", cc->core_id); > > goto out; > > } > > =20 > > - index =3D cc->core_id / smt; > > + index =3D spapr_dt_id_to_core_index(cc->core_id); > > if (index < 0 || index >=3D spapr_max_cores) { > > error_setg(&local_err, "core id %d out of range", cc->core_id); > > goto out; > > @@ -331,6 +328,21 @@ static void spapr_cpu_core_class_init(ObjectClass = *oc, void *data) > > dc->realize =3D spapr_cpu_core_realize; > > } > > =20 > > +unsigned spapr_core_index_to_dt_id(unsigned index) > > +{ > > + return index * kvmppc_smt_threads(); > > +} > > + > > +unsigned spapr_dt_id_to_core_index(unsigned dt_id) > > +{ > > + return dt_id / kvmppc_smt_threads(); > > +} > > + > > +unsigned spapr_dt_id_to_thread_index(unsigned dt_id) > > +{ > > + return dt_id % kvmppc_smt_threads(); > > +} > > + > > /* > > * instance_init routines from different flavours of sPAPR CPU cores. > > */ > > diff --git a/include/hw/ppc/spapr_cpu_core.h b/include/hw/ppc/spapr_cpu= _core.h > > index 1c9b3195cce9..be7be91308ce 100644 > > --- a/include/hw/ppc/spapr_cpu_core.h > > +++ b/include/hw/ppc/spapr_cpu_core.h > > @@ -33,4 +33,13 @@ void spapr_core_plug(HotplugHandler *hotplug_dev, De= viceState *dev, > > Error **errp); > > void spapr_core_unplug(HotplugHandler *hotplug_dev, DeviceState *dev, > > Error **errp); > > + > > +unsigned spapr_core_index_to_dt_id(unsigned index); > > +unsigned spapr_dt_id_to_core_index(unsigned dt_id); > > +unsigned spapr_dt_id_to_thread_index(unsigned dt_id); > > + > > +static inline bool spapr_core_dt_id_is_valid(unsigned dt_id) > > +{ > > + return spapr_dt_id_to_thread_index(dt_id) =3D=3D 0; > > +} > > #endif > > =20 >=20 --Sig_/Dkn/NqlqVaJ73O_DQLC7VcT Content-Type: application/pgp-signature Content-Description: OpenPGP digital signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iEYEARECAAYFAld+FncACgkQAvw66wEB28LYlQCeNZ0ElT/IMZbYPW9ET+Ny84Zv 3lEAoKa5gtd2Lgd+jHwD8ML1CDDt/l2P =05F4 -----END PGP SIGNATURE----- --Sig_/Dkn/NqlqVaJ73O_DQLC7VcT--