portENTER_CRITICAL portEXIT_CRITICAL enables interrupt when it should not.

[PR.10]

I am using freertos on stm32g071 devkit and I noticed following problem. Here is a simplified example:

void some_os_api(void)
{
 portENTER_CRITICAL();
 // do stuff
 portEXIT_CRITICAL();
}
 
void function_call_from_main_or_a_task(void)
{
 // save interrupt to restore state after and disable 
 uint32_t primask = __get_PRIMASK();
 __set_PRIMASK(1)
		
 // do something without interruption or task switch 
 some_os_api();
 // do something more here the interrupt should be still disabled, but it is not! 
 
 // restore interrupt to what ever it was before
 __set_PRIMASK(primask);
}

the problem is that portEXIT_CRITICAL does not care if interrupt was already disabled when portENTER_CRITICAL was called. It is obvious from the implementation that is will enable GI in either case which is wrong.

void vPortEnterCritical( void )
{
	portDISABLE_INTERRUPTS();
	uxCriticalNesting++;
}
 
void vPortExitCritical( void )
{
	uxCriticalNesting--;
	if( uxCriticalNesting == 0 )
		portENABLE_INTERRUPTS();
}

I know that it must not be called from interrupt context, but this is not the case.

I have not seen any note that interrupt must enabled prior to portEXIT_CRITICAL call. Or did I miss it? If not then it looks like a bug to me.

In my case it breaks the initialization. Because it will enable GI in the middle of the initialization and will execute an interrupt before all memory is initialized. That is obviously a big problem.

Any suggestion how to fix the port macro to behave safely?

[Pavel A.]

As you see, vPortExitCritical will not enable interrupts if uxCriticalNesting was > 1.

So, whoever disables interrupts in FreeRTOS presence must increment uxCriticalNesting accordingly.

[PR.10]

I was thinking about it, but I can not change uxCriticalNesting because it is internal variable of the generated code. It is defined as static. Probably exactly to prevent what you suggested. I can neither extern it, remove the "static", or add any acossor to port.c file. It would be lost during code generation update. Any idea how to go around that?

[Pavel A.]

Call vPortEnterCritical?

[PR.10]

I preferred the

uint32_t primask = __get_PRIMASK();

__set_PRIMASK(1)

because is that can be called from both interrupt, non-int, enabled or disabled. where vPortEnterCritical can not.

I could check interrupt context inside function to conditionally skip the vPortEnterCritical call.Not as fast or pretty, but I guess it will work with this port.

Would be nice to have some interrupt agnostic version of enter/exit critical though.

[Piranha]

Instead of port***() macros user code must use task***() macros.

https://www.freertos.org/a00020.html

Making the function_call_from_main_or_a_task() callable from interrupts doesn't make sense, because it calls the some_os_api(), which cannot be called from interrupts:

"taskENTER_CRITICAL() and taskEXIT_CRITICAL() must not be called from an interrupt service routine (ISR) - see taskENTER_CRITICAL_FROM_ISR() and taskEXIT_CRITICAL_FROM_ISR() for interrupt safe equivalents."

https://www.freertos.org/taskENTER_CRITICAL_taskEXIT_CRITICAL.html

And those task***_CRITICAL_FROM_ISR() macros do the same PRIMASK management as your example. Those macros are interrupt/thread mode agnostic, one just cannot call non-ISR API calls in between those macros. Also __disable_irq() does effectively the same as __set_PRIMASK(1), but is more efficient.