2024-05-30 12:25:18

by Kamlesh Gurudasani

[permalink] [raw]
Subject: [PATCH v3 0/6] Add support for MCRC64 engine to calculate 64-bit CRC in Full-CPU mode

From: Kamlesh Gurudasani <[email protected]>

MCRC64 engine calculates 64-bit cyclic redundancy checks (CRC)
according to the ISO 3309 standard.

The ISO 3309 64-bit CRC model parameters are as follows:
Generator Polynomial: x^64 + x^4 + x^3 + x + 1
Polynomial Value: 0x000000000000001B
Initial value: 0x0000000000000000
Reflected Input: False
Reflected Output: False
Xor Final: 0x0000000000000000

ISO 3309 which came in 1991 define this 64-bit CRC, ISO 3309 which
came in 1993 doesn't define 64-bit CRC algorithm at all, so ISO-3309
naming is unique enough for this CRC64 generator polynomial.

Tested with
CONFIG_CRYPTO_MANAGER_DISABLE_TESTS is not set
CONFIG_CRYPTO_MANAGER_EXTRA_TESTS=y

and tcrypt,
sudo modprobe tcrypt mode=329 sec=1

User space application implemented using algif_hash [0].

I understand that this is a old algorithm but,
most of TI's K3 based J7* and AM62* SOCs have MCRC64
which we want to use from User Space.

As per suggestions, we did the calculations and it turns out to be we
are saving good number of cpu cycles with HW offload[1].

Also, there are some automotive customers who have a safety
requirement to offload any parameters that are in Linux to ensure FFI.

[0] https://gist.github.com/ti-kamlesh/73abfcc1a33318bb3b199d36b6209e59
[1] https://patchwork.kernel.org/comment/25492483/

Signed-off-by: Kamlesh Gurudasani <[email protected]>
---
Changes in v3:
- Changed the name for algorithm from crc64-iso to crc64-iso3309
- Removed unused code from Patch 2/6.
- Updated dt-bindings according to review comments Patch 3/6
- Added option to select SW implementation of algorithm Patch
4/6
- Added depends ORed with COMPILE_TEST Patch 4/6
- Patch 6/6, added specific devices that will benefit out of this
- Marking patch 5,6/6 as DONOTMERGE as they will go in via subsystem
Maintainer tree

Changes in v2:
- Add generic implementation of crc64-iso
- Fixes according to review comments
- Link to v1: https://lore.kernel.org/r/[email protected]

---
Kamlesh Gurudasani (6):
lib: add ISO 3309 model crc64
crypto: crc64 - add crc64-iso3309 framework
dt-bindings: crypto: Add Texas Instruments MCRC64
crypto: ti - add driver for MCRC64 engine
arm64: dts: ti: k3-am62: Add dt node, cbass_main ranges for MCRC64
arm64: defconfig: enable TI MCRC64 module

Documentation/devicetree/bindings/crypto/ti,mcrc64.yaml | 48 +++++++
MAINTAINERS | 7 ++
arch/arm64/boot/dts/ti/k3-am62-main.dtsi | 7 ++
arch/arm64/boot/dts/ti/k3-am62.dtsi | 1 +
arch/arm64/configs/defconfig | 2 +
crypto/Kconfig | 11 ++
crypto/Makefile | 1 +
crypto/crc64_iso3309_generic.c | 119 ++++++++++++++++++
crypto/tcrypt.c | 6 +
crypto/testmgr.c | 7 ++
crypto/testmgr.h | 404 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
drivers/crypto/Kconfig | 1 +
drivers/crypto/Makefile | 1 +
drivers/crypto/ti/Kconfig | 11 ++
drivers/crypto/ti/Makefile | 2 +
drivers/crypto/ti/mcrc64.c | 442 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
include/linux/crc64.h | 2 +
lib/crc64.c | 27 ++++
lib/gen_crc64table.c | 6 +
19 files changed, 1105 insertions(+)
---
base-commit: 13909a0c88972c5ef5d13f44d1a8bf065a31bdf4
change-id: 20240524-mcrc64-upstream-e0ce4aad64ad

Best regards,
--
Kamlesh Gurudasani <[email protected]>


2024-05-30 12:25:37

by Kamlesh Gurudasani

[permalink] [raw]
Subject: [PATCH v3 3/6] dt-bindings: crypto: Add Texas Instruments MCRC64

From: Kamlesh Gurudasani <[email protected]>

Add binding for Texas Instruments MCRC64

MCRC64 engine calculates 64-bit cyclic redundancy checks (CRC)
according to the ISO 3309 standard.

The ISO 3309 64-bit CRC model parameters are as follows:
Generator Polynomial: x^64 + x^4 + x^3 + x + 1
Polynomial Value: 0x000000000000001B
Initial value: 0x0000000000000000
Reflected Input: False
Reflected Output: False
Xor Final: 0x0000000000000000

Signed-off-by: Kamlesh Gurudasani <[email protected]>
---
Documentation/devicetree/bindings/crypto/ti,mcrc64.yaml | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
MAINTAINERS | 5 +++++
2 files changed, 53 insertions(+)

diff --git a/Documentation/devicetree/bindings/crypto/ti,mcrc64.yaml b/Documentation/devicetree/bindings/crypto/ti,mcrc64.yaml
new file mode 100644
index 000000000000..52505cc40a57
--- /dev/null
+++ b/Documentation/devicetree/bindings/crypto/ti,mcrc64.yaml
@@ -0,0 +1,48 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/crypto/ti,mcrc64.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Texas Instruments MCRC64
+
+description:
+ The MCRC64 engine calculates 64-bit cyclic redundancy checks
+ (CRC) according to the ISO 3309 standard.
+
+maintainers:
+ - Kamlesh Gurudasani <[email protected]>
+
+properties:
+ compatible:
+ const: ti,am62-mcrc64
+
+ reg:
+ maxItems: 1
+
+ clocks:
+ maxItems: 1
+
+ power-domains:
+ maxItems: 1
+
+required:
+ - compatible
+ - reg
+ - clocks
+ - power-domains
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/soc/ti,sci_pm_domain.h>
+
+ crc@30300000 {
+ compatible = "ti,am62-mcrc64";
+ reg = <0x30300000 0x1000>;
+ clocks = <&k3_clks 116 0>;
+ power-domains = <&k3_pds 116 TI_SCI_PD_EXCLUSIVE>;
+ };
+
+...
diff --git a/MAINTAINERS b/MAINTAINERS
index 613604d2e999..0c6bd9c22b91 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -22191,6 +22191,11 @@ S: Maintained
F: Documentation/devicetree/bindings/iio/adc/ti,lmp92064.yaml
F: drivers/iio/adc/ti-lmp92064.c

+TI MEMORY CYCLIC REDUNDANCY CHECK (MCRC64) DRIVER
+M: Kamlesh Gurudasani <[email protected]>
+S: Maintained
+F: Documentation/devicetree/bindings/crypto/ti,mcrc64.yaml
+
TI PCM3060 ASoC CODEC DRIVER
M: Kirill Marinushkin <[email protected]>
L: [email protected] (moderated for non-subscribers)

--
2.34.1

2024-05-30 12:25:47

by Kamlesh Gurudasani

[permalink] [raw]
Subject: [PATCH v3 1/6] lib: add ISO 3309 model crc64

From: Kamlesh Gurudasani <[email protected]>

Add the polynomial to the crc64 table generation, and provide a
generic library routine implementing the algorithm.

64-bit cyclic redundancy checks (CRC) according to the ISO 3309:1991
standard.

The ISO 3309:1991 64-bit CRC model parameters are as follows:
Generator Polynomial: x^64 + x^4 + x^3 + x + 1
Polynomial Value: 0x000000000000001B
Initial value: 0x0000000000000000
Reflected Input: False
Reflected Output: False
Xor Final: 0x0000000000000000

Signed-off-by: Kamlesh Gurudasani <[email protected]>
---
include/linux/crc64.h | 1 +
lib/crc64.c | 27 +++++++++++++++++++++++++++
lib/gen_crc64table.c | 6 ++++++
3 files changed, 34 insertions(+)

diff --git a/include/linux/crc64.h b/include/linux/crc64.h
index e044c60d1e61..b791316f251c 100644
--- a/include/linux/crc64.h
+++ b/include/linux/crc64.h
@@ -10,6 +10,7 @@
#define CRC64_ROCKSOFT_STRING "crc64-rocksoft"

u64 __pure crc64_be(u64 crc, const void *p, size_t len);
+u64 __pure crc64_iso3309_generic(u64 crc, const void *p, size_t len);
u64 __pure crc64_rocksoft_generic(u64 crc, const void *p, size_t len);

u64 crc64_rocksoft(const unsigned char *buffer, size_t len);
diff --git a/lib/crc64.c b/lib/crc64.c
index 61ae8dfb6a1c..40369dd26812 100644
--- a/lib/crc64.c
+++ b/lib/crc64.c
@@ -22,6 +22,11 @@
* x^24 + x^23 + x^22 + x^21 + x^19 + x^17 + x^13 + x^12 + x^10 + x^9 +
* x^7 + x^4 + x + 1
*
+ * crc64iso3309table[256] table is from the ISO-3309:1991 specification
+ * polynomial defined as,
+ *
+ * x^64 + x^4 + x^3 + x + 1
+ *
* crc64rocksoft[256] table is from the Rocksoft specification polynomial
* defined as,
*
@@ -63,6 +68,28 @@ u64 __pure crc64_be(u64 crc, const void *p, size_t len)
}
EXPORT_SYMBOL_GPL(crc64_be);

