[Qemu-devel] [PATCH] pci: remove pci_del_option_rom()

[Cédric Le Goater]

PCI devices needing a ROM allocate an optional MemoryRegion with
pci_add_option_rom(). pci_del_option_rom() does the cleanup when the
device is destroyed. The only action taken by this routine is to call
vmstate_unregister_ram() which clears the id string of the optional
ROM RAMBlock and now, also flags the RAMBlock as non-migratable. This
was recently added by commit b895de502717 ("migration: discard
non-migratable RAMBlocks"), .

VFIO devices do their own allocation of the PCI ROM region. It is
initialized in vfio_pci_size_rom() in which the PCI attribute
'has_rom' is set to true but the RAMBlock of the ROM region is not
allocated. When the associated PCI device is deleted,
pci_del_option_rom() calls vmstate_unregister_ram() which tries to
flag a NULL RAMBlock because 'has_rom' is set, leading to a SEGV .

The use of vmstate_unregister_ram() in the PCI device was added in
commit b0e56e0b63f350691b52d3e75e89bb64143fbeff ("unset RAMBlock idstr
when unregister MemoryRegion") and from the archive in
http://lists.gnu.org/archive/html/qemu-devel/2014-04/msg00282.html, it
seems that it was trying to fix a reference count issue.

vmstate_unregister_ram() being a work around, let's remove it to fix
the current SEGV issue and let's try to find a fix for the initial ref
count issue if we can reproduce.

Signed-off-by: Cédric Le Goater <address@hidden>
---
 hw/pci/pci.c | 11 -----------
 1 file changed, 11 deletions(-)

diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index 80bc45930dee..78bf74e19f22 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -191,7 +191,6 @@ static PCIBus *pci_find_bus_nr(PCIBus *bus, int bus_num);
 static void pci_update_mappings(PCIDevice *d);
 static void pci_irq_handler(void *opaque, int irq_num, int level);
 static void pci_add_option_rom(PCIDevice *pdev, bool is_default_rom, Error **);
-static void pci_del_option_rom(PCIDevice *pdev);
 
 static uint16_t pci_default_sub_vendor_id = PCI_SUBVENDOR_ID_REDHAT_QUMRANET;
 static uint16_t pci_default_sub_device_id = PCI_SUBDEVICE_ID_QEMU;
@@ -1096,7 +1095,6 @@ static void pci_qdev_unrealize(DeviceState *dev, Error 
**errp)
     PCIDeviceClass *pc = PCI_DEVICE_GET_CLASS(pci_dev);
 
     pci_unregister_io_regions(pci_dev);
-    pci_del_option_rom(pci_dev);
 
     if (pc->exit) {
         pc->exit(pci_dev);
@@ -2262,15 +2260,6 @@ static void pci_add_option_rom(PCIDevice *pdev, bool 
is_default_rom,
     pci_register_bar(pdev, PCI_ROM_SLOT, 0, &pdev->rom);
 }
 
-static void pci_del_option_rom(PCIDevice *pdev)
-{
-    if (!pdev->has_rom)
-        return;
-
-    vmstate_unregister_ram(&pdev->rom, &pdev->qdev);
-    pdev->has_rom = false;
-}
-
 /*
  * On success, pci_add_capability() returns a positive value
  * that the offset of the pci capability.
-- 
2.13.6