[v2,0/6] Add support for Texas Instruments MCRC64 engine

  Add support for MCRC64 engine to calculate 64-bit CRC in Full-CPU mode

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

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,
https://gist.github.com/ti-kamlesh/73abfcc1a33318bb3b199d36b6209e59

Signed-off-by: Kamlesh Gurudasani <kamlesh@ti.com>
---
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-iso 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 |  47 ++++++++
 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_iso_generic.c                              | 119 ++++++++++++++++++
 crypto/tcrypt.c                                         |   5 +
 crypto/testmgr.c                                        |   7 ++
 crypto/testmgr.h                                        | 404 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 drivers/crypto/Kconfig                                  |   1 +
 drivers/crypto/Makefile                                 |   1 +
 drivers/crypto/ti/Kconfig                               |  10 ++
 drivers/crypto/ti/Makefile                              |   2 +
 drivers/crypto/ti/mcrc64.c                              | 442 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/crc64.h                                   |   5 +
 lib/crc64-iso.c                                         | 126 +++++++++++++++++++
 lib/crc64.c                                             |  27 +++++
 lib/gen_crc64table.c                                    |   6 +
 20 files changed, 1231 insertions(+)
---
base-commit: 21ef7b1e17d039053edaeaf41142423810572741
change-id: 20230719-mcrc-upstream-7ae9a75cab37

Best regards,
  

On Fri, Aug 11, 2023 at 12:58:47AM +0530, Kamlesh Gurudasani wrote:
> Add support for MCRC64 engine to calculate 64-bit CRC in Full-CPU mode
> 
> 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
> 
> 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,
> https://gist.github.com/ti-kamlesh/73abfcc1a33318bb3b199d36b6209e59
> 
> Signed-off-by: Kamlesh Gurudasani <kamlesh@ti.com>

I do not see any in-kernel user of this CRC variant being introduced, which
leaves algif_hash as the only use case.

Can you elaborate on the benefit this brings to your application?  Yes, it
allows you to use your hardware CRC engine.  But, that comes with all the
overhead from the syscalls, algif_hash, and the driver.  How does performance
compare to a properly optimized software CRC implementation on your platform,
i.e. an implementation using carryless multiplication instructions (e.g. ARMv8
CE) if available on your platform, otherwise an implementation using the
slice-by-8 or slice-by-16 method?

- Eric
  

On Fri, Aug 18, 2023 at 02:36:34PM +0530, Kamlesh Gurudasani wrote:
> Hi Eric,
> 
> We are more interested in offload than performance, with splice system
> call and DMA mode in driver(will be implemented after this series gets
> merged), good amount of cpu cycles will be saved.

So it's for power usage, then?  Or freeing up CPU for other tasks?

> There is one more mode(auto mode) in mcrc64 which helps to verify crc64
> values against pre calculated crc64, saving the efforts of comparing in
> userspace.

Is there any path forward to actually support this?

> 
> Current generic implementation of crc64-iso(part of this series)
> gives 173 Mb/s of speed as opposed to mcrc64 which gives speed of 812
> Mb/s when tested with tcrypt.

This doesn't answer my question, which to reiterate was:

    How does performance compare to a properly optimized software CRC
    implementation on your platform, i.e. an implementation using carryless
    multiplication instructions (e.g. ARMv8 CE) if available on your platform,
    otherwise an implementation using the slice-by-8 or slice-by-16 method?

The implementation you tested was slice-by-1.  Compared to that, it's common for
slice-by-8 to speed up CRCs by about 4 times and for folding with carryless
multiplication to speed up CRCs by 10-30 times, sometimes limited only by memory
bandwidth.  I don't know what specific results you would get on your specific
CPU and for this specific CRC, and you could certainly see something different
if you e.g. have some low-end embedded CPU.  But those are the typical results
I've seen for other CRCs on different CPUs.  So, a software implementation may
be more attractive than you realize.  It could very well be the case that a
PMULL based CRC implementation actually ends up with less CPU load than your
"hardware offload", when taking into syscall, algif_hash, and driver overhead...

