| Message ID | 20121017221215.1997.24717.stgit@bling.home |
|---|---|
| State | New |
| Headers | show |
On 2012-10-18 00:13, Alex Williamson wrote: > Rather than assert, simply return PCI_INTX_DISABLED when we don't > have a pci_route_irq_fn. PIIX already returns DISABLED for an > invalid pin, so users already deal with this state. Users of this > interface should only be acting on an ENABLED or INVERTED return > value (though we really have no support for INVERTED). Also > complain loudly when we hit this so we don't forget it's missing. > > Signed-off-by: Alex Williamson <alex.williamson@redhat.com> > --- > > v2: Turn up the annoyance factor for hitting this > > hw/pci.c | 8 +++++++- > 1 file changed, 7 insertions(+), 1 deletion(-) > > diff --git a/hw/pci.c b/hw/pci.c > index 83d262a..6a66b32 100644 > --- a/hw/pci.c > +++ b/hw/pci.c > @@ -1094,7 +1094,13 @@ PCIINTxRoute pci_device_route_intx_to_irq(PCIDevice *dev, int pin) > pin = bus->map_irq(dev, pin); > dev = bus->parent_dev; > } while (dev); > - assert(bus->route_intx_to_irq); > + > + if (!bus->route_intx_to_irq) { > + error_report("PCI: Bug - unimplemented PCI INTx routing (%s)\n", > + object_get_typename(OBJECT(bus->qbus.parent))); > + return (PCIINTxRoute) { PCI_INTX_DISABLED, -1 }; > + } > + > return bus->route_intx_to_irq(bus->irq_opaque, pin); > } > > I'm fine with this. I also see this as dead code in x86 (any x86 chipset will support this API, for sure), but maybe it helps on other archs. So: Acked-by: Jan Kiszka <jan.kiszka@siemens.com> Jan
On Wed, Oct 17, 2012 at 04:13:12PM -0600, Alex Williamson wrote: > Rather than assert, simply return PCI_INTX_DISABLED when we don't > have a pci_route_irq_fn. PIIX already returns DISABLED for an > invalid pin, so users already deal with this state. Users of this > interface should only be acting on an ENABLED or INVERTED return > value (though we really have no support for INVERTED). Also > complain loudly when we hit this so we don't forget it's missing. > > Signed-off-by: Alex Williamson <alex.williamson@redhat.com> > --- Thanks, applied. > v2: Turn up the annoyance factor for hitting this > > hw/pci.c | 8 +++++++- > 1 file changed, 7 insertions(+), 1 deletion(-) > > diff --git a/hw/pci.c b/hw/pci.c > index 83d262a..6a66b32 100644 > --- a/hw/pci.c > +++ b/hw/pci.c > @@ -1094,7 +1094,13 @@ PCIINTxRoute pci_device_route_intx_to_irq(PCIDevice *dev, int pin) > pin = bus->map_irq(dev, pin); > dev = bus->parent_dev; > } while (dev); > - assert(bus->route_intx_to_irq); > + > + if (!bus->route_intx_to_irq) { > + error_report("PCI: Bug - unimplemented PCI INTx routing (%s)\n", > + object_get_typename(OBJECT(bus->qbus.parent))); > + return (PCIINTxRoute) { PCI_INTX_DISABLED, -1 }; > + } > + > return bus->route_intx_to_irq(bus->irq_opaque, pin); > } >
diff --git a/hw/pci.c b/hw/pci.c index 83d262a..6a66b32 100644 --- a/hw/pci.c +++ b/hw/pci.c @@ -1094,7 +1094,13 @@ PCIINTxRoute pci_device_route_intx_to_irq(PCIDevice *dev, int pin) pin = bus->map_irq(dev, pin); dev = bus->parent_dev; } while (dev); - assert(bus->route_intx_to_irq); + + if (!bus->route_intx_to_irq) { + error_report("PCI: Bug - unimplemented PCI INTx routing (%s)\n", + object_get_typename(OBJECT(bus->qbus.parent))); + return (PCIINTxRoute) { PCI_INTX_DISABLED, -1 }; + } + return bus->route_intx_to_irq(bus->irq_opaque, pin); }
Rather than assert, simply return PCI_INTX_DISABLED when we don't have a pci_route_irq_fn. PIIX already returns DISABLED for an invalid pin, so users already deal with this state. Users of this interface should only be acting on an ENABLED or INVERTED return value (though we really have no support for INVERTED). Also complain loudly when we hit this so we don't forget it's missing. Signed-off-by: Alex Williamson <alex.williamson@redhat.com> --- v2: Turn up the annoyance factor for hitting this hw/pci.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)