2018-05-20 13:51:08

by Nipun Gupta

[permalink] [raw]
Subject: [PATCH 0/7 v5] Support for fsl-mc bus and its devices in SMMU

This patchset defines IOMMU DT binding for fsl-mc bus and adds
support in SMMU for fsl-mc bus.

The patch series is based on top of dma-mapping tree (for-next branch):
http://git.infradead.org/users/hch/dma-mapping.git

These patches
- Define property 'iommu-map' for fsl-mc bus (patch 1)
- Integrates the fsl-mc bus with the SMMU using this
IOMMU binding (patch 2,3,4)
- Adds the dma configuration support for fsl-mc bus (patch 5, 6)
- Updates the fsl-mc device node with iommu/dma related changes (patch 7)

Changes in v2:
- use iommu-map property for fsl-mc bus
- rebase over patchset https://patchwork.kernel.org/patch/10317337/
and make corresponding changes for dma configuration of devices on
fsl-mc bus

Changes in v3:
- move of_map_rid in drivers/of/address.c

Changes in v4:
- move of_map_rid in drivers/of/base.c

Changes in v5:
- break patch 5 in two separate patches (now patch 5/7 and patch 6/7)
- add changelog text in patch 3/7 and patch 5/7
- typo fix

Nipun Gupta (7):
Docs: dt: add fsl-mc iommu-map device-tree binding
iommu: of: make of_pci_map_rid() available for other devices too
iommu: support iommu configuration for fsl-mc devices
iommu: arm-smmu: Add support for the fsl-mc bus
bus: fsl-mc: support dma configure for devices on fsl-mc bus
bus: fsl-mc: set coherent dma mask for devices on fsl-mc bus
arm64: dts: ls208xa: comply with the iommu map binding for fsl_mc

.../devicetree/bindings/misc/fsl,qoriq-mc.txt | 39 ++++++++
arch/arm64/boot/dts/freescale/fsl-ls208xa.dtsi | 6 +-
drivers/bus/fsl-mc/fsl-mc-bus.c | 16 +++-
drivers/iommu/arm-smmu.c | 7 ++
drivers/iommu/iommu.c | 21 +++++
drivers/iommu/of_iommu.c | 25 ++++-
drivers/of/base.c | 102 +++++++++++++++++++++
drivers/of/irq.c | 5 +-
drivers/pci/of.c | 101 --------------------
include/linux/fsl/mc.h | 8 ++
include/linux/iommu.h | 2 +
include/linux/of.h | 11 +++
include/linux/of_pci.h | 10 --
13 files changed, 231 insertions(+), 122 deletions(-)

--
1.9.1



2018-05-20 13:51:24

by Nipun Gupta

[permalink] [raw]
Subject: [PATCH 2/7 v5] iommu: of: make of_pci_map_rid() available for other devices too

iommu-map property is also used by devices with fsl-mc. This
patch moves the of_pci_map_rid to generic location, so that it
can be used by other busses too.

'of_pci_map_rid' is renamed here to 'of_map_rid' and there is no
functional change done in the API.

Signed-off-by: Nipun Gupta <[email protected]>
Reviewed-by: Rob Herring <[email protected]>
Acked-by: Bjorn Helgaas <[email protected]>
---
drivers/iommu/of_iommu.c | 5 +--
drivers/of/base.c | 102 +++++++++++++++++++++++++++++++++++++++++++++++
drivers/of/irq.c | 5 +--
drivers/pci/of.c | 101 ----------------------------------------------
include/linux/of.h | 11 +++++
include/linux/of_pci.h | 10 -----
6 files changed, 117 insertions(+), 117 deletions(-)

diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
index 5c36a8b..811e160 100644
--- a/drivers/iommu/of_iommu.c
+++ b/drivers/iommu/of_iommu.c
@@ -149,9 +149,8 @@ static int of_pci_iommu_init(struct pci_dev *pdev, u16 alias, void *data)
struct of_phandle_args iommu_spec = { .args_count = 1 };
int err;

- err = of_pci_map_rid(info->np, alias, "iommu-map",
- "iommu-map-mask", &iommu_spec.np,
- iommu_spec.args);
+ err = of_map_rid(info->np, alias, "iommu-map", "iommu-map-mask",
+ &iommu_spec.np, iommu_spec.args);
if (err)
return err == -ENODEV ? NO_IOMMU : err;

diff --git a/drivers/of/base.c b/drivers/of/base.c
index 848f549..c7aac81 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -1995,3 +1995,105 @@ int of_find_last_cache_level(unsigned int cpu)

return cache_level;
}
+
+/**
+ * of_map_rid - Translate a requester ID through a downstream mapping.
+ * @np: root complex device node.
+ * @rid: device requester ID to map.
+ * @map_name: property name of the map to use.
+ * @map_mask_name: optional property name of the mask to use.
+ * @target: optional pointer to a target device node.
+ * @id_out: optional pointer to receive the translated ID.
+ *
+ * Given a device requester ID, look up the appropriate implementation-defined
+ * platform ID and/or the target device which receives transactions on that
+ * ID, as per the "iommu-map" and "msi-map" bindings. Either of @target or
+ * @id_out may be NULL if only the other is required. If @target points to
+ * a non-NULL device node pointer, only entries targeting that node will be
+ * matched; if it points to a NULL value, it will receive the device node of
+ * the first matching target phandle, with a reference held.
+ *
+ * Return: 0 on success or a standard error code on failure.
+ */
+int of_map_rid(struct device_node *np, u32 rid,
+ const char *map_name, const char *map_mask_name,
+ struct device_node **target, u32 *id_out)
+{
+ u32 map_mask, masked_rid;
+ int map_len;
+ const __be32 *map = NULL;
+
+ if (!np || !map_name || (!target && !id_out))
+ return -EINVAL;
+
+ map = of_get_property(np, map_name, &map_len);
+ if (!map) {
+ if (target)
+ return -ENODEV;
+ /* Otherwise, no map implies no translation */
+ *id_out = rid;
+ return 0;
+ }
+
+ if (!map_len || map_len % (4 * sizeof(*map))) {
+ pr_err("%pOF: Error: Bad %s length: %d\n", np,
+ map_name, map_len);
+ return -EINVAL;
+ }
+
+ /* The default is to select all bits. */
+ map_mask = 0xffffffff;
+
+ /*
+ * Can be overridden by "{iommu,msi}-map-mask" property.
+ * If of_property_read_u32() fails, the default is used.
+ */
+ if (map_mask_name)
+ of_property_read_u32(np, map_mask_name, &map_mask);
+
+ masked_rid = map_mask & rid;
+ for ( ; map_len > 0; map_len -= 4 * sizeof(*map), map += 4) {
+ struct device_node *phandle_node;
+ u32 rid_base = be32_to_cpup(map + 0);
+ u32 phandle = be32_to_cpup(map + 1);
+ u32 out_base = be32_to_cpup(map + 2);
+ u32 rid_len = be32_to_cpup(map + 3);
+
+ if (rid_base & ~map_mask) {
+ pr_err("%pOF: Invalid %s translation - %s-mask (0x%x) ignores rid-base (0x%x)\n",
+ np, map_name, map_name,
+ map_mask, rid_base);
+ return -EFAULT;
+ }
+
+ if (masked_rid < rid_base || masked_rid >= rid_base + rid_len)
+ continue;
+
+ phandle_node = of_find_node_by_phandle(phandle);
+ if (!phandle_node)
+ return -ENODEV;
+
+ if (target) {
+ if (*target)
+ of_node_put(phandle_node);
+ else
+ *target = phandle_node;
+
+ if (*target != phandle_node)
+ continue;
+ }
+
+ if (id_out)
+ *id_out = masked_rid - rid_base + out_base;
+
+ pr_debug("%pOF: %s, using mask %08x, rid-base: %08x, out-base: %08x, length: %08x, rid: %08x -> %08x\n",
+ np, map_name, map_mask, rid_base, out_base,
+ rid_len, rid, masked_rid - rid_base + out_base);
+ return 0;
+ }
+
+ pr_err("%pOF: Invalid %s translation - no match for rid 0x%x on %pOF\n",
+ np, map_name, rid, target && *target ? *target : NULL);
+ return -EFAULT;
+}
+EXPORT_SYMBOL_GPL(of_map_rid);
diff --git a/drivers/of/irq.c b/drivers/of/irq.c
index 02ad93a..e1f6f39 100644
--- a/drivers/of/irq.c
+++ b/drivers/of/irq.c
@@ -22,7 +22,6 @@
#include <linux/module.h>
#include <linux/of.h>
#include <linux/of_irq.h>
-#include <linux/of_pci.h>
#include <linux/string.h>
#include <linux/slab.h>

@@ -588,8 +587,8 @@ static u32 __of_msi_map_rid(struct device *dev, struct device_node **np,
* "msi-map" property.
*/
for (parent_dev = dev; parent_dev; parent_dev = parent_dev->parent)
- if (!of_pci_map_rid(parent_dev->of_node, rid_in, "msi-map",
- "msi-map-mask", np, &rid_out))
+ if (!of_map_rid(parent_dev->of_node, rid_in, "msi-map",
+ "msi-map-mask", np, &rid_out))
break;
return rid_out;
}
diff --git a/drivers/pci/of.c b/drivers/pci/of.c
index a28355c..d2cebbe 100644
--- a/drivers/pci/of.c
+++ b/drivers/pci/of.c
@@ -362,107 +362,6 @@ int of_pci_get_host_bridge_resources(struct device_node *dev,
EXPORT_SYMBOL_GPL(of_pci_get_host_bridge_resources);
#endif /* CONFIG_OF_ADDRESS */

-/**
- * of_pci_map_rid - Translate a requester ID through a downstream mapping.
- * @np: root complex device node.
- * @rid: PCI requester ID to map.
- * @map_name: property name of the map to use.
- * @map_mask_name: optional property name of the mask to use.
- * @target: optional pointer to a target device node.
- * @id_out: optional pointer to receive the translated ID.
- *
- * Given a PCI requester ID, look up the appropriate implementation-defined
- * platform ID and/or the target device which receives transactions on that
- * ID, as per the "iommu-map" and "msi-map" bindings. Either of @target or
- * @id_out may be NULL if only the other is required. If @target points to
- * a non-NULL device node pointer, only entries targeting that node will be
- * matched; if it points to a NULL value, it will receive the device node of
- * the first matching target phandle, with a reference held.
- *
- * Return: 0 on success or a standard error code on failure.
- */
-int of_pci_map_rid(struct device_node *np, u32 rid,
- const char *map_name, const char *map_mask_name,
- struct device_node **target, u32 *id_out)
-{
- u32 map_mask, masked_rid;
- int map_len;
- const __be32 *map = NULL;
-
- if (!np || !map_name || (!target && !id_out))
- return -EINVAL;
-
- map = of_get_property(np, map_name, &map_len);
- if (!map) {
- if (target)
- return -ENODEV;
- /* Otherwise, no map implies no translation */
- *id_out = rid;
- return 0;
- }
-
- if (!map_len || map_len % (4 * sizeof(*map))) {
- pr_err("%pOF: Error: Bad %s length: %d\n", np,
- map_name, map_len);
- return -EINVAL;
- }
-
- /* The default is to select all bits. */
- map_mask = 0xffffffff;
-
- /*
- * Can be overridden by "{iommu,msi}-map-mask" property.
- * If of_property_read_u32() fails, the default is used.
- */
- if (map_mask_name)
- of_property_read_u32(np, map_mask_name, &map_mask);
-
- masked_rid = map_mask & rid;
- for ( ; map_len > 0; map_len -= 4 * sizeof(*map), map += 4) {
- struct device_node *phandle_node;
- u32 rid_base = be32_to_cpup(map + 0);
- u32 phandle = be32_to_cpup(map + 1);
- u32 out_base = be32_to_cpup(map + 2);
- u32 rid_len = be32_to_cpup(map + 3);
-
- if (rid_base & ~map_mask) {
- pr_err("%pOF: Invalid %s translation - %s-mask (0x%x) ignores rid-base (0x%x)\n",
- np, map_name, map_name,
- map_mask, rid_base);
- return -EFAULT;
- }
-
- if (masked_rid < rid_base || masked_rid >= rid_base + rid_len)
- continue;
-
- phandle_node = of_find_node_by_phandle(phandle);
- if (!phandle_node)
- return -ENODEV;
-
- if (target) {
- if (*target)
- of_node_put(phandle_node);
- else
- *target = phandle_node;
-
- if (*target != phandle_node)
- continue;
- }
-
- if (id_out)
- *id_out = masked_rid - rid_base + out_base;
-
- pr_debug("%pOF: %s, using mask %08x, rid-base: %08x, out-base: %08x, length: %08x, rid: %08x -> %08x\n",
- np, map_name, map_mask, rid_base, out_base,
- rid_len, rid, masked_rid - rid_base + out_base);
- return 0;
- }
-
- pr_err("%pOF: Invalid %s translation - no match for rid 0x%x on %pOF\n",
- np, map_name, rid, target && *target ? *target : NULL);
- return -EFAULT;
-}
-
#if IS_ENABLED(CONFIG_OF_IRQ)
/**
* of_irq_parse_pci - Resolve the interrupt for a PCI device
diff --git a/include/linux/of.h b/include/linux/of.h
index 4d25e4f..f4251c3 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -545,6 +545,10 @@ const __be32 *of_prop_next_u32(struct property *prop, const __be32 *cur,

extern int of_cpu_node_to_id(struct device_node *np);

+int of_map_rid(struct device_node *np, u32 rid,
+ const char *map_name, const char *map_mask_name,
+ struct device_node **target, u32 *id_out);
+
#else /* CONFIG_OF */

static inline void of_core_init(void)
@@ -931,6 +935,13 @@ static inline int of_cpu_node_to_id(struct device_node *np)
return -ENODEV;
}

+static inline int of_map_rid(struct device_node *np, u32 rid,
+ const char *map_name, const char *map_mask_name,
+ struct device_node **target, u32 *id_out)
+{
+ return -EINVAL;
+}
+
#define of_match_ptr(_ptr) NULL
#define of_match_node(_matches, _node) NULL
#endif /* CONFIG_OF */
diff --git a/include/linux/of_pci.h b/include/linux/of_pci.h
index 091033a..a23b44a 100644
--- a/include/linux/of_pci.h
+++ b/include/linux/of_pci.h
@@ -17,9 +17,6 @@ struct device_node *of_pci_find_child_device(struct device_node *parent,
int of_get_pci_domain_nr(struct device_node *node);
int of_pci_get_max_link_speed(struct device_node *node);
void of_pci_check_probe_only(void);
-int of_pci_map_rid(struct device_node *np, u32 rid,
- const char *map_name, const char *map_mask_name,
- struct device_node **target, u32 *id_out);
#else
static inline struct device_node *of_pci_find_child_device(struct device_node *parent,
unsigned int devfn)
@@ -44,13 +41,6 @@ static inline int of_pci_get_devfn(struct device_node *np)
return -1;
}

-static inline int of_pci_map_rid(struct device_node *np, u32 rid,
- const char *map_name, const char *map_mask_name,
- struct device_node **target, u32 *id_out)
-{
- return -EINVAL;
-}
-
static inline int
of_pci_get_max_link_speed(struct device_node *node)
{
--
1.9.1


2018-05-20 13:51:47

by Nipun Gupta

[permalink] [raw]
Subject: [PATCH 4/7 v5] iommu: arm-smmu: Add support for the fsl-mc bus

Implement bus specific support for the fsl-mc bus including
registering arm_smmu_ops and bus specific device add operations.

Signed-off-by: Nipun Gupta <[email protected]>
---
drivers/iommu/arm-smmu.c | 7 +++++++
drivers/iommu/iommu.c | 21 +++++++++++++++++++++
include/linux/fsl/mc.h | 8 ++++++++
include/linux/iommu.h | 2 ++
4 files changed, 38 insertions(+)

diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index 69e7c60..e1d5090 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -52,6 +52,7 @@
#include <linux/spinlock.h>

#include <linux/amba/bus.h>
+#include <linux/fsl/mc.h>

#include "io-pgtable.h"
#include "arm-smmu-regs.h"
@@ -1459,6 +1460,8 @@ static struct iommu_group *arm_smmu_device_group(struct device *dev)

if (dev_is_pci(dev))
group = pci_device_group(dev);
+ else if (dev_is_fsl_mc(dev))
+ group = fsl_mc_device_group(dev);
else
group = generic_device_group(dev);

@@ -2037,6 +2040,10 @@ static void arm_smmu_bus_init(void)
bus_set_iommu(&pci_bus_type, &arm_smmu_ops);
}
#endif
+#ifdef CONFIG_FSL_MC_BUS
+ if (!iommu_present(&fsl_mc_bus_type))
+ bus_set_iommu(&fsl_mc_bus_type, &arm_smmu_ops);
+#endif
}

static int arm_smmu_device_probe(struct platform_device *pdev)
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index d2aa2320..6d4ce35 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -32,6 +32,7 @@
#include <linux/pci.h>
#include <linux/bitops.h>
#include <linux/property.h>
+#include <linux/fsl/mc.h>
#include <trace/events/iommu.h>

static struct kset *iommu_group_kset;
@@ -987,6 +988,26 @@ struct iommu_group *pci_device_group(struct device *dev)
return iommu_group_alloc();
}

+/* Get the IOMMU group for device on fsl-mc bus */
+struct iommu_group *fsl_mc_device_group(struct device *dev)
+{
+ struct device *cont_dev = fsl_mc_cont_dev(dev);
+ struct iommu_group *group;
+
+ /* Container device is responsible for creating the iommu group */
+ if (fsl_mc_is_cont_dev(dev)) {
+ group = iommu_group_alloc();
+ if (IS_ERR(group))
+ return NULL;
+ } else {
+ get_device(cont_dev);
+ group = iommu_group_get(cont_dev);
+ put_device(cont_dev);
+ }
+
+ return group;
+}
+
/**
* iommu_group_get_for_dev - Find or create the IOMMU group for a device
* @dev: target device
diff --git a/include/linux/fsl/mc.h b/include/linux/fsl/mc.h
index f27cb14..dddaca1 100644
--- a/include/linux/fsl/mc.h
+++ b/include/linux/fsl/mc.h
@@ -351,6 +351,14 @@ struct fsl_mc_io {
#define dev_is_fsl_mc(_dev) (0)
#endif

+/* Macro to check if a device is a container device */
+#define fsl_mc_is_cont_dev(_dev) (to_fsl_mc_device(_dev)->flags & \
+ FSL_MC_IS_DPRC)
+
+/* Macro to get the container device of a MC device */
+#define fsl_mc_cont_dev(_dev) (fsl_mc_is_cont_dev(_dev) ? \
+ (_dev) : (_dev)->parent)
+
/*
* module_fsl_mc_driver() - Helper macro for drivers that don't do
* anything special in module init/exit. This eliminates a lot of
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 19938ee..2981200 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -389,6 +389,8 @@ static inline size_t iommu_map_sg(struct iommu_domain *domain,
extern struct iommu_group *pci_device_group(struct device *dev);
/* Generic device grouping function */
extern struct iommu_group *generic_device_group(struct device *dev);
+/* FSL-MC device grouping function */
+struct iommu_group *fsl_mc_device_group(struct device *dev);

