On Fri, Mar 22, 2024 at 2:04 PM Xuan Zhuo <xuanz...@linux.alibaba.com> wrote:
>
> On Fri, 22 Mar 2024 13:13:36 +0800, Jason Wang <jasow...@redhat.com> wrote:
> > On Thu, Mar 21, 2024 at 4:22 PM Xuan Zhuo <xuanz...@linux.alibaba.com>
> > wrote:
> > >
> > > On Thu, 21 Mar 2024 14:02:14 +0800, Jason Wang <jasow...@redhat.com>
> > > wrote:
> > > > On Tue, Mar 12, 2024 at 11:36 AM Xuan Zhuo <xuanz...@linux.alibaba.com>
> > > > wrote:
> > > > >
> > > > > Now, the API virtqueue_set_dma_premapped just support to
> > > > > enable premapped mode.
> > > > >
> > > > > If we allow enabling the premapped dynamically, we should
> > > > > make this API to support disable the premapped mode.
> > > > >
> > > > > Signed-off-by: Xuan Zhuo <xuanz...@linux.alibaba.com>
> > > > > ---
> > > > > drivers/virtio/virtio_ring.c | 34 ++++++++++++++++++++++++++--------
> > > > > include/linux/virtio.h | 2 +-
> > > > > 2 files changed, 27 insertions(+), 9 deletions(-)
> > > > >
> > > > > diff --git a/drivers/virtio/virtio_ring.c
> > > > > b/drivers/virtio/virtio_ring.c
> > > > > index 34f4b2c0c31e..3bf69cae4965 100644
> > > > > --- a/drivers/virtio/virtio_ring.c
> > > > > +++ b/drivers/virtio/virtio_ring.c
> > > > > @@ -2801,6 +2801,7 @@ EXPORT_SYMBOL_GPL(virtqueue_resize);
> > > > > /**
> > > > > * virtqueue_set_dma_premapped - set the vring premapped mode
> > > > > * @_vq: the struct virtqueue we're talking about.
> > > > > + * @premapped: enable/disable the premapped mode.
> > > > > *
> > > > > * Enable the premapped mode of the vq.
> > > > > *
> > > > > @@ -2819,9 +2820,10 @@ EXPORT_SYMBOL_GPL(virtqueue_resize);
> > > > > * 0: success.
> > > > > * -EINVAL: vring does not use the dma api, so we can not enable
> > > > > premapped mode.
> > > > > */
> > > > > -int virtqueue_set_dma_premapped(struct virtqueue *_vq)
> > > > > +int virtqueue_set_dma_premapped(struct virtqueue *_vq, bool
> > > > > premapped)
> > > >
> > > > I think we need to document the requirement for calling this.
> > > >
> > > > Looking at the code, it seems it requires to stop the datapath and
> > > > detach all the used buffers?
> > >
> > >
> > > YES. The complete document is:
> > >
> > > /**
> > > * virtqueue_set_dma_premapped - set the vring premapped mode
> > > * @_vq: the struct virtqueue we're talking about.
> > > *
> > > * Enable the premapped mode of the vq.
> > > *
> > > * The vring in premapped mode does not do dma internally, so the driver
> > > must
> > > * do dma mapping in advance. The driver must pass the dma_address through
> > > * dma_address of scatterlist. When the driver got a used buffer from
> > > * the vring, it has to unmap the dma address.
> > > *
> > > * This function must be called immediately after creating the vq, or
> > > after vq
> > > * reset, and before adding any buffers to it.
> >
> > I'm not sure this is a good design but we need at least some guard for
> > this, probably WARN for num_added or others.
>
>
> int virtqueue_set_dma_premapped(struct virtqueue *_vq)
> {
> struct vring_virtqueue *vq = to_vvq(_vq);
> u32 num;
>
> START_USE(vq);
>
> num = vq->packed_ring ? vq->packed.vring.num : vq->split.vring.num;
>
> if (num != vq->vq.num_free) {
> END_USE(vq);
> return -EINVAL;
> }
>
>
> Now, we have checked the num_free.
Ok, let's add it to the doc.
Thanks
>
> Thanks.
>
>
> >
> > Thanks
> >
> > > *
> > > * Caller must ensure we don't call this with other virtqueue operations
> > > * at the same time (except where noted).
> > > *
> > > * Returns zero or a negative error.
> > > * 0: success.
> > > * -EINVAL: vring does not use the dma api, so we can not enable
> > > premapped mode.
> > > */
> > >
> > > Thanks
> > >
> > >
> > > >
> > > > Thanks
> > > >
> > >
> >
>
