virtio-fs.lists.linux.dev archive mirror
 help / color / mirror / Atom feed
* [Virtio-fs] [PATCH v4 0/6] virtio: cleanup vhost-user-generic and reduce c&p
@ 2023-10-09  9:59 Alex Bennée
  2023-10-09  9:59 ` [Virtio-fs] [PATCH v4 1/6] virtio: split into vhost-user-base and vhost-user-device Alex Bennée
                   ` (6 more replies)
  0 siblings, 7 replies; 14+ messages in thread
From: Alex Bennée @ 2023-10-09  9:59 UTC (permalink / raw)
  To: qemu-devel
  Cc: Marc-André Lureau, Erik Schilling, Fam Zheng,
	Mathieu Poirier, Gerd Hoffmann, Hanna Reitz, Stefan Hajnoczi,
	Alex Bennée, Kevin Wolf, Eric Blake, Michael S. Tsirkin,
	qemu-block, Eduardo Habkost, Paolo Bonzini, Mark Cave-Ayland,
	Viresh Kumar, virtio-fs, Gonglei (Arei),
	Markus Armbruster, Jason Wang, Raphael Norwitz,
	Daniel P. Berrangé

A lot of our vhost-user stubs are large chunks of boilerplate that do
(mostly) the same thing. This series continues the cleanups by
splitting the vhost-user-base and vhost-user-generic implementations.
After adding a new vq_size property the rng, gpio and i2c vhost-user
devices become simple specialisations of the common base defining the
ID, number of queues and potentially the config handling.

I've also added Manos' vhost-user-sound while I was at it.

Changes
-------

I've dropped the F_TRANSPORT work from this series to keep this small
and ready to merge. The changes for F_TRANSPORT are a bit more
invasive and still need a bit of debugging but I wanted to get this
stuff merged now.

Alex Bennée (5):
  virtio: split into vhost-user-base and vhost-user-device
  hw/virtio: derive vhost-user-rng from vhost-user-base
  hw/virtio: derive vhost-user-gpio from vhost-user-base
  hw/virtio: derive vhost-user-i2c from vhost-user-base
  docs/system: add a basic enumeration of vhost-user devices

Manos Pitsidianakis (1):
  hw/virtio: add vhost-user-snd and virtio-snd-pci devices

 docs/system/devices/vhost-user-rng.rst |   2 +
 docs/system/devices/vhost-user.rst     |  41 +++
 include/hw/virtio/vhost-user-base.h    |  49 +++
 include/hw/virtio/vhost-user-gpio.h    |  23 +-
 include/hw/virtio/vhost-user-i2c.h     |  14 +-
 include/hw/virtio/vhost-user-rng.h     |  11 +-
 include/hw/virtio/vhost-user-snd.h     |  26 ++
 hw/virtio/vhost-user-base.c            | 348 +++++++++++++++++++++
 hw/virtio/vhost-user-device-pci.c      |  10 +-
 hw/virtio/vhost-user-device.c          | 335 +-------------------
 hw/virtio/vhost-user-gpio.c            | 407 ++-----------------------
 hw/virtio/vhost-user-i2c.c             | 272 +----------------
 hw/virtio/vhost-user-rng.c             | 278 ++---------------
 hw/virtio/vhost-user-snd-pci.c         |  75 +++++
 hw/virtio/vhost-user-snd.c             |  67 ++++
 hw/virtio/Kconfig                      |   5 +
 hw/virtio/meson.build                  |  25 +-
 17 files changed, 705 insertions(+), 1283 deletions(-)
 create mode 100644 include/hw/virtio/vhost-user-base.h
 create mode 100644 include/hw/virtio/vhost-user-snd.h
 create mode 100644 hw/virtio/vhost-user-base.c
 create mode 100644 hw/virtio/vhost-user-snd-pci.c
 create mode 100644 hw/virtio/vhost-user-snd.c

-- 
2.39.2


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

* [Virtio-fs] [PATCH v4 1/6] virtio: split into vhost-user-base and vhost-user-device
  2023-10-09  9:59 [Virtio-fs] [PATCH v4 0/6] virtio: cleanup vhost-user-generic and reduce c&p Alex Bennée
@ 2023-10-09  9:59 ` Alex Bennée
  2023-10-10 15:08   ` Mark Cave-Ayland
  2023-10-09  9:59 ` [Virtio-fs] [PATCH v4 2/6] hw/virtio: derive vhost-user-rng from vhost-user-base Alex Bennée
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 14+ messages in thread
From: Alex Bennée @ 2023-10-09  9:59 UTC (permalink / raw)
  To: qemu-devel
  Cc: Marc-André Lureau, Erik Schilling, Fam Zheng,
	Mathieu Poirier, Gerd Hoffmann, Hanna Reitz, Stefan Hajnoczi,
	Alex Bennée, Kevin Wolf, Eric Blake, Michael S. Tsirkin,
	qemu-block, Eduardo Habkost, Paolo Bonzini, Mark Cave-Ayland,
	Viresh Kumar, virtio-fs, Gonglei (Arei),
	Markus Armbruster, Jason Wang, Raphael Norwitz,
	Daniel P. Berrangé

Lets keep a cleaner split between the base class and the derived
vhost-user-device which we can use for generic vhost-user stubs. This
includes an update to introduce the vq_size property so the number of
entries in a virtq can be defined.

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

---
v1
  - merge and re-base, reset testing/review tags
---
 include/hw/virtio/vhost-user-base.h |  49 ++++
 hw/virtio/vhost-user-base.c         | 348 ++++++++++++++++++++++++++++
 hw/virtio/vhost-user-device-pci.c   |  10 +-
 hw/virtio/vhost-user-device.c       | 335 +-------------------------
 hw/virtio/meson.build               |   1 +
 5 files changed, 410 insertions(+), 333 deletions(-)
 create mode 100644 include/hw/virtio/vhost-user-base.h
 create mode 100644 hw/virtio/vhost-user-base.c

diff --git a/include/hw/virtio/vhost-user-base.h b/include/hw/virtio/vhost-user-base.h
new file mode 100644
index 0000000000..cad377468b
--- /dev/null
+++ b/include/hw/virtio/vhost-user-base.h
@@ -0,0 +1,49 @@
+/*
+ * Vhost-user generic virtio device
+ *
+ * Copyright (c) 2023 Linaro Ltd
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#ifndef QEMU_VHOST_USER_BASE_H
+#define QEMU_VHOST_USER_BASE_H
+
+#include "hw/virtio/vhost.h"
+#include "hw/virtio/vhost-user.h"
+
+#define TYPE_VHOST_USER_BASE "vhost-user-base"
+
+OBJECT_DECLARE_TYPE(VHostUserBase, VHostUserBaseClass, VHOST_USER_BASE)
+
+struct VHostUserBase {
+    VirtIODevice parent;
+
+    /* Properties */
+    CharBackend chardev;
+    uint16_t virtio_id;
+    uint32_t num_vqs;
+    uint32_t vq_size; /* can't exceed VIRTIO_QUEUE_MAX */
+    uint32_t config_size;
+    /* State tracking */
+    VhostUserState vhost_user;
+    struct vhost_virtqueue *vhost_vq;
+    struct vhost_dev vhost_dev;
+    GPtrArray *vqs;
+    bool connected;
+};
+
+/*
+ * Needed so we can use the base realize after specialisation
+ * tweaks
+ */
+struct VHostUserBaseClass {
+    VirtioDeviceClass parent_class;
+
+    DeviceRealize parent_realize;
+};
+
+
+#define TYPE_VHOST_USER_DEVICE "vhost-user-device"
+
+#endif /* QEMU_VHOST_USER_BASE_H */
diff --git a/hw/virtio/vhost-user-base.c b/hw/virtio/vhost-user-base.c
new file mode 100644
index 0000000000..a8b811c394
--- /dev/null
+++ b/hw/virtio/vhost-user-base.c
@@ -0,0 +1,348 @@
+/*
+ * Base vhost-user-base implementation. This can be used to derive a
+ * more fully specified vhost-user backend either generically (see
+ * vhost-user-device) or via a specific stub for a device which
+ * encapsulates some fixed parameters.
+ *
+ * Copyright (c) 2023 Linaro Ltd
+ * Author: Alex Bennée <alex.bennee@linaro.org>
+ *
+ * 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-base.h"
+#include "qemu/error-report.h"
+
+static void vub_start(VirtIODevice *vdev)
+{
+    BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(vdev)));
+    VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
+    VHostUserBase *vub = VHOST_USER_BASE(vdev);
+    int ret, i;
+
+    if (!k->set_guest_notifiers) {
+        error_report("binding does not support guest notifiers");
+        return;
+    }
+
+    ret = vhost_dev_enable_notifiers(&vub->vhost_dev, vdev);
+    if (ret < 0) {
+        error_report("Error enabling host notifiers: %d", -ret);
+        return;
+    }
+
+    ret = k->set_guest_notifiers(qbus->parent, vub->vhost_dev.nvqs, true);
+    if (ret < 0) {
+        error_report("Error binding guest notifier: %d", -ret);
+        goto err_host_notifiers;
+    }
+
+    vub->vhost_dev.acked_features = vdev->guest_features;
+
+    ret = vhost_dev_start(&vub->vhost_dev, vdev, true);
+    if (ret < 0) {
+        error_report("Error starting vhost-user-base: %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 < vub->vhost_dev.nvqs; i++) {
+        vhost_virtqueue_mask(&vub->vhost_dev, vdev, i, false);
+    }
+
+    return;
+
+err_guest_notifiers:
+    k->set_guest_notifiers(qbus->parent, vub->vhost_dev.nvqs, false);
+err_host_notifiers:
+    vhost_dev_disable_notifiers(&vub->vhost_dev, vdev);
+}
+
+static void vub_stop(VirtIODevice *vdev)
+{
+    VHostUserBase *vub = VHOST_USER_BASE(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(&vub->vhost_dev, vdev, true);
+
+    ret = k->set_guest_notifiers(qbus->parent, vub->vhost_dev.nvqs, false);
+    if (ret < 0) {
+        error_report("vhost guest notifier cleanup failed: %d", ret);
+        return;
+    }
+
+    vhost_dev_disable_notifiers(&vub->vhost_dev, vdev);
+}
+
+static void vub_set_status(VirtIODevice *vdev, uint8_t status)
+{
+    VHostUserBase *vub = VHOST_USER_BASE(vdev);
+    bool should_start = virtio_device_should_start(vdev, status);
+
+    if (vhost_dev_is_started(&vub->vhost_dev) == should_start) {
+        return;
+    }
+
+    if (should_start) {
+        vub_start(vdev);
+    } else {
+        vub_stop(vdev);
+    }
+}
+
+/*
+ * For an implementation where everything is delegated to the backend
+ * we don't do anything other than return the full feature set offered
+ * by the daemon (module the reserved feature bit).
+ */
+static uint64_t vub_get_features(VirtIODevice *vdev,
+                                 uint64_t requested_features, Error **errp)
+{
+    VHostUserBase *vub = VHOST_USER_BASE(vdev);
+    /* This should be set when the vhost connection initialises */
+    g_assert(vub->vhost_dev.features);
+    return vub->vhost_dev.features & ~(1ULL << VHOST_USER_F_PROTOCOL_FEATURES);
+}
+
+/*
+ * To handle VirtIO config we need to know the size of the config
+ * space. We don't cache the config but re-fetch it from the guest
+ * every time in case something has changed.
+ */
+static void vub_get_config(VirtIODevice *vdev, uint8_t *config)
+{
+    VHostUserBase *vub = VHOST_USER_BASE(vdev);
+    Error *local_err = NULL;
+
+    /*
+     * There will have been a warning during vhost_dev_init, but lets
+     * assert here as nothing will go right now.
+     */
+    g_assert(vub->config_size && vub->vhost_user.supports_config == true);
+
+    if (vhost_dev_get_config(&vub->vhost_dev, config,
+                             vub->config_size, &local_err)) {
+        error_report_err(local_err);
+    }
+}
+
+/*
+ * When the daemon signals an update to the config we just need to
+ * signal the guest as we re-read the config on demand above.
+ */
+static int vub_config_notifier(struct vhost_dev *dev)
+{
+    virtio_notify_config(dev->vdev);
+    return 0;
+}
+
+const VhostDevConfigOps vub_config_ops = {
+    .vhost_dev_config_notifier = vub_config_notifier,
+};
+
+static void vub_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 do_vhost_user_cleanup(VirtIODevice *vdev, VHostUserBase *vub)
+{
+    vhost_user_cleanup(&vub->vhost_user);
+
+    for (int i = 0; i < vub->num_vqs; i++) {
+        VirtQueue *vq = g_ptr_array_index(vub->vqs, i);
+        virtio_delete_queue(vq);
+    }
+
+    virtio_cleanup(vdev);
+}
+
+static int vub_connect(DeviceState *dev)
+{
+    VirtIODevice *vdev = VIRTIO_DEVICE(dev);
+    VHostUserBase *vub = VHOST_USER_BASE(vdev);
+    struct vhost_dev *vhost_dev = &vub->vhost_dev;
+
+    if (vub->connected) {
+        return 0;
+    }
+    vub->connected = true;
+
+    /*
+     * If we support VHOST_USER_GET_CONFIG we must enable the notifier
+     * so we can ping the guest when it updates.
+     */
+    if (vub->vhost_user.supports_config) {
+        vhost_dev_set_config_notifier(vhost_dev, &vub_config_ops);
+    }
+
+    /* restore vhost state */
+    if (virtio_device_started(vdev, vdev->status)) {
+        vub_start(vdev);
+    }
+
+    return 0;
+}
+
+static void vub_disconnect(DeviceState *dev)
+{
+    VirtIODevice *vdev = VIRTIO_DEVICE(dev);
+    VHostUserBase *vub = VHOST_USER_BASE(vdev);
+
+    if (!vub->connected) {
+        return;
+    }
+    vub->connected = false;
+
+    if (vhost_dev_is_started(&vub->vhost_dev)) {
+        vub_stop(vdev);
+    }
+}
+
+static void vub_event(void *opaque, QEMUChrEvent event)
+{
+    DeviceState *dev = opaque;
+    VirtIODevice *vdev = VIRTIO_DEVICE(dev);
+    VHostUserBase *vub = VHOST_USER_BASE(vdev);
+
+    switch (event) {
+    case CHR_EVENT_OPENED:
+        if (vub_connect(dev) < 0) {
+            qemu_chr_fe_disconnect(&vub->chardev);
+            return;
+        }
+        break;
+    case CHR_EVENT_CLOSED:
+        vub_disconnect(dev);
+        break;
+    case CHR_EVENT_BREAK:
+    case CHR_EVENT_MUX_IN:
+    case CHR_EVENT_MUX_OUT:
+        /* Ignore */
+        break;
+    }
+}
+
+static void vub_device_realize(DeviceState *dev, Error **errp)
+{
+    VirtIODevice *vdev = VIRTIO_DEVICE(dev);
+    VHostUserBase *vub = VHOST_USER_BASE(dev);
+    int ret;
+
+    if (!vub->chardev.chr) {
+        error_setg(errp, "vhost-user-base: missing chardev");
+        return;
+    }
+
+    if (!vub->virtio_id) {
+        error_setg(errp, "vhost-user-base: need to define device id");
+        return;
+    }
+
+    if (!vub->num_vqs) {
+        vub->num_vqs = 1; /* reasonable default? */
+    }
+
+    if (!vub->vq_size) {
+        vub->vq_size = 64;
+    }
+
+    /*
+     * We can't handle config requests unless we know the size of the
+     * config region, specialisations of the vhost-user-base will be
+     * able to set this.
+     */
+    if (vub->config_size) {
+        vub->vhost_user.supports_config = true;
+    }
+
+    if (!vhost_user_init(&vub->vhost_user, &vub->chardev, errp)) {
+        return;
+    }
+
+    virtio_init(vdev, vub->virtio_id, vub->config_size);
+
+    /*
+     * Disable guest notifiers, by default all notifications will be via the
+     * asynchronous vhost-user socket.
+     */
+    vdev->use_guest_notifier_mask = false;
+
+    /* Allocate queues */
+    vub->vqs = g_ptr_array_sized_new(vub->num_vqs);
+    for (int i = 0; i < vub->num_vqs; i++) {
+        g_ptr_array_add(vub->vqs,
+                        virtio_add_queue(vdev, vub->vq_size, vub_handle_output));
+    }
+
+    vub->vhost_dev.nvqs = vub->num_vqs;
+    vub->vhost_dev.vqs = g_new0(struct vhost_virtqueue, vub->vhost_dev.nvqs);
+
+    /* connect to backend */
+    ret = vhost_dev_init(&vub->vhost_dev, &vub->vhost_user,
+                         VHOST_BACKEND_TYPE_USER, 0, errp);
+
+    if (ret < 0) {
+        do_vhost_user_cleanup(vdev, vub);
+    }
+
+    qemu_chr_fe_set_handlers(&vub->chardev, NULL, NULL, vub_event, NULL,
+                             dev, NULL, true);
+}
+
+static void vub_device_unrealize(DeviceState *dev)
+{
+    VirtIODevice *vdev = VIRTIO_DEVICE(dev);
+    VHostUserBase *vub = VHOST_USER_BASE(dev);
+    struct vhost_virtqueue *vhost_vqs = vub->vhost_dev.vqs;
+
+    /* This will stop vhost backend if appropriate. */
+    vub_set_status(vdev, 0);
+    vhost_dev_cleanup(&vub->vhost_dev);
+    g_free(vhost_vqs);
+    do_vhost_user_cleanup(vdev, vub);
+}
+
+static void vub_class_init(ObjectClass *klass, void *data)
+{
+    VirtioDeviceClass *vdc = VIRTIO_DEVICE_CLASS(klass);
+
+    vdc->realize = vub_device_realize;
+    vdc->unrealize = vub_device_unrealize;
+    vdc->get_features = vub_get_features;
+    vdc->get_config = vub_get_config;
+    vdc->set_status = vub_set_status;
+}
+
+static const TypeInfo vub_info = {
+    .name = TYPE_VHOST_USER_BASE,
+    .parent = TYPE_VIRTIO_DEVICE,
+    .instance_size = sizeof(VHostUserBase),
+    .class_init = vub_class_init,
+    .class_size = sizeof(VHostUserBaseClass),
+    .abstract = true
+};
+
+static void vu_register_types(void)
+{
+    type_register_static(&vub_info);
+}
+
+type_init(vu_register_types)
diff --git a/hw/virtio/vhost-user-device-pci.c b/hw/virtio/vhost-user-device-pci.c
index 41f9b7905b..22a5e95b9c 100644
--- a/hw/virtio/vhost-user-device-pci.c
+++ b/hw/virtio/vhost-user-device-pci.c
@@ -9,21 +9,18 @@
 
 #include "qemu/osdep.h"
 #include "hw/qdev-properties.h"
-#include "hw/virtio/vhost-user-device.h"
+#include "hw/virtio/vhost-user-base.h"
 #include "hw/virtio/virtio-pci.h"
 
 struct VHostUserDevicePCI {
     VirtIOPCIProxy parent_obj;
+
     VHostUserBase vub;
 };
 
-typedef struct VHostUserDevicePCI VHostUserDevicePCI;
-
 #define TYPE_VHOST_USER_DEVICE_PCI "vhost-user-device-pci-base"
 
-DECLARE_INSTANCE_CHECKER(VHostUserDevicePCI,
-                         VHOST_USER_DEVICE_PCI,
-                         TYPE_VHOST_USER_DEVICE_PCI)
+OBJECT_DECLARE_SIMPLE_TYPE(VHostUserDevicePCI, VHOST_USER_DEVICE_PCI)
 
 static void vhost_user_device_pci_realize(VirtIOPCIProxy *vpci_dev, Error **errp)
 {
@@ -39,6 +36,7 @@ static void vhost_user_device_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_device_pci_realize;
     set_bit(DEVICE_CATEGORY_INPUT, dc->categories);
     pcidev_k->vendor_id = PCI_VENDOR_ID_REDHAT_QUMRANET;
diff --git a/hw/virtio/vhost-user-device.c b/hw/virtio/vhost-user-device.c
index 2b028cae08..4adc1e8991 100644
--- a/hw/virtio/vhost-user-device.c
+++ b/hw/virtio/vhost-user-device.c
@@ -1,7 +1,10 @@
 /*
- * Generic vhost-user stub. This can be used to connect to any
- * vhost-user backend. All configuration details must be handled by
- * the vhost-user daemon itself
+ * Generic vhost-user-device implementation for any vhost-user-backend
+ *
+ * This is a concrete implementation of vhost-user-base which can be
+ * configured via properties. It is useful for development and
+ * prototyping. It expects configuration details (if any) to be
+ * handled by the vhost-user daemon itself.
  *
  * Copyright (c) 2023 Linaro Ltd
  * Author: Alex Bennée <alex.bennee@linaro.org>
@@ -13,329 +16,9 @@
 #include "qapi/error.h"
 #include "hw/qdev-properties.h"
 #include "hw/virtio/virtio-bus.h"
-#include "hw/virtio/vhost-user-device.h"
+#include "hw/virtio/vhost-user-base.h"
 #include "qemu/error-report.h"
 
-static void vub_start(VirtIODevice *vdev)
-{
-    BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(vdev)));
-    VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
-    VHostUserBase *vub = VHOST_USER_BASE(vdev);
-    int ret, i;
-
-    if (!k->set_guest_notifiers) {
-        error_report("binding does not support guest notifiers");
-        return;
-    }
-
-    ret = vhost_dev_enable_notifiers(&vub->vhost_dev, vdev);
-    if (ret < 0) {
-        error_report("Error enabling host notifiers: %d", -ret);
-        return;
-    }
-
-    ret = k->set_guest_notifiers(qbus->parent, vub->vhost_dev.nvqs, true);
-    if (ret < 0) {
-        error_report("Error binding guest notifier: %d", -ret);
-        goto err_host_notifiers;
-    }
-
-    vub->vhost_dev.acked_features = vdev->guest_features;
-
-    ret = vhost_dev_start(&vub->vhost_dev, vdev, true);
-    if (ret < 0) {
-        error_report("Error starting vhost-user-device: %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 < vub->vhost_dev.nvqs; i++) {
-        vhost_virtqueue_mask(&vub->vhost_dev, vdev, i, false);
-    }
-
-    return;
-
-err_guest_notifiers:
-    k->set_guest_notifiers(qbus->parent, vub->vhost_dev.nvqs, false);
-err_host_notifiers:
-    vhost_dev_disable_notifiers(&vub->vhost_dev, vdev);
-}
-
-static void vub_stop(VirtIODevice *vdev)
-{
-    VHostUserBase *vub = VHOST_USER_BASE(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(&vub->vhost_dev, vdev, true);
-
-    ret = k->set_guest_notifiers(qbus->parent, vub->vhost_dev.nvqs, false);
-    if (ret < 0) {
-        error_report("vhost guest notifier cleanup failed: %d", ret);
-        return;
-    }
-
-    vhost_dev_disable_notifiers(&vub->vhost_dev, vdev);
-}
-
-static void vub_set_status(VirtIODevice *vdev, uint8_t status)
-{
-    VHostUserBase *vub = VHOST_USER_BASE(vdev);
-    bool should_start = virtio_device_should_start(vdev, status);
-
-    if (vhost_dev_is_started(&vub->vhost_dev) == should_start) {
-        return;
-    }
-
-    if (should_start) {
-        vub_start(vdev);
-    } else {
-        vub_stop(vdev);
-    }
-}
-
-/*
- * For an implementation where everything is delegated to the backend
- * we don't do anything other than return the full feature set offered
- * by the daemon (module the reserved feature bit).
- */
-static uint64_t vub_get_features(VirtIODevice *vdev,
-                                 uint64_t requested_features, Error **errp)
-{
-    VHostUserBase *vub = VHOST_USER_BASE(vdev);
-    /* This should be set when the vhost connection initialises */
-    g_assert(vub->vhost_dev.features);
-    return vub->vhost_dev.features & ~(1ULL << VHOST_USER_F_PROTOCOL_FEATURES);
-}
-
-/*
- * To handle VirtIO config we need to know the size of the config
- * space. We don't cache the config but re-fetch it from the guest
- * every time in case something has changed.
- */
-static void vub_get_config(VirtIODevice *vdev, uint8_t *config)
-{
-    VHostUserBase *vub = VHOST_USER_BASE(vdev);
-    Error *local_err = NULL;
-
-    /*
-     * There will have been a warning during vhost_dev_init, but lets
-     * assert here as nothing will go right now.
-     */
-    g_assert(vub->config_size && vub->vhost_user.supports_config == true);
-
-    if (vhost_dev_get_config(&vub->vhost_dev, config,
-                             vub->config_size, &local_err)) {
-        error_report_err(local_err);
-    }
-}
-
-/*
- * When the daemon signals an update to the config we just need to
- * signal the guest as we re-read the config on demand above.
- */
-static int vub_config_notifier(struct vhost_dev *dev)
-{
-    virtio_notify_config(dev->vdev);
-    return 0;
-}
-
-const VhostDevConfigOps vub_config_ops = {
-    .vhost_dev_config_notifier = vub_config_notifier,
-};
-
-static void vub_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 do_vhost_user_cleanup(VirtIODevice *vdev, VHostUserBase *vub)
-{
-    vhost_user_cleanup(&vub->vhost_user);
-
-    for (int i = 0; i < vub->num_vqs; i++) {
-        VirtQueue *vq = g_ptr_array_index(vub->vqs, i);
-        virtio_delete_queue(vq);
-    }
-
-    virtio_cleanup(vdev);
-}
-
-static int vub_connect(DeviceState *dev)
-{
-    VirtIODevice *vdev = VIRTIO_DEVICE(dev);
-    VHostUserBase *vub = VHOST_USER_BASE(vdev);
-    struct vhost_dev *vhost_dev = &vub->vhost_dev;
-
-    if (vub->connected) {
-        return 0;
-    }
-    vub->connected = true;
-
-    /*
-     * If we support VHOST_USER_GET_CONFIG we must enable the notifier
-     * so we can ping the guest when it updates.
-     */
-    if (vub->vhost_user.supports_config) {
-        vhost_dev_set_config_notifier(vhost_dev, &vub_config_ops);
-    }
-
-    /* restore vhost state */
-    if (virtio_device_started(vdev, vdev->status)) {
-        vub_start(vdev);
-    }
-
-    return 0;
-}
-
-static void vub_disconnect(DeviceState *dev)
-{
-    VirtIODevice *vdev = VIRTIO_DEVICE(dev);
-    VHostUserBase *vub = VHOST_USER_BASE(vdev);
-
-    if (!vub->connected) {
-        return;
-    }
-    vub->connected = false;
-
-    if (vhost_dev_is_started(&vub->vhost_dev)) {
-        vub_stop(vdev);
-    }
-}
-
-static void vub_event(void *opaque, QEMUChrEvent event)
-{
-    DeviceState *dev = opaque;
-    VirtIODevice *vdev = VIRTIO_DEVICE(dev);
-    VHostUserBase *vub = VHOST_USER_BASE(vdev);
-
-    switch (event) {
-    case CHR_EVENT_OPENED:
-        if (vub_connect(dev) < 0) {
-            qemu_chr_fe_disconnect(&vub->chardev);
-            return;
-        }
-        break;
-    case CHR_EVENT_CLOSED:
-        vub_disconnect(dev);
-        break;
-    case CHR_EVENT_BREAK:
-    case CHR_EVENT_MUX_IN:
-    case CHR_EVENT_MUX_OUT:
-        /* Ignore */
-        break;
-    }
-}
-
-static void vub_device_realize(DeviceState *dev, Error **errp)
-{
-    VirtIODevice *vdev = VIRTIO_DEVICE(dev);
-    VHostUserBase *vub = VHOST_USER_BASE(dev);
-    int ret;
-
-    if (!vub->chardev.chr) {
-        error_setg(errp, "vhost-user-device: missing chardev");
-        return;
-    }
-
-    if (!vub->virtio_id) {
-        error_setg(errp, "vhost-user-device: need to define device id");
-        return;
-    }
-
-    if (!vub->num_vqs) {
-        vub->num_vqs = 1; /* reasonable default? */
-    }
-
-    /*
-     * We can't handle config requests unless we know the size of the
-     * config region, specialisations of the vhost-user-device will be
-     * able to set this.
-     */
-    if (vub->config_size) {
-        vub->vhost_user.supports_config = true;
-    }
-
-    if (!vhost_user_init(&vub->vhost_user, &vub->chardev, errp)) {
-        return;
-    }
-
-    virtio_init(vdev, vub->virtio_id, vub->config_size);
-
-    /*
-     * Disable guest notifiers, by default all notifications will be via the
-     * asynchronous vhost-user socket.
-     */
-    vdev->use_guest_notifier_mask = false;
-
-    /* Allocate queues */
-    vub->vqs = g_ptr_array_sized_new(vub->num_vqs);
-    for (int i = 0; i < vub->num_vqs; i++) {
-        g_ptr_array_add(vub->vqs,
-                        virtio_add_queue(vdev, 4, vub_handle_output));
-    }
-
-    vub->vhost_dev.nvqs = vub->num_vqs;
-    vub->vhost_dev.vqs = g_new0(struct vhost_virtqueue, vub->vhost_dev.nvqs);
-
-    /* connect to backend */
-    ret = vhost_dev_init(&vub->vhost_dev, &vub->vhost_user,
-                         VHOST_BACKEND_TYPE_USER, 0, errp);
-
-    if (ret < 0) {
-        do_vhost_user_cleanup(vdev, vub);
-    }
-
-    qemu_chr_fe_set_handlers(&vub->chardev, NULL, NULL, vub_event, NULL,
-                             dev, NULL, true);
-}
-
-static void vub_device_unrealize(DeviceState *dev)
-{
-    VirtIODevice *vdev = VIRTIO_DEVICE(dev);
-    VHostUserBase *vub = VHOST_USER_BASE(dev);
-    struct vhost_virtqueue *vhost_vqs = vub->vhost_dev.vqs;
-
-    /* This will stop vhost backend if appropriate. */
-    vub_set_status(vdev, 0);
-    vhost_dev_cleanup(&vub->vhost_dev);
-    g_free(vhost_vqs);
-    do_vhost_user_cleanup(vdev, vub);
-}
-
-static void vub_class_init(ObjectClass *klass, void *data)
-{
-    VirtioDeviceClass *vdc = VIRTIO_DEVICE_CLASS(klass);
-
-    vdc->realize = vub_device_realize;
-    vdc->unrealize = vub_device_unrealize;
-    vdc->get_features = vub_get_features;
-    vdc->get_config = vub_get_config;
-    vdc->set_status = vub_set_status;
-}
-
-static const TypeInfo vub_info = {
-    .name = TYPE_VHOST_USER_BASE,
-    .parent = TYPE_VIRTIO_DEVICE,
-    .instance_size = sizeof(VHostUserBase),
-    .class_init = vub_class_init,
-    .class_size = sizeof(VHostUserBaseClass),
-    .abstract = true
-};
-
-
 /*
  * The following is a concrete implementation of the base class which
  * allows the user to define the key parameters via the command line.
@@ -349,6 +32,7 @@ static const VMStateDescription vud_vmstate = {
 static Property vud_properties[] = {
     DEFINE_PROP_CHR("chardev", VHostUserBase, chardev),
     DEFINE_PROP_UINT16("virtio-id", VHostUserBase, virtio_id, 0),
+    DEFINE_PROP_UINT32("vq_size", VHostUserBase, vq_size, 64),
     DEFINE_PROP_UINT32("num_vqs", VHostUserBase, num_vqs, 1),
     DEFINE_PROP_UINT32("config_size", VHostUserBase, config_size, 0),
     DEFINE_PROP_END_OF_LIST(),
@@ -366,14 +50,11 @@ static void vud_class_init(ObjectClass *klass, void *data)
 static const TypeInfo vud_info = {
     .name = TYPE_VHOST_USER_DEVICE,
     .parent = TYPE_VHOST_USER_BASE,
-    .instance_size = sizeof(VHostUserBase),
     .class_init = vud_class_init,
-    .class_size = sizeof(VHostUserBaseClass),
 };
 
 static void vu_register_types(void)
 {
-    type_register_static(&vub_info);
     type_register_static(&vud_info);
 }
 
diff --git a/hw/virtio/meson.build b/hw/virtio/meson.build
index c0055a7832..51c3f97c2d 100644
--- a/hw/virtio/meson.build
+++ b/hw/virtio/meson.build
@@ -17,6 +17,7 @@ if have_vhost
   if have_vhost_user
     # fixme - this really should be generic
     specific_virtio_ss.add(files('vhost-user.c'))
+    system_virtio_ss.add(files('vhost-user-base.c'))
     system_virtio_ss.add(files('vhost-user-device.c'))
     system_virtio_ss.add(when: 'CONFIG_VIRTIO_PCI', if_true: files('vhost-user-device-pci.c'))
   endif
-- 
2.39.2


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

* [Virtio-fs] [PATCH v4 2/6] hw/virtio: derive vhost-user-rng from vhost-user-base
  2023-10-09  9:59 [Virtio-fs] [PATCH v4 0/6] virtio: cleanup vhost-user-generic and reduce c&p Alex Bennée
  2023-10-09  9:59 ` [Virtio-fs] [PATCH v4 1/6] virtio: split into vhost-user-base and vhost-user-device Alex Bennée
