[PATCH -next] crypto: Fix build error when CRYPTO_BLAKE2S_X86=m && CRYPTO_ALGAPI2=m && CONFIG_CRYPTO_MANAGER_EXTRA_TESTS=y

[PATCH -next] crypto: Fix build error when CRYPTO_BLAKE2S_X86=m && CRYPTO_ALGAPI2=m && CONFIG_CRYPTO_MANAGER_EXTRA_TESTS=y

[gaochao]

If CRYPTO=m, CRYPTO_ALGAPI=m, CRYPTO_ALGAPI2=m, CRYPTO_BLAKE2S_X86=m,
CONFIG_CRYPTO_MANAGER_EXTRA_TESTS=y
bulding fails:

arch/x86/crypto/blake2s-glue.o: In function `blake2s_compress':
(.text+0x5a): undefined reference to `crypto_simd_disabled_for_test'
make: *** [vmlinux] Error 1

When CRYPTO_MANAGER_EXTRA_TESTS=y, blake2s_compress will call
crypto_simd_disabled_for_test.
When CRYPTO_ALGAPI2=m, crypto_algapi build as a module,
but if CONFIG_CRYPTO_BLAKE2S_X86=m at the same time,
libblake2s-x86_64.o build with obj-y, this will accuse the above error.

To fix this error:
1 Choose CRYPTO_ALGAPI2 for CRYPTO_BLAKE2S_X86
when CRYPTO_MANAGER_EXTRA_TESTS=y.
2 build libblake2s-x86_64.o as a module when CONFIG_CRYPTO_BLAKE2S_X86=y

Fixes: 8fc5f2ad896b ("crypto: testmgr - Move crypto_simd_disabled_for_test out")
Reported-by: Hulk Robot <hulkci@huawei.com>
Signed-off-by: gaochao <gaochao49@huawei.com>
---
 arch/x86/crypto/Makefile | 2 +-
 crypto/Kconfig           | 1 +
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/x86/crypto/Makefile b/arch/x86/crypto/Makefile
index 2831685adf6f..54b2469fa49a 100644
--- a/arch/x86/crypto/Makefile
+++ b/arch/x86/crypto/Makefile
@@ -63,7 +63,7 @@ sha512-ssse3-y := sha512-ssse3-asm.o sha512-avx-asm.o sha512-avx2-asm.o sha512_s

 obj-$(CONFIG_CRYPTO_BLAKE2S_X86) += blake2s-x86_64.o
 blake2s-x86_64-y := blake2s-shash.o
-obj-$(if $(CONFIG_CRYPTO_BLAKE2S_X86),y) += libblake2s-x86_64.o
+obj-$(CONFIG_CRYPTO_BLAKE2S_X86) += libblake2s-x86_64.o
 libblake2s-x86_64-y := blake2s-core.o blake2s-glue.o

 obj-$(CONFIG_CRYPTO_GHASH_CLMUL_NI_INTEL) += ghash-clmulni-intel.o
diff --git a/crypto/Kconfig b/crypto/Kconfig
index 19197469cfab..e61598f8f8c5 100644
--- a/crypto/Kconfig
+++ b/crypto/Kconfig
@@ -714,6 +714,7 @@ config CRYPTO_BLAKE2S_X86
 	depends on X86 && 64BIT
 	select CRYPTO_LIB_BLAKE2S_GENERIC
 	select CRYPTO_ARCH_HAVE_LIB_BLAKE2S
+	select CRYPTO_ALGAPI2 if CRYPTO_MANAGER_EXTRA_TESTS

 config CRYPTO_CRCT10DIF
 	tristate "CRCT10DIF algorithm"
--
2.17.1

Re: [PATCH -next] crypto: Fix build error when CRYPTO_BLAKE2S_X86=m && CRYPTO_ALGAPI2=m && CONFIG_CRYPTO_MANAGER_EXTRA_TESTS=y

[gaochao (L)]

friendly ping 

