riscv: redefine CSR registers with new helper macros

[rmsyn]

Existing CSR registers could be redefined using the new helper macros to gain extra functionality, and consistent definitions across the crate.

Part of the 0.13.0 milestone.

Registers to redefine:

• marchid (*)
• mcause
• mcounteren
• medeleg
• mideleg
• mie
• mimpid (*)
• mip
• misa (*)
• mstatus
• mstatush
• mtvec
• mvendorid (*)
• pmpcfgx
• satp
• scause
• scounteren
• sie
• sip
• sstatus
• stvec

(*) Currently uses NonZeroUsize

• use CSR macros arm with sentinel value for checking implementation status

Remaining registers are all simple value registers with no bitfields.

[romancardenas]

I would like to do a release soon with all the new changes so people can start playing with it and detect any potential issues. We can leave these changes for the next release (hopefully before 2025). In fact, what do you think about nominating issues and working on them for the next milestone?

[rmsyn]

In fact, what do you think about nominating issues and working on them for the next milestone?

That sounds great! Would you like me to create an issue for each CSR register, just to keep things modular?

Alternatively, I could add checkboxes to this issue with each CSR, and check them off as implementations are completed.

[romancardenas]

I nominated a few issues to be part of the riscv 0.13.0 milestone.

In each issue, we should write a checkbox list to keep track of advances. We can also open a new issue (e.g., riscv 0.13.0), and each of us can assign one issue to work on.

Another option is using GitHub projects. It is a kind of Trello but strongly coupled to the repo.

Let me know what you prefer!

[rmsyn]

In each issue, we should write a checkbox list to keep track of advances.

This works for me. I'll go through the crate, and add a checkbox for each CSR.

[romancardenas]

Currently, when compiling riscv for RV32 targets, I get the following warnings:

warning: unused variable: `uxl`
   --> riscv/src/register/mstatus.rs:418:31
    |
418 |     pub fn set_uxl(&mut self, uxl: XLEN) {
    |                               ^^^ help: if this is intentional, prefix it with an underscore: `_uxl`
    |
    = note: `#[warn(unused_variables)]` on by default

warning: unused variable: `sxl`
   --> riscv/src/register/mstatus.rs:443:31
    |
443 |     pub fn set_sxl(&mut self, sxl: XLEN) {
    |                               ^^^ help: if this is intentional, prefix it with an underscore: `_sxl`

warning: unused variable: `endianness`
   --> riscv/src/register/mstatus.rs:468:31
    |
468 |     pub fn set_sbe(&mut self, endianness: Endianness) {
    |                               ^^^^^^^^^^ help: if this is intentional, prefix it with an underscore: `_endianness`

warning: unused variable: `endianness`
   --> riscv/src/register/mstatus.rs:492:31
    |
492 |     pub fn set_mbe(&mut self, endianness: Endianness) {
    |                               ^^^^^^^^^^ help: if this is intentional, prefix it with an underscore: `_endianness`

warning: `riscv` (lib) generated 4 warnings
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.65s

I think all these fields, in their try_ variant, should return an Unimplemented error, as they are only included in RV64 devices

[rmsyn]

I think all these fields, in their try_ variant, should return an Unimplemented error, as they are only included in RV64 devices

I agree, thanks for bringing this to my attention. When I get to the mstatus register, I'll make sure to handle this edge case.

We can either extend the helper macros, or use conditional compilation like:

#[cfg(target_arch = "riscv64")]
read_write_csr_field! {
    Mstatus,
    /// MBE docs
    mbe: 37,
}

impl Mstatus {
    #[cfg(target_arch = "riscv32")]
    pub fn set_mbe(&mut self, _endianness: Endianess) {
        unimplemented!("only available on RV64");
    }

    // ...
}

[romancardenas]

I would go for something more like:

impl Mstatus {
    #[cfg_attr(not(target_arch = "riscv64"), allow(unused_variables))]
    pub fn try_set_mbe(&mut self, endianness: Endianess) → Result<()> {
        match () {
            #[cfg(target_arch = "riscv64")]
            _ => {...},
            #[cfg(not(target_arch = "riscv64"))]
            _ => Err(Error::Unimplemented),
        }
    }
    pub fn set_mbe(&mut self, endianness: Endianess) { try_set_mbe(endianness).unwrap(); }
}

I wouldn't bother to add this to macros, I think it is a special case that should be handled by a manual implementation.