On Thu, May 16, 2024 at 01:28:45PM -0700, Charlie Jenkins wrote:
On Thu, May 16, 2024 at 05:24:25PM +0100, Conor Dooley wrote:
On Thu, May 16, 2024 at 10:00:12PM +0800, Andy Chiu wrote:
On Sat, May 4, 2024 at 2:21 AM Charlie Jenkins charlie@rivosinc.com wrote:
if (elf_hwcap & COMPAT_HWCAP_ISA_V && has_riscv_homogeneous_vlenb() < 0) {pr_warn("Unsupported heterogeneous vlen detected, vector extension disabled.\elf_hwcap &= ~COMPAT_HWCAP_ISA_V;}We only touch COMPAT_HWCAP_ISA_V and the failed case only turns off the rectified V. So here we have nothing to do with the Xtheadvector.
There's nothing t-head related in the tree at this point, so doing anything with it would cause build issues.
However, I am still confused because I think Xtheadvector would also need to call into this check, so as to setup vlenb.
Apart from that, it seems like some vendor stating Xtheadvector is actually vector-0.7.
The T-Head implementation is 0.7.x, but I am not really sure what you mean by this comment.
Andy, the idea of this patch was to be able to support this binding on more than just xtheadvector.
You are correct though Andy, this is a problem that a later patch in this series doesn't disable xtheadvector when vlenb is not homogeneous. I am going to wait to send out any more versions until after this merge window but I will fix this in the next version. Thank you!
Agreed on all counts :)
Please correct me if I speak anything wrong. One thing I noticed is that Xtheadvector wouldn't trap on reading th.vlenb but vector-0.7 would. If that is the case, should we require Xtheadvector to specify `riscv,vlenb` on the device tree?
In the world of Linux, "vector-0.7" isn't a thing. There's only 1.0, and after this patchset, "xtheadvector". My understanding, from discussion on earlier versions of this series the trap is actually accessing th.vlenb register, despite the documentation stating that it is unprivileged: https://github.com/T-head-Semi/thead-extension-spec/blob/master/xtheadvector... I assume Charlie tried it but was trapping, as v1 had a comment:
* Although xtheadvector states that th.vlenb exists and* overlaps with the vector 1.0 extension overlaps, an illegal* instruction is raised if read. These systems all currently* have a fixed vector length of 128, so hardcode that value.On my board with a c906 attempting to read th.vlenb (which is supposed to have the same encoding as vlenb) raises an illegal instruction exception from S-mode even though the documentation states that it shouldn't. Because the documentation states that vlenb is available, I haven't made it required for xtheadvector, I am not sure the proper solution for that.
Would you mind raising an issue on the T-Head extension spec repo about this?
Thanks, Conor.