On Wed, Jan 12, 2022 at 11:12 AM Si-Wei Liu <si-wei.liu@xxxxxxxxxx> wrote:
>
>
>
> On 1/11/2022 6:29 PM, Jason Wang wrote:
> > On Wed, Jan 12, 2022 at 6:15 AM Si-Wei Liu <si-wei.liu@xxxxxxxxxx> 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)?
> > So the upper layer is unaware of the device type. It's better to do
> > that in mlx5's set_features()
> That'll be fine. I thought the upper layer can be made device type aware
> and consolidate it to common library routines avoiding duplicated code
> in every individual driver of the same type.
This is possible but not implemented so far. It looks more like an
intermediate layer between vdpa and it's parent.
> If this is against the goal
> of making vdpa core device type agnostic, then it's perhaps not needed.
Yes, that's the goal when developing vDPA core.
Thanks
>
> -Siwei
>
> > according to the spec:
> >
> > The device MUST NOT offer a feature which requires another feature
> > which was not offered.
> >
> > Thanks
> >
> >> 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
