Message-ID: <CAMj1kXEAqxCZ_PNM9a=CyciSHUzDU_yqemKh51oncHyLbYUKtA@mail.gmail.com>
Date: Wed, 3 Jan 2024 15:00:29 +0100
From: Ard Biesheuvel <ardb@...nel.org>
To: Eric Biggers <ebiggers@...nel.org>
Cc: linux-crypto@...r.kernel.org, linux-riscv@...ts.infradead.org, 
	Jerry Shih <jerry.shih@...ive.com>, linux-kernel@...r.kernel.org, 
	Heiko Stuebner <heiko@...ech.de>, Phoebe Chen <phoebe.chen@...ive.com>, hongrong.hsu@...ive.com, 
	Paul Walmsley <paul.walmsley@...ive.com>, Palmer Dabbelt <palmer@...belt.com>, 
	Albert Ou <aou@...s.berkeley.edu>, Andy Chiu <andy.chiu@...ive.com>
Subject: Re: [RFC PATCH 00/13] RISC-V crypto with reworked asm files

On Tue, 2 Jan 2024 at 07:50, Eric Biggers <ebiggers@...nel.org> wrote:
>
> As discussed previously, the proposed use of the so-called perlasm for
> the RISC-V crypto assembly files makes them difficult to read, and these
> files have some other issues such extensively duplicating source code
> for the different AES key lengths and for the unrolled hash functions.
> There is/was a desire to share code with OpenSSL, but many of the files
> have already diverged significantly; also, for most of the algorithms
> the source code can be quite short anyway, due to the native support for
> them in the RISC-V vector crypto extensions combined with the way the
> RISC-V vector extension naturally scales to arbitrary vector lengths.
>
> Since we're still waiting for prerequisite patches to be merged anyway,
> we have a bit more time to get this cleaned up properly.  So I've had a
> go at cleaning up the patchset to use standard .S files, with the code
> duplication fixed.  I also made some tweaks to make the different
> algorithms consistent with each other and with what exists in the kernel
> already for other architectures, and tried to improve comments.
>
> The result is this series, which passes all tests and is about 2400
> lines shorter than the latest version with the perlasm
> (https://lore.kernel.org/linux-crypto/20231231152743.6304-1-jerry.shih@sifive.com/).
> All the same functionality and general optimizations are still included,
> except for some minor optimizations in XTS that I dropped since it's not
> clear they are worth the complexity.  (Note that almost all users of XTS
> in the kernel only use it with block-aligned messages, so it's not very
> important to optimize the ciphertext stealing case.)
>
> I'd appreciate people's thoughts on this series.  Jerry, I hope I'm not
> stepping on your toes too much here, but I think there are some big
> improvements here.
>

As I have indicated before, I fully agree with Eric here that avoiding
perlasm is preferable: sharing code with OpenSSL is great if we can
simply adopt the exact same code (and track OpenSSL as its upstream)
but this never really worked that well for skciphers, due to API
differences. (The SHA transforms can be reused a bit more easily)

I will also note that perlasm is not as useful for RISC-V as it is for
other architectures: in OpenSSL, perlasm is also used to abstract
differences in calling conventions between, e.g., x86_64 on Linux vs
Windows, or to support building with outdated [proprietary]
toolchains.

I do wonder if we could also use .req directives for register aliases
instead of CPP defines? It shouldn't matter for working code, but the
diagnostics tend to be a bit more useful if the aliases are visible to
the assembler.







> This series is based on riscv/for-next
> (https://git.kernel.org/pub/scm/linux/kernel/git/riscv/linux.git/log/?h=for-next)
> commit f352a28cc2fb4ee8d08c6a6362c9a861fcc84236, and for convenience
> I've included the prerequisite patches too.
>
> Andy Chiu (1):
>   riscv: vector: make Vector always available for softirq context
>
> Eric Biggers (1):
>   RISC-V: add TOOLCHAIN_HAS_VECTOR_CRYPTO
>
> Greentime Hu (1):
>   riscv: Add support for kernel mode vector
>
> Heiko Stuebner (2):
>   RISC-V: add helper function to read the vector VLEN
>   RISC-V: hook new crypto subdir into build-system
>
> Jerry Shih (8):
>   crypto: riscv - add vector crypto accelerated AES
>   crypto: riscv - add vector crypto accelerated AES-{ECB,CBC,CTR,XTS}
>   crypto: riscv - add vector crypto accelerated ChaCha20
>   crypto: riscv - add vector crypto accelerated GHASH
>   crypto: riscv - add vector crypto accelerated SHA-{256,224}
>   crypto: riscv - add vector crypto accelerated SHA-{512,384}
>   crypto: riscv - add vector crypto accelerated SM3
>   crypto: riscv - add vector crypto accelerated SM4
>
>  arch/riscv/Kbuild                             |   1 +
>  arch/riscv/Kconfig                            |   7 +
>  arch/riscv/crypto/Kconfig                     | 108 +++++
>  arch/riscv/crypto/Makefile                    |  28 ++
>  arch/riscv/crypto/aes-macros.S                | 156 +++++++
>  .../crypto/aes-riscv64-block-mode-glue.c      | 435 ++++++++++++++++++
>  arch/riscv/crypto/aes-riscv64-glue.c          | 123 +++++
>  arch/riscv/crypto/aes-riscv64-glue.h          |  15 +
>  .../crypto/aes-riscv64-zvkned-zvbb-zvkg.S     | 300 ++++++++++++
>  arch/riscv/crypto/aes-riscv64-zvkned-zvkb.S   | 146 ++++++
>  arch/riscv/crypto/aes-riscv64-zvkned.S        | 180 ++++++++
>  arch/riscv/crypto/chacha-riscv64-glue.c       | 101 ++++
>  arch/riscv/crypto/chacha-riscv64-zvkb.S       | 294 ++++++++++++
>  arch/riscv/crypto/ghash-riscv64-glue.c        | 170 +++++++
>  arch/riscv/crypto/ghash-riscv64-zvkg.S        |  72 +++
>  arch/riscv/crypto/sha256-riscv64-glue.c       | 137 ++++++
>  .../sha256-riscv64-zvknha_or_zvknhb-zvkb.S    | 225 +++++++++
>  arch/riscv/crypto/sha512-riscv64-glue.c       | 133 ++++++
>  .../riscv/crypto/sha512-riscv64-zvknhb-zvkb.S | 203 ++++++++
>  arch/riscv/crypto/sm3-riscv64-glue.c          | 112 +++++
>  arch/riscv/crypto/sm3-riscv64-zvksh-zvkb.S    | 123 +++++
>  arch/riscv/crypto/sm4-riscv64-glue.c          | 109 +++++
>  arch/riscv/crypto/sm4-riscv64-zvksed-zvkb.S   | 117 +++++
>  arch/riscv/include/asm/processor.h            |  14 +-
>  arch/riscv/include/asm/simd.h                 |  48 ++
>  arch/riscv/include/asm/vector.h               |  20 +
>  arch/riscv/kernel/Makefile                    |   1 +
>  arch/riscv/kernel/kernel_mode_vector.c        | 126 +++++
>  arch/riscv/kernel/process.c                   |   1 +
>  crypto/Kconfig                                |   3 +
>  30 files changed, 3507 insertions(+), 1 deletion(-)
>  create mode 100644 arch/riscv/crypto/Kconfig
>  create mode 100644 arch/riscv/crypto/Makefile
>  create mode 100644 arch/riscv/crypto/aes-macros.S
>  create mode 100644 arch/riscv/crypto/aes-riscv64-block-mode-glue.c
>  create mode 100644 arch/riscv/crypto/aes-riscv64-glue.c
>  create mode 100644 arch/riscv/crypto/aes-riscv64-glue.h
>  create mode 100644 arch/riscv/crypto/aes-riscv64-zvkned-zvbb-zvkg.S
>  create mode 100644 arch/riscv/crypto/aes-riscv64-zvkned-zvkb.S
>  create mode 100644 arch/riscv/crypto/aes-riscv64-zvkned.S
>  create mode 100644 arch/riscv/crypto/chacha-riscv64-glue.c
>  create mode 100644 arch/riscv/crypto/chacha-riscv64-zvkb.S
>  create mode 100644 arch/riscv/crypto/ghash-riscv64-glue.c
>  create mode 100644 arch/riscv/crypto/ghash-riscv64-zvkg.S
>  create mode 100644 arch/riscv/crypto/sha256-riscv64-glue.c
>  create mode 100644 arch/riscv/crypto/sha256-riscv64-zvknha_or_zvknhb-zvkb.S
>  create mode 100644 arch/riscv/crypto/sha512-riscv64-glue.c
>  create mode 100644 arch/riscv/crypto/sha512-riscv64-zvknhb-zvkb.S
>  create mode 100644 arch/riscv/crypto/sm3-riscv64-glue.c
>  create mode 100644 arch/riscv/crypto/sm3-riscv64-zvksh-zvkb.S
>  create mode 100644 arch/riscv/crypto/sm4-riscv64-glue.c
>  create mode 100644 arch/riscv/crypto/sm4-riscv64-zvksed-zvkb.S
>  create mode 100644 arch/riscv/include/asm/simd.h
>  create mode 100644 arch/riscv/kernel/kernel_mode_vector.c
>
>
> base-commit: f352a28cc2fb4ee8d08c6a6362c9a861fcc84236
> --
> 2.43.0
>

Powered by blists - more mailing lists