From 0cea71a207508c2b8f563b2644ac46009832c8f4 Mon Sep 17 00:00:00 2001 From: "Michael S. Tsirkin" Date: Mon, 24 Sep 2012 15:09:30 +0200 Subject: [PATCH 1/6] virtio: don't mark unaccessed memory as dirty offset of accessed buffer is calculated using iov_length, so it can exceed accessed len. If that happens math in len - offset wraps around, and size becomes wrong. As real value is 0, so this is harmless but unnecessary. Signed-off-by: Michael S. Tsirkin --- hw/virtio.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/virtio.c b/hw/virtio.c index 209c76375..b5764bb8f 100644 --- a/hw/virtio.c +++ b/hw/virtio.c @@ -241,7 +241,7 @@ void virtqueue_fill(VirtQueue *vq, const VirtQueueElement *elem, elem->in_sg[i].iov_len, 1, size); - offset += elem->in_sg[i].iov_len; + offset += size; } for (i = 0; i < elem->out_num; i++) From 40bad8f3deba15e2074ff34cfe923c12916b1cc5 Mon Sep 17 00:00:00 2001 From: "Michael S. Tsirkin" Date: Mon, 24 Sep 2012 15:15:43 +0200 Subject: [PATCH 2/6] virtio-net: fix used len for tx There is no out sg for TX, so used buf length for tx should always be 0. Signed-off-by: Michael S. Tsirkin --- hw/virtio-net.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/hw/virtio-net.c b/hw/virtio-net.c index 649074329..247d7bef5 100644 --- a/hw/virtio-net.c +++ b/hw/virtio-net.c @@ -690,7 +690,7 @@ static void virtio_net_tx_complete(NetClientState *nc, ssize_t len) { VirtIONet *n = DO_UPCAST(NICState, nc, nc)->opaque; - virtqueue_push(n->tx_vq, &n->async_tx.elem, n->async_tx.len); + virtqueue_push(n->tx_vq, &n->async_tx.elem, 0); virtio_notify(&n->vdev, n->tx_vq); n->async_tx.elem.out_num = n->async_tx.len = 0; @@ -754,7 +754,7 @@ static int32_t virtio_net_flush_tx(VirtIONet *n, VirtQueue *vq) len += ret; - virtqueue_push(vq, &elem, len); + virtqueue_push(vq, &elem, 0); virtio_notify(&n->vdev, vq); if (++num_packets >= n->tx_burst) { From 844b5cea8ea6cbe964670a26d1b34037067569df Mon Sep 17 00:00:00 2001 From: "Michael S. Tsirkin" Date: Mon, 24 Sep 2012 12:50:32 +0200 Subject: [PATCH 3/6] iov: add const annotation iov_from_buf does not change iov, make it const. Signed-off-by: Michael S. Tsirkin --- iov.c | 2 +- iov.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/iov.c b/iov.c index 60705c73a..c6a66f0af 100644 --- a/iov.c +++ b/iov.c @@ -26,7 +26,7 @@ # include #endif -size_t iov_from_buf(struct iovec *iov, unsigned int iov_cnt, +size_t iov_from_buf(const struct iovec *iov, unsigned int iov_cnt, size_t offset, const void *buf, size_t bytes) { size_t done; diff --git a/iov.h b/iov.h index 381f37a54..a73569f94 100644 --- a/iov.h +++ b/iov.h @@ -36,7 +36,7 @@ size_t iov_size(const struct iovec *iov, const unsigned int iov_cnt); * such "large" value is -1 (sinice size_t is unsigned), * so specifying `-1' as `bytes' means 'up to the end of iovec'. */ -size_t iov_from_buf(struct iovec *iov, unsigned int iov_cnt, +size_t iov_from_buf(const struct iovec *iov, unsigned int iov_cnt, size_t offset, const void *buf, size_t bytes); size_t iov_to_buf(const struct iovec *iov, const unsigned int iov_cnt, size_t offset, void *buf, size_t bytes); From 385ce95d9d060f20870402c8b2b503d0b6ab8af0 Mon Sep 17 00:00:00 2001 From: Amit Shah Date: Tue, 25 Sep 2012 00:05:14 +0530 Subject: [PATCH 4/6] virtio: use unsigned int for counting bytes in vq The virtqueue_avail_bytes() function counts bytes in an int. Use an unsigned int instead. Signed-off-by: Amit Shah Signed-off-by: Michael S. Tsirkin --- hw/virtio.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/virtio.c b/hw/virtio.c index b5764bb8f..cfad36361 100644 --- a/hw/virtio.c +++ b/hw/virtio.c @@ -338,7 +338,7 @@ static unsigned virtqueue_next_desc(target_phys_addr_t desc_pa, int virtqueue_avail_bytes(VirtQueue *vq, int in_bytes, int out_bytes) { unsigned int idx; - int total_bufs, in_total, out_total; + unsigned int total_bufs, in_total, out_total; idx = vq->last_avail_idx; From 0d8d7690850eb0cf2b2b60933cf47669a6b6f18f Mon Sep 17 00:00:00 2001 From: Amit Shah Date: Tue, 25 Sep 2012 00:05:15 +0530 Subject: [PATCH 5/6] virtio: Introduce virtqueue_get_avail_bytes() The current virtqueue_avail_bytes() is oddly named, and checks if a particular number of bytes are available in a vq. A better API is to fetch the number of bytes available in the vq, and let the caller do what's interesting with the numbers. Introduce virtqueue_get_avail_bytes(), which returns the number of bytes for buffers marked for both, in as well as out. virtqueue_avail_bytes() is made a wrapper over this new function. Signed-off-by: Amit Shah Signed-off-by: Michael S. Tsirkin --- hw/virtio.c | 28 +++++++++++++++++++++------- hw/virtio.h | 5 ++++- 2 files changed, 25 insertions(+), 8 deletions(-) diff --git a/hw/virtio.c b/hw/virtio.c index cfad36361..6821092df 100644 --- a/hw/virtio.c +++ b/hw/virtio.c @@ -335,7 +335,8 @@ static unsigned virtqueue_next_desc(target_phys_addr_t desc_pa, return next; } -int virtqueue_avail_bytes(VirtQueue *vq, int in_bytes, int out_bytes) +void virtqueue_get_avail_bytes(VirtQueue *vq, unsigned int *in_bytes, + unsigned int *out_bytes) { unsigned int idx; unsigned int total_bufs, in_total, out_total; @@ -380,13 +381,9 @@ int virtqueue_avail_bytes(VirtQueue *vq, int in_bytes, int out_bytes) } if (vring_desc_flags(desc_pa, i) & VRING_DESC_F_WRITE) { - if (in_bytes > 0 && - (in_total += vring_desc_len(desc_pa, i)) >= in_bytes) - return 1; + in_total += vring_desc_len(desc_pa, i); } else { - if (out_bytes > 0 && - (out_total += vring_desc_len(desc_pa, i)) >= out_bytes) - return 1; + out_total += vring_desc_len(desc_pa, i); } } while ((i = virtqueue_next_desc(desc_pa, i, max)) != max); @@ -395,7 +392,24 @@ int virtqueue_avail_bytes(VirtQueue *vq, int in_bytes, int out_bytes) else total_bufs++; } + if (in_bytes) { + *in_bytes = in_total; + } + if (out_bytes) { + *out_bytes = out_total; + } +} +int virtqueue_avail_bytes(VirtQueue *vq, unsigned int in_bytes, + unsigned int out_bytes) +{ + unsigned int in_total, out_total; + + virtqueue_get_avail_bytes(vq, &in_total, &out_total); + if ((in_bytes && in_bytes < in_total) + || (out_bytes && out_bytes < out_total)) { + return 1; + } return 0; } diff --git a/hw/virtio.h b/hw/virtio.h index 7a4f56452..80de3757e 100644 --- a/hw/virtio.h +++ b/hw/virtio.h @@ -147,7 +147,10 @@ void virtqueue_fill(VirtQueue *vq, const VirtQueueElement *elem, void virtqueue_map_sg(struct iovec *sg, target_phys_addr_t *addr, size_t num_sg, int is_write); int virtqueue_pop(VirtQueue *vq, VirtQueueElement *elem); -int virtqueue_avail_bytes(VirtQueue *vq, int in_bytes, int out_bytes); +int virtqueue_avail_bytes(VirtQueue *vq, unsigned int in_bytes, + unsigned int out_bytes); +void virtqueue_get_avail_bytes(VirtQueue *vq, unsigned int *in_bytes, + unsigned int *out_bytes); void virtio_notify(VirtIODevice *vdev, VirtQueue *vq); From ad3005ad8c70a69705149d3ce6d1e51fb76edb15 Mon Sep 17 00:00:00 2001 From: Amit Shah Date: Tue, 25 Sep 2012 00:05:16 +0530 Subject: [PATCH 6/6] virtio-serial-bus: let chardev know the exact number of bytes requested Using the virtqueue_avail_bytes() function had an unnecessarily crippling effect on the number of bytes needed by the guest as reported to the chardev layer in the can_read() callback. Using the new virtqueue_get_avail_bytes() function will let us advertise the exact number of bytes we can send to the guest. Signed-off-by: Amit Shah Signed-off-by: Michael S. Tsirkin --- hw/virtio-serial-bus.c | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c index 82073f5dc..d20bd8bf7 100644 --- a/hw/virtio-serial-bus.c +++ b/hw/virtio-serial-bus.c @@ -287,6 +287,7 @@ ssize_t virtio_serial_write(VirtIOSerialPort *port, const uint8_t *buf, size_t virtio_serial_guest_ready(VirtIOSerialPort *port) { VirtQueue *vq = port->ivq; + unsigned int bytes; if (!virtio_queue_ready(vq) || !(port->vser->vdev.status & VIRTIO_CONFIG_S_DRIVER_OK) || @@ -296,14 +297,8 @@ size_t virtio_serial_guest_ready(VirtIOSerialPort *port) if (use_multiport(port->vser) && !port->guest_connected) { return 0; } - - if (virtqueue_avail_bytes(vq, 4096, 0)) { - return 4096; - } - if (virtqueue_avail_bytes(vq, 1, 0)) { - return 1; - } - return 0; + virtqueue_get_avail_bytes(vq, &bytes, NULL); + return bytes; } static void flush_queued_data_bh(void *opaque)