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

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

[Matthew Rosato]

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

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 (4):
  s390x/cpu: Cleanup init in preparation for hotplug
  s390x/cpu: Set initial CPU state in common routine
  s390x/cpu: Add function to set CPU state
  s390x/cpu: Allow hotplug of CPUs

 hw/s390x/s390-virtio-ccw.c |  3 ++-
 hw/s390x/s390-virtio.c     | 41 +++++++++++++++++++++++----------------
 hw/s390x/s390-virtio.h     |  2 +-
 target-s390x/cpu.c         | 48 ++++++++++++++++++++++++++++++++++++++++++++--
 target-s390x/cpu.h         |  2 ++
 5 files changed, 75 insertions(+), 21 deletions(-)

-- 
1.9.1

[Qemu-devel] [PATCH 1/4] 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 84221f4..7561e18 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 cbde977..496898c 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;
@@ -297,7 +297,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 2/4] 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 496898c..b9cf058 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 3/4] s390x/cpu: Add function to set CPU state

[Matthew Rosato]

Introduce s390_cpu_set_cpustate, which will set/update 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>
Acked-by: David Hildenbrand <dahi@linux.vnet.ibm.com>
---
 hw/s390x/s390-virtio.c | 29 ++++++++++++++++++++---------
 target-s390x/cpu.c     |  3 ++-
 target-s390x/cpu.h     |  1 +
 3 files changed, 23 insertions(+), 10 deletions(-)

diff --git a/hw/s390x/s390-virtio.c b/hw/s390x/s390-virtio.c
index b9cf058..ea867e0 100644
--- a/hw/s390x/s390-virtio.c
+++ b/hw/s390x/s390-virtio.c
@@ -62,15 +62,30 @@
 #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];
+}
+
+void s390_cpu_set_cpustate(uint16_t cpu_addr, S390CPU *state)
+{
+    gchar *name;
+
+    cpu_states[cpu_addr] = state;
+    name = g_strdup_printf("cpu[%i]", cpu_addr);
+    object_property_add_link(qdev_get_machine(), name, TYPE_S390_CPU,
+                             (Object **) &cpu_states[cpu_addr],
+                             object_property_allow_set_link,
+                             OBJ_PROP_LINK_UNREF_ON_RELEASE,
+                             &error_abort);
+    g_free(name);
 }
 
 static int s390_virtio_hcall_notify(const uint64_t *args)
@@ -176,14 +191,10 @@ void s390_init_cpus(MachineState *machine)
         machine->cpu_model = "host";
     }
 
-    ipi_states = g_malloc(sizeof(S390CPU *) * smp_cpus);
+    cpu_states = g_malloc(sizeof(S390CPU *) * smp_cpus);
 
     for (i = 0; i < smp_cpus; i++) {
-        S390CPU *cpu;
-
-        cpu = cpu_s390x_init(machine->cpu_model);
-
-        ipi_states[i] = cpu;
+        cpu_s390x_init(machine->cpu_model);
     }
 }
 
diff --git a/target-s390x/cpu.c b/target-s390x/cpu.c
index aafbbdc..3228c3b 100644
--- a/target-s390x/cpu.c
+++ b/target-s390x/cpu.c
@@ -221,6 +221,7 @@ static void s390_cpu_initfn(Object *obj)
     cs->halted = 1;
     cs->exception_index = EXCP_HLT;
     cpu_exec_init(cs, &error_abort);
+    env->cpu_num = cpu_num++;
 #if !defined(CONFIG_USER_ONLY)
     qemu_register_reset(s390_cpu_machine_reset_cb, cpu);
     qemu_get_timedate(&tm, 0);
@@ -230,8 +231,8 @@ static void s390_cpu_initfn(Object *obj)
     env->tod_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, s390x_tod_timer, cpu);
     env->cpu_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, s390x_cpu_timer, cpu);
     s390_cpu_set_state(CPU_STATE_STOPPED, cpu);
+    s390_cpu_set_cpustate(env->cpu_num, cpu);
 #endif
-    env->cpu_num = cpu_num++;
 
     if (tcg_enabled() && !inited) {
         inited = true;
diff --git a/target-s390x/cpu.h b/target-s390x/cpu.h
index 658cd9d..803841b 100644
--- a/target-s390x/cpu.h
+++ b/target-s390x/cpu.h
@@ -531,6 +531,7 @@ static inline int s390_set_clock(uint8_t *tod_high, uint64_t *tod_low)
 }
 
 S390CPU *s390_cpu_addr2state(uint16_t cpu_addr);
