pci: hotplug bridge fixes
cover pci: hotplug bridge fixes
Hi,
While testing pci bridge hotplug via device_add, I ran into a couple of qemu segfaults.
The first one was caused by having a refcount greater than 0, in the object_delete() path. Once, I got past that error, I hit a second segfault due to the fact that pci_bridge_dev_initfn() didn't fully cleanup its state.
Thanks,
-Jason
Jason Baron (2):
qdev: release parent properties on dc->init failure
pci_bridge_dev: fix error path in pci_bridge_dev_initfn()
hw/pci_bridge_dev.c | 4 +++- hw/qdev.c | 1 + 2 files changed, 4 insertions(+), 1 deletions(-)
PATCH 1/2 qdev: release parent properties on dc->init failure
While looking into hot-plugging bridges, I can create a qemu segfault via:
$ device_add pci-bridge
Bridge chassis not specified. Each bridge is required to be assigned a unique chassis id > 0. **
ERROR:qom/object.c:389:object_delete: assertion failed: (obj->ref == 0)
I'm proposing to fix this by adding a call to 'object_unparent()', before the call to qdev_free(). I see there is already a precedent for this usage pattern as seen in qdev_simple_unplug_cb():
/* can be used as ->unplug() callback for the simple cases */ int qdev_simple_unplug_cb(DeviceState *dev)
{
/* just zap it */
object_unparent(OBJECT(dev));
qdev_free(dev);
return 0;
}--- hw/qdev.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-)
hw/qdev.c +1 -0
@@ -150,6 +150,7 @@ int qdev_init(DeviceState *dev) rc = dc->init (dev);if (rc <0 ) { +object_unparent (OBJECT (dev));qdev_free (dev);return rc; }
Signature · 1 line
1.7.1
PATCH 2/2 pci_bridge_dev: fix error path in pci_bridge_dev_initfn()
In reply to Jason BaronCurrently, we do not properly cleanup, if pci_bridge_dev_initfn fails to initialize properly. Make sure to call pci_bridge_exitfn() in the error path.
--- hw/pci_bridge_dev.c | 4 +++- 1 files changed, 3 insertions(+), 1 deletions(-)
hw/pci_bridge_dev.c +3 -1
@@ -52,7 +52,7 @@ static int pci_bridge_dev_initfn(PCIDevice *dev) {PCIBridge *br =DO_UPCAST (PCIBridge, dev, dev);PCIBridgeDev *bridge_dev =DO_UPCAST (PCIBridgeDev, bridge, br); -int err; +int err, ret;pci_bridge_map_irq (br,NULL , pci_bridge_dev_map_irq_fn); err =pci_bridge_initfn (dev);if (err) {
@@ -86,6 +86,8 @@ slotid_error:shpc_cleanup (dev, &bridge_dev->bar); shpc_error:memory_region_destroy (&bridge_dev->bar); + ret =pci_bridge_exitfn (dev); +assert (!ret); bridge_error:return err; }
Signature · 1 line
1.7.1
Michael S. Tsirkin
AppliedQuoted 5 lines · Jason Baron
Hi,
While testing pci bridge hotplug via
device_add, I ran into a couple of qemu segfaults.
Quoted 8 lines
The first one was caused by having a refcount greater than 0, in the
object_delete()path. Once, I got past that error, I hit a second segfault due to the fact thatpci_bridge_dev_initfn()didn't fully cleanup its state.Thanks,
-Jason
Applied, thanks.
Quoted 7 lines
Jason Baron (2):
qdev: release parent properties ondc->initfailure
pci_bridge_dev: fix error path inpci_bridge_dev_initfn()hw/pci_bridge_dev.c | 4 +++- hw/qdev.c | 1 + 2 files changed, 4 insertions(+), 1 deletions(-)