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: <CACGkMEtNCjvtDWySzeAqETGZtBSL0MR6=JySBBtm3=s19wB=1w@mail.gmail.com>
Date: Wed, 31 Jan 2024 17:12:29 +0800
From: Jason Wang <jasowang@...hat.com>
To: Xuan Zhuo <xuanzhuo@...ux.alibaba.com>
Cc: virtualization@...ts.linux.dev, Richard Weinberger <richard@....at>, 
	Anton Ivanov <anton.ivanov@...bridgegreys.com>, Johannes Berg <johannes@...solutions.net>, 
	"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>, Hans de Goede <hdegoede@...hat.com>, 
	Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>, 
	Vadim Pasternak <vadimp@...dia.com>, Bjorn Andersson <andersson@...nel.org>, 
	Mathieu Poirier <mathieu.poirier@...aro.org>, Cornelia Huck <cohuck@...hat.com>, 
	Halil Pasic <pasic@...ux.ibm.com>, Eric Farman <farman@...ux.ibm.com>, 
	Heiko Carstens <hca@...ux.ibm.com>, Vasily Gorbik <gor@...ux.ibm.com>, 
	Alexander Gordeev <agordeev@...ux.ibm.com>, Christian Borntraeger <borntraeger@...ux.ibm.com>, 
	Sven Schnelle <svens@...ux.ibm.com>, Alexei Starovoitov <ast@...nel.org>, 
	Daniel Borkmann <daniel@...earbox.net>, Jesper Dangaard Brouer <hawk@...nel.org>, 
	John Fastabend <john.fastabend@...il.com>, Benjamin Berg <benjamin.berg@...el.com>, 
	Yang Li <yang.lee@...ux.alibaba.com>, linux-um@...ts.infradead.org, 
	netdev@...r.kernel.org, platform-driver-x86@...r.kernel.org, 
	linux-remoteproc@...r.kernel.org, linux-s390@...r.kernel.org, 
	kvm@...r.kernel.org, bpf@...r.kernel.org
Subject: Re: [PATCH vhost 06/17] virtio_ring: no store dma info when unmap is
 not needed

