On Mon, Feb 21, 2022 at 2:45 PM Xuan Zhuo <xuanzhuo at linux.alibaba.com>
wrote:>
> On Mon, 21 Feb 2022 14:37:49 +0800, Jason Wang <jasowang at
redhat.com> wrote:
> > On Mon, Feb 21, 2022 at 2:20 PM Xuan Zhuo <xuanzhuo at
linux.alibaba.com> wrote:
> > >
> > > On Mon, 21 Feb 2022 13:59:06 +0800, Xuan Zhuo <xuanzhuo at
linux.alibaba.com> wrote:
> > > > 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.
> >
> > We can listen from Michael and others but I think it would be simpler.
> >
> > And we can even make the pre-mapped per driver. E.g for virtio-net we
> > just let the virtio-net driver do the DMA mapping. This allows us to
> > do a lot of optimizations (e.g page flip) as what other networking
> > drivers did.
> >
> > > >
> > > > 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.
> > >
> > > I originally thought that use_dma_api could be reused, but I
found that this is
> > > not the case. The logic of sg_phys() does not meet our ideas. We
still have a
> > > separate flag.
> >
> > Just to make sure I understand here, for this flag you mean
> >
> > 1) per buffer
> > 2) per virtqueue
> > or
> > 3) per device?
>
> To be honest, I haven't figured out what the advantage of the
driver's own
> management of dma is. If it just implements AF_XDP, then per virtqueue
should be
> fine.
Haven't thought it deeply but generally it allows driver to do ad-hoc
optimizations on DMA mapping.
But it should be also a question of complexity, if we don't see real
use case, we can choose the easier way. I thought using per virtqueue
or per device is simpler but I might be wrong. Most of ethernet driver
manage the DMA mapping by itself.
>
> And if it's per device, then I feel like another interesting job. Is
premapped
> dma address used by default?
If we go with this way, it should be job of the driver to enable it or not.
>
> Should I submit a patch set to make virtnet-net use the premapped dma
address
> directly by default?
It might be a better idea if it's simpler. (I haven't saw a driver
that mixes the per-mapped DMA with the managed DMA)
Thanks
>
> Thanks.
>
>
> >
> > Thanks
> >
> > >
> > > Thanks.
> > >
> > > >
> > > > 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
> > > > > > > > > >
> > > > > > > > >
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > > _______________________________________________
> > > > Virtualization mailing list
> > > > Virtualization at lists.linux-foundation.org
> > > >
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
> > >
> >
>