1. Patchew
  2. QEMU
  3. [PATCH v3 00/29] ACPI PCI Hotplug support on ARM
  4. View patch

[PATCH v3 26/29] hw/arm/virt: Plug pcihp hotplug/hotunplug callbacks

Eric Auger posted 29 patches 5 months ago
Maintainers: "Michael S. Tsirkin" <mst@redhat.com>, Igor Mammedov <imammedo@redhat.com>, Ani Sinha <anisinha@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>, Peter Maydell <peter.maydell@linaro.org>, Shannon Zhao <shannon.zhaosl@gmail.com>, "Daniel P. Berrangé" <berrange@redhat.com>, Eduardo Habkost <eduardo@habkost.net>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com>, Richard Henderson <richard.henderson@linaro.org>
There is a newer version of this series
[PATCH v3 26/29] hw/arm/virt: Plug pcihp hotplug/hotunplug callbacks
Posted by Eric Auger 5 months ago 
Signed-off-by: Eric Auger <eric.auger@redhat.com>

---
v2 -> v3:
- fix cohabitation with virtio-mem-pci device and tested
  hotplug/unplug of this latter (Igor)
---
 hw/arm/virt.c | 72 ++++++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 69 insertions(+), 3 deletions(-)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 8c882e0794..06b87e1050 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -1747,6 +1747,22 @@ static void virt_build_smbios(VirtMachineState *vms)
     }
 }
 
+static AcpiPciHpState *get_acpi_pcihp_state(VirtMachineState *vms)
+{
+    AcpiGedState *acpi_ged_state;
+    AcpiPciHpState *pcihp_state;
+
+    if (!vms->acpi_dev) {
+        return NULL;
+    }
+    acpi_ged_state = ACPI_GED(vms->acpi_dev);
+    pcihp_state = &acpi_ged_state->pcihp_state;
+    if (pcihp_state->use_acpi_hotplug_bridge) {
+        return pcihp_state;
+    }
+    return NULL;
+}
+
 static
 void virt_machine_done(Notifier *notifier, void *data)
 {
@@ -2907,6 +2923,13 @@ static void virt_machine_device_pre_plug_cb(HotplugHandler *hotplug_dev,
 {
     VirtMachineState *vms = VIRT_MACHINE(hotplug_dev);
 
+    if (object_dynamic_cast(OBJECT(dev), TYPE_PCI_DEVICE)) {
+        if (get_acpi_pcihp_state(vms)) {
+            acpi_pcihp_device_pre_plug_cb(HOTPLUG_HANDLER(vms->acpi_dev),
+                                          dev, errp);
+        }
+    }
+
     if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) {
         virt_memory_pre_plug(hotplug_dev, dev, errp);
     } else if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MD_PCI)) {
@@ -2961,6 +2984,15 @@ static void virt_machine_device_plug_cb(HotplugHandler *hotplug_dev,
         }
     }
 
+    if (object_dynamic_cast(OBJECT(dev), TYPE_PCI_DEVICE)) {
+        AcpiPciHpState *pcihp_state = get_acpi_pcihp_state(vms);
+
+        if (pcihp_state) {
+            acpi_pcihp_device_plug_cb(HOTPLUG_HANDLER(vms->acpi_dev),
+                                      pcihp_state, dev, errp);
+        }
+    }
+
     if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) {
         virt_memory_plug(hotplug_dev, dev, errp);
     } else if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MD_PCI)) {
@@ -3017,12 +3049,27 @@ out:
 static void virt_machine_device_unplug_request_cb(HotplugHandler *hotplug_dev,
                                           DeviceState *dev, Error **errp)
 {
+    bool supported = false;
+
     if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) {
         virt_dimm_unplug_request(hotplug_dev, dev, errp);
+        supported = true;
     } else if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MD_PCI)) {
         virtio_md_pci_unplug_request(VIRTIO_MD_PCI(dev), MACHINE(hotplug_dev),
                                      errp);
-    } else {
+        supported = true;
+    }
+    if (object_dynamic_cast(OBJECT(dev), TYPE_PCI_DEVICE)) {
+        VirtMachineState *vms = VIRT_MACHINE(hotplug_dev);
+        AcpiPciHpState *pcihp_state = get_acpi_pcihp_state(vms);
+
+        if (pcihp_state) {
+            acpi_pcihp_device_unplug_request_cb(HOTPLUG_HANDLER(vms->acpi_dev),
+                                                pcihp_state, dev, errp);
+            supported = true;
+        }
+    }
+    if (!supported) {
         error_setg(errp, "device unplug request for unsupported device"
                    " type: %s", object_get_typename(OBJECT(dev)));
     }
