Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add default implementations of vApplicationGetIdleTaskMemory and vApplicationGetTimerTaskMemory #790

Merged
merged 10 commits into from
Sep 20, 2023
Merged

Add default implementations of vApplicationGetIdleTaskMemory and vApplicationGetTimerTaskMemory #790

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
0d9fdd2
Add __weak definitions for vApplicationGetIdleTaskMemory and
mdnrz Sep 10, 2023
8fbc647
Fix formatting in tasks.c
mdnrz Sep 11, 2023
e422a21
Remove the need of compiler specific syntax
aggarg Sep 18, 2023
7d273c9
Fix formatting
aggarg Sep 18, 2023
272e917
Fix formatting
aggarg Sep 18, 2023
8ff0c61
Merge branch 'main' into mehdi
aggarg Sep 18, 2023
fb0e81d
Tag idle TCBs and Stacks as privileged data
aggarg Sep 18, 2023
0e47601
Merge branch 'main' into mehdi
aggarg Sep 19, 2023
dbee798
Make default implementations for non-MPU ports only
aggarg Sep 19, 2023
ce181f2
Merge branch 'main' into mehdi
aggarg Sep 20, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions include/FreeRTOS.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1131,6 +1131,10 @@
#define configSUPPORT_STATIC_ALLOCATION 0
#endif

#ifndef configKERNEL_PROVIDED_STATIC_MEMORY
#define configKERNEL_PROVIDED_STATIC_MEMORY 0
aggarg marked this conversation as resolved.
Show resolved Hide resolved
#endif

#ifndef configSUPPORT_DYNAMIC_ALLOCATION
/* Defaults to 1 for backward compatibility. */
#define configSUPPORT_DYNAMIC_ALLOCATION 1
Expand Down
49 changes: 49 additions & 0 deletions tasks.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7653,3 +7653,52 @@ static void prvAddCurrentTaskToDelayedList( TickType_t xTicksToWait,
#endif

#endif /* if ( configINCLUDE_FREERTOS_TASK_C_ADDITIONS_H == 1 ) */
/*-----------------------------------------------------------*/

#if ( ( configSUPPORT_STATIC_ALLOCATION == 1 ) && ( configKERNEL_PROVIDED_STATIC_MEMORY == 1 ) && ( portUSING_MPU_WRAPPERS == 0 ) )

/*
* This is the kernel provided implementation of vApplicationGetIdleTaskMemory()
* to provide the memory that is used by the Idle task. It is used when
* configKERNEL_PROVIDED_STATIC_MEMORY is set to 1. The application can provide
* it's own implementation of vApplicationGetIdleTaskMemory by setting
* configKERNEL_PROVIDED_STATIC_MEMORY to 0 or leaving it undefined.
*/
void vApplicationGetIdleTaskMemory( StaticTask_t ** ppxIdleTaskTCBBuffer,
StackType_t ** ppxIdleTaskStackBuffer,
uint32_t * pulIdleTaskStackSize )
{
static StaticTask_t xIdleTaskTCB;
aggarg marked this conversation as resolved.
Show resolved Hide resolved
static StackType_t uxIdleTaskStack[ configMINIMAL_STACK_SIZE ];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it make sense to align these to the region size for MPU ports?
It becomes wasted memory on the ArmV8-M MPU ports, as they only require 32 byte alignment, but it would make this usable for the ArmV7-M MPU ports, as they require MPU region alignment to the size of the region.
I think doing this would allow these functions to be used by all of our ports

Copy link
Member

@aggarg aggarg Sep 18, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These have been made part of privileged data and that section is aligned using linker scripts. There is no need of any explicit alignment.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well the task stack will be turned into an MPU region, so if that's not aligned correctly it can lead to issues depending on the architecture.
I know on the ArmV7-R that having an unaligned MPU region can lead to "Unpredictable Behaviour"
And it looks like on the ArmV7-M that "Any unaligned access to Device or Strongly Ordered memory generates an alignment UsageFault".

Where right now the portPRIVILEGED_RAM_REGION MPU region over-writes the portSTACK_REGION, where I don't think this is an issue, as you stated.

However, if we ever set portSTACK_REGION to have a higher priority than portPRIVILEGED_RAM_REGION, which would allow dynamic creation of unprivileged tasks, this could potentially be a pretty tough issue to debug.

All that said, I don't think this is something worth holding up this PR over.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, you are right. Stack memory needs to satisfy alignment requirements as we use an MPU region to guard that. I think it is best to supply these default implementations for non-MPU ports as alignment requirements very with the architecture.


*ppxIdleTaskTCBBuffer = &( xIdleTaskTCB );
*ppxIdleTaskStackBuffer = &( uxIdleTaskStack[ 0 ] );
*pulIdleTaskStackSize = configMINIMAL_STACK_SIZE;
}

#endif /* #if ( ( configSUPPORT_STATIC_ALLOCATION == 1 ) && ( configKERNEL_PROVIDED_STATIC_MEMORY == 1 ) && ( portUSING_MPU_WRAPPERS == 0 ) ) */
/*-----------------------------------------------------------*/

#if ( ( configSUPPORT_STATIC_ALLOCATION == 1 ) && ( configKERNEL_PROVIDED_STATIC_MEMORY == 1 ) && ( portUSING_MPU_WRAPPERS == 0 ) )

/*
* This is the kernel provided implementation of vApplicationGetTimerTaskMemory()
* to provide the memory that is used by the Timer service task. It is used when
* configKERNEL_PROVIDED_STATIC_MEMORY is set to 1. The application can provide
* it's own implementation of vApplicationGetTimerTaskMemory by setting
* configKERNEL_PROVIDED_STATIC_MEMORY to 0 or leaving it undefined.
*/
void vApplicationGetTimerTaskMemory( StaticTask_t ** ppxTimerTaskTCBBuffer,
StackType_t ** ppxTimerTaskStackBuffer,
uint32_t * pulTimerTaskStackSize )
{
static StaticTask_t xTimerTaskTCB;
static StackType_t uxTimerTaskStack[ configTIMER_TASK_STACK_DEPTH ];
aggarg marked this conversation as resolved.
Show resolved Hide resolved

*ppxTimerTaskTCBBuffer = &( xTimerTaskTCB );
*ppxTimerTaskStackBuffer = &( uxTimerTaskStack[ 0 ] );
*pulTimerTaskStackSize = configTIMER_TASK_STACK_DEPTH;
}

#endif /* #if ( ( configSUPPORT_STATIC_ALLOCATION == 1 ) && ( configKERNEL_PROVIDED_STATIC_MEMORY == 1 ) && ( portUSING_MPU_WRAPPERS == 0 ) ) */
/*-----------------------------------------------------------*/
Loading