Skip to contentSkip to conversation

pci: deassert intx when pci device unrealize

RFC PATCH In review Herongguang (Stephen) 2 participants 3 messages Apr 24, 2017 – Apr 25, 2017

lore
Share & tools
lore
#0 lore raw

RFC PATCH 1/1 pci: deassert intx when pci device unrealize

If a pci device is not reset by VM (by writing into config space) and unplugged by VM, after that when VM reboots, qemu may assert:
pcibus_reset: Assertion `bus->irq_count[i] == 0' failed

---

Is there need to call pci_do_device_reset()?

---
 hw/pci/pci.c | 1 +
 1 file changed, 1 insertion(+)
hw/pci/pci.c +1 -0
index 259483b..afe6397 100644
@@ -1078,6 +1078,7 @@ static void pci_qdev_unrealize(DeviceState *dev, Error **errp)
 
     pci_unregister_io_regions(pci_dev);
     pci_del_option_rom(pci_dev);
+    pci_device_deassert_intx(pci_dev);
 
     if (pc->exit) {
         pc->exit(pci_dev);

--
1.7.12.4

#1 lore raw
Quoted 5 lines · Herongguang (Stephen)

If a pci device is not reset by VM (by writing into config space) and unplugged by VM, after that when VM reboots, qemu may assert:
pcibus_reset: Assertion `bus->irq_count[i] == 0' failed

Good grief, I can't believe we have had this bug for so long. Thanks a lot for finding this.

Pls Cc stable on this patch.

Quoted 3 lines

---

Is there need to call pci_do_device_reset()?

I don't think so, why?

hw/pci/pci.c static void pci_qdev_unrealize(DeviceState *dev, Error **errp) · line 1081

Quoted 13 lines
---
 hw/pci/pci.c | 1 +
 1 file changed, 1 insertion(+)
hw/pci/pci.c +1 -0
index 259483b..afe6397 100644
@@ -1078,6 +1078,7 @@ static void pci_qdev_unrealize(DeviceState *dev, Error **errp)
 
     pci_unregister_io_regions(pci_dev);
     pci_del_option_rom(pci_dev);
+    pci_device_deassert_intx(pci_dev);

I think the right place to call this is after class exit when we know no one will try to assert the interrupt.

Quoted 5 lines
hw/pci/pci.c +0 -1
     if (pc->exit) {
         pc->exit(pci_dev);
--

1.7.12.4

#2 lore raw
Quoted 28 lines · Michael S. Tsirkin
Quoted 5 lines · Herongguang (Stephen)

If a pci device is not reset by VM (by writing into config space) and unplugged by VM, after that when VM reboots, qemu may assert:
pcibus_reset: Assertion `bus->irq_count[i] == 0' failed

Good grief, I can't believe we have had this bug for so long. Thanks a lot for finding this.

Pls Cc stable on this patch.

Quoted 3 lines

---

Is there need to call pci_do_device_reset()?

I don't think so, why?

Quoted 10 lines
---
  hw/pci/pci.c | 1 +
  1 file changed, 1 insertion(+)
hw/pci/pci.c +0 -0
index 259483b..afe6397 100644
@@ -1078,6 +1078,7 @@ static void pci_qdev_unrealize(DeviceState *dev, Error **errp)
 
Quoted 6 lines

hw/pci/pci.c static void pci_qdev_unrealize(DeviceState *dev, Error **errp) · line 1081

Quoted 3 lines
hw/pci/pci.c +1 -0
      pci_unregister_io_regions(pci_dev);
      pci_del_option_rom(pci_dev);
+    pci_device_deassert_intx(pci_dev);

I think the right place to call this is after class exit when we know no one will try to assert the interrupt.

OK, it's safer, will resend a new patch.

Quoted 8 lines
Quoted 6 lines
hw/pci/pci.c +0 -1
      if (pc->exit) {
          pc->exit(pci_dev);
--

1.7.12.4

.