[Qemu-devel] [RFC PATCH v1 0/4] Handle memory hotplug errors from guest OS.

[Qemu-devel] [RFC PATCH v1 0/4] Handle memory hotplug errors from guest OS.

[Tang Chen]

When doing memory hotplug, QEmu is not aware of guest OS error when hotplugging
memory devices. Even if guest OS failed to hot-add memory, the pc-dimm
device will be added to QEmu. Even if guest OS failed to hot-remove memory,
QEmu will remove the pc-dimm device.

An example is: for a Linux guest, the Linux kernel limited that the size of
hot-added memory should be mutiple of memory section (128MB by default).
If we add 130MB memory, the Linux kernel won't add it. We are not able
to handle the size check in QEmu commmand line because different OS may
have different limits.

And also, QEmu outputs nothing but guest OS failed to hot-add memory will
confuse users. We should at least report an error.

So, we should report the error to users, and cancel the memory hotplug
progress in QEmu.

QEmu thread sends a SCI to guest OS and return immediately. The vcpu thread
will emulate ACPI hardware operations. So this patch-set introduces a wait 
condition variable to synchronize these two threads.

Tang Chen (4):
  Use macro to define ACPI notification event.
  Add event handling for memory device insertion.
  Introduce wait condition to catch guest OS memory hotplug error.
  Handle memory hotplug error from guest OS in QEmu.

 hw/acpi/memory_hotplug.c | 146 +++++++++++++++++++++++++++++++++++++++++++++--
 include/hw/acpi/acpi.h   |  15 ++++-
 2 files changed, 153 insertions(+), 8 deletions(-)

-- 
1.8.4.2

[Qemu-devel] [RFC PATCH v1 1/4] Use macro to define ACPI notification event.

[Tang Chen]

According to ACPI spec, device object notification values define insertion
request (Device Check) as 1, and ejection request as 3.

Use macro to define them.

Signed-off-by: Tang Chen <tangchen@cn.fujitsu.com>
---
 hw/acpi/memory_hotplug.c | 7 +++++--
 include/hw/acpi/acpi.h   | 5 ++++-
 2 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/hw/acpi/memory_hotplug.c b/hw/acpi/memory_hotplug.c
