On Thu, May 09, 2024 at 09:25:25AM +0100, Conor Dooley wrote:
On Thu, May 09, 2024 at 08:48:09AM +0100, Conor Dooley wrote:
On Thu, May 09, 2024 at 02:56:30PM +0800, Andy Chiu wrote:
Hi Conor,
Should we check if "v" presents for vector crypto extensions in riscv_isa_extension_check()? We are not checking this for now. So a kernel compiled with RISCV_ISA_V still has a problem if its isa-string includes any of vector crypto ("zvbb, zvkg, etc") but not "v".
Yeah, one of the things I took away from this discussion is that we need to improve the implementation of both the methods we have at the moment for drivers etc to check if extensions are present and usable. In general, I don't think checks like that are "safe" to do in riscv_isa_extension_check(), because the dependencies may not all have been resolved when we probe an extension (Clement's current Zca etc series improves the situation though by only calling the checks after we probe all extensions).
The simple V cases are all fine though - the DT binding and ACPI rules for compatible strings all mandate that single-letter extensions must come before multi-letter ones. For riscv,isa-extensions we control the probe ordering and probe V before any multi-letter stuff. Additionally, we should make it a requirement for V to be present if things that depend on it are.
That said, is it permitted by the specs to have any of the extensions you mention without the full V extension, but with one of the cut-down variants you mention here? If not, I'd be more interested in figuring out the non-extension dependencies: whether or not the kernel itself supports vector and if the kernel has opted to disable vector due to detecting that harts have mismatching vector lengths.
TL;DR: I think we should add some checks in riscv_isa_extension_check().
Also, unless this only becomes a problem with this series that adds the cut-down forms of vector, I think this is a separate problem to solve and I can send some patches for it (along with some other cleanup I'd like to do as a result of Eric's comments) and you can just submit the v2 you were planning to without it. I can't, off the top of my head, think of why this particular series would break the vector crypto stuff though, the problems with enabling extensions seem underlying.
Here's something buggy that I chucked together as an idea of what I meant: https://git.kernel.org/pub/scm/linux/kernel/git/conor/linux.git/commit/?h=ri... Beware, it is entirely untested :) It's based on both this series and patches 2 & 3 of Charlie's series doing the T-Head vector stuff. It really needs Clement's extension_check() rework that I mentioned 2 mails ago to function correctly for any of these vector subsets. Without Clement's stuff, it'll have "random" behaviour depending on probe order for riscv,isa and a determinate, but incorrect, behaviour otherwise.
Cheers, Conor.