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

Use unified spec describe CSR registers #1809

Open
trdthg opened this issue Jan 16, 2025 · 3 comments
Open

Use unified spec describe CSR registers #1809

trdthg opened this issue Jan 16, 2025 · 3 comments

Comments

@trdthg
Copy link
Contributor

trdthg commented Jan 16, 2025

I found the description of jvt CSR in the manual to be very organized and clear, following the format below

Synopsis:
Address:
Permissions:
Format (RV32):
Format (RV64):
Description:
	Table 43. jvt.mode definition
Architectural State:
State Enable:

However, other old CSRs didn't follow such a format,This may be one of the reasons that causes confusion among users.

Perhaps we should try gradually to have all CSRs follow this format as much as possible?

@radimkrcmar
Copy link
Contributor

Image

(I'd prefer base[31:6]/26 and base[63:6]/58 when the registers are already split by XLEN and having equal display bit width for both registers, so the mode doesn't look different)

I think that having just one register layout definition with variable length fields is much better, because the reader doesn't have to compare all the other fields to see that they haven't changed.
RISC-V will also have registers that have fields sized on something other than XLEN and we definitely don't want to list all possible combinations. (Having the same thing for RV32/RV64/RV128 is already bad.)

wavedrom-bitfield currently doesn't support fields with variable length, so you have to use bytefield-svg and make it look inconsistent with other registers, but that's another issue.

Image

Instructions don't list their codes at instruction definition either, so the current CSR format is probably trying to do the same.
Having all relevant information near the definition seems better to me as well. (Links don't work very well on paper.)

Perhaps we should try gradually to have all CSRs follow this format as much as possible?

Having consistent definitions would be great. Probably not like jvt does it, though, and Sail is supposed to bring big changes to ISA generation, so I'm personally waiting for that before suggesting any improvements.

@trdthg
Copy link
Contributor Author

trdthg commented Jan 16, 2025

I think that having just one register layout definition with variable length fields is much better,

Agree, but mstatus has two variable-length fields, things might be a bit more complicated

Image

I'm personally waiting for that before suggesting any improvements.

makes sense, we should keep watching

@radimkrcmar
Copy link
Contributor

mstatus is indeed difficult to typeset as a single register, because it provides access to more than 32 bits of information through msatatush.
Position of SD bit is always the most significant bit in mstatus and some mstatush fields are WPRI but defined for RV64 and RV128.

Having RV32 separate from RV64+RV128 version seems most palatable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants