rotary: correct use of virtioqueue to fix bug

When using the rotary device, the segmentation fault occurs in the host.
Its cause is due to use the virtioqueue in the wrong way.
So correct with the kernel driver side.
Fix the fault and simplify complex sources.

Conflicts:
    ;; VIRTIO_ROTARY to VIRTIO_MARU_ROTARY
	tizen/src/hw/virtio/maru_virtio_rotary.c

Change-Id: I85d0517a6567de742bbd56cd756faa9b8f47e58c
Signed-off-by: Jinhyung Jo <jinhyung.jo@samsung.com>
(cherry picked from commit b4ec658343d4808aba2f4f3d04f26f65f3cb73c2)

diff --git a/tizen/src/hw/virtio/maru_virtio_rotary.c b/tizen/src/hw/virtio/maru_virtio_rotary.c
index 83ec80a..031b022 100644 (file)
--- a/tizen/src/hw/virtio/maru_virtio_rotary.c
+++ b/tizen/src/hw/virtio/maru_virtio_rotary.c
@@ -33,116 +33,138 @@
 
 DECLARE_DEBUG_CHANNEL(rotary);
 
+#define EVENT_BUF_SIZE (ROTARY_QUEUE_SIZE * sizeof(MrRotaryEvent))
+
 static VirtIORotary *g_vrtr;
