diff mbox

[18/22] target-i386: move IOAPIC to ICC bus

Message ID 1365172636-28628-20-git-send-email-imammedo@redhat.com
State New
Headers show

Commit Message

Igor Mammedov April 5, 2013, 2:37 p.m. UTC 
* inherit IOAPICs from ICCDevice and attach them to ICC bus
 * map IOAPIC's mmio at board level
 * make IOAPIC a child device of icc-bridge

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
 hw/i386/pc.c          | 23 +++++++++--------------
 hw/i386/pc_piix.c     |  2 +-
 hw/i386/pc_q35.c      |  2 +-
 hw/icc_bus.c          | 28 +++++++++++++++++++++++++++-
 hw/icc_bus.h          |  2 ++
 hw/ioapic_common.c    | 15 +++++++++++----
 hw/ioapic_internal.h  |  6 +++---
 hw/kvm/ioapic.c       |  2 +-
 hw/pc.h               |  2 +-
 include/exec/memory.h | 19 +++++++++++++++++++
 10 files changed, 75 insertions(+), 26 deletions(-)

Comments

Paolo Bonzini April 9, 2013, 11:33 a.m. UTC | #1 
Il 05/04/2013 16:37, Igor Mammedov ha scritto:
> +    const char *ioapic_name = "ioapic";
>  
>      if (kvm_irqchip_in_kernel()) {
> -        dev = qdev_create(NULL, "kvm-ioapic");
> -    } else {
> -        dev = qdev_create(NULL, "ioapic");
> -    }
> -    if (parent_name) {
> -        object_property_add_child(object_resolve_path(parent_name, NULL),
> -                                  "ioapic", OBJECT(dev), NULL);
> +        ioapic_name = "kvm-ioapic";
>      }
> -    qdev_init_nofail(dev);
> -    d = SYS_BUS_DEVICE(dev);
> -    sysbus_mmio_map(d, 0, IO_APIC_DEFAULT_ADDRESS);
> +    object_property_set_str(OBJECT(dev), ioapic_name, "ioapic-type", NULL);

Why do you need this?

Having the IOAPIC as a QOM child of the icc-bridge is not too important,
I think.  Perhaps not even too correct...

Paolo
Igor Mammedov April 9, 2013, 12:51 p.m. UTC | #2 
On Tue, 09 Apr 2013 13:33:56 +0200
Paolo Bonzini <pbonzini@redhat.com> wrote:

> Il 05/04/2013 16:37, Igor Mammedov ha scritto:
> > +    const char *ioapic_name = "ioapic";
> >  
> >      if (kvm_irqchip_in_kernel()) {
> > -        dev = qdev_create(NULL, "kvm-ioapic");
> > -    } else {
> > -        dev = qdev_create(NULL, "ioapic");
> > -    }
> > -    if (parent_name) {
> > -        object_property_add_child(object_resolve_path(parent_name, NULL),
> > -                                  "ioapic", OBJECT(dev), NULL);
> > +        ioapic_name = "kvm-ioapic";
> >      }
> > -    qdev_init_nofail(dev);
> > -    d = SYS_BUS_DEVICE(dev);
> > -    sysbus_mmio_map(d, 0, IO_APIC_DEFAULT_ADDRESS);
> > +    object_property_set_str(OBJECT(dev), ioapic_name, "ioapic-type",
> > NULL);
> 
> Why do you need this?
> 
> Having the IOAPIC as a QOM child of the icc-bridge is not too important,
> I think.  Perhaps not even too correct...
icc-bridge provides address space for IOAPIC, so it looked like a right thing
to make sure that destruction order would be IOAPIC first and only then parent
icc-bridge.

This patch is not really necessary for hot-plug, we could easily drop it, if
you prefer.

BTW:
 looks like this pre-reordering patch slipped in
 Correct one is "[PATCH 19/22] target-i386: move IOAPIC to ICC bus"

