linux-crypto.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 1/2] lib/crypto: blake2s: include as built-in
@ 2021-12-23 14:11 Jason A. Donenfeld
  2021-12-23 14:20 ` Ard Biesheuvel
                   ` (2 more replies)
  0 siblings, 3 replies; 23+ messages in thread
From: Jason A. Donenfeld @ 2021-12-23 14:11 UTC (permalink / raw)
  To: linux-kernel, tytso, gregkh
  Cc: Jason A. Donenfeld, Ard Biesheuvel, Masahiro Yamada,
	linux-kbuild, Herbert Xu, linux-crypto

In preparation for using blake2s in the RNG, we change the way that it
is wired-in to the build system. Instead of kconfig mazes and ifdefs, we
use weak symbols, so that an arch version can override the generic
version. Then we include the generic version in lib-y, so that it can be
removed from the image if the arch version doesn't fallback to it (as is
the case on arm though not x86). The result should be a bit simpler and
smaller than the code it replaces.

Cc: Ard Biesheuvel <ardb@kernel.org>
Cc: Masahiro Yamada <masahiroy@kernel.org>
Cc: linux-kbuild@vger.kernel.org
Cc: Herbert Xu <herbert@gondor.apana.org.au>
Cc: linux-crypto@vger.kernel.org
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
---
Herbert - I intend to take this via the crng/random.git tree, since it
forms a dependency and I'd like to send a pull early in 5.17 cycle.

 Makefile                          |  2 +-
 arch/arm/crypto/Kconfig           |  3 +--
 arch/arm/crypto/blake2s-core.S    |  8 ++++----
 arch/arm/crypto/blake2s-glue.c    |  6 +++---
 arch/s390/configs/debug_defconfig |  1 -
 arch/s390/configs/defconfig       |  1 -
 arch/x86/crypto/blake2s-glue.c    | 11 +++++------
 crypto/Kconfig                    |  5 +----
 drivers/net/Kconfig               |  1 -
 include/crypto/internal/blake2s.h |  6 +++---
 lib/Makefile                      |  2 +-
 lib/crypto/Kconfig                | 25 -------------------------
 lib/crypto/Makefile               |  7 +++----
 lib/crypto/blake2s-generic.c      |  6 +++++-
 lib/crypto/blake2s.c              |  6 ------
 15 files changed, 27 insertions(+), 63 deletions(-)

diff --git a/Makefile b/Makefile
index d85f1ff79f5c..892ea632ea63 100644
--- a/Makefile
+++ b/Makefile
@@ -668,7 +668,7 @@ drivers-y	:= drivers/ sound/
 drivers-$(CONFIG_SAMPLES) += samples/
 drivers-$(CONFIG_NET) += net/
 drivers-y	+= virt/
-libs-y		:= lib/
+libs-y		:= lib/ lib/crypto/
 endif # KBUILD_EXTMOD
 
 # The all: target is the default when no target is given on the
diff --git a/arch/arm/crypto/Kconfig b/arch/arm/crypto/Kconfig
index 2b575792363e..47cb22645746 100644
--- a/arch/arm/crypto/Kconfig
+++ b/arch/arm/crypto/Kconfig
@@ -63,8 +63,7 @@ config CRYPTO_SHA512_ARM
 	  using optimized ARM assembler and NEON, when available.
 
 config CRYPTO_BLAKE2S_ARM
-	tristate "BLAKE2s digest algorithm (ARM)"
-	select CRYPTO_ARCH_HAVE_LIB_BLAKE2S
+	bool "BLAKE2s digest algorithm (ARM)"
 	help
 	  BLAKE2s digest algorithm optimized with ARM scalar instructions.  This
 	  is faster than the generic implementations of BLAKE2s and BLAKE2b, but
diff --git a/arch/arm/crypto/blake2s-core.S b/arch/arm/crypto/blake2s-core.S
index 86345751bbf3..df40e46601f1 100644
--- a/arch/arm/crypto/blake2s-core.S
+++ b/arch/arm/crypto/blake2s-core.S
@@ -167,8 +167,8 @@
 .endm
 
 //
-// void blake2s_compress_arch(struct blake2s_state *state,
-//			      const u8 *block, size_t nblocks, u32 inc);
+// void blake2s_compress(struct blake2s_state *state,
+//			 const u8 *block, size_t nblocks, u32 inc);
 //
 // Only the first three fields of struct blake2s_state are used:
 //	u32 h[8];	(inout)
@@ -176,7 +176,7 @@
 //	u32 f[2];	(in)
 //
 	.align		5
-ENTRY(blake2s_compress_arch)
+ENTRY(blake2s_compress)
 	push		{r0-r2,r4-r11,lr}	// keep this an even number
 
 .Lnext_block:
@@ -303,4 +303,4 @@ ENTRY(blake2s_compress_arch)
 	str		r3, [r12], #4
 	bne		1b
 	b		.Lcopy_block_done
-ENDPROC(blake2s_compress_arch)
+ENDPROC(blake2s_compress)
diff --git a/arch/arm/crypto/blake2s-glue.c b/arch/arm/crypto/blake2s-glue.c
index f2cc1e5fc9ec..09d3a0cabd2c 100644
--- a/arch/arm/crypto/blake2s-glue.c
+++ b/arch/arm/crypto/blake2s-glue.c
@@ -11,17 +11,17 @@
 #include <linux/module.h>
 
 /* defined in blake2s-core.S */
-EXPORT_SYMBOL(blake2s_compress_arch);
+EXPORT_SYMBOL(blake2s_compress);
 
 static int crypto_blake2s_update_arm(struct shash_desc *desc,
 				     const u8 *in, unsigned int inlen)
 {
-	return crypto_blake2s_update(desc, in, inlen, blake2s_compress_arch);
+	return crypto_blake2s_update(desc, in, inlen, blake2s_compress);
 }
 
 static int crypto_blake2s_final_arm(struct shash_desc *desc, u8 *out)
 {
-	return crypto_blake2s_final(desc, out, blake2s_compress_arch);
+	return crypto_blake2s_final(desc, out, blake2s_compress);
 }
 
 #define BLAKE2S_ALG(name, driver_name, digest_size)			\
diff --git a/arch/s390/configs/debug_defconfig b/arch/s390/configs/debug_defconfig
index e45cc27716de..caa3d1d6a0e8 100644
--- a/arch/s390/configs/debug_defconfig
+++ b/arch/s390/configs/debug_defconfig
@@ -757,7 +757,6 @@ CONFIG_CRYPTO_USER_API_SKCIPHER=m
 CONFIG_CRYPTO_USER_API_RNG=m
 CONFIG_CRYPTO_USER_API_AEAD=m
 CONFIG_CRYPTO_STATS=y
-CONFIG_CRYPTO_LIB_BLAKE2S=m
 CONFIG_CRYPTO_LIB_CURVE25519=m
 CONFIG_CRYPTO_LIB_CHACHA20POLY1305=m
 CONFIG_ZCRYPT=m
diff --git a/arch/s390/configs/defconfig b/arch/s390/configs/defconfig
index 1c750bfca2d8..fffc6af5358c 100644
--- a/arch/s390/configs/defconfig
+++ b/arch/s390/configs/defconfig
@@ -744,7 +744,6 @@ CONFIG_CRYPTO_USER_API_SKCIPHER=m
 CONFIG_CRYPTO_USER_API_RNG=m
 CONFIG_CRYPTO_USER_API_AEAD=m
 CONFIG_CRYPTO_STATS=y
-CONFIG_CRYPTO_LIB_BLAKE2S=m
 CONFIG_CRYPTO_LIB_CURVE25519=m
 CONFIG_CRYPTO_LIB_CHACHA20POLY1305=m
 CONFIG_ZCRYPT=m
diff --git a/arch/x86/crypto/blake2s-glue.c b/arch/x86/crypto/blake2s-glue.c
index a40365ab301e..ef91a3167d27 100644
--- a/arch/x86/crypto/blake2s-glue.c
+++ b/arch/x86/crypto/blake2s-glue.c
@@ -28,9 +28,8 @@ asmlinkage void blake2s_compress_avx512(struct blake2s_state *state,
 static __ro_after_init DEFINE_STATIC_KEY_FALSE(blake2s_use_ssse3);
 static __ro_after_init DEFINE_STATIC_KEY_FALSE(blake2s_use_avx512);
 
-void blake2s_compress_arch(struct blake2s_state *state,
-			   const u8 *block, size_t nblocks,
-			   const u32 inc)
+void blake2s_compress(struct blake2s_state *state, const u8 *block,
+		      size_t nblocks, const u32 inc)
 {
 	/* SIMD disables preemption, so relax after processing each page. */
 	BUILD_BUG_ON(SZ_4K / BLAKE2S_BLOCK_SIZE < 8);
@@ -56,17 +55,17 @@ void blake2s_compress_arch(struct blake2s_state *state,
 		block += blocks * BLAKE2S_BLOCK_SIZE;
 	} while (nblocks);
 }
-EXPORT_SYMBOL(blake2s_compress_arch);
+EXPORT_SYMBOL(blake2s_compress);
 
 static int crypto_blake2s_update_x86(struct shash_desc *desc,
 				     const u8 *in, unsigned int inlen)
 {
-	return crypto_blake2s_update(desc, in, inlen, blake2s_compress_arch);
+	return crypto_blake2s_update(desc, in, inlen, blake2s_compress);
 }
 
 static int crypto_blake2s_final_x86(struct shash_desc *desc, u8 *out)
 {
-	return crypto_blake2s_final(desc, out, blake2s_compress_arch);
+	return crypto_blake2s_final(desc, out, blake2s_compress);
 }
 
 #define BLAKE2S_ALG(name, driver_name, digest_size)			\
diff --git a/crypto/Kconfig b/crypto/Kconfig
index 285f82647d2b..bfda2c82774d 100644
--- a/crypto/Kconfig
+++ b/crypto/Kconfig
@@ -685,7 +685,6 @@ config CRYPTO_BLAKE2B
 
 config CRYPTO_BLAKE2S
 	tristate "BLAKE2s digest algorithm"
-	select CRYPTO_LIB_BLAKE2S_GENERIC
 	select CRYPTO_HASH
 	help
 	  Implementation of cryptographic hash function BLAKE2s
@@ -702,10 +701,8 @@ config CRYPTO_BLAKE2S
 	  See https://blake2.net for further information.
 
 config CRYPTO_BLAKE2S_X86
-	tristate "BLAKE2s digest algorithm (x86 accelerated version)"
+	bool "BLAKE2s digest algorithm (x86 accelerated version)"
 	depends on X86 && 64BIT
-	select CRYPTO_LIB_BLAKE2S_GENERIC
-	select CRYPTO_ARCH_HAVE_LIB_BLAKE2S
 
 config CRYPTO_CRCT10DIF
 	tristate "CRCT10DIF algorithm"
diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
index 6cccc3dc00bc..b2a4f998c180 100644
--- a/drivers/net/Kconfig
+++ b/drivers/net/Kconfig
@@ -81,7 +81,6 @@ config WIREGUARD
 	select CRYPTO
 	select CRYPTO_LIB_CURVE25519
 	select CRYPTO_LIB_CHACHA20POLY1305
-	select CRYPTO_LIB_BLAKE2S
 	select CRYPTO_CHACHA20_X86_64 if X86 && 64BIT
 	select CRYPTO_POLY1305_X86_64 if X86 && 64BIT
 	select CRYPTO_BLAKE2S_X86 if X86 && 64BIT
diff --git a/include/crypto/internal/blake2s.h b/include/crypto/internal/blake2s.h
index 8e50d487500f..d39cfa0d333e 100644
--- a/include/crypto/internal/blake2s.h
+++ b/include/crypto/internal/blake2s.h
@@ -11,11 +11,11 @@
 #include <crypto/internal/hash.h>
 #include <linux/string.h>
 
-void blake2s_compress_generic(struct blake2s_state *state,const u8 *block,
+void blake2s_compress_generic(struct blake2s_state *state, const u8 *block,
 			      size_t nblocks, const u32 inc);
 
-void blake2s_compress_arch(struct blake2s_state *state,const u8 *block,
-			   size_t nblocks, const u32 inc);
+void blake2s_compress(struct blake2s_state *state, const u8 *block,
+		      size_t nblocks, const u32 inc);
 
 bool blake2s_selftest(void);
 
diff --git a/lib/Makefile b/lib/Makefile
index 364c23f15578..bb57b2e466fa 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -139,7 +139,7 @@ endif
 obj-$(CONFIG_DEBUG_INFO_REDUCED) += debug_info.o
 CFLAGS_debug_info.o += $(call cc-option, -femit-struct-debug-detailed=any)
 
-obj-y += math/ crypto/
+obj-y += math/
 
 obj-$(CONFIG_GENERIC_IOMAP) += iomap.o
 obj-$(CONFIG_GENERIC_PCI_IOMAP) += pci_iomap.o
diff --git a/lib/crypto/Kconfig b/lib/crypto/Kconfig
index 545ccbddf6a1..31c6e2be3b84 100644
--- a/lib/crypto/Kconfig
+++ b/lib/crypto/Kconfig
@@ -8,31 +8,6 @@ config CRYPTO_LIB_AES
 config CRYPTO_LIB_ARC4
 	tristate
 
-config CRYPTO_ARCH_HAVE_LIB_BLAKE2S
-	tristate
-	help
-	  Declares whether the architecture provides an arch-specific
-	  accelerated implementation of the Blake2s library interface,
-	  either builtin or as a module.
-
-config CRYPTO_LIB_BLAKE2S_GENERIC
-	tristate
-	help
-	  This symbol can be depended upon by arch implementations of the
-	  Blake2s library interface that require the generic code as a
-	  fallback, e.g., for SIMD implementations. If no arch specific
-	  implementation is enabled, this implementation serves the users
-	  of CRYPTO_LIB_BLAKE2S.
-
-config CRYPTO_LIB_BLAKE2S
-	tristate "BLAKE2s hash function library"
-	depends on CRYPTO_ARCH_HAVE_LIB_BLAKE2S || !CRYPTO_ARCH_HAVE_LIB_BLAKE2S
-	select CRYPTO_LIB_BLAKE2S_GENERIC if CRYPTO_ARCH_HAVE_LIB_BLAKE2S=n
-	help
-	  Enable the Blake2s library interface. This interface may be fulfilled
-	  by either the generic implementation or an arch-specific one, if one
-	  is available and enabled.
-
 config CRYPTO_ARCH_HAVE_LIB_CHACHA
 	tristate
 	help
diff --git a/lib/crypto/Makefile b/lib/crypto/Makefile
index 73205ed269ba..42e1d932c077 100644
--- a/lib/crypto/Makefile
+++ b/lib/crypto/Makefile
@@ -10,10 +10,9 @@ libaes-y					:= aes.o
 obj-$(CONFIG_CRYPTO_LIB_ARC4)			+= libarc4.o
 libarc4-y					:= arc4.o
 
-obj-$(CONFIG_CRYPTO_LIB_BLAKE2S_GENERIC)	+= libblake2s-generic.o
-libblake2s-generic-y				+= blake2s-generic.o
-
-obj-$(CONFIG_CRYPTO_LIB_BLAKE2S)		+= libblake2s.o
+# blake2s is used by the /dev/random driver which is always builtin
+lib-y						+= blake2s-generic.o
+obj-y						+= libblake2s.o
 libblake2s-y					+= blake2s.o
 
 obj-$(CONFIG_CRYPTO_LIB_CHACHA20POLY1305)	+= libchacha20poly1305.o
diff --git a/lib/crypto/blake2s-generic.c b/lib/crypto/blake2s-generic.c
index 04ff8df24513..75ccb3e633e6 100644
--- a/lib/crypto/blake2s-generic.c
+++ b/lib/crypto/blake2s-generic.c
@@ -37,7 +37,11 @@ static inline void blake2s_increment_counter(struct blake2s_state *state,
 	state->t[1] += (state->t[0] < inc);
 }
 
-void blake2s_compress_generic(struct blake2s_state *state,const u8 *block,
+void blake2s_compress(struct blake2s_state *state, const u8 *block,
+		      size_t nblocks, const u32 inc)
+		      __weak __alias(blake2s_compress_generic);
+
+void blake2s_compress_generic(struct blake2s_state *state, const u8 *block,
 			      size_t nblocks, const u32 inc)
 {
 	u32 m[16];
diff --git a/lib/crypto/blake2s.c b/lib/crypto/blake2s.c
index 4055aa593ec4..93f2ae051370 100644
--- a/lib/crypto/blake2s.c
+++ b/lib/crypto/blake2s.c
@@ -16,12 +16,6 @@
 #include <linux/init.h>
 #include <linux/bug.h>
 
-#if IS_ENABLED(CONFIG_CRYPTO_ARCH_HAVE_LIB_BLAKE2S)
-#  define blake2s_compress blake2s_compress_arch
-#else
-#  define blake2s_compress blake2s_compress_generic
-#endif
-
 void blake2s_update(struct blake2s_state *state, const u8 *in, size_t inlen)
 {
 	__blake2s_update(state, in, inlen, blake2s_compress);
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* Re: [PATCH v2 1/2] lib/crypto: blake2s: include as built-in
  2021-12-23 14:11 [PATCH v2 1/2] lib/crypto: blake2s: include as built-in Jason A. Donenfeld
@ 2021-12-23 14:20 ` Ard Biesheuvel
  2021-12-24 13:35 ` Greg KH
  2021-12-25  9:26 ` Masahiro Yamada
  2 siblings, 0 replies; 23+ messages in thread
From: Ard Biesheuvel @ 2021-12-23 14:20 UTC (permalink / raw)
  To: Jason A. Donenfeld
  Cc: Linux Kernel Mailing List, Theodore Y. Ts'o,
	Greg Kroah-Hartman, Masahiro Yamada, Linux Kbuild mailing list,
	Herbert Xu, Linux Crypto Mailing List

On Thu, 23 Dec 2021 at 15:11, Jason A. Donenfeld <Jason@zx2c4.com> wrote:
>
> In preparation for using blake2s in the RNG, we change the way that it
> is wired-in to the build system. Instead of kconfig mazes and ifdefs, we
> use weak symbols, so that an arch version can override the generic
> version. Then we include the generic version in lib-y, so that it can be
> removed from the image if the arch version doesn't fallback to it (as is
> the case on arm though not x86). The result should be a bit simpler and
> smaller than the code it replaces.
>
> Cc: Ard Biesheuvel <ardb@kernel.org>
> Cc: Masahiro Yamada <masahiroy@kernel.org>
> Cc: linux-kbuild@vger.kernel.org
> Cc: Herbert Xu <herbert@gondor.apana.org.au>
> Cc: linux-crypto@vger.kernel.org
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
> ---
> Herbert - I intend to take this via the crng/random.git tree, since it
> forms a dependency and I'd like to send a pull early in 5.17 cycle.
>
>  Makefile                          |  2 +-
>  arch/arm/crypto/Kconfig           |  3 +--
>  arch/arm/crypto/blake2s-core.S    |  8 ++++----
>  arch/arm/crypto/blake2s-glue.c    |  6 +++---
>  arch/s390/configs/debug_defconfig |  1 -
>  arch/s390/configs/defconfig       |  1 -

You can drop these two hunks - not worth the risk of a conflict with
the S390 tree.

Other than that,

Acked-by: Ard Biesheuvel <ardb@kernel.org>

>  arch/x86/crypto/blake2s-glue.c    | 11 +++++------
>  crypto/Kconfig                    |  5 +----
>  drivers/net/Kconfig               |  1 -
>  include/crypto/internal/blake2s.h |  6 +++---
>  lib/Makefile                      |  2 +-
>  lib/crypto/Kconfig                | 25 -------------------------
>  lib/crypto/Makefile               |  7 +++----
>  lib/crypto/blake2s-generic.c      |  6 +++++-
>  lib/crypto/blake2s.c              |  6 ------
>  15 files changed, 27 insertions(+), 63 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index d85f1ff79f5c..892ea632ea63 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -668,7 +668,7 @@ drivers-y   := drivers/ sound/
>  drivers-$(CONFIG_SAMPLES) += samples/
>  drivers-$(CONFIG_NET) += net/
>  drivers-y      += virt/
> -libs-y         := lib/
> +libs-y         := lib/ lib/crypto/
>  endif # KBUILD_EXTMOD
>
>  # The all: target is the default when no target is given on the
> diff --git a/arch/arm/crypto/Kconfig b/arch/arm/crypto/Kconfig
> index 2b575792363e..47cb22645746 100644
> --- a/arch/arm/crypto/Kconfig
> +++ b/arch/arm/crypto/Kconfig
> @@ -63,8 +63,7 @@ config CRYPTO_SHA512_ARM
>           using optimized ARM assembler and NEON, when available.
>
>  config CRYPTO_BLAKE2S_ARM
> -       tristate "BLAKE2s digest algorithm (ARM)"
> -       select CRYPTO_ARCH_HAVE_LIB_BLAKE2S
> +       bool "BLAKE2s digest algorithm (ARM)"
>         help
>           BLAKE2s digest algorithm optimized with ARM scalar instructions.  This
>           is faster than the generic implementations of BLAKE2s and BLAKE2b, but
> diff --git a/arch/arm/crypto/blake2s-core.S b/arch/arm/crypto/blake2s-core.S
> index 86345751bbf3..df40e46601f1 100644
> --- a/arch/arm/crypto/blake2s-core.S
> +++ b/arch/arm/crypto/blake2s-core.S
> @@ -167,8 +167,8 @@
>  .endm
>
>  //
> -// void blake2s_compress_arch(struct blake2s_state *state,
> -//                           const u8 *block, size_t nblocks, u32 inc);
> +// void blake2s_compress(struct blake2s_state *state,
> +//                      const u8 *block, size_t nblocks, u32 inc);
>  //
>  // Only the first three fields of struct blake2s_state are used:
>  //     u32 h[8];       (inout)
> @@ -176,7 +176,7 @@
>  //     u32 f[2];       (in)
>  //
>         .align          5
> -ENTRY(blake2s_compress_arch)
> +ENTRY(blake2s_compress)
>         push            {r0-r2,r4-r11,lr}       // keep this an even number
>
>  .Lnext_block:
> @@ -303,4 +303,4 @@ ENTRY(blake2s_compress_arch)
>         str             r3, [r12], #4
>         bne             1b
>         b               .Lcopy_block_done
> -ENDPROC(blake2s_compress_arch)
> +ENDPROC(blake2s_compress)
> diff --git a/arch/arm/crypto/blake2s-glue.c b/arch/arm/crypto/blake2s-glue.c
> index f2cc1e5fc9ec..09d3a0cabd2c 100644
> --- a/arch/arm/crypto/blake2s-glue.c
> +++ b/arch/arm/crypto/blake2s-glue.c
> @@ -11,17 +11,17 @@
>  #include <linux/module.h>
>
>  /* defined in blake2s-core.S */
> -EXPORT_SYMBOL(blake2s_compress_arch);
> +EXPORT_SYMBOL(blake2s_compress);
>
>  static int crypto_blake2s_update_arm(struct shash_desc *desc,
>                                      const u8 *in, unsigned int inlen)
>  {
> -       return crypto_blake2s_update(desc, in, inlen, blake2s_compress_arch);
> +       return crypto_blake2s_update(desc, in, inlen, blake2s_compress);
>  }
>
>  static int crypto_blake2s_final_arm(struct shash_desc *desc, u8 *out)
>  {
> -       return crypto_blake2s_final(desc, out, blake2s_compress_arch);
> +       return crypto_blake2s_final(desc, out, blake2s_compress);
>  }
>
>  #define BLAKE2S_ALG(name, driver_name, digest_size)                    \
> diff --git a/arch/s390/configs/debug_defconfig b/arch/s390/configs/debug_defconfig
> index e45cc27716de..caa3d1d6a0e8 100644
> --- a/arch/s390/configs/debug_defconfig
> +++ b/arch/s390/configs/debug_defconfig
> @@ -757,7 +757,6 @@ CONFIG_CRYPTO_USER_API_SKCIPHER=m
>  CONFIG_CRYPTO_USER_API_RNG=m
>  CONFIG_CRYPTO_USER_API_AEAD=m
>  CONFIG_CRYPTO_STATS=y
> -CONFIG_CRYPTO_LIB_BLAKE2S=m
>  CONFIG_CRYPTO_LIB_CURVE25519=m
>  CONFIG_CRYPTO_LIB_CHACHA20POLY1305=m
>  CONFIG_ZCRYPT=m
> diff --git a/arch/s390/configs/defconfig b/arch/s390/configs/defconfig
> index 1c750bfca2d8..fffc6af5358c 100644
> --- a/arch/s390/configs/defconfig
> +++ b/arch/s390/configs/defconfig
> @@ -744,7 +744,6 @@ CONFIG_CRYPTO_USER_API_SKCIPHER=m
>  CONFIG_CRYPTO_USER_API_RNG=m
>  CONFIG_CRYPTO_USER_API_AEAD=m
>  CONFIG_CRYPTO_STATS=y
> -CONFIG_CRYPTO_LIB_BLAKE2S=m
>  CONFIG_CRYPTO_LIB_CURVE25519=m
>  CONFIG_CRYPTO_LIB_CHACHA20POLY1305=m
>  CONFIG_ZCRYPT=m
> diff --git a/arch/x86/crypto/blake2s-glue.c b/arch/x86/crypto/blake2s-glue.c
> index a40365ab301e..ef91a3167d27 100644
> --- a/arch/x86/crypto/blake2s-glue.c
> +++ b/arch/x86/crypto/blake2s-glue.c
> @@ -28,9 +28,8 @@ asmlinkage void blake2s_compress_avx512(struct blake2s_state *state,
>  static __ro_after_init DEFINE_STATIC_KEY_FALSE(blake2s_use_ssse3);
>  static __ro_after_init DEFINE_STATIC_KEY_FALSE(blake2s_use_avx512);
>
> -void blake2s_compress_arch(struct blake2s_state *state,
> -                          const u8 *block, size_t nblocks,
> -                          const u32 inc)
> +void blake2s_compress(struct blake2s_state *state, const u8 *block,
> +                     size_t nblocks, const u32 inc)
>  {
>         /* SIMD disables preemption, so relax after processing each page. */
>         BUILD_BUG_ON(SZ_4K / BLAKE2S_BLOCK_SIZE < 8);
> @@ -56,17 +55,17 @@ void blake2s_compress_arch(struct blake2s_state *state,
>                 block += blocks * BLAKE2S_BLOCK_SIZE;
>         } while (nblocks);
>  }
> -EXPORT_SYMBOL(blake2s_compress_arch);
> +EXPORT_SYMBOL(blake2s_compress);
>
>  static int crypto_blake2s_update_x86(struct shash_desc *desc,
>                                      const u8 *in, unsigned int inlen)
>  {
> -       return crypto_blake2s_update(desc, in, inlen, blake2s_compress_arch);
> +       return crypto_blake2s_update(desc, in, inlen, blake2s_compress);
>  }
>
>  static int crypto_blake2s_final_x86(struct shash_desc *desc, u8 *out)
>  {
> -       return crypto_blake2s_final(desc, out, blake2s_compress_arch);
> +       return crypto_blake2s_final(desc, out, blake2s_compress);
>  }
>
>  #define BLAKE2S_ALG(name, driver_name, digest_size)                    \
> diff --git a/crypto/Kconfig b/crypto/Kconfig
> index 285f82647d2b..bfda2c82774d 100644
> --- a/crypto/Kconfig
> +++ b/crypto/Kconfig
> @@ -685,7 +685,6 @@ config CRYPTO_BLAKE2B
>
>  config CRYPTO_BLAKE2S
>         tristate "BLAKE2s digest algorithm"
> -       select CRYPTO_LIB_BLAKE2S_GENERIC
>         select CRYPTO_HASH
>         help
>           Implementation of cryptographic hash function BLAKE2s
> @@ -702,10 +701,8 @@ config CRYPTO_BLAKE2S
>           See https://blake2.net for further information.
>
>  config CRYPTO_BLAKE2S_X86
> -       tristate "BLAKE2s digest algorithm (x86 accelerated version)"
> +       bool "BLAKE2s digest algorithm (x86 accelerated version)"
>         depends on X86 && 64BIT
> -       select CRYPTO_LIB_BLAKE2S_GENERIC
> -       select CRYPTO_ARCH_HAVE_LIB_BLAKE2S
>
>  config CRYPTO_CRCT10DIF
>         tristate "CRCT10DIF algorithm"
> diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
> index 6cccc3dc00bc..b2a4f998c180 100644
> --- a/drivers/net/Kconfig
> +++ b/drivers/net/Kconfig
> @@ -81,7 +81,6 @@ config WIREGUARD
>         select CRYPTO
>         select CRYPTO_LIB_CURVE25519
>         select CRYPTO_LIB_CHACHA20POLY1305
> -       select CRYPTO_LIB_BLAKE2S
>         select CRYPTO_CHACHA20_X86_64 if X86 && 64BIT
>         select CRYPTO_POLY1305_X86_64 if X86 && 64BIT
>         select CRYPTO_BLAKE2S_X86 if X86 && 64BIT
> diff --git a/include/crypto/internal/blake2s.h b/include/crypto/internal/blake2s.h
> index 8e50d487500f..d39cfa0d333e 100644
> --- a/include/crypto/internal/blake2s.h
> +++ b/include/crypto/internal/blake2s.h
> @@ -11,11 +11,11 @@
>  #include <crypto/internal/hash.h>
>  #include <linux/string.h>
>
> -void blake2s_compress_generic(struct blake2s_state *state,const u8 *block,
> +void blake2s_compress_generic(struct blake2s_state *state, const u8 *block,
>                               size_t nblocks, const u32 inc);
>
> -void blake2s_compress_arch(struct blake2s_state *state,const u8 *block,
> -                          size_t nblocks, const u32 inc);
> +void blake2s_compress(struct blake2s_state *state, const u8 *block,
> +                     size_t nblocks, const u32 inc);
>
>  bool blake2s_selftest(void);
>
> diff --git a/lib/Makefile b/lib/Makefile
> index 364c23f15578..bb57b2e466fa 100644
> --- a/lib/Makefile
> +++ b/lib/Makefile
> @@ -139,7 +139,7 @@ endif
>  obj-$(CONFIG_DEBUG_INFO_REDUCED) += debug_info.o
>  CFLAGS_debug_info.o += $(call cc-option, -femit-struct-debug-detailed=any)
>
> -obj-y += math/ crypto/
> +obj-y += math/
>
>  obj-$(CONFIG_GENERIC_IOMAP) += iomap.o
>  obj-$(CONFIG_GENERIC_PCI_IOMAP) += pci_iomap.o
> diff --git a/lib/crypto/Kconfig b/lib/crypto/Kconfig
> index 545ccbddf6a1..31c6e2be3b84 100644
> --- a/lib/crypto/Kconfig
> +++ b/lib/crypto/Kconfig
> @@ -8,31 +8,6 @@ config CRYPTO_LIB_AES
>  config CRYPTO_LIB_ARC4
>         tristate
>
> -config CRYPTO_ARCH_HAVE_LIB_BLAKE2S
> -       tristate
> -       help
> -         Declares whether the architecture provides an arch-specific
> -         accelerated implementation of the Blake2s library interface,
> -         either builtin or as a module.
> -
> -config CRYPTO_LIB_BLAKE2S_GENERIC
> -       tristate
> -       help
> -         This symbol can be depended upon by arch implementations of the
> -         Blake2s library interface that require the generic code as a
> -         fallback, e.g., for SIMD implementations. If no arch specific
> -         implementation is enabled, this implementation serves the users
> -         of CRYPTO_LIB_BLAKE2S.
> -
> -config CRYPTO_LIB_BLAKE2S
> -       tristate "BLAKE2s hash function library"
> -       depends on CRYPTO_ARCH_HAVE_LIB_BLAKE2S || !CRYPTO_ARCH_HAVE_LIB_BLAKE2S
> -       select CRYPTO_LIB_BLAKE2S_GENERIC if CRYPTO_ARCH_HAVE_LIB_BLAKE2S=n
> -       help
> -         Enable the Blake2s library interface. This interface may be fulfilled
> -         by either the generic implementation or an arch-specific one, if one
> -         is available and enabled.
> -
>  config CRYPTO_ARCH_HAVE_LIB_CHACHA
>         tristate
>         help
> diff --git a/lib/crypto/Makefile b/lib/crypto/Makefile
> index 73205ed269ba..42e1d932c077 100644
> --- a/lib/crypto/Makefile
> +++ b/lib/crypto/Makefile
> @@ -10,10 +10,9 @@ libaes-y                                     := aes.o
>  obj-$(CONFIG_CRYPTO_LIB_ARC4)                  += libarc4.o
>  libarc4-y                                      := arc4.o
>
> -obj-$(CONFIG_CRYPTO_LIB_BLAKE2S_GENERIC)       += libblake2s-generic.o
> -libblake2s-generic-y                           += blake2s-generic.o
> -
> -obj-$(CONFIG_CRYPTO_LIB_BLAKE2S)               += libblake2s.o
> +# blake2s is used by the /dev/random driver which is always builtin
> +lib-y                                          += blake2s-generic.o
> +obj-y                                          += libblake2s.o
>  libblake2s-y                                   += blake2s.o
>
>  obj-$(CONFIG_CRYPTO_LIB_CHACHA20POLY1305)      += libchacha20poly1305.o
> diff --git a/lib/crypto/blake2s-generic.c b/lib/crypto/blake2s-generic.c
> index 04ff8df24513..75ccb3e633e6 100644
> --- a/lib/crypto/blake2s-generic.c
> +++ b/lib/crypto/blake2s-generic.c
> @@ -37,7 +37,11 @@ static inline void blake2s_increment_counter(struct blake2s_state *state,
>         state->t[1] += (state->t[0] < inc);
>  }
>
> -void blake2s_compress_generic(struct blake2s_state *state,const u8 *block,
> +void blake2s_compress(struct blake2s_state *state, const u8 *block,
> +                     size_t nblocks, const u32 inc)
> +                     __weak __alias(blake2s_compress_generic);
> +
> +void blake2s_compress_generic(struct blake2s_state *state, const u8 *block,
>                               size_t nblocks, const u32 inc)
>  {
>         u32 m[16];
> diff --git a/lib/crypto/blake2s.c b/lib/crypto/blake2s.c
> index 4055aa593ec4..93f2ae051370 100644
> --- a/lib/crypto/blake2s.c
> +++ b/lib/crypto/blake2s.c
> @@ -16,12 +16,6 @@
>  #include <linux/init.h>
>  #include <linux/bug.h>
>
> -#if IS_ENABLED(CONFIG_CRYPTO_ARCH_HAVE_LIB_BLAKE2S)
> -#  define blake2s_compress blake2s_compress_arch
> -#else
> -#  define blake2s_compress blake2s_compress_generic
> -#endif
> -
>  void blake2s_update(struct blake2s_state *state, const u8 *in, size_t inlen)
>  {
>         __blake2s_update(state, in, inlen, blake2s_compress);
> --
> 2.34.1
>

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v2 1/2] lib/crypto: blake2s: include as built-in
  2021-12-23 14:11 [PATCH v2 1/2] lib/crypto: blake2s: include as built-in Jason A. Donenfeld
  2021-12-23 14:20 ` Ard Biesheuvel
@ 2021-12-24 13:35 ` Greg KH
  2021-12-25  9:26 ` Masahiro Yamada
  2 siblings, 0 replies; 23+ messages in thread
From: Greg KH @ 2021-12-24 13:35 UTC (permalink / raw)
  To: Jason A. Donenfeld
  Cc: linux-kernel, tytso, Ard Biesheuvel, Masahiro Yamada,
	linux-kbuild, Herbert Xu, linux-crypto

On Thu, Dec 23, 2021 at 03:11:12PM +0100, Jason A. Donenfeld wrote:
> In preparation for using blake2s in the RNG, we change the way that it
> is wired-in to the build system. Instead of kconfig mazes and ifdefs, we
> use weak symbols, so that an arch version can override the generic
> version. Then we include the generic version in lib-y, so that it can be
> removed from the image if the arch version doesn't fallback to it (as is
> the case on arm though not x86). The result should be a bit simpler and
> smaller than the code it replaces.
> 
> Cc: Ard Biesheuvel <ardb@kernel.org>
> Cc: Masahiro Yamada <masahiroy@kernel.org>
> Cc: linux-kbuild@vger.kernel.org
> Cc: Herbert Xu <herbert@gondor.apana.org.au>
> Cc: linux-crypto@vger.kernel.org
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
> ---
> Herbert - I intend to take this via the crng/random.git tree, since it
> forms a dependency and I'd like to send a pull early in 5.17 cycle.
> 
>  Makefile                          |  2 +-
>  arch/arm/crypto/Kconfig           |  3 +--
>  arch/arm/crypto/blake2s-core.S    |  8 ++++----
>  arch/arm/crypto/blake2s-glue.c    |  6 +++---
>  arch/s390/configs/debug_defconfig |  1 -
>  arch/s390/configs/defconfig       |  1 -
>  arch/x86/crypto/blake2s-glue.c    | 11 +++++------
>  crypto/Kconfig                    |  5 +----
>  drivers/net/Kconfig               |  1 -
>  include/crypto/internal/blake2s.h |  6 +++---
>  lib/Makefile                      |  2 +-
>  lib/crypto/Kconfig                | 25 -------------------------
>  lib/crypto/Makefile               |  7 +++----
>  lib/crypto/blake2s-generic.c      |  6 +++++-
>  lib/crypto/blake2s.c              |  6 ------
>  15 files changed, 27 insertions(+), 63 deletions(-)

Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v2 1/2] lib/crypto: blake2s: include as built-in
  2021-12-23 14:11 [PATCH v2 1/2] lib/crypto: blake2s: include as built-in Jason A. Donenfeld
  2021-12-23 14:20 ` Ard Biesheuvel
  2021-12-24 13:35 ` Greg KH
@ 2021-12-25  9:26 ` Masahiro Yamada
  2021-12-25 10:26   ` Ard Biesheuvel
  2 siblings, 1 reply; 23+ messages in thread
From: Masahiro Yamada @ 2021-12-25  9:26 UTC (permalink / raw)
  To: Jason A. Donenfeld
  Cc: Linux Kernel Mailing List, Theodore Ts'o, Greg Kroah-Hartman,
	Ard Biesheuvel, Linux Kbuild mailing list, Herbert Xu,
	Linux Crypto Mailing List

On Thu, Dec 23, 2021 at 11:11 PM Jason A. Donenfeld <Jason@zx2c4.com> wrote:
>
> In preparation for using blake2s in the RNG, we change the way that it
> is wired-in to the build system. Instead of kconfig mazes and ifdefs, we
> use weak symbols, so that an arch version can override the generic
> version. Then we include the generic version in lib-y, so that it can be
> removed from the image if the arch version doesn't fallback to it (as is
> the case on arm though not x86).


As I replied in another email, this does not work like that.

Since 7273ad2b08f8ac9563579d16a3cf528857b26f49,
libs-y are all linked when CONFIG_MODULES=y.



So, what this patch is doing are:

 - Add __weak to the generic function
 - Make modules into built-in.


Both generic functions and ARM-specific ones
will remain in vmlinux.

__weak makes it difficult to track which function is
actually used.
Using #ifdef CONFIG_* (as the current code does)
is better.



>
> diff --git a/Makefile b/Makefile
> index d85f1ff79f5c..892ea632ea63 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -668,7 +668,7 @@ drivers-y   := drivers/ sound/
>  drivers-$(CONFIG_SAMPLES) += samples/
>  drivers-$(CONFIG_NET) += net/
>  drivers-y      += virt/
> -libs-y         := lib/
> +libs-y         := lib/ lib/crypto/


If this is merged, someone will try to
add random patterns.
libs-y         := lib/ lib/crypto/ lib/foo/bar/ lib/foo/ba/baz



lib-y and libs-y are a bad idea in the first place
and should not be extended any more.

Since this patch is not working as the commit description
claims, and it is going in the bad direction, so

NACK




-- 
Best Regards
Masahiro Yamada

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v2 1/2] lib/crypto: blake2s: include as built-in
  2021-12-25  9:26 ` Masahiro Yamada
@ 2021-12-25 10:26   ` Ard Biesheuvel
  2021-12-25 15:47     ` Masahiro Yamada
  0 siblings, 1 reply; 23+ messages in thread
From: Ard Biesheuvel @ 2021-12-25 10:26 UTC (permalink / raw)
  To: Masahiro Yamada
  Cc: Jason A. Donenfeld, Linux Kernel Mailing List, Theodore Ts'o,
	Greg Kroah-Hartman, Linux Kbuild mailing list, Herbert Xu,
	Linux Crypto Mailing List

On Sat, 25 Dec 2021 at 10:28, Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> On Thu, Dec 23, 2021 at 11:11 PM Jason A. Donenfeld <Jason@zx2c4.com> wrote:
> >
> > In preparation for using blake2s in the RNG, we change the way that it
> > is wired-in to the build system. Instead of kconfig mazes and ifdefs, we
> > use weak symbols, so that an arch version can override the generic
> > version. Then we include the generic version in lib-y, so that it can be
> > removed from the image if the arch version doesn't fallback to it (as is
> > the case on arm though not x86).
>
>
> As I replied in another email, this does not work like that.
>
> Since 7273ad2b08f8ac9563579d16a3cf528857b26f49,
> libs-y are all linked when CONFIG_MODULES=y.
>
>
>
> So, what this patch is doing are:
>
>  - Add __weak to the generic function
>  - Make modules into built-in.
>
>
> Both generic functions and ARM-specific ones
> will remain in vmlinux.
>
> __weak makes it difficult to track which function is
> actually used.
> Using #ifdef CONFIG_* (as the current code does)
> is better.
>
>
>
> >
> > diff --git a/Makefile b/Makefile
> > index d85f1ff79f5c..892ea632ea63 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -668,7 +668,7 @@ drivers-y   := drivers/ sound/
> >  drivers-$(CONFIG_SAMPLES) += samples/
> >  drivers-$(CONFIG_NET) += net/
> >  drivers-y      += virt/
> > -libs-y         := lib/
> > +libs-y         := lib/ lib/crypto/
>
>
> If this is merged, someone will try to
> add random patterns.
> libs-y         := lib/ lib/crypto/ lib/foo/bar/ lib/foo/ba/baz
>
>
>
> lib-y and libs-y are a bad idea in the first place
> and should not be extended any more.
>
> Since this patch is not working as the commit description
> claims, and it is going in the bad direction, so
>
> NACK
>

So we are no longer permitted to use static libraries to provide
routines that should only be pulled into vmlinux on demand? Has this
also changed for things like string routines etc?

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v2 1/2] lib/crypto: blake2s: include as built-in
  2021-12-25 10:26   ` Ard Biesheuvel
@ 2021-12-25 15:47     ` Masahiro Yamada
  2021-12-27 13:43       ` Jason A. Donenfeld
  0 siblings, 1 reply; 23+ messages in thread
From: Masahiro Yamada @ 2021-12-25 15:47 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: Jason A. Donenfeld, Linux Kernel Mailing List, Theodore Ts'o,
	Greg Kroah-Hartman, Linux Kbuild mailing list, Herbert Xu,
	Linux Crypto Mailing List

On Sat, Dec 25, 2021 at 7:26 PM Ard Biesheuvel <ardb@kernel.org> wrote:
>
> On Sat, 25 Dec 2021 at 10:28, Masahiro Yamada <masahiroy@kernel.org> wrote:
> >
> > On Thu, Dec 23, 2021 at 11:11 PM Jason A. Donenfeld <Jason@zx2c4.com> wrote:
> > >
> > > In preparation for using blake2s in the RNG, we change the way that it
> > > is wired-in to the build system. Instead of kconfig mazes and ifdefs, we
> > > use weak symbols, so that an arch version can override the generic
> > > version. Then we include the generic version in lib-y, so that it can be
> > > removed from the image if the arch version doesn't fallback to it (as is
> > > the case on arm though not x86).
> >
> >
> > As I replied in another email, this does not work like that.
> >
> > Since 7273ad2b08f8ac9563579d16a3cf528857b26f49,
> > libs-y are all linked when CONFIG_MODULES=y.
> >
> >
> >
> > So, what this patch is doing are:
> >
> >  - Add __weak to the generic function
> >  - Make modules into built-in.
> >
> >
> > Both generic functions and ARM-specific ones
> > will remain in vmlinux.
> >
> > __weak makes it difficult to track which function is
> > actually used.
> > Using #ifdef CONFIG_* (as the current code does)
> > is better.
> >
> >
> >
> > >
> > > diff --git a/Makefile b/Makefile
> > > index d85f1ff79f5c..892ea632ea63 100644
> > > --- a/Makefile
> > > +++ b/Makefile
> > > @@ -668,7 +668,7 @@ drivers-y   := drivers/ sound/
> > >  drivers-$(CONFIG_SAMPLES) += samples/
> > >  drivers-$(CONFIG_NET) += net/
> > >  drivers-y      += virt/
> > > -libs-y         := lib/
> > > +libs-y         := lib/ lib/crypto/
> >
> >
> > If this is merged, someone will try to
> > add random patterns.
> > libs-y         := lib/ lib/crypto/ lib/foo/bar/ lib/foo/ba/baz
> >
> >
> >
> > lib-y and libs-y are a bad idea in the first place
> > and should not be extended any more.
> >
> > Since this patch is not working as the commit description
> > claims, and it is going in the bad direction, so
> >
> > NACK
> >
>
> So we are no longer permitted to use static libraries to provide
> routines that should only be pulled into vmlinux on demand? Has this
> also changed for things like string routines etc?

