All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH v5 0/7] QEMU memory hot unplug support
@ 2015-03-27  9:20 Zhu Guihua
  2015-03-27  9:20 ` [Qemu-devel] [PATCH v5 1/7] docs: update documentation for memory hot unplug Zhu Guihua
                   ` (8 more replies)
  0 siblings, 9 replies; 18+ messages in thread
From: Zhu Guihua @ 2015-03-27  9:20 UTC (permalink / raw)
  To: qemu-devel, imammedo, pbonzini; +Cc: guz.fnst, izumi.taku, Zhu Guihua, tangchen

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

v5:
 -reorganize the patchset
 -add documentation to understand patch easily
 -add MEMORY_SLOT_EJECT for initiating device eject
 -add support to send qmp event to notify mgmt about memory unplug error

v4:
 -reorganize the patchset
 -drop the new API acpi_send_gpe_event()
 -update ssdt-mem

v3:
 -commit message changes
 -reorganize the patchset, squash and separate some patches
 -update specs about acpi_mem_hotplug
 -first cleanup external state, then un-map and un-register memory device

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

Tang Chen (3):
  acpi, mem-hotplug: add acpi_memory_slot_status() to get MemStatus
  acpi, mem-hotplug: add unplug request cb for memory device
  acpi, mem-hotplug: add unplug cb for memory device

Zhu Guihua (4):
  docs: update documentation for memory hot unplug
  acpi: extend aml_field() to support UpdateRule
  acpi: add hardware implementation for memory hot unplug
  qmp-event: add event notification for memory hot unplug error

 docs/memory-hotplug.txt           | 24 ++++++++--
 docs/qmp/qmp-events.txt           | 17 +++++++
 docs/specs/acpi_mem_hotplug.txt   | 32 ++++++++++++--
 hw/acpi/aml-build.c               |  4 +-
 hw/acpi/ich9.c                    | 19 ++++++--
 hw/acpi/memory_hotplug.c          | 93 ++++++++++++++++++++++++++++++++++++---
 hw/acpi/piix4.c                   | 17 +++++--
 hw/core/qdev.c                    |  2 +-
 hw/i386/acpi-build.c              | 25 ++++++++---
 hw/i386/acpi-dsdt-mem-hotplug.dsl | 13 +++++-
 hw/i386/pc.c                      | 62 ++++++++++++++++++++++++--
 include/hw/acpi/aml-build.h       | 10 ++++-
 include/hw/acpi/memory_hotplug.h  | 12 +++++
 include/hw/acpi/pc-hotplug.h      |  3 ++
 include/hw/qdev-core.h            |  1 +
 monitor.c                         |  1 +
 qapi/event.json                   | 14 ++++++
 trace-events                      |  2 +
 18 files changed, 315 insertions(+), 36 deletions(-)

-- 
1.9.3

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [Qemu-devel] [PATCH v5 1/7] docs: update documentation for memory hot unplug
  2015-03-27  9:20 [Qemu-devel] [PATCH v5 0/7] QEMU memory hot unplug support Zhu Guihua
@ 2015-03-27  9:20 ` Zhu Guihua
  2015-03-30 15:47   ` Igor Mammedov
  2015-03-27  9:20 ` [Qemu-devel] [PATCH v5 2/7] acpi, mem-hotplug: add acpi_memory_slot_status() to get MemStatus Zhu Guihua
                   ` (7 subsequent siblings)
  8 siblings, 1 reply; 18+ messages in thread
From: Zhu Guihua @ 2015-03-27  9:20 UTC (permalink / raw)
  To: qemu-devel, imammedo, pbonzini; +Cc: guz.fnst, izumi.taku, Zhu Guihua, tangchen

Add specification about how to use memory hot unplug, and add
a flow diagram to explain memory hot unplug process.

Signed-off-by: Zhu Guihua <zhugh.fnst@cn.fujitsu.com>
---
 docs/memory-hotplug.txt         | 24 ++++++++++++++++++++----
 docs/specs/acpi_mem_hotplug.txt | 16 +++++++++++++++-
 2 files changed, 35 insertions(+), 5 deletions(-)

diff --git a/docs/memory-hotplug.txt b/docs/memory-hotplug.txt
index f70571d..0de7f1b 100644
--- a/docs/memory-hotplug.txt
+++ b/docs/memory-hotplug.txt
@@ -1,12 +1,10 @@
 QEMU memory hotplug
 ===================
 
-This document explains how to use the memory hotplug feature in QEMU,
+This part explains how to use the memory hotplug feature in QEMU,
 which is present since v2.1.0.
 
-Please, note that memory hotunplug is not supported yet. This means
-that you're able to add memory, but you're not able to remove it.
-Also, proper guest support is required for memory hotplug to work.
+And, proper guest support is required for memory hotplug to work.
 
 Basic RAM hotplug
 -----------------
@@ -74,3 +72,21 @@ comes from regular RAM, 1GB is a 1GB hugepage page and 256MB is from
    -device pc-dimm,id=dimm1,memdev=mem1 \
    -object memory-backend-file,id=mem2,size=256M,mem-path=/mnt/hugepages-2MB \
    -device pc-dimm,id=dimm2,memdev=mem2
+
+
+QEMU memory hot unplug
+======================
+This part explains how to use the memory hot unplug feature in QEMU.
+
+In order to be able to hot unplug pc-dimm device, QEMU has to be told the id
+of pc-dimm device. The id was assigned when you hot plugged memory.
+
+One monitor command is used to hot unplug memory:
+
+ - "device_del": deletes a front-end pc-dimm device
+
+For example, assuming that the pc-dimm device with id "dimm1" was hotplugged
+into the guest, the following command deletes the pc-dimm device from the
+guest.
+
+  (qemu) device_del dimm1
diff --git a/docs/specs/acpi_mem_hotplug.txt b/docs/specs/acpi_mem_hotplug.txt
index 1290994..a88b286 100644
--- a/docs/specs/acpi_mem_hotplug.txt
+++ b/docs/specs/acpi_mem_hotplug.txt
@@ -2,7 +2,7 @@ QEMU<->ACPI BIOS memory hotplug interface
 --------------------------------------
 
 ACPI BIOS GPE.3 handler is dedicated for notifying OS about memory hot-add
-events.
+and hot-remove events.
 
 Memory hot-plug interface (IO port 0xa00-0xa17, 1-4 byte access):
 ---------------------------------------------------------------
@@ -42,3 +42,17 @@ Selecting memory device slot beyond present range has no effect on platform:
      ignored
    - read accesses to memory hot-plug registers not documented above return
      all bits set to 1.
+
+Memory hot remove process diagram:
+----------------------------------
+Memory hot unplug is rather complicated multi-stage process, it is asynchronous
+procedures. The follwoing diagram describes memory hot unplug process.
+
+             QEMU             |             Guest OS
+-------------------------------------------------------------------------------
+     unplug request cb ->     |
+                              |  OSPM processes ejection request
+                              |  <- OSPM clears device remove event
+    clear remove event ->     |
+                              |  <- OSPM issues _EJ0 and initiates device eject
+  unplug cb (eject device)    |
-- 
1.9.3

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [Qemu-devel] [PATCH v5 2/7] acpi, mem-hotplug: add acpi_memory_slot_status() to get MemStatus
  2015-03-27  9:20 [Qemu-devel] [PATCH v5 0/7] QEMU memory hot unplug support Zhu Guihua
  2015-03-27  9:20 ` [Qemu-devel] [PATCH v5 1/7] docs: update documentation for memory hot unplug Zhu Guihua
@ 2015-03-27  9:20 ` Zhu Guihua
  2015-03-27  9:20 ` [Qemu-devel] [PATCH v5 3/7] acpi, mem-hotplug: add unplug request cb for memory device Zhu Guihua
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 18+ messages in thread
From: Zhu Guihua @ 2015-03-27  9:20 UTC (permalink / raw)
  To: qemu-devel, imammedo, pbonzini; +Cc: guz.fnst, izumi.taku, Zhu Guihua, tangchen

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

Add a new API named acpi_memory_slot_status() 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 | 34 ++++++++++++++++++++++++++++------
 1 file changed, 28 insertions(+), 6 deletions(-)

diff --git a/hw/acpi/memory_hotplug.c b/hw/acpi/memory_hotplug.c
index c6580da..6af9303 100644
--- a/hw/acpi/memory_hotplug.c
+++ b/hw/acpi/memory_hotplug.c
@@ -163,29 +163,51 @@ 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)
+/**
+ * acpi_memory_slot_status:
+ * @mem_st: memory hotplug state
+ * @dev: device
+ * @errp: set in case of an error
+ *
+ * Obtain a single memory slot status.
+ *
+ * This function will be called by memory unplug request cb and unplug cb.
+ */
+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_slot_status: "
                    "device [%s] returned invalid memory slot[%d]",
                     dev_path, slot);
         g_free(dev_path);
+        return NULL;
+    }
+
+    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 = &mem_st->devs[slot];
     mdev->dimm = dev;
     mdev->is_enabled = true;
     mdev->is_inserting = true;
-- 
1.9.3

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [Qemu-devel] [PATCH v5 3/7] acpi, mem-hotplug: add unplug request cb for memory device
  2015-03-27  9:20 [Qemu-devel] [PATCH v5 0/7] QEMU memory hot unplug support Zhu Guihua
  2015-03-27  9:20 ` [Qemu-devel] [PATCH v5 1/7] docs: update documentation for memory hot unplug Zhu Guihua
  2015-03-27  9:20 ` [Qemu-devel] [PATCH v5 2/7] acpi, mem-hotplug: add acpi_memory_slot_status() to get MemStatus Zhu Guihua
@ 2015-03-27  9:20 ` Zhu Guihua
  2015-04-01  8:23   ` Igor Mammedov
  2015-03-27  9:20 ` [Qemu-devel] [PATCH v5 4/7] acpi, mem-hotplug: add unplug " Zhu Guihua
                   ` (5 subsequent siblings)
  8 siblings, 1 reply; 18+ messages in thread
From: Zhu Guihua @ 2015-03-27  9:20 UTC (permalink / raw)
  To: qemu-devel, imammedo, pbonzini; +Cc: guz.fnst, izumi.taku, Zhu Guihua, tangchen

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

This patch adds unplug request cb for memory device, and adds the
is_removing boolean field to MemStatus. This field is used to indicate
whether the memory slot is being removed. This field is set to true in
acpi_memory_unplug_request_cb().

Signed-off-by: Tang Chen <tangchen@cn.fujitsu.com>
Signed-off-by: Zhu Guihua <zhugh.fnst@cn.fujitsu.com>
---
 hw/acpi/ich9.c                   | 10 ++++++++--
 hw/acpi/memory_hotplug.c         | 19 +++++++++++++++++++
 hw/acpi/piix4.c                  |  6 +++++-
 hw/i386/pc.c                     | 28 ++++++++++++++++++++++++++--
 include/hw/acpi/memory_hotplug.h | 10 ++++++++++
 5 files changed, 68 insertions(+), 5 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,
diff --git a/hw/acpi/memory_hotplug.c b/hw/acpi/memory_hotplug.c
index 6af9303..42fe668 100644
--- a/hw/acpi/memory_hotplug.c
+++ b/hw/acpi/memory_hotplug.c
@@ -75,6 +75,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:
@@ -218,6 +219,24 @@ void acpi_memory_plug_cb(ACPIREGS *ar, qemu_irq irq, MemHotplugState *mem_st,
     return;
 }
 
+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 */
+    ar->gpe.sts[0] |= ACPI_MEMORY_HOTPLUG_STATUS;
+    acpi_update_sci(ar, irq);
+}
+
 static const VMStateDescription vmstate_memhp_sts = {
     .name = "memory hotplug device state",
     .version_id = 1,
diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
index d1f1179..f716e91 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 {
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index a8e6be1..9b0859c 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1677,6 +1677,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)
 {
@@ -1719,8 +1739,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,
diff --git a/include/hw/acpi/memory_hotplug.h b/include/hw/acpi/memory_hotplug.h
index 7bbf8a0..fbc2d65 100644
--- a/include/hw/acpi/memory_hotplug.h
+++ b/include/hw/acpi/memory_hotplug.h
@@ -7,10 +7,17 @@
 
 #define ACPI_MEMORY_HOTPLUG_STATUS 8
 
+/**
+ * MemStatus:
+ * @is_removing: Indicates whether the memory slot has been removed.
+ *
+ * This structure stores memory device's status.
+ */
 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 +35,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

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [Qemu-devel] [PATCH v5 4/7] acpi, mem-hotplug: add unplug cb for memory device
  2015-03-27  9:20 [Qemu-devel] [PATCH v5 0/7] QEMU memory hot unplug support Zhu Guihua
                   ` (2 preceding siblings ...)
  2015-03-27  9:20 ` [Qemu-devel] [PATCH v5 3/7] acpi, mem-hotplug: add unplug request cb for memory device Zhu Guihua
@ 2015-03-27  9:20 ` Zhu Guihua
  2015-03-27  9:20 ` [Qemu-devel] [PATCH v5 5/7] acpi: extend aml_field() to support UpdateRule Zhu Guihua
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 18+ messages in thread
From: Zhu Guihua @ 2015-03-27  9:20 UTC (permalink / raw)
  To: qemu-devel, imammedo, pbonzini; +Cc: guz.fnst, izumi.taku, Zhu Guihua, tangchen

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

This patch adds unplug cb for memory device. It resets memory status
"is_enabled" in acpi_memory_unplug_cb(), removes the corresponding
memory region, unregisters vmstate, and unparents the object.

Signed-off-by: Tang Chen <tangchen@cn.fujitsu.com>
Signed-off-by: Zhu Guihua <zhugh.fnst@cn.fujitsu.com>
---
 hw/acpi/ich9.c                   |  9 +++++++--
 hw/acpi/memory_hotplug.c         | 14 ++++++++++++++
 hw/acpi/piix4.c                  | 11 +++++++++--
 hw/i386/pc.c                     | 34 ++++++++++++++++++++++++++++++++--
 include/hw/acpi/memory_hotplug.h |  2 ++
 5 files changed, 64 insertions(+), 6 deletions(-)

diff --git a/hw/acpi/ich9.c b/hw/acpi/ich9.c
index b85eed4..84e5bb8 100644
--- a/hw/acpi/ich9.c
+++ b/hw/acpi/ich9.c
@@ -413,8 +413,13 @@ 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_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)
diff --git a/hw/acpi/memory_hotplug.c b/hw/acpi/memory_hotplug.c
index 42fe668..07e281f 100644
--- a/hw/acpi/memory_hotplug.c
+++ b/hw/acpi/memory_hotplug.c
@@ -237,6 +237,20 @@ void acpi_memory_unplug_request_cb(ACPIREGS *ar, qemu_irq irq,
     acpi_update_sci(ar, irq);
 }
 
+void acpi_memory_unplug_cb(MemHotplugState *mem_st,
+                           DeviceState *dev, Error **errp)
+{
+    MemStatus *mdev;
+
+    mdev = acpi_memory_slot_status(mem_st, dev, errp);
+    if (!mdev) {
+        return;
+    }
+
+    mdev->is_enabled = false;
+    mdev->dimm = NULL;
+}
+
 static const VMStateDescription vmstate_memhp_sts = {
     .name = "memory hotplug device state",
     .version_id = 1,
diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
index f716e91..1b28481 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 (s->acpi_memory_hotplug.is_enabled &&
+        object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) {
+        acpi_memory_unplug_cb(&s->acpi_memory_hotplug, dev, errp);
+    } else {
+        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)
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 9b0859c..769eb25 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1697,6 +1697,32 @@ 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;
+
+    hhc = HOTPLUG_HANDLER_GET_CLASS(pcms->acpi_dev);
+    hhc->unplug(HOTPLUG_HANDLER(pcms->acpi_dev), dev, &local_err);
+
+    if (local_err) {
+        goto out;
+    }
+
+    memory_region_del_subregion(&pcms->hotplug_memory, mr);
+    vmstate_unregister_ram(mr, dev);
+
+    object_unparent(OBJECT(dev));
+
+ out:
+    error_propagate(errp, local_err);
+}
+
 static void pc_cpu_plug(HotplugHandler *hotplug_dev,
                         DeviceState *dev, Error **errp)
 {
@@ -1750,8 +1776,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,
diff --git a/include/hw/acpi/memory_hotplug.h b/include/hw/acpi/memory_hotplug.h
index fbc2d65..8f9eb2d 100644
--- a/include/hw/acpi/memory_hotplug.h
+++ b/include/hw/acpi/memory_hotplug.h
@@ -38,6 +38,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(MemHotplugState *mem_st,
+                           DeviceState *dev, Error **errp);
 
 extern const VMStateDescription vmstate_memory_hotplug;
 #define VMSTATE_MEMORY_HOTPLUG(memhp, state) \
-- 
1.9.3

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [Qemu-devel] [PATCH v5 5/7] acpi: extend aml_field() to support UpdateRule
  2015-03-27  9:20 [Qemu-devel] [PATCH v5 0/7] QEMU memory hot unplug support Zhu Guihua
                   ` (3 preceding siblings ...)
  2015-03-27  9:20 ` [Qemu-devel] [PATCH v5 4/7] acpi, mem-hotplug: add unplug " Zhu Guihua
@ 2015-03-27  9:20 ` Zhu Guihua
  2015-04-01  8:39   ` Igor Mammedov
  2015-03-27  9:20 ` [Qemu-devel] [PATCH v5 6/7] acpi: add hardware implementation for memory hot unplug Zhu Guihua
                   ` (3 subsequent siblings)
  8 siblings, 1 reply; 18+ messages in thread
From: Zhu Guihua @ 2015-03-27  9:20 UTC (permalink / raw)
  To: qemu-devel, imammedo, pbonzini; +Cc: guz.fnst, izumi.taku, Zhu Guihua, tangchen

The flags field is declared with default update rule 'Preserve',
this patch is to extend aml_field() to support UpdateRule so that
we can specify different value to UpdateRule for different field.

Signed-off-by: Zhu Guihua <zhugh.fnst@cn.fujitsu.com>
---
 hw/acpi/aml-build.c         |  4 +++-
 hw/i386/acpi-build.c        | 13 ++++++++-----
 include/hw/acpi/aml-build.h | 10 ++++++++--
 3 files changed, 19 insertions(+), 8 deletions(-)

diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
index d7945f6..f926c9a 100644
--- a/hw/acpi/aml-build.c
+++ b/hw/acpi/aml-build.c
@@ -635,9 +635,11 @@ Aml *aml_reserved_field(unsigned length)
 }
 
 /* ACPI 1.0b: 16.2.5.2 Named Objects Encoding: DefField */
-Aml *aml_field(const char *name, AmlFieldFlags flags)
+Aml *aml_field(const char *name, AmlAccessType type, AmlUpdateRule rule)
 {
     Aml *var = aml_bundle(0x81 /* FieldOp */, AML_EXT_PACKAGE);
+    uint8_t flags = rule << 5 | type;
+
     build_append_namestring(var->buf, "%s", name);
     build_append_byte(var->buf, flags);
     return var;
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index d0a5c85..b767cbe 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -798,7 +798,7 @@ build_ssdt(GArray *table_data, GArray *linker,
 
         aml_append(dev, aml_operation_region("PEOR", aml_system_io,
                                               misc->pvpanic_port, 1));
-        field = aml_field("PEOR", aml_byte_acc);
+        field = aml_field("PEOR", aml_byte_acc, aml_preserve);
         aml_append(field, aml_named_field("PEPT", 8));
         aml_append(dev, field);
 
@@ -835,7 +835,7 @@ build_ssdt(GArray *table_data, GArray *linker,
         /* declare CPU hotplug MMIO region and PRS field to access it */
         aml_append(sb_scope, aml_operation_region(
             "PRST", aml_system_io, pm->cpu_hp_io_base, pm->cpu_hp_io_len));
-        field = aml_field("PRST", aml_byte_acc);
+        field = aml_field("PRST", aml_byte_acc, aml_preserve);
         aml_append(field, aml_named_field("PRS", 256));
         aml_append(sb_scope, field);
 
@@ -909,7 +909,8 @@ build_ssdt(GArray *table_data, GArray *linker,
             pm->mem_hp_io_base, pm->mem_hp_io_len)
         );
 
-        field = aml_field(stringify(MEMORY_HOTPLUG_IO_REGION), aml_dword_acc);
+        field = aml_field(stringify(MEMORY_HOTPLUG_IO_REGION), aml_dword_acc,
+                                    aml_preserve);
         aml_append(field, /* read only */
             aml_named_field(stringify(MEMORY_SLOT_ADDR_LOW), 32));
         aml_append(field, /* read only */
@@ -922,7 +923,8 @@ build_ssdt(GArray *table_data, GArray *linker,
             aml_named_field(stringify(MEMORY_SLOT_PROXIMITY), 32));
         aml_append(scope, field);
 
-        field = aml_field(stringify(MEMORY_HOTPLUG_IO_REGION), aml_byte_acc);
+        field = aml_field(stringify(MEMORY_HOTPLUG_IO_REGION), aml_byte_acc,
+                          aml_write_as_zeros);
         aml_append(field, aml_reserved_field(160 /* bits, Offset(20) */));
         aml_append(field, /* 1 if enabled, read only */
             aml_named_field(stringify(MEMORY_SLOT_ENABLED), 1));
@@ -931,7 +933,8 @@ build_ssdt(GArray *table_data, GArray *linker,
             aml_named_field(stringify(MEMORY_SLOT_INSERT_EVENT), 1));
         aml_append(scope, field);
 
-        field = aml_field(stringify(MEMORY_HOTPLUG_IO_REGION), aml_dword_acc);
+        field = aml_field(stringify(MEMORY_HOTPLUG_IO_REGION), aml_dword_acc,
+                          aml_preserve);
         aml_append(field, /* DIMM selector, write only */
             aml_named_field(stringify(MEMORY_SLOT_SLECTOR), 32));
         aml_append(field, /* _OST event code, write only */
diff --git a/include/hw/acpi/aml-build.h b/include/hw/acpi/aml-build.h
index 17d3beb..5aa5e7a 100644
--- a/include/hw/acpi/aml-build.h
+++ b/include/hw/acpi/aml-build.h
@@ -35,7 +35,13 @@ typedef enum {
     aml_dword_acc = 3,
     aml_qword_acc = 4,
     aml_buffer_acc = 5,
-} AmlFieldFlags;
+} AmlAccessType;
+
+typedef enum {
+    aml_preserve = 0,
+    aml_write_as_ones = 1,
+    aml_write_as_zeros = 2,
+} AmlUpdateRule;
 
 typedef enum {
     aml_system_memory = 0x00,
@@ -185,7 +191,7 @@ Aml *aml_if(Aml *predicate);
 Aml *aml_package(uint8_t num_elements);
 Aml *aml_buffer(void);
 Aml *aml_resource_template(void);
-Aml *aml_field(const char *name, AmlFieldFlags flags);
+Aml *aml_field(const char *name, AmlAccessType type, AmlUpdateRule rule);
 Aml *aml_varpackage(uint32_t num_elements);
 
 #endif
-- 
1.9.3

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [Qemu-devel] [PATCH v5 6/7] acpi: add hardware implementation for memory hot unplug
  2015-03-27  9:20 [Qemu-devel] [PATCH v5 0/7] QEMU memory hot unplug support Zhu Guihua
                   ` (4 preceding siblings ...)
  2015-03-27  9:20 ` [Qemu-devel] [PATCH v5 5/7] acpi: extend aml_field() to support UpdateRule Zhu Guihua
@ 2015-03-27  9:20 ` Zhu Guihua
  2015-03-30  9:09   ` Igor Mammedov
  2015-03-27  9:20 ` [Qemu-devel] [PATCH v5 7/7] qmp-event: add event notification for memory hot unplug error Zhu Guihua
                   ` (2 subsequent siblings)
  8 siblings, 1 reply; 18+ messages in thread
From: Zhu Guihua @ 2015-03-27  9:20 UTC (permalink / raw)
  To: qemu-devel, imammedo, pbonzini; +Cc: guz.fnst, izumi.taku, Zhu Guihua, tangchen

- implements QEMU hardware part of memory hot unplug protocol
  described at "docs/spec/acpi_mem_hotplug.txt"
- handles memory remove notification event
- handles device eject notification

Signed-off-by: Zhu Guihua <zhugh.fnst@cn.fujitsu.com>
---
 docs/specs/acpi_mem_hotplug.txt   | 16 +++++++++++++---
 hw/acpi/memory_hotplug.c          | 20 +++++++++++++++++++-
 hw/core/qdev.c                    |  2 +-
 hw/i386/acpi-build.c              | 12 ++++++++++++
 hw/i386/acpi-dsdt-mem-hotplug.dsl | 13 ++++++++++++-
 include/hw/acpi/pc-hotplug.h      |  3 +++
 include/hw/qdev-core.h            |  1 +
 trace-events                      |  2 ++
 8 files changed, 63 insertions(+), 6 deletions(-)

diff --git a/docs/specs/acpi_mem_hotplug.txt b/docs/specs/acpi_mem_hotplug.txt
index a88b286..96f9abe 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:
@@ -31,11 +33,19 @@ Memory hot-plug interface (IO port 0xa00-0xa17, 1-4 byte access):
       [0xc-0x13] reserved, writes into it are ignored
       [0x14] Memory device control fields
           bits:
-              0: reserved, OSPM must clear it before writing to register
+              0: reserved, OSPM must clear it before writing to register.
+                 Due to BUG in versions prior 2.4 that field isn't
+                 cleared when other fields are written.Keep it reserved
+                 and don't try to reuse it.
               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: if set to 1 initiates device eject, set by OSPM when it
+                 triggers memory device removal and calls _EJ0 method
+              4-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 07e281f..2a1b866 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"
 
@@ -91,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;
@@ -128,13 +131,28 @@ static void acpi_memory_hotplug_write(void *opaque, hwaddr addr, uint64_t data,
         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) {
+            mdev->is_removing = false;
+            trace_mhp_acpi_clear_remove_evt(mem_st->selector);
+        } else if (data & 8) {
+            if (!mdev->is_enabled) {
+                trace_mhp_acpi_ejecting_invalid_slot(mem_st->selector);
+                break;
+            }
+
+            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;
     }
 
 }
diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index 6e6a65d..b0f0f84 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/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index b767cbe..927250e 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -931,6 +931,12 @@ build_ssdt(GArray *table_data, GArray *linker,
         aml_append(field,
             /*(read) 1 if has a insert event. (write) 1 to clear event */
             aml_named_field(stringify(MEMORY_SLOT_INSERT_EVENT), 1));
+        aml_append(field,
+            /* (read) 1 if has a remove event. (write) 1 to clear event */
+            aml_named_field(stringify(MEMORY_SLOT_REMOVE_EVENT), 1));
+        aml_append(field,
+            /* initiates device eject, write only */
+            aml_named_field(stringify(MEMORY_SLOT_EJECT), 1));
         aml_append(scope, field);
 
         field = aml_field(stringify(MEMORY_HOTPLUG_IO_REGION), aml_dword_acc,
@@ -975,6 +981,12 @@ build_ssdt(GArray *table_data, GArray *linker,
             )));
             aml_append(dev, method);
 
+            method = aml_method("_EJ0", 1);
+            s = BASEPATH stringify(MEMORY_SLOT_EJECT_METHOD);
+            aml_append(method, aml_return(aml_call2(
+                       s, aml_name("_UID"), aml_arg(0))));
+            aml_append(dev, method);
+
             aml_append(sb_scope, dev);
         }
 
diff --git a/hw/i386/acpi-dsdt-mem-hotplug.dsl b/hw/i386/acpi-dsdt-mem-hotplug.dsl
index 1e9ec39..c2bb6a1 100644
--- a/hw/i386/acpi-dsdt-mem-hotplug.dsl
+++ b/hw/i386/acpi-dsdt-mem-hotplug.dsl
@@ -29,6 +29,8 @@
             External(MEMORY_SLOT_PROXIMITY, FieldUnitObj) // read only
             External(MEMORY_SLOT_ENABLED, FieldUnitObj) // 1 if enabled, read only
             External(MEMORY_SLOT_INSERT_EVENT, FieldUnitObj) // (read) 1 if has a insert event. (write) 1 to clear event
+            External(MEMORY_SLOT_REMOVE_EVENT, FieldUnitObj) // (read) 1 if has a remove event. (write) 1 to clear event
+            External(MEMORY_SLOT_EJECT, FieldUnitObj) // initiates device eject, write only
             External(MEMORY_SLOT_SLECTOR, FieldUnitObj) // DIMM selector, write only
             External(MEMORY_SLOT_OST_EVENT, FieldUnitObj) // _OST event code, write only
             External(MEMORY_SLOT_OST_STATUS, FieldUnitObj) // _OST status code, write only
@@ -55,8 +57,10 @@
                     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)
+                        Store(1, MEMORY_SLOT_REMOVE_EVENT)
                     }
-                    // TODO: handle memory eject request
                     Add(Local0, One, Local0) // goto next DIMM
                 }
                 Release(MEMORY_SLOT_LOCK)
@@ -156,5 +160,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(1, MEMORY_SLOT_EJECT)
+                Release(MEMORY_SLOT_LOCK)
+            }
         } // Device()
     } // Scope()
diff --git a/include/hw/acpi/pc-hotplug.h b/include/hw/acpi/pc-hotplug.h
index efa6ed7..0513c1c 100644
--- a/include/hw/acpi/pc-hotplug.h
+++ b/include/hw/acpi/pc-hotplug.h
@@ -43,6 +43,8 @@
 #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_EJECT            MEJ
 #define MEMORY_SLOT_SLECTOR          MSEL
 #define MEMORY_SLOT_OST_EVENT        MOEV
 #define MEMORY_SLOT_OST_STATUS       MOSC
@@ -51,6 +53,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
 
diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index 4e673f9..5b7acf1 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);
diff --git a/trace-events b/trace-events
index 30eba92..fabdd78 100644
--- a/trace-events
+++ b/trace-events
@@ -1562,6 +1562,7 @@ vfio_put_base_device(int fd) "close vdev->fd=%d"
 
 #hw/acpi/memory_hotplug.c
 mhp_acpi_invalid_slot_selected(uint32_t slot) "0x%"PRIx32
+mhp_acpi_ejecting_invalid_slot(uint32_t slot) "0x%"PRIx32
 mhp_acpi_read_addr_lo(uint32_t slot, uint32_t addr) "slot[0x%"PRIx32"] addr lo: 0x%"PRIx32
 mhp_acpi_read_addr_hi(uint32_t slot, uint32_t addr) "slot[0x%"PRIx32"] addr hi: 0x%"PRIx32
 mhp_acpi_read_size_lo(uint32_t slot, uint32_t size) "slot[0x%"PRIx32"] size lo: 0x%"PRIx32
@@ -1572,6 +1573,7 @@ mhp_acpi_write_slot(uint32_t slot) "set active slot: 0x%"PRIx32
 mhp_acpi_write_ost_ev(uint32_t slot, uint32_t ev) "slot[0x%"PRIx32"] OST EVENT: 0x%"PRIx32
 mhp_acpi_write_ost_status(uint32_t slot, uint32_t st) "slot[0x%"PRIx32"] OST STATUS: 0x%"PRIx32
 mhp_acpi_clear_insert_evt(uint32_t slot) "slot[0x%"PRIx32"] clear insert event"
+mhp_acpi_clear_remove_evt(uint32_t slot) "slot[0x%"PRIx32"] clear remove event"
 
 # hw/i386/pc.c
 mhp_pc_dimm_assigned_slot(int slot) "0x%d"
-- 
1.9.3

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [Qemu-devel] [PATCH v5 7/7] qmp-event: add event notification for memory hot unplug error
  2015-03-27  9:20 [Qemu-devel] [PATCH v5 0/7] QEMU memory hot unplug support Zhu Guihua
                   ` (5 preceding siblings ...)
  2015-03-27  9:20 ` [Qemu-devel] [PATCH v5 6/7] acpi: add hardware implementation for memory hot unplug Zhu Guihua
@ 2015-03-27  9:20 ` Zhu Guihua
  2015-03-27 14:53   ` Eric Blake
  2015-03-30  2:53 ` [Qemu-devel] [PATCH v5 0/7] QEMU memory hot unplug support Zhu Guihua
  2015-04-01  8:42 ` Igor Mammedov
  8 siblings, 1 reply; 18+ messages in thread
From: Zhu Guihua @ 2015-03-27  9:20 UTC (permalink / raw)
  To: qemu-devel, imammedo, pbonzini; +Cc: guz.fnst, izumi.taku, Zhu Guihua, tangchen

When memory hot unplug fails, this patch adds support to send
QMP event to notify mgmt about this failure.

Signed-off-by: Zhu Guihua <zhugh.fnst@cn.fujitsu.com>
---
 docs/qmp/qmp-events.txt  | 17 +++++++++++++++++
 hw/acpi/memory_hotplug.c |  8 +++++++-
 monitor.c                |  1 +
 qapi/event.json          | 14 ++++++++++++++
 4 files changed, 39 insertions(+), 1 deletion(-)

diff --git a/docs/qmp/qmp-events.txt b/docs/qmp/qmp-events.txt
index d759d19..7a05705 100644
--- a/docs/qmp/qmp-events.txt
+++ b/docs/qmp/qmp-events.txt
@@ -226,6 +226,23 @@ Example:
 { "event": "GUEST_PANICKED",
      "data": { "action": "pause" } }
 
+MEM_HOT_UNPLUG_ERROR
+--------------------
+Emitted when memory hot unplug occurs error.
+
+Data:
+
+- "device": device name (json-string)
+- "msg": Informative message (e.g., reason for the error) (json-string)
+
+Example:
+
+{ "event": "MEM_HOT_UNPLUG_ERROR"
+  "data": { "device": "dimm1",
+            "msg": "acpi: device unplug for not supported device"
+  },
+  "timestamp": { "seconds": 1265044230, "microseconds": 450486 } }
+
 NIC_RX_FILTER_CHANGED
 ---------------------
 
diff --git a/hw/acpi/memory_hotplug.c b/hw/acpi/memory_hotplug.c
index 2a1b866..f1cef71 100644
--- a/hw/acpi/memory_hotplug.c
+++ b/hw/acpi/memory_hotplug.c
@@ -94,6 +94,7 @@ static void acpi_memory_hotplug_write(void *opaque, hwaddr addr, uint64_t data,
     ACPIOSTInfo *info;
     DeviceState *dev = NULL;
     HotplugHandler *hotplug_ctrl = NULL;
+    Error *local_err = NULL;
 
     if (!mem_st->dev_count) {
         return;
@@ -148,7 +149,12 @@ static void acpi_memory_hotplug_write(void *opaque, hwaddr addr, uint64_t data,
             dev = DEVICE(mdev->dimm);
             hotplug_ctrl = qdev_get_hotplug_handler(dev);
             /* call pc-dimm unplug cb */
-            hotplug_handler_unplug(hotplug_ctrl, dev, NULL);
+            hotplug_handler_unplug(hotplug_ctrl, dev, &local_err);
+            if (local_err) {
+                qapi_event_send_mem_unplug_error(dev->id,
+                                                 error_get_pretty(local_err),
+                                                 &error_abort);
+            }
         }
         break;
     default:
diff --git a/monitor.c b/monitor.c
index 68873ec..d52ab87 100644
--- a/monitor.c
+++ b/monitor.c
@@ -587,6 +587,7 @@ static void monitor_qapi_event_init(void)
     monitor_qapi_event_throttle(QAPI_EVENT_QUORUM_REPORT_BAD, 1000);
     monitor_qapi_event_throttle(QAPI_EVENT_QUORUM_FAILURE, 1000);
     monitor_qapi_event_throttle(QAPI_EVENT_VSERPORT_CHANGE, 1000);
+    monitor_qapi_event_throttle(QAPI_EVENT_MEM_UNPLUG_ERROR, 1000);
 
     qmp_event_set_func_emit(monitor_qapi_event_queue);
 }
diff --git a/qapi/event.json b/qapi/event.json
index c51dc49..862df67 100644
--- a/qapi/event.json
+++ b/qapi/event.json
@@ -330,3 +330,17 @@
 ##
 { 'event': 'VSERPORT_CHANGE',
   'data': { 'id': 'str', 'open': 'bool' } }
+
+##
+# @MEM_UNPLUG_ERROR
+#
+# Emitted when memory hot unplug occurs error.
+#
+# @device: device name
+#
+# @msg: Informative message
+#
+# Since: 2.3
+##
+{ 'event': 'MEM_UNPLUG_ERROR',
+  'data': { 'device': 'str', 'msg': 'str' } }
-- 
1.9.3

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [Qemu-devel] [PATCH v5 7/7] qmp-event: add event notification for memory hot unplug error
  2015-03-27  9:20 ` [Qemu-devel] [PATCH v5 7/7] qmp-event: add event notification for memory hot unplug error Zhu Guihua
