On Mon, 29 Jan 2024 11:07:50 +0800, Jason Wang [off-list ref] wrote:

On Thu, Jan 25, 2024 at 1:58 PM Xuan Zhuo [off-list ref] wrote:

quoted

On Thu, 25 Jan 2024 11:39:03 +0800, Jason Wang [off-list ref] wrote:

quoted

On Tue, Jan 16, 2024 at 3:59 PM Xuan Zhuo [off-list ref] wrote:

quoted

Introduce helper virtqueue_get_dma_premapped(), then the driver
can know whether dma unmap is needed.

Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
---
 drivers/net/virtio/main.c       | 22 +++++++++-------------
 drivers/net/virtio/virtio_net.h |  3 ---
 drivers/virtio/virtio_ring.c    | 22 ++++++++++++++++++++++
 include/linux/virtio.h          |  1 +
 4 files changed, 32 insertions(+), 16 deletions(-)

diff --git a/drivers/net/virtio/main.c b/drivers/net/virtio/main.c
index 186b2cf5d8fc..4fbf612da235 100644
--- a/drivers/net/virtio/main.c
+++ b/drivers/net/virtio/main.c

@@ -483,7 +483,7 @@ static void *virtnet_rq_get_buf(struct virtnet_rq *rq, u32 *len, void **ctx)
        void *buf;

        buf = virtqueue_get_buf_ctx(rq->vq, len, ctx);
-       if (buf && rq->do_dma)
+       if (buf && virtqueue_get_dma_premapped(rq->vq))
                virtnet_rq_unmap(rq, buf, *len);

        return buf;

@@ -496,7 +496,7 @@ static void virtnet_rq_init_one_sg(struct virtnet_rq *rq, void *buf, u32 len)
        u32 offset;
        void *head;

-       if (!rq->do_dma) {
+       if (!virtqueue_get_dma_premapped(rq->vq)) {
                sg_init_one(rq->sg, buf, len);
                return;
        }

@@ -526,7 +526,7 @@ static void *virtnet_rq_alloc(struct virtnet_rq *rq, u32 size, gfp_t gfp)

        head = page_address(alloc_frag->page);

-       if (rq->do_dma) {
+       if (virtqueue_get_dma_premapped(rq->vq)) {
                dma = head;

                /* new pages */

@@ -580,12 +580,8 @@ static void virtnet_rq_set_premapped(struct virtnet_info *vi)
        if (!vi->mergeable_rx_bufs && vi->big_packets)
                return;

-       for (i = 0; i < vi->max_queue_pairs; i++) {
-               if (virtqueue_set_dma_premapped(vi->rq[i].vq))
-                       continue;
-
-               vi->rq[i].do_dma = true;
-       }
+       for (i = 0; i < vi->max_queue_pairs; i++)
+               virtqueue_set_dma_premapped(vi->rq[i].vq);
 }

 static void free_old_xmit(struct virtnet_sq *sq, bool in_napi)

@@ -1643,7 +1639,7 @@ static int add_recvbuf_small(struct virtnet_info *vi, struct virtnet_rq *rq,

        err = virtqueue_add_inbuf_ctx(rq->vq, rq->sg, 1, buf, ctx, gfp);
        if (err < 0) {
-               if (rq->do_dma)
+               if (virtqueue_get_dma_premapped(rq->vq))
                        virtnet_rq_unmap(rq, buf, 0);
                put_page(virt_to_head_page(buf));
        }

@@ -1758,7 +1754,7 @@ static int add_recvbuf_mergeable(struct virtnet_info *vi,
        ctx = mergeable_len_to_ctx(len + room, headroom);
        err = virtqueue_add_inbuf_ctx(rq->vq, rq->sg, 1, buf, ctx, gfp);
        if (err < 0) {
-               if (rq->do_dma)
+               if (virtqueue_get_dma_premapped(rq->vq))
                        virtnet_rq_unmap(rq, buf, 0);
                put_page(virt_to_head_page(buf));
        }

@@ -4007,7 +4003,7 @@ static void free_receive_page_frags(struct virtnet_info *vi)
        int i;
        for (i = 0; i < vi->max_queue_pairs; i++)
                if (vi->rq[i].alloc_frag.page) {
-                       if (vi->rq[i].do_dma && vi->rq[i].last_dma)
+                       if (virtqueue_get_dma_premapped(vi->rq[i].vq) && vi->rq[i].last_dma)
                                virtnet_rq_unmap(&vi->rq[i], vi->rq[i].last_dma, 0);
                        put_page(vi->rq[i].alloc_frag.page);
                }

@@ -4035,7 +4031,7 @@ static void virtnet_rq_free_unused_bufs(struct virtqueue *vq)
        rq = &vi->rq[i];

        while ((buf = virtqueue_detach_unused_buf(vq)) != NULL) {
-               if (rq->do_dma)
+               if (virtqueue_get_dma_premapped(rq->vq))
                        virtnet_rq_unmap(rq, buf, 0);

                virtnet_rq_free_buf(vi, rq, buf);

diff --git a/drivers/net/virtio/virtio_net.h b/drivers/net/virtio/virtio_net.h
index b28a4d0a3150..066a2b9d2b3c 100644
--- a/drivers/net/virtio/virtio_net.h
+++ b/drivers/net/virtio/virtio_net.h

@@ -115,9 +115,6 @@ struct virtnet_rq {

        /* Record the last dma info to free after new pages is allocated. */
        struct virtnet_rq_dma *last_dma;
-
-       /* Do dma by self */
-       bool do_dma;
 };

 struct virtnet_info {

diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index 2c5089d3b510..9092bcdebb53 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c

@@ -2905,6 +2905,28 @@ int virtqueue_set_dma_premapped(struct virtqueue *_vq)
 }
 EXPORT_SYMBOL_GPL(virtqueue_set_dma_premapped);

+/**
+ * virtqueue_get_dma_premapped - get the vring premapped mode
+ * @_vq: the struct virtqueue we're talking about.
+ *
+ * Get the premapped mode of the vq.
+ *
+ * Returns bool for the vq premapped mode.
+ */
+bool virtqueue_get_dma_premapped(struct virtqueue *_vq)
+{
+       struct vring_virtqueue *vq = to_vvq(_vq);
+       bool premapped;
+
+       START_USE(vq);
+       premapped = vq->premapped;
+       END_USE(vq);

Why do we need to protect premapped like this? Is the user allowed to
change it on the fly?

Just protect before accessing vq.

I meant how did that differ from other booleans? E.g use_dma_api, do_unmap etc.

Sorry, maybe I misunderstanded you.

Do you mean, should we put "premapped" to the struct virtqueue?
Then the user can read/write by the struct virtqueue directly?

If that, the reason is that when set premapped, we must check
use_dma_api.


int virtqueue_set_dma_premapped(struct virtqueue *_vq)
{
	struct vring_virtqueue *vq = to_vvq(_vq);
	u32 num;

	START_USE(vq);

	num = vq->packed_ring ? vq->packed.vring.num : vq->split.vring.num;

	if (num != vq->vq.num_free) {
		END_USE(vq);
		return -EINVAL;
	}

	if (!vq->use_dma_api) {
		END_USE(vq);
		return -EINVAL;
	}

	vq->buffer_is_premapped = true;

	END_USE(vq);

	return 0;
}

Thanks.

Thanks

quoted

Thanks.

quoted

Thanks