[Qemu-devel] [PATCH 0/6] add paravirtualization hwrng support

[Qemu-devel] [PATCH 0/6] add paravirtualization hwrng support

[Anthony Liguori]

Hi,

This series implements the backend and frontend infrastructure for virtio-rng.
This is similar to previous series sent out by both Amit and myself although it
has been trimmed down considerably.

In terms of backends, a file and EGD backend are supported.  The file defaults
to /dev/random based on the feedback from Peter.  It's still possible to support
/dev/urandom though as an entropy source by overriding the file name.

I think this series is ready to merge.

[Qemu-devel] [PATCH 1/6] vl: add -object option to create QOM objects from the command line

[Anthony Liguori]

This will create a new QOM object in the '/objects' path.  Note that properties
are set in order which allows for simple objects to be initialized entirely
with this option and then realized.

This option is roughly equivalent to -device but for things that are not
devices.

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
---
 qemu-config.c   | 10 ++++++++++
 qemu-options.hx |  8 ++++++++
 vl.c            | 56 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 74 insertions(+)

diff --git a/qemu-config.c b/qemu-config.c
index cd1ec21..feb3107 100644
--- a/qemu-config.c
+++ b/qemu-config.c
@@ -653,6 +653,15 @@ QemuOptsList qemu_boot_opts = {
     },
 };
 
+static QemuOptsList qemu_object_opts = {
+    .name = "object",
+    .implied_opt_name = "qom-type",
+    .head = QTAILQ_HEAD_INITIALIZER(qemu_object_opts.head),
+    .desc = {
+        { }
+    },
+};
+
 static QemuOptsList *vm_config_groups[32] = {
     &qemu_drive_opts,
     &qemu_chardev_opts,
@@ -669,6 +678,7 @@ static QemuOptsList *vm_config_groups[32] = {
     &qemu_boot_opts,
     &qemu_iscsi_opts,
     &qemu_sandbox_opts,
+    &qemu_object_opts,
     NULL,
 };
 
diff --git a/qemu-options.hx b/qemu-options.hx
index 46f0539..b72151e 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -2852,6 +2852,14 @@ STEXI
 Enable FIPS 140-2 compliance mode.
 ETEXI
 
+DEF("object", HAS_ARG, QEMU_OPTION_object,
+    "-object TYPENAME[,PROP1=VALUE1,...]\n"
+    "                create an new object of type TYPENAME setting properties\n"
+    "                in the order they are specified.  Note that the 'id'\n"
+    "                property must be set.  These objects are placed in the\n"
+    "                '/objects' path.\n",
+    QEMU_ARCH_ALL)
+
 HXCOMM This is the last statement. Insert new options before this line!
 STEXI
 @end table
diff --git a/vl.c b/vl.c
index ee3c43a..ffadbb6 100644
--- a/vl.c
+++ b/vl.c
@@ -168,6 +168,7 @@ int main(int argc, char **argv)
 #include "osdep.h"
 
 #include "ui/qemu-spice.h"
+#include "qapi/string-input-visitor.h"
 
 //#define DEBUG_NET
 //#define DEBUG_SLIRP
@@ -2357,6 +2358,53 @@ static void free_and_trace(gpointer mem)
     free(mem);
 }
 
+static int object_set_property(const char *name, const char *value, void *opaque)
+{
+    Object *obj = OBJECT(opaque);
+    StringInputVisitor *siv;
+    Error *local_err = NULL;
+
+    if (strcmp(name, "qom-type") == 0 || strcmp(name, "id") == 0) {
+        return 0;
+    }
+
+    siv = string_input_visitor_new(value);
+    object_property_set(obj, string_input_get_visitor(siv), name, &local_err);
+    string_input_visitor_cleanup(siv);
+
+    if (local_err) {
+        qerror_report_err(local_err);
+        error_free(local_err);
+        return -1;
+    }
+
+    return 0;
+}
+
+static int object_create(QemuOpts *opts, void *opaque)
+{
+    const char *type = qemu_opt_get(opts, "qom-type");
+    const char *id = qemu_opts_id(opts);
+    Object *obj;
+
+    g_assert(type != NULL);
+
+    if (id == NULL) {
+        qerror_report(QERR_MISSING_PARAMETER, "id");
+        return -1;
+    }
+
+    obj = object_new(type);
+    if (qemu_opt_foreach(opts, object_set_property, obj, 1) < 0) {
+        return -1;
+    }
+
+    object_property_add_child(container_get(object_get_root(), "/objects"),
+                              id, obj, NULL);
+
+    return 0;
+}
+
 int qemu_init_main_loop(void)
 {
     return main_loop_init();
@@ -3309,6 +3357,9 @@ int main(int argc, char **argv, char **envp)
                     exit(0);
                 }
                 break;
+            case QEMU_OPTION_object:
+                opts = qemu_opts_parse(qemu_find_opts("object"), optarg, 1);
+                break;
             default:
                 os_parse_cmd_args(popt->index, optarg);
             }
@@ -3329,6 +3380,11 @@ int main(int argc, char **argv, char **envp)
         qemu_set_version(machine->hw_version);
     }
 
