All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 0/4] virtio: Add vhost-user based RNG
@ 2021-07-10  0:59 Mathieu Poirier
  2021-07-10  0:59 ` [PATCH v3 1/4] vhost-user-rng: Add vhost-user-rng implementation Mathieu Poirier
                   ` (4 more replies)
  0 siblings, 5 replies; 15+ messages in thread
From: Mathieu Poirier @ 2021-07-10  0:59 UTC (permalink / raw)
  To: mst, alex.bennee; +Cc: qemu-devel

This sets adds a vhost-user based random number generator (RNG),
similar to what has been done for i2c and virtiofsd, with the
implementation following the patterns already set forth in those.

Applies cleanly to git://git.qemu.org/qemu.git master(05de778b5b8a).

Thanks,
Mathieu

Mathieu Poirier (4):
  vhost-user-rng: Add vhost-user-rng implementation
  vhost-user-rng-pci: Add vhost-user-rng-pci implementation
  vhost-user-rng: backend: Add RNG vhost-user daemon implementation
  docs: Add documentation for vhost based RNG implementation

----
New for V3:
- Rebased to latest master branch.
- Fixed documentation warning.
- Updated call to vhost_dev_init() to match new signature.
- Dropped MAINTAINERS patch since it was already applied. 

 docs/tools/index.rst                     |   1 +
 docs/tools/vhost-user-rng.rst            |  74 +++++
 hw/virtio/Kconfig                        |   5 +
 hw/virtio/meson.build                    |   2 +
 hw/virtio/vhost-user-rng-pci.c           |  79 +++++
 hw/virtio/vhost-user-rng.c               | 294 +++++++++++++++++
 include/hw/virtio/vhost-user-rng.h       |  33 ++
 tools/meson.build                        |   8 +
 tools/vhost-user-rng/50-qemu-rng.json.in |   5 +
 tools/vhost-user-rng/main.c              | 403 +++++++++++++++++++++++
 tools/vhost-user-rng/meson.build         |  10 +
 11 files changed, 914 insertions(+)
 create mode 100644 docs/tools/vhost-user-rng.rst
 create mode 100644 hw/virtio/vhost-user-rng-pci.c
 create mode 100644 hw/virtio/vhost-user-rng.c
 create mode 100644 include/hw/virtio/vhost-user-rng.h
 create mode 100644 tools/vhost-user-rng/50-qemu-rng.json.in
 create mode 100644 tools/vhost-user-rng/main.c
 create mode 100644 tools/vhost-user-rng/meson.build

-- 
2.25.1



^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH v3 1/4] vhost-user-rng: Add vhost-user-rng implementation
  2021-07-10  0:59 [PATCH v3 0/4] virtio: Add vhost-user based RNG Mathieu Poirier
@ 2021-07-10  0:59 ` Mathieu Poirier
  2021-07-21  8:52   ` Alex Bennée
  2021-07-10  0:59 ` [PATCH v3 2/4] vhost-user-rng-pci: Add vhost-user-rng-pci implementation Mathieu Poirier
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 15+ messages in thread
From: Mathieu Poirier @ 2021-07-10  0:59 UTC (permalink / raw)
  To: mst, alex.bennee; +Cc: qemu-devel

Following in the footsteps of what whas done for vhost-user-i2c
and virtiofsd, introduce a random number generator (RNG) backend
that communicates with a vhost-user server to retrieve entropy.
That way another VMM could be using the same vhost-user daemon and
avoid having to write yet another RNG driver.

Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
---
 hw/virtio/Kconfig                  |   5 +
 hw/virtio/meson.build              |   1 +
 hw/virtio/vhost-user-rng.c         | 294 +++++++++++++++++++++++++++++
 include/hw/virtio/vhost-user-rng.h |  33 ++++
 4 files changed, 333 insertions(+)
 create mode 100644 hw/virtio/vhost-user-rng.c
 create mode 100644 include/hw/virtio/vhost-user-rng.h

diff --git a/hw/virtio/Kconfig b/hw/virtio/Kconfig
index 0eda25c4e1bf..69066ab14e6d 100644
--- a/hw/virtio/Kconfig
+++ b/hw/virtio/Kconfig
@@ -58,3 +58,8 @@ config VIRTIO_MEM
     depends on LINUX
     depends on VIRTIO_MEM_SUPPORTED
     select MEM_DEVICE
+
+config VHOST_USER_RNG
+    bool
+    default y
+    depends on VIRTIO && VHOST_USER
diff --git a/hw/virtio/meson.build b/hw/virtio/meson.build
index fbff9bc9d4de..e386791f2a05 100644
--- a/hw/virtio/meson.build
+++ b/hw/virtio/meson.build
@@ -25,6 +25,7 @@ virtio_ss.add(when: 'CONFIG_VHOST_USER_VSOCK', if_true: files('vhost-user-vsock.
 virtio_ss.add(when: 'CONFIG_VIRTIO_RNG', if_true: files('virtio-rng.c'))
 virtio_ss.add(when: 'CONFIG_VIRTIO_IOMMU', if_true: files('virtio-iommu.c'))
 virtio_ss.add(when: 'CONFIG_VIRTIO_MEM', if_true: files('virtio-mem.c'))
+virtio_ss.add(when: 'CONFIG_VHOST_USER_RNG', if_true: files('vhost-user-rng.c'))
 
 virtio_pci_ss = ss.source_set()
 virtio_pci_ss.add(when: 'CONFIG_VHOST_VSOCK', if_true: files('vhost-vsock-pci.c'))
diff --git a/hw/virtio/vhost-user-rng.c b/hw/virtio/vhost-user-rng.c
new file mode 100644
index 000000000000..3825266bdf46
--- /dev/null
+++ b/hw/virtio/vhost-user-rng.c
@@ -0,0 +1,294 @@
+/*
+ * Vhost-user RNG virtio device
+ *
+ * Copyright (c) 2021 Mathieu Poirier <mathieu.poirier@linaro.org>
+ *
+ * Implementation seriously tailored on vhost-user-i2c.c
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include "qemu/osdep.h"
+#include "qapi/error.h"
+#include "hw/qdev-properties.h"
+#include "hw/virtio/virtio-bus.h"
+#include "hw/virtio/vhost-user-rng.h"
+#include "qemu/error-report.h"
+#include "standard-headers/linux/virtio_ids.h"
+
+static void vu_rng_start(VirtIODevice *vdev)
+{
+    VHostUserRNG *rng = VHOST_USER_RNG(vdev);
+    BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(vdev)));
+    VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
+    int ret;
+    int i;
+
+    if (!k->set_guest_notifiers) {
+        error_report("binding does not support guest notifiers");
+        return;
+    }
+
+    ret = vhost_dev_enable_notifiers(&rng->vhost_dev, vdev);
+    if (ret < 0) {
+        error_report("Error enabling host notifiers: %d", -ret);
+        return;
+    }
+
+    ret = k->set_guest_notifiers(qbus->parent, rng->vhost_dev.nvqs, true);
+    if (ret < 0) {
+        error_report("Error binding guest notifier: %d", -ret);
+        goto err_host_notifiers;
+    }
+
+    rng->vhost_dev.acked_features = vdev->guest_features;
+    ret = vhost_dev_start(&rng->vhost_dev, vdev);
+    if (ret < 0) {
+        error_report("Error starting vhost-user-rng: %d", -ret);
+        goto err_guest_notifiers;
+    }
+
+    /*
+     * guest_notifier_mask/pending not used yet, so just unmask
+     * everything here. virtio-pci will do the right thing by
+     * enabling/disabling irqfd.
+     */
+    for (i = 0; i < rng->vhost_dev.nvqs; i++) {
+        vhost_virtqueue_mask(&rng->vhost_dev, vdev, i, false);
+    }
+
+    return;
+
+err_guest_notifiers:
+    k->set_guest_notifiers(qbus->parent, rng->vhost_dev.nvqs, false);
+err_host_notifiers:
+    vhost_dev_disable_notifiers(&rng->vhost_dev, vdev);
+}
+
+static void vu_rng_stop(VirtIODevice *vdev)
+{
+    VHostUserRNG *rng = VHOST_USER_RNG(vdev);
+    BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(vdev)));
+    VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
+    int ret;
+
+    if (!k->set_guest_notifiers) {
+        return;
+    }
+
+    vhost_dev_stop(&rng->vhost_dev, vdev);
+
+    ret = k->set_guest_notifiers(qbus->parent, rng->vhost_dev.nvqs, false);
+    if (ret < 0) {
+        error_report("vhost guest notifier cleanup failed: %d", ret);
+        return;
+    }
+
+    vhost_dev_disable_notifiers(&rng->vhost_dev, vdev);
+}
+
+static void vu_rng_set_status(VirtIODevice *vdev, uint8_t status)
+{
+    VHostUserRNG *rng = VHOST_USER_RNG(vdev);
+    bool should_start = status & VIRTIO_CONFIG_S_DRIVER_OK;
+
+    if (!vdev->vm_running) {
+        should_start = false;
+    }
+
+    if (rng->vhost_dev.started == should_start) {
+        return;
+    }
+
+    if (should_start) {
+        vu_rng_start(vdev);
+    } else {
+        vu_rng_stop(vdev);
+    }
+}
+
+static uint64_t vu_rng_get_features(VirtIODevice *vdev,
+                                    uint64_t requested_features, Error **errp)
+{
+    /* No feature bits used yet */
+    return requested_features;
+}
+
+static void vu_rng_handle_output(VirtIODevice *vdev, VirtQueue *vq)
+{
+    /*
+     * Not normally called; it's the daemon that handles the queue;
+     * however virtio's cleanup path can call this.
+     */
+}
+
+static void vu_rng_guest_notifier_mask(VirtIODevice *vdev, int idx, bool mask)
+{
+    VHostUserRNG *rng = VHOST_USER_RNG(vdev);
+
+    vhost_virtqueue_mask(&rng->vhost_dev, vdev, idx, mask);
+}
+
+static bool vu_rng_guest_notifier_pending(VirtIODevice *vdev, int idx)
+{
+    VHostUserRNG *rng = VHOST_USER_RNG(vdev);
+
+    return vhost_virtqueue_pending(&rng->vhost_dev, idx);
+}
+
+static void vu_rng_connect(DeviceState *dev)
+{
+    VirtIODevice *vdev = VIRTIO_DEVICE(dev);
+    VHostUserRNG *rng = VHOST_USER_RNG(vdev);
+
+    if (rng->connected) {
+        return;
+    }
+
+    rng->connected = true;
+
+    /* restore vhost state */
+    if (virtio_device_started(vdev, vdev->status)) {
+        vu_rng_start(vdev);
+    }
+}
+
+static void vu_rng_disconnect(DeviceState *dev)
+{
+    VirtIODevice *vdev = VIRTIO_DEVICE(dev);
+    VHostUserRNG *rng = VHOST_USER_RNG(vdev);
+
+    if (!rng->connected) {
+        return;
+    }
+
+    rng->connected = false;
+
+    if (rng->vhost_dev.started) {
+        vu_rng_stop(vdev);
+    }
+}
+
+static void vu_rng_event(void *opaque, QEMUChrEvent event)
+{
+    DeviceState *dev = opaque;
+
+    switch (event) {
+    case CHR_EVENT_OPENED:
+        vu_rng_connect(dev);
+        break;
+    case CHR_EVENT_CLOSED:
+        vu_rng_disconnect(dev);
+        break;
+    case CHR_EVENT_BREAK:
+    case CHR_EVENT_MUX_IN:
+    case CHR_EVENT_MUX_OUT:
+        /* Ignore */
+        break;
+    }
+}
+
+static void vu_rng_device_realize(DeviceState *dev, Error **errp)
+{
+    VirtIODevice *vdev = VIRTIO_DEVICE(dev);
+    VHostUserRNG *rng = VHOST_USER_RNG(dev);
+    int ret;
+
+    if (!rng->chardev.chr) {
+        error_setg(errp, "missing chardev");
+        return;
+    }
+
+    if (!vhost_user_init(&rng->vhost_user, &rng->chardev, errp)) {
+        return;
+    }
+
+    virtio_init(vdev, "vhost-user-rng", VIRTIO_ID_RNG, 0);
+
+    rng->req_vq = virtio_add_queue(vdev, 4, vu_rng_handle_output);
+    if (!rng->req_vq) {
+        error_setg_errno(errp, -1, "virtio_add_queue() failed");
+        goto virtio_add_queue_failed;
+    }
+
+    rng->vhost_dev.nvqs = 1;
+    rng->vhost_dev.vqs = g_new0(struct vhost_virtqueue, rng->vhost_dev.nvqs);
+    if (!rng->vhost_dev.vqs) {
+        error_setg_errno(errp, -1, "memory allocation failed");
+        goto vhost_dev_init_failed;
+    }
+
+    ret = vhost_dev_init(&rng->vhost_dev, &rng->vhost_user,
+                         VHOST_BACKEND_TYPE_USER, 0, errp);
+    if (ret < 0) {
+        error_setg_errno(errp, -ret, "vhost_dev_init() failed");
+        goto vhost_dev_init_failed;
+    }
+
+    qemu_chr_fe_set_handlers(&rng->chardev, NULL, NULL, vu_rng_event, NULL,
+                             dev, NULL, true);
+
+    return;
+
+vhost_dev_init_failed:
+    virtio_delete_queue(rng->req_vq);
+virtio_add_queue_failed:
+    virtio_cleanup(vdev);
+    vhost_user_cleanup(&rng->vhost_user);
+}
+
+static void vu_rng_device_unrealize(DeviceState *dev)
+{
+    VirtIODevice *vdev = VIRTIO_DEVICE(dev);
+    VHostUserRNG *rng = VHOST_USER_RNG(dev);
+
+    vu_rng_set_status(vdev, 0);
+
+    vhost_dev_cleanup(&rng->vhost_dev);
+    g_free(rng->vhost_dev.vqs);
+    rng->vhost_dev.vqs = NULL;
+    virtio_delete_queue(rng->req_vq);
+    virtio_cleanup(vdev);
+    vhost_user_cleanup(&rng->vhost_user);
+}
+
+static const VMStateDescription vu_rng_vmstate = {
+    .name = "vhost-user-rng",
+    .unmigratable = 1,
+};
+
+static Property vu_rng_properties[] = {
+    DEFINE_PROP_CHR("chardev", VHostUserRNG, chardev),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
+static void vu_rng_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+    VirtioDeviceClass *vdc = VIRTIO_DEVICE_CLASS(klass);
+
+    device_class_set_props(dc, vu_rng_properties);
+    dc->vmsd = &vu_rng_vmstate;
+    set_bit(DEVICE_CATEGORY_INPUT, dc->categories);
+
+    vdc->realize = vu_rng_device_realize;
+    vdc->unrealize = vu_rng_device_unrealize;
+    vdc->get_features = vu_rng_get_features;
+    vdc->set_status = vu_rng_set_status;
+    vdc->guest_notifier_mask = vu_rng_guest_notifier_mask;
+    vdc->guest_notifier_pending = vu_rng_guest_notifier_pending;
+}
+
+static const TypeInfo vu_rng_info = {
+    .name = TYPE_VHOST_USER_RNG,
+    .parent = TYPE_VIRTIO_DEVICE,
+    .instance_size = sizeof(VHostUserRNG),
+    .class_init = vu_rng_class_init,
+};
+
+static void vu_rng_register_types(void)
+{
+    type_register_static(&vu_rng_info);
+}
+
+type_init(vu_rng_register_types)
diff --git a/include/hw/virtio/vhost-user-rng.h b/include/hw/virtio/vhost-user-rng.h
new file mode 100644
index 000000000000..071539996d1d
--- /dev/null
+++ b/include/hw/virtio/vhost-user-rng.h
@@ -0,0 +1,33 @@
+/*
+ * Vhost-user RNG virtio device
+ *
+ * Copyright (c) 2021 Mathieu Poirier <mathieu.poirier@linaro.org>
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#ifndef _QEMU_VHOST_USER_RNG_H
+#define _QEMU_VHOST_USER_RNG_H
+
+#include "hw/virtio/virtio.h"
+#include "hw/virtio/vhost.h"
+#include "hw/virtio/vhost-user.h"
+#include "chardev/char-fe.h"
+
+#define TYPE_VHOST_USER_RNG "vhost-user-rng"
+OBJECT_DECLARE_SIMPLE_TYPE(VHostUserRNG, VHOST_USER_RNG)
+
+struct VHostUserRNG {
+    /*< private >*/
+    VirtIODevice parent;
+    CharBackend chardev;
+    struct vhost_virtqueue *vhost_vq;
+    struct vhost_dev vhost_dev;
+    VhostUserState vhost_user;
+    VirtQueue *req_vq;
+    bool connected;
+
+    /*< public >*/
+};
+
+#endif /* _QEMU_VHOST_USER_RNG_H */
-- 
2.25.1



^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v3 2/4] vhost-user-rng-pci: Add vhost-user-rng-pci implementation
  2021-07-10  0:59 [PATCH v3 0/4] virtio: Add vhost-user based RNG Mathieu Poirier
  2021-07-10  0:59 ` [PATCH v3 1/4] vhost-user-rng: Add vhost-user-rng implementation Mathieu Poirier
@ 2021-07-10  0:59 ` Mathieu Poirier
  2021-07-21  8:56   ` Alex Bennée
  2021-07-21 20:15   ` Alex Bennée
  2021-07-10  0:59 ` [PATCH v3 3/4] vhost-user-rng: backend: Add RNG vhost-user daemon implementation Mathieu Poirier
                   ` (2 subsequent siblings)
  4 siblings, 2 replies; 15+ messages in thread
From: Mathieu Poirier @ 2021-07-10  0:59 UTC (permalink / raw)
  To: mst, alex.bennee; +Cc: qemu-devel

This patch provides a PCI bus interface to the vhost-user-rng backed.
The implentation is similar to what was done for vhost-user-i2c-pci and
vhost-user-fs-pci.

Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
---
 hw/virtio/meson.build          |  1 +
 hw/virtio/vhost-user-rng-pci.c | 79 ++++++++++++++++++++++++++++++++++
 2 files changed, 80 insertions(+)
 create mode 100644 hw/virtio/vhost-user-rng-pci.c

diff --git a/hw/virtio/meson.build b/hw/virtio/meson.build
index e386791f2a05..1430b370e64d 100644
--- a/hw/virtio/meson.build
+++ b/hw/virtio/meson.build
@@ -26,6 +26,7 @@ virtio_ss.add(when: 'CONFIG_VIRTIO_RNG', if_true: files('virtio-rng.c'))
 virtio_ss.add(when: 'CONFIG_VIRTIO_IOMMU', if_true: files('virtio-iommu.c'))
 virtio_ss.add(when: 'CONFIG_VIRTIO_MEM', if_true: files('virtio-mem.c'))
 virtio_ss.add(when: 'CONFIG_VHOST_USER_RNG', if_true: files('vhost-user-rng.c'))
+virtio_ss.add(when: ['CONFIG_VHOST_USER_RNG', 'CONFIG_VIRTIO_PCI'], if_true: files('vhost-user-rng-pci.c'))
 
 virtio_pci_ss = ss.source_set()
 virtio_pci_ss.add(when: 'CONFIG_VHOST_VSOCK', if_true: files('vhost-vsock-pci.c'))
diff --git a/hw/virtio/vhost-user-rng-pci.c b/hw/virtio/vhost-user-rng-pci.c
new file mode 100644
index 000000000000..ffff2de39fd4
--- /dev/null
+++ b/hw/virtio/vhost-user-rng-pci.c
@@ -0,0 +1,79 @@
+/*
+ * Vhost-user RNG virtio device PCI glue
+ *
+ * Copyright (c) 2021 Mathieu Poirier <mathieu.poirier@linaro.org>
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include "qemu/osdep.h"
+#include "hw/qdev-properties.h"
+#include "hw/virtio/vhost-user-rng.h"
+#include "virtio-pci.h"
+
+struct VHostUserRNGPCI {
+    VirtIOPCIProxy parent_obj;
+    VHostUserRNG vdev;
+};
+
+typedef struct VHostUserRNGPCI VHostUserRNGPCI;
+
+#define TYPE_VHOST_USER_RNG_PCI "vhost-user-rng-pci-base"
+
+DECLARE_INSTANCE_CHECKER(VHostUserRNGPCI, VHOST_USER_RNG_PCI,
+                         TYPE_VHOST_USER_RNG_PCI)
+
+static Property vhost_user_rng_pci_properties[] = {
+    DEFINE_PROP_UINT32("vectors", VirtIOPCIProxy, nvectors,
+                       DEV_NVECTORS_UNSPECIFIED),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
+static void vhost_user_rng_pci_realize(VirtIOPCIProxy *vpci_dev, Error **errp)
+{
+    VHostUserRNGPCI *dev = VHOST_USER_RNG_PCI(vpci_dev);
+    DeviceState *vdev = DEVICE(&dev->vdev);
+
+    if (vpci_dev->nvectors == DEV_NVECTORS_UNSPECIFIED) {
+        vpci_dev->nvectors = 1;
+    }
+
+    qdev_realize(vdev, BUS(&vpci_dev->bus), errp);
+}
+
+static void vhost_user_rng_pci_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+    VirtioPCIClass *k = VIRTIO_PCI_CLASS(klass);
+    PCIDeviceClass *pcidev_k = PCI_DEVICE_CLASS(klass);
+    k->realize = vhost_user_rng_pci_realize;
+    set_bit(DEVICE_CATEGORY_INPUT, dc->categories);
+    device_class_set_props(dc, vhost_user_rng_pci_properties);
+    pcidev_k->vendor_id = PCI_VENDOR_ID_REDHAT_QUMRANET;
+    pcidev_k->device_id = 0; /* Set by virtio-pci based on virtio id */
+    pcidev_k->revision = 0x00;
+    pcidev_k->class_id = PCI_CLASS_COMMUNICATION_OTHER;
+}
+
+static void vhost_user_rng_pci_instance_init(Object *obj)
+{
+    VHostUserRNGPCI *dev = VHOST_USER_RNG_PCI(obj);
+
+    virtio_instance_init_common(obj, &dev->vdev, sizeof(dev->vdev),
+                                TYPE_VHOST_USER_RNG);
+}
+
+static const VirtioPCIDeviceTypeInfo vhost_user_rng_pci_info = {
+    .base_name = TYPE_VHOST_USER_RNG_PCI,
+    .non_transitional_name = "vhost-user-rng-pci",
+    .instance_size = sizeof(VHostUserRNGPCI),
+    .instance_init = vhost_user_rng_pci_instance_init,
+    .class_init = vhost_user_rng_pci_class_init,
+};
+
+static void vhost_user_rng_pci_register(void)
+{
+    virtio_pci_types_register(&vhost_user_rng_pci_info);
+}
+
+type_init(vhost_user_rng_pci_register);
-- 
2.25.1



^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v3 3/4] vhost-user-rng: backend: Add RNG vhost-user daemon implementation
  2021-07-10  0:59 [PATCH v3 0/4] virtio: Add vhost-user based RNG Mathieu Poirier
  2021-07-10  0:59 ` [PATCH v3 1/4] vhost-user-rng: Add vhost-user-rng implementation Mathieu Poirier
  2021-07-10  0:59 ` [PATCH v3 2/4] vhost-user-rng-pci: Add vhost-user-rng-pci implementation Mathieu Poirier
@ 2021-07-10  0:59 ` Mathieu Poirier
  2021-07-21 11:30   ` Alex Bennée
  2021-07-21 20:14   ` Alex Bennée
  2021-07-10  0:59 ` [PATCH v3 4/4] docs: Add documentation for vhost based RNG implementation Mathieu Poirier
  2021-07-17 20:32 ` [PATCH v3 0/4] virtio: Add vhost-user based RNG Michael S. Tsirkin
  4 siblings, 2 replies; 15+ messages in thread
From: Mathieu Poirier @ 2021-07-10  0:59 UTC (permalink / raw)
  To: mst, alex.bennee; +Cc: qemu-devel

This patch provides the vhost-user backend implementation to work
in tandem with the vhost-user-rng implementation of the QEMU VMM.

It uses the vhost-user API so that other VMM can re-use the interface
without having to write the driver again.

Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
---
 tools/meson.build                        |   8 +
 tools/vhost-user-rng/50-qemu-rng.json.in |   5 +
 tools/vhost-user-rng/main.c              | 403 +++++++++++++++++++++++
 tools/vhost-user-rng/meson.build         |  10 +
 4 files changed, 426 insertions(+)
 create mode 100644 tools/vhost-user-rng/50-qemu-rng.json.in
 create mode 100644 tools/vhost-user-rng/main.c
 create mode 100644 tools/vhost-user-rng/meson.build

diff --git a/tools/meson.build b/tools/meson.build
index 3e5a0abfa29f..66b0a11fbb45 100644
--- a/tools/meson.build
+++ b/tools/meson.build
@@ -24,3 +24,11 @@ endif
 if have_virtiofsd
   subdir('virtiofsd')
 endif
