On Tue, May 05, 2015 at 05:28:38PM +1000, David Gibson wrote: > 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. > > > > Signed-off-by: Bharata B Rao <bhar...@linux.vnet.ibm.com> > > --- > > hw/ppc/spapr.c | 101 > > +++++++++++++++++++++++++++++++++++++++++++- > > target-ppc/translate_init.c | 10 +++++ > > 2 files changed, 110 insertions(+), 1 deletion(-) > > > > 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); > > } > > > > +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(DeviceState *dev, CPUState *cs, > > return fdt; > > } > > > > +static void spapr_cpu_release(DeviceState *dev, void *opaque) > > +{ > > + CPUState *cs; > > + int i; > > + int id = ppc_get_vcpu_dt_id(POWERPC_CPU(CPU(dev))); > > + > > + for (i = id; i < id + smp_threads; i++) { > > + CPU_FOREACH(cs) { > > + PowerPCCPU *cpu = POWERPC_CPU(cs); > > + > > + if (i == 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; > > } > > > > +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); > > + sPAPRDRConnectorClass *drck; > > + Error *local_err = NULL; > > + > > + /* > > + * 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; > > + } > > 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?
When I switch to core level addition in the next version, I will see how best this can be handled. > > > + g_assert(drc); > > + > > + drck = 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? If I am reading it right, PCI hotplug is also following the same order (detach followed by guest notification). Let me experiment with this in the next version. Regards, Bharata.