[Qemu-devel] [PATCH v2 1/2] cpu/apic: drop icc bus/bridge/

[Chen Fan]

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>
---
 hw/i386/pc.c                    | 29 +++++++++++------------------
 hw/i386/pc_piix.c               |  9 +--------
 hw/i386/pc_q35.c                |  9 +--------
 hw/intc/apic.c                  |  6 +++---
 hw/intc/apic_common.c           | 11 ++---------
 include/hw/i386/apic_internal.h |  5 ++---
 include/hw/i386/pc.h            |  2 +-
 target-i386/cpu.c               |  2 --
 8 files changed, 21 insertions(+), 52 deletions(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 4b46c29..5d15473 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -58,7 +58,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"
@@ -66,6 +65,7 @@
 #include "trace.h"
 #include "qapi/visitor.h"
 #include "qapi-visit.h"
+#include "hw/i386/apic_internal.h"
 
 /* debug PC/ISA interrupts */
 //#define DEBUG_IRQ
@@ -991,24 +991,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_unref(OBJECT(cpu));
-
     object_property_set_int(OBJECT(cpu), apic_id, "apic-id", &local_err);
     object_property_set_bool(OBJECT(cpu), true, "realized", &local_err);
 
@@ -1025,7 +1017,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) {
@@ -1052,17 +1043,16 @@ 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;
     Error *error = NULL;
     unsigned long apic_id_limit;
+    APICCommonState *apic = NULL;
 
     /* init CPUs */
     if (cpu_model == NULL) {
@@ -1083,7 +1073,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);
@@ -1093,8 +1083,11 @@ 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);
+        apic = APIC_COMMON(cpu->apic_state);
+        memory_region_add_subregion_overlap(CPU(cpu)->as->root,
+                                            APIC_DEFAULT_ADDRESS,
+                                            &apic->io_memory,
+                                            0x1000);
     }
 
     /* tell smbios about cpuid version and features */
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 36c69d7..5c51dfe 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;
     FWCfgState *fw_cfg = NULL;
     PcGuestInfo *guest_info;
     ram_addr_t lowmem;
@@ -143,11 +141,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();
@@ -230,7 +224,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 bc40537..e98bfbc 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 */
@@ -85,7 +84,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(machine->cpu_model, icc_bridge);
+    pc_cpus_init(machine->cpu_model);
     pc_acpi_init("q35-acpi-dsdt.aml");
 
     kvmclock_create();
@@ -240,7 +234,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..00ae0ec 100644
--- a/hw/intc/apic.c
+++ b/hw/intc/apic.c
@@ -376,7 +376,7 @@ 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(DEVICE(s)) || !pic_get_output(isa_pic)) {
         cpu_reset_interrupt(cpu, CPU_INTERRUPT_HARD);
     }
 }
@@ -549,10 +549,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(DEVICE(s)) || !pic_get_output(isa_pic)) {
         return false;
     }
-    apic_deliver_pic_intr(&s->busdev.qdev, 1);
+    apic_deliver_pic_intr(DEVICE(s), 1);
     return true;
 }
 
diff --git a/hw/intc/apic_common.c b/hw/intc/apic_common.c
index 0858b45..9cc6bda 100644
--- a/hw/intc/apic_common.c
+++ b/hw/intc/apic_common.c
@@ -302,7 +302,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.",
@@ -313,11 +312,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 &&
@@ -433,13 +427,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 +442,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..6eeceff 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,7 +92,7 @@ typedef struct APICCommonClass
 } APICCommonClass;
 
 struct APICCommonState {
-    ICCDevice busdev;
+    DeviceState dev;
 
     MemoryRegion io_memory;
     X86CPU *cpu;
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 1b35168..7c9d044 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -163,7 +163,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 f01690b..2385e6b 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -42,7 +42,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"
@@ -2941,7 +2940,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