[PATCH V7 0/8] Add support for privileged mappings

[PATCH V7 0/8] Add support for privileged mappings

[Sricharan R]

This series is a resend of the V5 that Mitch sent sometime back [2]
All the patches are the same and i have just rebased. Redid patch [3],
as it does not apply in this code base. Added a couple of more patches
[4], [5] from Robin for adding the privileged attributes to armv7s format
and arm-smmuv3 revert.

The following patch to the ARM SMMU driver:

    commit d346180e70b91b3d5a1ae7e5603e65593d4622bc
    Author: Robin Murphy <robin.murphy@arm.com>
    Date:   Tue Jan 26 18:06:34 2016 +0000
    
        iommu/arm-smmu: Treat all device transactions as unprivileged

started forcing all SMMU transactions to come through as "unprivileged".
The rationale given was that:

  (1) There is no way in the IOMMU API to even request privileged
      mappings.

  (2) It's difficult to implement a DMA mapper that correctly models the
      ARM VMSAv8 behavior of unprivileged-writeable =>
      privileged-execute-never.

This series rectifies (1) by introducing an IOMMU API for privileged
mappings and implements it in io-pgtable-arm.

This series rectifies (2) by introducing a new dma attribute
(DMA_ATTR_PRIVILEGED) for users of the DMA API that need privileged
mappings which are inaccessible to lesser-privileged execution levels, and
implements it in the arm64 IOMMU DMA mapper.  The one known user (pl330.c)
is converted over to the new attribute.

Jordan and Jeremy can provide more info on the use case if needed, but the
high level is that it's a security feature to prevent attacks such as [1].

Note that, i tested this on arm64 with arm-smmuv2, short descriptor changes,
and do not have an platform to test this with arm-smmuv3.

[1] https://github.com/robclark/kilroy
[2] https://lkml.org/lkml/2016/7/27/590
[3] https://patchwork.kernel.org/patch/9250493/
[4] http://www.linux-arm.org/git?p=linux-rm.git;a=commit;h=1291bd74f05d31da1dab3df02987cba5bd25849b
[5] http://www.linux-arm.org/git?p=linux-rm.git;a=commit;h=a79c1c6333f26849dba418cd92de26b60f5954f3

Changelog:
 v6..v7
    - Added couple of more patches, picked up acks, updated commit log

 v5..v6
    - Rebased all the patches and redid 6/6 as it does not apply in
      this code base. 

 v4..v5

    - Simplified patch 4/6 (suggested by Robin Murphy).

  v3..v4

    - Rebased and reworked on linux next due to the dma attrs rework going
      on over there.  Patches changed: 3/6, 4/6, and 5/6.

  v2..v3

    - Incorporated feedback from Robin:
      * Various comments and re-wordings.
      * Use existing bit definitions for IOMMU_PRIV implementation
        in io-pgtable-arm.
      * Renamed and redocumented dma_direction_to_prot.
      * Don't worry about executability in new DMA attr.

  v1..v2

    - Added a new DMA attribute to make executable privileged mappings
      work, and use that in the pl330 driver (suggested by Will).

Jeremy Gebben (1):
  iommu/io-pgtable-arm: add support for the IOMMU_PRIV flag

Mitchel Humpherys (4):
  iommu: add IOMMU_PRIV attribute
  common: DMA-mapping: add DMA_ATTR_PRIVILEGED attribute
  arm64/dma-mapping: Implement DMA_ATTR_PRIVILEGED
  dmaengine: pl330: Make sure microcode is privileged

Robin Murphy (2):
  iommu/io-pgtable-arm-v7s: Add support for the IOMMU_PRIV flag
  iommu/arm-smmu: Revert "iommu/arm-smmu: Set PRIVCFG in stage 1 STEs"

Sricharan R (1):
  iommu/arm-smmu: Set privileged attribute to 'default' instead of
    'unprivileged'

 Documentation/DMA-attributes.txt   | 10 ++++++++++
 arch/arm64/mm/dma-mapping.c        |  6 +++---
 drivers/dma/pl330.c                |  5 +++--
 drivers/iommu/arm-smmu-v3.c        |  7 +------
 drivers/iommu/arm-smmu.c           |  2 +-
 drivers/iommu/dma-iommu.c          | 10 ++++++++--
 drivers/iommu/io-pgtable-arm-v7s.c |  6 +++++-
 drivers/iommu/io-pgtable-arm.c     |  5 ++++-
 include/linux/dma-iommu.h          |  3 ++-
 include/linux/dma-mapping.h        |  7 +++++++
 include/linux/iommu.h              |  1 +
 11 files changed, 45 insertions(+), 17 deletions(-)

-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

[PATCH V7 1/8] iommu: add IOMMU_PRIV attribute

[Sricharan R]

From: Mitchel Humpherys <mitchelh@codeaurora.org>

Add the IOMMU_PRIV attribute, which is used to indicate privileged
mappings.

Reviewed-by: Robin Murphy <robin.murphy@arm.com>
Tested-by: Robin Murphy <robin.murphy@arm.com>
Signed-off-by: Mitchel Humpherys <mitchelh@codeaurora.org>
Acked-by: Will Deacon <will.deacon@arm.com>
---
 include/linux/iommu.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index f2960e4..bf22131 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -31,6 +31,7 @@
 #define IOMMU_CACHE	(1 << 2) /* DMA cache coherency */
 #define IOMMU_NOEXEC	(1 << 3)
 #define IOMMU_MMIO	(1 << 4) /* e.g. things like MSI doorbells */
+#define IOMMU_PRIV	(1 << 5) /* privileged */
 
 struct iommu_ops;
 struct iommu_group;
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

[PATCH V7 2/8] iommu/io-pgtable-arm: add support for the IOMMU_PRIV flag

[Sricharan R]

From: Jeremy Gebben <jgebben@codeaurora.org>

Allow the creation of privileged mode mappings, for stage 1 only.

Reviewed-by: Robin Murphy <robin.murphy@arm.com>
Tested-by: Robin Murphy <robin.murphy@arm.com>
Acked-by: Will Deacon <will.deacon@arm.com>
Signed-off-by: Jeremy Gebben <jgebben@codeaurora.org>
---
 drivers/iommu/io-pgtable-arm.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/iommu/io-pgtable-arm.c b/drivers/iommu/io-pgtable-arm.c
index f5c90e1..69ba83a 100644
--- a/drivers/iommu/io-pgtable-arm.c
+++ b/drivers/iommu/io-pgtable-arm.c
@@ -350,11 +350,14 @@ static arm_lpae_iopte arm_lpae_prot_to_pte(struct arm_lpae_io_pgtable *data,
 
 	if (data->iop.fmt == ARM_64_LPAE_S1 ||
 	    data->iop.fmt == ARM_32_LPAE_S1) {
-		pte = ARM_LPAE_PTE_AP_UNPRIV | ARM_LPAE_PTE_nG;
+		pte = ARM_LPAE_PTE_nG;
 
 		if (!(prot & IOMMU_WRITE) && (prot & IOMMU_READ))
 			pte |= ARM_LPAE_PTE_AP_RDONLY;
 
+		if (!(prot & IOMMU_PRIV))
+			pte |= ARM_LPAE_PTE_AP_UNPRIV;
+
 		if (prot & IOMMU_MMIO)
 			pte |= (ARM_LPAE_MAIR_ATTR_IDX_DEV
 				<< ARM_LPAE_PTE_ATTRINDX_SHIFT);
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

[PATCH V7 3/8] iommu/io-pgtable-arm-v7s: Add support for the IOMMU_PRIV flag

[Sricharan R]

From: Robin Murphy <robin.murphy@arm.com>

The short-descriptor format also allows privileged-only mappings, so
let's wire it up.

Signed-off-by: Robin Murphy <robin.murphy@arm.com>
Tested-by: Sricharan R <sricharan@codeaurora.org>
---
 drivers/iommu/io-pgtable-arm-v7s.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/iommu/io-pgtable-arm-v7s.c b/drivers/iommu/io-pgtable-arm-v7s.c
index f50e51c..1177782 100644
--- a/drivers/iommu/io-pgtable-arm-v7s.c
+++ b/drivers/iommu/io-pgtable-arm-v7s.c
@@ -265,7 +265,9 @@ static arm_v7s_iopte arm_v7s_prot_to_pte(int prot, int lvl,
 	if (!(prot & IOMMU_MMIO))
 		pte |= ARM_V7S_ATTR_TEX(1);
 	if (ap) {
-		pte |= ARM_V7S_PTE_AF | ARM_V7S_PTE_AP_UNPRIV;
+		pte |= ARM_V7S_PTE_AF;
+		if (!(prot & IOMMU_PRIV))
+			pte |= ARM_V7S_PTE_AP_UNPRIV;
 		if (!(prot & IOMMU_WRITE))
 			pte |= ARM_V7S_PTE_AP_RDONLY;
 	}
@@ -288,6 +290,8 @@ static int arm_v7s_pte_to_prot(arm_v7s_iopte pte, int lvl)
 
 	if (!(attr & ARM_V7S_PTE_AP_RDONLY))
 		prot |= IOMMU_WRITE;
+	if (!(attr & ARM_V7S_PTE_AP_UNPRIV))
+		prot |= IOMMU_PRIV;
 	if ((attr & (ARM_V7S_TEX_MASK << ARM_V7S_TEX_SHIFT)) == 0)
 		prot |= IOMMU_MMIO;
 	else if (pte & ARM_V7S_ATTR_C)
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

[PATCH V7 4/8] common: DMA-mapping: add DMA_ATTR_PRIVILEGED attribute

[Sricharan R]

From: Mitchel Humpherys <mitchelh@codeaurora.org>

This patch adds the DMA_ATTR_PRIVILEGED attribute to the DMA-mapping
subsystem.

Some advanced peripherals such as remote processors and GPUs perform
accesses to DMA buffers in both privileged "supervisor" and unprivileged
"user" modes.  This attribute is used to indicate to the DMA-mapping
subsystem that the buffer is fully accessible at the elevated privilege
level (and ideally inaccessible or at least read-only at the
lesser-privileged levels).

Cc: linux-doc@vger.kernel.org
Reviewed-by: Robin Murphy <robin.murphy@arm.com>
Tested-by: Robin Murphy <robin.murphy@arm.com>
Acked-by: Will Deacon <will.deacon@arm.com>
Signed-off-by: Mitchel Humpherys <mitchelh@codeaurora.org>
---
 Documentation/DMA-attributes.txt | 10 ++++++++++
 include/linux/dma-mapping.h      |  7 +++++++
 2 files changed, 17 insertions(+)

diff --git a/Documentation/DMA-attributes.txt b/Documentation/DMA-attributes.txt
index 98bf7ac..44c6bc4 100644
--- a/Documentation/DMA-attributes.txt
+++ b/Documentation/DMA-attributes.txt
@@ -143,3 +143,13 @@ So, this provides a way for drivers to avoid those error messages on calls
 where allocation failures are not a problem, and shouldn't bother the logs.
 
 NOTE: At the moment DMA_ATTR_NO_WARN is only implemented on PowerPC.
+
+DMA_ATTR_PRIVILEGED
+------------------------------
+
+Some advanced peripherals such as remote processors and GPUs perform
+accesses to DMA buffers in both privileged "supervisor" and unprivileged
+"user" modes.  This attribute is used to indicate to the DMA-mapping
+subsystem that the buffer is fully accessible at the elevated privilege
+level (and ideally inaccessible or at least read-only at the
+lesser-privileged levels).
diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
index 6f3e6ca..ee31ea1 100644
--- a/include/linux/dma-mapping.h
+++ b/include/linux/dma-mapping.h
@@ -63,6 +63,13 @@
 #define DMA_ATTR_NO_WARN	(1UL << 8)
 
 /*
+ * DMA_ATTR_PRIVILEGED: used to indicate that the buffer is fully
+ * accessible at an elevated privilege level (and ideally inaccessible or
+ * at least read-only at lesser-privileged levels).
+ */
+#define DMA_ATTR_PRIVILEGED		(1UL << 8)
+
+/*
  * A dma_addr_t can hold any valid DMA or bus address for the platform.
  * It can be given to a device to use as a DMA source or target.  A CPU cannot
  * reference a dma_addr_t directly because there may be translation between
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

[PATCH V7 5/8] arm64/dma-mapping: Implement DMA_ATTR_PRIVILEGED

[Sricharan R]

From: Mitchel Humpherys <mitchelh@codeaurora.org>

The newly added DMA_ATTR_PRIVILEGED is useful for creating mappings that
are only accessible to privileged DMA engines.  Implement it in
dma-iommu.c so that the ARM64 DMA IOMMU mapper can make use of it.

Reviewed-by: Robin Murphy <robin.murphy@arm.com>
Tested-by: Robin Murphy <robin.murphy@arm.com>
Acked-by: Will Deacon <will.deacon@arm.com>
Signed-off-by: Mitchel Humpherys <mitchelh@codeaurora.org>
---
 arch/arm64/mm/dma-mapping.c |  6 +++---
 drivers/iommu/dma-iommu.c   | 10 ++++++++--
 include/linux/dma-iommu.h   |  3 ++-
 3 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/arch/arm64/mm/dma-mapping.c b/arch/arm64/mm/dma-mapping.c
index 401f79a..ae76ead 100644
--- a/arch/arm64/mm/dma-mapping.c
+++ b/arch/arm64/mm/dma-mapping.c
@@ -557,7 +557,7 @@ static void *__iommu_alloc_attrs(struct device *dev, size_t size,
 				 unsigned long attrs)
 {
 	bool coherent = is_device_dma_coherent(dev);
-	int ioprot = dma_direction_to_prot(DMA_BIDIRECTIONAL, coherent);
+	int ioprot = dma_info_to_prot(DMA_BIDIRECTIONAL, coherent, attrs);
 	size_t iosize = size;
 	void *addr;
 
@@ -711,7 +711,7 @@ static dma_addr_t __iommu_map_page(struct device *dev, struct page *page,
 				   unsigned long attrs)
 {
 	bool coherent = is_device_dma_coherent(dev);
-	int prot = dma_direction_to_prot(dir, coherent);
+	int prot = dma_info_to_prot(dir, coherent, attrs);
 	dma_addr_t dev_addr = iommu_dma_map_page(dev, page, offset, size, prot);
 
 	if (!iommu_dma_mapping_error(dev, dev_addr) &&
@@ -769,7 +769,7 @@ static int __iommu_map_sg_attrs(struct device *dev, struct scatterlist *sgl,
 		__iommu_sync_sg_for_device(dev, sgl, nelems, dir);
 
 	return iommu_dma_map_sg(dev, sgl, nelems,
-			dma_direction_to_prot(dir, coherent));
+				dma_info_to_prot(dir, coherent, attrs));
 }
 
 static void __iommu_unmap_sg_attrs(struct device *dev,
diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index d2a7a46..756d5e0 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -182,16 +182,22 @@ int iommu_dma_init_domain(struct iommu_domain *domain, dma_addr_t base,
 EXPORT_SYMBOL(iommu_dma_init_domain);
 
 /**
- * dma_direction_to_prot - Translate DMA API directions to IOMMU API page flags
+ * dma_info_to_prot - Translate DMA API directions and attributes to IOMMU API
+ *                    page flags.
  * @dir: Direction of DMA transfer
  * @coherent: Is the DMA master cache-coherent?
+ * @attrs: DMA attributes for the mapping
  *
  * Return: corresponding IOMMU API page protection flags
  */
-int dma_direction_to_prot(enum dma_data_direction dir, bool coherent)
+int dma_info_to_prot(enum dma_data_direction dir, bool coherent,
+		     unsigned long attrs)
 {
 	int prot = coherent ? IOMMU_CACHE : 0;
 
+	if (attrs & DMA_ATTR_PRIVILEGED)
+		prot |= IOMMU_PRIV;
+
 	switch (dir) {
 	case DMA_BIDIRECTIONAL:
 		return prot | IOMMU_READ | IOMMU_WRITE;
diff --git a/include/linux/dma-iommu.h b/include/linux/dma-iommu.h
index 32c5890..a203181 100644
--- a/include/linux/dma-iommu.h
+++ b/include/linux/dma-iommu.h
@@ -34,7 +34,8 @@ int iommu_dma_init_domain(struct iommu_domain *domain, dma_addr_t base,
 		u64 size, struct device *dev);
 
 /* General helpers for DMA-API <-> IOMMU-API interaction */
-int dma_direction_to_prot(enum dma_data_direction dir, bool coherent);
+int dma_info_to_prot(enum dma_data_direction dir, bool coherent,
+		     unsigned long attrs);
 
 /*
  * These implement the bulk of the relevant DMA mapping callbacks, but require
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

[PATCH V7 6/8] dmaengine: pl330: Make sure microcode is privileged

[Sricharan R]

From: Mitchel Humpherys <mitchelh@codeaurora.org>

The PL330 is hard-wired such that instruction fetches on both the
manager and channel threads go out onto the bus with the "privileged"
bit set. This can become troublesome once there is an IOMMU or other
form of memory protection downstream, since those will typically be
programmed by the DMA mapping subsystem in the expectation of normal
unprivileged transactions (such as the PL330 channel threads' own data
accesses as currently configured by this driver).

To avoid the case of, say, an IOMMU blocking an unexpected privileged
transaction with a permission fault, use the newly-introduced
DMA_ATTR_PRIVILEGED attribute for the mapping of our microcode buffer.
That way the DMA layer can do whatever it needs to do to make things
continue to work as expected on more complex systems.

Cc: Dan Williams <dan.j.williams@intel.com>
Cc: Vinod Koul <vinod.koul@intel.com>
Reviewed-by: Robin Murphy <robin.murphy@arm.com>
Tested-by: Robin Murphy <robin.murphy@arm.com>
Acked-by: Will Deacon <will.deacon@arm.com>
Acked-by: Vinod Koul <vinod.koul@intel.com>
Signed-off-by: Mitchel Humpherys <mitchelh@codeaurora.org>
[rm: remove now-redundant local variable, clarify commit message]
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
 drivers/dma/pl330.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/dma/pl330.c b/drivers/dma/pl330.c
index 030fe05..1e5ae0c 100644
--- a/drivers/dma/pl330.c
+++ b/drivers/dma/pl330.c
@@ -1859,9 +1859,10 @@ static int dmac_alloc_resources(struct pl330_dmac *pl330)
 	 * Alloc MicroCode buffer for 'chans' Channel threads.
 	 * A channel's buffer offset is (Channel_Id * MCODE_BUFF_PERCHAN)
 	 */
-	pl330->mcode_cpu = dma_alloc_coherent(pl330->ddma.dev,
+	pl330->mcode_cpu = dma_alloc_attrs(pl330->ddma.dev,
 				chans * pl330->mcbufsz,
-				&pl330->mcode_bus, GFP_KERNEL);
+				&pl330->mcode_bus, GFP_KERNEL,
+				DMA_ATTR_PRIVILEGED);
 	if (!pl330->mcode_cpu) {
 		dev_err(pl330->ddma.dev, "%s:%d Can't allocate memory!\n",
 			__func__, __LINE__);
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

[PATCH V7 7/8] iommu/arm-smmu: Set privileged attribute to 'default' instead of 'unprivileged'

[Sricharan R]

Currently the driver sets all the device transactions privileges
to UNPRIVILEGED, but there are cases where the iommu masters wants
to isolate privileged supervisor and unprivileged user.
So don't override the privileged setting to unprivileged, instead
set it to default as incoming and let it be controlled by the pagetable
settings.

Acked-by: Will Deacon <will.deacon@arm.com>
Signed-off-by: Sricharan R <sricharan@codeaurora.org>
---
 drivers/iommu/arm-smmu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index eaa8f44..8bb0eea 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -1213,7 +1213,7 @@ static int arm_smmu_domain_add_master(struct arm_smmu_domain *smmu_domain,
 			continue;
 
 		s2cr[idx].type = type;
-		s2cr[idx].privcfg = S2CR_PRIVCFG_UNPRIV;
+		s2cr[idx].privcfg = S2CR_PRIVCFG_DEFAULT;
 		s2cr[idx].cbndx = cbndx;
 		arm_smmu_write_s2cr(smmu, idx);
 	}
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

[PATCH V7 8/8] iommu/arm-smmu: Revert "iommu/arm-smmu: Set PRIVCFG in stage 1 STEs"

[Sricharan R]

From: Robin Murphy <robin.murphy@arm.com>

Now that proper privileged mappings can be requested via IOMMU_PRIV,
unconditionally overriding the incoming PRIVCFG becomes the wrong thing
to do, so stop it.

This reverts commit df5e1a0f2a2d779ad467a691203bcbc74d75690e.

Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
 drivers/iommu/arm-smmu-v3.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index 257a6a3..0eca0553 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -269,9 +269,6 @@
 #define STRTAB_STE_1_SHCFG_INCOMING	1UL
 #define STRTAB_STE_1_SHCFG_SHIFT	44
 
-#define STRTAB_STE_1_PRIVCFG_UNPRIV	2UL
-#define STRTAB_STE_1_PRIVCFG_SHIFT	48
-
 #define STRTAB_STE_2_S2VMID_SHIFT	0
 #define STRTAB_STE_2_S2VMID_MASK	0xffffUL
 #define STRTAB_STE_2_VTCR_SHIFT		32
@@ -1073,9 +1070,7 @@ static void arm_smmu_write_strtab_ent(struct arm_smmu_device *smmu, u32 sid,
 #ifdef CONFIG_PCI_ATS
 			 STRTAB_STE_1_EATS_TRANS << STRTAB_STE_1_EATS_SHIFT |
 #endif
-			 STRTAB_STE_1_STRW_NSEL1 << STRTAB_STE_1_STRW_SHIFT |
-			 STRTAB_STE_1_PRIVCFG_UNPRIV <<
-			 STRTAB_STE_1_PRIVCFG_SHIFT);
+			 STRTAB_STE_1_STRW_NSEL1 << STRTAB_STE_1_STRW_SHIFT);
 
 		if (smmu->features & ARM_SMMU_FEAT_STALLS)
 			dst[1] |= cpu_to_le64(STRTAB_STE_1_S1STALLD);
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

Re: [PATCH V7 7/8] iommu/arm-smmu: Set privileged attribute to 'default' instead of 'unprivileged'

[Robin Murphy]

On 12/12/16 18:38, Sricharan R wrote:
> Currently the driver sets all the device transactions privileges
> to UNPRIVILEGED, but there are cases where the iommu masters wants
> to isolate privileged supervisor and unprivileged user.
> So don't override the privileged setting to unprivileged, instead
> set it to default as incoming and let it be controlled by the pagetable
> settings.
> 
> Acked-by: Will Deacon <will.deacon@arm.com>
> Signed-off-by: Sricharan R <sricharan@codeaurora.org>

Since everything else has already got my tags on it:

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

I'd say the whole series looks good to go now, thanks for picking it up.

Robin.

> ---
>  drivers/iommu/arm-smmu.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
> index eaa8f44..8bb0eea 100644
> --- a/drivers/iommu/arm-smmu.c
> +++ b/drivers/iommu/arm-smmu.c
> @@ -1213,7 +1213,7 @@ static int arm_smmu_domain_add_master(struct arm_smmu_domain *smmu_domain,
>  			continue;
>  
>  		s2cr[idx].type = type;
> -		s2cr[idx].privcfg = S2CR_PRIVCFG_UNPRIV;
> +		s2cr[idx].privcfg = S2CR_PRIVCFG_DEFAULT;
>  		s2cr[idx].cbndx = cbndx;
>  		arm_smmu_write_s2cr(smmu, idx);
>  	}
> 

Re: [PATCH V7 4/8] common: DMA-mapping: add DMA_ATTR_PRIVILEGED attribute

[Robin Murphy]

On 12/12/16 18:38, Sricharan R wrote:
> From: Mitchel Humpherys <mitchelh@codeaurora.org>
> 
> This patch adds the DMA_ATTR_PRIVILEGED attribute to the DMA-mapping
> subsystem.
> 
> Some advanced peripherals such as remote processors and GPUs perform
> accesses to DMA buffers in both privileged "supervisor" and unprivileged
> "user" modes.  This attribute is used to indicate to the DMA-mapping
> subsystem that the buffer is fully accessible at the elevated privilege
> level (and ideally inaccessible or at least read-only at the
> lesser-privileged levels).
> 
> Cc: linux-doc@vger.kernel.org
> Reviewed-by: Robin Murphy <robin.murphy@arm.com>
> Tested-by: Robin Murphy <robin.murphy@arm.com>
> Acked-by: Will Deacon <will.deacon@arm.com>
> Signed-off-by: Mitchel Humpherys <mitchelh@codeaurora.org>
> ---
>  Documentation/DMA-attributes.txt | 10 ++++++++++
>  include/linux/dma-mapping.h      |  7 +++++++
>  2 files changed, 17 insertions(+)
> 
> diff --git a/Documentation/DMA-attributes.txt b/Documentation/DMA-attributes.txt
> index 98bf7ac..44c6bc4 100644
> --- a/Documentation/DMA-attributes.txt
> +++ b/Documentation/DMA-attributes.txt
> @@ -143,3 +143,13 @@ So, this provides a way for drivers to avoid those error messages on calls
>  where allocation failures are not a problem, and shouldn't bother the logs.
>  
>  NOTE: At the moment DMA_ATTR_NO_WARN is only implemented on PowerPC.
> +
> +DMA_ATTR_PRIVILEGED
> +------------------------------
> +
> +Some advanced peripherals such as remote processors and GPUs perform
> +accesses to DMA buffers in both privileged "supervisor" and unprivileged
> +"user" modes.  This attribute is used to indicate to the DMA-mapping
> +subsystem that the buffer is fully accessible at the elevated privilege
> +level (and ideally inaccessible or at least read-only at the
> +lesser-privileged levels).
> diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
> index 6f3e6ca..ee31ea1 100644
> --- a/include/linux/dma-mapping.h
> +++ b/include/linux/dma-mapping.h
> @@ -63,6 +63,13 @@
>  #define DMA_ATTR_NO_WARN	(1UL << 8)
>  
>  /*
> + * DMA_ATTR_PRIVILEGED: used to indicate that the buffer is fully
> + * accessible at an elevated privilege level (and ideally inaccessible or
> + * at least read-only at lesser-privileged levels).
> + */
> +#define DMA_ATTR_PRIVILEGED		(1UL << 8)

Oops, I spoke slightly too soon - there's a value conflict here which
has been missed in the rebase.

Robin

> +
> +/*
>   * A dma_addr_t can hold any valid DMA or bus address for the platform.
>   * It can be given to a device to use as a DMA source or target.  A CPU cannot
>   * reference a dma_addr_t directly because there may be translation between
> 

Re: [PATCH V7 5/8] arm64/dma-mapping: Implement DMA_ATTR_PRIVILEGED

[Robin Murphy]

On 12/12/16 18:38, Sricharan R wrote:
> From: Mitchel Humpherys <mitchelh@codeaurora.org>
> 
> The newly added DMA_ATTR_PRIVILEGED is useful for creating mappings that
> are only accessible to privileged DMA engines.  Implement it in
> dma-iommu.c so that the ARM64 DMA IOMMU mapper can make use of it.
> 
> Reviewed-by: Robin Murphy <robin.murphy@arm.com>
> Tested-by: Robin Murphy <robin.murphy@arm.com>
> Acked-by: Will Deacon <will.deacon@arm.com>
> Signed-off-by: Mitchel Humpherys <mitchelh@codeaurora.org>
> ---
>  arch/arm64/mm/dma-mapping.c |  6 +++---
>  drivers/iommu/dma-iommu.c   | 10 ++++++++--
>  include/linux/dma-iommu.h   |  3 ++-
>  3 files changed, 13 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/arm64/mm/dma-mapping.c b/arch/arm64/mm/dma-mapping.c
> index 401f79a..ae76ead 100644
> --- a/arch/arm64/mm/dma-mapping.c
> +++ b/arch/arm64/mm/dma-mapping.c
> @@ -557,7 +557,7 @@ static void *__iommu_alloc_attrs(struct device *dev, size_t size,
>  				 unsigned long attrs)
>  {
>  	bool coherent = is_device_dma_coherent(dev);
> -	int ioprot = dma_direction_to_prot(DMA_BIDIRECTIONAL, coherent);
> +	int ioprot = dma_info_to_prot(DMA_BIDIRECTIONAL, coherent, attrs);
>  	size_t iosize = size;
>  	void *addr;
>  
> @@ -711,7 +711,7 @@ static dma_addr_t __iommu_map_page(struct device *dev, struct page *page,
>  				   unsigned long attrs)
>  {
>  	bool coherent = is_device_dma_coherent(dev);
> -	int prot = dma_direction_to_prot(dir, coherent);
> +	int prot = dma_info_to_prot(dir, coherent, attrs);
>  	dma_addr_t dev_addr = iommu_dma_map_page(dev, page, offset, size, prot);
>  
>  	if (!iommu_dma_mapping_error(dev, dev_addr) &&
> @@ -769,7 +769,7 @@ static int __iommu_map_sg_attrs(struct device *dev, struct scatterlist *sgl,
>  		__iommu_sync_sg_for_device(dev, sgl, nelems, dir);
>  
>  	return iommu_dma_map_sg(dev, sgl, nelems,
> -			dma_direction_to_prot(dir, coherent));
> +				dma_info_to_prot(dir, coherent, attrs));
>  }
>  
>  static void __iommu_unmap_sg_attrs(struct device *dev,
> diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
> index d2a7a46..756d5e0 100644
> --- a/drivers/iommu/dma-iommu.c
> +++ b/drivers/iommu/dma-iommu.c
> @@ -182,16 +182,22 @@ int iommu_dma_init_domain(struct iommu_domain *domain, dma_addr_t base,
>  EXPORT_SYMBOL(iommu_dma_init_domain);
>  
>  /**
> - * dma_direction_to_prot - Translate DMA API directions to IOMMU API page flags
> + * dma_info_to_prot - Translate DMA API directions and attributes to IOMMU API
> + *                    page flags.
>   * @dir: Direction of DMA transfer
>   * @coherent: Is the DMA master cache-coherent?
> + * @attrs: DMA attributes for the mapping
>   *
>   * Return: corresponding IOMMU API page protection flags
>   */
> -int dma_direction_to_prot(enum dma_data_direction dir, bool coherent)
> +int dma_info_to_prot(enum dma_data_direction dir, bool coherent,
> +		     unsigned long attrs)
>  {
>  	int prot = coherent ? IOMMU_CACHE : 0;
>  
> +	if (attrs & DMA_ATTR_PRIVILEGED)
> +		prot |= IOMMU_PRIV;
> +
>  	switch (dir) {
>  	case DMA_BIDIRECTIONAL:
>  		return prot | IOMMU_READ | IOMMU_WRITE;

...and applying against -next now also needs this hunk:

@@ -639,7 +639,7 @@ dma_addr_t iommu_dma_map_resource(struct device
*dev, phys_addr_t phys,
 		size_t size, enum dma_data_direction dir, unsigned long attrs)
 {
 	return __iommu_dma_map(dev, phys, size,
-			dma_direction_to_prot(dir, false) | IOMMU_MMIO);
+			dma_info_to_prot(dir, false, attrs) | IOMMU_MMIO);
 }

 void iommu_dma_unmap_resource(struct device *dev, dma_addr_t handle,

With those two issues fixed up, I've given the series (applied to
next-20161213) a spin on a SMMUv3/PL330 fast model and it still checks out.

Robin.

> diff --git a/include/linux/dma-iommu.h b/include/linux/dma-iommu.h
> index 32c5890..a203181 100644
> --- a/include/linux/dma-iommu.h
> +++ b/include/linux/dma-iommu.h
> @@ -34,7 +34,8 @@ int iommu_dma_init_domain(struct iommu_domain *domain, dma_addr_t base,
>  		u64 size, struct device *dev);
>  
>  /* General helpers for DMA-API <-> IOMMU-API interaction */
> -int dma_direction_to_prot(enum dma_data_direction dir, bool coherent);
> +int dma_info_to_prot(enum dma_data_direction dir, bool coherent,
> +		     unsigned long attrs);
>  
>  /*
>   * These implement the bulk of the relevant DMA mapping callbacks, but require
> 

RE: [PATCH V7 5/8] arm64/dma-mapping: Implement DMA_ATTR_PRIVILEGED

[Sricharan]

Hi Robin,

>-----Original Message-----
>From: linux-arm-kernel [mailto:linux-arm-kernel-bounces@lists.infradead.org] On Behalf Of Robin Murphy
>Sent: Tuesday, December 13, 2016 7:33 PM
>To: Sricharan R <sricharan@codeaurora.org>; jcrouse@codeaurora.org; pdaly@codeaurora.org; jgebben@codeaurora.org;
>joro@8bytes.org; linux-kernel@vger.kernel.org; pratikp@codeaurora.org; iommu@lists.linux-foundation.org; tzeng@codeaurora.org;
>linux-arm-kernel@lists.infradead.org; will.deacon@arm.com; mitchelh@codeaurora.org; vinod.koul@intel.com
>Cc: dan.j.williams@intel.com
>Subject: Re: [PATCH V7 5/8] arm64/dma-mapping: Implement DMA_ATTR_PRIVILEGED
>
>On 12/12/16 18:38, Sricharan R wrote:
>> From: Mitchel Humpherys <mitchelh@codeaurora.org>
>>
>> The newly added DMA_ATTR_PRIVILEGED is useful for creating mappings that
>> are only accessible to privileged DMA engines.  Implement it in
>> dma-iommu.c so that the ARM64 DMA IOMMU mapper can make use of it.
>>
>> Reviewed-by: Robin Murphy <robin.murphy@arm.com>
>> Tested-by: Robin Murphy <robin.murphy@arm.com>
>> Acked-by: Will Deacon <will.deacon@arm.com>
>> Signed-off-by: Mitchel Humpherys <mitchelh@codeaurora.org>
>> ---
>>  arch/arm64/mm/dma-mapping.c |  6 +++---
>>  drivers/iommu/dma-iommu.c   | 10 ++++++++--
>>  include/linux/dma-iommu.h   |  3 ++-
>>  3 files changed, 13 insertions(+), 6 deletions(-)
>>
>> diff --git a/arch/arm64/mm/dma-mapping.c b/arch/arm64/mm/dma-mapping.c
>> index 401f79a..ae76ead 100644
>> --- a/arch/arm64/mm/dma-mapping.c
>> +++ b/arch/arm64/mm/dma-mapping.c
>> @@ -557,7 +557,7 @@ static void *__iommu_alloc_attrs(struct device *dev, size_t size,
>>  				 unsigned long attrs)
>>  {
>>  	bool coherent = is_device_dma_coherent(dev);
>> -	int ioprot = dma_direction_to_prot(DMA_BIDIRECTIONAL, coherent);
>> +	int ioprot = dma_info_to_prot(DMA_BIDIRECTIONAL, coherent, attrs);
>>  	size_t iosize = size;
>>  	void *addr;
>>
>> @@ -711,7 +711,7 @@ static dma_addr_t __iommu_map_page(struct device *dev, struct page *page,
>>  				   unsigned long attrs)
>>  {
>>  	bool coherent = is_device_dma_coherent(dev);
>> -	int prot = dma_direction_to_prot(dir, coherent);
>> +	int prot = dma_info_to_prot(dir, coherent, attrs);
>>  	dma_addr_t dev_addr = iommu_dma_map_page(dev, page, offset, size, prot);
>>
>>  	if (!iommu_dma_mapping_error(dev, dev_addr) &&
>> @@ -769,7 +769,7 @@ static int __iommu_map_sg_attrs(struct device *dev, struct scatterlist *sgl,
>>  		__iommu_sync_sg_for_device(dev, sgl, nelems, dir);
>>
>>  	return iommu_dma_map_sg(dev, sgl, nelems,
>> -			dma_direction_to_prot(dir, coherent));
>> +				dma_info_to_prot(dir, coherent, attrs));
>>  }
>>
>>  static void __iommu_unmap_sg_attrs(struct device *dev,
>> diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
>> index d2a7a46..756d5e0 100644
>> --- a/drivers/iommu/dma-iommu.c
>> +++ b/drivers/iommu/dma-iommu.c
>> @@ -182,16 +182,22 @@ int iommu_dma_init_domain(struct iommu_domain *domain, dma_addr_t base,
>>  EXPORT_SYMBOL(iommu_dma_init_domain);
>>
>>  /**
>> - * dma_direction_to_prot - Translate DMA API directions to IOMMU API page flags
>> + * dma_info_to_prot - Translate DMA API directions and attributes to IOMMU API
>> + *                    page flags.
>>   * @dir: Direction of DMA transfer
>>   * @coherent: Is the DMA master cache-coherent?
>> + * @attrs: DMA attributes for the mapping
>>   *
>>   * Return: corresponding IOMMU API page protection flags
>>   */
>> -int dma_direction_to_prot(enum dma_data_direction dir, bool coherent)
>> +int dma_info_to_prot(enum dma_data_direction dir, bool coherent,
>> +		     unsigned long attrs)
>>  {
>>  	int prot = coherent ? IOMMU_CACHE : 0;
>>
>> +	if (attrs & DMA_ATTR_PRIVILEGED)
>> +		prot |= IOMMU_PRIV;
>> +
>>  	switch (dir) {
>>  	case DMA_BIDIRECTIONAL:
>>  		return prot | IOMMU_READ | IOMMU_WRITE;
>
>...and applying against -next now also needs this hunk:
>
>@@ -639,7 +639,7 @@ dma_addr_t iommu_dma_map_resource(struct device
>*dev, phys_addr_t phys,
> 		size_t size, enum dma_data_direction dir, unsigned long attrs)
> {
> 	return __iommu_dma_map(dev, phys, size,
>-			dma_direction_to_prot(dir, false) | IOMMU_MMIO);
>+			dma_info_to_prot(dir, false, attrs) | IOMMU_MMIO);
> }
>
> void iommu_dma_unmap_resource(struct device *dev, dma_addr_t handle,
>
>With those two issues fixed up, I've given the series (applied to
>next-20161213) a spin on a SMMUv3/PL330 fast model and it still checks out.
>

oops, sorry that i missed this in rebase. I can repost now with this fixed,
'checks out' you mean something is not working correct ?

Regards,
 Sricharan  

Re: [PATCH V7 5/8] arm64/dma-mapping: Implement DMA_ATTR_PRIVILEGED

[Robin Murphy]

On 13/12/16 14:38, Sricharan wrote:
> Hi Robin,
> 
>> -----Original Message-----
>> From: linux-arm-kernel [mailto:linux-arm-kernel-bounces@lists.infradead.org] On Behalf Of Robin Murphy
>> Sent: Tuesday, December 13, 2016 7:33 PM
>> To: Sricharan R <sricharan@codeaurora.org>; jcrouse@codeaurora.org; pdaly@codeaurora.org; jgebben@codeaurora.org;
>> joro@8bytes.org; linux-kernel@vger.kernel.org; pratikp@codeaurora.org; iommu@lists.linux-foundation.org; tzeng@codeaurora.org;
>> linux-arm-kernel@lists.infradead.org; will.deacon@arm.com; mitchelh@codeaurora.org; vinod.koul@intel.com
>> Cc: dan.j.williams@intel.com
>> Subject: Re: [PATCH V7 5/8] arm64/dma-mapping: Implement DMA_ATTR_PRIVILEGED
>>
>> On 12/12/16 18:38, Sricharan R wrote:
>>> From: Mitchel Humpherys <mitchelh@codeaurora.org>
>>>
>>> The newly added DMA_ATTR_PRIVILEGED is useful for creating mappings that
>>> are only accessible to privileged DMA engines.  Implement it in
>>> dma-iommu.c so that the ARM64 DMA IOMMU mapper can make use of it.
>>>
>>> Reviewed-by: Robin Murphy <robin.murphy@arm.com>
>>> Tested-by: Robin Murphy <robin.murphy@arm.com>
>>> Acked-by: Will Deacon <will.deacon@arm.com>
>>> Signed-off-by: Mitchel Humpherys <mitchelh@codeaurora.org>
>>> ---
>>>  arch/arm64/mm/dma-mapping.c |  6 +++---
>>>  drivers/iommu/dma-iommu.c   | 10 ++++++++--
>>>  include/linux/dma-iommu.h   |  3 ++-
>>>  3 files changed, 13 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/arch/arm64/mm/dma-mapping.c b/arch/arm64/mm/dma-mapping.c
>>> index 401f79a..ae76ead 100644
>>> --- a/arch/arm64/mm/dma-mapping.c
>>> +++ b/arch/arm64/mm/dma-mapping.c
>>> @@ -557,7 +557,7 @@ static void *__iommu_alloc_attrs(struct device *dev, size_t size,
>>>  				 unsigned long attrs)
>>>  {
>>>  	bool coherent = is_device_dma_coherent(dev);
>>> -	int ioprot = dma_direction_to_prot(DMA_BIDIRECTIONAL, coherent);
>>> +	int ioprot = dma_info_to_prot(DMA_BIDIRECTIONAL, coherent, attrs);
>>>  	size_t iosize = size;
>>>  	void *addr;
>>>
>>> @@ -711,7 +711,7 @@ static dma_addr_t __iommu_map_page(struct device *dev, struct page *page,
>>>  				   unsigned long attrs)
>>>  {
>>>  	bool coherent = is_device_dma_coherent(dev);
>>> -	int prot = dma_direction_to_prot(dir, coherent);
>>> +	int prot = dma_info_to_prot(dir, coherent, attrs);
>>>  	dma_addr_t dev_addr = iommu_dma_map_page(dev, page, offset, size, prot);
>>>
>>>  	if (!iommu_dma_mapping_error(dev, dev_addr) &&
>>> @@ -769,7 +769,7 @@ static int __iommu_map_sg_attrs(struct device *dev, struct scatterlist *sgl,
>>>  		__iommu_sync_sg_for_device(dev, sgl, nelems, dir);
>>>
>>>  	return iommu_dma_map_sg(dev, sgl, nelems,
>>> -			dma_direction_to_prot(dir, coherent));
>>> +				dma_info_to_prot(dir, coherent, attrs));
>>>  }
>>>
>>>  static void __iommu_unmap_sg_attrs(struct device *dev,
>>> diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
>>> index d2a7a46..756d5e0 100644
>>> --- a/drivers/iommu/dma-iommu.c
>>> +++ b/drivers/iommu/dma-iommu.c
>>> @@ -182,16 +182,22 @@ int iommu_dma_init_domain(struct iommu_domain *domain, dma_addr_t base,
>>>  EXPORT_SYMBOL(iommu_dma_init_domain);
>>>
>>>  /**
>>> - * dma_direction_to_prot - Translate DMA API directions to IOMMU API page flags
>>> + * dma_info_to_prot - Translate DMA API directions and attributes to IOMMU API
>>> + *                    page flags.
>>>   * @dir: Direction of DMA transfer
>>>   * @coherent: Is the DMA master cache-coherent?
>>> + * @attrs: DMA attributes for the mapping
>>>   *
>>>   * Return: corresponding IOMMU API page protection flags
>>>   */
>>> -int dma_direction_to_prot(enum dma_data_direction dir, bool coherent)
>>> +int dma_info_to_prot(enum dma_data_direction dir, bool coherent,
>>> +		     unsigned long attrs)
>>>  {
>>>  	int prot = coherent ? IOMMU_CACHE : 0;
>>>
>>> +	if (attrs & DMA_ATTR_PRIVILEGED)
>>> +		prot |= IOMMU_PRIV;
>>> +
>>>  	switch (dir) {
>>>  	case DMA_BIDIRECTIONAL:
>>>  		return prot | IOMMU_READ | IOMMU_WRITE;
>>
>> ...and applying against -next now also needs this hunk:
>>
>> @@ -639,7 +639,7 @@ dma_addr_t iommu_dma_map_resource(struct device
>> *dev, phys_addr_t phys,
>> 		size_t size, enum dma_data_direction dir, unsigned long attrs)
>> {
>> 	return __iommu_dma_map(dev, phys, size,
>> -			dma_direction_to_prot(dir, false) | IOMMU_MMIO);
>> +			dma_info_to_prot(dir, false, attrs) | IOMMU_MMIO);
>> }
>>
>> void iommu_dma_unmap_resource(struct device *dev, dma_addr_t handle,
>>
>> With those two issues fixed up, I've given the series (applied to
>> next-20161213) a spin on a SMMUv3/PL330 fast model and it still checks out.
>>
> 
> oops, sorry that i missed this in rebase. I can repost now with this fixed,
> 'checks out' you mean something is not working correct ?

No, I mean it _is_ still correct - I guess that's more of an idiom than
I thought :)

Robin.

> 
> Regards,
>  Sricharan  
> 

RE: [PATCH V7 5/8] arm64/dma-mapping: Implement DMA_ATTR_PRIVILEGED

[Sricharan]

Hi,

>
>On 13/12/16 14:38, Sricharan wrote:
>> Hi Robin,
>>
>>> -----Original Message-----
>>> From: linux-arm-kernel [mailto:linux-arm-kernel-bounces@lists.infradead.org] On Behalf Of Robin Murphy
>>> Sent: Tuesday, December 13, 2016 7:33 PM
>>> To: Sricharan R <sricharan@codeaurora.org>; jcrouse@codeaurora.org; pdaly@codeaurora.org; jgebben@codeaurora.org;
>>> joro@8bytes.org; linux-kernel@vger.kernel.org; pratikp@codeaurora.org; iommu@lists.linux-foundation.org;
>tzeng@codeaurora.org;
>>> linux-arm-kernel@lists.infradead.org; will.deacon@arm.com; mitchelh@codeaurora.org; vinod.koul@intel.com
>>> Cc: dan.j.williams@intel.com
>>> Subject: Re: [PATCH V7 5/8] arm64/dma-mapping: Implement DMA_ATTR_PRIVILEGED
>>>
>>> On 12/12/16 18:38, Sricharan R wrote:
>>>> From: Mitchel Humpherys <mitchelh@codeaurora.org>
>>>>
>>>> The newly added DMA_ATTR_PRIVILEGED is useful for creating mappings that
>>>> are only accessible to privileged DMA engines.  Implement it in
>>>> dma-iommu.c so that the ARM64 DMA IOMMU mapper can make use of it.
>>>>
>>>> Reviewed-by: Robin Murphy <robin.murphy@arm.com>
>>>> Tested-by: Robin Murphy <robin.murphy@arm.com>
>>>> Acked-by: Will Deacon <will.deacon@arm.com>
>>>> Signed-off-by: Mitchel Humpherys <mitchelh@codeaurora.org>
>>>> ---
>>>>  arch/arm64/mm/dma-mapping.c |  6 +++---
>>>>  drivers/iommu/dma-iommu.c   | 10 ++++++++--
>>>>  include/linux/dma-iommu.h   |  3 ++-
>>>>  3 files changed, 13 insertions(+), 6 deletions(-)
>>>>
>>>> diff --git a/arch/arm64/mm/dma-mapping.c b/arch/arm64/mm/dma-mapping.c
>>>> index 401f79a..ae76ead 100644
>>>> --- a/arch/arm64/mm/dma-mapping.c
>>>> +++ b/arch/arm64/mm/dma-mapping.c
>>>> @@ -557,7 +557,7 @@ static void *__iommu_alloc_attrs(struct device *dev, size_t size,
>>>>  				 unsigned long attrs)
>>>>  {
>>>>  	bool coherent = is_device_dma_coherent(dev);
>>>> -	int ioprot = dma_direction_to_prot(DMA_BIDIRECTIONAL, coherent);
>>>> +	int ioprot = dma_info_to_prot(DMA_BIDIRECTIONAL, coherent, attrs);
>>>>  	size_t iosize = size;
>>>>  	void *addr;
>>>>
>>>> @@ -711,7 +711,7 @@ static dma_addr_t __iommu_map_page(struct device *dev, struct page *page,
>>>>  				   unsigned long attrs)
>>>>  {
>>>>  	bool coherent = is_device_dma_coherent(dev);
>>>> -	int prot = dma_direction_to_prot(dir, coherent);
>>>> +	int prot = dma_info_to_prot(dir, coherent, attrs);
>>>>  	dma_addr_t dev_addr = iommu_dma_map_page(dev, page, offset, size, prot);
>>>>
>>>>  	if (!iommu_dma_mapping_error(dev, dev_addr) &&
>>>> @@ -769,7 +769,7 @@ static int __iommu_map_sg_attrs(struct device *dev, struct scatterlist *sgl,
>>>>  		__iommu_sync_sg_for_device(dev, sgl, nelems, dir);
>>>>
>>>>  	return iommu_dma_map_sg(dev, sgl, nelems,
>>>> -			dma_direction_to_prot(dir, coherent));
>>>> +				dma_info_to_prot(dir, coherent, attrs));
>>>>  }
>>>>
>>>>  static void __iommu_unmap_sg_attrs(struct device *dev,
>>>> diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
>>>> index d2a7a46..756d5e0 100644
>>>> --- a/drivers/iommu/dma-iommu.c
>>>> +++ b/drivers/iommu/dma-iommu.c
>>>> @@ -182,16 +182,22 @@ int iommu_dma_init_domain(struct iommu_domain *domain, dma_addr_t base,
>>>>  EXPORT_SYMBOL(iommu_dma_init_domain);
>>>>
>>>>  /**
>>>> - * dma_direction_to_prot - Translate DMA API directions to IOMMU API page flags
>>>> + * dma_info_to_prot - Translate DMA API directions and attributes to IOMMU API
>>>> + *                    page flags.
>>>>   * @dir: Direction of DMA transfer
>>>>   * @coherent: Is the DMA master cache-coherent?
>>>> + * @attrs: DMA attributes for the mapping
>>>>   *
>>>>   * Return: corresponding IOMMU API page protection flags
>>>>   */
>>>> -int dma_direction_to_prot(enum dma_data_direction dir, bool coherent)
>>>> +int dma_info_to_prot(enum dma_data_direction dir, bool coherent,
>>>> +		     unsigned long attrs)
>>>>  {
>>>>  	int prot = coherent ? IOMMU_CACHE : 0;
>>>>
>>>> +	if (attrs & DMA_ATTR_PRIVILEGED)
>>>> +		prot |= IOMMU_PRIV;
>>>> +
>>>>  	switch (dir) {
>>>>  	case DMA_BIDIRECTIONAL:
>>>>  		return prot | IOMMU_READ | IOMMU_WRITE;
>>>
>>> ...and applying against -next now also needs this hunk:
>>>
>>> @@ -639,7 +639,7 @@ dma_addr_t iommu_dma_map_resource(struct device
>>> *dev, phys_addr_t phys,
>>> 		size_t size, enum dma_data_direction dir, unsigned long attrs)
>>> {
>>> 	return __iommu_dma_map(dev, phys, size,
>>> -			dma_direction_to_prot(dir, false) | IOMMU_MMIO);
>>> +			dma_info_to_prot(dir, false, attrs) | IOMMU_MMIO);
>>> }
>>>
>>> void iommu_dma_unmap_resource(struct device *dev, dma_addr_t handle,
>>>
>>> With those two issues fixed up, I've given the series (applied to
>>> next-20161213) a spin on a SMMUv3/PL330 fast model and it still checks out.
>>>
>>
>> oops, sorry that i missed this in rebase. I can repost now with this fixed,
>> 'checks out' you mean something is not working correct ?
>
>No, I mean it _is_ still correct - I guess that's more of an idiom than
>I thought :)
>

ha ok, thanks for the testing as well. I will just send a v8 with those two fixed now.

Regards,
 Sricharan

RE: [PATCH V7 5/8] arm64/dma-mapping: Implement DMA_ATTR_PRIVILEGED

[Sricharan]

Hi Robin,

<snip..>

>>>>>  		return prot | IOMMU_READ | IOMMU_WRITE;
>>>>
>>>> ...and applying against -next now also needs this hunk:
>>>>
>>>> @@ -639,7 +639,7 @@ dma_addr_t iommu_dma_map_resource(struct device
>>>> *dev, phys_addr_t phys,
>>>> 		size_t size, enum dma_data_direction dir, unsigned long attrs)
>>>> {
>>>> 	return __iommu_dma_map(dev, phys, size,
>>>> -			dma_direction_to_prot(dir, false) | IOMMU_MMIO);
>>>> +			dma_info_to_prot(dir, false, attrs) | IOMMU_MMIO);
>>>> }
>>>>
>>>> void iommu_dma_unmap_resource(struct device *dev, dma_addr_t handle,
>>>>
>>>> With those two issues fixed up, I've given the series (applied to
>>>> next-20161213) a spin on a SMMUv3/PL330 fast model and it still checks out.
>>>>
>>>
>>> oops, sorry that i missed this in rebase. I can repost now with this fixed,
>>> 'checks out' you mean something is not working correct ?
>>
>>No, I mean it _is_ still correct - I guess that's more of an idiom than
>>I thought :)
>>
>
>ha ok, thanks for the testing as well. I will just send a v8 with those two fixed now.

Just while checking that i have not missed anything else, realized that the
dma-mapping apis in arm as to be modified to pass the PRIVILIGED attributes
as well. While my testing path was using the iommu_map directly i was not
seeing this, but then i did a patch like below. I will just figure out another
other codebase where the master uses the dma apis, test and add it in the
V8 that i would send.


From: Sricharan R <sricharan@codeaurora.org>
Date: Tue, 13 Dec 2016 23:25:01 +0530
Subject: [PATCH V8 6/9] arm/dma-mapping: Implement DMA_ATTR_PRIVILEGED

The newly added DMA_ATTR_PRIVILEGED is useful for creating mappings that
are only accessible to privileged DMA engines.  Implementing it in dma-mapping
for it to get used from the dma mappings apis.

Signed-off-by: Sricharan R <sricharan@codeaurora.org>
---
 arch/arm/mm/dma-mapping.c | 24 +++++++++++++++---------
 1 file changed, 15 insertions(+), 9 deletions(-)

diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
index ab77100..e0d9923 100644
--- a/arch/arm/mm/dma-mapping.c
+++ b/arch/arm/mm/dma-mapping.c
@@ -1394,7 +1394,8 @@ static int __iommu_free_buffer(struct device *dev, struct page **pages,
  * Create a mapping in device IO address space for specified pages
  */
 static dma_addr_t
-__iommu_create_mapping(struct device *dev, struct page **pages, size_t size)
+__iommu_create_mapping(struct device *dev, struct page **pages, size_t size,
+		       unsigned long attrs)
 {
 	struct dma_iommu_mapping *mapping = to_dma_iommu_mapping(dev);
 	unsigned int count = PAGE_ALIGN(size) >> PAGE_SHIFT;
@@ -1419,7 +1420,7 @@ static int __iommu_free_buffer(struct device *dev, struct page **pages,
 
 		len = (j - i) << PAGE_SHIFT;
 		ret = iommu_map(mapping->domain, iova, phys, len,
-				IOMMU_READ|IOMMU_WRITE);
+				__dma_info_to_prot(DMA_BIRECTIONAL, attrs));
 		if (ret < 0)
 			goto fail;
 		iova += len;
@@ -1476,7 +1477,8 @@ static struct page **__iommu_get_pages(void *cpu_addr, unsigned long attrs)
 }
 
 static void *__iommu_alloc_simple(struct device *dev, size_t size, gfp_t gfp,
-				  dma_addr_t *handle, int coherent_flag)
+				  dma_addr_t *handle, int coherent_flag,
+				  unsigned long attrs)
 {
 	struct page *page;
 	void *addr;
@@ -1488,7 +1490,7 @@ static void *__iommu_alloc_simple(struct device *dev, size_t size, gfp_t gfp,
 	if (!addr)
 		return NULL;
 
-	*handle = __iommu_create_mapping(dev, &page, size);
+	*handle = __iommu_create_mapping(dev, &page, size, attrs);
 	if (*handle == DMA_ERROR_CODE)
 		goto err_mapping;
 
@@ -1522,7 +1524,8 @@ static void *__arm_iommu_alloc_attrs(struct device *dev, size_t size,
 
 	if (coherent_flag  == COHERENT || !gfpflags_allow_blocking(gfp))
 		return __iommu_alloc_simple(dev, size, gfp, handle,
-					    coherent_flag);
+					    coherent_flag,
+					    attrs);
 
 	/*
 	 * Following is a work-around (a.k.a. hack) to prevent pages
@@ -1672,10 +1675,13 @@ static int arm_iommu_get_sgtable(struct device *dev, struct sg_table *sgt,
 					 GFP_KERNEL);
 }
 
-static int __dma_direction_to_prot(enum dma_data_direction dir)
+static int __dma_info_to_prot(enum dma_data_direction dir, unsigned long attrs)
 {
 	int prot;
 
+	if (attrs & DMA_ATTR_PRIVILEGED)
+		prot |= IOMMU_PRIV;
+
 	switch (dir) {
 	case DMA_BIDIRECTIONAL:
 		prot = IOMMU_READ | IOMMU_WRITE;
@@ -1722,7 +1728,7 @@ static int __map_sg_chunk(struct device *dev, struct scatterlist *sg,
 		if (!is_coherent && (attrs & DMA_ATTR_SKIP_CPU_SYNC) == 0)
 			__dma_page_cpu_to_dev(sg_page(s), s->offset, s->length, dir);
 
-		prot = __dma_direction_to_prot(dir);
+		prot = __dma_info_to_prot(dir, attrs);
 
 		ret = iommu_map(mapping->domain, iova, phys, len, prot);
 		if (ret < 0)
@@ -1930,7 +1936,7 @@ static dma_addr_t arm_coherent_iommu_map_page(struct device *dev, struct page *p
 	if (dma_addr == DMA_ERROR_CODE)
 		return dma_addr;
 
-	prot = __dma_direction_to_prot(dir);
+	prot = __dma_info_to_prot(dir, attrs);
 
 	ret = iommu_map(mapping->domain, dma_addr, page_to_phys(page), len, prot);
 	if (ret < 0)
@@ -2036,7 +2042,7 @@ static dma_addr_t arm_iommu_map_resource(struct device *dev,
 	if (dma_addr == DMA_ERROR_CODE)
 		return dma_addr;
 
-	prot = __dma_direction_to_prot(dir) | IOMMU_MMIO;
+	prot = __dma_info_to_prot(dir, attrs) | IOMMU_MMIO;
 
 	ret = iommu_map(mapping->domain, dma_addr, addr, len, prot);
 	if (ret < 0)
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

Regards,
 Sricharan

Re: [PATCH V7 5/8] arm64/dma-mapping: Implement DMA_ATTR_PRIVILEGED

[Robin Murphy]

On 13/12/16 18:46, Sricharan wrote:
> Hi Robin,
> 
> <snip..>
> 
>>>>>>  		return prot | IOMMU_READ | IOMMU_WRITE;
>>>>>
>>>>> ...and applying against -next now also needs this hunk:
>>>>>
>>>>> @@ -639,7 +639,7 @@ dma_addr_t iommu_dma_map_resource(struct device
>>>>> *dev, phys_addr_t phys,
>>>>> 		size_t size, enum dma_data_direction dir, unsigned long attrs)
>>>>> {
>>>>> 	return __iommu_dma_map(dev, phys, size,
>>>>> -			dma_direction_to_prot(dir, false) | IOMMU_MMIO);
>>>>> +			dma_info_to_prot(dir, false, attrs) | IOMMU_MMIO);
>>>>> }
>>>>>
>>>>> void iommu_dma_unmap_resource(struct device *dev, dma_addr_t handle,
>>>>>
>>>>> With those two issues fixed up, I've given the series (applied to
>>>>> next-20161213) a spin on a SMMUv3/PL330 fast model and it still checks out.
>>>>>
>>>>
>>>> oops, sorry that i missed this in rebase. I can repost now with this fixed,
>>>> 'checks out' you mean something is not working correct ?
>>>
>>> No, I mean it _is_ still correct - I guess that's more of an idiom than
>>> I thought :)
>>>
>>
>> ha ok, thanks for the testing as well. I will just send a v8 with those two fixed now.
> 
> Just while checking that i have not missed anything else, realized that the
> dma-mapping apis in arm as to be modified to pass the PRIVILIGED attributes
> as well. While my testing path was using the iommu_map directly i was not
> seeing this, but then i did a patch like below. I will just figure out another
> other codebase where the master uses the dma apis, test and add it in the
> V8 that i would send.

True, adding support to 32-bit as well can't hurt, and I guess it's
equally relevant to QC's GPU use-case. I haven't considered it myself
because AArch32 is immune to the specific PL330 problem which caught me
out - that subtle corner of VMSAv8 is unique to AArch64.

> From: Sricharan R <sricharan@codeaurora.org>
> Date: Tue, 13 Dec 2016 23:25:01 +0530
> Subject: [PATCH V8 6/9] arm/dma-mapping: Implement DMA_ATTR_PRIVILEGED
> 
> The newly added DMA_ATTR_PRIVILEGED is useful for creating mappings that
> are only accessible to privileged DMA engines.  Implementing it in dma-mapping
> for it to get used from the dma mappings apis.
> 
> Signed-off-by: Sricharan R <sricharan@codeaurora.org>
> ---
>  arch/arm/mm/dma-mapping.c | 24 +++++++++++++++---------
>  1 file changed, 15 insertions(+), 9 deletions(-)
> 
> diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
> index ab77100..e0d9923 100644
> --- a/arch/arm/mm/dma-mapping.c
> +++ b/arch/arm/mm/dma-mapping.c
> @@ -1394,7 +1394,8 @@ static int __iommu_free_buffer(struct device *dev, struct page **pages,
>   * Create a mapping in device IO address space for specified pages
>   */
>  static dma_addr_t
> -__iommu_create_mapping(struct device *dev, struct page **pages, size_t size)
> +__iommu_create_mapping(struct device *dev, struct page **pages, size_t size,
> +		       unsigned long attrs)
>  {
>  	struct dma_iommu_mapping *mapping = to_dma_iommu_mapping(dev);
>  	unsigned int count = PAGE_ALIGN(size) >> PAGE_SHIFT;
> @@ -1419,7 +1420,7 @@ static int __iommu_free_buffer(struct device *dev, struct page **pages,
>  
>  		len = (j - i) << PAGE_SHIFT;
>  		ret = iommu_map(mapping->domain, iova, phys, len,
> -				IOMMU_READ|IOMMU_WRITE);
> +				__dma_info_to_prot(DMA_BIRECTIONAL, attrs));
>  		if (ret < 0)
>  			goto fail;
>  		iova += len;
> @@ -1476,7 +1477,8 @@ static struct page **__iommu_get_pages(void *cpu_addr, unsigned long attrs)
>  }
>  
>  static void *__iommu_alloc_simple(struct device *dev, size_t size, gfp_t gfp,
> -				  dma_addr_t *handle, int coherent_flag)
> +				  dma_addr_t *handle, int coherent_flag,
> +				  unsigned long attrs)
>  {
>  	struct page *page;
>  	void *addr;
> @@ -1488,7 +1490,7 @@ static void *__iommu_alloc_simple(struct device *dev, size_t size, gfp_t gfp,
>  	if (!addr)
>  		return NULL;
>  
> -	*handle = __iommu_create_mapping(dev, &page, size);
> +	*handle = __iommu_create_mapping(dev, &page, size, attrs);
>  	if (*handle == DMA_ERROR_CODE)
>  		goto err_mapping;
>  
> @@ -1522,7 +1524,8 @@ static void *__arm_iommu_alloc_attrs(struct device *dev, size_t size,
>  
>  	if (coherent_flag  == COHERENT || !gfpflags_allow_blocking(gfp))
>  		return __iommu_alloc_simple(dev, size, gfp, handle,
> -					    coherent_flag);
> +					    coherent_flag,
> +					    attrs);

Super-nit: unnecessary line break.

>  
>  	/*
>  	 * Following is a work-around (a.k.a. hack) to prevent pages
> @@ -1672,10 +1675,13 @@ static int arm_iommu_get_sgtable(struct device *dev, struct sg_table *sgt,
>  					 GFP_KERNEL);
>  }
>  
> -static int __dma_direction_to_prot(enum dma_data_direction dir)
> +static int __dma_info_to_prot(enum dma_data_direction dir, unsigned long attrs)
>  {
>  	int prot;
>  
> +	if (attrs & DMA_ATTR_PRIVILEGED)
> +		prot |= IOMMU_PRIV;
> +
>  	switch (dir) {
>  	case DMA_BIDIRECTIONAL:
>  		prot = IOMMU_READ | IOMMU_WRITE;
> @@ -1722,7 +1728,7 @@ static int __map_sg_chunk(struct device *dev, struct scatterlist *sg,
>  		if (!is_coherent && (attrs & DMA_ATTR_SKIP_CPU_SYNC) == 0)
>  			__dma_page_cpu_to_dev(sg_page(s), s->offset, s->length, dir);
>  
> -		prot = __dma_direction_to_prot(dir);
> +		prot = __dma_info_to_prot(dir, attrs);
>  
>  		ret = iommu_map(mapping->domain, iova, phys, len, prot);
>  		if (ret < 0)
> @@ -1930,7 +1936,7 @@ static dma_addr_t arm_coherent_iommu_map_page(struct device *dev, struct page *p
>  	if (dma_addr == DMA_ERROR_CODE)
>  		return dma_addr;
>  
> -	prot = __dma_direction_to_prot(dir);
> +	prot = __dma_info_to_prot(dir, attrs);
>  
>  	ret = iommu_map(mapping->domain, dma_addr, page_to_phys(page), len, prot);
>  	if (ret < 0)
> @@ -2036,7 +2042,7 @@ static dma_addr_t arm_iommu_map_resource(struct device *dev,
>  	if (dma_addr == DMA_ERROR_CODE)
>  		return dma_addr;
>  
> -	prot = __dma_direction_to_prot(dir) | IOMMU_MMIO;
> +	prot = __dma_info_to_prot(dir, attrs) | IOMMU_MMIO;
>  
>  	ret = iommu_map(mapping->domain, dma_addr, addr, len, prot);
>  	if (ret < 0)
> 

Looks reasonable to me. Assuming it survives testing:

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

RE: [PATCH V7 5/8] arm64/dma-mapping: Implement DMA_ATTR_PRIVILEGED

[Sricharan]

Hi Robin,

>>
>> <snip..>
>>
>>>>>>>  		return prot | IOMMU_READ | IOMMU_WRITE;
>>>>>>
>>>>>> ...and applying against -next now also needs this hunk:
>>>>>>
>>>>>> @@ -639,7 +639,7 @@ dma_addr_t iommu_dma_map_resource(struct device
>>>>>> *dev, phys_addr_t phys,
>>>>>> 		size_t size, enum dma_data_direction dir, unsigned long attrs)
>>>>>> {
>>>>>> 	return __iommu_dma_map(dev, phys, size,
>>>>>> -			dma_direction_to_prot(dir, false) | IOMMU_MMIO);
>>>>>> +			dma_info_to_prot(dir, false, attrs) | IOMMU_MMIO);
>>>>>> }
>>>>>>
>>>>>> void iommu_dma_unmap_resource(struct device *dev, dma_addr_t handle,
>>>>>>
>>>>>> With those two issues fixed up, I've given the series (applied to
>>>>>> next-20161213) a spin on a SMMUv3/PL330 fast model and it still checks out.
>>>>>>
>>>>>
>>>>> oops, sorry that i missed this in rebase. I can repost now with this fixed,
>>>>> 'checks out' you mean something is not working correct ?
>>>>
>>>> No, I mean it _is_ still correct - I guess that's more of an idiom than
>>>> I thought :)
>>>>
>>>
>>> ha ok, thanks for the testing as well. I will just send a v8 with those two fixed now.
>>
>> Just while checking that i have not missed anything else, realized that the
>> dma-mapping apis in arm as to be modified to pass the PRIVILIGED attributes
>> as well. While my testing path was using the iommu_map directly i was not
>> seeing this, but then i did a patch like below. I will just figure out another
>> other codebase where the master uses the dma apis, test and add it in the
>> V8 that i would send.
>
>True, adding support to 32-bit as well can't hurt, and I guess it's
>equally relevant to QC's GPU use-case. I haven't considered it myself
>because AArch32 is immune to the specific PL330 problem which caught me
>out - that subtle corner of VMSAv8 is unique to AArch64.
>
>> From: Sricharan R <sricharan@codeaurora.org>
>> Date: Tue, 13 Dec 2016 23:25:01 +0530
>> Subject: [PATCH V8 6/9] arm/dma-mapping: Implement DMA_ATTR_PRIVILEGED
>>
>> The newly added DMA_ATTR_PRIVILEGED is useful for creating mappings that
>> are only accessible to privileged DMA engines.  Implementing it in dma-mapping
>> for it to get used from the dma mappings apis.
>>
>> Signed-off-by: Sricharan R <sricharan@codeaurora.org>
>> ---
>>  arch/arm/mm/dma-mapping.c | 24 +++++++++++++++---------
>>  1 file changed, 15 insertions(+), 9 deletions(-)
>>
>> diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
>> index ab77100..e0d9923 100644
>> --- a/arch/arm/mm/dma-mapping.c
>> +++ b/arch/arm/mm/dma-mapping.c
>> @@ -1394,7 +1394,8 @@ static int __iommu_free_buffer(struct device *dev, struct page **pages,
>>   * Create a mapping in device IO address space for specified pages
>>   */
>>  static dma_addr_t
>> -__iommu_create_mapping(struct device *dev, struct page **pages, size_t size)
>> +__iommu_create_mapping(struct device *dev, struct page **pages, size_t size,
>> +		       unsigned long attrs)
>>  {
>>  	struct dma_iommu_mapping *mapping = to_dma_iommu_mapping(dev);
>>  	unsigned int count = PAGE_ALIGN(size) >> PAGE_SHIFT;
>> @@ -1419,7 +1420,7 @@ static int __iommu_free_buffer(struct device *dev, struct page **pages,
>>
>>  		len = (j - i) << PAGE_SHIFT;
>>  		ret = iommu_map(mapping->domain, iova, phys, len,
>> -				IOMMU_READ|IOMMU_WRITE);
>> +				__dma_info_to_prot(DMA_BIRECTIONAL, attrs));
>>  		if (ret < 0)
>>  			goto fail;
>>  		iova += len;
>> @@ -1476,7 +1477,8 @@ static struct page **__iommu_get_pages(void *cpu_addr, unsigned long attrs)
>>  }
>>
>>  static void *__iommu_alloc_simple(struct device *dev, size_t size, gfp_t gfp,
>> -				  dma_addr_t *handle, int coherent_flag)
>> +				  dma_addr_t *handle, int coherent_flag,
>> +				  unsigned long attrs)
>>  {
>>  	struct page *page;
>>  	void *addr;
>> @@ -1488,7 +1490,7 @@ static void *__iommu_alloc_simple(struct device *dev, size_t size, gfp_t gfp,
>>  	if (!addr)
>>  		return NULL;
>>
>> -	*handle = __iommu_create_mapping(dev, &page, size);
>> +	*handle = __iommu_create_mapping(dev, &page, size, attrs);
>>  	if (*handle == DMA_ERROR_CODE)
>>  		goto err_mapping;
>>
>> @@ -1522,7 +1524,8 @@ static void *__arm_iommu_alloc_attrs(struct device *dev, size_t size,
>>
>>  	if (coherent_flag  == COHERENT || !gfpflags_allow_blocking(gfp))
>>  		return __iommu_alloc_simple(dev, size, gfp, handle,
>> -					    coherent_flag);
>> +					    coherent_flag,
>> +					    attrs);
>
>Super-nit: unnecessary line break.
>
>>
>>  	/*
>>  	 * Following is a work-around (a.k.a. hack) to prevent pages
>> @@ -1672,10 +1675,13 @@ static int arm_iommu_get_sgtable(struct device *dev, struct sg_table *sgt,
>>  					 GFP_KERNEL);
>>  }
>>
>> -static int __dma_direction_to_prot(enum dma_data_direction dir)
>> +static int __dma_info_to_prot(enum dma_data_direction dir, unsigned long attrs)
>>  {
>>  	int prot;
>>
>> +	if (attrs & DMA_ATTR_PRIVILEGED)
>> +		prot |= IOMMU_PRIV;
>> +
>>  	switch (dir) {
>>  	case DMA_BIDIRECTIONAL:
>>  		prot = IOMMU_READ | IOMMU_WRITE;
>> @@ -1722,7 +1728,7 @@ static int __map_sg_chunk(struct device *dev, struct scatterlist *sg,
>>  		if (!is_coherent && (attrs & DMA_ATTR_SKIP_CPU_SYNC) == 0)
>>  			__dma_page_cpu_to_dev(sg_page(s), s->offset, s->length, dir);
>>
>> -		prot = __dma_direction_to_prot(dir);
>> +		prot = __dma_info_to_prot(dir, attrs);
>>
>>  		ret = iommu_map(mapping->domain, iova, phys, len, prot);
>>  		if (ret < 0)
>> @@ -1930,7 +1936,7 @@ static dma_addr_t arm_coherent_iommu_map_page(struct device *dev, struct page *p
>>  	if (dma_addr == DMA_ERROR_CODE)
>>  		return dma_addr;
>>
>> -	prot = __dma_direction_to_prot(dir);
>> +	prot = __dma_info_to_prot(dir, attrs);
>>
>>  	ret = iommu_map(mapping->domain, dma_addr, page_to_phys(page), len, prot);
>>  	if (ret < 0)
>> @@ -2036,7 +2042,7 @@ static dma_addr_t arm_iommu_map_resource(struct device *dev,
>>  	if (dma_addr == DMA_ERROR_CODE)
>>  		return dma_addr;
>>
>> -	prot = __dma_direction_to_prot(dir) | IOMMU_MMIO;
>> +	prot = __dma_info_to_prot(dir, attrs) | IOMMU_MMIO;
>>
>>  	ret = iommu_map(mapping->domain, dma_addr, addr, len, prot);
>>  	if (ret < 0)
>>
>
>Looks reasonable to me. Assuming it survives testing:
>
>Acked-by: Robin Murphy <robin.murphy@arm.com>

Posted V8 [1]. I changed a few more things after the testing,
though functionally the same. So have not taken your ack and
would be nice to have it once again

[1] https://lkml.org/lkml/2017/1/2/224

Regards,
 Sricharan