From e1480f94eed101aaf1d72a926644f28c3c395a9d Mon Sep 17 00:00:00 2001 From: jellllly420 Date: Sat, 1 Jun 2024 16:10:51 +0800 Subject: [PATCH] fix: enable timely delivery of POSIX signals while busy-looping --- docs/src/framework/a-100-line-kernel.md | 10 +++-- framework/aster-frame/src/arch/x86/cpu.rs | 21 +++++++--- framework/aster-frame/src/user.rs | 48 +++++++++++++---------- kernel/aster-nix/src/thread/task.rs | 30 ++++++++------ regression/apps/Makefile | 1 + regression/apps/alarm/Makefile | 5 +++ regression/apps/alarm/alarm.c | 11 ++++++ 7 files changed, 83 insertions(+), 43 deletions(-) create mode 100644 regression/apps/alarm/Makefile create mode 100644 regression/apps/alarm/alarm.c diff --git a/docs/src/framework/a-100-line-kernel.md b/docs/src/framework/a-100-line-kernel.md index 8d666bd48..a908544f6 100644 --- a/docs/src/framework/a-100-line-kernel.md +++ b/docs/src/framework/a-100-line-kernel.md @@ -55,7 +55,7 @@ use alloc::vec; use aster_frame::cpu::UserContext; use aster_frame::prelude::*; use aster_frame::task::{Task, TaskOptions}; -use aster_frame::user::{UserEvent, UserMode, UserSpace}; +use aster_frame::user::{ReturnReason, UserMode, UserSpace}; use aster_frame::vm::{PageFlags, PAGE_SIZE, Vaddr, VmAllocOptions, VmIo, VmMapOptions, VmSpace}; /// The kernel's boot and initialization process is managed by Asterinas Framework. @@ -116,13 +116,15 @@ fn create_user_task(user_space: Arc) -> Arc { loop { // The execute method returns when system - // calls or CPU exceptions occur. - let user_event = user_mode.execute(); + // calls or CPU exceptions occur or some + // events specified by the kernel occur. + let return_reason = user_mode.execute(|| false); + // The CPU registers of the user space // can be accessed and manipulated via // the `UserContext` abstraction. let user_context = user_mode.context_mut(); - if UserEvent::Syscall == user_event { + if ReturnReason::UserSyscall == return_reason { handle_syscall(user_context, current.user_space().unwrap()); } } diff --git a/framework/aster-frame/src/arch/x86/cpu.rs b/framework/aster-frame/src/arch/x86/cpu.rs index 24c6ba04e..78200cdca 100644 --- a/framework/aster-frame/src/arch/x86/cpu.rs +++ b/framework/aster-frame/src/arch/x86/cpu.rs @@ -23,7 +23,7 @@ use x86_64::registers::rflags::RFlags; use crate::arch::tdx_guest::{handle_virtual_exception, TdxTrapFrame}; use crate::{ trap::call_irq_callback_functions, - user::{UserContextApi, UserContextApiInternal, UserEvent}, + user::{ReturnReason, UserContextApi, UserContextApiInternal}, }; /// Returns the number of CPUs. @@ -257,11 +257,15 @@ impl UserContext { } impl UserContextApiInternal for UserContext { - fn execute(&mut self) -> crate::user::UserEvent { + fn execute(&mut self, mut has_kernel_event: F) -> ReturnReason + where + F: FnMut() -> bool, + { // set interrupt flag so that in user mode it can receive external interrupts // set ID flag which means cpu support CPUID instruction self.user_context.general.rflags |= (RFlags::INTERRUPT_FLAG | RFlags::ID).bits() as usize; + let return_reason: ReturnReason; const SYSCALL_TRAPNUM: u16 = 0x100; let mut user_preemption = UserPreemption::new(); @@ -281,31 +285,36 @@ impl UserContextApiInternal for UserContext { || exception.typ == CpuExceptionType::Fault || exception.typ == CpuExceptionType::Trap { + return_reason = ReturnReason::UserException; break; } } None => { if self.user_context.trap_num as u16 == SYSCALL_TRAPNUM { + return_reason = ReturnReason::UserSyscall; break; } } }; call_irq_callback_functions(&self.as_trap_frame()); + if has_kernel_event() { + return_reason = ReturnReason::KernelEvent; + break; + } user_preemption.might_preempt(); } crate::arch::irq::enable_local(); - if self.user_context.trap_num as u16 != SYSCALL_TRAPNUM { + if return_reason == ReturnReason::UserException { self.cpu_exception_info = CpuExceptionInfo { page_fault_addr: unsafe { x86::controlregs::cr2() }, id: self.user_context.trap_num, error_code: self.user_context.error_code, }; - UserEvent::Exception - } else { - UserEvent::Syscall } + + return_reason } fn as_trap_frame(&self) -> trapframe::TrapFrame { diff --git a/framework/aster-frame/src/user.rs b/framework/aster-frame/src/user.rs index 0d996b1c7..1c229a50b 100644 --- a/framework/aster-frame/src/user.rs +++ b/framework/aster-frame/src/user.rs @@ -51,7 +51,9 @@ impl UserSpace { /// Only visible in aster-frame pub(crate) trait UserContextApiInternal { /// Starts executing in the user mode. - fn execute(&mut self) -> UserEvent; + fn execute(&mut self, has_kernel_event: F) -> ReturnReason + where + F: FnMut() -> bool; /// Use the information inside CpuContext to build a trapframe fn as_trap_frame(&self) -> TrapFrame; @@ -93,9 +95,9 @@ pub trait UserContextApi { /// .expect("the current task is associated with a user space"); /// let mut user_mode = user_space.user_mode(); /// loop { -/// // Execute in the user space until some interesting user event occurs -/// let user_event = user_mode.execute(); -/// todo!("handle the user event, e.g., syscall"); +/// // Execute in the user space until some interesting events occur. +/// let return_reason = user_mode.execute(|| false); +/// todo!("handle the event, e.g., syscall"); /// } /// ``` pub struct UserMode<'a> { @@ -118,17 +120,22 @@ impl<'a> UserMode<'a> { /// Starts executing in the user mode. Make sure current task is the task in `UserMode`. /// - /// The method returns for one of three possible reasons indicated by `UserEvent`. - /// 1. The user invokes a system call; - /// 2. The user triggers an exception; - /// 3. The user triggers a fault. + /// The method returns for one of three possible reasons indicated by `ReturnReason`. + /// 1. A system call is issued by the user space; + /// 2. A CPU exception is triggered by the user space; + /// 3. A kernel event is pending, as indicated by the given closure. /// - /// After handling the user event and updating the user-mode CPU context, + /// After handling whatever user or kernel events that + /// cause the method to return + /// and updating the user-mode CPU context, /// this method can be invoked again to go back to the user space. - pub fn execute(&mut self) -> UserEvent { + pub fn execute(&mut self, has_kernel_event: F) -> ReturnReason + where + F: FnMut() -> bool, + { self.user_space.vm_space().activate(); debug_assert!(Arc::ptr_eq(&self.current, &Task::current())); - self.context.execute() + self.context.execute(has_kernel_event) } /// Returns an immutable reference the user-mode CPU context. @@ -143,14 +150,13 @@ impl<'a> UserMode<'a> { } #[derive(PartialEq, Eq, PartialOrd, Ord, Debug)] -/// A user event is what brings back the control of the CPU back from -/// the user space to the kernel space. -/// -/// Note that hardware interrupts are not considered user events as they -/// are triggered by devices and not visible to user programs. -/// To handle interrupts, one should register callback funtions for -/// IRQ lines (`IrqLine`). -pub enum UserEvent { - Syscall, - Exception, +/// A reason as to why the control of the CPU is returned from +/// the user space to the kernel. +pub enum ReturnReason { + /// A system call is issued by the user space. + UserSyscall, + /// A CPU exception is triggered by the user space. + UserException, + /// A kernel event is pending + KernelEvent, } diff --git a/kernel/aster-nix/src/thread/task.rs b/kernel/aster-nix/src/thread/task.rs index a2b5afb22..7d6587cf8 100644 --- a/kernel/aster-nix/src/thread/task.rs +++ b/kernel/aster-nix/src/thread/task.rs @@ -1,20 +1,27 @@ // SPDX-License-Identifier: MPL-2.0 use aster_frame::{ - cpu::UserContext, task::{preempt, Task, TaskOptions}, - user::{UserContextApi, UserEvent, UserMode, UserSpace}, + user::{ReturnReason, UserContextApi, UserMode, UserSpace}, }; use super::Thread; use crate::{ - cpu::LinuxAbi, prelude::*, process::signal::handle_pending_signal, syscall::handle_syscall, + cpu::LinuxAbi, + prelude::*, + process::{posix_thread::PosixThreadExt, signal::handle_pending_signal}, + syscall::handle_syscall, thread::exception::handle_exception, }; /// create new task with userspace and parent process pub fn create_new_user_task(user_space: Arc, thread_ref: Weak) -> Arc { fn user_task_entry() { + fn has_pending_signal(current_thread: &Arc) -> bool { + let posix_thread = current_thread.as_posix_thread().unwrap(); + posix_thread.has_pending_signal() + } + let current_thread = current_thread!(); let current_task = current_thread.task(); let user_space = current_task @@ -34,11 +41,17 @@ pub fn create_new_user_task(user_space: Arc, thread_ref: Weak user_mode.context().syscall_ret() ); + #[allow(clippy::redundant_closure)] + let has_kernel_event_fn = || has_pending_signal(¤t_thread); loop { - let user_event = user_mode.execute(); + let return_reason = user_mode.execute(has_kernel_event_fn); let context = user_mode.context_mut(); // handle user event: - handle_user_event(user_event, context); + match return_reason { + ReturnReason::UserException => handle_exception(context), + ReturnReason::UserSyscall => handle_syscall(context), + ReturnReason::KernelEvent => {} + }; // should be do this comparison before handle signal? if current_thread.status().is_exited() { break; @@ -68,10 +81,3 @@ pub fn create_new_user_task(user_space: Arc, thread_ref: Weak .build() .expect("spawn task failed") } - -fn handle_user_event(user_event: UserEvent, context: &mut UserContext) { - match user_event { - UserEvent::Syscall => handle_syscall(context), - UserEvent::Exception => handle_exception(context), - } -} diff --git a/regression/apps/Makefile b/regression/apps/Makefile index 79bb03948..b32c2d2b8 100644 --- a/regression/apps/Makefile +++ b/regression/apps/Makefile @@ -10,6 +10,7 @@ REGRESSION_BUILD_DIR ?= $(INITRAMFS)/regression # These test apps are sorted by name TEST_APPS := \ + alarm \ clone3 \ eventfd2 \ execve \ diff --git a/regression/apps/alarm/Makefile b/regression/apps/alarm/Makefile new file mode 100644 index 000000000..cd19ae20b --- /dev/null +++ b/regression/apps/alarm/Makefile @@ -0,0 +1,5 @@ +# SPDX-License-Identifier: MPL-2.0 + +include ../test_common.mk + +EXTRA_C_FLAGS := -static diff --git a/regression/apps/alarm/alarm.c b/regression/apps/alarm/alarm.c new file mode 100644 index 000000000..bd8b7bf36 --- /dev/null +++ b/regression/apps/alarm/alarm.c @@ -0,0 +1,11 @@ +// SPDX-License-Identifier: MPL-2.0 + +#include + +int main() +{ + alarm(3); + while (1) { + } + return 0; +}