在 2022/5/17 11:36, gaochao 写道:
> If CRYPTO=m, CRYPTO_ALGAPI=m, CRYPTO_ALGAPI2=m, CRYPTO_BLAKE2S_X86=m,
> CONFIG_CRYPTO_MANAGER_EXTRA_TESTS=y
> bulding fails:
> 
> arch/x86/crypto/blake2s-glue.o: In function `blake2s_compress':
> (.text+0x5a): undefined reference to `crypto_simd_disabled_for_test'
> make: *** [vmlinux] Error 1
> 
> When CRYPTO_MANAGER_EXTRA_TESTS=y, blake2s_compress will call
> crypto_simd_disabled_for_test.
> When CRYPTO_ALGAPI2=m, crypto_algapi build as a module,
> but if CONFIG_CRYPTO_BLAKE2S_X86=m at the same time,
> libblake2s-x86_64.o build with obj-y, this will accuse the above error.
> 
> To fix this error:
> 1 Choose CRYPTO_ALGAPI2 for CRYPTO_BLAKE2S_X86
> when CRYPTO_MANAGER_EXTRA_TESTS=y.
> 2 build libblake2s-x86_64.o as a module when CONFIG_CRYPTO_BLAKE2S_X86=y
> 
> Fixes: 8fc5f2ad896b ("crypto: testmgr - Move crypto_simd_disabled_for_test out")
> Reported-by: Hulk Robot <hulkci@huawei.com>
> Signed-off-by: gaochao <gaochao49@huawei.com>
> ---
>  arch/x86/crypto/Makefile | 2 +-
>  crypto/Kconfig           | 1 +
>  2 files changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/crypto/Makefile b/arch/x86/crypto/Makefile
> index 2831685adf6f..54b2469fa49a 100644
> --- a/arch/x86/crypto/Makefile
> +++ b/arch/x86/crypto/Makefile
> @@ -63,7 +63,7 @@ sha512-ssse3-y := sha512-ssse3-asm.o sha512-avx-asm.o sha512-avx2-asm.o sha512_s
> 
>  obj-$(CONFIG_CRYPTO_BLAKE2S_X86) += blake2s-x86_64.o
>  blake2s-x86_64-y := blake2s-shash.o
> -obj-$(if $(CONFIG_CRYPTO_BLAKE2S_X86),y) += libblake2s-x86_64.o
> +obj-$(CONFIG_CRYPTO_BLAKE2S_X86) += libblake2s-x86_64.o
>  libblake2s-x86_64-y := blake2s-core.o blake2s-glue.o
> 
>  obj-$(CONFIG_CRYPTO_GHASH_CLMUL_NI_INTEL) += ghash-clmulni-intel.o
> diff --git a/crypto/Kconfig b/crypto/Kconfig
> index 19197469cfab..e61598f8f8c5 100644
> --- a/crypto/Kconfig
> +++ b/crypto/Kconfig
> @@ -714,6 +714,7 @@ config CRYPTO_BLAKE2S_X86
>  	depends on X86 && 64BIT
>  	select CRYPTO_LIB_BLAKE2S_GENERIC
>  	select CRYPTO_ARCH_HAVE_LIB_BLAKE2S
> +	select CRYPTO_ALGAPI2 if CRYPTO_MANAGER_EXTRA_TESTS
> 
>  config CRYPTO_CRCT10DIF
>  	tristate "CRCT10DIF algorithm"
> --
> 2.17.1
> 
> .

Re: [PATCH -next] crypto: Fix build error when CRYPTO_BLAKE2S_X86=m && CRYPTO_ALGAPI2=m && CONFIG_CRYPTO_MANAGER_EXTRA_TESTS=y

[Eric Biggers]

+Cc Jason

On Tue, May 17, 2022 at 11:36:30AM +0800, gaochao wrote:
> If CRYPTO=m, CRYPTO_ALGAPI=m, CRYPTO_ALGAPI2=m, CRYPTO_BLAKE2S_X86=m,
> CONFIG_CRYPTO_MANAGER_EXTRA_TESTS=y
> bulding fails:
> 
> arch/x86/crypto/blake2s-glue.o: In function `blake2s_compress':
> (.text+0x5a): undefined reference to `crypto_simd_disabled_for_test'
> make: *** [vmlinux] Error 1
> 
> When CRYPTO_MANAGER_EXTRA_TESTS=y, blake2s_compress will call
> crypto_simd_disabled_for_test.
> When CRYPTO_ALGAPI2=m, crypto_algapi build as a module,
> but if CONFIG_CRYPTO_BLAKE2S_X86=m at the same time,
> libblake2s-x86_64.o build with obj-y, this will accuse the above error.
> 
> To fix this error:
> 1 Choose CRYPTO_ALGAPI2 for CRYPTO_BLAKE2S_X86
> when CRYPTO_MANAGER_EXTRA_TESTS=y.
> 2 build libblake2s-x86_64.o as a module when CONFIG_CRYPTO_BLAKE2S_X86=y
> 
> Fixes: 8fc5f2ad896b ("crypto: testmgr - Move crypto_simd_disabled_for_test out")
> Reported-by: Hulk Robot <hulkci@huawei.com>
> Signed-off-by: gaochao <gaochao49@huawei.com>
> ---
>  arch/x86/crypto/Makefile | 2 +-
>  crypto/Kconfig           | 1 +
>  2 files changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/crypto/Makefile b/arch/x86/crypto/Makefile
> index 2831685adf6f..54b2469fa49a 100644
> --- a/arch/x86/crypto/Makefile
> +++ b/arch/x86/crypto/Makefile
> @@ -63,7 +63,7 @@ sha512-ssse3-y := sha512-ssse3-asm.o sha512-avx-asm.o sha512-avx2-asm.o sha512_s
> 
>  obj-$(CONFIG_CRYPTO_BLAKE2S_X86) += blake2s-x86_64.o
>  blake2s-x86_64-y := blake2s-shash.o
> -obj-$(if $(CONFIG_CRYPTO_BLAKE2S_X86),y) += libblake2s-x86_64.o
> +obj-$(CONFIG_CRYPTO_BLAKE2S_X86) += libblake2s-x86_64.o
>  libblake2s-x86_64-y := blake2s-core.o blake2s-glue.o

I think the (somewhat unusual) use of "obj-$(if $(CONFIG_CRYPTO_BLAKE2S_X86),y)"
is working as intended.  The goal to build the x86 BLAKE2s library into the core
kernel if CONFIG_CRYPTO_BLAKE2S_X86 is set to either y or m.  This change would
break that.  And if we were to make this change, then there would no longer be
any need to have blake2s-x86_64 and libblake2s-x86_64 be separate modules.

> diff --git a/crypto/Kconfig b/crypto/Kconfig
> index 19197469cfab..e61598f8f8c5 100644
> --- a/crypto/Kconfig
> +++ b/crypto/Kconfig
> @@ -714,6 +714,7 @@ config CRYPTO_BLAKE2S_X86
>  	depends on X86 && 64BIT
>  	select CRYPTO_LIB_BLAKE2S_GENERIC
>  	select CRYPTO_ARCH_HAVE_LIB_BLAKE2S
> +	select CRYPTO_ALGAPI2 if CRYPTO_MANAGER_EXTRA_TESTS

I expect that we'll run into this problem in the future too.  So just fixing it
for CRYPTO_BLAKE2S_X86 specifically isn't ideal.

How about just making CRYPTO_MANAGER_EXTRA_TESTS depend on CRYPTO_ALGAPI=y?

Or alternatively, move the crypto_simd_disabled_for_test variable into a file in
lib/crypto/.  But the other suggestion would be a bit simpler for now.

- Eric

Re: [PATCH -next] crypto: Fix build error when CRYPTO_BLAKE2S_X86=m && CRYPTO_ALGAPI2=m && CONFIG_CRYPTO_MANAGER_EXTRA_TESTS=y

[Jason A. Donenfeld]

Hi Eric,

Thanks for CCing me into this.

On 5/26/22, Eric Biggers <ebiggers@kernel.org> wrote:
> How about just making CRYPTO_MANAGER_EXTRA_TESTS depend on CRYPTO_ALGAPI=y?

I think this would definitely be the best option, and probably best
represents what somebody debugging / running tests has in mind.

I can send a patch for this today if you'd like.

Jason

Re: [PATCH -next] crypto: Fix build error when CRYPTO_BLAKE2S_X86=m && CRYPTO_ALGAPI2=m && CONFIG_CRYPTO_MANAGER_EXTRA_TESTS=y

[Herbert Xu]

On Thu, May 26, 2022 at 12:25:06AM -0700, Eric Biggers wrote:
>
> How about just making CRYPTO_MANAGER_EXTRA_TESTS depend on CRYPTO_ALGAPI=y?

That breaks my setups with ALGAPI=m.

> Or alternatively, move the crypto_simd_disabled_for_test variable into a file in
> lib/crypto/.  But the other suggestion would be a bit simpler for now.

Yes please do that instead.  If we ever get some arch-independent
file for SIMD then we could move it over there.

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

Re: [PATCH -next] crypto: Fix build error when CRYPTO_BLAKE2S_X86=m && CRYPTO_ALGAPI2=m && CONFIG_CRYPTO_MANAGER_EXTRA_TESTS=y

[Jason A. Donenfeld]

On Thu, May 26, 2022 at 04:26:35PM +0800, Herbert Xu wrote:
> On Thu, May 26, 2022 at 12:25:06AM -0700, Eric Biggers wrote:
> >
> > How about just making CRYPTO_MANAGER_EXTRA_TESTS depend on CRYPTO_ALGAPI=y?
> 
> That breaks my setups with ALGAPI=m.
> 
> > Or alternatively, move the crypto_simd_disabled_for_test variable into a file in
> > lib/crypto/.  But the other suggestion would be a bit simpler for now.
> 
> Yes please do that instead.  If we ever get some arch-independent
> file for SIMD then we could move it over there.

Actually, we were talking about getting rid of the blake2s shash before
anyway, because it has no purpose. I'll make that change and send a
series, which will fix the issue a different way.

Jason