index f29715a..1b21191 100644
--- a/hw/acpi/memory_hotplug.c
+++ b/hw/acpi/memory_hotplug.c
@@ -155,10 +155,13 @@ static void acpi_memory_hotplug_write(void *opaque, hwaddr addr, uint64_t data,
         break;
     case 0x4: /* _OST event  */
         mdev = &mem_st->devs[mem_st->selector];
-        if (data == 1) {
+        switch (data) {
+        case ACPI_NOTIFY_DEVICE_CHECK:
             /* TODO: handle device insert OST event */
-        } else if (data == 3) {
+            break;
+        case ACPI_NOTIFY_EJECT_REQUEST:
             /* TODO: handle device remove OST event */
+            break;
         }
         mdev->ost_event = data;
         trace_mhp_acpi_write_ost_ev(mem_st->selector, mdev->ost_event);
diff --git a/include/hw/acpi/acpi.h b/include/hw/acpi/acpi.h
index e105e45..e8499f6 100644
--- a/include/hw/acpi/acpi.h
+++ b/include/hw/acpi/acpi.h
@@ -92,9 +92,12 @@
 #define ACPI_BITMASK_ARB_DISABLE                0x0001
 
 /* OST_EVENT */
-#define ACPI_NOTIFY_EJECT_REQUEST               0x03
 #define ACPI_OSPM_EJECT                         0x103
 
+/* NOTIFY_EVENT */
+#define ACPI_NOTIFY_DEVICE_CHECK                0x1
+#define ACPI_NOTIFY_EJECT_REQUEST               0x3
+
 /* OST_STATUS */
 #define ACPI_SUCCESS                            0x0
 #define ACPI_FAILURE                            0x1
-- 
1.8.4.2

[Qemu-devel] [RFC PATCH v1 2/4] Add event handling for memory device insertion.

[Tang Chen]

Define device insertion OST event and status, and add a function to
handle memory insertion.

Signed-off-by: Tang Chen <tangchen@cn.fujitsu.com>
---
 hw/acpi/memory_hotplug.c | 18 ++++++++++++++++++
 include/hw/acpi/acpi.h   | 10 +++++++++-
 2 files changed, 27 insertions(+), 1 deletion(-)

diff --git a/hw/acpi/memory_hotplug.c b/hw/acpi/memory_hotplug.c
index 1b21191..fddb0fd 100644
--- a/hw/acpi/memory_hotplug.c
+++ b/hw/acpi/memory_hotplug.c
@@ -85,6 +85,20 @@ static uint64_t acpi_memory_hotplug_read(void *opaque, hwaddr addr,
     return val;
 }
 
+static void acpi_handle_insert(MemStatus *mdev)
+{
+    switch (mdev->ost_status) {
+    case ACPI_SUCCESS:
+    case ACPI_FAILURE:
+    case ACPI_UNRECOGNIZED_NOTIFY:
+    case ACPI_INSERT_DRIVER_LOAD_FAILURE:
+    case ACPI_INSERT_NOT_SUPPORTED:
+    case ACPI_INSERT_IN_PROGRESS:
+    default:
+        break;
+    }
+}
+
 static void acpi_handle_eject(MemStatus *mdev)
 {
     switch (mdev->ost_status) {
@@ -121,6 +135,10 @@ static void acpi_handle_eject(MemStatus *mdev)
 static void acpi_handle_ost_event(MemStatus *mdev)
 {
     switch (mdev->ost_event) {
+    case ACPI_NOTIFY_DEVICE_CHECK:
+    case ACPI_OSPM_INSERT:
+        acpi_handle_insert(mdev);
+        break;
     case ACPI_NOTIFY_EJECT_REQUEST:    /* Ejection triggered by hardware. */
     case ACPI_OSPM_EJECT:              /* Ejection triggered by guest OS. */
         acpi_handle_eject(mdev);
diff --git a/include/hw/acpi/acpi.h b/include/hw/acpi/acpi.h
index e8499f6..ad706d8 100644
--- a/include/hw/acpi/acpi.h
+++ b/include/hw/acpi/acpi.h
@@ -93,21 +93,29 @@
 
 /* OST_EVENT */
 #define ACPI_OSPM_EJECT                         0x103
+#define ACPI_OSPM_INSERT                        0x200
 
 /* NOTIFY_EVENT */
 #define ACPI_NOTIFY_DEVICE_CHECK                0x1
 #define ACPI_NOTIFY_EJECT_REQUEST               0x3
 
-/* OST_STATUS */
+/* general processing OST_STATUS */
 #define ACPI_SUCCESS                            0x0
 #define ACPI_FAILURE                            0x1
 #define ACPI_UNRECOGNIZED_NOTIFY                0x2
+
+/* ejection processing OST_STATUS */
 #define ACPI_EJECT_NOT_SUPPORTED                0x80
 #define ACPI_EJECT_DEVICE_IN_USE                0x81
 #define ACPI_EJECT_DEVICE_BUSY                  0x82
 #define ACPI_EJECT_DEPENDENCY_BUSY              0x83
 #define ACPI_EJECT_IN_PROGRESS                  0x84
 
+/* insertion processing OST_STATUS */
+#define ACPI_INSERT_IN_PROGRESS                 0x80
+#define ACPI_INSERT_DRIVER_LOAD_FAILURE         0x81
+#define ACPI_INSERT_NOT_SUPPORTED               0x82
+
 /* structs */
 typedef struct ACPIPMTimer ACPIPMTimer;
 typedef struct ACPIPM1EVT ACPIPM1EVT;
-- 
1.8.4.2

[Qemu-devel] [RFC PATCH v1 3/4] Introduce wait condition to catch guest OS memory hotplug error.

[Tang Chen]

When acpi_memory_plug_cb() sends a SCI to guest OS, vcpu thread will handle it.
And QEmu thread will return, who is not able to catch the error if guest OS
failed to handle the SCI.

Of course, if guest OS failed to handle SCI, it will give error messages. But
QEmu will not output anything.

Furthermore, if QEmu cannot catch these errors, applications based on QEmu,
like Libvirt, will also not able to catch them. This could be trouble to end users.

This patch introduces a condition variable named qemu_cond_memhp. QEmu will wait
on qemu_cond_memhp after sending SCI, and vcpu thread will signal QEmu when OST
status is written into ACPI register. This is used by the following patch.

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

diff --git a/hw/acpi/memory_hotplug.c b/hw/acpi/memory_hotplug.c
index fddb0fd..38d9654 100644
--- a/hw/acpi/memory_hotplug.c
+++ b/hw/acpi/memory_hotplug.c
@@ -4,6 +4,12 @@
 #include "hw/boards.h"
 #include "trace.h"
 #include "qapi-event.h"
+#include "qemu/thread.h"
+#include "qemu/main-loop.h"
+
+/* _OST called by guest OS */
+static QemuCond qemu_memhp_cond;
+static QemuMutex qemu_memhp_mutex;
 
 static ACPIOSTInfo *acpi_memory_device_status(int slot, MemStatus *mdev)
 {
@@ -93,6 +99,8 @@ static void acpi_handle_insert(MemStatus *mdev)
     case ACPI_UNRECOGNIZED_NOTIFY:
     case ACPI_INSERT_DRIVER_LOAD_FAILURE:
     case ACPI_INSERT_NOT_SUPPORTED:
+        /* Signal QEmu to continue.*/
+        qemu_cond_signal(&qemu_memhp_cond);
     case ACPI_INSERT_IN_PROGRESS:
     default:
         break;
@@ -106,6 +114,9 @@ static void acpi_handle_eject(MemStatus *mdev)
         object_unparent(OBJECT(mdev->dimm));
         mdev->is_removing = false;
         mdev->dimm = NULL;
+
+        /* Signal QEmu to continue.*/
+        qemu_cond_signal(&qemu_memhp_cond);
         break;
     case ACPI_EJECT_IN_PROGRESS:
         /* For ejection triggered by hardware (device_del command),
@@ -126,6 +137,9 @@ static void acpi_handle_eject(MemStatus *mdev)
     case ACPI_EJECT_DEPENDENCY_BUSY:
         mdev->is_removing = false;
         mdev->is_enabled = true;
+
+        /* Signal QEmu to catch errors. */
+        qemu_cond_signal(&qemu_memhp_cond);
         break;
     default:
         break;
@@ -181,12 +195,16 @@ static void acpi_memory_hotplug_write(void *opaque, hwaddr addr, uint64_t data,
             /* TODO: handle device remove OST event */
             break;
         }
+        qemu_mutex_lock(&qemu_memhp_mutex);
         mdev->ost_event = data;
+        qemu_mutex_unlock(&qemu_memhp_mutex);
         trace_mhp_acpi_write_ost_ev(mem_st->selector, mdev->ost_event);
         break;
     case 0x8: /* _OST status */
         mdev = &mem_st->devs[mem_st->selector];
+        qemu_mutex_lock(&qemu_memhp_mutex);
         mdev->ost_status = data;
+        qemu_mutex_unlock(&qemu_memhp_mutex);
         trace_mhp_acpi_write_ost_status(mem_st->selector, mdev->ost_status);
 
         info = acpi_memory_device_status(mem_st->selector, mdev);
@@ -234,6 +252,9 @@ void acpi_memory_hotplug_init(MemoryRegion *as, Object *owner,
     memory_region_init_io(&state->io, owner, &acpi_memory_hotplug_ops, state,
                           "acpi-mem-hotplug", ACPI_MEMORY_HOTPLUG_IO_LEN);
     memory_region_add_subregion(as, ACPI_MEMORY_HOTPLUG_BASE, &state->io);
+
+    qemu_cond_init(&qemu_memhp_cond);
+    qemu_mutex_init(&qemu_memhp_mutex);
 }
 
 void acpi_memory_plug_cb(ACPIREGS *ar, qemu_irq irq, MemHotplugState *mem_st,
@@ -265,6 +286,23 @@ void acpi_memory_plug_cb(ACPIREGS *ar, qemu_irq irq, MemHotplugState *mem_st,
     /* do ACPI magic */
     ar->gpe.sts[0] |= ACPI_MEMORY_HOTPLUG_STATUS;
     acpi_update_sci(ar, irq);
+
+    /* When SCI is sent, wait for guest OS handling and catch errors.
+     *
+     * QEmu locked all the iothreads before handling monitor command in
+     * os_host_main_loop_wait(). Unlock them so that vcpu threads are able
+     * to handle ACPI register writing requests from guest OS. And relock
+     * them after we are signaled.
+     */
+    qemu_mutex_unlock_iothread();
+    qemu_mutex_lock(&qemu_memhp_mutex);
+    /* qemu_cond_wait() will release qemu_memhp_mutex and wait,
+     * and will relock it when signaled.
+     */
+    qemu_cond_wait(&qemu_memhp_cond, &qemu_memhp_mutex);
+    qemu_mutex_unlock(&qemu_memhp_mutex);
+    qemu_mutex_lock_iothread();
+
     return;
 }
 
-- 
1.8.4.2

[Qemu-devel] [RFC PATCH v1 4/4] Handle memory hotplug error from guest OS in QEmu.

[Tang Chen]

Before this patch, QEmu is not aware of guest OS error when hotplugging
memory devices. Even if guest OS failed to hot-add memory, the pc-dimm
device will be added to QEmu. Even if guest OS failed to hot-remove memory,
QEmu will remove the pc-dimm device.

For example, for a Linux guest, the Linux kernel limited that the size of
hot-added memory should be mutiple of memory section (128MB by default).
If we add 130MB memory, the Linux kernel won't add it. We are not able
to handle the size check in QEmu commmand line because different OS may
have different limits.

And also, QEmu outputs nothing but guest OS failed to hot-add memory will
confuse users. We should at least report an error.

So, we should report the error to users, and cancel the memory hotplug
progress in QEmu.

Since after previous patch, QEmu will wait on qemu_cond_memhp after sending
SCI, and will be signaled by vcpu thread after OST status is written to
ACPI register, this patch checks OST status, and report an error to users,
and cancel hotplug progress if necessary.

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

diff --git a/hw/acpi/memory_hotplug.c b/hw/acpi/memory_hotplug.c
index 38d9654..59917c3 100644
--- a/hw/acpi/memory_hotplug.c
+++ b/hw/acpi/memory_hotplug.c
@@ -257,25 +257,95 @@ void acpi_memory_hotplug_init(MemoryRegion *as, Object *owner,
     qemu_mutex_init(&qemu_memhp_mutex);
 }
 
+static void acpi_handle_memory_insert_error(MemStatus *mdev, char *dev_path,
+                                            Error **errp)
+{
+    switch (mdev->ost_status) {
+    case ACPI_FAILURE:
+        error_setg(errp, "Failed to insert device [%s]", dev_path);
+        break;
+    case ACPI_UNRECOGNIZED_NOTIFY:
+        error_setg(errp, "Unrecongnized notification");
+        break;
+    case ACPI_INSERT_DRIVER_LOAD_FAILURE:
+        error_setg(errp, "Failed to load driver for device [%s]", dev_path);
+        break;
+    case ACPI_INSERT_NOT_SUPPORTED:
+        error_setg(errp, "Insertion is not supported for device [%s]",
+                   dev_path);
+        break;
+    default:
+        error_setg(errp, "Unknown error on insertion of device [%s]", dev_path);
+        break;
+    }
+}
+
+static void acpi_handle_memory_eject_error(MemStatus *mdev, char *dev_path,
+                                           Error **errp)
+{
+    switch (mdev->ost_status) {
+    case ACPI_FAILURE:
+        error_setg(errp, "Failed to eject device [%s]", dev_path);
+        break;
+    case ACPI_UNRECOGNIZED_NOTIFY:
+        error_setg(errp, "Unrecongnized notification");
+        break;
+    case ACPI_EJECT_DEVICE_IN_USE:
+    case ACPI_EJECT_DEVICE_BUSY:
+        error_setg(errp, "Device [%s] is busy", dev_path);
+        break;
+    case ACPI_EJECT_DEPENDENCY_BUSY:
+        error_setg(errp, "Ejection dependency is busy or not supported"
+                         " for device [%s]", dev_path);
+        break;
+    case ACPI_EJECT_NOT_SUPPORTED:
+        error_setg(errp, "Ejection is not supported for device [%s]",
+                   dev_path);
+        break;
+    default:
+        error_setg(errp, "Unknown error on ejection of device [%s]", dev_path);
+        break;
+    }
+}
+
+static void acpi_handle_memory_error(MemStatus *mdev, char *dev_path,
+                                     Error **errp)
+{
+    if (mdev->ost_status == ACPI_SUCCESS)
+        return;
+
+    switch (mdev->ost_event) {
+    case ACPI_NOTIFY_DEVICE_CHECK:
+    case ACPI_OSPM_INSERT:
+        acpi_handle_memory_insert_error(mdev, dev_path, errp);
+        break;
+    case ACPI_NOTIFY_EJECT_REQUEST:
+    case ACPI_OSPM_EJECT:
+        acpi_handle_memory_eject_error(mdev, dev_path, errp);
+        break;
+    default:
+        break;
+    }
+}
+
 void acpi_memory_plug_cb(ACPIREGS *ar, qemu_irq irq, MemHotplugState *mem_st,
                          DeviceState *dev, Error **errp)
 {
     MemStatus *mdev;
     Error *local_err = NULL;
     int slot = object_property_get_int(OBJECT(dev), "slot", &local_err);
+    char *dev_path = object_get_canonical_path(OBJECT(dev));
 
     if (local_err) {
         error_propagate(errp, local_err);
-        return;
+        goto ret;
     }
 
     if (slot >= mem_st->dev_count) {
-        char *dev_path = object_get_canonical_path(OBJECT(dev));
         error_setg(errp, "acpi_memory_plug_cb: "
                    "device [%s] returned invalid memory slot[%d]",
                     dev_path, slot);
-        g_free(dev_path);
-        return;
+        goto ret;
     }
 
     mdev = &mem_st->devs[slot];
@@ -300,9 +370,14 @@ void acpi_memory_plug_cb(ACPIREGS *ar, qemu_irq irq, MemHotplugState *mem_st,
      * and will relock it when signaled.
      */
     qemu_cond_wait(&qemu_memhp_cond, &qemu_memhp_mutex);
+
+    acpi_handle_memory_error(mdev, dev_path, errp);
+
     qemu_mutex_unlock(&qemu_memhp_mutex);
     qemu_mutex_lock_iothread();
 
+ret:
+    g_free(dev_path);
     return;
 }
 
-- 
1.8.4.2

Re: [Qemu-devel] [RFC PATCH v1 0/4] Handle memory hotplug errors from guest OS.

[tangchen]

Forgot to mention, this patch-set is based on the following patch-set:

[RESEND PATCH v3 0/8] QEmu memory hot unplug support.
https://www.mail-archive.com/qemu-devel@nongnu.org/msg253018.html

Thanks.

On 08/27/2014 04:09 PM, Tang Chen wrote:
> When doing memory hotplug, QEmu is not aware of guest OS error when hotplugging
> memory devices. Even if guest OS failed to hot-add memory, the pc-dimm
> device will be added to QEmu. Even if guest OS failed to hot-remove memory,
> QEmu will remove the pc-dimm device.
>
> An example is: for a Linux guest, the Linux kernel limited that the size of
> hot-added memory should be mutiple of memory section (128MB by default).
> If we add 130MB memory, the Linux kernel won't add it. We are not able
> to handle the size check in QEmu commmand line because different OS may
> have different limits.
>
> And also, QEmu outputs nothing but guest OS failed to hot-add memory will
> confuse users. We should at least report an error.
>
> So, we should report the error to users, and cancel the memory hotplug
> progress in QEmu.
>
> QEmu thread sends a SCI to guest OS and return immediately. The vcpu thread
> will emulate ACPI hardware operations. So this patch-set introduces a wait
> condition variable to synchronize these two threads.
>
> Tang Chen (4):
>    Use macro to define ACPI notification event.
>    Add event handling for memory device insertion.
>    Introduce wait condition to catch guest OS memory hotplug error.
>    Handle memory hotplug error from guest OS in QEmu.
>
>   hw/acpi/memory_hotplug.c | 146 +++++++++++++++++++++++++++++++++++++++++++++--
>   include/hw/acpi/acpi.h   |  15 ++++-
>   2 files changed, 153 insertions(+), 8 deletions(-)
>

Re: [Qemu-devel] [RFC PATCH v1 0/4] Handle memory hotplug errors from guest OS.

[tangchen]

Hi,

Would anyone help to review this patch-set ?
I'm not quit sure if this is a suitable way solve this problem.

Thanks.


On 08/27/2014 04:14 PM, tangchen wrote:
>
> Forgot to mention, this patch-set is based on the following patch-set:
>
> [RESEND PATCH v3 0/8] QEmu memory hot unplug support.
> https://www.mail-archive.com/qemu-devel@nongnu.org/msg253018.html
>
> Thanks.
>
> On 08/27/2014 04:09 PM, Tang Chen wrote:
>> When doing memory hotplug, QEmu is not aware of guest OS error when 
>> hotplugging
>> memory devices. Even if guest OS failed to hot-add memory, the pc-dimm
>> device will be added to QEmu. Even if guest OS failed to hot-remove 
>> memory,
>> QEmu will remove the pc-dimm device.
>>
>> An example is: for a Linux guest, the Linux kernel limited that the 
>> size of
>> hot-added memory should be mutiple of memory section (128MB by default).
>> If we add 130MB memory, the Linux kernel won't add it. We are not able
>> to handle the size check in QEmu commmand line because different OS may
>> have different limits.
>>
>> And also, QEmu outputs nothing but guest OS failed to hot-add memory 
>> will
>> confuse users. We should at least report an error.
>>
>> So, we should report the error to users, and cancel the memory hotplug
>> progress in QEmu.
>>
>> QEmu thread sends a SCI to guest OS and return immediately. The vcpu 
>> thread
>> will emulate ACPI hardware operations. So this patch-set introduces a 
>> wait
>> condition variable to synchronize these two threads.
>>
>> Tang Chen (4):
>>    Use macro to define ACPI notification event.
>>    Add event handling for memory device insertion.
>>    Introduce wait condition to catch guest OS memory hotplug error.
>>    Handle memory hotplug error from guest OS in QEmu.
>>
>>   hw/acpi/memory_hotplug.c | 146 
>> +++++++++++++++++++++++++++++++++++++++++++++--
>>   include/hw/acpi/acpi.h   |  15 ++++-
>>   2 files changed, 153 insertions(+), 8 deletions(-)
>>
>
> .
>