[PATCH v2 00/10] Adds CPU hot-plug support to Loongarch

[PATCH v2 00/10] 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.

For the modification of the public part of the code, we refer to the
arm-related patch, and the link address of the corresponding patch is
as follows:
https://lore.kernel.org/all/20200613213629.21984-1-salil.mehta@huawei.com/

In order to respect the work of "Salil Mehta", we will rebase the first
4 patches in the final patch, which are referenced here to ensure that
the loongarch cpu hotplug can work properly.

The last 6 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.

V2:
- Fix formatting and spelling errors
- Split large patches into smaller patches
  - Split the original patch
    <<Add basic CPU hot-(un)plug support for Loongarch>> into
    <<Added CPU topology support for Loongarch>>
    <<Optimize loongarch_irq_init function implementation >>
    <<Add basic CPU hot-(un)plug support for Loongarch>>.
  - Split the original patch
    <<Update the ACPI table for the Loongarch CPU>> into
    <<Add generic event device for Loongarch>>
    <<Update the ACPI table for the Loongarch CPU>>
- Added loongarch cpu topology calculation method.
- Change the position of the cpu topology patch.
- Change unreasonable variable and function names.



xianglai li (10):
  Update ACPI GED framework to support vcpu hot-(un)plug
  Update CPUs AML with cpu-(ctrl)dev change
  make qdev_disconnect_gpio_out_named() public
  Introduce the CPU address space destruction function
  Added CPU topology support for Loongarch
  Optimize loongarch_irq_init function implementation
  Add basic CPU hot-(un)plug support for Loongarch
  Add support of *unrealize* for Loongarch cpu
  Add generic event device for Loongarch
  Update the ACPI table for the Loongarch CPU

 .../devices/loongarch64-softmmu/default.mak   |   1 +
 docs/system/loongarch/virt.rst                |  31 ++
 hw/acpi/acpi-cpu-hotplug-stub.c               |  15 +
 hw/acpi/cpu.c                                 |  27 +-
 hw/acpi/generic_event_device.c                |  33 ++
 hw/core/gpio.c                                |   4 +-
 hw/i386/acpi-build.c                          |   2 +-
 hw/loongarch/acpi-build.c                     |  33 +-
 hw/loongarch/generic_event_device_loongarch.c |  36 ++
 hw/loongarch/meson.build                      |   2 +-
 hw/loongarch/virt.c                           | 424 +++++++++++++++---
 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                   |  10 +-
 include/hw/qdev-core.h                        |   2 +
 softmmu/physmem.c                             |  24 +
 target/loongarch/cpu.c                        |  35 +-
 target/loongarch/cpu.h                        |  13 +-
 21 files changed, 635 insertions(+), 87 deletions(-)
 create mode 100644 hw/loongarch/generic_event_device_loongarch.c

Cc: "Salil Mehta" <salil.mehta@opnsrc.net>
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>
Cc: Bibo Mao <maobibo@loongson.cn>
Cc: Xianglai li <lixianglai@loongson.cn>
--
2.39.1

[PATCH v2 01/10] 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.

Co-authored-by: "Salil Mehta" <salil.mehta@opnsrc.net>
Cc: "Salil Mehta" <salil.mehta@opnsrc.net>
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>
Cc: Bibo Mao <maobibo@loongson.cn>
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 |  5 ++++
 5 files changed, 54 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..d0a5a43abf 100644
--- a/include/hw/acpi/generic_event_device.h
+++ b/include/hw/acpi/generic_event_device.h
@@ -60,6 +60,7 @@
 #define HW_ACPI_GENERIC_EVENT_DEVICE_H
 
 #include "hw/sysbus.h"
+#include "hw/acpi/cpu_hotplug.h"
 #include "hw/acpi/memory_hotplug.h"
 #include "hw/acpi/ghes.h"
 #include "qom/object.h"
@@ -97,6 +98,7 @@ OBJECT_DECLARE_SIMPLE_TYPE(AcpiGedState, ACPI_GED)
 #define ACPI_GED_MEM_HOTPLUG_EVT   0x1
 #define ACPI_GED_PWR_DOWN_EVT      0x2
 #define ACPI_GED_NVDIMM_HOTPLUG_EVT 0x4
+#define ACPI_GED_CPU_HOTPLUG_EVT    0x8
 
 typedef struct GEDState {
     MemoryRegion evt;
@@ -108,6 +110,9 @@ struct AcpiGedState {
     SysBusDevice parent_obj;
     MemHotplugState memhp_state;
     MemoryRegion container_memhp;
+    CPUHotplugState cpuhp_state;
+    MemoryRegion container_cpuhp;
+    AcpiCpuHotplug cpuhp;
     GEDState ged_state;
     uint32_t ged_event_bitmap;
     qemu_irq irq;
-- 
2.39.1

[PATCH v2 02/10] 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 interface of the build cpus AML
function to accept both IO/MEMORY type regions and update the _CRS object
correspondingly.

Co-authored-by: "Salil Mehta" <salil.mehta@opnsrc.net>
Cc: "Salil Mehta" <salil.mehta@opnsrc.net>
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>
Cc: Bibo Mao <maobibo@loongson.cn>
Signed-off-by: xianglai li <lixianglai@loongson.cn>
---
 hw/acpi/cpu.c         | 20 +++++++++++++++-----
 hw/i386/acpi-build.c  |  2 +-
 include/hw/acpi/cpu.h |  5 +++--
 3 files changed, 19 insertions(+), 8 deletions(-)

diff --git a/hw/acpi/cpu.c b/hw/acpi/cpu.c
index 6897c8789a..41fc157ac9 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,22 @@ 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));
 
