Skip to content

Commit

Permalink
Cortex-M Assert when NVIC implements 8 PRIO bits (#639)
Browse files Browse the repository at this point in the history 
* Cortex-M Assert when NVIC implements 8 PRIO bits

* Fix CM3 ports

* Fix ARM_CM3_MPU

* Fix ARM CM3

* Fix ARM_CM4_MPU

* Fix ARM_CM4

* Fix GCC ARM_CM7

* Fix IAR ARM ports

* Uncrustify changes

* Fix MikroC_ARM_CM4F port

* Fix MikroC_ARM_CM4F port-(2)

* Fix RVDS ARM ports

* Revert changes for Tasking/ARM_CM4F port

* Revert changes for Tasking/ARM_CM4F port-(2)

* Update port.c

Fix GCC/ARM_CM4F port

* Update port.c

* update GCC\ARM_CM4F port

* update port.c

* Assert to check configMAX_SYSCALL_INTERRUPT_PRIORITY is set to higher priority

* Fix merge error: remove duplicate code

* Fix typos

---------

Co-authored-by: Gaurav-Aggarwal-AWS <[email protected]>
Co-authored-by: Ubuntu <[email protected]>
  • Loading branch information
@kar-rahul-aws @aggarg
3 people authored Mar 23, 2023
1 parent 9488ba2 commit 99797e1
Show file tree
Hide file tree
Showing 17 changed files with 805 additions and 366 deletions.
63 changes: 45 additions & 18 deletions portable/CCS/ARM_CM3/port.c
100644 → 100755
View file
Original file line number Diff line number Diff line change
Expand Up @@ -219,6 +219,7 @@ BaseType_t xPortStartScheduler( void )
#if ( configASSERT_DEFINED == 1 )
{
volatile uint32_t ulOriginalPriority;
volatile uint32_t ulImplementedPrioBits = 0;
volatile uint8_t * const pucFirstUserPriorityRegister = ( uint8_t * ) ( portNVIC_IP_REGISTERS_OFFSET_16 + portFIRST_USER_INTERRUPT_NUMBER );
volatile uint8_t ucMaxPriorityValue;

Expand Down Expand Up @@ -250,20 +251,46 @@ BaseType_t xPortStartScheduler( void )

/* Calculate the maximum acceptable priority group value for the number
* of bits read back. */
ulMaxPRIGROUPValue = portMAX_PRIGROUP_BITS;

while( ( ucMaxPriorityValue & portTOP_BIT_OF_BYTE ) == portTOP_BIT_OF_BYTE )
{
ulMaxPRIGROUPValue--;
ulImplementedPrioBits++;
ucMaxPriorityValue <<= ( uint8_t ) 0x01;
}

if( ulImplementedPrioBits == 8 )
{
/* When the hardware implements 8 priority bits, there is no way for
* the software to configure PRIGROUP to not have sub-priorities. As
* a result, the least significant bit is always used for sub-priority
* and there are 128 preemption priorities and 2 sub-priorities.
*
* This may cause some confusion in some cases - for example, if
* configMAX_SYSCALL_INTERRUPT_PRIORITY is set to 5, both 5 and 4
* priority interrupts will be masked in Critical Sections as those
* are at the same preemption priority. This may appear confusing as
* 4 is higher (numerically lower) priority than
* configMAX_SYSCALL_INTERRUPT_PRIORITY and therefore, should not
* have been masked. Instead, if we set configMAX_SYSCALL_INTERRUPT_PRIORITY
* to 4, this confusion does not happen and the behaviour remains the same.
*
* The following assert ensures that the sub-priority bit in the
* configMAX_SYSCALL_INTERRUPT_PRIORITY is clear to avoid the above mentioned
* confusion. */
configASSERT( ( configMAX_SYSCALL_INTERRUPT_PRIORITY & 0x1U ) == 0U );
ulMaxPRIGROUPValue = 0;
}
else
{
ulMaxPRIGROUPValue = portMAX_PRIGROUP_BITS - ulImplementedPrioBits;
}

#ifdef __NVIC_PRIO_BITS
{
/* Check the CMSIS configuration that defines the number of
* priority bits matches the number of priority bits actually queried
* from the hardware. */
configASSERT( ( portMAX_PRIGROUP_BITS - ulMaxPRIGROUPValue ) == __NVIC_PRIO_BITS );
configASSERT( ulImplementedPrioBits == __NVIC_PRIO_BITS );
}
#endif

Expand All @@ -272,7 +299,7 @@ BaseType_t xPortStartScheduler( void )
/* Check the FreeRTOS configuration that defines the number of
* priority bits matches the number of priority bits actually queried
* from the hardware. */
configASSERT( ( portMAX_PRIGROUP_BITS - ulMaxPRIGROUPValue ) == configPRIO_BITS );
configASSERT( ulImplementedPrioBits == configPRIO_BITS );
}
#endif

Expand Down Expand Up @@ -379,17 +406,17 @@ void xPortSysTickHandler( void )

/* Enter a critical section but don't use the taskENTER_CRITICAL()
* method as that will mask interrupts that should exit sleep mode. */
__asm( " cpsid i");
__asm( " dsb");
__asm( " isb");
__asm( " cpsid i" );
__asm( " dsb" );
__asm( " isb" );

/* If a context switch is pending or a task is waiting for the scheduler
* to be unsuspended then abandon the low power entry. */
if( eTaskConfirmSleepModeStatus() == eAbortSleep )
{
/* Re-enable interrupts - see comments above the cpsid instruction
* above. */
__asm( " cpsie i");
__asm( " cpsie i" );
}
else
{
Expand Down Expand Up @@ -450,27 +477,27 @@ void xPortSysTickHandler( void )

if( xModifiableIdleTime > 0 )
{
__asm( " dsb");
__asm( " wfi");
__asm( " isb");
__asm( " dsb" );
__asm( " wfi" );
__asm( " isb" );
}

configPOST_SLEEP_PROCESSING( xExpectedIdleTime );

/* Re-enable interrupts to allow the interrupt that brought the MCU
* out of sleep mode to execute immediately. See comments above
* the cpsid instruction above. */
__asm( " cpsie i");
__asm( " dsb");
__asm( " isb");
__asm( " cpsie i" );
__asm( " dsb" );
__asm( " isb" );

/* Disable interrupts again because the clock is about to be stopped
* and interrupts that execute while the clock is stopped will increase
* any slippage between the time maintained by the RTOS and calendar
* time. */
__asm( " cpsid i");
__asm( " dsb");
__asm( " isb");
__asm( " cpsid i" );
__asm( " dsb" );
__asm( " isb" );

/* Disable the SysTick clock without reading the
* portNVIC_SYSTICK_CTRL_REG register to ensure the
Expand Down Expand Up @@ -578,7 +605,7 @@ void xPortSysTickHandler( void )
vTaskStepTick( ulCompleteTickPeriods );

/* Exit with interrupts enabled. */
__asm( " cpsie i");
__asm( " cpsie i" );
}
}

