This patchset is to improve tlb flushing performance in iommu_map/unmap
for MediaTek IOMMU.
For iommu_map, currently MediaTek IOMMU use IO_PGTABLE_QUIRK_TLBI_ON_MAP
to do tlb_flush for each a memory chunk. this is so unnecessary. we could
improve it by tlb flushing one time at the end of iommu_map.
For iommu_unmap, currently we have already improve this performance by
gather. But the current gather should take care its granule size. if the
granule size is different, it will do tlb flush and gather again. Our HW
don't care about granule size. thus I gather the range in our file.
After this patchset, we could achieve only tlb flushing once in iommu_map
and iommu_unmap.
Regardless of sg, for each a segment, I did a simple test:
size = 20 * SZ_1M;
/* the worst case, all are 4k mapping. */
ret = iommu_map(domain, 0x5bb02000, 0x123f1000, size, IOMMU_READ);
iommu_unmap(domain, 0x5bb02000, size);
This is the comparing time(unit is us):
original-time after-improve
map-20M 59943 2347
unmap-20M 264 36
This patchset also flush tlb once in the iommu_map_sg case.
patch [1/7][2/7][3/7] are for map while the others are for unmap.
change note:
v4: a. base on v5.11-rc1.
b. Add a little helper _iommu_map.
c. Fix a build fail for tegra-gart.c. I didn't notice there is another place
call gart_iommu_sync_map.
d. Switch gather->end to the read end address("start + end - 1").
v3: https://lore.kernel.org/linux-iommu/20201216103607.23050-1-yong.wu@mediatek.com/#r
Refactor the unmap flow suggested by Robin.
v2: https://lore.kernel.org/linux-iommu/20201119061836.15238-1-yong.wu@mediatek.com/
Refactor all the code.
base on v5.10-rc1.
Yong Wu (7):
iommu: Move iotlb_sync_map out from __iommu_map
iommu: Add iova and size as parameters in iotlb_sync_map
iommu/mediatek: Add iotlb_sync_map to sync whole the iova range
iommu: Switch gather->end to the inclusive end
iommu/io-pgtable: Allow io_pgtable_tlb ops optional
iommu/mediatek: Gather iova in iommu_unmap to achieve tlb sync once
iommu/mediatek: Remove the tlb-ops for v7s
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 2 +-
drivers/iommu/iommu.c | 23 +++++++---
drivers/iommu/mtk_iommu.c | 47 +++++++++------------
drivers/iommu/tegra-gart.c | 7 ++-
include/linux/io-pgtable.h | 8 ++--
include/linux/iommu.h | 7 +--
6 files changed, 52 insertions(+), 42 deletions(-)
--
2.18.0
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
In the end of __iommu_map, It alway call iotlb_sync_map.
This patch moves iotlb_sync_map out from __iommu_map since it is
unnecessary to call this for each sg segment especially iotlb_sync_map
is flush tlb all currently. Add a little helper _iommu_map for this.
Signed-off-by: Yong Wu <yong.wu@mediatek.com>
Reviewed-by: Robin Murphy <robin.murphy@arm.com>
---
drivers/iommu/iommu.c | 23 ++++++++++++++++++-----
1 file changed, 18 insertions(+), 5 deletions(-)
@@ -2426,9 +2426,6 @@ static int __iommu_map(struct iommu_domain *domain, unsigned long iova,size-=pgsize;}-if(ops->iotlb_sync_map)-ops->iotlb_sync_map(domain);-/* unroll mapping in case something went wrong */if(ret)iommu_unmap(domain,orig_iova,orig_size-size);
@@ -2438,18 +2435,31 @@ static int __iommu_map(struct iommu_domain *domain, unsigned long iova,returnret;}+staticint_iommu_map(structiommu_domain*domain,unsignedlongiova,+phys_addr_tpaddr,size_tsize,intprot,gfp_tgfp)+{+conststructiommu_ops*ops=domain->ops;+intret;++ret=__iommu_map(domain,iova,paddr,size,prot,GFP_KERNEL);+if(ret==0&&ops->iotlb_sync_map)+ops->iotlb_sync_map(domain);++returnret;+}+intiommu_map(structiommu_domain*domain,unsignedlongiova,phys_addr_tpaddr,size_tsize,intprot){might_sleep();-return__iommu_map(domain,iova,paddr,size,prot,GFP_KERNEL);+return_iommu_map(domain,iova,paddr,size,prot,GFP_KERNEL);}EXPORT_SYMBOL_GPL(iommu_map);intiommu_map_atomic(structiommu_domain*domain,unsignedlongiova,phys_addr_tpaddr,size_tsize,intprot){-return__iommu_map(domain,iova,paddr,size,prot,GFP_ATOMIC);+return_iommu_map(domain,iova,paddr,size,prot,GFP_ATOMIC);}EXPORT_SYMBOL_GPL(iommu_map_atomic);
Specifically the above bug means we drop the "GFP_ATOMIC" here.
It means we trigger a warning, like this (on a downstream kernel with
the patch backported):
BUG: sleeping function called from invalid context at mm/page_alloc.c:4726
in_atomic(): 1, irqs_disabled(): 0, non_block: 0, pid: 9, name: ksoftirqd/0
CPU: 0 PID: 9 Comm: ksoftirqd/0 Not tainted 5.4.93-12508-gc10c93e28e39 #1
Call trace:
dump_backtrace+0x0/0x154
show_stack+0x20/0x2c
dump_stack+0xa0/0xfc
___might_sleep+0x11c/0x12c
__might_sleep+0x50/0x84
__alloc_pages_nodemask+0xf8/0x2bc
__arm_lpae_alloc_pages+0x48/0x1b4
__arm_lpae_map+0x124/0x274
__arm_lpae_map+0x1cc/0x274
arm_lpae_map+0x140/0x170
arm_smmu_map+0x78/0xbc
__iommu_map+0xd4/0x210
_iommu_map+0x4c/0x84
iommu_map_atomic+0x44/0x58
__iommu_dma_map+0x8c/0xc4
iommu_dma_map_page+0xac/0xf0
---
A quick (but not very tested) fix at:
https://lore.kernel.org/r/20210201170611.1.I64a7b62579287d668d7c89e105dcedf45d641063@changeid/
-Doug
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
This patch allows io_pgtable_tlb ops could be null since the IOMMU drivers
may use the tlb ops from iommu framework.
Signed-off-by: Yong Wu <yong.wu@mediatek.com>
---
include/linux/io-pgtable.h | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
iotlb_sync_map allow IOMMU drivers tlb sync after completing the whole
mapping. This patch adds iova and size as the parameters in it. then the
IOMMU driver could flush tlb with the whole range once after iova mapping
to improve performance.
Signed-off-by: Yong Wu <yong.wu@mediatek.com>
Reviewed-by: Robin Murphy <robin.murphy@arm.com>
---
drivers/iommu/iommu.c | 4 ++--
drivers/iommu/tegra-gart.c | 7 +++++--
include/linux/iommu.h | 3 ++-
3 files changed, 9 insertions(+), 5 deletions(-)
Remove IO_PGTABLE_QUIRK_TLBI_ON_MAP to avoid tlb sync for each a small
chunk memory, Use the new iotlb_sync_map to tlb_sync once for whole the
iova range of iommu_map.
Signed-off-by: Yong Wu <yong.wu@mediatek.com>
Reviewed-by: Robin Murphy <robin.murphy@arm.com>
---
drivers/iommu/mtk_iommu.c | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
Currently gather->end is "unsigned long" which may be overflow in
arch32 in the corner case: 0xfff00000 + 0x100000(iova + size).
Although it doesn't affect the size(end - start), it affects the checking
"gather->end < end"
This patch changes this "end" to the real end address
(end = start + size - 1). Correspondingly, update the length to
"end - start + 1".
Fixes: a7d20dc19d9e ("iommu: Introduce struct iommu_iotlb_gather for batching TLB flushes")
Signed-off-by: Yong Wu <yong.wu@mediatek.com>
---
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 2 +-
drivers/iommu/mtk_iommu.c | 2 +-
drivers/iommu/tegra-gart.c | 2 +-
include/linux/iommu.h | 4 ++--
4 files changed, 5 insertions(+), 5 deletions(-)
From: Robin Murphy <robin.murphy@arm.com> Date: 2021-01-18 18:52:12
On 2021-01-07 12:29, Yong Wu wrote:
quoted hunk
Currently gather->end is "unsigned long" which may be overflow in
arch32 in the corner case: 0xfff00000 + 0x100000(iova + size).
Although it doesn't affect the size(end - start), it affects the checking
"gather->end < end"
This patch changes this "end" to the real end address
(end = start + size - 1). Correspondingly, update the length to
"end - start + 1".
Fixes: a7d20dc19d9e ("iommu: Introduce struct iommu_iotlb_gather for batching TLB flushes")
Signed-off-by: Yong Wu <yong.wu@mediatek.com>
---
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 2 +-
drivers/iommu/mtk_iommu.c | 2 +-
drivers/iommu/tegra-gart.c | 2 +-
include/linux/iommu.h | 4 ++--
4 files changed, 5 insertions(+), 5 deletions(-)
TBH I don't think there's any need to bother doing precise calculations
on effectively-uninitialised data (this driver doesn't even do
address-based invalidation, let alone use the gather mechanism). In fact
it might make sense to flip things around and define gart_iommu_sync_map
in terms of gart_iommu_sync now just so there's one less unused argument
to make up. However we can always do cleanup on top, and right now I'm
more interested in getting these changes landed, so either way,
Reviewed-by: Robin Murphy <robin.murphy@arm.com>
Until now, we have already used the tlb operations from iommu framework,
then the tlb operations for v7s can be removed.
Correspondingly, Switch the paramenter "cookie" to the internal structure.
Signed-off-by: Yong Wu <yong.wu@mediatek.com>
---
drivers/iommu/mtk_iommu.c | 27 ++++-----------------------
1 file changed, 4 insertions(+), 23 deletions(-)
@@ -219,7 +217,7 @@ static void mtk_iommu_tlb_flush_range_sync(unsigned long iova, size_t size,if(ret){dev_warn(data->dev,"Partial TLB flush timed out, falling back to full flush\n");-mtk_iommu_tlb_flush_all(cookie);+mtk_iommu_tlb_flush_all(data);}/* Clear the CPE status */writel_relaxed(0,data->base+REG_MMU_CPE_DONE);
From: Robin Murphy <robin.murphy@arm.com> Date: 2021-01-18 18:49:27
On 2021-01-07 12:29, Yong Wu wrote:
Until now, we have already used the tlb operations from iommu framework,
then the tlb operations for v7s can be removed.
Correspondingly, Switch the paramenter "cookie" to the internal structure.
@@ -219,7 +217,7 @@ static void mtk_iommu_tlb_flush_range_sync(unsigned long iova, size_t size,if(ret){dev_warn(data->dev,"Partial TLB flush timed out, falling back to full flush\n");-mtk_iommu_tlb_flush_all(cookie);+mtk_iommu_tlb_flush_all(data);}/* Clear the CPE status */writel_relaxed(0,data->base+REG_MMU_CPE_DONE);
In current iommu_unmap, this code is:
iommu_iotlb_gather_init(&iotlb_gather);
ret = __iommu_unmap(domain, iova, size, &iotlb_gather);
iommu_iotlb_sync(domain, &iotlb_gather);
We could gather the whole iova range in __iommu_unmap, and then do tlb
synchronization in the iommu_iotlb_sync.
This patch implement this, Gather the range in mtk_iommu_unmap.
then iommu_iotlb_sync call tlb synchronization for the gathered iova range.
we don't call iommu_iotlb_gather_add_page since our tlb synchronization
could be regardless of granule size.
In this way, gather->start is impossible ULONG_MAX, remove the checking.
This patch aims to do tlb synchronization *once* in the iommu_unmap.
Signed-off-by: Yong Wu <yong.wu@mediatek.com>
---
drivers/iommu/mtk_iommu.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
From: Robin Murphy <robin.murphy@arm.com> Date: 2021-01-18 18:50:40
On 2021-01-07 12:29, Yong Wu wrote:
In current iommu_unmap, this code is:
iommu_iotlb_gather_init(&iotlb_gather);
ret = __iommu_unmap(domain, iova, size, &iotlb_gather);
iommu_iotlb_sync(domain, &iotlb_gather);
We could gather the whole iova range in __iommu_unmap, and then do tlb
synchronization in the iommu_iotlb_sync.
This patch implement this, Gather the range in mtk_iommu_unmap.
then iommu_iotlb_sync call tlb synchronization for the gathered iova range.
we don't call iommu_iotlb_gather_add_page since our tlb synchronization
could be regardless of granule size.
In this way, gather->start is impossible ULONG_MAX, remove the checking.
This patch aims to do tlb synchronization *once* in the iommu_unmap.
From: Will Deacon <will@kernel.org> Date: 2021-01-22 20:19:30
On Thu, Jan 07, 2021 at 08:29:02PM +0800, Yong Wu wrote:
This patchset is to improve tlb flushing performance in iommu_map/unmap
for MediaTek IOMMU.
For iommu_map, currently MediaTek IOMMU use IO_PGTABLE_QUIRK_TLBI_ON_MAP
to do tlb_flush for each a memory chunk. this is so unnecessary. we could
improve it by tlb flushing one time at the end of iommu_map.
For iommu_unmap, currently we have already improve this performance by
gather. But the current gather should take care its granule size. if the
granule size is different, it will do tlb flush and gather again. Our HW
don't care about granule size. thus I gather the range in our file.
After this patchset, we could achieve only tlb flushing once in iommu_map
and iommu_unmap.
Regardless of sg, for each a segment, I did a simple test:
size = 20 * SZ_1M;
/* the worst case, all are 4k mapping. */
ret = iommu_map(domain, 0x5bb02000, 0x123f1000, size, IOMMU_READ);
iommu_unmap(domain, 0x5bb02000, size);
This is the comparing time(unit is us):
original-time after-improve
map-20M 59943 2347
unmap-20M 264 36
This patchset also flush tlb once in the iommu_map_sg case.
patch [1/7][2/7][3/7] are for map while the others are for unmap.
change note:
v4: a. base on v5.11-rc1.
b. Add a little helper _iommu_map.
c. Fix a build fail for tegra-gart.c. I didn't notice there is another place
call gart_iommu_sync_map.
d. Switch gather->end to the read end address("start + end - 1").
v3: https://lore.kernel.org/linux-iommu/20201216103607.23050-1-yong.wu@mediatek.com/#r
Refactor the unmap flow suggested by Robin.
v2: https://lore.kernel.org/linux-iommu/20201119061836.15238-1-yong.wu@mediatek.com/
Refactor all the code.
base on v5.10-rc1.
Yong Wu (7):
iommu: Move iotlb_sync_map out from __iommu_map
iommu: Add iova and size as parameters in iotlb_sync_map
iommu/mediatek: Add iotlb_sync_map to sync whole the iova range
iommu: Switch gather->end to the inclusive end
iommu/io-pgtable: Allow io_pgtable_tlb ops optional
iommu/mediatek: Gather iova in iommu_unmap to achieve tlb sync once
iommu/mediatek: Remove the tlb-ops for v7s
For the series:
Acked-by: Will Deacon <will@kernel.org>
Joerg -- how would you like to handle merging this? I suppose either you
could host a separate branch that I could merge if needed, or I could
include this in my pull to you, or something else.
Please let me know what you prefer,
Cheers,
Will
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
From: Will Deacon <will@kernel.org> Date: 2021-01-27 13:23:20
On Thu, 7 Jan 2021 20:29:02 +0800, Yong Wu wrote:
This patchset is to improve tlb flushing performance in iommu_map/unmap
for MediaTek IOMMU.
For iommu_map, currently MediaTek IOMMU use IO_PGTABLE_QUIRK_TLBI_ON_MAP
to do tlb_flush for each a memory chunk. this is so unnecessary. we could
improve it by tlb flushing one time at the end of iommu_map.
[...]