+
+have_virtiorng = (have_system and
+    have_tools and
+    'CONFIG_LINUX' in config_host)
+
+if have_virtiorng
+  subdir('vhost-user-rng')
+endif
diff --git a/tools/vhost-user-rng/50-qemu-rng.json.in b/tools/vhost-user-rng/50-qemu-rng.json.in
new file mode 100644
index 000000000000..9186c3c6fe1d
--- /dev/null
+++ b/tools/vhost-user-rng/50-qemu-rng.json.in
@@ -0,0 +1,5 @@
+{
+  "description": "QEMU vhost-user-rng",
+  "type": "bridge",
+  "binary": "@libexecdir@/vhost-user-rng"
+}
diff --git a/tools/vhost-user-rng/main.c b/tools/vhost-user-rng/main.c
new file mode 100644
index 000000000000..c3b8f6922757
--- /dev/null
+++ b/tools/vhost-user-rng/main.c
@@ -0,0 +1,403 @@
+/*
+ * VIRTIO RNG Emulation via vhost-user
+ *
+ * Copyright (c) 2021 Mathieu Poirier <mathieu.poirier@linaro.org>
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#define G_LOG_DOMAIN "vhost-user-rng"
+#define G_LOG_USE_STRUCTURED 1
+
+#include <glib.h>
+#include <gio/gio.h>
+#include <gio/gunixsocketaddress.h>
+#include <glib-unix.h>
+#include <glib/gstdio.h>
+#include <pthread.h>
+#include <signal.h>
+#include <stdio.h>
+#include <stdbool.h>
+#include <string.h>
+#include <inttypes.h>
+#include <fcntl.h>
+#include <sys/ioctl.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <sys/mman.h>
+#include <time.h>
+#include <unistd.h>
+#include <endian.h>
+#include <assert.h>
+
+#include "qemu/cutils.h"
+#include "subprojects/libvhost-user/libvhost-user-glib.h"
+#include "subprojects/libvhost-user/libvhost-user.h"
+
+#ifndef container_of
+#define container_of(ptr, type, member) ({                      \
+        const typeof(((type *) 0)->member) * __mptr = (ptr);     \
+        (type *) ((char *) __mptr - offsetof(type, member)); })
+#endif
+
+typedef struct {
+    VugDev dev;
+    struct itimerspec ts;
+    timer_t rate_limit_timer;
+    pthread_mutex_t rng_mutex;
+    pthread_cond_t rng_cond;
+    int64_t quota_remaining;
+    bool activate_timer;
+    GMainLoop *loop;
+} VuRNG;
+
+static gboolean print_cap, verbose;
+static gchar *source_path, *socket_path;
+static gint source_fd, socket_fd = -1;
+
+/* Defaults tailored on virtio-rng.c */
+static uint32_t period_ms = 1 << 16;
+static uint64_t max_bytes = INT64_MAX;
+
+static void check_rate_limit(union sigval sv)
+{
+    VuRNG *rng = sv.sival_ptr;
+    bool wakeup = false;
+
+    pthread_mutex_lock(&rng->rng_mutex);
+    /*
+     * The timer has expired and the guest has used all available
+     * entropy, which means function vu_rng_handle_request() is waiting
+     * on us.  As such wake it up once we're done here.
+     */
+    if (rng->quota_remaining == 0) {
+        wakeup = true;
+    }
+
+    /*
+     * Reset the entropy available to the guest and tell function
+     * vu_rng_handle_requests() to start the timer before using it.
+     */
+    rng->quota_remaining = max_bytes;
+    rng->activate_timer = true;
+    pthread_mutex_unlock(&rng->rng_mutex);
+
+    if (wakeup) {
+        pthread_cond_signal(&rng->rng_cond);
+    }
+}
+
+static void setup_timer(VuRNG *rng)
+{
+    struct sigevent sev;
+    int ret;
+
+    memset(&rng->ts, 0, sizeof(struct itimerspec));
+    rng->ts.it_value.tv_sec = period_ms / 1000;
+    rng->ts.it_value.tv_nsec = (period_ms % 1000) * 1000000;
+
+    /*
+     * Call function check_rate_limit() as if it was the start of
+     * a new thread when the timer expires.
+     */
+    sev.sigev_notify = SIGEV_THREAD;
+    sev.sigev_notify_function = check_rate_limit;
+    sev.sigev_value.sival_ptr = rng;
+    /* Needs to be NULL if defaults attributes are to be used. */
+    sev.sigev_notify_attributes = NULL;
+    ret = timer_create(CLOCK_MONOTONIC, &sev, &rng->rate_limit_timer);
+    if (ret < 0) {
+        fprintf(stderr, "timer_create() failed\n");
+    }
+
+}
+
+
+/* Virtio helpers */
+static uint64_t rng_get_features(VuDev *dev)
+{
+    if (verbose) {
+        g_info("%s: replying", __func__);
+    }
+    return 0;
+}
+
+static void rng_set_features(VuDev *dev, uint64_t features)
+{
+    if (verbose && features) {
+        g_autoptr(GString) s = g_string_new("Requested un-handled feature");
+        g_string_append_printf(s, " 0x%" PRIx64 "", features);
+        g_info("%s: %s", __func__, s->str);
+    }
+}
+
+static void vu_rng_handle_requests(VuDev *dev, int qidx)
+{
+    VuRNG *rng = container_of(dev, VuRNG, dev.parent);
+    VuVirtq *vq = vu_get_queue(dev, qidx);
+    VuVirtqElement *elem;
+    size_t to_read;
+    int len, ret;
+
+    for (;;) {
+        /* Get element in the vhost virtqueue */
+        elem = vu_queue_pop(dev, vq, sizeof(VuVirtqElement));
+        if (!elem) {
+            break;
+        }
+
+        /* Get the amount of entropy to read from the vhost server */
+        to_read = elem->in_sg[0].iov_len;
+
+        pthread_mutex_lock(&rng->rng_mutex);
+
+        /*
+         * We have consumed all entropy available for this time slice.
+         * Wait for the timer (check_rate_limit()) to tell us about the
+         * start of a new time slice.
+         */
+        if (rng->quota_remaining == 0) {
+            pthread_cond_wait(&rng->rng_cond, &rng->rng_mutex);
+        }
+
+        /* Start the timer if the last time slice has expired */
+        if (rng->activate_timer == true) {
+            rng->activate_timer = false;
+            ret = timer_settime(rng->rate_limit_timer, 0, &rng->ts, NULL);
+            if (ret < 0) {
+                fprintf(stderr, "timer_settime() failed\n");
+            }
+        }
+
+        /* Make sure we don't read more than it's available */
+        if (rng->quota_remaining < to_read) {
+            to_read = rng->quota_remaining;
+        }
+
+        len = read(source_fd, elem->in_sg[0].iov_base, to_read);
+
+        /* Simply return 0 if an error occurs */
+        if (len < 0) {
+            len = 0;
+        }
+
+        rng->quota_remaining -= len;
+
+        pthread_mutex_unlock(&rng->rng_mutex);
+
+        vu_queue_push(dev, vq, elem, len);
+        free(elem);
+    }
+
+    vu_queue_notify(dev, vq);
+}
+
+static void
+vu_rng_queue_set_started(VuDev *dev, int qidx, bool started)
+{
+    VuVirtq *vq = vu_get_queue(dev, qidx);
+
+    g_debug("queue started %d:%d\n", qidx, started);
+
+    if (!qidx) {
+        vu_set_queue_handler(dev, vq, started ? vu_rng_handle_requests : NULL);
+    }
+}
+
+/*
+ * Any messages not handled here are processed by the libvhost library
+ * itself.
+ */
+static int rng_process_msg(VuDev *dev, VhostUserMsg *msg, int *do_reply)
+{
+    VuRNG *rng = container_of(dev, VuRNG, dev.parent);
+
+    if (msg->request == VHOST_USER_NONE) {
+        g_main_loop_quit(rng->loop);
+        return 1;
+    }
+
+    return 0;
+}
+
+static const VuDevIface vuiface = {
+    .set_features = rng_set_features,
+    .get_features = rng_get_features,
+    .queue_set_started = vu_rng_queue_set_started,
+    .process_msg = rng_process_msg,
+};
+
+static gboolean hangup(gpointer user_data)
+{
+    GMainLoop *loop = (GMainLoop *) user_data;
+
+    g_printerr("%s: caught hangup/quit signal, quitting", __func__);
+    g_main_loop_quit(loop);
+    return true;
+}
+
+static void panic(VuDev *dev, const char *msg)
+{
+    g_critical("%s\n", msg);
+    exit(EXIT_FAILURE);
+}
+
+/* Print vhost-user.json backend program capabilities */
+static void print_capabilities(void)
+{
+    printf("{\n");
+    printf("  \"type\": \"RNG\"\n");
+    printf("  \"filename\": [ RNG source ]\n");
+    printf("}\n");
+}
+
+static GOptionEntry options[] = {
+    { "socket-path", 's', 0, G_OPTION_ARG_FILENAME, &socket_path,
+      "Location of vhost-user Unix domain socket, incompatible with --fd",
+      "PATH" },
+    { "fd", 'f', 0, G_OPTION_ARG_INT, &socket_fd,
+      "Specify the backend file-descriptor, incompatible with --socket-path",
+      "FD" },
+    { "period", 'p', 0, G_OPTION_ARG_INT, &period_ms,
+      "Time needed (in ms) to transfer a maximum amount of byte", NULL },
+    { "max-bytes", 'm', 0, G_OPTION_ARG_INT64, &max_bytes,
+      "Maximum amount of byte that can be transferred in a period", NULL },
+    { "filename", 'n', 0, G_OPTION_ARG_FILENAME, &source_path,
+      "RNG source, defaults to /dev/urandom", "PATH" },
+    { "print-capabilities", 'c', 0, G_OPTION_ARG_NONE, &print_cap,
+      "Output to stdout the backend capabilities in JSON format and exit",
+      NULL},
+    { "verbose", 'v', 0, G_OPTION_ARG_NONE, &verbose,
+      "Be more verbose in output", NULL},
+    { NULL }
+};
+
+int main(int argc, char *argv[])
+{
+    GError *error = NULL;
+    GOptionContext *context;
+    g_autoptr(GSocket) socket = NULL;
+    char default_source[] = "/dev/urandom";
+    char *source = default_source;
+    VuRNG rng;
+
+    context = g_option_context_new("vhost-user emulation of RNG device");
+    g_option_context_add_main_entries(context, options, "vhost-user-rng");
+    if (!g_option_context_parse(context, &argc, &argv, &error)) {
+        g_printerr("option parsing failed: %s\n", error->message);
+        exit(1);
+    }
+
+    if (print_cap) {
+        print_capabilities();
+        exit(0);
+    }
+
+    if (!socket_path && socket_fd < 0) {
+        g_printerr("Please specify either --fd or --socket-path\n");
+        exit(EXIT_FAILURE);
+    }
+
+    if (socket_path && socket_fd > 0) {
+        g_printerr("Either --fd or --socket-path, not both\n");
+        exit(EXIT_FAILURE);
+    }
+
+    if (max_bytes > INT64_MAX) {
+        g_printerr("'max-bytes' parameter must be non-negative, "
+                   "and less than 2^63\n");
+        exit(EXIT_FAILURE);
+    }
+
+    if (period_ms <= 0) {
+        g_printerr("'period' parameter expects a positive integer\n");
+        exit(EXIT_FAILURE);
+    }
+
+    /*
+     * Now create a vhost-user socket that we will receive messages
+     * on. Once we have our handler set up we can enter the glib main
+     * loop.
+     */
+    if (socket_path) {
+        g_autoptr(GSocketAddress) addr = g_unix_socket_address_new(socket_path);
+        g_autoptr(GSocket) bind_socket = g_socket_new(G_SOCKET_FAMILY_UNIX,
+                                                      G_SOCKET_TYPE_STREAM,
+                                                      G_SOCKET_PROTOCOL_DEFAULT,
+                                                      &error);
+
+        if (!g_socket_bind(bind_socket, addr, false, &error)) {
+            g_printerr("Failed to bind to socket at %s (%s).\n",
+                       socket_path, error->message);
+            exit(EXIT_FAILURE);
+        }
+        if (!g_socket_listen(bind_socket, &error)) {
+            g_printerr("Failed to listen on socket %s (%s).\n",
+                       socket_path, error->message);
+        }
+        g_message("awaiting connection to %s", socket_path);
+        socket = g_socket_accept(bind_socket, NULL, &error);
+        if (!socket) {
+            g_printerr("Failed to accept on socket %s (%s).\n",
+                       socket_path, error->message);
+        }
+    } else {
+        socket = g_socket_new_from_fd(socket_fd, &error);
+        if (!socket) {
+            g_printerr("Failed to connect to FD %d (%s).\n",
+                       socket_fd, error->message);
+            exit(EXIT_FAILURE);
+        }
+    }
+
+    /* Overwrite default RNG source with what user provided, if any */
+    if (source_path) {
+        source = source_path;
+    }
+
+    source_fd = open(source, O_RDWR);
+    if (source_fd < 0) {
+        g_printerr("Failed to open RNG source %s\n", source);
+        g_socket_close(socket, &error);
+        unlink(socket_path);
+        exit(EXIT_FAILURE);
+    }
+
+    /* catch exit signals */
+    g_unix_signal_add(SIGHUP, hangup, rng.loop);
+    g_unix_signal_add(SIGINT, hangup, rng.loop);
+
+    /*
+     * Create the main loop first so all the various sources can be
+     * added. As well as catching signals we need to ensure vug_init
+     * can add it's GSource watches.
+     */
+    rng.loop = g_main_loop_new(NULL, FALSE);
+
+    if (!vug_init(&rng.dev, 1, g_socket_get_fd(socket),
+                  panic, &vuiface)) {
+        g_printerr("Failed to initialize libvhost-user-glib.\n");
+        exit(EXIT_FAILURE);
+    }
+
+    rng.quota_remaining = max_bytes;
+    rng.activate_timer = true;
+    pthread_mutex_init(&rng.rng_mutex, NULL);
+    pthread_cond_init(&rng.rng_cond, NULL);
+    setup_timer(&rng);
+
+    if (verbose) {
+        g_info("period_ms: %d tv_sec: %ld tv_nsec: %lu\n",
+               period_ms, rng.ts.it_value.tv_sec, rng.ts.it_value.tv_nsec);
+    }
+
+    g_message("entering main loop, awaiting messages");
+    g_main_loop_run(rng.loop);
+    g_message("finished main loop, cleaning up");
+
+    g_main_loop_unref(rng.loop);
+    vug_deinit(&rng.dev);
+    timer_delete(rng.rate_limit_timer);
+    close(source_fd);
+    unlink(socket_path);
+}
diff --git a/tools/vhost-user-rng/meson.build b/tools/vhost-user-rng/meson.build
new file mode 100644
index 000000000000..4dc386daf335
--- /dev/null
+++ b/tools/vhost-user-rng/meson.build
@@ -0,0 +1,10 @@
+executable('vhost-user-rng', files(
+  'main.c'),
+  dependencies: [qemuutil, glib, gio, rt],
+  install: true,
+  install_dir: get_option('libexecdir'))
+
+configure_file(input: '50-qemu-rng.json.in',
+               output: '50-qemu-rng.json',
+               configuration: config_host,
+               install_dir: qemu_datadir / 'vhost-user')
-- 
2.25.1



^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v3 4/4] docs: Add documentation for vhost based RNG implementation
  2021-07-10  0:59 [PATCH v3 0/4] virtio: Add vhost-user based RNG Mathieu Poirier
                   ` (2 preceding siblings ...)
  2021-07-10  0:59 ` [PATCH v3 3/4] vhost-user-rng: backend: Add RNG vhost-user daemon implementation Mathieu Poirier
