Re: [PATCH v3 2/3] MIPS: crypto: Add crc32 and crc32c hw accelerated module

[James Hogan]

On Thu, Feb 15, 2018 at 02:22:14PM -0800, Eric Biggers wrote:
> On Fri, Feb 09, 2018 at 10:11:06PM +, James Hogan wrote:
> > +static struct shash_alg crc32_alg = {
> > +   .digestsize =   CHKSUM_DIGEST_SIZE,
> > +   .setkey =   chksum_setkey,
> > +   .init   =   chksum_init,
> > +   .update =   chksum_update,
> > +   .final  =   chksum_final,
> > +   .finup  =   chksum_finup,
> > +   .digest =   chksum_digest,
> > +   .descsize   =   sizeof(struct chksum_desc_ctx),
> > +   .base   =   {
> > +   .cra_name   =   "crc32",
> > +   .cra_driver_name=   "crc32-mips-hw",
> > +   .cra_priority   =   300,
> > +   .cra_blocksize  =   CHKSUM_BLOCK_SIZE,
> > +   .cra_alignmask  =   0,
> > +   .cra_ctxsize=   sizeof(struct chksum_ctx),
> > +   .cra_module =   THIS_MODULE,
> > +   .cra_init   =   chksum_cra_init,
> > +   }
> > +};
> > +
> > +
> > +static struct shash_alg crc32c_alg = {
> > +   .digestsize =   CHKSUM_DIGEST_SIZE,
> > +   .setkey =   chksum_setkey,
> > +   .init   =   chksum_init,
> > +   .update =   chksumc_update,
> > +   .final  =   chksumc_final,
> > +   .finup  =   chksumc_finup,
> > +   .digest =   chksumc_digest,
> > +   .descsize   =   sizeof(struct chksum_desc_ctx),
> > +   .base   =   {
> > +   .cra_name   =   "crc32c",
> > +   .cra_driver_name=   "crc32c-mips-hw",
> > +   .cra_priority   =   300,
> > +   .cra_blocksize  =   CHKSUM_BLOCK_SIZE,
> > +   .cra_alignmask  =   0,
> > +   .cra_ctxsize=   sizeof(struct chksum_ctx),
> > +   .cra_module =   THIS_MODULE,
> > +   .cra_init   =   chksum_cra_init,
> > +   }
> > +};
>
> Does this actually work on the latest kernel?  Now hash algorithms must have
> CRYPTO_ALG_OPTIONAL_KEY in .cra_flags if they have a .setkey method but don't
> require it to be called, otherwise the crypto API will think it's a keyed hash
> and not allow it to be used without a key.  I had to add this flag to the 
> other
> CRC32 and CRC32C algorithms (commit a208fa8f33031).  One of the CRC32C test
> vectors even doesn't set a key so it should be causing the self-tests to fail
> for "crc32c-mips-hw".  (We should add such a test vector for CRC32 too, 
> though.)

Thanks Eric. It does indeed fail now with:

alg: hash: digest failed on test 1 for crc32c-mips-hw: ret=161

I'll squash in the following change:

