Initialization steps in toolchains #2160

LMESTM · 2016-07-13T08:02:39Z

This PR is a follow-up of "Fix issue #2124: IAR no rtos #2129" for other tooochains.
jamike analysis explained that mbed_sdk_init needs to be called once at the right time (after ram initialization and before C++ object creation) - this PR aims to fulfill this same requirement for the supported toolchains (gcc / arm / µarm) and also in case of ARM + rtos configuration.

Once this series is applied, few workarounds' can be removed from STM32 drivers.

LMESTM · 2016-07-13T08:05:43Z

Complete test report on all STM32 platforms, all TCs
fix_InitTCs_TestReport.txt
no regresssion seen

LMESTM · 2016-07-13T08:06:46Z

@jamike @adustm @svastm

0xc0170 · 2016-07-13T08:21:39Z

cc @c1728p9 @sg-

svastm · 2016-07-13T11:43:03Z

In the uARM fix, sys_open() is called 3 times in a row.
If we can't move the mbed_sdk_init() somewhere else, the function could be protected against multiple init.

static int mbed_sdk_inited = 0;
if (!mbed_sdk_inited) {
    mbed_sdk_inited = !mbed_sdk_inited;
    mbed_sdk_init();
}

LMESTM · 2016-07-13T17:40:39Z

@svastm Thanks for proposal indeed ...
any uarm expert that could even propose a better hook point ?

LMESTM · 2016-07-15T11:27:36Z

I took into account svastm proposal and I rebased to solve conflicts with latest master

0xc0170 · 2016-07-21T10:34:15Z

@mbed-bot: TEST

HOST_OSES=windows
BUILD_TOOLCHAINS=GCC_ARM,ARM,IAR
TARGETS=ALL

0xc0170 · 2016-07-21T10:35:01Z

