qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v6 7/7] s390x/cpu: Allow hotplug of CPUs


From: Andreas Färber
Subject: Re: [Qemu-devel] [PATCH v6 7/7] s390x/cpu: Allow hotplug of CPUs
Date: Mon, 22 Feb 2016 18:22:42 +0100
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.6.0

Am 22.02.2016 um 18:06 schrieb Matthew Rosato:
> Implement cpu hotplug routine and add the machine hook.
> 
> Signed-off-by: Matthew Rosato <address@hidden>
> ---
>  hw/s390x/s390-virtio-ccw.c | 33 +++++++++++++++++++++++++++++++++
>  target-s390x/cpu.c         |  7 +++++++
>  2 files changed, 40 insertions(+)
> 
> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
> index 3090e76..ea007e8 100644
> --- a/hw/s390x/s390-virtio-ccw.c
> +++ b/hw/s390x/s390-virtio-ccw.c
> @@ -22,6 +22,7 @@
>  #include "s390-pci-bus.h"
>  #include "hw/s390x/storage-keys.h"
>  #include "hw/compat.h"
> +#include "qom/cpu.h"
>  
>  #define TYPE_S390_CCW_MACHINE               "s390-ccw-machine"
>  
> @@ -185,6 +186,37 @@ static HotplugHandler 
> *s390_get_hotplug_handler(MachineState *machine,
>      return NULL;
>  }
>  
> +static 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)) {

This assumption about the order of the CPU list looks dangerous to me.
Aren't there global int fields used by x86 already for the number of
CPUs that you could use instead? That will allow you to drop the ugly
preceding renaming patch. Please avoid messing with the CPU list
directly from target code.

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

No proper error handling - that's why blindly reusing these old helpers
is a bad idea.

Regards,
Andreas

> +}
> +
>  static void ccw_machine_class_init(ObjectClass *oc, void *data)
>  {
>      MachineClass *mc = MACHINE_CLASS(oc);
> @@ -193,6 +225,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..1a4977e 100644
> --- a/target-s390x/cpu.c
> +++ b/target-s390x/cpu.c
> @@ -32,6 +32,7 @@
>  #include "trace.h"
>  #ifndef CONFIG_USER_ONLY
>  #include "sysemu/arch_init.h"
> +#include "hw/s390x/sclp.h"
>  #endif
>  
>  #define CR0_RESET       0xE0UL
> @@ -211,6 +212,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)
> 


-- 
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Graham Norton; HRB 21284 (AG Nürnberg)



reply via email to

[Prev in Thread] Current Thread [Next in Thread]