[Qemu-devel] [PATCH v5 0/6] Allow hotplug of s390 CPUs

[Qemu-devel] [PATCH v5 0/6] Allow hotplug of s390 CPUs

[Matthew Rosato]

Changes from v4->v5:
* Patch #3 - Drop static next_cpu_id (Igor)
* Patch #4 - Drop register_cpustate in favor of machine hotplug handler.
  Re-write commit message as a result. (Igor)
* Patch #5 - Revive hotplug handler code from previous patch set.  Use 
  plug callback to update property links instead of register_cpustate. (Igor)
* Patch #6 - Use last_cpu instead of next_cpu_id to sanity-check hotplugged 
  CPU id (Igor)

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

As discussed in the KVM call, we will go ahead with cpu_add for 
s390x to get cpu hotplug functionality in s390x now, until
architectures that require a more robust hotplug interface
settle on a design.

To configure a guest with 2 CPUs online at 
boot and 4 maximum:

qemu -smp 2,maxcpus=4

Or, when using libvirt:
  <domain>
    ...
    <vcpu current="2">4</vcpu>
    ...
  </domain> 


To subsequently hotplug a CPU:

Issue 'cpu-add <id>' from qemu monitor, or use virsh setvcpus --count <n> 
<domain>, where <n> is the total number of desired guest CPUs.

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.

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

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 CPU property links
  s390/virtio-ccw: Add hotplug handler
  s390x/cpu: Allow hotplug of CPUs

 hw/s390x/s390-virtio-ccw.c | 43 ++++++++++++++++++++++++++++++++++-
 hw/s390x/s390-virtio.c     | 36 ++++++++++++++++-------------
 hw/s390x/s390-virtio.h     |  2 +-
 target-s390x/cpu.c         | 56 +++++++++++++++++++++++++++++++++++++++++++---
 target-s390x/cpu.h         |  1 +
 5 files changed, 117 insertions(+), 21 deletions(-)

-- 
1.9.1

[Qemu-devel] [PATCH v5 1/6] 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 89f5d0d..b05ed8b 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -136,7 +136,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 c320878..b576811 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 v5 2/6] 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 b576811..b3707f4 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 73a910d..603c2a1 100644
--- a/target-s390x/cpu.c
+++ b/target-s390x/cpu.c
@@ -219,6 +219,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 v5 3/6] 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 | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/target-s390x/cpu.c b/target-s390x/cpu.c
index 603c2a1..8dfd063 100644
--- a/target-s390x/cpu.c
+++ b/target-s390x/cpu.c
@@ -195,7 +195,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 = cs->cpu_index;
     s390_cpu_gdb_init(cs);
     qemu_init_vcpu(cs);
 #if !defined(CONFIG_USER_ONLY)
@@ -213,7 +219,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
@@ -223,7 +228,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);
@@ -232,7 +236,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 v5 4/6] s390x/cpu: Add CPU property links

[Matthew Rosato]

Link each CPUState as property machine/cpu[n] during initialization.
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 | 26 +++++++++++++++++---------
 1 file changed, 17 insertions(+), 9 deletions(-)

diff --git a/hw/s390x/s390-virtio.c b/hw/s390x/s390-virtio.c
index b3707f4..6bd9803 100644
--- a/hw/s390x/s390-virtio.c
+++ b/hw/s390x/s390-virtio.c
@@ -60,15 +60,16 @@
 #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];
 }
 
 void s390_init_ipl_dev(const char *kernel_filename,
@@ -98,19 +99,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(OBJECT(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);
     }
 }
 
-- 
1.9.1

[Qemu-devel] [PATCH v5 5/6] s390/virtio-ccw: Add hotplug handler

[Matthew Rosato]

First consumer will be CPU hotplug, to update machine/cpu[n]
links during cpu plug.

Signed-off-by: Matthew Rosato <mjrosato@linux.vnet.ibm.com>
---
 hw/s390x/s390-virtio-ccw.c | 40 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 40 insertions(+)

diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index b05ed8b..31c1082 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -156,10 +156,46 @@ static void ccw_init(MachineState *machine)
                     gtod_save, gtod_load, kvm_state);
 }
 
