Re: [Qemu-devel] [PATCH 4/4] virtio-rng: hardware random number generator device

[Amit Shah <amit.shah@redhat.com>]

On (Tue) 26 Jun 2012 [08:01:20], Anthony Liguori wrote:
> On 06/26/2012 05:48 AM, Amit Shah wrote:
> >On (Mon) 25 Jun 2012 [17:59:28], Anthony Liguori wrote:
> >>On 06/25/2012 05:46 PM, Anthony Liguori wrote:
> >>>From: Amit Shah<amit.shah@redhat.com>
> >
> >>>diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c
> >
> >>>+static void virtio_rng_class_init(ObjectClass *klass, void *data)
> >>>+{
> >>>+    DeviceClass *dc = DEVICE_CLASS(klass);
> >>>+    PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
> >>>+
> >>>+    k->init = virtio_rng_init_pci;
> >>>+    k->exit = virtio_rng_exit_pci;
> >>>+    k->vendor_id = PCI_VENDOR_ID_REDHAT_QUMRANET;
> >>>+    k->device_id = PCI_DEVICE_ID_VIRTIO_RNG;
> >>>+    k->revision = VIRTIO_PCI_ABI_VERSION;
> >>>+    k->class_id = PCI_CLASS_OTHERS;
> >>
> >>WHQL tends to get very particular about PCI classes.  Do we
> >>understand the implications of making this CLASS_OTHERS and WHQL?
> >
> >I've not asked around; will update with info when I get it.
> 
> Thanks.

... and I heard back: PCI_CLASS_OTHERS is fine; no problem.

> >>>+/* 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;
> >>>+    size_t len;
> >>>+    int offset;
> >>>+
> >>>+    if (!is_guest_ready(vrng)) {
> >>>+        return;
> >>>+    }
> >>>+
> >>>+    offset = 0;
> >>>+    while (offset<   size) {
> >>>+        if (!pop_an_elem(vrng)) {
> >>>+            break;
> >>>+        }
> >>>+        len = iov_from_buf(vrng->elem.in_sg, vrng->elem.in_num,
> >>>+                           buf + offset, 0, size - offset);
> >>>+        offset += len;
> >>>+
> >>>+        virtqueue_push(vrng->vq,&vrng->elem, len);
> >>>+        vrng->popped = false;
> >>>+    }
> >>>+    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);
> >>>+    }
> >>
> >>Because of this above while() loop, you won't see entropy requests
> >>for every request that comes from the guest depending on how data
> >>gets buffered in the socket.
> >
> >So the issue is we currently can't get the iov_size without popping
> >the elem from the vq.
> 
> I think we could split out some of the logic in virtqueue_pop to
> implement a virtqueue_peek().

Just sent out a series that adds virtqueue_get_avail_bytes() which
does this.  A rebased series on top of that eliminates the need for
popping and save/load of the elem.

		Amit