Message ID | 20250226210201.238489-2-mjrosato@linux.ibm.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | s390x/pci: changes to hotplug error handling | expand |
On 26/02/2025 22.02, Matthew Rosato wrote: > In the unlikely event that we must fail hotplug of a PCI passthrough > device, ensure appropriate clean up of the associated zPCI device is > performed. > > Signed-off-by: Matthew Rosato <mjrosato@linux.ibm.com> > --- > hw/s390x/s390-pci-bus.c | 21 +++++++++++++++++++-- > 1 file changed, 19 insertions(+), 2 deletions(-) > > diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c > index 913d72cc74..6cc0e7538a 100644 > --- a/hw/s390x/s390-pci-bus.c > +++ b/hw/s390x/s390-pci-bus.c > @@ -1119,7 +1119,7 @@ static void s390_pcihost_plug(HotplugHandler *hotplug_dev, DeviceState *dev, > if (rc) { > error_setg(errp, "Plug failed for zPCI device in " > "interpretation mode: %d", rc); > - return; > + goto pbdev_cleanup; > } > } else { > trace_s390_pcihost("zPCI interpretation missing"); > @@ -1150,7 +1150,7 @@ static void s390_pcihost_plug(HotplugHandler *hotplug_dev, DeviceState *dev, > if (s390_pci_msix_init(pbdev) && !pbdev->interp) { > error_setg(errp, "MSI-X support is mandatory " > "in the S390 architecture"); > - return; > + goto pbdev_cleanup; > } > > if (dev->hotplugged) { > @@ -1168,6 +1168,23 @@ static void s390_pcihost_plug(HotplugHandler *hotplug_dev, DeviceState *dev, > } else { > g_assert_not_reached(); > } > + > + return; > + > + pbdev_cleanup: > + DeviceState *bdev = DEVICE(pbdev); > + > + if (pbdev->shutdown_notifier.notify) { > + notifier_remove(&pbdev->shutdown_notifier); > + } > + if (pbdev->iommu->dma_limit) { > + s390_pci_end_dma_count(s, pbdev->iommu->dma_limit); > + } > + s390_pci_iommu_free(s, pci_get_bus(pbdev->pdev), pbdev->pdev->devfn); > + QTAILQ_REMOVE(&s->zpci_devs, pbdev, link); > + g_hash_table_remove(s->zpci_table, &pbdev->idx); > + object_unparent(OBJECT(bdev)); > + qdev_unrealize(bdev); > } Not sure, but should bdev realy be unrealized here already? Or should that rather be done by the caller (when it sees the errp set)? Thomas
diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c index 913d72cc74..6cc0e7538a 100644 --- a/hw/s390x/s390-pci-bus.c +++ b/hw/s390x/s390-pci-bus.c @@ -1119,7 +1119,7 @@ static void s390_pcihost_plug(HotplugHandler *hotplug_dev, DeviceState *dev, if (rc) { error_setg(errp, "Plug failed for zPCI device in " "interpretation mode: %d", rc); - return; + goto pbdev_cleanup; } } else { trace_s390_pcihost("zPCI interpretation missing"); @@ -1150,7 +1150,7 @@ static void s390_pcihost_plug(HotplugHandler *hotplug_dev, DeviceState *dev, if (s390_pci_msix_init(pbdev) && !pbdev->interp) { error_setg(errp, "MSI-X support is mandatory " "in the S390 architecture"); - return; + goto pbdev_cleanup; } if (dev->hotplugged) { @@ -1168,6 +1168,23 @@ static void s390_pcihost_plug(HotplugHandler *hotplug_dev, DeviceState *dev, } else { g_assert_not_reached(); } + + return; + + pbdev_cleanup: + DeviceState *bdev = DEVICE(pbdev); + + if (pbdev->shutdown_notifier.notify) { + notifier_remove(&pbdev->shutdown_notifier); + } + if (pbdev->iommu->dma_limit) { + s390_pci_end_dma_count(s, pbdev->iommu->dma_limit); + } + s390_pci_iommu_free(s, pci_get_bus(pbdev->pdev), pbdev->pdev->devfn); + QTAILQ_REMOVE(&s->zpci_devs, pbdev, link); + g_hash_table_remove(s->zpci_table, &pbdev->idx); + object_unparent(OBJECT(bdev)); + qdev_unrealize(bdev); } static void s390_pcihost_unplug(HotplugHandler *hotplug_dev, DeviceState *dev,
In the unlikely event that we must fail hotplug of a PCI passthrough device, ensure appropriate clean up of the associated zPCI device is performed. Signed-off-by: Matthew Rosato <mjrosato@linux.ibm.com> --- hw/s390x/s390-pci-bus.c | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-)