diff --git a/arch/mips/crypto/crc32-mips.c b/arch/mips/crypto/crc32-mips.c
index 8d4122f37fa5..7d1d2425746f 100644
--- a/arch/mips/crypto/crc32-mips.c
+++ b/arch/mips/crypto/crc32-mips.c
@@ -284,6 +284,7 @@ static struct shash_alg crc32_alg = {
.cra_name   =   "crc32",
.cra_driver_name=   "crc32-mips-hw",
.cra_priority   =   300,
+   .cra_flags  =   CRYPTO_ALG_OPTIONAL_KEY,
.cra_blocksize  =   CHKSUM_BLOCK_SIZE,
.cra_alignmask  =   0,
.cra_ctxsize=   sizeof(struct chksum_ctx),
@@ -305,6 +306,7 @@ static struct shash_alg crc32c_alg = {
.cra_name   =   "crc32c",
.cra_driver_name=   "crc32c-mips-hw",
.cra_priority   =   300,
+   .cra_flags  =   CRYPTO_ALG_OPTIONAL_KEY,
.cra_blocksize  =   CHKSUM_BLOCK_SIZE,
.cra_alignmask  =   0,
.cra_ctxsize=   sizeof(struct chksum_ctx),

Cheers
James


signature.asc
Description: Digital signature

Re: [PATCH v3 2/3] MIPS: crypto: Add crc32 and crc32c hw accelerated module

[Eric Biggers]

On Fri, Feb 09, 2018 at 10:11:06PM +, James Hogan wrote:
> +static struct shash_alg crc32_alg = {
> + .digestsize =   CHKSUM_DIGEST_SIZE,
> + .setkey =   chksum_setkey,
> + .init   =   chksum_init,
> + .update =   chksum_update,
> + .final  =   chksum_final,
> + .finup  =   chksum_finup,
> + .digest =   chksum_digest,
> + .descsize   =   sizeof(struct chksum_desc_ctx),
> + .base   =   {
> + .cra_name   =   "crc32",
> + .cra_driver_name=   "crc32-mips-hw",
> + .cra_priority   =   300,
> + .cra_blocksize  =   CHKSUM_BLOCK_SIZE,
> + .cra_alignmask  =   0,
> + .cra_ctxsize=   sizeof(struct chksum_ctx),
> + .cra_module =   THIS_MODULE,
> + .cra_init   =   chksum_cra_init,
> + }
> +};
> +
> +
> +static struct shash_alg crc32c_alg = {
> +   .digestsize =   CHKSUM_DIGEST_SIZE,
> +   .setkey =   chksum_setkey,
> +   .init   =   chksum_init,
> +   .update =   chksumc_update,
> +   .final  =   chksumc_final,
> +   .finup  =   chksumc_finup,
> +   .digest =   chksumc_digest,
> +   .descsize   =   sizeof(struct chksum_desc_ctx),
> +   .base   =   {
> +   .cra_name   =   "crc32c",
> +   .cra_driver_name=   "crc32c-mips-hw",
> +   .cra_priority   =   300,
> +   .cra_blocksize  =   CHKSUM_BLOCK_SIZE,
> +   .cra_alignmask  =   0,
> +   .cra_ctxsize=   sizeof(struct chksum_ctx),
> +   .cra_module =   THIS_MODULE,
> +   .cra_init   =   chksum_cra_init,
> +   }
> +};

Does this actually work on the latest kernel?  Now hash algorithms must have
CRYPTO_ALG_OPTIONAL_KEY in .cra_flags if they have a .setkey method but don't
require it to be called, otherwise the crypto API will think it's a keyed hash
and not allow it to be used without a key.  I had to add this flag to the other
CRC32 and CRC32C algorithms (commit a208fa8f33031).  One of the CRC32C test
vectors even doesn't set a key so it should be causing the self-tests to fail
for "crc32c-mips-hw".  (We should add such a test vector for CRC32 too, though.)

Eric

Re: [PATCH v3 2/3] MIPS: crypto: Add crc32 and crc32c hw accelerated module

[James Hogan]

On Thu, Feb 15, 2018 at 04:33:16PM +0800, Herbert Xu wrote:
> Acked-by: Herbert Xu 
 
Thanks Herbert,

Series applied for 4.17.

Cheers
James


signature.asc
Description: Digital signature

Re: [PATCH v3 2/3] MIPS: crypto: Add crc32 and crc32c hw accelerated module

[Herbert Xu]

On Fri, Feb 09, 2018 at 10:11:06PM +, James Hogan wrote:
> From: Marcin Nowakowski 
> 
> This module registers crc32 and crc32c algorithms that use the
> optional CRC32[bhwd] and CRC32C[bhwd] instructions in MIPSr6 cores.
> 
> Signed-off-by: Marcin Nowakowski 
> Signed-off-by: James Hogan 
> Cc: Ralf Baechle 
> Cc: Herbert Xu 
> Cc: "David S. Miller" 
> Cc: linux-m...@linux-mips.org
> Cc: linux-crypto@vger.kernel.org
> ---
> Changes in v3:
>  - Convert to using assembler macros to support CRC instructions on
>older toolchains, using the helpers merged for 4.16. This removes the
>need to hardcode either rt or rs (i.e. as $v0 (CRC_REGISTER) and
>$at), and drops the C "register" keywords sprinkled everywhere.
>  - Minor whitespace rearrangement of _CRC32 macro.
>  - Add SPDX-License-Identifier to crc32-mips.c and the crypo Makefile.
>  - Update copyright from ImgTec to MIPS Tech, LLC.
>  - Update imgtec.com email addresses to mips.com.
> 
> Changes in v2:
>  - minor code refactoring as suggested by JamesH which produces
>a better assembly output for 32-bit builds
> ---
>  arch/mips/Kconfig |   4 +-
>  arch/mips/Makefile|   3 +-
>  arch/mips/crypto/Makefile |   6 +-
>  arch/mips/crypto/crc32-mips.c | 346 +++-
>  crypto/Kconfig|   9 +-
>  5 files changed, 368 insertions(+)
>  create mode 100644 arch/mips/crypto/Makefile
>  create mode 100644 arch/mips/crypto/crc32-mips.c

Acked-by: Herbert Xu 
-- 
Email: Herbert Xu 
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

[PATCH v3 2/3] MIPS: crypto: Add crc32 and crc32c hw accelerated module

[James Hogan]

From: Marcin Nowakowski 

This module registers crc32 and crc32c algorithms that use the
optional CRC32[bhwd] and CRC32C[bhwd] instructions in MIPSr6 cores.

Signed-off-by: Marcin Nowakowski 
Signed-off-by: James Hogan 
Cc: Ralf Baechle 
Cc: Herbert Xu 
Cc: "David S. Miller" 
Cc: linux-m...@linux-mips.org
Cc: linux-crypto@vger.kernel.org
---
Changes in v3:
 - Convert to using assembler macros to support CRC instructions on
   older toolchains, using the helpers merged for 4.16. This removes the
   need to hardcode either rt or rs (i.e. as $v0 (CRC_REGISTER) and
   $at), and drops the C "register" keywords sprinkled everywhere.
 - Minor whitespace rearrangement of _CRC32 macro.
 - Add SPDX-License-Identifier to crc32-mips.c and the crypo Makefile.
 - Update copyright from ImgTec to MIPS Tech, LLC.
 - Update imgtec.com email addresses to mips.com.

Changes in v2:
 - minor code refactoring as suggested by JamesH which produces
   a better assembly output for 32-bit builds
---
 arch/mips/Kconfig |   4 +-
 arch/mips/Makefile|   3 +-
 arch/mips/crypto/Makefile |   6 +-
 arch/mips/crypto/crc32-mips.c | 346 +++-
 crypto/Kconfig|   9 +-
 5 files changed, 368 insertions(+)
 create mode 100644 arch/mips/crypto/Makefile
 create mode 100644 arch/mips/crypto/crc32-mips.c

diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
index ac0f5bb10f0b..cccd17c07bfc 100644
--- a/arch/mips/Kconfig
+++ b/arch/mips/Kconfig
@@ -2023,6 +2023,7 @@ config CPU_MIPSR6
select CPU_HAS_RIXI
select HAVE_ARCH_BITREVERSE
select MIPS_ASID_BITS_VARIABLE
+   select MIPS_CRC_SUPPORT
select MIPS_SPRAM
 
 config EVA
@@ -2490,6 +2491,9 @@ config MIPS_ASID_BITS
 config MIPS_ASID_BITS_VARIABLE
bool
 
+config MIPS_CRC_SUPPORT
+   bool
+
 #
 # - Highmem only makes sense for the 32-bit kernel.
 # - The current highmem code will only work properly on physically indexed
diff --git a/arch/mips/Makefile b/arch/mips/Makefile
index d1ca839c3981..44a6ed53d018 100644
--- a/arch/mips/Makefile
+++ b/arch/mips/Makefile
@@ -222,6 +222,8 @@ xpa-cflags-y:= 
$(mips-cflags)
 xpa-cflags-$(micromips-ase)+= -mmicromips 
-Wa$(comma)-fatal-warnings
 toolchain-xpa  := $(call cc-option-yn,$(xpa-cflags-y) 
-mxpa)
 cflags-$(toolchain-xpa)+= -DTOOLCHAIN_SUPPORTS_XPA
+toolchain-crc  := $(call cc-option-yn,$(mips-cflags) 
-Wa$(comma)-mcrc)
+cflags-$(toolchain-crc)+= -DTOOLCHAIN_SUPPORTS_CRC
 
 #
 # Firmware support
@@ -330,6 +332,7 @@ libs-y  += arch/mips/math-emu/
 # See arch/mips/Kbuild for content of core part of the kernel
 core-y += arch/mips/
 
+drivers-$(CONFIG_MIPS_CRC_SUPPORT) += arch/mips/crypto/
 drivers-$(CONFIG_OPROFILE) += arch/mips/oprofile/
 
 # suspend and hibernation support
diff --git a/arch/mips/crypto/Makefile b/arch/mips/crypto/Makefile
new file mode 100644
index ..e07aca572c2e
--- /dev/null
+++ b/arch/mips/crypto/Makefile
@@ -0,0 +1,6 @@
+# SPDX-License-Identifier: GPL-2.0
+#
+# Makefile for MIPS crypto files..
+#
+
+obj-$(CONFIG_CRYPTO_CRC32_MIPS) += crc32-mips.o
diff --git a/arch/mips/crypto/crc32-mips.c b/arch/mips/crypto/crc32-mips.c
new file mode 100644
index ..8d4122f37fa5
--- /dev/null
+++ b/arch/mips/crypto/crc32-mips.c
@@ -0,0 +1,346 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * crc32-mips.c - CRC32 and CRC32C using optional MIPSr6 instructions
+ *
+ * Module based on arm64/crypto/crc32-arm.c
+ *
+ * Copyright (C) 2014 Linaro Ltd 
+ * Copyright (C) 2018 MIPS Tech, LLC
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+
+enum crc_op_size {
+   b, h, w, d,
+};
+
+enum crc_type {
+   crc32,
+   crc32c,
+};
+
+#ifndef TOOLCHAIN_SUPPORTS_CRC
+#define _ASM_MACRO_CRC32(OP, SZ, TYPE)   \
+_ASM_MACRO_3R(OP, rt, rs, rt2,   \
+   ".ifnc  \\rt, \\rt2\n\t"  \
+   ".error \"invalid operands \\\"" #OP " \\rt,\\rs,\\rt2\\\"\"\n\t" \
+   ".endif\n\t"  \
+   _ASM_INSN_IF_MIPS(0x7c0f | (__rt << 16) | (__rs << 21) |  \
+ ((SZ) <<  6) | ((TYPE) << 8))   \
+   _ASM_INSN32_IF_MM(0x0030 | (__rs << 16) | (__rt << 21) |  \
+ ((SZ) << 14) | ((TYPE) << 3)))
+_ASM_MACRO_CRC32(crc32b,  0, 0);
+_ASM_MACRO_CRC32(crc32h,  1, 0);
+_ASM_MACRO_CRC32(crc32w,  2, 0);
+_ASM_MACRO_CRC32(crc32d,  3, 0);
+_ASM_MACRO_CRC32(crc32cb, 0, 1);
+_ASM_MACRO_CRC32(crc32ch, 1, 1);
+_ASM_MACRO_CRC32(crc32cw, 2,