[PATCH 0/5] iommu/arm-smmu: Updates for 3.17

[PATCH 0/5] iommu/arm-smmu: Updates for 3.17

[Will Deacon]

Hi all,

Here is the set of arm/smmu patches I currently have that I'd like to get
in for 3.17 (pending review comments). The diffstat is bigger than usual,
since I've got my hands on a PCI-capable platform (model) and subsequently
started to get that up and running. I've also been playing with VFIO and
KVM guest device assignment, which revealed some issues with stage-2 mappings.

I'll post further RFC patches relating to VFIO separately, as discussion
is definitely needed there.

The other thing to note is that I plan to move to Thierry's generic IOMMU
devicetree bindings as soon as they are merged:

  http://lists.infradead.org/pipermail/linux-arm-kernel/2014-June/267061.html

That will break any users of the current binding, but I don't believe we
actually have any at the moment (there are certainly none in mainline).
The removal of chained SMMU support in this series is a prerequisite for
that conversion.

All feedback welcome,

Will


Will Deacon (5):
  iommu/arm-smmu: fix calculation of TCR.T0SZ
  iommu/arm-smmu: add support for PCI master devices
  iommu/arm-smmu: caps: add IOMMU_CAP_INTR_REMAP capability
  iommu/arm-smmu: remove support for chained SMMUs
  iommu/arm-smmu: prefer stage-1 mappings where we have a choice

 .../devicetree/bindings/iommu/arm,smmu.txt         |   6 -
 drivers/iommu/arm-smmu.c                           | 431 ++++++++++-----------
 2 files changed, 199 insertions(+), 238 deletions(-)

-- 
2.0.0

[PATCH 1/5] iommu/arm-smmu: fix calculation of TCR.T0SZ

[Will Deacon]

T0SZ controls the input address range for TTBR0, so use the input
address range rather than the output address range for the calculation.
For stage-2, this means using the output size of stage-1.

