[v3,0/6] Add support for MCRC64 engine to calculate 64-bit CRC in Full-CPU mode

From: Kamlesh Gurudasani <kamlesh@ti.com>

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 <kamlesh@ti.com>
---
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/20230719-mcrc-upstream-v1-0-dc8798a24c47@ti.com

---
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@ti.com> writes:

> From: Kamlesh Gurudasani <kamlesh@ti.com>
>
> 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

On Mon, Jun 10, 2024 at 08:03:44PM +0530, Kamlesh Gurudasani wrote:
> <kamlesh@ti.com> writes:
> 
> > From: Kamlesh Gurudasani <kamlesh@ti.com>
> >
> > 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.

Thanks,

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:
> > <kamlesh@ti.com> writes:
> > 
> > > From: Kamlesh Gurudasani <kamlesh@ti.com>
> > >
> > > 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

On Mon, Jun 10, 2024 at 08:13:14PM -0700, Eric Biggers wrote:
>
> 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?

I agree.  Perhaps this driver could instead be added as a simple
char device that is then used directly by the intended user without
going through the Crypto API at all.

That would make it a lot simpler.

Cheers,