On Wed, Feb 23, 2022 at 10:58 AM Jason Wang <jasowang@xxxxxxxxxx> wrote: > > On Tue, Feb 22, 2022 at 4:00 PM Xuan Zhuo <xuanzhuo@xxxxxxxxxxxxxxxxx> wrote: > > > > On Tue, 22 Feb 2022 12:02:14 +0800, Jason Wang <jasowang@xxxxxxxxxx> wrote: > > > > > > 在 2022/2/21 下午7:23, Xuan Zhuo 写道: > > > > On Mon, 21 Feb 2022 11:32:52 +0800, Jason Wang <jasowang@xxxxxxxxxx> wrote: > > > >> On Fri, Feb 18, 2022 at 5:00 PM Xuan Zhuo <xuanzhuo@xxxxxxxxxxxxxxxxx> wrote: > > > >>> On Thu, 17 Feb 2022 15:19:44 +0800, Jason Wang <jasowang@xxxxxxxxxx> wrote: > > > >>>> On Thu, Feb 10, 2022 at 4:51 PM Xuan Zhuo <xuanzhuo@xxxxxxxxxxxxxxxxx> 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. > > > >> > > > > Rethinking this question, I feel there is no essential difference between per > > > > virtqueue and per sgs. > > > > > > > > per virtqueue: > > > > 1. add buf: > > > > a. check vq->premapped for map every sg > > > > 2. detach: > > > > a. check vq->premaped for unmap > > > > > > > > per sgs: > > > > 1. add buf: > > > > a. check function parameter "premapped" for map every sg > > > > b. add flag to extra[head].flag > > > > > > > > 2. detach: > > > > a: check extra[head].flag for unmap > > > > > > > > > > > > Thanks. > > > > > > > > > Per-virtqueue is still a little bit easier at the first glance. > > > > > > Actually, per-sg have one advantage: it can be used without virtqueue > > > reset (to allow switching between the two modes). But I'm not sure > > > whether we had such requirements. > > > > > > I think to answer this question, we probably need a real use case (if we > > > can come up with a case that is more lightweight than AF_XDP, that would > > > be even better). > > > > Sadly, I didn't think of other scenarios. Hope someone can give a scenario. > > > > For per virtqueue, virtio-net will also switch to premapped. Because the tx > > queue is shared. > > > > But in the process of implementing this, I encountered a troublesome problem. We > > need to record the dma address in virtnet. For tx, since skb contains multiple > > frags, there will be many dma addresses. When unmap in virtnet It will be more > > troublesome. Because we have to regain these dma addresses. > > Right, actually, we store the dma address in desc_extra, but exposing > it to the driver seems like an overkill. > > > > > I think of two ways: > > > > 1. Let virtio return the addr of each desc when detached. > > 2. Allocate a block of memory for each sq/rq to hold the dma address. > > > > Thanks. > > So it looks to me having a per buffer flag seems ok. Let me go through > this series. > Ok, so the series looks good overall, but we need a user. I wonder if we can convert XDP to use that as an example. (and AF_XDP on top). Thanks > Thanks > > > > > > > > > Thanks > > > > > > > > > > > > > > > > > >> 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? > > > >> > > > >> 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@xxxxxxxxxxxxxxxxxxxxxxxxxx https://lists.linuxfoundation.org/mailman/listinfo/virtualization
