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

[PATCH v3 25/29] hw/arm/virt: Let virt support pci hotplug/unplug GED event

Eric Auger posted 29 patches 6 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 25/29] hw/arm/virt: Let virt support pci hotplug/unplug GED event
Posted by Eric Auger 6 months ago 
Set up the IO registers used to communicate between QEMU
and ACPI.

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

---
v2 -> v3:
- remove acpi_ged_state->pcihp_state.use_acpi_hotplug_bridge = true;
- use sysbus_mmio_map_name for all regs (Igor)
- create_pcie left at its original place

v1 -> v2:
- use ACPI_PCIHP_REGION_NAME
---
 include/hw/acpi/generic_event_device.h |  1 +
 hw/arm/virt.c                          | 19 +++++++++++++++++--
 2 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/include/hw/acpi/generic_event_device.h b/include/hw/acpi/generic_event_device.h
index ec8e1abe0a..8f5d903146 100644
--- a/include/hw/acpi/generic_event_device.h
+++ b/include/hw/acpi/generic_event_device.h
@@ -111,6 +111,7 @@ typedef struct GEDState {
 } GEDState;
 
 #define ACPI_PCIHP_REGION_NAME "pcihp container"
+#define ACPI_MEMHP_REGION_NAME "memhp container"
 
 struct AcpiGedState {
     SysBusDevice parent_obj;
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 41be8f6dbb..8c882e0794 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -684,6 +684,8 @@ static inline DeviceState *create_acpi_ged(VirtMachineState *vms)
     DeviceState *dev;
     MachineState *ms = MACHINE(vms);
     SysBusDevice *sbdev;
+    AcpiGedState *acpi_ged_state;
+    AcpiPciHpState *pcihp_state;
     int irq = vms->irqmap[VIRT_ACPI_GED];
     uint32_t event = ACPI_GED_PWR_DOWN_EVT;
 
@@ -696,13 +698,26 @@ static inline DeviceState *create_acpi_ged(VirtMachineState *vms)
     }
 
     dev = qdev_new(TYPE_ACPI_GED);
+    acpi_ged_state = ACPI_GED(dev);
+    pcihp_state = &acpi_ged_state->pcihp_state;
+    if (pcihp_state->use_acpi_hotplug_bridge) {
+        event |= ACPI_GED_PCI_HOTPLUG_EVT;
+    }
     qdev_prop_set_uint32(dev, "ged-event", event);
     object_property_set_link(OBJECT(dev), "bus", OBJECT(vms->bus), &error_abort);
     sbdev = SYS_BUS_DEVICE(dev);
     sysbus_realize_and_unref(sbdev, &error_fatal);
 
-    sysbus_mmio_map(sbdev, 0, vms->memmap[VIRT_ACPI_GED].base);
-    sysbus_mmio_map(sbdev, 1, vms->memmap[VIRT_PCDIMM_ACPI].base);
+    sysbus_mmio_map_name(sbdev, TYPE_ACPI_GED, vms->memmap[VIRT_ACPI_GED].base);
+    sysbus_mmio_map_name(sbdev, ACPI_MEMHP_REGION_NAME,
+                         vms->memmap[VIRT_PCDIMM_ACPI].base);
+    if (pcihp_state->use_acpi_hotplug_bridge) {
+        int pcihp_region_index;
+
+        pcihp_region_index = sysbus_mmio_map_name(sbdev, ACPI_PCIHP_REGION_NAME,
+                                                  vms->memmap[VIRT_ACPI_PCIHP].base);
+        assert(pcihp_region_index >= 0);
+    }
     sysbus_connect_irq(sbdev, 0, qdev_get_gpio_in(vms->gic, irq));
 
     return dev;
-- 
2.49.0
Re: [PATCH v3 25/29] hw/arm/virt: Let virt support pci hotplug/unplug GED event
Posted by Igor Mammedov 6 months ago 
On Mon, 16 Jun 2025 11:46:54 +0200
Eric Auger <eric.auger@redhat.com> wrote:

