All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] MIPS CRC instruction support
@ 2017-09-27 12:18 Marcin Nowakowski
  2017-09-27 12:18 ` [PATCH 1/2] MIPS: add crc instruction support flag to elf_hwcap Marcin Nowakowski
  2017-09-27 12:18   ` Marcin Nowakowski
  0 siblings, 2 replies; 12+ messages in thread
From: Marcin Nowakowski @ 2017-09-27 12:18 UTC (permalink / raw)
  To: Linux MIPS Mailing List; +Cc: Ralf Baechle, Marcin Nowakowski

MIPSr6 architecture introduces a new CRC32(C) instruction.
The following patches add a crypto acceleration module for
crc32 and crc32c algorithms using the new instructions.

Marcin Nowakowski (2):
  MIPS: add crc instruction support flag to elf_hwcap
  MIPS: crypto: Add crc32 and crc32c hw accelerated module

 arch/mips/Kconfig                  |   4 +
 arch/mips/Makefile                 |   3 +
 arch/mips/crypto/Makefile          |   5 +
 arch/mips/crypto/crc32-mips.c      | 361 +++++++++++++++++++++++++++++++++++++
 arch/mips/include/asm/mipsregs.h   |   1 +
 arch/mips/include/uapi/asm/hwcap.h |   1 +
 arch/mips/kernel/cpu-probe.c       |   3 +
 crypto/Kconfig                     |   9 +
 8 files changed, 387 insertions(+)
 create mode 100644 arch/mips/crypto/Makefile
 create mode 100644 arch/mips/crypto/crc32-mips.c

-- 
2.7.4

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

* [PATCH 1/2] MIPS: add crc instruction support flag to elf_hwcap
  2017-09-27 12:18 [PATCH 0/2] MIPS CRC instruction support Marcin Nowakowski
@ 2017-09-27 12:18 ` Marcin Nowakowski
  2017-09-29 21:35   ` James Hogan
  2017-09-27 12:18   ` Marcin Nowakowski
  1 sibling, 1 reply; 12+ messages in thread
From: Marcin Nowakowski @ 2017-09-27 12:18 UTC (permalink / raw)
  To: Linux MIPS Mailing List; +Cc: Ralf Baechle, Marcin Nowakowski

Indicate that CRC32 and CRC32C instuctions are supported by the CPU
through elf_hwcap flags.

This will be used by a follow-up commit that introduces crc32(c) crypto
acceleration modules and is required by GENERIC_CPU_AUTOPROBE feature.

Signed-off-by: Marcin Nowakowski <marcin.nowakowski@imgtec.com>
---
 arch/mips/include/asm/mipsregs.h   | 1 +
 arch/mips/include/uapi/asm/hwcap.h | 1 +
 arch/mips/kernel/cpu-probe.c       | 3 +++
 3 files changed, 5 insertions(+)

diff --git a/arch/mips/include/asm/mipsregs.h b/arch/mips/include/asm/mipsregs.h
index a681092..9db53cc 100644
--- a/arch/mips/include/asm/mipsregs.h
+++ b/arch/mips/include/asm/mipsregs.h
@@ -664,6 +664,7 @@
 #define MIPS_CONF5_FRE		(_ULCAST_(1) << 8)
 #define MIPS_CONF5_UFE		(_ULCAST_(1) << 9)
 #define MIPS_CONF5_CA2		(_ULCAST_(1) << 14)
+#define MIPS_CONF5_CRCP		(_ULCAST_(1) << 18)
 #define MIPS_CONF5_MSAEN	(_ULCAST_(1) << 27)
 #define MIPS_CONF5_EVA		(_ULCAST_(1) << 28)
 #define MIPS_CONF5_CV		(_ULCAST_(1) << 29)
diff --git a/arch/mips/include/uapi/asm/hwcap.h b/arch/mips/include/uapi/asm/hwcap.h
index c7484a7..c7d2cb6 100644
--- a/arch/mips/include/uapi/asm/hwcap.h
+++ b/arch/mips/include/uapi/asm/hwcap.h
@@ -4,5 +4,6 @@
 /* HWCAP flags */
 #define HWCAP_MIPS_R6		(1 << 0)
 #define HWCAP_MIPS_MSA		(1 << 1)
+#define HWCAP_MIPS_CRC32	(1 << 2)
 
 #endif /* _UAPI_ASM_HWCAP_H */
diff --git a/arch/mips/kernel/cpu-probe.c b/arch/mips/kernel/cpu-probe.c
index cf3fd54..6b07b73 100644
--- a/arch/mips/kernel/cpu-probe.c
+++ b/arch/mips/kernel/cpu-probe.c
@@ -848,6 +848,9 @@ static inline unsigned int decode_config5(struct cpuinfo_mips *c)
 	if (config5 & MIPS_CONF5_CA2)
 		c->ases |= MIPS_ASE_MIPS16E2;
 
+	if (config5 & MIPS_CONF5_CRCP)
+		elf_hwcap |= HWCAP_MIPS_CRC32;
+
 	return config5 & MIPS_CONF_M;
 }
 
-- 
2.7.4

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

* [PATCH 2/2] MIPS: crypto: Add crc32 and crc32c hw accelerated module
@ 2017-09-27 12:18   ` Marcin Nowakowski
  0 siblings, 0 replies; 12+ messages in thread
From: Marcin Nowakowski @ 2017-09-27 12:18 UTC (permalink / raw)
  To: Linux MIPS Mailing List
  Cc: Ralf Baechle, Marcin Nowakowski, linux-crypto, Herbert Xu,
	David S. Miller

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 <marcin.nowakowski@imgtec.com>
Cc: linux-crypto@vger.kernel.org
Cc: Herbert Xu <herbert@gondor.apana.org.au>
Cc: "David S. Miller" <davem@davemloft.net>

---
 arch/mips/Kconfig             |   4 +
 arch/mips/Makefile            |   3 +
 arch/mips/crypto/Makefile     |   5 +
 arch/mips/crypto/crc32-mips.c | 361 ++++++++++++++++++++++++++++++++++++++++++
 crypto/Kconfig                |   9 ++
 5 files changed, 382 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 cb7fcc4..0f96812 100644
--- a/arch/mips/Kconfig
+++ b/arch/mips/Kconfig
@@ -2036,6 +2036,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
@@ -2503,6 +2504,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 a96d97a..aa77536 100644
--- a/arch/mips/Makefile
+++ b/arch/mips/Makefile
@@ -216,6 +216,8 @@ cflags-$(toolchain-msa)			+= -DTOOLCHAIN_SUPPORTS_MSA
 endif
 toolchain-virt				:= $(call cc-option-yn,$(mips-cflags) -mvirt)
 cflags-$(toolchain-virt)		+= -DTOOLCHAIN_SUPPORTS_VIRT
+toolchain-crc				:= $(call cc-option-yn,$(mips-cflags) -Wa$(comma)-mcrc)
+cflags-$(toolchain-crc)			+= -DTOOLCHAIN_SUPPORTS_CRC
 
 #
 # Firmware support
