Linux Virtualization - Apr 2023 - [PATCH v6] virtio: add VIRTIO_F_NOTIFICATION_DATA feature support

Hi Viktor,
> diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
> index 4c3bb0ddeb9b..f9c6604352b4 100644
> --- a/drivers/virtio/virtio_ring.c
> +++ b/drivers/virtio/virtio_ring.c
> @@ -2752,6 +2752,23 @@ void vring_del_virtqueue(struct virtqueue *_vq)
>  }
>  EXPORT_SYMBOL_GPL(vring_del_virtqueue);
> 
> +u32 vring_notification_data(struct virtqueue *_vq)
> +{
> +       struct vring_virtqueue *vq = to_vvq(_vq);
> +       u16 next;
> +
> +       if (vq->packed_ring)
> +               next = (vq->packed.next_avail_idx &
> +                               ~(-(1 <<
VRING_PACKED_EVENT_F_WRAP_CTR))) |
> +                       vq->packed.avail_wrap_counter <<
> +                               VRING_PACKED_EVENT_F_WRAP_CTR;
> +       else
> +               next = vq->split.avail_idx_shadow;
> +
> +       return next << 16 | _vq->index;
> +}
> +EXPORT_SYMBOL_GPL(vring_notification_data);
> +
>  /* Manipulates transport-specific feature bits. */
>  void vring_transport_features(struct virtio_device *vdev)
>  {
> @@ -2771,6 +2788,8 @@ void vring_transport_features(struct virtio_device
*vdev)
>                         break;
>                 case VIRTIO_F_ORDER_PLATFORM:
>                         break;
> +               case VIRTIO_F_NOTIFICATION_DATA:
> +                       break;
This function is used by virtio_vdpa as well
(drivers/virtio/virtio_vdpa.c:virtio_vdpa_finalize_features).
A vDPA device can offer this feature and it will be accepted, even though
VIRTIO_F_NOTIFICATION_DATA is not a thing for the vDPA transport at the moment.

I don't know if this is bad, since offering VIRTIO_F_NOTIFICATION_DATA is
meaningless for a vDPA device at the moment.

I submitted a patch adding support for vDPA transport.
https://lore.kernel.org/virtualization/20230402081034.1021886-1-alvaro.karsz at
solid-run.com/T/#u
>                 default:
>                         /* We don't understand this bit. */
>                         __virtio_clear_bit(vdev, i);

On Sun, Apr 02, 2023 at 08:17:49AM +0000, Alvaro Karsz
wrote:
> Hi Viktor,
> 
> > diff --git a/drivers/virtio/virtio_ring.c
b/drivers/virtio/virtio_ring.c
> > index 4c3bb0ddeb9b..f9c6604352b4 100644
> > --- a/drivers/virtio/virtio_ring.c
> > +++ b/drivers/virtio/virtio_ring.c
> > @@ -2752,6 +2752,23 @@ void vring_del_virtqueue(struct virtqueue *_vq)
> >  }
> >  EXPORT_SYMBOL_GPL(vring_del_virtqueue);
> > 
> > +u32 vring_notification_data(struct virtqueue *_vq)
> > +{
> > +       struct vring_virtqueue *vq = to_vvq(_vq);
> > +       u16 next;
> > +
> > +       if (vq->packed_ring)
> > +               next = (vq->packed.next_avail_idx &
> > +                               ~(-(1 <<
VRING_PACKED_EVENT_F_WRAP_CTR))) |
> > +                       vq->packed.avail_wrap_counter <<
> > +                               VRING_PACKED_EVENT_F_WRAP_CTR;
> > +       else
> > +               next = vq->split.avail_idx_shadow;
> > +
> > +       return next << 16 | _vq->index;
> > +}
> > +EXPORT_SYMBOL_GPL(vring_notification_data);
> > +
> >  /* Manipulates transport-specific feature bits. */
> >  void vring_transport_features(struct virtio_device *vdev)
> >  {
> > @@ -2771,6 +2788,8 @@ void vring_transport_features(struct
virtio_device *vdev)
> >                         break;
> >                 case VIRTIO_F_ORDER_PLATFORM:
> >                         break;
> > +               case VIRTIO_F_NOTIFICATION_DATA:
> > +                       break;
> 
> This function is used by virtio_vdpa as well
(drivers/virtio/virtio_vdpa.c:virtio_vdpa_finalize_features).
> A vDPA device can offer this feature and it will be accepted, even though
VIRTIO_F_NOTIFICATION_DATA is not a thing for the vDPA transport at the moment.
> 
> I don't know if this is bad, since offering VIRTIO_F_NOTIFICATION_DATA
is meaningless for a vDPA device at the moment.
> 
> I submitted a patch adding support for vDPA transport.
>
https://lore.kernel.org/virtualization/20230402081034.1021886-1-alvaro.karsz at
solid-run.com/T/#u
Hmm.  So it seems we need to first apply yours then this patch,
is that right? Or the other way around? What is the right way to make it not
break bisect?
Do you mind including this patch with yours in a patchset
in the correct order?



> >                 default:
> >                         /* We don't understand this bit. */
> >                         __virtio_clear_bit(vdev, i);
>