+void s390_cpu_set_cpustate(uint16_t cpu_addr, S390CPU *state);
 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 4/4] 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>
Reviewed-by: David Hildenbrand <dahi@linux.vnet.ibm.com>
---
 hw/s390x/s390-virtio-ccw.c |  1 +
 target-s390x/cpu.c         | 45 +++++++++++++++++++++++++++++++++++++++++++--
 target-s390x/cpu.h         |  1 +
 3 files changed, 45 insertions(+), 2 deletions(-)

diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index 7561e18..cfcde8d 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -163,6 +163,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 3228c3b..48ecd34 100644
--- a/target-s390x/cpu.c
+++ b/target-s390x/cpu.c
@@ -31,11 +31,17 @@
 #include "trace.h"
 #ifndef CONFIG_USER_ONLY
 #include "sysemu/arch_init.h"
+#include "hw/boards.h"
+#include "hw/s390x/sclp.h"
+#include "sysemu/sysemu.h"
 #endif
 
 #define CR0_RESET       0xE0UL
 #define CR14_RESET      0xC2000000UL;
 
+/* Older kernels require CPUs to be added sequentially by id */
+int next_cpu_id;
+
 /* generate CPU information for cpu -? */
 void s390_cpu_list(FILE *f, fprintf_function cpu_fprintf)
 {
@@ -204,6 +210,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)
@@ -212,7 +224,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
@@ -221,7 +232,7 @@ static void s390_cpu_initfn(Object *obj)
     cs->halted = 1;
     cs->exception_index = EXCP_HLT;
     cpu_exec_init(cs, &error_abort);
-    env->cpu_num = cpu_num++;
+    env->cpu_num = next_cpu_id++;
 #if !defined(CONFIG_USER_ONLY)
     qemu_register_reset(s390_cpu_machine_reset_cb, cpu);
     qemu_get_timedate(&tm, 0);
@@ -251,6 +262,36 @@ 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 != next_cpu_id) {
+        error_setg(errp, "Unable to add CPU: %" PRIi64
+                   ", The next available id is %d", id, next_cpu_id);
+        return;
+    }
+
+    cpu_s390x_init(machine->cpu_model);
+}
+
 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 803841b..99daf79 100644
--- a/target-s390x/cpu.h
+++ b/target-s390x/cpu.h
@@ -542,6 +542,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 0/4] s390: Allow hotplug of s390 CPUs

[Andreas Färber]

Hi,

Am 09.11.2015 um 16:17 schrieb Matthew Rosato:
> 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.

What exactly is still missing for you to use the standard device_add?

Last time I checked (a while ago...) some patches were stuck on the x86
side, and I don't recall hearing any feedback from the s390x side in my
KVM Forum CPU hotplug session.

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 0/4] s390: Allow hotplug of s390 CPUs

[Christian Borntraeger]

Am 09.11.2015 um 16:28 schrieb Andreas Färber:
> Hi,
> 
> Am 09.11.2015 um 16:17 schrieb Matthew Rosato:
>> 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.
> 
> What exactly is still missing for you to use the standard device_add?
> 
> Last time I checked (a while ago...) some patches were stuck on the x86
> side, and I don't recall hearing any feedback from the s390x side in my
> KVM Forum CPU hotplug session.

libvirt uses "cpu-add" unconditionally for hotplugging, so we certainly 
want to support that. 

Re: [Qemu-devel] [PATCH 0/4] s390: Allow hotplug of s390 CPUs

[Christian Borntraeger]

Am 09.11.2015 um 16:35 schrieb Christian Borntraeger:
> Am 09.11.2015 um 16:28 schrieb Andreas Färber:
>> Hi,
>>
>> Am 09.11.2015 um 16:17 schrieb Matthew Rosato:
>>> 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.
>>
>> What exactly is still missing for you to use the standard device_add?
>>
>> Last time I checked (a while ago...) some patches were stuck on the x86
>> side, and I don't recall hearing any feedback from the s390x side in my
>> KVM Forum CPU hotplug session.
> 
> libvirt uses "cpu-add" unconditionally for hotplugging, so we certainly 
> want to support that. 

Sorry, hit send too early. I assume you want us to support device_add of
a CPU in addition to that. Correct? 

Re: [Qemu-devel] [PATCH 0/4] s390: Allow hotplug of s390 CPUs

