On Mon, Feb 21, 2022 at 2:45 PM Xuan Zhuo <[email protected]> wrote: > > On Mon, 21 Feb 2022 14:37:49 +0800, Jason Wang <[email protected]> wrote: > > On Mon, Feb 21, 2022 at 2:20 PM Xuan Zhuo <[email protected]> > > wrote: > > > > > > On Mon, 21 Feb 2022 13:59:06 +0800, Xuan Zhuo > > > <[email protected]> wrote: > > > > On Mon, 21 Feb 2022 11:53:33 +0800, Jason Wang <[email protected]> > > > > wrote: > > > > > On Mon, Feb 21, 2022 at 11:46 AM Xuan Zhuo > > > > > <[email protected]> wrote: > > > > > > > > > > > > On Mon, 21 Feb 2022 11:32:52 +0800, Jason Wang > > > > > > <[email protected]> wrote: > > > > > > > On Fri, Feb 18, 2022 at 5:00 PM Xuan Zhuo > > > > > > > <[email protected]> wrote: > > > > > > > > > > > > > > > > On Thu, 17 Feb 2022 15:19:44 +0800, Jason Wang > > > > > > > > <[email protected]> wrote: > > > > > > > > > On Thu, Feb 10, 2022 at 4:51 PM Xuan Zhuo > > > > > > > > > <[email protected]> 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 > > > > [email protected] > > > > https://lists.linuxfoundation.org/mailman/listinfo/virtualization > > > > > > _______________________________________________ Virtualization mailing list [email protected] https://lists.linuxfoundation.org/mailman/listinfo/virtualization