@@ -324,6 +326,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 0000000..665c725
--- /dev/null
+++ b/arch/mips/crypto/Makefile
@@ -0,0 +1,5 @@
+#
+# 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 0000000..dfa8bb1
--- /dev/null
+++ b/arch/mips/crypto/crc32-mips.c
@@ -0,0 +1,361 @@
+/*
+ * crc32-mips.c - CRC32 and CRC32C using optional MIPSr6 instructions
+ *
+ * Module based on arm64/crypto/crc32-arm.c
+ *
+ * Copyright (C) 2014 Linaro Ltd <yazen.ghannam@linaro.org>
+ * Copyright (C) 2017 Imagination Technologies, Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/unaligned/access_ok.h>
+#include <linux/cpufeature.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/string.h>
+
+#include <crypto/internal/hash.h>
+
+enum crc_op_size {
+	b, h, w, d,
+};
+
+enum crc_type {
+	crc32,
+	crc32c,
+};
+
+#ifdef TOOLCHAIN_SUPPORTS_CRC
+
+#define _CRC32(crc, value, size, type)		\
+do {						\
+	__asm__ __volatile__(			\
+	".set	push\n\t"			\
+	".set	crc\n\t"			\
+	#type #size "	%0, %1, %0\n\t"		\
+	".set	pop\n\t"			\
+	: "+r" (crc)				\
+	: "r" (value)				\
+);						\
+} while(0)
+
+#define CRC_REGISTER
+
+#else	/* TOOLCHAIN_SUPPORTS_CRC */
+/*
+ * Crc argument is currently ignored and the assembly below assumes
+ * the crc is stored in $2. As the register number is encoded in the
+ * instruction we can't let the compiler chose the register it wants.
+ * An alternative is to change the code to do
+ * move $2, %0
+ * crc32
+ * move %0, $2
+ * but that adds unnecessary operations that the crc32 operation is
+ * designed to avoid. This issue can go away once the assembler
+ * is extended to support this operation and the compiler can make
+ * the right register choice automatically
+ */
+
+#define _CRC32(crc, value, size, type)						\
+do {										\
+	__asm__ __volatile__(							\
+	".set	push\n\t"							\
+	".set	noat\n\t"							\
+	"move	$at, %1\n\t"							\
+	"# " #type #size "	%0, $at, %0\n\t"				\
+	_ASM_INSN_IF_MIPS(0x7c00000f | (2 << 16) | (1 << 21) | (%2 << 6) | (%3 << 8))	\
+	_ASM_INSN32_IF_MM(0x00000030 | (1 << 16) | (2 << 21) | (%2 << 14) | (%3 << 3))	\
+	".set	pop\n\t"							\
+	: "+r" (crc)								\
+	: "r" (value), "i" (size), "i" (type)					\
+);										\
+} while(0)
+
+#define CRC_REGISTER __asm__("$2")
+#endif	/* !TOOLCHAIN_SUPPORTS_CRC */
+
+#define CRC32(crc, value, size) \
+	_CRC32(crc, value, size, crc32)
+
+#define CRC32C(crc, value, size) \
+	_CRC32(crc, value, size, crc32c)
+
+static u32 crc32_mips_le_hw(u32 crc_, const u8 *p, unsigned int len)
+{
+	s64 length = len;
+	register u32 crc CRC_REGISTER = crc_;
+
+#ifdef CONFIG_64BIT
+	while ((length -= sizeof(u64)) >= 0) {
+		register u64 value = get_unaligned_le64(p);
+
+		CRC32(crc, value, d);
+		p += sizeof(u64);
+	}
+
+	if (length & sizeof(u32)) {
+#else /* !CONFIG_64BIT */
+	while ((length -= sizeof(u32)) >= 0) {
+#endif
+		register u32 value = get_unaligned_le32(p);
+
+		CRC32(crc, value, w);
+		p += sizeof(u32);
+	}
+
+	if (length & sizeof(u16)) {
+		register u16 value = get_unaligned_le16(p);
+
+		CRC32(crc, value, h);
+		p += sizeof(u16);
+	}
+
+	if (length & sizeof(u8)) {
+		register u8 value = *p++;
+
+		CRC32(crc, value, b);
+	}
+
+	return crc;
+}
+
+static u32 crc32c_mips_le_hw(u32 crc_, const u8 *p, unsigned int len)
+{
+	s64 length = len;
+	register u32 crc __asm__("$2") = crc_;
+
+#ifdef CONFIG_64BIT
+	while ((length -= sizeof(u64)) >= 0) {
+		register u64 value = get_unaligned_le64(p);
+
+		CRC32C(crc, value, d);
+		p += sizeof(u64);
+	}
+
+	if (length & sizeof(u32)) {
+#else /* !CONFIG_64BIT */
+	while ((length -= sizeof(u32)) >= 0) {
+#endif
+		register u32 value = get_unaligned_le32(p);
+
+		CRC32C(crc, value, w);
+		p += sizeof(u32);
+	}
+
+	if (length & sizeof(u16)) {
+		register u16 value = get_unaligned_le16(p);
+
+		CRC32C(crc, value, h);
+		p += sizeof(u16);
+	}
+
+	if (length & sizeof(u8)) {
+		register u8 value = *p++;
+
+		CRC32C(crc, value, b);
+	}
+	return crc;
+}
+
+#define CHKSUM_BLOCK_SIZE	1
+#define CHKSUM_DIGEST_SIZE	4
+
+struct chksum_ctx {
+	u32 key;
+};
+
+struct chksum_desc_ctx {
+	u32 crc;
+};
+
+static int chksum_init(struct shash_desc *desc)
+{
+	struct chksum_ctx *mctx = crypto_shash_ctx(desc->tfm);
+	struct chksum_desc_ctx *ctx = shash_desc_ctx(desc);
+
+	ctx->crc = mctx->key;
+
+	return 0;
+}
+
+/*
+ * Setting the seed allows arbitrary accumulators and flexible XOR policy
+ * If your algorithm starts with ~0, then XOR with ~0 before you set
+ * the seed.
+ */
+static int chksum_setkey(struct crypto_shash *tfm, const u8 *key,
+			 unsigned int keylen)
+{
+	struct chksum_ctx *mctx = crypto_shash_ctx(tfm);
+
+	if (keylen != sizeof(mctx->key)) {
+		crypto_shash_set_flags(tfm, CRYPTO_TFM_RES_BAD_KEY_LEN);
+		return -EINVAL;
+	}
+	mctx->key = get_unaligned_le32(key);
+	return 0;
+}
+
+static int chksum_update(struct shash_desc *desc, const u8 *data,
+			 unsigned int length)
+{
+	struct chksum_desc_ctx *ctx = shash_desc_ctx(desc);
+
+	ctx->crc = crc32_mips_le_hw(ctx->crc, data, length);
+	return 0;
+}
+
+static int chksumc_update(struct shash_desc *desc, const u8 *data,
+			 unsigned int length)
+{
+	struct chksum_desc_ctx *ctx = shash_desc_ctx(desc);
+
+	ctx->crc = crc32c_mips_le_hw(ctx->crc, data, length);
+	return 0;
+}
+
+static int chksum_final(struct shash_desc *desc, u8 *out)
+{
+	struct chksum_desc_ctx *ctx = shash_desc_ctx(desc);
+
+	put_unaligned_le32(ctx->crc, out);
+	return 0;
+}
+
+static int chksumc_final(struct shash_desc *desc, u8 *out)
+{
+	struct chksum_desc_ctx *ctx = shash_desc_ctx(desc);
+
+	put_unaligned_le32(~ctx->crc, out);
+	return 0;
+}
+
+static int __chksum_finup(u32 crc, const u8 *data, unsigned int len, u8 *out)
+{
+	put_unaligned_le32(crc32_mips_le_hw(crc, data, len), out);
+	return 0;
+}
+
+static int __chksumc_finup(u32 crc, const u8 *data, unsigned int len, u8 *out)
+{
+	put_unaligned_le32(~crc32c_mips_le_hw(crc, data, len), out);
+	return 0;
+}
+
+static int chksum_finup(struct shash_desc *desc, const u8 *data,
+			unsigned int len, u8 *out)
+{
+	struct chksum_desc_ctx *ctx = shash_desc_ctx(desc);
+
+	return __chksum_finup(ctx->crc, data, len, out);
+}
+
+static int chksumc_finup(struct shash_desc *desc, const u8 *data,
+			unsigned int len, u8 *out)
+{
+	struct chksum_desc_ctx *ctx = shash_desc_ctx(desc);
+
+	return __chksumc_finup(ctx->crc, data, len, out);
+}
+
+static int chksum_digest(struct shash_desc *desc, const u8 *data,
+			 unsigned int length, u8 *out)
+{
+	struct chksum_ctx *mctx = crypto_shash_ctx(desc->tfm);
+
+	return __chksum_finup(mctx->key, data, length, out);
+}
+
+static int chksumc_digest(struct shash_desc *desc, const u8 *data,
+			 unsigned int length, u8 *out)
+{
+	struct chksum_ctx *mctx = crypto_shash_ctx(desc->tfm);
+
+	return __chksumc_finup(mctx->key, data, length, out);
+}
+
+static int chksum_cra_init(struct crypto_tfm *tfm)
+{
+	struct chksum_ctx *mctx = crypto_tfm_ctx(tfm);
+
+	mctx->key = ~0;
+	return 0;
+}
+
+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,
+	}
+};
+
+static int __init crc32_mod_init(void)
+{
+	int err;
+
+	err = crypto_register_shash(&crc32_alg);
+
+	if (err)
+		return err;
+
+	err = crypto_register_shash(&crc32c_alg);
+
+	if (err) {
+		crypto_unregister_shash(&crc32_alg);
+		return err;
+	}
+
+	return 0;
+}
+
+static void __exit crc32_mod_exit(void)
+{
+	crypto_unregister_shash(&crc32_alg);
+	crypto_unregister_shash(&crc32c_alg);
+}
+
+MODULE_AUTHOR("Marcin Nowakowski <marcin.nowakowski@imgtec.com");
+MODULE_DESCRIPTION("CRC32 and CRC32C using optional MIPS instructions");
+MODULE_LICENSE("GPL v2");
+
+module_cpu_feature_match(MIPS_CRC32, crc32_mod_init);
+module_exit(crc32_mod_exit);
diff --git a/crypto/Kconfig b/crypto/Kconfig
index 28b1a0d..661971a 100644
--- a/crypto/Kconfig
+++ b/crypto/Kconfig
@@ -494,6 +494,15 @@ config CRYPTO_CRC32_PCLMUL
 	  which will enable any routine to use the CRC-32-IEEE 802.3 checksum
 	  and gain better performance as compared with the table implementation.
 
+config CRYPTO_CRC32_MIPS
+	tristate "CRC32c and CRC32 CRC algorithm (MIPS)"
+	depends on MIPS_CRC_SUPPORT
+	select CRYPTO_HASH
+	help
+	  CRC32c and CRC32 CRC algorithms implemented using mips crypto
+	  instructions, when available.
+
+
 config CRYPTO_CRCT10DIF
 	tristate "CRCT10DIF algorithm"
 	select CRYPTO_HASH
-- 
2.7.4

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

* [PATCH 2/2] MIPS: crypto: Add crc32 and crc32c hw accelerated module
@ 2017-09-27 12:18   ` Marcin Nowakowski
  0 siblings, 0 replies; 12+ messages in thread
From: Marcin Nowakowski @ 2017-09-27 12:18 UTC (permalink / raw)
  To: Linux MIPS Mailing List
  Cc: Ralf Baechle, Marcin Nowakowski, linux-crypto, Herbert Xu,
	David S. Miller

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 <marcin.nowakowski@imgtec.com>
Cc: linux-crypto@vger.kernel.org
Cc: Herbert Xu <herbert@gondor.apana.org.au>
Cc: "David S. Miller" <davem@davemloft.net>

---
 arch/mips/Kconfig             |   4 +
 arch/mips/Makefile            |   3 +
 arch/mips/crypto/Makefile     |   5 +
 arch/mips/crypto/crc32-mips.c | 361 ++++++++++++++++++++++++++++++++++++++++++
 crypto/Kconfig                |   9 ++
 5 files changed, 382 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 cb7fcc4..0f96812 100644
--- a/arch/mips/Kconfig
+++ b/arch/mips/Kconfig
@@ -2036,6 +2036,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
@@ -2503,6 +2504,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 a96d97a..aa77536 100644
--- a/arch/mips/Makefile
+++ b/arch/mips/Makefile
@@ -216,6 +216,8 @@ cflags-$(toolchain-msa)			+= -DTOOLCHAIN_SUPPORTS_MSA
 endif
 toolchain-virt				:= $(call cc-option-yn,$(mips-cflags) -mvirt)
 cflags-$(toolchain-virt)		+= -DTOOLCHAIN_SUPPORTS_VIRT
+toolchain-crc				:= $(call cc-option-yn,$(mips-cflags) -Wa$(comma)-mcrc)
+cflags-$(toolchain-crc)			+= -DTOOLCHAIN_SUPPORTS_CRC
 
 #
 # Firmware support
@@ -324,6 +326,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 0000000..665c725
--- /dev/null
+++ b/arch/mips/crypto/Makefile
@@ -0,0 +1,5 @@
+#
+# 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 0000000..dfa8bb1
--- /dev/null
+++ b/arch/mips/crypto/crc32-mips.c
@@ -0,0 +1,361 @@
+/*
+ * crc32-mips.c - CRC32 and CRC32C using optional MIPSr6 instructions
+ *
+ * Module based on arm64/crypto/crc32-arm.c
+ *
+ * Copyright (C) 2014 Linaro Ltd <yazen.ghannam@linaro.org>
+ * Copyright (C) 2017 Imagination Technologies, Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/unaligned/access_ok.h>
+#include <linux/cpufeature.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/string.h>
+
+#include <crypto/internal/hash.h>
+
+enum crc_op_size {
+	b, h, w, d,
+};
+
+enum crc_type {
+	crc32,
+	crc32c,
+};
+
+#ifdef TOOLCHAIN_SUPPORTS_CRC
+
+#define _CRC32(crc, value, size, type)		\
+do {						\
+	__asm__ __volatile__(			\
+	".set	push\n\t"			\
+	".set	crc\n\t"			\
+	#type #size "	%0, %1, %0\n\t"		\
+	".set	pop\n\t"			\
+	: "+r" (crc)				\
+	: "r" (value)				\
+);						\
+} while(0)
+
+#define CRC_REGISTER
+
+#else	/* TOOLCHAIN_SUPPORTS_CRC */
+/*
+ * Crc argument is currently ignored and the assembly below assumes
+ * the crc is stored in $2. As the register number is encoded in the
+ * instruction we can't let the compiler chose the register it wants.
+ * An alternative is to change the code to do
+ * move $2, %0
+ * crc32
+ * move %0, $2
+ * but that adds unnecessary operations that the crc32 operation is
+ * designed to avoid. This issue can go away once the assembler
+ * is extended to support this operation and the compiler can make
+ * the right register choice automatically
+ */
+
+#define _CRC32(crc, value, size, type)						\
+do {										\
+	__asm__ __volatile__(							\
+	".set	push\n\t"							\
+	".set	noat\n\t"							\
+	"move	$at, %1\n\t"							\
+	"# " #type #size "	%0, $at, %0\n\t"				\
+	_ASM_INSN_IF_MIPS(0x7c00000f | (2 << 16) | (1 << 21) | (%2 << 6) | (%3 << 8))	\
+	_ASM_INSN32_IF_MM(0x00000030 | (1 << 16) | (2 << 21) | (%2 << 14) | (%3 << 3))	\
+	".set	pop\n\t"							\
+	: "+r" (crc)								\
+	: "r" (value), "i" (size), "i" (type)					\
+);										\
+} while(0)
+
+#define CRC_REGISTER __asm__("$2")
+#endif	/* !TOOLCHAIN_SUPPORTS_CRC */
+
+#define CRC32(crc, value, size) \
+	_CRC32(crc, value, size, crc32)
+
+#define CRC32C(crc, value, size) \
+	_CRC32(crc, value, size, crc32c)
+
+static u32 crc32_mips_le_hw(u32 crc_, const u8 *p, unsigned int len)
+{
+	s64 length = len;
+	register u32 crc CRC_REGISTER = crc_;
+
+#ifdef CONFIG_64BIT
+	while ((length -= sizeof(u64)) >= 0) {
+		register u64 value = get_unaligned_le64(p);
+
+		CRC32(crc, value, d);
+		p += sizeof(u64);
+	}
+
+	if (length & sizeof(u32)) {
+#else /* !CONFIG_64BIT */
+	while ((length -= sizeof(u32)) >= 0) {
+#endif
+		register u32 value = get_unaligned_le32(p);
+
+		CRC32(crc, value, w);
+		p += sizeof(u32);
+	}
+
+	if (length & sizeof(u16)) {
+		register u16 value = get_unaligned_le16(p);
+
+		CRC32(crc, value, h);
+		p += sizeof(u16);
+	}
+
+	if (length & sizeof(u8)) {
+		register u8 value = *p++;
+
+		CRC32(crc, value, b);
+	}
+
+	return crc;
+}
+
+static u32 crc32c_mips_le_hw(u32 crc_, const u8 *p, unsigned int len)
+{
+	s64 length = len;
+	register u32 crc __asm__("$2") = crc_;
+
+#ifdef CONFIG_64BIT
+	while ((length -= sizeof(u64)) >= 0) {
+		register u64 value = get_unaligned_le64(p);
+
+		CRC32C(crc, value, d);
+		p += sizeof(u64);
+	}
+
+	if (length & sizeof(u32)) {
+#else /* !CONFIG_64BIT */
+	while ((length -= sizeof(u32)) >= 0) {
+#endif
+		register u32 value = get_unaligned_le32(p);
+
+		CRC32C(crc, value, w);
+		p += sizeof(u32);
+	}
+
+	if (length & sizeof(u16)) {
+		register u16 value = get_unaligned_le16(p);
+
+		CRC32C(crc, value, h);
+		p += sizeof(u16);
+	}
+
+	if (length & sizeof(u8)) {
+		register u8 value = *p++;
+
+		CRC32C(crc, value, b);
+	}
+	return crc;
+}
+
+#define CHKSUM_BLOCK_SIZE	1
+#define CHKSUM_DIGEST_SIZE	4
+
+struct chksum_ctx {
+	u32 key;
+};
+
+struct chksum_desc_ctx {
+	u32 crc;
+};
+
+static int chksum_init(struct shash_desc *desc)
+{
+	struct chksum_ctx *mctx = crypto_shash_ctx(desc->tfm);
+	struct chksum_desc_ctx *ctx = shash_desc_ctx(desc);
+
+	ctx->crc = mctx->key;
+
+	return 0;
+}
+
+/*
+ * Setting the seed allows arbitrary accumulators and flexible XOR policy
+ * If your algorithm starts with ~0, then XOR with ~0 before you set
+ * the seed.
+ */
+static int chksum_setkey(struct crypto_shash *tfm, const u8 *key,
+			 unsigned int keylen)
+{
+	struct chksum_ctx *mctx = crypto_shash_ctx(tfm);
+
+	if (keylen != sizeof(mctx->key)) {
+		crypto_shash_set_flags(tfm, CRYPTO_TFM_RES_BAD_KEY_LEN);
+		return -EINVAL;
+	}
+	mctx->key = get_unaligned_le32(key);
+	return 0;
+}
+
+static int chksum_update(struct shash_desc *desc, const u8 *data,
+			 unsigned int length)
+{
+	struct chksum_desc_ctx *ctx = shash_desc_ctx(desc);
+
+	ctx->crc = crc32_mips_le_hw(ctx->crc, data, length);
+	return 0;
+}
+
+static int chksumc_update(struct shash_desc *desc, const u8 *data,
+			 unsigned int length)
+{
+	struct chksum_desc_ctx *ctx = shash_desc_ctx(desc);
+
+	ctx->crc = crc32c_mips_le_hw(ctx->crc, data, length);
+	return 0;
+}
+
+static int chksum_final(struct shash_desc *desc, u8 *out)
+{
+	struct chksum_desc_ctx *ctx = shash_desc_ctx(desc);
+
+	put_unaligned_le32(ctx->crc, out);
+	return 0;
+}
+
+static int chksumc_final(struct shash_desc *desc, u8 *out)
+{
+	struct chksum_desc_ctx *ctx = shash_desc_ctx(desc);
+
+	put_unaligned_le32(~ctx->crc, out);
+	return 0;
+}
+
+static int __chksum_finup(u32 crc, const u8 *data, unsigned int len, u8 *out)
+{
+	put_unaligned_le32(crc32_mips_le_hw(crc, data, len), out);
+	return 0;
+}
+
+static int __chksumc_finup(u32 crc, const u8 *data, unsigned int len, u8 *out)
+{
+	put_unaligned_le32(~crc32c_mips_le_hw(crc, data, len), out);
+	return 0;
+}
+
+static int chksum_finup(struct shash_desc *desc, const u8 *data,
+			unsigned int len, u8 *out)
+{
+	struct chksum_desc_ctx *ctx = shash_desc_ctx(desc);
+
+	return __chksum_finup(ctx->crc, data, len, out);
+}
+
+static int chksumc_finup(struct shash_desc *desc, const u8 *data,
+			unsigned int len, u8 *out)
+{
+	struct chksum_desc_ctx *ctx = shash_desc_ctx(desc);
+
+	return __chksumc_finup(ctx->crc, data, len, out);
+}
+
+static int chksum_digest(struct shash_desc *desc, const u8 *data,
+			 unsigned int length, u8 *out)
+{
+	struct chksum_ctx *mctx = crypto_shash_ctx(desc->tfm);
+
+	return __chksum_finup(mctx->key, data, length, out);
+}
+
+static int chksumc_digest(struct shash_desc *desc, const u8 *data,
+			 unsigned int length, u8 *out)
+{
+	struct chksum_ctx *mctx = crypto_shash_ctx(desc->tfm);
+
+	return __chksumc_finup(mctx->key, data, length, out);
+}
+
+static int chksum_cra_init(struct crypto_tfm *tfm)
+{
+	struct chksum_ctx *mctx = crypto_tfm_ctx(tfm);
+
+	mctx->key = ~0;
+	return 0;
+}
+
+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,
+	}
+};
+
+static int __init crc32_mod_init(void)
+{
+	int err;
+
+	err = crypto_register_shash(&crc32_alg);
+
+	if (err)
+		return err;
+
+	err = crypto_register_shash(&crc32c_alg);
+
+	if (err) {
+		crypto_unregister_shash(&crc32_alg);
+		return err;
+	}
+
+	return 0;
+}
+
+static void __exit crc32_mod_exit(void)
+{
+	crypto_unregister_shash(&crc32_alg);
+	crypto_unregister_shash(&crc32c_alg);
+}
+
+MODULE_AUTHOR("Marcin Nowakowski <marcin.nowakowski@imgtec.com");
+MODULE_DESCRIPTION("CRC32 and CRC32C using optional MIPS instructions");
+MODULE_LICENSE("GPL v2");
+
+module_cpu_feature_match(MIPS_CRC32, crc32_mod_init);
+module_exit(crc32_mod_exit);
diff --git a/crypto/Kconfig b/crypto/Kconfig
index 28b1a0d..661971a 100644
--- a/crypto/Kconfig
+++ b/crypto/Kconfig
@@ -494,6 +494,15 @@ config CRYPTO_CRC32_PCLMUL
 	  which will enable any routine to use the CRC-32-IEEE 802.3 checksum
 	  and gain better performance as compared with the table implementation.
 
+config CRYPTO_CRC32_MIPS
+	tristate "CRC32c and CRC32 CRC algorithm (MIPS)"
+	depends on MIPS_CRC_SUPPORT
+	select CRYPTO_HASH
+	help
+	  CRC32c and CRC32 CRC algorithms implemented using mips crypto
+	  instructions, when available.
+
+
 config CRYPTO_CRCT10DIF
 	tristate "CRCT10DIF algorithm"
 	select CRYPTO_HASH
-- 
2.7.4

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

* Re: [PATCH 2/2] MIPS: crypto: Add crc32 and crc32c hw accelerated module
@ 2017-09-29 21:34     ` James Hogan
  0 siblings, 0 replies; 12+ messages in thread
From: James Hogan @ 2017-09-29 21:34 UTC (permalink / raw)
  To: Marcin Nowakowski
  Cc: Linux MIPS Mailing List, Ralf Baechle, linux-crypto, Herbert Xu,
	David S. Miller

[-- Attachment #1: Type: text/plain, Size: 13846 bytes --]

Hi Marcin,

On Wed, Sep 27, 2017 at 02:18:36PM +0200, Marcin Nowakowski wrote:
> 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 <marcin.nowakowski@imgtec.com>
> Cc: linux-crypto@vger.kernel.org
> Cc: Herbert Xu <herbert@gondor.apana.org.au>
> Cc: "David S. Miller" <davem@davemloft.net>
> 
> ---
>  arch/mips/Kconfig             |   4 +
>  arch/mips/Makefile            |   3 +
>  arch/mips/crypto/Makefile     |   5 +
>  arch/mips/crypto/crc32-mips.c | 361 ++++++++++++++++++++++++++++++++++++++++++
>  crypto/Kconfig                |   9 ++
>  5 files changed, 382 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 cb7fcc4..0f96812 100644
> --- a/arch/mips/Kconfig
> +++ b/arch/mips/Kconfig
> @@ -2036,6 +2036,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
> @@ -2503,6 +2504,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 a96d97a..aa77536 100644
> --- a/arch/mips/Makefile
> +++ b/arch/mips/Makefile
> @@ -216,6 +216,8 @@ cflags-$(toolchain-msa)			+= -DTOOLCHAIN_SUPPORTS_MSA
>  endif
>  toolchain-virt				:= $(call cc-option-yn,$(mips-cflags) -mvirt)
>  cflags-$(toolchain-virt)		+= -DTOOLCHAIN_SUPPORTS_VIRT
> +toolchain-crc				:= $(call cc-option-yn,$(mips-cflags) -Wa$(comma)-mcrc)
> +cflags-$(toolchain-crc)			+= -DTOOLCHAIN_SUPPORTS_CRC
>  
>  #
>  # Firmware support
> @@ -324,6 +326,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 0000000..665c725
> --- /dev/null
> +++ b/arch/mips/crypto/Makefile
> @@ -0,0 +1,5 @@
> +#
> +# 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 0000000..dfa8bb1
> --- /dev/null
> +++ b/arch/mips/crypto/crc32-mips.c
> @@ -0,0 +1,361 @@
> +/*
> + * crc32-mips.c - CRC32 and CRC32C using optional MIPSr6 instructions
> + *
> + * Module based on arm64/crypto/crc32-arm.c
> + *
> + * Copyright (C) 2014 Linaro Ltd <yazen.ghannam@linaro.org>
> + * Copyright (C) 2017 Imagination Technologies, Ltd.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/unaligned/access_ok.h>
> +#include <linux/cpufeature.h>
> +#include <linux/init.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/string.h>
> +
> +#include <crypto/internal/hash.h>
> +
> +enum crc_op_size {
> +	b, h, w, d,
> +};
> +
> +enum crc_type {
> +	crc32,
> +	crc32c,
> +};
> +
> +#ifdef TOOLCHAIN_SUPPORTS_CRC
> +
> +#define _CRC32(crc, value, size, type)		\
> +do {						\
> +	__asm__ __volatile__(			\
> +	".set	push\n\t"			\
> +	".set	crc\n\t"			\
> +	#type #size "	%0, %1, %0\n\t"		\
> +	".set	pop\n\t"			\

Technically the \n\t on the last line is redundant.

> +	: "+r" (crc)				\
> +	: "r" (value)				\
> +);						\
> +} while(0)
> +
> +#define CRC_REGISTER
> +
> +#else	/* TOOLCHAIN_SUPPORTS_CRC */
> +/*
> + * Crc argument is currently ignored and the assembly below assumes
> + * the crc is stored in $2. As the register number is encoded in the
> + * instruction we can't let the compiler chose the register it wants.
> + * An alternative is to change the code to do
> + * move $2, %0
> + * crc32
> + * move %0, $2
> + * but that adds unnecessary operations that the crc32 operation is
> + * designed to avoid. This issue can go away once the assembler
> + * is extended to support this operation and the compiler can make
> + * the right register choice automatically
> + */
> +
> +#define _CRC32(crc, value, size, type)						\
> +do {										\
> +	__asm__ __volatile__(							\
> +	".set	push\n\t"							\
> +	".set	noat\n\t"							\
> +	"move	$at, %1\n\t"							\
> +	"# " #type #size "	%0, $at, %0\n\t"				\
> +	_ASM_INSN_IF_MIPS(0x7c00000f | (2 << 16) | (1 << 21) | (%2 << 6) | (%3 << 8))	\
> +	_ASM_INSN32_IF_MM(0x00000030 | (1 << 16) | (2 << 21) | (%2 << 14) | (%3 << 3))	\

You should explicitly include <asm/mipsregs.h> for these macros.

> +	".set	pop\n\t"							\
> +	: "+r" (crc)								\
> +	: "r" (value), "i" (size), "i" (type)					\
> +);										\
> +} while(0)
> +
> +#define CRC_REGISTER __asm__("$2")
> +#endif	/* !TOOLCHAIN_SUPPORTS_CRC */
> +
> +#define CRC32(crc, value, size) \
> +	_CRC32(crc, value, size, crc32)
> +
> +#define CRC32C(crc, value, size) \
> +	_CRC32(crc, value, size, crc32c)
> +
> +static u32 crc32_mips_le_hw(u32 crc_, const u8 *p, unsigned int len)
> +{
> +	s64 length = len;

The need for 64-bit signed length is unfortunate. Do you get decent
assembly and comparable/better performance on 32-bit if you just use len
and only decrement it in the loops? i.e.

-	while ((length -= sizeof(uXX)) >= 0) {
+	while (len >= sizeof(uXX)) {
		register uXX value = get_unaligned_leXX(p);

		CRC32(crc, value, XX);
		p += sizeof(uXX);
+		len -= sizeof(uXX);
	}

That would be more readable too IMHO.

Just a thought.

Cheers
James


> +	register u32 crc CRC_REGISTER = crc_;
> +
> +#ifdef CONFIG_64BIT
> +	while ((length -= sizeof(u64)) >= 0) {
> +		register u64 value = get_unaligned_le64(p);
> +
> +		CRC32(crc, value, d);
> +		p += sizeof(u64);
> +	}
> +
> +	if (length & sizeof(u32)) {
> +#else /* !CONFIG_64BIT */
> +	while ((length -= sizeof(u32)) >= 0) {
> +#endif
> +		register u32 value = get_unaligned_le32(p);
> +
> +		CRC32(crc, value, w);
> +		p += sizeof(u32);
> +	}
> +
> +	if (length & sizeof(u16)) {
> +		register u16 value = get_unaligned_le16(p);
> +
> +		CRC32(crc, value, h);
> +		p += sizeof(u16);
> +	}
> +
> +	if (length & sizeof(u8)) {
> +		register u8 value = *p++;
> +
> +		CRC32(crc, value, b);
> +	}
> +
> +	return crc;
> +}
> +
> +static u32 crc32c_mips_le_hw(u32 crc_, const u8 *p, unsigned int len)
> +{
> +	s64 length = len;
> +	register u32 crc __asm__("$2") = crc_;
> +
> +#ifdef CONFIG_64BIT
> +	while ((length -= sizeof(u64)) >= 0) {
> +		register u64 value = get_unaligned_le64(p);
> +
> +		CRC32C(crc, value, d);
> +		p += sizeof(u64);
> +	}
> +
> +	if (length & sizeof(u32)) {
> +#else /* !CONFIG_64BIT */
> +	while ((length -= sizeof(u32)) >= 0) {
> +#endif
> +		register u32 value = get_unaligned_le32(p);
> +
> +		CRC32C(crc, value, w);
> +		p += sizeof(u32);
> +	}
> +
> +	if (length & sizeof(u16)) {
> +		register u16 value = get_unaligned_le16(p);
> +
> +		CRC32C(crc, value, h);
> +		p += sizeof(u16);
> +	}
> +
> +	if (length & sizeof(u8)) {
> +		register u8 value = *p++;
> +
> +		CRC32C(crc, value, b);
> +	}
> +	return crc;
> +}
> +
> +#define CHKSUM_BLOCK_SIZE	1
> +#define CHKSUM_DIGEST_SIZE	4
> +
> +struct chksum_ctx {
> +	u32 key;
> +};
> +
> +struct chksum_desc_ctx {
> +	u32 crc;
> +};
> +
> +static int chksum_init(struct shash_desc *desc)
> +{
> +	struct chksum_ctx *mctx = crypto_shash_ctx(desc->tfm);
> +	struct chksum_desc_ctx *ctx = shash_desc_ctx(desc);
> +
> +	ctx->crc = mctx->key;
> +
> +	return 0;
> +}
> +
> +/*
> + * Setting the seed allows arbitrary accumulators and flexible XOR policy
> + * If your algorithm starts with ~0, then XOR with ~0 before you set
> + * the seed.
> + */
> +static int chksum_setkey(struct crypto_shash *tfm, const u8 *key,
> +			 unsigned int keylen)
> +{
> +	struct chksum_ctx *mctx = crypto_shash_ctx(tfm);
> +
> +	if (keylen != sizeof(mctx->key)) {
> +		crypto_shash_set_flags(tfm, CRYPTO_TFM_RES_BAD_KEY_LEN);
> +		return -EINVAL;
> +	}
> +	mctx->key = get_unaligned_le32(key);
> +	return 0;
> +}
> +
> +static int chksum_update(struct shash_desc *desc, const u8 *data,
> +			 unsigned int length)
> +{
> +	struct chksum_desc_ctx *ctx = shash_desc_ctx(desc);
> +
> +	ctx->crc = crc32_mips_le_hw(ctx->crc, data, length);
> +	return 0;
> +}
> +
> +static int chksumc_update(struct shash_desc *desc, const u8 *data,
> +			 unsigned int length)
> +{
> +	struct chksum_desc_ctx *ctx = shash_desc_ctx(desc);
> +
> +	ctx->crc = crc32c_mips_le_hw(ctx->crc, data, length);
> +	return 0;
> +}
> +
> +static int chksum_final(struct shash_desc *desc, u8 *out)
> +{
> +	struct chksum_desc_ctx *ctx = shash_desc_ctx(desc);
> +
> +	put_unaligned_le32(ctx->crc, out);
> +	return 0;
> +}
> +
> +static int chksumc_final(struct shash_desc *desc, u8 *out)
> +{
> +	struct chksum_desc_ctx *ctx = shash_desc_ctx(desc);
> +
> +	put_unaligned_le32(~ctx->crc, out);
> +	return 0;
> +}
> +
> +static int __chksum_finup(u32 crc, const u8 *data, unsigned int len, u8 *out)
> +{
> +	put_unaligned_le32(crc32_mips_le_hw(crc, data, len), out);
> +	return 0;
> +}
> +
> +static int __chksumc_finup(u32 crc, const u8 *data, unsigned int len, u8 *out)
> +{
> +	put_unaligned_le32(~crc32c_mips_le_hw(crc, data, len), out);
> +	return 0;
> +}
> +
> +static int chksum_finup(struct shash_desc *desc, const u8 *data,
> +			unsigned int len, u8 *out)
> +{
> +	struct chksum_desc_ctx *ctx = shash_desc_ctx(desc);
> +
> +	return __chksum_finup(ctx->crc, data, len, out);
> +}
> +
> +static int chksumc_finup(struct shash_desc *desc, const u8 *data,
> +			unsigned int len, u8 *out)
> +{
> +	struct chksum_desc_ctx *ctx = shash_desc_ctx(desc);
> +
> +	return __chksumc_finup(ctx->crc, data, len, out);
> +}
> +
> +static int chksum_digest(struct shash_desc *desc, const u8 *data,
> +			 unsigned int length, u8 *out)
> +{
> +	struct chksum_ctx *mctx = crypto_shash_ctx(desc->tfm);
> +
> +	return __chksum_finup(mctx->key, data, length, out);
> +}
> +
> +static int chksumc_digest(struct shash_desc *desc, const u8 *data,
> +			 unsigned int length, u8 *out)
> +{
> +	struct chksum_ctx *mctx = crypto_shash_ctx(desc->tfm);
> +
> +	return __chksumc_finup(mctx->key, data, length, out);
> +}
> +
> +static int chksum_cra_init(struct crypto_tfm *tfm)
> +{
> +	struct chksum_ctx *mctx = crypto_tfm_ctx(tfm);
> +
> +	mctx->key = ~0;
> +	return 0;
> +}
> +
> +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,
> +	}
> +};
> +
> +static int __init crc32_mod_init(void)
> +{
> +	int err;
> +
> +	err = crypto_register_shash(&crc32_alg);
> +
> +	if (err)
> +		return err;
> +
> +	err = crypto_register_shash(&crc32c_alg);
> +
> +	if (err) {
> +		crypto_unregister_shash(&crc32_alg);
> +		return err;
> +	}
> +
> +	return 0;
> +}
> +
> +static void __exit crc32_mod_exit(void)
> +{
> +	crypto_unregister_shash(&crc32_alg);
> +	crypto_unregister_shash(&crc32c_alg);
> +}
> +
> +MODULE_AUTHOR("Marcin Nowakowski <marcin.nowakowski@imgtec.com");
> +MODULE_DESCRIPTION("CRC32 and CRC32C using optional MIPS instructions");
> +MODULE_LICENSE("GPL v2");
> +
> +module_cpu_feature_match(MIPS_CRC32, crc32_mod_init);
> +module_exit(crc32_mod_exit);
> diff --git a/crypto/Kconfig b/crypto/Kconfig
> index 28b1a0d..661971a 100644
> --- a/crypto/Kconfig
> +++ b/crypto/Kconfig
> @@ -494,6 +494,15 @@ config CRYPTO_CRC32_PCLMUL
>  	  which will enable any routine to use the CRC-32-IEEE 802.3 checksum
>  	  and gain better performance as compared with the table implementation.
>  
> +config CRYPTO_CRC32_MIPS
> +	tristate "CRC32c and CRC32 CRC algorithm (MIPS)"
> +	depends on MIPS_CRC_SUPPORT
> +	select CRYPTO_HASH
> +	help
> +	  CRC32c and CRC32 CRC algorithms implemented using mips crypto
> +	  instructions, when available.
> +
> +
>  config CRYPTO_CRCT10DIF
>  	tristate "CRCT10DIF algorithm"
>  	select CRYPTO_HASH
> -- 
> 2.7.4
> 
> 

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

* Re: [PATCH 2/2] MIPS: crypto: Add crc32 and crc32c hw accelerated module
@ 2017-09-29 21:34     ` James Hogan
  0 siblings, 0 replies; 12+ messages in thread
From: James Hogan @ 2017-09-29 21:34 UTC (permalink / raw)
  To: Marcin Nowakowski
  Cc: Linux MIPS Mailing List, Ralf Baechle, linux-crypto, Herbert Xu,
	David S. Miller

[-- Attachment #1: Type: text/plain, Size: 13846 bytes --]

Hi Marcin,

On Wed, Sep 27, 2017 at 02:18:36PM +0200, Marcin Nowakowski wrote:
> 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 <marcin.nowakowski@imgtec.com>
> Cc: linux-crypto@vger.kernel.org
> Cc: Herbert Xu <herbert@gondor.apana.org.au>
> Cc: "David S. Miller" <davem@davemloft.net>
> 
> ---
>  arch/mips/Kconfig             |   4 +
>  arch/mips/Makefile            |   3 +
>  arch/mips/crypto/Makefile     |   5 +
>  arch/mips/crypto/crc32-mips.c | 361 ++++++++++++++++++++++++++++++++++++++++++
>  crypto/Kconfig                |   9 ++
>  5 files changed, 382 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 cb7fcc4..0f96812 100644
> --- a/arch/mips/Kconfig
> +++ b/arch/mips/Kconfig
> @@ -2036,6 +2036,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
> @@ -2503,6 +2504,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 a96d97a..aa77536 100644
> --- a/arch/mips/Makefile
> +++ b/arch/mips/Makefile
> @@ -216,6 +216,8 @@ cflags-$(toolchain-msa)			+= -DTOOLCHAIN_SUPPORTS_MSA
>  endif
>  toolchain-virt				:= $(call cc-option-yn,$(mips-cflags) -mvirt)
>  cflags-$(toolchain-virt)		+= -DTOOLCHAIN_SUPPORTS_VIRT
> +toolchain-crc				:= $(call cc-option-yn,$(mips-cflags) -Wa$(comma)-mcrc)
> +cflags-$(toolchain-crc)			+= -DTOOLCHAIN_SUPPORTS_CRC
>  
>  #
>  # Firmware support
> @@ -324,6 +326,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 0000000..665c725
> --- /dev/null
> +++ b/arch/mips/crypto/Makefile
> @@ -0,0 +1,5 @@
> +#
> +# 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 0000000..dfa8bb1
> --- /dev/null
> +++ b/arch/mips/crypto/crc32-mips.c
> @@ -0,0 +1,361 @@
> +/*
> + * crc32-mips.c - CRC32 and CRC32C using optional MIPSr6 instructions
> + *
> + * Module based on arm64/crypto/crc32-arm.c
> + *
> + * Copyright (C) 2014 Linaro Ltd <yazen.ghannam@linaro.org>
> + * Copyright (C) 2017 Imagination Technologies, Ltd.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/unaligned/access_ok.h>
> +#include <linux/cpufeature.h>
> +#include <linux/init.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/string.h>
> +
> +#include <crypto/internal/hash.h>
> +
> +enum crc_op_size {
> +	b, h, w, d,
> +};
> +
> +enum crc_type {
> +	crc32,
> +	crc32c,
> +};
> +
> +#ifdef TOOLCHAIN_SUPPORTS_CRC
> +
> +#define _CRC32(crc, value, size, type)		\
> +do {						\
> +	__asm__ __volatile__(			\
> +	".set	push\n\t"			\
> +	".set	crc\n\t"			\
> +	#type #size "	%0, %1, %0\n\t"		\
> +	".set	pop\n\t"			\

Technically the \n\t on the last line is redundant.

> +	: "+r" (crc)				\
> +	: "r" (value)				\
> +);						\
> +} while(0)
> +
> +#define CRC_REGISTER
> +
> +#else	/* TOOLCHAIN_SUPPORTS_CRC */
> +/*
> + * Crc argument is currently ignored and the assembly below assumes
> + * the crc is stored in $2. As the register number is encoded in the
> + * instruction we can't let the compiler chose the register it wants.
> + * An alternative is to change the code to do
> + * move $2, %0
> + * crc32
> + * move %0, $2
> + * but that adds unnecessary operations that the crc32 operation is
> + * designed to avoid. This issue can go away once the assembler
> + * is extended to support this operation and the compiler can make
> + * the right register choice automatically
> + */
> +
> +#define _CRC32(crc, value, size, type)						\
> +do {										\
> +	__asm__ __volatile__(							\
> +	".set	push\n\t"							\
> +	".set	noat\n\t"							\
> +	"move	$at, %1\n\t"							\
> +	"# " #type #size "	%0, $at, %0\n\t"				\
> +	_ASM_INSN_IF_MIPS(0x7c00000f | (2 << 16) | (1 << 21) | (%2 << 6) | (%3 << 8))	\
> +	_ASM_INSN32_IF_MM(0x00000030 | (1 << 16) | (2 << 21) | (%2 << 14) | (%3 << 3))	\

You should explicitly include <asm/mipsregs.h> for these macros.

> +	".set	pop\n\t"							\
> +	: "+r" (crc)								\
> +	: "r" (value), "i" (size), "i" (type)					\
> +);										\
> +} while(0)
> +
> +#define CRC_REGISTER __asm__("$2")
> +#endif	/* !TOOLCHAIN_SUPPORTS_CRC */
> +
> +#define CRC32(crc, value, size) \
> +	_CRC32(crc, value, size, crc32)
> +
> +#define CRC32C(crc, value, size) \
> +	_CRC32(crc, value, size, crc32c)
> +
> +static u32 crc32_mips_le_hw(u32 crc_, const u8 *p, unsigned int len)
> +{
> +	s64 length = len;

The need for 64-bit signed length is unfortunate. Do you get decent
assembly and comparable/better performance on 32-bit if you just use len
and only decrement it in the loops? i.e.

-	while ((length -= sizeof(uXX)) >= 0) {
+	while (len >= sizeof(uXX)) {
		register uXX value = get_unaligned_leXX(p);

		CRC32(crc, value, XX);
		p += sizeof(uXX);
+		len -= sizeof(uXX);
	}

That would be more readable too IMHO.

Just a thought.

Cheers
James


> +	register u32 crc CRC_REGISTER = crc_;
> +
> +#ifdef CONFIG_64BIT
> +	while ((length -= sizeof(u64)) >= 0) {
> +		register u64 value = get_unaligned_le64(p);
> +
> +		CRC32(crc, value, d);
> +		p += sizeof(u64);
> +	}
> +
> +	if (length & sizeof(u32)) {
> +#else /* !CONFIG_64BIT */
> +	while ((length -= sizeof(u32)) >= 0) {
> +#endif
> +		register u32 value = get_unaligned_le32(p);
> +
> +		CRC32(crc, value, w);
> +		p += sizeof(u32);
> +	}
> +
> +	if (length & sizeof(u16)) {
> +		register u16 value = get_unaligned_le16(p);
> +
> +		CRC32(crc, value, h);
> +		p += sizeof(u16);
> +	}
> +
> +	if (length & sizeof(u8)) {
> +		register u8 value = *p++;
> +
> +		CRC32(crc, value, b);
> +	}
> +
> +	return crc;
> +}
> +
> +static u32 crc32c_mips_le_hw(u32 crc_, const u8 *p, unsigned int len)
> +{
> +	s64 length = len;
> +	register u32 crc __asm__("$2") = crc_;
> +
> +#ifdef CONFIG_64BIT
> +	while ((length -= sizeof(u64)) >= 0) {
> +		register u64 value = get_unaligned_le64(p);
> +
> +		CRC32C(crc, value, d);
> +		p += sizeof(u64);
> +	}
> +
> +	if (length & sizeof(u32)) {
> +#else /* !CONFIG_64BIT */
> +	while ((length -= sizeof(u32)) >= 0) {
> +#endif
> +		register u32 value = get_unaligned_le32(p);
> +
> +		CRC32C(crc, value, w);
> +		p += sizeof(u32);
> +	}
> +
> +	if (length & sizeof(u16)) {
> +		register u16 value = get_unaligned_le16(p);
> +
> +		CRC32C(crc, value, h);
> +		p += sizeof(u16);
> +	}
> +
> +	if (length & sizeof(u8)) {
> +		register u8 value = *p++;
> +
> +		CRC32C(crc, value, b);
> +	}
> +	return crc;
> +}
> +
> +#define CHKSUM_BLOCK_SIZE	1
> +#define CHKSUM_DIGEST_SIZE	4
> +
> +struct chksum_ctx {
> +	u32 key;
> +};
> +
> +struct chksum_desc_ctx {
> +	u32 crc;
> +};
> +
> +static int chksum_init(struct shash_desc *desc)
> +{
> +	struct chksum_ctx *mctx = crypto_shash_ctx(desc->tfm);
> +	struct chksum_desc_ctx *ctx = shash_desc_ctx(desc);
> +
> +	ctx->crc = mctx->key;
> +
> +	return 0;
> +}
> +
> +/*
> + * Setting the seed allows arbitrary accumulators and flexible XOR policy
> + * If your algorithm starts with ~0, then XOR with ~0 before you set
> + * the seed.
> + */
> +static int chksum_setkey(struct crypto_shash *tfm, const u8 *key,
> +			 unsigned int keylen)
> +{
> +	struct chksum_ctx *mctx = crypto_shash_ctx(tfm);
> +
> +	if (keylen != sizeof(mctx->key)) {
> +		crypto_shash_set_flags(tfm, CRYPTO_TFM_RES_BAD_KEY_LEN);
> +		return -EINVAL;
> +	}
> +	mctx->key = get_unaligned_le32(key);
> +	return 0;
> +}
> +
> +static int chksum_update(struct shash_desc *desc, const u8 *data,
> +			 unsigned int length)
> +{
> +	struct chksum_desc_ctx *ctx = shash_desc_ctx(desc);
> +
> +	ctx->crc = crc32_mips_le_hw(ctx->crc, data, length);
> +	return 0;
> +}
> +
> +static int chksumc_update(struct shash_desc *desc, const u8 *data,
> +			 unsigned int length)
> +{
> +	struct chksum_desc_ctx *ctx = shash_desc_ctx(desc);
> +
> +	ctx->crc = crc32c_mips_le_hw(ctx->crc, data, length);
> +	return 0;
> +}
> +
> +static int chksum_final(struct shash_desc *desc, u8 *out)
> +{
> +	struct chksum_desc_ctx *ctx = shash_desc_ctx(desc);
> +
> +	put_unaligned_le32(ctx->crc, out);
> +	return 0;
> +}
> +
> +static int chksumc_final(struct shash_desc *desc, u8 *out)
> +{
> +	struct chksum_desc_ctx *ctx = shash_desc_ctx(desc);
> +
> +	put_unaligned_le32(~ctx->crc, out);
> +	return 0;
> +}
> +
> +static int __chksum_finup(u32 crc, const u8 *data, unsigned int len, u8 *out)
> +{
> +	put_unaligned_le32(crc32_mips_le_hw(crc, data, len), out);
> +	return 0;
> +}
> +
> +static int __chksumc_finup(u32 crc, const u8 *data, unsigned int len, u8 *out)
> +{
> +	put_unaligned_le32(~crc32c_mips_le_hw(crc, data, len), out);
> +	return 0;
> +}
> +
> +static int chksum_finup(struct shash_desc *desc, const u8 *data,
> +			unsigned int len, u8 *out)
> +{
> +	struct chksum_desc_ctx *ctx = shash_desc_ctx(desc);
> +
> +	return __chksum_finup(ctx->crc, data, len, out);
> +}
> +
> +static int chksumc_finup(struct shash_desc *desc, const u8 *data,
> +			unsigned int len, u8 *out)
> +{
> +	struct chksum_desc_ctx *ctx = shash_desc_ctx(desc);
> +
> +	return __chksumc_finup(ctx->crc, data, len, out);
> +}
> +
> +static int chksum_digest(struct shash_desc *desc, const u8 *data,
> +			 unsigned int length, u8 *out)
> +{
> +	struct chksum_ctx *mctx = crypto_shash_ctx(desc->tfm);
> +
> +	return __chksum_finup(mctx->key, data, length, out);
> +}
> +
> +static int chksumc_digest(struct shash_desc *desc, const u8 *data,
> +			 unsigned int length, u8 *out)
> +{
> +	struct chksum_ctx *mctx = crypto_shash_ctx(desc->tfm);
> +
> +	return __chksumc_finup(mctx->key, data, length, out);
> +}
> +
> +static int chksum_cra_init(struct crypto_tfm *tfm)
> +{
> +	struct chksum_ctx *mctx = crypto_tfm_ctx(tfm);
> +
> +	mctx->key = ~0;
> +	return 0;
> +}
> +
> +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,
> +	}
> +};
> +
> +static int __init crc32_mod_init(void)
> +{
> +	int err;
> +
> +	err = crypto_register_shash(&crc32_alg);
> +
> +	if (err)
> +		return err;
> +
> +	err = crypto_register_shash(&crc32c_alg);
> +
> +	if (err) {
> +		crypto_unregister_shash(&crc32_alg);
> +		return err;
> +	}
> +
> +	return 0;
> +}
> +
> +static void __exit crc32_mod_exit(void)
> +{
> +	crypto_unregister_shash(&crc32_alg);
> +	crypto_unregister_shash(&crc32c_alg);
> +}
> +
> +MODULE_AUTHOR("Marcin Nowakowski <marcin.nowakowski@imgtec.com");
> +MODULE_DESCRIPTION("CRC32 and CRC32C using optional MIPS instructions");
> +MODULE_LICENSE("GPL v2");
> +
> +module_cpu_feature_match(MIPS_CRC32, crc32_mod_init);
> +module_exit(crc32_mod_exit);
> diff --git a/crypto/Kconfig b/crypto/Kconfig
> index 28b1a0d..661971a 100644
> --- a/crypto/Kconfig
> +++ b/crypto/Kconfig
> @@ -494,6 +494,15 @@ config CRYPTO_CRC32_PCLMUL
>  	  which will enable any routine to use the CRC-32-IEEE 802.3 checksum
>  	  and gain better performance as compared with the table implementation.
>  
> +config CRYPTO_CRC32_MIPS
> +	tristate "CRC32c and CRC32 CRC algorithm (MIPS)"
> +	depends on MIPS_CRC_SUPPORT
> +	select CRYPTO_HASH
> +	help
> +	  CRC32c and CRC32 CRC algorithms implemented using mips crypto
> +	  instructions, when available.
> +
> +
>  config CRYPTO_CRCT10DIF
>  	tristate "CRCT10DIF algorithm"
>  	select CRYPTO_HASH
> -- 
> 2.7.4
> 
> 

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

* Re: [PATCH 1/2] MIPS: add crc instruction support flag to elf_hwcap
  2017-09-27 12:18 ` [PATCH 1/2] MIPS: add crc instruction support flag to elf_hwcap Marcin Nowakowski
@ 2017-09-29 21:35   ` James Hogan
  0 siblings, 0 replies; 12+ messages in thread
From: James Hogan @ 2017-09-29 21:35 UTC (permalink / raw)
  To: Marcin Nowakowski; +Cc: Linux MIPS Mailing List, Ralf Baechle

[-- Attachment #1: Type: text/plain, Size: 2125 bytes --]

On Wed, Sep 27, 2017 at 02:18:35PM +0200, Marcin Nowakowski wrote:
> Indicate that CRC32 and CRC32C instuctions are supported by the CPU
> through elf_hwcap flags.
> 
> This will be used by a follow-up commit that introduces crc32(c) crypto
> acceleration modules and is required by GENERIC_CPU_AUTOPROBE feature.
> 
> Signed-off-by: Marcin Nowakowski <marcin.nowakowski@imgtec.com>

Looks good to me,
Reviewed-by: James Hogan <james.hogan@imgtec.com>

Cheers
James

> ---
>  arch/mips/include/asm/mipsregs.h   | 1 +
>  arch/mips/include/uapi/asm/hwcap.h | 1 +
>  arch/mips/kernel/cpu-probe.c       | 3 +++
>  3 files changed, 5 insertions(+)
> 
> diff --git a/arch/mips/include/asm/mipsregs.h b/arch/mips/include/asm/mipsregs.h
> index a681092..9db53cc 100644
> --- a/arch/mips/include/asm/mipsregs.h
> +++ b/arch/mips/include/asm/mipsregs.h
> @@ -664,6 +664,7 @@
>  #define MIPS_CONF5_FRE		(_ULCAST_(1) << 8)
>  #define MIPS_CONF5_UFE		(_ULCAST_(1) << 9)
>  #define MIPS_CONF5_CA2		(_ULCAST_(1) << 14)
> +#define MIPS_CONF5_CRCP		(_ULCAST_(1) << 18)
>  #define MIPS_CONF5_MSAEN	(_ULCAST_(1) << 27)
>  #define MIPS_CONF5_EVA		(_ULCAST_(1) << 28)
>  #define MIPS_CONF5_CV		(_ULCAST_(1) << 29)
> diff --git a/arch/mips/include/uapi/asm/hwcap.h b/arch/mips/include/uapi/asm/hwcap.h
> index c7484a7..c7d2cb6 100644
> --- a/arch/mips/include/uapi/asm/hwcap.h
> +++ b/arch/mips/include/uapi/asm/hwcap.h
> @@ -4,5 +4,6 @@
>  /* HWCAP flags */
>  #define HWCAP_MIPS_R6		(1 << 0)
>  #define HWCAP_MIPS_MSA		(1 << 1)
> +#define HWCAP_MIPS_CRC32	(1 << 2)
>  
>  #endif /* _UAPI_ASM_HWCAP_H */
> diff --git a/arch/mips/kernel/cpu-probe.c b/arch/mips/kernel/cpu-probe.c
> index cf3fd54..6b07b73 100644
> --- a/arch/mips/kernel/cpu-probe.c
> +++ b/arch/mips/kernel/cpu-probe.c
> @@ -848,6 +848,9 @@ static inline unsigned int decode_config5(struct cpuinfo_mips *c)
>  	if (config5 & MIPS_CONF5_CA2)
>  		c->ases |= MIPS_ASE_MIPS16E2;
>  
> +	if (config5 & MIPS_CONF5_CRCP)
> +		elf_hwcap |= HWCAP_MIPS_CRC32;
> +
>  	return config5 & MIPS_CONF_M;
>  }
>  
> -- 
> 2.7.4
> 
> 

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

* Re: [PATCH 2/2] MIPS: crypto: Add crc32 and crc32c hw accelerated module
  2017-09-29 21:34     ` James Hogan
  (?)
@ 2017-10-02 14:20     ` Jonas Gorski
  2017-10-03  6:38         ` Marcin Nowakowski
  -1 siblings, 1 reply; 12+ messages in thread
From: Jonas Gorski @ 2017-10-02 14:20 UTC (permalink / raw)
  To: James Hogan
  Cc: Marcin Nowakowski, Linux MIPS Mailing List, Ralf Baechle,
	linux-crypto, Herbert Xu, David S. Miller

On 29 September 2017 at 23:34, James Hogan <james.hogan@imgtec.com> wrote:
> Hi Marcin,
>
> On Wed, Sep 27, 2017 at 02:18:36PM +0200, Marcin Nowakowski wrote:
>> 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 <marcin.nowakowski@imgtec.com>
>> Cc: linux-crypto@vger.kernel.org
>> Cc: Herbert Xu <herbert@gondor.apana.org.au>
>> Cc: "David S. Miller" <davem@davemloft.net>
>>
>> ---
>>  arch/mips/Kconfig             |   4 +
>>  arch/mips/Makefile            |   3 +
>>  arch/mips/crypto/Makefile     |   5 +
>>  arch/mips/crypto/crc32-mips.c | 361 ++++++++++++++++++++++++++++++++++++++++++
>>  crypto/Kconfig                |   9 ++
>>  5 files changed, 382 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 cb7fcc4..0f96812 100644
>> --- a/arch/mips/Kconfig
>> +++ b/arch/mips/Kconfig
>> @@ -2036,6 +2036,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
>> @@ -2503,6 +2504,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 a96d97a..aa77536 100644
>> --- a/arch/mips/Makefile
>> +++ b/arch/mips/Makefile
>> @@ -216,6 +216,8 @@ cflags-$(toolchain-msa)                   += -DTOOLCHAIN_SUPPORTS_MSA
>>  endif
>>  toolchain-virt                               := $(call cc-option-yn,$(mips-cflags) -mvirt)
>>  cflags-$(toolchain-virt)             += -DTOOLCHAIN_SUPPORTS_VIRT
>> +toolchain-crc                                := $(call cc-option-yn,$(mips-cflags) -Wa$(comma)-mcrc)
>> +cflags-$(toolchain-crc)                      += -DTOOLCHAIN_SUPPORTS_CRC
>>
>>  #
>>  # Firmware support
>> @@ -324,6 +326,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 0000000..665c725
>> --- /dev/null
>> +++ b/arch/mips/crypto/Makefile
>> @@ -0,0 +1,5 @@
>> +#
>> +# 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 0000000..dfa8bb1
>> --- /dev/null
>> +++ b/arch/mips/crypto/crc32-mips.c
>> @@ -0,0 +1,361 @@
>> +/*
>> + * crc32-mips.c - CRC32 and CRC32C using optional MIPSr6 instructions
>> + *
>> + * Module based on arm64/crypto/crc32-arm.c
>> + *
>> + * Copyright (C) 2014 Linaro Ltd <yazen.ghannam@linaro.org>
>> + * Copyright (C) 2017 Imagination Technologies, Ltd.
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
>> + */
>> +
>> +#include <linux/unaligned/access_ok.h>
>> +#include <linux/cpufeature.h>
>> +#include <linux/init.h>
>> +#include <linux/kernel.h>
>> +#include <linux/module.h>
>> +#include <linux/string.h>
>> +
>> +#include <crypto/internal/hash.h>
>> +
>> +enum crc_op_size {
>> +     b, h, w, d,
>> +};
>> +
>> +enum crc_type {
>> +     crc32,
>> +     crc32c,
>> +};
>> +
>> +#ifdef TOOLCHAIN_SUPPORTS_CRC
>> +
>> +#define _CRC32(crc, value, size, type)               \
>> +do {                                         \
>> +     __asm__ __volatile__(                   \
>> +     ".set   push\n\t"                       \
>> +     ".set   crc\n\t"                        \
>> +     #type #size "   %0, %1, %0\n\t"         \
>> +     ".set   pop\n\t"                        \
>
> Technically the \n\t on the last line is redundant.
>
>> +     : "+r" (crc)                            \
>> +     : "r" (value)                           \
>> +);                                           \
>> +} while(0)
>> +
>> +#define CRC_REGISTER
>> +
>> +#else        /* TOOLCHAIN_SUPPORTS_CRC */
>> +/*
>> + * Crc argument is currently ignored and the assembly below assumes
>> + * the crc is stored in $2. As the register number is encoded in the
>> + * instruction we can't let the compiler chose the register it wants.
>> + * An alternative is to change the code to do
>> + * move $2, %0
>> + * crc32
>> + * move %0, $2
>> + * but that adds unnecessary operations that the crc32 operation is
>> + * designed to avoid. This issue can go away once the assembler
>> + * is extended to support this operation and the compiler can make
>> + * the right register choice automatically
>> + */
>> +
>> +#define _CRC32(crc, value, size, type)                                               \
>> +do {                                                                         \
>> +     __asm__ __volatile__(                                                   \
>> +     ".set   push\n\t"                                                       \
>> +     ".set   noat\n\t"                                                       \
>> +     "move   $at, %1\n\t"                                                    \
>> +     "# " #type #size "      %0, $at, %0\n\t"                                \
>> +     _ASM_INSN_IF_MIPS(0x7c00000f | (2 << 16) | (1 << 21) | (%2 << 6) | (%3 << 8))   \
>> +     _ASM_INSN32_IF_MM(0x00000030 | (1 << 16) | (2 << 21) | (%2 << 14) | (%3 << 3))  \
>
> You should explicitly include <asm/mipsregs.h> for these macros.
>
>> +     ".set   pop\n\t"                                                        \
>> +     : "+r" (crc)                                                            \
>> +     : "r" (value), "i" (size), "i" (type)                                   \
>> +);                                                                           \
>> +} while(0)
>> +
>> +#define CRC_REGISTER __asm__("$2")
>> +#endif       /* !TOOLCHAIN_SUPPORTS_CRC */
>> +
>> +#define CRC32(crc, value, size) \
>> +     _CRC32(crc, value, size, crc32)
>> +
>> +#define CRC32C(crc, value, size) \
>> +     _CRC32(crc, value, size, crc32c)
>> +
>> +static u32 crc32_mips_le_hw(u32 crc_, const u8 *p, unsigned int len)
>> +{
>> +     s64 length = len;
>
> The need for 64-bit signed length is unfortunate. Do you get decent
> assembly and comparable/better performance on 32-bit if you just use len
> and only decrement it in the loops? i.e.
>
> -       while ((length -= sizeof(uXX)) >= 0) {
> +       while (len >= sizeof(uXX)) {
>                 register uXX value = get_unaligned_leXX(p);
>
>                 CRC32(crc, value, XX);
>                 p += sizeof(uXX);
> +               len -= sizeof(uXX);
>         }
>
> That would be more readable too IMHO.

or maybe just do some pointer arithmetic like

  const u8 *end = p + len;

  while ((end - p) >= sizeof(uXX)) {
           register uXX value = get_unaligned_leXX(p);

           CRC32(crc, value, XX);
           p += sizeof(uXX);
  }


Regards
Jonas

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

* Re: [PATCH 2/2] MIPS: crypto: Add crc32 and crc32c hw accelerated module
@ 2017-10-03  6:38         ` Marcin Nowakowski
  0 siblings, 0 replies; 12+ messages in thread
From: Marcin Nowakowski @ 2017-10-03  6:38 UTC (permalink / raw)
  To: Jonas Gorski, James Hogan
  Cc: Linux MIPS Mailing List, Ralf Baechle, linux-crypto, Herbert Xu,
	David S. Miller

Hi Jonas, James,

On 02.10.2017 16:20, Jonas Gorski wrote:
> On 29 September 2017 at 23:34, James Hogan <james.hogan@imgtec.com> wrote:
>> Hi Marcin,
>>
>> On Wed, Sep 27, 2017 at 02:18:36PM +0200, Marcin Nowakowski wrote:
>>> 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 <marcin.nowakowski@imgtec.com>
>>> Cc: linux-crypto@vger.kernel.org
>>> Cc: Herbert Xu <herbert@gondor.apana.org.au>
>>> Cc: "David S. Miller" <davem@davemloft.net>
>>>
>>> ---
>>>   arch/mips/Kconfig             |   4 +
>>>   arch/mips/Makefile            |   3 +
>>>   arch/mips/crypto/Makefile     |   5 +
>>>   arch/mips/crypto/crc32-mips.c | 361 ++++++++++++++++++++++++++++++++++++++++++
>>>   crypto/Kconfig                |   9 ++
>>>   5 files changed, 382 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 cb7fcc4..0f96812 100644
>>> --- a/arch/mips/Kconfig
>>> +++ b/arch/mips/Kconfig
>>> @@ -2036,6 +2036,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
>>> @@ -2503,6 +2504,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 a96d97a..aa77536 100644
>>> --- a/arch/mips/Makefile
>>> +++ b/arch/mips/Makefile
>>> @@ -216,6 +216,8 @@ cflags-$(toolchain-msa)                   += -DTOOLCHAIN_SUPPORTS_MSA
>>>   endif
>>>   toolchain-virt                               := $(call cc-option-yn,$(mips-cflags) -mvirt)
>>>   cflags-$(toolchain-virt)             += -DTOOLCHAIN_SUPPORTS_VIRT
>>> +toolchain-crc                                := $(call cc-option-yn,$(mips-cflags) -Wa$(comma)-mcrc)
>>> +cflags-$(toolchain-crc)                      += -DTOOLCHAIN_SUPPORTS_CRC
>>>
>>>   #
>>>   # Firmware support
>>> @@ -324,6 +326,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 0000000..665c725
>>> --- /dev/null
>>> +++ b/arch/mips/crypto/Makefile
>>> @@ -0,0 +1,5 @@
>>> +#
>>> +# 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 0000000..dfa8bb1
>>> --- /dev/null
>>> +++ b/arch/mips/crypto/crc32-mips.c
>>> @@ -0,0 +1,361 @@
>>> +/*
>>> + * crc32-mips.c - CRC32 and CRC32C using optional MIPSr6 instructions
>>> + *
>>> + * Module based on arm64/crypto/crc32-arm.c
>>> + *
>>> + * Copyright (C) 2014 Linaro Ltd <yazen.ghannam@linaro.org>
>>> + * Copyright (C) 2017 Imagination Technologies, Ltd.
>>> + *
>>> + * This program is free software; you can redistribute it and/or modify
>>> + * it under the terms of the GNU General Public License version 2 as
>>> + * published by the Free Software Foundation.
>>> + */
>>> +
>>> +#include <linux/unaligned/access_ok.h>
>>> +#include <linux/cpufeature.h>
>>> +#include <linux/init.h>
>>> +#include <linux/kernel.h>
>>> +#include <linux/module.h>
>>> +#include <linux/string.h>
>>> +
>>> +#include <crypto/internal/hash.h>
>>> +
>>> +enum crc_op_size {
>>> +     b, h, w, d,
>>> +};
>>> +
>>> +enum crc_type {
>>> +     crc32,
>>> +     crc32c,
>>> +};
>>> +
>>> +#ifdef TOOLCHAIN_SUPPORTS_CRC
>>> +
>>> +#define _CRC32(crc, value, size, type)               \
>>> +do {                                         \
>>> +     __asm__ __volatile__(                   \
>>> +     ".set   push\n\t"                       \
>>> +     ".set   crc\n\t"                        \
>>> +     #type #size "   %0, %1, %0\n\t"         \
>>> +     ".set   pop\n\t"                        \
>>
>> Technically the \n\t on the last line is redundant.
>>
>>> +     : "+r" (crc)                            \
>>> +     : "r" (value)                           \
>>> +);                                           \
>>> +} while(0)
>>> +
>>> +#define CRC_REGISTER
>>> +
>>> +#else        /* TOOLCHAIN_SUPPORTS_CRC */
>>> +/*
>>> + * Crc argument is currently ignored and the assembly below assumes
>>> + * the crc is stored in $2. As the register number is encoded in the
>>> + * instruction we can't let the compiler chose the register it wants.
>>> + * An alternative is to change the code to do
>>> + * move $2, %0
>>> + * crc32
>>> + * move %0, $2
>>> + * but that adds unnecessary operations that the crc32 operation is
>>> + * designed to avoid. This issue can go away once the assembler
>>> + * is extended to support this operation and the compiler can make
>>> + * the right register choice automatically
>>> + */
>>> +
>>> +#define _CRC32(crc, value, size, type)                                               \
>>> +do {                                                                         \
>>> +     __asm__ __volatile__(                                                   \
>>> +     ".set   push\n\t"                                                       \
>>> +     ".set   noat\n\t"                                                       \
>>> +     "move   $at, %1\n\t"                                                    \
>>> +     "# " #type #size "      %0, $at, %0\n\t"                                \
>>> +     _ASM_INSN_IF_MIPS(0x7c00000f | (2 << 16) | (1 << 21) | (%2 << 6) | (%3 << 8))   \
>>> +     _ASM_INSN32_IF_MM(0x00000030 | (1 << 16) | (2 << 21) | (%2 << 14) | (%3 << 3))  \
>>
>> You should explicitly include <asm/mipsregs.h> for these macros.
>>
>>> +     ".set   pop\n\t"                                                        \
>>> +     : "+r" (crc)                                                            \
>>> +     : "r" (value), "i" (size), "i" (type)                                   \
>>> +);                                                                           \
>>> +} while(0)
>>> +
>>> +#define CRC_REGISTER __asm__("$2")
>>> +#endif       /* !TOOLCHAIN_SUPPORTS_CRC */
>>> +
>>> +#define CRC32(crc, value, size) \
>>> +     _CRC32(crc, value, size, crc32)
>>> +
>>> +#define CRC32C(crc, value, size) \
>>> +     _CRC32(crc, value, size, crc32c)
>>> +
>>> +static u32 crc32_mips_le_hw(u32 crc_, const u8 *p, unsigned int len)
>>> +{
>>> +     s64 length = len;
>>
>> The need for 64-bit signed length is unfortunate. Do you get decent
>> assembly and comparable/better performance on 32-bit if you just use len
>> and only decrement it in the loops? i.e.
>>
>> -       while ((length -= sizeof(uXX)) >= 0) {
>> +       while (len >= sizeof(uXX)) {
>>                  register uXX value = get_unaligned_leXX(p);
>>
>>                  CRC32(crc, value, XX);
>>                  p += sizeof(uXX);
>> +               len -= sizeof(uXX);
>>          }
>>
>> That would be more readable too IMHO.
> 
> or maybe just do some pointer arithmetic like
> 
>    const u8 *end = p + len;
> 
>    while ((end - p) >= sizeof(uXX)) {
>             register uXX value = get_unaligned_leXX(p);
> 
>             CRC32(crc, value, XX);
>             p += sizeof(uXX);
>    }

Thank you both for these suggestions. All solutions are very similar in 
terms of the assembly produced, although the original code is the 
smallest of all:

original vs James':
crc32_mips_le_hw                             104     132     +28
vermagic                                      72      78      +6
chksumc_finup                                 40      44      +4
chksumc_digest                                44      48      +4
chksum_finup                                  92      96      +4
chksum_digest                                100     104      +4

original vs Jonas':
add/remove: 0/0 grow/shrink: 7/0 up/down: 90/0 (90)
function                                     old     new   delta
crc32_mips_le_hw                             104     148     +44
vermagic                                      72      78      +6
chksumc_finup                                 40      44      +4
chksumc_digest                                44      48      +4
chksum_finup                                  92      96      +4
chksum_digest                                100     104      +4


However - the key thing which is the processing loop is 6 instructions 
long in all variants. It's only the pre/post loop processing that adds 
the extra instructions so all these solutions should be roughly equal in 
terms of performance.
I find James' code a bit more readable so I'll go with it and post an 
updated patch.


Thanks
Marcin

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

* Re: [PATCH 2/2] MIPS: crypto: Add crc32 and crc32c hw accelerated module
@ 2017-10-03  6:38         ` Marcin Nowakowski
  0 siblings, 0 replies; 12+ messages in thread
From: Marcin Nowakowski @ 2017-10-03  6:38 UTC (permalink / raw)
  To: Jonas Gorski, James Hogan
  Cc: Linux MIPS Mailing List, Ralf Baechle, linux-crypto, Herbert Xu,
	David S. Miller

Hi Jonas, James,

On 02.10.2017 16:20, Jonas Gorski wrote:
> On 29 September 2017 at 23:34, James Hogan <james.hogan@imgtec.com> wrote:
>> Hi Marcin,
>>
>> On Wed, Sep 27, 2017 at 02:18:36PM +0200, Marcin Nowakowski wrote:
>>> 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 <marcin.nowakowski@imgtec.com>
>>> Cc: linux-crypto@vger.kernel.org
>>> Cc: Herbert Xu <herbert@gondor.apana.org.au>
>>> Cc: "David S. Miller" <davem@davemloft.net>
>>>
>>> ---
>>>   arch/mips/Kconfig             |   4 +
>>>   arch/mips/Makefile            |   3 +
>>>   arch/mips/crypto/Makefile     |   5 +
>>>   arch/mips/crypto/crc32-mips.c | 361 ++++++++++++++++++++++++++++++++++++++++++
>>>   crypto/Kconfig                |   9 ++
>>>   5 files changed, 382 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 cb7fcc4..0f96812 100644
>>> --- a/arch/mips/Kconfig
>>> +++ b/arch/mips/Kconfig
>>> @@ -2036,6 +2036,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
>>> @@ -2503,6 +2504,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 a96d97a..aa77536 100644
>>> --- a/arch/mips/Makefile
>>> +++ b/arch/mips/Makefile
>>> @@ -216,6 +216,8 @@ cflags-$(toolchain-msa)                   += -DTOOLCHAIN_SUPPORTS_MSA
>>>   endif
>>>   toolchain-virt                               := $(call cc-option-yn,$(mips-cflags) -mvirt)
>>>   cflags-$(toolchain-virt)             += -DTOOLCHAIN_SUPPORTS_VIRT
>>> +toolchain-crc                                := $(call cc-option-yn,$(mips-cflags) -Wa$(comma)-mcrc)
>>> +cflags-$(toolchain-crc)                      += -DTOOLCHAIN_SUPPORTS_CRC
>>>
>>>   #
>>>   # Firmware support
>>> @@ -324,6 +326,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 0000000..665c725
>>> --- /dev/null
>>> +++ b/arch/mips/crypto/Makefile
>>> @@ -0,0 +1,5 @@
>>> +#
>>> +# 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 0000000..dfa8bb1
>>> --- /dev/null
>>> +++ b/arch/mips/crypto/crc32-mips.c
>>> @@ -0,0 +1,361 @@
>>> +/*
>>> + * crc32-mips.c - CRC32 and CRC32C using optional MIPSr6 instructions
>>> + *
>>> + * Module based on arm64/crypto/crc32-arm.c
>>> + *
>>> + * Copyright (C) 2014 Linaro Ltd <yazen.ghannam@linaro.org>
>>> + * Copyright (C) 2017 Imagination Technologies, Ltd.
>>> + *
>>> + * This program is free software; you can redistribute it and/or modify
>>> + * it under the terms of the GNU General Public License version 2 as
>>> + * published by the Free Software Foundation.
>>> + */
>>> +
>>> +#include <linux/unaligned/access_ok.h>
>>> +#include <linux/cpufeature.h>
>>> +#include <linux/init.h>
>>> +#include <linux/kernel.h>
>>> +#include <linux/module.h>
>>> +#include <linux/string.h>
>>> +
>>> +#include <crypto/internal/hash.h>
>>> +
>>> +enum crc_op_size {
>>> +     b, h, w, d,
>>> +};
>>> +
>>> +enum crc_type {
>>> +     crc32,
>>> +     crc32c,
>>> +};
>>> +
>>> +#ifdef TOOLCHAIN_SUPPORTS_CRC
>>> +
>>> +#define _CRC32(crc, value, size, type)               \
>>> +do {                                         \
>>> +     __asm__ __volatile__(                   \
>>> +     ".set   push\n\t"                       \
>>> +     ".set   crc\n\t"                        \
>>> +     #type #size "   %0, %1, %0\n\t"         \
>>> +     ".set   pop\n\t"                        \
>>
>> Technically the \n\t on the last line is redundant.
>>
>>> +     : "+r" (crc)                            \
>>> +     : "r" (value)                           \
>>> +);                                           \
>>> +} while(0)
>>> +
>>> +#define CRC_REGISTER
>>> +
>>> +#else        /* TOOLCHAIN_SUPPORTS_CRC */
>>> +/*
>>> + * Crc argument is currently ignored and the assembly below assumes
>>> + * the crc is stored in $2. As the register number is encoded in the
>>> + * instruction we can't let the compiler chose the register it wants.
>>> + * An alternative is to change the code to do
>>> + * move $2, %0
>>> + * crc32
>>> + * move %0, $2
>>> + * but that adds unnecessary operations that the crc32 operation is
>>> + * designed to avoid. This issue can go away once the assembler
>>> + * is extended to support this operation and the compiler can make
>>> + * the right register choice automatically
>>> + */
>>> +
>>> +#define _CRC32(crc, value, size, type)                                               \
>>> +do {                                                                         \
>>> +     __asm__ __volatile__(                                                   \
>>> +     ".set   push\n\t"                                                       \
>>> +     ".set   noat\n\t"                                                       \
>>> +     "move   $at, %1\n\t"                                                    \
>>> +     "# " #type #size "      %0, $at, %0\n\t"                                \
>>> +     _ASM_INSN_IF_MIPS(0x7c00000f | (2 << 16) | (1 << 21) | (%2 << 6) | (%3 << 8))   \
>>> +     _ASM_INSN32_IF_MM(0x00000030 | (1 << 16) | (2 << 21) | (%2 << 14) | (%3 << 3))  \
>>
>> You should explicitly include <asm/mipsregs.h> for these macros.
>>
>>> +     ".set   pop\n\t"                                                        \
>>> +     : "+r" (crc)                                                            \
>>> +     : "r" (value), "i" (size), "i" (type)                                   \
>>> +);                                                                           \
>>> +} while(0)
>>> +
>>> +#define CRC_REGISTER __asm__("$2")
>>> +#endif       /* !TOOLCHAIN_SUPPORTS_CRC */
>>> +
>>> +#define CRC32(crc, value, size) \
>>> +     _CRC32(crc, value, size, crc32)
>>> +
>>> +#define CRC32C(crc, value, size) \
>>> +     _CRC32(crc, value, size, crc32c)
>>> +
>>> +static u32 crc32_mips_le_hw(u32 crc_, const u8 *p, unsigned int len)
>>> +{
>>> +     s64 length = len;
>>
>> The need for 64-bit signed length is unfortunate. Do you get decent
>> assembly and comparable/better performance on 32-bit if you just use len
>> and only decrement it in the loops? i.e.
>>
>> -       while ((length -= sizeof(uXX)) >= 0) {
>> +       while (len >= sizeof(uXX)) {
>>                  register uXX value = get_unaligned_leXX(p);
>>
>>                  CRC32(crc, value, XX);
>>                  p += sizeof(uXX);
>> +               len -= sizeof(uXX);
>>          }
>>
>> That would be more readable too IMHO.
> 
> or maybe just do some pointer arithmetic like
> 
>    const u8 *end = p + len;
> 
>    while ((end - p) >= sizeof(uXX)) {
>             register uXX value = get_unaligned_leXX(p);
> 
>             CRC32(crc, value, XX);
>             p += sizeof(uXX);
>    }

Thank you both for these suggestions. All solutions are very similar in 
terms of the assembly produced, although the original code is the 
smallest of all:

original vs James':
crc32_mips_le_hw                             104     132     +28
vermagic                                      72      78      +6
chksumc_finup                                 40      44      +4
chksumc_digest                                44      48      +4
chksum_finup                                  92      96      +4
chksum_digest                                100     104      +4

original vs Jonas':
add/remove: 0/0 grow/shrink: 7/0 up/down: 90/0 (90)
function                                     old     new   delta
crc32_mips_le_hw                             104     148     +44
vermagic                                      72      78      +6
chksumc_finup                                 40      44      +4
chksumc_digest                                44      48      +4
chksum_finup                                  92      96      +4
chksum_digest                                100     104      +4


However - the key thing which is the processing loop is 6 instructions 
long in all variants. It's only the pre/post loop processing that adds 
the extra instructions so all these solutions should be roughly equal in 
terms of performance.
I find James' code a bit more readable so I'll go with it and post an 
updated patch.


Thanks
Marcin

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

* Re: [PATCH 2/2] MIPS: crypto: Add crc32 and crc32c hw accelerated module
@ 2017-10-04  7:49           ` Marcin Nowakowski
  0 siblings, 0 replies; 12+ messages in thread
From: Marcin Nowakowski @ 2017-10-04  7:49 UTC (permalink / raw)
  To: Jonas Gorski, James Hogan
  Cc: Linux MIPS Mailing List, Ralf Baechle, linux-crypto, Herbert Xu,
	David S. Miller

Hi James,

On 03.10.2017 08:38, Marcin Nowakowski wrote:

>>>
>>> The need for 64-bit signed length is unfortunate. Do you get decent
>>> assembly and comparable/better performance on 32-bit if you just use len
>>> and only decrement it in the loops? i.e.
>>>
>>> -       while ((length -= sizeof(uXX)) >= 0) {
>>> +       while (len >= sizeof(uXX)) {
>>>                  register uXX value = get_unaligned_leXX(p);
>>>
>>>                  CRC32(crc, value, XX);
>>>                  p += sizeof(uXX);
>>> +               len -= sizeof(uXX);
>>>          }
>>>
>>> That would be more readable too IMHO.
>>
>> or maybe just do some pointer arithmetic like
>>
>>    const u8 *end = p + len;
>>
>>    while ((end - p) >= sizeof(uXX)) {
>>             register uXX value = get_unaligned_leXX(p);
>>
>>             CRC32(crc, value, XX);
>>             p += sizeof(uXX);
>>    }
> 
> Thank you both for these suggestions. All solutions are very similar in 
> terms of the assembly produced, although the original code is the 
> smallest of all:
> 
> original vs James':
> crc32_mips_le_hw                             104     132     +28
> vermagic                                      72      78      +6
> chksumc_finup                                 40      44      +4
> chksumc_digest                                44      48      +4
> chksum_finup                                  92      96      +4
> chksum_digest                                100     104      +4
> 
> original vs Jonas':
> add/remove: 0/0 grow/shrink: 7/0 up/down: 90/0 (90)
> function                                     old     new   delta
> crc32_mips_le_hw                             104     148     +44
> vermagic                                      72      78      +6
> chksumc_finup                                 40      44      +4
> chksumc_digest                                44      48      +4
> chksum_finup                                  92      96      +4
> chksum_digest                                100     104      +4
> 
> 
> However - the key thing which is the processing loop is 6 instructions 
> long in all variants. It's only the pre/post loop processing that adds 
> the extra instructions so all these solutions should be roughly equal in 
> terms of performance.
> I find James' code a bit more readable so I'll go with it and post an 
> updated patch.
> 

The comparisons above were for 64-bit, where the difference is 
negligible. On 32-bit builds, however, the difference is more significant:

original vs James':

function                                     old     new   delta
vermagic                                      80      86      +6
crc32c_mips_le_hw                            144     104     -40
crc32_mips_le_hw                             144     104     -40

and the main crc loop is down from 9 to 5 instructions, so it's a 
significant reduction of the loop size.

Marcin

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

* Re: [PATCH 2/2] MIPS: crypto: Add crc32 and crc32c hw accelerated module
@ 2017-10-04  7:49           ` Marcin Nowakowski
  0 siblings, 0 replies; 12+ messages in thread
From: Marcin Nowakowski @ 2017-10-04  7:49 UTC (permalink / raw)
  To: Jonas Gorski, James Hogan
  Cc: Linux MIPS Mailing List, Ralf Baechle, linux-crypto, Herbert Xu,
	David S. Miller

Hi James,

On 03.10.2017 08:38, Marcin Nowakowski wrote:

>>>
>>> The need for 64-bit signed length is unfortunate. Do you get decent
>>> assembly and comparable/better performance on 32-bit if you just use len
>>> and only decrement it in the loops? i.e.
>>>
>>> -       while ((length -= sizeof(uXX)) >= 0) {
>>> +       while (len >= sizeof(uXX)) {
>>>                  register uXX value = get_unaligned_leXX(p);
>>>
>>>                  CRC32(crc, value, XX);
>>>                  p += sizeof(uXX);
>>> +               len -= sizeof(uXX);
>>>          }
>>>
>>> That would be more readable too IMHO.
>>
>> or maybe just do some pointer arithmetic like
>>
>>    const u8 *end = p + len;
>>
>>    while ((end - p) >= sizeof(uXX)) {
>>             register uXX value = get_unaligned_leXX(p);
>>
>>             CRC32(crc, value, XX);
>>             p += sizeof(uXX);
>>    }
> 
> Thank you both for these suggestions. All solutions are very similar in 
> terms of the assembly produced, although the original code is the 
> smallest of all:
> 
> original vs James':
> crc32_mips_le_hw                             104     132     +28
> vermagic                                      72      78      +6
> chksumc_finup                                 40      44      +4
> chksumc_digest                                44      48      +4
> chksum_finup                                  92      96      +4
> chksum_digest                                100     104      +4
> 
> original vs Jonas':
> add/remove: 0/0 grow/shrink: 7/0 up/down: 90/0 (90)
> function                                     old     new   delta
> crc32_mips_le_hw                             104     148     +44
> vermagic                                      72      78      +6
> chksumc_finup                                 40      44      +4
> chksumc_digest                                44      48      +4
> chksum_finup                                  92      96      +4
> chksum_digest                                100     104      +4
> 
> 
> However - the key thing which is the processing loop is 6 instructions 
> long in all variants. It's only the pre/post loop processing that adds 
> the extra instructions so all these solutions should be roughly equal in 
> terms of performance.
> I find James' code a bit more readable so I'll go with it and post an 
> updated patch.
> 

The comparisons above were for 64-bit, where the difference is 
negligible. On 32-bit builds, however, the difference is more significant:

original vs James':

function                                     old     new   delta
vermagic                                      80      86      +6
crc32c_mips_le_hw                            144     104     -40
crc32_mips_le_hw                             144     104     -40

and the main crc loop is down from 9 to 5 instructions, so it's a 
significant reduction of the loop size.

Marcin

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

end of thread, other threads:[~2017-10-04  7:49 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-09-27 12:18 [PATCH 0/2] MIPS CRC instruction support Marcin Nowakowski
2017-09-27 12:18 ` [PATCH 1/2] MIPS: add crc instruction support flag to elf_hwcap Marcin Nowakowski
2017-09-29 21:35   ` James Hogan
2017-09-27 12:18 ` [PATCH 2/2] MIPS: crypto: Add crc32 and crc32c hw accelerated module Marcin Nowakowski
2017-09-27 12:18   ` Marcin Nowakowski
2017-09-29 21:34   ` James Hogan
2017-09-29 21:34     ` James Hogan
2017-10-02 14:20     ` Jonas Gorski
2017-10-03  6:38       ` Marcin Nowakowski
2017-10-03  6:38         ` Marcin Nowakowski
2017-10-04  7:49         ` Marcin Nowakowski
2017-10-04  7:49           ` Marcin Nowakowski

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.