From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:41149) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bKyew-0007O8-CX for qemu-devel@nongnu.org; Wed, 06 Jul 2016 22:03:23 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bKyeu-0001vJ-LZ for qemu-devel@nongnu.org; Wed, 06 Jul 2016 22:03:22 -0400 Date: Thu, 7 Jul 2016 12:05:02 +1000 From: David Gibson Message-ID: <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> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="1hKfHPzOXWu1rh0v" Content-Disposition: inline In-Reply-To: <146780729482.26232.15157160444569956720.stgit@bahia.lab.toulouse-stg.fr.ibm.com> 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: Greg Kurz 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 --1hKfHPzOXWu1rh0v Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable 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 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 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. > --- > 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, sPAPRMachine= State *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, sPAPRMachine= State *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, sP= APRMachineState *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, sP= APRMachineState *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, core= _dt_id); > @@ -2386,7 +2384,6 @@ static HotpluggableCPUList *spapr_query_hotpluggabl= e_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_hotpluggabl= e_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, void= *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, void= *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, De= viceState *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->co= re_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_dev,= 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_model); > @@ -238,12 +235,12 @@ void spapr_core_pre_plug(HotplugHandler *hotplug_de= v, 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 *o= c, 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_c= ore.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, Devi= ceState *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 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 --1hKfHPzOXWu1rh0v Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJXfbjOAAoJEGw4ysog2bOSfTsP/RhofF3GhKfn3z8bA9mH8pEj g+jXKS0/d8U2jbDUqU//8lbW2izCQoWG8Xcuws8EtbwEb/u3xLpjD3dldVZppzvk mYnh06aEa6Fc7mninr5NuIlfbAt9UPm8SHDAsSRBKGoA0gSNFDlooNZQtFY0fnpH 6/tNyqQAwJtkOa6CM4DEo2FJ2qGb8ZZ9xaWOc/KFEGZqmE7121ATWqhQRzZ1D1QZ xt93M8czT6eaQtbuKQU7o6DFesEHiSA7S6ac5zVF+hQ3aXHhkIfE4Tf7H9s+ggrU lxvwHXkUYUQvfssuUzFmtGR0/5TaDhvfM1u117zXoqMnTJXk2xOELfCBj4Ismie5 9U86vyufDDzG2CBPI2o9Ah8Xotgl2guV4Jioh9fiPB14JfpC7itCVTYFo14Lnqzj Ar6ADxvwIQlF6UBdY4cIpa6dGNUkVCcLohpX49l9gqrsbpqjiuMOJwyAJfxaHRvV BSFl8OJhNfZ8x+NWktJE26dsOyd23h/f+CFt3M1Ga5XCu+BdshTgMVqx1bs4g9ju hHl3hxvWVR4fSQ8q+RMqEiqd/006xNaKOz3PnCO2N3YyQSHaCl+M3HaGEw5NWsNz uYWaFBa0ejzO/3yf3W3nKs5PHJO+al2riCN79zvquj35jnr7V6zNJ2wGUQUFJNBm 4Vc5MTkiqQGVA8Q3dVlN =CypO -----END PGP SIGNATURE----- --1hKfHPzOXWu1rh0v--