[Qemu-devel] [PATCH v2 0/9] s390: Allow hotplug of s390 CPUs

[Qemu-devel] [PATCH v2 0/9] s390: Allow hotplug of s390 CPUs

[Matthew Rosato]

Changes from v1->v2:

* Remove cpu-add support.  Instead use 'device_add s390-cpu'
* Add unplug support via 'device_del'.
* Pull in 2 patches from pseries set.  Patch 1 just required some rebasing.
  Patch 2 required minor changes due to previous upstream review comments.

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

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 (1):
  cpus: Add a sync version of cpu_remove()

Matthew Rosato (8):
  cpus: Reclaim vCPU objects
  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
  s390x/cpu: Extra cleanup during CPU finalize
  s390/virtio-ccw: Add hotplug handler and prepare for unplug
  s390x/cpu: Allow hot plug/unplug of CPUs

 cpus.c                     | 53 +++++++++++++++++++++++++++++
 hw/s390x/s390-virtio-ccw.c | 30 ++++++++++++++++-
 hw/s390x/s390-virtio.c     | 66 ++++++++++++++++++++++++++----------
 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         | 83 +++++++++++++++++++++++++++++++++++++++++++---
 target-s390x/cpu.h         |  4 +++
 10 files changed, 295 insertions(+), 24 deletions(-)

-- 
1.9.1

[Qemu-devel] [PATCH v2 1/9] cpus: 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()]
Signed-off-by: Matthew Rosato <mjrosato@linux.vnet.ibm.com>
---
 cpus.c               | 41 +++++++++++++++++++++++++++++++++++++
 include/qom/cpu.h    | 10 +++++++++
 include/sysemu/kvm.h |  1 +
 kvm-all.c            | 57 +++++++++++++++++++++++++++++++++++++++++++++++++++-
 kvm-stub.c           |  5 +++++
 5 files changed, 113 insertions(+), 1 deletion(-)

diff --git a/cpus.c b/cpus.c
index 877bd70..af2b274 100644
--- a/cpus.c
+++ b/cpus.c
@@ -953,6 +953,21 @@ 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.\n");
+        exit(EXIT_FAILURE);
+    }
+
+    object_unparent(OBJECT(cpu));
+}
+
+static void qemu_tcg_destroy_vcpu(CPUState *cpu)
+{
+    object_unparent(OBJECT(cpu));
+}
+
 static void flush_queued_work(CPUState *cpu)
 {
     struct qemu_work_item *wi;
@@ -1053,6 +1068,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(&qemu_global_mutex);
+            return NULL;
+        }
     }
 
     return NULL;
@@ -1108,6 +1128,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 +1166,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 +1332,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
 
@@ -1506,6 +1544,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 51a1323..6a1d887 100644
--- a/include/qom/cpu.h
+++ b/include/qom/cpu.h
@@ -224,6 +224,7 @@ struct kvm_run;
  * @stop: Indicates a pending stop request.
  * @stopped: Indicates the CPU has been artificially stopped.
  * @crash_occurred: Indicates the OS reported a crash (panic) for this CPU
+ * @exit: Indicates the CPU has exited due to an unplug operation.
  * @tcg_exit_req: Set to force TCG to stop executing linked TBs for this
  *           CPU and return to its top level loop.
  * @singlestep_enabled: Flags for single-stepping.
@@ -276,6 +277,7 @@ struct CPUState {
     bool stopped;
     bool crash_occurred;
     bool exit_request;
+    bool exit;
     uint32_t interrupt_request;
     int singlestep_enabled;
     int64_t icount_extra;
@@ -695,6 +697,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 b31f325..dd1b783 100644
--- a/include/sysemu/kvm.h
+++ b/include/sysemu/kvm.h
@@ -206,6 +206,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 c648b81..3befc59 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -60,6 +60,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;
@@ -93,6 +99,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;
@@ -235,6 +242,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;
@@ -243,7 +297,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;
@@ -1468,6 +1522,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 v2 2/9] cpus: 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>
Signed-off-by: Matthew Rosato <mjrosato@linux.vnet.ibm.com>
               [Call cpu_remove() directly from cpu_remove_sync()]
