On Mon, Feb 20, 2023 at 2:56 PM Xuan Zhuo <xuanzhuo@xxxxxxxxxxxxxxxxx> wrote:
>
> On Mon, 20 Feb 2023 13:38:13 +0800, Jason Wang <jasowang@xxxxxxxxxx> wrote:
> > On Tue, Feb 14, 2023 at 3:27 PM Xuan Zhuo <xuanzhuo@xxxxxxxxxxxxxxxxx> wrote:
> > >
> > > virtqueue_add_split() only supports virtual addresses, dma is completed
> > > in virtqueue_add_split().
> > >
> > > In some scenarios (such as the AF_XDP scenario), the memory is allocated
> > > and DMA is completed in advance, so it is necessary for us to support
> > > passing the DMA address to virtio core.
> > >
> > > Signed-off-by: Xuan Zhuo <xuanzhuo@xxxxxxxxxxxxxxxxx>
> > > ---
> > > drivers/virtio/virtio_ring.c | 100 +++++++++++++++++++++++++++++++++--
> > > include/linux/virtio.h | 5 ++
> > > 2 files changed, 100 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
> > > index 47b6f9152f9f..a31155abe101 100644
> > > --- a/drivers/virtio/virtio_ring.c
> > > +++ b/drivers/virtio/virtio_ring.c
> > > @@ -70,6 +70,7 @@
> > > struct vring_desc_state_split {
> > > void *data; /* Data for callback. */
> > > struct vring_desc *indir_desc; /* Indirect descriptor, if any. */
> > > + bool premapped;
> >
> > Better with a comment.
> >
> > Not native speaker, but "dma_addr" might be better?
> >
> > > };
> > >
> > > struct vring_desc_state_packed {
> > > @@ -440,7 +441,7 @@ static void vring_unmap_one_split_indirect(const struct vring_virtqueue *vq,
> > > }
> > >
> > > static unsigned int vring_unmap_one_split(const struct vring_virtqueue *vq,
> > > - unsigned int i)
> > > + unsigned int i, bool premapped)
> > > {
> > > struct vring_desc_extra *extra = vq->split.desc_extra;
> > > u16 flags;
> > > @@ -457,6 +458,9 @@ static unsigned int vring_unmap_one_split(const struct vring_virtqueue *vq,
> > > (flags & VRING_DESC_F_WRITE) ?
> > > DMA_FROM_DEVICE : DMA_TO_DEVICE);
> > > } else {
> > > + if (premapped)
> > > + goto out;
> > > +
> > > dma_unmap_page(vring_dma_dev(vq),
> > > extra[i].addr,
> > > extra[i].len,
> > > @@ -788,6 +792,47 @@ static inline int virtqueue_add_split(struct virtqueue *_vq,
> > > return err;
> > > }
> > >
> > > +static inline int virtqueue_add_split_premapped(struct virtqueue *_vq,
> > > + struct scatterlist *sgs[],
> > > + unsigned int total_sg,
> > > + unsigned int out_sgs,
> > > + unsigned int in_sgs,
> > > + void *data,
> > > + void *ctx,
> > > + gfp_t gfp)
> > > +{
> > > + struct vring_virtqueue *vq = to_vvq(_vq);
> > > + struct vring_desc *desc;
> > > + int head;
> > > + int err;
> > > +
> > > + START_USE(vq);
> > > +
> > > + /* check vq state and try to alloc desc for indirect. */
> > > + err = virtqueue_add_split_prepare(vq, total_sg, out_sgs, data, ctx, gfp, &desc);
> > > + if (err)
> > > + goto end;
> > > +
> > > + head = vq->free_head;
> > > + err = virtqueue_add_split_vring(vq, sgs, total_sg, out_sgs, in_sgs, desc);
> > > + if (err)
> > > + goto err;
> > > +
> > > + /* Store token and indirect buffer state. */
> > > + vq->split.desc_state[head].data = data;
> > > + vq->split.desc_state[head].indir_desc = desc ? desc : ctx;
> > > + vq->split.desc_state[head].premapped = true;
> >
> > This function duplicates most of the logic of virtqueue_add_split()
> > let's unify it.
>
> I want to know that the __virtqueue_add_split is the original
> virtqueue_add_split or my refactor virtqueue_add_split?
It's basically just the virtqueue_add_split_premapped() but with a
boolean to say if it is premapped.
>
> >
> > probably:
> >
> > __virtqueue_add_split(..., bool premapped);
> > virtqueue_add_split()
> > {
> > __virtqueue_add_split(..., false);
> > }
> >
> > virtqueue_add_split_premapped()
> > {
> > __virtqueue_add_split(..., true);
> > }
>
> I am trying to reduce the inspection of premapped.
>
> In fact, this is Michael's request, although I am not particularly sure that my
> implementation has met his requirements.
>
> https://lore.kernel.org/all/20230203041006-mutt-send-email-mst@xxxxxxxxxx/
I think there should be no conflict, the use of premapped was limited
to the above two functions?
Thanks
>
> Thanks.
>
>
> >
> > ?
> >
> > And so did for packed (patch 5).
> >
> > Thanks
> >
> >
> >
> > > +
> > > + goto end;
> > > +
> > > +err:
> > > + kfree(desc);
> > > +
> > > +end:
> > > + END_USE(vq);
> > > + return err;
> > > +}
> > > +
> > > static bool virtqueue_kick_prepare_split(struct virtqueue *_vq)
> > > {
> > > struct vring_virtqueue *vq = to_vvq(_vq);
> > > @@ -824,20 +869,23 @@ static void detach_buf_split(struct vring_virtqueue *vq, unsigned int head,
> > > {
> > > unsigned int i, j;
> > > __virtio16 nextflag = cpu_to_virtio16(vq->vq.vdev, VRING_DESC_F_NEXT);
> > > + bool premapped;
> > >
> > > /* Clear data ptr. */
> > > vq->split.desc_state[head].data = NULL;
> > >
> > > + premapped = vq->split.desc_state[head].premapped;
> > > +
> > > /* Put back on free list: unmap first-level descriptors and find end */
> > > i = head;
> > >
> > > while (vq->split.vring.desc[i].flags & nextflag) {
> > > - vring_unmap_one_split(vq, i);
> > > + vring_unmap_one_split(vq, i, premapped);
> > > i = vq->split.desc_extra[i].next;
> > > vq->vq.num_free++;
> > > }
> > >
> > > - vring_unmap_one_split(vq, i);
> > > + vring_unmap_one_split(vq, i, premapped);
> > > vq->split.desc_extra[i].next = vq->free_head;
> > > vq->free_head = head;
> > >
> > > @@ -859,8 +907,10 @@ static void detach_buf_split(struct vring_virtqueue *vq, unsigned int head,
> > > VRING_DESC_F_INDIRECT));
> > > BUG_ON(len == 0 || len % sizeof(struct vring_desc));
> > >
> > > - for (j = 0; j < len / sizeof(struct vring_desc); j++)
> > > - vring_unmap_one_split_indirect(vq, &indir_desc[j]);
> > > + if (!premapped) {
> > > + for (j = 0; j < len / sizeof(struct vring_desc); j++)
> > > + vring_unmap_one_split_indirect(vq, &indir_desc[j]);
> > > + }
> > >
> > > kfree(indir_desc);
> > > vq->split.desc_state[head].indir_desc = NULL;
> > > @@ -2204,6 +2254,21 @@ static inline int virtqueue_add(struct virtqueue *_vq,
> > > out_sgs, in_sgs, data, ctx, gfp);
> > > }
> > >
> > > +static inline int virtqueue_add_premapped(struct virtqueue *_vq,
> > > + struct scatterlist *sgs[],
> > > + unsigned int total_sg,
> > > + unsigned int out_sgs,
> > > + unsigned int in_sgs,
> > > + void *data,
> > > + void *ctx,
> > > + gfp_t gfp)
> > > +{
> > > + struct vring_virtqueue *vq = to_vvq(_vq);
> > > +
> > > + return virtqueue_add_split_premapped(_vq, sgs, total_sg, out_sgs,
> > > + in_sgs, data, ctx, gfp);
> > > +}
> > > +
> > > /**
> > > * virtqueue_add_sgs - expose buffers to other end
> > > * @_vq: the struct virtqueue we're talking about.
> > > @@ -2261,6 +2326,31 @@ int virtqueue_add_outbuf(struct virtqueue *vq,
> > > }
> > > EXPORT_SYMBOL_GPL(virtqueue_add_outbuf);
> > >
> > > +/**
> > > + * virtqueue_add_outbuf_premapped - expose output buffers to other end
> > > + * @vq: the struct virtqueue we're talking about.
> > > + * @sg: scatterlist (must be well-formed and terminated!)
> > > + * @num: the number of entries in @sg readable by other side
> > > + * @data: the token identifying the buffer.
> > > + * @gfp: how to do memory allocations (if necessary).
> > > + *
> > > + * Caller must ensure we don't call this with other virtqueue operations
> > > + * at the same time (except where noted).
> > > + *
> > > + * It is required that all addrs have completed DMA operations. And use
> > > + * sg->dma_address, sg->length to pass addr and length.
> > > + *
> > > + * Returns zero or a negative error (ie. ENOSPC, ENOMEM, EIO).
> > > + */
> > > +int virtqueue_add_outbuf_premapped(struct virtqueue *vq,
> > > + struct scatterlist *sg, unsigned int num,
> > > + void *data,
> > > + gfp_t gfp)
> > > +{
> > > + return virtqueue_add_premapped(vq, &sg, num, 1, 0, data, NULL, gfp);
> > > +}
> > > +EXPORT_SYMBOL_GPL(virtqueue_add_outbuf_premapped);
> > > +
> > > /**
> > > * virtqueue_add_inbuf - expose input buffers to other end
> > > * @vq: the struct virtqueue we're talking about.
> > > diff --git a/include/linux/virtio.h b/include/linux/virtio.h
> > > index dcab9c7e8784..d8b472a7dcae 100644
> > > --- a/include/linux/virtio.h
> > > +++ b/include/linux/virtio.h
> > > @@ -43,6 +43,11 @@ int virtqueue_add_outbuf(struct virtqueue *vq,
> > > void *data,
> > > gfp_t gfp);
> > >
> > > +int virtqueue_add_outbuf_premapped(struct virtqueue *vq,
> > > + struct scatterlist *sg, unsigned int num,
> > > + void *data,
> > > + gfp_t gfp);
> > > +
> > > int virtqueue_add_inbuf(struct virtqueue *vq,
> > > struct scatterlist sg[], unsigned int num,
> > > void *data,
> > > --
> > > 2.32.0.3.g01195cf9f
> > >
> >
>
_______________________________________________
Virtualization mailing list
Virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