@ 2023-10-09  9:59 ` Alex Bennée
  2023-10-09 13:07   ` Manos Pitsidianakis
  2023-10-09  9:59 ` [Virtio-fs] [PATCH v4 3/6] hw/virtio: derive vhost-user-gpio " Alex Bennée
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 14+ messages in thread
From: Alex Bennée @ 2023-10-09  9:59 UTC (permalink / raw)
  To: qemu-devel
  Cc: Marc-André Lureau, Erik Schilling, Fam Zheng,
	Mathieu Poirier, Gerd Hoffmann, Hanna Reitz, Stefan Hajnoczi,
	Alex Bennée, Kevin Wolf, Eric Blake, Michael S. Tsirkin,
	qemu-block, Eduardo Habkost, Paolo Bonzini, Mark Cave-Ayland,
	Viresh Kumar, virtio-fs, Gonglei (Arei),
	Markus Armbruster, Jason Wang, Raphael Norwitz,
	Daniel P. Berrangé

Now we can take advantage of our new base class and make
vhost-user-rng a much simpler boilerplate wrapper. Also as this
doesn't require any target specific hacks we only need to build the
stubs once.

Message-Id: <20230418162140.373219-10-alex.bennee@linaro.org>
Acked-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>

---
v2
  - new derivation layout
  - move directly to softmmu_virtio_ss
v3
  - use vqsize
---
 include/hw/virtio/vhost-user-rng.h |  11 +-
 hw/virtio/vhost-user-rng.c         | 278 +++--------------------------
 hw/virtio/meson.build              |  11 +-
 3 files changed, 31 insertions(+), 269 deletions(-)

diff --git a/include/hw/virtio/vhost-user-rng.h b/include/hw/virtio/vhost-user-rng.h
index ddd9f01eea..6cffe28807 100644
--- a/include/hw/virtio/vhost-user-rng.h
+++ b/include/hw/virtio/vhost-user-rng.h
@@ -12,21 +12,14 @@
 #include "hw/virtio/virtio.h"
 #include "hw/virtio/vhost.h"
 #include "hw/virtio/vhost-user.h"
-#include "chardev/char-fe.h"
+#include "hw/virtio/vhost-user-base.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;
-
+    VHostUserBase parent;
     /*< public >*/
 };
 
diff --git a/hw/virtio/vhost-user-rng.c b/hw/virtio/vhost-user-rng.c
index efc54cd3fb..01879c863d 100644
--- a/hw/virtio/vhost-user-rng.c
+++ b/hw/virtio/vhost-user-rng.c
@@ -3,7 +3,7 @@
  *
  * Copyright (c) 2021 Mathieu Poirier <mathieu.poirier@linaro.org>
  *
- * Implementation seriously tailored on vhost-user-i2c.c
+ * Simple wrapper of the generic vhost-user-device.
  *
  * SPDX-License-Identifier: GPL-2.0-or-later
  */
@@ -13,281 +13,47 @@
 #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 const int feature_bits[] = {
-    VIRTIO_F_RING_RESET,
-    VHOST_INVALID_FEATURE_BIT
-};
-
-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, true);
-    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, true);
-
-    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 = virtio_device_should_start(vdev, status);
-
-    if (vhost_dev_is_started(&rng->vhost_dev) == 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)
-{
-    VHostUserRNG *rng = VHOST_USER_RNG(vdev);
-
-    return vhost_get_features(&rng->vhost_dev, feature_bits,
-                              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 (vhost_dev_is_started(&rng->vhost_dev)) {
-        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, 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);
-    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:
-    g_free(rng->vhost_dev.vqs);
-    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);
-    struct vhost_virtqueue *vhost_vqs = rng->vhost_dev.vqs;
-
-    vu_rng_set_status(vdev, 0);
-
-    vhost_dev_cleanup(&rng->vhost_dev);
-    g_free(vhost_vqs);
-    virtio_delete_queue(rng->req_vq);
-    virtio_cleanup(vdev);
-    vhost_user_cleanup(&rng->vhost_user);
-}
-
-static struct vhost_dev *vu_rng_get_vhost(VirtIODevice *vdev)
-{
-    VHostUserRNG *rng = VHOST_USER_RNG(vdev);
-    return &rng->vhost_dev;
-}
-
 static const VMStateDescription vu_rng_vmstate = {
     .name = "vhost-user-rng",
     .unmigratable = 1,
 };
 
-static Property vu_rng_properties[] = {
-    DEFINE_PROP_CHR("chardev", VHostUserRNG, chardev),
+static Property vrng_properties[] = {
+    DEFINE_PROP_CHR("chardev", VHostUserBase, chardev),
     DEFINE_PROP_END_OF_LIST(),
 };
 
+static void vu_rng_base_realize(DeviceState *dev, Error **errp)
+{
+    VHostUserBase *vub = VHOST_USER_BASE(dev);
+    VHostUserBaseClass *vubs = VHOST_USER_BASE_GET_CLASS(dev);
+
+    /* Fixed for RNG */
+    vub->virtio_id = VIRTIO_ID_RNG;
+    vub->num_vqs = 1;
+    vub->vq_size = 4;
+
+    vubs->parent_realize(dev, errp);
+}
+
 static void vu_rng_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
-    VirtioDeviceClass *vdc = VIRTIO_DEVICE_CLASS(klass);
+    VHostUserBaseClass *vubc = VHOST_USER_BASE_CLASS(klass);
 
-    device_class_set_props(dc, vu_rng_properties);
     dc->vmsd = &vu_rng_vmstate;
-    set_bit(DEVICE_CATEGORY_INPUT, dc->categories);
+    device_class_set_props(dc, vrng_properties);
+    device_class_set_parent_realize(dc, vu_rng_base_realize,
+                                    &vubc->parent_realize);
 
-    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;
-    vdc->get_vhost = vu_rng_get_vhost;
+    set_bit(DEVICE_CATEGORY_INPUT, dc->categories);
 }
 
 static const TypeInfo vu_rng_info = {
     .name = TYPE_VHOST_USER_RNG,
-    .parent = TYPE_VIRTIO_DEVICE,
+    .parent = TYPE_VHOST_USER_BASE,
     .instance_size = sizeof(VHostUserRNG),
     .class_init = vu_rng_class_init,
 };
diff --git a/hw/virtio/meson.build b/hw/virtio/meson.build
index 51c3f97c2d..d0b963199c 100644
--- a/hw/virtio/meson.build
+++ b/hw/virtio/meson.build
@@ -18,8 +18,15 @@ if have_vhost
     # fixme - this really should be generic
     specific_virtio_ss.add(files('vhost-user.c'))
     system_virtio_ss.add(files('vhost-user-base.c'))
+
+    # MMIO Stubs
     system_virtio_ss.add(files('vhost-user-device.c'))
+    system_virtio_ss.add(when: 'CONFIG_VHOST_USER_RNG', if_true: files('vhost-user-rng.c'))
+
+    # PCI Stubs
     system_virtio_ss.add(when: 'CONFIG_VIRTIO_PCI', if_true: files('vhost-user-device-pci.c'))
+    system_virtio_ss.add(when: ['CONFIG_VIRTIO_PCI', 'CONFIG_VHOST_USER_RNG'],
+                         if_true: files('vhost-user-rng-pci.c'))
   endif
   if have_vhost_vdpa
     system_virtio_ss.add(files('vhost-vdpa.c'))
@@ -34,10 +41,8 @@ specific_virtio_ss.add(when: 'CONFIG_VHOST_USER_FS', if_true: files('vhost-user-
 specific_virtio_ss.add(when: 'CONFIG_VIRTIO_PMEM', if_true: files('virtio-pmem.c'))
 specific_virtio_ss.add(when: 'CONFIG_VHOST_VSOCK', if_true: files('vhost-vsock.c'))
 specific_virtio_ss.add(when: 'CONFIG_VHOST_USER_VSOCK', if_true: files('vhost-user-vsock.c'))
-specific_virtio_ss.add(when: 'CONFIG_VIRTIO_RNG', if_true: files('virtio-rng.c'))
 specific_virtio_ss.add(when: 'CONFIG_VIRTIO_MEM', if_true: files('virtio-mem.c'))
 specific_virtio_ss.add(when: 'CONFIG_VHOST_USER_I2C', if_true: files('vhost-user-i2c.c'))
-specific_virtio_ss.add(when: 'CONFIG_VHOST_USER_RNG', if_true: files('vhost-user-rng.c'))
 specific_virtio_ss.add(when: 'CONFIG_VHOST_USER_GPIO', if_true: files('vhost-user-gpio.c'))
 specific_virtio_ss.add(when: ['CONFIG_VIRTIO_PCI', 'CONFIG_VHOST_USER_GPIO'], if_true: files('vhost-user-gpio-pci.c'))
 specific_virtio_ss.add(when: 'CONFIG_VHOST_USER_SCMI', if_true: files('vhost-user-scmi.c'))
@@ -49,7 +54,6 @@ virtio_pci_ss.add(when: 'CONFIG_VHOST_USER_VSOCK', if_true: files('vhost-user-vs
 virtio_pci_ss.add(when: 'CONFIG_VHOST_USER_BLK', if_true: files('vhost-user-blk-pci.c'))
 virtio_pci_ss.add(when: 'CONFIG_VHOST_USER_I2C', if_true: files('vhost-user-i2c-pci.c'))
 virtio_pci_ss.add(when: 'CONFIG_VHOST_USER_INPUT', if_true: files('vhost-user-input-pci.c'))
-virtio_pci_ss.add(when: 'CONFIG_VHOST_USER_RNG', if_true: files('vhost-user-rng-pci.c'))
 virtio_pci_ss.add(when: 'CONFIG_VHOST_USER_SCSI', if_true: files('vhost-user-scsi-pci.c'))
 virtio_pci_ss.add(when: 'CONFIG_VHOST_SCSI', if_true: files('vhost-scsi-pci.c'))
 virtio_pci_ss.add(when: 'CONFIG_VHOST_USER_FS', if_true: files('vhost-user-fs-pci.c'))
@@ -57,7 +61,6 @@ virtio_pci_ss.add(when: 'CONFIG_VHOST_USER_FS', if_true: files('vhost-user-fs-pc
 virtio_pci_ss.add(when: 'CONFIG_VIRTIO_CRYPTO', if_true: files('virtio-crypto-pci.c'))
 virtio_pci_ss.add(when: 'CONFIG_VIRTIO_INPUT_HOST', if_true: files('virtio-input-host-pci.c'))
 virtio_pci_ss.add(when: 'CONFIG_VIRTIO_INPUT', if_true: files('virtio-input-pci.c'))
-virtio_pci_ss.add(when: 'CONFIG_VIRTIO_RNG', if_true: files('virtio-rng-pci.c'))
 virtio_pci_ss.add(when: 'CONFIG_VIRTIO_BALLOON', if_true: files('virtio-balloon-pci.c'))
 virtio_pci_ss.add(when: 'CONFIG_VIRTIO_9P', if_true: files('virtio-9p-pci.c'))
 virtio_pci_ss.add(when: 'CONFIG_VIRTIO_SCSI', if_true: files('virtio-scsi-pci.c'))
-- 
2.39.2


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

* [Virtio-fs] [PATCH v4 3/6] hw/virtio: derive vhost-user-gpio from vhost-user-base
  2023-10-09  9:59 [Virtio-fs] [PATCH v4 0/6] virtio: cleanup vhost-user-generic and reduce c&p Alex Bennée
  2023-10-09  9:59 ` [Virtio-fs] [PATCH v4 1/6] virtio: split into vhost-user-base and vhost-user-device Alex Bennée
  2023-10-09  9:59 ` [Virtio-fs] [PATCH v4 2/6] hw/virtio: derive vhost-user-rng from vhost-user-base Alex Bennée
@ 2023-10-09  9:59 ` Alex Bennée
  2023-10-09 10:59   ` Viresh Kumar
  2023-10-09  9:59 ` [Virtio-fs] [PATCH v4 4/6] hw/virtio: derive vhost-user-i2c " Alex Bennée
                   ` (3 subsequent siblings)
  6 siblings, 1 reply; 14+ messages in thread
From: Alex Bennée @ 2023-10-09  9:59 UTC (permalink / raw)
  To: qemu-devel
  Cc: Marc-André Lureau, Erik Schilling, Fam Zheng,
	Mathieu Poirier, Gerd Hoffmann, Hanna Reitz, Stefan Hajnoczi,
	Alex Bennée, Kevin Wolf, Eric Blake, Michael S. Tsirkin,
	qemu-block, Eduardo Habkost, Paolo Bonzini, Mark Cave-Ayland,
	Viresh Kumar, virtio-fs, Gonglei (Arei),
	Markus Armbruster, Jason Wang, Raphael Norwitz,
	Daniel P. Berrangé

Now the new base class supports config handling we can take advantage
and make vhost-user-gpio a much simpler boilerplate wrapper. Also as
this doesn't require any target specific hacks we only need to build
the stubs once.

Message-Id: <20230418162140.373219-12-alex.bennee@linaro.org>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Acked-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>

---
v2
  - use new vhost-user-base
  - move build to common code
v3
  - fix inadvertent double link
v4
  - merge conflict
  - update includes
---
 include/hw/virtio/vhost-user-gpio.h |  23 +-
 hw/virtio/vhost-user-gpio.c         | 407 ++--------------------------
 hw/virtio/meson.build               |   5 +-
 3 files changed, 22 insertions(+), 413 deletions(-)

diff --git a/include/hw/virtio/vhost-user-gpio.h b/include/hw/virtio/vhost-user-gpio.h
index a9d3f9b049..5201d5f072 100644
--- a/include/hw/virtio/vhost-user-gpio.h
+++ b/include/hw/virtio/vhost-user-gpio.h
@@ -12,33 +12,14 @@
 #include "hw/virtio/virtio.h"
 #include "hw/virtio/vhost.h"
 #include "hw/virtio/vhost-user.h"
-#include "standard-headers/linux/virtio_gpio.h"
-#include "chardev/char-fe.h"
+#include "hw/virtio/vhost-user-base.h"
 
 #define TYPE_VHOST_USER_GPIO "vhost-user-gpio-device"
 OBJECT_DECLARE_SIMPLE_TYPE(VHostUserGPIO, VHOST_USER_GPIO);
 
 struct VHostUserGPIO {
     /*< private >*/
-    VirtIODevice parent_obj;
-    CharBackend chardev;
-    struct virtio_gpio_config config;
-    struct vhost_virtqueue *vhost_vqs;
-    struct vhost_dev vhost_dev;
-    VhostUserState vhost_user;
-    VirtQueue *command_vq;
-    VirtQueue *interrupt_vq;
-    /**
-     * There are at least two steps of initialization of the
-     * vhost-user device. The first is a "connect" step and
-     * second is a "start" step. Make a separation between
-     * those initialization phases by using two fields.
-     *
-     * @connected: see vu_gpio_connect()/vu_gpio_disconnect()
-     * @started_vu: see vu_gpio_start()/vu_gpio_stop()
-     */
-    bool connected;
-    bool started_vu;
+    VHostUserBase parent;
     /*< public >*/
 };
 
diff --git a/hw/virtio/vhost-user-gpio.c b/hw/virtio/vhost-user-gpio.c
index 3d7fae3984..9f37c25415 100644
--- a/hw/virtio/vhost-user-gpio.c
+++ b/hw/virtio/vhost-user-gpio.c
@@ -11,388 +11,25 @@
 #include "hw/qdev-properties.h"
 #include "hw/virtio/virtio-bus.h"
 #include "hw/virtio/vhost-user-gpio.h"
-#include "qemu/error-report.h"
 #include "standard-headers/linux/virtio_ids.h"
-#include "trace.h"
+#include "standard-headers/linux/virtio_gpio.h"
 
-#define REALIZE_CONNECTION_RETRIES 3
-#define VHOST_NVQS 2
-
-/* Features required from VirtIO */
-static const int feature_bits[] = {
-    VIRTIO_F_VERSION_1,
-    VIRTIO_F_NOTIFY_ON_EMPTY,
-    VIRTIO_RING_F_INDIRECT_DESC,
-    VIRTIO_RING_F_EVENT_IDX,
-    VIRTIO_GPIO_F_IRQ,
-    VIRTIO_F_RING_RESET,
-    VHOST_INVALID_FEATURE_BIT
-};
-
-static void vu_gpio_get_config(VirtIODevice *vdev, uint8_t *config)
-{
-    VHostUserGPIO *gpio = VHOST_USER_GPIO(vdev);
-
-    memcpy(config, &gpio->config, sizeof(gpio->config));
-}
-
-static int vu_gpio_config_notifier(struct vhost_dev *dev)
-{
-    VHostUserGPIO *gpio = VHOST_USER_GPIO(dev->vdev);
-
-    memcpy(dev->vdev->config, &gpio->config, sizeof(gpio->config));
-    virtio_notify_config(dev->vdev);
-
-    return 0;
-}
-
-const VhostDevConfigOps gpio_ops = {
-    .vhost_dev_config_notifier = vu_gpio_config_notifier,
+static Property vgpio_properties[] = {
+    DEFINE_PROP_CHR("chardev", VHostUserBase, chardev),
+    DEFINE_PROP_END_OF_LIST(),
 };
 
-static int vu_gpio_start(VirtIODevice *vdev)
-{
-    BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(vdev)));
-    VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
-    VHostUserGPIO *gpio = VHOST_USER_GPIO(vdev);
-    struct vhost_dev *vhost_dev = &gpio->vhost_dev;
-    int ret, i;
-
-    if (!k->set_guest_notifiers) {
-        error_report("binding does not support guest notifiers");
-        return -ENOSYS;
-    }
-
-    ret = vhost_dev_enable_notifiers(vhost_dev, vdev);
-    if (ret < 0) {
-        error_report("Error enabling host notifiers: %d", ret);
-        return ret;
-    }
-
-    ret = k->set_guest_notifiers(qbus->parent, vhost_dev->nvqs, true);
-    if (ret < 0) {
-        error_report("Error binding guest notifier: %d", ret);
-        goto err_host_notifiers;
-    }
-
-    /*
-     * Before we start up we need to ensure we have the final feature
-     * set needed for the vhost configuration. The backend may also
-     * apply backend_features when the feature set is sent.
-     */
-    vhost_ack_features(&gpio->vhost_dev, feature_bits, vdev->guest_features);
-
-    ret = vhost_dev_start(&gpio->vhost_dev, vdev, false);
-    if (ret < 0) {
-        error_report("Error starting vhost-user-gpio: %d", ret);
-        goto err_guest_notifiers;
-    }
-    gpio->started_vu = true;
-
-    /*
-     * 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 < gpio->vhost_dev.nvqs; i++) {
-        vhost_virtqueue_mask(&gpio->vhost_dev, vdev, i, false);
-    }
-
-    /*
-     * As we must have VHOST_USER_F_PROTOCOL_FEATURES (because
-     * VHOST_USER_GET_CONFIG requires it) we need to explicitly enable
-     * the vrings.
-     */
-    g_assert(vhost_dev->vhost_ops &&
-             vhost_dev->vhost_ops->vhost_set_vring_enable);
-    ret = vhost_dev->vhost_ops->vhost_set_vring_enable(vhost_dev, true);
-    if (ret == 0) {
-        return 0;
-    }
-
-    error_report("Failed to start vrings for vhost-user-gpio: %d", ret);
-
-err_guest_notifiers:
-    k->set_guest_notifiers(qbus->parent, gpio->vhost_dev.nvqs, false);
-err_host_notifiers:
-    vhost_dev_disable_notifiers(&gpio->vhost_dev, vdev);
-
-    return ret;
-}
-
-static void vu_gpio_stop(VirtIODevice *vdev)
-{
-    VHostUserGPIO *gpio = VHOST_USER_GPIO(vdev);
-    BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(vdev)));
-    VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
-    struct vhost_dev *vhost_dev = &gpio->vhost_dev;
-    int ret;
-
-    if (!gpio->started_vu) {
-        return;
-    }
-    gpio->started_vu = false;
-
-    if (!k->set_guest_notifiers) {
-        return;
-    }
-
-    vhost_dev_stop(vhost_dev, vdev, false);
-
-    ret = k->set_guest_notifiers(qbus->parent, vhost_dev->nvqs, false);
-    if (ret < 0) {
-        error_report("vhost guest notifier cleanup failed: %d", ret);
-        return;
-    }
-
-    vhost_dev_disable_notifiers(vhost_dev, vdev);
-}
-
-static void vu_gpio_set_status(VirtIODevice *vdev, uint8_t status)
-{
-    VHostUserGPIO *gpio = VHOST_USER_GPIO(vdev);
-    bool should_start = virtio_device_should_start(vdev, status);
-
-    trace_virtio_gpio_set_status(status);
-
-    if (!gpio->connected) {
-        return;
-    }
-
-    if (vhost_dev_is_started(&gpio->vhost_dev) == should_start) {
-        return;
-    }
-
-    if (should_start) {
-        if (vu_gpio_start(vdev)) {
-            qemu_chr_fe_disconnect(&gpio->chardev);
-        }
-    } else {
-        vu_gpio_stop(vdev);
-    }
-}
-
-static uint64_t vu_gpio_get_features(VirtIODevice *vdev, uint64_t features,
-                                     Error **errp)
-{
-    VHostUserGPIO *gpio = VHOST_USER_GPIO(vdev);
-
-    return vhost_get_features(&gpio->vhost_dev, feature_bits, features);
-}
-
-static void vu_gpio_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_gpio_guest_notifier_mask(VirtIODevice *vdev, int idx, bool mask)
-{
-    VHostUserGPIO *gpio = VHOST_USER_GPIO(vdev);
-
-    /*
-     * Add the check for configure interrupt, Use VIRTIO_CONFIG_IRQ_IDX -1
-     * as the macro of configure interrupt's IDX, If this driver does not
-     * support, the function will return
-     */
-
-    if (idx == VIRTIO_CONFIG_IRQ_IDX) {
-        return;
-    }
-
-    vhost_virtqueue_mask(&gpio->vhost_dev, vdev, idx, mask);
-}
-
-static struct vhost_dev *vu_gpio_get_vhost(VirtIODevice *vdev)
-{
-    VHostUserGPIO *gpio = VHOST_USER_GPIO(vdev);
-    return &gpio->vhost_dev;
-}
-
-static void do_vhost_user_cleanup(VirtIODevice *vdev, VHostUserGPIO *gpio)
-{
-    virtio_delete_queue(gpio->command_vq);
-    virtio_delete_queue(gpio->interrupt_vq);
-    g_free(gpio->vhost_vqs);
-    virtio_cleanup(vdev);
-    vhost_user_cleanup(&gpio->vhost_user);
-}
-
-static int vu_gpio_connect(DeviceState *dev, Error **errp)
-{
-    VirtIODevice *vdev = VIRTIO_DEVICE(dev);
-    VHostUserGPIO *gpio = VHOST_USER_GPIO(vdev);
-    struct vhost_dev *vhost_dev = &gpio->vhost_dev;
-    int ret;
-
-    if (gpio->connected) {
-        return 0;
-    }
-    gpio->connected = true;
-
-    vhost_dev_set_config_notifier(vhost_dev, &gpio_ops);
-    gpio->vhost_user.supports_config = true;
-
-    gpio->vhost_dev.nvqs = VHOST_NVQS;
-    gpio->vhost_dev.vqs = gpio->vhost_vqs;
-
-    ret = vhost_dev_init(vhost_dev, &gpio->vhost_user,
-                         VHOST_BACKEND_TYPE_USER, 0, errp);
-    if (ret < 0) {
-        return ret;
-    }
-
-    /* restore vhost state */
-    if (virtio_device_started(vdev, vdev->status)) {
-        vu_gpio_start(vdev);
-    }
-
-    return 0;
-}
-
-static void vu_gpio_event(void *opaque, QEMUChrEvent event);
-
-static void vu_gpio_disconnect(DeviceState *dev)
-{
-    VirtIODevice *vdev = VIRTIO_DEVICE(dev);
-    VHostUserGPIO *gpio = VHOST_USER_GPIO(vdev);
-
-    if (!gpio->connected) {
-        return;
-    }
-    gpio->connected = false;
-
-    vu_gpio_stop(vdev);
-    vhost_dev_cleanup(&gpio->vhost_dev);
-
-    /* Re-instate the event handler for new connections */
-    qemu_chr_fe_set_handlers(&gpio->chardev,
-                             NULL, NULL, vu_gpio_event,
-                             NULL, dev, NULL, true);
-}
-
-static void vu_gpio_event(void *opaque, QEMUChrEvent event)
-{
-    DeviceState *dev = opaque;
-    VirtIODevice *vdev = VIRTIO_DEVICE(dev);
-    VHostUserGPIO *gpio = VHOST_USER_GPIO(vdev);
-    Error *local_err = NULL;
-
-    switch (event) {
-    case CHR_EVENT_OPENED:
-        if (vu_gpio_connect(dev, &local_err) < 0) {
-            qemu_chr_fe_disconnect(&gpio->chardev);
-            return;
-        }
-        break;
-    case CHR_EVENT_CLOSED:
-        /* defer close until later to avoid circular close */
-        vhost_user_async_close(dev, &gpio->chardev, &gpio->vhost_dev,
-                               vu_gpio_disconnect);
-        break;
-    case CHR_EVENT_BREAK:
-    case CHR_EVENT_MUX_IN:
-    case CHR_EVENT_MUX_OUT:
-        /* Ignore */
-        break;
-    }
-}
-
-static int vu_gpio_realize_connect(VHostUserGPIO *gpio, Error **errp)
+static void vgpio_realize(DeviceState *dev, Error **errp)
 {
-    VirtIODevice *vdev = &gpio->parent_obj;
-    DeviceState *dev = &vdev->parent_obj;
-    struct vhost_dev *vhost_dev = &gpio->vhost_dev;
-    int ret;
-
-    ret = qemu_chr_fe_wait_connected(&gpio->chardev, errp);
-    if (ret < 0) {
-        return ret;
-    }
-
-    /*
-     * vu_gpio_connect() may have already connected (via the event
-     * callback) in which case it will just report success.
-     */
-    ret = vu_gpio_connect(dev, errp);
-    if (ret < 0) {
-        qemu_chr_fe_disconnect(&gpio->chardev);
-        return ret;
-    }
-    g_assert(gpio->connected);
-
-    ret = vhost_dev_get_config(vhost_dev, (uint8_t *)&gpio->config,
-                               sizeof(gpio->config), errp);
-
-    if (ret < 0) {
-        error_report("vhost-user-gpio: get config failed");
-
-        qemu_chr_fe_disconnect(&gpio->chardev);
-        vhost_dev_cleanup(vhost_dev);
-        return ret;
-    }
-
-    return 0;
-}
-
-static void vu_gpio_device_realize(DeviceState *dev, Error **errp)
-{
-    ERRP_GUARD();
-
-    VirtIODevice *vdev = VIRTIO_DEVICE(dev);
-    VHostUserGPIO *gpio = VHOST_USER_GPIO(dev);
-    int retries, ret;
-
-    if (!gpio->chardev.chr) {
-        error_setg(errp, "vhost-user-gpio: chardev is mandatory");
-        return;
-    }
+    VHostUserBase *vub = VHOST_USER_BASE(dev);
+    VHostUserBaseClass *vubc = VHOST_USER_BASE_GET_CLASS(dev);
 
-    if (!vhost_user_init(&gpio->vhost_user, &gpio->chardev, errp)) {
-        return;
-    }
+    /* Fixed for GPIO */
+    vub->virtio_id = VIRTIO_ID_GPIO;
+    vub->num_vqs = 2;
+    vub->config_size = sizeof(struct virtio_gpio_config);
 
-    virtio_init(vdev, VIRTIO_ID_GPIO, sizeof(gpio->config));
-
-    gpio->command_vq = virtio_add_queue(vdev, 256, vu_gpio_handle_output);
-    gpio->interrupt_vq = virtio_add_queue(vdev, 256, vu_gpio_handle_output);
-    gpio->vhost_vqs = g_new0(struct vhost_virtqueue, VHOST_NVQS);
-
-    gpio->connected = false;
-
-    qemu_chr_fe_set_handlers(&gpio->chardev, NULL, NULL, vu_gpio_event, NULL,
-                             dev, NULL, true);
-
-    retries = REALIZE_CONNECTION_RETRIES;
-    g_assert(!*errp);
-    do {
-        if (*errp) {
-            error_prepend(errp, "Reconnecting after error: ");
-            error_report_err(*errp);
-            *errp = NULL;
-        }
-        ret = vu_gpio_realize_connect(gpio, errp);
-    } while (ret < 0 && retries--);
-
-    if (ret < 0) {
-        do_vhost_user_cleanup(vdev, gpio);
-    }
-
-    return;
-}
-
-static void vu_gpio_device_unrealize(DeviceState *dev)
-{
-    VirtIODevice *vdev = VIRTIO_DEVICE(dev);
-    VHostUserGPIO *gpio = VHOST_USER_GPIO(dev);
-
-    vu_gpio_set_status(vdev, 0);
-    qemu_chr_fe_set_handlers(&gpio->chardev, NULL, NULL, NULL, NULL, NULL, NULL,
-                             false);
-    vhost_dev_cleanup(&gpio->vhost_dev);
-    do_vhost_user_cleanup(vdev, gpio);
+    vubc->parent_realize(dev, errp);
 }
 
 static const VMStateDescription vu_gpio_vmstate = {
@@ -400,31 +37,21 @@ static const VMStateDescription vu_gpio_vmstate = {
     .unmigratable = 1,
 };
 
-static Property vu_gpio_properties[] = {
-    DEFINE_PROP_CHR("chardev", VHostUserGPIO, chardev),
-    DEFINE_PROP_END_OF_LIST(),
-};
-
 static void vu_gpio_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
-    VirtioDeviceClass *vdc = VIRTIO_DEVICE_CLASS(klass);
+    VHostUserBaseClass *vubc = VHOST_USER_BASE_CLASS(klass);
 
-    device_class_set_props(dc, vu_gpio_properties);
     dc->vmsd = &vu_gpio_vmstate;
+    device_class_set_props(dc, vgpio_properties);
+    device_class_set_parent_realize(dc, vgpio_realize,
+                                    &vubc->parent_realize);
     set_bit(DEVICE_CATEGORY_INPUT, dc->categories);
-    vdc->realize = vu_gpio_device_realize;
-    vdc->unrealize = vu_gpio_device_unrealize;
-    vdc->get_features = vu_gpio_get_features;
-    vdc->get_config = vu_gpio_get_config;
-    vdc->set_status = vu_gpio_set_status;
-    vdc->guest_notifier_mask = vu_gpio_guest_notifier_mask;
-    vdc->get_vhost = vu_gpio_get_vhost;
 }
 
 static const TypeInfo vu_gpio_info = {
     .name = TYPE_VHOST_USER_GPIO,
-    .parent = TYPE_VIRTIO_DEVICE,
+    .parent = TYPE_VHOST_USER_BASE,
     .instance_size = sizeof(VHostUserGPIO),
     .class_init = vu_gpio_class_init,
 };
diff --git a/hw/virtio/meson.build b/hw/virtio/meson.build
index d0b963199c..752d4a6905 100644
--- a/hw/virtio/meson.build
+++ b/hw/virtio/meson.build
@@ -21,10 +21,13 @@ if have_vhost
 
     # MMIO Stubs
     system_virtio_ss.add(files('vhost-user-device.c'))
+    system_virtio_ss.add(when: 'CONFIG_VHOST_USER_GPIO', if_true: files('vhost-user-gpio.c'))
     system_virtio_ss.add(when: 'CONFIG_VHOST_USER_RNG', if_true: files('vhost-user-rng.c'))
 
     # PCI Stubs
     system_virtio_ss.add(when: 'CONFIG_VIRTIO_PCI', if_true: files('vhost-user-device-pci.c'))
+    system_virtio_ss.add(when: ['CONFIG_VIRTIO_PCI', 'CONFIG_VHOST_USER_GPIO'],
+                         if_true: files('vhost-user-gpio-pci.c'))
     system_virtio_ss.add(when: ['CONFIG_VIRTIO_PCI', 'CONFIG_VHOST_USER_RNG'],
                          if_true: files('vhost-user-rng-pci.c'))
   endif
@@ -43,8 +46,6 @@ specific_virtio_ss.add(when: 'CONFIG_VHOST_VSOCK', if_true: files('vhost-vsock.c
 specific_virtio_ss.add(when: 'CONFIG_VHOST_USER_VSOCK', if_true: files('vhost-user-vsock.c'))
 specific_virtio_ss.add(when: 'CONFIG_VIRTIO_MEM', if_true: files('virtio-mem.c'))
 specific_virtio_ss.add(when: 'CONFIG_VHOST_USER_I2C', if_true: files('vhost-user-i2c.c'))
-specific_virtio_ss.add(when: 'CONFIG_VHOST_USER_GPIO', if_true: files('vhost-user-gpio.c'))
-specific_virtio_ss.add(when: ['CONFIG_VIRTIO_PCI', 'CONFIG_VHOST_USER_GPIO'], if_true: files('vhost-user-gpio-pci.c'))
 specific_virtio_ss.add(when: 'CONFIG_VHOST_USER_SCMI', if_true: files('vhost-user-scmi.c'))
 specific_virtio_ss.add(when: ['CONFIG_VIRTIO_PCI', 'CONFIG_VHOST_USER_SCMI'], if_true: files('vhost-user-scmi-pci.c'))
 
-- 
2.39.2


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

* [Virtio-fs] [PATCH v4 4/6] hw/virtio: derive vhost-user-i2c from vhost-user-base
  2023-10-09  9:59 [Virtio-fs] [PATCH v4 0/6] virtio: cleanup vhost-user-generic and reduce c&p Alex Bennée
                   ` (2 preceding siblings ...)
  2023-10-09  9:59 ` [Virtio-fs] [PATCH v4 3/6] hw/virtio: derive vhost-user-gpio " Alex Bennée
@ 2023-10-09  9:59 ` Alex Bennée
  2023-10-09 10:59   ` Viresh Kumar
  2023-10-09  9:59 ` [Virtio-fs] [PATCH v4 5/6] hw/virtio: add vhost-user-snd and virtio-snd-pci devices Alex Bennée
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 14+ messages in thread
From: Alex Bennée @ 2023-10-09  9:59 UTC (permalink / raw)
  To: qemu-devel
  Cc: Marc-André Lureau, Erik Schilling, Fam Zheng,
	Mathieu Poirier, Gerd Hoffmann, Hanna Reitz, Stefan Hajnoczi,
	Alex Bennée, Kevin Wolf, Eric Blake, Michael S. Tsirkin,
	qemu-block, Eduardo Habkost, Paolo Bonzini, Mark Cave-Ayland,
	Viresh Kumar, virtio-fs, Gonglei (Arei),
	Markus Armbruster, Jason Wang, Raphael Norwitz,
	Daniel P. Berrangé

