arch: arm: cortex_a_r: Fix memory corruption when disabling dcache #89051
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
github-actions
bot
requested review from
MaureenHelm,
bbolen,
carlocaione,
galak,
ithinuel,
microbuilder,
stephanosio and
wearyzen
JarmouniA
requested changes
Apr 25, 2025
On the Xilinx MPSoC (Cortex-R5) platform, erratic operation was often seen when an operation which disabled the dcache, such as sys_reboot, was performed. Usually this manifested as an undefined instruction trap due to the CPU jumping to an invalid memory address. It appears the problem was due to dirty cache lines being present at the time the cache is disabled. Once the cache is disabled, the CPU will ignore the cache contents and read the possibly out-of-date data in main memory. Likewise, since the cache was being cleaned after it was already disabled, if the CPU had already written through changes to some memory locations, cleaning the cache at that point would potentially overwrite those changes with older data. The fact that the arch_dcache_flush_and_invd_all function was being called to do the cleaning and invalidation also contributed to this problem, because it is a non-inline function which means the compiler will generate memory writes to the stack when the function is called and returns. Corruption of the stack can result in the CPU ending up jumping to garbage addresses when trying to return from functions. To avoid this problem, the cache is now cleaned and invalidated prior to the dcache being disabled. This is done by directly calling the L1C_CleanInvalidateDCacheAll function, which, as it is declared as force inline, should help ensure there are no memory accesses, which would populate new cache lines, between the cache cleaning and disabling the cache. Ideally, for maximum safety, the cache cleaning and cache disabling should be done in assembler code, to guarantee that there are no memory accesses generated by the compiler during these operations. However, the present change does appear to solve this issue. Signed-off-by: Robert Hancock <robert.hancock@calian.com>
robhancocksed
force-pushed
the
cortex-a-r-fix-cache-disable-corruption
branch
from
b5f9482 to
8ec959f
Compare
JarmouniA
approved these changes
Apr 25, 2025
Contributor
Author
|
The twister automated tests are showing a failure of integration.kernel on qemu_cortex_a53/qemu_cortex_a53/smp. However, running the same test locally on this branch passes. I think it is some kind of spurious failure as the binary generated for that test does not even contain/call the code modified here. |
Contributor
Did you build it with |
Contributor
Author
I built and ran the test successfully locally using "west twister". |
wearyzen
approved these changes
Apr 30, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
On the Xilinx MPSoC (Cortex-R5) platform, erratic operation was often seen when an operation which disabled the dcache, such as sys_reboot, was performed. Usually this manifested as an undefined instruction trap due to the CPU jumping to an invalid memory address.
It appears the problem was due to dirty cache lines being present at the time the cache is disabled. Once the cache is disabled, the CPU will ignore the cache contents and read the possibly out-of-date data in main memory. Likewise, since the cache was being cleaned after it was already disabled, if the CPU had already written through changes to some memory locations, cleaning the cache at that point would potentially overwrite those changes with older data.
The fact that the arch_dcache_flush_and_invd_all function was being called to do the cleaning and invalidation also contributed to this problem, because it is a non-inline function which means the compiler will generate memory writes to the stack when the function is called and returns. Corruption of the stack can result in the CPU ending up jumping to garbage addresses when trying to return from functions.
To avoid this problem, the cache is now cleaned prior to the dcache being disabled. This is done by directly calling the L1C_CleanDCacheAll function, which, as it is declared as force inline, should help ensure there are no memory writes, which would create new dirty cache lines, between the cache cleaning and disabling the cache.
Ideally, for maximum safety, the cache cleaning and cache disabling should be done in assembler code, to guarantee that there are no memory writes generated by the compiler during these operations. However, the present change does appear to solve this issue.