[PATCH v2 0/5] Exynos DRM: add support for ARM 64bit arch with IOMMU

[PATCH v2 0/5] Exynos DRM: add support for ARM 64bit arch with IOMMU

[Marek Szyprowski]

Hello,

This patch series reworks IOMMU integration code to add support for ARM
64bit architecture with DMA-IOMMU glue code. Current inplementation uses
conditional code (hidden in the exynos_drm_iommu.h file) because ARM
32bit and 64bit are not compatible in the area of DMA-mapping and IOMMU
glue code. Once both architectures are unified, the conditional code can
be removed, but for now it lets one to use Exynos DRM on both
architectures.

Best regards
Marek Szyprowski
Samsung R&D Institute Poland

Changelog:
v2:
- simplified ifdefs in exynos_drm_iommu.h, so when ARM32 bit will be converted
  to the generic IOMMU-DMA glue code, no changes will be needed to use generic
  code

v1:
- initial version

Patch summary:

Marek Szyprowski (5):
  drm/exynos: iommu: move dma_params configuration code to separate
    functions
  drm/exynos: iommu: add a check if all sub-devices have iommu
    controller
  drm/exynos: iommu: remove unused entries from exynos_drm_private
    strcuture
  drm/exynos: iommu: move ARM specific code to exynos_drm_iommu.h
  drm/exynos: iommu: add support for ARM64 specific code for IOMMU glue

 drivers/gpu/drm/exynos/Kconfig            |  2 +-
 drivers/gpu/drm/exynos/exynos_drm_drv.c   |  7 +--
 drivers/gpu/drm/exynos/exynos_drm_drv.h   |  2 -
 drivers/gpu/drm/exynos/exynos_drm_iommu.c | 77 +++++++++++---------------
 drivers/gpu/drm/exynos/exynos_drm_iommu.h | 91 +++++++++++++++++++++++++++++++
 5 files changed, 126 insertions(+), 53 deletions(-)

-- 
1.9.1

[PATCH v2 1/5] drm/exynos: iommu: move dma_params configuration code to separate functions

[Marek Szyprowski]

Move code for managing DMA max segment size parameter to separate
functions. This patch also replaces devm_kzalloc() with kzalloc() and
adds proper kfree call. devm_kzalloc() cannot be used for dma_params
structure, because it will be freed on driver remove not on device
release. This means in case of Exynos DRM being compiled as module and
loaded 2 times, a user-after-free issue will happen.

Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
 drivers/gpu/drm/exynos/exynos_drm_iommu.c | 28 ++++++++++++++++++++++------
 1 file changed, 22 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_iommu.c b/drivers/gpu/drm/exynos/exynos_drm_iommu.c
index 7ca09ee19656..1e82529e0c41 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_iommu.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_iommu.c
@@ -21,6 +21,23 @@
 #include "exynos_drm_drv.h"
 #include "exynos_drm_iommu.h"
 
+static inline int configure_dma_max_seg_size(struct device *dev)
+{
+	if (!dev->dma_parms)
+		dev->dma_parms = kzalloc(sizeof(*dev->dma_parms), GFP_KERNEL);
+	if (!dev->dma_parms)
+		return -ENOMEM;
+
+	dma_set_max_seg_size(dev, DMA_BIT_MASK(32));
+	return 0;
+}
+
+static inline void clear_dma_max_seg_size(struct device *dev)
+{
+	kfree(dev->dma_parms);
+	dev->dma_parms = NULL;
+}
+
 /*
  * drm_create_iommu_mapping - create a mapping structure
  *
@@ -80,13 +97,10 @@ int drm_iommu_attach_device(struct drm_device *drm_dev,
 	if (!priv->mapping)
 		return 0;
 
-	subdrv_dev->dma_parms = devm_kzalloc(subdrv_dev,
-					sizeof(*subdrv_dev->dma_parms),
-					GFP_KERNEL);
-	if (!subdrv_dev->dma_parms)
-		return -ENOMEM;
 
-	dma_set_max_seg_size(subdrv_dev, 0xffffffffu);
+	ret = configure_dma_max_seg_size(subdrv_dev);
+	if (ret)
+		return ret;
 
 	if (subdrv_dev->archdata.mapping)
 		arm_iommu_detach_device(subdrv_dev);
@@ -94,6 +108,7 @@ int drm_iommu_attach_device(struct drm_device *drm_dev,
 	ret = arm_iommu_attach_device(subdrv_dev, priv->mapping);
 	if (ret < 0) {
 		DRM_DEBUG_KMS("failed iommu attach.\n");
+		clear_dma_max_seg_size(subdrv_dev);
 		return ret;
 	}
 
@@ -119,4 +134,5 @@ void drm_iommu_detach_device(struct drm_device *drm_dev,
 		return;
 
 	arm_iommu_detach_device(subdrv_dev);
+	clear_dma_max_seg_size(subdrv_dev);
 }
-- 
1.9.1

Re: [PATCH v2 1/5] drm/exynos: iommu: move dma_params configuration code to separate functions

[Inki Dae]

Hi Marek,

2016년 06월 17일 16:54에 Marek Szyprowski 이(가) 쓴 글:
> Move code for managing DMA max segment size parameter to separate
> functions. This patch also replaces devm_kzalloc() with kzalloc() and
> adds proper kfree call. devm_kzalloc() cannot be used for dma_params
> structure, because it will be freed on driver remove not on device
> release. This means in case of Exynos DRM being compiled as module and
> loaded 2 times, a user-after-free issue will happen.

Picked this patch series up.

Thanks,
Inki Dae

> 
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> ---
>  drivers/gpu/drm/exynos/exynos_drm_iommu.c | 28 ++++++++++++++++++++++------
>  1 file changed, 22 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_iommu.c b/drivers/gpu/drm/exynos/exynos_drm_iommu.c
> index 7ca09ee19656..1e82529e0c41 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_iommu.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_iommu.c
> @@ -21,6 +21,23 @@
>  #include "exynos_drm_drv.h"
>  #include "exynos_drm_iommu.h"
>  
> +static inline int configure_dma_max_seg_size(struct device *dev)
> +{
> +	if (!dev->dma_parms)
> +		dev->dma_parms = kzalloc(sizeof(*dev->dma_parms), GFP_KERNEL);
> +	if (!dev->dma_parms)
> +		return -ENOMEM;
> +
> +	dma_set_max_seg_size(dev, DMA_BIT_MASK(32));
> +	return 0;
> +}
> +
> +static inline void clear_dma_max_seg_size(struct device *dev)
> +{
> +	kfree(dev->dma_parms);
> +	dev->dma_parms = NULL;
> +}
> +
>  /*
>   * drm_create_iommu_mapping - create a mapping structure
>   *
> @@ -80,13 +97,10 @@ int drm_iommu_attach_device(struct drm_device *drm_dev,
>  	if (!priv->mapping)
>  		return 0;
>  
> -	subdrv_dev->dma_parms = devm_kzalloc(subdrv_dev,
> -					sizeof(*subdrv_dev->dma_parms),
> -					GFP_KERNEL);
> -	if (!subdrv_dev->dma_parms)
> -		return -ENOMEM;
>  
> -	dma_set_max_seg_size(subdrv_dev, 0xffffffffu);
> +	ret = configure_dma_max_seg_size(subdrv_dev);
> +	if (ret)
> +		return ret;
>  
>  	if (subdrv_dev->archdata.mapping)
>  		arm_iommu_detach_device(subdrv_dev);
> @@ -94,6 +108,7 @@ int drm_iommu_attach_device(struct drm_device *drm_dev,
>  	ret = arm_iommu_attach_device(subdrv_dev, priv->mapping);
>  	if (ret < 0) {
>  		DRM_DEBUG_KMS("failed iommu attach.\n");
> +		clear_dma_max_seg_size(subdrv_dev);
>  		return ret;
>  	}
>  
> @@ -119,4 +134,5 @@ void drm_iommu_detach_device(struct drm_device *drm_dev,
>  		return;
>  
>  	arm_iommu_detach_device(subdrv_dev);
> +	clear_dma_max_seg_size(subdrv_dev);
>  }
> 
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 2/5] drm/exynos: iommu: add a check if all sub-devices have iommu controller

[Marek Szyprowski]

This patch adds a check if all devices belonging to Exynos DRM have the
same dma_map_ops set. This is required to enable operation with IOMMU
enabled.

Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
 drivers/gpu/drm/exynos/exynos_drm_iommu.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_iommu.c b/drivers/gpu/drm/exynos/exynos_drm_iommu.c
index 1e82529e0c41..36dde9691274 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_iommu.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_iommu.c
@@ -97,6 +97,11 @@ int drm_iommu_attach_device(struct drm_device *drm_dev,
 	if (!priv->mapping)
 		return 0;
 
+	if (get_dma_ops(priv->dma_dev) != get_dma_ops(subdrv_dev)) {
+		DRM_ERROR("Device %s lacks support for IOMMU\n",
+			  dev_name(subdrv_dev));
+		return -EINVAL;
+	}
 
 	ret = configure_dma_max_seg_size(subdrv_dev);
 	if (ret)
-- 
1.9.1

[PATCH v2 3/5] drm/exynos: iommu: remove unused entries from exynos_drm_private strcuture

[Marek Szyprowski]

This patch removes unused entries from exynos_drm_private strcuture.
da_start/da_space_size were only used in drm_create_iommu_mapping()
function and never set to other value than the defaults. Instead use
default values directly in arm_iommu_create_mapping() call.

Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
 drivers/gpu/drm/exynos/exynos_drm_drv.h   | 2 --
 drivers/gpu/drm/exynos/exynos_drm_iommu.c | 9 ++-------
 2 files changed, 2 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.h b/drivers/gpu/drm/exynos/exynos_drm_drv.h
index cc33ec9296e7..b39d521f093d 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_drv.h
+++ b/drivers/gpu/drm/exynos/exynos_drm_drv.h
@@ -224,8 +224,6 @@ struct exynos_drm_private {
 	struct drm_property *plane_zpos_property;
 
 	struct device *dma_dev;
-	unsigned long da_start;
-	unsigned long da_space_size;
 	void *mapping;
 
 	unsigned int pipe;
diff --git a/drivers/gpu/drm/exynos/exynos_drm_iommu.c b/drivers/gpu/drm/exynos/exynos_drm_iommu.c
index 36dde9691274..0229bad43bd1 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_iommu.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_iommu.c
@@ -48,13 +48,8 @@ int drm_create_iommu_mapping(struct drm_device *drm_dev)
 	struct dma_iommu_mapping *mapping = NULL;
 	struct exynos_drm_private *priv = drm_dev->dev_private;
 
-	if (!priv->da_start)
-		priv->da_start = EXYNOS_DEV_ADDR_START;
-	if (!priv->da_space_size)
-		priv->da_space_size = EXYNOS_DEV_ADDR_SIZE;
-
-	mapping = arm_iommu_create_mapping(&platform_bus_type, priv->da_start,
-						priv->da_space_size);
+	mapping = arm_iommu_create_mapping(&platform_bus_type,
+			EXYNOS_DEV_ADDR_START, EXYNOS_DEV_ADDR_SIZE);
 
 	if (IS_ERR(mapping))
 		return PTR_ERR(mapping);
-- 
1.9.1

[PATCH v2 4/5] drm/exynos: iommu: move ARM specific code to exynos_drm_iommu.h

[Marek Szyprowski]

This patch moves all ARM 32bit DMA-mapping/IOMMU dependant code from
exynos_drm_iommu.c to .h, to let it compile conditionally and prepare
for adding support for other architectures/IOMMU glue code (like ARM
64bit with IOMMU-DMA glue). Later, when ARM 32bit and 64bit will be
unified, this code can be removed.

Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
 drivers/gpu/drm/exynos/exynos_drm_iommu.c | 39 +++++--------------------------
 drivers/gpu/drm/exynos/exynos_drm_iommu.h | 36 ++++++++++++++++++++++++++++
 2 files changed, 42 insertions(+), 33 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_iommu.c b/drivers/gpu/drm/exynos/exynos_drm_iommu.c
index 0229bad43bd1..0f373702414e 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_iommu.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_iommu.c
@@ -14,9 +14,6 @@
 
 #include <linux/dma-mapping.h>
 #include <linux/iommu.h>
-#include <linux/kref.h>
-
-#include <asm/dma-iommu.h>
 
 #include "exynos_drm_drv.h"
 #include "exynos_drm_iommu.h"
@@ -45,33 +42,22 @@ static inline void clear_dma_max_seg_size(struct device *dev)
  */
 int drm_create_iommu_mapping(struct drm_device *drm_dev)
 {
-	struct dma_iommu_mapping *mapping = NULL;
 	struct exynos_drm_private *priv = drm_dev->dev_private;
 
-	mapping = arm_iommu_create_mapping(&platform_bus_type,
-			EXYNOS_DEV_ADDR_START, EXYNOS_DEV_ADDR_SIZE);
-
-	if (IS_ERR(mapping))
-		return PTR_ERR(mapping);
-
-	priv->mapping = mapping;
-
-	return 0;
+	return __exynos_iommu_create_mapping(priv, EXYNOS_DEV_ADDR_START,
+					     EXYNOS_DEV_ADDR_SIZE);
 }
 
 /*
  * drm_release_iommu_mapping - release iommu mapping structure
  *
  * @drm_dev: DRM device
- *
- * if mapping->kref becomes 0 then all things related to iommu mapping
- * will be released
  */
 void drm_release_iommu_mapping(struct drm_device *drm_dev)
 {
 	struct exynos_drm_private *priv = drm_dev->dev_private;
 
-	arm_iommu_release_mapping(priv->mapping);
+	__exynos_iommu_release_mapping(priv);
 }
 
 /*
@@ -89,9 +75,6 @@ int drm_iommu_attach_device(struct drm_device *drm_dev,
 	struct exynos_drm_private *priv = drm_dev->dev_private;
 	int ret;
 
-	if (!priv->mapping)
-		return 0;
-
 	if (get_dma_ops(priv->dma_dev) != get_dma_ops(subdrv_dev)) {
 		DRM_ERROR("Device %s lacks support for IOMMU\n",
 			  dev_name(subdrv_dev));
@@ -102,15 +85,9 @@ int drm_iommu_attach_device(struct drm_device *drm_dev,
 	if (ret)
 		return ret;
 
-	if (subdrv_dev->archdata.mapping)
-		arm_iommu_detach_device(subdrv_dev);
-
-	ret = arm_iommu_attach_device(subdrv_dev, priv->mapping);
-	if (ret < 0) {
-		DRM_DEBUG_KMS("failed iommu attach.\n");
+	ret = __exynos_iommu_attach(priv, subdrv_dev);
+	if (ret)
 		clear_dma_max_seg_size(subdrv_dev);
-		return ret;
-	}
 
 	return 0;
 }
@@ -128,11 +105,7 @@ void drm_iommu_detach_device(struct drm_device *drm_dev,
 				struct device *subdrv_dev)
 {
 	struct exynos_drm_private *priv = drm_dev->dev_private;
-	struct dma_iommu_mapping *mapping = priv->mapping;
-
-	if (!mapping || !mapping->domain)
-		return;
 
-	arm_iommu_detach_device(subdrv_dev);
+	__exynos_iommu_detach(priv, subdrv_dev);
 	clear_dma_max_seg_size(subdrv_dev);
 }
diff --git a/drivers/gpu/drm/exynos/exynos_drm_iommu.h b/drivers/gpu/drm/exynos/exynos_drm_iommu.h
index 5ffebe02ee4d..22e1df2ac62e 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_iommu.h
+++ b/drivers/gpu/drm/exynos/exynos_drm_iommu.h
@@ -17,6 +17,42 @@
 
 #ifdef CONFIG_DRM_EXYNOS_IOMMU
 
+#if defined(CONFIG_ARM_DMA_USE_IOMMU)
+#include <asm/dma-iommu.h>
+
+static inline int __exynos_iommu_create_mapping(struct exynos_drm_private *priv,
+					unsigned long start, unsigned long size)
+{
+	priv->mapping = arm_iommu_create_mapping(&platform_bus_type, start,
+						 size);
+	return IS_ERR(priv->mapping);
+}
+
+static inline void
+__exynos_iommu_release_mapping(struct exynos_drm_private *priv)
+{
+	arm_iommu_release_mapping(priv->mapping);
+}
+
+static inline int __exynos_iommu_attach(struct exynos_drm_private *priv,
+					struct device *dev)
+{
+	if (dev->archdata.mapping)
+		arm_iommu_detach_device(dev);
+
+	return arm_iommu_attach_device(dev, priv->mapping);
+}
+
+static inline void __exynos_iommu_detach(struct exynos_drm_private *priv,
+					 struct device *dev)
+{
+	arm_iommu_detach_device(dev);
+}
+
+#else
+#error Unsupported architecture and IOMMU/DMA-mapping glue code
+#endif
+
 int drm_create_iommu_mapping(struct drm_device *drm_dev);
 
 void drm_release_iommu_mapping(struct drm_device *drm_dev);
-- 
1.9.1

[PATCH v2 5/5] drm/exynos: iommu: add support for ARM64 specific code for IOMMU glue

[Marek Szyprowski]

This patch adds support for ARM 64bit architecture with IOMMU-DMA glue
code, so Exynos DRM can be now used on Exynos 5433 with IOMMU enabled.

Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
 drivers/gpu/drm/exynos/Kconfig            |  2 +-
 drivers/gpu/drm/exynos/exynos_drm_drv.c   |  7 +---
 drivers/gpu/drm/exynos/exynos_drm_iommu.h | 55 +++++++++++++++++++++++++++++++
 3 files changed, 57 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/exynos/Kconfig b/drivers/gpu/drm/exynos/Kconfig
index d814b3048ee5..343813a5fbc8 100644
--- a/drivers/gpu/drm/exynos/Kconfig
+++ b/drivers/gpu/drm/exynos/Kconfig
@@ -15,7 +15,7 @@ if DRM_EXYNOS
 
 config DRM_EXYNOS_IOMMU
 	bool
-	depends on EXYNOS_IOMMU && ARM_DMA_USE_IOMMU
+	depends on EXYNOS_IOMMU
 	default y
 
 comment "CRTCs"
diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c b/drivers/gpu/drm/exynos/exynos_drm_drv.c
index 2dd820e23b0c..062fa9fea0f2 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_drv.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c
@@ -159,12 +159,7 @@ static int exynos_drm_load(struct drm_device *dev, unsigned long flags)
 	DRM_INFO("Exynos DRM: using %s device for DMA mapping operations\n",
 		 dev_name(private->dma_dev));
 
-	/*
-	 * create mapping to manage iommu table and set a pointer to iommu
-	 * mapping structure to iommu_mapping of private data.
-	 * also this iommu_mapping can be used to check if iommu is supported
-	 * or not.
-	 */
+	/* create common IOMMU mapping for all devices attached to Exynos DRM */
 	ret = drm_create_iommu_mapping(dev);
 	if (ret < 0) {
 		DRM_ERROR("failed to create iommu mapping.\n");
diff --git a/drivers/gpu/drm/exynos/exynos_drm_iommu.h b/drivers/gpu/drm/exynos/exynos_drm_iommu.h
index 22e1df2ac62e..c8de4913fdbe 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_iommu.h
+++ b/drivers/gpu/drm/exynos/exynos_drm_iommu.h
@@ -49,6 +49,61 @@ static inline void __exynos_iommu_detach(struct exynos_drm_private *priv,
 	arm_iommu_detach_device(dev);
 }
 
+#elif defined(CONFIG_IOMMU_DMA)
+#include <linux/dma-iommu.h>
+
+static inline int __exynos_iommu_create_mapping(struct exynos_drm_private *priv,
+					unsigned long start, unsigned long size)
+{
+	struct iommu_domain *domain;
+	int ret;
+
+	domain = iommu_domain_alloc(priv->dma_dev->bus);
+	if (!domain)
+		return -ENOMEM;
+
+	ret = iommu_get_dma_cookie(domain);
+	if (ret)
+		goto free_domain;
+
+	ret = iommu_dma_init_domain(domain, start, size);
+	if (ret)
+		goto put_cookie;
+
+	priv->mapping = domain;
+	return 0;
+
+put_cookie:
+	iommu_put_dma_cookie(domain);
+free_domain:
+	iommu_domain_free(domain);
+	return ret;
+}
+
+static inline void __exynos_iommu_release_mapping(struct exynos_drm_private *priv)
+{
+	struct iommu_domain *domain = priv->mapping;
+
+	iommu_put_dma_cookie(domain);
+	iommu_domain_free(domain);
+	priv->mapping = NULL;
+}
+
+static inline int __exynos_iommu_attach(struct exynos_drm_private *priv,
+					struct device *dev)
+{
+	struct iommu_domain *domain = priv->mapping;
+
+	return iommu_attach_device(domain, dev);
+}
+
+static inline void __exynos_iommu_detach(struct exynos_drm_private *priv,
+					 struct device *dev)
+{
+	struct iommu_domain *domain = priv->mapping;
+
+	iommu_detach_device(domain, dev);
+}
 #else
 #error Unsupported architecture and IOMMU/DMA-mapping glue code
 #endif
-- 
1.9.1