Re: [PATCH 2/3] vdpa/mlx5: set_features should nack MQ if no CTRL_VQ

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

 



On Thu, Jan 13, 2022 at 4:09 PM Eli Cohen <elic@xxxxxxxxxx> 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@xxxxxxxxxx>
> > ---
> >  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
> >
>

_______________________________________________
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