> Set up the IO registers used to communicate between QEMU
> and ACPI.
> 
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
> 
> ---
> v2 -> v3:
> - remove acpi_ged_state->pcihp_state.use_acpi_hotplug_bridge = true;
> - use sysbus_mmio_map_name for all regs (Igor)
> - create_pcie left at its original place
> 
> v1 -> v2:
> - use ACPI_PCIHP_REGION_NAME
> ---
>  include/hw/acpi/generic_event_device.h |  1 +
>  hw/arm/virt.c                          | 19 +++++++++++++++++--
>  2 files changed, 18 insertions(+), 2 deletions(-)
> 
> diff --git a/include/hw/acpi/generic_event_device.h b/include/hw/acpi/generic_event_device.h
> index ec8e1abe0a..8f5d903146 100644
> --- a/include/hw/acpi/generic_event_device.h
> +++ b/include/hw/acpi/generic_event_device.h
> @@ -111,6 +111,7 @@ typedef struct GEDState {
>  } GEDState;
>  
>  #define ACPI_PCIHP_REGION_NAME "pcihp container"
> +#define ACPI_MEMHP_REGION_NAME "memhp container"
>  
>  struct AcpiGedState {
>      SysBusDevice parent_obj;
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index 41be8f6dbb..8c882e0794 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -684,6 +684,8 @@ static inline DeviceState *create_acpi_ged(VirtMachineState *vms)
>      DeviceState *dev;
>      MachineState *ms = MACHINE(vms);
>      SysBusDevice *sbdev;
> +    AcpiGedState *acpi_ged_state;
> +    AcpiPciHpState *pcihp_state;
>      int irq = vms->irqmap[VIRT_ACPI_GED];
>      uint32_t event = ACPI_GED_PWR_DOWN_EVT;
>  
> @@ -696,13 +698,26 @@ static inline DeviceState *create_acpi_ged(VirtMachineState *vms)
>      }
>  
>      dev = qdev_new(TYPE_ACPI_GED);

> +    acpi_ged_state = ACPI_GED(dev);
> +    pcihp_state = &acpi_ged_state->pcihp_state;
> +    if (pcihp_state->use_acpi_hotplug_bridge) {
> +        event |= ACPI_GED_PCI_HOTPLUG_EVT;
> +    }

Doesn't it belong to ged_realize()?

>      qdev_prop_set_uint32(dev, "ged-event", event);
>      object_property_set_link(OBJECT(dev), "bus", OBJECT(vms->bus), &error_abort);
>      sbdev = SYS_BUS_DEVICE(dev);
>      sysbus_realize_and_unref(sbdev, &error_fatal);
>  
> -    sysbus_mmio_map(sbdev, 0, vms->memmap[VIRT_ACPI_GED].base);
> -    sysbus_mmio_map(sbdev, 1, vms->memmap[VIRT_PCDIMM_ACPI].base);
> +    sysbus_mmio_map_name(sbdev, TYPE_ACPI_GED, vms->memmap[VIRT_ACPI_GED].base);
> +    sysbus_mmio_map_name(sbdev, ACPI_MEMHP_REGION_NAME,
> +                         vms->memmap[VIRT_PCDIMM_ACPI].base);
> +    if (pcihp_state->use_acpi_hotplug_bridge) {

like elsewhere, use property accessor

> +        int pcihp_region_index;
> +
> +        pcihp_region_index = sysbus_mmio_map_name(sbdev, ACPI_PCIHP_REGION_NAME,
> +                                                  vms->memmap[VIRT_ACPI_PCIHP].base);
> +        assert(pcihp_region_index >= 0);
> +    }
>      sysbus_connect_irq(sbdev, 0, qdev_get_gpio_in(vms->gic, irq));
>  
>      return dev;
Re: [PATCH v3 25/29] hw/arm/virt: Let virt support pci hotplug/unplug GED event
Posted by Eric Auger 5 months, 3 weeks ago 
Hi Igor,

