[PATCH 0/8] Adds CPU hot-plug support to Loongarch

[PATCH 0/8] Adds CPU hot-plug support to Loongarch

[xianglai li]

Hello everyone, We refer to the implementation of ARM CPU
Hot-Plug to add GED-based CPU Hot-Plug support to Loongarch.

The first 4 patches are changes to the QEMU common code,
including adding GED support for CPU Hot-Plug, updating
the ACPI table creation process, and adding qdev_disconnect_gpio_out_named
and cpu_address_space_destroy interfaces to release resources
when CPU un-plug.

The last four patches are Loongarch architecture-related,
and the modifications include the definition of the hook
function related to the CPU Hot-(UN)Plug, the allocation
and release of CPU resources when CPU Hot-(UN)Plug,
the creation process of updating the ACPI table,
and finally the custom switch for the CPU Hot-Plug.

xianglai li (8):
  Update ACPI GED framework to support vcpu hot-(un)plug
  Update CPUs AML with cpu-(ctrl)dev change
  Introduced a new function to disconnect GPIO connections
  Introduce the CPU address space destruction function
  Adds basic CPU hot-(un)plug support for Loongarch
  Add support of *unrealize* for loongarch cpu
  Update the ACPI table for the Loongarch CPU
  Turn on CPU hot-(un)plug customization for loongarch

 .../devices/loongarch64-softmmu/default.mak   |   1 +
 hw/acpi/acpi-cpu-hotplug-stub.c               |  15 +
 hw/acpi/cpu.c                                 |  37 +-
 hw/acpi/generic_event_device.c                |  33 ++
 hw/core/gpio.c                                |   4 +-
 hw/i386/acpi-build.c                          |   2 +-
 hw/loongarch/acpi-build.c                     |  35 +-
 hw/loongarch/generic_event_device_loongarch.c |  36 ++
 hw/loongarch/meson.build                      |   2 +-
 hw/loongarch/virt.c                           | 381 +++++++++++++++++-
 include/exec/cpu-common.h                     |   8 +
 include/hw/acpi/cpu.h                         |   5 +-
 include/hw/acpi/cpu_hotplug.h                 |  10 +
 include/hw/acpi/generic_event_device.h        |   6 +
 include/hw/core/cpu.h                         |   1 +
 include/hw/loongarch/virt.h                   |  11 +-
 include/hw/qdev-core.h                        |   2 +
 softmmu/physmem.c                             |  24 ++
 target/loongarch/cpu.c                        |  33 ++
 target/loongarch/cpu.h                        |   5 +
 20 files changed, 615 insertions(+), 36 deletions(-)
 create mode 100644 hw/loongarch/generic_event_device_loongarch.c

-- 
2.39.1

[PATCH 1/8] Update ACPI GED framework to support vcpu hot-(un)plug

[xianglai li]

ACPI GED shall be used to convey to the guest kernel about any cpu hot-(un)plug
events. Therefore, existing ACPI GED framework inside QEMU needs to be enhanced
to support CPU hot-(un)plug state and events.

Cc: Xiaojuan Yang <yangxiaojuan@loongson.cn>
Cc: Song Gao <gaosong@loongson.cn>
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Igor Mammedov <imammedo@redhat.com>
Cc: Ani Sinha <anisinha@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Richard Henderson <richard.henderson@linaro.org>
Cc: Eduardo Habkost <eduardo@habkost.net>
Cc: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
Cc: "Philippe Mathieu-Daudé" <philmd@linaro.org>
Cc: Yanan Wang <wangyanan55@huawei.com>
Cc: "Daniel P. Berrangé" <berrange@redhat.com>
Cc: Peter Xu <peterx@redhat.com>
Cc: David Hildenbrand <david@redhat.com>
Signed-off-by: xianglai li <lixianglai@loongson.cn>
---
 hw/acpi/acpi-cpu-hotplug-stub.c        |  6 +++++
 hw/acpi/cpu.c                          |  7 ------
 hw/acpi/generic_event_device.c         | 33 ++++++++++++++++++++++++++
 include/hw/acpi/cpu_hotplug.h          | 10 ++++++++
 include/hw/acpi/generic_event_device.h |  6 +++++
 5 files changed, 55 insertions(+), 7 deletions(-)

diff --git a/hw/acpi/acpi-cpu-hotplug-stub.c b/hw/acpi/acpi-cpu-hotplug-stub.c
index 3fc4b14c26..2aec90d968 100644
--- a/hw/acpi/acpi-cpu-hotplug-stub.c
+++ b/hw/acpi/acpi-cpu-hotplug-stub.c
@@ -24,6 +24,12 @@ void acpi_cpu_ospm_status(CPUHotplugState *cpu_st, ACPIOSTInfoList ***list)
     return;
 }
 
+void cpu_hotplug_hw_init(MemoryRegion *as, Object *owner,
+                         CPUHotplugState *state, hwaddr base_addr)
+{
+    return;
+}
+
 void acpi_cpu_plug_cb(HotplugHandler *hotplug_dev,
                       CPUHotplugState *cpu_st, DeviceState *dev, Error **errp)
 {
diff --git a/hw/acpi/cpu.c b/hw/acpi/cpu.c
index 19c154d78f..6897c8789a 100644
--- a/hw/acpi/cpu.c
+++ b/hw/acpi/cpu.c
@@ -6,13 +6,6 @@
 #include "trace.h"
 #include "sysemu/numa.h"
 
-#define ACPI_CPU_HOTPLUG_REG_LEN 12
-#define ACPI_CPU_SELECTOR_OFFSET_WR 0
-#define ACPI_CPU_FLAGS_OFFSET_RW 4
-#define ACPI_CPU_CMD_OFFSET_WR 5
-#define ACPI_CPU_CMD_DATA_OFFSET_RW 8
-#define ACPI_CPU_CMD_DATA2_OFFSET_R 0
-
 #define OVMF_CPUHP_SMI_CMD 4
 
 enum {
diff --git a/hw/acpi/generic_event_device.c b/hw/acpi/generic_event_device.c
index a3d31631fe..c5a70957b4 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,
 };
 
 /*
@@ -117,6 +119,10 @@ 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_CPU_HOTPLUG_EVT:
+                aml_append(if_ctx, aml_call0(ACPI_CPU_CONTAINER "."
+                                             ACPI_CPU_SCAN_METHOD));
+                break;
             default:
                 /*
                  * Please make sure all the events in ged_supported_events[]
@@ -234,6 +240,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 +256,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 +271,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 +284,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 +299,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);
@@ -318,6 +333,16 @@ static const VMStateDescription vmstate_memhp_state = {
     }
 };
 
+static const VMStateDescription vmstate_cpuhp_state = {
+    .name = "acpi-ged/cpuhp",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .fields      = (VMStateField[]) {
+        VMSTATE_CPU_HOTPLUG(cpuhp_state, AcpiGedState),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
 static const VMStateDescription vmstate_ged_state = {
     .name = "acpi-ged-state",
     .version_id = 1,
@@ -366,6 +391,7 @@ static const VMStateDescription vmstate_acpi_ged = {
     },
     .subsections = (const VMStateDescription * []) {
         &vmstate_memhp_state,
+        &vmstate_cpuhp_state,
         &vmstate_ghes_state,
         NULL
     }
@@ -400,6 +426,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/cpu_hotplug.h b/include/hw/acpi/cpu_hotplug.h
index 3b932abbbb..afee1ab996 100644
--- a/include/hw/acpi/cpu_hotplug.h
+++ b/include/hw/acpi/cpu_hotplug.h
@@ -19,6 +19,16 @@
 #include "hw/hotplug.h"
 #include "hw/acpi/cpu.h"
 
+#define ACPI_CPU_HOTPLUG_REG_LEN 12
+#define ACPI_CPU_SELECTOR_OFFSET_WR 0
+#define ACPI_CPU_FLAGS_OFFSET_RW 4
+#define ACPI_CPU_CMD_OFFSET_WR 5
+#define ACPI_CPU_CMD_DATA_OFFSET_RW 8
+#define ACPI_CPU_CMD_DATA2_OFFSET_R 0
+
+#define ACPI_CPU_SCAN_METHOD "CSCN"
+#define ACPI_CPU_CONTAINER "\\_SB.CPUS"
+
 typedef struct AcpiCpuHotplug {
     Object *device;
     MemoryRegion io;
diff --git a/include/hw/acpi/generic_event_device.h b/include/hw/acpi/generic_event_device.h
index d831bbd889..2923bd9d82 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"
@@ -70,6 +71,7 @@
 OBJECT_DECLARE_SIMPLE_TYPE(AcpiGedState, ACPI_GED)
 
 #define TYPE_ACPI_GED_X86 "acpi-ged-x86"
+#define TYPE_ACPI_GED_LOONGARCH "acpi-ged-loongarch"
 
 #define ACPI_GED_EVT_SEL_OFFSET    0x0
 #define ACPI_GED_EVT_SEL_LEN       0x4
@@ -97,6 +99,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;
@@ -108,6 +111,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;
-- 
2.39.1

[PATCH 2/8] Update CPUs AML with cpu-(ctrl)dev change

[xianglai li]

CPUs Control device(\\_SB.PCI0) register interface for the x86 arch is based on
PCI and is IO port based and hence existing cpus AML code assumes _CRS objects
would evaluate to a system resource which describes IO Port address. But on LOONGARCH
arch CPUs control device(\\_SB.PRES) register interface is memory-mapped hence
_CRS object should evaluate to system resource which describes memory-mapped
base address.

This cpus AML code change updates the existing inerface of the build cpus AML
function to accept both IO/MEMORY type regions and update the _CRS object
correspondingly.

Cc: Xiaojuan Yang <yangxiaojuan@loongson.cn>
Cc: Song Gao <gaosong@loongson.cn>
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Igor Mammedov <imammedo@redhat.com>
Cc: Ani Sinha <anisinha@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Richard Henderson <richard.henderson@linaro.org>
Cc: Eduardo Habkost <eduardo@habkost.net>
Cc: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
Cc: "Philippe Mathieu-Daudé" <philmd@linaro.org>
Cc: Yanan Wang <wangyanan55@huawei.com>
Cc: "Daniel P. Berrangé" <berrange@redhat.com>
Cc: Peter Xu <peterx@redhat.com>
Cc: David Hildenbrand <david@redhat.com>
Signed-off-by: xianglai li <lixianglai@loongson.cn>
---
 hw/acpi/cpu.c         | 30 +++++++++++++++++++++++-------
 hw/i386/acpi-build.c  |  2 +-
 include/hw/acpi/cpu.h |  5 +++--
 3 files changed, 27 insertions(+), 10 deletions(-)

diff --git a/hw/acpi/cpu.c b/hw/acpi/cpu.c
index 6897c8789a..3b945a1a40 100644
--- a/hw/acpi/cpu.c
+++ b/hw/acpi/cpu.c
@@ -5,6 +5,7 @@
 #include "qapi/qapi-events-acpi.h"
 #include "trace.h"
 #include "sysemu/numa.h"
+#include "hw/acpi/cpu_hotplug.h"
 
 #define OVMF_CPUHP_SMI_CMD 4
 
@@ -331,9 +332,10 @@ const VMStateDescription vmstate_cpu_hotplug = {
 #define CPU_FW_EJECT_EVENT "CEJF"
 
 void build_cpus_aml(Aml *table, MachineState *machine, CPUHotplugFeatures opts,
-                    hwaddr io_base,
+                    hwaddr mmap_io_base,
                     const char *res_root,
-                    const char *event_handler_method)
+                    const char *event_handler_method,
+                    AmlRegionSpace rs)
 {
     Aml *ifctx;
     Aml *field;
@@ -360,14 +362,28 @@ void build_cpus_aml(Aml *table, MachineState *machine, CPUHotplugFeatures opts,
         aml_append(cpu_ctrl_dev, aml_mutex(CPU_LOCK, 0));
 
         crs = aml_resource_template();
-        aml_append(crs, aml_io(AML_DECODE16, io_base, io_base, 1,
+        if (rs == AML_SYSTEM_IO) {
+            aml_append(crs, aml_io(AML_DECODE16, mmap_io_base, mmap_io_base, 1,
                                ACPI_CPU_HOTPLUG_REG_LEN));
+        } else {
+            aml_append(crs, aml_memory32_fixed(mmap_io_base,
+                               ACPI_CPU_HOTPLUG_REG_LEN, AML_READ_WRITE));
+        }
+
         aml_append(cpu_ctrl_dev, aml_name_decl("_CRS", crs));
 
-        /* declare CPU hotplug MMIO region with related access fields */
-        aml_append(cpu_ctrl_dev,
-            aml_operation_region("PRST", AML_SYSTEM_IO, aml_int(io_base),
-                                 ACPI_CPU_HOTPLUG_REG_LEN));
+        if (rs == AML_SYSTEM_IO) {
+            /* declare CPU hotplug MMIO region with related access fields */
+            aml_append(cpu_ctrl_dev,
+                aml_operation_region("PRST", AML_SYSTEM_IO,
+                                             aml_int(mmap_io_base),
+                                             ACPI_CPU_HOTPLUG_REG_LEN));
+        } else {
+            aml_append(cpu_ctrl_dev,
+                aml_operation_region("PRST", AML_SYSTEM_MEMORY,
+                                             aml_int(mmap_io_base),
+                                             ACPI_CPU_HOTPLUG_REG_LEN));
+        }
 
         field = aml_field("PRST", AML_BYTE_ACC, AML_NOLOCK,
                           AML_WRITE_AS_ZEROS);
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 9c74fa17ad..5d02690593 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -1548,7 +1548,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
             .fw_unplugs_cpu = pm->smi_on_cpu_unplug,
         };
         build_cpus_aml(dsdt, machine, opts, pm->cpu_hp_io_base,
-                       "\\_SB.PCI0", "\\_GPE._E02");
+                       "\\_SB.PCI0", "\\_GPE._E02", AML_SYSTEM_IO);
     }
 
     if (pcms->memhp_io_base && nr_mem) {
diff --git a/include/hw/acpi/cpu.h b/include/hw/acpi/cpu.h
index 999caaf510..cddea78333 100644
--- a/include/hw/acpi/cpu.h
+++ b/include/hw/acpi/cpu.h
@@ -56,9 +56,10 @@ typedef struct CPUHotplugFeatures {
 } CPUHotplugFeatures;
 
 void build_cpus_aml(Aml *table, MachineState *machine, CPUHotplugFeatures opts,
-                    hwaddr io_base,
+                    hwaddr mmap_io_base,
                     const char *res_root,
-                    const char *event_handler_method);
+                    const char *event_handler_method,
+                    AmlRegionSpace rs);
 
 void acpi_cpu_ospm_status(CPUHotplugState *cpu_st, ACPIOSTInfoList ***list);
 
-- 
2.39.1

[PATCH 3/8] Introduced a new function to disconnect GPIO connections

[xianglai li]

It introduces a new function to unwire the
vcpu<->exioi interrupts for the vcpu hot-(un)plug cases.

Cc: Xiaojuan Yang <yangxiaojuan@loongson.cn>
Cc: Song Gao <gaosong@loongson.cn>
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Igor Mammedov <imammedo@redhat.com>
Cc: Ani Sinha <anisinha@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Richard Henderson <richard.henderson@linaro.org>
Cc: Eduardo Habkost <eduardo@habkost.net>
Cc: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
Cc: "Philippe Mathieu-Daudé" <philmd@linaro.org>
Cc: Yanan Wang <wangyanan55@huawei.com>
Cc: "Daniel P. Berrangé" <berrange@redhat.com>
Cc: Peter Xu <peterx@redhat.com>
Cc: David Hildenbrand <david@redhat.com>
Signed-off-by: xianglai li <lixianglai@loongson.cn>
---
 hw/core/gpio.c         | 4 ++--
 include/hw/qdev-core.h | 2 ++
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/hw/core/gpio.c b/hw/core/gpio.c
index 80d07a6ec9..4fc6409545 100644
--- a/hw/core/gpio.c
+++ b/hw/core/gpio.c
@@ -143,8 +143,8 @@ qemu_irq qdev_get_gpio_out_connector(DeviceState *dev, const char *name, int n)
 
 /* disconnect a GPIO output, returning the disconnected input (if any) */
 
-static qemu_irq qdev_disconnect_gpio_out_named(DeviceState *dev,
-                                               const char *name, int n)
+qemu_irq qdev_disconnect_gpio_out_named(DeviceState *dev,
+                                        const char *name, int n)
 {
     char *propname = g_strdup_printf("%s[%d]",
                                      name ? name : "unnamed-gpio-out", n);
diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index 884c726a87..992f5419fa 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -739,6 +739,8 @@ qemu_irq qdev_get_gpio_out_connector(DeviceState *dev, const char *name, int n);
  */
 qemu_irq qdev_intercept_gpio_out(DeviceState *dev, qemu_irq icpt,
                                  const char *name, int n);
+qemu_irq qdev_disconnect_gpio_out_named(DeviceState *dev,
+                                               const char *name, int n);
 
 BusState *qdev_get_child_bus(DeviceState *dev, const char *name);
 
-- 
2.39.1

[PATCH 4/8] Introduce the CPU address space destruction function

[xianglai li]

Introduce new functions to destroy CPU address space resources
for cpu hot-(un)plug.

Cc: Xiaojuan Yang <yangxiaojuan@loongson.cn>
Cc: Song Gao <gaosong@loongson.cn>
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Igor Mammedov <imammedo@redhat.com>
Cc: Ani Sinha <anisinha@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Richard Henderson <richard.henderson@linaro.org>
Cc: Eduardo Habkost <eduardo@habkost.net>
Cc: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
Cc: "Philippe Mathieu-Daudé" <philmd@linaro.org>
Cc: Yanan Wang <wangyanan55@huawei.com>
Cc: "Daniel P. Berrangé" <berrange@redhat.com>
Cc: Peter Xu <peterx@redhat.com>
Cc: David Hildenbrand <david@redhat.com>
Signed-off-by: xianglai li <lixianglai@loongson.cn>
---
 include/exec/cpu-common.h |  8 ++++++++
 include/hw/core/cpu.h     |  1 +
 softmmu/physmem.c         | 24 ++++++++++++++++++++++++
 3 files changed, 33 insertions(+)

diff --git a/include/exec/cpu-common.h b/include/exec/cpu-common.h
index 87dc9a752c..27cd4d32b1 100644
--- a/include/exec/cpu-common.h
+++ b/include/exec/cpu-common.h
@@ -120,6 +120,14 @@ size_t qemu_ram_pagesize_largest(void);
  */
 void cpu_address_space_init(CPUState *cpu, int asidx,
                             const char *prefix, MemoryRegion *mr);
+/**
+ * cpu_address_space_destroy:
+ * @cpu: CPU for which address space needs to be destroyed
+ * @asidx: integer index of this address space
+ *
+ * Note that with KVM only one address space is supported.
+ */
+void cpu_address_space_destroy(CPUState *cpu, int asidx);
 
 void cpu_physical_memory_rw(hwaddr addr, void *buf,
                             hwaddr len, bool is_write);
diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
index fdcbe87352..d6d68dac12 100644
--- a/include/hw/core/cpu.h
+++ b/include/hw/core/cpu.h
@@ -366,6 +366,7 @@ struct CPUState {
     QSIMPLEQ_HEAD(, qemu_work_item) work_list;
 
     CPUAddressSpace *cpu_ases;
+    int cpu_ases_ref_count;
     int num_ases;
     AddressSpace *as;
     MemoryRegion *memory;
diff --git a/softmmu/physmem.c b/softmmu/physmem.c
index 3df73542e1..f4545b4508 100644
--- a/softmmu/physmem.c
+++ b/softmmu/physmem.c
@@ -762,6 +762,7 @@ void cpu_address_space_init(CPUState *cpu, int asidx,
 
     if (!cpu->cpu_ases) {
         cpu->cpu_ases = g_new0(CPUAddressSpace, cpu->num_ases);
+        cpu->cpu_ases_ref_count = cpu->num_ases;
     }
 
     newas = &cpu->cpu_ases[asidx];
@@ -775,6 +776,29 @@ void cpu_address_space_init(CPUState *cpu, int asidx,
     }
 }
 
+void cpu_address_space_destroy(CPUState *cpu, int asidx)
+{
+    CPUAddressSpace *cpuas;
+
+    assert(asidx < cpu->num_ases);
+    assert(asidx == 0 || !kvm_enabled());
+    assert(cpu->cpu_ases);
+
+    cpuas = &cpu->cpu_ases[asidx];
+    if (tcg_enabled()) {
+        memory_listener_unregister(&cpuas->tcg_as_listener);
+    }
+
+    address_space_destroy(cpuas->as);
+
+    cpu->cpu_ases_ref_count--;
+    if (cpu->cpu_ases_ref_count == 0) {
+        g_free(cpu->cpu_ases);
+        cpu->cpu_ases = NULL;
+    }
+
+}
+
 AddressSpace *cpu_get_address_space(CPUState *cpu, int asidx)
 {
     /* Return the AddressSpace corresponding to the specified index */
-- 
2.39.1

[PATCH 5/8] Adds basic CPU hot-(un)plug support for Loongarch

[xianglai li]

1.Add CPU topology related functions
2.Add CPU hot-plug related hook functions
3.Update the in-place CPU creation process at machine initialization

Cc: Xiaojuan Yang <yangxiaojuan@loongson.cn>
Cc: Song Gao <gaosong@loongson.cn>
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Igor Mammedov <imammedo@redhat.com>
Cc: Ani Sinha <anisinha@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Richard Henderson <richard.henderson@linaro.org>
Cc: Eduardo Habkost <eduardo@habkost.net>
Cc: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
Cc: "Philippe Mathieu-Daudé" <philmd@linaro.org>
Cc: Yanan Wang <wangyanan55@huawei.com>
Cc: "Daniel P. Berrangé" <berrange@redhat.com>
Cc: Peter Xu <peterx@redhat.com>
Cc: David Hildenbrand <david@redhat.com>
Signed-off-by: xianglai li <lixianglai@loongson.cn>
---
 hw/loongarch/virt.c         | 381 ++++++++++++++++++++++++++++++++++--
 include/hw/loongarch/virt.h |  11 +-
 target/loongarch/cpu.h      |   4 +
 3 files changed, 382 insertions(+), 14 deletions(-)

diff --git a/hw/loongarch/virt.c b/hw/loongarch/virt.c
index e19b042ce8..5919389f42 100644
--- a/hw/loongarch/virt.c
+++ b/hw/loongarch/virt.c
@@ -46,6 +46,9 @@
 #include "hw/block/flash.h"
 #include "qemu/error-report.h"
 
+static int virt_get_socket_id(const MachineState *ms, int cpu_index);
+static int virt_get_core_id(const MachineState *ms, int cpu_index);
+static int virt_get_thread_id(const MachineState *ms, int cpu_index);
 
 static void virt_flash_create(LoongArchMachineState *lams)
 {
@@ -447,12 +450,12 @@ static DeviceState *create_acpi_ged(DeviceState *pch_pic, LoongArchMachineState
 {
     DeviceState *dev;
     MachineState *ms = MACHINE(lams);
-    uint32_t event = ACPI_GED_PWR_DOWN_EVT;
+    uint32_t event = ACPI_GED_PWR_DOWN_EVT | ACPI_GED_CPU_HOTPLUG_EVT;
 
     if (ms->ram_slots) {
         event |= ACPI_GED_MEM_HOTPLUG_EVT;
     }
-    dev = qdev_new(TYPE_ACPI_GED);
+    dev = qdev_new(TYPE_ACPI_GED_LOONGARCH);
     qdev_prop_set_uint32(dev, "ged-event", event);
 
     /* ged event */
@@ -461,6 +464,7 @@ static DeviceState *create_acpi_ged(DeviceState *pch_pic, LoongArchMachineState
     sysbus_mmio_map(SYS_BUS_DEVICE(dev), 1, VIRT_GED_MEM_ADDR);
     /* ged regs used for reset and power down */
     sysbus_mmio_map(SYS_BUS_DEVICE(dev), 2, VIRT_GED_REG_ADDR);
+    sysbus_mmio_map(SYS_BUS_DEVICE(dev), 3, VIRT_GED_CPUHP_ADDR);
 
     sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0,
                        qdev_get_gpio_in(pch_pic, VIRT_SCI_IRQ - VIRT_GSI_BASE));
@@ -583,6 +587,7 @@ static void loongarch_irq_init(LoongArchMachineState *lams)
 
     extioi = qdev_new(TYPE_LOONGARCH_EXTIOI);
     sysbus_realize_and_unref(SYS_BUS_DEVICE(extioi), &error_fatal);
+    lams->extioi = extioi;
 
     /*
      * The connection of interrupts:
@@ -624,11 +629,11 @@ static void loongarch_irq_init(LoongArchMachineState *lams)
                                     sysbus_mmio_get_region(SYS_BUS_DEVICE(ipi),
                                     1));
         /*
-	 * extioi iocsr memory region
-	 * only one extioi is added on loongarch virt machine
-	 * external device interrupt can only be routed to cpu 0-3
-	 */
-	if (cpu < EXTIOI_CPUS)
+         * extioi iocsr memory region
+         * only one extioi is added on loongarch virt machine
+         * external device interrupt can only be routed to cpu 0-3
+         */
+        if (cpu < EXTIOI_CPUS)
             memory_region_add_subregion(&env->system_iocsr, APIC_BASE,
                                 sysbus_mmio_get_region(SYS_BUS_DEVICE(extioi),
                                 cpu));
@@ -789,7 +794,6 @@ static void loongarch_init(MachineState *machine)
     NodeInfo *numa_info = machine->numa_state->nodes;
     int i;
     hwaddr fdt_base;
-    const CPUArchIdList *possible_cpus;
     MachineClass *mc = MACHINE_GET_CLASS(machine);
     CPUState *cpu;
     char *ramName = NULL;
@@ -810,12 +814,40 @@ static void loongarch_init(MachineState *machine)
     create_fdt(lams);
     /* Init CPUs */
 
-    possible_cpus = mc->possible_cpu_arch_ids(machine);
-    for (i = 0; i < possible_cpus->len; i++) {
-        cpu = cpu_create(machine->cpu_type);
+    mc->possible_cpu_arch_ids(machine);
+
+    for (i = 0; i < machine->smp.cpus; i++) {
+        Object *cpuobj;
+        cpuobj = object_new(machine->cpu_type);
+
+        cpu = CPU(cpuobj);
         cpu->cpu_index = i;
-        machine->possible_cpus->cpus[i].cpu = OBJECT(cpu);
+
+        object_property_set_int(cpuobj, "socket-id",
+                                virt_get_socket_id(machine, i), NULL);
+        object_property_set_int(cpuobj, "core-id",
+                                virt_get_core_id(machine, i), NULL);
+        object_property_set_int(cpuobj, "thread-id",
+                                virt_get_thread_id(machine, i), NULL);
+        /*
+         * The CPU in place at the time of machine startup will also enter
+         * the CPU hot-plug process when it is created, but at this time,
+         * the GED device has not been created, resulting in exit in the CPU
+         * hot-plug process, which can avoid the incumbent CPU repeatedly
+         * applying for resources.
+         *
+         * The interrupt resource of the in-place CPU will be requested at
+         * the current function call loongarch_irq_init().
+         *
+         * The interrupt resource of the subsequently inserted CPU will be
+         * requested in the CPU hot-plug process.
+         */
+        qdev_realize(DEVICE(cpuobj), NULL, &error_fatal);
+        object_unref(cpuobj);
     }
+
+    lams->boot_cpus = machine->smp.cpus;
+
     fdt_add_cpu_nodes(lams);
 
     /* Node0 memory */
@@ -986,11 +1018,107 @@ static void virt_mem_pre_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
     pc_dimm_pre_plug(PC_DIMM(dev), MACHINE(hotplug_dev), NULL, errp);
 }
 
+static int virt_get_cpu_id_from_cpu_topo(const MachineState *ms,
+                                            DeviceState *dev)
+{
+    int cpu_index, sock_vcpu_num, core_vcpu_num;
+    LoongArchCPU *cpu = LOONGARCH_CPU(dev);
+
+    /* calculate total logical cpus across socket/cluster/core */
+    sock_vcpu_num = cpu->socket_id * (ms->smp.threads * ms->smp.cores);
+    core_vcpu_num = cpu->core_id * ms->smp.threads;
+
+    /* get vcpu-id(logical cpu index) for this vcpu from this topology */
+    cpu_index = (sock_vcpu_num + core_vcpu_num) + cpu->thread_id;
+
+    assert(cpu_index >= 0 && cpu_index < ms->possible_cpus->len);
+
+    return cpu_index;
+}
+
+/* find cpu slot in machine->possible_cpus by core_id */
+static CPUArchId *loongarch_find_cpu_slot(MachineState *ms, uint32_t cpu_index,
+                                        int *idx)
+{
+    int index = cpu_index;
+
+    if (index >= ms->possible_cpus->len) {
+        return NULL;
+    }
+    if (idx) {
+        *idx = index;
+    }
+    return &ms->possible_cpus->cpus[index];
+}
+
+static void loongarch_cpu_pre_plug(HotplugHandler *hotplug_dev,
+                            DeviceState *dev, Error **errp)
+{
+    MachineState *ms = MACHINE(OBJECT(hotplug_dev));
+    MachineClass *mc = MACHINE_GET_CLASS(hotplug_dev);
+    LoongArchCPU *cpu = LOONGARCH_CPU(dev);
+    CPUState *cs = CPU(dev);
+    CPUArchId *cpu_slot;
+    Error *local_err = NULL;
+
+    if (dev->hotplugged && !mc->has_hotpluggable_cpus) {
+        error_setg(&local_err, "CPU hotplug not supported for this machine");
+        goto out;
+    }
+
+    /* sanity check the cpu */
+    if (!object_dynamic_cast(OBJECT(cpu), ms->cpu_type)) {
+        error_setg(&local_err, "Invalid CPU type, expected cpu type: '%s'",
+                   ms->cpu_type);
+        goto out;
+    }
+
+    if ((cpu->thread_id < 0) || (cpu->thread_id >= ms->smp.threads)) {
+        error_setg(&local_err,
+                   "Invalid thread-id %u specified, must be in range 1:%u",
+                   cpu->thread_id, ms->smp.threads - 1);
+        goto out;
+    }
+
+    if ((cpu->core_id < 0) || (cpu->core_id >= ms->smp.cores)) {
+        error_setg(&local_err,
+                   "Invalid core-id %u specified, must be in range 1:%u",
+                   cpu->core_id, ms->smp.cores);
+        goto out;
+    }
+
+    if ((cpu->socket_id < 0) || (cpu->socket_id >= ms->smp.sockets)) {
+        error_setg(&local_err,
+                   "Invalid socket-id %u specified, must be in range 1:%u",
+                   cpu->socket_id, ms->smp.sockets - 1);
+        goto out;
+    }
+
+    cs->cpu_index = virt_get_cpu_id_from_cpu_topo(ms, dev);
+
+    cpu_slot = loongarch_find_cpu_slot(ms, cs->cpu_index, NULL);
+    if (CPU(cpu_slot->cpu)) {
+        error_setg(&local_err,
+                   "cpu(id%d=%d:%d:%d) with arch-id %" PRIu64 " exists",
+                   cs->cpu_index, cpu->socket_id, cpu->core_id,
+                   cpu->thread_id, cpu_slot->arch_id);
+        goto out;
+    }
+
+    numa_cpu_pre_plug(cpu_slot, dev, &local_err);
+
+    return ;
+out:
+    error_propagate(errp, local_err);
+}
+
 static void virt_machine_device_pre_plug(HotplugHandler *hotplug_dev,
                                             DeviceState *dev, Error **errp)
 {
     if (memhp_type_supported(dev)) {
         virt_mem_pre_plug(hotplug_dev, dev, errp);
+    } else if (object_dynamic_cast(OBJECT(dev), TYPE_LOONGARCH_CPU)) {
+        loongarch_cpu_pre_plug(hotplug_dev, dev, errp);
     }
 }
 
@@ -1004,11 +1132,45 @@ static void virt_mem_unplug_request(HotplugHandler *hotplug_dev,
                                    errp);
 }
 
+static void loongarch_cpu_unplug_request(HotplugHandler *hotplug_dev,
+                                        DeviceState *dev, Error **errp)
+{
+    MachineState *machine = MACHINE(OBJECT(hotplug_dev));
+    LoongArchMachineState *lsms = LOONGARCH_MACHINE(machine);
+    Error *local_err = NULL;
+    HotplugHandlerClass *hhc;
+    LoongArchCPU *cpu = LOONGARCH_CPU(dev);
+    CPUState *cs = CPU(dev);
+
+    if (!lsms->acpi_ged) {
+        error_setg(&local_err, "CPU hot unplug not supported without ACPI");
+        goto out;
+    }
+
+    if (cs->cpu_index == 0) {
+        error_setg(&local_err,
+                   "hot-unplug of boot cpu(id%d=%d:%d:%d) not supported",
+                   cs->cpu_index, cpu->socket_id,
+                   cpu->core_id, cpu->thread_id);
+        goto out;
+    }
+
+
+    hhc = HOTPLUG_HANDLER_GET_CLASS(lsms->acpi_ged);
+    hhc->unplug_request(HOTPLUG_HANDLER(lsms->acpi_ged), dev, &local_err);
+
+    return;
+ out:
+    error_propagate(errp, local_err);
+}
+
 static void virt_machine_device_unplug_request(HotplugHandler *hotplug_dev,
                                           DeviceState *dev, Error **errp)
 {
     if (memhp_type_supported(dev)) {
         virt_mem_unplug_request(hotplug_dev, dev, errp);
+    } else if (object_dynamic_cast(OBJECT(dev), TYPE_LOONGARCH_CPU)) {
+        loongarch_cpu_unplug_request(hotplug_dev, dev, errp);
     }
 }
 
@@ -1022,11 +1184,93 @@ static void virt_mem_unplug(HotplugHandler *hotplug_dev,
     qdev_unrealize(dev);
 }
 
+static void loongarch_cpu_destroy(MachineState *machine, LoongArchCPU *cpu)
+{
+    LoongArchMachineState *lsms = LOONGARCH_MACHINE(machine);
+    CPULoongArchState *env = &cpu->env;
+    DeviceState *ipi = env->ipistate;
+    CPUState *cs = CPU(cpu);
+    unsigned int cpu_index = cs->cpu_index;
+    DeviceState *extioi = lsms->extioi;
+    int pin;
+
+    qemu_unregister_reset(reset_load_elf, DEVICE(cpu));
+
+    lsms->boot_cpus--;
+    if (lsms->fw_cfg) {
+        fw_cfg_modify_i16(lsms->fw_cfg, FW_CFG_NB_CPUS,
+                          (uint16_t)lsms->boot_cpus);
+    }
+
+    /* disconnect ipi irq to cpu irq */
+    qdev_disconnect_gpio_out_named(ipi, NULL, 0);
+    /* del IPI iocsr memory region */
+    memory_region_del_subregion(&env->system_iocsr,
+                                sysbus_mmio_get_region(SYS_BUS_DEVICE(ipi),
+                                0));
+    memory_region_del_subregion(&env->system_iocsr,
+                                sysbus_mmio_get_region(SYS_BUS_DEVICE(ipi),
+                                1));
+
+    env->ipistate = NULL;
+    object_unparent(OBJECT(ipi));
+
+    /*
+     * disconnect ext irq to the cpu irq
+     * cpu_pin[9:2] <= intc_pin[7:0]
+     */
+    if (cpu_index < EXTIOI_CPUS) {
+        for (pin = 0; pin < LS3A_INTC_IP; pin++) {
+            qdev_disconnect_gpio_out_named(extioi, NULL, (cpu_index * 8 + pin));
+        }
+    }
+
+    /*
+     * del extioi iocsr memory region
+     * only one extioi is added on loongarch virt machine
+     * external device interrupt can only be routed to cpu 0-3
+     */
+    if (cpu_index < EXTIOI_CPUS)
+        memory_region_del_subregion(&env->system_iocsr,
+                            sysbus_mmio_get_region(SYS_BUS_DEVICE(extioi),
+                            cpu_index));
+}
+
+static void loongarch_cpu_unplug(HotplugHandler *hotplug_dev,
+                                DeviceState *dev, Error **errp)
+{
+    CPUArchId *found_cpu;
+    HotplugHandlerClass *hhc;
+    Error *local_err = NULL;
+    LoongArchCPU *cpu = LOONGARCH_CPU(dev);
+    MachineState *machine = MACHINE(OBJECT(hotplug_dev));
+    LoongArchMachineState *lsms = LOONGARCH_MACHINE(machine);
+    CPUState *cs = CPU(dev);
+
+    hhc = HOTPLUG_HANDLER_GET_CLASS(lsms->acpi_ged);
+    hhc->unplug(HOTPLUG_HANDLER(lsms->acpi_ged), dev, &local_err);
+
+    if (local_err) {
+        goto out;
+    }
+
+    loongarch_cpu_destroy(machine, cpu);
+
+    found_cpu = loongarch_find_cpu_slot(MACHINE(lsms), cs->cpu_index, NULL);
+    found_cpu->cpu = NULL;
+
+    return;
+out:
+    error_propagate(errp, local_err);
+}
+
 static void virt_machine_device_unplug(HotplugHandler *hotplug_dev,
                                           DeviceState *dev, Error **errp)
 {
     if (memhp_type_supported(dev)) {
         virt_mem_unplug(hotplug_dev, dev, errp);
+    } else if (object_dynamic_cast(OBJECT(dev), TYPE_LOONGARCH_CPU)) {
+        loongarch_cpu_unplug(hotplug_dev, dev, errp);
     }
 }
 
@@ -1040,6 +1284,92 @@ static void virt_mem_plug(HotplugHandler *hotplug_dev,
                          dev, &error_abort);
 }
 
+
+static LoongArchCPU *loongarch_cpu_create(MachineState *machine,
+                                LoongArchCPU *cpu, Error **errp)
+{
+    LoongArchMachineState *lsms = LOONGARCH_MACHINE(machine);
+    CPUState *cs = CPU(cpu);
+    unsigned int cpu_index = cs->cpu_index;
+    DeviceState *cpudev = DEVICE(cpu);
+    DeviceState *extioi = lsms->extioi;
+    CPULoongArchState *env = &cpu->env;
+    DeviceState *ipi;
+    int pin;
+
+    qemu_register_reset(reset_load_elf, cpu);
+
+    lsms->boot_cpus++;
+    if (lsms->fw_cfg) {
+        fw_cfg_modify_i16(lsms->fw_cfg, FW_CFG_NB_CPUS,
+                          (uint16_t)lsms->boot_cpus);
+    }
+
+    ipi = qdev_new(TYPE_LOONGARCH_IPI);
+    sysbus_realize_and_unref(SYS_BUS_DEVICE(ipi), errp);
+
+    /* connect ipi irq to cpu irq */
+    qdev_connect_gpio_out(ipi, 0, qdev_get_gpio_in(cpudev, IRQ_IPI));
+    /* IPI iocsr memory region */
+    memory_region_add_subregion(&env->system_iocsr, SMP_IPI_MAILBOX,
+                                sysbus_mmio_get_region(SYS_BUS_DEVICE(ipi),
+                                0));
+    memory_region_add_subregion(&env->system_iocsr, MAIL_SEND_ADDR,
+                                sysbus_mmio_get_region(SYS_BUS_DEVICE(ipi),
+                                1));
+    /*
+     * extioi iocsr memory region
+     * only one extioi is added on loongarch virt machine
+     * external device interrupt can only be routed to cpu 0-3
+     */
+    if (cpu_index < EXTIOI_CPUS)
+        memory_region_add_subregion(&env->system_iocsr, APIC_BASE,
+                            sysbus_mmio_get_region(SYS_BUS_DEVICE(extioi),
+                            cpu_index));
+    env->ipistate = ipi;
+
+    /*
+     * connect ext irq to the cpu irq
+     * cpu_pin[9:2] <= intc_pin[7:0]
+     */
+    if (cpu_index < EXTIOI_CPUS) {
+        for (pin = 0; pin < LS3A_INTC_IP; pin++) {
+            qdev_connect_gpio_out(extioi, (cpu_index * 8 + pin),
+                                  qdev_get_gpio_in(cpudev, pin + 2));
+        }
+    }
+
+    return cpu;
+}
+
+static void loongarch_cpu_plug(HotplugHandler *hotplug_dev,
+                                DeviceState *dev, Error **errp)
+{
+    CPUArchId *found_cpu;
+    HotplugHandlerClass *hhc;
+    Error *local_err = NULL;
+    MachineState *machine = MACHINE(OBJECT(hotplug_dev));
+    LoongArchMachineState *lsms = LOONGARCH_MACHINE(machine);
+    LoongArchCPU *cpu = LOONGARCH_CPU(dev);
+    CPUState *cs = CPU(dev);
+
+    if (lsms->acpi_ged) {
+        loongarch_cpu_create(machine, cpu, errp);
+        hhc = HOTPLUG_HANDLER_GET_CLASS(lsms->acpi_ged);
+        hhc->plug(HOTPLUG_HANDLER(lsms->acpi_ged), dev, &local_err);
+        if (local_err) {
+            goto out;
+        }
+    }
+
+    found_cpu = loongarch_find_cpu_slot(MACHINE(lsms), cs->cpu_index, NULL);
+    found_cpu->cpu = OBJECT(dev);
+
+    return;
+out:
+    error_propagate(errp, local_err);
+}
+
 static void loongarch_machine_device_plug_cb(HotplugHandler *hotplug_dev,
                                         DeviceState *dev, Error **errp)
 {
@@ -1053,6 +1383,8 @@ static void loongarch_machine_device_plug_cb(HotplugHandler *hotplug_dev,
         }
     } else if (memhp_type_supported(dev)) {
         virt_mem_plug(hotplug_dev, dev, errp);
+    } else if (object_dynamic_cast(OBJECT(dev), TYPE_LOONGARCH_CPU)) {
+        loongarch_cpu_plug(hotplug_dev, dev, errp);
     }
 }
 
@@ -1062,16 +1394,39 @@ static HotplugHandler *virt_machine_get_hotplug_handler(MachineState *machine,
     MachineClass *mc = MACHINE_GET_CLASS(machine);
 
     if (device_is_dynamic_sysbus(mc, dev) ||
+        object_dynamic_cast(OBJECT(dev), TYPE_LOONGARCH_CPU) ||
         memhp_type_supported(dev)) {
         return HOTPLUG_HANDLER(machine);
     }
     return NULL;
 }
 
+static int virt_get_socket_id(const MachineState *ms, int cpu_index)
+{
+    assert(cpu_index >= 0 && cpu_index < ms->possible_cpus->len);
+
+    return ms->possible_cpus->cpus[cpu_index].props.socket_id;
+}
+
+static int virt_get_core_id(const MachineState *ms, int cpu_index)
+{
+    assert(cpu_index >= 0 && cpu_index < ms->possible_cpus->len);
+
+    return ms->possible_cpus->cpus[cpu_index].props.core_id;
+}
+
+static int virt_get_thread_id(const MachineState *ms, int cpu_index)
+{
+    assert(cpu_index >= 0 && cpu_index < ms->possible_cpus->len);
+
+    return ms->possible_cpus->cpus[cpu_index].props.thread_id;
+}
+
 static const CPUArchIdList *virt_possible_cpu_arch_ids(MachineState *ms)
 {
     int n;
     unsigned int max_cpus = ms->smp.max_cpus;
+    unsigned int smp_threads = ms->smp.threads;
 
     if (ms->possible_cpus) {
         assert(ms->possible_cpus->len == max_cpus);
@@ -1082,6 +1437,7 @@ static const CPUArchIdList *virt_possible_cpu_arch_ids(MachineState *ms)
                                   sizeof(CPUArchId) * max_cpus);
     ms->possible_cpus->len = max_cpus;
     for (n = 0; n < ms->possible_cpus->len; n++) {
+        ms->possible_cpus->cpus[n].vcpus_count = smp_threads;
         ms->possible_cpus->cpus[n].type = ms->cpu_type;
         ms->possible_cpus->cpus[n].arch_id = n;
 
@@ -1125,6 +1481,7 @@ static void loongarch_class_init(ObjectClass *oc, void *data)
     MachineClass *mc = MACHINE_CLASS(oc);
     HotplugHandlerClass *hc = HOTPLUG_HANDLER_CLASS(oc);
 
+    mc->has_hotpluggable_cpus = true;
     mc->desc = "Loongson-3A5000 LS7A1000 machine";
     mc->init = loongarch_init;
     mc->default_ram_size = 1 * GiB;
diff --git a/include/hw/loongarch/virt.h b/include/hw/loongarch/virt.h
index f1659655c6..9ebdba676e 100644
--- a/include/hw/loongarch/virt.h
+++ b/include/hw/loongarch/virt.h
@@ -31,6 +31,7 @@
 #define VIRT_GED_EVT_ADDR       0x100e0000
 #define VIRT_GED_MEM_ADDR       (VIRT_GED_EVT_ADDR + ACPI_GED_EVT_SEL_LEN)
 #define VIRT_GED_REG_ADDR       (VIRT_GED_MEM_ADDR + MEMORY_HOTPLUG_IO_LEN)
+#define VIRT_GED_CPUHP_ADDR     (VIRT_GED_REG_ADDR + ACPI_CPU_HOTPLUG_REG_LEN)
 
 struct LoongArchMachineState {
     /*< private >*/
@@ -42,7 +43,7 @@ struct LoongArchMachineState {
     MemoryRegion bios;
     bool         bios_loaded;
     /* State for other subsystems/APIs: */
-    FWCfgState  *fw_cfg;
+    FWCfgState   *fw_cfg;
     Notifier     machine_done;
     Notifier     powerdown_notifier;
     OnOffAuto    acpi;
@@ -50,13 +51,19 @@ struct LoongArchMachineState {
     char         *oem_table_id;
     DeviceState  *acpi_ged;
     int          fdt_size;
-    DeviceState *platform_bus_dev;
+    DeviceState  *platform_bus_dev;
     PCIBus       *pci_bus;
     PFlashCFI01  *flash;
+    DeviceState  *extioi;
+    unsigned int boot_cpus;
 };
 
 #define TYPE_LOONGARCH_MACHINE  MACHINE_TYPE_NAME("virt")
 OBJECT_DECLARE_SIMPLE_TYPE(LoongArchMachineState, LOONGARCH_MACHINE)
 bool loongarch_is_acpi_enabled(LoongArchMachineState *lams);
 void loongarch_acpi_setup(LoongArchMachineState *lams);
+void virt_madt_cpu_entry(int uid,
+                         const CPUArchIdList *apic_ids, GArray *entry,
+                         bool force_enabled);
+
 #endif
diff --git a/target/loongarch/cpu.h b/target/loongarch/cpu.h
index ed04027af1..f4439c245f 100644
--- a/target/loongarch/cpu.h
+++ b/target/loongarch/cpu.h
@@ -367,6 +367,10 @@ struct ArchCPU {
     CPUState parent_obj;
     /*< public >*/
 
+    int32_t socket_id;  /* socket-id of this VCPU */
+    int32_t core_id;    /* core-id of this VCPU */
+    int32_t thread_id;  /* thread-id of this VCPU */
+    int32_t node_id;    /* NUMA node this CPU belongs to */
     CPUNegativeOffsetState neg;
     CPULoongArchState env;
     QEMUTimer timer;
-- 
2.39.1

[PATCH 6/8] Add support of *unrealize* for loongarch cpu

[xianglai li]

1.Add the Unrealize function to the Loongarch CPU for cpu hot-(un)plug
2.Add CPU topology-related properties to the Loongarch CPU for cpu hot-(un)plug

Cc: Xiaojuan Yang <yangxiaojuan@loongson.cn>
Cc: Song Gao <gaosong@loongson.cn>
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Igor Mammedov <imammedo@redhat.com>
Cc: Ani Sinha <anisinha@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Richard Henderson <richard.henderson@linaro.org>
Cc: Eduardo Habkost <eduardo@habkost.net>
Cc: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
Cc: "Philippe Mathieu-Daudé" <philmd@linaro.org>
Cc: Yanan Wang <wangyanan55@huawei.com>
Cc: "Daniel P. Berrangé" <berrange@redhat.com>
Cc: Peter Xu <peterx@redhat.com>
Cc: David Hildenbrand <david@redhat.com>
Signed-off-by: xianglai li <lixianglai@loongson.cn>
---
 target/loongarch/cpu.c | 33 +++++++++++++++++++++++++++++++++
 target/loongarch/cpu.h |  1 +
 2 files changed, 34 insertions(+)

diff --git a/target/loongarch/cpu.c b/target/loongarch/cpu.c
index ad93ecac92..97c577820f 100644
--- a/target/loongarch/cpu.c
+++ b/target/loongarch/cpu.c
@@ -18,6 +18,7 @@
 #include "cpu-csr.h"
 #include "sysemu/reset.h"
 #include "tcg/tcg.h"
+#include "hw/qdev-properties.h"
 
 const char * const regnames[32] = {
     "r0", "r1", "r2", "r3", "r4", "r5", "r6", "r7",
@@ -540,6 +541,24 @@ static void loongarch_cpu_realizefn(DeviceState *dev, Error **errp)
     lacc->parent_realize(dev, errp);
 }
 
+static void loongarch_cpu_unrealizefn(DeviceState *dev)
+{
+    LoongArchCPUClass *mcc = LOONGARCH_CPU_GET_CLASS(dev);
+
+#ifndef CONFIG_USER_ONLY
+    CPUState *cs = CPU(dev);
+    LoongArchCPU *cpu = LOONGARCH_CPU(dev);
+    CPULoongArchState *env = &cpu->env;
+
+    cpu_remove_sync(CPU(dev));
+    cpu_address_space_destroy(cs, 0);
+    address_space_destroy(&env->address_space_iocsr);
+    memory_region_del_subregion(&env->system_iocsr, &env->iocsr_mem);
+#endif
+
+    mcc->parent_unrealize(dev);
+}
+
 #ifndef CONFIG_USER_ONLY
 static void loongarch_qemu_write(void *opaque, hwaddr addr,
                                  uint64_t val, unsigned size)
@@ -697,6 +716,15 @@ static gchar *loongarch_gdb_arch_name(CPUState *cs)
     return g_strdup("loongarch64");
 }
 
+static Property loongarch_cpu_properties[] = {
+    DEFINE_PROP_INT32("socket-id", LoongArchCPU, socket_id, 0),
+    DEFINE_PROP_INT32("core-id", LoongArchCPU, core_id, 0),
+    DEFINE_PROP_INT32("thread-id", LoongArchCPU, thread_id, 0),
+    DEFINE_PROP_INT32("node-id", LoongArchCPU, node_id, CPU_UNSET_NUMA_NODE_ID),
+
+    DEFINE_PROP_END_OF_LIST()
+};
+
 static void loongarch_cpu_class_init(ObjectClass *c, void *data)
 {
     LoongArchCPUClass *lacc = LOONGARCH_CPU_CLASS(c);
@@ -704,8 +732,12 @@ static void loongarch_cpu_class_init(ObjectClass *c, void *data)
     DeviceClass *dc = DEVICE_CLASS(c);
     ResettableClass *rc = RESETTABLE_CLASS(c);
 
+    device_class_set_props(dc, loongarch_cpu_properties);
     device_class_set_parent_realize(dc, loongarch_cpu_realizefn,
                                     &lacc->parent_realize);
+    device_class_set_parent_unrealize(dc, loongarch_cpu_unrealizefn,
+                                  &lacc->parent_unrealize);
+
     resettable_class_set_parent_phases(rc, NULL, loongarch_cpu_reset_hold, NULL,
                                        &lacc->parent_phases);
 
@@ -730,6 +762,7 @@ static void loongarch_cpu_class_init(ObjectClass *c, void *data)
 #ifdef CONFIG_TCG
     cc->tcg_ops = &loongarch_tcg_ops;
 #endif
+    dc->user_creatable = true;
 }
 
 #define DEFINE_LOONGARCH_CPU_TYPE(model, initfn) \
diff --git a/target/loongarch/cpu.h b/target/loongarch/cpu.h
index f4439c245f..32feee4fe6 100644
--- a/target/loongarch/cpu.h
+++ b/target/loongarch/cpu.h
@@ -397,6 +397,7 @@ struct LoongArchCPUClass {
     /*< public >*/
 
     DeviceRealize parent_realize;
+    DeviceUnrealize parent_unrealize;
     ResettablePhases parent_phases;
 };
 
-- 
2.39.1

[PATCH 7/8] Update the ACPI table for the Loongarch CPU

[xianglai li]

1.Create a new GED device type for Loongarch,
mount cpu_madt function to update the ACPI table
2.Update the APIC table for loongarch based on
CPU information to support CPU hot-(un)plug

Cc: Xiaojuan Yang <yangxiaojuan@loongson.cn>
Cc: Song Gao <gaosong@loongson.cn>
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Igor Mammedov <imammedo@redhat.com>
Cc: Ani Sinha <anisinha@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Richard Henderson <richard.henderson@linaro.org>
Cc: Eduardo Habkost <eduardo@habkost.net>
Cc: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
Cc: "Philippe Mathieu-Daudé" <philmd@linaro.org>
Cc: Yanan Wang <wangyanan55@huawei.com>
Cc: "Daniel P. Berrangé" <berrange@redhat.com>
Cc: Peter Xu <peterx@redhat.com>
Cc: David Hildenbrand <david@redhat.com>
Signed-off-by: xianglai li <lixianglai@loongson.cn>
---
 hw/acpi/acpi-cpu-hotplug-stub.c               |  9 +++++
 hw/loongarch/acpi-build.c                     | 35 ++++++++++++++++--
 hw/loongarch/generic_event_device_loongarch.c | 36 +++++++++++++++++++
 hw/loongarch/meson.build                      |  2 +-
 4 files changed, 79 insertions(+), 3 deletions(-)
 create mode 100644 hw/loongarch/generic_event_device_loongarch.c

diff --git a/hw/acpi/acpi-cpu-hotplug-stub.c b/hw/acpi/acpi-cpu-hotplug-stub.c
index 2aec90d968..af9fda2cf4 100644
--- a/hw/acpi/acpi-cpu-hotplug-stub.c
+++ b/hw/acpi/acpi-cpu-hotplug-stub.c
@@ -19,6 +19,15 @@ void legacy_acpi_cpu_hotplug_init(MemoryRegion *parent, Object *owner,
     return;
 }
 
+void build_cpus_aml(Aml *table, MachineState *machine, CPUHotplugFeatures opts,
+                    hwaddr mmap_io_base,
+                    const char *res_root,
+                    const char *event_handler_method,
+                    AmlRegionSpace rs)
+{
+    return;
+}
+
 void acpi_cpu_ospm_status(CPUHotplugState *cpu_st, ACPIOSTInfoList ***list)
 {
     return;
diff --git a/hw/loongarch/acpi-build.c b/hw/loongarch/acpi-build.c
index 0b62c3a2f7..312908fb2f 100644
--- a/hw/loongarch/acpi-build.c
+++ b/hw/loongarch/acpi-build.c
@@ -46,6 +46,23 @@
 #define ACPI_BUILD_DPRINTF(fmt, ...)
 #endif
 
+void virt_madt_cpu_entry(int uid,
+                         const CPUArchIdList *apic_ids,
+                         GArray *entry, bool force_enabled)
+{
+    uint32_t apic_id = apic_ids->cpus[uid].arch_id;
+    /* Flags – Local APIC Flags */
+    uint32_t flags = apic_ids->cpus[uid].cpu != NULL || force_enabled ?
+                     1 /* Enabled */ : 0;
+
+    /* Rev 1.0b, Table 5-13 Processor Local APIC Structure */
+    build_append_int_noprefix(entry, 0, 1);       /* Type */
+    build_append_int_noprefix(entry, 8, 1);       /* Length */
+    build_append_int_noprefix(entry, uid, 1);     /* ACPI Processor ID */
+    build_append_int_noprefix(entry, apic_id, 1); /* APIC ID */
+    build_append_int_noprefix(entry, flags, 4); /* Flags */
+}
+
 /* build FADT */
 static void init_common_fadt_data(AcpiFadtData *data)
 {
@@ -121,15 +138,18 @@ build_madt(GArray *table_data, BIOSLinker *linker, LoongArchMachineState *lams)
     build_append_int_noprefix(table_data, 1 /* PCAT_COMPAT */, 4); /* Flags */
 
     for (i = 0; i < arch_ids->len; i++) {
+        uint32_t flags;
+
         /* Processor Core Interrupt Controller Structure */
         arch_id = arch_ids->cpus[i].arch_id;
+        flags = arch_ids->cpus[i].cpu ? 1 : 0;
 
         build_append_int_noprefix(table_data, 17, 1);    /* Type */
         build_append_int_noprefix(table_data, 15, 1);    /* Length */
         build_append_int_noprefix(table_data, 1, 1);     /* Version */
-        build_append_int_noprefix(table_data, i + 1, 4); /* ACPI Processor ID */
+        build_append_int_noprefix(table_data, i, 4);     /* ACPI Processor ID */
         build_append_int_noprefix(table_data, arch_id, 4); /* Core ID */
-        build_append_int_noprefix(table_data, 1, 4);     /* Flags */
+        build_append_int_noprefix(table_data, flags, 4);   /* Flags */
     }
 
     /* Extend I/O Interrupt Controller Structure */
@@ -292,6 +312,17 @@ build_la_ged_aml(Aml *dsdt, MachineState *machine)
                                  AML_SYSTEM_MEMORY,
                                  VIRT_GED_MEM_ADDR);
     }
+
+    if (event & ACPI_GED_CPU_HOTPLUG_EVT) {
+        CPUHotplugFeatures opts = {
+            .acpi_1_compatible = false,
+            .has_legacy_cphp = false
+        };
+
+        build_cpus_aml(dsdt, machine, opts, VIRT_GED_CPUHP_ADDR,
+                       "\\_SB", "\\_GPE._E01", AML_SYSTEM_MEMORY);
+
+    }
     acpi_dsdt_add_power_button(dsdt);
 }
 
diff --git a/hw/loongarch/generic_event_device_loongarch.c b/hw/loongarch/generic_event_device_loongarch.c
new file mode 100644
index 0000000000..1fe550239b
--- /dev/null
+++ b/hw/loongarch/generic_event_device_loongarch.c
@@ -0,0 +1,36 @@
+/*
+ * loongarch 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"
+#include "hw/loongarch/virt.h"
+
+static void acpi_ged_loongarch_class_init(ObjectClass *class, void *data)
+{
+    AcpiDeviceIfClass *adevc = ACPI_DEVICE_IF_CLASS(class);
+
+    adevc->madt_cpu = virt_madt_cpu_entry;
+}
+
+static const TypeInfo acpi_ged_loongarch_info = {
+    .name          = TYPE_ACPI_GED_LOONGARCH,
+    .parent        = TYPE_ACPI_GED,
+    .class_init    = acpi_ged_loongarch_class_init,
+    .interfaces = (InterfaceInfo[]) {
+        { TYPE_HOTPLUG_HANDLER },
+        { TYPE_ACPI_DEVICE_IF },
+        { }
+    }
+};
+
+static void acpi_ged_loongarch_register_types(void)
+{
+    type_register_static(&acpi_ged_loongarch_info);
+}
+
+type_init(acpi_ged_loongarch_register_types)
diff --git a/hw/loongarch/meson.build b/hw/loongarch/meson.build
index c0421502ab..8d21addee3 100644
--- a/hw/loongarch/meson.build
+++ b/hw/loongarch/meson.build
@@ -3,6 +3,6 @@ loongarch_ss.add(files(
     'fw_cfg.c',
 ))
 loongarch_ss.add(when: 'CONFIG_LOONGARCH_VIRT', if_true: [files('virt.c'), fdt])
-loongarch_ss.add(when: 'CONFIG_ACPI', if_true: files('acpi-build.c'))
+loongarch_ss.add(when: 'CONFIG_ACPI', if_true: files('acpi-build.c', 'generic_event_device_loongarch.c'))
 
 hw_arch += {'loongarch': loongarch_ss}
-- 
2.39.1

[PATCH 8/8] Turn on CPU hot-(un)plug customization for loongarch

[xianglai li]

Turn on CPU hot-(un)plug custom for loongarch in the configuration file

Cc: Xiaojuan Yang <yangxiaojuan@loongson.cn>
Cc: Song Gao <gaosong@loongson.cn>
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Igor Mammedov <imammedo@redhat.com>
Cc: Ani Sinha <anisinha@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Richard Henderson <richard.henderson@linaro.org>
Cc: Eduardo Habkost <eduardo@habkost.net>
Cc: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
Cc: "Philippe Mathieu-Daudé" <philmd@linaro.org>
Cc: Yanan Wang <wangyanan55@huawei.com>
Cc: "Daniel P. Berrangé" <berrange@redhat.com>
Cc: Peter Xu <peterx@redhat.com>
Cc: David Hildenbrand <david@redhat.com>
Signed-off-by: xianglai li <lixianglai@loongson.cn>
---
 configs/devices/loongarch64-softmmu/default.mak | 1 +
 1 file changed, 1 insertion(+)

diff --git a/configs/devices/loongarch64-softmmu/default.mak b/configs/devices/loongarch64-softmmu/default.mak
index 928bc117ef..e596706fab 100644
--- a/configs/devices/loongarch64-softmmu/default.mak
+++ b/configs/devices/loongarch64-softmmu/default.mak
@@ -1,3 +1,4 @@
 # Default configuration for loongarch64-softmmu
 
 CONFIG_LOONGARCH_VIRT=y
+CONFIG_ACPI_CPU_HOTPLUG=y
-- 
2.39.1

Re: [PATCH 0/8] Adds CPU hot-plug support to Loongarch

[Gavin Shan]

Hi Xianglai,

On 7/20/23 17:15, xianglai li wrote:
> Hello everyone, We refer to the implementation of ARM CPU
> Hot-Plug to add GED-based CPU Hot-Plug support to Loongarch.
> 
> The first 4 patches are changes to the QEMU common code,
> including adding GED support for CPU Hot-Plug, updating
> the ACPI table creation process, and adding qdev_disconnect_gpio_out_named
> and cpu_address_space_destroy interfaces to release resources
> when CPU un-plug.
> 
> The last four patches are Loongarch architecture-related,
> and the modifications include the definition of the hook
> function related to the CPU Hot-(UN)Plug, the allocation
> and release of CPU resources when CPU Hot-(UN)Plug,
> the creation process of updating the ACPI table,
> and finally the custom switch for the CPU Hot-Plug.
> 

[...]

It seems the design for ARM64 hotplug has been greatly referred, but the authors
are missed to be copied if you were referring to the following repository. There
will be conflicts between those two patchsets as I can see and I'm not sure how
it can be resolved. In theory, one patchset needs to be rebased on another one
since they're sharing large amount of codes.

   https://github.com/salil-mehta/qemu.git
   (branch: virt-cpuhp-armv8/rfc-v1-port11052023.dev-1)

I took a quick scan on this series. Loongarch doesn't have ARM64's constraint due
to vGIC3 where all vCPUs's file descriptor needs to be in place. It means the possible
and not yet present vCPU needs to be visible to KVM. Without this constraint, the
implementation is simplified a lot.

Besides, we found the vCPU hotplug isn't working for TCG when Salil's series was
tested. I'm not sure if we have same issue with this series, or TCG isn't a concern
to us at all?

Thanks,
Gavin

Re: [PATCH 0/8] Adds CPU hot-plug support to Loongarch

[lixianglai]

Hi Gavin and Salil,

On 7/27/23 8:57 AM, Gavin Shan wrote:
> Hi Xianglai,
>
> On 7/20/23 17:15, xianglai li wrote:
>> Hello everyone, We refer to the implementation of ARM CPU
>> Hot-Plug to add GED-based CPU Hot-Plug support to Loongarch.
>>
>> The first 4 patches are changes to the QEMU common code,
>> including adding GED support for CPU Hot-Plug, updating
>> the ACPI table creation process, and adding 
>> qdev_disconnect_gpio_out_named
>> and cpu_address_space_destroy interfaces to release resources
>> when CPU un-plug.
>>
>> The last four patches are Loongarch architecture-related,
>> and the modifications include the definition of the hook
>> function related to the CPU Hot-(UN)Plug, the allocation
>> and release of CPU resources when CPU Hot-(UN)Plug,
>> the creation process of updating the ACPI table,
>> and finally the custom switch for the CPU Hot-Plug.
>>
>
> [...]
>
> It seems the design for ARM64 hotplug has been greatly referred, but 
> the authors
> are missed to be copied if you were referring to the following 
> repository. There
> will be conflicts between those two patchsets as I can see and I'm not 
> sure how
> it can be resolved. In theory, one patchset needs to be rebased on 
> another one
> since they're sharing large amount of codes.
>
>   https://github.com/salil-mehta/qemu.git
>   (branch: virt-cpuhp-armv8/rfc-v1-port11052023.dev-1)
>
> I took a quick scan on this series. Loongarch doesn't have ARM64's 
> constraint due
> to vGIC3 where all vCPUs's file descriptor needs to be in place. It 
> means the possible
> and not yet present vCPU needs to be visible to KVM. Without this 
> constraint, the
> implementation is simplified a lot.

We have great respect and gratitude to Salil and his team for their work 
and contributions to CPU HotPlug,

which greatly reduced the difficulty of developing CPU HotPlug in Loongarch.

So, We plan to rebase the next version of patch based on their code.


Hi Salil,

I estimate that it will take quite some time for your patch series to 
merge in,

if allowed, can you merge your patch series changes to the common code 
section into the community first,

so that our code can be rebase and merged.


>
> Besides, we found the vCPU hotplug isn't working for TCG when Salil's 
> series was
> tested. I'm not sure if we have same issue with this series, or TCG 
> isn't a concern
> to us at all?

At present, QEMU only supports TCG under Loongarch,

and we test CPU HotPlug is also carried out under QEMU TCG,

and CPU HotPlug can work normally when testing.

Of course, we are also very happy to see you testing CPU hotplug under 
Loongarch,

which can find more problems and help us improve our patch.

>
> Thanks,
> Gavin

Thanks,

xianglai li

RE: [PATCH 0/8] Adds CPU hot-plug support to Loongarch

[Salil Mehta via]

Hello,

> From: lixianglai <lixianglai@loongson.cn>
> Sent: Thursday, July 27, 2023 3:14 AM
> To: Gavin Shan <gshan@redhat.com>; qemu-devel@nongnu.org; Salil Mehta
> <salil.mehta@huawei.com>; zhukeqian <zhukeqian1@huawei.com>; Bibo Mao
> <maobibo@loongson.cn>
> Subject: Re: [PATCH 0/8] Adds CPU hot-plug support to Loongarch
> 
> Hi Gavin and Salil,
> 
> On 7/27/23 8:57 AM, Gavin Shan wrote:
> > Hi Xianglai,
> >
> > On 7/20/23 17:15, xianglai li wrote:
> >> Hello everyone, We refer to the implementation of ARM CPU
> >> Hot-Plug to add GED-based CPU Hot-Plug support to Loongarch.
> >>
> >> The first 4 patches are changes to the QEMU common code,
> >> including adding GED support for CPU Hot-Plug, updating
> >> the ACPI table creation process, and adding
> >> qdev_disconnect_gpio_out_named
> >> and cpu_address_space_destroy interfaces to release resources
> >> when CPU un-plug.
> >>
> >> The last four patches are Loongarch architecture-related,
> >> and the modifications include the definition of the hook
> >> function related to the CPU Hot-(UN)Plug, the allocation
> >> and release of CPU resources when CPU Hot-(UN)Plug,
> >> the creation process of updating the ACPI table,
> >> and finally the custom switch for the CPU Hot-Plug.
> >>
> >
> > [...]
> >
> > It seems the design for ARM64 hotplug has been greatly referred, but the authors
> > are missed to be copied if you were referring to the following repository. There
> > will be conflicts between those two patchsets as I can see and I'm not sure how
> > it can be resolved. In theory, one patchset needs to be rebased on another one
> > since they're sharing large amount of codes.
> >
> >   https://github.com/salil-mehta/qemu.git
> >   (branch: virt-cpuhp-armv8/rfc-v1-port11052023.dev-1)
> >
> > I took a quick scan on this series. Loongarch doesn't have ARM64's constraint due
> > to vGIC3 where all vCPUs's file descriptor needs to be in place. It means the possible
> > and not yet present vCPU needs to be visible to KVM. Without this constraint, the
> > implementation is simplified a lot.
> 
> We have great respect and gratitude to Salil and his team for their work
> and contributions to CPU HotPlug,


Many thanks! Really appreciate this.


> which greatly reduced the difficulty of developing CPU HotPlug in
> Loongarch.


We are glad that this work is useful to other companies as well this
was one of our goal.


> So, We plan to rebase the next version of patch based on their code.


Great. Thanks for clarifying this as accountability is important
even though we are working in a collaborative environment.

As such, I am planning to send the RFC V2 in 2 weeks of time and
will make sure that the patches which are required by your patch-set
are formed in such a way that they can be independently accepted
w.r.t rest of the ARM64 architecture specific patch-set.


> Hi Salil,
> 
> I estimate that it will take quite some time for your patch series to
> merge in,
> 
> if allowed, can you merge your patch series changes to the common code
> section into the community first,
> 
> so that our code can be rebase and merged.


Sure, as clarified above, something similar, will create a patch-set
which will have patches which can be independently accepted/Ack'ed
and consumed even before the complete patch-set.

Although I think, even in current form most patches have been arranged
in such a way. But I will doubly check again before I float RFC V2.


> > Besides, we found the vCPU hotplug isn't working for TCG when Salil's
> > series was
> > tested. I'm not sure if we have same issue with this series, or TCG
> > isn't a concern
> > to us at all?
> 
> At present, QEMU only supports TCG under Loongarch,
> 
> and we test CPU HotPlug is also carried out under QEMU TCG,
> 
> and CPU HotPlug can work normally when testing.
> 
> Of course, we are also very happy to see you testing CPU hotplug under
> Loongarch,
> 
> which can find more problems and help us improve our patch.


We know that. There are some trivial fixes we already have, I will push
them as well for the completion. We were not sure if anybody needs them
as usage of vCPU Hotplug under 'accel=tcg' is highly unlikely except for
testing cases. Please let us know if you have any?


Many thanks!
Salil.

RE: [PATCH 0/8] Adds CPU hot-plug support to Loongarch

[Salil Mehta via]

Hi Gavin,

> From: Gavin Shan <gshan@redhat.com>
> Sent: Thursday, July 27, 2023 1:57 AM
> 
> Hi Xianglai,
> 
> On 7/20/23 17:15, xianglai li wrote:
> > Hello everyone, We refer to the implementation of ARM CPU
> > Hot-Plug to add GED-based CPU Hot-Plug support to Loongarch.
> >
> > The first 4 patches are changes to the QEMU common code,
> > including adding GED support for CPU Hot-Plug, updating
> > the ACPI table creation process, and adding qdev_disconnect_gpio_out_named
> > and cpu_address_space_destroy interfaces to release resources
> > when CPU un-plug.
> >
> > The last four patches are Loongarch architecture-related,
> > and the modifications include the definition of the hook
> > function related to the CPU Hot-(UN)Plug, the allocation
> > and release of CPU resources when CPU Hot-(UN)Plug,
> > the creation process of updating the ACPI table,
> > and finally the custom switch for the CPU Hot-Plug.

[...]

> It seems the design for ARM64 hotplug has been greatly referred, but the authors
> are missed to be copied if you were referring to the following repository.
> There will be conflicts between those two patchsets as I can see and I'm not sure
> how it can be resolved. In theory, one patchset needs to be rebased on another
> one since they're sharing large amount of codes.
> 
>    https://github.com/salil-mehta/qemu.git
>    (branch: virt-cpuhp-armv8/rfc-v1-port11052023.dev-1)
> 
> I took a quick scan on this series. Loongarch doesn't have ARM64's constraint due
> to vGIC3 where all vCPUs's file descriptor needs to be in place. It means the possible
> and not yet present vCPU needs to be visible to KVM. Without this constraint, the
> implementation is simplified a lot.
> 
> Besides, we found the vCPU hotplug isn't working for TCG when Salil's series was
> tested. I'm not sure if we have same issue with this series, or TCG isn't a concern
> to us at all?


Sorry, I should have replied this in the other mail but have been on/off in last
few days due to some medical reasons. But jotting it here:

Yes, you are correct. And there are some trivial fixes we already have to make
it work. In case it is useful to you, we are planning to add them for the sake
of completion. Maybe you can try that in the RFC V2 or I'll share with you the
repository once I push the fixes.

Many thanks!

Best wishes,
Salil.

Re: [PATCH 0/8] Adds CPU hot-plug support to Loongarch

[Gavin Shan]

Hi Salil,

On 7/28/23 00:58, Salil Mehta wrote:
>> From: Gavin Shan <gshan@redhat.com>
>> Sent: Thursday, July 27, 2023 1:57 AM
>> On 7/20/23 17:15, xianglai li wrote:
>>> Hello everyone, We refer to the implementation of ARM CPU
>>> Hot-Plug to add GED-based CPU Hot-Plug support to Loongarch.
>>>
>>> The first 4 patches are changes to the QEMU common code,
>>> including adding GED support for CPU Hot-Plug, updating
>>> the ACPI table creation process, and adding qdev_disconnect_gpio_out_named
>>> and cpu_address_space_destroy interfaces to release resources
>>> when CPU un-plug.
>>>
>>> The last four patches are Loongarch architecture-related,
>>> and the modifications include the definition of the hook
>>> function related to the CPU Hot-(UN)Plug, the allocation
>>> and release of CPU resources when CPU Hot-(UN)Plug,
>>> the creation process of updating the ACPI table,
>>> and finally the custom switch for the CPU Hot-Plug.
> 
> [...]
> 
>> It seems the design for ARM64 hotplug has been greatly referred, but the authors
>> are missed to be copied if you were referring to the following repository.
>> There will be conflicts between those two patchsets as I can see and I'm not sure
>> how it can be resolved. In theory, one patchset needs to be rebased on another
>> one since they're sharing large amount of codes.
>>
>>     https://github.com/salil-mehta/qemu.git
>>     (branch: virt-cpuhp-armv8/rfc-v1-port11052023.dev-1)
>>
>> I took a quick scan on this series. Loongarch doesn't have ARM64's constraint due
>> to vGIC3 where all vCPUs's file descriptor needs to be in place. It means the possible
>> and not yet present vCPU needs to be visible to KVM. Without this constraint, the
>> implementation is simplified a lot.
>>
>> Besides, we found the vCPU hotplug isn't working for TCG when Salil's series was
>> tested. I'm not sure if we have same issue with this series, or TCG isn't a concern
>> to us at all?
> 
> 
> Sorry, I should have replied this in the other mail but have been on/off in last
> few days due to some medical reasons. But jotting it here:
> 
> Yes, you are correct. And there are some trivial fixes we already have to make
> it work. In case it is useful to you, we are planning to add them for the sake
> of completion. Maybe you can try that in the RFC V2 or I'll share with you the
> repository once I push the fixes.
> 
> Many thanks!
> 

No worries, thanks a lot for your followup and clarify. I think it'd better to
make TCG working so that we have consistent vCPU hotplug capability for all
accelerators eventually. However, I didn't test with HVF and not sure about it.
I just checked your repository again and it seems these TCG fixes aren't there
yet.

   https://github.com/salil-mehta/qemu.git
   (virt-cpuhp-armv8/rfc-v1-port11052023.dev-1)

   /home/gshan/sandbox/src/qemu/main/build/qemu-system-aarch64           \
   -accel tcg -machine virt,gic-version=3,nvdimm=on -icount 1            \
   -cpu max -smp maxcpus=2,cpus=1,sockets=1,clusters=1,cores=1,threads=2
    :
   (qemu) device_add driver=max-arm-cpu,id=cpu1,thread-id=1
   Error: cpu(id1=0:0:0:1) with arch-id 1 exists

As to the cause, the CPU object isn't detached from the CPU slot as said
before.

Thanks,
Gavin

Re: [PATCH 1/8] Update ACPI GED framework to support vcpu hot-(un)plug

[Igor Mammedov]

On Thu, 20 Jul 2023 15:15:06 +0800
xianglai li <lixianglai@loongson.cn> wrote:

> ACPI GED shall be used to convey to the guest kernel about any cpu hot-(un)plug
> events. Therefore, existing ACPI GED framework inside QEMU needs to be enhanced
> to support CPU hot-(un)plug state and events.

skimmed through, it looks good to me

see nit below

> Cc: Xiaojuan Yang <yangxiaojuan@loongson.cn>
> Cc: Song Gao <gaosong@loongson.cn>
> Cc: "Michael S. Tsirkin" <mst@redhat.com>
> Cc: Igor Mammedov <imammedo@redhat.com>
> Cc: Ani Sinha <anisinha@redhat.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Richard Henderson <richard.henderson@linaro.org>
> Cc: Eduardo Habkost <eduardo@habkost.net>
> Cc: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
> Cc: "Philippe Mathieu-Daudé" <philmd@linaro.org>
> Cc: Yanan Wang <wangyanan55@huawei.com>
> Cc: "Daniel P. Berrangé" <berrange@redhat.com>
> Cc: Peter Xu <peterx@redhat.com>
> Cc: David Hildenbrand <david@redhat.com>
> Signed-off-by: xianglai li <lixianglai@loongson.cn>
> ---
>  hw/acpi/acpi-cpu-hotplug-stub.c        |  6 +++++
>  hw/acpi/cpu.c                          |  7 ------
>  hw/acpi/generic_event_device.c         | 33 ++++++++++++++++++++++++++
>  include/hw/acpi/cpu_hotplug.h          | 10 ++++++++
>  include/hw/acpi/generic_event_device.h |  6 +++++
>  5 files changed, 55 insertions(+), 7 deletions(-)
> 
> diff --git a/hw/acpi/acpi-cpu-hotplug-stub.c b/hw/acpi/acpi-cpu-hotplug-stub.c
> index 3fc4b14c26..2aec90d968 100644
> --- a/hw/acpi/acpi-cpu-hotplug-stub.c
> +++ b/hw/acpi/acpi-cpu-hotplug-stub.c
> @@ -24,6 +24,12 @@ void acpi_cpu_ospm_status(CPUHotplugState *cpu_st, ACPIOSTInfoList ***list)
>      return;
>  }
>  
> +void cpu_hotplug_hw_init(MemoryRegion *as, Object *owner,
> +                         CPUHotplugState *state, hwaddr base_addr)
> +{
> +    return;
> +}
> +
>  void acpi_cpu_plug_cb(HotplugHandler *hotplug_dev,
>                        CPUHotplugState *cpu_st, DeviceState *dev, Error **errp)
>  {
> diff --git a/hw/acpi/cpu.c b/hw/acpi/cpu.c
> index 19c154d78f..6897c8789a 100644
> --- a/hw/acpi/cpu.c
> +++ b/hw/acpi/cpu.c
> @@ -6,13 +6,6 @@
>  #include "trace.h"
>  #include "sysemu/numa.h"
>  
> -#define ACPI_CPU_HOTPLUG_REG_LEN 12
> -#define ACPI_CPU_SELECTOR_OFFSET_WR 0
> -#define ACPI_CPU_FLAGS_OFFSET_RW 4
> -#define ACPI_CPU_CMD_OFFSET_WR 5
> -#define ACPI_CPU_CMD_DATA_OFFSET_RW 8
> -#define ACPI_CPU_CMD_DATA2_OFFSET_R 0
> -
>  #define OVMF_CPUHP_SMI_CMD 4
>  
>  enum {
> diff --git a/hw/acpi/generic_event_device.c b/hw/acpi/generic_event_device.c
> index a3d31631fe..c5a70957b4 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,
>  };
>  
>  /*
> @@ -117,6 +119,10 @@ 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_CPU_HOTPLUG_EVT:
> +                aml_append(if_ctx, aml_call0(ACPI_CPU_CONTAINER "."
> +                                             ACPI_CPU_SCAN_METHOD));
> +                break;
>              default:
>                  /*
>                   * Please make sure all the events in ged_supported_events[]
> @@ -234,6 +240,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 +256,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 +271,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 +284,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 +299,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);
> @@ -318,6 +333,16 @@ static const VMStateDescription vmstate_memhp_state = {
>      }
>  };
>  
> +static const VMStateDescription vmstate_cpuhp_state = {
> +    .name = "acpi-ged/cpuhp",
> +    .version_id = 1,
> +    .minimum_version_id = 1,
> +    .fields      = (VMStateField[]) {
> +        VMSTATE_CPU_HOTPLUG(cpuhp_state, AcpiGedState),
> +        VMSTATE_END_OF_LIST()
> +    }
> +};
> +
>  static const VMStateDescription vmstate_ged_state = {
>      .name = "acpi-ged-state",
>      .version_id = 1,
> @@ -366,6 +391,7 @@ static const VMStateDescription vmstate_acpi_ged = {
>      },
>      .subsections = (const VMStateDescription * []) {
>          &vmstate_memhp_state,
> +        &vmstate_cpuhp_state,
>          &vmstate_ghes_state,
>          NULL
>      }
> @@ -400,6 +426,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/cpu_hotplug.h b/include/hw/acpi/cpu_hotplug.h
> index 3b932abbbb..afee1ab996 100644
> --- a/include/hw/acpi/cpu_hotplug.h
> +++ b/include/hw/acpi/cpu_hotplug.h
> @@ -19,6 +19,16 @@
>  #include "hw/hotplug.h"
>  #include "hw/acpi/cpu.h"
>  
> +#define ACPI_CPU_HOTPLUG_REG_LEN 12
> +#define ACPI_CPU_SELECTOR_OFFSET_WR 0
> +#define ACPI_CPU_FLAGS_OFFSET_RW 4
> +#define ACPI_CPU_CMD_OFFSET_WR 5
> +#define ACPI_CPU_CMD_DATA_OFFSET_RW 8
> +#define ACPI_CPU_CMD_DATA2_OFFSET_R 0
> +
> +#define ACPI_CPU_SCAN_METHOD "CSCN"
> +#define ACPI_CPU_CONTAINER "\\_SB.CPUS"
> +
>  typedef struct AcpiCpuHotplug {
>      Object *device;
>      MemoryRegion io;
> diff --git a/include/hw/acpi/generic_event_device.h b/include/hw/acpi/generic_event_device.h
> index d831bbd889..2923bd9d82 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"
> @@ -70,6 +71,7 @@
>  OBJECT_DECLARE_SIMPLE_TYPE(AcpiGedState, ACPI_GED)
>  
>  #define TYPE_ACPI_GED_X86 "acpi-ged-x86"

> +#define TYPE_ACPI_GED_LOONGARCH "acpi-ged-loongarch"
where is it used?
If it's for later patches, it should be in the patch that
will actually use it 

>  
>  #define ACPI_GED_EVT_SEL_OFFSET    0x0
>  #define ACPI_GED_EVT_SEL_LEN       0x4
> @@ -97,6 +99,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;
> @@ -108,6 +111,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;

Re: [PATCH 2/8] Update CPUs AML with cpu-(ctrl)dev change

[Igor Mammedov]

On Thu, 20 Jul 2023 15:15:07 +0800
xianglai li <lixianglai@loongson.cn> wrote:

> CPUs Control device(\\_SB.PCI0) register interface for the x86 arch is based on
> PCI and is IO port based and hence existing cpus AML code assumes _CRS objects
> would evaluate to a system resource which describes IO Port address. But on LOONGARCH
> arch CPUs control device(\\_SB.PRES) register interface is memory-mapped hence
> _CRS object should evaluate to system resource which describes memory-mapped
> base address.
> 
> This cpus AML code change updates the existing inerface of the build cpus AML
                                                 ^^^ typo
> function to accept both IO/MEMORY type regions and update the _CRS object
> correspondingly.

try to reformat commit message  to less than 80 character per line

> 
> Cc: Xiaojuan Yang <yangxiaojuan@loongson.cn>
> Cc: Song Gao <gaosong@loongson.cn>
> Cc: "Michael S. Tsirkin" <mst@redhat.com>
> Cc: Igor Mammedov <imammedo@redhat.com>
> Cc: Ani Sinha <anisinha@redhat.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Richard Henderson <richard.henderson@linaro.org>
> Cc: Eduardo Habkost <eduardo@habkost.net>
> Cc: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
> Cc: "Philippe Mathieu-Daudé" <philmd@linaro.org>
> Cc: Yanan Wang <wangyanan55@huawei.com>
> Cc: "Daniel P. Berrangé" <berrange@redhat.com>
> Cc: Peter Xu <peterx@redhat.com>
> Cc: David Hildenbrand <david@redhat.com>
> Signed-off-by: xianglai li <lixianglai@loongson.cn>
> ---
>  hw/acpi/cpu.c         | 30 +++++++++++++++++++++++-------
>  hw/i386/acpi-build.c  |  2 +-
>  include/hw/acpi/cpu.h |  5 +++--
>  3 files changed, 27 insertions(+), 10 deletions(-)
> 
> diff --git a/hw/acpi/cpu.c b/hw/acpi/cpu.c
> index 6897c8789a..3b945a1a40 100644
> --- a/hw/acpi/cpu.c
> +++ b/hw/acpi/cpu.c
> @@ -5,6 +5,7 @@
>  #include "qapi/qapi-events-acpi.h"
>  #include "trace.h"
>  #include "sysemu/numa.h"
> +#include "hw/acpi/cpu_hotplug.h"
>  
>  #define OVMF_CPUHP_SMI_CMD 4
>  
> @@ -331,9 +332,10 @@ const VMStateDescription vmstate_cpu_hotplug = {
>  #define CPU_FW_EJECT_EVENT "CEJF"
>  
>  void build_cpus_aml(Aml *table, MachineState *machine, CPUHotplugFeatures opts,
> -                    hwaddr io_base,
> +                    hwaddr mmap_io_base,
>                      const char *res_root,
> -                    const char *event_handler_method)
> +                    const char *event_handler_method,
> +                    AmlRegionSpace rs)
>  {
>      Aml *ifctx;
>      Aml *field;
> @@ -360,14 +362,28 @@ void build_cpus_aml(Aml *table, MachineState *machine, CPUHotplugFeatures opts,
>          aml_append(cpu_ctrl_dev, aml_mutex(CPU_LOCK, 0));
>  
>          crs = aml_resource_template();
> -        aml_append(crs, aml_io(AML_DECODE16, io_base, io_base, 1,
> +        if (rs == AML_SYSTEM_IO) {
> +            aml_append(crs, aml_io(AML_DECODE16, mmap_io_base, mmap_io_base, 1,
>                                 ACPI_CPU_HOTPLUG_REG_LEN));
> +        } else {
> +            aml_append(crs, aml_memory32_fixed(mmap_io_base,
> +                               ACPI_CPU_HOTPLUG_REG_LEN, AML_READ_WRITE));
> +        }
>          aml_append(cpu_ctrl_dev, aml_name_decl("_CRS", crs));
>  
> -        /* declare CPU hotplug MMIO region with related access fields */
> -        aml_append(cpu_ctrl_dev,
> -            aml_operation_region("PRST", AML_SYSTEM_IO, aml_int(io_base),
> -                                 ACPI_CPU_HOTPLUG_REG_LEN));
> +        if (rs == AML_SYSTEM_IO) {
> +            /* declare CPU hotplug MMIO region with related access fields */
> +            aml_append(cpu_ctrl_dev,
> +                aml_operation_region("PRST", AML_SYSTEM_IO,
> +                                             aml_int(mmap_io_base),
> +                                             ACPI_CPU_HOTPLUG_REG_LEN));
> +        } else {
> +            aml_append(cpu_ctrl_dev,
> +                aml_operation_region("PRST", AML_SYSTEM_MEMORY,
> +                                             aml_int(mmap_io_base),
> +                                             ACPI_CPU_HOTPLUG_REG_LEN));
> +        }


to reduce duplication, following could be better way to spell it:
 g_assert(rs == foo1 || rs == foo2)
 ... aml_operation_region("PRST", rs, aml_int(io_base), ...

>  
>          field = aml_field("PRST", AML_BYTE_ACC, AML_NOLOCK,
>                            AML_WRITE_AS_ZEROS);
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index 9c74fa17ad..5d02690593 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -1548,7 +1548,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
>              .fw_unplugs_cpu = pm->smi_on_cpu_unplug,
>          };
>          build_cpus_aml(dsdt, machine, opts, pm->cpu_hp_io_base,
> -                       "\\_SB.PCI0", "\\_GPE._E02");
> +                       "\\_SB.PCI0", "\\_GPE._E02", AML_SYSTEM_IO);
>      }
>  
>      if (pcms->memhp_io_base && nr_mem) {
> diff --git a/include/hw/acpi/cpu.h b/include/hw/acpi/cpu.h
> index 999caaf510..cddea78333 100644
> --- a/include/hw/acpi/cpu.h
> +++ b/include/hw/acpi/cpu.h
> @@ -56,9 +56,10 @@ typedef struct CPUHotplugFeatures {
>  } CPUHotplugFeatures;
>  
>  void build_cpus_aml(Aml *table, MachineState *machine, CPUHotplugFeatures opts,
> -                    hwaddr io_base,
> +                    hwaddr mmap_io_base,
>                      const char *res_root,
> -                    const char *event_handler_method);
> +                    const char *event_handler_method,
> +                    AmlRegionSpace rs);
>  
>  void acpi_cpu_ospm_status(CPUHotplugState *cpu_st, ACPIOSTInfoList ***list);
>  

Re: [PATCH 3/8] Introduced a new function to disconnect GPIO connections

[Igor Mammedov]

On Thu, 20 Jul 2023 15:15:08 +0800
xianglai li <lixianglai@loongson.cn> wrote:

> It introduces a new function to unwire the
> vcpu<->exioi interrupts for the vcpu hot-(un)plug cases.

it's not a new function.
You probably wanted to say:

subj: make foo() public

it will be reused .someplace. for ...

> 
> Cc: Xiaojuan Yang <yangxiaojuan@loongson.cn>
> Cc: Song Gao <gaosong@loongson.cn>
> Cc: "Michael S. Tsirkin" <mst@redhat.com>
> Cc: Igor Mammedov <imammedo@redhat.com>
> Cc: Ani Sinha <anisinha@redhat.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Richard Henderson <richard.henderson@linaro.org>
> Cc: Eduardo Habkost <eduardo@habkost.net>
> Cc: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
> Cc: "Philippe Mathieu-Daudé" <philmd@linaro.org>
> Cc: Yanan Wang <wangyanan55@huawei.com>
> Cc: "Daniel P. Berrangé" <berrange@redhat.com>
> Cc: Peter Xu <peterx@redhat.com>
> Cc: David Hildenbrand <david@redhat.com>
> Signed-off-by: xianglai li <lixianglai@loongson.cn>
> ---
>  hw/core/gpio.c         | 4 ++--
>  include/hw/qdev-core.h | 2 ++
>  2 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/core/gpio.c b/hw/core/gpio.c
> index 80d07a6ec9..4fc6409545 100644
> --- a/hw/core/gpio.c
> +++ b/hw/core/gpio.c
> @@ -143,8 +143,8 @@ qemu_irq qdev_get_gpio_out_connector(DeviceState *dev, const char *name, int n)
>  
>  /* disconnect a GPIO output, returning the disconnected input (if any) */
>  
> -static qemu_irq qdev_disconnect_gpio_out_named(DeviceState *dev,
> -                                               const char *name, int n)
> +qemu_irq qdev_disconnect_gpio_out_named(DeviceState *dev,
> +                                        const char *name, int n)
>  {
>      char *propname = g_strdup_printf("%s[%d]",
>                                       name ? name : "unnamed-gpio-out", n);
> diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
> index 884c726a87..992f5419fa 100644
> --- a/include/hw/qdev-core.h
> +++ b/include/hw/qdev-core.h
> @@ -739,6 +739,8 @@ qemu_irq qdev_get_gpio_out_connector(DeviceState *dev, const char *name, int n);
>   */
>  qemu_irq qdev_intercept_gpio_out(DeviceState *dev, qemu_irq icpt,
>                                   const char *name, int n);
> +qemu_irq qdev_disconnect_gpio_out_named(DeviceState *dev,
> +                                               const char *name, int n);

watch for proper alignment 

have you tried to run ./scripts/checkpatch.pl on you series?
(it should catch such cases)

>  
>  BusState *qdev_get_child_bus(DeviceState *dev, const char *name);
>  

Re: [PATCH 4/8] Introduce the CPU address space destruction function

[Igor Mammedov]

On Thu, 20 Jul 2023 15:15:09 +0800
xianglai li <lixianglai@loongson.cn> wrote:

> Introduce new functions to destroy CPU address space resources

s/functions/function/

> for cpu hot-(un)plug.
> 
> Cc: Xiaojuan Yang <yangxiaojuan@loongson.cn>
> Cc: Song Gao <gaosong@loongson.cn>
> Cc: "Michael S. Tsirkin" <mst@redhat.com>
> Cc: Igor Mammedov <imammedo@redhat.com>
> Cc: Ani Sinha <anisinha@redhat.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Richard Henderson <richard.henderson@linaro.org>
> Cc: Eduardo Habkost <eduardo@habkost.net>
> Cc: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
> Cc: "Philippe Mathieu-Daudé" <philmd@linaro.org>
> Cc: Yanan Wang <wangyanan55@huawei.com>
> Cc: "Daniel P. Berrangé" <berrange@redhat.com>
> Cc: Peter Xu <peterx@redhat.com>
> Cc: David Hildenbrand <david@redhat.com>
> Signed-off-by: xianglai li <lixianglai@loongson.cn>
> ---
>  include/exec/cpu-common.h |  8 ++++++++
>  include/hw/core/cpu.h     |  1 +
>  softmmu/physmem.c         | 24 ++++++++++++++++++++++++
>  3 files changed, 33 insertions(+)
> 
> diff --git a/include/exec/cpu-common.h b/include/exec/cpu-common.h
> index 87dc9a752c..27cd4d32b1 100644
> --- a/include/exec/cpu-common.h
> +++ b/include/exec/cpu-common.h
> @@ -120,6 +120,14 @@ size_t qemu_ram_pagesize_largest(void);
>   */
>  void cpu_address_space_init(CPUState *cpu, int asidx,
>                              const char *prefix, MemoryRegion *mr);
> +/**
> + * cpu_address_space_destroy:
> + * @cpu: CPU for which address space needs to be destroyed
> + * @asidx: integer index of this address space
> + *
> + * Note that with KVM only one address space is supported.
> + */
> +void cpu_address_space_destroy(CPUState *cpu, int asidx);
>  
>  void cpu_physical_memory_rw(hwaddr addr, void *buf,
>                              hwaddr len, bool is_write);
> diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
> index fdcbe87352..d6d68dac12 100644
> --- a/include/hw/core/cpu.h
> +++ b/include/hw/core/cpu.h
> @@ -366,6 +366,7 @@ struct CPUState {
>      QSIMPLEQ_HEAD(, qemu_work_item) work_list;
>  
>      CPUAddressSpace *cpu_ases;
> +    int cpu_ases_ref_count;

perhaps renaming it to num_ases would be better

>      int num_ases;
and this one can be named num__ases_[total|max]


>      AddressSpace *as;
>      MemoryRegion *memory;
> diff --git a/softmmu/physmem.c b/softmmu/physmem.c
> index 3df73542e1..f4545b4508 100644
> --- a/softmmu/physmem.c
> +++ b/softmmu/physmem.c
> @@ -762,6 +762,7 @@ void cpu_address_space_init(CPUState *cpu, int asidx,
>  
>      if (!cpu->cpu_ases) {
>          cpu->cpu_ases = g_new0(CPUAddressSpace, cpu->num_ases);
> +        cpu->cpu_ases_ref_count = cpu->num_ases;
>      }
>  
>      newas = &cpu->cpu_ases[asidx];
> @@ -775,6 +776,29 @@ void cpu_address_space_init(CPUState *cpu, int asidx,
>      }
>  }
>  
> +void cpu_address_space_destroy(CPUState *cpu, int asidx)
                                                 ^^^ should it be uintX_t ?

> +{
> +    CPUAddressSpace *cpuas;
> +
> +    assert(asidx < cpu->num_ases);
> +    assert(asidx == 0 || !kvm_enabled());
> +    assert(cpu->cpu_ases);
> +
> +    cpuas = &cpu->cpu_ases[asidx];
> +    if (tcg_enabled()) {
> +        memory_listener_unregister(&cpuas->tcg_as_listener);
> +    }
> +
> +    address_space_destroy(cpuas->as);
> +
> +    cpu->cpu_ases_ref_count--;
> +    if (cpu->cpu_ases_ref_count == 0) {
> +        g_free(cpu->cpu_ases);
> +        cpu->cpu_ases = NULL;
> +    }
> +
> +}
> +
>  AddressSpace *cpu_get_address_space(CPUState *cpu, int asidx)
>  {
>      /* Return the AddressSpace corresponding to the specified index */

Re: [PATCH 3/8] Introduced a new function to disconnect GPIO connections

[Peter Maydell]

On Thu, 20 Jul 2023 at 08:16, xianglai li <lixianglai@loongson.cn> wrote:
>
> It introduces a new function to unwire the
> vcpu<->exioi interrupts for the vcpu hot-(un)plug cases.
>
> Cc: Xiaojuan Yang <yangxiaojuan@loongson.cn>
> Cc: Song Gao <gaosong@loongson.cn>
> Cc: "Michael S. Tsirkin" <mst@redhat.com>
> Cc: Igor Mammedov <imammedo@redhat.com>
> Cc: Ani Sinha <anisinha@redhat.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Richard Henderson <richard.henderson@linaro.org>
> Cc: Eduardo Habkost <eduardo@habkost.net>
> Cc: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
> Cc: "Philippe Mathieu-Daudé" <philmd@linaro.org>
> Cc: Yanan Wang <wangyanan55@huawei.com>
> Cc: "Daniel P. Berrangé" <berrange@redhat.com>
> Cc: Peter Xu <peterx@redhat.com>
> Cc: David Hildenbrand <david@redhat.com>
> Signed-off-by: xianglai li <lixianglai@loongson.cn>
> ---
>  hw/core/gpio.c         | 4 ++--
>  include/hw/qdev-core.h | 2 ++
>  2 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/hw/core/gpio.c b/hw/core/gpio.c
> index 80d07a6ec9..4fc6409545 100644
> --- a/hw/core/gpio.c
> +++ b/hw/core/gpio.c
> @@ -143,8 +143,8 @@ qemu_irq qdev_get_gpio_out_connector(DeviceState *dev, const char *name, int n)
>
>  /* disconnect a GPIO output, returning the disconnected input (if any) */
>
> -static qemu_irq qdev_disconnect_gpio_out_named(DeviceState *dev,
> -                                               const char *name, int n)
> +qemu_irq qdev_disconnect_gpio_out_named(DeviceState *dev,
> +                                        const char *name, int n)
>  {
>      char *propname = g_strdup_printf("%s[%d]",
>                                       name ? name : "unnamed-gpio-out", n);

This function as currently written is intended only for
use in qdev_intercept_gpio_out(), which in turn is there
only for qtest test case use. I would be a bit cautious
about whether there are unexpected issues with trying
to use it in "production" functionality with a running
QEMU (eg when perhaps some device might be trying to
signal the gpio line in another thread while this one
is trying to disconnect it).

thanks
-- PMM

Re: [PATCH 5/8] Adds basic CPU hot-(un)plug support for Loongarch

[Igor Mammedov]

On Thu, 20 Jul 2023 15:15:10 +0800
xianglai li <lixianglai@loongson.cn> wrote:

> 1.Add CPU topology related functions
> 2.Add CPU hot-plug related hook functions
> 3.Update the in-place CPU creation process at machine initialization

patch is to large, split it at least on those ^^ 3 parts,
which would do a single distinct thing.
After that it will be easier to review this.


Also looking at hw/loongarch/acpi-build.c
you have cpu_index == arch_id == core_id /according to comments/
and they are mixed/used interchangeably. which is confusing
at least. So clean it up first to use arch_id consistently

then a separate patches to introduce socket/core/thread support
with proper documentation/pointers to specs as to how arch_id
should be calculated.

And once that is ready, add hotplug on top of it. 


> 
> Cc: Xiaojuan Yang <yangxiaojuan@loongson.cn>
> Cc: Song Gao <gaosong@loongson.cn>
> Cc: "Michael S. Tsirkin" <mst@redhat.com>
> Cc: Igor Mammedov <imammedo@redhat.com>
> Cc: Ani Sinha <anisinha@redhat.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Richard Henderson <richard.henderson@linaro.org>
> Cc: Eduardo Habkost <eduardo@habkost.net>
> Cc: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
> Cc: "Philippe Mathieu-Daudé" <philmd@linaro.org>
> Cc: Yanan Wang <wangyanan55@huawei.com>
> Cc: "Daniel P. Berrangé" <berrange@redhat.com>
> Cc: Peter Xu <peterx@redhat.com>
> Cc: David Hildenbrand <david@redhat.com>
> Signed-off-by: xianglai li <lixianglai@loongson.cn>
> ---
>  hw/loongarch/virt.c         | 381 ++++++++++++++++++++++++++++++++++--
>  include/hw/loongarch/virt.h |  11 +-
>  target/loongarch/cpu.h      |   4 +
>  3 files changed, 382 insertions(+), 14 deletions(-)
> 
> diff --git a/hw/loongarch/virt.c b/hw/loongarch/virt.c
> index e19b042ce8..5919389f42 100644
> --- a/hw/loongarch/virt.c
> +++ b/hw/loongarch/virt.c
> @@ -46,6 +46,9 @@
>  #include "hw/block/flash.h"
>  #include "qemu/error-report.h"
>  
> +static int virt_get_socket_id(const MachineState *ms, int cpu_index);
> +static int virt_get_core_id(const MachineState *ms, int cpu_index);
> +static int virt_get_thread_id(const MachineState *ms, int cpu_index);
>  
>  static void virt_flash_create(LoongArchMachineState *lams)
>  {
> @@ -447,12 +450,12 @@ static DeviceState *create_acpi_ged(DeviceState *pch_pic, LoongArchMachineState
>  {
>      DeviceState *dev;
>      MachineState *ms = MACHINE(lams);
> -    uint32_t event = ACPI_GED_PWR_DOWN_EVT;
> +    uint32_t event = ACPI_GED_PWR_DOWN_EVT | ACPI_GED_CPU_HOTPLUG_EVT;
>  
>      if (ms->ram_slots) {
>          event |= ACPI_GED_MEM_HOTPLUG_EVT;
>      }
> -    dev = qdev_new(TYPE_ACPI_GED);
> +    dev = qdev_new(TYPE_ACPI_GED_LOONGARCH);
>      qdev_prop_set_uint32(dev, "ged-event", event);
>  
>      /* ged event */
> @@ -461,6 +464,7 @@ static DeviceState *create_acpi_ged(DeviceState *pch_pic, LoongArchMachineState
>      sysbus_mmio_map(SYS_BUS_DEVICE(dev), 1, VIRT_GED_MEM_ADDR);
>      /* ged regs used for reset and power down */
>      sysbus_mmio_map(SYS_BUS_DEVICE(dev), 2, VIRT_GED_REG_ADDR);
> +    sysbus_mmio_map(SYS_BUS_DEVICE(dev), 3, VIRT_GED_CPUHP_ADDR);
>  
>      sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0,
>                         qdev_get_gpio_in(pch_pic, VIRT_SCI_IRQ - VIRT_GSI_BASE));
> @@ -583,6 +587,7 @@ static void loongarch_irq_init(LoongArchMachineState *lams)
>  
>      extioi = qdev_new(TYPE_LOONGARCH_EXTIOI);
>      sysbus_realize_and_unref(SYS_BUS_DEVICE(extioi), &error_fatal);
> +    lams->extioi = extioi;
>  
>      /*
>       * The connection of interrupts:
> @@ -624,11 +629,11 @@ static void loongarch_irq_init(LoongArchMachineState *lams)
>                                      sysbus_mmio_get_region(SYS_BUS_DEVICE(ipi),
>                                      1));
>          /*
> -	 * extioi iocsr memory region
> -	 * only one extioi is added on loongarch virt machine
> -	 * external device interrupt can only be routed to cpu 0-3
> -	 */
> -	if (cpu < EXTIOI_CPUS)
> +         * extioi iocsr memory region
> +         * only one extioi is added on loongarch virt machine
> +         * external device interrupt can only be routed to cpu 0-3
> +         */
> +        if (cpu < EXTIOI_CPUS)
>              memory_region_add_subregion(&env->system_iocsr, APIC_BASE,
>                                  sysbus_mmio_get_region(SYS_BUS_DEVICE(extioi),
>                                  cpu));
> @@ -789,7 +794,6 @@ static void loongarch_init(MachineState *machine)
>      NodeInfo *numa_info = machine->numa_state->nodes;
>      int i;
>      hwaddr fdt_base;
> -    const CPUArchIdList *possible_cpus;
>      MachineClass *mc = MACHINE_GET_CLASS(machine);
>      CPUState *cpu;
>      char *ramName = NULL;
> @@ -810,12 +814,40 @@ static void loongarch_init(MachineState *machine)
>      create_fdt(lams);
>      /* Init CPUs */
>  
> -    possible_cpus = mc->possible_cpu_arch_ids(machine);
> -    for (i = 0; i < possible_cpus->len; i++) {
> -        cpu = cpu_create(machine->cpu_type);
> +    mc->possible_cpu_arch_ids(machine);
> +
> +    for (i = 0; i < machine->smp.cpus; i++) {
> +        Object *cpuobj;
> +        cpuobj = object_new(machine->cpu_type);
> +
> +        cpu = CPU(cpuobj);

>          cpu->cpu_index = i;
I'd move this to foo_cpu_pre_plug()

> -        machine->possible_cpus->cpus[i].cpu = OBJECT(cpu);
and this to foo_cpu_plug()

> +
> +        object_property_set_int(cpuobj, "socket-id",
> +                                virt_get_socket_id(machine, i), NULL);
> +        object_property_set_int(cpuobj, "core-id",
> +                                virt_get_core_id(machine, i), NULL);
> +        object_property_set_int(cpuobj, "thread-id",
> +                                virt_get_thread_id(machine, i), NULL);

you don't need to calculate foo_ids here, they shall be calculated once at
the 1st time possible_cpu_arch_ids() are called and then reuse
CPUArchId.props here.
see x86_possible_cpu_arch_ids() for an example


> +        /*
> +         * The CPU in place at the time of machine startup will also enter
> +         * the CPU hot-plug process when it is created, but at this time,
> +         * the GED device has not been created, resulting in exit in the CPU
> +         * hot-plug process, which can avoid the incumbent CPU repeatedly
> +         * applying for resources.
> +         *
> +         * The interrupt resource of the in-place CPU will be requested at
> +         * the current function call loongarch_irq_init().
> +         *
> +         * The interrupt resource of the subsequently inserted CPU will be
> +         * requested in the CPU hot-plug process.
> +         */
> +        qdev_realize(DEVICE(cpuobj), NULL, &error_fatal);
> +        object_unref(cpuobj);
>      }
> +
> +    lams->boot_cpus = machine->smp.cpus;
> +
>      fdt_add_cpu_nodes(lams);
>  
>      /* Node0 memory */
> @@ -986,11 +1018,107 @@ static void virt_mem_pre_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
>      pc_dimm_pre_plug(PC_DIMM(dev), MACHINE(hotplug_dev), NULL, errp);
>  }
>  
> +static int virt_get_cpu_id_from_cpu_topo(const MachineState *ms,
> +                                            DeviceState *dev)

name suggest it's topo->id helper,
then feed it topo arguments instead of dev.

> +{
> +    int cpu_index, sock_vcpu_num, core_vcpu_num;
> +    LoongArchCPU *cpu = LOONGARCH_CPU(dev);
> +
> +    /* calculate total logical cpus across socket/cluster/core */
> +    sock_vcpu_num = cpu->socket_id * (ms->smp.threads * ms->smp.cores);
> +    core_vcpu_num = cpu->core_id * ms->smp.threads;
> +
> +    /* get vcpu-id(logical cpu index) for this vcpu from this topology */
> +    cpu_index = (sock_vcpu_num + core_vcpu_num) + cpu->thread_id;
> +
> +    assert(cpu_index >= 0 && cpu_index < ms->possible_cpus->len);
> +
> +    return cpu_index;
> +}
> +
> +/* find cpu slot in machine->possible_cpus by core_id */
> +static CPUArchId *loongarch_find_cpu_slot(MachineState *ms, uint32_t cpu_index,
> +                                        int *idx)
> +{
> +    int index = cpu_index;
> +
> +    if (index >= ms->possible_cpus->len) {
> +        return NULL;
> +    }
> +    if (idx) {
> +        *idx = index;
> +    }
> +    return &ms->possible_cpus->cpus[index];
> +}
> +
> +static void loongarch_cpu_pre_plug(HotplugHandler *hotplug_dev,
> +                            DeviceState *dev, Error **errp)
> +{
> +    MachineState *ms = MACHINE(OBJECT(hotplug_dev));
> +    MachineClass *mc = MACHINE_GET_CLASS(hotplug_dev);
> +    LoongArchCPU *cpu = LOONGARCH_CPU(dev);
> +    CPUState *cs = CPU(dev);
> +    CPUArchId *cpu_slot;
> +    Error *local_err = NULL;
> +
> +    if (dev->hotplugged && !mc->has_hotpluggable_cpus) {
> +        error_setg(&local_err, "CPU hotplug not supported for this machine");
> +        goto out;
> +    }
> +
> +    /* sanity check the cpu */
> +    if (!object_dynamic_cast(OBJECT(cpu), ms->cpu_type)) {
> +        error_setg(&local_err, "Invalid CPU type, expected cpu type: '%s'",
> +                   ms->cpu_type);
> +        goto out;
> +    }
> +
> +    if ((cpu->thread_id < 0) || (cpu->thread_id >= ms->smp.threads)) {
> +        error_setg(&local_err,
> +                   "Invalid thread-id %u specified, must be in range 1:%u",
> +                   cpu->thread_id, ms->smp.threads - 1);
> +        goto out;
> +    }
> +
> +    if ((cpu->core_id < 0) || (cpu->core_id >= ms->smp.cores)) {
> +        error_setg(&local_err,
> +                   "Invalid core-id %u specified, must be in range 1:%u",
> +                   cpu->core_id, ms->smp.cores);
> +        goto out;
> +    }
> +
> +    if ((cpu->socket_id < 0) || (cpu->socket_id >= ms->smp.sockets)) {
> +        error_setg(&local_err,
> +                   "Invalid socket-id %u specified, must be in range 1:%u",
> +                   cpu->socket_id, ms->smp.sockets - 1);
> +        goto out;
> +    }

> +    cs->cpu_index = virt_get_cpu_id_from_cpu_topo(ms, dev);
> +
> +    cpu_slot = loongarch_find_cpu_slot(ms, cs->cpu_index, NULL);
> +    if (CPU(cpu_slot->cpu)) {
> +        error_setg(&local_err,
> +                   "cpu(id%d=%d:%d:%d) with arch-id %" PRIu64 " exists",
> +                   cs->cpu_index, cpu->socket_id, cpu->core_id,
> +                   cpu->thread_id, cpu_slot->arch_id);
> +        goto out;
> +    }
> +
> +    numa_cpu_pre_plug(cpu_slot, dev, &local_err);
> +
> +    return ;
> +out:
> +    error_propagate(errp, local_err);
> +}
> +
>  static void virt_machine_device_pre_plug(HotplugHandler *hotplug_dev,
>                                              DeviceState *dev, Error **errp)
>  {
>      if (memhp_type_supported(dev)) {
>          virt_mem_pre_plug(hotplug_dev, dev, errp);
> +    } else if (object_dynamic_cast(OBJECT(dev), TYPE_LOONGARCH_CPU)) {
> +        loongarch_cpu_pre_plug(hotplug_dev, dev, errp);
>      }
>  }
>  
> @@ -1004,11 +1132,45 @@ static void virt_mem_unplug_request(HotplugHandler *hotplug_dev,
>                                     errp);
>  }
>  
> +static void loongarch_cpu_unplug_request(HotplugHandler *hotplug_dev,
> +                                        DeviceState *dev, Error **errp)
> +{
> +    MachineState *machine = MACHINE(OBJECT(hotplug_dev));
> +    LoongArchMachineState *lsms = LOONGARCH_MACHINE(machine);
> +    Error *local_err = NULL;
> +    HotplugHandlerClass *hhc;
> +    LoongArchCPU *cpu = LOONGARCH_CPU(dev);
> +    CPUState *cs = CPU(dev);
> +
> +    if (!lsms->acpi_ged) {
> +        error_setg(&local_err, "CPU hot unplug not supported without ACPI");
> +        goto out;
> +    }
> +
> +    if (cs->cpu_index == 0) {
> +        error_setg(&local_err,
> +                   "hot-unplug of boot cpu(id%d=%d:%d:%d) not supported",
> +                   cs->cpu_index, cpu->socket_id,
> +                   cpu->core_id, cpu->thread_id);
> +        goto out;
> +    }
> +
> +
> +    hhc = HOTPLUG_HANDLER_GET_CLASS(lsms->acpi_ged);
> +    hhc->unplug_request(HOTPLUG_HANDLER(lsms->acpi_ged), dev, &local_err);
> +
> +    return;
> + out:
> +    error_propagate(errp, local_err);
> +}
> +
>  static void virt_machine_device_unplug_request(HotplugHandler *hotplug_dev,
>                                            DeviceState *dev, Error **errp)
>  {
>      if (memhp_type_supported(dev)) {
>          virt_mem_unplug_request(hotplug_dev, dev, errp);
> +    } else if (object_dynamic_cast(OBJECT(dev), TYPE_LOONGARCH_CPU)) {
> +        loongarch_cpu_unplug_request(hotplug_dev, dev, errp);
>      }
>  }
>  
> @@ -1022,11 +1184,93 @@ static void virt_mem_unplug(HotplugHandler *hotplug_dev,
>      qdev_unrealize(dev);
>  }
>  
> +static void loongarch_cpu_destroy(MachineState *machine, LoongArchCPU *cpu)

I'd fold this into unplug handler, and the same for _cpu_create
you aren't destroying/creating CPU at this point but rather
wiring it up with other external to it components.

> +{
> +    LoongArchMachineState *lsms = LOONGARCH_MACHINE(machine);
> +    CPULoongArchState *env = &cpu->env;
> +    DeviceState *ipi = env->ipistate;
> +    CPUState *cs = CPU(cpu);
> +    unsigned int cpu_index = cs->cpu_index;
> +    DeviceState *extioi = lsms->extioi;
> +    int pin;
> +
> +    qemu_unregister_reset(reset_load_elf, DEVICE(cpu));
> +

> +    lsms->boot_cpus--;
> +    if (lsms->fw_cfg) {
> +        fw_cfg_modify_i16(lsms->fw_cfg, FW_CFG_NB_CPUS,
> +                          (uint16_t)lsms->boot_cpus);
> +    }
do you really need boot_cpus?
adding FWCFG variable is adding ABI, with potential to maintain it for a long time

it seems that before this series you also had support for multiple CPUs
and didn't care about FWCFG, so quest is why it's being added now. 

> +
> +    /* disconnect ipi irq to cpu irq */
> +    qdev_disconnect_gpio_out_named(ipi, NULL, 0);
> +    /* del IPI iocsr memory region */
> +    memory_region_del_subregion(&env->system_iocsr,
> +                                sysbus_mmio_get_region(SYS_BUS_DEVICE(ipi),
> +                                0));
> +    memory_region_del_subregion(&env->system_iocsr,
> +                                sysbus_mmio_get_region(SYS_BUS_DEVICE(ipi),
> +                                1));
> +
> +    env->ipistate = NULL;
> +    object_unparent(OBJECT(ipi));
> +
> +    /*
> +     * disconnect ext irq to the cpu irq
> +     * cpu_pin[9:2] <= intc_pin[7:0]
> +     */
> +    if (cpu_index < EXTIOI_CPUS) {
> +        for (pin = 0; pin < LS3A_INTC_IP; pin++) {
> +            qdev_disconnect_gpio_out_named(extioi, NULL, (cpu_index * 8 + pin));
> +        }
> +    }
> +
> +    /*
> +     * del extioi iocsr memory region
> +     * only one extioi is added on loongarch virt machine
> +     * external device interrupt can only be routed to cpu 0-3
> +     */
> +    if (cpu_index < EXTIOI_CPUS)
> +        memory_region_del_subregion(&env->system_iocsr,
> +                            sysbus_mmio_get_region(SYS_BUS_DEVICE(extioi),
> +                            cpu_index));
> +}
> +
> +static void loongarch_cpu_unplug(HotplugHandler *hotplug_dev,
> +                                DeviceState *dev, Error **errp)
> +{
> +    CPUArchId *found_cpu;
> +    HotplugHandlerClass *hhc;
> +    Error *local_err = NULL;
> +    LoongArchCPU *cpu = LOONGARCH_CPU(dev);
> +    MachineState *machine = MACHINE(OBJECT(hotplug_dev));
> +    LoongArchMachineState *lsms = LOONGARCH_MACHINE(machine);
> +    CPUState *cs = CPU(dev);
> +
> +    hhc = HOTPLUG_HANDLER_GET_CLASS(lsms->acpi_ged);
> +    hhc->unplug(HOTPLUG_HANDLER(lsms->acpi_ged), dev, &local_err);
> +
> +    if (local_err) {
> +        goto out;
> +    }
> +
> +    loongarch_cpu_destroy(machine, cpu);
> +
> +    found_cpu = loongarch_find_cpu_slot(MACHINE(lsms), cs->cpu_index, NULL);
> +    found_cpu->cpu = NULL;
> +
> +    return;
> +out:
> +    error_propagate(errp, local_err);
> +}
> +
>  static void virt_machine_device_unplug(HotplugHandler *hotplug_dev,
>                                            DeviceState *dev, Error **errp)
>  {
>      if (memhp_type_supported(dev)) {
>          virt_mem_unplug(hotplug_dev, dev, errp);
> +    } else if (object_dynamic_cast(OBJECT(dev), TYPE_LOONGARCH_CPU)) {
> +        loongarch_cpu_unplug(hotplug_dev, dev, errp);
>      }
>  }
>  
> @@ -1040,6 +1284,92 @@ static void virt_mem_plug(HotplugHandler *hotplug_dev,
>                           dev, &error_abort);
>  }
>  
> +
> +static LoongArchCPU *loongarch_cpu_create(MachineState *machine,
> +                                LoongArchCPU *cpu, Error **errp)
> +{
> +    LoongArchMachineState *lsms = LOONGARCH_MACHINE(machine);
> +    CPUState *cs = CPU(cpu);
> +    unsigned int cpu_index = cs->cpu_index;
> +    DeviceState *cpudev = DEVICE(cpu);
> +    DeviceState *extioi = lsms->extioi;
> +    CPULoongArchState *env = &cpu->env;
> +    DeviceState *ipi;
> +    int pin;
> +
> +    qemu_register_reset(reset_load_elf, cpu);
> +
> +    lsms->boot_cpus++;
> +    if (lsms->fw_cfg) {
> +        fw_cfg_modify_i16(lsms->fw_cfg, FW_CFG_NB_CPUS,
> +                          (uint16_t)lsms->boot_cpus);
> +    }
> +
> +    ipi = qdev_new(TYPE_LOONGARCH_IPI);
> +    sysbus_realize_and_unref(SYS_BUS_DEVICE(ipi), errp);
> +
> +    /* connect ipi irq to cpu irq */
> +    qdev_connect_gpio_out(ipi, 0, qdev_get_gpio_in(cpudev, IRQ_IPI));
> +    /* IPI iocsr memory region */
> +    memory_region_add_subregion(&env->system_iocsr, SMP_IPI_MAILBOX,
> +                                sysbus_mmio_get_region(SYS_BUS_DEVICE(ipi),
> +                                0));
> +    memory_region_add_subregion(&env->system_iocsr, MAIL_SEND_ADDR,
> +                                sysbus_mmio_get_region(SYS_BUS_DEVICE(ipi),
> +                                1));
> +    /*
> +     * extioi iocsr memory region
> +     * only one extioi is added on loongarch virt machine
> +     * external device interrupt can only be routed to cpu 0-3
> +     */
> +    if (cpu_index < EXTIOI_CPUS)
> +        memory_region_add_subregion(&env->system_iocsr, APIC_BASE,
> +                            sysbus_mmio_get_region(SYS_BUS_DEVICE(extioi),
> +                            cpu_index));
> +    env->ipistate = ipi;
> +
> +    /*
> +     * connect ext irq to the cpu irq
> +     * cpu_pin[9:2] <= intc_pin[7:0]
> +     */
> +    if (cpu_index < EXTIOI_CPUS) {
> +        for (pin = 0; pin < LS3A_INTC_IP; pin++) {
> +            qdev_connect_gpio_out(extioi, (cpu_index * 8 + pin),
> +                                  qdev_get_gpio_in(cpudev, pin + 2));
> +        }
> +    }
> +
> +    return cpu;
> +}
> +
> +static void loongarch_cpu_plug(HotplugHandler *hotplug_dev,
> +                                DeviceState *dev, Error **errp)
> +{
> +    CPUArchId *found_cpu;
> +    HotplugHandlerClass *hhc;
> +    Error *local_err = NULL;
> +    MachineState *machine = MACHINE(OBJECT(hotplug_dev));
> +    LoongArchMachineState *lsms = LOONGARCH_MACHINE(machine);
> +    LoongArchCPU *cpu = LOONGARCH_CPU(dev);
> +    CPUState *cs = CPU(dev);
> +
> +    if (lsms->acpi_ged) {
dont' you need CPUs if you don't have ged?
/is it possible that ged doesn't exists?/

> +        loongarch_cpu_create(machine, cpu, errp);
> +        hhc = HOTPLUG_HANDLER_GET_CLASS(lsms->acpi_ged);
> +        hhc->plug(HOTPLUG_HANDLER(lsms->acpi_ged), dev, &local_err);
> +        if (local_err) {
> +            goto out;
> +        }
> +    }
> +
> +    found_cpu = loongarch_find_cpu_slot(MACHINE(lsms), cs->cpu_index, NULL);
> +    found_cpu->cpu = OBJECT(dev);
> +
> +    return;
> +out:
> +    error_propagate(errp, local_err);
> +}
> +
>  static void loongarch_machine_device_plug_cb(HotplugHandler *hotplug_dev,
>                                          DeviceState *dev, Error **errp)
>  {
> @@ -1053,6 +1383,8 @@ static void loongarch_machine_device_plug_cb(HotplugHandler *hotplug_dev,
>          }
>      } else if (memhp_type_supported(dev)) {
>          virt_mem_plug(hotplug_dev, dev, errp);
> +    } else if (object_dynamic_cast(OBJECT(dev), TYPE_LOONGARCH_CPU)) {
> +        loongarch_cpu_plug(hotplug_dev, dev, errp);
>      }
>  }
>  
> @@ -1062,16 +1394,39 @@ static HotplugHandler *virt_machine_get_hotplug_handler(MachineState *machine,
>      MachineClass *mc = MACHINE_GET_CLASS(machine);
>  
>      if (device_is_dynamic_sysbus(mc, dev) ||
> +        object_dynamic_cast(OBJECT(dev), TYPE_LOONGARCH_CPU) ||
>          memhp_type_supported(dev)) {
>          return HOTPLUG_HANDLER(machine);
>      }
>      return NULL;
>  }
>  
> +static int virt_get_socket_id(const MachineState *ms, int cpu_index)
> +{
> +    assert(cpu_index >= 0 && cpu_index < ms->possible_cpus->len);
> +
> +    return ms->possible_cpus->cpus[cpu_index].props.socket_id;
> +}
> +
> +static int virt_get_core_id(const MachineState *ms, int cpu_index)
> +{
> +    assert(cpu_index >= 0 && cpu_index < ms->possible_cpus->len);
> +
> +    return ms->possible_cpus->cpus[cpu_index].props.core_id;
> +}
> +
> +static int virt_get_thread_id(const MachineState *ms, int cpu_index)
> +{
> +    assert(cpu_index >= 0 && cpu_index < ms->possible_cpus->len);
> +
> +    return ms->possible_cpus->cpus[cpu_index].props.thread_id;
> +}
> +
>  static const CPUArchIdList *virt_possible_cpu_arch_ids(MachineState *ms)
>  {
>      int n;
>      unsigned int max_cpus = ms->smp.max_cpus;
> +    unsigned int smp_threads = ms->smp.threads;
>  
>      if (ms->possible_cpus) {
>          assert(ms->possible_cpus->len == max_cpus);
> @@ -1082,6 +1437,7 @@ static const CPUArchIdList *virt_possible_cpu_arch_ids(MachineState *ms)
>                                    sizeof(CPUArchId) * max_cpus);
>      ms->possible_cpus->len = max_cpus;
>      for (n = 0; n < ms->possible_cpus->len; n++) {
> +        ms->possible_cpus->cpus[n].vcpus_count = smp_threads;
>          ms->possible_cpus->cpus[n].type = ms->cpu_type;
>          ms->possible_cpus->cpus[n].arch_id = n;
>  
> @@ -1125,6 +1481,7 @@ static void loongarch_class_init(ObjectClass *oc, void *data)
>      MachineClass *mc = MACHINE_CLASS(oc);
>      HotplugHandlerClass *hc = HOTPLUG_HANDLER_CLASS(oc);
>  
> +    mc->has_hotpluggable_cpus = true;
>      mc->desc = "Loongson-3A5000 LS7A1000 machine";
>      mc->init = loongarch_init;
>      mc->default_ram_size = 1 * GiB;
> diff --git a/include/hw/loongarch/virt.h b/include/hw/loongarch/virt.h
> index f1659655c6..9ebdba676e 100644
> --- a/include/hw/loongarch/virt.h
> +++ b/include/hw/loongarch/virt.h
> @@ -31,6 +31,7 @@
>  #define VIRT_GED_EVT_ADDR       0x100e0000
>  #define VIRT_GED_MEM_ADDR       (VIRT_GED_EVT_ADDR + ACPI_GED_EVT_SEL_LEN)
>  #define VIRT_GED_REG_ADDR       (VIRT_GED_MEM_ADDR + MEMORY_HOTPLUG_IO_LEN)
> +#define VIRT_GED_CPUHP_ADDR     (VIRT_GED_REG_ADDR + ACPI_CPU_HOTPLUG_REG_LEN)
>  
>  struct LoongArchMachineState {
>      /*< private >*/
> @@ -42,7 +43,7 @@ struct LoongArchMachineState {
>      MemoryRegion bios;
>      bool         bios_loaded;
>      /* State for other subsystems/APIs: */
> -    FWCfgState  *fw_cfg;
> +    FWCfgState   *fw_cfg;
>      Notifier     machine_done;
>      Notifier     powerdown_notifier;
>      OnOffAuto    acpi;
> @@ -50,13 +51,19 @@ struct LoongArchMachineState {
>      char         *oem_table_id;
>      DeviceState  *acpi_ged;
>      int          fdt_size;
> -    DeviceState *platform_bus_dev;
> +    DeviceState  *platform_bus_dev;
>      PCIBus       *pci_bus;
>      PFlashCFI01  *flash;
> +    DeviceState  *extioi;
> +    unsigned int boot_cpus;
>  };
>  
>  #define TYPE_LOONGARCH_MACHINE  MACHINE_TYPE_NAME("virt")
>  OBJECT_DECLARE_SIMPLE_TYPE(LoongArchMachineState, LOONGARCH_MACHINE)
>  bool loongarch_is_acpi_enabled(LoongArchMachineState *lams);
>  void loongarch_acpi_setup(LoongArchMachineState *lams);
> +void virt_madt_cpu_entry(int uid,
> +                         const CPUArchIdList *apic_ids, GArray *entry,
> +                         bool force_enabled);
> +
>  #endif
> diff --git a/target/loongarch/cpu.h b/target/loongarch/cpu.h
> index ed04027af1..f4439c245f 100644
> --- a/target/loongarch/cpu.h
> +++ b/target/loongarch/cpu.h
> @@ -367,6 +367,10 @@ struct ArchCPU {
>      CPUState parent_obj;
>      /*< public >*/
>  
> +    int32_t socket_id;  /* socket-id of this VCPU */
> +    int32_t core_id;    /* core-id of this VCPU */
> +    int32_t thread_id;  /* thread-id of this VCPU */
> +    int32_t node_id;    /* NUMA node this CPU belongs to */
>      CPUNegativeOffsetState neg;
>      CPULoongArchState env;
>      QEMUTimer timer;

Re: [PATCH 6/8] Add support of *unrealize* for loongarch cpu

[Igor Mammedov]

On Thu, 20 Jul 2023 15:15:11 +0800
xianglai li <lixianglai@loongson.cn> wrote:

> 1.Add the Unrealize function to the Loongarch CPU for cpu hot-(un)plug
> 2.Add CPU topology-related properties to the Loongarch CPU for cpu hot-(un)plug
> 
> Cc: Xiaojuan Yang <yangxiaojuan@loongson.cn>
> Cc: Song Gao <gaosong@loongson.cn>
> Cc: "Michael S. Tsirkin" <mst@redhat.com>
> Cc: Igor Mammedov <imammedo@redhat.com>
> Cc: Ani Sinha <anisinha@redhat.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Richard Henderson <richard.henderson@linaro.org>
> Cc: Eduardo Habkost <eduardo@habkost.net>
> Cc: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
> Cc: "Philippe Mathieu-Daudé" <philmd@linaro.org>
> Cc: Yanan Wang <wangyanan55@huawei.com>
> Cc: "Daniel P. Berrangé" <berrange@redhat.com>
> Cc: Peter Xu <peterx@redhat.com>
> Cc: David Hildenbrand <david@redhat.com>
> Signed-off-by: xianglai li <lixianglai@loongson.cn>
> ---
>  target/loongarch/cpu.c | 33 +++++++++++++++++++++++++++++++++
>  target/loongarch/cpu.h |  1 +
>  2 files changed, 34 insertions(+)
> 
> diff --git a/target/loongarch/cpu.c b/target/loongarch/cpu.c
> index ad93ecac92..97c577820f 100644
> --- a/target/loongarch/cpu.c
> +++ b/target/loongarch/cpu.c
> @@ -18,6 +18,7 @@
>  #include "cpu-csr.h"
>  #include "sysemu/reset.h"
>  #include "tcg/tcg.h"
> +#include "hw/qdev-properties.h"
>  
>  const char * const regnames[32] = {
>      "r0", "r1", "r2", "r3", "r4", "r5", "r6", "r7",
> @@ -540,6 +541,24 @@ static void loongarch_cpu_realizefn(DeviceState *dev, Error **errp)
>      lacc->parent_realize(dev, errp);
>  }
>  
> +static void loongarch_cpu_unrealizefn(DeviceState *dev)
> +{
> +    LoongArchCPUClass *mcc = LOONGARCH_CPU_GET_CLASS(dev);
> +
> +#ifndef CONFIG_USER_ONLY
> +    CPUState *cs = CPU(dev);
> +    LoongArchCPU *cpu = LOONGARCH_CPU(dev);
> +    CPULoongArchState *env = &cpu->env;
> +
> +    cpu_remove_sync(CPU(dev));
> +    cpu_address_space_destroy(cs, 0);
> +    address_space_destroy(&env->address_space_iocsr);
> +    memory_region_del_subregion(&env->system_iocsr, &env->iocsr_mem);
> +#endif
> +
> +    mcc->parent_unrealize(dev);
> +}
> +
>  #ifndef CONFIG_USER_ONLY
>  static void loongarch_qemu_write(void *opaque, hwaddr addr,
>                                   uint64_t val, unsigned size)
> @@ -697,6 +716,15 @@ static gchar *loongarch_gdb_arch_name(CPUState *cs)
>      return g_strdup("loongarch64");
>  }
>  


> +static Property loongarch_cpu_properties[] = {
> +    DEFINE_PROP_INT32("socket-id", LoongArchCPU, socket_id, 0),
> +    DEFINE_PROP_INT32("core-id", LoongArchCPU, core_id, 0),
> +    DEFINE_PROP_INT32("thread-id", LoongArchCPU, thread_id, 0),
> +    DEFINE_PROP_INT32("node-id", LoongArchCPU, node_id, CPU_UNSET_NUMA_NODE_ID),
> +
> +    DEFINE_PROP_END_OF_LIST()
> +};
this should be a part of topo patches

> +
>  static void loongarch_cpu_class_init(ObjectClass *c, void *data)
>  {
>      LoongArchCPUClass *lacc = LOONGARCH_CPU_CLASS(c);
> @@ -704,8 +732,12 @@ static void loongarch_cpu_class_init(ObjectClass *c, void *data)
>      DeviceClass *dc = DEVICE_CLASS(c);
>      ResettableClass *rc = RESETTABLE_CLASS(c);
>  
> +    device_class_set_props(dc, loongarch_cpu_properties);
>      device_class_set_parent_realize(dc, loongarch_cpu_realizefn,
>                                      &lacc->parent_realize);
> +    device_class_set_parent_unrealize(dc, loongarch_cpu_unrealizefn,
> +                                  &lacc->parent_unrealize);
> +
>      resettable_class_set_parent_phases(rc, NULL, loongarch_cpu_reset_hold, NULL,
>                                         &lacc->parent_phases);
>  
> @@ -730,6 +762,7 @@ static void loongarch_cpu_class_init(ObjectClass *c, void *data)
>  #ifdef CONFIG_TCG
>      cc->tcg_ops = &loongarch_tcg_ops;
>  #endif
> +    dc->user_creatable = true;
>  }
>  
>  #define DEFINE_LOONGARCH_CPU_TYPE(model, initfn) \
> diff --git a/target/loongarch/cpu.h b/target/loongarch/cpu.h
> index f4439c245f..32feee4fe6 100644
> --- a/target/loongarch/cpu.h
> +++ b/target/loongarch/cpu.h
> @@ -397,6 +397,7 @@ struct LoongArchCPUClass {
>      /*< public >*/
>  
>      DeviceRealize parent_realize;
> +    DeviceUnrealize parent_unrealize;
>      ResettablePhases parent_phases;
>  };
>  

Re: [PATCH 7/8] Update the ACPI table for the Loongarch CPU

[Igor Mammedov]

On Thu, 20 Jul 2023 15:15:12 +0800
xianglai li <lixianglai@loongson.cn> wrote:

> 1.Create a new GED device type for Loongarch,
> mount cpu_madt function to update the ACPI table

madt changes should be its own patch

> 2.Update the APIC table for loongarch based on
> CPU information to support CPU hot-(un)plug
> 
> Cc: Xiaojuan Yang <yangxiaojuan@loongson.cn>
> Cc: Song Gao <gaosong@loongson.cn>
> Cc: "Michael S. Tsirkin" <mst@redhat.com>
> Cc: Igor Mammedov <imammedo@redhat.com>
> Cc: Ani Sinha <anisinha@redhat.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Richard Henderson <richard.henderson@linaro.org>
> Cc: Eduardo Habkost <eduardo@habkost.net>
> Cc: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
> Cc: "Philippe Mathieu-Daudé" <philmd@linaro.org>
> Cc: Yanan Wang <wangyanan55@huawei.com>
> Cc: "Daniel P. Berrangé" <berrange@redhat.com>
> Cc: Peter Xu <peterx@redhat.com>
> Cc: David Hildenbrand <david@redhat.com>
> Signed-off-by: xianglai li <lixianglai@loongson.cn>
> ---
>  hw/acpi/acpi-cpu-hotplug-stub.c               |  9 +++++
>  hw/loongarch/acpi-build.c                     | 35 ++++++++++++++++--
>  hw/loongarch/generic_event_device_loongarch.c | 36 +++++++++++++++++++
>  hw/loongarch/meson.build                      |  2 +-
>  4 files changed, 79 insertions(+), 3 deletions(-)
>  create mode 100644 hw/loongarch/generic_event_device_loongarch.c
> 
> diff --git a/hw/acpi/acpi-cpu-hotplug-stub.c b/hw/acpi/acpi-cpu-hotplug-stub.c
> index 2aec90d968..af9fda2cf4 100644
> --- a/hw/acpi/acpi-cpu-hotplug-stub.c
> +++ b/hw/acpi/acpi-cpu-hotplug-stub.c
> @@ -19,6 +19,15 @@ void legacy_acpi_cpu_hotplug_init(MemoryRegion *parent, Object *owner,
>      return;
>  }
>  
> +void build_cpus_aml(Aml *table, MachineState *machine, CPUHotplugFeatures opts,
> +                    hwaddr mmap_io_base,
> +                    const char *res_root,
> +                    const char *event_handler_method,
> +                    AmlRegionSpace rs)
> +{
> +    return;
> +}
> +
>  void acpi_cpu_ospm_status(CPUHotplugState *cpu_st, ACPIOSTInfoList ***list)
>  {
>      return;
> diff --git a/hw/loongarch/acpi-build.c b/hw/loongarch/acpi-build.c
> index 0b62c3a2f7..312908fb2f 100644
> --- a/hw/loongarch/acpi-build.c
> +++ b/hw/loongarch/acpi-build.c
> @@ -46,6 +46,23 @@
>  #define ACPI_BUILD_DPRINTF(fmt, ...)
>  #endif
>  
> +void virt_madt_cpu_entry(int uid,
> +                         const CPUArchIdList *apic_ids,
> +                         GArray *entry, bool force_enabled)
> +{
> +    uint32_t apic_id = apic_ids->cpus[uid].arch_id;
> +    /* Flags – Local APIC Flags */
> +    uint32_t flags = apic_ids->cpus[uid].cpu != NULL || force_enabled ?
> +                     1 /* Enabled */ : 0;
> +
> +    /* Rev 1.0b, Table 5-13 Processor Local APIC Structure */
> +    build_append_int_noprefix(entry, 0, 1);       /* Type */
> +    build_append_int_noprefix(entry, 8, 1);       /* Length */
> +    build_append_int_noprefix(entry, uid, 1);     /* ACPI Processor ID */
> +    build_append_int_noprefix(entry, apic_id, 1); /* APIC ID */
> +    build_append_int_noprefix(entry, flags, 4); /* Flags */
> +}
> +
>  /* build FADT */
>  static void init_common_fadt_data(AcpiFadtData *data)
>  {
> @@ -121,15 +138,18 @@ build_madt(GArray *table_data, BIOSLinker *linker, LoongArchMachineState *lams)
>      build_append_int_noprefix(table_data, 1 /* PCAT_COMPAT */, 4); /* Flags */
>  
>      for (i = 0; i < arch_ids->len; i++) {
> +        uint32_t flags;
> +
>          /* Processor Core Interrupt Controller Structure */
>          arch_id = arch_ids->cpus[i].arch_id;
> +        flags = arch_ids->cpus[i].cpu ? 1 : 0;
>  
>          build_append_int_noprefix(table_data, 17, 1);    /* Type */
>          build_append_int_noprefix(table_data, 15, 1);    /* Length */
>          build_append_int_noprefix(table_data, 1, 1);     /* Version */
> -        build_append_int_noprefix(table_data, i + 1, 4); /* ACPI Processor ID */
> +        build_append_int_noprefix(table_data, i, 4);     /* ACPI Processor ID */
>          build_append_int_noprefix(table_data, arch_id, 4); /* Core ID */
> -        build_append_int_noprefix(table_data, 1, 4);     /* Flags */
> +        build_append_int_noprefix(table_data, flags, 4);   /* Flags */
>      }
>  
>      /* Extend I/O Interrupt Controller Structure */
> @@ -292,6 +312,17 @@ build_la_ged_aml(Aml *dsdt, MachineState *machine)
>                                   AML_SYSTEM_MEMORY,
>                                   VIRT_GED_MEM_ADDR);
>      }
> +
> +    if (event & ACPI_GED_CPU_HOTPLUG_EVT) {
> +        CPUHotplugFeatures opts = {
> +            .acpi_1_compatible = false,
> +            .has_legacy_cphp = false
> +        };
> +
> +        build_cpus_aml(dsdt, machine, opts, VIRT_GED_CPUHP_ADDR,
> +                       "\\_SB", "\\_GPE._E01", AML_SYSTEM_MEMORY);
> +
> +    }
>      acpi_dsdt_add_power_button(dsdt);
>  }
>  
> diff --git a/hw/loongarch/generic_event_device_loongarch.c b/hw/loongarch/generic_event_device_loongarch.c
> new file mode 100644
> index 0000000000..1fe550239b
> --- /dev/null
> +++ b/hw/loongarch/generic_event_device_loongarch.c
> @@ -0,0 +1,36 @@
> +/*
> + * loongarch 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"
> +#include "hw/loongarch/virt.h"
> +
> +static void acpi_ged_loongarch_class_init(ObjectClass *class, void *data)
> +{
> +    AcpiDeviceIfClass *adevc = ACPI_DEVICE_IF_CLASS(class);
> +
> +    adevc->madt_cpu = virt_madt_cpu_entry;
> +}
> +
> +static const TypeInfo acpi_ged_loongarch_info = {
> +    .name          = TYPE_ACPI_GED_LOONGARCH,
> +    .parent        = TYPE_ACPI_GED,
> +    .class_init    = acpi_ged_loongarch_class_init,
> +    .interfaces = (InterfaceInfo[]) {
> +        { TYPE_HOTPLUG_HANDLER },
> +        { TYPE_ACPI_DEVICE_IF },
> +        { }
> +    }
> +};
> +
> +static void acpi_ged_loongarch_register_types(void)
> +{
> +    type_register_static(&acpi_ged_loongarch_info);
> +}
> +
> +type_init(acpi_ged_loongarch_register_types)
> diff --git a/hw/loongarch/meson.build b/hw/loongarch/meson.build
> index c0421502ab..8d21addee3 100644
> --- a/hw/loongarch/meson.build
> +++ b/hw/loongarch/meson.build
> @@ -3,6 +3,6 @@ loongarch_ss.add(files(
>      'fw_cfg.c',
>  ))
>  loongarch_ss.add(when: 'CONFIG_LOONGARCH_VIRT', if_true: [files('virt.c'), fdt])
> -loongarch_ss.add(when: 'CONFIG_ACPI', if_true: files('acpi-build.c'))
> +loongarch_ss.add(when: 'CONFIG_ACPI', if_true: files('acpi-build.c', 'generic_event_device_loongarch.c'))
>  
>  hw_arch += {'loongarch': loongarch_ss}

Re: [PATCH 8/8] Turn on CPU hot-(un)plug customization for loongarch

[Igor Mammedov]

On Thu, 20 Jul 2023 15:15:13 +0800
xianglai li <lixianglai@loongson.cn> wrote:

> Turn on CPU hot-(un)plug custom for loongarch in the configuration file
> 
> Cc: Xiaojuan Yang <yangxiaojuan@loongson.cn>
> Cc: Song Gao <gaosong@loongson.cn>
> Cc: "Michael S. Tsirkin" <mst@redhat.com>
> Cc: Igor Mammedov <imammedo@redhat.com>
> Cc: Ani Sinha <anisinha@redhat.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Richard Henderson <richard.henderson@linaro.org>
> Cc: Eduardo Habkost <eduardo@habkost.net>
> Cc: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
> Cc: "Philippe Mathieu-Daudé" <philmd@linaro.org>
> Cc: Yanan Wang <wangyanan55@huawei.com>
> Cc: "Daniel P. Berrangé" <berrange@redhat.com>
> Cc: Peter Xu <peterx@redhat.com>
> Cc: David Hildenbrand <david@redhat.com>
> Signed-off-by: xianglai li <lixianglai@loongson.cn>
> ---
>  configs/devices/loongarch64-softmmu/default.mak | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/configs/devices/loongarch64-softmmu/default.mak b/configs/devices/loongarch64-softmmu/default.mak
> index 928bc117ef..e596706fab 100644
> --- a/configs/devices/loongarch64-softmmu/default.mak
> +++ b/configs/devices/loongarch64-softmmu/default.mak
> @@ -1,3 +1,4 @@
>  # Default configuration for loongarch64-softmmu
>  
>  CONFIG_LOONGARCH_VIRT=y
> +CONFIG_ACPI_CPU_HOTPLUG=y
this likely shall be part of prior patch (one that starts to use generic cpu hotplug functions)
otherwise you risk a broke bisection in the middle of series
(aka try to build series after applying each patch)

Re: [PATCH 0/8] Adds CPU hot-plug support to Loongarch

[lixianglai]

Hi,  Salil

On 2023/7/27 pm 10:51, Salil Mehta wrote:
> Hello,
>
>> From: lixianglai <lixianglai@loongson.cn>
>> Sent: Thursday, July 27, 2023 3:14 AM
>> To: Gavin Shan <gshan@redhat.com>; qemu-devel@nongnu.org; Salil Mehta
>> <salil.mehta@huawei.com>; zhukeqian <zhukeqian1@huawei.com>; Bibo Mao
>> <maobibo@loongson.cn>
>> Subject: Re: [PATCH 0/8] Adds CPU hot-plug support to Loongarch
>>
>> Hi Gavin and Salil,
>>
>> On 7/27/23 8:57 AM, Gavin Shan wrote:
>>> Hi Xianglai,
>>>
>>> On 7/20/23 17:15, xianglai li wrote:
>>>> Hello everyone, We refer to the implementation of ARM CPU
>>>> Hot-Plug to add GED-based CPU Hot-Plug support to Loongarch.
>>>>
>>>> The first 4 patches are changes to the QEMU common code,
>>>> including adding GED support for CPU Hot-Plug, updating
>>>> the ACPI table creation process, and adding
>>>> qdev_disconnect_gpio_out_named
>>>> and cpu_address_space_destroy interfaces to release resources
>>>> when CPU un-plug.
>>>>
>>>> The last four patches are Loongarch architecture-related,
>>>> and the modifications include the definition of the hook
>>>> function related to the CPU Hot-(UN)Plug, the allocation
>>>> and release of CPU resources when CPU Hot-(UN)Plug,
>>>> the creation process of updating the ACPI table,
>>>> and finally the custom switch for the CPU Hot-Plug.
>>>>
>>> [...]
>>>
>>> It seems the design for ARM64 hotplug has been greatly referred, but the authors
>>> are missed to be copied if you were referring to the following repository. There
>>> will be conflicts between those two patchsets as I can see and I'm not sure how
>>> it can be resolved. In theory, one patchset needs to be rebased on another one
>>> since they're sharing large amount of codes.
>>>
>>>    https://github.com/salil-mehta/qemu.git
>>>    (branch: virt-cpuhp-armv8/rfc-v1-port11052023.dev-1)
>>>
>>> I took a quick scan on this series. Loongarch doesn't have ARM64's constraint due
>>> to vGIC3 where all vCPUs's file descriptor needs to be in place. It means the possible
>>> and not yet present vCPU needs to be visible to KVM. Without this constraint, the
>>> implementation is simplified a lot.
>> We have great respect and gratitude to Salil and his team for their work
>> and contributions to CPU HotPlug,
>
> Many thanks! Really appreciate this.
>
>
>> which greatly reduced the difficulty of developing CPU HotPlug in
>> Loongarch.
>
> We are glad that this work is useful to other companies as well this
> was one of our goal.
>
>
>> So, We plan to rebase the next version of patch based on their code.
>
> Great. Thanks for clarifying this as accountability is important
> even though we are working in a collaborative environment.
>
> As such, I am planning to send the RFC V2 in 2 weeks of time and
> will make sure that the patches which are required by your patch-set
> are formed in such a way that they can be independently accepted
> w.r.t rest of the ARM64 architecture specific patch-set.
>
>
>> Hi Salil,
>>
>> I estimate that it will take quite some time for your patch series to
>> merge in,
>>
>> if allowed, can you merge your patch series changes to the common code
>> section into the community first,
>>
>> so that our code can be rebase and merged.
>
> Sure, as clarified above, something similar, will create a patch-set
> which will have patches which can be independently accepted/Ack'ed
> and consumed even before the complete patch-set.
>
> Although I think, even in current form most patches have been arranged
> in such a way. But I will doubly check again before I float RFC V2.
I am sorry for the late reply.

Thanks very much,We look forward to joining the community with your 
patch series.

>
>>> Besides, we found the vCPU hotplug isn't working for TCG when Salil's
>>> series was
>>> tested. I'm not sure if we have same issue with this series, or TCG
>>> isn't a concern
>>> to us at all?
>> At present, QEMU only supports TCG under Loongarch,
>>
>> and we test CPU HotPlug is also carried out under QEMU TCG,
>>
>> and CPU HotPlug can work normally when testing.
>>
>> Of course, we are also very happy to see you testing CPU hotplug under
>> Loongarch,
>>
>> which can find more problems and help us improve our patch.
>
> We know that. There are some trivial fixes we already have, I will push
> them as well for the completion. We were not sure if anybody needs them
> as usage of vCPU Hotplug under 'accel=tcg' is highly unlikely except for
> testing cases. Please let us know if you have any?
No, we don't have it yet.
>
> Many thanks!
> Salil.
>

Re: [PATCH 1/8] Update ACPI GED framework to support vcpu hot-(un)plug

[lixianglai]

Hi, Igor Mammedov :

On 7/28/23 7:45 PM, Igor Mammedov wrote:
> On Thu, 20 Jul 2023 15:15:06 +0800
> xianglai li <lixianglai@loongson.cn> wrote:
>
>> ACPI GED shall be used to convey to the guest kernel about any cpu hot-(un)plug
>> events. Therefore, existing ACPI GED framework inside QEMU needs to be enhanced
>> to support CPU hot-(un)plug state and events.
> skimmed through, it looks good to me
>
> see nit below
>
>> Cc: Xiaojuan Yang <yangxiaojuan@loongson.cn>
>> Cc: Song Gao <gaosong@loongson.cn>
>> Cc: "Michael S. Tsirkin" <mst@redhat.com>
>> Cc: Igor Mammedov <imammedo@redhat.com>
>> Cc: Ani Sinha <anisinha@redhat.com>
>> Cc: Paolo Bonzini <pbonzini@redhat.com>
>> Cc: Richard Henderson <richard.henderson@linaro.org>
>> Cc: Eduardo Habkost <eduardo@habkost.net>
>> Cc: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
>> Cc: "Philippe Mathieu-Daudé" <philmd@linaro.org>
>> Cc: Yanan Wang <wangyanan55@huawei.com>
>> Cc: "Daniel P. Berrangé" <berrange@redhat.com>
>> Cc: Peter Xu <peterx@redhat.com>
>> Cc: David Hildenbrand <david@redhat.com>
>> Signed-off-by: xianglai li <lixianglai@loongson.cn>
>> ---
>>   hw/acpi/acpi-cpu-hotplug-stub.c        |  6 +++++
>>   hw/acpi/cpu.c                          |  7 ------
>>   hw/acpi/generic_event_device.c         | 33 ++++++++++++++++++++++++++
>>   include/hw/acpi/cpu_hotplug.h          | 10 ++++++++
>>   include/hw/acpi/generic_event_device.h |  6 +++++
>>   5 files changed, 55 insertions(+), 7 deletions(-)
>>
>> diff --git a/hw/acpi/acpi-cpu-hotplug-stub.c b/hw/acpi/acpi-cpu-hotplug-stub.c
>> index 3fc4b14c26..2aec90d968 100644
>> --- a/hw/acpi/acpi-cpu-hotplug-stub.c
>> +++ b/hw/acpi/acpi-cpu-hotplug-stub.c
>> @@ -24,6 +24,12 @@ void acpi_cpu_ospm_status(CPUHotplugState *cpu_st, ACPIOSTInfoList ***list)
>>       return;
>>   }
>>   
>> +void cpu_hotplug_hw_init(MemoryRegion *as, Object *owner,
>> +                         CPUHotplugState *state, hwaddr base_addr)
>> +{
>> +    return;
>> +}
>> +
>>   void acpi_cpu_plug_cb(HotplugHandler *hotplug_dev,
>>                         CPUHotplugState *cpu_st, DeviceState *dev, Error **errp)
>>   {
>> diff --git a/hw/acpi/cpu.c b/hw/acpi/cpu.c
>> index 19c154d78f..6897c8789a 100644
>> --- a/hw/acpi/cpu.c
>> +++ b/hw/acpi/cpu.c
>> @@ -6,13 +6,6 @@
>>   #include "trace.h"
>>   #include "sysemu/numa.h"
>>   
>> -#define ACPI_CPU_HOTPLUG_REG_LEN 12
>> -#define ACPI_CPU_SELECTOR_OFFSET_WR 0
>> -#define ACPI_CPU_FLAGS_OFFSET_RW 4
>> -#define ACPI_CPU_CMD_OFFSET_WR 5
>> -#define ACPI_CPU_CMD_DATA_OFFSET_RW 8
>> -#define ACPI_CPU_CMD_DATA2_OFFSET_R 0
>> -
>>   #define OVMF_CPUHP_SMI_CMD 4
>>   
>>   enum {
>> diff --git a/hw/acpi/generic_event_device.c b/hw/acpi/generic_event_device.c
>> index a3d31631fe..c5a70957b4 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,
>>   };
>>   
>>   /*
>> @@ -117,6 +119,10 @@ 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_CPU_HOTPLUG_EVT:
>> +                aml_append(if_ctx, aml_call0(ACPI_CPU_CONTAINER "."
>> +                                             ACPI_CPU_SCAN_METHOD));
>> +                break;
>>               default:
>>                   /*
>>                    * Please make sure all the events in ged_supported_events[]
>> @@ -234,6 +240,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 +256,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 +271,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 +284,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 +299,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);
>> @@ -318,6 +333,16 @@ static const VMStateDescription vmstate_memhp_state = {
>>       }
>>   };
>>   
>> +static const VMStateDescription vmstate_cpuhp_state = {
>> +    .name = "acpi-ged/cpuhp",
>> +    .version_id = 1,
>> +    .minimum_version_id = 1,
>> +    .fields      = (VMStateField[]) {
>> +        VMSTATE_CPU_HOTPLUG(cpuhp_state, AcpiGedState),
>> +        VMSTATE_END_OF_LIST()
>> +    }
>> +};
>> +
>>   static const VMStateDescription vmstate_ged_state = {
>>       .name = "acpi-ged-state",
>>       .version_id = 1,
>> @@ -366,6 +391,7 @@ static const VMStateDescription vmstate_acpi_ged = {
>>       },
>>       .subsections = (const VMStateDescription * []) {
>>           &vmstate_memhp_state,
>> +        &vmstate_cpuhp_state,
>>           &vmstate_ghes_state,
>>           NULL
>>       }
>> @@ -400,6 +426,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/cpu_hotplug.h b/include/hw/acpi/cpu_hotplug.h
>> index 3b932abbbb..afee1ab996 100644
>> --- a/include/hw/acpi/cpu_hotplug.h
>> +++ b/include/hw/acpi/cpu_hotplug.h
>> @@ -19,6 +19,16 @@
>>   #include "hw/hotplug.h"
>>   #include "hw/acpi/cpu.h"
>>   
>> +#define ACPI_CPU_HOTPLUG_REG_LEN 12
>> +#define ACPI_CPU_SELECTOR_OFFSET_WR 0
>> +#define ACPI_CPU_FLAGS_OFFSET_RW 4
>> +#define ACPI_CPU_CMD_OFFSET_WR 5
>> +#define ACPI_CPU_CMD_DATA_OFFSET_RW 8
>> +#define ACPI_CPU_CMD_DATA2_OFFSET_R 0
>> +
>> +#define ACPI_CPU_SCAN_METHOD "CSCN"
>> +#define ACPI_CPU_CONTAINER "\\_SB.CPUS"
>> +
>>   typedef struct AcpiCpuHotplug {
>>       Object *device;
>>       MemoryRegion io;
>> diff --git a/include/hw/acpi/generic_event_device.h b/include/hw/acpi/generic_event_device.h
>> index d831bbd889..2923bd9d82 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"
>> @@ -70,6 +71,7 @@
>>   OBJECT_DECLARE_SIMPLE_TYPE(AcpiGedState, ACPI_GED)
>>   
>>   #define TYPE_ACPI_GED_X86 "acpi-ged-x86"
>> +#define TYPE_ACPI_GED_LOONGARCH "acpi-ged-loongarch"
> where is it used?
> If it's for later patches, it should be in the patch that
> will actually use it

Yes, it will be used in a later patch,

and I will modify its location in the next patch release.

>
>>   
>>   #define ACPI_GED_EVT_SEL_OFFSET    0x0
>>   #define ACPI_GED_EVT_SEL_LEN       0x4
>> @@ -97,6 +99,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;
>> @@ -108,6 +111,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.

xianglai

Re: [PATCH 2/8] Update CPUs AML with cpu-(ctrl)dev change

[lixianglai]

Hi, Igor Mammedov :

On 7/28/23 7:55 PM, Igor Mammedov wrote:
> On Thu, 20 Jul 2023 15:15:07 +0800
> xianglai li <lixianglai@loongson.cn> wrote:
>
>> CPUs Control device(\\_SB.PCI0) register interface for the x86 arch is based on
>> PCI and is IO port based and hence existing cpus AML code assumes _CRS objects
>> would evaluate to a system resource which describes IO Port address. But on LOONGARCH
>> arch CPUs control device(\\_SB.PRES) register interface is memory-mapped hence
>> _CRS object should evaluate to system resource which describes memory-mapped
>> base address.
>>
>> This cpus AML code change updates the existing inerface of the build cpus AML
>                                                   ^^^ typo
Oh, I'm sorry for the typo.
>> function to accept both IO/MEMORY type regions and update the _CRS object
>> correspondingly.
> try to reformat commit message  to less than 80 character per line
Ok! I'll modify it as suggested in the next version of patch.
>> Cc: Xiaojuan Yang <yangxiaojuan@loongson.cn>
>> Cc: Song Gao <gaosong@loongson.cn>
>> Cc: "Michael S. Tsirkin" <mst@redhat.com>
>> Cc: Igor Mammedov <imammedo@redhat.com>
>> Cc: Ani Sinha <anisinha@redhat.com>
>> Cc: Paolo Bonzini <pbonzini@redhat.com>
>> Cc: Richard Henderson <richard.henderson@linaro.org>
>> Cc: Eduardo Habkost <eduardo@habkost.net>
>> Cc: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
>> Cc: "Philippe Mathieu-Daudé" <philmd@linaro.org>
>> Cc: Yanan Wang <wangyanan55@huawei.com>
>> Cc: "Daniel P. Berrangé" <berrange@redhat.com>
>> Cc: Peter Xu <peterx@redhat.com>
>> Cc: David Hildenbrand <david@redhat.com>
>> Signed-off-by: xianglai li <lixianglai@loongson.cn>
>> ---
>>   hw/acpi/cpu.c         | 30 +++++++++++++++++++++++-------
>>   hw/i386/acpi-build.c  |  2 +-
>>   include/hw/acpi/cpu.h |  5 +++--
>>   3 files changed, 27 insertions(+), 10 deletions(-)
>>
>> diff --git a/hw/acpi/cpu.c b/hw/acpi/cpu.c
>> index 6897c8789a..3b945a1a40 100644
>> --- a/hw/acpi/cpu.c
>> +++ b/hw/acpi/cpu.c
>> @@ -5,6 +5,7 @@
>>   #include "qapi/qapi-events-acpi.h"
>>   #include "trace.h"
>>   #include "sysemu/numa.h"
>> +#include "hw/acpi/cpu_hotplug.h"
>>   
>>   #define OVMF_CPUHP_SMI_CMD 4
>>   
>> @@ -331,9 +332,10 @@ const VMStateDescription vmstate_cpu_hotplug = {
>>   #define CPU_FW_EJECT_EVENT "CEJF"
>>   
>>   void build_cpus_aml(Aml *table, MachineState *machine, CPUHotplugFeatures opts,
>> -                    hwaddr io_base,
>> +                    hwaddr mmap_io_base,
>>                       const char *res_root,
>> -                    const char *event_handler_method)
>> +                    const char *event_handler_method,
>> +                    AmlRegionSpace rs)
>>   {
>>       Aml *ifctx;
>>       Aml *field;
>> @@ -360,14 +362,28 @@ void build_cpus_aml(Aml *table, MachineState *machine, CPUHotplugFeatures opts,
>>           aml_append(cpu_ctrl_dev, aml_mutex(CPU_LOCK, 0));
>>   
>>           crs = aml_resource_template();
>> -        aml_append(crs, aml_io(AML_DECODE16, io_base, io_base, 1,
>> +        if (rs == AML_SYSTEM_IO) {
>> +            aml_append(crs, aml_io(AML_DECODE16, mmap_io_base, mmap_io_base, 1,
>>                                  ACPI_CPU_HOTPLUG_REG_LEN));
>> +        } else {
>> +            aml_append(crs, aml_memory32_fixed(mmap_io_base,
>> +                               ACPI_CPU_HOTPLUG_REG_LEN, AML_READ_WRITE));
>> +        }
>>           aml_append(cpu_ctrl_dev, aml_name_decl("_CRS", crs));
>>   
>> -        /* declare CPU hotplug MMIO region with related access fields */
>> -        aml_append(cpu_ctrl_dev,
>> -            aml_operation_region("PRST", AML_SYSTEM_IO, aml_int(io_base),
>> -                                 ACPI_CPU_HOTPLUG_REG_LEN));
>> +        if (rs == AML_SYSTEM_IO) {
>> +            /* declare CPU hotplug MMIO region with related access fields */
>> +            aml_append(cpu_ctrl_dev,
>> +                aml_operation_region("PRST", AML_SYSTEM_IO,
>> +                                             aml_int(mmap_io_base),
>> +                                             ACPI_CPU_HOTPLUG_REG_LEN));
>> +        } else {
>> +            aml_append(cpu_ctrl_dev,
>> +                aml_operation_region("PRST", AML_SYSTEM_MEMORY,
>> +                                             aml_int(mmap_io_base),
>> +                                             ACPI_CPU_HOTPLUG_REG_LEN));
>> +        }
>
> to reduce duplication, following could be better way to spell it:
>   g_assert(rs == foo1 || rs == foo2)
>   ... aml_operation_region("PRST", rs, aml_int(io_base), ...


Well, this suggestion makes the code look good, I'll fix it in the next 
patch release.


>>   
>>           field = aml_field("PRST", AML_BYTE_ACC, AML_NOLOCK,
>>                             AML_WRITE_AS_ZEROS);
>> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
>> index 9c74fa17ad..5d02690593 100644
>> --- a/hw/i386/acpi-build.c
>> +++ b/hw/i386/acpi-build.c
>> @@ -1548,7 +1548,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
>>               .fw_unplugs_cpu = pm->smi_on_cpu_unplug,
>>           };
>>           build_cpus_aml(dsdt, machine, opts, pm->cpu_hp_io_base,
>> -                       "\\_SB.PCI0", "\\_GPE._E02");
>> +                       "\\_SB.PCI0", "\\_GPE._E02", AML_SYSTEM_IO);
>>       }
>>   
>>       if (pcms->memhp_io_base && nr_mem) {
>> diff --git a/include/hw/acpi/cpu.h b/include/hw/acpi/cpu.h
>> index 999caaf510..cddea78333 100644
>> --- a/include/hw/acpi/cpu.h
>> +++ b/include/hw/acpi/cpu.h
>> @@ -56,9 +56,10 @@ typedef struct CPUHotplugFeatures {
>>   } CPUHotplugFeatures;
>>   
>>   void build_cpus_aml(Aml *table, MachineState *machine, CPUHotplugFeatures opts,
>> -                    hwaddr io_base,
>> +                    hwaddr mmap_io_base,
>>                       const char *res_root,
>> -                    const char *event_handler_method);
>> +                    const char *event_handler_method,
>> +                    AmlRegionSpace rs);
>>   
>>   void acpi_cpu_ospm_status(CPUHotplugState *cpu_st, ACPIOSTInfoList ***list);
>>   

Thanks!

xianglai.

Re: [PATCH 3/8] Introduced a new function to disconnect GPIO connections

[lixianglai]

Hi, Igor Mammedov:

On 7/28/23 7:59 PM, Igor Mammedov wrote:
> On Thu, 20 Jul 2023 15:15:08 +0800
> xianglai li <lixianglai@loongson.cn> wrote:
>
>> It introduces a new function to unwire the
>> vcpu<->exioi interrupts for the vcpu hot-(un)plug cases.
> it's not a new function.
> You probably wanted to say:
>
> subj: make foo() public
>
> it will be reused .someplace. for ...
Ok, I'll fix it in the next version of patch.
>> Cc: Xiaojuan Yang <yangxiaojuan@loongson.cn>
>> Cc: Song Gao <gaosong@loongson.cn>
>> Cc: "Michael S. Tsirkin" <mst@redhat.com>
>> Cc: Igor Mammedov <imammedo@redhat.com>
>> Cc: Ani Sinha <anisinha@redhat.com>
>> Cc: Paolo Bonzini <pbonzini@redhat.com>
>> Cc: Richard Henderson <richard.henderson@linaro.org>
>> Cc: Eduardo Habkost <eduardo@habkost.net>
>> Cc: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
>> Cc: "Philippe Mathieu-Daudé" <philmd@linaro.org>
>> Cc: Yanan Wang <wangyanan55@huawei.com>
>> Cc: "Daniel P. Berrangé" <berrange@redhat.com>
>> Cc: Peter Xu <peterx@redhat.com>
>> Cc: David Hildenbrand <david@redhat.com>
>> Signed-off-by: xianglai li <lixianglai@loongson.cn>
>> ---
>>   hw/core/gpio.c         | 4 ++--
>>   include/hw/qdev-core.h | 2 ++
>>   2 files changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/hw/core/gpio.c b/hw/core/gpio.c
>> index 80d07a6ec9..4fc6409545 100644
>> --- a/hw/core/gpio.c
>> +++ b/hw/core/gpio.c
>> @@ -143,8 +143,8 @@ qemu_irq qdev_get_gpio_out_connector(DeviceState *dev, const char *name, int n)
>>   
>>   /* disconnect a GPIO output, returning the disconnected input (if any) */
>>   
>> -static qemu_irq qdev_disconnect_gpio_out_named(DeviceState *dev,
>> -                                               const char *name, int n)
>> +qemu_irq qdev_disconnect_gpio_out_named(DeviceState *dev,
>> +                                        const char *name, int n)
>>   {
>>       char *propname = g_strdup_printf("%s[%d]",
>>                                        name ? name : "unnamed-gpio-out", n);
>> diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
>> index 884c726a87..992f5419fa 100644
>> --- a/include/hw/qdev-core.h
>> +++ b/include/hw/qdev-core.h
>> @@ -739,6 +739,8 @@ qemu_irq qdev_get_gpio_out_connector(DeviceState *dev, const char *name, int n);
>>    */
>>   qemu_irq qdev_intercept_gpio_out(DeviceState *dev, qemu_irq icpt,
>>                                    const char *name, int n);
>> +qemu_irq qdev_disconnect_gpio_out_named(DeviceState *dev,
>> +                                               const char *name, int n);
> watch for proper alignment
>
> have you tried to run ./scripts/checkpatch.pl on you series?
> (it should catch such cases)

Ok, I'll fix it in the next version of patch.

I ran ./scripts/checkpatch.pl on my patch.pl but didn't check for the 
problem.

>>   
>>   BusState *qdev_get_child_bus(DeviceState *dev, const char *name);
>>   

Thanks,

xianglai.

Re: [PATCH 4/8] Introduce the CPU address space destruction function

[lixianglai]

Hi Igor Mammedov:

The first four patches are written with reference to the patch in the public

modification section of Arm's CPU Hotplug, and the Arm CPU HotPlug-related

patches will be merged into the community in the near future, so the 
first four

patches will be discarded and rebase based on the latest code.


Thanks,

xianglai.


On 7/28/23 8:13 PM, Igor Mammedov wrote:
> On Thu, 20 Jul 2023 15:15:09 +0800
> xianglai li <lixianglai@loongson.cn> wrote:
>
>> Introduce new functions to destroy CPU address space resources
> s/functions/function/
>
>> for cpu hot-(un)plug.
>>
>> Cc: Xiaojuan Yang <yangxiaojuan@loongson.cn>
>> Cc: Song Gao <gaosong@loongson.cn>
>> Cc: "Michael S. Tsirkin" <mst@redhat.com>
>> Cc: Igor Mammedov <imammedo@redhat.com>
>> Cc: Ani Sinha <anisinha@redhat.com>
>> Cc: Paolo Bonzini <pbonzini@redhat.com>
>> Cc: Richard Henderson <richard.henderson@linaro.org>
>> Cc: Eduardo Habkost <eduardo@habkost.net>
>> Cc: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
>> Cc: "Philippe Mathieu-Daudé" <philmd@linaro.org>
>> Cc: Yanan Wang <wangyanan55@huawei.com>
>> Cc: "Daniel P. Berrangé" <berrange@redhat.com>
>> Cc: Peter Xu <peterx@redhat.com>
>> Cc: David Hildenbrand <david@redhat.com>
>> Signed-off-by: xianglai li <lixianglai@loongson.cn>
>> ---
>>   include/exec/cpu-common.h |  8 ++++++++
>>   include/hw/core/cpu.h     |  1 +
>>   softmmu/physmem.c         | 24 ++++++++++++++++++++++++
>>   3 files changed, 33 insertions(+)
>>
>> diff --git a/include/exec/cpu-common.h b/include/exec/cpu-common.h
>> index 87dc9a752c..27cd4d32b1 100644
>> --- a/include/exec/cpu-common.h
>> +++ b/include/exec/cpu-common.h
>> @@ -120,6 +120,14 @@ size_t qemu_ram_pagesize_largest(void);
>>    */
>>   void cpu_address_space_init(CPUState *cpu, int asidx,
>>                               const char *prefix, MemoryRegion *mr);
>> +/**
>> + * cpu_address_space_destroy:
>> + * @cpu: CPU for which address space needs to be destroyed
>> + * @asidx: integer index of this address space
>> + *
>> + * Note that with KVM only one address space is supported.
>> + */
>> +void cpu_address_space_destroy(CPUState *cpu, int asidx);
>>   
>>   void cpu_physical_memory_rw(hwaddr addr, void *buf,
>>                               hwaddr len, bool is_write);
>> diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
>> index fdcbe87352..d6d68dac12 100644
>> --- a/include/hw/core/cpu.h
>> +++ b/include/hw/core/cpu.h
>> @@ -366,6 +366,7 @@ struct CPUState {
>>       QSIMPLEQ_HEAD(, qemu_work_item) work_list;
>>   
>>       CPUAddressSpace *cpu_ases;
>> +    int cpu_ases_ref_count;
> perhaps renaming it to num_ases would be better
>
>>       int num_ases;
> and this one can be named num__ases_[total|max]
>
>
>>       AddressSpace *as;
>>       MemoryRegion *memory;
>> diff --git a/softmmu/physmem.c b/softmmu/physmem.c
>> index 3df73542e1..f4545b4508 100644
>> --- a/softmmu/physmem.c
>> +++ b/softmmu/physmem.c
>> @@ -762,6 +762,7 @@ void cpu_address_space_init(CPUState *cpu, int asidx,
>>   
>>       if (!cpu->cpu_ases) {
>>           cpu->cpu_ases = g_new0(CPUAddressSpace, cpu->num_ases);
>> +        cpu->cpu_ases_ref_count = cpu->num_ases;
>>       }
>>   
>>       newas = &cpu->cpu_ases[asidx];
>> @@ -775,6 +776,29 @@ void cpu_address_space_init(CPUState *cpu, int asidx,
>>       }
>>   }
>>   
>> +void cpu_address_space_destroy(CPUState *cpu, int asidx)
>                                                   ^^^ should it be uintX_t ?
>
>> +{
>> +    CPUAddressSpace *cpuas;
>> +
>> +    assert(asidx < cpu->num_ases);
>> +    assert(asidx == 0 || !kvm_enabled());
>> +    assert(cpu->cpu_ases);
>> +
>> +    cpuas = &cpu->cpu_ases[asidx];
>> +    if (tcg_enabled()) {
>> +        memory_listener_unregister(&cpuas->tcg_as_listener);
>> +    }
>> +
>> +    address_space_destroy(cpuas->as);
>> +
>> +    cpu->cpu_ases_ref_count--;
>> +    if (cpu->cpu_ases_ref_count == 0) {
>> +        g_free(cpu->cpu_ases);
>> +        cpu->cpu_ases = NULL;
>> +    }
>> +
>> +}
>> +
>>   AddressSpace *cpu_get_address_space(CPUState *cpu, int asidx)
>>   {
>>       /* Return the AddressSpace corresponding to the specified index */

Re: [PATCH 3/8] Introduced a new function to disconnect GPIO connections

[lixianglai]

Hi,Peter Maydell :


On 7/28/23 8:38 PM, Peter Maydell wrote:
> On Thu, 20 Jul 2023 at 08:16, xianglai li <lixianglai@loongson.cn> wrote:
>> It introduces a new function to unwire the
>> vcpu<->exioi interrupts for the vcpu hot-(un)plug cases.
>>
>> Cc: Xiaojuan Yang <yangxiaojuan@loongson.cn>
>> Cc: Song Gao <gaosong@loongson.cn>
>> Cc: "Michael S. Tsirkin" <mst@redhat.com>
>> Cc: Igor Mammedov <imammedo@redhat.com>
>> Cc: Ani Sinha <anisinha@redhat.com>
>> Cc: Paolo Bonzini <pbonzini@redhat.com>
>> Cc: Richard Henderson <richard.henderson@linaro.org>
>> Cc: Eduardo Habkost <eduardo@habkost.net>
>> Cc: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
>> Cc: "Philippe Mathieu-Daudé" <philmd@linaro.org>
>> Cc: Yanan Wang <wangyanan55@huawei.com>
>> Cc: "Daniel P. Berrangé" <berrange@redhat.com>
>> Cc: Peter Xu <peterx@redhat.com>
>> Cc: David Hildenbrand <david@redhat.com>
>> Signed-off-by: xianglai li <lixianglai@loongson.cn>
>> ---
>>   hw/core/gpio.c         | 4 ++--
>>   include/hw/qdev-core.h | 2 ++
>>   2 files changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/hw/core/gpio.c b/hw/core/gpio.c
>> index 80d07a6ec9..4fc6409545 100644
>> --- a/hw/core/gpio.c
>> +++ b/hw/core/gpio.c
>> @@ -143,8 +143,8 @@ qemu_irq qdev_get_gpio_out_connector(DeviceState *dev, const char *name, int n)
>>
>>   /* disconnect a GPIO output, returning the disconnected input (if any) */
>>
>> -static qemu_irq qdev_disconnect_gpio_out_named(DeviceState *dev,
>> -                                               const char *name, int n)
>> +qemu_irq qdev_disconnect_gpio_out_named(DeviceState *dev,
>> +                                        const char *name, int n)
>>   {
>>       char *propname = g_strdup_printf("%s[%d]",
>>                                        name ? name : "unnamed-gpio-out", n);
> This function as currently written is intended only for
> use in qdev_intercept_gpio_out(), which in turn is there
> only for qtest test case use. I would be a bit cautious
> about whether there are unexpected issues with trying
> to use it in "production" functionality with a running
> QEMU (eg when perhaps some device might be trying to
> signal the gpio line in another thread while this one
> is trying to disconnect it).

That's how I understand it:

Take Loongarch's extended interrupt controller, for example:

1.The interrupt pin of the extended interrupt controller is initialized in

static void loongarch_extioi_instance_init(Object *obj)

{

....

     for (cpu = 0; cpu < EXTIOI_CPUS; cpu++) {
....
         for (pin = 0; pin < LS3A_INTC_IP; pin++) {
             qdev_init_gpio_out(DEVICE(obj), &s->parent_irq[cpu][pin], 1);
         }
     }

.....

}

The above function binds the extended interrupt pin to the variable 
s->parent_irq[cpu][pin] pointer.

2.The assignment of the interrupt pin of the extended interrupt 
controller operates in

static LoongArchCPU *loongarch_cpu_create(MachineState *machine, 
LoongArchCPU *cpu, Error **errp)
{
...
         for (pin = 0; pin < LS3A_INTC_IP; pin++) {
             qdev_connect_gpio_out(extioi, (cpu_index * 8 + pin), 
qdev_get_gpio_in(cpudev, pin + 2));
         }
...
}

The above function binds the extended interrupt pin to the return value 
of the qdev_get_gpio_in(cpudev, pin + 2).

You actually do the following:

s->parent_irq[cpu][pin] = qdev_get_gpio_in(cpudev, pin + 2);

Variables are only accessed when the interrupt controller's IO space is 
extended to read and write.

There are QEMU thread locks when reading and writing in IO space：

static inline void glue(address_space_stw_internal, SUFFIX)(ARG1_DECL, 
hwaddr addr, uint16_t val, MemTxAttrs attrs, MemTxResult *result, enum 
device_endian endian)
{
...
     RCU_READ_LOCK();
...
     release_lock |= prepare_mmio_access(mr);
     r = memory_region_dispatch_write(mr, addr1, val, MO_16 | 
devend_memop(endian), attrs);
...
     if (release_lock) {
         qemu_mutex_unlock_iothread();
     }
     RCU_READ_UNLOCK();
...
}

Therefore, both are locked by the thread of QEMU when the CPU is 
unplugged or interrupted sent, which can ensure that the two are 
mutually exclusive.

 From the call stack below, it can be seen that interrupt sending and 
CPU unplugging are in two worker threads, but both worker threads are 
mutexe.

for example:

Thread 1 "qemu-system-loo" hit Breakpoint 5, qemu_set_irq
41          if (!irq)
(gdb) bt
#0  0x000000aaab1e3630 in qemu_set_irq
#1  0x000000aaab1324d8 in loongarch_msi_mem_write
#2  0x000000aaab1835e0 in memory_region_write_accessor
#3  0x000000aaab182e38 in access_with_adjusted_size
#4  0x000000aaab18323c in memory_region_dispatch_write
#5  0x000000aaab190cf8 in address_space_stl_internal
#6  0x000000aaab190cf8 in address_space_stl_le
#7  0x000000aaab3714a8 in aio_dispatch_handler
#8  0x000000aaab371e20 in aio_dispatch_handlers
#9  0x000000aaab371e20 in aio_dispatch
#10 0x000000aaab388ae0 in aio_ctx_dispatch
#11 0x000000fff724d334 in g_main_context_dispatch
#12 0x000000aaab38a390 in glib_pollfds_poll
#13 0x000000aaab38a390 in os_host_main_loop_wait
#14 0x000000aaab38a390 in main_loop_wait
#15 0x000000aaab05aa0c in qemu_main_loop
#16 0x000000aaab1ddf2c in qemu_default_main
#17 0x000000fff60d9670 in __libc_start_main
#18 0x000000aaaae719fc in _start ()
(gdb) call qemu_mutex_iothread_locked()
$5 = true

Thread 4 "qemu-system-loo" hit Breakpoint 2, loongarch_cpu_destroy
1177        LoongArchMachineState *lsms = LOONGARCH_MACHINE(machine);
(gdb) bt
#0  0x000000aaab0eeb60 in loongarch_cpu_destroy
#1  0x000000aaab0eeb60 in loongarch_cpu_unplug
#2  0x000000aaab0eeb60 in virt_machine_device_unplug
#3  0x000000aaab0eeb60 in virt_machine_device_unplug
#4  0x000000aaaaeca0d8 in cpu_hotplug_wr
#5  0x000000aaab1835e0 in memory_region_write_accessor
#6  0x000000aaab182e38 in access_with_adjusted_size
#7  0x000000aaab18323c in memory_region_dispatch_write
#8  0x000000aaab18a914 in flatview_write_continue
#9  0x000000aaab18aae0 in flatview_write
#10 0x000000aaab18ab7c in subpage_write
#11 0x000000aaab183328 in memory_region_write_with_attrs_accessor
#12 0x000000aaab182e38 in access_with_adjusted_size
#13 0x000000aaab18323c in memory_region_dispatch_write
#14 0x000000aaab1ca714 in io_writex
#15 0x000000aaab1cfdfc in helper_stb_mmu
#16 0x000000ffa55a2224 in code_gen_buffer ()
#17 0x000000aaab1be7d0 in tb_lookup
#18 0x000000aaab1be7d0 in cpu_exec_loop
#19 0x000000aaab1bf04c in cpu_exec_setjmp
#20 0x000000aaab1bf0f8 in cpu_exec
#21 0x000000aaab1da660 in tcg_cpus_exec
#22 0x000000aaab1da7e0 in mttcg_cpu_thread_fn
#23 0x000000aaab374fd4 in qemu_thread_start
#24 0x000000fff623ddc8 in start_thread
#25 0x000000fff618d8fc in __thread_start
(gdb) call qemu_mutex_iothread_locked()
$6 = true

Thanks,

xianglai

> thanks
> -- PMM

Re: [PATCH 6/8] Add support of *unrealize* for loongarch cpu

[lixianglai]

Hi Igor Mammedov:


On 7/28/23 9:23 PM, Igor Mammedov wrote:
> On Thu, 20 Jul 2023 15:15:11 +0800
> xianglai li <lixianglai@loongson.cn> wrote:
>
>> 1.Add the Unrealize function to the Loongarch CPU for cpu hot-(un)plug
>> 2.Add CPU topology-related properties to the Loongarch CPU for cpu hot-(un)plug
>>
>> Cc: Xiaojuan Yang <yangxiaojuan@loongson.cn>
>> Cc: Song Gao <gaosong@loongson.cn>
>> Cc: "Michael S. Tsirkin" <mst@redhat.com>
>> Cc: Igor Mammedov <imammedo@redhat.com>
>> Cc: Ani Sinha <anisinha@redhat.com>
>> Cc: Paolo Bonzini <pbonzini@redhat.com>
>> Cc: Richard Henderson <richard.henderson@linaro.org>
>> Cc: Eduardo Habkost <eduardo@habkost.net>
>> Cc: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
>> Cc: "Philippe Mathieu-Daudé" <philmd@linaro.org>
>> Cc: Yanan Wang <wangyanan55@huawei.com>
>> Cc: "Daniel P. Berrangé" <berrange@redhat.com>
>> Cc: Peter Xu <peterx@redhat.com>
>> Cc: David Hildenbrand <david@redhat.com>
>> Signed-off-by: xianglai li <lixianglai@loongson.cn>
>> ---
>>   target/loongarch/cpu.c | 33 +++++++++++++++++++++++++++++++++
>>   target/loongarch/cpu.h |  1 +
>>   2 files changed, 34 insertions(+)
>>
>> diff --git a/target/loongarch/cpu.c b/target/loongarch/cpu.c
>> index ad93ecac92..97c577820f 100644
>> --- a/target/loongarch/cpu.c
>> +++ b/target/loongarch/cpu.c
>> @@ -18,6 +18,7 @@
>>   #include "cpu-csr.h"
>>   #include "sysemu/reset.h"
>>   #include "tcg/tcg.h"
>> +#include "hw/qdev-properties.h"
>>   
>>   const char * const regnames[32] = {
>>       "r0", "r1", "r2", "r3", "r4", "r5", "r6", "r7",
>> @@ -540,6 +541,24 @@ static void loongarch_cpu_realizefn(DeviceState *dev, Error **errp)
>>       lacc->parent_realize(dev, errp);
>>   }
>>   
>> +static void loongarch_cpu_unrealizefn(DeviceState *dev)
>> +{
>> +    LoongArchCPUClass *mcc = LOONGARCH_CPU_GET_CLASS(dev);
>> +
>> +#ifndef CONFIG_USER_ONLY
>> +    CPUState *cs = CPU(dev);
>> +    LoongArchCPU *cpu = LOONGARCH_CPU(dev);
>> +    CPULoongArchState *env = &cpu->env;
>> +
>> +    cpu_remove_sync(CPU(dev));
>> +    cpu_address_space_destroy(cs, 0);
>> +    address_space_destroy(&env->address_space_iocsr);
>> +    memory_region_del_subregion(&env->system_iocsr, &env->iocsr_mem);
>> +#endif
>> +
>> +    mcc->parent_unrealize(dev);
>> +}
>> +
>>   #ifndef CONFIG_USER_ONLY
>>   static void loongarch_qemu_write(void *opaque, hwaddr addr,
>>                                    uint64_t val, unsigned size)
>> @@ -697,6 +716,15 @@ static gchar *loongarch_gdb_arch_name(CPUState *cs)
>>       return g_strdup("loongarch64");
>>   }
>>   
>
>> +static Property loongarch_cpu_properties[] = {
>> +    DEFINE_PROP_INT32("socket-id", LoongArchCPU, socket_id, 0),
>> +    DEFINE_PROP_INT32("core-id", LoongArchCPU, core_id, 0),
>> +    DEFINE_PROP_INT32("thread-id", LoongArchCPU, thread_id, 0),
>> +    DEFINE_PROP_INT32("node-id", LoongArchCPU, node_id, CPU_UNSET_NUMA_NODE_ID),
>> +
>> +    DEFINE_PROP_END_OF_LIST()
>> +};
> this should be a part of topo patches


Ok, I'll move it to the topo patch.


Thanks,

xianglai

>> +
>>   static void loongarch_cpu_class_init(ObjectClass *c, void *data)
>>   {
>>       LoongArchCPUClass *lacc = LOONGARCH_CPU_CLASS(c);
>> @@ -704,8 +732,12 @@ static void loongarch_cpu_class_init(ObjectClass *c, void *data)
>>       DeviceClass *dc = DEVICE_CLASS(c);
>>       ResettableClass *rc = RESETTABLE_CLASS(c);
>>   
>> +    device_class_set_props(dc, loongarch_cpu_properties);
>>       device_class_set_parent_realize(dc, loongarch_cpu_realizefn,
>>                                       &lacc->parent_realize);
>> +    device_class_set_parent_unrealize(dc, loongarch_cpu_unrealizefn,
>> +                                  &lacc->parent_unrealize);
>> +
>>       resettable_class_set_parent_phases(rc, NULL, loongarch_cpu_reset_hold, NULL,
>>                                          &lacc->parent_phases);
>>   
>> @@ -730,6 +762,7 @@ static void loongarch_cpu_class_init(ObjectClass *c, void *data)
>>   #ifdef CONFIG_TCG
>>       cc->tcg_ops = &loongarch_tcg_ops;
>>   #endif
>> +    dc->user_creatable = true;
>>   }
>>   
>>   #define DEFINE_LOONGARCH_CPU_TYPE(model, initfn) \
>> diff --git a/target/loongarch/cpu.h b/target/loongarch/cpu.h
>> index f4439c245f..32feee4fe6 100644
>> --- a/target/loongarch/cpu.h
>> +++ b/target/loongarch/cpu.h
>> @@ -397,6 +397,7 @@ struct LoongArchCPUClass {
>>       /*< public >*/
>>   
>>       DeviceRealize parent_realize;
>> +    DeviceUnrealize parent_unrealize;
>>       ResettablePhases parent_phases;
>>   };
>>   

Re: [PATCH 7/8] Update the ACPI table for the Loongarch CPU

[lixianglai]

Hi Igor Mammedov:


On 7/28/23 9:26 PM, Igor Mammedov wrote:
> On Thu, 20 Jul 2023 15:15:12 +0800
> xianglai li <lixianglai@loongson.cn> wrote:
>
>> 1.Create a new GED device type for Loongarch,
>> mount cpu_madt function to update the ACPI table
> madt changes should be its own patch


Okay, I'll put the Madt-related changes into a separate patch.


Thanks,

xianglai

>> 2.Update the APIC table for loongarch based on
>> CPU information to support CPU hot-(un)plug
>>
>> Cc: Xiaojuan Yang <yangxiaojuan@loongson.cn>
>> Cc: Song Gao <gaosong@loongson.cn>
>> Cc: "Michael S. Tsirkin" <mst@redhat.com>
>> Cc: Igor Mammedov <imammedo@redhat.com>
>> Cc: Ani Sinha <anisinha@redhat.com>
>> Cc: Paolo Bonzini <pbonzini@redhat.com>
>> Cc: Richard Henderson <richard.henderson@linaro.org>
>> Cc: Eduardo Habkost <eduardo@habkost.net>
>> Cc: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
>> Cc: "Philippe Mathieu-Daudé" <philmd@linaro.org>
>> Cc: Yanan Wang <wangyanan55@huawei.com>
>> Cc: "Daniel P. Berrangé" <berrange@redhat.com>
>> Cc: Peter Xu <peterx@redhat.com>
>> Cc: David Hildenbrand <david@redhat.com>
>> Signed-off-by: xianglai li <lixianglai@loongson.cn>
>> ---
>>   hw/acpi/acpi-cpu-hotplug-stub.c               |  9 +++++
>>   hw/loongarch/acpi-build.c                     | 35 ++++++++++++++++--
>>   hw/loongarch/generic_event_device_loongarch.c | 36 +++++++++++++++++++
>>   hw/loongarch/meson.build                      |  2 +-
>>   4 files changed, 79 insertions(+), 3 deletions(-)
>>   create mode 100644 hw/loongarch/generic_event_device_loongarch.c
>>
>> diff --git a/hw/acpi/acpi-cpu-hotplug-stub.c b/hw/acpi/acpi-cpu-hotplug-stub.c
>> index 2aec90d968..af9fda2cf4 100644
>> --- a/hw/acpi/acpi-cpu-hotplug-stub.c
>> +++ b/hw/acpi/acpi-cpu-hotplug-stub.c
>> @@ -19,6 +19,15 @@ void legacy_acpi_cpu_hotplug_init(MemoryRegion *parent, Object *owner,
>>       return;
>>   }
>>   
>> +void build_cpus_aml(Aml *table, MachineState *machine, CPUHotplugFeatures opts,
>> +                    hwaddr mmap_io_base,
>> +                    const char *res_root,
>> +                    const char *event_handler_method,
>> +                    AmlRegionSpace rs)
>> +{
>> +    return;
>> +}
>> +
>>   void acpi_cpu_ospm_status(CPUHotplugState *cpu_st, ACPIOSTInfoList ***list)
>>   {
>>       return;
>> diff --git a/hw/loongarch/acpi-build.c b/hw/loongarch/acpi-build.c
>> index 0b62c3a2f7..312908fb2f 100644
>> --- a/hw/loongarch/acpi-build.c
>> +++ b/hw/loongarch/acpi-build.c
>> @@ -46,6 +46,23 @@
>>   #define ACPI_BUILD_DPRINTF(fmt, ...)
>>   #endif
>>   
>> +void virt_madt_cpu_entry(int uid,
>> +                         const CPUArchIdList *apic_ids,
>> +                         GArray *entry, bool force_enabled)
>> +{
>> +    uint32_t apic_id = apic_ids->cpus[uid].arch_id;
>> +    /* Flags – Local APIC Flags */
>> +    uint32_t flags = apic_ids->cpus[uid].cpu != NULL || force_enabled ?
>> +                     1 /* Enabled */ : 0;
>> +
>> +    /* Rev 1.0b, Table 5-13 Processor Local APIC Structure */
>> +    build_append_int_noprefix(entry, 0, 1);       /* Type */
>> +    build_append_int_noprefix(entry, 8, 1);       /* Length */
>> +    build_append_int_noprefix(entry, uid, 1);     /* ACPI Processor ID */
>> +    build_append_int_noprefix(entry, apic_id, 1); /* APIC ID */
>> +    build_append_int_noprefix(entry, flags, 4); /* Flags */
>> +}
>> +
>>   /* build FADT */
>>   static void init_common_fadt_data(AcpiFadtData *data)
>>   {
>> @@ -121,15 +138,18 @@ build_madt(GArray *table_data, BIOSLinker *linker, LoongArchMachineState *lams)
>>       build_append_int_noprefix(table_data, 1 /* PCAT_COMPAT */, 4); /* Flags */
>>   
>>       for (i = 0; i < arch_ids->len; i++) {
>> +        uint32_t flags;
>> +
>>           /* Processor Core Interrupt Controller Structure */
>>           arch_id = arch_ids->cpus[i].arch_id;
>> +        flags = arch_ids->cpus[i].cpu ? 1 : 0;
>>   
>>           build_append_int_noprefix(table_data, 17, 1);    /* Type */
>>           build_append_int_noprefix(table_data, 15, 1);    /* Length */
>>           build_append_int_noprefix(table_data, 1, 1);     /* Version */
>> -        build_append_int_noprefix(table_data, i + 1, 4); /* ACPI Processor ID */
>> +        build_append_int_noprefix(table_data, i, 4);     /* ACPI Processor ID */
>>           build_append_int_noprefix(table_data, arch_id, 4); /* Core ID */
>> -        build_append_int_noprefix(table_data, 1, 4);     /* Flags */
>> +        build_append_int_noprefix(table_data, flags, 4);   /* Flags */
>>       }
>>   
>>       /* Extend I/O Interrupt Controller Structure */
>> @@ -292,6 +312,17 @@ build_la_ged_aml(Aml *dsdt, MachineState *machine)
>>                                    AML_SYSTEM_MEMORY,
>>                                    VIRT_GED_MEM_ADDR);
>>       }
>> +
>> +    if (event & ACPI_GED_CPU_HOTPLUG_EVT) {
>> +        CPUHotplugFeatures opts = {
>> +            .acpi_1_compatible = false,
>> +            .has_legacy_cphp = false
>> +        };
>> +
>> +        build_cpus_aml(dsdt, machine, opts, VIRT_GED_CPUHP_ADDR,
>> +                       "\\_SB", "\\_GPE._E01", AML_SYSTEM_MEMORY);
>> +
>> +    }
>>       acpi_dsdt_add_power_button(dsdt);
>>   }
>>   
>> diff --git a/hw/loongarch/generic_event_device_loongarch.c b/hw/loongarch/generic_event_device_loongarch.c
>> new file mode 100644
>> index 0000000000..1fe550239b
>> --- /dev/null
>> +++ b/hw/loongarch/generic_event_device_loongarch.c
>> @@ -0,0 +1,36 @@
>> +/*
>> + * loongarch 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"
>> +#include "hw/loongarch/virt.h"
>> +
>> +static void acpi_ged_loongarch_class_init(ObjectClass *class, void *data)
>> +{
>> +    AcpiDeviceIfClass *adevc = ACPI_DEVICE_IF_CLASS(class);
>> +
>> +    adevc->madt_cpu = virt_madt_cpu_entry;
>> +}
>> +
>> +static const TypeInfo acpi_ged_loongarch_info = {
>> +    .name          = TYPE_ACPI_GED_LOONGARCH,
>> +    .parent        = TYPE_ACPI_GED,
>> +    .class_init    = acpi_ged_loongarch_class_init,
>> +    .interfaces = (InterfaceInfo[]) {
>> +        { TYPE_HOTPLUG_HANDLER },
>> +        { TYPE_ACPI_DEVICE_IF },
>> +        { }
>> +    }
>> +};
>> +
>> +static void acpi_ged_loongarch_register_types(void)
>> +{
>> +    type_register_static(&acpi_ged_loongarch_info);
>> +}
>> +
>> +type_init(acpi_ged_loongarch_register_types)
>> diff --git a/hw/loongarch/meson.build b/hw/loongarch/meson.build
>> index c0421502ab..8d21addee3 100644
>> --- a/hw/loongarch/meson.build
>> +++ b/hw/loongarch/meson.build
>> @@ -3,6 +3,6 @@ loongarch_ss.add(files(
>>       'fw_cfg.c',
>>   ))
>>   loongarch_ss.add(when: 'CONFIG_LOONGARCH_VIRT', if_true: [files('virt.c'), fdt])
>> -loongarch_ss.add(when: 'CONFIG_ACPI', if_true: files('acpi-build.c'))
>> +loongarch_ss.add(when: 'CONFIG_ACPI', if_true: files('acpi-build.c', 'generic_event_device_loongarch.c'))
>>   
>>   hw_arch += {'loongarch': loongarch_ss}

Re: [PATCH 8/8] Turn on CPU hot-(un)plug customization for loongarch

[lixianglai]

Hi, Igor Mammedov:


On 7/28/23 9:30 PM, Igor Mammedov wrote:
> On Thu, 20 Jul 2023 15:15:13 +0800
> xianglai li <lixianglai@loongson.cn> wrote:
>
>> Turn on CPU hot-(un)plug custom for loongarch in the configuration file
>>
>> Cc: Xiaojuan Yang <yangxiaojuan@loongson.cn>
>> Cc: Song Gao <gaosong@loongson.cn>
>> Cc: "Michael S. Tsirkin" <mst@redhat.com>
>> Cc: Igor Mammedov <imammedo@redhat.com>
>> Cc: Ani Sinha <anisinha@redhat.com>
>> Cc: Paolo Bonzini <pbonzini@redhat.com>
>> Cc: Richard Henderson <richard.henderson@linaro.org>
>> Cc: Eduardo Habkost <eduardo@habkost.net>
>> Cc: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
>> Cc: "Philippe Mathieu-Daudé" <philmd@linaro.org>
>> Cc: Yanan Wang <wangyanan55@huawei.com>
>> Cc: "Daniel P. Berrangé" <berrange@redhat.com>
>> Cc: Peter Xu <peterx@redhat.com>
>> Cc: David Hildenbrand <david@redhat.com>
>> Signed-off-by: xianglai li <lixianglai@loongson.cn>
>> ---
>>   configs/devices/loongarch64-softmmu/default.mak | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/configs/devices/loongarch64-softmmu/default.mak b/configs/devices/loongarch64-softmmu/default.mak
>> index 928bc117ef..e596706fab 100644
>> --- a/configs/devices/loongarch64-softmmu/default.mak
>> +++ b/configs/devices/loongarch64-softmmu/default.mak
>> @@ -1,3 +1,4 @@
>>   # Default configuration for loongarch64-softmmu
>>   
>>   CONFIG_LOONGARCH_VIRT=y
>> +CONFIG_ACPI_CPU_HOTPLUG=y
> this likely shall be part of prior patch (one that starts to use generic cpu hotplug functions)
> otherwise you risk a broke bisection in the middle of series
> (aka try to build series after applying each patch)


Do you mean this patch should be inside the first patch?

Thanks,

xianglai

>

Re: [PATCH 5/8] Adds basic CPU hot-(un)plug support for Loongarch

[lixianglai]

Hi Igor Mammedov:

On 7/28/23 9:21 PM, Igor Mammedov wrote:
> On Thu, 20 Jul 2023 15:15:10 +0800
> xianglai li <lixianglai@loongson.cn> wrote:
>
>> 1.Add CPU topology related functions
>> 2.Add CPU hot-plug related hook functions
>> 3.Update the in-place CPU creation process at machine initialization
> patch is to large, split it at least on those ^^ 3 parts,
> which would do a single distinct thing.
> After that it will be easier to review this.


Ok, I'll split this patch further.


>
> Also looking at hw/loongarch/acpi-build.c
> you have cpu_index == arch_id == core_id /according to comments/
> and they are mixed/used interchangeably. which is confusing
> at least. So clean it up first to use arch_id consistently
>
> then a separate patches to introduce socket/core/thread support
> with proper documentation/pointers to specs as to how arch_id
> should be calculated.
>
> And once that is ready, add hotplug on top of it.
>

Okay, I'll do it according to your suggestion.


>> Cc: Xiaojuan Yang <yangxiaojuan@loongson.cn>
>> Cc: Song Gao <gaosong@loongson.cn>
>> Cc: "Michael S. Tsirkin" <mst@redhat.com>
>> Cc: Igor Mammedov <imammedo@redhat.com>
>> Cc: Ani Sinha <anisinha@redhat.com>
>> Cc: Paolo Bonzini <pbonzini@redhat.com>
>> Cc: Richard Henderson <richard.henderson@linaro.org>
>> Cc: Eduardo Habkost <eduardo@habkost.net>
>> Cc: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
>> Cc: "Philippe Mathieu-Daudé" <philmd@linaro.org>
>> Cc: Yanan Wang <wangyanan55@huawei.com>
>> Cc: "Daniel P. Berrangé" <berrange@redhat.com>
>> Cc: Peter Xu <peterx@redhat.com>
>> Cc: David Hildenbrand <david@redhat.com>
>> Signed-off-by: xianglai li <lixianglai@loongson.cn>
>> ---
>>   hw/loongarch/virt.c         | 381 ++++++++++++++++++++++++++++++++++--
>>   include/hw/loongarch/virt.h |  11 +-
>>   target/loongarch/cpu.h      |   4 +
>>   3 files changed, 382 insertions(+), 14 deletions(-)
>>
>> diff --git a/hw/loongarch/virt.c b/hw/loongarch/virt.c
>> index e19b042ce8..5919389f42 100644
>> --- a/hw/loongarch/virt.c
>> +++ b/hw/loongarch/virt.c
>> @@ -46,6 +46,9 @@
>>   #include "hw/block/flash.h"
>>   #include "qemu/error-report.h"
>>   
>> +static int virt_get_socket_id(const MachineState *ms, int cpu_index);
>> +static int virt_get_core_id(const MachineState *ms, int cpu_index);
>> +static int virt_get_thread_id(const MachineState *ms, int cpu_index);
>>   
>>   static void virt_flash_create(LoongArchMachineState *lams)
>>   {
>> @@ -447,12 +450,12 @@ static DeviceState *create_acpi_ged(DeviceState *pch_pic, LoongArchMachineState
>>   {
>>       DeviceState *dev;
>>       MachineState *ms = MACHINE(lams);
>> -    uint32_t event = ACPI_GED_PWR_DOWN_EVT;
>> +    uint32_t event = ACPI_GED_PWR_DOWN_EVT | ACPI_GED_CPU_HOTPLUG_EVT;
>>   
>>       if (ms->ram_slots) {
>>           event |= ACPI_GED_MEM_HOTPLUG_EVT;
>>       }
>> -    dev = qdev_new(TYPE_ACPI_GED);
>> +    dev = qdev_new(TYPE_ACPI_GED_LOONGARCH);
>>       qdev_prop_set_uint32(dev, "ged-event", event);
>>   
>>       /* ged event */
>> @@ -461,6 +464,7 @@ static DeviceState *create_acpi_ged(DeviceState *pch_pic, LoongArchMachineState
>>       sysbus_mmio_map(SYS_BUS_DEVICE(dev), 1, VIRT_GED_MEM_ADDR);
>>       /* ged regs used for reset and power down */
>>       sysbus_mmio_map(SYS_BUS_DEVICE(dev), 2, VIRT_GED_REG_ADDR);
>> +    sysbus_mmio_map(SYS_BUS_DEVICE(dev), 3, VIRT_GED_CPUHP_ADDR);
>>   
>>       sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0,
>>                          qdev_get_gpio_in(pch_pic, VIRT_SCI_IRQ - VIRT_GSI_BASE));
>> @@ -583,6 +587,7 @@ static void loongarch_irq_init(LoongArchMachineState *lams)
>>   
>>       extioi = qdev_new(TYPE_LOONGARCH_EXTIOI);
>>       sysbus_realize_and_unref(SYS_BUS_DEVICE(extioi), &error_fatal);
>> +    lams->extioi = extioi;
>>   
>>       /*
>>        * The connection of interrupts:
>> @@ -624,11 +629,11 @@ static void loongarch_irq_init(LoongArchMachineState *lams)
>>                                       sysbus_mmio_get_region(SYS_BUS_DEVICE(ipi),
>>                                       1));
>>           /*
>> -	 * extioi iocsr memory region
>> -	 * only one extioi is added on loongarch virt machine
>> -	 * external device interrupt can only be routed to cpu 0-3
>> -	 */
>> -	if (cpu < EXTIOI_CPUS)
>> +         * extioi iocsr memory region
>> +         * only one extioi is added on loongarch virt machine
>> +         * external device interrupt can only be routed to cpu 0-3
>> +         */
>> +        if (cpu < EXTIOI_CPUS)
>>               memory_region_add_subregion(&env->system_iocsr, APIC_BASE,
>>                                   sysbus_mmio_get_region(SYS_BUS_DEVICE(extioi),
>>                                   cpu));
>> @@ -789,7 +794,6 @@ static void loongarch_init(MachineState *machine)
>>       NodeInfo *numa_info = machine->numa_state->nodes;
>>       int i;
>>       hwaddr fdt_base;
>> -    const CPUArchIdList *possible_cpus;
>>       MachineClass *mc = MACHINE_GET_CLASS(machine);
>>       CPUState *cpu;
>>       char *ramName = NULL;
>> @@ -810,12 +814,40 @@ static void loongarch_init(MachineState *machine)
>>       create_fdt(lams);
>>       /* Init CPUs */
>>   
>> -    possible_cpus = mc->possible_cpu_arch_ids(machine);
>> -    for (i = 0; i < possible_cpus->len; i++) {
>> -        cpu = cpu_create(machine->cpu_type);
>> +    mc->possible_cpu_arch_ids(machine);
>> +
>> +    for (i = 0; i < machine->smp.cpus; i++) {
>> +        Object *cpuobj;
>> +        cpuobj = object_new(machine->cpu_type);
>> +
>> +        cpu = CPU(cpuobj);
>>           cpu->cpu_index = i;
> I'd move this to foo_cpu_pre_plug()

I guess I don't need to assign a value to the cpu->cpu_index,

it should be automatically assigned to the cpu_exec_realizefn->cpu_list_add.

I'll remove it in the next release.


>
>> -        machine->possible_cpus->cpus[i].cpu = OBJECT(cpu);
> and this to foo_cpu_plug()	


Yes, it's already inside, this one is to remove the line of code.


>> +
>> +        object_property_set_int(cpuobj, "socket-id",
>> +                                virt_get_socket_id(machine, i), NULL);
>> +        object_property_set_int(cpuobj, "core-id",
>> +                                virt_get_core_id(machine, i), NULL);
>> +        object_property_set_int(cpuobj, "thread-id",
>> +                                virt_get_thread_id(machine, i), NULL);
> you don't need to calculate foo_ids here, they shall be calculated once at
> the 1st time possible_cpu_arch_ids() are called and then reuse
> CPUArchId.props here.
> see x86_possible_cpu_arch_ids() for an example
>

Yes, I'm not calculating the foo_ids here, I'm just taking the value 
from the possible_cpu_arch_ids

and reusing it to assign the property of the cpuobj.


>> +        /*
>> +         * The CPU in place at the time of machine startup will also enter
>> +         * the CPU hot-plug process when it is created, but at this time,
>> +         * the GED device has not been created, resulting in exit in the CPU
>> +         * hot-plug process, which can avoid the incumbent CPU repeatedly
>> +         * applying for resources.
>> +         *
>> +         * The interrupt resource of the in-place CPU will be requested at
>> +         * the current function call loongarch_irq_init().
>> +         *
>> +         * The interrupt resource of the subsequently inserted CPU will be
>> +         * requested in the CPU hot-plug process.
>> +         */
>> +        qdev_realize(DEVICE(cpuobj), NULL, &error_fatal);
>> +        object_unref(cpuobj);
>>       }
>> +
>> +    lams->boot_cpus = machine->smp.cpus;
>> +
>>       fdt_add_cpu_nodes(lams);
>>   
>>       /* Node0 memory */
>> @@ -986,11 +1018,107 @@ static void virt_mem_pre_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
>>       pc_dimm_pre_plug(PC_DIMM(dev), MACHINE(hotplug_dev), NULL, errp);
>>   }
>>   
>> +static int virt_get_cpu_id_from_cpu_topo(const MachineState *ms,
>> +                                            DeviceState *dev)
> name suggest it's topo->id helper,
> then feed it topo arguments instead of dev.


Ok!


>> +{
>> +    int cpu_index, sock_vcpu_num, core_vcpu_num;
>> +    LoongArchCPU *cpu = LOONGARCH_CPU(dev);
>> +
>> +    /* calculate total logical cpus across socket/cluster/core */
>> +    sock_vcpu_num = cpu->socket_id * (ms->smp.threads * ms->smp.cores);
>> +    core_vcpu_num = cpu->core_id * ms->smp.threads;
>> +
>> +    /* get vcpu-id(logical cpu index) for this vcpu from this topology */
>> +    cpu_index = (sock_vcpu_num + core_vcpu_num) + cpu->thread_id;
>> +
>> +    assert(cpu_index >= 0 && cpu_index < ms->possible_cpus->len);
>> +
>> +    return cpu_index;
>> +}
>> +
>> +/* find cpu slot in machine->possible_cpus by core_id */
>> +static CPUArchId *loongarch_find_cpu_slot(MachineState *ms, uint32_t cpu_index,
>> +                                        int *idx)
>> +{
>> +    int index = cpu_index;
>> +
>> +    if (index >= ms->possible_cpus->len) {
>> +        return NULL;
>> +    }
>> +    if (idx) {
>> +        *idx = index;
>> +    }
>> +    return &ms->possible_cpus->cpus[index];
>> +}
>> +
>> +static void loongarch_cpu_pre_plug(HotplugHandler *hotplug_dev,
>> +                            DeviceState *dev, Error **errp)
>> +{
>> +    MachineState *ms = MACHINE(OBJECT(hotplug_dev));
>> +    MachineClass *mc = MACHINE_GET_CLASS(hotplug_dev);
>> +    LoongArchCPU *cpu = LOONGARCH_CPU(dev);
>> +    CPUState *cs = CPU(dev);
>> +    CPUArchId *cpu_slot;
>> +    Error *local_err = NULL;
>> +
>> +    if (dev->hotplugged && !mc->has_hotpluggable_cpus) {
>> +        error_setg(&local_err, "CPU hotplug not supported for this machine");
>> +        goto out;
>> +    }
>> +
>> +    /* sanity check the cpu */
>> +    if (!object_dynamic_cast(OBJECT(cpu), ms->cpu_type)) {
>> +        error_setg(&local_err, "Invalid CPU type, expected cpu type: '%s'",
>> +                   ms->cpu_type);
>> +        goto out;
>> +    }
>> +
>> +    if ((cpu->thread_id < 0) || (cpu->thread_id >= ms->smp.threads)) {
>> +        error_setg(&local_err,
>> +                   "Invalid thread-id %u specified, must be in range 1:%u",
>> +                   cpu->thread_id, ms->smp.threads - 1);
>> +        goto out;
>> +    }
>> +
>> +    if ((cpu->core_id < 0) || (cpu->core_id >= ms->smp.cores)) {
>> +        error_setg(&local_err,
>> +                   "Invalid core-id %u specified, must be in range 1:%u",
>> +                   cpu->core_id, ms->smp.cores);
>> +        goto out;
>> +    }
>> +
>> +    if ((cpu->socket_id < 0) || (cpu->socket_id >= ms->smp.sockets)) {
>> +        error_setg(&local_err,
>> +                   "Invalid socket-id %u specified, must be in range 1:%u",
>> +                   cpu->socket_id, ms->smp.sockets - 1);
>> +        goto out;
>> +    }
>> +    cs->cpu_index = virt_get_cpu_id_from_cpu_topo(ms, dev);
>> +
>> +    cpu_slot = loongarch_find_cpu_slot(ms, cs->cpu_index, NULL);
>> +    if (CPU(cpu_slot->cpu)) {
>> +        error_setg(&local_err,
>> +                   "cpu(id%d=%d:%d:%d) with arch-id %" PRIu64 " exists",
>> +                   cs->cpu_index, cpu->socket_id, cpu->core_id,
>> +                   cpu->thread_id, cpu_slot->arch_id);
>> +        goto out;
>> +    }
>> +
>> +    numa_cpu_pre_plug(cpu_slot, dev, &local_err);
>> +
>> +    return ;
>> +out:
>> +    error_propagate(errp, local_err);
>> +}
>> +
>>   static void virt_machine_device_pre_plug(HotplugHandler *hotplug_dev,
>>                                               DeviceState *dev, Error **errp)
>>   {
>>       if (memhp_type_supported(dev)) {
>>           virt_mem_pre_plug(hotplug_dev, dev, errp);
>> +    } else if (object_dynamic_cast(OBJECT(dev), TYPE_LOONGARCH_CPU)) {
>> +        loongarch_cpu_pre_plug(hotplug_dev, dev, errp);
>>       }
>>   }
>>   
>> @@ -1004,11 +1132,45 @@ static void virt_mem_unplug_request(HotplugHandler *hotplug_dev,
>>                                      errp);
>>   }
>>   
>> +static void loongarch_cpu_unplug_request(HotplugHandler *hotplug_dev,
>> +                                        DeviceState *dev, Error **errp)
>> +{
>> +    MachineState *machine = MACHINE(OBJECT(hotplug_dev));
>> +    LoongArchMachineState *lsms = LOONGARCH_MACHINE(machine);
>> +    Error *local_err = NULL;
>> +    HotplugHandlerClass *hhc;
>> +    LoongArchCPU *cpu = LOONGARCH_CPU(dev);
>> +    CPUState *cs = CPU(dev);
>> +
>> +    if (!lsms->acpi_ged) {
>> +        error_setg(&local_err, "CPU hot unplug not supported without ACPI");
>> +        goto out;
>> +    }
>> +
>> +    if (cs->cpu_index == 0) {
>> +        error_setg(&local_err,
>> +                   "hot-unplug of boot cpu(id%d=%d:%d:%d) not supported",
>> +                   cs->cpu_index, cpu->socket_id,
>> +                   cpu->core_id, cpu->thread_id);
>> +        goto out;
>> +    }
>> +
>> +
>> +    hhc = HOTPLUG_HANDLER_GET_CLASS(lsms->acpi_ged);
>> +    hhc->unplug_request(HOTPLUG_HANDLER(lsms->acpi_ged), dev, &local_err);
>> +
>> +    return;
>> + out:
>> +    error_propagate(errp, local_err);
>> +}
>> +
>>   static void virt_machine_device_unplug_request(HotplugHandler *hotplug_dev,
>>                                             DeviceState *dev, Error **errp)
>>   {
>>       if (memhp_type_supported(dev)) {
>>           virt_mem_unplug_request(hotplug_dev, dev, errp);
>> +    } else if (object_dynamic_cast(OBJECT(dev), TYPE_LOONGARCH_CPU)) {
>> +        loongarch_cpu_unplug_request(hotplug_dev, dev, errp);
>>       }
>>   }
>>   
>> @@ -1022,11 +1184,93 @@ static void virt_mem_unplug(HotplugHandler *hotplug_dev,
>>       qdev_unrealize(dev);
>>   }
>>   
>> +static void loongarch_cpu_destroy(MachineState *machine, LoongArchCPU *cpu)
> I'd fold this into unplug handler, and the same for _cpu_create
> you aren't destroying/creating CPU at this point but rather
> wiring it up with other external to it components.


What do you think about giving it a different name,

putting it inside the unplug handler will cause the unplug handler 
function to be too long?


>> +{
>> +    LoongArchMachineState *lsms = LOONGARCH_MACHINE(machine);
>> +    CPULoongArchState *env = &cpu->env;
>> +    DeviceState *ipi = env->ipistate;
>> +    CPUState *cs = CPU(cpu);
>> +    unsigned int cpu_index = cs->cpu_index;
>> +    DeviceState *extioi = lsms->extioi;
>> +    int pin;
>> +
>> +    qemu_unregister_reset(reset_load_elf, DEVICE(cpu));
>> +
>> +    lsms->boot_cpus--;
>> +    if (lsms->fw_cfg) {
>> +        fw_cfg_modify_i16(lsms->fw_cfg, FW_CFG_NB_CPUS,
>> +                          (uint16_t)lsms->boot_cpus);
>> +    }
> do you really need boot_cpus?
> adding FWCFG variable is adding ABI, with potential to maintain it for a long time
>
> it seems that before this series you also had support for multiple CPUs
> and didn't care about FWCFG, so quest is why it's being added now.


I looked up this fw_cfg about the number of CPUs we really don't need 
it, I'll delete it.

>> +
>> +    /* disconnect ipi irq to cpu irq */
>> +    qdev_disconnect_gpio_out_named(ipi, NULL, 0);
>> +    /* del IPI iocsr memory region */
>> +    memory_region_del_subregion(&env->system_iocsr,
>> +                                sysbus_mmio_get_region(SYS_BUS_DEVICE(ipi),
>> +                                0));
>> +    memory_region_del_subregion(&env->system_iocsr,
>> +                                sysbus_mmio_get_region(SYS_BUS_DEVICE(ipi),
>> +                                1));
>> +
>> +    env->ipistate = NULL;
>> +    object_unparent(OBJECT(ipi));
>> +
>> +    /*
>> +     * disconnect ext irq to the cpu irq
>> +     * cpu_pin[9:2] <= intc_pin[7:0]
>> +     */
>> +    if (cpu_index < EXTIOI_CPUS) {
>> +        for (pin = 0; pin < LS3A_INTC_IP; pin++) {
>> +            qdev_disconnect_gpio_out_named(extioi, NULL, (cpu_index * 8 + pin));
>> +        }
>> +    }
>> +
>> +    /*
>> +     * del extioi iocsr memory region
>> +     * only one extioi is added on loongarch virt machine
>> +     * external device interrupt can only be routed to cpu 0-3
>> +     */
>> +    if (cpu_index < EXTIOI_CPUS)
>> +        memory_region_del_subregion(&env->system_iocsr,
>> +                            sysbus_mmio_get_region(SYS_BUS_DEVICE(extioi),
>> +                            cpu_index));
>> +}
>> +
>> +static void loongarch_cpu_unplug(HotplugHandler *hotplug_dev,
>> +                                DeviceState *dev, Error **errp)
>> +{
>> +    CPUArchId *found_cpu;
>> +    HotplugHandlerClass *hhc;
>> +    Error *local_err = NULL;
>> +    LoongArchCPU *cpu = LOONGARCH_CPU(dev);
>> +    MachineState *machine = MACHINE(OBJECT(hotplug_dev));
>> +    LoongArchMachineState *lsms = LOONGARCH_MACHINE(machine);
>> +    CPUState *cs = CPU(dev);
>> +
>> +    hhc = HOTPLUG_HANDLER_GET_CLASS(lsms->acpi_ged);
>> +    hhc->unplug(HOTPLUG_HANDLER(lsms->acpi_ged), dev, &local_err);
>> +
>> +    if (local_err) {
>> +        goto out;
>> +    }
>> +
>> +    loongarch_cpu_destroy(machine, cpu);
>> +
>> +    found_cpu = loongarch_find_cpu_slot(MACHINE(lsms), cs->cpu_index, NULL);
>> +    found_cpu->cpu = NULL;
>> +
>> +    return;
>> +out:
>> +    error_propagate(errp, local_err);
>> +}
>> +
>>   static void virt_machine_device_unplug(HotplugHandler *hotplug_dev,
>>                                             DeviceState *dev, Error **errp)
>>   {
>>       if (memhp_type_supported(dev)) {
>>           virt_mem_unplug(hotplug_dev, dev, errp);
>> +    } else if (object_dynamic_cast(OBJECT(dev), TYPE_LOONGARCH_CPU)) {
>> +        loongarch_cpu_unplug(hotplug_dev, dev, errp);
>>       }
>>   }
>>   
>> @@ -1040,6 +1284,92 @@ static void virt_mem_plug(HotplugHandler *hotplug_dev,
>>                            dev, &error_abort);
>>   }
>>   
>> +
>> +static LoongArchCPU *loongarch_cpu_create(MachineState *machine,
>> +                                LoongArchCPU *cpu, Error **errp)
>> +{
>> +    LoongArchMachineState *lsms = LOONGARCH_MACHINE(machine);
>> +    CPUState *cs = CPU(cpu);
>> +    unsigned int cpu_index = cs->cpu_index;
>> +    DeviceState *cpudev = DEVICE(cpu);
>> +    DeviceState *extioi = lsms->extioi;
>> +    CPULoongArchState *env = &cpu->env;
>> +    DeviceState *ipi;
>> +    int pin;
>> +
>> +    qemu_register_reset(reset_load_elf, cpu);
>> +
>> +    lsms->boot_cpus++;
>> +    if (lsms->fw_cfg) {
>> +        fw_cfg_modify_i16(lsms->fw_cfg, FW_CFG_NB_CPUS,
>> +                          (uint16_t)lsms->boot_cpus);
>> +    }
>> +
>> +    ipi = qdev_new(TYPE_LOONGARCH_IPI);
>> +    sysbus_realize_and_unref(SYS_BUS_DEVICE(ipi), errp);
>> +
>> +    /* connect ipi irq to cpu irq */
>> +    qdev_connect_gpio_out(ipi, 0, qdev_get_gpio_in(cpudev, IRQ_IPI));
>> +    /* IPI iocsr memory region */
>> +    memory_region_add_subregion(&env->system_iocsr, SMP_IPI_MAILBOX,
>> +                                sysbus_mmio_get_region(SYS_BUS_DEVICE(ipi),
>> +                                0));
>> +    memory_region_add_subregion(&env->system_iocsr, MAIL_SEND_ADDR,
>> +                                sysbus_mmio_get_region(SYS_BUS_DEVICE(ipi),
>> +                                1));
>> +    /*
>> +     * extioi iocsr memory region
>> +     * only one extioi is added on loongarch virt machine
>> +     * external device interrupt can only be routed to cpu 0-3
>> +     */
>> +    if (cpu_index < EXTIOI_CPUS)
>> +        memory_region_add_subregion(&env->system_iocsr, APIC_BASE,
>> +                            sysbus_mmio_get_region(SYS_BUS_DEVICE(extioi),
>> +                            cpu_index));
>> +    env->ipistate = ipi;
>> +
>> +    /*
>> +     * connect ext irq to the cpu irq
>> +     * cpu_pin[9:2] <= intc_pin[7:0]
>> +     */
>> +    if (cpu_index < EXTIOI_CPUS) {
>> +        for (pin = 0; pin < LS3A_INTC_IP; pin++) {
>> +            qdev_connect_gpio_out(extioi, (cpu_index * 8 + pin),
>> +                                  qdev_get_gpio_in(cpudev, pin + 2));
>> +        }
>> +    }
>> +
>> +    return cpu;
>> +}
>> +
>> +static void loongarch_cpu_plug(HotplugHandler *hotplug_dev,
>> +                                DeviceState *dev, Error **errp)
>> +{
>> +    CPUArchId *found_cpu;
>> +    HotplugHandlerClass *hhc;
>> +    Error *local_err = NULL;
>> +    MachineState *machine = MACHINE(OBJECT(hotplug_dev));
>> +    LoongArchMachineState *lsms = LOONGARCH_MACHINE(machine);
>> +    LoongArchCPU *cpu = LOONGARCH_CPU(dev);
>> +    CPUState *cs = CPU(dev);
>> +
>> +    if (lsms->acpi_ged) {
> dont' you need CPUs if you don't have ged?
> /is it possible that ged doesn't exists?/

For those CPUs that are already in place at startup, their 
initialization predates the initialization of the GED,

and the initialization of these CPUs is done in the loongarch_irq_init 
function,

but I feel that this is somewhat redundant and I will integrate them in 
the CPU plug handler in the next version of the patch.



>
>> +        loongarch_cpu_create(machine, cpu, errp);
>> +        hhc = HOTPLUG_HANDLER_GET_CLASS(lsms->acpi_ged);
>> +        hhc->plug(HOTPLUG_HANDLER(lsms->acpi_ged), dev, &local_err);
>> +        if (local_err) {
>> +            goto out;
>> +        }
>> +    }
>> +
>> +    found_cpu = loongarch_find_cpu_slot(MACHINE(lsms), cs->cpu_index, NULL);
>> +    found_cpu->cpu = OBJECT(dev);
>> +
>> +    return;
>> +out:
>> +    error_propagate(errp, local_err);
>> +}
>> +
>>   static void loongarch_machine_device_plug_cb(HotplugHandler *hotplug_dev,
>>                                           DeviceState *dev, Error **errp)
>>   {
>> @@ -1053,6 +1383,8 @@ static void loongarch_machine_device_plug_cb(HotplugHandler *hotplug_dev,
>>           }
>>       } else if (memhp_type_supported(dev)) {
>>           virt_mem_plug(hotplug_dev, dev, errp);
>> +    } else if (object_dynamic_cast(OBJECT(dev), TYPE_LOONGARCH_CPU)) {
>> +        loongarch_cpu_plug(hotplug_dev, dev, errp);
>>       }
>>   }
>>   
>> @@ -1062,16 +1394,39 @@ static HotplugHandler *virt_machine_get_hotplug_handler(MachineState *machine,
>>       MachineClass *mc = MACHINE_GET_CLASS(machine);
>>   
>>       if (device_is_dynamic_sysbus(mc, dev) ||
>> +        object_dynamic_cast(OBJECT(dev), TYPE_LOONGARCH_CPU) ||
>>           memhp_type_supported(dev)) {
>>           return HOTPLUG_HANDLER(machine);
>>       }
>>       return NULL;
>>   }
>>   
>> +static int virt_get_socket_id(const MachineState *ms, int cpu_index)
>> +{
>> +    assert(cpu_index >= 0 && cpu_index < ms->possible_cpus->len);
>> +
>> +    return ms->possible_cpus->cpus[cpu_index].props.socket_id;
>> +}
>> +
>> +static int virt_get_core_id(const MachineState *ms, int cpu_index)
>> +{
>> +    assert(cpu_index >= 0 && cpu_index < ms->possible_cpus->len);
>> +
>> +    return ms->possible_cpus->cpus[cpu_index].props.core_id;
>> +}
>> +
>> +static int virt_get_thread_id(const MachineState *ms, int cpu_index)
>> +{
>> +    assert(cpu_index >= 0 && cpu_index < ms->possible_cpus->len);
>> +
>> +    return ms->possible_cpus->cpus[cpu_index].props.thread_id;
>> +}
>> +
>>   static const CPUArchIdList *virt_possible_cpu_arch_ids(MachineState *ms)
>>   {
>>       int n;
>>       unsigned int max_cpus = ms->smp.max_cpus;
>> +    unsigned int smp_threads = ms->smp.threads;
>>   
>>       if (ms->possible_cpus) {
>>           assert(ms->possible_cpus->len == max_cpus);
>> @@ -1082,6 +1437,7 @@ static const CPUArchIdList *virt_possible_cpu_arch_ids(MachineState *ms)
>>                                     sizeof(CPUArchId) * max_cpus);
>>       ms->possible_cpus->len = max_cpus;
>>       for (n = 0; n < ms->possible_cpus->len; n++) {
>> +        ms->possible_cpus->cpus[n].vcpus_count = smp_threads;
>>           ms->possible_cpus->cpus[n].type = ms->cpu_type;
>>           ms->possible_cpus->cpus[n].arch_id = n;
>>   
>> @@ -1125,6 +1481,7 @@ static void loongarch_class_init(ObjectClass *oc, void *data)
>>       MachineClass *mc = MACHINE_CLASS(oc);
>>       HotplugHandlerClass *hc = HOTPLUG_HANDLER_CLASS(oc);
>>   
>> +    mc->has_hotpluggable_cpus = true;
>>       mc->desc = "Loongson-3A5000 LS7A1000 machine";
>>       mc->init = loongarch_init;
>>       mc->default_ram_size = 1 * GiB;
>> diff --git a/include/hw/loongarch/virt.h b/include/hw/loongarch/virt.h
>> index f1659655c6..9ebdba676e 100644
>> --- a/include/hw/loongarch/virt.h
>> +++ b/include/hw/loongarch/virt.h
>> @@ -31,6 +31,7 @@
>>   #define VIRT_GED_EVT_ADDR       0x100e0000
>>   #define VIRT_GED_MEM_ADDR       (VIRT_GED_EVT_ADDR + ACPI_GED_EVT_SEL_LEN)
>>   #define VIRT_GED_REG_ADDR       (VIRT_GED_MEM_ADDR + MEMORY_HOTPLUG_IO_LEN)
>> +#define VIRT_GED_CPUHP_ADDR     (VIRT_GED_REG_ADDR + ACPI_CPU_HOTPLUG_REG_LEN)
>>   
>>   struct LoongArchMachineState {
>>       /*< private >*/
>> @@ -42,7 +43,7 @@ struct LoongArchMachineState {
>>       MemoryRegion bios;
>>       bool         bios_loaded;
>>       /* State for other subsystems/APIs: */
>> -    FWCfgState  *fw_cfg;
>> +    FWCfgState   *fw_cfg;
>>       Notifier     machine_done;
>>       Notifier     powerdown_notifier;
>>       OnOffAuto    acpi;
>> @@ -50,13 +51,19 @@ struct LoongArchMachineState {
>>       char         *oem_table_id;
>>       DeviceState  *acpi_ged;
>>       int          fdt_size;
>> -    DeviceState *platform_bus_dev;
>> +    DeviceState  *platform_bus_dev;
>>       PCIBus       *pci_bus;
>>       PFlashCFI01  *flash;
>> +    DeviceState  *extioi;
>> +    unsigned int boot_cpus;
>>   };
>>   
>>   #define TYPE_LOONGARCH_MACHINE  MACHINE_TYPE_NAME("virt")
>>   OBJECT_DECLARE_SIMPLE_TYPE(LoongArchMachineState, LOONGARCH_MACHINE)
>>   bool loongarch_is_acpi_enabled(LoongArchMachineState *lams);
>>   void loongarch_acpi_setup(LoongArchMachineState *lams);
>> +void virt_madt_cpu_entry(int uid,
>> +                         const CPUArchIdList *apic_ids, GArray *entry,
>> +                         bool force_enabled);
>> +
>>   #endif
>> diff --git a/target/loongarch/cpu.h b/target/loongarch/cpu.h
>> index ed04027af1..f4439c245f 100644
>> --- a/target/loongarch/cpu.h
>> +++ b/target/loongarch/cpu.h
>> @@ -367,6 +367,10 @@ struct ArchCPU {
>>       CPUState parent_obj;
>>       /*< public >*/
>>   
>> +    int32_t socket_id;  /* socket-id of this VCPU */
>> +    int32_t core_id;    /* core-id of this VCPU */
>> +    int32_t thread_id;  /* thread-id of this VCPU */
>> +    int32_t node_id;    /* NUMA node this CPU belongs to */
>>       CPUNegativeOffsetState neg;
>>       CPULoongArchState env;
>>       QEMUTimer timer;