Clearing CPUOFF at end of interrupt

[vadixidav]

I currently have a branch of embassy that I am using successfully for an MSP430i2041-based project over here: https://github.com/vadixidav/embassy/tree/msp430. This issue is related to the note I left in this commit: vadixidav/embassy@2a43df7.

Essentially, I would like for embassy to put the CPU to sleep when not in use. This would normally mean executing the following:

asm!("bis #16, R2", options(nomem, nostack, preserves_flags));

However, I am unable to do this because, at the end of my interrupts, the compiler generated several pop instructions that pop registers from the stack. This is a perfectly reasonable and valid thing for it to do. The problem is that this prevents me from using the bic instruction to clear the CPUOFF bit in the SR before the generated reti instruction. What I really want to do is perform a bic instruction immediately before the reti instruction at the end of the interrupt. Then I could guarantee that the SR is on the top of the stack and clear the CPUOFF bit. The only way I can see to achieve this (though my imagination only goes so far) without making too significant of changes to Rust's msp430 support would be to ensure that the outermost stack frame doesn't need to push any registers so bic #16, @R1 will do what I want.

This is where msp430-rt comes in. I believe that the interrupt macro (#[interrupt]) should automatically do the following:

• Wrap the interrupt specified by the user in an inner function
• Call this inner function using a call instruction (this forces the inner function to clean up its stack before returning)
• Add the line asm!("bic #16, @R1"); after the call instruction

This would unconditionally wake the CPU back up after an interrupt.

I don't think this is perfectly ideal, but I believe that it would be better to unconditionally assume the CPU should be waken up after an interrupt than to wait until there is a proper solution that allows the user to specify if they would like to wake the CPU back up or not. Later we could add an inner inner function that returns a bool indicating if it should wake up or not and then set a static variable.

This very basic functionality I am suggesting could be added as a feature flag like wake-after-interrupt for people who are using msp430-rt with embassy. Any alternatives to solve this problem are welcome. The MSP430 toolchain has a built-in that allows you to set flags in the SP for the function that was interrupted that can only be executed in the outermost stack frame of the interrupt, but I don't think we can reasonably implement this without adding special features to Rust that probably aren't worth the effort at this stage.

[cr1901]

The Low Power Modes are something I'm not very familiar w/ in terms of LLVM and Rust. I vaguely remember talking about implementing it a few years ago and concluding that the TI-provided LPM builtins cannot be implemented in terms of inline-assembly, precisely because of the stack pointer shenanigans you mention. I.e. it requires compiler support to know where the stack pointer to be modified is at the point you want to exit LPM, and LLVM doesn't have this support.

Your idea seems like a viable workaround that I never considered :). I would have to think about this tho, and my bandwidth is stretched thin as it is right now. But I'm not against the idea in principle as long as it's a default-off feature.

I'm worried about this line in the inline assembly rules:

You cannot assume that two asm! blocks adjacent in source code, even without any other code between them, will end up in successive addresses in the binary without any other instructions between them.

cc: @Amanieu Does this also mean "You cannot assume that an asm! block and a function epilog will be at successive addresses"? It's not technically two adjacent asm! blocks, but... inline asm is tricky. If so, then this would have to be an asm-only naked function (which is fine, embassy and msp430 already require nightly).

Cool that you're doing an MSP430 port of Embassy... saves me the trouble :P.

[Amanieu]

Using a naked function is the only way to make this work. Note that nightly isn't required, you can use global_asm! or the naked-function crate.

[pftbest]

@vadixidav

Using a naked function will incur significant overhead for simple handlers. You will have to push all the caller saved registers on stack before invoking the inner handler. (On Cortex-M the hardware does it automatically, but not on MSP430). That's why MSP430 uses the "msp430-interrupt" calling convention instead of naked functions to avoid this.

In C++ land on GCC, people use built-in function __bis_SR_register_on_exit to access the saved SR.

On LLVM, we can use the @llvm.frameaddress intrinsic to do the same thing. Here is an example code how to use it in Rust:

#![feature(link_llvm_intrinsics)]

extern {
    #[link_name="llvm.frameaddress"]
    fn frameaddress(frame: i32) -> * mut i16;
}

pub unsafe extern "msp430-interrupt" fn interrupt_handler() {
    // Do stuff
    // ...
    unsafe {
        *frameaddress(0).offset(1) |= 16;
    }
}

It should be easy to modify msp430-rt macros to emit this sequence if needed.

[cr1901]

You will have to push all the caller saved registers on stack before invoking the inner handler.

My idea was to make both the outer function and inner function use msp430-interrupt ABI. Since all registers are callee-saved w/ that ABI, this makes it the inner function's problem to save/restore registers.

That said, looks like frame_address might work. Tho I'm not exactly keen on adding a dependency on a new feature that will (rightfully so!) never be stabilized. I'm trying to make stable msp430 ergonomic and feasible (currently blocked on at least rust-lang/rust#38487). Contrast to naked which is likely to eventually be stabilized. Also, the msp430 ABI will be stabilized at some point (if in a different form from today).

[pftbest]

@cr1901 Ah, I see. That would be a good workaround.

Just calling them directly doesn't work but with inline assembly and naked function it's possible.

The overhead is only a jump instruction.

[vadixidav]

@cr1901 Ah, I see. That would be a good workaround.

Just calling them directly doesn't work but with inline assembly and naked function it's possible.

The overhead is only a jump instruction.

This was my plan, but now that I know it is possible to directly access the SR I might attempt that method. As mentioned above that probably wont be stabilized though, so if the maintainers wish to avoid the use of that feature then I'll use the inner function with the extra jump and return overhead. I will leave a comment when I begin working on this, but right now I am busy with other things.

[cr1901]

As mentioned above that probably wont be stabilized though, so if the maintainers wish to avoid the use of that feature then I'll use the inner function with the extra jump and return overhead.

While it's on my mind... a theoretical workaround is to put frame_address in the currently-nonexistent core::arch::msp430. Since libcore requires the nightly compiler, a downstream crate would be able to use frame_address in stable if core::arch::msp430 ever got stabilized.

However, I don't have plans to pursue this right now and would greatly prefer the trampoline method for now. Changing to the frame-address method in the future would be an implementation detail/semver-compat release.