netdev - Re: [PATCH 06/12] dma-direct: add a CONFIG_ARCH_DMA

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]

Date: Mon, 16 Oct 2023 12:33:36 +0100
From: Robin Murphy <robin.murphy@....com>
To: Christoph Hellwig <hch@....de>, Greg Ungerer <gerg@...ux-m68k.org>,
 iommu@...ts.linux.dev
Cc: Paul Walmsley <paul.walmsley@...ive.com>,
 Palmer Dabbelt <palmer@...belt.com>, Conor Dooley <conor@...nel.org>,
 Geert Uytterhoeven <geert+renesas@...der.be>,
 Magnus Damm <magnus.damm@...il.com>,
 Marek Szyprowski <m.szyprowski@...sung.com>,
 Geert Uytterhoeven <geert@...ux-m68k.org>, Wei Fang <wei.fang@....com>,
 Shenwei Wang <shenwei.wang@....com>, Clark Wang <xiaoning.wang@....com>,
 NXP Linux Team <linux-imx@....com>, linux-m68k@...ts.linux-m68k.org,
 netdev@...r.kernel.org, linux-riscv@...ts.infradead.org,
 linux-renesas-soc@...r.kernel.org, Jim Quinlan <james.quinlan@...adcom.com>
Subject: Re: [PATCH 06/12] dma-direct: add a CONFIG_ARCH_DMA_ALLOC symbol

On 16/10/2023 6:47 am, Christoph Hellwig wrote:
> Instead of using arch_dma_alloc if none of the generic coherent
> allocators are used, require the architectures to explicitly opt into
> providing it.  This will used to deal with the case of m68knommu and
> coldfire where we can't do any coherent allocations whatsoever, and
> also makes it clear that arch_dma_alloc is a last resort.

With one miniscule nit that possibly ARCH_HAS_DMA_ALLOC might be a 
little more consistent with other symbol names in this area (but feel 
free to disagree),

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

> Signed-off-by: Christoph Hellwig <hch@....de>
> ---
>   arch/arm/Kconfig    |  1 +
>   arch/m68k/Kconfig   |  1 +
>   arch/parisc/Kconfig |  1 +
>   kernel/dma/Kconfig  |  9 +++++++++
>   kernel/dma/direct.c | 12 ++----------
>   5 files changed, 14 insertions(+), 10 deletions(-)
> 
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index 9557808e8937b1..a3fdf584278f86 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -4,6 +4,7 @@ config ARM
>   	default y
>   	select ARCH_32BIT_OFF_T
>   	select ARCH_CORRECT_STACKTRACE_ON_KRETPROBE if HAVE_KRETPROBES && FRAME_POINTER && !ARM_UNWIND
> +	select ARCH_DMA_ALLOC if MMU
>   	select ARCH_HAS_BINFMT_FLAT
>   	select ARCH_HAS_CPU_FINALIZE_INIT if MMU
>   	select ARCH_HAS_CURRENT_STACK_POINTER
> diff --git a/arch/m68k/Kconfig b/arch/m68k/Kconfig
> index 3e318bf9504c5b..0430b8ba6b5cc6 100644
> --- a/arch/m68k/Kconfig
> +++ b/arch/m68k/Kconfig
> @@ -3,6 +3,7 @@ config M68K
>   	bool
>   	default y
>   	select ARCH_32BIT_OFF_T
> +	select ARCH_DMA_ALLOC if !MMU || COLDFIRE
>   	select ARCH_HAS_BINFMT_FLAT
>   	select ARCH_HAS_CPU_FINALIZE_INIT if MMU
>   	select ARCH_HAS_CURRENT_STACK_POINTER
> diff --git a/arch/parisc/Kconfig b/arch/parisc/Kconfig
> index a15ab147af2e07..30a4916fa9b0cc 100644
> --- a/arch/parisc/Kconfig
> +++ b/arch/parisc/Kconfig
> @@ -3,6 +3,7 @@ config PARISC
>   	def_bool y
>   	select ALTERNATE_USER_ADDRESS_SPACE
>   	select ARCH_32BIT_OFF_T if !64BIT
> +	select ARCH_DMA_ALLOC if PA11
>   	select ARCH_MIGHT_HAVE_PC_PARPORT
>   	select HAVE_FUNCTION_TRACER
>   	select HAVE_FUNCTION_GRAPH_TRACER
> diff --git a/kernel/dma/Kconfig b/kernel/dma/Kconfig
> index 4524db877eba36..515d2063b509ac 100644
> --- a/kernel/dma/Kconfig
> +++ b/kernel/dma/Kconfig
> @@ -144,6 +144,15 @@ config DMA_DIRECT_REMAP
>   	select DMA_COHERENT_POOL
>   	select DMA_NONCOHERENT_MMAP
>   
> +#
> +# Fallback to arch code for DMA allocations.  This should eventually go away.
> +#
> +config ARCH_DMA_ALLOC
> +	depends on !ARCH_HAS_DMA_SET_UNCACHED
> +	depends on !DMA_DIRECT_REMAP
> +	depends on !DMA_GLOBAL_POOL
> +	bool
> +
>   config DMA_CMA
>   	bool "DMA Contiguous Memory Allocator"
>   	depends on HAVE_DMA_CONTIGUOUS && CMA
> diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
> index 9596ae1aa0dacf..ec410af1d8a14e 100644
> --- a/kernel/dma/direct.c
> +++ b/kernel/dma/direct.c
> @@ -220,13 +220,7 @@ void *dma_direct_alloc(struct device *dev, size_t size,
>   		return dma_direct_alloc_no_mapping(dev, size, dma_handle, gfp);
>   
>   	if (!dev_is_dma_coherent(dev)) {
> -		/*
> -		 * Fallback to the arch handler if it exists.  This should
> -		 * eventually go away.
> -		 */
> -		if (!IS_ENABLED(CONFIG_ARCH_HAS_DMA_SET_UNCACHED) &&
> -		    !IS_ENABLED(CONFIG_DMA_DIRECT_REMAP) &&
> -		    !IS_ENABLED(CONFIG_DMA_GLOBAL_POOL) &&
> +		if (IS_ENABLED(CONFIG_ARCH_DMA_ALLOC) &&
>   		    !is_swiotlb_for_alloc(dev))
>   			return arch_dma_alloc(dev, size, dma_handle, gfp,
>   					      attrs);
> @@ -330,9 +324,7 @@ void dma_direct_free(struct device *dev, size_t size,
>   		return;
>   	}
>   
> -	if (!IS_ENABLED(CONFIG_ARCH_HAS_DMA_SET_UNCACHED) &&
> -	    !IS_ENABLED(CONFIG_DMA_DIRECT_REMAP) &&
> -	    !IS_ENABLED(CONFIG_DMA_GLOBAL_POOL) &&
> +	if (IS_ENABLED(CONFIG_ARCH_DMA_ALLOC) &&
>   	    !dev_is_dma_coherent(dev) &&
>   	    !is_swiotlb_for_alloc(dev)) {
>   		arch_dma_free(dev, size, cpu_addr, dma_addr, attrs);