[PATCH v4 0/4] Add queue ready message to VDUSE

[PATCH v4 0/4] Add queue ready message to VDUSE

[Eugenio Pérez]

This series introduces a new VDUSE message for VDUSE userland instance
to detect when a VirtQueue (VQ) is enabled, replacing the polling.

VirtIO net devices' dataplane is started after the control virtqueue so
QEMU can apply the configuration in the destination of a Live Migration.
Without this feature, the VDUSE instance must poll the VQs to check when
(and if) a VQ has been enabled.

This series also implements VDUSE feature flags allowing the VDUSE
devices to opt-in to the VQ ready message.  Devices that opt-in to this
feature will receive explicit notifications when a VQ is ready.  Devices
that do not set this flag remain unaffected, ensuring backward
compatibility without indefinitely incrementing API versions.

The VDUSE features is a 64 bit bitmap for simplicity, the same way as
vhost and vhost-net started.  It can be extended as a flexible array of
bits when we reach so many features, but it seems unlikely at this
point.

Error cases tested:
* Call VDUSE_GET_FEATURES without get the API VERSION (so API == 0) and
  with API VERSION set to 1 with VDUSE_SET_API_VERSION (-EINVAL returned
  from VDUSE_GET_FEATURES ioctl).
* Try to set invalid features.
* Test regular initialization of single queue devices with and without
  VDUSE_F_QUEUE_READY set.
* Test expected behavior when VDUSE userland instance returns
  VDUSE_REQ_RESULT_FAILED from VDUSE_SET_VQ_READY message.
* Repeat all the tests with multiqueue devices, by reverting
  56e71885b0349 ("vduse: Temporarily fail if control queue feature requested").

v4:
* Create its own spinlock for vq->ready instead of relying on kick and
  call spinlocks.
* Add VDUSE_SET_FEATURES ioctl and the corresponding validation paths instead
  of using device config.

v3:
* Remove API_VERSION bump to 2
* Add comment about struct vduse_dev_config:vduse_features is only valid
  if VDUSE_GET_FEATURES success.

v2:
* Fix comment of vduse_dev_request.vq_ready
* Set vq_ready before sending the message to the VDUSE userland
  instance, avoiding the need for SMP sync after receiving the message.
* Return -EINVAL if control ioctl called with version < 2, so userland
  visible reply is kept (Jason).

Eugenio Pérez (4):
  vduse: store control device pointer
  vduse: add VDUSE_GET_FEATURES ioctl
  vduse: add VDUSE_SET_FEATURES ioctl
  vduse: add F_QUEUE_READY feature

 drivers/vdpa/vdpa_user/vduse_dev.c | 111 +++++++++++++++++++++++------
 include/uapi/linux/vduse.h         |  24 +++++++
 2 files changed, 115 insertions(+), 20 deletions(-)

-- 
2.55.0

[PATCH v4 1/4] vduse: store control device pointer

[Eugenio Pérez]

This helps log the errors in next patches.  The alternative is to
perform a linear search for it with class_find_device_by_devt(class, devt),
as device_destroy do for cleaning.

Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
---
v4: Set control device pointer to NULL at device create error.
---
 drivers/vdpa/vdpa_user/vduse_dev.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c b/drivers/vdpa/vdpa_user/vduse_dev.c
index f15ad425e01f..2af5cb5e1ec6 100644
--- a/drivers/vdpa/vdpa_user/vduse_dev.c
+++ b/drivers/vdpa/vdpa_user/vduse_dev.c
@@ -164,6 +164,7 @@ static DEFINE_IDR(vduse_idr);
 
 static dev_t vduse_major;
 static struct cdev vduse_ctrl_cdev;
+static const struct device *vduse_ctrl_dev;
 static struct cdev vduse_cdev;
 static struct workqueue_struct *vduse_irq_wq;
 static struct workqueue_struct *vduse_irq_bound_wq;
