From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:35749) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1b8hgw-0005gX-P7 for qemu-devel@nongnu.org; Fri, 03 Jun 2016 01:30:44 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1b8hgu-0003Ep-En for qemu-devel@nongnu.org; Fri, 03 Jun 2016 01:30:42 -0400 Date: Fri, 3 Jun 2016 15:32:10 +1000 From: David Gibson Message-ID: <20160603053210.GK1087@voom.fritz.box> References: <1463024905-28401-1-git-send-email-bharata@linux.vnet.ibm.com> <1463024905-28401-9-git-send-email-bharata@linux.vnet.ibm.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="VJJoKLVEFXdmHQwR" Content-Disposition: inline In-Reply-To: <1463024905-28401-9-git-send-email-bharata@linux.vnet.ibm.com> Subject: Re: [Qemu-devel] [for-2.7 PATCH v3 08/15] spapr: convert boot CPUs into CPU core devices List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Bharata B Rao Cc: qemu-devel@nongnu.org, qemu-ppc@nongnu.org, afaerber@suse.de, imammedo@redhat.com, armbru@redhat.com, thuth@redhat.com, aik@ozlabs.ru, agraf@suse.de, pbonzini@redhat.com, ehabkost@redhat.com, pkrempa@redhat.com, mdroth@linux.vnet.ibm.com, eblake@redhat.com, mjrosato@linux.vnet.ibm.com, borntraeger@de.ibm.com --VJJoKLVEFXdmHQwR Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Thu, May 12, 2016 at 09:18:18AM +0530, Bharata B Rao wrote: > Introduce sPAPRMachineClass.dr_cpu_enabled to indicate support for > CPU core hotplug. Initialize boot time CPUs as core deivces and prevent > topologies that result in partially filled cores. Both of these are done > only if CPU core hotplug is supported. >=20 > Note: An unrelated change in the call to xics_system_init() is done > in this patch as it makes sense to use the local variable smt introduced > in this patch instead of kvmppc_smt_threads() call here. >=20 > TODO: We derive sPAPR core type by looking at -cpu . However > we don't take care of "compat=3D" feature yet for boot time as well > as hotplug CPUs. >=20 > Signed-off-by: Bharata B Rao This will need some tweaking for the changes I made to earlier patches, otherwise only a couple of tiny nits. Reviewed-by: David Gibson > --- > hw/ppc/spapr.c | 76 +++++++++++++++++++++++++++++++++++= ------ > hw/ppc/spapr_cpu_core.c | 58 +++++++++++++++++++++++++++++++ > include/hw/ppc/spapr.h | 2 ++ > include/hw/ppc/spapr_cpu_core.h | 3 ++ > 4 files changed, 129 insertions(+), 10 deletions(-) >=20 > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c > index 95db047..0f64218 100644 > --- a/hw/ppc/spapr.c > +++ b/hw/ppc/spapr.c > @@ -65,6 +65,7 @@ > =20 > #include "hw/compat.h" > #include "qemu/cutils.h" > +#include "hw/ppc/spapr_cpu_core.h" > =20 > #include > =20 > @@ -1605,6 +1606,10 @@ static void spapr_boot_set(void *opaque, const cha= r *boot_device, > machine->boot_order =3D g_strdup(boot_device); > } > =20 > +/* > + * TODO: Check if some of these can be moved to rtas_start_cpu() where > + * a few other things required for hotplugged CPUs are being done. > + */ > void spapr_cpu_init(sPAPRMachineState *spapr, PowerPCCPU *cpu, Error **e= rrp) > { > CPUPPCState *env =3D &cpu->env; > @@ -1628,6 +1633,7 @@ void spapr_cpu_init(sPAPRMachineState *spapr, Power= PCCPU *cpu, Error **errp) > xics_cpu_setup(spapr->icp, cpu); > =20 > qemu_register_reset(spapr_cpu_reset, cpu); > + spapr_cpu_reset(cpu); > } > =20 > /* > @@ -1711,7 +1717,6 @@ static void ppc_spapr_init(MachineState *machine) > const char *kernel_filename =3D machine->kernel_filename; > const char *kernel_cmdline =3D machine->kernel_cmdline; > const char *initrd_filename =3D machine->initrd_filename; > - PowerPCCPU *cpu; > PCIHostState *phb; > int i; > MemoryRegion *sysmem =3D get_system_memory(); > @@ -1725,6 +1730,22 @@ static void ppc_spapr_init(MachineState *machine) > long load_limit, fw_size; > bool kernel_le =3D false; > char *filename; > + int smt =3D kvmppc_smt_threads(); > + int spapr_cores =3D smp_cpus / smp_threads; > + int spapr_max_cores =3D max_cpus / smp_threads; > + > + if (smc->dr_cpu_enabled) { > + if (smp_cpus % smp_threads) { > + error_report("smp_cpus (%u) must be multiple of threads (%u)= ", > + smp_cpus, smp_threads); > + exit(1); > + } > + if (max_cpus % smp_threads) { > + error_report("max_cpus (%u) must be multiple of threads (%u)= ", > + max_cpus, smp_threads); > + exit(1); > + } > + } > =20 > msi_nonbroken =3D true; > =20 > @@ -1771,8 +1792,7 @@ static void ppc_spapr_init(MachineState *machine) > =20 > /* Set up Interrupt Controller before we create the VCPUs */ > spapr->icp =3D xics_system_init(machine, > - DIV_ROUND_UP(max_cpus * kvmppc_smt_thr= eads(), > - smp_threads), > + DIV_ROUND_UP(max_cpus * smt, smp_threa= ds), > XICS_IRQS, &error_fatal); > =20 > if (smc->dr_lmb_enabled) { > @@ -1783,13 +1803,37 @@ static void ppc_spapr_init(MachineState *machine) > if (machine->cpu_model =3D=3D NULL) { > machine->cpu_model =3D kvm_enabled() ? "host" : "POWER7"; > } > - for (i =3D 0; i < smp_cpus; i++) { > - cpu =3D cpu_ppc_init(machine->cpu_model); > - if (cpu =3D=3D NULL) { > - error_report("Unable to find PowerPC CPU definition"); > - exit(1); > + > + if (smc->dr_cpu_enabled) { > + spapr->cores =3D g_new0(Object *, spapr_max_cores); > + > + for (i =3D 0; i < spapr_cores; i++) { > + int core_dt_id =3D i * smt; > + char *type =3D spapr_get_cpu_core_type(machine->cpu_model); Probably makes sense to move this lookup outside the loop. > + Object *core; > + > + if (!object_class_by_name(type)) { > + error_report("Unable to find sPAPR CPU Core definition"); > + exit(1); > + } > + > + core =3D object_new(type); > + g_free(type); > + object_property_set_int(core, smp_threads, "threads", > + &error_fatal); > + object_property_set_int(core, core_dt_id, CPU_CORE_PROP_CORE, > + &error_fatal); > + object_property_set_bool(core, true, "realized", &error_fata= l); > } > - spapr_cpu_init(spapr, cpu, &error_fatal); > + } else { > + for (i =3D 0; i < smp_cpus; i++) { > + PowerPCCPU *cpu =3D cpu_ppc_init(machine->cpu_model); > + if (cpu =3D=3D NULL) { > + error_report("Unable to find PowerPC CPU definition"); > + exit(1); > + } > + spapr_cpu_init(spapr, cpu, &error_fatal); > + } > } > =20 > if (kvm_enabled()) { > @@ -2245,10 +2289,19 @@ static void spapr_machine_device_unplug(HotplugHa= ndler *hotplug_dev, > } > } > =20 > +static void spapr_machine_device_pre_plug(HotplugHandler *hotplug_dev, > + DeviceState *dev, Error **errp) > +{ > + if (object_dynamic_cast(OBJECT(dev), TYPE_SPAPR_CPU_CORE)) { > + spapr_core_pre_plug(hotplug_dev, dev, errp); > + } > +} > + > static HotplugHandler *spapr_get_hotpug_handler(MachineState *machine, > DeviceState *dev) > { > - if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) { > + if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM) || > + object_dynamic_cast(OBJECT(dev), TYPE_SPAPR_CPU_CORE)) { > return HOTPLUG_HANDLER(machine); > } > return NULL; > @@ -2287,11 +2340,13 @@ static void spapr_machine_class_init(ObjectClass = *oc, void *data) > mc->has_dynamic_sysbus =3D true; > mc->pci_allow_0_address =3D true; > mc->get_hotplug_handler =3D spapr_get_hotpug_handler; > + hc->pre_plug =3D spapr_machine_device_pre_plug; > 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; > =20 > smc->dr_lmb_enabled =3D true; > + smc->dr_cpu_enabled =3D true; > fwc->get_dev_path =3D spapr_get_fw_dev_path; > nc->nmi_monitor_handler =3D spapr_nmi; > } > @@ -2376,6 +2431,7 @@ static void spapr_machine_2_5_class_options(Machine= Class *mc) > =20 > spapr_machine_2_6_class_options(mc); > smc->use_ohci_by_default =3D true; > + smc->dr_cpu_enabled =3D false; > SET_MACHINE_COMPAT(mc, SPAPR_COMPAT_2_5); > } > =20 > diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c > index af63ed9..4450362 100644 > --- a/hw/ppc/spapr_cpu_core.c > +++ b/hw/ppc/spapr_cpu_core.c > @@ -14,6 +14,64 @@ > #include > #include "target-ppc/kvm_ppc.h" > =20 > +/* > + * Return the sPAPR CPU core type for @model which essentially is the CPU > + * model specified with -cpu cmdline option. > + */ > +char *spapr_get_cpu_core_type(const char *model) > +{ > + char core_type[32]; > + gchar **model_pieces =3D g_strsplit(model, ",", 2); > + > + snprintf(core_type, 32, "%s-%s", model_pieces[0], TYPE_SPAPR_CPU_COR= E); > + g_strfreev(model_pieces); > + return g_strdup(core_type); You could use g_strdup_printf() here. > +} > + > +void spapr_core_pre_plug(HotplugHandler *hotplug_dev, DeviceState *dev, > + Error **errp) > +{ > + MachineState *machine =3D MACHINE(OBJECT(hotplug_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); > + const char *type =3D object_get_typename(OBJECT(dev)); > + > + if (strcmp(base_core_type, type)) { > + error_setg(&local_err, "CPU core type should be %s", base_core_t= ype); > + goto out; > + } > + > + if (cc->threads !=3D smp_threads) { > + error_setg(&local_err, "threads must be %d", smp_threads); > + goto out; > + } > + > + if (cc->core % smt) { > + error_setg(&local_err, "invalid core id %d\n", cc->core); > + goto out; > + } > + > + index =3D cc->core / smt; > + if (index < 0 || index >=3D spapr_max_cores) { > + error_setg(&local_err, "core id %d out of range", cc->core); > + goto out; > + } > + > + if (spapr->cores[index]) { > + error_setg(&local_err, "core %d already populated", cc->core); > + goto out; > + } > + > +out: > + g_free(base_core_type); > + error_propagate(errp, local_err); > +} > + > static void spapr_cpu_core_create_threads(DeviceState *dev, int threads, > Error **errp) > { > diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h > index bcd9de6..1b3eeee 100644 > --- a/include/hw/ppc/spapr.h > +++ b/include/hw/ppc/spapr.h > @@ -36,6 +36,7 @@ struct sPAPRMachineClass { > =20 > /*< public >*/ > bool dr_lmb_enabled; /* enable dynamic-reconfig/hotplug of LMB= s */ > + bool dr_cpu_enabled; /* enable dynamic-reconfig/hotplug of CPU= s */ > bool use_ohci_by_default; /* use USB-OHCI instead of XHCI */ > }; > =20 > @@ -79,6 +80,7 @@ struct sPAPRMachineState { > /*< public >*/ > char *kvm_type; > MemoryHotplugState hotplug_memory; > + Object **cores; > }; > =20 > #define H_SUCCESS 0 > diff --git a/include/hw/ppc/spapr_cpu_core.h b/include/hw/ppc/spapr_cpu_c= ore.h > index b6aa39e..f1ebd85 100644 > --- a/include/hw/ppc/spapr_cpu_core.h > +++ b/include/hw/ppc/spapr_cpu_core.h > @@ -25,4 +25,7 @@ typedef struct sPAPRCPUCore { > ObjectClass *cpu; > } sPAPRCPUCore; > =20 > +void spapr_core_pre_plug(HotplugHandler *hotplug_dev, DeviceState *dev, > + Error **errp); > +char *spapr_get_cpu_core_type(const char *model); > #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 --VJJoKLVEFXdmHQwR Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJXURZaAAoJEGw4ysog2bOSoE8P/i3oj62W3QGt7m0I2diWtqde X1qgdUSil71dYlzIANwA7MUCfkPD76wuSjkB6gmj1s9vXDQM4I7qCcRY6KKmaZ5b LVsFHGvYuPc2vm/XY/QNRaPNJt20nKRzDM4OrbRHbxzpBhEkUPANxIKLPRQuqo6Z GD4zrEd//ND36KYH6gjWZti0/fLIK/ofU0233QQaqcZ+SJKyzJjL9w9NQyWq+mtt cChgacyZZT57au2qhL0vXgq2mRNDquL6KECg1uUFxTNd6CgZ0+Pi5gUyHy64Lxlb BnH3B8xi3yd1CoGs1IqsIqPbAFFJg1YONN26hPyGIqfCgCBXX/fQuNkTgARNipTE fst1LpVrbg1N57mtg0gG7f+dRi0qJoKOG2AmBd0yrNECOTCJhHF1pLUe8wRQ7FsG 6kWFNsY/FUWvUC32hav2EHbl7+nwsJIQfmaHJpUdB/jkrdvkT3ZbQim9scubSUXO +48ouVDnJepMF0Gwz5m+LpOKAakRiV2adDodJoxhaoChkPJQNraLvqY/HUPz3ZKe /cDXkTEw8idPxU1R4PpGDYhappP/8gk2RfEoIB6J7XjBJX2avMp/zbCeGpegzHGc PBuEKxPdTylGxz+8jVaB6DOqnJWt2lV+6W4LShMmSnhVQl7vSoZ+qNaygFfHnvBP 1gxogqLFulzB8cYMav9P =yXLZ -----END PGP SIGNATURE----- --VJJoKLVEFXdmHQwR--