Re: [PATCH v3 08/18] powerpc/crc32: expose CRC32 functions through lib

[Michael Ellerman <mpe@ellerman.id.au>]

Eric Biggers <ebiggers@kernel.org> writes:
> From: Eric Biggers <ebiggers@google.com>
>
> Move the powerpc CRC32C assembly code into the lib directory and wire it
> up to the library interface.  This allows it to be used without going
> through the crypto API.  It remains usable via the crypto API too via
> the shash algorithms that use the library interface.  Thus all the
> arch-specific "shash" code becomes unnecessary and is removed.
>
> Note: to see the diff from arch/powerpc/crypto/crc32c-vpmsum_glue.c to
> arch/powerpc/lib/crc32-glue.c, view this commit with 'git show -M10'.
>
> Reviewed-by: Ard Biesheuvel <ardb@kernel.org>
> Signed-off-by: Eric Biggers <ebiggers@google.com>
> ---
>  arch/powerpc/Kconfig                          |   1 +
>  arch/powerpc/configs/powernv_defconfig        |   1 -
>  arch/powerpc/configs/ppc64_defconfig          |   1 -
>  arch/powerpc/crypto/Kconfig                   |  15 +-
>  arch/powerpc/crypto/Makefile                  |   2 -
>  arch/powerpc/crypto/crc32c-vpmsum_glue.c      | 173 ------------------
>  arch/powerpc/crypto/crct10dif-vpmsum_asm.S    |   2 +-
>  arch/powerpc/lib/Makefile                     |   3 +
>  arch/powerpc/lib/crc32-glue.c                 |  92 ++++++++++
>  .../{crypto => lib}/crc32-vpmsum_core.S       |   0
>  .../{crypto => lib}/crc32c-vpmsum_asm.S       |   0
>  11 files changed, 98 insertions(+), 192 deletions(-)
>  delete mode 100644 arch/powerpc/crypto/crc32c-vpmsum_glue.c
>  create mode 100644 arch/powerpc/lib/crc32-glue.c
>  rename arch/powerpc/{crypto => lib}/crc32-vpmsum_core.S (100%)
>  rename arch/powerpc/{crypto => lib}/crc32c-vpmsum_asm.S (100%)

Acked-by: Michael Ellerman <mpe@ellerman.id.au> (powerpc)

...
> deleted file mode 100644
> index 63760b7dbb76..000000000000
> --- a/arch/powerpc/crypto/crc32c-vpmsum_glue.c
> +++ /dev/null
> @@ -1,173 +0,0 @@
> -// SPDX-License-Identifier: GPL-2.0-only
> -
...
> -static int __init crc32c_vpmsum_mod_init(void)
> -{
> -	if (!cpu_has_feature(CPU_FTR_ARCH_207S))
> -		return -ENODEV;
> -
> -	return crypto_register_shash(&alg);
> -}
> -
> -static void __exit crc32c_vpmsum_mod_fini(void)
> -{
> -	crypto_unregister_shash(&alg);
> -}
> -
> -module_cpu_feature_match(PPC_MODULE_FEATURE_VEC_CRYPTO, crc32c_vpmsum_mod_init);
> -module_exit(crc32c_vpmsum_mod_fini);
> -
> -MODULE_AUTHOR("Anton Blanchard <anton@samba.org>");
> -MODULE_DESCRIPTION("CRC32C using vector polynomial multiply-sum instructions");
> -MODULE_LICENSE("GPL");
> -MODULE_ALIAS_CRYPTO("crc32c");
> -MODULE_ALIAS_CRYPTO("crc32c-vpmsum");
...
> new file mode 100644
> index 000000000000..e9730f028afb
> --- /dev/null
> +++ b/arch/powerpc/lib/crc32-glue.c
> @@ -0,0 +1,92 @@
> +// SPDX-License-Identifier: GPL-2.0-only
...
> +
> +static int __init crc32_powerpc_init(void)
> +{
> +	if (cpu_has_feature(CPU_FTR_ARCH_207S) &&
> +	    (cur_cpu_spec->cpu_user_features2 & PPC_FEATURE2_VEC_CRYPTO))
> +		static_branch_enable(&have_vec_crypto);

For any other reviewers, this looks like a new cpu feature check, but
it's not. In the old code there was a module feature check:

  module_cpu_feature_match(PPC_MODULE_FEATURE_VEC_CRYPTO, crc32c_vpmsum_mod_init);

And PPC_MODULE_FEATURE_VEC_CRYPTO maps to PPC_FEATURE2_VEC_CRYPTO, so
the logic is equivalent.

cheers