scripts: align sram .text section when userspace and code reloation enabled #17042

wentongwu · 2019-06-25T10:31:18Z

1) Code relocation causes comipler randomly generates _stub that will
cause different memory layout in the magic building process for user
mode. And the different memory layout will generate wrong hash table
in priv_stacks_hash.c and it will cause user mode doesn't work.
Align sram .text section when userspace and code relocation enabled
to generate same .rodata, .data and .bss sections in sram region.
User can adjust Kconfig RELOCATION_SRAM_TEXT_ALIGN_SIZE to make less
wasting memory.

2) make mpu align inside sections instead of outside to
avoid overlap.

3) fix spell typo in target_relocation.cmake.

Fixes: #17038.

zephyrbot · 2019-06-25T10:32:36Z

All checks are passing now.

Review history of this comment for details about previous failed status.
Note that some checks might have not completed yet.

wentongwu · 2019-06-26T22:06:45Z

@andrewboie @ioannisg @nashif could you please take a look this change?Thanks

andrewboie · 2019-06-27T00:51:40Z

I think I understand this patch, can you describe in a bit more detail how the memory map is shifting before this fix was applied?

wentongwu · 2019-06-27T06:36:05Z

I think I understand this patch, can you describe in a bit more detail how the memory map is shifting before this fix was applied?

@andrewboie please see issue #17038 .

wentongwu · 2019-06-27T16:40:31Z

push again to solve conflict with master.

andrewboie · 2019-06-27T19:11:03Z

OK, thanks for the further details.
If I understand correctly, the issue is that the ".text.sedi_pm_pmu2nvic_isr.__stub" gets added to the text section in zephyr.elf, but not zephyr_prebuilt.elf. Something is causing this to be pulled into only the final binary. At this time it is not known what is causing this to happen. This patch is a workaround to reserve some room in the prebuilt binary for text area expansion, so that the memory addresses don't change in between the two builds.

wentongwu · 2019-06-27T21:37:32Z

shippable fail not related to my change

up_squared/samples/boards/up_squared/gpio_counter/sample.board.up_squared.gpio_counter failure: build_error
qemu_xtensa/tests/kernel/sched/preempt/kernel.sched.preempt failure: timeout
up_squared/samples/synchronization/sample.synchronization failure: build_error

Re-submit.

wentongwu · 2019-06-27T21:49:06Z

OK, thanks for the further details.
If I understand correctly, the issue is that the ".text.sedi_pm_pmu2nvic_isr.__stub" gets added to the text section in zephyr.elf, but not zephyr_prebuilt.elf. Something is causing this to be pulled into only the final binary. At this time it is not known what is causing this to happen. This patch is a workaround to reserve some room in the prebuilt binary for text area expansion, so that the memory addresses don't change in between the two builds.

@andrewboie yes, that's workaround for it so far.
could you please take a look this PR again, there is no code change for the re-submit, just to pass the shippable. Thanks a lot.

andrewboie · 2019-06-27T22:45:07Z

What steps have you taken to isolate why the extra data in the text section is being generated for zephyr.elf and not zephyr_prebuilt.elf? Where are these symbols coming from? Are they somehow being pulled in due to the gperf code we add in this phase?

andrewboie · 2019-06-27T22:47:08Z

Kconfig.zephyr

+	default 80000
+	depends on ARCH_HAS_USERSPACE && CODE_DATA_RELOCATION
+	help
+	  Align sram text section when userspace and code relocation


This help text really doesn't explain what is going on. You need to expand on this.

andrewboie · 2019-06-27T22:47:25Z

Kconfig.zephyr

+	help
+	  Align sram text section when userspace and code relocation
+	  enabled to generate same .rodata, .data and .bss sections
+	  in sram region for the magic building process.


"magic building process" is NOT a technical term

andrewboie · 2019-06-27T22:48:24Z

Kconfig.zephyr

@@ -82,6 +82,15 @@ config CODE_DATA_RELOCATION
 	  files should be specified in the CMakeList.txt file with
 	  a cmake API zephyr_code_relocation().

+config RELOCATION_SRAM_TEXT_ALIGN_SIZE
+	int "code relocation sram text section align size"
+	default 80000


What does this mean? We are by default reserving 80 Kilobytes of space? Why so much, given that the offset in the binary was only 8 bytes? The help text needs to clearly explain the semantics on the value we set here.

andrewboie · 2019-06-27T22:48:58Z

cmake/linker/ld/target_relocation.cmake

@@ -3,28 +3,31 @@
 # See root CMakeLists.txt for description and expectations of these macros

 macro(toolchain_ld_relocation)
-  set(MEM_RELOCATAION_LD   "${PROJECT_BINARY_DIR}/include/generated/linker_relocate.ld")
-  set(MEM_RELOCATAION_SRAM_DATA_LD
+  set(MEM_RELOCATION_LD   "${PROJECT_BINARY_DIR}/include/generated/linker_relocate.ld")


Did you make some whitespace changes or something? I can't tell the difference between the before and after, it looks like the code is unchanged.

Disregard, I see it now

wentongwu · 2019-06-28T10:40:00Z

What steps have you taken to isolate why the extra data in the text section is being generated for zephyr.elf and not zephyr_prebuilt.elf? Where are these symbols coming from? Are they somehow being pulled in due to the gperf code we add in this phase?

@andrewboie @jocelyn-li @nashif One method I used is to put *stub to one separate section and do small align, but it doesn't work. Another idea is trying to put the .text section behind .data section, so the extra code will not impact .data section, I didn't do this one in afraid of broken other boards. And finally it's the workaround method, it works well, but now I know it doesn't fit the opensource spirit, I'm sorry for that.

Look at it again today, I find the extra stubs are comprised of veneer functions, that's because the distance between calling code and the called function is too far(because of relocation, also there are other reasons to cause the far distance), the linker will automatically generate and insert veneer functions. Now I'm trying to figure out the solution. And now I want to close this PR.

Code relocation causes comipler randomly generates _stub that will cause different memory layout in the magic building process for user mode. And the different memory layout will generate wrong hash table in priv_stacks_hash.c and it will cause user mode doesn't work. Align sram .text section when userspace and code relocation enabled to generate same .rodata, .data and .bss sections in sram region. User can adjust Kconfig RELOCATION_SRAM_TEXT_ALIGN_SIZE to make less wasting memory. Fixes: zephyrproject-rtos#17038. Signed-off-by: Wentong Wu <[email protected]>

make mpu align inside sections instead of outside to avoid overlap. Signed-off-by: Wentong Wu <[email protected]>

fix spell typo in target_relocation.cmake. Signed-off-by: Wentong Wu <[email protected]>

wentongwu requested review from ioannisg and andrewboie June 25, 2019 10:31

wentongwu requested review from galak, MaureenHelm, nashif and SebastianBoe as code owners June 25, 2019 10:31

zephyrbot added area: API Changes to public APIs area: Build System area: Kconfig labels Jun 25, 2019

wentongwu force-pushed the non_xip_relocate branch from e01117e to 005e781 Compare June 25, 2019 10:33

wentongwu mentioned this pull request Jun 27, 2019

code relocation generating different memory layout cause user mode not working #17038

Closed

wentongwu added the backport v1.14-branch label Jun 27, 2019

wentongwu force-pushed the non_xip_relocate branch from 005e781 to c8fa054 Compare June 27, 2019 16:39

wentongwu force-pushed the non_xip_relocate branch from c8fa054 to 1304789 Compare June 27, 2019 21:38

andrewboie requested a review from dcpleung June 27, 2019 22:46

andrewboie reviewed Jun 27, 2019

View reviewed changes

wentongwu force-pushed the non_xip_relocate branch from 1304789 to f319dc0 Compare June 28, 2019 05:53

wentongwu removed the backport v1.14-branch label Jun 28, 2019

wentongwu closed this Jun 28, 2019

wentongwu added 2 commits June 28, 2019 21:50

scripts: make mpu align in sections.

f1772d0

make mpu align inside sections instead of outside to avoid overlap. Signed-off-by: Wentong Wu <[email protected]>

target_relocation.cmake: fix spell typo

f319dc0

fix spell typo in target_relocation.cmake. Signed-off-by: Wentong Wu <[email protected]>

wentongwu deleted the non_xip_relocate branch September 20, 2019 15:04

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

scripts: align sram .text section when userspace and code reloation enabled #17042

scripts: align sram .text section when userspace and code reloation enabled #17042

wentongwu commented Jun 25, 2019 •

edited

Loading

zephyrbot commented Jun 25, 2019 •

edited

Loading

wentongwu commented Jun 26, 2019

andrewboie commented Jun 27, 2019

wentongwu commented Jun 27, 2019

wentongwu commented Jun 27, 2019

andrewboie commented Jun 27, 2019

wentongwu commented Jun 27, 2019

wentongwu commented Jun 27, 2019 •

edited

Loading

andrewboie commented Jun 27, 2019 •

edited by wentongwu

Loading

andrewboie Jun 27, 2019

andrewboie Jun 27, 2019

andrewboie Jun 27, 2019

andrewboie Jun 27, 2019

andrewboie Jun 27, 2019

wentongwu commented Jun 28, 2019

scripts: align sram .text section when userspace and code reloation enabled #17042

scripts: align sram .text section when userspace and code reloation enabled #17042

Conversation

wentongwu commented Jun 25, 2019 • edited Loading

zephyrbot commented Jun 25, 2019 • edited Loading

wentongwu commented Jun 26, 2019

andrewboie commented Jun 27, 2019

wentongwu commented Jun 27, 2019

wentongwu commented Jun 27, 2019

andrewboie commented Jun 27, 2019

wentongwu commented Jun 27, 2019

wentongwu commented Jun 27, 2019 • edited Loading

andrewboie commented Jun 27, 2019 • edited by wentongwu Loading

andrewboie Jun 27, 2019

Choose a reason for hiding this comment

andrewboie Jun 27, 2019

Choose a reason for hiding this comment

andrewboie Jun 27, 2019

Choose a reason for hiding this comment

andrewboie Jun 27, 2019

Choose a reason for hiding this comment

andrewboie Jun 27, 2019

Choose a reason for hiding this comment

wentongwu commented Jun 28, 2019

wentongwu commented Jun 25, 2019 •

edited

Loading

zephyrbot commented Jun 25, 2019 •

edited

Loading

wentongwu commented Jun 27, 2019 •

edited

Loading

andrewboie commented Jun 27, 2019 •

edited by wentongwu

Loading