[PATCH v2 7/7] OF: Don't set default coherent DMA mask
From: robin.murphy@arm.com (Robin Murphy)
Date: 2018-07-27 20:42:14
Also in:
linux-acpi, linux-iommu
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.
quoted hunk ↗ jump to hunk
Could it be considered to do one the changes below? 1) assign mask in case of dtdiff --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.
quoted hunk ↗ jump to hunk
2) use BITS_PER_LONG for default DMA mask for of_platform devicesdiff --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.