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

Tracking/discussion issue for MMIO references #791

Open
adamgreig opened this issue Aug 27, 2024 · 2 comments
Open

Tracking/discussion issue for MMIO references #791

adamgreig opened this issue Aug 27, 2024 · 2 comments

Comments

@adamgreig
Copy link
Member

adamgreig commented Aug 27, 2024

It's been known since 2019 that having references to memory-mapped IO (for example the memory-mapped structs containing register fields used by svd2rust) could lead to UB because the compiler is generally free to insert an extra read from such references at any time.

#387 was opened to propose a change to svd2rust's API to avoid memory-mapped IO, but it wasn't merged and isn't likely to be after all this time. However, the issue thread has extensive discussion and detail about the problem and is worth a read. In the meantime it's become clear that this isn't considered a bug in the Rust compiler, and MMIO should be accessed only using raw pointers. It's possible some ergonomic improvements will land to using raw pointers, but in the meantime crates like chiptool show you can still have convenient and efficient APIs to register access without using memory-mapped structures.

I've closed #387 and opened this issue as a signpost back to #387 and a place to continue discussion on the topic.

This topic was also discussed at RustNL; the consensus there was to move svd2rust to a similar API as chiptool, avoiding the references to MMIO. Because this is not only a breaking syntax change but also requires changing the owned singleton concept, and since svd2rust generates PACs that are used by HALs that are eventually used by end users, it's a slow change that's taking a while to work through.

@RalfJung
Copy link

In the meantime it's become clear that this isn't considered a bug in the Rust compiler, and MMIO should be accessed only using raw pointers.

To be clear, a discussion can be had about changing this part of Rust. But it has to go via the usual process for changing the language, and it'll only happen if someone is motivated and able to go through the usual long-winded process for such changes. This issue here is probably not the right place to discuss such language changes (as I understand it, this issue is about how to make the existing code work with the existing language); here are some discussions around language improvements to volatile:

@kellda
Copy link

kellda commented Aug 28, 2024

Some context and thoughts on this issue:

  • AFAIK, SVD uses structures mapped to memory because that's how it's done in C, where it's an easy way to do MMIO. In Rust, however, one must call volatile_{read,read} for volatile accesses, so the advantages of this approach are unclear.

  • References to ZSTs are fine (correct me if I'm wrong), allowing to keep an API very similar to the current one, though not 1:1 compatible. In this alternative API, the Reg struct would be zero-sized, with methods obtaining a base address from their &self parameter, and an offset from a generic parameter (either through an associated constant on the RegisterSpec trait or via an additional const generic). RegisterBlock structs would continue to work as they do, but would be zero-sized since they would contain only ZSTs.

  • Peripherals being ZSTs is usually seen as a must-have for zero-cost abstraction and performance. Upon inspection of generated assembly, it turns out that it is often more efficient to pass peripherals as pointers to register blocks instead of ZST structs. This is because (on ARM, and likely most others architectures) load and store instruction don't accept immediate addresses, meaning that the addresses must first be loaded into register. It is thus more efficient to receive addresses as arguments than to load them in each function. One notable exception is "device structs" storing every peripheral in the device, with most of them likely never used.

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

3 participants