Detect more startup config errors on Cortex M

[jefftenney]

Description

Add new assert statements in xPortStartScheduler() on Cortex M to verify correct installation of the PendSV and SVCall handlers. These same assertion statements also verify VTOR is set properly for targets with a modifiable VTOR register. These are common causes of startup issues for Cortex M users.

On ARMv7-M and ARMv8-M, add explicit setting of SVCall priority to 0 (the maximum priority). Prior to this PR the port code depended on system startup code not to change the priority from its default of 0. This a rare cause of startup issues for Cortex M users. (See last link below.) For consistency, replicate this new method of setting SVCall priority to the ARMv7-M MPU ports, replacing the previous mechanism used by those ports to set SVCall priority.

Test Steps

ARMv7-M: Execute demo application with and without #define vPortSVCHandler SVC_Handler and #define xPortPendSVHandler PendSV_Handler in FreeRTOSConfig.h. Before this PR, the application works correctly with both #defines, and doesn't work correctly without either one. After this PR, the application still works correctly with both #defines, and has an assert failure without either one, as intended.

ARMv6-M: Same as above but the port doesn't use vPortSVCHandler; test with and without xPortPendSvHandler defined. XMC1000 demo cannot be used for this test.

ARMv7-M / ARMv8-M: Use demo application. First thing in main() set VTOR to the wrong value. Before this PR, xPortStartScheduler() hard faults. After this PR, an assert failure detects the issue, as intended.

Checklist:

• I have tested my changes. No regression in existing tests.
• I have modified and/or added unit-tests to cover the code changes in this Pull Request.

Related Issues

https://forums.freertos.org/t/prvportstartfirsttask-fails-at-svc-0/18288
https://forums.freertos.org/t/hard-fault-encountered-after-vtaskstartscheduler-is-invoked/17476
https://forums.freertos.org/t/freertos-does-not-execute-first-task-after-svc-0-call/16124
https://forums.freertos.org/t/xportstartscheduler-fails-because-basepri-is-non-zero/15509
...

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

[jefftenney]

In a week or so, I plan to replicate these changes to all the other ARMv7-M targets and to the ARMv8-M targets too. I might also be able to replicate them to ARMv6-M. Then I'll remove the "draft" flag on this PR.

Meantime, please feel free to review. Better now than after the replication. 😄

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (553caa1) 93.78% compared to head (53cfc6d) 93.78%.

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #832   +/-   ##
=======================================
  Coverage   93.78%   93.78%           
=======================================
  Files           6        6           
  Lines        3184     3184           
  Branches      885      885           
=======================================
  Hits         2986     2986           
  Misses         91       91           
  Partials      107      107           

Flag	Coverage Δ
unittests	93.78% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[jefftenney]

A couple of issues with Cortex-M0 (ARMv6-M).

1. The port (currently) functions properly without the SVCall handler being installed. However, requiring it is still a good idea for backward and forward compatibility. So I've kept it in for the M0 just like the ports that do require the SVCall handler being installed. EDIT: Nevermind - I removed it after finding some FreeRTOS demos that don't install the SVC handler on CM0.
2. A preexisting comment in port.c indicate that we cannot rely on VTOR to find the vector table. However my reading of the reference manual indicates that isn't true, even when the device doesn't support VTOR at all. In that case the VTOR address space is RAZ/WI. And a review of the operation code for core reset indicates the core itself uses VTOR to find the vectors, even if VTOR is not implemented. Thus if VTOR is not implemented, the vector table is always at 0x00000000.
   

TakeReset()
    VTOR = Zeros(32);
    for i = 0 to 12
        R[i] = bits(32) UNKNOWN;
    bits(32) vectortable = VTOR;
    [...]
    SP_main = MemA[vectortable,4] AND 0xFFFFFFFC<31:0>;
    SP_process = ((bits(30) UNKNOWN):’00’);
    start = MemA[vectortable+4,4]; // Load address of reset routine

I suggest we either remove the preexisting comment or revert ARMv6-M, leaving it out of this PR. Happy to do whatever the team would like. EDIT: I've updated the comment.

[jefftenney]

Applications doing something "special" with their vectors may encounter a backward compatibility issue after this PR. For example, if they "re-vector" the FreeRTOS interrupts (which must be done very carefully), then if they try to upgrade FreeRTOS to a version that includes this PR, the new assert statements could fail. Thus I have added configCHECK_HANDLER_INSTALLATION.

The PR as of this writing defaults configCHECK_HANDLER_INSTALLATION to 1 so the new checks can be effective. However, that default setting does have the backward-compatibility issue described above. The issue would be very rare. As an example, the XMC1000 re-vectors all interrupts, so that demo will have to disable the new checks. See jefftenney/FreeRTOS@7376b2.

[jefftenney]

@aggarg @urutva This PR is ready for review.

[urutva]

Also, ARM_CM7 port for both GCC and IAR is not updated. Can you please check?

[aggarg]

I am looking at it and should be able to merge in couple of days. Thank you for your patience.

[jefftenney]

@aggarg No worries at all. If you do decide to merge this, I will raise a PR to #define configCHECK_HANDLER_INSTALLATION 0 for the XMC1000 demo. That's the only demo affected from what I can see.

That is a concept issue with this PR -- that some existing applications will be affected.

[sonarqubecloud]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
0.0% Duplication

[jefftenney]

As usual thanks Gaurav for all the work. Next time I will be sure to point out specifically when I haven't compiled with IAR. That way, first reviewers will have a better chance of resolving any build issues I missed, before it gets to later reviewers. (Actually, a few months ago I asked IAR to sponsor a license for this kind of open-source contribution, but it didn't work out.)

[aggarg]

No worries at all Jeff. Thank you for this change - this will really help to catch issue which are otherwise hard to debug in forums!

The change request is addressed in this PR. Dismiss this change request.

[RichardBarry]

@jefftenney Thanks for this great contribution!