On Tue, Jan 30, 2024 at 7:42 PM Xuan Zhuo <xuanzhuo@...ux.alibaba.com> wrote:
>
> As discussed:
> http://lore.kernel.org/all/CACGkMEug-=C+VQhkMYSgUKMC==04m7-uem_yC21bgGkKZh845w@mail.gmail.com
>
> When the vq is premapped mode, the driver manages the dma
> info is a good way.
>
> So this commit make the virtio core not to store the dma
> info and release the memory which is used to store the dma
> info.
>
> If the use_dma_api is false, the memory is also not allocated.
>
> Signed-off-by: Xuan Zhuo <xuanzhuo@...ux.alibaba.com>
> ---
>  drivers/virtio/virtio_ring.c | 89 ++++++++++++++++++++++++++++--------
>  1 file changed, 70 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
> index 831667a57429..5bea25167259 100644
> --- a/drivers/virtio/virtio_ring.c
> +++ b/drivers/virtio/virtio_ring.c
> @@ -94,12 +94,15 @@ struct vring_desc_state_packed {
>  };
>
>  struct vring_desc_extra {
> -       dma_addr_t addr;                /* Descriptor DMA addr. */
> -       u32 len;                        /* Descriptor length. */
>         u16 flags;                      /* Descriptor flags. */
>         u16 next;                       /* The next desc state in a list. */
>  };
>
> +struct vring_desc_dma {
> +       dma_addr_t addr;                /* Descriptor DMA addr. */
> +       u32 len;                        /* Descriptor length. */
> +};
> +
>  struct vring_virtqueue_split {
>         /* Actual memory layout for this queue. */
>         struct vring vring;
> @@ -116,6 +119,7 @@ struct vring_virtqueue_split {
>         /* Per-descriptor state. */
>         struct vring_desc_state_split *desc_state;
>         struct vring_desc_extra *desc_extra;
> +       struct vring_desc_dma *desc_dma;
>
>         /* DMA address and size information */
>         dma_addr_t queue_dma_addr;
> @@ -156,6 +160,7 @@ struct vring_virtqueue_packed {
>         /* Per-descriptor state. */
>         struct vring_desc_state_packed *desc_state;
>         struct vring_desc_extra *desc_extra;
> +       struct vring_desc_dma *desc_dma;
>
>         /* DMA address and size information */
>         dma_addr_t ring_dma_addr;
> @@ -472,13 +477,14 @@ static unsigned int vring_unmap_one_split(const struct vring_virtqueue *vq,
>                                           unsigned int i)
>  {
>         struct vring_desc_extra *extra = vq->split.desc_extra;
> +       struct vring_desc_dma *dma = vq->split.desc_dma;
>         u16 flags;
>
>         flags = extra[i].flags;
>
>         dma_unmap_page(vring_dma_dev(vq),
> -                      extra[i].addr,
> -                      extra[i].len,
> +                      dma[i].addr,
> +                      dma[i].len,
>                        (flags & VRING_DESC_F_WRITE) ?
>                        DMA_FROM_DEVICE : DMA_TO_DEVICE);
>
> @@ -535,8 +541,11 @@ static inline unsigned int virtqueue_add_desc_split(struct virtqueue *vq,
>                 next = extra[i].next;
>                 desc[i].next = cpu_to_virtio16(vq->vdev, next);
>
> -               extra[i].addr = addr;
> -               extra[i].len = len;
> +               if (vring->split.desc_dma) {
> +                       vring->split.desc_dma[i].addr = addr;
> +                       vring->split.desc_dma[i].len = len;
> +               }
> +
>                 extra[i].flags = flags;
>         } else
>                 next = virtio16_to_cpu(vq->vdev, desc[i].next);
> @@ -1072,16 +1081,26 @@ static void virtqueue_vring_attach_split(struct vring_virtqueue *vq,
>         vq->free_head = 0;
>  }
>
> -static int vring_alloc_state_extra_split(struct vring_virtqueue_split *vring_split)
> +static int vring_alloc_state_extra_split(struct vring_virtqueue_split *vring_split,
> +                                        bool need_unmap)
>  {
>         struct vring_desc_state_split *state;
>         struct vring_desc_extra *extra;
> +       struct vring_desc_dma *dma;
>         u32 num = vring_split->vring.num;
>
>         state = kmalloc_array(num, sizeof(struct vring_desc_state_split), GFP_KERNEL);
>         if (!state)
>                 goto err_state;
>
> +       if (need_unmap) {
> +               dma = kmalloc_array(num, sizeof(struct vring_desc_dma), GFP_KERNEL);
> +               if (!dma)
> +                       goto err_dma;
> +       } else {
> +               dma = NULL;
> +       }
> +
>         extra = vring_alloc_desc_extra(num);
>         if (!extra)
>                 goto err_extra;
> @@ -1090,9 +1109,12 @@ static int vring_alloc_state_extra_split(struct vring_virtqueue_split *vring_spl
>
>         vring_split->desc_state = state;
>         vring_split->desc_extra = extra;
> +       vring_split->desc_dma = dma;
>         return 0;
>
>  err_extra:
> +       kfree(dma);
> +err_dma:
>         kfree(state);
>  err_state:
>         return -ENOMEM;
> @@ -1108,6 +1130,7 @@ static void vring_free_split(struct vring_virtqueue_split *vring_split,
>
>         kfree(vring_split->desc_state);
>         kfree(vring_split->desc_extra);
> +       kfree(vring_split->desc_dma);
>  }
>
>  static int vring_alloc_queue_split(struct vring_virtqueue_split *vring_split,
> @@ -1209,7 +1232,8 @@ static int virtqueue_resize_split(struct virtqueue *_vq, u32 num)
>         if (err)
>                 goto err;
>
> -       err = vring_alloc_state_extra_split(&vring_split);
> +       err = vring_alloc_state_extra_split(&vring_split,
> +                                           vring_need_unmap_buffer(vq));
>         if (err)
>                 goto err_state_extra;
>
> @@ -1245,14 +1269,16 @@ static u16 packed_last_used(u16 last_used_idx)
>
>  /* caller must check vring_need_unmap_buffer() */
>  static void vring_unmap_extra_packed(const struct vring_virtqueue *vq,
> -                                    const struct vring_desc_extra *extra)
> +                                    unsigned int i)
>  {
> +       const struct vring_desc_extra *extra = &vq->packed.desc_extra[i];
> +       const struct vring_desc_dma *dma = &vq->packed.desc_dma[i];
>         u16 flags;
>
>         flags = extra->flags;

I don't think this can be compiled.

Thanks


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