Re: [Qemu-devel] [PATCH v2 2/2] pci: Add INTx routing notifier

[Alex Williamson]

On Mon, 2012-07-02 at 14:38 +0200, Jan Kiszka wrote:
> This per-device notifier shall be triggered by any interrupt router
> along the path of a device's legacy interrupt signal on routing changes.
> For simplicity reasons and as this is a slow path anyway, no further
> details on the routing changes are provided. Instead, the callback is
> expected to use pci_device_route_intx_to_irq to check the effect of the
> change.
> 
> Will be used by KVM PCI device assignment and VFIO.
> 
> Signed-off-by: Jan Kiszka <address@hidden>
> ---
>  hw/pci.c      |   23 +++++++++++++++++++++++
>  hw/pci.h      |    7 +++++++
>  hw/piix_pci.c |    2 ++
>  3 files changed, 32 insertions(+), 0 deletions(-)
> 
> diff --git a/hw/pci.c b/hw/pci.c
> index 311ba09..772141a 100644
> --- a/hw/pci.c
> +++ b/hw/pci.c
> @@ -1095,6 +1095,29 @@ PCIINTxRoute pci_device_route_intx_to_irq(PCIDevice 
> *dev, int pin)
>      return bus->route_intx_to_irq(bus->irq_opaque, pin);
>  }
>  
> +void pci_bus_fire_intx_routing_notifier(PCIBus *bus)
> +{
> +    PCIDevice *dev;
> +    PCIBus *sec;
> +    int i;
> +
> +    for (i = 0; i < ARRAY_SIZE(bus->devices); ++i) {
> +        dev = bus->devices[i];
> +        if (dev && dev->intx_routing_notifier) {
> +            dev->intx_routing_notifier(dev);
> +        }
> +        QLIST_FOREACH(sec, &bus->child, sibling) {
> +            pci_bus_fire_intx_routing_notifier(sec);
> +        }
> +    }
> +}
> +
> +void pci_device_set_intx_routing_notifier(PCIDevice *dev,
> +                                          PCIINTxRoutingNotifier notifier)
> +{
> +    dev->intx_routing_notifier = notifier;
> +}
> +

nit, wish there was also an unset here too.  If we switch to MSI mode,
there's not much point in calling the notifier, so the driver could
unset it.  Also nice to have a set in the initfn and matching unset in
exitfn.  There's potentially a race between the driver freeing data and
the pci device going away, but I don't know if we can hit it in qemu.
Thanks,

Alex

>  /***********************************************************/
>  /* monitor info on PCI */
>  
> diff --git a/hw/pci.h b/hw/pci.h
> index 387111b..c4382a1 100644
> --- a/hw/pci.h
> +++ b/hw/pci.h
> @@ -182,6 +182,7 @@ typedef struct PCIDeviceClass {
>      const char *romfile;
>  } PCIDeviceClass;
>  
> +typedef void (*PCIINTxRoutingNotifier)(PCIDevice *dev);
>  typedef int (*MSIVectorUseNotifier)(PCIDevice *dev, unsigned int vector,
>                                        MSIMessage msg);
>  typedef void (*MSIVectorReleaseNotifier)(PCIDevice *dev, unsigned int 
> vector);
> @@ -259,6 +260,9 @@ struct PCIDevice {
>      MemoryRegion rom;
>      uint32_t rom_bar;
>  
> +    /* INTx routing notifier */
> +    PCIINTxRoutingNotifier intx_routing_notifier;
> +
>      /* MSI-X notifiers */
>      MSIVectorUseNotifier msix_vector_use_notifier;
>      MSIVectorReleaseNotifier msix_vector_release_notifier;
> @@ -319,6 +323,9 @@ PCIBus *pci_register_bus(DeviceState *parent, const char 
> *name,
>                           MemoryRegion *address_space_io,
>                           uint8_t devfn_min, int nirq);
>  PCIINTxRoute pci_device_route_intx_to_irq(PCIDevice *dev, int pin);
> +void pci_bus_fire_intx_routing_notifier(PCIBus *bus);
> +void pci_device_set_intx_routing_notifier(PCIDevice *dev,
> +                                          PCIINTxRoutingNotifier notifier);
>  void pci_device_reset(PCIDevice *dev);
>  void pci_bus_reset(PCIBus *bus);
>  
> diff --git a/hw/piix_pci.c b/hw/piix_pci.c
> index 347177f..8fd21f3 100644
> --- a/hw/piix_pci.c
> +++ b/hw/piix_pci.c
> @@ -422,6 +422,8 @@ static void piix3_write_config(PCIDevice *dev,
>      if (ranges_overlap(address, len, PIIX_PIRQC, 4)) {
>          PIIX3State *piix3 = DO_UPCAST(PIIX3State, dev, dev);
>          int pic_irq;
> +
> +        pci_bus_fire_intx_routing_notifier(piix3->dev.bus);
>          piix3_update_irq_levels(piix3);
>          for (pic_irq = 0; pic_irq < PIIX_NUM_PIC_IRQS; pic_irq++) {
>              piix3_set_irq_pic(piix3, pic_irq);