From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:60659) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YafRW-0000nm-NQ for qemu-devel@nongnu.org; Wed, 25 Mar 2015 03:09:36 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YafRR-0008RI-Ud for qemu-devel@nongnu.org; Wed, 25 Mar 2015 03:09:34 -0400 Date: Wed, 25 Mar 2015 16:44:48 +1100 From: David Gibson Message-ID: <20150325054448.GD25043@voom.fritz.box> References: <1427117764-23008-1-git-send-email-bharata@linux.vnet.ibm.com> <1427117764-23008-20-git-send-email-bharata@linux.vnet.ibm.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="6NcXcVaJXVYoR9dF" Content-Disposition: inline In-Reply-To: <1427117764-23008-20-git-send-email-bharata@linux.vnet.ibm.com> Subject: Re: [Qemu-devel] [RFC PATCH v2 19/23] 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, 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 --6NcXcVaJXVYoR9dF Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Mon, Mar 23, 2015 at 07:06:00PM +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. >=20 > Signed-off-by: Bharata B Rao > --- > hw/ppc/spapr.c | 78 +++++++++++++++++++++++++++++++++++++++++= +++++- > linux-headers/linux/kvm.h | 1 + > target-ppc/kvm.c | 7 +++++ > target-ppc/kvm_ppc.h | 6 ++++ > 4 files changed, 91 insertions(+), 1 deletion(-) >=20 > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c > index b48994b..7b8784d 100644 > --- a/hw/ppc/spapr.c > +++ b/hw/ppc/spapr.c > @@ -1468,6 +1468,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) > { > @@ -1880,6 +1886,18 @@ static void spapr_cpu_hotplug_add(DeviceState *dev= , CPUState *cs, Error **errp) > } > } > =20 > +static void spapr_cpu_hotplug_remove(DeviceState *dev, CPUState *cs, > + Error **errp) > +{ > + 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 =3D SPAPR_DR_CONNECTOR_GET_CLASS(drc); > + > + drck->detach(drc, dev, NULL, NULL, errp); > +} > + > static void spapr_cpu_plug(HotplugHandler *hotplug_dev, DeviceState *dev, > Error **errp) > { > @@ -1911,6 +1929,51 @@ 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); > + > + spapr_cpu_destroy(cpu); I may be misunderstanding something here, but don't you need to signal the removal to the guest (and get its ack) before you "physically" remove the cpu? > + /* > + * 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; > + } > + g_assert(drc); > + > + spapr_cpu_hotplug_remove(dev, cs, errp); > + if (*errp) { > + return -1; > + } > + spapr_hotplug_req_remove_event(drc); > + > + 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) > { > @@ -1926,10 +1989,21 @@ static void spapr_machine_device_plug(HotplugHand= ler *hotplug_dev, > } > } > =20 > +static void spapr_machine_device_unplug(HotplugHandler *hotplug_dev, > + DeviceState *dev, Error **errp) > +{ > + if (object_dynamic_cast(OBJECT(dev), TYPE_CPU_SOCKET)) { > + if (dev->hotplugged && spapr->dr_cpu_enabled) { > + 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; > @@ -1953,6 +2027,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/linux-headers/linux/kvm.h b/linux-headers/linux/kvm.h > index 12045a1..0c1be07 100644 > --- a/linux-headers/linux/kvm.h > +++ b/linux-headers/linux/kvm.h > @@ -761,6 +761,7 @@ struct kvm_ppc_smmu_info { > #define KVM_CAP_PPC_FIXUP_HCALL 103 > #define KVM_CAP_PPC_ENABLE_HCALL 104 > #define KVM_CAP_CHECK_EXTENSION_VM 105 > +#define KVM_CAP_SPAPR_REUSE_VCPU 107 Updates to linux-headers/ are usually put into their own patch for safety (along with an indication of what kernel commit the additions came from). > #ifdef KVM_CAP_IRQ_ROUTING > =20 > diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c > index 1edf2b5..ee23bf6 100644 > --- a/target-ppc/kvm.c > +++ b/target-ppc/kvm.c > @@ -72,6 +72,7 @@ static int cap_ppc_watchdog; > static int cap_papr; > static int cap_htab_fd; > static int cap_fixup_hcalls; > +static int cap_spapr_reuse_vcpu; AFAICT nothing in this patch checks this variable. Does it belong in this patch? > static uint32_t debug_inst_opcode; > =20 > @@ -114,6 +115,7 @@ int kvm_arch_init(KVMState *s) > * only activated after this by kvmppc_set_papr() */ > cap_htab_fd =3D kvm_check_extension(s, KVM_CAP_PPC_HTAB_FD); > cap_fixup_hcalls =3D kvm_check_extension(s, KVM_CAP_PPC_FIXUP_HCALL); > + cap_spapr_reuse_vcpu =3D kvm_check_extension(s, KVM_CAP_SPAPR_REUSE_= VCPU); > =20 > if (!cap_interrupt_level) { > fprintf(stderr, "KVM: Couldn't find level irq capability. Expect= the " > @@ -2408,3 +2410,8 @@ int kvm_arch_fixup_msi_route(struct kvm_irq_routing= _entry *route, > { > return 0; > } > + > +bool kvmppc_spapr_reuse_vcpu(void) > +{ > + return cap_spapr_reuse_vcpu; > +} > diff --git a/target-ppc/kvm_ppc.h b/target-ppc/kvm_ppc.h > index 2e0224c..c831229 100644 > --- a/target-ppc/kvm_ppc.h > +++ b/target-ppc/kvm_ppc.h > @@ -40,6 +40,7 @@ void *kvmppc_create_spapr_tce(uint32_t liobn, uint32_t = window_size, int *pfd, > int kvmppc_remove_spapr_tce(void *table, int pfd, uint32_t window_size); > int kvmppc_reset_htab(int shift_hint); > uint64_t kvmppc_rma_size(uint64_t current_size, unsigned int hash_shift); > +bool kvmppc_spapr_reuse_vcpu(void); > #endif /* !CONFIG_USER_ONLY */ > bool kvmppc_has_cap_epr(void); > int kvmppc_define_rtas_kernel_token(uint32_t token, const char *function= ); > @@ -185,6 +186,11 @@ static inline int kvmppc_update_sdr1(CPUPPCState *en= v) > return 0; > } > =20 > +static inline bool kvmppc_spapr_reuse_vcpu(void) > +{ > + return false; > +} > + > #endif /* !CONFIG_USER_ONLY */ > =20 > static inline bool kvmppc_has_cap_epr(void) --=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 --6NcXcVaJXVYoR9dF Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJVEktQAAoJEGw4ysog2bOS19kP/0buoQGRcTD2m8MllFPLW3f4 GzUiuyOz6z1Yjwf5MPpTWp+7AjHO3BEc6kGpQgxuuEoXGndVrjpOUn2pS0RtqTJm XoLt0S5433OZzlSSbeTvKpTor1MDvUhyynF05APVou12D1g1XzX22swWfMfvctxT I7ZjzxavTuZC8EQ7AK7LO/YxAnIOMSTbi8mDSLPjctN1sThQ2zHis5vxRw5HzEwi g7eaEFlVHzECKwdyQ7LOehOkPFOTAE4yJatvKCWO6fPZ7//oVQS2oui5aGS14lOb iF8SIQxol45t2rRuKQ3c/Us94A+as5Jqhyvj8HumGrIFeVYBptDcxx2XCuXXUyvL jo6p5frKAG4B8Bc51TufOBPajYmapX7Xp59ZneA1mjyGL6AWbNUGIWhQSpt8S8Pn pwPcCFXKJ+D9JVYSkL3FEY4drMapjKgiadf95w29NSnXXNY4xQ9bYj+2F+tezYf/ RXgoM8CzoKBpa4Ss7ptw8s4wAJWw/nDll1AZtN0wtGwu3pNkshaZB9VxdinprEvt iRf7HMNlPCgITfCA4lu6Kwkml6pgUEr5qbLkIIek+61+FkjOdMiTXlmkszTMCVtE fjdnr52JNdmp4k5zqvBq8nUwzi04SGHe0X4S23gpUKFKiD+q0H+n/oc0flGsBP24 TReTVad7zRXrMkRB8253 =V5MZ -----END PGP SIGNATURE----- --6NcXcVaJXVYoR9dF--