/**
* struct iommu_fwspec - per-device IOMMU instance data
--
1.9.1


2018-05-20 13:52:09

by Nipun Gupta

[permalink] [raw]
Subject: [PATCH 6/7 v5] bus: fsl-mc: set coherent dma mask for devices on fsl-mc bus

of_dma_configure() API expects coherent_dma_mask to be correctly
set in the devices. This patch does the needful.

Signed-off-by: Nipun Gupta <[email protected]>
---
drivers/bus/fsl-mc/fsl-mc-bus.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/drivers/bus/fsl-mc/fsl-mc-bus.c b/drivers/bus/fsl-mc/fsl-mc-bus.c
index fa43c7d..624828b 100644
--- a/drivers/bus/fsl-mc/fsl-mc-bus.c
+++ b/drivers/bus/fsl-mc/fsl-mc-bus.c
@@ -627,6 +627,7 @@ int fsl_mc_device_add(struct fsl_mc_obj_desc *obj_desc,
mc_dev->icid = parent_mc_dev->icid;
mc_dev->dma_mask = FSL_MC_DEFAULT_DMA_MASK;
mc_dev->dev.dma_mask = &mc_dev->dma_mask;
+ mc_dev->dev.coherent_dma_mask = mc_dev->dma_mask;
dev_set_msi_domain(&mc_dev->dev,
dev_get_msi_domain(&parent_mc_dev->dev));
}
--
1.9.1


2018-05-20 13:52:36

by Nipun Gupta

[permalink] [raw]
Subject: [PATCH 5/7 v5] bus: fsl-mc: support dma configure for devices on fsl-mc bus

This patch adds support of dma configuration for devices on fsl-mc
bus using 'dma_configure' callback for busses. Also, directly calling
arch_setup_dma_ops is removed from the fsl-mc bus.

Signed-off-by: Nipun Gupta <[email protected]>
Reviewed-by: Laurentiu Tudor <[email protected]>
---
drivers/bus/fsl-mc/fsl-mc-bus.c | 15 +++++++++++----
1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/drivers/bus/fsl-mc/fsl-mc-bus.c b/drivers/bus/fsl-mc/fsl-mc-bus.c
index 5d8266c..fa43c7d 100644
--- a/drivers/bus/fsl-mc/fsl-mc-bus.c
+++ b/drivers/bus/fsl-mc/fsl-mc-bus.c
@@ -127,6 +127,16 @@ static int fsl_mc_bus_uevent(struct device *dev, struct kobj_uevent_env *env)
return 0;
}

+static int fsl_mc_dma_configure(struct device *dev)
+{
+ struct device *dma_dev = dev;
+
+ while (dev_is_fsl_mc(dma_dev))
+ dma_dev = dma_dev->parent;
+
+ return of_dma_configure(dev, dma_dev->of_node, 0);
+}
+
static ssize_t modalias_show(struct device *dev, struct device_attribute *attr,
char *buf)
{
@@ -148,6 +158,7 @@ struct bus_type fsl_mc_bus_type = {
.name = "fsl-mc",
.match = fsl_mc_bus_match,
.uevent = fsl_mc_bus_uevent,
+ .dma_configure = fsl_mc_dma_configure,
.dev_groups = fsl_mc_dev_groups,
};
EXPORT_SYMBOL_GPL(fsl_mc_bus_type);
@@ -633,10 +644,6 @@ int fsl_mc_device_add(struct fsl_mc_obj_desc *obj_desc,
goto error_cleanup_dev;
}

- /* Objects are coherent, unless 'no shareability' flag set. */
- if (!(obj_desc->flags & FSL_MC_OBJ_FLAG_NO_MEM_SHAREABILITY))
- arch_setup_dma_ops(&mc_dev->dev, 0, 0, NULL, true);
-
/*
* The device-specific probe callback will get invoked by device_add()
*/
--
1.9.1


2018-05-20 13:53:04

by Nipun Gupta

[permalink] [raw]
Subject: [PATCH 1/7 v5] Docs: dt: add fsl-mc iommu-map device-tree binding

The existing IOMMU bindings cannot be used to specify the relationship
between fsl-mc devices and IOMMUs. This patch adds a generic binding for
mapping fsl-mc devices to IOMMUs, using iommu-map property.

Signed-off-by: Nipun Gupta <[email protected]>
Reviewed-by: Rob Herring <[email protected]>
---
.../devicetree/bindings/misc/fsl,qoriq-mc.txt | 39 ++++++++++++++++++++++
1 file changed, 39 insertions(+)

diff --git a/Documentation/devicetree/bindings/misc/fsl,qoriq-mc.txt b/Documentation/devicetree/bindings/misc/fsl,qoriq-mc.txt
index 6611a7c..8cbed4f 100644
--- a/Documentation/devicetree/bindings/misc/fsl,qoriq-mc.txt
+++ b/Documentation/devicetree/bindings/misc/fsl,qoriq-mc.txt
@@ -9,6 +9,25 @@ blocks that can be used to create functional hardware objects/devices
such as network interfaces, crypto accelerator instances, L2 switches,
etc.

+For an overview of the DPAA2 architecture and fsl-mc bus see:
+drivers/staging/fsl-mc/README.txt
+
+As described in the above overview, all DPAA2 objects in a DPRC share the
+same hardware "isolation context" and a 10-bit value called an ICID
+(isolation context id) is expressed by the hardware to identify
+the requester.
+
+The generic 'iommus' property is insufficient to describe the relationship
+between ICIDs and IOMMUs, so an iommu-map property is used to define
+the set of possible ICIDs under a root DPRC and how they map to
+an IOMMU.
+
+For generic IOMMU bindings, see
+Documentation/devicetree/bindings/iommu/iommu.txt.
+
+For arm-smmu binding, see:
+Documentation/devicetree/bindings/iommu/arm,smmu.txt.
+
Required properties:

- compatible
@@ -88,14 +107,34 @@ Sub-nodes:
Value type: <phandle>
Definition: Specifies the phandle to the PHY device node associated
with the this dpmac.
+Optional properties:
+
+- iommu-map: Maps an ICID to an IOMMU and associated iommu-specifier
+ data.
+
+ The property is an arbitrary number of tuples of
+ (icid-base,iommu,iommu-base,length).
+
+ Any ICID i in the interval [icid-base, icid-base + length) is
+ associated with the listed IOMMU, with the iommu-specifier
+ (i - icid-base + iommu-base).

Example:

+ smmu: iommu@5000000 {
+ compatible = "arm,mmu-500";
+ #iommu-cells = <2>;
+ stream-match-mask = <0x7C00>;
+ ...
+ };
+
fsl_mc: fsl-mc@80c000000 {
compatible = "fsl,qoriq-mc";
reg = <0x00000008 0x0c000000 0 0x40>, /* MC portal base */
<0x00000000 0x08340000 0 0x40000>; /* MC control reg */
msi-parent = <&its>;
+ /* define map for ICIDs 23-64 */
+ iommu-map = <23 &smmu 23 41>;
#address-cells = <3>;
#size-cells = <1>;

--
1.9.1


2018-05-20 13:53:07

by Nipun Gupta

[permalink] [raw]
Subject: [PATCH 7/7 v5] arm64: dts: ls208xa: comply with the iommu map binding for fsl_mc

fsl-mc bus support the new iommu-map property. Comply to this binding
for fsl_mc bus.

Signed-off-by: Nipun Gupta <[email protected]>
Reviewed-by: Laurentiu Tudor <[email protected]>
---
arch/arm64/boot/dts/freescale/fsl-ls208xa.dtsi | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/freescale/fsl-ls208xa.dtsi b/arch/arm64/boot/dts/freescale/fsl-ls208xa.dtsi
index 137ef4d..6010505 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls208xa.dtsi
+++ b/arch/arm64/boot/dts/freescale/fsl-ls208xa.dtsi
@@ -184,6 +184,7 @@
#address-cells = <2>;
#size-cells = <2>;
ranges;
+ dma-ranges = <0x0 0x0 0x0 0x0 0x10000 0x00000000>;

clockgen: clocking@1300000 {
compatible = "fsl,ls2080a-clockgen";
@@ -357,6 +358,8 @@
reg = <0x00000008 0x0c000000 0 0x40>, /* MC portal base */
<0x00000000 0x08340000 0 0x40000>; /* MC control reg */
msi-parent = <&its>;
+ iommu-map = <0 &smmu 0 0>; /* This is fixed-up by u-boot */
+ dma-coherent;
#address-cells = <3>;
#size-cells = <1>;

@@ -460,6 +463,8 @@
compatible = "arm,mmu-500";
reg = <0 0x5000000 0 0x800000>;
#global-interrupts = <12>;
+ #iommu-cells = <1>;
+ stream-match-mask = <0x7C00>;
interrupts = <0 13 4>, /* global secure fault */
<0 14 4>, /* combined secure interrupt */
<0 15 4>, /* global non-secure fault */
@@ -502,7 +507,6 @@
<0 204 4>, <0 205 4>,
<0 206 4>, <0 207 4>,
<0 208 4>, <0 209 4>;
- mmu-masters = <&fsl_mc 0x300 0>;
};

dspi: dspi@2100000 {
--
1.9.1


2018-05-20 13:54:01

by Nipun Gupta

[permalink] [raw]
Subject: [PATCH 3/7 v5] iommu: support iommu configuration for fsl-mc devices

With of_pci_map_rid available for all the busses, use the function
for configuration of devices on fsl-mc bus

Signed-off-by: Nipun Gupta <[email protected]>
---
drivers/iommu/of_iommu.c | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)

diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
index 811e160..284474d 100644
--- a/drivers/iommu/of_iommu.c
+++ b/drivers/iommu/of_iommu.c
@@ -24,6 +24,7 @@
#include <linux/of_iommu.h>
#include <linux/of_pci.h>
#include <linux/slab.h>
+#include <linux/fsl/mc.h>

#define NO_IOMMU 1

@@ -159,6 +160,23 @@ static int of_pci_iommu_init(struct pci_dev *pdev, u16 alias, void *data)
return err;
}