+    if (qemu_opts_foreach(qemu_find_opts("object"),
+                          object_create, NULL, 0) != 0) {
+        exit(1);
+    }
+
     /* Init CPU def lists, based on config
      * - Must be called after all the qemu_read_config_file() calls
      * - Must be called before list_cpus()
-- 
1.8.0

[Qemu-devel] [PATCH 2/6] object: add object_property_add_bool (v2)

[Anthony Liguori]

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
---
v1 -> v2
 - Fix whitespace (Andreas Faerber)
---
 include/qemu/object.h | 16 +++++++++++++++
 qom/object.c          | 56 +++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 72 insertions(+)

diff --git a/include/qemu/object.h b/include/qemu/object.h
index cc75fee..be707f1 100644
--- a/include/qemu/object.h
+++ b/include/qemu/object.h
@@ -947,6 +947,22 @@ void object_property_add_str(Object *obj, const char *name,
                              struct Error **errp);
 
 /**
+ * object_property_add_bool:
+ * @obj: the object to add a property to
+ * @name: the name of the property
+ * @get: the getter or NULL if the property is write-only.
+ * @set: the setter or NULL if the property is read-only
+ * @errp: if an error occurs, a pointer to an area to store the error
+ *
+ * Add a bool property using getters/setters.  This function will add a
+ * property of type 'bool'.
+ */
+void object_property_add_bool(Object *obj, const char *name,
+                              bool (*get)(Object *, struct Error **),
+                              void (*set)(Object *, bool, struct Error **),
+                              struct Error **errp);
+
+/**
  * object_child_foreach:
  * @obj: the object whose children will be navigated
  * @fn: the iterator function to be called
diff --git a/qom/object.c b/qom/object.c
index e3e9242..d7092b0 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -1183,6 +1183,62 @@ void object_property_add_str(Object *obj, const char *name,
                         prop, errp);
 }
 
+typedef struct BoolProperty
+{
+    bool (*get)(Object *, Error **);
+    void (*set)(Object *, bool, Error **);
+} BoolProperty;
+
+static void property_get_bool(Object *obj, Visitor *v, void *opaque,
+                              const char *name, Error **errp)
+{
+    BoolProperty *prop = opaque;
+    bool value;
+
+    value = prop->get(obj, errp);
+    visit_type_bool(v, &value, name, errp);
+}
+
+static void property_set_bool(Object *obj, Visitor *v, void *opaque,
+                              const char *name, Error **errp)
+{
+    BoolProperty *prop = opaque;
+    bool value;
+    Error *local_err = NULL;
+
+    visit_type_bool(v, &value, name, &local_err);
+    if (local_err) {
+        error_propagate(errp, local_err);
+        return;
+    }
+
+    prop->set(obj, value, errp);
+}
+
+static void property_release_bool(Object *obj, const char *name,
+                                  void *opaque)
+{
+    BoolProperty *prop = opaque;
+    g_free(prop);
+}
+
+void object_property_add_bool(Object *obj, const char *name,
+                              bool (*get)(Object *, Error **),
+                              void (*set)(Object *, bool, Error **),
+                              Error **errp)
+{
+    BoolProperty *prop = g_malloc0(sizeof(*prop));
+
+    prop->get = get;
+    prop->set = set;
+
+    object_property_add(obj, name, "bool",
+                        get ? property_get_bool : NULL,
+                        set ? property_set_bool : NULL,
+                        property_release_bool,
+                        prop, errp);
+}
+
 static char *qdev_get_type(Object *obj, Error **errp)
 {
     return g_strdup(object_get_typename(obj));
-- 
1.8.0

[Qemu-devel] [PATCH 3/6] rng: add RndBackend abstract object class

[Anthony Liguori]

This is the backend used by devices that need to request entropy.

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
---
 Makefile.objs          |  2 ++
 backends/Makefile.objs |  1 +
 backends/rng.c         | 93 ++++++++++++++++++++++++++++++++++++++++++++++++++
 include/qemu/rng.h     | 93 ++++++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 189 insertions(+)
 create mode 100644 backends/Makefile.objs
 create mode 100644 backends/rng.c
 create mode 100644 include/qemu/rng.h

diff --git a/Makefile.objs b/Makefile.objs
index 74b3542..a2decc5 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -100,6 +100,8 @@ common-obj-y += vl.o
 
 common-obj-$(CONFIG_SLIRP) += slirp/
 
+common-obj-y += backends/
+
 ######################################################################
 # libseccomp
 ifeq ($(CONFIG_SECCOMP),y)
diff --git a/backends/Makefile.objs b/backends/Makefile.objs
new file mode 100644
index 0000000..06e08c7
--- /dev/null
+++ b/backends/Makefile.objs
@@ -0,0 +1 @@
+common-obj-y += rng.o
diff --git a/backends/rng.c b/backends/rng.c
new file mode 100644
index 0000000..06f2611
--- /dev/null
+++ b/backends/rng.c
@@ -0,0 +1,93 @@
+/*
+ * QEMU Random Number Generator Backend
+ *
+ * Copyright IBM, Corp. 2012
+ *
+ * Authors:
+ *  Anthony Liguori   <aliguori@us.ibm.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include "qemu/rng.h"
+#include "qerror.h"
+
+void rng_backend_request_entropy(RngBackend *s, size_t size,
+                                 EntropyReceiveFunc *receive_entropy,
+                                 void *opaque)
+{
+    RngBackendClass *k = RNG_BACKEND_GET_CLASS(s);
+
+    if (k->request_entropy) {
+        k->request_entropy(s, size, receive_entropy, opaque);
+    }
+}
+
+void rng_backend_cancel_requests(RngBackend *s)
+{
+    RngBackendClass *k = RNG_BACKEND_GET_CLASS(s);
+
+    if (k->cancel_requests) {
+        k->cancel_requests(s);
+    }
+}
+
+static bool rng_backend_prop_get_opened(Object *obj, Error **errp)
+{
+    RngBackend *s = RNG_BACKEND(obj);
+
+    return s->opened;
+}
+
+void rng_backend_open(RngBackend *s, Error **errp)
+{
+    object_property_set_bool(OBJECT(s), true, "opened", errp);
+}
+
+static void rng_backend_prop_set_opened(Object *obj, bool value, Error **errp)
+{
+    RngBackend *s = RNG_BACKEND(obj);
+    RngBackendClass *k = RNG_BACKEND_GET_CLASS(s);
+
+    if (value == s->opened) {
+        return;
+    }
+
+    if (!value && s->opened) {
+        error_set(errp, QERR_PERMISSION_DENIED);
+        return;
+    }
+
+    if (k->opened) {
+        k->opened(s, errp);
+    }
+
+    if (!error_is_set(errp)) {
+        s->opened = value;
+    }
+}
+
+static void rng_backend_init(Object *obj)
+{
+    object_property_add_bool(obj, "opened",
+                             rng_backend_prop_get_opened,
+                             rng_backend_prop_set_opened,
+                             NULL);
+}
+
+static TypeInfo rng_backend_info = {
+    .name = TYPE_RNG_BACKEND,
+    .parent = TYPE_OBJECT,
+    .instance_size = sizeof(RngBackend),
+    .instance_init = rng_backend_init,
+    .class_size = sizeof(RngBackendClass),
+    .abstract = true,
+};
+
+static void register_types(void)
+{
+    type_register_static(&rng_backend_info);
+}
+
+type_init(register_types);
diff --git a/include/qemu/rng.h b/include/qemu/rng.h
new file mode 100644
index 0000000..7e9d672
--- /dev/null
+++ b/include/qemu/rng.h
@@ -0,0 +1,93 @@
+/*
+ * QEMU Random Number Generator Backend
+ *
+ * Copyright IBM, Corp. 2012
+ *
+ * Authors:
+ *  Anthony Liguori   <aliguori@us.ibm.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#ifndef QEMU_RNG_H
+#define QEMU_RNG_H
+
+#include "qemu/object.h"
+#include "qemu-common.h"
+#include "error.h"
+
+#define TYPE_RNG_BACKEND "rng-backend"
+#define RNG_BACKEND(obj) \
+    OBJECT_CHECK(RngBackend, (obj), TYPE_RNG_BACKEND)
+#define RNG_BACKEND_GET_CLASS(obj) \
+    OBJECT_GET_CLASS(RngBackendClass, (obj), TYPE_RNG_BACKEND)
+#define RNG_BACKEND_CLASS(klass) \
+    OBJECT_CLASS_CHECK(RngBackendClass, (klass), TYPE_RNG_BACKEND)
+
+typedef struct RngBackendClass RngBackendClass;
+typedef struct RngBackend RngBackend;
+
+typedef void (EntropyReceiveFunc)(void *opaque,
+                                  const void *data,
+                                  size_t size);
+
+struct RngBackendClass
+{
+    ObjectClass parent_class;
+
+    void (*request_entropy)(RngBackend *s, size_t size,
+                            EntropyReceiveFunc *recieve_entropy, void *opaque);
+    void (*cancel_requests)(RngBackend *s);
+
+    void (*opened)(RngBackend *s, Error **errp);
+};
+
+struct RngBackend
+{
+    Object parent;
+
+    /*< protected >*/
+    bool opened;
+};
+
+/**
+ * rng_backend_request_entropy:
+ * @s: the backend to request entropy from
+ * @size: the number of bytes of data to request
+ * @receive_entropy: a function to be invoked when entropy is available
+ * @opaque: data that should be passed to @receive_entropy
+ *
+ * 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.
+ *
+ * The backend does not need to pass the full amount of data to @receive_entropy
+ * but will pass at a value greater than 0.
+ */
+void rng_backend_request_entropy(RngBackend *s, size_t size,
+                                 EntropyReceiveFunc *receive_entropy,
+                                 void *opaque);
+
+/**
+ * rng_backend_cancel_requests:
+ * @s: the backend to cancel all pending requests in
+ *
+ * Cancels all pending requests submitted by @rng_backend_request_entropy.  This
+ * should be used by a device during reset or in preparation for live migration
+ * to stop tracking any request.
+ */
+void rng_backend_cancel_requests(RngBackend *s);
+
+/**
+ * rng_backend_open:
+ * @s: the backend to open
+ * @errp: a pointer to return the #Error object if an error occurs.
+ *
+ * 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
-- 
1.8.0

[Qemu-devel] [PATCH 4/6] rng-random: add an RNG backend that uses /dev/random

[Anthony Liguori]

The filename can be overridden but it expects a non-blocking source of entropy.
A typical invocation would be:

qemu -object rng-random,id=rng0 -device virtio-rng-pci,rng=rng0

This can also be used with /dev/urandom by using the command line:

qemu -object rng-random,filename=/dev/urandom,id=rng0 \
     -device virtio-rng-pci,rng=rng0

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
---
 backends/Makefile.objs |   2 +-
 backends/rng-random.c  | 163 +++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 164 insertions(+), 1 deletion(-)
 create mode 100644 backends/rng-random.c

diff --git a/backends/Makefile.objs b/backends/Makefile.objs
index 06e08c7..23ca19b 100644
--- a/backends/Makefile.objs
+++ b/backends/Makefile.objs
@@ -1 +1 @@
-common-obj-y += rng.o
+common-obj-y += rng.o rng-random.o
diff --git a/backends/rng-random.c b/backends/rng-random.c
new file mode 100644
index 0000000..6b6e4b4
--- /dev/null
+++ b/backends/rng-random.c
@@ -0,0 +1,163 @@
+/*
+ * QEMU Random Number Generator Backend
+ *
+ * Copyright IBM, Corp. 2012
+ *
+ * Authors:
+ *  Anthony Liguori   <aliguori@us.ibm.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include "qemu/rng.h"
+#include "qerror.h"
+#include "main-loop.h"
+
+#define TYPE_RNG_RANDOM "rng-random"
+#define RNG_RANDOM(obj) OBJECT_CHECK(RndRandom, (obj), TYPE_RNG_RANDOM)
+
+typedef struct RndRandom
+{
+    RngBackend parent;
+
+    int fd;
+    char *filename;
+
+    EntropyReceiveFunc *receive_func;
+    void *opaque;
+    size_t size;
+} RndRandom;
+
+/**
+ * A simple and incomplete backend to request entropy from /dev/random.
+ *
+ * This backend exposes an additional "filename" property that can be used to
+ * set the filename to use to open the backend.
+ */
+
+static void entropy_available(void *opaque)
+{
+    RndRandom *s = RNG_RANDOM(opaque);
+    uint8_t buffer[s->size];
+    ssize_t len;
+
+    len = read(s->fd, buffer, s->size);
+    g_assert(len != -1);
+
+    s->receive_func(s->opaque, buffer, s->size);
+    s->receive_func = NULL;
+
+    qemu_set_fd_handler(s->fd, NULL, NULL, NULL);
+}
+
+static void rng_random_request_entropy(RngBackend *b, size_t size,
+                                        EntropyReceiveFunc *receive_entropy,
+                                        void *opaque)
+{
+    RndRandom *s = RNG_RANDOM(b);
+
+    if (s->receive_func) {
+        s->receive_func(s->opaque, NULL, 0);
+    }
+
+    s->receive_func = receive_entropy;
+    s->opaque = opaque;
+    s->size = size;
+
+    qemu_set_fd_handler(s->fd, entropy_available, NULL, s);
+}
+
+static void rng_random_opened(RngBackend *b, Error **errp)
+{
+    RndRandom *s = RNG_RANDOM(b);
+
+    if (s->filename == NULL) {
+        error_set(errp, QERR_INVALID_PARAMETER_VALUE,
+                  "filename", "a valid filename");
+    } else {
+        s->fd = open(s->filename, O_RDONLY | O_NONBLOCK);
+
+        if (s->fd == -1) {
+            error_set(errp, QERR_OPEN_FILE_FAILED, s->filename);
+        }
+    }
+}
+
+static char *rng_random_get_filename(Object *obj, Error **errp)
+{
+    RndRandom *s = RNG_RANDOM(obj);
+
+    if (s->filename) {
+        return g_strdup(s->filename);
+    }
+
+    return NULL;
+}
+
+static void rng_random_set_filename(Object *obj, const char *filename,
+                                 Error **errp)
+{
+    RngBackend *b = RNG_BACKEND(obj);
+    RndRandom *s = RNG_RANDOM(obj);
+
+    if (b->opened) {
+        error_set(errp, QERR_PERMISSION_DENIED);
+        return;
+    }
+
+    if (s->filename) {
+        g_free(s->filename);
+    }
+
+    s->filename = g_strdup(filename);
+}
+
+static void rng_random_init(Object *obj)
+{
+    RndRandom *s = RNG_RANDOM(obj);
+
+    object_property_add_str(obj, "filename",
+                            rng_random_get_filename,
+                            rng_random_set_filename,
+                            NULL);
+
+    s->filename = g_strdup("/dev/random");
+}
+
+static void rng_random_finalize(Object *obj)
+{
+    RndRandom *s = RNG_RANDOM(obj);
+
+    qemu_set_fd_handler(s->fd, NULL, NULL, NULL);
+
+    if (s->fd != -1) {
+        close(s->fd);
+    }
+
+    g_free(s->filename);
+}
+
+static void rng_random_class_init(ObjectClass *klass, void *data)
+{
+    RngBackendClass *rbc = RNG_BACKEND_CLASS(klass);
+
+    rbc->request_entropy = rng_random_request_entropy;
+    rbc->opened = rng_random_opened;
+}
+
+static TypeInfo rng_random_info = {
+    .name = TYPE_RNG_RANDOM,
+    .parent = TYPE_RNG_BACKEND,
+    .instance_size = sizeof(RndRandom),
+    .class_init = rng_random_class_init,
+    .instance_init = rng_random_init,
+    .instance_finalize = rng_random_finalize,
+};
+
+static void register_types(void)
+{
+    type_register_static(&rng_random_info);
+}
+
+type_init(register_types);
-- 
1.8.0

[Qemu-devel] [PATCH 5/6] rng-egd: introduce EGD compliant RNG backend

[Anthony Liguori]

This backend talks EGD to a CharDriverState.  A typical way to invoke this would
be:

qemu -chardev socket,host=localhost,port=1024,id=chr0 \
     -object rng-egd,chardev=chr0,id=egd0 \
     -device virtio-rng-pci,rng=egd0

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
---
 backends/Makefile.objs |   2 +-
 backends/rng-egd.c     | 215 +++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 216 insertions(+), 1 deletion(-)
 create mode 100644 backends/rng-egd.c

diff --git a/backends/Makefile.objs b/backends/Makefile.objs
index 23ca19b..875eebc 100644
--- a/backends/Makefile.objs
+++ b/backends/Makefile.objs
@@ -1 +1 @@
-common-obj-y += rng.o rng-random.o
+common-obj-y += rng.o rng-random.o rng-egd.o
diff --git a/backends/rng-egd.c b/backends/rng-egd.c
new file mode 100644
index 0000000..ec58358
--- /dev/null
+++ b/backends/rng-egd.c
@@ -0,0 +1,215 @@
+/*
+ * QEMU Random Number Generator Backend
+ *
+ * Copyright IBM, Corp. 2012
+ *
+ * Authors:
+ *  Anthony Liguori   <aliguori@us.ibm.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include "qemu/rng.h"
+#include "qemu-char.h"
+#include "qerror.h"
+#include "hw/qdev.h" /* just for DEFINE_PROP_CHR */
+
+#define TYPE_RNG_EGD "rng-egd"
+#define RNG_EGD(obj) OBJECT_CHECK(RngEgd, (obj), TYPE_RNG_EGD)
+
+typedef struct RngEgd
+{
+    RngBackend parent;
+
+    CharDriverState *chr;
+    char *chr_name;
+
+    GSList *requests;
+} RngEgd;
+
+typedef struct RngRequest
+{
+    EntropyReceiveFunc *receive_entropy;
+    uint8_t *data;
+    void *opaque;
+    size_t offset;
+    size_t size;
+} RngRequest;
+
+static void rng_egd_request_entropy(RngBackend *b, size_t size,
+                                    EntropyReceiveFunc *receive_entropy,
+                                    void *opaque)
+{
+    RngEgd *s = RNG_EGD(b);
+    RngRequest *req;
+
+    req = g_malloc(sizeof(*req));
+
+    req->offset = 0;
+    req->size = size;
+    req->receive_entropy = receive_entropy;
+    req->opaque = opaque;
+    req->data = g_malloc(req->size);
+
+    while (size > 0) {
+        uint8_t header[2];
+        uint8_t len = MIN(size, 255);
+
+        /* synchronous entropy request */
+        header[0] = 0x02;
+        header[1] = len;
+
+        qemu_chr_fe_write(s->chr, header, sizeof(header));
+
+        size -= len;
+    }
+
+    s->requests = g_slist_append(s->requests, req);
+}
+
+static void rng_egd_free_request(RngRequest *req)
+{
+    g_free(req->data);
+    g_free(req);
+}
+
+static int rng_egd_chr_can_read(void *opaque)
+{
+    RngEgd *s = RNG_EGD(opaque);
+    GSList *i;
+    int size = 0;
+
+    for (i = s->requests; i; i = i->next) {
+        RngRequest *req = i->data;
+        size += req->size - req->offset;
+    }
+
+    return size;
+}
+
+static void rng_egd_chr_read(void *opaque, const uint8_t *buf, int size)
+{
+    RngEgd *s = RNG_EGD(opaque);
+
+    while (size > 0 && s->requests) {
+        RngRequest *req = s->requests->data;
+        int len = MIN(size, req->size - req->offset);
+
+        memcpy(req->data + req->offset, buf, len);
+        req->offset += len;
+        size -= len;
+
+        if (req->offset == req->size) {
+            s->requests = g_slist_remove_link(s->requests, s->requests);
+
+            req->receive_entropy(req->opaque, req->data, req->size);
+
+            rng_egd_free_request(req);
+        }
+    }
+}
+
+static void rng_egd_cancel_requests(RngBackend *b)
+{
+    RngEgd *s = RNG_EGD(b);
+
+    /* We simply delete the list of pending requests.  If there is data in the 
+     * queue waiting to be read, this is okay, because there will always be
+     * more data than we requested originally
+     */
+    g_slist_free_full(s->requests,
+                      (GDestroyNotify)rng_egd_free_request);
+    s->requests = NULL;
+}
+
+static void rng_egd_opened(RngBackend *b, Error **errp)
+{
+    RngEgd *s = RNG_EGD(b);
+
+    if (s->chr_name == NULL) {
+        error_set(errp, QERR_INVALID_PARAMETER_VALUE,
+                  "chardev", "a valid character device");
+        return;
+    }
+
+    s->chr = qemu_chr_find(s->chr_name);
+    if (s->chr == NULL) {
+        error_set(errp, QERR_DEVICE_NOT_FOUND, s->chr_name);
+        return;
+    }
+
+    /* FIXME we should resubmit pending requests when the CDS reconnects. */
+    qemu_chr_add_handlers(s->chr, rng_egd_chr_can_read, rng_egd_chr_read,
+                          NULL, s);
+}
+
+static void rng_egd_set_chardev(Object *obj, const char *value, Error **errp)
+{
+    RngBackend *b = RNG_BACKEND(obj);
+    RngEgd *s = RNG_EGD(b);
+
+    if (b->opened) {
+        error_set(errp, QERR_PERMISSION_DENIED);
+    } else {
+        g_free(s->chr_name);
+        s->chr_name = g_strdup(value);
+    }
+}
+
+static char *rng_egd_get_chardev(Object *obj, Error **errp)
+{
+    RngEgd *s = RNG_EGD(obj);
+
+    if (s->chr && s->chr->label) {
+        return g_strdup(s->chr->label);
+    }
+
+    return NULL;
+}
+
+static void rng_egd_init(Object *obj)
+{
+    object_property_add_str(obj, "chardev",
+                            rng_egd_get_chardev, rng_egd_set_chardev,
+                            NULL);
+}
+
+static void rng_egd_finalize(Object *obj)
+{
+    RngEgd *s = RNG_EGD(obj);
+
+    if (s->chr) {
+        qemu_chr_add_handlers(s->chr, NULL, NULL, NULL, NULL);
+    }
+
+    g_free(s->chr_name);
+
+    g_slist_free_full(s->requests, (GDestroyNotify)rng_egd_free_request);
+    s->requests = NULL;
+}
+
+static void rng_egd_class_init(ObjectClass *klass, void *data)
+{
+    RngBackendClass *rbc = RNG_BACKEND_CLASS(klass);
+
+    rbc->request_entropy = rng_egd_request_entropy;
+    rbc->cancel_requests = rng_egd_cancel_requests;
+    rbc->opened = rng_egd_opened;
+}
+
+static TypeInfo rng_egd_info = {
+    .name = TYPE_RNG_EGD,
+    .parent = TYPE_RNG_BACKEND,
+    .instance_size = sizeof(RngEgd),
+    .class_init = rng_egd_class_init,
+    .instance_init = rng_egd_init,
+    .instance_finalize = rng_egd_finalize,
+};
+
+static void register_types(void)
+{
+    type_register_static(&rng_egd_info);
+}
+
+type_init(register_types);
-- 
1.8.0

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

[Anthony Liguori]

From: Amit Shah <amit.shah@redhat.com>

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.

