[PATCH 0/5] crypto: arm - enable module autoloading

[PATCH 0/5] crypto: arm - enable module autoloading

[Ard Biesheuvel]

Now that the prerequisite changes are in place, we can modify the
various accelerated crypto modules that use special instructions to
expose the CPU feature they depend on. This allows udev to autoload
the module at boot.

Ard Biesheuvel (5):
  crypto: arm/aes-ce - enable module autoloading based on CPU feature
    bits
  crypto: arm/ghash-ce - enable module autoloading based on CPU feature
    bits
  crypto: arm/sha1-ce - enable module autoloading based on CPU feature
    bits
  crypto: arm/sha2-ce - enable module autoloading based on CPU feature
    bits
  crypto: arm/crc32 - enable module autoloading based on CPU feature
    bits

 arch/arm/crypto/aes-ce-glue.c   | 6 ++----
 arch/arm/crypto/crc32-ce-glue.c | 6 ++++++
 arch/arm/crypto/ghash-ce-glue.c | 6 ++----
 arch/arm/crypto/sha1-ce-glue.c  | 5 ++---
 arch/arm/crypto/sha2-ce-glue.c  | 5 ++---
 5 files changed, 14 insertions(+), 14 deletions(-)

-- 
2.7.4

[PATCH 1/5] crypto: arm/aes-ce - enable module autoloading based on CPU feature bits

[Ard Biesheuvel]

Make the module autoloadable by tying it to the CPU feature bit that
describes whether the optional instructions it relies on are implemented
by the current CPU.

Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 arch/arm/crypto/aes-ce-glue.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/arch/arm/crypto/aes-ce-glue.c b/arch/arm/crypto/aes-ce-glue.c
index 883b84d828c5..0f966a8ca1ce 100644
--- a/arch/arm/crypto/aes-ce-glue.c
+++ b/arch/arm/crypto/aes-ce-glue.c
@@ -14,6 +14,7 @@
 #include <crypto/aes.h>
 #include <crypto/internal/simd.h>
 #include <crypto/internal/skcipher.h>
+#include <linux/cpufeature.h>
 #include <linux/module.h>
 #include <crypto/xts.h>
 
@@ -425,9 +426,6 @@ static int __init aes_init(void)
 	int err;
 	int i;
 
-	if (!(elf_hwcap2 & HWCAP2_AES))
-		return -ENODEV;
-
 	err = crypto_register_skciphers(aes_algs, ARRAY_SIZE(aes_algs));
 	if (err)
 		return err;
@@ -451,5 +449,5 @@ static int __init aes_init(void)
 	return err;
 }
 
-module_init(aes_init);
+module_cpu_feature_match(AES, aes_init);
 module_exit(aes_exit);
-- 
2.7.4

[PATCH 2/5] crypto: arm/ghash-ce - enable module autoloading based on CPU feature bits

[Ard Biesheuvel]

Make the module autoloadable by tying it to the CPU feature bit that
describes whether the optional instructions it relies on are implemented
by the current CPU.

Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 arch/arm/crypto/ghash-ce-glue.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/arch/arm/crypto/ghash-ce-glue.c b/arch/arm/crypto/ghash-ce-glue.c
index 7546b3c02466..6bac8bea9f1e 100644
--- a/arch/arm/crypto/ghash-ce-glue.c
+++ b/arch/arm/crypto/ghash-ce-glue.c
@@ -15,6 +15,7 @@
 #include <crypto/cryptd.h>
 #include <crypto/internal/hash.h>
 #include <crypto/gf128mul.h>
+#include <linux/cpufeature.h>
 #include <linux/crypto.h>
 #include <linux/module.h>
 
@@ -311,9 +312,6 @@ static int __init ghash_ce_mod_init(void)
 {
 	int err;
 
-	if (!(elf_hwcap2 & HWCAP2_PMULL))
-		return -ENODEV;
-
 	err = crypto_register_shash(&ghash_alg);
 	if (err)
 		return err;
@@ -334,5 +332,5 @@ static void __exit ghash_ce_mod_exit(void)
 	crypto_unregister_shash(&ghash_alg);
 }
 
-module_init(ghash_ce_mod_init);
+module_cpu_feature_match(PMULL, ghash_ce_mod_init);
 module_exit(ghash_ce_mod_exit);
-- 
2.7.4

[PATCH 3/5] crypto: arm/sha1-ce - enable module autoloading based on CPU feature bits

[Ard Biesheuvel]

Make the module autoloadable by tying it to the CPU feature bit that
describes whether the optional instructions it relies on are implemented
by the current CPU.

Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 arch/arm/crypto/sha1-ce-glue.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/arch/arm/crypto/sha1-ce-glue.c b/arch/arm/crypto/sha1-ce-glue.c
index 80bc2fcd241a..555f72b5e659 100644
--- a/arch/arm/crypto/sha1-ce-glue.c
+++ b/arch/arm/crypto/sha1-ce-glue.c
@@ -11,6 +11,7 @@
 #include <crypto/internal/hash.h>
 #include <crypto/sha.h>
 #include <crypto/sha1_base.h>
+#include <linux/cpufeature.h>
 #include <linux/crypto.h>
 #include <linux/module.h>
 
@@ -82,8 +83,6 @@ static struct shash_alg alg = {
 
 static int __init sha1_ce_mod_init(void)
 {
-	if (!(elf_hwcap2 & HWCAP2_SHA1))
-		return -ENODEV;
 	return crypto_register_shash(&alg);
 }
 
@@ -92,5 +91,5 @@ static void __exit sha1_ce_mod_fini(void)
 	crypto_unregister_shash(&alg);
 }
 
-module_init(sha1_ce_mod_init);
+module_cpu_feature_match(SHA1, sha1_ce_mod_init);
 module_exit(sha1_ce_mod_fini);
-- 
2.7.4

[PATCH 4/5] crypto: arm/sha2-ce - enable module autoloading based on CPU feature bits

[Ard Biesheuvel]

Make the module autoloadable by tying it to the CPU feature bit that
describes whether the optional instructions it relies on are implemented
by the current CPU.

Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 arch/arm/crypto/sha2-ce-glue.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/arch/arm/crypto/sha2-ce-glue.c b/arch/arm/crypto/sha2-ce-glue.c
index 0755b2d657f3..df4dcef054ae 100644
--- a/arch/arm/crypto/sha2-ce-glue.c
+++ b/arch/arm/crypto/sha2-ce-glue.c
@@ -11,6 +11,7 @@
 #include <crypto/internal/hash.h>
 #include <crypto/sha.h>
 #include <crypto/sha256_base.h>
+#include <linux/cpufeature.h>
 #include <linux/crypto.h>
 #include <linux/module.h>
 
@@ -100,8 +101,6 @@ static struct shash_alg algs[] = { {
 
 static int __init sha2_ce_mod_init(void)
 {
-	if (!(elf_hwcap2 & HWCAP2_SHA2))
-		return -ENODEV;
 	return crypto_register_shashes(algs, ARRAY_SIZE(algs));
 }
 
@@ -110,5 +109,5 @@ static void __exit sha2_ce_mod_fini(void)
 	crypto_unregister_shashes(algs, ARRAY_SIZE(algs));
 }
 
-module_init(sha2_ce_mod_init);
+module_cpu_feature_match(SHA2, sha2_ce_mod_init);
 module_exit(sha2_ce_mod_fini);
-- 
2.7.4

[PATCH 5/5] crypto: arm/crc32 - enable module autoloading based on CPU feature bits

[Ard Biesheuvel]

Make the module autoloadable by tying it to the CPU feature bits that
describe whether the optional instructions it relies on are implemented
by the current CPU.

Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 arch/arm/crypto/crc32-ce-glue.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/arm/crypto/crc32-ce-glue.c b/arch/arm/crypto/crc32-ce-glue.c
index e1566bec1016..1b0e0e86ee9c 100644
--- a/arch/arm/crypto/crc32-ce-glue.c
+++ b/arch/arm/crypto/crc32-ce-glue.c
@@ -8,6 +8,7 @@
  * published by the Free Software Foundation.
  */
 
+#include <linux/cpufeature.h>
 #include <linux/crc32.h>
 #include <linux/init.h>
 #include <linux/kernel.h>
@@ -233,6 +234,11 @@ static void __exit crc32_pmull_mod_exit(void)
 				  ARRAY_SIZE(crc32_pmull_algs));
 }
 
+static const struct cpu_feature crc32_cpu_feature[] = {
+	{ cpu_feature(CRC32) }, { cpu_feature(PMULL) }, { }
+};
+MODULE_DEVICE_TABLE(cpu, crc32_cpu_feature);
+
 module_init(crc32_pmull_mod_init);
 module_exit(crc32_pmull_mod_exit);
 
-- 
2.7.4

[PATCH 0/5] crypto: arm - enable module autoloading

[Herbert Xu]

On Sun, May 21, 2017 at 10:23:35AM +0000, Ard Biesheuvel wrote:
> Now that the prerequisite changes are in place, we can modify the
> various accelerated crypto modules that use special instructions to
> expose the CPU feature they depend on. This allows udev to autoload
> the module at boot.

All applied.  Thanks.
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

[PATCH 1/5] crypto: arm/aes-ce - enable module autoloading based on CPU feature bits

[Stefan Agner]

Hi Ard,

On 21.05.2017 03:23, Ard Biesheuvel wrote:
> Make the module autoloadable by tying it to the CPU feature bit that
> describes whether the optional instructions it relies on are implemented
> by the current CPU.
> 

This leads to a compiler warning when compiling multi_v7_defconfig/ARM32
using Clang 6.0.1:

arch/arm/crypto/aes-ce-glue.c:450:1: warning: variable
'cpu_feature_match_AES' is not needed and will not
      be emitted [-Wunneeded-internal-declaration]
module_cpu_feature_match(AES, aes_init);

./include/linux/cpufeature.h:48:33: note: expanded from macro
'module_cpu_feature_match'
static struct cpu_feature const cpu_feature_match_ ## x[] =     \

<scratch space>:83:1: note: expanded from here
cpu_feature_match_AES
^
1 warning generated.

Do you happen to have an idea how to alleviate?

--
Stefan


> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>  arch/arm/crypto/aes-ce-glue.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/arm/crypto/aes-ce-glue.c b/arch/arm/crypto/aes-ce-glue.c
> index 883b84d828c5..0f966a8ca1ce 100644
> --- a/arch/arm/crypto/aes-ce-glue.c
> +++ b/arch/arm/crypto/aes-ce-glue.c
> @@ -14,6 +14,7 @@
>  #include <crypto/aes.h>
>  #include <crypto/internal/simd.h>
>  #include <crypto/internal/skcipher.h>
> +#include <linux/cpufeature.h>
>  #include <linux/module.h>
>  #include <crypto/xts.h>
>  
> @@ -425,9 +426,6 @@ static int __init aes_init(void)
>  	int err;
>  	int i;
>  
> -	if (!(elf_hwcap2 & HWCAP2_AES))
> -		return -ENODEV;
> -
>  	err = crypto_register_skciphers(aes_algs, ARRAY_SIZE(aes_algs));
>  	if (err)
>  		return err;
> @@ -451,5 +449,5 @@ static int __init aes_init(void)
>  	return err;
>  }
>  
> -module_init(aes_init);
> +module_cpu_feature_match(AES, aes_init);
>  module_exit(aes_exit);

[PATCH 1/5] crypto: arm/aes-ce - enable module autoloading based on CPU feature bits

[Ard Biesheuvel]

On 10 September 2018 at 08:21, Stefan Agner <stefan@agner.ch> wrote:
> Hi Ard,
>
> On 21.05.2017 03:23, Ard Biesheuvel wrote:
>> Make the module autoloadable by tying it to the CPU feature bit that
>> describes whether the optional instructions it relies on are implemented
>> by the current CPU.
>>
>
> This leads to a compiler warning when compiling multi_v7_defconfig/ARM32
> using Clang 6.0.1:
>
> arch/arm/crypto/aes-ce-glue.c:450:1: warning: variable
> 'cpu_feature_match_AES' is not needed and will not
>       be emitted [-Wunneeded-internal-declaration]
> module_cpu_feature_match(AES, aes_init);
>
> ./include/linux/cpufeature.h:48:33: note: expanded from macro
> 'module_cpu_feature_match'
> static struct cpu_feature const cpu_feature_match_ ## x[] =     \
>
> <scratch space>:83:1: note: expanded from here
> cpu_feature_match_AES
> ^
> 1 warning generated.
>
> Do you happen to have an idea how to alleviate?
>

I guess this only happens for modules that are selected as builtin,
and so MODULE_DEVICE_TABLE() resolves to nothing?
Does this only occur for CPU features?

[PATCH 1/5] crypto: arm/aes-ce - enable module autoloading based on CPU feature bits

[Stefan Agner]

On 10.09.2018 00:01, Ard Biesheuvel wrote:
> On 10 September 2018 at 08:21, Stefan Agner <stefan@agner.ch> wrote:
>> Hi Ard,
>>
>> On 21.05.2017 03:23, Ard Biesheuvel wrote:
>>> Make the module autoloadable by tying it to the CPU feature bit that
>>> describes whether the optional instructions it relies on are implemented
>>> by the current CPU.
>>>
>>
>> This leads to a compiler warning when compiling multi_v7_defconfig/ARM32
>> using Clang 6.0.1:
>>
>> arch/arm/crypto/aes-ce-glue.c:450:1: warning: variable
>> 'cpu_feature_match_AES' is not needed and will not
>>       be emitted [-Wunneeded-internal-declaration]
>> module_cpu_feature_match(AES, aes_init);
>>
>> ./include/linux/cpufeature.h:48:33: note: expanded from macro
>> 'module_cpu_feature_match'
>> static struct cpu_feature const cpu_feature_match_ ## x[] =     \
>>
>> <scratch space>:83:1: note: expanded from here
>> cpu_feature_match_AES
>> ^
>> 1 warning generated.
>>
>> Do you happen to have an idea how to alleviate?
>>
> 
> I guess this only happens for modules that are selected as builtin,
> and so MODULE_DEVICE_TABLE() resolves to nothing?
> Does this only occur for CPU features?

So in the above case CONFIG_ARM_CRYPTO=y, CONFIG_CRYPTO_AES_ARM_CE=m...

Right now I only saw it with CPU features... I remember seen similar issues, which got resolved. Digging in the git history I found 1f318a8bafcf ("modules: mark __inittest/__exittest as __maybe_unused"), 

This seems to resolve it:

--- a/include/linux/cpufeature.h
+++ b/include/linux/cpufeature.h
@@ -45,7 +45,7 @@
  * 'asm/cpufeature.h' of your favorite architecture.
  */
 #define module_cpu_feature_match(x, __initfunc)                        \
-static struct cpu_feature const cpu_feature_match_ ## x[] =    \
+static struct cpu_feature const __maybe_unused cpu_feature_match_ ## x[] = \
        { { .feature = cpu_feature(x) }, { } };                 \
 MODULE_DEVICE_TABLE(cpu, cpu_feature_match_ ## x);             \
                                                                \

Also arch/arm/crypto/crc32-ce-glue.c needs an extra __maybe_unused.

--
Stefan

[PATCH 1/5] crypto: arm/aes-ce - enable module autoloading based on CPU feature bits

[Ard Biesheuvel]

On 13 September 2018 at 08:24, Stefan Agner <stefan@agner.ch> wrote:
> On 10.09.2018 00:01, Ard Biesheuvel wrote:
>> On 10 September 2018 at 08:21, Stefan Agner <stefan@agner.ch> wrote:
>>> Hi Ard,
>>>
>>> On 21.05.2017 03:23, Ard Biesheuvel wrote:
>>>> Make the module autoloadable by tying it to the CPU feature bit that
>>>> describes whether the optional instructions it relies on are implemented
>>>> by the current CPU.
>>>>
>>>
>>> This leads to a compiler warning when compiling multi_v7_defconfig/ARM32
>>> using Clang 6.0.1:
>>>
>>> arch/arm/crypto/aes-ce-glue.c:450:1: warning: variable
>>> 'cpu_feature_match_AES' is not needed and will not
>>>       be emitted [-Wunneeded-internal-declaration]
>>> module_cpu_feature_match(AES, aes_init);
>>>
>>> ./include/linux/cpufeature.h:48:33: note: expanded from macro
>>> 'module_cpu_feature_match'
>>> static struct cpu_feature const cpu_feature_match_ ## x[] =     \
>>>
>>> <scratch space>:83:1: note: expanded from here
>>> cpu_feature_match_AES
>>> ^
>>> 1 warning generated.
>>>
>>> Do you happen to have an idea how to alleviate?
>>>
>>
>> I guess this only happens for modules that are selected as builtin,
>> and so MODULE_DEVICE_TABLE() resolves to nothing?
>> Does this only occur for CPU features?
>
> So in the above case CONFIG_ARM_CRYPTO=y, CONFIG_CRYPTO_AES_ARM_CE=m...
>
> Right now I only saw it with CPU features... I remember seen similar issues, which got resolved. Digging in the git history I found 1f318a8bafcf ("modules: mark __inittest/__exittest as __maybe_unused"),
>
> This seems to resolve it:
>
> --- a/include/linux/cpufeature.h
> +++ b/include/linux/cpufeature.h
> @@ -45,7 +45,7 @@
>   * 'asm/cpufeature.h' of your favorite architecture.
>   */
>  #define module_cpu_feature_match(x, __initfunc)                        \
> -static struct cpu_feature const cpu_feature_match_ ## x[] =    \
> +static struct cpu_feature const __maybe_unused cpu_feature_match_ ## x[] = \
>         { { .feature = cpu_feature(x) }, { } };                 \
>  MODULE_DEVICE_TABLE(cpu, cpu_feature_match_ ## x);             \
>                                                                 \
>
> Also arch/arm/crypto/crc32-ce-glue.c needs an extra __maybe_unused.
>

Yes, that looks like the right approach to me. The difference between
other uses of MODULE_DEVICE_TABLE() is that the second argument
usually gets referenced in some way in the driver struct. It the CPU
feature case, that does not happen and so the struct ends up being
unreferenced when building the code into the kernel.