Re: [PATCH v4 25/32] hw/acpi/ged: Prepare the device to react to PCI hotplug events

[Jonathan Cameron via <qemu-devel@nongnu.org>]

On Fri, 27 Jun 2025 11:55:14 +0200
Eric Auger <eric.auger@redhat.com> wrote:

> QEMU will notify the OS about PCI hotplug/hotunplug events through
> GED interrupts. Let the GED device handle a new PCI hotplug event.
> On its occurrence it calls the \\_SB.PCI0.PCNT method with the BLCK
> mutex held.
> 
> The GED device uses a dedicated MMIO region that will be mapped
> by the machine code.
> 
> At this point the GED still does not support PCI device hotplug in
> its TYPE_HOTPLUG_HANDLER implementation. This will come in a
> subsequent patch.
> 
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
Maybe call out why you aren't following the advice in the
docs for device_class_set_legacy_reset() to use the resettable API. 

One other question inline about setting of the event bitmap in
ged_realize rather than create_acpi_ged() in virt.c

There is not obviously right answer to where that should be but
what you have here seems inconsistent with existing code s
a comment may makes sense if you leave it as it stands.

> 
> ---
> v3 -> v4:
> - add qbus_set_hotplug_handler
> - root bus is not passed in acpi_pcihp_init arg
> 
> v2 -> v3:
> - pcihp_init and reset are put in ged code instead of machine code
>   (Igor)
> - Add ACPI_GED_PCI_HOTPLUG_EVT event depending on use_acpi_hotplug_bridge
>   (Igor)
> 
> v1 -> v2:
> - Introduce ACPI_PCIHP_REGION_NAME

> diff --git a/hw/acpi/generic_event_device.c b/hw/acpi/generic_event_device.c
> index 92b931758f..fc84bfb34e 100644
> --- a/hw/acpi/generic_event_device.c
> +++ b/hw/acpi/generic_event_device.c

> @@ -427,9 +437,13 @@ static void acpi_ged_realize(DeviceState *dev, Error **errp)
>  {
>      SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
>      AcpiGedState *s = ACPI_GED(dev);
> +    AcpiPciHpState *pcihp_state = &s->pcihp_state;
>      uint32_t ged_events;
>      int i;
>  
> +    if (pcihp_state->use_acpi_hotplug_bridge) {
> +        s->ged_event_bitmap |= ACPI_GED_PCI_HOTPLUG_EVT;
> +    }

Maybe a comment on why this belongs insider the GED code rather
than being provided in the event_bitmap from virt.c or similar
as is done for all the over elements of ged_event_bitmap.

Particularly as you get the acpi_pcihp property in create_acpi_ged()
in patch 29.


>      ged_events = ctpop32(s->ged_event_bitmap);
>