From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:41012) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YaoHD-000640-GP for qemu-devel@nongnu.org; Wed, 25 Mar 2015 12:35:32 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YaoH9-0004Em-89 for qemu-devel@nongnu.org; Wed, 25 Mar 2015 12:35:31 -0400 Received: from e23smtp03.au.ibm.com ([202.81.31.145]:59389) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YaoH8-0004EM-Jx for qemu-devel@nongnu.org; Wed, 25 Mar 2015 12:35:27 -0400 Received: from /spool/local by e23smtp03.au.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Thu, 26 Mar 2015 02:35:21 +1000 Date: Wed, 25 Mar 2015 22:04:18 +0530 From: Bharata B Rao Message-ID: <20150325163418.GH32581@in.ibm.com> References: <1427117764-23008-1-git-send-email-bharata@linux.vnet.ibm.com> <1427117764-23008-20-git-send-email-bharata@linux.vnet.ibm.com> <20150325054448.GD25043@voom.fritz.box> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20150325054448.GD25043@voom.fritz.box> Subject: Re: [Qemu-devel] [RFC PATCH v2 19/23] spapr: CPU hot unplug support Reply-To: bharata@linux.vnet.ibm.com List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: David Gibson 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 On Wed, Mar 25, 2015 at 04:44:48PM +1100, David Gibson wrote: > 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. > > > > 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(-) > > > > 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); > > } > > > > +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) > > } > > } > > > > +static void spapr_cpu_hotplug_remove(DeviceState *dev, CPUState *cs, > > + Error **errp) > > +{ > > + PowerPCCPU *cpu = POWERPC_CPU(cs); > > + int id = ppc_get_vcpu_dt_id(cpu); > > + sPAPRDRConnector *drc = > > + spapr_dr_connector_by_id(SPAPR_DR_CONNECTOR_TYPE_CPU, id); > > + sPAPRDRConnectorClass *drck = 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; > > } > > > > +static int spapr_cpu_unplug(Object *obj, void *opaque) > > +{ > > + Error **errp = opaque; > > + DeviceState *dev = DEVICE(obj); > > + CPUState *cs = CPU(dev); > > + PowerPCCPU *cpu = POWERPC_CPU(cs); > > + int id = ppc_get_vcpu_dt_id(cpu); > > + int smt = kvmppc_smt_threads(); > > + sPAPRDRConnector *drc = > > + 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? Oh yes, I should move this call to spapr_cpu_release(). > > > + /* > > + * SMT threads return from here, only main thread (core) will > > + * continue and signal hot unplug event to the guest. > > + */ > > + if ((id % smt) != 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 = 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(HotplugHandler *hotplug_dev, > > } > > } > > > > +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 *oc, void *data) > > mc->has_dynamic_sysbus = true; > > mc->get_hotplug_handler = spapr_get_hotpug_handler; > > hc->plug = spapr_machine_device_plug; > > + hc->unplug = spapr_machine_device_unplug; > > + > > smc->dr_phb_enabled = false; > > smc->dr_cpu_enabled = false; > > smc->dr_lmb_enabled = 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 > > > > 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? This is some remnant of an old approach I was trying. It got left out by mistake and sorry that you had to review this part. I will clean this up in the next post. Regards, Bharata.