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(-)
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); }
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);@@ -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);
}
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)
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);
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);
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
dri-devel@lists.freedesktop.org
-
Inki Dae -
Marek Szyprowski