Re: [RFC PATCH 1/8] of/device: Allow specifying a custom iommu_spec to of_dma_configure
From: Mikko Perttunen
Date: Tue Feb 16 2021 - 08:21:06 EST
On 2/16/21 2:47 PM, Robin Murphy wrote:
Hi Mikko,
On 2021-02-08 16:38, Mikko Perttunen wrote:
To allow for more customized device tree bindings that point to IOMMUs,
allow manual specification of iommu_spec to of_dma_configure.
The initial use case for this is with Host1x, where the driver manages
a set of device tree-defined IOMMU contexts that are dynamically
allocated to various users. These contexts don't correspond to
platform devices and are instead attached to dummy devices on a custom
software bus.
I'd suggest taking a closer look at the patches that made this
of_dma_configure_id() in the first place, and the corresponding bus code
in fsl-mc. At this level, Host1x sounds effectively identical to DPAA2
in terms of being a bus of logical devices composed from bits of
implicit behind-the-scenes hardware. I mean, compare your series title
to the fact that their identifiers are literally named "Isolation
Context ID" ;)
Please just use the existing mechanisms to describe a mapping between
Host1x context IDs and SMMU Stream IDs, rather than what looks like a
giant hacky mess here.
(This also reminds me I wanted to rip out all the PCI special-cases and
convert pci_dma_configure() over to passing its own IDs too, so thanks
for the memory-jog...)
Thanks Robin, not sure how I missed that the first time :) Maybe because
Host1x doesn't have a concept of its own "IDs" for these per se - the
hardware just uses stream IDs as is. I would need to count the number of
mapped IDs from the iommu-map property and introduce some 0..N range of
IDs at the software level. But maybe that's not too bad if we're able to
use the existing paths and bindings then.
I'll take a look at switching to iommu-map.
Thanks,
Mikko
Robin.
Signed-off-by: Mikko Perttunen <mperttunen@xxxxxxxxxx>
---
drivers/iommu/of_iommu.c | 12 ++++++++----
drivers/of/device.c | 9 +++++----
include/linux/of_device.h | 34 +++++++++++++++++++++++++++-------
include/linux/of_iommu.h | 6 ++++--
4 files changed, 44 insertions(+), 17 deletions(-)
diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
index e505b9130a1c..3fefa6c63863 100644
--- a/drivers/iommu/of_iommu.c
+++ b/drivers/iommu/of_iommu.c
@@ -87,8 +87,7 @@ int of_get_dma_window(struct device_node *dn, const
char *prefix, int index,
}
EXPORT_SYMBOL_GPL(of_get_dma_window);
-static int of_iommu_xlate(struct device *dev,
- struct of_phandle_args *iommu_spec)
+int of_iommu_xlate(struct device *dev, struct of_phandle_args
*iommu_spec)
{
const struct iommu_ops *ops;
struct fwnode_handle *fwnode = &iommu_spec->np->fwnode;
@@ -117,6 +116,7 @@ static int of_iommu_xlate(struct device *dev,
module_put(ops->owner);
return ret;
}
+EXPORT_SYMBOL_GPL(of_iommu_xlate);
static int of_iommu_configure_dev_id(struct device_node *master_np,
struct device *dev,
@@ -177,7 +177,8 @@ static int of_iommu_configure_device(struct
device_node *master_np,
const struct iommu_ops *of_iommu_configure(struct device *dev,
struct device_node *master_np,
- const u32 *id)
+ const u32 *id,
+ struct of_phandle_args *iommu_spec)
{
const struct iommu_ops *ops = NULL;
struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev);
@@ -209,7 +210,10 @@ 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 {
- err = of_iommu_configure_device(master_np, dev, id);
+ if (iommu_spec)
+ err = of_iommu_xlate(dev, iommu_spec);
+ else
+ err = of_iommu_configure_device(master_np, dev, id);
fwspec = dev_iommu_fwspec_get(dev);
if (!err && fwspec)
diff --git a/drivers/of/device.c b/drivers/of/device.c
index aedfaaafd3e7..84ada2110c5b 100644
--- a/drivers/of/device.c
+++ b/drivers/of/device.c
@@ -88,8 +88,9 @@ int of_device_add(struct platform_device *ofdev)
* can use a platform bus notifier and handle BUS_NOTIFY_ADD_DEVICE
events
* to fix up DMA configuration.
*/
-int of_dma_configure_id(struct device *dev, struct device_node *np,
- bool force_dma, const u32 *id)
+int __of_dma_configure(struct device *dev, struct device_node *np,
+ bool force_dma, const u32 *id,
+ struct of_phandle_args *iommu_spec)
{
const struct iommu_ops *iommu;
const struct bus_dma_region *map = NULL;
@@ -170,7 +171,7 @@ int of_dma_configure_id(struct device *dev, struct
device_node *np,
dev_dbg(dev, "device is%sdma coherent\n",
coherent ? " " : " not ");
- iommu = of_iommu_configure(dev, np, id);
+ iommu = of_iommu_configure(dev, np, id, iommu_spec);
if (PTR_ERR(iommu) == -EPROBE_DEFER) {
kfree(map);
return -EPROBE_DEFER;
@@ -184,7 +185,7 @@ int of_dma_configure_id(struct device *dev, struct
device_node *np,
dev->dma_range_map = map;
return 0;
}
-EXPORT_SYMBOL_GPL(of_dma_configure_id);
+EXPORT_SYMBOL_GPL(__of_dma_configure);
int of_device_register(struct platform_device *pdev)
{
diff --git a/include/linux/of_device.h b/include/linux/of_device.h
index 07ca187fc5e4..40cc3e788cb9 100644
--- a/include/linux/of_device.h
+++ b/include/linux/of_device.h
@@ -55,14 +55,27 @@ static inline struct device_node
*of_cpu_device_node_get(int cpu)
return of_node_get(cpu_dev->of_node);
}
-int of_dma_configure_id(struct device *dev,
+int __of_dma_configure(struct device *dev,
struct device_node *np,
- bool force_dma, const u32 *id);
+ bool force_dma, const u32 *id,
+ struct of_phandle_args *iommu_spec);
static inline int of_dma_configure(struct device *dev,
struct device_node *np,
bool force_dma)
{
- return of_dma_configure_id(dev, np, force_dma, NULL);
+ return __of_dma_configure(dev, np, force_dma, NULL, NULL);
+}
+static inline int of_dma_configure_id(struct device *dev,
+ struct device_node *np,
+ bool force_dma, const u32 *id)
+{
+ return __of_dma_configure(dev, np, force_dma, id, NULL);
+}
+static inline int
+of_dma_configure_iommu_spec(struct device *dev, struct device_node *np,
+ bool force_dma, struct of_phandle_args *iommu_spec)
+{
+ return __of_dma_configure(dev, np, force_dma, NULL, iommu_spec);
}
#else /* CONFIG_OF */
@@ -112,18 +125,25 @@ static inline struct device_node
*of_cpu_device_node_get(int cpu)
return NULL;
}
-static inline int of_dma_configure_id(struct device *dev,
+static inline int of_dma_configure(struct device *dev,
struct device_node *np,
bool force_dma)
{
return 0;
}
-static inline int of_dma_configure(struct device *dev,
- struct device_node *np,
- bool force_dma)
+
+static inline int of_dma_configure_id(struct device *dev,
+ struct device_node *np,
+ bool force_dma, const u32 *id)
{
return 0;
}
+
+static inline int
+of_dma_configure_iommu_spec(struct device *dev, struct device_node *np,
+ bool force_dma, struct of_phandle_args *iommu_spec)
+{ return 0;
+}
#endif /* CONFIG_OF */
#endif /* _LINUX_OF_DEVICE_H */
diff --git a/include/linux/of_iommu.h b/include/linux/of_iommu.h
index 16f4b3e87f20..e8d1e6d32d77 100644
--- a/include/linux/of_iommu.h
+++ b/include/linux/of_iommu.h
@@ -14,7 +14,8 @@ extern int of_get_dma_window(struct device_node *dn,
const char *prefix,
extern const struct iommu_ops *of_iommu_configure(struct device *dev,
struct device_node *master_np,
- const u32 *id);
+ const u32 *id,
+ struct of_phandle_args *iommu_spec);
#else
@@ -27,7 +28,8 @@ static inline int of_get_dma_window(struct
device_node *dn, const char *prefix,
static inline const struct iommu_ops *of_iommu_configure(struct
device *dev,
struct device_node *master_np,
- const u32 *id)
+ const u32 *id,
+ struct of_phandle_args *iommu_spec);
{
return NULL;
}