lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <1706162276.5311615-3-xuanzhuo@linux.alibaba.com>
Date: Thu, 25 Jan 2024 13:57:56 +0800
From: Xuan Zhuo <xuanzhuo@...ux.alibaba.com>
To: Jason Wang <jasowang@...hat.com>
Cc: netdev@...r.kernel.org,
 "Michael S. Tsirkin" <mst@...hat.com>,
 "David S. Miller" <davem@...emloft.net>,
 Eric Dumazet <edumazet@...gle.com>,
 Jakub Kicinski <kuba@...nel.org>,
 Paolo Abeni <pabeni@...hat.com>,
 Alexei Starovoitov <ast@...nel.org>,
 Daniel Borkmann <daniel@...earbox.net>,
 Jesper Dangaard Brouer <hawk@...nel.org>,
 John Fastabend <john.fastabend@...il.com>,
 virtualization@...ts.linux.dev,
 bpf@...r.kernel.org
Subject: Re: [PATCH net-next 4/5] virtio_ring: introduce virtqueue_get_dma_premapped()

On Thu, 25 Jan 2024 11:39:03 +0800, Jason Wang <jasowang@...hat.com> wrote:
> On Tue, Jan 16, 2024 at 3:59 PM Xuan Zhuo <xuanzhuo@...ux.alibaba.com> wrote:
> >
> > Introduce helper virtqueue_get_dma_premapped(), then the driver
> > can know whether dma unmap is needed.
> >
> > Signed-off-by: Xuan Zhuo <xuanzhuo@...ux.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.

Thanks.
>
> Thanks
>

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