* [Qemu-devel] [PATCH v7 1/8] docs: update documentation for memory hot unplug
2015-04-27 8:47 [Qemu-devel] [PATCH v7 0/8] QEMU memory hot unplug support Zhu Guihua
@ 2015-04-27 8:47 ` Zhu Guihua
2015-04-27 8:47 ` [Qemu-devel] [PATCH v7 2/8] acpi, mem-hotplug: add acpi_memory_slot_status() to get MemStatus Zhu Guihua
` (6 subsequent siblings)
7 siblings, 0 replies; 12+ messages in thread
From: Zhu Guihua @ 2015-04-27 8:47 UTC (permalink / raw)
To: qemu-devel, imammedo, mst, pbonzini, eblake
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.
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
Signed-off-by: Zhu Guihua <zhugh.fnst@cn.fujitsu.com>
---
docs/memory-hotplug.txt | 23 +++++++++++++++++++---
docs/specs/acpi_mem_hotplug.txt | 42 ++++++++++++++++++++++++++++++++++++++++-
2 files changed, 61 insertions(+), 4 deletions(-)
diff --git a/docs/memory-hotplug.txt b/docs/memory-hotplug.txt
index f70571d..56bdd0a 100644
--- a/docs/memory-hotplug.txt
+++ b/docs/memory-hotplug.txt
@@ -4,9 +4,7 @@ QEMU memory hotplug
This document 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.
+Guest support is required for memory hotplug to work.
Basic RAM hotplug
-----------------
@@ -74,3 +72,22 @@ 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
+
+
+RAM hot-unplug
+---------------
+
+In order to be able to hot unplug pc-dimm device, QEMU has to be told the ids
+of pc-dimm device and memory backend object. The ids were assigned when you hot
+plugged memory.
+
+Two monitor commands are used to hot unplug memory:
+
+ - "device_del": deletes a front-end pc-dimm device
+ - "object_del": deletes a memory backend object
+
+For example, assuming that the pc-dimm device with id "dimm1" exists, and its memory
+backend is "mem1", the following commands tries to remove it.
+
+ (qemu) device_del dimm1
+ (qemu) object_del mem1
diff --git a/docs/specs/acpi_mem_hotplug.txt b/docs/specs/acpi_mem_hotplug.txt
index 1290994..dcc4808 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,43 @@ 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:
+----------------------------------
+ +-------------+ +-----------------------+ +------------------+
+ | 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 _EJ0 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 |
+ +------------------+ | |
+ +-------------------+
--
1.9.3
^ permalink raw reply related [flat|nested] 12+ messages in thread* [Qemu-devel] [PATCH v7 2/8] acpi, mem-hotplug: add acpi_memory_slot_status() to get MemStatus
2015-04-27 8:47 [Qemu-devel] [PATCH v7 0/8] QEMU memory hot unplug support Zhu Guihua
2015-04-27 8:47 ` [Qemu-devel] [PATCH v7 1/8] docs: update documentation for memory hot unplug Zhu Guihua
@ 2015-04-27 8:47 ` Zhu Guihua
2015-04-27 8:47 ` [Qemu-devel] [PATCH v7 3/8] acpi, mem-hotplug: add unplug request cb for memory device Zhu Guihua
` (5 subsequent siblings)
7 siblings, 0 replies; 12+ messages in thread
From: Zhu Guihua @ 2015-04-27 8:47 UTC (permalink / raw)
To: qemu-devel, imammedo, mst, pbonzini, eblake
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.
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
Signed-off-by: Tang Chen <tangchen@cn.fujitsu.com>
Signed-off-by: Zhu Guihua <zhugh.fnst@cn.fujitsu.com>
---
hw/acpi/memory_hotplug.c | 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] 12+ messages in thread* [Qemu-devel] [PATCH v7 3/8] acpi, mem-hotplug: add unplug request cb for memory device
2015-04-27 8:47 [Qemu-devel] [PATCH v7 0/8] QEMU memory hot unplug support Zhu Guihua
2015-04-27 8:47 ` [Qemu-devel] [PATCH v7 1/8] docs: update documentation for memory hot unplug Zhu Guihua
2015-04-27 8:47 ` [Qemu-devel] [PATCH v7 2/8] acpi, mem-hotplug: add acpi_memory_slot_status() to get MemStatus Zhu Guihua
@ 2015-04-27 8:47 ` Zhu Guihua
2015-04-27 8:47 ` [Qemu-devel] [PATCH v7 4/8] acpi, mem-hotplug: add unplug " Zhu Guihua
` (4 subsequent siblings)
7 siblings, 0 replies; 12+ messages in thread
From: Zhu Guihua @ 2015-04-27 8:47 UTC (permalink / raw)
To: qemu-devel, imammedo, mst, pbonzini, eblake
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 device in slot has been requested to be ejected.
This field is set to true in acpi_memory_unplug_request_cb().
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
Signed-off-by: Tang Chen <tangchen@cn.fujitsu.com>
Signed-off-by: Zhu Guihua <zhugh.fnst@cn.fujitsu.com>
---
hw/acpi/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..9d7eee9 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: 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) \
--
1.9.3
^ permalink raw reply related [flat|nested] 12+ messages in thread* [Qemu-devel] [PATCH v7 4/8] acpi, mem-hotplug: add unplug cb for memory device
2015-04-27 8:47 [Qemu-devel] [PATCH v7 0/8] QEMU memory hot unplug support Zhu Guihua
` (2 preceding siblings ...)
2015-04-27 8:47 ` [Qemu-devel] [PATCH v7 3/8] acpi, mem-hotplug: add unplug request cb for memory device Zhu Guihua
@ 2015-04-27 8:47 ` Zhu Guihua
2015-04-27 8:47 ` [Qemu-devel] [PATCH v7 5/8] acpi: extend aml_field() to support UpdateRule Zhu Guihua
` (3 subsequent siblings)
7 siblings, 0 replies; 12+ messages in thread
From: Zhu Guihua @ 2015-04-27 8:47 UTC (permalink / raw)
To: qemu-devel, imammedo, mst, pbonzini, eblake
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.
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
Signed-off-by: Tang Chen <tangchen@cn.fujitsu.com>
Signed-off-by: Zhu Guihua <zhugh.fnst@cn.fujitsu.com>
---
hw/acpi/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 9d7eee9..986223b 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] 12+ messages in thread* [Qemu-devel] [PATCH v7 5/8] acpi: extend aml_field() to support UpdateRule
2015-04-27 8:47 [Qemu-devel] [PATCH v7 0/8] QEMU memory hot unplug support Zhu Guihua
` (3 preceding siblings ...)
2015-04-27 8:47 ` [Qemu-devel] [PATCH v7 4/8] acpi, mem-hotplug: add unplug " Zhu Guihua
@ 2015-04-27 8:47 ` Zhu Guihua
2015-04-27 8:47 ` [Qemu-devel] [PATCH v7 6/8] acpi: fix "Memory device control fields" register Zhu Guihua
` (2 subsequent siblings)
7 siblings, 0 replies; 12+ messages in thread
From: Zhu Guihua @ 2015-04-27 8:47 UTC (permalink / raw)
To: qemu-devel, imammedo, mst, pbonzini, eblake
Cc: guz.fnst, izumi.taku, Zhu Guihua, tangchen
The flags field is declared with default update rule 'Preserve',
this patch extends aml_field() to support UpdateRule so that we
can specify different values per field.
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
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 e761005..dc1b8e8 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_preserve);
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] 12+ messages in thread* [Qemu-devel] [PATCH v7 6/8] acpi: fix "Memory device control fields" register
2015-04-27 8:47 [Qemu-devel] [PATCH v7 0/8] QEMU memory hot unplug support Zhu Guihua
` (4 preceding siblings ...)
2015-04-27 8:47 ` [Qemu-devel] [PATCH v7 5/8] acpi: extend aml_field() to support UpdateRule Zhu Guihua
@ 2015-04-27 8:47 ` Zhu Guihua
2015-04-27 19:09 ` Michael S. Tsirkin
2015-04-27 8:47 ` [Qemu-devel] [PATCH v7 7/8] acpi: add hardware implementation for memory hot unplug Zhu Guihua
2015-04-27 8:47 ` [Qemu-devel] [PATCH v7 8/8] qmp-event: add event notification for memory hot unplug error Zhu Guihua
7 siblings, 1 reply; 12+ messages in thread
From: Zhu Guihua @ 2015-04-27 8:47 UTC (permalink / raw)
To: qemu-devel, imammedo, mst, pbonzini, eblake
Cc: guz.fnst, izumi.taku, Zhu Guihua, tangchen
0 bit in Memory device control fields must be cleared before writing to
register. But now this field isn't cleared when other fields are written.
To solve this bug, This patch fixes UpdateRule to WriteAsZeros in "Memory
device control fields" register.
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
Signed-off-by: Zhu Guihua <zhugh.fnst@cn.fujitsu.com>
---
docs/specs/acpi_mem_hotplug.txt | 5 ++++-
hw/i386/acpi-build.c | 2 +-
2 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/docs/specs/acpi_mem_hotplug.txt b/docs/specs/acpi_mem_hotplug.txt
index dcc4808..68daa14 100644
--- a/docs/specs/acpi_mem_hotplug.txt
+++ b/docs/specs/acpi_mem_hotplug.txt
@@ -31,7 +31,10 @@ 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
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index dc1b8e8..eb66fef 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -924,7 +924,7 @@ build_ssdt(GArray *table_data, GArray *linker,
aml_append(scope, field);
field = aml_field(stringify(MEMORY_HOTPLUG_IO_REGION), aml_byte_acc,
- aml_preserve);
+ 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));
--
1.9.3
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [Qemu-devel] [PATCH v7 6/8] acpi: fix "Memory device control fields" register
2015-04-27 8:47 ` [Qemu-devel] [PATCH v7 6/8] acpi: fix "Memory device control fields" register Zhu Guihua
@ 2015-04-27 19:09 ` Michael S. Tsirkin
2015-04-28 5:49 ` Zhu Guihua
0 siblings, 1 reply; 12+ messages in thread
From: Michael S. Tsirkin @ 2015-04-27 19:09 UTC (permalink / raw)
To: Zhu Guihua; +Cc: qemu-devel, tangchen, pbonzini, izumi.taku, guz.fnst, imammedo
On Mon, Apr 27, 2015 at 04:47:20PM +0800, Zhu Guihua wrote:
> 0 bit in Memory device control fields must be cleared before writing to
> register. But now this field isn't cleared when other fields are written.
>
> To solve this bug, This patch fixes UpdateRule to WriteAsZeros in "Memory
> device control fields" register.
>
> Reviewed-by: Igor Mammedov <imammedo@redhat.com>
> Signed-off-by: Zhu Guihua <zhugh.fnst@cn.fujitsu.com>
Applied, thanks!
Please check pci branch in my tree as I had to
resolve some trivial conflicts.
> ---
> docs/specs/acpi_mem_hotplug.txt | 5 ++++-
> hw/i386/acpi-build.c | 2 +-
> 2 files changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/docs/specs/acpi_mem_hotplug.txt b/docs/specs/acpi_mem_hotplug.txt
> index dcc4808..68daa14 100644
> --- a/docs/specs/acpi_mem_hotplug.txt
> +++ b/docs/specs/acpi_mem_hotplug.txt
> @@ -31,7 +31,10 @@ 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
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index dc1b8e8..eb66fef 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -924,7 +924,7 @@ build_ssdt(GArray *table_data, GArray *linker,
> aml_append(scope, field);
>
> field = aml_field(stringify(MEMORY_HOTPLUG_IO_REGION), aml_byte_acc,
> - aml_preserve);
> + 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));
> --
> 1.9.3
On Mon, Apr 27, 2015 at 04:47:21PM +0800, Zhu Guihua 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
>
> Reviewed-by: Igor Mammedov <imammedo@redhat.com>
> Signed-off-by: Zhu Guihua <zhugh.fnst@cn.fujitsu.com>
> ---
> docs/specs/acpi_mem_hotplug.txt | 11 +++++++++--
> hw/acpi/memory_hotplug.c | 21 ++++++++++++++++++++-
> 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 | 3 +++
> 8 files changed, 61 insertions(+), 5 deletions(-)
>
> diff --git a/docs/specs/acpi_mem_hotplug.txt b/docs/specs/acpi_mem_hotplug.txt
> index 68daa14..3df3620 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 eject request to OSPM was issued.
> + 3-7: reserved and should be ignored by OSPM
> [0x15-0x17] reserved
>
> write access:
> @@ -38,7 +40,12 @@ Memory hot-plug interface (IO port 0xa00-0xa17, 1-4 byte access):
> 1: if set to 1 clears device insert event, set by OSPM
> after it has emitted device check event for the
> selected memory device
> - 2-7: reserved, OSPM must clear them before writing to register
> + 2: if set to 1 clears device remove event, set by OSPM
> + after it has emitted device eject request 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..35bbfeb 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,29 @@ 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);
> + trace_mhp_acpi_pc_dimm_deleted(mem_st->selector);
> }
> 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 eb66fef..96c1db3 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..46f6ef0 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,8 @@ 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"
> +mhp_acpi_pc_dimm_deleted(uint32_t slot) "slot[0x%"PRIx32"] pc-dimm deleted"
>
> # hw/i386/pc.c
> mhp_pc_dimm_assigned_slot(int slot) "0x%d"
> --
> 1.9.3
On Mon, Apr 27, 2015 at 04:47:22PM +0800, Zhu Guihua wrote:
> When memory hot unplug fails, this patch adds support to send
> QMP event to notify mgmt about this failure.
>
> Reviewed-by: Igor Mammedov <imammedo@redhat.com>
> Signed-off-by: Zhu Guihua <zhugh.fnst@cn.fujitsu.com>
> ---
> docs/qmp/qmp-events.txt | 17 +++++++++++++++++
> hw/acpi/memory_hotplug.c | 10 +++++++++-
> qapi/event.json | 14 ++++++++++++++
> trace-events | 1 +
> 4 files changed, 41 insertions(+), 1 deletion(-)
>
> diff --git a/docs/qmp/qmp-events.txt b/docs/qmp/qmp-events.txt
> index d759d19..3be468f 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 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 unsupported 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 35bbfeb..34cef1e 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,14 @@ 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) {
> + trace_mhp_acpi_pc_dimm_delete_failed(mem_st->selector);
> + qapi_event_send_mem_unplug_error(dev->id,
> + error_get_pretty(local_err),
> + &error_abort);
> + break;
> + }
> trace_mhp_acpi_pc_dimm_deleted(mem_st->selector);
> }
> break;
> diff --git a/qapi/event.json b/qapi/event.json
> index c51dc49..378dda5 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 error occurs.
> +#
> +# @device: device name
> +#
> +# @msg: Informative message
> +#
> +# Since: 2.4
> +##
> +{ 'event': 'MEM_UNPLUG_ERROR',
> + 'data': { 'device': 'str', 'msg': 'str' } }
> diff --git a/trace-events b/trace-events
> index 46f6ef0..11387c3 100644
> --- a/trace-events
> +++ b/trace-events
> @@ -1575,6 +1575,7 @@ mhp_acpi_write_ost_status(uint32_t slot, uint32_t st) "slot[0x%"PRIx32"] OST STA
> 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"
> mhp_acpi_pc_dimm_deleted(uint32_t slot) "slot[0x%"PRIx32"] pc-dimm deleted"
> +mhp_acpi_pc_dimm_delete_failed(uint32_t slot) "slot[0x%"PRIx32"] pc-dimm delete failed"
>
> # hw/i386/pc.c
> mhp_pc_dimm_assigned_slot(int slot) "0x%d"
> --
> 1.9.3
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [Qemu-devel] [PATCH v7 6/8] acpi: fix "Memory device control fields" register
2015-04-27 19:09 ` Michael S. Tsirkin
@ 2015-04-28 5:49 ` Zhu Guihua
2015-04-28 6:24 ` Zhu Guihua
0 siblings, 1 reply; 12+ messages in thread
From: Zhu Guihua @ 2015-04-28 5:49 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: imammedo, qemu-devel, tangchen, izumi.taku, guz.fnst, pbonzini
On 04/28/2015 03:09 AM, Michael S. Tsirkin wrote:
> On Mon, Apr 27, 2015 at 04:47:20PM +0800, Zhu Guihua wrote:
>> 0 bit in Memory device control fields must be cleared before writing to
>> register. But now this field isn't cleared when other fields are written.
>>
>> To solve this bug, This patch fixes UpdateRule to WriteAsZeros in "Memory
>> device control fields" register.
>>
>> Reviewed-by: Igor Mammedov <imammedo@redhat.com>
>> Signed-off-by: Zhu Guihua <zhugh.fnst@cn.fujitsu.com>
> Applied, thanks!
> Please check pci branch in my tree as I had to
> resolve some trivial conflicts.
What about other un-applied patches in this series ?
Should I rebase them on your pci branch and resend then again ?
Thanks,
Zhu
>
>> ---
>> docs/specs/acpi_mem_hotplug.txt | 5 ++++-
>> hw/i386/acpi-build.c | 2 +-
>> 2 files changed, 5 insertions(+), 2 deletions(-)
>>
>> diff --git a/docs/specs/acpi_mem_hotplug.txt b/docs/specs/acpi_mem_hotplug.txt
>> index dcc4808..68daa14 100644
>> --- a/docs/specs/acpi_mem_hotplug.txt
>> +++ b/docs/specs/acpi_mem_hotplug.txt
>> @@ -31,7 +31,10 @@ 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
>> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
>> index dc1b8e8..eb66fef 100644
>> --- a/hw/i386/acpi-build.c
>> +++ b/hw/i386/acpi-build.c
>> @@ -924,7 +924,7 @@ build_ssdt(GArray *table_data, GArray *linker,
>> aml_append(scope, field);
>>
>> field = aml_field(stringify(MEMORY_HOTPLUG_IO_REGION), aml_byte_acc,
>> - aml_preserve);
>> + 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));
>> --
>> 1.9.3
> On Mon, Apr 27, 2015 at 04:47:21PM +0800, Zhu Guihua 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
>>
>> Reviewed-by: Igor Mammedov <imammedo@redhat.com>
>> Signed-off-by: Zhu Guihua <zhugh.fnst@cn.fujitsu.com>
>> ---
>> docs/specs/acpi_mem_hotplug.txt | 11 +++++++++--
>> hw/acpi/memory_hotplug.c | 21 ++++++++++++++++++++-
>> 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 | 3 +++
>> 8 files changed, 61 insertions(+), 5 deletions(-)
>>
>> diff --git a/docs/specs/acpi_mem_hotplug.txt b/docs/specs/acpi_mem_hotplug.txt
>> index 68daa14..3df3620 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 eject request to OSPM was issued.
>> + 3-7: reserved and should be ignored by OSPM
>> [0x15-0x17] reserved
>>
>> write access:
>> @@ -38,7 +40,12 @@ Memory hot-plug interface (IO port 0xa00-0xa17, 1-4 byte access):
>> 1: if set to 1 clears device insert event, set by OSPM
>> after it has emitted device check event for the
>> selected memory device
>> - 2-7: reserved, OSPM must clear them before writing to register
>> + 2: if set to 1 clears device remove event, set by OSPM
>> + after it has emitted device eject request 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..35bbfeb 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,29 @@ 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);
>> + trace_mhp_acpi_pc_dimm_deleted(mem_st->selector);
>> }
>> 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 eb66fef..96c1db3 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..46f6ef0 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,8 @@ 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"
>> +mhp_acpi_pc_dimm_deleted(uint32_t slot) "slot[0x%"PRIx32"] pc-dimm deleted"
>>
>> # hw/i386/pc.c
>> mhp_pc_dimm_assigned_slot(int slot) "0x%d"
>> --
>> 1.9.3
> On Mon, Apr 27, 2015 at 04:47:22PM +0800, Zhu Guihua wrote:
>> When memory hot unplug fails, this patch adds support to send
>> QMP event to notify mgmt about this failure.
>>
>> Reviewed-by: Igor Mammedov <imammedo@redhat.com>
>> Signed-off-by: Zhu Guihua <zhugh.fnst@cn.fujitsu.com>
>> ---
>> docs/qmp/qmp-events.txt | 17 +++++++++++++++++
>> hw/acpi/memory_hotplug.c | 10 +++++++++-
>> qapi/event.json | 14 ++++++++++++++
>> trace-events | 1 +
>> 4 files changed, 41 insertions(+), 1 deletion(-)
>>
>> diff --git a/docs/qmp/qmp-events.txt b/docs/qmp/qmp-events.txt
>> index d759d19..3be468f 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 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 unsupported 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 35bbfeb..34cef1e 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,14 @@ 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) {
>> + trace_mhp_acpi_pc_dimm_delete_failed(mem_st->selector);
>> + qapi_event_send_mem_unplug_error(dev->id,
>> + error_get_pretty(local_err),
>> + &error_abort);
>> + break;
>> + }
>> trace_mhp_acpi_pc_dimm_deleted(mem_st->selector);
>> }
>> break;
>> diff --git a/qapi/event.json b/qapi/event.json
>> index c51dc49..378dda5 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 error occurs.
>> +#
>> +# @device: device name
>> +#
>> +# @msg: Informative message
>> +#
>> +# Since: 2.4
>> +##
>> +{ 'event': 'MEM_UNPLUG_ERROR',
>> + 'data': { 'device': 'str', 'msg': 'str' } }
>> diff --git a/trace-events b/trace-events
>> index 46f6ef0..11387c3 100644
>> --- a/trace-events
>> +++ b/trace-events
>> @@ -1575,6 +1575,7 @@ mhp_acpi_write_ost_status(uint32_t slot, uint32_t st) "slot[0x%"PRIx32"] OST STA
>> 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"
>> mhp_acpi_pc_dimm_deleted(uint32_t slot) "slot[0x%"PRIx32"] pc-dimm deleted"
>> +mhp_acpi_pc_dimm_delete_failed(uint32_t slot) "slot[0x%"PRIx32"] pc-dimm delete failed"
>>
>> # hw/i386/pc.c
>> mhp_pc_dimm_assigned_slot(int slot) "0x%d"
>> --
>> 1.9.3
>
> .
>
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [Qemu-devel] [PATCH v7 6/8] acpi: fix "Memory device control fields" register
2015-04-28 5:49 ` Zhu Guihua
@ 2015-04-28 6:24 ` Zhu Guihua
0 siblings, 0 replies; 12+ messages in thread
From: Zhu Guihua @ 2015-04-28 6:24 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: qemu-devel, tangchen, pbonzini, izumi.taku, guz.fnst, imammedo
On 04/28/2015 01:49 PM, Zhu Guihua wrote:
>
> On 04/28/2015 03:09 AM, Michael S. Tsirkin wrote:
>> On Mon, Apr 27, 2015 at 04:47:20PM +0800, Zhu Guihua wrote:
>>> 0 bit in Memory device control fields must be cleared before writing to
>>> register. But now this field isn't cleared when other fields are
>>> written.
>>>
>>> To solve this bug, This patch fixes UpdateRule to WriteAsZeros in
>>> "Memory
>>> device control fields" register.
>>>
>>> Reviewed-by: Igor Mammedov <imammedo@redhat.com>
>>> Signed-off-by: Zhu Guihua <zhugh.fnst@cn.fujitsu.com>
>> Applied, thanks!
>> Please check pci branch in my tree as I had to
>> resolve some trivial conflicts.
>
> What about other un-applied patches in this series ?
> Should I rebase them on your pci branch and resend then again ?
Oh, I am sorry to look at this carelessly. All patches in this series were
applied in your tree, thanks.
And I have checked that, memory hot unplug works as expected.
Regards,
Zhu
>
>>
>>> ---
>>> docs/specs/acpi_mem_hotplug.txt | 5 ++++-
>>> hw/i386/acpi-build.c | 2 +-
>>> 2 files changed, 5 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/docs/specs/acpi_mem_hotplug.txt
>>> b/docs/specs/acpi_mem_hotplug.txt
>>> index dcc4808..68daa14 100644
>>> --- a/docs/specs/acpi_mem_hotplug.txt
>>> +++ b/docs/specs/acpi_mem_hotplug.txt
>>> @@ -31,7 +31,10 @@ 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
>>> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
>>> index dc1b8e8..eb66fef 100644
>>> --- a/hw/i386/acpi-build.c
>>> +++ b/hw/i386/acpi-build.c
>>> @@ -924,7 +924,7 @@ build_ssdt(GArray *table_data, GArray *linker,
>>> aml_append(scope, field);
>>> field = aml_field(stringify(MEMORY_HOTPLUG_IO_REGION),
>>> aml_byte_acc,
>>> - aml_preserve);
>>> + 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));
>>> --
>>> 1.9.3
>> On Mon, Apr 27, 2015 at 04:47:21PM +0800, Zhu Guihua 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
>>>
>>> Reviewed-by: Igor Mammedov <imammedo@redhat.com>
>>> Signed-off-by: Zhu Guihua <zhugh.fnst@cn.fujitsu.com>
>>> ---
>>> docs/specs/acpi_mem_hotplug.txt | 11 +++++++++--
>>> hw/acpi/memory_hotplug.c | 21 ++++++++++++++++++++-
>>> 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 | 3 +++
>>> 8 files changed, 61 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/docs/specs/acpi_mem_hotplug.txt
>>> b/docs/specs/acpi_mem_hotplug.txt
>>> index 68daa14..3df3620 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 eject request to OSPM was issued.
>>> + 3-7: reserved and should be ignored by OSPM
>>> [0x15-0x17] reserved
>>> write access:
>>> @@ -38,7 +40,12 @@ Memory hot-plug interface (IO port 0xa00-0xa17,
>>> 1-4 byte access):
>>> 1: if set to 1 clears device insert event, set by OSPM
>>> after it has emitted device check event for the
>>> selected memory device
>>> - 2-7: reserved, OSPM must clear them before writing to
>>> register
>>> + 2: if set to 1 clears device remove event, set by OSPM
>>> + after it has emitted device eject request 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..35bbfeb 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,29 @@ 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);
>>> + trace_mhp_acpi_pc_dimm_deleted(mem_st->selector);
>>> }
>>> 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 eb66fef..96c1db3 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..46f6ef0 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,8 @@ 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"
>>> +mhp_acpi_pc_dimm_deleted(uint32_t slot) "slot[0x%"PRIx32"] pc-dimm
>>> deleted"
>>> # hw/i386/pc.c
>>> mhp_pc_dimm_assigned_slot(int slot) "0x%d"
>>> --
>>> 1.9.3
>> On Mon, Apr 27, 2015 at 04:47:22PM +0800, Zhu Guihua wrote:
>>> When memory hot unplug fails, this patch adds support to send
>>> QMP event to notify mgmt about this failure.
>>>
>>> Reviewed-by: Igor Mammedov <imammedo@redhat.com>
>>> Signed-off-by: Zhu Guihua <zhugh.fnst@cn.fujitsu.com>
>>> ---
>>> docs/qmp/qmp-events.txt | 17 +++++++++++++++++
>>> hw/acpi/memory_hotplug.c | 10 +++++++++-
>>> qapi/event.json | 14 ++++++++++++++
>>> trace-events | 1 +
>>> 4 files changed, 41 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/docs/qmp/qmp-events.txt b/docs/qmp/qmp-events.txt
>>> index d759d19..3be468f 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 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 unsupported 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 35bbfeb..34cef1e 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,14 @@ 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) {
>>> + trace_mhp_acpi_pc_dimm_delete_failed(mem_st->selector);
>>> + qapi_event_send_mem_unplug_error(dev->id,
>>> + error_get_pretty(local_err),
>>> + &error_abort);
>>> + break;
>>> + }
>>> trace_mhp_acpi_pc_dimm_deleted(mem_st->selector);
>>> }
>>> break;
>>> diff --git a/qapi/event.json b/qapi/event.json
>>> index c51dc49..378dda5 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 error occurs.
>>> +#
>>> +# @device: device name
>>> +#
>>> +# @msg: Informative message
>>> +#
>>> +# Since: 2.4
>>> +##
>>> +{ 'event': 'MEM_UNPLUG_ERROR',
>>> + 'data': { 'device': 'str', 'msg': 'str' } }
>>> diff --git a/trace-events b/trace-events
>>> index 46f6ef0..11387c3 100644
>>> --- a/trace-events
>>> +++ b/trace-events
>>> @@ -1575,6 +1575,7 @@ mhp_acpi_write_ost_status(uint32_t slot,
>>> uint32_t st) "slot[0x%"PRIx32"] OST STA
>>> 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"
>>> mhp_acpi_pc_dimm_deleted(uint32_t slot) "slot[0x%"PRIx32"] pc-dimm
>>> deleted"
>>> +mhp_acpi_pc_dimm_delete_failed(uint32_t slot) "slot[0x%"PRIx32"]
>>> pc-dimm delete failed"
>>> # hw/i386/pc.c
>>> mhp_pc_dimm_assigned_slot(int slot) "0x%d"
>>> --
>>> 1.9.3
>>
>> .
>>
>
>
> .
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* [Qemu-devel] [PATCH v7 7/8] acpi: add hardware implementation for memory hot unplug
2015-04-27 8:47 [Qemu-devel] [PATCH v7 0/8] QEMU memory hot unplug support Zhu Guihua
` (5 preceding siblings ...)
2015-04-27 8:47 ` [Qemu-devel] [PATCH v7 6/8] acpi: fix "Memory device control fields" register Zhu Guihua
@ 2015-04-27 8:47 ` Zhu Guihua
2015-04-27 8:47 ` [Qemu-devel] [PATCH v7 8/8] qmp-event: add event notification for memory hot unplug error Zhu Guihua
7 siblings, 0 replies; 12+ messages in thread
From: Zhu Guihua @ 2015-04-27 8:47 UTC (permalink / raw)
To: qemu-devel, imammedo, mst, pbonzini, eblake
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
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
Signed-off-by: Zhu Guihua <zhugh.fnst@cn.fujitsu.com>
---
docs/specs/acpi_mem_hotplug.txt | 11 +++++++++--
hw/acpi/memory_hotplug.c | 21 ++++++++++++++++++++-
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 | 3 +++
8 files changed, 61 insertions(+), 5 deletions(-)
diff --git a/docs/specs/acpi_mem_hotplug.txt b/docs/specs/acpi_mem_hotplug.txt
index 68daa14..3df3620 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 eject request to OSPM was issued.
+ 3-7: reserved and should be ignored by OSPM
[0x15-0x17] reserved
write access:
@@ -38,7 +40,12 @@ Memory hot-plug interface (IO port 0xa00-0xa17, 1-4 byte access):
1: if set to 1 clears device insert event, set by OSPM
after it has emitted device check event for the
selected memory device
- 2-7: reserved, OSPM must clear them before writing to register
+ 2: if set to 1 clears device remove event, set by OSPM
+ after it has emitted device eject request 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..35bbfeb 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,29 @@ 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);
+ trace_mhp_acpi_pc_dimm_deleted(mem_st->selector);
}
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 eb66fef..96c1db3 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..46f6ef0 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,8 @@ 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"
+mhp_acpi_pc_dimm_deleted(uint32_t slot) "slot[0x%"PRIx32"] pc-dimm deleted"
# hw/i386/pc.c
mhp_pc_dimm_assigned_slot(int slot) "0x%d"
--
1.9.3
^ permalink raw reply related [flat|nested] 12+ messages in thread* [Qemu-devel] [PATCH v7 8/8] qmp-event: add event notification for memory hot unplug error
2015-04-27 8:47 [Qemu-devel] [PATCH v7 0/8] QEMU memory hot unplug support Zhu Guihua
` (6 preceding siblings ...)
2015-04-27 8:47 ` [Qemu-devel] [PATCH v7 7/8] acpi: add hardware implementation for memory hot unplug Zhu Guihua
@ 2015-04-27 8:47 ` Zhu Guihua
7 siblings, 0 replies; 12+ messages in thread
From: Zhu Guihua @ 2015-04-27 8:47 UTC (permalink / raw)
To: qemu-devel, imammedo, mst, pbonzini, eblake
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.
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
Signed-off-by: Zhu Guihua <zhugh.fnst@cn.fujitsu.com>
---
docs/qmp/qmp-events.txt | 17 +++++++++++++++++
hw/acpi/memory_hotplug.c | 10 +++++++++-
qapi/event.json | 14 ++++++++++++++
trace-events | 1 +
4 files changed, 41 insertions(+), 1 deletion(-)
diff --git a/docs/qmp/qmp-events.txt b/docs/qmp/qmp-events.txt
index d759d19..3be468f 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 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 unsupported 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 35bbfeb..34cef1e 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,14 @@ 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) {
+ trace_mhp_acpi_pc_dimm_delete_failed(mem_st->selector);
+ qapi_event_send_mem_unplug_error(dev->id,
+ error_get_pretty(local_err),
+ &error_abort);
+ break;
+ }
trace_mhp_acpi_pc_dimm_deleted(mem_st->selector);
}
break;
diff --git a/qapi/event.json b/qapi/event.json
index c51dc49..378dda5 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 error occurs.
+#
+# @device: device name
+#
+# @msg: Informative message
+#
+# Since: 2.4
+##
+{ 'event': 'MEM_UNPLUG_ERROR',
+ 'data': { 'device': 'str', 'msg': 'str' } }
diff --git a/trace-events b/trace-events
index 46f6ef0..11387c3 100644
--- a/trace-events
+++ b/trace-events
@@ -1575,6 +1575,7 @@ mhp_acpi_write_ost_status(uint32_t slot, uint32_t st) "slot[0x%"PRIx32"] OST STA
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"
mhp_acpi_pc_dimm_deleted(uint32_t slot) "slot[0x%"PRIx32"] pc-dimm deleted"
+mhp_acpi_pc_dimm_delete_failed(uint32_t slot) "slot[0x%"PRIx32"] pc-dimm delete failed"
# hw/i386/pc.c
mhp_pc_dimm_assigned_slot(int slot) "0x%d"
--
1.9.3
^ permalink raw reply related [flat|nested] 12+ messages in thread