Add support for Hisilicon SMMU architecture

This patch adds a description of the device tree binding for the Hisilicon System MMU architecture.

Signed-off-by Zhen Lei <[email protected]>

---
 .../devicetree/bindings/iommu/hisilicon,smmu.txt   | 70 ++++++++++++++++++++++
 1 file changed, 70 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iommu/hisilicon,smmu.txt

Documentation/devicetree/bindings/iommu/hisilicon,smmu.txt new file +7 -0

new file mode 100644
index 0000000..a2b2f23
@@ -0,0 +1,70 @@
+* Hisilicon System MMU Architecture Implementation
+
+Hisilicon SoCs may contain an implementation of the Hisilicon System Memory
+Management Unit Architecture, which can be used to provide 1 or 2 stages
+of address translation to bus masters external to the CPU.
+
+The SMMU may also raise interrupts in response to various fault

Documentation/devicetree/bindings/iommu/hisilicon,smmu.txt new file +14 -0

+
+- reg           : Base address and size of the SMMU.
+
+- #global-interrupts : The number of global interrupts exposed by the
+                       device.
+
+- interrupts    : Interrupt list, with the first #global-irqs entries
+                  corresponding to the global interrupts and exactly one
+                  following entry corresponding to context interrupt.
+
+- smmu-masters  : A list of phandles to device nodes representing bus
+                  masters for which the SMMU can provide a translation
+                  and their corresponding StreamIDs (see example below).
+

Documentation/devicetree/bindings/iommu/hisilicon,smmu.txt new file +7 -1

+
+                /*
+                 * Two DMA controllers, each with exactly one StreamID.
+                 */
+                smmu-masters = <&dma0 0x0001>,
+                               <&dma1 0x0002>;
+        };
--

1.8.0

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Documentation/devicetree/bindings/iommu/hisilicon,smmu.txt line 30

Quoted 4 lines · On Thursday 05 June 2014 21:37:09 Zhen Lei wrote:

+- smmu-masters : A list of phandles to device nodes representing bus

Documentation/devicetree/bindings/iommu/hisilicon,smmu.txt new file +2 -0

+                  masters for which the SMMU can provide a translation
+                  and their corresponding StreamIDs (see example below).

We're currently in the process of defining a generic binding for IOMMUs.

While the smmu-masters property was copied from an existing binding, I think we will end up migrating away from that towards a common way to express those things, and we shouldn't add another one doing this in a nonstandard way. Please have a look at the latest discussion about the iommu binding using #iommu-cells and a reference from the master to the iommu and see if you can migrate your code to use that.

Quoted 7 lines · Arnd Bergmann

Documentation/devicetree/bindings/iommu/hisilicon,smmu.txt line 30

Quoted 4 lines · On Thursday 05 June 2014 21:37:09 Zhen Lei wrote:

+- smmu-masters : A list of phandles to device nodes representing bus

Documentation/devicetree/bindings/iommu/hisilicon,smmu.txt new file +2 -0

+                  masters for which the SMMU can provide a translation
+                  and their corresponding StreamIDs (see example below).

Quoted 8 lines

We're currently in the process of defining a generic binding for IOMMUs.

While the smmu-masters property was copied from an existing binding, I think we will end up migrating away from that towards a common way to express those things, and we shouldn't add another one doing this in a nonstandard way. Please have a look at the latest discussion about the iommu binding using #iommu-cells and a reference from the master to the iommu and see if you can migrate your code to use that.

Thanks for making this point -- I was going to do so yesterday but then hesitated due to uncertainty about whether this should really be a new driver.

Either way, it would be very valuable at least to attempt to describe the Hisilicon SMMU implemenation using the new proposals, since that is a good test of how reusable the proposed generic binding actually is.

Cheers
---Dave

.

Quoted 7 lines · Arnd Bergmann

Documentation/devicetree/bindings/iommu/hisilicon,smmu.txt line 30

