[Qemu-devel] [PATCH 0/5 v9 for-1.5] target-i386: CPU hot-add with cpu-add QMP command

[Qemu-devel] [PATCH 0/5 v9 for-1.5] target-i386: CPU hot-add with cpu-add QMP command

[Igor Mammedov]

Implements alternative way for hot-adding CPU using cpu-add QMP command,
which could be useful until it would be possible to add CPUs via device_add.

To hot-add CPU use following command from qmp-shell:
 cpu-add id=[0..max-cpus - 1)

git tree for testing: https://github.com/imammedo/qemu/tree/cpu_add.v9

based on qom-cpu tree

v9->v7:
  * replace machine.cpu-model option with cpu_model field in QEMUMachine
  * x86: set default cpu_model statically in each machine and get it
    from there if user haven't provided it.

v8->v7:
  * skip already applied patches
  * split adding hot_add_cpu hook into separate function
  * add cpu-model machine option and use QemuOpts for getting it
    during CPU hotplug.

v7->v6:
  * skip already applied patches
  * rename icc-bus instance name to "icc"
  * pass icc_bridge from board as argument down CPU creation call chain,
    instead of dynamically resolving it for each CPU.

v6->v5:
  * override hot_add_cpu hook statically
  * extend and use memory_region_find() in IOAPIC
  * s/signal_cpu_creation/tcg_signal_cpu_creation/
  * add "since 1.5 to cpu-addQAPI schema description

v5->v4:
  * style fixes
  * new helper qemu_for_each_cpu()
  * switch to qemu_for_each_cpu() in cpu_exists()
  * "pc: update rtc ..." patch make depend it on "mc146818rtc: QOM'ify"
    and use QOM cast style
  * call CPU added notifier right before CPU becomes runable
  * s/resume_vcpu/cpu_resume/
  * acpi/piix4: add spec documentation for QEMU<->Seabios CPU hotplug
    interface
  * use error_propagate() in pc_new_cpu()
  * skip cpu_exists() check in apic-id property setter if new value is
    the same as current
  * embed icc-bus inside icc-bridge and use qbus_create_inplace()
  * move include/hw/i386/icc_bus.h into include/hw/cpu/
  * make missing icc-bus fatal error for softmmu target
  * split "move APIC to ICC bus" and "move IOAPIC to ICC bus" on smaller
    patches
  * use qdev_get_parent_bus() to get parent bus
  * split "add cpu-add command..." on smaller patches

v4->v3:
  * 'id' in cpu-add command will be a thread number instead of APIC ID
  * split off resume_vcpu() into separate patch
  * move notifier from rtc code into pc.c

v2->v3:
  * use local error & propagate_error() instead of operating on
    passed in errp in several places
  * replace CPUClass.get_firmware_id() with CPUClass.get_arch_id()
  * leave IOAPIC creation to board and just set bus to icc-bus
  * include kvm-stub.o in cpu libary if no KVM is configured
  * create resume_vcpu() stub and include it in libqemustub,
    and use it directly instead of CPU method
  * acpi_piix4: s/cpu_add_notifier/cpu_added_notifier/

v1->v2:
  * generalize cpu sync to KVM, resume and hot-plug notification and
    invoke them form CPUClass, to make available to all targets.
  * introduce cpu_exists() and CPUClass.get_firmware_id() and use
    the last one in acpi_piix to make code target independent.
  * move IOAPIC to ICC bus, it was suggested and easy to convert.
  * leave kvmvapic as SysBusDevice, it doesn't affect hot-plug and
    created only once for all APIC instances. I haven't found yet
    good/clean enough way to convert it to ICCDevice. May be follow-up
    though.
  * split one big ICC patch into several, one per converted device
  * add cpu_hot_add hook to machine and implement it for target-i386,
    instead of adding stabs. Could be used by other targets to
    implement cpu-add.
  * pre-allocate links<CPU> for all possible CPUs and make them available
    at /machine/icc-bridge/cpu[0..N] QOM path, so users could find out
    possible/free CPU IDs to use in cpu-add command.



Igor Mammedov (5):
  add hot_add_cpu hook to QEMUMachine
  QMP: add cpu-add command
  add cpu_model to QEMUMachine
  target-i386: get default cpu_model from QEMUMachine
  target-i386: implement machine->hot_add_cpu hook

 hw/i386/pc.c         |   32 +++++++++++++++++++++++---------
 hw/i386/pc_piix.c    |   17 +++++++++++++++--
 hw/i386/pc_q35.c     |    3 +++
 include/hw/boards.h  |    2 ++
 include/hw/i386/pc.h |    1 +
 qapi-schema.json     |   13 +++++++++++++
 qmp-commands.hx      |   23 +++++++++++++++++++++++
 qmp.c                |   10 ++++++++++
 target-i386/cpu.h    |    6 ++++++
 vl.c                 |    6 ++++++
 10 files changed, 102 insertions(+), 11 deletions(-)

[Qemu-devel] [PATCH 1/5] add hot_add_cpu hook to QEMUMachine

[Igor Mammedov]

Hook should be set by target that implements
CPU hot-add via cpu-add QMP command.

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
 include/hw/boards.h |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/include/hw/boards.h b/include/hw/boards.h
index 425bdc7..75cd127 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -43,6 +43,7 @@ typedef struct QEMUMachine {
     GlobalProperty *compat_props;
     struct QEMUMachine *next;
     const char *hw_version;
+    void (*hot_add_cpu)(const int64_t id, Error **errp);
 } QEMUMachine;
 
 int qemu_register_machine(QEMUMachine *m);
-- 
1.7.1

[Qemu-devel] [PATCH 2/5] QMP: add cpu-add command

[Igor Mammedov]

Adds "cpu-add id=xxx" QMP command.

cpu-add's "id" argument is a CPU number in a range [0..max-cpus)

Example QMP command:
 -> { "execute": "cpu-add", "arguments": { "id": 2 } }
 <- { "return": {} }

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Acked-by: Luiz Capitulino <lcapitulino@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
v7:
  * added "Since 1.5" to cpu-add qapi schema definition
v6:
  * added valid values description to qapi schema
  * split out cpu_hot_add hooks introduction into separate patch
  * split out implementation of cpu_hot_add for target-i386
v5:
  * accept id=[0..max_cpus) range in cpu-add command
v4:
  * merge "qmp: add cpu-add qmp command" & "target-i386: implement CPU hot-add" patches
  * move notifier call to CPUCLass.realize()
  * add hook cpu_hot_add to QEMUMachine
  * make QEMUMachineInitArgs global and keep default cpu_model there

v3:
  * it appears that 'online/offline' in cpu-set are confusing people
    with what command actually does and users might have to distinguish
    if 'offline' is not implemented by parsing error message. To simplify
    things replace cpu-set with cpu-add command to show more clear what
    command does and just add cpu-del when CPU remove is implemented.

v2:
  * s/cpu_set/cpu-set/
  * qmp doc style fix
  * use bool type instead of opencodding online/offline string
     suggested-by: Eric Blake <eblake@redhat.com>
---
 qapi-schema.json |   13 +++++++++++++
 qmp-commands.hx  |   23 +++++++++++++++++++++++
 qmp.c            |   10 ++++++++++
 3 files changed, 46 insertions(+), 0 deletions(-)

diff --git a/qapi-schema.json b/qapi-schema.json
index 5b0fb3b..6f58b0f 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -1387,6 +1387,19 @@
 { 'command': 'cpu', 'data': {'index': 'int'} }
 
 ##
