From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:48939) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a3Zxy-00074D-AB for qemu-devel@nongnu.org; Mon, 30 Nov 2015 20:42:52 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1a3Zxw-0001Ay-8t for qemu-devel@nongnu.org; Mon, 30 Nov 2015 20:42:50 -0500 Date: Tue, 1 Dec 2015 12:30:12 +1100 From: David Gibson Message-ID: <20151201013012.GG31343@voom.redhat.com> References: <1448024079-20808-1-git-send-email-bharata@linux.vnet.ibm.com> <1448024079-20808-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="Fnm8lRGFTVS/3GuM" Content-Disposition: inline In-Reply-To: <1448024079-20808-9-git-send-email-bharata@linux.vnet.ibm.com> Subject: Re: [Qemu-devel] [PATCH v5 08/10] spapr: CPU hotplug support List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Bharata B Rao Cc: mdroth@linux.vnet.ibm.com, aik@ozlabs.ru, agraf@suse.de, qemu-devel@nongnu.org, pbonzini@redhat.com, qemu-ppc@nongnu.org, tyreld@linux.vnet.ibm.com, nfont@linux.vnet.ibm.com, imammedo@redhat.com, afaerber@suse.de --Fnm8lRGFTVS/3GuM Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Fri, Nov 20, 2015 at 06:24:37PM +0530, Bharata B Rao wrote: > Support CPU hotplug via device-add command. Set up device tree > entries for the hotplugged CPU core and use the exising EPOW event > infrastructure to send CPU hotplug notification to the guest. >=20 > Create only cores explicitly from boot path as well as hotplug path > and let the ->plug() handler of the core create the threads of the core. >=20 > Also support cold plugged CPUs that are specified by -device option > on cmdline. >=20 > Signed-off-by: Bharata B Rao > --- > hw/ppc/spapr.c | 156 ++++++++++++++++++++++++++++++++++++++= +++++- > hw/ppc/spapr_events.c | 3 + > hw/ppc/spapr_rtas.c | 24 +++++++ > target-ppc/translate_init.c | 8 +++ > 4 files changed, 189 insertions(+), 2 deletions(-) >=20 > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c > index 814b0a6..4434d45 100644 > --- a/hw/ppc/spapr.c > +++ b/hw/ppc/spapr.c > @@ -596,6 +596,18 @@ static void spapr_populate_cpu_dt(CPUState *cs, void= *fdt, int offset, > size_t page_sizes_prop_size; > uint32_t vcpus_per_socket =3D smp_threads * smp_cores; > uint32_t pft_size_prop[] =3D {0, cpu_to_be32(spapr->htab_shift)}; > + sPAPRMachineClass *smc =3D SPAPR_MACHINE_GET_CLASS(qdev_get_machine(= )); > + sPAPRDRConnector *drc; > + sPAPRDRConnectorClass *drck; > + int drc_index; > + > + if (smc->dr_cpu_enabled) { > + drc =3D spapr_dr_connector_by_id(SPAPR_DR_CONNECTOR_TYPE_CPU, in= dex); > + g_assert(drc); > + drck =3D SPAPR_DR_CONNECTOR_GET_CLASS(drc); > + drc_index =3D drck->get_index(drc); > + _FDT((fdt_setprop_cell(fdt, offset, "ibm,my-drc-index", drc_inde= x))); > + } > =20 > /* Note: we keep CI large pages off for now because a 64K capable gu= est > * provisioned with large pages might otherwise try to map a qemu > @@ -1739,6 +1751,7 @@ static void ppc_spapr_init(MachineState *machine) > char *filename; > int smt =3D kvmppc_smt_threads(); > int smp_max_cores =3D DIV_ROUND_UP(max_cpus, smp_threads); > + int spapr_smp_cores =3D DIV_ROUND_UP(smp_cpus, smp_threads); > =20 > msi_supported =3D true; > =20 > @@ -1818,7 +1831,7 @@ 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++) { > + for (i =3D 0; i < spapr_smp_cores; i++) { > cpu =3D cpu_ppc_init(machine->cpu_model); > if (cpu =3D=3D NULL) { > fprintf(stderr, "Unable to find PowerPC CPU definition\n"); > @@ -2207,10 +2220,135 @@ out: > error_propagate(errp, local_err); > } > =20 > +static void *spapr_populate_hotplug_cpu_dt(DeviceState *dev, CPUState *c= s, > + int *fdt_offset, > + sPAPRMachineState *spapr) > +{ > + PowerPCCPU *cpu =3D POWERPC_CPU(cs); > + DeviceClass *dc =3D DEVICE_GET_CLASS(cs); > + int id =3D ppc_get_vcpu_dt_id(cpu); > + void *fdt; > + int offset, fdt_size; > + char *nodename; > + > + fdt =3D create_device_tree(&fdt_size); > + nodename =3D g_strdup_printf("%s@%x", dc->fw_name, id); > + offset =3D fdt_add_subnode(fdt, 0, nodename); > + > + spapr_populate_cpu_dt(cs, fdt, offset, spapr); > + g_free(nodename); > + > + *fdt_offset =3D offset; > + return fdt; > +} > + > +static void spapr_cpu_plug(HotplugHandler *hotplug_dev, DeviceState *dev, > + Error **errp) > +{ > + sPAPRMachineClass *smc =3D SPAPR_MACHINE_GET_CLASS(qdev_get_machine(= )); > + sPAPRMachineState *ms =3D SPAPR_MACHINE(hotplug_dev); > + CPUState *cs =3D CPU(dev); > + PowerPCCPU *cpu =3D POWERPC_CPU(cs); > + int id =3D ppc_get_vcpu_dt_id(cpu); > + sPAPRDRConnector *drc =3D > + spapr_dr_connector_by_id(SPAPR_DR_CONNECTOR_TYPE_CPU, id); > + sPAPRDRConnectorClass *drck; > + int smt =3D kvmppc_smt_threads(); > + Error *local_err =3D NULL; > + void *fdt =3D NULL; > + int i, fdt_offset =3D 0; > + > + /* Set NUMA node for the added CPUs */ > + for (i =3D 0; i < nb_numa_nodes; i++) { > + if (test_bit(cs->cpu_index, numa_info[i].node_cpu)) { > + cs->numa_node =3D i; > + break; > + } > + } > + > + /* > + * Currently CPU core and threads of a core aren't really different > + * from QEMU point of view since all of them are just CPU devices. H= ence > + * there is no separate realize routines for cores and threads. > + * We use the id check below to do things differently for cores and = threads. > + * > + * SMT threads return from here, only main thread (core) will > + * continue, create threads and signal hotplug event to the guest. > + */ > + if ((id % smt) !=3D 0) { > + return; > + } Hmm. It seems odd to me to have thread 0 of a core handle the initialization of the other threads, rather than creating an explicit Core QOM object and have that construct the cpu objects for all threads under it. It also causes a rather weird recursion of cpu_ppc_init() into cpu_ppc_init(), if I'm following the logic correctly. > + > + /* Create SMT threads of the core. */ > + for (i =3D 1; i < smp_threads; i++) { > + cpu =3D cpu_ppc_init(current_machine->cpu_model); > + if (!cpu) { > + error_report("Unable to find PowerPC CPU definition: %s", > + current_machine->cpu_model); > + exit(EXIT_FAILURE); > + } > + } > + > + if (!smc->dr_cpu_enabled) { > + /* > + * This is a cold plugged CPU but the machine doesn't support > + * DR. So skip the hotplug path ensuring that the CPU is brought > + * up online with out an associated DR connector. > + */ > + return; > + } > + > + g_assert(drc); > + > + /* > + * Setup CPU DT entries only for hotplugged CPUs. For boot time or > + * coldplugged CPUs DT entries are setup in spapr_finalize_fdt(). > + */ > + if (dev->hotplugged) { > + fdt =3D spapr_populate_hotplug_cpu_dt(dev, cs, &fdt_offset, ms); > + } > + > + drck =3D SPAPR_DR_CONNECTOR_GET_CLASS(drc); > + drck->attach(drc, dev, fdt, fdt_offset, !dev->hotplugged, &local_err= ); > + if (local_err) { > + g_free(fdt); > + error_propagate(errp, local_err); > + return; > + } > + > + /* > + * We send hotplug notification interrupt to the guest only in case > + * of hotplugged CPUs. > + */ > + if (dev->hotplugged) { > + spapr_hotplug_req_add_by_index(drc); > + } else { > + /* > + * HACK to support removal of hotplugged CPU after VM migration: > + * > + * Since we want to be able to hot-remove those coldplugged CPUs > + * started at boot time using -device option at the target VM, w= e set > + * the right allocation_state and isolation_state for them, whic= h for > + * the hotplugged CPUs would be set via RTAS calls done from the > + * guest during hotplug. > + * > + * This allows the coldplugged CPUs started using -device option= to > + * have the right isolation and allocation states as expected by= the > + * CPU hot removal code. > + * > + * This hack will be removed once we have DRC states migrated as= part > + * of VM migration. > + */ > + drck->set_allocation_state(drc, SPAPR_DR_ALLOCATION_STATE_USABLE= ); > + drck->set_isolation_state(drc, SPAPR_DR_ISOLATION_STATE_UNISOLAT= ED); > + } > +} > + > static void spapr_machine_device_plug(HotplugHandler *hotplug_dev, > DeviceState *dev, Error **errp) > { > sPAPRMachineClass *smc =3D SPAPR_MACHINE_GET_CLASS(qdev_get_machine(= )); > + sPAPRMachineState *ms =3D SPAPR_MACHINE(hotplug_dev); > =20 > if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) { > int node; > @@ -2247,6 +2385,19 @@ static void spapr_machine_device_plug(HotplugHandl= er *hotplug_dev, > } > =20 > spapr_memory_plug(hotplug_dev, dev, node, errp); > + } else if (object_dynamic_cast(OBJECT(dev), TYPE_CPU)) { > + CPUState *cs =3D CPU(dev); > + PowerPCCPU *cpu =3D POWERPC_CPU(cs); > + > + if (!smc->dr_cpu_enabled && dev->hotplugged) { > + error_setg(errp, "CPU hotplug not supported for this machine= "); > + cpu_remove_sync(cs); > + return; > + } > + > + spapr_cpu_init(ms, cpu); > + spapr_cpu_reset(cpu); > + spapr_cpu_plug(hotplug_dev, dev, errp); > } > } > =20 > @@ -2261,7 +2412,8 @@ static void spapr_machine_device_unplug(HotplugHand= ler *hotplug_dev, > 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_CPU)) { > return HOTPLUG_HANDLER(machine); > } > return NULL; > diff --git a/hw/ppc/spapr_events.c b/hw/ppc/spapr_events.c > index 744ea62..1063036 100644 > --- a/hw/ppc/spapr_events.c > +++ b/hw/ppc/spapr_events.c > @@ -436,6 +436,9 @@ static void spapr_hotplug_req_event(uint8_t hp_id, ui= nt8_t hp_action, > case SPAPR_DR_CONNECTOR_TYPE_LMB: > hp->hotplug_type =3D RTAS_LOG_V6_HP_TYPE_MEMORY; > break; > + case SPAPR_DR_CONNECTOR_TYPE_CPU: > + hp->hotplug_type =3D RTAS_LOG_V6_HP_TYPE_CPU; > + break; > default: > /* we shouldn't be signaling hotplug events for resources > * that don't support them > diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c > index 34b12a3..7baa862 100644 > --- a/hw/ppc/spapr_rtas.c > +++ b/hw/ppc/spapr_rtas.c > @@ -33,6 +33,7 @@ > =20 > #include "hw/ppc/spapr.h" > #include "hw/ppc/spapr_vio.h" > +#include "hw/ppc/ppc.h" > #include "qapi-event.h" > #include "hw/boards.h" > =20 > @@ -159,6 +160,27 @@ static void rtas_query_cpu_stopped_state(PowerPCCPU = *cpu_, > rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR); > } > =20 > +/* > + * Set the timebase offset of the CPU to that of first CPU. > + * This helps hotplugged CPU to have the correct timebase offset. > + */ > +static void spapr_cpu_update_tb_offset(PowerPCCPU *cpu) > +{ > + PowerPCCPU *fcpu =3D POWERPC_CPU(first_cpu); > + > + cpu->env.tb_env->tb_offset =3D fcpu->env.tb_env->tb_offset; > +} > + > +static void spapr_cpu_set_endianness(PowerPCCPU *cpu) > +{ > + PowerPCCPU *fcpu =3D POWERPC_CPU(first_cpu); > + PowerPCCPUClass *pcc =3D POWERPC_CPU_GET_CLASS(fcpu); > + > + if (!pcc->interrupts_big_endian(fcpu)) { > + cpu->env.spr[SPR_LPCR] |=3D LPCR_ILE; > + } > +} > + > static void rtas_start_cpu(PowerPCCPU *cpu_, sPAPRMachineState *spapr, > uint32_t token, uint32_t nargs, > target_ulong args, > @@ -195,6 +217,8 @@ static void rtas_start_cpu(PowerPCCPU *cpu_, sPAPRMac= hineState *spapr, > env->nip =3D start; > env->gpr[3] =3D r3; > cs->halted =3D 0; > + spapr_cpu_set_endianness(cpu); > + spapr_cpu_update_tb_offset(cpu); > =20 > qemu_cpu_kick(cs); > =20 > diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c > index e88dc7f..245d73a 100644 > --- a/target-ppc/translate_init.c > +++ b/target-ppc/translate_init.c > @@ -30,6 +30,9 @@ > #include "qemu/error-report.h" > #include "qapi/visitor.h" > #include "hw/qdev-properties.h" > +#if !defined(CONFIG_USER_ONLY) > +#include "sysemu/sysemu.h" > +#endif > =20 > //#define PPC_DUMP_CPU > //#define PPC_DEBUG_SPR > @@ -8933,6 +8936,11 @@ static void ppc_cpu_realizefn(DeviceState *dev, Er= ror **errp) > } > =20 > #if !defined(CONFIG_USER_ONLY) > + if (cs->cpu_index >=3D max_cpus) { > + error_setg(errp, "Cannot have more than %d CPUs", max_cpus); > + return; > + } > + > cpu->cpu_dt_id =3D (cs->cpu_index / smp_threads) * max_smt > + (cs->cpu_index % smp_threads); > #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 --Fnm8lRGFTVS/3GuM Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJWXPgkAAoJEGw4ysog2bOSFRwQANvbsWa+kuVhbAMQTKo9i61P gKg0tHu/C17UPR86jbZTIvWexiCisEh5OKwQn3j78Qy/vcYmrB2gCZur6YKYsu2w hDRdDOEIloC7Owh/KawHuCiD9b+A54bs/nR7yZjrADv+j5VZXDP5chmvaaJ/bP1V Rn69+/gYFT0wkxjDCB67pNWpwzwiL5/o0C3kJeAKiAS1bWFC4M/Tesv7Q9C0tOJo g3UU6seNg0fwcadYRvMliFh1y5gskkR6AATLFJAVkHseYNRKytZkc7Bf3ezYC0Ng sqlvs8ziT7EllJ+2sdjGhqVU6H2XTH5LmWIWjRGqLDZWQ7KD9mr2pNHktW5uBCy3 DxC/uAfi57vm5ueMwoCnCF9jB0gw/ijjEubhb1ODFN4sU4ggf80T1QDbxMuYhFVo LhFGwMJRdBSzv6yBL3PikRh4YAONWnrmgJMCa7fBrlYIR/LUZR/iHQhr1ESpEUqm WayE9s3VYYh573Jezb/YsWfPXPB3dH4TPG3ye/+l0y9tivAZmesav8VJYPr4+kD5 P8wF58VumfbQ036QvGoqTPnLhEZ815sukZgDx1V64ZHNasZ+Y5dJPVAEc9/UvZQZ 7euRQcf8pQlPDM0hKd3qrw/hPxB2tmDCryq8sbdbdR/uiUUNCkojOp1KXJAetYJT nRrZPL2GFLAn70k80Ac5 =z+4+ -----END PGP SIGNATURE----- --Fnm8lRGFTVS/3GuM--