maru-tablet: fix unexpected shutdown issue
authorJinhyung Jo <jinhyung.jo@samsung.com>
Thu, 3 Mar 2016 09:07:02 +0000 (18:07 +0900)
committerSeokYeon Hwang <syeon.hwang@samsung.com>
Tue, 8 Mar 2016 06:06:55 +0000 (15:06 +0900)
If a user uses this device to hard, the emulator dies unexpectedly.
Its reason is to use the virtio queue.
The tablet device accesses the queue that is not prepared
and then releases it.
Fix the issue and refactoring the code.
 - improve the use of the virtio queue
 - remove unused structure & variables
 - pthread mutex to qemu mutex
 - remove unnecessary function declaration
 - clean up logs.

Change-Id: I8adfd2e89c88185547333a45462eceaa1f061c5b
Signed-off-by: Jinhyung Jo <jinhyung.jo@samsung.com>
tizen/src/hw/virtio/maru_virtio_tablet.c
tizen/src/hw/virtio/maru_virtio_tablet.h

index 4217bae..e1a8b07 100644 (file)
@@ -4,6 +4,8 @@
  * Copyright (c) 2015 Samsung Electronics Co., Ltd All Rights Reserved
  *
  * Contact:
+ *  Jinhyung Jo <jinhyung.jo@samsung.com>
+ *  SeokYeon Hwang <syeon.hwang@samsung.com>
  *  Sungmin Ha <sungmin82.ha@samsung.com>
  *  Sangho Park <sangho.p@samsung.com>
  *
@@ -26,7 +28,6 @@
  *
  */
 
-#include <pthread.h>
 #include "emul_state.h"
 #include "maru_virtio_tablet.h"
 #include "hw/maru_device_ids.h"
 MULTI_DEBUG_CHANNEL(qemu, tablet);
 
 #define DEVICE_NAME "virtio-tablet"
-#define MAX_BUF_COUNT 256
-static int vqidx;
+#define MAX_BUF_COUNT 25
+/* virtio queue size must be a power of 2,
+ * but the value of (event buf *  buf count) is 500.
+ * so add padding to correct the value.
+*/
+#define MAX_VQ_SIZE 512
+
 /*
  * Tablet event queue
  */
@@ -48,7 +54,7 @@ typedef struct TabletEventEntry {
 } TabletEventEntry;
 
 /* the maximum number of Tablet event that can be put into a queue */
-#define MAX_TABLET_EVENT_CNT 256
+#define MAX_TABLET_EVENT_CNT  (MAX_BUF_COUNT)
 
 static TabletEventEntry _events_buf[MAX_TABLET_EVENT_CNT];
 static QTAILQ_HEAD(, TabletEventEntry) events_queue =
@@ -57,42 +63,21 @@ static QTAILQ_HEAD(, TabletEventEntry) events_queue =
 static unsigned int event_ringbuf_cnt; /* _events_buf */
 static unsigned int event_queue_cnt; /* events_queue */
 
