linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 0/6] Add support for privileged mappings
@ 2016-07-19 20:36 Mitchel Humpherys
  2016-07-19 20:36 ` [PATCH v3 1/6] iommu: add IOMMU_PRIV attribute Mitchel Humpherys
                   ` (6 more replies)
  0 siblings, 7 replies; 11+ messages in thread
From: Mitchel Humpherys @ 2016-07-19 20:36 UTC (permalink / raw)
  To: linux-arm-kernel

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].

[1] https://github.com/robclark/kilroy

Changelog:

  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 (5):
  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
  Revert "iommu/arm-smmu: Treat all device transactions as unprivileged"

 Documentation/DMA-attributes.txt | 10 ++++++++++
 arch/arm64/mm/dma-mapping.c      |  6 +++---
 drivers/dma/pl330.c              |  7 +++++--
 drivers/iommu/arm-smmu.c         |  5 +----
 drivers/iommu/dma-iommu.c        | 16 +++++++++++-----
 drivers/iommu/io-pgtable-arm.c   |  5 ++++-
 include/linux/dma-attrs.h        |  1 +
 include/linux/dma-iommu.h        |  3 ++-
 include/linux/iommu.h            |  1 +
 9 files changed, 38 insertions(+), 16 deletions(-)

-- 
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v3 1/6] iommu: add IOMMU_PRIV attribute
  2016-07-19 20:36 [PATCH v3 0/6] Add support for privileged mappings Mitchel Humpherys
@ 2016-07-19 20:36 ` Mitchel Humpherys
  2016-07-19 20:36 ` [PATCH v3 2/6] iommu/io-pgtable-arm: add support for the IOMMU_PRIV flag Mitchel Humpherys
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 11+ messages in thread
From: Mitchel Humpherys @ 2016-07-19 20:36 UTC (permalink / raw)
  To: linux-arm-kernel

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

Signed-off-by: Mitchel Humpherys <mitchelh@codeaurora.org>
---

