[v2,1/1] virtio-rng: hardware random number generator device

The Linux kernel already has a virtio-rng driver, this is the device
implementation.

When the guest asks for entropy from the virtio hwrng, it puts a buffer
in the vq.  We then put entropy into that buffer, and push it back to
the guest.

The chardev connected to this device is fed the data to be sent to the
guest.

Invocation is simple:

  $ qemu ... -device virtio-rng-pci,chardev=foo

In the guest, we see

  $ cat /sys/devices/virtual/misc/hw_random/rng_available
  virtio

  $ cat /sys/devices/virtual/misc/hw_random/rng_current
  virtio

  # cat /dev/hwrng

Simply feeding /dev/urandom from the host to the chardev is sufficient:

  $ qemu ... -chardev socket,path=/tmp/foo,server,nowait,id=foo \
             -device virtio-rng,chardev=foo

  $ nc -U /tmp/foo < /dev/urandom

A QMP event is sent for interested apps to monitor activity and send the
appropriate number of bytes that get asked by the guest:

  {"timestamp": {"seconds": 1337966878, "microseconds": 517009}, \
   "event": "ENTROPY_NEEDED", "data": {"bytes": 64}}

Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
 Makefile.objs        |    1 +
 hw/pci.h             |    1 +
 hw/s390-virtio-bus.c |   35 +++++++++
 hw/s390-virtio-bus.h |    2 +
 hw/virtio-pci.c      |   51 +++++++++++++
 hw/virtio-pci.h      |    2 +
 hw/virtio-rng.c      |  199 ++++++++++++++++++++++++++++++++++++++++++++++++++
 hw/virtio-rng.h      |   24 ++++++
 hw/virtio.h          |    3 +
 monitor.c            |    3 +
 monitor.h            |    1 +
 11 files changed, 322 insertions(+), 0 deletions(-)
 create mode 100644 hw/virtio-rng.c
 create mode 100644 hw/virtio-rng.h

On 05/25/2012 02:32 PM, Amit Shah wrote:
> The Linux kernel already has a virtio-rng driver, this is the device
> implementation.
>
> When the guest asks for entropy from the virtio hwrng, it puts a buffer
> in the vq.  We then put entropy into that buffer, and push it back to
> the guest.
>
> The chardev connected to this device is fed the data to be sent to the
> guest.
>
> Invocation is simple:
>
>    $ qemu ... -device virtio-rng-pci,chardev=foo
>
> In the guest, we see
>
>    $ cat /sys/devices/virtual/misc/hw_random/rng_available
>    virtio
>
>    $ cat /sys/devices/virtual/misc/hw_random/rng_current
>    virtio
>
>    # cat /dev/hwrng
>
> Simply feeding /dev/urandom from the host to the chardev is sufficient:
>
>    $ qemu ... -chardev socket,path=/tmp/foo,server,nowait,id=foo \
>               -device virtio-rng,chardev=foo
>
>    $ nc -U /tmp/foo<  /dev/urandom
>
> A QMP event is sent for interested apps to monitor activity and send the
> appropriate number of bytes that get asked by the guest:
>
>    {"timestamp": {"seconds": 1337966878, "microseconds": 517009}, \
>     "event": "ENTROPY_NEEDED", "data": {"bytes": 64}}

I don't understand the point of this event.  Can't a management app just create 
a socket and then it can see all the requests the guest makes?

Regards,

Anthony Liguori