Signed-off-by: Amit Shah <amit.shah@redhat.com>
Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
---
aliguori: converted to new RngBackend interface
aliguori: remove entropy needed event
aliguori: fix migration
---
 hw/Makefile.objs     |   1 +
 hw/pci.h             |   1 +
 hw/s390-virtio-bus.c |  37 +++++++++
 hw/s390-virtio-bus.h |   2 +
 hw/virtio-pci.c      |  60 +++++++++++++++
 hw/virtio-pci.h      |   2 +
 hw/virtio-rng.c      | 211 +++++++++++++++++++++++++++++++++++++++++++++++++++
 hw/virtio-rng.h      |  24 ++++++
 hw/virtio.h          |   3 +
 9 files changed, 341 insertions(+)
 create mode 100644 hw/virtio-rng.c
 create mode 100644 hw/virtio-rng.h

diff --git a/hw/Makefile.objs b/hw/Makefile.objs
index af4ab0c..ea46f81 100644
--- a/hw/Makefile.objs
+++ b/hw/Makefile.objs
@@ -1,6 +1,7 @@
 common-obj-y = usb/ ide/
 common-obj-y += loader.o
 common-obj-$(CONFIG_VIRTIO) += virtio-console.o
+common-obj-$(CONFIG_VIRTIO) += virtio-rng.o
 common-obj-$(CONFIG_VIRTIO_PCI) += virtio-pci.o
 common-obj-y += fw_cfg.o
 common-obj-$(CONFIG_PCI) += pci.o pci_bridge.o pci_bridge_dev.o
diff --git a/hw/pci.h b/hw/pci.h
index 1f902f5..93e2a46 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 5849a96..e0ac2d1 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"
@@ -206,6 +207,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;
@@ -448,6 +461,29 @@ static TypeInfo s390_virtio_serial = {
     .class_init    = s390_virtio_serial_class_init,
 };
 
+static void s390_virtio_rng_initfn(Object *obj)
+{
+    VirtIOS390Device *dev = VIRTIO_S390_DEVICE(obj);
+
+    object_property_add_link(obj, "rng", TYPE_RNG_BACKEND,
+                             (Object **)&dev->rng.rng, NULL);
+}
+
+static void s390_virtio_rng_class_init(ObjectClass *klass, void *data)
+{
+    VirtIOS390DeviceClass *k = VIRTIO_S390_DEVICE_CLASS(klass);
+
+    k->init = s390_virtio_rng_init;
+}
+
+static TypeInfo s390_virtio_rng = {
+    .name          = "virtio-rng-s390",
+    .parent        = TYPE_VIRTIO_S390_DEVICE,
+    .instance_size = sizeof(VirtIOS390Device),
+    .instance_init = s390_virtio_rng_initfn,
+    .class_init    = s390_virtio_rng_class_init,
+};
+
 static int s390_virtio_busdev_init(DeviceState *dev)
 {
     VirtIOS390Device *_dev = (VirtIOS390Device *)dev;
@@ -528,6 +564,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 4873134..a83afe7 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"
 
@@ -75,6 +76,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 c7f20c3..0dc2a06 100644
--- a/hw/virtio-pci.c
+++ b/hw/virtio-pci.c
@@ -880,6 +880,28 @@ static void virtio_balloon_exit_pci(PCIDevice *pci_dev)
     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 void 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);
+    virtio_exit_pci(pci_dev);
+}
+
 static Property virtio_blk_properties[] = {
     DEFINE_PROP_HEX32("class", VirtIOPCIProxy, class_code, 0),
     DEFINE_BLOCK_PROPERTIES(VirtIOPCIProxy, blk.conf),
@@ -1010,6 +1032,43 @@ static TypeInfo virtio_balloon_info = {
     .class_init    = virtio_balloon_class_init,
 };
 
+static void virtio_rng_initfn(Object *obj)
+{
+    PCIDevice *pci_dev = PCI_DEVICE(obj);
+    VirtIOPCIProxy *proxy = DO_UPCAST(VirtIOPCIProxy, pci_dev, pci_dev);
+
+    object_property_add_link(obj, "rng", TYPE_RNG_BACKEND,
+                             (Object **)&proxy->rng.rng, NULL);
+}
+
+static Property virtio_rng_properties[] = {
+    DEFINE_VIRTIO_COMMON_FEATURES(VirtIOPCIProxy, host_features),
+    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),
+    .instance_init = virtio_rng_initfn,
+    .class_init    = virtio_rng_class_init,
+};
+
 static int virtio_scsi_init_pci(PCIDevice *pci_dev)
 {
     VirtIOPCIProxy *proxy = DO_UPCAST(VirtIOPCIProxy, pci_dev, pci_dev);
@@ -1074,6 +1133,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 ac9d522..b58d9a2 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"
 
@@ -46,6 +47,7 @@ typedef struct {
     virtio_serial_conf serial;
     virtio_net_conf net;
     VirtIOSCSIConf scsi;
+    VirtIORNGConf rng;
     bool ioeventfd_disabled;
     bool ioeventfd_started;
     VirtIOIRQFD *vector_irqfd;
diff --git a/hw/virtio-rng.c b/hw/virtio-rng.c
new file mode 100644
index 0000000..b7fb5e9
--- /dev/null
+++ b/hw/virtio-rng.c
@@ -0,0 +1,211 @@
+/*
+ * 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 "virtio.h"
+#include "virtio-rng.h"
+#include "qemu/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;
+
+    RngBackend *rng;
+} 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 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;
+}
+
+/* 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,
+                           0, buf + offset, 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);
+    }
+}
+
+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) {
+        rng_backend_request_entropy(vrng->rng, size, chr_read, vrng);
+    }
+}
+
+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) {
+        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)
+{
+    VirtIORNG *vrng = opaque;
+
+    if (version_id != 1) {
+        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;
+}
+
+VirtIODevice *virtio_rng_init(DeviceState *dev, VirtIORNGConf *conf)
+{
+    VirtIORNG *vrng;
+    VirtIODevice *vdev;
+    Error *local_err = NULL;
+
+    vdev = virtio_common_init("virtio-rng", VIRTIO_ID_RNG, 0,
+                              sizeof(VirtIORNG));
+
+    vrng = DO_UPCAST(VirtIORNG, vdev, vdev);
+
+    vrng->rng = conf->rng;
+    if (vrng->rng == NULL) {
+        qerror_report(QERR_INVALID_PARAMETER_VALUE, "rng", "a valid object");
+        return NULL;
+    }
+
+    rng_backend_open(vrng->rng, &local_err);
+    if (local_err) {
+        qerror_report_err(local_err);
+        error_free(local_err);
+        return NULL;
+    }
+
+    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);
+
+    return vdev;
+}
+
+void virtio_rng_exit(VirtIODevice *vdev)
+{
+    VirtIORNG *vrng = DO_UPCAST(VirtIORNG, vdev, vdev);
+
+    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..fbb0104
--- /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/rng.h"
+
+/* The Virtio ID for the virtio rng device */
+#define VIRTIO_ID_RNG    4
+
+struct VirtIORNGConf {
+    RngBackend *rng;
+};
+
+#endif
diff --git a/hw/virtio.h b/hw/virtio.h
index ac482be..df8d0f7 100644
--- a/hw/virtio.h
+++ b/hw/virtio.h
@@ -203,6 +203,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
@@ -213,6 +215,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, \
-- 
1.8.0

Re: [Qemu-devel] [PATCH 0/6] add paravirtualization hwrng support

[Paolo Bonzini]

> This series implements the backend and frontend infrastructure for virtio-rng.
> This is similar to previous series sent out by both Amit and myself
> although it has been trimmed down considerably.
> 
> In terms of backends, a file and EGD backend are supported.  The file defaults
> to /dev/random based on the feedback from Peter.  It's still possible
> to support /dev/urandom though as an entropy source by overriding the file name.
> 
> I think this series is ready to merge.

Is /dev/random even appropriate to feed rngd?

rngd needs _a lot_ of entropy to even start working.  Its randomness test works in groups of 20000 bits. On a system without an hardware RNG, /dev/random can hardly produce 4000 bits/minute.  This means a guest will not get any entropy boost for 5 minutes after it's started, even if we allow it to exhaust the parent's entropy.

At this point, /dev/hwrng (or rdrand) seems just as good as /dev/random as a source for virtio-rng (and even better, it is not starved as easily).

I think RngBackend is over-engineered.  What other backends do you plan on adding?  Maybe rdrand, but that's just a chardev---so why isn't this enough:

  -chardev file,source=on,path=/dev/hwrng,id=chr0  -device virtio-rng-pci,file=chr0
  -chardev rdrand,id=chr0                          -device virtio-rng-pci,file=chr0
  -chardev socket,host=localhost,port=1024,id=chr0 -device virtio-rng-pci,rng=chr0,egd=on

(which I suggested in my reply to Amit)?

Paolo

Re: [Qemu-devel] [PATCH 0/6] add paravirtualization hwrng support

[Anthony Liguori]

Paolo Bonzini <pbonzini@redhat.com> writes:

>> This series implements the backend and frontend infrastructure for virtio-rng.
>> This is similar to previous series sent out by both Amit and myself
>> although it has been trimmed down considerably.
>> 
>> In terms of backends, a file and EGD backend are supported.  The file defaults
>> to /dev/random based on the feedback from Peter.  It's still possible
>> to support /dev/urandom though as an entropy source by overriding the file name.
>> 
>> I think this series is ready to merge.
>
> Is /dev/random even appropriate to feed rngd?
>
> rngd needs _a lot_ of entropy to even start working.  Its randomness
> test works in groups of 20000 bits. On a system without an hardware
> RNG, /dev/random can hardly produce 4000 bits/minute.  This means a
> guest will not get any entropy boost for 5 minutes after it's started,
> even if we allow it to exhaust the parent's entropy.

I don't know, but rng-random is a non-blocking backend so it can handle
/dev/random, /dev/urandom, or /dev/hwrng.

It's just a matter of what the default is and I feel comfortable that if
someone can provide a *concrete* demonstration of what the best default
is, we can change it later on.

> At this point, /dev/hwrng (or rdrand) seems just as good as
> /dev/random as a source for virtio-rng (and even better, it is not
> starved as easily).

I've been told that hwrng sources need to be passed through a whitening
function in order to be suitable for PRNG generators.  Since we expose a
/dev/hwrng in the guest, perhaps this doesn't matter...

> I think RngBackend is over-engineered. What other backends do you plan
> on adding?

Stefan Berger suggested a backend that uses a PRNG in FreeBL.  That's
probably the best default since it punts to a userspace library to deal
with ensuring there's adequate whitening/entropy to start with.

> Maybe rdrand, but that's just a chardev---so why isn't this enough:
>
>   -chardev file,source=on,path=/dev/hwrng,id=chr0  -device virtio-rng-pci,file=chr0
>   -chardev rdrand,id=chr0                          -device virtio-rng-pci,file=chr0
>   -chardev socket,host=localhost,port=1024,id=chr0 -device virtio-rng-pci,rng=chr0,egd=on
>
> (which I suggested in my reply to Amit)?

I don't like overloading chardev to representate any !block device
backend which is what I fear we're doing here.

EGD is more than just a dumb pipe of data too.  It's got a way to query
available entropy.  I have a strong suspicion that over time, we'll add
methods to virtio-rng to query available entropy.  That would mean
adding a backend specific ioctl to the chardev layer which is pretty
ugly.

The overhead of creating a separate backend to begin with is extremely
small.  We're talking about dozens of lines of code.  So I don't see
what the problem is.

Regards,

Anthony Liguori

>
> Paolo

Re: [Qemu-devel] [PATCH 0/6] add paravirtualization hwrng support

[H. Peter Anvin]

On 10/26/2012 08:42 AM, Anthony Liguori wrote:
>>
>> Is /dev/random even appropriate to feed rngd?
>>
>> rngd needs _a lot_ of entropy to even start working.  Its randomness
>> test works in groups of 20000 bits. On a system without an hardware
>> RNG, /dev/random can hardly produce 4000 bits/minute.  This means a
>> guest will not get any entropy boost for 5 minutes after it's started,
>> even if we allow it to exhaust the parent's entropy.
>
> I don't know, but rng-random is a non-blocking backend so it can handle
> /dev/random, /dev/urandom, or /dev/hwrng.
>

/dev/urandom is just plain *wrong*... it is feeding a PRNG into a PRNG 
which can best be described as "masturbation" and at worst as a 
"cryptographic usage violation."

/dev/hwrng is reasonable, in some ways; after all, the guest itself is 
expected to use rngd.  There are, however, at least two problems:

a) it means that the guest *has* to run rngd or a similar engine; if you 
have control over the guests it might be more efficient to run rngd in 
skip-test mode (I don't think that is currently implemented but it 
could/should be) and centralize all testing to the host.

A skip-test mode would also allow rngd to forward-feed shorter blocks 
than 2500 bytes.

b) if the host has no physical hwrng, /dev/hwrng will output nothing at 
all, which is worse than /dev/random in that situation.

