From d1605b581be8e3936783708fe68d2d3877d0dd47 Mon Sep 17 00:00:00 2001 From: sean Date: Tue, 3 Sep 2024 22:25:45 -0500 Subject: [PATCH 1/3] Adding GIC v2 interrupt group handling to GCC_AARCH64 port. --- portable/GCC/ARM_AARCH64/port.c | 3 ++ portable/GCC/ARM_AARCH64/portASM.S | 57 +++++++++++++++++++++++----- portable/GCC/ARM_AARCH64/portmacro.h | 21 ++++++---- 3 files changed, 64 insertions(+), 17 deletions(-) diff --git a/portable/GCC/ARM_AARCH64/port.c b/portable/GCC/ARM_AARCH64/port.c index 238874edc..41db96cfe 100644 --- a/portable/GCC/ARM_AARCH64/port.c +++ b/portable/GCC/ARM_AARCH64/port.c @@ -170,6 +170,9 @@ __attribute__( ( used ) ) const uint64_t ullICCEOIR = portICCEOIR_END_OF_INTERRU __attribute__( ( used ) ) const uint64_t ullICCIAR = portICCIAR_INTERRUPT_ACKNOWLEDGE_REGISTER_ADDRESS; __attribute__( ( used ) ) const uint64_t ullICCPMR = portICCPMR_PRIORITY_MASK_REGISTER_ADDRESS; __attribute__( ( used ) ) const uint64_t ullMaxAPIPriorityMask = ( configMAX_API_CALL_INTERRUPT_PRIORITY << portPRIORITY_SHIFT ); +__attribute__( ( used ) ) const uint64_t ullICCHPPIR = portICCHPPIR_HIGHEST_PRIORITY_INTERRUPT_REGISTER_ADDRESS; +__attribute__( ( used ) ) const uint64_t ullICCAIAR = portICCAIAR_ALIASED_INTERRUPT_ACKNOWLEDGE_REGISTER_ADDRESS; +__attribute__( ( used ) ) const uint64_t ullICCAEOIR = portICCAEOIR_ALIASED_END_OF_INTERRUPT_REGISTER_ADDRESS; /*-----------------------------------------------------------*/ diff --git a/portable/GCC/ARM_AARCH64/portASM.S b/portable/GCC/ARM_AARCH64/portASM.S index e684755bf..56eb64d67 100644 --- a/portable/GCC/ARM_AARCH64/portASM.S +++ b/portable/GCC/ARM_AARCH64/portASM.S @@ -307,13 +307,34 @@ FreeRTOS_IRQ_Handler: /* Maintain the interrupt nesting information across the function call. */ STP X1, X5, [SP, #-0x10]! - /* Read value from the interrupt acknowledge register, which is stored in W0 - for future parameter and interrupt clearing use. */ - LDR X2, ullICCIARConst - LDR X3, [X2] - LDR W0, [X3] /* ICCIAR in W0 as parameter. */ + /* Read value from the HPPI register + * In GIC v2, the HPPI register will contain: + * - 0x3FF on GIC ack or spurious IRQ + * - 0x3FE on pending IRQ within Group 1 (IRQ, non-secure) + * - IRQn on pending IRQ within Group 0 (FIQ, secure) + * + * X0 to contain IRQn + * X1 to contain IRQn Group number + */ + LDR X2, ullICCHPPIRConst + LDR X2, [X2] + LDR W3, [X2] + CMP W3, #0x3FE + B.NE 1f + + /* if IRQn Group 1, AIAR contains IRQn */ +2: LDR X2, ullICCAIARConst + MOV X1, #1 + B 0f + + /* if IRQn Group 0, IAR contains IRQn */ +1: LDR X2, ullICCIARConst + MOV X1, #0 - /* Maintain the ICCIAR value across the function call. */ +0: LDR W2, [X2] + LDR W0, [X2] + + /* Maintain the IRQn value across the function call. */ STP X0, X1, [SP, #-0x10]! /* Call the C handler. */ @@ -324,12 +345,25 @@ FreeRTOS_IRQ_Handler: DSB SY ISB SY - /* Restore the ICCIAR value. */ + /* Restore the IRqn value. */ LDP X0, X1, [SP], #0x10 - /* End IRQ processing by writing ICCIAR to the EOI register. */ - LDR X4, ullICCEOIRConst - LDR X4, [X4] + /* End IRQ processing by writing to the EOI register. + * In GIV v2, the EOI register to be used depends on the interrupt group: + * - IRQn Group 0 -> EOI + * - IRQn Group 1 -> AEOI + */ + CMP X1, #1 + B.NE 1f + + /* if IRQn Group 1, use AEOIR */ +2: LDR X4, ullICCAEOIRConst + B 0f + + /* if IRQn Group 0, use EOIR */ +1: LDR X4, ullICCEOIRConst + +0: LDR W4, [X4] STR W0, [X4] /* Restore the critical nesting count. */ @@ -420,6 +454,9 @@ ullPortInterruptNestingConst: .dword ullPortInterruptNesting ullPortYieldRequiredConst: .dword ullPortYieldRequired ullICCIARConst: .dword ullICCIAR ullICCEOIRConst: .dword ullICCEOIR +ullICCHPPIRConst: .dword ullICCHPPIR +ullICCAIARConst: .dword ullICCAIAR +ullICCAEOIRConst: .dword ullICCAEOIR vApplicationIRQHandlerConst: .word vApplicationIRQHandler diff --git a/portable/GCC/ARM_AARCH64/portmacro.h b/portable/GCC/ARM_AARCH64/portmacro.h index e89abb661..42e797f66 100644 --- a/portable/GCC/ARM_AARCH64/portmacro.h +++ b/portable/GCC/ARM_AARCH64/portmacro.h @@ -204,14 +204,21 @@ void FreeRTOS_Tick_Handler( void ); #define portICCEOIR_END_OF_INTERRUPT_OFFSET ( 0x10 ) #define portICCBPR_BINARY_POINT_OFFSET ( 0x08 ) #define portICCRPR_RUNNING_PRIORITY_OFFSET ( 0x14 ) +#define portICCHPPIR_HIGHEST_PRIORITY_INTERRUPT_OFFSET ( 0x18 ) +#define portICCAIAR_ALIASED_INTERRUPT_ACKNOWLEDGE_OFFSET ( 0x20 ) +#define portICCAEOIR_ALIASED_END_OF_INTERRUPT_OFFSET ( 0x24 ) + +#define portINTERRUPT_CONTROLLER_CPU_INTERFACE_ADDRESS ( configINTERRUPT_CONTROLLER_BASE_ADDRESS + configINTERRUPT_CONTROLLER_CPU_INTERFACE_OFFSET ) +#define portICCPMR_PRIORITY_MASK_REGISTER ( *( ( volatile uint32_t * ) ( portINTERRUPT_CONTROLLER_CPU_INTERFACE_ADDRESS + portICCPMR_PRIORITY_MASK_OFFSET ) ) ) +#define portICCIAR_INTERRUPT_ACKNOWLEDGE_REGISTER_ADDRESS ( portINTERRUPT_CONTROLLER_CPU_INTERFACE_ADDRESS + portICCIAR_INTERRUPT_ACKNOWLEDGE_OFFSET ) +#define portICCEOIR_END_OF_INTERRUPT_REGISTER_ADDRESS ( portINTERRUPT_CONTROLLER_CPU_INTERFACE_ADDRESS + portICCEOIR_END_OF_INTERRUPT_OFFSET ) +#define portICCPMR_PRIORITY_MASK_REGISTER_ADDRESS ( portINTERRUPT_CONTROLLER_CPU_INTERFACE_ADDRESS + portICCPMR_PRIORITY_MASK_OFFSET ) +#define portICCBPR_BINARY_POINT_REGISTER ( *( ( const volatile uint32_t * ) ( portINTERRUPT_CONTROLLER_CPU_INTERFACE_ADDRESS + portICCBPR_BINARY_POINT_OFFSET ) ) ) +#define portICCRPR_RUNNING_PRIORITY_REGISTER ( *( ( const volatile uint32_t * ) ( portINTERRUPT_CONTROLLER_CPU_INTERFACE_ADDRESS + portICCRPR_RUNNING_PRIORITY_OFFSET ) ) ) +#define portICCHPPIR_HIGHEST_PRIORITY_INTERRUPT_REGISTER_ADDRESS ( portINTERRUPT_CONTROLLER_CPU_INTERFACE_ADDRESS + portICCHPPIR_HIGHEST_PRIORITY_INTERRUPT_OFFSET ) +#define portICCAIAR_ALIASED_INTERRUPT_ACKNOWLEDGE_REGISTER_ADDRESS ( portINTERRUPT_CONTROLLER_CPU_INTERFACE_ADDRESS + portICCAIAR_ALIASED_INTERRUPT_ACKNOWLEDGE_OFFSET ) +#define portICCAEOIR_ALIASED_END_OF_INTERRUPT_REGISTER_ADDRESS ( portINTERRUPT_CONTROLLER_CPU_INTERFACE_ADDRESS + portICCAEOIR_ALIASED_END_OF_INTERRUPT_OFFSET ) -#define portINTERRUPT_CONTROLLER_CPU_INTERFACE_ADDRESS ( configINTERRUPT_CONTROLLER_BASE_ADDRESS + configINTERRUPT_CONTROLLER_CPU_INTERFACE_OFFSET ) -#define portICCPMR_PRIORITY_MASK_REGISTER ( *( ( volatile uint32_t * ) ( portINTERRUPT_CONTROLLER_CPU_INTERFACE_ADDRESS + portICCPMR_PRIORITY_MASK_OFFSET ) ) ) -#define portICCIAR_INTERRUPT_ACKNOWLEDGE_REGISTER_ADDRESS ( portINTERRUPT_CONTROLLER_CPU_INTERFACE_ADDRESS + portICCIAR_INTERRUPT_ACKNOWLEDGE_OFFSET ) -#define portICCEOIR_END_OF_INTERRUPT_REGISTER_ADDRESS ( portINTERRUPT_CONTROLLER_CPU_INTERFACE_ADDRESS + portICCEOIR_END_OF_INTERRUPT_OFFSET ) -#define portICCPMR_PRIORITY_MASK_REGISTER_ADDRESS ( portINTERRUPT_CONTROLLER_CPU_INTERFACE_ADDRESS + portICCPMR_PRIORITY_MASK_OFFSET ) -#define portICCBPR_BINARY_POINT_REGISTER ( *( ( const volatile uint32_t * ) ( portINTERRUPT_CONTROLLER_CPU_INTERFACE_ADDRESS + portICCBPR_BINARY_POINT_OFFSET ) ) ) -#define portICCRPR_RUNNING_PRIORITY_REGISTER ( *( ( const volatile uint32_t * ) ( portINTERRUPT_CONTROLLER_CPU_INTERFACE_ADDRESS + portICCRPR_RUNNING_PRIORITY_OFFSET ) ) ) #define portMEMORY_BARRIER() __asm volatile ( "" ::: "memory" ) From a882b105263c1fe53b1386fe17cb8b3ec90a388c Mon Sep 17 00:00:00 2001 From: Florian La Roche Date: Mon, 30 Jun 2025 08:35:46 +0200 Subject: [PATCH 2/3] fix possible NULL pointer dereference after call to configASSERT() (#1284) Compiling with clang static code analysis, possible NULL pointer dereference are found. Since configASSERT() can possibly return and continue "normal" operation, the code in queue.c and stream_buffer.c can be adjusted to avoid NULL pointer exceptions. Signed-off-by: Florian La Roche --- queue.c | 23 ++++++++--------------- stream_buffer.c | 6 ++---- 2 files changed, 10 insertions(+), 19 deletions(-) diff --git a/queue.c b/queue.c index fe06368d3..a967839de 100644 --- a/queue.c +++ b/queue.c @@ -1175,9 +1175,8 @@ BaseType_t xQueueGenericSendFromISR( QueueHandle_t xQueue, traceENTER_xQueueGenericSendFromISR( xQueue, pvItemToQueue, pxHigherPriorityTaskWoken, xCopyPosition ); - configASSERT( pxQueue ); - configASSERT( !( ( pvItemToQueue == NULL ) && ( pxQueue->uxItemSize != ( UBaseType_t ) 0U ) ) ); - configASSERT( !( ( xCopyPosition == queueOVERWRITE ) && ( pxQueue->uxLength != 1 ) ) ); + configASSERT( ( pxQueue != NULL ) && !( ( pvItemToQueue == NULL ) && ( pxQueue->uxItemSize != ( UBaseType_t ) 0U ) ) ); + configASSERT( ( pxQueue != NULL ) && !( ( xCopyPosition == queueOVERWRITE ) && ( pxQueue->uxLength != 1 ) ) ); /* RTOS ports that support interrupt nesting have the concept of a maximum * system call (or maximum API call) interrupt priority. Interrupts that are @@ -1351,16 +1350,14 @@ BaseType_t xQueueGiveFromISR( QueueHandle_t xQueue, * not (i.e. has a task with a higher priority than us been woken by this * post). */ - configASSERT( pxQueue ); - /* xQueueGenericSendFromISR() should be used instead of xQueueGiveFromISR() * if the item size is not 0. */ - configASSERT( pxQueue->uxItemSize == 0 ); + configASSERT( ( pxQueue != NULL ) && ( pxQueue->uxItemSize == 0 ) ); /* Normally a mutex would not be given from an interrupt, especially if * there is a mutex holder, as priority inheritance makes no sense for an - * interrupts, only tasks. */ - configASSERT( !( ( pxQueue->uxQueueType == queueQUEUE_IS_MUTEX ) && ( pxQueue->u.xSemaphore.xMutexHolder != NULL ) ) ); + * interrupt, only tasks. */ + configASSERT( ( pxQueue != NULL ) && !( ( pxQueue->uxQueueType == queueQUEUE_IS_MUTEX ) && ( pxQueue->u.xSemaphore.xMutexHolder != NULL ) ) ); /* RTOS ports that support interrupt nesting have the concept of a maximum * system call (or maximum API call) interrupt priority. Interrupts that are @@ -1895,12 +1892,9 @@ BaseType_t xQueuePeek( QueueHandle_t xQueue, traceENTER_xQueuePeek( xQueue, pvBuffer, xTicksToWait ); - /* Check the pointer is not NULL. */ - configASSERT( ( pxQueue ) ); - /* The buffer into which data is received can only be NULL if the data size * is zero (so no data is copied into the buffer. */ - configASSERT( !( ( ( pvBuffer ) == NULL ) && ( ( pxQueue )->uxItemSize != ( UBaseType_t ) 0U ) ) ); + configASSERT( ( pxQueue != NULL ) && !( ( ( pvBuffer ) == NULL ) && ( ( pxQueue )->uxItemSize != ( UBaseType_t ) 0U ) ) ); /* Cannot block if the scheduler is suspended. */ #if ( ( INCLUDE_xTaskGetSchedulerState == 1 ) || ( configUSE_TIMERS == 1 ) ) @@ -2152,9 +2146,8 @@ BaseType_t xQueuePeekFromISR( QueueHandle_t xQueue, traceENTER_xQueuePeekFromISR( xQueue, pvBuffer ); - configASSERT( pxQueue ); - configASSERT( !( ( pvBuffer == NULL ) && ( pxQueue->uxItemSize != ( UBaseType_t ) 0U ) ) ); - configASSERT( pxQueue->uxItemSize != 0 ); /* Can't peek a semaphore. */ + configASSERT( ( pxQueue != NULL ) && !( ( pvBuffer == NULL ) && ( pxQueue->uxItemSize != ( UBaseType_t ) 0U ) ) ); + configASSERT( ( pxQueue != NULL ) && ( pxQueue->uxItemSize != 0 ) ); /* Can't peek a semaphore. */ /* RTOS ports that support interrupt nesting have the concept of a maximum * system call (or maximum API call) interrupt priority. Interrupts that are diff --git a/stream_buffer.c b/stream_buffer.c index b1f9c0d57..4f13c679b 100644 --- a/stream_buffer.c +++ b/stream_buffer.c @@ -1653,11 +1653,9 @@ void vStreamBufferSetStreamBufferNotificationIndex( StreamBufferHandle_t xStream traceENTER_vStreamBufferSetStreamBufferNotificationIndex( xStreamBuffer, uxNotificationIndex ); - configASSERT( pxStreamBuffer ); - /* There should be no task waiting otherwise we'd never resume them. */ - configASSERT( pxStreamBuffer->xTaskWaitingToReceive == NULL ); - configASSERT( pxStreamBuffer->xTaskWaitingToSend == NULL ); + configASSERT( ( pxStreamBuffer != NULL ) && ( pxStreamBuffer->xTaskWaitingToReceive == NULL ) ); + configASSERT( ( pxStreamBuffer != NULL ) && ( pxStreamBuffer->xTaskWaitingToSend == NULL ) ); /* Check that the task notification index is valid. */ configASSERT( uxNotificationIndex < configTASK_NOTIFICATION_ARRAY_ENTRIES ); From 7225fbcbb94b945555beabfde42f756127b5a42c Mon Sep 17 00:00:00 2001 From: Gaurav-Aggarwal-AWS <33462878+aggarg@users.noreply.github.com> Date: Mon, 30 Jun 2025 18:11:30 +0530 Subject: [PATCH 3/3] Fix datatype of queue item length macros (#1286) The uxItemSize parameter in xQueueGenericCreate and xQueueGeneenericCreateStatic APIs expects a UBaseType_t type. Previously, the semSEMAPHORE_QUEUE_ITEM_LENGTH macro incorrectly cast the value to uint8_t, causing type mismatch warnings. This change resolves the issue by properly casting the value to UBaseType_t. This issue was reported here: https://github.com/FreeRTOS/FreeRTOS-Kernel/issues/1285. Signed-off-by: Gaurav Aggarwal --- include/semphr.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/include/semphr.h b/include/semphr.h index 8977acadb..7b44d78c0 100644 --- a/include/semphr.h +++ b/include/semphr.h @@ -37,8 +37,8 @@ typedef QueueHandle_t SemaphoreHandle_t; -#define semBINARY_SEMAPHORE_QUEUE_LENGTH ( ( uint8_t ) 1U ) -#define semSEMAPHORE_QUEUE_ITEM_LENGTH ( ( uint8_t ) 0U ) +#define semBINARY_SEMAPHORE_QUEUE_LENGTH ( ( UBaseType_t ) 1U ) +#define semSEMAPHORE_QUEUE_ITEM_LENGTH ( ( UBaseType_t ) 0U ) #define semGIVE_BLOCK_TIME ( ( TickType_t ) 0U )