@@ -2532,7 +2533,6 @@ static void vduse_mgmtdev_exit(void)
 static int vduse_init(void)
 {
 	int ret;
-	struct device *dev;
 
 	ret = class_register(&vduse_class);
 	if (ret)
@@ -2549,9 +2549,10 @@ static int vduse_init(void)
 	if (ret)
 		goto err_ctrl_cdev;
 
-	dev = device_create(&vduse_class, NULL, vduse_major, NULL, "control");
-	if (IS_ERR(dev)) {
-		ret = PTR_ERR(dev);
+	vduse_ctrl_dev = device_create(&vduse_class, NULL, vduse_major, NULL, "control");
+	if (IS_ERR(vduse_ctrl_dev)) {
+		ret = PTR_ERR(vduse_ctrl_dev);
+		vduse_ctrl_dev = NULL;
 		goto err_device;
 	}
 
-- 
2.55.0

[PATCH v4 2/4] vduse: add VDUSE_GET_FEATURES ioctl

[Eugenio Pérez]

Add an ioctl to allow VDUSE instances to query the available features
supported by the kernel module.

Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
---
v4:
* Store the features in VDUSE control instead of vduse device config.

v3:
* Remove check for API_VERSION < 2
* Add comment about struct vduse_dev_config:vduse_features is only valid
  if VDUSE_GET_FEATURES success.

v2:
* return -EINVAL if ioctl called with version < 2, so userland visible
  reply is kept (Jason).
---
 drivers/vdpa/vdpa_user/vduse_dev.c | 7 +++++++
 include/uapi/linux/vduse.h         | 3 +++
 2 files changed, 10 insertions(+)

diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c b/drivers/vdpa/vdpa_user/vduse_dev.c
index 2af5cb5e1ec6..e1930e38df7a 100644
--- a/drivers/vdpa/vdpa_user/vduse_dev.c
+++ b/drivers/vdpa/vdpa_user/vduse_dev.c
@@ -52,6 +52,9 @@
 
 #define IRQ_UNBOUND -1
 
+/* Supported VDUSE features */
+static const uint64_t vduse_features;
+
 /*
  * VDUSE instance have not asked the vduse API version, so assume 0.
  *
@@ -2343,6 +2346,10 @@ static long vduse_ioctl(struct file *file, unsigned int cmd,
 		ret = vduse_destroy_dev(name);
 		break;
 	}
+	case VDUSE_GET_FEATURES:
+		ret = put_user(vduse_features, (u64 __user *)argp);
+		break;
+
 	default:
 		ret = -EINVAL;
 		break;
diff --git a/include/uapi/linux/vduse.h b/include/uapi/linux/vduse.h
index 361eea511c21..89aa3b448c0a 100644
--- a/include/uapi/linux/vduse.h
+++ b/include/uapi/linux/vduse.h
@@ -63,6 +63,9 @@ struct vduse_dev_config {
  */
 #define VDUSE_DESTROY_DEV	_IOW(VDUSE_BASE, 0x03, char[VDUSE_NAME_MAX])
 
+/* Get the VDUSE supported features */
+#define VDUSE_GET_FEATURES	_IOR(VDUSE_BASE, 0x04, __u64)
+
 /* The ioctls for VDUSE device (/dev/vduse/$NAME) */
 
 /**
-- 
2.55.0

[PATCH v4 3/4] vduse: add VDUSE_SET_FEATURES ioctl

[Eugenio Pérez]

Add an ioctl to allow VDUSE instances to set the VDUSE features
supported by the userland VDUSE instance.

Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
---
v4: New in v4.
---
 drivers/vdpa/vdpa_user/vduse_dev.c | 24 ++++++++++++++++++++++++
 include/uapi/linux/vduse.h         |  3 +++
 2 files changed, 27 insertions(+)

diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c b/drivers/vdpa/vdpa_user/vduse_dev.c
index e1930e38df7a..4d561b9fbf5e 100644
--- a/drivers/vdpa/vdpa_user/vduse_dev.c
+++ b/drivers/vdpa/vdpa_user/vduse_dev.c
@@ -160,6 +160,7 @@ struct vduse_dev_msg {
 
 struct vduse_control {
 	u64 api_version;
+	u64 vduse_features;
 };
 
 static DEFINE_MUTEX(vduse_lock);
@@ -2349,7 +2350,29 @@ static long vduse_ioctl(struct file *file, unsigned int cmd,
 	case VDUSE_GET_FEATURES:
 		ret = put_user(vduse_features, (u64 __user *)argp);
 		break;
+	case VDUSE_SET_FEATURES: {
+		u64 features;
 
+		ret = -EFAULT;
+		if (get_user(features, (u64 __user *)argp)) {
+			dev_dbg(vduse_ctrl_dev, "Could not get vduse features");
+			break;
+		}
+
+		ret = -EINVAL;
+		if (features & ~vduse_features) {
+			dev_dbg(vduse_ctrl_dev,
+				"Invalid features in %llx, expected %llx",
+				features, vduse_features);
+			break;
+		}
+
+		ret = 0;
+		control->vduse_features = features;
+		dev_dbg(vduse_ctrl_dev, "Set features %llx", features);
+
+		break;
+	}
 	default:
 		ret = -EINVAL;
 		break;
@@ -2376,6 +2399,7 @@ static int vduse_open(struct inode *inode, struct file *file)
 		return -ENOMEM;
 
 	control->api_version = VDUSE_API_VERSION_NOT_ASKED;
+	control->vduse_features = 0;
 	file->private_data = control;
 
 	return 0;
diff --git a/include/uapi/linux/vduse.h b/include/uapi/linux/vduse.h
index 89aa3b448c0a..f14c965bb7f6 100644
--- a/include/uapi/linux/vduse.h
+++ b/include/uapi/linux/vduse.h
@@ -66,6 +66,9 @@ struct vduse_dev_config {
 /* Get the VDUSE supported features */
 #define VDUSE_GET_FEATURES	_IOR(VDUSE_BASE, 0x04, __u64)
 
+/* Set the VDUSE features */
+#define VDUSE_SET_FEATURES	_IOW(VDUSE_BASE, 0x05, __u64)
+
 /* The ioctls for VDUSE device (/dev/vduse/$NAME) */
 
 /**
-- 
2.55.0

[PATCH v4 4/4] vduse: add F_QUEUE_READY feature

[Eugenio Pérez]

Add the VDUSE_F_QUEUE_READY feature flag. This allows the kernel module
to explicitly signal userspace when a specific virtqueue has been
enabled.

In scenarios like Live Migration of VirtIO net devices, the dataplane
starts after the control virtqueue allowing QEMU to apply configuration
in the destination device.

Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
---
v4:
* Create its own spinlock for vq->ready instead of relying on kick and
  call spinlocks.

v2:
* Fix comment of vduse_dev_request.vq_ready
* Set vq_ready before sending the message to the VDUSE userland
  instance, avoiding the need for SMP sync after receiving the message.
---
 drivers/vdpa/vdpa_user/vduse_dev.c | 73 +++++++++++++++++++++++-------
 include/uapi/linux/vduse.h         | 18 ++++++++
 2 files changed, 74 insertions(+), 17 deletions(-)

diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c b/drivers/vdpa/vdpa_user/vduse_dev.c
index 4d561b9fbf5e..307d13fbe942 100644
--- a/drivers/vdpa/vdpa_user/vduse_dev.c
+++ b/drivers/vdpa/vdpa_user/vduse_dev.c
@@ -9,6 +9,7 @@
  */
 
 #include "linux/virtio_net.h"
+#include <linux/bits.h>
 #include <linux/cleanup.h>
 #include <linux/init.h>
 #include <linux/module.h>
@@ -53,7 +54,7 @@
 #define IRQ_UNBOUND -1
 
 /* Supported VDUSE features */
-static const uint64_t vduse_features;
+static const uint64_t vduse_features = BIT_U64(VDUSE_F_QUEUE_READY);
 
 /*
  * VDUSE instance have not asked the vduse API version, so assume 0.
@@ -77,6 +78,7 @@ struct vduse_virtqueue {
 	u32 group;
 	spinlock_t kick_lock;
 	spinlock_t irq_lock;
+	spinlock_t ready_lock;
 	struct eventfd_ctx *kickfd;
 	struct vdpa_callback cb;
 	struct work_struct inject;
@@ -120,6 +122,7 @@ struct vduse_dev {
 	char *name;
 	struct mutex lock;
 	spinlock_t msg_lock;
+	u64 vduse_features;
 	u64 msg_unique;
 	u32 msg_timeout;
 	wait_queue_head_t waitq;
@@ -514,7 +517,9 @@ static void vduse_dev_reset(struct vduse_dev *dev)
 	for (i = 0; i < dev->vq_num; i++) {
 		struct vduse_virtqueue *vq = dev->vqs[i];
 
-		vq->ready = false;
+		scoped_guard(spinlock_bh, &vq->ready_lock) {
+			vq->ready = false;
+		}
 		vq->desc_addr = 0;
 		vq->driver_addr = 0;
 		vq->device_addr = 0;
@@ -556,16 +561,15 @@ static int vduse_vdpa_set_vq_address(struct vdpa_device *vdpa, u16 idx,
 
 static void vduse_vq_kick(struct vduse_virtqueue *vq)
 {
-	spin_lock(&vq->kick_lock);
-	if (!vq->ready)
-		goto unlock;
+	guard(spinlock)(&vq->kick_lock);
+	scoped_guard(spinlock_bh, &vq->ready_lock)
+		if (!vq->ready)
+			return;
 
 	if (vq->kickfd)
 		eventfd_signal(vq->kickfd);
 	else
 		vq->kicked = true;
-unlock:
-	spin_unlock(&vq->kick_lock);
 }
 
 static void vduse_vq_kick_work(struct work_struct *work)
@@ -625,7 +629,30 @@ static void vduse_vdpa_set_vq_ready(struct vdpa_device *vdpa,
 {
 	struct vduse_dev *dev = vdpa_to_vduse(vdpa);
 	struct vduse_virtqueue *vq = dev->vqs[idx];
+	struct vduse_dev_msg msg = { 0 };
+	int r;
+
+	if (dev->vduse_features & BIT_U64(VDUSE_F_QUEUE_READY)) {
+		msg.req.type = VDUSE_SET_VQ_READY;
+		msg.req.vq_ready.num = idx;
+		msg.req.vq_ready.ready = !!ready;
+
+		r = vduse_dev_msg_sync(dev, &msg);
+
+		if (r < 0) {
+			dev_dbg(&vdpa->dev, "device refuses to set vq %u ready %u",
+					idx, ready);
 
+			/* We can't do better than break the device in this case */
+			spin_lock(&dev->msg_lock);
+			vduse_dev_broken(dev);
+			spin_unlock(&dev->msg_lock);
+
+			return;
+		}
+	}
+
+	guard(spinlock_bh)(&vq->ready_lock);
 	vq->ready = ready;
 }
 