+static void s390_cpu_plug(HotplugHandler *hotplug_dev,
+                        DeviceState *dev, Error **errp)
+{
+    gchar *name;
+    S390CPU *cpu = S390_CPU(dev);
+    CPUState *cs = CPU(dev);
+
+    name = g_strdup_printf("cpu[%i]", cpu->env.cpu_num);
+    object_property_set_link(OBJECT(qdev_get_machine()), OBJECT(cs), name,
+                             errp);
+}
+
+static void s390_machine_device_plug(HotplugHandler *hotplug_dev,
+                                     DeviceState *dev, Error **errp)
+{
+    if (object_dynamic_cast(OBJECT(dev), TYPE_CPU)) {
+        s390_cpu_plug(hotplug_dev, dev, errp);
+    }
+}
+
+static void s390_machine_device_unplug(HotplugHandler *hotplug_dev,
+                                       DeviceState *dev, Error **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;
@@ -171,6 +207,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;
 }
 
@@ -232,6 +271,7 @@ static const TypeInfo ccw_machine_info = {
     .class_init    = ccw_machine_class_init,
     .interfaces = (InterfaceInfo[]) {
         { TYPE_NMI },
+        { TYPE_HOTPLUG_HANDLER},
         { }
     },
 };
-- 
1.9.1

[Qemu-devel] [PATCH v5 6/6] s390x/cpu: Allow hotplug of CPUs

[Matthew Rosato]

Implement cpu hotplug routine and add the machine hook.

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

diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index 31c1082..3be41ee 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -199,6 +199,7 @@ static void ccw_machine_class_init(ObjectClass *oc, void *data)
 
     mc->init = ccw_init;
     mc->reset = s390_machine_reset;
+    mc->hot_add_cpu = s390_hot_add_cpu;
     mc->block_default_type = IF_VIRTIO;
     mc->no_cdrom = 1;
     mc->no_floppy = 1;
diff --git a/target-s390x/cpu.c b/target-s390x/cpu.c
index 8dfd063..99394ec 100644
--- a/target-s390x/cpu.c
+++ b/target-s390x/cpu.c
@@ -32,6 +32,12 @@
 #include "trace.h"
 #ifndef CONFIG_USER_ONLY
 #include "sysemu/arch_init.h"
+#include "sysemu/sysemu.h"
+#include "hw/boards.h"
+#include "hw/s390x/sclp.h"
+#include "qom/cpu.h"
+
+#define last_cpu QTAILQ_LAST(&cpus, CPUTailQ)
 #endif
 
 #define CR0_RESET       0xE0UL
@@ -211,6 +217,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)
@@ -254,6 +266,39 @@ static void s390_cpu_finalize(Object *obj)
 }
 
 #if !defined(CONFIG_USER_ONLY)
+void s390_hot_add_cpu(const int64_t id, Error **errp)
+{
+    MachineState *machine = MACHINE(qdev_get_machine());
+
+    if (id < 0) {
+        error_setg(errp, "Invalid CPU id: %" PRIi64, id);
+        return;
+    }
+
+    if (cpu_exists(id)) {
+        error_setg(errp, "Unable to add CPU: %" PRIi64
+                   ", it already exists", id);
+        return;
+    }
+
+    if (id >= max_cpus) {
+        error_setg(errp, "Unable to add CPU: %" PRIi64
+                   ", max allowed: %d", id, max_cpus - 1);
+        return;
+    }
+
+    if (id != (last_cpu->cpu_index + 1)) {
+        error_setg(errp, "Unable to add CPU: %" PRIi64
+                   ", The next available id is %d", id,
+                   (last_cpu->cpu_index + 1));
+        return;
+    }
+
+    cpu_s390x_init(machine->cpu_model);
+}
+#endif
+
+#if !defined(CONFIG_USER_ONLY)
 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 06ca60b..cf1e2bd 100644
--- a/target-s390x/cpu.h
+++ b/target-s390x/cpu.h
@@ -541,6 +541,7 @@ 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);
+void s390_hot_add_cpu(const int64_t id, Error **errp);
 
 /* service interrupts are floating therefore we must not pass an cpustate */
 void s390_sclp_extint(uint32_t parm);
-- 
1.9.1

Re: [Qemu-devel] [PATCH v5 6/6] s390x/cpu: Allow hotplug of CPUs

[Igor Mammedov]

