On 03/30/2010 09:13 AM, Ian Molton wrote:
> From 5f484301d73fa53009bbcd430f8ae85868b67772 Mon Sep 17 00:00:00 2001
From: Ian Molton<ian.mol...@collabora.co.uk>
Date: Tue, 17 Nov 2009 14:34:12 +0000
Subject: [PATCH 2/4] virtio: Add virtio-rng driver
This patch adds support for virtio-rng. Data is read from a chardev and
can be either raw entropy or received via the EGD protocol.
Signed-off-by: Ian Molton<ian.mol...@collabora.co.uk>
Hi Ian,
A couple review comments below. I've read the thread. virtio-rng
should stand on it's own as it's a spec'd out virtio device so I'm with
you there.
The SIZE qdev patch can't be applied until there's a consumer of it.
I've already expressed my opinion about the socket reconnect patch. I
don't think it's the right functionality for qemu to implement because
the semantics concern me. Really, a more thorough refactoring of
CharDrivers is needed such that it supported connected and disconnected
states along with QMP events for the transitions.
I agree with Paul that the EGD logic ought to be separated. It probably
makes sense to modify this a bit to have a split front-end/back-end.
For instance:
-device virtio-rng-pci,rngdev=foo -rngdev egd,addr=unix:/path/to/rng,id=foo
You could also have a syntax helper like:
-rng egd,addr:/path/to/rng
See the recent virtfs patch series for an example of this.
With those changes, I'd happily apply this series.
---
Makefile.target | 2 +-
hw/pci.h | 1 +
hw/virtio-pci.c | 29 ++++++++
hw/virtio-rng.c | 195
+++++++++++++++++++++++++++++++++++++++++++++++++++++++
hw/virtio-rng.h | 19 ++++++
hw/virtio.h | 2 +
rng.h | 14 ++++
7 files changed, 261 insertions(+), 1 deletions(-)
create mode 100644 hw/virtio-rng.c
create mode 100644 hw/virtio-rng.h
create mode 100644 rng.h
diff --git a/Makefile.target b/Makefile.target
index 16ea443..2f9c929 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -166,7 +166,7 @@ obj-y += qemu-timer.o
# virtio has to be here due to weird dependency between PCI and virtio-net.
# need to fix this properly
obj-y += virtio-blk.o virtio-balloon.o virtio-net.o virtio-serial-bus.o
-obj-y += rwhandler.o
+obj-y += virtio-rng.o rwhandler.o
obj-$(CONFIG_KVM) += kvm.o kvm-all.o
LIBS+=-lz
diff --git a/hw/pci.h b/hw/pci.h
index 20c670e..dc61cb5 100644
--- a/hw/pci.h
+++ b/hw/pci.h
@@ -69,6 +69,7 @@
#define PCI_DEVICE_ID_VIRTIO_BLOCK 0x1001
#define PCI_DEVICE_ID_VIRTIO_BALLOON 0x1002
#define PCI_DEVICE_ID_VIRTIO_CONSOLE 0x1003
+#define PCI_DEVICE_ID_VIRTIO_RNG 0x1004
#define FMT_PCIBUS PRIx64
diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c
index 6eb19cd..bee3105 100644
--- a/hw/virtio-pci.c
+++ b/hw/virtio-pci.c
@@ -23,6 +23,7 @@
#include "msix.h"
#include "net.h"
#include "block_int.h"
+#include "rng.h"
#include "loader.h"
/* from Linux's linux/virtio_pci.h */
@@ -95,6 +96,7 @@ typedef struct {
uint32_t nvectors;
BlockConf block;
NICConf nic;
+ RNGConf rng;
uint32_t host_features;
/* Max. number of ports we can have for a the virtio-serial device */
uint32_t max_virtserial_ports;
@@ -552,6 +554,21 @@ static int virtio_balloon_init_pci(PCIDevice *pci_dev)
return 0;
}
+static int virtio_rng_init_pci(PCIDevice *pci_dev)
+{
+ VirtIOPCIProxy *proxy = DO_UPCAST(VirtIOPCIProxy, pci_dev, pci_dev);
+ VirtIODevice *vdev;
+
+ vdev = virtio_rng_init(&pci_dev->qdev,&proxy->rng);
+ virtio_init_pci(proxy, vdev,
+ PCI_VENDOR_ID_REDHAT_QUMRANET,
+ PCI_DEVICE_ID_VIRTIO_RNG,
+ PCI_CLASS_OTHERS,
+ 0x00);
+
+ return 0;
+}
+
static PCIDeviceInfo virtio_info[] = {
{
.qdev.name = "virtio-blk-pci",
@@ -606,6 +623,18 @@ static PCIDeviceInfo virtio_info[] = {
},
.qdev.reset = virtio_pci_reset,
},{
+ .qdev.name = "virtio-rng-pci",
+ .qdev.size = sizeof(VirtIOPCIProxy),
+ .init = virtio_rng_init_pci,
+ .exit = virtio_exit_pci,
+ .qdev.props = (Property[]) {
+ DEFINE_PROP_CHR("chardev", VirtIOPCIProxy, rng.chrdev),
+ DEFINE_PROP_SIZE("rate", VirtIOPCIProxy, rng.rate, 0),
+ DEFINE_PROP_STRING("protocol", VirtIOPCIProxy, rng.proto),
+ DEFINE_PROP_END_OF_LIST(),
+ },
+ .qdev.reset = virtio_pci_reset,
+ },{
/* end of list */
}
};
diff --git a/hw/virtio-rng.c b/hw/virtio-rng.c
new file mode 100644
index 0000000..154ea76
--- /dev/null
+++ b/hw/virtio-rng.c
@@ -0,0 +1,195 @@
+/*
+ * Virtio RNG Device
+ *
+ * Copyright Collabora 2009
+ *
+ * Authors:
+ * Ian Molton<ian.mol...@collabora.co.uk>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2. See
+ * the COPYING file in the top-level directory.
+ *
+ */
+
+#include "hw.h"
+#include "qemu-char.h"
+#include "virtio.h"
+#include "virtio-rng.h"
+#include "rng.h"
+#include<sys/time.h>
+
+typedef struct VirtIORng
+{
+ VirtIODevice vdev;
+ VirtQueue *vq;
+ CharDriverState *chr;
+ struct timeval last;
+ int rate;
+ int egd;
+ int entropy_remaining;
+ int pool;
+} VirtIORng;
+
+/* Maximum size of the buffer the guest expects */
+#define BUFF_MAX 64
+
+/* EGD protocol - we only use this command */
+#define EGD_READ_BLOCK 0x2
+
+#define EGD_MAX_BLOCK_SIZE 255
+#define EGD_MAX_REQUESTS 3
+#define EGD_MAX_POOL_SIZE (EGD_MAX_BLOCK_SIZE * (EGD_MAX_REQUESTS-1))
+
+static inline void req_entropy(VirtIORng *s)
+{
+ static const unsigned char entropy_rq[2] = { EGD_READ_BLOCK,
+ EGD_MAX_BLOCK_SIZE };
+ if (s->egd) {
+ /* Let the socket buffer up the incoming data for us. Max block
size
+ for EGD protocol is (stupidly) 255, so make sure we always
have a
+ block pending for performance. We can have 3 outstanding
buffers */
Whitespace damaged (broken mailer?).
+ if (s->pool<= EGD_MAX_POOL_SIZE) {
+ s->chr->chr_write(s->chr, entropy_rq, sizeof(entropy_rq));
+ s->pool += EGD_MAX_BLOCK_SIZE;
+ }
+ } else {
+ s->pool = BUFF_MAX;
+ }
+}
+
+static int vrng_can_read(void *opaque)
+{
+ VirtIORng *s = (VirtIORng *) opaque;
+ struct timeval now, d;
+ int max_entropy;
+
+ if (!virtio_queue_ready(s->vq) ||
+ !(s->vdev.status& VIRTIO_CONFIG_S_DRIVER_OK) ||
+ virtio_queue_empty(s->vq))
+ return 0;
Missing braces (CODING_STYLE).
+ req_entropy(s);
+
+ if (s->rate) {
+ qemu_gettimeofday(&now);
+ timersub(&now,&s->last,&d);
+ if (d.tv_sec * 1000000 + d.tv_usec> 1000000) {
+ s->entropy_remaining = s->rate;
+ s->last = now;
+ }
Should be based on rt_clock
+ max_entropy = MIN(s->pool, s->entropy_remaining);
+ } else {
+ max_entropy = s->pool;
+ }
+
+ /* current implementations have a 64 byte buffer.
+ * We fall back to a one byte per read if there is not enough room.
+ */
+ max_entropy = MIN(max_entropy, BUFF_MAX);
+ if (max_entropy) {
+ if (virtqueue_avail_bytes(s->vq, max_entropy, 0))
+ return max_entropy;
+ if (virtqueue_avail_bytes(s->vq, 1, 0))
+ return 1;
Missing braces (CODING_STYLE).
+ }
+ return 0;
+}
+
+static void vrng_read(void *opaque, const uint8_t *buf, int size)
+{
+ VirtIORng *s = (VirtIORng *) opaque;
+ VirtQueueElement elem;
+ int offset = 0;
+
+ /* The current kernel implementation has only one outstanding input
+ * buffer of 64 bytes.
+ */
+ while (offset< size) {
+ int i = 0;
+ if (!virtqueue_pop(s->vq,&elem))
+ break;
Braces.
+ while (offset< size&& i< elem.in_num) {
+ int len = MIN(elem.in_sg[i].iov_len, size - offset);
+ memcpy(elem.in_sg[i].iov_base, buf + offset, len);
+ offset += len;
+ i++;
+ }
+ virtqueue_push(s->vq,&elem, size);
+ }
+
+ if (s->rate)
+ s->entropy_remaining -= size;
Braces (lots more too).
+ s->pool -= size;
+
+ virtio_notify(&s->vdev, s->vq);
+}
+
+static void vrng_event(void *opaque, int event)
+{
+
+}
+
+
+
+static void virtio_rng_handle(VirtIODevice *vdev, VirtQueue *vq)
+{
+ /* Nothing to do - we push data when its available */
+}
+
+static uint32_t virtio_rng_get_features(VirtIODevice *vdev, uint32_t
features)
+{
+ return 0;
+}
+
+static void virtio_rng_save(QEMUFile *f, void *opaque)
+{
+ VirtIORng *s = opaque;
+
+ virtio_save(&s->vdev, f);
+}
+
+static int virtio_rng_load(QEMUFile *f, void *opaque, int version_id)
+{
+ VirtIORng *s = opaque;
+
+ if (version_id != 1)
+ return -EINVAL;
+
+ virtio_load(&s->vdev, f);
+ return 0;
+}
+
+VirtIODevice *virtio_rng_init(DeviceState *dev, RNGConf *rngdev)
+{
+ VirtIORng *s;
+ s = (VirtIORng *)virtio_common_init("virtio-rng",
+ VIRTIO_ID_RNG,
+ 0, sizeof(VirtIORng));
+
+ if (!s)
+ return NULL;
+
+ s->vdev.get_features = virtio_rng_get_features;
+
+ s->vq = virtio_add_queue(&s->vdev, 128, virtio_rng_handle);
+ s->chr = rngdev->chrdev;
+ s->rate = rngdev->rate;
+ gettimeofday(&s->last, NULL);
+
+ if(rngdev->proto&& !strncmp(rngdev->proto, "egd", 3))
+ s->egd = 1;
+
+#ifdef DEBUG
It's more typical to introduce a debugging macro (like dprintf()) and
then have a single define at the top of the file to enable the macro.
This will make conversion to a more unified logging mechanism a lot
easier down the road.
Regards,
Anthony Liguori
+ printf("entropy being read from %s", rngdev->chrdev->label);
+ if(s->rate)
+ printf(" at %d bytes/sec max.", s->rate);
+ printf(" protocol: %s\n", s->egd?"egd":"raw");
+#endif
+
+ qemu_chr_add_handlers(s->chr, vrng_can_read, vrng_read, vrng_event, s);
+
+ register_savevm("virtio-rng", -1, 1, virtio_rng_save,
virtio_rng_load, s);
+
+ return&s->vdev;
+}
+
diff --git a/hw/virtio-rng.h b/hw/virtio-rng.h
new file mode 100644
index 0000000..bc4d2d8
--- /dev/null
+++ b/hw/virtio-rng.h
@@ -0,0 +1,19 @@
+/*
+ * Virtio RNG Support
+ *
+ * Copyright Collabora 2009
+ *
+ * Authors:
+ * Ian Molton<ian.mol...@collabora.co.uk>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2. See
+ * the COPYING file in the top-level directory.
+ *
+ */
+#ifndef _QEMU_VIRTIO_RNG_H
+#define _QEMU_VIRTIO_RNG_H
+
+/* The ID for virtio console */
+#define VIRTIO_ID_RNG 4
+
+#endif
diff --git a/hw/virtio.h b/hw/virtio.h
index 3baa2a3..749b2a6 100644
--- a/hw/virtio.h
+++ b/hw/virtio.h
@@ -16,6 +16,7 @@
#include "hw.h"
#include "net.h"
+#include "rng.h"
#include "qdev.h"
#include "sysemu.h"
#include "block_int.h"
@@ -174,6 +175,7 @@ VirtIODevice *virtio_blk_init(DeviceState *dev,
BlockConf *conf);
VirtIODevice *virtio_net_init(DeviceState *dev, NICConf *conf);
VirtIODevice *virtio_serial_init(DeviceState *dev, uint32_t max_nr_ports);
VirtIODevice *virtio_balloon_init(DeviceState *dev);
+VirtIODevice *virtio_rng_init(DeviceState *dev, RNGConf *rngdev);
void virtio_net_exit(VirtIODevice *vdev);
diff --git a/rng.h b/rng.h
new file mode 100644
index 0000000..14ae7e0
--- /dev/null
+++ b/rng.h
@@ -0,0 +1,14 @@
+#ifndef QEMU_RNG_H
+#define QEMU_RNG_H
+
+#include "qemu-option.h"
+
+/* qdev rng properties */
+
+typedef struct RNGConf {
+ CharDriverState *chrdev;
+ uint64_t rate;
+ char *proto;
+} RNGConf;
+
+#endif