-
Notifications
You must be signed in to change notification settings - Fork 6.7k
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
Script to relocate code/data/bss sections to different memory types. #10892
Script to relocate code/data/bss sections to different memory types. #10892
Conversation
varun-sha
commented
Oct 28, 2018
•
edited by AdithyaBaglody
Loading
edited by AdithyaBaglody
Codecov Report
@@ Coverage Diff @@
## master #10892 +/- ##
=======================================
Coverage 51.43% 51.43%
=======================================
Files 231 231
Lines 28751 28751
Branches 7152 7152
=======================================
Hits 14789 14789
Misses 11144 11144
Partials 2818 2818
Continue to review full report at Codecov.
|
96a9c88
to
0edb095
Compare
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.
Script to relocate code/data/bss sections for particular library/app
to another memory(ccm and aon etc) or to other memory partition.
Code should not be generated unless there is no reasonable way
to write code in it's source langauge.
Could you elaborate on our options for satisfying the same use-case by
writing linker script code?
I would like to see that it was attempted, and then deemed unfeasible to write code
before one decided to generate code.
ace1c8b
to
71302cc
Compare
This are my initial commits with basic testing done on qemu_cortex_m3 & relocation checked with readelf, zephyr.map/lst files. To test script, i have created generic sram2 partition(just for test purpose), pls check below commit:- to relocate particular file to particular memory or partition , add below property in CMakeLists.txt |
[Varun]=> Function/variables can be placed into specfic memory using source code by http://www.keil.com/support/man/docs/armlink/armlink_pge1362066000009.htm So i have created scripts/gen_relocate_app.py which will create another ld file called linker_relocate.ld, which has all sections which need to be relocated, extracted from obj files & will be included in main linker.cmd. It will be enabled/disabled based on property values set in CMakeLists.txt.. |
2c7dcb6
to
9c07de0
Compare
I see this is marked as RFC, what more work needs to be done before it is in a merge-able state? |
I see that this is limited in scope to the 'app' libraries object files. Note that for the 'app' library one can use custom-rwdata.ld etc., see the Kconfig options in zephyr/misc/Kconfig. This is not affected by "cluttering the code with ifdef's" as far as I can tell. Unless we can prove that there are problems with using the existing mechanism we should not introduce a code generator that covers an already covered use-case. |
Its not specific to "app" only, kernel files can be be relocated if zephyr_kernel_code_relocate(file mem_type) is called from kernel specific CMakeList.txt |
This was initial commit to get reviewers feedback , was initially tested on qemu_cortex_m3. |
bbe6e54
to
c6a856f
Compare
5b0ae8f
to
160aa15
Compare
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.
This document is better placed in the Developer Guide area rather than in the top-level toc.
|
||
Overview | ||
******** | ||
This script will relocate .text, .rodata, .data and .bss sections from |
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.
This script will relocate .text, .rodata, .data and .bss sections from | |
This script will relocate .text, .rodata, .data, and .bss sections from |
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.
@dbkinder I don't understand why there needs to be a comma before an "and" in this case. Hence I haven't fixed this.
Others are done. Please re-review.
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.
With so many contributors, we've tried to standardize on some simple editorial guidelines such as using English spellings (vs. British) and using the Microsoft Writing Style Guide and Merriam-Webster Dictionary as guidelines. Use of the Oxford (or serial) comma is the recommendation, to avoid potential confusion with lists.
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.
ohh okay. Changed to the recommendation.
@dbkinder Please re-review.
0a10c2e
to
e29d534
Compare
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.
LGTM, thanks.
kernel/Kconfig
Outdated
@@ -279,6 +279,13 @@ config WAITQ_DUMB | |||
|
|||
endchoice # WAITQ_ALGORITHM | |||
|
|||
config CODE_DATA_RELOCATION |
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 is this under kernel/Kconfig? Please move out of here to Kconfig.zephyr under linker options for example.
kernel/Kconfig
Outdated
depends on ARM | ||
help | ||
When selected this will relocate .text, data and .bss sections from | ||
required files and places it in the required memory region. |
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.
required files? required by what? Maybe specified file.
samples/test_relocation/sample.yaml
Outdated
regex: | ||
- "Hello World! (.*)" | ||
platform_whitelist: qemu_cortex_m3 mps2_an385 sam_e70_xplained | ||
tags: introduction |
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.
tags: linker
samples/test_relocation/sample.yaml
Outdated
@@ -0,0 +1,12 @@ | |||
sample: | |||
description: Code data relocation Sample | |||
name: test relocation |
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.
name: Code Relocation
@@ -0,0 +1,18 @@ | |||
cmake_minimum_required(VERSION 3.8.2) |
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.
Move this sample under samples/application_development
Also, do not call it test_***, if it is a test, it should not be under samples/
@@ -0,0 +1,370 @@ | |||
#!/usr/bin/env python3 |
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.
Please spend some time on this script and fix the syntax, you get it to a higher score, at least 8.00 :)
************* Module gen_relocate_app
scripts/gen_relocate_app.py:12:0: C0301: Line too long (110/100) (line-too-long)
scripts/gen_relocate_app.py:75:29: C0326: Exactly one space required before assignment
extern_linker_var_declaration= """
^ (bad-whitespace)
scripts/gen_relocate_app.py:101:15: C0326: Exactly one space required before assignment
memset_template= """
^ (bad-whitespace)
scripts/gen_relocate_app.py:108:26: C0326: No space allowed after bracket
full_lib = ELFFile( f)
^ (bad-whitespace)
scripts/gen_relocate_app.py:109:0: C0325: Unnecessary parens after 'if' keyword (superfluous-parens)
scripts/gen_relocate_app.py:110:40: C0326: Exactly one space required after comma
print("Error parsing file: ",filename)
^ (bad-whitespace)
scripts/gen_relocate_app.py:113:19: C0326: No space allowed after bracket
sections = [ x for x in full_lib.iter_sections()]
^ (bad-whitespace)
scripts/gen_relocate_app.py:118:0: C0325: Unnecessary parens after 'if' keyword (superfluous-parens)
scripts/gen_relocate_app.py:121:0: C0325: Unnecessary parens after 'if' keyword (superfluous-parens)
scripts/gen_relocate_app.py:124:0: C0325: Unnecessary parens after 'if' keyword (superfluous-parens)
scripts/gen_relocate_app.py:127:0: C0325: Unnecessary parens after 'if' keyword (superfluous-parens)
scripts/gen_relocate_app.py:135:0: C0325: Unnecessary parens after 'if' keyword (superfluous-parens)
scripts/gen_relocate_app.py:136:26: C0326: No space allowed after bracket
symbols = [ x for x in section.iter_symbols()]
^ (bad-whitespace)
scripts/gen_relocate_app.py:139:0: C0301: Line too long (120/100) (line-too-long)
scripts/gen_relocate_app.py:150:49: C0326: Exactly one space required after comma
memory_type = memory_type.replace("_TEXT","")
^ (bad-whitespace)
scripts/gen_relocate_app.py:153:51: C0326: Exactly one space required after comma
memory_type = memory_type.replace("_RODATA","")
^ (bad-whitespace)
scripts/gen_relocate_app.py:156:49: C0326: Exactly one space required after comma
memory_type = memory_type.replace("_DATA","")
^ (bad-whitespace)
scripts/gen_relocate_app.py:159:48: C0326: Exactly one space required after comma
memory_type = memory_type.replace("_BSS","")
^ (bad-whitespace)
scripts/gen_relocate_app.py:167:0: C0330: Wrong continued indentation before block (add 4 spaces).
full_list_of_sections[iter_sec] != []):
^ | (bad-continuation)
scripts/gen_relocate_app.py:173:64: C0326: No space allowed before bracket
tmp_list = {"text":[], "rodata":[], "data":[], "bss":[] }
^ (bad-whitespace)
scripts/gen_relocate_app.py:176:0: C0330: Wrong continued indentation before block (add 4 spaces).
full_list_of_sections[iter_sec] != []):
^ | (bad-continuation)
scripts/gen_relocate_app.py:197:0: C0325: Unnecessary parens after 'if' keyword (superfluous-parens)
scripts/gen_relocate_app.py:229:21: C0326: Exactly one space required after assignment
generate_section ={"text":False, "rodata":False, "data":False, "bss":False }
^ (bad-whitespace)
scripts/gen_relocate_app.py:229:79: C0326: No space allowed before bracket
generate_section ={"text":False, "rodata":False, "data":False, "bss":False }
^ (bad-whitespace)
scripts/gen_relocate_app.py:233:58: C0326: Exactly one space required after comma
memory_type = memory_type.replace(section_name,"")
^ (bad-whitespace)
scripts/gen_relocate_app.py:244:24: C0326: Exactly one space required after comma
for mtype in ["text","rodata", "data"]:
^ (bad-whitespace)
scripts/gen_relocate_app.py:263:0: W0301: Unnecessary semicolon (unnecessary-semicolon)
scripts/gen_relocate_app.py:287:0: C0330: Wrong continued indentation (add 12 spaces).
help="input src:memory type(sram2 or ccm or aon etc) string")
^ | (bad-continuation)
scripts/gen_relocate_app.py:291:67: C0326: No space allowed before keyword argument assignment
parser.add_argument("-v", "--verbose", action="count", default =0,
^ (bad-whitespace)
scripts/gen_relocate_app.py:304:51: C0326: Exactly one space required after comma
fullname = os.path.join(dirpath,filename1)
^ (bad-whitespace)
scripts/gen_relocate_app.py:325:46: C0326: Exactly one space required after comma
print("Memory region ", mem_region," Selected for file:", file_name_list)
^ (bad-whitespace)
scripts/gen_relocate_app.py:331:0: C0325: Unnecessary parens after 'return' keyword (superfluous-parens)
scripts/gen_relocate_app.py:346:20: C0326: Exactly one space required after comma
for memory_type,files in rel_dict.items():
^ (bad-whitespace)
scripts/gen_relocate_app.py:347:30: C0326: Exactly one space required after assignment
full_list_of_sections ={"text":[], "rodata":[], "data":[], "bss":[] }
^ (bad-whitespace)
scripts/gen_relocate_app.py:347:76: C0326: No space allowed before bracket
full_list_of_sections ={"text":[], "rodata":[], "data":[], "bss":[] }
^ (bad-whitespace)
scripts/gen_relocate_app.py:1:0: C0111: Missing module docstring (missing-docstring)
scripts/gen_relocate_app.py:37:0: C0103: Constant name "print_template" doesn't conform to UPPER_CASE naming style (invalid-name)
scripts/gen_relocate_app.py:41:0: C0103: Constant name "group_start_end_memory_seq" doesn't conform to UPPER_CASE naming style (invalid-name)
scripts/gen_relocate_app.py:45:0: C0103: Constant name "section_load_memory_seq" doesn't conform to UPPER_CASE naming style (invalid-name)
scripts/gen_relocate_app.py:49:0: C0103: Constant name "load_address_location_flash" doesn't conform to UPPER_CASE naming style (invalid-name)
scripts/gen_relocate_app.py:50:0: C0103: Constant name "load_address_location_bss" doesn't conform to UPPER_CASE naming style (invalid-name)
scripts/gen_relocate_app.py:53:0: C0103: Constant name "linker_section_seq" doesn't conform to UPPER_CASE naming style (invalid-name)
scripts/gen_relocate_app.py:68:0: C0103: Constant name "source_code_includes" doesn't conform to UPPER_CASE naming style (invalid-name)
scripts/gen_relocate_app.py:75:0: C0103: Constant name "extern_linker_var_declaration" doesn't conform to UPPER_CASE naming style (invalid-name)
scripts/gen_relocate_app.py:82:0: C0103: Constant name "data_copy_function" doesn't conform to UPPER_CASE naming style (invalid-name)
scripts/gen_relocate_app.py:89:0: C0103: Constant name "bss_zeroing_function" doesn't conform to UPPER_CASE naming style (invalid-name)
scripts/gen_relocate_app.py:96:0: C0103: Constant name "memcpy_template" doesn't conform to UPPER_CASE naming style (invalid-name)
scripts/gen_relocate_app.py:101:0: C0103: Constant name "memset_template" doesn't conform to UPPER_CASE naming style (invalid-name)
scripts/gen_relocate_app.py:106:0: C0111: Missing function docstring (missing-docstring)
scripts/gen_relocate_app.py:107:33: C0103: Variable name "f" doesn't conform to snake_case naming style (invalid-name)
scripts/gen_relocate_app.py:111:12: E1101: Module 'os' has no 'exit' member; maybe '_exit'? (no-member)
scripts/gen_relocate_app.py:144:0: C0111: Missing function docstring (missing-docstring)
scripts/gen_relocate_app.py:184:0: C0111: Missing function docstring (missing-docstring)
scripts/gen_relocate_app.py:185:4: W0621: Redefining name 'string' from outer scope (line 29) (redefined-outer-name)
scripts/gen_relocate_app.py:190:0: C0111: Missing function docstring (missing-docstring)
scripts/gen_relocate_app.py:192:4: W0621: Redefining name 'string' from outer scope (line 29) (redefined-outer-name)
scripts/gen_relocate_app.py:214:0: C0111: Missing function docstring (missing-docstring)
scripts/gen_relocate_app.py:215:4: W0621: Redefining name 'string' from outer scope (line 29) (redefined-outer-name)
scripts/gen_relocate_app.py:223:36: C0103: Variable name "fw" doesn't conform to snake_case naming style (invalid-name)
scripts/gen_relocate_app.py:226:0: C0111: Missing function docstring (missing-docstring)
scripts/gen_relocate_app.py:258:0: C0111: Missing function docstring (missing-docstring)
scripts/gen_relocate_app.py:259:4: W0621: Redefining name 'string' from outer scope (line 29) (redefined-outer-name)
scripts/gen_relocate_app.py:275:35: C0103: Variable name "fw" doesn't conform to snake_case naming style (invalid-name)
scripts/gen_relocate_app.py:279:0: C0111: Missing function docstring (missing-docstring)
scripts/gen_relocate_app.py:280:4: C0103: Constant name "args" doesn't conform to UPPER_CASE naming style (invalid-name)
scripts/gen_relocate_app.py:280:4: W0601: Global variable 'args' undefined at the module level (global-variable-undefined)
scripts/gen_relocate_app.py:296:0: C0111: Missing function docstring (missing-docstring)
scripts/gen_relocate_app.py:296:0: R1710: Either all return statements in a function should return an expression, or none of them should. (inconsistent-return-statements)
scripts/gen_relocate_app.py:300:17: W0612: Unused variable 'dirs' (unused-variable)
scripts/gen_relocate_app.py:309:0: C0111: Missing function docstring (missing-docstring)
scripts/gen_relocate_app.py:314:8: E1101: Module 'os' has no 'exit' member; maybe '_exit'? (no-member)
scripts/gen_relocate_app.py:334:0: C0111: Missing function docstring (missing-docstring)
scripts/gen_relocate_app.py:342:4: W0612: Unused variable 'raw' (unused-variable)
scripts/gen_relocate_app.py:25:0: W0611: Unused import sys (unused-import)
scripts/gen_relocate_app.py:28:0: W0611: Unused import re (unused-import)
scripts/gen_relocate_app.py:29:0: W0611: Unused import string (unused-import)
scripts/gen_relocate_app.py:31:0: C0411: standard import "import glob" should be placed before "from elftools.elf.elffile import ELFFile" (wrong-import-order)
scripts/gen_relocate_app.py:32:0: C0411: standard import "import warnings" should be placed before "from elftools.elf.elffile import ELFFile" (wrong-import-order)
-----------------------------------
Your code has been rated at 5.35/10
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.
@nashif from where can I run this?
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.
pylint
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.
@nashif comments handled. please re-review.
e29d534
to
4f1ef3c
Compare
This script will relocate .text .data and .bss sections from required files and places it in the required memory region. This memory region and file are given to this python script in the form of a string. Refer to the script for the format of this string and the procedure to invoke it. The main goal of this script is to provide a robust way to re-order the memory contents without actually having to modify the code (C source code and the linker code). In simple terms this script will do the job of __attribute__((section("name"))) for a bunch of files together. Signed-off-by: Varun Sharma <[email protected]> Signed-off-by: Adithya Baglody <[email protected]>
This patch creates a rule in the cmake to trigger the generation of linker_relocate.ld and code_relocation.c files. The linker_relocate.ld will create appropriate sections and will link the required functions or variables from all the selected files. The code_relocation.c will have code that is needed for initializing data sections and copy of text sections(if XIP). Also this will contain code that is needed for zeroing of bss. The procedure to invoke this feature is: 1. Enable CONFIG_CODE_RELOCATION in the prj.conf 2. Inside CMakeList.txt in the project we need to mention all the files that needs to get relocated. zephyr_kernel_code_relocate(src/*.c SRAM2) Where the first argument is the file/files and the second argument is the memory where it has be placed. NOTE: The file argument supports glob expressions. NOTE: Step 2 can be done as many times as required. And relative paths can be given here. Signed-off-by: Adithya Baglody <[email protected]>
This patch splits the text section into 2 parts. The first section will have some info regarding vector tables and debug info. The second section will have the complete text section. This is needed to force the required functions and data variables the correct locations. This is due to the behavior of the linker. The linker will only link once and hence this text section had to be split to make room for the generated linker script. Added a new Kconfig CODE_DATA_RELOCATION which when enabled will invoke the script, which does the required relocation. Added hooks inside init.c for bss zeroing and data copy operations. Needed when we have to copy data from ROM to required memory type. Signed-off-by: Adithya Baglody <[email protected]>
When code relocation feature with userspace mode is turned on we need a bit more memory to fit the text section for these 2 generated files. Signed-off-by: Adithya Baglody <[email protected]>
This sample provides an example for using the code relocation feature. This example will place text,data,bss from 3 files to various parts in the SRAM. For this a custom linker file is used which is derived from include/arch/arm/cortex_m/scripts/linker.ld. Signed-off-by: Varun Sharma <[email protected]> Signed-off-by: Adithya Baglody <[email protected]>
Added basic working details of code/data/bss relocation feature, how to use with examples & code sample details Signed-off-by: Varun Sharma <[email protected]>
This patch ensures that there are no .common variables in the test case. Signed-off-by: Adithya Baglody <[email protected]>
The header ipm_quark_se.h was creating a object. Hence removed it and placed the same in ipm_quark_se.c Signed-off-by: Adithya Baglody <[email protected]>
This header was creating an object instead of a typdef. Fixed the bug. Signed-off-by: Adithya Baglody <[email protected]>
This header was actually creating a struct in the bss. Fixed it. Signed-off-by: Adithya Baglody <[email protected]>
Instead of creating an typedef enum it was creating an object. Signed-off-by: Adithya Baglody <[email protected]>
These tests were creating objects from header file in the bss. Fixed it by moving the objects to appropriate object files. Signed-off-by: Adithya Baglody <[email protected]>
This sample was creating objects from header file in the bss. Fixed it by moving the objects to appropriate object files. Signed-off-by: Adithya Baglody <[email protected]>
This is needed to force the .common type to .bss type. This will force the .common variables to go into bss section in the compiled object file. As of now the movement to the .bss section was happening only during the final linking stage. The -fno-common option specifies that the compiler should place uninitialized global variables in the data section of the object file, rather than generating them as common blocks. This has the effect that if the same variable is declared (without "extern") in two different compilations, you will get a multiple-definition error when you link them. e.g: // file a.c // file-scope int b; // file b.c // file-scope int b; If there exist two non-extern declarations of the same variable then no-common will cause a linker error. This sequence would have compiled before, but will now cause a linker error. Signed-off-by: Adithya Baglody <[email protected]>
4f1ef3c
to
ff312b3
Compare
@nashif can we merge this now. |
@@ -0,0 +1,42 @@ | |||
/* | |||
* Copyright (c) 2013-2014 Wind River Systems, Inc. | |||
* |
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.
Shouldn't the copyright be updated? Same with few more files.