Setup MPU for code relocated to SRAM #34086

mmahadevan108 · 2021-04-07T14:12:35Z

Create a static MPU map for code relocated to SRAM
Update the python script to provide proper alignment for the SRAM region
Setup the static MPU regions before PRE_KERNEL_1 and PRE_KERNEL_2 functions are invoked. This will setup the MPU for SRAM regions in case code relocated to SRAM and invoked from any of these functions.

kernel/init.c

arch/arm/include/aarch32/kernel_arch_func.h

ioannisg · 2021-04-07T15:56:00Z

arch/arm/core/aarch32/cortex_m/mpu/arm_core_mpu.c

This is no different from the RAMFUNC support, at least in the sense of memory attributes. Would it be possible to group these things together?

I don't know of a clean way to do this.

This would make sense if the SRAM region was always the relocation destination, but it's not. ITCM is another possibility for some platforms.

understood, thanks

ioannisg · 2021-04-07T16:02:41Z

arch/arm/core/aarch32/cortex_m/mpu/arm_core_mpu.c

Are these symbols defined anywhere?

They get generated at build time and are placed in build/zephyr/include/generated/linker_relocate.ld

This assumes we always relocate to the SRAM region, which isn't necessarily true. It's possible to relocate to other regions.

ioannisg · 2021-04-07T16:34:15Z

I need to have a closer look into this. For now, I could only comment that the refactor you're doing seems to break the support in NXP MPU to start the MPU stuff at PRE KERNEL 2.

Also, for the whole thing to work efficiently, you probably need to have the RAM area for relocation inside the RAM_SECTIONS - is it the case?

mmahadevan108 · 2021-04-07T17:53:56Z

RAM_SECTIONS

