Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1405065 > unrolled thread

[PATCH 0/5] iommu/rockchip: fix bugs; enable iommu for ARM64

Back to article view | Back to linux.kernel

Contents

  [PATCH 0/5] iommu/rockchip: fix bugs; enable iommu for ARM64 Shunqian Zheng <zhengsq@rock-chips.com> - 2016-05-23 03:40 +0200
    [PATCH 5/5] iommu/rockchip: enable rockchip iommu on ARM64 platform Shunqian Zheng <zhengsq@rock-chips.com> - 2016-05-23 03:40 +0200
    [PATCH 2/5] iommu/rockchip: add map_sg callback for rk_iommu_ops Shunqian Zheng <zhengsq@rock-chips.com> - 2016-05-23 03:40 +0200
    [PATCH 3/5] iommu/rockchip: add sanity check for virtual device Shunqian Zheng <zhengsq@rock-chips.com> - 2016-05-23 03:40 +0200
    [PATCH 4/5] iommu/rockchip: add ARM64 cache flush operation for iommu Shunqian Zheng <zhengsq@rock-chips.com> - 2016-05-23 03:40 +0200
      Re: [PATCH 4/5] iommu/rockchip: add ARM64 cache flush operation for  iommu Robin Murphy <robin.murphy@arm.com> - 2016-05-23 12:50 +0200
        Re: [PATCH 4/5] iommu/rockchip: add ARM64 cache flush operation for  iommu Catalin Marinas <catalin.marinas@arm.com> - 2016-05-23 15:40 +0200
          Re: [PATCH 4/5] iommu/rockchip: add ARM64 cache flush operation for  iommu Shunqian Zheng <zhengsq@rock-chips.com> - 2016-05-24 04:40 +0200
            Re: [PATCH 4/5] iommu/rockchip: add ARM64 cache flush operation for  iommu Catalin Marinas <catalin.marinas@arm.com> - 2016-05-24 12:00 +0200

#1405065 — [PATCH 0/5] iommu/rockchip: fix bugs; enable iommu for ARM64

Fix a few bugs for rockchip iommu and enable for ARM64.

Simon (5):
  iommu/rockchip: fix devm_request_irq and devm_free_irq parameter
  iommu/rockchip: add map_sg callback for rk_iommu_ops
  iommu/rockchip: add sanity check for virtual device
  iommu/rockchip: add ARM64 cache flush operation for iommu
  iommu/rockchip: enable rockchip iommu on ARM64 platform

 drivers/iommu/Kconfig          |  2 +-
 drivers/iommu/rockchip-iommu.c | 16 +++++++++++++---
 2 files changed, 14 insertions(+), 4 deletions(-)

-- 
1.9.1

[toc] | [next] | [standalone]

#1405066 — [PATCH 5/5] iommu/rockchip: enable rockchip iommu on ARM64 platform

From: Simon <xxm@rock-chips.com>

Signed-off-by: Simon <xxm@rock-chips.com>
---
 drivers/iommu/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
index ad08603..5572621 100644
--- a/drivers/iommu/Kconfig
+++ b/drivers/iommu/Kconfig
@@ -218,7 +218,7 @@ config OMAP_IOMMU_DEBUG
 
 config ROCKCHIP_IOMMU
 	bool "Rockchip IOMMU Support"
-	depends on ARM
+	depends on ARM || ARM64
 	depends on ARCH_ROCKCHIP || COMPILE_TEST
 	select IOMMU_API
 	select ARM_DMA_USE_IOMMU
-- 
1.9.1

[toc] | [prev] | [next] | [standalone]

#1405067 — [PATCH 2/5] iommu/rockchip: add map_sg callback for rk_iommu_ops

From: Simon <xxm@rock-chips.com>

The iommu_dma_alloc() in iommu/dma-iommu.c calls iommu_map_sg()
that requires the callback iommu_ops .map_sg(). Adding the
default_iommu_map_sg() to rockchip iommu accordingly.

Signed-off-by: Simon <xxm@rock-chips.com>
---
 drivers/iommu/rockchip-iommu.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/iommu/rockchip-iommu.c b/drivers/iommu/rockchip-iommu.c
index ec0ce62..3c16ec3 100644
--- a/drivers/iommu/rockchip-iommu.c
+++ b/drivers/iommu/rockchip-iommu.c
@@ -1022,6 +1022,7 @@ static const struct iommu_ops rk_iommu_ops = {
 	.detach_dev = rk_iommu_detach_device,
 	.map = rk_iommu_map,
 	.unmap = rk_iommu_unmap,
+	.map_sg = default_iommu_map_sg,
 	.add_device = rk_iommu_add_device,
 	.remove_device = rk_iommu_remove_device,
 	.iova_to_phys = rk_iommu_iova_to_phys,
-- 
1.9.1

[toc] | [prev] | [next] | [standalone]

#1405068 — [PATCH 3/5] iommu/rockchip: add sanity check for virtual device

From: Simon <xxm@rock-chips.com>

We mark DRM as "virtual" device by checking it's group,
which may no longer true with the new rockchip drm drv in
https://patchwork.kernel.org/patch/8876331/.

Checking the group->iommu_data for virtual device.

Signed-off-by: Simon <xxm@rock-chips.com>
---
 drivers/iommu/rockchip-iommu.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/iommu/rockchip-iommu.c b/drivers/iommu/rockchip-iommu.c
index 3c16ec3..043d18c 100644
--- a/drivers/iommu/rockchip-iommu.c
+++ b/drivers/iommu/rockchip-iommu.c
@@ -764,6 +764,7 @@ static size_t rk_iommu_unmap(struct iommu_domain *domain, unsigned long _iova,
 	return unmap_size;
 }
 
+/* Return NULL for a virtual device like DRM */
 static struct rk_iommu *rk_iommu_from_dev(struct device *dev)
 {
 	struct iommu_group *group;
@@ -774,6 +775,11 @@ static struct rk_iommu *rk_iommu_from_dev(struct device *dev)
 	if (!group)
 		return NULL;
 	iommu_dev = iommu_group_get_iommudata(group);
+	if (!iommu_dev) {
+		dev_info(dev, "Possibly a virtual device\n");
+		return NULL;
+	}
+
 	rk_iommu = dev_get_drvdata(iommu_dev);
 	iommu_group_put(group);
 
@@ -791,7 +797,6 @@ static int rk_iommu_attach_device(struct iommu_domain *domain,
 
 	/*
 	 * Allow 'virtual devices' (e.g., drm) to attach to domain.
-	 * Such a device does not belong to an iommu group.
 	 */
 	iommu = rk_iommu_from_dev(dev);
 	if (!iommu)
-- 
1.9.1

[toc] | [prev] | [next] | [standalone]

#1405069 — [PATCH 4/5] iommu/rockchip: add ARM64 cache flush operation for iommu

From: Simon <xxm@rock-chips.com>

Signed-off-by: Simon <xxm@rock-chips.com>
---
 drivers/iommu/rockchip-iommu.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/iommu/rockchip-iommu.c b/drivers/iommu/rockchip-iommu.c
index 043d18c..1741b65 100644
--- a/drivers/iommu/rockchip-iommu.c
+++ b/drivers/iommu/rockchip-iommu.c
@@ -95,12 +95,16 @@ struct rk_iommu {
 
 static inline void rk_table_flush(u32 *va, unsigned int count)
 {
+#if defined(CONFIG_ARM)
 	phys_addr_t pa_start = virt_to_phys(va);
 	phys_addr_t pa_end = virt_to_phys(va + count);
 	size_t size = pa_end - pa_start;
 
 	__cpuc_flush_dcache_area(va, size);
 	outer_flush_range(pa_start, pa_end);
+#elif defined(CONFIG_ARM64)
+	__dma_flush_range(va, va + count);
+#endif
 }
 
 static struct rk_iommu_domain *to_rk_domain(struct iommu_domain *dom)
-- 
1.9.1

[toc] | [prev] | [next] | [standalone]

#1405229 — Re: [PATCH 4/5] iommu/rockchip: add ARM64 cache flush operation for iommu

On 23/05/16 02:37, Shunqian Zheng wrote:
> From: Simon <xxm@rock-chips.com>
>
> Signed-off-by: Simon <xxm@rock-chips.com>
> ---
>   drivers/iommu/rockchip-iommu.c | 4 ++++
>   1 file changed, 4 insertions(+)
>
> diff --git a/drivers/iommu/rockchip-iommu.c b/drivers/iommu/rockchip-iommu.c
> index 043d18c..1741b65 100644
> --- a/drivers/iommu/rockchip-iommu.c
> +++ b/drivers/iommu/rockchip-iommu.c
> @@ -95,12 +95,16 @@ struct rk_iommu {
>
>   static inline void rk_table_flush(u32 *va, unsigned int count)
>   {
> +#if defined(CONFIG_ARM)
>   	phys_addr_t pa_start = virt_to_phys(va);
>   	phys_addr_t pa_end = virt_to_phys(va + count);
>   	size_t size = pa_end - pa_start;
>
>   	__cpuc_flush_dcache_area(va, size);
>   	outer_flush_range(pa_start, pa_end);
> +#elif defined(CONFIG_ARM64)
> +	__dma_flush_range(va, va + count);
> +#endif

Ugh, please don't use arch-private cache maintenance functions directly 
from a driver. Allocating/mapping page tables to be read by the IOMMU is 
still DMA, so using the DMA APIs is the correct way to manage them, 
*especially* if it needs to work across multiple architectures.

Robin.

>   }
>
>   static struct rk_iommu_domain *to_rk_domain(struct iommu_domain *dom)
>

[toc] | [prev] | [next] | [standalone]

#1405380 — Re: [PATCH 4/5] iommu/rockchip: add ARM64 cache flush operation for iommu

On Mon, May 23, 2016 at 11:44:14AM +0100, Robin Murphy wrote:
> On 23/05/16 02:37, Shunqian Zheng wrote:
> >From: Simon <xxm@rock-chips.com>
> >
> >Signed-off-by: Simon <xxm@rock-chips.com>
> >---
> >  drivers/iommu/rockchip-iommu.c | 4 ++++
> >  1 file changed, 4 insertions(+)
> >
> >diff --git a/drivers/iommu/rockchip-iommu.c b/drivers/iommu/rockchip-iommu.c
> >index 043d18c..1741b65 100644
> >--- a/drivers/iommu/rockchip-iommu.c
> >+++ b/drivers/iommu/rockchip-iommu.c
> >@@ -95,12 +95,16 @@ struct rk_iommu {
> >
> >  static inline void rk_table_flush(u32 *va, unsigned int count)
> >  {
> >+#if defined(CONFIG_ARM)
> >  	phys_addr_t pa_start = virt_to_phys(va);
> >  	phys_addr_t pa_end = virt_to_phys(va + count);
> >  	size_t size = pa_end - pa_start;
> >
> >  	__cpuc_flush_dcache_area(va, size);
> >  	outer_flush_range(pa_start, pa_end);
> >+#elif defined(CONFIG_ARM64)
> >+	__dma_flush_range(va, va + count);
> >+#endif
> 
> Ugh, please don't use arch-private cache maintenance functions directly from
> a driver. Allocating/mapping page tables to be read by the IOMMU is still
> DMA, so using the DMA APIs is the correct way to manage them, *especially*
> if it needs to work across multiple architectures.

I fully agree, these functions should not be used in drivers.

-- 
Catalin

[toc] | [prev] | [next] | [standalone]

#1405796 — Re: [PATCH 4/5] iommu/rockchip: add ARM64 cache flush operation for iommu

Catalin, Robin,

On 2016年05月23日 21:35, Catalin Marinas wrote:
> On Mon, May 23, 2016 at 11:44:14AM +0100, Robin Murphy wrote:
>> On 23/05/16 02:37, Shunqian Zheng wrote:
>>> From: Simon <xxm@rock-chips.com>
>>>
>>> Signed-off-by: Simon <xxm@rock-chips.com>
>>> ---
>>>   drivers/iommu/rockchip-iommu.c | 4 ++++
>>>   1 file changed, 4 insertions(+)
>>>
>>> diff --git a/drivers/iommu/rockchip-iommu.c b/drivers/iommu/rockchip-iommu.c
>>> index 043d18c..1741b65 100644
>>> --- a/drivers/iommu/rockchip-iommu.c
>>> +++ b/drivers/iommu/rockchip-iommu.c
>>> @@ -95,12 +95,16 @@ struct rk_iommu {
>>>
>>>   static inline void rk_table_flush(u32 *va, unsigned int count)
>>>   {
>>> +#if defined(CONFIG_ARM)
>>>   	phys_addr_t pa_start = virt_to_phys(va);
>>>   	phys_addr_t pa_end = virt_to_phys(va + count);
>>>   	size_t size = pa_end - pa_start;
>>>
>>>   	__cpuc_flush_dcache_area(va, size);
>>>   	outer_flush_range(pa_start, pa_end);
>>> +#elif defined(CONFIG_ARM64)
>>> +	__dma_flush_range(va, va + count);
>>> +#endif
>> Ugh, please don't use arch-private cache maintenance functions directly from
>> a driver. Allocating/mapping page tables to be read by the IOMMU is still
>> DMA, so using the DMA APIs is the correct way to manage them, *especially*
>> if it needs to work across multiple architectures.
It's easier for us if changing  the __dma_flush_range() to 
__flush_dcache_area() is acceptable here?

Thank you,
- shunqian
> I fully agree, these functions should not be used in drivers.

[toc] | [prev] | [next] | [standalone]

#1406022 — Re: [PATCH 4/5] iommu/rockchip: add ARM64 cache flush operation for iommu

On Tue, May 24, 2016 at 10:31:17AM +0800, Shunqian Zheng wrote:
> On 2016年05月23日 21:35, Catalin Marinas wrote:
> >On Mon, May 23, 2016 at 11:44:14AM +0100, Robin Murphy wrote:
> >>On 23/05/16 02:37, Shunqian Zheng wrote:
> >>>From: Simon <xxm@rock-chips.com>
> >>>
> >>>Signed-off-by: Simon <xxm@rock-chips.com>
> >>>---
> >>>  drivers/iommu/rockchip-iommu.c | 4 ++++
> >>>  1 file changed, 4 insertions(+)
> >>>
> >>>diff --git a/drivers/iommu/rockchip-iommu.c b/drivers/iommu/rockchip-iommu.c
> >>>index 043d18c..1741b65 100644
> >>>--- a/drivers/iommu/rockchip-iommu.c
> >>>+++ b/drivers/iommu/rockchip-iommu.c
> >>>@@ -95,12 +95,16 @@ struct rk_iommu {
> >>>
> >>>  static inline void rk_table_flush(u32 *va, unsigned int count)
> >>>  {
> >>>+#if defined(CONFIG_ARM)
> >>>  	phys_addr_t pa_start = virt_to_phys(va);
> >>>  	phys_addr_t pa_end = virt_to_phys(va + count);
> >>>  	size_t size = pa_end - pa_start;
> >>>
> >>>  	__cpuc_flush_dcache_area(va, size);
> >>>  	outer_flush_range(pa_start, pa_end);
> >>>+#elif defined(CONFIG_ARM64)
> >>>+	__dma_flush_range(va, va + count);
> >>>+#endif
> >>Ugh, please don't use arch-private cache maintenance functions directly from
> >>a driver. Allocating/mapping page tables to be read by the IOMMU is still
> >>DMA, so using the DMA APIs is the correct way to manage them, *especially*
> >>if it needs to work across multiple architectures.
> 
> It's easier for us if changing  the __dma_flush_range() to
> __flush_dcache_area() is acceptable here?

It's not really acceptable for arm64, nor for arm32. Please fix this
driver in a similar way to commit e3c971960fd4 ("iommu/tegra-smmu:
Convert to use DMA API").

The only place where we allowed __flush_dcache_area() is in the GICv3
driver and that's because it hasn't been wired as a platform device
(yet).

-- 
Catalin

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web