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

[Matthew Rosato <mjrosato@linux.vnet.ibm.com>]

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