ACPI GED (as described in the ACPI 6.4 spec) uses an interrupt listed in the
_CRS object of GED to intimate OSPM about an event. Later then demultiplexes the
notified event by evaluating ACPI _EVT method to know the type of event. Use
ACPI GED to also notify the guest kernel about any CPU hot(un)plug events.
ACPI CPU hotplug related initialization should only happen if ACPI_CPU_HOTPLUG
support has been enabled for particular architecture. Add cpu_hotplug_hw_init()
stub to avoid compilation break.
Co-developed-by: Keqian Zhu <zhukeqian1@huawei.com>
Signed-off-by: Keqian Zhu <zhukeqian1@huawei.com>
Signed-off-by: Salil Mehta <salil.mehta@huawei.com>
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Reviewed-by: Gavin Shan <gshan@redhat.com>
Reviewed-by: David Hildenbrand <david@redhat.com>
Reviewed-by: Shaoqin Huang <shahuang@redhat.com>
Tested-by: Vishnu Pajjuri <vishnu@os.amperecomputing.com>
Tested-by: Xianglai Li <lixianglai@loongson.cn>
Tested-by: Miguel Luis <miguel.luis@oracle.com>
---
hw/acpi/acpi-cpu-hotplug-stub.c | 6 ++++++
hw/acpi/generic_event_device.c | 17 +++++++++++++++++
include/hw/acpi/generic_event_device.h | 4 ++++
3 files changed, 27 insertions(+)
diff --git a/hw/acpi/acpi-cpu-hotplug-stub.c b/hw/acpi/acpi-cpu-hotplug-stub.c
index 3fc4b14c26..c6c61bb9cd 100644
--- a/hw/acpi/acpi-cpu-hotplug-stub.c
+++ b/hw/acpi/acpi-cpu-hotplug-stub.c
@@ -19,6 +19,12 @@ void legacy_acpi_cpu_hotplug_init(MemoryRegion *parent, Object *owner,
return;
}
+void cpu_hotplug_hw_init(MemoryRegion *as, Object *owner,
+ CPUHotplugState *state, hwaddr base_addr)
+{
+ return;
+}
+
void acpi_cpu_ospm_status(CPUHotplugState *cpu_st, ACPIOSTInfoList ***list)
{
return;
diff --git a/hw/acpi/generic_event_device.c b/hw/acpi/generic_event_device.c
index a3d31631fe..57b0c2815b 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/cpu.h"
#include "hw/acpi/generic_event_device.h"
#include "hw/irq.h"
#include "hw/mem/pc-dimm.h"
@@ -25,6 +26,7 @@ static const uint32_t ged_supported_events[] = {
ACPI_GED_MEM_HOTPLUG_EVT,
ACPI_GED_PWR_DOWN_EVT,
ACPI_GED_NVDIMM_HOTPLUG_EVT,
+ ACPI_GED_CPU_HOTPLUG_EVT,
};
/*
@@ -234,6 +236,8 @@ static void acpi_ged_device_plug_cb(HotplugHandler *hotplug_dev,
} else {
acpi_memory_plug_cb(hotplug_dev, &s->memhp_state, dev, errp);
}
+ } else if (object_dynamic_cast(OBJECT(dev), TYPE_CPU)) {
+ acpi_cpu_plug_cb(hotplug_dev, &s->cpuhp_state, dev, errp);
} else {
error_setg(errp, "virt: device plug request for unsupported device"
" type: %s", object_get_typename(OBJECT(dev)));
@@ -248,6 +252,8 @@ static void acpi_ged_unplug_request_cb(HotplugHandler *hotplug_dev,
if ((object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM) &&
!(object_dynamic_cast(OBJECT(dev), TYPE_NVDIMM)))) {
acpi_memory_unplug_request_cb(hotplug_dev, &s->memhp_state, dev, errp);
+ } else if (object_dynamic_cast(OBJECT(dev), TYPE_CPU)) {
+ acpi_cpu_unplug_request_cb(hotplug_dev, &s->cpuhp_state, dev, errp);
} else {
error_setg(errp, "acpi: device unplug request for unsupported device"
" type: %s", object_get_typename(OBJECT(dev)));
@@ -261,6 +267,8 @@ static void acpi_ged_unplug_cb(HotplugHandler *hotplug_dev,
if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) {
acpi_memory_unplug_cb(&s->memhp_state, dev, errp);
+ } else if (object_dynamic_cast(OBJECT(dev), TYPE_CPU)) {
+ acpi_cpu_unplug_cb(&s->cpuhp_state, dev, errp);
} else {
error_setg(errp, "acpi: device unplug for unsupported device"
" type: %s", object_get_typename(OBJECT(dev)));
@@ -272,6 +280,7 @@ static void acpi_ged_ospm_status(AcpiDeviceIf *adev, ACPIOSTInfoList ***list)
AcpiGedState *s = ACPI_GED(adev);
acpi_memory_ospm_status(&s->memhp_state, list);
+ acpi_cpu_ospm_status(&s->cpuhp_state, list);
}
static void acpi_ged_send_event(AcpiDeviceIf *adev, AcpiEventStatusBits ev)
@@ -286,6 +295,8 @@ static void acpi_ged_send_event(AcpiDeviceIf *adev, AcpiEventStatusBits ev)
sel = ACPI_GED_PWR_DOWN_EVT;
} else if (ev & ACPI_NVDIMM_HOTPLUG_STATUS) {
sel = ACPI_GED_NVDIMM_HOTPLUG_EVT;
+ } else if (ev & ACPI_CPU_HOTPLUG_STATUS) {
+ sel = ACPI_GED_CPU_HOTPLUG_EVT;
} else {
/* Unknown event. Return without generating interrupt. */
warn_report("GED: Unsupported event %d. No irq injected", ev);
@@ -400,6 +411,12 @@ static void acpi_ged_initfn(Object *obj)
memory_region_init_io(&ged_st->regs, obj, &ged_regs_ops, ged_st,
TYPE_ACPI_GED "-regs", ACPI_GED_REG_COUNT);
sysbus_init_mmio(sbd, &ged_st->regs);
+
+ memory_region_init(&s->container_cpuhp, OBJECT(dev), "cpuhp container",
+ ACPI_CPU_HOTPLUG_REG_LEN);
+ sysbus_init_mmio(SYS_BUS_DEVICE(dev), &s->container_cpuhp);
+ cpu_hotplug_hw_init(&s->container_cpuhp, OBJECT(dev),
+ &s->cpuhp_state, 0);
}
static void acpi_ged_class_init(ObjectClass *class, void *data)
diff --git a/include/hw/acpi/generic_event_device.h b/include/hw/acpi/generic_event_device.h
index ba84ce0214..90fc41cbb8 100644
--- a/include/hw/acpi/generic_event_device.h
+++ b/include/hw/acpi/generic_event_device.h
@@ -60,6 +60,7 @@
#define HW_ACPI_GENERIC_EVENT_DEVICE_H
#include "hw/sysbus.h"
+#include "hw/acpi/cpu_hotplug.h"
#include "hw/acpi/memory_hotplug.h"
#include "hw/acpi/ghes.h"
#include "qom/object.h"
@@ -95,6 +96,7 @@ OBJECT_DECLARE_SIMPLE_TYPE(AcpiGedState, ACPI_GED)
#define ACPI_GED_MEM_HOTPLUG_EVT 0x1
#define ACPI_GED_PWR_DOWN_EVT 0x2
#define ACPI_GED_NVDIMM_HOTPLUG_EVT 0x4
+#define ACPI_GED_CPU_HOTPLUG_EVT 0x8
typedef struct GEDState {
MemoryRegion evt;
@@ -106,6 +108,8 @@ struct AcpiGedState {
SysBusDevice parent_obj;
MemHotplugState memhp_state;
MemoryRegion container_memhp;
+ CPUHotplugState cpuhp_state;
+ MemoryRegion container_cpuhp;
GEDState ged_state;
uint32_t ged_event_bitmap;
qemu_irq irq;
--
2.34.1Hi Salil,
On Mon, Nov 13, 2023 at 08:12:31PM +0000, Salil Mehta via wrote:
> Date: Mon, 13 Nov 2023 20:12:31 +0000
> From: Salil Mehta via <qemu-devel@nongnu.org>
> Subject: [PATCH V7 3/8] hw/acpi: Update ACPI GED framework to support vCPU
> Hotplug
> X-Mailer: git-send-email 2.8.3
>
[snip]
> @@ -400,6 +411,12 @@ static void acpi_ged_initfn(Object *obj)
> memory_region_init_io(&ged_st->regs, obj, &ged_regs_ops, ged_st,
> TYPE_ACPI_GED "-regs", ACPI_GED_REG_COUNT);
> sysbus_init_mmio(sbd, &ged_st->regs);
> +
> + memory_region_init(&s->container_cpuhp, OBJECT(dev), "cpuhp container",
> + ACPI_CPU_HOTPLUG_REG_LEN);
> + sysbus_init_mmio(SYS_BUS_DEVICE(dev), &s->container_cpuhp);
> + cpu_hotplug_hw_init(&s->container_cpuhp, OBJECT(dev),
> + &s->cpuhp_state, 0);
I find this cpu_hotplug_hw_init() can still cause qtest errors on x86
platforms as you mentioned in v6: https://lore.kernel.org/qemu-devel/15e70616-6abb-63a4-17d0-820f4a254607@opnsrc.net/T/#m108f102b2fe92b7dd7218f2f942f7b233a9d6af3
IIUC, microvm machine has its own 'possible_cpus_arch_ids' and that is
inherited from its parent x86 machine.
The above error is because device-introspect-test sets the none-machine:
# starting QEMU: exec ./qemu-system-i386 -qtest unix:/tmp/qtest-3094820.sock -qtest-log /dev/null -chardev socket,path=/tmp/qtest-3094820.qmp,id=char0 -mon chardev=char0,mode=control -display none -audio none -nodefaults -machine none -accel qtest
So what about just checking mc->possible_cpu_arch_ids instead of an
assert in cpu_hotplug_hw_init()?
diff --git a/hw/acpi/cpu.c b/hw/acpi/cpu.c
index 4b24a2500361..303f1f1f57bc 100644
--- a/hw/acpi/cpu.c
+++ b/hw/acpi/cpu.c
@@ -221,7 +221,10 @@ void cpu_hotplug_hw_init(MemoryRegion *as, Object *owner,
const CPUArchIdList *id_list;
int i;
- assert(mc->possible_cpu_arch_ids);
+ if (!mc->possible_cpu_arch_ids) {
+ return;
+ }
+
id_list = mc->possible_cpu_arch_ids(machine);
state->dev_count = id_list->len;
state->devs = g_new0(typeof(*state->devs), state->dev_count);
This check seems to be acceptable in the general code path? Not all machines
have possible_cpu_arch_ids, after all.
Thanks,
Zhao
> }
>
> static void acpi_ged_class_init(ObjectClass *class, void *data)
> diff --git a/include/hw/acpi/generic_event_device.h b/include/hw/acpi/generic_event_device.h
> index ba84ce0214..90fc41cbb8 100644
> --- a/include/hw/acpi/generic_event_device.h
> +++ b/include/hw/acpi/generic_event_device.h
> @@ -60,6 +60,7 @@
> #define HW_ACPI_GENERIC_EVENT_DEVICE_H
>
> #include "hw/sysbus.h"
> +#include "hw/acpi/cpu_hotplug.h"
> #include "hw/acpi/memory_hotplug.h"
> #include "hw/acpi/ghes.h"
> #include "qom/object.h"
> @@ -95,6 +96,7 @@ OBJECT_DECLARE_SIMPLE_TYPE(AcpiGedState, ACPI_GED)
> #define ACPI_GED_MEM_HOTPLUG_EVT 0x1
> #define ACPI_GED_PWR_DOWN_EVT 0x2
> #define ACPI_GED_NVDIMM_HOTPLUG_EVT 0x4
> +#define ACPI_GED_CPU_HOTPLUG_EVT 0x8
>
> typedef struct GEDState {
> MemoryRegion evt;
> @@ -106,6 +108,8 @@ struct AcpiGedState {
> SysBusDevice parent_obj;
> MemHotplugState memhp_state;
> MemoryRegion container_memhp;
> + CPUHotplugState cpuhp_state;
> + MemoryRegion container_cpuhp;
> GEDState ged_state;
> uint32_t ged_event_bitmap;
> qemu_irq irq;
> --
> 2.34.1
>
>
On Mon, 13 Nov 2023 20:12:31 +0000
Salil Mehta <salil.mehta@huawei.com> wrote:
> ACPI GED (as described in the ACPI 6.4 spec) uses an interrupt listed in the
> _CRS object of GED to intimate OSPM about an event. Later then demultiplexes the
^^^ typo?
> notified event by evaluating ACPI _EVT method to know the type of event. Use
^ new line/paragraph ?
> ACPI GED to also notify the guest kernel about any CPU hot(un)plug events.
> ACPI CPU hotplug related initialization should only happen if ACPI_CPU_HOTPLUG
> support has been enabled for particular architecture. Add cpu_hotplug_hw_init()
> stub to avoid compilation break.
>
> Co-developed-by: Keqian Zhu <zhukeqian1@huawei.com>
> Signed-off-by: Keqian Zhu <zhukeqian1@huawei.com>
> Signed-off-by: Salil Mehta <salil.mehta@huawei.com>
> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> Reviewed-by: Gavin Shan <gshan@redhat.com>
> Reviewed-by: David Hildenbrand <david@redhat.com>
> Reviewed-by: Shaoqin Huang <shahuang@redhat.com>
> Tested-by: Vishnu Pajjuri <vishnu@os.amperecomputing.com>
> Tested-by: Xianglai Li <lixianglai@loongson.cn>
> Tested-by: Miguel Luis <miguel.luis@oracle.com>
> ---
FYI,
add per version changes here, ex:
v7:
- Add cpu_hotplug_hw_init() stub ...
(instigated@by)
this way reviewer isn't forced to check previous versions of it.
otherwise:
Reviewed-by:Igor Mammedov <imammedo@redhat.com>
> hw/acpi/acpi-cpu-hotplug-stub.c | 6 ++++++
> hw/acpi/generic_event_device.c | 17 +++++++++++++++++
> include/hw/acpi/generic_event_device.h | 4 ++++
> 3 files changed, 27 insertions(+)
>
> diff --git a/hw/acpi/acpi-cpu-hotplug-stub.c b/hw/acpi/acpi-cpu-hotplug-stub.c
> index 3fc4b14c26..c6c61bb9cd 100644
> --- a/hw/acpi/acpi-cpu-hotplug-stub.c
> +++ b/hw/acpi/acpi-cpu-hotplug-stub.c
> @@ -19,6 +19,12 @@ void legacy_acpi_cpu_hotplug_init(MemoryRegion *parent, Object *owner,
> return;
> }
>
> +void cpu_hotplug_hw_init(MemoryRegion *as, Object *owner,
> + CPUHotplugState *state, hwaddr base_addr)
> +{
> + return;
> +}
> +
> void acpi_cpu_ospm_status(CPUHotplugState *cpu_st, ACPIOSTInfoList ***list)
> {
> return;
> diff --git a/hw/acpi/generic_event_device.c b/hw/acpi/generic_event_device.c
> index a3d31631fe..57b0c2815b 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/cpu.h"
> #include "hw/acpi/generic_event_device.h"
> #include "hw/irq.h"
> #include "hw/mem/pc-dimm.h"
> @@ -25,6 +26,7 @@ static const uint32_t ged_supported_events[] = {
> ACPI_GED_MEM_HOTPLUG_EVT,
> ACPI_GED_PWR_DOWN_EVT,
> ACPI_GED_NVDIMM_HOTPLUG_EVT,
> + ACPI_GED_CPU_HOTPLUG_EVT,
> };
>
> /*
> @@ -234,6 +236,8 @@ static void acpi_ged_device_plug_cb(HotplugHandler *hotplug_dev,
> } else {
> acpi_memory_plug_cb(hotplug_dev, &s->memhp_state, dev, errp);
> }
> + } else if (object_dynamic_cast(OBJECT(dev), TYPE_CPU)) {
> + acpi_cpu_plug_cb(hotplug_dev, &s->cpuhp_state, dev, errp);
> } else {
> error_setg(errp, "virt: device plug request for unsupported device"
> " type: %s", object_get_typename(OBJECT(dev)));
> @@ -248,6 +252,8 @@ static void acpi_ged_unplug_request_cb(HotplugHandler *hotplug_dev,
> if ((object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM) &&
> !(object_dynamic_cast(OBJECT(dev), TYPE_NVDIMM)))) {
> acpi_memory_unplug_request_cb(hotplug_dev, &s->memhp_state, dev, errp);
> + } else if (object_dynamic_cast(OBJECT(dev), TYPE_CPU)) {
> + acpi_cpu_unplug_request_cb(hotplug_dev, &s->cpuhp_state, dev, errp);
> } else {
> error_setg(errp, "acpi: device unplug request for unsupported device"
> " type: %s", object_get_typename(OBJECT(dev)));
> @@ -261,6 +267,8 @@ static void acpi_ged_unplug_cb(HotplugHandler *hotplug_dev,
>
> if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) {
> acpi_memory_unplug_cb(&s->memhp_state, dev, errp);
> + } else if (object_dynamic_cast(OBJECT(dev), TYPE_CPU)) {
> + acpi_cpu_unplug_cb(&s->cpuhp_state, dev, errp);
> } else {
> error_setg(errp, "acpi: device unplug for unsupported device"
> " type: %s", object_get_typename(OBJECT(dev)));
> @@ -272,6 +280,7 @@ static void acpi_ged_ospm_status(AcpiDeviceIf *adev, ACPIOSTInfoList ***list)
> AcpiGedState *s = ACPI_GED(adev);
>
> acpi_memory_ospm_status(&s->memhp_state, list);
> + acpi_cpu_ospm_status(&s->cpuhp_state, list);
> }
>
> static void acpi_ged_send_event(AcpiDeviceIf *adev, AcpiEventStatusBits ev)
> @@ -286,6 +295,8 @@ static void acpi_ged_send_event(AcpiDeviceIf *adev, AcpiEventStatusBits ev)
> sel = ACPI_GED_PWR_DOWN_EVT;
> } else if (ev & ACPI_NVDIMM_HOTPLUG_STATUS) {
> sel = ACPI_GED_NVDIMM_HOTPLUG_EVT;
> + } else if (ev & ACPI_CPU_HOTPLUG_STATUS) {
> + sel = ACPI_GED_CPU_HOTPLUG_EVT;
> } else {
> /* Unknown event. Return without generating interrupt. */
> warn_report("GED: Unsupported event %d. No irq injected", ev);
> @@ -400,6 +411,12 @@ static void acpi_ged_initfn(Object *obj)
> memory_region_init_io(&ged_st->regs, obj, &ged_regs_ops, ged_st,
> TYPE_ACPI_GED "-regs", ACPI_GED_REG_COUNT);
> sysbus_init_mmio(sbd, &ged_st->regs);
> +
> + memory_region_init(&s->container_cpuhp, OBJECT(dev), "cpuhp container",
> + ACPI_CPU_HOTPLUG_REG_LEN);
> + sysbus_init_mmio(SYS_BUS_DEVICE(dev), &s->container_cpuhp);
> + cpu_hotplug_hw_init(&s->container_cpuhp, OBJECT(dev),
> + &s->cpuhp_state, 0);
> }
> static void acpi_ged_class_init(ObjectClass *class, void *data)
> diff --git a/include/hw/acpi/generic_event_device.h b/include/hw/acpi/generic_event_device.h
> index ba84ce0214..90fc41cbb8 100644
> --- a/include/hw/acpi/generic_event_device.h
> +++ b/include/hw/acpi/generic_event_device.h
> @@ -60,6 +60,7 @@
> #define HW_ACPI_GENERIC_EVENT_DEVICE_H
>
> #include "hw/sysbus.h"
> +#include "hw/acpi/cpu_hotplug.h"
> #include "hw/acpi/memory_hotplug.h"
> #include "hw/acpi/ghes.h"
> #include "qom/object.h"
> @@ -95,6 +96,7 @@ OBJECT_DECLARE_SIMPLE_TYPE(AcpiGedState, ACPI_GED)
> #define ACPI_GED_MEM_HOTPLUG_EVT 0x1
> #define ACPI_GED_PWR_DOWN_EVT 0x2
> #define ACPI_GED_NVDIMM_HOTPLUG_EVT 0x4
> +#define ACPI_GED_CPU_HOTPLUG_EVT 0x8
>
> typedef struct GEDState {
> MemoryRegion evt;
> @@ -106,6 +108,8 @@ struct AcpiGedState {
> SysBusDevice parent_obj;
> MemHotplugState memhp_state;
> MemoryRegion container_memhp;
> + CPUHotplugState cpuhp_state;
> + MemoryRegion container_cpuhp;
> GEDState ged_state;
> uint32_t ged_event_bitmap;
> qemu_irq irq;
© 2016 - 2024 Red Hat, Inc.