Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
ConchuOD opened this issue Mar 2, 2023 · 5 comments
Open

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

ConchuOD opened this issue Mar 2, 2023 · 5 comments
Assignees
Labels
bug Something isn't working

Comments

@ConchuOD
Copy link

ConchuOD commented Mar 2, 2023

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
Copy link
Author

ConchuOD commented Mar 2, 2023

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
Copy link
Author

@gchatelet
Copy link
Collaborator

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.

@gchatelet gchatelet self-assigned this Apr 25, 2023
@gchatelet gchatelet added the bug Something isn't working label Apr 25, 2023
@ConchuOD
Copy link
Author

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 😂

@ConchuOD
Copy link
Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants