Re: [PATCH vhost v13 05/12] virtio_ring: introduce virtqueue_dma_dev()

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

On Mon, Aug 14, 2023 at 04:56:53PM +0800, Xuan Zhuo wrote:
> On Mon, 14 Aug 2023 11:05:49 +0800, Jason Wang <jasowang@redhat.com> wrote:
> > On Thu, Aug 10, 2023 at 8:31 PM Xuan Zhuo <xuanzhuo@linux.alibaba.com> wrote:
> > >
> > > Added virtqueue_dma_dev() to get DMA device for virtio. Then the
> > > caller can do dma operation in advance. The purpose is to keep memory
> > > mapped across multiple add/get buf operations.
> > >
> > > Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
> > > Acked-by: Jason Wang <jasowang@redhat.com>
> >
> > So I think we don't have actual users for this in this series? Can we
> > simply have another independent patch for this?
> 
> I am ok. I will remove this from the next version.
> 
> But I also help merge this to 6.6. Then we can let the virtio-net to support
> AF_XDP in 6.7+.

Is there going to be a next version? Because if yes it will be too late for the next release.
if all you want to do is drop this patch then just say so, no need
for another version.

> 
> >
> > > ---
> > >  drivers/virtio/virtio_ring.c | 17 +++++++++++++++++
> > >  include/linux/virtio.h       |  2 ++
> > >  2 files changed, 19 insertions(+)
> > >
> > > diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
> > > index f9f772e85a38..bb3d73d221cd 100644
> > > --- a/drivers/virtio/virtio_ring.c
> > > +++ b/drivers/virtio/virtio_ring.c
> > > @@ -2265,6 +2265,23 @@ int virtqueue_add_inbuf_ctx(struct virtqueue *vq,
> > >  }
> > >  EXPORT_SYMBOL_GPL(virtqueue_add_inbuf_ctx);
> > >
> > > +/**
> > > + * virtqueue_dma_dev - get the dma dev
> > > + * @_vq: the struct virtqueue we're talking about.
> > > + *
> > > + * Returns the dma dev. That can been used for dma api.
> > > + */
> > > +struct device *virtqueue_dma_dev(struct virtqueue *_vq)
> > > +{
> > > +       struct vring_virtqueue *vq = to_vvq(_vq);
> > > +
> > > +       if (vq->use_dma_api)
> > > +               return vring_dma_dev(vq);
> > > +       else
> > > +               return NULL;
> > > +}
> > > +EXPORT_SYMBOL_GPL(virtqueue_dma_dev);
> >
> > One possible concern is that exporting things like NULL may result in
> > the switch in the caller (driver). I wonder if it's better to do
> > BUG_ON() in the path of NULL?
> 
> 
> I agree.
> 
> But we need a new helper to tell the driver(or AF_XDP) that the device support
> ACCESS_PLATFORM or not.
> 
> We need a switch, but we can make the switch is irrelevant to the DMA.
> 
> Thanks.
> 
> 
> 
> >
> > Thanks
> >
> > > +
> > >  /**
> > >   * virtqueue_kick_prepare - first half of split virtqueue_kick call.
> > >   * @_vq: the struct virtqueue
> > > diff --git a/include/linux/virtio.h b/include/linux/virtio.h
> > > index 8add38038877..bd55a05eec04 100644
> > > --- a/include/linux/virtio.h
> > > +++ b/include/linux/virtio.h
> > > @@ -61,6 +61,8 @@ int virtqueue_add_sgs(struct virtqueue *vq,
> > >                       void *data,
> > >                       gfp_t gfp);
> > >
> > > +struct device *virtqueue_dma_dev(struct virtqueue *vq);
> > > +
> > >  bool virtqueue_kick(struct virtqueue *vq);
> > >
> > >  bool virtqueue_kick_prepare(struct virtqueue *vq);
> > > --
> > > 2.32.0.3.g01195cf9f
> > >
> >