Quoted 4 lines · On Thursday 05 June 2014 21:37:09 Zhen Lei wrote:

+- smmu-masters : A list of phandles to device nodes representing bus

Documentation/devicetree/bindings/iommu/hisilicon,smmu.txt new file +2 -0

+                  masters for which the SMMU can provide a translation
+                  and their corresponding StreamIDs (see example below).

Quoted 8 lines

We're currently in the process of defining a generic binding for IOMMUs.

While the smmu-masters property was copied from an existing binding, I think we will end up migrating away from that towards a common way to express those things, and we shouldn't add another one doing this in a nonstandard way. Please have a look at the latest discussion about the iommu binding using #iommu-cells and a reference from the master to the iommu and see if you can migrate your code to use that.

Thanks for making this point -- I was going to do so yesterday but then hesitated due to uncertainty about whether this should really be a new driver.

Either way, it would be very valuable at least to attempt to describe the Hisilicon SMMU implemenation using the new proposals, since that is a good test of how reusable the proposed generic binding actually is.

Quoted 17 lines · Dave Martin

Quoted 7 lines · Arnd Bergmann

Documentation/devicetree/bindings/iommu/hisilicon,smmu.txt line 30

Quoted 4 lines · On Thursday 05 June 2014 21:37:09 Zhen Lei wrote:

+- smmu-masters : A list of phandles to device nodes representing bus

Documentation/devicetree/bindings/iommu/hisilicon,smmu.txt new file +2 -0

+                  masters for which the SMMU can provide a translation
+                  and their corresponding StreamIDs (see example below).

Quoted 8 lines

We're currently in the process of defining a generic binding for IOMMUs.

While the smmu-masters property was copied from an existing binding, I think we will end up migrating away from that towards a common way to express those things, and we shouldn't add another one doing this in a nonstandard way. Please have a look at the latest discussion about the iommu binding using #iommu-cells and a reference from the master to the iommu and see if you can migrate your code to use that.

Quoted 7 lines

Thanks for making this point -- I was going to do so yesterday but then hesitated due to uncertainty about whether this should really be a new driver.

Either way, it would be very valuable at least to attempt to describe the Hisilicon SMMU implemenation using the new proposals, since that is a good test of how reusable the proposed generic binding actually is.

If this ends up being an addition to the existing ARM SMMU driver, I'm really not keen on using the new DT bindings. We're already stuck with the old bindings for that driver -- supporting both old and new in the same code only buys us maintenance headaches and pointless divergence within the driver.

Quoted 26 lines · On Monday 16 June 2014 17:26:53 Will Deacon wrote:

Quoted 17 lines · Dave Martin

Quoted 7 lines · Arnd Bergmann

Documentation/devicetree/bindings/iommu/hisilicon,smmu.txt line 30

Quoted 4 lines · On Thursday 05 June 2014 21:37:09 Zhen Lei wrote:

+- smmu-masters : A list of phandles to device nodes representing bus

Documentation/devicetree/bindings/iommu/hisilicon,smmu.txt new file +2 -0

+                  masters for which the SMMU can provide a translation
+                  and their corresponding StreamIDs (see example below).

Quoted 8 lines

We're currently in the process of defining a generic binding for IOMMUs.

While the smmu-masters property was copied from an existing binding, I think we will end up migrating away from that towards a common way to express those things, and we shouldn't add another one doing this in a nonstandard way. Please have a look at the latest discussion about the iommu binding using #iommu-cells and a reference from the master to the iommu and see if you can migrate your code to use that.

Quoted 7 lines

Thanks for making this point -- I was going to do so yesterday but then hesitated due to uncertainty about whether this should really be a new driver.

Either way, it would be very valuable at least to attempt to describe the Hisilicon SMMU implemenation using the new proposals, since that is a good test of how reusable the proposed generic binding actually is.

Quoted 5 lines

If this ends up being an addition to the existing ARM SMMU driver, I'm really not keen on using the new DT bindings. We're already stuck with the old bindings for that driver -- supporting both old and new in the same code only buys us maintenance headaches and pointless divergence within the driver.

We have to migrate the driver to the new binding anyway, it may be a bit painful, but there are not really any users yet so there is a chance we can remove the nonstandard code at some point, perhaps in a few years.

Quoted 33 lines · Arnd Bergmann

Quoted 26 lines · On Monday 16 June 2014 17:26:53 Will Deacon wrote:

Quoted 17 lines · Dave Martin

Quoted 7 lines · Arnd Bergmann

Documentation/devicetree/bindings/iommu/hisilicon,smmu.txt line 30

Quoted 4 lines · On Thursday 05 June 2014 21:37:09 Zhen Lei wrote:

+- smmu-masters : A list of phandles to device nodes representing bus

Documentation/devicetree/bindings/iommu/hisilicon,smmu.txt new file +2 -0

+                  masters for which the SMMU can provide a translation
+                  and their corresponding StreamIDs (see example below).

Quoted 8 lines

We're currently in the process of defining a generic binding for IOMMUs.

While the smmu-masters property was copied from an existing binding, I think we will end up migrating away from that towards a common way to express those things, and we shouldn't add another one doing this in a nonstandard way. Please have a look at the latest discussion about the iommu binding using #iommu-cells and a reference from the master to the iommu and see if you can migrate your code to use that.

Quoted 7 lines

Thanks for making this point -- I was going to do so yesterday but then hesitated due to uncertainty about whether this should really be a new driver.

Either way, it would be very valuable at least to attempt to describe the Hisilicon SMMU implemenation using the new proposals, since that is a good test of how reusable the proposed generic binding actually is.

Quoted 5 lines

If this ends up being an addition to the existing ARM SMMU driver, I'm really not keen on using the new DT bindings. We're already stuck with the old bindings for that driver -- supporting both old and new in the same code only buys us maintenance headaches and pointless divergence within the driver.

Quoted 4 lines

We have to migrate the driver to the new binding anyway, it may be a bit painful, but there are not really any users yet so there is a chance we can remove the nonstandard code at some point, perhaps in a few years.

The only way I see this working is if we kill the existing binding and move exclusively to the new one. I'm actually ok with that (we have no in-tree users), but it needs to happen ASAP in my opinion, otherwise we increase the window where the old binding can be adopted.

Note that the next version of the ARM SMMU (v3) is considerably different to the current architecture, so a new driver (using the new bindings) will be required.

This actually opens a wider question: if we don't have an in-tree user for a device-tree binding, do we consider that binding to be unused?

Quoted 6 lines · On Monday 16 June 2014 17:45:17 Will Deacon wrote:

Quoted 4 lines · Arnd Bergmann

We have to migrate the driver to the new binding anyway, it may be a bit painful, but there are not really any users yet so there is a chance we can remove the nonstandard code at some point, perhaps in a few years.

Quoted 4 lines

The only way I see this working is if we kill the existing binding and move exclusively to the new one. I'm actually ok with that (we have no in-tree users), but it needs to happen ASAP in my opinion, otherwise we increase the window where the old binding can be adopted.

I agree. I was hoping to get the generic binding ready for 3.16, but that didn't happen. Maybe we can add a small patch to the binding to explain that it will change in the future.

Quoted 4 lines

Note that the next version of the ARM SMMU (v3) is considerably different to the current architecture, so a new driver (using the new bindings) will be required.

Quoted 2 lines

This actually opens a wider question: if we don't have an in-tree user for a device-tree binding, do we consider that binding to be unused?

Not in general, but often. We don't require dts files to be in the kernel, so we have to apply a bit of common sense. If anyone knows of out-of-tree users of the binding that are actually working with upstream kernels, we need a migration path. Anything that also requires out-of-tree kernel patches however is something we don't have to worry about.

Quoted 12 lines · Arnd Bergmann