On Thu, 18 Feb 2016 23:03:37 -0500
Matthew Rosato <mjrosato@linux.vnet.ibm.com> wrote:

> Implement cpu hotplug routine and add the machine hook.
> 
> Signed-off-by: Matthew Rosato <mjrosato@linux.vnet.ibm.com>
> ---
>  hw/s390x/s390-virtio-ccw.c |  1 +
>  target-s390x/cpu.c         | 45 +++++++++++++++++++++++++++++++++++++++++++++
>  target-s390x/cpu.h         |  1 +
>  3 files changed, 47 insertions(+)
> 
> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
> index 31c1082..3be41ee 100644
> --- a/hw/s390x/s390-virtio-ccw.c
> +++ b/hw/s390x/s390-virtio-ccw.c
> @@ -199,6 +199,7 @@ static void ccw_machine_class_init(ObjectClass *oc, void *data)
>  
>      mc->init = ccw_init;
>      mc->reset = s390_machine_reset;
> +    mc->hot_add_cpu = s390_hot_add_cpu;
>      mc->block_default_type = IF_VIRTIO;
>      mc->no_cdrom = 1;
>      mc->no_floppy = 1;
> diff --git a/target-s390x/cpu.c b/target-s390x/cpu.c
> index 8dfd063..99394ec 100644
> --- a/target-s390x/cpu.c
> +++ b/target-s390x/cpu.c
> @@ -32,6 +32,12 @@
>  #include "trace.h"
>  #ifndef CONFIG_USER_ONLY
>  #include "sysemu/arch_init.h"
> +#include "sysemu/sysemu.h"
> +#include "hw/boards.h"
> +#include "hw/s390x/sclp.h"
> +#include "qom/cpu.h"
> +
> +#define last_cpu QTAILQ_LAST(&cpus, CPUTailQ)
this should be in qom/cpu.h, maybe a separate patch 

>  #endif
>  
>  #define CR0_RESET       0xE0UL
> @@ -211,6 +217,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)
> @@ -254,6 +266,39 @@ static void s390_cpu_finalize(Object *obj)
>  }
>  
>  #if !defined(CONFIG_USER_ONLY)
> +void s390_hot_add_cpu(const int64_t id, Error **errp)
> +{
this one looks like a func that belongs to board and should
be a static in hw/s390x/s390-virtio-ccw.c

> +    MachineState *machine = MACHINE(qdev_get_machine());
> +
> +    if (id < 0) {
> +        error_setg(errp, "Invalid CPU id: %" PRIi64, id);
> +        return;
> +    }
> +
> +    if (cpu_exists(id)) {
> +        error_setg(errp, "Unable to add CPU: %" PRIi64
> +                   ", it already exists", id);
> +        return;
> +    }
> +
> +    if (id >= max_cpus) {
> +        error_setg(errp, "Unable to add CPU: %" PRIi64
> +                   ", max allowed: %d", id, max_cpus - 1);
> +        return;
> +    }
> +
> +    if (id != (last_cpu->cpu_index + 1)) {
> +        error_setg(errp, "Unable to add CPU: %" PRIi64
> +                   ", The next available id is %d", id,
> +                   (last_cpu->cpu_index + 1));
> +        return;
> +    }
> +
> +    cpu_s390x_init(machine->cpu_model);
> +}
> +#endif
> +
> +#if !defined(CONFIG_USER_ONLY)
>  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 06ca60b..cf1e2bd 100644
> --- a/target-s390x/cpu.h
> +++ b/target-s390x/cpu.h
> @@ -541,6 +541,7 @@ 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);
> +void s390_hot_add_cpu(const int64_t id, Error **errp);
>  
>  /* service interrupts are floating therefore we must not pass an cpustate */
>  void s390_sclp_extint(uint32_t parm);

Re: [Qemu-devel] [PATCH v5 5/6] s390/virtio-ccw: Add hotplug handler

[Igor Mammedov]

On Thu, 18 Feb 2016 23:03:36 -0500
Matthew Rosato <mjrosato@linux.vnet.ibm.com> wrote:

