bsp: cvitek: riscv: use marco for linker script #10202
Conversation
unicornx
added
BSP: Cvitek
There was a problem hiding this comment.
Pull Request Overview
This PR improves the memory configuration for the CVITEK RISC-V BSP by using macro replacements in the linker script and cleaning up related memory layout configurations. Key changes include updating the linker flags to use a generated linker script, revising kernel virtual address definitions and version numbers, and adding a new script (gen_ld.py) to dynamically generate the linker script.
Reviewed Changes
Copilot reviewed 8 out of 13 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| bsp/cvitek/cv18xx_risc-v/rtconfig.py | Updated the linker flags to reference the generated linker script. |
| bsp/cvitek/cv18xx_risc-v/rtconfig.h | Revised kernel virtual address and version definitions; removed redundant blocks. |
| bsp/cvitek/cv18xx_risc-v/gen_ld.py | Introduced a new script to generate the linker script dynamically. |
| bsp/cvitek/cv18xx_risc-v/board/mem_layout.h | Added memory layout definitions; note potential naming typos. |
| bsp/cvitek/cv18xx_risc-v/board/board.c | Adjusted the kernel start address references to use the new memory layout macros. |
| bsp/cvitek/cv18xx_risc-v/SConstruct | Removed the legacy stack linker script generation and integrated the new generation logic. |
| bsp/cvitek/README.md | Updated documentation to reflect the new default kernel virtual address. |
Files not reviewed (5)
- bsp/cvitek/cv18xx_risc-v/.config: Language not supported
- bsp/cvitek/cv18xx_risc-v/.gitignore: Language not supported
- bsp/cvitek/cv18xx_risc-v/link.lds: Language not supported
- bsp/cvitek/cv18xx_risc-v/link_smart.lds: Language not supported
- bsp/cvitek/cv18xx_risc-v/link_stacksize.lds: Language not supported
| #define KERAL_OFFSETT 0x200000 | ||
|
|
||
| #ifdef RT_USING_SMART | ||
| #define KERNEL_START KERNEL_VADDR_START + KERAL_OFFSETT | ||
| #define KERENL_SIZE 64 * 1024 * 1024 | ||
| #else | ||
| #define KERNEL_START 0x80000000 + KERAL_OFFSETT | ||
| #define KERENL_SIZE 32 * 1024 * 1024 |
There was a problem hiding this comment.
The macro name 'KERAL_OFFSETT' appears to be misspelled. Consider renaming it to 'KERNEL_OFFSET' for clarity and consistency.
| #define KERAL_OFFSETT 0x200000 | |
| #ifdef RT_USING_SMART | |
| #define KERNEL_START KERNEL_VADDR_START + KERAL_OFFSETT | |
| #define KERENL_SIZE 64 * 1024 * 1024 | |
| #else | |
| #define KERNEL_START 0x80000000 + KERAL_OFFSETT | |
| #define KERENL_SIZE 32 * 1024 * 1024 | |
| #define KERNEL_OFFSET 0x200000 | |
| #ifdef RT_USING_SMART | |
| #define KERNEL_START KERNEL_VADDR_START + KERNEL_OFFSET | |
| #define KERENL_SIZE 64 * 1024 * 1024 | |
| #else | |
| #define KERNEL_START 0x80000000 + KERNEL_OFFSET |
Copilot uses AI. Check for mistakes.
| #ifndef MEMORY_LAYOUT_H__ | ||
| #define MEMORY_LAYOUT_H__ | ||
|
|
||
| #include "../rtconfig.h" |
There was a problem hiding this comment.
这块
#include "rtconfig.h"就可以了吧
不行哎，我发现不加 “../”, 在 “bsp/cvitek/cv18xx_risc-v/gen_ld.py" 中会找不到 ”rtconfig.h“，导致无法生成 link.lds.generated。
There was a problem hiding this comment.
Pull Request Overview
This PR simplifies the linker script configuration for the CVITEK RISC-V BSP by replacing multiple script variants and hard-coded memory offsets with a macro‐based approach using gcc’s preprocessor. Key changes include updating linker flags in rtconfig.py, revising version and memory address definitions in rtconfig.h, and introducing a new ldscript generator (gen_ld.py) along with corresponding updates in memory layout and board files.
Reviewed Changes
Copilot reviewed 8 out of 13 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| bsp/cvitek/cv18xx_risc-v/rtconfig.py | Updated linker flags to use a generated linker script. |
| bsp/cvitek/cv18xx_risc-v/rtconfig.h | Revised version and kernel virtual address macros. |
| bsp/cvitek/cv18xx_risc-v/gen_ld.py | New script to generate linker scripts using the gcc preprocessor. |
| bsp/cvitek/cv18xx_risc-v/board/mem_layout.h | New header defining memory layout constants with kernel offset. |
| bsp/cvitek/cv18xx_risc-v/board/board.h | Removed obsolete heap and page memory definitions. |
| bsp/cvitek/cv18xx_risc-v/board/board.c | Updated memory descriptor to use the new memory layout header. |
| bsp/cvitek/cv18xx_risc-v/SConstruct | Integrated generation of the linker script into the build flow. |
| bsp/cvitek/README.md | Updated documentation to reflect the new kernel virtual address. |
Files not reviewed (5)
- bsp/cvitek/cv18xx_risc-v/.config: Language not supported
- bsp/cvitek/cv18xx_risc-v/.gitignore: Language not supported
- bsp/cvitek/cv18xx_risc-v/link.lds: Language not supported
- bsp/cvitek/cv18xx_risc-v/link_smart.lds: Language not supported
- bsp/cvitek/cv18xx_risc-v/link_stacksize.lds: Language not supported
| #define KERNEL_START KERNEL_VADDR_START + KERNEL_OFFSET | ||
| #define KERNEL_SIZE 64 * 1024 * 1024 | ||
| #else | ||
| #define KERNEL_START 0x80000000 + KERNEL_OFFSET | ||
| #define KERNEL_SIZE 32 * 1024 * 1024 |
There was a problem hiding this comment.
Consider wrapping the expression in parentheses, e.g., '#define KERNEL_START (KERNEL_VADDR_START + KERNEL_OFFSET)', to ensure correct operator precedence in all usage contexts.
| #define KERNEL_START KERNEL_VADDR_START + KERNEL_OFFSET | |
| #define KERNEL_SIZE 64 * 1024 * 1024 | |
| #else | |
| #define KERNEL_START 0x80000000 + KERNEL_OFFSET | |
| #define KERNEL_SIZE 32 * 1024 * 1024 | |
| #define KERNEL_START (KERNEL_VADDR_START + KERNEL_OFFSET) | |
| #define KERNEL_SIZE 64 * 1024 * 1024 | |
| #else | |
| #define KERNEL_START (0x80000000 + KERNEL_OFFSET) |
Copilot uses AI. Check for mistakes.
| child = subprocess.Popen(gcc_cmd + f' -E -P -x c {input} -o {output}', stdout=subprocess.PIPE, stderr=subprocess.PIPE, shell=True) | ||
|
|
There was a problem hiding this comment.
[nitpick] Consider using a consistent argument format for subprocess.Popen across platforms (e.g., always passing a list of arguments) to avoid potential argument parsing issues.
| child = subprocess.Popen(gcc_cmd + f' -E -P -x c {input} -o {output}', stdout=subprocess.PIPE, stderr=subprocess.PIPE, shell=True) | |
| child = subprocess.Popen([gcc_cmd, '-E', '-P', '-x', 'c', input, '-o', output], stdout=subprocess.PIPE, stderr=subprocess.PIPE, shell=True) |
Copilot uses AI. Check for mistakes.
There was a problem hiding this comment.
尝试不成功，linux 上无法预处理生成，保持原写法。
Regarding link script, some memory offsets and sizes are different between the standard version and the smart version (such as "__STACKSIZE__", kernel start address, kernel memory size, etc.). Original solution is replacing link scripts and use INCLUDE, which is relatively complicated. This improvement uses macros to replace constants in the link script. The preprocessing mechanism provided by gcc is used to dynamically replace constant values during the build process. In addition, the kernel load address (the corresponding KERNEL_VADDR_START for smart) was originally configured as 0xFFFFFFC000200000, which is default value of riscv with kernelmap enabled (0xffffffc000000000) plus offset to skip over bootloader (0x200000). This caused a trouble: due to default bsp configuration is for smart, if we switched to the standard version and build, and then switched back to the smart version, the value of KERNEL_VADDR_START will be default back to 0xffffffc000000000, which is different from the original configuration value, resulting in the need to manually reconfigure it, which is easy to forget and cause problems. The current solution is to use the default value 0xffffffc000000000 in the configuration. Add offset to the code and link script when using it. This patch update the default .config and rtconfig.h for cv18xx_riscv. Signed-off-by: Chen Wang <[email protected]>
原先的链接脚本为支持 标准版和 smart 版之间一些内存偏移量和大小不同的问题，(譬如 "STACKSIZE", 内核起始地址，内核所占内存大小等），采用替换链接脚本和 INCLUDE 的机制，比较复杂。
本次改进，在链接脚本中利用宏替换常量。利用 gcc 提供的预处理机制，在构建过程中动态替换常量值。
本次改动还顺便改好了一个以前的老问题：
Fixed #9697
原先对于内核加载地址（smart 的是对应的
KERNEL_VADDR_START），在配置中提前加上了 offset（0x200000），反而引起麻烦，导致如果先编译了一个smart版本，切换成标准版本，再切换回 smart 版本时，由于默认的KERNEL_VADDR_START值和配置值不同，导致需要手动再配置一遍，很容易遗忘导致问题。现在的解决方法是配置中采用默认的值0xffffffc000000000。具体使用时在代码和链接脚本中再加上 offset。本次提交还刷新了 "cv18xx_riscv" bsp 的默认的
.config和rtconfig.h文件。拉取/合并请求描述：(PR description)
[
为什么提交这份PR (why to submit this PR)
你的解决方案是什么 (what is your solution)
请提供验证的bsp和config (provide the config and bsp)
]
当前拉取/合并请求的状态 Intent for your PR
必须选择一项 Choose one (Mandatory):
代码质量 Code Quality：
我在这个拉取/合并请求中已经考虑了 As part of this pull request, I've considered the following:
#if 0代码，不包含已经被注释了的代码 All redundant code is removed and cleaned up