rust-embedded / cortex-m-rt

Minimal startup / runtime for Cortex-M microcontrollers
https://rust-embedded.github.io/cortex-m-rt/
Apache License 2.0
358 stars 85 forks source link

[feature request] possibility to include asm stub before __pre_init for MCU that starts with RAM disabled #303

Closed allexoll closed 3 years ago

allexoll commented 3 years ago

Some processors start without RAM enabled. That's the case for the NXP LPC546XX family. Their solution is to add inline assembly to enable the ram on startup:

    // Enable SRAM clock used by Stack
    __asm volatile ("LDR R0, =0x40000220\n\t"
                    "MOV R1, #56\n\t"
                    "STR R1, [R0]");

I tried to do the same in the pre_init stub:

#[inline(always)]
#[pre_init]
unsafe fn pre_init() {
    asm!("ldr r0, =0x40000220");
    asm!("mov r1, #56");
    asm!("str r1, [r0]");
}

Using the __pre_init decorator does not allow for this kind of run-time setup since a push instruction is/might be generated inside the __pre_init stub, accessing the stack when RAM is not available. #[inline(always)] does not inline the function in the reset stub:

00000180 <__pre_init>:
 180:   b580        push    {r7, lr}  <- this cannot happen at this point in execution since sp points to a non-enable address space
 182:   466f        mov r7, sp
 184:   4803        ldr r0, [pc, #12]   ; (194 <__pre_init+0x14>)
 186:   e7ff        b.n 188 <__pre_init+0x8>
 188:   f04f 0138   mov.w   r1, #56 ; 0x38
 18c:   e7ff        b.n 18e <__pre_init+0xe>
 18e:   6001        str r1, [r0, #0]
 190:   e7ff        b.n 192 <__pre_init+0x12>
 192:   bd80        pop {r7, pc}
 194:   40000220    andmi   r0, r0, r0, lsr #4

Maybe there can be a possibility to include an asm stub, similarely to what is done with the memory.x file

Sympatron commented 3 years ago

If it's that critical I would probably do it in assembly in a separate .s file. That way you have full control over what's generated.

allexoll commented 3 years ago

I found a way to go around this limitation, but i'm certain it is not ideal:

both the PAC and app crate must be linked with a local folder containint the cortex-m-rt repository.

Then you need to modify cortex-m-rt/src/lib.rs to include you asm code (before the call to pre_init):

    extern "Rust" {
        // This symbol will be provided by the user via `#[pre_init]`
        fn __pre_init();
    }
    asm!("ldr r0, =0x40000220");  // load SYSCON.ahbclkctrl0 addr
    asm!("mov r1, #56");         // SRAM1 | SRAM2 | SRAM3
    asm!("str r1, [r0]");     // write 
    __pre_init();

    // Initialize RAM
    r0::zero_bss(&mut __sbss, &mut __ebss);
    r0::init_data(&mut __sdata, &mut __edata, &__sidata);
...

you then need to recompile the bins used by cortex-m-rt usign the assemble.sh script in cortex-m-rt

then build your app, and you can see the code is now in the right place without stack use:

00000124 <Reset>:
 124:   b580        push    {r7, lr}
 126:   466f        mov r7, sp
 128:   4813        ldr r0, [pc, #76]   ; (178 <_stext+0x54>)
 12a:   e7ff        b.n 12c <Reset+0x8>
 12c:   f04f 0138   mov.w   r1, #56 ; 0x38
 130:   e7ff        b.n 132 <Reset+0xe>
 132:   6001        str r1, [r0, #0]
 134:   e7ff        b.n 136 <Reset+0x12>
 136:   f000 f9f1   bl  51c <DefaultPreInit>

I'm sure there is a better way to integrate this kind of run-time config with cortex-m-rt, but for now that does the trick. but now that means i cannot publish a PAC for this family since it depends on a local modified rt crate.

I think it make sense to add this support with the PAC rather than the app, since it is chip-dependant, but i'm open to comments

jonas-schievink commented 3 years ago

It is UB to run Rust code without a stack in place. You need to wait for #301 to land and then provide an assembly stub that enables RAM in __pre_init.

adamgreig commented 3 years ago

I haven't tried this, but if you write your own assembly routine and export it with the name __pre_init, it should be called by cortex-m-rt without needing a custom cortex-m-rt. You can then enable the SRAM without using it in your own pre_init assembly blob, which I guess you'd need to write as an external .s file and build and link in. It would prevent end users from doing their own pre_init, though (you'd get conflicting symbols at link time). Technically until #301 lands this is still UB since the reset vector is in Rust still, but the very first thing the reset vector does is jump to `pre_init`, so I wouldn't be surprised if in practice this worked fine until #301 is released, at which point it should be totally legitimate.

Is there any good reason these chips do this, or are they just trying to be annoying? Why not have the SRAM clock enabled at reset? In your example at the top, it looks like your asm is inline inside a C function - presumably the reset handler - which means your C function is also running before memory is available, rather than having a reset vector written entirely in assembly.

It's possible being able to use #[naked] on functions would help here too I guess...

allexoll commented 3 years ago

Your suggestion is what i was setting up, and it works.

I made a startup.s file:

.globl __pre_init
__pre_init:
        ldr r0, =0x40000220
        mov r1, #56
        str r1, [r0]
        bx lr

that way cortex-m-rt does not need to be messed with on the user part. that code is build using a similar assemble.sh and build.rs script limited for M4/M4F.

I think that kind of needed-for-rt code should be included in the pac, but if anyone think otherwise, please comment.

So, still UB until #301 but it does work since no stack is used before ram init.

I have no idea why NXP decided to put the RAM disabled on boot, but the mind of hardware designers sometime works in mysterious ways. Fortunately we have found a way around that limitation.

If i still cant find any reason not to include this tweak in the pac, i'll do a PR over at lps-rs/lpc-pac for the complete LPC546XX family and document the tweak.

Thanks for your comments