From acfbc7efdc78559cf5af34033422f3c7eeb06d16 Mon Sep 17 00:00:00 2001 From: Zhang Junyang Date: Thu, 3 Aug 2023 11:05:06 +0800 Subject: [PATCH] Fix multiple issues in the initproc path --- .github/workflows/syscall_test.yml | 2 +- build/src/main.rs | 8 +- .../jinux-frame/src/arch/x86/boot/mod.rs | 2 +- framework/jinux-frame/src/boot/kcmdline.rs | 75 +++++-------------- framework/jinux-frame/src/boot/mod.rs | 33 ++++---- services/libs/jinux-std/src/lib.rs | 24 +++--- 6 files changed, 53 insertions(+), 91 deletions(-) diff --git a/.github/workflows/syscall_test.yml b/.github/workflows/syscall_test.yml index c17679678..54af65356 100644 --- a/.github/workflows/syscall_test.yml +++ b/.github/workflows/syscall_test.yml @@ -18,4 +18,4 @@ jobs: - name: Syscall Test id: syscall_test - run: make syscall_test ENABLE_KVM=false + run: make run AUTO_SYSCALL_TEST=1 ENABLE_KVM=0 diff --git a/build/src/main.rs b/build/src/main.rs index 1380ca374..8b0eff7fd 100644 --- a/build/src/main.rs +++ b/build/src/main.rs @@ -1,10 +1,10 @@ //! jinux-build is the Jinux runner script to ease the pain of running //! and testing Jinux inside a QEMU VM, which should be called as the //! cargo runner: https://doc.rust-lang.org/cargo/reference/config.html -//! +//! //! The runner generates the the filesystem image and the containing //! boot device image. Then it invokes QEMU to boot Jinux. -//! +//! use std::{ fs::{self, OpenOptions}, @@ -13,14 +13,13 @@ use std::{ process::Command, }; -use clap::{Parser, builder::Str}; +use clap::{builder::Str, Parser}; /// The CLI of this runner. #[derive(Parser, Debug)] #[command(author, version, about, long_about = None)] struct Args { // Positional arguments. - /// The Jinux binary path. path: PathBuf, @@ -29,7 +28,6 @@ struct Args { kcmdline: String, // Optional arguments. - /// Enable KVM when running QEMU. #[arg(long, default_value_t = false)] enable_kvm: bool, diff --git a/framework/jinux-frame/src/arch/x86/boot/mod.rs b/framework/jinux-frame/src/arch/x86/boot/mod.rs index d61101167..88a9f28df 100644 --- a/framework/jinux-frame/src/arch/x86/boot/mod.rs +++ b/framework/jinux-frame/src/arch/x86/boot/mod.rs @@ -5,5 +5,5 @@ //! on its way. //! -pub mod multiboot2; +mod multiboot2; pub use self::multiboot2::init_boot_args; diff --git a/framework/jinux-frame/src/boot/kcmdline.rs b/framework/jinux-frame/src/boot/kcmdline.rs index 63a412ce2..f0f32b96f 100644 --- a/framework/jinux-frame/src/boot/kcmdline.rs +++ b/framework/jinux-frame/src/boot/kcmdline.rs @@ -13,12 +13,9 @@ use alloc::{ vec, vec::Vec, }; -use log::debug; #[derive(PartialEq, Debug)] -struct InitprocArg { - // Since environment arguments can precede the init path argument, we - // have no choice but to wrap the path in `Option` and check it later. +struct InitprocArgs { path: Option, argv: Vec, envp: Vec, @@ -27,7 +24,7 @@ struct InitprocArg { /// The struct to store the parsed kernel command-line arguments. #[derive(Debug)] pub struct KCmdlineArg { - initproc: Option, + initproc: InitprocArgs, module_args: BTreeMap>, } @@ -35,18 +32,15 @@ pub struct KCmdlineArg { impl KCmdlineArg { /// Get the path of the initprocess. pub fn get_initproc_path(&self) -> Option<&str> { - self.initproc - .as_ref() - .and_then(|i| i.path.as_ref()) - .map(|s| s.as_str()) + self.initproc.path.as_ref().map(|s| s.as_str()) } /// Get the argument vector(argv) of the initprocess. - pub fn get_initproc_argv(&self) -> Option<&Vec> { - self.initproc.as_ref().map(|i| &i.argv) + pub fn get_initproc_argv(&self) -> &Vec { + &self.initproc.argv } /// Get the environment vector(envp) of the initprocess. - pub fn get_initproc_envp(&self) -> Option<&Vec> { - self.initproc.as_ref().map(|i| &i.argv) + pub fn get_initproc_envp(&self) -> &Vec { + &self.initproc.envp } /// Get the argument vector of a kernel module. pub fn get_module_args(&self, module: &str) -> Option<&Vec> { @@ -72,8 +66,12 @@ fn split_arg(input: &str) -> impl Iterator { impl From<&str> for KCmdlineArg { fn from(cmdline: &str) -> Self { // What we construct. - let mut result = KCmdlineArg { - initproc: None, + let mut result: KCmdlineArg = KCmdlineArg { + initproc: InitprocArgs { + path: None, + argv: Vec::new(), + envp: Vec::new(), + }, module_args: BTreeMap::new(), }; @@ -87,11 +85,10 @@ impl From<&str> for KCmdlineArg { // KernelArg => Arg "\s+" KernelArg | %empty // InitArg => Arg "\s+" InitArg | %empty if kcmdline_end { - if let Some(&mut ref mut i) = result.initproc.as_mut() { - i.argv.push(CString::new(arg).unwrap()); - } else { + if result.initproc.path == None { panic!("Initproc arguments provided but no initproc path specified!"); } + result.initproc.argv.push(CString::new(arg).unwrap()); continue; } if arg == "--" { @@ -134,32 +131,16 @@ impl From<&str> for KCmdlineArg { // The option has a value. match option { "init" => { - if let Some(&mut ref mut i) = result.initproc.as_mut() { - if let Some(v) = &i.path { - panic!("Initproc assigned twice in the command line!"); - } - i.path = Some(value.to_string()); - } else { - result.initproc = Some(InitprocArg { - path: Some(value.to_string()), - argv: Vec::new(), - envp: Vec::new(), - }); + if let Some(v) = &result.initproc.path { + panic!("Initproc assigned twice in the command line!"); } + result.initproc.path = Some(value.to_string()); } _ => { // If the option is not recognized, it is passed to the initproc. // Pattern 'option=value' is treated as the init environment. let envp_entry = CString::new(option.to_string() + "=" + value).unwrap(); - if let Some(&mut ref mut i) = result.initproc.as_mut() { - i.envp.push(envp_entry); - } else { - result.initproc = Some(InitprocArg { - path: None, - argv: Vec::new(), - envp: vec![envp_entry], - }); - } + result.initproc.envp.push(envp_entry); } } } else { @@ -169,28 +150,12 @@ impl From<&str> for KCmdlineArg { // If the option is not recognized, it is passed to the initproc. // Pattern 'option' without value is treated as the init argument. let argv_entry = CString::new(option.to_string()).unwrap(); - if let Some(&mut ref mut i) = result.initproc.as_mut() { - i.argv.push(argv_entry); - } else { - result.initproc = Some(InitprocArg { - path: None, - argv: vec![argv_entry], - envp: Vec::new(), - }); - } + result.initproc.argv.push(argv_entry); } } } } - debug!("{:?}", result); - - if let Some(&ref i) = result.initproc.as_ref() { - if i.path == None { - panic!("Initproc arguments provided but no initproc! Maybe have bad option."); - } - } - result } } diff --git a/framework/jinux-frame/src/boot/mod.rs b/framework/jinux-frame/src/boot/mod.rs index 4345b12a7..17e953d42 100644 --- a/framework/jinux-frame/src/boot/mod.rs +++ b/framework/jinux-frame/src/boot/mod.rs @@ -2,8 +2,6 @@ //! from the bootloader to the rest of the framework. //! -use crate::arch::boot::init_boot_args; - pub mod kcmdline; use kcmdline::KCmdlineArg; @@ -13,6 +11,8 @@ use self::memory_region::MemoryRegion; use alloc::{string::String, vec::Vec}; use spin::Once; +/// ACPI information from the bootloader. +/// /// The boot crate can choose either providing the raw RSDP physical address or /// providing the RSDT/XSDT physical address after parsing RSDP. /// This is because bootloaders differ in such behaviors. @@ -36,40 +36,39 @@ pub struct BootloaderFramebufferArg { pub bpp: usize, } -// Use a macro to simplify coding. macro_rules! define_global_static_boot_arguments { ( $( $lower:ident, $upper:ident, $typ:ty; )* ) => { - // Define statics and corresponding public get APIs. + // Define statics and corresponding public getter APIs. $( static $upper: Once<$typ> = Once::new(); - /// Macro generated public get API. + /// Macro generated public getter API. pub fn $lower() -> &'static $typ { $upper.get().unwrap() } )* + // Produce a init function call. The init function must // be defined in the `arch::boot` module conforming to this // definition. fn arch_init_boot_args() { - init_boot_args( $( &$upper, )* ); + crate::arch::boot::init_boot_args( $( &$upper, )* ); } }; } -// Define a series of static variable definitions and its APIs. The names in -// each line are: -// 1. The lowercase name of the variable, also the name of the get API; -// 2. the uppercase name of the variable; -// 3. the type of the variable. +// Define a series of static variables and their getter APIs. define_global_static_boot_arguments!( - bootloader_name, BOOTLOADER_NAME, String; - kernel_cmdline, KERNEL_CMDLINE, KCmdlineArg; - initramfs, INITRAMFS, &'static [u8]; - acpi_arg, ACPI_ARG, BootloaderAcpiArg; - framebuffer_arg, FRAMEBUFFER_ARG, BootloaderFramebufferArg; - memory_regions, MEMORY_REGIONS, Vec; + // Getter Names | Static Variables | Variable Types + bootloader_name, BOOTLOADER_NAME, String; + kernel_cmdline, KERNEL_CMDLINE, KCmdlineArg; + initramfs, INITRAMFS, &'static [u8]; + acpi_arg, ACPI_ARG, BootloaderAcpiArg; + framebuffer_arg, FRAMEBUFFER_ARG, BootloaderFramebufferArg; + memory_regions, MEMORY_REGIONS, Vec; ); +/// The initialization method of the boot module. +/// /// After initializing the boot module, the get functions could be called. /// The initialization must be done after the heap is set and before physical /// mappings are cancelled. diff --git a/services/libs/jinux-std/src/lib.rs b/services/libs/jinux-std/src/lib.rs index be860eea7..c95fd826c 100644 --- a/services/libs/jinux-std/src/lib.rs +++ b/services/libs/jinux-std/src/lib.rs @@ -81,25 +81,25 @@ fn init_thread() { let initproc = Process::spawn_user_process( karg.get_initproc_path().unwrap(), - karg.get_initproc_argv().unwrap().to_vec(), - karg.get_initproc_envp().unwrap().to_vec(), + karg.get_initproc_argv().to_vec(), + karg.get_initproc_envp().to_vec(), ) .expect("Run init process failed."); - loop { - // If initproc becomes zombie, then exit qemu. - if *initproc.status().lock() == ProcessStatus::Zombie { - let exit_code = if initproc.exit_code().load(Ordering::Relaxed) == 0 { - QemuExitCode::Success - } else { - QemuExitCode::Failed - }; - exit_qemu(exit_code); - } + // Wait till initproc become zombie. + while *initproc.status().lock() != ProcessStatus::Zombie { // We don't have preemptive scheduler now. // The long running init thread should yield its own execution to allow other tasks to go on. Thread::yield_now(); } + + // TODO: exit via qemu isa debug device should not be the only way. + let exit_code = if initproc.exit_code().load(Ordering::Relaxed) == 0 { + QemuExitCode::Success + } else { + QemuExitCode::Failed + }; + exit_qemu(exit_code); } /// first process never return