Re: [PATCH 16/22] hw/acpi/ged: Prepare the device to react to PCI hotplug events

["Philippe Mathieu-Daudé" <philmd@linaro.org>]

Hi Eric,

On 14/5/25 18:01, Eric Auger 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>
> ---
>   include/hw/acpi/generic_event_device.h |  5 +++++
>   include/hw/acpi/pcihp.h                |  2 ++
>   hw/acpi/generic_event_device.c         | 15 +++++++++++++++
>   hw/acpi/pcihp.c                        |  1 -
>   4 files changed, 22 insertions(+), 1 deletion(-)


> diff --git a/include/hw/acpi/pcihp.h b/include/hw/acpi/pcihp.h
> index bc31dbff39..bda5ea24b5 100644
> --- a/include/hw/acpi/pcihp.h
> +++ b/include/hw/acpi/pcihp.h
> @@ -38,6 +38,8 @@
>   #define ACPI_PCIHP_SEJ_BASE 0x8
>   #define ACPI_PCIHP_BNMR_BASE 0x10
>   
> +#define ACPI_PCIHP_SIZE 0x0018
> +
>   typedef struct AcpiPciHpPciStatus {
>       uint32_t up;
>       uint32_t down;
> diff --git a/hw/acpi/generic_event_device.c b/hw/acpi/generic_event_device.c
> index 7a62f8d5bc..dc3620553d 100644
> --- a/hw/acpi/generic_event_device.c
> +++ b/hw/acpi/generic_event_device.c
> @@ -12,6 +12,7 @@
>   #include "qemu/osdep.h"
>   #include "qapi/error.h"
>   #include "hw/acpi/acpi.h"
> +#include "hw/acpi/pcihp.h"
>   #include "hw/acpi/generic_event_device.h"
>   #include "hw/irq.h"
>   #include "hw/mem/pc-dimm.h"
> @@ -26,6 +27,7 @@ static const uint32_t ged_supported_events[] = {
>       ACPI_GED_PWR_DOWN_EVT,
>       ACPI_GED_NVDIMM_HOTPLUG_EVT,
>       ACPI_GED_CPU_HOTPLUG_EVT,
> +    ACPI_GED_PCI_HOTPLUG_EVT,
>   };
>   
>   /*
> @@ -121,6 +123,12 @@ void build_ged_aml(Aml *table, const char *name, HotplugHandler *hotplug_dev,
>                              aml_notify(aml_name("\\_SB.NVDR"),
>                                         aml_int(0x80)));
>                   break;
> +            case ACPI_GED_PCI_HOTPLUG_EVT:
> +                aml_append(if_ctx,
> +                           aml_acquire(aml_name("\\_SB.PCI0.BLCK"), 0xFFFF));
> +                aml_append(if_ctx, aml_call0("\\_SB.PCI0.PCNT"));
> +                aml_append(if_ctx, aml_release(aml_name("\\_SB.PCI0.BLCK")));
> +                break;
>               default:
>                   /*
>                    * Please make sure all the events in ged_supported_events[]
> @@ -299,6 +307,8 @@ static void acpi_ged_send_event(AcpiDeviceIf *adev, AcpiEventStatusBits ev)
>           sel = ACPI_GED_NVDIMM_HOTPLUG_EVT;
>       } else if (ev & ACPI_CPU_HOTPLUG_STATUS) {
>           sel = ACPI_GED_CPU_HOTPLUG_EVT;
> +    } else if (ev & ACPI_PCI_HOTPLUG_STATUS) {
> +        sel = ACPI_GED_PCI_HOTPLUG_EVT;
>       } else {
>           /* Unknown event. Return without generating interrupt. */
>           warn_report("GED: Unsupported event %d. No irq injected", ev);
> @@ -428,6 +438,11 @@ static void acpi_ged_realize(DeviceState *dev, Error **errp)
>               cpu_hotplug_hw_init(&s->container_cpuhp, OBJECT(dev),
>                                   &s->cpuhp_state, 0);
>               break;
> +        case ACPI_GED_PCI_HOTPLUG_EVT:
> +            memory_region_init(&s->container_pcihp, OBJECT(dev),
> +                                "pcihp container",

Since "pcihp container" isn't just a description anymore, can
we have a ACPI_PCIHP_REGION_NAME definition?

> +                                ACPI_PCIHP_SIZE);
> +            sysbus_init_mmio(sbd, &s->container_pcihp);
>           }
>           ged_events--;
>       }
> diff --git a/hw/acpi/pcihp.c b/hw/acpi/pcihp.c
> index 57fe8938b1..e87846a1fa 100644
> --- a/hw/acpi/pcihp.c
> +++ b/hw/acpi/pcihp.c
> @@ -44,7 +44,6 @@
>   #include "qobject/qnum.h"
>   #include "trace.h"
>   
> -#define ACPI_PCIHP_SIZE 0x0018
>   #define PCI_UP_BASE 0x0000
>   #define PCI_DOWN_BASE 0x0004
>   #define PCI_EJ_BASE 0x0008