Skip to content

Conversation

JensenWei007
Copy link
Contributor

No description provided.

@github-actions github-actions bot added the ambiguous The title of PR/issue doesn't match the format label Sep 29, 2025
@fslongjin
Copy link
Member

这里怎么这么多二进制文件

@Copilot Copilot AI review requested due to automatic review settings October 20, 2025 06:09
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds support for initramfs (initial RAM filesystem) and kexec (kernel execution) functionality to DragonOS. The changes enable the kernel to boot from an embedded compressed initramfs and provide the ability to execute a new kernel without going through the bootloader.

Key changes:

  • Implements initramfs support with XZ compression and CPIO archive handling
  • Adds kexec system call and related infrastructure for kernel switching
  • Introduces architecture-specific kexec implementations (primarily x86_64)
  • Refactors file descriptor management to use trait objects instead of concrete File types

Reviewed Changes

Copilot reviewed 83 out of 84 changed files in this pull request and generated 9 comments.

Show a summary per file
File Description
kernel/src/filesystem/vfs/file.rs Refactors File to implement FileOperations trait and changes FD table to use trait objects
kernel/src/filesystem/vfs/file_operations.rs Adds new FileOperations trait defining file operation interface
kernel/src/filesystem/ramfs/initram.rs Implements initramfs initialization and CPIO archive parsing
kernel/src/init/kexec/*.rs Adds kexec core functionality and system call implementation
kernel/src/arch/x86_64/kexec.rs Implements x86_64-specific kexec machine operations
kernel/src/arch/x86_64/asm/relocate_kernel_64.S Adds assembly code for kernel relocation during kexec
kernel/src/process/pidfd.rs Implements pidfd (process file descriptor) support
tools/run-qemu.sh Removes SMP parameter from QEMU arguments
kernel/src/syscall/misc.rs Implements umask system call with static storage

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.


QEMU_ARGUMENT+="-d ${QEMU_DISK_IMAGE} -m ${QEMU_MEMORY} -smp ${QEMU_SMP} -boot order=d ${QEMU_MONITOR} -d ${qemu_trace_std} "
#QEMU_ARGUMENT+="-d ${QEMU_DISK_IMAGE} -m ${QEMU_MEMORY} -smp ${QEMU_SMP} -boot order=d ${QEMU_MONITOR} -d ${qemu_trace_std} "
QEMU_ARGUMENT+="-d ${QEMU_DISK_IMAGE} -m ${QEMU_MEMORY} -boot order=d ${QEMU_MONITOR} -d ${qemu_trace_std} "
Copy link

Copilot AI Oct 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Duplicate -d flag in QEMU_ARGUMENT. The -d flag appears twice: once for ${QEMU_DISK_IMAGE} (which seems incorrect as -d is for debug options) and once for ${qemu_trace_std}. This will cause incorrect QEMU behavior.

Suggested change
QEMU_ARGUMENT+="-d ${QEMU_DISK_IMAGE} -m ${QEMU_MEMORY} -boot order=d ${QEMU_MONITOR} -d ${qemu_trace_std} "
QEMU_ARGUMENT+="-m ${QEMU_MEMORY} -boot order=d ${QEMU_MONITOR} -d ${qemu_trace_std} "

Copilot uses AI. Check for mistakes.


use super::{user_access::UserBufferWriter, Syscall};

static mut UMASK: usize = 0o777;
Copy link

Copilot AI Oct 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using a mutable static variable without synchronization is unsafe and not thread-safe. Multiple processes could race to read/write UMASK simultaneously. Consider using an AtomicUsize or per-process storage instead of a global static.

Copilot uses AI. Check for mistakes.

Comment on lines +75 to +77
crate::filesystem::vfs::get_link_true_file(argv[0].to_string_lossy().to_string())
.unwrap();
argv[0] = CString::new(real).unwrap();
Copy link

Copilot AI Oct 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using .unwrap() can cause a kernel panic if the link resolution fails. Consider proper error handling with a more graceful fallback or clear error message.

Suggested change
crate::filesystem::vfs::get_link_true_file(argv[0].to_string_lossy().to_string())
.unwrap();
argv[0] = CString::new(real).unwrap();
crate::filesystem::vfs::get_link_true_file(argv[0].to_string_lossy().to_string());
let real = match real {
Some(r) => r,
None => {
error!("Failed to resolve symlink for: {}", argv[0].to_string_lossy());
return Err(SystemError::ENOENT);
}
};
argv[0] = match CString::new(real) {
Ok(cstr) => cstr,
Err(_) => {
error!("Failed to create CString for resolved path");
return Err(SystemError::EINVAL);
}
};

Copilot uses AI. Check for mistakes.

Comment on lines +156 to +158
if pid.pid_vnr().data() == ProcessManager::current_pcb().raw_tgid().data() {
return Err(SystemError::ECHILD);
}
Copy link

Copilot AI Oct 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This check prevents a process from waiting on itself, but it appears in only one branch of the match statement (PidConverter::Pid). The same check should likely be present or unnecessary in other converter types for consistency.

Copilot uses AI. Check for mistakes.

Comment on lines +381 to +383
bp.arch.set_alt_mem_k(0x7fb40);
bp.arch.set_scratch(0x10000d);
bp.arch.init_setupheader();
Copy link

Copilot AI Oct 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hard-coded magic numbers (0x7fb40, 0x10000d) are being used without explanation. The comment mentions these should not be initialized this way. Add clear documentation explaining these values or refactor to obtain them dynamically.

Copilot uses AI. Check for mistakes.

# 0: x86_64
# 1: riscv64
# 2: loongarch64
arch = $1
Copy link

Copilot AI Oct 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bash variable assignment should not have spaces around the = operator. This should be arch=$1 instead of arch = $1.

Suggested change
arch = $1
arch=$1

Copilot uses AI. Check for mistakes.

let mut _rsp: usize = 0;

// 检查是否使用备用栈
if sigaction.flags().contains(SigFlags::SA_ONSTACK) {
Copy link

Copilot AI Oct 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The alternate signal stack implementation is incomplete. There's no validation that stack.sp is valid or that stack.size is non-zero before performing pointer arithmetic, which could lead to invalid stack pointer values.

Suggested change
if sigaction.flags().contains(SigFlags::SA_ONSTACK) {
if sigaction.flags().contains(SigFlags::SA_ONSTACK)
&& stack.sp != 0
&& stack.size != 0
{

Copilot uses AI. Check for mistakes.

// 判断标准: 是否存在 /init 程序, 与 Linux 相同
// 查找考虑链接
unsafe {
__INIT_ROOT_ENABLED = INIT_ROOT_INODE().find("init").is_ok();
Copy link

Copilot AI Oct 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using a global mutable static without proper synchronization. Although this is set during initialization, accessing __INIT_ROOT_ENABLED from multiple contexts could be unsafe. Consider using atomic operations or ensuring this is only accessed after initialization is complete.

Copilot uses AI. Check for mistakes.

let slice = core::slice::from_raw_parts_mut(virt.data() as *mut u8, 2048);
slice.fill(0);
// 这里先使用固定的写死的 cmdline, 后续等 DragonOS 的切换设置没问题了让 linux 能完成初始化的时候改成与 DragonOS 一样就行
let mess = "console=ttyS0 earlyprintk=serial,ttyS0,115200";
Copy link

Copilot AI Oct 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hard-coded kernel command line parameters should be configurable. The comment acknowledges this is temporary, but using a hard-coded command line limits flexibility for different boot scenarios.

Copilot uses AI. Check for mistakes.

- Support embedding initram and using Ramfs as the file system for extracting initram
- Support kexec series system calls, including load series and reboot
- Support u-root as the root file system to boot in Go language
- Add sysfs such as boot_crams and memmap
- Add a series of peripheral system calls related to the above

Signed-off-by: JensenWei007 <[email protected]>
@JensenWei007
Copy link
Contributor Author

bugbot run

@cursor
Copy link

cursor bot commented Oct 22, 2025

Skipping Bugbot: Unable to authenticate your request. Please make sure Bugbot is properly installed and configured for this repository.

@JensenWei007 JensenWei007 changed the title Add initram and kexec feat(kexec & initram):Add kexec and initram support for x86 architecture Oct 22, 2025
@github-actions github-actions bot removed the ambiguous The title of PR/issue doesn't match the format label Oct 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants