On Tue, Apr 11, 2023 at 01:25:53PM +0800, Jason Wang wrote: > On Sun, Apr 9, 2023 at 3:07 PM Alvaro Karsz <alvaro.karsz@xxxxxxxxxxxxx> wrote: > > > > Add VIRTIO_F_NOTIFICATION_DATA support for vDPA transport. > > If this feature is negotiated, the driver passes extra data when kicking > > a virtqueue. > > > > A device that offers this feature needs to implement the > > kick_vq_with_data callback. > > > > kick_vq_with_data receives the vDPA device and data. > > data includes: > > 16 bits vqn and 16 bits next available index for split virtqueues. > > 16 bits vqs, 15 least significant bits of next available index > > and 1 bit next_wrap for packed virtqueues. > > > > This patch follows a patch [1] by Viktor Prutyanov which adds support > > for the MMIO, channel I/O and modern PCI transports. > > > > This patch needs to be applied on top of Viktor's patch. > > > > [1] https://lore.kernel.org/lkml/20230324195029.2410503-1-viktor@xxxxxxxxxx/ > > > > Signed-off-by: Alvaro Karsz <alvaro.karsz@xxxxxxxxxxxxx> > > --- > > v2: > > - clear the feature bit if kick_vq_with_data is not implemented. > > - Fix kick_vq_with_data comment in include/linux/vdpa.h > > - Write in more detail about the extra data in the commit log > > > > drivers/virtio/virtio_vdpa.c | 23 +++++++++++++++++++++-- > > include/linux/vdpa.h | 9 +++++++++ > > 2 files changed, 30 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/virtio/virtio_vdpa.c b/drivers/virtio/virtio_vdpa.c > > index d7f5af62dda..737c1f36d32 100644 > > --- a/drivers/virtio/virtio_vdpa.c > > +++ b/drivers/virtio/virtio_vdpa.c > > @@ -112,6 +112,17 @@ static bool virtio_vdpa_notify(struct virtqueue *vq) > > return true; > > } > > > > +static bool virtio_vdpa_notify_with_data(struct virtqueue *vq) > > +{ > > + struct vdpa_device *vdpa = vd_get_vdpa(vq->vdev); > > + const struct vdpa_config_ops *ops = vdpa->config; > > + u32 data = vring_notification_data(vq); > > + > > + ops->kick_vq_with_data(vdpa, data); > > + > > + return true; > > +} > > + > > static irqreturn_t virtio_vdpa_config_cb(void *private) > > { > > struct virtio_vdpa_device *vd_dev = private; > > @@ -138,6 +149,7 @@ virtio_vdpa_setup_vq(struct virtio_device *vdev, unsigned int index, > > struct device *dma_dev; > > const struct vdpa_config_ops *ops = vdpa->config; > > struct virtio_vdpa_vq_info *info; > > + bool (*notify)(struct virtqueue *vq) = virtio_vdpa_notify; > > struct vdpa_callback cb; > > struct virtqueue *vq; > > u64 desc_addr, driver_addr, device_addr; > > @@ -154,6 +166,14 @@ virtio_vdpa_setup_vq(struct virtio_device *vdev, unsigned int index, > > if (index >= vdpa->nvqs) > > return ERR_PTR(-ENOENT); > > > > + /* We cannot accept VIRTIO_F_NOTIFICATION_DATA without kick_vq_with_data */ > > + if (__virtio_test_bit(vdev, VIRTIO_F_NOTIFICATION_DATA)) { > > + if (ops->kick_vq_with_data) > > + notify = virtio_vdpa_notify_with_data; > > + else > > If we do this, I wonder if we need to fail or at least warn in this case. > > Thanks Nope. Either device can work without VIRTIO_F_NOTIFICATION_DATA and then all is well or it can't and then it will fail FEATURES_OK. If drivers start failing when seeing new features we'll never be able to extend virtio again. Same for warns, some people run with panic on warn. > > + __virtio_clear_bit(vdev, VIRTIO_F_NOTIFICATION_DATA); > > + } > > + > > /* Queue shouldn't already be set up. */ > > if (ops->get_vq_ready(vdpa, index)) > > return ERR_PTR(-ENOENT); > > @@ -183,8 +203,7 @@ virtio_vdpa_setup_vq(struct virtio_device *vdev, unsigned int index, > > dma_dev = vdpa_get_dma_dev(vdpa); > > vq = vring_create_virtqueue_dma(index, max_num, align, vdev, > > true, may_reduce_num, ctx, > > - virtio_vdpa_notify, callback, > > - name, dma_dev); > > + notify, callback, name, dma_dev); > > if (!vq) { > > err = -ENOMEM; > > goto error_new_virtqueue; > > diff --git a/include/linux/vdpa.h b/include/linux/vdpa.h > > index 43f59ef10cc..04cdaad77dd 100644 > > --- a/include/linux/vdpa.h > > +++ b/include/linux/vdpa.h > > @@ -143,6 +143,14 @@ struct vdpa_map_file { > > * @kick_vq: Kick the virtqueue > > * @vdev: vdpa device > > * @idx: virtqueue index > > + * @kick_vq_with_data: Kick the virtqueue and supply extra data > > + * (only if VIRTIO_F_NOTIFICATION_DATA is negotiated) > > + * @vdev: vdpa device > > + * @data for split virtqueue: > > + * 16 bits vqn and 16 bits next available index. > > + * @data for packed virtqueue: > > + * 16 bits vqn, 15 least significant bits of > > + * next available index and 1 bit next_wrap. > > * @set_vq_cb: Set the interrupt callback function for > > * a virtqueue > > * @vdev: vdpa device > > @@ -300,6 +308,7 @@ struct vdpa_config_ops { > > u64 device_area); > > void (*set_vq_num)(struct vdpa_device *vdev, u16 idx, u32 num); > > void (*kick_vq)(struct vdpa_device *vdev, u16 idx); > > + void (*kick_vq_with_data)(struct vdpa_device *vdev, u32 data); > > void (*set_vq_cb)(struct vdpa_device *vdev, u16 idx, > > struct vdpa_callback *cb); > > void (*set_vq_ready)(struct vdpa_device *vdev, u16 idx, bool ready); > > -- > > 2.34.1 > > _______________________________________________ Virtualization mailing list Virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx https://lists.linuxfoundation.org/mailman/listinfo/virtualization