@ 2015-03-27 14:53   ` Eric Blake
  2015-03-30  3:07     ` Zhu Guihua
  0 siblings, 1 reply; 18+ messages in thread
From: Eric Blake @ 2015-03-27 14:53 UTC (permalink / raw)
  To: Zhu Guihua, qemu-devel, imammedo, pbonzini; +Cc: guz.fnst, izumi.taku, tangchen

[-- Attachment #1: Type: text/plain, Size: 3140 bytes --]

On 03/27/2015 03:20 AM, Zhu Guihua wrote:
> When memory hot unplug fails, this patch adds support to send
> QMP event to notify mgmt about this failure.
> 
> Signed-off-by: Zhu Guihua <zhugh.fnst@cn.fujitsu.com>
> ---
>  docs/qmp/qmp-events.txt  | 17 +++++++++++++++++
>  hw/acpi/memory_hotplug.c |  8 +++++++-
>  monitor.c                |  1 +
>  qapi/event.json          | 14 ++++++++++++++
>  4 files changed, 39 insertions(+), 1 deletion(-)
> 
> diff --git a/docs/qmp/qmp-events.txt b/docs/qmp/qmp-events.txt
> index d759d19..7a05705 100644
> --- a/docs/qmp/qmp-events.txt
> +++ b/docs/qmp/qmp-events.txt
> @@ -226,6 +226,23 @@ Example:
>  { "event": "GUEST_PANICKED",
>       "data": { "action": "pause" } }
>  
> +MEM_HOT_UNPLUG_ERROR
> +--------------------
> +Emitted when memory hot unplug occurs error.

s/occurs error/error occurs/

> +
> +Data:
> +
> +- "device": device name (json-string)
> +- "msg": Informative message (e.g., reason for the error) (json-string)
> +
> +Example:
> +
> +{ "event": "MEM_HOT_UNPLUG_ERROR"
> +  "data": { "device": "dimm1",
> +            "msg": "acpi: device unplug for not supported device"

s/not supported/unsupported/

> +  },
> +  "timestamp": { "seconds": 1265044230, "microseconds": 450486 } }
> +
>  NIC_RX_FILTER_CHANGED
>  ---------------------
>  
> diff --git a/hw/acpi/memory_hotplug.c b/hw/acpi/memory_hotplug.c
> index 2a1b866..f1cef71 100644
> --- a/hw/acpi/memory_hotplug.c
> +++ b/hw/acpi/memory_hotplug.c
> @@ -94,6 +94,7 @@ static void acpi_memory_hotplug_write(void *opaque, hwaddr addr, uint64_t data,
>      ACPIOSTInfo *info;
>      DeviceState *dev = NULL;
>      HotplugHandler *hotplug_ctrl = NULL;
> +    Error *local_err = NULL;
>  
>      if (!mem_st->dev_count) {
>          return;
> @@ -148,7 +149,12 @@ static void acpi_memory_hotplug_write(void *opaque, hwaddr addr, uint64_t data,
>              dev = DEVICE(mdev->dimm);
>              hotplug_ctrl = qdev_get_hotplug_handler(dev);
>              /* call pc-dimm unplug cb */
> -            hotplug_handler_unplug(hotplug_ctrl, dev, NULL);
> +            hotplug_handler_unplug(hotplug_ctrl, dev, &local_err);
> +            if (local_err) {
> +                qapi_event_send_mem_unplug_error(dev->id,
> +                                                 error_get_pretty(local_err),
> +                                                 &error_abort);
> +            }
>          }

Does this leak local_err?

> +++ b/qapi/event.json
> @@ -330,3 +330,17 @@
>  ##
>  { 'event': 'VSERPORT_CHANGE',
>    'data': { 'id': 'str', 'open': 'bool' } }
> +
> +##
> +# @MEM_UNPLUG_ERROR
> +#
> +# Emitted when memory hot unplug occurs error.

s/occurs error/error occurs/

> +#
> +# @device: device name
> +#
> +# @msg: Informative message
> +#
> +# Since: 2.3

You're awfully late for 2.3; this may need to be 2.4.

> +##
> +{ 'event': 'MEM_UNPLUG_ERROR',
> +  'data': { 'device': 'str', 'msg': 'str' } }
> 

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [Qemu-devel] [PATCH v5 0/7] QEMU memory hot unplug support
  2015-03-27  9:20 [Qemu-devel] [PATCH v5 0/7] QEMU memory hot unplug support Zhu Guihua
                   ` (6 preceding siblings ...)
  2015-03-27  9:20 ` [Qemu-devel] [PATCH v5 7/7] qmp-event: add event notification for memory hot unplug error Zhu Guihua
@ 2015-03-30  2:53 ` Zhu Guihua
  2015-03-30  7:49   ` Paolo Bonzini
  2015-04-01  8:42 ` Igor Mammedov
  8 siblings, 1 reply; 18+ messages in thread
From: Zhu Guihua @ 2015-03-30  2:53 UTC (permalink / raw)
  To: qemu-devel, imammedo, quintela, amit.shah, pbonzini
  Cc: guz.fnst, izumi.taku, tangchen

Question:

When migrating from old qemu to one with this hot remove feature, I found
this feature would not work any more in new qemu.

The reason is that DSDT table will not re-generate in new qemu when 
migration.
So the hot remove will not work in the new qemu even though qemu
has this feature.

Is it possible to re-generate dsdt table when migration?

Thanks,
Zhu




On 03/27/2015 05:20 PM, Zhu Guihua wrote:
> Memory hot unplug are both asynchronous procedures.
> When the unplug operation happens, unplug request cb is called first.
> And when guest OS finished handling unplug, unplug cb will be called
> to do the real removal of device.
>
> v5:
>   -reorganize the patchset
>   -add documentation to understand patch easily
>   -add MEMORY_SLOT_EJECT for initiating device eject
>   -add support to send qmp event to notify mgmt about memory unplug error
>
> v4:
>   -reorganize the patchset
>   -drop the new API acpi_send_gpe_event()
>   -update ssdt-mem
>
> v3:
>   -commit message changes
>   -reorganize the patchset, squash and separate some patches
>   -update specs about acpi_mem_hotplug
>   -first cleanup external state, then un-map and un-register memory device
>
> 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
>
> Tang Chen (3):
>    acpi, mem-hotplug: add acpi_memory_slot_status() to get MemStatus
>    acpi, mem-hotplug: add unplug request cb for memory device
>    acpi, mem-hotplug: add unplug cb for memory device
>
> Zhu Guihua (4):
>    docs: update documentation for memory hot unplug
>    acpi: extend aml_field() to support UpdateRule
>    acpi: add hardware implementation for memory hot unplug
>    qmp-event: add event notification for memory hot unplug error
>
>   docs/memory-hotplug.txt           | 24 ++++++++--
>   docs/qmp/qmp-events.txt           | 17 +++++++
>   docs/specs/acpi_mem_hotplug.txt   | 32 ++++++++++++--
>   hw/acpi/aml-build.c               |  4 +-
>   hw/acpi/ich9.c                    | 19 ++++++--
>   hw/acpi/memory_hotplug.c          | 93 ++++++++++++++++++++++++++++++++++++---
>   hw/acpi/piix4.c                   | 17 +++++--
>   hw/core/qdev.c                    |  2 +-
>   hw/i386/acpi-build.c              | 25 ++++++++---
>   hw/i386/acpi-dsdt-mem-hotplug.dsl | 13 +++++-
>   hw/i386/pc.c                      | 62 ++++++++++++++++++++++++--
>   include/hw/acpi/aml-build.h       | 10 ++++-
>   include/hw/acpi/memory_hotplug.h  | 12 +++++
>   include/hw/acpi/pc-hotplug.h      |  3 ++
>   include/hw/qdev-core.h            |  1 +
>   monitor.c                         |  1 +
>   qapi/event.json                   | 14 ++++++
>   trace-events                      |  2 +
>   18 files changed, 315 insertions(+), 36 deletions(-)
>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [Qemu-devel] [PATCH v5 7/7] qmp-event: add event notification for memory hot unplug error
  2015-03-27 14:53   ` Eric Blake
@ 2015-03-30  3:07     ` Zhu Guihua
  0 siblings, 0 replies; 18+ messages in thread
From: Zhu Guihua @ 2015-03-30  3:07 UTC (permalink / raw)
  To: Eric Blake, qemu-devel, imammedo, pbonzini; +Cc: guz.fnst, izumi.taku, tangchen


On 03/27/2015 10:53 PM, Eric Blake wrote:
> On 03/27/2015 03:20 AM, Zhu Guihua wrote:
>> When memory hot unplug fails, this patch adds support to send
>> QMP event to notify mgmt about this failure.
>>
>> Signed-off-by: Zhu Guihua <zhugh.fnst@cn.fujitsu.com>
>> ---
>>   docs/qmp/qmp-events.txt  | 17 +++++++++++++++++
>>   hw/acpi/memory_hotplug.c |  8 +++++++-
>>   monitor.c                |  1 +
>>   qapi/event.json          | 14 ++++++++++++++
>>   4 files changed, 39 insertions(+), 1 deletion(-)
>>
>> diff --git a/docs/qmp/qmp-events.txt b/docs/qmp/qmp-events.txt
>> index d759d19..7a05705 100644
>> --- a/docs/qmp/qmp-events.txt
>> +++ b/docs/qmp/qmp-events.txt
>> @@ -226,6 +226,23 @@ Example:
>>   { "event": "GUEST_PANICKED",
>>        "data": { "action": "pause" } }
>>   
>> +MEM_HOT_UNPLUG_ERROR
>> +--------------------
>> +Emitted when memory hot unplug occurs error.
> s/occurs error/error occurs/
>
>> +
>> +Data:
>> +
>> +- "device": device name (json-string)
>> +- "msg": Informative message (e.g., reason for the error) (json-string)
>> +
>> +Example:
>> +
>> +{ "event": "MEM_HOT_UNPLUG_ERROR"
>> +  "data": { "device": "dimm1",
>> +            "msg": "acpi: device unplug for not supported device"
> s/not supported/unsupported/
>
>> +  },
>> +  "timestamp": { "seconds": 1265044230, "microseconds": 450486 } }
>> +
>>   NIC_RX_FILTER_CHANGED
>>   ---------------------
>>   
>> diff --git a/hw/acpi/memory_hotplug.c b/hw/acpi/memory_hotplug.c
>> index 2a1b866..f1cef71 100644
>> --- a/hw/acpi/memory_hotplug.c
>> +++ b/hw/acpi/memory_hotplug.c
>> @@ -94,6 +94,7 @@ static void acpi_memory_hotplug_write(void *opaque, hwaddr addr, uint64_t data,
>>       ACPIOSTInfo *info;
>>       DeviceState *dev = NULL;
>>       HotplugHandler *hotplug_ctrl = NULL;
>> +    Error *local_err = NULL;
>>   
>>       if (!mem_st->dev_count) {
>>           return;
>> @@ -148,7 +149,12 @@ static void acpi_memory_hotplug_write(void *opaque, hwaddr addr, uint64_t data,
>>               dev = DEVICE(mdev->dimm);
>>               hotplug_ctrl = qdev_get_hotplug_handler(dev);
>>               /* call pc-dimm unplug cb */
>> -            hotplug_handler_unplug(hotplug_ctrl, dev, NULL);
>> +            hotplug_handler_unplug(hotplug_ctrl, dev, &local_err);
>> +            if (local_err) {
>> +                qapi_event_send_mem_unplug_error(dev->id,
>> +                                                 error_get_pretty(local_err),
>> +                                                 &error_abort);
>> +            }
>>           }
> Does this leak local_err?

Here should handle this error not to forget it.
It is a good way to send qmp event to notify mgmt.

>
>> +++ b/qapi/event.json
>> @@ -330,3 +330,17 @@
>>   ##
>>   { 'event': 'VSERPORT_CHANGE',
>>     'data': { 'id': 'str', 'open': 'bool' } }
>> +
>> +##
>> +# @MEM_UNPLUG_ERROR
>> +#
>> +# Emitted when memory hot unplug occurs error.
> s/occurs error/error occurs/
>
>> +#
>> +# @device: device name
>> +#
>> +# @msg: Informative message
>> +#
>> +# Since: 2.3
> You're awfully late for 2.3; this may need to be 2.4.

Got it, thanks.

Regards,
Zhu

>
>> +##
>> +{ 'event': 'MEM_UNPLUG_ERROR',
>> +  'data': { 'device': 'str', 'msg': 'str' } }
>>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [Qemu-devel] [PATCH v5 0/7] QEMU memory hot unplug support
  2015-03-30  2:53 ` [Qemu-devel] [PATCH v5 0/7] QEMU memory hot unplug support Zhu Guihua
@ 2015-03-30  7:49   ` Paolo Bonzini
  0 siblings, 0 replies; 18+ messages in thread
From: Paolo Bonzini @ 2015-03-30  7:49 UTC (permalink / raw)
  To: Zhu Guihua, qemu-devel, imammedo, quintela, amit.shah
  Cc: guz.fnst, izumi.taku, tangchen



On 30/03/2015 04:53, Zhu Guihua wrote:
> Question:
> 
> When migrating from old qemu to one with this hot remove feature, I found
> this feature would not work any more in new qemu.
> 
> The reason is that DSDT table will not re-generate in new qemu when
> migration.
> So the hot remove will not work in the new qemu even though qemu
> has this feature.
> 
> Is it possible to re-generate dsdt table when migration?

No, consider the case when the OS is _executing_ the DSDT table during
migration!

You can either ignore the issue, or disable hot-unplug for older machine
types---that is, only enable it for pc-2.4.  Then you'll get consistent
behavior wherever the VM is started in a cluster.

Paolo

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [Qemu-devel] [PATCH v5 6/7] acpi: add hardware implementation for memory hot unplug
  2015-03-27  9:20 ` [Qemu-devel] [PATCH v5 6/7] acpi: add hardware implementation for memory hot unplug Zhu Guihua
@ 2015-03-30  9:09   ` Igor Mammedov
  0 siblings, 0 replies; 18+ messages in thread
From: Igor Mammedov @ 2015-03-30  9:09 UTC (permalink / raw)
  To: Zhu Guihua; +Cc: guz.fnst, pbonzini, tangchen, qemu-devel, izumi.taku

On Fri, 27 Mar 2015 17:20:39 +0800
Zhu Guihua <zhugh.fnst@cn.fujitsu.com> wrote:

> - implements QEMU hardware part of memory hot unplug protocol
>   described at "docs/spec/acpi_mem_hotplug.txt"
> - handles memory remove notification event
> - handles device eject notification
> 
> Signed-off-by: Zhu Guihua <zhugh.fnst@cn.fujitsu.com>
> ---
>  docs/specs/acpi_mem_hotplug.txt   | 16 +++++++++++++---
>  hw/acpi/memory_hotplug.c          | 20 +++++++++++++++++++-
>  hw/core/qdev.c                    |  2 +-
>  hw/i386/acpi-build.c              | 12 ++++++++++++
>  hw/i386/acpi-dsdt-mem-hotplug.dsl | 13 ++++++++++++-
>  include/hw/acpi/pc-hotplug.h      |  3 +++
>  include/hw/qdev-core.h            |  1 +
>  trace-events                      |  2 ++
>  8 files changed, 63 insertions(+), 6 deletions(-)
> 
> diff --git a/docs/specs/acpi_mem_hotplug.txt b/docs/specs/acpi_mem_hotplug.txt
> index a88b286..96f9abe 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.
According to ACPI spec it's not device check but "Eject request"

> +              3-7: reserved and should be ignored by OSPM
>        [0x15-0x17] reserved
>  
>    write access:
> @@ -31,11 +33,19 @@ Memory hot-plug interface (IO port 0xa00-0xa17, 1-4 byte access):
>        [0xc-0x13] reserved, writes into it are ignored
>        [0x14] Memory device control fields
>            bits:
> -              0: reserved, OSPM must clear it before writing to register
> +              0: reserved, OSPM must clear it before writing to register.
> +                 Due to BUG in versions prior 2.4 that field isn't
> +                 cleared when other fields are written.Keep it reserved
> +                 and don't try to reuse it.
>                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
same as above

> +              3: if set to 1 initiates device eject, set by OSPM when it
> +                 triggers memory device removal and calls _EJ0 method
> +              4-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 07e281f..2a1b866 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"
>  
> @@ -91,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;
> @@ -128,13 +131,28 @@ static void acpi_memory_hotplug_write(void *opaque, hwaddr addr, uint64_t data,
>          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) {
> +            mdev->is_removing = false;
> +            trace_mhp_acpi_clear_remove_evt(mem_st->selector);
> +        } else if (data & 8) {
> +            if (!mdev->is_enabled) {
> +                trace_mhp_acpi_ejecting_invalid_slot(mem_st->selector);
why isn't there any trace event for success path?

> +                break;
> +            }
> +
> +            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;
>      }
>  
>  }
> diff --git a/hw/core/qdev.c b/hw/core/qdev.c
> index 6e6a65d..b0f0f84 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/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index b767cbe..927250e 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -931,6 +931,12 @@ build_ssdt(GArray *table_data, GArray *linker,
>          aml_append(field,
>              /*(read) 1 if has a insert event. (write) 1 to clear event */
>              aml_named_field(stringify(MEMORY_SLOT_INSERT_EVENT), 1));
> +        aml_append(field,
> +            /* (read) 1 if has a remove event. (write) 1 to clear event */
> +            aml_named_field(stringify(MEMORY_SLOT_REMOVE_EVENT), 1));
> +        aml_append(field,
> +            /* initiates device eject, write only */
> +            aml_named_field(stringify(MEMORY_SLOT_EJECT), 1));
>          aml_append(scope, field);
>  
>          field = aml_field(stringify(MEMORY_HOTPLUG_IO_REGION), aml_dword_acc,
> @@ -975,6 +981,12 @@ build_ssdt(GArray *table_data, GArray *linker,
>              )));
>              aml_append(dev, method);
>  
> +            method = aml_method("_EJ0", 1);
> +            s = BASEPATH stringify(MEMORY_SLOT_EJECT_METHOD);
> +            aml_append(method, aml_return(aml_call2(
> +                       s, aml_name("_UID"), aml_arg(0))));
> +            aml_append(dev, method);
> +
>              aml_append(sb_scope, dev);
>          }
>  
> diff --git a/hw/i386/acpi-dsdt-mem-hotplug.dsl b/hw/i386/acpi-dsdt-mem-hotplug.dsl
> index 1e9ec39..c2bb6a1 100644
> --- a/hw/i386/acpi-dsdt-mem-hotplug.dsl
> +++ b/hw/i386/acpi-dsdt-mem-hotplug.dsl
> @@ -29,6 +29,8 @@
>              External(MEMORY_SLOT_PROXIMITY, FieldUnitObj) // read only
>              External(MEMORY_SLOT_ENABLED, FieldUnitObj) // 1 if enabled, read only
>              External(MEMORY_SLOT_INSERT_EVENT, FieldUnitObj) // (read) 1 if has a insert event. (write) 1 to clear event
> +            External(MEMORY_SLOT_REMOVE_EVENT, FieldUnitObj) // (read) 1 if has a remove event. (write) 1 to clear event
> +            External(MEMORY_SLOT_EJECT, FieldUnitObj) // initiates device eject, write only
>              External(MEMORY_SLOT_SLECTOR, FieldUnitObj) // DIMM selector, write only
>              External(MEMORY_SLOT_OST_EVENT, FieldUnitObj) // _OST event code, write only
>              External(MEMORY_SLOT_OST_STATUS, FieldUnitObj) // _OST status code, write only
> @@ -55,8 +57,10 @@
>                      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)
> +                        Store(1, MEMORY_SLOT_REMOVE_EVENT)
>                      }
> -                    // TODO: handle memory eject request
>                      Add(Local0, One, Local0) // goto next DIMM
>                  }
>                  Release(MEMORY_SLOT_LOCK)
> @@ -156,5 +160,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(1, MEMORY_SLOT_EJECT)
> +                Release(MEMORY_SLOT_LOCK)
> +            }
>          } // Device()
>      } // Scope()
> diff --git a/include/hw/acpi/pc-hotplug.h b/include/hw/acpi/pc-hotplug.h
> index efa6ed7..0513c1c 100644
> --- a/include/hw/acpi/pc-hotplug.h
> +++ b/include/hw/acpi/pc-hotplug.h
> @@ -43,6 +43,8 @@
>  #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_EJECT            MEJ
>  #define MEMORY_SLOT_SLECTOR          MSEL
>  #define MEMORY_SLOT_OST_EVENT        MOEV
>  #define MEMORY_SLOT_OST_STATUS       MOSC
> @@ -51,6 +53,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
>  
> diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
> index 4e673f9..5b7acf1 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);
> diff --git a/trace-events b/trace-events
> index 30eba92..fabdd78 100644
> --- a/trace-events
> +++ b/trace-events
> @@ -1562,6 +1562,7 @@ vfio_put_base_device(int fd) "close vdev->fd=%d"
>  
>  #hw/acpi/memory_hotplug.c
>  mhp_acpi_invalid_slot_selected(uint32_t slot) "0x%"PRIx32
> +mhp_acpi_ejecting_invalid_slot(uint32_t slot) "0x%"PRIx32
>  mhp_acpi_read_addr_lo(uint32_t slot, uint32_t addr) "slot[0x%"PRIx32"] addr lo: 0x%"PRIx32
>  mhp_acpi_read_addr_hi(uint32_t slot, uint32_t addr) "slot[0x%"PRIx32"] addr hi: 0x%"PRIx32
>  mhp_acpi_read_size_lo(uint32_t slot, uint32_t size) "slot[0x%"PRIx32"] size lo: 0x%"PRIx32
> @@ -1572,6 +1573,7 @@ mhp_acpi_write_slot(uint32_t slot) "set active slot: 0x%"PRIx32
>  mhp_acpi_write_ost_ev(uint32_t slot, uint32_t ev) "slot[0x%"PRIx32"] OST EVENT: 0x%"PRIx32
>  mhp_acpi_write_ost_status(uint32_t slot, uint32_t st) "slot[0x%"PRIx32"] OST STATUS: 0x%"PRIx32
>  mhp_acpi_clear_insert_evt(uint32_t slot) "slot[0x%"PRIx32"] clear insert event"
> +mhp_acpi_clear_remove_evt(uint32_t slot) "slot[0x%"PRIx32"] clear remove event"
>  
>  # hw/i386/pc.c
>  mhp_pc_dimm_assigned_slot(int slot) "0x%d"

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [Qemu-devel] [PATCH v5 1/7] docs: update documentation for memory hot unplug
  2015-03-27  9:20 ` [Qemu-devel] [PATCH v5 1/7] docs: update documentation for memory hot unplug Zhu Guihua
@ 2015-03-30 15:47   ` Igor Mammedov
  0 siblings, 0 replies; 18+ messages in thread
From: Igor Mammedov @ 2015-03-30 15:47 UTC (permalink / raw)
  To: Zhu Guihua; +Cc: guz.fnst, pbonzini, tangchen, qemu-devel, izumi.taku

On Fri, 27 Mar 2015 17:20:34 +0800
Zhu Guihua <zhugh.fnst@cn.fujitsu.com> wrote:

> Add specification about how to use memory hot unplug, and add
> a flow diagram to explain memory hot unplug process.
> 
> Signed-off-by: Zhu Guihua <zhugh.fnst@cn.fujitsu.com>
> ---
>  docs/memory-hotplug.txt         | 24 ++++++++++++++++++++----
>  docs/specs/acpi_mem_hotplug.txt | 16 +++++++++++++++-
>  2 files changed, 35 insertions(+), 5 deletions(-)
> 
> diff --git a/docs/memory-hotplug.txt b/docs/memory-hotplug.txt
> index f70571d..0de7f1b 100644
> --- a/docs/memory-hotplug.txt
> +++ b/docs/memory-hotplug.txt
> @@ -1,12 +1,10 @@
>  QEMU memory hotplug
>  ===================
>  
> -This document explains how to use the memory hotplug feature in QEMU,
> +This part explains how to use the memory hotplug feature in QEMU,
>  which is present since v2.1.0.
not related change? It's better to leave as it was.

>  
> -Please, note that memory hotunplug is not supported yet. This means
> -that you're able to add memory, but you're not able to remove it.
> -Also, proper guest support is required for memory hotplug to work.
> +And, proper guest support is required for memory hotplug to work.
reword it like:
Guest support is required for memory hotplug to work.

>  
>  Basic RAM hotplug
>  -----------------
> @@ -74,3 +72,21 @@ comes from regular RAM, 1GB is a 1GB hugepage page and 256MB is from
>     -device pc-dimm,id=dimm1,memdev=mem1 \
>     -object memory-backend-file,id=mem2,size=256M,mem-path=/mnt/hugepages-2MB \
>     -device pc-dimm,id=dimm2,memdev=mem2
> +
> +
> +QEMU memory hot unplug
> +======================
s/^^^/
RAM hot-unplug
--------------
/

> +This part explains how to use the memory hot unplug feature in QEMU.
unnecessary, drop it.

> +
> +In order to be able to hot unplug pc-dimm device, QEMU has to be told the id
> +of pc-dimm device. The id was assigned when you hot plugged memory.
> +
> +One monitor command is used to hot unplug memory:
s/One/Two/

Haven't you forgotten about object_del ???
In typical use-case backend will be deleted as well

> +
> + - "device_del": deletes a front-end pc-dimm device
> +
> +For example, assuming that the pc-dimm device with id "dimm1"   
> +into the guest, the following command deletes the pc-dimm device from the
> +guest.
s/was hotplugged into the guest/exists/
s/deletes the pc-dimm device from the guest/tries to remove it/

> +
> +  (qemu) device_del dimm1
remove associated backend as well

> diff --git a/docs/specs/acpi_mem_hotplug.txt b/docs/specs/acpi_mem_hotplug.txt
> index 1290994..a88b286 100644
> --- a/docs/specs/acpi_mem_hotplug.txt
> +++ b/docs/specs/acpi_mem_hotplug.txt
> @@ -2,7 +2,7 @@ QEMU<->ACPI BIOS memory hotplug interface
>  --------------------------------------
>  
>  ACPI BIOS GPE.3 handler is dedicated for notifying OS about memory hot-add
> -events.
> +and hot-remove events.
>  
>  Memory hot-plug interface (IO port 0xa00-0xa17, 1-4 byte access):
>  ---------------------------------------------------------------
> @@ -42,3 +42,17 @@ Selecting memory device slot beyond present range has no effect on platform:
>       ignored
>     - read accesses to memory hot-plug registers not documented above return
>       all bits set to 1.
> +
> +Memory hot remove process diagram:
> +----------------------------------


> +Memory hot unplug is rather complicated multi-stage process, it is asynchronous
> +procedures. The follwoing diagram describes memory hot unplug process.
drop ^^^ hunk


> +
> +             QEMU             |             Guest OS
> +-------------------------------------------------------------------------------
> +     unplug request cb ->     |
> +                              |  OSPM processes ejection request
> +                              |  <- OSPM clears device remove event
> +    clear remove event ->     |
> +                              |  <- OSPM issues _EJ0 and initiates device eject
> +  unplug cb (eject device)    |
perhaps something like below would be better

 +-------------+     +-----------------------+      +------------------+     
 |  1. QEMU    |     | 2. QEMU               |      |3. QEMU           |     
 |  device_del +---->+ device unplug request +----->+Send SCI to guest,|     
 |             |     |         cb            |      |return control to |     
 +-------------+     +-----------------------+      |management        |     
                                                    +------------------+     
                                                                             
 +---------------------------------------------------------------------+     
                                                                             
 +---------------------+              +-------------------------+            
 | OSPM:               | remove event | OSPM:                   |            
 | send Eject Request, |              | Scan memory devices     |            
 | clear remove event  +<-------------+ for event flags         |            
 |                     |              |                         |            
 +---------------------+              +-------------------------+            
           |                                                                 
           |                                                                 
 +---------v--------+            +-----------------------+                   
 | Guest OS:        |  success   | OSPM:                 |                   
 | process Ejection +----------->+ Execute _EJ00 method, |                   
 | request          |            | set eject bit in flags|                   
 +------------------+            +-----------------------+                   
           |failure                         |                                
           v                                v                                
 +------------------------+      +-----------------------+                   
 | OSPM:                  |      | QEMU:                 |                   
 | set OST event & status |      | call device unplug cb |                   
 | fields                 |      |                       |                   
 +------------------------+      +-----------------------+                   
          |                                  |                               
          v                                  v                               
 +------------------+              +-------------------+                     
 |QEMU:             |              |QEMU:              |                     
 |Send OST QMP event|              |Send device deleted|                     
 |                  |              |QMP event          |                     
 +------------------+              |                   |                     
                                   +-------------------+                     
                                                                             
                                                                             

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [Qemu-devel] [PATCH v5 3/7] acpi, mem-hotplug: add unplug request cb for memory device
  2015-03-27  9:20 ` [Qemu-devel] [PATCH v5 3/7] acpi, mem-hotplug: add unplug request cb for memory device Zhu Guihua
@ 2015-04-01  8:23   ` Igor Mammedov
  0 siblings, 0 replies; 18+ messages in thread
From: Igor Mammedov @ 2015-04-01  8:23 UTC (permalink / raw)
  To: Zhu Guihua; +Cc: guz.fnst, pbonzini, tangchen, qemu-devel, izumi.taku

On Fri, 27 Mar 2015 17:20:36 +0800
Zhu Guihua <zhugh.fnst@cn.fujitsu.com> wrote:

> From: Tang Chen <tangchen@cn.fujitsu.com>
> 
> This patch adds unplug request cb for memory device, and adds the
> is_removing boolean field to MemStatus. This field is used to indicate
> whether the memory slot is being removed. This field is set to true in
> acpi_memory_unplug_request_cb().
> 
> Signed-off-by: Tang Chen <tangchen@cn.fujitsu.com>
> Signed-off-by: Zhu Guihua <zhugh.fnst@cn.fujitsu.com>
> ---
>  hw/acpi/ich9.c                   | 10 ++++++++--
>  hw/acpi/memory_hotplug.c         | 19 +++++++++++++++++++
>  hw/acpi/piix4.c                  |  6 +++++-
>  hw/i386/pc.c                     | 28 ++++++++++++++++++++++++++--
>  include/hw/acpi/memory_hotplug.h | 10 ++++++++++
>  5 files changed, 68 insertions(+), 5 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,
> diff --git a/hw/acpi/memory_hotplug.c b/hw/acpi/memory_hotplug.c
> index 6af9303..42fe668 100644
> --- a/hw/acpi/memory_hotplug.c
> +++ b/hw/acpi/memory_hotplug.c
> @@ -75,6 +75,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:
> @@ -218,6 +219,24 @@ void acpi_memory_plug_cb(ACPIREGS *ar, qemu_irq irq, MemHotplugState *mem_st,
>      return;
>  }
>  
> +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 */
> +    ar->gpe.sts[0] |= ACPI_MEMORY_HOTPLUG_STATUS;
> +    acpi_update_sci(ar, irq);
> +}
> +
>  static const VMStateDescription vmstate_memhp_sts = {
>      .name = "memory hotplug device state",
>      .version_id = 1,
> diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
> index d1f1179..f716e91 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 {
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index a8e6be1..9b0859c 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -1677,6 +1677,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)
>  {
> @@ -1719,8 +1739,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,
> diff --git a/include/hw/acpi/memory_hotplug.h b/include/hw/acpi/memory_hotplug.h
> index 7bbf8a0..fbc2d65 100644
> --- a/include/hw/acpi/memory_hotplug.h
> +++ b/include/hw/acpi/memory_hotplug.h
> @@ -7,10 +7,17 @@
>  
>  #define ACPI_MEMORY_HOTPLUG_STATUS 8
>  
> +/**
> + * MemStatus:
> + * @is_removing: Indicates whether the memory slot has been removed.
reword like:
the memory device in slot has been requested to be ejected

> + *
> + * This structure stores memory device's status.
> + */
>  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 +35,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) \

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [Qemu-devel] [PATCH v5 5/7] acpi: extend aml_field() to support UpdateRule
  2015-03-27  9:20 ` [Qemu-devel] [PATCH v5 5/7] acpi: extend aml_field() to support UpdateRule Zhu Guihua
@ 2015-04-01  8:39   ` Igor Mammedov
  0 siblings, 0 replies; 18+ messages in thread
From: Igor Mammedov @ 2015-04-01  8:39 UTC (permalink / raw)
  To: Zhu Guihua; +Cc: guz.fnst, pbonzini, tangchen, qemu-devel, izumi.taku

On Fri, 27 Mar 2015 17:20:38 +0800
Zhu Guihua <zhugh.fnst@cn.fujitsu.com> wrote:

> The flags field is declared with default update rule 'Preserve',
> this patch is to extend aml_field() to support UpdateRule so that
s/is to extend/extends/
> we can specify different value to UpdateRule for different field.
s/value to UpdateRule for different/values per/

Also you forgot to mention that you are fixing
"Memory device control fields" register with reason why.
I'd suggest to only extend aml_field() in this patch and
put fix in a separate patch.

> 
> Signed-off-by: Zhu Guihua <zhugh.fnst@cn.fujitsu.com>
> ---
>  hw/acpi/aml-build.c         |  4 +++-
>  hw/i386/acpi-build.c        | 13 ++++++++-----
>  include/hw/acpi/aml-build.h | 10 ++++++++--
>  3 files changed, 19 insertions(+), 8 deletions(-)
> 
> diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
> index d7945f6..f926c9a 100644
> --- a/hw/acpi/aml-build.c
> +++ b/hw/acpi/aml-build.c
> @@ -635,9 +635,11 @@ Aml *aml_reserved_field(unsigned length)
>  }
>  
>  /* ACPI 1.0b: 16.2.5.2 Named Objects Encoding: DefField */
> -Aml *aml_field(const char *name, AmlFieldFlags flags)
> +Aml *aml_field(const char *name, AmlAccessType type, AmlUpdateRule rule)
>  {
>      Aml *var = aml_bundle(0x81 /* FieldOp */, AML_EXT_PACKAGE);
> +    uint8_t flags = rule << 5 | type;
> +
>      build_append_namestring(var->buf, "%s", name);
>      build_append_byte(var->buf, flags);
>      return var;
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index d0a5c85..b767cbe 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -798,7 +798,7 @@ build_ssdt(GArray *table_data, GArray *linker,
>  
>          aml_append(dev, aml_operation_region("PEOR", aml_system_io,
>                                                misc->pvpanic_port, 1));
> -        field = aml_field("PEOR", aml_byte_acc);
> +        field = aml_field("PEOR", aml_byte_acc, aml_preserve);
>          aml_append(field, aml_named_field("PEPT", 8));
>          aml_append(dev, field);
>  
> @@ -835,7 +835,7 @@ build_ssdt(GArray *table_data, GArray *linker,
>          /* declare CPU hotplug MMIO region and PRS field to access it */
>          aml_append(sb_scope, aml_operation_region(
>              "PRST", aml_system_io, pm->cpu_hp_io_base, pm->cpu_hp_io_len));
> -        field = aml_field("PRST", aml_byte_acc);
> +        field = aml_field("PRST", aml_byte_acc, aml_preserve);
>          aml_append(field, aml_named_field("PRS", 256));
>          aml_append(sb_scope, field);
>  
> @@ -909,7 +909,8 @@ build_ssdt(GArray *table_data, GArray *linker,
>              pm->mem_hp_io_base, pm->mem_hp_io_len)
>          );
>  
> -        field = aml_field(stringify(MEMORY_HOTPLUG_IO_REGION), aml_dword_acc);
> +        field = aml_field(stringify(MEMORY_HOTPLUG_IO_REGION), aml_dword_acc,
> +                                    aml_preserve);
>          aml_append(field, /* read only */
>              aml_named_field(stringify(MEMORY_SLOT_ADDR_LOW), 32));
>          aml_append(field, /* read only */
> @@ -922,7 +923,8 @@ build_ssdt(GArray *table_data, GArray *linker,
>              aml_named_field(stringify(MEMORY_SLOT_PROXIMITY), 32));
>          aml_append(scope, field);
>  
> -        field = aml_field(stringify(MEMORY_HOTPLUG_IO_REGION), aml_byte_acc);
> +        field = aml_field(stringify(MEMORY_HOTPLUG_IO_REGION), aml_byte_acc,
> +                          aml_write_as_zeros);
>          aml_append(field, aml_reserved_field(160 /* bits, Offset(20) */));
>          aml_append(field, /* 1 if enabled, read only */
>              aml_named_field(stringify(MEMORY_SLOT_ENABLED), 1));
> @@ -931,7 +933,8 @@ build_ssdt(GArray *table_data, GArray *linker,
>              aml_named_field(stringify(MEMORY_SLOT_INSERT_EVENT), 1));
>          aml_append(scope, field);
>  
> -        field = aml_field(stringify(MEMORY_HOTPLUG_IO_REGION), aml_dword_acc);
> +        field = aml_field(stringify(MEMORY_HOTPLUG_IO_REGION), aml_dword_acc,
> +                          aml_preserve);
>          aml_append(field, /* DIMM selector, write only */
>              aml_named_field(stringify(MEMORY_SLOT_SLECTOR), 32));
>          aml_append(field, /* _OST event code, write only */
> diff --git a/include/hw/acpi/aml-build.h b/include/hw/acpi/aml-build.h
> index 17d3beb..5aa5e7a 100644
> --- a/include/hw/acpi/aml-build.h
> +++ b/include/hw/acpi/aml-build.h
> @@ -35,7 +35,13 @@ typedef enum {
>      aml_dword_acc = 3,
>      aml_qword_acc = 4,
>      aml_buffer_acc = 5,
> -} AmlFieldFlags;
> +} AmlAccessType;
> +
> +typedef enum {
> +    aml_preserve = 0,
> +    aml_write_as_ones = 1,
> +    aml_write_as_zeros = 2,
> +} AmlUpdateRule;
>  
>  typedef enum {
>      aml_system_memory = 0x00,
> @@ -185,7 +191,7 @@ Aml *aml_if(Aml *predicate);
>  Aml *aml_package(uint8_t num_elements);
>  Aml *aml_buffer(void);
>  Aml *aml_resource_template(void);
> -Aml *aml_field(const char *name, AmlFieldFlags flags);
> +Aml *aml_field(const char *name, AmlAccessType type, AmlUpdateRule rule);
>  Aml *aml_varpackage(uint32_t num_elements);
>  
>  #endif

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [Qemu-devel] [PATCH v5 0/7] QEMU memory hot unplug support
  2015-03-27  9:20 [Qemu-devel] [PATCH v5 0/7] QEMU memory hot unplug support Zhu Guihua
                   ` (7 preceding siblings ...)
  2015-03-30  2:53 ` [Qemu-devel] [PATCH v5 0/7] QEMU memory hot unplug support Zhu Guihua
@ 2015-04-01  8:42 ` Igor Mammedov
  2015-04-01  9:43   ` Zhu Guihua
  8 siblings, 1 reply; 18+ messages in thread
From: Igor Mammedov @ 2015-04-01  8:42 UTC (permalink / raw)
  To: Zhu Guihua; +Cc: guz.fnst, pbonzini, tangchen, qemu-devel, izumi.taku

On Fri, 27 Mar 2015 17:20:33 +0800
Zhu Guihua <zhugh.fnst@cn.fujitsu.com> wrote:

> Memory hot unplug are both asynchronous procedures.
> When the unplug operation happens, unplug request cb is called first.
> And when guest OS finished handling unplug, unplug cb will be called
> to do the real removal of device.
I think series is in good shape and ready for merging
after comments are fixed.

> 
> v5:
>  -reorganize the patchset
>  -add documentation to understand patch easily
>  -add MEMORY_SLOT_EJECT for initiating device eject
>  -add support to send qmp event to notify mgmt about memory unplug error
> 
> v4:
>  -reorganize the patchset
>  -drop the new API acpi_send_gpe_event()
>  -update ssdt-mem
> 
> v3:
>  -commit message changes
>  -reorganize the patchset, squash and separate some patches
>  -update specs about acpi_mem_hotplug
>  -first cleanup external state, then un-map and un-register memory device
> 
> 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
> 
> Tang Chen (3):
>   acpi, mem-hotplug: add acpi_memory_slot_status() to get MemStatus
>   acpi, mem-hotplug: add unplug request cb for memory device
>   acpi, mem-hotplug: add unplug cb for memory device
> 
> Zhu Guihua (4):
>   docs: update documentation for memory hot unplug
>   acpi: extend aml_field() to support UpdateRule
>   acpi: add hardware implementation for memory hot unplug
>   qmp-event: add event notification for memory hot unplug error
> 
>  docs/memory-hotplug.txt           | 24 ++++++++--
>  docs/qmp/qmp-events.txt           | 17 +++++++
>  docs/specs/acpi_mem_hotplug.txt   | 32 ++++++++++++--
>  hw/acpi/aml-build.c               |  4 +-
>  hw/acpi/ich9.c                    | 19 ++++++--
>  hw/acpi/memory_hotplug.c          | 93 ++++++++++++++++++++++++++++++++++++---
>  hw/acpi/piix4.c                   | 17 +++++--
>  hw/core/qdev.c                    |  2 +-
>  hw/i386/acpi-build.c              | 25 ++++++++---
>  hw/i386/acpi-dsdt-mem-hotplug.dsl | 13 +++++-
>  hw/i386/pc.c                      | 62 ++++++++++++++++++++++++--
>  include/hw/acpi/aml-build.h       | 10 ++++-
>  include/hw/acpi/memory_hotplug.h  | 12 +++++
>  include/hw/acpi/pc-hotplug.h      |  3 ++
>  include/hw/qdev-core.h            |  1 +
>  monitor.c                         |  1 +
>  qapi/event.json                   | 14 ++++++
>  trace-events                      |  2 +
>  18 files changed, 315 insertions(+), 36 deletions(-)
> 

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [Qemu-devel] [PATCH v5 0/7] QEMU memory hot unplug support
  2015-04-01  8:42 ` Igor Mammedov
@ 2015-04-01  9:43   ` Zhu Guihua
  0 siblings, 0 replies; 18+ messages in thread
From: Zhu Guihua @ 2015-04-01  9:43 UTC (permalink / raw)
  To: Igor Mammedov; +Cc: guz.fnst, pbonzini, tangchen, qemu-devel, izumi.taku


On 04/01/2015 04:42 PM, Igor Mammedov wrote:
> On Fri, 27 Mar 2015 17:20:33 +0800
> Zhu Guihua <zhugh.fnst@cn.fujitsu.com> wrote:
>
>> Memory hot unplug are both asynchronous procedures.
>> When the unplug operation happens, unplug request cb is called first.
>> And when guest OS finished handling unplug, unplug cb will be called
>> to do the real removal of device.
> I think series is in good shape and ready for merging
> after comments are fixed.

I will fix all comments ASAP. Thanks for your hard work.

Regards,
Zhu

>
>> v5:
>>   -reorganize the patchset
>>   -add documentation to understand patch easily
>>   -add MEMORY_SLOT_EJECT for initiating device eject
>>   -add support to send qmp event to notify mgmt about memory unplug error
>>
>> v4:
>>   -reorganize the patchset
>>   -drop the new API acpi_send_gpe_event()
>>   -update ssdt-mem
>>
>> v3:
>>   -commit message changes
>>   -reorganize the patchset, squash and separate some patches
>>   -update specs about acpi_mem_hotplug
>>   -first cleanup external state, then un-map and un-register memory device
>>
>> 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
>>
>> Tang Chen (3):
>>    acpi, mem-hotplug: add acpi_memory_slot_status() to get MemStatus
>>    acpi, mem-hotplug: add unplug request cb for memory device
>>    acpi, mem-hotplug: add unplug cb for memory device
>>
>> Zhu Guihua (4):
>>    docs: update documentation for memory hot unplug
>>    acpi: extend aml_field() to support UpdateRule
>>    acpi: add hardware implementation for memory hot unplug
>>    qmp-event: add event notification for memory hot unplug error
>>
>>   docs/memory-hotplug.txt           | 24 ++++++++--
>>   docs/qmp/qmp-events.txt           | 17 +++++++
>>   docs/specs/acpi_mem_hotplug.txt   | 32 ++++++++++++--
>>   hw/acpi/aml-build.c               |  4 +-
>>   hw/acpi/ich9.c                    | 19 ++++++--
>>   hw/acpi/memory_hotplug.c          | 93 ++++++++++++++++++++++++++++++++++++---
>>   hw/acpi/piix4.c                   | 17 +++++--
>>   hw/core/qdev.c                    |  2 +-
>>   hw/i386/acpi-build.c              | 25 ++++++++---
>>   hw/i386/acpi-dsdt-mem-hotplug.dsl | 13 +++++-
>>   hw/i386/pc.c                      | 62 ++++++++++++++++++++++++--
>>   include/hw/acpi/aml-build.h       | 10 ++++-
>>   include/hw/acpi/memory_hotplug.h  | 12 +++++
>>   include/hw/acpi/pc-hotplug.h      |  3 ++
>>   include/hw/qdev-core.h            |  1 +
>>   monitor.c                         |  1 +
>>   qapi/event.json                   | 14 ++++++
>>   trace-events                      |  2 +
>>   18 files changed, 315 insertions(+), 36 deletions(-)
>>
> .
>

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2015-04-01  9:45 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-03-27  9:20 [Qemu-devel] [PATCH v5 0/7] QEMU memory hot unplug support Zhu Guihua
2015-03-27  9:20 ` [Qemu-devel] [PATCH v5 1/7] docs: update documentation for memory hot unplug Zhu Guihua
2015-03-30 15:47   ` Igor Mammedov
2015-03-27  9:20 ` [Qemu-devel] [PATCH v5 2/7] acpi, mem-hotplug: add acpi_memory_slot_status() to get MemStatus Zhu Guihua
2015-03-27  9:20 ` [Qemu-devel] [PATCH v5 3/7] acpi, mem-hotplug: add unplug request cb for memory device Zhu Guihua
2015-04-01  8:23   ` Igor Mammedov
2015-03-27  9:20 ` [Qemu-devel] [PATCH v5 4/7] acpi, mem-hotplug: add unplug " Zhu Guihua
2015-03-27  9:20 ` [Qemu-devel] [PATCH v5 5/7] acpi: extend aml_field() to support UpdateRule Zhu Guihua
2015-04-01  8:39   ` Igor Mammedov
2015-03-27  9:20 ` [Qemu-devel] [PATCH v5 6/7] acpi: add hardware implementation for memory hot unplug Zhu Guihua
2015-03-30  9:09   ` Igor Mammedov
2015-03-27  9:20 ` [Qemu-devel] [PATCH v5 7/7] qmp-event: add event notification for memory hot unplug error Zhu Guihua
2015-03-27 14:53   ` Eric Blake
2015-03-30  3:07     ` Zhu Guihua
2015-03-30  2:53 ` [Qemu-devel] [PATCH v5 0/7] QEMU memory hot unplug support Zhu Guihua
2015-03-30  7:49   ` Paolo Bonzini
2015-04-01  8:42 ` Igor Mammedov
2015-04-01  9:43   ` Zhu Guihua

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.