[Qemu-devel] [PATCH v2 00/12] QEmu memory hot unplug support

[Qemu-devel] [PATCH v2 00/12] QEmu memory hot unplug support

[Zhu Guihua]

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 series depends on the following patchset.
[PATCH v2 0/5] Common unplug and unplug request cb for memory and CPU hot-unplug
https://lists.nongnu.org/archive/html/qemu-devel/2015-01/msg03929.html

v2:
- do a generic for acpi to send gpe event
- unparent object by PC_MACHINE
- update description in acpi_mem_hotplug.txt
- combine the last two patches in the last version
- cleanup external state in acpi_memory_unplug_cb

Hu Tao (1):
  acpi, piix4: Add memory hot unplug request support for piix4.

Tang Chen (11):
  acpi, mem-hotplug: Use PC_DIMM_SLOT_PROP in acpi_memory_plug_cb().
  acpi, mem-hotplug: Add acpi_memory_slot_status() 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   |   9 +++-
 hw/acpi/core.c                    |   7 +++
 hw/acpi/ich9.c                    |  20 +++++--
 hw/acpi/memory_hotplug.c          | 111 ++++++++++++++++++++++++++++++++------
 hw/acpi/piix4.c                   |  17 ++++--
 hw/core/qdev.c                    |   2 +-
 hw/i386/acpi-dsdt-mem-hotplug.dsl |  11 +++-
 hw/i386/pc.c                      |  48 +++++++++++++++--
 hw/i386/ssdt-mem.dsl              |   5 ++
 include/hw/acpi/acpi.h            |   3 ++
 include/hw/acpi/memory_hotplug.h  |   6 +++
 include/hw/acpi/pc-hotplug.h      |   2 +
 include/hw/qdev-core.h            |   1 +
 13 files changed, 211 insertions(+), 31 deletions(-)

-- 
1.9.3

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

[Zhu Guihua]

From: Tang Chen <tangchen@cn.fujitsu.com>

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

Reviewed-by: Igor Mammedov <imammedo@redhat.com>
Signed-off-by: Tang Chen <tangchen@cn.fujitsu.com>
Signed-off-by: Zhu Guihua <zhugh.fnst@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.9.3

[Qemu-devel] [PATCH v2 02/12] acpi, mem-hotplug: Add acpi_memory_slot_status() to get MemStatus.

[Zhu Guihua]