[Christian Borntraeger]

Am 09.11.2015 um 16:37 schrieb Christian Borntraeger:
> Am 09.11.2015 um 16:35 schrieb Christian Borntraeger:
>> Am 09.11.2015 um 16:28 schrieb Andreas Färber:
>>> Hi,
>>>
>>> Am 09.11.2015 um 16:17 schrieb Matthew Rosato:
>>>> 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.
>>>
>>> What exactly is still missing for you to use the standard device_add?
>>>
>>> Last time I checked (a while ago...) some patches were stuck on the x86
>>> side, and I don't recall hearing any feedback from the s390x side in my
>>> KVM Forum CPU hotplug session.
>>
>> libvirt uses "cpu-add" unconditionally for hotplugging, so we certainly 
>> want to support that. 
> 
> Sorry, hit send too early. I assume you want us to support device_add of
> a CPU in addition to that. Correct? 

I just did a quick test with Matts patch.

after applying

diff --git a/qom/cpu.c b/qom/cpu.c
index fb80d13..8152744 100644
--- a/qom/cpu.c
+++ b/qom/cpu.c
@@ -360,7 +360,7 @@ static void cpu_class_init(ObjectClass *klass, void *data)
      * Reason: CPUs still need special care by board code: wiring up
      * IRQs, adding reset handlers, halting non-first CPUs, ...
      */
-    dc->cannot_instantiate_with_device_add_yet = true;
+//    dc->cannot_instantiate_with_device_add_yet = true;
 }
 
 static const TypeInfo cpu_type_info = {

I was able to use

device_add s390-cpu

but no comprehensive testing was done.
I was even able to add more cpus than maxcpus, but the guest could not
make use of it. Fix is probably easy. But in general it looks like
that we _could_ enable device_add for CPUs as soon as others are ready
as well.
Matt, do you agree?



Christian

Re: [Qemu-devel] [PATCH 0/4] s390: Allow hotplug of s390 CPUs

[Andreas Färber]

Am 09.11.2015 um 16:37 schrieb Christian Borntraeger:
> Am 09.11.2015 um 16:35 schrieb Christian Borntraeger:
>> Am 09.11.2015 um 16:28 schrieb Andreas Färber:
>>> Hi,
>>>
>>> Am 09.11.2015 um 16:17 schrieb Matthew Rosato:
>>>> 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.
>>>
>>> What exactly is still missing for you to use the standard device_add?
>>>
>>> Last time I checked (a while ago...) some patches were stuck on the x86
>>> side, and I don't recall hearing any feedback from the s390x side in my
>>> KVM Forum CPU hotplug session.
>>
>> libvirt uses "cpu-add" unconditionally for hotplugging, so we certainly 
>> want to support that. 
> 
> Sorry, hit send too early. I assume you want us to support device_add of
> a CPU in addition to that. Correct? 

No, I mean as I've always said: cpu-add was a short-term hack to make
CPU hotplug work on x86. libvirt certainly needs to be fixed if it
assumes this to work for s390.

If device_add works, we don't need cpu-add. If device_add does not work,
we need to know which patches are missing. device_del is a separate
beast and there were also patches around that seemed incomplete
according to Eduardo.

Bharata did implement device_add for pseries, I thought.

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 0/4] s390: Allow hotplug of s390 CPUs

[Matthew Rosato]