@ 2021-07-10  0:59 ` Mathieu Poirier
  2021-07-21 16:55   ` Alex Bennée
  2021-07-17 20:32 ` [PATCH v3 0/4] virtio: Add vhost-user based RNG Michael S. Tsirkin
  4 siblings, 1 reply; 15+ messages in thread
From: Mathieu Poirier @ 2021-07-10  0:59 UTC (permalink / raw)
  To: mst, alex.bennee; +Cc: qemu-devel

Add description and example for the vhost-user based RNG implementation.
Tailored on Viresh Kumar's vhost-user-i2c documentation.

Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
---
 docs/tools/index.rst          |  1 +
 docs/tools/vhost-user-rng.rst | 74 +++++++++++++++++++++++++++++++++++
 2 files changed, 75 insertions(+)
 create mode 100644 docs/tools/vhost-user-rng.rst

diff --git a/docs/tools/index.rst b/docs/tools/index.rst
index d923834a7398..9d80fa89eceb 100644
--- a/docs/tools/index.rst
+++ b/docs/tools/index.rst
@@ -15,5 +15,6 @@ Contents:
    qemu-nbd
    qemu-pr-helper
    qemu-trace-stap
+   vhost-user-rng
    virtfs-proxy-helper
    virtiofsd
diff --git a/docs/tools/vhost-user-rng.rst b/docs/tools/vhost-user-rng.rst
new file mode 100644
index 000000000000..7f69d7bb3c58
--- /dev/null
+++ b/docs/tools/vhost-user-rng.rst
@@ -0,0 +1,74 @@
+QEMU vhost-user-rng - RNG emulation backend
+===========================================
+
+Synopsis
+--------
+
+**vhost-user-rng** [*OPTIONS*]
+
+Description
+-----------
+
+This program is a vhost-user backend that emulates a VirtIO random number
+generator (RNG).  It uses the host's random number generator pool,
+/dev/urandom by default but configurable at will, to satisfy requests from
+guests.
+
+This program is designed to work with QEMU's ``-device
+vhost-user-rng-pci`` but should work with any virtual machine monitor
+(VMM) that supports vhost-user. See the Examples section below.
+
+Options
+-------
+
+.. program:: vhost-user-rng
+
+.. option:: -h, --help
+
+  Print help.
+
+.. option:: -v, --verbose
+
+   Increase verbosity of output
+
+.. option:: -s, --socket-path=PATH
+
+  Listen on vhost-user UNIX domain socket at PATH. Incompatible with --fd.
+
+.. option:: -f, --fd=FDNUM
+
+  Accept connections from vhost-user UNIX domain socket file descriptor FDNUM.
+  The file descriptor must already be listening for connections.
+  Incompatible with --socket-path.
+
+.. option:: -p, --period
+
+  Rate, in milliseconds, at which the RNG hardware can generate random data.
+  Used in conjunction with the --max-bytes option.
+
+.. option:: -m, --max-bytes
+
+  In conjuction with the --period parameter, provides the maximum number of byte
+  per milliseconds a RNG device can generate.
+
+Examples
+--------
+
+The daemon should be started first:
+
+::
+
+  host# vhost-user-rng --socket-path=rng.sock --period=1000 --max-bytes=4096
+
+The QEMU invocation needs to create a chardev socket the device can
+use to communicate as well as share the guests memory over a memfd.
+
+::
+
+  host# qemu-system								\
+      -chardev socket,path=$(PATH)/rng.sock,id=rng0				\
+      -device vhost-user-rng-pci,chardev=rng0					\
+      -m 4096 									\
+      -object memory-backend-file,id=mem,size=4G,mem-path=/dev/shm,share=on	\
+      -numa node,memdev=mem							\
+      ...
-- 
2.25.1



^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH v3 0/4] virtio: Add vhost-user based RNG
  2021-07-10  0:59 [PATCH v3 0/4] virtio: Add vhost-user based RNG Mathieu Poirier
                   ` (3 preceding siblings ...)
  2021-07-10  0:59 ` [PATCH v3 4/4] docs: Add documentation for vhost based RNG implementation Mathieu Poirier
@ 2021-07-17 20:32 ` Michael S. Tsirkin
  4 siblings, 0 replies; 15+ messages in thread
From: Michael S. Tsirkin @ 2021-07-17 20:32 UTC (permalink / raw)
  To: Mathieu Poirier; +Cc: alex.bennee, qemu-devel

On Fri, Jul 09, 2021 at 06:59:25PM -0600, Mathieu Poirier wrote:
> This sets adds a vhost-user based random number generator (RNG),
> similar to what has been done for i2c and virtiofsd, with the
> implementation following the patterns already set forth in those.
> 
> Applies cleanly to git://git.qemu.org/qemu.git master(05de778b5b8a).

There were several meson-related issues related to this patchset,
so I dropped it from the pull request for now.
Please work to resolve, and re-submit, preferably after the release.



> Thanks,
> Mathieu
> 
> Mathieu Poirier (4):
>   vhost-user-rng: Add vhost-user-rng implementation
>   vhost-user-rng-pci: Add vhost-user-rng-pci implementation
>   vhost-user-rng: backend: Add RNG vhost-user daemon implementation
>   docs: Add documentation for vhost based RNG implementation
> 
> ----
> New for V3:
> - Rebased to latest master branch.
> - Fixed documentation warning.
> - Updated call to vhost_dev_init() to match new signature.
> - Dropped MAINTAINERS patch since it was already applied. 
> 
>  docs/tools/index.rst                     |   1 +
>  docs/tools/vhost-user-rng.rst            |  74 +++++
>  hw/virtio/Kconfig                        |   5 +
>  hw/virtio/meson.build                    |   2 +
>  hw/virtio/vhost-user-rng-pci.c           |  79 +++++
>  hw/virtio/vhost-user-rng.c               | 294 +++++++++++++++++
>  include/hw/virtio/vhost-user-rng.h       |  33 ++
>  tools/meson.build                        |   8 +
>  tools/vhost-user-rng/50-qemu-rng.json.in |   5 +
>  tools/vhost-user-rng/main.c              | 403 +++++++++++++++++++++++
>  tools/vhost-user-rng/meson.build         |  10 +
>  11 files changed, 914 insertions(+)
>  create mode 100644 docs/tools/vhost-user-rng.rst
>  create mode 100644 hw/virtio/vhost-user-rng-pci.c
>  create mode 100644 hw/virtio/vhost-user-rng.c
>  create mode 100644 include/hw/virtio/vhost-user-rng.h
>  create mode 100644 tools/vhost-user-rng/50-qemu-rng.json.in
>  create mode 100644 tools/vhost-user-rng/main.c
>  create mode 100644 tools/vhost-user-rng/meson.build
> 
> -- 
> 2.25.1



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v3 1/4] vhost-user-rng: Add vhost-user-rng implementation
  2021-07-10  0:59 ` [PATCH v3 1/4] vhost-user-rng: Add vhost-user-rng implementation Mathieu Poirier
@ 2021-07-21  8:52   ` Alex Bennée
  2021-07-22 16:44     ` Mathieu Poirier
  0 siblings, 1 reply; 15+ messages in thread
From: Alex Bennée @ 2021-07-21  8:52 UTC (permalink / raw)
  To: Mathieu Poirier; +Cc: qemu-devel, mst


Mathieu Poirier <mathieu.poirier@linaro.org> writes:

> Following in the footsteps of what whas done for vhost-user-i2c
> and virtiofsd, introduce a random number generator (RNG) backend
> that communicates with a vhost-user server to retrieve entropy.
> That way another VMM could be using the same vhost-user daemon and
> avoid having to write yet another RNG driver.
>
> Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
> ---
>  hw/virtio/Kconfig                  |   5 +
>  hw/virtio/meson.build              |   1 +

FWIW there are simple merge failures for the meson and Kconfig parts due
to I2C being merged.

<snip>
> +
> +    rng->vhost_dev.nvqs = 1;
> +    rng->vhost_dev.vqs = g_new0(struct vhost_virtqueue, rng->vhost_dev.nvqs);
> +    if (!rng->vhost_dev.vqs) {
> +        error_setg_errno(errp, -1, "memory allocation failed");
> +        goto vhost_dev_init_failed;
> +    }

g_new0 will abort on memory allocation failure (which is fine for
userspace ;-) so you don't need the check and erro handling exit here.

> +
> +    ret = vhost_dev_init(&rng->vhost_dev, &rng->vhost_user,
> +                         VHOST_BACKEND_TYPE_USER, 0, errp);
> +    if (ret < 0) {
> +        error_setg_errno(errp, -ret, "vhost_dev_init() failed");
> +        goto vhost_dev_init_failed;
> +    }
> +
> +    qemu_chr_fe_set_handlers(&rng->chardev, NULL, NULL, vu_rng_event, NULL,
> +                             dev, NULL, true);
> +
> +    return;
> +
> +vhost_dev_init_failed:
> +    virtio_delete_queue(rng->req_vq);
> +virtio_add_queue_failed:
> +    virtio_cleanup(vdev);
> +    vhost_user_cleanup(&rng->vhost_user);
> +}
> +
> +static void vu_rng_device_unrealize(DeviceState *dev)
> +{
> +    VirtIODevice *vdev = VIRTIO_DEVICE(dev);
> +    VHostUserRNG *rng = VHOST_USER_RNG(dev);
> +
> +    vu_rng_set_status(vdev, 0);
> +
> +    vhost_dev_cleanup(&rng->vhost_dev);
> +    g_free(rng->vhost_dev.vqs);
> +    rng->vhost_dev.vqs = NULL;
> +    virtio_delete_queue(rng->req_vq);
> +    virtio_cleanup(vdev);
> +    vhost_user_cleanup(&rng->vhost_user);
> +}
> +
> +static const VMStateDescription vu_rng_vmstate = {
> +    .name = "vhost-user-rng",
> +    .unmigratable = 1,
> +};
> +
> +static Property vu_rng_properties[] = {
> +    DEFINE_PROP_CHR("chardev", VHostUserRNG, chardev),
> +    DEFINE_PROP_END_OF_LIST(),
> +};
> +
> +static void vu_rng_class_init(ObjectClass *klass, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(klass);
> +    VirtioDeviceClass *vdc = VIRTIO_DEVICE_CLASS(klass);
> +
> +    device_class_set_props(dc, vu_rng_properties);
> +    dc->vmsd = &vu_rng_vmstate;
> +    set_bit(DEVICE_CATEGORY_INPUT, dc->categories);
> +
> +    vdc->realize = vu_rng_device_realize;
> +    vdc->unrealize = vu_rng_device_unrealize;
> +    vdc->get_features = vu_rng_get_features;
> +    vdc->set_status = vu_rng_set_status;
> +    vdc->guest_notifier_mask = vu_rng_guest_notifier_mask;
> +    vdc->guest_notifier_pending = vu_rng_guest_notifier_pending;
> +}
> +
> +static const TypeInfo vu_rng_info = {
> +    .name = TYPE_VHOST_USER_RNG,
> +    .parent = TYPE_VIRTIO_DEVICE,
> +    .instance_size = sizeof(VHostUserRNG),
> +    .class_init = vu_rng_class_init,
> +};
> +
> +static void vu_rng_register_types(void)
> +{
> +    type_register_static(&vu_rng_info);
> +}
> +
> +type_init(vu_rng_register_types)
> diff --git a/include/hw/virtio/vhost-user-rng.h b/include/hw/virtio/vhost-user-rng.h
> new file mode 100644
> index 000000000000..071539996d1d
> --- /dev/null
> +++ b/include/hw/virtio/vhost-user-rng.h
> @@ -0,0 +1,33 @@
> +/*
> + * Vhost-user RNG virtio device
> + *
> + * Copyright (c) 2021 Mathieu Poirier <mathieu.poirier@linaro.org>
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +
> +#ifndef _QEMU_VHOST_USER_RNG_H
> +#define _QEMU_VHOST_USER_RNG_H
> +
> +#include "hw/virtio/virtio.h"
> +#include "hw/virtio/vhost.h"
> +#include "hw/virtio/vhost-user.h"
> +#include "chardev/char-fe.h"
> +
> +#define TYPE_VHOST_USER_RNG "vhost-user-rng"
> +OBJECT_DECLARE_SIMPLE_TYPE(VHostUserRNG, VHOST_USER_RNG)
> +
> +struct VHostUserRNG {
> +    /*< private >*/
> +    VirtIODevice parent;
> +    CharBackend chardev;
> +    struct vhost_virtqueue *vhost_vq;
> +    struct vhost_dev vhost_dev;
> +    VhostUserState vhost_user;
> +    VirtQueue *req_vq;
> +    bool connected;
> +
> +    /*< public >*/
> +};
> +
> +#endif /* _QEMU_VHOST_USER_RNG_H */

Otherwise:

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

-- 
Alex Bennée


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v3 2/4] vhost-user-rng-pci: Add vhost-user-rng-pci implementation
  2021-07-10  0:59 ` [PATCH v3 2/4] vhost-user-rng-pci: Add vhost-user-rng-pci implementation Mathieu Poirier
@ 2021-07-21  8:56   ` Alex Bennée
  2021-07-21 20:15   ` Alex Bennée
  1 sibling, 0 replies; 15+ messages in thread
From: Alex Bennée @ 2021-07-21  8:56 UTC (permalink / raw)
  To: Mathieu Poirier; +Cc: qemu-devel, mst


Mathieu Poirier <mathieu.poirier@linaro.org> writes:

> This patch provides a PCI bus interface to the vhost-user-rng backed.
> The implentation is similar to what was done for vhost-user-i2c-pci and
> vhost-user-fs-pci.
>
> Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
<snip>
> diff --git a/hw/virtio/meson.build b/hw/virtio/meson.build
> index e386791f2a05..1430b370e64d 100644
> --- a/hw/virtio/meson.build
> +++ b/hw/virtio/meson.build
> @@ -26,6 +26,7 @@ virtio_ss.add(when: 'CONFIG_VIRTIO_RNG', if_true: files('virtio-rng.c'))
>  virtio_ss.add(when: 'CONFIG_VIRTIO_IOMMU', if_true: files('virtio-iommu.c'))
>  virtio_ss.add(when: 'CONFIG_VIRTIO_MEM', if_true: files('virtio-mem.c'))
>  virtio_ss.add(when: 'CONFIG_VHOST_USER_RNG', if_true: files('vhost-user-rng.c'))
> +virtio_ss.add(when: ['CONFIG_VHOST_USER_RNG', 'CONFIG_VIRTIO_PCI'], if_true: files('vhost-user-rng-pci.c'))
>

Another minor merge conflict here to be fixed on rebase. Otherwise:

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

-- 
Alex Bennée


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v3 3/4] vhost-user-rng: backend: Add RNG vhost-user daemon implementation
  2021-07-10  0:59 ` [PATCH v3 3/4] vhost-user-rng: backend: Add RNG vhost-user daemon implementation Mathieu Poirier
@ 2021-07-21 11:30   ` Alex Bennée
  2021-07-21 20:14   ` Alex Bennée
  1 sibling, 0 replies; 15+ messages in thread
From: Alex Bennée @ 2021-07-21 11:30 UTC (permalink / raw)
  To: Mathieu Poirier; +Cc: qemu-devel, mst


Mathieu Poirier <mathieu.poirier@linaro.org> writes:

> This patch provides the vhost-user backend implementation to work
> in tandem with the vhost-user-rng implementation of the QEMU VMM.
>
> It uses the vhost-user API so that other VMM can re-use the interface
> without having to write the driver again.
>
> Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
> ---
>  tools/meson.build                        |   8 +
>  tools/vhost-user-rng/50-qemu-rng.json.in |   5 +
>  tools/vhost-user-rng/main.c              | 403 +++++++++++++++++++++++
>  tools/vhost-user-rng/meson.build         |  10 +
>  4 files changed, 426 insertions(+)
>  create mode 100644 tools/vhost-user-rng/50-qemu-rng.json.in
>  create mode 100644 tools/vhost-user-rng/main.c
>  create mode 100644 tools/vhost-user-rng/meson.build
>
> diff --git a/tools/meson.build b/tools/meson.build
> index 3e5a0abfa29f..66b0a11fbb45 100644
> --- a/tools/meson.build
> +++ b/tools/meson.build
> @@ -24,3 +24,11 @@ endif
>  if have_virtiofsd
>    subdir('virtiofsd')
>  endif
> +
> +have_virtiorng = (have_system and
> +    have_tools and
> +    'CONFIG_LINUX' in config_host)
> +
> +if have_virtiorng
> +  subdir('vhost-user-rng')
> +endif
> diff --git a/tools/vhost-user-rng/50-qemu-rng.json.in b/tools/vhost-user-rng/50-qemu-rng.json.in
> new file mode 100644
> index 000000000000..9186c3c6fe1d
> --- /dev/null
> +++ b/tools/vhost-user-rng/50-qemu-rng.json.in
> @@ -0,0 +1,5 @@
> +{
> +  "description": "QEMU vhost-user-rng",
> +  "type": "bridge",
> +  "binary": "@libexecdir@/vhost-user-rng"
> +}
> diff --git a/tools/vhost-user-rng/main.c b/tools/vhost-user-rng/main.c
> new file mode 100644
> index 000000000000..c3b8f6922757
> --- /dev/null
> +++ b/tools/vhost-user-rng/main.c
> @@ -0,0 +1,403 @@
> +/*
> + * VIRTIO RNG Emulation via vhost-user
> + *
> + * Copyright (c) 2021 Mathieu Poirier <mathieu.poirier@linaro.org>
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +
> +#define G_LOG_DOMAIN "vhost-user-rng"
> +#define G_LOG_USE_STRUCTURED 1
> +
> +#include <glib.h>
> +#include <gio/gio.h>
> +#include <gio/gunixsocketaddress.h>
> +#include <glib-unix.h>
> +#include <glib/gstdio.h>
> +#include <pthread.h>
> +#include <signal.h>
> +#include <stdio.h>
> +#include <stdbool.h>
> +#include <string.h>
> +#include <inttypes.h>
> +#include <fcntl.h>
> +#include <sys/ioctl.h>
> +#include <sys/types.h>
> +#include <sys/stat.h>
> +#include <sys/mman.h>
> +#include <time.h>
> +#include <unistd.h>
> +#include <endian.h>
> +#include <assert.h>
> +
> +#include "qemu/cutils.h"
> +#include "subprojects/libvhost-user/libvhost-user-glib.h"
> +#include "subprojects/libvhost-user/libvhost-user.h"
> +
> +#ifndef container_of
> +#define container_of(ptr, type, member) ({                      \
> +        const typeof(((type *) 0)->member) * __mptr = (ptr);     \
> +        (type *) ((char *) __mptr - offsetof(type, member)); })
> +#endif
> +
> +typedef struct {
> +    VugDev dev;
> +    struct itimerspec ts;
> +    timer_t rate_limit_timer;
> +    pthread_mutex_t rng_mutex;
> +    pthread_cond_t rng_cond;

I'm confused by the need for a mutex in a single-threaded application.

> +    int64_t quota_remaining;
> +    bool activate_timer;
> +    GMainLoop *loop;
> +} VuRNG;
> +
> +static gboolean print_cap, verbose;
> +static gchar *source_path, *socket_path;
> +static gint source_fd, socket_fd = -1;
> +
> +/* Defaults tailored on virtio-rng.c */
> +static uint32_t period_ms = 1 << 16;
> +static uint64_t max_bytes = INT64_MAX;
> +
> +static void check_rate_limit(union sigval sv)
> +{
> +    VuRNG *rng = sv.sival_ptr;
> +    bool wakeup = false;
> +
> +    pthread_mutex_lock(&rng->rng_mutex);
> +    /*
> +     * The timer has expired and the guest has used all available
> +     * entropy, which means function vu_rng_handle_request() is waiting
> +     * on us.  As such wake it up once we're done here.
> +     */
> +    if (rng->quota_remaining == 0) {
> +        wakeup = true;
> +    }
> +
> +    /*
> +     * Reset the entropy available to the guest and tell function
> +     * vu_rng_handle_requests() to start the timer before using it.
> +     */
> +    rng->quota_remaining = max_bytes;
> +    rng->activate_timer = true;
> +    pthread_mutex_unlock(&rng->rng_mutex);
> +
> +    if (wakeup) {
> +        pthread_cond_signal(&rng->rng_cond);
> +    }
> +}
> +
> +static void setup_timer(VuRNG *rng)
> +{
> +    struct sigevent sev;
> +    int ret;
> +
> +    memset(&rng->ts, 0, sizeof(struct itimerspec));
> +    rng->ts.it_value.tv_sec = period_ms / 1000;
> +    rng->ts.it_value.tv_nsec = (period_ms % 1000) * 1000000;
> +
> +    /*
> +     * Call function check_rate_limit() as if it was the start of
> +     * a new thread when the timer expires.
> +     */
> +    sev.sigev_notify = SIGEV_THREAD;
> +    sev.sigev_notify_function = check_rate_limit;
> +    sev.sigev_value.sival_ptr = rng;
> +    /* Needs to be NULL if defaults attributes are to be used. */
> +    sev.sigev_notify_attributes = NULL;
> +    ret = timer_create(CLOCK_MONOTONIC, &sev, &rng->rate_limit_timer);
> +    if (ret < 0) {
> +        fprintf(stderr, "timer_create() failed\n");
> +    }

Ahh I see why now. I think you could avoid this by using glib's own
internal g_timeout_add() function. This would then create a timer which
would call it's callback periodically (if it returns true to persist the
GSource). As the whole execution is effectively event driven you would
avoid the need for locking.

> +
> +}
> +
> +
> +/* Virtio helpers */
> +static uint64_t rng_get_features(VuDev *dev)
> +{
> +    if (verbose) {
> +        g_info("%s: replying", __func__);
> +    }
> +    return 0;
> +}
> +
> +static void rng_set_features(VuDev *dev, uint64_t features)
> +{
> +    if (verbose && features) {
> +        g_autoptr(GString) s = g_string_new("Requested un-handled feature");
> +        g_string_append_printf(s, " 0x%" PRIx64 "", features);
> +        g_info("%s: %s", __func__, s->str);
> +    }
> +}
> +
> +static void vu_rng_handle_requests(VuDev *dev, int qidx)
> +{
> +    VuRNG *rng = container_of(dev, VuRNG, dev.parent);
> +    VuVirtq *vq = vu_get_queue(dev, qidx);
> +    VuVirtqElement *elem;
> +    size_t to_read;
> +    int len, ret;
> +
> +    for (;;) {
> +        /* Get element in the vhost virtqueue */
> +        elem = vu_queue_pop(dev, vq, sizeof(VuVirtqElement));
> +        if (!elem) {
> +            break;
> +        }
> +
> +        /* Get the amount of entropy to read from the vhost server */
> +        to_read = elem->in_sg[0].iov_len;
> +
> +        pthread_mutex_lock(&rng->rng_mutex);
> +
> +        /*
> +         * We have consumed all entropy available for this time slice.
> +         * Wait for the timer (check_rate_limit()) to tell us about the
> +         * start of a new time slice.
> +         */
> +        if (rng->quota_remaining == 0) {
> +            pthread_cond_wait(&rng->rng_cond, &rng->rng_mutex);
> +        }

Hmm this complicates things. Ideally you wouldn't want to block here on
processing the virtqueue. This will end up block the guest. I'll need to
think about this.

-- 
Alex Bennée


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v3 4/4] docs: Add documentation for vhost based RNG implementation
  2021-07-10  0:59 ` [PATCH v3 4/4] docs: Add documentation for vhost based RNG implementation Mathieu Poirier