Utility functions such as string routines are intended to be used
anywhere on demand, not only in vmlinux but also in loadable
modules.

Therefore, such functions are very likely to be EXPORT_SYMBOL'ed.
As a matter of fact, most of the files listed in lib-y
contain EXPORT_SYMBOL.

Historically, static libraries did not work well with EXPORT_SYMBOL.

Originally, lib-y dropped functions that had no callsite in vmlinux, but
it was a wrong behavior. We must always keep exported functions, which
might be used by modules, even if not by vmlinux.

7f2084fa55e6cb61f61b4224d4a8bafaeee55f9f
added a workaround so that all of EXPORT_SYMBOL
are considered "referenced".

Since then, most of lib-y objects were linked anyway,
given the following:

- Most of *.c files listed in lib-y contain at least one EXPORT_SYMBOL
- In static library, if any one of symbol is referenced, the entire object
  is linked

So, lib-y was not helpful for reducing the kernel image size.

The exceptional cases are CONFIG_MODULES=n
or CONFIG_TRIM_UNUSED_KSYMS=y, but neither of
them is a common use-case.

To remove unused functions,
CONFIG_LD_DEAD_CODE_DATA_ELIMINATION (per-symbol
removal) seems to be a more sensible solution to me.



-- 
Best Regards
Masahiro Yamada

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v2 1/2] lib/crypto: blake2s: include as built-in
  2021-12-25 15:47     ` Masahiro Yamada
@ 2021-12-27 13:43       ` Jason A. Donenfeld
  2021-12-27 13:47         ` [PATCH v3] " Jason A. Donenfeld
  0 siblings, 1 reply; 23+ messages in thread
From: Jason A. Donenfeld @ 2021-12-27 13:43 UTC (permalink / raw)
  To: Masahiro Yamada
  Cc: Ard Biesheuvel, Linux Kernel Mailing List, Theodore Ts'o,
	Greg Kroah-Hartman, Linux Kbuild mailing list, Herbert Xu,
	Linux Crypto Mailing List

Hi Masahiro,

Thanks for your feedback. Indeed it looks like you're right about the
CONFIG_MODULE case. We'll go back to using the kconfig system
normally, and cease tempting the beast with libs-y and such. I'll have
a v+1 for you shortly in case you're curious, though I expect it to be
sufficiently boring to be no longer worth your time :-).

Jason

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [PATCH v3] lib/crypto: blake2s: include as built-in
  2021-12-27 13:43       ` Jason A. Donenfeld
@ 2021-12-27 13:47         ` Jason A. Donenfeld
  2021-12-27 14:20           ` [PATCH v4] " Jason A. Donenfeld
  0 siblings, 1 reply; 23+ messages in thread
From: Jason A. Donenfeld @ 2021-12-27 13:47 UTC (permalink / raw)
  To: linux-kernel
  Cc: Jason A. Donenfeld, Ard Biesheuvel, Greg Kroah-Hartman,
	Masahiro Yamada, linux-kbuild, Herbert Xu, linux-crypto

In preparation for using blake2s in the RNG, we change the way that it
is wired-in to the build system. Instead of using ifdefs to select the
right symbol, we use weak symbols. And because ARM doesn't need the
generic implementation, we make the generic one default only if an arch
library doesn't need it already, and then have arch libraries that do
need it opt-in.

Acked-by: Ard Biesheuvel <ardb@kernel.org>
Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Masahiro Yamada <masahiroy@kernel.org>
Cc: linux-kbuild@vger.kernel.org
Cc: Herbert Xu <herbert@gondor.apana.org.au>
Cc: linux-crypto@vger.kernel.org
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
---
Herbert - As mentioned with the vPrev, I intend to take this via the
crng/random.git tree, since it forms a dependency and I'd like to send a
pull early in 5.17 cycle.

Changes v2->v3:
- Rather than using lib-y, use obj-y, and retain the kconfig symbols
  for selection.

 arch/arm/crypto/blake2s-core.S    |  8 ++++----
 arch/arm/crypto/blake2s-glue.c    |  6 +++---
 arch/x86/crypto/blake2s-glue.c    | 11 +++++------
 crypto/Kconfig                    |  1 -
 drivers/net/Kconfig               |  1 -
 include/crypto/internal/blake2s.h |  6 +++---
 lib/crypto/Kconfig                | 13 ++-----------
 lib/crypto/Makefile               |  9 ++++-----
 lib/crypto/blake2s-generic.c      |  6 +++++-
 lib/crypto/blake2s.c              |  6 ------
 10 files changed, 26 insertions(+), 41 deletions(-)

diff --git a/arch/arm/crypto/blake2s-core.S b/arch/arm/crypto/blake2s-core.S
index 86345751bbf3..df40e46601f1 100644
--- a/arch/arm/crypto/blake2s-core.S
+++ b/arch/arm/crypto/blake2s-core.S
@@ -167,8 +167,8 @@
 .endm
 
 //
-// void blake2s_compress_arch(struct blake2s_state *state,
-//			      const u8 *block, size_t nblocks, u32 inc);
+// void blake2s_compress(struct blake2s_state *state,
+//			 const u8 *block, size_t nblocks, u32 inc);
 //
 // Only the first three fields of struct blake2s_state are used:
 //	u32 h[8];	(inout)
@@ -176,7 +176,7 @@
 //	u32 f[2];	(in)
 //
 	.align		5
-ENTRY(blake2s_compress_arch)
+ENTRY(blake2s_compress)
 	push		{r0-r2,r4-r11,lr}	// keep this an even number
 
 .Lnext_block:
@@ -303,4 +303,4 @@ ENTRY(blake2s_compress_arch)
 	str		r3, [r12], #4
 	bne		1b
 	b		.Lcopy_block_done
-ENDPROC(blake2s_compress_arch)
+ENDPROC(blake2s_compress)
diff --git a/arch/arm/crypto/blake2s-glue.c b/arch/arm/crypto/blake2s-glue.c
index f2cc1e5fc9ec..09d3a0cabd2c 100644
--- a/arch/arm/crypto/blake2s-glue.c
+++ b/arch/arm/crypto/blake2s-glue.c
@@ -11,17 +11,17 @@
 #include <linux/module.h>
 
 /* defined in blake2s-core.S */
-EXPORT_SYMBOL(blake2s_compress_arch);
+EXPORT_SYMBOL(blake2s_compress);
 
 static int crypto_blake2s_update_arm(struct shash_desc *desc,
 				     const u8 *in, unsigned int inlen)
 {
-	return crypto_blake2s_update(desc, in, inlen, blake2s_compress_arch);
+	return crypto_blake2s_update(desc, in, inlen, blake2s_compress);
 }
 
 static int crypto_blake2s_final_arm(struct shash_desc *desc, u8 *out)
 {
-	return crypto_blake2s_final(desc, out, blake2s_compress_arch);
+	return crypto_blake2s_final(desc, out, blake2s_compress);
 }
 
 #define BLAKE2S_ALG(name, driver_name, digest_size)			\
