[PATCH v3 0/5] hw/loongarch/virt: Add cpu hotplug support

[PATCH v3 0/5] hw/loongarch/virt: Add cpu hotplug support

[Bibo Mao]

LoongArch cpu hotplug is based on ACPI GED device, there is a little
change about ipi and extioi device, the value of num-cpu property is
maximum cpu number rather than present cpu number.

It can be verified with qemu command:
  qemu-system-loongarch64 -smp 2,maxcpus=16,sockets=4,cores=4,threads=1
and vcpu can be added or remove with hmp command:
  device_add la464-loongarch-cpu,socket-id=0,core-id=2,thread-id=0,id=cpu-2
  device_del cpu-2

---
v2 ... v3:
  1. Use qdev_realize_and_unref() with qdev_realize() and object_unref().
  2. Set vcpus_count with 1 since vcpu object is created for every thread.
  3. Remove property hw-id, use internal variable hw_id to differentiate
     cold-plug cpus and hot-plug cpus.
  4. Add generic function virt_init_cpu_irq() to init interrupt pin
     of CPU object, used by both cold-plug and hot-plug CPUs

v1 ... v2:
  1. Add new property hw-id, property hw-id is set for cold-added CPUs,
     and property socket-id/core-id/thread-id is set for hot-added CPUs.
     The two properties can be generated from each other.
  2. Use general hotplug api such as hotplug_handler_pre_plug etc
  3. Reorganize the patch order, split the patch set into 4 small
     patches.
---
Bibo Mao (5):
  hw/loongarch/virt: Add CPU topology support
  hw/loongarch/virt: Implement cpu plug interface
  hw/loongarch/virt: Add generic function to init interrupt pin of CPU
  hw/loongarch/virt: Update the ACPI table for hotplug cpu
  hw/loongarch/virt: Enable cpu hotplug feature on virt machine

 docs/system/loongarch/virt.rst |  31 +++
 hw/loongarch/Kconfig           |   1 +
 hw/loongarch/acpi-build.c      |  35 +++-
 hw/loongarch/virt.c            | 349 +++++++++++++++++++++++++++++----
 include/hw/loongarch/virt.h    |   3 +
 target/loongarch/cpu.c         |  25 +++
 target/loongarch/cpu.h         |  17 ++
 7 files changed, 416 insertions(+), 45 deletions(-)


base-commit: 92ec7805190313c9e628f8fc4eb4f932c15247bd
-- 
2.39.3

[PATCH v3 1/5] hw/loongarch/virt: Add CPU topology support

[Bibo Mao]

Add topological relationship for Loongarch VCPU and initialize
topology member variables, the topo information includes socket-id,
core-id and thread-id. For cold-plug CPUs, its topo information
is calculated from physical cpuid, and for hot-cpu CPUs its physical
cpuid is calculated from topo information.

Co-developed-by: Xianglai Li <lixianglai@loongson.cn>
Signed-off-by: Bibo Mao <maobibo@loongson.cn>
---
 docs/system/loongarch/virt.rst | 31 ++++++++++++++++
 hw/loongarch/virt.c            | 67 ++++++++++++++++++++++++++++------
 target/loongarch/cpu.c         | 12 ++++++
 target/loongarch/cpu.h         | 17 +++++++++
 4 files changed, 115 insertions(+), 12 deletions(-)

diff --git a/docs/system/loongarch/virt.rst b/docs/system/loongarch/virt.rst
index 172fba079e..8daf60785f 100644
--- a/docs/system/loongarch/virt.rst
+++ b/docs/system/loongarch/virt.rst
@@ -28,6 +28,37 @@ The ``qemu-system-loongarch64`` provides emulation for virt
 machine. You can specify the machine type ``virt`` and
 cpu type ``la464``.
 
+CPU Topology
+------------
+
+The ``LA464`` type CPUs have the concept of Socket Core and Thread.
+
+For example:
+
+``-smp 1,maxcpus=M,sockets=S,cores=C,threads=T``
+
+The above parameters indicate that the machine has a maximum of ``M`` vCPUs and
+``S`` sockets, each socket has ``C`` cores, each core has ``T`` threads,
+and each thread corresponds to a vCPU.
+
+Then ``M`` ``S`` ``C`` ``T`` has the following relationship:
+
+``M = S * C * T``
+
+In the CPU topology relationship, When we know the ``socket_id`` ``core_id``
+and ``thread_id`` of the CPU, we can calculate its ``arch_id``:
+
+``arch_id = (socket_id * S) + (core_id * C) + (thread_id * T)``
+
+Similarly, when we know the ``arch_id`` of the CPU,
+we can also get its ``socket_id`` ``core_id`` and ``thread_id``:
+
+``socket_id = arch_id / (C * T)``
+
+``core_id = (arch_id / T) % C``
+
+``thread_id = arch_id % T``
+
 Boot options
 ------------
 
diff --git a/hw/loongarch/virt.c b/hw/loongarch/virt.c
index 9a635d1d3d..2e4a71bc90 100644
--- a/hw/loongarch/virt.c
+++ b/hw/loongarch/virt.c
@@ -1143,7 +1143,6 @@ static void virt_init(MachineState *machine)
     LoongArchVirtMachineState *lvms = LOONGARCH_VIRT_MACHINE(machine);
     int i;
     hwaddr base, size, ram_size = machine->ram_size;
-    const CPUArchIdList *possible_cpus;
     MachineClass *mc = MACHINE_GET_CLASS(machine);
     CPUState *cpu;
 
@@ -1163,13 +1162,27 @@ static void virt_init(MachineState *machine)
     memory_region_add_subregion(&lvms->system_iocsr, 0, &lvms->iocsr_mem);
 
     /* 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++) {
+        cpu = CPU(object_new(machine->cpu_type));
         cpu->cpu_index = i;
         machine->possible_cpus->cpus[i].cpu = cpu;
         lacpu = LOONGARCH_CPU(cpu);
         lacpu->phy_id = machine->possible_cpus->cpus[i].arch_id;
+        /*
+         * The cold-plug CPUs 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
+         * function virt_irq_init().
+         *
+         * The interrupt resource of the subsequently inserted CPU will be
+         * requested in the CPU hot-plug process.
+         */
+        qdev_realize_and_unref(DEVICE(cpu), NULL, &error_fatal);
     }
     fdt_add_cpu_nodes(lvms);
     fdt_add_memory_nodes(machine);
@@ -1286,6 +1299,35 @@ static void virt_initfn(Object *obj)
     virt_flash_create(lvms);
 }
 
