# \[PATCH v2 7/7\] OF: Don't set default coherent DMA mask

- From: Robin Murphy \<robin.murphy@arm.com\>
- Date: 2018-07-27 20:42:14 
- List: linux-arm-kernel
- Page: https://ratatoskr.run/linux-arm-kernel/2018/07/8771709
- Thread: https://ratatoskr.run/linux-arm-kernel/2018/07/8771456/t.md

```text
On 2018-07-27 7:45 PM, Grygorii Strashko wrote:
[...]
> But I have a question to all:
> - The patch [1] sets default DMA mask to DMA_BIT_MASK(32)
> +	dev->dev.coherent_dma_mask = DMA_BIT_MASK(32);
> +	if (!dev->dev.dma_mask)
> +		dev->dev.dma_mask = &dev->dev.coherent_dma_mask;
> 
> this will also work the same way for ARM64
> 
> - But of_dma_configure() still have code which does:
> 	dev->coherent_dma_mask &= mask;
> 	*dev->dma_mask &= mask;
> 
> As result, DMA mask supplied from DT will always be truncated
>   to 32 bit for platform devices. for example:
> 
> soc0: soc0 {
>          compatible = "simple-bus";
>          #address-cells = <2>;
>          #size-cells = <2>;
>          ranges;
> +        dma-ranges = <0 0 0 0 0x10000 0>;
> 
> ^ 48 bit DMA mask expected to be generated and assigned.
> 
> But real mask will be DMA_BIT_MASK(32). As result, any
> DMA capable driver will have be modified to do dma_set_xxx_mask(),
> which disregards DT DMA configuration (especially for HW modules
> which are used on ARM32 and ARM64).

That has always been the case. Drivers which want to use 
larger-than-32-bit masks *must* explicitly say so. The issue that the DT 
dma-ranges (and ACPI equivalents) cannot be preserved in the device DMA 
masks is the entire purpose of this series. At the moment there's only a 
couple of point fixes for specific places with known problems, but this 
is just the start of some ongoing work.

> Could it be considered to do one the changes below?
> 
> 1) assign mask in case of dt
> diff --git a/drivers/of/device.c b/drivers/of/device.c
> index 5957cd4..f7dc121 100644
> --- a/drivers/of/device.c
> +++ b/drivers/of/device.c
> @@ -150,8 +150,8 @@ int of_dma_configure(struct device *dev, struct device_node *np, bool force_dma)
>           */
>          mask = DMA_BIT_MASK(ilog2(dma_addr + size - 1) + 1);
>          dev->bus_dma_mask = mask;
> -       dev->coherent_dma_mask &= mask;
> -       *dev->dma_mask &= mask;
> +       dev->coherent_dma_mask = mask;
> +       *dev->dma_mask = mask;
>   
>          coherent = of_dma_is_coherent(np);

No, because that leads to a risk of DMA address truncation in hardware 
(and thus at worst random memory corruption) when drivers expect the 
default mask to be 32-bit and fail to explicitly set it as such.

> 2) use BITS_PER_LONG for default DMA mask for of_platform devices
> 
> diff --git a/drivers/of/platform.c b/drivers/of/platform.c
> index 7ba90c2..3f326e2 100644
> --- a/drivers/of/platform.c
> +++ b/drivers/of/platform.c
> @@ -185,7 +185,7 @@ static struct platform_device *of_platform_device_create_pdata(
>          if (!dev)
>                  goto err_clear_flag;
>   
> -       dev->dev.coherent_dma_mask = DMA_BIT_MASK(32);
> +       dev->dev.coherent_dma_mask = DMA_BIT_MASK(BITS_PER_LONG);
>          if (!dev->dev.dma_mask)
>                  dev->dev.dma_mask = &dev->dev.coherent_dma_mask;

No, because that leads to a risk of DMA address truncation in hardware 
(and thus at worst random memory corruption) when drivers expect the 
default mask to be 32-bit and fail to explicitly set it as such.

> 3) ...

Remember when we found out how many drivers expect the default mask to 
be 32-bit and fail to explicitly set it as such, because they all broke 
when some chump set it to 0 in linux-next for a day? ;)

Robin.
```