Re: [PATCH 06/12] dma-direct: add a CONFIG_ARCH_DMA_ALLOC symbol

[Robin Murphy <robin.murphy@arm.com>]

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@arm.com>

> Signed-off-by: Christoph Hellwig <hch@lst.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);