On Mon, 21 Feb 2022 11:53:33 +0800, Jason Wang <jasowang at redhat.com>
wrote:> On Mon, Feb 21, 2022 at 11:46 AM Xuan Zhuo <xuanzhuo at
linux.alibaba.com> wrote:
> >
> > On Mon, 21 Feb 2022 11:32:52 +0800, Jason Wang <jasowang at
redhat.com> wrote:
> > > On Fri, Feb 18, 2022 at 5:00 PM Xuan Zhuo <xuanzhuo at
linux.alibaba.com> wrote:
> > > >
> > > > On Thu, 17 Feb 2022 15:19:44 +0800, Jason Wang <jasowang
at redhat.com> wrote:
> > > > > On Thu, Feb 10, 2022 at 4:51 PM Xuan Zhuo <xuanzhuo
at linux.alibaba.com> wrote:
> > > > > >
> > > > > > virtqueue_add() only supports virtual addresses,
dma is completed in
> > > > > > virtqueue_add().
> > > > > >
> > > > > > In some scenarios (such as the AF_XDP scenario),
DMA is completed in advance, so
> > > > > > it is necessary for us to support passing the DMA
address to virtqueue_add().
> > > > >
> > > > > I'd suggest rename this feature as "unmanaged
DMA".
> > > >
> > > > OK
> > > >
> > > > >
> > > > > >
> > > > > > Record this predma information in extra->flags,
which can be skipped when
> > > > > > executing dma unmap.
> > > > >
> > > > > Question still, can we use per-virtqueue flag instead
of per
> > > > > descriptor flag? If my memory is correct, the answer is
yes in the
> > > > > discussion for the previous version.
> > > > >
> > > >
> > > > Yes.
> > > >
> > > > per-virtqueue? I guess it should be per-submit.
> > > >
> > > > This patch set only adds a flag to desc_extra[head].flags,
so that we can know
> > > > if we need to unmap dma when we detach.
> > >
> > > I meant if we can manage to make it per virtqueue, there's no
need to
> > > maintain per buffer flag.
> > >
> > > So we know something that needs to be mapped by virtio core
itself,
> > > e.g the indirect page. Other than this, all the rest could be
> > > pre-mapped.
> > >
> > > For vnet header, it could be mapped by virtio-net which could be
still
> > > treated as pre mapped DMA since it's not the virtio ring
code.
> > >
> > > Anything I miss here?
> >
> > I guess, your understanding is that after the queue is reset, the
queue is used
> > by xsk(AF_XDP), then all commits to this vq are premapped amd address.
> >
> > This is ok for rx.
> >
> > But for tx, just like XDP TX, although vq is used by xsk, the kernel
also passes
> > skb to it at the same time. It is shared.
>
> Right.
>
> >
> > We can guarantee that the sg of the sgs submitted at one time uses the
premapped
> > dma address or virtual address uniformly. It is not guaranteed that
all the sgs
> > to the vq are uniform
>
> Sorry, I don't understand here. We can let virtio-net do the mapping
> even for TX, then from the virtio_ring point of view, it's still
> pre-mapped?
>
Yes, we can do this. My previous thought was to keep the skb path unchanged.
Then we can make it clear that in the case of xsk, after completing the queue
reset, all the addresses submitted to virtio are the addresses of the completed
dma, including the skb case, the dma map operation must be completed first.
In this case, I feel like we can do without this patch set.
Thanks.
> Thanks
>
> >
> > Thanks.
> >
> > >
> > > Thanks
> > >
> > >
> > > >
> > > > Thanks.
> > > >
> > > > > Thanks
> > > > >
> > > > > >
> > > > > > v1:
> > > > > > 1. All sgs requested at one time are required
to be unified PREDMA, and several
> > > > > > of them are not supported to be PREDMA
> > > > > > 2. virtio_dma_map() is removed from this patch
set and will be submitted
> > > > > > together with the next time AF_XDP supports
virtio dma
> > > > > > 3. Added patch #2 #3 to remove the check for
flags when performing unmap
> > > > > > indirect desc
> > > > > >
> > > > > > Xuan Zhuo (6):
> > > > > > virtio: rename vring_unmap_state_packed() to
> > > > > > vring_unmap_extra_packed()
> > > > > > virtio: remove flags check for unmap split
indirect desc
> > > > > > virtio: remove flags check for unmap packed
indirect desc
> > > > > > virtio: virtqueue_add() support predma
> > > > > > virtio: split: virtqueue_add_split() support dma
address
> > > > > > virtio: packed: virtqueue_add_packed() support
dma address
> > > > > >
> > > > > > drivers/virtio/virtio_ring.c | 199
++++++++++++++++++++++-------------
> > > > > > 1 file changed, 126 insertions(+), 73
deletions(-)
> > > > > >
> > > > > > --
> > > > > > 2.31.0
> > > > > >
> > > > >
> > > >
> > >
> >
>