diff mbox series

[1/2] s390x/pci: fix cleanup of failed hotplug

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

Commit Message

Matthew Rosato Feb. 26, 2025, 9:02 p.m. UTC
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(-)

Comments

Thomas Huth March 5, 2025, 2:35 p.m. UTC | #1
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 mbox series

Patch

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,