+static int virt_get_arch_id_from_topo(MachineState *ms, LoongArchCPUTopo *topo)
+{
+    int arch_id, sock_vcpu_num, core_vcpu_num;
+
+    /*
+     * calculate total logical cpus across socket/core/thread.
+     * For more information on how to calculate the arch_id,
+     * you can refer to the CPU Topology chapter of the
+     * docs/system/loongarch/virt.rst document.
+     */
+    sock_vcpu_num = topo->socket_id * (ms->smp.threads * ms->smp.cores);
+    core_vcpu_num = topo->core_id * ms->smp.threads;
+
+    /* get vcpu-id(logical cpu index) for this vcpu from this topology */
+    arch_id = (sock_vcpu_num + core_vcpu_num) + topo->thread_id;
+
+    assert(arch_id >= 0 && arch_id < ms->possible_cpus->len);
+
+    return arch_id;
+}
+
+static void virt_get_topo_from_index(MachineState *ms,
+                                     LoongArchCPUTopo *topo, int index)
+{
+    topo->socket_id = index / (ms->smp.cores * ms->smp.threads);
+    topo->core_id = index / ms->smp.threads % ms->smp.cores;
+    topo->thread_id = index % ms->smp.threads;
+}
+
 static bool memhp_type_supported(DeviceState *dev)
 {
     /* we only support pc dimm now */
@@ -1385,8 +1427,9 @@ static HotplugHandler *virt_get_hotplug_handler(MachineState *machine,
 
 static const CPUArchIdList *virt_possible_cpu_arch_ids(MachineState *ms)
 {
-    int n;
+    int n, arch_id;
     unsigned int max_cpus = ms->smp.max_cpus;
+    LoongArchCPUTopo topo;
 
     if (ms->possible_cpus) {
         assert(ms->possible_cpus->len == max_cpus);
@@ -1397,17 +1440,17 @@ 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++) {
+        virt_get_topo_from_index(ms, &topo, n);
+        arch_id = virt_get_arch_id_from_topo(ms, &topo);
+        ms->possible_cpus->cpus[n].vcpus_count = 1;
         ms->possible_cpus->cpus[n].type = ms->cpu_type;
-        ms->possible_cpus->cpus[n].arch_id = n;
-
+        ms->possible_cpus->cpus[n].arch_id = arch_id;
         ms->possible_cpus->cpus[n].props.has_socket_id = true;
-        ms->possible_cpus->cpus[n].props.socket_id  =
-                                   n / (ms->smp.cores * ms->smp.threads);
+        ms->possible_cpus->cpus[n].props.socket_id = topo.socket_id;
         ms->possible_cpus->cpus[n].props.has_core_id = true;
-        ms->possible_cpus->cpus[n].props.core_id =
-                                   n / ms->smp.threads % ms->smp.cores;
+        ms->possible_cpus->cpus[n].props.core_id = topo.core_id;
         ms->possible_cpus->cpus[n].props.has_thread_id = true;
-        ms->possible_cpus->cpus[n].props.thread_id = n % ms->smp.threads;
+        ms->possible_cpus->cpus[n].props.thread_id = topo.thread_id;
     }
     return ms->possible_cpus;
 }
diff --git a/target/loongarch/cpu.c b/target/loongarch/cpu.c
index 7212fb5f8f..17b65d5946 100644
--- a/target/loongarch/cpu.c
+++ b/target/loongarch/cpu.c
@@ -16,6 +16,7 @@
 #include "kvm/kvm_loongarch.h"
 #include "exec/exec-all.h"
 #include "cpu.h"
+#include "hw/qdev-properties.h"
 #include "internals.h"
 #include "fpu/softfloat-helpers.h"
 #include "cpu-csr.h"
@@ -682,6 +683,7 @@ static void loongarch_cpu_init(Object *obj)
     timer_init_ns(&cpu->timer, QEMU_CLOCK_VIRTUAL,
                   &loongarch_constant_timer_cb, cpu);
 #endif
+    cpu->phy_id = UNSET_PHY_ID;
 #endif
 }
 
@@ -780,6 +782,14 @@ static int64_t loongarch_cpu_get_arch_id(CPUState *cs)
 }
 #endif
 
+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);
@@ -787,6 +797,7 @@ 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);
     resettable_class_set_parent_phases(rc, NULL, loongarch_cpu_reset_hold, NULL,
@@ -811,6 +822,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;
 }
 
 static const gchar *loongarch32_gdb_arch_name(CPUState *cs)
diff --git a/target/loongarch/cpu.h b/target/loongarch/cpu.h
index 6c41fafb70..fb50cbbeee 100644
--- a/target/loongarch/cpu.h
+++ b/target/loongarch/cpu.h
@@ -19,6 +19,12 @@
 #include "cpu-csr.h"
 #include "cpu-qom.h"
 
+/*
+ * CPU can't have 0xFFFFFFFF physical ID, use that value to distinguish
+ * that physical ID hasn't been set yet
+ */
+#define UNSET_PHY_ID 0xFFFFFFFF
+
 #define IOCSRF_TEMP             0
 #define IOCSRF_NODECNT          1
 #define IOCSRF_MSI              2
@@ -369,6 +375,12 @@ typedef struct CPUArchState {
 #endif
 } CPULoongArchState;
 
+typedef struct LoongArchCPUTopo {
+    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 */
+} LoongArchCPUTopo;
+
 /**
  * LoongArchCPU:
  * @env: #CPULoongArchState
@@ -381,6 +393,10 @@ struct ArchCPU {
     CPULoongArchState env;
     QEMUTimer timer;
     uint32_t  phy_id;
+    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 of this VCPU */
 
     /* 'compatible' string for this CPU for Linux device trees */
     const char *dtb_compatible;
@@ -399,6 +415,7 @@ struct LoongArchCPUClass {
     CPUClass parent_class;
 
     DeviceRealize parent_realize;
+    DeviceUnrealize parent_unrealize;
     ResettablePhases parent_phases;
 };
 
-- 
2.39.3

[PATCH v3 2/5] hw/loongarch/virt: Implement cpu plug interface

[Bibo Mao]

Add cpu hotplug interface, however cpu hotplug feature is still
disabled for the machine. When machine is on, all created vCPUs
go through hotplug interface, and there is no remaining vCPU which
can be hot-added after power on.

