linux-kernel - Re: [PATCH v4 1/2] dma: call unconditionally to unmap_page and unmap

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: <6304d2f3-4e35-46a7-a54d-eb39ba8f3094@arm.com>
Date: Tue, 13 Aug 2024 13:23:10 +0100
From: Robin Murphy <robin.murphy@....com>
To: Leon Romanovsky <leon@...nel.org>, Christoph Hellwig <hch@....de>,
 Joerg Roedel <joro@...tes.org>, Will Deacon <will@...nel.org>,
 Marek Szyprowski <m.szyprowski@...sung.com>
Cc: Leon Romanovsky <leonro@...dia.com>,
 Easwar Hariharan <eahariha@...ux.microsoft.com>,
 linux-kernel@...r.kernel.org, iommu@...ts.linux.dev,
 Jason Gunthorpe <jgg@...dia.com>,
 Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Subject: Re: [PATCH v4 1/2] dma: call unconditionally to unmap_page and
 unmap_sg callbacks

On 24/07/2024 7:04 pm, Leon Romanovsky wrote:
> From: Leon Romanovsky <leonro@...dia.com>
> 
> Almost all users of ->map_page()/map_sg() callbacks implement
> ->unmap_page()/unmap_sg() callbacks too. One user which doesn't do it,
> is dummy DMA ops interface, and the use of this interface is to fail
> the operation and in such case, there won't be any call to
> ->unmap_page()/unmap_sg().

Reviewed-by: Robin Murphy <robin.murphy@....com>

> This patch removes the existence checks of ->unmap_page()/unmap_sg()
> and calls to it directly to create symmetrical interface to
> ->map_page()/map_sg().
> 
> Signed-off-by: Leon Romanovsky <leonro@...dia.com>
> Signed-off-by: Leon Romanovsky <leon@...nel.org>
> ---
>   kernel/dma/dummy.c   | 21 +++++++++++++++++++++
>   kernel/dma/mapping.c |  4 ++--
>   2 files changed, 23 insertions(+), 2 deletions(-)
> 
> diff --git a/kernel/dma/dummy.c b/kernel/dma/dummy.c
> index b492d59ac77e..92de80e5b057 100644
> --- a/kernel/dma/dummy.c
> +++ b/kernel/dma/dummy.c
> @@ -17,6 +17,15 @@ static dma_addr_t dma_dummy_map_page(struct device *dev, struct page *page,
>   {
>   	return DMA_MAPPING_ERROR;
>   }
> +static void dma_dummy_unmap_page(struct device *dev, dma_addr_t dma_handle,
> +		size_t size, enum dma_data_direction dir, unsigned long attrs)
> +{
> +	/*
> +	 * Dummy ops doesn't support map_page, so unmap_page should never be
> +	 * called.
> +	 */
> +	WARN_ON_ONCE(true);
> +}
>   
>   static int dma_dummy_map_sg(struct device *dev, struct scatterlist *sgl,
>   		int nelems, enum dma_data_direction dir,
> @@ -25,6 +34,16 @@ static int dma_dummy_map_sg(struct device *dev, struct scatterlist *sgl,
>   	return -EINVAL;
>   }
>   
> +static void dma_dummy_unmap_sg(struct device *dev, struct scatterlist *sgl,
> +		int nelems, enum dma_data_direction dir,
> +		unsigned long attrs)
> +{
> +	/*
> +	 * Dummy ops doesn't support map_sg, so unmap_sg should never be called.
> +	 */
> +	WARN_ON_ONCE(true);
> +}
> +
>   static int dma_dummy_supported(struct device *hwdev, u64 mask)
>   {
>   	return 0;
> @@ -33,6 +52,8 @@ static int dma_dummy_supported(struct device *hwdev, u64 mask)
>   const struct dma_map_ops dma_dummy_ops = {
>   	.mmap                   = dma_dummy_mmap,
>   	.map_page               = dma_dummy_map_page,
> +	.unmap_page             = dma_dummy_unmap_page,
>   	.map_sg                 = dma_dummy_map_sg,
> +	.unmap_sg               = dma_dummy_unmap_sg,
>   	.dma_supported          = dma_dummy_supported,
>   };
> diff --git a/kernel/dma/mapping.c b/kernel/dma/mapping.c
> index 81de84318ccc..6832fd6f0796 100644
> --- a/kernel/dma/mapping.c
> +++ b/kernel/dma/mapping.c
> @@ -177,7 +177,7 @@ void dma_unmap_page_attrs(struct device *dev, dma_addr_t addr, size_t size,
>   	if (dma_map_direct(dev, ops) ||
>   	    arch_dma_unmap_page_direct(dev, addr + size))
>   		dma_direct_unmap_page(dev, addr, size, dir, attrs);
> -	else if (ops->unmap_page)
> +	else
>   		ops->unmap_page(dev, addr, size, dir, attrs);
>   	debug_dma_unmap_page(dev, addr, size, dir);
>   }
> @@ -291,7 +291,7 @@ void dma_unmap_sg_attrs(struct device *dev, struct scatterlist *sg,
>   	if (dma_map_direct(dev, ops) ||
>   	    arch_dma_unmap_sg_direct(dev, sg, nents))
>   		dma_direct_unmap_sg(dev, sg, nents, dir, attrs);
> -	else if (ops->unmap_sg)
> +	else
>   		ops->unmap_sg(dev, sg, nents, dir, attrs);
>   }
>   EXPORT_SYMBOL(dma_unmap_sg_attrs);