Browse Source

fixes and updates, basically on mem and scheduler

update(init_script): write to stderr on errors encountered
chore(Makefile): enable asm demangling
fix(mm_list): call invalidate_tlb after setting up cows
fix(scheduler): make preemption responsibility clear
fix(sys_brk): do not set a marker for the program break in `mm_list`
feat(syscall): print syscall invokes in debug mode
feat(init): open the console for init process
greatbridf 4 months ago
parent
commit
2c72c9ba77

+ 7 - 1
Makefile.src

@@ -42,7 +42,13 @@ clean-all: clean
 
 .PHONY: debug
 debug:
-	-$(GDB_BIN) --symbols=build/kernel.out --init-eval-command 'source pretty-print.py' --init-eval-command 'set pagination off' --init-eval-command 'set output-radix 16' --init-eval-command 'set print pretty on' --init-eval-command 'target remote:1234'
+	-$(GDB_BIN) --symbols=build/kernel.out \
+		-iex 'source pretty-print.py' \
+		-iex 'set pagination off' \
+		-iex 'set output-radix 16' \
+		-iex 'set print asm-demangle on' \
+		-iex 'set print pretty on' \
+		-iex 'target remote:1234'
 	-killall $(QEMU_BIN)
 
 build/boot.vdi: build/boot.img

+ 3 - 3
init_script.sh

@@ -3,7 +3,7 @@
 BUSYBOX=/mnt/busybox
 
 freeze() {
-    echo "an error occurred while executing '''$@''', freezing..." > /dev/console
+    echo "an error occurred while executing '''$@''', freezing..." >&2
 
     while true; do
         true
@@ -26,14 +26,14 @@ do_or_freeze $BUSYBOX mknod -m 666 /dev/zero c 1 5
 do_or_freeze $BUSYBOX mknod -m 666 /dev/sda b 8 0
 do_or_freeze $BUSYBOX mknod -m 666 /dev/sda1 b 8 1
 
-echo -n -e "deploying busybox... " > /dev/console
+echo -n -e "deploying busybox... " >&2
 
 do_or_freeze $BUSYBOX mkdir -p /bin
 do_or_freeze $BUSYBOX --install -s /bin
 
 export PATH="/bin"
 
-echo ok > /dev/console
+echo ok >&2
 
 do_or_freeze mkdir -p /etc /root /proc
 do_or_freeze mount -t procfs proc proc

+ 2 - 0
src/kernel/constants.rs

@@ -19,6 +19,7 @@ pub const CLOCK_MONOTONIC: u32 = 1;
 pub const ENOEXEC: u32 = 8;
 
 bitflags! {
+    #[derive(Debug, Clone, Copy)]
     pub struct UserMmapFlags: u32 {
         const MAP_SHARED = 0x01;
         const MAP_PRIVATE = 0x02;
@@ -26,6 +27,7 @@ bitflags! {
         const MAP_ANONYMOUS = 0x20;
     }
 
+    #[derive(Debug, Clone, Copy)]
     pub struct UserMmapProtocol: u32 {
         const PROT_READ = 0x01;
         const PROT_WRITE = 0x02;

+ 39 - 27
src/kernel/mem/mm_list.rs

@@ -97,7 +97,11 @@ impl MMListInner {
     }
 
     fn find_available(&self, hint: VAddr, len: usize) -> Option<VAddr> {
-        let mut range = VRange::new(hint.floor(), (hint + len).ceil());
+        let mut range = if hint == VAddr::NULL {
+            VRange::new(VAddr(0x1234000), VAddr(0x1234000 + len).ceil())
+        } else {
+            VRange::new(hint.floor(), (hint + len).ceil())
+        };
         let len = range.len();
 
         loop {
@@ -192,6 +196,18 @@ impl MMListInner {
         Ok(())
     }
 
+    fn grow(&self, area: &MMArea, len: usize) {
+        self.page_table.set_anonymous(
+            VRange::from(area.range().end()).grow(len),
+            Permission {
+                write: true,
+                execute: false,
+            },
+        );
+
+        area.grow(len);
+    }
+
     fn set_break(&mut self, pos: Option<VAddr>) -> VAddr {
         // SAFETY: `set_break` is only called in syscalls, where program break should be valid.
         assert!(self.break_start.is_some() && self.break_pos.is_some());
@@ -207,20 +223,23 @@ impl MMListInner {
             return current_break;
         }
 
-        self.page_table.set_anonymous(
-            range,
-            Permission {
-                write: true,
-                execute: false,
-            },
-        );
+        if !self.areas.contains(&break_start) {
+            self.areas.insert(MMArea::new(
+                break_start,
+                Mapping::Anonymous,
+                Permission {
+                    write: true,
+                    execute: false,
+                },
+            ));
+        }
 
         let program_break = self
             .areas
             .get(&break_start)
             .expect("Program break area should be valid");
-        program_break.grow(pos - current_break);
 
+        self.grow(program_break, pos - current_break);
         self.break_pos = Some(pos);
         pos
     }
@@ -238,9 +257,6 @@ impl MMList {
         })
     }
 
-    /// # Safety
-    /// Calling this function on the `MMList` of current process will need invalidating
-    /// the TLB cache after the clone will be done. We might set some of the pages as COW.
     pub fn new_cloned(&self) -> Arc<Self> {
         let inner = self.inner.lock_irq();
 
@@ -267,12 +283,13 @@ impl MMList {
             }
         }
 
+        // We set some pages as COW, so we need to invalidate TLB.
+        inner.page_table.lazy_invalidate_tlb_all();
+
         list
     }
 
-    /// # Safety
-    /// Calling this function on the `MMList` of current process will need invalidating
-    /// the TLB cache after the clone will be done. We might remove some mappings.
+    /// No need to do invalidation manually, `PageTable` already does it.
     pub fn clear_user(&self) {
         self.inner.lock_irq().clear_user()
     }
@@ -294,6 +311,12 @@ impl MMList {
         permission: Permission,
     ) -> KResult<VAddr> {
         let mut inner = self.inner.lock_irq();
+        if hint == VAddr::NULL {
+            let at = inner.find_available(hint, len).ok_or(ENOMEM)?;
+            inner.mmap(at, len, mapping, permission)?;
+            return Ok(at);
+        }
+
         match inner.mmap(hint, len, mapping.clone(), permission) {
             Ok(()) => Ok(hint),
             Err(EEXIST) => {
@@ -320,22 +343,11 @@ impl MMList {
         self.inner.lock_irq().set_break(pos)
     }
 
+    /// This should be called only **once** for every thread.
     pub fn register_break(&self, start: VAddr) {
         let mut inner = self.inner.lock_irq();
         assert!(inner.break_start.is_none() && inner.break_pos.is_none());
 
-        inner
-            .mmap(
-                start,
-                0,
-                Mapping::Anonymous,
-                Permission {
-                    write: true,
-                    execute: false,
-                },
-            )
-            .expect("Probably, we have a bug in the ELF loader?");
-
         inner.break_start = Some(start.into());
         inner.break_pos = Some(start);
     }

+ 6 - 0
src/kernel/mem/page_table.rs

@@ -252,6 +252,12 @@ impl PageTable {
         }
     }
 
+    pub fn lazy_invalidate_tlb_all(&self) {
+        if self.page.as_phys() == arch::vm::current_page_table() {
+            arch::vm::invlpg_all();
+        }
+    }
+
     pub fn set_mmapped(&self, range: VRange, permission: Permission) {
         // PA_RW is set during page fault handling.
         // PA_NXE is preserved across page faults, so we set PA_NXE now.

+ 2 - 0
src/kernel/mem/vrange.rs

@@ -16,6 +16,8 @@ pub struct VRange {
 const USER_SPACE_MEMORY_TOP: VAddr = VAddr(0x8000_0000_0000);
 
 impl VAddr {
+    pub const NULL: Self = Self(0);
+
     pub fn floor(&self) -> Self {
         VAddr(self.0 & !0xfff)
     }

+ 31 - 4
src/kernel/syscall.rs

@@ -94,17 +94,44 @@ macro_rules! arg_register {
     };
 }
 
+macro_rules! format_expand {
+    ($name:ident, $arg:tt) => {
+        format_args!("{}: {:x?}", stringify!($name), $arg)
+    };
+    ($name1:ident, $arg1:tt, $($name:ident, $arg:tt),*) => {
+        format_args!("{}: {:x?}, {}", stringify!($name1), $arg1, format_expand!($($name, $arg),*))
+    }
+}
+
 macro_rules! syscall32_call {
     ($is:ident, $handler:ident, $($arg:ident: $type:ty),*) => {{
         use $crate::kernel::syscall::{MapArgument, MapArgumentImpl, arg_register};
-        use $crate::kernel::syscall::{MapReturnValue};
+        use $crate::kernel::syscall::{MapReturnValue, format_expand};
+        use $crate::{kernel::task::Thread, println_info};
 
         $(
             let $arg: $type =
                 MapArgumentImpl::map_arg(arg_register!(${index()}, $is));
         )*
 
-        match $handler($($arg),*) {
+        println_info!(
+            "tid{}: {}({}) => {{",
+            Thread::current().tid,
+            stringify!($handler),
+            format_expand!($($arg, $arg),*),
+        );
+
+        let result = $handler($($arg),*);
+
+        println_info!(
+            "tid{}: {}({}) => }} = {:x?}",
+            Thread::current().tid,
+            stringify!($handler),
+            format_expand!($($arg, $arg),*),
+            result
+        );
+
+        match result {
             Ok(val) => MapReturnValue::map_ret(val),
             Err(err) => (-(err as i32)) as usize,
         }
@@ -146,7 +173,7 @@ macro_rules! register_syscall {
 
 use super::task::Thread;
 
-pub(self) use {arg_register, define_syscall32, register_syscall, syscall32_call};
+pub(self) use {arg_register, define_syscall32, format_expand, register_syscall, syscall32_call};
 
 pub(self) struct SyscallHandler {
     handler: fn(&mut interrupt_stack, &mut mmx_registers) -> usize,
@@ -185,7 +212,7 @@ pub fn handle_syscall32(no: usize, int_stack: &mut interrupt_stack, mmxregs: &mu
 
     match syscall {
         None => {
-            println_warn!("Syscall {no}({no:#x}) isn't implemented");
+            println_warn!("Syscall {no}({no:#x}) isn't implemented.");
             ProcessList::kill_current(Signal::SIGSYS);
         }
         Some(handler) => {

+ 36 - 4
src/kernel/syscall/procops.rs

@@ -1,3 +1,5 @@
+use core::arch::global_asm;
+
 use alloc::borrow::ToOwned;
 use alloc::ffi::CString;
 use bindings::{interrupt_stack, mmx_registers, EINVAL, ENOENT, ENOTDIR, ESRCH};
@@ -14,6 +16,7 @@ use crate::kernel::user::dataflow::UserString;
 use crate::kernel::user::{UserPointer, UserPointerMut};
 use crate::kernel::vfs::dentry::Dentry;
 use crate::path::Path;
+use crate::sync::preempt;
 use crate::{kernel::user::dataflow::UserBuffer, prelude::*};
 
 use crate::kernel::vfs::{self, FsContext};
@@ -189,6 +192,14 @@ fn do_waitpid(waitpid: u32, arg1: *mut u32, options: u32) -> KResult<u32> {
     }
 }
 
+fn do_wait4(waitpid: u32, arg1: *mut u32, options: u32, rusage: *mut ()) -> KResult<u32> {
+    if rusage.is_null() {
+        do_waitpid(waitpid, arg1, options)
+    } else {
+        unimplemented!("wait4 with rusage")
+    }
+}
+
 fn do_setsid() -> KResult<u32> {
     Thread::current().process.setsid()
 }
@@ -333,7 +344,7 @@ fn do_rt_sigprocmask(how: u32, set: *mut u64, oldset: *mut u64, sigsetsize: usiz
         UserPointerMut::new(oldset)?.write(old_mask)?;
     }
 
-    let new_mask = !if set.is_null() {
+    let new_mask = if !set.is_null() {
         UserPointer::new(set)?.read()?
     } else {
         return Ok(());
@@ -366,10 +377,10 @@ fn do_rt_sigaction(
     }
 
     if !act.is_null() {
-        let new_action = UserPointer::new(act as *mut _)?.read()?;
+        let new_action = UserPointer::new(act)?.read()?;
         Thread::current()
             .signal_list
-            .set_handler(signal, new_action)?;
+            .set_handler(signal, &new_action)?;
     }
 
     Ok(())
@@ -380,6 +391,7 @@ define_syscall32!(sys_umask, do_umask, mask: u32);
 define_syscall32!(sys_getcwd, do_getcwd, buffer: *mut u8, bufsize: usize);
 define_syscall32!(sys_exit, do_exit, status: u32);
 define_syscall32!(sys_waitpid, do_waitpid, waitpid: u32, arg1: *mut u32, options: u32);
+define_syscall32!(sys_wait4, do_wait4, waitpid: u32, arg1: *mut u32, options: u32, rusage: *mut ());
 define_syscall32!(sys_setsid, do_setsid);
 define_syscall32!(sys_setpgid, do_setpgid, pid: u32, pgid: i32);
 define_syscall32!(sys_getsid, do_getsid, pid: u32);
@@ -405,8 +417,27 @@ define_syscall32!(sys_rt_sigaction, do_rt_sigaction,
 
 extern "C" {
     fn ISR_stub_restore();
+    fn new_process_return();
+}
+
+unsafe extern "C" fn real_new_process_return() {
+    // We don't land on the typical `Scheduler::schedule()` function, so we need to
+    // manually enable preemption.
+    preempt::enable();
 }
 
+global_asm!(
+    r"
+        .globl new_process_return
+        new_process_return:
+            call {0}
+            jmp {1}
+    ",
+    sym real_new_process_return,
+    sym ISR_stub_restore,
+    options(att_syntax),
+);
+
 fn sys_fork(int_stack: &mut interrupt_stack, mmxregs: &mut mmx_registers) -> usize {
     let new_thread = Thread::new_cloned(Thread::current());
 
@@ -418,7 +449,7 @@ fn sys_fork(int_stack: &mut interrupt_stack, mmxregs: &mut mmx_registers) -> usi
 
         // We make the child process return to `ISR_stub_restore`, pretending that we've
         // just returned from a interrupt handler.
-        writer.entry = ISR_stub_restore;
+        writer.entry = new_process_return;
 
         let mut new_int_stack = int_stack.clone();
 
@@ -456,6 +487,7 @@ pub(super) fn register() {
     register_syscall!(0x3c, umask);
     register_syscall!(0x40, getppid);
     register_syscall!(0x42, setsid);
+    register_syscall!(0x72, wait4);
     register_syscall!(0x84, getpgid);
     register_syscall!(0x93, getsid);
     register_syscall!(0xac, prctl);

+ 1 - 3
src/kernel/task/kstack.rs

@@ -108,9 +108,7 @@ impl KernelStack {
         // TODO!!!: Make `TSS` a per cpu struct.
         // SAFETY: `TSS_RSP0` is always valid.
         unsafe {
-            TSS_RSP0
-                .as_ptr::<u64>()
-                .write_unaligned(*self.sp.get() as u64);
+            TSS_RSP0.as_ptr::<u64>().write_unaligned(self.bottom as u64);
         }
     }
 

+ 7 - 2
src/kernel/task/scheduler.rs

@@ -1,6 +1,6 @@
 use core::sync::atomic::{compiler_fence, Ordering};
 
-use crate::prelude::*;
+use crate::{prelude::*, sync::preempt};
 
 use alloc::{
     collections::vec_deque::VecDeque,
@@ -65,6 +65,7 @@ impl Scheduler {
             writer.entry = idle_task;
             writer.finish();
         });
+        // We don't wake the idle thread to prevent from accidentally being scheduled there.
 
         // TODO!!!: Set per cpu variable.
         unsafe { IDLE_TASK = Some(thread) };
@@ -144,7 +145,8 @@ impl Scheduler {
 }
 
 impl Scheduler {
-    /// Go to idle task. Call this with `preempt_count == 1`
+    /// Go to idle task. Call this with `preempt_count == 1`.
+    /// The preempt count will be decremented by this function.
     ///
     /// # Safety
     /// We might never return from here.
@@ -160,9 +162,12 @@ impl Scheduler {
         // Since we might never return to here, we can't take ownership of `current()`.
         // Is it safe to believe that `current()` will never change across calls?
         context_switch_light(Thread::current(), Scheduler::idle_task());
+        preempt::enable();
     }
 
     pub fn schedule_noreturn() -> ! {
+        println_debug!("Scheduler::schedule_noreturn()");
+        preempt::disable();
         Self::schedule();
         panic!("Scheduler::schedule_noreturn(): Should never return")
     }

+ 9 - 3
src/kernel/task/thread.rs

@@ -375,6 +375,12 @@ impl ProcessList {
     }
 
     pub fn kill_current(signal: Signal) -> ! {
+        println_debug!(
+            "Killing thread {} with signal {:?}",
+            Thread::current().tid,
+            signal,
+        );
+
         ProcessList::get().do_kill_process(
             &Thread::current().process,
             ((signal.to_signum() + 128) << 8) | (signal.to_signum() & 0xff),
@@ -463,15 +469,15 @@ impl ProcessList {
         }
 
         {
-            let parent = process.parent().unwrap();
             {
-                let mut parent_waits = parent.wait_list.wait_procs.lock();
+                let parent_wait_list = &inner.parent.as_ref().unwrap().wait_list;
+                let mut parent_waits = parent_wait_list.wait_procs.lock();
                 parent_waits.push_back(WaitObject {
                     pid: process.pid,
                     code: status,
                 });
+                parent_wait_list.cv_wait_procs.notify_all();
             }
-            parent.wait_list.cv_wait_procs.notify_all();
         }
 
         preempt::enable();

+ 7 - 0
src/kernel/timer.rs

@@ -29,7 +29,14 @@ impl Ticks {
 fn timer_interrupt() {
     TICKS.fetch_add(1, Ordering::Relaxed);
     if preempt::count() == 0 {
+        println_debug!("Timer interrupt reschedule");
+        preempt::disable();
+
+        // To make scheduler satisfied.
+        preempt::disable();
         Scheduler::schedule();
+
+        preempt::enable();
     }
 }
 

+ 1 - 1
src/kernel/user/dataflow.rs

@@ -31,7 +31,7 @@ pub struct UserPointer<'a, T: Copy, const CONST: bool> {
 }
 
 impl<'a, T: Copy, const CONST: bool> UserPointer<'a, T, CONST> {
-    pub fn new(ptr: *mut T) -> KResult<Self> {
+    pub fn new(ptr: *const T) -> KResult<Self> {
         let pointer = CheckedUserPointer::new(ptr as *const u8, core::mem::size_of::<T>())?;
 
         Ok(Self {

+ 23 - 0
src/kernel/vfs/filearray.rs

@@ -239,6 +239,29 @@ impl FileArray {
             _ => unimplemented!("fcntl: cmd={}", cmd),
         }
     }
+
+    pub fn open_console(&self) -> KResult<()> {
+        let mut inner = self.inner.lock();
+        let (stdin, stdout, stderr) = (inner.next_fd(), inner.next_fd(), inner.next_fd());
+
+        inner.do_insert(
+            stdin,
+            O_CLOEXEC as u64,
+            TerminalFile::new(CONSOLE.lock_irq().get_terminal().unwrap()),
+        );
+        inner.do_insert(
+            stdout,
+            O_CLOEXEC as u64,
+            TerminalFile::new(CONSOLE.lock_irq().get_terminal().unwrap()),
+        );
+        inner.do_insert(
+            stderr,
+            O_CLOEXEC as u64,
+            TerminalFile::new(CONSOLE.lock_irq().get_terminal().unwrap()),
+        );
+
+        Ok(())
+    }
 }
 
 impl FileArrayInner {

+ 12 - 2
src/lib.rs

@@ -29,7 +29,7 @@ use kernel::{
         paging::Page,
         phys::{CachedPP, PhysPtr as _},
     },
-    task::{init_multitasking, Thread},
+    task::{init_multitasking, Scheduler, Thread},
     vfs::{
         dentry::Dentry,
         mount::{do_mount, MS_NOATIME, MS_NODEV, MS_NOSUID, MS_RDONLY},
@@ -39,6 +39,7 @@ use kernel::{
 };
 use path::Path;
 use prelude::*;
+use sync::preempt;
 
 #[panic_handler]
 fn panic(info: &core::panic::PanicInfo) -> ! {
@@ -132,15 +133,22 @@ pub extern "C" fn rust_kinit(early_kstack_pfn: usize) -> ! {
         writer.finish();
     });
 
+    // To satisfy the `Scheduler` "preempt count == 0" assertion.
+    preempt::disable();
+
+    Scheduler::get().lock().uwake(Thread::current());
+
     arch::task::context_switch_light(
         CachedPP::new(early_kstack_pfn).as_ptr(), // We will never come back
-        unsafe { Thread::current().get_sp_ptr() },
+        unsafe { Scheduler::idle_task().get_sp_ptr() },
     );
     arch::task::freeze()
 }
 
+/// We enter this function with `preempt count == 0`
 extern "C" fn init_process(early_kstack_pfn: usize) {
     unsafe { Page::take_pfn(early_kstack_pfn, 9) };
+    preempt::enable();
 
     kernel::timer::init().unwrap();
 
@@ -196,6 +204,8 @@ extern "C" fn init_process(early_kstack_pfn: usize) {
             .unwrap()
     };
 
+    Thread::current().files.open_console();
+
     unsafe {
         asm!(
             "mov %ax, %fs",