+# @cpu-add
+#
+# Adds CPU with specified ID
+#
+# @id: ID of CPU to be created, valid values [0..max_cpus)
+#
+# Returns: Nothing on success
+#
+# Since 1.5
+##
+{ 'command': 'cpu-add', 'data': {'id': 'int'} }
+
+##
 # @memsave:
 #
 # Save a portion of guest memory to a file.
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 0e89132..ed99eb8 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -385,6 +385,29 @@ Note: CPUs' indexes are obtained with the 'query-cpus' command.
 EQMP
 
     {
+        .name       = "cpu-add",
+        .args_type  = "id:i",
+        .mhandler.cmd_new = qmp_marshal_input_cpu_add,
+    },
+
+SQMP
+cpu-add
+-------
+
+Adds virtual cpu
+
+Arguments:
+
+- "id": cpu id (json-int)
+
+Example:
+
+-> { "execute": "cpu-add", "arguments": { "id": 2 } }
+<- { "return": {} }
+
+EQMP
+
+    {
         .name       = "memsave",
         .args_type  = "val:l,size:i,filename:s,cpu:i?",
         .mhandler.cmd_new = qmp_marshal_input_memsave,
diff --git a/qmp.c b/qmp.c
index ed6c7ef..dd34be6 100644
--- a/qmp.c
+++ b/qmp.c
@@ -24,6 +24,7 @@
 #include "hw/qdev.h"
 #include "sysemu/blockdev.h"
 #include "qom/qom-qobject.h"
+#include "hw/boards.h"
 
 NameInfo *qmp_query_name(Error **errp)
 {
@@ -108,6 +109,15 @@ void qmp_cpu(int64_t index, Error **errp)
     /* Just do nothing */
 }
 
+void qmp_cpu_add(int64_t id, Error **errp)
+{
+    if (current_machine->hot_add_cpu) {
+        current_machine->hot_add_cpu(id, errp);
+    } else {
+        error_setg(errp, "Not supported");
+    }
+}
+
 #ifndef CONFIG_VNC
 /* If VNC support is enabled, the "true" query-vnc command is
    defined in the VNC subsystem */
-- 
1.7.1

[Qemu-devel] [PATCH 3/5] add cpu_model to QEMUMachine

[Igor Mammedov]

1. Use default cpu_model from machine if user haven't provided it.
   It will allow statically define default cpu_model instead of
   dynamically setting default value.
2. Provides globally accessible cpu_model via current_machine pointer.

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
Note:
 1. it will allow to remove dynamic setting of default cpu_model in pc.c:pc_cpus_init()
    and simplify pc_init_isa() since default value will be defined as machine
    definition.
 2. it will be used for cpu-add hook that will be set by target-i386.
---
 include/hw/boards.h |    1 +
 vl.c                |    6 ++++++
 2 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/include/hw/boards.h b/include/hw/boards.h
index 75cd127..4ced60a 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -44,6 +44,7 @@ typedef struct QEMUMachine {
     struct QEMUMachine *next;
     const char *hw_version;
     void (*hot_add_cpu)(const int64_t id, Error **errp);
+    const char *cpu_model;
 } QEMUMachine;
 
 int qemu_register_machine(QEMUMachine *m);
diff --git a/vl.c b/vl.c
index 7d30af5..3df6021 100644
--- a/vl.c
+++ b/vl.c
@@ -4283,6 +4283,12 @@ int main(int argc, char **argv, char **envp)
 
     qdev_machine_init();
 
+    if (cpu_model) {
+        machine->cpu_model = cpu_model;
+    } else {
+        cpu_model = machine->cpu_model;
+    }
+
     QEMUMachineInitArgs args = { .ram_size = ram_size,
                                  .boot_device = (boot_devices[0] == '\0') ?
                                                 machine->boot_order :
-- 
1.7.1

[Qemu-devel] [PATCH 4/5] target-i386: get default cpu_model from QEMUMachine

[Igor Mammedov]

allows to remove:
 * checks for cpu_model ==  NULL
 * and dynamic setting of default value

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
 hw/i386/pc.c      |    9 ---------
 hw/i386/pc_piix.c |   16 ++++++++++++++--
 hw/i386/pc_q35.c  |    2 ++
 target-i386/cpu.h |    6 ++++++
 4 files changed, 22 insertions(+), 11 deletions(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 28f958d..0b75016 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -925,15 +925,6 @@ void pc_cpus_init(const char *cpu_model, DeviceState *icc_bridge)
     X86CPU *cpu = NULL;
     Error *error = NULL;
 
-    /* init CPUs */
-    if (cpu_model == NULL) {
-#ifdef TARGET_X86_64
-        cpu_model = "qemu64";
-#else
-        cpu_model = "qemu32";
-#endif
-    }
-
     for (i = 0; i < smp_cpus; i++) {
         cpu = pc_new_cpu(cpu_model, x86_cpu_apic_id_from_index(i),
                          icc_bridge, &error);
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 3717796..465c48b 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -295,8 +295,6 @@ static void pc_init_isa(QEMUMachineInitArgs *args)
     const char *kernel_cmdline = args->kernel_cmdline;
     const char *initrd_filename = args->initrd_filename;
     const char *boot_device = args->boot_device;
-    if (cpu_model == NULL)
-        cpu_model = "486";
     disable_kvm_pv_eoi();
     enable_compat_apic_id_mode();
     pc_init1(get_system_memory(),
@@ -323,6 +321,7 @@ static QEMUMachine pc_i440fx_machine_v1_5 = {
     .desc = "Standard PC (i440FX + PIIX, 1996)",
     .init = pc_init_pci,
     .max_cpus = 255,
+    .cpu_model = DEFAULT_X86CPU_MODEL,
     .is_default = 1,
     DEFAULT_MACHINE_OPTIONS,
 };
@@ -332,6 +331,7 @@ static QEMUMachine pc_i440fx_machine_v1_4 = {
     .desc = "Standard PC (i440FX + PIIX, 1996)",
     .init = pc_init_pci_1_4,
     .max_cpus = 255,
+    .cpu_model = DEFAULT_X86CPU_MODEL,
     .compat_props = (GlobalProperty[]) {
         PC_COMPAT_1_4,
         { /* end of list */ }
@@ -364,6 +364,7 @@ static QEMUMachine pc_machine_v1_3 = {
     .desc = "Standard PC",
     .init = pc_init_pci_1_3,
     .max_cpus = 255,
+    .cpu_model = DEFAULT_X86CPU_MODEL,
     .compat_props = (GlobalProperty[]) {
         PC_COMPAT_1_3,
         { /* end of list */ }
@@ -404,6 +405,7 @@ static QEMUMachine pc_machine_v1_2 = {
     .desc = "Standard PC",
     .init = pc_init_pci_1_2,
     .max_cpus = 255,
+    .cpu_model = DEFAULT_X86CPU_MODEL,
     .compat_props = (GlobalProperty[]) {
         PC_COMPAT_1_2,
         { /* end of list */ }
@@ -448,6 +450,7 @@ static QEMUMachine pc_machine_v1_1 = {
     .desc = "Standard PC",
     .init = pc_init_pci_1_2,
     .max_cpus = 255,
+    .cpu_model = DEFAULT_X86CPU_MODEL,
     .compat_props = (GlobalProperty[]) {
         PC_COMPAT_1_1,
         { /* end of list */ }
@@ -484,6 +487,7 @@ static QEMUMachine pc_machine_v1_0 = {
     .desc = "Standard PC",
     .init = pc_init_pci_1_0,
     .max_cpus = 255,
+    .cpu_model = DEFAULT_X86CPU_MODEL,
     .compat_props = (GlobalProperty[]) {
         PC_COMPAT_1_0,
         { /* end of list */ }
@@ -500,6 +504,7 @@ static QEMUMachine pc_machine_v0_15 = {
     .desc = "Standard PC",
     .init = pc_init_pci_1_0,
     .max_cpus = 255,
+    .cpu_model = DEFAULT_X86CPU_MODEL,
     .compat_props = (GlobalProperty[]) {
         PC_COMPAT_0_15,
         { /* end of list */ }
@@ -533,6 +538,7 @@ static QEMUMachine pc_machine_v0_14 = {
     .desc = "Standard PC",
     .init = pc_init_pci_1_0,
     .max_cpus = 255,
+    .cpu_model = DEFAULT_X86CPU_MODEL,
     .compat_props = (GlobalProperty[]) {
         PC_COMPAT_0_14, 
         {
@@ -567,6 +573,7 @@ static QEMUMachine pc_machine_v0_13 = {
     .desc = "Standard PC",
     .init = pc_init_pci_no_kvmclock,
     .max_cpus = 255,
+    .cpu_model = DEFAULT_X86CPU_MODEL,
     .compat_props = (GlobalProperty[]) {
         PC_COMPAT_0_13,
         {
@@ -605,6 +612,7 @@ static QEMUMachine pc_machine_v0_12 = {
     .desc = "Standard PC",
     .init = pc_init_pci_no_kvmclock,
     .max_cpus = 255,
+    .cpu_model = DEFAULT_X86CPU_MODEL,
     .compat_props = (GlobalProperty[]) {
         PC_COMPAT_0_12,
         {
@@ -639,6 +647,7 @@ static QEMUMachine pc_machine_v0_11 = {
     .desc = "Standard PC, qemu 0.11",
     .init = pc_init_pci_no_kvmclock,
     .max_cpus = 255,
+    .cpu_model = DEFAULT_X86CPU_MODEL,
     .compat_props = (GlobalProperty[]) {
         PC_COMPAT_0_11,
         {
@@ -661,6 +670,7 @@ static QEMUMachine pc_machine_v0_10 = {
     .desc = "Standard PC, qemu 0.10",
     .init = pc_init_pci_no_kvmclock,
     .max_cpus = 255,
+    .cpu_model = DEFAULT_X86CPU_MODEL,
     .compat_props = (GlobalProperty[]) {
         PC_COMPAT_0_11,
         {
@@ -694,6 +704,7 @@ static QEMUMachine isapc_machine = {
     .name = "isapc",
     .desc = "ISA-only PC",
     .init = pc_init_isa,
+    .cpu_model = "486",
     .max_cpus = 1,
     .compat_props = (GlobalProperty[]) {
         {
@@ -712,6 +723,7 @@ static QEMUMachine xenfv_machine = {
     .desc = "Xen Fully-virtualized PC",
     .init = pc_xen_hvm_init,
     .max_cpus = HVM_MAX_VCPUS,
+    .cpu_model = DEFAULT_X86CPU_MODEL,
     .default_machine_opts = "accel=xen",
     DEFAULT_MACHINE_OPTIONS,
 };
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index 073543e..ab864db 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -214,6 +214,7 @@ static QEMUMachine pc_q35_machine_v1_5 = {
     .desc = "Standard PC (Q35 + ICH9, 2009)",
     .init = pc_q35_init,
     .max_cpus = 255,
+    .cpu_model = DEFAULT_X86CPU_MODEL,
     DEFAULT_MACHINE_OPTIONS,
 };
 
@@ -226,6 +227,7 @@ static QEMUMachine pc_q35_machine_v1_4 = {
         PC_COMPAT_1_4,
         { /* end of list */ }
     },
+    .cpu_model = DEFAULT_X86CPU_MODEL,
     DEFAULT_MACHINE_OPTIONS,
 };
 
diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index f193752..a49f68a 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -896,6 +896,12 @@ typedef struct CPUX86State {
 
 #include "cpu-qom.h"
 
+#ifdef TARGET_X86_64
+#define DEFAULT_X86CPU_MODEL "qemu64"
+#else
+#define DEFAULT_X86CPU_MODEL "qemu32"
+#endif
+
 X86CPU *cpu_x86_init(const char *cpu_model);
 X86CPU *cpu_x86_create(const char *cpu_model, DeviceState *icc_bridge,
                        Error **errp);
-- 
1.7.1

[Qemu-devel] [PATCH 5/5] target-i386: implement machine->hot_add_cpu hook

[Igor Mammedov]

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
v2:
  * override .hot_add_cpu statically starting with 1.5 machine
---
 hw/i386/pc.c         |   23 +++++++++++++++++++++++
 hw/i386/pc_piix.c    |    1 +
 hw/i386/pc_q35.c     |    1 +
 include/hw/i386/pc.h |    1 +
 4 files changed, 26 insertions(+), 0 deletions(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 0b75016..7453260 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
@@ -919,6 +920,28 @@ static X86CPU *pc_new_cpu(const char *cpu_model, int64_t apic_id,
     return cpu;
 }
 
+void pc_hot_add_cpu(const int64_t id, Error **errp)
+{
+    DeviceState *icc_bridge;
+    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;
+    }
+
+    icc_bridge = DEVICE(object_resolve_path_type("icc-bridge",
+                                                 TYPE_ICC_BRIDGE, NULL));
+    pc_new_cpu(current_machine->cpu_model, apic_id, icc_bridge, errp);
+}
+
 void pc_cpus_init(const char *cpu_model, DeviceState *icc_bridge)
 {
     int i;
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 465c48b..455faa1 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -320,6 +320,7 @@ static QEMUMachine pc_i440fx_machine_v1_5 = {
     .alias = "pc",
     .desc = "Standard PC (i440FX + PIIX, 1996)",
     .init = pc_init_pci,
+    .hot_add_cpu = pc_hot_add_cpu,
     .max_cpus = 255,
     .cpu_model = DEFAULT_X86CPU_MODEL,
     .is_default = 1,
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index ab864db..c706678 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -213,6 +213,7 @@ static QEMUMachine pc_q35_machine_v1_5 = {
     .alias = "q35",
     .desc = "Standard PC (Q35 + ICH9, 2009)",
     .init = pc_q35_init,
+    .hot_add_cpu = pc_hot_add_cpu,
     .max_cpus = 255,
     .cpu_model = DEFAULT_X86CPU_MODEL,
     DEFAULT_MACHINE_OPTIONS,
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 8a6e76c..0bbb7b4 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -79,6 +79,7 @@ void pc_register_ferr_irq(qemu_irq irq);
 void pc_acpi_smi_interrupt(void *opaque, int irq, int level);
 
 void pc_cpus_init(const char *cpu_model, DeviceState *icc_bridge);
+void pc_hot_add_cpu(const int64_t id, Error **errp);
 void pc_acpi_init(const char *default_dsdt);
 void *pc_memory_init(MemoryRegion *system_memory,
                     const char *kernel_filename,
-- 
1.7.1

Re: [Qemu-devel] [PATCH 2/5] QMP: add cpu-add command

[Eduardo Habkost]

On Tue, Apr 30, 2013 at 03:41:25PM +0200, Igor Mammedov wrote:
> Adds "cpu-add id=xxx" QMP command.
> 
> cpu-add's "id" argument is a CPU number in a range [0..max-cpus)
> 
> Example QMP command:
>  -> { "execute": "cpu-add", "arguments": { "id": 2 } }
>  <- { "return": {} }
> 
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> Acked-by: Luiz Capitulino <lcapitulino@redhat.com>
> Reviewed-by: Eric Blake <eblake@redhat.com>

Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>


> ---
> v7:
>   * added "Since 1.5" to cpu-add qapi schema definition
> v6:
>   * added valid values description to qapi schema
>   * split out cpu_hot_add hooks introduction into separate patch
>   * split out implementation of cpu_hot_add for target-i386
> v5:
>   * accept id=[0..max_cpus) range in cpu-add command
> v4:
>   * merge "qmp: add cpu-add qmp command" & "target-i386: implement CPU hot-add" patches
>   * move notifier call to CPUCLass.realize()
>   * add hook cpu_hot_add to QEMUMachine
>   * make QEMUMachineInitArgs global and keep default cpu_model there
> 
> v3:
>   * it appears that 'online/offline' in cpu-set are confusing people
>     with what command actually does and users might have to distinguish
>     if 'offline' is not implemented by parsing error message. To simplify
>     things replace cpu-set with cpu-add command to show more clear what
>     command does and just add cpu-del when CPU remove is implemented.
> 
> v2:
>   * s/cpu_set/cpu-set/
>   * qmp doc style fix
>   * use bool type instead of opencodding online/offline string
>      suggested-by: Eric Blake <eblake@redhat.com>
> ---
>  qapi-schema.json |   13 +++++++++++++
>  qmp-commands.hx  |   23 +++++++++++++++++++++++
>  qmp.c            |   10 ++++++++++
>  3 files changed, 46 insertions(+), 0 deletions(-)
> 
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 5b0fb3b..6f58b0f 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -1387,6 +1387,19 @@
>  { 'command': 'cpu', 'data': {'index': 'int'} }
>  
>  ##
> +# @cpu-add
> +#
> +# Adds CPU with specified ID
> +#
> +# @id: ID of CPU to be created, valid values [0..max_cpus)
> +#
> +# Returns: Nothing on success
> +#
> +# Since 1.5
> +##
> +{ 'command': 'cpu-add', 'data': {'id': 'int'} }
> +
> +##
>  # @memsave:
>  #
>  # Save a portion of guest memory to a file.
> diff --git a/qmp-commands.hx b/qmp-commands.hx
> index 0e89132..ed99eb8 100644
> --- a/qmp-commands.hx
> +++ b/qmp-commands.hx
> @@ -385,6 +385,29 @@ Note: CPUs' indexes are obtained with the 'query-cpus' command.
>  EQMP
>  
>      {
> +        .name       = "cpu-add",
> +        .args_type  = "id:i",
> +        .mhandler.cmd_new = qmp_marshal_input_cpu_add,
> +    },
> +
> +SQMP
> +cpu-add
> +-------
> +
> +Adds virtual cpu
> +
> +Arguments:
> +
> +- "id": cpu id (json-int)
> +
> +Example:
> +
> +-> { "execute": "cpu-add", "arguments": { "id": 2 } }
> +<- { "return": {} }
> +
> +EQMP
> +
> +    {
>          .name       = "memsave",
>          .args_type  = "val:l,size:i,filename:s,cpu:i?",
>          .mhandler.cmd_new = qmp_marshal_input_memsave,
> diff --git a/qmp.c b/qmp.c
> index ed6c7ef..dd34be6 100644
> --- a/qmp.c
> +++ b/qmp.c
> @@ -24,6 +24,7 @@
>  #include "hw/qdev.h"
>  #include "sysemu/blockdev.h"
>  #include "qom/qom-qobject.h"
> +#include "hw/boards.h"
>  
>  NameInfo *qmp_query_name(Error **errp)
>  {
> @@ -108,6 +109,15 @@ void qmp_cpu(int64_t index, Error **errp)
>      /* Just do nothing */
>  }
>  
> +void qmp_cpu_add(int64_t id, Error **errp)
> +{
> +    if (current_machine->hot_add_cpu) {
> +        current_machine->hot_add_cpu(id, errp);
> +    } else {
> +        error_setg(errp, "Not supported");
> +    }
> +}
> +
>  #ifndef CONFIG_VNC
>  /* If VNC support is enabled, the "true" query-vnc command is
>     defined in the VNC subsystem */
> -- 
> 1.7.1
> 

-- 
Eduardo

Re: [Qemu-devel] [PATCH 1/5] add hot_add_cpu hook to QEMUMachine

[Eduardo Habkost]

On Tue, Apr 30, 2013 at 03:41:24PM +0200, Igor Mammedov wrote:
> Hook should be set by target that implements
> CPU hot-add via cpu-add QMP command.
> 
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>

Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>


> ---
>  include/hw/boards.h |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
> 
> diff --git a/include/hw/boards.h b/include/hw/boards.h
> index 425bdc7..75cd127 100644
> --- a/include/hw/boards.h
> +++ b/include/hw/boards.h
> @@ -43,6 +43,7 @@ typedef struct QEMUMachine {
>      GlobalProperty *compat_props;
>      struct QEMUMachine *next;
>      const char *hw_version;
> +    void (*hot_add_cpu)(const int64_t id, Error **errp);
>  } QEMUMachine;
>  
>  int qemu_register_machine(QEMUMachine *m);
> -- 
> 1.7.1
> 

-- 
Eduardo

Re: [Qemu-devel] [PATCH 3/5] add cpu_model to QEMUMachine

[Eduardo Habkost]

On Tue, Apr 30, 2013 at 03:41:26PM +0200, Igor Mammedov wrote:
> 1. Use default cpu_model from machine if user haven't provided it.
>    It will allow statically define default cpu_model instead of
>    dynamically setting default value.
> 2. Provides globally accessible cpu_model via current_machine pointer.
> 
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>

My concern is that we already have a QEMUMachineInitArgs.cpu_model
field, and now QEMUMachine.cpu_model and QEMUMachineInitArgs.cpu_model
are redundant.

To make it worse, both variables can disagree with each other because we
have other code that set QEMUMachineInitArgs.cpu_model outside of
main():

hw/arm/realview.c:338:        args->cpu_model = "arm926";
hw/arm/realview.c:346:        args->cpu_model = "arm11mpcore";
hw/arm/realview.c:354:        args->cpu_model = "cortex-a8";
hw/arm/realview.c:362:        args->cpu_model = "cortex-a9";
hw/arm/versatilepb.c:189:        args->cpu_model = "arm926";

ARM doesn't have CPU hotplug, but this is still a bug waiting to happen.

The most appropriate solution would be to kill QEMUMachineInitArgs and
just put all the machine arguments inside QEMUMachine, but this is not
feasible in time for 1.5.

But still, I would prefer an alternative solution like this:

 * Add a machine_args field to QEMUMachine
 * Add a default_cpu_model field to QEMUMachine (to replace the one in
   this patch)
 * Use machine->machine_args->cpu_model inside pc_hot_add()

But considering that:

 * The only new code using the new field is main() and pc_hot_add()
   (that's x86-specific),
 * QEMUMachineInitArgs.cpu_model can disagree with QEMUMachine.cpu_model
   only on ARM,
 * I am documenting my concerns above for future reference,

I believe this is a reasonable intermediate solution for 1.5, so:

Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>



> ---
> Note:
>  1. it will allow to remove dynamic setting of default cpu_model in pc.c:pc_cpus_init()
>     and simplify pc_init_isa() since default value will be defined as machine
>     definition.
>  2. it will be used for cpu-add hook that will be set by target-i386.
> ---
>  include/hw/boards.h |    1 +
>  vl.c                |    6 ++++++
>  2 files changed, 7 insertions(+), 0 deletions(-)
> 
> diff --git a/include/hw/boards.h b/include/hw/boards.h
> index 75cd127..4ced60a 100644
> --- a/include/hw/boards.h
> +++ b/include/hw/boards.h
> @@ -44,6 +44,7 @@ typedef struct QEMUMachine {
>      struct QEMUMachine *next;
>      const char *hw_version;
>      void (*hot_add_cpu)(const int64_t id, Error **errp);
> +    const char *cpu_model;
>  } QEMUMachine;
>  
>  int qemu_register_machine(QEMUMachine *m);
> diff --git a/vl.c b/vl.c
> index 7d30af5..3df6021 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -4283,6 +4283,12 @@ int main(int argc, char **argv, char **envp)
>  
>      qdev_machine_init();
>  
> +    if (cpu_model) {
> +        machine->cpu_model = cpu_model;
> +    } else {
> +        cpu_model = machine->cpu_model;
> +    }
> +
>      QEMUMachineInitArgs args = { .ram_size = ram_size,
>                                   .boot_device = (boot_devices[0] == '\0') ?
>                                                  machine->boot_order :
> -- 
> 1.7.1
> 

-- 
Eduardo

Re: [Qemu-devel] [PATCH 4/5] target-i386: get default cpu_model from QEMUMachine

[Eduardo Habkost]

On Tue, Apr 30, 2013 at 03:41:27PM +0200, Igor Mammedov wrote:
> allows to remove:
>  * checks for cpu_model ==  NULL
>  * and dynamic setting of default value
> 
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>

Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>

> ---
>  hw/i386/pc.c      |    9 ---------
>  hw/i386/pc_piix.c |   16 ++++++++++++++--
>  hw/i386/pc_q35.c  |    2 ++
>  target-i386/cpu.h |    6 ++++++
>  4 files changed, 22 insertions(+), 11 deletions(-)
> 
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 28f958d..0b75016 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -925,15 +925,6 @@ void pc_cpus_init(const char *cpu_model, DeviceState *icc_bridge)
>      X86CPU *cpu = NULL;
>      Error *error = NULL;
>  
> -    /* init CPUs */
> -    if (cpu_model == NULL) {
> -#ifdef TARGET_X86_64
> -        cpu_model = "qemu64";
> -#else
> -        cpu_model = "qemu32";
> -#endif
> -    }
> -
>      for (i = 0; i < smp_cpus; i++) {
>          cpu = pc_new_cpu(cpu_model, x86_cpu_apic_id_from_index(i),
>                           icc_bridge, &error);
> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> index 3717796..465c48b 100644
> --- a/hw/i386/pc_piix.c
> +++ b/hw/i386/pc_piix.c
> @@ -295,8 +295,6 @@ static void pc_init_isa(QEMUMachineInitArgs *args)
>      const char *kernel_cmdline = args->kernel_cmdline;
>      const char *initrd_filename = args->initrd_filename;
>      const char *boot_device = args->boot_device;
> -    if (cpu_model == NULL)
> -        cpu_model = "486";
>      disable_kvm_pv_eoi();
>      enable_compat_apic_id_mode();
>      pc_init1(get_system_memory(),
> @@ -323,6 +321,7 @@ static QEMUMachine pc_i440fx_machine_v1_5 = {
>      .desc = "Standard PC (i440FX + PIIX, 1996)",
>      .init = pc_init_pci,
>      .max_cpus = 255,
> +    .cpu_model = DEFAULT_X86CPU_MODEL,
>      .is_default = 1,
>      DEFAULT_MACHINE_OPTIONS,
>  };
> @@ -332,6 +331,7 @@ static QEMUMachine pc_i440fx_machine_v1_4 = {
>      .desc = "Standard PC (i440FX + PIIX, 1996)",
>      .init = pc_init_pci_1_4,
>      .max_cpus = 255,
> +    .cpu_model = DEFAULT_X86CPU_MODEL,
>      .compat_props = (GlobalProperty[]) {
>          PC_COMPAT_1_4,
>          { /* end of list */ }
> @@ -364,6 +364,7 @@ static QEMUMachine pc_machine_v1_3 = {
>      .desc = "Standard PC",
>      .init = pc_init_pci_1_3,
>      .max_cpus = 255,
> +    .cpu_model = DEFAULT_X86CPU_MODEL,
>      .compat_props = (GlobalProperty[]) {
>          PC_COMPAT_1_3,
>          { /* end of list */ }
> @@ -404,6 +405,7 @@ static QEMUMachine pc_machine_v1_2 = {
>      .desc = "Standard PC",
>      .init = pc_init_pci_1_2,
>      .max_cpus = 255,
> +    .cpu_model = DEFAULT_X86CPU_MODEL,
>      .compat_props = (GlobalProperty[]) {
>          PC_COMPAT_1_2,
>          { /* end of list */ }
> @@ -448,6 +450,7 @@ static QEMUMachine pc_machine_v1_1 = {
>      .desc = "Standard PC",
>      .init = pc_init_pci_1_2,
>      .max_cpus = 255,
> +    .cpu_model = DEFAULT_X86CPU_MODEL,
>      .compat_props = (GlobalProperty[]) {
>          PC_COMPAT_1_1,
>          { /* end of list */ }
> @@ -484,6 +487,7 @@ static QEMUMachine pc_machine_v1_0 = {
>      .desc = "Standard PC",
>      .init = pc_init_pci_1_0,
>      .max_cpus = 255,
> +    .cpu_model = DEFAULT_X86CPU_MODEL,
>      .compat_props = (GlobalProperty[]) {
>          PC_COMPAT_1_0,
>          { /* end of list */ }
> @@ -500,6 +504,7 @@ static QEMUMachine pc_machine_v0_15 = {
>      .desc = "Standard PC",
>      .init = pc_init_pci_1_0,
>      .max_cpus = 255,
> +    .cpu_model = DEFAULT_X86CPU_MODEL,
>      .compat_props = (GlobalProperty[]) {
>          PC_COMPAT_0_15,
>          { /* end of list */ }
> @@ -533,6 +538,7 @@ static QEMUMachine pc_machine_v0_14 = {
>      .desc = "Standard PC",
>      .init = pc_init_pci_1_0,
>      .max_cpus = 255,
> +    .cpu_model = DEFAULT_X86CPU_MODEL,
>      .compat_props = (GlobalProperty[]) {
>          PC_COMPAT_0_14, 
>          {
> @@ -567,6 +573,7 @@ static QEMUMachine pc_machine_v0_13 = {
>      .desc = "Standard PC",
>      .init = pc_init_pci_no_kvmclock,
>      .max_cpus = 255,
> +    .cpu_model = DEFAULT_X86CPU_MODEL,
>      .compat_props = (GlobalProperty[]) {
>          PC_COMPAT_0_13,
>          {
> @@ -605,6 +612,7 @@ static QEMUMachine pc_machine_v0_12 = {
>      .desc = "Standard PC",
>      .init = pc_init_pci_no_kvmclock,
>      .max_cpus = 255,
> +    .cpu_model = DEFAULT_X86CPU_MODEL,
>      .compat_props = (GlobalProperty[]) {
>          PC_COMPAT_0_12,
>          {
> @@ -639,6 +647,7 @@ static QEMUMachine pc_machine_v0_11 = {
>      .desc = "Standard PC, qemu 0.11",
>      .init = pc_init_pci_no_kvmclock,
>      .max_cpus = 255,
> +    .cpu_model = DEFAULT_X86CPU_MODEL,
>      .compat_props = (GlobalProperty[]) {
>          PC_COMPAT_0_11,
>          {
> @@ -661,6 +670,7 @@ static QEMUMachine pc_machine_v0_10 = {
>      .desc = "Standard PC, qemu 0.10",
>      .init = pc_init_pci_no_kvmclock,
>      .max_cpus = 255,
> +    .cpu_model = DEFAULT_X86CPU_MODEL,
>      .compat_props = (GlobalProperty[]) {
>          PC_COMPAT_0_11,
>          {
> @@ -694,6 +704,7 @@ static QEMUMachine isapc_machine = {
>      .name = "isapc",
>      .desc = "ISA-only PC",
>      .init = pc_init_isa,
> +    .cpu_model = "486",
>      .max_cpus = 1,
>      .compat_props = (GlobalProperty[]) {
>          {
> @@ -712,6 +723,7 @@ static QEMUMachine xenfv_machine = {
>      .desc = "Xen Fully-virtualized PC",
>      .init = pc_xen_hvm_init,
>      .max_cpus = HVM_MAX_VCPUS,
> +    .cpu_model = DEFAULT_X86CPU_MODEL,
>      .default_machine_opts = "accel=xen",
>      DEFAULT_MACHINE_OPTIONS,
>  };
> diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
> index 073543e..ab864db 100644
> --- a/hw/i386/pc_q35.c
> +++ b/hw/i386/pc_q35.c
> @@ -214,6 +214,7 @@ static QEMUMachine pc_q35_machine_v1_5 = {
>      .desc = "Standard PC (Q35 + ICH9, 2009)",
>      .init = pc_q35_init,
>      .max_cpus = 255,
> +    .cpu_model = DEFAULT_X86CPU_MODEL,
>      DEFAULT_MACHINE_OPTIONS,
>  };
>  
> @@ -226,6 +227,7 @@ static QEMUMachine pc_q35_machine_v1_4 = {
>          PC_COMPAT_1_4,
>          { /* end of list */ }
>      },
> +    .cpu_model = DEFAULT_X86CPU_MODEL,
>      DEFAULT_MACHINE_OPTIONS,
>  };
>  
> diff --git a/target-i386/cpu.h b/target-i386/cpu.h
> index f193752..a49f68a 100644
> --- a/target-i386/cpu.h
> +++ b/target-i386/cpu.h
> @@ -896,6 +896,12 @@ typedef struct CPUX86State {
>  
>  #include "cpu-qom.h"
>  
> +#ifdef TARGET_X86_64
> +#define DEFAULT_X86CPU_MODEL "qemu64"
> +#else
> +#define DEFAULT_X86CPU_MODEL "qemu32"
> +#endif
> +
>  X86CPU *cpu_x86_init(const char *cpu_model);
>  X86CPU *cpu_x86_create(const char *cpu_model, DeviceState *icc_bridge,
>                         Error **errp);
> -- 
> 1.7.1
> 

-- 
Eduardo

[Qemu-devel] [PATCH] target-arm: cpu: set default cpu_model via QEMUMachine.cpu_model

[Igor Mammedov]

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
 compile tested only ...

---
 hw/arm/realview.c    |   16 ++++------------
 hw/arm/versatilepb.c |    5 ++---
 2 files changed, 6 insertions(+), 15 deletions(-)

diff --git a/hw/arm/realview.c b/hw/arm/realview.c
index d6f47bf..5327b66 100644
--- a/hw/arm/realview.c
+++ b/hw/arm/realview.c
@@ -334,33 +334,21 @@ static void realview_init(QEMUMachineInitArgs *args,
 
 static void realview_eb_init(QEMUMachineInitArgs *args)
 {
-    if (!args->cpu_model) {
-        args->cpu_model = "arm926";
-    }
     realview_init(args, BOARD_EB);
 }
 
 static void realview_eb_mpcore_init(QEMUMachineInitArgs *args)
 {
-    if (!args->cpu_model) {
-        args->cpu_model = "arm11mpcore";
-    }
     realview_init(args, BOARD_EB_MPCORE);
 }
 
 static void realview_pb_a8_init(QEMUMachineInitArgs *args)
 {
-    if (!args->cpu_model) {
-        args->cpu_model = "cortex-a8";
-    }
     realview_init(args, BOARD_PB_A8);
 }
 
 static void realview_pbx_a9_init(QEMUMachineInitArgs *args)
 {
-    if (!args->cpu_model) {
-        args->cpu_model = "cortex-a9";
-    }
     realview_init(args, BOARD_PBX_A9);
 }
 
@@ -369,6 +357,7 @@ static QEMUMachine realview_eb_machine = {
     .desc = "ARM RealView Emulation Baseboard (ARM926EJ-S)",
     .init = realview_eb_init,
     .block_default_type = IF_SCSI,
+    .cpu_model = "arm926",
     DEFAULT_MACHINE_OPTIONS,
 };
 
@@ -378,6 +367,7 @@ static QEMUMachine realview_eb_mpcore_machine = {
     .init = realview_eb_mpcore_init,
     .block_default_type = IF_SCSI,
     .max_cpus = 4,
+    .cpu_model = "arm11mpcore",
     DEFAULT_MACHINE_OPTIONS,
 };
 
@@ -385,6 +375,7 @@ static QEMUMachine realview_pb_a8_machine = {
     .name = "realview-pb-a8",
     .desc = "ARM RealView Platform Baseboard for Cortex-A8",
     .init = realview_pb_a8_init,
+    .cpu_model = "cortex-a8",
     DEFAULT_MACHINE_OPTIONS,
 };
 
@@ -394,6 +385,7 @@ static QEMUMachine realview_pbx_a9_machine = {
     .init = realview_pbx_a9_init,
     .block_default_type = IF_SCSI,
     .max_cpus = 4,
+    .cpu_model = "cortex-a9",
     DEFAULT_MACHINE_OPTIONS,
 };
 
diff --git a/hw/arm/versatilepb.c b/hw/arm/versatilepb.c
index 753757e..b7d0d24 100644
--- a/hw/arm/versatilepb.c
+++ b/hw/arm/versatilepb.c
@@ -185,9 +185,6 @@ static void versatile_init(QEMUMachineInitArgs *args, int board_id)
     int done_smc = 0;
     DriveInfo *dinfo;
 
-    if (!args->cpu_model) {
-        args->cpu_model = "arm926";
-    }
     cpu = cpu_arm_init(args->cpu_model);
     if (!cpu) {
         fprintf(stderr, "Unable to find CPU definition\n");
@@ -362,6 +359,7 @@ static QEMUMachine versatilepb_machine = {
     .desc = "ARM Versatile/PB (ARM926EJ-S)",
     .init = vpb_init,
     .block_default_type = IF_SCSI,
+    .cpu_model = "arm926",
     DEFAULT_MACHINE_OPTIONS,
 };
 
@@ -370,6 +368,7 @@ static QEMUMachine versatileab_machine = {
     .desc = "ARM Versatile/AB (ARM926EJ-S)",
     .init = vab_init,
     .block_default_type = IF_SCSI,
+    .cpu_model = "arm926",
     DEFAULT_MACHINE_OPTIONS,
 };
 
-- 
1.7.1

Re: [Qemu-devel] [PATCH 3/5] add cpu_model to QEMUMachine

[Peter Maydell]

On 30 April 2013 15:30, Eduardo Habkost <ehabkost@redhat.com> wrote:
> My concern is that we already have a QEMUMachineInitArgs.cpu_model
> field, and now QEMUMachine.cpu_model and QEMUMachineInitArgs.cpu_model
> are redundant.
>
> To make it worse, both variables can disagree with each other because we
> have other code that set QEMUMachineInitArgs.cpu_model outside of
> main():
>
> hw/arm/realview.c:338:        args->cpu_model = "arm926";
> hw/arm/realview.c:346:        args->cpu_model = "arm11mpcore";
> hw/arm/realview.c:354:        args->cpu_model = "cortex-a8";
> hw/arm/realview.c:362:        args->cpu_model = "cortex-a9";
> hw/arm/versatilepb.c:189:        args->cpu_model = "arm926";
>
> ARM doesn't have CPU hotplug, but this is still a bug waiting to happen.

This ARM code is just borrowing the cpu_model field as a convenient
place to stash the default value for the board. There are other
ARM boards which do a similar thing but in a purely local variable
(eg vexpress)...

I think really if you want to know what the current CPU model
is you need to fish the relevant QOM qbject out from somewhere
at runtime.

-- PMM

Re: [Qemu-devel] [PATCH] target-arm: cpu: set default cpu_model via QEMUMachine.cpu_model

[Peter Maydell]

On 30 April 2013 15:48, Igor Mammedov <imammedo@redhat.com> wrote:
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> ---
>  compile tested only ...
>
> ---
>  hw/arm/realview.c    |   16 ++++------------
>  hw/arm/versatilepb.c |    5 ++---

Anything that's only touching these two boards is clearly
not considering the whole situation -- more boards than
just these two have a board specific default CPU.

thanks
-- PMM

Re: [Qemu-devel] [PATCH] target-arm: cpu: set default cpu_model via QEMUMachine.cpu_model

[Igor Mammedov]

On Tue, 30 Apr 2013 15:55:24 +0100
Peter Maydell <peter.maydell@linaro.org> wrote:

> On 30 April 2013 15:48, Igor Mammedov <imammedo@redhat.com> wrote:
> > Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> > ---
> >  compile tested only ...
> >
> > ---
> >  hw/arm/realview.c    |   16 ++++------------
> >  hw/arm/versatilepb.c |    5 ++---
> 
> Anything that's only touching these two boards is clearly
> not considering the whole situation -- more boards than
> just these two have a board specific default CPU.
that was only ones that used QEMUMachineArgs.cpu_model as storage for
dynamically setting default value. 
Other machines should be checked and cleaned up as well.

perhaps commit subj should be:
target-arm: cpu: set default cpu_model via QEMUMachine.cpu_model for realview and versatilepb boards.
> 
> thanks
> -- PMM

Re: [Qemu-devel] [PATCH] target-arm: cpu: set default cpu_model via QEMUMachine.cpu_model

[Eduardo Habkost]

On Tue, Apr 30, 2013 at 03:55:24PM +0100, Peter Maydell wrote:
> On 30 April 2013 15:48, Igor Mammedov <imammedo@redhat.com> wrote:
> > Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> > ---
> >  compile tested only ...
> >
> > ---
> >  hw/arm/realview.c    |   16 ++++------------
> >  hw/arm/versatilepb.c |    5 ++---
> 
> Anything that's only touching these two boards is clearly
> not considering the whole situation -- more boards than
> just these two have a board specific default CPU.

You're right. In other words: QEMUMachineInitArgs.cpu_model may be an
easy-to-find redundant variable that should be eventually replaced by
QEMUMachine.cpu_model, but we probably have dozens of local variables
used for the same thing in other code.

-- 
Eduardo

Re: [Qemu-devel] [PATCH 3/5] add cpu_model to QEMUMachine

[Igor Mammedov]

On Tue, 30 Apr 2013 15:54:34 +0100
Peter Maydell <peter.maydell@linaro.org> wrote:

> On 30 April 2013 15:30, Eduardo Habkost <ehabkost@redhat.com> wrote:
> > My concern is that we already have a QEMUMachineInitArgs.cpu_model
> > field, and now QEMUMachine.cpu_model and QEMUMachineInitArgs.cpu_model
> > are redundant.
> >
> > To make it worse, both variables can disagree with each other because we
> > have other code that set QEMUMachineInitArgs.cpu_model outside of
> > main():
> >
> > hw/arm/realview.c:338:        args->cpu_model = "arm926";
> > hw/arm/realview.c:346:        args->cpu_model = "arm11mpcore";
> > hw/arm/realview.c:354:        args->cpu_model = "cortex-a8";
> > hw/arm/realview.c:362:        args->cpu_model = "cortex-a9";
> > hw/arm/versatilepb.c:189:        args->cpu_model = "arm926";
> >
> > ARM doesn't have CPU hotplug, but this is still a bug waiting to happen.
> 
> This ARM code is just borrowing the cpu_model field as a convenient
> place to stash the default value for the board. There are other
> ARM boards which do a similar thing but in a purely local variable
> (eg vexpress)...
yes, patch addresses Eduardo's concern about not setting cpu_model of machine args
anywhere else except of main. And later we could cleanup machine args usage
and default cpu_models for all targets.

> 
> I think really if you want to know what the current CPU model
> is you need to fish the relevant QOM qbject out from somewhere
> at runtime.

Ideally that QOM object would be QOMifyed QEMUMachine, but we are not there yet.

>
> -- PMM
> 

Re: [Qemu-devel] [PATCH 3/5] add cpu_model to QEMUMachine

[Peter Maydell]

On 30 April 2013 16:14, Igor Mammedov <imammedo@redhat.com> wrote:
> On Tue, 30 Apr 2013 15:54:34 +0100
> Peter Maydell <peter.maydell@linaro.org> wrote:
>> I think really if you want to know what the current CPU model
>> is you need to fish the relevant QOM qbject out from somewhere
>> at runtime.
>
> Ideally that QOM object would be QOMifyed QEMUMachine, but we
> are not there yet.

I don't want to close the door to being able to have a
QEMUMachine with two QOM CPUs on it (ie heterogenous)...

-- PMM

Re: [Qemu-devel] [PATCH 1/5] add hot_add_cpu hook to QEMUMachine

[Andreas Färber]

Am 30.04.2013 15:41, schrieb Igor Mammedov:
> Hook should be set by target that implements
> CPU hot-add via cpu-add QMP command.
> 
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> ---
>  include/hw/boards.h |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)

Thanks, applied to qom-cpu (using a typedef and regrouping it):
https://github.com/afaerber/qemu-cpu/commits/qom-cpu

This is obviously tied to how the QMP command looks like, but I don't
see much room to rearchitect that today - if machines can't support this
simplified interface due to complex SoC setups they simply don't
implement this hook and cpu-add should fail.

Andreas

> diff --git a/include/hw/boards.h b/include/hw/boards.h
> index 425bdc7..75cd127 100644
> --- a/include/hw/boards.h
> +++ b/include/hw/boards.h
> @@ -43,6 +43,7 @@ typedef struct QEMUMachine {
>      GlobalProperty *compat_props;
>      struct QEMUMachine *next;
>      const char *hw_version;
> +    void (*hot_add_cpu)(const int64_t id, Error **errp);
>  } QEMUMachine;
>  
>  int qemu_register_machine(QEMUMachine *m);

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH 2/5] QMP: add cpu-add command

[Andreas Färber]

Am 30.04.2013 15:41, schrieb Igor Mammedov:
> Adds "cpu-add id=xxx" QMP command.
> 
> cpu-add's "id" argument is a CPU number in a range [0..max-cpus)
> 
> Example QMP command:
>  -> { "execute": "cpu-add", "arguments": { "id": 2 } }
>  <- { "return": {} }
> 
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> Acked-by: Luiz Capitulino <lcapitulino@redhat.com>
> Reviewed-by: Eric Blake <eblake@redhat.com>

Thanks, applied to qom-cpu (relying on previous QMP review):
https://github.com/afaerber/qemu-cpu/commits/qom-cpu

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

[Qemu-devel] [PATCH 5/5] target-i386: implement machine->hot_add_cpu hook

[Igor Mammedov]

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
v3:
  * use local static variable for saving cpu_model

v2:
  * override .hot_add_cpu statically starting with 1.5 machine
---
 hw/i386/pc.c         |   26 ++++++++++++++++++++++++++
 hw/i386/pc_piix.c    |    1 +
 hw/i386/pc_q35.c     |    1 +
 include/hw/i386/pc.h |    1 +
 4 files changed, 29 insertions(+), 0 deletions(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 28f958d..197d218 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
@@ -919,6 +920,30 @@ static X86CPU *pc_new_cpu(const char *cpu_model, int64_t apic_id,
     return cpu;
 }
 
+static const char *current_cpu_model;
+
+void pc_hot_add_cpu(const int64_t id, Error **errp)
+{
+    DeviceState *icc_bridge;
+    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;
+    }
+
+    icc_bridge = DEVICE(object_resolve_path_type("icc-bridge",
+                                                 TYPE_ICC_BRIDGE, NULL));
+    pc_new_cpu(current_cpu_model, apic_id, icc_bridge, errp);
+}
+
 void pc_cpus_init(const char *cpu_model, DeviceState *icc_bridge)
 {
     int i;
@@ -933,6 +958,7 @@ void pc_cpus_init(const char *cpu_model, DeviceState *icc_bridge)
         cpu_model = "qemu32";
 #endif
     }
+    current_cpu_model = cpu_model;
 
     for (i = 0; i < smp_cpus; i++) {
         cpu = pc_new_cpu(cpu_model, x86_cpu_apic_id_from_index(i),
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 3717796..1727fb5 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -322,6 +322,7 @@ static QEMUMachine pc_i440fx_machine_v1_5 = {
     .alias = "pc",
     .desc = "Standard PC (i440FX + PIIX, 1996)",
     .init = pc_init_pci,
+    .hot_add_cpu = pc_hot_add_cpu,
     .max_cpus = 255,
     .is_default = 1,
     DEFAULT_MACHINE_OPTIONS,
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index 073543e..dbf6930 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -213,6 +213,7 @@ static QEMUMachine pc_q35_machine_v1_5 = {
     .alias = "q35",
     .desc = "Standard PC (Q35 + ICH9, 2009)",
     .init = pc_q35_init,
+    .hot_add_cpu = pc_hot_add_cpu,
     .max_cpus = 255,
     DEFAULT_MACHINE_OPTIONS,
 };
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 8a6e76c..0bbb7b4 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -79,6 +79,7 @@ void pc_register_ferr_irq(qemu_irq irq);
 void pc_acpi_smi_interrupt(void *opaque, int irq, int level);
 
 void pc_cpus_init(const char *cpu_model, DeviceState *icc_bridge);
+void pc_hot_add_cpu(const int64_t id, Error **errp);
 void pc_acpi_init(const char *default_dsdt);
 void *pc_memory_init(MemoryRegion *system_memory,
                     const char *kernel_filename,
-- 
1.7.1

Re: [Qemu-devel] [PATCH 5/5] target-i386: implement machine->hot_add_cpu hook

[Eduardo Habkost]

On Tue, Apr 30, 2013 at 06:00:53PM +0200, Igor Mammedov wrote:
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>

Not the best solution (I believe I prefer the QEMUMachine.cpu_model
alternative), but code looks correct, so:

Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>


> ---
> v3:
>   * use local static variable for saving cpu_model
> 
> v2:
>   * override .hot_add_cpu statically starting with 1.5 machine
> ---
>  hw/i386/pc.c         |   26 ++++++++++++++++++++++++++
>  hw/i386/pc_piix.c    |    1 +
>  hw/i386/pc_q35.c     |    1 +
>  include/hw/i386/pc.h |    1 +
>  4 files changed, 29 insertions(+), 0 deletions(-)
> 
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 28f958d..197d218 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
> @@ -919,6 +920,30 @@ static X86CPU *pc_new_cpu(const char *cpu_model, int64_t apic_id,
>      return cpu;
>  }
>  
> +static const char *current_cpu_model;
> +
> +void pc_hot_add_cpu(const int64_t id, Error **errp)
> +{
> +    DeviceState *icc_bridge;
> +    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;
> +    }
> +
> +    icc_bridge = DEVICE(object_resolve_path_type("icc-bridge",
> +                                                 TYPE_ICC_BRIDGE, NULL));
> +    pc_new_cpu(current_cpu_model, apic_id, icc_bridge, errp);
> +}
> +
>  void pc_cpus_init(const char *cpu_model, DeviceState *icc_bridge)
>  {
>      int i;
> @@ -933,6 +958,7 @@ void pc_cpus_init(const char *cpu_model, DeviceState *icc_bridge)
>          cpu_model = "qemu32";
>  #endif
>      }
> +    current_cpu_model = cpu_model;
>  
>      for (i = 0; i < smp_cpus; i++) {
>          cpu = pc_new_cpu(cpu_model, x86_cpu_apic_id_from_index(i),
> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> index 3717796..1727fb5 100644
> --- a/hw/i386/pc_piix.c
> +++ b/hw/i386/pc_piix.c
> @@ -322,6 +322,7 @@ static QEMUMachine pc_i440fx_machine_v1_5 = {
>      .alias = "pc",
>      .desc = "Standard PC (i440FX + PIIX, 1996)",
>      .init = pc_init_pci,
> +    .hot_add_cpu = pc_hot_add_cpu,
>      .max_cpus = 255,
>      .is_default = 1,
>      DEFAULT_MACHINE_OPTIONS,
> diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
> index 073543e..dbf6930 100644
> --- a/hw/i386/pc_q35.c
> +++ b/hw/i386/pc_q35.c
> @@ -213,6 +213,7 @@ static QEMUMachine pc_q35_machine_v1_5 = {
>      .alias = "q35",
>      .desc = "Standard PC (Q35 + ICH9, 2009)",
>      .init = pc_q35_init,
> +    .hot_add_cpu = pc_hot_add_cpu,
>      .max_cpus = 255,
>      DEFAULT_MACHINE_OPTIONS,
>  };
> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> index 8a6e76c..0bbb7b4 100644
> --- a/include/hw/i386/pc.h
> +++ b/include/hw/i386/pc.h
> @@ -79,6 +79,7 @@ void pc_register_ferr_irq(qemu_irq irq);
>  void pc_acpi_smi_interrupt(void *opaque, int irq, int level);
>  
>  void pc_cpus_init(const char *cpu_model, DeviceState *icc_bridge);
> +void pc_hot_add_cpu(const int64_t id, Error **errp);
>  void pc_acpi_init(const char *default_dsdt);
>  void *pc_memory_init(MemoryRegion *system_memory,
>                      const char *kernel_filename,
> -- 
> 1.7.1
> 

-- 
Eduardo

[Qemu-devel] [PATCH 5/5 v10] target-i386: implement machine->hot_add_cpu hook

[Igor Mammedov]

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
v3:
  * use local static variable for saving cpu_model

v2:
  * override .hot_add_cpu statically starting with 1.5 machine
---
 hw/i386/pc.c         |   26 ++++++++++++++++++++++++++
 hw/i386/pc_piix.c    |    1 +
 hw/i386/pc_q35.c     |    1 +
 include/hw/i386/pc.h |    1 +
 4 files changed, 29 insertions(+), 0 deletions(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 28f958d..197d218 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
@@ -919,6 +920,30 @@ static X86CPU *pc_new_cpu(const char *cpu_model, int64_t apic_id,
     return cpu;
 }
 
+static const char *current_cpu_model;
+
+void pc_hot_add_cpu(const int64_t id, Error **errp)
+{
+    DeviceState *icc_bridge;
+    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;
+    }
+
+    icc_bridge = DEVICE(object_resolve_path_type("icc-bridge",
+                                                 TYPE_ICC_BRIDGE, NULL));
+    pc_new_cpu(current_cpu_model, apic_id, icc_bridge, errp);
+}
+
 void pc_cpus_init(const char *cpu_model, DeviceState *icc_bridge)
 {
     int i;
@@ -933,6 +958,7 @@ void pc_cpus_init(const char *cpu_model, DeviceState *icc_bridge)
         cpu_model = "qemu32";
 #endif
     }
+    current_cpu_model = cpu_model;
 
     for (i = 0; i < smp_cpus; i++) {
         cpu = pc_new_cpu(cpu_model, x86_cpu_apic_id_from_index(i),
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 3717796..1727fb5 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -322,6 +322,7 @@ static QEMUMachine pc_i440fx_machine_v1_5 = {
     .alias = "pc",
     .desc = "Standard PC (i440FX + PIIX, 1996)",
     .init = pc_init_pci,
+    .hot_add_cpu = pc_hot_add_cpu,
     .max_cpus = 255,
     .is_default = 1,
     DEFAULT_MACHINE_OPTIONS,
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index 073543e..dbf6930 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -213,6 +213,7 @@ static QEMUMachine pc_q35_machine_v1_5 = {
     .alias = "q35",
     .desc = "Standard PC (Q35 + ICH9, 2009)",
     .init = pc_q35_init,
+    .hot_add_cpu = pc_hot_add_cpu,
     .max_cpus = 255,
     DEFAULT_MACHINE_OPTIONS,
 };
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 8a6e76c..0bbb7b4 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -79,6 +79,7 @@ void pc_register_ferr_irq(qemu_irq irq);
 void pc_acpi_smi_interrupt(void *opaque, int irq, int level);
 
 void pc_cpus_init(const char *cpu_model, DeviceState *icc_bridge);
+void pc_hot_add_cpu(const int64_t id, Error **errp);
 void pc_acpi_init(const char *default_dsdt);
 void *pc_memory_init(MemoryRegion *system_memory,
                     const char *kernel_filename,
-- 
1.7.1

Re: [Qemu-devel] [PATCH 5/5] target-i386: implement machine->hot_add_cpu hook

[Andreas Färber]

Am 30.04.2013 18:00, schrieb Igor Mammedov:
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> ---
> v3:
>   * use local static variable for saving cpu_model
> 
> v2:
>   * override .hot_add_cpu statically starting with 1.5 machine

Thanks, applied to qom-cpu:
https://github.com/afaerber/qemu-cpu/commits/qom-cpu

I verified it's working as expected in Windows 2012 Datacenter, but
neither in openSUSE 12.3 nor in Fedora 18 Live DVD did I see any change
in /proc/cpuinfo or GNOME System Monitor.

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH 5/5] target-i386: implement machine->hot_add_cpu hook

[Igor Mammedov]

On Wed, 01 May 2013 21:32:59 +0200
Andreas Färber <afaerber@suse.de> wrote:

> Am 30.04.2013 18:00, schrieb Igor Mammedov:
> > Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> > ---
> > v3:
> >   * use local static variable for saving cpu_model
> > 
> > v2:
> >   * override .hot_add_cpu statically starting with 1.5 machine
> 
> Thanks, applied to qom-cpu:
> https://github.com/afaerber/qemu-cpu/commits/qom-cpu
> 
> I verified it's working as expected in Windows 2012 Datacenter, but
> neither in openSUSE 12.3 nor in Fedora 18 Live DVD did I see any change
> in /proc/cpuinfo or GNOME System Monitor.
Right after hot-add a new CPU entry should appear in /sys/devices/system/cpu
then kernel doesn't online hot-added CPUs automatically, one needs online
a new CPU manually, for example issuing following command:
 echo 1 > /sys/devices/system/cpu/cpuXX/online




> Andreas
> 
> -- 
> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg


-- 
Regards,
  Igor

Re: [Qemu-devel] [PATCH 5/5] target-i386: implement machine->hot_add_cpu hook

[Andreas Färber]

Am 02.05.2013 09:18, schrieb Igor Mammedov:
> On Wed, 01 May 2013 21:32:59 +0200
> Andreas Färber <afaerber@suse.de> wrote:
> 
>> I verified it's working as expected in Windows 2012 Datacenter, but
>> neither in openSUSE 12.3 nor in Fedora 18 Live DVD did I see any change
>> in /proc/cpuinfo or GNOME System Monitor.
> Right after hot-add a new CPU entry should appear in /sys/devices/system/cpu
> then kernel doesn't online hot-added CPUs automatically, one needs online
> a new CPU manually, for example issuing following command:
>  echo 1 > /sys/devices/system/cpu/cpuXX/online

Ah thanks, that works for both then!

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg