Patchwork [03/13] pci: Introduce cached device INTx routing

login
register
mail settings
Submitter Jan Kiszka
Date June 4, 2012, 8:52 a.m.
Message ID <aa4ff6ec67c299a0a975c61d7b4698d32ba60877.1338799935.git.jan.kiszka@siemens.com>
Download mbox | patch
Permalink /patch/162678/
State New
Headers show

Comments

Jan Kiszka - June 4, 2012, 8:52 a.m.
Based on the original version by Michael Tsirkin: Instead of traversing
the PCI bus hierarchy from a device to the host bridge, cache this path
in PCIDevice and use it directly on interrupt delivery. This will
specifically pay off with more complex topologies than the current
single host bus.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 hw/pci.c |   53 ++++++++++++++++++++++++++++++++++++++++++++---------
 hw/pci.h |    4 ++++
 2 files changed, 48 insertions(+), 9 deletions(-)
Michael S. Tsirkin - June 10, 2012, 11:28 a.m.
On Mon, Jun 04, 2012 at 10:52:11AM +0200, Jan Kiszka wrote:
> Based on the original version by Michael Tsirkin: Instead of traversing
> the PCI bus hierarchy from a device to the host bridge, cache this path
> in PCIDevice and use it directly on interrupt delivery. This will
> specifically pay off with more complex topologies than the current
> single host bus.
> 
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>

OK, I applied this. I still think we should go the extra
mile and cache the final IRQ not just the root intx.
But that can be a patch on top.

> ---
>  hw/pci.c |   53 ++++++++++++++++++++++++++++++++++++++++++++---------
>  hw/pci.h |    4 ++++
>  2 files changed, 48 insertions(+), 9 deletions(-)
> 
> diff --git a/hw/pci.c b/hw/pci.c
> index 33452ab..771fb39 100644
> --- a/hw/pci.c
> +++ b/hw/pci.c
> @@ -68,6 +68,8 @@ static void pci_update_mappings(PCIDevice *d);
>  static void pci_set_irq(void *opaque, int irq_num, int level);
>  static int pci_add_option_rom(PCIDevice *pdev, bool is_default_rom);
>  static void pci_del_option_rom(PCIDevice *pdev);
> +static void pci_for_each_device_under_bus(PCIBus *bus,
> +                                          void (*fn)(PCIBus *b, PCIDevice *d));
>  
>  static uint16_t pci_default_sub_vendor_id = PCI_SUBVENDOR_ID_REDHAT_QUMRANET;
>  static uint16_t pci_default_sub_device_id = PCI_SUBDEVICE_ID_QEMU;
> @@ -112,18 +114,49 @@ static inline void pci_set_irq_state(PCIDevice *d, int irq_num, int level)
>  	d->irq_state |= level << irq_num;
>  }
>  
> -static void pci_change_irq_level(PCIDevice *pci_dev, int irq_num, int change)
> +static void pci_set_device_intx_routing(PCIBus *bus, PCIDevice *dev)
>  {
> -    PCIBus *bus;
> +    int pin, output_pin;
> +    PCIDevice *pci_dev;
> +
> +    /* We might be too early, i.e. before pci_bus_irqs was called.
> +     * We will be called again when this happened. */
> +    if (!bus->map_irq) {
> +        return;
> +    }
> +
> +    for (pin = 0; pin < PCI_NUM_PINS; pin++) {
> +        pci_dev = dev;
> +        output_pin = pin;
> +        do {
> +            bus = pci_dev->bus;
> +            output_pin = bus->map_irq(pci_dev, output_pin);
> +            pci_dev = bus->parent_dev;
> +        } while (pci_dev);
> +
> +        dev->host_intx_pin[pin] = output_pin;
> +        dev->host_bus = bus;
> +    }
> +}
> +
> +static void pci_set_bus_intx_routing(PCIBus *bus)
> +{
> +    PCIBus *sec;
> +
> +    pci_for_each_device_under_bus(bus, pci_set_device_intx_routing);
>  
> -    do {
> -        bus = pci_dev->bus;
> -        irq_num = bus->map_irq(pci_dev, irq_num);
> -        pci_dev = bus->parent_dev;
> -    } while (pci_dev);
> +    QLIST_FOREACH(sec, &bus->child, sibling) {
> +        pci_set_bus_intx_routing(sec);
> +    }
> +}
> +
> +static void pci_change_irq_level(PCIDevice *dev, int pin, int change)
> +{
> +    PCIBus *bus = dev->host_bus;
> +    int output_pin = dev->host_intx_pin[pin];
>  
> -    bus->irq_count[irq_num] += change;
> -    bus->set_irq(bus->irq_opaque, irq_num, bus->irq_count[irq_num] != 0);
> +    bus->irq_count[output_pin] += change;
> +    bus->set_irq(bus->irq_opaque, output_pin, bus->irq_count[output_pin] != 0);
>  }
>  
>  int pci_bus_get_irq_level(PCIBus *bus, int irq_num)
> @@ -293,6 +326,7 @@ void pci_bus_irqs(PCIBus *bus, pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
>      bus->irq_opaque = irq_opaque;
>      bus->nirq = nirq;
>      bus->irq_count = g_malloc0(nirq * sizeof(bus->irq_count[0]));
> +    pci_set_bus_intx_routing(bus);
>  }
>  
>  void pci_bus_hotplug(PCIBus *bus, pci_hotplug_fn hotplug, DeviceState *qdev)
> @@ -798,6 +832,7 @@ static PCIDevice *do_pci_register_device(PCIDevice *pci_dev, PCIBus *bus,
>      bus->devices[devfn] = pci_dev;
>      pci_dev->irq = qemu_allocate_irqs(pci_set_irq, pci_dev, PCI_NUM_PINS);
>      pci_dev->version_id = 2; /* Current pci device vmstate version */
> +    pci_set_device_intx_routing(bus, pci_dev);
>      return pci_dev;
>  }
>  
> diff --git a/hw/pci.h b/hw/pci.h
> index 7eaf90b..c4fd863 100644
> --- a/hw/pci.h
> +++ b/hw/pci.h
> @@ -211,6 +211,10 @@ struct PCIDevice {
>      /* Current IRQ levels.  Used internally by the generic PCI code.  */
>      uint8_t irq_state;
>  
> +    /* Used internally by PCI code to cache the interrupt routing */
> +    PCIBus *host_bus;
> +    int host_intx_pin[PCI_NUM_PINS];
> +

A better name would be root_XXXX: host is an overloaded term.
I applied such a patch.

>      /* Capability bits */
>      uint32_t cap_present;
>  
> -- 
> 1.7.3.4

Patch

diff --git a/hw/pci.c b/hw/pci.c
index 33452ab..771fb39 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -68,6 +68,8 @@  static void pci_update_mappings(PCIDevice *d);
 static void pci_set_irq(void *opaque, int irq_num, int level);
 static int pci_add_option_rom(PCIDevice *pdev, bool is_default_rom);
 static void pci_del_option_rom(PCIDevice *pdev);
+static void pci_for_each_device_under_bus(PCIBus *bus,
+                                          void (*fn)(PCIBus *b, PCIDevice *d));
 
 static uint16_t pci_default_sub_vendor_id = PCI_SUBVENDOR_ID_REDHAT_QUMRANET;
 static uint16_t pci_default_sub_device_id = PCI_SUBDEVICE_ID_QEMU;
@@ -112,18 +114,49 @@  static inline void pci_set_irq_state(PCIDevice *d, int irq_num, int level)
 	d->irq_state |= level << irq_num;
 }
 
-static void pci_change_irq_level(PCIDevice *pci_dev, int irq_num, int change)
+static void pci_set_device_intx_routing(PCIBus *bus, PCIDevice *dev)
 {
-    PCIBus *bus;
+    int pin, output_pin;
+    PCIDevice *pci_dev;
+
+    /* We might be too early, i.e. before pci_bus_irqs was called.
+     * We will be called again when this happened. */
+    if (!bus->map_irq) {
+        return;
+    }
+
+    for (pin = 0; pin < PCI_NUM_PINS; pin++) {
+        pci_dev = dev;
+        output_pin = pin;
+        do {
+            bus = pci_dev->bus;
+            output_pin = bus->map_irq(pci_dev, output_pin);
+            pci_dev = bus->parent_dev;
+        } while (pci_dev);
+
+        dev->host_intx_pin[pin] = output_pin;
+        dev->host_bus = bus;
+    }
+}
+
+static void pci_set_bus_intx_routing(PCIBus *bus)
+{
+    PCIBus *sec;
+
+    pci_for_each_device_under_bus(bus, pci_set_device_intx_routing);
 
-    do {
-        bus = pci_dev->bus;
-        irq_num = bus->map_irq(pci_dev, irq_num);
-        pci_dev = bus->parent_dev;
-    } while (pci_dev);
+    QLIST_FOREACH(sec, &bus->child, sibling) {
+        pci_set_bus_intx_routing(sec);
+    }
+}
+
+static void pci_change_irq_level(PCIDevice *dev, int pin, int change)
+{
+    PCIBus *bus = dev->host_bus;
+    int output_pin = dev->host_intx_pin[pin];
 
-    bus->irq_count[irq_num] += change;
-    bus->set_irq(bus->irq_opaque, irq_num, bus->irq_count[irq_num] != 0);
+    bus->irq_count[output_pin] += change;
+    bus->set_irq(bus->irq_opaque, output_pin, bus->irq_count[output_pin] != 0);
 }
 
 int pci_bus_get_irq_level(PCIBus *bus, int irq_num)
@@ -293,6 +326,7 @@  void pci_bus_irqs(PCIBus *bus, pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
     bus->irq_opaque = irq_opaque;
     bus->nirq = nirq;
     bus->irq_count = g_malloc0(nirq * sizeof(bus->irq_count[0]));
+    pci_set_bus_intx_routing(bus);
 }
 
 void pci_bus_hotplug(PCIBus *bus, pci_hotplug_fn hotplug, DeviceState *qdev)
@@ -798,6 +832,7 @@  static PCIDevice *do_pci_register_device(PCIDevice *pci_dev, PCIBus *bus,
     bus->devices[devfn] = pci_dev;
     pci_dev->irq = qemu_allocate_irqs(pci_set_irq, pci_dev, PCI_NUM_PINS);
     pci_dev->version_id = 2; /* Current pci device vmstate version */
+    pci_set_device_intx_routing(bus, pci_dev);
     return pci_dev;
 }
 
diff --git a/hw/pci.h b/hw/pci.h
index 7eaf90b..c4fd863 100644
--- a/hw/pci.h
+++ b/hw/pci.h
@@ -211,6 +211,10 @@  struct PCIDevice {
     /* Current IRQ levels.  Used internally by the generic PCI code.  */
     uint8_t irq_state;
 
+    /* Used internally by PCI code to cache the interrupt routing */
+    PCIBus *host_bus;
+    int host_intx_pin[PCI_NUM_PINS];
+
     /* Capability bits */
     uint32_t cap_present;