Patchwork [20/21] target-i386: implement machine->hot_add_cpu hook

login
register
mail settings
Submitter Igor Mammedov
Date April 23, 2013, 8:29 a.m.
Message ID <1366705795-24732-21-git-send-email-imammedo@redhat.com>
Download mbox | patch
Permalink /patch/238799/
State New
Headers show

Comments

Igor Mammedov - April 23, 2013, 8:29 a.m.
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
 hw/i386/pc.c |   22 ++++++++++++++++++++++
 1 files changed, 22 insertions(+), 0 deletions(-)
Andreas Färber - April 24, 2013, 5:31 p.m.
Am 23.04.2013 10:29, schrieb Igor Mammedov:
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> ---
>  hw/i386/pc.c |   22 ++++++++++++++++++++++
>  1 files changed, 22 insertions(+), 0 deletions(-)
> 
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 5e50127..b649ed5 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -54,6 +54,7 @@
>  #include "qemu/config-file.h"
>  #include "hw/acpi/acpi.h"
>  #include "hw/cpu/icc_bus.h"
> +#include "hw/boards.h"
>  
>  /* debug PC/ISA interrupts */
>  //#define DEBUG_IRQ
> @@ -914,6 +915,25 @@ static X86CPU *pc_new_cpu(const char *cpu_model, int64_t apic_id, Error **errp)
>      return cpu;
>  }
>  
> +static void do_cpu_hot_add(const int64_t id, Error **errp)
> +{
> +    int64_t apic_id = x86_cpu_apic_id_from_index(id);
> +
> +    if (cpu_exists(apic_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);

Why -1?

> +        return;
> +    }
> +
> +    pc_new_cpu(machine_args->cpu_model, apic_id, errp);
> +}
> +
>  void pc_cpus_init(const char *cpu_model)
>  {
>      int i;
> @@ -928,7 +948,9 @@ void pc_cpus_init(const char *cpu_model)
>  #else
>          cpu_model = "qemu32";
>  #endif
> +        machine_args->cpu_model = cpu_model;

This could be avoided by changing argument to const char **. :)
Caller will have access to QEMUMachineArgs*.

>      }
> +    current_machine->hot_add_cpu = do_cpu_hot_add;

That kind of answers my previous question, but why can't we have this
statically in the QEMUMachine(s)?

Andreas

>  
>      icc_bridge = SYS_BUS_DEVICE(object_resolve_path_type("icc-bridge",
>                                                   TYPE_ICC_BRIDGE, NULL));
>
Eduardo Habkost - April 24, 2013, 7:14 p.m.
On Wed, Apr 24, 2013 at 07:31:44PM +0200, Andreas Färber wrote:
> > +static void do_cpu_hot_add(const int64_t id, Error **errp)
> > +{
> > +    int64_t apic_id = x86_cpu_apic_id_from_index(id);
> > +
> > +    if (cpu_exists(apic_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);
> 
> Why -1?

max_cpus-1 makes sense to me, as the message is about "maximum ID
allowed", not "maximum number of CPUs".

> 
> > +        return;
> > +    }
> > +
> > +    pc_new_cpu(machine_args->cpu_model, apic_id, errp);
> > +}
> > +
> >  void pc_cpus_init(const char *cpu_model)
> >  {
> >      int i;
> > @@ -928,7 +948,9 @@ void pc_cpus_init(const char *cpu_model)
> >  #else
> >          cpu_model = "qemu32";
> >  #endif
> > +        machine_args->cpu_model = cpu_model;
> 
> This could be avoided by changing argument to const char **. :)
> Caller will have access to QEMUMachineArgs*.

I would prefer to simply pass a pointer to QEMUMachineArgs to
pc_cpus_init(), instead of const char **.

Patch

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 5e50127..b649ed5 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -54,6 +54,7 @@ 
 #include "qemu/config-file.h"
 #include "hw/acpi/acpi.h"
 #include "hw/cpu/icc_bus.h"
+#include "hw/boards.h"
 
 /* debug PC/ISA interrupts */
 //#define DEBUG_IRQ
@@ -914,6 +915,25 @@  static X86CPU *pc_new_cpu(const char *cpu_model, int64_t apic_id, Error **errp)
     return cpu;
 }
 
+static void do_cpu_hot_add(const int64_t id, Error **errp)
+{
+    int64_t apic_id = x86_cpu_apic_id_from_index(id);
+
+    if (cpu_exists(apic_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;
+    }
+
+    pc_new_cpu(machine_args->cpu_model, apic_id, errp);
+}
+
 void pc_cpus_init(const char *cpu_model)
 {
     int i;
@@ -928,7 +948,9 @@  void pc_cpus_init(const char *cpu_model)
 #else
         cpu_model = "qemu32";
 #endif
+        machine_args->cpu_model = cpu_model;
     }
+    current_machine->hot_add_cpu = do_cpu_hot_add;
 
     icc_bridge = SYS_BUS_DEVICE(object_resolve_path_type("icc-bridge",
                                                  TYPE_ICC_BRIDGE, NULL));