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.
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