+        g_assert(rs == AML_SYSTEM_IO || rs == AML_SYSTEM_MEMORY);
         /* 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));
+            aml_operation_region("PRST", rs,
+                                         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 bb12b0ad43..560f108d38 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -1550,7 +1550,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 v2 03/10] make qdev_disconnect_gpio_out_named() public

[xianglai li]

It will be reused in loongarch/virt.c for unwiring
the vcpu<->exioi interrupts for the vcpu hot-(un)plug
cases.

Co-authored-by: "Salil Mehta" <salil.mehta@opnsrc.net>
Cc: "Salil Mehta" <salil.mehta@opnsrc.net>
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>
Cc: Bibo Mao <maobibo@loongson.cn>
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 151d968238..32bb54163e 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

Re: [PATCH v2 03/10] make qdev_disconnect_gpio_out_named() public

[Philippe Mathieu-Daudé]

Hi,

On 12/9/23 04:11, xianglai li wrote:
> It will be reused in loongarch/virt.c for unwiring
> the vcpu<->exioi interrupts for the vcpu hot-(un)plug
> cases.

Since we never had to use this, I'm surprised we really need it.

QEMU IRQs/GPIOs are similar to hardware ones, and aren't expected
to be rewired at runtime. Usually another hot-pluggable bus layer is
used, and the bus is physically wired toward the hardware.


I suppose you want to add that because a unplugged vCPU is still
receiving IRQs. The question is, why? Before unplugging, I expect
the INTC (IPI) here to be signaled a vCPU is going to be unplugged,
so maybe you are missing handling the unplug event there. See
in loongarch_ipi_writel():

     switch (addr) {
     case CORE_EN_OFF:
         s->en = val;
         break;
     case CORE_SET_OFF:
         s->status |= val;
         if (s->status != 0 && (s->status & s->en) != 0) {
             qemu_irq_raise(s->irq);
         }
         break;
     case CORE_CLEAR_OFF:
         s->status &= ~val;
         if (s->status == 0 && s->en != 0) {
             qemu_irq_lower(s->irq);
         }
         break;

Maybe you need to factor ipi_raise/lower() helpers which check cores
are available & enabled before propagating IRQ?

> Co-authored-by: "Salil Mehta" <salil.mehta@opnsrc.net>
> Cc: "Salil Mehta" <salil.mehta@opnsrc.net>
> 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>
> Cc: Bibo Mao <maobibo@loongson.cn>
> 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 151d968238..32bb54163e 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);
>   

Re: [PATCH v2 03/10] make qdev_disconnect_gpio_out_named() public

[lixianglai]

Hi Philippe Mathieu-Daudé :
> Hi,
>
> On 12/9/23 04:11, xianglai li wrote:
>> It will be reused in loongarch/virt.c for unwiring
>> the vcpu<->exioi interrupts for the vcpu hot-(un)plug
>> cases.
>
> Since we never had to use this, I'm surprised we really need it.
>
> QEMU IRQs/GPIOs are similar to hardware ones, and aren't expected
> to be rewired at runtime. Usually another hot-pluggable bus layer is
> used, and the bus is physically wired toward the hardware.
>
>
> I suppose you want to add that because a unplugged vCPU is still
> receiving IRQs. The question is, why? Before unplugging, I expect
> the INTC (IPI) here to be signaled a vCPU is going to be unplugged,
> so maybe you are missing handling the unplug event there. See


I did not receive an interrupt signal after pulling out the vcpu.

Whether to use the qdev_disconnect_gpio_out_named function does not 
affect the cpu hot swap function.

The qdev_disconnect_gpio_out_named function is used only to make

loongarch_cpu_irq_init and loongarch_cpu_irq_uninit symmetrical.

I thought it would make the whole process more regular and nice.

If this causes confusion I think with the next patch I can remove the 
reference to this function.

Thanks,

xianglai.


> in loongarch_ipi_writel():
>
>     switch (addr) {
>     case CORE_EN_OFF:
>         s->en = val;
>         break;
>     case CORE_SET_OFF:
>         s->status |= val;
>         if (s->status != 0 && (s->status & s->en) != 0) {
>             qemu_irq_raise(s->irq);
>         }
>         break;
>     case CORE_CLEAR_OFF:
>         s->status &= ~val;
>         if (s->status == 0 && s->en != 0) {
>             qemu_irq_lower(s->irq);
>         }
>         break;
>
> Maybe you need to factor ipi_raise/lower() helpers which check cores
> are available & enabled before propagating IRQ?
>
>> Co-authored-by: "Salil Mehta" <salil.mehta@opnsrc.net>
>> Cc: "Salil Mehta" <salil.mehta@opnsrc.net>
>> 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>
>> Cc: Bibo Mao <maobibo@loongson.cn>
>> 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 151d968238..32bb54163e 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);

[PATCH v2 04/10] Introduce the CPU address space destruction function

[xianglai li]

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

Co-authored-by: "Salil Mehta" <salil.mehta@opnsrc.net>
Cc: "Salil Mehta" <salil.mehta@opnsrc.net>
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>
Cc: Bibo Mao <maobibo@loongson.cn>
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 41788c0bdd..eb56a228a2 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 92a4234439..c90cf3a162 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 18277ddd67..c75e3e8042 100644
--- a/softmmu/physmem.c
+++ b/softmmu/physmem.c
@@ -761,6 +761,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];
@@ -774,6 +775,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

Re: [PATCH v2 04/10] Introduce the CPU address space destruction function

[David Hildenbrand]

On 12.09.23 04:11, xianglai li wrote:
> Introduce new function to destroy CPU address space resources
> for cpu hot-(un)plug.
> 
How do other archs handle that? Or how are they able to get away without 
destroying?

-- 
Cheers,

David / dhildenb

Re: [PATCH v2 04/10] Introduce the CPU address space destruction function

[lixianglai]

Hi David:

> On 12.09.23 04:11, xianglai li wrote:
>> Introduce new function to destroy CPU address space resources
>> for cpu hot-(un)plug.
>>
> How do other archs handle that? Or how are they able to get away 
> without destroying?
>
They do not remove the cpu address space, taking the X86 architecture as 
an example:

1.Start the x86 VM:

./qemu-system-x86_64 \
-machine q35  \
-cpu Broadwell-IBRS \
-smp 1,maxcpus=100,sockets=100,cores=1,threads=1 \
-m 4G \
-drive file=~/anolis-8.8.qcow2  \
-serial stdio   \
-monitor telnet:localhost:4498,server,nowait   \
-nographic

2.Connect the qemu monitor

telnet 127.0.0.1 4498

info mtree

address-space: cpu-memory-0
address-space: memory
   0000000000000000-ffffffffffffffff (prio 0, i/o): system
     0000000000000000-000000007fffffff (prio 0, ram): alias ram-below-4g 
@pc.ram 0000000000000000-000000007fffffff
     0000000000000000-ffffffffffffffff (prio -1, i/o): pci
       00000000000a0000-00000000000bffff (prio 1, i/o): vga-lowmem

3.Perform cpu hot swap int qemu monitor

device_add 
Broadwell-IBRS-x86_64-cpu,socket-id=1,core-id=0,thread-id=0,id=cpu1
device_del cpu1

info mtree

address-space: cpu-memory-0
address-space: cpu-memory-1
address-space: memory
   0000000000000000-ffffffffffffffff (prio 0, i/o): system
     0000000000000000-000000007fffffff (prio 0, ram): alias ram-below-4g 
@pc.ram 0000000000000000-000000007fffffff
     0000000000000000-ffffffffffffffff (prio -1, i/o): pci
       00000000000a0000-00000000000bffff (prio 1, i/o): vga-lowmem


 From the above test, you can see whether the address space of cpu1 is 
residual after a cpu hot swap, and whether it is reasonable?

The address space destruction function of the CPU can be used to delete 
the residual address space of the CPU1.

Thanks,

xianglai.

Re: [PATCH v2 04/10] Introduce the CPU address space destruction function

[David Hildenbrand]

On 14.09.23 15:00, lixianglai wrote:
> Hi David:

Hi!

> 
>> On 12.09.23 04:11, xianglai li wrote:
>>> Introduce new function to destroy CPU address space resources
>>> for cpu hot-(un)plug.
>>>
>> How do other archs handle that? Or how are they able to get away
>> without destroying?
>>
> They do not remove the cpu address space, taking the X86 architecture as
> an example:
> 
> 1.Start the x86 VM:
> 
> ./qemu-system-x86_64 \
> -machine q35  \
> -cpu Broadwell-IBRS \
> -smp 1,maxcpus=100,sockets=100,cores=1,threads=1 \
> -m 4G \
> -drive file=~/anolis-8.8.qcow2  \
> -serial stdio   \
> -monitor telnet:localhost:4498,server,nowait   \
> -nographic
> 
> 2.Connect the qemu monitor
> 
> telnet 127.0.0.1 4498
> 
> info mtree
> 
> address-space: cpu-memory-0
> address-space: memory
>     0000000000000000-ffffffffffffffff (prio 0, i/o): system
>       0000000000000000-000000007fffffff (prio 0, ram): alias ram-below-4g
> @pc.ram 0000000000000000-000000007fffffff
>       0000000000000000-ffffffffffffffff (prio -1, i/o): pci
>         00000000000a0000-00000000000bffff (prio 1, i/o): vga-lowmem
> 
> 3.Perform cpu hot swap int qemu monitor
> 
> device_add
> Broadwell-IBRS-x86_64-cpu,socket-id=1,core-id=0,thread-id=0,id=cpu1
> device_del cpu1
> 

Hm, doesn't seem to work for me on upstream QEMU for some reason: 
"Error: acpi: device unplug request for not supported device type: 
Broadwell-IBRS-x86_64-cpu"

What happens if you re-add that CPU? Will we reuse the previous address 
space?

> info mtree
> 
> address-space: cpu-memory-0
> address-space: cpu-memory-1
> address-space: memory
>     0000000000000000-ffffffffffffffff (prio 0, i/o): system
>       0000000000000000-000000007fffffff (prio 0, ram): alias ram-below-4g
> @pc.ram 0000000000000000-000000007fffffff
>       0000000000000000-ffffffffffffffff (prio -1, i/o): pci
>         00000000000a0000-00000000000bffff (prio 1, i/o): vga-lowmem
> 
> 
>   From the above test, you can see whether the address space of cpu1 is
> residual after a cpu hot swap, and whether it is reasonable?


Probably we should teach other archs to destroy that address space as well.

Can we do that from the core, instead of having to do that in each CPU 
unrealize function?

-- 
Cheers,

David / dhildenb

Re: [PATCH v2 04/10] Introduce the CPU address space destruction function

[lixianglai]

Hi David Hildenbrand:
> On 14.09.23 15:00, lixianglai wrote:
>> Hi David:
>
> Hi!
>
>>
>>> On 12.09.23 04:11, xianglai li wrote:
>>>> Introduce new function to destroy CPU address space resources
>>>> for cpu hot-(un)plug.
>>>>
>>> How do other archs handle that? Or how are they able to get away
>>> without destroying?
>>>
>> They do not remove the cpu address space, taking the X86 architecture as
>> an example:
>>
>> 1.Start the x86 VM:
>>
>> ./qemu-system-x86_64 \
>> -machine q35  \
>> -cpu Broadwell-IBRS \
>> -smp 1,maxcpus=100,sockets=100,cores=1,threads=1 \
>> -m 4G \
>> -drive file=~/anolis-8.8.qcow2  \
>> -serial stdio   \
>> -monitor telnet:localhost:4498,server,nowait   \
>> -nographic
>>
>> 2.Connect the qemu monitor
>>
>> telnet 127.0.0.1 4498
>>
>> info mtree
>>
>> address-space: cpu-memory-0
>> address-space: memory
>>     0000000000000000-ffffffffffffffff (prio 0, i/o): system
>>       0000000000000000-000000007fffffff (prio 0, ram): alias 
>> ram-below-4g
>> @pc.ram 0000000000000000-000000007fffffff
>>       0000000000000000-ffffffffffffffff (prio -1, i/o): pci
>>         00000000000a0000-00000000000bffff (prio 1, i/o): vga-lowmem
>>
>> 3.Perform cpu hot swap int qemu monitor
>>
>> device_add
>> Broadwell-IBRS-x86_64-cpu,socket-id=1,core-id=0,thread-id=0,id=cpu1
>> device_del cpu1
>>
>
> Hm, doesn't seem to work for me on upstream QEMU for some reason: 
> "Error: acpi: device unplug request for not supported device type: 
> Broadwell-IBRS-x86_64-cpu"

>
> What happens if you re-add that CPU? Will we reuse the previous 
> address space?


Here is the memory layout where I inserted cpu1 again. It does not 
appear that the original address space was reused, and the address space 
is now duplicated

info mtree

address-space: cpu-memory-0
address-space: cpu-memory-1
address-space: cpu-memory-1
address-space: memory
   0000000000000000-ffffffffffffffff (prio 0, i/o): system
     0000000000000000-000000007fffffff (prio 0, ram): alias ram-below-4g 
@pc.ram 0000000000000000-000000007fffffff
     0000000000000000-ffffffffffffffff (prio -1, i/o): pci
       00000000000a0000-00000000000affff (prio 2, ram): alias vga.chain4 
@vga.vram 0000000000000000-000000000000ffff
       00000000000a0000-00000000000bffff (prio 1, i/o): vga-lowmem
       00000000000c0000-00000000000dffff (prio 1, rom): pc.rom
       00000000000e0000-00000000000fffff (prio 1, rom): alias isa-bios 
@pc.bios 0000000000020000-000000000003ffff
       00000000fd000000-00000000fdffffff (prio 1, ram): vga.vram


In addition, I do not find the corresponding resource release action for 
cpu->cpu_ases requested in function cpu_address_space_init.

I wonder if there is a leak in the memory space requested here. Maybe 
qemu automatically reclaims memory space

or frees resources somewhere else I didn't find? I thought I'd try 
running the following valgrind to see if I could verify my suspicions.

void cpu_address_space_init(CPUState *cpu, int asidx,
                             const char *prefix, MemoryRegion *mr)
{

...

     if (!cpu->cpu_ases) {
         cpu->cpu_ases = g_new0(CPUAddressSpace, cpu->num_ases);
     }

...

}

>
>> info mtree
>>
>> address-space: cpu-memory-0
>> address-space: cpu-memory-1
>> address-space: memory
>>     0000000000000000-ffffffffffffffff (prio 0, i/o): system
>>       0000000000000000-000000007fffffff (prio 0, ram): alias 
>> ram-below-4g
>> @pc.ram 0000000000000000-000000007fffffff
>>       0000000000000000-ffffffffffffffff (prio -1, i/o): pci
>>         00000000000a0000-00000000000bffff (prio 1, i/o): vga-lowmem
>>
>>
>>   From the above test, you can see whether the address space of cpu1 is
>> residual after a cpu hot swap, and whether it is reasonable?
>
>
> Probably we should teach other archs to destroy that address space as 
> well.
>
> Can we do that from the core, instead of having to do that in each CPU 
> unrealize function?
>
I think it can also be done in the public code flow. Since I refer to 
arm's scheme

(https://lore.kernel.org/all/20200613213629.21984-1-salil.mehta@huawei.com/), 


and arm's patch will be issued soon, I will conduct rebase based on arm 
patch in the future.

Therefore, I would like to see if arm has any good suggestions. If there 
are no good suggestions at this stage,

I think we can shelve this problem for the first time, and I can 
consider not referencing this function for the first time,

and we can submit another patch to solve this problem.

Hi Salil Mehta:

Is the cpu_address_space_destroy function still present in the new patch 
version of arm?

Can we put this function on the public path of cpu destroy?


Thanks,

xianglai.

Re: [PATCH v2 04/10] Introduce the CPU address space destruction function

[lixianglai]

Hi David Hildenbrand:

>
> Hi David Hildenbrand:
>> On 14.09.23 15:00, lixianglai wrote:
>>> Hi David:
>>
>> Hi!
>>
>>>
>>>> On 12.09.23 04:11, xianglai li wrote:
>>>>> Introduce new function to destroy CPU address space resources
>>>>> for cpu hot-(un)plug.
>>>>>
>>>> How do other archs handle that? Or how are they able to get away
>>>> without destroying?
>>>>
>>> They do not remove the cpu address space, taking the X86 
>>> architecture as
>>> an example:
>>>
>>> 1.Start the x86 VM:
>>>
>>> ./qemu-system-x86_64 \
>>> -machine q35  \
>>> -cpu Broadwell-IBRS \
>>> -smp 1,maxcpus=100,sockets=100,cores=1,threads=1 \
>>> -m 4G \
>>> -drive file=~/anolis-8.8.qcow2  \
>>> -serial stdio   \
>>> -monitor telnet:localhost:4498,server,nowait   \
>>> -nographic
>>>
>>> 2.Connect the qemu monitor
>>>
>>> telnet 127.0.0.1 4498
>>>
>>> info mtree
>>>
>>> address-space: cpu-memory-0
>>> address-space: memory
>>>     0000000000000000-ffffffffffffffff (prio 0, i/o): system
>>>       0000000000000000-000000007fffffff (prio 0, ram): alias 
>>> ram-below-4g
>>> @pc.ram 0000000000000000-000000007fffffff
>>>       0000000000000000-ffffffffffffffff (prio -1, i/o): pci
>>>         00000000000a0000-00000000000bffff (prio 1, i/o): vga-lowmem
>>>
>>> 3.Perform cpu hot swap int qemu monitor
>>>
>>> device_add
>>> Broadwell-IBRS-x86_64-cpu,socket-id=1,core-id=0,thread-id=0,id=cpu1
>>> device_del cpu1
>>>
>>
>> Hm, doesn't seem to work for me on upstream QEMU for some reason: 
>> "Error: acpi: device unplug request for not supported device type: 
>> Broadwell-IBRS-x86_64-cpu"
>
First I use qemu tcg, and then the cpu needs to be removed after the 
operating system is booted.

Thanks,

xianglai.


>>
>> What happens if you re-add that CPU? Will we reuse the previous 
>> address space?
>
>
> Here is the memory layout where I inserted cpu1 again. It does not 
> appear that the original address space was reused, and the address 
> space is now duplicated
>
> info mtree
>
> address-space: cpu-memory-0
> address-space: cpu-memory-1
> address-space: cpu-memory-1
> address-space: memory
>   0000000000000000-ffffffffffffffff (prio 0, i/o): system
>     0000000000000000-000000007fffffff (prio 0, ram): alias 
> ram-below-4g @pc.ram 0000000000000000-000000007fffffff
>     0000000000000000-ffffffffffffffff (prio -1, i/o): pci
>       00000000000a0000-00000000000affff (prio 2, ram): alias 
> vga.chain4 @vga.vram 0000000000000000-000000000000ffff
>       00000000000a0000-00000000000bffff (prio 1, i/o): vga-lowmem
>       00000000000c0000-00000000000dffff (prio 1, rom): pc.rom
>       00000000000e0000-00000000000fffff (prio 1, rom): alias isa-bios 
> @pc.bios 0000000000020000-000000000003ffff
>       00000000fd000000-00000000fdffffff (prio 1, ram): vga.vram
>
>
> In addition, I do not find the corresponding resource release action 
> for cpu->cpu_ases requested in function cpu_address_space_init.
>
> I wonder if there is a leak in the memory space requested here. Maybe 
> qemu automatically reclaims memory space
>
> or frees resources somewhere else I didn't find? I thought I'd try 
> running the following valgrind to see if I could verify my suspicions.
>
> void cpu_address_space_init(CPUState *cpu, int asidx,
>                             const char *prefix, MemoryRegion *mr)
> {
>
> ...
>
>     if (!cpu->cpu_ases) {
>         cpu->cpu_ases = g_new0(CPUAddressSpace, cpu->num_ases);
>     }
>
> ...
>
> }
>
>>
>>> info mtree
>>>
>>> address-space: cpu-memory-0
>>> address-space: cpu-memory-1
>>> address-space: memory
>>>     0000000000000000-ffffffffffffffff (prio 0, i/o): system
>>>       0000000000000000-000000007fffffff (prio 0, ram): alias 
>>> ram-below-4g
>>> @pc.ram 0000000000000000-000000007fffffff
>>>       0000000000000000-ffffffffffffffff (prio -1, i/o): pci
>>>         00000000000a0000-00000000000bffff (prio 1, i/o): vga-lowmem
>>>
>>>
>>>   From the above test, you can see whether the address space of cpu1 is
>>> residual after a cpu hot swap, and whether it is reasonable?
>>
>>
>> Probably we should teach other archs to destroy that address space as 
>> well.
>>
>> Can we do that from the core, instead of having to do that in each 
>> CPU unrealize function?
>>
> I think it can also be done in the public code flow. Since I refer to 
> arm's scheme
>
> (https://lore.kernel.org/all/20200613213629.21984-1-salil.mehta@huawei.com/), 
>
>
> and arm's patch will be issued soon, I will conduct rebase based on 
> arm patch in the future.
>
> Therefore, I would like to see if arm has any good suggestions. If 
> there are no good suggestions at this stage,
>
> I think we can shelve this problem for the first time, and I can 
> consider not referencing this function for the first time,
>
> and we can submit another patch to solve this problem.
>
> Hi Salil Mehta:
>
> Is the cpu_address_space_destroy function still present in the new 
> patch version of arm?
>
> Can we put this function on the public path of cpu destroy?
>
>
> Thanks,
>
> xianglai.
>

Re: [PATCH v2 04/10] Introduce the CPU address space destruction function

[David Hildenbrand]

On 15.09.23 04:53, lixianglai wrote:
> Hi David Hildenbrand:
> 
>>
>> Hi David Hildenbrand:
>>> On 14.09.23 15:00, lixianglai wrote:
>>>> Hi David:
>>>
>>> Hi!
>>>
>>>>
>>>>> On 12.09.23 04:11, xianglai li wrote:
>>>>>> Introduce new function to destroy CPU address space resources
>>>>>> for cpu hot-(un)plug.
>>>>>>
>>>>> How do other archs handle that? Or how are they able to get away
>>>>> without destroying?
>>>>>
>>>> They do not remove the cpu address space, taking the X86
>>>> architecture as
>>>> an example:
>>>>
>>>> 1.Start the x86 VM:
>>>>
>>>> ./qemu-system-x86_64 \
>>>> -machine q35  \
>>>> -cpu Broadwell-IBRS \
>>>> -smp 1,maxcpus=100,sockets=100,cores=1,threads=1 \
>>>> -m 4G \
>>>> -drive file=~/anolis-8.8.qcow2  \
>>>> -serial stdio   \
>>>> -monitor telnet:localhost:4498,server,nowait   \
>>>> -nographic
>>>>
>>>> 2.Connect the qemu monitor
>>>>
>>>> telnet 127.0.0.1 4498
>>>>
>>>> info mtree
>>>>
>>>> address-space: cpu-memory-0
>>>> address-space: memory
>>>>      0000000000000000-ffffffffffffffff (prio 0, i/o): system
>>>>        0000000000000000-000000007fffffff (prio 0, ram): alias
>>>> ram-below-4g
>>>> @pc.ram 0000000000000000-000000007fffffff
>>>>        0000000000000000-ffffffffffffffff (prio -1, i/o): pci
>>>>          00000000000a0000-00000000000bffff (prio 1, i/o): vga-lowmem
>>>>
>>>> 3.Perform cpu hot swap int qemu monitor
>>>>
>>>> device_add
>>>> Broadwell-IBRS-x86_64-cpu,socket-id=1,core-id=0,thread-id=0,id=cpu1
>>>> device_del cpu1
>>>>
>>>
>>> Hm, doesn't seem to work for me on upstream QEMU for some reason:
>>> "Error: acpi: device unplug request for not supported device type:
>>> Broadwell-IBRS-x86_64-cpu"
>>
> First I use qemu tcg, and then the cpu needs to be removed after the
> operating system is booted.

Ah, the last thing is the important bit. I can reproduce this with KVM 
easily.

Doing it a couple of times

address-space: cpu-memory-0
address-space: cpu-memory-1
address-space: cpu-memory-1
address-space: cpu-memory-1
address-space: cpu-memory-1
address-space: cpu-memory-1
address-space: cpu-memory-1
address-space: cpu-memory-1
address-space: cpu-memory-1
address-space: cpu-memory-1
address-space: cpu-memory-1
address-space: cpu-memory-1
address-space: cpu-memory-1
address-space: cpu-memory-1
address-space: cpu-memory-1
address-space: cpu-memory-1
address-space: cpu-memory-1
address-space: cpu-memory-1
address-space: cpu-memory-1
address-space: cpu-memory-1

Looks like a resource/memory leak.

[...]

>> I think it can also be done in the public code flow. Since I refer to
>> arm's scheme
>>
>> (https://lore.kernel.org/all/20200613213629.21984-1-salil.mehta@huawei.com/),
>>
>>
>> and arm's patch will be issued soon, I will conduct rebase based on
>> arm patch in the future.
>>
>> Therefore, I would like to see if arm has any good suggestions. If
>> there are no good suggestions at this stage,
>>
>> I think we can shelve this problem for the first time, and I can
>> consider not referencing this function for the first time,
>>
>> and we can submit another patch to solve this problem.
>>
>> Hi Salil Mehta:
>>
>> Is the cpu_address_space_destroy function still present in the new
>> patch version of arm?
>>
>> Can we put this function on the public path of cpu destroy?

Looks like this has to be fixed for all archs that support VCPU unplug.

The CPU implementation end up call qemu_init_vcpu() in their realize 
function; there should be something like qemu_destroy_vcpu() on the 
unrealize path that takes care of undoing any cpu_address_space_init().

We seem to have cpu_common_unrealizefn()->cpu_exec_unrealizefn() but 
that doesn't take care of address spaces.

Also, in qemu_init_vcpu() we do a cpus_accel->create_vcpu_thread(cpu). 
I'm, curious if we destroy that thread somehow.

-- 
Cheers,

David / dhildenb

Re: [PATCH v2 04/10] Introduce the CPU address space destruction function

[lixianglai]

Hi David :
> On 15.09.23 04:53, lixianglai wrote:
>> Hi David Hildenbrand:
>>
>>>
>>> Hi David Hildenbrand:
>>>> On 14.09.23 15:00, lixianglai wrote:
>>>>> Hi David:
>>>>
>>>> Hi!
>>>>
>>>>>
>>>>>> On 12.09.23 04:11, xianglai li wrote:
>>>>>>> Introduce new function to destroy CPU address space resources
>>>>>>> for cpu hot-(un)plug.
>>>>>>>
>>>>>> How do other archs handle that? Or how are they able to get away
>>>>>> without destroying?
>>>>>>
>>>>> They do not remove the cpu address space, taking the X86
>>>>> architecture as
>>>>> an example:
>>>>>
>>>>> 1.Start the x86 VM:
>>>>>
>>>>> ./qemu-system-x86_64 \
>>>>> -machine q35  \
>>>>> -cpu Broadwell-IBRS \
>>>>> -smp 1,maxcpus=100,sockets=100,cores=1,threads=1 \
>>>>> -m 4G \
>>>>> -drive file=~/anolis-8.8.qcow2  \
>>>>> -serial stdio   \
>>>>> -monitor telnet:localhost:4498,server,nowait   \
>>>>> -nographic
>>>>>
>>>>> 2.Connect the qemu monitor
>>>>>
>>>>> telnet 127.0.0.1 4498
>>>>>
>>>>> info mtree
>>>>>
>>>>> address-space: cpu-memory-0
>>>>> address-space: memory
>>>>>      0000000000000000-ffffffffffffffff (prio 0, i/o): system
>>>>>        0000000000000000-000000007fffffff (prio 0, ram): alias
>>>>> ram-below-4g
>>>>> @pc.ram 0000000000000000-000000007fffffff
>>>>>        0000000000000000-ffffffffffffffff (prio -1, i/o): pci
>>>>>          00000000000a0000-00000000000bffff (prio 1, i/o): vga-lowmem
>>>>>
>>>>> 3.Perform cpu hot swap int qemu monitor
>>>>>
>>>>> device_add
>>>>> Broadwell-IBRS-x86_64-cpu,socket-id=1,core-id=0,thread-id=0,id=cpu1
>>>>> device_del cpu1
>>>>>
>>>>
>>>> Hm, doesn't seem to work for me on upstream QEMU for some reason:
>>>> "Error: acpi: device unplug request for not supported device type:
>>>> Broadwell-IBRS-x86_64-cpu"
>>>
>> First I use qemu tcg, and then the cpu needs to be removed after the
>> operating system is booted.
>
> Ah, the last thing is the important bit. I can reproduce this with KVM 
> easily.
>
> Doing it a couple of times
>
> address-space: cpu-memory-0
> address-space: cpu-memory-1
> address-space: cpu-memory-1
> address-space: cpu-memory-1
> address-space: cpu-memory-1
> address-space: cpu-memory-1
> address-space: cpu-memory-1
> address-space: cpu-memory-1
> address-space: cpu-memory-1
> address-space: cpu-memory-1
> address-space: cpu-memory-1
> address-space: cpu-memory-1
> address-space: cpu-memory-1
> address-space: cpu-memory-1
> address-space: cpu-memory-1
> address-space: cpu-memory-1
> address-space: cpu-memory-1
> address-space: cpu-memory-1
> address-space: cpu-memory-1
> address-space: cpu-memory-1
>
> Looks like a resource/memory leak.
>
> [...]
>
>>> I think it can also be done in the public code flow. Since I refer to
>>> arm's scheme
>>>
>>> (https://lore.kernel.org/all/20200613213629.21984-1-salil.mehta@huawei.com/), 
>>>
>>>
>>>
>>> and arm's patch will be issued soon, I will conduct rebase based on
>>> arm patch in the future.
>>>
>>> Therefore, I would like to see if arm has any good suggestions. If
>>> there are no good suggestions at this stage,
>>>
>>> I think we can shelve this problem for the first time, and I can
>>> consider not referencing this function for the first time,
>>>
>>> and we can submit another patch to solve this problem.
>>>
>>> Hi Salil Mehta:
>>>
>>> Is the cpu_address_space_destroy function still present in the new
>>> patch version of arm?
>>>
>>> Can we put this function on the public path of cpu destroy?
>
> Looks like this has to be fixed for all archs that support VCPU unplug.
>
> The CPU implementation end up call qemu_init_vcpu() in their realize 
> function; there should be something like qemu_destroy_vcpu() on the 
> unrealize path that takes care of undoing any cpu_address_space_init().
>
> We seem to have cpu_common_unrealizefn()->cpu_exec_unrealizefn() but 
> that doesn't take care of address spaces.
>
> Also, in qemu_init_vcpu() we do a cpus_accel->create_vcpu_thread(cpu). 
> I'm, curious if we destroy that thread somehow.
>

Although there is no visible destory of the vcpu thread, no trace of the 
thread was found when debugging through gdb,

it should be the loop in the thread ended the thread naturally exited.

Thanks,

xianglai.

Re: [PATCH v2 04/10] Introduce the CPU address space destruction function

[Philippe Mathieu-Daudé]

On 15/9/23 10:07, David Hildenbrand wrote:

> The CPU implementation end up call qemu_init_vcpu() in their realize 
> function; there should be something like qemu_destroy_vcpu() on the 
> unrealize path that takes care of undoing any cpu_address_space_init().
> 
> We seem to have cpu_common_unrealizefn()->cpu_exec_unrealizefn() but 
> that doesn't take care of address spaces.
> 
> Also, in qemu_init_vcpu() we do a cpus_accel->create_vcpu_thread(cpu). 
> I'm, curious if we destroy that thread somehow.

Implicitly in cpu_remove_sync().

Re: [PATCH v2 04/10] Introduce the CPU address space destruction function

[David Hildenbrand]

On 15.09.23 16:19, Philippe Mathieu-Daudé wrote:
> On 15/9/23 10:07, David Hildenbrand wrote:
> 
>> The CPU implementation end up call qemu_init_vcpu() in their realize
>> function; there should be something like qemu_destroy_vcpu() on the
>> unrealize path that takes care of undoing any cpu_address_space_init().
>>
>> We seem to have cpu_common_unrealizefn()->cpu_exec_unrealizefn() but
>> that doesn't take care of address spaces.
>>
>> Also, in qemu_init_vcpu() we do a cpus_accel->create_vcpu_thread(cpu).
>> I'm, curious if we destroy that thread somehow.
> 
> Implicitly in cpu_remove_sync().

Ah, indeed. there is the qemu_thread_join().

-- 
Cheers,

David / dhildenb

RE: [PATCH v2 04/10] Introduce the CPU address space destruction function

[Salil Mehta via]

> From: David Hildenbrand <david@redhat.com>
> Sent: Friday, September 15, 2023 4:22 PM
> To: Philippe Mathieu-Daudé <philmd@linaro.org>; lixianglai
> <lixianglai@loongson.cn>; qemu-devel@nongnu.org; Salil Mehta
> <salil.mehta@huawei.com>
> Cc: Salil Mehta <salil.mehta@opnsrc.net>; Xiaojuan Yang
> <yangxiaojuan@loongson.cn>; Song Gao <gaosong@loongson.cn>; Michael S.
> Tsirkin <mst@redhat.com>; Igor Mammedov <imammedo@redhat.com>; Ani Sinha
> <anisinha@redhat.com>; Paolo Bonzini <pbonzini@redhat.com>; Richard
> Henderson <richard.henderson@linaro.org>; Eduardo Habkost
> <eduardo@habkost.net>; Marcel Apfelbaum <marcel.apfelbaum@gmail.com>;
> wangyanan (Y) <wangyanan55@huawei.com>; Daniel P. Berrangé
> <berrange@redhat.com>; Peter Xu <peterx@redhat.com>; Bibo Mao
> <maobibo@loongson.cn>
> Subject: Re: [PATCH v2 04/10] Introduce the CPU address space destruction
> function
> 
> On 15.09.23 16:19, Philippe Mathieu-Daudé wrote:
> > On 15/9/23 10:07, David Hildenbrand wrote:
> >
> >> The CPU implementation end up call qemu_init_vcpu() in their realize
> >> function; there should be something like qemu_destroy_vcpu() on the
> >> unrealize path that takes care of undoing any cpu_address_space_init().
> >>
> >> We seem to have cpu_common_unrealizefn()->cpu_exec_unrealizefn() but
> >> that doesn't take care of address spaces.
> >>
> >> Also, in qemu_init_vcpu() we do a cpus_accel->create_vcpu_thread(cpu).
> >> I'm, curious if we destroy that thread somehow.
> >
> > Implicitly in cpu_remove_sync().
> 
> Ah, indeed. there is the qemu_thread_join().

Correct. This one. 

For ARM, this was necessary because of the design approach
we have taken as a workaround to the ARM architecture limitations.
Th KVM needs to know everything at the VM initialization time.

Request to help review the patches:
https://lore.kernel.org/qemu-devel/20230926100436.28284-1-salil.mehta@huawei.com/T/#m523b37819c4811c7827333982004e07a1ef03879


Thanks
Salil.

RE: [PATCH v2 04/10] Introduce the CPU address space destruction function

[Salil Mehta via]

Hi David,

> From: David Hildenbrand <david@redhat.com>
> Sent: Friday, September 15, 2023 9:07 AM
> To: lixianglai <lixianglai@loongson.cn>; qemu-devel@nongnu.org; Salil Mehta
> <salil.mehta@huawei.com>
> Cc: Salil Mehta <salil.mehta@opnsrc.net>; Xiaojuan Yang
> <yangxiaojuan@loongson.cn>; Song Gao <gaosong@loongson.cn>; Michael S.
> Tsirkin <mst@redhat.com>; Igor Mammedov <imammedo@redhat.com>; Ani Sinha
> <anisinha@redhat.com>; Paolo Bonzini <pbonzini@redhat.com>; Richard
> Henderson <richard.henderson@linaro.org>; Eduardo Habkost
> <eduardo@habkost.net>; Marcel Apfelbaum <marcel.apfelbaum@gmail.com>;
> Philippe Mathieu-Daudé <philmd@linaro.org>; wangyanan (Y)
> <wangyanan55@huawei.com>; Daniel P. Berrangé <berrange@redhat.com>; Peter
> Xu <peterx@redhat.com>; Bibo Mao <maobibo@loongson.cn>
> Subject: Re: [PATCH v2 04/10] Introduce the CPU address space destruction
> function
> 
> On 15.09.23 04:53, lixianglai wrote:
> > Hi David Hildenbrand:
> >
> >>
> >> Hi David Hildenbrand:
> >>> On 14.09.23 15:00, lixianglai wrote:
> >>>> Hi David:
> >>>
> >>> Hi!
> >>>
> >>>>
> >>>>> On 12.09.23 04:11, xianglai li wrote:
> >>>>>> Introduce new function to destroy CPU address space resources
> >>>>>> for cpu hot-(un)plug.
> >>>>>>
> >>>>> How do other archs handle that? Or how are they able to get away
> >>>>> without destroying?
> >>>>>
> >>>> They do not remove the cpu address space, taking the X86
> >>>> architecture as
> >>>> an example:
> >>>>
> >>>> 1.Start the x86 VM:
> >>>>
> >>>> ./qemu-system-x86_64 \
> >>>> -machine q35  \
> >>>> -cpu Broadwell-IBRS \
> >>>> -smp 1,maxcpus=100,sockets=100,cores=1,threads=1 \
> >>>> -m 4G \
> >>>> -drive file=~/anolis-8.8.qcow2  \
> >>>> -serial stdio   \
> >>>> -monitor telnet:localhost:4498,server,nowait   \
> >>>> -nographic
> >>>>
> >>>> 2.Connect the qemu monitor
> >>>>
> >>>> telnet 127.0.0.1 4498
> >>>>
> >>>> info mtree
> >>>>
> >>>> address-space: cpu-memory-0
> >>>> address-space: memory
> >>>>      0000000000000000-ffffffffffffffff (prio 0, i/o): system
> >>>>        0000000000000000-000000007fffffff (prio 0, ram): alias
> >>>> ram-below-4g
> >>>> @pc.ram 0000000000000000-000000007fffffff
> >>>>        0000000000000000-ffffffffffffffff (prio -1, i/o): pci
> >>>>          00000000000a0000-00000000000bffff (prio 1, i/o): vga-lowmem
> >>>>
> >>>> 3.Perform cpu hot swap int qemu monitor
> >>>>
> >>>> device_add
> >>>> Broadwell-IBRS-x86_64-cpu,socket-id=1,core-id=0,thread-id=0,id=cpu1
> >>>> device_del cpu1
> >>>>
> >>>
> >>> Hm, doesn't seem to work for me on upstream QEMU for some reason:
> >>> "Error: acpi: device unplug request for not supported device type:
> >>> Broadwell-IBRS-x86_64-cpu"
> >>
> > First I use qemu tcg, and then the cpu needs to be removed after the
> > operating system is booted.
> 
> Ah, the last thing is the important bit. I can reproduce this with KVM
> easily.
> 
> Doing it a couple of times
> 
> address-space: cpu-memory-0
> address-space: cpu-memory-1
> address-space: cpu-memory-1
> address-space: cpu-memory-1
> address-space: cpu-memory-1
> address-space: cpu-memory-1
> address-space: cpu-memory-1
> address-space: cpu-memory-1
> address-space: cpu-memory-1
> address-space: cpu-memory-1
> address-space: cpu-memory-1
> address-space: cpu-memory-1
> address-space: cpu-memory-1
> address-space: cpu-memory-1
> address-space: cpu-memory-1
> address-space: cpu-memory-1
> address-space: cpu-memory-1
> address-space: cpu-memory-1
> address-space: cpu-memory-1
> address-space: cpu-memory-1
> 
> Looks like a resource/memory leak.

Yes, there was. Thanks for identifying it. I have fixed in the
latest RFC V2. Please check here:

https://lore.kernel.org/qemu-devel/20230926100436.28284-1-salil.mehta@huawei.com/T/#m5f5ae40b091d69d01012880d7500d96874a9d39c

I have tested and AddressSpace comes and goes away cleanly
on CPU hot(un)plug action.



[...]


> >> Hi Salil Mehta:
> >>
> >> Is the cpu_address_space_destroy function still present in the new
> >> patch version of arm?
> >>
> >> Can we put this function on the public path of cpu destroy?
> 
> Looks like this has to be fixed for all archs that support VCPU unplug.
> 
> The CPU implementation end up call qemu_init_vcpu() in their realize
> function; there should be something like qemu_destroy_vcpu() on the
> unrealize path that takes care of undoing any cpu_address_space_init().
> 
> We seem to have cpu_common_unrealizefn()->cpu_exec_unrealizefn() but
> that doesn't take care of address spaces.

Yes, the current concept can be extended

> 
> Also, in qemu_init_vcpu() we do a cpus_accel->create_vcpu_thread(cpu).
> I'm, curious if we destroy that thread somehow.


Yes we do. In ARM RFC, this happens after CPU has been unplugged and
CPU is being destroyed and unparented (all of this happens in context
ACPI _EJx method evaluated by OSPM). This eventually leads to
architecture specific CPU unrealize and call to cpu_remove_sync().


Thanks
Salil.

RE: [PATCH v2 04/10] Introduce the CPU address space destruction function

[Salil Mehta via]

> From: Salil Mehta
> Sent: Tuesday, September 26, 2023 12:21 PM
> To: 'David Hildenbrand' <david@redhat.com>; lixianglai
> <lixianglai@loongson.cn>; qemu-devel@nongnu.org
> Cc: Salil Mehta <salil.mehta@opnsrc.net>; Xiaojuan Yang
> <yangxiaojuan@loongson.cn>; Song Gao <gaosong@loongson.cn>; Michael S.
> Tsirkin <mst@redhat.com>; Igor Mammedov <imammedo@redhat.com>; Ani Sinha
> <anisinha@redhat.com>; Paolo Bonzini <pbonzini@redhat.com>; Richard
> Henderson <richard.henderson@linaro.org>; Eduardo Habkost
> <eduardo@habkost.net>; Marcel Apfelbaum <marcel.apfelbaum@gmail.com>;
> Philippe Mathieu-Daudé <philmd@linaro.org>; wangyanan (Y)
> <wangyanan55@huawei.com>; Daniel P. Berrangé <berrange@redhat.com>; Peter
> Xu <peterx@redhat.com>; Bibo Mao <maobibo@loongson.cn>
> Subject: RE: [PATCH v2 04/10] Introduce the CPU address space destruction
> function
> 
> Hi David,
> 
> > From: David Hildenbrand <david@redhat.com>
> > Sent: Friday, September 15, 2023 9:07 AM
> > To: lixianglai <lixianglai@loongson.cn>; qemu-devel@nongnu.org; Salil
> Mehta
> > <salil.mehta@huawei.com>
> > Cc: Salil Mehta <salil.mehta@opnsrc.net>; Xiaojuan Yang
> > <yangxiaojuan@loongson.cn>; Song Gao <gaosong@loongson.cn>; Michael S.
> > Tsirkin <mst@redhat.com>; Igor Mammedov <imammedo@redhat.com>; Ani Sinha
> > <anisinha@redhat.com>; Paolo Bonzini <pbonzini@redhat.com>; Richard
> > Henderson <richard.henderson@linaro.org>; Eduardo Habkost
> > <eduardo@habkost.net>; Marcel Apfelbaum <marcel.apfelbaum@gmail.com>;
> > Philippe Mathieu-Daudé <philmd@linaro.org>; wangyanan (Y)
> > <wangyanan55@huawei.com>; Daniel P. Berrangé <berrange@redhat.com>; Peter
> > Xu <peterx@redhat.com>; Bibo Mao <maobibo@loongson.cn>
> > Subject: Re: [PATCH v2 04/10] Introduce the CPU address space destruction
> > function
> >
> > On 15.09.23 04:53, lixianglai wrote:
> > > Hi David Hildenbrand:
> > >
> > >>
> > >> Hi David Hildenbrand:
> > >>> On 14.09.23 15:00, lixianglai wrote:
> > >>>> Hi David:
> > >>>
> > >>> Hi!
> > >>>
> > >>>>
> > >>>>> On 12.09.23 04:11, xianglai li wrote:
> > >>>>>> Introduce new function to destroy CPU address space resources
> > >>>>>> for cpu hot-(un)plug.
> > >>>>>>
> > >>>>> How do other archs handle that? Or how are they able to get away
> > >>>>> without destroying?
> > >>>>>
> > >>>> They do not remove the cpu address space, taking the X86
> > >>>> architecture as
> > >>>> an example:
> > >>>>
> > >>>> 1.Start the x86 VM:
> > >>>>
> > >>>> ./qemu-system-x86_64 \
> > >>>> -machine q35  \
> > >>>> -cpu Broadwell-IBRS \
> > >>>> -smp 1,maxcpus=100,sockets=100,cores=1,threads=1 \
> > >>>> -m 4G \
> > >>>> -drive file=~/anolis-8.8.qcow2  \
> > >>>> -serial stdio   \
> > >>>> -monitor telnet:localhost:4498,server,nowait   \
> > >>>> -nographic
> > >>>>
> > >>>> 2.Connect the qemu monitor
> > >>>>
> > >>>> telnet 127.0.0.1 4498
> > >>>>
> > >>>> info mtree
> > >>>>
> > >>>> address-space: cpu-memory-0
> > >>>> address-space: memory
> > >>>>      0000000000000000-ffffffffffffffff (prio 0, i/o): system
> > >>>>        0000000000000000-000000007fffffff (prio 0, ram): alias
> > >>>> ram-below-4g
> > >>>> @pc.ram 0000000000000000-000000007fffffff
> > >>>>        0000000000000000-ffffffffffffffff (prio -1, i/o): pci
> > >>>>          00000000000a0000-00000000000bffff (prio 1, i/o): vga-lowmem
> > >>>>
> > >>>> 3.Perform cpu hot swap int qemu monitor
> > >>>>
> > >>>> device_add
> > >>>> Broadwell-IBRS-x86_64-cpu,socket-id=1,core-id=0,thread-id=0,id=cpu1
> > >>>> device_del cpu1
> > >>>>
> > >>>
> > >>> Hm, doesn't seem to work for me on upstream QEMU for some reason:
> > >>> "Error: acpi: device unplug request for not supported device type:
> > >>> Broadwell-IBRS-x86_64-cpu"
> > >>
> > > First I use qemu tcg, and then the cpu needs to be removed after the
> > > operating system is booted.
> >
> > Ah, the last thing is the important bit. I can reproduce this with KVM
> > easily.
> >
> > Doing it a couple of times
> >
> > address-space: cpu-memory-0
> > address-space: cpu-memory-1
> > address-space: cpu-memory-1
> > address-space: cpu-memory-1
> > address-space: cpu-memory-1
> > address-space: cpu-memory-1
> > address-space: cpu-memory-1
> > address-space: cpu-memory-1
> > address-space: cpu-memory-1
> > address-space: cpu-memory-1
> > address-space: cpu-memory-1
> > address-space: cpu-memory-1
> > address-space: cpu-memory-1
> > address-space: cpu-memory-1
> > address-space: cpu-memory-1
> > address-space: cpu-memory-1
> > address-space: cpu-memory-1
> > address-space: cpu-memory-1
> > address-space: cpu-memory-1
> > address-space: cpu-memory-1
> >
> > Looks like a resource/memory leak.
> 
> Yes, there was. Thanks for identifying it. I have fixed in the
> latest RFC V2. Please check here:
> 
> https://lore.kernel.org/qemu-devel/20230926100436.28284-1-
> salil.mehta@huawei.com/T/#m5f5ae40b091d69d01012880d7500d96874a9d39c
> 
> I have tested and AddressSpace comes and goes away cleanly
> on CPU hot(un)plug action.

Hi David/Xianglai,
Are you okay if I put Reported-by and give reference to this
conversation?

Many thanks
Salil

Re: [PATCH v2 04/10] Introduce the CPU address space destruction function

[David Hildenbrand]

On 26.09.23 13:55, Salil Mehta wrote:
>> From: Salil Mehta
>> Sent: Tuesday, September 26, 2023 12:21 PM
>> To: 'David Hildenbrand' <david@redhat.com>; lixianglai
>> <lixianglai@loongson.cn>; qemu-devel@nongnu.org
>> Cc: Salil Mehta <salil.mehta@opnsrc.net>; Xiaojuan Yang
>> <yangxiaojuan@loongson.cn>; Song Gao <gaosong@loongson.cn>; Michael S.
>> Tsirkin <mst@redhat.com>; Igor Mammedov <imammedo@redhat.com>; Ani Sinha
>> <anisinha@redhat.com>; Paolo Bonzini <pbonzini@redhat.com>; Richard
>> Henderson <richard.henderson@linaro.org>; Eduardo Habkost
>> <eduardo@habkost.net>; Marcel Apfelbaum <marcel.apfelbaum@gmail.com>;
>> Philippe Mathieu-Daudé <philmd@linaro.org>; wangyanan (Y)
>> <wangyanan55@huawei.com>; Daniel P. Berrangé <berrange@redhat.com>; Peter
>> Xu <peterx@redhat.com>; Bibo Mao <maobibo@loongson.cn>
>> Subject: RE: [PATCH v2 04/10] Introduce the CPU address space destruction
>> function
>>
>> Hi David,
>>
>>> From: David Hildenbrand <david@redhat.com>
>>> Sent: Friday, September 15, 2023 9:07 AM
>>> To: lixianglai <lixianglai@loongson.cn>; qemu-devel@nongnu.org; Salil
>> Mehta
>>> <salil.mehta@huawei.com>
>>> Cc: Salil Mehta <salil.mehta@opnsrc.net>; Xiaojuan Yang
>>> <yangxiaojuan@loongson.cn>; Song Gao <gaosong@loongson.cn>; Michael S.
>>> Tsirkin <mst@redhat.com>; Igor Mammedov <imammedo@redhat.com>; Ani Sinha
>>> <anisinha@redhat.com>; Paolo Bonzini <pbonzini@redhat.com>; Richard
>>> Henderson <richard.henderson@linaro.org>; Eduardo Habkost
>>> <eduardo@habkost.net>; Marcel Apfelbaum <marcel.apfelbaum@gmail.com>;
>>> Philippe Mathieu-Daudé <philmd@linaro.org>; wangyanan (Y)
>>> <wangyanan55@huawei.com>; Daniel P. Berrangé <berrange@redhat.com>; Peter
>>> Xu <peterx@redhat.com>; Bibo Mao <maobibo@loongson.cn>
>>> Subject: Re: [PATCH v2 04/10] Introduce the CPU address space destruction
>>> function
>>>
>>> On 15.09.23 04:53, lixianglai wrote:
>>>> Hi David Hildenbrand:
>>>>
>>>>>
>>>>> Hi David Hildenbrand:
>>>>>> On 14.09.23 15:00, lixianglai wrote:
>>>>>>> Hi David:
>>>>>>
>>>>>> Hi!
>>>>>>
>>>>>>>
>>>>>>>> On 12.09.23 04:11, xianglai li wrote:
>>>>>>>>> Introduce new function to destroy CPU address space resources
>>>>>>>>> for cpu hot-(un)plug.
>>>>>>>>>
>>>>>>>> How do other archs handle that? Or how are they able to get away
>>>>>>>> without destroying?
>>>>>>>>
>>>>>>> They do not remove the cpu address space, taking the X86
>>>>>>> architecture as
>>>>>>> an example:
>>>>>>>
>>>>>>> 1.Start the x86 VM:
>>>>>>>
>>>>>>> ./qemu-system-x86_64 \
>>>>>>> -machine q35  \
>>>>>>> -cpu Broadwell-IBRS \
>>>>>>> -smp 1,maxcpus=100,sockets=100,cores=1,threads=1 \
>>>>>>> -m 4G \
>>>>>>> -drive file=~/anolis-8.8.qcow2  \
>>>>>>> -serial stdio   \
>>>>>>> -monitor telnet:localhost:4498,server,nowait   \
>>>>>>> -nographic
>>>>>>>
>>>>>>> 2.Connect the qemu monitor
>>>>>>>
>>>>>>> telnet 127.0.0.1 4498
>>>>>>>
>>>>>>> info mtree
>>>>>>>
>>>>>>> address-space: cpu-memory-0
>>>>>>> address-space: memory
>>>>>>>       0000000000000000-ffffffffffffffff (prio 0, i/o): system
>>>>>>>         0000000000000000-000000007fffffff (prio 0, ram): alias
>>>>>>> ram-below-4g
>>>>>>> @pc.ram 0000000000000000-000000007fffffff
>>>>>>>         0000000000000000-ffffffffffffffff (prio -1, i/o): pci
>>>>>>>           00000000000a0000-00000000000bffff (prio 1, i/o): vga-lowmem
>>>>>>>
>>>>>>> 3.Perform cpu hot swap int qemu monitor
>>>>>>>
>>>>>>> device_add
>>>>>>> Broadwell-IBRS-x86_64-cpu,socket-id=1,core-id=0,thread-id=0,id=cpu1
>>>>>>> device_del cpu1
>>>>>>>
>>>>>>
>>>>>> Hm, doesn't seem to work for me on upstream QEMU for some reason:
>>>>>> "Error: acpi: device unplug request for not supported device type:
>>>>>> Broadwell-IBRS-x86_64-cpu"
>>>>>
>>>> First I use qemu tcg, and then the cpu needs to be removed after the
>>>> operating system is booted.
>>>
>>> Ah, the last thing is the important bit. I can reproduce this with KVM
>>> easily.
>>>
>>> Doing it a couple of times
>>>
>>> address-space: cpu-memory-0
>>> address-space: cpu-memory-1
>>> address-space: cpu-memory-1
>>> address-space: cpu-memory-1
>>> address-space: cpu-memory-1
>>> address-space: cpu-memory-1
>>> address-space: cpu-memory-1
>>> address-space: cpu-memory-1
>>> address-space: cpu-memory-1
>>> address-space: cpu-memory-1
>>> address-space: cpu-memory-1
>>> address-space: cpu-memory-1
>>> address-space: cpu-memory-1
>>> address-space: cpu-memory-1
>>> address-space: cpu-memory-1
>>> address-space: cpu-memory-1
>>> address-space: cpu-memory-1
>>> address-space: cpu-memory-1
>>> address-space: cpu-memory-1
>>> address-space: cpu-memory-1
>>>
>>> Looks like a resource/memory leak.
>>
>> Yes, there was. Thanks for identifying it. I have fixed in the
>> latest RFC V2. Please check here:
>>
>> https://lore.kernel.org/qemu-devel/20230926100436.28284-1-
>> salil.mehta@huawei.com/T/#m5f5ae40b091d69d01012880d7500d96874a9d39c
>>
>> I have tested and AddressSpace comes and goes away cleanly
>> on CPU hot(un)plug action.
> 
> Hi David/Xianglai,
> Are you okay if I put Reported-by and give reference to this
> conversation?

Yes. And ideally, send the fixes separately from the other arm patches.

-- 
Cheers,

David / dhildenb

RE: [PATCH v2 04/10] Introduce the CPU address space destruction function

[Salil Mehta via]

> From: David Hildenbrand <david@redhat.com>
> Sent: Tuesday, September 26, 2023 1:24 PM
> 
> On 26.09.23 13:55, Salil Mehta wrote:
> >> From: Salil Mehta
> >> Sent: Tuesday, September 26, 2023 12:21 PM
> >> To: 'David Hildenbrand' <david@redhat.com>; lixianglai
> >> <lixianglai@loongson.cn>; qemu-devel@nongnu.org
> >> Cc: Salil Mehta <salil.mehta@opnsrc.net>; Xiaojuan Yang
> >> <yangxiaojuan@loongson.cn>; Song Gao <gaosong@loongson.cn>; Michael S.
> >> Tsirkin <mst@redhat.com>; Igor Mammedov <imammedo@redhat.com>; Ani Sinha
> >> <anisinha@redhat.com>; Paolo Bonzini <pbonzini@redhat.com>; Richard
> >> Henderson <richard.henderson@linaro.org>; Eduardo Habkost
> >> <eduardo@habkost.net>; Marcel Apfelbaum <marcel.apfelbaum@gmail.com>;
> >> Philippe Mathieu-Daudé <philmd@linaro.org>; wangyanan (Y)
> >> <wangyanan55@huawei.com>; Daniel P. Berrangé <berrange@redhat.com>;
> Peter
> >> Xu <peterx@redhat.com>; Bibo Mao <maobibo@loongson.cn>
> >> Subject: RE: [PATCH v2 04/10] Introduce the CPU address space
> destruction
> >> function
> >>
> >> Hi David,
> >>
> >>> From: David Hildenbrand <david@redhat.com>
> >>> Sent: Friday, September 15, 2023 9:07 AM
> >>> To: lixianglai <lixianglai@loongson.cn>; qemu-devel@nongnu.org; Salil
> >> Mehta
> >>> <salil.mehta@huawei.com>
> >>> Cc: Salil Mehta <salil.mehta@opnsrc.net>; Xiaojuan Yang
> >>> <yangxiaojuan@loongson.cn>; Song Gao <gaosong@loongson.cn>; Michael S.
> >>> Tsirkin <mst@redhat.com>; Igor Mammedov <imammedo@redhat.com>; Ani
> Sinha
> >>> <anisinha@redhat.com>; Paolo Bonzini <pbonzini@redhat.com>; Richard
> >>> Henderson <richard.henderson@linaro.org>; Eduardo Habkost
> >>> <eduardo@habkost.net>; Marcel Apfelbaum <marcel.apfelbaum@gmail.com>;
> >>> Philippe Mathieu-Daudé <philmd@linaro.org>; wangyanan (Y)
> >>> <wangyanan55@huawei.com>; Daniel P. Berrangé <berrange@redhat.com>;
> Peter
> >>> Xu <peterx@redhat.com>; Bibo Mao <maobibo@loongson.cn>
> >>> Subject: Re: [PATCH v2 04/10] Introduce the CPU address space
> destruction
> >>> function
> >>>
> >>> On 15.09.23 04:53, lixianglai wrote:
> >>>> Hi David Hildenbrand:
> >>>>
> >>>>>
> >>>>> Hi David Hildenbrand:
> >>>>>> On 14.09.23 15:00, lixianglai wrote:
> >>>>>>> Hi David:
> >>>>>>
> >>>>>> Hi!
> >>>>>>
> >>>>>>>
> >>>>>>>> On 12.09.23 04:11, xianglai li wrote:
> >>>>>>>>> Introduce new function to destroy CPU address space resources
> >>>>>>>>> for cpu hot-(un)plug.
> >>>>>>>>>
> >>>>>>>> How do other archs handle that? Or how are they able to get away
> >>>>>>>> without destroying?
> >>>>>>>>
> >>>>>>> They do not remove the cpu address space, taking the X86
> >>>>>>> architecture as
> >>>>>>> an example:
> >>>>>>>
> >>>>>>> 1.Start the x86 VM:
> >>>>>>>
> >>>>>>> ./qemu-system-x86_64 \
> >>>>>>> -machine q35  \
> >>>>>>> -cpu Broadwell-IBRS \
> >>>>>>> -smp 1,maxcpus=100,sockets=100,cores=1,threads=1 \
> >>>>>>> -m 4G \
> >>>>>>> -drive file=~/anolis-8.8.qcow2  \
> >>>>>>> -serial stdio   \
> >>>>>>> -monitor telnet:localhost:4498,server,nowait   \
> >>>>>>> -nographic
> >>>>>>>
> >>>>>>> 2.Connect the qemu monitor
> >>>>>>>
> >>>>>>> telnet 127.0.0.1 4498
> >>>>>>>
> >>>>>>> info mtree
> >>>>>>>
> >>>>>>> address-space: cpu-memory-0
> >>>>>>> address-space: memory
> >>>>>>>       0000000000000000-ffffffffffffffff (prio 0, i/o): system
> >>>>>>>         0000000000000000-000000007fffffff (prio 0, ram): alias
> >>>>>>> ram-below-4g
> >>>>>>> @pc.ram 0000000000000000-000000007fffffff
> >>>>>>>         0000000000000000-ffffffffffffffff (prio -1, i/o): pci
> >>>>>>>           00000000000a0000-00000000000bffff (prio 1, i/o): vga-
> lowmem
> >>>>>>>
> >>>>>>> 3.Perform cpu hot swap int qemu monitor
> >>>>>>>
> >>>>>>> device_add
> >>>>>>> Broadwell-IBRS-x86_64-cpu,socket-id=1,core-id=0,thread-id=0,id=cpu1
> >>>>>>> device_del cpu1
> >>>>>>>
> >>>>>>
> >>>>>> Hm, doesn't seem to work for me on upstream QEMU for some reason:
> >>>>>> "Error: acpi: device unplug request for not supported device type:
> >>>>>> Broadwell-IBRS-x86_64-cpu"
> >>>>>
> >>>> First I use qemu tcg, and then the cpu needs to be removed after the
> >>>> operating system is booted.
> >>>
> >>> Ah, the last thing is the important bit. I can reproduce this with KVM
> >>> easily.
> >>>
> >>> Doing it a couple of times
> >>>
> >>> address-space: cpu-memory-0
> >>> address-space: cpu-memory-1
> >>> address-space: cpu-memory-1
> >>> address-space: cpu-memory-1
> >>> address-space: cpu-memory-1
> >>> address-space: cpu-memory-1
> >>> address-space: cpu-memory-1
> >>> address-space: cpu-memory-1
> >>> address-space: cpu-memory-1
> >>> address-space: cpu-memory-1
> >>> address-space: cpu-memory-1
> >>> address-space: cpu-memory-1
> >>> address-space: cpu-memory-1
> >>> address-space: cpu-memory-1
> >>> address-space: cpu-memory-1
> >>> address-space: cpu-memory-1
> >>> address-space: cpu-memory-1
> >>> address-space: cpu-memory-1
> >>> address-space: cpu-memory-1
> >>> address-space: cpu-memory-1
> >>>
> >>> Looks like a resource/memory leak.
> >>
> >> Yes, there was. Thanks for identifying it. I have fixed in the
> >> latest RFC V2. Please check here:
> >>
> >> https://lore.kernel.org/qemu-devel/20230926100436.28284-1-
> >> salil.mehta@huawei.com/T/#m5f5ae40b091d69d01012880d7500d96874a9d39c
> >>
> >> I have tested and AddressSpace comes and goes away cleanly
> >> on CPU hot(un)plug action.
> >
> > Hi David/Xianglai,
> > Are you okay if I put Reported-by and give reference to this
> > conversation?
> 
> Yes. And ideally, send the fixes separately from the other arm patches.

ARM Virtual CPU Hotplug support patches are still under review.

Thanks
Salil.

Re: [PATCH v2 04/10] Introduce the CPU address space destruction function

[David Hildenbrand]

On 26.09.23 14:32, Salil Mehta wrote:
>> From: David Hildenbrand <david@redhat.com>
>> Sent: Tuesday, September 26, 2023 1:24 PM
>>
>> On 26.09.23 13:55, Salil Mehta wrote:
>>>> From: Salil Mehta
>>>> Sent: Tuesday, September 26, 2023 12:21 PM
>>>> To: 'David Hildenbrand' <david@redhat.com>; lixianglai
>>>> <lixianglai@loongson.cn>; qemu-devel@nongnu.org
>>>> Cc: Salil Mehta <salil.mehta@opnsrc.net>; Xiaojuan Yang
>>>> <yangxiaojuan@loongson.cn>; Song Gao <gaosong@loongson.cn>; Michael S.
>>>> Tsirkin <mst@redhat.com>; Igor Mammedov <imammedo@redhat.com>; Ani Sinha
>>>> <anisinha@redhat.com>; Paolo Bonzini <pbonzini@redhat.com>; Richard
>>>> Henderson <richard.henderson@linaro.org>; Eduardo Habkost
>>>> <eduardo@habkost.net>; Marcel Apfelbaum <marcel.apfelbaum@gmail.com>;
>>>> Philippe Mathieu-Daudé <philmd@linaro.org>; wangyanan (Y)
>>>> <wangyanan55@huawei.com>; Daniel P. Berrangé <berrange@redhat.com>;
>> Peter
>>>> Xu <peterx@redhat.com>; Bibo Mao <maobibo@loongson.cn>
>>>> Subject: RE: [PATCH v2 04/10] Introduce the CPU address space
>> destruction
>>>> function
>>>>
>>>> Hi David,
>>>>
>>>>> From: David Hildenbrand <david@redhat.com>
>>>>> Sent: Friday, September 15, 2023 9:07 AM
>>>>> To: lixianglai <lixianglai@loongson.cn>; qemu-devel@nongnu.org; Salil
>>>> Mehta
>>>>> <salil.mehta@huawei.com>
>>>>> Cc: Salil Mehta <salil.mehta@opnsrc.net>; Xiaojuan Yang
>>>>> <yangxiaojuan@loongson.cn>; Song Gao <gaosong@loongson.cn>; Michael S.
>>>>> Tsirkin <mst@redhat.com>; Igor Mammedov <imammedo@redhat.com>; Ani
>> Sinha
>>>>> <anisinha@redhat.com>; Paolo Bonzini <pbonzini@redhat.com>; Richard
>>>>> Henderson <richard.henderson@linaro.org>; Eduardo Habkost
>>>>> <eduardo@habkost.net>; Marcel Apfelbaum <marcel.apfelbaum@gmail.com>;
>>>>> Philippe Mathieu-Daudé <philmd@linaro.org>; wangyanan (Y)
>>>>> <wangyanan55@huawei.com>; Daniel P. Berrangé <berrange@redhat.com>;
>> Peter
>>>>> Xu <peterx@redhat.com>; Bibo Mao <maobibo@loongson.cn>
>>>>> Subject: Re: [PATCH v2 04/10] Introduce the CPU address space
>> destruction
>>>>> function
>>>>>
>>>>> On 15.09.23 04:53, lixianglai wrote:
>>>>>> Hi David Hildenbrand:
>>>>>>
>>>>>>>
>>>>>>> Hi David Hildenbrand:
>>>>>>>> On 14.09.23 15:00, lixianglai wrote:
>>>>>>>>> Hi David:
>>>>>>>>
>>>>>>>> Hi!
>>>>>>>>
>>>>>>>>>
>>>>>>>>>> On 12.09.23 04:11, xianglai li wrote:
>>>>>>>>>>> Introduce new function to destroy CPU address space resources
>>>>>>>>>>> for cpu hot-(un)plug.
>>>>>>>>>>>
>>>>>>>>>> How do other archs handle that? Or how are they able to get away
>>>>>>>>>> without destroying?
>>>>>>>>>>
>>>>>>>>> They do not remove the cpu address space, taking the X86
>>>>>>>>> architecture as
>>>>>>>>> an example:
>>>>>>>>>
>>>>>>>>> 1.Start the x86 VM:
>>>>>>>>>
>>>>>>>>> ./qemu-system-x86_64 \
>>>>>>>>> -machine q35  \
>>>>>>>>> -cpu Broadwell-IBRS \
>>>>>>>>> -smp 1,maxcpus=100,sockets=100,cores=1,threads=1 \
>>>>>>>>> -m 4G \
>>>>>>>>> -drive file=~/anolis-8.8.qcow2  \
>>>>>>>>> -serial stdio   \
>>>>>>>>> -monitor telnet:localhost:4498,server,nowait   \
>>>>>>>>> -nographic
>>>>>>>>>
>>>>>>>>> 2.Connect the qemu monitor
>>>>>>>>>
>>>>>>>>> telnet 127.0.0.1 4498
>>>>>>>>>
>>>>>>>>> info mtree
>>>>>>>>>
>>>>>>>>> address-space: cpu-memory-0
>>>>>>>>> address-space: memory
>>>>>>>>>        0000000000000000-ffffffffffffffff (prio 0, i/o): system
>>>>>>>>>          0000000000000000-000000007fffffff (prio 0, ram): alias
>>>>>>>>> ram-below-4g
>>>>>>>>> @pc.ram 0000000000000000-000000007fffffff
>>>>>>>>>          0000000000000000-ffffffffffffffff (prio -1, i/o): pci
>>>>>>>>>            00000000000a0000-00000000000bffff (prio 1, i/o): vga-
>> lowmem
>>>>>>>>>
>>>>>>>>> 3.Perform cpu hot swap int qemu monitor
>>>>>>>>>
>>>>>>>>> device_add
>>>>>>>>> Broadwell-IBRS-x86_64-cpu,socket-id=1,core-id=0,thread-id=0,id=cpu1
>>>>>>>>> device_del cpu1
>>>>>>>>>
>>>>>>>>
>>>>>>>> Hm, doesn't seem to work for me on upstream QEMU for some reason:
>>>>>>>> "Error: acpi: device unplug request for not supported device type:
>>>>>>>> Broadwell-IBRS-x86_64-cpu"
>>>>>>>
>>>>>> First I use qemu tcg, and then the cpu needs to be removed after the
>>>>>> operating system is booted.
>>>>>
>>>>> Ah, the last thing is the important bit. I can reproduce this with KVM
>>>>> easily.
>>>>>
>>>>> Doing it a couple of times
>>>>>
>>>>> address-space: cpu-memory-0
>>>>> address-space: cpu-memory-1
>>>>> address-space: cpu-memory-1
>>>>> address-space: cpu-memory-1
>>>>> address-space: cpu-memory-1
>>>>> address-space: cpu-memory-1
>>>>> address-space: cpu-memory-1
>>>>> address-space: cpu-memory-1
>>>>> address-space: cpu-memory-1
>>>>> address-space: cpu-memory-1
>>>>> address-space: cpu-memory-1
>>>>> address-space: cpu-memory-1
>>>>> address-space: cpu-memory-1
>>>>> address-space: cpu-memory-1
>>>>> address-space: cpu-memory-1
>>>>> address-space: cpu-memory-1
>>>>> address-space: cpu-memory-1
>>>>> address-space: cpu-memory-1
>>>>> address-space: cpu-memory-1
>>>>> address-space: cpu-memory-1
>>>>>
>>>>> Looks like a resource/memory leak.
>>>>
>>>> Yes, there was. Thanks for identifying it. I have fixed in the
>>>> latest RFC V2. Please check here:
>>>>
>>>> https://lore.kernel.org/qemu-devel/20230926100436.28284-1-
>>>> salil.mehta@huawei.com/T/#m5f5ae40b091d69d01012880d7500d96874a9d39c
>>>>
>>>> I have tested and AddressSpace comes and goes away cleanly
>>>> on CPU hot(un)plug action.
>>>
>>> Hi David/Xianglai,
>>> Are you okay if I put Reported-by and give reference to this
>>> conversation?
>>
>> Yes. And ideally, send the fixes separately from the other arm patches.
> 
> ARM Virtual CPU Hotplug support patches are still under review.

The other architectures (as shown, x86 is affected) can be fixed 
independent of that support.

-- 
Cheers,

David / dhildenb

RE: [PATCH v2 04/10] Introduce the CPU address space destruction function

[Salil Mehta via]

> From: David Hildenbrand <david@redhat.com>
> Sent: Tuesday, September 26, 2023 1:37 PM
> To: Salil Mehta <salil.mehta@huawei.com>; lixianglai
> <lixianglai@loongson.cn>; qemu-devel@nongnu.org
> Cc: Salil Mehta <salil.mehta@opnsrc.net>; Xiaojuan Yang
> <yangxiaojuan@loongson.cn>; Song Gao <gaosong@loongson.cn>; Michael S.
> Tsirkin <mst@redhat.com>; Igor Mammedov <imammedo@redhat.com>; Ani Sinha
> <anisinha@redhat.com>; Paolo Bonzini <pbonzini@redhat.com>; Richard
> Henderson <richard.henderson@linaro.org>; Eduardo Habkost
> <eduardo@habkost.net>; Marcel Apfelbaum <marcel.apfelbaum@gmail.com>;
> Philippe Mathieu-Daudé <philmd@linaro.org>; wangyanan (Y)
> <wangyanan55@huawei.com>; Daniel P. Berrangé <berrange@redhat.com>; Peter
> Xu <peterx@redhat.com>; Bibo Mao <maobibo@loongson.cn>
> Subject: Re: [PATCH v2 04/10] Introduce the CPU address space destruction
> function
> 
> On 26.09.23 14:32, Salil Mehta wrote:
> >> From: David Hildenbrand <david@redhat.com>
> >> Sent: Tuesday, September 26, 2023 1:24 PM
> >>
> >> On 26.09.23 13:55, Salil Mehta wrote:
> >>>> From: Salil Mehta
> >>>> Sent: Tuesday, September 26, 2023 12:21 PM
> >>>> To: 'David Hildenbrand' <david@redhat.com>; lixianglai
> >>>> <lixianglai@loongson.cn>; qemu-devel@nongnu.org
> >>>> Cc: Salil Mehta <salil.mehta@opnsrc.net>; Xiaojuan Yang
> >>>> <yangxiaojuan@loongson.cn>; Song Gao <gaosong@loongson.cn>; Michael S.
> >>>> Tsirkin <mst@redhat.com>; Igor Mammedov <imammedo@redhat.com>; Ani
> Sinha
> >>>> <anisinha@redhat.com>; Paolo Bonzini <pbonzini@redhat.com>; Richard
> >>>> Henderson <richard.henderson@linaro.org>; Eduardo Habkost
> >>>> <eduardo@habkost.net>; Marcel Apfelbaum <marcel.apfelbaum@gmail.com>;
> >>>> Philippe Mathieu-Daudé <philmd@linaro.org>; wangyanan (Y)
> >>>> <wangyanan55@huawei.com>; Daniel P. Berrangé <berrange@redhat.com>;
> >> Peter
> >>>> Xu <peterx@redhat.com>; Bibo Mao <maobibo@loongson.cn>
> >>>> Subject: RE: [PATCH v2 04/10] Introduce the CPU address space
> >> destruction
> >>>> function
> >>>>
> >>>> Hi David,
> >>>>
> >>>>> From: David Hildenbrand <david@redhat.com>
> >>>>> Sent: Friday, September 15, 2023 9:07 AM
> >>>>> To: lixianglai <lixianglai@loongson.cn>; qemu-devel@nongnu.org; Salil
> >>>> Mehta
> >>>>> <salil.mehta@huawei.com>
> >>>>> Cc: Salil Mehta <salil.mehta@opnsrc.net>; Xiaojuan Yang
> >>>>> <yangxiaojuan@loongson.cn>; Song Gao <gaosong@loongson.cn>; Michael
> S.
> >>>>> Tsirkin <mst@redhat.com>; Igor Mammedov <imammedo@redhat.com>; Ani
> >> Sinha
> >>>>> <anisinha@redhat.com>; Paolo Bonzini <pbonzini@redhat.com>; Richard
> >>>>> Henderson <richard.henderson@linaro.org>; Eduardo Habkost
> >>>>> <eduardo@habkost.net>; Marcel Apfelbaum <marcel.apfelbaum@gmail.com>;
> >>>>> Philippe Mathieu-Daudé <philmd@linaro.org>; wangyanan (Y)
> >>>>> <wangyanan55@huawei.com>; Daniel P. Berrangé <berrange@redhat.com>;
> >> Peter
> >>>>> Xu <peterx@redhat.com>; Bibo Mao <maobibo@loongson.cn>
> >>>>> Subject: Re: [PATCH v2 04/10] Introduce the CPU address space
> >> destruction
> >>>>> function
> >>>>>
> >>>>> On 15.09.23 04:53, lixianglai wrote:
> >>>>>> Hi David Hildenbrand:
> >>>>>>
> >>>>>>>
> >>>>>>> Hi David Hildenbrand:
> >>>>>>>> On 14.09.23 15:00, lixianglai wrote:
> >>>>>>>>> Hi David:
> >>>>>>>>
> >>>>>>>> Hi!
> >>>>>>>>
> >>>>>>>>>
> >>>>>>>>>> On 12.09.23 04:11, xianglai li wrote:
> >>>>>>>>>>> Introduce new function to destroy CPU address space resources
> >>>>>>>>>>> for cpu hot-(un)plug.
> >>>>>>>>>>>
> >>>>>>>>>> How do other archs handle that? Or how are they able to get away
> >>>>>>>>>> without destroying?
> >>>>>>>>>>
> >>>>>>>>> They do not remove the cpu address space, taking the X86
> >>>>>>>>> architecture as
> >>>>>>>>> an example:
> >>>>>>>>>
> >>>>>>>>> 1.Start the x86 VM:
> >>>>>>>>>
> >>>>>>>>> ./qemu-system-x86_64 \
> >>>>>>>>> -machine q35  \
> >>>>>>>>> -cpu Broadwell-IBRS \
> >>>>>>>>> -smp 1,maxcpus=100,sockets=100,cores=1,threads=1 \
> >>>>>>>>> -m 4G \
> >>>>>>>>> -drive file=~/anolis-8.8.qcow2  \
> >>>>>>>>> -serial stdio   \
> >>>>>>>>> -monitor telnet:localhost:4498,server,nowait   \
> >>>>>>>>> -nographic
> >>>>>>>>>
> >>>>>>>>> 2.Connect the qemu monitor
> >>>>>>>>>
> >>>>>>>>> telnet 127.0.0.1 4498
> >>>>>>>>>
> >>>>>>>>> info mtree
> >>>>>>>>>
> >>>>>>>>> address-space: cpu-memory-0
> >>>>>>>>> address-space: memory
> >>>>>>>>>        0000000000000000-ffffffffffffffff (prio 0, i/o): system
> >>>>>>>>>          0000000000000000-000000007fffffff (prio 0, ram): alias
> >>>>>>>>> ram-below-4g
> >>>>>>>>> @pc.ram 0000000000000000-000000007fffffff
> >>>>>>>>>          0000000000000000-ffffffffffffffff (prio -1, i/o): pci
> >>>>>>>>>            00000000000a0000-00000000000bffff (prio 1, i/o): vga-
> >> lowmem
> >>>>>>>>>
> >>>>>>>>> 3.Perform cpu hot swap int qemu monitor
> >>>>>>>>>
> >>>>>>>>> device_add
> >>>>>>>>> Broadwell-IBRS-x86_64-cpu,socket-id=1,core-id=0,thread-
> id=0,id=cpu1
> >>>>>>>>> device_del cpu1
> >>>>>>>>>
> >>>>>>>>
> >>>>>>>> Hm, doesn't seem to work for me on upstream QEMU for some reason:
> >>>>>>>> "Error: acpi: device unplug request for not supported device type:
> >>>>>>>> Broadwell-IBRS-x86_64-cpu"
> >>>>>>>
> >>>>>> First I use qemu tcg, and then the cpu needs to be removed after the
> >>>>>> operating system is booted.
> >>>>>
> >>>>> Ah, the last thing is the important bit. I can reproduce this with
> KVM
> >>>>> easily.
> >>>>>
> >>>>> Doing it a couple of times
> >>>>>
> >>>>> address-space: cpu-memory-0
> >>>>> address-space: cpu-memory-1
> >>>>> address-space: cpu-memory-1
> >>>>> address-space: cpu-memory-1
> >>>>> address-space: cpu-memory-1
> >>>>> address-space: cpu-memory-1
> >>>>> address-space: cpu-memory-1
> >>>>> address-space: cpu-memory-1
> >>>>> address-space: cpu-memory-1
> >>>>> address-space: cpu-memory-1
> >>>>> address-space: cpu-memory-1
> >>>>> address-space: cpu-memory-1
> >>>>> address-space: cpu-memory-1
> >>>>> address-space: cpu-memory-1
> >>>>> address-space: cpu-memory-1
> >>>>> address-space: cpu-memory-1
> >>>>> address-space: cpu-memory-1
> >>>>> address-space: cpu-memory-1
> >>>>> address-space: cpu-memory-1
> >>>>> address-space: cpu-memory-1
> >>>>>
> >>>>> Looks like a resource/memory leak.
> >>>>
> >>>> Yes, there was. Thanks for identifying it. I have fixed in the
> >>>> latest RFC V2. Please check here:
> >>>>
> >>>> https://lore.kernel.org/qemu-devel/20230926100436.28284-1-
> >>>> salil.mehta@huawei.com/T/#m5f5ae40b091d69d01012880d7500d96874a9d39c
> >>>>
> >>>> I have tested and AddressSpace comes and goes away cleanly
> >>>> on CPU hot(un)plug action.
> >>>
> >>> Hi David/Xianglai,
> >>> Are you okay if I put Reported-by and give reference to this
> >>> conversation?
> >>
> >> Yes. And ideally, send the fixes separately from the other arm patches.
> >
> > ARM Virtual CPU Hotplug support patches are still under review.
> 
> The other architectures (as shown, x86 is affected) can be fixed
> independent of that support.

Yes, they can be and the TCG as well. Unrealize part of TCG is
broken even on ARM. Need some way to cleanly unassign Translation
blocks from the Region trees. That’s a pending work at our end.
But you are more than welcome to help and contribute in that
as well.

Also, I can help to contribute, if required.

Many thanks
Salil.

Re: [PATCH v2 04/10] Introduce the CPU address space destruction function

[David Hildenbrand]

On 26.09.23 14:44, Salil Mehta wrote:
>> From: David Hildenbrand <david@redhat.com>
>> Sent: Tuesday, September 26, 2023 1:37 PM
>> To: Salil Mehta <salil.mehta@huawei.com>; lixianglai
>> <lixianglai@loongson.cn>; qemu-devel@nongnu.org
>> Cc: Salil Mehta <salil.mehta@opnsrc.net>; Xiaojuan Yang
>> <yangxiaojuan@loongson.cn>; Song Gao <gaosong@loongson.cn>; Michael S.
>> Tsirkin <mst@redhat.com>; Igor Mammedov <imammedo@redhat.com>; Ani Sinha
>> <anisinha@redhat.com>; Paolo Bonzini <pbonzini@redhat.com>; Richard
>> Henderson <richard.henderson@linaro.org>; Eduardo Habkost
>> <eduardo@habkost.net>; Marcel Apfelbaum <marcel.apfelbaum@gmail.com>;
>> Philippe Mathieu-Daudé <philmd@linaro.org>; wangyanan (Y)
>> <wangyanan55@huawei.com>; Daniel P. Berrangé <berrange@redhat.com>; Peter
>> Xu <peterx@redhat.com>; Bibo Mao <maobibo@loongson.cn>
>> Subject: Re: [PATCH v2 04/10] Introduce the CPU address space destruction
>> function
>>
>> On 26.09.23 14:32, Salil Mehta wrote:
>>>> From: David Hildenbrand <david@redhat.com>
>>>> Sent: Tuesday, September 26, 2023 1:24 PM
>>>>
>>>> On 26.09.23 13:55, Salil Mehta wrote:
>>>>>> From: Salil Mehta
>>>>>> Sent: Tuesday, September 26, 2023 12:21 PM
>>>>>> To: 'David Hildenbrand' <david@redhat.com>; lixianglai
>>>>>> <lixianglai@loongson.cn>; qemu-devel@nongnu.org
>>>>>> Cc: Salil Mehta <salil.mehta@opnsrc.net>; Xiaojuan Yang
>>>>>> <yangxiaojuan@loongson.cn>; Song Gao <gaosong@loongson.cn>; Michael S.
>>>>>> Tsirkin <mst@redhat.com>; Igor Mammedov <imammedo@redhat.com>; Ani
>> Sinha
>>>>>> <anisinha@redhat.com>; Paolo Bonzini <pbonzini@redhat.com>; Richard
>>>>>> Henderson <richard.henderson@linaro.org>; Eduardo Habkost
>>>>>> <eduardo@habkost.net>; Marcel Apfelbaum <marcel.apfelbaum@gmail.com>;
>>>>>> Philippe Mathieu-Daudé <philmd@linaro.org>; wangyanan (Y)
>>>>>> <wangyanan55@huawei.com>; Daniel P. Berrangé <berrange@redhat.com>;
>>>> Peter
>>>>>> Xu <peterx@redhat.com>; Bibo Mao <maobibo@loongson.cn>
>>>>>> Subject: RE: [PATCH v2 04/10] Introduce the CPU address space
>>>> destruction
>>>>>> function
>>>>>>
>>>>>> Hi David,
>>>>>>
>>>>>>> From: David Hildenbrand <david@redhat.com>
>>>>>>> Sent: Friday, September 15, 2023 9:07 AM
>>>>>>> To: lixianglai <lixianglai@loongson.cn>; qemu-devel@nongnu.org; Salil
>>>>>> Mehta
>>>>>>> <salil.mehta@huawei.com>
>>>>>>> Cc: Salil Mehta <salil.mehta@opnsrc.net>; Xiaojuan Yang
>>>>>>> <yangxiaojuan@loongson.cn>; Song Gao <gaosong@loongson.cn>; Michael
>> S.
>>>>>>> Tsirkin <mst@redhat.com>; Igor Mammedov <imammedo@redhat.com>; Ani
>>>> Sinha
>>>>>>> <anisinha@redhat.com>; Paolo Bonzini <pbonzini@redhat.com>; Richard
>>>>>>> Henderson <richard.henderson@linaro.org>; Eduardo Habkost
>>>>>>> <eduardo@habkost.net>; Marcel Apfelbaum <marcel.apfelbaum@gmail.com>;
>>>>>>> Philippe Mathieu-Daudé <philmd@linaro.org>; wangyanan (Y)
>>>>>>> <wangyanan55@huawei.com>; Daniel P. Berrangé <berrange@redhat.com>;
>>>> Peter
>>>>>>> Xu <peterx@redhat.com>; Bibo Mao <maobibo@loongson.cn>
>>>>>>> Subject: Re: [PATCH v2 04/10] Introduce the CPU address space
>>>> destruction
>>>>>>> function
>>>>>>>
>>>>>>> On 15.09.23 04:53, lixianglai wrote:
>>>>>>>> Hi David Hildenbrand:
>>>>>>>>
>>>>>>>>>
>>>>>>>>> Hi David Hildenbrand:
>>>>>>>>>> On 14.09.23 15:00, lixianglai wrote:
>>>>>>>>>>> Hi David:
>>>>>>>>>>
>>>>>>>>>> Hi!
>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>> On 12.09.23 04:11, xianglai li wrote:
>>>>>>>>>>>>> Introduce new function to destroy CPU address space resources
>>>>>>>>>>>>> for cpu hot-(un)plug.
>>>>>>>>>>>>>
>>>>>>>>>>>> How do other archs handle that? Or how are they able to get away
>>>>>>>>>>>> without destroying?
>>>>>>>>>>>>
>>>>>>>>>>> They do not remove the cpu address space, taking the X86
>>>>>>>>>>> architecture as
>>>>>>>>>>> an example:
>>>>>>>>>>>
>>>>>>>>>>> 1.Start the x86 VM:
>>>>>>>>>>>
>>>>>>>>>>> ./qemu-system-x86_64 \
>>>>>>>>>>> -machine q35  \
>>>>>>>>>>> -cpu Broadwell-IBRS \
>>>>>>>>>>> -smp 1,maxcpus=100,sockets=100,cores=1,threads=1 \
>>>>>>>>>>> -m 4G \
>>>>>>>>>>> -drive file=~/anolis-8.8.qcow2  \
>>>>>>>>>>> -serial stdio   \
>>>>>>>>>>> -monitor telnet:localhost:4498,server,nowait   \
>>>>>>>>>>> -nographic
>>>>>>>>>>>
>>>>>>>>>>> 2.Connect the qemu monitor
>>>>>>>>>>>
>>>>>>>>>>> telnet 127.0.0.1 4498
>>>>>>>>>>>
>>>>>>>>>>> info mtree
>>>>>>>>>>>
>>>>>>>>>>> address-space: cpu-memory-0
>>>>>>>>>>> address-space: memory
>>>>>>>>>>>         0000000000000000-ffffffffffffffff (prio 0, i/o): system
>>>>>>>>>>>           0000000000000000-000000007fffffff (prio 0, ram): alias
>>>>>>>>>>> ram-below-4g
>>>>>>>>>>> @pc.ram 0000000000000000-000000007fffffff
>>>>>>>>>>>           0000000000000000-ffffffffffffffff (prio -1, i/o): pci
>>>>>>>>>>>             00000000000a0000-00000000000bffff (prio 1, i/o): vga-
>>>> lowmem
>>>>>>>>>>>
>>>>>>>>>>> 3.Perform cpu hot swap int qemu monitor
>>>>>>>>>>>
>>>>>>>>>>> device_add
>>>>>>>>>>> Broadwell-IBRS-x86_64-cpu,socket-id=1,core-id=0,thread-
>> id=0,id=cpu1
>>>>>>>>>>> device_del cpu1
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Hm, doesn't seem to work for me on upstream QEMU for some reason:
>>>>>>>>>> "Error: acpi: device unplug request for not supported device type:
>>>>>>>>>> Broadwell-IBRS-x86_64-cpu"
>>>>>>>>>
>>>>>>>> First I use qemu tcg, and then the cpu needs to be removed after the
>>>>>>>> operating system is booted.
>>>>>>>
>>>>>>> Ah, the last thing is the important bit. I can reproduce this with
>> KVM
>>>>>>> easily.
>>>>>>>
>>>>>>> Doing it a couple of times
>>>>>>>
>>>>>>> address-space: cpu-memory-0
>>>>>>> address-space: cpu-memory-1
>>>>>>> address-space: cpu-memory-1
>>>>>>> address-space: cpu-memory-1
>>>>>>> address-space: cpu-memory-1
>>>>>>> address-space: cpu-memory-1
>>>>>>> address-space: cpu-memory-1
>>>>>>> address-space: cpu-memory-1
>>>>>>> address-space: cpu-memory-1
>>>>>>> address-space: cpu-memory-1
>>>>>>> address-space: cpu-memory-1
>>>>>>> address-space: cpu-memory-1
>>>>>>> address-space: cpu-memory-1
>>>>>>> address-space: cpu-memory-1
>>>>>>> address-space: cpu-memory-1
>>>>>>> address-space: cpu-memory-1
>>>>>>> address-space: cpu-memory-1
>>>>>>> address-space: cpu-memory-1
>>>>>>> address-space: cpu-memory-1
>>>>>>> address-space: cpu-memory-1
>>>>>>>
>>>>>>> Looks like a resource/memory leak.
>>>>>>
>>>>>> Yes, there was. Thanks for identifying it. I have fixed in the
>>>>>> latest RFC V2. Please check here:
>>>>>>
>>>>>> https://lore.kernel.org/qemu-devel/20230926100436.28284-1-
>>>>>> salil.mehta@huawei.com/T/#m5f5ae40b091d69d01012880d7500d96874a9d39c
>>>>>>
>>>>>> I have tested and AddressSpace comes and goes away cleanly
>>>>>> on CPU hot(un)plug action.
>>>>>
>>>>> Hi David/Xianglai,
>>>>> Are you okay if I put Reported-by and give reference to this
>>>>> conversation?
>>>>
>>>> Yes. And ideally, send the fixes separately from the other arm patches.
>>>
>>> ARM Virtual CPU Hotplug support patches are still under review.
>>
>> The other architectures (as shown, x86 is affected) can be fixed
>> independent of that support.
> 
> Yes, they can be and the TCG as well. Unrealize part of TCG is
> broken even on ARM. Need some way to cleanly unassign Translation
> blocks from the Region trees. That’s a pending work at our end.
> But you are more than welcome to help and contribute in that
> as well.

I have absolutely no time for that, happy to review patches.

-- 
Cheers,

David / dhildenb

Re: [PATCH v2 04/10] Introduce the CPU address space destruction function

[lixianglai]

Hi Salil Mehta:
>> From: Salil Mehta
>> Sent: Tuesday, September 26, 2023 12:21 PM
>> To: 'David Hildenbrand' <david@redhat.com>; lixianglai
>> <lixianglai@loongson.cn>; qemu-devel@nongnu.org
>> Cc: Salil Mehta <salil.mehta@opnsrc.net>; Xiaojuan Yang
>> <yangxiaojuan@loongson.cn>; Song Gao <gaosong@loongson.cn>; Michael S.
>> Tsirkin <mst@redhat.com>; Igor Mammedov <imammedo@redhat.com>; Ani Sinha
>> <anisinha@redhat.com>; Paolo Bonzini <pbonzini@redhat.com>; Richard
>> Henderson <richard.henderson@linaro.org>; Eduardo Habkost
>> <eduardo@habkost.net>; Marcel Apfelbaum <marcel.apfelbaum@gmail.com>;
>> Philippe Mathieu-Daudé <philmd@linaro.org>; wangyanan (Y)
>> <wangyanan55@huawei.com>; Daniel P. Berrangé <berrange@redhat.com>; Peter
>> Xu <peterx@redhat.com>; Bibo Mao <maobibo@loongson.cn>
>> Subject: RE: [PATCH v2 04/10] Introduce the CPU address space destruction
>> function
>>
>> Hi David,
>>
>>> From: David Hildenbrand <david@redhat.com>
>>> Sent: Friday, September 15, 2023 9:07 AM
>>> To: lixianglai <lixianglai@loongson.cn>; qemu-devel@nongnu.org; Salil
>> Mehta
>>> <salil.mehta@huawei.com>
>>> Cc: Salil Mehta <salil.mehta@opnsrc.net>; Xiaojuan Yang
>>> <yangxiaojuan@loongson.cn>; Song Gao <gaosong@loongson.cn>; Michael S.
>>> Tsirkin <mst@redhat.com>; Igor Mammedov <imammedo@redhat.com>; Ani Sinha
>>> <anisinha@redhat.com>; Paolo Bonzini <pbonzini@redhat.com>; Richard
>>> Henderson <richard.henderson@linaro.org>; Eduardo Habkost
>>> <eduardo@habkost.net>; Marcel Apfelbaum <marcel.apfelbaum@gmail.com>;
>>> Philippe Mathieu-Daudé <philmd@linaro.org>; wangyanan (Y)
>>> <wangyanan55@huawei.com>; Daniel P. Berrangé <berrange@redhat.com>; Peter
>>> Xu <peterx@redhat.com>; Bibo Mao <maobibo@loongson.cn>
>>> Subject: Re: [PATCH v2 04/10] Introduce the CPU address space destruction
>>> function
>>>
>>> On 15.09.23 04:53, lixianglai wrote:
>>>> Hi David Hildenbrand:
>>>>
>>>>> Hi David Hildenbrand:
>>>>>> On 14.09.23 15:00, lixianglai wrote:
>>>>>>> Hi David:
>>>>>> Hi!
>>>>>>
>>>>>>>> On 12.09.23 04:11, xianglai li wrote:
>>>>>>>>> Introduce new function to destroy CPU address space resources
>>>>>>>>> for cpu hot-(un)plug.
>>>>>>>>>
>>>>>>>> How do other archs handle that? Or how are they able to get away
>>>>>>>> without destroying?
>>>>>>>>
>>>>>>> They do not remove the cpu address space, taking the X86
>>>>>>> architecture as
>>>>>>> an example:
>>>>>>>
>>>>>>> 1.Start the x86 VM:
>>>>>>>
>>>>>>> ./qemu-system-x86_64 \
>>>>>>> -machine q35  \
>>>>>>> -cpu Broadwell-IBRS \
>>>>>>> -smp 1,maxcpus=100,sockets=100,cores=1,threads=1 \
>>>>>>> -m 4G \
>>>>>>> -drive file=~/anolis-8.8.qcow2  \
>>>>>>> -serial stdio   \
>>>>>>> -monitor telnet:localhost:4498,server,nowait   \
>>>>>>> -nographic
>>>>>>>
>>>>>>> 2.Connect the qemu monitor
>>>>>>>
>>>>>>> telnet 127.0.0.1 4498
>>>>>>>
>>>>>>> info mtree
>>>>>>>
>>>>>>> address-space: cpu-memory-0
>>>>>>> address-space: memory
>>>>>>>       0000000000000000-ffffffffffffffff (prio 0, i/o): system
>>>>>>>         0000000000000000-000000007fffffff (prio 0, ram): alias
>>>>>>> ram-below-4g
>>>>>>> @pc.ram 0000000000000000-000000007fffffff
>>>>>>>         0000000000000000-ffffffffffffffff (prio -1, i/o): pci
>>>>>>>           00000000000a0000-00000000000bffff (prio 1, i/o): vga-lowmem
>>>>>>>
>>>>>>> 3.Perform cpu hot swap int qemu monitor
>>>>>>>
>>>>>>> device_add
>>>>>>> Broadwell-IBRS-x86_64-cpu,socket-id=1,core-id=0,thread-id=0,id=cpu1
>>>>>>> device_del cpu1
>>>>>>>
>>>>>> Hm, doesn't seem to work for me on upstream QEMU for some reason:
>>>>>> "Error: acpi: device unplug request for not supported device type:
>>>>>> Broadwell-IBRS-x86_64-cpu"
>>>> First I use qemu tcg, and then the cpu needs to be removed after the
>>>> operating system is booted.
>>> Ah, the last thing is the important bit. I can reproduce this with KVM
>>> easily.
>>>
>>> Doing it a couple of times
>>>
>>> address-space: cpu-memory-0
>>> address-space: cpu-memory-1
>>> address-space: cpu-memory-1
>>> address-space: cpu-memory-1
>>> address-space: cpu-memory-1
>>> address-space: cpu-memory-1
>>> address-space: cpu-memory-1
>>> address-space: cpu-memory-1
>>> address-space: cpu-memory-1
>>> address-space: cpu-memory-1
>>> address-space: cpu-memory-1
>>> address-space: cpu-memory-1
>>> address-space: cpu-memory-1
>>> address-space: cpu-memory-1
>>> address-space: cpu-memory-1
>>> address-space: cpu-memory-1
>>> address-space: cpu-memory-1
>>> address-space: cpu-memory-1
>>> address-space: cpu-memory-1
>>> address-space: cpu-memory-1
>>>
>>> Looks like a resource/memory leak.
>> Yes, there was. Thanks for identifying it. I have fixed in the
>> latest RFC V2. Please check here:
>>
>> https://lore.kernel.org/qemu-devel/20230926100436.28284-1-
>> salil.mehta@huawei.com/T/#m5f5ae40b091d69d01012880d7500d96874a9d39c
>>
>> I have tested and AddressSpace comes and goes away cleanly
>> on CPU hot(un)plug action.
> Hi David/Xianglai,
> Are you okay if I put Reported-by and give reference to this
> conversation?

That's Ok for me.

Thanks,

Xianglai.


> Many thanks
> Salil
>
>

RE: [PATCH v2 04/10] Introduce the CPU address space destruction function

[Salil Mehta via]

Hi Xianglai,

> From: lixianglai <lixianglai@loongson.cn>
> Sent: Friday, September 15, 2023 3:48 AM
> To: David Hildenbrand <david@redhat.com>; qemu-devel@nongnu.org; Salil
> Mehta <salil.mehta@huawei.com>
> Cc: Salil Mehta <salil.mehta@opnsrc.net>; Xiaojuan Yang
> <yangxiaojuan@loongson.cn>; Song Gao <gaosong@loongson.cn>; Michael S.
> Tsirkin <mst@redhat.com>; Igor Mammedov <imammedo@redhat.com>; Ani Sinha
> <anisinha@redhat.com>; Paolo Bonzini <pbonzini@redhat.com>; Richard
> Henderson <richard.henderson@linaro.org>; Eduardo Habkost
> <eduardo@habkost.net>; Marcel Apfelbaum <marcel.apfelbaum@gmail.com>;
> Philippe Mathieu-Daudé <philmd@linaro.org>; wangyanan (Y)
> <wangyanan55@huawei.com>; Daniel P. Berrangé <berrange@redhat.com>; Peter
> Xu <peterx@redhat.com>; Bibo Mao <maobibo@loongson.cn>
> Subject: Re: [PATCH v2 04/10] Introduce the CPU address space destruction
> function
> 
> 
> Hi David Hildenbrand:
> > On 14.09.23 15:00, lixianglai wrote:
> >> Hi David:
> >
> > Hi!
> >
> >>
> >>> On 12.09.23 04:11, xianglai li wrote:
> >>>> Introduce new function to destroy CPU address space resources
> >>>> for cpu hot-(un)plug.
> >>>>
> >>> How do other archs handle that? Or how are they able to get away
> >>> without destroying?
> >>>
> >> They do not remove the cpu address space, taking the X86 architecture as
> >> an example:
> >>
> >> 1.Start the x86 VM:
> >>
> >> ./qemu-system-x86_64 \
> >> -machine q35  \
> >> -cpu Broadwell-IBRS \
> >> -smp 1,maxcpus=100,sockets=100,cores=1,threads=1 \
> >> -m 4G \
> >> -drive file=~/anolis-8.8.qcow2  \
> >> -serial stdio   \
> >> -monitor telnet:localhost:4498,server,nowait   \
> >> -nographic
> >>
> >> 2.Connect the qemu monitor
> >>
> >> telnet 127.0.0.1 4498
> >>
> >> info mtree
> >>
> >> address-space: cpu-memory-0
> >> address-space: memory
> >>     0000000000000000-ffffffffffffffff (prio 0, i/o): system
> >>       0000000000000000-000000007fffffff (prio 0, ram): alias
> >> ram-below-4g
> >> @pc.ram 0000000000000000-000000007fffffff
> >>       0000000000000000-ffffffffffffffff (prio -1, i/o): pci
> >>         00000000000a0000-00000000000bffff (prio 1, i/o): vga-lowmem
> >>
> >> 3.Perform cpu hot swap int qemu monitor
> >>
> >> device_add
> >> Broadwell-IBRS-x86_64-cpu,socket-id=1,core-id=0,thread-id=0,id=cpu1
> >> device_del cpu1
> >>
> >
> > Hm, doesn't seem to work for me on upstream QEMU for some reason:
> > "Error: acpi: device unplug request for not supported device type:
> > Broadwell-IBRS-x86_64-cpu"
> 
> >
> > What happens if you re-add that CPU? Will we reuse the previous
> > address space?
> 
> 
> Here is the memory layout where I inserted cpu1 again. It does not
> appear that the original address space was reused, and the address space
> is now duplicated
> 
> info mtree
> 
> address-space: cpu-memory-0
> address-space: cpu-memory-1
> address-space: cpu-memory-1
> address-space: memory
>    0000000000000000-ffffffffffffffff (prio 0, i/o): system
>      0000000000000000-000000007fffffff (prio 0, ram): alias ram-below-4g
> @pc.ram 0000000000000000-000000007fffffff
>      0000000000000000-ffffffffffffffff (prio -1, i/o): pci
>        00000000000a0000-00000000000affff (prio 2, ram): alias vga.chain4
> @vga.vram 0000000000000000-000000000000ffff
>        00000000000a0000-00000000000bffff (prio 1, i/o): vga-lowmem
>        00000000000c0000-00000000000dffff (prio 1, rom): pc.rom
>        00000000000e0000-00000000000fffff (prio 1, rom): alias isa-bios
> @pc.bios 0000000000020000-000000000003ffff
>        00000000fd000000-00000000fdffffff (prio 1, ram): vga.vram
> 
> 
> In addition, I do not find the corresponding resource release action for
> cpu->cpu_ases requested in function cpu_address_space_init.
> 
> I wonder if there is a leak in the memory space requested here. Maybe
> qemu automatically reclaims memory space
> 
> or frees resources somewhere else I didn't find? I thought I'd try
> running the following valgrind to see if I could verify my suspicions.
> 
> void cpu_address_space_init(CPUState *cpu, int asidx,
>                              const char *prefix, MemoryRegion *mr)
> {
> 
> ...
> 
>      if (!cpu->cpu_ases) {
>          cpu->cpu_ases = g_new0(CPUAddressSpace, cpu->num_ases);
>      }
> 
> ...
> 
> }
> 
> >
> >> info mtree
> >>
> >> address-space: cpu-memory-0
> >> address-space: cpu-memory-1
> >> address-space: memory
> >>     0000000000000000-ffffffffffffffff (prio 0, i/o): system
> >>       0000000000000000-000000007fffffff (prio 0, ram): alias
> >> ram-below-4g
> >> @pc.ram 0000000000000000-000000007fffffff
> >>       0000000000000000-ffffffffffffffff (prio -1, i/o): pci
> >>         00000000000a0000-00000000000bffff (prio 1, i/o): vga-lowmem
> >>
> >>
> >>   From the above test, you can see whether the address space of cpu1 is
> >> residual after a cpu hot swap, and whether it is reasonable?
> >
> >
> > Probably we should teach other archs to destroy that address space as
> > well.
> >
> > Can we do that from the core, instead of having to do that in each CPU
> > unrealize function?
> >
> I think it can also be done in the public code flow. Since I refer to
> arm's scheme
> 
> (https://lore.kernel.org/all/20200613213629.21984-1-
> salil.mehta@huawei.com/),
> 
> 
> and arm's patch will be issued soon, I will conduct rebase based on arm
> patch in the future.


Here it is:
https://lore.kernel.org/qemu-devel/20230926100436.28284-1-salil.mehta@huawei.com/T/#m523b37819c4811c7827333982004e07a1ef03879



> 
> Therefore, I would like to see if arm has any good suggestions. If there
> are no good suggestions at this stage,
> 
> I think we can shelve this problem for the first time, and I can
> consider not referencing this function for the first time,
> 
> and we can submit another patch to solve this problem.
> 
> Hi Salil Mehta:
> 
> Is the cpu_address_space_destroy function still present in the new patch
> version of arm?

Yes, this is present in the RFC V2. Please find it here

https://lore.kernel.org/qemu-devel/20230926100436.28284-1-salil.mehta@huawei.com/T/#mfb2a525081c412917a0026d558e72f48875e386d



> 
> Can we put this function on the public path of cpu destroy?

Yes, AddressSpace destruction is already part of the Architecture
agnostic patches. Please rebase you patch-set and you will
see your bugs disappearing :)


Thanks
Salil.

RE: [PATCH v2 04/10] Introduce the CPU address space destruction function

[Salil Mehta via]

> From: qemu-devel-bounces+salil.mehta=huawei.com@nongnu.org <qemu-devel-
> bounces+salil.mehta=huawei.com@nongnu.org> On Behalf Of lixianglai
> Sent: Thursday, September 14, 2023 2:01 PM
> To: David Hildenbrand <david@redhat.com>; qemu-devel@nongnu.org
> Cc: Salil Mehta <salil.mehta@opnsrc.net>; Xiaojuan Yang
> <yangxiaojuan@loongson.cn>; Song Gao <gaosong@loongson.cn>; Michael S.
> Tsirkin <mst@redhat.com>; Igor Mammedov <imammedo@redhat.com>; Ani Sinha
> <anisinha@redhat.com>; Paolo Bonzini <pbonzini@redhat.com>; Richard
> Henderson <richard.henderson@linaro.org>; Eduardo Habkost
> <eduardo@habkost.net>; Marcel Apfelbaum <marcel.apfelbaum@gmail.com>;
> Philippe Mathieu-Daudé <philmd@linaro.org>; wangyanan (Y)
> <wangyanan55@huawei.com>; Daniel P. Berrangé <berrange@redhat.com>; Peter
> Xu <peterx@redhat.com>; Bibo Mao <maobibo@loongson.cn>
> Subject: Re: [PATCH v2 04/10] Introduce the CPU address space destruction
> function
> 
> Hi David:
> 
> > On 12.09.23 04:11, xianglai li wrote:
> >> Introduce new function to destroy CPU address space resources
> >> for cpu hot-(un)plug.
> >>
> > How do other archs handle that? Or how are they able to get away
> > without destroying?
> >
> They do not remove the cpu address space, taking the X86 architecture as
> an example:
> 
> 1.Start the x86 VM:
> 
> ./qemu-system-x86_64 \
> -machine q35  \
> -cpu Broadwell-IBRS \
> -smp 1,maxcpus=100,sockets=100,cores=1,threads=1 \
> -m 4G \
> -drive file=~/anolis-8.8.qcow2  \
> -serial stdio   \
> -monitor telnet:localhost:4498,server,nowait   \
> -nographic
> 
> 2.Connect the qemu monitor
> 
> telnet 127.0.0.1 4498
> 
> info mtree
> 
> address-space: cpu-memory-0
> address-space: memory
>    0000000000000000-ffffffffffffffff (prio 0, i/o): system
>      0000000000000000-000000007fffffff (prio 0, ram): alias ram-below-4g
> @pc.ram 0000000000000000-000000007fffffff
>      0000000000000000-ffffffffffffffff (prio -1, i/o): pci
>        00000000000a0000-00000000000bffff (prio 1, i/o): vga-lowmem
> 
> 3.Perform cpu hot swap int qemu monitor
> 
> device_add
> Broadwell-IBRS-x86_64-cpu,socket-id=1,core-id=0,thread-id=0,id=cpu1
> device_del cpu1
> 
> info mtree
> 
> address-space: cpu-memory-0
> address-space: cpu-memory-1
> address-space: memory
>    0000000000000000-ffffffffffffffff (prio 0, i/o): system
>      0000000000000000-000000007fffffff (prio 0, ram): alias ram-below-4g
> @pc.ram 0000000000000000-000000007fffffff
>      0000000000000000-ffffffffffffffff (prio -1, i/o): pci
>        00000000000a0000-00000000000bffff (prio 1, i/o): vga-lowmem
> 
> 
>  From the above test, you can see whether the address space of cpu1 is
> residual after a cpu hot swap, and whether it is reasonable?
> 
> The address space destruction function of the CPU can be used to delete
> the residual address space of the CPU1.

Yes, it can be but I would interested to know if there is any problem
which might have gone unnoticed while doing it.

Many thanks
Salil.

RE: [PATCH v2 04/10] Introduce the CPU address space destruction function

[Salil Mehta via]

Hi David,

> From: qemu-devel-bounces+salil.mehta=huawei.com@nongnu.org <qemu-devel-
> bounces+salil.mehta=huawei.com@nongnu.org> On Behalf Of David Hildenbrand
> Sent: Tuesday, September 12, 2023 8:00 AM
> To: xianglai li <lixianglai@loongson.cn>; qemu-devel@nongnu.org
> Cc: Salil Mehta <salil.mehta@opnsrc.net>; Xiaojuan Yang
> <yangxiaojuan@loongson.cn>; Song Gao <gaosong@loongson.cn>; Michael S.
> Tsirkin <mst@redhat.com>; Igor Mammedov <imammedo@redhat.com>; Ani Sinha
> <anisinha@redhat.com>; Paolo Bonzini <pbonzini@redhat.com>; Richard
> Henderson <richard.henderson@linaro.org>; Eduardo Habkost
> <eduardo@habkost.net>; Marcel Apfelbaum <marcel.apfelbaum@gmail.com>;
> Philippe Mathieu-Daudé <philmd@linaro.org>; wangyanan (Y)
> <wangyanan55@huawei.com>; Daniel P. Berrangé <berrange@redhat.com>; Peter
> Xu <peterx@redhat.com>; Bibo Mao <maobibo@loongson.cn>
> Subject: Re: [PATCH v2 04/10] Introduce the CPU address space destruction
> function
> 
> On 12.09.23 04:11, xianglai li wrote:
> > Introduce new function to destroy CPU address space resources
> > for cpu hot-(un)plug.
> >
> How do other archs handle that? Or how are they able to get away without
> destroying?

This patch-set is based on the ARM RFC. We do destroy AddressSpace there.
Is there any reason you are hinting why it should not be done?

I have posted the RFC V2 Virtual CPU Hotplug Support on ARM today and
You are CC'ed in it. Please have a look at the implementation:

https://lore.kernel.org/qemu-devel/20230926100436.28284-1-salil.mehta@huawei.com/T/#m523b37819c4811c7827333982004e07a1ef03879

Thanks
Salil.

[PATCH v2 05/10] Added CPU topology support for Loongarch

[xianglai li]

1.Add topological relationships for Loongarch VCPU
and initialize topology member variables.
2.Add a description of the calculation method of
the arch_id and the topological relationship of the CPU.

Cc: "Salil Mehta" <salil.mehta@opnsrc.net>
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>
Cc: Bibo Mao <maobibo@loongson.cn>
Signed-off-by: xianglai li <lixianglai@loongson.cn>
---
 docs/system/loongarch/virt.rst |  31 ++++++++++
 hw/loongarch/virt.c            | 101 ++++++++++++++++++++++++++-------
 target/loongarch/cpu.c         |  13 ++++-
 target/loongarch/cpu.h         |  12 +++-
 4 files changed, 134 insertions(+), 23 deletions(-)

diff --git a/docs/system/loongarch/virt.rst b/docs/system/loongarch/virt.rst
index c37268b404..eaba9e2fd7 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 2629128aed..b8474e7b94 100644
--- a/hw/loongarch/virt.c
+++ b/hw/loongarch/virt.c
@@ -624,11 +624,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,9 +789,7 @@ 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;
 
     if (!cpu_model) {
@@ -803,16 +801,41 @@ static void loongarch_init(MachineState *machine)
         exit(1);
     }
     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);
-        cpu->cpu_index = i;
-        machine->possible_cpus->cpus[i].cpu = OBJECT(cpu);
-        lacpu = LOONGARCH_CPU(cpu);
-        lacpu->phy_id = machine->possible_cpus->cpus[i].arch_id;
+    /* Init CPUs */
+    mc->possible_cpu_arch_ids(machine);
+    for (i = 0; i < machine->smp.cpus; i++) {
+        Object *cpuobj;
+        cpuobj = object_new(machine->cpu_type);
+        lacpu = LOONGARCH_CPU(cpuobj);
+
+        lacpu->arch_id = machine->possible_cpus->cpus[i].arch_id;
+        object_property_set_int(cpuobj, "socket-id",
+                                machine->possible_cpus->cpus[i].props.socket_id,
+                                NULL);
+        object_property_set_int(cpuobj, "core-id",
+                                machine->possible_cpus->cpus[i].props.core_id,
+                                NULL);
+        object_property_set_int(cpuobj, "thread-id",
+                                machine->possible_cpus->cpus[i].props.thread_id,
+                                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);
     }
+
     fdt_add_cpu_nodes(lams);
 
     /* Node0 memory */
@@ -983,6 +1006,37 @@ 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_arch_id_from_cpu_topo(const MachineState *ms,
+                                          LoongArchCPUTopo *cpu_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 = cpu_topo->socket_id * (ms->smp.threads * ms->smp.cores);
+    core_vcpu_num = cpu_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) + cpu_topo->thread_id;
+
+    assert(arch_id >= 0 && arch_id < ms->possible_cpus->len);
+
+    return arch_id;
+}
+
+static void virt_get_cpu_topo_by_cpu_index(const MachineState *ms,
+                                           LoongArchCPUTopo *cpu_topo,
+                                           int cpu_index)
+{
+    cpu_topo->socket_id  = cpu_index / (ms->smp.cores * ms->smp.threads);
+    cpu_topo->core_id = cpu_index / ms->smp.threads % ms->smp.cores;
+    cpu_topo->thread_id = cpu_index % ms->smp.threads;
+}
+
 static void virt_machine_device_pre_plug(HotplugHandler *hotplug_dev,
                                             DeviceState *dev, Error **errp)
 {
@@ -1069,6 +1123,8 @@ 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;
+    LoongArchCPUTopo cpu_topo;
 
     if (ms->possible_cpus) {
         assert(ms->possible_cpus->len == max_cpus);
@@ -1079,17 +1135,20 @@ 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;
+
+        virt_get_cpu_topo_by_cpu_index(ms, &cpu_topo, n);
 
         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  = cpu_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 = cpu_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 = cpu_topo.thread_id;
+
+        ms->possible_cpus->cpus[n].arch_id =
+                                virt_get_arch_id_from_cpu_topo(ms, &cpu_topo);
     }
     return ms->possible_cpus;
 }
diff --git a/target/loongarch/cpu.c b/target/loongarch/cpu.c
index 65f9320e34..a5153cb3bc 100644
--- a/target/loongarch/cpu.c
+++ b/target/loongarch/cpu.c
@@ -19,6 +19,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",
@@ -728,10 +729,19 @@ static int64_t loongarch_cpu_get_arch_id(CPUState *cs)
 {
     LoongArchCPU *cpu = LOONGARCH_CPU(cs);
 
-    return cpu->phy_id;
+    return cpu->arch_id;
 }
 #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);
@@ -739,6 +749,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,
diff --git a/target/loongarch/cpu.h b/target/loongarch/cpu.h
index 4d7201995a..058bc53bde 100644
--- a/target/loongarch/cpu.h
+++ b/target/loongarch/cpu.h
@@ -362,6 +362,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
@@ -373,10 +379,14 @@ 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;
-    uint32_t  phy_id;
+    uint32_t  arch_id;
 
     /* 'compatible' string for this CPU for Linux device trees */
     const char *dtb_compatible;
-- 
2.39.1

[PATCH v2 06/10] Optimize loongarch_irq_init function implementation

[xianglai li]

Optimize loongarch_irq_init function implementation
and abstract the function loongarch_cpu_irq_init from it.

Cc: "Salil Mehta" <salil.mehta@opnsrc.net>
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>
Cc: Bibo Mao <maobibo@loongson.cn>
Signed-off-by: xianglai li <lixianglai@loongson.cn>
---
 hw/loongarch/virt.c         | 105 ++++++++++++++++++++----------------
 include/hw/loongarch/virt.h |   5 +-
 2 files changed, 62 insertions(+), 48 deletions(-)