-static VirtQueueElement elem;
+
+/* really simple queue implementation, stability is not considered */
+static void rotaryqueue_reset(VirtIORotaryQueue *q)
+{
+    /* reset all: count, write pointer, read pointer & event datas */
+    memset(q, 0x00, sizeof(VirtIORotaryQueue));
+}
+
+static int rotaryqueue_push_tail(VirtIORotaryQueue *q, MrRotaryEvent *e)
+{
+    if (q->count > ROTARY_QUEUE_SIZE) {
+        return -1;
+    }
+    memcpy(&q->event[q->wptr++], e, sizeof(MrRotaryEvent));
+    q->count++;
+    LOG_TRACE("[%s] count(%d), wptr(%d)\n", __func__, q->count, q->wptr);
+
+    return 0;
+}
+
+static int rotaryqueue_check_avail(VirtIORotaryQueue *q)
+{
+    return q->count;
+}
+
+static MrRotaryEvent *rotaryqueue_pop_head(VirtIORotaryQueue *q)
+{
+    MrRotaryEvent *e = NULL;
+    if (q->count) {
+        e = &q->event[q->rptr++];
+        q->count--;
+    }
+
+    return e;
+}
 
 static void virtio_rotary_handle(VirtIODevice *vdev, VirtQueue *vq)
 {
     VirtIORotary *vrtr = VIRTIO_MARU_ROTARY(vdev);
-    int index = 0;
 
     if (virtio_queue_empty(vrtr->vq)) {
         LOG_INFO("[%s] virtio queue is empty\n", __func__);
         return;
     }
 
-    /* Get a queue buffer which is written by guest side. */
-    do {
-        index = virtqueue_pop(vq, &elem);
-        LOG_TRACE("[%s] virtqueue_pop: index(%d)\n", __func__, index);
-    } while (index < ROTARY_QUEUE_SIZE);
+    qemu_mutex_lock(&vrtr->mutex);
+    vrtr->avail = true;
+    qemu_mutex_unlock(&vrtr->mutex);
 }
 
 void maru_rotary_event(int32_t delta, int32_t type)
 {
     MrRotaryEvent event = {0, };
-    uint32_t *index = NULL;
-
-    LOG_TRACE("[%s] ENTER: delta(%d), type(%d)\n", __func__, delta, type);
+    int ret;
 
+    /* skip useless data */
+    if (!delta && !type) {
+        return;
+    }
     if (!g_vrtr) {
-        LOG_SEVERE("[%s] VirtIORotary instance is NULL\n", __func__);
+        LOG_WARNING("[%s] VirtIORotary instance is NULL\n", __func__);
         return;
     }
-
     if (!virtio_queue_ready(g_vrtr->vq)) {
         LOG_INFO("[%s] virtqueue is not ready\n", __func__);
         return;
     }
 
-    index = &(g_vrtr->queue.idx);
-    LOG_TRACE("[%s] wptr(%u)\n", __func__, g_vrtr->queue.wptr);
-
-    if (*index == ROTARY_QUEUE_SIZE) {
-        *index = 0;
-    }
+    LOG_TRACE("[%s] delta(%d), type(%d)\n", __func__, delta, type);
     event.delta = delta;
     event.type = type;
 
     qemu_mutex_lock(&g_vrtr->mutex);
-    memcpy(&g_vrtr->queue.event[*index], &event, sizeof(MrRotaryEvent));
-    LOG_TRACE("[%s] event: delta(%d), type(%d), index(%u)\n",
-        __func__, event.delta, event.type, *index);
-    (*index)++;
-    g_vrtr->queue.wptr++;
+    ret = rotaryqueue_push_tail(&g_vrtr->queue, &event);
     qemu_mutex_unlock(&g_vrtr->mutex);
+    if (ret) {
+        LOG_WARNING("failed to process rotary event: rotaryqueue is full\n");
+        return;
+    }
     qemu_bh_schedule(g_vrtr->bh);
-    LOG_TRACE("[%s] LEAVE\n", __func__);
 }
 
 static void virtio_rotary_bh(void *opaque)
 {
     VirtIORotary *vrtr = (VirtIORotary *)opaque;
-    MrRotaryEvent *cur_event;
-    uint32_t exec_cnt = 0;
-
-    LOG_TRACE("[%s] ENTER\n", __func__);
+    MrRotaryEvent *event;
+    VirtQueueElement elem;
+    int sg_num;
+    uint32_t push_len = 0;
 
     if (!vrtr) {
-        LOG_SEVERE("[%s] VirtIORotary instance is NULL\n", __func__);
+        LOG_WARNING("[%s] VirtIORotary instance is NULL\n", __func__);
     }
-
     if (unlikely(!virtio_queue_ready(vrtr->vq))) {
-        LOG_WARNING("[%s] virt queue is not ready\n", __func__);
+        LOG_WARNING("[%s] virtio queue is not ready\n", __func__);
         return;
     }
-
-    if (vrtr->queue.rptr == ROTARY_QUEUE_SIZE) {
-        vrtr->queue.rptr = 0;
+    qemu_mutex_lock(&vrtr->mutex);
+    if (!vrtr->avail) {
+        LOG_TRACE("[%s] guest inbuf is not ready\n", __func__);
+        qemu_mutex_unlock(&vrtr->mutex);
+        return;
     }
+    qemu_mutex_unlock(&vrtr->mutex);
+
+    /* get a queue buffer which is written by guest side. */
+    do {
+        sg_num = virtqueue_pop(vrtr->vq, &elem);
+        LOG_TRACE("[%s] virtqueue_pop: sg_num(%d)\n", __func__, sg_num);
+    } while (sg_num != 0);
 
     qemu_mutex_lock(&vrtr->mutex);
-    exec_cnt = vrtr->queue.wptr;
-
-    while (exec_cnt--) {
-        cur_event = &vrtr->queue.event[vrtr->queue.rptr];
-
-        if (((MrRotaryEvent *)(elem.in_sg[vrtr->queue.rptr].iov_base))->delta
-            != 0) {
-            LOG_TRACE("[%s] FIXME: virtio queue is full\n", __func__);
-        }
-
-        memcpy(elem.in_sg[vrtr->queue.rptr].iov_base,
-            cur_event,
-            sizeof(MrRotaryEvent));
-        memset(cur_event, 0x00, sizeof(MrRotaryEvent));
-
-        if (vrtr->queue.wptr > 0) {
-            vrtr->queue.wptr--;
-            LOG_TRACE("[%s] exec_cnt(%u), wptr(%u), rptr(%u)\n",
-                    __func__, exec_cnt, vrtr->queue.wptr, vrtr->queue.rptr);
-        }
-
-        vrtr->queue.rptr++;
-        if (vrtr->queue.rptr == ROTARY_QUEUE_SIZE) {
-            vrtr->queue.rptr = 0;
-        }
+    while (rotaryqueue_check_avail(&vrtr->queue)) {
+        event = rotaryqueue_pop_head(&vrtr->queue);
+        /* rotary kernel driver has only one scattergather list */
+        memcpy(elem.in_sg[0].iov_base + push_len,
+               event, sizeof(MrRotaryEvent));
+        push_len += sizeof(MrRotaryEvent);
     }
-    qemu_mutex_unlock(&vrtr->mutex);
 
-    virtqueue_push(vrtr->vq, &elem, sizeof(MrRotaryEvent));
+    /* length which is really written to the guest */
+    virtqueue_push(vrtr->vq, &elem, push_len);
     virtio_notify(&vrtr->vdev, vrtr->vq);
-    LOG_TRACE("[%s] LEAVE\n", __func__);
+    rotaryqueue_reset(&vrtr->queue);
+    vrtr->avail = false;
+    qemu_mutex_unlock(&vrtr->mutex);
+
+    LOG_TRACE("[%s] push len(%d), push events(%d)\n",
+              __func__, push_len, push_len / sizeof(MrRotaryEvent));
 }
 
 static uint64_t virtio_rotary_get_features(VirtIODevice *vdev,
@@ -156,6 +178,10 @@ static void virtio_rotary_device_realize(DeviceState *dev, Error **errp)
     VirtIODevice *vdev = VIRTIO_DEVICE(dev);
     VirtIORotary *vrtr = VIRTIO_MARU_ROTARY(vdev);
 
+    if (g_vrtr) {
+        LOG_WARNING("rotary device is already exist, skip\n");
+    }
+
     if (!vrtr) {
         LOG_SEVERE("failed to initialize rotary device\n");
         return;
@@ -164,14 +190,12 @@ static void virtio_rotary_device_realize(DeviceState *dev, Error **errp)
     virtio_init(vdev, TYPE_VIRTIO_MARU_ROTARY, VIRTIO_ID_MARU_ROTARY, 0);
     qemu_mutex_init(&vrtr->mutex);
 
+    vrtr->avail = false;
     vrtr->vq = virtio_add_queue(&vrtr->vdev,
-                                ROTARY_QUEUE_SIZE,
+                                EVENT_BUF_SIZE,
                                 virtio_rotary_handle);
-
     vrtr->qdev = dev;
-
     vrtr->bh = qemu_bh_new(virtio_rotary_bh, vrtr);
-
     g_vrtr = vrtr;
 
     LOG_INFO("initialize rotary device\n");
@@ -196,7 +220,8 @@ static void virtio_rotary_device_reset(VirtIODevice *vdev)
 {
     VirtIORotary *vrtr = VIRTIO_MARU_ROTARY(vdev);
 
-    memset(&vrtr->queue, 0x00, sizeof(VirtIORotaryQueue));
+    vrtr->avail = false;
+    rotaryqueue_reset(&vrtr->queue);
     LOG_INFO("reset roraty device\n");
 }
 

diff --git a/tizen/src/hw/virtio/maru_virtio_rotary.h b/tizen/src/hw/virtio/maru_virtio_rotary.h
index 82bf8d4..d3c65a0 100644 (file)
--- a/tizen/src/hw/virtio/maru_virtio_rotary.h
+++ b/tizen/src/hw/virtio/maru_virtio_rotary.h
@@ -37,7 +37,7 @@ extern "C" {
 #include "hw/virtio/virtio.h"
 
 /* number of queue must be power of 2 */
-#define ROTARY_QUEUE_SIZE   512
+#define ROTARY_QUEUE_SIZE   32
 
 typedef struct MrRotaryEvent {
     int32_t delta;
@@ -45,7 +45,7 @@ typedef struct MrRotaryEvent {
 } MrRotaryEvent;
 
 typedef struct VirtIORotaryQueue {
-    uint32_t idx;
+    uint32_t count;
     uint32_t rptr;
     uint32_t wptr;
     MrRotaryEvent event[ROTARY_QUEUE_SIZE];
@@ -58,6 +58,7 @@ typedef struct VirtIORotary {
 
     QEMUBH              *bh;
     QemuMutex           mutex;
+    bool                avail; /* is guest inbuf avail */
     VirtIORotaryQueue   queue;
 } VirtIORotary;