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 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.
quoted hunk ↗ jump to hunk
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.