> Stefan Berger suggested a backend that uses a PRNG in FreeBL.  That's
> probably the best default since it punts to a userspace library to deal
> with ensuring there's adequate whitening/entropy to start with.

We SHOULD NOT expose a PRNG here!  It is the same fail as using 
/dev/urandom (but worse)  The whole point is to inject actual entropy... 
a PRNG can (and typically will) just run in guest space.

>> Maybe rdrand, but that's just a chardev---so why isn't this enough:
>>
>>    -chardev file,source=on,path=/dev/hwrng,id=chr0  -device virtio-rng-pci,file=chr0
>>    -chardev rdrand,id=chr0                          -device virtio-rng-pci,file=chr0
>>    -chardev socket,host=localhost,port=1024,id=chr0 -device virtio-rng-pci,rng=chr0,egd=on
>>
>> (which I suggested in my reply to Amit)?

If you have rdrand you might just use it in the guest directly, unless 
you have a strong reason (migration?) not to do that.  Either way, for 
rdrand you need whitening similar to what rngd is doing (for *rdseed* 
you do not, but rdseed is not shipping yet.)

The startup issue is an interesting problem.  If you have full control 
over the guest it might be best to simply inject some entropy into the 
guest on startup via the initramfs or a disk image; that has its own 
awkwardness too, of course.  The one bit that could potentially be 
solved in Qemu would be an option to "don't start the guest until X 
bytes of entropy have been gathered."

Overall, I want to emphasize that we don't want to try solve generic 
problems in virtualization space... resource constraints on /dev/random 
is a generic host OS issue for example.

	-hpa

-- 
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.

Re: [Qemu-devel] [PATCH 0/6] add paravirtualization hwrng support

[Anthony Liguori]

"H. Peter Anvin" <hpa@zytor.com> writes:

> On 10/26/2012 08:42 AM, Anthony Liguori wrote:
>>>
>>> Is /dev/random even appropriate to feed rngd?
>>>
>>> rngd needs _a lot_ of entropy to even start working.  Its randomness
>>> test works in groups of 20000 bits. On a system without an hardware
>>> RNG, /dev/random can hardly produce 4000 bits/minute.  This means a
>>> guest will not get any entropy boost for 5 minutes after it's started,
>>> even if we allow it to exhaust the parent's entropy.
>>
>> I don't know, but rng-random is a non-blocking backend so it can handle
>> /dev/random, /dev/urandom, or /dev/hwrng.
>>
>
> /dev/urandom is just plain *wrong*... it is feeding a PRNG into a PRNG 
> which can best be described as "masturbation" and at worst as a 
> "cryptographic usage violation."

I don't understand your logic here.

>From the discussions I've had, the quality of the randomness from a
*well seeded* PRNG ought to be good enough to act as an entropy source
within the guest.

What qualifies as well seeded is a bit difficult to pin down with more
specificity than "kilobytes of data".

I stayed away from /dev/urandom primarily because it's impossible to
determine if it's well seeded or not making urandom dangerous to use.

But using a PRNG makes sense to me when dealing with multiple guests.
If you have a finite source of entropy in the host, using a PRNG to
create unique entropy for each guest is certainly better than
duplicating entropy.

Adding Ted T'so and a few others to CC in hopes that they can chime in
here too.

FWIW, none of this should affect this series being merged as it can use
a variety of different inputs.  But I would like to have a strong
recommendation for what people should use (and make that default) so I'd
really like to get a clear answer here.

Regards,

Anthony Liguori

>
> /dev/hwrng is reasonable, in some ways; after all, the guest itself is 
> expected to use rngd.  There are, however, at least two problems:
>
> a) it means that the guest *has* to run rngd or a similar engine; if you 
> have control over the guests it might be more efficient to run rngd in 
> skip-test mode (I don't think that is currently implemented but it 
> could/should be) and centralize all testing to the host.
>
> A skip-test mode would also allow rngd to forward-feed shorter blocks 
> than 2500 bytes.
>
> b) if the host has no physical hwrng, /dev/hwrng will output nothing at 
> all, which is worse than /dev/random in that situation.
>
>> Stefan Berger suggested a backend that uses a PRNG in FreeBL.  That's
>> probably the best default since it punts to a userspace library to deal
>> with ensuring there's adequate whitening/entropy to start with.
>
> We SHOULD NOT expose a PRNG here!  It is the same fail as using 
> /dev/urandom (but worse)  The whole point is to inject actual entropy... 
> a PRNG can (and typically will) just run in guest space.
>
>>> Maybe rdrand, but that's just a chardev---so why isn't this enough:
>>>
>>>    -chardev file,source=on,path=/dev/hwrng,id=chr0  -device virtio-rng-pci,file=chr0
>>>    -chardev rdrand,id=chr0                          -device virtio-rng-pci,file=chr0
>>>    -chardev socket,host=localhost,port=1024,id=chr0 -device virtio-rng-pci,rng=chr0,egd=on
>>>
>>> (which I suggested in my reply to Amit)?
>
> If you have rdrand you might just use it in the guest directly, unless 
> you have a strong reason (migration?) not to do that.  Either way, for 
> rdrand you need whitening similar to what rngd is doing (for *rdseed* 
> you do not, but rdseed is not shipping yet.)
>
> The startup issue is an interesting problem.  If you have full control 
> over the guest it might be best to simply inject some entropy into the 
> guest on startup via the initramfs or a disk image; that has its own 
> awkwardness too, of course.  The one bit that could potentially be 
> solved in Qemu would be an option to "don't start the guest until X 
> bytes of entropy have been gathered."
>
> Overall, I want to emphasize that we don't want to try solve generic 
> problems in virtualization space... resource constraints on /dev/random 
> is a generic host OS issue for example.
>
> 	-hpa
>
> -- 
> H. Peter Anvin, Intel Open Source Technology Center
> I work for Intel.  I don't speak on their behalf.

Re: [Qemu-devel] [PATCH 0/6] add paravirtualization hwrng support

[H. Peter Anvin]

PRNGs don't "create entropy."  Period.

The guest will run its own PRNG.

Anthony Liguori <aliguori@us.ibm.com> wrote:

>"H. Peter Anvin" <hpa@zytor.com> writes:
>
>> On 10/26/2012 08:42 AM, Anthony Liguori wrote:
>>>>
>>>> Is /dev/random even appropriate to feed rngd?
>>>>
>>>> rngd needs _a lot_ of entropy to even start working.  Its
>randomness
>>>> test works in groups of 20000 bits. On a system without an hardware
>>>> RNG, /dev/random can hardly produce 4000 bits/minute.  This means a
>>>> guest will not get any entropy boost for 5 minutes after it's
>started,
>>>> even if we allow it to exhaust the parent's entropy.
>>>
>>> I don't know, but rng-random is a non-blocking backend so it can
>handle
>>> /dev/random, /dev/urandom, or /dev/hwrng.
>>>
>>
>> /dev/urandom is just plain *wrong*... it is feeding a PRNG into a
>PRNG 
>> which can best be described as "masturbation" and at worst as a 
>> "cryptographic usage violation."
>
>I don't understand your logic here.
>
>From the discussions I've had, the quality of the randomness from a
>*well seeded* PRNG ought to be good enough to act as an entropy source
>within the guest.
>
>What qualifies as well seeded is a bit difficult to pin down with more
>specificity than "kilobytes of data".
>
>I stayed away from /dev/urandom primarily because it's impossible to
>determine if it's well seeded or not making urandom dangerous to use.
>
>But using a PRNG makes sense to me when dealing with multiple guests.
>If you have a finite source of entropy in the host, using a PRNG to
>create unique entropy for each guest is certainly better than
>duplicating entropy.
>
>Adding Ted T'so and a few others to CC in hopes that they can chime in
>here too.
>
>FWIW, none of this should affect this series being merged as it can use
>a variety of different inputs.  But I would like to have a strong
>recommendation for what people should use (and make that default) so
>I'd
>really like to get a clear answer here.
>
>Regards,
>
>Anthony Liguori
>
>>
>> /dev/hwrng is reasonable, in some ways; after all, the guest itself
>is 
>> expected to use rngd.  There are, however, at least two problems:
>>
>> a) it means that the guest *has* to run rngd or a similar engine; if
>you 
>> have control over the guests it might be more efficient to run rngd
>in 
>> skip-test mode (I don't think that is currently implemented but it 
>> could/should be) and centralize all testing to the host.
>>
>> A skip-test mode would also allow rngd to forward-feed shorter blocks
>
>> than 2500 bytes.
>>
>> b) if the host has no physical hwrng, /dev/hwrng will output nothing
>at 
>> all, which is worse than /dev/random in that situation.
>>
>>> Stefan Berger suggested a backend that uses a PRNG in FreeBL. 
>That's
>>> probably the best default since it punts to a userspace library to
>deal
>>> with ensuring there's adequate whitening/entropy to start with.
>>
>> We SHOULD NOT expose a PRNG here!  It is the same fail as using 
>> /dev/urandom (but worse)  The whole point is to inject actual
>entropy... 
>> a PRNG can (and typically will) just run in guest space.
>>
>>>> Maybe rdrand, but that's just a chardev---so why isn't this enough:
>>>>
>>>>    -chardev file,source=on,path=/dev/hwrng,id=chr0  -device
>virtio-rng-pci,file=chr0
>>>>    -chardev rdrand,id=chr0                          -device
>virtio-rng-pci,file=chr0
>>>>    -chardev socket,host=localhost,port=1024,id=chr0 -device
>virtio-rng-pci,rng=chr0,egd=on
>>>>
>>>> (which I suggested in my reply to Amit)?
>>
>> If you have rdrand you might just use it in the guest directly,
>unless 
>> you have a strong reason (migration?) not to do that.  Either way,
>for 
>> rdrand you need whitening similar to what rngd is doing (for *rdseed*
>
>> you do not, but rdseed is not shipping yet.)
>>
>> The startup issue is an interesting problem.  If you have full
>control 
>> over the guest it might be best to simply inject some entropy into
>the 
>> guest on startup via the initramfs or a disk image; that has its own 
>> awkwardness too, of course.  The one bit that could potentially be 
>> solved in Qemu would be an option to "don't start the guest until X 
>> bytes of entropy have been gathered."
>>
>> Overall, I want to emphasize that we don't want to try solve generic 
>> problems in virtualization space... resource constraints on
>/dev/random 
>> is a generic host OS issue for example.
>>
>> 	-hpa
>>
>> -- 
>> H. Peter Anvin, Intel Open Source Technology Center
>> I work for Intel.  I don't speak on their behalf.

-- 
Sent from my mobile phone. Please excuse brevity and lack of formatting.

Re: [Qemu-devel] [PATCH 0/6] add paravirtualization hwrng support

[Amit Shah]

On (Fri) 26 Oct 2012 [13:24:06], Anthony Liguori wrote:
> "H. Peter Anvin" <hpa@zytor.com> writes:
> 
> > On 10/26/2012 08:42 AM, Anthony Liguori wrote:
> >>>
> >>> Is /dev/random even appropriate to feed rngd?
> >>>
> >>> rngd needs _a lot_ of entropy to even start working.  Its randomness
> >>> test works in groups of 20000 bits. On a system without an hardware
> >>> RNG, /dev/random can hardly produce 4000 bits/minute.  This means a
> >>> guest will not get any entropy boost for 5 minutes after it's started,
> >>> even if we allow it to exhaust the parent's entropy.
> >>
> >> I don't know, but rng-random is a non-blocking backend so it can handle
> >> /dev/random, /dev/urandom, or /dev/hwrng.
> >>
> >
> > /dev/urandom is just plain *wrong*... it is feeding a PRNG into a PRNG 
> > which can best be described as "masturbation" and at worst as a 
> > "cryptographic usage violation."
> 
> I don't understand your logic here.
> 
> From the discussions I've had, the quality of the randomness from a
> *well seeded* PRNG ought to be good enough to act as an entropy source
> within the guest.
> 
> What qualifies as well seeded is a bit difficult to pin down with more
> specificity than "kilobytes of data".
> 
> I stayed away from /dev/urandom primarily because it's impossible to
> determine if it's well seeded or not making urandom dangerous to use.
> 
> But using a PRNG makes sense to me when dealing with multiple guests.
> If you have a finite source of entropy in the host, using a PRNG to
> create unique entropy for each guest is certainly better than
> duplicating entropy.

One solution could be to feed host's /dev/urandom to readers of
guests' /dev/urandom.  We could then pass the rare true entropy bits
from host's /dev/hwrng or /dev/random to the guest via
virtio-rng-pci's /dev/hwrng interface in the guest.

If this is a valid idea (host /dev/urandom goes directly to guest's
/dev/urandom), we would need some guest-side surgery, but it shouldn't
be huge work, and would remove several bottlenecks.

Is this a very crazy idea?


		Amit

Re: [Qemu-devel] [PATCH 0/6] add paravirtualization hwrng support

[H. Peter Anvin]

On 10/28/2012 11:23 PM, Amit Shah wrote:
> One solution could be to feed host's /dev/urandom to readers of
> guests' /dev/urandom.  We could then pass the rare true entropy bits
> from host's /dev/hwrng or /dev/random to the guest via
> virtio-rng-pci's /dev/hwrng interface in the guest.
>
> If this is a valid idea (host /dev/urandom goes directly to guest's
> /dev/urandom), we would need some guest-side surgery, but it shouldn't
> be huge work, and would remove several bottlenecks.
>
> Is this a very crazy idea?

It's not crazy, it's just pointless.  You're doing a completely 
unnecessary hypercall to run the PRNG in host space.

	-hpa


-- 
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.

Re: [Qemu-devel] [PATCH 0/6] add paravirtualization hwrng support

[Paolo Bonzini]

Il 26/10/2012 18:09, H. Peter Anvin ha scritto:
> a) it means that the guest *has* to run rngd or a similar engine; if you
> have control over the guests it might be more efficient to run rngd in
> skip-test mode (I don't think that is currently implemented but it
> could/should be) and centralize all testing to the host.
> 
> A skip-test mode would also allow rngd to forward-feed shorter blocks
> than 2500 bytes.

That would be something we can communicate via virtio-rng-pci feature
bits.  Perhaps /dev/hwrng could also expose a need-whitening property in
sysfs.

> b) if the host has no physical hwrng, /dev/hwrng will output nothing at
> all, which is worse than /dev/random in that situation.

Not really, it would just mean that the guest takes more time to gather
entropy, just like if you had no virtio-rng-pci at all.

> If you have rdrand you might just use it in the guest directly, unless
> you have a strong reason (migration?) not to do that.  Either way, for
> rdrand you need whitening similar to what rngd is doing (for *rdseed*
> you do not, but rdseed is not shipping yet.)

Yes, migration is a good reason.

> The startup issue is an interesting problem.  If you have full control
> over the guest it might be best to simply inject some entropy into the
> guest on startup via the initramfs or a disk image; that has its own
> awkwardness too, of course.  The one bit that could potentially be
> solved in Qemu would be an option to "don't start the guest until X
> bytes of entropy have been gathered."
> 
> Overall, I want to emphasize that we don't want to try solve generic
> problems in virtualization space... resource constraints on /dev/random
> is a generic host OS issue for example.

Yes, but the agreed solution right now is that programs should not ask
for more than 32 bytes or so from /dev/random.  Which means /dev/random
is not a suitable backend for virtio-rng-pci as things stand now.

Paolo

Re: [Qemu-devel] [PATCH 0/6] add paravirtualization hwrng support

[H. Peter Anvin]

This is surreal.  Output from /dev/hwrng turns into output for /dev/random... it us guaranteed worse; period, end of story.

I don't know who the "agreement" is with, but it is ridiculous in this case.

As far as the need whitening issue, I discussed that with Ted at Kernel Summit and we came up with an outline for what we can do to improve the current situation.  It is challenging to deal with the patanoia crowd at the same time.

Paolo Bonzini <pbonzini@redhat.com> wrote:

>Il 26/10/2012 18:09, H. Peter Anvin ha scritto:
>> a) it means that the guest *has* to run rngd or a similar engine; if
>you
>> have control over the guests it might be more efficient to run rngd
>in
>> skip-test mode (I don't think that is currently implemented but it
>> could/should be) and centralize all testing to the host.
>> 
>> A skip-test mode would also allow rngd to forward-feed shorter blocks
>> than 2500 bytes.
>
>That would be something we can communicate via virtio-rng-pci feature
>bits.  Perhaps /dev/hwrng could also expose a need-whitening property
>in
>sysfs.
>
>> b) if the host has no physical hwrng, /dev/hwrng will output nothing
>at
>> all, which is worse than /dev/random in that situation.
>
>Not really, it would just mean that the guest takes more time to gather
>entropy, just like if you had no virtio-rng-pci at all.
>
>> If you have rdrand you might just use it in the guest directly,
>unless
>> you have a strong reason (migration?) not to do that.  Either way,
>for
>> rdrand you need whitening similar to what rngd is doing (for *rdseed*
>> you do not, but rdseed is not shipping yet.)
>
>Yes, migration is a good reason.
>
>> The startup issue is an interesting problem.  If you have full
>control
>> over the guest it might be best to simply inject some entropy into
>the
>> guest on startup via the initramfs or a disk image; that has its own
>> awkwardness too, of course.  The one bit that could potentially be
>> solved in Qemu would be an option to "don't start the guest until X
>> bytes of entropy have been gathered."
>> 
>> Overall, I want to emphasize that we don't want to try solve generic
>> problems in virtualization space... resource constraints on
>/dev/random
>> is a generic host OS issue for example.
>
>Yes, but the agreed solution right now is that programs should not ask
>for more than 32 bytes or so from /dev/random.  Which means /dev/random
>is not a suitable backend for virtio-rng-pci as things stand now.
>
>Paolo

-- 
Sent from my mobile phone. Please excuse brevity and lack of formatting.

Re: [Qemu-devel] [PATCH 0/6] add paravirtualization hwrng support

[Paolo Bonzini]

Il 26/10/2012 21:07, H. Peter Anvin ha scritto:
> This is surreal.  Output from /dev/hwrng turns into output for /dev/random... it us guaranteed worse; period, end of story.

Isn't that exactly what happens in bare-metal?  hwrng -> rngd -> random.  Instead here
we'd have, host hwrng -> virtio-rng-pci -> guest hwrng -> guest rngd -> guest random.

The only difference is that you paravirtualize access to the host hwrng to a) distribute
entropy to multiple guests; b) support migration across hosts with different CPUs and
hardware.

> I don't know who the "agreement" is with, but it is ridiculous in this case.

man 4 random:

       While some safety margin above that minimum is reasonable, as a guard against
       flaws  in the CPRNG algorithm, no cryptographic primitive available today can
       hope to promise more than 256 bits of security, so if any program reads  more
       than  256  bits (32 bytes) from the kernel random pool per invocation, or per
       reasonable reseed interval (not less than one minute), that should  be  taken
       as a sign that its cryptography is not skilfully implemented.

Paolo

Re: [Qemu-devel] [PATCH 0/6] add paravirtualization hwrng support

[H. Peter Anvin]

That statement is pretty toxic... I wonder where it came from.  It is at best horribly misleading and actively encourages dangerous behaviours even for the cases where it isn't actively wrong.

Paolo Bonzini <pbonzini@redhat.com> wrote:

>Il 26/10/2012 21:07, H. Peter Anvin ha scritto:
>> This is surreal.  Output from /dev/hwrng turns into output for
>/dev/random... it us guaranteed worse; period, end of story.
>
>Isn't that exactly what happens in bare-metal?  hwrng -> rngd ->
>random.  Instead here
>we'd have, host hwrng -> virtio-rng-pci -> guest hwrng -> guest rngd ->
>guest random.
>
>The only difference is that you paravirtualize access to the host hwrng
>to a) distribute
>entropy to multiple guests; b) support migration across hosts with
>different CPUs and
>hardware.
>
>> I don't know who the "agreement" is with, but it is ridiculous in
>this case.
>
>man 4 random:
>
>While some safety margin above that minimum is reasonable, as a guard
>against
>flaws  in the CPRNG algorithm, no cryptographic primitive available
>today can
>hope to promise more than 256 bits of security, so if any program reads
> more
>than  256  bits (32 bytes) from the kernel random pool per invocation,
>or per
>reasonable reseed interval (not less than one minute), that should  be 
>taken
>       as a sign that its cryptography is not skilfully implemented.
>
>Paolo

-- 
Sent from my mobile phone. Please excuse brevity and lack of formatting.

Re: [Qemu-devel] [PATCH 0/6] add paravirtualization hwrng support

[H. Peter Anvin]

On 10/26/2012 12:51 PM, Paolo Bonzini wrote:
> Il 26/10/2012 21:07, H. Peter Anvin ha scritto:
>> This is surreal.  Output from /dev/hwrng turns into output for /dev/random... it us guaranteed worse; period, end of story.
> 
> Isn't that exactly what happens in bare-metal?  hwrng -> rngd -> random.  Instead here
> we'd have, host hwrng -> virtio-rng-pci -> guest hwrng -> guest rngd -> guest random.
> 
> The only difference is that you paravirtualize access to the host hwrng to a) distribute
> entropy to multiple guests; b) support migration across hosts with different CPUs and
> hardware.

First, hwrng is only one of the sources used by rngd.  It can also
(currently) use RDRAND or TPM; additional sources are likely to be added
in the future.

Second, the harvesting of environmental noise -- timings -- is not as
good in a VM as on plain hardware, so for the no-hwrng case it is better
for this to be done in the host than in the VM.

	-hpa

Re: [Qemu-devel] [PATCH 0/6] add paravirtualization hwrng support

[Paolo Bonzini]

Il 26/10/2012 22:29, H. Peter Anvin ha scritto:
>>> This is surreal.  Output from /dev/hwrng turns into output for /dev/random... it us guaranteed worse; period, end of story.
>> > 
>> > Isn't that exactly what happens in bare-metal?  hwrng -> rngd -> random.  Instead here
>> > we'd have, host hwrng -> virtio-rng-pci -> guest hwrng -> guest rngd -> guest random.
>> > 
>> > The only difference is that you paravirtualize access to the host hwrng to a) distribute
>> > entropy to multiple guests; b) support migration across hosts with different CPUs and
>> > hardware.
> First, hwrng is only one of the sources used by rngd.  It can also
> (currently) use RDRAND or TPM; additional sources are likely to be added
> in the future.
> 
> Second, the harvesting of environmental noise -- timings -- is not as
> good in a VM as on plain hardware, so for the no-hwrng case it is better
> for this to be done in the host than in the VM.

Neither of these make /dev/random with virtio-rng-pci worse than without
(as would be the case if you fed /dev/urandom).  And migration works.
This, and avoiding denial of service for the host's /dev/random, is all
I care about at this time.

There is always time to change defaults to something better.

Paolo

Re: [Qemu-devel] [PATCH 0/6] add paravirtualization hwrng support

[H. Peter Anvin]

On 10/29/2012 01:45 AM, Paolo Bonzini wrote:
>> First, hwrng is only one of the sources used by rngd.  It can also
>> (currently) use RDRAND or TPM; additional sources are likely to be added
>> in the future.
>>
>> Second, the harvesting of environmental noise -- timings -- is not as
>> good in a VM as on plain hardware, so for the no-hwrng case it is better
>> for this to be done in the host than in the VM.
>
> Neither of these make /dev/random with virtio-rng-pci worse than without
> (as would be the case if you fed /dev/urandom).  And migration works.
 >
> This, and avoiding denial of service for the host's /dev/random, is all
> I care about at this time.
>
> There is always time to change defaults to something better.

Your logic are roughly on the level with the people who caused the 
Debian bug.  You are proposing something utterly reckless.  Sorry, 
please stop.

	-hpa

-- 
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.

Re: [Qemu-devel] [PATCH 0/6] add paravirtualization hwrng support

[H. Peter Anvin]

On 10/29/2012 01:45 AM, Paolo Bonzini wrote:
> Il 26/10/2012 22:29, H. Peter Anvin ha scritto:
>>>> This is surreal.  Output from /dev/hwrng turns into output for /dev/random... it us guaranteed worse; period, end of story.
>>>>
>>>> Isn't that exactly what happens in bare-metal?  hwrng -> rngd -> random.  Instead here
>>>> we'd have, host hwrng -> virtio-rng-pci -> guest hwrng -> guest rngd -> guest random.
>>>>
>>>> The only difference is that you paravirtualize access to the host hwrng to a) distribute
>>>> entropy to multiple guests; b) support migration across hosts with different CPUs and
>>>> hardware.
>> First, hwrng is only one of the sources used by rngd.  It can also
>> (currently) use RDRAND or TPM; additional sources are likely to be added
>> in the future.
>>
>> Second, the harvesting of environmental noise -- timings -- is not as
>> good in a VM as on plain hardware, so for the no-hwrng case it is better
>> for this to be done in the host than in the VM.
>
> Neither of these make /dev/random with virtio-rng-pci worse than without
> (as would be the case if you fed /dev/urandom).  And migration works.
> This, and avoiding denial of service for the host's /dev/random, is all
> I care about at this time.
>
> There is always time to change defaults to something better.
>

Let me be more specific.

First of all, feeding /dev/urandom to the guest is dangerous -- you are 
feeding it PRNG contents but telling it that it is real entropy.  This 
is a security hole.

