[PATCH v3 20/29] hw/acpi/ged: Prepare the device to react to PCI hotplug events

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 20/29] hw/acpi/ged: Prepare the device to react to PCI hotplug events
Posted by Eric Auger 5 months ago
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>

---
v2 -> v3:
- pcihp_init and reset are put in ged code instead of machine code
  (Igor)

v1 -> v2:
- Introduce ACPI_PCIHP_REGION_NAME
---
 include/hw/acpi/generic_event_device.h |  4 ++++
 hw/acpi/generic_event_device.c         | 26 ++++++++++++++++++++++++++
 hw/acpi/pcihp.c                        |  1 -
 3 files changed, 30 insertions(+), 1 deletion(-)

diff --git a/include/hw/acpi/generic_event_device.h b/include/hw/acpi/generic_event_device.h
index f5ffa67a39..ec8e1abe0a 100644
--- a/include/hw/acpi/generic_event_device.h
+++ b/include/hw/acpi/generic_event_device.h
@@ -102,6 +102,7 @@ OBJECT_DECLARE_SIMPLE_TYPE(AcpiGedState, ACPI_GED)
 #define ACPI_GED_PWR_DOWN_EVT      0x2
 #define ACPI_GED_NVDIMM_HOTPLUG_EVT 0x4
 #define ACPI_GED_CPU_HOTPLUG_EVT    0x8