Notes:
    v2..v3
    
      - Added comment

 include/linux/iommu.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 664683aedcce..4ae46254b915 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 Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v3 2/6] iommu/io-pgtable-arm: add support for the IOMMU_PRIV flag
  2016-07-19 20:36 [PATCH v3 0/6] Add support for privileged mappings Mitchel Humpherys
  2016-07-19 20:36 ` [PATCH v3 1/6] iommu: add IOMMU_PRIV attribute Mitchel Humpherys
@ 2016-07-19 20:36 ` Mitchel Humpherys
  2016-07-19 20:36 ` [PATCH v3 3/6] common: DMA-mapping: add DMA_ATTR_PRIVILEGED attribute Mitchel Humpherys
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 11+ messages in thread
From: Mitchel Humpherys @ 2016-07-19 20:36 UTC (permalink / raw)
  To: linux-arm-kernel

From: Jeremy Gebben <jgebben@codeaurora.org>

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

Signed-off-by: Jeremy Gebben <jgebben@codeaurora.org>
---

Notes:
    v2..v3
    
      - Use existing bit definitions.

 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 a1ed1b73fed4..2a7d28ab8241 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 Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v3 3/6] common: DMA-mapping: add DMA_ATTR_PRIVILEGED attribute
  2016-07-19 20:36 [PATCH v3 0/6] Add support for privileged mappings Mitchel Humpherys
  2016-07-19 20:36 ` [PATCH v3 1/6] iommu: add IOMMU_PRIV attribute Mitchel Humpherys
  2016-07-19 20:36 ` [PATCH v3 2/6] iommu/io-pgtable-arm: add support for the IOMMU_PRIV flag Mitchel Humpherys
@ 2016-07-19 20:36 ` Mitchel Humpherys
  2016-07-19 20:36 ` [PATCH v3 4/6] arm64/dma-mapping: Implement DMA_ATTR_PRIVILEGED Mitchel Humpherys
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 11+ messages in thread
From: Mitchel Humpherys @ 2016-07-19 20:36 UTC (permalink / raw)
  To: linux-arm-kernel

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 at vger.kernel.org
Signed-off-by: Mitchel Humpherys <mitchelh@codeaurora.org>
---

Notes:
    v2..v3
    
      - Not worrying about executability.

 Documentation/DMA-attributes.txt | 10 ++++++++++
 include/linux/dma-attrs.h        |  1 +
 2 files changed, 11 insertions(+)

diff --git a/Documentation/DMA-attributes.txt b/Documentation/DMA-attributes.txt
index e8cf9cf873b3..d985effd0053 100644
--- a/Documentation/DMA-attributes.txt
+++ b/Documentation/DMA-attributes.txt
@@ -126,3 +126,13 @@ means that we won't try quite as hard to get them.
 
 NOTE: At the moment DMA_ATTR_ALLOC_SINGLE_PAGES is only implemented on ARM,
 though ARM64 patches will likely be posted soon.
+
+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-attrs.h b/include/linux/dma-attrs.h
index 5246239a4953..3bc2208e1765 100644
--- a/include/linux/dma-attrs.h
+++ b/include/linux/dma-attrs.h
@@ -19,6 +19,7 @@ enum dma_attr {
 	DMA_ATTR_SKIP_CPU_SYNC,
 	DMA_ATTR_FORCE_CONTIGUOUS,
 	DMA_ATTR_ALLOC_SINGLE_PAGES,
+	DMA_ATTR_PRIVILEGED,
 	DMA_ATTR_MAX,
 };
 
-- 
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v3 4/6] arm64/dma-mapping: Implement DMA_ATTR_PRIVILEGED
  2016-07-19 20:36 [PATCH v3 0/6] Add support for privileged mappings Mitchel Humpherys
                   ` (2 preceding siblings ...)
  2016-07-19 20:36 ` [PATCH v3 3/6] common: DMA-mapping: add DMA_ATTR_PRIVILEGED attribute Mitchel Humpherys
@ 2016-07-19 20:36 ` Mitchel Humpherys
  2016-07-19 20:36 ` [PATCH v3 5/6] dmaengine: pl330: Make sure microcode is privileged Mitchel Humpherys
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 11+ messages in thread
From: Mitchel Humpherys @ 2016-07-19 20:36 UTC (permalink / raw)
  To: linux-arm-kernel

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.

Signed-off-by: Mitchel Humpherys <mitchelh@codeaurora.org>
---

Notes:
    v2..v3
    
      - Renamed and redocumented dma_direction_to_prot.
      - Dropped the stuff making all privileged mappings read-only.

 arch/arm64/mm/dma-mapping.c |  6 +++---
 drivers/iommu/dma-iommu.c   | 16 +++++++++++-----
 include/linux/dma-iommu.h   |  3 ++-
 3 files changed, 16 insertions(+), 9 deletions(-)

diff --git a/arch/arm64/mm/dma-mapping.c b/arch/arm64/mm/dma-mapping.c
index c566ec83719f..5da8946ead9e 100644
--- a/arch/arm64/mm/dma-mapping.c
+++ b/arch/arm64/mm/dma-mapping.c
@@ -543,7 +543,7 @@ static void *__iommu_alloc_attrs(struct device *dev, size_t size,
 				 struct dma_attrs *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;
 
@@ -697,7 +697,7 @@ static dma_addr_t __iommu_map_page(struct device *dev, struct page *page,
 				   struct dma_attrs *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) &&
@@ -755,7 +755,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 ea5a9ebf0f78..cc2c9cccde1f 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -129,26 +129,32 @@ int iommu_dma_init_domain(struct iommu_domain *domain, dma_addr_t base, u64 size
 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,
+		     struct dma_attrs *attrs)
 {
 	int prot = coherent ? IOMMU_CACHE : 0;
 
 	switch (dir) {
 	case DMA_BIDIRECTIONAL:
-		return prot | IOMMU_READ | IOMMU_WRITE;
+		prot |= IOMMU_READ | IOMMU_WRITE;
 	case DMA_TO_DEVICE:
-		return prot | IOMMU_READ;
+		prot |= IOMMU_READ;
 	case DMA_FROM_DEVICE:
-		return prot | IOMMU_WRITE;
+		prot |= IOMMU_WRITE;
 	default:
 		return 0;
 	}
+	if (dma_get_attr(DMA_ATTR_PRIVILEGED, attrs))
+		prot |= IOMMU_PRIV;
+	return prot;
 }
 
 static struct iova *__alloc_iova(struct iova_domain *iovad, size_t size,
diff --git a/include/linux/dma-iommu.h b/include/linux/dma-iommu.h
index 8443bbb5c071..b0dbcff2d73e 100644
--- a/include/linux/dma-iommu.h
+++ b/include/linux/dma-iommu.h
@@ -32,7 +32,8 @@ void iommu_put_dma_cookie(struct iommu_domain *domain);
 int iommu_dma_init_domain(struct iommu_domain *domain, dma_addr_t base, u64 size);
 
 /* 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,
+		     struct dma_attrs *attrs);
 
 /*
  * These implement the bulk of the relevant DMA mapping callbacks, but require
-- 
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v3 5/6] dmaengine: pl330: Make sure microcode is privileged
  2016-07-19 20:36 [PATCH v3 0/6] Add support for privileged mappings Mitchel Humpherys
                   ` (3 preceding siblings ...)
  2016-07-19 20:36 ` [PATCH v3 4/6] arm64/dma-mapping: Implement DMA_ATTR_PRIVILEGED Mitchel Humpherys
@ 2016-07-19 20:36 ` Mitchel Humpherys
  2016-07-19 20:36 ` [PATCH v3 6/6] Revert "iommu/arm-smmu: Treat all device transactions as unprivileged" Mitchel Humpherys
  2016-07-22 16:51 ` [PATCH v3 0/6] Add support for privileged mappings Will Deacon
  6 siblings, 0 replies; 11+ messages in thread
From: Mitchel Humpherys @ 2016-07-19 20:36 UTC (permalink / raw)
  To: linux-arm-kernel

The PL330 performs privileged instruction fetches.  This can result in
SMMU permission faults on SMMUs that implement the ARMv8 VMSA, which
specifies that mappings that are writeable at one execution level shall
not be executable at any higher-privileged level.  Fix this by using the
DMA_ATTR_PRIVILEGED attribute, which will ensure that the microcode
IOMMU mapping is only accessible to the privileged level.

Cc: Dan Williams <dan.j.williams@intel.com>
Cc: Jassi Brar <jassi.brar@samsung.com>
Signed-off-by: Mitchel Humpherys <mitchelh@codeaurora.org>
---
 drivers/dma/pl330.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/dma/pl330.c b/drivers/dma/pl330.c
index 372b4359da97..7297cd1d03c8 100644
--- a/drivers/dma/pl330.c
+++ b/drivers/dma/pl330.c
@@ -1854,14 +1854,17 @@ static int dmac_alloc_resources(struct pl330_dmac *pl330)
 {
 	int chans = pl330->pcfg.num_chan;
 	int ret;
+	DEFINE_DMA_ATTRS(attrs);
 
+	dma_set_attr(DMA_ATTR_PRIVILEGED, &attrs);
 	/*
 	 * 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,
+				&attrs);
 	if (!pl330->mcode_cpu) {
 		dev_err(pl330->ddma.dev, "%s:%d Can't allocate memory!\n",
 			__func__, __LINE__);
-- 
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v3 6/6] Revert "iommu/arm-smmu: Treat all device transactions as unprivileged"
  2016-07-19 20:36 [PATCH v3 0/6] Add support for privileged mappings Mitchel Humpherys
                   ` (4 preceding siblings ...)
  2016-07-19 20:36 ` [PATCH v3 5/6] dmaengine: pl330: Make sure microcode is privileged Mitchel Humpherys
@ 2016-07-19 20:36 ` Mitchel Humpherys
  2016-07-22 16:51 ` [PATCH v3 0/6] Add support for privileged mappings Will Deacon
  6 siblings, 0 replies; 11+ messages in thread
From: Mitchel Humpherys @ 2016-07-19 20:36 UTC (permalink / raw)
  To: linux-arm-kernel

This reverts commit d346180e70b9 ("iommu/arm-smmu: Treat all device
transactions as unprivileged") since some platforms actually make use of
privileged transactions.

Signed-off-by: Mitchel Humpherys <mitchelh@codeaurora.org>
---

Notes:
    v2..v3
    
      - Moved to the end of the series.

 drivers/iommu/arm-smmu.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index 9345a3fcb706..d0627ef26b05 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -178,9 +178,6 @@
 #define S2CR_TYPE_BYPASS		(1 << S2CR_TYPE_SHIFT)
 #define S2CR_TYPE_FAULT			(2 << S2CR_TYPE_SHIFT)
 
-#define S2CR_PRIVCFG_SHIFT		24
-#define S2CR_PRIVCFG_UNPRIV		(2 << S2CR_PRIVCFG_SHIFT)
-
 /* Context bank attribute registers */
 #define ARM_SMMU_GR1_CBAR(n)		(0x0 + ((n) << 2))
 #define CBAR_VMID_SHIFT			0
@@ -1175,7 +1172,7 @@ static int arm_smmu_domain_add_master(struct arm_smmu_domain *smmu_domain,
 		u32 idx, s2cr;
 
 		idx = cfg->smrs ? cfg->smrs[i].idx : cfg->streamids[i];
-		s2cr = S2CR_TYPE_TRANS | S2CR_PRIVCFG_UNPRIV |
+		s2cr = S2CR_TYPE_TRANS |
 		       (smmu_domain->cfg.cbndx << S2CR_CBNDX_SHIFT);
 		writel_relaxed(s2cr, gr0_base + ARM_SMMU_GR0_S2CR(idx));
 	}
-- 
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v3 0/6] Add support for privileged mappings
  2016-07-19 20:36 [PATCH v3 0/6] Add support for privileged mappings Mitchel Humpherys
                   ` (5 preceding siblings ...)
  2016-07-19 20:36 ` [PATCH v3 6/6] Revert "iommu/arm-smmu: Treat all device transactions as unprivileged" Mitchel Humpherys
