cpu/cortexm_common: function to check address validity

[fjmolinas]

Contribution description

This PR retakes the work done in #10051. The PR was ready to merge except for missing white-list in the new test. As discussed in #10051, this is exactly the same PR but adding some boards to the test white-list. The rest of the PR description is as it was in the original one from @olegart.

Adds extremely useful function to check memory addresses for validity on Cortex-M3/M4/M7 MCUs.

Can be used to determine memory sizes, peripherals availability, etc. in run-time to build firmware that runs effectively on a different MCUs without recompiling.

Testing procedure

Build and run tests/cpu_cortexm_address_check for your board, then type 'check address' in the shell, where address is a memory address in hex notation (with '0x' prefix). Check your MCUs' datasheet for memory map.

E.g. on STM32L152RET6:

check 0x1FF02000
Address 0x1ff02000 is NOT valid. Accessing it will result in BusFault

check 0x1FF01999
Address 0x1ff01999 is valid. Feel free to access it

(0x1FF01999 is an upper address of the System Memory section)

Issues/PRs references

closes #10051

[aabadie]

Just a couple of small things. I think comments from #10051 are addressed by this PR.

Maybe also add a couple of kinetis in the list of whitelisted boards (there are only stm32 and sam for the moment) ?
I also think an automated python test script could be added in a follow-up PR.

[kaspar030]

IMO this should be guarded, possibly by a feature.

[fjmolinas]

@kaspar030, instead of providing a whitelist the is a cortexm_cpu_address_check pseodomodule feature requirements, is this what you where suggesting?

@aabadie thoughs? @jcarrano maybe you have some input since you where reviewing the original PR.

[kaspar030]

@kaspar030, instead of providing a whitelist the is a cortexm_cpu_address_check pseodomodule feature requirements, is this what you where suggesting?

Yes, that was one part. The other would be to guard the hard fault support code with it. As it is now, all cortexm0 builds gain weight, even if cpu_address_check is not used.

Maybe "cpu_address_check" is sufficient as feature name. All Cortex-M support the feature after all, but other architectures might add something like it.

[fjmolinas]

@kaspar030 you are right! I think latest changes should address your comments! If everyone is happy with this changes I can squash!

[kaspar030]

Looking good! Minor nits. Feel free to squash directly!

[fjmolinas]

@kaspar030 I was doing a final round of testing and found that the M0 catch wasn't working, well it was but the function was allwats returning 1. Looking in to it i found that:

register uint32_t result __asm("r5") = 1;

was being optimized in a way where result was always though to be 1. My fix was to load the register with 1 before assigning result to "r5". I'm honestly not very knowledge in assembly, so I would love your input on that fix, or a suggestion for a different one.

    __asm__ volatile (
        "ldr  r5, =0x1   \n" /* load r5 with 1 to set result    */
    );

    /* R5 will be set to 0 by HardFault handler */
    /* to indicate HardFault has occured */
    register uint32_t result __asm("r5");

PS: didn't squash this late modification since I'm not sure why it wasn't a problem in the original branch, maybe my compiler version?

[olegart]

Hi Yes, it seems recent gcc optimizes it out as it can't see that variable can be changed between declaration and return This should work: unwireddevices@b8f4e59 -- Sincerely yours,Oleg Artamonov+7 (916) 631-34-90www.unwds.com   11.04.2019, 19:04, "Francisco" <[email protected]>:@kaspar030 I was dooing a final round of testing and found that the M0 catch wasn't working, well it was but the function was allwats returning 1. Looking in to it i found that:register uint32_t result __asm("r5") = 1;was being optimized in a way where result was always though to be 1. My fix was to load the register with 1 before assigning result to "r5". I'm honestly not very knowledge in assembly, so I would love your input on that fix, or a suggestion for a different one.    __asm__ volatile (        "ldr  r5, =0x1   \n" /* load r5 with 1 to set result    */    );    /* R5 will be set to 0 by HardFault handler */    /* to indicate HardFault has occured */    register uint32_t result __asm("r5"); PS: didn't squash this late modification since I'm not sure why it wasn't a problem in the original branch, maybe my compiler version?—You are receiving this because you were mentioned.Reply to this email directly, view it on GitHub, or mute the thread.

[jcarrano]

@fjmolinas I have PR'd what I think is a neat fix to your branch: fjmolinas#1

It is a fixup commit that would replace the second commit in this PR.

[fjmolinas]

@olegart @jcarrano thanks for the suggestions I will go with @jcarrano suggestion since the code is easier to read while the firmware size remains the same for both options.

[fjmolinas]

@kaspar030 @aabadie any final comments?

[fjmolinas]

@kaspar030 @aabadie ping!

[jcarrano]

I think this is OK. I'd like a more automated test, but let's not delay it further.

[jcarrano]

@fjmolinas please squash the fixup and then I merge it.

[fjmolinas]

@jcarrano Sorry for the delay, squashed! Waiting for murdock to be happy!

[fjmolinas]

@jcarrano should I remove the needs squash tag? (Murdock failed because of it)

@aabadie @kaspar030 ping!!

[jcarrano]

@fjmolinas Yes, I prefer to remove it before squashing, so as not to trigger Murdock twice (and waste resources). IMO Murdock should not check for that label and it should be on a separate task.

I removed it and re-triggered the build.

Stale review