[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 <address@hidden>
---
 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