[PATCH v2] arm64: do not set dma masks that device connection can't handle

[Nikita Yushchenko]

It is possible that device is capable of 64-bit DMA addresses, and
device driver tries to set wide DMA mask, but bridge or bus used to
connect device to the system can't handle wide addresses.

With swiotlb, memory above 4G still can be used by drivers for streaming
DMA, but *dev->mask and dev->dma_coherent_mask must still keep values
that hardware handles physically.

This patch enforces that. Based on original version by
Arnd Bergmann , extended with coherent mask hadnling.

Signed-off-by: Nikita Yushchenko 
CC: Arnd Bergmann 
---
Changes since v1:
- fixed issues noted by Sergei Shtylyov 
  - save mask, not size
  - remove doube empty line

 arch/arm64/Kconfig  |  3 +++
 arch/arm64/include/asm/device.h |  1 +
 arch/arm64/mm/dma-mapping.c | 51 +
 3 files changed, 55 insertions(+)

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 1117421..afb2c08 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -216,6 +216,9 @@ config NEED_DMA_MAP_STATE
 config NEED_SG_DMA_LENGTH
def_bool y
 
+config ARCH_HAS_DMA_SET_COHERENT_MASK
+   def_bool y
+
 config SMP
def_bool y
 
diff --git a/arch/arm64/include/asm/device.h b/arch/arm64/include/asm/device.h
index 243ef25..a57e7bb 100644
--- a/arch/arm64/include/asm/device.h
+++ b/arch/arm64/include/asm/device.h
@@ -22,6 +22,7 @@ struct dev_archdata {
void *iommu;/* private IOMMU data */
 #endif
bool dma_coherent;
+   u64 parent_dma_mask;
 };
 
 struct pdev_archdata {
diff --git a/arch/arm64/mm/dma-mapping.c b/arch/arm64/mm/dma-mapping.c
index e040827..5ab15ce 100644
--- a/arch/arm64/mm/dma-mapping.c
+++ b/arch/arm64/mm/dma-mapping.c
@@ -352,6 +352,30 @@ static int __swiotlb_dma_supported(struct device *hwdev, 
u64 mask)
return 1;
 }
 
