Re: [PATCH v3 08/11] vdpa: Add eventfd for the vdpa callback

[Yongji Xie]

On Thu, Mar 16, 2023 at 5:26 PM Jason Wang <jasowang@xxxxxxxxxx> wrote:
>
>
> 在 2023/2/28 17:41, Xie Yongji 写道:
> > Add eventfd for the vdpa callback so that user
> > can signal it directly instead of running the
> > callback. It will be used for vhost-vdpa case.
> >
> > Signed-off-by: Xie Yongji <xieyongji@xxxxxxxxxxxxx>
> > ---
> >   drivers/vhost/vdpa.c         | 2 ++
> >   drivers/virtio/virtio_vdpa.c | 1 +
> >   include/linux/vdpa.h         | 3 +++
> >   3 files changed, 6 insertions(+)
> >
> > diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c
> > index dc12dbd5b43b..ae89c0ccc2bb 100644
> > --- a/drivers/vhost/vdpa.c
> > +++ b/drivers/vhost/vdpa.c
> > @@ -599,9 +599,11 @@ static long vhost_vdpa_vring_ioctl(struct vhost_vdpa *v, unsigned int cmd,
> >               if (vq->call_ctx.ctx) {
> >                       cb.callback = vhost_vdpa_virtqueue_cb;
> >                       cb.private = vq;
> > +                     cb.irq_ctx = vq->call_ctx.ctx;
> >               } else {
> >                       cb.callback = NULL;
> >                       cb.private = NULL;
> > +                     cb.irq_ctx = NULL;
> >               }
> >               ops->set_vq_cb(vdpa, idx, &cb);
> >               vhost_vdpa_setup_vq_irq(v, idx);
> > diff --git a/drivers/virtio/virtio_vdpa.c b/drivers/virtio/virtio_vdpa.c
> > index 9eee8afabda8..a5cecafbc2d1 100644
> > --- a/drivers/virtio/virtio_vdpa.c
> > +++ b/drivers/virtio/virtio_vdpa.c
> > @@ -195,6 +195,7 @@ virtio_vdpa_setup_vq(struct virtio_device *vdev, unsigned int index,
> >       /* Setup virtqueue callback */
> >       cb.callback = callback ? virtio_vdpa_virtqueue_cb : NULL;
> >       cb.private = info;
> > +     cb.irq_ctx = NULL;
> >       ops->set_vq_cb(vdpa, index, &cb);
> >       ops->set_vq_num(vdpa, index, virtqueue_get_vring_size(vq));
> >
> > diff --git a/include/linux/vdpa.h b/include/linux/vdpa.h
> > index 10bd22387276..94a7ec49583a 100644
> > --- a/include/linux/vdpa.h
> > +++ b/include/linux/vdpa.h
> > @@ -13,10 +13,13 @@
> >    * struct vdpa_calllback - vDPA callback definition.
> >    * @callback: interrupt callback function
> >    * @private: the data passed to the callback function
> > + * @irq_ctx: the eventfd for the callback, user can signal
> > + *           it directly instead of running the callback
>
>
> I'd suggest to do more tweaks to mention:
>
> 1) irq_ctx is optional
> 2) that when the irq_ctx is set, the vDPA driver must guarantee that
> signaling it is functional equivalent to triggering the callback. When
> set, vDPA parent can signal it directly instead of triggering the callback.
>

OK, I will add more comments for it.

> >    */
> >   struct vdpa_callback {
> >       irqreturn_t (*callback)(void *data);
> >       void *private;
> > +     struct eventfd_ctx *irq_ctx;
>
>
> There's no IRQ concept at the virtual vDPA bus level, so it's probably
> better to rename it as "trigger".
>

LGTM.

Thanks,
Yongji