From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:39947) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YpXXX-0001mM-Pg for qemu-devel@nongnu.org; Tue, 05 May 2015 03:45:17 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YpXXQ-00032N-1R for qemu-devel@nongnu.org; Tue, 05 May 2015 03:45:15 -0400 Date: Tue, 5 May 2015 17:28:38 +1000 From: David Gibson Message-ID: <20150505072838.GQ14090@voom.redhat.com> References: <1429858066-12088-1-git-send-email-bharata@linux.vnet.ibm.com> <1429858066-12088-21-git-send-email-bharata@linux.vnet.ibm.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="Mh8CTEa8Ax54aLHp" Content-Disposition: inline In-Reply-To: <1429858066-12088-21-git-send-email-bharata@linux.vnet.ibm.com> Subject: Re: [Qemu-devel] [RFC PATCH v3 20/24] spapr: CPU hot unplug 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, qemu-ppc@nongnu.org, tyreld@linux.vnet.ibm.com, nfont@linux.vnet.ibm.com, imammedo@redhat.com, afaerber@suse.de --Mh8CTEa8Ax54aLHp Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Fri, Apr 24, 2015 at 12:17:42PM +0530, Bharata B Rao wrote: > Support hot removal of CPU for sPAPR guests by sending the hot unplug > notification to the guest via EPOW interrupt. Release the vCPU object > after CPU hot unplug so that vCPU fd can be parked and reused. >=20 > Signed-off-by: Bharata B Rao > --- > hw/ppc/spapr.c | 101 ++++++++++++++++++++++++++++++++++++++= +++++- > target-ppc/translate_init.c | 10 +++++ > 2 files changed, 110 insertions(+), 1 deletion(-) >=20 > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c > index 9b0701c..910a50f 100644 > --- a/hw/ppc/spapr.c > +++ b/hw/ppc/spapr.c > @@ -1481,6 +1481,12 @@ static void spapr_cpu_init(PowerPCCPU *cpu) > qemu_register_reset(spapr_cpu_reset, cpu); > } > =20 > +static void spapr_cpu_destroy(PowerPCCPU *cpu) > +{ > + xics_cpu_destroy(spapr->icp, cpu); > + qemu_unregister_reset(spapr_cpu_reset, cpu); > +} > + > /* pSeries LPAR / sPAPR hardware init */ > static void ppc_spapr_init(MachineState *machine) > { > @@ -1883,6 +1889,24 @@ static void *spapr_populate_hotplug_cpu_dt(DeviceS= tate *dev, CPUState *cs, > return fdt; > } > =20 > +static void spapr_cpu_release(DeviceState *dev, void *opaque) > +{ > + CPUState *cs; > + int i; > + int id =3D ppc_get_vcpu_dt_id(POWERPC_CPU(CPU(dev))); > + > + for (i =3D id; i < id + smp_threads; i++) { > + CPU_FOREACH(cs) { > + PowerPCCPU *cpu =3D POWERPC_CPU(cs); > + > + if (i =3D=3D ppc_get_vcpu_dt_id(cpu)) { > + spapr_cpu_destroy(cpu); > + cpu_remove(cs); > + } > + } > + } > +} > + > static void spapr_cpu_plug(HotplugHandler *hotplug_dev, DeviceState *dev, > Error **errp) > { > @@ -1970,6 +1994,59 @@ static void spapr_cpu_plug(HotplugHandler *hotplug= _dev, DeviceState *dev, > return; > } > =20 > +static int spapr_cpu_unplug(Object *obj, void *opaque) > +{ > + Error **errp =3D opaque; > + DeviceState *dev =3D DEVICE(obj); > + CPUState *cs =3D CPU(dev); > + PowerPCCPU *cpu =3D POWERPC_CPU(cs); > + int id =3D ppc_get_vcpu_dt_id(cpu); > + int smt =3D kvmppc_smt_threads(); > + sPAPRDRConnector *drc =3D > + spapr_dr_connector_by_id(SPAPR_DR_CONNECTOR_TYPE_CPU, id); > + sPAPRDRConnectorClass *drck; > + Error *local_err =3D NULL; > + > + /* > + * SMT threads return from here, only main thread (core) will > + * continue and signal hot unplug event to the guest. > + */ > + if ((id % smt) !=3D 0) { > + return 0; > + } As with the in-plug side couldn't this be done more naturally by attaching this function to the cpu core object rather than the thread? > + g_assert(drc); > + > + drck =3D SPAPR_DR_CONNECTOR_GET_CLASS(drc); > + drck->detach(drc, dev, spapr_cpu_release, NULL, &local_err); > + if (local_err) { > + error_propagate(errp, local_err); > + return -1; > + } > + > + /* > + * In addition to hotplugged CPUs, send the hot-unplug notification > + * interrupt to the guest for coldplugged CPUs started via -device > + * option too. > + */ > + spapr_hotplug_req_remove_event(drc); Um.. doesn't the remove notification need to go *before* the "physical" unplug? Along with a wait for acknowledgement from the guest? > + return 0; > +} > + > +static int spapr_cpu_core_unplug(Object *obj, void *opaque) > +{ > + Error **errp =3D opaque; > + > + object_child_foreach(obj, spapr_cpu_unplug, errp); > + return 0; > +} > + > +static void spapr_cpu_socket_unplug(HotplugHandler *hotplug_dev, > + DeviceState *dev, Error **errp) > +{ > + object_child_foreach(OBJECT(dev), spapr_cpu_core_unplug, errp); > +} > + > static void spapr_machine_device_plug(HotplugHandler *hotplug_dev, > DeviceState *dev, Error **errp) > { > @@ -1984,16 +2061,36 @@ static void spapr_machine_device_plug(HotplugHand= ler *hotplug_dev, > * Fail hotplug on machines where CPU DR isn't enabled. > */ > if (!spapr->dr_cpu_enabled && dev->hotplugged) { > + /* > + * FIXME: Ideally should fail hotplug here by doing an error= _setg, > + * but failing hotplug here doesn't work well with the vCPU = object > + * removal code. Hence silently refusing to add CPUs here. > + */ > + spapr_cpu_destroy(cpu); > + cpu_remove(cs); > return; > } > spapr_cpu_plug(hotplug_dev, dev, errp); > } > } > =20 > +static void spapr_machine_device_unplug(HotplugHandler *hotplug_dev, > + DeviceState *dev, Error **errp) > +{ > + if (object_dynamic_cast(OBJECT(dev), TYPE_CPU_SOCKET)) { > + if (!spapr->dr_cpu_enabled) { > + error_setg(errp, "CPU hot unplug not supported on this machi= ne"); > + return; > + } > + spapr_cpu_socket_unplug(hotplug_dev, dev, errp); > + } > +} > + > static HotplugHandler *spapr_get_hotpug_handler(MachineState *machine, > DeviceState *dev) > { > - if (object_dynamic_cast(OBJECT(dev), TYPE_CPU)) { > + if (object_dynamic_cast(OBJECT(dev), TYPE_CPU) || > + object_dynamic_cast(OBJECT(dev), TYPE_CPU_SOCKET)) { > return HOTPLUG_HANDLER(machine); > } > return NULL; > @@ -2017,6 +2114,8 @@ static void spapr_machine_class_init(ObjectClass *o= c, void *data) > mc->has_dynamic_sysbus =3D true; > mc->get_hotplug_handler =3D spapr_get_hotpug_handler; > hc->plug =3D spapr_machine_device_plug; > + hc->unplug =3D spapr_machine_device_unplug; > + > smc->dr_phb_enabled =3D false; > smc->dr_cpu_enabled =3D false; > smc->dr_lmb_enabled =3D false; > diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c > index fccee82..8e24235 100644 > --- a/target-ppc/translate_init.c > +++ b/target-ppc/translate_init.c > @@ -30,6 +30,7 @@ > #include "qemu/error-report.h" > #include "qapi/visitor.h" > #include "hw/qdev-properties.h" > +#include "migration/vmstate.h" > =20 > //#define PPC_DUMP_CPU > //#define PPC_DEBUG_SPR > @@ -9143,9 +9144,18 @@ static void ppc_cpu_unrealizefn(DeviceState *dev, = Error **errp) > { > PowerPCCPU *cpu =3D POWERPC_CPU(dev); > CPUPPCState *env =3D &cpu->env; > + CPUClass *cc =3D CPU_GET_CLASS(dev); > opc_handler_t **table; > int i, j; > =20 > + if (qdev_get_vmsd(dev) =3D=3D NULL) { > + vmstate_unregister(NULL, &vmstate_cpu_common, cpu); > + } > + > + if (cc->vmsd !=3D NULL) { > + vmstate_unregister(NULL, cc->vmsd, cpu); > + } > + > cpu_exec_exit(CPU(dev)); > =20 > for (i =3D 0; i < PPC_CPU_OPCODES_LEN; i++) { --=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 --Mh8CTEa8Ax54aLHp Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJVSHEmAAoJEGw4ysog2bOS7akQAOW6t9Yc9mX4d9J59tUadSkf H7PxS7Z8ySoT9GVHdY1sCpwaq+RMrC3wn7JbHYg2MSzwtZJPaLlCs9gYEELq+jXS 3fcfGTCazrLUowLxos5pJrZ2tgPvryXchKtYb2wnlfO0/IV1nmbTRpoR0MuZuN7N PUCU5tvlWAvnqHNnwVAIddn54dipIUeMCea495A+9gKOzbZTplTu50fiDhNVwLQx 3uevl6PFpNLKHlfAt9WoPoFuiJXwpg5UkhVINYleVjV86K9oo7TKvcAOkE3Gf+Dz Su639hbcQK78s2ILgr1j1JKyEkoYIUDBfxw7yd9ed7B6yar94jXA3SgVn0QaNIfN +uRb6LCff+2MrV4d9A+7eXRVMpucHV6XcXVP/52/jriH/0dWbOcZlXeuaMSDk0vQ H1aOLKgGdammuesAWx4zuiT5lq4nUlfcPkzbj9kSWrhgyaN0/bktTTPNBJgqgM7n O5TJ3nHoZEls0DCdKF0jg4MA+9h+bPXMsztLm5+Hgz51f8uZ8VGEFOjHSF3NL3t8 z36t5DJOh6Jp/HPu6A/igKwEtBZEed4AiBscRiMs9LPke4xKokpv7Rf2Ivz+yg70 8hYsIg2ojiJm83r4eYtWR+dvnIjC2inBeDzUwSVb8ZzvO7pr9xvBVkcfnlfgIX3y L3eA3y0lHzhlPzOughLC =r17e -----END PGP SIGNATURE----- --Mh8CTEa8Ax54aLHp--