[PATCH] iommu/io-pgtable-arm: Allow non-coherent masters to use system cache
commit ecd7274fb4cd ("iommu: Remove unused IOMMU_SYS_CACHE_ONLY flag")
removed unused IOMMU_SYS_CACHE_ONLY prot flag and along with it went
the memory type setting required for the non-coherent masters to use
system cache. Now that system cache support for GPU is added, we will
need to mark the memory as normal sys-cached for GPU to use system cache.
Without this, the system cache lines are not allocated for GPU. We use
the IO_PGTABLE_QUIRK_ARM_OUTER_WBWA quirk instead of a page protection
flag as the flag cannot be exposed via DMA api because of no in-tree
users.
Signed-off-by: Sai Prakash Ranjan
---
drivers/iommu/io-pgtable-arm.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/iommu/io-pgtable-arm.c b/drivers/iommu/io-pgtable-arm.c
index 7c9ea9d7874a..3fb7de8304a2 100644
--- a/drivers/iommu/io-pgtable-arm.c
+++ b/drivers/iommu/io-pgtable-arm.c
@@ -415,6 +415,9 @@ static arm_lpae_iopte arm_lpae_prot_to_pte(struct
arm_lpae_io_pgtable *data,
else if (prot & IOMMU_CACHE)
pte |= (ARM_LPAE_MAIR_ATTR_IDX_CACHE
<< ARM_LPAE_PTE_ATTRINDX_SHIFT);
+ else if (data->iop.cfg.quirks & IO_PGTABLE_QUIRK_ARM_OUTER_WBWA)
+ pte |= (ARM_LPAE_MAIR_ATTR_IDX_INC_OCACHE
+ << ARM_LPAE_PTE_ATTRINDX_SHIFT);
}
if (prot & IOMMU_CACHE)
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 1/7] iommu/io-pgtable: Introduce dynamic io-pgtable fmt registration
Hi "Isaac,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on iommu/next]
[also build test WARNING on soc/for-next xlnx/master linus/master v5.10
next-20201223]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url:
https://github.com/0day-ci/linux/commits/Isaac-J-Manjarres/iommu-Permit-modular-builds-of-io-pgtable-drivers/20201222-085121
base: https://git.kernel.org/pub/scm/linux/kernel/git/joro/iommu.git next
config: alpha-randconfig-r023-20201221 (attached as .config)
compiler: alpha-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
wget
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O
~/bin/make.cross
chmod +x ~/bin/make.cross
#
https://github.com/0day-ci/linux/commit/9cc3cfc5f79e5c9072aea2218dd9080227933caa
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review
Isaac-J-Manjarres/iommu-Permit-modular-builds-of-io-pgtable-drivers/20201222-085121
git checkout 9cc3cfc5f79e5c9072aea2218dd9080227933caa
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross
ARCH=alpha
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot
All warnings (new ones prefixed by >>):
In file included from include/linux/printk.h:7,
from include/linux/kernel.h:16,
from include/asm-generic/bug.h:20,
from arch/alpha/include/asm/bug.h:23,
from include/linux/bug.h:5,
from include/linux/scatterlist.h:7,
from include/linux/iommu.h:10,
from include/linux/io-pgtable.h:6,
from drivers/iommu/io-pgtable-arm.c:14:
drivers/iommu/io-pgtable-arm.c: In function 'arm_lpae_init':
>> include/linux/kern_levels.h:5:18: warning: format '%d' expects a matching
>> 'int' argument [-Wformat=]
5 | #define KERN_SOH "\001" /* ASCII Start Of Header */
| ^~
include/linux/kern_levels.h:11:18: note: in expansion of macro 'KERN_SOH'
11 | #define KERN_ERR KERN_SOH "3" /* error conditions */
| ^~~~
include/linux/printk.h:343:9: note: in expansion of macro 'KERN_ERR'
343 | printk(KERN_ERR pr_fmt(fmt), ##__VA_ARGS__)
| ^~~~
drivers/iommu/io-pgtable-arm.c:1257:4: note: in expansion of macro 'pr_err'
1257 |pr_err("Failed to register ARM LPAE fmt: %d\n");
|^~
drivers/iommu/io-pgtable-arm.c:1257:46: note: format string is defined here
1257 |pr_err("Failed to register ARM LPAE fmt: %d\n");
| ~^
| |
| int
vim +5 include/linux/kern_levels.h
314ba3520e513a7 Joe Perches 2012-07-30 4
04d2c8c83d0e3ac Joe Perches 2012-07-30 @5 #define KERN_SOH "\001"
/* ASCII Start Of Header */
04d2c8c83d0e3ac Joe Perches 2012-07-30 6 #define KERN_SOH_ASCII '\001'
04d2c8c83d0e3ac Joe Perches 2012-07-30 7
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org
.config.gz
Description: application/gzip
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 1/7] iommu/io-pgtable: Introduce dynamic io-pgtable fmt registration
Hi Isaac,
On 22.12.20 2:44, Isaac J. Manjarres wrote:
The io-pgtable code constructs an array of init functions for each
page table format at compile time. This is not ideal, as this
increases the footprint of the io-pgtable code, as well as prevents
io-pgtable formats from being built as kernel modules.
In preparation for modularizing the io-pgtable formats, switch to a
dynamic registration scheme, where each io-pgtable format can register
their init functions with the io-pgtable code at boot or module
insertion time.
Signed-off-by: Isaac J. Manjarres
---
drivers/iommu/io-pgtable-arm-v7s.c | 34 +-
drivers/iommu/io-pgtable-arm.c | 90 ++--
drivers/iommu/io-pgtable.c | 94 --
include/linux/io-pgtable.h | 51 +
4 files changed, 209 insertions(+), 60 deletions(-)
diff --git a/drivers/iommu/io-pgtable-arm-v7s.c
b/drivers/iommu/io-pgtable-arm-v7s.c
index 1d92ac9..89aad2f 100644
--- a/drivers/iommu/io-pgtable-arm-v7s.c
+++ b/drivers/iommu/io-pgtable-arm-v7s.c
@@ -28,6 +28,7 @@
[..]
+static int __init arm_lpae_init(void)
+{
+ int ret, i;
+
+ for (i = 0; i < ARRAY_SIZE(io_pgtable_arm_lpae_init_fns); i++) {
+ ret = io_pgtable_ops_register(_pgtable_arm_lpae_init_fns[i]);
+ if (ret < 0) {
+ pr_err("Failed to register ARM LPAE fmt: %d\n");
I guess we want to print the format here?
Thanks,
Georgi
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 1/7] iommu/io-pgtable: Introduce dynamic io-pgtable fmt registration
On 2020-12-23 05:44, Robin Murphy wrote:
On 2020-12-22 19:54, isa...@codeaurora.org wrote:
On 2020-12-22 11:27, Robin Murphy wrote:
On 2020-12-22 00:44, Isaac J. Manjarres wrote:
The io-pgtable code constructs an array of init functions for each
page table format at compile time. This is not ideal, as this
increases the footprint of the io-pgtable code, as well as prevents
io-pgtable formats from being built as kernel modules.
In preparation for modularizing the io-pgtable formats, switch to a
dynamic registration scheme, where each io-pgtable format can
register
their init functions with the io-pgtable code at boot or module
insertion time.
Signed-off-by: Isaac J. Manjarres
---
drivers/iommu/io-pgtable-arm-v7s.c | 34 +-
drivers/iommu/io-pgtable-arm.c | 90
++--
drivers/iommu/io-pgtable.c | 94
--
include/linux/io-pgtable.h | 51 +
4 files changed, 209 insertions(+), 60 deletions(-)
diff --git a/drivers/iommu/io-pgtable-arm-v7s.c
b/drivers/iommu/io-pgtable-arm-v7s.c
index 1d92ac9..89aad2f 100644
--- a/drivers/iommu/io-pgtable-arm-v7s.c
+++ b/drivers/iommu/io-pgtable-arm-v7s.c
@@ -28,6 +28,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -835,7 +836,8 @@ static struct io_pgtable
*arm_v7s_alloc_pgtable(struct io_pgtable_cfg *cfg,
return NULL;
}
-struct io_pgtable_init_fns io_pgtable_arm_v7s_init_fns = {
+static struct io_pgtable_init_fns io_pgtable_arm_v7s_init_fns = {
+ .fmt = ARM_V7S,
.alloc = arm_v7s_alloc_pgtable,
.free = arm_v7s_free_pgtable,
};
@@ -982,5 +984,33 @@ static int __init arm_v7s_do_selftests(void)
pr_info("self test ok\n");
return 0;
}
-subsys_initcall(arm_v7s_do_selftests);
+#else
+static int arm_v7s_do_selftests(void)
+{
+ return 0;
+}
#endif
+
+static int __init arm_v7s_init(void)
+{
+ int ret;
+
+ ret = io_pgtable_ops_register(_pgtable_arm_v7s_init_fns);
+ if (ret < 0) {
+ pr_err("Failed to register ARM V7S format\n");
Super-nit: I think "v7s" should probably be lowercase there. Also
general consistency WRT to showing the error code and whether or not
to abbreviate "format" would be nice.
Ok, I can fix this accordingly.
+ return ret;
+ }
+
+ ret = arm_v7s_do_selftests();
+ if (ret < 0)
+ io_pgtable_ops_unregister(_pgtable_arm_v7s_init_fns);
+
+ return ret;
+}
+core_initcall(arm_v7s_init);
+
+static void __exit arm_v7s_exit(void)
+{
+ io_pgtable_ops_unregister(_pgtable_arm_v7s_init_fns);
+}
+module_exit(arm_v7s_exit);
diff --git a/drivers/iommu/io-pgtable-arm.c
b/drivers/iommu/io-pgtable-arm.c
index 87def58..ff0ea2f 100644
--- a/drivers/iommu/io-pgtable-arm.c
+++ b/drivers/iommu/io-pgtable-arm.c
@@ -13,6 +13,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -1043,29 +1044,32 @@ arm_mali_lpae_alloc_pgtable(struct
io_pgtable_cfg *cfg, void *cookie)
return NULL;
}
-struct io_pgtable_init_fns io_pgtable_arm_64_lpae_s1_init_fns = {
- .alloc = arm_64_lpae_alloc_pgtable_s1,
- .free = arm_lpae_free_pgtable,
-};
-
-struct io_pgtable_init_fns io_pgtable_arm_64_lpae_s2_init_fns = {
- .alloc = arm_64_lpae_alloc_pgtable_s2,
- .free = arm_lpae_free_pgtable,
-};
-
-struct io_pgtable_init_fns io_pgtable_arm_32_lpae_s1_init_fns = {
- .alloc = arm_32_lpae_alloc_pgtable_s1,
- .free = arm_lpae_free_pgtable,
-};
-
-struct io_pgtable_init_fns io_pgtable_arm_32_lpae_s2_init_fns = {
- .alloc = arm_32_lpae_alloc_pgtable_s2,
- .free = arm_lpae_free_pgtable,
-};
-
-struct io_pgtable_init_fns io_pgtable_arm_mali_lpae_init_fns = {
- .alloc = arm_mali_lpae_alloc_pgtable,
- .free = arm_lpae_free_pgtable,
+static struct io_pgtable_init_fns io_pgtable_arm_lpae_init_fns[] =
{
+ {
+ .fmt = ARM_32_LPAE_S1,
+ .alloc = arm_32_lpae_alloc_pgtable_s1,
+ .free = arm_lpae_free_pgtable,
+ },
+ {
+ .fmt = ARM_32_LPAE_S2,
+ .alloc = arm_32_lpae_alloc_pgtable_s2,
+ .free = arm_lpae_free_pgtable,
+ },
+ {
+ .fmt = ARM_64_LPAE_S1,
+ .alloc = arm_64_lpae_alloc_pgtable_s1,
+ .free = arm_lpae_free_pgtable,
+ },
+ {
+ .fmt = ARM_64_LPAE_S2,
+ .alloc = arm_64_lpae_alloc_pgtable_s2,
+ .free = arm_lpae_free_pgtable,
+ },
+ {
+ .fmt = ARM_MALI_LPAE,
+ .alloc = arm_mali_lpae_alloc_pgtable,
+ .free = arm_lpae_free_pgtable,
+ },
};
#ifdef CONFIG_IOMMU_IO_PGTABLE_LPAE_SELFTEST
@@ -1250,5 +1254,43 @@ static int __init arm_lpae_do_selftests(void)
pr_info("selftest: completed with %d PASS %d FAIL\n", pass,
fail);
return fail ? -EFAULT : 0;
}
-subsys_initcall(arm_lpae_do_selftests);
+#else
+static int __init arm_lpae_do_selftests(void)
+{
+ return
[RFC PATCH 1/2] dt-bindings: iommu: add bindings for sprd iommu
From: Chunyan Zhang
This patch only adds bindings to support display iommu, support for others
would be added once finished tests with those devices, such as Image
codec(jpeg) processor, a few signal processors, including VSP(video),
GSP(graphic), ISP(image), and camera CPP, etc.
Signed-off-by: Chunyan Zhang
---
.../devicetree/bindings/iommu/sprd,iommu.yaml | 44 +++
1 file changed, 44 insertions(+)
create mode 100644 Documentation/devicetree/bindings/iommu/sprd,iommu.yaml
diff --git a/Documentation/devicetree/bindings/iommu/sprd,iommu.yaml
b/Documentation/devicetree/bindings/iommu/sprd,iommu.yaml
new file mode 100644
index ..4d9a578a7cc9
--- /dev/null
+++ b/Documentation/devicetree/bindings/iommu/sprd,iommu.yaml
@@ -0,0 +1,44 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+# Copyright 2020 Unisoc Inc.
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iommu/sprd,iommu.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Unisoc IOMMU and Multi-media MMU
+
+maintainers:
+ - Chunyan Zhang
+
+properties:
+ compatible:
+enum:
+ - sprd,iommu-disp
+
+ reg:
+maxItems: 1
+
+ "#iommu-cells":
+const: 0
+description:
+ Unisoc IOMMUs are all single-master IOMMU devices, therefore no
+ additional information needs to associate with its master device.
+ Please refer to the generic bindings document for more details,
+ Documentation/devicetree/bindings/iommu/iommu.txt
+
+required:
+ - compatible
+ - reg
+ - "#iommu-cells"
+
+additionalProperties: false
+
+examples:
+ - |
+iommu_disp: iommu@6300 {
+ compatible = "sprd,iommu-disp";
+ reg = <0x6300 0x880>;
+ #iommu-cells = <0>;
+};
+
+...
--
2.25.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 1/7] iommu/io-pgtable: Introduce dynamic io-pgtable fmt registration
On 2020-12-22 19:54, isa...@codeaurora.org wrote:
On 2020-12-22 11:27, Robin Murphy wrote:
On 2020-12-22 00:44, Isaac J. Manjarres wrote:
The io-pgtable code constructs an array of init functions for each
page table format at compile time. This is not ideal, as this
increases the footprint of the io-pgtable code, as well as prevents
io-pgtable formats from being built as kernel modules.
In preparation for modularizing the io-pgtable formats, switch to a
dynamic registration scheme, where each io-pgtable format can register
their init functions with the io-pgtable code at boot or module
insertion time.
Signed-off-by: Isaac J. Manjarres
---
drivers/iommu/io-pgtable-arm-v7s.c | 34 +-
drivers/iommu/io-pgtable-arm.c | 90
++--
drivers/iommu/io-pgtable.c | 94
--
include/linux/io-pgtable.h | 51 +
4 files changed, 209 insertions(+), 60 deletions(-)
diff --git a/drivers/iommu/io-pgtable-arm-v7s.c
b/drivers/iommu/io-pgtable-arm-v7s.c
index 1d92ac9..89aad2f 100644
--- a/drivers/iommu/io-pgtable-arm-v7s.c
+++ b/drivers/iommu/io-pgtable-arm-v7s.c
@@ -28,6 +28,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -835,7 +836,8 @@ static struct io_pgtable
*arm_v7s_alloc_pgtable(struct io_pgtable_cfg *cfg,
return NULL;
}
-struct io_pgtable_init_fns io_pgtable_arm_v7s_init_fns = {
+static struct io_pgtable_init_fns io_pgtable_arm_v7s_init_fns = {
+ .fmt = ARM_V7S,
.alloc = arm_v7s_alloc_pgtable,
.free = arm_v7s_free_pgtable,
};
@@ -982,5 +984,33 @@ static int __init arm_v7s_do_selftests(void)
pr_info("self test ok\n");
return 0;
}
-subsys_initcall(arm_v7s_do_selftests);
+#else
+static int arm_v7s_do_selftests(void)
+{
+ return 0;
+}
#endif
+
+static int __init arm_v7s_init(void)
+{
+ int ret;
+
+ ret = io_pgtable_ops_register(_pgtable_arm_v7s_init_fns);
+ if (ret < 0) {
+ pr_err("Failed to register ARM V7S format\n");
Super-nit: I think "v7s" should probably be lowercase there. Also
general consistency WRT to showing the error code and whether or not
to abbreviate "format" would be nice.
Ok, I can fix this accordingly.
+ return ret;
+ }
+
+ ret = arm_v7s_do_selftests();
+ if (ret < 0)
+ io_pgtable_ops_unregister(_pgtable_arm_v7s_init_fns);
+
+ return ret;
+}
+core_initcall(arm_v7s_init);
+
+static void __exit arm_v7s_exit(void)
+{
+ io_pgtable_ops_unregister(_pgtable_arm_v7s_init_fns);
+}
+module_exit(arm_v7s_exit);
diff --git a/drivers/iommu/io-pgtable-arm.c
b/drivers/iommu/io-pgtable-arm.c
index 87def58..ff0ea2f 100644
--- a/drivers/iommu/io-pgtable-arm.c
+++ b/drivers/iommu/io-pgtable-arm.c
@@ -13,6 +13,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -1043,29 +1044,32 @@ arm_mali_lpae_alloc_pgtable(struct
io_pgtable_cfg *cfg, void *cookie)
return NULL;
}
-struct io_pgtable_init_fns io_pgtable_arm_64_lpae_s1_init_fns = {
- .alloc = arm_64_lpae_alloc_pgtable_s1,
- .free = arm_lpae_free_pgtable,
-};
-
-struct io_pgtable_init_fns io_pgtable_arm_64_lpae_s2_init_fns = {
- .alloc = arm_64_lpae_alloc_pgtable_s2,
- .free = arm_lpae_free_pgtable,
-};
-
-struct io_pgtable_init_fns io_pgtable_arm_32_lpae_s1_init_fns = {
- .alloc = arm_32_lpae_alloc_pgtable_s1,
- .free = arm_lpae_free_pgtable,
-};
-
-struct io_pgtable_init_fns io_pgtable_arm_32_lpae_s2_init_fns = {
- .alloc = arm_32_lpae_alloc_pgtable_s2,
- .free = arm_lpae_free_pgtable,
-};
-
-struct io_pgtable_init_fns io_pgtable_arm_mali_lpae_init_fns = {
- .alloc = arm_mali_lpae_alloc_pgtable,
- .free = arm_lpae_free_pgtable,
+static struct io_pgtable_init_fns io_pgtable_arm_lpae_init_fns[] = {
+ {
+ .fmt = ARM_32_LPAE_S1,
+ .alloc = arm_32_lpae_alloc_pgtable_s1,
+ .free = arm_lpae_free_pgtable,
+ },
+ {
+ .fmt = ARM_32_LPAE_S2,
+ .alloc = arm_32_lpae_alloc_pgtable_s2,
+ .free = arm_lpae_free_pgtable,
+ },
+ {
+ .fmt = ARM_64_LPAE_S1,
+ .alloc = arm_64_lpae_alloc_pgtable_s1,
+ .free = arm_lpae_free_pgtable,
+ },
+ {
+ .fmt = ARM_64_LPAE_S2,
+ .alloc = arm_64_lpae_alloc_pgtable_s2,
+ .free = arm_lpae_free_pgtable,
+ },
+ {
+ .fmt = ARM_MALI_LPAE,
+ .alloc = arm_mali_lpae_alloc_pgtable,
+ .free = arm_lpae_free_pgtable,
+ },
};
#ifdef CONFIG_IOMMU_IO_PGTABLE_LPAE_SELFTEST
@@ -1250,5 +1254,43 @@ static int __init arm_lpae_do_selftests(void)
pr_info("selftest: completed with %d PASS %d FAIL\n", pass, fail);
return fail ? -EFAULT : 0;
}
-subsys_initcall(arm_lpae_do_selftests);
+#else
+static int __init arm_lpae_do_selftests(void)
+{
+ return 0;
+}
#endif
+
+static int __init
Re: [PATCH v2 3/7] iommu/arm-smmu: Add dependency on io-pgtable format modules
On 2020-12-22 19:49, isa...@codeaurora.org wrote:
On 2020-12-22 11:27, Robin Murphy wrote:
On 2020-12-22 00:44, Isaac J. Manjarres wrote:
The SMMU driver depends on the availability of the ARM LPAE and
ARM V7S io-pgtable format code to work properly. In preparation
Nit: we don't really depend on v7s - we *can* use it if it's
available, address constraints are suitable, and the SMMU
implementation actually supports it (many don't), but we can still
quite happily not use it even so. LPAE is mandatory in the
architecture so that's our only hard requirement, embodied in the
kconfig select.
This does mean there may technically still be a corner case involving
ARM_SMMU=y and IO_PGTABLE_ARM_V7S=m, but at worst it's now a runtime
failure rather than a build error, so unless and until anyone
demonstrates that it actually matters I don't feel particularly
inclined to give it much thought.
Robin.
Okay, I'll fix up the commit message, as well as the code, so that it
only depends on io-pgtable-arm.
Well, IIUC it would make sense to keep the softdep for when the v7s
module *is* present; I just wanted to clarify that it's more of a
nice-to-have rather than a necessity.
Robin.
Thanks,
Isaac
for having the io-pgtable formats as modules, add a "pre"
dependency with MODULE_SOFTDEP() to ensure that the io-pgtable
format modules are loaded before loading the ARM SMMU driver module.
Signed-off-by: Isaac J. Manjarres
---
drivers/iommu/arm/arm-smmu/arm-smmu.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.c
b/drivers/iommu/arm/arm-smmu/arm-smmu.c
index d8c6bfd..a72649f 100644
--- a/drivers/iommu/arm/arm-smmu/arm-smmu.c
+++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c
@@ -2351,3 +2351,4 @@ MODULE_DESCRIPTION("IOMMU API for ARM
architected SMMU implementations");
MODULE_AUTHOR("Will Deacon ");
MODULE_ALIAS("platform:arm-smmu");
MODULE_LICENSE("GPL v2");
+MODULE_SOFTDEP("pre: io-pgtable-arm io-pgtable-arm-v7s");
___
linux-arm-kernel mailing list
linux-arm-ker...@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v3 6/7] iommu/mediatek: Gather iova in iommu_unmap to achieve tlb sync once
On 2020-12-23 08:56, Tomasz Figa wrote:
On Wed, Dec 16, 2020 at 06:36:06PM +0800, Yong Wu wrote:
In current iommu_unmap, this code is:
iommu_iotlb_gather_init(_gather);
ret = __iommu_unmap(domain, iova, size, _gather);
iommu_iotlb_sync(domain, _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
---
drivers/iommu/mtk_iommu.c | 8 +---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
index db7d43adb06b..89cec51405cd 100644
--- a/drivers/iommu/mtk_iommu.c
+++ b/drivers/iommu/mtk_iommu.c
@@ -506,7 +506,12 @@ static size_t mtk_iommu_unmap(struct iommu_domain *domain,
struct iommu_iotlb_gather *gather)
{
struct mtk_iommu_domain *dom = to_mtk_domain(domain);
+ unsigned long long end = iova + size;
+ if (gather->start > iova)
+ gather->start = iova;
+ if (gather->end < end)
+ gather->end = end;
I don't know how common the case is, but what happens if
gather->start...gather->end is a disjoint range from iova...end? E.g.
| gather | ..XXX... | iova |
| | | |
gather->start | iova |
gather->end end
We would also end up invalidating the TLB for the XXX area, which could
affect the performance.
Take a closer look at iommu_unmap() - the gather data is scoped to each
individual call, so that can't possibly happen.
Also, why is the existing code in __arm_v7s_unmap() not enough? It seems
to call io_pgtable_tlb_add_page() already, so it should be batching the
flushes.
Because if we leave io-pgtable in charge of maintenance it will also
inject additional invalidations and syncs for the sake of strictly
correct walk cache maintenance. Apparently we can get away without that
on this hardware, so the fundamental purpose of this series is to
sidestep it.
It's proven to be cleaner overall to devolve this kind of "non-standard"
TLB maintenance back to drivers rather than try to cram yet more
special-case complexity into io-pgtable itself. I'm planning to clean up
the remains of the TLBI_ON_MAP quirk entirely after this.
Robin.
return dom->iop->unmap(dom->iop, iova, size, gather);
}
@@ -523,9 +528,6 @@ static void mtk_iommu_iotlb_sync(struct iommu_domain *domain,
struct mtk_iommu_domain *dom = to_mtk_domain(domain);
size_t length = gather->end - gather->start;
- if (gather->start == ULONG_MAX)
- return;
-
mtk_iommu_tlb_flush_range_sync(gather->start, length, gather->pgsize,
dom->data);
}
--
2.18.0
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 3/3] iommu/vt-d: Fix ineffective devTLB invalidation for subdevices
Hi Yi,
On 2020/12/23 14:27, Liu Yi L wrote:
iommu_flush_dev_iotlb() is called to invalidate caches on device. It only
loops the devices which are full-attached to the domain. For sub-devices,
this is ineffective. This results in invalid caching entries left on the
device. Fix it by adding loop for subdevices as well. Also, the domain->
has_iotlb_device needs to be updated when attaching to subdevices.
Fixes: 67b8e02b5e761 ("iommu/vt-d: Aux-domain specific domain attach/detach")
Signed-off-by: Liu Yi L
---
drivers/iommu/intel/iommu.c | 63 +++--
1 file changed, 47 insertions(+), 16 deletions(-)
diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index acfe0a5b955e..e97c5ac1d7fc 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -726,6 +726,8 @@ static int domain_update_device_node(struct dmar_domain
*domain)
return nid;
}
+static void domain_update_iotlb(struct dmar_domain *domain);
+
/* Some capabilities may be different across iommus */
static void domain_update_iommu_cap(struct dmar_domain *domain)
{
@@ -739,6 +741,8 @@ static void domain_update_iommu_cap(struct dmar_domain
*domain)
*/
if (domain->nid == NUMA_NO_NODE)
domain->nid = domain_update_device_node(domain);
+
+ domain_update_iotlb(domain);
}
struct context_entry *iommu_context_addr(struct intel_iommu *iommu, u8 bus,
@@ -1459,6 +1463,18 @@ iommu_support_dev_iotlb (struct dmar_domain *domain,
struct intel_iommu *iommu,
return NULL;
}
+static bool dev_iotlb_enabled(struct device_domain_info *info)
+{
+ struct pci_dev *pdev;
+
+ if (!info->dev || !dev_is_pci(info->dev))
+ return false;
+
+ pdev = to_pci_dev(info->dev);
+
+ return !!pdev->ats_enabled;
+}
I know this is just separated from below function. But isn't "(info &&
info->ats_enabled)" is enough?
+
static void domain_update_iotlb(struct dmar_domain *domain)
{
struct device_domain_info *info;
@@ -1466,17 +1482,20 @@ static void domain_update_iotlb(struct dmar_domain
*domain)
assert_spin_locked(_domain_lock);
- list_for_each_entry(info, >devices, link) {
- struct pci_dev *pdev;
-
- if (!info->dev || !dev_is_pci(info->dev))
- continue;
-
- pdev = to_pci_dev(info->dev);
- if (pdev->ats_enabled) {
+ list_for_each_entry(info, >devices, link)
+ if (dev_iotlb_enabled(info)) {
has_iotlb_device = true;
break;
}
+
+ if (!has_iotlb_device) {
+ struct subdev_domain_info *sinfo;
+
+ list_for_each_entry(sinfo, >subdevices, link_domain)
+ if (dev_iotlb_enabled(get_domain_info(sinfo->pdev))) {
Please make the code easier for reading by:
info = get_domain_info(sinfo->pdev);
if (dev_iotlb_enabled(info))
Best regards,
baolu
+ has_iotlb_device = true;
+ break;
+ }
}
domain->has_iotlb_device = has_iotlb_device;
@@ -1557,25 +1576,37 @@ static void iommu_disable_dev_iotlb(struct
device_domain_info *info)
#endif
}
+static void __iommu_flush_dev_iotlb(struct device_domain_info *info,
+ u64 addr, unsigned int mask)
+{
+ u16 sid, qdep;
+
+ if (!info || !info->ats_enabled)
+ return;
+
+ sid = info->bus << 8 | info->devfn;
+ qdep = info->ats_qdep;
+ qi_flush_dev_iotlb(info->iommu, sid, info->pfsid,
+ qdep, addr, mask);
+}
+
static void iommu_flush_dev_iotlb(struct dmar_domain *domain,
u64 addr, unsigned mask)
{
- u16 sid, qdep;
unsigned long flags;
struct device_domain_info *info;
+ struct subdev_domain_info *sinfo;
if (!domain->has_iotlb_device)
return;
spin_lock_irqsave(_domain_lock, flags);
- list_for_each_entry(info, >devices, link) {
- if (!info->ats_enabled)
- continue;
+ list_for_each_entry(info, >devices, link)
+ __iommu_flush_dev_iotlb(info, addr, mask);
- sid = info->bus << 8 | info->devfn;
- qdep = info->ats_qdep;
- qi_flush_dev_iotlb(info->iommu, sid, info->pfsid,
- qdep, addr, mask);
+ list_for_each_entry(sinfo, >subdevices, link_domain) {
+ __iommu_flush_dev_iotlb(get_domain_info(sinfo->pdev),
+ addr, mask);
}
spin_unlock_irqrestore(_domain_lock, flags);
}
___
iommu mailing list
iommu@lists.linux-foundation.org
Re: [PATCH v3 6/7] iommu/mediatek: Gather iova in iommu_unmap to achieve tlb sync once
On Wed, Dec 16, 2020 at 06:36:06PM +0800, Yong Wu wrote:
> In current iommu_unmap, this code is:
>
> iommu_iotlb_gather_init(_gather);
> ret = __iommu_unmap(domain, iova, size, _gather);
> iommu_iotlb_sync(domain, _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
> ---
> drivers/iommu/mtk_iommu.c | 8 +---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
> index db7d43adb06b..89cec51405cd 100644
> --- a/drivers/iommu/mtk_iommu.c
> +++ b/drivers/iommu/mtk_iommu.c
> @@ -506,7 +506,12 @@ static size_t mtk_iommu_unmap(struct iommu_domain
> *domain,
> struct iommu_iotlb_gather *gather)
> {
> struct mtk_iommu_domain *dom = to_mtk_domain(domain);
> + unsigned long long end = iova + size;
>
> + if (gather->start > iova)
> + gather->start = iova;
> + if (gather->end < end)
> + gather->end = end;
I don't know how common the case is, but what happens if
gather->start...gather->end is a disjoint range from iova...end? E.g.
| gather | ..XXX... | iova |
| | | |
gather->start | iova |
gather->end end
We would also end up invalidating the TLB for the XXX area, which could
affect the performance.
Also, why is the existing code in __arm_v7s_unmap() not enough? It seems
to call io_pgtable_tlb_add_page() already, so it should be batching the
flushes.
> return dom->iop->unmap(dom->iop, iova, size, gather);
> }
>
> @@ -523,9 +528,6 @@ static void mtk_iommu_iotlb_sync(struct iommu_domain
> *domain,
> struct mtk_iommu_domain *dom = to_mtk_domain(domain);
> size_t length = gather->end - gather->start;
>
> - if (gather->start == ULONG_MAX)
> - return;
> -
> mtk_iommu_tlb_flush_range_sync(gather->start, length, gather->pgsize,
> dom->data);
> }
> --
> 2.18.0
>
> ___
> iommu mailing list
> iommu@lists.linux-foundation.org
> https://lists.linuxfoundation.org/mailman/listinfo/iommu
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v3 1/7] iommu: Move iotlb_sync_map out from __iommu_map
On Wed, Dec 16, 2020 at 06:36:01PM +0800, Yong Wu wrote: > 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. > > Signed-off-by: Yong Wu > Reviewed-by: Robin Murphy What about adding a little helper that does the NULL check and method call instead of duplicating it all over? ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v5 18/27] iommu/mediatek: Add power-domain operation
On Wed, Dec 09, 2020 at 04:00:53PM +0800, Yong Wu wrote:
> In the previous SoC, the M4U HW is in the EMI power domain which is
> always on. the latest M4U is in the display power domain which may be
> turned on/off, thus we have to add pm_runtime interface for it.
>
> When the engine work, the engine always enable the power and clocks for
> smi-larb/smi-common, then the M4U's power will always be powered on
> automatically via the device link with smi-common.
>
> Note: we don't enable the M4U power in iommu_map/unmap for tlb flush.
> If its power already is on, of course it is ok. if the power is off,
> the main tlb will be reset while M4U power on, thus the tlb flush while
> m4u power off is unnecessary, just skip it.
>
> There will be one case that pm runctime status is not expected when tlb
> flush. After boot, the display may call dma_alloc_attrs before it call
> pm_runtime_get(disp-dev), then the m4u's pm status is not active inside
> the dma_alloc_attrs. Since it only happens after boot, the tlb is clean
> at that time, I also think this is ok.
>
> Signed-off-by: Yong Wu
> ---
> drivers/iommu/mtk_iommu.c | 41 +--
> 1 file changed, 35 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
> index 6fe3ee2b2bf5..0e9c03cbab32 100644
> --- a/drivers/iommu/mtk_iommu.c
> +++ b/drivers/iommu/mtk_iommu.c
> @@ -184,6 +184,8 @@ static void mtk_iommu_tlb_flush_all(void *cookie)
> struct mtk_iommu_data *data = cookie;
>
> for_each_m4u(data) {
> + if (!pm_runtime_active(data->dev))
> + continue;
Is it guaranteed that the status is active in the check above, but then
the process is preempted and it goes down here?
Shouldn't we do something like below?
ret = pm_runtime_get_if_active();
if (!ret)
continue;
if (ret < 0)
// handle error
// Flush
pm_runtime_put();
Similar comment to the other places being changed by this patch.
> writel_relaxed(F_INVLD_EN1 | F_INVLD_EN0,
> data->base + data->plat_data->inv_sel_reg);
> writel_relaxed(F_ALL_INVLD, data->base + REG_MMU_INVALIDATE);
> @@ -200,6 +202,10 @@ static void mtk_iommu_tlb_flush_range_sync(unsigned long
> iova, size_t size,
> u32 tmp;
>
> for_each_m4u(data) {
> + /* skip tlb flush when pm is not active. */
> + if (!pm_runtime_active(data->dev))
> + continue;
> +
> spin_lock_irqsave(>tlb_lock, flags);
> writel_relaxed(F_INVLD_EN1 | F_INVLD_EN0,
> data->base + data->plat_data->inv_sel_reg);
> @@ -384,6 +390,8 @@ static int mtk_iommu_attach_device(struct iommu_domain
> *domain,
> {
> struct mtk_iommu_data *data = dev_iommu_priv_get(dev);
> struct mtk_iommu_domain *dom = to_mtk_domain(domain);
> + struct device *m4udev = data->dev;
> + bool pm_enabled = pm_runtime_enabled(m4udev);
> int ret;
>
> if (!data)
> @@ -391,12 +399,25 @@ static int mtk_iommu_attach_device(struct iommu_domain
> *domain,
>
> /* Update the pgtable base address register of the M4U HW */
> if (!data->m4u_dom) {
> + if (pm_enabled) {
> + ret = pm_runtime_get_sync(m4udev);
> + if (ret < 0) {
> + pm_runtime_put_noidle(m4udev);
> + return ret;
> + }
> + }
> ret = mtk_iommu_hw_init(data);
> - if (ret)
> + if (ret) {
> + if (pm_enabled)
> + pm_runtime_put(m4udev);
> return ret;
> + }
> data->m4u_dom = dom;
> writel(dom->cfg.arm_v7s_cfg.ttbr & MMU_PT_ADDR_MASK,
> data->base + REG_MMU_PT_BASE_ADDR);
> +
> + if (pm_enabled)
> + pm_runtime_put(m4udev);
> }
>
> mtk_iommu_config(data, dev, true);
> @@ -747,10 +768,13 @@ static int mtk_iommu_probe(struct platform_device *pdev)
> if (dev->pm_domain) {
> struct device_link *link;
>
> + pm_runtime_enable(dev);
> +
> link = device_link_add(data->smicomm_dev, dev,
> DL_FLAG_STATELESS | DL_FLAG_PM_RUNTIME);
> if (!link) {
> dev_err(dev, "Unable link %s.\n",
> dev_name(data->smicomm_dev));
> + pm_runtime_disable(dev);
> return -EINVAL;
> }
> }
> @@ -785,8 +809,10 @@ static int mtk_iommu_probe(struct platform_device *pdev)
> out_sysfs_remove:
> iommu_device_sysfs_remove(>iommu);
> out_link_remove:
> - if (dev->pm_domain)
> + if (dev->pm_domain) {
>
Re: [PATCH v5 17/27] iommu/mediatek: Add pm runtime callback
On Wed, Dec 09, 2020 at 04:00:52PM +0800, Yong Wu wrote:
> This patch adds pm runtime callback.
>
> In pm runtime case, all the registers backup/restore and bclk are
> controlled in the pm_runtime callback, then pm_suspend is not needed in
> this case.
>
> runtime PM is disabled when suspend, thus we call
> pm_runtime_status_suspended instead of pm_runtime_suspended.
>
> And, m4u doesn't have its special pm runtime domain in previous SoC, in
> this case dev->power.runtime_status is RPM_SUSPENDED defaultly,
This sounds wrong and could lead to hard to debug errors when the driver
is changed in the future. Would it be possible to make the behavior
consistent across the SoCs instead, so that runtime PM status is ACTIVE
when needed, even on SoCs without an IOMMU PM domain?
> thus add
> a "dev->pm_domain" checking for the SoC that has pm runtime domain.
>
> Signed-off-by: Yong Wu
> ---
> drivers/iommu/mtk_iommu.c | 22 --
> 1 file changed, 20 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
> index 5614015e5b96..6fe3ee2b2bf5 100644
> --- a/drivers/iommu/mtk_iommu.c
> +++ b/drivers/iommu/mtk_iommu.c
> @@ -808,7 +808,7 @@ static int mtk_iommu_remove(struct platform_device *pdev)
> return 0;
> }
>
> -static int __maybe_unused mtk_iommu_suspend(struct device *dev)
> +static int __maybe_unused mtk_iommu_runtime_suspend(struct device *dev)
> {
> struct mtk_iommu_data *data = dev_get_drvdata(dev);
> struct mtk_iommu_suspend_reg *reg = >reg;
> @@ -826,7 +826,7 @@ static int __maybe_unused mtk_iommu_suspend(struct device
> *dev)
> return 0;
> }
>
> -static int __maybe_unused mtk_iommu_resume(struct device *dev)
> +static int __maybe_unused mtk_iommu_runtime_resume(struct device *dev)
> {
> struct mtk_iommu_data *data = dev_get_drvdata(dev);
> struct mtk_iommu_suspend_reg *reg = >reg;
> @@ -853,7 +853,25 @@ static int __maybe_unused mtk_iommu_resume(struct device
> *dev)
> return 0;
> }
>
> +static int __maybe_unused mtk_iommu_suspend(struct device *dev)
> +{
> + /* runtime PM is disabled when suspend in pm_runtime case. */
> + if (dev->pm_domain && pm_runtime_status_suspended(dev))
> + return 0;
> +
> + return mtk_iommu_runtime_suspend(dev);
> +}
> +
> +static int __maybe_unused mtk_iommu_resume(struct device *dev)
> +{
> + if (dev->pm_domain && pm_runtime_status_suspended(dev))
> + return 0;
> +
> + return mtk_iommu_runtime_resume(dev);
> +}
Wouldn't it be enough to just use pm_runtime_force_suspend() and
pm_runtime_force_resume() as system sleep ops?
> +
> static const struct dev_pm_ops mtk_iommu_pm_ops = {
> + SET_RUNTIME_PM_OPS(mtk_iommu_runtime_suspend, mtk_iommu_runtime_resume,
> NULL)
> SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(mtk_iommu_suspend, mtk_iommu_resume)
> };
>
> --
> 2.18.0
>
> ___
> iommu mailing list
> iommu@lists.linux-foundation.org
> https://lists.linuxfoundation.org/mailman/listinfo/iommu
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v5 16/27] iommu/mediatek: Add device link for smi-common and m4u
On Wed, Dec 09, 2020 at 04:00:51PM +0800, Yong Wu wrote:
> In the lastest SoC, M4U has its special power domain. thus, If the engine
> begin to work, it should help enable the power for M4U firstly.
> Currently if the engine work, it always enable the power/clocks for
> smi-larbs/smi-common. This patch adds device_link for smi-common and M4U.
> then, if smi-common power is enabled, the M4U power also is powered on
> automatically.
>
> Normally M4U connect with several smi-larbs and their smi-common always
> are the same, In this patch it get smi-common dev from the first smi-larb
> device(i==0), then add the device_link only while m4u has power-domain.
>
> Signed-off-by: Yong Wu
> ---
> drivers/iommu/mtk_iommu.c | 30 --
> drivers/iommu/mtk_iommu.h | 1 +
> 2 files changed, 29 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
> index 09c8c58feb78..5614015e5b96 100644
> --- a/drivers/iommu/mtk_iommu.c
> +++ b/drivers/iommu/mtk_iommu.c
> @@ -20,6 +20,7 @@
> #include
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -706,7 +707,7 @@ static int mtk_iommu_probe(struct platform_device *pdev)
> return larb_nr;
>
> for (i = 0; i < larb_nr; i++) {
> - struct device_node *larbnode;
> + struct device_node *larbnode, *smicomm_node;
> struct platform_device *plarbdev;
> u32 id;
>
> @@ -732,6 +733,26 @@ static int mtk_iommu_probe(struct platform_device *pdev)
>
> component_match_add_release(dev, , release_of,
> compare_of, larbnode);
> + if (i != 0)
> + continue;
How about using the last larb instead and moving the code below outside
of the loop?
> + smicomm_node = of_parse_phandle(larbnode, "mediatek,smi", 0);
> + if (!smicomm_node)
> + return -EINVAL;
> +
> + plarbdev = of_find_device_by_node(smicomm_node);
> + of_node_put(smicomm_node);
> + data->smicomm_dev = >dev;
> + }
> +
> + if (dev->pm_domain) {
> + struct device_link *link;
> +
> + link = device_link_add(data->smicomm_dev, dev,
> +DL_FLAG_STATELESS | DL_FLAG_PM_RUNTIME);
> + if (!link) {
> + dev_err(dev, "Unable link %s.\n",
> dev_name(data->smicomm_dev));
> + return -EINVAL;
> + }
> }
>
> platform_set_drvdata(pdev, data);
> @@ -739,7 +760,7 @@ static int mtk_iommu_probe(struct platform_device *pdev)
> ret = iommu_device_sysfs_add(>iommu, dev, NULL,
>"mtk-iommu.%pa", );
> if (ret)
> - return ret;
> + goto out_link_remove;
>
> iommu_device_set_ops(>iommu, _iommu_ops);
> iommu_device_set_fwnode(>iommu, >dev.of_node->fwnode);
> @@ -763,6 +784,9 @@ static int mtk_iommu_probe(struct platform_device *pdev)
> iommu_device_unregister(>iommu);
> out_sysfs_remove:
> iommu_device_sysfs_remove(>iommu);
> +out_link_remove:
> + if (dev->pm_domain)
> + device_link_remove(data->smicomm_dev, dev);
> return ret;
> }
>
> @@ -777,6 +801,8 @@ static int mtk_iommu_remove(struct platform_device *pdev)
> bus_set_iommu(_bus_type, NULL);
>
> clk_disable_unprepare(data->bclk);
> + if (pdev->dev.pm_domain)
> + device_link_remove(data->smicomm_dev, >dev);
> devm_free_irq(>dev, data->irq, data);
> component_master_del(>dev, _iommu_com_ops);
> return 0;
> diff --git a/drivers/iommu/mtk_iommu.h b/drivers/iommu/mtk_iommu.h
> index d0c93652bdbe..5e03a029c4dc 100644
> --- a/drivers/iommu/mtk_iommu.h
> +++ b/drivers/iommu/mtk_iommu.h
> @@ -68,6 +68,7 @@ struct mtk_iommu_data {
>
> struct iommu_device iommu;
> const struct mtk_iommu_plat_data *plat_data;
> + struct device *smicomm_dev;
>
> struct dma_iommu_mapping*mapping; /* For mtk_iommu_v1.c */
>
> --
> 2.18.0
>
> ___
> iommu mailing list
> iommu@lists.linux-foundation.org
> https://lists.linuxfoundation.org/mailman/listinfo/iommu
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v5 06/27] dt-bindings: mediatek: Add binding for mt8192 IOMMU
On Wed, Dec 09, 2020 at 04:00:41PM +0800, Yong Wu wrote: > This patch adds decriptions for mt8192 IOMMU and SMI. > > mt8192 also is MTK IOMMU gen2 which uses ARM Short-Descriptor translation > table format. The M4U-SMI HW diagram is as below: > > EMI >| > M4U >| > >SMI Common > >| > +---+--+--+--+---+ > | | | | .. | | > | | | | | | > larb0 larb1 larb2 larb4 .. larb19 larb20 > disp0 disp1 mdpvdec IPE IPE > > All the connections are HW fixed, SW can NOT adjust it. > > mt8192 M4U support 0~16GB iova range. we preassign different engines > into different iova ranges: > > domain-id module iova-range larbs >0 disp0 ~ 4G larb0/1 >1 vcodec 4G ~ 8G larb4/5/7 >2 cam/mdp 8G ~ 12G larb2/9/11/13/14/16/17/18/19/20 Why do we preassign these addresses in DT? Shouldn't it be a user's or integrator's decision to split the 16 GB address range into sub-ranges and define which larbs those sub-ranges are shared with? Best regards, Tomasz >3 CCU00x4000_ ~ 0x43ff_ larb13: port 9/10 >4 CCU10x4400_ ~ 0x47ff_ larb14: port 4/5 > > The iova range for CCU0/1(camera control unit) is HW requirement. > > Signed-off-by: Yong Wu > Reviewed-by: Rob Herring > --- > .../bindings/iommu/mediatek,iommu.yaml| 18 +- > include/dt-bindings/memory/mt8192-larb-port.h | 240 ++ > 2 files changed, 257 insertions(+), 1 deletion(-) > create mode 100644 include/dt-bindings/memory/mt8192-larb-port.h > > diff --git a/Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml > b/Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml > index ba6626347381..0f26fe14c8e2 100644 > --- a/Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml > +++ b/Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml > @@ -76,6 +76,7 @@ properties: >- mediatek,mt8167-m4u # generation two >- mediatek,mt8173-m4u # generation two >- mediatek,mt8183-m4u # generation two > + - mediatek,mt8192-m4u # generation two > >- description: mt7623 generation one > items: > @@ -115,7 +116,11 @@ properties: >dt-binding/memory/mt6779-larb-port.h for mt6779, >dt-binding/memory/mt8167-larb-port.h for mt8167, >dt-binding/memory/mt8173-larb-port.h for mt8173, > - dt-binding/memory/mt8183-larb-port.h for mt8183. > + dt-binding/memory/mt8183-larb-port.h for mt8183, > + dt-binding/memory/mt8192-larb-port.h for mt8192. > + > + power-domains: > +maxItems: 1 > > required: >- compatible > @@ -133,11 +138,22 @@ allOf: >- mediatek,mt2701-m4u >- mediatek,mt2712-m4u >- mediatek,mt8173-m4u > + - mediatek,mt8192-m4u > > then: >required: > - clocks > > + - if: > + properties: > +compatible: > + enum: > +- mediatek,mt8192-m4u > + > +then: > + required: > +- power-domains > + > additionalProperties: false > > examples: > diff --git a/include/dt-bindings/memory/mt8192-larb-port.h > b/include/dt-bindings/memory/mt8192-larb-port.h > new file mode 100644 > index ..ec1ac2ba7094 > --- /dev/null > +++ b/include/dt-bindings/memory/mt8192-larb-port.h > @@ -0,0 +1,240 @@ > +/* SPDX-License-Identifier: GPL-2.0-only */ > +/* > + * Copyright (c) 2020 MediaTek Inc. > + * > + * Author: Chao Hao > + * Author: Yong Wu > + */ > +#ifndef _DT_BINDINGS_MEMORY_MT8192_LARB_PORT_H_ > +#define _DT_BINDINGS_MEMORY_MT8192_LARB_PORT_H_ > + > +#include > + > +/* > + * MM IOMMU: > + * domain 0: display: larb0, larb1. > + * domain 1: vcodec: larb4, larb5, larb7. > + * domain 2: CAM/MDP: larb2, larb9, larb11, larb13, larb14, larb16, > + * larb17, larb18, larb19, larb20, > + * domain 3: CCU0: larb13 - port9/10. > + * domain 4: CCU1: larb14 - port4/5. > + * > + * larb3/6/8/10/12/15 is null. > + */ > + > +/* larb0 */ > +#define M4U_PORT_L0_DISP_POSTMASK0 MTK_M4U_DOM_ID(0, 0, 0) > +#define M4U_PORT_L0_OVL_RDMA0_HDRMTK_M4U_DOM_ID(0, 0, 1) > +#define M4U_PORT_L0_OVL_RDMA0MTK_M4U_DOM_ID(0, 0, 2) > +#define M4U_PORT_L0_DISP_RDMA0 MTK_M4U_DOM_ID(0, 0, 3) > +#define M4U_PORT_L0_DISP_WDMA0 MTK_M4U_DOM_ID(0, 0, 4) > +#define M4U_PORT_L0_DISP_FAKE0 MTK_M4U_DOM_ID(0, 0, 5) > + > +/* larb1 */ > +#define M4U_PORT_L1_OVL_2L_RDMA0_HDR
Re: [PATCH v5 15/27] iommu/mediatek: Add fail handle for sysfs_add and device_register
On Wed, Dec 09, 2020 at 04:00:50PM +0800, Yong Wu wrote:
> Add fail handle for iommu_device_sysfs_add and iommu_device_register.
>
> Fixes: b16c0170b53c ("iommu/mediatek: Make use of iommu_device_register
> interface")
> Signed-off-by: Yong Wu
> ---
> drivers/iommu/mtk_iommu.c | 13 +++--
> 1 file changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
> index 39478cfbe0f1..09c8c58feb78 100644
> --- a/drivers/iommu/mtk_iommu.c
> +++ b/drivers/iommu/mtk_iommu.c
> @@ -746,7 +746,7 @@ static int mtk_iommu_probe(struct platform_device *pdev)
>
> ret = iommu_device_register(>iommu);
> if (ret)
> - return ret;
> + goto out_sysfs_remove;
>
> spin_lock_init(>tlb_lock);
> list_add_tail(>list, );
> @@ -754,7 +754,16 @@ static int mtk_iommu_probe(struct platform_device *pdev)
> if (!iommu_present(_bus_type))
> bus_set_iommu(_bus_type, _iommu_ops);
>
> - return component_master_add_with_match(dev, _iommu_com_ops, match);
> + ret = component_master_add_with_match(dev, _iommu_com_ops, match);
> + if (ret)
> + goto out_dev_unreg;
> + return ret;
> +
> +out_dev_unreg:
Shouldn't other operations be undone as well? I can see that above
bus_set_iommu() is set and an entry is added to m4ulist.
> + iommu_device_unregister(>iommu);
> +out_sysfs_remove:
> + iommu_device_sysfs_remove(>iommu);
> + return ret;
> }
>
> static int mtk_iommu_remove(struct platform_device *pdev)
> --
> 2.18.0
>
> ___
> iommu mailing list
> iommu@lists.linux-foundation.org
> https://lists.linuxfoundation.org/mailman/listinfo/iommu
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v5 09/27] iommu/io-pgtable-arm-v7s: Extend PA34 for MediaTek
On Wed, Dec 09, 2020 at 04:00:44PM +0800, Yong Wu wrote:
> MediaTek extend the bit5 in lvl1 and lvl2 descriptor as PA34.
>
> Signed-off-by: Yong Wu
> Acked-by: Will Deacon
> Reviewed-by: Robin Murphy
> ---
> drivers/iommu/io-pgtable-arm-v7s.c | 9 +++--
> drivers/iommu/mtk_iommu.c | 2 +-
> include/linux/io-pgtable.h | 4 ++--
> 3 files changed, 10 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/iommu/io-pgtable-arm-v7s.c
> b/drivers/iommu/io-pgtable-arm-v7s.c
> index e880745ab1e8..4d0aa079470f 100644
> --- a/drivers/iommu/io-pgtable-arm-v7s.c
> +++ b/drivers/iommu/io-pgtable-arm-v7s.c
> @@ -112,9 +112,10 @@
> #define ARM_V7S_TEX_MASK 0x7
> #define ARM_V7S_ATTR_TEX(val)(((val) & ARM_V7S_TEX_MASK) <<
> ARM_V7S_TEX_SHIFT)
>
> -/* MediaTek extend the two bits for PA 32bit/33bit */
> +/* MediaTek extend the bits below for PA 32bit/33bit/34bit */
> #define ARM_V7S_ATTR_MTK_PA_BIT32BIT(9)
> #define ARM_V7S_ATTR_MTK_PA_BIT33BIT(4)
> +#define ARM_V7S_ATTR_MTK_PA_BIT34BIT(5)
>
> /* *well, except for TEX on level 2 large pages, of course :( */
> #define ARM_V7S_CONT_PAGE_TEX_SHIFT 6
> @@ -194,6 +195,8 @@ static arm_v7s_iopte paddr_to_iopte(phys_addr_t paddr,
> int lvl,
> pte |= ARM_V7S_ATTR_MTK_PA_BIT32;
> if (paddr & BIT_ULL(33))
> pte |= ARM_V7S_ATTR_MTK_PA_BIT33;
> + if (paddr & BIT_ULL(34))
> + pte |= ARM_V7S_ATTR_MTK_PA_BIT34;
> return pte;
> }
>
> @@ -218,6 +221,8 @@ static phys_addr_t iopte_to_paddr(arm_v7s_iopte pte, int
> lvl,
> paddr |= BIT_ULL(32);
> if (pte & ARM_V7S_ATTR_MTK_PA_BIT33)
> paddr |= BIT_ULL(33);
> + if (pte & ARM_V7S_ATTR_MTK_PA_BIT34)
> + paddr |= BIT_ULL(34);
> return paddr;
> }
>
> @@ -754,7 +759,7 @@ static struct io_pgtable *arm_v7s_alloc_pgtable(struct
> io_pgtable_cfg *cfg,
> if (cfg->ias > ARM_V7S_ADDR_BITS)
> return NULL;
>
> - if (cfg->oas > (arm_v7s_is_mtk_enabled(cfg) ? 34 : ARM_V7S_ADDR_BITS))
> + if (cfg->oas > (arm_v7s_is_mtk_enabled(cfg) ? 35 : ARM_V7S_ADDR_BITS))
> return NULL;
>
> if (cfg->quirks & ~(IO_PGTABLE_QUIRK_ARM_NS |
> diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
> index 6451d83753e1..ec3c87d4b172 100644
> --- a/drivers/iommu/mtk_iommu.c
> +++ b/drivers/iommu/mtk_iommu.c
> @@ -320,7 +320,7 @@ static int mtk_iommu_domain_finalise(struct
> mtk_iommu_domain *dom)
> IO_PGTABLE_QUIRK_ARM_MTK_EXT,
> .pgsize_bitmap = mtk_iommu_ops.pgsize_bitmap,
> .ias = 32,
> - .oas = 34,
> + .oas = 35,
Shouldn't this be set according to the real hardware capabilities,
instead of always setting it to 35?
Best regards,
Tomasz
> .tlb = _iommu_flush_ops,
> .iommu_dev = data->dev,
> };
> diff --git a/include/linux/io-pgtable.h b/include/linux/io-pgtable.h
> index 4cde111e425b..1ae0757f4f94 100644
> --- a/include/linux/io-pgtable.h
> +++ b/include/linux/io-pgtable.h
> @@ -77,8 +77,8 @@ struct io_pgtable_cfg {
>* TLB maintenance when mapping as well as when unmapping.
>*
>* IO_PGTABLE_QUIRK_ARM_MTK_EXT: (ARM v7s format) MediaTek IOMMUs extend
> - * to support up to 34 bits PA where the bit32 and bit33 are
> - * encoded in the bit9 and bit4 of the PTE respectively.
> + * to support up to 35 bits PA where the bit32, bit33 and bit34 are
> + * encoded in the bit9, bit4 and bit5 of the PTE respectively.
>*
>* IO_PGTABLE_QUIRK_NON_STRICT: Skip issuing synchronous leaf TLBIs
>* on unmap, for DMA domains using the flush queue mechanism for
> --
> 2.18.0
>
> ___
> iommu mailing list
> iommu@lists.linux-foundation.org
> https://lists.linuxfoundation.org/mailman/listinfo/iommu
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v5 04/27] dt-bindings: memory: mediatek: Add domain definition
Hi Yong, On Wed, Dec 09, 2020 at 04:00:39PM +0800, Yong Wu wrote: > In the latest SoC, there are several HW IP require a sepecial iova > range, mainly CCU and VPU has this requirement. Take CCU as a example, > CCU require its iova locate in the range(0x4000_ ~ 0x43ff_). Is this really a domain? Does the address range come from the design of the IOMMU? Best regards, Tomasz > > In this patch we add a domain definition for the special port. In the > example of CCU, If we preassign CCU port in domain1, then iommu driver > will prepare a independent iommu domain of the special iova range for it, > then the iova got from dma_alloc_attrs(ccu-dev) will locate in its special > range. > > This is a preparing patch for multi-domain support. > > Signed-off-by: Yong Wu > Acked-by: Krzysztof Kozlowski > Acked-by: Rob Herring > --- > include/dt-bindings/memory/mtk-smi-larb-port.h | 9 - > 1 file changed, 8 insertions(+), 1 deletion(-) > > diff --git a/include/dt-bindings/memory/mtk-smi-larb-port.h > b/include/dt-bindings/memory/mtk-smi-larb-port.h > index 7d64103209af..2d4c973c174f 100644 > --- a/include/dt-bindings/memory/mtk-smi-larb-port.h > +++ b/include/dt-bindings/memory/mtk-smi-larb-port.h > @@ -7,9 +7,16 @@ > #define __DT_BINDINGS_MEMORY_MTK_MEMORY_PORT_H_ > > #define MTK_LARB_NR_MAX 32 > +#define MTK_M4U_DOM_NR_MAX 8 > + > +#define MTK_M4U_DOM_ID(domid, larb, port)\ > + (((domid) & 0x7) << 16 | (((larb) & 0x1f) << 5) | ((port) & 0x1f)) > + > +/* The default dom id is 0. */ > +#define MTK_M4U_ID(larb, port) MTK_M4U_DOM_ID(0, larb, port) > > -#define MTK_M4U_ID(larb, port) (((larb) << 5) | (port)) > #define MTK_M4U_TO_LARB(id) (((id) >> 5) & 0x1f) > #define MTK_M4U_TO_PORT(id) ((id) & 0x1f) > +#define MTK_M4U_TO_DOM(id) (((id) >> 16) & 0x7) > > #endif > -- > 2.18.0 > > ___ > iommu mailing list > iommu@lists.linux-foundation.org > https://lists.linuxfoundation.org/mailman/listinfo/iommu ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