diff --git a/hw/loongarch/virt.c b/hw/loongarch/virt.c
index b8474e7b94..fb06b4ab4e 100644
--- a/hw/loongarch/virt.c
+++ b/hw/loongarch/virt.c
@@ -46,6 +46,8 @@
 #include "hw/block/flash.h"
 #include "qemu/error-report.h"
 
+static LoongArchCPU *loongarch_cpu_irq_init(MachineState *machine,
+                                LoongArchCPU *cpu, Error **errp);
 
 static void virt_flash_create(LoongArchMachineState *lams)
 {
@@ -573,16 +575,16 @@ static void loongarch_devices_init(DeviceState *pch_pic, LoongArchMachineState *
 static void loongarch_irq_init(LoongArchMachineState *lams)
 {
     MachineState *ms = MACHINE(lams);
-    DeviceState *pch_pic, *pch_msi, *cpudev;
-    DeviceState *ipi, *extioi;
+    DeviceState *pch_pic, *pch_msi;
+    DeviceState *extioi;
     SysBusDevice *d;
     LoongArchCPU *lacpu;
-    CPULoongArchState *env;
     CPUState *cpu_state;
-    int cpu, pin, i, start, num;
+    int cpu, i, start, num;
 
     extioi = qdev_new(TYPE_LOONGARCH_EXTIOI);
     sysbus_realize_and_unref(SYS_BUS_DEVICE(extioi), &error_fatal);
+    lams->extioi = extioi;
 
     /*
      * The connection of interrupts:
@@ -607,44 +609,8 @@ static void loongarch_irq_init(LoongArchMachineState *lams)
      */
     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);
-
-        ipi = qdev_new(TYPE_LOONGARCH_IPI);
-        sysbus_realize_and_unref(SYS_BUS_DEVICE(ipi), &error_fatal);
-
-        /* 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 < EXTIOI_CPUS)
-            memory_region_add_subregion(&env->system_iocsr, APIC_BASE,
-                                sysbus_mmio_get_region(SYS_BUS_DEVICE(extioi),
-                                cpu));
-        env->ipistate = ipi;
-    }
-
-    /*
-     * connect ext irq to the cpu irq
-     * cpu_pin[9:2] <= intc_pin[7:0]
-     */
-    for (cpu = 0; cpu < MIN(ms->smp.cpus, EXTIOI_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));
-        }
+        loongarch_cpu_irq_init(ms, lacpu, &error_fatal);
     }
 
     pch_pic = qdev_new(TYPE_LOONGARCH_PCH_PIC);
@@ -927,11 +893,7 @@ static void loongarch_init(MachineState *machine)
         }
     }
     fdt_add_flash_node(lams);
-    /* register reset function */
-    for (i = 0; i < machine->smp.cpus; i++) {
-        lacpu = LOONGARCH_CPU(qemu_get_cpu(i));
-        qemu_register_reset(reset_load_elf, lacpu);
-    }
+
     /* Initialize the IO interrupt subsystem */
     loongarch_irq_init(lams);
     fdt_add_irqchip_node(lams);
@@ -1091,6 +1053,57 @@ static void virt_mem_plug(HotplugHandler *hotplug_dev,
                          dev, &error_abort);
 }
 
+static LoongArchCPU *loongarch_cpu_irq_init(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);
+
+    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_machine_device_plug_cb(HotplugHandler *hotplug_dev,
                                         DeviceState *dev, Error **errp)
 {
diff --git a/include/hw/loongarch/virt.h b/include/hw/loongarch/virt.h
index f1659655c6..176dc43a93 100644
--- a/include/hw/loongarch/virt.h
+++ b/include/hw/loongarch/virt.h
@@ -42,7 +42,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,9 +50,10 @@ 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;
 };
 
 #define TYPE_LOONGARCH_MACHINE  MACHINE_TYPE_NAME("virt")