Quoted 10 lines · On Monday 16 June 2014 17:45:17 Will Deacon wrote:

Quoted 4 lines · Arnd Bergmann

We have to migrate the driver to the new binding anyway, it may be a bit painful, but there are not really any users yet so there is a chance we can remove the nonstandard code at some point, perhaps in a few years.

The only way I see this working is if we kill the existing binding and move exclusively to the new one. I'm actually ok with that (we have no in-tree users), but it needs to happen ASAP in my opinion, otherwise we increase the window where the old binding can be adopted.

Quoted 3 lines

I agree. I was hoping to get the generic binding ready for 3.16, but that didn't happen. Maybe we can add a small patch to the binding to explain that it will change in the future.

Perhaps, but saying "don't use this" isn't much better than just ripping out the support altogether. That said, I won't object to a patch adding a big fat warning to the current binding docs if it dissuades people from using what we currently have.

Quoted 4 lines

Quoted 4 lines

Note that the next version of the ARM SMMU (v3) is considerably different to the current architecture, so a new driver (using the new bindings) will be required.

Quoted 8 lines

Quoted 2 lines

This actually opens a wider question: if we don't have an in-tree user for a device-tree binding, do we consider that binding to be unused?

Not in general, but often. We don't require dts files to be in the kernel, so we have to apply a bit of common sense. If anyone knows of out-of-tree users of the binding that are actually working with upstream kernels, we need a migration path. Anything that also requires out-of-tree kernel patches however is something we don't have to worry about.

Ok. If Thierry's binding gets in for 3.17, then I'll try to convert the ARM SMMU driver over to it for 3.18 providing we don't grow any in-tree users of the existing binding in the meantime (or 3.17 depending on how early it gets queued).

Sound fair?

Quoted 13 lines · On Monday 16 June 2014 19:26:45 Will Deacon wrote:

Quoted 12 lines · Arnd Bergmann

Quoted 10 lines · On Monday 16 June 2014 17:45:17 Will Deacon wrote:

Quoted 4 lines · Arnd Bergmann

We have to migrate the driver to the new binding anyway, it may be a bit painful, but there are not really any users yet so there is a chance we can remove the nonstandard code at some point, perhaps in a few years.

The only way I see this working is if we kill the existing binding and move exclusively to the new one. I'm actually ok with that (we have no in-tree users), but it needs to happen ASAP in my opinion, otherwise we increase the window where the old binding can be adopted.

Quoted 8 lines

Quoted 3 lines

I agree. I was hoping to get the generic binding ready for 3.16, but that didn't happen. Maybe we can add a small patch to the binding to explain that it will change in the future.

Perhaps, but saying "don't use this" isn't much better than just ripping out the support altogether. That said, I won't object to a patch adding a big fat warning to the current binding docs if it dissuades people from using what we currently have.

Ok.

Quoted 18 lines

Quoted 4 lines

Quoted 4 lines

Note that the next version of the ARM SMMU (v3) is considerably different to the current architecture, so a new driver (using the new bindings) will be required.

Quoted 8 lines

Quoted 2 lines

This actually opens a wider question: if we don't have an in-tree user for a device-tree binding, do we consider that binding to be unused?

Not in general, but often. We don't require dts files to be in the kernel, so we have to apply a bit of common sense. If anyone knows of out-of-tree users of the binding that are actually working with upstream kernels, we need a migration path. Anything that also requires out-of-tree kernel patches however is something we don't have to worry about.

Ok. If Thierry's binding gets in for 3.17, then I'll try to convert the ARM SMMU driver over to it for 3.18 providing we don't grow any in-tree users of the existing binding in the meantime (or 3.17 depending on how early it gets queued).

Quoted 1 line

Sound fair?

Sounds good. Let's make sure we get this done quickly now. I think there isn't much controversy left with the binding, though this particular one has been tough in the past.

-----Original Message-----
From: linux-arm-kernel [mailto:linux-arm-kernel-
bounces at lists.infradead.org] On Behalf Of Will Deacon
Sent: Monday, June 16, 2014 11:57 PM
To: Arnd Bergmann
Cc: Kefeng Wang; Catalin Marinas; Tianhong Ding; huxinwei at huawei.com; Zefan Li; Zhen Lei; Dave P Martin; linux-arm-kernel at lists.infradead.org
Subject: Re: [PATCH RFC v1 1/2] documentation/iommu: Add description of Hisilicon System MMU binding

Quoted 11 lines · Arnd Bergmann

Quoted 10 lines · On Monday 16 June 2014 17:45:17 Will Deacon wrote:

Quoted 4 lines · Arnd Bergmann

We have to migrate the driver to the new binding anyway, it may be a bit painful, but there are not really any users yet so there is a chance we can remove the nonstandard code at some point, perhaps in a few years.

The only way I see this working is if we kill the existing binding and move exclusively to the new one. I'm actually ok with that (we have no in-tree users), but it needs to happen ASAP in my opinion, otherwise we increase the window where the old binding can be

adopted.

Quoted 4 lines

I agree. I was hoping to get the generic binding ready for 3.16, but that didn't happen. Maybe we can add a small patch to the binding to explain that it will change in the future.

Perhaps, but saying "don't use this" isn't much better than just ripping out the support altogether. That said, I won't object to a patch adding a big fat warning to the current binding docs if it dissuades people from using what we currently have.

Quoted 6 lines

Quoted 6 lines

Note that the next version of the ARM SMMU (v3) is considerably different to the current architecture, so a new driver (using the new bindings) will be required.

This actually opens a wider question: if we don't have an in-tree user for a device-tree binding, do we consider that binding to be

unused?

Quoted 6 lines

Not in general, but often. We don't require dts files to be in the kernel, so we have to apply a bit of common sense. If anyone knows of out-of-tree users of the binding that are actually working with upstream kernels, we need a migration path. Anything that also requires out-of-tree kernel patches however is something we don't have

to worry about.

Ok. If Thierry's binding gets in for 3.17, then I'll try to convert the ARM SMMU driver over to it for 3.18 providing we don't grow any in-tree users of the existing binding in the meantime (or 3.17 depending on how early it gets queued).

Sound fair?

Hi Will,

Quoted 6 lines

Ok. If Thierry's binding gets in for 3.17, then I'll try to convert the ARM SMMU driver over to it for 3.18 providing we don't grow any in-tree users of the existing binding in the meantime (or 3.17 depending on how early it gets queued).

Sound fair?

Would you be considering the handling of hot plug masters in the arm-smmu driver while incorporating the new binding?

-----Original Message-----
From: Will Deacon [mailto:will.deacon at arm.com]
Sent: Wednesday, June 18, 2014 6:01 PM
To: Sethi Varun-B16395
Cc: Arnd Bergmann; Kefeng Wang; Catalin Marinas; Tianhong Ding; huxinwei at huawei.com; Zefan Li; Zhen Lei; Dave P Martin; linux-arm- kernel at lists.infradead.org
Subject: Re: [PATCH RFC v1 1/2] documentation/iommu: Add description of Hisilicon System MMU binding

Quoted 1 line · Varun Sethi

Hi Will,

Hello,

Quoted 8 lines

Quoted 6 lines

Ok. If Thierry's binding gets in for 3.17, then I'll try to convert the ARM SMMU driver over to it for 3.18 providing we don't grow any in-tree users of the existing binding in the meantime (or 3.17 depending on how early it gets queued).

Sound fair?

Would you be considering the handling of hot plug masters in the arm-smmu driver while incorporating the new binding?

Not yet, no. I've not seen any bindings going near hotplug yet, so somebody would have to propose an extension to what we have.

Note that I *have* been playing with PCI on the ARM SMMU (see the patch below) but I currently just assume RequesterID == StreamID, which is true for the platform I'm using.

Will

--->8