> 
> Paolo
>
diff mbox

Patch

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 7a608c5..fbc0bcf 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -51,6 +51,7 @@ 
 #include "exec/address-spaces.h"
 #include "sysemu/arch_init.h"
 #include "qemu/bitmap.h"
+#include "hw/icc_bus.h"
 
 /* debug PC/ISA interrupts */
 //#define DEBUG_IRQ
@@ -1166,26 +1167,20 @@  void pc_pci_device_init(PCIBus *pci_bus)
     }
 }
 
-void ioapic_init_gsi(GSIState *gsi_state, const char *parent_name)
+void ioapic_init_gsi(GSIState *gsi_state, DeviceState *dev)
 {
-    DeviceState *dev;
-    SysBusDevice *d;
     unsigned int i;
+    DeviceState *ioapic;
+    const char *ioapic_name = "ioapic";
 
     if (kvm_irqchip_in_kernel()) {
-        dev = qdev_create(NULL, "kvm-ioapic");
-    } else {
-        dev = qdev_create(NULL, "ioapic");
-    }
-    if (parent_name) {
-        object_property_add_child(object_resolve_path(parent_name, NULL),
-                                  "ioapic", OBJECT(dev), NULL);
+        ioapic_name = "kvm-ioapic";
     }
-    qdev_init_nofail(dev);
-    d = SYS_BUS_DEVICE(dev);
-    sysbus_mmio_map(d, 0, IO_APIC_DEFAULT_ADDRESS);
+    object_property_set_str(OBJECT(dev), ioapic_name, "ioapic-type", NULL);
 
+    sysbus_mmio_map(SYS_BUS_DEVICE(dev), 1, IO_APIC_DEFAULT_ADDRESS);
+    ioapic = DEVICE(object_resolve_path_component(OBJECT(dev), "ioapic"));
     for (i = 0; i < IOAPIC_NUM_PINS; i++) {
-        gsi_state->ioapic_irq[i] = qdev_get_gpio_in(dev, i);
+        gsi_state->ioapic_irq[i] = qdev_get_gpio_in(ioapic, i);
     }
 }
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 8bf5440..bf1e88c 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -164,7 +164,7 @@  static void pc_init1(MemoryRegion *system_memory,
         gsi_state->i8259_irq[i] = i8259[i];
     }
     if (pci_enabled) {
-        ioapic_init_gsi(gsi_state, "i440fx");
+        ioapic_init_gsi(gsi_state, icc_bridge);
     }
     qdev_init_nofail(icc_bridge);
 
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index fb701b4..94a3952 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -172,7 +172,7 @@  static void pc_q35_init(QEMUMachineInitArgs *args)
         gsi_state->i8259_irq[i] = i8259[i];
     }
     if (pci_enabled) {
-        ioapic_init_gsi(gsi_state, NULL);
+        ioapic_init_gsi(gsi_state, icc_bridge);
     }
     qdev_init_nofail(icc_bridge);
 
