pci: Don't call pci_irq_handler() for a negative intx
PATCH 1/1 pci: Don't call pci_irq_handler() for a negative intx
Under some circumstances, pci_intx() can return -1 (when the interrupt pin in the config space is 0 which normally means no interrupt).
I have seen cases of pci_set_irq() being called on such devices, in turn causing pci_irq_handler() to be called with "-1" as an argument which doesn't seem like a terribly good idea.
--- hw/pci/pci.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
hw/pci/pci.c +3 -1
@@ -1281,7 +1281,9 @@ qemu_irq pci_allocate_irq(PCIDevice *pci_dev)void pci_set_irq (PCIDevice *pci_dev,int level) {int intx =pci_intx (pci_dev); -pci_irq_handler (pci_dev, intx, level); +if (intx >=0 ) { +pci_irq_handler (pci_dev, intx, level); + } }/* Special hooks used by device assignment */
Quoted 8 lines · Benjamin Herrenschmidt
Under some circumstances,
pci_intx()can return -1 (when the interrupt pin in the config space is 0 which normally means no interrupt).I have seen cases of
pci_set_irq()being called on such devices, in turn causingpci_irq_handler()to be called with "-1" as an argument which doesn't seem like a terribly good idea.Signed-off-by Benjamin Herrenschmidt <[email protected]>
Isn't this a device bug though?
I did a grep over all callers of pci_set_irq and didn't find any that fails to set an interrupt pin.
So how about an assert instead?
And maybe stick it in pci_intx to make sure all callers get checked.
Quoted 20 lines
--- hw/pci/pci.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)hw/pci/pci.c +3
-1@@ -1281,7 +1281,9 @@ qemu_irq pci_allocate_irq(PCIDevice *pci_dev)void pci_set_irq (PCIDevice *pci_dev,int level) {int intx =pci_intx (pci_dev); -pci_irq_handler (pci_dev, intx, level); +if (intx >=0 ) { +pci_irq_handler (pci_dev, intx, level); + } }/* Special hooks used by device assignment */
Quoted 11 lines · Michael S. Tsirkin
Quoted 8 lines · Benjamin Herrenschmidt
Under some circumstances,
pci_intx()can return -1 (when the interrupt pin in the config space is 0 which normally means no interrupt).I have seen cases of
pci_set_irq()being called on such devices, in turn causingpci_irq_handler()to be called with "-1" as an argument which doesn't seem like a terribly good idea.Signed-off-by Benjamin Herrenschmidt <[email protected]>Isn't this a device bug though?
Possibly, I can try to dig a bit more see if I can reproduce and find out who is causing it.
Quoted 7 lines
I did a grep over all callers of
pci_set_irqand didn't find any that fails to set an interrupt pin.So how about an assert instead?
And maybe stick it in
pci_intxto make sure all callers get checked.
Ok, It's also possible that this doesn't happen anymore, I've carried that patch for monthes and rebased several times on top of newer qemu's.
I *think* it might have been something that happens due to some generic code initializations, something like pci_update_irq_disabled() in pci_default_write_config()... I'll dbl check.
Cheers,
Ben.