[PATCH 0/3] iommu: replace IOMMU_EXEC with IOMMU_EXEC and update ARM SMMU driver

[PATCH 0/3] iommu: replace IOMMU_EXEC with IOMMU_EXEC and update ARM SMMU driver

[Antonios Motakis]

This patch series applies to Joerg Roedel's iommu/next branch, commit 09b5269a.
It replaces the IOMMU_EXEC flag used by the ARM SMMU driver to IOMMU_NOEXEC.
This is more enforceable, since the lack of the flag on hardware that doesn't
support it implies that the target memory will be executable.

Antonios Motakis (3):
  iommu/arm-smmu: change IOMMU_EXEC to IOMMU_NOEXEC
  iommu: add capability IOMMU_CAP_NOEXEC
  iommu/arm-smmu: add IOMMU_CAP_NOEXEC to the ARM SMMU driver

 drivers/iommu/arm-smmu.c | 11 +++++++----
 include/linux/iommu.h    |  3 ++-
 2 files changed, 9 insertions(+), 5 deletions(-)

-- 
2.1.1

[PATCH 1/3] iommu/arm-smmu: change IOMMU_EXEC to IOMMU_NOEXEC

[Antonios Motakis]

Exposing the XN flag of the SMMU driver as IOMMU_NOEXEC instead of
IOMMU_EXEC makes it enforceable, since for IOMMUs that don't support
the XN flag pages will always be executable.

