From e3cb4d25e3a19e8a865d11cce830dbc9e330f4ab Mon Sep 17 00:00:00 2001
From: Zhang Junyang <junyang@stu.pku.edu.cn>
Date: Tue, 9 Apr 2024 11:52:56 +0800
Subject: [PATCH] Fix the kernel stack alignment

---
 framework/aster-frame/src/task/task.rs | 62 ++++++++------------------
 1 file changed, 18 insertions(+), 44 deletions(-)

diff --git a/framework/aster-frame/src/task/task.rs b/framework/aster-frame/src/task/task.rs
index a1feb50a..660d2513 100644
--- a/framework/aster-frame/src/task/task.rs
+++ b/framework/aster-frame/src/task/task.rs
@@ -1,5 +1,4 @@
 // SPDX-License-Identifier: MPL-2.0
-use core::mem::size_of;
 
 use intrusive_collections::{intrusive_adapter, LinkedListAtomicLink};
 
@@ -101,6 +100,10 @@ impl Drop for KernelStack {
 }
 
 /// A task that executes a function to the end.
+///
+/// Each task is associated with per-task data and an optional user space.
+/// If having a user space, the task can switch to the user space to
+/// execute user code. Multiple tasks can share a single user space.
 pub struct Task {
     func: Box<dyn Fn() + Send + Sync>,
     data: Box<dyn Any + Send + Sync>,
@@ -251,11 +254,11 @@ impl TaskOptions {
         self
     }
 
-    /// Builds a new task but not run it immediately.
+    /// Build a new task without running it immediately.
     pub fn build(self) -> Result<Arc<Task>> {
         /// all task will entering this function
         /// this function is mean to executing the task_fn in Task
-        fn kernel_task_entry() {
+        extern "sysv64" fn kernel_task_entry() {
             let current_task = current_task()
                 .expect("no current task, it should have current task in kernel task entry");
             current_task.func.call(());
@@ -278,52 +281,23 @@ impl TaskOptions {
 
         result.task_inner.lock().task_status = TaskStatus::Runnable;
         result.task_inner.lock().ctx.rip = kernel_task_entry as usize;
-        // Subtract 8 bytes to reserve space for the return address, otherwise
-        // we will write across the page bondary.
+        // We should reserve space for the return address in the stack, otherwise
+        // we will write across the page boundary due to the implementation of
+        // the context switch.
+        // According to the System V AMD64 ABI, the stack pointer should be aligned
+        // to at least 16 bytes. And a larger alignment is needed if larger arguments
+        // are passed to the function. The `kernel_task_entry` function does not
+        // have any arguments, so we only need to align the stack pointer to 16 bytes.
         result.task_inner.lock().ctx.regs.rsp =
-            (crate::vm::paddr_to_vaddr(result.kstack.end_paddr() - size_of::<u64>())) as u64;
+            (crate::vm::paddr_to_vaddr(result.kstack.end_paddr() - 16)) as u64;
 
         Ok(Arc::new(result))
     }
 
-    /// Builds a new task and run it immediately.
-    ///
-    /// Each task is associated with a per-task data and an optional user space.
-    /// If having a user space, then the task can switch to the user space to
-    /// execute user code. Multiple tasks can share a single user space.
+    /// Build a new task and run it immediately.
     pub fn spawn(self) -> Result<Arc<Task>> {
-        /// all task will entering this function
-        /// this function is mean to executing the task_fn in Task
-        fn kernel_task_entry() {
-            let current_task = current_task()
-                .expect("no current task, it should have current task in kernel task entry");
-            current_task.func.call(());
-            current_task.exit();
-        }
-        let result = Task {
-            func: self.func.unwrap(),
-            data: self.data.unwrap(),
-            user_space: self.user_space,
-            task_inner: SpinLock::new(TaskInner {
-                task_status: TaskStatus::Runnable,
-                ctx: TaskContext::default(),
-            }),
-            exit_code: 0,
-            kstack: KernelStack::new_with_guard_page()?,
-            link: LinkedListAtomicLink::new(),
-            priority: self.priority,
-            cpu_affinity: self.cpu_affinity,
-        };
-
-        result.task_inner.lock().task_status = TaskStatus::Runnable;
-        result.task_inner.lock().ctx.rip = kernel_task_entry as usize;
-        // Subtract 8 bytes to reserve space for the return address, otherwise
-        // we will write across the page bondary.
-        result.task_inner.lock().ctx.regs.rsp =
-            (crate::vm::paddr_to_vaddr(result.kstack.end_paddr() - size_of::<u64>())) as u64;
-
-        let arc_self = Arc::new(result);
-        arc_self.run();
-        Ok(arc_self)
+        let task = self.build()?;
+        task.run();
+        Ok(task)
     }
 }