Co-developed-by: Xianglai Li <lixianglai@loongson.cn>
Signed-off-by: Bibo Mao <maobibo@loongson.cn>
---
 hw/loongarch/virt.c    | 145 ++++++++++++++++++++++++++++++++++++++++-
 target/loongarch/cpu.c |  13 ++++
 2 files changed, 156 insertions(+), 2 deletions(-)

diff --git a/hw/loongarch/virt.c b/hw/loongarch/virt.c
index 2e4a71bc90..7128940d61 100644
--- a/hw/loongarch/virt.c
+++ b/hw/loongarch/virt.c
@@ -1165,8 +1165,6 @@ static void virt_init(MachineState *machine)
     mc->possible_cpu_arch_ids(machine);
     for (i = 0; i < machine->smp.cpus; i++) {
         cpu = CPU(object_new(machine->cpu_type));
-        cpu->cpu_index = i;
-        machine->possible_cpus->cpus[i].cpu = cpu;
         lacpu = LOONGARCH_CPU(cpu);
         lacpu->phy_id = machine->possible_cpus->cpus[i].arch_id;
         /*
@@ -1328,6 +1326,140 @@ static void virt_get_topo_from_index(MachineState *ms,
     topo->thread_id = index % ms->smp.threads;
 }
 
+/* find cpu slot in machine->possible_cpus by arch_id */
+static CPUArchId *virt_find_cpu_slot(MachineState *ms, int arch_id, int *index)
+{
+    int n;
+    for (n = 0; n < ms->possible_cpus->len; n++) {
+        if (ms->possible_cpus->cpus[n].arch_id == arch_id) {
+            if (index) {
+                *index = n;
+            }
+            return &ms->possible_cpus->cpus[n];
+        }
+    }
+
+    return NULL;
+}
+
+static void virt_cpu_pre_plug(HotplugHandler *hotplug_dev,
+                              DeviceState *dev, Error **errp)
+{
+    LoongArchVirtMachineState *lvms = LOONGARCH_VIRT_MACHINE(hotplug_dev);
+    MachineState *ms = MACHINE(OBJECT(hotplug_dev));
+    LoongArchCPU *cpu = LOONGARCH_CPU(dev);
+    CPUState *cs = CPU(dev);
+    CPUArchId *cpu_slot;
+    Error *local_err = NULL;
+    LoongArchCPUTopo topo;
+    int arch_id, index = 0;
+
+    /* 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 (lvms->acpi_ged) {
+        hotplug_handler_pre_plug(HOTPLUG_HANDLER(lvms->acpi_ged), dev,
+                                 &local_err);
+        if (local_err) {
+            error_propagate(errp, local_err);
+           return;
+        }
+     }
+
+    if (cpu->phy_id == UNSET_PHY_ID) {
+        error_setg(&local_err, "CPU hotplug not supported");
+        goto out;
+    } else {
+        /*
+         * For cold-add cpu, topo property is not set. And only physical id
+         * is set, setup topo information from physical id.
+         */
+        arch_id = cpu->phy_id;
+        cpu_slot = virt_find_cpu_slot(ms, arch_id, &index);
+        virt_get_topo_from_index(ms, &topo, index);
+        cpu->socket_id = topo.socket_id;
+        cpu->core_id   = topo.core_id;
+        cpu->thread_id = topo.thread_id;
+    }
+
+    /*
+     * update cpu_index calculation method since it is easily used as index
+     * with possible_cpus array by function virt_cpu_index_to_props
+     */
+    cs->cpu_index = index;
+    numa_cpu_pre_plug(cpu_slot, dev, &local_err);
+    return ;
+
+out:
+    error_propagate(errp, local_err);
+}
+
+static void virt_cpu_unplug_request(HotplugHandler *hotplug_dev,
+                                       DeviceState *dev, Error **errp)
+{
+    LoongArchVirtMachineState *lvms = LOONGARCH_VIRT_MACHINE(hotplug_dev);
+    Error *local_err = NULL;
+    LoongArchCPU *cpu = LOONGARCH_CPU(dev);
+    CPUState *cs = CPU(dev);
+
+    if (!lvms->acpi_ged) {
+        error_setg(&local_err, "CPU hot unplug not supported without ACPI");
+        error_propagate(errp, local_err);
+        return;
+    }
+
+    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);
+        error_propagate(errp, local_err);
+        return;
+    }
+
+    hotplug_handler_unplug_request(HOTPLUG_HANDLER(lvms->acpi_ged), dev,
+                                   &local_err);
+    if (local_err) {
+        error_propagate(errp, local_err);
+        return;
+    }
+}
+
+static void virt_cpu_unplug(HotplugHandler *hotplug_dev,
+                                DeviceState *dev, Error **errp)
+{
+    CPUArchId *cpu_slot;
+    Error *local_err = NULL;
+    LoongArchCPU *cpu = LOONGARCH_CPU(dev);
+    LoongArchVirtMachineState *lvms = LOONGARCH_VIRT_MACHINE(hotplug_dev);
+
+    hotplug_handler_unplug(HOTPLUG_HANDLER(lvms->acpi_ged), dev, &local_err);
+    if (local_err) {
+        error_propagate(errp, local_err);
+        return;
+    }
+
+    cpu_slot = virt_find_cpu_slot(MACHINE(lvms), cpu->phy_id, NULL);
+    cpu_slot->cpu = NULL;
+    return;
+}
+
+static void virt_cpu_plug(HotplugHandler *hotplug_dev,
+                                DeviceState *dev, Error **errp)
+{
+    CPUArchId *cpu_slot;
+    LoongArchCPU *cpu = LOONGARCH_CPU(dev);
+    LoongArchVirtMachineState *lvms = LOONGARCH_VIRT_MACHINE(hotplug_dev);
+
+    cpu_slot = virt_find_cpu_slot(MACHINE(lvms), cpu->phy_id, NULL);
+    cpu_slot->cpu = CPU(dev);
+    return;
+}
+
 static bool memhp_type_supported(DeviceState *dev)
 {
     /* we only support pc dimm now */
@@ -1346,6 +1478,8 @@ static void virt_device_pre_plug(HotplugHandler *hotplug_dev,
 {
     if (memhp_type_supported(dev)) {
         virt_mem_pre_plug(hotplug_dev, dev, errp);
+    } else if (object_dynamic_cast(OBJECT(dev), TYPE_LOONGARCH_CPU)) {
+        virt_cpu_pre_plug(hotplug_dev, dev, errp);
     }
 }
 
@@ -1364,6 +1498,8 @@ static void virt_device_unplug_request(HotplugHandler *hotplug_dev,
 {
     if (memhp_type_supported(dev)) {
         virt_mem_unplug_request(hotplug_dev, dev, errp);
+    } else if (object_dynamic_cast(OBJECT(dev), TYPE_LOONGARCH_CPU)) {
+        virt_cpu_unplug_request(hotplug_dev, dev, errp);
     }
 }
 
@@ -1382,6 +1518,8 @@ static void virt_device_unplug(HotplugHandler *hotplug_dev,
 {
     if (memhp_type_supported(dev)) {
         virt_mem_unplug(hotplug_dev, dev, errp);
+    } else if (object_dynamic_cast(OBJECT(dev), TYPE_LOONGARCH_CPU)) {
+        virt_cpu_unplug(hotplug_dev, dev, errp);
     }
 }
 
@@ -1409,6 +1547,8 @@ static void virt_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)) {
+        virt_cpu_plug(hotplug_dev, dev, errp);
     }
 }
 
@@ -1418,6 +1558,7 @@ static HotplugHandler *virt_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) ||
         object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_IOMMU_PCI) ||
         memhp_type_supported(dev)) {
         return HOTPLUG_HANDLER(machine);
diff --git a/target/loongarch/cpu.c b/target/loongarch/cpu.c
index 17b65d5946..0d1dc2c1c7 100644
--- a/target/loongarch/cpu.c
+++ b/target/loongarch/cpu.c
@@ -613,6 +613,17 @@ 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
+    cpu_remove_sync(CPU(dev));
+#endif
+
+    mcc->parent_unrealize(dev);
+}
+
 static bool loongarch_get_lsx(Object *obj, Error **errp)
 {
     LoongArchCPU *cpu = LOONGARCH_CPU(obj);
@@ -800,6 +811,8 @@ static void loongarch_cpu_class_init(ObjectClass *c, void *data)
     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);
 
