Re: [PATCH v2 5/7] iommu/arm-smmu: Implement of_xlate() for SMMUv3

On 14/06/16 16:07, Will Deacon wrote:

On Fri, Jun 03, 2016 at 06:15:40PM +0100, Robin Murphy wrote:

quoted

Now that we can properly describe the mapping between PCI RIDs and
stream IDs via "iommu-map", and have it fed it to the driver
automatically via of_xlate(), rework the SMMUv3 driver to benefit from
that. Initially, this just gets rid of the misuse of the "iommus"
binding without changing the driver's existing level of functionality,
but does at least pave the way to extending it more easily in future.

Signed-off-by: Robin Murphy <redacted>
---

v2: New.

  drivers/iommu/arm-smmu-v3.c | 119 +++++++++++++++++++++++++++-----------------
  1 file changed, 73 insertions(+), 46 deletions(-)

diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index 94b68213c50d..7631639cc209 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c

@@ -30,6 +30,7 @@
  #include <linux/msi.h>
  #include <linux/of.h>
  #include <linux/of_address.h>
+#include <linux/of_iommu.h>
  #include <linux/of_platform.h>
  #include <linux/pci.h>
  #include <linux/platform_device.h>

@@ -638,6 +639,12 @@ struct arm_smmu_domain {
  	struct iommu_domain		domain;
  };

+/* SMMU private data for each master */
+struct arm_smmu_master_data {
+	struct arm_smmu_device		*smmu;
+	u32				sid;
+};

Hmm, we already have this information in arm_smmu_group so I'd rather
avoid the duplication if we can.

For of_xlate() to work at all, we need to associate the IDs with their 
device long before groups are even available (because the device is only 
partially constructed), so at a bare minimum we need something to carry 
the data through until we see it again in add_device, where we can 
allocate a group.

quoted

@@ -1804,23 +1790,41 @@ static int arm_smmu_add_device(struct device *dev)
  {
  	int i, ret;
  	u32 sid, *sids;
-	struct pci_dev *pdev;
  	struct iommu_group *group;
+	struct device_node *np;
  	struct arm_smmu_group *smmu_group;
-	struct arm_smmu_device *smmu;
+	struct arm_smmu_device *smmu = NULL;
+	struct arm_smmu_master_data *data = dev->archdata.iommu;

-	/* We only support PCI, for now */
-	if (!dev_is_pci(dev))
+	if (!data)
  		return -ENODEV;

-	pdev = to_pci_dev(dev);
+	np = (struct device_node *)data->smmu;
+	smmu = data->smmu = arm_smmu_get_by_node(np);
+	of_node_put(np);
+	if (!smmu)
+		return -ENODEV;

Why can't we continue to use the group here?

I don't follow; at this point we've still not even allocated/found a 
group for this device yet. The code a few dozen lines later that takes 
the IDs and stashes them in the groupdata once it _does_ exist is 
unchanged in this patch, all that's changing is how those IDs got here 
in the first place.

Robin.

Will

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html