[RFC 0/5] DT property to reserve IOMMU regions

[RFC 0/5] DT property to reserve IOMMU regions

[Jitendra Bhivare]

ARM SoCs need a way to reserve IOMMU regions. Some address ranges used 
by devices sitting behind IOMMU are treated specially. To prevent these
ranges from getting mapped in IOVA space, use the existing framework and
code to reserve the ranges using DT properties.

This patch set is created against 4.15-rc7.

Jitendra Bhivare (5):
  dt-bindings: iommu: Add reserved-dma-region for IOMMU device
  dt-bindings: brcm: Add reserved-dma-region for iPROC
  dt-bindings: arm-smmu: Add reserved-msi-region
  iommu/of: Reserve IOMMU DMA regions using DT
  iommu/arm-smmu: Allow direct mapping for MSI region

 .../devicetree/bindings/iommu/arm,smmu.txt         |  12 +++
 Documentation/devicetree/bindings/iommu/iommu.txt  |  17 ++++
 .../devicetree/bindings/pci/brcm,iproc-pcie.txt    |   3 +
 drivers/iommu/arm-smmu.c                           |  14 ++-
 drivers/iommu/dma-iommu.c                          |   6 ++
 drivers/iommu/of_iommu.c                           | 100 +++++++++++----------
 include/linux/iommu.h                              |   3 +
 include/linux/of_iommu.h                           |  25 ++++--
 8 files changed, 126 insertions(+), 54 deletions(-)

-- 
2.7.4

[RFC 0/5] DT property to reserve IOMMU regions

[Jitendra Bhivare]

ARM SoCs need a way to reserve IOMMU regions. Some address ranges used 
by devices sitting behind IOMMU are treated specially. To prevent these
ranges from getting mapped in IOVA space, use the existing framework and
code to reserve the ranges using DT properties.

This patch set is created against 4.15-rc7.

Jitendra Bhivare (5):
  dt-bindings: iommu: Add reserved-dma-region for IOMMU device
  dt-bindings: brcm: Add reserved-dma-region for iPROC
  dt-bindings: arm-smmu: Add reserved-msi-region
  iommu/of: Reserve IOMMU DMA regions using DT
  iommu/arm-smmu: Allow direct mapping for MSI region

 .../devicetree/bindings/iommu/arm,smmu.txt         |  12 +++
 Documentation/devicetree/bindings/iommu/iommu.txt  |  17 ++++
 .../devicetree/bindings/pci/brcm,iproc-pcie.txt    |   3 +
 drivers/iommu/arm-smmu.c                           |  14 ++-
 drivers/iommu/dma-iommu.c                          |   6 ++
 drivers/iommu/of_iommu.c                           | 100 +++++++++++----------
 include/linux/iommu.h                              |   3 +
 include/linux/of_iommu.h                           |  25 ++++--
 8 files changed, 126 insertions(+), 54 deletions(-)

-- 
2.7.4

[RFC 1/5] dt-bindings: iommu: Add reserved-dma-region for IOMMU device

[Jitendra Bhivare]

Certain regions in IO virtual address space may need to be reserved to be
not used for devices configured behind IOMMU.

Add documentation of the property to allow such regions to be specified in
DT to be reserved using IOMMU_RESV_RESERVED type.

Reviewed-by: Ray Jui <ray.jui@broadcom.com>
Reviewed-by: Vikram Prakash <vikram.prakash@broadcom.com>
Reviewed-by: Scott Branden <scott.branden@broadcom.com>
Signed-off-by: Jitendra Bhivare <jitendra.bhivare@broadcom.com>
---
 Documentation/devicetree/bindings/iommu/iommu.txt | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/Documentation/devicetree/bindings/iommu/iommu.txt b/Documentation/devicetree/bindings/iommu/iommu.txt
index 5a8b462..5a58ef2 100644
--- a/Documentation/devicetree/bindings/iommu/iommu.txt
+++ b/Documentation/devicetree/bindings/iommu/iommu.txt
@@ -98,6 +98,20 @@ requirements of that use-case haven't been fully determined yet. Implementing
 this is therefore not recommended without further discussion and extension of
 this binding.
 
+Optional properties:
+--------------------
+- reserved-dma-region: This specifies DMA region to be reserved with specific
+  prot in IOVA space. It is in tuples of (busno,prot,bus_addr,size).
+
+- #region-address-cells: specifies number of cells needed to encode bus_addr
+
+- #region-size-cells: specifies number of cells needed to encode size
+
+Notes:
+======
+This can be applied to IOMMU master node or to children (such as PCI host
+bridges) on the bus behind IOMMU.
+
 
 Examples:
 =========
@@ -173,6 +187,9 @@ Multiple-master IOMMU with configurable DMA window:
 			 * master (i.e. the I/O virtual address space).
 			 */
 			#iommu-cells = <4>;
+			#region-address-cells = <2>;
+			#region-size-cells = <2>;
+			reserved-dma-region = <0x0 0x0 0x04 0x0 0x04 0x0>;
 		};
 
 		master {
-- 
2.7.4

[RFC 1/5] dt-bindings: iommu: Add reserved-dma-region for IOMMU device

[Jitendra Bhivare]

Certain regions in IO virtual address space may need to be reserved to be
not used for devices configured behind IOMMU.

Add documentation of the property to allow such regions to be specified in
DT to be reserved using IOMMU_RESV_RESERVED type.

Reviewed-by: Ray Jui <ray.jui@broadcom.com>
Reviewed-by: Vikram Prakash <vikram.prakash@broadcom.com>
Reviewed-by: Scott Branden <scott.branden@broadcom.com>
Signed-off-by: Jitendra Bhivare <jitendra.bhivare@broadcom.com>
---
 Documentation/devicetree/bindings/iommu/iommu.txt | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/Documentation/devicetree/bindings/iommu/iommu.txt b/Documentation/devicetree/bindings/iommu/iommu.txt
index 5a8b462..5a58ef2 100644
--- a/Documentation/devicetree/bindings/iommu/iommu.txt
+++ b/Documentation/devicetree/bindings/iommu/iommu.txt
@@ -98,6 +98,20 @@ requirements of that use-case haven't been fully determined yet. Implementing
 this is therefore not recommended without further discussion and extension of
 this binding.
 
+Optional properties:
+--------------------
+- reserved-dma-region: This specifies DMA region to be reserved with specific
+  prot in IOVA space. It is in tuples of (busno,prot,bus_addr,size).
+
+- #region-address-cells: specifies number of cells needed to encode bus_addr
+
+- #region-size-cells: specifies number of cells needed to encode size
+
+Notes:
+======
+This can be applied to IOMMU master node or to children (such as PCI host
+bridges) on the bus behind IOMMU.
+
 
 Examples:
 =========
@@ -173,6 +187,9 @@ Multiple-master IOMMU with configurable DMA window:
 			 * master (i.e. the I/O virtual address space).
 			 */
 			#iommu-cells = <4>;
+			#region-address-cells = <2>;
+			#region-size-cells = <2>;
+			reserved-dma-region = <0x0 0x0 0x04 0x0 0x04 0x0>;
 		};
 
 		master {
-- 
2.7.4

[RFC 2/5] dt-bindings: brcm: Add reserved-dma-region for iPROC

[Jitendra Bhivare]

With SoC wide DMA mask of 40-bit, the mappings for entire IOVA space can't
be specified in the PAXBv2 PCIe RC of SoC. The holes in IOVA space needs to
be reserved to prevent any IOVA allocations in those spaces.

reserved-dma-region property is added to specify the ranges which should
never be mapped and given to devices sitting behind.

Reviewed-by: Ray Jui <ray.jui@broadcom.com>
Reviewed-by: Vikram Prakash <vikram.prakash@broadcom.com>
Reviewed-by: Scott Branden <scott.branden@broadcom.com>
Signed-off-by: Jitendra Bhivare <jitendra.bhivare@broadcom.com>
---
 Documentation/devicetree/bindings/pci/brcm,iproc-pcie.txt | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/pci/brcm,iproc-pcie.txt b/Documentation/devicetree/bindings/pci/brcm,iproc-pcie.txt
index b8e48b4..3be0fe3 100644
--- a/Documentation/devicetree/bindings/pci/brcm,iproc-pcie.txt
+++ b/Documentation/devicetree/bindings/pci/brcm,iproc-pcie.txt
@@ -30,6 +30,9 @@ Optional properties:
 - dma-ranges: Some PAXB-based root complexes do not have inbound mapping done
   by the ASIC after power on reset.  In this case, SW is required to configure
 the mapping, based on inbound memory regions specified by this property.
+- reserved-dma-region: PAXBv2 with IOMMU enabled cannot provide mappings for
+  entire IOVA space specified by DMA mask. Hence this is used to reserve the
+  gaps in dma-ranges.
 
 - brcm,pcie-ob: Some iProc SoCs do not have the outbound address mapping done
 by the ASIC after power on reset. In this case, SW needs to configure it
-- 
2.7.4

[RFC 2/5] dt-bindings: brcm: Add reserved-dma-region for iPROC

[Jitendra Bhivare]

With SoC wide DMA mask of 40-bit, the mappings for entire IOVA space can't
be specified in the PAXBv2 PCIe RC of SoC. The holes in IOVA space needs to
be reserved to prevent any IOVA allocations in those spaces.

reserved-dma-region property is added to specify the ranges which should
never be mapped and given to devices sitting behind.

Reviewed-by: Ray Jui <ray.jui@broadcom.com>
Reviewed-by: Vikram Prakash <vikram.prakash@broadcom.com>
Reviewed-by: Scott Branden <scott.branden@broadcom.com>
Signed-off-by: Jitendra Bhivare <jitendra.bhivare@broadcom.com>
---
 Documentation/devicetree/bindings/pci/brcm,iproc-pcie.txt | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/pci/brcm,iproc-pcie.txt b/Documentation/devicetree/bindings/pci/brcm,iproc-pcie.txt
index b8e48b4..3be0fe3 100644
--- a/Documentation/devicetree/bindings/pci/brcm,iproc-pcie.txt
+++ b/Documentation/devicetree/bindings/pci/brcm,iproc-pcie.txt
@@ -30,6 +30,9 @@ Optional properties:
 - dma-ranges: Some PAXB-based root complexes do not have inbound mapping done
   by the ASIC after power on reset.  In this case, SW is required to configure
 the mapping, based on inbound memory regions specified by this property.
+- reserved-dma-region: PAXBv2 with IOMMU enabled cannot provide mappings for
+  entire IOVA space specified by DMA mask. Hence this is used to reserve the
+  gaps in dma-ranges.
 
 - brcm,pcie-ob: Some iProc SoCs do not have the outbound address mapping done
 by the ASIC after power on reset. In this case, SW needs to configure it
-- 
2.7.4

[RFC 3/5] dt-bindings: arm-smmu: Add reserved-msi-region

[Jitendra Bhivare]

iPROC SoC has a special device window to treat GICv3 ITS MSI.

Current code maps MSI to IOVA space. Add SMMU node property to use
direct mappings for MSI region.

This property is read and reserved when arm_smmu_get_resv_regions
gets called.

Signed-off-by: Jitendra Bhivare <jitendra.bhivare@broadcom.com>
---
 Documentation/devicetree/bindings/iommu/arm,smmu.txt | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.txt b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
index 8a6ffce..13fa2b9 100644
--- a/Documentation/devicetree/bindings/iommu/arm,smmu.txt
+++ b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
@@ -71,6 +71,15 @@ conditions.
                   or using stream matching with #iommu-cells = <2>, and
                   may be ignored if present in such cases.
 
+- reserved-msi-region: MSI region to be reserved with specific prot in IOVA
+		  space for direct mapping. The region is specified in tuple
+		  of (busno,prot,bus_addr,size).
+
+- #region-address-cells: specifies number of cells needed to encode bus_addr
+
+- #region-size-cells: specifies number of cells needed to encode size
+
+
 ** Deprecated properties:
 
 - mmu-masters (deprecated in favour of the generic "iommus" binding) :
@@ -95,6 +104,9 @@ conditions.
                              <0 36 4>,
                              <0 37 4>;
                 #iommu-cells = <1>;
+		#region-address-cells = <1>;
+		#region-size-cells = <1>;
+		reserved-msi-region = <0x0 0x1a 0x63c3000 0x8000>;
         };
 
         /* device with two stream IDs, 0 and 7 */
-- 
2.7.4

[RFC 3/5] dt-bindings: arm-smmu: Add reserved-msi-region

[Jitendra Bhivare]

iPROC SoC has a special device window to treat GICv3 ITS MSI.

Current code maps MSI to IOVA space. Add SMMU node property to use
direct mappings for MSI region.

This property is read and reserved when arm_smmu_get_resv_regions
gets called.

Signed-off-by: Jitendra Bhivare <jitendra.bhivare@broadcom.com>
---
 Documentation/devicetree/bindings/iommu/arm,smmu.txt | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.txt b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
index 8a6ffce..13fa2b9 100644
--- a/Documentation/devicetree/bindings/iommu/arm,smmu.txt
+++ b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
@@ -71,6 +71,15 @@ conditions.
                   or using stream matching with #iommu-cells = <2>, and
                   may be ignored if present in such cases.
 
+- reserved-msi-region: MSI region to be reserved with specific prot in IOVA
+		  space for direct mapping. The region is specified in tuple
+		  of (busno,prot,bus_addr,size).
+
+- #region-address-cells: specifies number of cells needed to encode bus_addr
+
+- #region-size-cells: specifies number of cells needed to encode size
+
+
 ** Deprecated properties:
 
 - mmu-masters (deprecated in favour of the generic "iommus" binding) :
@@ -95,6 +104,9 @@ conditions.
                              <0 36 4>,
                              <0 37 4>;
                 #iommu-cells = <1>;
+		#region-address-cells = <1>;
+		#region-size-cells = <1>;
+		reserved-msi-region = <0x0 0x1a 0x63c3000 0x8000>;
         };
 
         /* device with two stream IDs, 0 and 7 */
-- 
2.7.4

[RFC 4/5] iommu/of: Reserve IOMMU DMA regions using DT

[Jitendra Bhivare]

iPROC SoC provides restricted windows to access sparsed memory as per ARM
memory map to the devices sitting behind IOMMU. The windows cover enough
DDR space but not the entire 40-bit 1TB space used by IOMMU code as per
the DMA mask set.

Current IOMMU code does not provide a hook to reserve these holes. Use
defined but unused of_get_dma_window to get reserved DMA regions from DT.

To reserve the regions for IOVA allocation, ranges can be specified in DT
as <busno prot bus_addr size>. These regions are marked as
IOMMU_RESV_RESERVED when device behind SMMU gets added and
arm_smmu_get_resv_regions gets called.

This provision is also used in reserving region for GICv3 ITS using
IOMMU_RESV_DIRECT as it falls in special device window.

Reviewed-by: Scott Branden <scott.branden@broadcom.com>
Signed-off-by: Jitendra Bhivare <jitendra.bhivare@broadcom.com>
---
 drivers/iommu/dma-iommu.c |   6 +++
 drivers/iommu/of_iommu.c  | 100 ++++++++++++++++++++++++----------------------
 include/linux/iommu.h     |   3 ++
 include/linux/of_iommu.h  |  25 +++++++++---
 4 files changed, 81 insertions(+), 53 deletions(-)

diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index 25914d3..433d427 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -27,6 +27,7 @@
 #include <linux/iova.h>
 #include <linux/irq.h>
 #include <linux/mm.h>
+#include <linux/of_iommu.h>
 #include <linux/pci.h>
 #include <linux/scatterlist.h>
 #include <linux/vmalloc.h>
@@ -173,6 +174,7 @@ void iommu_dma_get_resv_regions(struct device *dev, struct list_head *list)
 {
 	struct pci_host_bridge *bridge;
 	struct resource_entry *window;
+	struct device_node *np;
 
 	if (!dev_is_pci(dev))
 		return;
@@ -195,6 +197,10 @@ void iommu_dma_get_resv_regions(struct device *dev, struct list_head *list)
 
 		list_add_tail(&region->list, list);
 	}
+	/* check platform device representing bridge for reserved DMA windows */
+	np = bridge->dev.parent->of_node;
+	if (np)
+		of_iommu_resv_dma_regions(np, list);
 }
 EXPORT_SYMBOL(iommu_dma_get_resv_regions);
 
diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
index 50947eb..cebcbaa 100644
--- a/drivers/iommu/of_iommu.c
+++ b/drivers/iommu/of_iommu.c
@@ -31,72 +31,78 @@ static const struct of_device_id __iommu_of_table_sentinel
 	__used __section(__iommu_of_table_end);
 
 /**
- * of_get_dma_window - Parse *dma-window property and returns 0 if found.
+ * of_get_resv_region - Parse reserved-*-region property and returns 0 if found.
  *
  * @dn: device node
- * @prefix: prefix for property name if any
+ * @region: region for property name if any
  * @index: index to start to parse
- * @busno: Returns busno if supported. Otherwise pass NULL
- * @addr: Returns address that DMA starts
- * @size: Returns the range that DMA can handle
+ * @of_region: returns read of_iommu_resv_region <busno prot bus_addr size>
  *
- * This supports different formats flexibly. "prefix" can be
- * configured if any. "busno" and "index" are optionally
- * specified. Set 0(or NULL) if not used.
+ * This supports different formats using "region" configured if any.
  */
-int of_get_dma_window(struct device_node *dn, const char *prefix, int index,
-		      unsigned long *busno, dma_addr_t *addr, size_t *size)
+int of_get_resv_region(struct device_node *dn, const char *region, int *index,
+		      struct of_iommu_resv_region *of_region)
 {
-	const __be32 *dma_window, *end;
-	int bytes, cur_index = 0;
-	char propname[NAME_MAX], addrname[NAME_MAX], sizename[NAME_MAX];
+	char propname[NAME_MAX];
+	int na, ns, len, pos;
+	const __be32 *prop;
 
-	if (!dn || !addr || !size)
+	if (!dn || !of_region || !index)
 		return -EINVAL;
 
-	if (!prefix)
-		prefix = "";
+	if (!region)
+		region = "";
 
-	snprintf(propname, sizeof(propname), "%sdma-window", prefix);
-	snprintf(addrname, sizeof(addrname), "%s#dma-address-cells", prefix);
-	snprintf(sizename, sizeof(sizename), "%s#dma-size-cells", prefix);
+	prop = of_get_property(dn, "#region-address-cells", NULL);
+	na = prop ? be32_to_cpup(prop) : of_n_addr_cells(dn);
+	prop = of_get_property(dn, "#region-size-cells", NULL);
+	ns = prop ? be32_to_cpup(prop) : of_n_size_cells(dn);
 
-	dma_window = of_get_property(dn, propname, &bytes);
-	if (!dma_window)
-		return -ENODEV;
-	end = dma_window + bytes / sizeof(*dma_window);
+	snprintf(propname, sizeof(propname), "reserved-%s-region", region);
+	prop = of_get_property(dn, propname, &len);
+	if (!prop)
+		return -ENOENT;
 
-	while (dma_window < end) {
-		u32 cells;
-		const void *prop;
-
-		/* busno is one cell if supported */
-		if (busno)
-			*busno = be32_to_cpup(dma_window++);
+	len /= sizeof(*prop);
+	pos = *index;
+	/* prot and busno takes one cell each */
+	if (pos >= len || (len - pos) % (na + ns + 2))
+		return -EINVAL;
 
-		prop = of_get_property(dn, addrname, NULL);
-		if (!prop)
-			prop = of_get_property(dn, "#address-cells", NULL);
+	memset(of_region, 0, sizeof(*of_region));
+	of_region->busno = be32_to_cpup(prop + pos++);
+	of_region->prot = be32_to_cpup(prop + pos++);
+	if (of_region->prot && !(of_region->prot & IOMMU_PROT_FLAGS))
+		return -EINVAL;
 
-		cells = prop ? be32_to_cpup(prop) : of_n_addr_cells(dn);
-		if (!cells)
-			return -EINVAL;
-		*addr = of_read_number(dma_window, cells);
-		dma_window += cells;
+	of_region->bus_addr = of_read_number(prop + pos, na);
+	pos += na;
+	of_region->size = of_read_number(prop + pos, ns);
+	pos += ns;
+	*index = pos;
+	return 0;
 
-		prop = of_get_property(dn, sizename, NULL);
-		cells = prop ? be32_to_cpup(prop) : of_n_size_cells(dn);
-		if (!cells)
-			return -EINVAL;
-		*size = of_read_number(dma_window, cells);
-		dma_window += cells;
+}
+EXPORT_SYMBOL_GPL(of_get_resv_region);
 
-		if (cur_index++ == index)
+void of_iommu_resv_dma_regions(struct device_node *np, struct list_head *list)
+{
+	struct of_iommu_resv_region of_region;
+	struct iommu_resv_region *region;
+	int index = 0;
+
+	while (!of_get_resv_region(np, "dma", &index, &of_region)) {
+		region = iommu_alloc_resv_region(of_region.bus_addr,
+						 of_region.size,
+						 of_region.prot,
+						 IOMMU_RESV_RESERVED);
+		if (!region)
 			break;
+
+		list_add_tail(&region->list, list);
 	}
-	return 0;
 }
-EXPORT_SYMBOL_GPL(of_get_dma_window);
+EXPORT_SYMBOL_GPL(of_iommu_resv_dma_regions);
 
 static bool of_iommu_driver_present(struct device_node *np)
 {
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 41b8c57..56bf906 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -41,6 +41,9 @@
  * if the IOMMU page table format is equivalent.
  */
 #define IOMMU_PRIV	(1 << 5)
+#define IOMMU_PROT_FLAGS (IOMMU_READ | IOMMU_WRITE | \
+			  IOMMU_CACHE | IOMMU_NOEXEC | IOMMU_MMIO)
+
 
 struct iommu_ops;
 struct iommu_group;
diff --git a/include/linux/of_iommu.h b/include/linux/of_iommu.h
index cddfaff..b891bac 100644
--- a/include/linux/of_iommu.h
+++ b/include/linux/of_iommu.h
@@ -6,20 +6,27 @@
 #include <linux/iommu.h>
 #include <linux/of.h>
 
+struct of_iommu_resv_region {
+	u32 busno;
+	u32 prot;
+	u64 bus_addr;
+	u64 size;
+};
+
 #ifdef CONFIG_OF_IOMMU
 
-extern int of_get_dma_window(struct device_node *dn, const char *prefix,
-			     int index, unsigned long *busno, dma_addr_t *addr,
-			     size_t *size);
+int of_get_resv_region(struct device_node *dn, const char *region, int *index,
+		      struct of_iommu_resv_region *of_region);
 
 extern const struct iommu_ops *of_iommu_configure(struct device *dev,
 					struct device_node *master_np);
 
+void of_iommu_resv_dma_regions(struct device_node *np, struct list_head *list);
+
 #else
 
-static inline int of_get_dma_window(struct device_node *dn, const char *prefix,
-			    int index, unsigned long *busno, dma_addr_t *addr,
-			    size_t *size)
+static inline int of_get_resv_region(struct device_node *dn, const char *region,
+			int *index, struct of_iommu_resv_region *of_region)
 {
 	return -EINVAL;
 }
@@ -30,6 +37,12 @@ static inline const struct iommu_ops *of_iommu_configure(struct device *dev,
 	return NULL;
 }
 
+static inline int of_iommu_resv_dma_regions(struct device_node *np,
+					    struct list_head *list)
+{
+	return -EINVAL;
+}
+
 #endif	/* CONFIG_OF_IOMMU */
 
 extern struct of_device_id __iommu_of_table;
-- 
2.7.4

[RFC 4/5] iommu/of: Reserve IOMMU DMA regions using DT

[Jitendra Bhivare]

iPROC SoC provides restricted windows to access sparsed memory as per ARM
memory map to the devices sitting behind IOMMU. The windows cover enough
DDR space but not the entire 40-bit 1TB space used by IOMMU code as per
the DMA mask set.

Current IOMMU code does not provide a hook to reserve these holes. Use
defined but unused of_get_dma_window to get reserved DMA regions from DT.

To reserve the regions for IOVA allocation, ranges can be specified in DT
as <busno prot bus_addr size>. These regions are marked as
IOMMU_RESV_RESERVED when device behind SMMU gets added and
arm_smmu_get_resv_regions gets called.

This provision is also used in reserving region for GICv3 ITS using
IOMMU_RESV_DIRECT as it falls in special device window.

Reviewed-by: Scott Branden <scott.branden@broadcom.com>
Signed-off-by: Jitendra Bhivare <jitendra.bhivare@broadcom.com>
---
 drivers/iommu/dma-iommu.c |   6 +++
 drivers/iommu/of_iommu.c  | 100 ++++++++++++++++++++++++----------------------
 include/linux/iommu.h     |   3 ++
 include/linux/of_iommu.h  |  25 +++++++++---
 4 files changed, 81 insertions(+), 53 deletions(-)

diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index 25914d3..433d427 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -27,6 +27,7 @@
 #include <linux/iova.h>
 #include <linux/irq.h>
 #include <linux/mm.h>
+#include <linux/of_iommu.h>
 #include <linux/pci.h>
 #include <linux/scatterlist.h>
 #include <linux/vmalloc.h>
@@ -173,6 +174,7 @@ void iommu_dma_get_resv_regions(struct device *dev, struct list_head *list)
 {
 	struct pci_host_bridge *bridge;
 	struct resource_entry *window;
+	struct device_node *np;
 
 	if (!dev_is_pci(dev))
 		return;
@@ -195,6 +197,10 @@ void iommu_dma_get_resv_regions(struct device *dev, struct list_head *list)
 
 		list_add_tail(&region->list, list);
 	}
+	/* check platform device representing bridge for reserved DMA windows */
+	np = bridge->dev.parent->of_node;
+	if (np)
+		of_iommu_resv_dma_regions(np, list);
 }
 EXPORT_SYMBOL(iommu_dma_get_resv_regions);
 
diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
index 50947eb..cebcbaa 100644
--- a/drivers/iommu/of_iommu.c
+++ b/drivers/iommu/of_iommu.c
@@ -31,72 +31,78 @@ static const struct of_device_id __iommu_of_table_sentinel
 	__used __section(__iommu_of_table_end);
 
 /**
- * of_get_dma_window - Parse *dma-window property and returns 0 if found.
+ * of_get_resv_region - Parse reserved-*-region property and returns 0 if found.
  *
  * @dn: device node
- * @prefix: prefix for property name if any
+ * @region: region for property name if any
  * @index: index to start to parse
- * @busno: Returns busno if supported. Otherwise pass NULL
- * @addr: Returns address that DMA starts
- * @size: Returns the range that DMA can handle
+ * @of_region: returns read of_iommu_resv_region <busno prot bus_addr size>
  *
- * This supports different formats flexibly. "prefix" can be
- * configured if any. "busno" and "index" are optionally
- * specified. Set 0(or NULL) if not used.
+ * This supports different formats using "region" configured if any.
  */
-int of_get_dma_window(struct device_node *dn, const char *prefix, int index,
-		      unsigned long *busno, dma_addr_t *addr, size_t *size)
+int of_get_resv_region(struct device_node *dn, const char *region, int *index,
+		      struct of_iommu_resv_region *of_region)
 {
-	const __be32 *dma_window, *end;
-	int bytes, cur_index = 0;
-	char propname[NAME_MAX], addrname[NAME_MAX], sizename[NAME_MAX];
+	char propname[NAME_MAX];
+	int na, ns, len, pos;
+	const __be32 *prop;
 
-	if (!dn || !addr || !size)
+	if (!dn || !of_region || !index)
 		return -EINVAL;
 
-	if (!prefix)
-		prefix = "";
+	if (!region)
+		region = "";
 
-	snprintf(propname, sizeof(propname), "%sdma-window", prefix);
-	snprintf(addrname, sizeof(addrname), "%s#dma-address-cells", prefix);
-	snprintf(sizename, sizeof(sizename), "%s#dma-size-cells", prefix);
+	prop = of_get_property(dn, "#region-address-cells", NULL);
+	na = prop ? be32_to_cpup(prop) : of_n_addr_cells(dn);
+	prop = of_get_property(dn, "#region-size-cells", NULL);
+	ns = prop ? be32_to_cpup(prop) : of_n_size_cells(dn);
 
-	dma_window = of_get_property(dn, propname, &bytes);
-	if (!dma_window)
-		return -ENODEV;
-	end = dma_window + bytes / sizeof(*dma_window);
+	snprintf(propname, sizeof(propname), "reserved-%s-region", region);
+	prop = of_get_property(dn, propname, &len);
+	if (!prop)
+		return -ENOENT;
 
-	while (dma_window < end) {
-		u32 cells;
-		const void *prop;
-
-		/* busno is one cell if supported */
-		if (busno)
-			*busno = be32_to_cpup(dma_window++);
+	len /= sizeof(*prop);
+	pos = *index;
+	/* prot and busno takes one cell each */
+	if (pos >= len || (len - pos) % (na + ns + 2))
+		return -EINVAL;
 
-		prop = of_get_property(dn, addrname, NULL);
-		if (!prop)
-			prop = of_get_property(dn, "#address-cells", NULL);
+	memset(of_region, 0, sizeof(*of_region));
+	of_region->busno = be32_to_cpup(prop + pos++);
+	of_region->prot = be32_to_cpup(prop + pos++);
+	if (of_region->prot && !(of_region->prot & IOMMU_PROT_FLAGS))
+		return -EINVAL;
 
-		cells = prop ? be32_to_cpup(prop) : of_n_addr_cells(dn);
-		if (!cells)
-			return -EINVAL;
-		*addr = of_read_number(dma_window, cells);
-		dma_window += cells;
+	of_region->bus_addr = of_read_number(prop + pos, na);
+	pos += na;
+	of_region->size = of_read_number(prop + pos, ns);
+	pos += ns;
+	*index = pos;
+	return 0;
 
-		prop = of_get_property(dn, sizename, NULL);
-		cells = prop ? be32_to_cpup(prop) : of_n_size_cells(dn);
-		if (!cells)
-			return -EINVAL;
-		*size = of_read_number(dma_window, cells);
-		dma_window += cells;
+}
+EXPORT_SYMBOL_GPL(of_get_resv_region);
 
-		if (cur_index++ == index)
+void of_iommu_resv_dma_regions(struct device_node *np, struct list_head *list)
+{
+	struct of_iommu_resv_region of_region;
+	struct iommu_resv_region *region;
+	int index = 0;
+
+	while (!of_get_resv_region(np, "dma", &index, &of_region)) {
+		region = iommu_alloc_resv_region(of_region.bus_addr,
+						 of_region.size,
+						 of_region.prot,
+						 IOMMU_RESV_RESERVED);
+		if (!region)
 			break;
+
+		list_add_tail(&region->list, list);
 	}
-	return 0;
 }
-EXPORT_SYMBOL_GPL(of_get_dma_window);
+EXPORT_SYMBOL_GPL(of_iommu_resv_dma_regions);
 
 static bool of_iommu_driver_present(struct device_node *np)
 {
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 41b8c57..56bf906 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -41,6 +41,9 @@
  * if the IOMMU page table format is equivalent.
  */
 #define IOMMU_PRIV	(1 << 5)
+#define IOMMU_PROT_FLAGS (IOMMU_READ | IOMMU_WRITE | \
+			  IOMMU_CACHE | IOMMU_NOEXEC | IOMMU_MMIO)
+
 
 struct iommu_ops;
 struct iommu_group;
diff --git a/include/linux/of_iommu.h b/include/linux/of_iommu.h
index cddfaff..b891bac 100644
--- a/include/linux/of_iommu.h
+++ b/include/linux/of_iommu.h
@@ -6,20 +6,27 @@
 #include <linux/iommu.h>
 #include <linux/of.h>
 
+struct of_iommu_resv_region {
+	u32 busno;
+	u32 prot;
+	u64 bus_addr;
+	u64 size;
+};
+
 #ifdef CONFIG_OF_IOMMU
 
-extern int of_get_dma_window(struct device_node *dn, const char *prefix,
-			     int index, unsigned long *busno, dma_addr_t *addr,
-			     size_t *size);
+int of_get_resv_region(struct device_node *dn, const char *region, int *index,
+		      struct of_iommu_resv_region *of_region);
 
 extern const struct iommu_ops *of_iommu_configure(struct device *dev,
 					struct device_node *master_np);
 
+void of_iommu_resv_dma_regions(struct device_node *np, struct list_head *list);
+
 #else
 
-static inline int of_get_dma_window(struct device_node *dn, const char *prefix,
-			    int index, unsigned long *busno, dma_addr_t *addr,
-			    size_t *size)
+static inline int of_get_resv_region(struct device_node *dn, const char *region,
+			int *index, struct of_iommu_resv_region *of_region)
 {
 	return -EINVAL;
 }
@@ -30,6 +37,12 @@ static inline const struct iommu_ops *of_iommu_configure(struct device *dev,
 	return NULL;
 }
 
+static inline int of_iommu_resv_dma_regions(struct device_node *np,
+					    struct list_head *list)
+{
+	return -EINVAL;
+}
+
 #endif	/* CONFIG_OF_IOMMU */
 
 extern struct of_device_id __iommu_of_table;
-- 
2.7.4

[RFC 5/5] iommu/arm-smmu: Allow direct mapping for MSI region

[Jitendra Bhivare]

Current ARM SMMU virtualizes MSI addresses to IOVA space.

In iProc SoCs, MSI needs to be steered. Allow reserving MSI address range
using DT node entry as direct region.

Signed-off-by: Jitendra Bhivare <jitendra.bhivare@broadcom.com>
---
 drivers/iommu/arm-smmu.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index 78d4c6b..7b34980 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -1533,8 +1533,11 @@ static int arm_smmu_of_xlate(struct device *dev, struct of_phandle_args *args)
 static void arm_smmu_get_resv_regions(struct device *dev,
 				      struct list_head *head)
 {
+	struct iommu_fwspec *fwspec = dev->iommu_fwspec;
+	struct arm_smmu_device *smmu = fwspec_smmu(fwspec);
 	struct iommu_resv_region *region;
-	int prot = IOMMU_WRITE | IOMMU_NOEXEC | IOMMU_MMIO;
+	struct of_iommu_resv_region of_region;
+	int index = 0, prot = IOMMU_WRITE | IOMMU_NOEXEC | IOMMU_MMIO;
 
 	region = iommu_alloc_resv_region(MSI_IOVA_BASE, MSI_IOVA_LENGTH,
 					 prot, IOMMU_RESV_SW_MSI);
@@ -1543,6 +1546,15 @@ static void arm_smmu_get_resv_regions(struct device *dev,
 
 	list_add_tail(&region->list, head);
 
+	if (!of_get_resv_region(smmu->dev->of_node, "msi", &index,
+				&of_region)) {
+		region = iommu_alloc_resv_region(of_region.bus_addr,
+						 of_region.size,
+						 of_region.prot,
+						 IOMMU_RESV_DIRECT);
+		if (region)
+			list_add_tail(&region->list, head);
+	}
 	iommu_dma_get_resv_regions(dev, head);
 }
 
-- 
2.7.4

[RFC 5/5] iommu/arm-smmu: Allow direct mapping for MSI region

[Jitendra Bhivare]

Current ARM SMMU virtualizes MSI addresses to IOVA space.

In iProc SoCs, MSI needs to be steered. Allow reserving MSI address range
using DT node entry as direct region.

Signed-off-by: Jitendra Bhivare <jitendra.bhivare@broadcom.com>
---
 drivers/iommu/arm-smmu.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index 78d4c6b..7b34980 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -1533,8 +1533,11 @@ static int arm_smmu_of_xlate(struct device *dev, struct of_phandle_args *args)
 static void arm_smmu_get_resv_regions(struct device *dev,
 				      struct list_head *head)
 {
+	struct iommu_fwspec *fwspec = dev->iommu_fwspec;
+	struct arm_smmu_device *smmu = fwspec_smmu(fwspec);
 	struct iommu_resv_region *region;
-	int prot = IOMMU_WRITE | IOMMU_NOEXEC | IOMMU_MMIO;
+	struct of_iommu_resv_region of_region;
+	int index = 0, prot = IOMMU_WRITE | IOMMU_NOEXEC | IOMMU_MMIO;
 
 	region = iommu_alloc_resv_region(MSI_IOVA_BASE, MSI_IOVA_LENGTH,
 					 prot, IOMMU_RESV_SW_MSI);
@@ -1543,6 +1546,15 @@ static void arm_smmu_get_resv_regions(struct device *dev,
 
 	list_add_tail(&region->list, head);
 
+	if (!of_get_resv_region(smmu->dev->of_node, "msi", &index,
+				&of_region)) {
+		region = iommu_alloc_resv_region(of_region.bus_addr,
+						 of_region.size,
+						 of_region.prot,
+						 IOMMU_RESV_DIRECT);
+		if (region)
+			list_add_tail(&region->list, head);
+	}
 	iommu_dma_get_resv_regions(dev, head);
 }
 
-- 
2.7.4

Re: [RFC 1/5] dt-bindings: iommu: Add reserved-dma-region for IOMMU device

[Robin Murphy]

On 06/03/18 04:59, Jitendra Bhivare wrote:
> Certain regions in IO virtual address space may need to be reserved to be
> not used for devices configured behind IOMMU.
> 
> Add documentation of the property to allow such regions to be specified in
> DT to be reserved using IOMMU_RESV_RESERVED type.

That sounds like software policy; DT is for describing hardware.

> Reviewed-by: Ray Jui <ray.jui-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
> Reviewed-by: Vikram Prakash <vikram.prakash-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
> Reviewed-by: Scott Branden <scott.branden-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
> Signed-off-by: Jitendra Bhivare <jitendra.bhivare-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
> ---
>   Documentation/devicetree/bindings/iommu/iommu.txt | 17 +++++++++++++++++
>   1 file changed, 17 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/iommu/iommu.txt b/Documentation/devicetree/bindings/iommu/iommu.txt
> index 5a8b462..5a58ef2 100644
> --- a/Documentation/devicetree/bindings/iommu/iommu.txt
> +++ b/Documentation/devicetree/bindings/iommu/iommu.txt
> @@ -98,6 +98,20 @@ requirements of that use-case haven't been fully determined yet. Implementing
>   this is therefore not recommended without further discussion and extension of
>   this binding.
>   
> +Optional properties:
> +--------------------
> +- reserved-dma-region: This specifies DMA region to be reserved with specific
> +  prot in IOVA space. It is in tuples of (busno,prot,bus_addr,size).

What do busno and prot actually mean, and what are valid values for them?

Robin.

> +- #region-address-cells: specifies number of cells needed to encode bus_addr
> +
> +- #region-size-cells: specifies number of cells needed to encode size
> +
> +Notes:
> +======
> +This can be applied to IOMMU master node or to children (such as PCI host
> +bridges) on the bus behind IOMMU.
> +
>   
>   Examples:
>   =========
> @@ -173,6 +187,9 @@ Multiple-master IOMMU with configurable DMA window:
>   			 * master (i.e. the I/O virtual address space).
>   			 */
>   			#iommu-cells = <4>;
> +			#region-address-cells = <2>;
> +			#region-size-cells = <2>;
> +			reserved-dma-region = <0x0 0x0 0x04 0x0 0x04 0x0>;
>   		};
>   
>   		master {
> 

[RFC 1/5] dt-bindings: iommu: Add reserved-dma-region for IOMMU device

[Robin Murphy]

On 06/03/18 04:59, Jitendra Bhivare wrote:
> Certain regions in IO virtual address space may need to be reserved to be
> not used for devices configured behind IOMMU.
> 
> Add documentation of the property to allow such regions to be specified in
> DT to be reserved using IOMMU_RESV_RESERVED type.

That sounds like software policy; DT is for describing hardware.

> Reviewed-by: Ray Jui <ray.jui@broadcom.com>
> Reviewed-by: Vikram Prakash <vikram.prakash@broadcom.com>
> Reviewed-by: Scott Branden <scott.branden@broadcom.com>
> Signed-off-by: Jitendra Bhivare <jitendra.bhivare@broadcom.com>
> ---
>   Documentation/devicetree/bindings/iommu/iommu.txt | 17 +++++++++++++++++
>   1 file changed, 17 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/iommu/iommu.txt b/Documentation/devicetree/bindings/iommu/iommu.txt
> index 5a8b462..5a58ef2 100644
> --- a/Documentation/devicetree/bindings/iommu/iommu.txt
> +++ b/Documentation/devicetree/bindings/iommu/iommu.txt
> @@ -98,6 +98,20 @@ requirements of that use-case haven't been fully determined yet. Implementing
>   this is therefore not recommended without further discussion and extension of
>   this binding.
>   
> +Optional properties:
> +--------------------
> +- reserved-dma-region: This specifies DMA region to be reserved with specific
> +  prot in IOVA space. It is in tuples of (busno,prot,bus_addr,size).

What do busno and prot actually mean, and what are valid values for them?

Robin.

> +- #region-address-cells: specifies number of cells needed to encode bus_addr
> +
> +- #region-size-cells: specifies number of cells needed to encode size
> +
> +Notes:
> +======
> +This can be applied to IOMMU master node or to children (such as PCI host
> +bridges) on the bus behind IOMMU.
> +
>   
>   Examples:
>   =========
> @@ -173,6 +187,9 @@ Multiple-master IOMMU with configurable DMA window:
>   			 * master (i.e. the I/O virtual address space).
>   			 */
>   			#iommu-cells = <4>;
> +			#region-address-cells = <2>;
> +			#region-size-cells = <2>;
> +			reserved-dma-region = <0x0 0x0 0x04 0x0 0x04 0x0>;
>   		};
>   
>   		master {
> 

Re: [RFC 2/5] dt-bindings: brcm: Add reserved-dma-region for iPROC

[Robin Murphy]

On 06/03/18 04:59, Jitendra Bhivare wrote:
> With SoC wide DMA mask of 40-bit, the mappings for entire IOVA space can't
> be specified in the PAXBv2 PCIe RC of SoC. The holes in IOVA space needs to
> be reserved to prevent any IOVA allocations in those spaces.

Can you clarify why? If this is the PCI inbound window thing again, let 
me say once again that "dma-ranges" is the appropriate way for DT to 
describe the hardware.

Robin.

> reserved-dma-region property is added to specify the ranges which should
> never be mapped and given to devices sitting behind.
> 
> Reviewed-by: Ray Jui <ray.jui-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
> Reviewed-by: Vikram Prakash <vikram.prakash-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
> Reviewed-by: Scott Branden <scott.branden-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
> Signed-off-by: Jitendra Bhivare <jitendra.bhivare-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
> ---
>   Documentation/devicetree/bindings/pci/brcm,iproc-pcie.txt | 3 +++
>   1 file changed, 3 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/pci/brcm,iproc-pcie.txt b/Documentation/devicetree/bindings/pci/brcm,iproc-pcie.txt
> index b8e48b4..3be0fe3 100644
> --- a/Documentation/devicetree/bindings/pci/brcm,iproc-pcie.txt
> +++ b/Documentation/devicetree/bindings/pci/brcm,iproc-pcie.txt
> @@ -30,6 +30,9 @@ Optional properties:
>   - dma-ranges: Some PAXB-based root complexes do not have inbound mapping done
>     by the ASIC after power on reset.  In this case, SW is required to configure
>   the mapping, based on inbound memory regions specified by this property.
> +- reserved-dma-region: PAXBv2 with IOMMU enabled cannot provide mappings for
> +  entire IOVA space specified by DMA mask. Hence this is used to reserve the
> +  gaps in dma-ranges.
>   
>   - brcm,pcie-ob: Some iProc SoCs do not have the outbound address mapping done
>   by the ASIC after power on reset. In this case, SW needs to configure it
> 

[RFC 2/5] dt-bindings: brcm: Add reserved-dma-region for iPROC

[Robin Murphy]

On 06/03/18 04:59, Jitendra Bhivare wrote:
> With SoC wide DMA mask of 40-bit, the mappings for entire IOVA space can't
> be specified in the PAXBv2 PCIe RC of SoC. The holes in IOVA space needs to
> be reserved to prevent any IOVA allocations in those spaces.

Can you clarify why? If this is the PCI inbound window thing again, let 
me say once again that "dma-ranges" is the appropriate way for DT to 
describe the hardware.

Robin.

> reserved-dma-region property is added to specify the ranges which should
> never be mapped and given to devices sitting behind.
> 
> Reviewed-by: Ray Jui <ray.jui@broadcom.com>
> Reviewed-by: Vikram Prakash <vikram.prakash@broadcom.com>
> Reviewed-by: Scott Branden <scott.branden@broadcom.com>
> Signed-off-by: Jitendra Bhivare <jitendra.bhivare@broadcom.com>
> ---
>   Documentation/devicetree/bindings/pci/brcm,iproc-pcie.txt | 3 +++
>   1 file changed, 3 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/pci/brcm,iproc-pcie.txt b/Documentation/devicetree/bindings/pci/brcm,iproc-pcie.txt
> index b8e48b4..3be0fe3 100644
> --- a/Documentation/devicetree/bindings/pci/brcm,iproc-pcie.txt
> +++ b/Documentation/devicetree/bindings/pci/brcm,iproc-pcie.txt
> @@ -30,6 +30,9 @@ Optional properties:
>   - dma-ranges: Some PAXB-based root complexes do not have inbound mapping done
>     by the ASIC after power on reset.  In this case, SW is required to configure
>   the mapping, based on inbound memory regions specified by this property.
> +- reserved-dma-region: PAXBv2 with IOMMU enabled cannot provide mappings for
> +  entire IOVA space specified by DMA mask. Hence this is used to reserve the
> +  gaps in dma-ranges.
>   
>   - brcm,pcie-ob: Some iProc SoCs do not have the outbound address mapping done
>   by the ASIC after power on reset. In this case, SW needs to configure it
> 

Re: [RFC 3/5] dt-bindings: arm-smmu: Add reserved-msi-region

[Robin Murphy]

On 06/03/18 04:59, Jitendra Bhivare wrote:
> iPROC SoC has a special device window to treat GICv3 ITS MSI.

Ugh, really? After preferably printing out 100 copies of the SBSA, 
binding them all together and dropping the lot onto the hardware 
designers from a great height, could you clarify what exactly the 
special behaviour is here?

Robin.

> Current code maps MSI to IOVA space. Add SMMU node property to use
> direct mappings for MSI region.
> 
> This property is read and reserved when arm_smmu_get_resv_regions
> gets called.
> 
> Signed-off-by: Jitendra Bhivare <jitendra.bhivare-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
> ---
>   Documentation/devicetree/bindings/iommu/arm,smmu.txt | 12 ++++++++++++
>   1 file changed, 12 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.txt b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
> index 8a6ffce..13fa2b9 100644
> --- a/Documentation/devicetree/bindings/iommu/arm,smmu.txt
> +++ b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
> @@ -71,6 +71,15 @@ conditions.
>                     or using stream matching with #iommu-cells = <2>, and
>                     may be ignored if present in such cases.
>   
> +- reserved-msi-region: MSI region to be reserved with specific prot in IOVA
> +		  space for direct mapping. The region is specified in tuple
> +		  of (busno,prot,bus_addr,size).
> +
> +- #region-address-cells: specifies number of cells needed to encode bus_addr
> +
> +- #region-size-cells: specifies number of cells needed to encode size
> +
> +
>   ** Deprecated properties:
>   
>   - mmu-masters (deprecated in favour of the generic "iommus" binding) :
> @@ -95,6 +104,9 @@ conditions.
>                                <0 36 4>,
>                                <0 37 4>;
>                   #iommu-cells = <1>;
> +		#region-address-cells = <1>;
> +		#region-size-cells = <1>;
> +		reserved-msi-region = <0x0 0x1a 0x63c3000 0x8000>;
>           };
>   
>           /* device with two stream IDs, 0 and 7 */
> 

[RFC 3/5] dt-bindings: arm-smmu: Add reserved-msi-region

[Robin Murphy]

On 06/03/18 04:59, Jitendra Bhivare wrote:
> iPROC SoC has a special device window to treat GICv3 ITS MSI.

Ugh, really? After preferably printing out 100 copies of the SBSA, 
binding them all together and dropping the lot onto the hardware 
designers from a great height, could you clarify what exactly the 
special behaviour is here?

Robin.

> Current code maps MSI to IOVA space. Add SMMU node property to use
> direct mappings for MSI region.
> 
> This property is read and reserved when arm_smmu_get_resv_regions
> gets called.
> 
> Signed-off-by: Jitendra Bhivare <jitendra.bhivare@broadcom.com>
> ---
>   Documentation/devicetree/bindings/iommu/arm,smmu.txt | 12 ++++++++++++
>   1 file changed, 12 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.txt b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
> index 8a6ffce..13fa2b9 100644
> --- a/Documentation/devicetree/bindings/iommu/arm,smmu.txt
> +++ b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
> @@ -71,6 +71,15 @@ conditions.
>                     or using stream matching with #iommu-cells = <2>, and
>                     may be ignored if present in such cases.
>   
> +- reserved-msi-region: MSI region to be reserved with specific prot in IOVA
> +		  space for direct mapping. The region is specified in tuple
> +		  of (busno,prot,bus_addr,size).
> +
> +- #region-address-cells: specifies number of cells needed to encode bus_addr
> +
> +- #region-size-cells: specifies number of cells needed to encode size
> +
> +
>   ** Deprecated properties:
>   
>   - mmu-masters (deprecated in favour of the generic "iommus" binding) :
> @@ -95,6 +104,9 @@ conditions.
>                                <0 36 4>,
>                                <0 37 4>;
>                   #iommu-cells = <1>;
> +		#region-address-cells = <1>;
> +		#region-size-cells = <1>;
> +		reserved-msi-region = <0x0 0x1a 0x63c3000 0x8000>;
>           };
>   
>           /* device with two stream IDs, 0 and 7 */
> 

Re: [RFC 1/5] dt-bindings: iommu: Add reserved-dma-region for IOMMU device

[Jitendra Bhivare via iommu]

[-- Attachment #1.1: Type: text/plain, Size: 2920 bytes --]

On Tue, Mar 6, 2018 at 5:11 PM, Robin Murphy <robin.murphy-5wv7dgnIgG8@public.gmane.org> wrote:

> On 06/03/18 04:59, Jitendra Bhivare wrote:
>
>> Certain regions in IO virtual address space may need to be reserved to be
>> not used for devices configured behind IOMMU.
>>
>> Add documentation of the property to allow such regions to be specified in
>> DT to be reserved using IOMMU_RESV_RESERVED type.
>>
>
> That sounds like software policy; DT is for describing hardware.

It is to describe the holes in address space for which hardware can't
provide mappings.

>
>
> Reviewed-by: Ray Jui <ray.jui-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
>> Reviewed-by: Vikram Prakash <vikram.prakash-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
>> Reviewed-by: Scott Branden <scott.branden-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
>> Signed-off-by: Jitendra Bhivare <jitendra.bhivare-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
>> ---
>>   Documentation/devicetree/bindings/iommu/iommu.txt | 17
>> +++++++++++++++++
>>   1 file changed, 17 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/iommu/iommu.txt
>> b/Documentation/devicetree/bindings/iommu/iommu.txt
>> index 5a8b462..5a58ef2 100644
>> --- a/Documentation/devicetree/bindings/iommu/iommu.txt
>> +++ b/Documentation/devicetree/bindings/iommu/iommu.txt
>> @@ -98,6 +98,20 @@ requirements of that use-case haven't been fully
>> determined yet. Implementing
>>   this is therefore not recommended without further discussion and
>> extension of
>>   this binding.
>>   +Optional properties:
>> +--------------------
>> +- reserved-dma-region: This specifies DMA region to be reserved with
>> specific
>> +  prot in IOVA space. It is in tuples of (busno,prot,bus_addr,size).
>>
>
> What do busno and prot actually mean, and what are valid values for them?

busno was part of initial commit 4e0ee78f in implementation of
of_get_dma_window.
I am not sure what was the use of it in Powerpc. Similar implementation can
be seen
in of_parse_dma_window.
prot is access permission flags for the region for reserving in SW.This I
will remove.

>
>
> Robin.
>
>
> +- #region-address-cells: specifies number of cells needed to encode
>> bus_addr
>> +
>> +- #region-size-cells: specifies number of cells needed to encode size
>> +
>> +Notes:
>> +======
>> +This can be applied to IOMMU master node or to children (such as PCI host
>> +bridges) on the bus behind IOMMU.
>> +
>>     Examples:
>>   =========
>> @@ -173,6 +187,9 @@ Multiple-master IOMMU with configurable DMA window:
>>                          * master (i.e. the I/O virtual address space).
>>                          */
>>                         #iommu-cells = <4>;
>> +                       #region-address-cells = <2>;
>> +                       #region-size-cells = <2>;
>> +                       reserved-dma-region = <0x0 0x0 0x04 0x0 0x04 0x0>;
>>                 };
>>                 master {
>>
>>

[-- Attachment #1.2: Type: text/html, Size: 4751 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [RFC 1/5] dt-bindings: iommu: Add reserved-dma-region for IOMMU device

[Jitendra Bhivare via iommu]

On Tue, Mar 6, 2018 at 5:11 PM, Robin Murphy <robin.murphy-5wv7dgnIgG8@public.gmane.org> wrote:
>
> On 06/03/18 04:59, Jitendra Bhivare wrote:
>>
>> Certain regions in IO virtual address space may need to be reserved to be
>> not used for devices configured behind IOMMU.
>>
>> Add documentation of the property to allow such regions to be specified in
>> DT to be reserved using IOMMU_RESV_RESERVED type.
>
>
> That sounds like software policy; DT is for describing hardware.
It is to describe the holes in address space for which hardware can't
provide mappings.
>
>> Reviewed-by: Ray Jui <ray.jui-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
>> Reviewed-by: Vikram Prakash <vikram.prakash-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
>> Reviewed-by: Scott Branden <scott.branden-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
>> Signed-off-by: Jitendra Bhivare <jitendra.bhivare-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
>> ---
>>   Documentation/devicetree/bindings/iommu/iommu.txt | 17 +++++++++++++++++
>>   1 file changed, 17 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/iommu/iommu.txt b/Documentation/devicetree/bindings/iommu/iommu.txt
>> index 5a8b462..5a58ef2 100644
>> --- a/Documentation/devicetree/bindings/iommu/iommu.txt
>> +++ b/Documentation/devicetree/bindings/iommu/iommu.txt
>> @@ -98,6 +98,20 @@ requirements of that use-case haven't been fully determined yet. Implementing
>>   this is therefore not recommended without further discussion and extension of
>>   this binding.
>>   +Optional properties:
>> +--------------------
>> +- reserved-dma-region: This specifies DMA region to be reserved with specific
>> +  prot in IOVA space. It is in tuples of (busno,prot,bus_addr,size).
>
>
> What do busno and prot actually mean, and what are valid values for them?
>
> Robin.
busno was part of initial commit 4e0ee78f in implementation of
of_get_dma_window.
I am not sure what was the use of it in PowerPC. Similar
implementation can be seen
in of_parse_dma_window.
prot is access permission flags for the region for reserving in SW.
This I will remove.
>
>
>> +- #region-address-cells: specifies number of cells needed to encode bus_addr
>> +
>> +- #region-size-cells: specifies number of cells needed to encode size
>> +
>> +Notes:
>> +======
>> +This can be applied to IOMMU master node or to children (such as PCI host
>> +bridges) on the bus behind IOMMU.
>> +
>>     Examples:
>>   =========
>> @@ -173,6 +187,9 @@ Multiple-master IOMMU with configurable DMA window:
>>                          * master (i.e. the I/O virtual address space).
>>                          */
>>                         #iommu-cells = <4>;
>> +                       #region-address-cells = <2>;
>> +                       #region-size-cells = <2>;
>> +                       reserved-dma-region = <0x0 0x0 0x04 0x0 0x04 0x0>;
>>                 };
>>                 master {
>>

[RFC 1/5] dt-bindings: iommu: Add reserved-dma-region for IOMMU device

[Jitendra Bhivare]

On Tue, Mar 6, 2018 at 5:11 PM, Robin Murphy <robin.murphy@arm.com> wrote:
>
> On 06/03/18 04:59, Jitendra Bhivare wrote:
>>
>> Certain regions in IO virtual address space may need to be reserved to be
>> not used for devices configured behind IOMMU.
>>
>> Add documentation of the property to allow such regions to be specified in
>> DT to be reserved using IOMMU_RESV_RESERVED type.
>
>
> That sounds like software policy; DT is for describing hardware.
It is to describe the holes in address space for which hardware can't
provide mappings.
>
>> Reviewed-by: Ray Jui <ray.jui@broadcom.com>
>> Reviewed-by: Vikram Prakash <vikram.prakash@broadcom.com>
>> Reviewed-by: Scott Branden <scott.branden@broadcom.com>
>> Signed-off-by: Jitendra Bhivare <jitendra.bhivare@broadcom.com>
>> ---
>>   Documentation/devicetree/bindings/iommu/iommu.txt | 17 +++++++++++++++++
>>   1 file changed, 17 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/iommu/iommu.txt b/Documentation/devicetree/bindings/iommu/iommu.txt
>> index 5a8b462..5a58ef2 100644
>> --- a/Documentation/devicetree/bindings/iommu/iommu.txt
>> +++ b/Documentation/devicetree/bindings/iommu/iommu.txt
>> @@ -98,6 +98,20 @@ requirements of that use-case haven't been fully determined yet. Implementing
>>   this is therefore not recommended without further discussion and extension of
>>   this binding.
>>   +Optional properties:
>> +--------------------
>> +- reserved-dma-region: This specifies DMA region to be reserved with specific
>> +  prot in IOVA space. It is in tuples of (busno,prot,bus_addr,size).
>
>
> What do busno and prot actually mean, and what are valid values for them?
>
> Robin.
busno was part of initial commit 4e0ee78f in implementation of
of_get_dma_window.
I am not sure what was the use of it in PowerPC. Similar
implementation can be seen
in of_parse_dma_window.
prot is access permission flags for the region for reserving in SW.
This I will remove.
>
>
>> +- #region-address-cells: specifies number of cells needed to encode bus_addr
>> +
>> +- #region-size-cells: specifies number of cells needed to encode size
>> +
>> +Notes:
>> +======
>> +This can be applied to IOMMU master node or to children (such as PCI host
>> +bridges) on the bus behind IOMMU.
>> +
>>     Examples:
>>   =========
>> @@ -173,6 +187,9 @@ Multiple-master IOMMU with configurable DMA window:
>>                          * master (i.e. the I/O virtual address space).
>>                          */
>>                         #iommu-cells = <4>;
>> +                       #region-address-cells = <2>;
>> +                       #region-size-cells = <2>;
>> +                       reserved-dma-region = <0x0 0x0 0x04 0x0 0x04 0x0>;
>>                 };
>>                 master {
>>

Re: [RFC 2/5] dt-bindings: brcm: Add reserved-dma-region for iPROC

[Jitendra Bhivare via iommu]

On Tue, Mar 6, 2018 at 5:12 PM, Robin Murphy <robin.murphy-5wv7dgnIgG8@public.gmane.org> wrote:
> On 06/03/18 04:59, Jitendra Bhivare wrote:
>>
>> With SoC wide DMA mask of 40-bit, the mappings for entire IOVA space can't
>> be specified in the PAXBv2 PCIe RC of SoC. The holes in IOVA space needs
>> to
>> be reserved to prevent any IOVA allocations in those spaces.
>
>
> Can you clarify why? If this is the PCI inbound window thing again, let me
> say once again that "dma-ranges" is the appropriate way for DT to describe
> the hardware.
>
> Robin.
dma-ranges = < \
0x43000000 0x00 0x00000000 0x00 0x80000000 0x00 0x80000000 \
0x43000000 0x08 0x00000000 0x08 0x00000000 0x08 0x00000000 \
0x43000000 0x80 0x00000000 0x80 0x00000000 0x80 0x00000000>

Yes, its for PCI inbound windows. In our HW, they are limited by sizes
specified in
ARM memory maps which was done for non-IOMMU cases to catch any transfer
outside the ranges.
dma-ranges are already being used to program these inbound windows and SoC
wide DMA mask is already specified but IOMMU code can still allocate IOVAs
in the gaps for which translation will fail in PCIe RC.
>
>
>> reserved-dma-region property is added to specify the ranges which should
>> never be mapped and given to devices sitting behind.
>>
>> Reviewed-by: Ray Jui <ray.jui-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
>> Reviewed-by: Vikram Prakash <vikram.prakash-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
>> Reviewed-by: Scott Branden <scott.branden-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
>> Signed-off-by: Jitendra Bhivare <jitendra.bhivare-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
>> ---
>>   Documentation/devicetree/bindings/pci/brcm,iproc-pcie.txt | 3 +++
>>   1 file changed, 3 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/pci/brcm,iproc-pcie.txt
>> b/Documentation/devicetree/bindings/pci/brcm,iproc-pcie.txt
>> index b8e48b4..3be0fe3 100644
>> --- a/Documentation/devicetree/bindings/pci/brcm,iproc-pcie.txt
>> +++ b/Documentation/devicetree/bindings/pci/brcm,iproc-pcie.txt
>> @@ -30,6 +30,9 @@ Optional properties:
>>   - dma-ranges: Some PAXB-based root complexes do not have inbound mapping
>> done
>>     by the ASIC after power on reset.  In this case, SW is required to
>> configure
>>   the mapping, based on inbound memory regions specified by this property.
>> +- reserved-dma-region: PAXBv2 with IOMMU enabled cannot provide mappings
>> for
>> +  entire IOVA space specified by DMA mask. Hence this is used to reserve
>> the
>> +  gaps in dma-ranges.
>>     - brcm,pcie-ob: Some iProc SoCs do not have the outbound address
>> mapping done
>>   by the ASIC after power on reset. In this case, SW needs to configure it
>>
>

[RFC 2/5] dt-bindings: brcm: Add reserved-dma-region for iPROC

[Jitendra Bhivare]

On Tue, Mar 6, 2018 at 5:12 PM, Robin Murphy <robin.murphy@arm.com> wrote:
> On 06/03/18 04:59, Jitendra Bhivare wrote:
>>
>> With SoC wide DMA mask of 40-bit, the mappings for entire IOVA space can't
>> be specified in the PAXBv2 PCIe RC of SoC. The holes in IOVA space needs
>> to
>> be reserved to prevent any IOVA allocations in those spaces.
>
>
> Can you clarify why? If this is the PCI inbound window thing again, let me
> say once again that "dma-ranges" is the appropriate way for DT to describe
> the hardware.
>
> Robin.
dma-ranges = < \
0x43000000 0x00 0x00000000 0x00 0x80000000 0x00 0x80000000 \
0x43000000 0x08 0x00000000 0x08 0x00000000 0x08 0x00000000 \
0x43000000 0x80 0x00000000 0x80 0x00000000 0x80 0x00000000>

Yes, its for PCI inbound windows. In our HW, they are limited by sizes
specified in
ARM memory maps which was done for non-IOMMU cases to catch any transfer
outside the ranges.
dma-ranges are already being used to program these inbound windows and SoC
wide DMA mask is already specified but IOMMU code can still allocate IOVAs
in the gaps for which translation will fail in PCIe RC.
>
>
>> reserved-dma-region property is added to specify the ranges which should
>> never be mapped and given to devices sitting behind.
>>
>> Reviewed-by: Ray Jui <ray.jui@broadcom.com>
>> Reviewed-by: Vikram Prakash <vikram.prakash@broadcom.com>
>> Reviewed-by: Scott Branden <scott.branden@broadcom.com>
>> Signed-off-by: Jitendra Bhivare <jitendra.bhivare@broadcom.com>
>> ---
>>   Documentation/devicetree/bindings/pci/brcm,iproc-pcie.txt | 3 +++
>>   1 file changed, 3 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/pci/brcm,iproc-pcie.txt
>> b/Documentation/devicetree/bindings/pci/brcm,iproc-pcie.txt
>> index b8e48b4..3be0fe3 100644
>> --- a/Documentation/devicetree/bindings/pci/brcm,iproc-pcie.txt
>> +++ b/Documentation/devicetree/bindings/pci/brcm,iproc-pcie.txt
>> @@ -30,6 +30,9 @@ Optional properties:
>>   - dma-ranges: Some PAXB-based root complexes do not have inbound mapping
>> done
>>     by the ASIC after power on reset.  In this case, SW is required to
>> configure
>>   the mapping, based on inbound memory regions specified by this property.
>> +- reserved-dma-region: PAXBv2 with IOMMU enabled cannot provide mappings
>> for
>> +  entire IOVA space specified by DMA mask. Hence this is used to reserve
>> the
>> +  gaps in dma-ranges.
>>     - brcm,pcie-ob: Some iProc SoCs do not have the outbound address
>> mapping done
>>   by the ASIC after power on reset. In this case, SW needs to configure it
>>
>

Re: [RFC 3/5] dt-bindings: arm-smmu: Add reserved-msi-region

[Jitendra Bhivare via iommu]

On Tue, Mar 6, 2018 at 5:17 PM, Robin Murphy <robin.murphy-5wv7dgnIgG8@public.gmane.org> wrote:
> On 06/03/18 04:59, Jitendra Bhivare wrote:
>>
>> iPROC SoC has a special device window to treat GICv3 ITS MSI.
>
>
> Ugh, really? After preferably printing out 100 copies of the SBSA, binding
> them all together and dropping the lot onto the hardware designers from a
> great height, could you clarify what exactly the special behaviour is here?
>
> Robin.
>
The special device window needs to programmed so that writes to
specified address
region helps for specific ordering of traffic prior to it.
>
>> Current code maps MSI to IOVA space. Add SMMU node property to use
>> direct mappings for MSI region.
>>
>> This property is read and reserved when arm_smmu_get_resv_regions
>> gets called.
>>
>> Signed-off-by: Jitendra Bhivare <jitendra.bhivare-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
>> ---
>>   Documentation/devicetree/bindings/iommu/arm,smmu.txt | 12 ++++++++++++
>>   1 file changed, 12 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.txt
>> b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
>> index 8a6ffce..13fa2b9 100644
>> --- a/Documentation/devicetree/bindings/iommu/arm,smmu.txt
>> +++ b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
>> @@ -71,6 +71,15 @@ conditions.
>>                     or using stream matching with #iommu-cells = <2>, and
>>                     may be ignored if present in such cases.
>>   +- reserved-msi-region: MSI region to be reserved with specific prot in
>> IOVA
>> +                 space for direct mapping. The region is specified in
>> tuple
>> +                 of (busno,prot,bus_addr,size).
>> +
>> +- #region-address-cells: specifies number of cells needed to encode
>> bus_addr
>> +
>> +- #region-size-cells: specifies number of cells needed to encode size
>> +
>> +
>>   ** Deprecated properties:
>>     - mmu-masters (deprecated in favour of the generic "iommus" binding) :
>> @@ -95,6 +104,9 @@ conditions.
>>                                <0 36 4>,
>>                                <0 37 4>;
>>                   #iommu-cells = <1>;
>> +               #region-address-cells = <1>;
>> +               #region-size-cells = <1>;
>> +               reserved-msi-region = <0x0 0x1a 0x63c3000 0x8000>;
>>           };
>>             /* device with two stream IDs, 0 and 7 */
>>
>

[RFC 3/5] dt-bindings: arm-smmu: Add reserved-msi-region

[Jitendra Bhivare]

On Tue, Mar 6, 2018 at 5:17 PM, Robin Murphy <robin.murphy@arm.com> wrote:
> On 06/03/18 04:59, Jitendra Bhivare wrote:
>>
>> iPROC SoC has a special device window to treat GICv3 ITS MSI.
>
>
> Ugh, really? After preferably printing out 100 copies of the SBSA, binding
> them all together and dropping the lot onto the hardware designers from a
> great height, could you clarify what exactly the special behaviour is here?
>
> Robin.
>
The special device window needs to programmed so that writes to
specified address
region helps for specific ordering of traffic prior to it.
>
>> Current code maps MSI to IOVA space. Add SMMU node property to use
>> direct mappings for MSI region.
>>
>> This property is read and reserved when arm_smmu_get_resv_regions
>> gets called.
>>
>> Signed-off-by: Jitendra Bhivare <jitendra.bhivare@broadcom.com>
>> ---
>>   Documentation/devicetree/bindings/iommu/arm,smmu.txt | 12 ++++++++++++
>>   1 file changed, 12 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.txt
>> b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
>> index 8a6ffce..13fa2b9 100644
>> --- a/Documentation/devicetree/bindings/iommu/arm,smmu.txt
>> +++ b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
>> @@ -71,6 +71,15 @@ conditions.
>>                     or using stream matching with #iommu-cells = <2>, and
>>                     may be ignored if present in such cases.
>>   +- reserved-msi-region: MSI region to be reserved with specific prot in
>> IOVA
>> +                 space for direct mapping. The region is specified in
>> tuple
>> +                 of (busno,prot,bus_addr,size).
>> +
>> +- #region-address-cells: specifies number of cells needed to encode
>> bus_addr
>> +
>> +- #region-size-cells: specifies number of cells needed to encode size
>> +
>> +
>>   ** Deprecated properties:
>>     - mmu-masters (deprecated in favour of the generic "iommus" binding) :
>> @@ -95,6 +104,9 @@ conditions.
>>                                <0 36 4>,
>>                                <0 37 4>;
>>                   #iommu-cells = <1>;
>> +               #region-address-cells = <1>;
>> +               #region-size-cells = <1>;
>> +               reserved-msi-region = <0x0 0x1a 0x63c3000 0x8000>;
>>           };
>>             /* device with two stream IDs, 0 and 7 */
>>
>

Re: [RFC 2/5] dt-bindings: brcm: Add reserved-dma-region for iPROC

[Robin Murphy]

On 12/03/18 07:03, Jitendra Bhivare wrote:
> On Tue, Mar 6, 2018 at 5:12 PM, Robin Murphy <robin.murphy-5wv7dgnIgG8@public.gmane.org> wrote:
>> On 06/03/18 04:59, Jitendra Bhivare wrote:
>>>
>>> With SoC wide DMA mask of 40-bit, the mappings for entire IOVA space can't
>>> be specified in the PAXBv2 PCIe RC of SoC. The holes in IOVA space needs
>>> to
>>> be reserved to prevent any IOVA allocations in those spaces.
>>
>>
>> Can you clarify why? If this is the PCI inbound window thing again, let me
>> say once again that "dma-ranges" is the appropriate way for DT to describe
>> the hardware.
>>
>> Robin.
> dma-ranges = < \
> 0x43000000 0x00 0x00000000 0x00 0x80000000 0x00 0x80000000 \
> 0x43000000 0x08 0x00000000 0x08 0x00000000 0x08 0x00000000 \
> 0x43000000 0x80 0x00000000 0x80 0x00000000 0x80 0x00000000>
> 
> Yes, its for PCI inbound windows. In our HW, they are limited by sizes
> specified in
> ARM memory maps which was done for non-IOMMU cases to catch any transfer
> outside the ranges.
> dma-ranges are already being used to program these inbound windows and SoC
> wide DMA mask is already specified but IOMMU code can still allocate IOVAs
> in the gaps for which translation will fail in PCIe RC.

Right, so make iommu-dma reserve the gaps. No need to clutter up the DT 
with redundant information which gets handled pretty much identically 
anyway.

Robin.

>>
>>
>>> reserved-dma-region property is added to specify the ranges which should
>>> never be mapped and given to devices sitting behind.
>>>
>>> Reviewed-by: Ray Jui <ray.jui-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
>>> Reviewed-by: Vikram Prakash <vikram.prakash-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
>>> Reviewed-by: Scott Branden <scott.branden-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
>>> Signed-off-by: Jitendra Bhivare <jitendra.bhivare-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
>>> ---
>>>    Documentation/devicetree/bindings/pci/brcm,iproc-pcie.txt | 3 +++
>>>    1 file changed, 3 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/pci/brcm,iproc-pcie.txt
>>> b/Documentation/devicetree/bindings/pci/brcm,iproc-pcie.txt
>>> index b8e48b4..3be0fe3 100644
>>> --- a/Documentation/devicetree/bindings/pci/brcm,iproc-pcie.txt
>>> +++ b/Documentation/devicetree/bindings/pci/brcm,iproc-pcie.txt
>>> @@ -30,6 +30,9 @@ Optional properties:
>>>    - dma-ranges: Some PAXB-based root complexes do not have inbound mapping
>>> done
>>>      by the ASIC after power on reset.  In this case, SW is required to
>>> configure
>>>    the mapping, based on inbound memory regions specified by this property.
>>> +- reserved-dma-region: PAXBv2 with IOMMU enabled cannot provide mappings
>>> for
>>> +  entire IOVA space specified by DMA mask. Hence this is used to reserve
>>> the
>>> +  gaps in dma-ranges.
>>>      - brcm,pcie-ob: Some iProc SoCs do not have the outbound address
>>> mapping done
>>>    by the ASIC after power on reset. In this case, SW needs to configure it
>>>
>>

[RFC 2/5] dt-bindings: brcm: Add reserved-dma-region for iPROC

[Robin Murphy]

On 12/03/18 07:03, Jitendra Bhivare wrote:
> On Tue, Mar 6, 2018 at 5:12 PM, Robin Murphy <robin.murphy@arm.com> wrote:
>> On 06/03/18 04:59, Jitendra Bhivare wrote:
>>>
>>> With SoC wide DMA mask of 40-bit, the mappings for entire IOVA space can't
>>> be specified in the PAXBv2 PCIe RC of SoC. The holes in IOVA space needs
>>> to
>>> be reserved to prevent any IOVA allocations in those spaces.
>>
>>
>> Can you clarify why? If this is the PCI inbound window thing again, let me
>> say once again that "dma-ranges" is the appropriate way for DT to describe
>> the hardware.
>>
>> Robin.
> dma-ranges = < \
> 0x43000000 0x00 0x00000000 0x00 0x80000000 0x00 0x80000000 \
> 0x43000000 0x08 0x00000000 0x08 0x00000000 0x08 0x00000000 \
> 0x43000000 0x80 0x00000000 0x80 0x00000000 0x80 0x00000000>
> 
> Yes, its for PCI inbound windows. In our HW, they are limited by sizes
> specified in
> ARM memory maps which was done for non-IOMMU cases to catch any transfer
> outside the ranges.
> dma-ranges are already being used to program these inbound windows and SoC
> wide DMA mask is already specified but IOMMU code can still allocate IOVAs
> in the gaps for which translation will fail in PCIe RC.

Right, so make iommu-dma reserve the gaps. No need to clutter up the DT 
with redundant information which gets handled pretty much identically 
anyway.

Robin.

>>
>>
>>> reserved-dma-region property is added to specify the ranges which should
>>> never be mapped and given to devices sitting behind.
>>>
>>> Reviewed-by: Ray Jui <ray.jui@broadcom.com>
>>> Reviewed-by: Vikram Prakash <vikram.prakash@broadcom.com>
>>> Reviewed-by: Scott Branden <scott.branden@broadcom.com>
>>> Signed-off-by: Jitendra Bhivare <jitendra.bhivare@broadcom.com>
>>> ---
>>>    Documentation/devicetree/bindings/pci/brcm,iproc-pcie.txt | 3 +++
>>>    1 file changed, 3 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/pci/brcm,iproc-pcie.txt
>>> b/Documentation/devicetree/bindings/pci/brcm,iproc-pcie.txt
>>> index b8e48b4..3be0fe3 100644
>>> --- a/Documentation/devicetree/bindings/pci/brcm,iproc-pcie.txt
>>> +++ b/Documentation/devicetree/bindings/pci/brcm,iproc-pcie.txt
>>> @@ -30,6 +30,9 @@ Optional properties:
>>>    - dma-ranges: Some PAXB-based root complexes do not have inbound mapping
>>> done
>>>      by the ASIC after power on reset.  In this case, SW is required to
>>> configure
>>>    the mapping, based on inbound memory regions specified by this property.
>>> +- reserved-dma-region: PAXBv2 with IOMMU enabled cannot provide mappings
>>> for
>>> +  entire IOVA space specified by DMA mask. Hence this is used to reserve
>>> the
>>> +  gaps in dma-ranges.
>>>      - brcm,pcie-ob: Some iProc SoCs do not have the outbound address
>>> mapping done
>>>    by the ASIC after power on reset. In this case, SW needs to configure it
>>>
>>

Re: [RFC 2/5] dt-bindings: brcm: Add reserved-dma-region for iPROC

[Jitendra Bhivare via iommu]

On Thu, Mar 15, 2018 at 12:15 AM, Robin Murphy <robin.murphy-5wv7dgnIgG8@public.gmane.org> wrote:
> On 12/03/18 07:03, Jitendra Bhivare wrote:
>>
>> On Tue, Mar 6, 2018 at 5:12 PM, Robin Murphy <robin.murphy-5wv7dgnIgG8@public.gmane.org> wrote:
>>>
>>> On 06/03/18 04:59, Jitendra Bhivare wrote:
>>>>
>>>>
>>>> With SoC wide DMA mask of 40-bit, the mappings for entire IOVA space
>>>> can't
>>>> be specified in the PAXBv2 PCIe RC of SoC. The holes in IOVA space needs
>>>> to
>>>> be reserved to prevent any IOVA allocations in those spaces.
>>>
>>>
>>>
>>> Can you clarify why? If this is the PCI inbound window thing again, let
>>> me
>>> say once again that "dma-ranges" is the appropriate way for DT to
>>> describe
>>> the hardware.
>>>
>>> Robin.
>>
>> dma-ranges = < \
>> 0x43000000 0x00 0x00000000 0x00 0x80000000 0x00 0x80000000 \
>> 0x43000000 0x08 0x00000000 0x08 0x00000000 0x08 0x00000000 \
>> 0x43000000 0x80 0x00000000 0x80 0x00000000 0x80 0x00000000>
>>
>> Yes, its for PCI inbound windows. In our HW, they are limited by sizes
>> specified in
>> ARM memory maps which was done for non-IOMMU cases to catch any transfer
>> outside the ranges.
>> dma-ranges are already being used to program these inbound windows and SoC
>> wide DMA mask is already specified but IOMMU code can still allocate IOVAs
>> in the gaps for which translation will fail in PCIe RC.
>
>
> Right, so make iommu-dma reserve the gaps. No need to clutter up the DT with
> redundant information which gets handled pretty much identically anyway.
>
> Robin.
There was a mistake in pasting dma-ranges mentioned. This is what we use it in
IOMMU case.
#define PCIE_DMA_RANGES dma-ranges = < \
0x43000000 0x00 0x00000000 0x00 0x00000000 0x04 0x00000000 \
0x43000000 0x08 0x00000000 0x08 0x00000000 0x08 0x00000000 \
0x43000000 0x80 0x00000000 0x80 0x00000000 0x80 0x00000000>

#define PCIE_RESERVED_DMA_REGION reserved-dma-region = < \
0x0 0x0 0x04 0x0 0x04 0x0 \
0x0 0x0 0x10 0x0 0x70 0x0>

This is non-iommu case which is per ARM memory map.
#define PCIE_DMA_RANGES dma-ranges = < \
0x43000000 0x00 0x80000000 0x00 0x80000000 0x00 0x80000000 \
0x43000000 0x08 0x00000000 0x08 0x00000000 0x08 0x00000000 \
0x43000000 0x80 0x00000000 0x80 0x00000000 0x40 0x00000000>

In IOMMU case, we had to increase first in-bound window i.e.
dma-ranges first entry
from 2GB-4GB (in non-iommu case) to 0-16GB because out-bound window specified
from 2GB-4GB implicitly gets reserved by the PCI iommu code. This allowed IOVA
allocations from 0-2GB and 4-16GB mapped through in-bound windows.

Basically, my point is that dma-ranges specified is being used to
program our in-bound
windows and defines PCI space to CPU address space supported mappings of HW.
In the same way, it would be better to explicitly specify
reserved-dma-region to clarify
this as HW hole than implicity reserving the regions.

We can at least have single property 'reserved-dma-window' for MSI
region and DMA regions
to help describe the holes in the HW than let SW compute it.

>>>
>>>
>>>> reserved-dma-region property is added to specify the ranges which should
>>>> never be mapped and given to devices sitting behind.
>>>>
>>>> Reviewed-by: Ray Jui <ray.jui-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
>>>> Reviewed-by: Vikram Prakash <vikram.prakash-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
>>>> Reviewed-by: Scott Branden <scott.branden-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
>>>> Signed-off-by: Jitendra Bhivare <jitendra.bhivare-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
>>>> ---
>>>>    Documentation/devicetree/bindings/pci/brcm,iproc-pcie.txt | 3 +++
>>>>    1 file changed, 3 insertions(+)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/pci/brcm,iproc-pcie.txt
>>>> b/Documentation/devicetree/bindings/pci/brcm,iproc-pcie.txt
>>>> index b8e48b4..3be0fe3 100644
>>>> --- a/Documentation/devicetree/bindings/pci/brcm,iproc-pcie.txt
>>>> +++ b/Documentation/devicetree/bindings/pci/brcm,iproc-pcie.txt
>>>> @@ -30,6 +30,9 @@ Optional properties:
>>>>    - dma-ranges: Some PAXB-based root complexes do not have inbound
>>>> mapping
>>>> done
>>>>      by the ASIC after power on reset.  In this case, SW is required to
>>>> configure
>>>>    the mapping, based on inbound memory regions specified by this
>>>> property.
>>>> +- reserved-dma-region: PAXBv2 with IOMMU enabled cannot provide
>>>> mappings
>>>> for
>>>> +  entire IOVA space specified by DMA mask. Hence this is used to
>>>> reserve
>>>> the
>>>> +  gaps in dma-ranges.
>>>>      - brcm,pcie-ob: Some iProc SoCs do not have the outbound address
>>>> mapping done
>>>>    by the ASIC after power on reset. In this case, SW needs to configure
>>>> it
>>>>
>>>
>

[RFC 2/5] dt-bindings: brcm: Add reserved-dma-region for iPROC

[Jitendra Bhivare]

On Thu, Mar 15, 2018 at 12:15 AM, Robin Murphy <robin.murphy@arm.com> wrote:
> On 12/03/18 07:03, Jitendra Bhivare wrote:
>>
>> On Tue, Mar 6, 2018 at 5:12 PM, Robin Murphy <robin.murphy@arm.com> wrote:
>>>
>>> On 06/03/18 04:59, Jitendra Bhivare wrote:
>>>>
>>>>
>>>> With SoC wide DMA mask of 40-bit, the mappings for entire IOVA space
>>>> can't
>>>> be specified in the PAXBv2 PCIe RC of SoC. The holes in IOVA space needs
>>>> to
>>>> be reserved to prevent any IOVA allocations in those spaces.
>>>
>>>
>>>
>>> Can you clarify why? If this is the PCI inbound window thing again, let
>>> me
>>> say once again that "dma-ranges" is the appropriate way for DT to
>>> describe
>>> the hardware.
>>>
>>> Robin.
>>
>> dma-ranges = < \
>> 0x43000000 0x00 0x00000000 0x00 0x80000000 0x00 0x80000000 \
>> 0x43000000 0x08 0x00000000 0x08 0x00000000 0x08 0x00000000 \
>> 0x43000000 0x80 0x00000000 0x80 0x00000000 0x80 0x00000000>
>>
>> Yes, its for PCI inbound windows. In our HW, they are limited by sizes
>> specified in
>> ARM memory maps which was done for non-IOMMU cases to catch any transfer
>> outside the ranges.
>> dma-ranges are already being used to program these inbound windows and SoC
>> wide DMA mask is already specified but IOMMU code can still allocate IOVAs
>> in the gaps for which translation will fail in PCIe RC.
>
>
> Right, so make iommu-dma reserve the gaps. No need to clutter up the DT with
> redundant information which gets handled pretty much identically anyway.
>
> Robin.
There was a mistake in pasting dma-ranges mentioned. This is what we use it in
IOMMU case.
#define PCIE_DMA_RANGES dma-ranges = < \
0x43000000 0x00 0x00000000 0x00 0x00000000 0x04 0x00000000 \
0x43000000 0x08 0x00000000 0x08 0x00000000 0x08 0x00000000 \
0x43000000 0x80 0x00000000 0x80 0x00000000 0x80 0x00000000>

#define PCIE_RESERVED_DMA_REGION reserved-dma-region = < \
0x0 0x0 0x04 0x0 0x04 0x0 \
0x0 0x0 0x10 0x0 0x70 0x0>

This is non-iommu case which is per ARM memory map.
#define PCIE_DMA_RANGES dma-ranges = < \
0x43000000 0x00 0x80000000 0x00 0x80000000 0x00 0x80000000 \
0x43000000 0x08 0x00000000 0x08 0x00000000 0x08 0x00000000 \
0x43000000 0x80 0x00000000 0x80 0x00000000 0x40 0x00000000>

In IOMMU case, we had to increase first in-bound window i.e.
dma-ranges first entry
from 2GB-4GB (in non-iommu case) to 0-16GB because out-bound window specified
from 2GB-4GB implicitly gets reserved by the PCI iommu code. This allowed IOVA
allocations from 0-2GB and 4-16GB mapped through in-bound windows.

Basically, my point is that dma-ranges specified is being used to
program our in-bound
windows and defines PCI space to CPU address space supported mappings of HW.
In the same way, it would be better to explicitly specify
reserved-dma-region to clarify
this as HW hole than implicity reserving the regions.

We can at least have single property 'reserved-dma-window' for MSI
region and DMA regions
to help describe the holes in the HW than let SW compute it.

>>>
>>>
>>>> reserved-dma-region property is added to specify the ranges which should
>>>> never be mapped and given to devices sitting behind.
>>>>
>>>> Reviewed-by: Ray Jui <ray.jui@broadcom.com>
>>>> Reviewed-by: Vikram Prakash <vikram.prakash@broadcom.com>
>>>> Reviewed-by: Scott Branden <scott.branden@broadcom.com>
>>>> Signed-off-by: Jitendra Bhivare <jitendra.bhivare@broadcom.com>
>>>> ---
>>>>    Documentation/devicetree/bindings/pci/brcm,iproc-pcie.txt | 3 +++
>>>>    1 file changed, 3 insertions(+)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/pci/brcm,iproc-pcie.txt
>>>> b/Documentation/devicetree/bindings/pci/brcm,iproc-pcie.txt
>>>> index b8e48b4..3be0fe3 100644
>>>> --- a/Documentation/devicetree/bindings/pci/brcm,iproc-pcie.txt
>>>> +++ b/Documentation/devicetree/bindings/pci/brcm,iproc-pcie.txt
>>>> @@ -30,6 +30,9 @@ Optional properties:
>>>>    - dma-ranges: Some PAXB-based root complexes do not have inbound
>>>> mapping
>>>> done
>>>>      by the ASIC after power on reset.  In this case, SW is required to
>>>> configure
>>>>    the mapping, based on inbound memory regions specified by this
>>>> property.
>>>> +- reserved-dma-region: PAXBv2 with IOMMU enabled cannot provide
>>>> mappings
>>>> for
>>>> +  entire IOVA space specified by DMA mask. Hence this is used to
>>>> reserve
>>>> the
>>>> +  gaps in dma-ranges.
>>>>      - brcm,pcie-ob: Some iProc SoCs do not have the outbound address
>>>> mapping done
>>>>    by the ASIC after power on reset. In this case, SW needs to configure
>>>> it
>>>>
>>>
>

Re: [RFC 3/5] dt-bindings: arm-smmu: Add reserved-msi-region

[Robin Murphy]

On 12/03/18 07:19, Jitendra Bhivare wrote:
> On Tue, Mar 6, 2018 at 5:17 PM, Robin Murphy <robin.murphy-5wv7dgnIgG8@public.gmane.org> wrote:
>> On 06/03/18 04:59, Jitendra Bhivare wrote:
>>>
>>> iPROC SoC has a special device window to treat GICv3 ITS MSI.
>>
>>
>> Ugh, really? After preferably printing out 100 copies of the SBSA, binding
>> them all together and dropping the lot onto the hardware designers from a
>> great height, could you clarify what exactly the special behaviour is here?
>>
>> Robin.
>>
> The special device window needs to programmed so that writes to
> specified address
> region helps for specific ordering of traffic prior to it.

OK, I know of PCIe root complexes having address matching to give the 
MSI write the appropriate AXI memory type attributes when the SMMU is 
bypassed, but ordering is a new one. I guess you have some glue logic on 
the root complex master interface which injects an ACE barrier 
transaction in front of any write to the ITS doorbell address?
>>> Current code maps MSI to IOVA space. Add SMMU node property to use
>>> direct mappings for MSI region.
>>>
>>> This property is read and reserved when arm_smmu_get_resv_regions
>>> gets called.

Either way, this should be a generic, not SMMU-specific, property - 
there are other platforms that would also make use of it to describe a 
similar hardware situation (which we currently only support via ACPI). 
The big question is where to put it: hardware-wise, the property of 
"MSIs won't work properly if the doorbell is remapped to a different 
address" belongs firmly to the root complex, not the IOMMU, while the 
address itself is already a property of the MSI controller. The IOMMU is 
just the innocent guy in the middle who has to discover and respect the 
constraints.

I'd like to think we could just have some flag to say "you can't remap 
my MSIs!" then go and chase the msi-parent/msi-map phandle to the MSI 
controller to get the address from its reg property, which is more or 
less the equivalent of what the current ACPI workaround does - see 
commit 8b4282e6b8e2 ("ACPI/IORT: Add msi address regions reservation 
helper") in linux-next - but I can already think of ways in which that 
might not work. For one, there's not necessarily any guarantee that the 
MSI controller's programming interface and doorbell are in the same 
place, so we probably do need to describe the specific MSI address(es) 
that the root complex cares about, from the root complex end :/

Robin.

>>>
>>> Signed-off-by: Jitendra Bhivare <jitendra.bhivare-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
>>> ---
>>>    Documentation/devicetree/bindings/iommu/arm,smmu.txt | 12 ++++++++++++
>>>    1 file changed, 12 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.txt
>>> b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
>>> index 8a6ffce..13fa2b9 100644
>>> --- a/Documentation/devicetree/bindings/iommu/arm,smmu.txt
>>> +++ b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
>>> @@ -71,6 +71,15 @@ conditions.
>>>                      or using stream matching with #iommu-cells = <2>, and
>>>                      may be ignored if present in such cases.
>>>    +- reserved-msi-region: MSI region to be reserved with specific prot in
>>> IOVA
>>> +                 space for direct mapping. The region is specified in
>>> tuple
>>> +                 of (busno,prot,bus_addr,size).
>>> +
>>> +- #region-address-cells: specifies number of cells needed to encode
>>> bus_addr
>>> +
>>> +- #region-size-cells: specifies number of cells needed to encode size
>>> +
>>> +
>>>    ** Deprecated properties:
>>>      - mmu-masters (deprecated in favour of the generic "iommus" binding) :
>>> @@ -95,6 +104,9 @@ conditions.
>>>                                 <0 36 4>,
>>>                                 <0 37 4>;
>>>                    #iommu-cells = <1>;
>>> +               #region-address-cells = <1>;
>>> +               #region-size-cells = <1>;
>>> +               reserved-msi-region = <0x0 0x1a 0x63c3000 0x8000>;
>>>            };
>>>              /* device with two stream IDs, 0 and 7 */
>>>
>>

[RFC 3/5] dt-bindings: arm-smmu: Add reserved-msi-region

[Robin Murphy]

On 12/03/18 07:19, Jitendra Bhivare wrote:
> On Tue, Mar 6, 2018 at 5:17 PM, Robin Murphy <robin.murphy@arm.com> wrote:
>> On 06/03/18 04:59, Jitendra Bhivare wrote:
>>>
>>> iPROC SoC has a special device window to treat GICv3 ITS MSI.
>>
>>
>> Ugh, really? After preferably printing out 100 copies of the SBSA, binding
>> them all together and dropping the lot onto the hardware designers from a
>> great height, could you clarify what exactly the special behaviour is here?
>>
>> Robin.
>>
> The special device window needs to programmed so that writes to
> specified address
> region helps for specific ordering of traffic prior to it.

OK, I know of PCIe root complexes having address matching to give the 
MSI write the appropriate AXI memory type attributes when the SMMU is 
bypassed, but ordering is a new one. I guess you have some glue logic on 
the root complex master interface which injects an ACE barrier 
transaction in front of any write to the ITS doorbell address?
>>> Current code maps MSI to IOVA space. Add SMMU node property to use
>>> direct mappings for MSI region.
>>>
>>> This property is read and reserved when arm_smmu_get_resv_regions
>>> gets called.

Either way, this should be a generic, not SMMU-specific, property - 
there are other platforms that would also make use of it to describe a 
similar hardware situation (which we currently only support via ACPI). 
The big question is where to put it: hardware-wise, the property of 
"MSIs won't work properly if the doorbell is remapped to a different 
address" belongs firmly to the root complex, not the IOMMU, while the 
address itself is already a property of the MSI controller. The IOMMU is 
just the innocent guy in the middle who has to discover and respect the 
constraints.

I'd like to think we could just have some flag to say "you can't remap 
my MSIs!" then go and chase the msi-parent/msi-map phandle to the MSI 
controller to get the address from its reg property, which is more or 
less the equivalent of what the current ACPI workaround does - see 
commit 8b4282e6b8e2 ("ACPI/IORT: Add msi address regions reservation 
helper") in linux-next - but I can already think of ways in which that 
might not work. For one, there's not necessarily any guarantee that the 
MSI controller's programming interface and doorbell are in the same 
place, so we probably do need to describe the specific MSI address(es) 
that the root complex cares about, from the root complex end :/

Robin.

>>>
>>> Signed-off-by: Jitendra Bhivare <jitendra.bhivare@broadcom.com>
>>> ---
>>>    Documentation/devicetree/bindings/iommu/arm,smmu.txt | 12 ++++++++++++
>>>    1 file changed, 12 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.txt
>>> b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
>>> index 8a6ffce..13fa2b9 100644
>>> --- a/Documentation/devicetree/bindings/iommu/arm,smmu.txt
>>> +++ b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
>>> @@ -71,6 +71,15 @@ conditions.
>>>                      or using stream matching with #iommu-cells = <2>, and
>>>                      may be ignored if present in such cases.
>>>    +- reserved-msi-region: MSI region to be reserved with specific prot in
>>> IOVA
>>> +                 space for direct mapping. The region is specified in
>>> tuple
>>> +                 of (busno,prot,bus_addr,size).
>>> +
>>> +- #region-address-cells: specifies number of cells needed to encode
>>> bus_addr
>>> +
>>> +- #region-size-cells: specifies number of cells needed to encode size
>>> +
>>> +
>>>    ** Deprecated properties:
>>>      - mmu-masters (deprecated in favour of the generic "iommus" binding) :
>>> @@ -95,6 +104,9 @@ conditions.
>>>                                 <0 36 4>,
>>>                                 <0 37 4>;
>>>                    #iommu-cells = <1>;
>>> +               #region-address-cells = <1>;
>>> +               #region-size-cells = <1>;
>>> +               reserved-msi-region = <0x0 0x1a 0x63c3000 0x8000>;
>>>            };
>>>              /* device with two stream IDs, 0 and 7 */
>>>
>>

Re: [RFC 2/5] dt-bindings: brcm: Add reserved-dma-region for iPROC

[Robin Murphy]

On 15/03/18 12:03, Jitendra Bhivare wrote:
> On Thu, Mar 15, 2018 at 12:15 AM, Robin Murphy <robin.murphy-5wv7dgnIgG8@public.gmane.org> wrote:
>> On 12/03/18 07:03, Jitendra Bhivare wrote:
>>>
>>> On Tue, Mar 6, 2018 at 5:12 PM, Robin Murphy <robin.murphy-5wv7dgnIgG8@public.gmane.org> wrote:
>>>>
>>>> On 06/03/18 04:59, Jitendra Bhivare wrote:
>>>>>
>>>>>
>>>>> With SoC wide DMA mask of 40-bit, the mappings for entire IOVA space
>>>>> can't
>>>>> be specified in the PAXBv2 PCIe RC of SoC. The holes in IOVA space needs
>>>>> to
>>>>> be reserved to prevent any IOVA allocations in those spaces.
>>>>
>>>>
>>>>
>>>> Can you clarify why? If this is the PCI inbound window thing again, let
>>>> me
>>>> say once again that "dma-ranges" is the appropriate way for DT to
>>>> describe
>>>> the hardware.
>>>>
>>>> Robin.
>>>
>>> dma-ranges = < \
>>> 0x43000000 0x00 0x00000000 0x00 0x80000000 0x00 0x80000000 \
>>> 0x43000000 0x08 0x00000000 0x08 0x00000000 0x08 0x00000000 \
>>> 0x43000000 0x80 0x00000000 0x80 0x00000000 0x80 0x00000000>
>>>
>>> Yes, its for PCI inbound windows. In our HW, they are limited by sizes
>>> specified in
>>> ARM memory maps which was done for non-IOMMU cases to catch any transfer
>>> outside the ranges.
>>> dma-ranges are already being used to program these inbound windows and SoC
>>> wide DMA mask is already specified but IOMMU code can still allocate IOVAs
>>> in the gaps for which translation will fail in PCIe RC.
>>
>>
>> Right, so make iommu-dma reserve the gaps. No need to clutter up the DT with
>> redundant information which gets handled pretty much identically anyway.
>>
>> Robin.
> There was a mistake in pasting dma-ranges mentioned. This is what we use it in
> IOMMU case.
> #define PCIE_DMA_RANGES dma-ranges = < \
> 0x43000000 0x00 0x00000000 0x00 0x00000000 0x04 0x00000000 \
> 0x43000000 0x08 0x00000000 0x08 0x00000000 0x08 0x00000000 \
> 0x43000000 0x80 0x00000000 0x80 0x00000000 0x80 0x00000000>
> 
> #define PCIE_RESERVED_DMA_REGION reserved-dma-region = < \
> 0x0 0x0 0x04 0x0 0x04 0x0 \
> 0x0 0x0 0x10 0x0 0x70 0x0>
> 
> This is non-iommu case which is per ARM memory map.
> #define PCIE_DMA_RANGES dma-ranges = < \
> 0x43000000 0x00 0x80000000 0x00 0x80000000 0x00 0x80000000 \
> 0x43000000 0x08 0x00000000 0x08 0x00000000 0x08 0x00000000 \
> 0x43000000 0x80 0x00000000 0x80 0x00000000 0x40 0x00000000>
> 
> In IOMMU case, we had to increase first in-bound window i.e.
> dma-ranges first entry
> from 2GB-4GB (in non-iommu case) to 0-16GB because out-bound window specified
> from 2GB-4GB implicitly gets reserved by the PCI iommu code. This allowed IOVA
> allocations from 0-2GB and 4-16GB mapped through in-bound windows.
> 
> Basically, my point is that dma-ranges specified is being used to
> program our in-bound
> windows and defines PCI space to CPU address space supported mappings of HW.
> In the same way, it would be better to explicitly specify
> reserved-dma-region to clarify
> this as HW hole than implicity reserving the regions.

Why? If DMA traffic can only pass through inbound windows, and you 
program the inbound windows based on dma-ranges, then by definition 
dma-ranges tells you exactly the set of addresses that are usable for 
DMA, and by extension, IOMMU remapping. I fail to see how it's then 
"better" to add a second slightly different description of the exact 
same information with the bonus maintenance burden of then having to 
ensure the two actually match.

Robin.

> We can at least have single property 'reserved-dma-window' for MSI
> region and DMA regions
> to help describe the holes in the HW than let SW compute it.
> 
>>>>
>>>>
>>>>> reserved-dma-region property is added to specify the ranges which should
>>>>> never be mapped and given to devices sitting behind.
>>>>>
>>>>> Reviewed-by: Ray Jui <ray.jui-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
>>>>> Reviewed-by: Vikram Prakash <vikram.prakash-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
>>>>> Reviewed-by: Scott Branden <scott.branden-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
>>>>> Signed-off-by: Jitendra Bhivare <jitendra.bhivare-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
>>>>> ---
>>>>>     Documentation/devicetree/bindings/pci/brcm,iproc-pcie.txt | 3 +++
>>>>>     1 file changed, 3 insertions(+)
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/pci/brcm,iproc-pcie.txt
>>>>> b/Documentation/devicetree/bindings/pci/brcm,iproc-pcie.txt
>>>>> index b8e48b4..3be0fe3 100644
>>>>> --- a/Documentation/devicetree/bindings/pci/brcm,iproc-pcie.txt
>>>>> +++ b/Documentation/devicetree/bindings/pci/brcm,iproc-pcie.txt
>>>>> @@ -30,6 +30,9 @@ Optional properties:
>>>>>     - dma-ranges: Some PAXB-based root complexes do not have inbound
>>>>> mapping
>>>>> done
>>>>>       by the ASIC after power on reset.  In this case, SW is required to
>>>>> configure
>>>>>     the mapping, based on inbound memory regions specified by this
>>>>> property.
>>>>> +- reserved-dma-region: PAXBv2 with IOMMU enabled cannot provide
>>>>> mappings
>>>>> for
>>>>> +  entire IOVA space specified by DMA mask. Hence this is used to
>>>>> reserve
>>>>> the
>>>>> +  gaps in dma-ranges.
>>>>>       - brcm,pcie-ob: Some iProc SoCs do not have the outbound address
>>>>> mapping done
>>>>>     by the ASIC after power on reset. In this case, SW needs to configure
>>>>> it
>>>>>
>>>>
>>

[RFC 2/5] dt-bindings: brcm: Add reserved-dma-region for iPROC

[Robin Murphy]

On 15/03/18 12:03, Jitendra Bhivare wrote:
> On Thu, Mar 15, 2018 at 12:15 AM, Robin Murphy <robin.murphy@arm.com> wrote:
>> On 12/03/18 07:03, Jitendra Bhivare wrote:
>>>
>>> On Tue, Mar 6, 2018 at 5:12 PM, Robin Murphy <robin.murphy@arm.com> wrote:
>>>>
>>>> On 06/03/18 04:59, Jitendra Bhivare wrote:
>>>>>
>>>>>
>>>>> With SoC wide DMA mask of 40-bit, the mappings for entire IOVA space
>>>>> can't
>>>>> be specified in the PAXBv2 PCIe RC of SoC. The holes in IOVA space needs
>>>>> to
>>>>> be reserved to prevent any IOVA allocations in those spaces.
>>>>
>>>>
>>>>
>>>> Can you clarify why? If this is the PCI inbound window thing again, let
>>>> me
>>>> say once again that "dma-ranges" is the appropriate way for DT to
>>>> describe
>>>> the hardware.
>>>>
>>>> Robin.
>>>
>>> dma-ranges = < \
>>> 0x43000000 0x00 0x00000000 0x00 0x80000000 0x00 0x80000000 \
>>> 0x43000000 0x08 0x00000000 0x08 0x00000000 0x08 0x00000000 \
>>> 0x43000000 0x80 0x00000000 0x80 0x00000000 0x80 0x00000000>
>>>
>>> Yes, its for PCI inbound windows. In our HW, they are limited by sizes
>>> specified in
>>> ARM memory maps which was done for non-IOMMU cases to catch any transfer
>>> outside the ranges.
>>> dma-ranges are already being used to program these inbound windows and SoC
>>> wide DMA mask is already specified but IOMMU code can still allocate IOVAs
>>> in the gaps for which translation will fail in PCIe RC.
>>
>>
>> Right, so make iommu-dma reserve the gaps. No need to clutter up the DT with
>> redundant information which gets handled pretty much identically anyway.
>>
>> Robin.
> There was a mistake in pasting dma-ranges mentioned. This is what we use it in
> IOMMU case.
> #define PCIE_DMA_RANGES dma-ranges = < \
> 0x43000000 0x00 0x00000000 0x00 0x00000000 0x04 0x00000000 \
> 0x43000000 0x08 0x00000000 0x08 0x00000000 0x08 0x00000000 \
> 0x43000000 0x80 0x00000000 0x80 0x00000000 0x80 0x00000000>
> 
> #define PCIE_RESERVED_DMA_REGION reserved-dma-region = < \
> 0x0 0x0 0x04 0x0 0x04 0x0 \
> 0x0 0x0 0x10 0x0 0x70 0x0>
> 
> This is non-iommu case which is per ARM memory map.
> #define PCIE_DMA_RANGES dma-ranges = < \
> 0x43000000 0x00 0x80000000 0x00 0x80000000 0x00 0x80000000 \
> 0x43000000 0x08 0x00000000 0x08 0x00000000 0x08 0x00000000 \
> 0x43000000 0x80 0x00000000 0x80 0x00000000 0x40 0x00000000>
> 
> In IOMMU case, we had to increase first in-bound window i.e.
> dma-ranges first entry
> from 2GB-4GB (in non-iommu case) to 0-16GB because out-bound window specified
> from 2GB-4GB implicitly gets reserved by the PCI iommu code. This allowed IOVA
> allocations from 0-2GB and 4-16GB mapped through in-bound windows.
> 
> Basically, my point is that dma-ranges specified is being used to
> program our in-bound
> windows and defines PCI space to CPU address space supported mappings of HW.
> In the same way, it would be better to explicitly specify
> reserved-dma-region to clarify
> this as HW hole than implicity reserving the regions.

Why? If DMA traffic can only pass through inbound windows, and you 
program the inbound windows based on dma-ranges, then by definition 
dma-ranges tells you exactly the set of addresses that are usable for 
DMA, and by extension, IOMMU remapping. I fail to see how it's then 
"better" to add a second slightly different description of the exact 
same information with the bonus maintenance burden of then having to 
ensure the two actually match.

Robin.

> We can at least have single property 'reserved-dma-window' for MSI
> region and DMA regions
> to help describe the holes in the HW than let SW compute it.
> 
>>>>
>>>>
>>>>> reserved-dma-region property is added to specify the ranges which should
>>>>> never be mapped and given to devices sitting behind.
>>>>>
>>>>> Reviewed-by: Ray Jui <ray.jui@broadcom.com>
>>>>> Reviewed-by: Vikram Prakash <vikram.prakash@broadcom.com>
>>>>> Reviewed-by: Scott Branden <scott.branden@broadcom.com>
>>>>> Signed-off-by: Jitendra Bhivare <jitendra.bhivare@broadcom.com>
>>>>> ---
>>>>>     Documentation/devicetree/bindings/pci/brcm,iproc-pcie.txt | 3 +++
>>>>>     1 file changed, 3 insertions(+)
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/pci/brcm,iproc-pcie.txt
>>>>> b/Documentation/devicetree/bindings/pci/brcm,iproc-pcie.txt
>>>>> index b8e48b4..3be0fe3 100644
>>>>> --- a/Documentation/devicetree/bindings/pci/brcm,iproc-pcie.txt
>>>>> +++ b/Documentation/devicetree/bindings/pci/brcm,iproc-pcie.txt
>>>>> @@ -30,6 +30,9 @@ Optional properties:
>>>>>     - dma-ranges: Some PAXB-based root complexes do not have inbound
>>>>> mapping
>>>>> done
>>>>>       by the ASIC after power on reset.  In this case, SW is required to
>>>>> configure
>>>>>     the mapping, based on inbound memory regions specified by this
>>>>> property.
>>>>> +- reserved-dma-region: PAXBv2 with IOMMU enabled cannot provide
>>>>> mappings
>>>>> for
>>>>> +  entire IOVA space specified by DMA mask. Hence this is used to
>>>>> reserve
>>>>> the
>>>>> +  gaps in dma-ranges.
>>>>>       - brcm,pcie-ob: Some iProc SoCs do not have the outbound address
>>>>> mapping done
>>>>>     by the ASIC after power on reset. In this case, SW needs to configure
>>>>> it
>>>>>
>>>>
>>