From: Tang Chen <tangchen@cn.fujitsu.com>

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>
Signed-off-by: Zhu Guihua <zhugh.fnst@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..ddbe01b 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_slot_status(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_slot_status(mem_st, dev, errp);
+    if (!mdev)
+        return;
+
     mdev->dimm = dev;
     mdev->is_enabled = true;
     mdev->is_inserting = true;
-- 
1.9.3

Re: [Qemu-devel] [PATCH v2 02/12] acpi, mem-hotplug: Add acpi_memory_slot_status() to get MemStatus.

[Igor Mammedov]

On Wed, 4 Feb 2015 10:51:16 +0800
Zhu Guihua <zhugh.fnst@cn.fujitsu.com> wrote:

> From: Tang Chen <tangchen@cn.fujitsu.com>
> 
> 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>
> Signed-off-by: Zhu Guihua <zhugh.fnst@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..ddbe01b 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_slot_status(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_slot_status(mem_st, dev, errp);
> +    if (!mdev)
> +        return;
run patches through scripts/checkpatch.pl

> +
>      mdev->dimm = dev;
>      mdev->is_enabled = true;
>      mdev->is_inserting = true;

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

[Zhu Guihua]

From: Tang Chen <tangchen@cn.fujitsu.com>

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>
Signed-off-by: Zhu Guihua <zhugh.fnst@cn.fujitsu.com>
---
 hw/acpi/core.c           | 7 +++++++
 hw/acpi/memory_hotplug.c | 6 ++----
 include/hw/acpi/acpi.h   | 3 +++
 3 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/hw/acpi/core.c b/hw/acpi/core.c
index 51913d6..98ca994 100644
--- a/hw/acpi/core.c
+++ b/hw/acpi/core.c
@@ -666,6 +666,13 @@ uint32_t acpi_gpe_ioport_readb(ACPIREGS *ar, uint32_t addr)
     return val;
 }
 
+void acpi_send_gpe_event(ACPIREGS *ar, qemu_irq irq,
+                         unsigned int hotplug_status)
+{
+    ar->gpe.sts[0] |= hotplug_status;
+    acpi_update_sci(ar, irq);
+}
+
 void acpi_update_sci(ACPIREGS *regs, qemu_irq irq)
 {
     int sci_level, pm1a_sts;
diff --git a/hw/acpi/memory_hotplug.c b/hw/acpi/memory_hotplug.c
index ddbe01b..f30d8f9 100644
--- a/hw/acpi/memory_hotplug.c
+++ b/hw/acpi/memory_hotplug.c
@@ -201,10 +201,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_send_gpe_event(ar, irq, ACPI_MEMORY_HOTPLUG_STATUS);
 }
 
 static const VMStateDescription vmstate_memhp_sts = {
diff --git a/include/hw/acpi/acpi.h b/include/hw/acpi/acpi.h
index 1f678b4..7a0a209 100644
--- a/include/hw/acpi/acpi.h
+++ b/include/hw/acpi/acpi.h
@@ -172,6 +172,9 @@ void acpi_gpe_reset(ACPIREGS *ar);
 void acpi_gpe_ioport_writeb(ACPIREGS *ar, uint32_t addr, uint32_t val);
 uint32_t acpi_gpe_ioport_readb(ACPIREGS *ar, uint32_t addr);
 
+void acpi_send_gpe_event(ACPIREGS *ar, qemu_irq irq,
+                         unsigned int hotplug_status);
+
 void acpi_update_sci(ACPIREGS *acpi_regs, qemu_irq irq);
 
 /* acpi.c */
-- 
1.9.3

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

[Igor Mammedov]

On Wed, 4 Feb 2015 10:51:17 +0800
Zhu Guihua <zhugh.fnst@cn.fujitsu.com> wrote:

> From: Tang Chen <tangchen@cn.fujitsu.com>
> 
> 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.
Commit subj/message should be amended to reflect changes in patch.
i.e. acpi_memory_hotplug_sci doesn't exist anymore

> 
> Signed-off-by: Tang Chen <tangchen@cn.fujitsu.com>
> Signed-off-by: Zhu Guihua <zhugh.fnst@cn.fujitsu.com>
> ---
>  hw/acpi/core.c           | 7 +++++++
>  hw/acpi/memory_hotplug.c | 6 ++----
>  include/hw/acpi/acpi.h   | 3 +++
>  3 files changed, 12 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/acpi/core.c b/hw/acpi/core.c
> index 51913d6..98ca994 100644
> --- a/hw/acpi/core.c
> +++ b/hw/acpi/core.c
> @@ -666,6 +666,13 @@ uint32_t acpi_gpe_ioport_readb(ACPIREGS *ar, uint32_t addr)
>      return val;
>  }
>  
> +void acpi_send_gpe_event(ACPIREGS *ar, qemu_irq irq,
> +                         unsigned int hotplug_status)
> +{
> +    ar->gpe.sts[0] |= hotplug_status;
> +    acpi_update_sci(ar, irq);
> +}
> +
>  void acpi_update_sci(ACPIREGS *regs, qemu_irq irq)
>  {
>      int sci_level, pm1a_sts;
> diff --git a/hw/acpi/memory_hotplug.c b/hw/acpi/memory_hotplug.c
> index ddbe01b..f30d8f9 100644
> --- a/hw/acpi/memory_hotplug.c
> +++ b/hw/acpi/memory_hotplug.c
> @@ -201,10 +201,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_send_gpe_event(ar, irq, ACPI_MEMORY_HOTPLUG_STATUS);
>  }
>  
>  static const VMStateDescription vmstate_memhp_sts = {
> diff --git a/include/hw/acpi/acpi.h b/include/hw/acpi/acpi.h
> index 1f678b4..7a0a209 100644
> --- a/include/hw/acpi/acpi.h
> +++ b/include/hw/acpi/acpi.h
> @@ -172,6 +172,9 @@ void acpi_gpe_reset(ACPIREGS *ar);
>  void acpi_gpe_ioport_writeb(ACPIREGS *ar, uint32_t addr, uint32_t val);
>  uint32_t acpi_gpe_ioport_readb(ACPIREGS *ar, uint32_t addr);
>  
> +void acpi_send_gpe_event(ACPIREGS *ar, qemu_irq irq,
> +                         unsigned int hotplug_status);
> +
>  void acpi_update_sci(ACPIREGS *acpi_regs, qemu_irq irq);
>  
>  /* acpi.c */

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

[Zhu Guihua]

From: Tang Chen <tangchen@cn.fujitsu.com>

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>
Signed-off-by: Zhu Guihua <zhugh.fnst@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 f30d8f9..3d3c1ec 100644
--- a/hw/acpi/memory_hotplug.c
+++ b/hw/acpi/memory_hotplug.c
@@ -205,6 +205,22 @@ void acpi_memory_plug_cb(ACPIREGS *ar, qemu_irq irq, MemHotplugState *mem_st,
     acpi_send_gpe_event(ar, irq, ACPI_MEMORY_HOTPLUG_STATUS);
 }
 
+void acpi_memory_unplug_request_cb(ACPIREGS *ar, qemu_irq irq,
+                                   MemHotplugState *mem_st,
+                                   DeviceState *dev, Error **errp)
+{
+    MemStatus *mdev;
+
+    mdev = acpi_memory_slot_status(mem_st, dev, errp);
+    if (!mdev)
+        return;
+
+    mdev->is_removing = true;
+
+    /* Do ACPI magic */
+    acpi_send_gpe_event(ar, irq, ACPI_MEMORY_HOTPLUG_STATUS);
+}
+
 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.9.3

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

[Zhu Guihua]

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>
Signed-off-by: Zhu Guihua <zhugh.fnst@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 14d40a0..8bd9007 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.9.3

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

[Zhu Guihua]

From: Tang Chen <tangchen@cn.fujitsu.com>

Call memory unplug request cb in ich9_pm_device_unplug_request_cb().

Signed-off-by: Zhu Guihua <zhugh.fnst@cn.fujitsu.com>
---
 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 5352e19..b85eed4 100644
--- a/hw/acpi/ich9.c
+++ b/hw/acpi/ich9.c
@@ -400,8 +400,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.9.3

[Qemu-devel] [PATCH v2 07/12] pc-dimm: Add memory hot unplug request support for pc-dimm.

[Zhu Guihua]

From: Tang Chen <tangchen@cn.fujitsu.com>

Implement memory unplug request cb for pc-dimm, and call it in
pc_machine_device_unplug_request_cb().

Signed-off-by: Zhu Guihua <zhugh.fnst@cn.fujitsu.com>
---
 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 850b6b5..ddc0190 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1641,6 +1641,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)
 {
@@ -1683,8 +1703,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.9.3

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

[Zhu Guihua]

From: Tang Chen <tangchen@cn.fujitsu.com>

Reset all memory status, and unparent the memory device.

Signed-off-by: Zhu Guihua <zhugh.fnst@cn.fujitsu.com>
---
 hw/acpi/memory_hotplug.c         | 34 ++++++++++++++++++++++++++++++++++
 hw/core/qdev.c                   |  2 +-
 include/hw/acpi/memory_hotplug.h |  2 ++
 include/hw/qdev-core.h           |  1 +
 4 files changed, 38 insertions(+), 1 deletion(-)

diff --git a/hw/acpi/memory_hotplug.c b/hw/acpi/memory_hotplug.c
index 3d3c1ec..3ae9629 100644
--- a/hw/acpi/memory_hotplug.c
+++ b/hw/acpi/memory_hotplug.c
@@ -1,6 +1,7 @@
 #include "hw/acpi/memory_hotplug.h"
 #include "hw/acpi/pc-hotplug.h"
 #include "hw/mem/pc-dimm.h"
+#include "hw/i386/pc.h"
 #include "hw/boards.h"
 #include "trace.h"
 #include "qapi-event.h"
@@ -221,6 +222,39 @@ void acpi_memory_unplug_request_cb(ACPIREGS *ar, qemu_irq irq,
     acpi_send_gpe_event(ar, irq, ACPI_MEMORY_HOTPLUG_STATUS);
 }
 
+void acpi_memory_unplug_cb(ACPIREGS *ar, qemu_irq irq,
+                           MemHotplugState *mem_st,
+                           DeviceState *dev, Error **errp)
+{
+    MemStatus *mdev;
+    HotplugHandler *hotplug_dev;
+    PCMachineState *pcms;
+    PCDIMMDevice *dimm;
+    PCDIMMDeviceClass *ddc;
+    MemoryRegion *mr;
+
+    if (!mem_st->is_enabled) {
+        error_setg(errp, "memory hotplug is not supported");
+        return;
+    }
+
+    mdev = acpi_memory_slot_status(mem_st, dev, errp);
+    if (!mdev)
+        return;
+
+    mdev->is_enabled = false;
+    mdev->dimm = NULL;
+
+    hotplug_dev = qdev_get_hotplug_handler(dev);
+    pcms = PC_MACHINE(hotplug_dev);
+    dimm = PC_DIMM(dev);
+    ddc = PC_DIMM_GET_CLASS(dimm);
+    mr = ddc->get_memory_region(dimm);
+
+    memory_region_del_subregion(&pcms->hotplug_memory, mr);
+    vmstate_unregister_ram(mr, dev);
+}
+
 static const VMStateDescription vmstate_memhp_sts = {
     .name = "memory hotplug device state",
     .version_id = 1,
diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index 2eacac0..2f3d1df 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -273,7 +273,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/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) \
diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index 15a226f..03d6239 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -266,6 +266,7 @@ int qdev_init(DeviceState *dev) QEMU_WARN_UNUSED_RESULT;
 void qdev_init_nofail(DeviceState *dev);
 void qdev_set_legacy_instance_id(DeviceState *dev, int alias_id,
                                  int required_for_version);
+HotplugHandler *qdev_get_hotplug_handler(DeviceState *dev);
 void qdev_unplug(DeviceState *dev, Error **errp);
 void qdev_simple_device_unplug_cb(HotplugHandler *hotplug_dev,
                                   DeviceState *dev, Error **errp);
-- 
1.9.3

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

[Igor Mammedov]

On Wed, 4 Feb 2015 10:51:22 +0800
Zhu Guihua <zhugh.fnst@cn.fujitsu.com> wrote:

> From: Tang Chen <tangchen@cn.fujitsu.com>
> 
> Reset all memory status, and unparent the memory device.
> 
> Signed-off-by: Zhu Guihua <zhugh.fnst@cn.fujitsu.com>
> ---
>  hw/acpi/memory_hotplug.c         | 34 ++++++++++++++++++++++++++++++++++
>  hw/core/qdev.c                   |  2 +-
>  include/hw/acpi/memory_hotplug.h |  2 ++
>  include/hw/qdev-core.h           |  1 +
>  4 files changed, 38 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/acpi/memory_hotplug.c b/hw/acpi/memory_hotplug.c
> index 3d3c1ec..3ae9629 100644
> --- a/hw/acpi/memory_hotplug.c
> +++ b/hw/acpi/memory_hotplug.c
> @@ -1,6 +1,7 @@
>  #include "hw/acpi/memory_hotplug.h"
>  #include "hw/acpi/pc-hotplug.h"
>  #include "hw/mem/pc-dimm.h"
> +#include "hw/i386/pc.h"
>  #include "hw/boards.h"
>  #include "trace.h"
>  #include "qapi-event.h"
> @@ -221,6 +222,39 @@ void acpi_memory_unplug_request_cb(ACPIREGS *ar, qemu_irq irq,
>      acpi_send_gpe_event(ar, irq, ACPI_MEMORY_HOTPLUG_STATUS);
>  }
>  
> +void acpi_memory_unplug_cb(ACPIREGS *ar, qemu_irq irq,
> +                           MemHotplugState *mem_st,
> +                           DeviceState *dev, Error **errp)
> +{
[...]
> +
> +    hotplug_dev = qdev_get_hotplug_handler(dev);
> +    pcms = PC_MACHINE(hotplug_dev);
> +    dimm = PC_DIMM(dev);
> +    ddc = PC_DIMM_GET_CLASS(dimm);
> +    mr = ddc->get_memory_region(dimm);
> +
> +    memory_region_del_subregion(&pcms->hotplug_memory, mr);
> +    vmstate_unregister_ram(mr, dev);
perhaps I wasn't clear previous time,
acpi device has no business in un-mapping and un-registering.

Pls, leave lines that deal with internal to acpi device state
/i.e. mdev/ and move memory_region_del_subregion/vmstate_unregister_ram
to pc_dimm_unplug().

> +}
> +
>  static const VMStateDescription vmstate_memhp_sts = {
>      .name = "memory hotplug device state",
>      .version_id = 1,




> diff --git a/hw/core/qdev.c b/hw/core/qdev.c
> index 2eacac0..2f3d1df 100644
> --- a/hw/core/qdev.c
> +++ b/hw/core/qdev.c
> @@ -273,7 +273,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 15a226f..03d6239 100644
> --- a/include/hw/qdev-core.h
> +++ b/include/hw/qdev-core.h
> @@ -266,6 +266,7 @@ int qdev_init(DeviceState *dev) QEMU_WARN_UNUSED_RESULT;
>  void qdev_init_nofail(DeviceState *dev);
>  void qdev_set_legacy_instance_id(DeviceState *dev, int alias_id,
>                                   int required_for_version);
> +HotplugHandler *qdev_get_hotplug_handler(DeviceState *dev);
>  void qdev_unplug(DeviceState *dev, Error **errp);
>  void qdev_simple_device_unplug_cb(HotplugHandler *hotplug_dev,
>                                    DeviceState *dev, Error **errp);
move qdev_get_hotplug_handler changes into a separate patch,
though it won't be used in this patch it will be used by the last patch anyway.

[Qemu-devel] [PATCH v2 09/12] acpi, piix4: Add memory hot unplug support for piix4.

[Zhu Guihua]

From: Tang Chen <tangchen@cn.fujitsu.com>

Call memory unplug cb in piix4_device_unplug_cb().

Signed-off-by: Zhu Guihua <zhugh.fnst@cn.fujitsu.com>
---
 hw/acpi/piix4.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
index 8bd9007..acd054e 100644
--- a/hw/acpi/piix4.c
+++ b/hw/acpi/piix4.c
@@ -377,8 +377,15 @@ 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 (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.9.3

[Qemu-devel] [PATCH v2 10/12] acpi, ich9: Add memory hot unplug support for ich9.

[Zhu Guihua]

From: Tang Chen <tangchen@cn.fujitsu.com>

Call memory unplug cb in ich9_pm_device_unplug_cb().

Signed-off-by: Zhu Guihua <zhugh.fnst@cn.fujitsu.com>
---
 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 b85eed4..3a8d712 100644
--- a/hw/acpi/ich9.c
+++ b/hw/acpi/ich9.c
@@ -413,8 +413,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.9.3

[Qemu-devel] [PATCH v2 11/12] pc-dimm: Add memory hot unplug support for pc-dimm.

[Zhu Guihua]

From: Tang Chen <tangchen@cn.fujitsu.com>

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.

Signed-off-by: Zhu Guihua <zhugh.fnst@cn.fujitsu.com>
---
 hw/i386/pc.c | 20 ++++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index ddc0190..4c03ee5 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1661,6 +1661,17 @@ out:
     error_propagate(errp, local_err);
 }
 
+static void pc_dimm_unplug(HotplugHandler *hotplug_dev,
+                           DeviceState *dev, Error **errp)
+{
+    PCMachineState *pcms = PC_MACHINE(hotplug_dev);
+    HotplugHandlerClass *hhc;
+    Error *local_err = NULL;
+
+    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)
 {
@@ -1714,8 +1725,13 @@ 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);
+        object_unparent(OBJECT(dev));
+    } 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.9.3

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

[Zhu Guihua]

From: Tang Chen <tangchen@cn.fujitsu.com>

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>
Signed-off-by: Zhu Guihua <zhugh.fnst@cn.fujitsu.com>
---
 docs/specs/acpi_mem_hotplug.txt   |  9 +++++++--
 hw/acpi/memory_hotplug.c          | 25 ++++++++++++++++++++++---
 hw/i386/acpi-dsdt-mem-hotplug.dsl | 11 ++++++++++-
 hw/i386/ssdt-mem.dsl              |  5 +++++
 include/hw/acpi/pc-hotplug.h      |  2 ++
 5 files changed, 46 insertions(+), 6 deletions(-)

diff --git a/docs/specs/acpi_mem_hotplug.txt b/docs/specs/acpi_mem_hotplug.txt
index 1290994..9805f1a 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 distinguish device for which
+                 no device check event to OSPM was issued.
+              3-7: reserved and should be ignored by OSPM
       [0x15-0x17] reserved
 
   write access:
@@ -35,7 +37,10 @@ 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: if set to 1 clears device remove event, set by OSPM
+                 after it has emitted device check event for the
+                 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 3ae9629..a6fc3b3 100644
--- a/hw/acpi/memory_hotplug.c
+++ b/hw/acpi/memory_hotplug.c
@@ -3,6 +3,7 @@
 #include "hw/mem/pc-dimm.h"
 #include "hw/i386/pc.h"
 #include "hw/boards.h"
+#include "hw/qdev-core.h"
 #include "trace.h"
 #include "qapi-event.h"
 
@@ -76,6 +77,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:
@@ -91,6 +93,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;
@@ -122,21 +126,36 @@ 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 */
+            mdev->is_removing = false;
+            trace_mhp_acpi_clear_remove_evt(mem_st->selector);
+            /*
+             * 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/i386/acpi-dsdt-mem-hotplug.dsl b/hw/i386/acpi-dsdt-mem-hotplug.dsl
index 2a36c47..b53bf77 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, // (read) 1 if has a remove event. (write) 1 to clear event
             }
 
             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.9.3

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

[Igor Mammedov]

On Wed, 4 Feb 2015 10:51:26 +0800
Zhu Guihua <zhugh.fnst@cn.fujitsu.com> wrote:

> From: Tang Chen <tangchen@cn.fujitsu.com>
> 
> 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>
> Signed-off-by: Zhu Guihua <zhugh.fnst@cn.fujitsu.com>
> ---
>  docs/specs/acpi_mem_hotplug.txt   |  9 +++++++--
>  hw/acpi/memory_hotplug.c          | 25 ++++++++++++++++++++++---
>  hw/i386/acpi-dsdt-mem-hotplug.dsl | 11 ++++++++++-
>  hw/i386/ssdt-mem.dsl              |  5 +++++
>  include/hw/acpi/pc-hotplug.h      |  2 ++
>  5 files changed, 46 insertions(+), 6 deletions(-)
> 
> diff --git a/docs/specs/acpi_mem_hotplug.txt b/docs/specs/acpi_mem_hotplug.txt
> index 1290994..9805f1a 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 distinguish device for which
> +                 no device check event to OSPM was issued.
> +              3-7: reserved and should be ignored by OSPM
>        [0x15-0x17] reserved
>  
>    write access:
> @@ -35,7 +37,10 @@ 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: if set to 1 clears device remove event, set by OSPM
> +                 after it has emitted device check event for the
> +                 selected memory device
it doesn't match what this patch does, it clears removal event when
OSPM evaluates EJ0 handler.

What if OSPM fails to unplug device and therefore won't evaluate EJ0
method at all. So when next memory hotplug SCI arrives it will 
send removal notify to OSPM again.
I doesn't seem right, pls fix patch to behave as documented above,
i.e. clear event after device check event.
If guest fails to eject device it should send OST event about it
and forget about device removal. It's up to management tools to repeat
device_del if it's failed and tools still wish to do so.


> +              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 3ae9629..a6fc3b3 100644
> --- a/hw/acpi/memory_hotplug.c
> +++ b/hw/acpi/memory_hotplug.c
> @@ -3,6 +3,7 @@
>  #include "hw/mem/pc-dimm.h"
>  #include "hw/i386/pc.h"
>  #include "hw/boards.h"
> +#include "hw/qdev-core.h"
>  #include "trace.h"
>  #include "qapi-event.h"
>  
> @@ -76,6 +77,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:
> @@ -91,6 +93,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;
> @@ -122,21 +126,36 @@ 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 */
> +            mdev->is_removing = false;
> +            trace_mhp_acpi_clear_remove_evt(mem_st->selector);
> +            /*
> +             * 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/i386/acpi-dsdt-mem-hotplug.dsl b/hw/i386/acpi-dsdt-mem-hotplug.dsl
> index 2a36c47..b53bf77 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, // (read) 1 if has a remove event. (write) 1 to clear event
>              }
>  
>              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)
> +            }
could you split changes to hw/i386/ssdt-mem.dsl into a separate patch
so we could simply swap it with AML API version if it's merged first.


>          }
>      }
>  }
> 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] [PATCH v2 00/12] QEmu memory hot unplug support

[Michael S. Tsirkin]

Re: subject - it's really QEMU - not QEmu.

you have this in code as well, so I'm guessing you didn't
run your patches through checkpatch.pl. Please do
and fix fallout if any.

-- 
MST

Re: [Qemu-devel] [PATCH v2 00/12] QEmu memory hot unplug support

[Zhu Guihua]

On Wed, 2015-02-04 at 11:28 +0100, Michael S. Tsirkin wrote:
> Re: subject - it's really QEMU - not QEmu.
> 
> you have this in code as well, so I'm guessing you didn't
> run your patches through checkpatch.pl. Please do
> and fix fallout if any.

I do it now, and I find warnings about this.
I will fix it. Thanks.

Regards,
Zhu

> 

Re: [Qemu-devel] [PATCH v2 00/12] QEmu memory hot unplug support

[Igor Mammedov]

On Wed, 4 Feb 2015 10:51:14 +0800
Zhu Guihua <zhugh.fnst@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.

please check and substitute throughout patches:
s/asynchronize[d]/asynchronous/
s/ghest/guest/

> 
> This series depends on the following patchset.
> [PATCH v2 0/5] Common unplug and unplug request cb for memory and CPU hot-unplug
> https://lists.nongnu.org/archive/html/qemu-devel/2015-01/msg03929.html
> 
> v2:
> - do a generic for acpi to send gpe event
> - unparent object by PC_MACHINE
> - update description in acpi_mem_hotplug.txt
> - combine the last two patches in the last version
> - cleanup external state in acpi_memory_unplug_cb
> 
> Hu Tao (1):
>   acpi, piix4: Add memory hot unplug request support for piix4.
> 
> Tang Chen (11):
>   acpi, mem-hotplug: Use PC_DIMM_SLOT_PROP in acpi_memory_plug_cb().
>   acpi, mem-hotplug: Add acpi_memory_slot_status() 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   |   9 +++-
>  hw/acpi/core.c                    |   7 +++
>  hw/acpi/ich9.c                    |  20 +++++--
>  hw/acpi/memory_hotplug.c          | 111 ++++++++++++++++++++++++++++++++------
>  hw/acpi/piix4.c                   |  17 ++++--
>  hw/core/qdev.c                    |   2 +-
>  hw/i386/acpi-dsdt-mem-hotplug.dsl |  11 +++-
>  hw/i386/pc.c                      |  48 +++++++++++++++--
>  hw/i386/ssdt-mem.dsl              |   5 ++
>  include/hw/acpi/acpi.h            |   3 ++
>  include/hw/acpi/memory_hotplug.h  |   6 +++
>  include/hw/acpi/pc-hotplug.h      |   2 +
>  include/hw/qdev-core.h            |   1 +
>  13 files changed, 211 insertions(+), 31 deletions(-)
>