This patchset contains the fixes for a few issues uncovered during the
review for the "Allow for configuring max number of virtqueue pairs"
series.
It is based on Eli's fixes:
2e4cda633a22 ("vdpa/mlx5: Fix tracking of current number of VQs")
in the vhost tree.
Si-Wei Liu (3):
vdpa: factor out vdpa_set_features_unlocked for vdpa internal use
vdpa/mlx5: set_features should nack MQ if no CTRL_VQ
vdpa/mlx5: validate the queue pair value from driver
drivers/vdpa/mlx5/net/mlx5_vnet.c | 26 +++++++++++++++++++++++---
drivers/vdpa/vdpa.c | 2 +-
drivers/vhost/vdpa.c | 2 +-
drivers/virtio/virtio_vdpa.c | 2 +-
include/linux/vdpa.h | 18 ++++++++++++------
5 files changed, 38 insertions(+), 12 deletions(-)
--
1.8.3.1
Si-Wei Liu
2022-Jan-13 05:10 UTC
[PATCH 1/3] vdpa: factor out vdpa_set_features_unlocked for vdpa internal use
No functional change introduced. vdpa bus driver such as virtio_vdpa
or vhost_vdpa is not supposed to take care of the locking for core
by its own. The locked API vdpa_set_features should suffice the
bus driver's need.
Signed-off-by: Si-Wei Liu<si-wei.liu at oracle.com>
---
drivers/vdpa/vdpa.c | 2 +-
drivers/vhost/vdpa.c | 2 +-
drivers/virtio/virtio_vdpa.c | 2 +-
include/linux/vdpa.h | 18 ++++++++++++------
4 files changed, 15 insertions(+), 9 deletions(-)
diff --git a/drivers/vdpa/vdpa.c b/drivers/vdpa/vdpa.c
index 9846c9d..1ea5254 100644
--- a/drivers/vdpa/vdpa.c
+++ b/drivers/vdpa/vdpa.c
@@ -393,7 +393,7 @@ static void vdpa_get_config_unlocked(struct vdpa_device
*vdev,
* If it does happen we assume a legacy guest.
*/
if (!vdev->features_valid)
- vdpa_set_features(vdev, 0, true);
+ vdpa_set_features_unlocked(vdev, 0);
ops->get_config(vdev, offset, buf, len);
}
diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c
index 8515398..ec5249e 100644
--- a/drivers/vhost/vdpa.c
+++ b/drivers/vhost/vdpa.c
@@ -286,7 +286,7 @@ static long vhost_vdpa_set_features(struct vhost_vdpa *v,
u64 __user *featurep)
if (copy_from_user(&features, featurep, sizeof(features)))
return -EFAULT;
- if (vdpa_set_features(vdpa, features, false))
+ if (vdpa_set_features(vdpa, features))
return -EINVAL;
return 0;
diff --git a/drivers/virtio/virtio_vdpa.c b/drivers/virtio/virtio_vdpa.c
index 7767a7f..7650455 100644
--- a/drivers/virtio/virtio_vdpa.c
+++ b/drivers/virtio/virtio_vdpa.c
@@ -317,7 +317,7 @@ static int virtio_vdpa_finalize_features(struct
virtio_device *vdev)
/* Give virtio_ring a chance to accept features. */
vring_transport_features(vdev);
- return vdpa_set_features(vdpa, vdev->features, false);
+ return vdpa_set_features(vdpa, vdev->features);
}
static const char *virtio_vdpa_bus_name(struct virtio_device *vdev)
diff --git a/include/linux/vdpa.h b/include/linux/vdpa.h
index 2de442e..721089b 100644
--- a/include/linux/vdpa.h
+++ b/include/linux/vdpa.h
@@ -401,18 +401,24 @@ static inline int vdpa_reset(struct vdpa_device *vdev)
return ret;
}
-static inline int vdpa_set_features(struct vdpa_device *vdev, u64 features,
bool locked)
+static inline int vdpa_set_features_unlocked(struct vdpa_device *vdev, u64
features)
{
const struct vdpa_config_ops *ops = vdev->config;
int ret;
- if (!locked)
- mutex_lock(&vdev->cf_mutex);
-
vdev->features_valid = true;
ret = ops->set_driver_features(vdev, features);
- if (!locked)
- mutex_unlock(&vdev->cf_mutex);
+
+ return ret;
+}
+
+static inline int vdpa_set_features(struct vdpa_device *vdev, u64 features)
+{
+ int ret;
+
+ mutex_lock(&vdev->cf_mutex);
+ ret = vdpa_set_features_unlocked(vdev, features);
+ mutex_unlock(&vdev->cf_mutex);
return ret;
}
--
1.8.3.1
Si-Wei Liu
2022-Jan-13 05:10 UTC
[PATCH 2/3] vdpa/mlx5: set_features should nack MQ if no CTRL_VQ
Made corresponding change per spec:
The device MUST NOT offer a feature which requires another feature
which was not offered.
Fixes: 52893733f2c5 ("vdpa/mlx5: Add multiqueue support")
Signed-off-by: Si-Wei Liu<si-wei.liu at oracle.com>
---
drivers/vdpa/mlx5/net/mlx5_vnet.c | 16 +++++++++++++---
1 file changed, 13 insertions(+), 3 deletions(-)
diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c
b/drivers/vdpa/mlx5/net/mlx5_vnet.c
index b53603d..46d4deb 100644
--- a/drivers/vdpa/mlx5/net/mlx5_vnet.c
+++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c
@@ -1897,11 +1897,21 @@ static u64 mlx5_vdpa_get_device_features(struct
vdpa_device *vdev)
return ndev->mvdev.mlx_features;
}
-static int verify_min_features(struct mlx5_vdpa_dev *mvdev, u64 features)
+static int verify_driver_features(struct mlx5_vdpa_dev *mvdev, u64 *features)
{
- if (!(features & BIT_ULL(VIRTIO_F_ACCESS_PLATFORM)))
+ /* minimum features to expect */
+ if (!(*features & BIT_ULL(VIRTIO_F_ACCESS_PLATFORM)))
return -EOPNOTSUPP;
+ /* Double check features combination sent down by the driver.
+ * NACK invalid feature due to the absence of depended feature.
+ * Driver is expected to re-read the negotiated features once
+ * return from set_driver_features.
+ */
+ if ((*features & (BIT_ULL(VIRTIO_NET_F_MQ) |
BIT_ULL(VIRTIO_NET_F_CTRL_VQ))) =+ BIT_ULL(VIRTIO_NET_F_MQ))
+ *features &= ~BIT_ULL(VIRTIO_NET_F_MQ);
+
return 0;
}
@@ -1977,7 +1987,7 @@ static int mlx5_vdpa_set_driver_features(struct
vdpa_device *vdev, u64 features)
print_features(mvdev, features, true);
- err = verify_min_features(mvdev, features);
+ err = verify_driver_features(mvdev, &features);
if (err)
return err;
--
1.8.3.1
Si-Wei Liu
2022-Jan-13 05:10 UTC
[PATCH 3/3] vdpa/mlx5: validate the queue pair value from driver
Fixes: 52893733f2c5 ("vdpa/mlx5: Add multiqueue support")
Signed-off-by: Si-Wei Liu<si-wei.liu at oracle.com>
---
drivers/vdpa/mlx5/net/mlx5_vnet.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c
b/drivers/vdpa/mlx5/net/mlx5_vnet.c
index 46d4deb..491127f 100644
--- a/drivers/vdpa/mlx5/net/mlx5_vnet.c
+++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c
@@ -1563,11 +1563,21 @@ static virtio_net_ctrl_ack handle_ctrl_mq(struct
mlx5_vdpa_dev *mvdev, u8 cmd)
switch (cmd) {
case VIRTIO_NET_CTRL_MQ_VQ_PAIRS_SET:
+ /* This mq feature check aligns with pre-existing userspace implementation,
+ * although the spec doesn't mandate so.
+ */
+ if (!MLX5_FEATURE(mvdev, VIRTIO_NET_F_MQ))
+ break;
+
read = vringh_iov_pull_iotlb(&cvq->vring, &cvq->riov, (void
*)&mq, sizeof(mq));
if (read != sizeof(mq))
break;
newqps = mlx5vdpa16_to_cpu(mvdev, mq.virtqueue_pairs);
+ if (newqps < VIRTIO_NET_CTRL_MQ_VQ_PAIRS_MIN ||
+ newqps > mlx5_vdpa_max_qps(mvdev->max_vqs))
+ break;
+
if (ndev->cur_num_vqs == 2 * newqps) {
status = VIRTIO_NET_OK;
break;
--
1.8.3.1
Michael S. Tsirkin
2022-Jan-13 07:03 UTC
[PATCH 0/3] fixes for mlx5_vdpa multiqueue support
On Thu, Jan 13, 2022 at 12:10:48AM -0500, Si-Wei Liu wrote:> This patchset contains the fixes for a few issues uncovered during the > review for the "Allow for configuring max number of virtqueue pairs" > series. > > It is based on Eli's fixes: > 2e4cda633a22 ("vdpa/mlx5: Fix tracking of current number of VQs") > in the vhost tree.It's really cleanups more than fixes. I'm not sure about the code changes (the vdpa change looks ok, mlx5 ones need review by nvidia folks) but from documentation POV this patchset needs more work.> > Si-Wei Liu (3): > vdpa: factor out vdpa_set_features_unlocked for vdpa internal use > vdpa/mlx5: set_features should nack MQ if no CTRL_VQ > vdpa/mlx5: validate the queue pair value from driver > > drivers/vdpa/mlx5/net/mlx5_vnet.c | 26 +++++++++++++++++++++++--- > drivers/vdpa/vdpa.c | 2 +- > drivers/vhost/vdpa.c | 2 +- > drivers/virtio/virtio_vdpa.c | 2 +- > include/linux/vdpa.h | 18 ++++++++++++------ > 5 files changed, 38 insertions(+), 12 deletions(-) > > -- > 1.8.3.1