@ 2016-07-22 16:51 ` Will Deacon
  2016-07-22 20:39   ` Mitchel Humpherys
  6 siblings, 1 reply; 11+ messages in thread
From: Will Deacon @ 2016-07-22 16:51 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Jul 19, 2016 at 01:36:49PM -0700, Mitchel Humpherys wrote:
> 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].

This all looks good to me:

Acked-by: Will Deacon <will.deacon@arm.com>

It looks pretty fiddly to merge, however. How are you planning to get
this upstream?

Will

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v3 0/6] Add support for privileged mappings
  2016-07-22 16:51 ` [PATCH v3 0/6] Add support for privileged mappings Will Deacon
@ 2016-07-22 20:39   ` Mitchel Humpherys
  2016-07-25  9:50     ` Will Deacon
  0 siblings, 1 reply; 11+ messages in thread
From: Mitchel Humpherys @ 2016-07-22 20:39 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, Jul 22 2016 at 05:51:07 PM, Will Deacon <will.deacon@arm.com> wrote:
> On Tue, Jul 19, 2016 at 01:36:49PM -0700, Mitchel Humpherys wrote:
>> 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].
>
> This all looks good to me:
>
> Acked-by: Will Deacon <will.deacon@arm.com>
>
> It looks pretty fiddly to merge, however. How are you planning to get
> this upstream?

Fiddly in what way?  Do you mean in relation to "dma-mapping: Use
unsigned long for dma_attrs" [1]?  I admit I wasn't aware of that
activity until Robin mentioned it.  It looks like it's merged on
next/master, shall I rebase/rework on that and resend?

[1] https://lkml.org/lkml/2016/7/13/198


-Mitch

-- 
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v3 0/6] Add support for privileged mappings
  2016-07-22 20:39   ` Mitchel Humpherys
@ 2016-07-25  9:50     ` Will Deacon
  2016-07-25 19:01       ` Mitchel Humpherys
  0 siblings, 1 reply; 11+ messages in thread
From: Will Deacon @ 2016-07-25  9:50 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, Jul 22, 2016 at 01:39:45PM -0700, Mitchel Humpherys wrote:
> On Fri, Jul 22 2016 at 05:51:07 PM, Will Deacon <will.deacon@arm.com> wrote:
> > On Tue, Jul 19, 2016 at 01:36:49PM -0700, Mitchel Humpherys wrote:
> >> 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].
> >
> > This all looks good to me:
> >
> > Acked-by: Will Deacon <will.deacon@arm.com>
> >
> > It looks pretty fiddly to merge, however. How are you planning to get
> > this upstream?
> 
> Fiddly in what way?  Do you mean in relation to "dma-mapping: Use
> unsigned long for dma_attrs" [1]?  I admit I wasn't aware of that
> activity until Robin mentioned it.  It looks like it's merged on
> next/master, shall I rebase/rework on that and resend?

Fiddly in that it touches multiple subsystems. I guess routing it via
the iommu tree (Joerg) might be the best bet.

Will

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v3 0/6] Add support for privileged mappings
  2016-07-25  9:50     ` Will Deacon
@ 2016-07-25 19:01       ` Mitchel Humpherys
  0 siblings, 0 replies; 11+ messages in thread
From: Mitchel Humpherys @ 2016-07-25 19:01 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Jul 25 2016 at 10:50:13 AM, Will Deacon <will.deacon@arm.com> wrote:
> On Fri, Jul 22, 2016 at 01:39:45PM -0700, Mitchel Humpherys wrote:
>> On Fri, Jul 22 2016 at 05:51:07 PM, Will Deacon <will.deacon@arm.com> wrote:
>> > On Tue, Jul 19, 2016 at 01:36:49PM -0700, Mitchel Humpherys wrote:
>> >> 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].
>> >
>> > This all looks good to me:
>> >
>> > Acked-by: Will Deacon <will.deacon@arm.com>
>> >
>> > It looks pretty fiddly to merge, however. How are you planning to get
>> > this upstream?
>> 
>> Fiddly in what way?  Do you mean in relation to "dma-mapping: Use
>> unsigned long for dma_attrs" [1]?  I admit I wasn't aware of that
>> activity until Robin mentioned it.  It looks like it's merged on
>> next/master, shall I rebase/rework on that and resend?
>
> Fiddly in that it touches multiple subsystems. I guess routing it via
> the iommu tree (Joerg) might be the best bet.

Sounds good.  I'm going to rebase on linux-next as well anyways to get
the new dma attrs format and resend.


-Mitch

-- 
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2016-07-25 19:01 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-07-19 20:36 [PATCH v3 0/6] Add support for privileged mappings Mitchel Humpherys
2016-07-19 20:36 ` [PATCH v3 1/6] iommu: add IOMMU_PRIV attribute Mitchel Humpherys
2016-07-19 20:36 ` [PATCH v3 2/6] iommu/io-pgtable-arm: add support for the IOMMU_PRIV flag Mitchel Humpherys
2016-07-19 20:36 ` [PATCH v3 3/6] common: DMA-mapping: add DMA_ATTR_PRIVILEGED attribute Mitchel Humpherys
2016-07-19 20:36 ` [PATCH v3 4/6] arm64/dma-mapping: Implement DMA_ATTR_PRIVILEGED Mitchel Humpherys
2016-07-19 20:36 ` [PATCH v3 5/6] dmaengine: pl330: Make sure microcode is privileged Mitchel Humpherys
2016-07-19 20:36 ` [PATCH v3 6/6] Revert "iommu/arm-smmu: Treat all device transactions as unprivileged" Mitchel Humpherys
2016-07-22 16:51 ` [PATCH v3 0/6] Add support for privileged mappings Will Deacon
2016-07-22 20:39   ` Mitchel Humpherys
2016-07-25  9:50     ` Will Deacon
2016-07-25 19:01       ` Mitchel Humpherys

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).