Re: [RFC PATCH 0/7] Introduce automatic DMA configuration for IOMMU masters
Hi Will, On 2014-08-29 17:54, Will Deacon wrote: This patch series is an RFC to implement IOMMU master configuration into of_dma_configure. I haven't yet ported any IOMMU drivers to it, so it remains untested, but I wanted to get some early feedback to ensure that this doesn't end up going in the wrong direction. The idea comes out of my understanding following discussions with Arnd and David at Kernel Summit / LinuxCon in Chicago. Essentially: - Allow IOMMUs to be probed early and set up per-instance data on their of_node - Add a new callback to the iommu_ops structure for adding a device with a set of opaque IDs (e.g. Stream IDs or Requester IDs) - Add an of_iommu_configure function, called from of_dma_configure, which registers the master IDs with the correspond IOMMU before probing the master itself - Where applicable, create an IOMMU domain per device and swizzle the DMA ops for that device to use the IOMMU variants. I haven't bothered doing anything clever with the DMA masks yet, so I wouldn't be surprised if we end up chewing through tonnes of memory allocating IOMMU domains that are far too big at the moment. Again, this is just an RFC and I'd welcome comments on the general direction of the series. Thanks for your patches, I wasn't aware the fact that you are working on this. When do you plan to send a second version? I would like to rebase my Exynos IOMMU patches (https://lkml.org/lkml/2014/8/5/183) on top of your work, but I wonder if I should select this version as a base or wait a bit for an update. Best regards -- Marek Szyprowski, PhD Samsung RD Institute Poland ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC PATCH 1/7] iommu: provide early initialisation hook for IOMMU drivers
On Monday 01 September 2014 09:52:19 Thierry Reding wrote: On Fri, Aug 29, 2014 at 04:54:24PM +0100, Will Deacon wrote: IOMMU drivers must be initialised before any of their upstream devices, otherwise the relevant iommu_ops won't be configured for the bus in question. To solve this, a number of IOMMU drivers use initcalls to initialise the driver before anything has a chance to be probed. Whilst this solves the immediate problem, it leaves the job of probing the IOMMU completely separate from the iommu_ops to configure the IOMMU, which are called on a per-bus basis and require the driver to figure out exactly which instance of the IOMMU is being requested. In particular, the add_device callback simply passes a struct device to the driver, which then has to parse firmware tables or probe buses to identify the relevant IOMMU instance. This patch takes the first step in addressing this problem by adding an early initialisation pass for IOMMU drivers, giving them the ability to set some per-instance data on their of_node. This data can then be passed back to a new add_device function (added in a later patch) to identify the IOMMU instance in question. Signed-off-by: Will Deacon will.dea...@arm.com --- drivers/iommu/of_iommu.c | 14 ++ include/asm-generic/vmlinux.lds.h | 2 ++ include/linux/of_iommu.h | 21 + 3 files changed, 37 insertions(+) I don't think this is the right direction. We've been preaching that using initcall ordering is a bad thing and people should be using deferred probe instead. Now we have a bunch of these OF tables that do the exact opposite. Note that these are nothing more than a variant of initcalls that get called at platform-specific points in time. My idea to solve this problem was to defer probing of the bus master device from the add_device IOMMU operation. This obviously won't work with add_device called from the BUS_NOTIFY_ADD_DEVICE notifier, which led me to naively wonder whether we couldn't call the add_device operation from the BUS_NOTIFY_BIND_DRIVER notifier instead. There are also ongoing discussions about how to change the device probe order to use dependencies (such as phandles from device tree) to prevent excessive deferred probing. With that in place this would be solved in a much more generic way. -- Regards, Laurent Pinchart signature.asc Description: This is a digitally signed message part. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC PATCH 0/7] Introduce automatic DMA configuration for IOMMU masters
On Tue, Sep 02, 2014 at 07:26:01AM +0100, Marek Szyprowski wrote: Hi Will, Hi Marek, On 2014-08-29 17:54, Will Deacon wrote: This patch series is an RFC to implement IOMMU master configuration into of_dma_configure. I haven't yet ported any IOMMU drivers to it, so it remains untested, but I wanted to get some early feedback to ensure that this doesn't end up going in the wrong direction. The idea comes out of my understanding following discussions with Arnd and David at Kernel Summit / LinuxCon in Chicago. Essentially: - Allow IOMMUs to be probed early and set up per-instance data on their of_node - Add a new callback to the iommu_ops structure for adding a device with a set of opaque IDs (e.g. Stream IDs or Requester IDs) - Add an of_iommu_configure function, called from of_dma_configure, which registers the master IDs with the correspond IOMMU before probing the master itself - Where applicable, create an IOMMU domain per device and swizzle the DMA ops for that device to use the IOMMU variants. I haven't bothered doing anything clever with the DMA masks yet, so I wouldn't be surprised if we end up chewing through tonnes of memory allocating IOMMU domains that are far too big at the moment. Again, this is just an RFC and I'd welcome comments on the general direction of the series. Thanks for your patches, I wasn't aware the fact that you are working on this. When do you plan to send a second version? I would like to rebase my Exynos IOMMU patches (https://lkml.org/lkml/2014/8/5/183) on top of your work, but I wonder if I should select this version as a base or wait a bit for an update. I'll try and get something out today/tomorrow depending on how easily the review comments fall out. It would be really great if you get an IOMMU working with this (I was going to look at the ARM SMMU once this stops moving) -- I have concerns that allocating one domain per master might be too much, but it's hard to tell without an IOMMU driver ported over. I'll CC you on v2. Cheers, Will ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC PATCH 0/7] Introduce automatic DMA configuration for IOMMU masters
Hi Will, On 2014-09-02 10:31, Will Deacon wrote: On Tue, Sep 02, 2014 at 07:26:01AM +0100, Marek Szyprowski wrote: On 2014-08-29 17:54, Will Deacon wrote: This patch series is an RFC to implement IOMMU master configuration into of_dma_configure. I haven't yet ported any IOMMU drivers to it, so it remains untested, but I wanted to get some early feedback to ensure that this doesn't end up going in the wrong direction. The idea comes out of my understanding following discussions with Arnd and David at Kernel Summit / LinuxCon in Chicago. Essentially: - Allow IOMMUs to be probed early and set up per-instance data on their of_node - Add a new callback to the iommu_ops structure for adding a device with a set of opaque IDs (e.g. Stream IDs or Requester IDs) - Add an of_iommu_configure function, called from of_dma_configure, which registers the master IDs with the correspond IOMMU before probing the master itself - Where applicable, create an IOMMU domain per device and swizzle the DMA ops for that device to use the IOMMU variants. I haven't bothered doing anything clever with the DMA masks yet, so I wouldn't be surprised if we end up chewing through tonnes of memory allocating IOMMU domains that are far too big at the moment. Again, this is just an RFC and I'd welcome comments on the general direction of the series. Thanks for your patches, I wasn't aware the fact that you are working on this. When do you plan to send a second version? I would like to rebase my Exynos IOMMU patches (https://lkml.org/lkml/2014/8/5/183) on top of your work, but I wonder if I should select this version as a base or wait a bit for an update. I'll try and get something out today/tomorrow depending on how easily the review comments fall out. It would be really great if you get an IOMMU working with this (I was going to look at the ARM SMMU once this stops moving) Great, I will wait then for v2. -- I have concerns that allocating one domain per master might be too much, but it's hard to tell without an IOMMU driver ported over. One domain per master is IMHO a sane default configuration. The only default alternative I see is to have only one domain (related with dma-mapping subsystem) and bind all devices to it. However I really don't see any disadvantage of having separate domain per each master and such configuration gives devices better separation. However we also need to figure out how to let drivers to make their own configuration, like it is required by Exynos DRM subsystem, which consist of several devices, each having its own IOMMU controller, but for convenience those drivers assume that they all have been bound to the same, single domain. Best regards -- Marek Szyprowski, PhD Samsung RD Institute Poland ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC PATCH 0/7] Introduce automatic DMA configuration for IOMMU masters
On Tuesday 02 September 2014 10:48:02 Marek Szyprowski wrote: -- I have concerns that allocating one domain per master might be too much, but it's hard to tell without an IOMMU driver ported over. One domain per master is IMHO a sane default configuration. The only default alternative I see is to have only one domain (related with dma-mapping subsystem) and bind all devices to it. However I really don't see any disadvantage of having separate domain per each master and such configuration gives devices better separation. I was expecting that the dma-mapping implementation would by default use one domain for all devices, since that is what the simpler IOMMUs without domain support have to do anyway. For isolation purposes, it can only help to have more domains, but I would guess that there is some space overhead in maintaining lots of page tables. However we also need to figure out how to let drivers to make their own configuration, like it is required by Exynos DRM subsystem, which consist of several devices, each having its own IOMMU controller, but for convenience those drivers assume that they all have been bound to the same, single domain. IIRC with the way we ended up putting the mask into the iommu descriptor of the ARM SMMU, you can put multiple devices into the same iommu group, and have them automatically share a domain. I don't know if the same would work for the Samsung implementation. Arnd ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v3 0/3] Support for nesting IOMMUs in VFIO
Hello, This is version three of the patches I originally posted here: RFCv1: http://permalink.gmane.org/gmane.linux.kernel.iommu/5552 RFCv2: http://permalink.gmane.org/gmane.linux.kernel.iommu/5700 Changes since RFCv2 include: - Dropped the RFC tag - Rebased onto 3.17-rc* - Iterate the support bus_types (currently just PCI) and check that nesting is actually supported The corresponding arm-smmu changes are included to show how the new domain attribute can be used. All feedback welcome, Will --8 Will Deacon (3): iommu: introduce domain attribute for nesting IOMMUs vfio/iommu_type1: add new VFIO_TYPE1_NESTING_IOMMU IOMMU type iommu/arm-smmu: add support for DOMAIN_ATTR_NESTING attribute drivers/iommu/arm-smmu.c| 110 drivers/vfio/vfio_iommu_type1.c | 87 --- include/linux/iommu.h | 1 + include/uapi/linux/vfio.h | 2 + 4 files changed, 173 insertions(+), 27 deletions(-) -- 2.1.0 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v3 1/3] iommu: introduce domain attribute for nesting IOMMUs
Some IOMMUs, such as the ARM SMMU, support two stages of translation.
The idea behind such a scheme is to allow a guest operating system to
use the IOMMU for DMA mappings in the first stage of translation, with
the hypervisor then installing mappings in the second stage to provide
isolation of the DMA to the physical range assigned to that virtual
machine.
In order to allow IOMMU domains to be used for second-stage translation,
this patch adds a new iommu_attr (IOMMU_ATTR_NESTING) for setting
second-stage domains prior to device attach. The attribute can also be
queried to see if a domain is actually making use of nesting.
Cc: Joerg Roedel j...@8bytes.org
Cc: Alex Williamson alex.william...@redhat.com
Signed-off-by: Will Deacon will.dea...@arm.com
---
include/linux/iommu.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 20f9a527922a..7b02bcc85b9e 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -80,6 +80,7 @@ enum iommu_attr {
DOMAIN_ATTR_FSL_PAMU_STASH,
DOMAIN_ATTR_FSL_PAMU_ENABLE,
DOMAIN_ATTR_FSL_PAMUV1,
+ DOMAIN_ATTR_NESTING,/* two stages of translation */
DOMAIN_ATTR_MAX,
};
--
2.1.0
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v3 3/3] iommu/arm-smmu: add support for DOMAIN_ATTR_NESTING attribute
When domains are set with the DOMAIN_ATTR_NESTING flag, we must ensure
that we allocate them to stage-2 context banks if the hardware permits
it.
This patch adds support for the attribute to the ARM SMMU driver, with
the actual stage being determined depending on the features supported
by the hardware.
Signed-off-by: Will Deacon will.dea...@arm.com
---
drivers/iommu/arm-smmu.c | 110 ++-
1 file changed, 90 insertions(+), 20 deletions(-)
diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index d2d8cdaf4f0b..c745296b170f 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -404,9 +404,16 @@ struct arm_smmu_cfg {
#define ARM_SMMU_CB_ASID(cfg) ((cfg)-cbndx)
#define ARM_SMMU_CB_VMID(cfg) ((cfg)-cbndx + 1)
+enum arm_smmu_domain_stage {
+ ARM_SMMU_DOMAIN_S1 = 0,
+ ARM_SMMU_DOMAIN_S2,
+ ARM_SMMU_DOMAIN_NESTED,
+};
+
struct arm_smmu_domain {
struct arm_smmu_device *smmu;
struct arm_smmu_cfg cfg;
+ enum arm_smmu_domain_stage stage;
spinlock_t lock;
};
@@ -899,19 +906,46 @@ static int arm_smmu_init_domain_context(struct
iommu_domain *domain,
if (smmu_domain-smmu)
goto out_unlock;
- if (smmu-features ARM_SMMU_FEAT_TRANS_NESTED) {
+ /*
+* Mapping the requested stage onto what we support is surprisingly
+* complicated, mainly because the spec allows S1+S2 SMMUs without
+* support for nested translation. That means we end up with the
+* following table:
+*
+* RequestedSupportedActual
+* S1 N S1
+* S1 S1+S2S1
+* S1 S2 S2
+* S1 S1 S1
+* NN N
+* N S1+S2S2
+* NS2 S2
+* NS1 S1
+*
+* Note that you can't actually request stage-2 mappings.
+*/
+ if (!(smmu-features ARM_SMMU_FEAT_TRANS_S1))
+ smmu_domain-stage = ARM_SMMU_DOMAIN_S2;
+ if (!(smmu-features ARM_SMMU_FEAT_TRANS_S2))
+ smmu_domain-stage = ARM_SMMU_DOMAIN_S1;
+
+ switch (smmu_domain-stage) {
+ case ARM_SMMU_DOMAIN_S1:
+ cfg-cbar = CBAR_TYPE_S1_TRANS_S2_BYPASS;
+ start = smmu-num_s2_context_banks;
+ break;
+ case ARM_SMMU_DOMAIN_NESTED:
/*
* We will likely want to change this if/when KVM gets
* involved.
*/
- cfg-cbar = CBAR_TYPE_S1_TRANS_S2_BYPASS;
- start = smmu-num_s2_context_banks;
- } else if (smmu-features ARM_SMMU_FEAT_TRANS_S1) {
- cfg-cbar = CBAR_TYPE_S1_TRANS_S2_BYPASS;
- start = smmu-num_s2_context_banks;
- } else {
+ case ARM_SMMU_DOMAIN_S2:
cfg-cbar = CBAR_TYPE_S2_TRANS;
start = 0;
+ break;
+ default:
+ ret = -EINVAL;
+ goto out_unlock;
}
ret = __arm_smmu_alloc_bitmap(smmu-context_map, start,
@@ -1637,20 +1671,56 @@ static void arm_smmu_remove_device(struct device *dev)
iommu_group_remove_device(dev);
}
+static int arm_smmu_domain_get_attr(struct iommu_domain *domain,
+ enum iommu_attr attr, void *data)
+{
+ struct arm_smmu_domain *smmu_domain = domain-priv;
+
+ switch (attr) {
+ case DOMAIN_ATTR_NESTING:
+ *(int *)data = (smmu_domain-stage == ARM_SMMU_DOMAIN_NESTED);
+ return 0;
+ default:
+ return -ENODEV;
+ }
+}
+
+static int arm_smmu_domain_set_attr(struct iommu_domain *domain,
+ enum iommu_attr attr, void *data)
+{
+ struct arm_smmu_domain *smmu_domain = domain-priv;
+
+ switch (attr) {
+ case DOMAIN_ATTR_NESTING:
+ if (smmu_domain-smmu)
+ return -EPERM;
+ if (*(int *)data)
+ smmu_domain-stage = ARM_SMMU_DOMAIN_NESTED;
+ else
+ smmu_domain-stage = ARM_SMMU_DOMAIN_S1;
+
+ return 0;
+ default:
+ return -ENODEV;
+ }
+}
+
static const struct iommu_ops arm_smmu_ops = {
- .domain_init= arm_smmu_domain_init,
- .domain_destroy = arm_smmu_domain_destroy,
- .attach_dev = arm_smmu_attach_dev,
- .detach_dev = arm_smmu_detach_dev,
- .map= arm_smmu_map,
- .unmap = arm_smmu_unmap,
- .iova_to_phys = arm_smmu_iova_to_phys,
- .domain_has_cap = arm_smmu_domain_has_cap,
- .add_device = arm_smmu_add_device,
Re: [RFC PATCH 0/7] Introduce automatic DMA configuration for IOMMU masters
Hi Arnd, On 2014-09-02 10:56, Arnd Bergmann wrote: On Tuesday 02 September 2014 10:48:02 Marek Szyprowski wrote: -- I have concerns that allocating one domain per master might be too much, but it's hard to tell without an IOMMU driver ported over. One domain per master is IMHO a sane default configuration. The only default alternative I see is to have only one domain (related with dma-mapping subsystem) and bind all devices to it. However I really don't see any disadvantage of having separate domain per each master and such configuration gives devices better separation. I was expecting that the dma-mapping implementation would by default use one domain for all devices, since that is what the simpler IOMMUs without domain support have to do anyway. For isolation purposes, it can only help to have more domains, but I would guess that there is some space overhead in maintaining lots of page tables. I'm okay with both approaches (separate domain for each device vs. single common domain for all devices). Maybe this can be some kind of Kconfig option added to DMA debugging? Separation might be really helpful when debugging strange device behavior. However we also need to figure out how to let drivers to make their own configuration, like it is required by Exynos DRM subsystem, which consist of several devices, each having its own IOMMU controller, but for convenience those drivers assume that they all have been bound to the same, single domain. IIRC with the way we ended up putting the mask into the iommu descriptor of the ARM SMMU, you can put multiple devices into the same iommu group, and have them automatically share a domain. I don't know if the same would work for the Samsung implementation. The question is how to transfer such information from the device drivers, that need/benefit from such configuration to iommu driver, which does all the setup? This is something completely internal to particular drivers and should not be exported to device tree or userspace. Thierry suggested to hardcode this information in the iommu driver, but I'm looking for other approaches. Maybe simply releasing device from the default dma-mapping domain before attaching to custom one will be the easiest solution. Best regards -- Marek Szyprowski, PhD Samsung RD Institute Poland ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC PATCH 4/7] iommu: provide helper function to configure an IOMMU for an of master
On Tue, Sep 02, 2014 at 11:51:54AM +0100, Laurent Pinchart wrote:
Hi Will,
Hi Laurent,
On Friday 29 August 2014 16:54:27 Will Deacon wrote:
diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
index dd5112265cc9..6d13f962f156 100644
--- a/drivers/iommu/Kconfig
+++ b/drivers/iommu/Kconfig
@@ -15,7 +15,7 @@ if IOMMU_SUPPORT
config OF_IOMMU
def_bool y
- depends on OF
+ depends on OF IOMMU_API
config FSL_PAMU
bool Freescale IOMMU support
diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
index f9209666157c..a7d3b3a13b83 100644
--- a/drivers/iommu/of_iommu.c
+++ b/drivers/iommu/of_iommu.c
@@ -19,6 +19,7 @@
#include linux/export.h
#include linux/limits.h
+#include linux/iommu.h
Pet peeve of mine, how about keeping the headers alphabetically sorted ?
D'oh, I'm an idiot. Will fix.
#include linux/of.h
#include linux/of_iommu.h
@@ -90,6 +91,41 @@ int of_get_dma_window(struct device_node *dn, const char
*prefix, int index, }
EXPORT_SYMBOL_GPL(of_get_dma_window);
+int of_iommu_configure(struct device *dev)
+{
+ struct of_phandle_args iommu_spec;
+ struct bus_type *bus = dev-bus;
+ const struct iommu_ops *ops = bus-iommu_ops;
+ int ret = -EINVAL, idx = 0;
+
+ if (!iommu_present(bus))
+ return -ENODEV;
+
+ /*
+* We don't currently walk up the tree looking for a parent IOMMU.
+* See the `Notes:' section of
+* Documentation/devicetree/bindings/iommu/iommu.txt
+*/
+ while (!of_parse_phandle_with_args(dev-of_node, iommus,
+ #iommu-cells, idx,
+ iommu_spec)) {
+ void *data = of_iommu_get_data(iommu_spec.np);
+
+ of_node_put(iommu_spec.np);
+ if (!ops-add_device_master_ids)
+ return -ENODEV;
Can't you move this outside of the loop ?
Yup, done.
+
+ ret = ops-add_device_master_ids(dev, iommu_spec.args_count,
+iommu_spec.args, data);
I'm curious, how do you envision support for devices connected to multiple
IOMMUs ? IOMMU drivers usually allocate a per-device archdata structure in
the
add_device operation and store it in the dev-archdata.iommu field. How would
that work with multiple invocations of add_device or add_device_master_ids ?
Even devices with a single IOMMU could have multiple callbacks, since the
IOMMU gets called back once per ID in reality. That means the IOMMU drivers
will need to be tolerant of adding a master they already know about (by
looking up the device and adding the additional IDs).
Once I've reworked the data argument to be a struct iommu, we can add fields
there if they're generally useful.
Will
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
RE: [PATCH v3 3/3] iommu/arm-smmu: add support for DOMAIN_ATTR_NESTING attribute
Hi Will,
We would still need a mechanism to distinguish stage1 mapping from stage2
mapping i.e. for nested translation we should be able to specify whether the
mapping corresponds to stage1 or stage 2 translation. Also, correspondingly we
would require different context banks for stage 1 and stage 2 translation.
Regards
Varun
-Original Message-
From: iommu-boun...@lists.linux-foundation.org [mailto:iommu-
boun...@lists.linux-foundation.org] On Behalf Of Will Deacon
Sent: Tuesday, September 02, 2014 3:24 PM
To: iommu@lists.linux-foundation.org
Cc: Will Deacon
Subject: [PATCH v3 3/3] iommu/arm-smmu: add support for
DOMAIN_ATTR_NESTING attribute
When domains are set with the DOMAIN_ATTR_NESTING flag, we must ensure
that we allocate them to stage-2 context banks if the hardware permits it.
This patch adds support for the attribute to the ARM SMMU driver, with the
actual stage being determined depending on the features supported by the
hardware.
Signed-off-by: Will Deacon will.dea...@arm.com
---
drivers/iommu/arm-smmu.c | 110
++-
1 file changed, 90 insertions(+), 20 deletions(-)
diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c index
d2d8cdaf4f0b..c745296b170f 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -404,9 +404,16 @@ struct arm_smmu_cfg {
#define ARM_SMMU_CB_ASID(cfg)((cfg)-cbndx)
#define ARM_SMMU_CB_VMID(cfg)((cfg)-cbndx + 1)
+enum arm_smmu_domain_stage {
+ ARM_SMMU_DOMAIN_S1 = 0,
+ ARM_SMMU_DOMAIN_S2,
+ ARM_SMMU_DOMAIN_NESTED,
+};
+
struct arm_smmu_domain {
struct arm_smmu_device *smmu;
struct arm_smmu_cfg cfg;
+ enum arm_smmu_domain_stage stage;
spinlock_t lock;
};
@@ -899,19 +906,46 @@ static int arm_smmu_init_domain_context(struct
iommu_domain *domain,
if (smmu_domain-smmu)
goto out_unlock;
- if (smmu-features ARM_SMMU_FEAT_TRANS_NESTED) {
+ /*
+ * Mapping the requested stage onto what we support is surprisingly
+ * complicated, mainly because the spec allows S1+S2 SMMUs without
+ * support for nested translation. That means we end up with the
+ * following table:
+ *
+ * RequestedSupportedActual
+ * S1 N S1
+ * S1 S1+S2S1
+ * S1 S2 S2
+ * S1 S1 S1
+ * NN N
+ * N S1+S2S2
+ * NS2 S2
+ * NS1 S1
+ *
+ * Note that you can't actually request stage-2 mappings.
+ */
+ if (!(smmu-features ARM_SMMU_FEAT_TRANS_S1))
+ smmu_domain-stage = ARM_SMMU_DOMAIN_S2;
+ if (!(smmu-features ARM_SMMU_FEAT_TRANS_S2))
+ smmu_domain-stage = ARM_SMMU_DOMAIN_S1;
+
+ switch (smmu_domain-stage) {
+ case ARM_SMMU_DOMAIN_S1:
+ cfg-cbar = CBAR_TYPE_S1_TRANS_S2_BYPASS;
+ start = smmu-num_s2_context_banks;
+ break;
+ case ARM_SMMU_DOMAIN_NESTED:
/*
* We will likely want to change this if/when KVM gets
* involved.
*/
- cfg-cbar = CBAR_TYPE_S1_TRANS_S2_BYPASS;
- start = smmu-num_s2_context_banks;
- } else if (smmu-features ARM_SMMU_FEAT_TRANS_S1) {
- cfg-cbar = CBAR_TYPE_S1_TRANS_S2_BYPASS;
- start = smmu-num_s2_context_banks;
- } else {
+ case ARM_SMMU_DOMAIN_S2:
cfg-cbar = CBAR_TYPE_S2_TRANS;
start = 0;
+ break;
+ default:
+ ret = -EINVAL;
+ goto out_unlock;
}
ret = __arm_smmu_alloc_bitmap(smmu-context_map, start, @@ -
1637,20 +1671,56 @@ static void arm_smmu_remove_device(struct device
*dev)
iommu_group_remove_device(dev);
}
+static int arm_smmu_domain_get_attr(struct iommu_domain *domain,
+ enum iommu_attr attr, void *data) {
+ struct arm_smmu_domain *smmu_domain = domain-priv;
+
+ switch (attr) {
+ case DOMAIN_ATTR_NESTING:
+ *(int *)data = (smmu_domain-stage ==
ARM_SMMU_DOMAIN_NESTED);
+ return 0;
+ default:
+ return -ENODEV;
+ }
+}
+
+static int arm_smmu_domain_set_attr(struct iommu_domain *domain,
+ enum iommu_attr attr, void *data) {
+ struct arm_smmu_domain *smmu_domain = domain-priv;
+
+ switch (attr) {
+ case DOMAIN_ATTR_NESTING:
+ if (smmu_domain-smmu)
+ return -EPERM;
+ if (*(int *)data)
+ smmu_domain-stage =
Re: [PATCH v3 3/3] iommu/arm-smmu: add support for DOMAIN_ATTR_NESTING attribute
On Tue, Sep 02, 2014 at 12:12:20PM +0100, Varun Sethi wrote: We would still need a mechanism to distinguish stage1 mapping from stage2 mapping i.e. for nested translation we should be able to specify whether the mapping corresponds to stage1 or stage 2 translation. Also, correspondingly we would require different context banks for stage 1 and stage 2 translation. Please take a look at my iommu/pci branch where I have much more support for nested translation with the ARM SMMU driver. This series is purely about getting the stage-2 part configured -- you need a virtual SMMU interface for the stage-1 component. Will ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC PATCH 4/7] iommu: provide helper function to configure an IOMMU for an of master
On Tuesday 02 September 2014 11:03:42 Will Deacon wrote: On Mon, Sep 01, 2014 at 09:18:26PM +0100, Arnd Bergmann wrote: On Monday 01 September 2014 17:40:00 Will Deacon wrote: On Mon, Sep 01, 2014 at 03:46:18PM +0100, Arnd Bergmann wrote: On Monday 01 September 2014 10:29:40 Thierry Reding wrote: I think this could use a bit more formalization. As I said in another reply earlier, there's very little standardization in the IOMMU API. That certainly gives us a lot of flexibility but it also has the downside that it's difficult to handle these abstractions in the core, which is really what the core is all about, isn't it? One method that worked really well for this in the past for other subsystems is to allow drivers to specify an .of_xlate() function that takes the controller device and a struct of_phandle_args. It is that function's responsibility to take the information in an of_phandle_args structure and use that to create some subsystem specific handle that represents this information in a way that it can readily be used. Yes, good idea. Hmm, how does this work for PCI devices? The current RFC takes care to ensure that the core changes work just as well for OF devices as PCI devices, and the of-specific functions and data structures are not part of it. I don't mind handling PCI devices separately. They are different in a number of ways already, in particular the way that they don't normally have an of_node attached to them but actually have a PCI bus/dev/function number. Sure, but at the point when we call back into the iommu_ops structure we really don't want bus specific functions. That's why I avoided any OF data structures being passed to add_device_master_ids. Well, we clearly need some format that the caller and the callee agree on. It can't be a completely opaque pointer because it's something that has to be filled out by someone who knows the format. Using the DT format has the advantage that the caller does not have to know anything about the underlying driver except for #size-cells, and it just passes the data it gets from DT into the driver. This is how we do the association in a lot of other subsystems. Anyway, I'll try to hack something together shortly. I think the proposal is: - Make add_device_master_ids take a generic structure (struct iommu) - Add an of_xlate callback into iommu_ops which returns a populated struct iommu based on the of_node We may have been talking past one another. What I meant with 'struct iommu' is something that identifies the iommu instance, not the connection to a particular master. What you describe here would work, but then I think the structure should have a different name. However, it seems easier to not have the add_device_master_ids at and just do the association in the xlate callback instead. We still need to figure out how to do it for PCI of course. One possibility would be to add another argument to the xlate function and have that called by the PCI device probing method with the iommus property of the PCI host controller along with the a u64 number that is generated by the host bridge driver based on the bus/device/function number of the device. This means that the new callback function for the iommu API remains DT specific, but is not really bus specific. It does however not solve the embedded x86 use case, which may need some other callback. We might be lucky there if we are able to just use the PCI b/d/f number as a unique identifier and have a NULL argument for the respective iommus property. Arnd ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC PATCH 0/7] Introduce automatic DMA configuration for IOMMU masters
On Tuesday 02 September 2014 12:42:13 Marek Szyprowski wrote: Hi Arnd, On 2014-09-02 10:56, Arnd Bergmann wrote: On Tuesday 02 September 2014 10:48:02 Marek Szyprowski wrote: -- I have concerns that allocating one domain per master might be too much, but it's hard to tell without an IOMMU driver ported over. One domain per master is IMHO a sane default configuration. The only default alternative I see is to have only one domain (related with dma-mapping subsystem) and bind all devices to it. However I really don't see any disadvantage of having separate domain per each master and such configuration gives devices better separation. I was expecting that the dma-mapping implementation would by default use one domain for all devices, since that is what the simpler IOMMUs without domain support have to do anyway. For isolation purposes, it can only help to have more domains, but I would guess that there is some space overhead in maintaining lots of page tables. I'm okay with both approaches (separate domain for each device vs. single common domain for all devices). Maybe this can be some kind of Kconfig option added to DMA debugging? Separation might be really helpful when debugging strange device behavior. We should probably support the iommu=strict command line option that some other architectures have. This is mainly meant to ensure that IOTLBs are shot down as soon as the driver unmaps some memory, which you often want to avoid for performance reasons. The iommu driver itself can then decide to also use separate domains for iommu=strict but a shared domain otherwise. For hardware on which the shared domain is hard to do, the driver might always use separate domains. However we also need to figure out how to let drivers to make their own configuration, like it is required by Exynos DRM subsystem, which consist of several devices, each having its own IOMMU controller, but for convenience those drivers assume that they all have been bound to the same, single domain. IIRC with the way we ended up putting the mask into the iommu descriptor of the ARM SMMU, you can put multiple devices into the same iommu group, and have them automatically share a domain. I don't know if the same would work for the Samsung implementation. The question is how to transfer such information from the device drivers, that need/benefit from such configuration to iommu driver, which does all the setup? This is something completely internal to particular drivers and should not be exported to device tree or userspace. Thierry suggested to hardcode this information in the iommu driver, but I'm looking for other approaches. Maybe simply releasing device from the default dma-mapping domain before attaching to custom one will be the easiest solution. For the ARM SMMU, the problem is that there is not necessarily a good way to partition the masters into IOMMU groups automatically, therefore we want to provide some hints in DT. On a machine that can have more domains than it has masters, this is not a problem and we can always use an all-ones mask, but for a machine on which this is not the case, the problem is simplified a lot of we hardcode the masks in a way that can always work, putting multiple devices into an iommu group if necessary. This is similar to how we do things for pinctrl, where you might have a theoretically endless space of options to set stuff up, but we can simplify it by defining the useful configurations. Arnd ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC PATCH 0/7] Introduce automatic DMA configuration for IOMMU masters
Hi Will, On 2014-09-02 12:57, Will Deacon wrote: On Tue, Sep 02, 2014 at 11:42:13AM +0100, Marek Szyprowski wrote: On 2014-09-02 10:56, Arnd Bergmann wrote: On Tuesday 02 September 2014 10:48:02 Marek Szyprowski wrote: -- I have concerns that allocating one domain per master might be too much, but it's hard to tell without an IOMMU driver ported over. One domain per master is IMHO a sane default configuration. The only default alternative I see is to have only one domain (related with dma-mapping subsystem) and bind all devices to it. However I really don't see any disadvantage of having separate domain per each master and such configuration gives devices better separation. I was expecting that the dma-mapping implementation would by default use one domain for all devices, since that is what the simpler IOMMUs without domain support have to do anyway. For isolation purposes, it can only help to have more domains, but I would guess that there is some space overhead in maintaining lots of page tables. I'm okay with both approaches (separate domain for each device vs. single common domain for all devices). Maybe this can be some kind of Kconfig option added to DMA debugging? Separation might be really helpful when debugging strange device behavior. One potential problem with a single domain is when you have multiple instances of a given IOMMU, each with different hardware restrictions. Then you can end up with multiple sets of page tables for the domain which, although not impossible to work with, is a bit of a mess. Maybe the default dma-mapping domain should be one per a given IOMMU instance? This will simplify a lot of things in such case. I think having one domain per IOMMU instance would make the most sense, but then you have to teach more of the stack about the IOMMU topology. I think we'll get there in the end, but that's a little way off right now. Right, those seems to be a details. Best regards -- Marek Szyprowski, PhD Samsung RD Institute Poland ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC PATCH 0/7] Introduce automatic DMA configuration for IOMMU masters
Hi Arnd, On 2014-09-02 14:22, Arnd Bergmann wrote: On Tuesday 02 September 2014 12:42:13 Marek Szyprowski wrote: On 2014-09-02 10:56, Arnd Bergmann wrote: On Tuesday 02 September 2014 10:48:02 Marek Szyprowski wrote: -- I have concerns that allocating one domain per master might be too much, but it's hard to tell without an IOMMU driver ported over. One domain per master is IMHO a sane default configuration. The only default alternative I see is to have only one domain (related with dma-mapping subsystem) and bind all devices to it. However I really don't see any disadvantage of having separate domain per each master and such configuration gives devices better separation. I was expecting that the dma-mapping implementation would by default use one domain for all devices, since that is what the simpler IOMMUs without domain support have to do anyway. For isolation purposes, it can only help to have more domains, but I would guess that there is some space overhead in maintaining lots of page tables. I'm okay with both approaches (separate domain for each device vs. single common domain for all devices). Maybe this can be some kind of Kconfig option added to DMA debugging? Separation might be really helpful when debugging strange device behavior. We should probably support the iommu=strict command line option that some other architectures have. This is mainly meant to ensure that IOTLBs are shot down as soon as the driver unmaps some memory, which you often want to avoid for performance reasons. The iommu driver itself can then decide to also use separate domains for iommu=strict but a shared domain otherwise. For hardware on which the shared domain is hard to do, the driver might always use separate domains. Just to let you know, lazy unmapping is not yet implemented in ARM dma-mapping implementation based on IOMMU. However we also need to figure out how to let drivers to make their own configuration, like it is required by Exynos DRM subsystem, which consist of several devices, each having its own IOMMU controller, but for convenience those drivers assume that they all have been bound to the same, single domain. IIRC with the way we ended up putting the mask into the iommu descriptor of the ARM SMMU, you can put multiple devices into the same iommu group, and have them automatically share a domain. I don't know if the same would work for the Samsung implementation. The question is how to transfer such information from the device drivers, that need/benefit from such configuration to iommu driver, which does all the setup? This is something completely internal to particular drivers and should not be exported to device tree or userspace. Thierry suggested to hardcode this information in the iommu driver, but I'm looking for other approaches. Maybe simply releasing device from the default dma-mapping domain before attaching to custom one will be the easiest solution. For the ARM SMMU, the problem is that there is not necessarily a good way to partition the masters into IOMMU groups automatically, therefore we want to provide some hints in DT. On a machine that can have more domains than it has masters, this is not a problem and we can always use an all-ones mask, but for a machine on which this is not the case, the problem is simplified a lot of we hardcode the masks in a way that can always work, putting multiple devices into an iommu group if necessary. Well, I was talking about the Exynos IOMMU case, where there are no hw restrictions and grouping is done just to make things easier for the Exynos DRM drivers (a buffer gets the same DMA address for all devices, which are a part of virtual Exynos DRM device). This is similar to how we do things for pinctrl, where you might have a theoretically endless space of options to set stuff up, but we can simplify it by defining the useful configurations. Right, if hardware is limited, a sane working configuration is something that should be encoded in device tree. Best regards -- Marek Szyprowski, PhD Samsung RD Institute Poland ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC PATCH 0/7] Introduce automatic DMA configuration for IOMMU masters
On Tuesday 02 September 2014 14:24:18 Marek Szyprowski wrote: For isolation purposes, it can only help to have more domains, but I would guess that there is some space overhead in maintaining lots of page tables. I'm okay with both approaches (separate domain for each device vs. single common domain for all devices). Maybe this can be some kind of Kconfig option added to DMA debugging? Separation might be really helpful when debugging strange device behavior. One potential problem with a single domain is when you have multiple instances of a given IOMMU, each with different hardware restrictions. Then you can end up with multiple sets of page tables for the domain which, although not impossible to work with, is a bit of a mess. Maybe the default dma-mapping domain should be one per a given IOMMU instance? This will simplify a lot of things in such case. Yes, that sounds like a good idea. Arnd ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC PATCH 0/7] Introduce automatic DMA configuration for IOMMU masters
On Tuesday 02 September 2014 14:30:36 Marek Szyprowski wrote: However we also need to figure out how to let drivers to make their own configuration, like it is required by Exynos DRM subsystem, which consist of several devices, each having its own IOMMU controller, but for convenience those drivers assume that they all have been bound to the same, single domain. IIRC with the way we ended up putting the mask into the iommu descriptor of the ARM SMMU, you can put multiple devices into the same iommu group, and have them automatically share a domain. I don't know if the same would work for the Samsung implementation. The question is how to transfer such information from the device drivers, that need/benefit from such configuration to iommu driver, which does all the setup? This is something completely internal to particular drivers and should not be exported to device tree or userspace. Thierry suggested to hardcode this information in the iommu driver, but I'm looking for other approaches. Maybe simply releasing device from the default dma-mapping domain before attaching to custom one will be the easiest solution. For the ARM SMMU, the problem is that there is not necessarily a good way to partition the masters into IOMMU groups automatically, therefore we want to provide some hints in DT. On a machine that can have more domains than it has masters, this is not a problem and we can always use an all-ones mask, but for a machine on which this is not the case, the problem is simplified a lot of we hardcode the masks in a way that can always work, putting multiple devices into an iommu group if necessary. Well, I was talking about the Exynos IOMMU case, where there are no hw restrictions and grouping is done just to make things easier for the Exynos DRM drivers (a buffer gets the same DMA address for all devices, which are a part of virtual Exynos DRM device). Does that mean you don't actually need to use multiple contexts here and could actually just use the normal dma-mapping interface if there is a way to ensure the mappings are always shared across these masters? Or do you need this in addition to being able to use multiple masters so you can have multiple rendering contexts in user space? Arnd ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC PATCH 4/7] iommu: provide helper function to configure an IOMMU for an of master
On Tue, Sep 02, 2014 at 01:15:06PM +0100, Arnd Bergmann wrote:
On Tuesday 02 September 2014 11:03:42 Will Deacon wrote:
On Mon, Sep 01, 2014 at 09:18:26PM +0100, Arnd Bergmann wrote:
I don't mind handling PCI devices separately. They are different in a
number
of ways already, in particular the way that they don't normally have an
of_node attached to them but actually have a PCI bus/dev/function number.
Sure, but at the point when we call back into the iommu_ops structure we
really don't want bus specific functions. That's why I avoided any OF
data structures being passed to add_device_master_ids.
Well, we clearly need some format that the caller and the callee agree
on. It can't be a completely opaque pointer because it's something
that has to be filled out by someone who knows the format.
Using the DT format has the advantage that the caller does not have
to know anything about the underlying driver except for #size-cells,
and it just passes the data it gets from DT into the driver. This is
how we do the association in a lot of other subsystems.
Ok. More below.
Anyway, I'll try to hack something together shortly. I think the proposal
is:
- Make add_device_master_ids take a generic structure (struct iommu)
- Add an of_xlate callback into iommu_ops which returns a populated
struct iommu based on the of_node
We may have been talking past one another. What I meant with 'struct iommu'
is something that identifies the iommu instance, not the connection to
a particular master. What you describe here would work, but then I think
the structure should have a different name. However, it seems easier to
not have the add_device_master_ids at and just do the association in the
xlate callback instead.
Yes, I think we were talking about two different things. If we move all
of the master handling into the xlate callback, then we can just use
of_phandle_args as the generic master representation (using the PCI host
controller node for PCI devices, as you mentioned). However, xlate is a
bit of a misnomer then, as it's not actually translating anything; the
handle used for DMA masters is still struct device, and we have that
already in of_dma_configure.
I'm still unsure about what to put inside struct iommu other than a private
data pointer. You previously suggested:
struct iommu {
struct device *dev;
const struct iommu_ops *ops;
struct list_head domains;
void *private;
};
but that has the following problems:
- We don't have a struct device * for the IOMMU until it's been probed
via the standard driver probing mechanisms, which may well be after
we've started registering masters
- The ops are still registered on a per-bus basis, and each domain has
a pointer to them anyway
- The IOMMU driver really doesn't care about the domains, as the domain
in question is always passed back to the functions that need it (e.g.
attach, map, ...).
The only useful field I can think of is something like a tree of masters,
but then we have to define a generic wrapper around struct device, which
is at odds with the rest of the IOMMU API.
One alternative is having the xlate call populate device-archdata.iommu,
but that's arch-specific and is essentially another opaque pointer.
Will
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC PATCH 0/7] Introduce automatic DMA configuration for IOMMU masters
On 2014-09-02 14:46, Arnd Bergmann wrote: On Tuesday 02 September 2014 14:30:36 Marek Szyprowski wrote: However we also need to figure out how to let drivers to make their own configuration, like it is required by Exynos DRM subsystem, which consist of several devices, each having its own IOMMU controller, but for convenience those drivers assume that they all have been bound to the same, single domain. IIRC with the way we ended up putting the mask into the iommu descriptor of the ARM SMMU, you can put multiple devices into the same iommu group, and have them automatically share a domain. I don't know if the same would work for the Samsung implementation. The question is how to transfer such information from the device drivers, that need/benefit from such configuration to iommu driver, which does all the setup? This is something completely internal to particular drivers and should not be exported to device tree or userspace. Thierry suggested to hardcode this information in the iommu driver, but I'm looking for other approaches. Maybe simply releasing device from the default dma-mapping domain before attaching to custom one will be the easiest solution. For the ARM SMMU, the problem is that there is not necessarily a good way to partition the masters into IOMMU groups automatically, therefore we want to provide some hints in DT. On a machine that can have more domains than it has masters, this is not a problem and we can always use an all-ones mask, but for a machine on which this is not the case, the problem is simplified a lot of we hardcode the masks in a way that can always work, putting multiple devices into an iommu group if necessary. Well, I was talking about the Exynos IOMMU case, where there are no hw restrictions and grouping is done just to make things easier for the Exynos DRM drivers (a buffer gets the same DMA address for all devices, which are a part of virtual Exynos DRM device). Does that mean you don't actually need to use multiple contexts here and could actually just use the normal dma-mapping interface if there is a way to ensure the mappings are always shared across these masters? Well, a default, shared single domain for dma-mapping interface will work with Exynos DRM and its multiple masters, although I never thought about such configuration. Or do you need this in addition to being able to use multiple masters so you can have multiple rendering contexts in user space? Such advanced IO space management is not yet implemented. However there are also devices (like multimedia codec - exynos mfc and camera capture subsystem exynos isp), which have limited DMA/IO window (256MiB in case of video codec), so they will still need to use their own separate domain. Best regards -- Marek Szyprowski, PhD Samsung RD Institute Poland ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC PATCH 4/7] iommu: provide helper function to configure an IOMMU for an of master
On Tuesday 02 September 2014 14:05:08 Will Deacon wrote:
On Tue, Sep 02, 2014 at 01:15:06PM +0100, Arnd Bergmann wrote:
Anyway, I'll try to hack something together shortly. I think the proposal
is:
- Make add_device_master_ids take a generic structure (struct iommu)
- Add an of_xlate callback into iommu_ops which returns a populated
struct iommu based on the of_node
We may have been talking past one another. What I meant with 'struct iommu'
is something that identifies the iommu instance, not the connection to
a particular master. What you describe here would work, but then I think
the structure should have a different name. However, it seems easier to
not have the add_device_master_ids at and just do the association in the
xlate callback instead.
Yes, I think we were talking about two different things. If we move all
of the master handling into the xlate callback, then we can just use
of_phandle_args as the generic master representation (using the PCI host
controller node for PCI devices, as you mentioned). However, xlate is a
bit of a misnomer then, as it's not actually translating anything; the
handle used for DMA masters is still struct device, and we have that
already in of_dma_configure.
I'm still unsure about what to put inside struct iommu other than a private
data pointer. You previously suggested:
struct iommu {
struct device *dev;
const struct iommu_ops *ops;
struct list_head domains;
void *private;
};
but that has the following problems:
- We don't have a struct device * for the IOMMU until it's been probed
via the standard driver probing mechanisms, which may well be after
we've started registering masters
Right, this may have to be the device_node pointer. I also realized that if
we have this structure, we can stop using the device_node-data field
and instead walk a list of iommu structures.
- The ops are still registered on a per-bus basis, and each domain has
a pointer to them anyway
I think that has to change in the long run: we may well require distinct
IOMMU operations if we have multiple IOMMUs used on the same bus_type.
- The IOMMU driver really doesn't care about the domains, as the domain
in question is always passed back to the functions that need it (e.g.
attach, map, ...).
This is an artifact of the API being single-instance at the moment.
We might not in fact need it, I was just trying to think of things
that naturally fit in there and that are probably already linked
together in the individual iommu drivers.
The only useful field I can think of is something like a tree of masters,
but then we have to define a generic wrapper around struct device, which
is at odds with the rest of the IOMMU API.
Maybe a list of the groups instead? I don't think you should need
a list of every single master here.
One alternative is having the xlate call populate device-archdata.iommu,
but that's arch-specific and is essentially another opaque pointer.
I think every architecture that supports IOMMUs needs to have this
archdata pointer though, so that is still a good place to put private
stuff.
Arnd
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 10/29] drivers: add DRIVER_HAS_OWN_IOMMU_MANAGER flag
On Mon, Sep 01, 2014 at 07:22:32AM +0200, Marek Szyprowski wrote:
Hi Greg,
On 2014-08-05 12:47, Marek Szyprowski wrote:
This patch adds a new flags for device drivers. This flag instructs
kernel that the device driver does it own management of IOMMU assisted
IO address space translations, so no default dma-mapping structures
should be initialized.
Signed-off-by: Marek Szyprowski m.szyprow...@samsung.com
---
include/linux/device.h | 2 ++
1 file changed, 2 insertions(+)
diff --git a/include/linux/device.h b/include/linux/device.h
index 5f4ff02..2e62371 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -253,6 +253,8 @@ struct device_driver {
/* disables bind/unbind via sysfs */
#define DRIVER_SUPPRESS_BIND_ATTRS(1 0)
+/* driver uses own methods to manage IO address space */
+#define DRIVER_HAS_OWN_IOMMU_MANAGER (1 1)
extern int __must_check driver_register(struct device_driver *drv);
extern void driver_unregister(struct device_driver *drv);
Could you comment if the approach of using flags in the struct driver
could be accepted? I've converted suppress_bind_attrs entry to flags to
avoid extending the structure, please see patch [PATCH 05/29] drivers:
convert suppress_bind_attrs parameter into flags.
Is this really necessary? What I did as part of an RFC series for Tegra
IOMMU support is keep this knowledge within the IOMMU driver rather than
export it to the driver core.
The idea being that the IOMMU driver wouldn't create an ARM DMA/IOMMU
mapping by default but rather allow individual drivers to be marked as
kernel-internal and use the DMA/IOMMU glue in that case. Drivers such
as DRM would use the IOMMU API directly.
Thierry
pgpolh0RH6pd3.pgp
Description: PGP signature
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
RE: [RFC PATCH 1/7] iommu: provide early initialisation hook for IOMMU drivers
Hi Will,
-Original Message-
From: Will Deacon [mailto:will.dea...@arm.com]
Sent: Friday, August 29, 2014 9:24 PM
To: linux-arm-ker...@lists.infradead.org; iommu@lists.linux-foundation.org
Cc: a...@arndb.de; dw...@infradead.org; jroe...@suse.de;
hd...@nvidia.com; Sethi Varun-B16395; thierry.red...@gmail.com;
laurent.pinch...@ideasonboard.com; Will Deacon
Subject: [RFC PATCH 1/7] iommu: provide early initialisation hook for IOMMU
drivers
IOMMU drivers must be initialised before any of their upstream devices,
otherwise the relevant iommu_ops won't be configured for the bus in
question. To solve this, a number of IOMMU drivers use initcalls to
initialise the driver before anything has a chance to be probed.
Whilst this solves the immediate problem, it leaves the job of probing
the IOMMU completely separate from the iommu_ops to configure the
IOMMU,
which are called on a per-bus basis and require the driver to figure out
exactly which instance of the IOMMU is being requested. In particular,
the add_device callback simply passes a struct device to the driver,
which then has to parse firmware tables or probe buses to identify the
relevant IOMMU instance.
This patch takes the first step in addressing this problem by adding an
early initialisation pass for IOMMU drivers, giving them the ability to
set some per-instance data on their of_node. This data can then be
passed back to a new add_device function (added in a later patch) to
identify the IOMMU instance in question.
Signed-off-by: Will Deacon will.dea...@arm.com
---
drivers/iommu/of_iommu.c | 14 ++
include/asm-generic/vmlinux.lds.h | 2 ++
include/linux/of_iommu.h | 21 +
3 files changed, 37 insertions(+)
diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
index e550ccb7634e..f9209666157c 100644
--- a/drivers/iommu/of_iommu.c
+++ b/drivers/iommu/of_iommu.c
@@ -89,3 +89,17 @@ int of_get_dma_window(struct device_node *dn, const
char *prefix, int index,
return 0;
}
EXPORT_SYMBOL_GPL(of_get_dma_window);
+
+void __init of_iommu_init(void)
+{
+ struct device_node *np;
+ const struct of_device_id *match, *matches = __iommu_of_table;
+
+ for_each_matching_node_and_match(np, matches, match) {
+ const int (*init_fn)(struct device_node *) = match-data;
Is the init function also responsible for setting iommu_ops (per bus)? We need
to take in to consideration that iommu API (iommu.c) initialization happens
during arch_init. When we setup bus iommu ops add_device would be called for
devices which have already been probed. Also, as I can see from the code we
have of_platform_populate which gets called right after of_iommu_init, which
would in turn also lead to add_device invocation (after add_device_master_ids).
I believe this would happen before iommu API initialization which would cause
issues.
-Varun
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
RE: [RFC PATCH 4/7] iommu: provide helper function to configure an IOMMU for an of master
Hi Will,
I am not clear on the functionality we want to achieve with this new API. Is
this a way to link devices to a particular IOMMU? Would this be used to filter
out add_device invocations i.e. iommu group creations just for the devices
attached to a particular IOMMU?
What is the purpose of the data pointer, that is being passed to this API?
Also, how would this be relevant to PCIe devices (also other hot plug devices).
Regards
Varun
-Original Message-
From: Will Deacon [mailto:will.dea...@arm.com]
Sent: Friday, August 29, 2014 9:24 PM
To: linux-arm-ker...@lists.infradead.org; iommu@lists.linux-foundation.org
Cc: a...@arndb.de; dw...@infradead.org; jroe...@suse.de;
hd...@nvidia.com; Sethi Varun-B16395; thierry.red...@gmail.com;
laurent.pinch...@ideasonboard.com; Will Deacon
Subject: [RFC PATCH 4/7] iommu: provide helper function to configure an
IOMMU for an of master
The generic IOMMU device-tree bindings can be used to add arbitrary OF
masters to an IOMMU with a compliant binding.
This patch introduces of_iommu_configure, which does exactly that.
Signed-off-by: Will Deacon will.dea...@arm.com
---
drivers/iommu/Kconfig| 2 +-
drivers/iommu/of_iommu.c | 36
include/linux/of_iommu.h | 6 ++
3 files changed, 43 insertions(+), 1 deletion(-)
diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig index
dd5112265cc9..6d13f962f156 100644
--- a/drivers/iommu/Kconfig
+++ b/drivers/iommu/Kconfig
@@ -15,7 +15,7 @@ if IOMMU_SUPPORT
config OF_IOMMU
def_bool y
- depends on OF
+ depends on OF IOMMU_API
config FSL_PAMU
bool Freescale IOMMU support
diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c index
f9209666157c..a7d3b3a13b83 100644
--- a/drivers/iommu/of_iommu.c
+++ b/drivers/iommu/of_iommu.c
@@ -19,6 +19,7 @@
#include linux/export.h
#include linux/limits.h
+#include linux/iommu.h
#include linux/of.h
#include linux/of_iommu.h
@@ -90,6 +91,41 @@ int of_get_dma_window(struct device_node *dn, const
char *prefix, int index, } EXPORT_SYMBOL_GPL(of_get_dma_window);
+int of_iommu_configure(struct device *dev) {
+ struct of_phandle_args iommu_spec;
+ struct bus_type *bus = dev-bus;
+ const struct iommu_ops *ops = bus-iommu_ops;
+ int ret = -EINVAL, idx = 0;
+
+ if (!iommu_present(bus))
+ return -ENODEV;
+
+ /*
+ * We don't currently walk up the tree looking for a parent IOMMU.
+ * See the `Notes:' section of
+ * Documentation/devicetree/bindings/iommu/iommu.txt
+ */
+ while (!of_parse_phandle_with_args(dev-of_node, iommus,
+#iommu-cells, idx,
+iommu_spec)) {
+ void *data = of_iommu_get_data(iommu_spec.np);
+
+ of_node_put(iommu_spec.np);
+ if (!ops-add_device_master_ids)
+ return -ENODEV;
+
+ ret = ops-add_device_master_ids(dev,
iommu_spec.args_count,
+ iommu_spec.args, data);
+ if (ret)
+ break;
+
+ idx++;
+ }
+
+ return ret;
+}
+
void __init of_iommu_init(void)
{
struct device_node *np;
diff --git a/include/linux/of_iommu.h b/include/linux/of_iommu.h index
29f2f3f88d6a..85c6d1152624 100644
--- a/include/linux/of_iommu.h
+++ b/include/linux/of_iommu.h
@@ -1,6 +1,7 @@
#ifndef __OF_IOMMU_H
#define __OF_IOMMU_H
+#include linux/device.h
#include linux/of.h
#ifdef CONFIG_OF_IOMMU
@@ -10,6 +11,7 @@ extern int of_get_dma_window(struct device_node *dn,
const char *prefix,
size_t *size);
extern void of_iommu_init(void);
+extern int of_iommu_configure(struct device *dev);
#else
@@ -21,6 +23,10 @@ static inline int of_get_dma_window(struct device_node
*dn, const char *prefix, }
static inline void of_iommu_init(void) { }
+static inline int of_iommu_configure(struct device *dev) {
+ return -ENODEV;
+}
#endif /* CONFIG_OF_IOMMU */
--
2.1.0
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC PATCH 1/7] iommu: provide early initialisation hook for IOMMU drivers
On Tuesday 02 September 2014 14:47:54 Varun Sethi wrote:
+void __init of_iommu_init(void)
+{
+ struct device_node *np;
+ const struct of_device_id *match, *matches = __iommu_of_table;
+
+ for_each_matching_node_and_match(np, matches, match) {
+ const int (*init_fn)(struct device_node *) = match-data;
Is the init function also responsible for setting iommu_ops (per bus)?
We need to take in to consideration that iommu API (iommu.c)
initialization happens during arch_init.
I would hope that as part as one of the next steps, we can skip
the step of setting the iommu_ops per bus_type. It's really
not that meaningful when only some of the devices on one bus
are able to use an iommu, and some may need other ops than others.
When we setup bus iommu
ops add_device would be called for devices which have already been
probed.
As soon as you move to the generic way of probing the IOMMU for
DT, you should not need an add_device callback any more.
Each iommu driver should do one one or the other, but not both.
Also, as I can see from the code we have of_platform_populate
which gets called right after of_iommu_init, which would in turn also
lead to add_device invocation (after add_device_master_ids).
I believe this would happen before iommu API initialization which
would cause issues.
of_iommu_init should be the point where the iommu API gets
initialized.
Arnd
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC PATCH v6 15/20] vfio/platform: support for maskable and automasked interrupts
On Sun, Jun 8, 2014 at 12:17 PM, Christoffer Dall
christoffer.d...@linaro.org wrote:
On Thu, Jun 05, 2014 at 07:03:23PM +0200, Antonios Motakis wrote:
Adds support to mask interrupts, and also for automasked interrupts.
Level sensitive interrupts are exposed as automasked interrupts and
are masked and disabled automatically when they fire.
Signed-off-by: Antonios Motakis a.mota...@virtualopensystems.com
---
drivers/vfio/platform/vfio_platform_irq.c | 112
--
drivers/vfio/platform/vfio_platform_private.h | 2 +
2 files changed, 109 insertions(+), 5 deletions(-)
diff --git a/drivers/vfio/platform/vfio_platform_irq.c
b/drivers/vfio/platform/vfio_platform_irq.c
index d79f5af..10dfbf0 100644
--- a/drivers/vfio/platform/vfio_platform_irq.c
+++ b/drivers/vfio/platform/vfio_platform_irq.c
@@ -51,9 +51,17 @@ int vfio_platform_irq_init(struct vfio_platform_device
*vdev)
if (hwirq 0)
goto err;
- vdev-irq[i].flags = VFIO_IRQ_INFO_EVENTFD;
+ spin_lock_init(vdev-irq[i].lock);
+
+ vdev-irq[i].flags = VFIO_IRQ_INFO_EVENTFD
+ | VFIO_IRQ_INFO_MASKABLE;
+
+ if (irq_get_trigger_type(hwirq) IRQ_TYPE_LEVEL_MASK)
+ vdev-irq[i].flags |= VFIO_IRQ_INFO_AUTOMASKED;
This seems to rely on the fact that you had actually loaded a driver for
your device to set the right type. Is this assumption always correct?
It seems to me that this configuration bit should now be up to your user
space drive who is the best candidate to know details about your device
at this point?
Hm, I see this type being set usually either in a device tree source,
or in the support code for a specific platform. Are there any
situations where this is actually set by the driver? If I understand
right this is not the case for the PL330, but if it is possible that
it is the case for another device then I need to rethink this. Though
as far as I understand this should not be the case.
+
vdev-irq[i].count = 1;
vdev-irq[i].hwirq = hwirq;
+ vdev-irq[i].masked = false;
}
vdev-num_irqs = cnt;
@@ -77,11 +85,27 @@ void vfio_platform_irq_cleanup(struct
vfio_platform_device *vdev)
static irqreturn_t vfio_irq_handler(int irq, void *dev_id)
{
- struct eventfd_ctx *trigger = dev_id;
+ struct vfio_platform_irq *irq_ctx = dev_id;
+ unsigned long flags;
+ int ret = IRQ_NONE;
+
+ spin_lock_irqsave(irq_ctx-lock, flags);
+
+ if (!irq_ctx-masked) {
+ ret = IRQ_HANDLED;
+
+ if (irq_ctx-flags VFIO_IRQ_INFO_AUTOMASKED) {
+ disable_irq_nosync(irq_ctx-hwirq);
+ irq_ctx-masked = true;
+ }
+ }
- eventfd_signal(trigger, 1);
+ spin_unlock_irqrestore(irq_ctx-lock, flags);
- return IRQ_HANDLED;
+ if (ret == IRQ_HANDLED)
+ eventfd_signal(irq_ctx-trigger, 1);
+
+ return ret;
}
static int vfio_set_trigger(struct vfio_platform_device *vdev,
@@ -162,6 +186,82 @@ static int vfio_platform_set_irq_trigger(struct
vfio_platform_device *vdev,
return -EFAULT;
}
+static int vfio_platform_set_irq_unmask(struct vfio_platform_device *vdev,
+ unsigned index, unsigned start,
+ unsigned count, uint32_t flags, void *data)
+{
+ uint8_t arr;
arr?
arr for array! As in, the VFIO API allows an array of IRQs. However
for platform devices we don't use this, each IRQ is exposed
independently as an array of 1 IRQ.
+
+ if (start != 0 || count != 1)
+ return -EINVAL;
+
+ switch (flags VFIO_IRQ_SET_DATA_TYPE_MASK) {
+ case VFIO_IRQ_SET_DATA_BOOL:
+ if (copy_from_user(arr, data, sizeof(uint8_t)))
+ return -EFAULT;
+
+ if (arr != 0x1)
+ return -EINVAL;
why the fallthrough, what's this about?
The VFIO API allows to unmask/mask an array of IRQs, however with
platform devices we only have arrays of 1 IRQ (so not really arrays).
So if the user uses VFIO_IRQ_SET_DATA_BOOL, we need to check that arr
== 0x1. When that is the case, a fallthrough to the same code for
VFIO_IRQ_SET_DATA_NONE is safe.
If that is not readable enough, then I can add a comment or duplicate
the code that does the unmasking. I realize that if you don't know the
VFIO API well, then this can look confusing.
+
+ case VFIO_IRQ_SET_DATA_NONE:
+
+ spin_lock_irq(vdev-irq[index].lock);
+
+ if (vdev-irq[index].masked) {
+ enable_irq(vdev-irq[index].hwirq);
+ vdev-irq[index].masked = false;
+ }
+
+ spin_unlock_irq(vdev-irq[index].lock);
+
+ return 0;
+
+ case VFIO_IRQ_SET_DATA_EVENTFD: /* XXX not implemented yet */
+
Re: [RFC PATCH v6 14/20] vfio/platform: initial interrupts support
On Sun, Jun 8, 2014 at 12:09 PM, Christoffer Dall
christoffer.d...@linaro.org wrote:
On Thu, Jun 05, 2014 at 07:03:22PM +0200, Antonios Motakis wrote:
This patch allows to set an eventfd for a patform device's interrupt,
and also to trigger the interrupt eventfd from userspace for testing.
Signed-off-by: Antonios Motakis a.mota...@virtualopensystems.com
---
drivers/vfio/platform/vfio_platform.c | 36 ++-
drivers/vfio/platform/vfio_platform_irq.c | 130
+-
drivers/vfio/platform/vfio_platform_private.h | 7 ++
3 files changed, 169 insertions(+), 4 deletions(-)
diff --git a/drivers/vfio/platform/vfio_platform.c
b/drivers/vfio/platform/vfio_platform.c
index 192291c..f4c06c6 100644
--- a/drivers/vfio/platform/vfio_platform.c
+++ b/drivers/vfio/platform/vfio_platform.c
@@ -177,10 +177,40 @@ static long vfio_platform_ioctl(void *device_data,
return copy_to_user((void __user *)arg, info, minsz);
- } else if (cmd == VFIO_DEVICE_SET_IRQS)
- return -EINVAL;
+ } else if (cmd == VFIO_DEVICE_SET_IRQS) {
+ struct vfio_irq_set hdr;
+ int ret = 0;
+
+ minsz = offsetofend(struct vfio_irq_set, count);
+
+ if (copy_from_user(hdr, (void __user *)arg, minsz))
+ return -EFAULT;
+
+ if (hdr.argsz minsz)
+ return -EINVAL;
+
+ if (hdr.index = vdev-num_irqs)
+ return -EINVAL;
+
+ if (hdr.start != 0 || hdr.count 1)
+ return -EINVAL;
+
+ if (hdr.count == 0
+ (!(hdr.flags VFIO_IRQ_SET_DATA_NONE) ||
+ !(hdr.flags VFIO_IRQ_SET_ACTION_TRIGGER)))
+ return -EINVAL;
+
+ if (hdr.flags ~(VFIO_IRQ_SET_DATA_TYPE_MASK |
+ VFIO_IRQ_SET_ACTION_TYPE_MASK))
+ return -EINVAL;
+
+ ret = vfio_platform_set_irqs_ioctl(vdev, hdr.flags, hdr.index,
+hdr.start, hdr.count,
+(void *)arg+minsz);
+
+ return ret;
- else if (cmd == VFIO_DEVICE_RESET)
+ } else if (cmd == VFIO_DEVICE_RESET)
return -EINVAL;
return -ENOTTY;
diff --git a/drivers/vfio/platform/vfio_platform_irq.c
b/drivers/vfio/platform/vfio_platform_irq.c
index 22c214f..d79f5af 100644
--- a/drivers/vfio/platform/vfio_platform_irq.c
+++ b/drivers/vfio/platform/vfio_platform_irq.c
@@ -31,6 +31,9 @@
#include vfio_platform_private.h
+static int vfio_set_trigger(struct vfio_platform_device *vdev,
+ int index, int fd);
+
int vfio_platform_irq_init(struct vfio_platform_device *vdev)
{
int cnt = 0, i;
@@ -43,17 +46,142 @@ int vfio_platform_irq_init(struct vfio_platform_device
*vdev)
return -ENOMEM;
for (i = 0; i cnt; i++) {
- vdev-irq[i].flags = 0;
+ int hwirq = platform_get_irq(vdev-pdev, i);
+
+ if (hwirq 0)
+ goto err;
+
+ vdev-irq[i].flags = VFIO_IRQ_INFO_EVENTFD;
vdev-irq[i].count = 1;
+ vdev-irq[i].hwirq = hwirq;
}
vdev-num_irqs = cnt;
return 0;
+err:
+ kfree(vdev-irq);
+ return -EINVAL;
}
void vfio_platform_irq_cleanup(struct vfio_platform_device *vdev)
{
+ int i;
+
+ for (i = 0; i vdev-num_irqs; i++)
+ vfio_set_trigger(vdev, i, -1);
+
vdev-num_irqs = 0;
kfree(vdev-irq);
}
+
+static irqreturn_t vfio_irq_handler(int irq, void *dev_id)
+{
+ struct eventfd_ctx *trigger = dev_id;
+
+ eventfd_signal(trigger, 1);
+
+ return IRQ_HANDLED;
+}
+
+static int vfio_set_trigger(struct vfio_platform_device *vdev,
+ int index, int fd)
+{
+ struct vfio_platform_irq *irq = vdev-irq[index];
+ struct eventfd_ctx *trigger;
+ int ret;
+
+ if (irq-trigger) {
+ free_irq(irq-hwirq, irq);
+ kfree(irq-name);
+ eventfd_ctx_put(irq-trigger);
+ irq-trigger = NULL;
+ }
this feels incredibly racy, is some lock protecting this access?
Good catch; there should have been a mutex earlier protecting it, but
it is missing. Thanks.
-Christoffer
--
Antonios Motakis
Virtual Open Systems
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[RFC PATCH v2 6/7] arm: call iommu_init before of_platform_populate
We need to ensure that the IOMMUs in the system have a chance to perform
some basic initialisation before we start adding masters to them.
This patch adds a call to of_iommu_init before of_platform_populate.
Signed-off-by: Will Deacon will.dea...@arm.com
---
arch/arm/kernel/setup.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c
index 84db893dedc2..38a0203523af 100644
--- a/arch/arm/kernel/setup.c
+++ b/arch/arm/kernel/setup.c
@@ -18,6 +18,7 @@
#include linux/bootmem.h
#include linux/seq_file.h
#include linux/screen_info.h
+#include linux/of_iommu.h
#include linux/of_platform.h
#include linux/init.h
#include linux/kexec.h
@@ -803,9 +804,11 @@ static int __init customize_machine(void)
if (machine_desc-init_machine)
machine_desc-init_machine();
#ifdef CONFIG_OF
- else
+ else {
+ of_iommu_init();
of_platform_populate(NULL, of_default_bus_match_table,
NULL, NULL);
+ }
#endif
return 0;
}
--
2.1.0
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[RFC PATCH v2 3/7] iommu: add new iommu_ops callback for adding an OF device
This patch adds a new function to the iommu_ops structure to allow an
OF device to be added to a specific IOMMU instance using the recently
merged generic devicetree binding for IOMMUs. The callback (of_xlate)
takes a struct device representing the master and an of_phandle_args
representing the IOMMU and the correspondong IDs for the new master.
Signed-off-by: Will Deacon will.dea...@arm.com
---
include/linux/iommu.h | 5 +
1 file changed, 5 insertions(+)
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index fdddb14cd8f5..3e766b85daa3 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -21,6 +21,7 @@
#include linux/errno.h
#include linux/err.h
+#include linux/of.h
#include linux/types.h
#include trace/events/iommu.h
@@ -136,6 +137,10 @@ struct iommu_ops {
/* Get the numer of window per domain */
u32 (*domain_get_windows)(struct iommu_domain *domain);
+#ifdef CONFIG_OF_IOMMU
+ int (*of_xlate)(struct device *dev, struct of_phandle_args *args);
+#endif
+
unsigned long pgsize_bitmap;
};
--
2.1.0
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[RFC PATCH v2 7/7] arm: dma-mapping: plumb our iommu mapping ops into arch_setup_dma_ops
This patch plumbs the existing ARM IOMMU DMA infrastructure (which isn't
actually called outside of a few drivers) into arch_setup_dma_ops, so
that we can use IOMMUs for DMA transfers in a more generic fashion.
Since this significantly complicates the arch_setup_dma_ops function,
it is moved out of line into dma-mapping.c. If CONFIG_ARM_DMA_USE_IOMMU
is not set, the iommu paramater is ignored and the normal ops are used
instead.
Signed-off-by: Will Deacon will.dea...@arm.com
---
arch/arm/include/asm/dma-mapping.h | 17 +++--
arch/arm/mm/dma-mapping.c | 72 +-
2 files changed, 68 insertions(+), 21 deletions(-)
diff --git a/arch/arm/include/asm/dma-mapping.h
b/arch/arm/include/asm/dma-mapping.h
index 788b8eef40e8..2f648b9b000d 100644
--- a/arch/arm/include/asm/dma-mapping.h
+++ b/arch/arm/include/asm/dma-mapping.h
@@ -121,20 +121,11 @@ static inline unsigned long dma_max_pfn(struct device
*dev)
}
#define dma_max_pfn(dev) dma_max_pfn(dev)
-static inline void arch_setup_dma_ops(struct device *dev, u64 mask,
- u64 dma_base, u64 size,
- unsigned long offset,
- struct iommu_dma_mapping *iommu,
- bool coherent)
-{
- dev-coherent_dma_mask = mask;
- dev-dma_mask = dev-coherent_dma_mask;
- dev-dma_pfn_offset = offset;
-
- if (coherent)
- set_dma_ops(dev, arm_coherent_dma_ops);
-}
#define arch_setup_dma_ops arch_setup_dma_ops
+extern void arch_setup_dma_ops(struct device *dev, u64 mask, u64 dma_base,
+ u64 size, unsigned long offset,
+ struct iommu_dma_mapping *iommu,
+ bool coherent);
static inline dma_addr_t phys_to_dma(struct device *dev, phys_addr_t paddr)
{
diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
index 7a996aaa061e..07179b2003eb 100644
--- a/arch/arm/mm/dma-mapping.c
+++ b/arch/arm/mm/dma-mapping.c
@@ -2041,10 +2041,9 @@ EXPORT_SYMBOL_GPL(arm_iommu_release_mapping);
* @mapping: io address space mapping structure (returned from
* arm_iommu_create_mapping)
*
- * Attaches specified io address space mapping to the provided device,
- * this replaces the dma operations (dma_map_ops pointer) with the
- * IOMMU aware version. More than one client might be attached to
- * the same io address space mapping.
+ * Attaches specified io address space mapping to the provided device.
+ * More than one client might be attached to the same io address space
+ * mapping.
*/
int arm_iommu_attach_device(struct device *dev,
struct dma_iommu_mapping *mapping)
@@ -2057,7 +2056,6 @@ int arm_iommu_attach_device(struct device *dev,
kref_get(mapping-kref);
dev-archdata.mapping = mapping;
- set_dma_ops(dev, iommu_ops);
pr_debug(Attached IOMMU controller to %s device.\n, dev_name(dev));
return 0;
@@ -2069,7 +2067,6 @@ EXPORT_SYMBOL_GPL(arm_iommu_attach_device);
* @dev: valid struct device pointer
*
* Detaches the provided device from a previously attached map.
- * This voids the dma operations (dma_map_ops pointer)
*/
void arm_iommu_detach_device(struct device *dev)
{
@@ -2084,10 +2081,69 @@ void arm_iommu_detach_device(struct device *dev)
iommu_detach_device(mapping-domain, dev);
kref_put(mapping-kref, release_iommu_mapping);
dev-archdata.mapping = NULL;
- set_dma_ops(dev, NULL);
pr_debug(Detached IOMMU controller from %s device.\n, dev_name(dev));
}
EXPORT_SYMBOL_GPL(arm_iommu_detach_device);
-#endif
+static bool arm_setup_iommu_dma_ops(struct device *dev, u64 dma_base, u64 size)
+{
+ struct dma_iommu_mapping *mapping;
+
+ mapping = arm_iommu_create_mapping(dev-bus, dma_base, size);
+ if (IS_ERR(mapping)) {
+ pr_warn(Failed to create %llu-byte IOMMU mapping for device
%s\n,
+ size, dev_name(dev));
+ return false;
+ }
+
+ if (arm_iommu_attach_device(dev, mapping)) {
+ pr_warn(Failed to attach device %s to IOMMU mapping\n,
+ dev_name(dev));
+ arm_iommu_release_mapping(mapping);
+ return false;
+ }
+
+ return true;
+}
+
+static struct dma_map_ops *arm_get_iommu_dma_map_ops(bool coherent)
+{
+ return coherent ? iommu_coherent_ops : iommu_ops;
+}
+
+#else
+
+static bool arm_setup_iommu_dma_ops(struct device *dev, u64 dma_base, u64 size)
+{
+ return false;
+}
+
+#define arm_get_iommu_dma_map_ops arm_get_dma_map_ops
+
+#endif /* CONFIG_ARM_DMA_USE_IOMMU */
+
+static struct dma_map_ops *arm_get_dma_map_ops(bool coherent)
+{
+ return coherent ? arm_coherent_dma_ops : arm_dma_ops;
+}
+
+void arch_setup_dma_ops(struct device *dev, u64 mask, u64 dma_base, u64
[RFC PATCH v2 2/7] dma-mapping: replace set_arch_dma_coherent_ops with arch_setup_dma_ops
set_arch_dma_coherent_ops is called from of_dma_configure in order to
swizzle the architectural dma-mapping functions over to a cache-coherent
implementation. This is currently implemented only for ARM.
In anticipation of re-using this mechanism for IOMMU-backed dma-mapping
ops too, this patch replaces the function with a broader
arch_setup_dma_ops callback which is also responsible for setting the
DMA mask and offset as well as selecting the correct mapping functions.
A further advantage of this split is that it nicely isolates the
of-specific code from the dma-mapping code, allowing potential reuse by
other buses (e.g. PCI) in the future.
Signed-off-by: Will Deacon will.dea...@arm.com
---
arch/arm/include/asm/dma-mapping.h | 13
drivers/of/platform.c | 42 ++
include/linux/dma-mapping.h| 8 +++-
3 files changed, 23 insertions(+), 40 deletions(-)
diff --git a/arch/arm/include/asm/dma-mapping.h
b/arch/arm/include/asm/dma-mapping.h
index c45b61a4b4a5..dad006dabbe6 100644
--- a/arch/arm/include/asm/dma-mapping.h
+++ b/arch/arm/include/asm/dma-mapping.h
@@ -121,12 +121,17 @@ static inline unsigned long dma_max_pfn(struct device
*dev)
}
#define dma_max_pfn(dev) dma_max_pfn(dev)
-static inline int set_arch_dma_coherent_ops(struct device *dev)
+static inline void arch_setup_dma_ops(struct device *dev, u64 mask,
+ unsigned long offset, bool coherent)
{
- set_dma_ops(dev, arm_coherent_dma_ops);
- return 0;
+ dev-coherent_dma_mask = mask;
+ dev-dma_mask = dev-coherent_dma_mask;
+ dev-dma_pfn_offset = offset;
+
+ if (coherent)
+ set_dma_ops(dev, arm_coherent_dma_ops);
}
-#define set_arch_dma_coherent_ops(dev) set_arch_dma_coherent_ops(dev)
+#define arch_setup_dma_ops arch_setup_dma_ops
static inline dma_addr_t phys_to_dma(struct device *dev, phys_addr_t paddr)
{
diff --git a/drivers/of/platform.c b/drivers/of/platform.c
index 0197725e033a..484c558c63a6 100644
--- a/drivers/of/platform.c
+++ b/drivers/of/platform.c
@@ -164,43 +164,23 @@ static void of_dma_configure(struct platform_device *pdev)
{
u64 dma_addr, paddr, size;
int ret;
+ bool coherent;
+ unsigned long offset;
struct device *dev = pdev-dev;
- /*
-* Set default dma-mask to 32 bit. Drivers are expected to setup
-* the correct supported dma_mask.
-*/
- dev-coherent_dma_mask = DMA_BIT_MASK(32);
-
- /*
-* Set it to coherent_dma_mask by default if the architecture
-* code has not set it.
-*/
- if (!dev-dma_mask)
- dev-dma_mask = dev-coherent_dma_mask;
+ ret = of_dma_get_range(dev-of_node, dma_addr, paddr, size);
+ offset = ret 0 ? 0 : PFN_DOWN(paddr - dma_addr);
+ dev_dbg(dev, dma_pfn_offset(%#08lx)\n, dev-dma_pfn_offset);
- /*
-* if dma-coherent property exist, call arch hook to setup
-* dma coherent operations.
-*/
- if (of_dma_is_coherent(dev-of_node)) {
- set_arch_dma_coherent_ops(dev);
- dev_dbg(dev, device is dma coherent\n);
- }
+ coherent = of_dma_is_coherent(dev-of_node);
+ dev_dbg(dev, device is%sdma coherent\n,
+ coherent ? : not );
/*
-* if dma-ranges property doesn't exist - just return else
-* setup the dma offset
+* Set default dma-mask to 32 bit. Drivers are expected to setup
+* the correct supported dma_mask.
*/
- ret = of_dma_get_range(dev-of_node, dma_addr, paddr, size);
- if (ret 0) {
- dev_dbg(dev, no dma range information to setup\n);
- return;
- }
-
- /* DMA ranges found. Calculate and set dma_pfn_offset */
- dev-dma_pfn_offset = PFN_DOWN(paddr - dma_addr);
- dev_dbg(dev, dma_pfn_offset(%#08lx)\n, dev-dma_pfn_offset);
+ arch_setup_dma_ops(dev, DMA_BIT_MASK(32), offset, coherent);
}
/**
diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
index 931b70986272..0f7f7b68b0db 100644
--- a/include/linux/dma-mapping.h
+++ b/include/linux/dma-mapping.h
@@ -129,11 +129,9 @@ static inline int dma_coerce_mask_and_coherent(struct
device *dev, u64 mask)
extern u64 dma_get_required_mask(struct device *dev);
-#ifndef set_arch_dma_coherent_ops
-static inline int set_arch_dma_coherent_ops(struct device *dev)
-{
- return 0;
-}
+#ifndef arch_setup_dma_ops
+static inline void arch_setup_dma_ops(struct device *dev, u64 mask,
+ unsigned long offset, bool coherent) { }
#endif
static inline unsigned int dma_get_max_seg_size(struct device *dev)
--
2.1.0
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC PATCH v2 6/7] arm: call iommu_init before of_platform_populate
On Tuesday 02 September 2014 18:56:26 Will Deacon wrote:
@@ -803,9 +804,11 @@ static int __init customize_machine(void)
if (machine_desc-init_machine)
machine_desc-init_machine();
#ifdef CONFIG_OF
- else
+ else {
+ of_iommu_init();
of_platform_populate(NULL, of_default_bus_match_table,
NULL, NULL);
+ }
#endif
return 0;
}
I think it would be better to not have this depend on the presence
of an init_machine callback. Why not move of_iommu_init() in front
of the if()? It shouldn't do anything unless we have an iommu
driver that we should be using.
Arnd
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC PATCH v2 7/7] arm: dma-mapping: plumb our iommu mapping ops into arch_setup_dma_ops
On Tuesday 02 September 2014 18:56:27 Will Deacon wrote: This patch plumbs the existing ARM IOMMU DMA infrastructure (which isn't actually called outside of a few drivers) into arch_setup_dma_ops, so that we can use IOMMUs for DMA transfers in a more generic fashion. Since this significantly complicates the arch_setup_dma_ops function, it is moved out of line into dma-mapping.c. If CONFIG_ARM_DMA_USE_IOMMU is not set, the iommu paramater is ignored and the normal ops are used instead. Signed-off-by: Will Deacon will.dea...@arm.com Looks great to me. Arnd ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC PATCH v2 4/7] iommu: provide helper function to configure an IOMMU for an of master
On Tuesday 02 September 2014 18:56:24 Will Deacon wrote:
+struct iommu_dma_mapping *of_iommu_configure(struct device *dev)
+{
+ struct of_phandle_args iommu_spec;
+ struct iommu_dma_mapping *mapping;
+ struct bus_type *bus = dev-bus;
+ const struct iommu_ops *ops = bus-iommu_ops;
I think it would be best to not even introduce the tight coupling
between bus_type and iommu_ops here, one of the main reasons we
are doing this is to break that connection.
Better put the iommu_ops into the iommu_data pointer that gets looked
up per iommu device.
+ struct iommu_data *iommu = NULL;
+ int idx = 0;
+
+ if (!iommu_present(bus) || !ops-of_xlate)
+ return NULL;
+
+ /*
+* We don't currently walk up the tree looking for a parent IOMMU.
+* See the `Notes:' section of
+* Documentation/devicetree/bindings/iommu/iommu.txt
+*/
+*/
+ while (!of_parse_phandle_with_args(dev-of_node, iommus,
+ #iommu-cells, idx,
+ iommu_spec)) {
+ struct device_node *np = iommu_spec.np;
+ struct iommu_data *data = of_iommu_get_data(np);
+
+ if (!iommu) {
+ if (!ops-of_xlate(dev, iommu_spec))
+ iommu = data;
I would make the first argument of the of_xlate function the iommu_data,
so the code can find the right instance.
Maybe also add an extra argument at the end that can be used by the
PCI code and potentially other drivers with multiple master IDs
behind one iommus property to pass some value identifying which of
the masters is meant.
The format of that is unfortunately bus specific and our platform_bus
really covers a number of different hardware buses (AXI, AHB, OPB, ...)
but the caller should be able to provide the data in the right form
that the iommu understands. I would try using a single u64 argument
as a start, hoping that this covers all the buses we need. At least
it's enough for PCI (bus/device/function) and for AXI (requester-id?).
+ } else if (iommu != data) {
+ pr_warn(Rejecting device %s with multiple IOMMU
instances\n,
+ dev_name(dev));
+ iommu = NULL;
+ }
+
+ of_node_put(np);
+
+ if (!iommu)
+ break;
+
+ idx++;
+ }
+
+ if (!iommu)
+ return NULL;
+
+ mapping = devm_kzalloc(dev, sizeof(*mapping), GFP_KERNEL);
+ if (!mapping)
+ return NULL;
I don't think it's safe to use devm_* functions here: this is during
device discovery, and this data will be freed if probe() fails or
the device gets removed from a driver.
Arnd
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC PATCH v2 0/7] Introduce automatic DMA configuration for IOMMU masters
On Tuesday 02 September 2014 18:56:20 Will Deacon wrote: The main difference since v1 is that I've introduced some generic structures to represent IOMMUs and their mappings (struct iommu_data and struct iommu_dma_mapping). This allows us to propagate the per-instance data all the way down to the DMA-mapping code, which can then manage a per-instance domain. Note that the ARM dma-mapping implementation does not currently make use of this information. Overall I'm pretty happy with how this is turning out so far. I have commented on a few patches, everything else looks good to me. Arnd ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC PATCH 4/7] iommu: provide helper function to configure an IOMMU for an of master
On Tue, Sep 02, 2014 at 04:01:32PM +0200, Arnd Bergmann wrote: This is an artifact of the API being single-instance at the moment. We might not in fact need it, I was just trying to think of things that naturally fit in there and that are probably already linked together in the individual iommu drivers. I am not sure what you mean by single-instance. Is it that currently the API only supports one type of iommu_ops per bus? That should be fine as long as there is only one type of IOMMU on the bus. Besides that, it is a feature of the IOMMU-API to hide the details about all the hardware IOMMUs in the system from its users. Joerg ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC PATCH 0/7] Introduce automatic DMA configuration for IOMMU masters
On Tuesday 02 September 2014 14:43:18 Arnd Bergmann wrote: On Tuesday 02 September 2014 14:24:18 Marek Szyprowski wrote: For isolation purposes, it can only help to have more domains, but I would guess that there is some space overhead in maintaining lots of page tables. I'm okay with both approaches (separate domain for each device vs. single common domain for all devices). Maybe this can be some kind of Kconfig option added to DMA debugging? Separation might be really helpful when debugging strange device behavior. One potential problem with a single domain is when you have multiple instances of a given IOMMU, each with different hardware restrictions. Then you can end up with multiple sets of page tables for the domain which, although not impossible to work with, is a bit of a mess. Maybe the default dma-mapping domain should be one per a given IOMMU instance? This will simplify a lot of things in such case. Yes, that sounds like a good idea. That would work as a default configuration for the Renesas IPMMU IOMMU, which supports four TLBs to be shared between more than four bus masters. Ideally I'd like to make the bus master to TLB association somehow configurable, but associating all bus masters with a single TLB as a first step is fine. -- Regards, Laurent Pinchart ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
