-
Notifications
You must be signed in to change notification settings - Fork 6.8k
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.
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
arm: cortex a: cache management #53269
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it better to split the cache.c
to cortex_a_r/cache.c
and cortex_m/cache.c
cause there are too many #if defined
s
CC: @microbuilder @carlocaione @stephanosio
In my opinion this would make a lot of sense, I just didn't want to take that initiative on my own. |
go for it. |
Implement cache management functions not only for Cortex-M but also for Cortex-A. Use CMSIS low level calls. Signed-off-by: Théophile Ranquet <[email protected]>
73e4eda
to
19363fc
Compare
@carlocaione Mind having another look? |
uintptr_t addr = (uintptr_t)start_addr; | ||
uintptr_t end_addr = (uintptr_t)addr + size; | ||
|
||
addr /= CONFIG_DCACHE_LINE_SIZE; | ||
addr *= CONFIG_DCACHE_LINE_SIZE; | ||
|
||
for (; addr < end_addr; addr += CONFIG_DCACHE_LINE_SIZE) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we put this preamble in an inline helper maybe?
addr *= CONFIG_DCACHE_LINE_SIZE; | ||
|
||
for (; addr < end_addr; addr += CONFIG_DCACHE_LINE_SIZE) | ||
L1C_InvalidateDCacheMVA((void *)start_addr); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why are we acting on start_addr
?
addr /= CONFIG_DCACHE_LINE_SIZE; | ||
addr *= CONFIG_DCACHE_LINE_SIZE; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this correct? Let's say we have a 32 bytes cache line and you want to invalidate address 0x00000030
, according to this code you end up invalidating only 0x00000020
to 0x00000040
, if the memory content at 0x00000020
to 0x00000030
is dirty, you are corrupting the memory.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@yroeht FYI, it might help:)
zephyr/arch/arm64/core/cache.c
Lines 82 to 95 in 7015578
* For the data cache invalidate operation, clean and invalidate | |
* the partial cache lines at both ends of the given range to | |
* prevent data corruption. | |
* | |
* For example (assume cache line size is 64 bytes): | |
* There are 2 consecutive 32-byte buffers, which can be cached in | |
* one line like below. | |
* +------------------+------------------+ | |
* Cache line: | buffer 0 (dirty) | buffer 1 | | |
* +------------------+------------------+ | |
* For the start address not aligned case, when invalidate the | |
* buffer 1, the full cache line will be invalidated, if the buffer | |
* 0 is dirty, its data will be lost. | |
* The same logic applies to the not aligned end address. |
This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time. |
@yroeht Can you address some of the change requests here? |
Superseded by #56178 |
Implement cache management functions not only for Cortex-M but also for Cortex-A. Use CMSIS low level calls.
Signed-off-by: Théophile Ranquet [email protected]