+static int __swiotlb_set_dma_mask(struct device *dev, u64 mask)
+{
+   /* device is not DMA capable */
+   if (!dev->dma_mask)
+   return -EIO;
+
+   /* mask is below swiotlb bounce buffer, so fail */
+   if (!swiotlb_dma_supported(dev, mask))
+   return -EIO;
+
+   /*
+* because of the swiotlb, we can return success for
+* larger masks, but need to ensure that bounce buffers
+* are used above parent_dma_mask, so set that as
+* the effective mask.
+*/
+   if (mask > dev->archdata.parent_dma_mask)
+   mask = dev->archdata.parent_dma_mask;
+
+   *dev->dma_mask = mask;
+
+   return 0;
+}
+
 static struct dma_map_ops swiotlb_dma_ops = {
.alloc = __dma_alloc,
.free = __dma_free,
@@ -367,8 +391,23 @@ static struct dma_map_ops swiotlb_dma_ops = {
.sync_sg_for_device = __swiotlb_sync_sg_for_device,
.dma_supported = __swiotlb_dma_supported,
.mapping_error = swiotlb_dma_mapping_error,
+   .set_dma_mask = __swiotlb_set_dma_mask,
 };
 
+int dma_set_coherent_mask(struct device *dev, u64 mask)
+{
+   if (!dma_supported(dev, mask))
+   return -EIO;
+
+   if (get_dma_ops(dev) == _dma_ops &&
+   mask > dev->archdata.parent_dma_mask)
+   mask = dev->archdata.parent_dma_mask;
+
+   dev->coherent_dma_mask = mask;
+   return 0;
+}
+EXPORT_SYMBOL(dma_set_coherent_mask);
+
 static int __init atomic_pool_init(void)
 {
pgprot_t prot = __pgprot(PROT_NORMAL_NC);
@@ -958,6 +997,18 @@ void arch_setup_dma_ops(struct device *dev, u64 dma_base, 
u64 size,
if (!dev->archdata.dma_ops)
dev->archdata.dma_ops = _dma_ops;
 
+   /*
+* we don't yet support buses that have a non-zero mapping.
+*  Let's hope we won't need it
+*/
+   WARN_ON(dma_base != 0);
+
+   /*
+* Whatever the parent bus can set. A device must not set
+* a DMA mask larger than this.
+*/
+   dev->archdata.parent_dma_mask = size - 1;
+
dev->archdata.dma_coherent = coherent;
__iommu_setup_dma_ops(dev, dma_base, size, iommu);
 }
-- 
2.1.4

Re: [PATCH] arm64: do not set dma masks that device connection can't handle

[Nikita Yushchenko]

>> +if (mask > dev->archdata.parent_dma_mask)
>> +mask = dev->archdata.parent_dma_mask;
>> +
>> +
> 
>One empty line is enough...

Ok

>> +/*
>> + * Whatever the parent bus can set. A device must not set
>> + * a DMA mask larger than this.
>> + */
>> +dev->archdata.parent_dma_mask = size;
> 
>Not 'size - 1'?

Good question.

Indeed of_dma_configure() calls arch_setup_dma_ops() with size, not
mask. Which implies '-1' is needed here. Although better fix may be to
change caller side - to make DMA_BIT_MASK(64) case cleaner.

Will repost path.

Nikita

[PATCH] drivers: phy: constify phy_ops structures

[Bhumika Goyal]

Declare phy_ops structures as const as they are only passed as an
argument to the function devm_phy_create. This argument is of type const
struct phy_ops *, so phy_ops structures having this property can be
declared as const.
Done using Coccinelle:

@r1 disable optional_qualifier @
identifier i;
position p;
@@
static struct phy_ops i@p = {...};

@ok1@
identifier r1.i;
position p;
@@
devm_phy_create(...,@p)

@bad@
position p!={r1.p,ok1.p};
identifier r1.i;
@@
i@p

@depends on !bad disable optional_qualifier@
identifier r1.i;
@@
+const
struct phy_ops i;

File size details:

   textdata bss dec hex filename
   1504 264   01768 6e8 phy/phy-bcm-cygnus-pcie.o
   1576 192   01768 6e8 phy/phy-bcm-cygnus-pcie.o

   1083 264   01347 543 phy/phy-hi6220-usb.o
   1155 192   01347 543 phy/phy-hi6220-usb.o

   2767 264   03031 bd7 phy/phy-mt65xx-usb3.o
   2829 192   03021 bcd phy/phy-mt65xx-usb3.o

   2710 304   03014 bc6 phy/phy-rcar-gen3-usb2.o
   2766 240   03006 bbe phy/phy-rcar-gen3-usb2.o

Signed-off-by: Bhumika Goyal 
---
 drivers/phy/phy-bcm-cygnus-pcie.c | 2 +-
 drivers/phy/phy-hi6220-usb.c  | 2 +-
 drivers/phy/phy-mt65xx-usb3.c | 2 +-
 drivers/phy/phy-rcar-gen3-usb2.c  | 2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/phy/phy-bcm-cygnus-pcie.c 
b/drivers/phy/phy-bcm-cygnus-pcie.c
index 082c03f..0f4ac5d 100644
--- a/drivers/phy/phy-bcm-cygnus-pcie.c
+++ b/drivers/phy/phy-bcm-cygnus-pcie.c
@@ -114,7 +114,7 @@ static int cygnus_pcie_phy_power_off(struct phy *p)
return cygnus_pcie_power_config(phy, false);
 }
 
-static struct phy_ops cygnus_pcie_phy_ops = {
+static const struct phy_ops cygnus_pcie_phy_ops = {
.power_on = cygnus_pcie_phy_power_on,
.power_off = cygnus_pcie_phy_power_off,
.owner = THIS_MODULE,
diff --git a/drivers/phy/phy-hi6220-usb.c b/drivers/phy/phy-hi6220-usb.c
index b2141cb..398c102 100644
--- a/drivers/phy/phy-hi6220-usb.c
+++ b/drivers/phy/phy-hi6220-usb.c
@@ -112,7 +112,7 @@ static int hi6220_phy_exit(struct phy *phy)
return hi6220_phy_setup(priv, false);
 }
 
-static struct phy_ops hi6220_phy_ops = {
+static const struct phy_ops hi6220_phy_ops = {
.init   = hi6220_phy_start,
.exit   = hi6220_phy_exit,
.owner  = THIS_MODULE,
diff --git a/drivers/phy/phy-mt65xx-usb3.c b/drivers/phy/phy-mt65xx-usb3.c
index 4d85e73..d972067 100644
--- a/drivers/phy/phy-mt65xx-usb3.c
+++ b/drivers/phy/phy-mt65xx-usb3.c
@@ -506,7 +506,7 @@ static struct phy *mt65xx_phy_xlate(struct device *dev,
return instance->phy;
 }
 
-static struct phy_ops mt65xx_u3phy_ops = {
+static const struct phy_ops mt65xx_u3phy_ops = {
.init   = mt65xx_phy_init,
.exit   = mt65xx_phy_exit,
.power_on   = mt65xx_phy_power_on,
diff --git a/drivers/phy/phy-rcar-gen3-usb2.c b/drivers/phy/phy-rcar-gen3-usb2.c
index c63da1b..17be045 100644
--- a/drivers/phy/phy-rcar-gen3-usb2.c
+++ b/drivers/phy/phy-rcar-gen3-usb2.c
@@ -350,7 +350,7 @@ static int rcar_gen3_phy_usb2_power_off(struct phy *p)
return ret;
 }
 
-static struct phy_ops rcar_gen3_phy_usb2_ops = {
+static const struct phy_ops rcar_gen3_phy_usb2_ops = {
.init   = rcar_gen3_phy_usb2_init,
.exit   = rcar_gen3_phy_usb2_exit,
.power_on   = rcar_gen3_phy_usb2_power_on,
-- 
1.9.1