On 11/09/2015 10:55 AM, Christian Borntraeger wrote:
> Am 09.11.2015 um 16:37 schrieb Christian Borntraeger:
>> Am 09.11.2015 um 16:35 schrieb Christian Borntraeger:
>>> Am 09.11.2015 um 16:28 schrieb Andreas Färber:
>>>> Hi,
>>>>
>>>> Am 09.11.2015 um 16:17 schrieb Matthew Rosato:
>>>>> 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.
>>>>
>>>> What exactly is still missing for you to use the standard device_add?
>>>>
>>>> Last time I checked (a while ago...) some patches were stuck on the x86
>>>> side, and I don't recall hearing any feedback from the s390x side in my
>>>> KVM Forum CPU hotplug session.
>>>
>>> libvirt uses "cpu-add" unconditionally for hotplugging, so we certainly 
>>> want to support that. 
>>
>> Sorry, hit send too early. I assume you want us to support device_add of
>> a CPU in addition to that. Correct? 
> 
> I just did a quick test with Matts patch.
> 
> after applying
> 
> diff --git a/qom/cpu.c b/qom/cpu.c
> index fb80d13..8152744 100644
> --- a/qom/cpu.c
> +++ b/qom/cpu.c
> @@ -360,7 +360,7 @@ static void cpu_class_init(ObjectClass *klass, void *data)
>       * Reason: CPUs still need special care by board code: wiring up
>       * IRQs, adding reset handlers, halting non-first CPUs, ...
>       */
> -    dc->cannot_instantiate_with_device_add_yet = true;
> +//    dc->cannot_instantiate_with_device_add_yet = true;
>  }
> 
>  static const TypeInfo cpu_type_info = {
> 
> I was able to use
> 
> device_add s390-cpu
> 
> but no comprehensive testing was done.
> I was even able to add more cpus than maxcpus, but the guest could not
> make use of it. Fix is probably easy. But in general it looks like
> that we _could_ enable device_add for CPUs as soon as others are ready
> as well.
> Matt, do you agree?
> 

Yes, I'm seeing the same behavior.  We're just not checking max_cpus in
device init, but the "extra" CPUs cannot be brought online.

Matt

Re: [Qemu-devel] [PATCH 0/4] s390: Allow hotplug of s390 CPUs

[Christian Borntraeger]

Am 09.11.2015 um 16:56 schrieb Andreas Färber:
> Am 09.11.2015 um 16:37 schrieb Christian Borntraeger:
>> Am 09.11.2015 um 16:35 schrieb Christian Borntraeger:
>>> Am 09.11.2015 um 16:28 schrieb Andreas Färber:
>>>> Hi,
>>>>
>>>> Am 09.11.2015 um 16:17 schrieb Matthew Rosato:
>>>>> 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.
>>>>
>>>> What exactly is still missing for you to use the standard device_add?
>>>>
>>>> Last time I checked (a while ago...) some patches were stuck on the x86
>>>> side, and I don't recall hearing any feedback from the s390x side in my
>>>> KVM Forum CPU hotplug session.
>>>
>>> libvirt uses "cpu-add" unconditionally for hotplugging, so we certainly 
>>> want to support that. 
>>
>> Sorry, hit send too early. I assume you want us to support device_add of
>> a CPU in addition to that. Correct? 
> 
> No, I mean as I've always said: cpu-add was a short-term hack to make
> CPU hotplug work on x86. libvirt certainly needs to be fixed if it
> assumes this to work for s390.
> 
> If device_add works, we don't need cpu-add. If device_add does not work,
> we need to know which patches are missing. device_del is a separate
> beast and there were also patches around that seemed incomplete
> according to Eduardo.

Given that s390 plugs CPUs without sockets, it would boild down to an opaque
cpu, so
- Matt will remove the hot_add function from patch 4 (which makes it smaller)
- we provide a followup patch for libvirt
- Matt fixes the maxcpus overrun
- In addition we would need something like 
diff --git a/target-s390x/cpu.c b/target-s390x/cpu.c
index 266575c..cfcff52 100644
--- a/target-s390x/cpu.c
+++ b/target-s390x/cpu.c
@@ -377,7 +377,7 @@ static void s390_cpu_class_init(ObjectClass *oc, void *data)
 
     scc->parent_realize = dc->realize;
     dc->realize = s390_cpu_realizefn;
-
+    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

With that device_add s390-cpu seems to work. 

> Bharata did implement device_add for pseries, I thought.

Seems that the patches did not make it into upstream yet.
Bharata, is cpu hotplug on pseries still missing?


Christian

Re: [Qemu-devel] [PATCH 0/4] s390: Allow hotplug of s390 CPUs

[Bharata B Rao]

On Mon, Nov 09, 2015 at 09:04:27PM +0100, Christian Borntraeger wrote:
> 
> > Bharata did implement device_add for pseries, I thought.

Yes. For pseries, my patchset did CPU hotplug via device_add and
device_del commands. And that's what I am planning to stick to
going forward.

> 
> Seems that the patches did not make it into upstream yet.
> Bharata, is cpu hotplug on pseries still missing?

Yes, the last version I posted is at
https://lists.gnu.org/archive/html/qemu-devel/2015-08/msg00650.html

Andreas mentioned that he would like to see some generic changes for
supporting device_add for CPU, socket vs core level hotplug, defining the
semantics etc. There was some discussion towards that and this is last I
heard about it:

https://lists.nongnu.org/archive/html/qemu-devel/2015-10/msg05347.html

Regards,
Bharata.