On Thu, Apr 18, 2024 at 08:52:56AM -0700, Eric Biggers wrote:
Hi Conor,
On Thu, Apr 18, 2024 at 12:02:10PM +0100, Conor Dooley wrote:
+CC Eric, Jerry
On Fri, Apr 12, 2024 at 02:49:03PM +0800, Andy Chiu wrote:
Make has_vector take one argument. This argument represents the minimum Vector subextension that the following Vector actions assume.
Also, change riscv_v_first_use_handler(), and boot code that calls riscv_v_setup_vsize() to accept the minimum Vector sub-extension, ZVE32X.
Most kernel/user interfaces requires minimum of ZVE32X. Thus, programs compiled and run with ZVE32X should be supported by the kernel on most aspects. This includes context-switch, signal, ptrace, prctl, and hwprobe.
One exception is that ELF_HWCAP returns 'V' only if full V is supported on the platform. This means that the system without a full V must not rely on ELF_HWCAP to tell whether it is allowable to execute Vector without first invoking a prctl() check.
Signed-off-by: Andy Chiu andy.chiu@sifive.com Acked-by: Joel Granados j.granados@samsung.com
I'm not sure that I like this patch to be honest. As far as I can tell, every user here of has_vector(ext) is ZVE32X, so why bother actually having an argument?
Could we just document that has_vector() is just a tyre kick of "is there a vector unit and are we allowed to use it", and anything requiring more than the bare-minimum (so zve32x?)must explicitly check for that form of vector using riscv_has_extension_[un]likely()?
Finally, the in-kernel crypto stuff or other things that use can_use_simd() to check for vector support - do they all function correctly with all of the vector flavours? I don't understand the vector extensions well enough to evaluate that - I know that they do check for the individual extensions like Zvkb during probe but don't have anything for the vector version (at least in the chacha20 and sha256 glue code). If they don't, then we need to make sure those drivers do not probe with the cut-down variants.
As far as I know, none of the RISC-V vector crypto code has been tested with Zve* yet. Currently it always checks for VLEN >= 128, which should exclude most Zve* implementations.
Currently it doesn't check for EEW >= 64, even though it sometimes assumes that. It looks like a check for EEW >= 64 needs to be added in order to exclude Zve32x and Zve32f implementations that don't support EEW == 64.
Cool, glad I asked then :)
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.
Thanks, Conor.