-
Notifications
You must be signed in to change notification settings - Fork 861
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
feat: add possibility for custom CSRs #1853
Conversation
@aswaterman @jerryz123, could you please take a look |
This change looks good. @arrv-sc are you able to identify what might be causing the regression failure? It isn't obvious to me. |
@jerryz123 I added new target to create-ci-binary-tarball script. Testing script fetches tar archive with binaries compiled for riscv and then executes them. Pipeline fails because archive does not contain executable compiled from my new customcsr.c file. As far as I know this tarball has to be updated manually and I cannot do that. |
Ah right, I'll upload the updated tar soon. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, too.
Currently in riscv-isa-sim there's no way to make a custom extension that adds new CSRs. This simple patch makes it possible via new virtual function in extension_t class.
@aswaterman there is also a question of whether or not we should check for overwriting standard CSRs with custom ones that use the same addresses. In this commit I didn't implemented this check as it turned out to require some refactoring and I didn't want to make this change too big. Should we implement it after all? |
Non-conforming extensions can use standard CSR addresses, but only if the address is not used by a standard extension that the implementation claims to support. For example, the address of IMO, extending Spike is a feature for experts, so the error checking would be nice to have, but is not absolutely necessary. If the refactoring is very messy, I would be OK with skipping it. |
Currently in riscv-isa-sim there's no way to make a custom extension that adds new CSRs. This simple patch makes it possible via new virtual function in extension_t class.