Second of all, you're doing something pointless: you are still 
exhausting the entropy pool on the host at the same rate, and all you 
end up with is something that isn't what you want.  You still have the 
same DoS on the host /dev/random that you're worried about.

Third, you're doing something inefficient: you're running a PRNG in the 
host which could be run more efficiently in guest space.

 From an Intel perspective I guess I should be happy, as it functionally 
would mean that unless you have RDRAND in the host you're insecure, but 
I'd much rather see the Right Thing done.

	-hpa

-- 
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.

Re: [Qemu-devel] [PATCH 0/6] add paravirtualization hwrng support

[Paolo Bonzini]

Il 30/10/2012 05:43, H. Peter Anvin ha scritto:
> Let me be more specific.
> 
> First of all, feeding /dev/urandom to the guest is dangerous -- you are
> feeding it PRNG contents but telling it that it is real entropy.  This
> is a security hole.
> 
> Second of all, you're doing something pointless: you are still
> exhausting the entropy pool on the host at the same rate, and all you
> end up with is something that isn't what you want.  You still have the
> same DoS on the host /dev/random that you're worried about.
> 
> Third, you're doing something inefficient: you're running a PRNG in the
> host which could be run more efficiently in guest space.

Either you're not reading what I wrote, or you're confusing me with
someone else.

I *never* mentioned passing /dev/urandom, and in fact I explained to
Anthony that it is wrong.  Please take a look at
http://permalink.gmane.org/gmane.comp.emulators.qemu/178123

What I said that passing /dev/hwrng or rdrand would:

- not make /dev/random with virtio-rng-pci worse than without

- make migration working

- avoiding denial of service for the host's /dev/random


> From an Intel perspective I guess I should be happy, as it functionally
> would mean that unless you have RDRAND in the host you're insecure, but
> I'd much rather see the Right Thing done.

:)

Paolo

Re: [Qemu-devel] [PATCH 0/6] add paravirtualization hwrng support

[H. Peter Anvin]

On 10/30/2012 02:05 AM, Paolo Bonzini wrote:
> Either you're not reading what I wrote, or you're confusing me with
> someone else.

My apologies, you are indeed correct.  I misinterpreted your emails,
probably because I got you confused with someone else.

> I *never* mentioned passing /dev/urandom, and in fact I explained to
> Anthony that it is wrong.  Please take a look at
> http://permalink.gmane.org/gmane.comp.emulators.qemu/178123
> 
> What I said that passing /dev/hwrng or rdrand would:
> 
> - not make /dev/random with virtio-rng-pci worse than without

It wouldn't, but it would make virtio-rng-pci a potential noop on a
system where it could genuinely do better.

> - make migration working
> 
> - avoiding denial of service for the host's /dev/random

However, it means that if there is an rngd-readable source on the host
(e.g. TPM, DRNG) then the guest cannot take advantage of it; if it
accesses /dev/random then it would be able to.  This is particularly
toxic if you turn off DRNG to the host in the name of migration; the
DRNG is a very high bandwidth source which is processed directly by rngd
since there is no point in doing a detour via /dev/hwrng in the kernel.
 As such, with your proposed version you would take one of the best
possible situations and turn it into the worst possible situation.

Furthermore, you are in many ways still causing a DoS on the host, since
you are eating up entropy that would otherwise be fed into /dev/random.
 So there are cases where the situation is much worse with /dev/hwrng
than with /dev/random.

	-hpa

Re: [Qemu-devel] [PATCH 0/6] add paravirtualization hwrng support

[Paolo Bonzini]

Il 30/10/2012 22:11, H. Peter Anvin ha scritto:
>> What I said that passing /dev/hwrng or rdrand would:
>>
>> - not make /dev/random with virtio-rng-pci worse than without
> 
> It wouldn't, but it would make virtio-rng-pci a potential noop on a
> system where it could genuinely do better.

True, but it can be fixed later without changing the guest hardware.  It
will just perform better, transparently.

>> - make migration working
>>
>> - avoiding denial of service for the host's /dev/random
> 
> However, it means that if there is an rngd-readable source on the host
> (e.g. TPM, DRNG) then the guest cannot take advantage of it;

Right.  A DRNG backend for virtio-rng-pci however can easily be
implemented in QEMU.  In this case you do have a slowish roundtrip to
the host, but only if you need to migrate to older hosts (hopefully not
a concern in smaller deployments---or in a few years).

Related to this, rdrand's "entropy content" in the worst case will be
only 1/255th of the data it produces: Intel documents that one 256-bit
seed will result in up to 1022 64-bit random numbers.  Yet, it is good
enough to drive rngd.  Would it make sense for QEMU to implement the
same kind of stretching of /dev/random data, to avoid depleting the
host's entropy pool too fast?

> Furthermore, you are in many ways still causing a DoS on the host, since
> you are eating up entropy that would otherwise be fed into /dev/random.
> So there are cases where the situation is much worse with /dev/hwrng
> than with /dev/random.

How is it worse, at least from the host point of view?  At least the
entropy bits collected by the kernel will remain untouched.  From the
guest point of view it doesn't have to be too good, as long as it's not
/dev/urandom.

If you know you won't run any task that requires /dev/random on the
host, and/or you somehow trust the guests, you certainly can pass it to
the guests and disable the test in rngd.  However, the same
configuration needs to be applied to all hosts, because the
test-disabled feature bit must not go from 1 to 0 over migration.

Paolo

Re: [Qemu-devel] [PATCH 0/6] add paravirtualization hwrng support

[H. Peter Anvin]

On 10/31/2012 12:29 AM, Paolo Bonzini wrote:
>
> Related to this, rdrand's "entropy content" in the worst case will be
> only 1/255th of the data it produces: Intel documents that one 256-bit
> seed will result in up to 1022 64-bit random numbers.  Yet, it is good
> enough to drive rngd.  Would it make sense for QEMU to implement the
> same kind of stretching of /dev/random data, to avoid depleting the
> host's entropy pool too fast?
>

Absolutely not; in fact, we have to do data reduction in rngd for 
exactly this reason (and a Qemu backend would have to do the same). 
There is a new RDSEED instruction in newer CPUs to correct this.

	-hpa

-- 
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.

Re: [Qemu-devel] [PATCH 0/6] add paravirtualization hwrng support

[Paolo Bonzini]

Il 31/10/2012 15:15, H. Peter Anvin ha scritto:
>>
>> Related to this, rdrand's "entropy content" in the worst case will be
>> only 1/255th of the data it produces: Intel documents that one 256-bit
>> seed will result in up to 1022 64-bit random numbers.  Yet, it is good
>> enough to drive rngd.  Would it make sense for QEMU to implement the
>> same kind of stretching of /dev/random data, to avoid depleting the
>> host's entropy pool too fast?
>>
> 
> Absolutely not; in fact, we have to do data reduction in rngd for
> exactly this reason (and a Qemu backend would have to do the same).
> There is a new RDSEED instruction in newer CPUs to correct this.

Ok, thanks.  At least I asked. :)

Paolo

Re: [Qemu-devel] [PATCH 0/6] add paravirtualization hwrng support

[Paolo Bonzini]

Il 26/10/2012 17:42, Anthony Liguori ha scritto:
>> Maybe rdrand, but that's just a chardev---so why isn't this enough:
>>
>>   -chardev file,source=on,path=/dev/hwrng,id=chr0  -device virtio-rng-pci,file=chr0
>>   -chardev rdrand,id=chr0                          -device virtio-rng-pci,file=chr0
>>   -chardev socket,host=localhost,port=1024,id=chr0 -device virtio-rng-pci,rng=chr0,egd=on
>>
>> (which I suggested in my reply to Amit)?
> 
> I don't like overloading chardev to representate any !block device
> backend which is what I fear we're doing here.

Like -chardev msmouse you mean? ;)

> EGD is more than just a dumb pipe of data too.  It's got a way to query
> available entropy.  I have a strong suspicion that over time, we'll add
> methods to virtio-rng to query available entropy.  That would mean
> adding a backend specific ioctl to the chardev layer which is pretty
> ugly.
> 
> The overhead of creating a separate backend to begin with is extremely
> small.  We're talking about dozens of lines of code.  So I don't see
> what the problem is.

If you just make rng-random take a chardev, I have no problem with the
series.

Paolo

Re: [Qemu-devel] [PATCH 0/6] add paravirtualization hwrng support

[Amit Shah]

On (Fri) 26 Oct 2012 [09:43:34], Anthony Liguori wrote:
> Hi,
> 
> This series implements the backend and frontend infrastructure for virtio-rng.
> This is similar to previous series sent out by both Amit and myself although it
> has been trimmed down considerably.
> 
> In terms of backends, a file and EGD backend are supported.  The file defaults
> to /dev/random based on the feedback from Peter.  It's still possible to support
> /dev/urandom though as an entropy source by overriding the file name.
> 
> I think this series is ready to merge.

I have a small diff to this series that I had merged in mine.  Please
apply to your tree as well.

(Gets rid of savevm/loadvm complexities by using the new
virtqueue_get_avail_bytes(), fixes typos/whitespace in rng.h)

diff --git b/hw/virtio-rng.c a/hw/virtio-rng.c
index b7fb5e9..290b2b6 100644
--- b/hw/virtio-rng.c
+++ a/hw/virtio-rng.c
@@ -22,14 +22,9 @@ 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 +37,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,27 +59,16 @@ 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);
-
-    /*
-     * 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)
@@ -96,7 +76,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 +92,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 +102,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 +134,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);
 
diff --git b/include/qemu/rng.h a/include/qemu/rng.h
index 7e9d672..9836463 100644
--- b/include/qemu/rng.h
+++ a/include/qemu/rng.h
@@ -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



		Amit