commit a492cdaa266afc087e7d27692030455690fbca62

Author: Will Deacon <[email protected]>
Date:   Thu May 1 18:05:08 2014 +0100

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.

Signed-off-by Will Deacon <[email protected]>

drivers/iommu/arm-smmu.c index renamed from drivers/iommu/arm-smmu.c +0 -0

1599354e974d..0e2a12bd58a2 100644

drivers/iommu/arm-smmu.c +41 -12

@@ -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:

drivers/iommu/arm-smmu.c +36 -0

 	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) {

drivers/iommu/arm-smmu.c +3 -1

 	int idx;

@@ -853,7 +918,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; @@ -866,15
+932,15 @@ static int arm_smmu_init_domain_context(struct iommu_domain

*domain,

drivers/iommu/arm-smmu.c +1 -1

 	 * 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;

drivers/iommu/arm-smmu.c +4 -3

 	} 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;
 	}

@@ -918,7 +984,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); @@ -
1054,7 +1121,7 @@ static void arm_smmu_domain_destroy(struct iommu_domain *domain) }

static int arm_smmu_master_configure_smrs(struct arm_smmu_device *smmu,

drivers/iommu/arm-smmu.c +6 -6

-					  struct arm_smmu_master *master)
+					  struct arm_smmu_master_cfg *cfg)
 {
 	int i;
 	struct arm_smmu_smr *smrs;

@@ -1063,18 +1130,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,

drivers/iommu/arm-smmu.c +3 -3

 						  smmu->num_mapping_groups);
 		if (IS_ERR_VALUE(idx)) {

@@ -1085,18 +1152,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:

drivers/iommu/arm-smmu.c +12 -12

@@ -1107,44 +1174,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;

@@ -1159,14 +1226,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 |

drivers/iommu/arm-smmu.c +0 -0

 		       (smmu_domain->root_cfg.cbndx << S2CR_CBNDX_SHIFT);
 		writel_relaxed(s2cr, gr0_base + ARM_SMMU_GR0_S2CR(idx)); @@ -

1176,7 +1243,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,

drivers/iommu/arm-smmu.c +6 -5

-					  struct arm_smmu_master *master)
+					  struct arm_smmu_master_cfg *cfg)
 {
 	struct arm_smmu_device *smmu = smmu_domain->root_cfg.smmu;

@@ -1184,18 +1251,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");

drivers/iommu/arm-smmu.c +1 -3

 		return -ENXIO;

@@ -1208,11 +1276,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", @@ -1223,11 +1289,11 @@ static int arm_smmu_attach_dev(struct iommu_domain *domain, struct device *dev)

drivers/iommu/arm-smmu.c +4 -3

 	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); @@ -1237,11
