On 1/11/2022 10:37 PM, Eli Cohen wrote:
On Tue, Jan 11, 2022 at 02:14:47PM -0800, Si-Wei Liu wrote:
On 1/11/2022 10:34 AM, Eli Cohen wrote:
Modify the code such that ndev->cur_num_vqs better reflects the actual
number of data virtqueues. The value can be accurately realized after
features have been negotiated.
This is to prevent possible failures when modifying the RQT object if
the cur_num_vqs bears invalid value.
No issue was actually encountered but this also makes the code more
readable.
Fixes: c5a5cd3d3217 ("vdpa/mlx5: Support configuring max data virtqueue")
Signed-off-by: Eli Cohen <elic@xxxxxxxxxx>
---
drivers/vdpa/mlx5/net/mlx5_vnet.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c b/drivers/vdpa/mlx5/net/mlx5_vnet.c
index 9eacfdb48434..b53603d94082 100644
--- a/drivers/vdpa/mlx5/net/mlx5_vnet.c
+++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c
@@ -1246,8 +1246,7 @@ static int create_rqt(struct mlx5_vdpa_net *ndev)
if (!(ndev->mvdev.actual_features & BIT_ULL(VIRTIO_NET_F_MQ)))
num = 1;
else
- num = mlx5vdpa16_to_cpu(&ndev->mvdev,
- ndev->config.max_virtqueue_pairs);
+ num = ndev->cur_num_vqs / 2;
Nit: the if branch can be consolidated
max_rqt = min_t(int, roundup_pow_of_two(num),
1 << MLX5_CAP_GEN(ndev->mvdev.mdev, log_max_rqt_size));
@@ -1983,6 +1982,11 @@ static int mlx5_vdpa_set_driver_features(struct vdpa_device *vdev, u64 features)
return err;
ndev->mvdev.actual_features = features & ndev->mvdev.mlx_features;
+ if (ndev->mvdev.actual_features & BIT_ULL(VIRTIO_NET_F_MQ))
+ ndev->cur_num_vqs = 2 * mlx5vdpa16_to_cpu(mvdev, ndev->config.max_virtqueue_pairs);
Hmmm, not this patch, but there should've been validation done in the upper
layer to guarantee set_featuers() for VIRTIO_NET_F_MQ always comes with
VIRTIO_NET_F_CTRL_VQ. Maybe checking both: BIT_ULL(VIRTIO_NET_F_MQ) |
BIT_ULL(VIRTIO_NET_F_CTRL_VQ)?
Not sure I follow. It's valid to have only CTRL_VQ feature but not MQ.
In that case we should have only two data VQs but then cur_num_vqs would
be set to whatever was configured by the user which could more than 2.
In that case (CTRL_VQ && !MQ), it will end up with just two data VQs
(plus one control queue which is not accounted in 'ethtool -l')
negotiated. There should be some validation in handle_ctrl_mq to
disallow user from configuring more than 2 data VQs for that case. You
don't need to worry too much for that at the moment, I'll post a patch
following your series to fix it. Let's keep it simple for just checking
both BIT_ULL(VIRTIO_NET_F_MQ) | BIT_ULL(VIRTIO_NET_F_CTRL_VQ) together.
Thanks,
-Siwei
otherwise it looks good to me.
Reviewed-by: Si-Wei Liu<si-wei.liu@xxxxxxxxxx>
+ else
+ ndev->cur_num_vqs = 2;
+
update_cvq_info(mvdev);
return err;
}
@@ -2233,6 +2237,7 @@ static int mlx5_vdpa_reset(struct vdpa_device *vdev)
clear_vqs_ready(ndev);
mlx5_vdpa_destroy_mr(&ndev->mvdev);
ndev->mvdev.status = 0;
+ ndev->cur_num_vqs = 0;
memset(ndev->event_cbs, 0, sizeof(*ndev->event_cbs) * (mvdev->max_vqs + 1));
ndev->mvdev.actual_features = 0;
++mvdev->generation;
@@ -2641,9 +2646,8 @@ static int mlx5_vdpa_dev_add(struct vdpa_mgmt_dev *v_mdev, const char *name,
ndev->nb.notifier_call = event_handler;
mlx5_notifier_register(mdev, &ndev->nb);
- ndev->cur_num_vqs = 2 * mlx5_vdpa_max_qps(max_vqs);
mvdev->vdev.mdev = &mgtdev->mgtdev;
- err = _vdpa_register_device(&mvdev->vdev, ndev->cur_num_vqs + 1);
+ err = _vdpa_register_device(&mvdev->vdev, 2 * mlx5_vdpa_max_qps(max_vqs) + 1);
if (err)
goto err_reg;
_______________________________________________
Virtualization mailing list
Virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
