From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:47691) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1chFeB-0004bz-0t for qemu-devel@nongnu.org; Fri, 24 Feb 2017 08:10:57 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1chFe6-0004ts-Uu for qemu-devel@nongnu.org; Fri, 24 Feb 2017 08:10:54 -0500 Date: Sat, 25 Feb 2017 00:03:57 +1100 From: David Gibson Message-ID: <20170224130357.GF17615@umbus.fritz.box> References: <20170222063348.32176-38-david@gibson.dropbear.id.au> <1487761555-93223-1-git-send-email-imammedo@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="L2zmpNdaDxUzilj2" Content-Disposition: inline In-Reply-To: <1487761555-93223-1-git-send-email-imammedo@redhat.com> Subject: Re: [Qemu-devel] [PATCH 37/43] spapr: reuse machine->possible_cpus instead of cores[] List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Igor Mammedov Cc: qemu-devel@nongnu.org, agraf@suse.de, thuth@redhat.com, lvivier@redhat.com, qemu-ppc@nongnu.org, mdroth@linux.vnet.ibm.com, peter.maydell@linaro.org --L2zmpNdaDxUzilj2 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Wed, Feb 22, 2017 at 12:05:55PM +0100, Igor Mammedov wrote: > Replace SPAPR specific cores[] array with generic > machine->possible_cpus and store core objects there. > It makes cores bookkeeping similar to x86 cpus and > will allow to unify similar code. > It would allow to replace cpu_index based NUMA node > mapping with property based one (for -device created > cores) since possible_cpus carries board defined > topology/layout. >=20 > Signed-off-by: Igor Mammedov > Acked-by: David Gibson > Signed-off-by: David Gibson Sorry Igor, Peter has applied the original version. Can you resend these updates as deltas against the applied versions of the patches. > --- > v3: > - drop "// TODO" comment as todo is completed in next patch anyway > fixes checkpatch error wrt // comment > v2: > - remove extra ';' at the end of expression (David Gibson) > --- > include/hw/ppc/spapr.h | 1 - > hw/ppc/spapr.c | 127 ++++++++++++++++++++++++++++++++++---------= ------ > 2 files changed, 89 insertions(+), 39 deletions(-) >=20 > diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h > index a2d8964..f9b17d8 100644 > --- a/include/hw/ppc/spapr.h > +++ b/include/hw/ppc/spapr.h > @@ -94,7 +94,6 @@ struct sPAPRMachineState { > /*< public >*/ > char *kvm_type; > MemoryHotplugState hotplug_memory; > - Object **cores; > }; > =20 > #define H_SUCCESS 0 > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c > index 37cb338..735f53d 100644 > --- a/hw/ppc/spapr.c > +++ b/hw/ppc/spapr.c > @@ -1751,13 +1751,28 @@ static void spapr_validate_node_memory(MachineSta= te *machine, Error **errp) > } > } > =20 > +/* find cpu slot in machine->possible_cpus by core_id */ > +static CPUArchId *spapr_find_cpu_slot(MachineState *ms, uint32_t id, int= *idx) > +{ > + int index =3D id / smp_threads; > + > + if (index >=3D ms->possible_cpus->len) { > + return NULL; > + } > + if (idx) { > + *idx =3D index; > + } > + return &ms->possible_cpus->cpus[index]; > +} > + > static void spapr_init_cpus(sPAPRMachineState *spapr) > { > MachineState *machine =3D MACHINE(spapr); > MachineClass *mc =3D MACHINE_GET_CLASS(machine); > char *type =3D spapr_get_cpu_core_type(machine->cpu_model); > int smt =3D kvmppc_smt_threads(); > - int spapr_max_cores, spapr_cores; > + const CPUArchIdList *possible_cpus; > + int boot_cores_nr =3D smp_cpus / smp_threads; > int i; > =20 > if (!type) { > @@ -1765,6 +1780,7 @@ static void spapr_init_cpus(sPAPRMachineState *spap= r) > exit(1); > } > =20 > + possible_cpus =3D mc->possible_cpu_arch_ids(machine); > if (mc->query_hotpluggable_cpus) { > if (smp_cpus % smp_threads) { > error_report("smp_cpus (%u) must be multiple of threads (%u)= ", > @@ -1776,21 +1792,15 @@ static void spapr_init_cpus(sPAPRMachineState *sp= apr) > max_cpus, smp_threads); > exit(1); > } > - > - spapr_max_cores =3D max_cpus / smp_threads; > - spapr_cores =3D smp_cpus / smp_threads; > } else { > if (max_cpus !=3D smp_cpus) { > error_report("This machine version does not support CPU hotp= lug"); > exit(1); > } > - > - spapr_max_cores =3D QEMU_ALIGN_UP(smp_cpus, smp_threads) / smp_t= hreads; > - spapr_cores =3D spapr_max_cores; > + boot_cores_nr =3D possible_cpus->len; > } > =20 > - spapr->cores =3D g_new0(Object *, spapr_max_cores); > - for (i =3D 0; i < spapr_max_cores; i++) { > + for (i =3D 0; i < possible_cpus->len; i++) { > int core_id =3D i * smp_threads; > =20 > if (mc->query_hotpluggable_cpus) { > @@ -1802,7 +1812,7 @@ static void spapr_init_cpus(sPAPRMachineState *spap= r) > qemu_register_reset(spapr_drc_reset, drc); > } > =20 > - if (i < spapr_cores) { > + if (i < boot_cores_nr) { > Object *core =3D object_new(type); > int nr_threads =3D smp_threads; > =20 > @@ -2491,10 +2501,11 @@ void *spapr_populate_hotplug_cpu_dt(CPUState *cs,= int *fdt_offset, > static void spapr_core_unplug(HotplugHandler *hotplug_dev, DeviceState *= dev, > Error **errp) > { > - sPAPRMachineState *spapr =3D SPAPR_MACHINE(qdev_get_machine()); > + MachineState *ms =3D MACHINE(qdev_get_machine()); > CPUCore *cc =3D CPU_CORE(dev); > + CPUArchId *core_slot =3D spapr_find_cpu_slot(ms, cc->core_id, NULL); > =20 > - spapr->cores[cc->core_id / smp_threads] =3D NULL; > + core_slot->cpu =3D NULL; > object_unparent(OBJECT(dev)); > } > =20 > @@ -2510,19 +2521,24 @@ static > void spapr_core_unplug_request(HotplugHandler *hotplug_dev, DeviceState = *dev, > Error **errp) > { > - CPUCore *cc =3D CPU_CORE(dev); > - int smt =3D kvmppc_smt_threads(); > - int index =3D cc->core_id / smp_threads; > - sPAPRDRConnector *drc =3D > - spapr_dr_connector_by_id(SPAPR_DR_CONNECTOR_TYPE_CPU, index * sm= t); > + int index; > + sPAPRDRConnector *drc; > sPAPRDRConnectorClass *drck; > Error *local_err =3D NULL; > + CPUCore *cc =3D CPU_CORE(dev); > + int smt =3D kvmppc_smt_threads(); > =20 > + if (!spapr_find_cpu_slot(MACHINE(hotplug_dev), cc->core_id, &index))= { > + error_setg(errp, "Unable to find CPU core with core-id: %d", > + cc->core_id); > + return; > + } > if (index =3D=3D 0) { > error_setg(errp, "Boot CPU core may not be unplugged"); > return; > } > =20 > + drc =3D spapr_dr_connector_by_id(SPAPR_DR_CONNECTOR_TYPE_CPU, index = * smt); > g_assert(drc); > =20 > drck =3D SPAPR_DR_CONNECTOR_GET_CLASS(drc); > @@ -2547,11 +2563,17 @@ static void spapr_core_plug(HotplugHandler *hotpl= ug_dev, DeviceState *dev, > Error *local_err =3D NULL; > void *fdt =3D NULL; > int fdt_offset =3D 0; > - int index =3D cc->core_id / smp_threads; > int smt =3D kvmppc_smt_threads(); > + CPUArchId *core_slot; > + int index; > =20 > + core_slot =3D spapr_find_cpu_slot(MACHINE(hotplug_dev), cc->core_id,= &index); > + if (!core_slot) { > + error_setg(errp, "Unable to find CPU core with core-id: %d", > + cc->core_id); > + return; > + } > drc =3D spapr_dr_connector_by_id(SPAPR_DR_CONNECTOR_TYPE_CPU, index = * smt); > - spapr->cores[index] =3D OBJECT(dev); > =20 > g_assert(drc || !mc->query_hotpluggable_cpus); > =20 > @@ -2568,7 +2590,6 @@ static void spapr_core_plug(HotplugHandler *hotplug= _dev, DeviceState *dev, > drck->attach(drc, dev, fdt, fdt_offset, !dev->hotplugged, &local= _err); > if (local_err) { > g_free(fdt); > - spapr->cores[index] =3D NULL; > error_propagate(errp, local_err); > return; > } > @@ -2590,6 +2611,7 @@ static void spapr_core_plug(HotplugHandler *hotplug= _dev, DeviceState *dev, > drck->set_isolation_state(drc, SPAPR_DR_ISOLATION_STATE_UNIS= OLATED); > } > } > + core_slot->cpu =3D OBJECT(dev); > } > =20 > static void spapr_core_pre_plug(HotplugHandler *hotplug_dev, DeviceState= *dev, > @@ -2597,13 +2619,12 @@ static void spapr_core_pre_plug(HotplugHandler *h= otplug_dev, DeviceState *dev, > { > MachineState *machine =3D MACHINE(OBJECT(hotplug_dev)); > MachineClass *mc =3D MACHINE_GET_CLASS(hotplug_dev); > - sPAPRMachineState *spapr =3D SPAPR_MACHINE(OBJECT(hotplug_dev)); > - int spapr_max_cores =3D max_cpus / smp_threads; > - int index; > Error *local_err =3D NULL; > CPUCore *cc =3D CPU_CORE(dev); > char *base_core_type =3D spapr_get_cpu_core_type(machine->cpu_model); > const char *type =3D object_get_typename(OBJECT(dev)); > + CPUArchId *core_slot; > + int index; > =20 > if (dev->hotplugged && !mc->query_hotpluggable_cpus) { > error_setg(&local_err, "CPU hotplug not supported for this machi= ne"); > @@ -2620,13 +2641,13 @@ static void spapr_core_pre_plug(HotplugHandler *h= otplug_dev, DeviceState *dev, > goto out; > } > =20 > - index =3D cc->core_id / smp_threads; > - if (index < 0 || index >=3D spapr_max_cores) { > + core_slot =3D spapr_find_cpu_slot(MACHINE(hotplug_dev), cc->core_id,= &index); > + if (!core_slot) { > error_setg(&local_err, "core id %d out of range", cc->core_id); > goto out; > } > =20 > - if (spapr->cores[index]) { > + if (core_slot->cpu) { > error_setg(&local_err, "core %d already populated", cc->core_id); > goto out; > } > @@ -2758,29 +2779,58 @@ static unsigned spapr_cpu_index_to_socket_id(unsi= gned cpu_index) > return cpu_index / smp_threads / smp_cores; > } > =20 > +static const CPUArchIdList *spapr_possible_cpu_arch_ids(MachineState *ma= chine) > +{ > + int i; > + int spapr_max_cores =3D max_cpus / smp_threads; > + MachineClass *mc =3D MACHINE_GET_CLASS(machine); > + > + if (!mc->query_hotpluggable_cpus) { > + spapr_max_cores =3D QEMU_ALIGN_UP(smp_cpus, smp_threads) / smp_t= hreads; > + } > + if (machine->possible_cpus) { > + assert(machine->possible_cpus->len =3D=3D spapr_max_cores); > + return machine->possible_cpus; > + } > + > + machine->possible_cpus =3D g_malloc0(sizeof(CPUArchIdList) + > + sizeof(CPUArchId) * spapr_max_cores); > + machine->possible_cpus->len =3D spapr_max_cores; > + for (i =3D 0; i < machine->possible_cpus->len; i++) { > + int core_id =3D i * smp_threads; > + > + machine->possible_cpus->cpus[i].arch_id =3D core_id; > + machine->possible_cpus->cpus[i].props.has_core_id =3D true; > + machine->possible_cpus->cpus[i].props.core_id =3D core_id; > + /* TODO: add 'has_node/node' here to describe > + to which node core belongs */ > + } > + return machine->possible_cpus; > +} > + > static HotpluggableCPUList *spapr_query_hotpluggable_cpus(MachineState *= machine) > { > int i; > + Object *cpu; > HotpluggableCPUList *head =3D NULL; > - sPAPRMachineState *spapr =3D SPAPR_MACHINE(machine); > - int spapr_max_cores =3D max_cpus / smp_threads; > + const char *cpu_type; > =20 > - for (i =3D 0; i < spapr_max_cores; i++) { > + cpu =3D machine->possible_cpus->cpus[0].cpu; > + assert(cpu); /* Boot cpu is always present */ > + cpu_type =3D object_get_typename(cpu); > + for (i =3D 0; i < machine->possible_cpus->len; i++) { > HotpluggableCPUList *list_item =3D g_new0(typeof(*list_item), 1); > HotpluggableCPU *cpu_item =3D g_new0(typeof(*cpu_item), 1); > - CpuInstanceProperties *cpu_props =3D g_new0(typeof(*cpu_props), = 1); > =20 > - cpu_item->type =3D spapr_get_cpu_core_type(machine->cpu_model); > + cpu_item->type =3D g_strdup(cpu_type); > cpu_item->vcpus_count =3D smp_threads; > - cpu_props->has_core_id =3D true; > - cpu_props->core_id =3D i * smp_threads; > - /* TODO: add 'has_node/node' here to describe > - to which node core belongs */ > + cpu_item->props =3D g_memdup(&machine->possible_cpus->cpus[i].pr= ops, > + sizeof(*cpu_item->props)); > =20 > - cpu_item->props =3D cpu_props; > - if (spapr->cores[i]) { > + cpu =3D machine->possible_cpus->cpus[i].cpu; > + if (cpu) { > cpu_item->has_qom_path =3D true; > - cpu_item->qom_path =3D object_get_canonical_path(spapr->core= s[i]); > + cpu_item->qom_path =3D object_get_canonical_path(cpu); > } > list_item->value =3D cpu_item; > list_item->next =3D head; > @@ -2872,6 +2922,7 @@ static void spapr_machine_class_init(ObjectClass *o= c, void *data) > hc->plug =3D spapr_machine_device_plug; > hc->unplug =3D spapr_machine_device_unplug; > mc->cpu_index_to_socket_id =3D spapr_cpu_index_to_socket_id; > + mc->possible_cpu_arch_ids =3D spapr_possible_cpu_arch_ids; > hc->unplug_request =3D spapr_machine_device_unplug_request; > =20 > smc->dr_lmb_enabled =3D true; --=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 --L2zmpNdaDxUzilj2 Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQIcBAEBCAAGBQJYsC86AAoJEGw4ysog2bOS0joQAKHlFBRdKY70JkyjbodJao8z N23obvwiP6xF0SVY+gD9AqBOvtDse/OYiQIVdWT8R8B5eXL/MvbvWoMcQ4NkeN6K 1wgxseaA/qYiBjtPZ3xEWx2u8YCsn7YvcgdmdG5SQ5dwY+a1tZp7OSUvlmBuVCns oHpFrJrCFAlWxuHFWxaqqffQAzzJ5RFgtgxzLqoKVUsK8PZ5dT8tKqDRG7cK+X+S peaQP4cCKFZCjeQkHbyRYeuLvdHDPSSOxN5a4ij412p8cASRFAVL4geiNTM5XDVQ nTjulZhnj20KcPOG5PSLwa39QO0VxSmd/o/aOrTc37Htgu5u+vhuiHSn7BUcg3vW k5is+EFdKbQF1Lnc3q7sIiC0Nz2a5oxEvcyg4TskrT2Ucy8MMHQcMloGHLKRe89z O53UBlRbtH9rFyU61EpzF9HNq6WEHzWGOcu1QCjYT2SaVwZRG+rn201T3xdo/vOu 1ELRNyBOKOkkJBpXzmCYDcofnvazoh8L26M6d17KK5GTBnmfOTX8FyzVMJTh+Kn4 xYNoh9WxBO8z2VSdlCOfOnNwmn4LJNzS6YszugCLywhlaM88dYiq4hOqZXnTG//6 VytMUgZmsDP+BMCXyy5ubgqI3d5Dgd/RiRvbYUMK4CxNyLEkFeOgZFdFWsDMc4aC 36rSVgXwoBWf99oje/ct =0rs6 -----END PGP SIGNATURE----- --L2zmpNdaDxUzilj2--