[Qemu-devel] [RESEND PATCH v1 00/13] QEmu memory hot unplug support.

[Qemu-devel] [RESEND PATCH v1 00/13] QEmu memory hot unplug support.

[Tang Chen]

Memory hot unplug are both asynchronize procedures.
When the unplug operation happens, unplug request cb is called first.
And when ghest OS finished handling unplug, unplug cb will be called
to do the real removal of device.

This patch-set is based on QEmu 2.2

This series depends on the following patchset.
[PATCH] Common unplug and unplug request cb for memory and CPU hot-unplug.
https://www.mail-archive.com/qemu-devel@nongnu.org/msg272745.html

Hu Tao (2):
  acpi, piix4: Add memory hot unplug request support for piix4.
  pc, acpi bios: Add memory hot unplug interface.

Tang Chen (11):
  acpi, mem-hotplug: Use PC_DIMM_SLOT_PROP in acpi_memory_plug_cb().
  acpi, mem-hotplug: Add acpi_memory_get_slot_status_descriptor() to get
    MemStatus.
  acpi, mem-hotplug: Add acpi_memory_hotplug_sci() to rise sci for
    memory hotplug.
  acpi, mem-hotplug: Add unplug request cb for memory device.
  acpi, ich9: Add memory hot unplug request support for ich9.
  pc-dimm: Add memory hot unplug request support for pc-dimm.
  acpi, mem-hotplug: Add unplug cb for memory device.
  acpi, piix4: Add memory hot unplug support for piix4.
  acpi, ich9: Add memory hot unplug support for ich9.
  pc-dimm: Add memory hot unplug support for pc-dimm.
  acpi: Add hardware implementation for memory hot unplug.

 docs/specs/acpi_mem_hotplug.txt   |  8 +++-
 hw/acpi/ich9.c                    | 20 ++++++--
 hw/acpi/memory_hotplug.c          | 97 ++++++++++++++++++++++++++++++++-------
 hw/acpi/piix4.c                   | 18 ++++++--
 hw/core/qdev.c                    |  2 +-
 hw/i386/acpi-dsdt-mem-hotplug.dsl | 11 ++++-
 hw/i386/pc.c                      | 53 +++++++++++++++++++--
 hw/i386/ssdt-mem.dsl              |  5 ++
 include/hw/acpi/memory_hotplug.h  |  6 +++
 include/hw/acpi/pc-hotplug.h      |  2 +
 include/hw/qdev-core.h            |  1 +
 11 files changed, 192 insertions(+), 31 deletions(-)

-- 
1.8.4.2

[Qemu-devel] [RESEND PATCH v1 01/13] acpi, mem-hotplug: Use PC_DIMM_SLOT_PROP in acpi_memory_plug_cb().

[Tang Chen]

Replace string "slot" in acpi_memory_plug_cb() with MACRO PC_DIMM_SLOT_PROP.

Signed-off-by: Tang Chen <tangchen@cn.fujitsu.com>
---
 hw/acpi/memory_hotplug.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/acpi/memory_hotplug.c b/hw/acpi/memory_hotplug.c
index ed39241..c6580da 100644
--- a/hw/acpi/memory_hotplug.c
+++ b/hw/acpi/memory_hotplug.c
@@ -168,7 +168,8 @@ void acpi_memory_plug_cb(ACPIREGS *ar, qemu_irq irq, MemHotplugState *mem_st,
 {
     MemStatus *mdev;
     Error *local_err = NULL;
-    int slot = object_property_get_int(OBJECT(dev), "slot", &local_err);
+    int slot = object_property_get_int(OBJECT(dev), PC_DIMM_SLOT_PROP,
+                                       &local_err);
 
     if (local_err) {
         error_propagate(errp, local_err);
-- 
1.8.4.2

[Qemu-devel] [RESEND PATCH v1 02/13] acpi, mem-hotplug: Add acpi_memory_get_slot_status_descriptor() to get MemStatus.

[Tang Chen]

Add a new API named acpi_memory_get_slot_status_descriptor() to obtain
a single memory slot status. Doing this is because this procedure will
be used by other functions in the next coming patches.

Signed-off-by: Tang Chen <tangchen@cn.fujitsu.com>
---
 hw/acpi/memory_hotplug.c | 27 +++++++++++++++++++--------
 1 file changed, 19 insertions(+), 8 deletions(-)

diff --git a/hw/acpi/memory_hotplug.c b/hw/acpi/memory_hotplug.c
index c6580da..ef56bf6 100644
--- a/hw/acpi/memory_hotplug.c
+++ b/hw/acpi/memory_hotplug.c
@@ -163,29 +163,40 @@ void acpi_memory_hotplug_init(MemoryRegion *as, Object *owner,
     memory_region_add_subregion(as, ACPI_MEMORY_HOTPLUG_BASE, &state->io);
 }
 
-void acpi_memory_plug_cb(ACPIREGS *ar, qemu_irq irq, MemHotplugState *mem_st,
-                         DeviceState *dev, Error **errp)
+static MemStatus *
+acpi_memory_get_slot_status_descriptor(MemHotplugState *mem_st,
+                                       DeviceState *dev, Error **errp)
 {
-    MemStatus *mdev;
     Error *local_err = NULL;
     int slot = object_property_get_int(OBJECT(dev), PC_DIMM_SLOT_PROP,
                                        &local_err);
 
     if (local_err) {
         error_propagate(errp, local_err);
-        return;
+        return NULL;
     }
 
     if (slot >= mem_st->dev_count) {
         char *dev_path = object_get_canonical_path(OBJECT(dev));
-        error_setg(errp, "acpi_memory_plug_cb: "
+        error_setg(errp, "acpi_memory_get_slot_status_descriptor: "
                    "device [%s] returned invalid memory slot[%d]",
-                    dev_path, slot);
+                   dev_path, slot);
         g_free(dev_path);
-        return;
+        return NULL;
     }
 
-    mdev = &mem_st->devs[slot];
+    return &mem_st->devs[slot];
+}
+
+void acpi_memory_plug_cb(ACPIREGS *ar, qemu_irq irq, MemHotplugState *mem_st,
+                         DeviceState *dev, Error **errp)
+{
+    MemStatus *mdev;
+
+    mdev = acpi_memory_get_slot_status_descriptor(mem_st, dev, errp);
+    if (!mdev)
+        return;
+
     mdev->dimm = dev;
     mdev->is_enabled = true;
     mdev->is_inserting = true;
-- 
1.8.4.2

[Qemu-devel] [RESEND PATCH v1 03/13] acpi, mem-hotplug: Add acpi_memory_hotplug_sci() to rise sci for memory hotplug.

[Tang Chen]

Add a new API named acpi_memory_hotplug_sci() to send memory hotplug SCI.
Doing this is because this procedure will be used by other functions in the
next coming patches.

Signed-off-by: Tang Chen <tangchen@cn.fujitsu.com>
---
 hw/acpi/memory_hotplug.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/hw/acpi/memory_hotplug.c b/hw/acpi/memory_hotplug.c
index ef56bf6..9839963 100644
--- a/hw/acpi/memory_hotplug.c
+++ b/hw/acpi/memory_hotplug.c
@@ -188,6 +188,12 @@ acpi_memory_get_slot_status_descriptor(MemHotplugState *mem_st,
     return &mem_st->devs[slot];
 }
 
+static void acpi_memory_hotplug_sci(ACPIREGS *ar, qemu_irq irq)
+{
+    ar->gpe.sts[0] |= ACPI_MEMORY_HOTPLUG_STATUS;
+    acpi_update_sci(ar, irq);
+}
+
 void acpi_memory_plug_cb(ACPIREGS *ar, qemu_irq irq, MemHotplugState *mem_st,
                          DeviceState *dev, Error **errp)
 {
@@ -201,10 +207,8 @@ void acpi_memory_plug_cb(ACPIREGS *ar, qemu_irq irq, MemHotplugState *mem_st,
     mdev->is_enabled = true;
     mdev->is_inserting = true;
 
-    /* do ACPI magic */
-    ar->gpe.sts[0] |= ACPI_MEMORY_HOTPLUG_STATUS;
-    acpi_update_sci(ar, irq);
-    return;
+    /* Do ACPI magic */
+    acpi_memory_hotplug_sci(ar, irq);
 }
 
 static const VMStateDescription vmstate_memhp_sts = {
-- 
1.8.4.2

[Qemu-devel] [RESEND PATCH v1 04/13] acpi, mem-hotplug: Add unplug request cb for memory device.

[Tang Chen]

Memory hot unplug are both asynchronize procedures.
When the unplug operation happens, unplug request cb is called first.
And when ghest OS finished handling unplug, unplug cb will be called
to do the real removal of device.

This patch adds unplug request cb for memory device. Add a new bool
member named is_removing to MemStatus indicating that the memory slot
is being removed. Set it to true in acpi_memory_unplug_request_cb(),
and send SCI to guest.

Signed-off-by: Tang Chen <tangchen@cn.fujitsu.com>
---
 hw/acpi/memory_hotplug.c         | 16 ++++++++++++++++
 include/hw/acpi/memory_hotplug.h |  4 ++++
 2 files changed, 20 insertions(+)

diff --git a/hw/acpi/memory_hotplug.c b/hw/acpi/memory_hotplug.c
index 9839963..3d8e398 100644
--- a/hw/acpi/memory_hotplug.c
+++ b/hw/acpi/memory_hotplug.c
@@ -211,6 +211,22 @@ void acpi_memory_plug_cb(ACPIREGS *ar, qemu_irq irq, MemHotplugState *mem_st,
     acpi_memory_hotplug_sci(ar, irq);
 }
 
+void acpi_memory_unplug_request_cb(ACPIREGS *ar, qemu_irq irq,
+                                   MemHotplugState *mem_st,
+                                   DeviceState *dev, Error **errp)
+{
+    MemStatus *mdev;
+
+    mdev = acpi_memory_get_slot_status_descriptor(mem_st, dev, errp);
+    if (!mdev)
+        return;
+
+    mdev->is_removing = true;
+
+    /* Do ACPI magic */
+    acpi_memory_hotplug_sci(ar, irq);
+}
+
 static const VMStateDescription vmstate_memhp_sts = {
     .name = "memory hotplug device state",
     .version_id = 1,
diff --git a/include/hw/acpi/memory_hotplug.h b/include/hw/acpi/memory_hotplug.h
index 7bbf8a0..c437a85 100644
--- a/include/hw/acpi/memory_hotplug.h
+++ b/include/hw/acpi/memory_hotplug.h
@@ -11,6 +11,7 @@ typedef struct MemStatus {
     DeviceState *dimm;
     bool is_enabled;
     bool is_inserting;
+    bool is_removing;
     uint32_t ost_event;
     uint32_t ost_status;
 } MemStatus;
@@ -28,6 +29,9 @@ void acpi_memory_hotplug_init(MemoryRegion *as, Object *owner,
 
 void acpi_memory_plug_cb(ACPIREGS *ar, qemu_irq irq, MemHotplugState *mem_st,
                          DeviceState *dev, Error **errp);
+void acpi_memory_unplug_request_cb(ACPIREGS *ar, qemu_irq irq,
+                                   MemHotplugState *mem_st,
+                                   DeviceState *dev, Error **errp);
 
 extern const VMStateDescription vmstate_memory_hotplug;
 #define VMSTATE_MEMORY_HOTPLUG(memhp, state) \
-- 
1.8.4.2

[Qemu-devel] [RESEND PATCH v1 05/13] acpi, piix4: Add memory hot unplug request support for piix4.

[Tang Chen]

From: Hu Tao <hutao@cn.fujitsu.com>

Call memory unplug request cb in piix4_device_unplug_request_cb().

Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
Signed-off-by: Tang Chen <tangchen@cn.fujitsu.com>
---
 hw/acpi/piix4.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
index 4407388..f809c3a 100644
--- a/hw/acpi/piix4.c
+++ b/hw/acpi/piix4.c
@@ -361,7 +361,11 @@ static void piix4_device_unplug_request_cb(HotplugHandler *hotplug_dev,
 {
     PIIX4PMState *s = PIIX4_PM(hotplug_dev);
 
-    if (object_dynamic_cast(OBJECT(dev), TYPE_PCI_DEVICE)) {
+    if (s->acpi_memory_hotplug.is_enabled &&
+        object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) {
+        acpi_memory_unplug_request_cb(&s->ar, s->irq, &s->acpi_memory_hotplug,
+                                      dev, errp);
+    } else if (object_dynamic_cast(OBJECT(dev), TYPE_PCI_DEVICE)) {
         acpi_pcihp_device_unplug_cb(&s->ar, s->irq, &s->acpi_pci_hotplug, dev,
                                     errp);
     } else {
-- 
1.8.4.2

[Qemu-devel] [RESEND PATCH v1 06/13] acpi, ich9: Add memory hot unplug request support for ich9.

[Tang Chen]

Call memory unplug request cb in ich9_pm_device_unplug_request_cb().
---
 hw/acpi/ich9.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/hw/acpi/ich9.c b/hw/acpi/ich9.c
index c48d176..841f57d 100644
--- a/hw/acpi/ich9.c
+++ b/hw/acpi/ich9.c
@@ -304,8 +304,14 @@ void ich9_pm_device_plug_cb(ICH9LPCPMRegs *pm, DeviceState *dev, Error **errp)
 void ich9_pm_device_unplug_request_cb(ICH9LPCPMRegs *pm, DeviceState *dev,
                                       Error **errp)
 {
-    error_setg(errp, "acpi: device unplug request for not supported device"
-               " type: %s", object_get_typename(OBJECT(dev)));
+    if (pm->acpi_memory_hotplug.is_enabled &&
+        object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) {
+        acpi_memory_unplug_request_cb(&pm->acpi_regs, pm->irq,
+                                      &pm->acpi_memory_hotplug, dev, errp);
+    } else {
+        error_setg(errp, "acpi: device unplug request for not supported device"
+                   " type: %s", object_get_typename(OBJECT(dev)));
+    }
 }
 
 void ich9_pm_device_unplug_cb(ICH9LPCPMRegs *pm, DeviceState *dev,
-- 
1.8.4.2

[Qemu-devel] [RESEND PATCH v1 07/13] pc-dimm: Add memory hot unplug request support for pc-dimm.

[Tang Chen]

Implement memory unplug request cb for pc-dimm, and call it in
pc_machine_device_unplug_request_cb().
---
 hw/i386/pc.c | 28 ++++++++++++++++++++++++++--
 1 file changed, 26 insertions(+), 2 deletions(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 0f3b1e0..f501f1f 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1671,6 +1671,26 @@ out:
     error_propagate(errp, local_err);
 }
 
+static void pc_dimm_unplug_request(HotplugHandler *hotplug_dev,
+                                   DeviceState *dev, Error **errp)
+{
+    HotplugHandlerClass *hhc;
+    Error *local_err = NULL;
+    PCMachineState *pcms = PC_MACHINE(hotplug_dev);
+
+    if (!pcms->acpi_dev) {
+        error_setg(&local_err,
+                   "memory hotplug is not enabled: missing acpi device");
+	goto out;
+    }
+
+    hhc = HOTPLUG_HANDLER_GET_CLASS(pcms->acpi_dev);
+    hhc->unplug_request(HOTPLUG_HANDLER(pcms->acpi_dev), dev, &local_err);
+
+out:
+    error_propagate(errp, local_err);
+}
+
 static void pc_cpu_plug(HotplugHandler *hotplug_dev,
                         DeviceState *dev, Error **errp)
 {
@@ -1713,8 +1733,12 @@ static void pc_machine_device_plug_cb(HotplugHandler *hotplug_dev,
 static void pc_machine_device_unplug_request_cb(HotplugHandler *hotplug_dev,
                                                 DeviceState *dev, Error **errp)
 {
-    error_setg(errp, "acpi: device unplug request for not supported device"
-               " type: %s", object_get_typename(OBJECT(dev)));
+    if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) {
+        pc_dimm_unplug_request(hotplug_dev, dev, errp);
+    } else {
+        error_setg(errp, "acpi: device unplug request for not supported device"
+                   " type: %s", object_get_typename(OBJECT(dev)));
+    }
 }
 
 static void pc_machine_device_unplug_cb(HotplugHandler *hotplug_dev,
-- 
1.8.4.2

[Qemu-devel] [RESEND PATCH v1 08/13] acpi, mem-hotplug: Add unplug cb for memory device.

[Tang Chen]

Reset all memory status, and unparent the memory device.
---
 hw/acpi/memory_hotplug.c         | 16 ++++++++++++++++
 include/hw/acpi/memory_hotplug.h |  2 ++
 2 files changed, 18 insertions(+)

diff --git a/hw/acpi/memory_hotplug.c b/hw/acpi/memory_hotplug.c
index 3d8e398..2b0c8ca 100644
--- a/hw/acpi/memory_hotplug.c
+++ b/hw/acpi/memory_hotplug.c
@@ -227,6 +227,22 @@ void acpi_memory_unplug_request_cb(ACPIREGS *ar, qemu_irq irq,
     acpi_memory_hotplug_sci(ar, irq);
 }
 
+void acpi_memory_unplug_cb(ACPIREGS *ar, qemu_irq irq,
+                           MemHotplugState *mem_st,
+                           DeviceState *dev, Error **errp)
+{
+    MemStatus *mdev;
+
+    mdev = acpi_memory_get_slot_status_descriptor(mem_st, dev, errp);
+    if (!mdev)
+        return;
+
+    mdev->is_removing = false;
+    mdev->is_enabled = false;
+    object_unparent(OBJECT(mdev->dimm));
+    mdev->dimm = NULL;
+}
+
 static const VMStateDescription vmstate_memhp_sts = {
     .name = "memory hotplug device state",
     .version_id = 1,
diff --git a/include/hw/acpi/memory_hotplug.h b/include/hw/acpi/memory_hotplug.h
index c437a85..6b8d9f7 100644
--- a/include/hw/acpi/memory_hotplug.h
+++ b/include/hw/acpi/memory_hotplug.h
@@ -32,6 +32,8 @@ void acpi_memory_plug_cb(ACPIREGS *ar, qemu_irq irq, MemHotplugState *mem_st,
 void acpi_memory_unplug_request_cb(ACPIREGS *ar, qemu_irq irq,
                                    MemHotplugState *mem_st,
                                    DeviceState *dev, Error **errp);
+void acpi_memory_unplug_cb(ACPIREGS *ar, qemu_irq irq, MemHotplugState *mem_st,
+                           DeviceState *dev, Error **errp);
 
 extern const VMStateDescription vmstate_memory_hotplug;
 #define VMSTATE_MEMORY_HOTPLUG(memhp, state) \
-- 
1.8.4.2

[Qemu-devel] [RESEND PATCH v1 09/13] acpi, piix4: Add memory hot unplug support for piix4.

[Tang Chen]

Call memory unplug cb in piix4_device_unplug_cb().
---
 hw/acpi/piix4.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
index f809c3a..4ae4867 100644
--- a/hw/acpi/piix4.c
+++ b/hw/acpi/piix4.c
@@ -377,8 +377,16 @@ static void piix4_device_unplug_request_cb(HotplugHandler *hotplug_dev,
 static void piix4_device_unplug_cb(HotplugHandler *hotplug_dev,
                                    DeviceState *dev, Error **errp)
 {
-    error_setg(errp, "acpi: device unplug for not supported device"
-               " type: %s", object_get_typename(OBJECT(dev)));
+    PIIX4PMState *s = PIIX4_PM(hotplug_dev);
+
+    if (s->acpi_memory_hotplug.is_enabled &&
+        object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) {
+        acpi_memory_unplug_cb(&s->ar, s->irq, &s->acpi_memory_hotplug,
+                              dev, errp);
+    } else {
+        error_setg(errp, "acpi: device unplug for not supported device"
+                   " type: %s", object_get_typename(OBJECT(dev)));
+    }
 }
 
 static void piix4_update_bus_hotplug(PCIBus *pci_bus, void *opaque)
-- 
1.8.4.2

[Qemu-devel] [RESEND PATCH v1 10/13] acpi, ich9: Add memory hot unplug support for ich9.

[Tang Chen]

Call memory unplug cb in ich9_pm_device_unplug_cb().
---
 hw/acpi/ich9.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/hw/acpi/ich9.c b/hw/acpi/ich9.c
index 841f57d..0a8e757 100644
--- a/hw/acpi/ich9.c
+++ b/hw/acpi/ich9.c
@@ -317,8 +317,14 @@ void ich9_pm_device_unplug_request_cb(ICH9LPCPMRegs *pm, DeviceState *dev,
 void ich9_pm_device_unplug_cb(ICH9LPCPMRegs *pm, DeviceState *dev,
                               Error **errp)
 {
-    error_setg(errp, "acpi: device unplug for not supported device"
-               " type: %s", object_get_typename(OBJECT(dev)));
+    if (pm->acpi_memory_hotplug.is_enabled &&
+        object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) {
+        acpi_memory_unplug_cb(&pm->acpi_regs, pm->irq,
+                              &pm->acpi_memory_hotplug, dev, errp);
+    } else {
+        error_setg(errp, "acpi: device unplug for not supported device"
+                   " type: %s", object_get_typename(OBJECT(dev)));
+    }
 }
 
 void ich9_pm_ospm_status(AcpiDeviceIf *adev, ACPIOSTInfoList ***list)
-- 
1.8.4.2

[Qemu-devel] [RESEND PATCH v1 11/13] pc-dimm: Add memory hot unplug support for pc-dimm.

[Tang Chen]

Implement unplug cb for pc-dimm. It remove the corresponding
memory region, and unregister vmstat. At last, it calls memory
unplug cb to reset memory status and do unparenting.
---
 hw/i386/pc.c | 25 +++++++++++++++++++++++--
 1 file changed, 23 insertions(+), 2 deletions(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index f501f1f..3732f67 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1691,6 +1691,23 @@ out:
     error_propagate(errp, local_err);
 }
 
+static void pc_dimm_unplug(HotplugHandler *hotplug_dev,
+                           DeviceState *dev, Error **errp)
+{
+    PCMachineState *pcms = PC_MACHINE(hotplug_dev);
+    PCDIMMDevice *dimm = PC_DIMM(dev);
+    PCDIMMDeviceClass *ddc = PC_DIMM_GET_CLASS(dimm);
+    MemoryRegion *mr = ddc->get_memory_region(dimm);
+    HotplugHandlerClass *hhc;
+    Error *local_err = NULL;
+
+    memory_region_del_subregion(&pcms->hotplug_memory, mr);
+    vmstate_unregister_ram(mr, dev);
+
+    hhc = HOTPLUG_HANDLER_GET_CLASS(pcms->acpi_dev);
+    hhc->unplug(HOTPLUG_HANDLER(pcms->acpi_dev), dev, &local_err);
+}
+
 static void pc_cpu_plug(HotplugHandler *hotplug_dev,
                         DeviceState *dev, Error **errp)
 {
@@ -1744,8 +1761,12 @@ static void pc_machine_device_unplug_request_cb(HotplugHandler *hotplug_dev,
 static void pc_machine_device_unplug_cb(HotplugHandler *hotplug_dev,
                                         DeviceState *dev, Error **errp)
 {
-    error_setg(errp, "acpi: device unplug for not supported device"
-               " type: %s", object_get_typename(OBJECT(dev)));
+    if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) {
+        pc_dimm_unplug(hotplug_dev, dev, errp);
+    } else {
+        error_setg(errp, "acpi: device unplug for not supported device"
+                   " type: %s", object_get_typename(OBJECT(dev)));
+    }
 }
 
 static HotplugHandler *pc_get_hotpug_handler(MachineState *machine,
-- 
1.8.4.2

[Qemu-devel] [RESEND PATCH v1 12/13] acpi: Add hardware implementation for memory hot unplug.

[Tang Chen]

This patch adds a new bit to memory hotplug IO port indicating that
ej0 has been evaluated by guest OS. And call pc-dimm unplug cb to do
the real removal.


Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
Signed-off-by: Tang Chen <tangchen@cn.fujitsu.com>
---
 docs/specs/acpi_mem_hotplug.txt |  8 ++++++--
 hw/acpi/memory_hotplug.c        | 23 ++++++++++++++++++++---
 hw/core/qdev.c                  |  2 +-
 include/hw/qdev-core.h          |  1 +
 4 files changed, 28 insertions(+), 6 deletions(-)

diff --git a/docs/specs/acpi_mem_hotplug.txt b/docs/specs/acpi_mem_hotplug.txt
index 1290994..28a1ffa 100644
--- a/docs/specs/acpi_mem_hotplug.txt
+++ b/docs/specs/acpi_mem_hotplug.txt
@@ -19,7 +19,9 @@ Memory hot-plug interface (IO port 0xa00-0xa17, 1-4 byte access):
               1: Device insert event, used to distinguish device for which
                  no device check event to OSPM was issued.
                  It's valid only when bit 1 is set.
-              2-7: reserved and should be ignored by OSPM
+              2: Device remove event, used to indicate that device is being
+		 removed.
+              3-7: reserved and should be ignored by OSPM
       [0x15-0x17] reserved
 
   write access:
@@ -35,7 +37,9 @@ Memory hot-plug interface (IO port 0xa00-0xa17, 1-4 byte access):
               1: if set to 1 clears device insert event, set by OSPM
                  after it has emitted device check event for the
                  selected memory device
-              2-7: reserved, OSPM must clear them before writing to register
+              2: set by hardware after it has emitted device eject event for
+		 selected memory device
+              3-7: reserved, OSPM must clear them before writing to register
 
 Selecting memory device slot beyond present range has no effect on platform:
    - write accesses to memory hot-plug registers not documented above are
diff --git a/hw/acpi/memory_hotplug.c b/hw/acpi/memory_hotplug.c
index 2b0c8ca..b9e8752 100644
--- a/hw/acpi/memory_hotplug.c
+++ b/hw/acpi/memory_hotplug.c
@@ -2,6 +2,7 @@
 #include "hw/acpi/pc-hotplug.h"
 #include "hw/mem/pc-dimm.h"
 #include "hw/boards.h"
+#include "hw/qdev-core.h"
 #include "trace.h"
 #include "qapi-event.h"
 
@@ -75,6 +76,7 @@ static uint64_t acpi_memory_hotplug_read(void *opaque, hwaddr addr,
     case 0x14: /* pack and return is_* fields */
         val |= mdev->is_enabled   ? 1 : 0;
         val |= mdev->is_inserting ? 2 : 0;
+        val |= mdev->is_removing  ? 4 : 0;
         trace_mhp_acpi_read_flags(mem_st->selector, val);
         break;
     default:
@@ -90,6 +92,8 @@ static void acpi_memory_hotplug_write(void *opaque, hwaddr addr, uint64_t data,
     MemHotplugState *mem_st = opaque;
     MemStatus *mdev;
     ACPIOSTInfo *info;
+    DeviceState *dev = NULL;
+    HotplugHandler *hotplug_ctrl = NULL;
 
     if (!mem_st->dev_count) {
         return;
@@ -121,21 +125,34 @@ static void acpi_memory_hotplug_write(void *opaque, hwaddr addr, uint64_t data,
         mdev = &mem_st->devs[mem_st->selector];
         mdev->ost_status = data;
         trace_mhp_acpi_write_ost_status(mem_st->selector, mdev->ost_status);
-        /* TODO: implement memory removal on guest signal */
 
         info = acpi_memory_device_status(mem_st->selector, mdev);
         qapi_event_send_acpi_device_ost(info, &error_abort);
         qapi_free_ACPIOSTInfo(info);
         break;
-    case 0x14:
+    case 0x14: /* set is_* fields */
         mdev = &mem_st->devs[mem_st->selector];
+
         if (data & 2) { /* clear insert event */
             mdev->is_inserting  = false;
             trace_mhp_acpi_clear_insert_evt(mem_st->selector);
+        } else if (data & 4) { /* request removal of device */
+            /*
+             * QEmu memory hot unplug is an asynchronized procedure. QEmu first
+             * calls pc-dimm unplug request cb to send a SCI to guest. When the
+             * Guest OS finished handling the SCI, it evaluates ACPI ej0, and
+             * QEmu calls pc-dimm unplug cb to remove memory device.
+             */
+            dev = DEVICE(mdev->dimm);
+            hotplug_ctrl = qdev_get_hotplug_handler(dev);
+            /* Call pc-dimm unplug cb. */
+            hotplug_handler_unplug(hotplug_ctrl, dev, NULL);
         }
+
+        break;
+    default:
         break;
     }
-
 }
 static const MemoryRegionOps acpi_memory_hotplug_ops = {
     .read = acpi_memory_hotplug_read,
diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index 901f289..9f08fe6 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -223,7 +223,7 @@ void qdev_set_legacy_instance_id(DeviceState *dev, int alias_id,
     dev->alias_required_for_version = required_for_version;
 }
 
-static HotplugHandler *qdev_get_hotplug_handler(DeviceState *dev)
+HotplugHandler *qdev_get_hotplug_handler(DeviceState *dev)
 {
     HotplugHandler *hotplug_ctrl = NULL;
 
diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index 589bbe7..60f549e 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -261,6 +261,7 @@ void qdev_init_nofail(DeviceState *dev);
 void qdev_set_legacy_instance_id(DeviceState *dev, int alias_id,
                                  int required_for_version);
 void qdev_unplug(DeviceState *dev, Error **errp);
+HotplugHandler *qdev_get_hotplug_handler(DeviceState *dev);
 void qdev_simple_device_unplug_cb(HotplugHandler *hotplug_dev,
                                   DeviceState *dev, Error **errp);
 void qdev_machine_creation_done(void);
-- 
1.8.4.2

[Qemu-devel] [RESEND PATCH v1 13/13] pc, acpi bios: Add memory hot unplug interface.

[Tang Chen]

From: Hu Tao <hutao@cn.fujitsu.com>

This patch implements MEMORY_SLOT_EJECT_METHOD according to ACPI spec.

Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
Signed-off-by: Tang Chen <tangchen@cn.fujitsu.com>
---
 hw/i386/acpi-dsdt-mem-hotplug.dsl | 11 ++++++++++-
 hw/i386/ssdt-mem.dsl              |  5 +++++
 include/hw/acpi/pc-hotplug.h      |  2 ++
 3 files changed, 17 insertions(+), 1 deletion(-)

diff --git a/hw/i386/acpi-dsdt-mem-hotplug.dsl b/hw/i386/acpi-dsdt-mem-hotplug.dsl
index 2a36c47..2a35534 100644
--- a/hw/i386/acpi-dsdt-mem-hotplug.dsl
+++ b/hw/i386/acpi-dsdt-mem-hotplug.dsl
@@ -50,6 +50,7 @@
                 Offset(20),
                 MEMORY_SLOT_ENABLED,  1, // 1 if enabled, read only
                 MEMORY_SLOT_INSERT_EVENT, 1, // (read) 1 if has a insert event. (write) 1 to clear event
+                MEMORY_SLOT_REMOVE_EVENT, 1, // 1 if DIMM has a remove request, read only
             }
 
             Mutex (MEMORY_SLOT_LOCK, 0)
@@ -71,8 +72,9 @@
                     If (LEqual(MEMORY_SLOT_INSERT_EVENT, One)) { // Memory device needs check
                         MEMORY_SLOT_NOTIFY_METHOD(Local0, 1)
                         Store(1, MEMORY_SLOT_INSERT_EVENT)
+                    } Elseif (LEqual(MEMORY_SLOT_REMOVE_EVENT, One)) { // Ejection request
+                        MEMORY_SLOT_NOTIFY_METHOD(Local0, 3)
                     }
-                    // TODO: handle memory eject request
                     Add(Local0, One, Local0) // goto next DIMM
                 }
                 Release(MEMORY_SLOT_LOCK)
@@ -172,5 +174,12 @@
                 Store(Arg2, MEMORY_SLOT_OST_STATUS)
                 Release(MEMORY_SLOT_LOCK)
             }
+
+            Method(MEMORY_SLOT_EJECT_METHOD, 2) {
+                Acquire(MEMORY_SLOT_LOCK, 0xFFFF)
+                Store(ToInteger(Arg0), MEMORY_SLOT_SLECTOR) // select DIMM
+                Store(One, MEMORY_SLOT_REMOVE_EVENT)
+                Release(MEMORY_SLOT_LOCK)
+            }
         } // Device()
     } // Scope()
diff --git a/hw/i386/ssdt-mem.dsl b/hw/i386/ssdt-mem.dsl
index 22ff5dd..1416639 100644
--- a/hw/i386/ssdt-mem.dsl
+++ b/hw/i386/ssdt-mem.dsl
@@ -43,6 +43,7 @@ DefinitionBlock ("ssdt-mem.aml", "SSDT", 0x02, "BXPC", "CSSDT", 0x1)
     External(\_SB.PCI0.MEMORY_HOTPLUG_DEVICE.MEMORY_SLOT_STATUS_METHOD, MethodObj)
     External(\_SB.PCI0.MEMORY_HOTPLUG_DEVICE.MEMORY_SLOT_OST_METHOD, MethodObj)
     External(\_SB.PCI0.MEMORY_HOTPLUG_DEVICE.MEMORY_SLOT_PROXIMITY_METHOD, MethodObj)
+    External(\_SB.PCI0.MEMORY_HOTPLUG_DEVICE.MEMORY_SLOT_EJECT_METHOD, MethodObj)
 
     Scope(\_SB) {
 /*  v------------------ DO NOT EDIT ------------------v */
@@ -72,6 +73,10 @@ DefinitionBlock ("ssdt-mem.aml", "SSDT", 0x02, "BXPC", "CSSDT", 0x1)
             Method(_OST, 3) {
                 \_SB.PCI0.MEMORY_HOTPLUG_DEVICE.MEMORY_SLOT_OST_METHOD(_UID, Arg0, Arg1, Arg2)
             }
+
+            Method(_EJ0, 1) {
+                \_SB.PCI0.MEMORY_HOTPLUG_DEVICE.MEMORY_SLOT_EJECT_METHOD(_UID, Arg0)
+            }
         }
     }
 }
diff --git a/include/hw/acpi/pc-hotplug.h b/include/hw/acpi/pc-hotplug.h
index b9db295..b61b6ea 100644
--- a/include/hw/acpi/pc-hotplug.h
+++ b/include/hw/acpi/pc-hotplug.h
@@ -42,6 +42,7 @@
 #define MEMORY_SLOT_PROXIMITY        MPX
 #define MEMORY_SLOT_ENABLED          MES
 #define MEMORY_SLOT_INSERT_EVENT     MINS
+#define MEMORY_SLOT_REMOVE_EVENT     MRMV
 #define MEMORY_SLOT_SLECTOR          MSEL
 #define MEMORY_SLOT_OST_EVENT        MOEV
 #define MEMORY_SLOT_OST_STATUS       MOSC
@@ -50,6 +51,7 @@
 #define MEMORY_SLOT_CRS_METHOD       MCRS
 #define MEMORY_SLOT_OST_METHOD       MOST
 #define MEMORY_SLOT_PROXIMITY_METHOD MPXM
+#define MEMORY_SLOT_EJECT_METHOD     MEJ0
 #define MEMORY_SLOT_NOTIFY_METHOD    MTFY
 #define MEMORY_SLOT_SCAN_METHOD      MSCN
 
-- 
1.8.4.2

Re: [Qemu-devel] [RESEND PATCH v1 01/13] acpi, mem-hotplug: Use PC_DIMM_SLOT_PROP in acpi_memory_plug_cb().

[Igor Mammedov]

On Thu, 8 Jan 2015 09:06:08 +0800
Tang Chen <tangchen@cn.fujitsu.com> wrote:

> Replace string "slot" in acpi_memory_plug_cb() with MACRO PC_DIMM_SLOT_PROP.
> 
> Signed-off-by: Tang Chen <tangchen@cn.fujitsu.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>

> ---
>  hw/acpi/memory_hotplug.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/acpi/memory_hotplug.c b/hw/acpi/memory_hotplug.c
> index ed39241..c6580da 100644
> --- a/hw/acpi/memory_hotplug.c
> +++ b/hw/acpi/memory_hotplug.c
> @@ -168,7 +168,8 @@ void acpi_memory_plug_cb(ACPIREGS *ar, qemu_irq irq, MemHotplugState *mem_st,
>  {
>      MemStatus *mdev;
>      Error *local_err = NULL;
> -    int slot = object_property_get_int(OBJECT(dev), "slot", &local_err);
> +    int slot = object_property_get_int(OBJECT(dev), PC_DIMM_SLOT_PROP,
> +                                       &local_err);
>  
>      if (local_err) {
>          error_propagate(errp, local_err);

Re: [Qemu-devel] [RESEND PATCH v1 02/13] acpi, mem-hotplug: Add acpi_memory_get_slot_status_descriptor() to get MemStatus.

[Igor Mammedov]

On Thu, 8 Jan 2015 09:06:09 +0800
Tang Chen <tangchen@cn.fujitsu.com> wrote:

> Add a new API named acpi_memory_get_slot_status_descriptor() to obtain
> a single memory slot status. Doing this is because this procedure will
> be used by other functions in the next coming patches.
> 
> Signed-off-by: Tang Chen <tangchen@cn.fujitsu.com>
> ---
>  hw/acpi/memory_hotplug.c | 27 +++++++++++++++++++--------
>  1 file changed, 19 insertions(+), 8 deletions(-)
> 
> diff --git a/hw/acpi/memory_hotplug.c b/hw/acpi/memory_hotplug.c
> index c6580da..ef56bf6 100644
> --- a/hw/acpi/memory_hotplug.c
> +++ b/hw/acpi/memory_hotplug.c
> @@ -163,29 +163,40 @@ void acpi_memory_hotplug_init(MemoryRegion *as, Object *owner,
>      memory_region_add_subregion(as, ACPI_MEMORY_HOTPLUG_BASE, &state->io);
>  }
>  
> -void acpi_memory_plug_cb(ACPIREGS *ar, qemu_irq irq, MemHotplugState *mem_st,
> -                         DeviceState *dev, Error **errp)
> +static MemStatus *
> +acpi_memory_get_slot_status_descriptor(MemHotplugState *mem_st,
> +                                       DeviceState *dev, Error **errp)
how about
  s/acpi_memory_get_slot_status_descriptor/acpi_memory_slot_status/


>  {
> -    MemStatus *mdev;
>      Error *local_err = NULL;
>      int slot = object_property_get_int(OBJECT(dev), PC_DIMM_SLOT_PROP,
>                                         &local_err);
>  
>      if (local_err) {
>          error_propagate(errp, local_err);
> -        return;
> +        return NULL;
>      }
>  
>      if (slot >= mem_st->dev_count) {
>          char *dev_path = object_get_canonical_path(OBJECT(dev));
> -        error_setg(errp, "acpi_memory_plug_cb: "
> +        error_setg(errp, "acpi_memory_get_slot_status_descriptor: "
>                     "device [%s] returned invalid memory slot[%d]",
> -                    dev_path, slot);
> +                   dev_path, slot);
>          g_free(dev_path);
> -        return;
> +        return NULL;
>      }
>  
> -    mdev = &mem_st->devs[slot];
> +    return &mem_st->devs[slot];
> +}
> +
> +void acpi_memory_plug_cb(ACPIREGS *ar, qemu_irq irq, MemHotplugState *mem_st,
> +                         DeviceState *dev, Error **errp)
> +{
> +    MemStatus *mdev;
> +
> +    mdev = acpi_memory_get_slot_status_descriptor(mem_st, dev, errp);
> +    if (!mdev)
> +        return;
> +
>      mdev->dimm = dev;
>      mdev->is_enabled = true;
>      mdev->is_inserting = true;

Re: [Qemu-devel] [RESEND PATCH v1 03/13] acpi, mem-hotplug: Add acpi_memory_hotplug_sci() to rise sci for memory hotplug.

[Igor Mammedov]

On Thu, 8 Jan 2015 09:06:10 +0800
Tang Chen <tangchen@cn.fujitsu.com> wrote:

> Add a new API named acpi_memory_hotplug_sci() to send memory hotplug SCI.
> Doing this is because this procedure will be used by other functions in the
> next coming patches.
> 
> Signed-off-by: Tang Chen <tangchen@cn.fujitsu.com>
> ---
>  hw/acpi/memory_hotplug.c | 12 ++++++++----
>  1 file changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/acpi/memory_hotplug.c b/hw/acpi/memory_hotplug.c
> index ef56bf6..9839963 100644
> --- a/hw/acpi/memory_hotplug.c
> +++ b/hw/acpi/memory_hotplug.c
> @@ -188,6 +188,12 @@ acpi_memory_get_slot_status_descriptor(MemHotplugState *mem_st,
>      return &mem_st->devs[slot];
>  }
>  
> +static void acpi_memory_hotplug_sci(ACPIREGS *ar, qemu_irq irq)
> +{
> +    ar->gpe.sts[0] |= ACPI_MEMORY_HOTPLUG_STATUS;
> +    acpi_update_sci(ar, irq);
> +}
I'd suggest do a generic:

acpi_send_gpe_event(ar, irq, ACPI_MEMORY_HOTPLUG_STATUS)

which could be reused by CPU and PCI hotplug as well

>  void acpi_memory_plug_cb(ACPIREGS *ar, qemu_irq irq, MemHotplugState *mem_st,
>                           DeviceState *dev, Error **errp)
>  {
> @@ -201,10 +207,8 @@ void acpi_memory_plug_cb(ACPIREGS *ar, qemu_irq irq, MemHotplugState *mem_st,
>      mdev->is_enabled = true;
>      mdev->is_inserting = true;
>  
> -    /* do ACPI magic */
> -    ar->gpe.sts[0] |= ACPI_MEMORY_HOTPLUG_STATUS;
> -    acpi_update_sci(ar, irq);
> -    return;
> +    /* Do ACPI magic */
> +    acpi_memory_hotplug_sci(ar, irq);
>  }
>  
>  static const VMStateDescription vmstate_memhp_sts = {

Re: [Qemu-devel] [RESEND PATCH v1 07/13] pc-dimm: Add memory hot unplug request support for pc-dimm.

[Igor Mammedov]

On Thu, 8 Jan 2015 09:06:14 +0800
Tang Chen <tangchen@cn.fujitsu.com> wrote:

> Implement memory unplug request cb for pc-dimm, and call it in
> pc_machine_device_unplug_request_cb().
> ---
>  hw/i386/pc.c | 28 ++++++++++++++++++++++++++--
>  1 file changed, 26 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 0f3b1e0..f501f1f 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -1671,6 +1671,26 @@ out:
>      error_propagate(errp, local_err);
>  }
>  
> +static void pc_dimm_unplug_request(HotplugHandler *hotplug_dev,
> +                                   DeviceState *dev, Error **errp)
> +{
> +    HotplugHandlerClass *hhc;
> +    Error *local_err = NULL;
> +    PCMachineState *pcms = PC_MACHINE(hotplug_dev);
> +
> +    if (!pcms->acpi_dev) {
> +        error_setg(&local_err,
> +                   "memory hotplug is not enabled: missing acpi device");
> +	goto out;
wrong indent

> +    }
> +
> +    hhc = HOTPLUG_HANDLER_GET_CLASS(pcms->acpi_dev);
> +    hhc->unplug_request(HOTPLUG_HANDLER(pcms->acpi_dev), dev, &local_err);
> +
> +out:
> +    error_propagate(errp, local_err);
> +}
> +
>  static void pc_cpu_plug(HotplugHandler *hotplug_dev,
>                          DeviceState *dev, Error **errp)
>  {
> @@ -1713,8 +1733,12 @@ static void pc_machine_device_plug_cb(HotplugHandler *hotplug_dev,
>  static void pc_machine_device_unplug_request_cb(HotplugHandler *hotplug_dev,
>                                                  DeviceState *dev, Error **errp)
>  {
> -    error_setg(errp, "acpi: device unplug request for not supported device"
> -               " type: %s", object_get_typename(OBJECT(dev)));
> +    if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) {
> +        pc_dimm_unplug_request(hotplug_dev, dev, errp);
> +    } else {
> +        error_setg(errp, "acpi: device unplug request for not supported device"
> +                   " type: %s", object_get_typename(OBJECT(dev)));
> +    }
>  }
>  
>  static void pc_machine_device_unplug_cb(HotplugHandler *hotplug_dev,

Re: [Qemu-devel] [RESEND PATCH v1 09/13] acpi, piix4: Add memory hot unplug support for piix4.

[Igor Mammedov]

On Thu, 8 Jan 2015 09:06:16 +0800
Tang Chen <tangchen@cn.fujitsu.com> wrote:

> Call memory unplug cb in piix4_device_unplug_cb().
> ---
>  hw/acpi/piix4.c | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
> index f809c3a..4ae4867 100644
> --- a/hw/acpi/piix4.c
> +++ b/hw/acpi/piix4.c
> @@ -377,8 +377,16 @@ static void piix4_device_unplug_request_cb(HotplugHandler *hotplug_dev,
>  static void piix4_device_unplug_cb(HotplugHandler *hotplug_dev,
>                                     DeviceState *dev, Error **errp)
>  {
> -    error_setg(errp, "acpi: device unplug for not supported device"
> -               " type: %s", object_get_typename(OBJECT(dev)));
> +    PIIX4PMState *s = PIIX4_PM(hotplug_dev);
> +
> +    if (s->acpi_memory_hotplug.is_enabled &&
is above check really necessary? If yes, do it inside common acpi_memory_unplug_cb()

> +        object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) {
> +        acpi_memory_unplug_cb(&s->ar, s->irq, &s->acpi_memory_hotplug,
> +                              dev, errp);
> +    } else {
> +        error_setg(errp, "acpi: device unplug for not supported device"
> +                   " type: %s", object_get_typename(OBJECT(dev)));
> +    }
>  }
>  
>  static void piix4_update_bus_hotplug(PCIBus *pci_bus, void *opaque)

Re: [Qemu-devel] [RESEND PATCH v1 08/13] acpi, mem-hotplug: Add unplug cb for memory device.

[Igor Mammedov]

On Thu, 8 Jan 2015 09:06:15 +0800
Tang Chen <tangchen@cn.fujitsu.com> wrote:

> Reset all memory status, and unparent the memory device.
> ---
>  hw/acpi/memory_hotplug.c         | 16 ++++++++++++++++
>  include/hw/acpi/memory_hotplug.h |  2 ++
>  2 files changed, 18 insertions(+)
> 
> diff --git a/hw/acpi/memory_hotplug.c b/hw/acpi/memory_hotplug.c
> index 3d8e398..2b0c8ca 100644
> --- a/hw/acpi/memory_hotplug.c
> +++ b/hw/acpi/memory_hotplug.c
> @@ -227,6 +227,22 @@ void acpi_memory_unplug_request_cb(ACPIREGS *ar, qemu_irq irq,
>      acpi_memory_hotplug_sci(ar, irq);
>  }
>  
> +void acpi_memory_unplug_cb(ACPIREGS *ar, qemu_irq irq,
> +                           MemHotplugState *mem_st,
> +                           DeviceState *dev, Error **errp)
> +{
> +    MemStatus *mdev;
> +
> +    mdev = acpi_memory_get_slot_status_descriptor(mem_st, dev, errp);
> +    if (!mdev)
> +        return;
> +
> +    mdev->is_removing = false;
should be done in acpi_memory_hotplug_write() in the same manner
as it's done for is_inserting field


> +    mdev->is_enabled = false;
> +    object_unparent(OBJECT(mdev->dimm));
it seem wrong, does acpi_memory_plug_cb() do parenting?

Unparenting probably should be done as the last step by PC_MACHINE
when it's cleaned up all connections to dimm.

> +    mdev->dimm = NULL;
> +}
> +
>  static const VMStateDescription vmstate_memhp_sts = {
>      .name = "memory hotplug device state",
>      .version_id = 1,
> diff --git a/include/hw/acpi/memory_hotplug.h b/include/hw/acpi/memory_hotplug.h
> index c437a85..6b8d9f7 100644
> --- a/include/hw/acpi/memory_hotplug.h
> +++ b/include/hw/acpi/memory_hotplug.h
> @@ -32,6 +32,8 @@ void acpi_memory_plug_cb(ACPIREGS *ar, qemu_irq irq, MemHotplugState *mem_st,
>  void acpi_memory_unplug_request_cb(ACPIREGS *ar, qemu_irq irq,
>                                     MemHotplugState *mem_st,
>                                     DeviceState *dev, Error **errp);
> +void acpi_memory_unplug_cb(ACPIREGS *ar, qemu_irq irq, MemHotplugState *mem_st,
> +                           DeviceState *dev, Error **errp);
>  
>  extern const VMStateDescription vmstate_memory_hotplug;
>  #define VMSTATE_MEMORY_HOTPLUG(memhp, state) \

Re: [Qemu-devel] [RESEND PATCH v1 12/13] acpi: Add hardware implementation for memory hot unplug.

[Igor Mammedov]

On Thu, 8 Jan 2015 09:06:19 +0800
Tang Chen <tangchen@cn.fujitsu.com> wrote:

> This patch adds a new bit to memory hotplug IO port indicating that
> ej0 has been evaluated by guest OS. And call pc-dimm unplug cb to do
> the real removal.
> 
> 
> Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
> Signed-off-by: Tang Chen <tangchen@cn.fujitsu.com>
> ---
>  docs/specs/acpi_mem_hotplug.txt |  8 ++++++--
>  hw/acpi/memory_hotplug.c        | 23 ++++++++++++++++++++---
>  hw/core/qdev.c                  |  2 +-
>  include/hw/qdev-core.h          |  1 +
>  4 files changed, 28 insertions(+), 6 deletions(-)
> 
> diff --git a/docs/specs/acpi_mem_hotplug.txt b/docs/specs/acpi_mem_hotplug.txt
> index 1290994..28a1ffa 100644
> --- a/docs/specs/acpi_mem_hotplug.txt
> +++ b/docs/specs/acpi_mem_hotplug.txt
> @@ -19,7 +19,9 @@ Memory hot-plug interface (IO port 0xa00-0xa17, 1-4 byte access):
>                1: Device insert event, used to distinguish device for which
>                   no device check event to OSPM was issued.
>                   It's valid only when bit 1 is set.
> -              2-7: reserved and should be ignored by OSPM
> +              2: Device remove event, used to indicate that device is being
> +		 removed.
bit has the same semantic as bit 1,
Please update description to match it.

> +              3-7: reserved and should be ignored by OSPM
>        [0x15-0x17] reserved
>  
>    write access:
> @@ -35,7 +37,9 @@ Memory hot-plug interface (IO port 0xa00-0xa17, 1-4 byte access):
>                1: if set to 1 clears device insert event, set by OSPM
>                   after it has emitted device check event for the
>                   selected memory device
> -              2-7: reserved, OSPM must clear them before writing to register
> +              2: set by hardware after it has emitted device eject event for
> +		 selected memory device
same here

> +              3-7: reserved, OSPM must clear them before writing to register
>  
>  Selecting memory device slot beyond present range has no effect on platform:
>     - write accesses to memory hot-plug registers not documented above are
> diff --git a/hw/acpi/memory_hotplug.c b/hw/acpi/memory_hotplug.c
> index 2b0c8ca..b9e8752 100644
> --- a/hw/acpi/memory_hotplug.c
> +++ b/hw/acpi/memory_hotplug.c
> @@ -2,6 +2,7 @@
>  #include "hw/acpi/pc-hotplug.h"
>  #include "hw/mem/pc-dimm.h"
>  #include "hw/boards.h"
> +#include "hw/qdev-core.h"
>  #include "trace.h"
>  #include "qapi-event.h"
>  
> @@ -75,6 +76,7 @@ static uint64_t acpi_memory_hotplug_read(void *opaque, hwaddr addr,
>      case 0x14: /* pack and return is_* fields */
>          val |= mdev->is_enabled   ? 1 : 0;
>          val |= mdev->is_inserting ? 2 : 0;
> +        val |= mdev->is_removing  ? 4 : 0;
>          trace_mhp_acpi_read_flags(mem_st->selector, val);
>          break;
>      default:
> @@ -90,6 +92,8 @@ static void acpi_memory_hotplug_write(void *opaque, hwaddr addr, uint64_t data,
>      MemHotplugState *mem_st = opaque;
>      MemStatus *mdev;
>      ACPIOSTInfo *info;
> +    DeviceState *dev = NULL;
> +    HotplugHandler *hotplug_ctrl = NULL;
>  
>      if (!mem_st->dev_count) {
>          return;
> @@ -121,21 +125,34 @@ static void acpi_memory_hotplug_write(void *opaque, hwaddr addr, uint64_t data,
>          mdev = &mem_st->devs[mem_st->selector];
>          mdev->ost_status = data;
>          trace_mhp_acpi_write_ost_status(mem_st->selector, mdev->ost_status);
> -        /* TODO: implement memory removal on guest signal */
>  
>          info = acpi_memory_device_status(mem_st->selector, mdev);
>          qapi_event_send_acpi_device_ost(info, &error_abort);
>          qapi_free_ACPIOSTInfo(info);
>          break;
> -    case 0x14:
> +    case 0x14: /* set is_* fields */
>          mdev = &mem_st->devs[mem_st->selector];
> +
>          if (data & 2) { /* clear insert event */
>              mdev->is_inserting  = false;
>              trace_mhp_acpi_clear_insert_evt(mem_st->selector);
> +        } else if (data & 4) { /* request removal of device */
add here a tracing event similar to above one ^^^

> +            /*
> +             * QEmu memory hot unplug is an asynchronized procedure. QEmu first
> +             * calls pc-dimm unplug request cb to send a SCI to guest. When the
> +             * Guest OS finished handling the SCI, it evaluates ACPI ej0, and
> +             * QEmu calls pc-dimm unplug cb to remove memory device.
> +             */
> +            dev = DEVICE(mdev->dimm);
> +            hotplug_ctrl = qdev_get_hotplug_handler(dev);
> +            /* Call pc-dimm unplug cb. */
> +            hotplug_handler_unplug(hotplug_ctrl, dev, NULL);
>          }
> +
> +        break;
> +    default:
>          break;
>      }
> -
>  }
>  static const MemoryRegionOps acpi_memory_hotplug_ops = {
>      .read = acpi_memory_hotplug_read,
> diff --git a/hw/core/qdev.c b/hw/core/qdev.c
> index 901f289..9f08fe6 100644
> --- a/hw/core/qdev.c
> +++ b/hw/core/qdev.c
> @@ -223,7 +223,7 @@ void qdev_set_legacy_instance_id(DeviceState *dev, int alias_id,
>      dev->alias_required_for_version = required_for_version;
>  }
>  
> -static HotplugHandler *qdev_get_hotplug_handler(DeviceState *dev)
> +HotplugHandler *qdev_get_hotplug_handler(DeviceState *dev)
>  {
>      HotplugHandler *hotplug_ctrl = NULL;
>  
> diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
> index 589bbe7..60f549e 100644
> --- a/include/hw/qdev-core.h
> +++ b/include/hw/qdev-core.h
> @@ -261,6 +261,7 @@ void qdev_init_nofail(DeviceState *dev);
>  void qdev_set_legacy_instance_id(DeviceState *dev, int alias_id,
>                                   int required_for_version);
>  void qdev_unplug(DeviceState *dev, Error **errp);
> +HotplugHandler *qdev_get_hotplug_handler(DeviceState *dev);
>  void qdev_simple_device_unplug_cb(HotplugHandler *hotplug_dev,
>                                    DeviceState *dev, Error **errp);
>  void qdev_machine_creation_done(void);

Re: [Qemu-devel] [RESEND PATCH v1 13/13] pc, acpi bios: Add memory hot unplug interface.

[Igor Mammedov]

On Thu, 8 Jan 2015 09:06:20 +0800
Tang Chen <tangchen@cn.fujitsu.com> wrote:

> From: Hu Tao <hutao@cn.fujitsu.com>
> 
> This patch implements MEMORY_SLOT_EJECT_METHOD according to ACPI spec.
squash this patch in previous one

> 
> Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
> Signed-off-by: Tang Chen <tangchen@cn.fujitsu.com>
> ---
>  hw/i386/acpi-dsdt-mem-hotplug.dsl | 11 ++++++++++-
>  hw/i386/ssdt-mem.dsl              |  5 +++++
>  include/hw/acpi/pc-hotplug.h      |  2 ++
>  3 files changed, 17 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/i386/acpi-dsdt-mem-hotplug.dsl b/hw/i386/acpi-dsdt-mem-hotplug.dsl
> index 2a36c47..2a35534 100644
> --- a/hw/i386/acpi-dsdt-mem-hotplug.dsl
> +++ b/hw/i386/acpi-dsdt-mem-hotplug.dsl
> @@ -50,6 +50,7 @@
>                  Offset(20),
>                  MEMORY_SLOT_ENABLED,  1, // 1 if enabled, read only
>                  MEMORY_SLOT_INSERT_EVENT, 1, // (read) 1 if has a insert event. (write) 1 to clear event
> +                MEMORY_SLOT_REMOVE_EVENT, 1, // 1 if DIMM has a remove request, read only
it's not just remove event, you write into it to make eject,
so update it to match MEMORY_SLOT_INSERT_EVENT description with what happens
on write.

>              }
>  
>              Mutex (MEMORY_SLOT_LOCK, 0)
> @@ -71,8 +72,9 @@
>                      If (LEqual(MEMORY_SLOT_INSERT_EVENT, One)) { // Memory device needs check
>                          MEMORY_SLOT_NOTIFY_METHOD(Local0, 1)
>                          Store(1, MEMORY_SLOT_INSERT_EVENT)
> +                    } Elseif (LEqual(MEMORY_SLOT_REMOVE_EVENT, One)) { // Ejection request
> +                        MEMORY_SLOT_NOTIFY_METHOD(Local0, 3)
>                      }
> -                    // TODO: handle memory eject request
>                      Add(Local0, One, Local0) // goto next DIMM
>                  }
>                  Release(MEMORY_SLOT_LOCK)
> @@ -172,5 +174,12 @@
>                  Store(Arg2, MEMORY_SLOT_OST_STATUS)
>                  Release(MEMORY_SLOT_LOCK)
>              }
> +
> +            Method(MEMORY_SLOT_EJECT_METHOD, 2) {
> +                Acquire(MEMORY_SLOT_LOCK, 0xFFFF)
> +                Store(ToInteger(Arg0), MEMORY_SLOT_SLECTOR) // select DIMM
> +                Store(One, MEMORY_SLOT_REMOVE_EVENT)
> +                Release(MEMORY_SLOT_LOCK)
> +            }
>          } // Device()
>      } // Scope()
> diff --git a/hw/i386/ssdt-mem.dsl b/hw/i386/ssdt-mem.dsl
> index 22ff5dd..1416639 100644
> --- a/hw/i386/ssdt-mem.dsl
> +++ b/hw/i386/ssdt-mem.dsl
> @@ -43,6 +43,7 @@ DefinitionBlock ("ssdt-mem.aml", "SSDT", 0x02, "BXPC", "CSSDT", 0x1)
>      External(\_SB.PCI0.MEMORY_HOTPLUG_DEVICE.MEMORY_SLOT_STATUS_METHOD, MethodObj)
>      External(\_SB.PCI0.MEMORY_HOTPLUG_DEVICE.MEMORY_SLOT_OST_METHOD, MethodObj)
>      External(\_SB.PCI0.MEMORY_HOTPLUG_DEVICE.MEMORY_SLOT_PROXIMITY_METHOD, MethodObj)
> +    External(\_SB.PCI0.MEMORY_HOTPLUG_DEVICE.MEMORY_SLOT_EJECT_METHOD, MethodObj)
>  
>      Scope(\_SB) {
>  /*  v------------------ DO NOT EDIT ------------------v */
> @@ -72,6 +73,10 @@ DefinitionBlock ("ssdt-mem.aml", "SSDT", 0x02, "BXPC", "CSSDT", 0x1)
>              Method(_OST, 3) {
>                  \_SB.PCI0.MEMORY_HOTPLUG_DEVICE.MEMORY_SLOT_OST_METHOD(_UID, Arg0, Arg1, Arg2)
>              }
> +
> +            Method(_EJ0, 1) {
> +                \_SB.PCI0.MEMORY_HOTPLUG_DEVICE.MEMORY_SLOT_EJECT_METHOD(_UID, Arg0)
> +            }
>          }
>      }
>  }
> diff --git a/include/hw/acpi/pc-hotplug.h b/include/hw/acpi/pc-hotplug.h
> index b9db295..b61b6ea 100644
> --- a/include/hw/acpi/pc-hotplug.h
> +++ b/include/hw/acpi/pc-hotplug.h
> @@ -42,6 +42,7 @@
>  #define MEMORY_SLOT_PROXIMITY        MPX
>  #define MEMORY_SLOT_ENABLED          MES
>  #define MEMORY_SLOT_INSERT_EVENT     MINS
> +#define MEMORY_SLOT_REMOVE_EVENT     MRMV
>  #define MEMORY_SLOT_SLECTOR          MSEL
>  #define MEMORY_SLOT_OST_EVENT        MOEV
>  #define MEMORY_SLOT_OST_STATUS       MOSC
> @@ -50,6 +51,7 @@
>  #define MEMORY_SLOT_CRS_METHOD       MCRS
>  #define MEMORY_SLOT_OST_METHOD       MOST
>  #define MEMORY_SLOT_PROXIMITY_METHOD MPXM
> +#define MEMORY_SLOT_EJECT_METHOD     MEJ0
>  #define MEMORY_SLOT_NOTIFY_METHOD    MTFY
>  #define MEMORY_SLOT_SCAN_METHOD      MSCN
>  

Re: [Qemu-devel] [RESEND PATCH v1 00/13] QEmu memory hot unplug support.

[Igor Mammedov]

On Thu, 8 Jan 2015 09:06:07 +0800
Tang Chen <tangchen@cn.fujitsu.com> wrote:

> Memory hot unplug are both asynchronize procedures.
> When the unplug operation happens, unplug request cb is called first.
> And when ghest OS finished handling unplug, unplug cb will be called
> to do the real removal of device.
> 
> This patch-set is based on QEmu 2.2
> 
> This series depends on the following patchset.
> [PATCH] Common unplug and unplug request cb for memory and CPU hot-unplug.
> https://www.mail-archive.com/qemu-devel@nongnu.org/msg272745.html
> 
> Hu Tao (2):
>   acpi, piix4: Add memory hot unplug request support for piix4.
>   pc, acpi bios: Add memory hot unplug interface.
> 
> Tang Chen (11):
>   acpi, mem-hotplug: Use PC_DIMM_SLOT_PROP in acpi_memory_plug_cb().
>   acpi, mem-hotplug: Add acpi_memory_get_slot_status_descriptor() to get
>     MemStatus.
>   acpi, mem-hotplug: Add acpi_memory_hotplug_sci() to rise sci for
>     memory hotplug.

>   acpi, mem-hotplug: Add unplug request cb for memory device.
>   acpi, ich9: Add memory hot unplug request support for ich9.
>   pc-dimm: Add memory hot unplug request support for pc-dimm.
squash this patches together

>   acpi, mem-hotplug: Add unplug cb for memory device.
>   acpi, piix4: Add memory hot unplug support for piix4.
>   acpi, ich9: Add memory hot unplug support for ich9.
squash this patches together

>   pc-dimm: Add memory hot unplug support for pc-dimm.
>   acpi: Add hardware implementation for memory hot unplug.
squash this patches together

> 
>  docs/specs/acpi_mem_hotplug.txt   |  8 +++-
>  hw/acpi/ich9.c                    | 20 ++++++--
>  hw/acpi/memory_hotplug.c          | 97 ++++++++++++++++++++++++++++++++-------
>  hw/acpi/piix4.c                   | 18 ++++++--
>  hw/core/qdev.c                    |  2 +-
>  hw/i386/acpi-dsdt-mem-hotplug.dsl | 11 ++++-
>  hw/i386/pc.c                      | 53 +++++++++++++++++++--
>  hw/i386/ssdt-mem.dsl              |  5 ++
>  include/hw/acpi/memory_hotplug.h  |  6 +++
>  include/hw/acpi/pc-hotplug.h      |  2 +
>  include/hw/qdev-core.h            |  1 +
>  11 files changed, 192 insertions(+), 31 deletions(-)
> 

Re: [Qemu-devel] [RESEND PATCH v1 11/13] pc-dimm: Add memory hot unplug support for pc-dimm.

[Igor Mammedov]

On Thu, 8 Jan 2015 09:06:18 +0800
Tang Chen <tangchen@cn.fujitsu.com> wrote:

> Implement unplug cb for pc-dimm. It remove the corresponding
> memory region, and unregister vmstat. At last, it calls memory
> unplug cb to reset memory status and do unparenting.
> ---
>  hw/i386/pc.c | 25 +++++++++++++++++++++++--
>  1 file changed, 23 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index f501f1f..3732f67 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -1691,6 +1691,23 @@ out:
>      error_propagate(errp, local_err);
>  }
>  
> +static void pc_dimm_unplug(HotplugHandler *hotplug_dev,
> +                           DeviceState *dev, Error **errp)
> +{
> +    PCMachineState *pcms = PC_MACHINE(hotplug_dev);
> +    PCDIMMDevice *dimm = PC_DIMM(dev);
> +    PCDIMMDeviceClass *ddc = PC_DIMM_GET_CLASS(dimm);
> +    MemoryRegion *mr = ddc->get_memory_region(dimm);
> +    HotplugHandlerClass *hhc;
> +    Error *local_err = NULL;
> +
> +    memory_region_del_subregion(&pcms->hotplug_memory, mr);
> +    vmstate_unregister_ram(mr, dev);
I'd first cleanup external state by calling acpi unplug handler
and only the do above.

> +
> +    hhc = HOTPLUG_HANDLER_GET_CLASS(pcms->acpi_dev);
> +    hhc->unplug(HOTPLUG_HANDLER(pcms->acpi_dev), dev, &local_err);
> +}
> +
>  static void pc_cpu_plug(HotplugHandler *hotplug_dev,
>                          DeviceState *dev, Error **errp)
>  {
> @@ -1744,8 +1761,12 @@ static void pc_machine_device_unplug_request_cb(HotplugHandler *hotplug_dev,
>  static void pc_machine_device_unplug_cb(HotplugHandler *hotplug_dev,
>                                          DeviceState *dev, Error **errp)
>  {
> -    error_setg(errp, "acpi: device unplug for not supported device"
> -               " type: %s", object_get_typename(OBJECT(dev)));
> +    if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) {
> +        pc_dimm_unplug(hotplug_dev, dev, errp);
> +    } else {
> +        error_setg(errp, "acpi: device unplug for not supported device"
> +                   " type: %s", object_get_typename(OBJECT(dev)));
> +    }
>  }
>  
>  static HotplugHandler *pc_get_hotpug_handler(MachineState *machine,

Re: [Qemu-devel] [RESEND PATCH v1 00/13] QEmu memory hot unplug support.

[Zhi Yong Wu]

HI,

Can you push the patchset to a branch on github? It will be convenient
for other guys to do some tests.

On Thu, Jan 8, 2015 at 9:06 AM, Tang Chen <tangchen@cn.fujitsu.com> wrote:
> Memory hot unplug are both asynchronize procedures.
> When the unplug operation happens, unplug request cb is called first.
> And when ghest OS finished handling unplug, unplug cb will be called
> to do the real removal of device.
>
> This patch-set is based on QEmu 2.2
>
> This series depends on the following patchset.
> [PATCH] Common unplug and unplug request cb for memory and CPU hot-unplug.
> https://www.mail-archive.com/qemu-devel@nongnu.org/msg272745.html
>
> Hu Tao (2):
>   acpi, piix4: Add memory hot unplug request support for piix4.
>   pc, acpi bios: Add memory hot unplug interface.
>
> Tang Chen (11):
>   acpi, mem-hotplug: Use PC_DIMM_SLOT_PROP in acpi_memory_plug_cb().
>   acpi, mem-hotplug: Add acpi_memory_get_slot_status_descriptor() to get
>     MemStatus.
>   acpi, mem-hotplug: Add acpi_memory_hotplug_sci() to rise sci for
>     memory hotplug.
>   acpi, mem-hotplug: Add unplug request cb for memory device.
>   acpi, ich9: Add memory hot unplug request support for ich9.
>   pc-dimm: Add memory hot unplug request support for pc-dimm.
>   acpi, mem-hotplug: Add unplug cb for memory device.
>   acpi, piix4: Add memory hot unplug support for piix4.
>   acpi, ich9: Add memory hot unplug support for ich9.
>   pc-dimm: Add memory hot unplug support for pc-dimm.
>   acpi: Add hardware implementation for memory hot unplug.
>
>  docs/specs/acpi_mem_hotplug.txt   |  8 +++-
>  hw/acpi/ich9.c                    | 20 ++++++--
>  hw/acpi/memory_hotplug.c          | 97 ++++++++++++++++++++++++++++++++-------
>  hw/acpi/piix4.c                   | 18 ++++++--
>  hw/core/qdev.c                    |  2 +-
>  hw/i386/acpi-dsdt-mem-hotplug.dsl | 11 ++++-
>  hw/i386/pc.c                      | 53 +++++++++++++++++++--
>  hw/i386/ssdt-mem.dsl              |  5 ++
>  include/hw/acpi/memory_hotplug.h  |  6 +++
>  include/hw/acpi/pc-hotplug.h      |  2 +
>  include/hw/qdev-core.h            |  1 +
>  11 files changed, 192 insertions(+), 31 deletions(-)
>
> --
> 1.8.4.2
>
>



-- 
Regards,

Zhi Yong Wu

Re: [Qemu-devel] [RESEND PATCH v1 11/13] pc-dimm: Add memory hot unplug support for pc-dimm.

[Zhu Guihua]

On 01/29/2015 09:34 PM, Igor Mammedov wrote:
> On Thu, 8 Jan 2015 09:06:18 +0800
> Tang Chen <tangchen@cn.fujitsu.com> wrote:
>
>> Implement unplug cb for pc-dimm. It remove the corresponding
>> memory region, and unregister vmstat. At last, it calls memory
>> unplug cb to reset memory status and do unparenting.
>> ---
>>   hw/i386/pc.c | 25 +++++++++++++++++++++++--
>>   1 file changed, 23 insertions(+), 2 deletions(-)
>>
>> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
>> index f501f1f..3732f67 100644
>> --- a/hw/i386/pc.c
>> +++ b/hw/i386/pc.c
>> @@ -1691,6 +1691,23 @@ out:
>>       error_propagate(errp, local_err);
>>   }
>>   
>> +static void pc_dimm_unplug(HotplugHandler *hotplug_dev,
>> +                           DeviceState *dev, Error **errp)
>> +{
>> +    PCMachineState *pcms = PC_MACHINE(hotplug_dev);
>> +    PCDIMMDevice *dimm = PC_DIMM(dev);
>> +    PCDIMMDeviceClass *ddc = PC_DIMM_GET_CLASS(dimm);
>> +    MemoryRegion *mr = ddc->get_memory_region(dimm);
>> +    HotplugHandlerClass *hhc;
>> +    Error *local_err = NULL;
>> +
>> +    memory_region_del_subregion(&pcms->hotplug_memory, mr);
>> +    vmstate_unregister_ram(mr, dev);
> I'd first cleanup external state by calling acpi unplug handler
> and only the do above.

your meaning is to put the above behind hhc->unplug ?

Regards,
Zhu

>> +
>> +    hhc = HOTPLUG_HANDLER_GET_CLASS(pcms->acpi_dev);
>> +    hhc->unplug(HOTPLUG_HANDLER(pcms->acpi_dev), dev, &local_err);
>> +}
>> +
>>   static void pc_cpu_plug(HotplugHandler *hotplug_dev,
>>                           DeviceState *dev, Error **errp)
>>   {
>> @@ -1744,8 +1761,12 @@ static void pc_machine_device_unplug_request_cb(HotplugHandler *hotplug_dev,
>>   static void pc_machine_device_unplug_cb(HotplugHandler *hotplug_dev,
>>                                           DeviceState *dev, Error **errp)
>>   {
>> -    error_setg(errp, "acpi: device unplug for not supported device"
>> -               " type: %s", object_get_typename(OBJECT(dev)));
>> +    if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) {
>> +        pc_dimm_unplug(hotplug_dev, dev, errp);
>> +    } else {
>> +        error_setg(errp, "acpi: device unplug for not supported device"
>> +                   " type: %s", object_get_typename(OBJECT(dev)));
>> +    }
>>   }
>>   
>>   static HotplugHandler *pc_get_hotpug_handler(MachineState *machine,
> .
>

Re: [Qemu-devel] [RESEND PATCH v1 11/13] pc-dimm: Add memory hot unplug support for pc-dimm.

[Igor Mammedov]

On Wed, 25 Feb 2015 19:21:43 +0800
Zhu Guihua <zhugh.fnst@cn.fujitsu.com> wrote:

> 
> On 01/29/2015 09:34 PM, Igor Mammedov wrote:
> > On Thu, 8 Jan 2015 09:06:18 +0800
> > Tang Chen <tangchen@cn.fujitsu.com> wrote:
> >
> >> Implement unplug cb for pc-dimm. It remove the corresponding
> >> memory region, and unregister vmstat. At last, it calls memory
> >> unplug cb to reset memory status and do unparenting.
> >> ---
> >>   hw/i386/pc.c | 25 +++++++++++++++++++++++--
> >>   1 file changed, 23 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> >> index f501f1f..3732f67 100644
> >> --- a/hw/i386/pc.c
> >> +++ b/hw/i386/pc.c
> >> @@ -1691,6 +1691,23 @@ out:
> >>       error_propagate(errp, local_err);
> >>   }
> >>   
> >> +static void pc_dimm_unplug(HotplugHandler *hotplug_dev,
> >> +                           DeviceState *dev, Error **errp)
> >> +{
> >> +    PCMachineState *pcms = PC_MACHINE(hotplug_dev);
> >> +    PCDIMMDevice *dimm = PC_DIMM(dev);
> >> +    PCDIMMDeviceClass *ddc = PC_DIMM_GET_CLASS(dimm);
> >> +    MemoryRegion *mr = ddc->get_memory_region(dimm);
> >> +    HotplugHandlerClass *hhc;
> >> +    Error *local_err = NULL;
> >> +
> >> +    memory_region_del_subregion(&pcms->hotplug_memory, mr);
> >> +    vmstate_unregister_ram(mr, dev);
> > I'd first cleanup external state by calling acpi unplug handler
> > and only the do above.
> 
> your meaning is to put the above behind hhc->unplug ?

yes

> 
> Regards,
> Zhu
> 
> >> +
> >> +    hhc = HOTPLUG_HANDLER_GET_CLASS(pcms->acpi_dev);
> >> +    hhc->unplug(HOTPLUG_HANDLER(pcms->acpi_dev), dev, &local_err);
> >> +}
> >> +
> >>   static void pc_cpu_plug(HotplugHandler *hotplug_dev,
> >>                           DeviceState *dev, Error **errp)
> >>   {
> >> @@ -1744,8 +1761,12 @@ static void pc_machine_device_unplug_request_cb(HotplugHandler *hotplug_dev,
> >>   static void pc_machine_device_unplug_cb(HotplugHandler *hotplug_dev,
> >>                                           DeviceState *dev, Error **errp)
> >>   {
> >> -    error_setg(errp, "acpi: device unplug for not supported device"
> >> -               " type: %s", object_get_typename(OBJECT(dev)));
> >> +    if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) {
> >> +        pc_dimm_unplug(hotplug_dev, dev, errp);
> >> +    } else {
> >> +        error_setg(errp, "acpi: device unplug for not supported device"
> >> +                   " type: %s", object_get_typename(OBJECT(dev)));
> >> +    }
> >>   }
> >>   
> >>   static HotplugHandler *pc_get_hotpug_handler(MachineState *machine,
> > .
> >
>