@@ -3031,11 +3078,27 @@ static void virt_machine_device_unplug_request_cb(HotplugHandler *hotplug_dev,
 static void virt_machine_device_unplug_cb(HotplugHandler *hotplug_dev,
                                           DeviceState *dev, Error **errp)
 {
+    bool supported = false;
+
     if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) {
         virt_dimm_unplug(hotplug_dev, dev, errp);
+        supported = true;
     } else if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MD_PCI)) {
         virtio_md_pci_unplug(VIRTIO_MD_PCI(dev), MACHINE(hotplug_dev), errp);
-    } else {
+        supported = true;
+    }
+    if (object_dynamic_cast(OBJECT(dev), TYPE_PCI_DEVICE)) {
+        VirtMachineState *vms = VIRT_MACHINE(hotplug_dev);
+
+        AcpiPciHpState *pcihp_state = get_acpi_pcihp_state(vms);
+
+        if (pcihp_state) {
+            acpi_pcihp_device_unplug_cb(HOTPLUG_HANDLER(vms->acpi_dev),
+                                        pcihp_state, dev, errp);
+            supported = true;
+        }
+    }
+    if (!supported) {
         error_setg(errp, "virt: device unplug for unsupported device"
                    " type: %s", object_get_typename(OBJECT(dev)));
     }
@@ -3045,11 +3108,14 @@ static HotplugHandler *virt_machine_get_hotplug_handler(MachineState *machine,
                                                         DeviceState *dev)
 {
     MachineClass *mc = MACHINE_GET_CLASS(machine);
+    VirtMachineState *vms = VIRT_MACHINE(machine);
 
     if (device_is_dynamic_sysbus(mc, dev) ||
         object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM) ||
         object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MD_PCI) ||
-        object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_IOMMU_PCI)) {
+        object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_IOMMU_PCI) ||
+        (object_dynamic_cast(OBJECT(dev), TYPE_PCI_DEVICE) &&
+                             get_acpi_pcihp_state(vms))) {
         return HOTPLUG_HANDLER(machine);
     }
     return NULL;
-- 
2.49.0
Re: [PATCH v3 26/29] hw/arm/virt: Plug pcihp hotplug/hotunplug callbacks
Posted by Igor Mammedov 4 months, 4 weeks ago 
On Mon, 16 Jun 2025 11:46:55 +0200
Eric Auger <eric.auger@redhat.com> wrote:

> Signed-off-by: Eric Auger <eric.auger@redhat.com>

why do we still need this?

pcihp code should override native pcie bus handlers, and then
when device_add calls bus hotplug handlers it will be pcihp ones.

