Virtualization - Jan 2022 - [PATCH 2/3] vdpa/mlx5: set_features should nack MQ if no CTRL_VQ

On Thu, Jan 13, 2022 at 4:09 PM Eli Cohen <elic at nvidia.com>
wrote:
>
> On Thu, Jan 13, 2022 at 12:10:50AM -0500, Si-Wei Liu wrote:
> > 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);
>
> I would not expect this kind check to be enforced in vhost_vdpa and
> apply to all drivers.
We want to make vhost_vdpa type agnostic to make it simple and clean.
So there's no type specific code there. So my understanding is that,
if this is the mandate behavior of the device, it needs to be done at
device level (mlx5_vdpa) right now.

Thanks
>
> > +
> >       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
> >
>