All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH v11 0/5] remove icc bus/bridge
@ 2015-09-02  9:36 Zhu Guihua
  2015-09-02  9:36 ` [Qemu-devel] [PATCH v11 1/5] apic: move APIC's MMIO region mapping into APIC Zhu Guihua
                   ` (5 more replies)
  0 siblings, 6 replies; 11+ messages in thread
From: Zhu Guihua @ 2015-09-02  9:36 UTC (permalink / raw)
  To: qemu-devel, imammedo, afaerber, pbonzini, ehabkost
  Cc: chen.fan.fnst, izumi.taku, 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 and
'[PATCH v3] i386: keep cpu_model field in MachineState uptodate'
https://lists.nongnu.org/archive/html/qemu-devel/2015-08/msg03375.html
which has been reviewed but not been merged.

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

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

* [Qemu-devel] [PATCH v11 1/5] apic: move APIC's MMIO region mapping into APIC
  2015-09-02  9:36 [Qemu-devel] [PATCH v11 0/5] remove icc bus/bridge Zhu Guihua
@ 2015-09-02  9:36 ` Zhu Guihua
  2015-09-14 13:16   ` Igor Mammedov
  2015-09-02  9:36 ` [Qemu-devel] [PATCH v11 2/5] apic: use per CPU AS to map APIC MMIO for TCG Zhu Guihua
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 11+ messages in thread
From: Zhu Guihua @ 2015-09-02  9:36 UTC (permalink / raw)
  To: qemu-devel, imammedo, afaerber, pbonzini, ehabkost
  Cc: chen.fan.fnst, izumi.taku, 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 b1c96a8..e15971c 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1157,13 +1157,6 @@ void pc_cpus_init(PCMachineState *pcms, 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..66b6b0d 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -2745,6 +2745,7 @@ 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)
@@ -2789,8 +2790,10 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp)
     X86CPU *cpu = X86_CPU(dev);
     X86CPUClass *xcc = X86_CPU_GET_CLASS(dev);
     CPUX86State *env = &cpu->env;
+    APICCommonState *apic;
     Error *local_err = NULL;
     static bool ht_warned;
+    static bool apic_mmio_map_once;
 
     if (cpu->apic_id < 0) {
         error_setg(errp, "apic-id property was not initialized properly");
@@ -2877,6 +2880,18 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp)
     if (local_err != NULL) {
         goto out;
     }
+
+    /* 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;
+    }
+
     cpu_reset(cs);
 
     xcc->parent_realize(dev, &local_err);
-- 
1.9.3

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

* [Qemu-devel] [PATCH v11 2/5] apic: use per CPU AS to map APIC MMIO for TCG
  2015-09-02  9:36 [Qemu-devel] [PATCH v11 0/5] remove icc bus/bridge Zhu Guihua
  2015-09-02  9:36 ` [Qemu-devel] [PATCH v11 1/5] apic: move APIC's MMIO region mapping into APIC Zhu Guihua
@ 2015-09-02  9:36 ` Zhu Guihua
  2015-09-02  9:36 ` [Qemu-devel] [PATCH v11 3/5] x86: use new method to correct reset sequence Zhu Guihua
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 11+ messages in thread
From: Zhu Guihua @ 2015-09-02  9:36 UTC (permalink / raw)
  To: qemu-devel, imammedo, afaerber, pbonzini, ehabkost
  Cc: chen.fan.fnst, izumi.taku, 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 indepenedent 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 66b6b0d..0a95162 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -2881,9 +2881,18 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp)
         goto out;
     }
 
