Hi Igor,
On 6/20/25 3:10 PM, Igor Mammedov wrote:
> On Mon, 16 Jun 2025 11:46:55 +0200
> Eric Auger <eric.au...@redhat.com> wrote:
>
>> Signed-off-by: Eric Auger <eric.au...@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;
