[PATCH 0/5] Implement SMMU passthrough using the default domain

[PATCH 0/5] Implement SMMU passthrough using the default domain

[Will Deacon]

Hi all,

A number of people have expressed interest in having the SMMU come up in
a passthrough configuration, and then allow subsequent translation for
things such as VFIO. Rather than do this in each SMMU driver, it's much
cleaner to allow the default domain to be configured to be something other
than DMA.

This patch series implements a command-line option to configure the
default domain type. Currently, it supports "dma" and "identity" which
is sufficient for the passthrough use-case.

Tested on an ARM fastmodel.

All feedback welcome,

Will

--->8

Will Deacon (5):
  iommu/arm-smmu: Restrict domain attributes to UNMANAGED domains
  iommu/arm-smmu: Install bypass S2CRs for IOMMU_DOMAIN_IDENTITY domains
  iommu/arm-smmu-v3: Install bypass STEs for IOMMU_DOMAIN_IDENTITY
    domains
  arm64: dma-mapping: Only swizzle DMA ops for IOMMU_DOMAIN_DMA
  iommu: Allow default domain type to be set on the kernel command line

 arch/arm64/mm/dma-mapping.c | 17 ++++++++++++-----
 drivers/iommu/arm-smmu-v3.c | 20 ++++++++++++++++++--
 drivers/iommu/arm-smmu.c    | 26 +++++++++++++++++++++++---
 drivers/iommu/iommu.c       | 19 +++++++++++++++++--
 4 files changed, 70 insertions(+), 12 deletions(-)

-- 
2.1.4

[PATCH 0/5] Implement SMMU passthrough using the default domain

[Will Deacon]

Hi all,

A number of people have expressed interest in having the SMMU come up in
a passthrough configuration, and then allow subsequent translation for
things such as VFIO. Rather than do this in each SMMU driver, it's much
cleaner to allow the default domain to be configured to be something other
than DMA.

This patch series implements a command-line option to configure the
default domain type. Currently, it supports "dma" and "identity" which
is sufficient for the passthrough use-case.

Tested on an ARM fastmodel.

All feedback welcome,

Will

--->8

Will Deacon (5):
  iommu/arm-smmu: Restrict domain attributes to UNMANAGED domains
  iommu/arm-smmu: Install bypass S2CRs for IOMMU_DOMAIN_IDENTITY domains
  iommu/arm-smmu-v3: Install bypass STEs for IOMMU_DOMAIN_IDENTITY
    domains
  arm64: dma-mapping: Only swizzle DMA ops for IOMMU_DOMAIN_DMA
  iommu: Allow default domain type to be set on the kernel command line

 arch/arm64/mm/dma-mapping.c | 17 ++++++++++++-----
 drivers/iommu/arm-smmu-v3.c | 20 ++++++++++++++++++--
 drivers/iommu/arm-smmu.c    | 26 +++++++++++++++++++++++---
 drivers/iommu/iommu.c       | 19 +++++++++++++++++--
 4 files changed, 70 insertions(+), 12 deletions(-)

-- 
2.1.4

[PATCH 1/5] iommu/arm-smmu: Restrict domain attributes to UNMANAGED domains

[Will Deacon]

The ARM SMMU drivers provide a DOMAIN_ATTR_NESTING domain attribute,
which allows callers of the IOMMU API to request that the page table
for a domain is installed at stage-2, if supported by the hardware.

Since setting this attribute only makes sense for UNMANAGED domains,
this patch returns -ENODEV if the domain_{get,set}_attr operations are
called on other domain types.

Signed-off-by: Will Deacon <will.deacon-5wv7dgnIgG8@public.gmane.org>
---
 drivers/iommu/arm-smmu-v3.c | 6 ++++++
 drivers/iommu/arm-smmu.c    | 6 ++++++
 2 files changed, 12 insertions(+)

diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index 4d6ec444a9d6..c254325b0c7a 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -1839,6 +1839,9 @@ static int arm_smmu_domain_get_attr(struct iommu_domain *domain,
 {
 	struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
 
+	if (domain->type != IOMMU_DOMAIN_UNMANAGED)
+		return -ENODEV;
+
 	switch (attr) {
 	case DOMAIN_ATTR_NESTING:
 		*(int *)data = (smmu_domain->stage == ARM_SMMU_DOMAIN_NESTED);
@@ -1854,6 +1857,9 @@ static int arm_smmu_domain_set_attr(struct iommu_domain *domain,
 	int ret = 0;
 	struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
 
+	if (domain->type != IOMMU_DOMAIN_UNMANAGED)
+		return -ENODEV;
+
 	mutex_lock(&smmu_domain->init_mutex);
 
 	switch (attr) {
diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index a60cded8a6ed..a328ffb75509 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -1497,6 +1497,9 @@ static int arm_smmu_domain_get_attr(struct iommu_domain *domain,
 {
 	struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
 
+	if (domain->type != IOMMU_DOMAIN_UNMANAGED)
+		return -ENODEV;
+
 	switch (attr) {
 	case DOMAIN_ATTR_NESTING:
 		*(int *)data = (smmu_domain->stage == ARM_SMMU_DOMAIN_NESTED);
@@ -1512,6 +1515,9 @@ static int arm_smmu_domain_set_attr(struct iommu_domain *domain,
 	int ret = 0;
 	struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
 
+	if (domain->type != IOMMU_DOMAIN_UNMANAGED)
+		return -ENODEV;
+
 	mutex_lock(&smmu_domain->init_mutex);
 
 	switch (attr) {
-- 
2.1.4

[PATCH 1/5] iommu/arm-smmu: Restrict domain attributes to UNMANAGED domains

[Will Deacon]

The ARM SMMU drivers provide a DOMAIN_ATTR_NESTING domain attribute,
which allows callers of the IOMMU API to request that the page table
for a domain is installed at stage-2, if supported by the hardware.

Since setting this attribute only makes sense for UNMANAGED domains,
this patch returns -ENODEV if the domain_{get,set}_attr operations are
called on other domain types.

Signed-off-by: Will Deacon <will.deacon@arm.com>
---
 drivers/iommu/arm-smmu-v3.c | 6 ++++++
 drivers/iommu/arm-smmu.c    | 6 ++++++
 2 files changed, 12 insertions(+)

diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index 4d6ec444a9d6..c254325b0c7a 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -1839,6 +1839,9 @@ static int arm_smmu_domain_get_attr(struct iommu_domain *domain,
 {
 	struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
 
+	if (domain->type != IOMMU_DOMAIN_UNMANAGED)
+		return -ENODEV;
+
 	switch (attr) {
 	case DOMAIN_ATTR_NESTING:
 		*(int *)data = (smmu_domain->stage == ARM_SMMU_DOMAIN_NESTED);
@@ -1854,6 +1857,9 @@ static int arm_smmu_domain_set_attr(struct iommu_domain *domain,
 	int ret = 0;
 	struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
 
+	if (domain->type != IOMMU_DOMAIN_UNMANAGED)
+		return -ENODEV;
+
 	mutex_lock(&smmu_domain->init_mutex);
 
 	switch (attr) {
diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index a60cded8a6ed..a328ffb75509 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -1497,6 +1497,9 @@ static int arm_smmu_domain_get_attr(struct iommu_domain *domain,
 {
 	struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
 
+	if (domain->type != IOMMU_DOMAIN_UNMANAGED)
+		return -ENODEV;
+
 	switch (attr) {
 	case DOMAIN_ATTR_NESTING:
 		*(int *)data = (smmu_domain->stage == ARM_SMMU_DOMAIN_NESTED);
@@ -1512,6 +1515,9 @@ static int arm_smmu_domain_set_attr(struct iommu_domain *domain,
 	int ret = 0;
 	struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
 
+	if (domain->type != IOMMU_DOMAIN_UNMANAGED)
+		return -ENODEV;
+
 	mutex_lock(&smmu_domain->init_mutex);
 
 	switch (attr) {
-- 
2.1.4

[PATCH 2/5] iommu/arm-smmu: Install bypass S2CRs for IOMMU_DOMAIN_IDENTITY domains

[Will Deacon]

In preparation for allowing the default domain type to be overridden,
this patch adds support for IOMMU_DOMAIN_IDENTITY domains to the
ARM SMMU driver.

An identity domain is created by placing the corresponding S2CR
registers into "bypass" mode, which allows transactions to flow through
the SMMU without any translation.

Signed-off-by: Will Deacon <will.deacon-5wv7dgnIgG8@public.gmane.org>
---
 drivers/iommu/arm-smmu.c | 20 +++++++++++++++++---
 1 file changed, 17 insertions(+), 3 deletions(-)

diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index a328ffb75509..0f5e42a719e5 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -404,6 +404,7 @@ enum arm_smmu_domain_stage {
 	ARM_SMMU_DOMAIN_S1 = 0,
 	ARM_SMMU_DOMAIN_S2,
 	ARM_SMMU_DOMAIN_NESTED,
+	ARM_SMMU_DOMAIN_BYPASS,
 };
 
 struct arm_smmu_domain {
@@ -824,6 +825,12 @@ static int arm_smmu_init_domain_context(struct iommu_domain *domain,
 	if (smmu_domain->smmu)
 		goto out_unlock;
 
+	if (domain->type == IOMMU_DOMAIN_IDENTITY) {
+		smmu_domain->stage = ARM_SMMU_DOMAIN_BYPASS;
+		smmu_domain->smmu = smmu;
+		goto out_unlock;
+	}
+
 	/*
 	 * Mapping the requested stage onto what we support is surprisingly
 	 * complicated, mainly because the spec allows S1+S2 SMMUs without
@@ -984,7 +991,7 @@ static void arm_smmu_destroy_domain_context(struct iommu_domain *domain)
 	void __iomem *cb_base;
 	int irq;
 
-	if (!smmu)
+	if (!smmu || domain->type == IOMMU_DOMAIN_IDENTITY)
 		return;
 
 	/*
@@ -1007,7 +1014,9 @@ static struct iommu_domain *arm_smmu_domain_alloc(unsigned type)
 {
 	struct arm_smmu_domain *smmu_domain;
 
-	if (type != IOMMU_DOMAIN_UNMANAGED && type != IOMMU_DOMAIN_DMA)
+	if (type != IOMMU_DOMAIN_UNMANAGED &&
+	    type != IOMMU_DOMAIN_DMA &&
+	    type != IOMMU_DOMAIN_IDENTITY)
 		return NULL;
 	/*
 	 * Allocate the domain and initialise some of its data structures.
@@ -1205,10 +1214,15 @@ static int arm_smmu_domain_add_master(struct arm_smmu_domain *smmu_domain,
 {
 	struct arm_smmu_device *smmu = smmu_domain->smmu;
 	struct arm_smmu_s2cr *s2cr = smmu->s2crs;
-	enum arm_smmu_s2cr_type type = S2CR_TYPE_TRANS;
 	u8 cbndx = smmu_domain->cfg.cbndx;
+	enum arm_smmu_s2cr_type type;
 	int i, idx;
 
+	if (smmu_domain->stage == ARM_SMMU_DOMAIN_BYPASS)
+		type = S2CR_TYPE_BYPASS;
+	else
+		type = S2CR_TYPE_TRANS;
+
 	for_each_cfg_sme(fwspec, i, idx) {
 		if (type == s2cr[idx].type && cbndx == s2cr[idx].cbndx)
 			continue;
-- 
2.1.4

[PATCH 2/5] iommu/arm-smmu: Install bypass S2CRs for IOMMU_DOMAIN_IDENTITY domains

[Will Deacon]

In preparation for allowing the default domain type to be overridden,
this patch adds support for IOMMU_DOMAIN_IDENTITY domains to the
ARM SMMU driver.

An identity domain is created by placing the corresponding S2CR
registers into "bypass" mode, which allows transactions to flow through
the SMMU without any translation.

Signed-off-by: Will Deacon <will.deacon@arm.com>
---
 drivers/iommu/arm-smmu.c | 20 +++++++++++++++++---
 1 file changed, 17 insertions(+), 3 deletions(-)

diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index a328ffb75509..0f5e42a719e5 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -404,6 +404,7 @@ enum arm_smmu_domain_stage {
 	ARM_SMMU_DOMAIN_S1 = 0,
 	ARM_SMMU_DOMAIN_S2,
 	ARM_SMMU_DOMAIN_NESTED,
+	ARM_SMMU_DOMAIN_BYPASS,
 };
 
 struct arm_smmu_domain {
@@ -824,6 +825,12 @@ static int arm_smmu_init_domain_context(struct iommu_domain *domain,
 	if (smmu_domain->smmu)
 		goto out_unlock;
 
+	if (domain->type == IOMMU_DOMAIN_IDENTITY) {
+		smmu_domain->stage = ARM_SMMU_DOMAIN_BYPASS;
+		smmu_domain->smmu = smmu;
+		goto out_unlock;
+	}
+
 	/*
 	 * Mapping the requested stage onto what we support is surprisingly
 	 * complicated, mainly because the spec allows S1+S2 SMMUs without
@@ -984,7 +991,7 @@ static void arm_smmu_destroy_domain_context(struct iommu_domain *domain)
 	void __iomem *cb_base;
 	int irq;
 
-	if (!smmu)
+	if (!smmu || domain->type == IOMMU_DOMAIN_IDENTITY)
 		return;
 
 	/*
@@ -1007,7 +1014,9 @@ static struct iommu_domain *arm_smmu_domain_alloc(unsigned type)
 {
 	struct arm_smmu_domain *smmu_domain;
 
-	if (type != IOMMU_DOMAIN_UNMANAGED && type != IOMMU_DOMAIN_DMA)
+	if (type != IOMMU_DOMAIN_UNMANAGED &&
+	    type != IOMMU_DOMAIN_DMA &&
+	    type != IOMMU_DOMAIN_IDENTITY)
 		return NULL;
 	/*
 	 * Allocate the domain and initialise some of its data structures.
@@ -1205,10 +1214,15 @@ static int arm_smmu_domain_add_master(struct arm_smmu_domain *smmu_domain,
 {
 	struct arm_smmu_device *smmu = smmu_domain->smmu;
 	struct arm_smmu_s2cr *s2cr = smmu->s2crs;
-	enum arm_smmu_s2cr_type type = S2CR_TYPE_TRANS;
 	u8 cbndx = smmu_domain->cfg.cbndx;
+	enum arm_smmu_s2cr_type type;
 	int i, idx;
 
+	if (smmu_domain->stage == ARM_SMMU_DOMAIN_BYPASS)
+		type = S2CR_TYPE_BYPASS;
+	else
+		type = S2CR_TYPE_TRANS;
+
 	for_each_cfg_sme(fwspec, i, idx) {
 		if (type == s2cr[idx].type && cbndx == s2cr[idx].cbndx)
 			continue;
-- 
2.1.4

[PATCH 3/5] iommu/arm-smmu-v3: Install bypass STEs for IOMMU_DOMAIN_IDENTITY domains

[Will Deacon]

In preparation for allowing the default domain type to be overridden,
this patch adds support for IOMMU_DOMAIN_IDENTITY domains to the
ARM SMMUv3 driver.

An identity domain is created by placing the corresponding stream table
entries into "bypass" mode, which allows transactions to flow through
the SMMU without any translation.

Signed-off-by: Will Deacon <will.deacon-5wv7dgnIgG8@public.gmane.org>
---
 drivers/iommu/arm-smmu-v3.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index c254325b0c7a..d33291274455 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -629,6 +629,7 @@ enum arm_smmu_domain_stage {
 	ARM_SMMU_DOMAIN_S1 = 0,
 	ARM_SMMU_DOMAIN_S2,
 	ARM_SMMU_DOMAIN_NESTED,
+	ARM_SMMU_DOMAIN_BYPASS,
 };
 
 struct arm_smmu_domain {
@@ -1385,7 +1386,9 @@ static struct iommu_domain *arm_smmu_domain_alloc(unsigned type)
 {
 	struct arm_smmu_domain *smmu_domain;
 
-	if (type != IOMMU_DOMAIN_UNMANAGED && type != IOMMU_DOMAIN_DMA)
+	if (type != IOMMU_DOMAIN_UNMANAGED &&
+	    type != IOMMU_DOMAIN_DMA &&
+	    type != IOMMU_DOMAIN_IDENTITY)
 		return NULL;
 
 	/*
@@ -1516,6 +1519,11 @@ static int arm_smmu_domain_finalise(struct iommu_domain *domain)
 	struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
 	struct arm_smmu_device *smmu = smmu_domain->smmu;
 
+	if (domain->type == IOMMU_DOMAIN_IDENTITY) {
+		smmu_domain->stage = ARM_SMMU_DOMAIN_BYPASS;
+		return 0;
+	}
+
 	/* Restrict the stage to what we can actually support */
 	if (!(smmu->features & ARM_SMMU_FEAT_TRANS_S1))
 		smmu_domain->stage = ARM_SMMU_DOMAIN_S2;
@@ -1651,7 +1659,9 @@ static int arm_smmu_attach_dev(struct iommu_domain *domain, struct device *dev)
 	ste->bypass = false;
 	ste->valid = true;
 
-	if (smmu_domain->stage == ARM_SMMU_DOMAIN_S1) {
+	if (smmu_domain->stage == ARM_SMMU_DOMAIN_BYPASS) {
+		ste->bypass = true;
+	} else if (smmu_domain->stage == ARM_SMMU_DOMAIN_S1) {
 		ste->s1_cfg = &smmu_domain->s1_cfg;
 		ste->s2_cfg = NULL;
 		arm_smmu_write_ctx_desc(smmu, ste->s1_cfg);
-- 
2.1.4

[PATCH 3/5] iommu/arm-smmu-v3: Install bypass STEs for IOMMU_DOMAIN_IDENTITY domains

[Will Deacon]

In preparation for allowing the default domain type to be overridden,
this patch adds support for IOMMU_DOMAIN_IDENTITY domains to the
ARM SMMUv3 driver.

An identity domain is created by placing the corresponding stream table
entries into "bypass" mode, which allows transactions to flow through
the SMMU without any translation.

Signed-off-by: Will Deacon <will.deacon@arm.com>
---
 drivers/iommu/arm-smmu-v3.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index c254325b0c7a..d33291274455 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -629,6 +629,7 @@ enum arm_smmu_domain_stage {
 	ARM_SMMU_DOMAIN_S1 = 0,
 	ARM_SMMU_DOMAIN_S2,
 	ARM_SMMU_DOMAIN_NESTED,
+	ARM_SMMU_DOMAIN_BYPASS,
 };
 
 struct arm_smmu_domain {
@@ -1385,7 +1386,9 @@ static struct iommu_domain *arm_smmu_domain_alloc(unsigned type)
 {
 	struct arm_smmu_domain *smmu_domain;
 
-	if (type != IOMMU_DOMAIN_UNMANAGED && type != IOMMU_DOMAIN_DMA)
+	if (type != IOMMU_DOMAIN_UNMANAGED &&
+	    type != IOMMU_DOMAIN_DMA &&
+	    type != IOMMU_DOMAIN_IDENTITY)
 		return NULL;
 
 	/*
@@ -1516,6 +1519,11 @@ static int arm_smmu_domain_finalise(struct iommu_domain *domain)
 	struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
 	struct arm_smmu_device *smmu = smmu_domain->smmu;
 
+	if (domain->type == IOMMU_DOMAIN_IDENTITY) {
+		smmu_domain->stage = ARM_SMMU_DOMAIN_BYPASS;
+		return 0;
+	}
+
 	/* Restrict the stage to what we can actually support */
 	if (!(smmu->features & ARM_SMMU_FEAT_TRANS_S1))
 		smmu_domain->stage = ARM_SMMU_DOMAIN_S2;
@@ -1651,7 +1659,9 @@ static int arm_smmu_attach_dev(struct iommu_domain *domain, struct device *dev)
 	ste->bypass = false;
 	ste->valid = true;
 
-	if (smmu_domain->stage == ARM_SMMU_DOMAIN_S1) {
+	if (smmu_domain->stage == ARM_SMMU_DOMAIN_BYPASS) {
+		ste->bypass = true;
+	} else if (smmu_domain->stage == ARM_SMMU_DOMAIN_S1) {
 		ste->s1_cfg = &smmu_domain->s1_cfg;
 		ste->s2_cfg = NULL;
 		arm_smmu_write_ctx_desc(smmu, ste->s1_cfg);
-- 
2.1.4

[PATCH 4/5] arm64: dma-mapping: Only swizzle DMA ops for IOMMU_DOMAIN_DMA

[Will Deacon]

The arm64 DMA-mapping implementation sets the DMA ops to the IOMMU DMA
ops if we detect that an IOMMU is present for the master and the DMA
ranges are valid.

In the case when the IOMMU domain for the device is not of type
IOMMU_DOMAIN_DMA, then we have no business swizzling the ops, since
we're not in control of the underlying address space. This patch leaves
the DMA ops alone for masters attached to non-DMA IOMMU domains.

Signed-off-by: Will Deacon <will.deacon-5wv7dgnIgG8@public.gmane.org>
---
 arch/arm64/mm/dma-mapping.c | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/arch/arm64/mm/dma-mapping.c b/arch/arm64/mm/dma-mapping.c
index e04082700bb1..5d3c6ad621e8 100644
--- a/arch/arm64/mm/dma-mapping.c
+++ b/arch/arm64/mm/dma-mapping.c
@@ -831,14 +831,21 @@ static bool do_iommu_attach(struct device *dev, const struct iommu_ops *ops,
 	 * then the IOMMU core will have already configured a group for this
 	 * device, and allocated the default domain for that group.
 	 */
-	if (!domain || iommu_dma_init_domain(domain, dma_base, size, dev)) {
-		pr_warn("Failed to set up IOMMU for device %s; retaining platform DMA ops\n",
-			dev_name(dev));
-		return false;
+	if (!domain)
+		goto out_err;
+
+	if (domain->type == IOMMU_DOMAIN_DMA) {
+		if (iommu_dma_init_domain(domain, dma_base, size, dev))
+			goto out_err;
+
+		dev->archdata.dma_ops = &iommu_dma_ops;
 	}
 
-	dev->archdata.dma_ops = &iommu_dma_ops;
 	return true;
+out_err:
+	pr_warn("Failed to set up IOMMU for device %s; retaining platform DMA ops\n",
+		 dev_name(dev));
+	return false;
 }
 
 static void queue_iommu_attach(struct device *dev, const struct iommu_ops *ops,
-- 
2.1.4

[PATCH 4/5] arm64: dma-mapping: Only swizzle DMA ops for IOMMU_DOMAIN_DMA

[Will Deacon]

The arm64 DMA-mapping implementation sets the DMA ops to the IOMMU DMA
ops if we detect that an IOMMU is present for the master and the DMA
ranges are valid.

In the case when the IOMMU domain for the device is not of type
IOMMU_DOMAIN_DMA, then we have no business swizzling the ops, since
we're not in control of the underlying address space. This patch leaves
the DMA ops alone for masters attached to non-DMA IOMMU domains.

Signed-off-by: Will Deacon <will.deacon@arm.com>
---
 arch/arm64/mm/dma-mapping.c | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/arch/arm64/mm/dma-mapping.c b/arch/arm64/mm/dma-mapping.c
index e04082700bb1..5d3c6ad621e8 100644
--- a/arch/arm64/mm/dma-mapping.c
+++ b/arch/arm64/mm/dma-mapping.c
@@ -831,14 +831,21 @@ static bool do_iommu_attach(struct device *dev, const struct iommu_ops *ops,
 	 * then the IOMMU core will have already configured a group for this
 	 * device, and allocated the default domain for that group.
 	 */
-	if (!domain || iommu_dma_init_domain(domain, dma_base, size, dev)) {
-		pr_warn("Failed to set up IOMMU for device %s; retaining platform DMA ops\n",
-			dev_name(dev));
-		return false;
+	if (!domain)
+		goto out_err;
+
+	if (domain->type == IOMMU_DOMAIN_DMA) {
+		if (iommu_dma_init_domain(domain, dma_base, size, dev))
+			goto out_err;
+
+		dev->archdata.dma_ops = &iommu_dma_ops;
 	}
 
-	dev->archdata.dma_ops = &iommu_dma_ops;
 	return true;
+out_err:
+	pr_warn("Failed to set up IOMMU for device %s; retaining platform DMA ops\n",
+		 dev_name(dev));
+	return false;
 }
 
 static void queue_iommu_attach(struct device *dev, const struct iommu_ops *ops,
-- 
2.1.4

[PATCH 5/5] iommu: Allow default domain type to be set on the kernel command line

[Will Deacon]

The IOMMU core currently initialises the default domain for each group
to IOMMU_DOMAIN_DMA, under the assumption that devices will use
IOMMU-backed DMA ops by default. However, in some cases it is desirable
for the DMA ops to bypass the IOMMU for performance reasons, reserving
use of translation for subsystems such as VFIO that require it for
enforcing device isolation.

Rather than modify each IOMMU driver to provide different semantics for
DMA domains, instead we introduce a command line parameter that can be
used to change the type of the default domain. Passthrough can then be
specified using "iommu.default_domain=identity" on the kernel command
line.

Signed-off-by: Will Deacon <will.deacon-5wv7dgnIgG8@public.gmane.org>
---
 drivers/iommu/iommu.c | 19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index dbe7f653bb7c..69f7f1a75543 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -36,6 +36,7 @@
 
 static struct kset *iommu_group_kset;
 static DEFINE_IDA(iommu_group_ida);
+static unsigned int iommu_def_domain_type = IOMMU_DOMAIN_DMA;
 
 struct iommu_callback_data {
 	const struct iommu_ops *ops;
@@ -86,6 +87,20 @@ static int __iommu_attach_group(struct iommu_domain *domain,
 static void __iommu_detach_group(struct iommu_domain *domain,
 				 struct iommu_group *group);
 
+static int __init iommu_set_def_domain_type(char *str)
+{
+	if (!str)
+		return -EINVAL;
+
+	if (!strcmp(str, "identity"))
+		iommu_def_domain_type = IOMMU_DOMAIN_IDENTITY;
+	else if (!strcmp(str, "dma"))
+		iommu_def_domain_type = IOMMU_DOMAIN_DMA;
+
+	return 0;
+}
+early_param("iommu.default_domain", iommu_set_def_domain_type);
+
 static ssize_t iommu_group_attr_show(struct kobject *kobj,
 				     struct attribute *__attr, char *buf)
 {
@@ -847,8 +862,8 @@ struct iommu_group *iommu_group_get_for_dev(struct device *dev)
 	 * IOMMU driver.
 	 */
 	if (!group->default_domain) {
-		group->default_domain = __iommu_domain_alloc(dev->bus,
-							     IOMMU_DOMAIN_DMA);
+		group->default_domain =
+			__iommu_domain_alloc(dev->bus, iommu_def_domain_type);
 		if (!group->domain)
 			group->domain = group->default_domain;
 	}
-- 
2.1.4

[PATCH 5/5] iommu: Allow default domain type to be set on the kernel command line

[Will Deacon]

The IOMMU core currently initialises the default domain for each group
to IOMMU_DOMAIN_DMA, under the assumption that devices will use
IOMMU-backed DMA ops by default. However, in some cases it is desirable
for the DMA ops to bypass the IOMMU for performance reasons, reserving
use of translation for subsystems such as VFIO that require it for
enforcing device isolation.

Rather than modify each IOMMU driver to provide different semantics for
DMA domains, instead we introduce a command line parameter that can be
used to change the type of the default domain. Passthrough can then be
specified using "iommu.default_domain=identity" on the kernel command
line.

Signed-off-by: Will Deacon <will.deacon@arm.com>
---
 drivers/iommu/iommu.c | 19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index dbe7f653bb7c..69f7f1a75543 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -36,6 +36,7 @@
 
 static struct kset *iommu_group_kset;
 static DEFINE_IDA(iommu_group_ida);
+static unsigned int iommu_def_domain_type = IOMMU_DOMAIN_DMA;
 
 struct iommu_callback_data {
 	const struct iommu_ops *ops;
@@ -86,6 +87,20 @@ static int __iommu_attach_group(struct iommu_domain *domain,
 static void __iommu_detach_group(struct iommu_domain *domain,
 				 struct iommu_group *group);
 
+static int __init iommu_set_def_domain_type(char *str)
+{
+	if (!str)
+		return -EINVAL;
+
+	if (!strcmp(str, "identity"))
+		iommu_def_domain_type = IOMMU_DOMAIN_IDENTITY;
+	else if (!strcmp(str, "dma"))
+		iommu_def_domain_type = IOMMU_DOMAIN_DMA;
+
+	return 0;
+}
+early_param("iommu.default_domain", iommu_set_def_domain_type);
+
 static ssize_t iommu_group_attr_show(struct kobject *kobj,
 				     struct attribute *__attr, char *buf)
 {
@@ -847,8 +862,8 @@ struct iommu_group *iommu_group_get_for_dev(struct device *dev)
 	 * IOMMU driver.
 	 */
 	if (!group->default_domain) {
-		group->default_domain = __iommu_domain_alloc(dev->bus,
-							     IOMMU_DOMAIN_DMA);
+		group->default_domain =
+			__iommu_domain_alloc(dev->bus, iommu_def_domain_type);
 		if (!group->domain)
 			group->domain = group->default_domain;
 	}
-- 
2.1.4

Re: [PATCH 1/5] iommu/arm-smmu: Restrict domain attributes to UNMANAGED domains

[Robin Murphy]

On 19/01/17 18:19, Will Deacon wrote:
> The ARM SMMU drivers provide a DOMAIN_ATTR_NESTING domain attribute,
> which allows callers of the IOMMU API to request that the page table
> for a domain is installed at stage-2, if supported by the hardware.
> 
> Since setting this attribute only makes sense for UNMANAGED domains,
> this patch returns -ENODEV if the domain_{get,set}_attr operations are
> called on other domain types.

For the sake of discussion, would it make sense to enforce this in
domain_set_attr() itself? The intersection of drivers providing these
callbacks and drivers supporting anything other than unmanaged domains
is currently these two below, so it clearly wouldn't break anything to
put this check in core code today. Looking forward, is there likely to
be any plausible situation where users of a managed domain would be
legitimate in mucking about with its attrs, on any platform?

Robin.

> Signed-off-by: Will Deacon <will.deacon-5wv7dgnIgG8@public.gmane.org>
> ---
>  drivers/iommu/arm-smmu-v3.c | 6 ++++++
>  drivers/iommu/arm-smmu.c    | 6 ++++++
>  2 files changed, 12 insertions(+)
> 
> diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
> index 4d6ec444a9d6..c254325b0c7a 100644
> --- a/drivers/iommu/arm-smmu-v3.c
> +++ b/drivers/iommu/arm-smmu-v3.c
> @@ -1839,6 +1839,9 @@ static int arm_smmu_domain_get_attr(struct iommu_domain *domain,
>  {
>  	struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
>  
> +	if (domain->type != IOMMU_DOMAIN_UNMANAGED)
> +		return -ENODEV;
> +
>  	switch (attr) {
>  	case DOMAIN_ATTR_NESTING:
>  		*(int *)data = (smmu_domain->stage == ARM_SMMU_DOMAIN_NESTED);
> @@ -1854,6 +1857,9 @@ static int arm_smmu_domain_set_attr(struct iommu_domain *domain,
>  	int ret = 0;
>  	struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
>  
> +	if (domain->type != IOMMU_DOMAIN_UNMANAGED)
> +		return -ENODEV;
> +
>  	mutex_lock(&smmu_domain->init_mutex);
>  
>  	switch (attr) {
> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
> index a60cded8a6ed..a328ffb75509 100644
> --- a/drivers/iommu/arm-smmu.c
> +++ b/drivers/iommu/arm-smmu.c
> @@ -1497,6 +1497,9 @@ static int arm_smmu_domain_get_attr(struct iommu_domain *domain,
>  {
>  	struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
>  
> +	if (domain->type != IOMMU_DOMAIN_UNMANAGED)
> +		return -ENODEV;
> +
>  	switch (attr) {
>  	case DOMAIN_ATTR_NESTING:
>  		*(int *)data = (smmu_domain->stage == ARM_SMMU_DOMAIN_NESTED);
> @@ -1512,6 +1515,9 @@ static int arm_smmu_domain_set_attr(struct iommu_domain *domain,
>  	int ret = 0;
>  	struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
>  
> +	if (domain->type != IOMMU_DOMAIN_UNMANAGED)
> +		return -ENODEV;
> +
>  	mutex_lock(&smmu_domain->init_mutex);
>  
>  	switch (attr) {
> 

[PATCH 1/5] iommu/arm-smmu: Restrict domain attributes to UNMANAGED domains

[Robin Murphy]

On 19/01/17 18:19, Will Deacon wrote:
> The ARM SMMU drivers provide a DOMAIN_ATTR_NESTING domain attribute,
> which allows callers of the IOMMU API to request that the page table
> for a domain is installed at stage-2, if supported by the hardware.
> 
> Since setting this attribute only makes sense for UNMANAGED domains,
> this patch returns -ENODEV if the domain_{get,set}_attr operations are
> called on other domain types.

For the sake of discussion, would it make sense to enforce this in
domain_set_attr() itself? The intersection of drivers providing these
callbacks and drivers supporting anything other than unmanaged domains
is currently these two below, so it clearly wouldn't break anything to
put this check in core code today. Looking forward, is there likely to
be any plausible situation where users of a managed domain would be
legitimate in mucking about with its attrs, on any platform?

Robin.

> Signed-off-by: Will Deacon <will.deacon@arm.com>
> ---
>  drivers/iommu/arm-smmu-v3.c | 6 ++++++
>  drivers/iommu/arm-smmu.c    | 6 ++++++
>  2 files changed, 12 insertions(+)
> 
> diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
> index 4d6ec444a9d6..c254325b0c7a 100644
> --- a/drivers/iommu/arm-smmu-v3.c
> +++ b/drivers/iommu/arm-smmu-v3.c
> @@ -1839,6 +1839,9 @@ static int arm_smmu_domain_get_attr(struct iommu_domain *domain,
>  {
>  	struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
>  
> +	if (domain->type != IOMMU_DOMAIN_UNMANAGED)
> +		return -ENODEV;
> +
>  	switch (attr) {
>  	case DOMAIN_ATTR_NESTING:
>  		*(int *)data = (smmu_domain->stage == ARM_SMMU_DOMAIN_NESTED);
> @@ -1854,6 +1857,9 @@ static int arm_smmu_domain_set_attr(struct iommu_domain *domain,
>  	int ret = 0;
>  	struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
>  
> +	if (domain->type != IOMMU_DOMAIN_UNMANAGED)
> +		return -ENODEV;
> +
>  	mutex_lock(&smmu_domain->init_mutex);
>  
>  	switch (attr) {
> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
> index a60cded8a6ed..a328ffb75509 100644
> --- a/drivers/iommu/arm-smmu.c
> +++ b/drivers/iommu/arm-smmu.c
> @@ -1497,6 +1497,9 @@ static int arm_smmu_domain_get_attr(struct iommu_domain *domain,
>  {
>  	struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
>  
> +	if (domain->type != IOMMU_DOMAIN_UNMANAGED)
> +		return -ENODEV;
> +
>  	switch (attr) {
>  	case DOMAIN_ATTR_NESTING:
>  		*(int *)data = (smmu_domain->stage == ARM_SMMU_DOMAIN_NESTED);
> @@ -1512,6 +1515,9 @@ static int arm_smmu_domain_set_attr(struct iommu_domain *domain,
>  	int ret = 0;
>  	struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
>  
> +	if (domain->type != IOMMU_DOMAIN_UNMANAGED)
> +		return -ENODEV;
> +
>  	mutex_lock(&smmu_domain->init_mutex);
>  
>  	switch (attr) {
> 

Re: [PATCH 2/5] iommu/arm-smmu: Install bypass S2CRs for IOMMU_DOMAIN_IDENTITY domains

[Robin Murphy]

On 19/01/17 18:19, Will Deacon wrote:
> In preparation for allowing the default domain type to be overridden,
> this patch adds support for IOMMU_DOMAIN_IDENTITY domains to the
> ARM SMMU driver.
> 
> An identity domain is created by placing the corresponding S2CR
> registers into "bypass" mode, which allows transactions to flow through
> the SMMU without any translation.

The other subtle nicety this opens the door to is being able to disable
unmatched stream bypass without needing enough context banks for every
single known device, since S2CRs are generally more abundant.

Reviewed-by: Robin Murphy <robin.murphy@arm.com>

> Signed-off-by: Will Deacon <will.deacon@arm.com>
> ---
>  drivers/iommu/arm-smmu.c | 20 +++++++++++++++++---
>  1 file changed, 17 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
> index a328ffb75509..0f5e42a719e5 100644
> --- a/drivers/iommu/arm-smmu.c
> +++ b/drivers/iommu/arm-smmu.c
> @@ -404,6 +404,7 @@ enum arm_smmu_domain_stage {
>  	ARM_SMMU_DOMAIN_S1 = 0,
>  	ARM_SMMU_DOMAIN_S2,
>  	ARM_SMMU_DOMAIN_NESTED,
> +	ARM_SMMU_DOMAIN_BYPASS,
>  };
>  
>  struct arm_smmu_domain {
> @@ -824,6 +825,12 @@ static int arm_smmu_init_domain_context(struct iommu_domain *domain,
>  	if (smmu_domain->smmu)
>  		goto out_unlock;
>  
> +	if (domain->type == IOMMU_DOMAIN_IDENTITY) {
> +		smmu_domain->stage = ARM_SMMU_DOMAIN_BYPASS;
> +		smmu_domain->smmu = smmu;
> +		goto out_unlock;
> +	}
> +
>  	/*
>  	 * Mapping the requested stage onto what we support is surprisingly
>  	 * complicated, mainly because the spec allows S1+S2 SMMUs without
> @@ -984,7 +991,7 @@ static void arm_smmu_destroy_domain_context(struct iommu_domain *domain)
>  	void __iomem *cb_base;
>  	int irq;
>  
> -	if (!smmu)
> +	if (!smmu || domain->type == IOMMU_DOMAIN_IDENTITY)
>  		return;
>  
>  	/*
> @@ -1007,7 +1014,9 @@ static struct iommu_domain *arm_smmu_domain_alloc(unsigned type)
>  {
>  	struct arm_smmu_domain *smmu_domain;
>  
> -	if (type != IOMMU_DOMAIN_UNMANAGED && type != IOMMU_DOMAIN_DMA)
> +	if (type != IOMMU_DOMAIN_UNMANAGED &&
> +	    type != IOMMU_DOMAIN_DMA &&
> +	    type != IOMMU_DOMAIN_IDENTITY)
>  		return NULL;
>  	/*
>  	 * Allocate the domain and initialise some of its data structures.
> @@ -1205,10 +1214,15 @@ static int arm_smmu_domain_add_master(struct arm_smmu_domain *smmu_domain,
>  {
>  	struct arm_smmu_device *smmu = smmu_domain->smmu;
>  	struct arm_smmu_s2cr *s2cr = smmu->s2crs;
> -	enum arm_smmu_s2cr_type type = S2CR_TYPE_TRANS;
>  	u8 cbndx = smmu_domain->cfg.cbndx;
> +	enum arm_smmu_s2cr_type type;
>  	int i, idx;
>  
> +	if (smmu_domain->stage == ARM_SMMU_DOMAIN_BYPASS)
> +		type = S2CR_TYPE_BYPASS;
> +	else
> +		type = S2CR_TYPE_TRANS;
> +
>  	for_each_cfg_sme(fwspec, i, idx) {
>  		if (type == s2cr[idx].type && cbndx == s2cr[idx].cbndx)
>  			continue;
> 

[PATCH 2/5] iommu/arm-smmu: Install bypass S2CRs for IOMMU_DOMAIN_IDENTITY domains

[Robin Murphy]

On 19/01/17 18:19, Will Deacon wrote:
> In preparation for allowing the default domain type to be overridden,
> this patch adds support for IOMMU_DOMAIN_IDENTITY domains to the
> ARM SMMU driver.
> 
> An identity domain is created by placing the corresponding S2CR
> registers into "bypass" mode, which allows transactions to flow through
> the SMMU without any translation.

The other subtle nicety this opens the door to is being able to disable
unmatched stream bypass without needing enough context banks for every
single known device, since S2CRs are generally more abundant.

Reviewed-by: Robin Murphy <robin.murphy@arm.com>

> Signed-off-by: Will Deacon <will.deacon@arm.com>
> ---
>  drivers/iommu/arm-smmu.c | 20 +++++++++++++++++---
>  1 file changed, 17 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
> index a328ffb75509..0f5e42a719e5 100644
> --- a/drivers/iommu/arm-smmu.c
> +++ b/drivers/iommu/arm-smmu.c
> @@ -404,6 +404,7 @@ enum arm_smmu_domain_stage {
>  	ARM_SMMU_DOMAIN_S1 = 0,
>  	ARM_SMMU_DOMAIN_S2,
>  	ARM_SMMU_DOMAIN_NESTED,
> +	ARM_SMMU_DOMAIN_BYPASS,
>  };
>  
>  struct arm_smmu_domain {
> @@ -824,6 +825,12 @@ static int arm_smmu_init_domain_context(struct iommu_domain *domain,
>  	if (smmu_domain->smmu)
>  		goto out_unlock;
>  
> +	if (domain->type == IOMMU_DOMAIN_IDENTITY) {
> +		smmu_domain->stage = ARM_SMMU_DOMAIN_BYPASS;
> +		smmu_domain->smmu = smmu;
> +		goto out_unlock;
> +	}
> +
>  	/*
>  	 * Mapping the requested stage onto what we support is surprisingly
>  	 * complicated, mainly because the spec allows S1+S2 SMMUs without
> @@ -984,7 +991,7 @@ static void arm_smmu_destroy_domain_context(struct iommu_domain *domain)
>  	void __iomem *cb_base;
>  	int irq;
>  
> -	if (!smmu)
> +	if (!smmu || domain->type == IOMMU_DOMAIN_IDENTITY)
>  		return;
>  
>  	/*
> @@ -1007,7 +1014,9 @@ static struct iommu_domain *arm_smmu_domain_alloc(unsigned type)
>  {
>  	struct arm_smmu_domain *smmu_domain;
>  
> -	if (type != IOMMU_DOMAIN_UNMANAGED && type != IOMMU_DOMAIN_DMA)
> +	if (type != IOMMU_DOMAIN_UNMANAGED &&
> +	    type != IOMMU_DOMAIN_DMA &&
> +	    type != IOMMU_DOMAIN_IDENTITY)
>  		return NULL;
>  	/*
>  	 * Allocate the domain and initialise some of its data structures.
> @@ -1205,10 +1214,15 @@ static int arm_smmu_domain_add_master(struct arm_smmu_domain *smmu_domain,
>  {
>  	struct arm_smmu_device *smmu = smmu_domain->smmu;
>  	struct arm_smmu_s2cr *s2cr = smmu->s2crs;
> -	enum arm_smmu_s2cr_type type = S2CR_TYPE_TRANS;
>  	u8 cbndx = smmu_domain->cfg.cbndx;
> +	enum arm_smmu_s2cr_type type;
>  	int i, idx;
>  
> +	if (smmu_domain->stage == ARM_SMMU_DOMAIN_BYPASS)
> +		type = S2CR_TYPE_BYPASS;
> +	else
> +		type = S2CR_TYPE_TRANS;
> +
>  	for_each_cfg_sme(fwspec, i, idx) {
>  		if (type == s2cr[idx].type && cbndx == s2cr[idx].cbndx)
>  			continue;
> 

Re: [PATCH 3/5] iommu/arm-smmu-v3: Install bypass STEs for IOMMU_DOMAIN_IDENTITY domains

[Robin Murphy]

On 19/01/17 18:19, Will Deacon wrote:
> In preparation for allowing the default domain type to be overridden,
> this patch adds support for IOMMU_DOMAIN_IDENTITY domains to the
> ARM SMMUv3 driver.
> 
> An identity domain is created by placing the corresponding stream table
> entries into "bypass" mode, which allows transactions to flow through
> the SMMU without any translation.
> 
> Signed-off-by: Will Deacon <will.deacon-5wv7dgnIgG8@public.gmane.org>
> ---
>  drivers/iommu/arm-smmu-v3.c | 14 ++++++++++++--
>  1 file changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
> index c254325b0c7a..d33291274455 100644
> --- a/drivers/iommu/arm-smmu-v3.c
> +++ b/drivers/iommu/arm-smmu-v3.c
> @@ -629,6 +629,7 @@ enum arm_smmu_domain_stage {
>  	ARM_SMMU_DOMAIN_S1 = 0,
>  	ARM_SMMU_DOMAIN_S2,
>  	ARM_SMMU_DOMAIN_NESTED,
> +	ARM_SMMU_DOMAIN_BYPASS,
>  };
>  
>  struct arm_smmu_domain {
> @@ -1385,7 +1386,9 @@ static struct iommu_domain *arm_smmu_domain_alloc(unsigned type)
>  {
>  	struct arm_smmu_domain *smmu_domain;
>  
> -	if (type != IOMMU_DOMAIN_UNMANAGED && type != IOMMU_DOMAIN_DMA)
> +	if (type != IOMMU_DOMAIN_UNMANAGED &&
> +	    type != IOMMU_DOMAIN_DMA &&
> +	    type != IOMMU_DOMAIN_IDENTITY)
>  		return NULL;
>  
>  	/*
> @@ -1516,6 +1519,11 @@ static int arm_smmu_domain_finalise(struct iommu_domain *domain)
>  	struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
>  	struct arm_smmu_device *smmu = smmu_domain->smmu;
>  
> +	if (domain->type == IOMMU_DOMAIN_IDENTITY) {
> +		smmu_domain->stage = ARM_SMMU_DOMAIN_BYPASS;
> +		return 0;
> +	}
> +
>  	/* Restrict the stage to what we can actually support */
>  	if (!(smmu->features & ARM_SMMU_FEAT_TRANS_S1))
>  		smmu_domain->stage = ARM_SMMU_DOMAIN_S2;
> @@ -1651,7 +1659,9 @@ static int arm_smmu_attach_dev(struct iommu_domain *domain, struct device *dev)
>  	ste->bypass = false;
>  	ste->valid = true;
>  
> -	if (smmu_domain->stage == ARM_SMMU_DOMAIN_S1) {
> +	if (smmu_domain->stage == ARM_SMMU_DOMAIN_BYPASS) {
> +		ste->bypass = true;

How is this intended to interact with the disable_bypass parameter? at
the moment, that will still end up transforming this into a faulting
STE, and I'm not sure that's right. I'd say we want to treat "bypass
because not attached to a domain" and "bypass because attached to a
passthrough domain" as distinct things, and it's only really the former
which makes sense to disable.

Robin.

> +	} else if (smmu_domain->stage == ARM_SMMU_DOMAIN_S1) {
>  		ste->s1_cfg = &smmu_domain->s1_cfg;
>  		ste->s2_cfg = NULL;
>  		arm_smmu_write_ctx_desc(smmu, ste->s1_cfg);
> 

[PATCH 3/5] iommu/arm-smmu-v3: Install bypass STEs for IOMMU_DOMAIN_IDENTITY domains

[Robin Murphy]

On 19/01/17 18:19, Will Deacon wrote:
> In preparation for allowing the default domain type to be overridden,
> this patch adds support for IOMMU_DOMAIN_IDENTITY domains to the
> ARM SMMUv3 driver.
> 
> An identity domain is created by placing the corresponding stream table
> entries into "bypass" mode, which allows transactions to flow through
> the SMMU without any translation.
> 
> Signed-off-by: Will Deacon <will.deacon@arm.com>
> ---
>  drivers/iommu/arm-smmu-v3.c | 14 ++++++++++++--
>  1 file changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
> index c254325b0c7a..d33291274455 100644
> --- a/drivers/iommu/arm-smmu-v3.c
> +++ b/drivers/iommu/arm-smmu-v3.c
> @@ -629,6 +629,7 @@ enum arm_smmu_domain_stage {
>  	ARM_SMMU_DOMAIN_S1 = 0,
>  	ARM_SMMU_DOMAIN_S2,
>  	ARM_SMMU_DOMAIN_NESTED,
> +	ARM_SMMU_DOMAIN_BYPASS,
>  };
>  
>  struct arm_smmu_domain {
> @@ -1385,7 +1386,9 @@ static struct iommu_domain *arm_smmu_domain_alloc(unsigned type)
>  {
>  	struct arm_smmu_domain *smmu_domain;
>  
> -	if (type != IOMMU_DOMAIN_UNMANAGED && type != IOMMU_DOMAIN_DMA)
> +	if (type != IOMMU_DOMAIN_UNMANAGED &&
> +	    type != IOMMU_DOMAIN_DMA &&
> +	    type != IOMMU_DOMAIN_IDENTITY)
>  		return NULL;
>  
>  	/*
> @@ -1516,6 +1519,11 @@ static int arm_smmu_domain_finalise(struct iommu_domain *domain)
>  	struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
>  	struct arm_smmu_device *smmu = smmu_domain->smmu;
>  
> +	if (domain->type == IOMMU_DOMAIN_IDENTITY) {
> +		smmu_domain->stage = ARM_SMMU_DOMAIN_BYPASS;
> +		return 0;
> +	}
> +
>  	/* Restrict the stage to what we can actually support */
>  	if (!(smmu->features & ARM_SMMU_FEAT_TRANS_S1))
>  		smmu_domain->stage = ARM_SMMU_DOMAIN_S2;
> @@ -1651,7 +1659,9 @@ static int arm_smmu_attach_dev(struct iommu_domain *domain, struct device *dev)
>  	ste->bypass = false;
>  	ste->valid = true;
>  
> -	if (smmu_domain->stage == ARM_SMMU_DOMAIN_S1) {
> +	if (smmu_domain->stage == ARM_SMMU_DOMAIN_BYPASS) {
> +		ste->bypass = true;

How is this intended to interact with the disable_bypass parameter? at
the moment, that will still end up transforming this into a faulting
STE, and I'm not sure that's right. I'd say we want to treat "bypass
because not attached to a domain" and "bypass because attached to a
passthrough domain" as distinct things, and it's only really the former
which makes sense to disable.

Robin.

> +	} else if (smmu_domain->stage == ARM_SMMU_DOMAIN_S1) {
>  		ste->s1_cfg = &smmu_domain->s1_cfg;
>  		ste->s2_cfg = NULL;
>  		arm_smmu_write_ctx_desc(smmu, ste->s1_cfg);
> 

Re: [PATCH 4/5] arm64: dma-mapping: Only swizzle DMA ops for IOMMU_DOMAIN_DMA

[Robin Murphy]

On 19/01/17 18:19, Will Deacon wrote:
> The arm64 DMA-mapping implementation sets the DMA ops to the IOMMU DMA
> ops if we detect that an IOMMU is present for the master and the DMA
> ranges are valid.
> 
> In the case when the IOMMU domain for the device is not of type
> IOMMU_DOMAIN_DMA, then we have no business swizzling the ops, since
> we're not in control of the underlying address space. This patch leaves
> the DMA ops alone for masters attached to non-DMA IOMMU domains.

In fact, I don't think there would be any harm in taking this one
through arm64 straight away. The DMA ops can't be expected to work
successfully on any old domain, so it's a reasonable sanity check
regardless.

Reviewed-by: Robin Murphy <robin.murphy-5wv7dgnIgG8@public.gmane.org>

> Signed-off-by: Will Deacon <will.deacon-5wv7dgnIgG8@public.gmane.org>
> ---
>  arch/arm64/mm/dma-mapping.c | 17 ++++++++++++-----
>  1 file changed, 12 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/arm64/mm/dma-mapping.c b/arch/arm64/mm/dma-mapping.c
> index e04082700bb1..5d3c6ad621e8 100644
> --- a/arch/arm64/mm/dma-mapping.c
> +++ b/arch/arm64/mm/dma-mapping.c
> @@ -831,14 +831,21 @@ static bool do_iommu_attach(struct device *dev, const struct iommu_ops *ops,
>  	 * then the IOMMU core will have already configured a group for this
>  	 * device, and allocated the default domain for that group.
>  	 */
> -	if (!domain || iommu_dma_init_domain(domain, dma_base, size, dev)) {
> -		pr_warn("Failed to set up IOMMU for device %s; retaining platform DMA ops\n",
> -			dev_name(dev));
> -		return false;
> +	if (!domain)
> +		goto out_err;
> +
> +	if (domain->type == IOMMU_DOMAIN_DMA) {
> +		if (iommu_dma_init_domain(domain, dma_base, size, dev))
> +			goto out_err;
> +
> +		dev->archdata.dma_ops = &iommu_dma_ops;
>  	}
>  
> -	dev->archdata.dma_ops = &iommu_dma_ops;
>  	return true;
> +out_err:
> +	pr_warn("Failed to set up IOMMU for device %s; retaining platform DMA ops\n",
> +		 dev_name(dev));
> +	return false;
>  }
>  
>  static void queue_iommu_attach(struct device *dev, const struct iommu_ops *ops,
> 

[PATCH 4/5] arm64: dma-mapping: Only swizzle DMA ops for IOMMU_DOMAIN_DMA

[Robin Murphy]

On 19/01/17 18:19, Will Deacon wrote:
> The arm64 DMA-mapping implementation sets the DMA ops to the IOMMU DMA
> ops if we detect that an IOMMU is present for the master and the DMA
> ranges are valid.
> 
> In the case when the IOMMU domain for the device is not of type
> IOMMU_DOMAIN_DMA, then we have no business swizzling the ops, since
> we're not in control of the underlying address space. This patch leaves
> the DMA ops alone for masters attached to non-DMA IOMMU domains.

In fact, I don't think there would be any harm in taking this one
through arm64 straight away. The DMA ops can't be expected to work
successfully on any old domain, so it's a reasonable sanity check
regardless.

Reviewed-by: Robin Murphy <robin.murphy@arm.com>

> Signed-off-by: Will Deacon <will.deacon@arm.com>
> ---
>  arch/arm64/mm/dma-mapping.c | 17 ++++++++++++-----
>  1 file changed, 12 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/arm64/mm/dma-mapping.c b/arch/arm64/mm/dma-mapping.c
> index e04082700bb1..5d3c6ad621e8 100644
> --- a/arch/arm64/mm/dma-mapping.c
> +++ b/arch/arm64/mm/dma-mapping.c
> @@ -831,14 +831,21 @@ static bool do_iommu_attach(struct device *dev, const struct iommu_ops *ops,
>  	 * then the IOMMU core will have already configured a group for this
>  	 * device, and allocated the default domain for that group.
>  	 */
> -	if (!domain || iommu_dma_init_domain(domain, dma_base, size, dev)) {
> -		pr_warn("Failed to set up IOMMU for device %s; retaining platform DMA ops\n",
> -			dev_name(dev));
> -		return false;
> +	if (!domain)
> +		goto out_err;
> +
> +	if (domain->type == IOMMU_DOMAIN_DMA) {
> +		if (iommu_dma_init_domain(domain, dma_base, size, dev))
> +			goto out_err;
> +
> +		dev->archdata.dma_ops = &iommu_dma_ops;
>  	}
>  
> -	dev->archdata.dma_ops = &iommu_dma_ops;
>  	return true;
> +out_err:
> +	pr_warn("Failed to set up IOMMU for device %s; retaining platform DMA ops\n",
> +		 dev_name(dev));
> +	return false;
>  }
>  
>  static void queue_iommu_attach(struct device *dev, const struct iommu_ops *ops,
> 

RE: [PATCH 0/5] Implement SMMU passthrough using the default domain

[Sricharan]

Hi Will,

>Hi all,
>
>A number of people have expressed interest in having the SMMU come up in
>a passthrough configuration, and then allow subsequent translation for
>things such as VFIO. Rather than do this in each SMMU driver, it's much
>cleaner to allow the default domain to be configured to be something other
>than DMA.
>
>This patch series implements a command-line option to configure the
>default domain type. Currently, it supports "dma" and "identity" which
>is sufficient for the passthrough use-case.
>
>Tested on an ARM fastmodel.
>
>All feedback welcome,
>

Thanks for this series. We had a case with the GPU.
The GPU's iommu was setup by kernel and the GPU
also does dynamic updates for on-the-fly switching between
process pagetables.  GPU driver was not using DMA domain and
the GPU's firmware was always expecting to run out  of contextbank
 '0' (although not correct) , which was not the case after the DMA domain
was made default  as '0' was getting allocated for DMA domain and
there were concerns about reusing the DMA domain as well.
Now with this series, looks there is an way out of that that can be tried.

So should the default domain not be per device specific selectable ?

Regards,
 Sricharan



>Will
>
>--->8
>
>Will Deacon (5):
>  iommu/arm-smmu: Restrict domain attributes to UNMANAGED domains
>  iommu/arm-smmu: Install bypass S2CRs for IOMMU_DOMAIN_IDENTITY domains
>  iommu/arm-smmu-v3: Install bypass STEs for IOMMU_DOMAIN_IDENTITY
>    domains
>  arm64: dma-mapping: Only swizzle DMA ops for IOMMU_DOMAIN_DMA
>  iommu: Allow default domain type to be set on the kernel command line
>
> arch/arm64/mm/dma-mapping.c | 17 ++++++++++++-----
> drivers/iommu/arm-smmu-v3.c | 20 ++++++++++++++++++--
> drivers/iommu/arm-smmu.c    | 26 +++++++++++++++++++++++---
> drivers/iommu/iommu.c       | 19 +++++++++++++++++--
> 4 files changed, 70 insertions(+), 12 deletions(-)
>
>--
>2.1.4
>
>
>_______________________________________________
>linux-arm-kernel mailing list
>linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
>http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 0/5] Implement SMMU passthrough using the default domain

[Sricharan]

Hi Will,

>Hi all,
>
>A number of people have expressed interest in having the SMMU come up in
>a passthrough configuration, and then allow subsequent translation for
>things such as VFIO. Rather than do this in each SMMU driver, it's much
>cleaner to allow the default domain to be configured to be something other
>than DMA.
>
>This patch series implements a command-line option to configure the
>default domain type. Currently, it supports "dma" and "identity" which
>is sufficient for the passthrough use-case.
>
>Tested on an ARM fastmodel.
>
>All feedback welcome,
>

Thanks for this series. We had a case with the GPU.
The GPU's iommu was setup by kernel and the GPU
also does dynamic updates for on-the-fly switching between
process pagetables.  GPU driver was not using DMA domain and
the GPU's firmware was always expecting to run out  of contextbank
 '0' (although not correct) , which was not the case after the DMA domain
was made default  as '0' was getting allocated for DMA domain and
there were concerns about reusing the DMA domain as well.
Now with this series, looks there is an way out of that that can be tried.

So should the default domain not be per device specific selectable ?

Regards,
 Sricharan



>Will
>
>--->8
>
>Will Deacon (5):
>  iommu/arm-smmu: Restrict domain attributes to UNMANAGED domains
>  iommu/arm-smmu: Install bypass S2CRs for IOMMU_DOMAIN_IDENTITY domains
>  iommu/arm-smmu-v3: Install bypass STEs for IOMMU_DOMAIN_IDENTITY
>    domains
>  arm64: dma-mapping: Only swizzle DMA ops for IOMMU_DOMAIN_DMA
>  iommu: Allow default domain type to be set on the kernel command line
>
> arch/arm64/mm/dma-mapping.c | 17 ++++++++++++-----
> drivers/iommu/arm-smmu-v3.c | 20 ++++++++++++++++++--
> drivers/iommu/arm-smmu.c    | 26 +++++++++++++++++++++++---
> drivers/iommu/iommu.c       | 19 +++++++++++++++++--
> 4 files changed, 70 insertions(+), 12 deletions(-)
>
>--
>2.1.4
>
>
>_______________________________________________
>linux-arm-kernel mailing list
>linux-arm-kernel at lists.infradead.org
>http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/5] iommu/arm-smmu: Restrict domain attributes to UNMANAGED domains

[Joerg Roedel]

On Thu, Jan 19, 2017 at 06:19:11PM +0000, Will Deacon wrote:
> The ARM SMMU drivers provide a DOMAIN_ATTR_NESTING domain attribute,
> which allows callers of the IOMMU API to request that the page table
> for a domain is installed at stage-2, if supported by the hardware.
> 
> Since setting this attribute only makes sense for UNMANAGED domains,
> this patch returns -ENODEV if the domain_{get,set}_attr operations are
> called on other domain types.

Isn't -EINVAL more suitable here? In the end the domain passed in is
invalid because it does not support attributes, no?



	Joerg

[PATCH 1/5] iommu/arm-smmu: Restrict domain attributes to UNMANAGED domains

[Joerg Roedel]

On Thu, Jan 19, 2017 at 06:19:11PM +0000, Will Deacon wrote:
> The ARM SMMU drivers provide a DOMAIN_ATTR_NESTING domain attribute,
> which allows callers of the IOMMU API to request that the page table
> for a domain is installed at stage-2, if supported by the hardware.
> 
> Since setting this attribute only makes sense for UNMANAGED domains,
> this patch returns -ENODEV if the domain_{get,set}_attr operations are
> called on other domain types.

Isn't -EINVAL more suitable here? In the end the domain passed in is
invalid because it does not support attributes, no?



	Joerg

Re: [PATCH 1/5] iommu/arm-smmu: Restrict domain attributes to UNMANAGED domains

[Joerg Roedel]

On Thu, Jan 19, 2017 at 06:41:34PM +0000, Robin Murphy wrote:
> For the sake of discussion, would it make sense to enforce this in
> domain_set_attr() itself? The intersection of drivers providing these
> callbacks and drivers supporting anything other than unmanaged domains
> is currently these two below, so it clearly wouldn't break anything to
> put this check in core code today. Looking forward, is there likely to
> be any plausible situation where users of a managed domain would be
> legitimate in mucking about with its attrs, on any platform?

No, this belongs in driver code. I am pretty certain there will be a use
for attributes in unmanaged domains at some point. Crazy things happen
all the time :)


	Joerg

[PATCH 1/5] iommu/arm-smmu: Restrict domain attributes to UNMANAGED domains

[Joerg Roedel]

On Thu, Jan 19, 2017 at 06:41:34PM +0000, Robin Murphy wrote:
> For the sake of discussion, would it make sense to enforce this in
> domain_set_attr() itself? The intersection of drivers providing these
> callbacks and drivers supporting anything other than unmanaged domains
> is currently these two below, so it clearly wouldn't break anything to
> put this check in core code today. Looking forward, is there likely to
> be any plausible situation where users of a managed domain would be
> legitimate in mucking about with its attrs, on any platform?

No, this belongs in driver code. I am pretty certain there will be a use
for attributes in unmanaged domains at some point. Crazy things happen
all the time :)


	Joerg

Re: [PATCH 5/5] iommu: Allow default domain type to be set on the kernel command line

[Joerg Roedel]

On Thu, Jan 19, 2017 at 06:19:15PM +0000, Will Deacon wrote:
> Rather than modify each IOMMU driver to provide different semantics for
> DMA domains, instead we introduce a command line parameter that can be
> used to change the type of the default domain. Passthrough can then be
> specified using "iommu.default_domain=identity" on the kernel command
> line.

I like the general idea of this, but the above is a terrible name for a
kernel commandline-parameter. The x86 iommus support iommu=pt which is
pretty much the same as this patch does.

How about something like "iommu.passthrough=0/1"? And please add the
parameter to the kernel documentation too.


	Joerg

[PATCH 5/5] iommu: Allow default domain type to be set on the kernel command line

[Joerg Roedel]

On Thu, Jan 19, 2017 at 06:19:15PM +0000, Will Deacon wrote:
> Rather than modify each IOMMU driver to provide different semantics for
> DMA domains, instead we introduce a command line parameter that can be
> used to change the type of the default domain. Passthrough can then be
> specified using "iommu.default_domain=identity" on the kernel command
> line.

I like the general idea of this, but the above is a terrible name for a
kernel commandline-parameter. The x86 iommus support iommu=pt which is
pretty much the same as this patch does.

How about something like "iommu.passthrough=0/1"? And please add the
parameter to the kernel documentation too.


	Joerg

Re: [PATCH 0/5] Implement SMMU passthrough using the default domain

[Joerg Roedel]

On Tue, Jan 24, 2017 at 08:42:23PM +0530, Sricharan wrote:
> Thanks for this series. We had a case with the GPU.
> The GPU's iommu was setup by kernel and the GPU
> also does dynamic updates for on-the-fly switching between
> process pagetables.  GPU driver was not using DMA domain and
> the GPU's firmware was always expecting to run out  of contextbank
>  '0' (although not correct) , which was not the case after the DMA domain
> was made default  as '0' was getting allocated for DMA domain and
> there were concerns about reusing the DMA domain as well.
> Now with this series, looks there is an way out of that that can be tried.
> 
> So should the default domain not be per device specific selectable ?

Note that iommu-drivers can request direct-mapping for any given device
on its initializtion. This is used on x86 for devices that need a 1-1
mapping for some reason.

Also device drivers can use the iommu-api and assign their own domain to
a device, which allows them to manage the dma address space on their
own.


	Joerg

[PATCH 0/5] Implement SMMU passthrough using the default domain

[Joerg Roedel]

On Tue, Jan 24, 2017 at 08:42:23PM +0530, Sricharan wrote:
> Thanks for this series. We had a case with the GPU.
> The GPU's iommu was setup by kernel and the GPU
> also does dynamic updates for on-the-fly switching between
> process pagetables.  GPU driver was not using DMA domain and
> the GPU's firmware was always expecting to run out  of contextbank
>  '0' (although not correct) , which was not the case after the DMA domain
> was made default  as '0' was getting allocated for DMA domain and
> there were concerns about reusing the DMA domain as well.
> Now with this series, looks there is an way out of that that can be tried.
> 
> So should the default domain not be per device specific selectable ?

Note that iommu-drivers can request direct-mapping for any given device
on its initializtion. This is used on x86 for devices that need a 1-1
mapping for some reason.

Also device drivers can use the iommu-api and assign their own domain to
a device, which allows them to manage the dma address space on their
own.


	Joerg

Re: [PATCH 5/5] iommu: Allow default domain type to be set on the kernel command line

[Robin Murphy]

On 26/01/17 17:15, Joerg Roedel wrote:
> On Thu, Jan 19, 2017 at 06:19:15PM +0000, Will Deacon wrote:
>> Rather than modify each IOMMU driver to provide different semantics for
>> DMA domains, instead we introduce a command line parameter that can be
>> used to change the type of the default domain. Passthrough can then be
>> specified using "iommu.default_domain=identity" on the kernel command
>> line.
> 
> I like the general idea of this, but the above is a terrible name for a
> kernel commandline-parameter. The x86 iommus support iommu=pt which is
> pretty much the same as this patch does.

Indeed, I was keen on making "iommu=pt" also do this default domain
switch itself so we wouldn't need a new option - it didn't *appear* that
that would break the AMD driver (as the only other default domain user
supporting identity domains) but I may have overlooked something.

Robin.

> How about something like "iommu.passthrough=0/1"? And please add the
> parameter to the kernel documentation too.
> 
> 
> 	Joerg
> 

[PATCH 5/5] iommu: Allow default domain type to be set on the kernel command line

[Robin Murphy]

On 26/01/17 17:15, Joerg Roedel wrote:
> On Thu, Jan 19, 2017 at 06:19:15PM +0000, Will Deacon wrote:
>> Rather than modify each IOMMU driver to provide different semantics for
>> DMA domains, instead we introduce a command line parameter that can be
>> used to change the type of the default domain. Passthrough can then be
>> specified using "iommu.default_domain=identity" on the kernel command
>> line.
> 
> I like the general idea of this, but the above is a terrible name for a
> kernel commandline-parameter. The x86 iommus support iommu=pt which is
> pretty much the same as this patch does.

Indeed, I was keen on making "iommu=pt" also do this default domain
switch itself so we wouldn't need a new option - it didn't *appear* that
that would break the AMD driver (as the only other default domain user
supporting identity domains) but I may have overlooked something.

Robin.

> How about something like "iommu.passthrough=0/1"? And please add the
> parameter to the kernel documentation too.
> 
> 
> 	Joerg
> 

Re: [PATCH 5/5] iommu: Allow default domain type to be set on the kernel command line

[Will Deacon]

On Thu, Jan 26, 2017 at 06:15:55PM +0100, Joerg Roedel wrote:
> On Thu, Jan 19, 2017 at 06:19:15PM +0000, Will Deacon wrote:
> > Rather than modify each IOMMU driver to provide different semantics for
> > DMA domains, instead we introduce a command line parameter that can be
> > used to change the type of the default domain. Passthrough can then be
> > specified using "iommu.default_domain=identity" on the kernel command
> > line.
> 
> I like the general idea of this, but the above is a terrible name for a
> kernel commandline-parameter. The x86 iommus support iommu=pt which is
> pretty much the same as this patch does.

Happy to bikeshed the name ;)

> How about something like "iommu.passthrough=0/1"? And please add the
> parameter to the kernel documentation too.

Sure, if you think that the identity domain is the only thing we'll ever
want to set (so far, it's the only thing people have asked me for).

Will

[PATCH 5/5] iommu: Allow default domain type to be set on the kernel command line

[Will Deacon]

On Thu, Jan 26, 2017 at 06:15:55PM +0100, Joerg Roedel wrote:
> On Thu, Jan 19, 2017 at 06:19:15PM +0000, Will Deacon wrote:
> > Rather than modify each IOMMU driver to provide different semantics for
> > DMA domains, instead we introduce a command line parameter that can be
> > used to change the type of the default domain. Passthrough can then be
> > specified using "iommu.default_domain=identity" on the kernel command
> > line.
> 
> I like the general idea of this, but the above is a terrible name for a
> kernel commandline-parameter. The x86 iommus support iommu=pt which is
> pretty much the same as this patch does.

Happy to bikeshed the name ;)

> How about something like "iommu.passthrough=0/1"? And please add the
> parameter to the kernel documentation too.

Sure, if you think that the identity domain is the only thing we'll ever
want to set (so far, it's the only thing people have asked me for).

Will

Re: [PATCH 1/5] iommu/arm-smmu: Restrict domain attributes to UNMANAGED domains

[Will Deacon]

On Thu, Jan 26, 2017 at 06:03:30PM +0100, Joerg Roedel wrote:
> On Thu, Jan 19, 2017 at 06:19:11PM +0000, Will Deacon wrote:
> > The ARM SMMU drivers provide a DOMAIN_ATTR_NESTING domain attribute,
> > which allows callers of the IOMMU API to request that the page table
> > for a domain is installed at stage-2, if supported by the hardware.
> > 
> > Since setting this attribute only makes sense for UNMANAGED domains,
> > this patch returns -ENODEV if the domain_{get,set}_attr operations are
> > called on other domain types.
> 
> Isn't -EINVAL more suitable here? In the end the domain passed in is
> invalid because it does not support attributes, no?

Sure, I can change that.

Will

[PATCH 1/5] iommu/arm-smmu: Restrict domain attributes to UNMANAGED domains

[Will Deacon]

On Thu, Jan 26, 2017 at 06:03:30PM +0100, Joerg Roedel wrote:
> On Thu, Jan 19, 2017 at 06:19:11PM +0000, Will Deacon wrote:
> > The ARM SMMU drivers provide a DOMAIN_ATTR_NESTING domain attribute,
> > which allows callers of the IOMMU API to request that the page table
> > for a domain is installed at stage-2, if supported by the hardware.
> > 
> > Since setting this attribute only makes sense for UNMANAGED domains,
> > this patch returns -ENODEV if the domain_{get,set}_attr operations are
> > called on other domain types.
> 
> Isn't -EINVAL more suitable here? In the end the domain passed in is
> invalid because it does not support attributes, no?

Sure, I can change that.

Will

Re: [PATCH 4/5] arm64: dma-mapping: Only swizzle DMA ops for IOMMU_DOMAIN_DMA

[Will Deacon]

On Thu, Jan 19, 2017 at 07:00:25PM +0000, Robin Murphy wrote:
> On 19/01/17 18:19, Will Deacon wrote:
> > The arm64 DMA-mapping implementation sets the DMA ops to the IOMMU DMA
> > ops if we detect that an IOMMU is present for the master and the DMA
> > ranges are valid.
> > 
> > In the case when the IOMMU domain for the device is not of type
> > IOMMU_DOMAIN_DMA, then we have no business swizzling the ops, since
> > we're not in control of the underlying address space. This patch leaves
> > the DMA ops alone for masters attached to non-DMA IOMMU domains.
> 
> In fact, I don't think there would be any harm in taking this one
> through arm64 straight away. The DMA ops can't be expected to work
> successfully on any old domain, so it's a reasonable sanity check
> regardless.
> 
> Reviewed-by: Robin Murphy <robin.murphy@arm.com>

Good point; I'll queue this one for 4.11 via arm64.

Will

[PATCH 4/5] arm64: dma-mapping: Only swizzle DMA ops for IOMMU_DOMAIN_DMA

[Will Deacon]

On Thu, Jan 19, 2017 at 07:00:25PM +0000, Robin Murphy wrote:
> On 19/01/17 18:19, Will Deacon wrote:
> > The arm64 DMA-mapping implementation sets the DMA ops to the IOMMU DMA
> > ops if we detect that an IOMMU is present for the master and the DMA
> > ranges are valid.
> > 
> > In the case when the IOMMU domain for the device is not of type
> > IOMMU_DOMAIN_DMA, then we have no business swizzling the ops, since
> > we're not in control of the underlying address space. This patch leaves
> > the DMA ops alone for masters attached to non-DMA IOMMU domains.
> 
> In fact, I don't think there would be any harm in taking this one
> through arm64 straight away. The DMA ops can't be expected to work
> successfully on any old domain, so it's a reasonable sanity check
> regardless.
> 
> Reviewed-by: Robin Murphy <robin.murphy@arm.com>

Good point; I'll queue this one for 4.11 via arm64.

Will

Re: [PATCH 0/5] Implement SMMU passthrough using the default domain

[Rob Clark]

On Thu, Jan 26, 2017 at 12:18 PM, Joerg Roedel <joro-zLv9SwRftAIdnm+yROfE0A@public.gmane.org> wrote:
> On Tue, Jan 24, 2017 at 08:42:23PM +0530, Sricharan wrote:
>> Thanks for this series. We had a case with the GPU.
>> The GPU's iommu was setup by kernel and the GPU
>> also does dynamic updates for on-the-fly switching between
>> process pagetables.  GPU driver was not using DMA domain and
>> the GPU's firmware was always expecting to run out  of contextbank
>>  '0' (although not correct) , which was not the case after the DMA domain
>> was made default  as '0' was getting allocated for DMA domain and
>> there were concerns about reusing the DMA domain as well.
>> Now with this series, looks there is an way out of that that can be tried.
>>
>> So should the default domain not be per device specific selectable ?
>
> Note that iommu-drivers can request direct-mapping for any given device
> on its initializtion. This is used on x86 for devices that need a 1-1
> mapping for some reason.
>
> Also device drivers can use the iommu-api and assign their own domain to
> a device, which allows them to manage the dma address space on their
> own.

Part of the problem is that dev->archdata.dma_ops gets wired up to
iommu_dma_ops.  Which isn't so bad on it's own, except that cache ops
are not exposed to drivers, forcing us to use dma-mapping API
(dma_map_sg, etc) for cache operations.

Possibly we should just expose cache op's to drivers bypass this abuse
of dma-mapping.

btw, Will, we definitely want this to *not* rely on kcmdline for the
gpu with it's own private iommu case..

BR,
-R

[PATCH 0/5] Implement SMMU passthrough using the default domain

[Rob Clark]

On Thu, Jan 26, 2017 at 12:18 PM, Joerg Roedel <joro@8bytes.org> wrote:
> On Tue, Jan 24, 2017 at 08:42:23PM +0530, Sricharan wrote:
>> Thanks for this series. We had a case with the GPU.
>> The GPU's iommu was setup by kernel and the GPU
>> also does dynamic updates for on-the-fly switching between
>> process pagetables.  GPU driver was not using DMA domain and
>> the GPU's firmware was always expecting to run out  of contextbank
>>  '0' (although not correct) , which was not the case after the DMA domain
>> was made default  as '0' was getting allocated for DMA domain and
>> there were concerns about reusing the DMA domain as well.
>> Now with this series, looks there is an way out of that that can be tried.
>>
>> So should the default domain not be per device specific selectable ?
>
> Note that iommu-drivers can request direct-mapping for any given device
> on its initializtion. This is used on x86 for devices that need a 1-1
> mapping for some reason.
>
> Also device drivers can use the iommu-api and assign their own domain to
> a device, which allows them to manage the dma address space on their
> own.

Part of the problem is that dev->archdata.dma_ops gets wired up to
iommu_dma_ops.  Which isn't so bad on it's own, except that cache ops
are not exposed to drivers, forcing us to use dma-mapping API
(dma_map_sg, etc) for cache operations.

Possibly we should just expose cache op's to drivers bypass this abuse
of dma-mapping.

btw, Will, we definitely want this to *not* rely on kcmdline for the
gpu with it's own private iommu case..

BR,
-R

Re: [PATCH 0/5] Implement SMMU passthrough using the default domain

[Will Deacon]

On Thu, Feb 02, 2017 at 10:02:50AM -0500, Rob Clark wrote:
> On Thu, Jan 26, 2017 at 12:18 PM, Joerg Roedel <joro-zLv9SwRftAIdnm+yROfE0A@public.gmane.org> wrote:
> > On Tue, Jan 24, 2017 at 08:42:23PM +0530, Sricharan wrote:
> >> Thanks for this series. We had a case with the GPU.
> >> The GPU's iommu was setup by kernel and the GPU
> >> also does dynamic updates for on-the-fly switching between
> >> process pagetables.  GPU driver was not using DMA domain and
> >> the GPU's firmware was always expecting to run out  of contextbank
> >>  '0' (although not correct) , which was not the case after the DMA domain
> >> was made default  as '0' was getting allocated for DMA domain and
> >> there were concerns about reusing the DMA domain as well.
> >> Now with this series, looks there is an way out of that that can be tried.
> >>
> >> So should the default domain not be per device specific selectable ?
> >
> > Note that iommu-drivers can request direct-mapping for any given device
> > on its initializtion. This is used on x86 for devices that need a 1-1
> > mapping for some reason.
> >
> > Also device drivers can use the iommu-api and assign their own domain to
> > a device, which allows them to manage the dma address space on their
> > own.
> 
> Part of the problem is that dev->archdata.dma_ops gets wired up to
> iommu_dma_ops.  Which isn't so bad on it's own, except that cache ops
> are not exposed to drivers, forcing us to use dma-mapping API
> (dma_map_sg, etc) for cache operations.
> 
> Possibly we should just expose cache op's to drivers bypass this abuse
> of dma-mapping.
> 
> btw, Will, we definitely want this to *not* rely on kcmdline for the
> gpu with it's own private iommu case..

I still need to understand the unmanaged domain case, but I don't really
see why that's related to this series to be honest.

Will

[PATCH 0/5] Implement SMMU passthrough using the default domain

[Will Deacon]

On Thu, Feb 02, 2017 at 10:02:50AM -0500, Rob Clark wrote:
> On Thu, Jan 26, 2017 at 12:18 PM, Joerg Roedel <joro@8bytes.org> wrote:
> > On Tue, Jan 24, 2017 at 08:42:23PM +0530, Sricharan wrote:
> >> Thanks for this series. We had a case with the GPU.
> >> The GPU's iommu was setup by kernel and the GPU
> >> also does dynamic updates for on-the-fly switching between
> >> process pagetables.  GPU driver was not using DMA domain and
> >> the GPU's firmware was always expecting to run out  of contextbank
> >>  '0' (although not correct) , which was not the case after the DMA domain
> >> was made default  as '0' was getting allocated for DMA domain and
> >> there were concerns about reusing the DMA domain as well.
> >> Now with this series, looks there is an way out of that that can be tried.
> >>
> >> So should the default domain not be per device specific selectable ?
> >
> > Note that iommu-drivers can request direct-mapping for any given device
> > on its initializtion. This is used on x86 for devices that need a 1-1
> > mapping for some reason.
> >
> > Also device drivers can use the iommu-api and assign their own domain to
> > a device, which allows them to manage the dma address space on their
> > own.
> 
> Part of the problem is that dev->archdata.dma_ops gets wired up to
> iommu_dma_ops.  Which isn't so bad on it's own, except that cache ops
> are not exposed to drivers, forcing us to use dma-mapping API
> (dma_map_sg, etc) for cache operations.
> 
> Possibly we should just expose cache op's to drivers bypass this abuse
> of dma-mapping.
> 
> btw, Will, we definitely want this to *not* rely on kcmdline for the
> gpu with it's own private iommu case..

I still need to understand the unmanaged domain case, but I don't really
see why that's related to this series to be honest.

Will

RE: [PATCH 0/5] Implement SMMU passthrough using the default domain

[Sricharan]

Hi Rob,

>-----Original Message-----
>From: linux-arm-kernel [mailto:linux-arm-kernel-bounces-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org] On Behalf Of Rob Clark
>Sent: Thursday, February 02, 2017 8:33 PM
>To: Joerg Roedel <joro-zLv9SwRftAIdnm+yROfE0A@public.gmane.org>
>Cc: Will Deacon <will.deacon-5wv7dgnIgG8@public.gmane.org>; iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org; Sricharan <sricharan-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>; linux-arm-
>kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
>Subject: Re: [PATCH 0/5] Implement SMMU passthrough using the default domain
>
>On Thu, Jan 26, 2017 at 12:18 PM, Joerg Roedel <joro-zLv9SwRftAIdnm+yROfE0A@public.gmane.org> wrote:
>> On Tue, Jan 24, 2017 at 08:42:23PM +0530, Sricharan wrote:
>>> Thanks for this series. We had a case with the GPU.
>>> The GPU's iommu was setup by kernel and the GPU
>>> also does dynamic updates for on-the-fly switching between
>>> process pagetables.  GPU driver was not using DMA domain and
>>> the GPU's firmware was always expecting to run out  of contextbank
>>>  '0' (although not correct) , which was not the case after the DMA domain
>>> was made default  as '0' was getting allocated for DMA domain and
>>> there were concerns about reusing the DMA domain as well.
>>> Now with this series, looks there is an way out of that that can be tried.
>>>
>>> So should the default domain not be per device specific selectable ?
>>
>> Note that iommu-drivers can request direct-mapping for any given device
>> on its initializtion. This is used on x86 for devices that need a 1-1
>> mapping for some reason.
>>
>> Also device drivers can use the iommu-api and assign their own domain to
>> a device, which allows them to manage the dma address space on their
>> own.
>
>Part of the problem is that dev->archdata.dma_ops gets wired up to
>iommu_dma_ops.  Which isn't so bad on it's own, except that cache ops
>are not exposed to drivers, forcing us to use dma-mapping API
>(dma_map_sg, etc) for cache operations.
>
>Possibly we should just expose cache op's to drivers bypass this abuse
>of dma-mapping.
>
[1], with this, when the default domain in not DOMAIN_DMA, then
dev->archdata.dma_ops is not set to iommu_dma_ops , instead remains
to be swiotlb_ops. Is that not correct for gpu's unmanaged domain case ?

https://www.spinics.net/lists/arm-kernel/msg556209.html

>btw, Will, we definitely want this to *not* rely on kcmdline for the
>gpu with it's own private iommu case..

Ya, that changes behavior for all devices and some might want 
DMA_DOMAIN and some UNMANAGED.

Regards,
 Sricharan

>
>BR,
>-R
>
>_______________________________________________
>linux-arm-kernel mailing list
>linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
>http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 0/5] Implement SMMU passthrough using the default domain

[Sricharan]

Hi Rob,

>-----Original Message-----
>From: linux-arm-kernel [mailto:linux-arm-kernel-bounces at lists.infradead.org] On Behalf Of Rob Clark
>Sent: Thursday, February 02, 2017 8:33 PM
>To: Joerg Roedel <joro@8bytes.org>
>Cc: Will Deacon <will.deacon@arm.com>; iommu at lists.linux-foundation.org; Sricharan <sricharan@codeaurora.org>; linux-arm-
>kernel at lists.infradead.org
>Subject: Re: [PATCH 0/5] Implement SMMU passthrough using the default domain
>
>On Thu, Jan 26, 2017 at 12:18 PM, Joerg Roedel <joro@8bytes.org> wrote:
>> On Tue, Jan 24, 2017 at 08:42:23PM +0530, Sricharan wrote:
>>> Thanks for this series. We had a case with the GPU.
>>> The GPU's iommu was setup by kernel and the GPU
>>> also does dynamic updates for on-the-fly switching between
>>> process pagetables.  GPU driver was not using DMA domain and
>>> the GPU's firmware was always expecting to run out  of contextbank
>>>  '0' (although not correct) , which was not the case after the DMA domain
>>> was made default  as '0' was getting allocated for DMA domain and
>>> there were concerns about reusing the DMA domain as well.
>>> Now with this series, looks there is an way out of that that can be tried.
>>>
>>> So should the default domain not be per device specific selectable ?
>>
>> Note that iommu-drivers can request direct-mapping for any given device
>> on its initializtion. This is used on x86 for devices that need a 1-1
>> mapping for some reason.
>>
>> Also device drivers can use the iommu-api and assign their own domain to
>> a device, which allows them to manage the dma address space on their
>> own.
>
>Part of the problem is that dev->archdata.dma_ops gets wired up to
>iommu_dma_ops.  Which isn't so bad on it's own, except that cache ops
>are not exposed to drivers, forcing us to use dma-mapping API
>(dma_map_sg, etc) for cache operations.
>
>Possibly we should just expose cache op's to drivers bypass this abuse
>of dma-mapping.
>
[1], with this, when the default domain in not DOMAIN_DMA, then
dev->archdata.dma_ops is not set to iommu_dma_ops , instead remains
to be swiotlb_ops. Is that not correct for gpu's unmanaged domain case ?

https://www.spinics.net/lists/arm-kernel/msg556209.html

>btw, Will, we definitely want this to *not* rely on kcmdline for the
>gpu with it's own private iommu case..

Ya, that changes behavior for all devices and some might want 
DMA_DOMAIN and some UNMANAGED.

Regards,
 Sricharan

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

Re: [PATCH 0/5] Implement SMMU passthrough using the default domain

[Will Deacon]

On Thu, Feb 02, 2017 at 09:15:19PM +0530, Sricharan wrote:
> Hi Rob,
> 
> >-----Original Message-----
> >From: linux-arm-kernel [mailto:linux-arm-kernel-bounces-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org] On Behalf Of Rob Clark
> >Sent: Thursday, February 02, 2017 8:33 PM
> >To: Joerg Roedel <joro-zLv9SwRftAIdnm+yROfE0A@public.gmane.org>
> >Cc: Will Deacon <will.deacon-5wv7dgnIgG8@public.gmane.org>; iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org; Sricharan <sricharan-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>; linux-arm-
> >kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
> >Subject: Re: [PATCH 0/5] Implement SMMU passthrough using the default domain
> >
> >On Thu, Jan 26, 2017 at 12:18 PM, Joerg Roedel <joro-zLv9SwRftAIdnm+yROfE0A@public.gmane.org> wrote:
> >> On Tue, Jan 24, 2017 at 08:42:23PM +0530, Sricharan wrote:
> >>> Thanks for this series. We had a case with the GPU.
> >>> The GPU's iommu was setup by kernel and the GPU
> >>> also does dynamic updates for on-the-fly switching between
> >>> process pagetables.  GPU driver was not using DMA domain and
> >>> the GPU's firmware was always expecting to run out  of contextbank
> >>>  '0' (although not correct) , which was not the case after the DMA domain
> >>> was made default  as '0' was getting allocated for DMA domain and
> >>> there were concerns about reusing the DMA domain as well.
> >>> Now with this series, looks there is an way out of that that can be tried.
> >>>
> >>> So should the default domain not be per device specific selectable ?
> >>
> >> Note that iommu-drivers can request direct-mapping for any given device
> >> on its initializtion. This is used on x86 for devices that need a 1-1
> >> mapping for some reason.
> >>
> >> Also device drivers can use the iommu-api and assign their own domain to
> >> a device, which allows them to manage the dma address space on their
> >> own.
> >
> >Part of the problem is that dev->archdata.dma_ops gets wired up to
> >iommu_dma_ops.  Which isn't so bad on it's own, except that cache ops
> >are not exposed to drivers, forcing us to use dma-mapping API
> >(dma_map_sg, etc) for cache operations.
> >
> >Possibly we should just expose cache op's to drivers bypass this abuse
> >of dma-mapping.
> >
> [1], with this, when the default domain in not DOMAIN_DMA, then
> dev->archdata.dma_ops is not set to iommu_dma_ops , instead remains
> to be swiotlb_ops. Is that not correct for gpu's unmanaged domain case ?
> 
> https://www.spinics.net/lists/arm-kernel/msg556209.html
> 
> >btw, Will, we definitely want this to *not* rely on kcmdline for the
> >gpu with it's own private iommu case..
> 
> Ya, that changes behavior for all devices and some might want 
> DMA_DOMAIN and some UNMANAGED.

My patch changes the *default* domain. When would this ever be UNMANAGED?

If you're using an UNMANAGED domain, I think you need to take care of the
DMA ops yourself. There are likely missing functions to do that, but they
should be added in a separate series.

Will

[PATCH 0/5] Implement SMMU passthrough using the default domain

[Will Deacon]

On Thu, Feb 02, 2017 at 09:15:19PM +0530, Sricharan wrote:
> Hi Rob,
> 
> >-----Original Message-----
> >From: linux-arm-kernel [mailto:linux-arm-kernel-bounces at lists.infradead.org] On Behalf Of Rob Clark
> >Sent: Thursday, February 02, 2017 8:33 PM
> >To: Joerg Roedel <joro@8bytes.org>
> >Cc: Will Deacon <will.deacon@arm.com>; iommu at lists.linux-foundation.org; Sricharan <sricharan@codeaurora.org>; linux-arm-
> >kernel at lists.infradead.org
> >Subject: Re: [PATCH 0/5] Implement SMMU passthrough using the default domain
> >
> >On Thu, Jan 26, 2017 at 12:18 PM, Joerg Roedel <joro@8bytes.org> wrote:
> >> On Tue, Jan 24, 2017 at 08:42:23PM +0530, Sricharan wrote:
> >>> Thanks for this series. We had a case with the GPU.
> >>> The GPU's iommu was setup by kernel and the GPU
> >>> also does dynamic updates for on-the-fly switching between
> >>> process pagetables.  GPU driver was not using DMA domain and
> >>> the GPU's firmware was always expecting to run out  of contextbank
> >>>  '0' (although not correct) , which was not the case after the DMA domain
> >>> was made default  as '0' was getting allocated for DMA domain and
> >>> there were concerns about reusing the DMA domain as well.
> >>> Now with this series, looks there is an way out of that that can be tried.
> >>>
> >>> So should the default domain not be per device specific selectable ?
> >>
> >> Note that iommu-drivers can request direct-mapping for any given device
> >> on its initializtion. This is used on x86 for devices that need a 1-1
> >> mapping for some reason.
> >>
> >> Also device drivers can use the iommu-api and assign their own domain to
> >> a device, which allows them to manage the dma address space on their
> >> own.
> >
> >Part of the problem is that dev->archdata.dma_ops gets wired up to
> >iommu_dma_ops.  Which isn't so bad on it's own, except that cache ops
> >are not exposed to drivers, forcing us to use dma-mapping API
> >(dma_map_sg, etc) for cache operations.
> >
> >Possibly we should just expose cache op's to drivers bypass this abuse
> >of dma-mapping.
> >
> [1], with this, when the default domain in not DOMAIN_DMA, then
> dev->archdata.dma_ops is not set to iommu_dma_ops , instead remains
> to be swiotlb_ops. Is that not correct for gpu's unmanaged domain case ?
> 
> https://www.spinics.net/lists/arm-kernel/msg556209.html
> 
> >btw, Will, we definitely want this to *not* rely on kcmdline for the
> >gpu with it's own private iommu case..
> 
> Ya, that changes behavior for all devices and some might want 
> DMA_DOMAIN and some UNMANAGED.

My patch changes the *default* domain. When would this ever be UNMANAGED?

If you're using an UNMANAGED domain, I think you need to take care of the
DMA ops yourself. There are likely missing functions to do that, but they
should be added in a separate series.

Will

Re: [PATCH 0/5] Implement SMMU passthrough using the default domain

[Rob Clark]

On Thu, Feb 2, 2017 at 10:12 AM, Will Deacon <will.deacon-5wv7dgnIgG8@public.gmane.org> wrote:
> On Thu, Feb 02, 2017 at 10:02:50AM -0500, Rob Clark wrote:
>> On Thu, Jan 26, 2017 at 12:18 PM, Joerg Roedel <joro-zLv9SwRftAIdnm+yROfE0A@public.gmane.org> wrote:
>> > On Tue, Jan 24, 2017 at 08:42:23PM +0530, Sricharan wrote:
>> >> Thanks for this series. We had a case with the GPU.
>> >> The GPU's iommu was setup by kernel and the GPU
>> >> also does dynamic updates for on-the-fly switching between
>> >> process pagetables.  GPU driver was not using DMA domain and
>> >> the GPU's firmware was always expecting to run out  of contextbank
>> >>  '0' (although not correct) , which was not the case after the DMA domain
>> >> was made default  as '0' was getting allocated for DMA domain and
>> >> there were concerns about reusing the DMA domain as well.
>> >> Now with this series, looks there is an way out of that that can be tried.
>> >>
>> >> So should the default domain not be per device specific selectable ?
>> >
>> > Note that iommu-drivers can request direct-mapping for any given device
>> > on its initializtion. This is used on x86 for devices that need a 1-1
>> > mapping for some reason.
>> >
>> > Also device drivers can use the iommu-api and assign their own domain to
>> > a device, which allows them to manage the dma address space on their
>> > own.
>>
>> Part of the problem is that dev->archdata.dma_ops gets wired up to
>> iommu_dma_ops.  Which isn't so bad on it's own, except that cache ops
>> are not exposed to drivers, forcing us to use dma-mapping API
>> (dma_map_sg, etc) for cache operations.
>>
>> Possibly we should just expose cache op's to drivers bypass this abuse
>> of dma-mapping.
>>
>> btw, Will, we definitely want this to *not* rely on kcmdline for the
>> gpu with it's own private iommu case..
>
> I still need to understand the unmanaged domain case, but I don't really
> see why that's related to this series to be honest.
>

Only relation is if we were trying to solve the case of a driver that
needs to manage it's own domain with this patchset..  but that seems a
bit like "when all you have is a hammer, everything looks like a
nail"..

BR,
-R

[PATCH 0/5] Implement SMMU passthrough using the default domain

[Rob Clark]

On Thu, Feb 2, 2017 at 10:12 AM, Will Deacon <will.deacon@arm.com> wrote:
> On Thu, Feb 02, 2017 at 10:02:50AM -0500, Rob Clark wrote:
>> On Thu, Jan 26, 2017 at 12:18 PM, Joerg Roedel <joro@8bytes.org> wrote:
>> > On Tue, Jan 24, 2017 at 08:42:23PM +0530, Sricharan wrote:
>> >> Thanks for this series. We had a case with the GPU.
>> >> The GPU's iommu was setup by kernel and the GPU
>> >> also does dynamic updates for on-the-fly switching between
>> >> process pagetables.  GPU driver was not using DMA domain and
>> >> the GPU's firmware was always expecting to run out  of contextbank
>> >>  '0' (although not correct) , which was not the case after the DMA domain
>> >> was made default  as '0' was getting allocated for DMA domain and
>> >> there were concerns about reusing the DMA domain as well.
>> >> Now with this series, looks there is an way out of that that can be tried.
>> >>
>> >> So should the default domain not be per device specific selectable ?
>> >
>> > Note that iommu-drivers can request direct-mapping for any given device
>> > on its initializtion. This is used on x86 for devices that need a 1-1
>> > mapping for some reason.
>> >
>> > Also device drivers can use the iommu-api and assign their own domain to
>> > a device, which allows them to manage the dma address space on their
>> > own.
>>
>> Part of the problem is that dev->archdata.dma_ops gets wired up to
>> iommu_dma_ops.  Which isn't so bad on it's own, except that cache ops
>> are not exposed to drivers, forcing us to use dma-mapping API
>> (dma_map_sg, etc) for cache operations.
>>
>> Possibly we should just expose cache op's to drivers bypass this abuse
>> of dma-mapping.
>>
>> btw, Will, we definitely want this to *not* rely on kcmdline for the
>> gpu with it's own private iommu case..
>
> I still need to understand the unmanaged domain case, but I don't really
> see why that's related to this series to be honest.
>

Only relation is if we were trying to solve the case of a driver that
needs to manage it's own domain with this patchset..  but that seems a
bit like "when all you have is a hammer, everything looks like a
nail"..

BR,
-R

RE: [PATCH 0/5] Implement SMMU passthrough using the default domain

[Sricharan]

Hi Will, Rob,

>-----Original Message-----
>From: linux-arm-kernel [mailto:linux-arm-kernel-bounces-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org] On Behalf Of Will Deacon
>Sent: Thursday, February 02, 2017 9:41 PM
>To: Sricharan <sricharan-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
>Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org; 'Rob Clark' <robdclark-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>; 'Joerg Roedel' <joro-zLv9SwRftAIdnm+yROfE0A@public.gmane.org>; iommu-cunTk1MwBs/ROKNJybVBZg@public.gmane.org
>foundation.org
>Subject: Re: [PATCH 0/5] Implement SMMU passthrough using the default domain
>
>On Thu, Feb 02, 2017 at 09:15:19PM +0530, Sricharan wrote:
>> Hi Rob,
>>
>> >-----Original Message-----
>> >From: linux-arm-kernel [mailto:linux-arm-kernel-bounces-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org] On Behalf Of Rob Clark
>> >Sent: Thursday, February 02, 2017 8:33 PM
>> >To: Joerg Roedel <joro-zLv9SwRftAIdnm+yROfE0A@public.gmane.org>
>> >Cc: Will Deacon <will.deacon-5wv7dgnIgG8@public.gmane.org>; iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org; Sricharan <sricharan-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>; linux-arm-
>> >kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
>> >Subject: Re: [PATCH 0/5] Implement SMMU passthrough using the default domain
>> >
>> >On Thu, Jan 26, 2017 at 12:18 PM, Joerg Roedel <joro-zLv9SwRftAIdnm+yROfE0A@public.gmane.org> wrote:
>> >> On Tue, Jan 24, 2017 at 08:42:23PM +0530, Sricharan wrote:
>> >>> Thanks for this series. We had a case with the GPU.
>> >>> The GPU's iommu was setup by kernel and the GPU
>> >>> also does dynamic updates for on-the-fly switching between
>> >>> process pagetables.  GPU driver was not using DMA domain and
>> >>> the GPU's firmware was always expecting to run out  of contextbank
>> >>>  '0' (although not correct) , which was not the case after the DMA domain
>> >>> was made default  as '0' was getting allocated for DMA domain and
>> >>> there were concerns about reusing the DMA domain as well.
>> >>> Now with this series, looks there is an way out of that that can be tried.
>> >>>
>> >>> So should the default domain not be per device specific selectable ?
>> >>
>> >> Note that iommu-drivers can request direct-mapping for any given device
>> >> on its initializtion. This is used on x86 for devices that need a 1-1
>> >> mapping for some reason.
>> >>
>> >> Also device drivers can use the iommu-api and assign their own domain to
>> >> a device, which allows them to manage the dma address space on their
>> >> own.
>> >
>> >Part of the problem is that dev->archdata.dma_ops gets wired up to
>> >iommu_dma_ops.  Which isn't so bad on it's own, except that cache ops
>> >are not exposed to drivers, forcing us to use dma-mapping API
>> >(dma_map_sg, etc) for cache operations.
>> >
>> >Possibly we should just expose cache op's to drivers bypass this abuse
>> >of dma-mapping.
>> >
>> [1], with this, when the default domain in not DOMAIN_DMA, then
>> dev->archdata.dma_ops is not set to iommu_dma_ops , instead remains
>> to be swiotlb_ops. Is that not correct for gpu's unmanaged domain case ?
>>
>> https://www.spinics.net/lists/arm-kernel/msg556209.html
>>
>> >btw, Will, we definitely want this to *not* rely on kcmdline for the
>> >gpu with it's own private iommu case..
>>
>> Ya, that changes behavior for all devices and some might want
>> DMA_DOMAIN and some UNMANAGED.
>
>My patch changes the *default* domain. When would this ever be UNMANAGED?
>
>If you're using an UNMANAGED domain, I think you need to take care of the
>DMA ops yourself. There are likely missing functions to do that, but they
>should be added in a separate series.

Sorry, i should have said DOMAIN_DMA or DOMAIN_IDENTITY is set as
*default* domains from your patchset. So i have not tested this series
yet. But the requirement that i thought for the gpu was
    * a context bank (specially '0') should not be reserved for DOMAIN_DMA
   * dma_ops should not be set to iommu_dma_ops for the gpu device.

I was seeing that both of that are satisfied in this series, if we choose
IDENTITY_DOMAIN as default and later gpu attaches it own
*UNMANAGED* domain.

The only thing i was thinking was, since we are changing the default domain
for all device from command line, some device which require DOMAIN_DMA
and some which require DOMAIN_IDENTITY as default was not possible.

Regards,
 Sricharan

[PATCH 0/5] Implement SMMU passthrough using the default domain

[Sricharan]

Hi Will, Rob,

>-----Original Message-----
>From: linux-arm-kernel [mailto:linux-arm-kernel-bounces at lists.infradead.org] On Behalf Of Will Deacon
>Sent: Thursday, February 02, 2017 9:41 PM
>To: Sricharan <sricharan@codeaurora.org>
>Cc: linux-arm-kernel at lists.infradead.org; 'Rob Clark' <robdclark@gmail.com>; 'Joerg Roedel' <joro@8bytes.org>; iommu at lists.linux-
>foundation.org
>Subject: Re: [PATCH 0/5] Implement SMMU passthrough using the default domain
>
>On Thu, Feb 02, 2017 at 09:15:19PM +0530, Sricharan wrote:
>> Hi Rob,
>>
>> >-----Original Message-----
>> >From: linux-arm-kernel [mailto:linux-arm-kernel-bounces at lists.infradead.org] On Behalf Of Rob Clark
>> >Sent: Thursday, February 02, 2017 8:33 PM
>> >To: Joerg Roedel <joro@8bytes.org>
>> >Cc: Will Deacon <will.deacon@arm.com>; iommu at lists.linux-foundation.org; Sricharan <sricharan@codeaurora.org>; linux-arm-
>> >kernel at lists.infradead.org
>> >Subject: Re: [PATCH 0/5] Implement SMMU passthrough using the default domain
>> >
>> >On Thu, Jan 26, 2017 at 12:18 PM, Joerg Roedel <joro@8bytes.org> wrote:
>> >> On Tue, Jan 24, 2017 at 08:42:23PM +0530, Sricharan wrote:
>> >>> Thanks for this series. We had a case with the GPU.
>> >>> The GPU's iommu was setup by kernel and the GPU
>> >>> also does dynamic updates for on-the-fly switching between
>> >>> process pagetables.  GPU driver was not using DMA domain and
>> >>> the GPU's firmware was always expecting to run out  of contextbank
>> >>>  '0' (although not correct) , which was not the case after the DMA domain
>> >>> was made default  as '0' was getting allocated for DMA domain and
>> >>> there were concerns about reusing the DMA domain as well.
>> >>> Now with this series, looks there is an way out of that that can be tried.
>> >>>
>> >>> So should the default domain not be per device specific selectable ?
>> >>
>> >> Note that iommu-drivers can request direct-mapping for any given device
>> >> on its initializtion. This is used on x86 for devices that need a 1-1
>> >> mapping for some reason.
>> >>
>> >> Also device drivers can use the iommu-api and assign their own domain to
>> >> a device, which allows them to manage the dma address space on their
>> >> own.
>> >
>> >Part of the problem is that dev->archdata.dma_ops gets wired up to
>> >iommu_dma_ops.  Which isn't so bad on it's own, except that cache ops
>> >are not exposed to drivers, forcing us to use dma-mapping API
>> >(dma_map_sg, etc) for cache operations.
>> >
>> >Possibly we should just expose cache op's to drivers bypass this abuse
>> >of dma-mapping.
>> >
>> [1], with this, when the default domain in not DOMAIN_DMA, then
>> dev->archdata.dma_ops is not set to iommu_dma_ops , instead remains
>> to be swiotlb_ops. Is that not correct for gpu's unmanaged domain case ?
>>
>> https://www.spinics.net/lists/arm-kernel/msg556209.html
>>
>> >btw, Will, we definitely want this to *not* rely on kcmdline for the
>> >gpu with it's own private iommu case..
>>
>> Ya, that changes behavior for all devices and some might want
>> DMA_DOMAIN and some UNMANAGED.
>
>My patch changes the *default* domain. When would this ever be UNMANAGED?
>
>If you're using an UNMANAGED domain, I think you need to take care of the
>DMA ops yourself. There are likely missing functions to do that, but they
>should be added in a separate series.

Sorry, i should have said DOMAIN_DMA or DOMAIN_IDENTITY is set as
*default* domains from your patchset. So i have not tested this series
yet. But the requirement that i thought for the gpu was
    * a context bank (specially '0') should not be reserved for DOMAIN_DMA
   * dma_ops should not be set to iommu_dma_ops for the gpu device.

I was seeing that both of that are satisfied in this series, if we choose
IDENTITY_DOMAIN as default and later gpu attaches it own
*UNMANAGED* domain.

The only thing i was thinking was, since we are changing the default domain
for all device from command line, some device which require DOMAIN_DMA
and some which require DOMAIN_IDENTITY as default was not possible.

Regards,
 Sricharan