-/*
- * VirtQueueElement queue
- */
-typedef struct ElementEntry {
-    unsigned int el_index;
-    unsigned int sg_index;
-    VirtQueueElement elem;
-
-    QTAILQ_ENTRY(ElementEntry) node;
-} ElementEntry;
-
-static QTAILQ_HEAD(, ElementEntry) elem_queue =
-    QTAILQ_HEAD_INITIALIZER(elem_queue);
-
-static unsigned int elem_ringbuf_cnt; /* _elem_buf */
-static unsigned int elem_queue_cnt; /* elem_queue */
-
-VirtIOTablet *vtk;
-VirtQueueElement elem_vtk;
-
-/* lock for between communication thread and IO thread */
-static pthread_mutex_t event_mutex = PTHREAD_MUTEX_INITIALIZER;
+VirtIOTablet *g_vt;
 
 void maru_tablet_event(int event_type, int x, int y, int btn, int btn_status)
 {
     TabletEventEntry *entry = NULL;
 
-    if (!vtk) {
+    if (!g_vt) {
         INFO("Tablet device can not be used.\n");
         return;
     }
 
     if (unlikely(event_queue_cnt >= MAX_TABLET_EVENT_CNT)) {
-        INFO("full tablet event queue, lose event\n", event_queue_cnt);
+        INFO("full tablet event queue, lose event\n");
 
-        qemu_bh_schedule(vtk->bh);
+        qemu_bh_schedule(g_vt->bh);
         return;
     }
 
@@ -105,7 +90,7 @@ void maru_tablet_event(int event_type, int x, int y, int btn, int btn_status)
     entry->tablet.btn_status = btn_status;
     entry->tablet.event_type = event_type;
 
-    pthread_mutex_lock(&event_mutex);
+    qemu_mutex_lock(&g_vt->mutex);
 
     event_ringbuf_cnt++;
 
@@ -114,70 +99,80 @@ void maru_tablet_event(int event_type, int x, int y, int btn, int btn_status)
 
     QTAILQ_INSERT_TAIL(&events_queue, entry, node);
 
-    pthread_mutex_unlock(&event_mutex);
+    qemu_mutex_unlock(&g_vt->mutex);
 
-    /* call maru_virtio_tablet_notify */
-    qemu_bh_schedule(vtk->bh);
+    /* call maru_virtio_tablet_bh */
+    qemu_bh_schedule(g_vt->bh);
 
-    TRACE("tablet event (%d) : x=%d, y=%d, btn=%d, btn_status=%d, event_type=%d\n",
+    TRACE("tablet event(%u): x=%u, y=%u, btn=%u, status=%u, event_type=%u\n",
         entry->index, entry->tablet.x, entry->tablet.y, entry->tablet.btn,
         entry->tablet.btn_status, entry->tablet.event_type);
 }
 
 static void maru_virtio_tablet_handle(VirtIODevice *vdev, VirtQueue *vq)
 {
-    int virt_sg_index = 0;
-
-    TRACE("maru_virtio_tablet_handle\n");
+    VirtIOTablet *vt = VIRTIO_MARU_TABLET(vdev);
 
-    if (unlikely(virtio_queue_empty(vtk->vq))) {
+    if (unlikely(virtio_queue_empty(vt->vq))) {
         TRACE("virtqueue is empty\n");
         return;
     }
-    /* Get a queue buffer which is written by guest side. */
-    do {
-        virt_sg_index = virtqueue_pop(vq, &elem_vtk);
-    } while (virt_sg_index < MAX_BUF_COUNT);
+    /* The virtio queue buffer is prepared by the kernel driver */
+    qemu_mutex_lock(&vt->mutex);
+    vt->avail = true;
+    qemu_mutex_unlock(&vt->mutex);
+
+    TRACE("maru_virtio_tablet_handle\n");
 }
 
-void maru_virtio_tablet_notify(void)
+static void maru_tablet_bh(void *opaque)
 {
+    VirtIOTablet *vt = (VirtIOTablet *)opaque;
     TabletEventEntry *event_entry = NULL;
+    VirtQueueElement elem;
+    int virt_sg_index = 0;
+    uint32_t push_len = 0;
 
-    TRACE("maru_virtio_tablet_notify\n");
-
-    if (unlikely(!virtio_queue_ready(vtk->vq))) {
+    if (unlikely(!virtio_queue_ready(vt->vq))) {
         ERR("virtio queue is not ready\n");
         return;
     }
 
-    while (true) {
-        if (event_queue_cnt == 0) {
-            TRACE("no event\n");
-            break;
-        }
+    qemu_mutex_lock(&vt->mutex);
+    if (!vt->avail) {
+        TRACE("guest queue buffer is not ready\n");
+        qemu_mutex_unlock(&vt->mutex);
+        return;
+    }
+    qemu_mutex_unlock(&vt->mutex);
+
+    /* Get a queue buffer which is written by guest side. */
+    do {
+        virt_sg_index = virtqueue_pop(vt->vq, &elem);
+    } while (virt_sg_index != 0);
 
+    qemu_mutex_lock(&vt->mutex);
+    while (!QTAILQ_EMPTY(&events_queue)) {
         /* get tablet event from host queue */
         event_entry = QTAILQ_FIRST(&events_queue);
 
         /* copy event into virtio buffer */
-        memcpy(elem_vtk.in_sg[vqidx++].iov_base, &(event_entry->tablet),
-                sizeof(EmulTabletEvent));
-        if (vqidx == MAX_BUF_COUNT) {
-            vqidx = 0;
-        }
-
-        virtqueue_push(vtk->vq, &elem_vtk, sizeof(EmulTabletEvent));
-        virtio_notify(&vtk->vdev, vtk->vq);
-
-        pthread_mutex_lock(&event_mutex);
+        memcpy(elem.in_sg[0].iov_base + push_len,
+               &(event_entry->tablet),
+               sizeof(EmulTabletEvent));
+        push_len += sizeof(EmulTabletEvent);
 
         /* remove host event */
         QTAILQ_REMOVE(&events_queue, event_entry, node);
         event_queue_cnt--;
-
-        pthread_mutex_unlock(&event_mutex);
     }
+    virtqueue_push(vt->vq, &elem, push_len);
+    virtio_notify(&vt->vdev, vt->vq);
+    vt->avail = false;
+    qemu_mutex_unlock(&vt->mutex);
+
+    TRACE("maru_virtio_tablet_bh: push_len(%u), push events(%zu)\n",
+          push_len, push_len / sizeof(EmulTabletEvent));
 }
 
 static uint64_t virtio_tablet_get_features(
@@ -186,58 +181,69 @@ static uint64_t virtio_tablet_get_features(
     return request_features;
 }
 
-static void maru_tablet_bh(void *opaque)
-{
-    maru_virtio_tablet_notify();
-}
-
 static void virtio_tablet_device_realize(DeviceState *dev, Error **errp)
 {
-    INFO("initialize the tablet device\n");
     VirtIODevice *vdev = VIRTIO_DEVICE(dev);
-    vtk = VIRTIO_MARU_TABLET(dev);
+    VirtIOTablet *vt = NULL;
 
     if (vdev == NULL) {
         ERR("failed to initialize the tablet device\n");
         return;
     }
+    if (g_vt) {
+        INFO("the tablet device is already exsit\n");
+        return;
+    }
 
-    virtio_init(vdev, TYPE_VIRTIO_MARU_TABLET, VIRTIO_ID_MARU_TABLET, 0);
+    vt = VIRTIO_MARU_TABLET(vdev);
 
-    vtk->vq = virtio_add_queue(vdev, MAX_BUF_COUNT, maru_virtio_tablet_handle);
+    virtio_init(vdev, TYPE_VIRTIO_MARU_TABLET, VIRTIO_ID_MARU_TABLET, 0);
 
-    vtk->qdev = dev;
+    vt->vq = virtio_add_queue(vdev,
+                              MAX_VQ_SIZE,
+                              maru_virtio_tablet_handle);
 
-    /* reset the counters */
-    pthread_mutex_lock(&event_mutex);
-    event_queue_cnt = event_ringbuf_cnt = 0;
-    pthread_mutex_unlock(&event_mutex);
+    vt->qdev = dev;
 
-    elem_queue_cnt = elem_ringbuf_cnt = 0;
+    qemu_mutex_init(&vt->mutex);
 
     /* bottom-half */
-    vtk->bh = qemu_bh_new(maru_tablet_bh, vtk);
+    vt->bh = qemu_bh_new(maru_tablet_bh, vt);
+    g_vt = vt;
+
+    INFO("initialize the tablet device\n");
 }
 
 static void virtio_tablet_device_unrealize(DeviceState *dev, Error **errp)
 {
     VirtIODevice *vdev = VIRTIO_DEVICE(dev);
+    VirtIOTablet *vt = VIRTIO_MARU_TABLET(vdev);
 
     INFO("exit the tablet device\n");
 
-    if (vtk->bh) {
-        qemu_bh_delete(vtk->bh);
+    if (vt->bh) {
+        qemu_bh_delete(vt->bh);
     }
 
     virtio_cleanup(vdev);
 
-    pthread_mutex_destroy(&event_mutex);
+    qemu_mutex_destroy(&vt->mutex);
 }
 
 static void virtio_tablet_device_reset(VirtIODevice *vdev)
 {
+    VirtIOTablet *vt = VIRTIO_MARU_TABLET(vdev);
+
+    /* reset the counters */
+    qemu_mutex_lock(&vt->mutex);
+    event_queue_cnt = event_ringbuf_cnt = 0;
+    memset(_events_buf, 0x00,
+           sizeof(TabletEventEntry) * MAX_TABLET_EVENT_CNT);
+    QTAILQ_INIT(&events_queue);
+    vt->avail = false;
+    qemu_mutex_unlock(&vt->mutex);
+
     INFO("reset tablet device\n");
-    vqidx = 0;
 }
 
 static void virtio_tablet_class_init(ObjectClass *klass, void *data)
index 069582e..0ea5556 100644 (file)
@@ -4,6 +4,8 @@
  * Copyright (c) 2015 Samsung Electronics Co., Ltd All Rights Reserved
  *
  * Contact:
+ *  Jinhyung Jo <jinhyung.jo@samsung.com>
+ *  SeokYeon Hwang <syeon.hwang@samsung.com>
  *  Sungmin Ha <sungmin82.ha@samsung.com>
  *  Sangho Park <sangho.p@samsung.com>
  *
@@ -29,7 +31,6 @@
 #ifndef MARU_TABLET_H_
 #define MARU_TABLET_H_
 
-#include "ui/console.h"
 #include "hw/virtio/virtio.h"
 
 #define TYPE_VIRTIO_MARU_TABLET "virtio-maru-tablet-device"
@@ -45,6 +46,8 @@ typedef struct VirtIOTablet
 
     QEMUBH *bh;
     DeviceState *qdev;
+    QemuMutex mutex;
+    bool avail; /* is guest inbuf avail */
 } VirtIOTablet;
 
 /* This structure must match the kernel definitions */
@@ -60,6 +63,5 @@ VirtIODevice *maru_virtio_tablet_init(DeviceState *dev);
 void maru_virtio_tablet_exit(VirtIODevice *vdev);
 
 void maru_tablet_event(int event_type, int x, int y, int btn, int btn_status);
-void maru_virtio_tablet_notify(void);
 
 #endif /* MARU_TABLET_H_ */