>
> Signed-off-by: Amit Shah<amit.shah@redhat.com>
> ---
>   Makefile.objs        |    1 +
>   hw/pci.h             |    1 +
>   hw/s390-virtio-bus.c |   35 +++++++++
>   hw/s390-virtio-bus.h |    2 +
>   hw/virtio-pci.c      |   51 +++++++++++++
>   hw/virtio-pci.h      |    2 +
>   hw/virtio-rng.c      |  199 ++++++++++++++++++++++++++++++++++++++++++++++++++
>   hw/virtio-rng.h      |   24 ++++++
>   hw/virtio.h          |    3 +
>   monitor.c            |    3 +
>   monitor.h            |    1 +
>   11 files changed, 322 insertions(+), 0 deletions(-)
>   create mode 100644 hw/virtio-rng.c
>   create mode 100644 hw/virtio-rng.h
>
> diff --git a/Makefile.objs b/Makefile.objs
> index 70c5c79..5850762 100644
> --- a/Makefile.objs
> +++ b/Makefile.objs
> @@ -210,6 +210,7 @@ user-obj-y += $(qom-obj-twice-y)
>   hw-obj-y =
>   hw-obj-y += vl.o loader.o
>   hw-obj-$(CONFIG_VIRTIO) += virtio-console.o
> +hw-obj-$(CONFIG_VIRTIO) += virtio-rng.o
>   hw-obj-y += usb/libhw.o
>   hw-obj-$(CONFIG_VIRTIO_PCI) += virtio-pci.o
>   hw-obj-y += fw_cfg.o
> diff --git a/hw/pci.h b/hw/pci.h
> index 8d0aa49..0a22f91 100644
> --- a/hw/pci.h
> +++ b/hw/pci.h
> @@ -76,6 +76,7 @@
>   #define PCI_DEVICE_ID_VIRTIO_BALLOON     0x1002
>   #define PCI_DEVICE_ID_VIRTIO_CONSOLE     0x1003
>   #define PCI_DEVICE_ID_VIRTIO_SCSI        0x1004
> +#define PCI_DEVICE_ID_VIRTIO_RNG         0x1005
>
>   #define FMT_PCIBUS                      PRIx64
>
> diff --git a/hw/s390-virtio-bus.c b/hw/s390-virtio-bus.c
> index 1d38a8f..e75711e 100644
> --- a/hw/s390-virtio-bus.c
> +++ b/hw/s390-virtio-bus.c
> @@ -26,6 +26,7 @@
>   #include "loader.h"
>   #include "elf.h"
>   #include "hw/virtio.h"
> +#include "hw/virtio-rng.h"
>   #include "hw/virtio-serial.h"
>   #include "hw/virtio-net.h"
>   #include "hw/sysbus.h"
> @@ -204,6 +205,18 @@ static int s390_virtio_scsi_init(VirtIOS390Device *dev)
>       return s390_virtio_device_init(dev, vdev);
>   }
>
> +static int s390_virtio_rng_init(VirtIOS390Device *dev)
> +{
> +    VirtIODevice *vdev;
> +
> +    vdev = virtio_rng_init((DeviceState *)dev,&dev->rng);
> +    if (!vdev) {
> +        return -1;
> +    }
> +
> +    return s390_virtio_device_init(dev, vdev);
> +}
> +
>   static uint64_t s390_virtio_device_vq_token(VirtIOS390Device *dev, int vq)
>   {
>       ram_addr_t token_off;
> @@ -445,6 +458,27 @@ static TypeInfo s390_virtio_serial = {
>       .class_init    = s390_virtio_serial_class_init,
>   };
>
> +static Property s390_virtio_rng_properties[] = {
> +    DEFINE_PROP_CHR("chardev", VirtIOS390Device, rng.chr),
> +    DEFINE_PROP_END_OF_LIST(),
> +};
> +
> +static void s390_virtio_rng_class_init(ObjectClass *klass, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(klass);
> +    VirtIOS390DeviceClass *k = VIRTIO_S390_DEVICE_CLASS(klass);
> +
> +    k->init = s390_virtio_rng_init;
> +    dc->props = s390_virtio_rng_properties;
> +}
> +
> +static TypeInfo s390_virtio_rng = {
> +    .name          = "virtio-rng-s390",
> +    .parent        = TYPE_VIRTIO_S390_DEVICE,
> +    .instance_size = sizeof(VirtIOS390Device),
> +    .class_init    = s390_virtio_rng_class_init,
> +};
> +
>   static int s390_virtio_busdev_init(DeviceState *dev)
>   {
>       VirtIOS390Device *_dev = (VirtIOS390Device *)dev;
> @@ -524,6 +558,7 @@ static void s390_virtio_register_types(void)
>       type_register_static(&s390_virtio_blk);
>       type_register_static(&s390_virtio_net);
>       type_register_static(&s390_virtio_scsi);
> +    type_register_static(&s390_virtio_rng);
>       type_register_static(&s390_virtio_bridge_info);
>   }
>
> diff --git a/hw/s390-virtio-bus.h b/hw/s390-virtio-bus.h
> index 4b99d02..5fc53b3 100644
> --- a/hw/s390-virtio-bus.h
> +++ b/hw/s390-virtio-bus.h
> @@ -19,6 +19,7 @@
>
>   #include "virtio-blk.h"
>   #include "virtio-net.h"
> +#include "virtio-rng.h"
>   #include "virtio-serial.h"
>   #include "virtio-scsi.h"
>
> @@ -71,6 +72,7 @@ struct VirtIOS390Device {
>       virtio_serial_conf serial;
>       virtio_net_conf net;
>       VirtIOSCSIConf scsi;
> +    VirtIORNGConf rng;
>   };
>
>   typedef struct VirtIOS390Bus {
> diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c
> index 79b86f1..bd5851e 100644
> --- a/hw/virtio-pci.c
> +++ b/hw/virtio-pci.c
> @@ -810,6 +810,28 @@ static int virtio_balloon_exit_pci(PCIDevice *pci_dev)
>       return virtio_exit_pci(pci_dev);
>   }
>
> +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);
> +    if (!vdev) {
> +        return -1;
> +    }
> +    virtio_init_pci(proxy, vdev);
> +    return 0;
> +}
> +
> +static int virtio_rng_exit_pci(PCIDevice *pci_dev)
> +{
> +    VirtIOPCIProxy *proxy = DO_UPCAST(VirtIOPCIProxy, pci_dev, pci_dev);
> +
> +    virtio_pci_stop_ioeventfd(proxy);
> +    virtio_rng_exit(proxy->vdev);
> +    return virtio_exit_pci(pci_dev);
> +}
> +
>   static Property virtio_blk_properties[] = {
>       DEFINE_PROP_HEX32("class", VirtIOPCIProxy, class_code, 0),
>       DEFINE_BLOCK_PROPERTIES(VirtIOPCIProxy, blk.conf),
> @@ -938,6 +960,34 @@ static TypeInfo virtio_balloon_info = {
>       .class_init    = virtio_balloon_class_init,
>   };
>
> +static Property virtio_rng_properties[] = {
> +    DEFINE_VIRTIO_COMMON_FEATURES(VirtIOPCIProxy, host_features),
> +    DEFINE_PROP_CHR("chardev", VirtIOPCIProxy, rng.chr),
> +    DEFINE_PROP_END_OF_LIST(),
> +};
> +
> +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;
> +    dc->reset = virtio_pci_reset;
> +    dc->props = virtio_rng_properties;
> +}
> +
> +static TypeInfo virtio_rng_info = {
> +    .name          = "virtio-rng-pci",
> +    .parent        = TYPE_PCI_DEVICE,
> +    .instance_size = sizeof(VirtIOPCIProxy),
> +    .class_init    = virtio_rng_class_init,
> +};
> +
>   static int virtio_scsi_init_pci(PCIDevice *pci_dev)
>   {
>       VirtIOPCIProxy *proxy = DO_UPCAST(VirtIOPCIProxy, pci_dev, pci_dev);
> @@ -999,6 +1049,7 @@ static void virtio_pci_register_types(void)
>       type_register_static(&virtio_serial_info);
>       type_register_static(&virtio_balloon_info);
>       type_register_static(&virtio_scsi_info);
> +    type_register_static(&virtio_rng_info);
>   }
>
>   type_init(virtio_pci_register_types)
> diff --git a/hw/virtio-pci.h b/hw/virtio-pci.h
> index 889e59e..cc3f828 100644
> --- a/hw/virtio-pci.h
> +++ b/hw/virtio-pci.h
> @@ -17,6 +17,7 @@
>
>   #include "virtio-blk.h"
>   #include "virtio-net.h"
> +#include "virtio-rng.h"
>   #include "virtio-serial.h"
>   #include "virtio-scsi.h"
>
> @@ -42,6 +43,7 @@ typedef struct {
>       virtio_serial_conf serial;
>       virtio_net_conf net;
>       VirtIOSCSIConf scsi;
> +    VirtIORNGConf rng;
>       bool ioeventfd_disabled;
>       bool ioeventfd_started;
>   } VirtIOPCIProxy;
> diff --git a/hw/virtio-rng.c b/hw/virtio-rng.c
> new file mode 100644
> index 0000000..f4f9078
> --- /dev/null
> +++ b/hw/virtio-rng.c
> @@ -0,0 +1,199 @@
> +/*
> + * A virtio device implementing a hardware random number generator.
> + *
> + * Copyright 2012 Red Hat, Inc.
> + * Copyright 2012 Amit Shah<amit.shah@redhat.com>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or
> + * (at your option) any later version.  See the COPYING file in the
> + * top-level directory.
> + */
> +
> +#include "iov.h"
> +#include "qdev.h"
> +#include "qjson.h"
> +#include "virtio.h"
> +#include "virtio-rng.h"
> +
> +typedef struct VirtIORNG {
> +    VirtIODevice vdev;
> +
> +    DeviceState *qdev;
> +
> +    /* 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;
> +} VirtIORNG;
> +
> +static bool is_guest_ready(VirtIORNG *vrng)
> +{
> +    if (virtio_queue_ready(vrng->vq)
> +&&  (vrng->vdev.status&  VIRTIO_CONFIG_S_DRIVER_OK)) {
> +        return true;
> +    }
> +    return false;
> +}
> +
> +static void send_qevent_for_entropy(size_t size)
> +{
> +    QObject *data;
> +
> +    data = qobject_from_jsonf("{ 'bytes': %" PRId64 " }",
> +                              size);
> +    monitor_protocol_event(QEVENT_ENTROPY_NEEDED, data);
> +    qobject_decref(data);
> +}
> +
> +static size_t pop_an_elem(VirtIORNG *vrng)
> +{
> +    size_t size;
> +
> +    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;
> +}
> +
> +static void handle_input(VirtIODevice *vdev, VirtQueue *vq)
> +{
> +    VirtIORNG *vrng = DO_UPCAST(VirtIORNG, vdev, vdev);
> +    size_t size;
> +
> +    size = pop_an_elem(vrng);
> +    if (size) {
> +        send_qevent_for_entropy(size);
> +    }
> +}
> +
> +static int chr_can_read(void *opaque)
> +{
> +    VirtIORNG *vrng = opaque;
> +
> +    if (!is_guest_ready(vrng)) {
> +        return 0;
> +    }
> +    return pop_an_elem(vrng);
> +}
> +
> +/* Send data from a char device over to the guest */
> +static void chr_read(void *opaque, const uint8_t *buf, int 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.  We can't stick this into chr_can_read(), as it'll just
> +     * end up spamming the management app.
> +     */
> +    len = pop_an_elem(vrng);
> +    if (len) {
> +        send_qevent_for_entropy(len);
> +    }
> +}
> +
> +static uint32_t get_features(VirtIODevice *vdev, uint32_t f)
> +{
> +    return f;
> +}
> +
> +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) {
> +        qemu_put_buffer(f, (unsigned char *)&vrng->elem,
> +                        sizeof(vrng->elem));
> +    }
> +}
> +
> +static int virtio_rng_load(QEMUFile *f, void *opaque, int version_id)
> +{
> +    VirtIORNG *vrng = opaque;
> +
> +    if (version_id != 1) {
> +        return -EINVAL;
> +    }
> +    virtio_load(&vrng->vdev, f);
> +
> +    vrng->popped = qemu_get_byte(f);
> +    if (vrng->popped) {
> +        qemu_get_buffer(f, (unsigned char *)&vrng->elem,
> +                        sizeof(vrng->elem));
> +        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;
> +}
> +
> +VirtIODevice *virtio_rng_init(DeviceState *dev, VirtIORNGConf *conf)
> +{
> +    VirtIORNG *vrng;
> +    VirtIODevice *vdev;
> +
> +    if (!conf->chr) {
> +        error_report("chardev property expected");
> +        return NULL;
> +    }
> +
> +    vdev = virtio_common_init("virtio-rng", VIRTIO_ID_RNG, 0,
> +                              sizeof(VirtIORNG));
> +
> +    vrng = DO_UPCAST(VirtIORNG, vdev, vdev);
> +
> +    vrng->vq = virtio_add_queue(vdev, 8, handle_input);
> +    vrng->vdev.get_features = get_features;
> +
> +    vrng->qdev = dev;
> +    vrng->conf = conf;
> +    vrng->popped = false;
> +    register_savevm(dev, "virtio-rng", -1, 1, virtio_rng_save,
> +                    virtio_rng_load, vrng);
> +
> +    qemu_chr_add_handlers(conf->chr, chr_can_read, chr_read, NULL, vrng);
> +
> +    return vdev;
> +}
> +
> +void virtio_rng_exit(VirtIODevice *vdev)
> +{
> +    VirtIORNG *vrng = DO_UPCAST(VirtIORNG, vdev, vdev);
> +
> +    qemu_chr_add_handlers(vrng->conf->chr, NULL, NULL, NULL, NULL);
> +    unregister_savevm(vrng->qdev, "virtio-rng", vrng);
> +    virtio_cleanup(vdev);
> +}
> diff --git a/hw/virtio-rng.h b/hw/virtio-rng.h
> new file mode 100644
> index 0000000..b132acd
> --- /dev/null
> +++ b/hw/virtio-rng.h
> @@ -0,0 +1,24 @@
> +/*
> + * Virtio RNG Support
> + *
> + * Copyright Red Hat, Inc. 2012
> + * Copyright Amit Shah<amit.shah@redhat.com>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or
> + * (at your option) any later version.  See the COPYING file in the
> + * top-level directory.
> + */
> +
> +#ifndef _QEMU_VIRTIO_RNG_H
> +#define _QEMU_VIRTIO_RNG_H
> +
> +#include "qemu-char.h"
> +
> +/* The Virtio ID for the virtio rng device */
> +#define VIRTIO_ID_RNG    4
> +
> +struct VirtIORNGConf {
> +    CharDriverState *chr;
> +};
> +
> +#endif
> diff --git a/hw/virtio.h b/hw/virtio.h
> index 85aabe5..b4b5bf6 100644
> --- a/hw/virtio.h
> +++ b/hw/virtio.h
> @@ -201,6 +201,8 @@ VirtIODevice *virtio_serial_init(DeviceState *dev, virtio_serial_conf *serial);
>   VirtIODevice *virtio_balloon_init(DeviceState *dev);
>   typedef struct VirtIOSCSIConf VirtIOSCSIConf;
>   VirtIODevice *virtio_scsi_init(DeviceState *dev, VirtIOSCSIConf *conf);
> +typedef struct VirtIORNGConf VirtIORNGConf;
> +VirtIODevice *virtio_rng_init(DeviceState *dev, VirtIORNGConf *conf);
>   #ifdef CONFIG_LINUX
>   VirtIODevice *virtio_9p_init(DeviceState *dev, V9fsConf *conf);
>   #endif
> @@ -211,6 +213,7 @@ void virtio_blk_exit(VirtIODevice *vdev);
>   void virtio_serial_exit(VirtIODevice *vdev);
>   void virtio_balloon_exit(VirtIODevice *vdev);
>   void virtio_scsi_exit(VirtIODevice *vdev);
> +void virtio_rng_exit(VirtIODevice *vdev);
>
>   #define DEFINE_VIRTIO_COMMON_FEATURES(_state, _field) \
>   	DEFINE_PROP_BIT("indirect_desc", _state, _field, \
> diff --git a/monitor.c b/monitor.c
> index 12a6fe2..177b73e 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -493,6 +493,9 @@ void monitor_protocol_event(MonitorEvent event, QObject *data)
>           case QEVENT_WAKEUP:
>               event_name = "WAKEUP";
>               break;
> +        case QEVENT_ENTROPY_NEEDED:
> +            event_name = "ENTROPY_NEEDED";
> +            break;
>           default:
>               abort();
>               break;
> diff --git a/monitor.h b/monitor.h
> index 0d49800..731d599 100644
> --- a/monitor.h
> +++ b/monitor.h
> @@ -41,6 +41,7 @@ typedef enum MonitorEvent {
>       QEVENT_DEVICE_TRAY_MOVED,
>       QEVENT_SUSPEND,
>       QEVENT_WAKEUP,
> +    QEVENT_ENTROPY_NEEDED,
>       QEVENT_MAX,
>   } MonitorEvent;
>

On (Fri) 25 May 2012 [15:00:53], Anthony Liguori wrote:
> On 05/25/2012 02:32 PM, Amit Shah wrote:
> >The Linux kernel already has a virtio-rng driver, this is the device
> >implementation.
> >
> >When the guest asks for entropy from the virtio hwrng, it puts a buffer
> >in the vq.  We then put entropy into that buffer, and push it back to
> >the guest.
> >
> >The chardev connected to this device is fed the data to be sent to the
> >guest.
> >
> >Invocation is simple:
> >
> >   $ qemu ... -device virtio-rng-pci,chardev=foo
> >
> >In the guest, we see
> >
> >   $ cat /sys/devices/virtual/misc/hw_random/rng_available
> >   virtio
> >
> >   $ cat /sys/devices/virtual/misc/hw_random/rng_current
> >   virtio
> >
> >   # cat /dev/hwrng
> >
> >Simply feeding /dev/urandom from the host to the chardev is sufficient:
> >
> >   $ qemu ... -chardev socket,path=/tmp/foo,server,nowait,id=foo \
> >              -device virtio-rng,chardev=foo
> >
> >   $ nc -U /tmp/foo<  /dev/urandom
> >
> >A QMP event is sent for interested apps to monitor activity and send the
> >appropriate number of bytes that get asked by the guest:
> >
> >   {"timestamp": {"seconds": 1337966878, "microseconds": 517009}, \
> >    "event": "ENTROPY_NEEDED", "data": {"bytes": 64}}
> 
> I don't understand the point of this event.  Can't a management app
> just create a socket and then it can see all the requests the guest
> makes?

How?  With the chardev, it can only keep feeding data, and that data
will be consumed when chr_can_read() returns > 0.  And even then the
mgmt app has no idea how much data was asked for, and how much was
consumed.

		Amit

On Sat, May 26, 2012 at 01:02:49AM +0530, Amit Shah wrote:
> The Linux kernel already has a virtio-rng driver, this is the device
> implementation.
> 
> When the guest asks for entropy from the virtio hwrng, it puts a buffer
> in the vq.  We then put entropy into that buffer, and push it back to
> the guest.
> 
> The chardev connected to this device is fed the data to be sent to the
> guest.
> 
> Invocation is simple:
> 
>   $ qemu ... -device virtio-rng-pci,chardev=foo
> 
> In the guest, we see
> 
>   $ cat /sys/devices/virtual/misc/hw_random/rng_available
>   virtio
> 
>   $ cat /sys/devices/virtual/misc/hw_random/rng_current
>   virtio
> 
>   # cat /dev/hwrng
> 
> Simply feeding /dev/urandom from the host to the chardev is sufficient:
> 
>   $ qemu ... -chardev socket,path=/tmp/foo,server,nowait,id=foo \
>              -device virtio-rng,chardev=foo
> 
>   $ nc -U /tmp/foo < /dev/urandom

ACK to this ARGV design from a libvirt POV.

> A QMP event is sent for interested apps to monitor activity and send the
> appropriate number of bytes that get asked by the guest:
> 
>   {"timestamp": {"seconds": 1337966878, "microseconds": 517009}, \
>    "event": "ENTROPY_NEEDED", "data": {"bytes": 64}}

IIUC, there are three ways mgmt apps can use the RNG with the
chardev

 - Wire it up to a source that just blindly provides all the
   entropy QEMU desires (as you /dev/urandom example above)

 - Feed in a fixed amount of entropy every minute, regardless
   of how much QEMU desires

 - Feed in entropy on demand, in response to the ENTROPY_NEEDED
   event notification (possibly throttling)

These options sounds like they should cover all reasonable needs,
so gets my vote. Probably want to include the ENTROPY_NEEDED
event in my patch which adds rate limiting to guest initiated
events.

Daniel

On (Mon) 28 May 2012 [09:33:57], Daniel P. Berrange wrote:
> On Sat, May 26, 2012 at 01:02:49AM +0530, Amit Shah wrote:
> > The Linux kernel already has a virtio-rng driver, this is the device
> > implementation.
> > 
> > When the guest asks for entropy from the virtio hwrng, it puts a buffer
> > in the vq.  We then put entropy into that buffer, and push it back to
> > the guest.
> > 
> > The chardev connected to this device is fed the data to be sent to the
> > guest.
> > 
> > Invocation is simple:
> > 
> >   $ qemu ... -device virtio-rng-pci,chardev=foo
> > 
> > In the guest, we see
> > 
> >   $ cat /sys/devices/virtual/misc/hw_random/rng_available
> >   virtio
> > 
> >   $ cat /sys/devices/virtual/misc/hw_random/rng_current
> >   virtio
> > 
> >   # cat /dev/hwrng
> > 
> > Simply feeding /dev/urandom from the host to the chardev is sufficient:
> > 
> >   $ qemu ... -chardev socket,path=/tmp/foo,server,nowait,id=foo \
> >              -device virtio-rng,chardev=foo
> > 
> >   $ nc -U /tmp/foo < /dev/urandom
> 
> ACK to this ARGV design from a libvirt POV.
> 
> > A QMP event is sent for interested apps to monitor activity and send the
> > appropriate number of bytes that get asked by the guest:
> > 
> >   {"timestamp": {"seconds": 1337966878, "microseconds": 517009}, \
> >    "event": "ENTROPY_NEEDED", "data": {"bytes": 64}}
> 
> IIUC, there are three ways mgmt apps can use the RNG with the
> chardev
> 
>  - Wire it up to a source that just blindly provides all the
>    entropy QEMU desires (as you /dev/urandom example above)
> 
>  - Feed in a fixed amount of entropy every minute, regardless
>    of how much QEMU desires

This option currently won't do anything -- i.e. till the guest sends
across a buffer to be filled in, nothing will go to the guest, and the
data will just be buffered in the chardev layer till such a buffer
comes along.  It can be debatable on feeding entropy by pushing every
particular timeout, or just providing the freshest on-demand.
Advantage could be we have more random bits, disadvantage is this
could be throttled as the host went out of enough entropy.

>  - Feed in entropy on demand, in response to the ENTROPY_NEEDED
>    event notification (possibly throttling)
> 
> These options sounds like they should cover all reasonable needs,
> so gets my vote.

Great!

> Probably want to include the ENTROPY_NEEDED
> event in my patch which adds rate limiting to guest initiated
> events.

Yes, just depends in which order the patches go in.

Thanks,
		Amit

On 05/26/2012 04:20 AM, Amit Shah wrote:
> On (Fri) 25 May 2012 [15:00:53], Anthony Liguori wrote:
>> On 05/25/2012 02:32 PM, Amit Shah wrote:
>>> The Linux kernel already has a virtio-rng driver, this is the device
>>> implementation.
>>>
>>> When the guest asks for entropy from the virtio hwrng, it puts a buffer
>>> in the vq.  We then put entropy into that buffer, and push it back to
>>> the guest.
>>>
>>> The chardev connected to this device is fed the data to be sent to the
>>> guest.
>>>
>>> Invocation is simple:
>>>
>>>    $ qemu ... -device virtio-rng-pci,chardev=foo
>>>
>>> In the guest, we see
>>>
>>>    $ cat /sys/devices/virtual/misc/hw_random/rng_available
>>>    virtio
>>>
>>>    $ cat /sys/devices/virtual/misc/hw_random/rng_current
>>>    virtio
>>>
>>>    # cat /dev/hwrng
>>>
>>> Simply feeding /dev/urandom from the host to the chardev is sufficient:
>>>
>>>    $ qemu ... -chardev socket,path=/tmp/foo,server,nowait,id=foo \
>>>               -device virtio-rng,chardev=foo
>>>
>>>    $ nc -U /tmp/foo<   /dev/urandom
>>>
>>> A QMP event is sent for interested apps to monitor activity and send the
>>> appropriate number of bytes that get asked by the guest:
>>>
>>>    {"timestamp": {"seconds": 1337966878, "microseconds": 517009}, \
>>>     "event": "ENTROPY_NEEDED", "data": {"bytes": 64}}
>>
>> I don't understand the point of this event.  Can't a management app
>> just create a socket and then it can see all the requests the guest
>> makes?
>
> How?  With the chardev, it can only keep feeding data, and that data
> will be consumed when chr_can_read() returns>  0.  And even then the
> mgmt app has no idea how much data was asked for, and how much was
> consumed.

Okay, then the right approach is to use a message protocol where QEMU asks for a 
certain amount of data and then the daemon sends it back.

I think this is pretty much why the egd protocol exists, no?  Why not just 
implement egd protocol support?

Once we introduce a protocol of any form (even raw), we have to support it 
forever so let's not do it carelessly.

Regards,

Anthony Liguori

>
> 		Amit
>
>

On (Mon) 04 Jun 2012 [19:04:41], Anthony Liguori wrote:
> On 05/26/2012 04:20 AM, Amit Shah wrote:
> >On (Fri) 25 May 2012 [15:00:53], Anthony Liguori wrote:
> >>On 05/25/2012 02:32 PM, Amit Shah wrote:
> >>>The Linux kernel already has a virtio-rng driver, this is the device
> >>>implementation.
> >>>
> >>>When the guest asks for entropy from the virtio hwrng, it puts a buffer
> >>>in the vq.  We then put entropy into that buffer, and push it back to
> >>>the guest.
> >>>
> >>>The chardev connected to this device is fed the data to be sent to the
> >>>guest.
> >>>
> >>>Invocation is simple:
> >>>
> >>>   $ qemu ... -device virtio-rng-pci,chardev=foo
> >>>
> >>>In the guest, we see
> >>>
> >>>   $ cat /sys/devices/virtual/misc/hw_random/rng_available
> >>>   virtio
> >>>
> >>>   $ cat /sys/devices/virtual/misc/hw_random/rng_current
> >>>   virtio
> >>>
> >>>   # cat /dev/hwrng
> >>>
> >>>Simply feeding /dev/urandom from the host to the chardev is sufficient:
> >>>
> >>>   $ qemu ... -chardev socket,path=/tmp/foo,server,nowait,id=foo \
> >>>              -device virtio-rng,chardev=foo
> >>>
> >>>   $ nc -U /tmp/foo<   /dev/urandom
> >>>
> >>>A QMP event is sent for interested apps to monitor activity and send the
> >>>appropriate number of bytes that get asked by the guest:
> >>>
> >>>   {"timestamp": {"seconds": 1337966878, "microseconds": 517009}, \
> >>>    "event": "ENTROPY_NEEDED", "data": {"bytes": 64}}
> >>
> >>I don't understand the point of this event.  Can't a management app
> >>just create a socket and then it can see all the requests the guest
> >>makes?
> >
> >How?  With the chardev, it can only keep feeding data, and that data
> >will be consumed when chr_can_read() returns>  0.  And even then the
> >mgmt app has no idea how much data was asked for, and how much was
> >consumed.
> 
> Okay, then the right approach is to use a message protocol where
> QEMU asks for a certain amount of data and then the daemon sends it
> back.

Why is a message protocol necessary?  And a daemon isn't necessary,
too.

> I think this is pretty much why the egd protocol exists, no?  Why
> not just implement egd protocol support?

Perhaps I'm not getting what you're saying.

What we have here, via the event, is to tell the mgmt app that the
guest has need for entropy.  The guest also tells us exactly how many
bytes it needs.  We're just passing on that info to the mgmt app, if
it needs it.

In any case, any data pushed down the chardev will be consumed by the
device, and sent to the guest when the guest puts a buf in the vq.

> Once we introduce a protocol of any form (even raw), we have to
> support it forever so let's not do it carelessly.

There's no need for a protocol at all here.  Can you skim through the
code and point out where you think we're disagreeing?

		Amit

On 06/05/2012 05:41 PM, Amit Shah wrote:
> On (Mon) 04 Jun 2012 [19:04:41], Anthony Liguori wrote:
>> On 05/26/2012 04:20 AM, Amit Shah wrote:
>>> On (Fri) 25 May 2012 [15:00:53], Anthony Liguori wrote:
>>>> On 05/25/2012 02:32 PM, Amit Shah wrote:
>>>>> The Linux kernel already has a virtio-rng driver, this is the device
>>>>> implementation.
>>>>>
>>>>> When the guest asks for entropy from the virtio hwrng, it puts a buffer
>>>>> in the vq.  We then put entropy into that buffer, and push it back to
>>>>> the guest.
>>>>>
>>>>> The chardev connected to this device is fed the data to be sent to the
>>>>> guest.
>>>>>
>>>>> Invocation is simple:
>>>>>
>>>>>    $ qemu ... -device virtio-rng-pci,chardev=foo
>>>>>
>>>>> In the guest, we see
>>>>>
>>>>>    $ cat /sys/devices/virtual/misc/hw_random/rng_available
>>>>>    virtio
>>>>>
>>>>>    $ cat /sys/devices/virtual/misc/hw_random/rng_current
>>>>>    virtio
>>>>>
>>>>>    # cat /dev/hwrng
>>>>>
>>>>> Simply feeding /dev/urandom from the host to the chardev is sufficient:
>>>>>
>>>>>    $ qemu ... -chardev socket,path=/tmp/foo,server,nowait,id=foo \
>>>>>               -device virtio-rng,chardev=foo
>>>>>
>>>>>    $ nc -U /tmp/foo<    /dev/urandom
>>>>>
>>>>> A QMP event is sent for interested apps to monitor activity and send the
>>>>> appropriate number of bytes that get asked by the guest:
>>>>>
>>>>>    {"timestamp": {"seconds": 1337966878, "microseconds": 517009}, \
>>>>>     "event": "ENTROPY_NEEDED", "data": {"bytes": 64}}
>>>>
>>>> I don't understand the point of this event.  Can't a management app
>>>> just create a socket and then it can see all the requests the guest
>>>> makes?
>>>
>>> How?  With the chardev, it can only keep feeding data, and that data
>>> will be consumed when chr_can_read() returns>   0.  And even then the
>>> mgmt app has no idea how much data was asked for, and how much was
>>> consumed.
>>
>> Okay, then the right approach is to use a message protocol where
>> QEMU asks for a certain amount of data and then the daemon sends it
>> back.
>
> Why is a message protocol necessary?  And a daemon isn't necessary,
> too.
>
>> I think this is pretty much why the egd protocol exists, no?  Why
>> not just implement egd protocol support?
>
> Perhaps I'm not getting what you're saying.
>
> What we have here, via the event, is to tell the mgmt app that the
> guest has need for entropy.

*out-of-band*

Why not just make that all in-band?

I understand you're trying to accomodate a use-case of feeding /dev/random to 
QEMU.  I don't think this is a necessary use-case though because I don't think 
it's correct.  You need to use an entropy daemon of some sort to make sure 
entropy is being distributed in a fair fashion.

> The guest also tells us exactly how many
> bytes it needs.  We're just passing on that info to the mgmt app, if
> it needs it.
>
> In any case, any data pushed down the chardev will be consumed by the
> device, and sent to the guest when the guest puts a buf in the vq.
>
>> Once we introduce a protocol of any form (even raw), we have to
>> support it forever so let's not do it carelessly.
>
> There's no need for a protocol at all here.

The event *is* a protocol.  That's the point.

Here's what you're making a management app do:

1) Open a socket
2) Feed socket fd to QEMU
3) Listen for event on QMP for entropy request
4) Read entropy based on (3) from /dev/random
5) Send entropy to socket to QEMU

This is pretty silly when you can just do:

1) Open a socket
2) Listen for entropy request
3) Read entry based on (2) from /dev/random
4) Respond to (2) with (3)

And there's already a tool and protocol for exactly the above--egd.

There's simply no good reason to involve QMP here.

Regards,

Anthony Liguori

   Can you skim through the
> code and point out where you think we're disagreeing?
>
> 		Amit

On (Tue) 05 Jun 2012 [17:54:30], Anthony Liguori wrote:
> On 06/05/2012 05:41 PM, Amit Shah wrote:
> >On (Mon) 04 Jun 2012 [19:04:41], Anthony Liguori wrote:
> >>On 05/26/2012 04:20 AM, Amit Shah wrote:
> >>>On (Fri) 25 May 2012 [15:00:53], Anthony Liguori wrote:
> >>>>On 05/25/2012 02:32 PM, Amit Shah wrote:
> >>>>>The Linux kernel already has a virtio-rng driver, this is the device
> >>>>>implementation.
> >>>>>
> >>>>>When the guest asks for entropy from the virtio hwrng, it puts a buffer
> >>>>>in the vq.  We then put entropy into that buffer, and push it back to
> >>>>>the guest.
> >>>>>
> >>>>>The chardev connected to this device is fed the data to be sent to the
> >>>>>guest.
> >>>>>
> >>>>>Invocation is simple:
> >>>>>
> >>>>>   $ qemu ... -device virtio-rng-pci,chardev=foo
> >>>>>
> >>>>>In the guest, we see
> >>>>>
> >>>>>   $ cat /sys/devices/virtual/misc/hw_random/rng_available
> >>>>>   virtio
> >>>>>
> >>>>>   $ cat /sys/devices/virtual/misc/hw_random/rng_current
> >>>>>   virtio
> >>>>>
> >>>>>   # cat /dev/hwrng
> >>>>>
> >>>>>Simply feeding /dev/urandom from the host to the chardev is sufficient:
> >>>>>
> >>>>>   $ qemu ... -chardev socket,path=/tmp/foo,server,nowait,id=foo \
> >>>>>              -device virtio-rng,chardev=foo
> >>>>>
> >>>>>   $ nc -U /tmp/foo<    /dev/urandom
> >>>>>
> >>>>>A QMP event is sent for interested apps to monitor activity and send the
> >>>>>appropriate number of bytes that get asked by the guest:
> >>>>>
> >>>>>   {"timestamp": {"seconds": 1337966878, "microseconds": 517009}, \
> >>>>>    "event": "ENTROPY_NEEDED", "data": {"bytes": 64}}
> >>>>
> >>>>I don't understand the point of this event.  Can't a management app
> >>>>just create a socket and then it can see all the requests the guest
> >>>>makes?
> >>>
> >>>How?  With the chardev, it can only keep feeding data, and that data
> >>>will be consumed when chr_can_read() returns>   0.  And even then the
> >>>mgmt app has no idea how much data was asked for, and how much was
> >>>consumed.
> >>
> >>Okay, then the right approach is to use a message protocol where
> >>QEMU asks for a certain amount of data and then the daemon sends it
> >>back.
> >
> >Why is a message protocol necessary?  And a daemon isn't necessary,
> >too.
> >
> >>I think this is pretty much why the egd protocol exists, no?  Why
> >>not just implement egd protocol support?
> >
> >Perhaps I'm not getting what you're saying.
> >
> >What we have here, via the event, is to tell the mgmt app that the
> >guest has need for entropy.
> 
> *out-of-band*
> 
> Why not just make that all in-band?
> 
> I understand you're trying to accomodate a use-case of feeding
> /dev/random to QEMU.  I don't think this is a necessary use-case
> though because I don't think it's correct.  You need to use an
> entropy daemon of some sort to make sure entropy is being
> distributed in a fair fashion.

In my view, qemu doesn't operate at a system-level where it can see
how system entropy is being used.

libvirt does operate at that level.

For casual development, I don't want to have egd on my system to feed
in entropy to the guest, and I don't need to know how to configure
egd and then provide its output to qemu.

libvirt can do that for me.  But for development, I'd just want to be
able to do something like

qemu -chardev socket,path=/tmp/foo,id=foo,server,nowait \
     -device virtio-rng,chardev=foo

and then just feed in some data to /tmp/foo.

> >The guest also tells us exactly how many
> >bytes it needs.  We're just passing on that info to the mgmt app, if
> >it needs it.
> >
> >In any case, any data pushed down the chardev will be consumed by the
> >device, and sent to the guest when the guest puts a buf in the vq.
> >
> >>Once we introduce a protocol of any form (even raw), we have to
> >>support it forever so let's not do it carelessly.
> >
> >There's no need for a protocol at all here.
> 
> The event *is* a protocol.  That's the point.

But there is no requirement to use the event.  From the cmdline above,
I can just do something like

nc -U /tmp/foo < /dev/urandom

and forget about the event.  The event is just a way to tell
interested apps what the guest did.  Nothing more.

> Here's what you're making a management app do:
> 
> 1) Open a socket
> 2) Feed socket fd to QEMU
> 3) Listen for event on QMP for entropy request

