qemu-devel.nongnu.org archive mirror
 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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).