+/**
+ * crc64_iso3309_generic - Calculate bitwise ISO3309 CRC64
+ * @crc: seed value for computation. 0 for a new CRC calculation, or the
+ * previous crc64 value if computing incrementally.
+ * @p: pointer to buffer over which CRC64 is run
+ * @len: length of buffer @p
+ */
+u64 __pure crc64_iso3309_generic(u64 crc, const void *p, size_t len)
+{
+ size_t i, t;
+
+ const unsigned char *_p = p;
+
+ for (i = 0; i < len; i++) {
+ t = ((crc >> 56) ^ (*_p++)) & 0xFF;
+ crc = crc64iso3309table[t] ^ (crc << 8);
+ }
+
+ return crc;
+}
+EXPORT_SYMBOL_GPL(crc64_iso3309_generic);
+
/**
* crc64_rocksoft_generic - Calculate bitwise Rocksoft CRC64
* @crc: seed value for computation. 0 for a new CRC calculation, or the
diff --git a/lib/gen_crc64table.c b/lib/gen_crc64table.c
index 55e222acd0b8..abc860487463 100644
--- a/lib/gen_crc64table.c
+++ b/lib/gen_crc64table.c
@@ -17,9 +17,11 @@
#include <stdio.h>

#define CRC64_ECMA182_POLY 0x42F0E1EBA9EA3693ULL
+#define CRC64_ISO3309_POLY 0x000000000000001BULL
#define CRC64_ROCKSOFT_POLY 0x9A6C9329AC4BC9B5ULL

static uint64_t crc64_table[256] = {0};
+static uint64_t crc64_iso3309_table[256] = {0};
static uint64_t crc64_rocksoft_table[256] = {0};

static void generate_reflected_crc64_table(uint64_t table[256], uint64_t poly)
@@ -82,6 +84,9 @@ static void print_crc64_tables(void)
printf("static const u64 ____cacheline_aligned crc64table[256] = {\n");
output_table(crc64_table);

+ printf("\nstatic const u64 ____cacheline_aligned crc64iso3309table[256] = {\n");
+ output_table(crc64_iso3309_table);
+
printf("\nstatic const u64 ____cacheline_aligned crc64rocksofttable[256] = {\n");
output_table(crc64_rocksoft_table);
}
@@ -89,6 +94,7 @@ static void print_crc64_tables(void)
int main(int argc, char *argv[])
{
generate_crc64_table(crc64_table, CRC64_ECMA182_POLY);
+ generate_crc64_table(crc64_iso3309_table, CRC64_ISO3309_POLY);
generate_reflected_crc64_table(crc64_rocksoft_table, CRC64_ROCKSOFT_POLY);
print_crc64_tables();
return 0;

--
2.34.1

2024-05-30 12:26:50

by Kamlesh Gurudasani

[permalink] [raw]
Subject: [PATCH v3 6/6 DONOTMERGE] arm64: defconfig: enable TI MCRC64 module

From: Kamlesh Gurudasani <[email protected]>

J722S and all AM62** devices include MCRC64 engine for crc64 calculation.
Enable module to be built for them.

Also enable algif_hash module, which is needed to access MCRC64 module
from userspace.

Signed-off-by: Kamlesh Gurudasani <[email protected]>
---
arch/arm64/configs/defconfig | 2 ++
1 file changed, 2 insertions(+)

diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
index 2c30d617e180..aaeee392df10 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -1630,6 +1630,7 @@ CONFIG_CRYPTO_TEST=m
CONFIG_CRYPTO_ECHAINIV=y
CONFIG_CRYPTO_MICHAEL_MIC=m
CONFIG_CRYPTO_ANSI_CPRNG=y
+CONFIG_CRYPTO_USER_API_HASH=m
CONFIG_CRYPTO_USER_API_RNG=m
CONFIG_CRYPTO_CHACHA20_NEON=m
CONFIG_CRYPTO_GHASH_ARM64_CE=y
@@ -1653,6 +1654,7 @@ CONFIG_CRYPTO_DEV_HISI_ZIP=m
CONFIG_CRYPTO_DEV_HISI_HPRE=m
CONFIG_CRYPTO_DEV_HISI_TRNG=m
CONFIG_CRYPTO_DEV_SA2UL=m
+CONFIG_CRYPTO_DEV_TI_MCRC64=m
CONFIG_DMA_RESTRICTED_POOL=y
CONFIG_CMA_SIZE_MBYTES=32
CONFIG_PRINTK_TIME=y

--
2.34.1

2024-05-31 09:03:24

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v3 3/6] dt-bindings: crypto: Add Texas Instruments MCRC64

On 30/05/2024 14:24, [email protected] wrote:
> From: Kamlesh Gurudasani <[email protected]>
>
> Add binding for Texas Instruments MCRC64

Reviewed-by: Krzysztof Kozlowski <[email protected]>

Best regards,
Krzysztof


2024-06-10 14:34:40

by Kamlesh Gurudasani

[permalink] [raw]
Subject: Re: [PATCH v3 0/6] Add support for MCRC64 engine to calculate 64-bit CRC in Full-CPU mode

<[email protected]> writes:

> From: Kamlesh Gurudasani <[email protected]>
>
> MCRC64 engine calculates 64-bit cyclic redundancy checks (CRC)
> according to the ISO 3309 standard.
>

Hi Herbert,

Could you please review this and let me know if any changes are needed
to get it merged.

Thanks,
Kamlesh

2024-06-11 03:13:23

by Eric Biggers

[permalink] [raw]
Subject: Re: [PATCH v3 0/6] Add support for MCRC64 engine to calculate 64-bit CRC in Full-CPU mode

On Tue, Jun 11, 2024 at 10:31:45AM +0800, Herbert Xu wrote:
> On Mon, Jun 10, 2024 at 08:03:44PM +0530, Kamlesh Gurudasani wrote:
> > <[email protected]> writes:
> >
> > > From: Kamlesh Gurudasani <[email protected]>
> > >
> > > MCRC64 engine calculates 64-bit cyclic redundancy checks (CRC)
> > > according to the ISO 3309 standard.
> >
> > Could you please review this and let me know if any changes are needed
> > to get it merged.
>
> Eric Biggers had concerns about adding this to the kernel. I'd
> like know if he's OK with this or not.
>

I thought the rule is that there needs to be an in-kernel user to add algorithms
to the crypto API? Is there any precedent for adding new algorithms purely so
that accelerators that implement them can be accessed from userspace via AF_ALG?

Even if acceptable, the motivation for this one does seem weak, given that a
userspace software implementation would actually be faster. It could be
marginally useful for freeing up the CPU for other tasks if the inputs being
processed are very large (probably at least several megabytes), though.

- Eric

2024-06-11 03:14:16

by Eric Biggers

[permalink] [raw]
Subject: Re: [PATCH v3 1/6] lib: add ISO 3309 model crc64

On Thu, May 30, 2024 at 05:54:23PM +0530, [email protected] wrote:
> diff --git a/lib/crc64.c b/lib/crc64.c
> index 61ae8dfb6a1c..40369dd26812 100644
> --- a/lib/crc64.c
> +++ b/lib/crc64.c
> @@ -22,6 +22,11 @@
> * x^24 + x^23 + x^22 + x^21 + x^19 + x^17 + x^13 + x^12 + x^10 + x^9 +
> * x^7 + x^4 + x + 1
> *
> + * crc64iso3309table[256] table is from the ISO-3309:1991 specification
> + * polynomial defined as,
> + *
> + * x^64 + x^4 + x^3 + x + 1
> + *
> * crc64rocksoft[256] table is from the Rocksoft specification polynomial
> * defined as,
> *
> @@ -63,6 +68,28 @@ u64 __pure crc64_be(u64 crc, const void *p, size_t len)
> }
> EXPORT_SYMBOL_GPL(crc64_be);
>
> +/**
> + * crc64_iso3309_generic - Calculate bitwise ISO3309 CRC64
> + * @crc: seed value for computation. 0 for a new CRC calculation, or the
> + * previous crc64 value if computing incrementally.
> + * @p: pointer to buffer over which CRC64 is run
> + * @len: length of buffer @p
> + */
> +u64 __pure crc64_iso3309_generic(u64 crc, const void *p, size_t len)
> +{
> + size_t i, t;
> +
> + const unsigned char *_p = p;
> +
> + for (i = 0; i < len; i++) {
> + t = ((crc >> 56) ^ (*_p++)) & 0xFF;
> + crc = crc64iso3309table[t] ^ (crc << 8);
> + }
> +
> + return crc;
> +}
> +EXPORT_SYMBOL_GPL(crc64_iso3309_generic);

Putting this in lib/ seems premature, given that this is only used by
crypto/crc64_iso3309_generic.c.

- Eric