All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH 0/7 v7 for 1.5] target-i386: CPU hot-add with cpu-add QMP command
@ 2013-04-29 15:02 Igor Mammedov
  2013-04-29 15:02 ` [Qemu-devel] [PATCH 1/7] target-i386: Introduce ICC bus/device/bridge Igor Mammedov
                   ` (6 more replies)
  0 siblings, 7 replies; 19+ messages in thread
From: Igor Mammedov @ 2013-04-29 15:02 UTC (permalink / raw)
  To: qemu-devel
  Cc: aliguori, ehabkost, mst, jan.kiszka, claudio.fontana, aderumier,
	lcapitulino, jfrei, yang.z.zhang, pbonzini, afaerber, lig.fnst,
	rth

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.v7

based on qom-cpu tree

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 (7):
  target-i386: Introduce ICC bus/device/bridge
  target-i386: Attach ICC bus to CPU on its creation
  target-i386: Move APIC to ICC bus
  pc: pass QEMUMachineInitArgs down to pc_cpus_init()
  add hot_add_cpu hook to QEMUMachine and export machine_args
  target-i386: implement machine->hot_add_cpu hook
  QMP: add cpu-add command

 MAINTAINERS                        |   6 ++
 default-configs/i386-softmmu.mak   |   1 +
 default-configs/x86_64-softmmu.mak |   1 +
 hw/cpu/Makefile.objs               |   1 +
 hw/cpu/icc_bus.c                   | 119 +++++++++++++++++++++++++++++++++++++
 hw/i386/pc.c                       |  47 ++++++++++++---
 hw/i386/pc_piix.c                  |  74 ++++++++---------------
 hw/i386/pc_q35.c                   |  31 +++++-----
 hw/intc/apic_common.c              |  18 ++++--
 include/hw/boards.h                |   3 +
 include/hw/cpu/icc_bus.h           |  82 +++++++++++++++++++++++++
 include/hw/i386/apic_internal.h    |   6 +-
 include/hw/i386/pc.h               |   4 +-
 qapi-schema.json                   |  13 ++++
 qmp-commands.hx                    |  23 +++++++
 qmp.c                              |  10 ++++
 target-i386/cpu.c                  |  27 ++++-----
 target-i386/cpu.h                  |   3 +-
 vl.c                               |   8 ++-
 19 files changed, 378 insertions(+), 99 deletions(-)
 create mode 100644 hw/cpu/icc_bus.c
 create mode 100644 include/hw/cpu/icc_bus.h

-- 
1.8.2.1

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [Qemu-devel] [PATCH 1/7] target-i386: Introduce ICC bus/device/bridge
  2013-04-29 15:02 [Qemu-devel] [PATCH 0/7 v7 for 1.5] target-i386: CPU hot-add with cpu-add QMP command Igor Mammedov
@ 2013-04-29 15:02 ` Igor Mammedov
  2013-04-29 16:39   ` Andreas Färber
  2013-04-29 15:02 ` [Qemu-devel] [PATCH 2/7] target-i386: Attach ICC bus to CPU on its creation Igor Mammedov
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 19+ messages in thread
From: Igor Mammedov @ 2013-04-29 15:02 UTC (permalink / raw)
  To: qemu-devel
  Cc: aliguori, ehabkost, mst, jan.kiszka, claudio.fontana, aderumier,
	lcapitulino, jfrei, yang.z.zhang, pbonzini, afaerber, lig.fnst,
	rth

Provides a hotpluggable bus for APIC and CPU.

* icc-bridge will serve as a parent for icc-bus and provide
  mmio mapping services to child icc-devices.
* icc-device will replace SysBusDevice as a parent of APIC
  and IOAPIC devices.

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
[AF: QOM cleanups]
Signed-off-by: Andreas Färber <afaerber@suse.de>
---
v2:
 - rename instance name of icc-bus to "icc"

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
 MAINTAINERS                        |   6 ++
 default-configs/i386-softmmu.mak   |   1 +
 default-configs/x86_64-softmmu.mak |   1 +
 hw/cpu/Makefile.objs               |   1 +
 hw/cpu/icc_bus.c                   | 109 +++++++++++++++++++++++++++++++++++++
 hw/i386/pc_piix.c                  |   7 +++
 hw/i386/pc_q35.c                   |   7 +++
 include/hw/cpu/icc_bus.h           |  79 +++++++++++++++++++++++++++
 8 files changed, 211 insertions(+)
 create mode 100644 hw/cpu/icc_bus.c
 create mode 100644 include/hw/cpu/icc_bus.h

diff --git a/MAINTAINERS b/MAINTAINERS
index 4dfd8bf..be02724 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -644,6 +644,12 @@ F: qom/cpu.c
 F: include/qemu/cpu.h
 F: target-i386/cpu.c
 
+ICC Bus
+M: Igor Mammedov <imammedo@redhat.com>
+S: Supported
+F: include/hw/cpu/icc_bus.h
+F: hw/cpu/icc_bus.c
+
 Device Tree
 M: Peter Crosthwaite <peter.crosthwaite@petalogix.com>
 M: Alexander Graf <agraf@suse.de>
diff --git a/default-configs/i386-softmmu.mak b/default-configs/i386-softmmu.mak
index 368a776..833e722 100644
--- a/default-configs/i386-softmmu.mak
+++ b/default-configs/i386-softmmu.mak
@@ -44,3 +44,4 @@ CONFIG_LPC_ICH9=y
 CONFIG_PCI_Q35=y
 CONFIG_APIC=y
 CONFIG_IOAPIC=y
+CONFIG_ICC_BUS=y
diff --git a/default-configs/x86_64-softmmu.mak b/default-configs/x86_64-softmmu.mak
index 2711b83..b749eb7 100644
--- a/default-configs/x86_64-softmmu.mak
+++ b/default-configs/x86_64-softmmu.mak
@@ -44,3 +44,4 @@ CONFIG_LPC_ICH9=y
 CONFIG_PCI_Q35=y
 CONFIG_APIC=y
 CONFIG_IOAPIC=y
+CONFIG_ICC_BUS=y
diff --git a/hw/cpu/Makefile.objs b/hw/cpu/Makefile.objs
index a49ca04..4461ece 100644
--- a/hw/cpu/Makefile.objs
+++ b/hw/cpu/Makefile.objs
@@ -1,4 +1,5 @@
 obj-$(CONFIG_ARM11MPCORE) += arm11mpcore.o
 obj-$(CONFIG_ARM9MPCORE) += a9mpcore.o
 obj-$(CONFIG_ARM15MPCORE) += a15mpcore.o
+obj-$(CONFIG_ICC_BUS) += icc_bus.o
 
diff --git a/hw/cpu/icc_bus.c b/hw/cpu/icc_bus.c
new file mode 100644
index 0000000..3ac8eeb
--- /dev/null
+++ b/hw/cpu/icc_bus.c
@@ -0,0 +1,109 @@
+/* icc_bus.c
+ * emulate x86 ICC (Interrupt Controller Communications) bus
+ *
+ * Copyright (c) 2013 Red Hat, Inc
+ *
+ * Authors:
+ *     Igor Mammedov <imammedo@redhat.com>
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see <http://www.gnu.org/licenses/>
+ */
+#include "hw/cpu/icc_bus.h"
+#include "hw/sysbus.h"
+
+/* icc-bridge implementation */
+
+static void icc_bus_init(Object *obj)
+{
+    BusState *b = BUS(obj);
+
+    b->allow_hotplug = true;
+}
+
+static const TypeInfo icc_bus_info = {
+    .name = TYPE_ICC_BUS,
+    .parent = TYPE_BUS,
+    .instance_size = sizeof(ICCBus),
+    .instance_init = icc_bus_init,
+};
+
+
+/* icc-device implementation */
+
+static void icc_device_realize(DeviceState *dev, Error **errp)
+{
+    ICCDevice *id = ICC_DEVICE(dev);
+    ICCDeviceClass *idc = ICC_DEVICE_GET_CLASS(id);
+
+    if (idc->init) {
+        if (idc->init(id) < 0) {
+            error_setg(errp, "%s initialization failed.",
+                       object_get_typename(OBJECT(dev)));
+        }
+    }
+}
+
+static void icc_device_class_init(ObjectClass *oc, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(oc);
+
+    dc->realize = icc_device_realize;
+    dc->bus_type = TYPE_ICC_BUS;
+}
+
+static const TypeInfo icc_device_info = {
+    .name = TYPE_ICC_DEVICE,
+    .parent = TYPE_DEVICE,
+    .abstract = true,
+    .instance_size = sizeof(ICCDevice),
+    .class_size = sizeof(ICCDeviceClass),
+    .class_init = icc_device_class_init,
+};
+
+
+/*  icc-bridge implementation */
+
+typedef struct ICCBridgeState {
+    /*< private >*/
+    SysBusDevice parent_obj;
+    /*< public >*/
+
+    ICCBus icc_bus;
+} ICCBridgeState;
+
+#define ICC_BRIGDE(obj) OBJECT_CHECK(ICCBridgeState, (obj), TYPE_ICC_BRIDGE)
+
+static void icc_bridge_init(Object *obj)
+{
+    ICCBridgeState *s = ICC_BRIGDE(obj);
+
+    qbus_create_inplace(&s->icc_bus, TYPE_ICC_BUS, DEVICE(s), "icc");
+}
+
+static const TypeInfo icc_bridge_info = {
+    .name  = TYPE_ICC_BRIDGE,
+    .parent = TYPE_SYS_BUS_DEVICE,
+    .instance_init  = icc_bridge_init,
+    .instance_size  = sizeof(ICCBridgeState),
+};
+
+
+static void icc_bus_register_types(void)
+{
+    type_register_static(&icc_bus_info);
+    type_register_static(&icc_device_info);
+    type_register_static(&icc_bridge_info);
+}
+
+type_init(icc_bus_register_types)
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 89b4cb4..51b738a 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -37,6 +37,7 @@
 #include "hw/kvm/clock.h"
 #include "sysemu/sysemu.h"
 #include "hw/sysbus.h"
+#include "hw/cpu/icc_bus.h"
 #include "sysemu/arch_init.h"
 #include "sysemu/blockdev.h"
 #include "hw/i2c/smbus.h"
@@ -85,8 +86,13 @@ static void pc_init1(MemoryRegion *system_memory,
     MemoryRegion *ram_memory;
     MemoryRegion *pci_memory;
     MemoryRegion *rom_memory;
+    DeviceState *icc_bridge;
     void *fw_cfg = NULL;
 
+    icc_bridge = qdev_create(NULL, TYPE_ICC_BRIDGE);
+    object_property_add_child(qdev_get_machine(), "icc-bridge",
+                              OBJECT(icc_bridge), NULL);
+
     pc_cpus_init(cpu_model);
     pc_acpi_init("acpi-dsdt.aml");
 
@@ -161,6 +167,7 @@ static void pc_init1(MemoryRegion *system_memory,
     if (pci_enabled) {
         ioapic_init_gsi(gsi_state, "i440fx");
     }
+    qdev_init_nofail(icc_bridge);
 
     pc_register_ferr_irq(gsi[13]);
 
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index f160893..317dd0c 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -41,6 +41,7 @@
 #include "hw/ide/pci.h"
 #include "hw/ide/ahci.h"
 #include "hw/usb.h"
+#include "hw/cpu/icc_bus.h"
 
 /* ICH9 AHCI has 6 ports */
 #define MAX_SATA_PORTS     6
@@ -73,6 +74,11 @@ static void pc_q35_init(QEMUMachineInitArgs *args)
     int i;
     ICH9LPCState *ich9_lpc;
     PCIDevice *ahci;
+    DeviceState *icc_bridge;
+
+    icc_bridge = qdev_create(NULL, TYPE_ICC_BRIDGE);
+    object_property_add_child(qdev_get_machine(), "icc-bridge",
+                              OBJECT(icc_bridge), NULL);
 
     pc_cpus_init(cpu_model);
     pc_acpi_init("q35-acpi-dsdt.aml");
@@ -156,6 +162,7 @@ static void pc_q35_init(QEMUMachineInitArgs *args)
     if (pci_enabled) {
         ioapic_init_gsi(gsi_state, NULL);
     }
+    qdev_init_nofail(icc_bridge);
 
     pc_register_ferr_irq(gsi[13]);
 
diff --git a/include/hw/cpu/icc_bus.h b/include/hw/cpu/icc_bus.h
new file mode 100644
index 0000000..d728a7d
--- /dev/null
+++ b/include/hw/cpu/icc_bus.h
@@ -0,0 +1,79 @@
+/* icc_bus.h
+ * emulate x86 ICC (Interrupt Controller Communications) bus
+ *
+ * Copyright (c) 2013 Red Hat, Inc
+ *
+ * Authors:
+ *     Igor Mammedov <imammedo@redhat.com>
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see <http://www.gnu.org/licenses/>
+ */
+#ifndef ICC_BUS_H
+#define ICC_BUS_H
+
+#include "hw/qdev-core.h"
+
+#define TYPE_ICC_BUS "icc-bus"
+
+#ifndef CONFIG_USER_ONLY
+
+/**
+ * ICCBus:
+ *
+ * ICC bus
+ */
+typedef struct ICCBus {
+    /*< private >*/
+    BusState parent_obj;
+    /*< public >*/
+} ICCBus;
+
+#define ICC_BUS(obj) OBJECT_CHECK(ICCBus, (obj), TYPE_ICC_BUS)
+
+/**
+ * ICCDevice:
+ *
+ * ICC device
+ */
+typedef struct ICCDevice {
+    /*< private >*/
+    DeviceState qdev;
+    /*< public >*/
+} ICCDevice;
+
+/**
+ * ICCDeviceClass:
+ * @init: Initialization callback for derived classes.
+ *
+ * ICC device class
+ */
+typedef struct ICCDeviceClass {
+    /*< private >*/
+    DeviceClass parent_class;
+    /*< public >*/
+
+    int (*init)(ICCDevice *dev); /* TODO replace with QOM realize */
+} ICCDeviceClass;
+
+#define TYPE_ICC_DEVICE "icc-device"
+#define ICC_DEVICE(obj) OBJECT_CHECK(ICCDevice, (obj), TYPE_ICC_DEVICE)
+#define ICC_DEVICE_CLASS(klass) \
+     OBJECT_CLASS_CHECK(ICCDeviceClass, (klass), TYPE_ICC_DEVICE)
+#define ICC_DEVICE_GET_CLASS(obj) \
+     OBJECT_GET_CLASS(ICCDeviceClass, (obj), TYPE_ICC_DEVICE)
+
+#define TYPE_ICC_BRIDGE "icc-bridge"
+
+#endif /* CONFIG_USER_ONLY */
+#endif
-- 
1.8.2.1

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [Qemu-devel] [PATCH 2/7] target-i386: Attach ICC bus to CPU on its creation
  2013-04-29 15:02 [Qemu-devel] [PATCH 0/7 v7 for 1.5] target-i386: CPU hot-add with cpu-add QMP command Igor Mammedov
  2013-04-29 15:02 ` [Qemu-devel] [PATCH 1/7] target-i386: Introduce ICC bus/device/bridge Igor Mammedov