@ 2021-07-21 16:55   ` Alex Bennée
  0 siblings, 0 replies; 15+ messages in thread
From: Alex Bennée @ 2021-07-21 16:55 UTC (permalink / raw)
  To: Mathieu Poirier; +Cc: qemu-devel, mst


Mathieu Poirier <mathieu.poirier@linaro.org> writes:

> Add description and example for the vhost-user based RNG implementation.
> Tailored on Viresh Kumar's vhost-user-i2c documentation.
>
> Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
> ---
>  docs/tools/index.rst          |  1 +
>  docs/tools/vhost-user-rng.rst | 74 +++++++++++++++++++++++++++++++++++
>  2 files changed, 75 insertions(+)
>  create mode 100644 docs/tools/vhost-user-rng.rst
>
> diff --git a/docs/tools/index.rst b/docs/tools/index.rst
> index d923834a7398..9d80fa89eceb 100644
> --- a/docs/tools/index.rst
> +++ b/docs/tools/index.rst
> @@ -15,5 +15,6 @@ Contents:
>     qemu-nbd
>     qemu-pr-helper
>     qemu-trace-stap
> +   vhost-user-rng
>     virtfs-proxy-helper
>     virtiofsd
> diff --git a/docs/tools/vhost-user-rng.rst b/docs/tools/vhost-user-rng.rst
> new file mode 100644
> index 000000000000..7f69d7bb3c58
> --- /dev/null
> +++ b/docs/tools/vhost-user-rng.rst
> @@ -0,0 +1,74 @@
> +QEMU vhost-user-rng - RNG emulation backend
> +===========================================
> +
> +Synopsis
> +--------
> +
> +**vhost-user-rng** [*OPTIONS*]
> +
> +Description
> +-----------
> +
> +This program is a vhost-user backend that emulates a VirtIO random number
> +generator (RNG).  It uses the host's random number generator pool,
> +/dev/urandom by default but configurable at will, to satisfy requests from
> +guests.
> +
> +This program is designed to work with QEMU's ``-device
> +vhost-user-rng-pci`` but should work with any virtual machine monitor
> +(VMM) that supports vhost-user. See the Examples section below.
> +
> +Options
> +-------
> +
> +.. program:: vhost-user-rng
> +
> +.. option:: -h, --help
> +
> +  Print help.
> +
> +.. option:: -v, --verbose
> +
> +   Increase verbosity of output
> +
> +.. option:: -s, --socket-path=PATH
> +
> +  Listen on vhost-user UNIX domain socket at PATH. Incompatible with --fd.
> +
> +.. option:: -f, --fd=FDNUM
> +
> +  Accept connections from vhost-user UNIX domain socket file descriptor FDNUM.
> +  The file descriptor must already be listening for connections.
> +  Incompatible with --socket-path.
> +
> +.. option:: -p, --period
> +
> +  Rate, in milliseconds, at which the RNG hardware can generate random data.
> +  Used in conjunction with the --max-bytes option.
> +
> +.. option:: -m, --max-bytes
> +
> +  In conjuction with the --period parameter, provides the maximum number of byte
> +  per milliseconds a RNG device can generate.
> +
> +Examples
> +--------
> +
> +The daemon should be started first:
> +
> +::
> +
> +  host# vhost-user-rng --socket-path=rng.sock --period=1000 --max-bytes=4096
> +
> +The QEMU invocation needs to create a chardev socket the device can
> +use to communicate as well as share the guests memory over a memfd.
> +
> +::
> +
> +  host# qemu-system								\
> +      -chardev socket,path=$(PATH)/rng.sock,id=rng0				\
> +      -device vhost-user-rng-pci,chardev=rng0					\
> +      -m 4096 									\
> +      -object memory-backend-file,id=mem,size=4G,mem-path=/dev/shm,share=on	\
> +      -numa node,memdev=mem							\
> +      ...

Would it be worth pointing out how a guest may consume the randomness? I
appreciate this will be guest specific but currently I'm struggling how
to consume the entropy in a Linux guest.

  cat /dev/urandom > /dev/null

didn't seem to cause any to be consumed above what was during boot up.

-- 
Alex Bennée


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v3 3/4] vhost-user-rng: backend: Add RNG vhost-user daemon implementation
  2021-07-10  0:59 ` [PATCH v3 3/4] vhost-user-rng: backend: Add RNG vhost-user daemon implementation Mathieu Poirier
  2021-07-21 11:30   ` Alex Bennée
@ 2021-07-21 20:14   ` Alex Bennée
  2021-07-22 17:54     ` Mathieu Poirier
  1 sibling, 1 reply; 15+ messages in thread
From: Alex Bennée @ 2021-07-21 20:14 UTC (permalink / raw)
  To: Mathieu Poirier; +Cc: qemu-devel, mst


Mathieu Poirier <mathieu.poirier@linaro.org> writes:

> This patch provides the vhost-user backend implementation to work
> in tandem with the vhost-user-rng implementation of the QEMU VMM.
>
> It uses the vhost-user API so that other VMM can re-use the interface
> without having to write the driver again.
>
> Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>

Try the following patch which creates a nested main loop and runs it
until the g_timeout_add fires again.

--8<---------------cut here---------------start------------->8---
tools/virtio/vhost-user-rng: avoid mutex by using nested main loop

As we are blocking anyway all we really need to do is run a main loop
until the timer fires and the data is consumed.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>

1 file changed, 30 insertions(+), 76 deletions(-)
tools/vhost-user-rng/main.c | 106 +++++++++++++-------------------------------

modified   tools/vhost-user-rng/main.c
@@ -42,13 +42,10 @@
 
 typedef struct {
     VugDev dev;
-    struct itimerspec ts;
-    timer_t rate_limit_timer;
-    pthread_mutex_t rng_mutex;
-    pthread_cond_t rng_cond;
     int64_t quota_remaining;
-    bool activate_timer;
+    guint timer;
     GMainLoop *loop;
+    GMainLoop *blocked;
 } VuRNG;
 
 static gboolean print_cap, verbose;
@@ -59,66 +56,26 @@ static gint source_fd, socket_fd = -1;
 static uint32_t period_ms = 1 << 16;
 static uint64_t max_bytes = INT64_MAX;
 
-static void check_rate_limit(union sigval sv)
+static gboolean check_rate_limit(gpointer user_data)
 {
-    VuRNG *rng = sv.sival_ptr;
-    bool wakeup = false;
+    VuRNG *rng = (VuRNG *) user_data;
 
-    pthread_mutex_lock(&rng->rng_mutex);
-    /*
-     * The timer has expired and the guest has used all available
-     * entropy, which means function vu_rng_handle_request() is waiting
-     * on us.  As such wake it up once we're done here.
-     */
-    if (rng->quota_remaining == 0) {
-        wakeup = true;
+    if (rng->blocked) {
+        g_info("%s: called while blocked", __func__);
+        g_main_loop_quit(rng->blocked);
     }
-
     /*
      * Reset the entropy available to the guest and tell function
      * vu_rng_handle_requests() to start the timer before using it.
      */
     rng->quota_remaining = max_bytes;
-    rng->activate_timer = true;
-    pthread_mutex_unlock(&rng->rng_mutex);
-
-    if (wakeup) {
-        pthread_cond_signal(&rng->rng_cond);
-    }
-}
-
-static void setup_timer(VuRNG *rng)
-{
-    struct sigevent sev;
-    int ret;
-
-    memset(&rng->ts, 0, sizeof(struct itimerspec));
-    rng->ts.it_value.tv_sec = period_ms / 1000;
-    rng->ts.it_value.tv_nsec = (period_ms % 1000) * 1000000;
-
-    /*
-     * Call function check_rate_limit() as if it was the start of
-     * a new thread when the timer expires.
-     */
-    sev.sigev_notify = SIGEV_THREAD;
-    sev.sigev_notify_function = check_rate_limit;
-    sev.sigev_value.sival_ptr = rng;
-    /* Needs to be NULL if defaults attributes are to be used. */
-    sev.sigev_notify_attributes = NULL;
-    ret = timer_create(CLOCK_MONOTONIC, &sev, &rng->rate_limit_timer);
-    if (ret < 0) {
-        fprintf(stderr, "timer_create() failed\n");
-    }
-
+    return true;
 }
 
-
 /* Virtio helpers */
 static uint64_t rng_get_features(VuDev *dev)
 {
-    if (verbose) {
-        g_info("%s: replying", __func__);
-    }
+    g_info("%s: replying", __func__);
     return 0;
 }
 
@@ -137,7 +94,7 @@ static void vu_rng_handle_requests(VuDev *dev, int qidx)
     VuVirtq *vq = vu_get_queue(dev, qidx);
     VuVirtqElement *elem;
     size_t to_read;
-    int len, ret;
+    int len;
 
     for (;;) {
         /* Get element in the vhost virtqueue */
@@ -149,24 +106,21 @@ static void vu_rng_handle_requests(VuDev *dev, int qidx)
         /* Get the amount of entropy to read from the vhost server */
         to_read = elem->in_sg[0].iov_len;
 
-        pthread_mutex_lock(&rng->rng_mutex);
-
         /*
          * We have consumed all entropy available for this time slice.
          * Wait for the timer (check_rate_limit()) to tell us about the
          * start of a new time slice.
          */
         if (rng->quota_remaining == 0) {
-            pthread_cond_wait(&rng->rng_cond, &rng->rng_mutex);
-        }
-
-        /* Start the timer if the last time slice has expired */
-        if (rng->activate_timer == true) {
-            rng->activate_timer = false;
-            ret = timer_settime(rng->rate_limit_timer, 0, &rng->ts, NULL);
-            if (ret < 0) {
-                fprintf(stderr, "timer_settime() failed\n");
-            }
+            g_assert(!rng->blocked);
+            rng->blocked = g_main_loop_new(g_main_loop_get_context(rng->loop), false);
+            g_info("attempting to consume %ld bytes but no quota left (%s)",
+                   to_read,
+                   g_main_loop_is_running(rng->loop) ? "running" : "not running");
+            g_main_loop_run(rng->blocked);
+            g_info("return from blocked loop: %ld", rng->quota_remaining);
+            g_main_loop_unref(rng->blocked);
+            rng->blocked = false;
         }
 
         /* Make sure we don't read more than it's available */
@@ -183,8 +137,6 @@ static void vu_rng_handle_requests(VuDev *dev, int qidx)
 
         rng->quota_remaining -= len;
 
-        pthread_mutex_unlock(&rng->rng_mutex);
-
         vu_queue_push(dev, vq, elem, len);
         free(elem);
     }
@@ -373,6 +325,7 @@ int main(int argc, char *argv[])
      * can add it's GSource watches.
      */
     rng.loop = g_main_loop_new(NULL, FALSE);
+    rng.blocked = NULL;
 
     if (!vug_init(&rng.dev, 1, g_socket_get_fd(socket),
                   panic, &vuiface)) {
@@ -380,24 +333,25 @@ int main(int argc, char *argv[])
         exit(EXIT_FAILURE);
     }
 
-    rng.quota_remaining = max_bytes;
-    rng.activate_timer = true;
-    pthread_mutex_init(&rng.rng_mutex, NULL);
-    pthread_cond_init(&rng.rng_cond, NULL);
-    setup_timer(&rng);
-
     if (verbose) {
-        g_info("period_ms: %d tv_sec: %ld tv_nsec: %lu\n",
-               period_ms, rng.ts.it_value.tv_sec, rng.ts.it_value.tv_nsec);
+        g_log_set_handler(NULL, G_LOG_LEVEL_MASK, g_log_default_handler, NULL);
+        g_setenv("G_MESSAGES_DEBUG", "all", true);
+    } else {
+        g_log_set_handler(NULL,
+                          G_LOG_LEVEL_WARNING | G_LOG_LEVEL_CRITICAL | G_LOG_LEVEL_ERROR,
+                          g_log_default_handler, NULL);
     }
 
+    rng.quota_remaining = max_bytes;
+    rng.timer = g_timeout_add(period_ms, check_rate_limit, &rng);
+    g_info("period_ms: %"PRId32", timer %d\n", period_ms, rng.timer);
+
     g_message("entering main loop, awaiting messages");
     g_main_loop_run(rng.loop);
     g_message("finished main loop, cleaning up");
 
     g_main_loop_unref(rng.loop);
     vug_deinit(&rng.dev);
-    timer_delete(rng.rate_limit_timer);
     close(source_fd);
     unlink(socket_path);
 }
--8<---------------cut here---------------end--------------->8---

-- 
Alex Bennée


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v3 2/4] vhost-user-rng-pci: Add vhost-user-rng-pci implementation
  2021-07-10  0:59 ` [PATCH v3 2/4] vhost-user-rng-pci: Add vhost-user-rng-pci implementation Mathieu Poirier
  2021-07-21  8:56   ` Alex Bennée
@ 2021-07-21 20:15   ` Alex Bennée
  1 sibling, 0 replies; 15+ messages in thread
From: Alex Bennée @ 2021-07-21 20:15 UTC (permalink / raw)
  To: Mathieu Poirier; +Cc: qemu-devel, mst


Mathieu Poirier <mathieu.poirier@linaro.org> writes:

> This patch provides a PCI bus interface to the vhost-user-rng backed.
> The implentation is similar to what was done for vhost-user-i2c-pci and
> vhost-user-fs-pci.
>
> Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
> ---
<snip>
> +static void vhost_user_rng_pci_class_init(ObjectClass *klass, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(klass);
> +    VirtioPCIClass *k = VIRTIO_PCI_CLASS(klass);
> +    PCIDeviceClass *pcidev_k = PCI_DEVICE_CLASS(klass);
> +    k->realize = vhost_user_rng_pci_realize;
> +    set_bit(DEVICE_CATEGORY_INPUT, dc->categories);
> +    device_class_set_props(dc, vhost_user_rng_pci_properties);
> +    pcidev_k->vendor_id = PCI_VENDOR_ID_REDHAT_QUMRANET;
> +    pcidev_k->device_id = 0; /* Set by virtio-pci based on virtio id */
> +    pcidev_k->revision = 0x00;
> +    pcidev_k->class_id = PCI_CLASS_COMMUNICATION_OTHER;

I noticed the other RNGs use:

 pcidev_k->class_id = PCI_CLASS_OTHERS;

-- 
Alex Bennée


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v3 1/4] vhost-user-rng: Add vhost-user-rng implementation
  2021-07-21  8:52   ` Alex Bennée
@ 2021-07-22 16:44     ` Mathieu Poirier
  0 siblings, 0 replies; 15+ messages in thread
From: Mathieu Poirier @ 2021-07-22 16:44 UTC (permalink / raw)
  To: Alex Bennée; +Cc: qemu-devel, mst

Hi Alex,

On Wed, Jul 21, 2021 at 09:52:50AM +0100, Alex Bennée wrote:
> 
> Mathieu Poirier <mathieu.poirier@linaro.org> writes:
> 
> > Following in the footsteps of what whas done for vhost-user-i2c
> > and virtiofsd, introduce a random number generator (RNG) backend
> > that communicates with a vhost-user server to retrieve entropy.
> > That way another VMM could be using the same vhost-user daemon and
> > avoid having to write yet another RNG driver.
> >
> > Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
> > ---
> >  hw/virtio/Kconfig                  |   5 +
> >  hw/virtio/meson.build              |   1 +
> 
> FWIW there are simple merge failures for the meson and Kconfig parts due
> to I2C being merged.
> 

Right, merging I2C before this set will definitely break things.

> <snip>
> > +
> > +    rng->vhost_dev.nvqs = 1;
> > +    rng->vhost_dev.vqs = g_new0(struct vhost_virtqueue, rng->vhost_dev.nvqs);
> > +    if (!rng->vhost_dev.vqs) {
> > +        error_setg_errno(errp, -1, "memory allocation failed");
> > +        goto vhost_dev_init_failed;
> > +    }
> 
> g_new0 will abort on memory allocation failure (which is fine for
> userspace ;-) so you don't need the check and erro handling exit here.
> 

Ok, I'll fix this.

