[Qemu-devel] [PATCH v3 00/10] Allow hotplug of s390 CPUs

[Qemu-devel] [PATCH v3 00/10] Allow hotplug of s390 CPUs

[Matthew Rosato]

Changes from v2->v3:

* Call cpu_remove_sync rather than cpu_remove().
* Pull latest version of patches from pseries set (v6).  Trivial change to 
  "Reclaim VCPU objects" to fix checkpatch error.
* Add object_unparent during s390_cpu_release to accomodate changes in 
  Patch 4 "Reclaim VCPU objects."
* Remove a cleanup patch in favor of 2 patches from pseries set.

**************

The following patchset enables hotplug of s390 CPUs.

The standard interface is used -- to configure a guest with 2 CPUs online at 
boot and 4 maximum:

qemu -smp 2,maxcpus=4

To subsequently hotplug a CPU:

Issue 'device_add s390-cpu,id=<id>' from monitor.

At this point, the guest must bring the CPU online for use -- This can be 
achieved via "echo 1 > /sys/devices/system/cpu/cpuX/online" or via a management 
tool like cpuplugd.

Hot unplug support is provided via 'device_del <id>', however s390 does not have
a mechanism for gracefully handling a CPU that has been removed, so this event
triggers a reset of the guest in order to force recognition.  

This patch set is based on work previously done by Jason Herne.

Bharata B Rao (3):
  exec: Remove cpu from cpus list during cpu_exec_exit()
  exec: Do vmstate unregistration from cpu_exec_exit()
  cpu: Add a sync version of cpu_remove()

Gu Zheng (1):
  cpu: Reclaim vCPU objects

Matthew Rosato (6):
  s390x/cpu: Cleanup init in preparation for hotplug
  s390x/cpu: Set initial CPU state in common routine
  s390x/cpu: Move some CPU initialization into realize
  s390x/cpu: Add functions to (un)register CPU state
  s390/virtio-ccw: Add hotplug handler and prepare for unplug
  s390x/cpu: Allow hot plug/unplug of CPUs

 cpus.c                     | 50 +++++++++++++++++++++++++++++++++
 exec.c                     | 30 ++++++++++++++++++++
 hw/s390x/s390-virtio-ccw.c | 30 +++++++++++++++++++-
 hw/s390x/s390-virtio.c     | 64 +++++++++++++++++++++++++++++++-----------
 hw/s390x/s390-virtio.h     |  2 +-
 include/qom/cpu.h          | 18 ++++++++++++
 include/sysemu/kvm.h       |  1 +
 kvm-all.c                  | 57 ++++++++++++++++++++++++++++++++++++-
 kvm-stub.c                 |  5 ++++
 target-s390x/cpu.c         | 70 +++++++++++++++++++++++++++++++++++++++++++---
 target-s390x/cpu.h         |  4 +++
 11 files changed, 308 insertions(+), 23 deletions(-)

-- 
1.9.1

[Qemu-devel] [PATCH v3 01/10] exec: Remove cpu from cpus list during cpu_exec_exit()

[Matthew Rosato]

From: Bharata B Rao <bharata@linux.vnet.ibm.com>

CPUState *cpu gets added to the cpus list during cpu_exec_init(). It
should be removed from cpu_exec_exit().

cpu_exec_init() is called from generic CPU::instance_finalize and some
archs like PowerPC call it from CPU unrealizefn. So ensure that we
dequeue the cpu only once.

Now -1 value for cpu->cpu_index indicates that we have already dequeued
the cpu for CONFIG_USER_ONLY case also.

Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
---
 exec.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/exec.c b/exec.c
index 7115403..c8da9d4 100644
--- a/exec.c
+++ b/exec.c
@@ -596,6 +596,7 @@ void cpu_exec_exit(CPUState *cpu)
         return;
     }
 
+    QTAILQ_REMOVE(&cpus, cpu, node);
     bitmap_clear(cpu_index_map, cpu->cpu_index, 1);
     cpu->cpu_index = -1;
 }
@@ -614,6 +615,15 @@ static int cpu_get_free_index(Error **errp)
 
 void cpu_exec_exit(CPUState *cpu)
 {
+    cpu_list_lock();
+    if (cpu->cpu_index == -1) {
+        cpu_list_unlock();
+        return;
+    }
+
+    QTAILQ_REMOVE(&cpus, cpu, node);
+    cpu->cpu_index = -1;
+    cpu_list_unlock();
 }
 #endif
 
-- 
1.9.1

[Qemu-devel] [PATCH v3 02/10] exec: Do vmstate unregistration from cpu_exec_exit()

[Matthew Rosato]

From: Bharata B Rao <bharata@linux.vnet.ibm.com>

cpu_exec_init() does vmstate_register and register_savevm for the CPU device.
These need to be undone from cpu_exec_exit(). These changes are needed to
support CPU hot removal and also to correctly fail hotplug attempts
beyond max_cpus.

Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
---
 exec.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/exec.c b/exec.c
