[RESEND PATCH V6 0/6] Add support for privileged mappings

[RESEND PATCH V6 0/6] 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. Not sure why this
finally did not make it last time. The last patch in the previous
series does not apply now [3], so just redid that. Also Copied the tags
that he had from last time as well.

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
[2] https://lkml.org/lkml/2016/7/27/590
[3] https://patchwork.kernel.org/patch/9250493/

Changelog:

 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

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              |  6 ++++--
 drivers/iommu/arm-smmu.c         |  2 +-
 drivers/iommu/dma-iommu.c        | 10 ++++++++--
 drivers/iommu/io-pgtable-arm.c   |  5 ++++-
 include/linux/dma-iommu.h        |  3 ++-
 include/linux/dma-mapping.h      |  7 +++++++
 include/linux/iommu.h            |  1 +
 9 files changed, 40 insertions(+), 10 deletions(-)

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

[RESEND PATCH V6 1/6] 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>
Acked-by: Will Deacon <will.deacon@arm.com>
Signed-off-by: Mitchel Humpherys <mitchelh@codeaurora.org>
---

[V6] No change

 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

[RESEND PATCH V6 2/6] 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>
---

[V6] No change

 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

[RESEND PATCH V6 3/6] 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>
---

[V6] No change

 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

[RESEND PATCH V6 4/6] 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>
---

[V6] No change

 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

[RESEND PATCH V6 5/6] dmaengine: pl330: Make sure microcode is privileged

[Sricharan R]

From: Mitchel Humpherys <mitchelh@codeaurora.org>

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

[V6] No change

 drivers/dma/pl330.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/dma/pl330.c b/drivers/dma/pl330.c
index 030fe05..1a8bac2 100644
--- a/drivers/dma/pl330.c
+++ b/drivers/dma/pl330.c
@@ -1854,14 +1854,16 @@ static int dmac_alloc_resources(struct pl330_dmac *pl330)
 {
 	int chans = pl330->pcfg.num_chan;
 	int ret;
+	unsigned long dma_attrs = DMA_ATTR_PRIVILEGED;
 
 	/*
 	 * 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_attrs);
 	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 V6 6/6] 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.

Signed-off-by: Sricharan R <sricharan@codeaurora.org>
---

[V6] V5 was doing this with a 'revert'[1] patch, which no more
     applies on this code base. So changed the same like this.
     [1] https://patchwork.kernel.org/patch/9250493/

 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

Re: [RESEND PATCH V6 0/6] Add support for privileged mappings

[Robin Murphy]

Hi Sricharan,

On 02/12/16 14:55, Sricharan R wrote:
> 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. Not sure why this
> finally did not make it last time. The last patch in the previous
> series does not apply now [3], so just redid that. Also Copied the tags
> that he had from last time as well.

Heh, I was assuming this would be down to me to pick up. Vinod did have
some complaints last time about the commit message on the PL330 patch -
I did get as far as rewriting that and reworking onto my SMMU
changes[1], I just hadn't got round to sending it, so it fell onto the
"after the next merge window" pile.

I'd give some review comments, but they'd essentially be a diff against
that branch :)

Robin.

[1]:http://www.linux-arm.org/git?p=linux-rm.git;a=shortlog;h=refs/heads/mh/dma-priv

> 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
> [2] https://lkml.org/lkml/2016/7/27/590
> [3] https://patchwork.kernel.org/patch/9250493/
> 
> Changelog:
> 
>  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
> 
> 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              |  6 ++++--
>  drivers/iommu/arm-smmu.c         |  2 +-
>  drivers/iommu/dma-iommu.c        | 10 ++++++++--
>  drivers/iommu/io-pgtable-arm.c   |  5 ++++-
>  include/linux/dma-iommu.h        |  3 ++-
>  include/linux/dma-mapping.h      |  7 +++++++
>  include/linux/iommu.h            |  1 +
>  9 files changed, 40 insertions(+), 10 deletions(-)
> 

RE: [RESEND PATCH V6 0/6] Add support for privileged mappings

[Sricharan]

Hi Robin,

>Hi Sricharan,
>
>On 02/12/16 14:55, Sricharan R wrote:
>> 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. Not sure why this
>> finally did not make it last time. The last patch in the previous
>> series does not apply now [3], so just redid that. Also Copied the tags
>> that he had from last time as well.
>
>Heh, I was assuming this would be down to me to pick up. Vinod did have
>some complaints last time about the commit message on the PL330 patch -
>I did get as far as rewriting that and reworking onto my SMMU
>changes[1], I just hadn't got round to sending it, so it fell onto the
>"after the next merge window" pile.
>
>I'd give some review comments, but they'd essentially be a diff against
>that branch :)
>

Sure, i did not knew that you were on this already. I can repost with the diff
from your branch taken in or wait for you as well. I am fine with either ways
that you suggest.

I checked the patches against your branch, i see that the changes are,

1) one patch for implementing it for armv7s descriptor
2) Changes on pl330 patch commit logs and
3) One patch for doing the revert on arm-smmuv3 as well.

Regards,
 Sricharan

Re: [RESEND PATCH V6 5/6] dmaengine: pl330: Make sure microcode is privileged

[Vinod Koul]

On Fri, Dec 02, 2016 at 08:25:08PM +0530, Sricharan R wrote:
> From: Mitchel Humpherys <mitchelh@codeaurora.org>
> 
> 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.

Acked-by: Vinod Koul <vinod.koul@intel.com>

-- 
~Vinod

Re: [PATCH V6 6/6] iommu/arm-smmu: Set privileged attribute to 'default' instead of 'unprivileged'

[Will Deacon]

On Fri, Dec 02, 2016 at 08:25:09PM +0530, 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.
> 
> Signed-off-by: Sricharan R <sricharan@codeaurora.org>

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

I'll let you and Robin sort out what you want to do with this series :)

Will

Re: [RESEND PATCH V6 0/6] Add support for privileged mappings

[Robin Murphy]

On 04/12/16 07:48, Sricharan wrote:
> Hi Robin,
> 
>> Hi Sricharan,
>>
>> On 02/12/16 14:55, Sricharan R wrote:
>>> 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. Not sure why this
>>> finally did not make it last time. The last patch in the previous
>>> series does not apply now [3], so just redid that. Also Copied the tags
>>> that he had from last time as well.
>>
>> Heh, I was assuming this would be down to me to pick up. Vinod did have
>> some complaints last time about the commit message on the PL330 patch -
>> I did get as far as rewriting that and reworking onto my SMMU
>> changes[1], I just hadn't got round to sending it, so it fell onto the
>> "after the next merge window" pile.
>>
>> I'd give some review comments, but they'd essentially be a diff against
>> that branch :)
>>
> 
> Sure, i did not knew that you were on this already. I can repost with the diff
> from your branch taken in or wait for you as well. I am fine with either ways
> that you suggest.
> 
> I checked the patches against your branch, i see that the changes are,
> 
> 1) one patch for implementing it for armv7s descriptor
> 2) Changes on pl330 patch commit logs and
> 3) One patch for doing the revert on arm-smmuv3 as well.

If you want to pick up my short-descriptor and SMMUv3 patches and run
with them you're more than welcome - the rest is just cosmetic stuff
which doesn't really matter, especially as it's picking up acks as-is.

Robin.

> Regards,
>  Sricharan
> 
> 

RE: [RESEND PATCH V6 0/6] Add support for privileged mappings

[Sricharan]

Hi Robin,

>>> Hi Sricharan,
>>>
>>> On 02/12/16 14:55, Sricharan R wrote:
>>>> 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. Not sure why this
>>>> finally did not make it last time. The last patch in the previous
>>>> series does not apply now [3], so just redid that. Also Copied the tags
>>>> that he had from last time as well.
>>>
>>> Heh, I was assuming this would be down to me to pick up. Vinod did have
>>> some complaints last time about the commit message on the PL330 patch -
>>> I did get as far as rewriting that and reworking onto my SMMU
>>> changes[1], I just hadn't got round to sending it, so it fell onto the
>>> "after the next merge window" pile.
>>>
>>> I'd give some review comments, but they'd essentially be a diff against
>>> that branch :)
>>>
>>
>> Sure, i did not knew that you were on this already. I can repost with the diff
>> from your branch taken in or wait for you as well. I am fine with either ways
>> that you suggest.
>>
>> I checked the patches against your branch, i see that the changes are,
>>
>> 1) one patch for implementing it for armv7s descriptor
>> 2) Changes on pl330 patch commit logs and
>> 3) One patch for doing the revert on arm-smmuv3 as well.
>
>If you want to pick up my short-descriptor and SMMUv3 patches and run
>with them you're more than welcome - the rest is just cosmetic stuff
>which doesn't really matter, especially as it's picking up acks as-is.
>

Sure, i will repost with additional stuff picked up from your branch and
the acks as well.

Regards,
 Sricharan

RE: [RESEND PATCH V6 0/6] Add support for privileged mappings

[Sricharan]

Hi Robin,


>Hi Robin,
>
>>>> Hi Sricharan,
>>>>
>>>> On 02/12/16 14:55, Sricharan R wrote:
>>>>> 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. Not sure why this
>>>>> finally did not make it last time. The last patch in the previous
>>>>> series does not apply now [3], so just redid that. Also Copied the tags
>>>>> that he had from last time as well.
>>>>
>>>> Heh, I was assuming this would be down to me to pick up. Vinod did have
>>>> some complaints last time about the commit message on the PL330 patch -
>>>> I did get as far as rewriting that and reworking onto my SMMU
>>>> changes[1], I just hadn't got round to sending it, so it fell onto the
>>>> "after the next merge window" pile.
>>>>
>>>> I'd give some review comments, but they'd essentially be a diff against
>>>> that branch :)
>>>>
>>>
>>> Sure, i did not knew that you were on this already. I can repost with the diff
>>> from your branch taken in or wait for you as well. I am fine with either ways
>>> that you suggest.
>>>
>>> I checked the patches against your branch, i see that the changes are,
>>>
>>> 1) one patch for implementing it for armv7s descriptor
>>> 2) Changes on pl330 patch commit logs and
>>> 3) One patch for doing the revert on arm-smmuv3 as well.
>>
>>If you want to pick up my short-descriptor and SMMUv3 patches and run
>>with them you're more than welcome - the rest is just cosmetic stuff
>>which doesn't really matter, especially as it's picking up acks as-is.
>>
>
>Sure, i will repost with additional stuff picked up from your branch and
>the acks as well.

Posted V7 [1], while i tested the additional short descriptor changes,
but was not able to get hold of board with arm-smmuv3 in it.

[1] https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1291140.html

Regards,
 Sricharan