+1303,11 @@ err_unlock:
 static void arm_smmu_detach_dev(struct iommu_domain *domain, struct

device *dev) {

drivers/iommu/arm-smmu.c +4 -4

 	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,8 +1615,7 @@ static int arm_smmu_domain_has_cap(struct iommu_domain *domain,

static int arm_smmu_add_device(struct device *dev) {

drivers/iommu/arm-smmu.c +6 -31

-	struct arm_smmu_device *child, *parent, *smmu;
-	struct arm_smmu_master *master = NULL;
+	struct arm_smmu_device *smmu;
 	struct iommu_group *group;
 	int ret;

@@ -1559,35 +1624,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();

@@ -1596,15 +1634,35 @@ 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);

Hi Will,

Quoted 3 lines

Note that I *have* been playing with PCI on the ARM SMMU (see the patch below) but I currently just assume RequesterID == StreamID, which is true for the platform I'm using.

Quoted 17 lines

drivers/iommu/arm-smmu.c +12 -0

@@ -1596,15 +1634,35 @@ 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;
+             cfg->streamids[0] = PCI_DEVID(pdev->bus->number, pdev-

Review static int arm_smmu_add_device(struct device · line 1650

Quoted 1 line

(quoted diff) +0 -0

 >devfn);

[Sethi Varun-B16395] We should be considering the bus topology i.e. what if the device is setting behind a bridge? It's possible the requestor id for the DMA transaction belongs to the bridge. Also, the iommu group creation should also take in to account the topology.

-----Original Message-----
From: Will Deacon [mailto:will.deacon at arm.com]
Sent: Friday, June 20, 2014 11:20 PM
To: Sethi Varun-B16395
Cc: Arnd Bergmann; Kefeng Wang; Catalin Marinas; Tianhong Ding; huxinwei at huawei.com; Zefan Li; Zhen Lei; Dave P Martin; linux-arm- kernel at lists.infradead.org
Subject: Re: [PATCH RFC v1 1/2] documentation/iommu: Add description of Hisilicon System MMU binding

Quoted 1 line · Varun Sethi

Hi Will,

Hello,

Quoted 4 lines

Quoted 4 lines

Note that I *have* been playing with PCI on the ARM SMMU (see the patch
below) but I currently just assume RequesterID == StreamID, which is true for the platform I'm using.

[...]

Quoted 17 lines

Quoted 17 lines

drivers/iommu/arm-smmu.c +12 -0

@@ -1596,15 +1634,35 @@ 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;
+             cfg->streamids[0] = PCI_DEVID(pdev->bus->number, pdev-

Quoted 5 lines

Review static int arm_smmu_add_device(struct device · line 1650

Quoted 1 line

(quoted diff) +0 -0

 >devfn);

[Sethi Varun-B16395] We should be considering the bus topology i.e. what if the device is setting behind a bridge? It's possible the requestor id for the DMA transaction belongs to the bridge. Also, the iommu group creation should also take in to account the topology.

Yeah, as I mentioned above, this assumes that RequesterID == StreamID. Are you simply alluding to a non-transparent PCI bridge, or do you have something different? For non-transparent bridges, I guess we can re-use the code already in the kernel (VFIO handles this with its groups IIUC)?

Quoted 9 lines

Quoted 4 lines

[Sethi Varun-B16395] We should be considering the bus topology i.e. what if the device is setting behind a bridge? It's possible the requestor id for the DMA transaction belongs to the bridge. Also, the iommu group creation should also take in to account the topology.

Yeah, as I mentioned above, this assumes that RequesterID == StreamID. Are you simply alluding to a non-transparent PCI bridge, or do you have something different? For non-transparent bridges, I guess we can re-use the code already in the kernel (VFIO handles this with its groups IIUC)?

Yes, we can use the API from patch posted by Alex Williamson.
https://lkml.org/lkml/2014/5/10/129

Quoted 26 lines · On Monday 16 June 2014 17:26:53 Will Deacon wrote:

Quoted 17 lines · Dave Martin

Quoted 7 lines · Arnd Bergmann

Documentation/devicetree/bindings/iommu/hisilicon,smmu.txt line 30

Quoted 4 lines · On Thursday 05 June 2014 21:37:09 Zhen Lei wrote:

+- smmu-masters : A list of phandles to device nodes representing bus

Documentation/devicetree/bindings/iommu/hisilicon,smmu.txt new file +2 -0

+                  masters for which the SMMU can provide a translation
+                  and their corresponding StreamIDs (see example below).

Quoted 8 lines

We're currently in the process of defining a generic binding for IOMMUs.

While the smmu-masters property was copied from an existing binding, I think we will end up migrating away from that towards a common way to express those things, and we shouldn't add another one doing this in a nonstandard way. Please have a look at the latest discussion about the iommu binding using #iommu-cells and a reference from the master to the iommu and see if you can migrate your code to use that.

Quoted 7 lines

Thanks for making this point -- I was going to do so yesterday but then hesitated due to uncertainty about whether this should really be a new driver.

Either way, it would be very valuable at least to attempt to describe the Hisilicon SMMU implemenation using the new proposals, since that is a good test of how reusable the proposed generic binding actually is.

Quoted 5 lines

If this ends up being an addition to the existing ARM SMMU driver, I'm really not keen on using the new DT bindings. We're already stuck with the old bindings for that driver -- supporting both old and new in the same code only buys us maintenance headaches and pointless divergence within the driver.

We have to migrate the driver to the new binding anyway, it may be a bit painful, but there are not really any users yet so there is a chance we can remove the nonstandard code at some point, perhaps in a few years.

Arnd

.

Documentation/devicetree/bindings/iommu/hisilicon,smmu.txt line 30

Quoted 4 lines · On Thursday 05 June 2014 21:37:09 Zhen Lei wrote:

+- smmu-masters : A list of phandles to device nodes representing bus

Documentation/devicetree/bindings/iommu/hisilicon,smmu.txt new file +2 -0

+                  masters for which the SMMU can provide a translation
+                  and their corresponding StreamIDs (see example below).

We're currently in the process of defining a generic binding for IOMMUs.

While the smmu-masters property was copied from an existing binding, I think we will end up migrating away from that towards a common way to express those things, and we shouldn't add another one doing this in a nonstandard way. Please have a look at the latest discussion about the iommu binding using #iommu-cells and a reference from the master to the iommu and see if you can migrate your code to use that.

This driver is base on arm_smmu.c, beause of hisilicon smmu-v1 is similar to arm-smmu, some code can be direct reused. for example: map and unmap, device tree configuration, and the software framework. Somebody may doubted: why not strip the share code in one file, and put hardware-dependent part into other files? Yeah, it's right. But now, in order to make the code flow clearly, just put all hisilicon smmu driver code in hisi_smmu.c.If this driver is acceptable, I will update and submit a revised verion.

Zhen Lei (2):
documentation/iommu: Add description of Hisilicon System MMU binding iommu/hisilicon: Add support for Hisilicon Ltd. System MMU
architecture

 .../devicetree/bindings/iommu/hisilicon,smmu.txt   |   70 +
 drivers/iommu/Kconfig                              |   11 +
 drivers/iommu/Makefile                             |    1 +
 drivers/iommu/hisi-smmu.c                          | 1686 ++++++++++++++++++++
 4 files changed, 1768 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iommu/hisilicon,smmu.txt
 create mode 100644 drivers/iommu/hisi-smmu.c

--
1.8.0

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Quoted 7 lines · Zhen Lei

This driver is base on arm_smmu.c, beause of hisilicon smmu-v1 is similar to arm-smmu, some code can be direct reused. for example: map and unmap, device tree configuration, and the software framework. Somebody may doubted: why not strip the share code in one file, and put hardware-dependent part into other files? Yeah, it's right. But now, in order to make the code flow clearly, just put all hisilicon smmu driver code in hisi_smmu.c.If this driver is acceptable, I will update and submit a revised verion.

This does not make the code flow clearer. If anything this makes it harder to review the series, because it's not clear what the delta from the existing ARM SMMU driver is.

No-one is going to be happy merging a redundant copy of (portions of) the existing ARM SMMU driver. Whatever gets merged will look very different due to the refactoring required to remedy that, so there's almost no point reviewing this in any depth until that's sorted out.

Mark.

Quoted 19 lines

Zhen Lei (2):
documentation/iommu: Add description of Hisilicon System MMU binding iommu/hisilicon: Add support for Hisilicon Ltd. System MMU
architecture

 .../devicetree/bindings/iommu/hisilicon,smmu.txt   |   70 +
 drivers/iommu/Kconfig                              |   11 +
 drivers/iommu/Makefile                             |    1 +
 drivers/iommu/hisi-smmu.c                          | 1686 ++++++++++++++++++++
 4 files changed, 1768 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iommu/hisilicon,smmu.txt
 create mode 100644 drivers/iommu/hisi-smmu.c

--
1.8.0

Quoted 5 lines

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

.