Signed-off-by: Antonios Motakis <a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org>
Acked-by: Joerg Roedel <jroedel-l3A5Bk7waGM@public.gmane.org>
---
 drivers/iommu/arm-smmu.c | 9 +++++----
 include/linux/iommu.h    | 2 +-
 2 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index 60558f7..566c176 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -1281,7 +1281,7 @@ static int arm_smmu_alloc_init_pte(struct arm_smmu_device *smmu, pmd_t *pmd,
 				   unsigned long pfn, int prot, int stage)
 {
 	pte_t *pte, *start;
-	pteval_t pteval = ARM_SMMU_PTE_PAGE | ARM_SMMU_PTE_AF | ARM_SMMU_PTE_XN;
+	pteval_t pteval = ARM_SMMU_PTE_PAGE | ARM_SMMU_PTE_AF;
 
 	if (pmd_none(*pmd)) {
 		/* Allocate a new set of tables */
@@ -1315,10 +1315,11 @@ static int arm_smmu_alloc_init_pte(struct arm_smmu_device *smmu, pmd_t *pmd,
 			pteval |= ARM_SMMU_PTE_MEMATTR_NC;
 	}
 
+	if (prot & IOMMU_NOEXEC)
+		pteval |= ARM_SMMU_PTE_XN;
+
 	/* If no access, create a faulting entry to avoid TLB fills */
-	if (prot & IOMMU_EXEC)
-		pteval &= ~ARM_SMMU_PTE_XN;
-	else if (!(prot & (IOMMU_READ | IOMMU_WRITE)))
+	if (!(prot & (IOMMU_READ | IOMMU_WRITE)))
 		pteval &= ~ARM_SMMU_PTE_PAGE;
 
 	pteval |= ARM_SMMU_PTE_SH_IS;
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 379a617..5f6f71c 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -27,7 +27,7 @@
 #define IOMMU_READ	(1 << 0)
 #define IOMMU_WRITE	(1 << 1)
 #define IOMMU_CACHE	(1 << 2) /* DMA cache coherency */
-#define IOMMU_EXEC	(1 << 3)
+#define IOMMU_NOEXEC	(1 << 3)
 
 struct iommu_ops;
 struct iommu_group;
-- 
2.1.1

[PATCH 1/3] iommu/arm-smmu: change IOMMU_EXEC to IOMMU_NOEXEC

[Antonios Motakis]

Exposing the XN flag of the SMMU driver as IOMMU_NOEXEC instead of
IOMMU_EXEC makes it enforceable, since for IOMMUs that don't support
the XN flag pages will always be executable.

Signed-off-by: Antonios Motakis <a.motakis@virtualopensystems.com>
Acked-by: Joerg Roedel <jroedel@suse.de>
---
 drivers/iommu/arm-smmu.c | 9 +++++----
 include/linux/iommu.h    | 2 +-
 2 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index 60558f7..566c176 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -1281,7 +1281,7 @@ static int arm_smmu_alloc_init_pte(struct arm_smmu_device *smmu, pmd_t *pmd,
 				   unsigned long pfn, int prot, int stage)
 {
 	pte_t *pte, *start;
-	pteval_t pteval = ARM_SMMU_PTE_PAGE | ARM_SMMU_PTE_AF | ARM_SMMU_PTE_XN;
+	pteval_t pteval = ARM_SMMU_PTE_PAGE | ARM_SMMU_PTE_AF;
 
 	if (pmd_none(*pmd)) {
 		/* Allocate a new set of tables */
@@ -1315,10 +1315,11 @@ static int arm_smmu_alloc_init_pte(struct arm_smmu_device *smmu, pmd_t *pmd,
 			pteval |= ARM_SMMU_PTE_MEMATTR_NC;
 	}
 
+	if (prot & IOMMU_NOEXEC)
+		pteval |= ARM_SMMU_PTE_XN;
+
 	/* If no access, create a faulting entry to avoid TLB fills */
-	if (prot & IOMMU_EXEC)
-		pteval &= ~ARM_SMMU_PTE_XN;
-	else if (!(prot & (IOMMU_READ | IOMMU_WRITE)))
+	if (!(prot & (IOMMU_READ | IOMMU_WRITE)))
 		pteval &= ~ARM_SMMU_PTE_PAGE;
 
 	pteval |= ARM_SMMU_PTE_SH_IS;
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 379a617..5f6f71c 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -27,7 +27,7 @@
 #define IOMMU_READ	(1 << 0)
 #define IOMMU_WRITE	(1 << 1)
 #define IOMMU_CACHE	(1 << 2) /* DMA cache coherency */
-#define IOMMU_EXEC	(1 << 3)
+#define IOMMU_NOEXEC	(1 << 3)
 
 struct iommu_ops;
 struct iommu_group;
-- 
2.1.1

[PATCH 1/3] iommu/arm-smmu: change IOMMU_EXEC to IOMMU_NOEXEC

[Antonios Motakis]

Exposing the XN flag of the SMMU driver as IOMMU_NOEXEC instead of
IOMMU_EXEC makes it enforceable, since for IOMMUs that don't support
the XN flag pages will always be executable.

Signed-off-by: Antonios Motakis <a.motakis@virtualopensystems.com>
Acked-by: Joerg Roedel <jroedel@suse.de>
---
 drivers/iommu/arm-smmu.c | 9 +++++----
 include/linux/iommu.h    | 2 +-
 2 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index 60558f7..566c176 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -1281,7 +1281,7 @@ static int arm_smmu_alloc_init_pte(struct arm_smmu_device *smmu, pmd_t *pmd,
 				   unsigned long pfn, int prot, int stage)
 {
 	pte_t *pte, *start;
-	pteval_t pteval = ARM_SMMU_PTE_PAGE | ARM_SMMU_PTE_AF | ARM_SMMU_PTE_XN;
+	pteval_t pteval = ARM_SMMU_PTE_PAGE | ARM_SMMU_PTE_AF;
 
 	if (pmd_none(*pmd)) {
 		/* Allocate a new set of tables */
@@ -1315,10 +1315,11 @@ static int arm_smmu_alloc_init_pte(struct arm_smmu_device *smmu, pmd_t *pmd,
 			pteval |= ARM_SMMU_PTE_MEMATTR_NC;
 	}
 
+	if (prot & IOMMU_NOEXEC)
+		pteval |= ARM_SMMU_PTE_XN;
+
 	/* If no access, create a faulting entry to avoid TLB fills */
-	if (prot & IOMMU_EXEC)
-		pteval &= ~ARM_SMMU_PTE_XN;
-	else if (!(prot & (IOMMU_READ | IOMMU_WRITE)))
+	if (!(prot & (IOMMU_READ | IOMMU_WRITE)))
 		pteval &= ~ARM_SMMU_PTE_PAGE;
 
 	pteval |= ARM_SMMU_PTE_SH_IS;
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 379a617..5f6f71c 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -27,7 +27,7 @@
 #define IOMMU_READ	(1 << 0)
 #define IOMMU_WRITE	(1 << 1)
 #define IOMMU_CACHE	(1 << 2) /* DMA cache coherency */
-#define IOMMU_EXEC	(1 << 3)
+#define IOMMU_NOEXEC	(1 << 3)
 
 struct iommu_ops;
 struct iommu_group;
-- 
2.1.1

[PATCH 2/3] iommu: add capability IOMMU_CAP_NOEXEC

[Antonios Motakis]

Some IOMMUs accept an IOMMU_NOEXEC protection flag in addition to
IOMMU_READ and IOMMU_WRITE. Expose this as an IOMMU capability.

Signed-off-by: Antonios Motakis <a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org>
Acked-by: Joerg Roedel <jroedel-l3A5Bk7waGM@public.gmane.org>
---
 include/linux/iommu.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 5f6f71c..ba026f1 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -61,6 +61,7 @@ enum iommu_cap {
 	IOMMU_CAP_CACHE_COHERENCY,	/* IOMMU can enforce cache coherent DMA
 					   transactions */
 	IOMMU_CAP_INTR_REMAP,		/* IOMMU supports interrupt isolation */
+	IOMMU_CAP_NOEXEC,		/* IOMMU_NOEXEC flag */
 };
 
 /*
-- 
2.1.1

[PATCH 2/3] iommu: add capability IOMMU_CAP_NOEXEC

[Antonios Motakis]

Some IOMMUs accept an IOMMU_NOEXEC protection flag in addition to
IOMMU_READ and IOMMU_WRITE. Expose this as an IOMMU capability.

Signed-off-by: Antonios Motakis <a.motakis@virtualopensystems.com>
Acked-by: Joerg Roedel <jroedel@suse.de>
---
 include/linux/iommu.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 5f6f71c..ba026f1 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -61,6 +61,7 @@ enum iommu_cap {
 	IOMMU_CAP_CACHE_COHERENCY,	/* IOMMU can enforce cache coherent DMA
 					   transactions */
 	IOMMU_CAP_INTR_REMAP,		/* IOMMU supports interrupt isolation */
+	IOMMU_CAP_NOEXEC,		/* IOMMU_NOEXEC flag */
 };
 
 /*
-- 
2.1.1

[PATCH 3/3] iommu/arm-smmu: add IOMMU_CAP_NOEXEC to the ARM SMMU driver

[Antonios Motakis]

The ARM SMMU supports the IOMMU_NOEXEC protection flag. Add the
corresponding IOMMU capability.

Signed-off-by: Antonios Motakis <a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org>
---
 drivers/iommu/arm-smmu.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index 566c176..c8fc02f 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -1569,6 +1569,8 @@ static bool arm_smmu_capable(enum iommu_cap cap)
 		return true;
 	case IOMMU_CAP_INTR_REMAP:
 		return true; /* MSIs are just memory writes */
+	case IOMMU_CAP_NOEXEC:
+		return true;
 	default:
 		return false;
 	}
-- 
2.1.1

[PATCH 3/3] iommu/arm-smmu: add IOMMU_CAP_NOEXEC to the ARM SMMU driver

[Antonios Motakis]

The ARM SMMU supports the IOMMU_NOEXEC protection flag. Add the
corresponding IOMMU capability.

Signed-off-by: Antonios Motakis <a.motakis@virtualopensystems.com>
---
 drivers/iommu/arm-smmu.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index 566c176..c8fc02f 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -1569,6 +1569,8 @@ static bool arm_smmu_capable(enum iommu_cap cap)
 		return true;
 	case IOMMU_CAP_INTR_REMAP:
 		return true; /* MSIs are just memory writes */
+	case IOMMU_CAP_NOEXEC:
+		return true;
 	default:
 		return false;
 	}
-- 
2.1.1

[PATCH 3/3] iommu/arm-smmu: add IOMMU_CAP_NOEXEC to the ARM SMMU driver

[Antonios Motakis]

The ARM SMMU supports the IOMMU_NOEXEC protection flag. Add the
corresponding IOMMU capability.

Signed-off-by: Antonios Motakis <a.motakis@virtualopensystems.com>
---
 drivers/iommu/arm-smmu.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index 566c176..c8fc02f 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -1569,6 +1569,8 @@ static bool arm_smmu_capable(enum iommu_cap cap)
 		return true;
 	case IOMMU_CAP_INTR_REMAP:
 		return true; /* MSIs are just memory writes */
+	case IOMMU_CAP_NOEXEC:
+		return true;
 	default:
 		return false;
 	}
-- 
2.1.1

Re: [PATCH 0/3] iommu: replace IOMMU_EXEC with IOMMU_EXEC and update ARM SMMU driver

[Will Deacon]

Hi Antonios,

On Mon, Oct 13, 2014 at 02:06:15PM +0100, Antonios Motakis wrote:
> This patch series applies to Joerg Roedel's iommu/next branch, commit 09b5269a.
> It replaces the IOMMU_EXEC flag used by the ARM SMMU driver to IOMMU_NOEXEC.
> This is more enforceable, since the lack of the flag on hardware that doesn't
> support it implies that the target memory will be executable.

Looks good to me; I'll take this via the arm-smmu tree and send it to Joerg
along with anything else that gets queued for 3.19.

Thanks,

Will

Re: [PATCH 0/3] iommu: replace IOMMU_EXEC with IOMMU_EXEC and update ARM SMMU driver

[Will Deacon]

On Mon, Oct 20, 2014 at 04:39:15PM +0100, Will Deacon wrote:
> On Mon, Oct 13, 2014 at 02:06:15PM +0100, Antonios Motakis wrote:
> > This patch series applies to Joerg Roedel's iommu/next branch, commit 09b5269a.
> > It replaces the IOMMU_EXEC flag used by the ARM SMMU driver to IOMMU_NOEXEC.
> > This is more enforceable, since the lack of the flag on hardware that doesn't
> > support it implies that the target memory will be executable.
> 
> Looks good to me; I'll take this via the arm-smmu tree and send it to Joerg
> along with anything else that gets queued for 3.19.

The 0-day builder spotted a new warning from this series:

   drivers/iommu/amd_iommu.c: In function 'amd_iommu_capable':
>> drivers/iommu/amd_iommu.c:3409:2: warning: enumeration value 'IOMMU_CAP_NOEXEC' not handled in switch [-Wswitch]
     switch (cap) {
     ^

I fixed it with the patch below, but I'd appreciate you and Joerg taking
a look too.

Cheers,

Will

--->8

diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
index 505a9adac2d5..3d78a8fb5a6a 100644
--- a/drivers/iommu/amd_iommu.c
+++ b/drivers/iommu/amd_iommu.c
@@ -3411,6 +3411,8 @@ static bool amd_iommu_capable(enum iommu_cap cap)
                return true;
        case IOMMU_CAP_INTR_REMAP:
                return (irq_remapping_enabled == 1);
+       case IOMMU_CAP_NOEXEC:
+               return false;
        }
 
        return false;

Re: [PATCH 0/3] iommu: replace IOMMU_EXEC with IOMMU_EXEC and update ARM SMMU driver

[Antonios Motakis]

On Mon, Oct 20, 2014 at 8:42 PM, Will Deacon <will.deacon-5wv7dgnIgG8@public.gmane.org> wrote:
> On Mon, Oct 20, 2014 at 04:39:15PM +0100, Will Deacon wrote:
>> On Mon, Oct 13, 2014 at 02:06:15PM +0100, Antonios Motakis wrote:
>> > This patch series applies to Joerg Roedel's iommu/next branch, commit 09b5269a.
>> > It replaces the IOMMU_EXEC flag used by the ARM SMMU driver to IOMMU_NOEXEC.
>> > This is more enforceable, since the lack of the flag on hardware that doesn't
>> > support it implies that the target memory will be executable.
>>
>> Looks good to me; I'll take this via the arm-smmu tree and send it to Joerg
>> along with anything else that gets queued for 3.19.
>
> The 0-day builder spotted a new warning from this series:
>
>    drivers/iommu/amd_iommu.c: In function 'amd_iommu_capable':
>>> drivers/iommu/amd_iommu.c:3409:2: warning: enumeration value 'IOMMU_CAP_NOEXEC' not handled in switch [-Wswitch]
>      switch (cap) {
>      ^
>
> I fixed it with the patch below, but I'd appreciate you and Joerg taking
> a look too.
>

Maybe a "default:" case would be more appropriate, in order to avoid a
similar situation in the future?


> Cheers,
>
> Will
>
> --->8
>
> diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
> index 505a9adac2d5..3d78a8fb5a6a 100644
> --- a/drivers/iommu/amd_iommu.c
> +++ b/drivers/iommu/amd_iommu.c
> @@ -3411,6 +3411,8 @@ static bool amd_iommu_capable(enum iommu_cap cap)
>                 return true;
>         case IOMMU_CAP_INTR_REMAP:
>                 return (irq_remapping_enabled == 1);
> +       case IOMMU_CAP_NOEXEC:
> +               return false;
>         }
>
>         return false;



-- 
Antonios Motakis
Virtual Open Systems

Re: [PATCH 0/3] iommu: replace IOMMU_EXEC with IOMMU_EXEC and update ARM SMMU driver

[jroedel-l3A5Bk7waGM]

On Mon, Oct 20, 2014 at 07:42:01PM +0100, Will Deacon wrote:
> On Mon, Oct 20, 2014 at 04:39:15PM +0100, Will Deacon wrote:
> > On Mon, Oct 13, 2014 at 02:06:15PM +0100, Antonios Motakis wrote:
> > > This patch series applies to Joerg Roedel's iommu/next branch, commit 09b5269a.
> > > It replaces the IOMMU_EXEC flag used by the ARM SMMU driver to IOMMU_NOEXEC.
> > > This is more enforceable, since the lack of the flag on hardware that doesn't
> > > support it implies that the target memory will be executable.
> > 
> > Looks good to me; I'll take this via the arm-smmu tree and send it to Joerg
> > along with anything else that gets queued for 3.19.
> 
> The 0-day builder spotted a new warning from this series:
> 
>    drivers/iommu/amd_iommu.c: In function 'amd_iommu_capable':
> >> drivers/iommu/amd_iommu.c:3409:2: warning: enumeration value 'IOMMU_CAP_NOEXEC' not handled in switch [-Wswitch]
>      switch (cap) {
>      ^
> 
> I fixed it with the patch below, but I'd appreciate you and Joerg taking
> a look too.
> 
> Cheers,
> 
> Will
> 
> --->8
> 
> diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
> index 505a9adac2d5..3d78a8fb5a6a 100644
> --- a/drivers/iommu/amd_iommu.c
> +++ b/drivers/iommu/amd_iommu.c
> @@ -3411,6 +3411,8 @@ static bool amd_iommu_capable(enum iommu_cap cap)
>                 return true;
>         case IOMMU_CAP_INTR_REMAP:
>                 return (irq_remapping_enabled == 1);
> +       case IOMMU_CAP_NOEXEC:
> +               return false;
>         }

Looks good to me.