-    /* 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

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

* [Qemu-devel] [PATCH v11 3/5] x86: use new method to correct reset sequence
  2015-09-02  9:36 [Qemu-devel] [PATCH v11 0/5] remove icc bus/bridge Zhu Guihua
  2015-09-02  9:36 ` [Qemu-devel] [PATCH v11 1/5] apic: move APIC's MMIO region mapping into APIC Zhu Guihua
  2015-09-02  9:36 ` [Qemu-devel] [PATCH v11 2/5] apic: use per CPU AS to map APIC MMIO for TCG Zhu Guihua
@ 2015-09-02  9:36 ` Zhu Guihua
  2015-09-02  9:36 ` [Qemu-devel] [PATCH v11 4/5] cpu/apic: drop icc bus/bridge Zhu Guihua
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 11+ messages in thread
From: Zhu Guihua @ 2015-09-02  9:36 UTC (permalink / raw)
  To: qemu-devel, imammedo, afaerber, pbonzini, ehabkost
  Cc: chen.fan.fnst, izumi.taku, 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 e15971c..4b4a7f3 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1928,6 +1928,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;
@@ -1948,6 +1967,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

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

* [Qemu-devel] [PATCH v11 4/5] cpu/apic: drop icc bus/bridge
  2015-09-02  9:36 [Qemu-devel] [PATCH v11 0/5] remove icc bus/bridge Zhu Guihua
                   ` (2 preceding siblings ...)
  2015-09-02  9:36 ` [Qemu-devel] [PATCH v11 3/5] x86: use new method to correct reset sequence Zhu Guihua
@ 2015-09-02  9:36 ` Zhu Guihua
  2015-09-14 13:18   ` Igor Mammedov
  2015-09-02  9:36 ` [Qemu-devel] [PATCH v11 5/5] icc_bus: drop the unused files Zhu Guihua
  2015-09-10 10:21 ` [Qemu-devel] [PATCH v11 0/5] remove icc bus/bridge Zhu Guihua
  5 siblings, 1 reply; 11+ messages in thread
From: Zhu Guihua @ 2015-09-02  9:36 UTC (permalink / raw)
  To: qemu-devel, imammedo, afaerber, pbonzini, ehabkost
  Cc: chen.fan.fnst, izumi.taku, 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 ~200LOC 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 4b4a7f3..4c1d68a 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);
 
@@ -1083,7 +1075,6 @@ out:
 
 void pc_hot_add_cpu(const int64_t id, Error **errp)
 {
-    DeviceState *icc_bridge;
     MachineState *machine = MACHINE(qdev_get_machine());
     X86CPU *cpu;
     int64_t apic_id = x86_cpu_apic_id_from_index(id);
@@ -1113,9 +1104,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(machine->cpu_model, apic_id, icc_bridge, &local_err);
+    cpu = pc_new_cpu(machine->cpu_model, apic_id, &local_err);
     if (local_err) {
         error_propagate(errp, local_err);
         return;
@@ -1123,7 +1112,7 @@ void pc_hot_add_cpu(const int64_t id, Error **errp)
     object_unref(OBJECT(cpu));
 }
 
-void pc_cpus_init(PCMachineState *pcms, DeviceState *icc_bridge)
+void pc_cpus_init(PCMachineState *pcms)
 {
     int i;
     X86CPU *cpu = NULL;
@@ -1149,7 +1138,7 @@ void pc_cpus_init(PCMachineState *pcms, DeviceState *icc_bridge)
 
     for (i = 0; i < smp_cpus; i++) {
         cpu = pc_new_cpu(machine->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 fd6130d..3a97826 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"
@@ -96,7 +95,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(pcms, icc_bridge);
+    pc_cpus_init(pcms);
 
     if (kvm_enabled() && kvmclock_enabled) {
         kvmclock_create();
@@ -223,7 +217,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 4f76535..414cbbb 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];
@@ -132,11 +130,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(pcms, icc_bridge);
+    pc_cpus_init(pcms);
     pc_acpi_init("q35-acpi-dsdt.aml");
 
     kvmclock_create();
@@ -237,7 +231,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 3732ab5..8eef558 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -161,7 +161,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(PCMachineState *pcms, DeviceState *icc_bridge);
+void pc_cpus_init(PCMachineState *pcms);
 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 0a95162..6686039 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

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

* [Qemu-devel] [PATCH v11 5/5] icc_bus: drop the unused files
  2015-09-02  9:36 [Qemu-devel] [PATCH v11 0/5] remove icc bus/bridge Zhu Guihua
                   ` (3 preceding siblings ...)
  2015-09-02  9:36 ` [Qemu-devel] [PATCH v11 4/5] cpu/apic: drop icc bus/bridge Zhu Guihua
@ 2015-09-02  9:36 ` Zhu Guihua
  2015-09-10 10:21 ` [Qemu-devel] [PATCH v11 0/5] remove icc bus/bridge Zhu Guihua
  5 siblings, 0 replies; 11+ messages in thread
From: Zhu Guihua @ 2015-09-02  9:36 UTC (permalink / raw)
  To: qemu-devel, imammedo, afaerber, pbonzini, ehabkost
  Cc: chen.fan.fnst, izumi.taku, 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 5eaafa1..70391ed 100644
--- a/default-configs/i386-softmmu.mak
+++ b/default-configs/i386-softmmu.mak
@@ -43,7 +43,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

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

* Re: [Qemu-devel] [PATCH v11 0/5] remove icc bus/bridge
  2015-09-02  9:36 [Qemu-devel] [PATCH v11 0/5] remove icc bus/bridge Zhu Guihua
                   ` (4 preceding siblings ...)
  2015-09-02  9:36 ` [Qemu-devel] [PATCH v11 5/5] icc_bus: drop the unused files Zhu Guihua
@ 2015-09-10 10:21 ` Zhu Guihua
  5 siblings, 0 replies; 11+ messages in thread
From: Zhu Guihua @ 2015-09-10 10:21 UTC (permalink / raw)
  To: qemu-devel, imammedo, afaerber, pbonzini, ehabkost
  Cc: chen.fan.fnst, izumi.taku

ping ...

On 09/02/2015 05:36 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 and
> '[PATCH v3] i386: keep cpu_model field in MachineState uptodate'
> https://lists.nongnu.org/archive/html/qemu-devel/2015-08/msg03375.html
> which has been reviewed but not been merged.
>
> 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
>

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

* Re: [Qemu-devel] [PATCH v11 1/5] apic: move APIC's MMIO region mapping into APIC
  2015-09-02  9:36 ` [Qemu-devel] [PATCH v11 1/5] apic: move APIC's MMIO region mapping into APIC Zhu Guihua
@ 2015-09-14 13:16   ` Igor Mammedov
  0 siblings, 0 replies; 11+ messages in thread
From: Igor Mammedov @ 2015-09-14 13:16 UTC (permalink / raw)
  To: Zhu Guihua
  Cc: ehabkost, qemu-devel, izumi.taku, chen.fan.fnst, pbonzini,
	afaerber

On Wed, 2 Sep 2015 17:36:18 +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>
> ---
>  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 b1c96a8..e15971c 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -1157,13 +1157,6 @@ void pc_cpus_init(PCMachineState *pcms, 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..66b6b0d 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
> @@ -2745,6 +2745,7 @@ 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)
> @@ -2789,8 +2790,10 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp)
>      X86CPU *cpu = X86_CPU(dev);
>      X86CPUClass *xcc = X86_CPU_GET_CLASS(dev);
>      CPUX86State *env = &cpu->env;

> +    APICCommonState *apic;
this and below hunks break build for *-linux-user targets,
suggest to move it to x86_cpu_apic_realize() which is empty for
user-only and softmmu builds.

>      Error *local_err = NULL;
>      static bool ht_warned;
> +    static bool apic_mmio_map_once;
>  
>      if (cpu->apic_id < 0) {
>          error_setg(errp, "apic-id property was not initialized properly");
> @@ -2877,6 +2880,18 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp)
>      if (local_err != NULL) {
>          goto out;
>      }
> +
> +    /* 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;
> +    }
> +
>      cpu_reset(cs);
>  
>      xcc->parent_realize(dev, &local_err);

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

* Re: [Qemu-devel] [PATCH v11 4/5] cpu/apic: drop icc bus/bridge
  2015-09-02  9:36 ` [Qemu-devel] [PATCH v11 4/5] cpu/apic: drop icc bus/bridge Zhu Guihua
@ 2015-09-14 13:18   ` Igor Mammedov
  2015-09-15  0:52     ` Zhu Guihua
  0 siblings, 1 reply; 11+ messages in thread
From: Igor Mammedov @ 2015-09-14 13:18 UTC (permalink / raw)
  To: Zhu Guihua
  Cc: ehabkost, qemu-devel, izumi.taku, chen.fan.fnst, pbonzini,
	afaerber

On Wed, 2 Sep 2015 17:36:21 +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 ~200LOC of not needed anymore ICCBus related code.
this patch drops about 30LOCs only ...


PS:
Doesn't apply to current master, please rebase.

> 
> 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 4b4a7f3..4c1d68a 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);
>  
> @@ -1083,7 +1075,6 @@ out:
>  
>  void pc_hot_add_cpu(const int64_t id, Error **errp)
>  {
> -    DeviceState *icc_bridge;
>      MachineState *machine = MACHINE(qdev_get_machine());
>      X86CPU *cpu;
>      int64_t apic_id = x86_cpu_apic_id_from_index(id);
> @@ -1113,9 +1104,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(machine->cpu_model, apic_id, icc_bridge, &local_err);
> +    cpu = pc_new_cpu(machine->cpu_model, apic_id, &local_err);
>      if (local_err) {
>          error_propagate(errp, local_err);
>          return;
> @@ -1123,7 +1112,7 @@ void pc_hot_add_cpu(const int64_t id, Error **errp)
>      object_unref(OBJECT(cpu));
>  }
>  
> -void pc_cpus_init(PCMachineState *pcms, DeviceState *icc_bridge)
> +void pc_cpus_init(PCMachineState *pcms)
>  {
>      int i;
>      X86CPU *cpu = NULL;
> @@ -1149,7 +1138,7 @@ void pc_cpus_init(PCMachineState *pcms, DeviceState *icc_bridge)
>  
>      for (i = 0; i < smp_cpus; i++) {
>          cpu = pc_new_cpu(machine->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 fd6130d..3a97826 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"
> @@ -96,7 +95,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(pcms, icc_bridge);
> +    pc_cpus_init(pcms);
>  
>      if (kvm_enabled() && kvmclock_enabled) {
>          kvmclock_create();
> @@ -223,7 +217,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 4f76535..414cbbb 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];
> @@ -132,11 +130,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(pcms, icc_bridge);
> +    pc_cpus_init(pcms);
>      pc_acpi_init("q35-acpi-dsdt.aml");
>  
>      kvmclock_create();
> @@ -237,7 +231,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 3732ab5..8eef558 100644
> --- a/include/hw/i386/pc.h
> +++ b/include/hw/i386/pc.h
> @@ -161,7 +161,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(PCMachineState *pcms, DeviceState *icc_bridge);
> +void pc_cpus_init(PCMachineState *pcms);
>  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 0a95162..6686039 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;

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

* Re: [Qemu-devel] [PATCH v11 4/5] cpu/apic: drop icc bus/bridge
  2015-09-14 13:18   ` Igor Mammedov
@ 2015-09-15  0:52     ` Zhu Guihua
  2015-09-15  8:15       ` Igor Mammedov
  0 siblings, 1 reply; 11+ messages in thread
From: Zhu Guihua @ 2015-09-15  0:52 UTC (permalink / raw)
  To: Igor Mammedov
  Cc: ehabkost, qemu-devel, izumi.taku, chen.fan.fnst, pbonzini,
	afaerber


On 09/14/2015 09:18 PM, Igor Mammedov wrote:
> On Wed, 2 Sep 2015 17:36:21 +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 ~200LOC of not needed anymore ICCBus related code.
> this patch drops about 30LOCs only ...
>
>
> PS:
> Doesn't apply to current master, please rebase.

This patch rebased on my another patch
'[PATCH v3] i386: keep cpu_model field in MachineState uptodate'
https://lists.nongnu.org/archive/html/qemu-devel/2015-08/msg03375.html

The patch has been reviewed by Eduardo, but not been merged.

Should I only rebase on current master?

Thanks,
Zhu

>> 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 4b4a7f3..4c1d68a 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);
>>   
>> @@ -1083,7 +1075,6 @@ out:
>>   
>>   void pc_hot_add_cpu(const int64_t id, Error **errp)
>>   {
>> -    DeviceState *icc_bridge;
>>       MachineState *machine = MACHINE(qdev_get_machine());
>>       X86CPU *cpu;
>>       int64_t apic_id = x86_cpu_apic_id_from_index(id);
>> @@ -1113,9 +1104,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(machine->cpu_model, apic_id, icc_bridge, &local_err);
>> +    cpu = pc_new_cpu(machine->cpu_model, apic_id, &local_err);
>>       if (local_err) {
>>           error_propagate(errp, local_err);
>>           return;
>> @@ -1123,7 +1112,7 @@ void pc_hot_add_cpu(const int64_t id, Error **errp)
>>       object_unref(OBJECT(cpu));
>>   }
>>   
>> -void pc_cpus_init(PCMachineState *pcms, DeviceState *icc_bridge)
>> +void pc_cpus_init(PCMachineState *pcms)
>>   {
>>       int i;
>>       X86CPU *cpu = NULL;
>> @@ -1149,7 +1138,7 @@ void pc_cpus_init(PCMachineState *pcms, DeviceState *icc_bridge)
>>   
>>       for (i = 0; i < smp_cpus; i++) {
>>           cpu = pc_new_cpu(machine->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 fd6130d..3a97826 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"
>> @@ -96,7 +95,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(pcms, icc_bridge);
>> +    pc_cpus_init(pcms);
>>   
>>       if (kvm_enabled() && kvmclock_enabled) {
>>           kvmclock_create();
>> @@ -223,7 +217,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 4f76535..414cbbb 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];
>> @@ -132,11 +130,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(pcms, icc_bridge);
>> +    pc_cpus_init(pcms);
>>       pc_acpi_init("q35-acpi-dsdt.aml");
>>   
>>       kvmclock_create();
>> @@ -237,7 +231,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 3732ab5..8eef558 100644
>> --- a/include/hw/i386/pc.h
>> +++ b/include/hw/i386/pc.h
>> @@ -161,7 +161,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(PCMachineState *pcms, DeviceState *icc_bridge);
>> +void pc_cpus_init(PCMachineState *pcms);
>>   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 0a95162..6686039 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;
> .
>

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

* Re: [Qemu-devel] [PATCH v11 4/5] cpu/apic: drop icc bus/bridge
  2015-09-15  0:52     ` Zhu Guihua
@ 2015-09-15  8:15       ` Igor Mammedov
  0 siblings, 0 replies; 11+ messages in thread
From: Igor Mammedov @ 2015-09-15  8:15 UTC (permalink / raw)
  To: Zhu Guihua
  Cc: ehabkost, qemu-devel, pbonzini, chen.fan.fnst, izumi.taku,
	afaerber

On Tue, 15 Sep 2015 08:52:34 +0800
Zhu Guihua <zhugh.fnst@cn.fujitsu.com> wrote:

> 
> On 09/14/2015 09:18 PM, Igor Mammedov wrote:
> > On Wed, 2 Sep 2015 17:36:21 +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 ~200LOC of not needed anymore ICCBus related code.
> > this patch drops about 30LOCs only ...
> >
> >
> > PS:
> > Doesn't apply to current master, please rebase.
> 
> This patch rebased on my another patch
> '[PATCH v3] i386: keep cpu_model field in MachineState uptodate'
> https://lists.nongnu.org/archive/html/qemu-devel/2015-08/msg03375.html
> 
> The patch has been reviewed by Eduardo, but not been merged.
> 
> Should I only rebase on current master?
you could pull dependency in this series, it's only one patch.

> 
> Thanks,
> Zhu
> 
> >> 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 4b4a7f3..4c1d68a 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);
> >>   
> >> @@ -1083,7 +1075,6 @@ out:
> >>   
> >>   void pc_hot_add_cpu(const int64_t id, Error **errp)
> >>   {
> >> -    DeviceState *icc_bridge;
> >>       MachineState *machine = MACHINE(qdev_get_machine());
> >>       X86CPU *cpu;
> >>       int64_t apic_id = x86_cpu_apic_id_from_index(id);
> >> @@ -1113,9 +1104,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(machine->cpu_model, apic_id, icc_bridge, &local_err);
> >> +    cpu = pc_new_cpu(machine->cpu_model, apic_id, &local_err);
> >>       if (local_err) {
> >>           error_propagate(errp, local_err);
> >>           return;
> >> @@ -1123,7 +1112,7 @@ void pc_hot_add_cpu(const int64_t id, Error **errp)
> >>       object_unref(OBJECT(cpu));
> >>   }
> >>   
> >> -void pc_cpus_init(PCMachineState *pcms, DeviceState *icc_bridge)
> >> +void pc_cpus_init(PCMachineState *pcms)
> >>   {
> >>       int i;
> >>       X86CPU *cpu = NULL;
> >> @@ -1149,7 +1138,7 @@ void pc_cpus_init(PCMachineState *pcms, DeviceState *icc_bridge)
> >>   
> >>       for (i = 0; i < smp_cpus; i++) {
> >>           cpu = pc_new_cpu(machine->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 fd6130d..3a97826 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"
> >> @@ -96,7 +95,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(pcms, icc_bridge);
> >> +    pc_cpus_init(pcms);
> >>   
> >>       if (kvm_enabled() && kvmclock_enabled) {
> >>           kvmclock_create();
> >> @@ -223,7 +217,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 4f76535..414cbbb 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];
> >> @@ -132,11 +130,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(pcms, icc_bridge);
> >> +    pc_cpus_init(pcms);
> >>       pc_acpi_init("q35-acpi-dsdt.aml");
> >>   
> >>       kvmclock_create();
> >> @@ -237,7 +231,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 3732ab5..8eef558 100644
> >> --- a/include/hw/i386/pc.h
> >> +++ b/include/hw/i386/pc.h
> >> @@ -161,7 +161,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(PCMachineState *pcms, DeviceState *icc_bridge);
> >> +void pc_cpus_init(PCMachineState *pcms);
> >>   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 0a95162..6686039 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;
> > .
> >
> 
> 

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

end of thread, other threads:[~2015-09-15  8:15 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-09-02  9:36 [Qemu-devel] [PATCH v11 0/5] remove icc bus/bridge Zhu Guihua
2015-09-02  9:36 ` [Qemu-devel] [PATCH v11 1/5] apic: move APIC's MMIO region mapping into APIC Zhu Guihua
2015-09-14 13:16   ` Igor Mammedov
2015-09-02  9:36 ` [Qemu-devel] [PATCH v11 2/5] apic: use per CPU AS to map APIC MMIO for TCG Zhu Guihua
2015-09-02  9:36 ` [Qemu-devel] [PATCH v11 3/5] x86: use new method to correct reset sequence Zhu Guihua
2015-09-02  9:36 ` [Qemu-devel] [PATCH v11 4/5] cpu/apic: drop icc bus/bridge Zhu Guihua
2015-09-14 13:18   ` Igor Mammedov
2015-09-15  0:52     ` Zhu Guihua
2015-09-15  8:15       ` Igor Mammedov
2015-09-02  9:36 ` [Qemu-devel] [PATCH v11 5/5] icc_bus: drop the unused files Zhu Guihua
2015-09-10 10:21 ` [Qemu-devel] [PATCH v11 0/5] remove icc bus/bridge Zhu Guihua

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