From: Jinhyung Jo Date: Tue, 23 Feb 2016 08:28:18 +0000 (+0900) Subject: rotary: correct use of virtioqueue to fix bug X-Git-Tag: Tizen_Studio_1.3_Release_p2.3.2~57 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=a3a912add02087e6a65f5e80bd336de514a21ab4;p=sdk%2Femulator%2Fqemu.git 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 (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 83ec80a2e5..031b022db2 100644 --- 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 82bf8d4615..d3c65a087f 100644 --- 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;