Expand Down
63 changes: 45 additions & 18 deletions portable/CCS/ARM_CM4F/port.c
100644 → 100755
View file
Original file line number Diff line number Diff line change
Expand Up @@ -238,6 +238,7 @@ BaseType_t xPortStartScheduler( void )
#if ( configASSERT_DEFINED == 1 )
{
volatile uint32_t ulOriginalPriority;
volatile uint32_t ulImplementedPrioBits = 0;
volatile uint8_t * const pucFirstUserPriorityRegister = ( uint8_t * ) ( portNVIC_IP_REGISTERS_OFFSET_16 + portFIRST_USER_INTERRUPT_NUMBER );
volatile uint8_t ucMaxPriorityValue;

Expand Down Expand Up @@ -269,20 +270,46 @@ BaseType_t xPortStartScheduler( void )

/* Calculate the maximum acceptable priority group value for the number
* of bits read back. */
ulMaxPRIGROUPValue = portMAX_PRIGROUP_BITS;

while( ( ucMaxPriorityValue & portTOP_BIT_OF_BYTE ) == portTOP_BIT_OF_BYTE )
{
ulMaxPRIGROUPValue--;
ulImplementedPrioBits++;
ucMaxPriorityValue <<= ( uint8_t ) 0x01;
}

if( ulImplementedPrioBits == 8 )
{
/* When the hardware implements 8 priority bits, there is no way for
* the software to configure PRIGROUP to not have sub-priorities. As
* a result, the least significant bit is always used for sub-priority
* and there are 128 preemption priorities and 2 sub-priorities.
*
* This may cause some confusion in some cases - for example, if
* configMAX_SYSCALL_INTERRUPT_PRIORITY is set to 5, both 5 and 4
* priority interrupts will be masked in Critical Sections as those
* are at the same preemption priority. This may appear confusing as
* 4 is higher (numerically lower) priority than
* configMAX_SYSCALL_INTERRUPT_PRIORITY and therefore, should not
* have been masked. Instead, if we set configMAX_SYSCALL_INTERRUPT_PRIORITY
* to 4, this confusion does not happen and the behaviour remains the same.
*
* The following assert ensures that the sub-priority bit in the
* configMAX_SYSCALL_INTERRUPT_PRIORITY is clear to avoid the above mentioned
* confusion. */
configASSERT( ( configMAX_SYSCALL_INTERRUPT_PRIORITY & 0x1U ) == 0U );
ulMaxPRIGROUPValue = 0;
}
else
{
ulMaxPRIGROUPValue = portMAX_PRIGROUP_BITS - ulImplementedPrioBits;
}

#ifdef __NVIC_PRIO_BITS
{
/* Check the CMSIS configuration that defines the number of
* priority bits matches the number of priority bits actually queried
* from the hardware. */
configASSERT( ( portMAX_PRIGROUP_BITS - ulMaxPRIGROUPValue ) == __NVIC_PRIO_BITS );
configASSERT( ulImplementedPrioBits == __NVIC_PRIO_BITS );
}
#endif

Expand All @@ -291,7 +318,7 @@ BaseType_t xPortStartScheduler( void )
/* Check the FreeRTOS configuration that defines the number of
* priority bits matches the number of priority bits actually queried
* from the hardware. */
configASSERT( ( portMAX_PRIGROUP_BITS - ulMaxPRIGROUPValue ) == configPRIO_BITS );
configASSERT( ulImplementedPrioBits == configPRIO_BITS );
}
#endif

Expand Down Expand Up @@ -404,17 +431,17 @@ void xPortSysTickHandler( void )

/* Enter a critical section but don't use the taskENTER_CRITICAL()
* method as that will mask interrupts that should exit sleep mode. */
__asm( " cpsid i");
__asm( " dsb");
__asm( " isb");
__asm( " cpsid i" );
__asm( " dsb" );
__asm( " isb" );

/* If a context switch is pending or a task is waiting for the scheduler
* to be unsuspended then abandon the low power entry. */
if( eTaskConfirmSleepModeStatus() == eAbortSleep )
{
/* Re-enable interrupts - see comments above the cpsid instruction
* above. */
__asm( " cpsie i");
__asm( " cpsie i" );
}
else
{
Expand Down Expand Up @@ -475,27 +502,27 @@ void xPortSysTickHandler( void )

if( xModifiableIdleTime > 0 )
{
__asm( " dsb");
__asm( " wfi");
__asm( " isb");
__asm( " dsb" );
__asm( " wfi" );
__asm( " isb" );
}

configPOST_SLEEP_PROCESSING( xExpectedIdleTime );

/* Re-enable interrupts to allow the interrupt that brought the MCU
* out of sleep mode to execute immediately. See comments above
* the cpsid instruction above. */
__asm( " cpsie i");
__asm( " dsb");
__asm( " isb");
__asm( " cpsie i" );
__asm( " dsb" );
__asm( " isb" );

/* Disable interrupts again because the clock is about to be stopped
* and interrupts that execute while the clock is stopped will increase
* any slippage between the time maintained by the RTOS and calendar
* time. */
__asm( " cpsid i");
__asm( " dsb");
__asm( " isb");
__asm( " cpsid i" );
__asm( " dsb" );
__asm( " isb" );

/* Disable the SysTick clock without reading the
* portNVIC_SYSTICK_CTRL_REG register to ensure the
Expand Down Expand Up @@ -603,7 +630,7 @@ void xPortSysTickHandler( void )
vTaskStepTick( ulCompleteTickPeriods );

/* Exit with interrupts enabled. */
__asm( " cpsie i");
__asm( " cpsie i" );
}
}

Expand Down
37 changes: 32 additions & 5 deletions portable/GCC/ARM_CM3/port.c
100644 → 100755
View file
Original file line number Diff line number Diff line change
Expand Up @@ -262,6 +262,7 @@ BaseType_t xPortStartScheduler( void )
#if ( configASSERT_DEFINED == 1 )
{
volatile uint32_t ulOriginalPriority;
volatile uint32_t ulImplementedPrioBits = 0;
volatile uint8_t * const pucFirstUserPriorityRegister = ( volatile uint8_t * const ) ( portNVIC_IP_REGISTERS_OFFSET_16 + portFIRST_USER_INTERRUPT_NUMBER );
volatile uint8_t ucMaxPriorityValue;

Expand Down Expand Up @@ -293,20 +294,46 @@ BaseType_t xPortStartScheduler( void )

/* Calculate the maximum acceptable priority group value for the number
* of bits read back. */
ulMaxPRIGROUPValue = portMAX_PRIGROUP_BITS;

while( ( ucMaxPriorityValue & portTOP_BIT_OF_BYTE ) == portTOP_BIT_OF_BYTE )
{
ulMaxPRIGROUPValue--;
ulImplementedPrioBits++;
ucMaxPriorityValue <<= ( uint8_t ) 0x01;
}

if( ulImplementedPrioBits == 8 )
{
/* When the hardware implements 8 priority bits, there is no way for
* the software to configure PRIGROUP to not have sub-priorities. As
* a result, the least significant bit is always used for sub-priority
* and there are 128 preemption priorities and 2 sub-priorities.
*
* This may cause some confusion in some cases - for example, if
* configMAX_SYSCALL_INTERRUPT_PRIORITY is set to 5, both 5 and 4
* priority interrupts will be masked in Critical Sections as those
* are at the same preemption priority. This may appear confusing as
* 4 is higher (numerically lower) priority than
* configMAX_SYSCALL_INTERRUPT_PRIORITY and therefore, should not
* have been masked. Instead, if we set configMAX_SYSCALL_INTERRUPT_PRIORITY
* to 4, this confusion does not happen and the behaviour remains the same.
*
* The following assert ensures that the sub-priority bit in the
* configMAX_SYSCALL_INTERRUPT_PRIORITY is clear to avoid the above mentioned
* confusion. */
configASSERT( ( configMAX_SYSCALL_INTERRUPT_PRIORITY & 0x1U ) == 0U );
ulMaxPRIGROUPValue = 0;
}
else
{
ulMaxPRIGROUPValue = portMAX_PRIGROUP_BITS - ulImplementedPrioBits;
}

#ifdef __NVIC_PRIO_BITS
{
/* Check the CMSIS configuration that defines the number of
* priority bits matches the number of priority bits actually queried
* from the hardware. */
configASSERT( ( portMAX_PRIGROUP_BITS - ulMaxPRIGROUPValue ) == __NVIC_PRIO_BITS );
configASSERT( ulImplementedPrioBits == __NVIC_PRIO_BITS );
}
#endif

Expand All @@ -315,7 +342,7 @@ BaseType_t xPortStartScheduler( void )
/* Check the FreeRTOS configuration that defines the number of
* priority bits matches the number of priority bits actually queried
* from the hardware. */
configASSERT( ( portMAX_PRIGROUP_BITS - ulMaxPRIGROUPValue ) == configPRIO_BITS );
configASSERT( ulImplementedPrioBits == configPRIO_BITS );
}
#endif

Expand Down Expand Up @@ -756,4 +783,4 @@ __attribute__( ( weak ) ) void vPortSetupTimerInterrupt( void )
configASSERT( ( portAIRCR_REG & portPRIORITY_GROUP_MASK ) <= ulMaxPRIGROUPValue );
}

#endif /* configASSERT_DEFINED */
#endif /* configASSERT_DEFINED */
Loading

0 comments on commit 99797e1

Please sign in to comment.