[Qemu-devel] [PATCH v12 0/5] remove icc bus/bridge

[Qemu-devel] [PATCH v12 0/5] remove icc bus/bridge

[Zhu Guihua]

ICC Bus was used for providing a hotpluggable bus for APIC and CPU, but now we
use HotplugHandler to make hotplug. So ICC Bus is unnecessary.

This code has passed the new pc-cpu-test.
And I have tested with kvm along with kernel_irqchip=on/off, it works fine.

This patch series is based on the latest master.

v12:
 -move APIC MMIO mapping into x86_cpu_apic_realize()
 -change commit message in PATCH 4

v11:
 -improve commit messages
 -split per CPU AS change into a separate patch

v10:
 -improve commit messages in patch 1 and 2
 -make the check of cpu->cpu_as_root simplier

v9:
 -use a callback to correct reset sequence for x86
 -update apic mmio mapping

Chen Fan (2):
  apic: move APIC's MMIO region mapping into APIC
  cpu/apic: drop icc bus/bridge

Zhu Guihua (3):
  apic: use per CPU AS to map APIC MMIO for TCG
  x86: use new method to correct reset sequence
  icc_bus: drop the unused files

 default-configs/i386-softmmu.mak   |   1 -
 default-configs/x86_64-softmmu.mak |   1 -
 hw/cpu/Makefile.objs               |   1 -
 hw/cpu/icc_bus.c                   | 118 -------------------------------------
 hw/i386/pc.c                       |  46 ++++++++-------
 hw/i386/pc_piix.c                  |   9 +--
 hw/i386/pc_q35.c                   |   9 +--
 hw/intc/apic_common.c              |  11 +---
 include/hw/cpu/icc_bus.h           |  82 --------------------------
 include/hw/i386/apic_internal.h    |   7 ++-
 include/hw/i386/pc.h               |   2 +-
 target-i386/cpu.c                  |  33 ++++++++---
 12 files changed, 58 insertions(+), 262 deletions(-)
 delete mode 100644 hw/cpu/icc_bus.c
 delete mode 100644 include/hw/cpu/icc_bus.h

-- 
1.9.3

[Qemu-devel] [PATCH v12 1/5] apic: move APIC's MMIO region mapping into APIC

[Zhu Guihua]

From: Chen Fan <chen.fan.fnst@cn.fujitsu.com>

When ICC bus/bridge is removed, APIC MMIO will be left
unmapped since it was mapped into system's address space
indirectly by ICC bridge.
Fix it by moving mapping into APIC code, so it would be
possible to remove ICC bus/bridge code later.

Signed-off-by: Chen Fan <chen.fan.fnst@cn.fujitsu.com>
Signed-off-by: Zhu Guihua <zhugh.fnst@cn.fujitsu.com>
---
 hw/i386/pc.c          |  7 -------
 hw/intc/apic_common.c |  6 ------
 target-i386/cpu.c     | 15 +++++++++++++++
 3 files changed, 15 insertions(+), 13 deletions(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 56aecce..631f89f 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1158,13 +1158,6 @@ void pc_cpus_init(const char *cpu_model, DeviceState *icc_bridge)
         object_unref(OBJECT(cpu));
     }
 
-    /* map APIC MMIO area if CPU has APIC */
-    if (cpu && cpu->apic_state) {
-        /* XXX: what if the base changes? */
-        sysbus_mmio_map_overlap(SYS_BUS_DEVICE(icc_bridge), 0,
-                                APIC_DEFAULT_ADDRESS, 0x1000);
-    }
-
     /* tell smbios about cpuid version and features */
     smbios_set_cpuid(cpu->env.cpuid_version, cpu->env.features[FEAT_1_EDX]);
 }
diff --git a/hw/intc/apic_common.c b/hw/intc/apic_common.c
index 0032b97..c0b32eb 100644
--- a/hw/intc/apic_common.c
+++ b/hw/intc/apic_common.c
@@ -296,7 +296,6 @@ static void apic_common_realize(DeviceState *dev, Error **errp)
     APICCommonClass *info;
     static DeviceState *vapic;
     static int apic_no;