> First consumer will be CPU hotplug, to update machine/cpu[n]
> links during cpu plug.
> 
> Signed-off-by: Matthew Rosato <mjrosato@linux.vnet.ibm.com>
> ---
>  hw/s390x/s390-virtio-ccw.c | 40 ++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 40 insertions(+)
> 
> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
> index b05ed8b..31c1082 100644
> --- a/hw/s390x/s390-virtio-ccw.c
> +++ b/hw/s390x/s390-virtio-ccw.c
> @@ -156,10 +156,46 @@ static void ccw_init(MachineState *machine)
>                      gtod_save, gtod_load, kvm_state);
>  }
>  
> +static void s390_cpu_plug(HotplugHandler *hotplug_dev,
> +                        DeviceState *dev, Error **errp)
> +{
> +    gchar *name;
> +    S390CPU *cpu = S390_CPU(dev);
> +    CPUState *cs = CPU(dev);
> +
> +    name = g_strdup_printf("cpu[%i]", cpu->env.cpu_num);
> +    object_property_set_link(OBJECT(qdev_get_machine()), OBJECT(cs), name,
> +                             errp);
> +}
> +
> +static void s390_machine_device_plug(HotplugHandler *hotplug_dev,
> +                                     DeviceState *dev, Error **errp)
> +{
> +    if (object_dynamic_cast(OBJECT(dev), TYPE_CPU)) {
> +        s390_cpu_plug(hotplug_dev, dev, errp);
> +    }
> +}
> +
> +static void s390_machine_device_unplug(HotplugHandler *hotplug_dev,
> +                                       DeviceState *dev, Error **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;
> @@ -171,6 +207,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;
drop unplug handler as does nothing, if it's NULL then generic qdev code
will ignore it, see qdev_unplug().

>      nc->nmi_monitor_handler = s390_nmi;
>  }
>  
> @@ -232,6 +271,7 @@ static const TypeInfo ccw_machine_info = {
>      .class_init    = ccw_machine_class_init,
>      .interfaces = (InterfaceInfo[]) {
>          { TYPE_NMI },
> +        { TYPE_HOTPLUG_HANDLER},
>          { }
>      },
>  };

Re: [Qemu-devel] [PATCH v5 6/6] s390x/cpu: Allow hotplug of CPUs

[Matthew Rosato]

On 02/19/2016 09:28 AM, Igor Mammedov wrote:
> On Thu, 18 Feb 2016 23:03:37 -0500
> Matthew Rosato <mjrosato@linux.vnet.ibm.com> wrote:
> 
>> Implement cpu hotplug routine and add the machine hook.
>>
>> Signed-off-by: Matthew Rosato <mjrosato@linux.vnet.ibm.com>
>> ---
>>  hw/s390x/s390-virtio-ccw.c |  1 +
>>  target-s390x/cpu.c         | 45 +++++++++++++++++++++++++++++++++++++++++++++
>>  target-s390x/cpu.h         |  1 +
>>  3 files changed, 47 insertions(+)
>>
>> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
>> index 31c1082..3be41ee 100644
>> --- a/hw/s390x/s390-virtio-ccw.c
>> +++ b/hw/s390x/s390-virtio-ccw.c
>> @@ -199,6 +199,7 @@ static void ccw_machine_class_init(ObjectClass *oc, void *data)
>>  
>>      mc->init = ccw_init;
>>      mc->reset = s390_machine_reset;
>> +    mc->hot_add_cpu = s390_hot_add_cpu;
>>      mc->block_default_type = IF_VIRTIO;
>>      mc->no_cdrom = 1;
>>      mc->no_floppy = 1;
>> diff --git a/target-s390x/cpu.c b/target-s390x/cpu.c
>> index 8dfd063..99394ec 100644
>> --- a/target-s390x/cpu.c
>> +++ b/target-s390x/cpu.c
>> @@ -32,6 +32,12 @@
>>  #include "trace.h"
>>  #ifndef CONFIG_USER_ONLY
>>  #include "sysemu/arch_init.h"
>> +#include "sysemu/sysemu.h"
>> +#include "hw/boards.h"
>> +#include "hw/s390x/sclp.h"
>> +#include "qom/cpu.h"
>> +
>> +#define last_cpu QTAILQ_LAST(&cpus, CPUTailQ)
> this should be in qom/cpu.h, maybe a separate patch 
> 

I did that originally, but trips up due to usage of the phrase
'last_cpu' elsewhere in the code.  Looking now, hw/intc/openpic.c is the
culprit.

I could split this out into a patch that adds last_cpu to qom/cpu.h +
changes the variable name in openpic.c.

