[Qemu-devel] [PATCH v4 01/10] cpu/apic: drop icc bus/bridge/

[Zhu Guihua]

From: Chen Fan <address@hidden>

ICC bus was invented only to provide hotplug capability to
CPU and APIC because at the time being hotplug was available only for
BUS attached devices.

Now this patch is to drop ICC bus impl, and switch to bus-less
CPU+APIC hotplug, handling them in the same manner as pc-dimm.

Signed-off-by: Chen Fan <address@hidden>
Signed-off-by: Zhu Guihua <address@hidden>
---
 hw/i386/kvm/apic.c              | 10 ++++++++--
 hw/i386/pc.c                    | 21 +++++----------------
 hw/i386/pc_piix.c               |  9 +--------
 hw/i386/pc_q35.c                |  9 +--------
 hw/intc/apic.c                  | 16 +++++++++++-----
 hw/intc/apic_common.c           | 14 +++++++++-----
 include/hw/i386/apic_internal.h |  6 ++----
 include/hw/i386/pc.h            |  3 ++-
 target-i386/cpu.c               | 19 +++----------------
 target-i386/cpu.h               |  3 +--
 10 files changed, 43 insertions(+), 67 deletions(-)

diff --git a/hw/i386/kvm/apic.c b/hw/i386/kvm/apic.c
index 5b47056..4b7ce57 100644
--- a/hw/i386/kvm/apic.c
+++ b/hw/i386/kvm/apic.c
@@ -10,6 +10,7 @@
  * See the COPYING file in the top-level directory.
  */
 #include "hw/i386/apic_internal.h"
+#include "hw/i386/pc.h"
 #include "hw/pci/msi.h"
 #include "sysemu/kvm.h"
 
@@ -180,9 +181,14 @@ static void kvm_apic_reset(APICCommonState *s)
 static void kvm_apic_realize(DeviceState *dev, Error **errp)
 {
     APICCommonState *s = APIC_COMMON(dev);
+    PCMachineState *pcms = PC_MACHINE(qdev_get_machine());
+    static bool mmio_registered;
 
-    memory_region_init_io(&s->io_memory, NULL, &kvm_apic_io_ops, s, 
"kvm-apic-msi",
-                          APIC_SPACE_SIZE);
+    if (!mmio_registered) {
+        memory_region_init_io(&pcms->apic_mmio, NULL, &kvm_apic_io_ops, s,
+                              "kvm-apic-msi", APIC_SPACE_SIZE);
+        mmio_registered = true;
+    }
 
     if (kvm_has_gsi_routing()) {
         msi_supported = true;
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index c7af6aa..500d369 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -54,7 +54,6 @@
 #include "qemu/config-file.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"
@@ -954,12 +953,12 @@ 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;
     Error *local_err = NULL;
 
-    cpu = cpu_x86_create(cpu_model, icc_bridge, &local_err);
+    cpu = cpu_x86_create(cpu_model, &local_err);
     if (local_err != NULL) {
         error_propagate(errp, local_err);
         return NULL;
@@ -980,7 +979,6 @@ static const char *current_cpu_model;
 
 void pc_hot_add_cpu(const int64_t id, Error **errp)
 {
-    DeviceState *icc_bridge;
     int64_t apic_id = x86_cpu_apic_id_from_index(id);
 
     if (id < 0) {
@@ -1007,12 +1005,10 @@ 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));
-    pc_new_cpu(current_cpu_model, apic_id, icc_bridge, errp);
+    pc_new_cpu(current_cpu_model, apic_id, errp);
 }
 
-void pc_cpus_init(const char *cpu_model, DeviceState *icc_bridge)
+void pc_cpus_init(const char *cpu_model)
 {
     int i;
     X86CPU *cpu = NULL;
@@ -1038,7 +1034,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("%s", error_get_pretty(error));
             error_free(error);
@@ -1046,13 +1042,6 @@ void pc_cpus_init(const char *cpu_model, DeviceState 
*icc_bridge)
         }
     }
 
-    /* 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/i386/pc_piix.c b/hw/i386/pc_piix.c
index 38b42b0..7a581f2 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"
@@ -97,7 +96,6 @@ static void pc_init1(MachineState *machine,
     MemoryRegion *ram_memory;
     MemoryRegion *pci_memory;
     MemoryRegion *rom_memory;
-    DeviceState *icc_bridge;
     FWCfgState *fw_cfg = NULL;
     PcGuestInfo *guest_info;
     ram_addr_t lowmem;
@@ -142,11 +140,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();
@@ -228,7 +222,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 63027ee..baceafb 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"
 
 /* ICH9 AHCI has 6 ports */
@@ -84,7 +83,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];
@@ -131,11 +129,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();
@@ -238,7 +232,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.c b/hw/intc/apic.c
index 0f97b47..e48974f 100644
--- a/hw/intc/apic.c
+++ b/hw/intc/apic.c
@@ -376,7 +376,8 @@ static void apic_update_irq(APICCommonState *s)
         cpu_interrupt(cpu, CPU_INTERRUPT_POLL);
     } else if (apic_irq_pending(s) > 0) {
         cpu_interrupt(cpu, CPU_INTERRUPT_HARD);
-    } else if (!apic_accept_pic_intr(&s->busdev.qdev) || 
!pic_get_output(isa_pic)) {
+    } else if (!apic_accept_pic_intr(s->cpu->apic_state) ||
+               !pic_get_output(isa_pic)) {
         cpu_reset_interrupt(cpu, CPU_INTERRUPT_HARD);
     }
 }
@@ -549,10 +550,10 @@ static void apic_deliver(DeviceState *dev, uint8_t dest, 
uint8_t dest_mode,
 
 static bool apic_check_pic(APICCommonState *s)
 {
-    if (!apic_accept_pic_intr(&s->busdev.qdev) || !pic_get_output(isa_pic)) {
+    if (!apic_accept_pic_intr(s->cpu->apic_state) || !pic_get_output(isa_pic)) 
{
         return false;
     }
-    apic_deliver_pic_intr(&s->busdev.qdev, 1);
+    apic_deliver_pic_intr(s->cpu->apic_state, 1);
     return true;
 }
 
@@ -879,9 +880,14 @@ static const MemoryRegionOps apic_io_ops = {
 static void apic_realize(DeviceState *dev, Error **errp)
 {
     APICCommonState *s = APIC_COMMON(dev);
+    PCMachineState *pcms = PC_MACHINE(qdev_get_machine());
+    static bool mmio_registered;
 
-    memory_region_init_io(&s->io_memory, OBJECT(s), &apic_io_ops, s, 
"apic-msi",
-                          APIC_SPACE_SIZE);
+    if (!mmio_registered) {
+        memory_region_init_io(&pcms->apic_mmio, OBJECT(s), &apic_io_ops, s,
+                              "apic-msi", APIC_SPACE_SIZE);
+        mmio_registered = true;
+    }
 
     s->timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, apic_timer, s);
     local_apics[s->idx] = s;
diff --git a/hw/intc/apic_common.c b/hw/intc/apic_common.c
index 0858b45..bfaebe2 100644
--- a/hw/intc/apic_common.c
+++ b/hw/intc/apic_common.c
@@ -19,6 +19,7 @@
  */
 #include "hw/i386/apic.h"
 #include "hw/i386/apic_internal.h"
+#include "hw/i386/pc.h"
 #include "trace.h"
 #include "sysemu/kvm.h"
 #include "hw/qdev.h"
@@ -302,6 +303,7 @@ static void apic_common_realize(DeviceState *dev, Error 
**errp)
     APICCommonClass *info;
     static DeviceState *vapic;
     static int apic_no;
+    PCMachineState *pcms = PC_MACHINE(qdev_get_machine());
     static bool mmio_registered;
 
     if (apic_no >= MAX_APICS) {
@@ -313,9 +315,12 @@ 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);
+        memory_region_add_subregion_overlap(get_system_memory(),
+                                            APIC_DEFAULT_ADDRESS,
+                                            &pcms->apic_mmio,
+                                            0x1000);
         mmio_registered = true;
     }
 
@@ -433,13 +438,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()
@@ -449,7 +453,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 dc7a89d..0421c20 100644
--- a/include/hw/i386/apic_internal.h
+++ b/include/hw/i386/apic_internal.h
@@ -21,7 +21,6 @@
 #define QEMU_APIC_INTERNAL_H
 
 #include "exec/memory.h"
-#include "hw/cpu/icc_bus.h"
 #include "qemu/timer.h"
 
 /* APIC Local Vector Table */
@@ -78,7 +77,7 @@ typedef struct APICCommonState APICCommonState;
 
 typedef struct APICCommonClass
 {
-    ICCDeviceClass parent_class;
+    DeviceClass parent_class;
 
     DeviceRealize realize;
     void (*set_base)(APICCommonState *s, uint64_t val);
@@ -93,9 +92,8 @@ typedef struct APICCommonClass
 } APICCommonClass;
 
 struct APICCommonState {
-    ICCDevice busdev;
+    DeviceState parent;
 
-    MemoryRegion io_memory;
     X86CPU *cpu;
     uint32_t apicbase;
     uint8_t id;
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 69d9cf8..ce0143c 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -34,6 +34,7 @@ struct PCMachineState {
     /* <public> */
     ram_addr_t hotplug_memory_base;
     MemoryRegion hotplug_memory;
+    MemoryRegion apic_mmio;
 
     HotplugHandler *acpi_dev;
     ISADevice *rtc;
@@ -177,7 +178,7 @@ extern int fd_bootchk;
 void pc_register_ferr_irq(qemu_irq irq);
 void pc_acpi_smi_interrupt(void *opaque, int irq, int level);
 
-void pc_cpus_init(const char *cpu_model, DeviceState *icc_bridge);
+void pc_cpus_init(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 3a9b32e..9e9f830 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 "hw/xen/xen.h"
 #include "hw/i386/apic_internal.h"
@@ -2091,8 +2090,7 @@ static void x86_cpu_load_def(X86CPU *cpu, 
X86CPUDefinition *def, Error **errp)
 
 }
 
-X86CPU *cpu_x86_create(const char *cpu_model, DeviceState *icc_bridge,
-                       Error **errp)
+X86CPU *cpu_x86_create(const char *cpu_model, Error **errp)
 {
     X86CPU *cpu = NULL;
     X86CPUClass *xcc;
@@ -2123,15 +2121,6 @@ X86CPU *cpu_x86_create(const char *cpu_model, 
DeviceState *icc_bridge,
 
     cpu = X86_CPU(object_new(object_class_get_name(oc)));
 
-#ifndef CONFIG_USER_ONLY
-    if (icc_bridge == NULL) {
-        error_setg(&error, "Invalid icc-bridge value");
-        goto out;
-    }
-    qdev_set_parent_bus(DEVICE(cpu), qdev_get_child_bus(icc_bridge, "icc"));
-    object_unref(OBJECT(cpu));
-#endif
-
     x86_cpu_parse_featurestr(CPU(cpu), features, &error);
     if (error) {
         goto out;
@@ -2154,7 +2143,7 @@ X86CPU *cpu_x86_init(const char *cpu_model)
     Error *error = NULL;
     X86CPU *cpu;
 
-    cpu = cpu_x86_create(cpu_model, NULL, &error);
+    cpu = cpu_x86_create(cpu_model, &error);
     if (error) {
         goto out;
     }
@@ -2723,7 +2712,6 @@ static void mce_init(X86CPU *cpu)
 static void x86_cpu_apic_create(X86CPU *cpu, Error **errp)
 {
     CPUX86State *env = &cpu->env;
-    DeviceState *dev = DEVICE(cpu);
     APICCommonState *apic;
     const char *apic_type = "apic";
 
@@ -2733,7 +2721,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);
+    cpu->apic_state = qdev_try_create(NULL, apic_type);
     if (cpu->apic_state == NULL) {
         error_setg(errp, "APIC device '%s' could not be created", apic_type);
         return;
@@ -3004,7 +2992,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;
diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index 478450c..f4711ba 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -983,8 +983,7 @@ typedef struct CPUX86State {
 #include "cpu-qom.h"
 
 X86CPU *cpu_x86_init(const char *cpu_model);
-X86CPU *cpu_x86_create(const char *cpu_model, DeviceState *icc_bridge,
-                       Error **errp);
+X86CPU *cpu_x86_create(const char *cpu_model, Error **errp);
 int cpu_x86_exec(CPUX86State *s);
 void x86_cpu_list(FILE *f, fprintf_function cpu_fprintf);
 void x86_cpudef_setup(void);
-- 
1.9.3