[PATCH v4] crypto: riscv/poly1305 - import OpenSSL/CRYPTOGAMS implementation

[Eric Biggers]

Hi Zhihang,

On Wed, Jun 11, 2025 at 11:31:51AM +0800, Zhihang Shao wrote:
> From: Zhihang Shao <zhihang.shao.iscas at gmail.com>
> 
> This is a straight import of the OpenSSL/CRYPTOGAMS Poly1305
> implementation for riscv authored by Andy Polyakov.
> The file 'poly1305-riscv.pl' is taken straight from this upstream
> GitHub repository [0] at commit 33fe84bc21219a16825459b37c825bf4580a0a7b,
> and this commit fixed a bug in riscv 64bit implementation.
> 
> [0] https://github.com/dot-asm/cryptogams
> 
> Signed-off-by: Zhihang Shao <zhihang.shao.iscas at gmail.com>

I wanted to let you know that I haven't forgotten about this.  However, the
kernel currently lacks a proper test for Poly1305, and Poly1305 has some edge
cases that are prone to bugs.  So I'd like to find some time to add a proper
Poly1305 test and properly review this.  Also, I'm in the middle of fixing how
the kernel's architecture-optimized crypto code is integrated; for example, I've
just moved arch/*/lib/crypto/ to lib/crypto/.

There will also need to be benchmark results that show that this code is
actually worthwhile, on both RV32 and RV64.  I am planning for the
poly1305_kunit test to have a benchmark built-in, after which it will be
straightforward to collect these.

(The "perlasm" file does have some benchmark results in a comment, but they do
not necessarily apply to the Poly1305 code in the kernel.)

So this isn't a perfect time to be adding a new Poly1305 implementation, as
we're not quite ready for it.  But I'd indeed like to take this eventually.

A few comments below:

> diff --git a/arch/riscv/lib/crypto/Makefile b/arch/riscv/lib/crypto/Makefile
> index b7cb877a2c07..93ddb62ef0f9 100644
> --- a/arch/riscv/lib/crypto/Makefile
> +++ b/arch/riscv/lib/crypto/Makefile
> @@ -3,5 +3,23 @@
>  obj-$(CONFIG_CRYPTO_CHACHA_RISCV64) += chacha-riscv64.o
>  chacha-riscv64-y := chacha-riscv64-glue.o chacha-riscv64-zvkb.o
>  
> +obj-$(CONFIG_CRYPTO_POLY1305_RISCV) += poly1305-riscv.o
> +poly1305-riscv-y := poly1305-core.o poly1305-glue.o
> +AFLAGS_poly1305-core.o += -Dpoly1305_init=poly1305_block_init_arch
> +AFLAGS_poly1305-core.o += -Dpoly1305_blocks=poly1305_blocks_arch
> +AFLAGS_poly1305-core.o += -Dpoly1305_emit=poly1305_emit_arch
> +
>  obj-$(CONFIG_CRYPTO_SHA256_RISCV64) += sha256-riscv64.o
>  sha256-riscv64-y := sha256.o sha256-riscv64-zvknha_or_zvknhb-zvkb.o
> +
> +ifeq ($(CONFIG_64BIT),y)
> +PERLASM_ARCH := 64
> +else
> +PERLASM_ARCH := void
> +endif
> +
> +quiet_cmd_perlasm = PERLASM $@
> +      cmd_perlasm = $(PERL) $(<) $(PERLASM_ARCH) $(@)
> +
> +$(obj)/%-core.S: $(src)/%-riscv.pl
> +	$(call cmd,perlasm)

Missing:

    clean-files += poly1305-core.S

> diff --git a/arch/riscv/lib/crypto/poly1305-glue.c b/arch/riscv/lib/crypto/poly1305-glue.c
> new file mode 100644
> index 000000000000..ddc73741faf5
> --- /dev/null
> +++ b/arch/riscv/lib/crypto/poly1305-glue.c
> @@ -0,0 +1,37 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * OpenSSL/Cryptogams accelerated Poly1305 transform for riscv
> + *
> + * Copyright (C) 2025 Institute of Software, CAS.
> + */
> +
> +#include <asm/hwcap.h>
> +#include <asm/simd.h>
> +#include <crypto/internal/poly1305.h>
> +#include <linux/cpufeature.h>
> +#include <linux/jump_label.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/unaligned.h>

Reduce the include list to:

#include <crypto/internal/poly1305.h>
#include <linux/export.h>
#include <linux/module.h>

> +.globl	poly1305_init
> +.type	poly1305_init,\@function
> +poly1305_init:
> +#ifdef	__riscv_zicfilp
> +	lpad	0
> +#endif

The 'lpad' instructions aren't present in the upstream CRYPTOGAMS source.

If they are necessary, this addition needs to be documented.

But they appear to be unnecessary.

> +#ifndef	__CHERI_PURE_CAPABILITY__
> +	andi	$tmp0,$inp,7		# $inp % 8
> +	andi	$inp,$inp,-8		# align $inp
> +	slli	$tmp0,$tmp0,3		# byte to bit offset
> +#endif
> +	ld	$in0,0($inp)
> +	ld	$in1,8($inp)
> +#ifndef	__CHERI_PURE_CAPABILITY__
> +	beqz	$tmp0,.Laligned_key
> +
> +	ld	$tmp2,16($inp)
> +	neg	$tmp1,$tmp0		# implicit &63 in sll
> +	srl	$in0,$in0,$tmp0
> +	sll	$tmp3,$in1,$tmp1
> +	srl	$in1,$in1,$tmp0
> +	sll	$tmp2,$tmp2,$tmp1
> +	or	$in0,$in0,$tmp3
> +	or	$in1,$in1,$tmp2
> +
> +.Laligned_key:

This code is going through a lot of trouble to work on RISC-V CPUs that don't
support efficient misaligned memory accesses.  That includes issuing loads of
memory outside the bounds of the given buffer, which is questionable (even if
it's guaranteed to not cross a page boundary).

Is there any chance we can just make the RISC-V Poly1305 code be conditional on
CONFIG_RISCV_EFFICIENT_UNALIGNED_ACCESS=y?  Or do people not actually use that?

The rest of the kernel's RISC-V crypto code, which is based on the vector
extension, just assumes that efficient misaligned memory accesses are supported.

On a related topic, if this patch is accepted, the result will be inconsistent
optimization of ChaCha vs. Poly1305, which are usually paired:

    (1) ChaCha optimized with the RISC-V vector extension
    (2) Poly1305 optimized with RISC-V scalar instructions

Surely a RISC-V vector extension optimized Poly1305 is going to be needed too?

But with that being the case, will a RISC-V scalar optimized Poly1305 actually
be worthwhile to add too?  Especially without optimized ChaCha alongside it?

- Eric