hal/common/retarget.cpp


 extern "C" FILEHANDLE PREFIX(_open)(const char* name, int openmode) {
    #if defined(__MICROLIB) && (__ARMCC_VERSION>5030000)
    // Before version 5.03, we were using a patched version of microlib with proper names
    // This is the workaround that the microlib author suggested us
    static int n = 0;
+    static int mbed_sdk_inited = 0;
+    if (!mbed_sdk_inited) {
+        mbed_sdk_inited = 1;;


nit ;; double

thx - will correct now

0xc0170 · 2016-07-21T10:44:32Z

@c1728p9 Any comments to this changeset?

mbed-bot · 2016-07-21T11:54:10Z

[Build 660]
FAILURE: Something went wrong when building and testing.

0xc0170 · 2016-07-21T11:59:58Z

Test results look good

c1728p9 · 2016-07-21T13:22:20Z

rtos/rtx/TARGET_CORTEX_M/RTX_CM_lib.h

@@ -648,6 +649,7 @@ __asm void __rt_entry (void) {
  LDR     R4,=armcc_heap_top
  STR     R0,[R3]
  STR     R2,[R4]
+  BL      _platform_post_stackheap_init


Does this need to be called before the RTOS has been initialized? If not you might want to move this to pre_main.

@c1728p9 I actually have tried the pre_main hook as this was a good candidate. The call needs to be placed before the C++ objects are initialized, which is done I think in __rt_lib_init calling to _cpp_initializaze__aeabi. When RTOS gets initialized, this is too late unfortunately ...

0xc0170 · 2016-08-12T08:01:16Z

@LMESTM @c1728p9 Shall we proceed this. there are some minor comments.

LMESTM · 2016-08-17T14:25:07Z

@0xc0170 : rebased and updated according to comment
F0 & F4 STM32 families tested ok on ARM. Other TCs tests ongoing as well
More feedback and tests welcome

LMESTM · 2016-08-18T09:18:16Z

I confirm that all tests are OK on STM32 boards

LMESTM · 2016-08-24T15:05:50Z

@0xc0170 just rebased as there were conflicts

c1728p9 · 2016-08-26T17:20:21Z

hal/common/retarget.cpp

+    static int mbed_sdk_inited = 0;
+    if (!mbed_sdk_inited) {
+        mbed_sdk_inited = 1;
+        mbed_sdk_init();


Why is mbed_sdk_init only being called on _open for microlib? Also, here mbed_sdk_inited is checked, but it is not checked below so this might get called twice.

@c1728p9: it just happen that _open "seems" to be called on every init and that we are looking for a path that is called after RAM init and before C++ object initialization. If you happen to know another alternative, that would be welcome.

Here is what I wrote in commit message :

In uARM, the library's hook _platform_post_stackheap_init does not seem to
exist and I couldnot find a documentation describing the initialisation
flow. All we know is that _open is called after RAM init and before the
C++ init, so this is a working placeholder.

This is maybe not acceptable so a uARM lib expert may propose a better
hook to fullfil the requirement.

@c1728p9 About being called twice, I think that various calls are under different #defines - the one in _open is only for uARM toolchain, the other calls are in case of GCC or ARM toolchain. But I may be wrong, which line to you suspect to be the 2nd call ?

Ah I see. I didn't notice the define wrapping it before.

Various toolchains supported in MBED don't follow the same initialization steps. This can have impacts on platform behavior. For STM32, it is needed to call the HAL_Init() _after_ the RAM has been initialized (sdata from flash / zero initialized data) and _before_ the C++ objects are being created, especially if those objects require support of tickers for instance. In GCC, this is easily done because SystemInit is called after the ram initialisation, so HAL_Init does not need to called from mbed_sdk_init. this is covered by the changes in mbed_overrides.c files. This series should solve issue reported here: STM32 (At least F401) breaks if Tickers are activated in a global object ARMmbed#2115

Various toolchains supported in MBED don't followthe same initialization steps. This can have impacts on platform behavior. For STM32, it is needed to call the HAL_Init() _after_ the RAM has been initialized (sdata from flash / zero initialized data) and _before_ the C++ objects are being created, especially if those objects require support of tickers for instance. In GCC and IAR, this was done in previous commit to avoid HAL_Init() to be called twice. In ARM this there is no hook defined in MBED yet to place the call. The proposal is to take benefit of the library's _platform_post_stackheap_init function that is going to be called before __rt_lib_init where the C++ object init is done (__cpp_initialize__aeabi_) This series should solve issue reported here: STM32 (At least F401) breaks if Tickers are activated in a global object ARMmbed#2115

In uARM, the library's hook _platform_post_stackheap_init does not seem to exist and I couldnot find a documentation describing the initialisation flow. All we know is that _open is called after RAM init and before the C++ init, so this is a working placeholder. This is maybe not acceptable so a uARM lib expert may propose a better hook to fullfil the requirement. At least this is a workign setup. This series should solve issue reported here: STM32 (At least F401) breaks if Tickers are activated in a global object ARMmbed#2115

Supported toolchains initialization steps have been modified to make sure that mbed_sdk_initi is called _after_ RAM initialization and _before_ C++ objects creation. since this was done, there is no need to redundant SystemCoreClockUpdates in the drivers

LMESTM · 2016-08-29T10:03:09Z

rebased to solve conflicts with master ...

c1728p9 · 2016-08-29T13:24:57Z

/morph test

mbed-bot · 2016-08-29T15:07:34Z

Result: FAILURE

Your command has finished executing! Here's what you wrote!

/morph test

Output

mbed Build Number: 738

Test Prep failed!

bridadan · 2016-08-29T15:32:45Z

/morph test

mbed-bot · 2016-08-29T16:15:28Z

Result: FAILURE

Your command has finished executing! Here's what you wrote!

/morph test

Output

mbed Build Number: 739

Test Prep failed!

bridadan · 2016-08-29T18:04:49Z

Looks like there was bug introduced with the latest version of setuptools, oh joy! I've deployed a workaround that should fix the test prep issue for now until they patch setuptools.

/morph test

mbed-bot · 2016-08-29T19:18:14Z

Result: ABORTED

Your command has finished executing! Here's what you wrote!

/morph test

Output

mbed Build Number: 743

Build Cleanup failed!

LMESTM · 2016-08-31T15:56:16Z

@bridadan so what's next ? Is there something broken with my PR or with the test tool ?

bridadan · 2016-08-31T16:53:14Z

@LMESTM sorry for the delay on this. The above issue was caused by a faulty version of pip package that was released. I will rerun the CI now.

/morph test

mbed-bot · 2016-08-31T17:59:35Z

Result: FAILURE

Your command has finished executing! Here's what you wrote!

/morph test

Output

mbed Build Number: 760

Build failed!

bridadan · 2016-08-31T18:02:38Z

/morph test

mbed-bot · 2016-08-31T18:58:59Z

Result: SUCCESS

Your command has finished executing! Here's what you wrote!

/morph test

Output

mbed Build Number: 761

All builds and test passed!

bridadan · 2016-08-31T19:44:30Z

mbed OS 5 tests look good! Thanks for your patience!

sg- · 2016-09-08T22:56:48Z

mbed_sdk_init has been called just before main since the beginning of time. If a hook is needed for post RAM init and pre ctor then we should add a new one, not change the behavior of something existing. Looking at the history, it was introduced to solve a problem in that our IO constructors didn't allow a logic level to be initialized in the ctor. This has since been added invalidating the initial reason for existence but it will be safer to provide a new hook rather than reusing an existing one. We need to determine whether this happens before or after the RTOS is initialized. Ideas??

LMESTM · 2016-09-09T07:46:05Z

mbed_sdk_init has been called just before main since the beginning of time. If a hook is needed for post RAM init and pre ctor then we should add a new one, not change the behavior of something existing.

The point is that this behavior is not consistent over all toolchains (since the beginning also I guess ...)

This has since been added invalidating the initial reason for existence but it will be safer to provide a new hook rather than reusing an existing one

Which toolchain are you referring to in this comment ?
I think this is what I am proposing for ARM toolcahin in this series, but indeed for uARM I couldn't find a suitable place

We need to determine whether this happens before or after the RTOS is initialized. Ideas??

All of this happens before RTOS init as far as I've seen.
Can you confirm which of the changes seem acceptable and which ones need further thinking ? I may split the PR in 2 if needed ...

sg- · 2016-09-09T19:35:19Z

The point is that this behavior is not consistent over all toolchains (since the beginning also I guess ...)

That may have changed over time so we need to fix it.

Which toolchain are you referring to in this comment ?
I think this is what I am proposing for ARM toolcahin in this series, but indeed for uARM I couldn't find a suitable place

No, the reason for this hook was a IO API not providing the necessary initializer option

All of this happens before RTOS init as far as I've seen.

Quite the opposite https://github.com/ARMmbed/mbed-os/blob/master/hal/common/retarget.cpp#L523 IAR was the only one that specifically hooked pre ctor. 3062999

We'll take the action to add proper hooks and document the bootstrap process

LMESTM · 2016-09-12T13:01:23Z

@sg-

We'll take the action to add proper hooks and document the bootstrap process

when do you think this would this be done ? We have other ongoing developments that would be impacted if we can't solve this. Most of my colleagues are using this series in advance to not hit the init related issues.

There are actually 5 commits for 5 cases proposed in this PR:
GCC no RTOS
ARM no RTOS
uARM no RTOS
GCC+RTOS
ARM+RTOS
Are none of them acceptable for now ?

LMESTM · 2016-09-22T07:12:12Z

@sg @0xc0170 any news or plan for the next steps ?

sg- · 2016-10-03T16:41:20Z

Can we move forward with this and just fix what C027 does to make it safe (add init calls)?
This would mean:

SystemInit // cmsis-core
mbed_sdk_init // already in place and moved from post ctor to pre rtos init
rtos // system call
ctor // system call
mbed_main // already in place
main // already in place

sg- · 2016-10-05T05:55:32Z

Resolved by #2917

Thanks @LMESTM

…..4a19dc4 4a19dc4 Import new thread files f6a021d Removed test files b9e842a Merge branch 'release_internal' into release_external 7d5d869 Merge pull request ARMmbed#2167 from ARMmbed/release_internal_merge 26e2d43 Merge branch 'master' into release_internal f43620f Add support for India band (ARMmbed#2166) 122f158 Merge pull request ARMmbed#2165 from ARMmbed/release_internal_merge 0e65ee5 Added disabling of NA for Thread BR PPP backbone 4c50e52 Added disabling of NA for Wi-SUN BR PPP backbone d2ea325 Moved DAD enabled check to Ipv6 SLAAC handler 49994fc Added PPP interface to nanostack 3383e91 Merge pull request ARMmbed#2163 from ARMmbed/IOTTHD-3558 81f7511 MAC: print RF configs 397240a MAC: Implemented CCA threshold and TX power setting 5907042 Added check for allocation failures in EAPOL 9ed97c9 ETX update: b489415 Add own certificate handling APIS (ARMmbed#2149) 888a0fb fhss_ws: check if 0 used as divider 586f2f2 Merge pull request ARMmbed#2160 from hugueskamba/hk-iotcore-1299-remove-fp-usage-ns_monitor f1d03b1 Remove floating-point usage in Nanostack heap monitor ef88f64 Removed rank comprae and also probe 5 best on the list. a2887d6 Clean PAN id compare trace print. f37dcf2 Wi-SUN NS NUD & Probe send update f7133f8 Merge pull request ARMmbed#2158 from ARMmbed/remove_temp_debug_traces 2dc1a8e fhss_ws: removed temporary debug traces 96f962a Reduce wi-sun NS Probe 0a1beb2 GTK update trigger fix a1d172e Limit Pan config sol timeout after 5 solication. 9d7414b Limit PAE supplikant GTK re-use for authentication from 2->1. 662df08 Fixed Key request address set issue if GTK mismatch is detected. a56b908 Merge pull request ARMmbed#2153 from ARMmbed/IOTTHD-3650 9b33e98 Fixed mac_pd_sap CCA_PREPARE active ACK handler. 035af9a Enhanced ACK GEN and TX update b1beb5d fhss_ws: typecast drift to int32_t f786fc9 Merge pull request ARMmbed#2152 from ARMmbed/fhss_coverity_fix 6efff35 fhss_ws: Coverity fixes d743e91 WS LLC brodacst shedule fix 6a6fb0c Removed old configuration options from Border router API a051865 Merge pull request ARMmbed#2135 from ARMmbed/IOTTHD-3232 ff771b1 Added empty interface function for network name set e94da3c Merge pull request ARMmbed#2146 from ARMmbed/IOTTHD-3571_2 234e649 added network name change function to public API 1770465 fhss_ws: Added temporary debug traces (IOTTHD-3571) d400859 Fix Thread 1.1 unitests (ARMmbed#2145) 38978f3 wi-sun ETX update: 4a71b04 Adjust Thread functions defined for Thread 1.2 (ARMmbed#2139) 4d8dc0d remove border router from pan size calculation fb3363e Merge pull request ARMmbed#2141 from ARMmbed/IOTTHD-3571 f01c5f2 fhss_ws: conversion macros/functions to support int64 a7b0027 Suprress dio sending whenRPL is not yet ready f8c9d54 Adjust tracing (ARMmbed#2138) 678eaf8 Moved Thread 1.2 code to to correct place f39d07e Merge pull request ARMmbed#2136 from ARMmbed/IOTTHD-3571 ab23116 FHSS: temporary debug traces (IOTTHD-3571) 09d4b06 MAC: Implemented PHY statistics git-subtree-dir: features/nanostack/sal-stack-nanostack git-subtree-split: 4a19dc4

0xc0170 added needs: review needs: CI labels Jul 13, 2016

LMESTM mentioned this pull request Jul 15, 2016

STM32 (At least F401) breaks if Tickers are activated in a global object #2115

Closed

LMESTM force-pushed the fix_InitTCs branch 2 times, most recently from 884d3d4 to fecc8e3 Compare July 15, 2016 11:24

0xc0170 reviewed Jul 21, 2016
View reviewed changes

c1728p9 reviewed Jul 21, 2016
View reviewed changes

LMESTM force-pushed the fix_InitTCs branch from fecc8e3 to 926cafc Compare August 17, 2016 14:21

LMESTM force-pushed the fix_InitTCs branch from 926cafc to c2998c0 Compare August 24, 2016 15:05

c1728p9 reviewed Aug 26, 2016
View reviewed changes

Laurent MEUNIER added 3 commits August 29, 2016 09:38

LMESTM force-pushed the fix_InitTCs branch from c2998c0 to 6ae979f Compare August 29, 2016 10:02

sg- added needs: work do not merge and removed needs: CI labels Sep 8, 2016

sg- removed needs: review needs: work labels Sep 9, 2016

c1728p9 mentioned this pull request Oct 4, 2016

Initialization steps in toolchains #2917

Merged

1 task

sg- closed this Oct 5, 2016

Initialization steps in toolchains #2160

Initialization steps in toolchains #2160

Conversation

LMESTM commented Jul 13, 2016

LMESTM commented Jul 13, 2016

LMESTM commented Jul 13, 2016

0xc0170 commented Jul 13, 2016

svastm commented Jul 13, 2016 • edited Loading

LMESTM commented Jul 13, 2016

LMESTM commented Jul 15, 2016

0xc0170 commented Jul 21, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

0xc0170 commented Jul 21, 2016

mbed-bot commented Jul 21, 2016

0xc0170 commented Jul 21, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

0xc0170 commented Aug 12, 2016

LMESTM commented Aug 17, 2016

LMESTM commented Aug 18, 2016

LMESTM commented Aug 24, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

LMESTM commented Aug 29, 2016

c1728p9 commented Aug 29, 2016

mbed-bot commented Aug 29, 2016

Output

bridadan commented Aug 29, 2016

mbed-bot commented Aug 29, 2016

Output

bridadan commented Aug 29, 2016

mbed-bot commented Aug 29, 2016

Output

LMESTM commented Aug 31, 2016

bridadan commented Aug 31, 2016

mbed-bot commented Aug 31, 2016

Output

bridadan commented Aug 31, 2016

mbed-bot commented Aug 31, 2016

Output

bridadan commented Aug 31, 2016

sg- commented Sep 8, 2016

LMESTM commented Sep 9, 2016

sg- commented Sep 9, 2016

LMESTM commented Sep 12, 2016

LMESTM commented Sep 22, 2016

sg- commented Oct 3, 2016

sg- commented Oct 5, 2016

svastm commented Jul 13, 2016 •

edited

Loading