+static int of_fsl_mc_iommu_init(struct fsl_mc_device *mc_dev,
+ struct device_node *master_np)
+{
+ struct of_phandle_args iommu_spec = { .args_count = 1 };
+ int err;
+
+ err = of_map_rid(master_np, mc_dev->icid, "iommu-map",
+ "iommu-map-mask", &iommu_spec.np,
+ iommu_spec.args);
+ if (err)
+ return err == -ENODEV ? NO_IOMMU : err;
+
+ err = of_iommu_xlate(&mc_dev->dev, &iommu_spec);
+ of_node_put(iommu_spec.np);
+ return err;
+}
+
const struct iommu_ops *of_iommu_configure(struct device *dev,
struct device_node *master_np)
{
@@ -190,6 +208,8 @@ const struct iommu_ops *of_iommu_configure(struct device *dev,

err = pci_for_each_dma_alias(to_pci_dev(dev),
of_pci_iommu_init, &info);
+ } else if (dev_is_fsl_mc(dev)) {
+ err = of_fsl_mc_iommu_init(to_fsl_mc_device(dev), master_np);
} else {
struct of_phandle_args iommu_spec;
int idx = 0;
--
1.9.1


2018-05-22 07:06:14

by Laurentiu Tudor

[permalink] [raw]
Subject: Re: [PATCH 6/7 v5] bus: fsl-mc: set coherent dma mask for devices on fsl-mc bus



On 05/20/2018 04:49 PM, Nipun Gupta wrote:
> of_dma_configure() API expects coherent_dma_mask to be correctly
> set in the devices. This patch does the needful.
>
> Signed-off-by: Nipun Gupta <[email protected]>

Acked-by: Laurentiu Tudor <[email protected]>

---
Best Regards, Laurentiu

> ---
> drivers/bus/fsl-mc/fsl-mc-bus.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/bus/fsl-mc/fsl-mc-bus.c b/drivers/bus/fsl-mc/fsl-mc-bus.c
> index fa43c7d..624828b 100644
> --- a/drivers/bus/fsl-mc/fsl-mc-bus.c
> +++ b/drivers/bus/fsl-mc/fsl-mc-bus.c
> @@ -627,6 +627,7 @@ int fsl_mc_device_add(struct fsl_mc_obj_desc *obj_desc,
> mc_dev->icid = parent_mc_dev->icid;
> mc_dev->dma_mask = FSL_MC_DEFAULT_DMA_MASK;
> mc_dev->dev.dma_mask = &mc_dev->dma_mask;
> + mc_dev->dev.coherent_dma_mask = mc_dev->dma_mask;
> dev_set_msi_domain(&mc_dev->dev,
> dev_get_msi_domain(&parent_mc_dev->dev));
> }
>

2018-06-21 04:00:30

by Nipun Gupta

[permalink] [raw]
Subject: RE: [PATCH 0/7 v5] Support for fsl-mc bus and its devices in SMMU

Hi Robin/Greg k-h,

Will this patch-set be taken for the next kernel release (and via which tree)?

Thanks,
Nipun

> -----Original Message-----
> From: Nipun Gupta
> Sent: Sunday, May 20, 2018 7:20 PM
> To: [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; Laurentiu Tudor <[email protected]>;
> [email protected]
> Cc: [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected]
> foundation.org; [email protected]; [email protected];
> [email protected]; [email protected]; linux-
> [email protected]; Bharat Bhushan <[email protected]>;
> [email protected]; Leo Li <[email protected]>; Nipun Gupta
> <[email protected]>
> Subject: [PATCH 0/7 v5] Support for fsl-mc bus and its devices in SMMU
>
> This patchset defines IOMMU DT binding for fsl-mc bus and adds
> support in SMMU for fsl-mc bus.
>
> The patch series is based on top of dma-mapping tree (for-next branch):
> http://git.infradead.org/users/hch/dma-mapping.git
>
> These patches
> - Define property 'iommu-map' for fsl-mc bus (patch 1)
> - Integrates the fsl-mc bus with the SMMU using this
> IOMMU binding (patch 2,3,4)
> - Adds the dma configuration support for fsl-mc bus (patch 5, 6)
> - Updates the fsl-mc device node with iommu/dma related changes (patch 7)
>
> Changes in v2:
> - use iommu-map property for fsl-mc bus
> - rebase over patchset https://patchwork.kernel.org/patch/10317337/
> and make corresponding changes for dma configuration of devices on
> fsl-mc bus
>
> Changes in v3:
> - move of_map_rid in drivers/of/address.c
>
> Changes in v4:
> - move of_map_rid in drivers/of/base.c
>
> Changes in v5:
> - break patch 5 in two separate patches (now patch 5/7 and patch 6/7)
> - add changelog text in patch 3/7 and patch 5/7
> - typo fix
>
> Nipun Gupta (7):
> Docs: dt: add fsl-mc iommu-map device-tree binding
> iommu: of: make of_pci_map_rid() available for other devices too
> iommu: support iommu configuration for fsl-mc devices
> iommu: arm-smmu: Add support for the fsl-mc bus
> bus: fsl-mc: support dma configure for devices on fsl-mc bus
> bus: fsl-mc: set coherent dma mask for devices on fsl-mc bus
> arm64: dts: ls208xa: comply with the iommu map binding for fsl_mc
>
> .../devicetree/bindings/misc/fsl,qoriq-mc.txt | 39 ++++++++
> arch/arm64/boot/dts/freescale/fsl-ls208xa.dtsi | 6 +-
> drivers/bus/fsl-mc/fsl-mc-bus.c | 16 +++-
> drivers/iommu/arm-smmu.c | 7 ++
> drivers/iommu/iommu.c | 21 +++++
> drivers/iommu/of_iommu.c | 25 ++++-
> drivers/of/base.c | 102 +++++++++++++++++++++
> drivers/of/irq.c | 5 +-
> drivers/pci/of.c | 101 --------------------
> include/linux/fsl/mc.h | 8 ++
> include/linux/iommu.h | 2 +
> include/linux/of.h | 11 +++
> include/linux/of_pci.h | 10 --
> 13 files changed, 231 insertions(+), 122 deletions(-)
>
> --
> 1.9.1


2018-06-21 11:41:54

by Will Deacon

[permalink] [raw]
Subject: Re: [PATCH 0/7 v5] Support for fsl-mc bus and its devices in SMMU

Hi Nipun,

On Thu, Jun 21, 2018 at 03:59:27AM +0000, Nipun Gupta wrote:
> Will this patch-set be taken for the next kernel release (and via which
> tree)?

I think you need Acks from Robin and Joerg in order for this to be queued.
Robin should be back at the beginning of next month, so there's still time
for 4.19.

Will

2018-07-03 14:41:05

by Robin Murphy

[permalink] [raw]
Subject: Re: [PATCH 1/7 v5] Docs: dt: add fsl-mc iommu-map device-tree binding

On 20/05/18 14:49, Nipun Gupta wrote:
> The existing IOMMU bindings cannot be used to specify the relationship
> between fsl-mc devices and IOMMUs. This patch adds a generic binding for
> mapping fsl-mc devices to IOMMUs, using iommu-map property.
>
> Signed-off-by: Nipun Gupta <[email protected]>
> Reviewed-by: Rob Herring <[email protected]>
> ---
> .../devicetree/bindings/misc/fsl,qoriq-mc.txt | 39 ++++++++++++++++++++++
> 1 file changed, 39 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/misc/fsl,qoriq-mc.txt b/Documentation/devicetree/bindings/misc/fsl,qoriq-mc.txt
> index 6611a7c..8cbed4f 100644
> --- a/Documentation/devicetree/bindings/misc/fsl,qoriq-mc.txt
> +++ b/Documentation/devicetree/bindings/misc/fsl,qoriq-mc.txt
> @@ -9,6 +9,25 @@ blocks that can be used to create functional hardware objects/devices
> such as network interfaces, crypto accelerator instances, L2 switches,
> etc.
>
> +For an overview of the DPAA2 architecture and fsl-mc bus see:
> +drivers/staging/fsl-mc/README.txt

Nit: Looks like that's Documentation/networking/dpaa2/overview.rst now.

> +
> +As described in the above overview, all DPAA2 objects in a DPRC share the
> +same hardware "isolation context" and a 10-bit value called an ICID
> +(isolation context id) is expressed by the hardware to identify
> +the requester.
> +
> +The generic 'iommus' property is insufficient to describe the relationship
> +between ICIDs and IOMMUs, so an iommu-map property is used to define
> +the set of possible ICIDs under a root DPRC and how they map to
> +an IOMMU.
> +
> +For generic IOMMU bindings, see
> +Documentation/devicetree/bindings/iommu/iommu.txt.
> +
> +For arm-smmu binding, see:
> +Documentation/devicetree/bindings/iommu/arm,smmu.txt.
> +
> Required properties:
>
> - compatible
> @@ -88,14 +107,34 @@ Sub-nodes:
> Value type: <phandle>
> Definition: Specifies the phandle to the PHY device node associated
> with the this dpmac.
> +Optional properties:
> +
> +- iommu-map: Maps an ICID to an IOMMU and associated iommu-specifier
> + data.
> +
> + The property is an arbitrary number of tuples of
> + (icid-base,iommu,iommu-base,length).
> +
> + Any ICID i in the interval [icid-base, icid-base + length) is
> + associated with the listed IOMMU, with the iommu-specifier
> + (i - icid-base + iommu-base).
>
> Example:
>
> + smmu: iommu@5000000 {
> + compatible = "arm,mmu-500";
> + #iommu-cells = <2>;

This should be 1 if stream-match-mask is present. Bad example is bad :)

Robin.

> + stream-match-mask = <0x7C00>;
> + ...
> + };
> +
> fsl_mc: fsl-mc@80c000000 {
> compatible = "fsl,qoriq-mc";
> reg = <0x00000008 0x0c000000 0 0x40>, /* MC portal base */
> <0x00000000 0x08340000 0 0x40000>; /* MC control reg */
> msi-parent = <&its>;
> + /* define map for ICIDs 23-64 */
> + iommu-map = <23 &smmu 23 41>;
> #address-cells = <3>;
> #size-cells = <1>;
>
>

2018-07-03 14:42:57

by Robin Murphy

[permalink] [raw]
Subject: Re: [PATCH 2/7 v5] iommu: of: make of_pci_map_rid() available for other devices too

On 20/05/18 14:49, Nipun Gupta wrote:
> iommu-map property is also used by devices with fsl-mc. This
> patch moves the of_pci_map_rid to generic location, so that it
> can be used by other busses too.
>
> 'of_pci_map_rid' is renamed here to 'of_map_rid' and there is no
> functional change done in the API.

Reviewed-by: Robin Murphy <[email protected]>
> Signed-off-by: Nipun Gupta <[email protected]>
> Reviewed-by: Rob Herring <[email protected]>
> Acked-by: Bjorn Helgaas <[email protected]>
> ---
> drivers/iommu/of_iommu.c | 5 +--
> drivers/of/base.c | 102 +++++++++++++++++++++++++++++++++++++++++++++++
> drivers/of/irq.c | 5 +--
> drivers/pci/of.c | 101 ----------------------------------------------
> include/linux/of.h | 11 +++++
> include/linux/of_pci.h | 10 -----
> 6 files changed, 117 insertions(+), 117 deletions(-)
>
> diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
> index 5c36a8b..811e160 100644
> --- a/drivers/iommu/of_iommu.c
> +++ b/drivers/iommu/of_iommu.c
> @@ -149,9 +149,8 @@ static int of_pci_iommu_init(struct pci_dev *pdev, u16 alias, void *data)
> struct of_phandle_args iommu_spec = { .args_count = 1 };
> int err;
>
> - err = of_pci_map_rid(info->np, alias, "iommu-map",
> - "iommu-map-mask", &iommu_spec.np,
> - iommu_spec.args);
> + err = of_map_rid(info->np, alias, "iommu-map", "iommu-map-mask",
> + &iommu_spec.np, iommu_spec.args);
> if (err)
> return err == -ENODEV ? NO_IOMMU : err;
>
> diff --git a/drivers/of/base.c b/drivers/of/base.c
> index 848f549..c7aac81 100644
> --- a/drivers/of/base.c
> +++ b/drivers/of/base.c
> @@ -1995,3 +1995,105 @@ int of_find_last_cache_level(unsigned int cpu)
>
> return cache_level;
> }
> +
> +/**
> + * of_map_rid - Translate a requester ID through a downstream mapping.
> + * @np: root complex device node.
> + * @rid: device requester ID to map.
> + * @map_name: property name of the map to use.
> + * @map_mask_name: optional property name of the mask to use.
> + * @target: optional pointer to a target device node.
> + * @id_out: optional pointer to receive the translated ID.
> + *
> + * Given a device requester ID, look up the appropriate implementation-defined
> + * platform ID and/or the target device which receives transactions on that
> + * ID, as per the "iommu-map" and "msi-map" bindings. Either of @target or
> + * @id_out may be NULL if only the other is required. If @target points to
> + * a non-NULL device node pointer, only entries targeting that node will be
> + * matched; if it points to a NULL value, it will receive the device node of
> + * the first matching target phandle, with a reference held.
> + *
> + * Return: 0 on success or a standard error code on failure.
> + */
> +int of_map_rid(struct device_node *np, u32 rid,
> + const char *map_name, const char *map_mask_name,
> + struct device_node **target, u32 *id_out)
> +{
> + u32 map_mask, masked_rid;
> + int map_len;
> + const __be32 *map = NULL;
> +
> + if (!np || !map_name || (!target && !id_out))
> + return -EINVAL;
> +
> + map = of_get_property(np, map_name, &map_len);
> + if (!map) {
> + if (target)
> + return -ENODEV;
> + /* Otherwise, no map implies no translation */
> + *id_out = rid;
> + return 0;
> + }
> +
> + if (!map_len || map_len % (4 * sizeof(*map))) {
> + pr_err("%pOF: Error: Bad %s length: %d\n", np,
> + map_name, map_len);
> + return -EINVAL;
> + }
> +
> + /* The default is to select all bits. */
> + map_mask = 0xffffffff;
> +
> + /*
> + * Can be overridden by "{iommu,msi}-map-mask" property.
> + * If of_property_read_u32() fails, the default is used.
> + */
> + if (map_mask_name)
> + of_property_read_u32(np, map_mask_name, &map_mask);
> +
> + masked_rid = map_mask & rid;
> + for ( ; map_len > 0; map_len -= 4 * sizeof(*map), map += 4) {
> + struct device_node *phandle_node;
> + u32 rid_base = be32_to_cpup(map + 0);
> + u32 phandle = be32_to_cpup(map + 1);
> + u32 out_base = be32_to_cpup(map + 2);
> + u32 rid_len = be32_to_cpup(map + 3);
> +
> + if (rid_base & ~map_mask) {
> + pr_err("%pOF: Invalid %s translation - %s-mask (0x%x) ignores rid-base (0x%x)\n",
> + np, map_name, map_name,
> + map_mask, rid_base);
> + return -EFAULT;
> + }
> +
> + if (masked_rid < rid_base || masked_rid >= rid_base + rid_len)
> + continue;
> +
> + phandle_node = of_find_node_by_phandle(phandle);
> + if (!phandle_node)
> + return -ENODEV;
> +
> + if (target) {
> + if (*target)
> + of_node_put(phandle_node);
> + else
> + *target = phandle_node;
> +
> + if (*target != phandle_node)
> + continue;
> + }
> +
> + if (id_out)
> + *id_out = masked_rid - rid_base + out_base;
> +
> + pr_debug("%pOF: %s, using mask %08x, rid-base: %08x, out-base: %08x, length: %08x, rid: %08x -> %08x\n",
> + np, map_name, map_mask, rid_base, out_base,
> + rid_len, rid, masked_rid - rid_base + out_base);
> + return 0;
> + }
> +
> + pr_err("%pOF: Invalid %s translation - no match for rid 0x%x on %pOF\n",
> + np, map_name, rid, target && *target ? *target : NULL);
> + return -EFAULT;
> +}
> +EXPORT_SYMBOL_GPL(of_map_rid);
> diff --git a/drivers/of/irq.c b/drivers/of/irq.c
> index 02ad93a..e1f6f39 100644
> --- a/drivers/of/irq.c
> +++ b/drivers/of/irq.c
> @@ -22,7 +22,6 @@
> #include <linux/module.h>
> #include <linux/of.h>
> #include <linux/of_irq.h>
> -#include <linux/of_pci.h>
> #include <linux/string.h>
> #include <linux/slab.h>
>
> @@ -588,8 +587,8 @@ static u32 __of_msi_map_rid(struct device *dev, struct device_node **np,
> * "msi-map" property.
> */
> for (parent_dev = dev; parent_dev; parent_dev = parent_dev->parent)
> - if (!of_pci_map_rid(parent_dev->of_node, rid_in, "msi-map",
> - "msi-map-mask", np, &rid_out))
> + if (!of_map_rid(parent_dev->of_node, rid_in, "msi-map",
> + "msi-map-mask", np, &rid_out))
> break;
> return rid_out;
> }
> diff --git a/drivers/pci/of.c b/drivers/pci/of.c
> index a28355c..d2cebbe 100644
> --- a/drivers/pci/of.c
> +++ b/drivers/pci/of.c
> @@ -362,107 +362,6 @@ int of_pci_get_host_bridge_resources(struct device_node *dev,
> EXPORT_SYMBOL_GPL(of_pci_get_host_bridge_resources);
> #endif /* CONFIG_OF_ADDRESS */
>
> -/**
> - * of_pci_map_rid - Translate a requester ID through a downstream mapping.
> - * @np: root complex device node.
> - * @rid: PCI requester ID to map.
> - * @map_name: property name of the map to use.
> - * @map_mask_name: optional property name of the mask to use.
> - * @target: optional pointer to a target device node.
> - * @id_out: optional pointer to receive the translated ID.
> - *
> - * Given a PCI requester ID, look up the appropriate implementation-defined
> - * platform ID and/or the target device which receives transactions on that
> - * ID, as per the "iommu-map" and "msi-map" bindings. Either of @target or
> - * @id_out may be NULL if only the other is required. If @target points to
> - * a non-NULL device node pointer, only entries targeting that node will be
> - * matched; if it points to a NULL value, it will receive the device node of
> - * the first matching target phandle, with a reference held.
> - *
> - * Return: 0 on success or a standard error code on failure.
> - */
> -int of_pci_map_rid(struct device_node *np, u32 rid,
> - const char *map_name, const char *map_mask_name,
> - struct device_node **target, u32 *id_out)
> -{
> - u32 map_mask, masked_rid;
> - int map_len;
> - const __be32 *map = NULL;
> -
> - if (!np || !map_name || (!target && !id_out))
> - return -EINVAL;
> -
> - map = of_get_property(np, map_name, &map_len);
> - if (!map) {
> - if (target)
> - return -ENODEV;
> - /* Otherwise, no map implies no translation */
> - *id_out = rid;
> - return 0;
> - }
> -
> - if (!map_len || map_len % (4 * sizeof(*map))) {
> - pr_err("%pOF: Error: Bad %s length: %d\n", np,
> - map_name, map_len);
> - return -EINVAL;
> - }
> -
> - /* The default is to select all bits. */
> - map_mask = 0xffffffff;
> -
> - /*
> - * Can be overridden by "{iommu,msi}-map-mask" property.
> - * If of_property_read_u32() fails, the default is used.
> - */
> - if (map_mask_name)
> - of_property_read_u32(np, map_mask_name, &map_mask);
> -
> - masked_rid = map_mask & rid;
> - for ( ; map_len > 0; map_len -= 4 * sizeof(*map), map += 4) {
> - struct device_node *phandle_node;
> - u32 rid_base = be32_to_cpup(map + 0);
> - u32 phandle = be32_to_cpup(map + 1);
> - u32 out_base = be32_to_cpup(map + 2);
> - u32 rid_len = be32_to_cpup(map + 3);
> -
> - if (rid_base & ~map_mask) {
> - pr_err("%pOF: Invalid %s translation - %s-mask (0x%x) ignores rid-base (0x%x)\n",
> - np, map_name, map_name,
> - map_mask, rid_base);
> - return -EFAULT;
> - }
> -
> - if (masked_rid < rid_base || masked_rid >= rid_base + rid_len)
> - continue;
> -
> - phandle_node = of_find_node_by_phandle(phandle);
> - if (!phandle_node)
> - return -ENODEV;
> -
> - if (target) {
> - if (*target)
> - of_node_put(phandle_node);
> - else
> - *target = phandle_node;
> -
> - if (*target != phandle_node)
> - continue;
> - }
> -
> - if (id_out)
> - *id_out = masked_rid - rid_base + out_base;
> -
> - pr_debug("%pOF: %s, using mask %08x, rid-base: %08x, out-base: %08x, length: %08x, rid: %08x -> %08x\n",
> - np, map_name, map_mask, rid_base, out_base,
> - rid_len, rid, masked_rid - rid_base + out_base);
> - return 0;
> - }
> -
> - pr_err("%pOF: Invalid %s translation - no match for rid 0x%x on %pOF\n",
> - np, map_name, rid, target && *target ? *target : NULL);
> - return -EFAULT;
> -}
> -
> #if IS_ENABLED(CONFIG_OF_IRQ)
> /**
> * of_irq_parse_pci - Resolve the interrupt for a PCI device
> diff --git a/include/linux/of.h b/include/linux/of.h
> index 4d25e4f..f4251c3 100644
> --- a/include/linux/of.h
> +++ b/include/linux/of.h
> @@ -545,6 +545,10 @@ const __be32 *of_prop_next_u32(struct property *prop, const __be32 *cur,
>
> extern int of_cpu_node_to_id(struct device_node *np);
>
> +int of_map_rid(struct device_node *np, u32 rid,
> + const char *map_name, const char *map_mask_name,
> + struct device_node **target, u32 *id_out);
> +
> #else /* CONFIG_OF */
>
> static inline void of_core_init(void)
> @@ -931,6 +935,13 @@ static inline int of_cpu_node_to_id(struct device_node *np)
> return -ENODEV;
> }
>
> +static inline int of_map_rid(struct device_node *np, u32 rid,
> + const char *map_name, const char *map_mask_name,
> + struct device_node **target, u32 *id_out)
> +{
> + return -EINVAL;
> +}
> +
> #define of_match_ptr(_ptr) NULL
> #define of_match_node(_matches, _node) NULL
> #endif /* CONFIG_OF */
> diff --git a/include/linux/of_pci.h b/include/linux/of_pci.h
> index 091033a..a23b44a 100644
> --- a/include/linux/of_pci.h
> +++ b/include/linux/of_pci.h
> @@ -17,9 +17,6 @@ struct device_node *of_pci_find_child_device(struct device_node *parent,
> int of_get_pci_domain_nr(struct device_node *node);
> int of_pci_get_max_link_speed(struct device_node *node);
> void of_pci_check_probe_only(void);
> -int of_pci_map_rid(struct device_node *np, u32 rid,
> - const char *map_name, const char *map_mask_name,
> - struct device_node **target, u32 *id_out);
> #else
> static inline struct device_node *of_pci_find_child_device(struct device_node *parent,
> unsigned int devfn)
> @@ -44,13 +41,6 @@ static inline int of_pci_get_devfn(struct device_node *np)
> return -1;
> }
>
> -static inline int of_pci_map_rid(struct device_node *np, u32 rid,
> - const char *map_name, const char *map_mask_name,
> - struct device_node **target, u32 *id_out)
> -{
> - return -EINVAL;
> -}
> -
> static inline int
> of_pci_get_max_link_speed(struct device_node *node)
> {
>

2018-07-03 15:31:52

by Robin Murphy

[permalink] [raw]
Subject: Re: [PATCH 3/7 v5] iommu: support iommu configuration for fsl-mc devices

On 20/05/18 14:49, Nipun Gupta wrote:
> With of_pci_map_rid available for all the busses, use the function
> for configuration of devices on fsl-mc bus

FWIW I had a quick hack at factoring out the commonality with
of_pci_iommu_init(), at which point I reckon this change is easier to
follow as-is for the moment, so:

Reviewed-by: Robin Murphy <[email protected]>

> Signed-off-by: Nipun Gupta <[email protected]>
> ---
> drivers/iommu/of_iommu.c | 20 ++++++++++++++++++++
> 1 file changed, 20 insertions(+)
>
> diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
> index 811e160..284474d 100644
> --- a/drivers/iommu/of_iommu.c
> +++ b/drivers/iommu/of_iommu.c
> @@ -24,6 +24,7 @@
> #include <linux/of_iommu.h>
> #include <linux/of_pci.h>
> #include <linux/slab.h>
> +#include <linux/fsl/mc.h>
>
> #define NO_IOMMU 1
>
> @@ -159,6 +160,23 @@ static int of_pci_iommu_init(struct pci_dev *pdev, u16 alias, void *data)
> return err;
> }
>
> +static int of_fsl_mc_iommu_init(struct fsl_mc_device *mc_dev,
> + struct device_node *master_np)
> +{
> + struct of_phandle_args iommu_spec = { .args_count = 1 };
> + int err;
> +
> + err = of_map_rid(master_np, mc_dev->icid, "iommu-map",
> + "iommu-map-mask", &iommu_spec.np,
> + iommu_spec.args);
> + if (err)
> + return err == -ENODEV ? NO_IOMMU : err;
> +
> + err = of_iommu_xlate(&mc_dev->dev, &iommu_spec);
> + of_node_put(iommu_spec.np);
> + return err;
> +}
> +
> const struct iommu_ops *of_iommu_configure(struct device *dev,
> struct device_node *master_np)
> {
> @@ -190,6 +208,8 @@ const struct iommu_ops *of_iommu_configure(struct device *dev,
>
> err = pci_for_each_dma_alias(to_pci_dev(dev),
> of_pci_iommu_init, &info);
> + } else if (dev_is_fsl_mc(dev)) {
> + err = of_fsl_mc_iommu_init(to_fsl_mc_device(dev), master_np);
> } else {
> struct of_phandle_args iommu_spec;
> int idx = 0;
>

2018-07-03 16:03:52

by Robin Murphy

[permalink] [raw]
Subject: Re: [PATCH 4/7 v5] iommu: arm-smmu: Add support for the fsl-mc bus

On 20/05/18 14:49, Nipun Gupta wrote:
> Implement bus specific support for the fsl-mc bus including
> registering arm_smmu_ops and bus specific device add operations.
>
> Signed-off-by: Nipun Gupta <[email protected]>
> ---
> drivers/iommu/arm-smmu.c | 7 +++++++
> drivers/iommu/iommu.c | 21 +++++++++++++++++++++
> include/linux/fsl/mc.h | 8 ++++++++
> include/linux/iommu.h | 2 ++
> 4 files changed, 38 insertions(+)
>
> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
> index 69e7c60..e1d5090 100644
> --- a/drivers/iommu/arm-smmu.c
> +++ b/drivers/iommu/arm-smmu.c
> @@ -52,6 +52,7 @@
> #include <linux/spinlock.h>
>
> #include <linux/amba/bus.h>
> +#include <linux/fsl/mc.h>
>
> #include "io-pgtable.h"
> #include "arm-smmu-regs.h"
> @@ -1459,6 +1460,8 @@ static struct iommu_group *arm_smmu_device_group(struct device *dev)
>
> if (dev_is_pci(dev))
> group = pci_device_group(dev);
> + else if (dev_is_fsl_mc(dev))
> + group = fsl_mc_device_group(dev);
> else
> group = generic_device_group(dev);
>
> @@ -2037,6 +2040,10 @@ static void arm_smmu_bus_init(void)
> bus_set_iommu(&pci_bus_type, &arm_smmu_ops);
> }
> #endif
> +#ifdef CONFIG_FSL_MC_BUS
> + if (!iommu_present(&fsl_mc_bus_type))
> + bus_set_iommu(&fsl_mc_bus_type, &arm_smmu_ops);
> +#endif
> }
>
> static int arm_smmu_device_probe(struct platform_device *pdev)
> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> index d2aa2320..6d4ce35 100644
> --- a/drivers/iommu/iommu.c
> +++ b/drivers/iommu/iommu.c
> @@ -32,6 +32,7 @@
> #include <linux/pci.h>
> #include <linux/bitops.h>
> #include <linux/property.h>
> +#include <linux/fsl/mc.h>
> #include <trace/events/iommu.h>
>
> static struct kset *iommu_group_kset;
> @@ -987,6 +988,26 @@ struct iommu_group *pci_device_group(struct device *dev)
> return iommu_group_alloc();
> }
>
> +/* Get the IOMMU group for device on fsl-mc bus */
> +struct iommu_group *fsl_mc_device_group(struct device *dev)
> +{
> + struct device *cont_dev = fsl_mc_cont_dev(dev);
> + struct iommu_group *group;
> +
> + /* Container device is responsible for creating the iommu group */
> + if (fsl_mc_is_cont_dev(dev)) {

Why duplicate what fsl_mc_cont_dev() has already figured out?

AFAICS the overall operation here boils down to something like:

cont_dev = fsl_mc_cont_dev(dev);
group = iommu_group_get(cont_dev);
if (!group)
group = iommu_group_alloc();
return group;

> + group = iommu_group_alloc();
> + if (IS_ERR(group))
> + return NULL;

iommu_group_get_for_dev() expects a PTR_ERR, so I don't think munging
the return here is the right thing to do.

> + } else {
> + get_device(cont_dev);

If races are a concern, isn't this a bit late? Maybe in that case you
want {get,put}_fsl_mc_cont_dev() routines instead of the simple macro
below. But on the other hand if dev already has cont_dev as its parent,
wouldn't the reference taken in device_add() be sufficient to prevent it
from vanishing unexpectedly in this timescale?

Robin.

> + group = iommu_group_get(cont_dev);
> + put_device(cont_dev);
> + }
> +
> + return group;
> +}
> +
> /**
> * iommu_group_get_for_dev - Find or create the IOMMU group for a device
> * @dev: target device
> diff --git a/include/linux/fsl/mc.h b/include/linux/fsl/mc.h
> index f27cb14..dddaca1 100644
> --- a/include/linux/fsl/mc.h
> +++ b/include/linux/fsl/mc.h
> @@ -351,6 +351,14 @@ struct fsl_mc_io {
> #define dev_is_fsl_mc(_dev) (0)
> #endif
>
> +/* Macro to check if a device is a container device */
> +#define fsl_mc_is_cont_dev(_dev) (to_fsl_mc_device(_dev)->flags & \
> + FSL_MC_IS_DPRC)
> +
> +/* Macro to get the container device of a MC device */
> +#define fsl_mc_cont_dev(_dev) (fsl_mc_is_cont_dev(_dev) ? \
> + (_dev) : (_dev)->parent)
> +
> /*
> * module_fsl_mc_driver() - Helper macro for drivers that don't do
> * anything special in module init/exit. This eliminates a lot of
> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
> index 19938ee..2981200 100644
> --- a/include/linux/iommu.h
> +++ b/include/linux/iommu.h
> @@ -389,6 +389,8 @@ static inline size_t iommu_map_sg(struct iommu_domain *domain,
> extern struct iommu_group *pci_device_group(struct device *dev);
> /* Generic device grouping function */
> extern struct iommu_group *generic_device_group(struct device *dev);
> +/* FSL-MC device grouping function */
> +struct iommu_group *fsl_mc_device_group(struct device *dev);
>
> /**
> * struct iommu_fwspec - per-device IOMMU instance data
>

2018-07-03 16:15:32

by Robin Murphy

[permalink] [raw]
Subject: Re: [PATCH 5/7 v5] bus: fsl-mc: support dma configure for devices on fsl-mc bus

On 20/05/18 14:49, Nipun Gupta wrote:
> This patch adds support of dma configuration for devices on fsl-mc
> bus using 'dma_configure' callback for busses. Also, directly calling
> arch_setup_dma_ops is removed from the fsl-mc bus.

Looks like this is the final arch_setup_dma_ops offender, yay!

> Signed-off-by: Nipun Gupta <[email protected]>
> Reviewed-by: Laurentiu Tudor <[email protected]>
> ---
> drivers/bus/fsl-mc/fsl-mc-bus.c | 15 +++++++++++----
> 1 file changed, 11 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/bus/fsl-mc/fsl-mc-bus.c b/drivers/bus/fsl-mc/fsl-mc-bus.c
> index 5d8266c..fa43c7d 100644
> --- a/drivers/bus/fsl-mc/fsl-mc-bus.c
> +++ b/drivers/bus/fsl-mc/fsl-mc-bus.c
> @@ -127,6 +127,16 @@ static int fsl_mc_bus_uevent(struct device *dev, struct kobj_uevent_env *env)
> return 0;
> }
>
> +static int fsl_mc_dma_configure(struct device *dev)
> +{
> + struct device *dma_dev = dev;
> +
> + while (dev_is_fsl_mc(dma_dev))
> + dma_dev = dma_dev->parent;
> +
> + return of_dma_configure(dev, dma_dev->of_node, 0);
> +}
> +
> static ssize_t modalias_show(struct device *dev, struct device_attribute *attr,
> char *buf)
> {
> @@ -148,6 +158,7 @@ struct bus_type fsl_mc_bus_type = {
> .name = "fsl-mc",
> .match = fsl_mc_bus_match,
> .uevent = fsl_mc_bus_uevent,
> + .dma_configure = fsl_mc_dma_configure,
> .dev_groups = fsl_mc_dev_groups,
> };
> EXPORT_SYMBOL_GPL(fsl_mc_bus_type);
> @@ -633,10 +644,6 @@ int fsl_mc_device_add(struct fsl_mc_obj_desc *obj_desc,
> goto error_cleanup_dev;
> }
>
> - /* Objects are coherent, unless 'no shareability' flag set. */
> - if (!(obj_desc->flags & FSL_MC_OBJ_FLAG_NO_MEM_SHAREABILITY))