> > +
> > +    ret = vhost_dev_init(&rng->vhost_dev, &rng->vhost_user,
> > +                         VHOST_BACKEND_TYPE_USER, 0, errp);
> > +    if (ret < 0) {
> > +        error_setg_errno(errp, -ret, "vhost_dev_init() failed");
> > +        goto vhost_dev_init_failed;
> > +    }
> > +
> > +    qemu_chr_fe_set_handlers(&rng->chardev, NULL, NULL, vu_rng_event, NULL,
> > +                             dev, NULL, true);
> > +
> > +    return;
> > +
> > +vhost_dev_init_failed:
> > +    virtio_delete_queue(rng->req_vq);
> > +virtio_add_queue_failed:
> > +    virtio_cleanup(vdev);
> > +    vhost_user_cleanup(&rng->vhost_user);
> > +}
> > +
> > +static void vu_rng_device_unrealize(DeviceState *dev)
> > +{
> > +    VirtIODevice *vdev = VIRTIO_DEVICE(dev);
> > +    VHostUserRNG *rng = VHOST_USER_RNG(dev);
> > +
> > +    vu_rng_set_status(vdev, 0);
> > +
> > +    vhost_dev_cleanup(&rng->vhost_dev);
> > +    g_free(rng->vhost_dev.vqs);
> > +    rng->vhost_dev.vqs = NULL;
> > +    virtio_delete_queue(rng->req_vq);
> > +    virtio_cleanup(vdev);
> > +    vhost_user_cleanup(&rng->vhost_user);
> > +}
> > +
> > +static const VMStateDescription vu_rng_vmstate = {
> > +    .name = "vhost-user-rng",
> > +    .unmigratable = 1,
> > +};
> > +
> > +static Property vu_rng_properties[] = {
> > +    DEFINE_PROP_CHR("chardev", VHostUserRNG, chardev),
> > +    DEFINE_PROP_END_OF_LIST(),
> > +};
> > +
> > +static void vu_rng_class_init(ObjectClass *klass, void *data)
> > +{
> > +    DeviceClass *dc = DEVICE_CLASS(klass);
> > +    VirtioDeviceClass *vdc = VIRTIO_DEVICE_CLASS(klass);
> > +
> > +    device_class_set_props(dc, vu_rng_properties);
> > +    dc->vmsd = &vu_rng_vmstate;
> > +    set_bit(DEVICE_CATEGORY_INPUT, dc->categories);
> > +
> > +    vdc->realize = vu_rng_device_realize;
> > +    vdc->unrealize = vu_rng_device_unrealize;
> > +    vdc->get_features = vu_rng_get_features;
> > +    vdc->set_status = vu_rng_set_status;
> > +    vdc->guest_notifier_mask = vu_rng_guest_notifier_mask;
> > +    vdc->guest_notifier_pending = vu_rng_guest_notifier_pending;
> > +}
> > +
> > +static const TypeInfo vu_rng_info = {
> > +    .name = TYPE_VHOST_USER_RNG,
> > +    .parent = TYPE_VIRTIO_DEVICE,
> > +    .instance_size = sizeof(VHostUserRNG),
> > +    .class_init = vu_rng_class_init,
> > +};
> > +
> > +static void vu_rng_register_types(void)
> > +{
> > +    type_register_static(&vu_rng_info);
> > +}
> > +
> > +type_init(vu_rng_register_types)
> > diff --git a/include/hw/virtio/vhost-user-rng.h b/include/hw/virtio/vhost-user-rng.h
> > new file mode 100644
> > index 000000000000..071539996d1d
> > --- /dev/null
> > +++ b/include/hw/virtio/vhost-user-rng.h
> > @@ -0,0 +1,33 @@
> > +/*
> > + * Vhost-user RNG virtio device
> > + *
> > + * Copyright (c) 2021 Mathieu Poirier <mathieu.poirier@linaro.org>
> > + *
> > + * SPDX-License-Identifier: GPL-2.0-or-later
> > + */
> > +
> > +#ifndef _QEMU_VHOST_USER_RNG_H
> > +#define _QEMU_VHOST_USER_RNG_H
> > +
> > +#include "hw/virtio/virtio.h"
> > +#include "hw/virtio/vhost.h"
> > +#include "hw/virtio/vhost-user.h"
> > +#include "chardev/char-fe.h"
> > +
> > +#define TYPE_VHOST_USER_RNG "vhost-user-rng"
> > +OBJECT_DECLARE_SIMPLE_TYPE(VHostUserRNG, VHOST_USER_RNG)
> > +
> > +struct VHostUserRNG {
> > +    /*< private >*/
> > +    VirtIODevice parent;
> > +    CharBackend chardev;
> > +    struct vhost_virtqueue *vhost_vq;
> > +    struct vhost_dev vhost_dev;
> > +    VhostUserState vhost_user;
> > +    VirtQueue *req_vq;
> > +    bool connected;
> > +
> > +    /*< public >*/
> > +};
> > +
> > +#endif /* _QEMU_VHOST_USER_RNG_H */
> 
> Otherwise:
> 
> Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

Thanks for taking the time to review this set.

> 
> -- 
> Alex Bennée


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v3 3/4] vhost-user-rng: backend: Add RNG vhost-user daemon implementation
  2021-07-21 20:14   ` Alex Bennée
@ 2021-07-22 17:54     ` Mathieu Poirier
  2021-07-23  9:01       ` Alex Bennée
  0 siblings, 1 reply; 15+ messages in thread
From: Mathieu Poirier @ 2021-07-22 17:54 UTC (permalink / raw)
  To: Alex Bennée; +Cc: qemu-devel, mst

On Wed, Jul 21, 2021 at 09:14:31PM +0100, Alex Bennée wrote:
> 
> Mathieu Poirier <mathieu.poirier@linaro.org> writes:
> 
> > This patch provides the vhost-user backend implementation to work
> > in tandem with the vhost-user-rng implementation of the QEMU VMM.
> >
> > It uses the vhost-user API so that other VMM can re-use the interface
> > without having to write the driver again.
> >
> > Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
> 
> Try the following patch which creates a nested main loop and runs it
> until the g_timeout_add fires again.
> 
> --8<---------------cut here---------------start------------->8---
> tools/virtio/vhost-user-rng: avoid mutex by using nested main loop
> 
> As we are blocking anyway all we really need to do is run a main loop
> until the timer fires and the data is consumed.
> 

Right, I made the implemenation blocking to be as close as possible to what
virtio-rng does.

I took a look at your patch below and it should do the trick.  Testing yielded
the same results as my solution so this is good.  To me the nested loop is a
little unorthodox to solve this kind of problem but it has less lines of code
and avoids spinning a new thread to deal with the timer.

I'll send another revision.

Thanks for the review,
Mathieu

> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> 
> 1 file changed, 30 insertions(+), 76 deletions(-)
> tools/vhost-user-rng/main.c | 106 +++++++++++++-------------------------------
> 
> modified   tools/vhost-user-rng/main.c
> @@ -42,13 +42,10 @@
>  
>  typedef struct {
>      VugDev dev;
> -    struct itimerspec ts;
> -    timer_t rate_limit_timer;
> -    pthread_mutex_t rng_mutex;
> -    pthread_cond_t rng_cond;
>      int64_t quota_remaining;
> -    bool activate_timer;
> +    guint timer;
>      GMainLoop *loop;
> +    GMainLoop *blocked;
>  } VuRNG;
>  
>  static gboolean print_cap, verbose;
> @@ -59,66 +56,26 @@ static gint source_fd, socket_fd = -1;
>  static uint32_t period_ms = 1 << 16;
>  static uint64_t max_bytes = INT64_MAX;
>  
> -static void check_rate_limit(union sigval sv)
> +static gboolean check_rate_limit(gpointer user_data)
>  {
> -    VuRNG *rng = sv.sival_ptr;
> -    bool wakeup = false;
> +    VuRNG *rng = (VuRNG *) user_data;
>  
> -    pthread_mutex_lock(&rng->rng_mutex);
> -    /*
> -     * The timer has expired and the guest has used all available
> -     * entropy, which means function vu_rng_handle_request() is waiting
> -     * on us.  As such wake it up once we're done here.
> -     */
> -    if (rng->quota_remaining == 0) {
> -        wakeup = true;
> +    if (rng->blocked) {
> +        g_info("%s: called while blocked", __func__);
> +        g_main_loop_quit(rng->blocked);
>      }
> -
>      /*
>       * Reset the entropy available to the guest and tell function
>       * vu_rng_handle_requests() to start the timer before using it.
>       */
>      rng->quota_remaining = max_bytes;
> -    rng->activate_timer = true;
> -    pthread_mutex_unlock(&rng->rng_mutex);
> -
> -    if (wakeup) {
> -        pthread_cond_signal(&rng->rng_cond);
> -    }
> -}
> -
> -static void setup_timer(VuRNG *rng)
> -{
> -    struct sigevent sev;
> -    int ret;
> -
> -    memset(&rng->ts, 0, sizeof(struct itimerspec));
> -    rng->ts.it_value.tv_sec = period_ms / 1000;
> -    rng->ts.it_value.tv_nsec = (period_ms % 1000) * 1000000;
> -
> -    /*
> -     * Call function check_rate_limit() as if it was the start of
> -     * a new thread when the timer expires.
> -     */
> -    sev.sigev_notify = SIGEV_THREAD;
> -    sev.sigev_notify_function = check_rate_limit;
> -    sev.sigev_value.sival_ptr = rng;
> -    /* Needs to be NULL if defaults attributes are to be used. */
> -    sev.sigev_notify_attributes = NULL;
> -    ret = timer_create(CLOCK_MONOTONIC, &sev, &rng->rate_limit_timer);
> -    if (ret < 0) {
> -        fprintf(stderr, "timer_create() failed\n");
> -    }
> -
> +    return true;
>  }
>  
> -
>  /* Virtio helpers */
>  static uint64_t rng_get_features(VuDev *dev)
>  {
> -    if (verbose) {
> -        g_info("%s: replying", __func__);
> -    }
> +    g_info("%s: replying", __func__);
>      return 0;
>  }
>  
> @@ -137,7 +94,7 @@ static void vu_rng_handle_requests(VuDev *dev, int qidx)
>      VuVirtq *vq = vu_get_queue(dev, qidx);
>      VuVirtqElement *elem;
>      size_t to_read;
> -    int len, ret;
> +    int len;
>  
>      for (;;) {
>          /* Get element in the vhost virtqueue */
> @@ -149,24 +106,21 @@ static void vu_rng_handle_requests(VuDev *dev, int qidx)
>          /* Get the amount of entropy to read from the vhost server */
>          to_read = elem->in_sg[0].iov_len;
>  
> -        pthread_mutex_lock(&rng->rng_mutex);
> -
>          /*
>           * We have consumed all entropy available for this time slice.
>           * Wait for the timer (check_rate_limit()) to tell us about the
>           * start of a new time slice.
>           */
>          if (rng->quota_remaining == 0) {
> -            pthread_cond_wait(&rng->rng_cond, &rng->rng_mutex);
> -        }
> -
> -        /* Start the timer if the last time slice has expired */
> -        if (rng->activate_timer == true) {
> -            rng->activate_timer = false;
> -            ret = timer_settime(rng->rate_limit_timer, 0, &rng->ts, NULL);
> -            if (ret < 0) {
> -                fprintf(stderr, "timer_settime() failed\n");
> -            }
> +            g_assert(!rng->blocked);
> +            rng->blocked = g_main_loop_new(g_main_loop_get_context(rng->loop), false);
> +            g_info("attempting to consume %ld bytes but no quota left (%s)",
> +                   to_read,
> +                   g_main_loop_is_running(rng->loop) ? "running" : "not running");
> +            g_main_loop_run(rng->blocked);
> +            g_info("return from blocked loop: %ld", rng->quota_remaining);
> +            g_main_loop_unref(rng->blocked);
> +            rng->blocked = false;
>          }
>  
>          /* Make sure we don't read more than it's available */
> @@ -183,8 +137,6 @@ static void vu_rng_handle_requests(VuDev *dev, int qidx)
>  
>          rng->quota_remaining -= len;
>  
> -        pthread_mutex_unlock(&rng->rng_mutex);
> -
>          vu_queue_push(dev, vq, elem, len);
>          free(elem);
>      }
> @@ -373,6 +325,7 @@ int main(int argc, char *argv[])
>       * can add it's GSource watches.
>       */
>      rng.loop = g_main_loop_new(NULL, FALSE);
> +    rng.blocked = NULL;
>  
>      if (!vug_init(&rng.dev, 1, g_socket_get_fd(socket),
>                    panic, &vuiface)) {
> @@ -380,24 +333,25 @@ int main(int argc, char *argv[])
>          exit(EXIT_FAILURE);
>      }
>  
> -    rng.quota_remaining = max_bytes;
> -    rng.activate_timer = true;
> -    pthread_mutex_init(&rng.rng_mutex, NULL);
> -    pthread_cond_init(&rng.rng_cond, NULL);
> -    setup_timer(&rng);
> -
>      if (verbose) {
> -        g_info("period_ms: %d tv_sec: %ld tv_nsec: %lu\n",
> -               period_ms, rng.ts.it_value.tv_sec, rng.ts.it_value.tv_nsec);
> +        g_log_set_handler(NULL, G_LOG_LEVEL_MASK, g_log_default_handler, NULL);
> +        g_setenv("G_MESSAGES_DEBUG", "all", true);
> +    } else {
> +        g_log_set_handler(NULL,
> +                          G_LOG_LEVEL_WARNING | G_LOG_LEVEL_CRITICAL | G_LOG_LEVEL_ERROR,
> +                          g_log_default_handler, NULL);
>      }
>  
> +    rng.quota_remaining = max_bytes;
> +    rng.timer = g_timeout_add(period_ms, check_rate_limit, &rng);
> +    g_info("period_ms: %"PRId32", timer %d\n", period_ms, rng.timer);
> +
>      g_message("entering main loop, awaiting messages");
>      g_main_loop_run(rng.loop);
>      g_message("finished main loop, cleaning up");
>  
>      g_main_loop_unref(rng.loop);
>      vug_deinit(&rng.dev);
> -    timer_delete(rng.rate_limit_timer);
>      close(source_fd);
>      unlink(socket_path);
>  }
> --8<---------------cut here---------------end--------------->8---
> 
> -- 
> Alex Bennée


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v3 3/4] vhost-user-rng: backend: Add RNG vhost-user daemon implementation
  2021-07-22 17:54     ` Mathieu Poirier
@ 2021-07-23  9:01       ` Alex Bennée
  0 siblings, 0 replies; 15+ messages in thread
From: Alex Bennée @ 2021-07-23  9:01 UTC (permalink / raw)
  To: Mathieu Poirier; +Cc: qemu-devel, mst


Mathieu Poirier <mathieu.poirier@linaro.org> writes:

> On Wed, Jul 21, 2021 at 09:14:31PM +0100, Alex Bennée wrote:
>> 
>> Mathieu Poirier <mathieu.poirier@linaro.org> writes:
>> 
>> > This patch provides the vhost-user backend implementation to work
>> > in tandem with the vhost-user-rng implementation of the QEMU VMM.
>> >
>> > It uses the vhost-user API so that other VMM can re-use the interface
>> > without having to write the driver again.
>> >
>> > Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
>> 
>> Try the following patch which creates a nested main loop and runs it
>> until the g_timeout_add fires again.
>> 
>> --8<---------------cut here---------------start------------->8---
>> tools/virtio/vhost-user-rng: avoid mutex by using nested main loop
>> 
>> As we are blocking anyway all we really need to do is run a main loop
>> until the timer fires and the data is consumed.
>> 
>
> Right, I made the implemenation blocking to be as close as possible to what
> virtio-rng does.
>
> I took a look at your patch below and it should do the trick.  Testing yielded
> the same results as my solution so this is good.  To me the nested loop is a
> little unorthodox to solve this kind of problem but it has less lines of code
> and avoids spinning a new thread to deal with the timer.

It might be worth considering just running a g_main_context_iteration()
of the main loop:

  https://developer.gnome.org/glib/stable/glib-The-Main-Event-Loop.html#g-main-context-iteration

and then you could avoid the hassle of having a special blocking loop
(and having to special case the quit). However you can't be sure the
quota has filled up so you need to test on the loop. My hack up (on top
of this patch):

--8<---------------cut here---------------start------------->8---
modified   tools/vhost-user-rng/main.c
@@ -45,7 +45,6 @@ typedef struct {
     int64_t quota_remaining;
     guint timer;
     GMainLoop *loop;
-    GMainLoop *blocked;
 } VuRNG;
 
 static gboolean print_cap, verbose;
@@ -59,10 +58,8 @@ static uint64_t max_bytes = INT64_MAX;
 static gboolean check_rate_limit(gpointer user_data)
 {
     VuRNG *rng = (VuRNG *) user_data;
-
-    if (rng->blocked) {
-        g_info("%s: called while blocked", __func__);
-        g_main_loop_quit(rng->blocked);
+    if (!rng->quota_remaining) {
+        g_info("%s: replenishing empty quota", __func__);
     }
     /*
      * Reset the entropy available to the guest and tell function
@@ -112,15 +109,12 @@ static void vu_rng_handle_requests(VuDev *dev, int qidx)
          * start of a new time slice.
          */
         if (rng->quota_remaining == 0) {
-            g_assert(!rng->blocked);
-            rng->blocked = g_main_loop_new(g_main_loop_get_context(rng->loop), false);
-            g_info("attempting to consume %ld bytes but no quota left (%s)",
-                   to_read,
-                   g_main_loop_is_running(rng->loop) ? "running" : "not running");
-            g_main_loop_run(rng->blocked);
-            g_info("return from blocked loop: %ld", rng->quota_remaining);
-            g_main_loop_unref(rng->blocked);
-            rng->blocked = false;
+            g_info("blocking on consuming %ld bytes as no quota", to_read);
+            do {
+                g_main_context_iteration(g_main_loop_get_context(rng->loop),
+                                         true);
+                g_info("return from blocked loop: %ld", rng->quota_remaining);
+            } while (!rng->quota_remaining);
         }
 
         /* Make sure we don't read more than it's available */
@@ -325,7 +319,6 @@ int main(int argc, char *argv[])
      * can add it's GSource watches.
      */
     rng.loop = g_main_loop_new(NULL, FALSE);
-    rng.blocked = NULL;
 
     if (!vug_init(&rng.dev, 1, g_socket_get_fd(socket),
                   panic, &vuiface)) {
--8<---------------cut here---------------end--------------->8---

And then with:

  10:24 root@buster/aarch64  [~] >dd if=/dev/hwrng of=/dev/null status=progress
  77312 bytes (77 kB, 76 KiB) copied, 150 s, 0.5 kB/s

and:

  ./tools/vhost-user-rng/vhost-user-rng --socket-path vrng.sock -v -m 512 -p 1000

I was seeing:

  vhost-user-rng-INFO: 10:27:14.453: blocking on consuming 64 bytes as no quota
  vhost-user-rng-INFO: 10:27:14.453: return from blocked loop: 0
  vhost-user-rng-INFO: 10:27:15.451: check_rate_limit: replenishing empty quota
  vhost-user-rng-INFO: 10:27:15.451: return from blocked loop: 512
  vhost-user-rng-INFO: 10:27:15.457: blocking on consuming 64 bytes as no quota
  vhost-user-rng-INFO: 10:27:15.457: return from blocked loop: 0
  vhost-user-rng-INFO: 10:27:16.453: check_rate_limit: replenishing empty quota
  vhost-user-rng-INFO: 10:27:16.453: return from blocked loop: 512
  vhost-user-rng-INFO: 10:27:16.456: blocking on consuming 64 bytes as no quota
  vhost-user-rng-INFO: 10:27:16.456: return from blocked loop: 0
  vhost-user-rng-INFO: 10:27:17.454: check_rate_limit: replenishing empty quota
  vhost-user-rng-INFO: 10:27:17.454: return from blocked loop: 512
  vhost-user-rng-INFO: 10:27:17.456: blocking on consuming 64 bytes as no quota
  vhost-user-rng-INFO: 10:27:17.456: return from blocked loop: 0

which seemed reasonable enough...

>
> I'll send another revision.
>
> Thanks for the review,
> Mathieu
>
>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>> 
>> 1 file changed, 30 insertions(+), 76 deletions(-)
>> tools/vhost-user-rng/main.c | 106 +++++++++++++-------------------------------
>> 
>> modified   tools/vhost-user-rng/main.c
>> @@ -42,13 +42,10 @@
>>  
>>  typedef struct {
>>      VugDev dev;
>> -    struct itimerspec ts;
>> -    timer_t rate_limit_timer;
>> -    pthread_mutex_t rng_mutex;
>> -    pthread_cond_t rng_cond;
>>      int64_t quota_remaining;
>> -    bool activate_timer;
>> +    guint timer;
>>      GMainLoop *loop;
>> +    GMainLoop *blocked;
>>  } VuRNG;
>>  
>>  static gboolean print_cap, verbose;
>> @@ -59,66 +56,26 @@ static gint source_fd, socket_fd = -1;
>>  static uint32_t period_ms = 1 << 16;
>>  static uint64_t max_bytes = INT64_MAX;
>>  
>> -static void check_rate_limit(union sigval sv)
>> +static gboolean check_rate_limit(gpointer user_data)
>>  {
>> -    VuRNG *rng = sv.sival_ptr;
>> -    bool wakeup = false;
>> +    VuRNG *rng = (VuRNG *) user_data;
>>  
>> -    pthread_mutex_lock(&rng->rng_mutex);
>> -    /*
>> -     * The timer has expired and the guest has used all available
>> -     * entropy, which means function vu_rng_handle_request() is waiting
>> -     * on us.  As such wake it up once we're done here.
>> -     */
>> -    if (rng->quota_remaining == 0) {
>> -        wakeup = true;
>> +    if (rng->blocked) {
>> +        g_info("%s: called while blocked", __func__);
>> +        g_main_loop_quit(rng->blocked);
>>      }
>> -
>>      /*
>>       * Reset the entropy available to the guest and tell function
>>       * vu_rng_handle_requests() to start the timer before using it.
>>       */
>>      rng->quota_remaining = max_bytes;
>> -    rng->activate_timer = true;
>> -    pthread_mutex_unlock(&rng->rng_mutex);
>> -
>> -    if (wakeup) {
>> -        pthread_cond_signal(&rng->rng_cond);
>> -    }
>> -}
>> -
>> -static void setup_timer(VuRNG *rng)
>> -{
>> -    struct sigevent sev;
>> -    int ret;
>> -
>> -    memset(&rng->ts, 0, sizeof(struct itimerspec));
>> -    rng->ts.it_value.tv_sec = period_ms / 1000;
>> -    rng->ts.it_value.tv_nsec = (period_ms % 1000) * 1000000;
>> -
>> -    /*
>> -     * Call function check_rate_limit() as if it was the start of
>> -     * a new thread when the timer expires.
>> -     */
>> -    sev.sigev_notify = SIGEV_THREAD;
>> -    sev.sigev_notify_function = check_rate_limit;
>> -    sev.sigev_value.sival_ptr = rng;
>> -    /* Needs to be NULL if defaults attributes are to be used. */
>> -    sev.sigev_notify_attributes = NULL;
>> -    ret = timer_create(CLOCK_MONOTONIC, &sev, &rng->rate_limit_timer);
>> -    if (ret < 0) {
>> -        fprintf(stderr, "timer_create() failed\n");
>> -    }
>> -
>> +    return true;
>>  }
>>  
>> -
>>  /* Virtio helpers */
>>  static uint64_t rng_get_features(VuDev *dev)
>>  {
>> -    if (verbose) {
>> -        g_info("%s: replying", __func__);
>> -    }
>> +    g_info("%s: replying", __func__);
>>      return 0;
>>  }
>>  
>> @@ -137,7 +94,7 @@ static void vu_rng_handle_requests(VuDev *dev, int qidx)
>>      VuVirtq *vq = vu_get_queue(dev, qidx);
>>      VuVirtqElement *elem;
>>      size_t to_read;
>> -    int len, ret;
>> +    int len;
>>  
>>      for (;;) {
>>          /* Get element in the vhost virtqueue */
>> @@ -149,24 +106,21 @@ static void vu_rng_handle_requests(VuDev *dev, int qidx)
>>          /* Get the amount of entropy to read from the vhost server */
>>          to_read = elem->in_sg[0].iov_len;
>>  
>> -        pthread_mutex_lock(&rng->rng_mutex);
>> -
>>          /*
>>           * We have consumed all entropy available for this time slice.
>>           * Wait for the timer (check_rate_limit()) to tell us about the
>>           * start of a new time slice.
>>           */
>>          if (rng->quota_remaining == 0) {
>> -            pthread_cond_wait(&rng->rng_cond, &rng->rng_mutex);
>> -        }
>> -
>> -        /* Start the timer if the last time slice has expired */
>> -        if (rng->activate_timer == true) {
>> -            rng->activate_timer = false;
>> -            ret = timer_settime(rng->rate_limit_timer, 0, &rng->ts, NULL);
>> -            if (ret < 0) {
>> -                fprintf(stderr, "timer_settime() failed\n");
>> -            }
>> +            g_assert(!rng->blocked);
>> +            rng->blocked = g_main_loop_new(g_main_loop_get_context(rng->loop), false);
>> +            g_info("attempting to consume %ld bytes but no quota left (%s)",
>> +                   to_read,
>> +                   g_main_loop_is_running(rng->loop) ? "running" : "not running");
>> +            g_main_loop_run(rng->blocked);
>> +            g_info("return from blocked loop: %ld", rng->quota_remaining);
>> +            g_main_loop_unref(rng->blocked);
>> +            rng->blocked = false;
>>          }
>>  
>>          /* Make sure we don't read more than it's available */
>> @@ -183,8 +137,6 @@ static void vu_rng_handle_requests(VuDev *dev, int qidx)
>>  
>>          rng->quota_remaining -= len;
>>  
>> -        pthread_mutex_unlock(&rng->rng_mutex);
>> -
>>          vu_queue_push(dev, vq, elem, len);
>>          free(elem);
>>      }
>> @@ -373,6 +325,7 @@ int main(int argc, char *argv[])
>>       * can add it's GSource watches.
>>       */
>>      rng.loop = g_main_loop_new(NULL, FALSE);
>> +    rng.blocked = NULL;
>>  
>>      if (!vug_init(&rng.dev, 1, g_socket_get_fd(socket),
>>                    panic, &vuiface)) {
>> @@ -380,24 +333,25 @@ int main(int argc, char *argv[])
>>          exit(EXIT_FAILURE);
>>      }
>>  
>> -    rng.quota_remaining = max_bytes;
>> -    rng.activate_timer = true;
>> -    pthread_mutex_init(&rng.rng_mutex, NULL);
>> -    pthread_cond_init(&rng.rng_cond, NULL);
>> -    setup_timer(&rng);
>> -
>>      if (verbose) {
>> -        g_info("period_ms: %d tv_sec: %ld tv_nsec: %lu\n",
>> -               period_ms, rng.ts.it_value.tv_sec, rng.ts.it_value.tv_nsec);
>> +        g_log_set_handler(NULL, G_LOG_LEVEL_MASK, g_log_default_handler, NULL);
>> +        g_setenv("G_MESSAGES_DEBUG", "all", true);
>> +    } else {
>> +        g_log_set_handler(NULL,
>> +                          G_LOG_LEVEL_WARNING | G_LOG_LEVEL_CRITICAL | G_LOG_LEVEL_ERROR,
>> +                          g_log_default_handler, NULL);
>>      }
>>  
>> +    rng.quota_remaining = max_bytes;
>> +    rng.timer = g_timeout_add(period_ms, check_rate_limit, &rng);
>> +    g_info("period_ms: %"PRId32", timer %d\n", period_ms, rng.timer);
>> +
>>      g_message("entering main loop, awaiting messages");
>>      g_main_loop_run(rng.loop);
>>      g_message("finished main loop, cleaning up");
>>  
>>      g_main_loop_unref(rng.loop);
>>      vug_deinit(&rng.dev);
>> -    timer_delete(rng.rate_limit_timer);
>>      close(source_fd);
>>      unlink(socket_path);
>>  }
>> --8<---------------cut here---------------end--------------->8---
>> 
>> -- 
>> Alex Bennée


-- 
Alex Bennée


^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2021-07-23  9:30 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-10  0:59 [PATCH v3 0/4] virtio: Add vhost-user based RNG Mathieu Poirier
2021-07-10  0:59 ` [PATCH v3 1/4] vhost-user-rng: Add vhost-user-rng implementation Mathieu Poirier
2021-07-21  8:52   ` Alex Bennée
2021-07-22 16:44     ` Mathieu Poirier
2021-07-10  0:59 ` [PATCH v3 2/4] vhost-user-rng-pci: Add vhost-user-rng-pci implementation Mathieu Poirier
2021-07-21  8:56   ` Alex Bennée
2021-07-21 20:15   ` Alex Bennée
2021-07-10  0:59 ` [PATCH v3 3/4] vhost-user-rng: backend: Add RNG vhost-user daemon implementation Mathieu Poirier
2021-07-21 11:30   ` Alex Bennée
2021-07-21 20:14   ` Alex Bennée
2021-07-22 17:54     ` Mathieu Poirier
2021-07-23  9:01       ` Alex Bennée
2021-07-10  0:59 ` [PATCH v3 4/4] docs: Add documentation for vhost based RNG implementation Mathieu Poirier
2021-07-21 16:55   ` Alex Bennée
2021-07-17 20:32 ` [PATCH v3 0/4] virtio: Add vhost-user based RNG Michael S. Tsirkin

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.