Hello,
Can we assume that every machine type will have all the features which a GED Device can multiplex present together? like will Memory and CPU Hotplug makes sense for all the type of machines?

If answer is no, then shouldn't every machine type override the base GED type and define it own versions of instance_init() function? AFAICS, GED can multiplex non-hotplug events as well.

To support CPU Htoplug on ARM platforms we are using GED but x86/microvm does not supports hot-plugging and while creating TYPE_GED_DEVICE it will end up initializing CPU Hotplug regions and code as well. This is far from clean.

Beside 'qtest' fails for x86/microvm machine type because 'possible_cpus_arch_ids' is not defined for x86/microvm so we get errors like below:

stderr:
qemu-system-x86_64: ../hw/acpi/cpu.c:224: cpu_hotplug_hw_init: Assertion `mc->possible_cpu_arch_ids' failed.
Broken pipe
../tests/qtest/libqtest.c:200: kill_qemu() detected QEMU death from signal 6 (Aborted) (core dumped)

Above can be avoided if cpu_hotplug_hw_init() does not gets called for x86/microvm machine.

ARM can have its own version of generic_event_device_arm64.c with its own version of instance_init() having a call to cpu_hotplug_hw_init().

Maybe I have missed something here?


Many thanks
Salil.


On 05/10/2023 04:44, Michael S. Tsirkin wrote:
From: Bernhard Beschow <shen...@gmail.com>

Now that TYPE_ACPI_GED_X86 doesn't assign AcpiDeviceIfClass::madt_cpu any more
it is the same as TYPE_ACPI_GED.

Signed-off-by: Bernhard Beschow <shen...@gmail.com>
Reviewed-by: Philippe Mathieu-Daudé <phi...@linaro.org>
Message-Id: <20230908084234.17642-6-shen...@gmail.com>
Reviewed-by: Michael S. Tsirkin <m...@redhat.com>
Signed-off-by: Michael S. Tsirkin <m...@redhat.com>
---
  include/hw/acpi/generic_event_device.h |  2 --
  hw/i386/generic_event_device_x86.c     | 27 --------------------------
  hw/i386/microvm.c                      |  2 +-
  hw/i386/meson.build                    |  1 -
  4 files changed, 1 insertion(+), 31 deletions(-)
  delete mode 100644 hw/i386/generic_event_device_x86.c

diff --git a/include/hw/acpi/generic_event_device.h 
b/include/hw/acpi/generic_event_device.h
index d831bbd889..ba84ce0214 100644
--- a/include/hw/acpi/generic_event_device.h
+++ b/include/hw/acpi/generic_event_device.h
@@ -69,8 +69,6 @@
  #define TYPE_ACPI_GED "acpi-ged"
  OBJECT_DECLARE_SIMPLE_TYPE(AcpiGedState, ACPI_GED)
-#define TYPE_ACPI_GED_X86 "acpi-ged-x86"
-
  #define ACPI_GED_EVT_SEL_OFFSET    0x0
  #define ACPI_GED_EVT_SEL_LEN       0x4
diff --git a/hw/i386/generic_event_device_x86.c b/hw/i386/generic_event_device_x86.c
deleted file mode 100644
index 8fc233e1f1..0000000000
--- a/hw/i386/generic_event_device_x86.c
+++ /dev/null
@@ -1,27 +0,0 @@
-/*
- * x86 variant of the generic event device for hw reduced acpi
- *
- * This program is free software; you can redistribute it and/or modify it
- * under the terms and conditions of the GNU General Public License,
- * version 2 or later, as published by the Free Software Foundation.
- */
-
-#include "qemu/osdep.h"
-#include "hw/acpi/generic_event_device.h"
-
-static const TypeInfo acpi_ged_x86_info = {
-    .name          = TYPE_ACPI_GED_X86,
-    .parent        = TYPE_ACPI_GED,
-    .interfaces = (InterfaceInfo[]) {
-        { TYPE_HOTPLUG_HANDLER },
-        { TYPE_ACPI_DEVICE_IF },
-        { }
-    }
-};
-
-static void acpi_ged_x86_register_types(void)
-{
-    type_register_static(&acpi_ged_x86_info);
-}
-
-type_init(acpi_ged_x86_register_types)
diff --git a/hw/i386/microvm.c b/hw/i386/microvm.c
index 8deeb62774..b9c93039e2 100644
--- a/hw/i386/microvm.c
+++ b/hw/i386/microvm.c
@@ -204,7 +204,7 @@ static void microvm_devices_init(MicrovmMachineState *mms)
/* Optional and legacy devices */
      if (x86_machine_is_acpi_enabled(x86ms)) {
-        DeviceState *dev = qdev_new(TYPE_ACPI_GED_X86);
+        DeviceState *dev = qdev_new(TYPE_ACPI_GED);
          qdev_prop_set_uint32(dev, "ged-event", ACPI_GED_PWR_DOWN_EVT);
          sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, GED_MMIO_BASE);
          /* sysbus_mmio_map(SYS_BUS_DEVICE(dev), 1, GED_MMIO_BASE_MEMHP); */
diff --git a/hw/i386/meson.build b/hw/i386/meson.build
index cfdbfdcbcb..ff879069c9 100644
--- a/hw/i386/meson.build
+++ b/hw/i386/meson.build
@@ -20,7 +20,6 @@ i386_ss.add(when: 'CONFIG_SGX', if_true: 
files('sgx-epc.c','sgx.c'),
                                  if_false: files('sgx-stub.c'))
i386_ss.add(when: 'CONFIG_ACPI', if_true: files('acpi-common.c'))
-i386_ss.add(when: 'CONFIG_ACPI_HW_REDUCED', if_true: 
files('generic_event_device_x86.c'))
  i386_ss.add(when: 'CONFIG_PC', if_true: files(
    'pc.c',
    'pc_sysfw.c',


Reply via email to