This step isn't necessary.

> 4) Read entropy based on (3) from /dev/random
> 5) Send entropy to socket to QEMU
> 
> This is pretty silly when you can just do:
> 
> 1) Open a socket
> 2) Listen for entropy request
> 3) Read entry based on (2) from /dev/random
> 4) Respond to (2) with (3)

Quoting danpb's mail from this thread:

> IIUC, there are three ways mgmt apps can use the RNG with the
> chardev
> 
>  - Wire it up to a source that just blindly provides all the
>    entropy QEMU desires (as you /dev/urandom example above)
> 
>  - Feed in a fixed amount of entropy every minute, regardless
>    of how much QEMU desires
> 
>  - Feed in entropy on demand, in response to the ENTROPY_NEEDED
>    event notification (possibly throttling)
> 
> These options sounds like they should cover all reasonable needs,
> so gets my vote. Probably want to include the ENTROPY_NEEDED
> event in my patch which adds rate limiting to guest initiated
> events.

		Amit

On Tue, Jun 05, 2012 at 03:46:31PM +0530, Amit Shah wrote:
> On (Tue) 05 Jun 2012 [17:54:30], Anthony Liguori wrote:
> > On 06/05/2012 05:41 PM, Amit Shah wrote:
> > >On (Mon) 04 Jun 2012 [19:04:41], Anthony Liguori wrote:
> > >>On 05/26/2012 04:20 AM, Amit Shah wrote:
> > >>>On (Fri) 25 May 2012 [15:00:53], Anthony Liguori wrote:
> > >>>>On 05/25/2012 02:32 PM, Amit Shah wrote:
> > >>>>>The Linux kernel already has a virtio-rng driver, this is the device
> > >>>>>implementation.
> > >>>>>
> > >>>>>When the guest asks for entropy from the virtio hwrng, it puts a buffer
> > >>>>>in the vq.  We then put entropy into that buffer, and push it back to
> > >>>>>the guest.
> > >>>>>
> > >>>>>The chardev connected to this device is fed the data to be sent to the
> > >>>>>guest.
> > >>>>>
> > >>>>>Invocation is simple:
> > >>>>>
> > >>>>>   $ qemu ... -device virtio-rng-pci,chardev=foo
> > >>>>>
> > >>>>>In the guest, we see
> > >>>>>
> > >>>>>   $ cat /sys/devices/virtual/misc/hw_random/rng_available
> > >>>>>   virtio
> > >>>>>
> > >>>>>   $ cat /sys/devices/virtual/misc/hw_random/rng_current
> > >>>>>   virtio
> > >>>>>
> > >>>>>   # cat /dev/hwrng
> > >>>>>
> > >>>>>Simply feeding /dev/urandom from the host to the chardev is sufficient:
> > >>>>>
> > >>>>>   $ qemu ... -chardev socket,path=/tmp/foo,server,nowait,id=foo \
> > >>>>>              -device virtio-rng,chardev=foo
> > >>>>>
> > >>>>>   $ nc -U /tmp/foo<    /dev/urandom
> > >>>>>
> > >>>>>A QMP event is sent for interested apps to monitor activity and send the
> > >>>>>appropriate number of bytes that get asked by the guest:
> > >>>>>
> > >>>>>   {"timestamp": {"seconds": 1337966878, "microseconds": 517009}, \
> > >>>>>    "event": "ENTROPY_NEEDED", "data": {"bytes": 64}}
> > >>>>
> > >>>>I don't understand the point of this event.  Can't a management app
> > >>>>just create a socket and then it can see all the requests the guest
> > >>>>makes?
> > >>>
> > >>>How?  With the chardev, it can only keep feeding data, and that data
> > >>>will be consumed when chr_can_read() returns>   0.  And even then the
> > >>>mgmt app has no idea how much data was asked for, and how much was
> > >>>consumed.
> > >>
> > >>Okay, then the right approach is to use a message protocol where
> > >>QEMU asks for a certain amount of data and then the daemon sends it
> > >>back.
> > >
> > >Why is a message protocol necessary?  And a daemon isn't necessary,
> > >too.
> > >
> > >>I think this is pretty much why the egd protocol exists, no?  Why
> > >>not just implement egd protocol support?
> > >
> > >Perhaps I'm not getting what you're saying.
> > >
> > >What we have here, via the event, is to tell the mgmt app that the
> > >guest has need for entropy.
> > 
> > *out-of-band*
> > 
> > Why not just make that all in-band?
> > 
> > I understand you're trying to accomodate a use-case of feeding
> > /dev/random to QEMU.  I don't think this is a necessary use-case
> > though because I don't think it's correct.  You need to use an
> > entropy daemon of some sort to make sure entropy is being
> > distributed in a fair fashion.
> 
> In my view, qemu doesn't operate at a system-level where it can see
> how system entropy is being used.
> 
> libvirt does operate at that level.
> 
> For casual development, I don't want to have egd on my system to feed
> in entropy to the guest, and I don't need to know how to configure
> egd and then provide its output to qemu.
> 
> libvirt can do that for me.  But for development, I'd just want to be
> able to do something like
> 
> qemu -chardev socket,path=/tmp/foo,id=foo,server,nowait \
>      -device virtio-rng,chardev=foo
> 
> and then just feed in some data to /tmp/foo.

I tend to agree, while the EGD wire protocol may be useful if you
are wanting to connect QEMU to EGD, for apps that are just directly
mnanaging QEMU it imposes a unnecessary burden on apps, to learn
a new protocol in addition to their existing knowledge of QMP.

In addition, I don't think implementing the EGD protocol is mutually
exclusive with raw QEMU chardevs. eg enablement of an EGD protocol on
the chardev could reasonably be done as an optional feature ontop of
the raw chardev, in the same way that chardevs can be made to optionally
run the telnet line discipline.

I'd like to see us have the raw chardev impl + QMP event as a first
step, and then add EGD protocol on top of that (if desired) as a
second step.

Regards,
Daniel