google / cpu_features

A cross platform C99 library to get cpu features at runtime.
Apache License 2.0
2.46k stars 262 forks source link

RISC-V devicetree "riscv,isa" comment is no longer accurate #301

Open ConchuOD opened 1 year ago

ConchuOD commented 1 year ago

The comment about RISC-V devicetree extension order is not aligned with the URL in the comment.

This is mea culpa, because I initially got it wrong & came back and changed it. In your comment, the order is ^rv(?:64|32)imaf?d?q?c?b?v?k?h?(?:_[hsxz](?:[a-z])+)*$ The pattern was changed to ^rv(?:64|32)imaf?d?q?c?b?k?j?p?v?h?(?:[hsxz](?:[a-z])+)?(?:_[hsxz](?:[a-z])+)*$ which is more complicated than it was before, because the first multiletter extension doesn't have to have the leading _, sigh.

Per my commit message, v (vector) was also resorted slightly and p (packed-simd) & j (dynamic languages) were added. I don't think the re-order has any impact for you as you don't look for any of v, k, h or p - but allowing people to omit the leading may impact your acquisition of zicsr and zifencei.

On that note though, "riscv,isa" can't be used to tell whether zicsr/zifencei are present in the cpu. Unfortunately, as they used to be part of i, there's no way of telling if a devicetree containing "i" also contains zicsr/zifencei. Sure, if they're in the "riscv,isa" string, then they are there but if they're not in the string they may still be in the hardware :)

ConchuOD commented 1 year ago

Hmm, in addition to that you're talking about the riscv,isa devicetree string, but that's not actually what get's put into /proc/cpuinfo - only the bits of it that the kernel actually supports are put in, and in an order that the kernel, rather than devicetree, decides. Perhaps this is more of the sort of thing you should be using as reference: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/riscv/uabi.rst

IIRC, we never put zicsr or zifencei into /proc/cpuinfo.

ConchuOD commented 1 year ago
gchatelet commented 1 year ago

Sorry for the lag here, I'll look into it. There is also this link on wikichip https://en.wikichip.org/wiki/risc-v/standard_extensions. I'll check whether the different documentations agree and report back.

ConchuOD commented 1 year ago

Sorry for the lag here, I'll look into it.

No worries chief.

There is also this link on wikichip https://en.wikichip.org/wiki/risc-v/standard_extensions. I'll check whether the different documentations agree and report back.

FWIW, whether they agree or not doesn't really matter at this point, since it is in that uabi doc it is set in stone.

The order does at least match some version of the RISC-V ISA spec though :joy:

ConchuOD commented 1 year ago

IIRC, we never put zicsr or zifencei into /proc/cpuinfo.

Since I wrote this issue, this has changed. We will likely start showing Zicsr and Zifencei in /proc/cpuinfo from 6.5 onwards.

Similarly, there is a new syscall added for RISC-V that can be used to get cpufeatures without parsing /proc/cpuinfo. Perhaps some folk care about it: https://docs.kernel.org/riscv/hwprobe.html