diff --git a/arch/x86/crypto/blake2s-glue.c b/arch/x86/crypto/blake2s-glue.c
index a40365ab301e..ef91a3167d27 100644
--- a/arch/x86/crypto/blake2s-glue.c
+++ b/arch/x86/crypto/blake2s-glue.c
@@ -28,9 +28,8 @@ asmlinkage void blake2s_compress_avx512(struct blake2s_state *state,
 static __ro_after_init DEFINE_STATIC_KEY_FALSE(blake2s_use_ssse3);
 static __ro_after_init DEFINE_STATIC_KEY_FALSE(blake2s_use_avx512);
 
-void blake2s_compress_arch(struct blake2s_state *state,
-			   const u8 *block, size_t nblocks,
-			   const u32 inc)
+void blake2s_compress(struct blake2s_state *state, const u8 *block,
+		      size_t nblocks, const u32 inc)
 {
 	/* SIMD disables preemption, so relax after processing each page. */
 	BUILD_BUG_ON(SZ_4K / BLAKE2S_BLOCK_SIZE < 8);
@@ -56,17 +55,17 @@ void blake2s_compress_arch(struct blake2s_state *state,
 		block += blocks * BLAKE2S_BLOCK_SIZE;
 	} while (nblocks);
 }
-EXPORT_SYMBOL(blake2s_compress_arch);
+EXPORT_SYMBOL(blake2s_compress);
 
 static int crypto_blake2s_update_x86(struct shash_desc *desc,
 				     const u8 *in, unsigned int inlen)
 {
-	return crypto_blake2s_update(desc, in, inlen, blake2s_compress_arch);
+	return crypto_blake2s_update(desc, in, inlen, blake2s_compress);
 }
 
 static int crypto_blake2s_final_x86(struct shash_desc *desc, u8 *out)
 {
-	return crypto_blake2s_final(desc, out, blake2s_compress_arch);
+	return crypto_blake2s_final(desc, out, blake2s_compress);
 }
 
 #define BLAKE2S_ALG(name, driver_name, digest_size)			\
diff --git a/crypto/Kconfig b/crypto/Kconfig
index 285f82647d2b..f2cb34515afa 100644
--- a/crypto/Kconfig
+++ b/crypto/Kconfig
@@ -685,7 +685,6 @@ config CRYPTO_BLAKE2B
 
 config CRYPTO_BLAKE2S
 	tristate "BLAKE2s digest algorithm"
-	select CRYPTO_LIB_BLAKE2S_GENERIC
 	select CRYPTO_HASH
 	help
 	  Implementation of cryptographic hash function BLAKE2s
diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
index 6cccc3dc00bc..b2a4f998c180 100644
--- a/drivers/net/Kconfig
+++ b/drivers/net/Kconfig
@@ -81,7 +81,6 @@ config WIREGUARD
 	select CRYPTO
 	select CRYPTO_LIB_CURVE25519
 	select CRYPTO_LIB_CHACHA20POLY1305
-	select CRYPTO_LIB_BLAKE2S
 	select CRYPTO_CHACHA20_X86_64 if X86 && 64BIT
 	select CRYPTO_POLY1305_X86_64 if X86 && 64BIT
 	select CRYPTO_BLAKE2S_X86 if X86 && 64BIT
diff --git a/include/crypto/internal/blake2s.h b/include/crypto/internal/blake2s.h
index 8e50d487500f..d39cfa0d333e 100644
--- a/include/crypto/internal/blake2s.h
+++ b/include/crypto/internal/blake2s.h
@@ -11,11 +11,11 @@
 #include <crypto/internal/hash.h>
 #include <linux/string.h>
 
-void blake2s_compress_generic(struct blake2s_state *state,const u8 *block,
+void blake2s_compress_generic(struct blake2s_state *state, const u8 *block,
 			      size_t nblocks, const u32 inc);
 
-void blake2s_compress_arch(struct blake2s_state *state,const u8 *block,
-			   size_t nblocks, const u32 inc);
+void blake2s_compress(struct blake2s_state *state, const u8 *block,
+		      size_t nblocks, const u32 inc);
 
 bool blake2s_selftest(void);
 
diff --git a/lib/crypto/Kconfig b/lib/crypto/Kconfig
index 545ccbddf6a1..0f27976b5038 100644
--- a/lib/crypto/Kconfig
+++ b/lib/crypto/Kconfig
@@ -9,14 +9,14 @@ config CRYPTO_LIB_ARC4
 	tristate
 
 config CRYPTO_ARCH_HAVE_LIB_BLAKE2S
-	tristate
+	bool
 	help
 	  Declares whether the architecture provides an arch-specific
 	  accelerated implementation of the Blake2s library interface,
 	  either builtin or as a module.
 
 config CRYPTO_LIB_BLAKE2S_GENERIC
-	tristate
+	def_bool !CRYPTO_ARCH_HAVE_LIB_BLAKE2S
 	help
 	  This symbol can be depended upon by arch implementations of the
 	  Blake2s library interface that require the generic code as a
@@ -24,15 +24,6 @@ config CRYPTO_LIB_BLAKE2S_GENERIC
 	  implementation is enabled, this implementation serves the users
 	  of CRYPTO_LIB_BLAKE2S.
 
-config CRYPTO_LIB_BLAKE2S
-	tristate "BLAKE2s hash function library"
-	depends on CRYPTO_ARCH_HAVE_LIB_BLAKE2S || !CRYPTO_ARCH_HAVE_LIB_BLAKE2S
-	select CRYPTO_LIB_BLAKE2S_GENERIC if CRYPTO_ARCH_HAVE_LIB_BLAKE2S=n
-	help
-	  Enable the Blake2s library interface. This interface may be fulfilled
-	  by either the generic implementation or an arch-specific one, if one
-	  is available and enabled.
-
 config CRYPTO_ARCH_HAVE_LIB_CHACHA
 	tristate
 	help
diff --git a/lib/crypto/Makefile b/lib/crypto/Makefile
index 73205ed269ba..ed43a41f2dcc 100644
--- a/lib/crypto/Makefile
+++ b/lib/crypto/Makefile
@@ -10,11 +10,10 @@ libaes-y					:= aes.o
 obj-$(CONFIG_CRYPTO_LIB_ARC4)			+= libarc4.o
 libarc4-y					:= arc4.o
 
-obj-$(CONFIG_CRYPTO_LIB_BLAKE2S_GENERIC)	+= libblake2s-generic.o
-libblake2s-generic-y				+= blake2s-generic.o
-
-obj-$(CONFIG_CRYPTO_LIB_BLAKE2S)		+= libblake2s.o
-libblake2s-y					+= blake2s.o
+# blake2s is used by the /dev/random driver which is always builtin
+obj-y						+= libblake2s.o
+libblake2s-y					:= blake2s.o
+libblake2s-$(CONFIG_CRYPTO_LIB_BLAKE2S_GENERIC)	+= blake2s-generic.o
 
 obj-$(CONFIG_CRYPTO_LIB_CHACHA20POLY1305)	+= libchacha20poly1305.o
 libchacha20poly1305-y				+= chacha20poly1305.o
diff --git a/lib/crypto/blake2s-generic.c b/lib/crypto/blake2s-generic.c
index 04ff8df24513..75ccb3e633e6 100644
--- a/lib/crypto/blake2s-generic.c
+++ b/lib/crypto/blake2s-generic.c
@@ -37,7 +37,11 @@ static inline void blake2s_increment_counter(struct blake2s_state *state,
 	state->t[1] += (state->t[0] < inc);
 }
 
-void blake2s_compress_generic(struct blake2s_state *state,const u8 *block,
+void blake2s_compress(struct blake2s_state *state, const u8 *block,
+		      size_t nblocks, const u32 inc)
+		      __weak __alias(blake2s_compress_generic);
+
+void blake2s_compress_generic(struct blake2s_state *state, const u8 *block,
 			      size_t nblocks, const u32 inc)
 {
 	u32 m[16];
diff --git a/lib/crypto/blake2s.c b/lib/crypto/blake2s.c
index 4055aa593ec4..93f2ae051370 100644
--- a/lib/crypto/blake2s.c
+++ b/lib/crypto/blake2s.c
@@ -16,12 +16,6 @@
 #include <linux/init.h>
 #include <linux/bug.h>
 
-#if IS_ENABLED(CONFIG_CRYPTO_ARCH_HAVE_LIB_BLAKE2S)
-#  define blake2s_compress blake2s_compress_arch
-#else
-#  define blake2s_compress blake2s_compress_generic
-#endif
-
 void blake2s_update(struct blake2s_state *state, const u8 *in, size_t inlen)
 {
 	__blake2s_update(state, in, inlen, blake2s_compress);
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v4] lib/crypto: blake2s: include as built-in
  2021-12-27 13:47         ` [PATCH v3] " Jason A. Donenfeld
@ 2021-12-27 14:20           ` Jason A. Donenfeld
  2022-01-01 15:59             ` [PATCH v5] " Jason A. Donenfeld
  0 siblings, 1 reply; 23+ messages in thread
From: Jason A. Donenfeld @ 2021-12-27 14:20 UTC (permalink / raw)
  To: linux-kernel
  Cc: Jason A. Donenfeld, Ard Biesheuvel, Greg Kroah-Hartman,
	Masahiro Yamada, linux-kbuild, Herbert Xu, linux-crypto

In preparation for using blake2s in the RNG, we change the way that it
is wired-in to the build system. Instead of using ifdefs to select the
right symbol, we use weak symbols. And because ARM doesn't need the
generic implementation, we make the generic one default only if an arch
library doesn't need it already, and then have arch libraries that do
need it opt-in.

Acked-by: Ard Biesheuvel <ardb@kernel.org>
Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Masahiro Yamada <masahiroy@kernel.org>
Cc: linux-kbuild@vger.kernel.org
Cc: Herbert Xu <herbert@gondor.apana.org.au>
Cc: linux-crypto@vger.kernel.org
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
---
Herbert - As mentioned with the vPrev, I intend to take this via the
crng/random.git tree, since it forms a dependency and I'd like to send a
pull early in 5.17 cycle.

Changes v3->v4:
- Keep the generic one for the generic shash implementation.
Changes v2->v3:
- Rather than using lib-y, use obj-y, and retain the kconfig symbols
  for selection.

 arch/arm/crypto/blake2s-core.S    |  8 ++++----
 arch/arm/crypto/blake2s-glue.c    |  6 +++---
 arch/x86/crypto/blake2s-glue.c    | 11 +++++------
 drivers/net/Kconfig               |  1 -
 include/crypto/internal/blake2s.h |  6 +++---
 lib/crypto/Kconfig                | 13 ++-----------
 lib/crypto/Makefile               |  9 ++++-----
 lib/crypto/blake2s-generic.c      |  6 +++++-
 lib/crypto/blake2s.c              |  6 ------
 9 files changed, 26 insertions(+), 40 deletions(-)

diff --git a/arch/arm/crypto/blake2s-core.S b/arch/arm/crypto/blake2s-core.S
index 86345751bbf3..df40e46601f1 100644
--- a/arch/arm/crypto/blake2s-core.S
+++ b/arch/arm/crypto/blake2s-core.S
@@ -167,8 +167,8 @@
 .endm
 
 //
-// void blake2s_compress_arch(struct blake2s_state *state,
-//			      const u8 *block, size_t nblocks, u32 inc);
+// void blake2s_compress(struct blake2s_state *state,
+//			 const u8 *block, size_t nblocks, u32 inc);
 //
 // Only the first three fields of struct blake2s_state are used:
 //	u32 h[8];	(inout)
@@ -176,7 +176,7 @@
 //	u32 f[2];	(in)
 //
 	.align		5
-ENTRY(blake2s_compress_arch)
+ENTRY(blake2s_compress)
 	push		{r0-r2,r4-r11,lr}	// keep this an even number
 
 .Lnext_block:
@@ -303,4 +303,4 @@ ENTRY(blake2s_compress_arch)
 	str		r3, [r12], #4
 	bne		1b
 	b		.Lcopy_block_done
-ENDPROC(blake2s_compress_arch)
+ENDPROC(blake2s_compress)
diff --git a/arch/arm/crypto/blake2s-glue.c b/arch/arm/crypto/blake2s-glue.c
index f2cc1e5fc9ec..09d3a0cabd2c 100644
--- a/arch/arm/crypto/blake2s-glue.c
+++ b/arch/arm/crypto/blake2s-glue.c
@@ -11,17 +11,17 @@
 #include <linux/module.h>
 
 /* defined in blake2s-core.S */
-EXPORT_SYMBOL(blake2s_compress_arch);
+EXPORT_SYMBOL(blake2s_compress);
 
 static int crypto_blake2s_update_arm(struct shash_desc *desc,
 				     const u8 *in, unsigned int inlen)
 {
-	return crypto_blake2s_update(desc, in, inlen, blake2s_compress_arch);
+	return crypto_blake2s_update(desc, in, inlen, blake2s_compress);
 }
 
 static int crypto_blake2s_final_arm(struct shash_desc *desc, u8 *out)
 {
-	return crypto_blake2s_final(desc, out, blake2s_compress_arch);
+	return crypto_blake2s_final(desc, out, blake2s_compress);
 }
 
 #define BLAKE2S_ALG(name, driver_name, digest_size)			\
diff --git a/arch/x86/crypto/blake2s-glue.c b/arch/x86/crypto/blake2s-glue.c
index a40365ab301e..ef91a3167d27 100644
--- a/arch/x86/crypto/blake2s-glue.c
+++ b/arch/x86/crypto/blake2s-glue.c
@@ -28,9 +28,8 @@ asmlinkage void blake2s_compress_avx512(struct blake2s_state *state,
 static __ro_after_init DEFINE_STATIC_KEY_FALSE(blake2s_use_ssse3);
 static __ro_after_init DEFINE_STATIC_KEY_FALSE(blake2s_use_avx512);
 
-void blake2s_compress_arch(struct blake2s_state *state,
-			   const u8 *block, size_t nblocks,
-			   const u32 inc)
+void blake2s_compress(struct blake2s_state *state, const u8 *block,
+		      size_t nblocks, const u32 inc)
 {
 	/* SIMD disables preemption, so relax after processing each page. */
 	BUILD_BUG_ON(SZ_4K / BLAKE2S_BLOCK_SIZE < 8);
@@ -56,17 +55,17 @@ void blake2s_compress_arch(struct blake2s_state *state,
 		block += blocks * BLAKE2S_BLOCK_SIZE;
 	} while (nblocks);
 }
-EXPORT_SYMBOL(blake2s_compress_arch);
+EXPORT_SYMBOL(blake2s_compress);
 
 static int crypto_blake2s_update_x86(struct shash_desc *desc,
 				     const u8 *in, unsigned int inlen)
 {
-	return crypto_blake2s_update(desc, in, inlen, blake2s_compress_arch);
+	return crypto_blake2s_update(desc, in, inlen, blake2s_compress);
 }
 
 static int crypto_blake2s_final_x86(struct shash_desc *desc, u8 *out)
 {
-	return crypto_blake2s_final(desc, out, blake2s_compress_arch);
+	return crypto_blake2s_final(desc, out, blake2s_compress);
 }
 
 #define BLAKE2S_ALG(name, driver_name, digest_size)			\
diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
index 6cccc3dc00bc..b2a4f998c180 100644
--- a/drivers/net/Kconfig
+++ b/drivers/net/Kconfig
@@ -81,7 +81,6 @@ config WIREGUARD
 	select CRYPTO
 	select CRYPTO_LIB_CURVE25519
 	select CRYPTO_LIB_CHACHA20POLY1305
-	select CRYPTO_LIB_BLAKE2S
 	select CRYPTO_CHACHA20_X86_64 if X86 && 64BIT
 	select CRYPTO_POLY1305_X86_64 if X86 && 64BIT
 	select CRYPTO_BLAKE2S_X86 if X86 && 64BIT
diff --git a/include/crypto/internal/blake2s.h b/include/crypto/internal/blake2s.h
index 8e50d487500f..d39cfa0d333e 100644
--- a/include/crypto/internal/blake2s.h
+++ b/include/crypto/internal/blake2s.h
@@ -11,11 +11,11 @@
 #include <crypto/internal/hash.h>
 #include <linux/string.h>
 
-void blake2s_compress_generic(struct blake2s_state *state,const u8 *block,
+void blake2s_compress_generic(struct blake2s_state *state, const u8 *block,
 			      size_t nblocks, const u32 inc);
 
-void blake2s_compress_arch(struct blake2s_state *state,const u8 *block,
-			   size_t nblocks, const u32 inc);
+void blake2s_compress(struct blake2s_state *state, const u8 *block,
+		      size_t nblocks, const u32 inc);
 
 bool blake2s_selftest(void);
 
diff --git a/lib/crypto/Kconfig b/lib/crypto/Kconfig
index 545ccbddf6a1..0f27976b5038 100644
--- a/lib/crypto/Kconfig
+++ b/lib/crypto/Kconfig
@@ -9,14 +9,14 @@ config CRYPTO_LIB_ARC4
 	tristate
 
 config CRYPTO_ARCH_HAVE_LIB_BLAKE2S
-	tristate
+	bool
 	help
 	  Declares whether the architecture provides an arch-specific
 	  accelerated implementation of the Blake2s library interface,
 	  either builtin or as a module.
 
 config CRYPTO_LIB_BLAKE2S_GENERIC
-	tristate
+	def_bool !CRYPTO_ARCH_HAVE_LIB_BLAKE2S
 	help
 	  This symbol can be depended upon by arch implementations of the
 	  Blake2s library interface that require the generic code as a
@@ -24,15 +24,6 @@ config CRYPTO_LIB_BLAKE2S_GENERIC
 	  implementation is enabled, this implementation serves the users
 	  of CRYPTO_LIB_BLAKE2S.
 
-config CRYPTO_LIB_BLAKE2S
-	tristate "BLAKE2s hash function library"
-	depends on CRYPTO_ARCH_HAVE_LIB_BLAKE2S || !CRYPTO_ARCH_HAVE_LIB_BLAKE2S
-	select CRYPTO_LIB_BLAKE2S_GENERIC if CRYPTO_ARCH_HAVE_LIB_BLAKE2S=n
-	help
-	  Enable the Blake2s library interface. This interface may be fulfilled
-	  by either the generic implementation or an arch-specific one, if one
-	  is available and enabled.
-
 config CRYPTO_ARCH_HAVE_LIB_CHACHA
 	tristate
 	help
diff --git a/lib/crypto/Makefile b/lib/crypto/Makefile
index 73205ed269ba..ed43a41f2dcc 100644
--- a/lib/crypto/Makefile
+++ b/lib/crypto/Makefile
@@ -10,11 +10,10 @@ libaes-y					:= aes.o
 obj-$(CONFIG_CRYPTO_LIB_ARC4)			+= libarc4.o
 libarc4-y					:= arc4.o
 
-obj-$(CONFIG_CRYPTO_LIB_BLAKE2S_GENERIC)	+= libblake2s-generic.o
-libblake2s-generic-y				+= blake2s-generic.o
-
-obj-$(CONFIG_CRYPTO_LIB_BLAKE2S)		+= libblake2s.o
-libblake2s-y					+= blake2s.o
+# blake2s is used by the /dev/random driver which is always builtin
+obj-y						+= libblake2s.o
+libblake2s-y					:= blake2s.o
+libblake2s-$(CONFIG_CRYPTO_LIB_BLAKE2S_GENERIC)	+= blake2s-generic.o
 
 obj-$(CONFIG_CRYPTO_LIB_CHACHA20POLY1305)	+= libchacha20poly1305.o
 libchacha20poly1305-y				+= chacha20poly1305.o
diff --git a/lib/crypto/blake2s-generic.c b/lib/crypto/blake2s-generic.c
index 04ff8df24513..75ccb3e633e6 100644
--- a/lib/crypto/blake2s-generic.c
+++ b/lib/crypto/blake2s-generic.c
@@ -37,7 +37,11 @@ static inline void blake2s_increment_counter(struct blake2s_state *state,
 	state->t[1] += (state->t[0] < inc);
 }
 
-void blake2s_compress_generic(struct blake2s_state *state,const u8 *block,
+void blake2s_compress(struct blake2s_state *state, const u8 *block,
+		      size_t nblocks, const u32 inc)
+		      __weak __alias(blake2s_compress_generic);
+
+void blake2s_compress_generic(struct blake2s_state *state, const u8 *block,
 			      size_t nblocks, const u32 inc)
 {
 	u32 m[16];
diff --git a/lib/crypto/blake2s.c b/lib/crypto/blake2s.c
index 4055aa593ec4..93f2ae051370 100644
--- a/lib/crypto/blake2s.c
+++ b/lib/crypto/blake2s.c
@@ -16,12 +16,6 @@
 #include <linux/init.h>
 #include <linux/bug.h>
 
-#if IS_ENABLED(CONFIG_CRYPTO_ARCH_HAVE_LIB_BLAKE2S)
-#  define blake2s_compress blake2s_compress_arch
-#else
-#  define blake2s_compress blake2s_compress_generic
-#endif
-
 void blake2s_update(struct blake2s_state *state, const u8 *in, size_t inlen)
 {
 	__blake2s_update(state, in, inlen, blake2s_compress);
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v5] lib/crypto: blake2s: include as built-in
  2021-12-27 14:20           ` [PATCH v4] " Jason A. Donenfeld
@ 2022-01-01 15:59             ` Jason A. Donenfeld
  2022-01-02 20:42               ` [PATCH v6] " Jason A. Donenfeld
  0 siblings, 1 reply; 23+ messages in thread
From: Jason A. Donenfeld @ 2022-01-01 15:59 UTC (permalink / raw)
  To: linux-kernel
  Cc: Jason A. Donenfeld, Ard Biesheuvel, Greg Kroah-Hartman,
	Masahiro Yamada, linux-kbuild, Herbert Xu, linux-crypto

In preparation for using blake2s in the RNG, we change the way that it
is wired-in to the build system. Instead of using ifdefs to select the
right symbol, we use weak symbols. And because ARM doesn't need the
generic implementation, we make the generic one default only if an arch
library doesn't need it already, and then have arch libraries that do
need it opt-in.

Acked-by: Ard Biesheuvel <ardb@kernel.org>
Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Masahiro Yamada <masahiroy@kernel.org>
Cc: linux-kbuild@vger.kernel.org
Cc: Herbert Xu <herbert@gondor.apana.org.au>
Cc: linux-crypto@vger.kernel.org
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
---
Herbert - As mentioned with the vPrev, I intend to take this via the
crng/random.git tree, since it forms a dependency and I'd like to send a
pull early in 5.17 cycle.

Changes v4->v5:
- Move sourcing the lib/crypto Kconfig file outside of 'if CRYPTO'.

 arch/arm/crypto/blake2s-core.S    |  8 ++++----
 arch/arm/crypto/blake2s-glue.c    |  6 +++---
 arch/x86/crypto/blake2s-glue.c    | 11 +++++------
 crypto/Kconfig                    |  3 ++-
 drivers/net/Kconfig               |  1 -
 include/crypto/internal/blake2s.h |  6 +++---
 lib/crypto/Kconfig                | 13 ++-----------
 lib/crypto/Makefile               |  9 ++++-----
 lib/crypto/blake2s-generic.c      |  6 +++++-
 lib/crypto/blake2s.c              |  6 ------
 10 files changed, 28 insertions(+), 41 deletions(-)

diff --git a/arch/arm/crypto/blake2s-core.S b/arch/arm/crypto/blake2s-core.S
index 86345751bbf3..df40e46601f1 100644
--- a/arch/arm/crypto/blake2s-core.S
+++ b/arch/arm/crypto/blake2s-core.S
@@ -167,8 +167,8 @@
 .endm
 
 //
-// void blake2s_compress_arch(struct blake2s_state *state,
-//			      const u8 *block, size_t nblocks, u32 inc);
+// void blake2s_compress(struct blake2s_state *state,
+//			 const u8 *block, size_t nblocks, u32 inc);
 //
 // Only the first three fields of struct blake2s_state are used:
 //	u32 h[8];	(inout)
@@ -176,7 +176,7 @@
 //	u32 f[2];	(in)
 //
 	.align		5
-ENTRY(blake2s_compress_arch)
+ENTRY(blake2s_compress)
 	push		{r0-r2,r4-r11,lr}	// keep this an even number
 
 .Lnext_block:
@@ -303,4 +303,4 @@ ENTRY(blake2s_compress_arch)
 	str		r3, [r12], #4
 	bne		1b
 	b		.Lcopy_block_done
-ENDPROC(blake2s_compress_arch)
+ENDPROC(blake2s_compress)
diff --git a/arch/arm/crypto/blake2s-glue.c b/arch/arm/crypto/blake2s-glue.c
index f2cc1e5fc9ec..09d3a0cabd2c 100644
--- a/arch/arm/crypto/blake2s-glue.c
+++ b/arch/arm/crypto/blake2s-glue.c
@@ -11,17 +11,17 @@
 #include <linux/module.h>
 
 /* defined in blake2s-core.S */
-EXPORT_SYMBOL(blake2s_compress_arch);
+EXPORT_SYMBOL(blake2s_compress);
 
 static int crypto_blake2s_update_arm(struct shash_desc *desc,
 				     const u8 *in, unsigned int inlen)
 {
-	return crypto_blake2s_update(desc, in, inlen, blake2s_compress_arch);
+	return crypto_blake2s_update(desc, in, inlen, blake2s_compress);
 }
 
 static int crypto_blake2s_final_arm(struct shash_desc *desc, u8 *out)
 {
-	return crypto_blake2s_final(desc, out, blake2s_compress_arch);
+	return crypto_blake2s_final(desc, out, blake2s_compress);
 }
 
 #define BLAKE2S_ALG(name, driver_name, digest_size)			\
diff --git a/arch/x86/crypto/blake2s-glue.c b/arch/x86/crypto/blake2s-glue.c
index a40365ab301e..ef91a3167d27 100644
--- a/arch/x86/crypto/blake2s-glue.c
+++ b/arch/x86/crypto/blake2s-glue.c
@@ -28,9 +28,8 @@ asmlinkage void blake2s_compress_avx512(struct blake2s_state *state,
 static __ro_after_init DEFINE_STATIC_KEY_FALSE(blake2s_use_ssse3);
 static __ro_after_init DEFINE_STATIC_KEY_FALSE(blake2s_use_avx512);
 
-void blake2s_compress_arch(struct blake2s_state *state,
-			   const u8 *block, size_t nblocks,
-			   const u32 inc)
+void blake2s_compress(struct blake2s_state *state, const u8 *block,
+		      size_t nblocks, const u32 inc)
 {
 	/* SIMD disables preemption, so relax after processing each page. */
 	BUILD_BUG_ON(SZ_4K / BLAKE2S_BLOCK_SIZE < 8);
@@ -56,17 +55,17 @@ void blake2s_compress_arch(struct blake2s_state *state,
 		block += blocks * BLAKE2S_BLOCK_SIZE;
 	} while (nblocks);
 }
-EXPORT_SYMBOL(blake2s_compress_arch);
+EXPORT_SYMBOL(blake2s_compress);
 
 static int crypto_blake2s_update_x86(struct shash_desc *desc,
 				     const u8 *in, unsigned int inlen)
 {
-	return crypto_blake2s_update(desc, in, inlen, blake2s_compress_arch);
+	return crypto_blake2s_update(desc, in, inlen, blake2s_compress);
 }
 
 static int crypto_blake2s_final_x86(struct shash_desc *desc, u8 *out)
 {
-	return crypto_blake2s_final(desc, out, blake2s_compress_arch);
+	return crypto_blake2s_final(desc, out, blake2s_compress);
 }
 
 #define BLAKE2S_ALG(name, driver_name, digest_size)			\
diff --git a/crypto/Kconfig b/crypto/Kconfig
index 285f82647d2b..55718de56137 100644
--- a/crypto/Kconfig
+++ b/crypto/Kconfig
@@ -1919,9 +1919,10 @@ config CRYPTO_STATS
 config CRYPTO_HASH_INFO
 	bool
 
-source "lib/crypto/Kconfig"
 source "drivers/crypto/Kconfig"
 source "crypto/asymmetric_keys/Kconfig"
 source "certs/Kconfig"
 
 endif	# if CRYPTO
+
+source "lib/crypto/Kconfig"
diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
index 6cccc3dc00bc..b2a4f998c180 100644
--- a/drivers/net/Kconfig
+++ b/drivers/net/Kconfig
@@ -81,7 +81,6 @@ config WIREGUARD
 	select CRYPTO
 	select CRYPTO_LIB_CURVE25519
 	select CRYPTO_LIB_CHACHA20POLY1305
-	select CRYPTO_LIB_BLAKE2S
 	select CRYPTO_CHACHA20_X86_64 if X86 && 64BIT
 	select CRYPTO_POLY1305_X86_64 if X86 && 64BIT
 	select CRYPTO_BLAKE2S_X86 if X86 && 64BIT
diff --git a/include/crypto/internal/blake2s.h b/include/crypto/internal/blake2s.h
index 8e50d487500f..d39cfa0d333e 100644
--- a/include/crypto/internal/blake2s.h
+++ b/include/crypto/internal/blake2s.h
@@ -11,11 +11,11 @@
 #include <crypto/internal/hash.h>
 #include <linux/string.h>
 
-void blake2s_compress_generic(struct blake2s_state *state,const u8 *block,
+void blake2s_compress_generic(struct blake2s_state *state, const u8 *block,
 			      size_t nblocks, const u32 inc);
 
-void blake2s_compress_arch(struct blake2s_state *state,const u8 *block,
-			   size_t nblocks, const u32 inc);
+void blake2s_compress(struct blake2s_state *state, const u8 *block,
+		      size_t nblocks, const u32 inc);
 
 bool blake2s_selftest(void);
 
diff --git a/lib/crypto/Kconfig b/lib/crypto/Kconfig
index 545ccbddf6a1..0f27976b5038 100644
--- a/lib/crypto/Kconfig
+++ b/lib/crypto/Kconfig
@@ -9,14 +9,14 @@ config CRYPTO_LIB_ARC4
 	tristate
 
 config CRYPTO_ARCH_HAVE_LIB_BLAKE2S
-	tristate
+	bool
 	help
 	  Declares whether the architecture provides an arch-specific
 	  accelerated implementation of the Blake2s library interface,
 	  either builtin or as a module.
 
 config CRYPTO_LIB_BLAKE2S_GENERIC
-	tristate
+	def_bool !CRYPTO_ARCH_HAVE_LIB_BLAKE2S
 	help
 	  This symbol can be depended upon by arch implementations of the
 	  Blake2s library interface that require the generic code as a
@@ -24,15 +24,6 @@ config CRYPTO_LIB_BLAKE2S_GENERIC
 	  implementation is enabled, this implementation serves the users
 	  of CRYPTO_LIB_BLAKE2S.
 
-config CRYPTO_LIB_BLAKE2S
-	tristate "BLAKE2s hash function library"
-	depends on CRYPTO_ARCH_HAVE_LIB_BLAKE2S || !CRYPTO_ARCH_HAVE_LIB_BLAKE2S
-	select CRYPTO_LIB_BLAKE2S_GENERIC if CRYPTO_ARCH_HAVE_LIB_BLAKE2S=n
-	help
-	  Enable the Blake2s library interface. This interface may be fulfilled
-	  by either the generic implementation or an arch-specific one, if one
-	  is available and enabled.
-
 config CRYPTO_ARCH_HAVE_LIB_CHACHA
 	tristate
 	help
diff --git a/lib/crypto/Makefile b/lib/crypto/Makefile
index 73205ed269ba..ed43a41f2dcc 100644
--- a/lib/crypto/Makefile
+++ b/lib/crypto/Makefile
@@ -10,11 +10,10 @@ libaes-y					:= aes.o
 obj-$(CONFIG_CRYPTO_LIB_ARC4)			+= libarc4.o
 libarc4-y					:= arc4.o
 
-obj-$(CONFIG_CRYPTO_LIB_BLAKE2S_GENERIC)	+= libblake2s-generic.o
-libblake2s-generic-y				+= blake2s-generic.o
-
-obj-$(CONFIG_CRYPTO_LIB_BLAKE2S)		+= libblake2s.o
-libblake2s-y					+= blake2s.o
+# blake2s is used by the /dev/random driver which is always builtin
+obj-y						+= libblake2s.o
+libblake2s-y					:= blake2s.o
+libblake2s-$(CONFIG_CRYPTO_LIB_BLAKE2S_GENERIC)	+= blake2s-generic.o
 
 obj-$(CONFIG_CRYPTO_LIB_CHACHA20POLY1305)	+= libchacha20poly1305.o
 libchacha20poly1305-y				+= chacha20poly1305.o
diff --git a/lib/crypto/blake2s-generic.c b/lib/crypto/blake2s-generic.c
index 04ff8df24513..75ccb3e633e6 100644
--- a/lib/crypto/blake2s-generic.c
+++ b/lib/crypto/blake2s-generic.c
@@ -37,7 +37,11 @@ static inline void blake2s_increment_counter(struct blake2s_state *state,
 	state->t[1] += (state->t[0] < inc);
 }
 
-void blake2s_compress_generic(struct blake2s_state *state,const u8 *block,
+void blake2s_compress(struct blake2s_state *state, const u8 *block,
+		      size_t nblocks, const u32 inc)
+		      __weak __alias(blake2s_compress_generic);
+
+void blake2s_compress_generic(struct blake2s_state *state, const u8 *block,
 			      size_t nblocks, const u32 inc)
 {
 	u32 m[16];
diff --git a/lib/crypto/blake2s.c b/lib/crypto/blake2s.c
index 4055aa593ec4..93f2ae051370 100644
--- a/lib/crypto/blake2s.c
+++ b/lib/crypto/blake2s.c
@@ -16,12 +16,6 @@
 #include <linux/init.h>
 #include <linux/bug.h>
 
-#if IS_ENABLED(CONFIG_CRYPTO_ARCH_HAVE_LIB_BLAKE2S)
-#  define blake2s_compress blake2s_compress_arch
-#else
-#  define blake2s_compress blake2s_compress_generic
-#endif
-
 void blake2s_update(struct blake2s_state *state, const u8 *in, size_t inlen)
 {
 	__blake2s_update(state, in, inlen, blake2s_compress);
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v6] lib/crypto: blake2s: include as built-in
  2022-01-01 15:59             ` [PATCH v5] " Jason A. Donenfeld
@ 2022-01-02 20:42               ` Jason A. Donenfeld
  2022-01-03  3:23                 ` Herbert Xu
  0 siblings, 1 reply; 23+ messages in thread
From: Jason A. Donenfeld @ 2022-01-02 20:42 UTC (permalink / raw)
  To: linux-kernel
  Cc: Jason A. Donenfeld, Ard Biesheuvel, Greg Kroah-Hartman,
	Masahiro Yamada, linux-kbuild, Herbert Xu, linux-crypto

In preparation for using blake2s in the RNG, we change the way that it
is wired-in to the build system. Instead of using ifdefs to select the
right symbol, we use weak symbols. And because ARM doesn't need the
generic implementation, we make the generic one default only if an arch
library doesn't need it already, and then have arch libraries that do
need it opt-in.

Acked-by: Ard Biesheuvel <ardb@kernel.org>
Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Masahiro Yamada <masahiroy@kernel.org>
Cc: linux-kbuild@vger.kernel.org
Cc: Herbert Xu <herbert@gondor.apana.org.au>
Cc: linux-crypto@vger.kernel.org
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
---
Herbert - As mentioned with the vPrev, I intend to take this via the
crng/random.git tree, since it forms a dependency and I'd like to send a
pull early in 5.17 cycle.

Changes v5->v6:
- Make accelerated versions bool instead of tristate.
Changes v4->v5:
- Move sourcing the lib/crypto Kconfig file outside of 'if CRYPTO'.

 arch/arm/crypto/Kconfig           |  2 +-
 arch/arm/crypto/blake2s-core.S    |  8 ++++----
 arch/arm/crypto/blake2s-glue.c    |  6 +++---
 arch/x86/crypto/blake2s-glue.c    | 11 +++++------
 crypto/Kconfig                    |  5 +++--
 drivers/net/Kconfig               |  1 -
 include/crypto/internal/blake2s.h |  6 +++---
 lib/crypto/Kconfig                | 13 ++-----------
 lib/crypto/Makefile               |  9 ++++-----
 lib/crypto/blake2s-generic.c      |  6 +++++-
 lib/crypto/blake2s.c              |  6 ------
 11 files changed, 30 insertions(+), 43 deletions(-)

diff --git a/arch/arm/crypto/Kconfig b/arch/arm/crypto/Kconfig
index 2b575792363e..f1bcf804b8b5 100644
--- a/arch/arm/crypto/Kconfig
+++ b/arch/arm/crypto/Kconfig
@@ -63,7 +63,7 @@ config CRYPTO_SHA512_ARM
 	  using optimized ARM assembler and NEON, when available.
 
 config CRYPTO_BLAKE2S_ARM
-	tristate "BLAKE2s digest algorithm (ARM)"
+	bool "BLAKE2s digest algorithm (ARM)"
 	select CRYPTO_ARCH_HAVE_LIB_BLAKE2S
 	help
 	  BLAKE2s digest algorithm optimized with ARM scalar instructions.  This
diff --git a/arch/arm/crypto/blake2s-core.S b/arch/arm/crypto/blake2s-core.S
index 86345751bbf3..df40e46601f1 100644
--- a/arch/arm/crypto/blake2s-core.S
+++ b/arch/arm/crypto/blake2s-core.S
@@ -167,8 +167,8 @@
 .endm
 
 //
-// void blake2s_compress_arch(struct blake2s_state *state,
-//			      const u8 *block, size_t nblocks, u32 inc);
+// void blake2s_compress(struct blake2s_state *state,
+//			 const u8 *block, size_t nblocks, u32 inc);
 //
 // Only the first three fields of struct blake2s_state are used:
 //	u32 h[8];	(inout)
@@ -176,7 +176,7 @@
 //	u32 f[2];	(in)
 //
 	.align		5
-ENTRY(blake2s_compress_arch)
+ENTRY(blake2s_compress)
 	push		{r0-r2,r4-r11,lr}	// keep this an even number
 
 .Lnext_block:
@@ -303,4 +303,4 @@ ENTRY(blake2s_compress_arch)
 	str		r3, [r12], #4
 	bne		1b
 	b		.Lcopy_block_done
-ENDPROC(blake2s_compress_arch)
+ENDPROC(blake2s_compress)
diff --git a/arch/arm/crypto/blake2s-glue.c b/arch/arm/crypto/blake2s-glue.c
index f2cc1e5fc9ec..09d3a0cabd2c 100644
--- a/arch/arm/crypto/blake2s-glue.c
+++ b/arch/arm/crypto/blake2s-glue.c
@@ -11,17 +11,17 @@
 #include <linux/module.h>
 
 /* defined in blake2s-core.S */
-EXPORT_SYMBOL(blake2s_compress_arch);
+EXPORT_SYMBOL(blake2s_compress);
 
 static int crypto_blake2s_update_arm(struct shash_desc *desc,
 				     const u8 *in, unsigned int inlen)
 {
-	return crypto_blake2s_update(desc, in, inlen, blake2s_compress_arch);
+	return crypto_blake2s_update(desc, in, inlen, blake2s_compress);
 }
 
 static int crypto_blake2s_final_arm(struct shash_desc *desc, u8 *out)
 {
-	return crypto_blake2s_final(desc, out, blake2s_compress_arch);
+	return crypto_blake2s_final(desc, out, blake2s_compress);
 }
 
 #define BLAKE2S_ALG(name, driver_name, digest_size)			\
diff --git a/arch/x86/crypto/blake2s-glue.c b/arch/x86/crypto/blake2s-glue.c
index a40365ab301e..ef91a3167d27 100644
--- a/arch/x86/crypto/blake2s-glue.c
+++ b/arch/x86/crypto/blake2s-glue.c
@@ -28,9 +28,8 @@ asmlinkage void blake2s_compress_avx512(struct blake2s_state *state,
 static __ro_after_init DEFINE_STATIC_KEY_FALSE(blake2s_use_ssse3);
 static __ro_after_init DEFINE_STATIC_KEY_FALSE(blake2s_use_avx512);
 
-void blake2s_compress_arch(struct blake2s_state *state,
-			   const u8 *block, size_t nblocks,
-			   const u32 inc)
+void blake2s_compress(struct blake2s_state *state, const u8 *block,
+		      size_t nblocks, const u32 inc)
 {
 	/* SIMD disables preemption, so relax after processing each page. */
 	BUILD_BUG_ON(SZ_4K / BLAKE2S_BLOCK_SIZE < 8);
@@ -56,17 +55,17 @@ void blake2s_compress_arch(struct blake2s_state *state,
 		block += blocks * BLAKE2S_BLOCK_SIZE;
 	} while (nblocks);
 }
-EXPORT_SYMBOL(blake2s_compress_arch);
+EXPORT_SYMBOL(blake2s_compress);
 
 static int crypto_blake2s_update_x86(struct shash_desc *desc,
 				     const u8 *in, unsigned int inlen)
 {
-	return crypto_blake2s_update(desc, in, inlen, blake2s_compress_arch);
+	return crypto_blake2s_update(desc, in, inlen, blake2s_compress);
 }
 
 static int crypto_blake2s_final_x86(struct shash_desc *desc, u8 *out)
 {
-	return crypto_blake2s_final(desc, out, blake2s_compress_arch);
+	return crypto_blake2s_final(desc, out, blake2s_compress);
 }
 
 #define BLAKE2S_ALG(name, driver_name, digest_size)			\
diff --git a/crypto/Kconfig b/crypto/Kconfig
index 285f82647d2b..b7a2e50dcbc8 100644
--- a/crypto/Kconfig
+++ b/crypto/Kconfig
@@ -702,7 +702,7 @@ config CRYPTO_BLAKE2S
 	  See https://blake2.net for further information.
 
 config CRYPTO_BLAKE2S_X86
-	tristate "BLAKE2s digest algorithm (x86 accelerated version)"
+	bool "BLAKE2s digest algorithm (x86 accelerated version)"
 	depends on X86 && 64BIT
 	select CRYPTO_LIB_BLAKE2S_GENERIC
 	select CRYPTO_ARCH_HAVE_LIB_BLAKE2S
@@ -1919,9 +1919,10 @@ config CRYPTO_STATS
 config CRYPTO_HASH_INFO
 	bool
 
-source "lib/crypto/Kconfig"
 source "drivers/crypto/Kconfig"
 source "crypto/asymmetric_keys/Kconfig"
 source "certs/Kconfig"
 
 endif	# if CRYPTO
+
+source "lib/crypto/Kconfig"
diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
index 6cccc3dc00bc..b2a4f998c180 100644
--- a/drivers/net/Kconfig
+++ b/drivers/net/Kconfig
@@ -81,7 +81,6 @@ config WIREGUARD
 	select CRYPTO
 	select CRYPTO_LIB_CURVE25519
 	select CRYPTO_LIB_CHACHA20POLY1305
-	select CRYPTO_LIB_BLAKE2S
 	select CRYPTO_CHACHA20_X86_64 if X86 && 64BIT
 	select CRYPTO_POLY1305_X86_64 if X86 && 64BIT
 	select CRYPTO_BLAKE2S_X86 if X86 && 64BIT
diff --git a/include/crypto/internal/blake2s.h b/include/crypto/internal/blake2s.h
index 8e50d487500f..d39cfa0d333e 100644
--- a/include/crypto/internal/blake2s.h
+++ b/include/crypto/internal/blake2s.h
@@ -11,11 +11,11 @@
 #include <crypto/internal/hash.h>
 #include <linux/string.h>
 
-void blake2s_compress_generic(struct blake2s_state *state,const u8 *block,
+void blake2s_compress_generic(struct blake2s_state *state, const u8 *block,
 			      size_t nblocks, const u32 inc);
 
-void blake2s_compress_arch(struct blake2s_state *state,const u8 *block,
-			   size_t nblocks, const u32 inc);
+void blake2s_compress(struct blake2s_state *state, const u8 *block,
+		      size_t nblocks, const u32 inc);
 
 bool blake2s_selftest(void);
 
diff --git a/lib/crypto/Kconfig b/lib/crypto/Kconfig
index 545ccbddf6a1..0f27976b5038 100644
--- a/lib/crypto/Kconfig
+++ b/lib/crypto/Kconfig
@@ -9,14 +9,14 @@ config CRYPTO_LIB_ARC4
 	tristate
 
 config CRYPTO_ARCH_HAVE_LIB_BLAKE2S
-	tristate
+	bool
 	help
 	  Declares whether the architecture provides an arch-specific
 	  accelerated implementation of the Blake2s library interface,
 	  either builtin or as a module.
 
 config CRYPTO_LIB_BLAKE2S_GENERIC
-	tristate
+	def_bool !CRYPTO_ARCH_HAVE_LIB_BLAKE2S
 	help
 	  This symbol can be depended upon by arch implementations of the
 	  Blake2s library interface that require the generic code as a
@@ -24,15 +24,6 @@ config CRYPTO_LIB_BLAKE2S_GENERIC
 	  implementation is enabled, this implementation serves the users
 	  of CRYPTO_LIB_BLAKE2S.
 
-config CRYPTO_LIB_BLAKE2S
-	tristate "BLAKE2s hash function library"
-	depends on CRYPTO_ARCH_HAVE_LIB_BLAKE2S || !CRYPTO_ARCH_HAVE_LIB_BLAKE2S
-	select CRYPTO_LIB_BLAKE2S_GENERIC if CRYPTO_ARCH_HAVE_LIB_BLAKE2S=n
-	help
-	  Enable the Blake2s library interface. This interface may be fulfilled
-	  by either the generic implementation or an arch-specific one, if one
-	  is available and enabled.
-
 config CRYPTO_ARCH_HAVE_LIB_CHACHA
 	tristate
 	help
diff --git a/lib/crypto/Makefile b/lib/crypto/Makefile
index 73205ed269ba..ed43a41f2dcc 100644
--- a/lib/crypto/Makefile
+++ b/lib/crypto/Makefile
@@ -10,11 +10,10 @@ libaes-y					:= aes.o
 obj-$(CONFIG_CRYPTO_LIB_ARC4)			+= libarc4.o
 libarc4-y					:= arc4.o
 
-obj-$(CONFIG_CRYPTO_LIB_BLAKE2S_GENERIC)	+= libblake2s-generic.o
-libblake2s-generic-y				+= blake2s-generic.o
-
-obj-$(CONFIG_CRYPTO_LIB_BLAKE2S)		+= libblake2s.o
-libblake2s-y					+= blake2s.o
+# blake2s is used by the /dev/random driver which is always builtin
+obj-y						+= libblake2s.o
+libblake2s-y					:= blake2s.o
+libblake2s-$(CONFIG_CRYPTO_LIB_BLAKE2S_GENERIC)	+= blake2s-generic.o
 
 obj-$(CONFIG_CRYPTO_LIB_CHACHA20POLY1305)	+= libchacha20poly1305.o
 libchacha20poly1305-y				+= chacha20poly1305.o
diff --git a/lib/crypto/blake2s-generic.c b/lib/crypto/blake2s-generic.c
index 04ff8df24513..75ccb3e633e6 100644
--- a/lib/crypto/blake2s-generic.c
+++ b/lib/crypto/blake2s-generic.c
@@ -37,7 +37,11 @@ static inline void blake2s_increment_counter(struct blake2s_state *state,
 	state->t[1] += (state->t[0] < inc);
 }
 
-void blake2s_compress_generic(struct blake2s_state *state,const u8 *block,
+void blake2s_compress(struct blake2s_state *state, const u8 *block,
+		      size_t nblocks, const u32 inc)
+		      __weak __alias(blake2s_compress_generic);
+
+void blake2s_compress_generic(struct blake2s_state *state, const u8 *block,
 			      size_t nblocks, const u32 inc)
 {
 	u32 m[16];
diff --git a/lib/crypto/blake2s.c b/lib/crypto/blake2s.c
index 4055aa593ec4..93f2ae051370 100644
--- a/lib/crypto/blake2s.c
+++ b/lib/crypto/blake2s.c
@@ -16,12 +16,6 @@
 #include <linux/init.h>
 #include <linux/bug.h>
 
-#if IS_ENABLED(CONFIG_CRYPTO_ARCH_HAVE_LIB_BLAKE2S)
-#  define blake2s_compress blake2s_compress_arch
-#else
-#  define blake2s_compress blake2s_compress_generic
-#endif
-
 void blake2s_update(struct blake2s_state *state, const u8 *in, size_t inlen)
 {
 	__blake2s_update(state, in, inlen, blake2s_compress);
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* Re: [PATCH v6] lib/crypto: blake2s: include as built-in
  2022-01-02 20:42               ` [PATCH v6] " Jason A. Donenfeld
@ 2022-01-03  3:23                 ` Herbert Xu
  2022-01-03  3:45                   ` Jason A. Donenfeld
  0 siblings, 1 reply; 23+ messages in thread
From: Herbert Xu @ 2022-01-03  3:23 UTC (permalink / raw)
  To: Jason A. Donenfeld
  Cc: linux-kernel, Ard Biesheuvel, Greg Kroah-Hartman,
	Masahiro Yamada, linux-kbuild, linux-crypto

On Sun, Jan 02, 2022 at 09:42:03PM +0100, Jason A. Donenfeld wrote:
> In preparation for using blake2s in the RNG, we change the way that it
> is wired-in to the build system. Instead of using ifdefs to select the
> right symbol, we use weak symbols. And because ARM doesn't need the
> generic implementation, we make the generic one default only if an arch
> library doesn't need it already, and then have arch libraries that do
> need it opt-in.
> 
> Acked-by: Ard Biesheuvel <ardb@kernel.org>
> Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Masahiro Yamada <masahiroy@kernel.org>
> Cc: linux-kbuild@vger.kernel.org
> Cc: Herbert Xu <herbert@gondor.apana.org.au>
> Cc: linux-crypto@vger.kernel.org
> Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
> ---
> Herbert - As mentioned with the vPrev, I intend to take this via the
> crng/random.git tree, since it forms a dependency and I'd like to send a
> pull early in 5.17 cycle.

At this point I think we should push this through crypto.  The
changes are too invasive with respect to the crypto Kconfig files.

> diff --git a/crypto/Kconfig b/crypto/Kconfig
> index 285f82647d2b..b7a2e50dcbc8 100644
> --- a/crypto/Kconfig
> +++ b/crypto/Kconfig
> @@ -702,7 +702,7 @@ config CRYPTO_BLAKE2S
>  	  See https://blake2.net for further information.
>  
>  config CRYPTO_BLAKE2S_X86
> -	tristate "BLAKE2s digest algorithm (x86 accelerated version)"
> +	bool "BLAKE2s digest algorithm (x86 accelerated version)"
>  	depends on X86 && 64BIT
>  	select CRYPTO_LIB_BLAKE2S_GENERIC
>  	select CRYPTO_ARCH_HAVE_LIB_BLAKE2S

This will break when CRYPTO is disabled because the x86 crypto
glue code depends on the crypto subsystem.

Cheers,
-- 
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

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v6] lib/crypto: blake2s: include as built-in
  2022-01-03  3:23                 ` Herbert Xu
@ 2022-01-03  3:45                   ` Jason A. Donenfeld
  2022-01-03  4:06                     ` Herbert Xu
  0 siblings, 1 reply; 23+ messages in thread
From: Jason A. Donenfeld @ 2022-01-03  3:45 UTC (permalink / raw)
  To: Herbert Xu
  Cc: linux-kernel, Ard Biesheuvel, Greg Kroah-Hartman,
	Masahiro Yamada, linux-kbuild, linux-crypto

On 1/3/22, Herbert Xu <herbert@gondor.apana.org.au> wrote:
> At this point I think we should push this through crypto.  The
> changes are too invasive with respect to the crypto Kconfig files.

Ugh, can we please not? That will really make things much harder and
more annoying for me. I have an early pull planned, and you'll quickly
be able to rebase on top of it. It also doesn't appear to conflict
with anything you have queued up. Please, I would really appreciate
some straight forward linearity here, and I don't think my taking it
will negatively impact the flow.

>
>> diff --git a/crypto/Kconfig b/crypto/Kconfig
>> index 285f82647d2b..b7a2e50dcbc8 100644
>> --- a/crypto/Kconfig
>> +++ b/crypto/Kconfig
>> @@ -702,7 +702,7 @@ config CRYPTO_BLAKE2S
>>  	  See https://blake2.net for further information.
>>
>>  config CRYPTO_BLAKE2S_X86
>> -	tristate "BLAKE2s digest algorithm (x86 accelerated version)"
>> +	bool "BLAKE2s digest algorithm (x86 accelerated version)"
>>  	depends on X86 && 64BIT
>>  	select CRYPTO_LIB_BLAKE2S_GENERIC
>>  	select CRYPTO_ARCH_HAVE_LIB_BLAKE2S
>
> This will break when CRYPTO is disabled because the x86 crypto
> glue code depends on the crypto subsystem.

That snippet is inside an 'if CRYPTO' block, so it can't be selected
without CRYPTO being enabled.

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v6] lib/crypto: blake2s: include as built-in
  2022-01-03  3:45                   ` Jason A. Donenfeld
@ 2022-01-03  4:06                     ` Herbert Xu
  2022-01-03 11:57                       ` Jason A. Donenfeld
  0 siblings, 1 reply; 23+ messages in thread
From: Herbert Xu @ 2022-01-03  4:06 UTC (permalink / raw)
  To: Jason A. Donenfeld
  Cc: linux-kernel, Ard Biesheuvel, Greg Kroah-Hartman,
	Masahiro Yamada, linux-kbuild, linux-crypto

On Mon, Jan 03, 2022 at 04:45:10AM +0100, Jason A. Donenfeld wrote:
> 
> Ugh, can we please not? That will really make things much harder and
> more annoying for me. I have an early pull planned, and you'll quickly
> be able to rebase on top of it. It also doesn't appear to conflict
> with anything you have queued up. Please, I would really appreciate
> some straight forward linearity here, and I don't think my taking it
> will negatively impact the flow.

Your patches as they stand will break the crypto tree.  So
that's why they should not go in without the proper changes.

> That snippet is inside an 'if CRYPTO' block, so it can't be selected
> without CRYPTO being enabled.

No CONFIG_CRYPTO is not the issue.  This depends on specific
bits of the Crypto API such as CRYPTO_HASH.  Simply selecting
it is also not acceptable because you will be forcing all of the
Crypto API into vmlinux even though none of it is required by
/dev/random.

Cheers,
-- 
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

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v6] lib/crypto: blake2s: include as built-in
  2022-01-03  4:06                     ` Herbert Xu
@ 2022-01-03 11:57                       ` Jason A. Donenfeld
  2022-01-03 12:31                         ` [PATCH v7] " Jason A. Donenfeld
  0 siblings, 1 reply; 23+ messages in thread
From: Jason A. Donenfeld @ 2022-01-03 11:57 UTC (permalink / raw)
  To: Herbert Xu
  Cc: LKML, Ard Biesheuvel, Greg Kroah-Hartman, Masahiro Yamada,
	Linux Kbuild mailing list, Linux Crypto Mailing List

Hi Herbert,

On Mon, Jan 3, 2022 at 5:07 AM Herbert Xu <herbert@gondor.apana.org.au> wrote:
>
> On Mon, Jan 03, 2022 at 04:45:10AM +0100, Jason A. Donenfeld wrote:
> >
> > Ugh, can we please not? That will really make things much harder and
> > more annoying for me. I have an early pull planned, and you'll quickly
> > be able to rebase on top of it. It also doesn't appear to conflict
> > with anything you have queued up. Please, I would really appreciate
> > some straight forward linearity here, and I don't think my taking it
> > will negatively impact the flow.
>
> Your patches as they stand will break the crypto tree.  So
> that's why they should not go in without the proper changes.

Okay, I'll try to fix them up so that they don't break the crypto
tree, and given below, I think I should be able to do this with fewer
changes to some of the Kconfig, which will hopefully address your
concerns and enable me to take this patch so that things are a bit
more straightforward.


>
> > That snippet is inside an 'if CRYPTO' block, so it can't be selected
> > without CRYPTO being enabled.
>
> No CONFIG_CRYPTO is not the issue.  This depends on specific
> bits of the Crypto API such as CRYPTO_HASH.  Simply selecting
> it is also not acceptable because you will be forcing all of the
> Crypto API into vmlinux even though none of it is required by
> /dev/random.

Thanks, I think I see what your concern is now. I'll take a stab at
addressing that.

Jason

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [PATCH v7] lib/crypto: blake2s: include as built-in
  2022-01-03 11:57                       ` Jason A. Donenfeld
@ 2022-01-03 12:31                         ` Jason A. Donenfeld
  2022-01-04  1:21                           ` Herbert Xu
  0 siblings, 1 reply; 23+ messages in thread
From: Jason A. Donenfeld @ 2022-01-03 12:31 UTC (permalink / raw)
  To: linux-kernel
  Cc: Jason A. Donenfeld, Ard Biesheuvel, Greg Kroah-Hartman,
	Masahiro Yamada, linux-kbuild, Herbert Xu, linux-crypto

In preparation for using blake2s in the RNG, we change the way that it
is wired-in to the build system. Instead of using ifdefs to select the
right symbol, we use weak symbols. And because ARM doesn't need the
generic implementation, we make the generic one default only if an arch
library doesn't need it already, and then have arch libraries that do
need it opt-in. So that the arch libraries can remain tristate rather
than bool, we then split the shash part from the glue code.

Acked-by: Ard Biesheuvel <ardb@kernel.org>
Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Masahiro Yamada <masahiroy@kernel.org>
Cc: linux-kbuild@vger.kernel.org
Cc: Herbert Xu <herbert@gondor.apana.org.au>
Cc: linux-crypto@vger.kernel.org
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
---
Herbert - As discussed, I still intend to take this via the
crng/random.git tree because it forms a dependency, and I'd like to send
a pull very early in the 5.17 cycle. I've taken some care to minimize
changes to the {arch/*/}crypto/Kconfig files, as you mentioned this
might cause some conflicts. Your tree should work cleanly on top of this
commit.

Changes v6->v7:
- Split arch shash implementations out from the glue code, so that they
  can remain as tristates, and we thus don't need to touch
  arch/*/crypto/Kconfig at all.
Changes v5->v6:
- Make accelerated versions bool instead of tristate.
Changes v4->v5:
- Move sourcing the lib/crypto Kconfig file outside of 'if CRYPTO'.

 arch/arm/crypto/Makefile          |  4 +-
 arch/arm/crypto/blake2s-core.S    |  8 ++--
 arch/arm/crypto/blake2s-glue.c    | 73 +----------------------------
 arch/arm/crypto/blake2s-shash.c   | 75 ++++++++++++++++++++++++++++++
 arch/x86/crypto/Makefile          |  4 +-
 arch/x86/crypto/blake2s-glue.c    | 68 +++------------------------
 arch/x86/crypto/blake2s-shash.c   | 77 +++++++++++++++++++++++++++++++
 crypto/Kconfig                    |  3 +-
 drivers/net/Kconfig               |  1 -
 include/crypto/internal/blake2s.h |  6 +--
 lib/crypto/Kconfig                | 23 +++------
 lib/crypto/Makefile               |  9 ++--
 lib/crypto/blake2s-generic.c      |  6 ++-
 lib/crypto/blake2s.c              |  6 ---
 14 files changed, 189 insertions(+), 174 deletions(-)
 create mode 100644 arch/arm/crypto/blake2s-shash.c
 create mode 100644 arch/x86/crypto/blake2s-shash.c

diff --git a/arch/arm/crypto/Makefile b/arch/arm/crypto/Makefile
index eafa898ba6a7..0274f81cc8ea 100644
--- a/arch/arm/crypto/Makefile
+++ b/arch/arm/crypto/Makefile
@@ -10,6 +10,7 @@ obj-$(CONFIG_CRYPTO_SHA1_ARM_NEON) += sha1-arm-neon.o
 obj-$(CONFIG_CRYPTO_SHA256_ARM) += sha256-arm.o
 obj-$(CONFIG_CRYPTO_SHA512_ARM) += sha512-arm.o
 obj-$(CONFIG_CRYPTO_BLAKE2S_ARM) += blake2s-arm.o
+obj-$(if $(CONFIG_CRYPTO_BLAKE2S_ARM),y) += libblake2s-arm.o
 obj-$(CONFIG_CRYPTO_BLAKE2B_NEON) += blake2b-neon.o
 obj-$(CONFIG_CRYPTO_CHACHA20_NEON) += chacha-neon.o
 obj-$(CONFIG_CRYPTO_POLY1305_ARM) += poly1305-arm.o
@@ -31,7 +32,8 @@ sha256-arm-neon-$(CONFIG_KERNEL_MODE_NEON) := sha256_neon_glue.o
 sha256-arm-y	:= sha256-core.o sha256_glue.o $(sha256-arm-neon-y)
 sha512-arm-neon-$(CONFIG_KERNEL_MODE_NEON) := sha512-neon-glue.o
 sha512-arm-y	:= sha512-core.o sha512-glue.o $(sha512-arm-neon-y)
-blake2s-arm-y   := blake2s-core.o blake2s-glue.o
+blake2s-arm-y   := blake2s-shash.o
+libblake2s-arm-y:= blake2s-core.o blake2s-glue.o
 blake2b-neon-y  := blake2b-neon-core.o blake2b-neon-glue.o
 sha1-arm-ce-y	:= sha1-ce-core.o sha1-ce-glue.o
 sha2-arm-ce-y	:= sha2-ce-core.o sha2-ce-glue.o
diff --git a/arch/arm/crypto/blake2s-core.S b/arch/arm/crypto/blake2s-core.S
index 86345751bbf3..df40e46601f1 100644
--- a/arch/arm/crypto/blake2s-core.S
+++ b/arch/arm/crypto/blake2s-core.S
@@ -167,8 +167,8 @@
 .endm
 
 //
-// void blake2s_compress_arch(struct blake2s_state *state,
-//			      const u8 *block, size_t nblocks, u32 inc);
+// void blake2s_compress(struct blake2s_state *state,
+//			 const u8 *block, size_t nblocks, u32 inc);
 //
 // Only the first three fields of struct blake2s_state are used:
 //	u32 h[8];	(inout)
@@ -176,7 +176,7 @@
 //	u32 f[2];	(in)
 //
 	.align		5
-ENTRY(blake2s_compress_arch)
+ENTRY(blake2s_compress)
 	push		{r0-r2,r4-r11,lr}	// keep this an even number
 
 .Lnext_block:
@@ -303,4 +303,4 @@ ENTRY(blake2s_compress_arch)
 	str		r3, [r12], #4
 	bne		1b
 	b		.Lcopy_block_done
-ENDPROC(blake2s_compress_arch)
+ENDPROC(blake2s_compress)
diff --git a/arch/arm/crypto/blake2s-glue.c b/arch/arm/crypto/blake2s-glue.c
index f2cc1e5fc9ec..0238a70d9581 100644
--- a/arch/arm/crypto/blake2s-glue.c
+++ b/arch/arm/crypto/blake2s-glue.c
@@ -1,78 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0-or-later
-/*
- * BLAKE2s digest algorithm, ARM scalar implementation
- *
- * Copyright 2020 Google LLC
- */
 
 #include <crypto/internal/blake2s.h>
-#include <crypto/internal/hash.h>
-
 #include <linux/module.h>
 
 /* defined in blake2s-core.S */
-EXPORT_SYMBOL(blake2s_compress_arch);
-
-static int crypto_blake2s_update_arm(struct shash_desc *desc,
-				     const u8 *in, unsigned int inlen)
-{
-	return crypto_blake2s_update(desc, in, inlen, blake2s_compress_arch);
-}
-
-static int crypto_blake2s_final_arm(struct shash_desc *desc, u8 *out)
-{
-	return crypto_blake2s_final(desc, out, blake2s_compress_arch);
-}
-
-#define BLAKE2S_ALG(name, driver_name, digest_size)			\
-	{								\
-		.base.cra_name		= name,				\
-		.base.cra_driver_name	= driver_name,			\
-		.base.cra_priority	= 200,				\
-		.base.cra_flags		= CRYPTO_ALG_OPTIONAL_KEY,	\
-		.base.cra_blocksize	= BLAKE2S_BLOCK_SIZE,		\
-		.base.cra_ctxsize	= sizeof(struct blake2s_tfm_ctx), \
-		.base.cra_module	= THIS_MODULE,			\
-		.digestsize		= digest_size,			\
-		.setkey			= crypto_blake2s_setkey,	\
-		.init			= crypto_blake2s_init,		\
-		.update			= crypto_blake2s_update_arm,	\
-		.final			= crypto_blake2s_final_arm,	\
-		.descsize		= sizeof(struct blake2s_state),	\
-	}
-
-static struct shash_alg blake2s_arm_algs[] = {
-	BLAKE2S_ALG("blake2s-128", "blake2s-128-arm", BLAKE2S_128_HASH_SIZE),
-	BLAKE2S_ALG("blake2s-160", "blake2s-160-arm", BLAKE2S_160_HASH_SIZE),
-	BLAKE2S_ALG("blake2s-224", "blake2s-224-arm", BLAKE2S_224_HASH_SIZE),
-	BLAKE2S_ALG("blake2s-256", "blake2s-256-arm", BLAKE2S_256_HASH_SIZE),
-};
-
-static int __init blake2s_arm_mod_init(void)
-{
-	return IS_REACHABLE(CONFIG_CRYPTO_HASH) ?
-		crypto_register_shashes(blake2s_arm_algs,
-					ARRAY_SIZE(blake2s_arm_algs)) : 0;
-}
-
-static void __exit blake2s_arm_mod_exit(void)
-{
-	if (IS_REACHABLE(CONFIG_CRYPTO_HASH))
-		crypto_unregister_shashes(blake2s_arm_algs,
-					  ARRAY_SIZE(blake2s_arm_algs));
-}
-
-module_init(blake2s_arm_mod_init);
-module_exit(blake2s_arm_mod_exit);
-
-MODULE_DESCRIPTION("BLAKE2s digest algorithm, ARM scalar implementation");
-MODULE_LICENSE("GPL");
-MODULE_AUTHOR("Eric Biggers <ebiggers@google.com>");
-MODULE_ALIAS_CRYPTO("blake2s-128");
-MODULE_ALIAS_CRYPTO("blake2s-128-arm");
-MODULE_ALIAS_CRYPTO("blake2s-160");
-MODULE_ALIAS_CRYPTO("blake2s-160-arm");
-MODULE_ALIAS_CRYPTO("blake2s-224");
-MODULE_ALIAS_CRYPTO("blake2s-224-arm");
-MODULE_ALIAS_CRYPTO("blake2s-256");
-MODULE_ALIAS_CRYPTO("blake2s-256-arm");
+EXPORT_SYMBOL(blake2s_compress);
diff --git a/arch/arm/crypto/blake2s-shash.c b/arch/arm/crypto/blake2s-shash.c
new file mode 100644
index 000000000000..17c1c3bfe2f5
--- /dev/null
+++ b/arch/arm/crypto/blake2s-shash.c
@@ -0,0 +1,75 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * BLAKE2s digest algorithm, ARM scalar implementation
+ *
+ * Copyright 2020 Google LLC
+ */
+
+#include <crypto/internal/blake2s.h>
+#include <crypto/internal/hash.h>
+
+#include <linux/module.h>
+
+static int crypto_blake2s_update_arm(struct shash_desc *desc,
+				     const u8 *in, unsigned int inlen)
+{
+	return crypto_blake2s_update(desc, in, inlen, blake2s_compress);
+}
+
+static int crypto_blake2s_final_arm(struct shash_desc *desc, u8 *out)
+{
+	return crypto_blake2s_final(desc, out, blake2s_compress);
+}
+
+#define BLAKE2S_ALG(name, driver_name, digest_size)			\
+	{								\
+		.base.cra_name		= name,				\
+		.base.cra_driver_name	= driver_name,			\
+		.base.cra_priority	= 200,				\
+		.base.cra_flags		= CRYPTO_ALG_OPTIONAL_KEY,	\
+		.base.cra_blocksize	= BLAKE2S_BLOCK_SIZE,		\
+		.base.cra_ctxsize	= sizeof(struct blake2s_tfm_ctx), \
+		.base.cra_module	= THIS_MODULE,			\
+		.digestsize		= digest_size,			\
+		.setkey			= crypto_blake2s_setkey,	\
+		.init			= crypto_blake2s_init,		\
+		.update			= crypto_blake2s_update_arm,	\
+		.final			= crypto_blake2s_final_arm,	\
+		.descsize		= sizeof(struct blake2s_state),	\
+	}
+
+static struct shash_alg blake2s_arm_algs[] = {
+	BLAKE2S_ALG("blake2s-128", "blake2s-128-arm", BLAKE2S_128_HASH_SIZE),
+	BLAKE2S_ALG("blake2s-160", "blake2s-160-arm", BLAKE2S_160_HASH_SIZE),
+	BLAKE2S_ALG("blake2s-224", "blake2s-224-arm", BLAKE2S_224_HASH_SIZE),
+	BLAKE2S_ALG("blake2s-256", "blake2s-256-arm", BLAKE2S_256_HASH_SIZE),
+};
+
+static int __init blake2s_arm_mod_init(void)
+{
+	return IS_REACHABLE(CONFIG_CRYPTO_HASH) ?
+		crypto_register_shashes(blake2s_arm_algs,
+					ARRAY_SIZE(blake2s_arm_algs)) : 0;
+}
+
+static void __exit blake2s_arm_mod_exit(void)
+{
+	if (IS_REACHABLE(CONFIG_CRYPTO_HASH))
+		crypto_unregister_shashes(blake2s_arm_algs,
+					  ARRAY_SIZE(blake2s_arm_algs));
+}
+
+module_init(blake2s_arm_mod_init);
+module_exit(blake2s_arm_mod_exit);
+
+MODULE_DESCRIPTION("BLAKE2s digest algorithm, ARM scalar implementation");
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Eric Biggers <ebiggers@google.com>");
+MODULE_ALIAS_CRYPTO("blake2s-128");
+MODULE_ALIAS_CRYPTO("blake2s-128-arm");
+MODULE_ALIAS_CRYPTO("blake2s-160");
+MODULE_ALIAS_CRYPTO("blake2s-160-arm");
+MODULE_ALIAS_CRYPTO("blake2s-224");
+MODULE_ALIAS_CRYPTO("blake2s-224-arm");
+MODULE_ALIAS_CRYPTO("blake2s-256");
+MODULE_ALIAS_CRYPTO("blake2s-256-arm");
diff --git a/arch/x86/crypto/Makefile b/arch/x86/crypto/Makefile
index f307c93fc90a..c3af959648e6 100644
--- a/arch/x86/crypto/Makefile
+++ b/arch/x86/crypto/Makefile
@@ -62,7 +62,9 @@ obj-$(CONFIG_CRYPTO_SHA512_SSSE3) += sha512-ssse3.o
 sha512-ssse3-y := sha512-ssse3-asm.o sha512-avx-asm.o sha512-avx2-asm.o sha512_ssse3_glue.o
 
 obj-$(CONFIG_CRYPTO_BLAKE2S_X86) += blake2s-x86_64.o
-blake2s-x86_64-y := blake2s-core.o blake2s-glue.o
+blake2s-x86_64-y := blake2s-shash.o
+obj-$(if $(CONFIG_CRYPTO_BLAKE2S_X86),y) += 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
 ghash-clmulni-intel-y := ghash-clmulni-intel_asm.o ghash-clmulni-intel_glue.o
diff --git a/arch/x86/crypto/blake2s-glue.c b/arch/x86/crypto/blake2s-glue.c
index a40365ab301e..69853c13e8fb 100644
--- a/arch/x86/crypto/blake2s-glue.c
+++ b/arch/x86/crypto/blake2s-glue.c
@@ -5,7 +5,6 @@
 
 #include <crypto/internal/blake2s.h>
 #include <crypto/internal/simd.h>
-#include <crypto/internal/hash.h>
 
 #include <linux/types.h>
 #include <linux/jump_label.h>
@@ -28,9 +27,8 @@ asmlinkage void blake2s_compress_avx512(struct blake2s_state *state,
 static __ro_after_init DEFINE_STATIC_KEY_FALSE(blake2s_use_ssse3);
 static __ro_after_init DEFINE_STATIC_KEY_FALSE(blake2s_use_avx512);
 
-void blake2s_compress_arch(struct blake2s_state *state,
-			   const u8 *block, size_t nblocks,
-			   const u32 inc)
+void blake2s_compress(struct blake2s_state *state, const u8 *block,
+		      size_t nblocks, const u32 inc)
 {
 	/* SIMD disables preemption, so relax after processing each page. */
 	BUILD_BUG_ON(SZ_4K / BLAKE2S_BLOCK_SIZE < 8);
@@ -56,49 +54,12 @@ void blake2s_compress_arch(struct blake2s_state *state,
 		block += blocks * BLAKE2S_BLOCK_SIZE;
 	} while (nblocks);
 }
-EXPORT_SYMBOL(blake2s_compress_arch);
-
-static int crypto_blake2s_update_x86(struct shash_desc *desc,
-				     const u8 *in, unsigned int inlen)
-{
-	return crypto_blake2s_update(desc, in, inlen, blake2s_compress_arch);
-}
-
-static int crypto_blake2s_final_x86(struct shash_desc *desc, u8 *out)
-{
-	return crypto_blake2s_final(desc, out, blake2s_compress_arch);
-}
-
-#define BLAKE2S_ALG(name, driver_name, digest_size)			\
-	{								\
-		.base.cra_name		= name,				\
-		.base.cra_driver_name	= driver_name,			\
-		.base.cra_priority	= 200,				\
-		.base.cra_flags		= CRYPTO_ALG_OPTIONAL_KEY,	\
-		.base.cra_blocksize	= BLAKE2S_BLOCK_SIZE,		\
-		.base.cra_ctxsize	= sizeof(struct blake2s_tfm_ctx), \
-		.base.cra_module	= THIS_MODULE,			\
-		.digestsize		= digest_size,			\
-		.setkey			= crypto_blake2s_setkey,	\
-		.init			= crypto_blake2s_init,		\
-		.update			= crypto_blake2s_update_x86,	\
-		.final			= crypto_blake2s_final_x86,	\
-		.descsize		= sizeof(struct blake2s_state),	\
-	}
-
-static struct shash_alg blake2s_algs[] = {
-	BLAKE2S_ALG("blake2s-128", "blake2s-128-x86", BLAKE2S_128_HASH_SIZE),
-	BLAKE2S_ALG("blake2s-160", "blake2s-160-x86", BLAKE2S_160_HASH_SIZE),
-	BLAKE2S_ALG("blake2s-224", "blake2s-224-x86", BLAKE2S_224_HASH_SIZE),
-	BLAKE2S_ALG("blake2s-256", "blake2s-256-x86", BLAKE2S_256_HASH_SIZE),
-};
+EXPORT_SYMBOL(blake2s_compress);
 
 static int __init blake2s_mod_init(void)
 {
-	if (!boot_cpu_has(X86_FEATURE_SSSE3))
-		return 0;
-
-	static_branch_enable(&blake2s_use_ssse3);
+	if (boot_cpu_has(X86_FEATURE_SSSE3))
+		static_branch_enable(&blake2s_use_ssse3);
 
 	if (IS_ENABLED(CONFIG_AS_AVX512) &&
 	    boot_cpu_has(X86_FEATURE_AVX) &&
@@ -109,26 +70,9 @@ static int __init blake2s_mod_init(void)
 			      XFEATURE_MASK_AVX512, NULL))
 		static_branch_enable(&blake2s_use_avx512);
 
-	return IS_REACHABLE(CONFIG_CRYPTO_HASH) ?
-		crypto_register_shashes(blake2s_algs,
-					ARRAY_SIZE(blake2s_algs)) : 0;
-}
-
-static void __exit blake2s_mod_exit(void)
-{
-	if (IS_REACHABLE(CONFIG_CRYPTO_HASH) && boot_cpu_has(X86_FEATURE_SSSE3))
-		crypto_unregister_shashes(blake2s_algs, ARRAY_SIZE(blake2s_algs));
+	return 0;
 }
 
 module_init(blake2s_mod_init);
-module_exit(blake2s_mod_exit);
 
-MODULE_ALIAS_CRYPTO("blake2s-128");
-MODULE_ALIAS_CRYPTO("blake2s-128-x86");
-MODULE_ALIAS_CRYPTO("blake2s-160");
-MODULE_ALIAS_CRYPTO("blake2s-160-x86");
-MODULE_ALIAS_CRYPTO("blake2s-224");
-MODULE_ALIAS_CRYPTO("blake2s-224-x86");
-MODULE_ALIAS_CRYPTO("blake2s-256");
-MODULE_ALIAS_CRYPTO("blake2s-256-x86");
 MODULE_LICENSE("GPL v2");
diff --git a/arch/x86/crypto/blake2s-shash.c b/arch/x86/crypto/blake2s-shash.c
new file mode 100644
index 000000000000..f9e2fecdb761
--- /dev/null
+++ b/arch/x86/crypto/blake2s-shash.c
@@ -0,0 +1,77 @@
+// SPDX-License-Identifier: GPL-2.0 OR MIT
+/*
+ * Copyright (C) 2015-2019 Jason A. Donenfeld <Jason@zx2c4.com>. All Rights Reserved.
+ */
+
+#include <crypto/internal/blake2s.h>
+#include <crypto/internal/simd.h>
+#include <crypto/internal/hash.h>
+
+#include <linux/types.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/sizes.h>
+
+#include <asm/cpufeature.h>
+#include <asm/processor.h>
+
+static int crypto_blake2s_update_x86(struct shash_desc *desc,
+				     const u8 *in, unsigned int inlen)
+{
+	return crypto_blake2s_update(desc, in, inlen, blake2s_compress);
+}
+
+static int crypto_blake2s_final_x86(struct shash_desc *desc, u8 *out)
+{
+	return crypto_blake2s_final(desc, out, blake2s_compress);
+}
+
+#define BLAKE2S_ALG(name, driver_name, digest_size)			\
+	{								\
+		.base.cra_name		= name,				\
+		.base.cra_driver_name	= driver_name,			\
+		.base.cra_priority	= 200,				\
+		.base.cra_flags		= CRYPTO_ALG_OPTIONAL_KEY,	\
+		.base.cra_blocksize	= BLAKE2S_BLOCK_SIZE,		\
+		.base.cra_ctxsize	= sizeof(struct blake2s_tfm_ctx), \
+		.base.cra_module	= THIS_MODULE,			\
+		.digestsize		= digest_size,			\
+		.setkey			= crypto_blake2s_setkey,	\
+		.init			= crypto_blake2s_init,		\
+		.update			= crypto_blake2s_update_x86,	\
+		.final			= crypto_blake2s_final_x86,	\
+		.descsize		= sizeof(struct blake2s_state),	\
+	}
+
+static struct shash_alg blake2s_algs[] = {
+	BLAKE2S_ALG("blake2s-128", "blake2s-128-x86", BLAKE2S_128_HASH_SIZE),
+	BLAKE2S_ALG("blake2s-160", "blake2s-160-x86", BLAKE2S_160_HASH_SIZE),
+	BLAKE2S_ALG("blake2s-224", "blake2s-224-x86", BLAKE2S_224_HASH_SIZE),
+	BLAKE2S_ALG("blake2s-256", "blake2s-256-x86", BLAKE2S_256_HASH_SIZE),
+};
+
+static int __init blake2s_mod_init(void)
+{
+	if (IS_REACHABLE(CONFIG_CRYPTO_HASH) && boot_cpu_has(X86_FEATURE_SSSE3))
+		return crypto_register_shashes(blake2s_algs, ARRAY_SIZE(blake2s_algs));
+	return 0;
+}
+
+static void __exit blake2s_mod_exit(void)
+{
+	if (IS_REACHABLE(CONFIG_CRYPTO_HASH) && boot_cpu_has(X86_FEATURE_SSSE3))
+		crypto_unregister_shashes(blake2s_algs, ARRAY_SIZE(blake2s_algs));
+}
+
+module_init(blake2s_mod_init);
+module_exit(blake2s_mod_exit);
+
+MODULE_ALIAS_CRYPTO("blake2s-128");
+MODULE_ALIAS_CRYPTO("blake2s-128-x86");
+MODULE_ALIAS_CRYPTO("blake2s-160");
+MODULE_ALIAS_CRYPTO("blake2s-160-x86");
+MODULE_ALIAS_CRYPTO("blake2s-224");
+MODULE_ALIAS_CRYPTO("blake2s-224-x86");
+MODULE_ALIAS_CRYPTO("blake2s-256");
+MODULE_ALIAS_CRYPTO("blake2s-256-x86");
+MODULE_LICENSE("GPL v2");
diff --git a/crypto/Kconfig b/crypto/Kconfig
index 285f82647d2b..55718de56137 100644
--- a/crypto/Kconfig
+++ b/crypto/Kconfig
@@ -1919,9 +1919,10 @@ config CRYPTO_STATS
 config CRYPTO_HASH_INFO
 	bool
 
-source "lib/crypto/Kconfig"
 source "drivers/crypto/Kconfig"
 source "crypto/asymmetric_keys/Kconfig"
 source "certs/Kconfig"
 
 endif	# if CRYPTO
+
+source "lib/crypto/Kconfig"
diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
index 6cccc3dc00bc..b2a4f998c180 100644
--- a/drivers/net/Kconfig
+++ b/drivers/net/Kconfig
@@ -81,7 +81,6 @@ config WIREGUARD
 	select CRYPTO
 	select CRYPTO_LIB_CURVE25519
 	select CRYPTO_LIB_CHACHA20POLY1305
-	select CRYPTO_LIB_BLAKE2S
 	select CRYPTO_CHACHA20_X86_64 if X86 && 64BIT
 	select CRYPTO_POLY1305_X86_64 if X86 && 64BIT
 	select CRYPTO_BLAKE2S_X86 if X86 && 64BIT
diff --git a/include/crypto/internal/blake2s.h b/include/crypto/internal/blake2s.h
index 8e50d487500f..d39cfa0d333e 100644
--- a/include/crypto/internal/blake2s.h
+++ b/include/crypto/internal/blake2s.h
@@ -11,11 +11,11 @@
 #include <crypto/internal/hash.h>
 #include <linux/string.h>
 
-void blake2s_compress_generic(struct blake2s_state *state,const u8 *block,
+void blake2s_compress_generic(struct blake2s_state *state, const u8 *block,
 			      size_t nblocks, const u32 inc);
 
-void blake2s_compress_arch(struct blake2s_state *state,const u8 *block,
-			   size_t nblocks, const u32 inc);
+void blake2s_compress(struct blake2s_state *state, const u8 *block,
+		      size_t nblocks, const u32 inc);
 
 bool blake2s_selftest(void);
 
diff --git a/lib/crypto/Kconfig b/lib/crypto/Kconfig
index 545ccbddf6a1..8620f38e117c 100644
--- a/lib/crypto/Kconfig
+++ b/lib/crypto/Kconfig
@@ -1,7 +1,5 @@
 # SPDX-License-Identifier: GPL-2.0
 
-comment "Crypto library routines"
-
 config CRYPTO_LIB_AES
 	tristate
 
@@ -9,14 +7,14 @@ config CRYPTO_LIB_ARC4
 	tristate
 
 config CRYPTO_ARCH_HAVE_LIB_BLAKE2S
-	tristate
+	bool
 	help
 	  Declares whether the architecture provides an arch-specific
 	  accelerated implementation of the Blake2s library interface,
 	  either builtin or as a module.
 
 config CRYPTO_LIB_BLAKE2S_GENERIC
-	tristate
+	def_bool !CRYPTO_ARCH_HAVE_LIB_BLAKE2S
 	help
 	  This symbol can be depended upon by arch implementations of the
 	  Blake2s library interface that require the generic code as a
@@ -24,15 +22,6 @@ config CRYPTO_LIB_BLAKE2S_GENERIC
 	  implementation is enabled, this implementation serves the users
 	  of CRYPTO_LIB_BLAKE2S.
 
-config CRYPTO_LIB_BLAKE2S
-	tristate "BLAKE2s hash function library"
-	depends on CRYPTO_ARCH_HAVE_LIB_BLAKE2S || !CRYPTO_ARCH_HAVE_LIB_BLAKE2S
-	select CRYPTO_LIB_BLAKE2S_GENERIC if CRYPTO_ARCH_HAVE_LIB_BLAKE2S=n
-	help
-	  Enable the Blake2s library interface. This interface may be fulfilled
-	  by either the generic implementation or an arch-specific one, if one
-	  is available and enabled.
-
 config CRYPTO_ARCH_HAVE_LIB_CHACHA
 	tristate
 	help
@@ -51,7 +40,7 @@ config CRYPTO_LIB_CHACHA_GENERIC
 	  of CRYPTO_LIB_CHACHA.
 
 config CRYPTO_LIB_CHACHA
-	tristate "ChaCha library interface"
+	tristate
 	depends on CRYPTO_ARCH_HAVE_LIB_CHACHA || !CRYPTO_ARCH_HAVE_LIB_CHACHA
 	select CRYPTO_LIB_CHACHA_GENERIC if CRYPTO_ARCH_HAVE_LIB_CHACHA=n
 	help
@@ -76,7 +65,7 @@ config CRYPTO_LIB_CURVE25519_GENERIC
 	  of CRYPTO_LIB_CURVE25519.
 
 config CRYPTO_LIB_CURVE25519
-	tristate "Curve25519 scalar multiplication library"
+	tristate
 	depends on CRYPTO_ARCH_HAVE_LIB_CURVE25519 || !CRYPTO_ARCH_HAVE_LIB_CURVE25519
 	select CRYPTO_LIB_CURVE25519_GENERIC if CRYPTO_ARCH_HAVE_LIB_CURVE25519=n
 	help
@@ -111,7 +100,7 @@ config CRYPTO_LIB_POLY1305_GENERIC
 	  of CRYPTO_LIB_POLY1305.
 
 config CRYPTO_LIB_POLY1305
-	tristate "Poly1305 library interface"
+	tristate
 	depends on CRYPTO_ARCH_HAVE_LIB_POLY1305 || !CRYPTO_ARCH_HAVE_LIB_POLY1305
 	select CRYPTO_LIB_POLY1305_GENERIC if CRYPTO_ARCH_HAVE_LIB_POLY1305=n
 	help
@@ -120,7 +109,7 @@ config CRYPTO_LIB_POLY1305
 	  is available and enabled.
 
 config CRYPTO_LIB_CHACHA20POLY1305
-	tristate "ChaCha20-Poly1305 AEAD support (8-byte nonce library version)"
+	tristate
 	depends on CRYPTO_ARCH_HAVE_LIB_CHACHA || !CRYPTO_ARCH_HAVE_LIB_CHACHA
 	depends on CRYPTO_ARCH_HAVE_LIB_POLY1305 || !CRYPTO_ARCH_HAVE_LIB_POLY1305
 	select CRYPTO_LIB_CHACHA
diff --git a/lib/crypto/Makefile b/lib/crypto/Makefile
index 73205ed269ba..ed43a41f2dcc 100644
--- a/lib/crypto/Makefile
+++ b/lib/crypto/Makefile
@@ -10,11 +10,10 @@ libaes-y					:= aes.o
 obj-$(CONFIG_CRYPTO_LIB_ARC4)			+= libarc4.o
 libarc4-y					:= arc4.o
 
-obj-$(CONFIG_CRYPTO_LIB_BLAKE2S_GENERIC)	+= libblake2s-generic.o
-libblake2s-generic-y				+= blake2s-generic.o
-
-obj-$(CONFIG_CRYPTO_LIB_BLAKE2S)		+= libblake2s.o
-libblake2s-y					+= blake2s.o
+# blake2s is used by the /dev/random driver which is always builtin
+obj-y						+= libblake2s.o
+libblake2s-y					:= blake2s.o
+libblake2s-$(CONFIG_CRYPTO_LIB_BLAKE2S_GENERIC)	+= blake2s-generic.o
 
 obj-$(CONFIG_CRYPTO_LIB_CHACHA20POLY1305)	+= libchacha20poly1305.o
 libchacha20poly1305-y				+= chacha20poly1305.o
diff --git a/lib/crypto/blake2s-generic.c b/lib/crypto/blake2s-generic.c
index 04ff8df24513..75ccb3e633e6 100644
--- a/lib/crypto/blake2s-generic.c
+++ b/lib/crypto/blake2s-generic.c
@@ -37,7 +37,11 @@ static inline void blake2s_increment_counter(struct blake2s_state *state,
 	state->t[1] += (state->t[0] < inc);
 }
 
-void blake2s_compress_generic(struct blake2s_state *state,const u8 *block,
+void blake2s_compress(struct blake2s_state *state, const u8 *block,
+		      size_t nblocks, const u32 inc)
+		      __weak __alias(blake2s_compress_generic);
+
+void blake2s_compress_generic(struct blake2s_state *state, const u8 *block,
 			      size_t nblocks, const u32 inc)
 {
 	u32 m[16];
diff --git a/lib/crypto/blake2s.c b/lib/crypto/blake2s.c
index 4055aa593ec4..93f2ae051370 100644
--- a/lib/crypto/blake2s.c
+++ b/lib/crypto/blake2s.c
@@ -16,12 +16,6 @@
 #include <linux/init.h>
 #include <linux/bug.h>
 
-#if IS_ENABLED(CONFIG_CRYPTO_ARCH_HAVE_LIB_BLAKE2S)
-#  define blake2s_compress blake2s_compress_arch
-#else
-#  define blake2s_compress blake2s_compress_generic
-#endif
-
 void blake2s_update(struct blake2s_state *state, const u8 *in, size_t inlen)
 {
 	__blake2s_update(state, in, inlen, blake2s_compress);
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* Re: [PATCH v7] lib/crypto: blake2s: include as built-in
  2022-01-03 12:31                         ` [PATCH v7] " Jason A. Donenfeld
@ 2022-01-04  1:21                           ` Herbert Xu
  2022-01-04 17:02                             ` Ard Biesheuvel
  0 siblings, 1 reply; 23+ messages in thread
From: Herbert Xu @ 2022-01-04  1:21 UTC (permalink / raw)
  To: Jason A. Donenfeld
  Cc: linux-kernel, Ard Biesheuvel, Greg Kroah-Hartman,
	Masahiro Yamada, linux-kbuild, linux-crypto

On Mon, Jan 03, 2022 at 01:31:52PM +0100, Jason A. Donenfeld wrote:
> In preparation for using blake2s in the RNG, we change the way that it
> is wired-in to the build system. Instead of using ifdefs to select the
> right symbol, we use weak symbols. And because ARM doesn't need the
> generic implementation, we make the generic one default only if an arch
> library doesn't need it already, and then have arch libraries that do
> need it opt-in. So that the arch libraries can remain tristate rather
> than bool, we then split the shash part from the glue code.
> 
> Acked-by: Ard Biesheuvel <ardb@kernel.org>
> Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Masahiro Yamada <masahiroy@kernel.org>
> Cc: linux-kbuild@vger.kernel.org
> Cc: Herbert Xu <herbert@gondor.apana.org.au>
> Cc: linux-crypto@vger.kernel.org
> Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
> ---
> Herbert - As discussed, I still intend to take this via the
> crng/random.git tree because it forms a dependency, and I'd like to send
> a pull very early in the 5.17 cycle. I've taken some care to minimize
> changes to the {arch/*/}crypto/Kconfig files, as you mentioned this
> might cause some conflicts. Your tree should work cleanly on top of this
> commit.

Acked-by: Herbert Xu <herbert@gondor.apana.org.au>

> Changes v6->v7:
> - Split arch shash implementations out from the glue code, so that they
>   can remain as tristates, and we thus don't need to touch
>   arch/*/crypto/Kconfig at all.

This looks good to me although I confess that I haven't actually
tried to build it :) Hopefully the build robots will take care of
this.

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

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v7] lib/crypto: blake2s: include as built-in
  2022-01-04  1:21                           ` Herbert Xu
@ 2022-01-04 17:02                             ` Ard Biesheuvel
  2022-01-04 17:04                               ` Jason A. Donenfeld
                                                 ` (2 more replies)
  0 siblings, 3 replies; 23+ messages in thread
From: Ard Biesheuvel @ 2022-01-04 17:02 UTC (permalink / raw)
  To: Herbert Xu
  Cc: Jason A. Donenfeld, Linux Kernel Mailing List,
	Greg Kroah-Hartman, Masahiro Yamada, Linux Kbuild mailing list,
	Linux Crypto Mailing List

On Tue, 4 Jan 2022 at 02:22, Herbert Xu <herbert@gondor.apana.org.au> wrote:
>
> On Mon, Jan 03, 2022 at 01:31:52PM +0100, Jason A. Donenfeld wrote:
> > In preparation for using blake2s in the RNG, we change the way that it
> > is wired-in to the build system. Instead of using ifdefs to select the
> > right symbol, we use weak symbols. And because ARM doesn't need the
> > generic implementation, we make the generic one default only if an arch
> > library doesn't need it already, and then have arch libraries that do
> > need it opt-in. So that the arch libraries can remain tristate rather
> > than bool, we then split the shash part from the glue code.
> >
> > Acked-by: Ard Biesheuvel <ardb@kernel.org>
> > Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > Cc: Masahiro Yamada <masahiroy@kernel.org>
> > Cc: linux-kbuild@vger.kernel.org
> > Cc: Herbert Xu <herbert@gondor.apana.org.au>
> > Cc: linux-crypto@vger.kernel.org
> > Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
> > ---
> > Herbert - As discussed, I still intend to take this via the
> > crng/random.git tree because it forms a dependency, and I'd like to send
> > a pull very early in the 5.17 cycle. I've taken some care to minimize
> > changes to the {arch/*/}crypto/Kconfig files, as you mentioned this
> > might cause some conflicts. Your tree should work cleanly on top of this
> > commit.
>
> Acked-by: Herbert Xu <herbert@gondor.apana.org.au>
>
> > Changes v6->v7:
> > - Split arch shash implementations out from the glue code, so that they
> >   can remain as tristates, and we thus don't need to touch
> >   arch/*/crypto/Kconfig at all.
>
> This looks good to me although I confess that I haven't actually
> tried to build it :) Hopefully the build robots will take care of
> this.
>

The only downside here is that the ARM/x86 accelerated shashes and the
generic shash now use the same core transform, right? Given that the
generic blake2s shash is never used for anything in the kernel, the
only reason for its existence was to be able to use the randomized
crypto testing infrastructure to test the arch code.

Ergo, there is no point in retaining the blake2s shashes and we can
simply remove all of them. (Note that blake2b is used as an shash via
the crypto API by btrfs, but blake2s is only used via the library API)

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v7] lib/crypto: blake2s: include as built-in
  2022-01-04 17:02                             ` Ard Biesheuvel
@ 2022-01-04 17:04                               ` Jason A. Donenfeld
  2022-01-05  0:28                               ` Herbert Xu
  2022-01-05 21:53                               ` Eric Biggers
  2 siblings, 0 replies; 23+ messages in thread
From: Jason A. Donenfeld @ 2022-01-04 17:04 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: Herbert Xu, Linux Kernel Mailing List, Greg Kroah-Hartman,
	Masahiro Yamada, Linux Kbuild mailing list,
	Linux Crypto Mailing List

Hi Ard,

On Tue, Jan 4, 2022 at 6:03 PM Ard Biesheuvel <ardb@kernel.org> wrote:
> The only downside here is that the ARM/x86 accelerated shashes and the
> generic shash now use the same core transform, right? Given that the
> generic blake2s shash is never used for anything in the kernel, the
> only reason for its existence was to be able to use the randomized
> crypto testing infrastructure to test the arch code.
>
> Ergo, there is no point in retaining the blake2s shashes and we can
> simply remove all of them. (Note that blake2b is used as an shash via
> the crypto API by btrfs, but blake2s is only used via the library API)

That makes sense and is fine with me. Let's do that in a separate
commit later. I've got a bunch of things I'd like to fix up in the
general lib/crypto vs crypto split that are kind of interrelated.

Jason

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v7] lib/crypto: blake2s: include as built-in
  2022-01-04 17:02                             ` Ard Biesheuvel
  2022-01-04 17:04                               ` Jason A. Donenfeld
@ 2022-01-05  0:28                               ` Herbert Xu
  2022-01-05 21:53                               ` Eric Biggers
  2 siblings, 0 replies; 23+ messages in thread
From: Herbert Xu @ 2022-01-05  0:28 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: Jason A. Donenfeld, Linux Kernel Mailing List,
	Greg Kroah-Hartman, Masahiro Yamada, Linux Kbuild mailing list,
	Linux Crypto Mailing List

On Tue, Jan 04, 2022 at 06:02:52PM +0100, Ard Biesheuvel wrote:
>
> The only downside here is that the ARM/x86 accelerated shashes and the
> generic shash now use the same core transform, right? Given that the
> generic blake2s shash is never used for anything in the kernel, the
> only reason for its existence was to be able to use the randomized
> crypto testing infrastructure to test the arch code.
> 
> Ergo, there is no point in retaining the blake2s shashes and we can
> simply remove all of them. (Note that blake2b is used as an shash via
> the crypto API by btrfs, but blake2s is only used via the library API)

I have no objections to removing blake2s.

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

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v7] lib/crypto: blake2s: include as built-in
  2022-01-04 17:02                             ` Ard Biesheuvel
  2022-01-04 17:04                               ` Jason A. Donenfeld
  2022-01-05  0:28                               ` Herbert Xu
@ 2022-01-05 21:53                               ` Eric Biggers
  2022-01-05 22:01                                 ` Ard Biesheuvel
  2 siblings, 1 reply; 23+ messages in thread
From: Eric Biggers @ 2022-01-05 21:53 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: Herbert Xu, Jason A. Donenfeld, Linux Kernel Mailing List,
	Greg Kroah-Hartman, Masahiro Yamada, Linux Kbuild mailing list,
	Linux Crypto Mailing List

On Tue, Jan 04, 2022 at 06:02:52PM +0100, Ard Biesheuvel wrote:
> The only downside here is that the ARM/x86 accelerated shashes and the
> generic shash now use the same core transform, right?

I don't see how this is the case, given that crypto/blake2s_generic.c still uses
blake2s_compress_generic(), not blake2s_compress().

- Eric

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v7] lib/crypto: blake2s: include as built-in
  2022-01-05 21:53                               ` Eric Biggers
@ 2022-01-05 22:01                                 ` Ard Biesheuvel
  2022-01-05 22:09                                   ` Eric Biggers
  0 siblings, 1 reply; 23+ messages in thread
From: Ard Biesheuvel @ 2022-01-05 22:01 UTC (permalink / raw)
  To: Eric Biggers
  Cc: Herbert Xu, Jason A. Donenfeld, Linux Kernel Mailing List,
	Greg Kroah-Hartman, Masahiro Yamada, Linux Kbuild mailing list,
	Linux Crypto Mailing List

On Wed, 5 Jan 2022 at 22:53, Eric Biggers <ebiggers@kernel.org> wrote:
>
> On Tue, Jan 04, 2022 at 06:02:52PM +0100, Ard Biesheuvel wrote:
> > The only downside here is that the ARM/x86 accelerated shashes and the
> > generic shash now use the same core transform, right?
>
> I don't see how this is the case, given that crypto/blake2s_generic.c still uses
> blake2s_compress_generic(), not blake2s_compress().
>

Ah ok, I stand corrected then.

So what are your thoughts on this? Should we keep the shashes while
they have no users?

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v7] lib/crypto: blake2s: include as built-in
  2022-01-05 22:01                                 ` Ard Biesheuvel
@ 2022-01-05 22:09                                   ` Eric Biggers
  0 siblings, 0 replies; 23+ messages in thread
From: Eric Biggers @ 2022-01-05 22:09 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: Herbert Xu, Jason A. Donenfeld, Linux Kernel Mailing List,
	Greg Kroah-Hartman, Masahiro Yamada, Linux Kbuild mailing list,
	Linux Crypto Mailing List

On Wed, Jan 05, 2022 at 11:01:04PM +0100, Ard Biesheuvel wrote:
> On Wed, 5 Jan 2022 at 22:53, Eric Biggers <ebiggers@kernel.org> wrote:
> >
> > On Tue, Jan 04, 2022 at 06:02:52PM +0100, Ard Biesheuvel wrote:
> > > The only downside here is that the ARM/x86 accelerated shashes and the
> > > generic shash now use the same core transform, right?
> >
> > I don't see how this is the case, given that crypto/blake2s_generic.c still uses
> > blake2s_compress_generic(), not blake2s_compress().
> >
> 
> Ah ok, I stand corrected then.
> 
> So what are your thoughts on this? Should we keep the shashes while
> they have no users?

I don't know.  Removing unused stuff is good per se, but I wouldn't have
expected this to be something that is being considered here.  It's not like this
is a "controversial" algorithm, blake2b is already supported, and there could be
users of it already (dm-integrity, dm-verity, AF_ALG, etc.).  If this is going
to happen, then the acceptance criteria for new algorithms need to get *much*
stricter, so that algorithms aren't constantly being added and removed.

- Eric

^ permalink raw reply	[flat|nested] 23+ messages in thread

end of thread, other threads:[~2022-01-05 22:11 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-12-23 14:11 [PATCH v2 1/2] lib/crypto: blake2s: include as built-in Jason A. Donenfeld
2021-12-23 14:20 ` Ard Biesheuvel
2021-12-24 13:35 ` Greg KH
2021-12-25  9:26 ` Masahiro Yamada
2021-12-25 10:26   ` Ard Biesheuvel
2021-12-25 15:47     ` Masahiro Yamada
2021-12-27 13:43       ` Jason A. Donenfeld
2021-12-27 13:47         ` [PATCH v3] " Jason A. Donenfeld
2021-12-27 14:20           ` [PATCH v4] " Jason A. Donenfeld
2022-01-01 15:59             ` [PATCH v5] " Jason A. Donenfeld
2022-01-02 20:42               ` [PATCH v6] " Jason A. Donenfeld
2022-01-03  3:23                 ` Herbert Xu
2022-01-03  3:45                   ` Jason A. Donenfeld
2022-01-03  4:06                     ` Herbert Xu
2022-01-03 11:57                       ` Jason A. Donenfeld
2022-01-03 12:31                         ` [PATCH v7] " Jason A. Donenfeld
2022-01-04  1:21                           ` Herbert Xu
2022-01-04 17:02                             ` Ard Biesheuvel
2022-01-04 17:04                               ` Jason A. Donenfeld
2022-01-05  0:28                               ` Herbert Xu
2022-01-05 21:53                               ` Eric Biggers
2022-01-05 22:01                                 ` Ard Biesheuvel
2022-01-05 22:09                                   ` Eric Biggers

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).