---
 cpus.c            | 12 ++++++++++++
 include/qom/cpu.h |  8 ++++++++
 2 files changed, 20 insertions(+)

diff --git a/cpus.c b/cpus.c
index af2b274..c2444ba 100644
--- a/cpus.c
+++ b/cpus.c
@@ -1070,6 +1070,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(&qemu_global_mutex);
             return NULL;
         }
@@ -1174,6 +1176,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;
         }
     }
@@ -1339,6 +1343,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 6a1d887..dc2b566 100644
--- a/include/qom/cpu.h
+++ b/include/qom/cpu.h
@@ -705,6 +705,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 v2 3/9] 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     | 10 +++++-----
 hw/s390x/s390-virtio.h     |  2 +-
 3 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index 5a52ff2..4f6ed57 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 51dc0a8..6304433 100644
--- a/hw/s390x/s390-virtio.c
+++ b/hw/s390x/s390-virtio.c
@@ -168,12 +168,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);
@@ -182,7 +182,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;
@@ -301,7 +301,7 @@ static void s390_init(MachineState *machine)
                               virtio_region_len);
 
     /* init CPUs */
-    s390_init_cpus(machine->cpu_model);
+    s390_init_cpus(machine);
 
     /* Create VirtIO network adapters */
     s390_create_virtio_net((BusState *)s390_bus, "virtio-net-s390");
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 v2 4/9] 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 6304433..84cde80 100644
--- a/hw/s390x/s390-virtio.c
+++ b/hw/s390x/s390-virtio.c
@@ -180,14 +180,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 189a2af..aafbbdc 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 v2 5/9] 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 aafbbdc..bc821f7 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 v2 6/9] 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 84cde80..b6effc3 100644
--- a/hw/s390x/s390-virtio.c
+++ b/hw/s390x/s390-virtio.c
@@ -61,17 +61,46 @@
 #define S390_TOD_CLOCK_VALUE_PRESENT    0x01
 
 static VirtIOS390Bus *s390_bus;
-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);
+ }
+
 static int s390_virtio_hcall_notify(const uint64_t *args)
 {
     uint64_t mem = args[0];
@@ -171,19 +200,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 bc821f7..0ef67a1 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 658cd9d..524a7e4 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 v2 7/9] s390x/cpu: Extra cleanup during CPU finalize

[Matthew Rosato]

In preparation for unplug, do some additional cleanup
work to undo work originally done in cpu_exec_init.

This patch is based on work done by Bharata B Rao.

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

diff --git a/target-s390x/cpu.c b/target-s390x/cpu.c
index 0ef67a1..060a3cc 100644
--- a/target-s390x/cpu.c
+++ b/target-s390x/cpu.c
@@ -261,6 +261,20 @@ static void s390_cpu_finalize(Object *obj)
 {
 #if !defined(CONFIG_USER_ONLY)
     S390CPU *cpu = S390_CPU(obj);
+    CPUState *cs = CPU(cpu);
+    CPUClass *cc = CPU_GET_CLASS(cs);
+    CPUS390XState *env = &cpu->env;
+
+    QTAILQ_REMOVE(&cpus, cs, node);
+    if (cc->vmsd != NULL) {
+        vmstate_unregister(NULL, cc->vmsd, cs);
+    }
+#if defined(CPU_SAVE_VERSION)
+    unregister_savevm(NULL, "cpu", cs->env_ptr);
+#endif
+    if (qdev_get_vmsd(DEVICE(cs)) == NULL) {
+        vmstate_unregister(NULL, &vmstate_cpu_common, cs);
+    }
 
     qemu_unregister_reset(s390_cpu_machine_reset_cb, cpu);
     g_free(cpu->irqstate);
-- 
1.9.1

[Qemu-devel] [PATCH v2 8/9] 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         | 32 +++++++++++++++++++++++++++++++-
 target-s390x/cpu.h         |  2 ++
 3 files changed, 61 insertions(+), 1 deletion(-)

diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index 4f6ed57..ba0a90d 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 060a3cc..4b83a09 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;
@@ -282,6 +281,37 @@ 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(cs);
+}
+
+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 524a7e4..81554b5 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 v2 9/9] 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 4b83a09..a4be7b7 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)
@@ -398,6 +405,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 v2 2/9] cpus: Add a sync version of cpu_remove()

