Re: [PATCH 3/7] vdpa/mlx5: Support configuring max data virtqueue pairs

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 12/2/2021 11:31 PM, Parav Pandit wrote:

From: Si-Wei Liu <si-wei.liu@xxxxxxxxxx>
Sent: Friday, December 3, 2021 12:58 PM

On 12/1/2021 11:57 AM, Eli Cohen wrote:
Check whether the max number of data virtqueue pairs was provided when
a adding a new device and verify the new value does not exceed device
capabilities.

In addition, change the arrays holding virtqueue and callback contexts
to be dynamically allocated.

Signed-off-by: Eli Cohen <elic@xxxxxxxxxx>
---
   drivers/vdpa/mlx5/net/mlx5_vnet.c | 33 ++++++++++++++++++++-----------
   1 file changed, 21 insertions(+), 12 deletions(-)

diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c
b/drivers/vdpa/mlx5/net/mlx5_vnet.c
index b66f41ccbac2..62aba5dbd8fa 100644
--- a/drivers/vdpa/mlx5/net/mlx5_vnet.c
+++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c
@@ -131,11 +131,6 @@ struct mlx5_vdpa_virtqueue {
   	struct mlx5_vq_restore_info ri;
   };

-/* We will remove this limitation once mlx5_vdpa_alloc_resources()
- * provides for driver space allocation
Is this comment outdated, i.e. driver space allocation in
mlx5_vdpa_alloc_resources() already provided?

- */
-#define MLX5_MAX_SUPPORTED_VQS 16
-
   static bool is_index_valid(struct mlx5_vdpa_dev *mvdev, u16 idx)
   {
   	if (unlikely(idx > mvdev->max_idx)) @@ -148,8 +143,8 @@ struct
mlx5_vdpa_net {
   	struct mlx5_vdpa_dev mvdev;
   	struct mlx5_vdpa_net_resources res;
   	struct virtio_net_config config;
-	struct mlx5_vdpa_virtqueue vqs[MLX5_MAX_SUPPORTED_VQS];
-	struct vdpa_callback event_cbs[MLX5_MAX_SUPPORTED_VQS + 1];
+	struct mlx5_vdpa_virtqueue *vqs;
+	struct vdpa_callback *event_cbs;

   	/* Serialize vq resources creation and destruction. This is required
   	 * since memory map might change and we need to destroy and create
@@ -1218,7 +1213,7 @@ static void suspend_vqs(struct mlx5_vdpa_net
*ndev)
   {
   	int i;

-	for (i = 0; i < MLX5_MAX_SUPPORTED_VQS; i++)
+	for (i = 0; i < ndev->mvdev.max_vqs; i++)
   		suspend_vq(ndev, &ndev->vqs[i]);
   }

@@ -1245,7 +1240,7 @@ static int create_rqt(struct mlx5_vdpa_net *ndev)
   	int i, j;
   	int err;

-	max_rqt = min_t(int, MLX5_MAX_SUPPORTED_VQS / 2,
+	max_rqt = min_t(int, ndev->mvdev.max_vqs  / 2,
   			1 << MLX5_CAP_GEN(ndev->mvdev.mdev,
log_max_rqt_size));
   	if (max_rqt < 1)
   		return -EOPNOTSUPP;
@@ -2235,7 +2230,7 @@ static int mlx5_vdpa_reset(struct vdpa_device
*vdev)
   	clear_vqs_ready(ndev);
   	mlx5_vdpa_destroy_mr(&ndev->mvdev);
   	ndev->mvdev.status = 0;
-	memset(ndev->event_cbs, 0, sizeof(ndev->event_cbs));
+	memset(ndev->event_cbs, 0, sizeof(*ndev->event_cbs) *
+(mvdev->max_vqs + 1));
   	ndev->mvdev.actual_features = 0;
   	++mvdev->generation;
   	if (MLX5_CAP_GEN(mvdev->mdev, umem_uid_0)) { @@ -2308,6
+2303,8 @@
static void mlx5_vdpa_free(struct vdpa_device *vdev)
   	}
   	mlx5_vdpa_free_resources(&ndev->mvdev);
   	mutex_destroy(&ndev->reslock);
+	kfree(ndev->event_cbs);
+	kfree(ndev->vqs);
   }

   static struct vdpa_notification_area mlx5_get_vq_notification(struct
vdpa_device *vdev, u16 idx) @@ -2547,13 +2544,24 @@ static int
mlx5_vdpa_dev_add(struct vdpa_mgmt_dev *v_mdev, const char *name,

   	/* we save one virtqueue for control virtqueue should we require it */
   	max_vqs = MLX5_CAP_DEV_VDPA_EMULATION(mdev,
max_num_virtio_queues);
-	max_vqs = min_t(u32, max_vqs, MLX5_MAX_SUPPORTED_VQS);
+	if (add_config->mask &
BIT_ULL(VDPA_ATTR_DEV_NET_CFG_MAX_VQP)) {
+		if (add_config->max_vq_pairs & (add_config->max_vq_pairs -
1) ||
+		    add_config->max_vq_pairs > max_vqs / 2)
+			return -EINVAL;
It'd be the best to describe the failing cause here, the power of 2 limitation is
known to me, but not friendly enough for first time user.
dev_warn would work for me.
User commands shouldn't be creating dmsg unwanted messages.
dev_warn_once() is better.
I said it works for me because there's already a dev_warn() instance in the same function.

But instead, extack error message should be returned that reaches the user who issues iproute2 command and makes user aware better.
Yep, extack's more preferred than dev_warn_once and dev_warn.

-Siwei

_______________________________________________
Virtualization mailing list
Virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linuxfoundation.org/mailman/listinfo/virtualization



[Index of Archives]     [KVM Development]     [Libvirt Development]     [Libvirt Users]     [CentOS Virtualization]     [Netdev]     [Ethernet Bridging]     [Linux Wireless]     [Kernel Newbies]     [Security]     [Linux for Hams]     [Netfilter]     [Bugtraq]     [Yosemite Forum]     [MIPS Linux]     [ARM Linux]     [Linux RAID]     [Linux Admin]     [Samba]

  Powered by Linux