- Eric
  

Kamlesh Gurudasani <kamlesh@ti.com> writes:
...

> Hi Eric, thanks for your detailed and valuable inputs.
>
> As per your suggestion, we did some profiling. 
>
> Use case is to calculate crc32/crc64 for file input from user space.
>
> Instead of directly implementing PMULL based CRC64, we made first comparison between 
> Case 1.
> CRC32 (splice() + kernel space SW driver) 
> https://gist.github.com/ti-kamlesh/5be75dbde292e122135ddf795fad9f21
>
> Case 2.
> CRC32(mmap() + userspace armv8 crc32 instruction implementation)
> (tried read() as well to get contents of file, but that lost to mmap() so not mentioning number here)
> https://gist.github.com/ti-kamlesh/002df094dd522422c6cb62069e15c40d
>
> Case 3.
> CRC64 (splice() + MCRC64 HW)
> https://gist.github.com/ti-kamlesh/98b1fc36c9a7c3defcc2dced4136b8a0
>
>
> Overall, overhead of userspace + af_alg + driver in (Case 1) and
> ( Case 3) is ~0.025s, which is constant for any file size.
> This is calculated using real time to calculate crc  -
> driver time (time spend inside init() + update() +final()) = overhead ~0.025s    
>
>
>
> +-------------------+-----------------------------+-----------------------+------------------------+------------------------+
> |                   |                             |                       |                        |                        |
> | File size         | 120mb(ideal size for us)    | 20mb                  | 15mb                   | 5mb                    |
> +===================+=============================+=======================+========================+========================+
> |                   |                             |                       |                        |                        |
> | CRC32 (Case 1)    | Driver time 0.155s          | Driver time 0.0325s   | Driver time 0.019s     | Driver time 0.0062s    |
> |                   |    real time 0.18s          |    real time 0.06s    |    real time 0.04s     |    real time 0.03s     |
> |                   |    overhead 0.025s          |    overhead 0.025s    |    overhead 0.021s     |    overhead ~0.023s    |
> +-------------------+-----------------------------+-----------------------+------------------------+------------------------+
> |                   |                             |                       |                        |                        |
> | CRC32 (Case 2)    | Real time 0.30s             | Real time 0.05s       | Real time 0.04s        | Real time 0.02s        |
> +-------------------+-----------------------------+-----------------------+------------------------+------------------------+
> |                   |                             |                       |                        |                        |
> | CRC64 (Case 3)    | Driver time   0.385s        | Driver time 0.0665s   | Driver time 0.0515s    | Driver time 0.019s     |
> |                   |    real time 0.41s          |    real time 0.09s    |    real time 0.08s     |    real time 0.04s     |
> |                   |    overhead 0.025s          |    overhead 0.025s    |    overhead ~0.025s    |    overhead ~0.021s    |
> +-------------------+-----------------------------+-----------------------+------------------------+------------------------+
>
> Here, if we consider similar numbers for crc64 PMULL implementation as
> crc32 (case 2) , we save good number of cpu cycles using mcrc64
> in case of files bigger than 5-10mb as most of the time is being spent in HW offload.
>
> Regards,
> Kamlesh

Hi Eric,

Please let me know if above numbers make sense to you and I should send
next revision.

Regards,
Kamlesh
  

Kamlesh Gurudasani <kamlesh@ti.com> writes:

>>
>> Here, if we consider similar numbers for crc64 PMULL implementation as
>> crc32 (case 2) , we save good number of cpu cycles using mcrc64
>> in case of files bigger than 5-10mb as most of the time is being spent in HW offload.
>>
>> Regards,
>> Kamlesh
>
> Hi Eric,
>
> Please let me know if above numbers make sense to you and I should send
> next revision.

Hi Eric,

I understand that there is no in-kernel user for crc64-iso3309 and this
is new algorithm that we are trying to add in linux kernel.

As per your suggestion we did the calculations and it turns out to be we
are saving good number of cpu cycles with HW offload.

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

Let me know if you are willing to accept this driver, so that I can put
efforts to send next revision.

Regards,
Kamlesh