Add zero-init-ram feature

[inorick]

Add the 'zero-init-ram' feature that initializes the RAM with zeros during startup. This is normally
not necessary but might be required on custom hardware. If this step is skipped on such hardware,
reading from memory that was never written to will cause a hard-fault.

[adamgreig]

Do you have any examples of hardware that requires this? It seems very unusual to need to initialise the stack as Rust already doesn't assume it's been zero-initialised; usually I've only seen this in the context of stack painting to measure a high water mark, but that's perhaps better left to debugging tools rather than the init code.

[inorick]

Do you have any examples of hardware that requires this?

It is a piece of automotive hardware. The fault is caused by an integrity protection feature that uses checksums for every block of 8 bytes. Writing a single byte causes a read-modify-write of the block that contains that byte. If the block was never written before, the integrity verification during the read step will fail as the checksum for that block was never generated. This will cause the whole read-modify-write to fail.

I also tried to initialize the RAM using the pre-init mechanism, but returning from pre-init is not possible once the RAM is wiped. It might be possible if pre-init was naked and inlined but those attributes are not allowed for pre-init.

[thejpster]

To me, it feels like ram_start and ram_end are backwards. I think of most regions as proceeding "up" through memory (lower number is start, higher number is end). The stack is the specific exception because it grows down on Arm.

[inorick]

To me, it feels like ram_start and ram_end are backwards.

Ah, yes. It only worked because I also swapped the arguments in the assembly routine. I corrected the order.

[thejpster]

Would this work as a custom __pre_init hook in your application? Or does it need to be in cortex-m-rt

D'oh, this is covered above.

[adamgreig]

I think maybe this could work as a pre-init hook if you wrote it in global asm instead? Like, in your main.rs,

global_asm!("
  .cfi_sections .debug_frame
  .section .__pre_init, \"ax\"
  .global __pre_init
  .type __pre_init,%function
  .thumb_func
  .cfi_startproc
  __pre_init:
  ldr r0, =_ram_start
  ldr r1, =_ram_end
  movs r2, #0
  0:
  cmp r1, r0
  beq 1f
  stm r0!, {{r2}}
  b 0b
  .cfi_endproc
  .size __pre_init, . - __pre_init
");

Still it's a bit unwieldy and requires adding the ram_start/ram_end symbols to your own memory.x too. Not terrible if we document it, and it does seem like a very niche requirement, but it's also not especially difficult to include in cortex-m-rt itself. The overhead of making sure it's tested, maintained, and documented is more annoying than the actual code.

[inorick]

Thanks, I would also prefer a pre_init solution. I tried adding the global asm __pre_init hook from above, both to my main.rs and the lib.rs of the cortex-m-rt crate, but only got:

cargo clean; cargo build
[...]
   Compiling cortex-m-rt-macros v0.7.0
error: Undefined temporary symbol .Ltmp1

The _ram_start and _ram_end symbols are still defined in the linker script. Do I have to add anything else to get it to work?

[adamgreig]

Hmm, I hadn't tested it but just tried on another project and it does at least build without errors. Could the error be from anything else?

[inorick]

Found the problem. The hook was missing the 1 label and a branch back to lr. Here is the fixed version:

global_asm!(
    ".cfi_sections .debug_frame
     .section .__pre_init, \"ax\"
     .global __pre_init
     .type __pre_init,%function
     .thumb_func
     .cfi_startproc
     __pre_init:
     ldr r0, =_ram_start
     ldr r1, =_ram_end
     movs r2, #0
     0:
     cmp r1, r0
     beq 1f
     stm r0!, {{r2}}
     b 0b
     1:
     bx lr
     .cfi_endproc
     .size __pre_init, . - __pre_init"
);

This means that together with the label definitions in memory.x no changes to the crate are necessary.

Thanks for the help. I think this PR can be closed.

[inorick]

I can imagine that other ASIL D compliant automotive ECUs have similar RAM protection mechanisms. So maybe this feature is not as niche as it appears to be.

[adamgreig]

We discussed this feature in this week's meeting and decided it does make sense to include it in cortex-m-rt.

However, before we can merge this the new feature needs to be described in the documentation (in lib.rs, next to set-sp and set-vtor), tested in CI (in ci/script.sh), and mentioned in the CHANGELOG. Would you be able to update those files in this PR?

[inorick]

I am glad to hear it. I'll update the PR but it may take a few weeks until I have the time to do so.

[inorick]

I made the requested changes. Please let me know what you think.

[adamgreig]

Thanks for the updates, looks good.

One concern I have now is that the initialising is done before calling the pre_init hook. Sometimes this is good (if pre_init accesses RAM you need it to have been zeroed already), but sometimes bad (if you need to use pre_init to configure/disable a watchdog that would time out in the time it takes to zero all the ram, or you want to bump clock speeds so it takes less time to zero the ram, or you need to enable power to the ram before it can be written to, etc). But having two pre_init hooks seems messy, and we're probably down into edge-case-inside-edge-case territory too. I'm not sure what the best ordering is exactly, any thoughts?

[inorick]

One concern I have now is that the initialising is done before calling the pre_init hook. Sometimes this is good (if pre_init accesses RAM you need it to have been zeroed already), but sometimes bad (if you need to use pre_init to configure/disable a watchdog that would time out in the time it takes to zero all the ram, or you want to bump clock speeds so it takes less time to zero the ram, or you need to enable power to the ram before it can be written to, etc). But having two pre_init hooks seems messy, and we're probably down into edge-case-inside-edge-case territory too. I'm not sure what the best ordering is exactly, any thoughts?

True, both orders of pre-init and zero-init-ram can make sense. I do not see a generic way to make this order configurable other than splitting the zero-init-ram into two which is probably overkill. Maybe we should wait until someone actually needs that?

[adamgreig]

Thanks again for this feature! I've rebased onto master and moved the zero-init-ram routine to just before bss, so that it occurs after pre_init, because many of the use cases for pre_init don't work after writing all of RAM, whereas any RAM that must be accessed during pre_init can conceivably be written to by the user.

Possibly we could add a late_pre_init that runs after memory initialisation (mainly as a pre-main hook for HALs etc) as an optional feature, and maybe even more the normal pre_init to an optional-feature early_pre_init in the next breaking release.

bors r+

[bors]

Build succeeded:

• ci-linux (1.59.0)
• ci-linux (stable)
• clippy
• hil-compile-rtt
• hil-qemu
• rt-ci-linux (1.59.0)
• rt-ci-linux (stable)
• rt-ci-other-os (macOS-latest)
• rt-ci-other-os (windows-latest)
• rustfmt