Now we can take advantage of the new base class and make
vhost-user-i2c a much simpler boilerplate wrapper. Also as this
doesn't require any target specific hacks we only need to build the
stubs once.

Message-Id: <20230418162140.373219-13-alex.bennee@linaro.org>
Acked-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>

---
v2
  - update to new inheritance scheme
  - move build to common code
v3
  - fix merge conflict in meson
  - style updates, remove duplicate includes
v4
  - use vqsize
---
 include/hw/virtio/vhost-user-i2c.h |  14 +-
 hw/virtio/vhost-user-i2c.c         | 272 ++---------------------------
 hw/virtio/meson.build              |   5 +-
 3 files changed, 23 insertions(+), 268 deletions(-)

diff --git a/include/hw/virtio/vhost-user-i2c.h b/include/hw/virtio/vhost-user-i2c.h
index 0f7acd40e3..a8fcb108db 100644
--- a/include/hw/virtio/vhost-user-i2c.h
+++ b/include/hw/virtio/vhost-user-i2c.h
@@ -9,23 +9,17 @@
 #ifndef QEMU_VHOST_USER_I2C_H
 #define QEMU_VHOST_USER_I2C_H
 
+#include "hw/virtio/virtio.h"
 #include "hw/virtio/vhost.h"
 #include "hw/virtio/vhost-user.h"