-- 
2.39.1

[PATCH v2 07/10] Add basic CPU hot-(un)plug support for Loongarch

[xianglai li]

Add CPU hot-(un)plug related hook functions and
turn on the CPU hot-(un)plug custom switch.

Cc: "Salil Mehta" <salil.mehta@opnsrc.net>
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>
Cc: Bibo Mao <maobibo@loongson.cn>
Signed-off-by: xianglai li <lixianglai@loongson.cn>
---
 .../devices/loongarch64-softmmu/default.mak   |   1 +
 hw/loongarch/virt.c                           | 223 ++++++++++++++++++
 2 files changed, 224 insertions(+)

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
diff --git a/hw/loongarch/virt.c b/hw/loongarch/virt.c
index fb06b4ab4e..aabe8aa814 100644
--- a/hw/loongarch/virt.c
+++ b/hw/loongarch/virt.c
@@ -999,11 +999,93 @@ static void virt_get_cpu_topo_by_cpu_index(const MachineState *ms,
     cpu_topo->thread_id = cpu_index % ms->smp.threads;
 }
 
+/* find cpu slot in machine->possible_cpus by arch_id */
+static CPUArchId *loongarch_find_cpu_slot(MachineState *ms, int arch_id)
+{
+    int n;
+    for (n = 0; n < ms->possible_cpus->len; n++) {
+        if (ms->possible_cpus->cpus[n].arch_id == arch_id) {
+            return &ms->possible_cpus->cpus[n];
+        }
+    }
+
+    return NULL;
+}
+
+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;
+    LoongArchCPUTopo cpu_topo;
+    int arch_id;
+
+    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;
+    }
+
+    cpu_topo.socket_id = cpu->socket_id;
+    cpu_topo.core_id = cpu->core_id;
+    cpu_topo.thread_id = cpu->thread_id;
+    arch_id = virt_get_arch_id_from_cpu_topo(ms, &cpu_topo);
+
+    cpu_slot = loongarch_find_cpu_slot(ms, arch_id);
+    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->arch_id = arch_id;
+
+    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);
     }
 }
 