-- 
2.39.3

[PATCH v3 3/5] hw/loongarch/virt: Add generic function to init interrupt pin of CPU

[Bibo Mao]

Here generic function virt_init_cpu_irq() is added to init interrupt
pin of CPU object, IPI and extioi interrupt controllers are connected
to interrupt pin of CPU object.

The generic function can be used to both cold-plug and hot-plug CPUs.

Signed-off-by: Bibo Mao <maobibo@loongson.cn>
---
 hw/loongarch/virt.c | 76 +++++++++++++++++++++++++++++----------------
 1 file changed, 49 insertions(+), 27 deletions(-)

diff --git a/hw/loongarch/virt.c b/hw/loongarch/virt.c
index 7128940d61..ab0ac6cf0f 100644
--- a/hw/loongarch/virt.c
+++ b/hw/loongarch/virt.c
@@ -58,6 +58,20 @@ static bool virt_is_veiointc_enabled(LoongArchVirtMachineState *lvms)
     return true;
 }
 
+static CPUState *virt_get_cpu(MachineState *ms, int index)
+{
+    MachineClass *mc = MACHINE_GET_CLASS(ms);
+    const CPUArchIdList *possible_cpus;
+
+    /* Init CPUs */
+    possible_cpus = mc->possible_cpu_arch_ids(ms);
+    if (index < 0 || index >= possible_cpus->len) {
+        return NULL;
+    }
+
+    return possible_cpus->cpus[index].cpu;
+}
+
 static void virt_get_veiointc(Object *obj, Visitor *v, const char *name,
                               void *opaque, Error **errp)
 {
@@ -365,7 +379,7 @@ static void create_fdt(LoongArchVirtMachineState *lvms)
 static void fdt_add_cpu_nodes(const LoongArchVirtMachineState *lvms)
 {
     int num;
-    const MachineState *ms = MACHINE(lvms);
+    MachineState *ms = MACHINE(lvms);
     int smp_cpus = ms->smp.cpus;
 
     qemu_fdt_add_subnode(ms->fdt, "/cpus");
@@ -375,7 +389,7 @@ static void fdt_add_cpu_nodes(const LoongArchVirtMachineState *lvms)
     /* cpu nodes */
     for (num = smp_cpus - 1; num >= 0; num--) {
         char *nodename = g_strdup_printf("/cpus/cpu@%d", num);
-        LoongArchCPU *cpu = LOONGARCH_CPU(qemu_get_cpu(num));
+        LoongArchCPU *cpu = LOONGARCH_CPU(virt_get_cpu(ms, num));
         CPUState *cs = CPU(cpu);
 
         qemu_fdt_add_subnode(ms->fdt, nodename);
@@ -783,16 +797,42 @@ static void virt_devices_init(DeviceState *pch_pic,
     lvms->platform_bus_dev = create_platform_bus(pch_pic);
 }
 
+static void virt_init_cpu_irq(MachineState *ms, CPUState *cs)
+{
+    LoongArchCPU *cpu = LOONGARCH_CPU(cs);
+    CPULoongArchState *env;
+    LoongArchVirtMachineState *lvms = LOONGARCH_VIRT_MACHINE(ms);
+    int pin;
+
+    if (!lvms->ipi || !lvms->extioi) {
+        return;
+    }
+
+    env = &(cpu->env);
+    env->address_space_iocsr = &lvms->as_iocsr;
+    env->ipistate = lvms->ipi;
+    /* connect ipi irq to cpu irq, logic cpu index used here */
+    qdev_connect_gpio_out(lvms->ipi, cs->cpu_index,
+                              qdev_get_gpio_in(DEVICE(cs), IRQ_IPI));
+
+    /*
+     * connect ext irq to the cpu irq
+     * cpu_pin[9:2] <= intc_pin[7:0]
+     */
+    for (pin = 0; pin < LS3A_INTC_IP; pin++) {
+        qdev_connect_gpio_out(lvms->extioi, cs->cpu_index * LS3A_INTC_IP + pin,
+                              qdev_get_gpio_in(DEVICE(cs), pin + 2));
+    }
+}
+
 static void virt_irq_init(LoongArchVirtMachineState *lvms)
 {
     MachineState *ms = MACHINE(lvms);
-    DeviceState *pch_pic, *pch_msi, *cpudev;
+    DeviceState *pch_pic, *pch_msi;
     DeviceState *ipi, *extioi;
     SysBusDevice *d;
-    LoongArchCPU *lacpu;
-    CPULoongArchState *env;
     CPUState *cpu_state;
-    int cpu, pin, i, start, num;
+    int cpu, i, start, num;
     uint32_t cpuintc_phandle, eiointc_phandle, pch_pic_phandle, pch_msi_phandle;
 
     /*
@@ -853,18 +893,6 @@ static void virt_irq_init(LoongArchVirtMachineState *lvms)
     /* Add cpu interrupt-controller */
     fdt_add_cpuic_node(lvms, &cpuintc_phandle);
 
-    for (cpu = 0; cpu < ms->smp.cpus; cpu++) {
-        cpu_state = qemu_get_cpu(cpu);
-        cpudev = DEVICE(cpu_state);
-        lacpu = LOONGARCH_CPU(cpu_state);
-        env = &(lacpu->env);
-        env->address_space_iocsr = &lvms->as_iocsr;
-
-        /* connect ipi irq to cpu irq */
-        qdev_connect_gpio_out(ipi, cpu, qdev_get_gpio_in(cpudev, IRQ_IPI));
-        env->ipistate = ipi;
-    }
-
     /* Create EXTIOI device */
     extioi = qdev_new(TYPE_LOONGARCH_EXTIOI);
     qdev_prop_set_uint32(extioi, "num-cpu", ms->smp.cpus);
@@ -879,16 +907,10 @@ static void virt_irq_init(LoongArchVirtMachineState *lvms)
                     sysbus_mmio_get_region(SYS_BUS_DEVICE(extioi), 1));
     }
 
-    /*
-     * connect ext irq to the cpu irq
-     * cpu_pin[9:2] <= intc_pin[7:0]
-     */
+    /* Connect irq to cpu, including ipi and extioi irqchip */
     for (cpu = 0; cpu < ms->smp.cpus; cpu++) {
-        cpudev = DEVICE(qemu_get_cpu(cpu));
-        for (pin = 0; pin < LS3A_INTC_IP; pin++) {
-            qdev_connect_gpio_out(extioi, (cpu * 8 + pin),
-                                  qdev_get_gpio_in(cpudev, pin + 2));
-        }
+        cpu_state = virt_get_cpu(ms, cpu);
+        virt_init_cpu_irq(ms, cpu_state);
     }
 
     /* Add Extend I/O Interrupt Controller node */
-- 
2.39.3

[PATCH v3 4/5] hw/loongarch/virt: Update the ACPI table for hotplug cpu

[Bibo Mao]

On LoongArch virt machine, ACPI GED hardware is used for CPU hotplug
handler, here CPU hotplug support feature is added based on GED handler,
also CPU scan and reject method is added about CPU device in DSDT table.

Co-developed-by: Xianglai Li <lixianglai@loongson.cn>
Signed-off-by: Bibo Mao <maobibo@loongson.cn>
---
 hw/loongarch/Kconfig        |  1 +
 hw/loongarch/acpi-build.c   | 35 +++++++++++++++++++++++++++++++++--
 hw/loongarch/virt.c         | 10 ++++++++++
 include/hw/loongarch/virt.h |  1 +
 4 files changed, 45 insertions(+), 2 deletions(-)

diff --git a/hw/loongarch/Kconfig b/hw/loongarch/Kconfig
index fe1c6feac1..bb2838b7b5 100644
--- a/hw/loongarch/Kconfig
+++ b/hw/loongarch/Kconfig
@@ -17,6 +17,7 @@ config LOONGARCH_VIRT
     select LOONGARCH_EXTIOI
     select LS7A_RTC
     select SMBIOS
+    select ACPI_CPU_HOTPLUG
     select ACPI_PCI
     select ACPI_HW_REDUCED
     select FW_CFG_DMA
diff --git a/hw/loongarch/acpi-build.c b/hw/loongarch/acpi-build.c
index 50709bda0f..c220edec68 100644
--- a/hw/loongarch/acpi-build.c
+++ b/hw/loongarch/acpi-build.c
@@ -47,6 +47,22 @@
 #define ACPI_BUILD_DPRINTF(fmt, ...)
 #endif
 
+static void virt_madt_cpu_entry(int uid,
+                                const CPUArchIdList *apic_ids,
+                                GArray *entry, bool force_enabled)
+{
+    uint32_t flags, apic_id = apic_ids->cpus[uid].arch_id;
+
+    flags = apic_ids->cpus[uid].cpu || 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)
 {
@@ -123,15 +139,17 @@ build_madt(GArray *table_data, BIOSLinker *linker,
     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, 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 */
@@ -334,6 +352,7 @@ build_la_ged_aml(Aml *dsdt, MachineState *machine)
 {
     uint32_t event;
     LoongArchVirtMachineState *lvms = LOONGARCH_VIRT_MACHINE(machine);
+    CPUHotplugFeatures opts;
 
     build_ged_aml(dsdt, "\\_SB."GED_DEVICE,
                   HOTPLUG_HANDLER(lvms->acpi_ged),
@@ -346,6 +365,18 @@ build_la_ged_aml(Aml *dsdt, MachineState *machine)
                                  AML_SYSTEM_MEMORY,
                                  VIRT_GED_MEM_ADDR);
     }
+
+    if (event & ACPI_GED_CPU_HOTPLUG_EVT) {
+        opts.acpi_1_compatible = false;
+        opts.has_legacy_cphp = false;
+        opts.fw_unplugs_cpu = false;
+        opts.smi_path = NULL;
+
+        build_cpus_aml(dsdt, machine, opts, virt_madt_cpu_entry,
+                       VIRT_GED_CPUHP_ADDR, "\\_SB",
+                       AML_GED_EVT_CPU_SCAN_METHOD, AML_SYSTEM_MEMORY);
+    }
+
     acpi_dsdt_add_power_button(dsdt);
 }
 
diff --git a/hw/loongarch/virt.c b/hw/loongarch/virt.c
index ab0ac6cf0f..e73f689c83 100644
--- a/hw/loongarch/virt.c
+++ b/hw/loongarch/virt.c
@@ -666,11 +666,17 @@ static DeviceState *create_acpi_ged(DeviceState *pch_pic,
 {
     DeviceState *dev;
     MachineState *ms = MACHINE(lvms);
+    MachineClass *mc = MACHINE_GET_CLASS(lvms);
     uint32_t event = ACPI_GED_PWR_DOWN_EVT;
 
     if (ms->ram_slots) {
         event |= ACPI_GED_MEM_HOTPLUG_EVT;
     }
+
+    if (mc->has_hotpluggable_cpus) {
+        event |= ACPI_GED_CPU_HOTPLUG_EVT;
+    }
+
     dev = qdev_new(TYPE_ACPI_GED);
     qdev_prop_set_uint32(dev, "ged-event", event);
     sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
@@ -682,6 +688,10 @@ static DeviceState *create_acpi_ged(DeviceState *pch_pic,
     /* ged regs used for reset and power down */
     sysbus_mmio_map(SYS_BUS_DEVICE(dev), 2, VIRT_GED_REG_ADDR);
 
+    if (mc->has_hotpluggable_cpus) {
+        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));
     return dev;
diff --git a/include/hw/loongarch/virt.h b/include/hw/loongarch/virt.h
index 9ba47793ef..861034d614 100644
--- a/include/hw/loongarch/virt.h
+++ b/include/hw/loongarch/virt.h
@@ -30,6 +30,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_GED_REG_COUNT)
 
 #define COMMAND_LINE_SIZE       512
 
-- 
2.39.3

[PATCH v3 5/5] hw/loongarch/virt: Enable cpu hotplug feature on virt machine

[Bibo Mao]

On virt machine, enable CPU hotplug feature has_hotpluggable_cpus. For
hot-added CPUs after power on, interrupt pin of extioi and ipi interrupt
controller need connect to pins of new CPU.

Also change num-cpu property of extioi and ipi from smp.cpus to
smp.max_cpus

Co-developed-by: Xianglai Li <lixianglai@loongson.cn>
Signed-off-by: Bibo Mao <maobibo@loongson.cn>
---
 hw/loongarch/virt.c         | 57 +++++++++++++++++++++++++++++++++----
 include/hw/loongarch/virt.h |  2 ++
 2 files changed, 54 insertions(+), 5 deletions(-)

diff --git a/hw/loongarch/virt.c b/hw/loongarch/virt.c
index e73f689c83..6673493109 100644
--- a/hw/loongarch/virt.c
+++ b/hw/loongarch/virt.c
@@ -891,8 +891,9 @@ static void virt_irq_init(LoongArchVirtMachineState *lvms)
 
     /* Create IPI device */
     ipi = qdev_new(TYPE_LOONGARCH_IPI);
-    qdev_prop_set_uint32(ipi, "num-cpu", ms->smp.cpus);
+    qdev_prop_set_uint32(ipi, "num-cpu", ms->smp.max_cpus);
     sysbus_realize_and_unref(SYS_BUS_DEVICE(ipi), &error_fatal);
+    lvms->ipi = ipi;
 
     /* IPI iocsr memory region */
     memory_region_add_subregion(&lvms->system_iocsr, SMP_IPI_MAILBOX,
@@ -905,11 +906,12 @@ static void virt_irq_init(LoongArchVirtMachineState *lvms)
 
     /* Create EXTIOI device */
     extioi = qdev_new(TYPE_LOONGARCH_EXTIOI);
-    qdev_prop_set_uint32(extioi, "num-cpu", ms->smp.cpus);
+    qdev_prop_set_uint32(extioi, "num-cpu", ms->smp.max_cpus);
     if (virt_is_veiointc_enabled(lvms)) {
         qdev_prop_set_bit(extioi, "has-virtualization-extension", true);
     }
     sysbus_realize_and_unref(SYS_BUS_DEVICE(extioi), &error_fatal);
+    lvms->extioi = extioi;
     memory_region_add_subregion(&lvms->system_iocsr, APIC_BASE,
                     sysbus_mmio_get_region(SYS_BUS_DEVICE(extioi), 0));
     if (virt_is_veiointc_enabled(lvms)) {
@@ -1403,8 +1405,40 @@ static void virt_cpu_pre_plug(HotplugHandler *hotplug_dev,
      }
 
     if (cpu->phy_id == UNSET_PHY_ID) {
-        error_setg(&local_err, "CPU hotplug not supported");
-        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 - 1);
+            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;
+        }
+
+        topo.socket_id = cpu->socket_id;
+        topo.core_id = cpu->core_id;
+        topo.thread_id = cpu->thread_id;
+        arch_id =  virt_get_arch_id_from_topo(ms, &topo);
+        cpu_slot = virt_find_cpu_slot(ms, arch_id, &index);
+        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;
+        }
+        cpu->phy_id = arch_id;
     } else {
         /*
          * For cold-add cpu, topo property is not set. And only physical id
@@ -1484,10 +1518,22 @@ static void virt_cpu_plug(HotplugHandler *hotplug_dev,
                                 DeviceState *dev, Error **errp)
 {
     CPUArchId *cpu_slot;
+    Error *local_err = NULL;
     LoongArchCPU *cpu = LOONGARCH_CPU(dev);
+    MachineState *ms = MACHINE(hotplug_dev);
     LoongArchVirtMachineState *lvms = LOONGARCH_VIRT_MACHINE(hotplug_dev);
 
-    cpu_slot = virt_find_cpu_slot(MACHINE(lvms), cpu->phy_id, NULL);
+    if (lvms->acpi_ged) {
+        /* Connect irq to cpu, including ipi and extioi irqchip */
+        virt_init_cpu_irq(ms, CPU(cpu));
+        hotplug_handler_plug(HOTPLUG_HANDLER(lvms->acpi_ged), dev, &local_err);
+        if (local_err) {
+            error_propagate(errp, local_err);
+            return;
+        }
+    }
+
+    cpu_slot = virt_find_cpu_slot(ms, cpu->phy_id, NULL);
     cpu_slot->cpu = CPU(dev);
     return;
 }
@@ -1671,6 +1717,7 @@ static void virt_class_init(ObjectClass *oc, void *data)
     mc->numa_mem_supported = true;
     mc->auto_enable_numa_with_memhp = true;
     mc->auto_enable_numa_with_memdev = true;
+    mc->has_hotpluggable_cpus = true;
     mc->get_hotplug_handler = virt_get_hotplug_handler;
     mc->default_nic = "virtio-net-pci";
     hc->plug = virt_device_plug_cb;
diff --git a/include/hw/loongarch/virt.h b/include/hw/loongarch/virt.h
index 861034d614..79a85723c9 100644
--- a/include/hw/loongarch/virt.h
+++ b/include/hw/loongarch/virt.h
@@ -61,6 +61,8 @@ struct LoongArchVirtMachineState {
     MemoryRegion iocsr_mem;
     AddressSpace as_iocsr;
     struct loongarch_boot_info bootinfo;
+    DeviceState *ipi;
+    DeviceState *extioi;
 };
 
 #define TYPE_LOONGARCH_VIRT_MACHINE  MACHINE_TYPE_NAME("virt")
-- 
2.39.3

Re: [PATCH v3 5/5] hw/loongarch/virt: Enable cpu hotplug feature on virt machine

[Igor Mammedov]

On Mon,  4 Nov 2024 14:34:35 +0800
Bibo Mao <maobibo@loongson.cn> wrote:

> On virt machine, enable CPU hotplug feature has_hotpluggable_cpus. For
> hot-added CPUs after power on, interrupt pin of extioi and ipi interrupt
> controller need connect to pins of new CPU.
> 
> Also change num-cpu property of extioi and ipi from smp.cpus to
> smp.max_cpus
> 
> Co-developed-by: Xianglai Li <lixianglai@loongson.cn>
> Signed-off-by: Bibo Mao <maobibo@loongson.cn>
> ---
>  hw/loongarch/virt.c         | 57 +++++++++++++++++++++++++++++++++----
>  include/hw/loongarch/virt.h |  2 ++
>  2 files changed, 54 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/loongarch/virt.c b/hw/loongarch/virt.c
> index e73f689c83..6673493109 100644
> --- a/hw/loongarch/virt.c
> +++ b/hw/loongarch/virt.c
> @@ -891,8 +891,9 @@ static void virt_irq_init(LoongArchVirtMachineState *lvms)
>  
>      /* Create IPI device */
>      ipi = qdev_new(TYPE_LOONGARCH_IPI);
> -    qdev_prop_set_uint32(ipi, "num-cpu", ms->smp.cpus);
> +    qdev_prop_set_uint32(ipi, "num-cpu", ms->smp.max_cpus);
>      sysbus_realize_and_unref(SYS_BUS_DEVICE(ipi), &error_fatal);
> +    lvms->ipi = ipi;
>  
>      /* IPI iocsr memory region */
>      memory_region_add_subregion(&lvms->system_iocsr, SMP_IPI_MAILBOX,
> @@ -905,11 +906,12 @@ static void virt_irq_init(LoongArchVirtMachineState *lvms)
>  
>      /* Create EXTIOI device */
>      extioi = qdev_new(TYPE_LOONGARCH_EXTIOI);
> -    qdev_prop_set_uint32(extioi, "num-cpu", ms->smp.cpus);
> +    qdev_prop_set_uint32(extioi, "num-cpu", ms->smp.max_cpus);
>      if (virt_is_veiointc_enabled(lvms)) {
>          qdev_prop_set_bit(extioi, "has-virtualization-extension", true);
>      }
>      sysbus_realize_and_unref(SYS_BUS_DEVICE(extioi), &error_fatal);
> +    lvms->extioi = extioi;
>      memory_region_add_subregion(&lvms->system_iocsr, APIC_BASE,
>                      sysbus_mmio_get_region(SYS_BUS_DEVICE(extioi), 0));
>      if (virt_is_veiointc_enabled(lvms)) {
> @@ -1403,8 +1405,40 @@ static void virt_cpu_pre_plug(HotplugHandler *hotplug_dev,
>       }
>  
>      if (cpu->phy_id == UNSET_PHY_ID) {
> -        error_setg(&local_err, "CPU hotplug not supported");
> -        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 - 1);
> +            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;
> +        }
> +
> +        topo.socket_id = cpu->socket_id;
> +        topo.core_id = cpu->core_id;
> +        topo.thread_id = cpu->thread_id;
> +        arch_id =  virt_get_arch_id_from_topo(ms, &topo);
> +        cpu_slot = virt_find_cpu_slot(ms, arch_id, &index);
> +        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;
> +        }
> +        cpu->phy_id = arch_id;
>      } else {
>          /*
>           * For cold-add cpu, topo property is not set. And only physical id
> @@ -1484,10 +1518,22 @@ static void virt_cpu_plug(HotplugHandler *hotplug_dev,
>                                  DeviceState *dev, Error **errp)
>  {
>      CPUArchId *cpu_slot;
> +    Error *local_err = NULL;
>      LoongArchCPU *cpu = LOONGARCH_CPU(dev);
> +    MachineState *ms = MACHINE(hotplug_dev);
>      LoongArchVirtMachineState *lvms = LOONGARCH_VIRT_MACHINE(hotplug_dev);
>  
> -    cpu_slot = virt_find_cpu_slot(MACHINE(lvms), cpu->phy_id, NULL);
> +    if (lvms->acpi_ged) {
> +        /* Connect irq to cpu, including ipi and extioi irqchip */

> +        virt_init_cpu_irq(ms, CPU(cpu));

why it depends on GED?
Can't you just call it unconditionally here for both hotplugged and coldplugged CPUs?
/and drop this call from for() loop of coldplug CPUs/

> +        hotplug_handler_plug(HOTPLUG_HANDLER(lvms->acpi_ged), dev, &local_err);
> +        if (local_err) {
> +            error_propagate(errp, local_err);
> +            return;
> +        }
> +    }
> +
> +    cpu_slot = virt_find_cpu_slot(ms, cpu->phy_id, NULL);
>      cpu_slot->cpu = CPU(dev);
>      return;
>  }
> @@ -1671,6 +1717,7 @@ static void virt_class_init(ObjectClass *oc, void *data)
>      mc->numa_mem_supported = true;
>      mc->auto_enable_numa_with_memhp = true;
>      mc->auto_enable_numa_with_memdev = true;
> +    mc->has_hotpluggable_cpus = true;
>      mc->get_hotplug_handler = virt_get_hotplug_handler;
>      mc->default_nic = "virtio-net-pci";
>      hc->plug = virt_device_plug_cb;
> diff --git a/include/hw/loongarch/virt.h b/include/hw/loongarch/virt.h
> index 861034d614..79a85723c9 100644
> --- a/include/hw/loongarch/virt.h
> +++ b/include/hw/loongarch/virt.h
> @@ -61,6 +61,8 @@ struct LoongArchVirtMachineState {
>      MemoryRegion iocsr_mem;
>      AddressSpace as_iocsr;
>      struct loongarch_boot_info bootinfo;
> +    DeviceState *ipi;
> +    DeviceState *extioi;
>  };
>  
>  #define TYPE_LOONGARCH_VIRT_MACHINE  MACHINE_TYPE_NAME("virt")

