On Nov 28, 2023, at 12:25, Eric Biggers <ebiggers@xxxxxxxxxx> wrote: > On Mon, Nov 27, 2023 at 03:07:03PM +0800, Jerry Shih wrote: >> +config CRYPTO_CHACHA20_RISCV64 > > Can you call this kconfig option just CRYPTO_CHACHA_RISCV64? I.e. drop the > "20". The ChaCha family of ciphers includes more than just ChaCha20. > > The other architectures do use "CHACHA20" in their equivalent option, even when > they implement XChaCha12 too. But that's for historical reasons -- we didn't > want to break anything by renaming the kconfig options. For a new option we > should use the more general name from the beginning, even if initially only > ChaCha20 is implemented (which is fine). I will use `CRYPTO_CHACHA_RISCV64` instead. >> +static int chacha20_encrypt(struct skcipher_request *req) > > riscv64_chacha_crypt(), please. chacha20_encrypt() is dangerously close to > being the same name as chacha20_crypt() which already exists in crypto/chacha.h. The function will will have additional prefix/suffix. >> +static inline bool check_chacha20_ext(void) >> +{ >> + return riscv_isa_extension_available(NULL, ZVKB) && >> + riscv_vector_vlen() >= 128; >> +} > > Just to double check: your intent is to simply require VLEN >= 128 for all the > RISC-V vector crypto code, even when some might work with a shorter VLEN? I > don't see anything in chacha-riscv64-zvkb.pl that assumes VLEN >= 128, for > example. I think it would even work with VLEN == 32. Yes, the chacha algorithm here only needs the VLEN>=32. But I think we will not get benefits with that kind of hw. > I think requiring VLEN >= 128 anyway makes sense so that we don't have to worry > about validating the code with shorter VLEN. And "application processors" are > supposed to have VLEN >= 128. But I just wanted to make sure this is what you > intended too. The standard "V" extension assumes VLEN>=128. I just follow that assumption. https://github.com/riscv/riscv-v-spec/blob/master/v-spec.adoc#183-v-vector-extension-for-application-processors -Jerry