-    static bool mmio_registered;
 
     if (apic_no >= MAX_APICS) {
         error_setg(errp, "%s initialization failed.",
@@ -307,11 +306,6 @@ static void apic_common_realize(DeviceState *dev, Error **errp)
 
     info = APIC_COMMON_GET_CLASS(s);
     info->realize(dev, errp);
-    if (!mmio_registered) {
-        ICCBus *b = ICC_BUS(qdev_get_parent_bus(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 &&
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index cfb8aa7..050ce11 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -2745,15 +2745,30 @@ static void x86_cpu_apic_create(X86CPU *cpu, Error **errp)
     /* TODO: convert to link<> */
     apic = APIC_COMMON(cpu->apic_state);
     apic->cpu = cpu;
+    apic->apicbase = APIC_DEFAULT_ADDRESS | MSR_IA32_APICBASE_ENABLE;
 }
 
 static void x86_cpu_apic_realize(X86CPU *cpu, Error **errp)
 {
+    APICCommonState *apic;
+    static bool apic_mmio_map_once;
+
     if (cpu->apic_state == NULL) {
         return;
     }
     object_property_set_bool(OBJECT(cpu->apic_state), true, "realized",
                              errp);
+
+    /* Map APIC MMIO area */
+    apic = APIC_COMMON(cpu->apic_state);
+    if (!apic_mmio_map_once) {
+        memory_region_add_subregion_overlap(get_system_memory(),
+                                            apic->apicbase &
+                                            MSR_IA32_APICBASE_BASE,
+                                            &apic->io_memory,
+                                            0x1000);
+        apic_mmio_map_once = true;
+     }
 }
 
 static void x86_cpu_machine_done(Notifier *n, void *unused)
-- 
1.9.3

[Qemu-devel] [PATCH v12 2/5] apic: use per CPU AS to map APIC MMIO for TCG

[Zhu Guihua]

TCG supports per CPU address space, and the emulation quality is
a bit better with it.
So use per CPU address space to map APIC MMIO area. This allows the
APIC base address of each cpu to be moved independent of others.

Signed-off-by: Zhu Guihua <zhugh.fnst@cn.fujitsu.com>
---
 target-i386/cpu.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 050ce11..692a284 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -2759,9 +2759,18 @@ static void x86_cpu_apic_realize(X86CPU *cpu, Error **errp)
     object_property_set_bool(OBJECT(cpu->apic_state), true, "realized",
                              errp);
 
-    /* Map APIC MMIO area */
+    /* Map APIC MMIO area, use per-CPU address space if available (TCG
+     * supports it, KVM doesn't). This allows the APIC base address of
+     * each CPU to be moved independently.
+     */
     apic = APIC_COMMON(cpu->apic_state);
-    if (!apic_mmio_map_once) {
+    if (tcg_enabled()) {
+        memory_region_add_subregion_overlap(cpu->cpu_as_root,
+                                            apic->apicbase &
+                                            MSR_IA32_APICBASE_BASE,
+                                            &apic->io_memory,
+                                            0x1000);
+    } else if (!apic_mmio_map_once) {
         memory_region_add_subregion_overlap(get_system_memory(),
                                             apic->apicbase &
                                             MSR_IA32_APICBASE_BASE,
-- 
1.9.3

[Qemu-devel] [PATCH v12 3/5] x86: use new method to correct reset sequence

[Zhu Guihua]

During reset some devices (such as hpet, rtc) might send IRQ to APIC
which changes APIC's state from default one it's supposed to have
at machine startup time.
Fix this by resetting APIC after devices have been reset to cancel
any changes that qemu_devices_reset() might have done to its state.

Signed-off-by: Zhu Guihua <zhugh.fnst@cn.fujitsu.com>
---
 hw/i386/pc.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 631f89f..b414055 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1924,6 +1924,25 @@ static void pc_machine_initfn(Object *obj)
                              NULL, &error_abort);
 }
 
+static void pc_machine_reset(void)
+{
+    CPUState *cs;
+    X86CPU *cpu;
+
+    qemu_devices_reset();
+
+    /* Reset APIC after devices have been reset to cancel
+     * any changes that qemu_devices_reset() might have done.
+     */
+    CPU_FOREACH(cs) {
+        cpu = X86_CPU(cs);
+
+        if (cpu->apic_state) {
+            device_reset(cpu->apic_state);
+        }
+    }
+}
+
 static unsigned pc_cpu_index_to_socket_id(unsigned cpu_index)
 {
     unsigned pkg_id, core_id, smt_id;
@@ -1944,6 +1963,7 @@ static void pc_machine_class_init(ObjectClass *oc, void *data)
     mc->default_boot_order = "cad";
     mc->hot_add_cpu = pc_hot_add_cpu;
     mc->max_cpus = 255;
+    mc->reset = pc_machine_reset;
     hc->plug = pc_machine_device_plug_cb;
     hc->unplug_request = pc_machine_device_unplug_request_cb;
     hc->unplug = pc_machine_device_unplug_cb;
-- 
1.9.3

[Qemu-devel] [PATCH v12 4/5] cpu/apic: drop icc bus/bridge

[Zhu Guihua]

From: Chen Fan <chen.fan.fnst@cn.fujitsu.com>

After CPU hotplug has been converted to BUS-less hot-plug infrastructure,
the only function ICC bus performs is to propagate reset to LAPICs. However
LAPIC could be reset by registering its reset handler after all device are
initialized.
Do so and drop ~30LOC of not needed anymore ICCBus related code.

Signed-off-by: Chen Fan <chen.fan.fnst@cn.fujitsu.com>
Signed-off-by: Zhu Guihua <zhugh.fnst@cn.fujitsu.com>
---
 hw/i386/pc.c                    | 19 ++++---------------
 hw/i386/pc_piix.c               |  9 +--------
 hw/i386/pc_q35.c                |  9 +--------
 hw/intc/apic_common.c           |  5 ++---
 include/hw/i386/apic_internal.h |  7 ++++---
 include/hw/i386/pc.h            |  2 +-
 target-i386/cpu.c               |  9 +--------
 7 files changed, 14 insertions(+), 46 deletions(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index b414055..1a690d0 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -59,7 +59,6 @@
 #include "qemu/error-report.h"
 #include "hw/acpi/acpi.h"
 #include "hw/acpi/cpu_hotplug.h"
-#include "hw/cpu/icc_bus.h"
 #include "hw/boards.h"
 #include "hw/pci/pci_host.h"
 #include "acpi-build.h"
@@ -1052,23 +1051,16 @@ void pc_acpi_smi_interrupt(void *opaque, int irq, int level)
 }
 
 static X86CPU *pc_new_cpu(const char *cpu_model, int64_t apic_id,
-                          DeviceState *icc_bridge, Error **errp)
+                          Error **errp)
 {
     X86CPU *cpu = NULL;
     Error *local_err = NULL;
 
-    if (icc_bridge == NULL) {
-        error_setg(&local_err, "Invalid icc-bridge value");
-        goto out;
-    }
-
     cpu = cpu_x86_create(cpu_model, &local_err);
     if (local_err != NULL) {
         goto out;
     }
 
-    qdev_set_parent_bus(DEVICE(cpu), qdev_get_child_bus(icc_bridge, "icc"));
-
     object_property_set_int(OBJECT(cpu), apic_id, "apic-id", &local_err);
     object_property_set_bool(OBJECT(cpu), true, "realized", &local_err);
 
@@ -1085,7 +1077,6 @@ static const char *current_cpu_model;
 
 void pc_hot_add_cpu(const int64_t id, Error **errp)
 {
-    DeviceState *icc_bridge;
     X86CPU *cpu;
     int64_t apic_id = x86_cpu_apic_id_from_index(id);
     Error *local_err = NULL;
@@ -1114,9 +1105,7 @@ void pc_hot_add_cpu(const int64_t id, Error **errp)
         return;
     }
 
-    icc_bridge = DEVICE(object_resolve_path_type("icc-bridge",
-                                                 TYPE_ICC_BRIDGE, NULL));
-    cpu = pc_new_cpu(current_cpu_model, apic_id, icc_bridge, &local_err);
+    cpu = pc_new_cpu(current_cpu_model, apic_id, &local_err);
     if (local_err) {
         error_propagate(errp, local_err);
         return;
@@ -1124,7 +1113,7 @@ void pc_hot_add_cpu(const int64_t id, Error **errp)
     object_unref(OBJECT(cpu));
 }
 
-void pc_cpus_init(const char *cpu_model, DeviceState *icc_bridge)
+void pc_cpus_init(const char *cpu_model)
 {
     int i;
     X86CPU *cpu = NULL;
@@ -1150,7 +1139,7 @@ void pc_cpus_init(const char *cpu_model, DeviceState *icc_bridge)
 
     for (i = 0; i < smp_cpus; i++) {
         cpu = pc_new_cpu(cpu_model, x86_cpu_apic_id_from_index(i),
-                         icc_bridge, &error);
+                         &error);
         if (error) {
             error_report_err(error);
             exit(1);
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 3f925b2..4335e9e 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -39,7 +39,6 @@
 #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/block-backend.h"
 #include "hw/i2c/smbus.h"
@@ -98,7 +97,6 @@ static void pc_init1(MachineState *machine,
     MemoryRegion *ram_memory;
     MemoryRegion *pci_memory;
     MemoryRegion *rom_memory;
-    DeviceState *icc_bridge;
     PcGuestInfo *guest_info;
     ram_addr_t lowmem;
 
@@ -141,11 +139,7 @@ static void pc_init1(MachineState *machine,
         exit(1);
     }
 
-    icc_bridge = qdev_create(NULL, TYPE_ICC_BRIDGE);
-    object_property_add_child(qdev_get_machine(), "icc-bridge",
-                              OBJECT(icc_bridge), NULL);
-
-    pc_cpus_init(machine->cpu_model, icc_bridge);
+    pc_cpus_init(machine->cpu_model);
 
     if (kvm_enabled() && kvmclock_enabled) {
         kvmclock_create();
@@ -226,7 +220,6 @@ static void pc_init1(MachineState *machine,
     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 11601ab..616ec68 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -43,7 +43,6 @@
 #include "hw/ide/pci.h"
 #include "hw/ide/ahci.h"
 #include "hw/usb.h"
-#include "hw/cpu/icc_bus.h"
 #include "qemu/error-report.h"
 #include "migration/migration.h"
 
@@ -83,7 +82,6 @@ static void pc_q35_init(MachineState *machine)
     int i;
     ICH9LPCState *ich9_lpc;
     PCIDevice *ahci;
-    DeviceState *icc_bridge;
     PcGuestInfo *guest_info;
     ram_addr_t lowmem;
     DriveInfo *hd[MAX_SATA_PORTS];
@@ -130,11 +128,7 @@ static void pc_q35_init(MachineState *machine)
         exit(1);
     }
 
-    icc_bridge = qdev_create(NULL, TYPE_ICC_BRIDGE);
-    object_property_add_child(qdev_get_machine(), "icc-bridge",
-                              OBJECT(icc_bridge), NULL);
-
-    pc_cpus_init(machine->cpu_model, icc_bridge);
+    pc_cpus_init(machine->cpu_model);
     pc_acpi_init("q35-acpi-dsdt.aml");
 
     kvmclock_create();
@@ -236,7 +230,6 @@ static void pc_q35_init(MachineState *machine)
     if (pci_enabled) {
         ioapic_init_gsi(gsi_state, "q35");
     }
-    qdev_init_nofail(icc_bridge);
 
     pc_register_ferr_irq(gsi[13]);
 
diff --git a/hw/intc/apic_common.c b/hw/intc/apic_common.c
index c0b32eb..ad959c4 100644
--- a/hw/intc/apic_common.c
+++ b/hw/intc/apic_common.c
@@ -419,13 +419,12 @@ static Property apic_properties_common[] = {
 
 static void apic_common_class_init(ObjectClass *klass, void *data)
 {
-    ICCDeviceClass *idc = ICC_DEVICE_CLASS(klass);
     DeviceClass *dc = DEVICE_CLASS(klass);
 
     dc->vmsd = &vmstate_apic_common;
     dc->reset = apic_reset_common;
     dc->props = apic_properties_common;
-    idc->realize = apic_common_realize;
+    dc->realize = apic_common_realize;
     /*
      * Reason: APIC and CPU need to be wired up by
      * x86_cpu_apic_create()
@@ -435,7 +434,7 @@ static void apic_common_class_init(ObjectClass *klass, void *data)
 
 static const TypeInfo apic_common_type = {
     .name = TYPE_APIC_COMMON,
-    .parent = TYPE_ICC_DEVICE,
+    .parent = TYPE_DEVICE,
     .instance_size = sizeof(APICCommonState),
     .class_size = sizeof(APICCommonClass),
     .class_init = apic_common_class_init,
diff --git a/include/hw/i386/apic_internal.h b/include/hw/i386/apic_internal.h
index 26632ac..905ada7 100644
--- a/include/hw/i386/apic_internal.h
+++ b/include/hw/i386/apic_internal.h
@@ -22,7 +22,6 @@
 
 #include "cpu.h"
 #include "exec/memory.h"
-#include "hw/cpu/icc_bus.h"
 #include "qemu/timer.h"
 
 /* APIC Local Vector Table */
@@ -79,7 +78,7 @@ typedef struct APICCommonState APICCommonState;
 
 typedef struct APICCommonClass
 {
-    ICCDeviceClass parent_class;
+    DeviceClass parent_class;
 
     DeviceRealize realize;
     void (*set_base)(APICCommonState *s, uint64_t val);
@@ -94,7 +93,9 @@ typedef struct APICCommonClass
 } APICCommonClass;
 
 struct APICCommonState {
-    ICCDevice busdev;
+    /*< private >*/
+    DeviceState parent_obj;
+    /*< public >*/
 
     MemoryRegion io_memory;
     X86CPU *cpu;
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 3e002c9..8a9c57a 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -162,7 +162,7 @@ bool pc_machine_is_smm_enabled(PCMachineState *pcms);
 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(const char *cpu_model);
 void pc_hot_add_cpu(const int64_t id, Error **errp);
 void pc_acpi_init(const char *default_dsdt);
 
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 692a284..8228fc7 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -43,7 +43,6 @@
 
 #include "sysemu/sysemu.h"
 #include "hw/qdev-properties.h"
-#include "hw/cpu/icc_bus.h"
 #ifndef CONFIG_USER_ONLY
 #include "exec/address-spaces.h"
 #include "hw/xen/xen.h"
@@ -2723,7 +2722,6 @@ static void mce_init(X86CPU *cpu)
 #ifndef CONFIG_USER_ONLY
 static void x86_cpu_apic_create(X86CPU *cpu, Error **errp)
 {
-    DeviceState *dev = DEVICE(cpu);
     APICCommonState *apic;
     const char *apic_type = "apic";
 
@@ -2733,11 +2731,7 @@ static void x86_cpu_apic_create(X86CPU *cpu, Error **errp)
         apic_type = "xen-apic";
     }
 
-    cpu->apic_state = qdev_try_create(qdev_get_parent_bus(dev), apic_type);
-    if (cpu->apic_state == NULL) {
-        error_setg(errp, "APIC device '%s' could not be created", apic_type);
-        return;
-    }
+    cpu->apic_state = DEVICE(object_new(apic_type));
 
     object_property_add_child(OBJECT(cpu), "apic",
                               OBJECT(cpu->apic_state), NULL);
@@ -3162,7 +3156,6 @@ 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;
     dc->props = x86_cpu_properties;
 
     xcc->parent_reset = cc->reset;
-- 
1.9.3

[Qemu-devel] [PATCH v12 5/5] icc_bus: drop the unused files

[Zhu Guihua]

ICC bus impl has been droped, so all icc related files are not useful
any more; delete them.

Signed-off-by: Zhu Guihua <zhugh.fnst@cn.fujitsu.com>
---
 default-configs/i386-softmmu.mak   |   1 -
 default-configs/x86_64-softmmu.mak |   1 -
 hw/cpu/Makefile.objs               |   1 -
 hw/cpu/icc_bus.c                   | 118 -------------------------------------
 include/hw/cpu/icc_bus.h           |  82 --------------------------
 5 files changed, 203 deletions(-)
 delete mode 100644 hw/cpu/icc_bus.c
 delete mode 100644 include/hw/cpu/icc_bus.h

diff --git a/default-configs/i386-softmmu.mak b/default-configs/i386-softmmu.mak
index 9393cf0..43c96d1 100644
--- a/default-configs/i386-softmmu.mak
+++ b/default-configs/i386-softmmu.mak
@@ -44,7 +44,6 @@ CONFIG_LPC_ICH9=y
 CONFIG_PCI_Q35=y
 CONFIG_APIC=y
 CONFIG_IOAPIC=y
-CONFIG_ICC_BUS=y
 CONFIG_PVPANIC=y
 CONFIG_MEM_HOTPLUG=y
 CONFIG_XIO3130=y
diff --git a/default-configs/x86_64-softmmu.mak b/default-configs/x86_64-softmmu.mak
index 28e2099..dfb8095 100644
--- a/default-configs/x86_64-softmmu.mak
+++ b/default-configs/x86_64-softmmu.mak
@@ -44,7 +44,6 @@ CONFIG_LPC_ICH9=y
 CONFIG_PCI_Q35=y
 CONFIG_APIC=y
 CONFIG_IOAPIC=y
-CONFIG_ICC_BUS=y
 CONFIG_PVPANIC=y
 CONFIG_MEM_HOTPLUG=y
 CONFIG_XIO3130=y
diff --git a/hw/cpu/Makefile.objs b/hw/cpu/Makefile.objs
index 6381238..0954a18 100644
--- a/hw/cpu/Makefile.objs
+++ b/hw/cpu/Makefile.objs
@@ -2,5 +2,4 @@ obj-$(CONFIG_ARM11MPCORE) += arm11mpcore.o
 obj-$(CONFIG_REALVIEW) += realview_mpcore.o
 obj-$(CONFIG_A9MPCORE) += a9mpcore.o
 obj-$(CONFIG_A15MPCORE) += a15mpcore.o
-obj-$(CONFIG_ICC_BUS) += icc_bus.o
 
diff --git a/hw/cpu/icc_bus.c b/hw/cpu/icc_bus.c
deleted file mode 100644
index 6646ea2..0000000
--- a/hw/cpu/icc_bus.c
+++ /dev/null
@@ -1,118 +0,0 @@
-/* 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 const TypeInfo icc_bus_info = {
-    .name = TYPE_ICC_BUS,
-    .parent = TYPE_BUS,
-    .instance_size = sizeof(ICCBus),
-};
-
-
-/* icc-device implementation */
-
-static void icc_device_realize(DeviceState *dev, Error **errp)
-{
-    ICCDeviceClass *idc = ICC_DEVICE_GET_CLASS(dev);
-
-    /* convert to QOM */
-    if (idc->realize) {
-        idc->realize(dev, errp);
-    }
-
-}
-
-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;
-    MemoryRegion apic_container;
-} ICCBridgeState;
-
-#define ICC_BRIDGE(obj) OBJECT_CHECK(ICCBridgeState, (obj), TYPE_ICC_BRIDGE)
-
-static void icc_bridge_init(Object *obj)
-{
-    ICCBridgeState *s = ICC_BRIDGE(obj);
-    SysBusDevice *sb = SYS_BUS_DEVICE(obj);
-
-    qbus_create_inplace(&s->icc_bus, sizeof(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, obj, "icc-apic-container",
-                       APIC_SPACE_SIZE);
-    sysbus_init_mmio(sb, &s->apic_container);
-    s->icc_bus.apic_address_space = &s->apic_container;
-}
-
-static void icc_bridge_class_init(ObjectClass *oc, void *data)
-{
-    DeviceClass *dc = DEVICE_CLASS(oc);
-
-    set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories);
-}
-
-static const TypeInfo icc_bridge_info = {
-    .name  = TYPE_ICC_BRIDGE,
-    .parent = TYPE_SYS_BUS_DEVICE,
-    .instance_init  = icc_bridge_init,
-    .instance_size  = sizeof(ICCBridgeState),
-    .class_init = icc_bridge_class_init,
-};
-
-
-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/include/hw/cpu/icc_bus.h b/include/hw/cpu/icc_bus.h
deleted file mode 100644
index 98a979f..0000000
--- a/include/hw/cpu/icc_bus.h
+++ /dev/null
@@ -1,82 +0,0 @@
-/* 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 "exec/memory.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 >*/
-
-    MemoryRegion *apic_address_space;
-} 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 >*/
-
-    DeviceRealize 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.9.3

Re: [Qemu-devel] [PATCH v12 1/5] apic: move APIC's MMIO region mapping into APIC

[Igor Mammedov]

On Wed, 16 Sep 2015 17:19:11 +0800
Zhu Guihua <zhugh.fnst@cn.fujitsu.com> wrote:

> From: Chen Fan <chen.fan.fnst@cn.fujitsu.com>
> 
> When ICC bus/bridge is removed, APIC MMIO will be left
> unmapped since it was mapped into system's address space
> indirectly by ICC bridge.
> Fix it by moving mapping into APIC code, so it would be
> possible to remove ICC bus/bridge code later.
> 
> Signed-off-by: Chen Fan <chen.fan.fnst@cn.fujitsu.com>
> Signed-off-by: Zhu Guihua <zhugh.fnst@cn.fujitsu.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>

> ---
>  hw/i386/pc.c          |  7 -------
>  hw/intc/apic_common.c |  6 ------
>  target-i386/cpu.c     | 15 +++++++++++++++
>  3 files changed, 15 insertions(+), 13 deletions(-)
> 
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 56aecce..631f89f 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -1158,13 +1158,6 @@ void pc_cpus_init(const char *cpu_model, DeviceState *icc_bridge)
>          object_unref(OBJECT(cpu));
>      }
>  
> -    /* map APIC MMIO area if CPU has APIC */
> -    if (cpu && cpu->apic_state) {
> -        /* XXX: what if the base changes? */
> -        sysbus_mmio_map_overlap(SYS_BUS_DEVICE(icc_bridge), 0,
> -                                APIC_DEFAULT_ADDRESS, 0x1000);
> -    }
> -
>      /* tell smbios about cpuid version and features */
>      smbios_set_cpuid(cpu->env.cpuid_version, cpu->env.features[FEAT_1_EDX]);
>  }
> diff --git a/hw/intc/apic_common.c b/hw/intc/apic_common.c
> index 0032b97..c0b32eb 100644
> --- a/hw/intc/apic_common.c
> +++ b/hw/intc/apic_common.c
> @@ -296,7 +296,6 @@ static void apic_common_realize(DeviceState *dev, Error **errp)
>      APICCommonClass *info;
>      static DeviceState *vapic;
>      static int apic_no;
> -    static bool mmio_registered;
>  
>      if (apic_no >= MAX_APICS) {
>          error_setg(errp, "%s initialization failed.",
> @@ -307,11 +306,6 @@ static void apic_common_realize(DeviceState *dev, Error **errp)
>  
>      info = APIC_COMMON_GET_CLASS(s);
>      info->realize(dev, errp);
> -    if (!mmio_registered) {
> -        ICCBus *b = ICC_BUS(qdev_get_parent_bus(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 &&
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index cfb8aa7..050ce11 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
> @@ -2745,15 +2745,30 @@ static void x86_cpu_apic_create(X86CPU *cpu, Error **errp)
>      /* TODO: convert to link<> */
>      apic = APIC_COMMON(cpu->apic_state);
>      apic->cpu = cpu;
> +    apic->apicbase = APIC_DEFAULT_ADDRESS | MSR_IA32_APICBASE_ENABLE;
>  }
>  
>  static void x86_cpu_apic_realize(X86CPU *cpu, Error **errp)
>  {
> +    APICCommonState *apic;
> +    static bool apic_mmio_map_once;
> +
>      if (cpu->apic_state == NULL) {
>          return;
>      }
>      object_property_set_bool(OBJECT(cpu->apic_state), true, "realized",
>                               errp);
> +
> +    /* Map APIC MMIO area */
> +    apic = APIC_COMMON(cpu->apic_state);
> +    if (!apic_mmio_map_once) {
> +        memory_region_add_subregion_overlap(get_system_memory(),
> +                                            apic->apicbase &
> +                                            MSR_IA32_APICBASE_BASE,
> +                                            &apic->io_memory,
> +                                            0x1000);
> +        apic_mmio_map_once = true;
> +     }
>  }
>  
>  static void x86_cpu_machine_done(Notifier *n, void *unused)

Re: [Qemu-devel] [PATCH v12 2/5] apic: use per CPU AS to map APIC MMIO for TCG

[Igor Mammedov]

On Wed, 16 Sep 2015 17:19:12 +0800
Zhu Guihua <zhugh.fnst@cn.fujitsu.com> wrote:

> TCG supports per CPU address space, and the emulation quality is
> a bit better with it.
> So use per CPU address space to map APIC MMIO area. This allows the
> APIC base address of each cpu to be moved independent of others.
> 
> Signed-off-by: Zhu Guihua <zhugh.fnst@cn.fujitsu.com>

Reviewed-by: Igor Mammedov <imammedo@redhat.com>

> ---
>  target-i386/cpu.c | 13 +++++++++++--
>  1 file changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index 050ce11..692a284 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
> @@ -2759,9 +2759,18 @@ static void x86_cpu_apic_realize(X86CPU *cpu, Error **errp)
>      object_property_set_bool(OBJECT(cpu->apic_state), true, "realized",
>                               errp);
>  
> -    /* Map APIC MMIO area */
> +    /* Map APIC MMIO area, use per-CPU address space if available (TCG
> +     * supports it, KVM doesn't). This allows the APIC base address of
> +     * each CPU to be moved independently.
> +     */
>      apic = APIC_COMMON(cpu->apic_state);
> -    if (!apic_mmio_map_once) {
> +    if (tcg_enabled()) {
> +        memory_region_add_subregion_overlap(cpu->cpu_as_root,
> +                                            apic->apicbase &
> +                                            MSR_IA32_APICBASE_BASE,
> +                                            &apic->io_memory,
> +                                            0x1000);
> +    } else if (!apic_mmio_map_once) {
>          memory_region_add_subregion_overlap(get_system_memory(),
>                                              apic->apicbase &
>                                              MSR_IA32_APICBASE_BASE,

Re: [Qemu-devel] [PATCH v12 3/5] x86: use new method to correct reset sequence

[Igor Mammedov]

On Wed, 16 Sep 2015 17:19:13 +0800
Zhu Guihua <zhugh.fnst@cn.fujitsu.com> wrote:

> During reset some devices (such as hpet, rtc) might send IRQ to APIC
> which changes APIC's state from default one it's supposed to have
> at machine startup time.
> Fix this by resetting APIC after devices have been reset to cancel
> any changes that qemu_devices_reset() might have done to its state.
> 
> Signed-off-by: Zhu Guihua <zhugh.fnst@cn.fujitsu.com>

Reviewed-by: Igor Mammedov <imammedo@redhat.com>

> ---
>  hw/i386/pc.c | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 631f89f..b414055 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -1924,6 +1924,25 @@ static void pc_machine_initfn(Object *obj)
>                               NULL, &error_abort);
>  }
>  
> +static void pc_machine_reset(void)
> +{
> +    CPUState *cs;
> +    X86CPU *cpu;
> +
> +    qemu_devices_reset();
> +
> +    /* Reset APIC after devices have been reset to cancel
> +     * any changes that qemu_devices_reset() might have done.
> +     */
> +    CPU_FOREACH(cs) {
> +        cpu = X86_CPU(cs);
> +
> +        if (cpu->apic_state) {
> +            device_reset(cpu->apic_state);
> +        }
> +    }
> +}
> +
>  static unsigned pc_cpu_index_to_socket_id(unsigned cpu_index)
>  {
>      unsigned pkg_id, core_id, smt_id;
> @@ -1944,6 +1963,7 @@ static void pc_machine_class_init(ObjectClass *oc, void *data)
>      mc->default_boot_order = "cad";
>      mc->hot_add_cpu = pc_hot_add_cpu;
>      mc->max_cpus = 255;
> +    mc->reset = pc_machine_reset;
>      hc->plug = pc_machine_device_plug_cb;
>      hc->unplug_request = pc_machine_device_unplug_request_cb;
>      hc->unplug = pc_machine_device_unplug_cb;

Re: [Qemu-devel] [PATCH v12 4/5] cpu/apic: drop icc bus/bridge

[Igor Mammedov]

On Wed, 16 Sep 2015 17:19:14 +0800
Zhu Guihua <zhugh.fnst@cn.fujitsu.com> wrote:

> From: Chen Fan <chen.fan.fnst@cn.fujitsu.com>
> 
> After CPU hotplug has been converted to BUS-less hot-plug infrastructure,
> the only function ICC bus performs is to propagate reset to LAPICs. However
> LAPIC could be reset by registering its reset handler after all device are
> initialized.
> Do so and drop ~30LOC of not needed anymore ICCBus related code.
> 
> Signed-off-by: Chen Fan <chen.fan.fnst@cn.fujitsu.com>
> Signed-off-by: Zhu Guihua <zhugh.fnst@cn.fujitsu.com>

Reviewed-by: Igor Mammedov <imammedo@redhat.com>

> ---
>  hw/i386/pc.c                    | 19 ++++---------------
>  hw/i386/pc_piix.c               |  9 +--------
>  hw/i386/pc_q35.c                |  9 +--------
>  hw/intc/apic_common.c           |  5 ++---
>  include/hw/i386/apic_internal.h |  7 ++++---
>  include/hw/i386/pc.h            |  2 +-
>  target-i386/cpu.c               |  9 +--------
>  7 files changed, 14 insertions(+), 46 deletions(-)
> 
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index b414055..1a690d0 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -59,7 +59,6 @@
>  #include "qemu/error-report.h"
>  #include "hw/acpi/acpi.h"
>  #include "hw/acpi/cpu_hotplug.h"
> -#include "hw/cpu/icc_bus.h"
>  #include "hw/boards.h"
>  #include "hw/pci/pci_host.h"
>  #include "acpi-build.h"
> @@ -1052,23 +1051,16 @@ void pc_acpi_smi_interrupt(void *opaque, int irq, int level)
>  }
>  
>  static X86CPU *pc_new_cpu(const char *cpu_model, int64_t apic_id,
> -                          DeviceState *icc_bridge, Error **errp)
> +                          Error **errp)
>  {
>      X86CPU *cpu = NULL;
>      Error *local_err = NULL;
>  
> -    if (icc_bridge == NULL) {
> -        error_setg(&local_err, "Invalid icc-bridge value");
> -        goto out;
> -    }
> -
>      cpu = cpu_x86_create(cpu_model, &local_err);
>      if (local_err != NULL) {
>          goto out;
>      }
>  
> -    qdev_set_parent_bus(DEVICE(cpu), qdev_get_child_bus(icc_bridge, "icc"));
> -
>      object_property_set_int(OBJECT(cpu), apic_id, "apic-id", &local_err);
>      object_property_set_bool(OBJECT(cpu), true, "realized", &local_err);
>  
> @@ -1085,7 +1077,6 @@ static const char *current_cpu_model;
>  
>  void pc_hot_add_cpu(const int64_t id, Error **errp)
>  {
> -    DeviceState *icc_bridge;
>      X86CPU *cpu;
>      int64_t apic_id = x86_cpu_apic_id_from_index(id);
>      Error *local_err = NULL;
> @@ -1114,9 +1105,7 @@ void pc_hot_add_cpu(const int64_t id, Error **errp)
>          return;
>      }
>  
> -    icc_bridge = DEVICE(object_resolve_path_type("icc-bridge",
> -                                                 TYPE_ICC_BRIDGE, NULL));
> -    cpu = pc_new_cpu(current_cpu_model, apic_id, icc_bridge, &local_err);
> +    cpu = pc_new_cpu(current_cpu_model, apic_id, &local_err);
>      if (local_err) {
>          error_propagate(errp, local_err);
>          return;
> @@ -1124,7 +1113,7 @@ void pc_hot_add_cpu(const int64_t id, Error **errp)
>      object_unref(OBJECT(cpu));
>  }
>  
> -void pc_cpus_init(const char *cpu_model, DeviceState *icc_bridge)
> +void pc_cpus_init(const char *cpu_model)
>  {
>      int i;
>      X86CPU *cpu = NULL;
> @@ -1150,7 +1139,7 @@ void pc_cpus_init(const char *cpu_model, DeviceState *icc_bridge)
>  
>      for (i = 0; i < smp_cpus; i++) {
>          cpu = pc_new_cpu(cpu_model, x86_cpu_apic_id_from_index(i),
> -                         icc_bridge, &error);
> +                         &error);
>          if (error) {
>              error_report_err(error);
>              exit(1);
> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> index 3f925b2..4335e9e 100644
> --- a/hw/i386/pc_piix.c
> +++ b/hw/i386/pc_piix.c
> @@ -39,7 +39,6 @@
>  #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/block-backend.h"
>  #include "hw/i2c/smbus.h"
> @@ -98,7 +97,6 @@ static void pc_init1(MachineState *machine,
>      MemoryRegion *ram_memory;
>      MemoryRegion *pci_memory;
>      MemoryRegion *rom_memory;
> -    DeviceState *icc_bridge;
>      PcGuestInfo *guest_info;
>      ram_addr_t lowmem;
>  
> @@ -141,11 +139,7 @@ static void pc_init1(MachineState *machine,
>          exit(1);
>      }
>  
> -    icc_bridge = qdev_create(NULL, TYPE_ICC_BRIDGE);
> -    object_property_add_child(qdev_get_machine(), "icc-bridge",
> -                              OBJECT(icc_bridge), NULL);
> -
> -    pc_cpus_init(machine->cpu_model, icc_bridge);
> +    pc_cpus_init(machine->cpu_model);
>  
>      if (kvm_enabled() && kvmclock_enabled) {
>          kvmclock_create();
> @@ -226,7 +220,6 @@ static void pc_init1(MachineState *machine,
>      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 11601ab..616ec68 100644
> --- a/hw/i386/pc_q35.c
> +++ b/hw/i386/pc_q35.c
> @@ -43,7 +43,6 @@
>  #include "hw/ide/pci.h"
>  #include "hw/ide/ahci.h"
>  #include "hw/usb.h"
> -#include "hw/cpu/icc_bus.h"
>  #include "qemu/error-report.h"
>  #include "migration/migration.h"
>  
> @@ -83,7 +82,6 @@ static void pc_q35_init(MachineState *machine)
>      int i;
>      ICH9LPCState *ich9_lpc;
>      PCIDevice *ahci;
> -    DeviceState *icc_bridge;
>      PcGuestInfo *guest_info;
>      ram_addr_t lowmem;
>      DriveInfo *hd[MAX_SATA_PORTS];
> @@ -130,11 +128,7 @@ static void pc_q35_init(MachineState *machine)
>          exit(1);
>      }
>  
> -    icc_bridge = qdev_create(NULL, TYPE_ICC_BRIDGE);
> -    object_property_add_child(qdev_get_machine(), "icc-bridge",
> -                              OBJECT(icc_bridge), NULL);
> -
> -    pc_cpus_init(machine->cpu_model, icc_bridge);
> +    pc_cpus_init(machine->cpu_model);
>      pc_acpi_init("q35-acpi-dsdt.aml");
>  
>      kvmclock_create();
> @@ -236,7 +230,6 @@ static void pc_q35_init(MachineState *machine)
>      if (pci_enabled) {
>          ioapic_init_gsi(gsi_state, "q35");
>      }
> -    qdev_init_nofail(icc_bridge);
>  
>      pc_register_ferr_irq(gsi[13]);
>  
> diff --git a/hw/intc/apic_common.c b/hw/intc/apic_common.c
> index c0b32eb..ad959c4 100644
> --- a/hw/intc/apic_common.c
> +++ b/hw/intc/apic_common.c
> @@ -419,13 +419,12 @@ static Property apic_properties_common[] = {
>  
>  static void apic_common_class_init(ObjectClass *klass, void *data)
>  {
> -    ICCDeviceClass *idc = ICC_DEVICE_CLASS(klass);
>      DeviceClass *dc = DEVICE_CLASS(klass);
>  
>      dc->vmsd = &vmstate_apic_common;
>      dc->reset = apic_reset_common;
>      dc->props = apic_properties_common;
> -    idc->realize = apic_common_realize;
> +    dc->realize = apic_common_realize;
>      /*
>       * Reason: APIC and CPU need to be wired up by
>       * x86_cpu_apic_create()
> @@ -435,7 +434,7 @@ static void apic_common_class_init(ObjectClass *klass, void *data)
>  
>  static const TypeInfo apic_common_type = {
>      .name = TYPE_APIC_COMMON,
> -    .parent = TYPE_ICC_DEVICE,
> +    .parent = TYPE_DEVICE,
>      .instance_size = sizeof(APICCommonState),
>      .class_size = sizeof(APICCommonClass),
>      .class_init = apic_common_class_init,
> diff --git a/include/hw/i386/apic_internal.h b/include/hw/i386/apic_internal.h
> index 26632ac..905ada7 100644
> --- a/include/hw/i386/apic_internal.h
> +++ b/include/hw/i386/apic_internal.h
> @@ -22,7 +22,6 @@
>  
>  #include "cpu.h"
>  #include "exec/memory.h"
> -#include "hw/cpu/icc_bus.h"
>  #include "qemu/timer.h"
>  
>  /* APIC Local Vector Table */
> @@ -79,7 +78,7 @@ typedef struct APICCommonState APICCommonState;
>  
>  typedef struct APICCommonClass
>  {
> -    ICCDeviceClass parent_class;
> +    DeviceClass parent_class;
>  
>      DeviceRealize realize;
>      void (*set_base)(APICCommonState *s, uint64_t val);
> @@ -94,7 +93,9 @@ typedef struct APICCommonClass
>  } APICCommonClass;
>  
>  struct APICCommonState {
> -    ICCDevice busdev;
> +    /*< private >*/
> +    DeviceState parent_obj;
> +    /*< public >*/
>  
>      MemoryRegion io_memory;
>      X86CPU *cpu;
> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> index 3e002c9..8a9c57a 100644
> --- a/include/hw/i386/pc.h
> +++ b/include/hw/i386/pc.h
> @@ -162,7 +162,7 @@ bool pc_machine_is_smm_enabled(PCMachineState *pcms);
>  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(const char *cpu_model);
>  void pc_hot_add_cpu(const int64_t id, Error **errp);
>  void pc_acpi_init(const char *default_dsdt);
>  
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index 692a284..8228fc7 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
> @@ -43,7 +43,6 @@
>  
>  #include "sysemu/sysemu.h"
>  #include "hw/qdev-properties.h"
> -#include "hw/cpu/icc_bus.h"
>  #ifndef CONFIG_USER_ONLY
>  #include "exec/address-spaces.h"
>  #include "hw/xen/xen.h"
> @@ -2723,7 +2722,6 @@ static void mce_init(X86CPU *cpu)
>  #ifndef CONFIG_USER_ONLY
>  static void x86_cpu_apic_create(X86CPU *cpu, Error **errp)
>  {
> -    DeviceState *dev = DEVICE(cpu);
>      APICCommonState *apic;
>      const char *apic_type = "apic";
>  
> @@ -2733,11 +2731,7 @@ static void x86_cpu_apic_create(X86CPU *cpu, Error **errp)
>          apic_type = "xen-apic";
>      }
>  
> -    cpu->apic_state = qdev_try_create(qdev_get_parent_bus(dev), apic_type);
> -    if (cpu->apic_state == NULL) {
> -        error_setg(errp, "APIC device '%s' could not be created", apic_type);
> -        return;
> -    }
> +    cpu->apic_state = DEVICE(object_new(apic_type));
>  
>      object_property_add_child(OBJECT(cpu), "apic",
>                                OBJECT(cpu->apic_state), NULL);
> @@ -3162,7 +3156,6 @@ 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;
>      dc->props = x86_cpu_properties;
>  
>      xcc->parent_reset = cc->reset;

Re: [Qemu-devel] [PATCH v12 5/5] icc_bus: drop the unused files

[Igor Mammedov]

On Wed, 16 Sep 2015 17:19:15 +0800
Zhu Guihua <zhugh.fnst@cn.fujitsu.com> wrote:

> ICC bus impl has been droped, so all icc related files are not useful
> any more; delete them.
> 
> Signed-off-by: Zhu Guihua <zhugh.fnst@cn.fujitsu.com>

Reviewed-by: Igor Mammedov <imammedo@redhat.com>

> ---
>  default-configs/i386-softmmu.mak   |   1 -
>  default-configs/x86_64-softmmu.mak |   1 -
>  hw/cpu/Makefile.objs               |   1 -
>  hw/cpu/icc_bus.c                   | 118 -------------------------------------
>  include/hw/cpu/icc_bus.h           |  82 --------------------------
>  5 files changed, 203 deletions(-)
>  delete mode 100644 hw/cpu/icc_bus.c
>  delete mode 100644 include/hw/cpu/icc_bus.h
> 
> diff --git a/default-configs/i386-softmmu.mak b/default-configs/i386-softmmu.mak
> index 9393cf0..43c96d1 100644
> --- a/default-configs/i386-softmmu.mak
> +++ b/default-configs/i386-softmmu.mak
> @@ -44,7 +44,6 @@ CONFIG_LPC_ICH9=y
>  CONFIG_PCI_Q35=y
>  CONFIG_APIC=y
>  CONFIG_IOAPIC=y
> -CONFIG_ICC_BUS=y
>  CONFIG_PVPANIC=y
>  CONFIG_MEM_HOTPLUG=y
>  CONFIG_XIO3130=y
> diff --git a/default-configs/x86_64-softmmu.mak b/default-configs/x86_64-softmmu.mak
> index 28e2099..dfb8095 100644
> --- a/default-configs/x86_64-softmmu.mak
> +++ b/default-configs/x86_64-softmmu.mak
> @@ -44,7 +44,6 @@ CONFIG_LPC_ICH9=y
>  CONFIG_PCI_Q35=y
>  CONFIG_APIC=y
>  CONFIG_IOAPIC=y
> -CONFIG_ICC_BUS=y
>  CONFIG_PVPANIC=y
>  CONFIG_MEM_HOTPLUG=y
>  CONFIG_XIO3130=y
> diff --git a/hw/cpu/Makefile.objs b/hw/cpu/Makefile.objs
> index 6381238..0954a18 100644
> --- a/hw/cpu/Makefile.objs
> +++ b/hw/cpu/Makefile.objs
> @@ -2,5 +2,4 @@ obj-$(CONFIG_ARM11MPCORE) += arm11mpcore.o
>  obj-$(CONFIG_REALVIEW) += realview_mpcore.o
>  obj-$(CONFIG_A9MPCORE) += a9mpcore.o
>  obj-$(CONFIG_A15MPCORE) += a15mpcore.o
> -obj-$(CONFIG_ICC_BUS) += icc_bus.o
>  
> diff --git a/hw/cpu/icc_bus.c b/hw/cpu/icc_bus.c
> deleted file mode 100644
> index 6646ea2..0000000
> --- a/hw/cpu/icc_bus.c
> +++ /dev/null
> @@ -1,118 +0,0 @@
> -/* 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 const TypeInfo icc_bus_info = {
> -    .name = TYPE_ICC_BUS,
> -    .parent = TYPE_BUS,
> -    .instance_size = sizeof(ICCBus),
> -};
> -
> -
> -/* icc-device implementation */
> -
> -static void icc_device_realize(DeviceState *dev, Error **errp)
> -{
> -    ICCDeviceClass *idc = ICC_DEVICE_GET_CLASS(dev);
> -
> -    /* convert to QOM */
> -    if (idc->realize) {
> -        idc->realize(dev, errp);
> -    }
> -
> -}
> -
> -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;
> -    MemoryRegion apic_container;
> -} ICCBridgeState;
> -
> -#define ICC_BRIDGE(obj) OBJECT_CHECK(ICCBridgeState, (obj), TYPE_ICC_BRIDGE)
> -
> -static void icc_bridge_init(Object *obj)
> -{
> -    ICCBridgeState *s = ICC_BRIDGE(obj);
> -    SysBusDevice *sb = SYS_BUS_DEVICE(obj);
> -
> -    qbus_create_inplace(&s->icc_bus, sizeof(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, obj, "icc-apic-container",
> -                       APIC_SPACE_SIZE);
> -    sysbus_init_mmio(sb, &s->apic_container);
> -    s->icc_bus.apic_address_space = &s->apic_container;
> -}
> -
> -static void icc_bridge_class_init(ObjectClass *oc, void *data)
> -{
> -    DeviceClass *dc = DEVICE_CLASS(oc);
> -
> -    set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories);
> -}
> -
> -static const TypeInfo icc_bridge_info = {
> -    .name  = TYPE_ICC_BRIDGE,
> -    .parent = TYPE_SYS_BUS_DEVICE,
> -    .instance_init  = icc_bridge_init,
> -    .instance_size  = sizeof(ICCBridgeState),
> -    .class_init = icc_bridge_class_init,
> -};
> -
> -
> -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/include/hw/cpu/icc_bus.h b/include/hw/cpu/icc_bus.h
> deleted file mode 100644
> index 98a979f..0000000
> --- a/include/hw/cpu/icc_bus.h
> +++ /dev/null
> @@ -1,82 +0,0 @@
> -/* 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 "exec/memory.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 >*/
> -
> -    MemoryRegion *apic_address_space;
> -} 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 >*/
> -
> -    DeviceRealize 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

Re: [Qemu-devel] [PATCH v12 0/5] remove icc bus/bridge

[Zhu Guihua]

Hi Eduardo,

Can you help merge this patch series to your x86 tree?

Thanks,
Zhu

On 09/16/2015 05:19 PM, Zhu Guihua wrote:
> ICC Bus was used for providing a hotpluggable bus for APIC and CPU, but now we
> use HotplugHandler to make hotplug. So ICC Bus is unnecessary.
>
> This code has passed the new pc-cpu-test.
> And I have tested with kvm along with kernel_irqchip=on/off, it works fine.
>
> This patch series is based on the latest master.
>
> v12:
>   -move APIC MMIO mapping into x86_cpu_apic_realize()
>   -change commit message in PATCH 4
>
> v11:
>   -improve commit messages
>   -split per CPU AS change into a separate patch
>
> v10:
>   -improve commit messages in patch 1 and 2
>   -make the check of cpu->cpu_as_root simplier
>
> v9:
>   -use a callback to correct reset sequence for x86
>   -update apic mmio mapping
>
> Chen Fan (2):
>    apic: move APIC's MMIO region mapping into APIC
>    cpu/apic: drop icc bus/bridge
>
> Zhu Guihua (3):
>    apic: use per CPU AS to map APIC MMIO for TCG
>    x86: use new method to correct reset sequence
>    icc_bus: drop the unused files
>
>   default-configs/i386-softmmu.mak   |   1 -
>   default-configs/x86_64-softmmu.mak |   1 -
>   hw/cpu/Makefile.objs               |   1 -
>   hw/cpu/icc_bus.c                   | 118 -------------------------------------
>   hw/i386/pc.c                       |  46 ++++++++-------
>   hw/i386/pc_piix.c                  |   9 +--
>   hw/i386/pc_q35.c                   |   9 +--
>   hw/intc/apic_common.c              |  11 +---
>   include/hw/cpu/icc_bus.h           |  82 --------------------------
>   include/hw/i386/apic_internal.h    |   7 ++-
>   include/hw/i386/pc.h               |   2 +-
>   target-i386/cpu.c                  |  33 ++++++++---
>   12 files changed, 58 insertions(+), 262 deletions(-)
>   delete mode 100644 hw/cpu/icc_bus.c
>   delete mode 100644 include/hw/cpu/icc_bus.h
>

Re: [Qemu-devel] [PATCH v12 0/5] remove icc bus/bridge

[Eduardo Habkost]

On Wed, Sep 30, 2015 at 02:35:28PM +0800, Zhu Guihua wrote:
> Hi Eduardo,
> 
> Can you help merge this patch series to your x86 tree?

Sorry for taking so long. Series applied to x86 tree. Thanks!

-- 
Eduardo

Re: [Qemu-devel] [PATCH v12 2/5] apic: use per CPU AS to map APIC MMIO for TCG

[Eduardo Habkost]

On Tue, Sep 22, 2015 at 02:29:01PM +0200, Igor Mammedov wrote:
> On Wed, 16 Sep 2015 17:19:12 +0800
> Zhu Guihua <zhugh.fnst@cn.fujitsu.com> wrote:
> 
> > TCG supports per CPU address space, and the emulation quality is
> > a bit better with it.
> > So use per CPU address space to map APIC MMIO area. This allows the
> > APIC base address of each cpu to be moved independent of others.
> > 
> > Signed-off-by: Zhu Guihua <zhugh.fnst@cn.fujitsu.com>
> 
> Reviewed-by: Igor Mammedov <imammedo@redhat.com>

The series seems to be OK when using KVM, but something is wrong with TCG.

After applying this patch, my Fedora 20 image hangs on boot at:

[  OK  ] Started udev Coldplug all Devices.
         Starting dracut initqueue hook...
[  OK  ] Reached target System Initialization.
[  OK  ] Reached target Basic System.
[    8.988420] ACPI: PCI Interrupt Link [LNKD] enabled at IRQ 11


Before this patch, boot continues with:

[  OK  ] Reached target Basic System.
[    8.523293] ACPI: PCI Interrupt Link [LNKD] enabled at IRQ 11
[    9.230955]  vda: vda1
[  OK  ] Started dracut initqueue hook.
         Mounting /sysroot...

I am running QEMU as:
  $ ./x86_64-softmmu/qemu-system-x86_64 -drive if=virtio,file=/home/ehabkost/system/vmachines/Fedora-x86_64-20-20131211.1-sda.qcow2,format=qcow2 -machine pc,accel=tcg  -smp 8,sockets=2,threads=2,cores=2,maxcpus=16 -serial stdio -nographic -monitor none

-- 
Eduardo

Re: [Qemu-devel] [PATCH v12 2/5] apic: use per CPU AS to map APIC MMIO for TCG

[Eduardo Habkost]

On Fri, Oct 02, 2015 at 03:24:24PM -0300, Eduardo Habkost wrote:
> On Tue, Sep 22, 2015 at 02:29:01PM +0200, Igor Mammedov wrote:
> > On Wed, 16 Sep 2015 17:19:12 +0800
> > Zhu Guihua <zhugh.fnst@cn.fujitsu.com> wrote:
> > 
> > > TCG supports per CPU address space, and the emulation quality is
> > > a bit better with it.
> > > So use per CPU address space to map APIC MMIO area. This allows the
> > > APIC base address of each cpu to be moved independent of others.
> > > 
> > > Signed-off-by: Zhu Guihua <zhugh.fnst@cn.fujitsu.com>
> > 
> > Reviewed-by: Igor Mammedov <imammedo@redhat.com>
> 
> The series seems to be OK when using KVM, but something is wrong with TCG.
> 
> After applying this patch, my Fedora 20 image hangs on boot at:
> 
> [  OK  ] Started udev Coldplug all Devices.
>          Starting dracut initqueue hook...
> [  OK  ] Reached target System Initialization.
> [  OK  ] Reached target Basic System.
> [    8.988420] ACPI: PCI Interrupt Link [LNKD] enabled at IRQ 11
> 
> 
> Before this patch, boot continues with:
> 
> [  OK  ] Reached target Basic System.
> [    8.523293] ACPI: PCI Interrupt Link [LNKD] enabled at IRQ 11
> [    9.230955]  vda: vda1
> [  OK  ] Started dracut initqueue hook.
>          Mounting /sysroot...
> 
> I am running QEMU as:
>   $ ./x86_64-softmmu/qemu-system-x86_64 -drive if=virtio,file=/home/ehabkost/system/vmachines/Fedora-x86_64-20-20131211.1-sda.qcow2,format=qcow2 -machine pc,accel=tcg  -smp 8,sockets=2,threads=2,cores=2,maxcpus=16 -serial stdio -nographic -monitor none

Debugging revealed that this broke MSI delivery of virtio notifications.
Writes to PCIDevice::bus_master_as (at msi_send_message()) don't get
delivered to the APIC memory region anymore.

This patch doesn't seem to be required for the removal of ICC bus, so I
am applying the rest of the series anyway. If you see any problem with
that, please let me know.

-- 
Eduardo

Re: [Qemu-devel] [PATCH v12 2/5] apic: use per CPU AS to map APIC MMIO for TCG

[Zhu Guihua]

On 10/03/2015 03:21 AM, Eduardo Habkost wrote:
> On Fri, Oct 02, 2015 at 03:24:24PM -0300, Eduardo Habkost wrote:
>> On Tue, Sep 22, 2015 at 02:29:01PM +0200, Igor Mammedov wrote:
>>> On Wed, 16 Sep 2015 17:19:12 +0800
>>> Zhu Guihua <zhugh.fnst@cn.fujitsu.com> wrote:
>>>
>>>> TCG supports per CPU address space, and the emulation quality is
>>>> a bit better with it.
>>>> So use per CPU address space to map APIC MMIO area. This allows the
>>>> APIC base address of each cpu to be moved independent of others.
>>>>
>>>> Signed-off-by: Zhu Guihua <zhugh.fnst@cn.fujitsu.com>
>>> Reviewed-by: Igor Mammedov <imammedo@redhat.com>
>> The series seems to be OK when using KVM, but something is wrong with TCG.
>>
>> After applying this patch, my Fedora 20 image hangs on boot at:
>>
>> [  OK  ] Started udev Coldplug all Devices.
>>           Starting dracut initqueue hook...
>> [  OK  ] Reached target System Initialization.
>> [  OK  ] Reached target Basic System.
>> [    8.988420] ACPI: PCI Interrupt Link [LNKD] enabled at IRQ 11
>>
>>
>> Before this patch, boot continues with:
>>
>> [  OK  ] Reached target Basic System.
>> [    8.523293] ACPI: PCI Interrupt Link [LNKD] enabled at IRQ 11
>> [    9.230955]  vda: vda1
>> [  OK  ] Started dracut initqueue hook.
>>           Mounting /sysroot...
>>
>> I am running QEMU as:
>>    $ ./x86_64-softmmu/qemu-system-x86_64 -drive if=virtio,file=/home/ehabkost/system/vmachines/Fedora-x86_64-20-20131211.1-sda.qcow2,format=qcow2 -machine pc,accel=tcg  -smp 8,sockets=2,threads=2,cores=2,maxcpus=16 -serial stdio -nographic -monitor none
> Debugging revealed that this broke MSI delivery of virtio notifications.
> Writes to PCIDevice::bus_master_as (at msi_send_message()) don't get
> delivered to the APIC memory region anymore.
>
> This patch doesn't seem to be required for the removal of ICC bus, so I
> am applying the rest of the series anyway. If you see any problem with
> that, please let me know.
>
Sorry for late reply.   I'll look at this.

Thanks,
Zhu