Re: [PATCH v3 5/5] hw/loongarch/virt: Enable cpu hotplug feature on virt machine

[maobibo]

On 2024/11/5 下午9:58, Igor Mammedov wrote:
> On Mon,  4 Nov 2024 14:34:35 +0800
> Bibo Mao <maobibo@loongson.cn> wrote:
> 
>> On virt machine, enable CPU hotplug feature has_hotpluggable_cpus. For
>> hot-added CPUs after power on, interrupt pin of extioi and ipi interrupt
>> controller need connect to pins of new CPU.
>>
>> Also change num-cpu property of extioi and ipi from smp.cpus to
>> smp.max_cpus
>>
>> Co-developed-by: Xianglai Li <lixianglai@loongson.cn>
>> Signed-off-by: Bibo Mao <maobibo@loongson.cn>
>> ---
>>   hw/loongarch/virt.c         | 57 +++++++++++++++++++++++++++++++++----
>>   include/hw/loongarch/virt.h |  2 ++
>>   2 files changed, 54 insertions(+), 5 deletions(-)
>>
>> diff --git a/hw/loongarch/virt.c b/hw/loongarch/virt.c
>> index e73f689c83..6673493109 100644
>> --- a/hw/loongarch/virt.c
>> +++ b/hw/loongarch/virt.c
>> @@ -891,8 +891,9 @@ static void virt_irq_init(LoongArchVirtMachineState *lvms)
>>   
>>       /* Create IPI device */
>>       ipi = qdev_new(TYPE_LOONGARCH_IPI);
>> -    qdev_prop_set_uint32(ipi, "num-cpu", ms->smp.cpus);
>> +    qdev_prop_set_uint32(ipi, "num-cpu", ms->smp.max_cpus);
>>       sysbus_realize_and_unref(SYS_BUS_DEVICE(ipi), &error_fatal);
>> +    lvms->ipi = ipi;
>>   
>>       /* IPI iocsr memory region */
>>       memory_region_add_subregion(&lvms->system_iocsr, SMP_IPI_MAILBOX,
>> @@ -905,11 +906,12 @@ static void virt_irq_init(LoongArchVirtMachineState *lvms)
>>   
>>       /* Create EXTIOI device */
>>       extioi = qdev_new(TYPE_LOONGARCH_EXTIOI);
>> -    qdev_prop_set_uint32(extioi, "num-cpu", ms->smp.cpus);
>> +    qdev_prop_set_uint32(extioi, "num-cpu", ms->smp.max_cpus);
>>       if (virt_is_veiointc_enabled(lvms)) {
>>           qdev_prop_set_bit(extioi, "has-virtualization-extension", true);
>>       }
>>       sysbus_realize_and_unref(SYS_BUS_DEVICE(extioi), &error_fatal);
>> +    lvms->extioi = extioi;
>>       memory_region_add_subregion(&lvms->system_iocsr, APIC_BASE,
>>                       sysbus_mmio_get_region(SYS_BUS_DEVICE(extioi), 0));
>>       if (virt_is_veiointc_enabled(lvms)) {
>> @@ -1403,8 +1405,40 @@ static void virt_cpu_pre_plug(HotplugHandler *hotplug_dev,
>>        }
>>   
>>       if (cpu->phy_id == UNSET_PHY_ID) {
>> -        error_setg(&local_err, "CPU hotplug not supported");
>> -        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 - 1);
>> +            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;
>> +        }
>> +
>> +        topo.socket_id = cpu->socket_id;
>> +        topo.core_id = cpu->core_id;
>> +        topo.thread_id = cpu->thread_id;
>> +        arch_id =  virt_get_arch_id_from_topo(ms, &topo);
>> +        cpu_slot = virt_find_cpu_slot(ms, arch_id, &index);
>> +        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;
>> +        }
>> +        cpu->phy_id = arch_id;
>>       } else {
>>           /*
>>            * For cold-add cpu, topo property is not set. And only physical id
>> @@ -1484,10 +1518,22 @@ static void virt_cpu_plug(HotplugHandler *hotplug_dev,
>>                                   DeviceState *dev, Error **errp)
>>   {
>>       CPUArchId *cpu_slot;
>> +    Error *local_err = NULL;
>>       LoongArchCPU *cpu = LOONGARCH_CPU(dev);
>> +    MachineState *ms = MACHINE(hotplug_dev);
>>       LoongArchVirtMachineState *lvms = LOONGARCH_VIRT_MACHINE(hotplug_dev);
>>   
>> -    cpu_slot = virt_find_cpu_slot(MACHINE(lvms), cpu->phy_id, NULL);
>> +    if (lvms->acpi_ged) {
>> +        /* Connect irq to cpu, including ipi and extioi irqchip */
> 
>> +        virt_init_cpu_irq(ms, CPU(cpu));
> 
> why it depends on GED?
> Can't you just call it unconditionally here for both hotplugged and coldplugged CPUs?
> /and drop this call from for() loop of coldplug CPUs/
yes, will do in this way, then it will be unified for hotplugged and 
coldplugged CPUs.

CPU objects need be created after interrupt controller, also it can be 
created after acpi ged device. From the code function 
hotplug_handler_plug() will check PHASE_MACHINE_READY and hotplugged 
state, and do nothing for coldplugged CPUs.

And thanks for your guidance, will send the patch in next round.

Regards
Bibo Mao
> 
>> +        hotplug_handler_plug(HOTPLUG_HANDLER(lvms->acpi_ged), dev, &local_err);
>> +        if (local_err) {
>> +            error_propagate(errp, local_err);
>> +            return;
>> +        }
>> +    }
>> +
>> +    cpu_slot = virt_find_cpu_slot(ms, cpu->phy_id, NULL);
>>       cpu_slot->cpu = CPU(dev);
>>       return;
>>   }
>> @@ -1671,6 +1717,7 @@ static void virt_class_init(ObjectClass *oc, void *data)
>>       mc->numa_mem_supported = true;
>>       mc->auto_enable_numa_with_memhp = true;
>>       mc->auto_enable_numa_with_memdev = true;
>> +    mc->has_hotpluggable_cpus = true;
>>       mc->get_hotplug_handler = virt_get_hotplug_handler;
>>       mc->default_nic = "virtio-net-pci";
>>       hc->plug = virt_device_plug_cb;
>> diff --git a/include/hw/loongarch/virt.h b/include/hw/loongarch/virt.h
>> index 861034d614..79a85723c9 100644
>> --- a/include/hw/loongarch/virt.h
>> +++ b/include/hw/loongarch/virt.h
>> @@ -61,6 +61,8 @@ struct LoongArchVirtMachineState {
>>       MemoryRegion iocsr_mem;
>>       AddressSpace as_iocsr;
>>       struct loongarch_boot_info bootinfo;
>> +    DeviceState *ipi;
>> +    DeviceState *extioi;
>>   };
>>   
>>   #define TYPE_LOONGARCH_VIRT_MACHINE  MACHINE_TYPE_NAME("virt")
>