Signed-off-by: Will Deacon <will.deacon-5wv7dgnIgG8@public.gmane.org>
---
 drivers/iommu/arm-smmu.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index 1599354e974d..81e8ec290756 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -800,6 +800,8 @@ static void arm_smmu_init_context_bank(struct arm_smmu_domain *smmu_domain)
 			reg = TTBCR_TG0_64K;
 
 		if (!stage1) {
+			reg |= (64 - smmu->s1_output_size) << TTBCR_T0SZ_SHIFT;
+
 			switch (smmu->s2_output_size) {
 			case 32:
 				reg |= (TTBCR2_ADDR_32 << TTBCR_PASIZE_SHIFT);
@@ -821,7 +823,7 @@ static void arm_smmu_init_context_bank(struct arm_smmu_domain *smmu_domain)
 				break;
 			}
 		} else {
-			reg |= (64 - smmu->s1_output_size) << TTBCR_T0SZ_SHIFT;
+			reg |= (64 - smmu->input_size) << TTBCR_T0SZ_SHIFT;
 		}
 	} else {
 		reg = 0;
-- 
2.0.0

[PATCH 2/5] iommu/arm-smmu: add support for PCI master devices

[Will Deacon]

This patch extends the ARM SMMU driver so that it can handle PCI master
devices in addition to platform devices described in the device tree.

The driver is informed about the PCI host controller in the DT via a
phandle to the host controller in the mmu-masters property. The host
controller is then added to the master tree for that SMMU, just like a
normal master (although it probably doesn't advertise any StreamIDs).

When a device is added to the PCI bus, we set the archdata.iommu pointer
for that device to describe its StreamID (actually its RequesterID for
the moment). This allows us to re-use our existing data structures using
the host controller of_node for everything apart from StreamID
configuration, where we reach into the archdata for the information we
require.

Cc: Varun Sethi <varun.sethi-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
Signed-off-by: Will Deacon <will.deacon-5wv7dgnIgG8@public.gmane.org>
---
 drivers/iommu/arm-smmu.c | 242 ++++++++++++++++++++++++++++++-----------------
 1 file changed, 156 insertions(+), 86 deletions(-)

diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index 81e8ec290756..e4eebc50612c 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -39,6 +39,7 @@
 #include <linux/mm.h>
 #include <linux/module.h>
 #include <linux/of.h>
+#include <linux/pci.h>
 #include <linux/platform_device.h>
 #include <linux/slab.h>
 #include <linux/spinlock.h>
@@ -329,14 +330,7 @@ struct arm_smmu_smr {
 	u16				id;
 };
 
-struct arm_smmu_master {
-	struct device_node		*of_node;
-
-	/*
-	 * The following is specific to the master's position in the
-	 * SMMU chain.
-	 */
-	struct rb_node			node;
+struct arm_smmu_master_cfg {
 	int				num_streamids;
 	u16				streamids[MAX_MASTER_STREAMIDS];
 
@@ -347,6 +341,17 @@ struct arm_smmu_master {
 	struct arm_smmu_smr		*smrs;
 };
 
+struct arm_smmu_master {
+	struct device_node		*of_node;
+
+	/*
+	 * The following is specific to the master's position in the
+	 * SMMU chain.
+	 */
+	struct rb_node			node;
+	struct arm_smmu_master_cfg	cfg;
+};
+
 struct arm_smmu_device {
 	struct device			*dev;
 	struct device_node		*parent_of_node;
@@ -437,6 +442,18 @@ static void parse_driver_options(struct arm_smmu_device *smmu)
 	} while (arm_smmu_options[++i].opt);
 }
 
+static struct device *dev_get_master_dev(struct device *dev)
+{
+	if (dev_is_pci(dev)) {
+		struct pci_bus *bus = to_pci_dev(dev)->bus;
+		while (!pci_is_root_bus(bus))
+			bus = bus->parent;
+		return bus->bridge->parent;
+	}
+
+	return dev;
+}
+
 static struct arm_smmu_master *find_smmu_master(struct arm_smmu_device *smmu,
 						struct device_node *dev_node)
 {
@@ -457,6 +474,18 @@ static struct arm_smmu_master *find_smmu_master(struct arm_smmu_device *smmu,
 	return NULL;
 }
 
+static struct arm_smmu_master_cfg *
+find_smmu_master_cfg(struct arm_smmu_device *smmu, struct device *dev)
+{
+	struct arm_smmu_master *master;
+
+	if (dev_is_pci(dev))
+		return dev->archdata.iommu;
+
+	master = find_smmu_master(smmu, dev_get_master_dev(dev)->of_node);
+	return master ? &master->cfg : NULL;
+}
+
 static int insert_smmu_master(struct arm_smmu_device *smmu,
 			      struct arm_smmu_master *master)
 {
@@ -508,11 +537,11 @@ static int register_smmu_master(struct arm_smmu_device *smmu,
 	if (!master)
 		return -ENOMEM;
 
-	master->of_node		= masterspec->np;
-	master->num_streamids	= masterspec->args_count;
+	master->of_node			= masterspec->np;
+	master->cfg.num_streamids	= masterspec->args_count;
 
-	for (i = 0; i < master->num_streamids; ++i)
-		master->streamids[i] = masterspec->args[i];
+	for (i = 0; i < master->cfg.num_streamids; ++i)
+		master->cfg.streamids[i] = masterspec->args[i];
 
 	return insert_smmu_master(smmu, master);
 }
@@ -537,6 +566,42 @@ out_unlock:
 	return parent;
 }
 
+static struct arm_smmu_device *find_parent_smmu_for_device(struct device *dev)
+{
+	struct arm_smmu_device *child, *parent, *smmu;
+	struct arm_smmu_master *master = NULL;
+	struct device_node *dev_node = dev_get_master_dev(dev)->of_node;
+
+	spin_lock(&arm_smmu_devices_lock);
+	list_for_each_entry(parent, &arm_smmu_devices, list) {
+		smmu = parent;
+
+		/* Try to find a child of the current SMMU. */
+		list_for_each_entry(child, &arm_smmu_devices, list) {
+			if (child->parent_of_node == parent->dev->of_node) {
+				/* Does the child sit above our master? */
+				master = find_smmu_master(child, dev_node);
+				if (master) {
+					smmu = NULL;
+					break;
+				}
+			}
+		}
+
+		/* We found some children, so keep searching. */
+		if (!smmu) {
+			master = NULL;
+			continue;
+		}
+
+		master = find_smmu_master(smmu, dev_node);
+		if (master)
+			break;
+	}
+	spin_unlock(&arm_smmu_devices_lock);
+	return master ? smmu : NULL;
+}
+
 static int __arm_smmu_alloc_bitmap(unsigned long *map, int start, int end)
 {
 	int idx;
@@ -855,7 +920,8 @@ static void arm_smmu_init_context_bank(struct arm_smmu_domain *smmu_domain)
 }
 
 static int arm_smmu_init_domain_context(struct iommu_domain *domain,
-					struct device *dev)
+					struct device *dev,
+					struct arm_smmu_device *device_smmu)
 {
 	int irq, ret, start;
 	struct arm_smmu_domain *smmu_domain = domain->priv;
@@ -868,15 +934,15 @@ static int arm_smmu_init_domain_context(struct iommu_domain *domain,
 	 * early, and therefore check that the root SMMU does indeed have
 	 * a StreamID for the master in question.
 	 */
-	parent = dev->archdata.iommu;
+	parent = device_smmu;
 	smmu_domain->output_mask = -1;
 	do {
 		smmu = parent;
 		smmu_domain->output_mask &= (1ULL << smmu->s2_output_size) - 1;
 	} while ((parent = find_parent_smmu(smmu)));
 
-	if (!find_smmu_master(smmu, dev->of_node)) {
-		dev_err(dev, "unable to find root SMMU for device\n");
+	if (!find_smmu_master_cfg(smmu, dev)) {
+		dev_err(dev, "unable to find root SMMU config for device\n");
 		return -ENODEV;
 	}
 
@@ -920,7 +986,8 @@ static int arm_smmu_init_domain_context(struct iommu_domain *domain,
 
 	root_cfg->smmu = smmu;
 	arm_smmu_init_context_bank(smmu_domain);
-	return ret;
+	smmu_domain->leaf_smmu = device_smmu;
+	return 0;
 
 out_free_context:
 	__arm_smmu_free_bitmap(smmu->context_map, root_cfg->cbndx);
@@ -1056,7 +1123,7 @@ static void arm_smmu_domain_destroy(struct iommu_domain *domain)
 }
 
 static int arm_smmu_master_configure_smrs(struct arm_smmu_device *smmu,
-					  struct arm_smmu_master *master)
+					  struct arm_smmu_master_cfg *cfg)
 {
 	int i;
 	struct arm_smmu_smr *smrs;
@@ -1065,18 +1132,18 @@ static int arm_smmu_master_configure_smrs(struct arm_smmu_device *smmu,
 	if (!(smmu->features & ARM_SMMU_FEAT_STREAM_MATCH))
 		return 0;
 
-	if (master->smrs)
+	if (cfg->smrs)
 		return -EEXIST;
 
-	smrs = kmalloc(sizeof(*smrs) * master->num_streamids, GFP_KERNEL);
+	smrs = kmalloc(sizeof(*smrs) * cfg->num_streamids, GFP_KERNEL);
 	if (!smrs) {
-		dev_err(smmu->dev, "failed to allocate %d SMRs for master %s\n",
-			master->num_streamids, master->of_node->name);
+		dev_err(smmu->dev, "failed to allocate %d SMRs\n",
+			cfg->num_streamids);
 		return -ENOMEM;
 	}
 
 	/* Allocate the SMRs on the root SMMU */
-	for (i = 0; i < master->num_streamids; ++i) {
+	for (i = 0; i < cfg->num_streamids; ++i) {
 		int idx = __arm_smmu_alloc_bitmap(smmu->smr_map, 0,
 						  smmu->num_mapping_groups);
 		if (IS_ERR_VALUE(idx)) {
@@ -1087,18 +1154,18 @@ static int arm_smmu_master_configure_smrs(struct arm_smmu_device *smmu,
 		smrs[i] = (struct arm_smmu_smr) {
 			.idx	= idx,
 			.mask	= 0, /* We don't currently share SMRs */
-			.id	= master->streamids[i],
+			.id	= cfg->streamids[i],
 		};
 	}
 
 	/* It worked! Now, poke the actual hardware */
-	for (i = 0; i < master->num_streamids; ++i) {
+	for (i = 0; i < cfg->num_streamids; ++i) {
 		u32 reg = SMR_VALID | smrs[i].id << SMR_ID_SHIFT |
 			  smrs[i].mask << SMR_MASK_SHIFT;
 		writel_relaxed(reg, gr0_base + ARM_SMMU_GR0_SMR(smrs[i].idx));
 	}
 
-	master->smrs = smrs;
+	cfg->smrs = smrs;
 	return 0;
 
 err_free_smrs:
@@ -1109,44 +1176,44 @@ err_free_smrs:
 }
 
 static void arm_smmu_master_free_smrs(struct arm_smmu_device *smmu,
-				      struct arm_smmu_master *master)
+				      struct arm_smmu_master_cfg *cfg)
 {
 	int i;
 	void __iomem *gr0_base = ARM_SMMU_GR0(smmu);
-	struct arm_smmu_smr *smrs = master->smrs;
+	struct arm_smmu_smr *smrs = cfg->smrs;
 
 	/* Invalidate the SMRs before freeing back to the allocator */
-	for (i = 0; i < master->num_streamids; ++i) {
+	for (i = 0; i < cfg->num_streamids; ++i) {
 		u8 idx = smrs[i].idx;
 		writel_relaxed(~SMR_VALID, gr0_base + ARM_SMMU_GR0_SMR(idx));
 		__arm_smmu_free_bitmap(smmu->smr_map, idx);
 	}
 
-	master->smrs = NULL;
+	cfg->smrs = NULL;
 	kfree(smrs);
 }
 
 static void arm_smmu_bypass_stream_mapping(struct arm_smmu_device *smmu,
-					   struct arm_smmu_master *master)
+					   struct arm_smmu_master_cfg *cfg)
 {
 	int i;
 	void __iomem *gr0_base = ARM_SMMU_GR0(smmu);
 
-	for (i = 0; i < master->num_streamids; ++i) {
-		u16 sid = master->streamids[i];
+	for (i = 0; i < cfg->num_streamids; ++i) {
+		u16 sid = cfg->streamids[i];
 		writel_relaxed(S2CR_TYPE_BYPASS,
 			       gr0_base + ARM_SMMU_GR0_S2CR(sid));
 	}
 }
 
 static int arm_smmu_domain_add_master(struct arm_smmu_domain *smmu_domain,
-				      struct arm_smmu_master *master)
+				      struct arm_smmu_master_cfg *cfg)
 {
 	int i, ret;
 	struct arm_smmu_device *parent, *smmu = smmu_domain->root_cfg.smmu;
 	void __iomem *gr0_base = ARM_SMMU_GR0(smmu);
 
-	ret = arm_smmu_master_configure_smrs(smmu, master);
+	ret = arm_smmu_master_configure_smrs(smmu, cfg);
 	if (ret)
 		return ret;
 
@@ -1161,14 +1228,14 @@ static int arm_smmu_domain_add_master(struct arm_smmu_domain *smmu_domain,
 		if (smmu->features & ARM_SMMU_FEAT_STREAM_MATCH)
 			continue;
 
-		arm_smmu_bypass_stream_mapping(smmu, master);
+		arm_smmu_bypass_stream_mapping(smmu, cfg);
 		smmu = parent;
 	}
 
 	/* Now we're at the root, time to point at our context bank */
-	for (i = 0; i < master->num_streamids; ++i) {
+	for (i = 0; i < cfg->num_streamids; ++i) {
 		u32 idx, s2cr;
-		idx = master->smrs ? master->smrs[i].idx : master->streamids[i];
+		idx = cfg->smrs ? cfg->smrs[i].idx : cfg->streamids[i];
 		s2cr = S2CR_TYPE_TRANS |
 		       (smmu_domain->root_cfg.cbndx << S2CR_CBNDX_SHIFT);
 		writel_relaxed(s2cr, gr0_base + ARM_SMMU_GR0_S2CR(idx));
@@ -1178,7 +1245,7 @@ static int arm_smmu_domain_add_master(struct arm_smmu_domain *smmu_domain,
 }
 
 static void arm_smmu_domain_remove_master(struct arm_smmu_domain *smmu_domain,
-					  struct arm_smmu_master *master)
+					  struct arm_smmu_master_cfg *cfg)
 {
 	struct arm_smmu_device *smmu = smmu_domain->root_cfg.smmu;
 
@@ -1186,18 +1253,19 @@ static void arm_smmu_domain_remove_master(struct arm_smmu_domain *smmu_domain,
 	 * We *must* clear the S2CR first, because freeing the SMR means
 	 * that it can be re-allocated immediately.
 	 */
-	arm_smmu_bypass_stream_mapping(smmu, master);
-	arm_smmu_master_free_smrs(smmu, master);
+	arm_smmu_bypass_stream_mapping(smmu, cfg);
+	arm_smmu_master_free_smrs(smmu, cfg);
 }
 
 static int arm_smmu_attach_dev(struct iommu_domain *domain, struct device *dev)
 {
 	int ret = -EINVAL;
 	struct arm_smmu_domain *smmu_domain = domain->priv;
-	struct arm_smmu_device *device_smmu = dev->archdata.iommu;
-	struct arm_smmu_master *master;
+	struct arm_smmu_device *device_smmu;
+	struct arm_smmu_master_cfg *cfg;
 	unsigned long flags;
 
+	device_smmu = dev_get_master_dev(dev)->archdata.iommu;
 	if (!device_smmu) {
 		dev_err(dev, "cannot attach to SMMU, is it on the same bus?\n");
 		return -ENXIO;
@@ -1210,11 +1278,9 @@ static int arm_smmu_attach_dev(struct iommu_domain *domain, struct device *dev)
 	spin_lock_irqsave(&smmu_domain->lock, flags);
 	if (!smmu_domain->leaf_smmu) {
 		/* Now that we have a master, we can finalise the domain */
-		ret = arm_smmu_init_domain_context(domain, dev);
+		ret = arm_smmu_init_domain_context(domain, dev, device_smmu);
 		if (IS_ERR_VALUE(ret))
 			goto err_unlock;
-
-		smmu_domain->leaf_smmu = device_smmu;
 	} else if (smmu_domain->leaf_smmu != device_smmu) {
 		dev_err(dev,
 			"cannot attach to SMMU %s whilst already attached to domain on SMMU %s\n",
@@ -1225,11 +1291,11 @@ static int arm_smmu_attach_dev(struct iommu_domain *domain, struct device *dev)
 	spin_unlock_irqrestore(&smmu_domain->lock, flags);
 
 	/* Looks ok, so add the device to the domain */
-	master = find_smmu_master(smmu_domain->leaf_smmu, dev->of_node);
-	if (!master)
+	cfg = find_smmu_master_cfg(smmu_domain->leaf_smmu, dev);
+	if (!cfg)
 		return -ENODEV;
 
-	return arm_smmu_domain_add_master(smmu_domain, master);
+	return arm_smmu_domain_add_master(smmu_domain, cfg);
 
 err_unlock:
 	spin_unlock_irqrestore(&smmu_domain->lock, flags);
@@ -1239,11 +1305,11 @@ err_unlock:
 static void arm_smmu_detach_dev(struct iommu_domain *domain, struct device *dev)
 {
 	struct arm_smmu_domain *smmu_domain = domain->priv;
-	struct arm_smmu_master *master;
+	struct arm_smmu_master_cfg *cfg;
 
-	master = find_smmu_master(smmu_domain->leaf_smmu, dev->of_node);
-	if (master)
-		arm_smmu_domain_remove_master(smmu_domain, master);
+	cfg = find_smmu_master_cfg(smmu_domain->leaf_smmu, dev);
+	if (cfg)
+		arm_smmu_domain_remove_master(smmu_domain, cfg);
 }
 
 static bool arm_smmu_pte_is_contiguous_range(unsigned long addr,
@@ -1549,10 +1615,15 @@ static int arm_smmu_domain_has_cap(struct iommu_domain *domain,
 	return !!(cap & caps);
 }
 
+static int __arm_smmu_get_pci_sid(struct pci_dev *pdev, u16 alias, void *data)
+{
+	*((u16 *)data) = alias;
+	return 0; /* Continue walking */
+}
+
 static int arm_smmu_add_device(struct device *dev)
 {
-	struct arm_smmu_device *child, *parent, *smmu;
-	struct arm_smmu_master *master = NULL;
+	struct arm_smmu_device *smmu;
 	struct iommu_group *group;
 	int ret;
 
@@ -1561,35 +1632,8 @@ static int arm_smmu_add_device(struct device *dev)
 		return -EINVAL;
 	}
 
-	spin_lock(&arm_smmu_devices_lock);
-	list_for_each_entry(parent, &arm_smmu_devices, list) {
-		smmu = parent;
-
-		/* Try to find a child of the current SMMU. */
-		list_for_each_entry(child, &arm_smmu_devices, list) {
-			if (child->parent_of_node == parent->dev->of_node) {
-				/* Does the child sit above our master? */
-				master = find_smmu_master(child, dev->of_node);
-				if (master) {
-					smmu = NULL;
-					break;
-				}
-			}
-		}
-
-		/* We found some children, so keep searching. */
-		if (!smmu) {
-			master = NULL;
-			continue;
-		}
-
-		master = find_smmu_master(smmu, dev->of_node);
-		if (master)
-			break;
-	}
-	spin_unlock(&arm_smmu_devices_lock);
-
-	if (!master)
+	smmu = find_parent_smmu_for_device(dev);
+	if (!smmu)
 		return -ENODEV;
 
 	group = iommu_group_alloc();
@@ -1598,15 +1642,36 @@ static int arm_smmu_add_device(struct device *dev)
 		return PTR_ERR(group);
 	}
 
+	if (dev_is_pci(dev)) {
+		struct arm_smmu_master_cfg *cfg;
+		struct pci_dev *pdev = to_pci_dev(dev);
+
+		cfg = kzalloc(sizeof(*cfg), GFP_KERNEL);
+		if (!cfg) {
+			ret = -ENOMEM;
+			goto out_put_group;
+		}
+
+		cfg->num_streamids = 1;
+		pci_for_each_dma_alias(pdev, __arm_smmu_get_pci_sid,
+				       &cfg->streamids[0]);
+		dev->archdata.iommu = cfg;
+	} else {
+		dev->archdata.iommu = smmu;
+	}
+
 	ret = iommu_group_add_device(group, dev);
-	iommu_group_put(group);
-	dev->archdata.iommu = smmu;
 
+out_put_group:
+	iommu_group_put(group);
 	return ret;
 }
 
 static void arm_smmu_remove_device(struct device *dev)
 {
+	if (dev_is_pci(dev))
+		kfree(dev->archdata.iommu);
+
 	dev->archdata.iommu = NULL;
 	iommu_group_remove_device(dev);
 }
@@ -2050,6 +2115,11 @@ static int __init arm_smmu_init(void)
 		bus_set_iommu(&amba_bustype, &arm_smmu_ops);
 #endif
 
+#ifdef CONFIG_PCI
+	if (!iommu_present(&pci_bus_type))
+		bus_set_iommu(&pci_bus_type, &arm_smmu_ops);
+#endif
+
 	return 0;
 }
 
-- 
2.0.0

RE: [PATCH 2/5] iommu/arm-smmu: add support for PCI master devices

[Varun Sethi]

Hi Will,

> -----Original Message-----
> From: Will Deacon [mailto:will.deacon-5wv7dgnIgG8@public.gmane.org]
> Sent: Monday, June 30, 2014 4:22 PM
> To: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org; iommu-cunTk1MwBs/ROKNJybVBZg@public.gmane.org
> foundation.org
> Cc: thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org; arnd-r2nGTMty4D4@public.gmane.org; ohaugan-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org;
> Sethi Varun-B16395; joro-zLv9SwRftAIdnm+yROfE0A@public.gmane.org; a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org;
> marc.zyngier-5wv7dgnIgG8@public.gmane.org; Will Deacon; Sethi Varun-B16395
> Subject: [PATCH 2/5] iommu/arm-smmu: add support for PCI master devices
> 
> This patch extends the ARM SMMU driver so that it can handle PCI master
> devices in addition to platform devices described in the device tree.
> 
> The driver is informed about the PCI host controller in the DT via a
> phandle to the host controller in the mmu-masters property. The host
> controller is then added to the master tree for that SMMU, just like a
> normal master (although it probably doesn't advertise any StreamIDs).
> 
> When a device is added to the PCI bus, we set the archdata.iommu pointer
> for that device to describe its StreamID (actually its RequesterID for
> the moment). This allows us to re-use our existing data structures using
> the host controller of_node for everything apart from StreamID
> configuration, where we reach into the archdata for the information we
> require.
> 
> Cc: Varun Sethi <varun.sethi-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
> Signed-off-by: Will Deacon <will.deacon-5wv7dgnIgG8@public.gmane.org>
> ---
>  drivers/iommu/arm-smmu.c | 242 ++++++++++++++++++++++++++++++-----------
> ------
>  1 file changed, 156 insertions(+), 86 deletions(-)
> 
> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c index
> 81e8ec290756..e4eebc50612c 100644
> --- a/drivers/iommu/arm-smmu.c
> +++ b/drivers/iommu/arm-smmu.c
> @@ -39,6 +39,7 @@
>  #include <linux/mm.h>
>  #include <linux/module.h>
>  #include <linux/of.h>
> +#include <linux/pci.h>
>  #include <linux/platform_device.h>
>  #include <linux/slab.h>
>  #include <linux/spinlock.h>
> @@ -329,14 +330,7 @@ struct arm_smmu_smr {
>  	u16				id;
>  };
> 
> -struct arm_smmu_master {
> -	struct device_node		*of_node;
> -
> -	/*
> -	 * The following is specific to the master's position in the
> -	 * SMMU chain.
> -	 */
> -	struct rb_node			node;
> +struct arm_smmu_master_cfg {
>  	int				num_streamids;
>  	u16				streamids[MAX_MASTER_STREAMIDS];
> 
> @@ -347,6 +341,17 @@ struct arm_smmu_master {
>  	struct arm_smmu_smr		*smrs;
>  };
> 
> +struct arm_smmu_master {
> +	struct device_node		*of_node;
> +
> +	/*
> +	 * The following is specific to the master's position in the
> +	 * SMMU chain.
> +	 */
> +	struct rb_node			node;
> +	struct arm_smmu_master_cfg	cfg;
> +};
> +
>  struct arm_smmu_device {
>  	struct device			*dev;
>  	struct device_node		*parent_of_node;
> @@ -437,6 +442,18 @@ static void parse_driver_options(struct
> arm_smmu_device *smmu)
>  	} while (arm_smmu_options[++i].opt);
>  }
> 
> +static struct device *dev_get_master_dev(struct device *dev) {
> +	if (dev_is_pci(dev)) {
> +		struct pci_bus *bus = to_pci_dev(dev)->bus;
> +		while (!pci_is_root_bus(bus))
> +			bus = bus->parent;
> +		return bus->bridge->parent;
> +	}
> +
> +	return dev;
> +}
> +
>  static struct arm_smmu_master *find_smmu_master(struct arm_smmu_device
> *smmu,
>  						struct device_node *dev_node)
>  {
> @@ -457,6 +474,18 @@ static struct arm_smmu_master
> *find_smmu_master(struct arm_smmu_device *smmu,
>  	return NULL;
>  }
> 
> +static struct arm_smmu_master_cfg *
> +find_smmu_master_cfg(struct arm_smmu_device *smmu, struct device *dev)
> +{
> +	struct arm_smmu_master *master;
> +
> +	if (dev_is_pci(dev))
> +		return dev->archdata.iommu;
> +
> +	master = find_smmu_master(smmu, dev_get_master_dev(dev)->of_node);
[Sethi Varun-B16395] Why use dev_get_master_dev over here? You can simply use dev.

> +	return master ? &master->cfg : NULL;
> +}
> +
>  static int insert_smmu_master(struct arm_smmu_device *smmu,
>  			      struct arm_smmu_master *master)  { @@ -508,11
> +537,11 @@ static int register_smmu_master(struct arm_smmu_device *smmu,
>  	if (!master)
>  		return -ENOMEM;
> 
> -	master->of_node		= masterspec->np;
> -	master->num_streamids	= masterspec->args_count;
> +	master->of_node			= masterspec->np;
> +	master->cfg.num_streamids	= masterspec->args_count;
> 
> -	for (i = 0; i < master->num_streamids; ++i)
> -		master->streamids[i] = masterspec->args[i];
> +	for (i = 0; i < master->cfg.num_streamids; ++i)
> +		master->cfg.streamids[i] = masterspec->args[i];
> 
>  	return insert_smmu_master(smmu, master);  } @@ -537,6 +566,42 @@
> out_unlock:
>  	return parent;
>  }
> 
> +static struct arm_smmu_device *find_parent_smmu_for_device(struct
> +device *dev) {
> +	struct arm_smmu_device *child, *parent, *smmu;
> +	struct arm_smmu_master *master = NULL;
> +	struct device_node *dev_node = dev_get_master_dev(dev)->of_node;
> +
> +	spin_lock(&arm_smmu_devices_lock);
> +	list_for_each_entry(parent, &arm_smmu_devices, list) {
> +		smmu = parent;
> +
> +		/* Try to find a child of the current SMMU. */
> +		list_for_each_entry(child, &arm_smmu_devices, list) {
> +			if (child->parent_of_node == parent->dev->of_node) {
> +				/* Does the child sit above our master? */
> +				master = find_smmu_master(child, dev_node);
> +				if (master) {
> +					smmu = NULL;
> +					break;
> +				}
> +			}
> +		}
> +
> +		/* We found some children, so keep searching. */
> +		if (!smmu) {
> +			master = NULL;
> +			continue;
> +		}
> +
> +		master = find_smmu_master(smmu, dev_node);
> +		if (master)
> +			break;
> +	}
> +	spin_unlock(&arm_smmu_devices_lock);
> +	return master ? smmu : NULL;
> +}
> +
>  static int __arm_smmu_alloc_bitmap(unsigned long *map, int start, int
> end)  {
>  	int idx;
> @@ -855,7 +920,8 @@ static void arm_smmu_init_context_bank(struct
> arm_smmu_domain *smmu_domain)  }
> 
>  static int arm_smmu_init_domain_context(struct iommu_domain *domain,
> -					struct device *dev)
> +					struct device *dev,
> +					struct arm_smmu_device *device_smmu)
>  {
>  	int irq, ret, start;
>  	struct arm_smmu_domain *smmu_domain = domain->priv; @@ -868,15
> +934,15 @@ static int arm_smmu_init_domain_context(struct iommu_domain
> *domain,
>  	 * early, and therefore check that the root SMMU does indeed have
>  	 * a StreamID for the master in question.
>  	 */
> -	parent = dev->archdata.iommu;
> +	parent = device_smmu;
>  	smmu_domain->output_mask = -1;
>  	do {
>  		smmu = parent;
>  		smmu_domain->output_mask &= (1ULL << smmu->s2_output_size) -
> 1;
>  	} while ((parent = find_parent_smmu(smmu)));
> 
> -	if (!find_smmu_master(smmu, dev->of_node)) {
> -		dev_err(dev, "unable to find root SMMU for device\n");
> +	if (!find_smmu_master_cfg(smmu, dev)) {
> +		dev_err(dev, "unable to find root SMMU config for device\n");
>  		return -ENODEV;
>  	}
> 
> @@ -920,7 +986,8 @@ static int arm_smmu_init_domain_context(struct
> iommu_domain *domain,
> 
>  	root_cfg->smmu = smmu;
>  	arm_smmu_init_context_bank(smmu_domain);
> -	return ret;
> +	smmu_domain->leaf_smmu = device_smmu;
> +	return 0;
> 
>  out_free_context:
>  	__arm_smmu_free_bitmap(smmu->context_map, root_cfg->cbndx); @@ -
> 1056,7 +1123,7 @@ static void arm_smmu_domain_destroy(struct iommu_domain
> *domain)  }
> 
>  static int arm_smmu_master_configure_smrs(struct arm_smmu_device *smmu,
> -					  struct arm_smmu_master *master)
> +					  struct arm_smmu_master_cfg *cfg)
>  {
>  	int i;
>  	struct arm_smmu_smr *smrs;
> @@ -1065,18 +1132,18 @@ static int arm_smmu_master_configure_smrs(struct
> arm_smmu_device *smmu,
>  	if (!(smmu->features & ARM_SMMU_FEAT_STREAM_MATCH))
>  		return 0;
> 
> -	if (master->smrs)
> +	if (cfg->smrs)
>  		return -EEXIST;
> 
> -	smrs = kmalloc(sizeof(*smrs) * master->num_streamids, GFP_KERNEL);
> +	smrs = kmalloc(sizeof(*smrs) * cfg->num_streamids, GFP_KERNEL);
>  	if (!smrs) {
> -		dev_err(smmu->dev, "failed to allocate %d SMRs for master
> %s\n",
> -			master->num_streamids, master->of_node->name);
> +		dev_err(smmu->dev, "failed to allocate %d SMRs\n",
> +			cfg->num_streamids);
>  		return -ENOMEM;
>  	}
> 
>  	/* Allocate the SMRs on the root SMMU */
> -	for (i = 0; i < master->num_streamids; ++i) {
> +	for (i = 0; i < cfg->num_streamids; ++i) {
>  		int idx = __arm_smmu_alloc_bitmap(smmu->smr_map, 0,
>  						  smmu->num_mapping_groups);
>  		if (IS_ERR_VALUE(idx)) {
> @@ -1087,18 +1154,18 @@ static int arm_smmu_master_configure_smrs(struct
> arm_smmu_device *smmu,
>  		smrs[i] = (struct arm_smmu_smr) {
>  			.idx	= idx,
>  			.mask	= 0, /* We don't currently share SMRs */
> -			.id	= master->streamids[i],
> +			.id	= cfg->streamids[i],
>  		};
>  	}
> 
>  	/* It worked! Now, poke the actual hardware */
> -	for (i = 0; i < master->num_streamids; ++i) {
> +	for (i = 0; i < cfg->num_streamids; ++i) {
>  		u32 reg = SMR_VALID | smrs[i].id << SMR_ID_SHIFT |
>  			  smrs[i].mask << SMR_MASK_SHIFT;
>  		writel_relaxed(reg, gr0_base +
> ARM_SMMU_GR0_SMR(smrs[i].idx));
>  	}
> 
> -	master->smrs = smrs;
> +	cfg->smrs = smrs;
>  	return 0;
> 
>  err_free_smrs:
> @@ -1109,44 +1176,44 @@ err_free_smrs:
>  }
> 
>  static void arm_smmu_master_free_smrs(struct arm_smmu_device *smmu,
> -				      struct arm_smmu_master *master)
> +				      struct arm_smmu_master_cfg *cfg)
>  {
>  	int i;
>  	void __iomem *gr0_base = ARM_SMMU_GR0(smmu);
> -	struct arm_smmu_smr *smrs = master->smrs;
> +	struct arm_smmu_smr *smrs = cfg->smrs;
> 
>  	/* Invalidate the SMRs before freeing back to the allocator */
> -	for (i = 0; i < master->num_streamids; ++i) {
> +	for (i = 0; i < cfg->num_streamids; ++i) {
>  		u8 idx = smrs[i].idx;
>  		writel_relaxed(~SMR_VALID, gr0_base + ARM_SMMU_GR0_SMR(idx));
>  		__arm_smmu_free_bitmap(smmu->smr_map, idx);
>  	}
> 
> -	master->smrs = NULL;
> +	cfg->smrs = NULL;
>  	kfree(smrs);
>  }
> 
>  static void arm_smmu_bypass_stream_mapping(struct arm_smmu_device *smmu,
> -					   struct arm_smmu_master *master)
> +					   struct arm_smmu_master_cfg *cfg)
>  {
>  	int i;
>  	void __iomem *gr0_base = ARM_SMMU_GR0(smmu);
> 
> -	for (i = 0; i < master->num_streamids; ++i) {
> -		u16 sid = master->streamids[i];
> +	for (i = 0; i < cfg->num_streamids; ++i) {
> +		u16 sid = cfg->streamids[i];
>  		writel_relaxed(S2CR_TYPE_BYPASS,
>  			       gr0_base + ARM_SMMU_GR0_S2CR(sid));
>  	}
>  }
> 
>  static int arm_smmu_domain_add_master(struct arm_smmu_domain
> *smmu_domain,
> -				      struct arm_smmu_master *master)
> +				      struct arm_smmu_master_cfg *cfg)
>  {
>  	int i, ret;
>  	struct arm_smmu_device *parent, *smmu = smmu_domain->root_cfg.smmu;
>  	void __iomem *gr0_base = ARM_SMMU_GR0(smmu);
> 
> -	ret = arm_smmu_master_configure_smrs(smmu, master);
> +	ret = arm_smmu_master_configure_smrs(smmu, cfg);
>  	if (ret)
>  		return ret;
> 
> @@ -1161,14 +1228,14 @@ static int arm_smmu_domain_add_master(struct
> arm_smmu_domain *smmu_domain,
>  		if (smmu->features & ARM_SMMU_FEAT_STREAM_MATCH)
>  			continue;
> 
> -		arm_smmu_bypass_stream_mapping(smmu, master);
> +		arm_smmu_bypass_stream_mapping(smmu, cfg);
>  		smmu = parent;
>  	}
> 
>  	/* Now we're at the root, time to point at our context bank */
> -	for (i = 0; i < master->num_streamids; ++i) {
> +	for (i = 0; i < cfg->num_streamids; ++i) {
>  		u32 idx, s2cr;
> -		idx = master->smrs ? master->smrs[i].idx : master-
> >streamids[i];
> +		idx = cfg->smrs ? cfg->smrs[i].idx : cfg->streamids[i];
>  		s2cr = S2CR_TYPE_TRANS |
>  		       (smmu_domain->root_cfg.cbndx << S2CR_CBNDX_SHIFT);
>  		writel_relaxed(s2cr, gr0_base + ARM_SMMU_GR0_S2CR(idx)); @@ -
> 1178,7 +1245,7 @@ static int arm_smmu_domain_add_master(struct
> arm_smmu_domain *smmu_domain,  }
> 
>  static void arm_smmu_domain_remove_master(struct arm_smmu_domain
> *smmu_domain,
> -					  struct arm_smmu_master *master)
> +					  struct arm_smmu_master_cfg *cfg)
>  {
>  	struct arm_smmu_device *smmu = smmu_domain->root_cfg.smmu;
> 
> @@ -1186,18 +1253,19 @@ static void arm_smmu_domain_remove_master(struct
> arm_smmu_domain *smmu_domain,
>  	 * We *must* clear the S2CR first, because freeing the SMR means
>  	 * that it can be re-allocated immediately.
>  	 */
> -	arm_smmu_bypass_stream_mapping(smmu, master);
> -	arm_smmu_master_free_smrs(smmu, master);
> +	arm_smmu_bypass_stream_mapping(smmu, cfg);
> +	arm_smmu_master_free_smrs(smmu, cfg);
>  }
> 
>  static int arm_smmu_attach_dev(struct iommu_domain *domain, struct
> device *dev)  {
>  	int ret = -EINVAL;
>  	struct arm_smmu_domain *smmu_domain = domain->priv;
> -	struct arm_smmu_device *device_smmu = dev->archdata.iommu;
> -	struct arm_smmu_master *master;
> +	struct arm_smmu_device *device_smmu;
> +	struct arm_smmu_master_cfg *cfg;
>  	unsigned long flags;
> 
> +	device_smmu = dev_get_master_dev(dev)->archdata.iommu;
>  	if (!device_smmu) {
>  		dev_err(dev, "cannot attach to SMMU, is it on the same
> bus?\n");
>  		return -ENXIO;
> @@ -1210,11 +1278,9 @@ static int arm_smmu_attach_dev(struct iommu_domain
> *domain, struct device *dev)
>  	spin_lock_irqsave(&smmu_domain->lock, flags);
>  	if (!smmu_domain->leaf_smmu) {
>  		/* Now that we have a master, we can finalise the domain */
> -		ret = arm_smmu_init_domain_context(domain, dev);
> +		ret = arm_smmu_init_domain_context(domain, dev, device_smmu);
>  		if (IS_ERR_VALUE(ret))
>  			goto err_unlock;
> -
> -		smmu_domain->leaf_smmu = device_smmu;
>  	} else if (smmu_domain->leaf_smmu != device_smmu) {
>  		dev_err(dev,
>  			"cannot attach to SMMU %s whilst already attached to
> domain on SMMU %s\n", @@ -1225,11 +1291,11 @@ static int
> arm_smmu_attach_dev(struct iommu_domain *domain, struct device *dev)
>  	spin_unlock_irqrestore(&smmu_domain->lock, flags);
> 
>  	/* Looks ok, so add the device to the domain */
> -	master = find_smmu_master(smmu_domain->leaf_smmu, dev->of_node);
> -	if (!master)
> +	cfg = find_smmu_master_cfg(smmu_domain->leaf_smmu, dev);
> +	if (!cfg)
>  		return -ENODEV;
> 
> -	return arm_smmu_domain_add_master(smmu_domain, master);
> +	return arm_smmu_domain_add_master(smmu_domain, cfg);
> 
>  err_unlock:
>  	spin_unlock_irqrestore(&smmu_domain->lock, flags); @@ -1239,11
> +1305,11 @@ err_unlock:
>  static void arm_smmu_detach_dev(struct iommu_domain *domain, struct
> device *dev)  {
>  	struct arm_smmu_domain *smmu_domain = domain->priv;
> -	struct arm_smmu_master *master;
> +	struct arm_smmu_master_cfg *cfg;
> 
> -	master = find_smmu_master(smmu_domain->leaf_smmu, dev->of_node);
> -	if (master)
> -		arm_smmu_domain_remove_master(smmu_domain, master);
> +	cfg = find_smmu_master_cfg(smmu_domain->leaf_smmu, dev);
> +	if (cfg)
> +		arm_smmu_domain_remove_master(smmu_domain, cfg);
>  }
> 
>  static bool arm_smmu_pte_is_contiguous_range(unsigned long addr, @@ -
> 1549,10 +1615,15 @@ static int arm_smmu_domain_has_cap(struct
> iommu_domain *domain,
>  	return !!(cap & caps);
>  }
> 
> +static int __arm_smmu_get_pci_sid(struct pci_dev *pdev, u16 alias, void
> +*data) {
> +	*((u16 *)data) = alias;
> +	return 0; /* Continue walking */
> +}
> +
>  static int arm_smmu_add_device(struct device *dev)  {
> -	struct arm_smmu_device *child, *parent, *smmu;
> -	struct arm_smmu_master *master = NULL;
> +	struct arm_smmu_device *smmu;
>  	struct iommu_group *group;
>  	int ret;
> 
> @@ -1561,35 +1632,8 @@ static int arm_smmu_add_device(struct device *dev)
>  		return -EINVAL;
>  	}
> 
> -	spin_lock(&arm_smmu_devices_lock);
> -	list_for_each_entry(parent, &arm_smmu_devices, list) {
> -		smmu = parent;
> -
> -		/* Try to find a child of the current SMMU. */
> -		list_for_each_entry(child, &arm_smmu_devices, list) {
> -			if (child->parent_of_node == parent->dev->of_node) {
> -				/* Does the child sit above our master? */
> -				master = find_smmu_master(child, dev->of_node);
> -				if (master) {
> -					smmu = NULL;
> -					break;
> -				}
> -			}
> -		}
> -
> -		/* We found some children, so keep searching. */
> -		if (!smmu) {
> -			master = NULL;
> -			continue;
> -		}
> -
> -		master = find_smmu_master(smmu, dev->of_node);
> -		if (master)
> -			break;
> -	}
> -	spin_unlock(&arm_smmu_devices_lock);
> -
> -	if (!master)
> +	smmu = find_parent_smmu_for_device(dev);
> +	if (!smmu)
>  		return -ENODEV;
> 
>  	group = iommu_group_alloc();
> @@ -1598,15 +1642,36 @@ static int arm_smmu_add_device(struct device
> *dev)
>  		return PTR_ERR(group);
>  	}
> 
> +	if (dev_is_pci(dev)) {
> +		struct arm_smmu_master_cfg *cfg;
> +		struct pci_dev *pdev = to_pci_dev(dev);
> +
> +		cfg = kzalloc(sizeof(*cfg), GFP_KERNEL);
> +		if (!cfg) {
> +			ret = -ENOMEM;
> +			goto out_put_group;
> +		}
> +
> +		cfg->num_streamids = 1;
> +		pci_for_each_dma_alias(pdev, __arm_smmu_get_pci_sid,
> +				       &cfg->streamids[0]);
[Sethi Varun-B16395] We need to look for upstream DMA device. We should be using pci_find_dma_isolation_root here. Also, this would also imply that there could be multiple devices sharing the same stream ID. So, we should check if a particular stream ID value has already been configured in the SMR registers.

-Varun

Re: [PATCH 2/5] iommu/arm-smmu: add support for PCI master devices

[Will Deacon]

On Thu, Jul 03, 2014 at 03:22:37PM +0100, Varun Sethi wrote:
> Hi Will,

Hi Varun,

Thanks for taking a look at this!

> > +static struct arm_smmu_master_cfg *
> > +find_smmu_master_cfg(struct arm_smmu_device *smmu, struct device *dev)
> > +{
> > +     struct arm_smmu_master *master;
> > +
> > +     if (dev_is_pci(dev))
> > +             return dev->archdata.iommu;
> > +
> > +     master = find_smmu_master(smmu, dev_get_master_dev(dev)->of_node);
> [Sethi Varun-B16395] Why use dev_get_master_dev over here? You can simply use dev.

True; we've already done the PCI check above. I'll tidy this up.

> > @@ -1598,15 +1642,36 @@ static int arm_smmu_add_device(struct device
> > *dev)
> >               return PTR_ERR(group);
> >       }
> >
> > +     if (dev_is_pci(dev)) {
> > +             struct arm_smmu_master_cfg *cfg;
> > +             struct pci_dev *pdev = to_pci_dev(dev);
> > +
> > +             cfg = kzalloc(sizeof(*cfg), GFP_KERNEL);
> > +             if (!cfg) {
> > +                     ret = -ENOMEM;
> > +                     goto out_put_group;
> > +             }
> > +
> > +             cfg->num_streamids = 1;
> > +             pci_for_each_dma_alias(pdev, __arm_smmu_get_pci_sid,
> > +                                    &cfg->streamids[0]);
> [Sethi Varun-B16395] We need to look for upstream DMA device. We should be
> using pci_find_dma_isolation_root here. Also, this would also imply that
> there could be multiple devices sharing the same stream ID. So, we should
> check if a particular stream ID value has already been configured in the
> SMR registers.

That function doesn't seem to appear in mainline or next. I can move to it
when it's available, but in the meantime the above is working for me. I'm
making the assumption here that the system is configured so that there
aren't any duplicate stream IDs. What we actually need is a function here
which maps the requester ID to a stream ID using firmware tables (provided
by DT or ACPI). In the absence of those tables at the moment, I just assign
the ID directly, which happens to work on my platform (1:1 mapping).

Once Thierry's generic IOMMU binding is sorted, we should look at adding
support for the Stream ID description. Have you looked at that at all?

Will

BTW: You seem to have a rather strange quoting style on your replies. Is
there any way to configure your editor to limit your lines to 80 columns?
You also don't need the prefix with your name and number in brackets!

RE: [PATCH 2/5] iommu/arm-smmu: add support for PCI master devices

[Varun Sethi]

Hi Will,

> -----Original Message-----
> From: Will Deacon [mailto:will.deacon-5wv7dgnIgG8@public.gmane.org]
> Sent: Thursday, July 03, 2014 8:14 PM
> To: Sethi Varun-B16395
> Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org; iommu-cunTk1MwBs/ROKNJybVBZg@public.gmane.org
> foundation.org; thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org; arnd-r2nGTMty4D4@public.gmane.org;
> ohaugan-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org; joro-zLv9SwRftAIdnm+yROfE0A@public.gmane.org;
> a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org; Marc Zyngier
> Subject: Re: [PATCH 2/5] iommu/arm-smmu: add support for PCI master
> devices
> 
> On Thu, Jul 03, 2014 at 03:22:37PM +0100, Varun Sethi wrote:
> > Hi Will,
> 
> Hi Varun,
> 
> Thanks for taking a look at this!
> 
> > > +static struct arm_smmu_master_cfg * find_smmu_master_cfg(struct
> > > +arm_smmu_device *smmu, struct device *dev) {
> > > +     struct arm_smmu_master *master;
> > > +
> > > +     if (dev_is_pci(dev))
> > > +             return dev->archdata.iommu;
> > > +
> > > +     master = find_smmu_master(smmu,
> > > + dev_get_master_dev(dev)->of_node);
> > [Sethi Varun-B16395] Why use dev_get_master_dev over here? You can
> simply use dev.
> 
> True; we've already done the PCI check above. I'll tidy this up.
> 
> > > @@ -1598,15 +1642,36 @@ static int arm_smmu_add_device(struct device
> > > *dev)
> > >               return PTR_ERR(group);
> > >       }
> > >
> > > +     if (dev_is_pci(dev)) {
> > > +             struct arm_smmu_master_cfg *cfg;
> > > +             struct pci_dev *pdev = to_pci_dev(dev);
> > > +
> > > +             cfg = kzalloc(sizeof(*cfg), GFP_KERNEL);
> > > +             if (!cfg) {
> > > +                     ret = -ENOMEM;
> > > +                     goto out_put_group;
> > > +             }
> > > +
> > > +             cfg->num_streamids = 1;
> > > +             pci_for_each_dma_alias(pdev, __arm_smmu_get_pci_sid,
> > > +                                    &cfg->streamids[0]);
> > [Sethi Varun-B16395] We need to look for upstream DMA device. We
> > should be using pci_find_dma_isolation_root here. Also, this would
> > also imply that there could be multiple devices sharing the same
> > stream ID. So, we should check if a particular stream ID value has
> > already been configured in the SMR registers.
> 
> That function doesn't seem to appear in mainline or next. I can move to
> it when it's available, but in the meantime the above is working for me.
> I'm making the assumption here that the system is configured so that
> there aren't any duplicate stream IDs. What we actually need is a
> function here which maps the requester ID to a stream ID using firmware
> tables (provided by DT or ACPI). In the absence of those tables at the
> moment, I just assign the ID directly, which happens to work on my
> platform (1:1 mapping).
> 
> Once Thierry's generic IOMMU binding is sorted, we should look at adding
> support for the Stream ID description. Have you looked at that at all?
> 
Yes, I have looked at the bindings. Would we need to represent the stream ids for PCI devices in the device tree? Why do we want to depend on the firmware to map the requestor id to the stream id? It can be handled using the APIs proposed by Alex Williamson. This is similar to IOMMU group determination, which is handled by the IOMMU driver.

> Will
> 
> BTW: You seem to have a rather strange quoting style on your replies. Is
> there any way to configure your editor to limit your lines to 80 columns?
> You also don't need the prefix with your name and number in brackets!


Will try to sort out issues with my e-mail client.

-Varun

Re: [PATCH 2/5] iommu/arm-smmu: add support for PCI master devices

[Will Deacon]

On Fri, Jul 04, 2014 at 08:41:53AM +0100, Varun Sethi wrote:
> Hi Will,

Hey Varun,

> > Once Thierry's generic IOMMU binding is sorted, we should look at adding
> > support for the Stream ID description. Have you looked at that at all?
> > 
> Yes, I have looked at the bindings. Would we need to represent the stream
> ids for PCI devices in the device tree? Why do we want to depend on the
> firmware to map the requestor id to the stream id? It can be handled using
> the APIs proposed by Alex Williamson. This is similar to IOMMU group
> determination, which is handled by the IOMMU driver.

Well, there could easily be a fixed mapping from the ID at the host controller
and the ID seem by the SMMU (e.g. two host controllers sharing an SMMU?). I
don't think walking the PCI buses can help you there.

The way I was thinking to handle this is that we express SID = RID +
offset. In the device-tree, we can then describe a range of RIDs on the host
controller, a single offset, and we get back a range of SIDs.

In the worst case scenario, each RID maps to a totally random SID, so then
you have a huge table describing the mapping. I *think* this is actually
unlikely, and if we ever see such a device we can either have a large
mapping or put it into C code for that specific SoC (if it's really huge).

FWIW: I believe that the ACPI folks are thinking along similar lines.

Will

Re: [PATCH 2/5] iommu/arm-smmu: add support for PCI master devices

[Will Deacon]

[Adding Alex; question below]

On Thu, Jul 03, 2014 at 03:22:37PM +0100, Varun Sethi wrote:
> > +static int __arm_smmu_get_pci_sid(struct pci_dev *pdev, u16 alias, void
> > +*data) {
> > +     *((u16 *)data) = alias;
> > +     return 0; /* Continue walking */
> > +}

[...]

> > @@ -1598,15 +1642,36 @@ static int arm_smmu_add_device(struct device
> > *dev)
> >               return PTR_ERR(group);
> >       }
> >
> > +     if (dev_is_pci(dev)) {
> > +             struct arm_smmu_master_cfg *cfg;
> > +             struct pci_dev *pdev = to_pci_dev(dev);
> > +
> > +             cfg = kzalloc(sizeof(*cfg), GFP_KERNEL);
> > +             if (!cfg) {
> > +                     ret = -ENOMEM;
> > +                     goto out_put_group;
> > +             }
> > +
> > +             cfg->num_streamids = 1;
> > +             pci_for_each_dma_alias(pdev, __arm_smmu_get_pci_sid,
> > +                                    &cfg->streamids[0]);
> [Sethi Varun-B16395] We need to look for upstream DMA device. We should be
> using pci_find_dma_isolation_root here. Also, this would also imply that
> there could be multiple devices sharing the same stream ID. So, we should
> check if a particular stream ID value has already been configured in the
> SMR registers.

pci_find_dma_isolation_root doesn't exist in any of the trees I have. Alex,
is this queued anywhere and do I actually need it?

The purpose of this code is to find the requester ID of a device as it
appears at the host controller. At this point, we can map it (via firmware
tables that are TBD) to a Stream ID for the SMMU. It looks to me like
pci_for_each_dma_alias walks over non-transparent PCI bridges correctly, so
the callback I provide just updates the alias until the walk has completed.

Will

Re: [PATCH 2/5] iommu/arm-smmu: add support for PCI master devices

[Alex Williamson]

On Wed, 2014-07-09 at 14:26 +0100, Will Deacon wrote:
> [Adding Alex; question below]
> 
> On Thu, Jul 03, 2014 at 03:22:37PM +0100, Varun Sethi wrote:
> > > +static int __arm_smmu_get_pci_sid(struct pci_dev *pdev, u16 alias, void
> > > +*data) {
> > > +     *((u16 *)data) = alias;
> > > +     return 0; /* Continue walking */
> > > +}
> 
> [...]
> 
> > > @@ -1598,15 +1642,36 @@ static int arm_smmu_add_device(struct device
> > > *dev)
> > >               return PTR_ERR(group);
> > >       }
> > >
> > > +     if (dev_is_pci(dev)) {
> > > +             struct arm_smmu_master_cfg *cfg;
> > > +             struct pci_dev *pdev = to_pci_dev(dev);
> > > +
> > > +             cfg = kzalloc(sizeof(*cfg), GFP_KERNEL);
> > > +             if (!cfg) {
> > > +                     ret = -ENOMEM;
> > > +                     goto out_put_group;
> > > +             }
> > > +
> > > +             cfg->num_streamids = 1;
> > > +             pci_for_each_dma_alias(pdev, __arm_smmu_get_pci_sid,
> > > +                                    &cfg->streamids[0]);
> > [Sethi Varun-B16395] We need to look for upstream DMA device. We should be
> > using pci_find_dma_isolation_root here. Also, this would also imply that
> > there could be multiple devices sharing the same stream ID. So, we should
> > check if a particular stream ID value has already been configured in the
> > SMR registers.
> 
> pci_find_dma_isolation_root doesn't exist in any of the trees I have. Alex,
> is this queued anywhere and do I actually need it?

That function was in the v3 DMA alias series, in v4 it got replaced.
iommu_group_get_for_pci_dev() is the common function for finding or
creating a group for a device and pci_for_each_dma_alias() is the
interface to walk each alias in a PCI topology.  The pci_ interface is
in 3.16-rc and the iommu_ pieces are current in next via the iommu tree.

> The purpose of this code is to find the requester ID of a device as it
> appears at the host controller. At this point, we can map it (via firmware
> tables that are TBD) to a Stream ID for the SMMU. It looks to me like
> pci_for_each_dma_alias walks over non-transparent PCI bridges correctly, so
> the callback I provide just updates the alias until the walk has completed.

Yep, that's the intended usage.  There are cases in VT-d where it wants
to add context entries for every alias and cases elsewhere that we just
want the final alias.  pci_for_each_dma_alias() is meant to fit both use
cases.  Thanks,

Alex

Re: [PATCH 2/5] iommu/arm-smmu: add support for PCI master devices

[Will Deacon]

On Wed, Jul 09, 2014 at 03:13:04PM +0100, Alex Williamson wrote:
> On Wed, 2014-07-09 at 14:26 +0100, Will Deacon wrote:
> > [Adding Alex; question below]
> > 
> > On Thu, Jul 03, 2014 at 03:22:37PM +0100, Varun Sethi wrote:
> > > > +static int __arm_smmu_get_pci_sid(struct pci_dev *pdev, u16 alias, void
> > > > +*data) {
> > > > +     *((u16 *)data) = alias;
> > > > +     return 0; /* Continue walking */
> > > > +}
> > 
> > [...]
> > 
> > > > @@ -1598,15 +1642,36 @@ static int arm_smmu_add_device(struct device
> > > > *dev)
> > > >               return PTR_ERR(group);
> > > >       }
> > > >
> > > > +     if (dev_is_pci(dev)) {
> > > > +             struct arm_smmu_master_cfg *cfg;
> > > > +             struct pci_dev *pdev = to_pci_dev(dev);
> > > > +
> > > > +             cfg = kzalloc(sizeof(*cfg), GFP_KERNEL);
> > > > +             if (!cfg) {
> > > > +                     ret = -ENOMEM;
> > > > +                     goto out_put_group;
> > > > +             }
> > > > +
> > > > +             cfg->num_streamids = 1;
> > > > +             pci_for_each_dma_alias(pdev, __arm_smmu_get_pci_sid,
> > > > +                                    &cfg->streamids[0]);
> > > [Sethi Varun-B16395] We need to look for upstream DMA device. We should be
> > > using pci_find_dma_isolation_root here. Also, this would also imply that
> > > there could be multiple devices sharing the same stream ID. So, we should
> > > check if a particular stream ID value has already been configured in the
> > > SMR registers.
> > 
> > pci_find_dma_isolation_root doesn't exist in any of the trees I have. Alex,
> > is this queued anywhere and do I actually need it?
> 
> That function was in the v3 DMA alias series, in v4 it got replaced.
> iommu_group_get_for_pci_dev() is the common function for finding or
> creating a group for a device and pci_for_each_dma_alias() is the
> interface to walk each alias in a PCI topology.  The pci_ interface is
> in 3.16-rc and the iommu_ pieces are current in next via the iommu tree.

Cheers, Alex. I'll move my driver to using iommu_group_get_for_pci_dev once
that's hit mainline (since I currently put each master into its own group).

> > The purpose of this code is to find the requester ID of a device as it
> > appears at the host controller. At this point, we can map it (via firmware
> > tables that are TBD) to a Stream ID for the SMMU. It looks to me like
> > pci_for_each_dma_alias walks over non-transparent PCI bridges correctly, so
> > the callback I provide just updates the alias until the walk has completed.
> 
> Yep, that's the intended usage.  There are cases in VT-d where it wants
> to add context entries for every alias and cases elsewhere that we just
> want the final alias.  pci_for_each_dma_alias() is meant to fit both use
> cases.  Thanks,

Cracking, cheers for confirming.

Will

Re: [PATCH 2/5] iommu/arm-smmu: add support for PCI master devices

[Will Deacon]

Hi Alex,

On Wed, Jul 09, 2014 at 03:13:04PM +0100, Alex Williamson wrote:
> On Wed, 2014-07-09 at 14:26 +0100, Will Deacon wrote:
> > pci_find_dma_isolation_root doesn't exist in any of the trees I have. Alex,
> > is this queued anywhere and do I actually need it?
> 
> That function was in the v3 DMA alias series, in v4 it got replaced.
> iommu_group_get_for_pci_dev() is the common function for finding or
> creating a group for a device and pci_for_each_dma_alias() is the
> interface to walk each alias in a PCI topology.  The pci_ interface is
> in 3.16-rc and the iommu_ pieces are current in next via the iommu tree.
> 
> > The purpose of this code is to find the requester ID of a device as it
> > appears at the host controller. At this point, we can map it (via firmware
> > tables that are TBD) to a Stream ID for the SMMU. It looks to me like
> > pci_for_each_dma_alias walks over non-transparent PCI bridges correctly, so
> > the callback I provide just updates the alias until the walk has completed.
> 
> Yep, that's the intended usage.  There are cases in VT-d where it wants
> to add context entries for every alias and cases elsewhere that we just
> want the final alias.  pci_for_each_dma_alias() is meant to fit both use
> cases.  Thanks,

I'm looking at moving the arm-smmu driver over to using
iommu_group_get_for_dev, but I've hit a slight snag. Once I have the group,
I actually need to get hold of the alias for that group, since that will be
converted into a StreamID used to program the SMMU. I can do this by
open-coding my own version of iommu_group_get_for_pci_dev, but that's pretty
horrible.

What's the best way to get hold of the alias for a given group?

Cheers,

Will

RE: [PATCH 2/5] iommu/arm-smmu: add support for PCI master devices

[Varun Sethi]

Hi Will,

> -----Original Message-----
> From: Will Deacon [mailto:will.deacon-5wv7dgnIgG8@public.gmane.org]
> Sent: Wednesday, July 09, 2014 6:57 PM
> To: Sethi Varun-B16395; alex.williamson-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org
> Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org; iommu-cunTk1MwBs/ROKNJybVBZg@public.gmane.org
> foundation.org; thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org; arnd-r2nGTMty4D4@public.gmane.org;
> ohaugan-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org; joro-zLv9SwRftAIdnm+yROfE0A@public.gmane.org;
> a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org; Marc Zyngier
> Subject: Re: [PATCH 2/5] iommu/arm-smmu: add support for PCI master
> devices
> 
> [Adding Alex; question below]
> 
> On Thu, Jul 03, 2014 at 03:22:37PM +0100, Varun Sethi wrote:
> > > +static int __arm_smmu_get_pci_sid(struct pci_dev *pdev, u16 alias,
> > > +void
> > > +*data) {
> > > +     *((u16 *)data) = alias;
> > > +     return 0; /* Continue walking */ }
> 
> [...]
> 
> > > @@ -1598,15 +1642,36 @@ static int arm_smmu_add_device(struct device
> > > *dev)
> > >               return PTR_ERR(group);
> > >       }
> > >
> > > +     if (dev_is_pci(dev)) {
> > > +             struct arm_smmu_master_cfg *cfg;
> > > +             struct pci_dev *pdev = to_pci_dev(dev);
> > > +
> > > +             cfg = kzalloc(sizeof(*cfg), GFP_KERNEL);
> > > +             if (!cfg) {
> > > +                     ret = -ENOMEM;
> > > +                     goto out_put_group;
> > > +             }
> > > +
> > > +             cfg->num_streamids = 1;
> > > +             pci_for_each_dma_alias(pdev, __arm_smmu_get_pci_sid,
> > > +                                    &cfg->streamids[0]);
> > [Sethi Varun-B16395] We need to look for upstream DMA device. We
> > should be using pci_find_dma_isolation_root here. Also, this would
> > also imply that there could be multiple devices sharing the same
> > stream ID. So, we should check if a particular stream ID value has
> > already been configured in the SMR registers.
> 
> pci_find_dma_isolation_root doesn't exist in any of the trees I have.
> Alex, is this queued anywhere and do I actually need it?
> 
> The purpose of this code is to find the requester ID of a device as it
> appears at the host controller. At this point, we can map it (via
> firmware tables that are TBD) to a Stream ID for the SMMU. It looks to me
> like pci_for_each_dma_alias walks over non-transparent PCI bridges
> correctly, so the callback I provide just updates the alias until the
> walk has completed.
I think pci_for_each_dma_alias should work fine here. Isolation would be specifically required while setting up the iommu groups.

-Varun

[PATCH 3/5] iommu/arm-smmu: caps: add IOMMU_CAP_INTR_REMAP capability

[Will Deacon]

MSIs are just seen as bog standard memory writes by the ARM SMMU, so
they can be translated (and isolated) in the same way.

This patch adds the IOMMU_CAP_INTR_REMAP capability to the ARM SMMU
driver and reworks our capabaility code so that we don't assume the
caps are organised as bits in a bitmask (since this isn't the intention).

Signed-off-by: Will Deacon <will.deacon-5wv7dgnIgG8@public.gmane.org>
---
 drivers/iommu/arm-smmu.c | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index e4eebc50612c..835d01f47302 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -1606,13 +1606,17 @@ static phys_addr_t arm_smmu_iova_to_phys(struct iommu_domain *domain,
 static int arm_smmu_domain_has_cap(struct iommu_domain *domain,
 				   unsigned long cap)
 {
-	unsigned long caps = 0;
 	struct arm_smmu_domain *smmu_domain = domain->priv;
+	u32 features = smmu_domain->root_cfg.smmu->features;
 
-	if (smmu_domain->root_cfg.smmu->features & ARM_SMMU_FEAT_COHERENT_WALK)
-		caps |= IOMMU_CAP_CACHE_COHERENCY;
-
-	return !!(cap & caps);
+	switch (cap) {
+	case IOMMU_CAP_CACHE_COHERENCY:
+		return features & ARM_SMMU_FEAT_COHERENT_WALK;
+	case IOMMU_CAP_INTR_REMAP:
+		return 1; /* MSIs are just memory writes */
+	default:
+		return 0;
+	}
 }
 
 static int __arm_smmu_get_pci_sid(struct pci_dev *pdev, u16 alias, void *data)
-- 
2.0.0

[PATCH 4/5] iommu/arm-smmu: remove support for chained SMMUs

[Will Deacon]

The ARM SMMU driver has supported chained SMMUs (i.e. SMMUs connected
back-to-back in series) via the smmu-parent property in device tree.
This was in anticipation of somebody building such a configuration,
however that seems not to be the case.

This patch removes the unused chained SMMU hack from the driver. We can
consider adding it back later if somebody decided they need it, but for
the time being it's just pointless mess that we're carrying in mainline.

Removal of the feature also makes migration to the generic IOMMU bindings
easier.

Signed-off-by: Will Deacon <will.deacon-5wv7dgnIgG8@public.gmane.org>
---
 .../devicetree/bindings/iommu/arm,smmu.txt         |   6 -
 drivers/iommu/arm-smmu.c                           | 263 ++++++---------------
 2 files changed, 77 insertions(+), 192 deletions(-)

diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.txt b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
index f284b99402bc..2d0f7cd867ea 100644
--- a/Documentation/devicetree/bindings/iommu/arm,smmu.txt
+++ b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
@@ -42,12 +42,6 @@ conditions.
 
 ** System MMU optional properties:
 
-- smmu-parent   : When multiple SMMUs are chained together, this
-                  property can be used to provide a phandle to the
-                  parent SMMU (that is the next SMMU on the path going
-                  from the mmu-masters towards memory) node for this
-                  SMMU.
-
 - calxeda,smmu-secure-config-access : Enable proper handling of buggy
                   implementations that always use secure access to
                   SMMU configuration registers. In this case non-secure
diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index 835d01f47302..60986de3ada8 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -333,28 +333,17 @@ struct arm_smmu_smr {
 struct arm_smmu_master_cfg {
 	int				num_streamids;
 	u16				streamids[MAX_MASTER_STREAMIDS];
-
-	/*
-	 * We only need to allocate these on the root SMMU, as we
-	 * configure unmatched streams to bypass translation.
-	 */
 	struct arm_smmu_smr		*smrs;
 };
 
 struct arm_smmu_master {
 	struct device_node		*of_node;
-
-	/*
-	 * The following is specific to the master's position in the
-	 * SMMU chain.
-	 */
 	struct rb_node			node;
 	struct arm_smmu_master_cfg	cfg;
 };
 
 struct arm_smmu_device {
 	struct device			*dev;
-	struct device_node		*parent_of_node;
 
 	void __iomem			*base;
 	unsigned long			size;
@@ -392,7 +381,6 @@ struct arm_smmu_device {
 };
 
 struct arm_smmu_cfg {
-	struct arm_smmu_device		*smmu;
 	u8				cbndx;
 	u8				irptndx;
 	u32				cbar;
@@ -404,15 +392,8 @@ struct arm_smmu_cfg {
 #define ARM_SMMU_CB_VMID(cfg)		((cfg)->cbndx + 1)
 
 struct arm_smmu_domain {
-	/*
-	 * A domain can span across multiple, chained SMMUs and requires
-	 * all devices within the domain to follow the same translation
-	 * path.
-	 */
-	struct arm_smmu_device		*leaf_smmu;
-	struct arm_smmu_cfg		root_cfg;
-	phys_addr_t			output_mask;
-
+	struct arm_smmu_device		*smmu;
+	struct arm_smmu_cfg		cfg;
 	spinlock_t			lock;
 };
 
@@ -546,59 +527,20 @@ static int register_smmu_master(struct arm_smmu_device *smmu,
 	return insert_smmu_master(smmu, master);
 }
 
-static struct arm_smmu_device *find_parent_smmu(struct arm_smmu_device *smmu)
+static struct arm_smmu_device *find_smmu_for_device(struct device *dev)
 {
-	struct arm_smmu_device *parent;
-
-	if (!smmu->parent_of_node)
-		return NULL;
-
-	spin_lock(&arm_smmu_devices_lock);
-	list_for_each_entry(parent, &arm_smmu_devices, list)
-		if (parent->dev->of_node == smmu->parent_of_node)
-			goto out_unlock;
-
-	parent = NULL;
-	dev_warn(smmu->dev,
-		 "Failed to find SMMU parent despite parent in DT\n");
-out_unlock:
-	spin_unlock(&arm_smmu_devices_lock);
-	return parent;
-}
-
-static struct arm_smmu_device *find_parent_smmu_for_device(struct device *dev)
-{
-	struct arm_smmu_device *child, *parent, *smmu;
+	struct arm_smmu_device *smmu;
 	struct arm_smmu_master *master = NULL;
 	struct device_node *dev_node = dev_get_master_dev(dev)->of_node;
 
 	spin_lock(&arm_smmu_devices_lock);
-	list_for_each_entry(parent, &arm_smmu_devices, list) {
-		smmu = parent;
-
-		/* Try to find a child of the current SMMU. */
-		list_for_each_entry(child, &arm_smmu_devices, list) {
-			if (child->parent_of_node == parent->dev->of_node) {
-				/* Does the child sit above our master? */
-				master = find_smmu_master(child, dev_node);
-				if (master) {
-					smmu = NULL;
-					break;
-				}
-			}
-		}
-
-		/* We found some children, so keep searching. */
-		if (!smmu) {
-			master = NULL;
-			continue;
-		}
-
+	list_for_each_entry(smmu, &arm_smmu_devices, list) {
 		master = find_smmu_master(smmu, dev_node);
 		if (master)
 			break;
 	}
 	spin_unlock(&arm_smmu_devices_lock);
+
 	return master ? smmu : NULL;
 }
 
@@ -639,9 +581,10 @@ static void arm_smmu_tlb_sync(struct arm_smmu_device *smmu)
 	}
 }
 
-static void arm_smmu_tlb_inv_context(struct arm_smmu_cfg *cfg)
+static void arm_smmu_tlb_inv_context(struct arm_smmu_domain *smmu_domain)
 {
-	struct arm_smmu_device *smmu = cfg->smmu;
+	struct arm_smmu_cfg *cfg = &smmu_domain->cfg;
+	struct arm_smmu_device *smmu = smmu_domain->smmu;
 	void __iomem *base = ARM_SMMU_GR0(smmu);
 	bool stage1 = cfg->cbar != CBAR_TYPE_S2_TRANS;
 
@@ -665,11 +608,11 @@ static irqreturn_t arm_smmu_context_fault(int irq, void *dev)
 	unsigned long iova;
 	struct iommu_domain *domain = dev;
 	struct arm_smmu_domain *smmu_domain = domain->priv;
-	struct arm_smmu_cfg *root_cfg = &smmu_domain->root_cfg;
-	struct arm_smmu_device *smmu = root_cfg->smmu;
+	struct arm_smmu_cfg *cfg = &smmu_domain->cfg;
+	struct arm_smmu_device *smmu = smmu_domain->smmu;
 	void __iomem *cb_base;
 
-	cb_base = ARM_SMMU_CB_BASE(smmu) + ARM_SMMU_CB(smmu, root_cfg->cbndx);
+	cb_base = ARM_SMMU_CB_BASE(smmu) + ARM_SMMU_CB(smmu, cfg->cbndx);
 	fsr = readl_relaxed(cb_base + ARM_SMMU_CB_FSR);
 
 	if (!(fsr & FSR_FAULT))
@@ -696,7 +639,7 @@ static irqreturn_t arm_smmu_context_fault(int irq, void *dev)
 	} else {
 		dev_err_ratelimited(smmu->dev,
 		    "Unhandled context fault: iova=0x%08lx, fsynr=0x%x, cb=%d\n",
-		    iova, fsynr, root_cfg->cbndx);
+		    iova, fsynr, cfg->cbndx);
 		ret = IRQ_NONE;
 		resume = RESUME_TERMINATE;
 	}
@@ -761,19 +704,19 @@ static void arm_smmu_init_context_bank(struct arm_smmu_domain *smmu_domain)
 {
 	u32 reg;
 	bool stage1;
-	struct arm_smmu_cfg *root_cfg = &smmu_domain->root_cfg;
-	struct arm_smmu_device *smmu = root_cfg->smmu;
+	struct arm_smmu_cfg *cfg = &smmu_domain->cfg;
+	struct arm_smmu_device *smmu = smmu_domain->smmu;
 	void __iomem *cb_base, *gr0_base, *gr1_base;
 
 	gr0_base = ARM_SMMU_GR0(smmu);
 	gr1_base = ARM_SMMU_GR1(smmu);
-	stage1 = root_cfg->cbar != CBAR_TYPE_S2_TRANS;
-	cb_base = ARM_SMMU_CB_BASE(smmu) + ARM_SMMU_CB(smmu, root_cfg->cbndx);
+	stage1 = cfg->cbar != CBAR_TYPE_S2_TRANS;
+	cb_base = ARM_SMMU_CB_BASE(smmu) + ARM_SMMU_CB(smmu, cfg->cbndx);
 
 	/* CBAR */
-	reg = root_cfg->cbar;
+	reg = cfg->cbar;
 	if (smmu->version == 1)
-	      reg |= root_cfg->irptndx << CBAR_IRPTNDX_SHIFT;
+	      reg |= cfg->irptndx << CBAR_IRPTNDX_SHIFT;
 
 	/*
 	 * Use the weakest shareability/memory types, so they are
@@ -783,9 +726,9 @@ static void arm_smmu_init_context_bank(struct arm_smmu_domain *smmu_domain)
 		reg |= (CBAR_S1_BPSHCFG_NSH << CBAR_S1_BPSHCFG_SHIFT) |
 			(CBAR_S1_MEMATTR_WB << CBAR_S1_MEMATTR_SHIFT);
 	} else {
-		reg |= ARM_SMMU_CB_VMID(root_cfg) << CBAR_VMID_SHIFT;
+		reg |= ARM_SMMU_CB_VMID(cfg) << CBAR_VMID_SHIFT;
 	}
-	writel_relaxed(reg, gr1_base + ARM_SMMU_GR1_CBAR(root_cfg->cbndx));
+	writel_relaxed(reg, gr1_base + ARM_SMMU_GR1_CBAR(cfg->cbndx));
 
 	if (smmu->version > 1) {
 		/* CBA2R */
@@ -795,7 +738,7 @@ static void arm_smmu_init_context_bank(struct arm_smmu_domain *smmu_domain)
 		reg = CBA2R_RW64_32BIT;
 #endif
 		writel_relaxed(reg,
-			       gr1_base + ARM_SMMU_GR1_CBA2R(root_cfg->cbndx));
+			       gr1_base + ARM_SMMU_GR1_CBA2R(cfg->cbndx));
 
 		/* TTBCR2 */
 		switch (smmu->input_size) {
@@ -845,13 +788,13 @@ static void arm_smmu_init_context_bank(struct arm_smmu_domain *smmu_domain)
 	}
 
 	/* TTBR0 */
-	arm_smmu_flush_pgtable(smmu, root_cfg->pgd,
+	arm_smmu_flush_pgtable(smmu, cfg->pgd,
 			       PTRS_PER_PGD * sizeof(pgd_t));
-	reg = __pa(root_cfg->pgd);
+	reg = __pa(cfg->pgd);
 	writel_relaxed(reg, cb_base + ARM_SMMU_CB_TTBR0_LO);
-	reg = (phys_addr_t)__pa(root_cfg->pgd) >> 32;
+	reg = (phys_addr_t)__pa(cfg->pgd) >> 32;
 	if (stage1)
-		reg |= ARM_SMMU_CB_ASID(root_cfg) << TTBRn_HI_ASID_SHIFT;
+		reg |= ARM_SMMU_CB_ASID(cfg) << TTBRn_HI_ASID_SHIFT;
 	writel_relaxed(reg, cb_base + ARM_SMMU_CB_TTBR0_HI);
 
 	/*
@@ -920,44 +863,24 @@ static void arm_smmu_init_context_bank(struct arm_smmu_domain *smmu_domain)
 }
 
 static int arm_smmu_init_domain_context(struct iommu_domain *domain,
-					struct device *dev,
-					struct arm_smmu_device *device_smmu)
+					struct arm_smmu_device *smmu)
 {
 	int irq, ret, start;
 	struct arm_smmu_domain *smmu_domain = domain->priv;
-	struct arm_smmu_cfg *root_cfg = &smmu_domain->root_cfg;
-	struct arm_smmu_device *smmu, *parent;
-
-	/*
-	 * Walk the SMMU chain to find the root device for this chain.
-	 * We assume that no masters have translations which terminate
-	 * early, and therefore check that the root SMMU does indeed have
-	 * a StreamID for the master in question.
-	 */
-	parent = device_smmu;
-	smmu_domain->output_mask = -1;
-	do {
-		smmu = parent;
-		smmu_domain->output_mask &= (1ULL << smmu->s2_output_size) - 1;
-	} while ((parent = find_parent_smmu(smmu)));
-
-	if (!find_smmu_master_cfg(smmu, dev)) {
-		dev_err(dev, "unable to find root SMMU config for device\n");
-		return -ENODEV;
-	}
+	struct arm_smmu_cfg *cfg = &smmu_domain->cfg;
 
 	if (smmu->features & ARM_SMMU_FEAT_TRANS_NESTED) {
 		/*
 		 * We will likely want to change this if/when KVM gets
 		 * involved.
 		 */
-		root_cfg->cbar = CBAR_TYPE_S1_TRANS_S2_BYPASS;
+		cfg->cbar = CBAR_TYPE_S1_TRANS_S2_BYPASS;
 		start = smmu->num_s2_context_banks;
 	} else if (smmu->features & ARM_SMMU_FEAT_TRANS_S2) {
-		root_cfg->cbar = CBAR_TYPE_S2_TRANS;
+		cfg->cbar = CBAR_TYPE_S2_TRANS;
 		start = 0;
 	} else {
-		root_cfg->cbar = CBAR_TYPE_S1_TRANS_S2_BYPASS;
+		cfg->cbar = CBAR_TYPE_S1_TRANS_S2_BYPASS;
 		start = smmu->num_s2_context_banks;
 	}
 
@@ -966,39 +889,38 @@ static int arm_smmu_init_domain_context(struct iommu_domain *domain,
 	if (IS_ERR_VALUE(ret))
 		return ret;
 
-	root_cfg->cbndx = ret;
+	cfg->cbndx = ret;
 	if (smmu->version == 1) {
-		root_cfg->irptndx = atomic_inc_return(&smmu->irptndx);
-		root_cfg->irptndx %= smmu->num_context_irqs;
+		cfg->irptndx = atomic_inc_return(&smmu->irptndx);
+		cfg->irptndx %= smmu->num_context_irqs;
 	} else {
-		root_cfg->irptndx = root_cfg->cbndx;
+		cfg->irptndx = cfg->cbndx;
 	}
 
-	irq = smmu->irqs[smmu->num_global_irqs + root_cfg->irptndx];
+	irq = smmu->irqs[smmu->num_global_irqs + cfg->irptndx];
 	ret = request_irq(irq, arm_smmu_context_fault, IRQF_SHARED,
 			  "arm-smmu-context-fault", domain);
 	if (IS_ERR_VALUE(ret)) {
 		dev_err(smmu->dev, "failed to request context IRQ %d (%u)\n",
-			root_cfg->irptndx, irq);
-		root_cfg->irptndx = INVALID_IRPTNDX;
+			cfg->irptndx, irq);
+		cfg->irptndx = INVALID_IRPTNDX;
 		goto out_free_context;
 	}
 
-	root_cfg->smmu = smmu;
+	smmu_domain->smmu = smmu;
 	arm_smmu_init_context_bank(smmu_domain);
-	smmu_domain->leaf_smmu = device_smmu;
 	return 0;
 
 out_free_context:
-	__arm_smmu_free_bitmap(smmu->context_map, root_cfg->cbndx);
+	__arm_smmu_free_bitmap(smmu->context_map, cfg->cbndx);
 	return ret;
 }
 
 static void arm_smmu_destroy_domain_context(struct iommu_domain *domain)
 {
 	struct arm_smmu_domain *smmu_domain = domain->priv;
-	struct arm_smmu_cfg *root_cfg = &smmu_domain->root_cfg;
-	struct arm_smmu_device *smmu = root_cfg->smmu;
+	struct arm_smmu_device *smmu = smmu_domain->smmu;
+	struct arm_smmu_cfg *cfg = &smmu_domain->cfg;
 	void __iomem *cb_base;
 	int irq;
 
@@ -1006,16 +928,16 @@ static void arm_smmu_destroy_domain_context(struct iommu_domain *domain)
 		return;
 
 	/* Disable the context bank and nuke the TLB before freeing it. */
-	cb_base = ARM_SMMU_CB_BASE(smmu) + ARM_SMMU_CB(smmu, root_cfg->cbndx);
+	cb_base = ARM_SMMU_CB_BASE(smmu) + ARM_SMMU_CB(smmu, cfg->cbndx);
 	writel_relaxed(0, cb_base + ARM_SMMU_CB_SCTLR);
-	arm_smmu_tlb_inv_context(root_cfg);
+	arm_smmu_tlb_inv_context(smmu_domain);
 
-	if (root_cfg->irptndx != INVALID_IRPTNDX) {
-		irq = smmu->irqs[smmu->num_global_irqs + root_cfg->irptndx];
+	if (cfg->irptndx != INVALID_IRPTNDX) {
+		irq = smmu->irqs[smmu->num_global_irqs + cfg->irptndx];
 		free_irq(irq, domain);
 	}
 
-	__arm_smmu_free_bitmap(smmu->context_map, root_cfg->cbndx);
+	__arm_smmu_free_bitmap(smmu->context_map, cfg->cbndx);
 }
 
 static int arm_smmu_domain_init(struct iommu_domain *domain)
@@ -1035,7 +957,7 @@ static int arm_smmu_domain_init(struct iommu_domain *domain)
 	pgd = kzalloc(PTRS_PER_PGD * sizeof(pgd_t), GFP_KERNEL);
 	if (!pgd)
 		goto out_free_domain;
-	smmu_domain->root_cfg.pgd = pgd;
+	smmu_domain->cfg.pgd = pgd;
 
 	spin_lock_init(&smmu_domain->lock);
 	domain->priv = smmu_domain;
@@ -1090,8 +1012,8 @@ static void arm_smmu_free_puds(pgd_t *pgd)
 static void arm_smmu_free_pgtables(struct arm_smmu_domain *smmu_domain)
 {
 	int i;
-	struct arm_smmu_cfg *root_cfg = &smmu_domain->root_cfg;
-	pgd_t *pgd, *pgd_base = root_cfg->pgd;
+	struct arm_smmu_cfg *cfg = &smmu_domain->cfg;
+	pgd_t *pgd, *pgd_base = cfg->pgd;
 
 	/*
 	 * Recursively free the page tables for this domain. We don't
@@ -1142,7 +1064,7 @@ static int arm_smmu_master_configure_smrs(struct arm_smmu_device *smmu,
 		return -ENOMEM;
 	}
 
-	/* Allocate the SMRs on the root SMMU */
+	/* Allocate the SMRs on the SMMU */
 	for (i = 0; i < cfg->num_streamids; ++i) {
 		int idx = __arm_smmu_alloc_bitmap(smmu->smr_map, 0,
 						  smmu->num_mapping_groups);
@@ -1210,34 +1132,18 @@ static int arm_smmu_domain_add_master(struct arm_smmu_domain *smmu_domain,
 				      struct arm_smmu_master_cfg *cfg)
 {
 	int i, ret;
-	struct arm_smmu_device *parent, *smmu = smmu_domain->root_cfg.smmu;
+	struct arm_smmu_device *smmu = smmu_domain->smmu;
 	void __iomem *gr0_base = ARM_SMMU_GR0(smmu);
 
 	ret = arm_smmu_master_configure_smrs(smmu, cfg);
 	if (ret)
 		return ret;
 
-	/* Bypass the leaves */
-	smmu = smmu_domain->leaf_smmu;
-	while ((parent = find_parent_smmu(smmu))) {
-		/*
-		 * We won't have a StreamID match for anything but the root
-		 * smmu, so we only need to worry about StreamID indexing,
-		 * where we must install bypass entries in the S2CRs.
-		 */
-		if (smmu->features & ARM_SMMU_FEAT_STREAM_MATCH)
-			continue;
-
-		arm_smmu_bypass_stream_mapping(smmu, cfg);
-		smmu = parent;
-	}
-
-	/* Now we're at the root, time to point at our context bank */
 	for (i = 0; i < cfg->num_streamids; ++i) {
 		u32 idx, s2cr;
 		idx = cfg->smrs ? cfg->smrs[i].idx : cfg->streamids[i];
 		s2cr = S2CR_TYPE_TRANS |
-		       (smmu_domain->root_cfg.cbndx << S2CR_CBNDX_SHIFT);
+		       (smmu_domain->cfg.cbndx << S2CR_CBNDX_SHIFT);
 		writel_relaxed(s2cr, gr0_base + ARM_SMMU_GR0_S2CR(idx));
 	}
 
@@ -1247,7 +1153,7 @@ static int arm_smmu_domain_add_master(struct arm_smmu_domain *smmu_domain,
 static void arm_smmu_domain_remove_master(struct arm_smmu_domain *smmu_domain,
 					  struct arm_smmu_master_cfg *cfg)
 {
-	struct arm_smmu_device *smmu = smmu_domain->root_cfg.smmu;
+	struct arm_smmu_device *smmu = smmu_domain->smmu;
 
 	/*
 	 * We *must* clear the S2CR first, because freeing the SMR means
@@ -1261,37 +1167,37 @@ static int arm_smmu_attach_dev(struct iommu_domain *domain, struct device *dev)
 {
 	int ret = -EINVAL;
 	struct arm_smmu_domain *smmu_domain = domain->priv;
-	struct arm_smmu_device *device_smmu;
+	struct arm_smmu_device *smmu;
 	struct arm_smmu_master_cfg *cfg;
 	unsigned long flags;
 
-	device_smmu = dev_get_master_dev(dev)->archdata.iommu;
-	if (!device_smmu) {
+	smmu = dev_get_master_dev(dev)->archdata.iommu;
+	if (!smmu) {
 		dev_err(dev, "cannot attach to SMMU, is it on the same bus?\n");
 		return -ENXIO;
 	}
 
 	/*
-	 * Sanity check the domain. We don't currently support domains
-	 * that cross between different SMMU chains.
+	 * Sanity check the domain. We don't support domains across
+	 * different SMMUs.
 	 */
 	spin_lock_irqsave(&smmu_domain->lock, flags);
-	if (!smmu_domain->leaf_smmu) {
+	if (!smmu_domain->smmu) {
 		/* Now that we have a master, we can finalise the domain */
-		ret = arm_smmu_init_domain_context(domain, dev, device_smmu);
+		ret = arm_smmu_init_domain_context(domain, smmu);
 		if (IS_ERR_VALUE(ret))
 			goto err_unlock;
-	} else if (smmu_domain->leaf_smmu != device_smmu) {
+	} else if (smmu_domain->smmu != smmu) {
 		dev_err(dev,
 			"cannot attach to SMMU %s whilst already attached to domain on SMMU %s\n",
-			dev_name(smmu_domain->leaf_smmu->dev),
-			dev_name(device_smmu->dev));
+			dev_name(smmu_domain->smmu->dev),
+			dev_name(smmu->dev));
 		goto err_unlock;
 	}
 	spin_unlock_irqrestore(&smmu_domain->lock, flags);
 
 	/* Looks ok, so add the device to the domain */
-	cfg = find_smmu_master_cfg(smmu_domain->leaf_smmu, dev);
+	cfg = find_smmu_master_cfg(smmu_domain->smmu, dev);
 	if (!cfg)
 		return -ENODEV;
 
@@ -1307,7 +1213,7 @@ static void arm_smmu_detach_dev(struct iommu_domain *domain, struct device *dev)
 	struct arm_smmu_domain *smmu_domain = domain->priv;
 	struct arm_smmu_master_cfg *cfg;
 
-	cfg = find_smmu_master_cfg(smmu_domain->leaf_smmu, dev);
+	cfg = find_smmu_master_cfg(smmu_domain->smmu, dev);
 	if (cfg)
 		arm_smmu_domain_remove_master(smmu_domain, cfg);
 }
@@ -1497,12 +1403,12 @@ static int arm_smmu_handle_mapping(struct arm_smmu_domain *smmu_domain,
 	int ret, stage;
 	unsigned long end;
 	phys_addr_t input_mask, output_mask;
-	struct arm_smmu_cfg *root_cfg = &smmu_domain->root_cfg;
-	pgd_t *pgd = root_cfg->pgd;
-	struct arm_smmu_device *smmu = root_cfg->smmu;
+	struct arm_smmu_device *smmu = smmu_domain->smmu;
+	struct arm_smmu_cfg *cfg = &smmu_domain->cfg;
+	pgd_t *pgd = cfg->pgd;
 	unsigned long flags;
 
-	if (root_cfg->cbar == CBAR_TYPE_S2_TRANS) {
+	if (cfg->cbar == CBAR_TYPE_S2_TRANS) {
 		stage = 2;
 		output_mask = (1ULL << smmu->s2_output_size) - 1;
 	} else {
@@ -1552,10 +1458,6 @@ static int arm_smmu_map(struct iommu_domain *domain, unsigned long iova,
 	if (!smmu_domain)
 		return -ENODEV;
 
-	/* Check for silent address truncation up the SMMU chain. */
-	if ((phys_addr_t)iova & ~smmu_domain->output_mask)
-		return -ERANGE;
-
 	return arm_smmu_handle_mapping(smmu_domain, iova, paddr, size, prot);
 }
 
@@ -1566,7 +1468,7 @@ static size_t arm_smmu_unmap(struct iommu_domain *domain, unsigned long iova,
 	struct arm_smmu_domain *smmu_domain = domain->priv;
 
 	ret = arm_smmu_handle_mapping(smmu_domain, iova, 0, size, 0);
-	arm_smmu_tlb_inv_context(&smmu_domain->root_cfg);
+	arm_smmu_tlb_inv_context(smmu_domain);
 	return ret ? 0 : size;
 }
 
@@ -1578,9 +1480,9 @@ static phys_addr_t arm_smmu_iova_to_phys(struct iommu_domain *domain,
 	pmd_t pmd;
 	pte_t pte;
 	struct arm_smmu_domain *smmu_domain = domain->priv;
-	struct arm_smmu_cfg *root_cfg = &smmu_domain->root_cfg;
+	struct arm_smmu_cfg *cfg = &smmu_domain->cfg;
 
-	pgdp = root_cfg->pgd;
+	pgdp = cfg->pgd;
 	if (!pgdp)
 		return 0;
 
@@ -1607,7 +1509,7 @@ static int arm_smmu_domain_has_cap(struct iommu_domain *domain,
 				   unsigned long cap)
 {
 	struct arm_smmu_domain *smmu_domain = domain->priv;
-	u32 features = smmu_domain->root_cfg.smmu->features;
+	u32 features = smmu_domain->smmu->features;
 
 	switch (cap) {
 	case IOMMU_CAP_CACHE_COHERENCY:
@@ -1636,7 +1538,7 @@ static int arm_smmu_add_device(struct device *dev)
 		return -EINVAL;
 	}
 
-	smmu = find_parent_smmu_for_device(dev);
+	smmu = find_smmu_for_device(dev);
 	if (!smmu)
 		return -ENODEV;
 
@@ -1914,7 +1816,6 @@ static int arm_smmu_device_dt_probe(struct platform_device *pdev)
 {
 	struct resource *res;
 	struct arm_smmu_device *smmu;
-	struct device_node *dev_node;
 	struct device *dev = &pdev->dev;
 	struct rb_node *node;
 	struct of_phandle_args masterspec;
@@ -1984,12 +1885,9 @@ static int arm_smmu_device_dt_probe(struct platform_device *pdev)
 	}
 	dev_notice(dev, "registered %d master devices\n", i);
 
-	if ((dev_node = of_parse_phandle(dev->of_node, "smmu-parent", 0)))
-		smmu->parent_of_node = dev_node;
-
 	err = arm_smmu_device_cfg_probe(smmu);
 	if (err)
-		goto out_put_parent;
+		goto out_put_masters;
 
 	parse_driver_options(smmu);
 
@@ -1999,7 +1897,7 @@ static int arm_smmu_device_dt_probe(struct platform_device *pdev)
 			"found only %d context interrupt(s) but %d required\n",
 			smmu->num_context_irqs, smmu->num_context_banks);
 		err = -ENODEV;
-		goto out_put_parent;
+		goto out_put_masters;
 	}
 
 	for (i = 0; i < smmu->num_global_irqs; ++i) {
@@ -2027,10 +1925,6 @@ out_free_irqs:
 	while (i--)
 		free_irq(smmu->irqs[i], smmu);
 
-out_put_parent:
-	if (smmu->parent_of_node)
-		of_node_put(smmu->parent_of_node);
-
 out_put_masters:
 	for (node = rb_first(&smmu->masters); node; node = rb_next(node)) {
 		struct arm_smmu_master *master;
@@ -2061,9 +1955,6 @@ static int arm_smmu_device_remove(struct platform_device *pdev)
 	if (!smmu)
 		return -ENODEV;
 
-	if (smmu->parent_of_node)
-		of_node_put(smmu->parent_of_node);
-
 	for (node = rb_first(&smmu->masters); node; node = rb_next(node)) {
 		struct arm_smmu_master *master;
 		master = container_of(node, struct arm_smmu_master, node);
-- 
2.0.0

[PATCH 5/5] iommu/arm-smmu: prefer stage-1 mappings where we have a choice

[Will Deacon]

For an SMMU that supports both Stage-1 and Stage-2 mappings (but not
nested translation), then we should prefer stage-1 mappings as we
otherwise rely on the memory attributes of the incoming transactions
for IOMMU_CACHE mappings.

Signed-off-by: Will Deacon <will.deacon-5wv7dgnIgG8@public.gmane.org>
---
 drivers/iommu/arm-smmu.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index 60986de3ada8..a6e38982d09c 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -876,12 +876,12 @@ static int arm_smmu_init_domain_context(struct iommu_domain *domain,
 		 */
 		cfg->cbar = CBAR_TYPE_S1_TRANS_S2_BYPASS;
 		start = smmu->num_s2_context_banks;
-	} else if (smmu->features & ARM_SMMU_FEAT_TRANS_S2) {
-		cfg->cbar = CBAR_TYPE_S2_TRANS;
-		start = 0;
-	} else {
+	} else if (smmu->features & ARM_SMMU_FEAT_TRANS_S1) {
 		cfg->cbar = CBAR_TYPE_S1_TRANS_S2_BYPASS;
 		start = smmu->num_s2_context_banks;
+	} else {
+		cfg->cbar = CBAR_TYPE_S2_TRANS;
+		start = 0;
 	}
 
 	ret = __arm_smmu_alloc_bitmap(smmu->context_map, start,
-- 
2.0.0