diff mbox

[v4,5/5] s390x/cpu: Allow hotplug of CPUs

Message ID 1455739955-28139-6-git-send-email-mjrosato@linux.vnet.ibm.com
State New
Headers show

Commit Message

Matthew Rosato Feb. 17, 2016, 8:12 p.m. UTC
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         | 40 ++++++++++++++++++++++++++++++++++++++++
 target-s390x/cpu.h         |  1 +
 3 files changed, 42 insertions(+)

Comments

David Hildenbrand Feb. 18, 2016, 9:03 a.m. UTC | #1
> 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         | 40 ++++++++++++++++++++++++++++++++++++++++
>  target-s390x/cpu.h         |  1 +
>  3 files changed, 42 insertions(+)
> 
> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
> index b05ed8b..a15904d 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 620b2e3..b4c59c9 100644
> --- a/target-s390x/cpu.c
> +++ b/target-s390x/cpu.c
> @@ -33,6 +33,8 @@
>  #ifndef CONFIG_USER_ONLY
>  #include "sysemu/arch_init.h"
>  #include "sysemu/sysemu.h"
> +#include "hw/boards.h"
> +#include "hw/s390x/sclp.h"
>  #endif
>  
>  #define CR0_RESET       0xE0UL
> @@ -226,6 +228,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)
> @@ -269,6 +277,38 @@ 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);
> +}
> +#endif
> +
> +#if !defined(CONFIG_USER_ONLY)

This #if block could be combined with the previous one. But it's not worth
blocking this series so

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

As we have a clean interface now (cpu-add id=x) most of this series is internal
stuff, so I think this is ready to be picked up :) .

David
Igor Mammedov Feb. 18, 2016, 9:41 a.m. UTC | #2
On Wed, 17 Feb 2016 15:12:35 -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         | 40 ++++++++++++++++++++++++++++++++++++++++
>  target-s390x/cpu.h         |  1 +
>  3 files changed, 42 insertions(+)
> 
> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
> index b05ed8b..a15904d 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 620b2e3..b4c59c9 100644
> --- a/target-s390x/cpu.c
> +++ b/target-s390x/cpu.c
> @@ -33,6 +33,8 @@
>  #ifndef CONFIG_USER_ONLY
>  #include "sysemu/arch_init.h"
>  #include "sysemu/sysemu.h"
> +#include "hw/boards.h"
> +#include "hw/s390x/sclp.h"
>  #endif
>  
>  #define CR0_RESET       0xE0UL
> @@ -226,6 +228,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)
> @@ -269,6 +277,38 @@ static void s390_cpu_finalize(Object *obj)
>  }
>  
>  #if !defined(CONFIG_USER_ONLY)
> +void s390_hot_add_cpu(const int64_t id, Error **errp)
> +{
to make it future-proof wrt migration it could be better to
enforce here that 'id' grows in +1 steps so user
won't be able create cpus with gaps.


> +    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);
> +}
> +#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 429bf90..3752932 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);
David Hildenbrand Feb. 18, 2016, 9:46 a.m. UTC | #3
> >  #if !defined(CONFIG_USER_ONLY)
> > +void s390_hot_add_cpu(const int64_t id, Error **errp)
> > +{  
> to make it future-proof wrt migration it could be better to
> enforce here that 'id' grows in +1 steps so user
> won't be able create cpus with gaps.

That should be already covered by:

if (id != next_cpu_id)
...

or am I missing something?

David
Igor Mammedov Feb. 18, 2016, 5:59 p.m. UTC | #4
On Thu, 18 Feb 2016 10:46:52 +0100
David Hildenbrand <dahi@linux.vnet.ibm.com> wrote:

> > >  #if !defined(CONFIG_USER_ONLY)
> > > +void s390_hot_add_cpu(const int64_t id, Error **errp)
> > > +{    
> > to make it future-proof wrt migration it could be better to
> > enforce here that 'id' grows in +1 steps so user
> > won't be able create cpus with gaps.  
> 
> That should be already covered by:
> 
> if (id != next_cpu_id)
> ...
> 
> or am I missing something?
yep, that should do the job, but it might be better to introduce
next_cpu_id in this patch so it would be clear were it comes from
and how it's used while dropping it completely from previous one.

essentially next_cpu_id is (cpu_index + 1) so you don't even need
to create a global var, just get the last CPU and to math,
something like that:
include/qom/cpu.h:
+#define last_cpu QTAILQ_LAST(&cpus)
 
if (id != (last_cpu->cpu_index + 1)) { .... }

> 
> David
>
diff mbox

Patch

diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index b05ed8b..a15904d 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 620b2e3..b4c59c9 100644
--- a/target-s390x/cpu.c
+++ b/target-s390x/cpu.c
@@ -33,6 +33,8 @@ 
 #ifndef CONFIG_USER_ONLY
 #include "sysemu/arch_init.h"
 #include "sysemu/sysemu.h"
+#include "hw/boards.h"
+#include "hw/s390x/sclp.h"
 #endif
 
 #define CR0_RESET       0xE0UL
@@ -226,6 +228,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)
@@ -269,6 +277,38 @@  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);
+}
+#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 429bf90..3752932 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);