diff mbox

[v5,6/6] s390x/cpu: Allow hotplug of CPUs

Message ID 1455854617-30964-7-git-send-email-mjrosato@linux.vnet.ibm.com
State New
Headers show

Commit Message

Matthew Rosato Feb. 19, 2016, 4:03 a.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         | 45 +++++++++++++++++++++++++++++++++++++++++++++
 target-s390x/cpu.h         |  1 +
 3 files changed, 47 insertions(+)

Comments

Igor Mammedov Feb. 19, 2016, 2:28 p.m. UTC | #1 
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);
Matthew Rosato Feb. 19, 2016, 2:40 p.m. UTC | #2 
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
Igor Mammedov Feb. 19, 2016, 4:18 p.m. UTC | #3 
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
>
diff mbox

Patch

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);