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

riscv: All the CSR write operations should be unsafe by default #209

Open
jsgf opened this issue May 14, 2024 · 3 comments · May be fixed by #251
Open

riscv: All the CSR write operations should be unsafe by default #209

jsgf opened this issue May 14, 2024 · 3 comments · May be fixed by #251
Milestone

Comments

@jsgf
Copy link
Contributor

jsgf commented May 14, 2024

In general we should assume that writing to CSRs could do something that potentially violates the Rust abstract model.

Macros like write_csr_as and write_csr_as_usize should at least default to unsafe, and maybe have an option to make a safe variant on a CSR by CSR basis.

@romancardenas romancardenas changed the title All the CSR write operations should be unsafe by default riscv: All the CSR write operations should be unsafe by default May 14, 2024
@romancardenas
Copy link
Contributor

romancardenas commented May 14, 2024

As I always do, I checked the cortex-m crate and... they use the same fashion you propose 😁

In other words, I am in favor of your proposal.

@alistair23
Copy link

I agree. Writing to RISC-V CSRs can cause all sorts of unsafe behaviour. There are a few that probably aren't unsafe but they need some reasoning about why they aren't. unsafe by default is the way to go

@romancardenas
Copy link
Contributor

We commented this issue in yesterday's meeting and the general impression is that we should:

  • By default, make all writes unsafe for every CSR
  • Then go CSR by CSR analyzing their implications and, if it applies, make particular cases safe (when it is not harmful).

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

Successfully merging a pull request may close this issue.

3 participants