[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 2/2] pci: Add INTx routing notifier
From: |
Alex Williamson |
Subject: |
Re: [Qemu-devel] [PATCH v2 2/2] pci: Add INTx routing notifier |
Date: |
Mon, 02 Jul 2012 08:49:14 -0600 |
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 <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.
>
> 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 <address@hidden>
> > 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