| Message ID | 0de594d38ac7479645bb71547665ebf64418c9ce.1341232709.git.jan.kiszka@siemens.com |
|---|---|
| State | New |
| Headers | show |
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 <jan.kiszka@siemens.com> > --- > 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);
On 2012-07-02 16:26, Alex Williamson wrote: > 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 <jan.kiszka@siemens.com> >> --- >> 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. We could add static inline pci_device_unset_intx_routing_notifier(PCIDevice *dev) { dev->intx_routing_notifier = NULL; } - or simply do pci_device_set_intx_routing_notifier(dev, NULL);. > 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. Everything should be synchronized by the BQL, so far. Or not? Jan
On Mon, 2012-07-02 at 16:31 +0200, Jan Kiszka wrote: > On 2012-07-02 16:26, Alex Williamson wrote: > > 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 <jan.kiszka@siemens.com> > >> --- > >> 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. > > We could add > > static inline pci_device_unset_intx_routing_notifier(PCIDevice *dev) > { > dev->intx_routing_notifier = NULL; > } > > - or simply do pci_device_set_intx_routing_notifier(dev, NULL);. Duh... sorry, too early yet for me ;) This is fine. Acked-by: Alex Williamson <alex.williamson@redhat.com> > > 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. > > Everything should be synchronized by the BQL, so far. Or not? Yes, I imagine the lock protects getting called in that gap. Thanks, Alex
On Mon, Jul 02, 2012 at 02:38:47PM +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 <jan.kiszka@siemens.com> Thanks, applied. > --- > 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; > +} > + > /***********************************************************/ > /* 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); > -- > 1.7.3.4
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; +} + /***********************************************************/ /* 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);
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 <jan.kiszka@siemens.com> --- hw/pci.c | 23 +++++++++++++++++++++++ hw/pci.h | 7 +++++++ hw/piix_pci.c | 2 ++ 3 files changed, 32 insertions(+), 0 deletions(-)