+#include "hw/virtio/vhost-user-base.h"
 
 #define TYPE_VHOST_USER_I2C "vhost-user-i2c-device"
+
 OBJECT_DECLARE_SIMPLE_TYPE(VHostUserI2C, VHOST_USER_I2C)
 
 struct VHostUserI2C {
-    VirtIODevice parent;
-    CharBackend chardev;
-    struct vhost_virtqueue *vhost_vq;
-    struct vhost_dev vhost_dev;
-    VhostUserState vhost_user;
-    VirtQueue *vq;
-    bool connected;
+    VHostUserBase parent;
 };
 
-/* Virtio Feature bits */
-#define VIRTIO_I2C_F_ZERO_LENGTH_REQUEST		0
-
 #endif /* QEMU_VHOST_USER_I2C_H */
diff --git a/hw/virtio/vhost-user-i2c.c b/hw/virtio/vhost-user-i2c.c
index 4eef3f0633..a464f5e039 100644
--- a/hw/virtio/vhost-user-i2c.c
+++ b/hw/virtio/vhost-user-i2c.c
@@ -14,253 +14,22 @@
 #include "qemu/error-report.h"
 #include "standard-headers/linux/virtio_ids.h"
 
-static const int feature_bits[] = {
-    VIRTIO_I2C_F_ZERO_LENGTH_REQUEST,
-    VIRTIO_F_RING_RESET,
-    VHOST_INVALID_FEATURE_BIT
+static Property vi2c_properties[] = {
+    DEFINE_PROP_CHR("chardev", VHostUserBase, chardev),
+    DEFINE_PROP_END_OF_LIST(),
 };
 
-static void vu_i2c_start(VirtIODevice *vdev)
-{
-    BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(vdev)));
-    VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
-    VHostUserI2C *i2c = VHOST_USER_I2C(vdev);
-    int ret, i;
-
-    if (!k->set_guest_notifiers) {
-        error_report("binding does not support guest notifiers");
-        return;
-    }
-
-    ret = vhost_dev_enable_notifiers(&i2c->vhost_dev, vdev);
-    if (ret < 0) {
-        error_report("Error enabling host notifiers: %d", -ret);
-        return;
-    }
-
-    ret = k->set_guest_notifiers(qbus->parent, i2c->vhost_dev.nvqs, true);
-    if (ret < 0) {
-        error_report("Error binding guest notifier: %d", -ret);
-        goto err_host_notifiers;
-    }
-
-    i2c->vhost_dev.acked_features = vdev->guest_features;
-
-    ret = vhost_dev_start(&i2c->vhost_dev, vdev, true);
-    if (ret < 0) {
-        error_report("Error starting vhost-user-i2c: %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 < i2c->vhost_dev.nvqs; i++) {
-        vhost_virtqueue_mask(&i2c->vhost_dev, vdev, i, false);
-    }
-
-    return;
-
-err_guest_notifiers:
-    k->set_guest_notifiers(qbus->parent, i2c->vhost_dev.nvqs, false);
-err_host_notifiers:
-    vhost_dev_disable_notifiers(&i2c->vhost_dev, vdev);
-}
-
-static void vu_i2c_stop(VirtIODevice *vdev)
-{
-    VHostUserI2C *i2c = VHOST_USER_I2C(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(&i2c->vhost_dev, vdev, true);
-
-    ret = k->set_guest_notifiers(qbus->parent, i2c->vhost_dev.nvqs, false);
-    if (ret < 0) {
-        error_report("vhost guest notifier cleanup failed: %d", ret);
-        return;
-    }
-
-    vhost_dev_disable_notifiers(&i2c->vhost_dev, vdev);
-}
-
-static void vu_i2c_set_status(VirtIODevice *vdev, uint8_t status)
-{
-    VHostUserI2C *i2c = VHOST_USER_I2C(vdev);
-    bool should_start = virtio_device_should_start(vdev, status);
-
-    if (vhost_dev_is_started(&i2c->vhost_dev) == should_start) {
-        return;
-    }
-
-    if (should_start) {
-        vu_i2c_start(vdev);
-    } else {
-        vu_i2c_stop(vdev);
-    }
-}
-
-static uint64_t vu_i2c_get_features(VirtIODevice *vdev,
-                                    uint64_t requested_features, Error **errp)
-{
-    VHostUserI2C *i2c = VHOST_USER_I2C(vdev);
-
-    virtio_add_feature(&requested_features, VIRTIO_I2C_F_ZERO_LENGTH_REQUEST);
-    return vhost_get_features(&i2c->vhost_dev, feature_bits, requested_features);
-}
-
-static void vu_i2c_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_i2c_guest_notifier_mask(VirtIODevice *vdev, int idx, bool mask)
-{
-    VHostUserI2C *i2c = VHOST_USER_I2C(vdev);
-
-    /*
-     * We don't support interrupts, return early if index is set to
-     * VIRTIO_CONFIG_IRQ_IDX.
-     */
-    if (idx == VIRTIO_CONFIG_IRQ_IDX) {
-        return;
-    }
-
-    vhost_virtqueue_mask(&i2c->vhost_dev, vdev, idx, mask);
-}
-
-static bool vu_i2c_guest_notifier_pending(VirtIODevice *vdev, int idx)
-{
-    VHostUserI2C *i2c = VHOST_USER_I2C(vdev);
-
-    /*
-     * We don't support interrupts, return early if index is set to
-     * VIRTIO_CONFIG_IRQ_IDX.
-     */
-    if (idx == VIRTIO_CONFIG_IRQ_IDX) {
-        return false;
-    }
-
-    return vhost_virtqueue_pending(&i2c->vhost_dev, idx);
-}
-
-static void do_vhost_user_cleanup(VirtIODevice *vdev, VHostUserI2C *i2c)
-{
-    vhost_user_cleanup(&i2c->vhost_user);
-    virtio_delete_queue(i2c->vq);
-    virtio_cleanup(vdev);
-}
-
-static int vu_i2c_connect(DeviceState *dev)
+static void vi2c_realize(DeviceState *dev, Error **errp)
 {
-    VirtIODevice *vdev = VIRTIO_DEVICE(dev);
-    VHostUserI2C *i2c = VHOST_USER_I2C(vdev);
+    VHostUserBase *vub = VHOST_USER_BASE(dev);
+    VHostUserBaseClass *vubc = VHOST_USER_BASE_GET_CLASS(dev);
 
-    if (i2c->connected) {
-        return 0;
-    }
-    i2c->connected = true;
+    /* Fixed for I2C */
+    vub->virtio_id = VIRTIO_ID_I2C_ADAPTER;
+    vub->num_vqs = 1;
+    vub->vq_size = 4;
 
-    /* restore vhost state */
-    if (virtio_device_started(vdev, vdev->status)) {
-        vu_i2c_start(vdev);
-    }
-
-    return 0;
-}
-
-static void vu_i2c_disconnect(DeviceState *dev)
-{
-    VirtIODevice *vdev = VIRTIO_DEVICE(dev);
-    VHostUserI2C *i2c = VHOST_USER_I2C(vdev);
-
-    if (!i2c->connected) {
-        return;
-    }
-    i2c->connected = false;
-
-    if (vhost_dev_is_started(&i2c->vhost_dev)) {
-        vu_i2c_stop(vdev);
-    }
-}
-
-static void vu_i2c_event(void *opaque, QEMUChrEvent event)
-{
-    DeviceState *dev = opaque;
-    VirtIODevice *vdev = VIRTIO_DEVICE(dev);
-    VHostUserI2C *i2c = VHOST_USER_I2C(vdev);
-
-    switch (event) {
-    case CHR_EVENT_OPENED:
-        if (vu_i2c_connect(dev) < 0) {
-            qemu_chr_fe_disconnect(&i2c->chardev);
-            return;
-        }
-        break;
-    case CHR_EVENT_CLOSED:
-        vu_i2c_disconnect(dev);
-        break;
-    case CHR_EVENT_BREAK:
-    case CHR_EVENT_MUX_IN:
-    case CHR_EVENT_MUX_OUT:
-        /* Ignore */
-        break;
-    }
-}
-
-static void vu_i2c_device_realize(DeviceState *dev, Error **errp)
-{
-    VirtIODevice *vdev = VIRTIO_DEVICE(dev);
-    VHostUserI2C *i2c = VHOST_USER_I2C(dev);
-    int ret;
-
-    if (!i2c->chardev.chr) {
-        error_setg(errp, "vhost-user-i2c: missing chardev");
-        return;
-    }
-
-    if (!vhost_user_init(&i2c->vhost_user, &i2c->chardev, errp)) {
-        return;
-    }
-
-    virtio_init(vdev, VIRTIO_ID_I2C_ADAPTER, 0);
-
-    i2c->vhost_dev.nvqs = 1;
-    i2c->vq = virtio_add_queue(vdev, 4, vu_i2c_handle_output);
-    i2c->vhost_dev.vqs = g_new0(struct vhost_virtqueue, i2c->vhost_dev.nvqs);
-
-    ret = vhost_dev_init(&i2c->vhost_dev, &i2c->vhost_user,
-                         VHOST_BACKEND_TYPE_USER, 0, errp);
-    if (ret < 0) {
-        g_free(i2c->vhost_dev.vqs);
-        do_vhost_user_cleanup(vdev, i2c);
-    }
-
-    qemu_chr_fe_set_handlers(&i2c->chardev, NULL, NULL, vu_i2c_event, NULL,
-                             dev, NULL, true);
-}
-
-static void vu_i2c_device_unrealize(DeviceState *dev)
-{
-    VirtIODevice *vdev = VIRTIO_DEVICE(dev);
-    VHostUserI2C *i2c = VHOST_USER_I2C(dev);
-    struct vhost_virtqueue *vhost_vqs = i2c->vhost_dev.vqs;
-
-    /* This will stop vhost backend if appropriate. */
-    vu_i2c_set_status(vdev, 0);
-    vhost_dev_cleanup(&i2c->vhost_dev);
-    g_free(vhost_vqs);
-    do_vhost_user_cleanup(vdev, i2c);
+    vubc->parent_realize(dev, errp);
 }
 
 static const VMStateDescription vu_i2c_vmstate = {
@@ -268,30 +37,21 @@ static const VMStateDescription vu_i2c_vmstate = {
     .unmigratable = 1,
 };
 
-static Property vu_i2c_properties[] = {
-    DEFINE_PROP_CHR("chardev", VHostUserI2C, chardev),
-    DEFINE_PROP_END_OF_LIST(),
-};
-
 static void vu_i2c_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
-    VirtioDeviceClass *vdc = VIRTIO_DEVICE_CLASS(klass);
+    VHostUserBaseClass *vubc = VHOST_USER_BASE_CLASS(klass);
 
-    device_class_set_props(dc, vu_i2c_properties);
     dc->vmsd = &vu_i2c_vmstate;
+    device_class_set_props(dc, vi2c_properties);
+    device_class_set_parent_realize(dc, vi2c_realize,
+                                    &vubc->parent_realize);
     set_bit(DEVICE_CATEGORY_INPUT, dc->categories);
-    vdc->realize = vu_i2c_device_realize;
-    vdc->unrealize = vu_i2c_device_unrealize;
-    vdc->get_features = vu_i2c_get_features;
-    vdc->set_status = vu_i2c_set_status;
-    vdc->guest_notifier_mask = vu_i2c_guest_notifier_mask;
-    vdc->guest_notifier_pending = vu_i2c_guest_notifier_pending;
 }
 
 static const TypeInfo vu_i2c_info = {
     .name = TYPE_VHOST_USER_I2C,
-    .parent = TYPE_VIRTIO_DEVICE,
+    .parent = TYPE_VHOST_USER_BASE,
     .instance_size = sizeof(VHostUserI2C),
     .class_init = vu_i2c_class_init,
 };
diff --git a/hw/virtio/meson.build b/hw/virtio/meson.build
index 752d4a6905..f3e1ccc9c9 100644
--- a/hw/virtio/meson.build
+++ b/hw/virtio/meson.build
@@ -22,12 +22,15 @@ if have_vhost
     # MMIO Stubs
     system_virtio_ss.add(files('vhost-user-device.c'))
     system_virtio_ss.add(when: 'CONFIG_VHOST_USER_GPIO', if_true: files('vhost-user-gpio.c'))
+    system_virtio_ss.add(when: 'CONFIG_VHOST_USER_I2C', if_true: files('vhost-user-i2c.c'))
     system_virtio_ss.add(when: 'CONFIG_VHOST_USER_RNG', if_true: files('vhost-user-rng.c'))
 
     # PCI Stubs
     system_virtio_ss.add(when: 'CONFIG_VIRTIO_PCI', if_true: files('vhost-user-device-pci.c'))
     system_virtio_ss.add(when: ['CONFIG_VIRTIO_PCI', 'CONFIG_VHOST_USER_GPIO'],
                          if_true: files('vhost-user-gpio-pci.c'))
+    system_virtio_ss.add(when: ['CONFIG_VIRTIO_PCI', 'CONFIG_VHOST_USER_I2C'],
+                         if_true: files('vhost-user-i2c-pci.c'))
     system_virtio_ss.add(when: ['CONFIG_VIRTIO_PCI', 'CONFIG_VHOST_USER_RNG'],
                          if_true: files('vhost-user-rng-pci.c'))
   endif
@@ -45,7 +48,6 @@ specific_virtio_ss.add(when: 'CONFIG_VIRTIO_PMEM', if_true: files('virtio-pmem.c
 specific_virtio_ss.add(when: 'CONFIG_VHOST_VSOCK', if_true: files('vhost-vsock.c'))
 specific_virtio_ss.add(when: 'CONFIG_VHOST_USER_VSOCK', if_true: files('vhost-user-vsock.c'))
 specific_virtio_ss.add(when: 'CONFIG_VIRTIO_MEM', if_true: files('virtio-mem.c'))
-specific_virtio_ss.add(when: 'CONFIG_VHOST_USER_I2C', if_true: files('vhost-user-i2c.c'))
 specific_virtio_ss.add(when: 'CONFIG_VHOST_USER_SCMI', if_true: files('vhost-user-scmi.c'))
 specific_virtio_ss.add(when: ['CONFIG_VIRTIO_PCI', 'CONFIG_VHOST_USER_SCMI'], if_true: files('vhost-user-scmi-pci.c'))
 
@@ -53,7 +55,6 @@ virtio_pci_ss = ss.source_set()
 virtio_pci_ss.add(when: 'CONFIG_VHOST_VSOCK', if_true: files('vhost-vsock-pci.c'))
 virtio_pci_ss.add(when: 'CONFIG_VHOST_USER_VSOCK', if_true: files('vhost-user-vsock-pci.c'))
 virtio_pci_ss.add(when: 'CONFIG_VHOST_USER_BLK', if_true: files('vhost-user-blk-pci.c'))
-virtio_pci_ss.add(when: 'CONFIG_VHOST_USER_I2C', if_true: files('vhost-user-i2c-pci.c'))
 virtio_pci_ss.add(when: 'CONFIG_VHOST_USER_INPUT', if_true: files('vhost-user-input-pci.c'))
 virtio_pci_ss.add(when: 'CONFIG_VHOST_USER_SCSI', if_true: files('vhost-user-scsi-pci.c'))
 virtio_pci_ss.add(when: 'CONFIG_VHOST_SCSI', if_true: files('vhost-scsi-pci.c'))
-- 
2.39.2


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

* [Virtio-fs] [PATCH v4 5/6] hw/virtio: add vhost-user-snd and virtio-snd-pci devices
  2023-10-09  9:59 [Virtio-fs] [PATCH v4 0/6] virtio: cleanup vhost-user-generic and reduce c&p Alex Bennée
                   ` (3 preceding siblings ...)
  2023-10-09  9:59 ` [Virtio-fs] [PATCH v4 4/6] hw/virtio: derive vhost-user-i2c " Alex Bennée
@ 2023-10-09  9:59 ` Alex Bennée
  2023-10-09  9:59 ` [Virtio-fs] [PATCH v4 6/6] docs/system: add a basic enumeration of vhost-user devices Alex Bennée
  2023-10-10 15:14 ` [Virtio-fs] [PATCH v4 0/6] virtio: cleanup vhost-user-generic and reduce c&p Mark Cave-Ayland
  6 siblings, 0 replies; 14+ messages in thread
From: Alex Bennée @ 2023-10-09  9:59 UTC (permalink / raw)
  To: qemu-devel
  Cc: Marc-André Lureau, Erik Schilling, Fam Zheng,
	Mathieu Poirier, Gerd Hoffmann, Hanna Reitz, Stefan Hajnoczi,
	Alex Bennée, Kevin Wolf, Eric Blake, Michael S. Tsirkin,
	qemu-block, Eduardo Habkost, Paolo Bonzini, Mark Cave-Ayland,
	Viresh Kumar, virtio-fs, Gonglei (Arei),
	Markus Armbruster, Jason Wang, Raphael Norwitz,
	Daniel P. Berrangé,
	Manos Pitsidianakis

From: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>

Tested with rust-vmm vhost-user-sound daemon:

    RUST_LOG=trace cargo run --bin vhost-user-sound -- --socket /tmp/snd.sock --backend null

Invocation:

    qemu-system-x86_64  \
            -qmp unix:./qmp-sock,server,wait=off  \
            -m 4096 \
            -numa node,memdev=mem \
            -object memory-backend-file,id=mem,size=4G,mem-path=/dev/shm,share=on \
            -D qemu.log \
            -d guest_errors,trace:\*snd\*,trace:\*sound\*,trace:\*vhost\* \
            -chardev socket,id=vsnd,path=/tmp/snd.sock \
            -device vhost-user-snd-pci,chardev=vsnd,id=snd \
            /path/to/disk

[AJB: imported from https://github.com/epilys/qemu-virtio-snd/commit/54ae1cdd15fef2d88e9e387a175f099a38c636f4.patch]
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>

---
v1
  - import and test
---
 include/hw/virtio/vhost-user-snd.h | 26 +++++++++++
 hw/virtio/vhost-user-snd-pci.c     | 75 ++++++++++++++++++++++++++++++
 hw/virtio/vhost-user-snd.c         | 67 ++++++++++++++++++++++++++
 hw/virtio/Kconfig                  |  5 ++
 hw/virtio/meson.build              |  3 ++
 5 files changed, 176 insertions(+)
 create mode 100644 include/hw/virtio/vhost-user-snd.h
 create mode 100644 hw/virtio/vhost-user-snd-pci.c
 create mode 100644 hw/virtio/vhost-user-snd.c

diff --git a/include/hw/virtio/vhost-user-snd.h b/include/hw/virtio/vhost-user-snd.h
new file mode 100644
index 0000000000..a1627003a0
--- /dev/null
+++ b/include/hw/virtio/vhost-user-snd.h
@@ -0,0 +1,26 @@
+/*
+ * Vhost-user Sound virtio device
+ *
+ * Copyright (c) 2021 Mathieu Poirier <mathieu.poirier@linaro.org>
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#ifndef QEMU_VHOST_USER_SND_H
+#define QEMU_VHOST_USER_SND_H
+
+#include "hw/virtio/virtio.h"
+#include "hw/virtio/vhost.h"
+#include "hw/virtio/vhost-user.h"
+#include "hw/virtio/vhost-user-base.h"
+
+#define TYPE_VHOST_USER_SND "vhost-user-snd"
+OBJECT_DECLARE_SIMPLE_TYPE(VHostUserSound, VHOST_USER_SND)
+
+struct VHostUserSound {
+    /*< private >*/
+    VHostUserBase parent;
+    /*< public >*/
+};
+
+#endif /* QEMU_VHOST_USER_SND_H */
diff --git a/hw/virtio/vhost-user-snd-pci.c b/hw/virtio/vhost-user-snd-pci.c
new file mode 100644
index 0000000000..d61cfdae63
--- /dev/null
+++ b/hw/virtio/vhost-user-snd-pci.c
@@ -0,0 +1,75 @@
+/*
+ * Vhost-user Sound virtio device PCI glue
+ *
+ * Copyright (c) 2023 Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include "qemu/osdep.h"
+#include "hw/qdev-properties.h"
+#include "hw/virtio/vhost-user-snd.h"
+#include "hw/virtio/virtio-pci.h"
+
+struct VHostUserSoundPCI {
+    VirtIOPCIProxy parent_obj;
+    VHostUserSound vdev;
+};
+
+typedef struct VHostUserSoundPCI VHostUserSoundPCI;
+
+#define TYPE_VHOST_USER_SND_PCI "vhost-user-snd-pci-base"
+
+DECLARE_INSTANCE_CHECKER(VHostUserSoundPCI, VHOST_USER_SND_PCI,
+                         TYPE_VHOST_USER_SND_PCI)
+
+static Property vhost_user_snd_pci_properties[] = {
+    DEFINE_PROP_END_OF_LIST(),
+};
+
+static void vhost_user_snd_pci_realize(VirtIOPCIProxy *vpci_dev, Error **errp)
+{
+    VHostUserSoundPCI *dev = VHOST_USER_SND_PCI(vpci_dev);
+    DeviceState *vdev = DEVICE(&dev->vdev);
+
+    vpci_dev->nvectors = 1;
+
+    qdev_realize(vdev, BUS(&vpci_dev->bus), errp);
+}
+
+static void vhost_user_snd_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_snd_pci_realize;
+    set_bit(DEVICE_CATEGORY_SOUND, dc->categories);
+    device_class_set_props(dc, vhost_user_snd_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_MULTIMEDIA_AUDIO;
+}
+
+static void vhost_user_snd_pci_instance_init(Object *obj)
+{
+    VHostUserSoundPCI *dev = VHOST_USER_SND_PCI(obj);
+
+    virtio_instance_init_common(obj, &dev->vdev, sizeof(dev->vdev),
+                                TYPE_VHOST_USER_SND);
+}
+
+static const VirtioPCIDeviceTypeInfo vhost_user_snd_pci_info = {
+    .base_name = TYPE_VHOST_USER_SND_PCI,
+    .non_transitional_name = "vhost-user-snd-pci",
+    .instance_size = sizeof(VHostUserSoundPCI),
+    .instance_init = vhost_user_snd_pci_instance_init,
+    .class_init = vhost_user_snd_pci_class_init,
+};
+
+static void vhost_user_snd_pci_register(void)
+{
+    virtio_pci_types_register(&vhost_user_snd_pci_info);
+}
+
+type_init(vhost_user_snd_pci_register);
diff --git a/hw/virtio/vhost-user-snd.c b/hw/virtio/vhost-user-snd.c
new file mode 100644
index 0000000000..9a217543f8
--- /dev/null
+++ b/hw/virtio/vhost-user-snd.c
@@ -0,0 +1,67 @@
+/*
+ * Vhost-user snd virtio device
+ *
+ * Copyright (c) 2023 Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
+ *
+ * Simple wrapper of the generic vhost-user-device.
+ *
+ * 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-snd.h"
+#include "standard-headers/linux/virtio_ids.h"
+#include "standard-headers/linux/virtio_snd.h"
+
+static const VMStateDescription vu_snd_vmstate = {
+    .name = "vhost-user-snd",
+    .unmigratable = 1,
+};
+
+static Property vsnd_properties[] = {
+    DEFINE_PROP_CHR("chardev", VHostUserBase, chardev),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
+static void vu_snd_base_realize(DeviceState *dev, Error **errp)
+{
+    VHostUserBase *vub = VHOST_USER_BASE(dev);
+    VHostUserBaseClass *vubs = VHOST_USER_BASE_GET_CLASS(dev);
+
+    vub->virtio_id = VIRTIO_ID_SOUND;
+    vub->num_vqs = 4;
+    vub->config_size = sizeof(struct virtio_snd_config);
+    vub->vq_size = 64;
+
+    vubs->parent_realize(dev, errp);
+}
+
+static void vu_snd_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+    VHostUserBaseClass *vubc = VHOST_USER_BASE_CLASS(klass);
+
+    dc->vmsd = &vu_snd_vmstate;
+    device_class_set_props(dc, vsnd_properties);
+    device_class_set_parent_realize(dc, vu_snd_base_realize,
+                                    &vubc->parent_realize);
+
+    set_bit(DEVICE_CATEGORY_SOUND, dc->categories);
+}
+
+static const TypeInfo vu_snd_info = {
+    .name = TYPE_VHOST_USER_SND,
+    .parent = TYPE_VHOST_USER_BASE,
+    .instance_size = sizeof(VHostUserSound),
+    .class_init = vu_snd_class_init,
+};
+
+static void vu_snd_register_types(void)
+{
+    type_register_static(&vu_snd_info);
+}
+
+type_init(vu_snd_register_types)
diff --git a/hw/virtio/Kconfig b/hw/virtio/Kconfig
index 92c9cf6c96..aa63ff7fd4 100644
--- a/hw/virtio/Kconfig
+++ b/hw/virtio/Kconfig
@@ -101,6 +101,11 @@ config VHOST_VDPA_DEV
     default y
     depends on VIRTIO && VHOST_VDPA && LINUX
 
+config VHOST_USER_SND
+    bool
+    default y
+    depends on VIRTIO && VHOST_USER
+
 config VHOST_USER_SCMI
     bool
     default y
diff --git a/hw/virtio/meson.build b/hw/virtio/meson.build
index f3e1ccc9c9..e00de61da9 100644
--- a/hw/virtio/meson.build
+++ b/hw/virtio/meson.build
@@ -24,6 +24,7 @@ if have_vhost
     system_virtio_ss.add(when: 'CONFIG_VHOST_USER_GPIO', if_true: files('vhost-user-gpio.c'))
     system_virtio_ss.add(when: 'CONFIG_VHOST_USER_I2C', if_true: files('vhost-user-i2c.c'))
     system_virtio_ss.add(when: 'CONFIG_VHOST_USER_RNG', if_true: files('vhost-user-rng.c'))
+    system_virtio_ss.add(when: 'CONFIG_VHOST_USER_SND', if_true: files('vhost-user-snd.c'))
 
     # PCI Stubs
     system_virtio_ss.add(when: 'CONFIG_VIRTIO_PCI', if_true: files('vhost-user-device-pci.c'))
@@ -33,6 +34,8 @@ if have_vhost
                          if_true: files('vhost-user-i2c-pci.c'))
     system_virtio_ss.add(when: ['CONFIG_VIRTIO_PCI', 'CONFIG_VHOST_USER_RNG'],
                          if_true: files('vhost-user-rng-pci.c'))
+    system_virtio_ss.add(when: ['CONFIG_VIRTIO_PCI', 'CONFIG_VHOST_USER_SND'],
+                         if_true: files('vhost-user-snd-pci.c'))
   endif
   if have_vhost_vdpa
     system_virtio_ss.add(files('vhost-vdpa.c'))
-- 
2.39.2


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

* [Virtio-fs] [PATCH v4 6/6] docs/system: add a basic enumeration of vhost-user devices
  2023-10-09  9:59 [Virtio-fs] [PATCH v4 0/6] virtio: cleanup vhost-user-generic and reduce c&p Alex Bennée
                   ` (4 preceding siblings ...)
  2023-10-09  9:59 ` [Virtio-fs] [PATCH v4 5/6] hw/virtio: add vhost-user-snd and virtio-snd-pci devices Alex Bennée
@ 2023-10-09  9:59 ` Alex Bennée
  2023-10-09 13:38   ` Manos Pitsidianakis
  2023-10-10 15:14 ` [Virtio-fs] [PATCH v4 0/6] virtio: cleanup vhost-user-generic and reduce c&p Mark Cave-Ayland
  6 siblings, 1 reply; 14+ messages in thread
From: Alex Bennée @ 2023-10-09  9:59 UTC (permalink / raw)
  To: qemu-devel
  Cc: Marc-André Lureau, Erik Schilling, Fam Zheng,
	Mathieu Poirier, Gerd Hoffmann, Hanna Reitz, Stefan Hajnoczi,
	Alex Bennée, Kevin Wolf, Eric Blake, Michael S. Tsirkin,
	qemu-block, Eduardo Habkost, Paolo Bonzini, Mark Cave-Ayland,
	Viresh Kumar, virtio-fs, Gonglei (Arei),
	Markus Armbruster, Jason Wang, Raphael Norwitz,
	Daniel P. Berrangé

Make it clear the vhost-user-device is intended for expert use only.

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

---
v2
  - make clear vhost-user-device for expert use
---
 docs/system/devices/vhost-user-rng.rst |  2 ++
 docs/system/devices/vhost-user.rst     | 41 ++++++++++++++++++++++++++
 2 files changed, 43 insertions(+)

diff --git a/docs/system/devices/vhost-user-rng.rst b/docs/system/devices/vhost-user-rng.rst
index a145d4105c..ead1405326 100644
--- a/docs/system/devices/vhost-user-rng.rst
+++ b/docs/system/devices/vhost-user-rng.rst
@@ -1,3 +1,5 @@
+.. _vhost_user_rng:
+
 QEMU vhost-user-rng - RNG emulation
 ===================================
 
diff --git a/docs/system/devices/vhost-user.rst b/docs/system/devices/vhost-user.rst
index a80e95a48a..0f9eec3f00 100644
--- a/docs/system/devices/vhost-user.rst
+++ b/docs/system/devices/vhost-user.rst
@@ -15,6 +15,47 @@ to the guest. The code is mostly boilerplate although each device has
 a ``chardev`` option which specifies the ID of the ``--chardev``
 device that connects via a socket to the vhost-user *daemon*.
 
+Each device will have an virtio-mmio and virtio-pci variant. See your
+platform details for what sort of virtio bus to use.
+
+.. list-table:: vhost-user devices
+  :widths: 20 20 60
+  :header-rows: 1
+
+  * - Device
+    - Type
+    - Notes
+  * - vhost-user-device
+    - Generic Development Device
+    - You must manually specify ``virtio-id`` and the correct ``num_vqs``. Intended for expert use.
+  * - vhost-user-blk
+    - Block storage
+    -
+  * - vhost-user-fs
+    - File based storage driver
+    - See https://gitlab.com/virtio-fs/virtiofsd
+  * - vhost-user-scsi
+    - SCSI based storage
+    - See contrib/vhost-user/scsi
+  * - vhost-user-gpio
+    - Proxy gpio pins to host
+    - See https://github.com/rust-vmm/vhost-device
+  * - vhost-user-i2c
+    - Proxy i2c devices to host
+    - See https://github.com/rust-vmm/vhost-device
+  * - vhost-user-input
+    - Generic input driver
+    - See contrib/vhost-user-input
+  * - vhost-user-rng
+    - Entropy driver
+    - :ref:`vhost_user_rng`
+  * - vhost-user-gpu
+    - GPU driver
+    -
+  * - vhost-user-vsock
+    - Socket based communication
+    - See https://github.com/rust-vmm/vhost-device
+
 vhost-user daemon
 =================
 
-- 
2.39.2


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

* Re: [Virtio-fs] [PATCH v4 3/6] hw/virtio: derive vhost-user-gpio from vhost-user-base
  2023-10-09  9:59 ` [Virtio-fs] [PATCH v4 3/6] hw/virtio: derive vhost-user-gpio " Alex Bennée
@ 2023-10-09 10:59   ` Viresh Kumar
  0 siblings, 0 replies; 14+ messages in thread
From: Viresh Kumar @ 2023-10-09 10:59 UTC (permalink / raw)
  To: Alex Bennée
  Cc: qemu-devel, Marc-André Lureau, Erik Schilling, Fam Zheng,
	Mathieu Poirier, Gerd Hoffmann, Hanna Reitz, Stefan Hajnoczi,
	Kevin Wolf, Eric Blake, Michael S. Tsirkin, qemu-block,
	Eduardo Habkost, Paolo Bonzini, Mark Cave-Ayland, virtio-fs,
	Gonglei (Arei),
	Markus Armbruster, Jason Wang, Raphael Norwitz,
	Daniel P. Berrangé

On 09-10-23, 10:59, Alex Bennée wrote:
> Now the new base class supports config handling we can take advantage
> and make vhost-user-gpio a much simpler boilerplate wrapper. Also as
> this doesn't require any target specific hacks we only need to build
> the stubs once.
> 
> Message-Id: <20230418162140.373219-12-alex.bennee@linaro.org>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> Acked-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> 
> ---
> v2
>   - use new vhost-user-base
>   - move build to common code
> v3
>   - fix inadvertent double link
> v4
>   - merge conflict
>   - update includes
> ---
>  include/hw/virtio/vhost-user-gpio.h |  23 +-
>  hw/virtio/vhost-user-gpio.c         | 407 ++--------------------------
>  hw/virtio/meson.build               |   5 +-
>  3 files changed, 22 insertions(+), 413 deletions(-)

Looks nice. Thanks.

Acked-by: Viresh Kumar <viresh.kumar@linaro.org>

-- 
viresh


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

* Re: [Virtio-fs] [PATCH v4 4/6] hw/virtio: derive vhost-user-i2c from vhost-user-base
  2023-10-09  9:59 ` [Virtio-fs] [PATCH v4 4/6] hw/virtio: derive vhost-user-i2c " Alex Bennée
@ 2023-10-09 10:59   ` Viresh Kumar
  0 siblings, 0 replies; 14+ messages in thread
From: Viresh Kumar @ 2023-10-09 10:59 UTC (permalink / raw)
  To: Alex Bennée
  Cc: qemu-devel, Marc-André Lureau, Erik Schilling, Fam Zheng,
	Mathieu Poirier, Gerd Hoffmann, Hanna Reitz, Stefan Hajnoczi,
	Kevin Wolf, Eric Blake, Michael S. Tsirkin, qemu-block,
	Eduardo Habkost, Paolo Bonzini, Mark Cave-Ayland, virtio-fs,
	Gonglei (Arei),
	Markus Armbruster, Jason Wang, Raphael Norwitz,
	Daniel P. Berrangé

On 09-10-23, 10:59, Alex Bennée wrote:
> Now we can take advantage of the new base class and make
> vhost-user-i2c a much simpler boilerplate wrapper. Also as this
> doesn't require any target specific hacks we only need to build the
> stubs once.
> 
> Message-Id: <20230418162140.373219-13-alex.bennee@linaro.org>
> Acked-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> 
> ---
> v2
>   - update to new inheritance scheme
>   - move build to common code
> v3
>   - fix merge conflict in meson
>   - style updates, remove duplicate includes
> v4
>   - use vqsize
> ---
>  include/hw/virtio/vhost-user-i2c.h |  14 +-
>  hw/virtio/vhost-user-i2c.c         | 272 ++---------------------------
>  hw/virtio/meson.build              |   5 +-
>  3 files changed, 23 insertions(+), 268 deletions(-)

Acked-by: Viresh Kumar <viresh.kumar@linaro.org>

-- 
viresh


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

* Re: [Virtio-fs] [PATCH v4 2/6] hw/virtio: derive vhost-user-rng from vhost-user-base
  2023-10-09  9:59 ` [Virtio-fs] [PATCH v4 2/6] hw/virtio: derive vhost-user-rng from vhost-user-base Alex Bennée
@ 2023-10-09 13:07   ` Manos Pitsidianakis
  2023-10-09 13:48     ` Alex Bennée
  0 siblings, 1 reply; 14+ messages in thread
From: Manos Pitsidianakis @ 2023-10-09 13:07 UTC (permalink / raw)
  To: qemu-devel, Alex Benné e
  Cc: Marc-André Lureau, Erik Schilling, Fam Zheng,
	Mathieu Poirier, Gerd Hoffmann, Hanna Reitz, Stefan Hajnoczi,
	Alex Benné e, Kevin Wolf, Eric Blake, Michael S. Tsirkin,
	qemu-block, Eduardo Habkost, Paolo Bonzini, Mark Cave-Ayland,
	Viresh Kumar, virtio-fs, Gonglei (Arei),
	Markus Armbruster, Jason Wang, Raphael Norwitz,
	Daniel P. Berrangé

On Mon, 09 Oct 2023 12:59, Alex Bennée <alex.bennee@linaro.org> wrote:
>diff --git a/hw/virtio/meson.build b/hw/virtio/meson.build
>index 51c3f97c2d..d0b963199c 100644
>--- a/hw/virtio/meson.build
>+++ b/hw/virtio/meson.build
>@@ -18,8 +18,15 @@ if have_vhost
>     # fixme - this really should be generic
>     specific_virtio_ss.add(files('vhost-user.c'))
>     system_virtio_ss.add(files('vhost-user-base.c'))
>+
>+    # MMIO Stubs
>     system_virtio_ss.add(files('vhost-user-device.c'))
>+    system_virtio_ss.add(when: 'CONFIG_VHOST_USER_RNG', if_true: files('vhost-user-rng.c'))
>+
>+    # PCI Stubs
>     system_virtio_ss.add(when: 'CONFIG_VIRTIO_PCI', if_true: files('vhost-user-device-pci.c'))
>+    system_virtio_ss.add(when: ['CONFIG_VIRTIO_PCI', 'CONFIG_VHOST_USER_RNG'],
>+                         if_true: files('vhost-user-rng-pci.c'))

Is there a reason why the target was moved to system_virtio_ss from 
virtio_pci_ss?

>   endif
>   if have_vhost_vdpa
>     system_virtio_ss.add(files('vhost-vdpa.c'))
>@@ -34,10 +41,8 @@ specific_virtio_ss.add(when: 'CONFIG_VHOST_USER_FS', if_true: files('vhost-user-
> specific_virtio_ss.add(when: 'CONFIG_VIRTIO_PMEM', if_true: files('virtio-pmem.c'))
> specific_virtio_ss.add(when: 'CONFIG_VHOST_VSOCK', if_true: files('vhost-vsock.c'))
> specific_virtio_ss.add(when: 'CONFIG_VHOST_USER_VSOCK', if_true: files('vhost-user-vsock.c'))
>-specific_virtio_ss.add(when: 'CONFIG_VIRTIO_RNG', if_true: files('virtio-rng.c'))

Was this accidental? It's not added anywhere else, only deleted.

>@@ -57,7 +61,6 @@ virtio_pci_ss.add(when: 'CONFIG_VHOST_USER_FS', 
> if_true: files('vhost-user-fs-pc
> virtio_pci_ss.add(when: 'CONFIG_VIRTIO_CRYPTO', if_true: files('virtio-crypto-pci.c'))
> virtio_pci_ss.add(when: 'CONFIG_VIRTIO_INPUT_HOST', if_true: files('virtio-input-host-pci.c'))
> virtio_pci_ss.add(when: 'CONFIG_VIRTIO_INPUT', if_true: files('virtio-input-pci.c'))
>-virtio_pci_ss.add(when: 'CONFIG_VIRTIO_RNG', if_true: files('virtio-rng-pci.c'))
> virtio_pci_ss.add(when: 'CONFIG_VIRTIO_BALLOON', if_true: files('virtio-balloon-pci.c'))
> virtio_pci_ss.add(when: 'CONFIG_VIRTIO_9P', if_true: files('virtio-9p-pci.c'))
> virtio_pci_ss.add(when: 'CONFIG_VIRTIO_SCSI', if_true: files('virtio-scsi-pci.c'))

Same here

Manos


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

* Re: [Virtio-fs] [PATCH v4 6/6] docs/system: add a basic enumeration of vhost-user devices
  2023-10-09  9:59 ` [Virtio-fs] [PATCH v4 6/6] docs/system: add a basic enumeration of vhost-user devices Alex Bennée
@ 2023-10-09 13:38   ` Manos Pitsidianakis
  0 siblings, 0 replies; 14+ messages in thread
From: Manos Pitsidianakis @ 2023-10-09 13:38 UTC (permalink / raw)
  To: qemu-block, Alex Benné e, qemu-devel
  Cc: Marc-André Lureau, Erik Schilling, Fam Zheng,
	Mathieu Poirier, Gerd Hoffmann, Hanna Reitz, Stefan Hajnoczi,
	Alex Benné e, Kevin Wolf, Eric Blake, Michael S. Tsirkin,
	Eduardo Habkost, Paolo Bonzini, Mark Cave-Ayland, Viresh Kumar,
	virtio-fs, Gonglei (Arei),
	Markus Armbruster, Jason Wang, Raphael Norwitz,
	Daniel P. Berrangé

On Mon, 09 Oct 2023 12:59, Alex Bennée <alex.bennee@linaro.org> wrote:
>diff --git a/docs/system/devices/vhost-user.rst 
>b/docs/system/devices/vhost-user.rst
>index a80e95a48a..0f9eec3f00 100644
>--- a/docs/system/devices/vhost-user.rst
>+++ b/docs/system/devices/vhost-user.rst
>@@ -15,6 +15,47 @@ to the guest. The code is mostly boilerplate although each device has
> a ``chardev`` option which specifies the ID of the ``--chardev``
> device that connects via a socket to the vhost-user *daemon*.
> 
>+Each device will have an virtio-mmio and virtio-pci variant. See your
>+platform details for what sort of virtio bus to use.
>+
>+.. list-table:: vhost-user devices
>+  :widths: 20 20 60
>+  :header-rows: 1
>+
>+  * - Device
>+    - Type
>+    - Notes
>+  * - vhost-user-device
>+    - Generic Development Device
>+    - You must manually specify ``virtio-id`` and the correct ``num_vqs``. Intended for expert use.

May be worth specifying they are `VHostUserBase` interface fields since 
it's not directly obvious if you come across this before reading the 
vhost-user-device code.

>+  * - vhost-user-blk
>+    - Block storage
>+    -
>+  * - vhost-user-fs
>+    - File based storage driver
>+    - See https://gitlab.com/virtio-fs/virtiofsd
>+  * - vhost-user-scsi
>+    - SCSI based storage
>+    - See contrib/vhost-user/scsi
>+  * - vhost-user-gpio
>+    - Proxy gpio pins to host
>+    - See https://github.com/rust-vmm/vhost-device
>+  * - vhost-user-i2c
>+    - Proxy i2c devices to host
>+    - See https://github.com/rust-vmm/vhost-device
>+  * - vhost-user-input
>+    - Generic input driver
>+    - See contrib/vhost-user-input
>+  * - vhost-user-rng
>+    - Entropy driver
>+    - :ref:`vhost_user_rng`
>+  * - vhost-user-gpu
>+    - GPU driver
>+    -
>+  * - vhost-user-vsock
>+    - Socket based communication
>+    - See https://github.com/rust-vmm/vhost-device
>+

There's also:

- hw/virtio/vhost-user-scmi.c
- hw/virtio/vhost-user-snd.c


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

* Re: [Virtio-fs] [PATCH v4 2/6] hw/virtio: derive vhost-user-rng from vhost-user-base
  2023-10-09 13:07   ` Manos Pitsidianakis
@ 2023-10-09 13:48     ` Alex Bennée
  0 siblings, 0 replies; 14+ messages in thread
From: Alex Bennée @ 2023-10-09 13:48 UTC (permalink / raw)
  To: Manos Pitsidianakis
  Cc: qemu-devel, Marc-André Lureau, Erik Schilling, Fam Zheng,
	Mathieu Poirier, Gerd Hoffmann, Hanna Reitz, Stefan Hajnoczi,
	Kevin Wolf, Eric Blake, Michael S. Tsirkin, qemu-block,
	Eduardo Habkost, Paolo Bonzini, Mark Cave-Ayland, Viresh Kumar,
	virtio-fs, Gonglei, Markus Armbruster, Jason Wang,
	Raphael Norwitz, Daniel P. Berrangé


Manos Pitsidianakis <manos.pitsidianakis@linaro.org> writes:

> On Mon, 09 Oct 2023 12:59, Alex Bennée <alex.bennee@linaro.org> wrote:
>>diff --git a/hw/virtio/meson.build b/hw/virtio/meson.build
>>index 51c3f97c2d..d0b963199c 100644
>>--- a/hw/virtio/meson.build
>>+++ b/hw/virtio/meson.build
>>@@ -18,8 +18,15 @@ if have_vhost
>>     # fixme - this really should be generic
>>     specific_virtio_ss.add(files('vhost-user.c'))
>>     system_virtio_ss.add(files('vhost-user-base.c'))
>>+
>>+    # MMIO Stubs
>>     system_virtio_ss.add(files('vhost-user-device.c'))
>>+    system_virtio_ss.add(when: 'CONFIG_VHOST_USER_RNG', if_true: files('vhost-user-rng.c'))
>>+
>>+    # PCI Stubs
>>     system_virtio_ss.add(when: 'CONFIG_VIRTIO_PCI', if_true: files('vhost-user-device-pci.c'))
>>+    system_virtio_ss.add(when: ['CONFIG_VIRTIO_PCI', 'CONFIG_VHOST_USER_RNG'],
>>+                         if_true: files('vhost-user-rng-pci.c'))
>
> Is there a reason why the target was moved to system_virtio_ss from
> virtio_pci_ss?

So we build it once, virtio_pci_ss is still:

  specific_virtio_ss.add_all(when: 'CONFIG_VIRTIO_PCI', if_true: virtio_pci_ss)

which means we build it once for every target which is overkill.

>
>>   endif
>>   if have_vhost_vdpa
>>     system_virtio_ss.add(files('vhost-vdpa.c'))
>>@@ -34,10 +41,8 @@ specific_virtio_ss.add(when: 'CONFIG_VHOST_USER_FS', if_true: files('vhost-user-
>> specific_virtio_ss.add(when: 'CONFIG_VIRTIO_PMEM', if_true: files('virtio-pmem.c'))
>> specific_virtio_ss.add(when: 'CONFIG_VHOST_VSOCK', if_true: files('vhost-vsock.c'))
>> specific_virtio_ss.add(when: 'CONFIG_VHOST_USER_VSOCK', if_true: files('vhost-user-vsock.c'))
>>-specific_virtio_ss.add(when: 'CONFIG_VIRTIO_RNG', if_true: files('virtio-rng.c'))
>
> Was this accidental? It's not added anywhere else, only deleted.

Oops yes. Didn't mean to delete the in-tree emulation. Will fix.

>
>> @@ -57,7 +61,6 @@ virtio_pci_ss.add(when: 'CONFIG_VHOST_USER_FS',
>> if_true: files('vhost-user-fs-pc
>> virtio_pci_ss.add(when: 'CONFIG_VIRTIO_CRYPTO', if_true: files('virtio-crypto-pci.c'))
>> virtio_pci_ss.add(when: 'CONFIG_VIRTIO_INPUT_HOST', if_true: files('virtio-input-host-pci.c'))
>> virtio_pci_ss.add(when: 'CONFIG_VIRTIO_INPUT', if_true: files('virtio-input-pci.c'))
>>-virtio_pci_ss.add(when: 'CONFIG_VIRTIO_RNG', if_true: files('virtio-rng-pci.c'))
>> virtio_pci_ss.add(when: 'CONFIG_VIRTIO_BALLOON', if_true: files('virtio-balloon-pci.c'))
>> virtio_pci_ss.add(when: 'CONFIG_VIRTIO_9P', if_true: files('virtio-9p-pci.c'))
>> virtio_pci_ss.add(when: 'CONFIG_VIRTIO_SCSI', if_true: files('virtio-scsi-pci.c'))
>
> Same here
>
> Manos


-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro


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

* Re: [Virtio-fs] [PATCH v4 1/6] virtio: split into vhost-user-base and vhost-user-device
  2023-10-09  9:59 ` [Virtio-fs] [PATCH v4 1/6] virtio: split into vhost-user-base and vhost-user-device Alex Bennée
@ 2023-10-10 15:08   ` Mark Cave-Ayland
  0 siblings, 0 replies; 14+ messages in thread
From: Mark Cave-Ayland @ 2023-10-10 15:08 UTC (permalink / raw)
  To: Alex Bennée, qemu-devel
  Cc: Marc-André Lureau, Erik Schilling, Fam Zheng,
	Mathieu Poirier, Gerd Hoffmann, Hanna Reitz, Stefan Hajnoczi,
	Kevin Wolf, Eric Blake, Michael S. Tsirkin, qemu-block,
	Eduardo Habkost, Paolo Bonzini, Viresh Kumar, virtio-fs,
	Gonglei (Arei),
	Markus Armbruster, Jason Wang, Raphael Norwitz,
	Daniel P. Berrangé

On 09/10/2023 10:59, Alex Bennée wrote:

> Lets keep a cleaner split between the base class and the derived
> vhost-user-device which we can use for generic vhost-user stubs. This
> includes an update to introduce the vq_size property so the number of
> entries in a virtq can be defined.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> 
> ---
> v1
>    - merge and re-base, reset testing/review tags
> ---
>   include/hw/virtio/vhost-user-base.h |  49 ++++
>   hw/virtio/vhost-user-base.c         | 348 ++++++++++++++++++++++++++++
>   hw/virtio/vhost-user-device-pci.c   |  10 +-
>   hw/virtio/vhost-user-device.c       | 335 +-------------------------
>   hw/virtio/meson.build               |   1 +
>   5 files changed, 410 insertions(+), 333 deletions(-)
>   create mode 100644 include/hw/virtio/vhost-user-base.h
>   create mode 100644 hw/virtio/vhost-user-base.c
> 
> diff --git a/include/hw/virtio/vhost-user-base.h b/include/hw/virtio/vhost-user-base.h
> new file mode 100644
> index 0000000000..cad377468b
> --- /dev/null
> +++ b/include/hw/virtio/vhost-user-base.h
> @@ -0,0 +1,49 @@
> +/*
> + * Vhost-user generic virtio device
> + *
> + * Copyright (c) 2023 Linaro Ltd
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +
> +#ifndef QEMU_VHOST_USER_BASE_H
> +#define QEMU_VHOST_USER_BASE_H
> +
> +#include "hw/virtio/vhost.h"
> +#include "hw/virtio/vhost-user.h"
> +
> +#define TYPE_VHOST_USER_BASE "vhost-user-base"
> +
> +OBJECT_DECLARE_TYPE(VHostUserBase, VHostUserBaseClass, VHOST_USER_BASE)
> +
> +struct VHostUserBase {
> +    VirtIODevice parent;

This one should be parent_obj here.

> +
> +    /* Properties */
> +    CharBackend chardev;
> +    uint16_t virtio_id;
> +    uint32_t num_vqs;
> +    uint32_t vq_size; /* can't exceed VIRTIO_QUEUE_MAX */
> +    uint32_t config_size;
> +    /* State tracking */
> +    VhostUserState vhost_user;
> +    struct vhost_virtqueue *vhost_vq;
> +    struct vhost_dev vhost_dev;
> +    GPtrArray *vqs;
> +    bool connected;
> +};
> +
> +/*
> + * Needed so we can use the base realize after specialisation
> + * tweaks
> + */
> +struct VHostUserBaseClass {
> +    VirtioDeviceClass parent_class;
> +
> +    DeviceRealize parent_realize;
> +};
> +
> +
> +#define TYPE_VHOST_USER_DEVICE "vhost-user-device"
> +
> +#endif /* QEMU_VHOST_USER_BASE_H */
> diff --git a/hw/virtio/vhost-user-base.c b/hw/virtio/vhost-user-base.c
> new file mode 100644
> index 0000000000..a8b811c394
> --- /dev/null
> +++ b/hw/virtio/vhost-user-base.c
> @@ -0,0 +1,348 @@
> +/*
> + * Base vhost-user-base implementation. This can be used to derive a
> + * more fully specified vhost-user backend either generically (see
> + * vhost-user-device) or via a specific stub for a device which
> + * encapsulates some fixed parameters.
> + *
> + * Copyright (c) 2023 Linaro Ltd
> + * Author: Alex Bennée <alex.bennee@linaro.org>
> + *
> + * 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-base.h"
> +#include "qemu/error-report.h"
> +
> +static void vub_start(VirtIODevice *vdev)
> +{
> +    BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(vdev)));
> +    VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
> +    VHostUserBase *vub = VHOST_USER_BASE(vdev);
> +    int ret, i;
> +
> +    if (!k->set_guest_notifiers) {
> +        error_report("binding does not support guest notifiers");
> +        return;
> +    }
> +
> +    ret = vhost_dev_enable_notifiers(&vub->vhost_dev, vdev);
> +    if (ret < 0) {
> +        error_report("Error enabling host notifiers: %d", -ret);
> +        return;
> +    }
> +
> +    ret = k->set_guest_notifiers(qbus->parent, vub->vhost_dev.nvqs, true);
> +    if (ret < 0) {
> +        error_report("Error binding guest notifier: %d", -ret);
> +        goto err_host_notifiers;
> +    }
> +
> +    vub->vhost_dev.acked_features = vdev->guest_features;
> +
> +    ret = vhost_dev_start(&vub->vhost_dev, vdev, true);
> +    if (ret < 0) {
> +        error_report("Error starting vhost-user-base: %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 < vub->vhost_dev.nvqs; i++) {
> +        vhost_virtqueue_mask(&vub->vhost_dev, vdev, i, false);
> +    }
> +
> +    return;
> +
> +err_guest_notifiers:
> +    k->set_guest_notifiers(qbus->parent, vub->vhost_dev.nvqs, false);
> +err_host_notifiers:
> +    vhost_dev_disable_notifiers(&vub->vhost_dev, vdev);
> +}
> +
> +static void vub_stop(VirtIODevice *vdev)
> +{
> +    VHostUserBase *vub = VHOST_USER_BASE(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(&vub->vhost_dev, vdev, true);
> +
> +    ret = k->set_guest_notifiers(qbus->parent, vub->vhost_dev.nvqs, false);
> +    if (ret < 0) {
> +        error_report("vhost guest notifier cleanup failed: %d", ret);
> +        return;
> +    }
> +
> +    vhost_dev_disable_notifiers(&vub->vhost_dev, vdev);
> +}
> +
> +static void vub_set_status(VirtIODevice *vdev, uint8_t status)
> +{
> +    VHostUserBase *vub = VHOST_USER_BASE(vdev);
> +    bool should_start = virtio_device_should_start(vdev, status);
> +
> +    if (vhost_dev_is_started(&vub->vhost_dev) == should_start) {
> +        return;
> +    }
> +
> +    if (should_start) {
> +        vub_start(vdev);
> +    } else {
> +        vub_stop(vdev);
> +    }
> +}
> +
> +/*
> + * For an implementation where everything is delegated to the backend
> + * we don't do anything other than return the full feature set offered
> + * by the daemon (module the reserved feature bit).
> + */
> +static uint64_t vub_get_features(VirtIODevice *vdev,
> +                                 uint64_t requested_features, Error **errp)
> +{
> +    VHostUserBase *vub = VHOST_USER_BASE(vdev);
> +    /* This should be set when the vhost connection initialises */
> +    g_assert(vub->vhost_dev.features);
> +    return vub->vhost_dev.features & ~(1ULL << VHOST_USER_F_PROTOCOL_FEATURES);
> +}
> +
> +/*
> + * To handle VirtIO config we need to know the size of the config
> + * space. We don't cache the config but re-fetch it from the guest
> + * every time in case something has changed.
> + */
> +static void vub_get_config(VirtIODevice *vdev, uint8_t *config)
> +{
> +    VHostUserBase *vub = VHOST_USER_BASE(vdev);
> +    Error *local_err = NULL;
> +
> +    /*
> +     * There will have been a warning during vhost_dev_init, but lets
> +     * assert here as nothing will go right now.
> +     */
> +    g_assert(vub->config_size && vub->vhost_user.supports_config == true);
> +
> +    if (vhost_dev_get_config(&vub->vhost_dev, config,
> +                             vub->config_size, &local_err)) {
> +        error_report_err(local_err);
> +    }
> +}
> +
> +/*
> + * When the daemon signals an update to the config we just need to
> + * signal the guest as we re-read the config on demand above.
> + */
> +static int vub_config_notifier(struct vhost_dev *dev)
> +{
> +    virtio_notify_config(dev->vdev);
> +    return 0;
> +}
> +
> +const VhostDevConfigOps vub_config_ops = {
> +    .vhost_dev_config_notifier = vub_config_notifier,
> +};
> +
> +static void vub_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 do_vhost_user_cleanup(VirtIODevice *vdev, VHostUserBase *vub)
> +{
> +    vhost_user_cleanup(&vub->vhost_user);
> +
> +    for (int i = 0; i < vub->num_vqs; i++) {
> +        VirtQueue *vq = g_ptr_array_index(vub->vqs, i);
> +        virtio_delete_queue(vq);
> +    }
> +
> +    virtio_cleanup(vdev);
> +}
> +
> +static int vub_connect(DeviceState *dev)
> +{
> +    VirtIODevice *vdev = VIRTIO_DEVICE(dev);
> +    VHostUserBase *vub = VHOST_USER_BASE(vdev);
> +    struct vhost_dev *vhost_dev = &vub->vhost_dev;
> +
> +    if (vub->connected) {
> +        return 0;
> +    }
> +    vub->connected = true;
> +
> +    /*
> +     * If we support VHOST_USER_GET_CONFIG we must enable the notifier
> +     * so we can ping the guest when it updates.
> +     */
> +    if (vub->vhost_user.supports_config) {
> +        vhost_dev_set_config_notifier(vhost_dev, &vub_config_ops);
> +    }
> +
> +    /* restore vhost state */
> +    if (virtio_device_started(vdev, vdev->status)) {
> +        vub_start(vdev);
> +    }
> +
> +    return 0;
> +}
> +
> +static void vub_disconnect(DeviceState *dev)
> +{
> +    VirtIODevice *vdev = VIRTIO_DEVICE(dev);
> +    VHostUserBase *vub = VHOST_USER_BASE(vdev);
> +
> +    if (!vub->connected) {
> +        return;
> +    }
> +    vub->connected = false;
> +
> +    if (vhost_dev_is_started(&vub->vhost_dev)) {
> +        vub_stop(vdev);
> +    }
> +}
> +
> +static void vub_event(void *opaque, QEMUChrEvent event)
> +{
> +    DeviceState *dev = opaque;
> +    VirtIODevice *vdev = VIRTIO_DEVICE(dev);
> +    VHostUserBase *vub = VHOST_USER_BASE(vdev);
> +
> +    switch (event) {
> +    case CHR_EVENT_OPENED:
> +        if (vub_connect(dev) < 0) {
> +            qemu_chr_fe_disconnect(&vub->chardev);
> +            return;
> +        }
> +        break;
> +    case CHR_EVENT_CLOSED:
> +        vub_disconnect(dev);
> +        break;
> +    case CHR_EVENT_BREAK:
> +    case CHR_EVENT_MUX_IN:
> +    case CHR_EVENT_MUX_OUT:
> +        /* Ignore */
> +        break;
> +    }
> +}
> +
> +static void vub_device_realize(DeviceState *dev, Error **errp)
> +{
> +    VirtIODevice *vdev = VIRTIO_DEVICE(dev);
> +    VHostUserBase *vub = VHOST_USER_BASE(dev);
> +    int ret;
> +
> +    if (!vub->chardev.chr) {
> +        error_setg(errp, "vhost-user-base: missing chardev");
> +        return;
> +    }
> +
> +    if (!vub->virtio_id) {
> +        error_setg(errp, "vhost-user-base: need to define device id");
> +        return;
> +    }
> +
> +    if (!vub->num_vqs) {
> +        vub->num_vqs = 1; /* reasonable default? */
> +    }
> +
> +    if (!vub->vq_size) {
> +        vub->vq_size = 64;
> +    }
> +
> +    /*
> +     * We can't handle config requests unless we know the size of the
> +     * config region, specialisations of the vhost-user-base will be
> +     * able to set this.
> +     */
> +    if (vub->config_size) {
> +        vub->vhost_user.supports_config = true;
> +    }
> +
> +    if (!vhost_user_init(&vub->vhost_user, &vub->chardev, errp)) {
> +        return;
> +    }
> +
> +    virtio_init(vdev, vub->virtio_id, vub->config_size);
> +
> +    /*
> +     * Disable guest notifiers, by default all notifications will be via the
> +     * asynchronous vhost-user socket.
> +     */
> +    vdev->use_guest_notifier_mask = false;
> +
> +    /* Allocate queues */
> +    vub->vqs = g_ptr_array_sized_new(vub->num_vqs);
> +    for (int i = 0; i < vub->num_vqs; i++) {
> +        g_ptr_array_add(vub->vqs,
> +                        virtio_add_queue(vdev, vub->vq_size, vub_handle_output));
> +    }
> +
> +    vub->vhost_dev.nvqs = vub->num_vqs;
> +    vub->vhost_dev.vqs = g_new0(struct vhost_virtqueue, vub->vhost_dev.nvqs);
> +
> +    /* connect to backend */
> +    ret = vhost_dev_init(&vub->vhost_dev, &vub->vhost_user,
> +                         VHOST_BACKEND_TYPE_USER, 0, errp);
> +
> +    if (ret < 0) {
> +        do_vhost_user_cleanup(vdev, vub);
> +    }
> +
> +    qemu_chr_fe_set_handlers(&vub->chardev, NULL, NULL, vub_event, NULL,
> +                             dev, NULL, true);
> +}
> +
> +static void vub_device_unrealize(DeviceState *dev)
> +{
> +    VirtIODevice *vdev = VIRTIO_DEVICE(dev);
> +    VHostUserBase *vub = VHOST_USER_BASE(dev);
> +    struct vhost_virtqueue *vhost_vqs = vub->vhost_dev.vqs;
> +
> +    /* This will stop vhost backend if appropriate. */
> +    vub_set_status(vdev, 0);
> +    vhost_dev_cleanup(&vub->vhost_dev);
> +    g_free(vhost_vqs);
> +    do_vhost_user_cleanup(vdev, vub);
> +}
> +
> +static void vub_class_init(ObjectClass *klass, void *data)
> +{
> +    VirtioDeviceClass *vdc = VIRTIO_DEVICE_CLASS(klass);
> +
> +    vdc->realize = vub_device_realize;
> +    vdc->unrealize = vub_device_unrealize;
> +    vdc->get_features = vub_get_features;
> +    vdc->get_config = vub_get_config;
> +    vdc->set_status = vub_set_status;
> +}
> +
> +static const TypeInfo vub_info = {
> +    .name = TYPE_VHOST_USER_BASE,
> +    .parent = TYPE_VIRTIO_DEVICE,
> +    .instance_size = sizeof(VHostUserBase),
> +    .class_init = vub_class_init,
> +    .class_size = sizeof(VHostUserBaseClass),
> +    .abstract = true
> +};
> +
> +static void vu_register_types(void)
> +{
> +    type_register_static(&vub_info);
> +}
> +
> +type_init(vu_register_types)
> diff --git a/hw/virtio/vhost-user-device-pci.c b/hw/virtio/vhost-user-device-pci.c
> index 41f9b7905b..22a5e95b9c 100644
> --- a/hw/virtio/vhost-user-device-pci.c
> +++ b/hw/virtio/vhost-user-device-pci.c
> @@ -9,21 +9,18 @@
>   
>   #include "qemu/osdep.h"
>   #include "hw/qdev-properties.h"
> -#include "hw/virtio/vhost-user-device.h"
> +#include "hw/virtio/vhost-user-base.h"
>   #include "hw/virtio/virtio-pci.h"
>   
>   struct VHostUserDevicePCI {
>       VirtIOPCIProxy parent_obj;
> +
>       VHostUserBase vub;
>   };
>   
> -typedef struct VHostUserDevicePCI VHostUserDevicePCI;
> -
>   #define TYPE_VHOST_USER_DEVICE_PCI "vhost-user-device-pci-base"
>   
> -DECLARE_INSTANCE_CHECKER(VHostUserDevicePCI,
> -                         VHOST_USER_DEVICE_PCI,
> -                         TYPE_VHOST_USER_DEVICE_PCI)
> +OBJECT_DECLARE_SIMPLE_TYPE(VHostUserDevicePCI, VHOST_USER_DEVICE_PCI)
>   
>   static void vhost_user_device_pci_realize(VirtIOPCIProxy *vpci_dev, Error **errp)
>   {
> @@ -39,6 +36,7 @@ static void vhost_user_device_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_device_pci_realize;
>       set_bit(DEVICE_CATEGORY_INPUT, dc->categories);
>       pcidev_k->vendor_id = PCI_VENDOR_ID_REDHAT_QUMRANET;
> diff --git a/hw/virtio/vhost-user-device.c b/hw/virtio/vhost-user-device.c
> index 2b028cae08..4adc1e8991 100644
> --- a/hw/virtio/vhost-user-device.c
> +++ b/hw/virtio/vhost-user-device.c
> @@ -1,7 +1,10 @@
>   /*
> - * Generic vhost-user stub. This can be used to connect to any
> - * vhost-user backend. All configuration details must be handled by
> - * the vhost-user daemon itself
> + * Generic vhost-user-device implementation for any vhost-user-backend
> + *
> + * This is a concrete implementation of vhost-user-base which can be
> + * configured via properties. It is useful for development and
> + * prototyping. It expects configuration details (if any) to be
> + * handled by the vhost-user daemon itself.
>    *
>    * Copyright (c) 2023 Linaro Ltd
>    * Author: Alex Bennée <alex.bennee@linaro.org>
> @@ -13,329 +16,9 @@
>   #include "qapi/error.h"
>   #include "hw/qdev-properties.h"
>   #include "hw/virtio/virtio-bus.h"
> -#include "hw/virtio/vhost-user-device.h"
> +#include "hw/virtio/vhost-user-base.h"
>   #include "qemu/error-report.h"
>   
> -static void vub_start(VirtIODevice *vdev)
> -{
> -    BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(vdev)));
> -    VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
> -    VHostUserBase *vub = VHOST_USER_BASE(vdev);
> -    int ret, i;
> -
> -    if (!k->set_guest_notifiers) {
> -        error_report("binding does not support guest notifiers");
> -        return;
> -    }
> -
> -    ret = vhost_dev_enable_notifiers(&vub->vhost_dev, vdev);
> -    if (ret < 0) {
> -        error_report("Error enabling host notifiers: %d", -ret);
> -        return;
> -    }
> -
> -    ret = k->set_guest_notifiers(qbus->parent, vub->vhost_dev.nvqs, true);
> -    if (ret < 0) {
> -        error_report("Error binding guest notifier: %d", -ret);
> -        goto err_host_notifiers;
> -    }
> -
> -    vub->vhost_dev.acked_features = vdev->guest_features;
> -
> -    ret = vhost_dev_start(&vub->vhost_dev, vdev, true);
> -    if (ret < 0) {
> -        error_report("Error starting vhost-user-device: %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 < vub->vhost_dev.nvqs; i++) {
> -        vhost_virtqueue_mask(&vub->vhost_dev, vdev, i, false);
> -    }
> -
> -    return;
> -
> -err_guest_notifiers:
> -    k->set_guest_notifiers(qbus->parent, vub->vhost_dev.nvqs, false);
> -err_host_notifiers:
> -    vhost_dev_disable_notifiers(&vub->vhost_dev, vdev);
> -}
> -
> -static void vub_stop(VirtIODevice *vdev)
> -{
> -    VHostUserBase *vub = VHOST_USER_BASE(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(&vub->vhost_dev, vdev, true);
> -
> -    ret = k->set_guest_notifiers(qbus->parent, vub->vhost_dev.nvqs, false);
> -    if (ret < 0) {
> -        error_report("vhost guest notifier cleanup failed: %d", ret);
> -        return;
> -    }
> -
> -    vhost_dev_disable_notifiers(&vub->vhost_dev, vdev);
> -}
> -
> -static void vub_set_status(VirtIODevice *vdev, uint8_t status)
> -{
> -    VHostUserBase *vub = VHOST_USER_BASE(vdev);
> -    bool should_start = virtio_device_should_start(vdev, status);
> -
> -    if (vhost_dev_is_started(&vub->vhost_dev) == should_start) {
> -        return;
> -    }
> -
> -    if (should_start) {
> -        vub_start(vdev);
> -    } else {
> -        vub_stop(vdev);
> -    }
> -}
> -
> -/*
> - * For an implementation where everything is delegated to the backend
> - * we don't do anything other than return the full feature set offered
> - * by the daemon (module the reserved feature bit).
> - */
> -static uint64_t vub_get_features(VirtIODevice *vdev,
> -                                 uint64_t requested_features, Error **errp)
> -{
> -    VHostUserBase *vub = VHOST_USER_BASE(vdev);
> -    /* This should be set when the vhost connection initialises */
> -    g_assert(vub->vhost_dev.features);
> -    return vub->vhost_dev.features & ~(1ULL << VHOST_USER_F_PROTOCOL_FEATURES);
> -}
> -
> -/*
> - * To handle VirtIO config we need to know the size of the config
> - * space. We don't cache the config but re-fetch it from the guest
> - * every time in case something has changed.
> - */
> -static void vub_get_config(VirtIODevice *vdev, uint8_t *config)
> -{
> -    VHostUserBase *vub = VHOST_USER_BASE(vdev);
> -    Error *local_err = NULL;
> -
> -    /*
> -     * There will have been a warning during vhost_dev_init, but lets
> -     * assert here as nothing will go right now.
> -     */
> -    g_assert(vub->config_size && vub->vhost_user.supports_config == true);
> -
> -    if (vhost_dev_get_config(&vub->vhost_dev, config,
> -                             vub->config_size, &local_err)) {
> -        error_report_err(local_err);
> -    }
> -}
> -
> -/*
> - * When the daemon signals an update to the config we just need to
> - * signal the guest as we re-read the config on demand above.
> - */
> -static int vub_config_notifier(struct vhost_dev *dev)
> -{
> -    virtio_notify_config(dev->vdev);
> -    return 0;
> -}
> -
> -const VhostDevConfigOps vub_config_ops = {
> -    .vhost_dev_config_notifier = vub_config_notifier,
> -};
> -
> -static void vub_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 do_vhost_user_cleanup(VirtIODevice *vdev, VHostUserBase *vub)
> -{
> -    vhost_user_cleanup(&vub->vhost_user);
> -
> -    for (int i = 0; i < vub->num_vqs; i++) {
> -        VirtQueue *vq = g_ptr_array_index(vub->vqs, i);
> -        virtio_delete_queue(vq);
> -    }
> -
> -    virtio_cleanup(vdev);
> -}
> -
> -static int vub_connect(DeviceState *dev)
> -{
> -    VirtIODevice *vdev = VIRTIO_DEVICE(dev);
> -    VHostUserBase *vub = VHOST_USER_BASE(vdev);
> -    struct vhost_dev *vhost_dev = &vub->vhost_dev;
> -
> -    if (vub->connected) {
> -        return 0;
> -    }
> -    vub->connected = true;
> -
> -    /*
> -     * If we support VHOST_USER_GET_CONFIG we must enable the notifier
> -     * so we can ping the guest when it updates.
> -     */
> -    if (vub->vhost_user.supports_config) {
> -        vhost_dev_set_config_notifier(vhost_dev, &vub_config_ops);
> -    }
> -
> -    /* restore vhost state */
> -    if (virtio_device_started(vdev, vdev->status)) {
> -        vub_start(vdev);
> -    }
> -
> -    return 0;
> -}
> -
> -static void vub_disconnect(DeviceState *dev)
> -{
> -    VirtIODevice *vdev = VIRTIO_DEVICE(dev);
> -    VHostUserBase *vub = VHOST_USER_BASE(vdev);
> -
> -    if (!vub->connected) {
> -        return;
> -    }
> -    vub->connected = false;
> -
> -    if (vhost_dev_is_started(&vub->vhost_dev)) {
> -        vub_stop(vdev);
> -    }
> -}
> -
> -static void vub_event(void *opaque, QEMUChrEvent event)
> -{
> -    DeviceState *dev = opaque;
> -    VirtIODevice *vdev = VIRTIO_DEVICE(dev);
> -    VHostUserBase *vub = VHOST_USER_BASE(vdev);
> -
> -    switch (event) {
> -    case CHR_EVENT_OPENED:
> -        if (vub_connect(dev) < 0) {
> -            qemu_chr_fe_disconnect(&vub->chardev);
> -            return;
> -        }
> -        break;
> -    case CHR_EVENT_CLOSED:
> -        vub_disconnect(dev);
> -        break;
> -    case CHR_EVENT_BREAK:
> -    case CHR_EVENT_MUX_IN:
> -    case CHR_EVENT_MUX_OUT:
> -        /* Ignore */
> -        break;
> -    }
> -}
> -
> -static void vub_device_realize(DeviceState *dev, Error **errp)
> -{
> -    VirtIODevice *vdev = VIRTIO_DEVICE(dev);
> -    VHostUserBase *vub = VHOST_USER_BASE(dev);
> -    int ret;
> -
> -    if (!vub->chardev.chr) {
> -        error_setg(errp, "vhost-user-device: missing chardev");
> -        return;
> -    }
> -
> -    if (!vub->virtio_id) {
> -        error_setg(errp, "vhost-user-device: need to define device id");
> -        return;
> -    }
> -
> -    if (!vub->num_vqs) {
> -        vub->num_vqs = 1; /* reasonable default? */
> -    }
> -
> -    /*
> -     * We can't handle config requests unless we know the size of the
> -     * config region, specialisations of the vhost-user-device will be
> -     * able to set this.
> -     */
> -    if (vub->config_size) {
> -        vub->vhost_user.supports_config = true;
> -    }
> -
> -    if (!vhost_user_init(&vub->vhost_user, &vub->chardev, errp)) {
> -        return;
> -    }
> -
> -    virtio_init(vdev, vub->virtio_id, vub->config_size);
> -
> -    /*
> -     * Disable guest notifiers, by default all notifications will be via the
> -     * asynchronous vhost-user socket.
> -     */
> -    vdev->use_guest_notifier_mask = false;
> -
> -    /* Allocate queues */
> -    vub->vqs = g_ptr_array_sized_new(vub->num_vqs);
> -    for (int i = 0; i < vub->num_vqs; i++) {
> -        g_ptr_array_add(vub->vqs,
> -                        virtio_add_queue(vdev, 4, vub_handle_output));
> -    }
> -
> -    vub->vhost_dev.nvqs = vub->num_vqs;
> -    vub->vhost_dev.vqs = g_new0(struct vhost_virtqueue, vub->vhost_dev.nvqs);
> -
> -    /* connect to backend */
> -    ret = vhost_dev_init(&vub->vhost_dev, &vub->vhost_user,
> -                         VHOST_BACKEND_TYPE_USER, 0, errp);
> -
> -    if (ret < 0) {
> -        do_vhost_user_cleanup(vdev, vub);
> -    }
> -
> -    qemu_chr_fe_set_handlers(&vub->chardev, NULL, NULL, vub_event, NULL,
> -                             dev, NULL, true);
> -}
> -
> -static void vub_device_unrealize(DeviceState *dev)
> -{
> -    VirtIODevice *vdev = VIRTIO_DEVICE(dev);
> -    VHostUserBase *vub = VHOST_USER_BASE(dev);
> -    struct vhost_virtqueue *vhost_vqs = vub->vhost_dev.vqs;
> -
> -    /* This will stop vhost backend if appropriate. */
> -    vub_set_status(vdev, 0);
> -    vhost_dev_cleanup(&vub->vhost_dev);
> -    g_free(vhost_vqs);
> -    do_vhost_user_cleanup(vdev, vub);
> -}
> -
> -static void vub_class_init(ObjectClass *klass, void *data)
> -{
> -    VirtioDeviceClass *vdc = VIRTIO_DEVICE_CLASS(klass);
> -
> -    vdc->realize = vub_device_realize;
> -    vdc->unrealize = vub_device_unrealize;
> -    vdc->get_features = vub_get_features;
> -    vdc->get_config = vub_get_config;
> -    vdc->set_status = vub_set_status;
> -}
> -
> -static const TypeInfo vub_info = {
> -    .name = TYPE_VHOST_USER_BASE,
> -    .parent = TYPE_VIRTIO_DEVICE,
> -    .instance_size = sizeof(VHostUserBase),
> -    .class_init = vub_class_init,
> -    .class_size = sizeof(VHostUserBaseClass),
> -    .abstract = true
> -};
> -
> -
>   /*
>    * The following is a concrete implementation of the base class which
>    * allows the user to define the key parameters via the command line.
> @@ -349,6 +32,7 @@ static const VMStateDescription vud_vmstate = {
>   static Property vud_properties[] = {
>       DEFINE_PROP_CHR("chardev", VHostUserBase, chardev),
>       DEFINE_PROP_UINT16("virtio-id", VHostUserBase, virtio_id, 0),
> +    DEFINE_PROP_UINT32("vq_size", VHostUserBase, vq_size, 64),
>       DEFINE_PROP_UINT32("num_vqs", VHostUserBase, num_vqs, 1),
>       DEFINE_PROP_UINT32("config_size", VHostUserBase, config_size, 0),
>       DEFINE_PROP_END_OF_LIST(),
> @@ -366,14 +50,11 @@ static void vud_class_init(ObjectClass *klass, void *data)
>   static const TypeInfo vud_info = {
>       .name = TYPE_VHOST_USER_DEVICE,
>       .parent = TYPE_VHOST_USER_BASE,
> -    .instance_size = sizeof(VHostUserBase),
>       .class_init = vud_class_init,
> -    .class_size = sizeof(VHostUserBaseClass),
>   };
>   
>   static void vu_register_types(void)
>   {
> -    type_register_static(&vub_info);
>       type_register_static(&vud_info);
>   }

And if you're touching this part here, it's worth updating to use the DEFINE_TYPES() 
macro instead.

> diff --git a/hw/virtio/meson.build b/hw/virtio/meson.build
> index c0055a7832..51c3f97c2d 100644
> --- a/hw/virtio/meson.build
> +++ b/hw/virtio/meson.build
> @@ -17,6 +17,7 @@ if have_vhost
>     if have_vhost_user
>       # fixme - this really should be generic
>       specific_virtio_ss.add(files('vhost-user.c'))
> +    system_virtio_ss.add(files('vhost-user-base.c'))
>       system_virtio_ss.add(files('vhost-user-device.c'))
>       system_virtio_ss.add(when: 'CONFIG_VIRTIO_PCI', if_true: files('vhost-user-device-pci.c'))
>     endif


ATB,

Mark.


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

* Re: [Virtio-fs] [PATCH v4 0/6] virtio: cleanup vhost-user-generic and reduce c&p
  2023-10-09  9:59 [Virtio-fs] [PATCH v4 0/6] virtio: cleanup vhost-user-generic and reduce c&p Alex Bennée
                   ` (5 preceding siblings ...)
  2023-10-09  9:59 ` [Virtio-fs] [PATCH v4 6/6] docs/system: add a basic enumeration of vhost-user devices Alex Bennée
@ 2023-10-10 15:14 ` Mark Cave-Ayland
  6 siblings, 0 replies; 14+ messages in thread
From: Mark Cave-Ayland @ 2023-10-10 15:14 UTC (permalink / raw)
  To: Alex Bennée, qemu-devel
  Cc: Marc-André Lureau, Erik Schilling, Fam Zheng,
	Mathieu Poirier, Gerd Hoffmann, Hanna Reitz, Stefan Hajnoczi,
	Kevin Wolf, Eric Blake, Michael S. Tsirkin, qemu-block,
	Eduardo Habkost, Paolo Bonzini, Viresh Kumar, virtio-fs,
	Gonglei (Arei),
	Markus Armbruster, Jason Wang, Raphael Norwitz,
	Daniel P. Berrangé

On 09/10/2023 10:59, Alex Bennée wrote:

> A lot of our vhost-user stubs are large chunks of boilerplate that do
> (mostly) the same thing. This series continues the cleanups by
> splitting the vhost-user-base and vhost-user-generic implementations.
> After adding a new vq_size property the rng, gpio and i2c vhost-user
> devices become simple specialisations of the common base defining the
> ID, number of queues and potentially the config handling.
> 
> I've also added Manos' vhost-user-sound while I was at it.
> 
> Changes
> -------
> 
> I've dropped the F_TRANSPORT work from this series to keep this small
> and ready to merge. The changes for F_TRANSPORT are a bit more
> invasive and still need a bit of debugging but I wanted to get this
> stuff merged now.
> 
> Alex Bennée (5):
>    virtio: split into vhost-user-base and vhost-user-device
>    hw/virtio: derive vhost-user-rng from vhost-user-base
>    hw/virtio: derive vhost-user-gpio from vhost-user-base
>    hw/virtio: derive vhost-user-i2c from vhost-user-base
>    docs/system: add a basic enumeration of vhost-user devices
> 
> Manos Pitsidianakis (1):
>    hw/virtio: add vhost-user-snd and virtio-snd-pci devices
> 
>   docs/system/devices/vhost-user-rng.rst |   2 +
>   docs/system/devices/vhost-user.rst     |  41 +++
>   include/hw/virtio/vhost-user-base.h    |  49 +++
>   include/hw/virtio/vhost-user-gpio.h    |  23 +-
>   include/hw/virtio/vhost-user-i2c.h     |  14 +-
>   include/hw/virtio/vhost-user-rng.h     |  11 +-
>   include/hw/virtio/vhost-user-snd.h     |  26 ++
>   hw/virtio/vhost-user-base.c            | 348 +++++++++++++++++++++
>   hw/virtio/vhost-user-device-pci.c      |  10 +-
>   hw/virtio/vhost-user-device.c          | 335 +-------------------
>   hw/virtio/vhost-user-gpio.c            | 407 ++-----------------------
>   hw/virtio/vhost-user-i2c.c             | 272 +----------------
>   hw/virtio/vhost-user-rng.c             | 278 ++---------------
>   hw/virtio/vhost-user-snd-pci.c         |  75 +++++
>   hw/virtio/vhost-user-snd.c             |  67 ++++
>   hw/virtio/Kconfig                      |   5 +
>   hw/virtio/meson.build                  |  25 +-
>   17 files changed, 705 insertions(+), 1283 deletions(-)
>   create mode 100644 include/hw/virtio/vhost-user-base.h
>   create mode 100644 include/hw/virtio/vhost-user-snd.h
>   create mode 100644 hw/virtio/vhost-user-base.c
>   create mode 100644 hw/virtio/vhost-user-snd-pci.c
>   create mode 100644 hw/virtio/vhost-user-snd.c

Hi Alex,

 From a quick skim the QOM modelling looks good to me. The only thing I've noticed is 
that the parent object for VHostUserBase derived classes is sometimes named as 
"VHostUserBase parent" instead of "VHostUserBase parent_obj" as per the QOM naming 
guidelines.


ATB,

Mark.


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

end of thread, other threads:[~2023-10-10 15:14 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-10-09  9:59 [Virtio-fs] [PATCH v4 0/6] virtio: cleanup vhost-user-generic and reduce c&p Alex Bennée
2023-10-09  9:59 ` [Virtio-fs] [PATCH v4 1/6] virtio: split into vhost-user-base and vhost-user-device Alex Bennée
2023-10-10 15:08   ` Mark Cave-Ayland
2023-10-09  9:59 ` [Virtio-fs] [PATCH v4 2/6] hw/virtio: derive vhost-user-rng from vhost-user-base Alex Bennée
2023-10-09 13:07   ` Manos Pitsidianakis
2023-10-09 13:48     ` Alex Bennée
2023-10-09  9:59 ` [Virtio-fs] [PATCH v4 3/6] hw/virtio: derive vhost-user-gpio " Alex Bennée
2023-10-09 10:59   ` Viresh Kumar
2023-10-09  9:59 ` [Virtio-fs] [PATCH v4 4/6] hw/virtio: derive vhost-user-i2c " Alex Bennée
2023-10-09 10:59   ` Viresh Kumar
2023-10-09  9:59 ` [Virtio-fs] [PATCH v4 5/6] hw/virtio: add vhost-user-snd and virtio-snd-pci devices Alex Bennée
2023-10-09  9:59 ` [Virtio-fs] [PATCH v4 6/6] docs/system: add a basic enumeration of vhost-user devices Alex Bennée
2023-10-09 13:38   ` Manos Pitsidianakis
2023-10-10 15:14 ` [Virtio-fs] [PATCH v4 0/6] virtio: cleanup vhost-user-generic and reduce c&p Mark Cave-Ayland

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).