netdev - Re: [PATCH vhost 1/6] virtio_ring: introduce dma map api for page

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: <1db181fd-8d08-4f6d-b32f-20d06029360c@intel.com>
Date: Thu, 11 Apr 2024 13:45:28 +0200
From: Alexander Lobakin <aleksander.lobakin@...el.com>
To: Xuan Zhuo <xuanzhuo@...ux.alibaba.com>
CC: <virtualization@...ts.linux.dev>, "Michael S. Tsirkin" <mst@...hat.com>,
	Jason Wang <jasowang@...hat.com>, "David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>, "Paolo
 Abeni" <pabeni@...hat.com>, <netdev@...r.kernel.org>
Subject: Re: [PATCH vhost 1/6] virtio_ring: introduce dma map api for page

From: Xuan Zhuo <xuanzhuo@...ux.alibaba.com>
Date: Thu, 11 Apr 2024 10:51:22 +0800

> The virtio-net big mode sq will use these APIs to map the pages.
> 
> dma_addr_t virtqueue_dma_map_page_attrs(struct virtqueue *_vq, struct page *page,
>                                        size_t offset, size_t size,
>                                        enum dma_data_direction dir,
>                                        unsigned long attrs);
> void virtqueue_dma_unmap_page_attrs(struct virtqueue *_vq, dma_addr_t addr,
>                                    size_t size, enum dma_data_direction dir,
>                                    unsigned long attrs);
> 
> Signed-off-by: Xuan Zhuo <xuanzhuo@...ux.alibaba.com>
> ---
>  drivers/virtio/virtio_ring.c | 52 ++++++++++++++++++++++++++++++++++++
>  include/linux/virtio.h       |  7 +++++
>  2 files changed, 59 insertions(+)
> 
> diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
> index 70de1a9a81a3..1b9fb680cff3 100644
> --- a/drivers/virtio/virtio_ring.c
> +++ b/drivers/virtio/virtio_ring.c
> @@ -3100,6 +3100,58 @@ void virtqueue_dma_unmap_single_attrs(struct virtqueue *_vq, dma_addr_t addr,
>  }
>  EXPORT_SYMBOL_GPL(virtqueue_dma_unmap_single_attrs);
>  
> +/**
> + * virtqueue_dma_map_page_attrs - map DMA for _vq
> + * @_vq: the struct virtqueue we're talking about.
> + * @page: the page to do dma
> + * @offset: the offset inside the page
> + * @size: the size of the page to do dma
> + * @dir: DMA direction
> + * @attrs: DMA Attrs
> + *
> + * The caller calls this to do dma mapping in advance. The DMA address can be
> + * passed to this _vq when it is in pre-mapped mode.
> + *
> + * return DMA address. Caller should check that by virtqueue_dma_mapping_error().
> + */
> +dma_addr_t virtqueue_dma_map_page_attrs(struct virtqueue *_vq, struct page *page,
> +					size_t offset, size_t size,
> +					enum dma_data_direction dir,
> +					unsigned long attrs)
> +{
> +	struct vring_virtqueue *vq = to_vvq(_vq);
> +
> +	if (!vq->use_dma_api)
> +		return page_to_phys(page) + offset;

page_to_phys() and the actual page DMA address may differ. See
page_to_dma()/virt_to_dma(). I believe this is not correct.

> +
> +	return dma_map_page_attrs(vring_dma_dev(vq), page, offset, size, dir, attrs);
> +}
> +EXPORT_SYMBOL_GPL(virtqueue_dma_map_page_attrs);

Could you try make these functions static inlines and run bloat-o-meter?
They seem to be small and probably you'd get better performance.

> +
> +/**
> + * virtqueue_dma_unmap_page_attrs - unmap DMA for _vq
> + * @_vq: the struct virtqueue we're talking about.
> + * @addr: the dma address to unmap
> + * @size: the size of the buffer
> + * @dir: DMA direction
> + * @attrs: DMA Attrs
> + *
> + * Unmap the address that is mapped by the virtqueue_dma_map_* APIs.
> + *
> + */
> +void virtqueue_dma_unmap_page_attrs(struct virtqueue *_vq, dma_addr_t addr,
> +				    size_t size, enum dma_data_direction dir,
> +				    unsigned long attrs)
> +{
> +	struct vring_virtqueue *vq = to_vvq(_vq);
> +
> +	if (!vq->use_dma_api)
> +		return;
> +
> +	dma_unmap_page_attrs(vring_dma_dev(vq), addr, size, dir, attrs);
> +}
> +EXPORT_SYMBOL_GPL(virtqueue_dma_unmap_page_attrs);
> +
>  /**
>   * virtqueue_dma_mapping_error - check dma address
>   * @_vq: the struct virtqueue we're talking about.
> diff --git a/include/linux/virtio.h b/include/linux/virtio.h
> index 26c4325aa373..d6c699553979 100644
> --- a/include/linux/virtio.h
> +++ b/include/linux/virtio.h
> @@ -228,6 +228,13 @@ dma_addr_t virtqueue_dma_map_single_attrs(struct virtqueue *_vq, void *ptr, size
>  void virtqueue_dma_unmap_single_attrs(struct virtqueue *_vq, dma_addr_t addr,
>  				      size_t size, enum dma_data_direction dir,
>  				      unsigned long attrs);
> +dma_addr_t virtqueue_dma_map_page_attrs(struct virtqueue *_vq, struct page *page,
> +					size_t offset, size_t size,
> +					enum dma_data_direction dir,
> +					unsigned long attrs);
> +void virtqueue_dma_unmap_page_attrs(struct virtqueue *_vq, dma_addr_t addr,
> +				    size_t size, enum dma_data_direction dir,
> +				    unsigned long attrs);
>  int virtqueue_dma_mapping_error(struct virtqueue *_vq, dma_addr_t addr);
>  
>  bool virtqueue_dma_need_sync(struct virtqueue *_vq, dma_addr_t addr);

Thanks,
Olek