index c8da9d4..aa41032 100644
--- a/exec.c
+++ b/exec.c
@@ -591,6 +591,8 @@ static int cpu_get_free_index(Error **errp)
 
 void cpu_exec_exit(CPUState *cpu)
 {
+    CPUClass *cc = CPU_GET_CLASS(cpu);
+
     if (cpu->cpu_index == -1) {
         /* cpu_index was never allocated by this @cpu or was already freed. */
         return;
@@ -599,6 +601,15 @@ void cpu_exec_exit(CPUState *cpu)
     QTAILQ_REMOVE(&cpus, cpu, node);
     bitmap_clear(cpu_index_map, cpu->cpu_index, 1);
     cpu->cpu_index = -1;
+    if (cc->vmsd != NULL) {
+        vmstate_unregister(NULL, cc->vmsd, cpu);
+    }
+#if defined(CPU_SAVE_VERSION)
+    unregister_savevm(NULL, "cpu", cpu->env_ptr);
+#endif
+    if (qdev_get_vmsd(DEVICE(cpu)) == NULL) {
+        vmstate_unregister(NULL, &vmstate_cpu_common, cpu);
+    }
 }
 #else
 
@@ -615,6 +626,8 @@ static int cpu_get_free_index(Error **errp)
 
 void cpu_exec_exit(CPUState *cpu)
 {
+    CPUClass *cc = CPU_GET_CLASS(cpu);
+
     cpu_list_lock();
     if (cpu->cpu_index == -1) {
         cpu_list_unlock();
@@ -624,6 +637,13 @@ void cpu_exec_exit(CPUState *cpu)
     QTAILQ_REMOVE(&cpus, cpu, node);
     cpu->cpu_index = -1;
     cpu_list_unlock();
+
+    if (cc->vmsd != NULL) {
+        vmstate_unregister(NULL, cc->vmsd, cpu);
+    }
+    if (qdev_get_vmsd(DEVICE(cpu)) == NULL) {
+        vmstate_unregister(NULL, &vmstate_cpu_common, cpu);
+    }
 }
 #endif
 
-- 
1.9.1

[Qemu-devel] [PATCH v3 03/10] cpu: Reclaim vCPU objects

[Matthew Rosato]

From: Gu Zheng <guz.fnst@cn.fujitsu.com>

In order to deal well with the kvm vcpus (which can not be removed without any
protection), we do not close KVM vcpu fd, just record and mark it as stopped
into a list, so that we can reuse it for the appending cpu hot-add request if
possible. It is also the approach that kvm guys suggested:
https://www.mail-archive.com/kvm@vger.kernel.org/msg102839.html

Signed-off-by: Chen Fan <chen.fan.fnst@cn.fujitsu.com>
Signed-off-by: Gu Zheng <guz.fnst@cn.fujitsu.com>
Signed-off-by: Zhu Guihua <zhugh.fnst@cn.fujitsu.com>
Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com>
               [- Explicit CPU_REMOVE() from qemu_kvm/tcg_destroy_vcpu()
                  isn't needed as it is done from cpu_exec_exit()
                - Use iothread mutex instead of global mutex during
                  destroy
                - Don't cleanup vCPU object from vCPU thread context
                  but leave it to the callers (device_add/device_del)]
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
---
 cpus.c               | 38 +++++++++++++++++++++++++++++++++++
 include/qom/cpu.h    | 10 +++++++++
 include/sysemu/kvm.h |  1 +
 kvm-all.c            | 57 +++++++++++++++++++++++++++++++++++++++++++++++++++-
 kvm-stub.c           |  5 +++++
 5 files changed, 110 insertions(+), 1 deletion(-)

diff --git a/cpus.c b/cpus.c
index 1e97cc4..76aecd5 100644
--- a/cpus.c
+++ b/cpus.c
@@ -953,6 +953,18 @@ void async_run_on_cpu(CPUState *cpu, void (*func)(void *data), void *data)
     qemu_cpu_kick(cpu);
 }
 
+static void qemu_kvm_destroy_vcpu(CPUState *cpu)
+{
+    if (kvm_destroy_vcpu(cpu) < 0) {
+        error_report("kvm_destroy_vcpu failed.");
+        exit(EXIT_FAILURE);
+    }
+}
+
+static void qemu_tcg_destroy_vcpu(CPUState *cpu)
+{
+}
+
 static void flush_queued_work(CPUState *cpu)
 {
     struct qemu_work_item *wi;
@@ -1053,6 +1065,11 @@ static void *qemu_kvm_cpu_thread_fn(void *arg)
             }
         }
         qemu_kvm_wait_io_event(cpu);
+        if (cpu->exit && !cpu_can_run(cpu)) {
+            qemu_kvm_destroy_vcpu(cpu);
+            qemu_mutex_unlock_iothread();
+            return NULL;
+        }
     }
 
     return NULL;
@@ -1108,6 +1125,7 @@ static void tcg_exec_all(void);
 static void *qemu_tcg_cpu_thread_fn(void *arg)
 {
     CPUState *cpu = arg;
+    CPUState *remove_cpu = NULL;
 
     rcu_register_thread();
 
@@ -1145,6 +1163,16 @@ static void *qemu_tcg_cpu_thread_fn(void *arg)
             }
         }
         qemu_tcg_wait_io_event(QTAILQ_FIRST(&cpus));
+        CPU_FOREACH(cpu) {
+            if (cpu->exit && !cpu_can_run(cpu)) {
+                remove_cpu = cpu;
+                break;
+            }
+        }
+        if (remove_cpu) {
+            qemu_tcg_destroy_vcpu(remove_cpu);
+            remove_cpu = NULL;
+        }
     }
 
     return NULL;
@@ -1301,6 +1329,13 @@ void resume_all_vcpus(void)
     }
 }
 
+void cpu_remove(CPUState *cpu)
+{
+    cpu->stop = true;
+    cpu->exit = true;
+    qemu_cpu_kick(cpu);
+}
+
 /* For temporary buffers for forming a name */
 #define VCPU_THREAD_NAME_SIZE 16
 
@@ -1517,6 +1552,9 @@ static void tcg_exec_all(void)
                 break;
             }
         } else if (cpu->stop || cpu->stopped) {
+            if (cpu->exit) {
+                next_cpu = CPU_NEXT(cpu);
+            }
             break;
         }
     }
diff --git a/include/qom/cpu.h b/include/qom/cpu.h
index 2e5229d..32a2c71 100644
--- a/include/qom/cpu.h
+++ b/include/qom/cpu.h
@@ -232,6 +232,7 @@ struct kvm_run;
  * @halted: Nonzero if the CPU is in suspended state.
  * @stop: Indicates a pending stop request.
  * @stopped: Indicates the CPU has been artificially stopped.
+ * @exit: Indicates the CPU has exited due to an unplug operation.
  * @crash_occurred: Indicates the OS reported a crash (panic) for this CPU
  * @tcg_exit_req: Set to force TCG to stop executing linked TBs for this
  *           CPU and return to its top level loop.
@@ -284,6 +285,7 @@ struct CPUState {
     bool created;
     bool stop;
     bool stopped;
+    bool exit;
     bool crash_occurred;
     bool exit_request;
     uint32_t interrupt_request;
@@ -751,6 +753,14 @@ void cpu_exit(CPUState *cpu);
 void cpu_resume(CPUState *cpu);
 
 /**
+ * cpu_remove:
+ * @cpu: The CPU to remove.
+ *
+ * Requests the CPU to be removed.
+ */
+void cpu_remove(CPUState *cpu);
+
+/**
  * qemu_init_vcpu:
  * @cpu: The vCPU to initialize.
  *
diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h
index 7741f91..7324fa9 100644
--- a/include/sysemu/kvm.h
+++ b/include/sysemu/kvm.h
@@ -218,6 +218,7 @@ int kvm_has_intx_set_mask(void);
 
 int kvm_init_vcpu(CPUState *cpu);
 int kvm_cpu_exec(CPUState *cpu);
+int kvm_destroy_vcpu(CPUState *cpu);
 
 #ifdef NEED_CPU_H
 
diff --git a/kvm-all.c b/kvm-all.c
index 9148889..699c1ce 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -62,6 +62,12 @@
 
 #define KVM_MSI_HASHTAB_SIZE    256
 
+struct KVMParkedVcpu {
+    unsigned long vcpu_id;
+    int kvm_fd;
+    QLIST_ENTRY(KVMParkedVcpu) node;
+};
+
 struct KVMState
 {
     AccelState parent_obj;
@@ -95,6 +101,7 @@ struct KVMState
     QTAILQ_HEAD(msi_hashtab, KVMMSIRoute) msi_hashtab[KVM_MSI_HASHTAB_SIZE];
 #endif
     KVMMemoryListener memory_listener;
+    QLIST_HEAD(, KVMParkedVcpu) kvm_parked_vcpus;
 };
 
 KVMState *kvm_state;
@@ -238,6 +245,53 @@ static int kvm_set_user_memory_region(KVMMemoryListener *kml, KVMSlot *slot)
     return kvm_vm_ioctl(s, KVM_SET_USER_MEMORY_REGION, &mem);
 }
 
+int kvm_destroy_vcpu(CPUState *cpu)
+{
+    KVMState *s = kvm_state;
+    long mmap_size;
+    struct KVMParkedVcpu *vcpu = NULL;
+    int ret = 0;
+
+    DPRINTF("kvm_destroy_vcpu\n");
+
+    mmap_size = kvm_ioctl(s, KVM_GET_VCPU_MMAP_SIZE, 0);
+    if (mmap_size < 0) {
+        ret = mmap_size;
+        DPRINTF("KVM_GET_VCPU_MMAP_SIZE failed\n");
+        goto err;
+    }
+
+    ret = munmap(cpu->kvm_run, mmap_size);
+    if (ret < 0) {
+        goto err;
+    }
+
+    vcpu = g_malloc0(sizeof(*vcpu));
+    vcpu->vcpu_id = kvm_arch_vcpu_id(cpu);
+    vcpu->kvm_fd = cpu->kvm_fd;
+    QLIST_INSERT_HEAD(&kvm_state->kvm_parked_vcpus, vcpu, node);
+err:
+    return ret;
+}
+
+static int kvm_get_vcpu(KVMState *s, unsigned long vcpu_id)
+{
+    struct KVMParkedVcpu *cpu;
+
+    QLIST_FOREACH(cpu, &s->kvm_parked_vcpus, node) {
+        if (cpu->vcpu_id == vcpu_id) {
+            int kvm_fd;
+
+            QLIST_REMOVE(cpu, node);
+            kvm_fd = cpu->kvm_fd;
+            g_free(cpu);
+            return kvm_fd;
+        }
+    }
+
+    return kvm_vm_ioctl(s, KVM_CREATE_VCPU, (void *)vcpu_id);
+}
+
 int kvm_init_vcpu(CPUState *cpu)
 {
     KVMState *s = kvm_state;
@@ -246,7 +300,7 @@ int kvm_init_vcpu(CPUState *cpu)
 
     DPRINTF("kvm_init_vcpu\n");
 
-    ret = kvm_vm_ioctl(s, KVM_CREATE_VCPU, (void *)kvm_arch_vcpu_id(cpu));
+    ret = kvm_get_vcpu(s, kvm_arch_vcpu_id(cpu));
     if (ret < 0) {
         DPRINTF("kvm_create_vcpu failed\n");
         goto err;
@@ -1509,6 +1563,7 @@ static int kvm_init(MachineState *ms)
 #ifdef KVM_CAP_SET_GUEST_DEBUG
     QTAILQ_INIT(&s->kvm_sw_breakpoints);
 #endif
+    QLIST_INIT(&s->kvm_parked_vcpus);
     s->vmfd = -1;
     s->fd = qemu_open("/dev/kvm", O_RDWR);
     if (s->fd == -1) {
diff --git a/kvm-stub.c b/kvm-stub.c
index dc97a5e..0b39456 100644
--- a/kvm-stub.c
+++ b/kvm-stub.c
@@ -32,6 +32,11 @@ bool kvm_allowed;
 bool kvm_readonly_mem_allowed;
 bool kvm_ioeventfd_any_length_allowed;
 
+int kvm_destroy_vcpu(CPUState *cpu)
+{
+    return -ENOSYS;
+}
+
 int kvm_init_vcpu(CPUState *cpu)
 {
     return -ENOSYS;
-- 
1.9.1

[Qemu-devel] [PATCH v3 04/10] cpu: Add a sync version of cpu_remove()

[Matthew Rosato]

From: Bharata B Rao <bharata@linux.vnet.ibm.com>

This sync API will be used by the CPU hotplug code to wait for the CPU to
completely get removed before flagging the failure to the device_add
command.

Sync version of this call is needed to correctly recover from CPU
realization failures when ->plug() handler fails.

Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
---
 cpus.c            | 12 ++++++++++++
 include/qom/cpu.h |  8 ++++++++
 2 files changed, 20 insertions(+)

diff --git a/cpus.c b/cpus.c
index 76aecd5..a94db65 100644
--- a/cpus.c
+++ b/cpus.c
@@ -1067,6 +1067,8 @@ static void *qemu_kvm_cpu_thread_fn(void *arg)
         qemu_kvm_wait_io_event(cpu);
         if (cpu->exit && !cpu_can_run(cpu)) {
             qemu_kvm_destroy_vcpu(cpu);
+            cpu->created = false;
+            qemu_cond_signal(&qemu_cpu_cond);
             qemu_mutex_unlock_iothread();
             return NULL;
         }
@@ -1171,6 +1173,8 @@ static void *qemu_tcg_cpu_thread_fn(void *arg)
         }
         if (remove_cpu) {
             qemu_tcg_destroy_vcpu(remove_cpu);
+            cpu->created = false;
+            qemu_cond_signal(&qemu_cpu_cond);
             remove_cpu = NULL;
         }
     }
@@ -1336,6 +1340,14 @@ void cpu_remove(CPUState *cpu)
     qemu_cpu_kick(cpu);
 }
 
+void cpu_remove_sync(CPUState *cpu)
+{
+    cpu_remove(cpu);
+    while (cpu->created) {
+        qemu_cond_wait(&qemu_cpu_cond, &qemu_global_mutex);
+    }
+}
+
 /* For temporary buffers for forming a name */
 #define VCPU_THREAD_NAME_SIZE 16
 
diff --git a/include/qom/cpu.h b/include/qom/cpu.h
index 32a2c71..bed8654 100644
--- a/include/qom/cpu.h
+++ b/include/qom/cpu.h
@@ -760,6 +760,14 @@ void cpu_resume(CPUState *cpu);
  */
 void cpu_remove(CPUState *cpu);
 
+ /**
+ * cpu_remove_sync:
+ * @cpu: The CPU to remove.
+ *
+ * Requests the CPU to be removed and waits till it is removed.
+ */
+void cpu_remove_sync(CPUState *cpu);
+
 /**
  * qemu_init_vcpu:
  * @cpu: The vCPU to initialize.
-- 
1.9.1

[Qemu-devel] [PATCH v3 05/10] s390x/cpu: Cleanup init in preparation for hotplug

[Matthew Rosato]

Ensure a valid cpu_model is set upfront by setting the
default value directly into the MachineState when none is
specified.  This is needed to ensure hotplugged CPUs share
the same cpu_model.

Signed-off-by: Matthew Rosato <mjrosato@linux.vnet.ibm.com>
Reviewed-by: David Hildenbrand <dahi@linux.vnet.ibm.com>
---
 hw/s390x/s390-virtio-ccw.c | 2 +-
 hw/s390x/s390-virtio.c     | 8 ++++----
 hw/s390x/s390-virtio.h     | 2 +-
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index 586ddbb..909c42f 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -135,7 +135,7 @@ static void ccw_init(MachineState *machine)
     virtio_ccw_register_hcalls();
 
     /* init CPUs */
-    s390_init_cpus(machine->cpu_model);
+    s390_init_cpus(machine);
 
     if (kvm_enabled()) {
         kvm_s390_enable_css_support(s390_cpu_addr2state(0));
diff --git a/hw/s390x/s390-virtio.c b/hw/s390x/s390-virtio.c
index 946325f..e5d3875 100644
--- a/hw/s390x/s390-virtio.c
+++ b/hw/s390x/s390-virtio.c
@@ -95,12 +95,12 @@ void s390_init_ipl_dev(const char *kernel_filename,
     qdev_init_nofail(dev);
 }
 
-void s390_init_cpus(const char *cpu_model)
+void s390_init_cpus(MachineState *machine)
 {
     int i;
 
-    if (cpu_model == NULL) {
-        cpu_model = "host";
+    if (machine->cpu_model == NULL) {
+        machine->cpu_model = "host";
     }
 
     ipi_states = g_malloc(sizeof(S390CPU *) * smp_cpus);
@@ -109,7 +109,7 @@ void s390_init_cpus(const char *cpu_model)
         S390CPU *cpu;
         CPUState *cs;
 
-        cpu = cpu_s390x_init(cpu_model);
+        cpu = cpu_s390x_init(machine->cpu_model);
         cs = CPU(cpu);
 
         ipi_states[i] = cpu;
diff --git a/hw/s390x/s390-virtio.h b/hw/s390x/s390-virtio.h
index eebce8e..ffd014c 100644
--- a/hw/s390x/s390-virtio.h
+++ b/hw/s390x/s390-virtio.h
@@ -19,7 +19,7 @@
 typedef int (*s390_virtio_fn)(const uint64_t *args);
 void s390_register_virtio_hypercall(uint64_t code, s390_virtio_fn fn);
 
-void s390_init_cpus(const char *cpu_model);
+void s390_init_cpus(MachineState *machine);
 void s390_init_ipl_dev(const char *kernel_filename,
                        const char *kernel_cmdline,
                        const char *initrd_filename,
-- 
1.9.1

[Qemu-devel] [PATCH v3 06/10] s390x/cpu: Set initial CPU state in common routine

[Matthew Rosato]

Both initial and hotplugged CPUs need to set the same initial
state.

Signed-off-by: Matthew Rosato <mjrosato@linux.vnet.ibm.com>
Reviewed-by: David Hildenbrand <dahi@linux.vnet.ibm.com>
---
 hw/s390x/s390-virtio.c | 4 ----
 target-s390x/cpu.c     | 2 ++
 2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/hw/s390x/s390-virtio.c b/hw/s390x/s390-virtio.c
index e5d3875..4a0aab0 100644
--- a/hw/s390x/s390-virtio.c
+++ b/hw/s390x/s390-virtio.c
@@ -107,14 +107,10 @@ void s390_init_cpus(MachineState *machine)
 
     for (i = 0; i < smp_cpus; i++) {
         S390CPU *cpu;
-        CPUState *cs;
 
         cpu = cpu_s390x_init(machine->cpu_model);
-        cs = CPU(cpu);
 
         ipi_states[i] = cpu;
-        cs->halted = 1;
-        cs->exception_index = EXCP_HLT;
     }
 }
 
diff --git a/target-s390x/cpu.c b/target-s390x/cpu.c
index e5a3f65..b0f95ce 100644
--- a/target-s390x/cpu.c
+++ b/target-s390x/cpu.c
@@ -218,6 +218,8 @@ static void s390_cpu_initfn(Object *obj)
 #endif
 
     cs->env_ptr = env;
+    cs->halted = 1;
+    cs->exception_index = EXCP_HLT;
     cpu_exec_init(cs, &error_abort);
 #if !defined(CONFIG_USER_ONLY)
     qemu_register_reset(s390_cpu_machine_reset_cb, cpu);
-- 
1.9.1

[Qemu-devel] [PATCH v3 07/10] s390x/cpu: Move some CPU initialization into realize

[Matthew Rosato]

In preparation for hotplug, defer some CPU initialization
until the device is actually being realized.

Signed-off-by: Matthew Rosato <mjrosato@linux.vnet.ibm.com>
---
 target-s390x/cpu.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/target-s390x/cpu.c b/target-s390x/cpu.c
index b0f95ce..4aa00ef 100644
--- a/target-s390x/cpu.c
+++ b/target-s390x/cpu.c
@@ -36,6 +36,9 @@
 #define CR0_RESET       0xE0UL
 #define CR14_RESET      0xC2000000UL;
 
+/* Older kernels require CPUs to be added sequentially by id */
+static int next_cpu_id;
+
 /* generate CPU information for cpu -? */
 void s390_cpu_list(FILE *f, fprintf_function cpu_fprintf)
 {
@@ -194,7 +197,13 @@ static void s390_cpu_realizefn(DeviceState *dev, Error **errp)
 {
     CPUState *cs = CPU(dev);
     S390CPUClass *scc = S390_CPU_GET_CLASS(dev);
+    S390CPU *cpu = S390_CPU(dev);
+    CPUS390XState *env = &cpu->env;
 
+#if !defined(CONFIG_USER_ONLY)
+    qemu_register_reset(s390_cpu_machine_reset_cb, cpu);
+#endif
+    env->cpu_num = next_cpu_id++;
     s390_cpu_gdb_init(cs);
     qemu_init_vcpu(cs);
 #if !defined(CONFIG_USER_ONLY)
@@ -212,7 +221,6 @@ static void s390_cpu_initfn(Object *obj)
     S390CPU *cpu = S390_CPU(obj);
     CPUS390XState *env = &cpu->env;
     static bool inited;
-    static int cpu_num = 0;
 #if !defined(CONFIG_USER_ONLY)
     struct tm tm;
 #endif
@@ -222,7 +230,6 @@ static void s390_cpu_initfn(Object *obj)
     cs->exception_index = EXCP_HLT;
     cpu_exec_init(cs, &error_abort);
 #if !defined(CONFIG_USER_ONLY)
-    qemu_register_reset(s390_cpu_machine_reset_cb, cpu);
     qemu_get_timedate(&tm, 0);
     env->tod_offset = TOD_UNIX_EPOCH +
                       (time2tod(mktimegm(&tm)) * 1000000000ULL);
@@ -231,7 +238,6 @@ static void s390_cpu_initfn(Object *obj)
     env->cpu_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, s390x_cpu_timer, cpu);
     s390_cpu_set_state(CPU_STATE_STOPPED, cpu);
 #endif
-    env->cpu_num = cpu_num++;
 
     if (tcg_enabled() && !inited) {
         inited = true;
-- 
1.9.1

[Qemu-devel] [PATCH v3 08/10] s390x/cpu: Add functions to (un)register CPU state

[Matthew Rosato]

Introduce s390_(un)register_cpustate, which will set the
machine/cpu[n] link with the current CPU state.  Additionally,
maintain an array of state pointers indexed by CPU id for fast lookup
during interrupt handling.

Signed-off-by: Matthew Rosato <mjrosato@linux.vnet.ibm.com>
---
 hw/s390x/s390-virtio.c | 54 +++++++++++++++++++++++++++++++++++++++++---------
 target-s390x/cpu.c     | 14 ++++++++++++-
 target-s390x/cpu.h     |  2 ++
 3 files changed, 60 insertions(+), 10 deletions(-)

diff --git a/hw/s390x/s390-virtio.c b/hw/s390x/s390-virtio.c
index 4a0aab0..9ac1b72 100644
--- a/hw/s390x/s390-virtio.c
+++ b/hw/s390x/s390-virtio.c
@@ -60,15 +60,44 @@
 #define S390_TOD_CLOCK_VALUE_MISSING    0x00
 #define S390_TOD_CLOCK_VALUE_PRESENT    0x01
 
-static S390CPU **ipi_states;
+static S390CPU **cpu_states;
 
 S390CPU *s390_cpu_addr2state(uint16_t cpu_addr)
 {
-    if (cpu_addr >= smp_cpus) {
+    if (cpu_addr >= max_cpus) {
         return NULL;
     }
 
-    return ipi_states[cpu_addr];
+    /* Fast lookup via CPU ID */
+    return cpu_states[cpu_addr];
+}
+
+int s390_register_cpustate(uint16_t cpu_addr, S390CPU *state)
+{
+    gchar *name;
+    int r = 0;
+
+    name = g_strdup_printf("cpu[%i]", cpu_addr);
+    if (object_property_get_link(qdev_get_machine(), name, NULL)) {
+        r = -EEXIST;
+        goto out;
+    }
+
+    object_property_set_link(qdev_get_machine(), OBJECT(state), name,\
+                             &error_abort);
+
+out:
+    g_free(name);
+    return r;
+}
+
+void s390_unregister_cpustate(uint16_t cpu_addr)
+{
+    gchar *name;
+
+    name = g_strdup_printf("cpu[%i]", cpu_addr);
+    object_property_set_link(qdev_get_machine(), NULL, name, &error_abort);
+    g_free(name);
 }
 
 void s390_init_ipl_dev(const char *kernel_filename,
@@ -98,19 +127,26 @@ void s390_init_ipl_dev(const char *kernel_filename,
 void s390_init_cpus(MachineState *machine)
 {
     int i;
+    gchar *name;
 
     if (machine->cpu_model == NULL) {
         machine->cpu_model = "host";
     }
 
-    ipi_states = g_malloc(sizeof(S390CPU *) * smp_cpus);
+    cpu_states = g_malloc0(sizeof(S390CPU *) * max_cpus);
 
-    for (i = 0; i < smp_cpus; i++) {
-        S390CPU *cpu;
-
-        cpu = cpu_s390x_init(machine->cpu_model);
+    for (i = 0; i < max_cpus; i++) {
+        name = g_strdup_printf("cpu[%i]", i);
+        object_property_add_link(qdev_get_machine(), name, TYPE_S390_CPU,
+                                 (Object **) &cpu_states[i],
+                                 object_property_allow_set_link,
+                                 OBJ_PROP_LINK_UNREF_ON_RELEASE,
+                                 &error_abort);
+        g_free(name);
+    }
 
-        ipi_states[i] = cpu;
+    for (i = 0; i < smp_cpus; i++) {
+        cpu_s390x_init(machine->cpu_model);
     }
 }
 
diff --git a/target-s390x/cpu.c b/target-s390x/cpu.c
index 4aa00ef..0a669ac 100644
--- a/target-s390x/cpu.c
+++ b/target-s390x/cpu.c
@@ -31,6 +31,7 @@
 #include "trace.h"
 #ifndef CONFIG_USER_ONLY
 #include "sysemu/arch_init.h"
+#include "sysemu/sysemu.h"
 #endif
 
 #define CR0_RESET       0xE0UL
@@ -201,9 +202,20 @@ static void s390_cpu_realizefn(DeviceState *dev, Error **errp)
     CPUS390XState *env = &cpu->env;
 
 #if !defined(CONFIG_USER_ONLY)
+    if (s390_register_cpustate(next_cpu_id, cpu) < 0) {
+        error_setg(errp, "Cannot have more than %d CPUs", max_cpus);
+        return;
+    }
     qemu_register_reset(s390_cpu_machine_reset_cb, cpu);
-#endif
+    env->cpu_num = next_cpu_id;
+    while (next_cpu_id < max_cpus - 1) {
+        if (!cpu_exists(++next_cpu_id)) {
+            break;
+        }
+    }
+#else
     env->cpu_num = next_cpu_id++;
+#endif
     s390_cpu_gdb_init(cs);
     qemu_init_vcpu(cs);
 #if !defined(CONFIG_USER_ONLY)
diff --git a/target-s390x/cpu.h b/target-s390x/cpu.h
index 06ca60b..ae36fcc 100644
--- a/target-s390x/cpu.h
+++ b/target-s390x/cpu.h
@@ -531,6 +531,8 @@ static inline int s390_set_clock(uint8_t *tod_high, uint64_t *tod_low)
 }
 
 S390CPU *s390_cpu_addr2state(uint16_t cpu_addr);
+int s390_register_cpustate(uint16_t cpu_addr, S390CPU *state);
+void s390_unregister_cpustate(uint16_t cpu_addr);
 unsigned int s390_cpu_halt(S390CPU *cpu);
 void s390_cpu_unhalt(S390CPU *cpu);
 unsigned int s390_cpu_set_state(uint8_t cpu_state, S390CPU *cpu);
-- 
1.9.1

[Qemu-devel] [PATCH v3 09/10] s390/virtio-ccw: Add hotplug handler and prepare for unplug

[Matthew Rosato]

Prepare for hotplug and unplug of s390-cpu.  In the case
of unplug, s390 does not have a safe way of communicating
the loss of CPU to a guest, so a full system reset will
be performed on the guest.

Signed-off-by: Matthew Rosato <mjrosato@linux.vnet.ibm.com>
---
 hw/s390x/s390-virtio-ccw.c | 28 ++++++++++++++++++++++++++++
 target-s390x/cpu.c         | 33 ++++++++++++++++++++++++++++++++-
 target-s390x/cpu.h         |  2 ++
 3 files changed, 62 insertions(+), 1 deletion(-)

diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index 909c42f..547e070 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -155,10 +155,34 @@ static void ccw_init(MachineState *machine)
                     gtod_save, gtod_load, kvm_state);
 }
 
+static void s390_machine_device_plug(HotplugHandler *hotplug_dev,
+                                     DeviceState *dev, Error **errp)
+{
+
+}
+
+static void s390_machine_device_unplug(HotplugHandler *hotplug_dev,
+                                       DeviceState *dev, Error **errp)
+{
+    if (object_dynamic_cast(OBJECT(dev), TYPE_CPU)) {
+        s390_cpu_unplug(hotplug_dev, dev, errp);
+    }
+}
+
+static HotplugHandler *s390_get_hotplug_handler(MachineState *machine,
+                                                DeviceState *dev)
+{
+    if (object_dynamic_cast(OBJECT(dev), TYPE_CPU)) {
+        return HOTPLUG_HANDLER(machine);
+    }
+    return NULL;
+}
+
 static void ccw_machine_class_init(ObjectClass *oc, void *data)
 {
     MachineClass *mc = MACHINE_CLASS(oc);
     NMIClass *nc = NMI_CLASS(oc);
+    HotplugHandlerClass *hc = HOTPLUG_HANDLER_CLASS(oc);
 
     mc->init = ccw_init;
     mc->reset = s390_machine_reset;
@@ -170,6 +194,9 @@ static void ccw_machine_class_init(ObjectClass *oc, void *data)
     mc->no_sdcard = 1;
     mc->use_sclp = 1;
     mc->max_cpus = 255;
+    mc->get_hotplug_handler = s390_get_hotplug_handler;
+    hc->plug = s390_machine_device_plug;
+    hc->unplug = s390_machine_device_unplug;
     nc->nmi_monitor_handler = s390_nmi;
 }
 
@@ -231,6 +258,7 @@ static const TypeInfo ccw_machine_info = {
     .class_init    = ccw_machine_class_init,
     .interfaces = (InterfaceInfo[]) {
         { TYPE_NMI },
+        { TYPE_HOTPLUG_HANDLER},
         { }
     },
 };
diff --git a/target-s390x/cpu.c b/target-s390x/cpu.c
index 0a669ac..46b1115 100644
--- a/target-s390x/cpu.c
+++ b/target-s390x/cpu.c
@@ -236,7 +236,6 @@ static void s390_cpu_initfn(Object *obj)
 #if !defined(CONFIG_USER_ONLY)
     struct tm tm;
 #endif
-
     cs->env_ptr = env;
     cs->halted = 1;
     cs->exception_index = EXCP_HLT;
@@ -268,6 +267,38 @@ static void s390_cpu_finalize(Object *obj)
 }
 
 #if !defined(CONFIG_USER_ONLY)
+static void s390_cpu_destroy(S390CPU *cpu)
+{
+    CPUS390XState *env = &cpu->env;
+    s390_unregister_cpustate(env->cpu_num);
+}
+
+static void s390_cpu_release(DeviceState *dev, void *opaque)
+{
+    CPUState *cs = CPU(dev);
+
+    s390_cpu_destroy(S390_CPU(cs));
+    cpu_remove_sync(cs);
+    object_unparent(OBJECT(dev));
+}
+
+int s390_cpu_unplug(HotplugHandler *hotplug_dev, DeviceState *dev,
+                           Error **errp)
+{
+    S390CPU *cpu = S390_CPU(dev);
+    CPUS390XState *env = &cpu->env;
+
+    if (next_cpu_id > env->cpu_num) {
+        next_cpu_id = env->cpu_num;
+    }
+    s390_cpu_release(dev, NULL);
+
+    /* Perform a full system reset */
+    qemu_system_reset_request();
+
+    return 0;
+}
+
 static bool disabled_wait(CPUState *cpu)
 {
     return cpu->halted && !(S390_CPU(cpu)->env.psw.mask &
diff --git a/target-s390x/cpu.h b/target-s390x/cpu.h
index ae36fcc..c68c405 100644
--- a/target-s390x/cpu.h
+++ b/target-s390x/cpu.h
@@ -543,6 +543,8 @@ static inline uint8_t s390_cpu_get_state(S390CPU *cpu)
 
 void gtod_save(QEMUFile *f, void *opaque);
 int gtod_load(QEMUFile *f, void *opaque, int version_id);
+int s390_cpu_unplug(HotplugHandler *hotplug_dev, DeviceState *dev,
+                    Error **errp);
 
 /* service interrupts are floating therefore we must not pass an cpustate */
 void s390_sclp_extint(uint32_t parm);
-- 
1.9.1

[Qemu-devel] [PATCH v3 10/10] s390x/cpu: Allow hot plug/unplug of CPUs

[Matthew Rosato]

Allow hotplug of s390-cpu devices via device_add, and unplug
via device_del.

Signed-off-by: Matthew Rosato <mjrosato@linux.vnet.ibm.com>
---
 target-s390x/cpu.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/target-s390x/cpu.c b/target-s390x/cpu.c
index 46b1115..7160f33 100644
--- a/target-s390x/cpu.c
+++ b/target-s390x/cpu.c
@@ -31,6 +31,7 @@
 #include "trace.h"
 #ifndef CONFIG_USER_ONLY
 #include "sysemu/arch_init.h"
+#include "hw/s390x/sclp.h"
 #include "sysemu/sysemu.h"
 #endif
 
@@ -225,6 +226,12 @@ static void s390_cpu_realizefn(DeviceState *dev, Error **errp)
 #endif
 
     scc->parent_realize(dev, errp);
+
+#if !defined(CONFIG_USER_ONLY)
+    if (dev->hotplugged) {
+        raise_irq_cpu_hotplug();
+    }
+#endif
 }
 
 static void s390_cpu_initfn(Object *obj)
@@ -385,6 +392,10 @@ static void s390_cpu_class_init(ObjectClass *oc, void *data)
     scc->parent_realize = dc->realize;
     dc->realize = s390_cpu_realizefn;
 
+    /* Necessary prep-work for s390-cpu is handled in
+     * instance_init() and realize(), so allow device_add */
+    dc->cannot_instantiate_with_device_add_yet = false;
+
     scc->parent_reset = cc->reset;
 #if !defined(CONFIG_USER_ONLY)
     scc->load_normal = s390_cpu_load_normal;
-- 
1.9.1

Re: [Qemu-devel] [PATCH v3 07/10] s390x/cpu: Move some CPU initialization into realize

[David Hildenbrand]

> In preparation for hotplug, defer some CPU initialization
> until the device is actually being realized.
> 
> Signed-off-by: Matthew Rosato <mjrosato@linux.vnet.ibm.com>
> ---

Reviewed-by: David Hildenbrand <dahi@linux.vnet.ibm.com>

David

Re: [Qemu-devel] [PATCH v3 00/10] Allow hotplug of s390 CPUs

[Christian Borntraeger]

On 01/27/2016 05:53 PM, Matthew Rosato wrote:
> Changes from v2->v3:
> 
> * Call cpu_remove_sync rather than cpu_remove().
> * Pull latest version of patches from pseries set (v6).  Trivial change to 
>   "Reclaim VCPU objects" to fix checkpatch error.
> * Add object_unparent during s390_cpu_release to accomodate changes in 
>   Patch 4 "Reclaim VCPU objects."
> * Remove a cleanup patch in favor of 2 patches from pseries set.
> 
> **************
> 
> The following patchset enables hotplug of s390 CPUs.
> 
> The standard interface is used -- to configure a guest with 2 CPUs online at 
> boot and 4 maximum:
> 
> qemu -smp 2,maxcpus=4
> 
> To subsequently hotplug a CPU:
> 
> Issue 'device_add s390-cpu,id=<id>' from monitor.
> 
> At this point, the guest must bring the CPU online for use -- This can be 
> achieved via "echo 1 > /sys/devices/system/cpu/cpuX/online" or via a management 
> tool like cpuplugd.
> 
> Hot unplug support is provided via 'device_del <id>', however s390 does not have
> a mechanism for gracefully handling a CPU that has been removed, so this event
> triggers a reset of the guest in order to force recognition.  
> 
> This patch set is based on work previously done by Jason Herne.
> 
> Bharata B Rao (3):
>   exec: Remove cpu from cpus list during cpu_exec_exit()
>   exec: Do vmstate unregistration from cpu_exec_exit()
>   cpu: Add a sync version of cpu_remove()
> 
> Gu Zheng (1):
>   cpu: Reclaim vCPU objects
> 
> Matthew Rosato (6):
>   s390x/cpu: Cleanup init in preparation for hotplug
>   s390x/cpu: Set initial CPU state in common routine
>   s390x/cpu: Move some CPU initialization into realize
>   s390x/cpu: Add functions to (un)register CPU state
>   s390/virtio-ccw: Add hotplug handler and prepare for unplug
>   s390x/cpu: Allow hot plug/unplug of CPUs
> 
>  cpus.c                     | 50 +++++++++++++++++++++++++++++++++
>  exec.c                     | 30 ++++++++++++++++++++
>  hw/s390x/s390-virtio-ccw.c | 30 +++++++++++++++++++-
>  hw/s390x/s390-virtio.c     | 64 +++++++++++++++++++++++++++++++-----------
>  hw/s390x/s390-virtio.h     |  2 +-
>  include/qom/cpu.h          | 18 ++++++++++++
>  include/sysemu/kvm.h       |  1 +
>  kvm-all.c                  | 57 ++++++++++++++++++++++++++++++++++++-
>  kvm-stub.c                 |  5 ++++
>  target-s390x/cpu.c         | 70 +++++++++++++++++++++++++++++++++++++++++++---
>  target-s390x/cpu.h         |  4 +++
>  11 files changed, 308 insertions(+), 23 deletions(-)


Acked-by: Christian Borntraeger <borntraeger@de.ibm.com>


Alexander, if you are too busy at the moment, we could carry
these patches via the s390/kvm tree?

We want these patches merged, since we have to libvirt as well to
use device_add instead of cpu_add (sigh).

Christian

Re: [Qemu-devel] [PATCH v3 00/10] Allow hotplug of s390 CPUs

[David Hildenbrand]

> Changes from v2->v3:
> 
> * Call cpu_remove_sync rather than cpu_remove().
> * Pull latest version of patches from pseries set (v6).  Trivial change to 
>   "Reclaim VCPU objects" to fix checkpatch error.
> * Add object_unparent during s390_cpu_release to accomodate changes in 
>   Patch 4 "Reclaim VCPU objects."
> * Remove a cleanup patch in favor of 2 patches from pseries set.
> 
> **************
> 
> The following patchset enables hotplug of s390 CPUs.
> 
> The standard interface is used -- to configure a guest with 2 CPUs online at 
> boot and 4 maximum:
> 
> qemu -smp 2,maxcpus=4
> 
> To subsequently hotplug a CPU:
> 
> Issue 'device_add s390-cpu,id=<id>' from monitor.

(questions for the bigger audience)

For x86, cpu models are realized by making x86_64-cpu an abstract class and
creating loads of new classes, e.g. host-x86_64-cpu or haswell-x86_64-cpu.

How does 'device_add <cpu class>' play together with the x86 cpu model
approach? And with cpu models specified via "-cpu" in general?

Or does that in return mean, that "making models own classes" is outdated? Or
will some internal conversion happen that I am missing?

What is the plan for cpu models and cpu hotplug? How are cpu models to be
defined in the future?

David

Re: [Qemu-devel] [PATCH v3 00/10] Allow hotplug of s390 CPUs

[Andreas Färber]

Am 10.02.2016 um 16:28 schrieb David Hildenbrand:
> For x86, cpu models are realized by making x86_64-cpu an abstract class and
> creating loads of new classes, e.g. host-x86_64-cpu or haswell-x86_64-cpu.
> 
> How does 'device_add <cpu class>' play together with the x86 cpu model
> approach? And with cpu models specified via "-cpu" in general?

device_add needs to use an instantiatable type, like the ones you sketch
above.

> Or does that in return mean, that "making models own classes" is outdated? Or
> will some internal conversion happen that I am missing?
> 
> What is the plan for cpu models and cpu hotplug? How are cpu models to be
> defined in the future?

Someone at IBM was working on defining models for s390x - not sure what
the status is?

Regards,
Andreas

-- 
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Graham Norton; HRB 21284 (AG Nürnberg)

Re: [Qemu-devel] [PATCH v3 00/10] Allow hotplug of s390 CPUs

[David Hildenbrand]

> Am 10.02.2016 um 16:28 schrieb David Hildenbrand:
> > For x86, cpu models are realized by making x86_64-cpu an abstract class and
> > creating loads of new classes, e.g. host-x86_64-cpu or haswell-x86_64-cpu.
> > 
> > How does 'device_add <cpu class>' play together with the x86 cpu model
> > approach? And with cpu models specified via "-cpu" in general?  
> 
> device_add needs to use an instantiatable type, like the ones you sketch
> above.
> 
> > Or does that in return mean, that "making models own classes" is outdated? Or
> > will some internal conversion happen that I am missing?
> > 
> > What is the plan for cpu models and cpu hotplug? How are cpu models to be
> > defined in the future?  
> 
> Someone at IBM was working on defining models for s390x - not sure what
> the status is?

That one is me right now :) Michael Mueller was working on a version without
explicit features last year. I'm now looking into models with features that can
be turned on/off - like x86 has.

As I'm trying to get a view of the bigger picture I also have to take care of
cpu hotplug, and I am not quite sure yet if we (s390) really want or need a
device_add.

a) Specification of cpu model and cpu hotplug on QEMU start

-smp 2,maxcpus=6 -cpu zBC12,+feata,+featb,prop=value

Here, it is quite clear that all cpus will get the same feature set. We don't
need any information about internals (e.g. which class is used internally for
the cpu)

b) Adding cpus via the monitor "cpu_add"

cpu-add id=3

Quite easy, we get what we ordered when starting QEMU, a cpu just like the
others.

c) Adding a cpu via device_add

device_add s390-cpu,id=3
-> Not uniform. We _want_ cpu models as cpu subclasses
(http://wiki.qemu.org/Features/CPUHotplug)

OR

device_add zBC12-s390-cpu,id=3
-> Not uniform. We don't specify the properties. But we have to specify some
magic class that we didn't have to specify on the command line. Implicitly
used information for a device.

OR

device_add zBC12-s390-cpu,id=3,feata=on,featb=off,prop=value
-> Fully specified. Again. And we need information about the internally used
class. Implicitly used information for a device.

Especially the last two examples are bad:
1) We could hotplug _different_ cpus, which is absolutely not what we want on
s390.
2) In every sane setup, we have to respecify what we already setup up at QEMU
start. (and I don't see any benefit)
3) Interface that is much more complex and more error prone to use.


d) Benefits of the new interface.

Unfortunately I can't seem to find any
(http://wiki.qemu.org/Features/CPUHotplug) but what I can think of

1) Specify something like topology more detailed (IMHO not applicable for s390)
2) Do a device_del (IMHO not applicable for s390)


Both of these points could easily be realized by extending the existing cpu-add
and by introducing a cpu_del (if really needed).


I am not against this, I just want to understand what the plan is. Because this
highly overcomplicates matter for us (s390) and requires yet another interface
to be maintained (I have some quote about new interfaces in the back of my hand
from some Linus guy ;) )

"Targets are encouraged to (re)design CPU creation so that it would be possible
to use device_add/device-del interface for it. However if due to target
design or a necessary long re-factoring time to use CPU with
device_add/device-del interface, it is possible speed-up CPU hot-add feature
development by using cpu-add interface."
(http://wiki.qemu.org/Features/CPUHotplug)

If nobody can convince me that this is the way to go and everything I said is
already clear or wrong, then I'd vote for keeping it simple and using cpu-add.

David