@@ -1017,11 +1099,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);
     }
 }
 
@@ -1035,11 +1151,87 @@ static void virt_mem_unplug(HotplugHandler *hotplug_dev,
     qdev_unrealize(dev);
 }
 
+static void loongarch_cpu_irq_uninit(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));
+
+    /* 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);
+
+    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_irq_uninit(machine, cpu);
+
+    found_cpu = loongarch_find_cpu_slot(MACHINE(lsms), cpu->arch_id);
+    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);
     }
 }
 
@@ -1104,6 +1296,33 @@ static LoongArchCPU *loongarch_cpu_irq_init(MachineState *machine,
     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);
+
+    if (lsms->acpi_ged) {
+        loongarch_cpu_irq_init(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), cpu->arch_id);
+    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)
 {
@@ -1117,6 +1336,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);
     }
 }
 
@@ -1126,6 +1347,7 @@ 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);
     }
@@ -1194,6 +1416,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;
-- 
2.39.1

[PATCH v2 08/10] Add support of *unrealize* for Loongarch cpu

[xianglai li]

Add the unrealize function to the Loongarch CPU for cpu hot-(un)plug

Cc: "Salil Mehta" <salil.mehta@opnsrc.net>
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>
Cc: Bibo Mao <maobibo@loongson.cn>
Signed-off-by: xianglai li <lixianglai@loongson.cn>
---
 target/loongarch/cpu.c | 22 ++++++++++++++++++++++
 target/loongarch/cpu.h |  1 +
 2 files changed, 23 insertions(+)

diff --git a/target/loongarch/cpu.c b/target/loongarch/cpu.c
index a5153cb3bc..03134d7bac 100644
--- a/target/loongarch/cpu.c
+++ b/target/loongarch/cpu.c
@@ -572,6 +572,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)
@@ -752,6 +770,9 @@ 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);
 
@@ -773,6 +794,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 gchar *loongarch32_gdb_arch_name(CPUState *cs)
diff --git a/target/loongarch/cpu.h b/target/loongarch/cpu.h
index 058bc53bde..4143138b12 100644
--- a/target/loongarch/cpu.h
+++ b/target/loongarch/cpu.h
@@ -412,6 +412,7 @@ struct LoongArchCPUClass {
     /*< public >*/
 
     DeviceRealize parent_realize;
+    DeviceUnrealize parent_unrealize;
     ResettablePhases parent_phases;
 };
 
-- 
2.39.1

[PATCH v2 09/10] Add generic event device for Loongarch

[xianglai li]

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

Cc: "Salil Mehta" <salil.mehta@opnsrc.net>
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>
Cc: Bibo Mao <maobibo@loongson.cn>
Signed-off-by: xianglai li <lixianglai@loongson.cn>
---
 hw/loongarch/acpi-build.c                     | 17 +++++++++
 hw/loongarch/generic_event_device_loongarch.c | 36 +++++++++++++++++++
 hw/loongarch/meson.build                      |  2 +-
 include/hw/acpi/generic_event_device.h        |  1 +
 include/hw/loongarch/virt.h                   |  4 +++
 5 files changed, 59 insertions(+), 1 deletion(-)
 create mode 100644 hw/loongarch/generic_event_device_loongarch.c

diff --git a/hw/loongarch/acpi-build.c b/hw/loongarch/acpi-build.c
index ae292fc543..66fad295cc 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)
 {
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}
diff --git a/include/hw/acpi/generic_event_device.h b/include/hw/acpi/generic_event_device.h
index d0a5a43abf..2923bd9d82 100644
--- a/include/hw/acpi/generic_event_device.h
+++ b/include/hw/acpi/generic_event_device.h
@@ -71,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
diff --git a/include/hw/loongarch/virt.h b/include/hw/loongarch/virt.h
index 176dc43a93..f6c9495af2 100644
--- a/include/hw/loongarch/virt.h
+++ b/include/hw/loongarch/virt.h
@@ -60,4 +60,8 @@ struct LoongArchMachineState {
 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
-- 
2.39.1

[PATCH v2 10/10] Update the ACPI table for the Loongarch CPU

[xianglai li]

Add new types of GED devices for Loongarch machines,
add CPU hot-(un)plug event response and address spaces,
and update the ACPI table.

Cc: "Salil Mehta" <salil.mehta@opnsrc.net>
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>
Cc: Bibo Mao <maobibo@loongson.cn>
Signed-off-by: xianglai li <lixianglai@loongson.cn>
---
 hw/acpi/acpi-cpu-hotplug-stub.c |  9 +++++++++
 hw/loongarch/acpi-build.c       | 16 +++++++++++++++-
 hw/loongarch/virt.c             |  5 +++--
 include/hw/loongarch/virt.h     |  1 +
 4 files changed, 28 insertions(+), 3 deletions(-)

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 66fad295cc..312908fb2f 100644
--- a/hw/loongarch/acpi-build.c
+++ b/hw/loongarch/acpi-build.c
@@ -138,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, 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 */
@@ -309,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/virt.c b/hw/loongarch/virt.c
index aabe8aa814..c4ec9dd6a7 100644
--- a/hw/loongarch/virt.c
+++ b/hw/loongarch/virt.c
@@ -449,12 +449,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 */
@@ -463,6 +463,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));
diff --git a/include/hw/loongarch/virt.h b/include/hw/loongarch/virt.h
index f6c9495af2..82ac238f4f 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 >*/
-- 
2.39.1

RE: [PATCH v2 00/10] Adds CPU hot-plug support to Loongarch

[Salil Mehta via]

Hi Xianglai,

> From: qemu-devel-bounces+salil.mehta=huawei.com@nongnu.org <qemu-devel-
> bounces+salil.mehta=huawei.com@nongnu.org> On Behalf Of xianglai li
> Sent: Tuesday, September 12, 2023 3:12 AM
> To: qemu-devel@nongnu.org
> Cc: Salil Mehta <salil.mehta@opnsrc.net>; Xiaojuan Yang
> <yangxiaojuan@loongson.cn>; Song Gao <gaosong@loongson.cn>; Michael S.
> Tsirkin <mst@redhat.com>; Igor Mammedov <imammedo@redhat.com>; Ani Sinha
> <anisinha@redhat.com>; Paolo Bonzini <pbonzini@redhat.com>; Richard
> Henderson <richard.henderson@linaro.org>; Eduardo Habkost
> <eduardo@habkost.net>; Marcel Apfelbaum <marcel.apfelbaum@gmail.com>;
> Philippe Mathieu-Daudé <philmd@linaro.org>; wangyanan (Y)
> <wangyanan55@huawei.com>; Daniel P. Berrangé <berrange@redhat.com>; Peter
> Xu <peterx@redhat.com>; David Hildenbrand <david@redhat.com>; Bibo Mao
> <maobibo@loongson.cn>; Xianglai li <lixianglai@loongson.cn>
> Subject: [PATCH v2 00/10] Adds CPU hot-plug support to Loongarch
> 
> 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.
> 
> For the modification of the public part of the code, we refer to the
> arm-related patch, and the link address of the corresponding patch is
> as follows:
> https://lore.kernel.org/all/20200613213629.21984-1-salil.mehta@huawei.com/
> 
> In order to respect the work of "Salil Mehta", we will rebase the first
> 4 patches in the final patch, which are referenced here to ensure that
> the loongarch cpu hotplug can work properly.


Just to let you know RFC V2 for above link is undergoing internal review
process and I will be posting the patches to community soon.

Also, I am planning to post RFC V2 as one complete patch-set initially.
(This is required to reflect the clear change from RFC V1)
This will have patches which are ARM specific and architecture common.
Later patches can be cherry picked and compiled independently.

After RFC V2 has been posted, and you have confirmed that architecture
common patches works well with your changes, I will split the RFC V2
further into 2 patch-sets,
1. Architecture common patch-set (This will come with no RFC) 
2. ARM specific patch-set  (This will continue as RFC V3)

This will help patch-set 1 getting absorbed earlier in this Qemu
cycle if everything goes well.


Thanks
Salil.


> 
> The last 6 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.
> 
> V2:
> - Fix formatting and spelling errors
> - Split large patches into smaller patches
>   - Split the original patch
>     <<Add basic CPU hot-(un)plug support for Loongarch>> into
>     <<Added CPU topology support for Loongarch>>
>     <<Optimize loongarch_irq_init function implementation >>
>     <<Add basic CPU hot-(un)plug support for Loongarch>>.
>   - Split the original patch
>     <<Update the ACPI table for the Loongarch CPU>> into
>     <<Add generic event device for Loongarch>>
>     <<Update the ACPI table for the Loongarch CPU>>
> - Added loongarch cpu topology calculation method.
> - Change the position of the cpu topology patch.
> - Change unreasonable variable and function names.
> 
> 
> 
> xianglai li (10):
>   Update ACPI GED framework to support vcpu hot-(un)plug
>   Update CPUs AML with cpu-(ctrl)dev change
>   make qdev_disconnect_gpio_out_named() public
>   Introduce the CPU address space destruction function
>   Added CPU topology support for Loongarch
>   Optimize loongarch_irq_init function implementation
>   Add basic CPU hot-(un)plug support for Loongarch
>   Add support of *unrealize* for Loongarch cpu
>   Add generic event device for Loongarch
>   Update the ACPI table for the Loongarch CPU
> 
>  .../devices/loongarch64-softmmu/default.mak   |   1 +
>  docs/system/loongarch/virt.rst                |  31 ++
>  hw/acpi/acpi-cpu-hotplug-stub.c               |  15 +
>  hw/acpi/cpu.c                                 |  27 +-
>  hw/acpi/generic_event_device.c                |  33 ++
>  hw/core/gpio.c                                |   4 +-
>  hw/i386/acpi-build.c                          |   2 +-
>  hw/loongarch/acpi-build.c                     |  33 +-
>  hw/loongarch/generic_event_device_loongarch.c |  36 ++
>  hw/loongarch/meson.build                      |   2 +-
>  hw/loongarch/virt.c                           | 424 +++++++++++++++---
>  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                   |  10 +-
>  include/hw/qdev-core.h                        |   2 +
>  softmmu/physmem.c                             |  24 +
>  target/loongarch/cpu.c                        |  35 +-
>  target/loongarch/cpu.h                        |  13 +-
>  21 files changed, 635 insertions(+), 87 deletions(-)
>  create mode 100644 hw/loongarch/generic_event_device_loongarch.c
> 
> Cc: "Salil Mehta" <salil.mehta@opnsrc.net>
> 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>
> Cc: Bibo Mao <maobibo@loongson.cn>
> Cc: Xianglai li <lixianglai@loongson.cn>
> --
> 2.39.1
> 
> 

Re: [PATCH v2 00/10] Adds CPU hot-plug support to Loongarch

[lixianglai]

Hi, Salil Mehta :
> Hi Xianglai,
>
>> From: qemu-devel-bounces+salil.mehta=huawei.com@nongnu.org <qemu-devel-
>> bounces+salil.mehta=huawei.com@nongnu.org> On Behalf Of xianglai li
>> Sent: Tuesday, September 12, 2023 3:12 AM
>> To: qemu-devel@nongnu.org
>> Cc: Salil Mehta <salil.mehta@opnsrc.net>; Xiaojuan Yang
>> <yangxiaojuan@loongson.cn>; Song Gao <gaosong@loongson.cn>; Michael S.
>> Tsirkin <mst@redhat.com>; Igor Mammedov <imammedo@redhat.com>; Ani Sinha
>> <anisinha@redhat.com>; Paolo Bonzini <pbonzini@redhat.com>; Richard
>> Henderson <richard.henderson@linaro.org>; Eduardo Habkost
>> <eduardo@habkost.net>; Marcel Apfelbaum <marcel.apfelbaum@gmail.com>;
>> Philippe Mathieu-Daudé <philmd@linaro.org>; wangyanan (Y)
>> <wangyanan55@huawei.com>; Daniel P. Berrangé <berrange@redhat.com>; Peter
>> Xu <peterx@redhat.com>; David Hildenbrand <david@redhat.com>; Bibo Mao
>> <maobibo@loongson.cn>; Xianglai li <lixianglai@loongson.cn>
>> Subject: [PATCH v2 00/10] Adds CPU hot-plug support to Loongarch
>>
>> 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.
>>
>> For the modification of the public part of the code, we refer to the
>> arm-related patch, and the link address of the corresponding patch is
>> as follows:
>> https://lore.kernel.org/all/20200613213629.21984-1-salil.mehta@huawei.com/
>>
>> In order to respect the work of "Salil Mehta", we will rebase the first
>> 4 patches in the final patch, which are referenced here to ensure that
>> the loongarch cpu hotplug can work properly.
>
> Just to let you know RFC V2 for above link is undergoing internal review
> process and I will be posting the patches to community soon.
>
> Also, I am planning to post RFC V2 as one complete patch-set initially.
> (This is required to reflect the clear change from RFC V1)
> This will have patches which are ARM specific and architecture common.
> Later patches can be cherry picked and compiled independently.
>
> After RFC V2 has been posted, and you have confirmed that architecture
> common patches works well with your changes, I will split the RFC V2
> further into 2 patch-sets,
> 1. Architecture common patch-set (This will come with no RFC)
> 2. ARM specific patch-set  (This will continue as RFC V3)
>
> This will help patch-set 1 getting absorbed earlier in this Qemu
> cycle if everything goes well.
>
>
> Thanks
> Salil.
>
That's great,Looking forward to your patch.

However, I suggest that you pay attention to the community's feedback

on patch 3 and patch 4 in the patch series I sent.

I think it may be helpful for your later patch submission.

And I'm still working on how to reply to the community.

>> The last 6 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.
>>
>> V2:
>> - Fix formatting and spelling errors
>> - Split large patches into smaller patches
>>    - Split the original patch
>>      <<Add basic CPU hot-(un)plug support for Loongarch>> into
>>      <<Added CPU topology support for Loongarch>>
>>      <<Optimize loongarch_irq_init function implementation >>
>>      <<Add basic CPU hot-(un)plug support for Loongarch>>.
>>    - Split the original patch
>>      <<Update the ACPI table for the Loongarch CPU>> into
>>      <<Add generic event device for Loongarch>>
>>      <<Update the ACPI table for the Loongarch CPU>>
>> - Added loongarch cpu topology calculation method.
>> - Change the position of the cpu topology patch.
>> - Change unreasonable variable and function names.
>>
>>
>>
>> xianglai li (10):
>>    Update ACPI GED framework to support vcpu hot-(un)plug
>>    Update CPUs AML with cpu-(ctrl)dev change
>>    make qdev_disconnect_gpio_out_named() public
>>    Introduce the CPU address space destruction function
>>    Added CPU topology support for Loongarch
>>    Optimize loongarch_irq_init function implementation
>>    Add basic CPU hot-(un)plug support for Loongarch
>>    Add support of *unrealize* for Loongarch cpu
>>    Add generic event device for Loongarch
>>    Update the ACPI table for the Loongarch CPU
>>
>>   .../devices/loongarch64-softmmu/default.mak   |   1 +
>>   docs/system/loongarch/virt.rst                |  31 ++
>>   hw/acpi/acpi-cpu-hotplug-stub.c               |  15 +
>>   hw/acpi/cpu.c                                 |  27 +-
>>   hw/acpi/generic_event_device.c                |  33 ++
>>   hw/core/gpio.c                                |   4 +-
>>   hw/i386/acpi-build.c                          |   2 +-
>>   hw/loongarch/acpi-build.c                     |  33 +-
>>   hw/loongarch/generic_event_device_loongarch.c |  36 ++
>>   hw/loongarch/meson.build                      |   2 +-
>>   hw/loongarch/virt.c                           | 424 +++++++++++++++---
>>   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                   |  10 +-
>>   include/hw/qdev-core.h                        |   2 +
>>   softmmu/physmem.c                             |  24 +
>>   target/loongarch/cpu.c                        |  35 +-
>>   target/loongarch/cpu.h                        |  13 +-
>>   21 files changed, 635 insertions(+), 87 deletions(-)
>>   create mode 100644 hw/loongarch/generic_event_device_loongarch.c
>>
>> Cc: "Salil Mehta" <salil.mehta@opnsrc.net>
>> 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>
>> Cc: Bibo Mao <maobibo@loongson.cn>
>> Cc: Xianglai li <lixianglai@loongson.cn>
>> --
>> 2.39.1
>>
>>