Although it seems we do end up without any handling of this
"non-coherent object behind coherent MC" case, and I'm not sure how
easily that could be accommodated by generic code... :/ How important is
the quirk?

Robin.

> - arch_setup_dma_ops(&mc_dev->dev, 0, 0, NULL, true);
> -
> /*
> * The device-specific probe callback will get invoked by device_add()
> */
>

2018-07-03 16:17:12

by Robin Murphy

[permalink] [raw]
Subject: Re: [PATCH 6/7 v5] bus: fsl-mc: set coherent dma mask for devices on fsl-mc bus

On 20/05/18 14:49, Nipun Gupta wrote:
> of_dma_configure() API expects coherent_dma_mask to be correctly
> set in the devices. This patch does the needful.

Reviewed-by: Robin Murphy <[email protected]>

> Signed-off-by: Nipun Gupta <[email protected]>
> ---
> drivers/bus/fsl-mc/fsl-mc-bus.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/bus/fsl-mc/fsl-mc-bus.c b/drivers/bus/fsl-mc/fsl-mc-bus.c
> index fa43c7d..624828b 100644
> --- a/drivers/bus/fsl-mc/fsl-mc-bus.c
> +++ b/drivers/bus/fsl-mc/fsl-mc-bus.c
> @@ -627,6 +627,7 @@ int fsl_mc_device_add(struct fsl_mc_obj_desc *obj_desc,
> mc_dev->icid = parent_mc_dev->icid;
> mc_dev->dma_mask = FSL_MC_DEFAULT_DMA_MASK;
> mc_dev->dev.dma_mask = &mc_dev->dma_mask;
> + mc_dev->dev.coherent_dma_mask = mc_dev->dma_mask;
> dev_set_msi_domain(&mc_dev->dev,
> dev_get_msi_domain(&parent_mc_dev->dev));
> }
>

