[Qemu-devel] [PATCH v2 06/10] acpi: cpuhp: implement hot-remove parts of CPU hotplug interface

[Igor Mammedov]

it adds hw registers needed for handling CPU hot-remove and
corresponding AML methods to request and eject a CPU with
necessary hotplug callbacks in pc,piix4,ich9 code.

Signed-off-by: Igor Mammedov <address@hidden>
---
 hw/acpi/cpu.c         | 90 ++++++++++++++++++++++++++++++++++++++++++++++++---
 hw/acpi/ich9.c        |  7 ++++
 hw/acpi/piix4.c       |  6 ++++
 hw/i386/pc.c          | 47 +++++++++++++++++++++++++++
 include/hw/acpi/cpu.h |  8 +++++
 trace-events          |  5 ++-
 6 files changed, 158 insertions(+), 5 deletions(-)

diff --git a/hw/acpi/cpu.c b/hw/acpi/cpu.c
index 811be8a..483b808 100644
--- a/hw/acpi/cpu.c
+++ b/hw/acpi/cpu.c
@@ -30,6 +30,7 @@ static uint64_t cpu_hotplug_rd(void *opaque, hwaddr addr, 
unsigned size)
     case ACPI_CPU_FLAGS_OFFSET_RW: /* pack and return is_* fields */
         val |= cdev->cpu ? 1 : 0;
         val |= cdev->is_inserting ? 2 : 0;
+        val |= cdev->is_removing  ? 4 : 0;
         trace_cpuhp_acpi_read_flags(cpu_st->selector, val);
         break;
     case ACPI_CPU_CMD_DATA_OFFSET_RW:
@@ -73,6 +74,22 @@ static void cpu_hotplug_wr(void *opaque, hwaddr addr, 
uint64_t data,
         if (data & 2) { /* clear insert event */
             cdev->is_inserting = false;
             trace_cpuhp_acpi_clear_inserting_evt(cpu_st->selector);
+        } else if (data & 4) { /* clear remove event */
+            cdev->is_removing = false;
+            trace_cpuhp_acpi_clear_remove_evt(cpu_st->selector);
+        } else if (data & 8) {
+            DeviceState *dev = NULL;
+            HotplugHandler *hotplug_ctrl = NULL;
+
+            if (!cdev->cpu) {
+                trace_cpuhp_acpi_ejecting_invalid_cpu(cpu_st->selector);
+                break;
+            }
+
+            trace_cpuhp_acpi_ejecting_cpu(cpu_st->selector);
+            dev = DEVICE(cdev->cpu);
+            hotplug_ctrl = qdev_get_hotplug_handler(dev);
+            hotplug_handler_unplug(hotplug_ctrl, dev, NULL);
         }
         break;
     case ACPI_CPU_CMD_OFFSET_WR:
@@ -84,10 +101,10 @@ static void cpu_hotplug_wr(void *opaque, hwaddr addr, 
uint64_t data,
 
                 do {
                     cdev = &cpu_st->devs[iter];
-                    if (cdev->is_inserting) {
+                    if (cdev->is_inserting || cdev->is_removing) {
                         cpu_st->selector = iter;
                         trace_cpuhp_acpi_cpu_has_events(cpu_st->selector,
-                            cdev->is_inserting);
+                            cdev->is_inserting, cdev->is_removing);
                         break;
                     }
                     iter = iter + 1 < cpu_st->dev_count ? iter + 1 : 0;
@@ -163,6 +180,34 @@ void acpi_cpu_plug_cb(HotplugHandler *hotplug_dev,
     }
 }
 
+void acpi_cpu_unplug_request_cb(HotplugHandler *hotplug_dev,
+                                CPUHotplugState *cpu_st,
+                                DeviceState *dev, Error **errp)
+{
+    AcpiCpuStatus *cdev;
+
+    cdev = get_cpu_status(cpu_st, dev);
+    if (!cdev) {
+        return;
+    }
+
+    cdev->is_removing = true;
+    acpi_send_event(DEVICE(hotplug_dev), ACPI_CPU_HOTPLUG_STATUS);
+}
+
+void acpi_cpu_unplug_cb(CPUHotplugState *cpu_st,
+                        DeviceState *dev, Error **errp)
+{
+    AcpiCpuStatus *cdev;
+
+    cdev = get_cpu_status(cpu_st, dev);
+    if (!cdev) {
+        return;
+    }
+
+    cdev->cpu = NULL;
+}
+
 static const VMStateDescription vmstate_cpuhp_sts = {
     .name = "CPU hotplug device state",
     .version_id = 1,
@@ -170,6 +215,7 @@ static const VMStateDescription vmstate_cpuhp_sts = {
     .minimum_version_id_old = 1,
     .fields      = (VMStateField[]) {
         VMSTATE_BOOL(is_inserting, AcpiCpuStatus),
+        VMSTATE_BOOL(is_removing, AcpiCpuStatus),
         VMSTATE_END_OF_LIST()
     }
 };
@@ -194,12 +240,15 @@ const VMStateDescription vmstate_cpu_hotplug = {
 #define CPU_STS_METHOD    "CSTA"
 #define CPU_SCAN_METHOD   "CSCN"
 #define CPU_NOTIFY_METHOD "CTFY"
+#define CPU_EJECT_METHOD  "CEJ0"
 
 #define CPU_ENABLED       "CPEN"
 #define CPU_SELECTOR      "CSEL"
 #define CPU_COMMAND       "CCMD"
 #define CPU_DATA          "CDAT"
 #define CPU_INSERT_EVENT  "CINS"
+#define CPU_REMOVE_EVENT  "CRMV"
+#define CPU_EJECT_EVENT   "CEJ0"
 
 void build_cpus_aml(Aml *table, MachineState *machine, CPUHotplugFeatures opts,
                     hwaddr io_base,
@@ -248,7 +297,11 @@ void build_cpus_aml(Aml *table, MachineState *machine, 
CPUHotplugFeatures opts,
         aml_append(field, aml_named_field(CPU_ENABLED, 1));
         /* (read) 1 if has a insert event. (write) 1 to clear event */
         aml_append(field, aml_named_field(CPU_INSERT_EVENT, 1));
-        aml_append(field, aml_reserved_field(6));
+        /* (read) 1 if has a remove event. (write) 1 to clear event */
+        aml_append(field, aml_named_field(CPU_REMOVE_EVENT, 1));
+        /* initiates device eject, write only */
+        aml_append(field, aml_named_field(CPU_EJECT_EVENT, 1));
+        aml_append(field, aml_reserved_field(4));
         aml_append(field, aml_named_field(CPU_COMMAND, 8));
         aml_append(cpu_ctrl_dev, field);
 
@@ -272,6 +325,8 @@ void build_cpus_aml(Aml *table, MachineState *machine, 
CPUHotplugFeatures opts,
         Aml *cpu_cmd = aml_name("%s.%s", cphp_res_path, CPU_COMMAND);
         Aml *cpu_data = aml_name("%s.%s", cphp_res_path, CPU_DATA);
         Aml *ins_evt = aml_name("%s.%s", cphp_res_path, CPU_INSERT_EVENT);
+        Aml *rm_evt = aml_name("%s.%s", cphp_res_path, CPU_REMOVE_EVENT);
+        Aml *ej_evt = aml_name("%s.%s", cphp_res_path, CPU_EJECT_EVENT);
 
         aml_append(cpus_dev, aml_name_decl("_HID", aml_string("ACPI0010")));
         aml_append(cpus_dev, aml_name_decl("_CID", aml_eisaid("PNP0A05")));
@@ -308,18 +363,31 @@ void build_cpus_aml(Aml *table, MachineState *machine, 
CPUHotplugFeatures opts,
         }
         aml_append(cpus_dev, method);
 
+        method = aml_method(CPU_EJECT_METHOD, 1, AML_SERIALIZED);
+        {
+            Aml *idx = aml_arg(0);
+
+            aml_append(method, aml_acquire(ctrl_lock, 0xFFFF));
+            aml_append(method, aml_store(idx, cpu_selector));
+            aml_append(method, aml_store(one, ej_evt));
+            aml_append(method, aml_release(ctrl_lock));
+        }
+        aml_append(cpus_dev, method);
+
         method = aml_method(CPU_SCAN_METHOD, 0, AML_SERIALIZED);
         {
+            Aml *else_ctx;
             Aml *while_ctx;
             Aml *has_event = aml_local(0);
             Aml *dev_chk = aml_int(1);
+            Aml *eject_req = aml_int(3);
             Aml *next_cpu_cmd = aml_int(CPHP_GET_NEXT_CPU_WITH_EVENT_CMD);
 
             aml_append(method, aml_acquire(ctrl_lock, 0xFFFF));
             aml_append(method, aml_store(one, has_event));
             while_ctx = aml_while(aml_equal(has_event, one));
             {
-                 /* clear loop exit condition, ins_evt check
+                 /* clear loop exit condition, ins_evt/rm_evt checks
                   * will set it to 1 while next_cpu_cmd returns a CPU
                   * with events */
                  aml_append(while_ctx, aml_store(zero, has_event));
@@ -332,6 +400,16 @@ void build_cpus_aml(Aml *table, MachineState *machine, 
CPUHotplugFeatures opts,
                      aml_append(ifctx, aml_store(one, has_event));
                  }
                  aml_append(while_ctx, ifctx);
+                 else_ctx = aml_else();
+                 ifctx = aml_if(aml_equal(rm_evt, one));
+                 {
+                     aml_append(ifctx,
+                         aml_call2(CPU_NOTIFY_METHOD, cpu_data, eject_req));
+                     aml_append(ifctx, aml_store(one, rm_evt));
+                     aml_append(ifctx, aml_store(one, has_event));
+                 }
+                 aml_append(else_ctx, ifctx);
+                 aml_append(while_ctx, else_ctx);
             }
             aml_append(method, while_ctx);
             aml_append(method, aml_release(ctrl_lock));
@@ -373,6 +451,10 @@ void build_cpus_aml(Aml *table, MachineState *machine, 
CPUHotplugFeatures opts,
                 aml_buffer(madt_buf->len, (uint8_t *)madt_buf->data)));
             g_array_free(madt_buf, true);
 
+            method = aml_method("_EJ0", 1, AML_NOTSERIALIZED);
+            aml_append(method, aml_call1(CPU_EJECT_METHOD, uid));
+            aml_append(dev, method);
+
             aml_append(cpus_dev, dev);
         }
     }
diff --git a/hw/acpi/ich9.c b/hw/acpi/ich9.c
index 9a81da8..0abe2ce 100644
--- a/hw/acpi/ich9.c
+++ b/hw/acpi/ich9.c
@@ -481,6 +481,10 @@ void ich9_pm_device_unplug_request_cb(HotplugHandler 
*hotplug_dev,
         acpi_memory_unplug_request_cb(hotplug_dev,
                                       &lpc->pm.acpi_memory_hotplug, dev,
                                       errp);
+    } else if (object_dynamic_cast(OBJECT(dev), TYPE_CPU) &&
+               !lpc->pm.cpu_hotplug_legacy) {
+        acpi_cpu_unplug_request_cb(hotplug_dev, &lpc->pm.cpuhp_state,
+                                   dev, errp);
     } else {
         error_setg(errp, "acpi: device unplug request for not supported device"
                    " type: %s", object_get_typename(OBJECT(dev)));
@@ -495,6 +499,9 @@ void ich9_pm_device_unplug_cb(HotplugHandler *hotplug_dev, 
DeviceState *dev,
     if (lpc->pm.acpi_memory_hotplug.is_enabled &&
         object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) {
         acpi_memory_unplug_cb(&lpc->pm.acpi_memory_hotplug, dev, errp);
+    } else if (object_dynamic_cast(OBJECT(dev), TYPE_CPU) &&
+               !lpc->pm.cpu_hotplug_legacy) {
+        acpi_cpu_unplug_cb(&lpc->pm.cpuhp_state, dev, errp);
     } else {
         error_setg(errp, "acpi: device unplug for not supported device"
                    " type: %s", object_get_typename(OBJECT(dev)));
diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
index 6d24cb5..8cdc1da 100644
--- a/hw/acpi/piix4.c
+++ b/hw/acpi/piix4.c
@@ -378,6 +378,9 @@ static void piix4_device_unplug_request_cb(HotplugHandler 
*hotplug_dev,
     } else if (object_dynamic_cast(OBJECT(dev), TYPE_PCI_DEVICE)) {
         acpi_pcihp_device_unplug_cb(hotplug_dev, &s->acpi_pci_hotplug, dev,
                                     errp);
+    } else if (object_dynamic_cast(OBJECT(dev), TYPE_CPU) &&
+               !s->cpu_hotplug_legacy) {
+        acpi_cpu_unplug_request_cb(hotplug_dev, &s->cpuhp_state, dev, errp);
     } else {
         error_setg(errp, "acpi: device unplug request for not supported device"
                    " type: %s", object_get_typename(OBJECT(dev)));
@@ -392,6 +395,9 @@ static void piix4_device_unplug_cb(HotplugHandler 
*hotplug_dev,
     if (s->acpi_memory_hotplug.is_enabled &&
         object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) {
         acpi_memory_unplug_cb(&s->acpi_memory_hotplug, dev, errp);
+    } else if (object_dynamic_cast(OBJECT(dev), TYPE_CPU) &&
+               !s->cpu_hotplug_legacy) {
+        acpi_cpu_unplug_cb(&s->cpuhp_state, dev, errp);
     } else {
         error_setg(errp, "acpi: device unplug for not supported device"
                    " type: %s", object_get_typename(OBJECT(dev)));
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 7198ed5..dbfba5c 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1707,6 +1707,49 @@ static void pc_cpu_plug(HotplugHandler *hotplug_dev,
 out:
     error_propagate(errp, local_err);
 }
+static void pc_cpu_unplug_request_cb(HotplugHandler *hotplug_dev,
+                                     DeviceState *dev, Error **errp)
+{
+    HotplugHandlerClass *hhc;
+    Error *local_err = NULL;
+    PCMachineState *pcms = PC_MACHINE(hotplug_dev);
+
+    hhc = HOTPLUG_HANDLER_GET_CLASS(pcms->acpi_dev);
+    hhc->unplug_request(HOTPLUG_HANDLER(pcms->acpi_dev), dev, &local_err);
+
+    if (local_err) {
+        goto out;
+    }
+
+ out:
+    error_propagate(errp, local_err);
+
+}
+
+static void pc_cpu_unplug_cb(HotplugHandler *hotplug_dev,
+                             DeviceState *dev, Error **errp)
+{
+    HotplugHandlerClass *hhc;
+    Error *local_err = NULL;
+    PCMachineState *pcms = PC_MACHINE(hotplug_dev);
+
+    hhc = HOTPLUG_HANDLER_GET_CLASS(pcms->acpi_dev);
+    hhc->unplug(HOTPLUG_HANDLER(pcms->acpi_dev), dev, &local_err);
+
+    if (local_err) {
+        goto out;
+    }
+
+    /*
+     * TODO: enable unplug once generic CPU remove bits land
+     * for now guest will be able to eject CPU ACPI wise but
+     * it will come back again on machine reset.
+     */
+    /*  object_unparent(OBJECT(dev)); */
+
+ out:
+    error_propagate(errp, local_err);
+}
 
 static void pc_machine_device_plug_cb(HotplugHandler *hotplug_dev,
                                       DeviceState *dev, Error **errp)
@@ -1723,6 +1766,8 @@ static void 
pc_machine_device_unplug_request_cb(HotplugHandler *hotplug_dev,
 {
     if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) {
         pc_dimm_unplug_request(hotplug_dev, dev, errp);
+    } else if (object_dynamic_cast(OBJECT(dev), TYPE_CPU)) {
+        pc_cpu_unplug_request_cb(hotplug_dev, dev, errp);
     } else {
         error_setg(errp, "acpi: device unplug request for not supported device"
                    " type: %s", object_get_typename(OBJECT(dev)));
@@ -1734,6 +1779,8 @@ static void pc_machine_device_unplug_cb(HotplugHandler 
*hotplug_dev,
 {
     if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) {
         pc_dimm_unplug(hotplug_dev, dev, errp);
+    } else if (object_dynamic_cast(OBJECT(dev), TYPE_CPU)) {
+        pc_cpu_unplug_cb(hotplug_dev, dev, errp);
     } else {
         error_setg(errp, "acpi: device unplug for not supported device"
                    " type: %s", object_get_typename(OBJECT(dev)));
diff --git a/include/hw/acpi/cpu.h b/include/hw/acpi/cpu.h
index 55c3166..f334221 100644
--- a/include/hw/acpi/cpu.h
+++ b/include/hw/acpi/cpu.h
@@ -21,6 +21,7 @@ typedef struct AcpiCpuStatus {
     struct CPUState *cpu;
     uint64_t arch_id;
     bool is_inserting;
+    bool is_removing;
 } AcpiCpuStatus;
 
 typedef struct CPUHotplugState {
@@ -34,6 +35,13 @@ typedef struct CPUHotplugState {
 void acpi_cpu_plug_cb(HotplugHandler *hotplug_dev,
                       CPUHotplugState *cpu_st, DeviceState *dev, Error **errp);
 
+void acpi_cpu_unplug_request_cb(HotplugHandler *hotplug_dev,
+                                CPUHotplugState *cpu_st,
+                                DeviceState *dev, Error **errp);
+
+void acpi_cpu_unplug_cb(CPUHotplugState *cpu_st,
+                        DeviceState *dev, Error **errp);
+
 void cpu_hotplug_hw_init(MemoryRegion *as, Object *owner,
                          CPUHotplugState *state, hwaddr base_addr);
 
diff --git a/trace-events b/trace-events
index fac1f51..53ad387 100644
--- a/trace-events
+++ b/trace-events
@@ -2171,5 +2171,8 @@ cpuhp_acpi_read_flags(uint32_t idx, uint8_t flags) 
"idx[0x%"PRIx32"] flags: 0x%"
 cpuhp_acpi_write_idx(uint32_t idx) "set active cpu idx: 0x%"PRIx32
 cpuhp_acpi_write_cmd(uint32_t idx, uint8_t cmd) "idx[0x%"PRIx32"] cmd: 
0x%"PRIx8
 cpuhp_acpi_read_cmd_data(uint32_t idx, uint32_t data) "idx[0x%"PRIx32"] data: 
0x%"PRIx32
-cpuhp_acpi_cpu_has_events(uint32_t idx, bool ins) "idx[0x%"PRIx32"] inserting: 
%d"
+cpuhp_acpi_cpu_has_events(uint32_t idx, bool ins, bool rm) "idx[0x%"PRIx32"] 
inserting: %d, removing: %d"
 cpuhp_acpi_clear_inserting_evt(uint32_t idx) "idx[0x%"PRIx32"]"
+cpuhp_acpi_clear_remove_evt(uint32_t idx) "idx[0x%"PRIx32"]"
+cpuhp_acpi_ejecting_invalid_cpu(uint32_t idx) "0x%"PRIx32
+cpuhp_acpi_ejecting_cpu(uint32_t idx) "0x%"PRIx32
-- 
1.8.3.1