BASEPRI manipulation

In the port for ARM_CM7 GCC, interrupts are enabled/disabled by acting on BASEPRI, which masks interrupts whose logical priority is lower than a threshold.

The following macros and functions are relevant to the topic.
See:

#define portSET_INTERRUPT_MASK_FROM_ISR()         ulPortRaiseBASEPRI()
#define portCLEAR_INTERRUPT_MASK_FROM_ISR( x )    vPortSetBASEPRI( x )
#define portDISABLE_INTERRUPTS()                  vPortRaiseBASEPRI()
#define portENABLE_INTERRUPTS()                   vPortSetBASEPRI( 0 )

portFORCE_INLINE static void vPortRaiseBASEPRI( void )
{
    uint32_t ulNewBASEPRI;

    __asm volatile
    (
        "	mov %0, %1												\n"\
        "	cpsid i													\n"\
        "	msr basepri, %0											\n"\
        "	isb														\n"\
        "	dsb														\n"\
        "	cpsie i													\n"\
        : "=r" ( ulNewBASEPRI ) : "i" ( configMAX_SYSCALL_INTERRUPT_PRIORITY ) : "memory"
    );
}

portFORCE_INLINE static uint32_t ulPortRaiseBASEPRI( void )
{
    uint32_t ulOriginalBASEPRI, ulNewBASEPRI;

    __asm volatile
    (
        "	mrs %0, basepri											\n"\
        "	mov %1, %2												\n"\
        "	cpsid i													\n"\
        "	msr basepri, %1											\n"\
        "	isb														\n"\
        "	dsb														\n"\
        "	cpsie i													\n"\
        : "=r" ( ulOriginalBASEPRI ), "=r" ( ulNewBASEPRI ) : "i" ( configMAX_SYSCALL_INTERRUPT_PRIORITY ) : "memory"
    );

    /* This return will not be reached but is necessary to prevent compiler
     * warnings. */
    return ulOriginalBASEPRI;
}

The “raise BASEPRI” functions do not actually raise it, they just set it to a value (configMAX_SYSCALL_INTERRUPT_PRIORITY) that is appropriate to exclude all interrupts that are allowed to use FreeRTOS functions, so that from the RTOS point of view, interrupts are effectively disabled.
However, wouldn’t it be better to actually raise the BASEPRI rather than set it? If for any reason the BASEPRI is currently set to a priority higher than configMAX_SYSCALL_INTERRUPT_PRIORITY, then the “disable interrupt” macros would actually re-enable some interrupts (though all of them outside of the RTOS scope).
This new behavior would be simple to obtain: instead of using the istruction “msr basepri, %1”, the functions could use “msr basepri_max, %1”. This doesn’t affect performances, as far as I can tell.
I don’t know if basepri_max applies to architectures other than ARM_CM7.

That change might give developers the impression that the port handles BASEPRI more carefully than it actually does.

It’s essentially a rule of the Cortex M ports that BASEPRI should be 0 or configMAX_SYSCALL_INTERRUPT_PRIORITY, and nothing else (except inside ISRs). In your example, how would BASEPRI get set to a higher priority than configMAX_SYSCALL_INTERRUPT_PRIORITY?

I think it is a valid point but the kernel assumes it has control of the BASEPRI register, so would always know the value it contained and there would not be contention, and as per jefftenney@ there are only ever two values.

I’m not saying that BASEPRI can ever have any other value besides those two, if only FreeRTOS is allowed to touch it.
It’s that the application code itself might need to raise BASEPRI as a form of locking.
Say we have interrupts A and B, with priorities pA < pB (logical priority; B can nest inside A).
Say that A executes some code, and part of it is to access a resource R shared with B:

void A_handler(void) {
	some code 1
	access R
	some code 2
}

A can safely access R this way:

void A_handler(void) {
	some code 1
	raise BASEPRI to pB
	access R
	restore BASEPRI
	some code 2
}

I know that there are alternatives and caveats, but I think this is a possible and valid scenario.
For example:

  • A and B could both be at priority <= configMAX_SYSCALL_INTERRUPT_PRIORITY, and use FreeRTOS locking (mutexes, critical sections, etc.). However, maybe it’s useful to have them at higher priority, to service them faster.
  • A could completely disable interrupts. However, there might be other higher-priority interrupts that shouldn’t be masked out.
  • It might be better not to access R in the ISR. However, this depends on the application needs.

In the end, I don’t think that not being able to set BASEPRI is a big problem, but it would be useful to be able to.

I’m not clear what you mean by “the port handles BASEPRI more carefully than it actually does”: as far as I can tell from my limited familiarity with FreeRTOS, the change would cause no problems for this pair of calls:

#define portSET_INTERRUPT_MASK_FROM_ISR()    ulPortRaiseBASEPRI()
#define portCLEAR_INTERRUPT_MASK_FROM_ISR( x )    vPortSetBASEPRI( x ) 

It might only cause problems here:

#define portDISABLE_INTERRUPTS()    vPortRaiseBASEPRI()
#define portENABLE_INTERRUPTS()    vPortSetBASEPRI( 0 )

since BASEPRI would be restored to 0 instead of the previous value. However, this could be easily fixed by always using ulPortRaiseBASEPRI() and vPortSetBASEPRI( 0 ), so that BASEPRI is always restored. The difference is just a single instruction to store the current value (plus one more register being used), so the performance impact would not be too bad, I guess.
Do you think that more extensive changes would be needed to have this working properly?

In ISRs, you can do exactly what you propose already, and FreeRTOS will be perfectly happy and perfectly unaware. Save BASEPRI, then raise BASEPRI to whatever you want, then do some work, and then restore BASEPRI. I presume you wouldn’t be calling any FreeRTOS functions inside this special critical section since you’re masking a higher priority interrupt that musn’t be delayed by FreeRTOS. Any calls to FreeRTOS inside this critical section could have the problem you describe – namely an unintentional enabling of some interrupt priorities.

The Cortex-M ports don’t provide a function to set BASEPRI to whatever you want, but I think that function would be outside the scope of FreeRTOS anyway. You could use CMSIS for example, __get_BASEPRI() and __set_BASEPRI().

I’m assuming you also have a use case where task code (not an ISR) wants to manipulate BASEPRI manually. In that case, as you suggested, the port code would require additional updates to ensure that BASEPRI is always restored. Functions vPortEnterCritical() and vPortExitCritical() would be problematic. Their job would now include saving/restoring BASEPRI. Since critical sections nest, implementing the “restore” concept would be challenging.

You’re right about ISRs; my example was not really meaningful, since FreeRTOS is out of scope there.
Anyway, as you mentioned, the same applies if A is just regular task code, accessing a resource shared with some ISR.
You’re right about the restore being challenging too: it would affect application code, which would need to save the return value of portENTER_CRITICAL() and pass it to portEXIT_CRITICAL().
It doesn’t seem like a keeping a FreeRTOS-private list/array of restore values is feasible - it either needs dynamic memory or a maximum nesting level.
Would it be too bad to have a second pair of enter/exit critical macros that also perform the save/restore of BASEPRI? This way only application code interested in this extended functionality would need to change (as well as internal FreeRTOS calls, if needed).
Or maybe changing the current definition of portENTER_CRITICAL/portEXIT_CRITICAL based on a configuration option?

Keep in mind that kernel code also calls portENTER_CRITICAL() and portEXIT_CRITICAL(), and not using your newly proposed API.

To achieve the functionality you want, those two functions must handle this issue completely on their own, as in your final suggestion of defining them differently in a build option. For example, the functions could keep an array of counters instead of a single counter (uxCriticalNesting). The array size would be the number of interrupt levels.

There are also other complicated issues. For example, consider a low-priority task that sets the BASEPRI priority to something below configMAX_SYSCALL_INTERRUPT_PRIORITY to block some low-priority interrupts. A higher-priority interrupt comes and wakes a higher priority task. The higher-priority task now waits for the lower-priority task to finish its work because task switching occurs in the lowest priority interrupt (which is masked).

In my opinion (just one developer) I think FreeRTOS should continue to promote simple application design. A design with task code that masks interrupts to varying levels is probably overly complex.

I see your point.
Thanks for all the clarifications and insights.