> ---
> v2 -> v3:
> - fix cohabitation with virtio-mem-pci device and tested
>   hotplug/unplug of this latter (Igor)
> ---
>  hw/arm/virt.c | 72 ++++++++++++++++++++++++++++++++++++++++++++++++---
>  1 file changed, 69 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index 8c882e0794..06b87e1050 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -1747,6 +1747,22 @@ static void virt_build_smbios(VirtMachineState *vms)
>      }
>  }
>  
> +static AcpiPciHpState *get_acpi_pcihp_state(VirtMachineState *vms)
> +{
> +    AcpiGedState *acpi_ged_state;
> +    AcpiPciHpState *pcihp_state;
> +
> +    if (!vms->acpi_dev) {
> +        return NULL;
> +    }
> +    acpi_ged_state = ACPI_GED(vms->acpi_dev);
> +    pcihp_state = &acpi_ged_state->pcihp_state;
> +    if (pcihp_state->use_acpi_hotplug_bridge) {
> +        return pcihp_state;
> +    }
> +    return NULL;
> +}
> +
>  static
>  void virt_machine_done(Notifier *notifier, void *data)
>  {
> @@ -2907,6 +2923,13 @@ static void virt_machine_device_pre_plug_cb(HotplugHandler *hotplug_dev,
>  {
>      VirtMachineState *vms = VIRT_MACHINE(hotplug_dev);
>  
> +    if (object_dynamic_cast(OBJECT(dev), TYPE_PCI_DEVICE)) {
> +        if (get_acpi_pcihp_state(vms)) {
> +            acpi_pcihp_device_pre_plug_cb(HOTPLUG_HANDLER(vms->acpi_dev),
> +                                          dev, errp);
> +        }
> +    }
> +
>      if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) {
>          virt_memory_pre_plug(hotplug_dev, dev, errp);
>      } else if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MD_PCI)) {
> @@ -2961,6 +2984,15 @@ static void virt_machine_device_plug_cb(HotplugHandler *hotplug_dev,
>          }
>      }
>  
> +    if (object_dynamic_cast(OBJECT(dev), TYPE_PCI_DEVICE)) {
> +        AcpiPciHpState *pcihp_state = get_acpi_pcihp_state(vms);
> +
> +        if (pcihp_state) {
> +            acpi_pcihp_device_plug_cb(HOTPLUG_HANDLER(vms->acpi_dev),
> +                                      pcihp_state, dev, errp);
> +        }
> +    }
> +
>      if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) {
>          virt_memory_plug(hotplug_dev, dev, errp);
>      } else if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MD_PCI)) {
> @@ -3017,12 +3049,27 @@ out:
>  static void virt_machine_device_unplug_request_cb(HotplugHandler *hotplug_dev,
>                                            DeviceState *dev, Error **errp)
>  {
> +    bool supported = false;
> +
>      if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) {
>          virt_dimm_unplug_request(hotplug_dev, dev, errp);
> +        supported = true;
>      } else if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MD_PCI)) {
>          virtio_md_pci_unplug_request(VIRTIO_MD_PCI(dev), MACHINE(hotplug_dev),
>                                       errp);
> -    } else {
> +        supported = true;
> +    }
> +    if (object_dynamic_cast(OBJECT(dev), TYPE_PCI_DEVICE)) {
> +        VirtMachineState *vms = VIRT_MACHINE(hotplug_dev);
> +        AcpiPciHpState *pcihp_state = get_acpi_pcihp_state(vms);
> +
> +        if (pcihp_state) {
> +            acpi_pcihp_device_unplug_request_cb(HOTPLUG_HANDLER(vms->acpi_dev),
> +                                                pcihp_state, dev, errp);
> +            supported = true;
> +        }
> +    }
> +    if (!supported) {
>          error_setg(errp, "device unplug request for unsupported device"
>                     " type: %s", object_get_typename(OBJECT(dev)));
>      }
> @@ -3031,11 +3078,27 @@ static void virt_machine_device_unplug_request_cb(HotplugHandler *hotplug_dev,
>  static void virt_machine_device_unplug_cb(HotplugHandler *hotplug_dev,
>                                            DeviceState *dev, Error **errp)
>  {
> +    bool supported = false;
> +
>      if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) {
>          virt_dimm_unplug(hotplug_dev, dev, errp);
> +        supported = true;
>      } else if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MD_PCI)) {
>          virtio_md_pci_unplug(VIRTIO_MD_PCI(dev), MACHINE(hotplug_dev), errp);
> -    } else {
> +        supported = true;
> +    }
> +    if (object_dynamic_cast(OBJECT(dev), TYPE_PCI_DEVICE)) {
> +        VirtMachineState *vms = VIRT_MACHINE(hotplug_dev);
> +
> +        AcpiPciHpState *pcihp_state = get_acpi_pcihp_state(vms);
> +
> +        if (pcihp_state) {
> +            acpi_pcihp_device_unplug_cb(HOTPLUG_HANDLER(vms->acpi_dev),
> +                                        pcihp_state, dev, errp);
> +            supported = true;
> +        }
> +    }
> +    if (!supported) {
>          error_setg(errp, "virt: device unplug for unsupported device"
>                     " type: %s", object_get_typename(OBJECT(dev)));
>      }
> @@ -3045,11 +3108,14 @@ static HotplugHandler *virt_machine_get_hotplug_handler(MachineState *machine,
>                                                          DeviceState *dev)
>  {
>      MachineClass *mc = MACHINE_GET_CLASS(machine);
> +    VirtMachineState *vms = VIRT_MACHINE(machine);
>  
>      if (device_is_dynamic_sysbus(mc, dev) ||
>          object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM) ||
>          object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MD_PCI) ||
> -        object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_IOMMU_PCI)) {
> +        object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_IOMMU_PCI) ||
> +        (object_dynamic_cast(OBJECT(dev), TYPE_PCI_DEVICE) &&
> +                             get_acpi_pcihp_state(vms))) {
>          return HOTPLUG_HANDLER(machine);
>      }
>      return NULL;
Re: [PATCH v3 26/29] hw/arm/virt: Plug pcihp hotplug/hotunplug callbacks
Posted by Eric Auger 4 months, 3 weeks ago 
Hi Igor,

On 6/20/25 3:10 PM, Igor Mammedov wrote:
> On Mon, 16 Jun 2025 11:46:55 +0200
> Eric Auger <eric.auger@redhat.com> wrote:
>
>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
> why do we still need this?
>
> pcihp code should override native pcie bus handlers, and then
> when device_add calls bus hotplug handlers it will be pcihp ones.
It was needed because I did not call qbus_set_hotplug_handler() as done in
ich9.c. So I think I should be able to get rid of this patch

Thanks

Eric
>
>> ---
>> v2 -> v3:
>> - fix cohabitation with virtio-mem-pci device and tested
>>   hotplug/unplug of this latter (Igor)
>> ---
>>  hw/arm/virt.c | 72 ++++++++++++++++++++++++++++++++++++++++++++++++---
>>  1 file changed, 69 insertions(+), 3 deletions(-)
>>
>> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
>> index 8c882e0794..06b87e1050 100644
>> --- a/hw/arm/virt.c
>> +++ b/hw/arm/virt.c
>> @@ -1747,6 +1747,22 @@ static void virt_build_smbios(VirtMachineState *vms)
>>      }
>>  }
>>  
>> +static AcpiPciHpState *get_acpi_pcihp_state(VirtMachineState *vms)
>> +{
>> +    AcpiGedState *acpi_ged_state;
>> +    AcpiPciHpState *pcihp_state;
>> +
>> +    if (!vms->acpi_dev) {
>> +        return NULL;
>> +    }
>> +    acpi_ged_state = ACPI_GED(vms->acpi_dev);
>> +    pcihp_state = &acpi_ged_state->pcihp_state;
>> +    if (pcihp_state->use_acpi_hotplug_bridge) {
>> +        return pcihp_state;
>> +    }
>> +    return NULL;
>> +}
>> +
>>  static
>>  void virt_machine_done(Notifier *notifier, void *data)
>>  {
>> @@ -2907,6 +2923,13 @@ static void virt_machine_device_pre_plug_cb(HotplugHandler *hotplug_dev,
>>  {
>>      VirtMachineState *vms = VIRT_MACHINE(hotplug_dev);
>>  
>> +    if (object_dynamic_cast(OBJECT(dev), TYPE_PCI_DEVICE)) {
>> +        if (get_acpi_pcihp_state(vms)) {
>> +            acpi_pcihp_device_pre_plug_cb(HOTPLUG_HANDLER(vms->acpi_dev),
>> +                                          dev, errp);
>> +        }
>> +    }
>> +
>>      if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) {
>>          virt_memory_pre_plug(hotplug_dev, dev, errp);
>>      } else if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MD_PCI)) {
>> @@ -2961,6 +2984,15 @@ static void virt_machine_device_plug_cb(HotplugHandler *hotplug_dev,
>>          }
>>      }
>>  
>> +    if (object_dynamic_cast(OBJECT(dev), TYPE_PCI_DEVICE)) {
>> +        AcpiPciHpState *pcihp_state = get_acpi_pcihp_state(vms);
>> +
>> +        if (pcihp_state) {
>> +            acpi_pcihp_device_plug_cb(HOTPLUG_HANDLER(vms->acpi_dev),
>> +                                      pcihp_state, dev, errp);
>> +        }
>> +    }
>> +
>>      if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) {
>>          virt_memory_plug(hotplug_dev, dev, errp);
>>      } else if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MD_PCI)) {
>> @@ -3017,12 +3049,27 @@ out:
>>  static void virt_machine_device_unplug_request_cb(HotplugHandler *hotplug_dev,
>>                                            DeviceState *dev, Error **errp)
>>  {
>> +    bool supported = false;
>> +
>>      if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) {
>>          virt_dimm_unplug_request(hotplug_dev, dev, errp);
>> +        supported = true;
>>      } else if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MD_PCI)) {
>>          virtio_md_pci_unplug_request(VIRTIO_MD_PCI(dev), MACHINE(hotplug_dev),
>>                                       errp);
>> -    } else {
>> +        supported = true;
>> +    }
>> +    if (object_dynamic_cast(OBJECT(dev), TYPE_PCI_DEVICE)) {
>> +        VirtMachineState *vms = VIRT_MACHINE(hotplug_dev);
>> +        AcpiPciHpState *pcihp_state = get_acpi_pcihp_state(vms);
>> +
>> +        if (pcihp_state) {
>> +            acpi_pcihp_device_unplug_request_cb(HOTPLUG_HANDLER(vms->acpi_dev),
>> +                                                pcihp_state, dev, errp);
>> +            supported = true;
>> +        }
>> +    }
>> +    if (!supported) {
>>          error_setg(errp, "device unplug request for unsupported device"
>>                     " type: %s", object_get_typename(OBJECT(dev)));
>>      }
>> @@ -3031,11 +3078,27 @@ static void virt_machine_device_unplug_request_cb(HotplugHandler *hotplug_dev,
>>  static void virt_machine_device_unplug_cb(HotplugHandler *hotplug_dev,
>>                                            DeviceState *dev, Error **errp)
>>  {
>> +    bool supported = false;
>> +
>>      if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) {
>>          virt_dimm_unplug(hotplug_dev, dev, errp);
>> +        supported = true;
>>      } else if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MD_PCI)) {
>>          virtio_md_pci_unplug(VIRTIO_MD_PCI(dev), MACHINE(hotplug_dev), errp);
>> -    } else {
>> +        supported = true;
>> +    }
>> +    if (object_dynamic_cast(OBJECT(dev), TYPE_PCI_DEVICE)) {
>> +        VirtMachineState *vms = VIRT_MACHINE(hotplug_dev);
>> +
>> +        AcpiPciHpState *pcihp_state = get_acpi_pcihp_state(vms);
>> +
>> +        if (pcihp_state) {
>> +            acpi_pcihp_device_unplug_cb(HOTPLUG_HANDLER(vms->acpi_dev),
>> +                                        pcihp_state, dev, errp);
>> +            supported = true;
>> +        }
>> +    }
>> +    if (!supported) {
>>          error_setg(errp, "virt: device unplug for unsupported device"
>>                     " type: %s", object_get_typename(OBJECT(dev)));
>>      }
>> @@ -3045,11 +3108,14 @@ static HotplugHandler *virt_machine_get_hotplug_handler(MachineState *machine,
>>                                                          DeviceState *dev)
>>  {
>>      MachineClass *mc = MACHINE_GET_CLASS(machine);
>> +    VirtMachineState *vms = VIRT_MACHINE(machine);
>>  
>>      if (device_is_dynamic_sysbus(mc, dev) ||
>>          object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM) ||
>>          object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MD_PCI) ||
>> -        object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_IOMMU_PCI)) {
>> +        object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_IOMMU_PCI) ||
>> +        (object_dynamic_cast(OBJECT(dev), TYPE_PCI_DEVICE) &&
>> +                             get_acpi_pcihp_state(vms))) {
>>          return HOTPLUG_HANDLER(machine);
>>      }
>>      return NULL;
Re: [PATCH v3 26/29] hw/arm/virt: Plug pcihp hotplug/hotunplug callbacks
Posted by Jonathan Cameron via 4 months, 4 weeks ago 
On Mon, 16 Jun 2025 11:46:55 +0200
Eric Auger <eric.auger@redhat.com> wrote:

Maybe needs a little bit of description.

> Signed-off-by: Eric Auger <eric.auger@redhat.com>

A few trivial things inline.
Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com>

> 
> ---
> v2 -> v3:
> - fix cohabitation with virtio-mem-pci device and tested
>   hotplug/unplug of this latter (Igor)
> ---
>  hw/arm/virt.c | 72 ++++++++++++++++++++++++++++++++++++++++++++++++---
>  1 file changed, 69 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index 8c882e0794..06b87e1050 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -1747,6 +1747,22 @@ static void virt_build_smbios(VirtMachineState *vms)
>      }
>  }
>  
> +static AcpiPciHpState *get_acpi_pcihp_state(VirtMachineState *vms)
> +{
> +    AcpiGedState *acpi_ged_state;
> +    AcpiPciHpState *pcihp_state;
> +
> +    if (!vms->acpi_dev) {
> +        return NULL;
> +    }
> +    acpi_ged_state = ACPI_GED(vms->acpi_dev);
> +    pcihp_state = &acpi_ged_state->pcihp_state;
> +    if (pcihp_state->use_acpi_hotplug_bridge) {
> +        return pcihp_state;
> +    }
> +    return NULL;

Really trivial but if this isn't going to gain extra stuff I'd consider
the NULL return the 'error' path and prefer that out of line for consistency.
That is.

    if(!pcihp_state->use_acpi_hotplug_bridge) {
        return NULL;
    }

    return pcihp_state;


That just ends up a tiny bit more consistent with the !vms->acpi_dev test
earlier in the function.

> +}
> +
>  static
>  void virt_machine_done(Notifier *notifier, void *data)
>  {
> @@ -2907,6 +2923,13 @@ static void virt_machine_device_pre_plug_cb(HotplugHandler *hotplug_dev,
>  {
>      VirtMachineState *vms = VIRT_MACHINE(hotplug_dev);
>  

Maybe add a comment on why this is not in the if/else stack.  I assume
because a few of those are also TYPE_PCI_DEVICE.
Something in the patch description would also work for me.

> +    if (object_dynamic_cast(OBJECT(dev), TYPE_PCI_DEVICE)) {
> +        if (get_acpi_pcihp_state(vms)) {
> +            acpi_pcihp_device_pre_plug_cb(HOTPLUG_HANDLER(vms->acpi_dev),
> +                                          dev, errp);
> +        }
> +    }
> +
>      if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) {
>          virt_memory_pre_plug(hotplug_dev, dev, errp);
>      } else if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MD_PCI)) {
> @@ -2961,6 +2984,15 @@ static void virt_machine_device_plug_cb(HotplugHandler *hotplug_dev,
>          }
>      }
>  
> +    if (object_dynamic_cast(OBJECT(dev), TYPE_PCI_DEVICE)) {
> +        AcpiPciHpState *pcihp_state = get_acpi_pcihp_state(vms);
> +
> +        if (pcihp_state) {
> +            acpi_pcihp_device_plug_cb(HOTPLUG_HANDLER(vms->acpi_dev),
> +                                      pcihp_state, dev, errp);
> +        }
> +    }
> +
>      if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) {
>          virt_memory_plug(hotplug_dev, dev, errp);
>      } else if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MD_PCI)) {
> @@ -3017,12 +3049,27 @@ out:
>  static void virt_machine_device_unplug_request_cb(HotplugHandler *hotplug_dev,
>                                            DeviceState *dev, Error **errp)
>  {
> +    bool supported = false;
> +
>      if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) {
>          virt_dimm_unplug_request(hotplug_dev, dev, errp);
> +        supported = true;
>      } else if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MD_PCI)) {
>          virtio_md_pci_unplug_request(VIRTIO_MD_PCI(dev), MACHINE(hotplug_dev),
>                                       errp);
> -    } else {
> +        supported = true;
> +    }
> +    if (object_dynamic_cast(OBJECT(dev), TYPE_PCI_DEVICE)) {
> +        VirtMachineState *vms = VIRT_MACHINE(hotplug_dev);
> +        AcpiPciHpState *pcihp_state = get_acpi_pcihp_state(vms);
> +
> +        if (pcihp_state) {
> +            acpi_pcihp_device_unplug_request_cb(HOTPLUG_HANDLER(vms->acpi_dev),
> +                                                pcihp_state, dev, errp);
> +            supported = true;
> +        }
> +    }
> +    if (!supported) {
>          error_setg(errp, "device unplug request for unsupported device"
>                     " type: %s", object_get_typename(OBJECT(dev)));
>      }
> @@ -3031,11 +3078,27 @@ static void virt_machine_device_unplug_request_cb(HotplugHandler *hotplug_dev,
>  static void virt_machine_device_unplug_cb(HotplugHandler *hotplug_dev,
>                                            DeviceState *dev, Error **errp)
>  {
> +    bool supported = false;
> +
>      if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) {
>          virt_dimm_unplug(hotplug_dev, dev, errp);
> +        supported = true;
>      } else if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MD_PCI)) {
>          virtio_md_pci_unplug(VIRTIO_MD_PCI(dev), MACHINE(hotplug_dev), errp);
> -    } else {
> +        supported = true;
> +    }
> +    if (object_dynamic_cast(OBJECT(dev), TYPE_PCI_DEVICE)) {
> +        VirtMachineState *vms = VIRT_MACHINE(hotplug_dev);
> +
> +        AcpiPciHpState *pcihp_state = get_acpi_pcihp_state(vms);
> +
> +        if (pcihp_state) {
> +            acpi_pcihp_device_unplug_cb(HOTPLUG_HANDLER(vms->acpi_dev),
> +                                        pcihp_state, dev, errp);
> +            supported = true;
> +        }
> +    }
> +    if (!supported) {
>          error_setg(errp, "virt: device unplug for unsupported device"
>                     " type: %s", object_get_typename(OBJECT(dev)));
>      }

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.