ACLINT and CLINT peripherals

[romancardenas]

This PR provides a generic implementation of the ACLINT and CLINT peripherals.

As suggested by @Dirbaio, I'm using a completely different approach compared to the PLIC peripheral. Instead of using a register block and the volatile-register, I use raw pointers and read_volatile and write_volatile.

Personally, I like this new approach more, as it makes it more ergonomic to interface with the registers. The const generic things turned out to be a little bit limiting, especially when dealing with multi-HART architectures.

As a side note, I added to the peripheral::common module the WARL register type. In essence, it is just an RW register (i.e., it does not have anything special). However, it implies that only legal values will be written, and therefore you cannot assume that the register will hold any value.

Please take a look and provide me with all the feedback you can. If you prefer this way (as I do), I'll happily rework the PLIC peripheral and add it to this PR before merging it.

[romancardenas]

No const fn with generics on stable Rust 1.59 :( Any suggestion?

[romancardenas]

I checked and the new MRSV would be 1.61.0. I think it is rather an important upgrade... How bad would it be if we remove the const for creating new Reg structs?

[romancardenas]

I've been looking at the Peripherals struct of the PACs generated by svd2rust and they are not const, so it does not make sense to keep these peripheral things const and bump the MSRV.

[romancardenas]

I did a rework of the peripherals with all the comments @Dirbaio gave me:

• Peripheral registers are in essence raw pointers that do volatile reads/writes.
• The new(...) functions of the registers are unsafe, as we need to make sure that the pointer is valid.
• The read() and write(...) methods are volatile/atomic-ish, so they are now safe.
• The modify(...) method does a read-modify-write operation. The docs now say that this function may result in wrong behavior, but it is safe. Essentially, we are not directly dealing with memory addresses, and thus we are not breaking the Rust memory model (if there is such a thing).
• We avoid using the volatile-register crate.
• The ACLINT, CLINT, and PLIC peripherals now work in this fashion (I still need to test them on my boards).

There is still something "funny" that deserves a discussion:

• Reg::write(...) is safe because it is an atomic operation with no side effects
• However, writing on a register that deals with interrupt priorities is usually marked as unsafe, because you can break priority mask-based critical sections.
• Currently, the PRIORITY register of the PLIC has a safe write(...) method that allows you to write any value and an unsafe set_priority<P: PriorityNumber>(...) method that forces you to i) provide a valid priority level and ii) add an unsafe block because you are changing priorities.

Personally, I think this approach is conceptually sound: it is a volatile register so you can safely write on it any bit stream with no meaning but 1s and 0s. But if your intention is to set_priority of an interrupt source (i.e., you are providing more context), then you must consider additional issues such as breaking critical sections. Somehow, both approaches live together currently, which is quite flexible. However, these things deserve further discussion to come up with a "formal"/"consistent" design pattern and document it.

[romancardenas]

I modified the peripheral macros to address the aforementioned issue more elegantly:

• Now, the inner "raw register" of peripheral registers is private.
• The safe_peripheral! macro assumes that it is OK to read/write ANY value to the register. Thus, the resulting structure implements the Deref trait to the inner "raw register".
• The unsafe_peripheral! macro is for those peripheral registers that need extra care when reading/writing. These do not implement the Deref trait. Instead, they assume that we will provide additional methods to interact with the peripheral correctly. Nevertheless, this macro also adds the unsafe fn get_register(self) function so we can still access the inner raw register at our own risk.

This new approach should be OK. I'll test the peripherals with my E310x board before asking the @rust-embedded/riscv team to review and merge them to master.

[romancardenas]

Closing this now. I'm developing RISC-V peripherals in the riscv-peripheral crate. My plan is, once we have a decent version of this crate, to nominate it to leave the RISC-V team to maintain it. Eventually, we can add all the peripherals to riscv again once we are happy with the result.