On 6/20/25 3:06 PM, Igor Mammedov wrote:
> On Mon, 16 Jun 2025 11:46:54 +0200
> Eric Auger <eric.auger@redhat.com> wrote:
>
>> Set up the IO registers used to communicate between QEMU
>> and ACPI.
>>
>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>>
>> ---
>> v2 -> v3:
>> - remove acpi_ged_state->pcihp_state.use_acpi_hotplug_bridge = true;
>> - use sysbus_mmio_map_name for all regs (Igor)
>> - create_pcie left at its original place
>>
>> v1 -> v2:
>> - use ACPI_PCIHP_REGION_NAME
>> ---
>>  include/hw/acpi/generic_event_device.h |  1 +
>>  hw/arm/virt.c                          | 19 +++++++++++++++++--
>>  2 files changed, 18 insertions(+), 2 deletions(-)
>>
>> diff --git a/include/hw/acpi/generic_event_device.h b/include/hw/acpi/generic_event_device.h
>> index ec8e1abe0a..8f5d903146 100644
>> --- a/include/hw/acpi/generic_event_device.h
>> +++ b/include/hw/acpi/generic_event_device.h
>> @@ -111,6 +111,7 @@ typedef struct GEDState {
>>  } GEDState;
>>  
>>  #define ACPI_PCIHP_REGION_NAME "pcihp container"
>> +#define ACPI_MEMHP_REGION_NAME "memhp container"
>>  
>>  struct AcpiGedState {
>>      SysBusDevice parent_obj;
>> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
>> index 41be8f6dbb..8c882e0794 100644
>> --- a/hw/arm/virt.c
>> +++ b/hw/arm/virt.c
>> @@ -684,6 +684,8 @@ static inline DeviceState *create_acpi_ged(VirtMachineState *vms)
>>      DeviceState *dev;
>>      MachineState *ms = MACHINE(vms);
>>      SysBusDevice *sbdev;
>> +    AcpiGedState *acpi_ged_state;
>> +    AcpiPciHpState *pcihp_state;
>>      int irq = vms->irqmap[VIRT_ACPI_GED];
>>      uint32_t event = ACPI_GED_PWR_DOWN_EVT;
>>  
>> @@ -696,13 +698,26 @@ static inline DeviceState *create_acpi_ged(VirtMachineState *vms)
>>      }
>>  
>>      dev = qdev_new(TYPE_ACPI_GED);
>> +    acpi_ged_state = ACPI_GED(dev);
>> +    pcihp_state = &acpi_ged_state->pcihp_state;
>> +    if (pcihp_state->use_acpi_hotplug_bridge) {
>> +        event |= ACPI_GED_PCI_HOTPLUG_EVT;
>> +    }
> Doesn't it belong to ged_realize()?
moved there
>
>>      qdev_prop_set_uint32(dev, "ged-event", event);
>>      object_property_set_link(OBJECT(dev), "bus", OBJECT(vms->bus), &error_abort);
>>      sbdev = SYS_BUS_DEVICE(dev);
>>      sysbus_realize_and_unref(sbdev, &error_fatal);
>>  
>> -    sysbus_mmio_map(sbdev, 0, vms->memmap[VIRT_ACPI_GED].base);
>> -    sysbus_mmio_map(sbdev, 1, vms->memmap[VIRT_PCDIMM_ACPI].base);
>> +    sysbus_mmio_map_name(sbdev, TYPE_ACPI_GED, vms->memmap[VIRT_ACPI_GED].base);
>> +    sysbus_mmio_map_name(sbdev, ACPI_MEMHP_REGION_NAME,
>> +                         vms->memmap[VIRT_PCDIMM_ACPI].base);
>> +    if (pcihp_state->use_acpi_hotplug_bridge) {
> like elsewhere, use property accessor
yup

Thanks!

Eric
>
>> +        int pcihp_region_index;
>> +
>> +        pcihp_region_index = sysbus_mmio_map_name(sbdev, ACPI_PCIHP_REGION_NAME,
>> +                                                  vms->memmap[VIRT_ACPI_PCIHP].base);
>> +        assert(pcihp_region_index >= 0);
>> +    }
>>      sysbus_connect_irq(sbdev, 0, qdev_get_gpio_in(vms->gic, irq));
>>  
>>      return dev;
Re: [PATCH v3 25/29] hw/arm/virt: Let virt support pci hotplug/unplug GED event
Posted by Jonathan Cameron via 6 months ago 
On Mon, 16 Jun 2025 11:46:54 +0200
Eric Auger <eric.auger@redhat.com> wrote:

> Set up the IO registers used to communicate between QEMU
> and ACPI.
> 
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
> 
> ---
> v2 -> v3:
> - remove acpi_ged_state->pcihp_state.use_acpi_hotplug_bridge = true;
> - use sysbus_mmio_map_name for all regs (Igor)

I'm very much in favor of this change but maybe break them out to
a separate patch - perhaps even one that can run ahead of the rest
of this series?

Mind you if this is going to land shortly perhaps not worth the bother.

Either way

Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com>


> - create_pcie left at its original place
> 
> v1 -> v2:
> - use ACPI_PCIHP_REGION_NAME
> ---
>  include/hw/acpi/generic_event_device.h |  1 +
>  hw/arm/virt.c                          | 19 +++++++++++++++++--
>  2 files changed, 18 insertions(+), 2 deletions(-)
> 
> diff --git a/include/hw/acpi/generic_event_device.h b/include/hw/acpi/generic_event_device.h
> index ec8e1abe0a..8f5d903146 100644
> --- a/include/hw/acpi/generic_event_device.h
> +++ b/include/hw/acpi/generic_event_device.h
> @@ -111,6 +111,7 @@ typedef struct GEDState {
>  } GEDState;
>  
>  #define ACPI_PCIHP_REGION_NAME "pcihp container"
> +#define ACPI_MEMHP_REGION_NAME "memhp container"
>  
>  struct AcpiGedState {
>      SysBusDevice parent_obj;
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index 41be8f6dbb..8c882e0794 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -684,6 +684,8 @@ static inline DeviceState *create_acpi_ged(VirtMachineState *vms)
>      DeviceState *dev;
>      MachineState *ms = MACHINE(vms);
>      SysBusDevice *sbdev;
> +    AcpiGedState *acpi_ged_state;
> +    AcpiPciHpState *pcihp_state;
>      int irq = vms->irqmap[VIRT_ACPI_GED];
>      uint32_t event = ACPI_GED_PWR_DOWN_EVT;
>  
> @@ -696,13 +698,26 @@ static inline DeviceState *create_acpi_ged(VirtMachineState *vms)
>      }
>  
>      dev = qdev_new(TYPE_ACPI_GED);
> +    acpi_ged_state = ACPI_GED(dev);
> +    pcihp_state = &acpi_ged_state->pcihp_state;
> +    if (pcihp_state->use_acpi_hotplug_bridge) {
> +        event |= ACPI_GED_PCI_HOTPLUG_EVT;
> +    }
>      qdev_prop_set_uint32(dev, "ged-event", event);
>      object_property_set_link(OBJECT(dev), "bus", OBJECT(vms->bus), &error_abort);
>      sbdev = SYS_BUS_DEVICE(dev);
>      sysbus_realize_and_unref(sbdev, &error_fatal);
>  
> -    sysbus_mmio_map(sbdev, 0, vms->memmap[VIRT_ACPI_GED].base);
> -    sysbus_mmio_map(sbdev, 1, vms->memmap[VIRT_PCDIMM_ACPI].base);
> +    sysbus_mmio_map_name(sbdev, TYPE_ACPI_GED, vms->memmap[VIRT_ACPI_GED].base);
> +    sysbus_mmio_map_name(sbdev, ACPI_MEMHP_REGION_NAME,
> +                         vms->memmap[VIRT_PCDIMM_ACPI].base);
> +    if (pcihp_state->use_acpi_hotplug_bridge) {
> +        int pcihp_region_index;
> +
> +        pcihp_region_index = sysbus_mmio_map_name(sbdev, ACPI_PCIHP_REGION_NAME,
> +                                                  vms->memmap[VIRT_ACPI_PCIHP].base);
> +        assert(pcihp_region_index >= 0);
> +    }
>      sysbus_connect_irq(sbdev, 0, qdev_get_gpio_in(vms->gic, irq));
>  
>      return dev;
Re: [PATCH v3 25/29] hw/arm/virt: Let virt support pci hotplug/unplug GED event
Posted by Eric Auger 5 months, 3 weeks ago 
Hi Jonathan,

On 6/20/25 12:17 PM, Jonathan Cameron wrote:
> On Mon, 16 Jun 2025 11:46:54 +0200
> Eric Auger <eric.auger@redhat.com> wrote:
>
>> Set up the IO registers used to communicate between QEMU
>> and ACPI.
>>
>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>>
>> ---
>> v2 -> v3:
>> - remove acpi_ged_state->pcihp_state.use_acpi_hotplug_bridge = true;
>> - use sysbus_mmio_map_name for all regs (Igor)
> I'm very much in favor of this change but maybe break them out to
> a separate patch - perhaps even one that can run ahead of the rest
> of this series?

I split it in a separate patch.

Thanks!

Eric
>
> Mind you if this is going to land shortly perhaps not worth the bother.
>
> Either way
>
> Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com>
>
>
>> - create_pcie left at its original place
>>
>> v1 -> v2:
>> - use ACPI_PCIHP_REGION_NAME
>> ---
>>  include/hw/acpi/generic_event_device.h |  1 +
>>  hw/arm/virt.c                          | 19 +++++++++++++++++--
>>  2 files changed, 18 insertions(+), 2 deletions(-)
>>
>> diff --git a/include/hw/acpi/generic_event_device.h b/include/hw/acpi/generic_event_device.h
>> index ec8e1abe0a..8f5d903146 100644
>> --- a/include/hw/acpi/generic_event_device.h
>> +++ b/include/hw/acpi/generic_event_device.h
>> @@ -111,6 +111,7 @@ typedef struct GEDState {
>>  } GEDState;
>>  
>>  #define ACPI_PCIHP_REGION_NAME "pcihp container"
>> +#define ACPI_MEMHP_REGION_NAME "memhp container"
>>  
>>  struct AcpiGedState {
>>      SysBusDevice parent_obj;
>> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
>> index 41be8f6dbb..8c882e0794 100644
>> --- a/hw/arm/virt.c
>> +++ b/hw/arm/virt.c
>> @@ -684,6 +684,8 @@ static inline DeviceState *create_acpi_ged(VirtMachineState *vms)
>>      DeviceState *dev;
>>      MachineState *ms = MACHINE(vms);
>>      SysBusDevice *sbdev;
>> +    AcpiGedState *acpi_ged_state;
>> +    AcpiPciHpState *pcihp_state;
>>      int irq = vms->irqmap[VIRT_ACPI_GED];
>>      uint32_t event = ACPI_GED_PWR_DOWN_EVT;
>>  
>> @@ -696,13 +698,26 @@ static inline DeviceState *create_acpi_ged(VirtMachineState *vms)
>>      }
>>  
>>      dev = qdev_new(TYPE_ACPI_GED);
>> +    acpi_ged_state = ACPI_GED(dev);
>> +    pcihp_state = &acpi_ged_state->pcihp_state;
>> +    if (pcihp_state->use_acpi_hotplug_bridge) {
>> +        event |= ACPI_GED_PCI_HOTPLUG_EVT;
>> +    }
>>      qdev_prop_set_uint32(dev, "ged-event", event);
>>      object_property_set_link(OBJECT(dev), "bus", OBJECT(vms->bus), &error_abort);
>>      sbdev = SYS_BUS_DEVICE(dev);
>>      sysbus_realize_and_unref(sbdev, &error_fatal);
>>  
>> -    sysbus_mmio_map(sbdev, 0, vms->memmap[VIRT_ACPI_GED].base);
>> -    sysbus_mmio_map(sbdev, 1, vms->memmap[VIRT_PCDIMM_ACPI].base);
>> +    sysbus_mmio_map_name(sbdev, TYPE_ACPI_GED, vms->memmap[VIRT_ACPI_GED].base);
>> +    sysbus_mmio_map_name(sbdev, ACPI_MEMHP_REGION_NAME,
>> +                         vms->memmap[VIRT_PCDIMM_ACPI].base);
>> +    if (pcihp_state->use_acpi_hotplug_bridge) {
>> +        int pcihp_region_index;
>> +
>> +        pcihp_region_index = sysbus_mmio_map_name(sbdev, ACPI_PCIHP_REGION_NAME,
>> +                                                  vms->memmap[VIRT_ACPI_PCIHP].base);
>> +        assert(pcihp_region_index >= 0);
>> +    }
>>      sysbus_connect_irq(sbdev, 0, qdev_get_gpio_in(vms->gic, irq));
>>  
>>      return dev;

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.