2018-07-03 16:36:45

by Robin Murphy

[permalink] [raw]
Subject: Re: [PATCH 7/7 v5] arm64: dts: ls208xa: comply with the iommu map binding for fsl_mc

On 20/05/18 14:49, Nipun Gupta wrote:
> fsl-mc bus support the new iommu-map property. Comply to this binding
> for fsl_mc bus.
>
> Signed-off-by: Nipun Gupta <[email protected]>
> Reviewed-by: Laurentiu Tudor <[email protected]>
> ---
> arch/arm64/boot/dts/freescale/fsl-ls208xa.dtsi | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm64/boot/dts/freescale/fsl-ls208xa.dtsi b/arch/arm64/boot/dts/freescale/fsl-ls208xa.dtsi
> index 137ef4d..6010505 100644
> --- a/arch/arm64/boot/dts/freescale/fsl-ls208xa.dtsi
> +++ b/arch/arm64/boot/dts/freescale/fsl-ls208xa.dtsi
> @@ -184,6 +184,7 @@
> #address-cells = <2>;
> #size-cells = <2>;
> ranges;
> + dma-ranges = <0x0 0x0 0x0 0x0 0x10000 0x00000000>;
>
> clockgen: clocking@1300000 {
> compatible = "fsl,ls2080a-clockgen";
> @@ -357,6 +358,8 @@
> reg = <0x00000008 0x0c000000 0 0x40>, /* MC portal base */
> <0x00000000 0x08340000 0 0x40000>; /* MC control reg */
> msi-parent = <&its>;
> + iommu-map = <0 &smmu 0 0>; /* This is fixed-up by u-boot */
> + dma-coherent;
> #address-cells = <3>;
> #size-cells = <1>;
>
> @@ -460,6 +463,8 @@
> compatible = "arm,mmu-500";
> reg = <0 0x5000000 0 0x800000>;
> #global-interrupts = <12>;
> + #iommu-cells = <1>;
> + stream-match-mask = <0x7C00>;
> interrupts = <0 13 4>, /* global secure fault */
> <0 14 4>, /* combined secure interrupt */
> <0 15 4>, /* global non-secure fault */
> @@ -502,7 +507,6 @@
> <0 204 4>, <0 205 4>,
> <0 206 4>, <0 207 4>,
> <0 208 4>, <0 209 4>;
> - mmu-masters = <&fsl_mc 0x300 0>;

Since we're in here, is the SMMU itself also coherent? If it is, you
probably want to say so and avoid the overhead of pointlessly cleaning
cache lines on every page table update.

Robin.

> };
>
> dspi: dspi@2100000 {
>

2018-07-06 11:15:32

by Joerg Roedel

[permalink] [raw]
Subject: Re: [PATCH 0/7 v5] Support for fsl-mc bus and its devices in SMMU

Hi Nipun,

On Thu, Jun 21, 2018 at 03:59:27AM +0000, Nipun Gupta wrote:
> Will this patch-set be taken for the next kernel release (and via which tree)?

I can take this through IOMMU tree if nobody objects. Please work out
the last remaining comment on patch 7 with Robin and then re-send with
all Acks and Reviewed-by: tags included.

Oh, and please use 'iommu/of:' prefix instead of 'iommu: of:' and follow
that sheme for the other patches/prefixes as well.

Thanks,

Joerg


2018-07-06 11:21:27

by Nipun Gupta

[permalink] [raw]
Subject: RE: [PATCH 0/7 v5] Support for fsl-mc bus and its devices in SMMU


Thanks Joro,

I will be sending it by mid of next week.

Regards,
Nipun

> -----Original Message-----
> From: [email protected] [mailto:[email protected]]
> Sent: Friday, July 6, 2018 4:43 PM
> To: Nipun Gupta <[email protected]>
> Cc: [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected]
> foundation.org; [email protected]; [email protected];
> [email protected]; [email protected]; linux-
> [email protected]; Bharat Bhushan <[email protected]>;
> [email protected]; Leo Li <[email protected]>
> Subject: Re: [PATCH 0/7 v5] Support for fsl-mc bus and its devices in SMMU
>
> Hi Nipun,
>
> On Thu, Jun 21, 2018 at 03:59:27AM +0000, Nipun Gupta wrote:
> > Will this patch-set be taken for the next kernel release (and via which tree)?
>
> I can take this through IOMMU tree if nobody objects. Please work out
> the last remaining comment on patch 7 with Robin and then re-send with
> all Acks and Reviewed-by: tags included.
>
> Oh, and please use 'iommu/of:' prefix instead of 'iommu: of:' and follow
> that sheme for the other patches/prefixes as well.
>
> Thanks,
>
> Joerg


2018-07-06 12:13:16

by Nipun Gupta

[permalink] [raw]
Subject: RE: [PATCH 1/7 v5] Docs: dt: add fsl-mc iommu-map device-tree binding



> -----Original Message-----
> From: Robin Murphy [mailto:[email protected]]
> Sent: Tuesday, July 3, 2018 8:10 PM
> To: Nipun Gupta <[email protected]>; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; Laurentiu Tudor
> <[email protected]>; [email protected]
> Cc: [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected]
> foundation.org; [email protected]; [email protected];
> [email protected]; [email protected]; linux-
> [email protected]; Bharat Bhushan <[email protected]>;
> [email protected]; Leo Li <[email protected]>
> Subject: Re: [PATCH 1/7 v5] Docs: dt: add fsl-mc iommu-map device-tree
> binding
>
> On 20/05/18 14:49, Nipun Gupta wrote:
> > The existing IOMMU bindings cannot be used to specify the relationship
> > between fsl-mc devices and IOMMUs. This patch adds a generic binding for
> > mapping fsl-mc devices to IOMMUs, using iommu-map property.
> >
> > Signed-off-by: Nipun Gupta <[email protected]>
> > Reviewed-by: Rob Herring <[email protected]>
> > ---
> > .../devicetree/bindings/misc/fsl,qoriq-mc.txt | 39
> ++++++++++++++++++++++
> > 1 file changed, 39 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/misc/fsl,qoriq-mc.txt
> b/Documentation/devicetree/bindings/misc/fsl,qoriq-mc.txt
> > index 6611a7c..8cbed4f 100644
> > --- a/Documentation/devicetree/bindings/misc/fsl,qoriq-mc.txt
> > +++ b/Documentation/devicetree/bindings/misc/fsl,qoriq-mc.txt
> > @@ -9,6 +9,25 @@ blocks that can be used to create functional hardware
> objects/devices
> > such as network interfaces, crypto accelerator instances, L2 switches,
> > etc.
> >
> > +For an overview of the DPAA2 architecture and fsl-mc bus see:
> > +drivers/staging/fsl-mc/README.txt
>
> Nit: Looks like that's Documentation/networking/dpaa2/overview.rst now.
>
> > +
> > +As described in the above overview, all DPAA2 objects in a DPRC share the
> > +same hardware "isolation context" and a 10-bit value called an ICID
> > +(isolation context id) is expressed by the hardware to identify
> > +the requester.
> > +
> > +The generic 'iommus' property is insufficient to describe the relationship
> > +between ICIDs and IOMMUs, so an iommu-map property is used to
> define
> > +the set of possible ICIDs under a root DPRC and how they map to
> > +an IOMMU.
> > +
> > +For generic IOMMU bindings, see
> > +Documentation/devicetree/bindings/iommu/iommu.txt.
> > +
> > +For arm-smmu binding, see:
> > +Documentation/devicetree/bindings/iommu/arm,smmu.txt.
> > +
> > Required properties:
> >
> > - compatible
> > @@ -88,14 +107,34 @@ Sub-nodes:
> > Value type: <phandle>
> > Definition: Specifies the phandle to the PHY device node associated
> > with the this dpmac.
> > +Optional properties:
> > +
> > +- iommu-map: Maps an ICID to an IOMMU and associated iommu-
> specifier
> > + data.
> > +
> > + The property is an arbitrary number of tuples of
> > + (icid-base,iommu,iommu-base,length).
> > +
> > + Any ICID i in the interval [icid-base, icid-base + length) is
> > + associated with the listed IOMMU, with the iommu-specifier
> > + (i - icid-base + iommu-base).
> >
> > Example:
> >
> > + smmu: iommu@5000000 {
> > + compatible = "arm,mmu-500";
> > + #iommu-cells = <2>;
>
> This should be 1 if stream-match-mask is present. Bad example is bad :)

Agree :). Ill update it.