@ 2013-04-29 15:02 ` Igor Mammedov
  2013-04-29 16:24   ` Andreas Färber
  2013-04-29 16:54   ` [Qemu-devel] [PATCH 2/7 v8] " Igor Mammedov
  2013-04-29 15:02 ` [Qemu-devel] [PATCH 3/7] target-i386: Move APIC to ICC bus Igor Mammedov
                   ` (4 subsequent siblings)
  6 siblings, 2 replies; 19+ messages in thread
From: Igor Mammedov @ 2013-04-29 15:02 UTC (permalink / raw)
  To: qemu-devel
  Cc: aliguori, ehabkost, mst, jan.kiszka, claudio.fontana, aderumier,
	lcapitulino, jfrei, yang.z.zhang, pbonzini, afaerber, lig.fnst,
	rth

X86CPU should have parent bus so it could provide bus for child APIC.

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Signed-off-by: Andreas Färber <afaerber@suse.de>
---
TODO: fix unplug of bus-less devices in device-del

v2:
  - pass icc_bridge to cpu_x86_create instead of resolving it each time
---
 hw/i386/pc.c         | 10 ++++++----
 hw/i386/pc_piix.c    |  2 +-
 hw/i386/pc_q35.c     |  2 +-
 include/hw/i386/pc.h |  2 +-
 target-i386/cpu.c    | 11 +++++++++--
 target-i386/cpu.h    |  3 ++-
 6 files changed, 20 insertions(+), 10 deletions(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 7c7dd86..658ff6c 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -890,12 +890,13 @@ void pc_acpi_smi_interrupt(void *opaque, int irq, int level)
     }
 }
 
-static X86CPU *pc_new_cpu(const char *cpu_model, int64_t apic_id, Error **errp)
+static X86CPU *pc_new_cpu(const char *cpu_model, int64_t apic_id,
+                          DeviceState *icc_bridge, Error **errp)
 {
     X86CPU *cpu;
     Error *local_err = NULL;
 
-    cpu = cpu_x86_create(cpu_model, errp);
+    cpu = cpu_x86_create(cpu_model, icc_bridge, errp);
     if (!cpu) {
         return cpu;
     }
@@ -913,7 +914,7 @@ static X86CPU *pc_new_cpu(const char *cpu_model, int64_t apic_id, Error **errp)
     return cpu;
 }
 
-void pc_cpus_init(const char *cpu_model)
+void pc_cpus_init(const char *cpu_model, DeviceState *icc_bridge)
 {
     int i;
     Error *error = NULL;
@@ -928,7 +929,8 @@ void pc_cpus_init(const char *cpu_model)
     }
 
     for (i = 0; i < smp_cpus; i++) {
-        pc_new_cpu(cpu_model, x86_cpu_apic_id_from_index(i), &error);
+        pc_new_cpu(cpu_model, x86_cpu_apic_id_from_index(i),
+                   icc_bridge, &error);
         if (error) {
             fprintf(stderr, "%s\n", error_get_pretty(error));
             error_free(error);
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 51b738a..2190f0a 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -93,7 +93,7 @@ static void pc_init1(MemoryRegion *system_memory,
     object_property_add_child(qdev_get_machine(), "icc-bridge",
                               OBJECT(icc_bridge), NULL);
 
-    pc_cpus_init(cpu_model);
+    pc_cpus_init(cpu_model, icc_bridge);
     pc_acpi_init("acpi-dsdt.aml");
 
     if (kvmclock_enabled) {
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index 317dd0c..a926e38 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -80,7 +80,7 @@ static void pc_q35_init(QEMUMachineInitArgs *args)
     object_property_add_child(qdev_get_machine(), "icc-bridge",
                               OBJECT(icc_bridge), NULL);
 
-    pc_cpus_init(cpu_model);
+    pc_cpus_init(cpu_model, icc_bridge);
     pc_acpi_init("q35-acpi-dsdt.aml");
 
     kvmclock_create();
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 14b504c..8a6e76c 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -78,7 +78,7 @@ extern int fd_bootchk;
 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);
+void pc_cpus_init(const char *cpu_model, DeviceState *icc_bridge);
 void pc_acpi_init(const char *default_dsdt);
 void *pc_memory_init(MemoryRegion *system_memory,
                     const char *kernel_filename,
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 0d9493d..94856ec 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -41,6 +41,7 @@
 #endif
 
 #include "sysemu/sysemu.h"
+#include "hw/cpu/icc_bus.h"
 #ifndef CONFIG_USER_ONLY
 #include "hw/xen/xen.h"
 #include "hw/sysbus.h"
@@ -1618,7 +1619,8 @@ static void cpu_x86_register(X86CPU *cpu, const char *name, Error **errp)
     object_property_set_str(OBJECT(cpu), def->model_id, "model-id", errp);
 }
 
-X86CPU *cpu_x86_create(const char *cpu_model, Error **errp)
+X86CPU *cpu_x86_create(const char *cpu_model, DeviceState *icc_bridge,
+                       Error **errp)
 {
     X86CPU *cpu = NULL;
     CPUX86State *env;
@@ -1635,6 +1637,10 @@ X86CPU *cpu_x86_create(const char *cpu_model, Error **errp)
     features = model_pieces[1];
 
     cpu = X86_CPU(object_new(TYPE_X86_CPU));
+#ifndef CONFIG_USER_ONLY
+    qdev_set_parent_bus(DEVICE(cpu), qdev_get_child_bus(icc_bridge, "icc"));
+    object_unref(OBJECT(cpu));
+#endif
     env = &cpu->env;
     env->cpu_model_str = cpu_model;
 
@@ -1659,7 +1665,7 @@ X86CPU *cpu_x86_init(const char *cpu_model)
     Error *error = NULL;
     X86CPU *cpu;
 
-    cpu = cpu_x86_create(cpu_model, &error);
+    cpu = cpu_x86_create(cpu_model, NULL, &error);
     if (error) {
         goto out;
     }
@@ -2346,6 +2352,7 @@ static void x86_cpu_common_class_init(ObjectClass *oc, void *data)
 
     xcc->parent_realize = dc->realize;
     dc->realize = x86_cpu_realizefn;
+    dc->bus_type = TYPE_ICC_BUS;
 
     xcc->parent_reset = cc->reset;
     cc->reset = x86_cpu_reset;
diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index ab151d5..f193752 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -897,7 +897,8 @@ typedef struct CPUX86State {
 #include "cpu-qom.h"
 
 X86CPU *cpu_x86_init(const char *cpu_model);
-X86CPU *cpu_x86_create(const char *cpu_model, Error **errp);
+X86CPU *cpu_x86_create(const char *cpu_model, DeviceState *icc_bridge,
+                       Error **errp);
 int cpu_x86_exec(CPUX86State *s);
 void x86_cpu_list(FILE *f, fprintf_function cpu_fprintf);
 void x86_cpudef_setup(void);
-- 
1.8.2.1

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [Qemu-devel] [PATCH 3/7] target-i386: Move APIC to ICC bus
  2013-04-29 15:02 [Qemu-devel] [PATCH 0/7 v7 for 1.5] target-i386: CPU hot-add with cpu-add QMP command Igor Mammedov
  2013-04-29 15:02 ` [Qemu-devel] [PATCH 1/7] target-i386: Introduce ICC bus/device/bridge Igor Mammedov
  2013-04-29 15:02 ` [Qemu-devel] [PATCH 2/7] target-i386: Attach ICC bus to CPU on its creation Igor Mammedov
@ 2013-04-29 15:02 ` Igor Mammedov
  2013-04-29 16:36   ` Igor Mammedov
  2013-04-29 17:03   ` [Qemu-devel] [PATCH 3/7 v8] " Igor Mammedov
  2013-04-29 15:02 ` [Qemu-devel] [PATCH 4/7] pc: pass QEMUMachineInitArgs down to pc_cpus_init() Igor Mammedov
                   ` (3 subsequent siblings)
  6 siblings, 2 replies; 19+ messages in thread
From: Igor Mammedov @ 2013-04-29 15:02 UTC (permalink / raw)
  To: qemu-devel
  Cc: aliguori, ehabkost, mst, jan.kiszka, claudio.fontana, aderumier,
	lcapitulino, jfrei, yang.z.zhang, pbonzini, afaerber, lig.fnst,
	rth

It allows APIC to be hotplugged.

 * map APIC's mmio at board level if it is present
 * do not register mmio region for each APIC, since
   only one is used/mapped

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Signed-off-by: Andreas Färber <afaerber@suse.de>
---
v2:
  - use icc-bridge from args instead of resolving it
---
 hw/cpu/icc_bus.c                | 10 ++++++++++
 hw/i386/pc.c                    | 12 ++++++++++--
 hw/intc/apic_common.c           | 18 ++++++++++++------
 include/hw/cpu/icc_bus.h        |  3 +++
 include/hw/i386/apic_internal.h |  6 +++---
 target-i386/cpu.c               | 16 +++-------------
 6 files changed, 41 insertions(+), 24 deletions(-)

diff --git a/hw/cpu/icc_bus.c b/hw/cpu/icc_bus.c
index 3ac8eeb..73a1dc9 100644
--- a/hw/cpu/icc_bus.c
+++ b/hw/cpu/icc_bus.c
@@ -80,6 +80,7 @@ typedef struct ICCBridgeState {
     /*< public >*/
 
     ICCBus icc_bus;
+    MemoryRegion apic_container;
 } ICCBridgeState;
 
 #define ICC_BRIGDE(obj) OBJECT_CHECK(ICCBridgeState, (obj), TYPE_ICC_BRIDGE)
