[Qemu-devel] [PATCH 16/16] ipmi: Add a thread to better simulate a BMC

[minyard@acm.org]

From: Corey Minyard <cminyard@mvista.com>

Run the IPMI BMC in a separate thread.  This provides a little better
simulation, since a BMC will normally be asynchronous to the rest of
the system.

Signed-off-by: Corey Minyard <cminyard@mvista.com>
---
 hw/ipmi/ipmi.c        | 33 +++++++++++++++++++++++++++++++++
 hw/ipmi/ipmi.h        | 47 +++++++++++++++++++++++++++++++++++++++++++----
 hw/ipmi/ipmi_bt.c     |  6 ++++++
 hw/ipmi/ipmi_extern.c | 17 +++++++++++++++++
 hw/ipmi/ipmi_kcs.c    |  5 +++++
 hw/ipmi/isa_ipmi.c    |  3 +++
 6 files changed, 107 insertions(+), 4 deletions(-)

diff --git a/hw/ipmi/ipmi.c b/hw/ipmi/ipmi.c
index f3e5e9e..27cf888 100644
--- a/hw/ipmi/ipmi.c
+++ b/hw/ipmi/ipmi.c
@@ -27,6 +27,27 @@
 #include "sysemu/sysemu.h"
 #include "qmp-commands.h"
 
