On Thu, Apr 18, 2024 at 10:32:03AM -0700, Eric Biggers wrote: > On Thu, Apr 18, 2024 at 05:53:55PM +0100, Conor Dooley wrote: > > > If it would be useful to do so, we should be able to enable some of the code > > > with a smaller VLEN and/or EEW once it has been tested in those configurations. > > > Some of it should work, but some of it won't be able to work. (For example, the > > > SHA512 instructions require EEW==64.) > > > > > > Also note that currently all the RISC-V vector crypto code only supports riscv64 > > > (XLEN=64). Similarly, that could be relaxed in the future if people really need > > > the vector crypto acceleration on 32-bit CPUs... But similarly, the code would > > > need to be revised and tested in that configuration. > > > > > > > Eric/Jerry (although read the previous paragraph too): > > > > I noticed that the sha256 glue code calls crypto_simd_usable(), and in > > > > turn may_use_simd() before kernel_vector_begin(). The chacha20 glue code > > > > does not call either, which seems to violate the edict in > > > > kernel_vector_begin()'s kerneldoc: > > > > "Must not be called unless may_use_simd() returns true." > > > > > > skcipher algorithms can only be invoked in process and softirq context. This > > > differs from shash algorithms which can be invoked in any context. > > > > > > My understanding is that, like arm64, RISC-V always allows non-nested > > > kernel-mode vector to be used in process and softirq context -- and in fact, > > > this was intentionally done in order to support use cases like this. So that's > > > why the RISC-V skcipher algorithms don't check for may_use_simd() before calling > > > kernel_vector_begin(). > > > > I see, thanks for explaining that. I think you should probably check > > somewhere if has_vector() returns true in that driver though before > > using vector instructions. Only checking vlen seems to me like relying on > > an implementation detail and if we set vlen for the T-Head/0.7.1 vector > > it'd be fooled. That said, I don't think that any of the 0.7.1 vector > > systems actually support Zvkb, but I hope you get my drift. > > All the algorithms check for at least one of the vector crypto extensions being > supported, for example Zvkb. 'if (riscv_isa_extension_available(NULL, ZVKB))' > should return whether the ratified version of Zvkb is supported, and likewise > for the other vector crypto extensions. The ratified version of the vector > crypto extensions depends on the ratified version of the vector extension. So > there should be no issue. If there is, the RISC-V core architecture code needs > to be fixed to not declare that extensions are supported when they are actually > incompatible non-standard versions of those extensions. Incompatible > non-standard extensions should be represented as separate extensions. > It probably makes sense to check has_vector() to exclude Zve* for now, though. I am just concerned about how you're suggesting that non-standard extensions might be pretending to be standard ones and individual users of kernel-mode vector would need to work around that. I think that neither has_vector() nor 'if (riscv_isa_extension_available(NULL, ZVKB))' should return true if the CPU's vector extension is non-standard. - Eric