>
> Robin.
>
> > + stream-match-mask = <0x7C00>;
> > + ...
> > + };
> > +
> > fsl_mc: fsl-mc@80c000000 {
> > compatible = "fsl,qoriq-mc";
> > reg = <0x00000008 0x0c000000 0 0x40>, /* MC portal base */
> > <0x00000000 0x08340000 0 0x40000>; /* MC control reg */
> > msi-parent = <&its>;
> > + /* define map for ICIDs 23-64 */
> > + iommu-map = <23 &smmu 23 41>;
> > #address-cells = <3>;
> > #size-cells = <1>;
> >
> >

2018-07-06 12:18:19

by Nipun Gupta

[permalink] [raw]
Subject: RE: [PATCH 5/7 v5] bus: fsl-mc: support dma configure for devices on fsl-mc bus



> -----Original Message-----
> From: Robin Murphy [mailto:[email protected]]
> Sent: Tuesday, July 3, 2018 9:44 PM
> To: Nipun Gupta <[email protected]>; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; Laurentiu Tudor
> <[email protected]>; [email protected]
> Cc: [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected]
> foundation.org; [email protected]; [email protected];
> [email protected]; [email protected]; linux-
> [email protected]; Bharat Bhushan <[email protected]>;
> [email protected]; Leo Li <[email protected]>
> Subject: Re: [PATCH 5/7 v5] bus: fsl-mc: support dma configure for devices
> on fsl-mc bus
>
> On 20/05/18 14:49, Nipun Gupta wrote:
> > This patch adds support of dma configuration for devices on fsl-mc
> > bus using 'dma_configure' callback for busses. Also, directly calling
> > arch_setup_dma_ops is removed from the fsl-mc bus.
>
> Looks like this is the final arch_setup_dma_ops offender, yay!
>
> > Signed-off-by: Nipun Gupta <[email protected]>
> > Reviewed-by: Laurentiu Tudor <[email protected]>
> > ---
> > drivers/bus/fsl-mc/fsl-mc-bus.c | 15 +++++++++++----
> > 1 file changed, 11 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/bus/fsl-mc/fsl-mc-bus.c b/drivers/bus/fsl-mc/fsl-mc-
> bus.c
> > index 5d8266c..fa43c7d 100644
> > --- a/drivers/bus/fsl-mc/fsl-mc-bus.c
> > +++ b/drivers/bus/fsl-mc/fsl-mc-bus.c
> > @@ -127,6 +127,16 @@ static int fsl_mc_bus_uevent(struct device *dev,
> struct kobj_uevent_env *env)
> > return 0;
> > }
> >
> > +static int fsl_mc_dma_configure(struct device *dev)
> > +{
> > + struct device *dma_dev = dev;
> > +
> > + while (dev_is_fsl_mc(dma_dev))
> > + dma_dev = dma_dev->parent;
> > +
> > + return of_dma_configure(dev, dma_dev->of_node, 0);
> > +}
> > +
> > static ssize_t modalias_show(struct device *dev, struct device_attribute
> *attr,
> > char *buf)
> > {
> > @@ -148,6 +158,7 @@ struct bus_type fsl_mc_bus_type = {
> > .name = "fsl-mc",
> > .match = fsl_mc_bus_match,
> > .uevent = fsl_mc_bus_uevent,
> > + .dma_configure = fsl_mc_dma_configure,
> > .dev_groups = fsl_mc_dev_groups,
> > };
> > EXPORT_SYMBOL_GPL(fsl_mc_bus_type);
> > @@ -633,10 +644,6 @@ int fsl_mc_device_add(struct fsl_mc_obj_desc
> *obj_desc,
> > goto error_cleanup_dev;
> > }
> >
> > - /* Objects are coherent, unless 'no shareability' flag set. */
> > - if (!(obj_desc->flags &
> FSL_MC_OBJ_FLAG_NO_MEM_SHAREABILITY))
>
> Although it seems we do end up without any handling of this
> "non-coherent object behind coherent MC" case, and I'm not sure how
> easily that could be accommodated by generic code... :/ How important is
> the quirk?

We have all the devices as coherent in our SoC's now :) So this is fine.
We have internally discussed it.

Regards,
Nipun

>
> Robin.
>
> > - arch_setup_dma_ops(&mc_dev->dev, 0, 0, NULL, true);
> > -
> > /*
> > * The device-specific probe callback will get invoked by device_add()
> > */
> >

2018-07-06 12:19:52

by Nipun Gupta

[permalink] [raw]
Subject: RE: [PATCH 7/7 v5] arm64: dts: ls208xa: comply with the iommu map binding for fsl_mc



> -----Original Message-----
> From: Robin Murphy [mailto:[email protected]]
> Sent: Tuesday, July 3, 2018 10:06 PM
> To: Nipun Gupta <[email protected]>; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; Laurentiu Tudor
> <[email protected]>; [email protected]
> Cc: [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected]
> foundation.org; [email protected]; [email protected];
> [email protected]; [email protected]; linux-
> [email protected]; Bharat Bhushan <[email protected]>;
> [email protected]; Leo Li <[email protected]>
> Subject: Re: [PATCH 7/7 v5] arm64: dts: ls208xa: comply with the iommu map
> binding for fsl_mc
>
> On 20/05/18 14:49, Nipun Gupta wrote:
> > fsl-mc bus support the new iommu-map property. Comply to this binding
> > for fsl_mc bus.
> >
> > Signed-off-by: Nipun Gupta <[email protected]>
> > Reviewed-by: Laurentiu Tudor <[email protected]>
> > ---
> > arch/arm64/boot/dts/freescale/fsl-ls208xa.dtsi | 6 +++++-
> > 1 file changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/arm64/boot/dts/freescale/fsl-ls208xa.dtsi
> b/arch/arm64/boot/dts/freescale/fsl-ls208xa.dtsi
> > index 137ef4d..6010505 100644
> > --- a/arch/arm64/boot/dts/freescale/fsl-ls208xa.dtsi
> > +++ b/arch/arm64/boot/dts/freescale/fsl-ls208xa.dtsi
> > @@ -184,6 +184,7 @@
> > #address-cells = <2>;
> > #size-cells = <2>;
> > ranges;
> > + dma-ranges = <0x0 0x0 0x0 0x0 0x10000 0x00000000>;
> >
> > clockgen: clocking@1300000 {
> > compatible = "fsl,ls2080a-clockgen";
> > @@ -357,6 +358,8 @@
> > reg = <0x00000008 0x0c000000 0 0x40>, /* MC portal
> base */
> > <0x00000000 0x08340000 0 0x40000>; /* MC
> control reg */
> > msi-parent = <&its>;
> > + iommu-map = <0 &smmu 0 0>; /* This is fixed-up by
> u-boot */
> > + dma-coherent;
> > #address-cells = <3>;
> > #size-cells = <1>;
> >
> > @@ -460,6 +463,8 @@
> > compatible = "arm,mmu-500";
> > reg = <0 0x5000000 0 0x800000>;
> > #global-interrupts = <12>;
> > + #iommu-cells = <1>;
> > + stream-match-mask = <0x7C00>;
> > interrupts = <0 13 4>, /* global secure fault */
> > <0 14 4>, /* combined secure interrupt */
> > <0 15 4>, /* global non-secure fault */
> > @@ -502,7 +507,6 @@
> > <0 204 4>, <0 205 4>,
> > <0 206 4>, <0 207 4>,
> > <0 208 4>, <0 209 4>;
> > - mmu-masters = <&fsl_mc 0x300 0>;
>
> Since we're in here, is the SMMU itself also coherent? If it is, you
> probably want to say so and avoid the overhead of pointlessly cleaning
> cache lines on every page table update.

Yes, dma-coherent property is also required here. I missed it somehow.
Thanks for pointing this.

Regards,
Nipun

>
> Robin.
>
> > };
> >
> > dspi: dspi@2100000 {
> >