Hi Eric,
On 4/28/25 07:25, 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 occurence it calls the \\_SB.PCI0.PCNT method with the BLCK
nit: occurrence -^
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
nit: period missing ^
Signed-off-by: Eric Auger <eric.au...@redhat.com>
---
include/hw/acpi/generic_event_device.h | 5 +++++
include/hw/acpi/pcihp.h | 2 ++
hw/acpi/generic_event_device.c | 14 ++++++++++++++
3 files changed, 21 insertions(+)
diff --git a/include/hw/acpi/generic_event_device.h
b/include/hw/acpi/generic_event_device.h
index d2dac87b4a..28be6c0582 100644
--- a/include/hw/acpi/generic_event_device.h
+++ b/include/hw/acpi/generic_event_device.h
@@ -63,6 +63,7 @@
#include "hw/acpi/memory_hotplug.h"
#include "hw/acpi/ghes.h"
#include "hw/acpi/cpu.h"
+#include "hw/acpi/pcihp.h"
#include "qom/object.h"
#define ACPI_POWER_BUTTON_DEVICE "PWRB"
@@ -101,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;
@@ -114,6 +116,9 @@ struct AcpiGedState {
MemoryRegion container_memhp;
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/include/hw/acpi/pcihp.h b/include/hw/acpi/pcihp.h
index 451ef74284..abb6e14549 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_PCI_HOTPLUG_REG_LEN 0x14
This is wrong, no? It should be 0x18, i.e. 24 bytes or 6 registers of 4 bytes.
I think it's better to move the defines in pcihp.c into pcihp.h and
use them to define ACPI_PCI_HOTPLUG_REG_LEN so the values are consistent, like:
diff --git a/hw/acpi/pcihp.c b/hw/acpi/pcihp.c
index 82b8ea2811..3ab8b9de94 100644
--- a/hw/acpi/pcihp.c
+++ b/hw/acpi/pcihp.c
@@ -44,14 +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
-#define PCI_RMV_BASE 0x000c
-#define PCI_SEL_BASE 0x0010
-#define PCI_AIDX_BASE 0x0014
-
typedef struct AcpiPciHpFind {
int bsel;
PCIBus *bus;
diff --git a/include/hw/acpi/pcihp.h b/include/hw/acpi/pcihp.h
index abb6e14549..6b79adcaef 100644
--- a/include/hw/acpi/pcihp.h
+++ b/include/hw/acpi/pcihp.h
@@ -38,7 +38,15 @@
#define ACPI_PCIHP_SEJ_BASE 0x8
#define ACPI_PCIHP_BNMR_BASE 0x10
-#define ACPI_PCI_HOTPLUG_REG_LEN 0x14
+#define ACPI_PCIHP_SIZE 0x0018
+#define PCI_UP_BASE 0x0000
+#define PCI_DOWN_BASE 0x0004
+#define PCI_EJ_BASE 0x0008
+#define PCI_RMV_BASE 0x000c
+#define PCI_SEL_BASE 0x0010
+#define PCI_AIDX_BASE 0x0014
+
+#define ACPI_PCI_HOTPLUG_REG_LEN ACPI_PCIHP_SIZE
typedef struct AcpiPciHpPciStatus {
uint32_t up;
or even use only one of them (ACPI_PCI_HOTPLUG_REG_LEN, ACPI_PCIHP_SIZE, or
another new name that would fit better in both contexts).
Cheers,
Gustavo
+
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 7b2d582fff..0fd8baba97 100644
--- a/hw/acpi/generic_event_device.c
+++ b/hw/acpi/generic_event_device.c
@@ -26,6 +26,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 +122,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 +306,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 +437,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",
+ ACPI_PCI_HOTPLUG_REG_LEN);
+ sysbus_init_mmio(sbd, &s->container_pcihp);
}
ged_events--;
}
