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?
> + 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?
> + 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)
> {
> @@ -1984,16 +2061,36 @@ static void spapr_machine_device_plug(HotplugHandler
> *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);
> }
> }
>
> +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 machine");
> + 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 *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/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"
>
> //#define PPC_DUMP_CPU
> //#define PPC_DEBUG_SPR
> @@ -9143,9 +9144,18 @@ static void ppc_cpu_unrealizefn(DeviceState *dev,
> Error **errp)
> {
> PowerPCCPU *cpu = POWERPC_CPU(dev);
> CPUPPCState *env = &cpu->env;
> + CPUClass *cc = CPU_GET_CLASS(dev);
> opc_handler_t **table;
> int i, j;
>
> + if (qdev_get_vmsd(dev) == NULL) {
> + vmstate_unregister(NULL, &vmstate_cpu_common, cpu);
> + }
> +
> + if (cc->vmsd != NULL) {
> + vmstate_unregister(NULL, cc->vmsd, cpu);
> + }
> +
> cpu_exec_exit(CPU(dev));
>
> for (i = 0; i < PPC_CPU_OPCODES_LEN; i++) {
--
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
pgpEm3hj4j1iJ.pgp
Description: PGP signature