[Paolo Bonzini]

On 19/11/2015 16:10, Matthew Rosato wrote:
> 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>
> Signed-off-by: Matthew Rosato <mjrosato@linux.vnet.ibm.com>
>                [Call cpu_remove() directly from cpu_remove_sync()]

Are you using it in these patches?

Paolo

> ---
>  cpus.c            | 12 ++++++++++++
>  include/qom/cpu.h |  8 ++++++++
>  2 files changed, 20 insertions(+)
> 
> diff --git a/cpus.c b/cpus.c
> index af2b274..c2444ba 100644
> --- a/cpus.c
> +++ b/cpus.c
> @@ -1070,6 +1070,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(&qemu_global_mutex);
>              return NULL;
>          }
> @@ -1174,6 +1176,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;
>          }
>      }
> @@ -1339,6 +1343,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 6a1d887..dc2b566 100644
> --- a/include/qom/cpu.h
> +++ b/include/qom/cpu.h
> @@ -705,6 +705,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.
> 

Re: [Qemu-devel] [PATCH v2 2/9] cpus: Add a sync version of cpu_remove()

[Matthew Rosato]

On 11/19/2015 10:25 AM, Paolo Bonzini wrote:
> 
> 
> On 19/11/2015 16:10, Matthew Rosato wrote:
>> 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>
>> Signed-off-by: Matthew Rosato <mjrosato@linux.vnet.ibm.com>
>>                [Call cpu_remove() directly from cpu_remove_sync()]
> 
> Are you using it in these patches?
>

Ugh, nice catch -- I am not using it anymore, I decided it was fine to
just use cpu_remove in patch 8 directly.  So, this patch can be dropped
from the set.

Matt

Re: [Qemu-devel] [PATCH v2 1/9] cpus: Reclaim vCPU objects

[Bharata B Rao]

On Thu, Nov 19, 2015 at 10:10:06AM -0500, Matthew Rosato wrote:
> 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()]

I didn't look very closely but the patch that removes cpu from the list
from cpu_exec_exit() isn't part of this series. The above change requires

https://lists.gnu.org/archive/html/qemu-devel/2015-08/msg00656.html

I have just cleaned that patch a bit and will be posting early next
week with another patch that does CPU vmstate unregistration too from
cpu_exec_exit(). I think since we do vmstate registration from cpu_exec_init()
it makes sense to do unregistration from cpu_exec_exit() instead of
archs doing it themselves. I had a version of this at

https://lists.gnu.org/archive/html/qemu-devel/2015-08/msg00649.html

With the above patch, you woudn't need 7/9 in this series.

Regards,
Bharata.

Re: [Qemu-devel] [PATCH v2 1/9] cpus: Reclaim vCPU objects

[Matthew Rosato]

On 11/19/2015 09:33 PM, Bharata B Rao wrote:
> On Thu, Nov 19, 2015 at 10:10:06AM -0500, Matthew Rosato wrote:
>> 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()]
> 
> I didn't look very closely but the patch that removes cpu from the list
> from cpu_exec_exit() isn't part of this series. The above change requires
> 
> https://lists.gnu.org/archive/html/qemu-devel/2015-08/msg00656.html
> 
> I have just cleaned that patch a bit and will be posting early next
> week with another patch that does CPU vmstate unregistration too from
> cpu_exec_exit(). I think since we do vmstate registration from cpu_exec_init()
> it makes sense to do unregistration from cpu_exec_exit() instead of
> archs doing it themselves. I had a version of this at
> 
> https://lists.gnu.org/archive/html/qemu-devel/2015-08/msg00649.html
> 
> With the above patch, you woudn't need 7/9 in this series.
> 

Hi Bharata -- Looking at the mailing list discussion from your patch
set, I got the impression that handling this in cpu_exec_exit() might
not be acceptable for all architectures.  So, my patch just tries to
handle the s390 case in patch 7/9, doing list removal and vmstate
unregistration.

FWIW, the 2 patches you referenced would be fine for s390, so if you can
get those approved I'd have no problem dropping 7/9 in favor of your
patches.

Matt