Or use a different name for the macro.  (cpu_last for example).

Preference?

>>  #endif
>>  
>>  #define CR0_RESET       0xE0UL
>> @@ -211,6 +217,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)
>> @@ -254,6 +266,39 @@ static void s390_cpu_finalize(Object *obj)
>>  }
>>  
>>  #if !defined(CONFIG_USER_ONLY)
>> +void s390_hot_add_cpu(const int64_t id, Error **errp)
>> +{
> this one looks like a func that belongs to board and should
> be a static in hw/s390x/s390-virtio-ccw.c
> 

OK, sure.

Matt

Re: [Qemu-devel] [PATCH v5 6/6] s390x/cpu: Allow hotplug of CPUs

[Igor Mammedov]

On Fri, 19 Feb 2016 09:40:13 -0500
Matthew Rosato <mjrosato@linux.vnet.ibm.com> wrote:

> On 02/19/2016 09:28 AM, Igor Mammedov wrote:
> > On Thu, 18 Feb 2016 23:03:37 -0500
> > Matthew Rosato <mjrosato@linux.vnet.ibm.com> wrote:
> >   
> >> Implement cpu hotplug routine and add the machine hook.
> >>
> >> Signed-off-by: Matthew Rosato <mjrosato@linux.vnet.ibm.com>
> >> ---
> >>  hw/s390x/s390-virtio-ccw.c |  1 +
> >>  target-s390x/cpu.c         | 45 +++++++++++++++++++++++++++++++++++++++++++++
> >>  target-s390x/cpu.h         |  1 +
> >>  3 files changed, 47 insertions(+)
> >>
> >> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
> >> index 31c1082..3be41ee 100644
> >> --- a/hw/s390x/s390-virtio-ccw.c
> >> +++ b/hw/s390x/s390-virtio-ccw.c
> >> @@ -199,6 +199,7 @@ static void ccw_machine_class_init(ObjectClass *oc, void *data)
> >>  
> >>      mc->init = ccw_init;
> >>      mc->reset = s390_machine_reset;
> >> +    mc->hot_add_cpu = s390_hot_add_cpu;
> >>      mc->block_default_type = IF_VIRTIO;
> >>      mc->no_cdrom = 1;
> >>      mc->no_floppy = 1;
> >> diff --git a/target-s390x/cpu.c b/target-s390x/cpu.c
> >> index 8dfd063..99394ec 100644
> >> --- a/target-s390x/cpu.c
> >> +++ b/target-s390x/cpu.c
> >> @@ -32,6 +32,12 @@
> >>  #include "trace.h"
> >>  #ifndef CONFIG_USER_ONLY
> >>  #include "sysemu/arch_init.h"
> >> +#include "sysemu/sysemu.h"
> >> +#include "hw/boards.h"
> >> +#include "hw/s390x/sclp.h"
> >> +#include "qom/cpu.h"
> >> +
> >> +#define last_cpu QTAILQ_LAST(&cpus, CPUTailQ)  
> > this should be in qom/cpu.h, maybe a separate patch 
> >   
> 
> I did that originally, but trips up due to usage of the phrase
> 'last_cpu' elsewhere in the code.  Looking now, hw/intc/openpic.c is the
> culprit.
> 
> I could split this out into a patch that adds last_cpu to qom/cpu.h +
> changes the variable name in openpic.c.
I'd change variable name in openpic.c.

> 
> Or use a different name for the macro.  (cpu_last for example).
> 
> Preference?
typically macros should be UPPER_CASED, I guess first_cpu is lower
cased due historical reasons to avoid renaming it across a bunch of files.

Andreas,
do you have any preference in which way fix last_cpu issue?

> 
> >>  #endif
> >>  
> >>  #define CR0_RESET       0xE0UL
> >> @@ -211,6 +217,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)
> >> @@ -254,6 +266,39 @@ static void s390_cpu_finalize(Object *obj)
> >>  }
> >>  
> >>  #if !defined(CONFIG_USER_ONLY)
> >> +void s390_hot_add_cpu(const int64_t id, Error **errp)
> >> +{  
> > this one looks like a func that belongs to board and should
> > be a static in hw/s390x/s390-virtio-ccw.c
> >   
> 
> OK, sure.
> 
> Matt
>