Fixes for virtio-rng
cover Fixes for virtio-rng
Hey Anthony,
Here are the fixes for the virtio-rng code -- ideally these should be folded in the series before you merge, but if the original series is ready to apply, these can go in as follow-on patches.
Amit Shah (3):
virtio-rng: use virtqueue_get_avail_bytes, fix migration
virtio-rng: remove extra request for entropy
virtio-rng: fix typos, comments
hw/virtio-rng.c | 80 ++++++---------------------------------------------- include/qemu/rng.h | 6 ++-- 2 files changed, 12 insertions(+), 74 deletions(-)
Signature · 1 line
1.7.7.6
PATCH 1/3 virtio-rng: use virtqueue_get_avail_bytes, fix migration
Popping an elem from the vq just to find out its length causes problems with save/load later on. Use the new virtqueue_get_avail_bytes() function instead, saves us the complexity in the migration code.
--- hw/virtio-rng.c | 69 +++++++----------------------------------------------- 1 files changed, 9 insertions(+), 60 deletions(-)
hw/virtio-rng.c +9 -60
@@ -22,14 +22,10 @@ typedef struct VirtIORNG {/* Only one vq - guest puts buffer(s) on it when it needs entropy */ VirtQueue *vq; -VirtQueueElement elem;/* Config data for the device -- currently only chardev */ VirtIORNGConf *conf; -/* Whether we've popped a vq element into 'elem' above */ -bool popped; -RngBackend *rng; } VirtIORNG;
@@ -42,23 +38,19 @@ static bool is_guest_ready(VirtIORNG *vrng)return false; } -static size_t pop_an_elem (VirtIORNG *vrng) +static size_t get_request_size (VirtQueue *vq) { -size_t size; +unsigned int in, out; -if (!vrng->popped && !virtqueue_pop (vrng->vq, &vrng->elem)) { -return 0 ; - } - vrng->popped = true; - - size =iov_size (vrng->elem.in_sg, vrng->elem.in_num); -return size; +virtqueue_get_avail_bytes (vq, &in, &out); +return in; }/* Send data from a char device over to the guest */ static void chr_read (void *opaque,const void *buf,size_t size) {VirtIORNG *vrng = opaque; +VirtQueueElement elem;size_t len;int offset;
@@ -68,15 +60,14 @@ static void chr_read(void *opaque, const void *buf, size_t size) offset =0 ;while (offset < size) { -if (!pop_an_elem (vrng)) { +if (!virtqueue_pop (vrng->vq, &elem)) {break ; } - len =iov_from_buf (vrng->elem.in_sg, vrng->elem.in_num, + len =iov_from_buf (elem.in_sg, elem.in_num,0 , buf + offset, size - offset); offset += len; -virtqueue_push (vrng->vq, &vrng->elem, len); - vrng->popped = false; +virtqueue_push (vrng->vq, &elem, len); }virtio_notify (&vrng->vdev, vrng->vq);
@@ -96,7 +87,7 @@ static void handle_input(VirtIODevice *vdev, VirtQueue *vq)VirtIORNG *vrng =DO_UPCAST (VirtIORNG, vdev, vdev);size_t size; - size =pop_an_elem (vrng); + size =get_request_size (vq);if (size) {rng_backend_request_entropy (vrng->rng, size, chr_read, vrng); }
@@ -112,23 +103,6 @@ static void virtio_rng_save(QEMUFile *f, void *opaque)VirtIORNG *vrng = opaque;virtio_save (&vrng->vdev, f); - -qemu_put_byte (f, vrng->popped); -if (vrng->popped) { -int i; - -qemu_put_be32 (f, vrng->elem.index); - -qemu_put_be32 (f, vrng->elem.in_num); -for (i =0 ; i < vrng->elem.in_num; i++) { -qemu_put_be64 (f, vrng->elem.in_addr[i]); - } - -qemu_put_be32 (f, vrng->elem.out_num); -for (i =0 ; i < vrng->elem.out_num; i++) { -qemu_put_be64 (f, vrng->elem.out_addr[i]); - } - } }static int virtio_rng_load (QEMUFile *f,void *opaque,int version_id)
@@ -139,30 +113,6 @@ static int virtio_rng_load(QEMUFile *f, void *opaque, int version_id)return -EINVAL ; }virtio_load (&vrng->vdev, f); - - vrng->popped =qemu_get_byte (f); -if (vrng->popped) { -int i; - - vrng->elem.index =qemu_get_be32 (f); - - vrng->elem.in_num =qemu_get_be32 (f); -g_assert (vrng->elem.in_num <VIRTQUEUE_MAX_SIZE ); -for (i =0 ; i < vrng->elem.in_num; i++) { - vrng->elem.in_addr[i] =qemu_get_be64 (f); - } - - vrng->elem.out_num =qemu_get_be32 (f); -g_assert (vrng->elem.out_num <VIRTQUEUE_MAX_SIZE ); -for (i =0 ; i < vrng->elem.out_num; i++) { - vrng->elem.out_addr[i] =qemu_get_be64 (f); - } - -virtqueue_map_sg (vrng->elem.in_sg, vrng->elem.in_addr, - vrng->elem.in_num,1 ); -virtqueue_map_sg (vrng->elem.out_sg, vrng->elem.out_addr, - vrng->elem.out_num,0 ); - }return 0 ; }
@@ -195,7 +145,6 @@ VirtIODevice *virtio_rng_init(DeviceState *dev, VirtIORNGConf *conf) vrng->qdev = dev; vrng->conf = conf; - vrng->popped = false;register_savevm (dev,"virtio-rng" ,-1 ,1 , virtio_rng_save, virtio_rng_load, vrng);
Signature · 1 line
1.7.7.6
PATCH 2/3 virtio-rng: remove extra request for entropy
In reply to Amit ShahIf we just got a few bytes from the backends, but the guest is ready to accept more, don't poke the backends once more for more bytes; it already knows (from the request in handle_input()).
--- hw/virtio-rng.c | 10 ---------- 1 files changed, 0 insertions(+), 10 deletions(-)
hw/virtio-rng.c +0 -10
@@ -70,16 +70,6 @@ static void chr_read(void *opaque, const void *buf, size_t size)virtqueue_push (vrng->vq, &elem, len); }virtio_notify (&vrng->vdev, vrng->vq); - -/* -* Lastly, if we had multiple elems queued by the guest, and we -* didn't have enough data to fill them all, indicate we want more -* data. -*/ - len =pop_an_elem (vrng); -if (len) { -rng_backend_request_entropy (vrng->rng, size, chr_read, vrng); - } }static void handle_input (VirtIODevice *vdev,VirtQueue *vq)
Signature · 1 line
1.7.7.6
PATCH 3/3 virtio-rng: fix typos, comments
In reply to Amit ShahFix typos and update comments to match current implementation.
--- hw/virtio-rng.c | 1 - include/qemu/rng.h | 6 +++--- 2 files changed, 3 insertions(+), 4 deletions(-)
hw/virtio-rng.c +0 -1
@@ -23,7 +23,6 @@ typedef struct VirtIORNG {/* Only one vq - guest puts buffer(s) on it when it needs entropy */ VirtQueue *vq; -/* Config data for the device -- currently only chardev */ VirtIORNGConf *conf;RngBackend *rng;
include/qemu/rng.h +3 -3
@@ -61,10 +61,10 @@ struct RngBackend* This function is used by the front-end to request entropy from an entropy * source. This function can be called multiple times before @receive_entropy * is invoked with different values of @receive_entropy and @opaque. The -* backend will queue each request and handle appropriate. +* backend will queue each request and handle appropriately. * * The backend does not need to pass the full amount of data to @receive_entropy -* but will pass at a value greater than 0. +* but will pass a a value greater than 0. */ void rng_backend_request_entropy(RngBackend *s, size_t size, EntropyReceiveFunc *receive_entropy,
@@ -87,7 +87,7 @@ void rng_backend_cancel_requests(RngBackend *s);* * This function will open the backend if it is not already open. Calling this * function on an already opened backend will not result in an error. -*/ +*/ void rng_backend_open (RngBackend *s,Error **errp);#endif
Signature · 1 line
1.7.7.6