+#define ACPI_GED_PCI_HOTPLUG_EVT    0x10
 
 typedef struct GEDState {
     MemoryRegion evt;
@@ -109,6 +110,8 @@ typedef struct GEDState {
     uint32_t     sel;
 } GEDState;
 
+#define ACPI_PCIHP_REGION_NAME "pcihp container"
+
 struct AcpiGedState {
     SysBusDevice parent_obj;
     MemHotplugState memhp_state;
@@ -116,6 +119,7 @@ struct AcpiGedState {
     CPUHotplugState cpuhp_state;
     MemoryRegion container_cpuhp;
     AcpiPciHpState pcihp_state;
+    MemoryRegion container_pcihp;
     GEDState ged_state;
     uint32_t ged_event_bitmap;
     qemu_irq irq;
diff --git a/hw/acpi/generic_event_device.c b/hw/acpi/generic_event_device.c
index ef1c1ec51f..b4eefb0106 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/pci/pci.h"
 #include "hw/irq.h"
@@ -27,6 +28,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,
 };
 
 /*
@@ -122,6 +124,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[]
@@ -300,6 +308,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);
@@ -433,6 +443,12 @@ 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),
+                               ACPI_PCIHP_REGION_NAME, ACPI_PCIHP_SIZE);
+            sysbus_init_mmio(sbd, &s->container_pcihp);
+            acpi_pcihp_init(OBJECT(s), &s->pcihp_state,
+                            s->pcihp_state.root, &s->container_pcihp, 0);
         }
         ged_events--;
     }
@@ -474,6 +490,15 @@ static void acpi_ged_initfn(Object *obj)
     sysbus_init_mmio(sbd, &ged_st->regs);
 }
 
+static void ged_reset(DeviceState *dev)
+{
+    AcpiGedState *s = ACPI_GED(dev);
+
+    if (s->pcihp_state.use_acpi_hotplug_bridge) {
+        acpi_pcihp_reset(&s->pcihp_state);
+    }
+}
+
 static void acpi_ged_class_init(ObjectClass *class, const void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(class);
@@ -488,6 +513,7 @@ static void acpi_ged_class_init(ObjectClass *class, const void *data)
     hc->plug = acpi_ged_device_plug_cb;
     hc->unplug_request = acpi_ged_unplug_request_cb;
     hc->unplug = acpi_ged_unplug_cb;
+    device_class_set_legacy_reset(dc, ged_reset);
 
     adevc->ospm_status = acpi_ged_ospm_status;
     adevc->send_event = acpi_ged_send_event;
diff --git a/hw/acpi/pcihp.c b/hw/acpi/pcihp.c
index e79a24b821..36492aa0d9 100644
--- a/hw/acpi/pcihp.c
+++ b/hw/acpi/pcihp.c
@@ -43,7 +43,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
-- 
2.49.0
Re: [PATCH v3 20/29] hw/acpi/ged: Prepare the device to react to PCI hotplug events
Posted by Igor Mammedov 4 months, 4 weeks ago
On Mon, 16 Jun 2025 11:46:49 +0200
Eric Auger <eric.auger@redhat.com> wrote:

[...]

> @@ -433,6 +443,12 @@ 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),
> +                               ACPI_PCIHP_REGION_NAME, ACPI_PCIHP_SIZE);
> +            sysbus_init_mmio(sbd, &s->container_pcihp);
> +            acpi_pcihp_init(OBJECT(s), &s->pcihp_state,
> +                            s->pcihp_state.root, &s->container_pcihp, 0);

bus flow looks weird,
I'd suggest to
  * set root bus link on x86 as well for consistency reasons and proper ref accounting
  * and then drop 'bus' from acpi_pcihp_init() its arguments

>          }
>          ged_events--;
>      }
[...]
Re: [PATCH v3 20/29] hw/acpi/ged: Prepare the device to react to PCI hotplug events
Posted by Igor Mammedov 4 months, 4 weeks ago
On Mon, 16 Jun 2025 11:46:49 +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.

looks out of order, perhaps it should be before this patch

> 
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
> 
> ---
> v2 -> v3:
> - pcihp_init and reset are put in ged code instead of machine code
>   (Igor)
> 
> v1 -> v2:
> - Introduce ACPI_PCIHP_REGION_NAME
> ---
>  include/hw/acpi/generic_event_device.h |  4 ++++
>  hw/acpi/generic_event_device.c         | 26 ++++++++++++++++++++++++++
>  hw/acpi/pcihp.c                        |  1 -
>  3 files changed, 30 insertions(+), 1 deletion(-)
> 
> diff --git a/include/hw/acpi/generic_event_device.h b/include/hw/acpi/generic_event_device.h
> index f5ffa67a39..ec8e1abe0a 100644
> --- a/include/hw/acpi/generic_event_device.h
> +++ b/include/hw/acpi/generic_event_device.h
> @@ -102,6 +102,7 @@ OBJECT_DECLARE_SIMPLE_TYPE(AcpiGedState, ACPI_GED)
>  #define ACPI_GED_PWR_DOWN_EVT      0x2
>  #define ACPI_GED_NVDIMM_HOTPLUG_EVT 0x4
>  #define ACPI_GED_CPU_HOTPLUG_EVT    0x8
> +#define ACPI_GED_PCI_HOTPLUG_EVT    0x10
>  
>  typedef struct GEDState {
>      MemoryRegion evt;
> @@ -109,6 +110,8 @@ typedef struct GEDState {
>      uint32_t     sel;
>  } GEDState;
>  
> +#define ACPI_PCIHP_REGION_NAME "pcihp container"
> +
>  struct AcpiGedState {
>      SysBusDevice parent_obj;
>      MemHotplugState memhp_state;
> @@ -116,6 +119,7 @@ struct AcpiGedState {
>      CPUHotplugState cpuhp_state;
>      MemoryRegion container_cpuhp;
>      AcpiPciHpState pcihp_state;
> +    MemoryRegion container_pcihp;
>      GEDState ged_state;
>      uint32_t ged_event_bitmap;
>      qemu_irq irq;
> diff --git a/hw/acpi/generic_event_device.c b/hw/acpi/generic_event_device.c
> index ef1c1ec51f..b4eefb0106 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/pci/pci.h"
>  #include "hw/irq.h"
> @@ -27,6 +28,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,
>  };
>  
>  /*
> @@ -122,6 +124,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[]
> @@ -300,6 +308,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);
> @@ -433,6 +443,12 @@ 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),
> +                               ACPI_PCIHP_REGION_NAME, ACPI_PCIHP_SIZE);
> +            sysbus_init_mmio(sbd, &s->container_pcihp);
> +            acpi_pcihp_init(OBJECT(s), &s->pcihp_state,
> +                            s->pcihp_state.root, &s->container_pcihp, 0);
>          }
>          ged_events--;
>      }
> @@ -474,6 +490,15 @@ static void acpi_ged_initfn(Object *obj)
>      sysbus_init_mmio(sbd, &ged_st->regs);
>  }
>  
> +static void ged_reset(DeviceState *dev)
> +{
> +    AcpiGedState *s = ACPI_GED(dev);
> +
> +    if (s->pcihp_state.use_acpi_hotplug_bridge) {
> +        acpi_pcihp_reset(&s->pcihp_state);
> +    }
> +}
> +
>  static void acpi_ged_class_init(ObjectClass *class, const void *data)
>  {
>      DeviceClass *dc = DEVICE_CLASS(class);
> @@ -488,6 +513,7 @@ static void acpi_ged_class_init(ObjectClass *class, const void *data)
>      hc->plug = acpi_ged_device_plug_cb;
>      hc->unplug_request = acpi_ged_unplug_request_cb;
>      hc->unplug = acpi_ged_unplug_cb;
> +    device_class_set_legacy_reset(dc, ged_reset);
>  
>      adevc->ospm_status = acpi_ged_ospm_status;
>      adevc->send_event = acpi_ged_send_event;
> diff --git a/hw/acpi/pcihp.c b/hw/acpi/pcihp.c
> index e79a24b821..36492aa0d9 100644
> --- a/hw/acpi/pcihp.c
> +++ b/hw/acpi/pcihp.c
> @@ -43,7 +43,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
Re: [PATCH v3 20/29] hw/acpi/ged: Prepare the device to react to PCI hotplug events
Posted by Eric Auger 4 months, 3 weeks ago

On 6/20/25 2:57 PM, Igor Mammedov wrote:
> On Mon, 16 Jun 2025 11:46:49 +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.
> looks out of order, perhaps it should be before this patch
done, swapped both patches.

Eric
>
>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>>
>> ---
>> v2 -> v3:
>> - pcihp_init and reset are put in ged code instead of machine code
>>   (Igor)
>>
>> v1 -> v2:
>> - Introduce ACPI_PCIHP_REGION_NAME
>> ---
>>  include/hw/acpi/generic_event_device.h |  4 ++++
>>  hw/acpi/generic_event_device.c         | 26 ++++++++++++++++++++++++++
>>  hw/acpi/pcihp.c                        |  1 -
>>  3 files changed, 30 insertions(+), 1 deletion(-)
>>
>> diff --git a/include/hw/acpi/generic_event_device.h b/include/hw/acpi/generic_event_device.h
>> index f5ffa67a39..ec8e1abe0a 100644
>> --- a/include/hw/acpi/generic_event_device.h
>> +++ b/include/hw/acpi/generic_event_device.h
>> @@ -102,6 +102,7 @@ OBJECT_DECLARE_SIMPLE_TYPE(AcpiGedState, ACPI_GED)
>>  #define ACPI_GED_PWR_DOWN_EVT      0x2
>>  #define ACPI_GED_NVDIMM_HOTPLUG_EVT 0x4
>>  #define ACPI_GED_CPU_HOTPLUG_EVT    0x8
>> +#define ACPI_GED_PCI_HOTPLUG_EVT    0x10
>>  
>>  typedef struct GEDState {
>>      MemoryRegion evt;
>> @@ -109,6 +110,8 @@ typedef struct GEDState {
>>      uint32_t     sel;
>>  } GEDState;
>>  
>> +#define ACPI_PCIHP_REGION_NAME "pcihp container"
>> +
>>  struct AcpiGedState {
>>      SysBusDevice parent_obj;
>>      MemHotplugState memhp_state;
>> @@ -116,6 +119,7 @@ struct AcpiGedState {
>>      CPUHotplugState cpuhp_state;
>>      MemoryRegion container_cpuhp;
>>      AcpiPciHpState pcihp_state;
>> +    MemoryRegion container_pcihp;
>>      GEDState ged_state;
>>      uint32_t ged_event_bitmap;
>>      qemu_irq irq;
>> diff --git a/hw/acpi/generic_event_device.c b/hw/acpi/generic_event_device.c
>> index ef1c1ec51f..b4eefb0106 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/pci/pci.h"
>>  #include "hw/irq.h"
>> @@ -27,6 +28,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,
>>  };
>>  
>>  /*
>> @@ -122,6 +124,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[]
>> @@ -300,6 +308,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);
>> @@ -433,6 +443,12 @@ 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),
>> +                               ACPI_PCIHP_REGION_NAME, ACPI_PCIHP_SIZE);
>> +            sysbus_init_mmio(sbd, &s->container_pcihp);
>> +            acpi_pcihp_init(OBJECT(s), &s->pcihp_state,
>> +                            s->pcihp_state.root, &s->container_pcihp, 0);
>>          }
>>          ged_events--;
>>      }
>> @@ -474,6 +490,15 @@ static void acpi_ged_initfn(Object *obj)
>>      sysbus_init_mmio(sbd, &ged_st->regs);
>>  }
>>  
>> +static void ged_reset(DeviceState *dev)
>> +{
>> +    AcpiGedState *s = ACPI_GED(dev);
>> +
>> +    if (s->pcihp_state.use_acpi_hotplug_bridge) {
>> +        acpi_pcihp_reset(&s->pcihp_state);
>> +    }
>> +}
>> +
>>  static void acpi_ged_class_init(ObjectClass *class, const void *data)
>>  {
>>      DeviceClass *dc = DEVICE_CLASS(class);
>> @@ -488,6 +513,7 @@ static void acpi_ged_class_init(ObjectClass *class, const void *data)
>>      hc->plug = acpi_ged_device_plug_cb;
>>      hc->unplug_request = acpi_ged_unplug_request_cb;
>>      hc->unplug = acpi_ged_unplug_cb;
>> +    device_class_set_legacy_reset(dc, ged_reset);
>>  
>>      adevc->ospm_status = acpi_ged_ospm_status;
>>      adevc->send_event = acpi_ged_send_event;
>> diff --git a/hw/acpi/pcihp.c b/hw/acpi/pcihp.c
>> index e79a24b821..36492aa0d9 100644
>> --- a/hw/acpi/pcihp.c
>> +++ b/hw/acpi/pcihp.c
>> @@ -43,7 +43,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
Re: [PATCH v3 20/29] hw/acpi/ged: Prepare the device to react to PCI hotplug events
Posted by Jonathan Cameron via 4 months, 4 weeks ago
On Mon, 16 Jun 2025 11:46:49 +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>

Been a little while since I looked at how all this fits together
but this looks right from what I can remember.  Hopefully others
can give a more definitive review!

One stray change that belongs in an earlier patch though.
With that fixed up.

So with that in mind.
Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com>



> diff --git a/hw/acpi/pcihp.c b/hw/acpi/pcihp.c
> index e79a24b821..36492aa0d9 100644
> --- a/hw/acpi/pcihp.c
> +++ b/hw/acpi/pcihp.c
> @@ -43,7 +43,6 @@
>  #include "qobject/qnum.h"
>  #include "trace.h"
>  
> -#define ACPI_PCIHP_SIZE 0x0018

It goes away from here, but doesn't get added anywhere?

Looks like this belongs in patch 17?

>  #define PCI_UP_BASE 0x0000
>  #define PCI_DOWN_BASE 0x0004
>  #define PCI_EJ_BASE 0x0008
Re: [PATCH v3 20/29] hw/acpi/ged: Prepare the device to react to PCI hotplug events
Posted by Eric Auger 4 months, 3 weeks ago

On 6/20/25 11:44 AM, Jonathan Cameron wrote:
> On Mon, 16 Jun 2025 11:46:49 +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>
> Been a little while since I looked at how all this fits together
> but this looks right from what I can remember.  Hopefully others
> can give a more definitive review!
>
> One stray change that belongs in an earlier patch though.
> With that fixed up.
>
> So with that in mind.
> Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com>
thanks!
>
>
>
>> diff --git a/hw/acpi/pcihp.c b/hw/acpi/pcihp.c
>> index e79a24b821..36492aa0d9 100644
>> --- a/hw/acpi/pcihp.c
>> +++ b/hw/acpi/pcihp.c
>> @@ -43,7 +43,6 @@
>>  #include "qobject/qnum.h"
>>  #include "trace.h"
>>  
>> -#define ACPI_PCIHP_SIZE 0x0018
> It goes away from here, but doesn't get added anywhere?
>
> Looks like this belongs in patch 17?
removed from 17!

Eric
>
>>  #define PCI_UP_BASE 0x0000
>>  #define PCI_DOWN_BASE 0x0004
>>  #define PCI_EJ_BASE 0x0008