Update cortex-m-rt, cortex-m, PACs; fix things #2260
Draft
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Our pre-init routine contains things that need to happen before anything touches RAM. This is not reliably possible from a Rust function, since the compiler may elect to set up a stack frame (and, in our case, definitely does so today). This commit moves it into straight assembly language, ensuring that we have control over the use of RAM. As a pleasant side effect, this also makes it really hard to accidentally put Rust code before it. This is not sufficient, by itself, to eliminate use of RAM early in boot, since the ancient version of cortex-m-rt that we're using has a bug here. But I'll tackle that in a follow-on commit.
I noticed that sprot-api depended on spi-api, which is otherwise a very STM32-specific crate (for better or worse). This turns out to be only due to the SprotError::Spi variant... which is never constructed, anywhere. So I removed it. This unblocks the upgrade to stm32h7 0.15.
cbiffle
force-pushed
the
cbiffle/cortex-m-update
branch
from
c48d046 to
cbfb8fa
Compare
Hiffy was unconditionally pulling in the drv-spi-api crate, which meant it needed to build on all platforms, even those for which we don't have SPI support. This is complicated by its build script, which requires SPI config.
This build script was throwing away any failure to parse global config, which means that e.g. misspelling "DIV256" would instead get you unrelated compile errors in apparently unrelated crates, since code generation would just switch off. This appears to have been done because the lpc55 code was depending on this crate, and it of course doesn't _have_ an stm32h7 global SPI config stanza in its config TOML. I broke that dependency in an earlier commit, so now I'm making the code generation mandatory with error reporting.
I noticed that our cortex-m-rt version was generating potentially invalid code in the Reset function -- setting up and tearing down a stack frame, before we've turned on RAM! This bug got fixed in the upstream cortex-m-rt crate, but we haven't upgraded in ... quite a while. This upgrades to a compromise point -- 0.15 of the stm32 pacs. Upgrading farther than that causes massive API incompatibilities, since it looks like they've comprehensively changed the svd2rust generated code. Rather than touch literally every driver in the system, I've settled on 0.15 and left 0.16 as a trap for the future. As you can see, this still required a fair amount of churn, mostly due to renamed enum variants.
cbiffle
force-pushed
the
cbiffle/cortex-m-update
branch
from
cbfb8fa to
1d4e7f8
Compare
aapoalas
reviewed
Oct 1, 2025
| " movw r0, :lower16:{RCC_addr}", | ||
| " movt r0, :upper16:{RCC_addr}", | ||
| " ldr r1, [r0, #{RCC_AHB2ENR_offset}]", | ||
| " orrs r1, #((1 << {RCC_AHB2ENR_SRAM1EN_bit}) \ |
There was a problem hiding this comment.
question: Is orr available in Thumb ISA? Since you don't care about the condition flags here and below. I couldn't find a definite answer from ARMs documentation (the usually excellent docs seem to have a bit of a blindspot to instruction listing on Thumb? Or they seem to suggest that rather ORRS is not a thing?). Not that it really matters any.
| ".cfi_endproc", | ||
| ".size __pre_init, . - __pre_init", | ||
|
|
||
| PWR_addr = const 0x5802_4800, //device::PWR::ptr(), |
There was a problem hiding this comment.
note: Support for const pointers seems to be pretty close to working on nightly: rust-lang/rust#138618
| let name = name.to_uppercase(); | ||
| writeln!(&mut file, " pub const {name}: u8 = {i};")?; | ||
| } | ||
| let global_config = build_util::config::<SpiGlobalConfig>()?; |
There was a problem hiding this comment.
praise: Wow, this would've probably been a really confusing bug: build succeeds but devices are somehow missing from the output?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This started as a followup to #2259, trying to ensure that no startup code used RAM before RAM was all the way turned on. It has ... grown, since then.
The scope so far turns out to be:
pre_initroutine in assembler for more precise control over the use of stack.