Re: [PATCH 1/3] virtio_pci_modern: Remove unnecessary num zero check

["Michael S. Tsirkin" <mst@redhat.com>]

On Wed, Mar 08, 2023 at 01:52:43PM +0800, Jason Wang wrote:
> On Tue, Mar 7, 2023 at 11:57 AM Feng Liu <feliu@nvidia.com> wrote:
> >
> > is_power_of_2() already performs the zero check. Hence avoid duplicate
> > check. While at it, move the query of size check also adjacent to where
> > its used for the disabled vq.
> >
> > Signed-off-by: Feng Liu <feliu@nvidia.com>
> > Reviewed-by: Jiri Pirko <jiri@nvidia.com>
> > Reviewed-by: Parav Pandit <parav@nvidia.com>
> > Reviewed-by: Gavin Li <gavinl@nvidia.com>
> > Reviewed-by: Bodong Wang <bodong@nvidia.com>
> > ---
> >  drivers/virtio/virtio_pci_modern.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/virtio/virtio_pci_modern.c b/drivers/virtio/virtio_pci_modern.c
> > index 9e496e288cfa..3d7144f8f959 100644
> > --- a/drivers/virtio/virtio_pci_modern.c
> > +++ b/drivers/virtio/virtio_pci_modern.c
> > @@ -306,10 +306,10 @@ static struct virtqueue *setup_vq(struct virtio_pci_device *vp_dev,
> >                 return ERR_PTR(-EINVAL);
> >
> >         /* Check if queue is either not available or already active. */
> > -       num = vp_modern_get_queue_size(mdev, index);
> > -       if (!num || vp_modern_get_queue_enable(mdev, index))
> > +       if (vp_modern_get_queue_enable(mdev, index))
> >                 return ERR_PTR(-ENOENT);
> 
> Spec allows non power of 2 size for packed virtqueue, so I think we
> should fix it in this way.
> 
> """
> Queue Size corresponds to the maximum number of descriptors in the
> virtqueue5. The Queue Size value does not have to be a power of 2.
> """
> 
> Thanks

Oh yack. How come we never noticed :(

And it's been like this from day 1, too.


Let's fix this and backport everywhere we can.

> >
> > +       num = vp_modern_get_queue_size(mdev, index);
> >         if (!is_power_of_2(num)) {
> >                 dev_warn(&vp_dev->pci_dev->dev, "bad queue size %u", num);
> >                 return ERR_PTR(-EINVAL);
> > --
> > 2.34.1
> >

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization