Skip to content
This repository has been archived by the owner on Jan 24, 2022. It is now read-only.

Allow running code before main #51

Closed
wants to merge 4 commits into from

Conversation

yodaldevoid
Copy link
Contributor

I found I needed support for running code at startup recently when I found that the .data region in one project grew large enough that initializing it was taking long enough that the default hardware watchdog was triggering a reset. Because of this usecase r0::run_init_array is placed before r0::zero_bss and r0::init_data in case either takes too long.

The only downside from the user's perspective of the current implementation is the requirement on users to add #![feature(used)] to their crate if they want to use the macro.

From the perspective of cortex-m-rt this does add another unstable feature in use_extern_macros (tracking issue). It appears that this feature will stay unstable until macros 2.0 lands. Users do not need to enable use_extern_macros and can use the macro either fromr0 or through cortex-m-rt via #[macro_use].

@yodaldevoid
Copy link
Contributor Author

@japaric Do you have any questions, comments, or concerns on this addition?

@japaric
Copy link
Member

japaric commented Jan 13, 2018

Sorry, I'm preparing several releases and don't have time to look in detail. I'll check this once I'm done with that.

One concern I have about this though, regardless of the implementation, is that, with the new singletons approach, if you take the peripherals in a "before main" function you won't be able to use the peripherals in main / the rest of the program unless we add some mechanism to send values from the "before main" function to the main function, or something like that.

I suggest you try this change with the master branches of cortex-m and svd2rust to see how / if it works.

@yodaldevoid
Copy link
Contributor Author

yodaldevoid commented Jan 13, 2018

Don't worry about not getting to this soon, I just wanted to double check it wasn't lost to a flood of notifications.

I will look at getting this to work with the new peripheral mechanism in cortex-m and svd2rust.

@yodaldevoid
Copy link
Contributor Author

I wish I had seen the design coming down the pipeline a month ago. I agree that with the current implementation takeing the peripherals before main would stop you from using them later. While you could send the peripherals somehow to main, all the ideas I can think of doing so don't sit right with me.

Passing the Peripherals into main solves the problem, but then we have a non-standard main function. The Peripherals could be passed up to some static that then could be called upon in main to get the Peripherals, and possibly any other bits and bobs the user wants to pass along with it, but then there are two methods to get the Peripherals and the user might not have a good indication which one to use. The simplest solution would be to add a drop impl to Peripherals that sets CORE_PERIPHERALSto false, but doing that breaks some of the advantages to the design and this possibility was specifically called out in the RFC as something that was not being done.

@yodaldevoid
Copy link
Contributor Author

I have updated the pull request with a method of passing the Peripherals from cortex-m from init_array to main.

The main idea of this method passes the peripherals through a static. In addition this static is of a typeInitArrayPeripherals (I know, creative naming. Please bikeshed) which mirrors Peripherals, but each peripheral is contained in an Option. This allows peripherals to optionally not be passed from init_array. In addition, while the InitArrayPeripherals struct can be passed back in init_array it is not allowed to be passed back in main. Trying to do so simply drops the struct and all peripherals inside. A alternative to dropping the stuct would be to return it back on failure, but I do not see this as a valid usecase as it is clear where InitArrayPeripherals should be given back and where it should not.

This does not solve passing other structures, such as peripherals from board crates, from init_array. A similar method would have to be done for each crate.

@yodaldevoid
Copy link
Contributor Author

Hello, I know you are a very busy man, but I was wondering if I could get some feedback on the current solution.

@japaric
Copy link
Member

japaric commented May 13, 2018

@yodaldevoid sorry for the long radio silence here.

I'm afraid that your proposed solution is not general enough. One of the use cases for this feature is configuring the clock to operate at higher speed before initializing the RAM with the goal of making the boot / pre-main process faster. Your solution doesn't solve that problem because configuring the clock requires access to device specific peripherals. I also think that the extensive use of Option and unwrap is unfortunate.

I'm currently leaning towards not solving the peripherals access problem in this crate, at least not right now. Instead we simply add the mechanism (e.g. .pre_init) to run custom code before zero_bss / init_data and leave it up to other crates to iterate and develop safe mechanisms to share peripherals between pre_init functions and main.

One such mechanism could be a macro that uses the unsafe Peripherals::steal under the hood. See below (jus a sketch off the top of my head, untested and likely contains errors):

/* user code */
entry!(stm32f103xx, before_main, main);

// NOTE unsafe because accessing `static` variables in this context is forbidden
// (a lint instead of unsafe would be ideal)
unsafe fn before_main(c: &mut cortex_m::Peripherals, d: &mut stm32f103xx::Peripherals) {
    // ..
}

fn main(c: cortex_m::Peripherals, d: stm32f103xx::Peripherals) -> ! {
    // ..
}
/* end of user code */

/* expansion of `entry!` */
#[link_section = ".pre_init"]
pub static PRE_INIT: unsafe extern "C" fn() = pre_init;

unsafe extern "C" fn pre_init() {
    before_main(&mut cortex_m::Peripherals::steal(), &mut stm32f103xx::Peripherals::steal());
}

#[export_name = "main"]
pub unsafe extern "C" fn __impl_main() -> ! {
    main(cortex_m::Peripherals::steal(), stm32f103xx::Peripherals::steal())
}
/* end of expansion of `entry!` */

/* cortex-m-rt */
#[no_mangle]
pub unsafe extern "C" fn Reset() -> ! {
    for f in PRE_INIT_ARRAY {
        f();
    }

    r0::zero_bss(..);
    r0::init_data(..);

    extern "C" {
        fn main() -> !;
    }

    main()
}

@yodaldevoid
Copy link
Contributor Author

That solution sounds fine by me. I understand not wanting to add hacked together solutions to this crate and as I think I mentioned before I had had reservations about this solution anyway. As it sounds like you are leaning towards the other (simpler) solution, I will close this and wait with baited breath.

@yodaldevoid yodaldevoid deleted the init_array branch August 22, 2018 14:23
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants