Fix interrupt::free.

[reitermarkus]

Don't pass CricialSection to closure passed to interrupt::free.

Depends on rust-embedded/critical-section#13.

[rust-highfive]

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @adamgreig (or someone else) soon.

Please see the contribution instructions for more information.

[adamgreig]

Thanks for the PR! We discussed this a bit in last week's meeting here; the trouble is basically that interrupt::free() doesn't meet the guarantees required by CriticalSection on multi-core systems or systems executing in unprivileged mode. Ultimately the solution is probably to move towards something like the critical-section crate (possibly integrating that into bare-metal). In the meantime for cortex-m 0.8, it might be best to actually have interrupt::free() not give the closure a CriticalSection token at all, and just execute the closure in the context of disabled interrupts. That's still useful, but it wouldn't permit safe access to a Mutex any longer, so we'd want something else in place before releasing 0.8.

[reitermarkus]

I guess in this case we need three things:

• interrupt::free which does not pass a bare_metal::CriticalSection section.
• Implementation of critical_section::CriticalSection for single and multi-core.
• Some function which does pass a bare_metal::CriticalSection to a closure so we can actually lock a bare_metal::Mutex.

[thejpster]

As far as I am aware, you can’t implement it for multi-core devices using only the standard Cortex-M peripherals. For example, on the Raspberry Silicon RP2040 you need to use the Spinlock registers in the SIO peripheral.

I also don’t think you could even know you were on a multi-core system using only the standard Cortex-M peripherals.

[reitermarkus]

The critical_section::CriticalSection implementation would have to be an opt-in feature anyways.

I think it makes sense to then have a HAL either provide this implementation or opt into the single-core version in this crate.

[thejpster]

I think the HALs should provide an implementation. I’m unsure whether it’s ok for them to re-export one from here. Perhaps if it’s behind a feature flag, or in a macro, it’s OK.

[reitermarkus]

I’m unsure whether it’s ok for them to re-export one from here.

Not sure what you mean by re-export here. The HAL (for a single-core chip) will then simply activate the cortex-m/singlecore feature.

[reitermarkus]

Some function which does pass a bare_metal::CriticalSection to a closure so we can actually lock a bare_metal::Mutex.

Ah, critical_section::CriticalSection is a re-export of bare_metal::CriticalSection, I though it would have to be the other way around. So critical_section::with already allows this.

[Dirbaio]

I've released critical-section 1.0.0-alpha.1. Unless concerns are found, I think it's likely this'll be the final 1.0 design.

[reitermarkus]

Fixed in #447.