@@ -634,6 +661,7 @@ static bool vduse_vdpa_get_vq_ready(struct vdpa_device *vdpa, u16 idx)
 	struct vduse_dev *dev = vdpa_to_vduse(vdpa);
 	struct vduse_virtqueue *vq = dev->vqs[idx];
 
+	guard(spinlock_bh)(&vq->ready_lock);
 	return vq->ready;
 }
 
@@ -1121,15 +1149,16 @@ static int vduse_kickfd_setup(struct vduse_dev *dev,
 	} else if (eventfd->fd != VDUSE_EVENTFD_DEASSIGN)
 		return 0;
 
-	spin_lock(&vq->kick_lock);
+	guard(spinlock)(&vq->kick_lock);
 	if (vq->kickfd)
 		eventfd_ctx_put(vq->kickfd);
 	vq->kickfd = ctx;
+
+	guard(spinlock_bh)(&vq->ready_lock);
 	if (vq->ready && vq->kicked && vq->kickfd) {
 		eventfd_signal(vq->kickfd);
 		vq->kicked = false;
 	}
-	spin_unlock(&vq->kick_lock);
 
 	return 0;
 }
@@ -1160,10 +1189,10 @@ static void vduse_vq_irq_inject(struct work_struct *work)
 	struct vduse_virtqueue *vq = container_of(work,
 					struct vduse_virtqueue, inject);
 
-	spin_lock_bh(&vq->irq_lock);
+	guard(spinlock_bh)(&vq->irq_lock);
+	guard(spinlock_bh)(&vq->ready_lock);
 	if (vq->ready && vq->cb.callback)
 		vq->cb.callback(vq->cb.private);
-	spin_unlock_bh(&vq->irq_lock);
 }
 
 static bool vduse_vq_signal_irqfd(struct vduse_virtqueue *vq)
@@ -1173,12 +1202,12 @@ static bool vduse_vq_signal_irqfd(struct vduse_virtqueue *vq)
 	if (!vq->cb.trigger)
 		return false;
 
-	spin_lock_irq(&vq->irq_lock);
+	guard(spinlock_irq)(&vq->irq_lock);
+	guard(spinlock_irq)(&vq->ready_lock);
 	if (vq->ready && vq->cb.trigger) {
 		eventfd_signal(vq->cb.trigger);
 		signal = true;
 	}
-	spin_unlock_irq(&vq->irq_lock);
 
 	return signal;
 }
@@ -1516,7 +1545,9 @@ static long vduse_dev_ioctl(struct file *file, unsigned int cmd,
 			vq_info.split.avail_index =
 				vq->state.split.avail_index;
 
-		vq_info.ready = vq->ready;
+		scoped_guard(spinlock_bh, &vq->ready_lock) {
+			vq_info.ready = vq->ready;
+		}
 
 		ret = -EFAULT;
 		if (copy_to_user(argp, &vq_info, sizeof(vq_info)))
@@ -1746,7 +1777,9 @@ static long vduse_dev_compat_ioctl(struct file *file, unsigned int cmd,
 			vq_info.split.avail_index =
 				vq->state.split.avail_index;
 
-		vq_info.ready = vq->ready;
+		scoped_guard(spinlock_bh, &vq->ready_lock) {
+			vq_info.ready = vq->ready;
+		}
 
 		ret = -EFAULT;
 		if (copy_to_user(argp, &vq_info,
@@ -1959,6 +1992,7 @@ static int vduse_dev_init_vqs(struct vduse_dev *dev, u32 vq_align, u32 vq_num)
 		INIT_WORK(&dev->vqs[i]->kick, vduse_vq_kick_work);
 		spin_lock_init(&dev->vqs[i]->kick_lock);
 		spin_lock_init(&dev->vqs[i]->irq_lock);
+		spin_lock_init(&dev->vqs[i]->ready_lock);
 		cpumask_setall(&dev->vqs[i]->irq_affinity);
 
 		kobject_init(&dev->vqs[i]->kobj, &vq_type);
@@ -2194,7 +2228,8 @@ static struct attribute *vduse_dev_attrs[] = {
 ATTRIBUTE_GROUPS(vduse_dev);
 
 static int vduse_create_dev(struct vduse_dev_config *config,
-			    void *config_buf, u64 api_version)
+			    void *config_buf, u64 api_version,
+			    uint64_t vduse_features)
 {
 	int ret;
 	struct vduse_dev *dev;
@@ -2216,6 +2251,9 @@ static int vduse_create_dev(struct vduse_dev_config *config,
 	dev->device_features = config->features;
 	dev->device_id = config->device_id;
 	dev->vendor_id = config->vendor_id;
+	dev->vduse_features = vduse_features;
+	dev_dbg(vduse_ctrl_dev, "Creating device %s with features 0x%llx",
+		config->name, vduse_features);
 
 	dev->nas = (dev->api_version < VDUSE_API_VERSION_1) ? 1 : config->nas;
 	dev->as = kzalloc_objs(dev->as[0], dev->nas);
@@ -2331,7 +2369,8 @@ static long vduse_ioctl(struct file *file, unsigned int cmd,
 			break;
 		}
 		config.name[VDUSE_NAME_MAX - 1] = '\0';
-		ret = vduse_create_dev(&config, buf, control->api_version);
+		ret = vduse_create_dev(&config, buf, control->api_version,
+				       control->vduse_features);
 		if (ret)
 			kvfree(buf);
 		break;
diff --git a/include/uapi/linux/vduse.h b/include/uapi/linux/vduse.h
index f14c965bb7f6..7285f8570237 100644
--- a/include/uapi/linux/vduse.h
+++ b/include/uapi/linux/vduse.h
@@ -14,6 +14,9 @@
 
 #define VDUSE_API_VERSION_1	1
 
+/* The VDUSE instance expects a request for vq ready */
+#define VDUSE_F_QUEUE_READY	0
+
 /*
  * Get the version of VDUSE API that kernel supported (VDUSE_API_VERSION).
  * This is used for future extension.
@@ -331,6 +334,7 @@ enum vduse_req_type {
 	VDUSE_SET_STATUS,
 	VDUSE_UPDATE_IOTLB,
 	VDUSE_SET_VQ_GROUP_ASID,
+	VDUSE_SET_VQ_READY,
 };
 
 /**
@@ -378,6 +382,15 @@ struct vduse_iova_range_v2 {
 	__u32 padding;
 };
 
+/**
+ * struct vduse_vq_ready - Virtqueue ready request message
+ * @num: Virtqueue number
+ */
+struct vduse_vq_ready {
+	__u32 num;
+	__u32 ready;
+};
+
 /**
  * struct vduse_dev_request - control request
  * @type: request type
@@ -388,6 +401,7 @@ struct vduse_iova_range_v2 {
  * @iova: IOVA range for updating
  * @iova_v2: IOVA range for updating if API_VERSION >= 1
  * @vq_group_asid: ASID of a virtqueue group
+ * @vq_ready: Virtqueue ready request
  * @padding: padding
  *
  * Structure used by read(2) on /dev/vduse/$NAME.
@@ -405,6 +419,10 @@ struct vduse_dev_request {
 		 */
 		struct vduse_iova_range_v2 iova_v2;
 		struct vduse_vq_group_asid vq_group_asid;
+
+		/* Only if VDUSE_F_QUEUE_READY is negotiated */
+		struct vduse_vq_ready vq_ready;
+
 		__u32 padding[32];
 	};
 };
-- 
2.55.0