Hi Salil,

On 9/26/23 20:04, Salil Mehta wrote:
ACPI GED(as described in the ACPI 6.2 spec) can be used to generate ACPI events
when OSPM/guest receives an interrupt listed in the _CRS object of GED. OSPM
then maps or demultiplexes the event by evaluating _EVT method.

This change adds the support of cpu hotplug event initialization in the
existing GED framework.

Co-developed-by: Salil Mehta <salil.me...@huawei.com>
Signed-off-by: Salil Mehta <salil.me...@huawei.com>
Co-developed-by: Keqian Zhu <zhukeqi...@huawei.com>
Signed-off-by: Keqian Zhu <zhukeqi...@huawei.com>
Signed-off-by: Salil Mehta <salil.me...@huawei.com>
---
  hw/acpi/generic_event_device.c         | 8 ++++++++
  include/hw/acpi/generic_event_device.h | 5 +++++
  2 files changed, 13 insertions(+)


It looks a bit strange you're co-developing the patch with yourself.
It seems all patches follow this particular pattern. I could be changed
to:

Co-developed-by: Keqian Zhu <zhukeqi...@huawei.com>
Signed-off-by: Keqian Zhu <zhukeqi...@huawei.com>
Signed-off-by: Salil Mehta <salil.me...@huawei.com>

The code changes look good to me with the following nits addressed:

Reviewed-by: Gavin Shan <gs...@redhat.com>

diff --git a/hw/acpi/generic_event_device.c b/hw/acpi/generic_event_device.c
index a3d31631fe..d2fa1d0e4a 100644
--- a/hw/acpi/generic_event_device.c
+++ b/hw/acpi/generic_event_device.c
@@ -25,6 +25,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,
  };

Can we move ACPI_GED_CPU_HOTPLUG_EVT ahead of ACPI_GED_MEM_HOTPLUG_EVT?

  /*
@@ -400,6 +401,13 @@ 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);
+
+    s->cpuhp.device = OBJECT(s);
+    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 d831bbd889..d0a5a43abf 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"
@@ -97,6 +98,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

#define ACPI_GED_CPU_HOTPLUG_EVT  0x1
#define ACPI_GED_MEM_HOTPLUG_EVT  0x2
  :

If the adjustment is friendly to live migration.

  typedef struct GEDState {
      MemoryRegion evt;
@@ -108,6 +110,9 @@ struct AcpiGedState {
      SysBusDevice parent_obj;
      MemHotplugState memhp_state;
      MemoryRegion container_memhp;
+    CPUHotplugState cpuhp_state;
+    MemoryRegion container_cpuhp;
+    AcpiCpuHotplug cpuhp;
      GEDState ged_state;
      uint32_t ged_event_bitmap;
      qemu_irq irq;

Thanks,
Gavin


Reply via email to