[Qemu-devel] [PATCH 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                    | 21 +++++----------------
 hw/i386/pc_piix.c               |  9 +--------
 hw/i386/pc_q35.c                |  9 +--------
 hw/intc/apic.c                  |  6 +++---
 hw/intc/apic_common.c           | 24 +++++++++++++++++++-----
 include/hw/i386/apic_internal.h |  5 ++---
 include/hw/i386/pc.h            |  2 +-
 target-i386/cpu.c               | 15 ++-------------
 target-i386/cpu.h               |  2 +-
 9 files changed, 35 insertions(+), 58 deletions(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index b5b2aad..660e244 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -57,7 +57,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"
@@ -990,12 +989,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;
@@ -1016,7 +1015,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) {
@@ -1043,12 +1041,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;
@@ -1074,20 +1070,13 @@ 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);
         }
     }
 
-    /* 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 8eab4ba..cc9adee 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 c0f21fe..d018e05 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..020219e 100644
--- a/hw/intc/apic_common.c
+++ b/hw/intc/apic_common.c
@@ -27,6 +27,8 @@
 static int apic_irq_delivered;
 bool apic_report_tpr_access;
 
+static MemoryRegion *apic_container;
+
 void cpu_set_apic_base(DeviceState *dev, uint64_t val)
 {
     trace_cpu_set_apic_base(val);
@@ -314,8 +316,11 @@ 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(apic_container, 0, &s->io_memory);
+        memory_region_add_subregion_overlap(CPU(s->cpu)->as->root,
+                                            APIC_DEFAULT_ADDRESS,
+                                            apic_container,
+                                            0x1000);
         mmio_registered = true;
     }
 
@@ -431,15 +436,23 @@ static Property apic_properties_common[] = {
     DEFINE_PROP_END_OF_LIST(),
 };
 
+static void apic_common_initfn(Object *obj)
+{
+    if (!apic_container) {
+        apic_container = g_malloc(sizeof(*apic_container));
+        memory_region_init(apic_container, obj, "apic-container",
+                           APIC_SPACE_SIZE);
+    }
+}
+
 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,8 +462,9 @@ 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),
+    .instance_init = apic_common_initfn,
     .class_size = sizeof(APICCommonClass),
     .class_init = apic_common_class_init,
     .abstract = true,
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 ed7e5d5..4993ab4 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"
@@ -2076,7 +2075,7 @@ static void x86_cpu_load_def(X86CPU *cpu, 
X86CPUDefinition *def, Error **errp)
 
 }
 
-X86CPU *cpu_x86_create(const char *cpu_model, DeviceState *icc_bridge,
+X86CPU *cpu_x86_create(const char *cpu_model,
                        Error **errp)
 {
     X86CPU *cpu = NULL;
@@ -2108,15 +2107,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;
@@ -2139,7 +2129,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;
     }
@@ -2951,7 +2941,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 e4c27b1..52c30f9 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -982,7 +982,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,
+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);
-- 
1.9.3