FIXED BUG configKERNEL_INTERRUPT_PRIORITY must be left shifted accord… by cversek · Pull Request #64 · stm32duino/STM32FreeRTOS · GitHub
Skip to content

FIXED BUG configKERNEL_INTERRUPT_PRIORITY must be left shifted accord…#64

Merged
fpistm merged 1 commit into
stm32duino:mainfrom
cversek:main
Jun 6, 2023
Merged

FIXED BUG configKERNEL_INTERRUPT_PRIORITY must be left shifted accord…#64
fpistm merged 1 commit into
stm32duino:mainfrom
cversek:main

Conversation

@cversek

@cversek cversek commented Jun 5, 2023

Copy link
Copy Markdown

The value of setting configKERNEL_INTERRUPT_PRIORITY is interpreted at a low level on Cortex M ports as a priority setting mask, so it must be left shifted to the most significant bits.

REF: "Cortex-M Internal Priority Representation" https://www.freertos.org/RTOS-Cortex-M3-M4.html

The configMAX_SYSCALL_INTERRUPT_PRIORITY and configKERNEL_INTERRUPT_PRIORITY settings found in FreeRTOSConfig.h require their priority values to be specified as the ARM Cortex-M core itself wants them - already shifted to the most significant bits of the byte. That is why configKERNEL_INTERRUPT_PRIORITY, which should be set to the lowest interrupt priority, is set to 255 (1111 1111 in binary) in the FreeRTOSConfig.h header files delivered with each official FreeRTOS demo. The values are specified this way for a number of reasons: The RTOS kernel accesses the ARM Cortex-M3 hardware directly (without going through any third party library function), the RTOS kernel implementation pre-dates most library function implementations, and this was the scheme used by the first ARM Cortex-M3 libraries to come to market.

The line brought in at commit 0ab4bbd9507f2c8fd15ae1ca755aee6334860ad9:

#define configKERNEL_INTERRUPT_PRIORITY 14

is a BUG, since it sets the kernel preemption interrupt to a very high priority (numerically low). This can lead to subtle memory corruption issues which are fairly hard to debug. The proper value consistent with how this priority setting is used by the port and the intention of the mistaken commit to lower the value (raise the logical priority) from the actual lowest priority level of 15 would thus be:

#define configKERNEL_INTERRUPT_PRIORITY   ( 14 << (8 - configPRIO_BITS) )

Background:
I discovered this bug, because I had used the Github main branch as a reference to make my own STM32FreeRTOSConfig.h file. I was experiencing persistent hard faults and submitted this issue for help. Eventually, I solved this problem when I accidentally remade my config from an older stable version before the above mentioned commit. I was confused about why one of my test systems became stable but the other didn't. I eventually diffed the configs to find out this mistaken interrupt priority configuration in the main branch.

@jmailloux

Copy link
Copy Markdown

@fpistm fpistm added the bug Something isn't working label Jun 6, 2023
@fpistm fpistm added this to the 10.3.2 milestone Jun 6, 2023
@fpistm

fpistm commented Jun 6, 2023

Copy link
Copy Markdown
Member

Hi @cversek
I've talked about this with @ABOSTM yesterday and planned to do a PR today 😉
You're right, it is a bug. We made a mistake 😩
Thanks for the fix. Fortunately, it was not release.

@fpistm fpistm merged commit 6d463ec into stm32duino:main Jun 6, 2023
@cversek

cversek commented Jun 6, 2023

Copy link
Copy Markdown
Author

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants