Adding additional asserts for valid priority levels to cortex m4 port

Kernel
1

I realized the configKERNEL_INTERRUPT_PRIORITY and configMAX_SYSCALL_INTERRUPT_PRIORITY macros in my example configuration were incorrect; I assumed it would be shifted into the available configPRIO_BITS internally, but this is not the case.

2

Glad that you figured it.

3

Yeah, though I am a little surprised there isn’t an assert in the port to catch this kind of issue; there are several asserts in the port already related to the priority masks, but none that confirm that the chosen priorities are nonzero with respect to the configPRIO_BITS parameter or the hardware. Maybe these could be added?

4

Here is a patch that accomplishes this. (The existing assert on the unmasked configMAX_SYSCALL_INTERRUPT_PRIORITY is obviated by the masked check.) I tested it on my board and it catches my original problem.

diff --git a/portable/GCC/ARM_CM4F/port.c b/portable/GCC/ARM_CM4F/port.c
index b946e6e94..bd0a37ad3 100644
--- a/portable/GCC/ARM_CM4F/port.c
+++ b/portable/GCC/ARM_CM4F/port.c
@@ -295,10 +295,6 @@ static void prvPortStartFirstTask( void )
  */
 BaseType_t xPortStartScheduler( void )
 {
-    /* configMAX_SYSCALL_INTERRUPT_PRIORITY must not be set to 0.
-     * See https://www.FreeRTOS.org/RTOS-Cortex-M3-M4.html */
-    configASSERT( configMAX_SYSCALL_INTERRUPT_PRIORITY );
-
     /* This port can be used on all revisions of the Cortex-M7 core other than
      * the r0p1 parts.  r0p1 parts should use the port from the
      * /source/portable/GCC/ARM_CM7/r0p1 directory. */
@@ -329,6 +325,15 @@ BaseType_t xPortStartScheduler( void )
         /* Use the same mask on the maximum system call priority. */
         ucMaxSysCallPriority = configMAX_SYSCALL_INTERRUPT_PRIORITY & ucMaxPriorityValue;
 
+        /* Check that the maximum system call priority and the kernel interrupt
+         * priority are valid (nonzero) after accounting for the number of
+         * priority bits supported by the hardware. A priority of 0 is invalid
+         * for either, because setting the BASEPRI register to 0 unmasks all
+         * interrupts, and interrupts with priority 0 cannot be masked using
+         * BASEPRI. See https://www.FreeRTOS.org/RTOS-Cortex-M3-M4.html */
+        configASSERT( ucMaxSysCallPriority );
+        configASSERT( configKERNEL_INTERRUPT_PRIORITY & ucMaxPriorityValue );
+
         /* Calculate the maximum acceptable priority group value for the number
          * of bits read back. */
         ulMaxPRIGROUPValue = portMAX_PRIGROUP_BITS;
5

The change from configMAX_SYSCALL_INTERRUPT_PRIORITY to ucMaxSysCallPriority is a nice improvement. :+1:

However, if we add a check on configKERNEL_INTERRUPT_PRIORITY, it may be better to check that it is set to the lowest priority as required by this port.

configASSERT( ( configKERNEL_INTERRUPT_PRIORITY & ucMaxPriorityValue ) == ucMaxPriorityValue );

6

Yeah, that makes sense. I will make a PR for this later and get more feedback there.

7

PR here: Add more interrupt priority asserts for ARM_CM3, ARM_CM4, and ARM_CM7 by chrisnc · Pull Request #602 · FreeRTOS/FreeRTOS-Kernel · GitHub

1 Like
8

Thank you for your contribution. I left some comments on the PR.