@@ -87,8 +88,17 @@ typedef struct ICCBridgeState {
 static void icc_bridge_init(Object *obj)
 {
     ICCBridgeState *s = ICC_BRIGDE(obj);
+    SysBusDevice *sb = SYS_BUS_DEVICE(obj);
 
     qbus_create_inplace(&s->icc_bus, TYPE_ICC_BUS, DEVICE(s), "icc");
+
+    /* Do not change order of registering regions,
+     * APIC must be first registered region, board maps it by 0 index
+     */
+    memory_region_init(&s->apic_container, "icc-apic-container",
+                       APIC_SPACE_SIZE);
+    sysbus_init_mmio(sb, &s->apic_container);
+    s->icc_bus.apic_address_space = &s->apic_container;
 }
 
 static const TypeInfo icc_bridge_info = {
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 658ff6c..ce9357e 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -53,6 +53,7 @@
 #include "qemu/bitmap.h"
 #include "qemu/config-file.h"
 #include "hw/acpi/acpi.h"
+#include "hw/cpu/icc_bus.h"
 
 /* debug PC/ISA interrupts */
 //#define DEBUG_IRQ
@@ -917,6 +918,7 @@ static X86CPU *pc_new_cpu(const char *cpu_model, int64_t apic_id,
 void pc_cpus_init(const char *cpu_model, DeviceState *icc_bridge)
 {
     int i;
+    X86CPU *cpu = NULL;
     Error *error = NULL;
 
     /* init CPUs */
@@ -929,14 +931,20 @@ void pc_cpus_init(const char *cpu_model, DeviceState *icc_bridge)
     }
 
     for (i = 0; i < smp_cpus; i++) {
-        pc_new_cpu(cpu_model, x86_cpu_apic_id_from_index(i),
-                   icc_bridge, &error);
+        cpu = pc_new_cpu(cpu_model, x86_cpu_apic_id_from_index(i),
+                         icc_bridge, &error);
         if (error) {
             fprintf(stderr, "%s\n", error_get_pretty(error));
             error_free(error);
             exit(1);
         }
     }
+
+    /* map APIC MMIO area if CPU has APIC */
+    if (cpu && cpu->env.apic_state) {
+        /* XXX: what if the base changes? */
+        sysbus_mmio_map_overlap(icc_bridge, 0, APIC_DEFAULT_ADDRESS, 0x1000);
+    }
 }
 
 void pc_acpi_init(const char *default_dsdt)
diff --git a/hw/intc/apic_common.c b/hw/intc/apic_common.c
index e0ae07a..b03e904 100644
--- a/hw/intc/apic_common.c
+++ b/hw/intc/apic_common.c
@@ -21,6 +21,8 @@
 #include "hw/i386/apic_internal.h"
 #include "trace.h"
 #include "sysemu/kvm.h"
+#include "hw/qdev.h"
+#include "hw/sysbus.h"
 
 static int apic_irq_delivered;
 bool apic_report_tpr_access;
@@ -282,12 +284,13 @@ static int apic_load_old(QEMUFile *f, void *opaque, int version_id)
     return 0;
 }
 
-static int apic_init_common(SysBusDevice *dev)
+static int apic_init_common(ICCDevice *dev)
 {
     APICCommonState *s = APIC_COMMON(dev);
     APICCommonClass *info;
     static DeviceState *vapic;
     static int apic_no;
+    static bool mmio_registered;
 
     if (apic_no >= MAX_APICS) {
         return -1;
@@ -296,8 +299,11 @@ static int apic_init_common(SysBusDevice *dev)
 
     info = APIC_COMMON_GET_CLASS(s);
     info->init(s);
-
-    sysbus_init_mmio(dev, &s->io_memory);
+    if (!mmio_registered) {
+        ICCBus *b = ICC_BUS(qdev_get_parent_bus(DEVICE(dev)));
+        memory_region_add_subregion(b->apic_address_space, 0, &s->io_memory);
+        mmio_registered = true;
+    }
 
     /* Note: We need at least 1M to map the VAPIC option ROM */
     if (!vapic && s->vapic_control & VAPIC_ENABLE_MASK &&
@@ -375,19 +381,19 @@ static Property apic_properties_common[] = {
 
 static void apic_common_class_init(ObjectClass *klass, void *data)
 {
-    SysBusDeviceClass *sc = SYS_BUS_DEVICE_CLASS(klass);
+    ICCDeviceClass *idc = ICC_DEVICE_CLASS(klass);
     DeviceClass *dc = DEVICE_CLASS(klass);
 
     dc->vmsd = &vmstate_apic_common;
     dc->reset = apic_reset_common;
     dc->no_user = 1;
     dc->props = apic_properties_common;
-    sc->init = apic_init_common;
+    idc->init = apic_init_common;
 }
 
 static const TypeInfo apic_common_type = {
     .name = TYPE_APIC_COMMON,
-    .parent = TYPE_SYS_BUS_DEVICE,
+    .parent = TYPE_ICC_DEVICE,
     .instance_size = sizeof(APICCommonState),
     .class_size = sizeof(APICCommonClass),
     .class_init = apic_common_class_init,
diff --git a/include/hw/cpu/icc_bus.h b/include/hw/cpu/icc_bus.h
index d728a7d..b550070 100644
--- a/include/hw/cpu/icc_bus.h
+++ b/include/hw/cpu/icc_bus.h
@@ -22,6 +22,7 @@
 #ifndef ICC_BUS_H
 #define ICC_BUS_H
 
+#include "exec/memory.h"
 #include "hw/qdev-core.h"
 
 #define TYPE_ICC_BUS "icc-bus"
@@ -37,6 +38,8 @@ typedef struct ICCBus {
     /*< private >*/
     BusState parent_obj;
     /*< public >*/
+
+    MemoryRegion *apic_address_space;
 } ICCBus;
 
 #define ICC_BUS(obj) OBJECT_CHECK(ICCBus, (obj), TYPE_ICC_BUS)
diff --git a/include/hw/i386/apic_internal.h b/include/hw/i386/apic_internal.h
index aac6290..1b0a7fb 100644
--- a/include/hw/i386/apic_internal.h
+++ b/include/hw/i386/apic_internal.h
@@ -21,7 +21,7 @@
 #define QEMU_APIC_INTERNAL_H
 
 #include "exec/memory.h"
-#include "hw/sysbus.h"
+#include "hw/cpu/icc_bus.h"
 #include "qemu/timer.h"
 
 /* APIC Local Vector Table */
@@ -78,7 +78,7 @@ typedef struct APICCommonState APICCommonState;
 
 typedef struct APICCommonClass
 {
-    SysBusDeviceClass parent_class;
+    ICCDeviceClass parent_class;
 
     void (*init)(APICCommonState *s);
     void (*set_base)(APICCommonState *s, uint64_t val);
@@ -92,7 +92,7 @@ typedef struct APICCommonClass
 } APICCommonClass;
 
 struct APICCommonState {
-    SysBusDevice busdev;
+    ICCDevice busdev;
 
     MemoryRegion io_memory;
     X86CPU *cpu;
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 94856ec..ac1a189 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -41,10 +41,10 @@
 #endif
 
 #include "sysemu/sysemu.h"
+#include "hw/qdev-properties.h"
 #include "hw/cpu/icc_bus.h"
 #ifndef CONFIG_USER_ONLY
 #include "hw/xen/xen.h"
-#include "hw/sysbus.h"
 #include "hw/i386/apic_internal.h"
 #endif
 
@@ -2127,6 +2127,7 @@ static void mce_init(X86CPU *cpu)
 static void x86_cpu_apic_create(X86CPU *cpu, Error **errp)
 {
     CPUX86State *env = &cpu->env;
+    DeviceState *dev = DEVICE(cpu);
     APICCommonState *apic;
     const char *apic_type = "apic";
 
@@ -2136,7 +2137,7 @@ static void x86_cpu_apic_create(X86CPU *cpu, Error **errp)
         apic_type = "xen-apic";
     }
 
-    env->apic_state = qdev_try_create(NULL, apic_type);
+    env->apic_state = qdev_try_create(qdev_get_parent_bus(dev), apic_type);
     if (env->apic_state == NULL) {
         error_setg(errp, "APIC device '%s' could not be created", apic_type);
         return;
@@ -2153,7 +2154,6 @@ static void x86_cpu_apic_create(X86CPU *cpu, Error **errp)
 static void x86_cpu_apic_realize(X86CPU *cpu, Error **errp)
 {
     CPUX86State *env = &cpu->env;
-    static int apic_mapped;
 
     if (env->apic_state == NULL) {
         return;
@@ -2164,16 +2164,6 @@ static void x86_cpu_apic_realize(X86CPU *cpu, Error **errp)
                    object_get_typename(OBJECT(env->apic_state)));
         return;
     }
-
-    /* XXX: mapping more APICs at the same memory location */
-    if (apic_mapped == 0) {
-        /* NOTE: the APIC is directly connected to the CPU - it is not
-           on the global memory bus. */
-        /* XXX: what if the base changes? */
-        sysbus_mmio_map_overlap(SYS_BUS_DEVICE(env->apic_state), 0,
-                                APIC_DEFAULT_ADDRESS, 0x1000);
-        apic_mapped = 1;
-    }
 }
 #else
 static void x86_cpu_apic_realize(X86CPU *cpu, Error **errp)
-- 
1.8.2.1

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [Qemu-devel] [PATCH 4/7] pc: pass QEMUMachineInitArgs down to pc_cpus_init()
  2013-04-29 15:02 [Qemu-devel] [PATCH 0/7 v7 for 1.5] target-i386: CPU hot-add with cpu-add QMP command Igor Mammedov
                   ` (2 preceding siblings ...)
  2013-04-29 15:02 ` [Qemu-devel] [PATCH 3/7] target-i386: Move APIC to ICC bus Igor Mammedov
@ 2013-04-29 15:02 ` Igor Mammedov
  2013-04-30  5:43   ` li guang
  2013-04-29 15:02 ` [Qemu-devel] [PATCH 5/7] add hot_add_cpu hook to QEMUMachine and export machine_args Igor Mammedov
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 19+ messages in thread
From: Igor Mammedov @ 2013-04-29 15:02 UTC (permalink / raw)
  To: qemu-devel
  Cc: aliguori, ehabkost, mst, jan.kiszka, claudio.fontana, aderumier,
	lcapitulino, jfrei, yang.z.zhang, pbonzini, afaerber, lig.fnst,
	rth

It allows to store default cpu_model if none was specified.

As side effect reduces size of the pc_piix.c code.

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
 hw/i386/pc.c         | 13 ++++++-----
 hw/i386/pc_piix.c    | 66 +++++++++++++---------------------------------------
 hw/i386/pc_q35.c     | 23 +++++++-----------
 include/hw/i386/pc.h |  3 ++-
 4 files changed, 34 insertions(+), 71 deletions(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index ce9357e..26644a1 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -915,23 +915,23 @@ static X86CPU *pc_new_cpu(const char *cpu_model, int64_t apic_id,
     return cpu;
 }
 
-void pc_cpus_init(const char *cpu_model, DeviceState *icc_bridge)
+void pc_cpus_init(QEMUMachineInitArgs *args, DeviceState *icc_bridge)
 {
     int i;
     X86CPU *cpu = NULL;
     Error *error = NULL;
 
     /* init CPUs */
-    if (cpu_model == NULL) {
+    if (args->cpu_model == NULL) {
 #ifdef TARGET_X86_64
-        cpu_model = "qemu64";
+        args->cpu_model = "qemu64";
 #else
-        cpu_model = "qemu32";
+        args->cpu_model = "qemu32";
 #endif
     }
 
     for (i = 0; i < smp_cpus; i++) {
-        cpu = pc_new_cpu(cpu_model, x86_cpu_apic_id_from_index(i),
+        cpu = pc_new_cpu(args->cpu_model, x86_cpu_apic_id_from_index(i),
                          icc_bridge, &error);
         if (error) {
             fprintf(stderr, "%s\n", error_get_pretty(error));
@@ -943,7 +943,8 @@ void pc_cpus_init(const char *cpu_model, DeviceState *icc_bridge)
     /* map APIC MMIO area if CPU has APIC */
     if (cpu && cpu->env.apic_state) {
         /* XXX: what if the base changes? */
-        sysbus_mmio_map_overlap(icc_bridge, 0, APIC_DEFAULT_ADDRESS, 0x1000);
+        sysbus_mmio_map_overlap(SYS_BUS_DEVICE(icc_bridge), 0,
+                                APIC_DEFAULT_ADDRESS, 0x1000);
     }
 }
 
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 2190f0a..bdfac59 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -59,12 +59,7 @@ static const int ide_irq[MAX_IDE_BUS] = { 14, 15 };
 /* PC hardware initialisation */
 static void pc_init1(MemoryRegion *system_memory,
                      MemoryRegion *system_io,
-                     ram_addr_t ram_size,
-                     const char *boot_device,
-                     const char *kernel_filename,
-                     const char *kernel_cmdline,
-                     const char *initrd_filename,
-                     const char *cpu_model,
+                     QEMUMachineInitArgs *args,
                      int pci_enabled,
                      int kvmclock_enabled)
 {
@@ -93,19 +88,19 @@ static void pc_init1(MemoryRegion *system_memory,
     object_property_add_child(qdev_get_machine(), "icc-bridge",
                               OBJECT(icc_bridge), NULL);
 
-    pc_cpus_init(cpu_model, icc_bridge);
+    pc_cpus_init(args, icc_bridge);
     pc_acpi_init("acpi-dsdt.aml");
 
     if (kvmclock_enabled) {
         kvmclock_create();
     }
 
-    if (ram_size >= 0xe0000000 ) {
-        above_4g_mem_size = ram_size - 0xe0000000;
+    if (args->ram_size >= 0xe0000000 ) {
+        above_4g_mem_size = args->ram_size - 0xe0000000;
         below_4g_mem_size = 0xe0000000;
     } else {
         above_4g_mem_size = 0;
-        below_4g_mem_size = ram_size;
+        below_4g_mem_size = args->ram_size;
     }
 
     if (pci_enabled) {
@@ -120,9 +115,9 @@ static void pc_init1(MemoryRegion *system_memory,
     /* allocate ram and load rom/bios */
     if (!xen_enabled()) {
         fw_cfg = pc_memory_init(system_memory,
-                       kernel_filename, kernel_cmdline, initrd_filename,
-                       below_4g_mem_size, above_4g_mem_size,
-                       rom_memory, &ram_memory);
+                                args->kernel_filename, args->kernel_cmdline,
+                                args->initrd_filename, below_4g_mem_size,
+                                above_4g_mem_size, rom_memory, &ram_memory);
     }
 
     gsi_state = g_malloc0(sizeof(*gsi_state));
@@ -136,7 +131,7 @@ static void pc_init1(MemoryRegion *system_memory,
 
     if (pci_enabled) {
         pci_bus = i440fx_init(&i440fx_state, &piix3_devfn, &isa_bus, gsi,
-                              system_memory, system_io, ram_size,
+                              system_memory, system_io, args->ram_size,
                               below_4g_mem_size,
                               0x100000000ULL - below_4g_mem_size,
                               0x100000000ULL + above_4g_mem_size,
@@ -203,7 +198,7 @@ static void pc_init1(MemoryRegion *system_memory,
 
     audio_init(isa_bus, pci_enabled ? pci_bus : NULL);
 
-    pc_cmos_init(below_4g_mem_size, above_4g_mem_size, boot_device,
+    pc_cmos_init(below_4g_mem_size, above_4g_mem_size, args->boot_device,
                  floppy, idebus[0], idebus[1], rtc_state);
 
     if (pci_enabled && usb_enabled(false)) {
@@ -229,17 +224,7 @@ static void pc_init1(MemoryRegion *system_memory,
 
 static void pc_init_pci(QEMUMachineInitArgs *args)
 {
-    ram_addr_t ram_size = args->ram_size;
-    const char *cpu_model = args->cpu_model;
-    const char *kernel_filename = args->kernel_filename;
-    const char *kernel_cmdline = args->kernel_cmdline;
-    const char *initrd_filename = args->initrd_filename;
-    const char *boot_device = args->boot_device;
-    pc_init1(get_system_memory(),
-             get_system_io(),
-             ram_size, boot_device,
-             kernel_filename, kernel_cmdline,
-             initrd_filename, cpu_model, 1, 1);
+    pc_init1(get_system_memory(), get_system_io(), args, 1, 1);
 }
 
 static void pc_init_pci_1_4(QEMUMachineInitArgs *args)
@@ -275,38 +260,19 @@ static void pc_init_pci_1_0(QEMUMachineInitArgs *args)
 /* PC init function for pc-0.10 to pc-0.13, and reused by xenfv */
 static void pc_init_pci_no_kvmclock(QEMUMachineInitArgs *args)
 {
-    ram_addr_t ram_size = args->ram_size;
-    const char *cpu_model = args->cpu_model;
-    const char *kernel_filename = args->kernel_filename;
-    const char *kernel_cmdline = args->kernel_cmdline;
-    const char *initrd_filename = args->initrd_filename;
-    const char *boot_device = args->boot_device;
     disable_kvm_pv_eoi();
     enable_compat_apic_id_mode();
-    pc_init1(get_system_memory(),
-             get_system_io(),
-             ram_size, boot_device,
-             kernel_filename, kernel_cmdline,
-             initrd_filename, cpu_model, 1, 0);
+    pc_init1(get_system_memory(), get_system_io(), args, 1, 0);
 }
 
 static void pc_init_isa(QEMUMachineInitArgs *args)
 {
-    ram_addr_t ram_size = args->ram_size;
-    const char *cpu_model = args->cpu_model;
-    const char *kernel_filename = args->kernel_filename;
-    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";
+    if (args->cpu_model == NULL) {
+        args->cpu_model = "486";
+    }
     disable_kvm_pv_eoi();
     enable_compat_apic_id_mode();
-    pc_init1(get_system_memory(),
-             get_system_io(),
-             ram_size, boot_device,
-             kernel_filename, kernel_cmdline,
-             initrd_filename, cpu_model, 0, 1);
+    pc_init1(get_system_memory(), get_system_io(), args, 0, 1);
 }
 
 #ifdef CONFIG_XEN
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index a926e38..a6f0960 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -49,12 +49,6 @@
 /* PC hardware initialisation */
 static void pc_q35_init(QEMUMachineInitArgs *args)
 {
-    ram_addr_t ram_size = args->ram_size;
-    const char *cpu_model = args->cpu_model;
-    const char *kernel_filename = args->kernel_filename;
-    const char *kernel_cmdline = args->kernel_cmdline;
-    const char *initrd_filename = args->initrd_filename;
-    const char *boot_device = args->boot_device;
     ram_addr_t below_4g_mem_size, above_4g_mem_size;
     Q35PCIHost *q35_host;
     PCIBus *host_bus;
@@ -80,17 +74,17 @@ static void pc_q35_init(QEMUMachineInitArgs *args)
     object_property_add_child(qdev_get_machine(), "icc-bridge",
                               OBJECT(icc_bridge), NULL);
 
-    pc_cpus_init(cpu_model, icc_bridge);
+    pc_cpus_init(args, icc_bridge);
     pc_acpi_init("q35-acpi-dsdt.aml");
 
     kvmclock_create();
 
-    if (ram_size >= 0xb0000000) {
-        above_4g_mem_size = ram_size - 0xb0000000;
+    if (args->ram_size >= 0xb0000000) {
+        above_4g_mem_size = args->ram_size - 0xb0000000;
         below_4g_mem_size = 0xb0000000;
     } else {
         above_4g_mem_size = 0;
-        below_4g_mem_size = ram_size;
+        below_4g_mem_size = args->ram_size;
     }
 
     /* pci enabled */
@@ -105,9 +99,10 @@ static void pc_q35_init(QEMUMachineInitArgs *args)
 
     /* allocate ram and load rom/bios */
     if (!xen_enabled()) {
-        pc_memory_init(get_system_memory(), kernel_filename, kernel_cmdline,
-                       initrd_filename, below_4g_mem_size, above_4g_mem_size,
-                       rom_memory, &ram_memory);
+        pc_memory_init(get_system_memory(),
+                       args->kernel_filename, args->kernel_cmdline,
+                       args->initrd_filename, below_4g_mem_size,
+                       above_4g_mem_size, rom_memory, &ram_memory);
     }
 
     /* irq lines */
@@ -191,7 +186,7 @@ static void pc_q35_init(QEMUMachineInitArgs *args)
                                     0xb100),
                       8, NULL, 0);
 
-    pc_cmos_init(below_4g_mem_size, above_4g_mem_size, boot_device,
+    pc_cmos_init(below_4g_mem_size, above_4g_mem_size, args->boot_device,
                  floppy, idebus[0], idebus[1], rtc_state);
 
     /* the rest devices to which pci devfn is automatically assigned */
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 8a6e76c..c86059f 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -9,6 +9,7 @@
 #include "net/net.h"
 #include "exec/memory.h"
 #include "hw/i386/ioapic.h"
+#include "hw/boards.h"
 
 /* PC-style peripherals (also used by other machines).  */
 
@@ -78,7 +79,7 @@ extern int fd_bootchk;
 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_cpus_init(QEMUMachineInitArgs *args, DeviceState *icc_bridge);
 void pc_acpi_init(const char *default_dsdt);
 void *pc_memory_init(MemoryRegion *system_memory,
                     const char *kernel_filename,
-- 
1.8.2.1

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [Qemu-devel] [PATCH 5/7] add hot_add_cpu hook to QEMUMachine and export machine_args
  2013-04-29 15:02 [Qemu-devel] [PATCH 0/7 v7 for 1.5] target-i386: CPU hot-add with cpu-add QMP command Igor Mammedov
                   ` (3 preceding siblings ...)
  2013-04-29 15:02 ` [Qemu-devel] [PATCH 4/7] pc: pass QEMUMachineInitArgs down to pc_cpus_init() Igor Mammedov
@ 2013-04-29 15:02 ` Igor Mammedov
  2013-04-30  5:47   ` li guang
  2013-04-29 15:02 ` [Qemu-devel] [PATCH 6/7] target-i386: implement machine->hot_add_cpu hook Igor Mammedov
  2013-04-29 15:02 ` [Qemu-devel] [PATCH 7/7] QMP: add cpu-add command Igor Mammedov
  6 siblings, 1 reply; 19+ messages in thread
From: Igor Mammedov @ 2013-04-29 15:02 UTC (permalink / raw)
  To: qemu-devel
  Cc: aliguori, ehabkost, mst, jan.kiszka, claudio.fontana, aderumier,
	lcapitulino, jfrei, yang.z.zhang, pbonzini, afaerber, lig.fnst,
	rth

hot_add_cpu hook should be overriden by target that implements
cpu hot-add via cpu-add QMP command.

Make machine_args available globaly, it allows to reuse
machine_args->cpu_model during hotplug, instead of adding target
specific globals to keep a copy of cpu_model.

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
v2:
  * convert stack veriable args to global machine_args
  * don't move current_machine before machine->init(), override hot_add_cpu
    staticaly instead.
---
 include/hw/boards.h | 3 +++
 vl.c                | 8 ++++++--
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/include/hw/boards.h b/include/hw/boards.h
index 425bdc7..657f379 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -18,6 +18,8 @@ typedef struct QEMUMachineInitArgs {
     const char *cpu_model;
 } QEMUMachineInitArgs;
 
+extern QEMUMachineInitArgs machine_args;
+
 typedef void QEMUMachineInitFunc(QEMUMachineInitArgs *args);
 
 typedef void QEMUMachineResetFunc(void);
@@ -43,6 +45,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);
diff --git a/vl.c b/vl.c
index 1e7d474..3fe5e94 100644
--- a/vl.c
+++ b/vl.c
@@ -179,6 +179,8 @@ int main(int argc, char **argv)
 #define MAX_VIRTIO_CONSOLES 1
 #define MAX_SCLP_CONSOLES 1
 
+QEMUMachineInitArgs machine_args;
+
 static const char *data_dir[16];
 static int data_dir_idx;
 const char *bios_name = NULL;
@@ -4291,7 +4293,8 @@ int main(int argc, char **argv, char **envp)
 
     qdev_machine_init();
 
-    QEMUMachineInitArgs args = { .ram_size = ram_size,
+    machine_args = (QEMUMachineInitArgs){
+                                 .ram_size = ram_size,
                                  .boot_device = (boot_devices[0] == '\0') ?
                                                 machine->boot_order :
                                                 boot_devices,
@@ -4299,7 +4302,8 @@ int main(int argc, char **argv, char **envp)
                                  .kernel_cmdline = kernel_cmdline,
                                  .initrd_filename = initrd_filename,
                                  .cpu_model = cpu_model };
-    machine->init(&args);
+
+    machine->init(&machine_args);
 
     cpu_synchronize_all_post_init();
 
-- 
1.8.2.1

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [Qemu-devel] [PATCH 6/7] target-i386: implement machine->hot_add_cpu hook
  2013-04-29 15:02 [Qemu-devel] [PATCH 0/7 v7 for 1.5] target-i386: CPU hot-add with cpu-add QMP command Igor Mammedov
                   ` (4 preceding siblings ...)
  2013-04-29 15:02 ` [Qemu-devel] [PATCH 5/7] add hot_add_cpu hook to QEMUMachine and export machine_args Igor Mammedov
@ 2013-04-29 15:02 ` Igor Mammedov
  2013-04-29 15:02 ` [Qemu-devel] [PATCH 7/7] QMP: add cpu-add command Igor Mammedov
  6 siblings, 0 replies; 19+ messages in thread
From: Igor Mammedov @ 2013-04-29 15:02 UTC (permalink / raw)
  To: qemu-devel
  Cc: aliguori, ehabkost, mst, jan.kiszka, claudio.fontana, aderumier,
	lcapitulino, jfrei, yang.z.zhang, pbonzini, afaerber, lig.fnst,
	rth

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

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 26644a1..2bdad17 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -915,6 +915,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(machine_args.cpu_model, apic_id, icc_bridge, errp);
+}
+
 void pc_cpus_init(QEMUMachineInitArgs *args, DeviceState *icc_bridge)
 {
     int i;
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index bdfac59..ccb230e 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -291,6 +291,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 a6f0960..24dc34f 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -209,6 +209,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 c86059f..89bf4e1 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -80,6 +80,7 @@ void pc_register_ferr_irq(qemu_irq irq);
 void pc_acpi_smi_interrupt(void *opaque, int irq, int level);
 
 void pc_cpus_init(QEMUMachineInitArgs *args, 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.8.2.1

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [Qemu-devel] [PATCH 7/7] QMP: add cpu-add command
  2013-04-29 15:02 [Qemu-devel] [PATCH 0/7 v7 for 1.5] target-i386: CPU hot-add with cpu-add QMP command Igor Mammedov
                   ` (5 preceding siblings ...)
  2013-04-29 15:02 ` [Qemu-devel] [PATCH 6/7] target-i386: implement machine->hot_add_cpu hook Igor Mammedov
@ 2013-04-29 15:02 ` Igor Mammedov
  6 siblings, 0 replies; 19+ messages in thread
From: Igor Mammedov @ 2013-04-29 15:02 UTC (permalink / raw)
  To: qemu-devel
  Cc: aliguori, ehabkost, mst, jan.kiszka, claudio.fontana, aderumier,
	lcapitulino, jfrei, yang.z.zhang, pbonzini, afaerber, lig.fnst,
	rth

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

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.8.2.1

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [Qemu-devel] [PATCH 2/7] target-i386: Attach ICC bus to CPU on its creation
  2013-04-29 15:02 ` [Qemu-devel] [PATCH 2/7] target-i386: Attach ICC bus to CPU on its creation Igor Mammedov
@ 2013-04-29 16:24   ` Andreas Färber
  2013-04-29 16:33     ` Igor Mammedov
  2013-04-29 16:54   ` [Qemu-devel] [PATCH 2/7 v8] " Igor Mammedov
  1 sibling, 1 reply; 19+ messages in thread
From: Andreas Färber @ 2013-04-29 16:24 UTC (permalink / raw)
  To: Igor Mammedov
  Cc: aliguori, ehabkost, mst, jan.kiszka, claudio.fontana, qemu-devel,
	aderumier, lcapitulino, jfrei, yang.z.zhang, pbonzini, lig.fnst,
	rth

Am 29.04.2013 17:02, schrieb Igor Mammedov:
> X86CPU should have parent bus so it could provide bus for child APIC.
> 
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> Signed-off-by: Andreas Färber <afaerber@suse.de>
> ---
> TODO: fix unplug of bus-less devices in device-del
> 
> v2:
>   - pass icc_bridge to cpu_x86_create instead of resolving it each time
> ---
>  hw/i386/pc.c         | 10 ++++++----
>  hw/i386/pc_piix.c    |  2 +-
>  hw/i386/pc_q35.c     |  2 +-
>  include/hw/i386/pc.h |  2 +-
>  target-i386/cpu.c    | 11 +++++++++--
>  target-i386/cpu.h    |  3 ++-
>  6 files changed, 20 insertions(+), 10 deletions(-)
> 
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 7c7dd86..658ff6c 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -890,12 +890,13 @@ void pc_acpi_smi_interrupt(void *opaque, int irq, int level)
>      }
>  }
>  
> -static X86CPU *pc_new_cpu(const char *cpu_model, int64_t apic_id, Error **errp)
> +static X86CPU *pc_new_cpu(const char *cpu_model, int64_t apic_id,
> +                          DeviceState *icc_bridge, Error **errp)
>  {
>      X86CPU *cpu;
>      Error *local_err = NULL;
>  
> -    cpu = cpu_x86_create(cpu_model, errp);
> +    cpu = cpu_x86_create(cpu_model, icc_bridge, errp);
>      if (!cpu) {
>          return cpu;
>      }
> @@ -913,7 +914,7 @@ static X86CPU *pc_new_cpu(const char *cpu_model, int64_t apic_id, Error **errp)
>      return cpu;
>  }
>  
> -void pc_cpus_init(const char *cpu_model)
> +void pc_cpus_init(const char *cpu_model, DeviceState *icc_bridge)
>  {
>      int i;
>      Error *error = NULL;
> @@ -928,7 +929,8 @@ void pc_cpus_init(const char *cpu_model)
>      }
>  
>      for (i = 0; i < smp_cpus; i++) {
> -        pc_new_cpu(cpu_model, x86_cpu_apic_id_from_index(i), &error);
> +        pc_new_cpu(cpu_model, x86_cpu_apic_id_from_index(i),
> +                   icc_bridge, &error);
>          if (error) {
>              fprintf(stderr, "%s\n", error_get_pretty(error));
>              error_free(error);
> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> index 51b738a..2190f0a 100644
> --- a/hw/i386/pc_piix.c
> +++ b/hw/i386/pc_piix.c
> @@ -93,7 +93,7 @@ static void pc_init1(MemoryRegion *system_memory,
>      object_property_add_child(qdev_get_machine(), "icc-bridge",
>                                OBJECT(icc_bridge), NULL);
>  
> -    pc_cpus_init(cpu_model);
> +    pc_cpus_init(cpu_model, icc_bridge);
>      pc_acpi_init("acpi-dsdt.aml");
>  
>      if (kvmclock_enabled) {
> diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
> index 317dd0c..a926e38 100644
> --- a/hw/i386/pc_q35.c
> +++ b/hw/i386/pc_q35.c
> @@ -80,7 +80,7 @@ static void pc_q35_init(QEMUMachineInitArgs *args)
>      object_property_add_child(qdev_get_machine(), "icc-bridge",
>                                OBJECT(icc_bridge), NULL);
>  
> -    pc_cpus_init(cpu_model);
> +    pc_cpus_init(cpu_model, icc_bridge);
>      pc_acpi_init("q35-acpi-dsdt.aml");
>  
>      kvmclock_create();
> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> index 14b504c..8a6e76c 100644
> --- a/include/hw/i386/pc.h
> +++ b/include/hw/i386/pc.h
> @@ -78,7 +78,7 @@ extern int fd_bootchk;
>  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);
> +void pc_cpus_init(const char *cpu_model, DeviceState *icc_bridge);
>  void pc_acpi_init(const char *default_dsdt);
>  void *pc_memory_init(MemoryRegion *system_memory,
>                      const char *kernel_filename,
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index 0d9493d..94856ec 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
> @@ -41,6 +41,7 @@
>  #endif
>  
>  #include "sysemu/sysemu.h"
> +#include "hw/cpu/icc_bus.h"
>  #ifndef CONFIG_USER_ONLY
>  #include "hw/xen/xen.h"
>  #include "hw/sysbus.h"
> @@ -1618,7 +1619,8 @@ static void cpu_x86_register(X86CPU *cpu, const char *name, Error **errp)
>      object_property_set_str(OBJECT(cpu), def->model_id, "model-id", errp);
>  }
>  
> -X86CPU *cpu_x86_create(const char *cpu_model, Error **errp)
> +X86CPU *cpu_x86_create(const char *cpu_model, DeviceState *icc_bridge,
> +                       Error **errp)
>  {
>      X86CPU *cpu = NULL;
>      CPUX86State *env;
> @@ -1635,6 +1637,10 @@ X86CPU *cpu_x86_create(const char *cpu_model, Error **errp)
>      features = model_pieces[1];
>  
>      cpu = X86_CPU(object_new(TYPE_X86_CPU));
> +#ifndef CONFIG_USER_ONLY
> +    qdev_set_parent_bus(DEVICE(cpu), qdev_get_child_bus(icc_bridge, "icc"));
> +    object_unref(OBJECT(cpu));
> +#endif
>      env = &cpu->env;
>      env->cpu_model_str = cpu_model;
>  
> @@ -1659,7 +1665,7 @@ X86CPU *cpu_x86_init(const char *cpu_model)
>      Error *error = NULL;
>      X86CPU *cpu;
>  
> -    cpu = cpu_x86_create(cpu_model, &error);
> +    cpu = cpu_x86_create(cpu_model, NULL, &error);
>      if (error) {
>          goto out;
>      }

This strikes me as unsafe: It relies on cpu_x86_init() / cpu_init() not
being used for softmmu. Could you please send a follow-up to squash
adding icc_bridge == NULL error handling in the #ifdef?

Andreas

> @@ -2346,6 +2352,7 @@ static void x86_cpu_common_class_init(ObjectClass *oc, void *data)
>  
>      xcc->parent_realize = dc->realize;
>      dc->realize = x86_cpu_realizefn;
> +    dc->bus_type = TYPE_ICC_BUS;
>  
>      xcc->parent_reset = cc->reset;
>      cc->reset = x86_cpu_reset;
> diff --git a/target-i386/cpu.h b/target-i386/cpu.h
> index ab151d5..f193752 100644
> --- a/target-i386/cpu.h
> +++ b/target-i386/cpu.h
> @@ -897,7 +897,8 @@ typedef struct CPUX86State {
>  #include "cpu-qom.h"
>  
>  X86CPU *cpu_x86_init(const char *cpu_model);
> -X86CPU *cpu_x86_create(const char *cpu_model, Error **errp);
> +X86CPU *cpu_x86_create(const char *cpu_model, DeviceState *icc_bridge,
> +                       Error **errp);
>  int cpu_x86_exec(CPUX86State *s);
>  void x86_cpu_list(FILE *f, fprintf_function cpu_fprintf);
>  void x86_cpudef_setup(void);
> 


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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [Qemu-devel] [PATCH 2/7] target-i386: Attach ICC bus to CPU on its creation
  2013-04-29 16:24   ` Andreas Färber
@ 2013-04-29 16:33     ` Igor Mammedov
  0 siblings, 0 replies; 19+ messages in thread
From: Igor Mammedov @ 2013-04-29 16:33 UTC (permalink / raw)
  To: Andreas Färber
  Cc: aliguori, ehabkost, mst, jan.kiszka, claudio.fontana, qemu-devel,
	aderumier, lcapitulino, jfrei, yang.z.zhang, pbonzini, lig.fnst,
	rth

On Mon, 29 Apr 2013 18:24:23 +0200
Andreas Färber <afaerber@suse.de> wrote:

> Am 29.04.2013 17:02, schrieb Igor Mammedov:
> > X86CPU should have parent bus so it could provide bus for child APIC.
> > 
> > Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> > Signed-off-by: Andreas Färber <afaerber@suse.de>
> > ---
> > TODO: fix unplug of bus-less devices in device-del
> > 
> > v2:
> >   - pass icc_bridge to cpu_x86_create instead of resolving it each time
> > ---
> >  hw/i386/pc.c         | 10 ++++++----
> >  hw/i386/pc_piix.c    |  2 +-
> >  hw/i386/pc_q35.c     |  2 +-
> >  include/hw/i386/pc.h |  2 +-
> >  target-i386/cpu.c    | 11 +++++++++--
> >  target-i386/cpu.h    |  3 ++-
> >  6 files changed, 20 insertions(+), 10 deletions(-)
> > 
> > diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> > index 7c7dd86..658ff6c 100644
> > --- a/hw/i386/pc.c
> > +++ b/hw/i386/pc.c
> > @@ -890,12 +890,13 @@ void pc_acpi_smi_interrupt(void *opaque, int irq, int level)
> >      }
> >  }
> >  
> > -static X86CPU *pc_new_cpu(const char *cpu_model, int64_t apic_id, Error **errp)
> > +static X86CPU *pc_new_cpu(const char *cpu_model, int64_t apic_id,
> > +                          DeviceState *icc_bridge, Error **errp)
> >  {
> >      X86CPU *cpu;
> >      Error *local_err = NULL;
> >  
> > -    cpu = cpu_x86_create(cpu_model, errp);
> > +    cpu = cpu_x86_create(cpu_model, icc_bridge, errp);
> >      if (!cpu) {
> >          return cpu;
> >      }
> > @@ -913,7 +914,7 @@ static X86CPU *pc_new_cpu(const char *cpu_model, int64_t apic_id, Error **errp)
> >      return cpu;
> >  }
> >  
> > -void pc_cpus_init(const char *cpu_model)
> > +void pc_cpus_init(const char *cpu_model, DeviceState *icc_bridge)
> >  {
> >      int i;
> >      Error *error = NULL;
> > @@ -928,7 +929,8 @@ void pc_cpus_init(const char *cpu_model)
> >      }
> >  
> >      for (i = 0; i < smp_cpus; i++) {
> > -        pc_new_cpu(cpu_model, x86_cpu_apic_id_from_index(i), &error);
> > +        pc_new_cpu(cpu_model, x86_cpu_apic_id_from_index(i),
> > +                   icc_bridge, &error);
> >          if (error) {
> >              fprintf(stderr, "%s\n", error_get_pretty(error));
> >              error_free(error);
> > diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> > index 51b738a..2190f0a 100644
> > --- a/hw/i386/pc_piix.c
> > +++ b/hw/i386/pc_piix.c
> > @@ -93,7 +93,7 @@ static void pc_init1(MemoryRegion *system_memory,
> >      object_property_add_child(qdev_get_machine(), "icc-bridge",
> >                                OBJECT(icc_bridge), NULL);
> >  
> > -    pc_cpus_init(cpu_model);
> > +    pc_cpus_init(cpu_model, icc_bridge);
> >      pc_acpi_init("acpi-dsdt.aml");
> >  
> >      if (kvmclock_enabled) {
> > diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
> > index 317dd0c..a926e38 100644
> > --- a/hw/i386/pc_q35.c
> > +++ b/hw/i386/pc_q35.c
> > @@ -80,7 +80,7 @@ static void pc_q35_init(QEMUMachineInitArgs *args)
> >      object_property_add_child(qdev_get_machine(), "icc-bridge",
> >                                OBJECT(icc_bridge), NULL);
> >  
> > -    pc_cpus_init(cpu_model);
> > +    pc_cpus_init(cpu_model, icc_bridge);
> >      pc_acpi_init("q35-acpi-dsdt.aml");
> >  
> >      kvmclock_create();
> > diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> > index 14b504c..8a6e76c 100644
> > --- a/include/hw/i386/pc.h
> > +++ b/include/hw/i386/pc.h
> > @@ -78,7 +78,7 @@ extern int fd_bootchk;
> >  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);
> > +void pc_cpus_init(const char *cpu_model, DeviceState *icc_bridge);
> >  void pc_acpi_init(const char *default_dsdt);
> >  void *pc_memory_init(MemoryRegion *system_memory,
> >                      const char *kernel_filename,
> > diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> > index 0d9493d..94856ec 100644
> > --- a/target-i386/cpu.c
> > +++ b/target-i386/cpu.c
> > @@ -41,6 +41,7 @@
> >  #endif
> >  
> >  #include "sysemu/sysemu.h"
> > +#include "hw/cpu/icc_bus.h"
> >  #ifndef CONFIG_USER_ONLY
> >  #include "hw/xen/xen.h"
> >  #include "hw/sysbus.h"
> > @@ -1618,7 +1619,8 @@ static void cpu_x86_register(X86CPU *cpu, const char *name, Error **errp)
> >      object_property_set_str(OBJECT(cpu), def->model_id, "model-id", errp);
> >  }
> >  
> > -X86CPU *cpu_x86_create(const char *cpu_model, Error **errp)
> > +X86CPU *cpu_x86_create(const char *cpu_model, DeviceState *icc_bridge,
> > +                       Error **errp)
> >  {
> >      X86CPU *cpu = NULL;
> >      CPUX86State *env;
> > @@ -1635,6 +1637,10 @@ X86CPU *cpu_x86_create(const char *cpu_model, Error **errp)
> >      features = model_pieces[1];
> >  
> >      cpu = X86_CPU(object_new(TYPE_X86_CPU));
> > +#ifndef CONFIG_USER_ONLY
> > +    qdev_set_parent_bus(DEVICE(cpu), qdev_get_child_bus(icc_bridge, "icc"));
> > +    object_unref(OBJECT(cpu));
> > +#endif
> >      env = &cpu->env;
> >      env->cpu_model_str = cpu_model;
> >  
> > @@ -1659,7 +1665,7 @@ X86CPU *cpu_x86_init(const char *cpu_model)
> >      Error *error = NULL;
> >      X86CPU *cpu;
> >  
> > -    cpu = cpu_x86_create(cpu_model, &error);
> > +    cpu = cpu_x86_create(cpu_model, NULL, &error);
> >      if (error) {
> >          goto out;
> >      }
> 
> This strikes me as unsafe: It relies on cpu_x86_init() / cpu_init() not
> being used for softmmu. Could you please send a follow-up to squash
> adding icc_bridge == NULL error handling in the #ifdef?
Intention was to make in unsafe, on target0i386 softmmu there is only one
user pc_cpus_init().
But printing nice error instead of NULL deference would be nice, I'll resend
it.

> 
> Andreas
> 
> > @@ -2346,6 +2352,7 @@ static void x86_cpu_common_class_init(ObjectClass *oc, void *data)
> >  
> >      xcc->parent_realize = dc->realize;
> >      dc->realize = x86_cpu_realizefn;
> > +    dc->bus_type = TYPE_ICC_BUS;
> >  
> >      xcc->parent_reset = cc->reset;
> >      cc->reset = x86_cpu_reset;
> > diff --git a/target-i386/cpu.h b/target-i386/cpu.h
> > index ab151d5..f193752 100644
> > --- a/target-i386/cpu.h
> > +++ b/target-i386/cpu.h
> > @@ -897,7 +897,8 @@ typedef struct CPUX86State {
> >  #include "cpu-qom.h"
> >  
> >  X86CPU *cpu_x86_init(const char *cpu_model);
> > -X86CPU *cpu_x86_create(const char *cpu_model, Error **errp);
> > +X86CPU *cpu_x86_create(const char *cpu_model, DeviceState *icc_bridge,
> > +                       Error **errp);
> >  int cpu_x86_exec(CPUX86State *s);
> >  void x86_cpu_list(FILE *f, fprintf_function cpu_fprintf);
> >  void x86_cpudef_setup(void);
> > 
> 
> 
> -- 
> 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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [Qemu-devel] [PATCH 3/7] target-i386: Move APIC to ICC bus
  2013-04-29 15:02 ` [Qemu-devel] [PATCH 3/7] target-i386: Move APIC to ICC bus Igor Mammedov
@ 2013-04-29 16:36   ` Igor Mammedov
  2013-04-29 17:03   ` [Qemu-devel] [PATCH 3/7 v8] " Igor Mammedov
  1 sibling, 0 replies; 19+ messages in thread
From: Igor Mammedov @ 2013-04-29 16:36 UTC (permalink / raw)
  To: Igor Mammedov
  Cc: aliguori, ehabkost, mst, jan.kiszka, claudio.fontana, qemu-devel,
	aderumier, lcapitulino, jfrei, yang.z.zhang, pbonzini, afaerber,
	lig.fnst, rth

On Mon, 29 Apr 2013 17:02:52 +0200
Igor Mammedov <imammedo@redhat.com> wrote:

> It allows APIC to be hotplugged.
> 
>  * map APIC's mmio at board level if it is present
>  * do not register mmio region for each APIC, since
>    only one is used/mapped
> 
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> Signed-off-by: Andreas Färber <afaerber@suse.de>
> ---
> v2:
>   - use icc-bridge from args instead of resolving it
> ---
>  hw/cpu/icc_bus.c                | 10 ++++++++++
>  hw/i386/pc.c                    | 12 ++++++++++--
>  hw/intc/apic_common.c           | 18 ++++++++++++------
>  include/hw/cpu/icc_bus.h        |  3 +++
>  include/hw/i386/apic_internal.h |  6 +++---
>  target-i386/cpu.c               | 16 +++-------------
>  6 files changed, 41 insertions(+), 24 deletions(-)
> 
> diff --git a/hw/cpu/icc_bus.c b/hw/cpu/icc_bus.c
> index 3ac8eeb..73a1dc9 100644
> --- a/hw/cpu/icc_bus.c
> +++ b/hw/cpu/icc_bus.c
> @@ -80,6 +80,7 @@ typedef struct ICCBridgeState {
>      /*< public >*/
>  
>      ICCBus icc_bus;
> +    MemoryRegion apic_container;
>  } ICCBridgeState;
>  
>  #define ICC_BRIGDE(obj) OBJECT_CHECK(ICCBridgeState, (obj), TYPE_ICC_BRIDGE)
> @@ -87,8 +88,17 @@ typedef struct ICCBridgeState {
>  static void icc_bridge_init(Object *obj)
>  {
>      ICCBridgeState *s = ICC_BRIGDE(obj);
> +    SysBusDevice *sb = SYS_BUS_DEVICE(obj);
>  
>      qbus_create_inplace(&s->icc_bus, TYPE_ICC_BUS, DEVICE(s), "icc");
> +
> +    /* Do not change order of registering regions,
> +     * APIC must be first registered region, board maps it by 0 index
> +     */
> +    memory_region_init(&s->apic_container, "icc-apic-container",
> +                       APIC_SPACE_SIZE);
> +    sysbus_init_mmio(sb, &s->apic_container);
> +    s->icc_bus.apic_address_space = &s->apic_container;
>  }
>  
>  static const TypeInfo icc_bridge_info = {
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 658ff6c..ce9357e 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -53,6 +53,7 @@
>  #include "qemu/bitmap.h"
>  #include "qemu/config-file.h"
>  #include "hw/acpi/acpi.h"
> +#include "hw/cpu/icc_bus.h"
>  
>  /* debug PC/ISA interrupts */
>  //#define DEBUG_IRQ
> @@ -917,6 +918,7 @@ static X86CPU *pc_new_cpu(const char *cpu_model, int64_t apic_id,
>  void pc_cpus_init(const char *cpu_model, DeviceState *icc_bridge)
>  {
>      int i;
> +    X86CPU *cpu = NULL;
>      Error *error = NULL;
>  
>      /* init CPUs */
> @@ -929,14 +931,20 @@ void pc_cpus_init(const char *cpu_model, DeviceState *icc_bridge)
>      }
>  
>      for (i = 0; i < smp_cpus; i++) {
> -        pc_new_cpu(cpu_model, x86_cpu_apic_id_from_index(i),
> -                   icc_bridge, &error);
> +        cpu = pc_new_cpu(cpu_model, x86_cpu_apic_id_from_index(i),
> +                         icc_bridge, &error);
>          if (error) {
>              fprintf(stderr, "%s\n", error_get_pretty(error));
>              error_free(error);
>              exit(1);
>          }
>      }
> +
> +    /* map APIC MMIO area if CPU has APIC */
> +    if (cpu && cpu->env.apic_state) {
> +        /* XXX: what if the base changes? */
> +        sysbus_mmio_map_overlap(icc_bridge, 0, APIC_DEFAULT_ADDRESS, 0x1000);
> +    }
hunk that makes icc_bridge SysBusDevice escaped into next patch, sorry. I'll
resend it.


-- 
Regards,
  Igor

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [Qemu-devel] [PATCH 1/7] target-i386: Introduce ICC bus/device/bridge
  2013-04-29 15:02 ` [Qemu-devel] [PATCH 1/7] target-i386: Introduce ICC bus/device/bridge Igor Mammedov
@ 2013-04-29 16:39   ` Andreas Färber
  0 siblings, 0 replies; 19+ messages in thread
From: Andreas Färber @ 2013-04-29 16:39 UTC (permalink / raw)
  To: Igor Mammedov
  Cc: aliguori, ehabkost, mst, jan.kiszka, claudio.fontana, qemu-devel,
	aderumier, lcapitulino, jfrei, yang.z.zhang, pbonzini, lig.fnst,
	rth

Am 29.04.2013 17:02, schrieb Igor Mammedov:
> Provides a hotpluggable bus for APIC and CPU.
> 
> * icc-bridge will serve as a parent for icc-bus and provide
>   mmio mapping services to child icc-devices.
> * icc-device will replace SysBusDevice as a parent of APIC
>   and IOAPIC devices.
> 
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> [AF: QOM cleanups]
> Signed-off-by: Andreas Färber <afaerber@suse.de>
> ---
> v2:
>  - rename instance name of icc-bus to "icc"
> 
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>

Thanks, applied to qom-cpu (simplifying the SoBs):
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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [Qemu-devel] [PATCH 2/7 v8] target-i386: Attach ICC bus to CPU on its creation
  2013-04-29 15:02 ` [Qemu-devel] [PATCH 2/7] target-i386: Attach ICC bus to CPU on its creation Igor Mammedov
  2013-04-29 16:24   ` Andreas Färber
@ 2013-04-29 16:54   ` Igor Mammedov
  2013-04-29 17:04     ` Andreas Färber
  1 sibling, 1 reply; 19+ messages in thread
From: Igor Mammedov @ 2013-04-29 16:54 UTC (permalink / raw)
  To: qemu-devel; +Cc: aliguori, afaerber, ehabkost

X86CPU should have parent bus so it could provide bus for child APIC.

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Signed-off-by: Andreas Färber <afaerber@suse.de>
---
TODO: fix unplug of bus-less devices in device-del

v3:
  - check icc_bridge for NULL and report error nicely instead of SIGSEGV
v2:
  - pass icc_bridge to cpu_x86_create instead of resolving it each time
---
 hw/i386/pc.c         | 10 ++++++----
 hw/i386/pc_piix.c    |  2 +-
 hw/i386/pc_q35.c     |  2 +-
 include/hw/i386/pc.h |  2 +-
 target-i386/cpu.c    | 15 +++++++++++++--
 target-i386/cpu.h    |  3 ++-
 6 files changed, 24 insertions(+), 10 deletions(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 7c7dd86..658ff6c 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -890,12 +890,13 @@ void pc_acpi_smi_interrupt(void *opaque, int irq, int level)
     }
 }
 
-static X86CPU *pc_new_cpu(const char *cpu_model, int64_t apic_id, Error **errp)
+static X86CPU *pc_new_cpu(const char *cpu_model, int64_t apic_id,
+                          DeviceState *icc_bridge, Error **errp)
 {
     X86CPU *cpu;
     Error *local_err = NULL;
 
-    cpu = cpu_x86_create(cpu_model, errp);
+    cpu = cpu_x86_create(cpu_model, icc_bridge, errp);
     if (!cpu) {
         return cpu;
     }
@@ -913,7 +914,7 @@ static X86CPU *pc_new_cpu(const char *cpu_model, int64_t apic_id, Error **errp)
     return cpu;
 }
 
-void pc_cpus_init(const char *cpu_model)
+void pc_cpus_init(const char *cpu_model, DeviceState *icc_bridge)
 {
     int i;
     Error *error = NULL;
@@ -928,7 +929,8 @@ void pc_cpus_init(const char *cpu_model)
     }
 
     for (i = 0; i < smp_cpus; i++) {
-        pc_new_cpu(cpu_model, x86_cpu_apic_id_from_index(i), &error);
+        pc_new_cpu(cpu_model, x86_cpu_apic_id_from_index(i),
+                   icc_bridge, &error);
         if (error) {
             fprintf(stderr, "%s\n", error_get_pretty(error));
             error_free(error);
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 51b738a..2190f0a 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -93,7 +93,7 @@ static void pc_init1(MemoryRegion *system_memory,
     object_property_add_child(qdev_get_machine(), "icc-bridge",
                               OBJECT(icc_bridge), NULL);
 
-    pc_cpus_init(cpu_model);
+    pc_cpus_init(cpu_model, icc_bridge);
     pc_acpi_init("acpi-dsdt.aml");
 
     if (kvmclock_enabled) {
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index 317dd0c..a926e38 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -80,7 +80,7 @@ static void pc_q35_init(QEMUMachineInitArgs *args)
     object_property_add_child(qdev_get_machine(), "icc-bridge",
                               OBJECT(icc_bridge), NULL);
 
-    pc_cpus_init(cpu_model);
+    pc_cpus_init(cpu_model, icc_bridge);
     pc_acpi_init("q35-acpi-dsdt.aml");
 
     kvmclock_create();
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 14b504c..8a6e76c 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -78,7 +78,7 @@ extern int fd_bootchk;
 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);
+void pc_cpus_init(const char *cpu_model, DeviceState *icc_bridge);
 void pc_acpi_init(const char *default_dsdt);
 void *pc_memory_init(MemoryRegion *system_memory,
                     const char *kernel_filename,
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 0d9493d..4fe4325 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -41,6 +41,7 @@
 #endif
 
 #include "sysemu/sysemu.h"
+#include "hw/cpu/icc_bus.h"
 #ifndef CONFIG_USER_ONLY
 #include "hw/xen/xen.h"
 #include "hw/sysbus.h"
@@ -1618,7 +1619,8 @@ static void cpu_x86_register(X86CPU *cpu, const char *name, Error **errp)
     object_property_set_str(OBJECT(cpu), def->model_id, "model-id", errp);
 }
 
-X86CPU *cpu_x86_create(const char *cpu_model, Error **errp)
+X86CPU *cpu_x86_create(const char *cpu_model, DeviceState *icc_bridge,
+                       Error **errp)
 {
     X86CPU *cpu = NULL;
     CPUX86State *env;
@@ -1635,6 +1637,14 @@ X86CPU *cpu_x86_create(const char *cpu_model, Error **errp)
     features = model_pieces[1];
 
     cpu = X86_CPU(object_new(TYPE_X86_CPU));
+#ifndef CONFIG_USER_ONLY
+    if (icc_bridge == NULL) {
+        error_setg(&error, "Invalid icc-bridge value");
+        goto out;
+    }
+    qdev_set_parent_bus(DEVICE(cpu), qdev_get_child_bus(icc_bridge, "icc"));
+    object_unref(OBJECT(cpu));
+#endif
     env = &cpu->env;
     env->cpu_model_str = cpu_model;
 
@@ -1659,7 +1669,7 @@ X86CPU *cpu_x86_init(const char *cpu_model)
     Error *error = NULL;
     X86CPU *cpu;
 
-    cpu = cpu_x86_create(cpu_model, &error);
+    cpu = cpu_x86_create(cpu_model, NULL, &error);
     if (error) {
         goto out;
     }
@@ -2346,6 +2356,7 @@ static void x86_cpu_common_class_init(ObjectClass *oc, void *data)
 
     xcc->parent_realize = dc->realize;
     dc->realize = x86_cpu_realizefn;
+    dc->bus_type = TYPE_ICC_BUS;
 
     xcc->parent_reset = cc->reset;
     cc->reset = x86_cpu_reset;
diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index ab151d5..f193752 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -897,7 +897,8 @@ typedef struct CPUX86State {
 #include "cpu-qom.h"
 
 X86CPU *cpu_x86_init(const char *cpu_model);
-X86CPU *cpu_x86_create(const char *cpu_model, Error **errp);
+X86CPU *cpu_x86_create(const char *cpu_model, DeviceState *icc_bridge,
+                       Error **errp);
 int cpu_x86_exec(CPUX86State *s);
 void x86_cpu_list(FILE *f, fprintf_function cpu_fprintf);
 void x86_cpudef_setup(void);
-- 
1.8.2.1

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [Qemu-devel] [PATCH 3/7 v8] target-i386: Move APIC to ICC bus
  2013-04-29 15:02 ` [Qemu-devel] [PATCH 3/7] target-i386: Move APIC to ICC bus Igor Mammedov
  2013-04-29 16:36   ` Igor Mammedov
@ 2013-04-29 17:03   ` Igor Mammedov
  2013-04-29 17:15     ` Andreas Färber
  1 sibling, 1 reply; 19+ messages in thread
From: Igor Mammedov @ 2013-04-29 17:03 UTC (permalink / raw)
  To: qemu-devel; +Cc: aliguori, afaerber, ehabkost

It allows APIC to be hotplugged.

 * map APIC's mmio at board level if it is present
 * do not register mmio region for each APIC, since
   only one is used/mapped

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Signed-off-by: Andreas Färber <afaerber@suse.de>
---
v3:
  - fix compile error caused by mismerged hunk
v2:
  - use icc-bridge from args instead of resolving it
---
 hw/cpu/icc_bus.c                | 10 ++++++++++
 hw/i386/pc.c                    | 13 +++++++++++--
 hw/intc/apic_common.c           | 18 ++++++++++++------
 include/hw/cpu/icc_bus.h        |  3 +++
 include/hw/i386/apic_internal.h |  6 +++---
 target-i386/cpu.c               | 16 +++-------------
 6 files changed, 42 insertions(+), 24 deletions(-)

diff --git a/hw/cpu/icc_bus.c b/hw/cpu/icc_bus.c
index 3ac8eeb..73a1dc9 100644
--- a/hw/cpu/icc_bus.c
+++ b/hw/cpu/icc_bus.c
@@ -80,6 +80,7 @@ typedef struct ICCBridgeState {
     /*< public >*/
 
     ICCBus icc_bus;
+    MemoryRegion apic_container;
 } ICCBridgeState;
 
 #define ICC_BRIGDE(obj) OBJECT_CHECK(ICCBridgeState, (obj), TYPE_ICC_BRIDGE)
@@ -87,8 +88,17 @@ typedef struct ICCBridgeState {
 static void icc_bridge_init(Object *obj)
 {
     ICCBridgeState *s = ICC_BRIGDE(obj);
+    SysBusDevice *sb = SYS_BUS_DEVICE(obj);
 
     qbus_create_inplace(&s->icc_bus, TYPE_ICC_BUS, DEVICE(s), "icc");
+
+    /* Do not change order of registering regions,
+     * APIC must be first registered region, board maps it by 0 index
+     */
+    memory_region_init(&s->apic_container, "icc-apic-container",
+                       APIC_SPACE_SIZE);
+    sysbus_init_mmio(sb, &s->apic_container);
+    s->icc_bus.apic_address_space = &s->apic_container;
 }
 
 static const TypeInfo icc_bridge_info = {
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 658ff6c..6b3faac 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -53,6 +53,7 @@
 #include "qemu/bitmap.h"
 #include "qemu/config-file.h"
 #include "hw/acpi/acpi.h"
+#include "hw/cpu/icc_bus.h"
 
 /* debug PC/ISA interrupts */
 //#define DEBUG_IRQ
@@ -917,6 +918,7 @@ static X86CPU *pc_new_cpu(const char *cpu_model, int64_t apic_id,
 void pc_cpus_init(const char *cpu_model, DeviceState *icc_bridge)
 {
     int i;
+    X86CPU *cpu = NULL;
     Error *error = NULL;
 
     /* init CPUs */
@@ -929,14 +931,21 @@ void pc_cpus_init(const char *cpu_model, DeviceState *icc_bridge)
     }
 
     for (i = 0; i < smp_cpus; i++) {
-        pc_new_cpu(cpu_model, x86_cpu_apic_id_from_index(i),
-                   icc_bridge, &error);
+        cpu = pc_new_cpu(cpu_model, x86_cpu_apic_id_from_index(i),
+                         icc_bridge, &error);
         if (error) {
             fprintf(stderr, "%s\n", error_get_pretty(error));
             error_free(error);
             exit(1);
         }
     }
+
+    /* map APIC MMIO area if CPU has APIC */
+    if (cpu && cpu->env.apic_state) {
+        /* XXX: what if the base changes? */
+        sysbus_mmio_map_overlap(SYS_BUS_DEVICE(icc_bridge), 0,
+                                APIC_DEFAULT_ADDRESS, 0x1000);
+    }
 }
 
 void pc_acpi_init(const char *default_dsdt)
diff --git a/hw/intc/apic_common.c b/hw/intc/apic_common.c
index e0ae07a..b03e904 100644
--- a/hw/intc/apic_common.c
+++ b/hw/intc/apic_common.c
@@ -21,6 +21,8 @@
 #include "hw/i386/apic_internal.h"
 #include "trace.h"
 #include "sysemu/kvm.h"
+#include "hw/qdev.h"
+#include "hw/sysbus.h"
 
 static int apic_irq_delivered;
 bool apic_report_tpr_access;
@@ -282,12 +284,13 @@ static int apic_load_old(QEMUFile *f, void *opaque, int version_id)
     return 0;
 }
 
-static int apic_init_common(SysBusDevice *dev)
+static int apic_init_common(ICCDevice *dev)
 {
     APICCommonState *s = APIC_COMMON(dev);
     APICCommonClass *info;
     static DeviceState *vapic;
     static int apic_no;
+    static bool mmio_registered;
 
     if (apic_no >= MAX_APICS) {
         return -1;
@@ -296,8 +299,11 @@ static int apic_init_common(SysBusDevice *dev)
 
     info = APIC_COMMON_GET_CLASS(s);
     info->init(s);
-
-    sysbus_init_mmio(dev, &s->io_memory);
+    if (!mmio_registered) {
+        ICCBus *b = ICC_BUS(qdev_get_parent_bus(DEVICE(dev)));
+        memory_region_add_subregion(b->apic_address_space, 0, &s->io_memory);
+        mmio_registered = true;
+    }
 
     /* Note: We need at least 1M to map the VAPIC option ROM */
     if (!vapic && s->vapic_control & VAPIC_ENABLE_MASK &&
@@ -375,19 +381,19 @@ static Property apic_properties_common[] = {
 
 static void apic_common_class_init(ObjectClass *klass, void *data)
 {
-    SysBusDeviceClass *sc = SYS_BUS_DEVICE_CLASS(klass);
+    ICCDeviceClass *idc = ICC_DEVICE_CLASS(klass);
     DeviceClass *dc = DEVICE_CLASS(klass);
 
     dc->vmsd = &vmstate_apic_common;
     dc->reset = apic_reset_common;
     dc->no_user = 1;
     dc->props = apic_properties_common;
-    sc->init = apic_init_common;
+    idc->init = apic_init_common;
 }
 
 static const TypeInfo apic_common_type = {
     .name = TYPE_APIC_COMMON,
-    .parent = TYPE_SYS_BUS_DEVICE,
+    .parent = TYPE_ICC_DEVICE,
     .instance_size = sizeof(APICCommonState),
     .class_size = sizeof(APICCommonClass),
     .class_init = apic_common_class_init,
diff --git a/include/hw/cpu/icc_bus.h b/include/hw/cpu/icc_bus.h
index d728a7d..b550070 100644
--- a/include/hw/cpu/icc_bus.h
+++ b/include/hw/cpu/icc_bus.h
@@ -22,6 +22,7 @@
 #ifndef ICC_BUS_H
 #define ICC_BUS_H
 
+#include "exec/memory.h"
 #include "hw/qdev-core.h"
 
 #define TYPE_ICC_BUS "icc-bus"
@@ -37,6 +38,8 @@ typedef struct ICCBus {
     /*< private >*/
     BusState parent_obj;
     /*< public >*/
+
+    MemoryRegion *apic_address_space;
 } ICCBus;
 
 #define ICC_BUS(obj) OBJECT_CHECK(ICCBus, (obj), TYPE_ICC_BUS)
diff --git a/include/hw/i386/apic_internal.h b/include/hw/i386/apic_internal.h
index aac6290..1b0a7fb 100644
--- a/include/hw/i386/apic_internal.h
+++ b/include/hw/i386/apic_internal.h
@@ -21,7 +21,7 @@
 #define QEMU_APIC_INTERNAL_H
 
 #include "exec/memory.h"
-#include "hw/sysbus.h"
+#include "hw/cpu/icc_bus.h"
 #include "qemu/timer.h"
 
 /* APIC Local Vector Table */
@@ -78,7 +78,7 @@ typedef struct APICCommonState APICCommonState;
 
 typedef struct APICCommonClass
 {
-    SysBusDeviceClass parent_class;
+    ICCDeviceClass parent_class;
 
     void (*init)(APICCommonState *s);
     void (*set_base)(APICCommonState *s, uint64_t val);
@@ -92,7 +92,7 @@ typedef struct APICCommonClass
 } APICCommonClass;
 
 struct APICCommonState {
-    SysBusDevice busdev;
+    ICCDevice busdev;
 
     MemoryRegion io_memory;
     X86CPU *cpu;
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 4fe4325..dc92b97 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -41,10 +41,10 @@
 #endif
 
 #include "sysemu/sysemu.h"
+#include "hw/qdev-properties.h"
 #include "hw/cpu/icc_bus.h"
 #ifndef CONFIG_USER_ONLY
 #include "hw/xen/xen.h"
-#include "hw/sysbus.h"
 #include "hw/i386/apic_internal.h"
 #endif
 
@@ -2131,6 +2131,7 @@ static void mce_init(X86CPU *cpu)
 static void x86_cpu_apic_create(X86CPU *cpu, Error **errp)
 {
     CPUX86State *env = &cpu->env;
+    DeviceState *dev = DEVICE(cpu);
     APICCommonState *apic;
     const char *apic_type = "apic";
 
@@ -2140,7 +2141,7 @@ static void x86_cpu_apic_create(X86CPU *cpu, Error **errp)
         apic_type = "xen-apic";
     }
 
-    env->apic_state = qdev_try_create(NULL, apic_type);
+    env->apic_state = qdev_try_create(qdev_get_parent_bus(dev), apic_type);
     if (env->apic_state == NULL) {
         error_setg(errp, "APIC device '%s' could not be created", apic_type);
         return;
@@ -2157,7 +2158,6 @@ static void x86_cpu_apic_create(X86CPU *cpu, Error **errp)
 static void x86_cpu_apic_realize(X86CPU *cpu, Error **errp)
 {
     CPUX86State *env = &cpu->env;
-    static int apic_mapped;
 
     if (env->apic_state == NULL) {
         return;
@@ -2168,16 +2168,6 @@ static void x86_cpu_apic_realize(X86CPU *cpu, Error **errp)
                    object_get_typename(OBJECT(env->apic_state)));
         return;
     }
-
-    /* XXX: mapping more APICs at the same memory location */
-    if (apic_mapped == 0) {
-        /* NOTE: the APIC is directly connected to the CPU - it is not
-           on the global memory bus. */
-        /* XXX: what if the base changes? */
-        sysbus_mmio_map_overlap(SYS_BUS_DEVICE(env->apic_state), 0,
-                                APIC_DEFAULT_ADDRESS, 0x1000);
-        apic_mapped = 1;
-    }
 }
 #else
 static void x86_cpu_apic_realize(X86CPU *cpu, Error **errp)
-- 
1.8.2.1

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [Qemu-devel] [PATCH 2/7 v8] target-i386: Attach ICC bus to CPU on its creation
  2013-04-29 16:54   ` [Qemu-devel] [PATCH 2/7 v8] " Igor Mammedov
@ 2013-04-29 17:04     ` Andreas Färber
  0 siblings, 0 replies; 19+ messages in thread
From: Andreas Färber @ 2013-04-29 17:04 UTC (permalink / raw)
  To: Igor Mammedov; +Cc: aliguori, qemu-devel, ehabkost

Am 29.04.2013 18:54, schrieb Igor Mammedov:
> X86CPU should have parent bus so it could provide bus for child APIC.
> 
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> Signed-off-by: Andreas Färber <afaerber@suse.de>
> ---
> TODO: fix unplug of bus-less devices in device-del
> 
> v3:
>   - check icc_bridge for NULL and report error nicely instead of SIGSEGV
> v2:
>   - pass icc_bridge to cpu_x86_create instead of resolving it each time
> ---
>  hw/i386/pc.c         | 10 ++++++----
>  hw/i386/pc_piix.c    |  2 +-
>  hw/i386/pc_q35.c     |  2 +-
>  include/hw/i386/pc.h |  2 +-
>  target-i386/cpu.c    | 15 +++++++++++++--
>  target-i386/cpu.h    |  3 ++-
>  6 files changed, 24 insertions(+), 10 deletions(-)

Thanks, applied to qom-cpu:
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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [Qemu-devel] [PATCH 3/7 v8] target-i386: Move APIC to ICC bus
  2013-04-29 17:03   ` [Qemu-devel] [PATCH 3/7 v8] " Igor Mammedov
@ 2013-04-29 17:15     ` Andreas Färber
  2013-04-29 17:34       ` Igor Mammedov
  0 siblings, 1 reply; 19+ messages in thread
From: Andreas Färber @ 2013-04-29 17:15 UTC (permalink / raw)
  To: Igor Mammedov; +Cc: aliguori, qemu-devel, ehabkost

Am 29.04.2013 19:03, schrieb Igor Mammedov:
> It allows APIC to be hotplugged.
> 
>  * map APIC's mmio at board level if it is present
>  * do not register mmio region for each APIC, since
>    only one is used/mapped
> 
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> Signed-off-by: Andreas Färber <afaerber@suse.de>
> ---
> v3:
>   - fix compile error caused by mismerged hunk
> v2:
>   - use icc-bridge from args instead of resolving it

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

One question though...

[...]
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 658ff6c..6b3faac 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
[...]
> @@ -929,14 +931,21 @@ void pc_cpus_init(const char *cpu_model, DeviceState *icc_bridge)
>      }
>  
>      for (i = 0; i < smp_cpus; i++) {
> -        pc_new_cpu(cpu_model, x86_cpu_apic_id_from_index(i),
> -                   icc_bridge, &error);
> +        cpu = pc_new_cpu(cpu_model, x86_cpu_apic_id_from_index(i),
> +                         icc_bridge, &error);
>          if (error) {
>              fprintf(stderr, "%s\n", error_get_pretty(error));
>              error_free(error);
>              exit(1);
>          }
>      }
> +
> +    /* map APIC MMIO area if CPU has APIC */
> +    if (cpu && cpu->env.apic_state) {

Might sysbus_mmio_get_region(SYS_BUS_DEVICE(icc_bridge), 0) != NULL be a
more straightforward guard? We're not accessing apic_state at all. :)

Andreas

> +        /* XXX: what if the base changes? */
> +        sysbus_mmio_map_overlap(SYS_BUS_DEVICE(icc_bridge), 0,
> +                                APIC_DEFAULT_ADDRESS, 0x1000);
> +    }
>  }
>  
>  void pc_acpi_init(const char *default_dsdt)
[snip]


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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [Qemu-devel] [PATCH 3/7 v8] target-i386: Move APIC to ICC bus
  2013-04-29 17:15     ` Andreas Färber
@ 2013-04-29 17:34       ` Igor Mammedov
  0 siblings, 0 replies; 19+ messages in thread
From: Igor Mammedov @ 2013-04-29 17:34 UTC (permalink / raw)
  To: Andreas Färber; +Cc: aliguori, qemu-devel, ehabkost

On Mon, 29 Apr 2013 19:15:56 +0200
Andreas Färber <afaerber@suse.de> wrote:

> Am 29.04.2013 19:03, schrieb Igor Mammedov:
> > It allows APIC to be hotplugged.
> > 
> >  * map APIC's mmio at board level if it is present
> >  * do not register mmio region for each APIC, since
> >    only one is used/mapped
> > 
> > Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> > Signed-off-by: Andreas Färber <afaerber@suse.de>
> > ---
> > v3:
> >   - fix compile error caused by mismerged hunk
> > v2:
> >   - use icc-bridge from args instead of resolving it
> 
> Thanks, applied to qom-cpu:
> https://github.com/afaerber/qemu-cpu/commits/qom-cpu
> 
> One question though...
> 
> [...]
> > diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> > index 658ff6c..6b3faac 100644
> > --- a/hw/i386/pc.c
> > +++ b/hw/i386/pc.c
> [...]
> > @@ -929,14 +931,21 @@ void pc_cpus_init(const char *cpu_model, DeviceState *icc_bridge)
> >      }
> >  
> >      for (i = 0; i < smp_cpus; i++) {
> > -        pc_new_cpu(cpu_model, x86_cpu_apic_id_from_index(i),
> > -                   icc_bridge, &error);
> > +        cpu = pc_new_cpu(cpu_model, x86_cpu_apic_id_from_index(i),
> > +                         icc_bridge, &error);
> >          if (error) {
> >              fprintf(stderr, "%s\n", error_get_pretty(error));
> >              error_free(error);
> >              exit(1);
> >          }
> >      }
> > +
> > +    /* map APIC MMIO area if CPU has APIC */
> > +    if (cpu && cpu->env.apic_state) {
> 
> Might sysbus_mmio_get_region(SYS_BUS_DEVICE(icc_bridge), 0) != NULL be a
> more straightforward guard? We're not accessing apic_state at all. :)
it won't be NULL, it's icc_bridge's region that always exists.

> 
> Andreas
> 
> > +        /* XXX: what if the base changes? */
> > +        sysbus_mmio_map_overlap(SYS_BUS_DEVICE(icc_bridge), 0,
> > +                                APIC_DEFAULT_ADDRESS, 0x1000);
> > +    }
> >  }
> >  
> >  void pc_acpi_init(const char *default_dsdt)
> [snip]
> 
> 
> -- 
> 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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [Qemu-devel] [PATCH 4/7] pc: pass QEMUMachineInitArgs down to pc_cpus_init()
  2013-04-29 15:02 ` [Qemu-devel] [PATCH 4/7] pc: pass QEMUMachineInitArgs down to pc_cpus_init() Igor Mammedov
@ 2013-04-30  5:43   ` li guang
  0 siblings, 0 replies; 19+ messages in thread
From: li guang @ 2013-04-30  5:43 UTC (permalink / raw)
  To: Igor Mammedov
  Cc: aliguori, ehabkost, mst, jan.kiszka, claudio.fontana, qemu-devel,
	aderumier, lcapitulino, jfrei, yang.z.zhang, pbonzini, afaerber,
	rth

Reviewed-by: liguang <lig.fnst@cn.fujitsu.com>


在 2013-04-29一的 17:02 +0200,Igor Mammedov写道:
> It allows to store default cpu_model if none was specified.
> 
> As side effect reduces size of the pc_piix.c code.
> 
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> ---
>  hw/i386/pc.c         | 13 ++++++-----
>  hw/i386/pc_piix.c    | 66 +++++++++++++---------------------------------------
>  hw/i386/pc_q35.c     | 23 +++++++-----------
>  include/hw/i386/pc.h |  3 ++-
>  4 files changed, 34 insertions(+), 71 deletions(-)
> 
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index ce9357e..26644a1 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -915,23 +915,23 @@ static X86CPU *pc_new_cpu(const char *cpu_model, int64_t apic_id,
>      return cpu;
>  }
>  
> -void pc_cpus_init(const char *cpu_model, DeviceState *icc_bridge)
> +void pc_cpus_init(QEMUMachineInitArgs *args, DeviceState *icc_bridge)
>  {
>      int i;
>      X86CPU *cpu = NULL;
>      Error *error = NULL;
>  
>      /* init CPUs */
> -    if (cpu_model == NULL) {
> +    if (args->cpu_model == NULL) {
>  #ifdef TARGET_X86_64
> -        cpu_model = "qemu64";
> +        args->cpu_model = "qemu64";
>  #else
> -        cpu_model = "qemu32";
> +        args->cpu_model = "qemu32";
>  #endif
>      }
>  
>      for (i = 0; i < smp_cpus; i++) {
> -        cpu = pc_new_cpu(cpu_model, x86_cpu_apic_id_from_index(i),
> +        cpu = pc_new_cpu(args->cpu_model, x86_cpu_apic_id_from_index(i),
>                           icc_bridge, &error);
>          if (error) {
>              fprintf(stderr, "%s\n", error_get_pretty(error));
> @@ -943,7 +943,8 @@ void pc_cpus_init(const char *cpu_model, DeviceState *icc_bridge)
>      /* map APIC MMIO area if CPU has APIC */
>      if (cpu && cpu->env.apic_state) {
>          /* XXX: what if the base changes? */
> -        sysbus_mmio_map_overlap(icc_bridge, 0, APIC_DEFAULT_ADDRESS, 0x1000);
> +        sysbus_mmio_map_overlap(SYS_BUS_DEVICE(icc_bridge), 0,
> +                                APIC_DEFAULT_ADDRESS, 0x1000);
>      }
>  }
>  
> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> index 2190f0a..bdfac59 100644
> --- a/hw/i386/pc_piix.c
> +++ b/hw/i386/pc_piix.c
> @@ -59,12 +59,7 @@ static const int ide_irq[MAX_IDE_BUS] = { 14, 15 };
>  /* PC hardware initialisation */
>  static void pc_init1(MemoryRegion *system_memory,
>                       MemoryRegion *system_io,
> -                     ram_addr_t ram_size,
> -                     const char *boot_device,
> -                     const char *kernel_filename,
> -                     const char *kernel_cmdline,
> -                     const char *initrd_filename,
> -                     const char *cpu_model,
> +                     QEMUMachineInitArgs *args,
>                       int pci_enabled,
>                       int kvmclock_enabled)
>  {
> @@ -93,19 +88,19 @@ static void pc_init1(MemoryRegion *system_memory,
>      object_property_add_child(qdev_get_machine(), "icc-bridge",
>                                OBJECT(icc_bridge), NULL);
>  
> -    pc_cpus_init(cpu_model, icc_bridge);
> +    pc_cpus_init(args, icc_bridge);
>      pc_acpi_init("acpi-dsdt.aml");
>  
>      if (kvmclock_enabled) {
>          kvmclock_create();
>      }
>  
> -    if (ram_size >= 0xe0000000 ) {
> -        above_4g_mem_size = ram_size - 0xe0000000;
> +    if (args->ram_size >= 0xe0000000 ) {
> +        above_4g_mem_size = args->ram_size - 0xe0000000;
>          below_4g_mem_size = 0xe0000000;
>      } else {
>          above_4g_mem_size = 0;
> -        below_4g_mem_size = ram_size;
> +        below_4g_mem_size = args->ram_size;
>      }
>  
>      if (pci_enabled) {
> @@ -120,9 +115,9 @@ static void pc_init1(MemoryRegion *system_memory,
>      /* allocate ram and load rom/bios */
>      if (!xen_enabled()) {
>          fw_cfg = pc_memory_init(system_memory,
> -                       kernel_filename, kernel_cmdline, initrd_filename,
> -                       below_4g_mem_size, above_4g_mem_size,
> -                       rom_memory, &ram_memory);
> +                                args->kernel_filename, args->kernel_cmdline,
> +                                args->initrd_filename, below_4g_mem_size,
> +                                above_4g_mem_size, rom_memory, &ram_memory);
>      }
>  
>      gsi_state = g_malloc0(sizeof(*gsi_state));
> @@ -136,7 +131,7 @@ static void pc_init1(MemoryRegion *system_memory,
>  
>      if (pci_enabled) {
>          pci_bus = i440fx_init(&i440fx_state, &piix3_devfn, &isa_bus, gsi,
> -                              system_memory, system_io, ram_size,
> +                              system_memory, system_io, args->ram_size,
>                                below_4g_mem_size,
>                                0x100000000ULL - below_4g_mem_size,
>                                0x100000000ULL + above_4g_mem_size,
> @@ -203,7 +198,7 @@ static void pc_init1(MemoryRegion *system_memory,
>  
>      audio_init(isa_bus, pci_enabled ? pci_bus : NULL);
>  
> -    pc_cmos_init(below_4g_mem_size, above_4g_mem_size, boot_device,
> +    pc_cmos_init(below_4g_mem_size, above_4g_mem_size, args->boot_device,
>                   floppy, idebus[0], idebus[1], rtc_state);
>  
>      if (pci_enabled && usb_enabled(false)) {
> @@ -229,17 +224,7 @@ static void pc_init1(MemoryRegion *system_memory,
>  
>  static void pc_init_pci(QEMUMachineInitArgs *args)
>  {
> -    ram_addr_t ram_size = args->ram_size;
> -    const char *cpu_model = args->cpu_model;
> -    const char *kernel_filename = args->kernel_filename;
> -    const char *kernel_cmdline = args->kernel_cmdline;
> -    const char *initrd_filename = args->initrd_filename;
> -    const char *boot_device = args->boot_device;
> -    pc_init1(get_system_memory(),
> -             get_system_io(),
> -             ram_size, boot_device,
> -             kernel_filename, kernel_cmdline,
> -             initrd_filename, cpu_model, 1, 1);
> +    pc_init1(get_system_memory(), get_system_io(), args, 1, 1);
>  }
>  
>  static void pc_init_pci_1_4(QEMUMachineInitArgs *args)
> @@ -275,38 +260,19 @@ static void pc_init_pci_1_0(QEMUMachineInitArgs *args)
>  /* PC init function for pc-0.10 to pc-0.13, and reused by xenfv */
>  static void pc_init_pci_no_kvmclock(QEMUMachineInitArgs *args)
>  {
> -    ram_addr_t ram_size = args->ram_size;
> -    const char *cpu_model = args->cpu_model;
> -    const char *kernel_filename = args->kernel_filename;
> -    const char *kernel_cmdline = args->kernel_cmdline;
> -    const char *initrd_filename = args->initrd_filename;
> -    const char *boot_device = args->boot_device;
>      disable_kvm_pv_eoi();
>      enable_compat_apic_id_mode();
> -    pc_init1(get_system_memory(),
> -             get_system_io(),
> -             ram_size, boot_device,
> -             kernel_filename, kernel_cmdline,
> -             initrd_filename, cpu_model, 1, 0);
> +    pc_init1(get_system_memory(), get_system_io(), args, 1, 0);
>  }
>  
>  static void pc_init_isa(QEMUMachineInitArgs *args)
>  {
> -    ram_addr_t ram_size = args->ram_size;
> -    const char *cpu_model = args->cpu_model;
> -    const char *kernel_filename = args->kernel_filename;
> -    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";
> +    if (args->cpu_model == NULL) {
> +        args->cpu_model = "486";
> +    }
>      disable_kvm_pv_eoi();
>      enable_compat_apic_id_mode();
> -    pc_init1(get_system_memory(),
> -             get_system_io(),
> -             ram_size, boot_device,
> -             kernel_filename, kernel_cmdline,
> -             initrd_filename, cpu_model, 0, 1);
> +    pc_init1(get_system_memory(), get_system_io(), args, 0, 1);
>  }
>  
>  #ifdef CONFIG_XEN
> diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
> index a926e38..a6f0960 100644
> --- a/hw/i386/pc_q35.c
> +++ b/hw/i386/pc_q35.c
> @@ -49,12 +49,6 @@
>  /* PC hardware initialisation */
>  static void pc_q35_init(QEMUMachineInitArgs *args)
>  {
> -    ram_addr_t ram_size = args->ram_size;
> -    const char *cpu_model = args->cpu_model;
> -    const char *kernel_filename = args->kernel_filename;
> -    const char *kernel_cmdline = args->kernel_cmdline;
> -    const char *initrd_filename = args->initrd_filename;
> -    const char *boot_device = args->boot_device;
>      ram_addr_t below_4g_mem_size, above_4g_mem_size;
>      Q35PCIHost *q35_host;
>      PCIBus *host_bus;
> @@ -80,17 +74,17 @@ static void pc_q35_init(QEMUMachineInitArgs *args)
>      object_property_add_child(qdev_get_machine(), "icc-bridge",
>                                OBJECT(icc_bridge), NULL);
>  
> -    pc_cpus_init(cpu_model, icc_bridge);
> +    pc_cpus_init(args, icc_bridge);
>      pc_acpi_init("q35-acpi-dsdt.aml");
>  
>      kvmclock_create();
>  
> -    if (ram_size >= 0xb0000000) {
> -        above_4g_mem_size = ram_size - 0xb0000000;
> +    if (args->ram_size >= 0xb0000000) {
> +        above_4g_mem_size = args->ram_size - 0xb0000000;
>          below_4g_mem_size = 0xb0000000;
>      } else {
>          above_4g_mem_size = 0;
> -        below_4g_mem_size = ram_size;
> +        below_4g_mem_size = args->ram_size;
>      }
>  
>      /* pci enabled */
> @@ -105,9 +99,10 @@ static void pc_q35_init(QEMUMachineInitArgs *args)
>  
>      /* allocate ram and load rom/bios */
>      if (!xen_enabled()) {
> -        pc_memory_init(get_system_memory(), kernel_filename, kernel_cmdline,
> -                       initrd_filename, below_4g_mem_size, above_4g_mem_size,
> -                       rom_memory, &ram_memory);
> +        pc_memory_init(get_system_memory(),
> +                       args->kernel_filename, args->kernel_cmdline,
> +                       args->initrd_filename, below_4g_mem_size,
> +                       above_4g_mem_size, rom_memory, &ram_memory);
>      }
>  
>      /* irq lines */
> @@ -191,7 +186,7 @@ static void pc_q35_init(QEMUMachineInitArgs *args)
>                                      0xb100),
>                        8, NULL, 0);
>  
> -    pc_cmos_init(below_4g_mem_size, above_4g_mem_size, boot_device,
> +    pc_cmos_init(below_4g_mem_size, above_4g_mem_size, args->boot_device,
>                   floppy, idebus[0], idebus[1], rtc_state);
>  
>      /* the rest devices to which pci devfn is automatically assigned */
> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> index 8a6e76c..c86059f 100644
> --- a/include/hw/i386/pc.h
> +++ b/include/hw/i386/pc.h
> @@ -9,6 +9,7 @@
>  #include "net/net.h"
>  #include "exec/memory.h"
>  #include "hw/i386/ioapic.h"
> +#include "hw/boards.h"
>  
>  /* PC-style peripherals (also used by other machines).  */
>  
> @@ -78,7 +79,7 @@ extern int fd_bootchk;
>  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_cpus_init(QEMUMachineInitArgs *args, DeviceState *icc_bridge);
>  void pc_acpi_init(const char *default_dsdt);
>  void *pc_memory_init(MemoryRegion *system_memory,
>                      const char *kernel_filename,

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [Qemu-devel] [PATCH 5/7] add hot_add_cpu hook to QEMUMachine and export machine_args
  2013-04-29 15:02 ` [Qemu-devel] [PATCH 5/7] add hot_add_cpu hook to QEMUMachine and export machine_args Igor Mammedov
@ 2013-04-30  5:47   ` li guang
  0 siblings, 0 replies; 19+ messages in thread
From: li guang @ 2013-04-30  5:47 UTC (permalink / raw)
  To: Igor Mammedov
  Cc: aliguori, ehabkost, mst, jan.kiszka, claudio.fontana, qemu-devel,
	aderumier, lcapitulino, jfrei, yang.z.zhang, pbonzini, afaerber,
	rth

在 2013-04-29一的 17:02 +0200,Igor Mammedov写道:
> hot_add_cpu hook should be overriden by target that implements
> cpu hot-add via cpu-add QMP command.
> 
> Make machine_args available globaly, it allows to reuse
> machine_args->cpu_model during hotplug, instead of adding target
> specific globals to keep a copy of cpu_model.
> 
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> ---
> v2:
>   * convert stack veriable args to global machine_args
>   * don't move current_machine before machine->init(), override hot_add_cpu
>     staticaly instead.
> ---
>  include/hw/boards.h | 3 +++
>  vl.c                | 8 ++++++--
>  2 files changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/include/hw/boards.h b/include/hw/boards.h
> index 425bdc7..657f379 100644
> --- a/include/hw/boards.h
> +++ b/include/hw/boards.h
> @@ -18,6 +18,8 @@ typedef struct QEMUMachineInitArgs {
>      const char *cpu_model;
>  } QEMUMachineInitArgs;
>  
> +extern QEMUMachineInitArgs machine_args;
> +

I'm goint to think whether we can pus machine_args into
current_machine, that is add QEMUMachineInitArgs member
for QEMUMachine.
by this, we can avoid this global extern.

>  typedef void QEMUMachineInitFunc(QEMUMachineInitArgs *args);
>  
>  typedef void QEMUMachineResetFunc(void);
> @@ -43,6 +45,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);
> diff --git a/vl.c b/vl.c
> index 1e7d474..3fe5e94 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -179,6 +179,8 @@ int main(int argc, char **argv)
>  #define MAX_VIRTIO_CONSOLES 1
>  #define MAX_SCLP_CONSOLES 1
>  
> +QEMUMachineInitArgs machine_args;
> +
>  static const char *data_dir[16];
>  static int data_dir_idx;
>  const char *bios_name = NULL;
> @@ -4291,7 +4293,8 @@ int main(int argc, char **argv, char **envp)
>  
>      qdev_machine_init();
>  
> -    QEMUMachineInitArgs args = { .ram_size = ram_size,
> +    machine_args = (QEMUMachineInitArgs){
> +                                 .ram_size = ram_size,
>                                   .boot_device = (boot_devices[0] == '\0') ?
>                                                  machine->boot_order :
>                                                  boot_devices,
> @@ -4299,7 +4302,8 @@ int main(int argc, char **argv, char **envp)
>                                   .kernel_cmdline = kernel_cmdline,
>                                   .initrd_filename = initrd_filename,
>                                   .cpu_model = cpu_model };
> -    machine->init(&args);
> +
> +    machine->init(&machine_args);
>  
>      cpu_synchronize_all_post_init();
>  

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2013-04-30  5:50 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-04-29 15:02 [Qemu-devel] [PATCH 0/7 v7 for 1.5] target-i386: CPU hot-add with cpu-add QMP command Igor Mammedov
2013-04-29 15:02 ` [Qemu-devel] [PATCH 1/7] target-i386: Introduce ICC bus/device/bridge Igor Mammedov
2013-04-29 16:39   ` Andreas Färber
2013-04-29 15:02 ` [Qemu-devel] [PATCH 2/7] target-i386: Attach ICC bus to CPU on its creation Igor Mammedov
2013-04-29 16:24   ` Andreas Färber
2013-04-29 16:33     ` Igor Mammedov
2013-04-29 16:54   ` [Qemu-devel] [PATCH 2/7 v8] " Igor Mammedov
2013-04-29 17:04     ` Andreas Färber
2013-04-29 15:02 ` [Qemu-devel] [PATCH 3/7] target-i386: Move APIC to ICC bus Igor Mammedov
2013-04-29 16:36   ` Igor Mammedov
2013-04-29 17:03   ` [Qemu-devel] [PATCH 3/7 v8] " Igor Mammedov
2013-04-29 17:15     ` Andreas Färber
2013-04-29 17:34       ` Igor Mammedov
2013-04-29 15:02 ` [Qemu-devel] [PATCH 4/7] pc: pass QEMUMachineInitArgs down to pc_cpus_init() Igor Mammedov
2013-04-30  5:43   ` li guang
2013-04-29 15:02 ` [Qemu-devel] [PATCH 5/7] add hot_add_cpu hook to QEMUMachine and export machine_args Igor Mammedov
2013-04-30  5:47   ` li guang
2013-04-29 15:02 ` [Qemu-devel] [PATCH 6/7] target-i386: implement machine->hot_add_cpu hook Igor Mammedov
2013-04-29 15:02 ` [Qemu-devel] [PATCH 7/7] QMP: add cpu-add command Igor Mammedov

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.