+/*
+ * Create a separate thread for the IPMI interface itself.  This is a
+ * better simulation and lets the IPMI interface do things asynchronously
+ * if necessary.
+ */
+static void *ipmi_thread(void *opaque)
+{
+    IPMIInterface *s = opaque;
+
+    qemu_mutex_lock(&s->lock);
+    for (;;) {
+        qemu_cond_wait(&s->waker, &s->lock);
+        while (s->do_wake) {
+            s->do_wake = 0;
+            (IPMI_INTERFACE_GET_CLASS(s))->handle_if_event(s);
+        }
+    }
+    qemu_mutex_unlock(&s->lock);
+    return NULL;
+}
+
 static int ipmi_do_hw_op(IPMIInterface *s, enum ipmi_op op, int checkonly)
 {
     switch (op) {
@@ -90,6 +111,18 @@ void ipmi_interface_init(IPMIInterface *s, Error **errp)
     if (!s->slave_addr) {
         s->slave_addr = 0x20;
     }
+
+    if (s->threaded_bmc) {
+        qemu_mutex_init(&s->lock);
+        qemu_cond_init(&s->waker);
+        qemu_thread_create(&s->thread, "ipmi-bmc", ipmi_thread, s, 0);
+    }
+
+    if (s->threaded_bmc) {
+        qemu_mutex_init(&s->lock);
+        qemu_cond_init(&s->waker);
+        qemu_thread_create(&s->thread, "ipmi-bmc", ipmi_thread, s, 0);
+    }
 }
 
 static void ipmi_interface_class_init(ObjectClass *class, void *data)
diff --git a/hw/ipmi/ipmi.h b/hw/ipmi/ipmi.h
index 56cb423..0634b7c 100644
--- a/hw/ipmi/ipmi.h
+++ b/hw/ipmi/ipmi.h
@@ -28,6 +28,7 @@
 #include "exec/memory.h"
 #include "qemu-common.h"
 #include "hw/qdev.h"
+#include "qemu/thread.h"
 
 #define MAX_IPMI_MSG_SIZE 300
 
@@ -87,6 +88,16 @@ typedef struct IPMIInterface {
 
     IPMIBmc *bmc;
 
+    bool threaded_bmc;
+
+    /* For threaded BMC */
+    QemuThread thread;
+    QemuCond waker;
+    QemuMutex lock;
+
+    /* For non-threaded BMC */
+    int lockcount;
+
     bool do_wake;
 
     qemu_irq irq;
@@ -133,6 +144,7 @@ typedef struct IPMIInterfaceClass {
     /*
      * Handle an event that occurred on the interface, generally the.
      * target writing to a register.
+     * Must be called with ipmi_lock held.
      */
     void (*handle_if_event)(struct IPMIInterface *s);
 
@@ -148,6 +160,7 @@ typedef struct IPMIInterfaceClass {
 
     /*
      * Handle a response from the bmc.
+     * Must be called with ipmi_lock held.
      */
     void (*handle_rsp)(struct IPMIInterface *s, uint8_t msg_id,
                        unsigned char *rsp, unsigned int rsp_len);
@@ -172,12 +185,37 @@ void ipmi_interface_reset(IPMIInterface *s);
 #define TYPE_IPMI_BMC_EXTERN    "ipmi-bmc-extern"
 #define TYPE_IPMI_BMC_SIMULATOR "ipmi-bmc-sim"
 
+static inline void ipmi_lock(IPMIInterface *s)
+{
+    if (s->threaded_bmc) {
+        qemu_mutex_lock(&s->lock);
+    } else {
+        s->lockcount++;
+    }
+}
+
+static inline void ipmi_unlock(IPMIInterface *s)
+{
+    if (s->threaded_bmc) {
+        qemu_mutex_unlock(&s->lock);
+    } else {
+        s->lockcount--;
+    }
+}
+
 static inline void ipmi_signal(IPMIInterface *s)
 {
-    s->do_wake = 1;
-    while (s->do_wake) {
-        s->do_wake = 0;
-        (IPMI_INTERFACE_GET_CLASS(s))->handle_if_event(s);
+    if (s->threaded_bmc) {
+        s->do_wake = 1;
+        qemu_cond_signal(&s->waker);
+    } else {
+        s->do_wake = 1;
+        s->lockcount++;
+        while (s->do_wake) {
+            s->do_wake = 0;
+            (IPMI_INTERFACE_GET_CLASS(s))->handle_if_event(s);
+        }
+        s->lockcount--;
     }
 }
 
@@ -198,6 +236,7 @@ typedef struct IPMIBmcClass {
 
     /*
      * Handle a command to the bmc.
+     * Must be called with ipmi_lock held.
      */
     void (*handle_command)(struct IPMIBmc *s,
                            uint8_t *cmd, unsigned int cmd_len,
diff --git a/hw/ipmi/ipmi_bt.c b/hw/ipmi/ipmi_bt.c
index 105b978..aecf6c9 100644
--- a/hw/ipmi/ipmi_bt.c
+++ b/hw/ipmi/ipmi_bt.c
@@ -107,6 +107,7 @@ static void ipmi_bt_handle_event(IPMIInterface *s)
 {
     IPMIBtInterface *bt = IPMI_INTERFACE_BT(s);
 
+    /* ipmi_lock is already claimed. */
     if (s->inlen < 4) {
         goto out;
     }
@@ -165,6 +166,7 @@ static void ipmi_bt_handle_rsp(IPMIInterface *s, uint8_t msg_id,
 {
     IPMIBtInterface *bt = IPMI_INTERFACE_BT(s);
 
+    /* ipmi_lock is already claimed. */
     if (bt->waiting_rsp == msg_id) {
         bt->waiting_rsp++;
         if (rsp_len > (sizeof(s->outmsg) - 2)) {
@@ -199,6 +201,7 @@ static uint64_t ipmi_bt_ioport_read(void *opaque, hwaddr addr, unsigned size)
     IPMIInterface *s = &bt->intf;
     uint32_t ret = 0xff;
 
+    ipmi_lock(s);
     switch (addr & 3) {
     case 0:
         ret = bt->control_reg;
@@ -219,6 +222,7 @@ static uint64_t ipmi_bt_ioport_read(void *opaque, hwaddr addr, unsigned size)
         ret = bt->mask_reg;
         break;
     }
+    ipmi_unlock(s);
     return ret;
 }
 
@@ -228,6 +232,7 @@ static void ipmi_bt_ioport_write(void *opaque, hwaddr addr, uint64_t val,
     IPMIBtInterface *bt = opaque;
     IPMIInterface *s = &bt->intf;
 
+    ipmi_lock(s);
     switch (addr & 3) {
     case 0:
         if (IPMI_BT_GET_CLR_WR(val)) {
@@ -284,6 +289,7 @@ static void ipmi_bt_ioport_write(void *opaque, hwaddr addr, uint64_t val,
         }
         break;
     }
+    ipmi_unlock(s);
 }
 
 static const MemoryRegionOps ipmi_bt_io_ops = {
diff --git a/hw/ipmi/ipmi_extern.c b/hw/ipmi/ipmi_extern.c
index aace0b3..981a97b 100644
--- a/hw/ipmi/ipmi_extern.c
+++ b/hw/ipmi/ipmi_extern.c
@@ -140,6 +140,7 @@ static void extern_timeout(void *opaque)
     IPMIExternBmc *es = opaque;
     IPMIInterface *s = es->parent.intf;
 
+    ipmi_lock(s);
     if (es->connected) {
         if (es->waiting_rsp && (es->outlen == 0)) {
             IPMIInterfaceClass *k = IPMI_INTERFACE_GET_CLASS(s);
@@ -153,6 +154,7 @@ static void extern_timeout(void *opaque)
             continue_send(es);
         }
     }
+    ipmi_unlock(s);
 }
 
 static void addchar(IPMIExternBmc *es, unsigned char ch)
@@ -182,6 +184,7 @@ static void ipmi_extern_handle_command(IPMIBmc *b,
     uint8_t err = 0, csum;
     unsigned int i;
 
+    ipmi_lock(s);
     if (es->outlen) {
         /* We already have a command queued.  Shouldn't ever happen. */
         fprintf(stderr, "IPMI KCS: Got command when not finished with the"
@@ -222,6 +225,7 @@ static void ipmi_extern_handle_command(IPMIBmc *b,
     continue_send(es);
 
  out:
+    ipmi_unlock(s);
     return;
 }
 
@@ -304,9 +308,11 @@ static int can_receive(void *opaque)
 static void receive(void *opaque, const uint8_t *buf, int size)
 {
     IPMIExternBmc *es = opaque;
+    IPMIInterface *s = es->parent.intf;
     int i;
     unsigned char hw_op;
 
+    ipmi_lock(s);
     for (i = 0; i < size; i++) {
         unsigned char ch = buf[i];
 
@@ -361,9 +367,15 @@ static void receive(void *opaque, const uint8_t *buf, int size)
             break;
         }
     }
+    ipmi_unlock(s);
     return;
 
  out_hw_op:
+    ipmi_unlock(s);
+    /*
+     * We don't want to handle hardware operations while holding the
+     * lock, that may call back into this code to report a reset.
+     */
     handle_hw_op(es, hw_op);
 }
 
@@ -374,6 +386,7 @@ static void chr_event(void *opaque, int event)
     IPMIInterfaceClass *k = IPMI_INTERFACE_GET_CLASS(s);
     unsigned char v;
 
+    ipmi_lock(s);
     switch (event) {
     case CHR_EVENT_OPENED:
         es->connected = 1;
@@ -415,14 +428,18 @@ static void chr_event(void *opaque, int event)
         }
         break;
     }
+    ipmi_unlock(s);
 }
 
 static void ipmi_extern_handle_reset(IPMIBmc *b)
 {
     IPMIExternBmc *es = IPMI_BMC_EXTERN(b);
+    IPMIInterface *s = es->parent.intf;
 
+    ipmi_lock(s);
     es->send_reset = 1;
     continue_send(es);
+    ipmi_unlock(s);
 }
 
 static int ipmi_extern_post_migrate(void *opaque, int version_id)
diff --git a/hw/ipmi/ipmi_kcs.c b/hw/ipmi/ipmi_kcs.c
index e3af39e..ef50b83 100644
--- a/hw/ipmi/ipmi_kcs.c
+++ b/hw/ipmi/ipmi_kcs.c
@@ -197,6 +197,7 @@ static void ipmi_kcs_handle_rsp(IPMIInterface *s, uint8_t msg_id,
 {
     IPMIKcsInterface *kcs = IPMI_INTERFACE_KCS(s);
 
+    /* ipmi_lock is already claimed. */
     if (kcs->waiting_rsp == msg_id) {
         kcs->waiting_rsp++;
         if (rsp_len > sizeof(s->outmsg)) {
@@ -220,6 +221,7 @@ static uint64_t ipmi_kcs_ioport_read(void *opaque, hwaddr addr, unsigned size)
     IPMIKcsInterface *kcs = opaque;
     uint32_t ret;
 
+    ipmi_lock(&kcs->intf);
     switch (addr & 1) {
     case 0:
         ret = kcs->data_out_reg;
@@ -241,6 +243,7 @@ static uint64_t ipmi_kcs_ioport_read(void *opaque, hwaddr addr, unsigned size)
         }
         break;
     }
+    ipmi_unlock(&kcs->intf);
     return ret;
 }
 
@@ -254,6 +257,7 @@ static void ipmi_kcs_ioport_write(void *opaque, hwaddr addr, uint64_t val,
         return;
     }
 
+    ipmi_lock(s);
     switch (addr & 1) {
     case 0:
         kcs->data_in_reg = val;
@@ -265,6 +269,7 @@ static void ipmi_kcs_ioport_write(void *opaque, hwaddr addr, uint64_t val,
     }
     IPMI_KCS_SET_IBF(kcs->status_reg, 1);
     ipmi_signal(s);
+    ipmi_unlock(s);
 }
 
 const MemoryRegionOps ipmi_kcs_io_ops = {
diff --git a/hw/ipmi/isa_ipmi.c b/hw/ipmi/isa_ipmi.c
index 1652166..aa4bb3c 100644
--- a/hw/ipmi/isa_ipmi.c
+++ b/hw/ipmi/isa_ipmi.c
@@ -46,6 +46,7 @@ typedef struct ISAIPMIDevice {
     int32 isairq;
     uint8_t slave_addr;
     uint8_t version;
+    bool threaded_bmc;
     CharDriverState *chr;
     IPMIInterface *intf;
 } ISAIPMIDevice;
@@ -258,6 +259,7 @@ static void ipmi_isa_realizefn(DeviceState *dev, Error **errp)
     intf->slave_addr = ipmi->slave_addr;
     ipmi->intftype = intfk->smbios_type;
     ipmi->version = 0x20; /* Version 2.0 */
+    intf->threaded_bmc = ipmi->threaded_bmc;
     ipmi_interface_init(intf, errp);
     if (*errp) {
         return;
@@ -306,6 +308,7 @@ static Property ipmi_isa_properties[] = {
     DEFINE_PROP_INT32("irq",   ISAIPMIDevice, isairq,  5),
     DEFINE_PROP_UINT8("slave_addr", ISAIPMIDevice, slave_addr,  0),
     DEFINE_PROP_CHR("chardev",  ISAIPMIDevice, chr),
+    DEFINE_PROP_BOOL("threadbmc",  ISAIPMIDevice, threaded_bmc, false),
     DEFINE_PROP_END_OF_LIST(),
 };
 
-- 
1.8.3.1