fix(riscv64): boot StarryOS with RustSBI on riscv64 without IllegalInstruction trap#131
fix(riscv64): boot StarryOS with RustSBI on riscv64 without IllegalInstruction trap#131LetsWalkInLine wants to merge 2 commits intoStarry-OS:mainfrom
Conversation
Booting StarryOS with RustSBI on riscv64 could panic with IllegalInstruction (stval=0xf2000053) during task scheduling. Initialize sstatus.FS early in kernel entry to make fp-simd execution state explicit and avoid firmware-dependent boot behavior. Closes Starry-OS#130
- Introduce a configurable `QEMU_BIOS` make variable to avoid hardcoded `-bios default` in QEMU run arguments for `riscv64`. - Keep the default behavior unchanged by setting `QEMU_BIOS ?= default`, so existing `make run` workflows are not affected. - Wire `qemu_args-riscv64` to use `-bios $(QEMU_BIOS)` and document this option in make argument help comments. - This change is intended to simplify local validation for RustSBI boot scenarios (e.g. `rustsbi-prototyper-dynamic.bin`) while keeping runtime semantics backward-compatible. Closes Starry-OS#130
There was a problem hiding this comment.
Pull request overview
Fixes a riscv64 boot failure under RustSBI by explicitly enabling the kernel’s FP/SIMD execution state early in kernel init, and adds a Make/QEMU knob to swap BIOS images for easier reproduction/validation.
Changes:
- Add a riscv64-only early init helper to set
sstatus.FS, preventingIllegalInstructiontraps when FP/SIMD code runs under RustSBI. - Add
QEMU_BIOS ?= defaultto Makefile options and route riscv64 QEMU-biosthrough it. - Update build/run docs in the Makefile to mention the new QEMU option.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| kernel/src/entry.rs | Enables RISC-V FP/SIMD state via sstatus.FS before initproc setup. |
| make/Makefile | Adds QEMU_BIOS variable and documents it in the QEMU options list. |
| make/qemu.mk | Uses -bios $(QEMU_BIOS) for riscv64 QEMU runs. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| #[cfg(target_arch = "riscv64")] | ||
| #[inline] | ||
| fn init_riscv_fp_state() { | ||
| use riscv::register::sstatus::{self, FS}; | ||
| unsafe { sstatus::set_fs(FS::Dirty) }; | ||
| } | ||
|
|
||
| /// Initialize and run initproc. | ||
| pub fn init(args: &[String], envs: &[String]) { | ||
| #[cfg(target_arch = "riscv64")] | ||
| init_riscv_fp_state(); | ||
|
|
There was a problem hiding this comment.
sstatus.FS is a per-hart CSR on RISC-V. Initializing it only in entry::init() (which appears to run once during initproc setup) may leave secondary harts with FS=Off, so any FP/SIMD instruction executed on those harts could still trap. Consider moving this to an earlier per-CPU/hart init hook (e.g., in the platform/axruntime CPU bring-up path) or otherwise ensuring it runs on every hart before scheduling begins there.
| # - `MEM`: Memory size (default is 128M) | ||
| # - `DISK_IMG`: Path to the virtual disk image | ||
| # - `ACCEL`: Enable hardware acceleration (KVM on linux) | ||
| # - `QEMU_BIOS`: QEMU BIOS firmware path (default is `default`) |
There was a problem hiding this comment.
The QEMU_BIOS help text says it's a “firmware path”, but the default value is the special QEMU selector default (not a filesystem path). Consider rewording to clarify it can be either default or a BIOS/firmware image path so users don’t assume it must be a path.
| # - `QEMU_BIOS`: QEMU BIOS firmware path (default is `default`) | |
| # - `QEMU_BIOS`: QEMU BIOS selector (`default`) or firmware image path (default is `default`) |
|
You should add this fix to https://github.com/arceos-org/axcpu/blob/dev/src/riscv/context.rs or https://github.com/arceos-org/axcpu/blob/dev/src/riscv/init.rs, but not here. |
Thanks for the guidance — makes sense. |
A kindly reminder: you can work upon |
Closes #130
Background
When booting StarryOS with RustSBI on riscv64, the kernel could panic with:
Unhandled trap Exception(IllegalInstruction)sepc=0xffffffc08037499cstval=0xf2000053axtask::run_queue::switch_toThe same image could boot with default BIOS, indicating a firmware-dependent init assumption.
Root Cause
The fp-simd path could execute before FP state was explicitly initialized for this boot path.
With RustSBI, this exposed an initialization gap and resulted in an illegal instruction trap.
Changes
kernel/src/entry.rsinit_riscv_fp_state()riscv::register::sstatus::FStoFS::Dirtyentry::init()before user init work.make/MakefileQEMU_BIOS ?= defaultQEMU_BIOSin QEMU options comment block.make/qemu.mk-bios default-bios $(QEMU_BIOS)This allows direct reproduction and verification with RustSBI via:
Validation Performed
IllegalInstructiontrap.fp-simdremoved the trap, confirming FP-state relation.ci-test: passes (Boot into BusyBox shell).Impact
Files Included In This PR
kernel/src/entry.rsmake/Makefilemake/qemu.mkI’m still learning kernel development, so feedback is very welcome.
If any part of this fix should be done differently, I’m happy to revise.