Re: [PATCH v2 1/7] iommu/io-pgtable: Introduce dynamic io-pgtable fmt registration

2020-12-23 Thread kernel test robot 
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

2020-12-23 Thread Georgi Djakov 

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

2020-12-23 Thread isaacm 

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

Re: [PATCH v2 1/7] iommu/io-pgtable: Introduce dynamic io-pgtable fmt registration

2020-12-23 Thread Robin Murphy 

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 1/7] iommu/io-pgtable: Introduce dynamic io-pgtable fmt registration

2020-12-22 Thread isaacm 

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;
  }

Re: [PATCH v2 1/7] iommu/io-pgtable: Introduce dynamic io-pgtable fmt registration

2020-12-22 Thread Robin Murphy 

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.



+   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

[PATCH v2 1/7] iommu/io-pgtable: Introduce dynamic io-pgtable fmt registration

2020-12-21 Thread Isaac J. Manjarres 
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");
+   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 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]);