Relocation is done using zephyr_code_relocate(src/*.c SRAM) function which places the files in SRAM region. scripts/gen_relocate_app.py will generate the linker_relocate.ld and code_relocation.c files at build time.

mmahadevan108 · 2021-04-13T13:51:19Z

Rebased

mmahadevan108 · 2021-04-14T13:30:31Z

Any comments?

mmahadevan108 · 2021-04-14T20:47:58Z

I need to have a closer look into this. For now, I could only comment that the refactor you're doing seems to break the support in NXP MPU to start the MPU stuff at PRE KERNEL 2.

I have ran userspaces tests on the FRDM_K64F board and the tests pass. The tests I ran were

tests/kernel/mem_protect/userspace
samples/userspace/shared_mem
samples/userspace/hello_world_user
samples/userspace/prod_consumer/

ioannisg · 2021-04-15T14:45:42Z

I need to have a closer look into this. For now, I could only comment that the refactor you're doing seems to break the support in NXP MPU to start the MPU stuff at PRE KERNEL 2.

I have ran userspaces tests on the FRDM_K64F board and the tests pass. The tests I ran were
tests/kernel/mem_protect/userspace
samples/userspace/shared_mem
samples/userspace/hello_world_user
samples/userspace/prod_consumer/

I am not saying that the tests are breaking. I only commented that there was no option any more for POST KERNEL initialization of the module. :)

mmahadevan108 · 2021-04-16T14:01:07Z

I need to have a closer look into this. For now, I could only comment that the refactor you're doing seems to break the support in NXP MPU to start the MPU stuff at PRE KERNEL 2.

I have ran userspaces tests on the FRDM_K64F board and the tests pass. The tests I ran were
tests/kernel/mem_protect/userspace
samples/userspace/shared_mem
samples/userspace/hello_world_user
samples/userspace/prod_consumer/
I am not saying that the tests are breaking. I only commented that there was no option any more for POST KERNEL initialization of the module. :)

The NXP MPU driver was delaying the init to PRE_KERNEL2 for logging purposes. I think this was meant to help debugging during MPU driver init.

mmahadevan108 · 2021-04-19T13:43:48Z

Any questions on this PR?

mmahadevan108 · 2021-04-22T15:03:39Z

Any questions on this PR?

ioannisg · 2021-04-22T17:36:57Z

arch/arm/core/aarch32/cortex_m/mpu/arm_core_mpu.c

The reason that this generally works today is that all the "static" regions are located (forced by the linker, actually) in the beginning of SRAM, before the kernel ram (which includes the application memory). This placement is a requirement, currently, for Cortex-M33, because the static memory regions are actually partitioning the background SRAM, because overlapping regions are not allowed. (btw, you cannot currently call configure_static_regions for Cortex-M33 for a region that is not in RAM, unless i am missing something).

In ARMv7-M this is not a requirement, as the MPU static regions do not need to partition the SRAM, and you can configure static regions basically everywhere.

So I am not sure if this solution is generic enough to be accepted.

@MaureenHelm @mmahadevan108 did you try to test this PR on a Cortex-M33 with MPU? Especially if the relocation is not on SRAM. If so, I 'd like to hear how this works.

Besides this, I have no more objections for this change.

I am running this code on a Cortex-M33. The code is relocated to SRAM.

Right, but how about relocating to outside SRAM as @MaureenHelm was suggesting? I am not sure this would work right now for CM33, because of the way the mpu driver is expecting the static regions be in SRAM.

So, should we accept this solution, we must add a constraint that this won't work with relocation outside SRAM.

We relocate to ITCM instead of SRAM on i.MX RT1064, which is a Cortex-M7. Not an M33. We talked about it a bit in #31132 (comment) when I tried to add an MPU region for ITCM, but we then decided it wasn't necessary.

I recall it now, thanks.
In my opinion, we should make sure the solution that is presented here works in a generic way. If not, then we need to restrict it on the ARCHes that support it.

To address this issue I have created a new Kconfig called CODE_DATA_RELOCATION_SRAM. The changes made to arm_code_mpu.c file are now using #if defined(CONFIG_CODE_DATA_RELOCATION_SRAM) which allows the MPU entry to be added only when code is relocated to SRAM.

Kconfig.zephyr

1. This will help us identify if the relocation is to SRAM which is used when setting up the MPU entry for the SRAM region where code is relocated 2. Move CODE_DATA_RELOCATION configs to ARM specific folder Signed-off-by: Mahesh Mahadevan <[email protected]>

Use the CODE_DATA_RELOCATION_SRAM config to indentify code relocated to SRAM so we can setup the MPU for the SRAM region used for code relocation. Signed-off-by: Mahesh Mahadevan <[email protected]>

Code relocated to SRAM need MPU setup. Signed-off-by: Mahesh Mahadevan <[email protected]>

Code relocated using CONFIG_CODE_DATA_RELOCATION_SRAM should be allowed to execute from SRAM Signed-off-by: Mahesh Mahadevan <[email protected]>

Setup the static MPU regions before PRE_KERNEL_1 and PRE_KERNEL_2 functions are invoked. This will setup the MPU for SRAM regions in case code relocated to SRAM is invoked from any of these functions. Signed-off-by: Mahesh Mahadevan <[email protected]>

mmahadevan108 · 2021-05-03T14:42:53Z

Apologies to keep asking about this PR, I would like to get this in before the merge window closes. Also a few other open PR's are dependent on this.

ioannisg · 2021-05-04T12:15:19Z

arch/arm/core/aarch32/cortex_m/mpu/arm_core_mpu.c

+		/* RAM area for relocated text */
+		.start = (uint32_t)&__sram_text_start,
+		.size = (uint32_t)&__sram_text_size,
+		.attr = K_MEM_PARTITION_P_RX_U_RX,


This partition has exactly the same attribution as the ramfunc section. I wonder if that one could be re-used. Not against, for now, to accept adding one more region. But i really do not see why we need to have 2 static regions with the same attribution.

mmahadevan108 requested review from MaureenHelm, andyross, dcpleung, galak, ioannisg and nashif as code owners April 7, 2021 14:12

github-actions bot added area: ARM ARM (32-bit) Architecture area: Kernel labels Apr 7, 2021

mmahadevan108 mentioned this pull request Apr 7, 2021

Add support for flash controller with XIP on i.MX RT series #33701

Merged

ioannisg reviewed Apr 7, 2021

View reviewed changes

kernel/init.c Outdated Show resolved Hide resolved

ioannisg reviewed Apr 7, 2021

View reviewed changes

arch/arm/include/aarch32/kernel_arch_func.h Outdated Show resolved Hide resolved

ioannisg reviewed Apr 7, 2021

View reviewed changes

mmahadevan108 added the dev-review To be discussed in dev-review meeting label Apr 20, 2021

ioannisg reviewed Apr 22, 2021

View reviewed changes

MaureenHelm mentioned this pull request Apr 23, 2021

Add MIMXRT685 FlexSPI XIP support. #34502

Merged

mmahadevan108 requested a review from nvlsianpu as a code owner April 29, 2021 00:57

github-actions bot added area: Kconfig platform: NXP NXP labels Apr 29, 2021

ioannisg reviewed Apr 29, 2021

View reviewed changes

Kconfig.zephyr Outdated Show resolved Hide resolved

MaureenHelm approved these changes Apr 29, 2021

View reviewed changes

Kconfig.zephyr Outdated Show resolved Hide resolved

MaureenHelm added this to the v2.6.0 milestone Apr 29, 2021

mmahadevan108 mentioned this pull request Apr 30, 2021

MXRT600 add sleep mode support #34743

Merged

mmahadevan108 added 5 commits April 30, 2021 14:27

drivers: flash: MCUX: Split the relocate define based on target memory

c052951

Use the CODE_DATA_RELOCATION_SRAM config to indentify code relocated to SRAM so we can setup the MPU for the SRAM region used for code relocation. Signed-off-by: Mahesh Mahadevan <[email protected]>

scripts: Set align size for code relocated

7faafe9

Code relocated to SRAM need MPU setup. Signed-off-by: Mahesh Mahadevan <[email protected]>

arch: arm: Create a MPU entry for relocated code

370058b

Code relocated using CONFIG_CODE_DATA_RELOCATION_SRAM should be allowed to execute from SRAM Signed-off-by: Mahesh Mahadevan <[email protected]>

ioannisg self-assigned this May 3, 2021

ioannisg reviewed May 4, 2021

View reviewed changes

ioannisg approved these changes May 4, 2021

View reviewed changes

ioannisg merged commit d6b5023 into zephyrproject-rtos:master May 4, 2021

mmahadevan108 deleted the Setup_MPU_For_Relocate branch May 4, 2021 14:20

erwango mentioned this pull request May 11, 2021

stm32h7: systematic crash at each second boot with NETWORKING=y #35089

Closed

This was referenced May 14, 2021

kernel: mem_domain: test max available partitions on add_partition #35273

Closed

tests/kernel/mem_protect/mem_protect: frdm_k82f/frdm_k64f unexpected fatal error #34911

Closed

Setup MPU for code relocated to SRAM #34086

Setup MPU for code relocated to SRAM #34086

Uh oh!

Conversation

mmahadevan108 commented Apr 7, 2021

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ioannisg commented Apr 7, 2021

Uh oh!

mmahadevan108 commented Apr 7, 2021

Uh oh!

mmahadevan108 commented Apr 13, 2021

Uh oh!

mmahadevan108 commented Apr 14, 2021

Uh oh!

mmahadevan108 commented Apr 14, 2021

Uh oh!

ioannisg commented Apr 15, 2021

Uh oh!

mmahadevan108 commented Apr 16, 2021

Uh oh!

mmahadevan108 commented Apr 19, 2021

Uh oh!

mmahadevan108 commented Apr 22, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mmahadevan108 Apr 29, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

mmahadevan108 commented May 3, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

mmahadevan108 Apr 29, 2021 •

edited

Loading