Riscv port basic thread #137
Conversation
Add RISC-V architecture support for running on RP2350 with Pico SDK: - quirks/pico-sdk-riscv: Pico SDK RISC-V integration specifics - Custom CRT0 wrapper for IRQ exit context switch hook - ecall exception handler for CMRX syscalls - CMake integration for firmware builds - src/os/arch/riscv: Architecture-specific kernel components - sched.c: Thread stack setup, boot, and sleep primitives - mpu.c: Memory protection stubs (PMP not implemented) - static.c: Static thread/application table accessors - corelocal.h: Single-core lock/unlock via IRQ disable - sysenter.h: ecall-based syscall implementation - cmake/arch/riscv/hal: Build system support - CMRX.cmake: add_firmware() function - cmrx_sections.ld: Linker symbols for static init tables - src/lib/arch/riscv: Library support - mutex.c: Mutex implementation using HAL primitives
ventZl
left a comment
ventZl
left a comment
There was a problem hiding this comment.
Thank you for your submission!
Except of the requirement to move RISC-V-related tests into riscv subtree, all remaining comments are just general non-binding stuff and can be resolved later.
| os_notify_wait_object.c | ||
| os_notify_futex.c | ||
| os_shutdown.c | ||
| riscv_hal.c |
There was a problem hiding this comment.
These tests should probably live inside arch/riscv/hal/testing subdirectory, so they don't get included into test suite when RISC-V port is not being used.
No platform-specific code should be placed outside of arch subtree, being production or test code.
|
|
||
| /* Syscall/trap entry is platform-specific. */ | ||
| #define __SYSCALL | ||
| #define __SVC(x) return 0; |
There was a problem hiding this comment.
It would be better to return E_NOTAVAIL here rather than 0. It will make whatever code that calls it fail early and serve as a reminder that there is something waiting to be implemented.
| bool os_riscv_context_switch_is_pending(void); | ||
| void os_riscv_context_switch_safe_point(void); | ||
|
|
||
| #if defined(UNIT_TESTING) |
There was a problem hiding this comment.
If you move RISC-V related tests into separate test driver hosted under arch/riscv/hal/testing you can move this over there and get rid of #ifdefs, can't you?
| uint32_t *new_saved_sp = NULL; | ||
| uint32_t *new_sp_after = NULL; | ||
|
|
||
| riscv_setup_switch(&old_sp, &new_saved_sp, &new_sp_after); |
There was a problem hiding this comment.
You can use CTEST_DATA / CTEST_SETUP / CTEST2 macros to automate this. Many kernel tests use this pattern.
CTEST_DATA serves purpose of defining per-group specific test data. In most cases this is not used, so you keep this empty.
CTEST_SETUP is executed automatically before every test starts, so you can reinitialize system into clean state there without having to call anything explicitly.
Use of CTEST2 macro rather than CTEST one is needed to trigger this semantics.
CTEST_DATA/CTEST_SETUP are grouped per test group (1st argument). You can have as many you need.
| * - mepc points to ecall instruction, must be incremented by 4 to continue | ||
| */ | ||
|
|
||
| #define FRAME_OFFSET_A0 16 |
There was a problem hiding this comment.
Couldn't this be replaced by something like type ExceptionFrame in arch/arm/cmsis/arch/cortex.h ?
| @@ -0,0 +1,34 @@ | |||
| # CMRX quirk for Pico SDK RISC-V (RP2350) | |||
There was a problem hiding this comment.
The rpi-sdk code was sent to exile here because it was modifying some of pico-sdk targets but I don't see anything similar here. All changes are to the CMRX's own os target.
Thus the code here seems to be pretty generic and not specific to RPI SDK. Of course, various SDKs can call these handlers various names which will cause us to fail to bind to the vectors, but this can be addressed later I guess.
Which is a good news because bunch of the code can be moved into arch/riscv rather than live here.
I hope that in long run, the we could get rid of the whole quirks subtree as portability will be recognized as an advantage in the world of embedded SW development.
2 participants
Summary
This PR is my first attempt at scaffolding RISC-V support for CMRX thread switching, targeting to use RP2350 (Pico 2) with Pico SDK as the validation platform. The goal is to get a minimal context switcher + syscall/trap path wired enough to start doing on-target debugging and validation.
What’s included
ecall) and handler wiring so kernel syscalls can be dispatched.Current status / limitations
I’d appreciate your review/feedback