diff --git a/hw/icc_bus.c b/hw/icc_bus.c
index 7e75a0e..fc8d892 100644
--- a/hw/icc_bus.c
+++ b/hw/icc_bus.c
@@ -59,13 +59,39 @@  static const TypeInfo icc_device_info = {
 
 typedef struct ICCBridgeState {
     SysBusDevice busdev;
+    MemoryRegion ioapic_container;
 } ICCBridgeState;
 #define ICC_BRIGDE(obj) OBJECT_CHECK(ICCBridgeState, (obj), TYPE_ICC_BRIDGE)
 
+static void icc_bridge_prop_set_ioapic_type(Object *obj, const char *value,
+                                            Error **errp)
+{
+    BusState *bus = BUS(object_resolve_path_component(obj, "icc-bus"));
+    DeviceState *ioapic;
+
+    if (value != NULL) {
+        ioapic = qdev_create(bus, value);
+        object_property_add_child(obj, "ioapic", OBJECT(ioapic), NULL);
+        qdev_init_nofail(ioapic);
+    }
+}
 
 static void icc_bridge_initfn(Object *obj)
 {
-    qbus_create(TYPE_ICC_BUS, DEVICE(obj), "icc-bus");
+    ICCBridgeState *s = ICC_BRIGDE(obj);
+    SysBusDevice *sb = SYS_BUS_DEVICE(obj);
+    ICCBus *ibus;
+
+    object_property_add_str(obj, "ioapic-type",
+                             NULL,
+                             icc_bridge_prop_set_ioapic_type,
+                             NULL);
+
+    ibus = ICC_BUS(qbus_create(TYPE_ICC_BUS, DEVICE(obj), "icc-bus"));
+
+    memory_region_init(&s->ioapic_container, "icc-ioapic-container", 0x1000);
+    sysbus_init_mmio(sb, &s->ioapic_container);
+    ibus->ioapic_address_space = &s->ioapic_container;
 }
 
 static const TypeInfo icc_bridge_info = {
diff --git a/hw/icc_bus.h b/hw/icc_bus.h
index f959a43..b8e2032 100644
--- a/hw/icc_bus.h
+++ b/hw/icc_bus.h
@@ -19,6 +19,7 @@ 
 #ifndef ICC_BUS_H
 #define ICC_BUS_H
 
+#include "exec/memory.h"
 #include "hw/qdev-core.h"
 
 #define TYPE_ICC_BUS "icc-bus"
@@ -26,6 +27,7 @@ 
 #ifndef CONFIG_USER_ONLY
 typedef struct ICCBus {
     BusState qbus;
+    MemoryRegion *ioapic_address_space;
 } ICCBus;
 #define ICC_BUS(obj) OBJECT_CHECK(ICCBus, (obj), TYPE_ICC_BUS)
 
diff --git a/hw/ioapic_common.c b/hw/ioapic_common.c
index 561b987..a366bae 100644
--- a/hw/ioapic_common.c
+++ b/hw/ioapic_common.c
@@ -57,11 +57,13 @@  static int ioapic_dispatch_post_load(void *opaque, int version_id)
     return 0;
 }
 
-static int ioapic_init_common(SysBusDevice *dev)
+static int ioapic_init_common(ICCDevice *dev)
 {
     IOAPICCommonState *s = IOAPIC_COMMON(dev);
+    DeviceState *d = DEVICE(dev);
     IOAPICCommonClass *info;
     static int ioapic_no;
+    static bool mmio_registered;
 
     if (ioapic_no >= MAX_IOAPICS) {
         return -1;
@@ -70,7 +72,12 @@  static int ioapic_init_common(SysBusDevice *dev)
     info = IOAPIC_COMMON_GET_CLASS(s);
     info->init(s, ioapic_no);
 
-    sysbus_init_mmio(&s->busdev, &s->io_memory);
+    if (!mmio_registered) {
+        MemoryRegion *as = ICC_BUS(d->parent_bus)->ioapic_address_space;
+        memory_region_add_subregion(as, 0, &s->io_memory);
+        mmio_registered = true;
+    }
+
     ioapic_no++;
 
     return 0;
@@ -95,7 +102,7 @@  static const VMStateDescription vmstate_ioapic_common = {
 
 static void ioapic_common_class_init(ObjectClass *klass, void *data)
 {
-    SysBusDeviceClass *sc = SYS_BUS_DEVICE_CLASS(klass);
+    ICCDeviceClass *sc = ICC_DEVICE_CLASS(klass);
     DeviceClass *dc = DEVICE_CLASS(klass);
 
     sc->init = ioapic_init_common;
@@ -105,7 +112,7 @@  static void ioapic_common_class_init(ObjectClass *klass, void *data)
 
 static const TypeInfo ioapic_common_type = {
     .name = TYPE_IOAPIC_COMMON,
-    .parent = TYPE_SYS_BUS_DEVICE,
+    .parent = TYPE_ICC_DEVICE,
     .instance_size = sizeof(IOAPICCommonState),
     .class_size = sizeof(IOAPICCommonClass),
     .class_init = ioapic_common_class_init,
diff --git a/hw/ioapic_internal.h b/hw/ioapic_internal.h
index 25576c8..c1db3f0 100644
--- a/hw/ioapic_internal.h
+++ b/hw/ioapic_internal.h
@@ -24,7 +24,7 @@ 
 
 #include "hw/hw.h"
 #include "exec/memory.h"
-#include "hw/sysbus.h"
+#include "hw/icc_bus.h"
 
 #define MAX_IOAPICS                     1
 
@@ -82,14 +82,14 @@  typedef struct IOAPICCommonState IOAPICCommonState;
      OBJECT_GET_CLASS(IOAPICCommonClass, (obj), TYPE_IOAPIC_COMMON)
 
 typedef struct IOAPICCommonClass {
-    SysBusDeviceClass parent_class;
+    ICCDeviceClass parent_class;
     void (*init)(IOAPICCommonState *s, int instance_no);
     void (*pre_save)(IOAPICCommonState *s);
     void (*post_load)(IOAPICCommonState *s);
 } IOAPICCommonClass;
 
 struct IOAPICCommonState {
-    SysBusDevice busdev;
+    ICCDevice busdev;
     MemoryRegion io_memory;
     uint8_t id;
     uint8_t ioregsel;
diff --git a/hw/kvm/ioapic.c b/hw/kvm/ioapic.c
index 23877d4..ca9b249 100644
--- a/hw/kvm/ioapic.c
+++ b/hw/kvm/ioapic.c
@@ -96,7 +96,7 @@  static void kvm_ioapic_put(IOAPICCommonState *s)
 
     kioapic->id = s->id;
     kioapic->ioregsel = s->ioregsel;
-    kioapic->base_address = s->busdev.mmio[0].addr;
+    kioapic->base_address = memory_region_get_address(&s->io_memory);
     kioapic->irr = s->irr;
     for (i = 0; i < IOAPIC_NUM_PINS; i++) {
         kioapic->redirtbl[i].bits = s->ioredtbl[i];
diff --git a/hw/pc.h b/hw/pc.h
index 8e1dd4c..dbd5794 100644
--- a/hw/pc.h
+++ b/hw/pc.h
@@ -105,7 +105,7 @@  void pc_pci_device_init(PCIBus *pci_bus);
 typedef void (*cpu_set_smm_t)(int smm, void *arg);
 void cpu_smm_register(cpu_set_smm_t callback, void *arg);
 
-void ioapic_init_gsi(GSIState *gsi_state, const char *parent_name);
+void ioapic_init_gsi(GSIState *gsi_state, DeviceState *ioapic);
 
 /* acpi.c */
 extern int acpi_enabled;
diff --git a/include/exec/memory.h b/include/exec/memory.h
index 2322732..223008a5 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -711,6 +711,25 @@  void memory_region_set_enabled(MemoryRegion *mr, bool enabled);
 void memory_region_set_address(MemoryRegion *mr, hwaddr addr);
 
 /*
+ * memory_region_get_address: get current the address of a region
+ *
+ * Returns the absolute address of a region.
+ * May be used on regions that are currently part of a memory hierarchy.
+ *
+ * @mr: the region being queried
+ */
+static inline hwaddr memory_region_get_address(MemoryRegion *mr)
+{
+    hwaddr addr = mr->addr;
+
+    while (mr->parent) {
+        mr = mr->parent;
+        addr += mr->addr;
+    }
+    return addr;
+}
+
+/*
  * memory_region_set_alias_offset: dynamically update a memory alias's offset
  *
  * Dynamically updates the offset into the target region that an alias points