refactor: better ABI for SignalDestination passing

kernel/src/proc/mod.rs

@@ -3,10 +3,11 @@
 use crate::init;
 use crate::sync::IrqSafeSpinLock;
 use alloc::collections::BTreeMap;
+use libsys::proc::Pid;
 
 pub mod elf;
 pub mod process;
-pub use process::{Pid, Process, ProcessRef, State as ProcessState};
+pub use process::{Process, ProcessRef, State as ProcessState};
 pub mod io;
 pub use io::ProcessIo;
 

kernel/src/proc/process.rs

@@ -11,23 +11,13 @@ use alloc::rc::Rc;
 use core::cell::UnsafeCell;
 use core::fmt;
 use core::sync::atomic::{AtomicU32, Ordering};
-use libsys::{error::Errno, signal::Signal};
+use libsys::{error::Errno, signal::Signal, proc::{ExitCode, Pid}};
 
 pub use crate::arch::platform::context::{self, Context};
 
 /// Wrapper type for a process struct reference
 pub type ProcessRef = Rc<Process>;
 
-/// Wrapper type for process exit code
-#[derive(Clone, Copy, PartialOrd, Ord, PartialEq, Eq, Debug)]
-#[repr(transparent)]
-pub struct ExitCode(i32);
-
-/// Wrapper type for process ID
-#[derive(Clone, Copy, PartialOrd, Ord, PartialEq, Eq)]
-#[repr(transparent)]
-pub struct Pid(u32);
-
 /// List of possible process states
 #[derive(Clone, Copy, Debug, PartialEq)]
 pub enum State {
@@ -64,93 +54,6 @@ pub struct Process {
     pub io: IrqSafeSpinLock<ProcessIo>,
 }
 
-impl From<i32> for ExitCode {
-    fn from(f: i32) -> Self {
-        Self(f)
-    }
-}
-
-impl From<()> for ExitCode {
-    fn from(_: ()) -> Self {
-        Self(0)
-    }
-}
-
-impl From<ExitCode> for i32 {
-    fn from(f: ExitCode) -> Self {
-        f.0
-    }
-}
-
-impl Pid {
-    /// Kernel idle process always has PID of zero
-    pub const IDLE: Self = Self(Self::KERNEL_BIT);
-
-    const KERNEL_BIT: u32 = 1 << 31;
-
-    /// Constructs an instance of user-space PID
-    pub const fn user(id: u32) -> Self {
-        assert!(id < 256, "PID is too high");
-        Self(id)
-    }
-
-    /// Allocates a new kernel-space PID
-    pub fn new_kernel() -> Self {
-        static LAST: AtomicU32 = AtomicU32::new(0);
-        let id = LAST.fetch_add(1, Ordering::Relaxed);
-        assert!(id & Self::KERNEL_BIT == 0, "Out of kernel PIDs");
-        Self(id | Self::KERNEL_BIT)
-    }
-
-    /// Allocates a new user-space PID.
-    ///
-    /// First user PID is #1.
-    pub fn new_user() -> Self {
-        static LAST: AtomicU32 = AtomicU32::new(1);
-        let id = LAST.fetch_add(1, Ordering::Relaxed);
-        assert!(id < 256, "Out of user PIDs");
-        Self(id)
-    }
-
-    /// Returns `true` if this PID belongs to a kernel process
-    pub fn is_kernel(self) -> bool {
-        self.0 & Self::KERNEL_BIT != 0
-    }
-
-    /// Returns address space ID of a user-space process.
-    ///
-    /// Panics if called on kernel process PID.
-    pub fn asid(self) -> u8 {
-        assert!(!self.is_kernel());
-        self.0 as u8
-    }
-
-    /// Returns bit value of this pid
-    pub const fn value(self) -> u32 {
-        self.0
-    }
-
-    /// Constructs [Pid] from raw [u32] value
-    ///
-    /// # Safety
-    ///
-    /// Unsafe: does not check `num`
-    pub const unsafe fn from_raw(num: u32) -> Self {
-        Self(num)
-    }
-}
-
-impl fmt::Display for Pid {
-    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-        write!(
-            f,
-            "Pid(#{}{})",
-            if self.is_kernel() { "K" } else { "U" },
-            self.0 & !Self::KERNEL_BIT
-        )
-    }
-}
-
 impl Process {
     const USTACK_VIRT_TOP: usize = 0x100000000;
     const USTACK_PAGES: usize = 4;
@@ -341,7 +244,7 @@ impl Process {
 
     /// Creates a new kernel process
     pub fn new_kernel(entry: extern "C" fn(usize) -> !, arg: usize) -> Result<ProcessRef, Errno> {
-        let id = Pid::new_kernel();
+        let id = new_kernel_pid();
         let res = Rc::new(Self {
             ctx: UnsafeCell::new(Context::kernel(entry as usize, arg)),
             signal_ctx: UnsafeCell::new(Context::empty()),
@@ -359,7 +262,7 @@ impl Process {
                 state: State::Ready,
             }),
         });
-        debugln!("New kernel process: {}", id);
+        debugln!("New kernel process: {:?}", id);
         assert!(PROCESSES.lock().insert(id, res.clone()).is_none());
         Ok(res)
     }
@@ -370,7 +273,7 @@ impl Process {
         let src_io = self.io.lock();
         let mut src_inner = self.inner.lock();
 
-        let dst_id = Pid::new_user();
+        let dst_id = new_user_pid();
         let dst_space = src_inner.space.as_mut().unwrap().fork()?;
         let dst_space_phys = (dst_space as *mut _ as usize) - mem::KERNEL_OFFSET;
         let dst_ttbr0 = dst_space_phys | ((dst_id.asid() as usize) << 48);
@@ -392,7 +295,7 @@ impl Process {
                 wait_flag: false,
             }),
         });
-        debugln!("Process {} forked into {}", src_inner.id, dst_id);
+        debugln!("Process {:?} forked into {:?}", src_inner.id, dst_id);
         assert!(PROCESSES.lock().insert(dst_id, dst).is_none());
         SCHED.enqueue(dst_id);
 
@@ -405,7 +308,7 @@ impl Process {
         let drop = {
             let mut lock = self.inner.lock();
             let drop = lock.state == State::Running;
-            infoln!("Process {} is exiting: {:?}", lock.id, status);
+            infoln!("Process {:?} is exiting: {:?}", lock.id, status);
             assert!(lock.exit.is_none());
             lock.exit = Some(status);
             lock.state = State::Finished;
@@ -450,7 +353,7 @@ impl Process {
             if let Some(r) = proc.collect() {
                 // TODO drop the process struct itself
                 PROCESSES.lock().remove(&proc.id());
-                debugln!("pid {} has {} refs", proc.id(), Rc::strong_count(&proc));
+                debugln!("pid {:?} has {} refs", proc.id(), Rc::strong_count(&proc));
                 return Ok(r);
             }
 
@@ -477,9 +380,9 @@ impl Process {
                 proc_lock.remove(&old_pid).is_some(),
                 "Failed to downgrade kernel process (remove kernel pid)"
             );
-            lock.id = Pid::new_user();
+            lock.id = new_user_pid();
             debugln!(
-                "Process downgrades from kernel to user: {} -> {}",
+                "Process downgrades from kernel to user: {:?} -> {:?}",
                 old_pid,
                 lock.id
             );
@@ -541,6 +444,23 @@ impl Process {
 
 impl Drop for Process {
     fn drop(&mut self) {
-        debugln!("Dropping process {}", self.id());
+        debugln!("Dropping process {:?}", self.id());
     }
 }
+
+/// Allocates a new kernel-space PID
+pub fn new_kernel_pid() -> Pid {
+    static LAST: AtomicU32 = AtomicU32::new(0);
+    let id = LAST.fetch_add(1, Ordering::Relaxed);
+    Pid::kernel(id)
+}
+
+/// Allocates a new user-space PID.
+///
+/// First user PID is #1.
+pub fn new_user_pid() -> Pid {
+    static LAST: AtomicU32 = AtomicU32::new(1);
+    let id = LAST.fetch_add(1, Ordering::Relaxed);
+    assert!(id < 256, "Out of user PIDs");
+    Pid::user(id)
+}

kernel/src/proc/wait.rs

@@ -2,11 +2,11 @@
 
 use crate::arch::machine;
 use crate::dev::timer::TimestampSource;
-use crate::proc::{self, sched::SCHED, Pid, Process, ProcessRef};
+use crate::proc::{self, sched::SCHED, Process, ProcessRef};
 use crate::sync::IrqSafeSpinLock;
 use alloc::collections::LinkedList;
 use core::time::Duration;
-use libsys::{error::Errno, stat::FdSet};
+use libsys::{error::Errno, stat::FdSet, proc::Pid};
 
 /// Wait channel structure. Contains a queue of processes
 /// waiting for some event to happen.

kernel/src/syscall/mod.rs

@@ -2,7 +2,7 @@
 
 use crate::arch::platform::exception::ExceptionFrame;
 use crate::debug::Level;
-use crate::proc::{elf, wait, Pid, Process, ProcessIo};
+use crate::proc::{elf, wait, Process, ProcessIo};
 use core::cmp::Ordering;
 use core::mem::size_of;
 use core::ops::DerefMut;
@@ -11,7 +11,8 @@ use libsys::{
     abi,
     error::Errno,
     ioctl::IoctlCmd,
-    signal::Signal,
+    proc::Pid,
+    signal::{Signal, SignalDestination},
     stat::{FdSet, FileMode, OpenFlags, Stat, AT_EMPTY_PATH, AT_FDCWD},
     traits::{Read, Write},
 };
@@ -182,16 +183,16 @@ pub fn syscall(num: usize, args: &[usize]) -> Result<usize, Errno> {
             panic!("This code won't run");
         }
         abi::SYS_EX_KILL => {
-            let pid = args[0] as i32;
+            let target = SignalDestination::from(args[0] as isize);
             let signal = Signal::try_from(args[1] as u32)?;
-            let proc = match pid.cmp(&0) {
-                Ordering::Greater => {
-                    Process::get(unsafe { Pid::from_raw(pid as u32) }).ok_or(Errno::DoesNotExist)?
-                }
-                Ordering::Equal => Process::current(),
-                Ordering::Less => todo!(),
+
+            match target {
+                SignalDestination::This => Process::current().set_signal(signal),
+                SignalDestination::Process(pid) => Process::get(pid)
+                    .ok_or(Errno::DoesNotExist)?
+                    .set_signal(signal),
+                _ => todo!(),
             };
-            proc.set_signal(signal);
             Ok(0)
         }
 

libsys/src/calls.rs

@@ -1,7 +1,7 @@
 use crate::abi;
 use crate::{
     ioctl::IoctlCmd,
-    signal::Signal,
+    signal::{Signal, SignalDestination},
     stat::{FdSet, FileMode, OpenFlags, Stat},
 };
 
@@ -214,8 +214,8 @@ pub unsafe fn sys_ex_sigreturn() -> ! {
 }
 
 #[inline(always)]
-pub unsafe fn sys_ex_kill(pid: u32, signum: Signal) -> i32 {
-    syscall!(abi::SYS_EX_KILL, argn!(pid), argn!(signum as u32)) as i32
+pub unsafe fn sys_ex_kill(pid: SignalDestination, signum: Signal) -> i32 {
+    syscall!(abi::SYS_EX_KILL, argn!(isize::from(pid)), argn!(signum as u32)) as i32
 }
 
 #[inline(always)]

libsys/src/lib.rs

@@ -1,17 +1,18 @@
-#![feature(asm)]
+#![feature(asm, const_panic)]
 #![no_std]
 
 #[macro_use]
 extern crate bitflags;
 
 pub mod abi;
-pub mod stat;
-pub mod ioctl;
-pub mod termios;
-pub mod signal;
 pub mod error;
-pub mod path;
+pub mod ioctl;
 pub mod mem;
+pub mod path;
+pub mod proc;
+pub mod signal;
+pub mod stat;
+pub mod termios;
 pub mod traits;
 
 #[cfg(feature = "user")]

libsys/src/proc.rs

@@ -0,0 +1,116 @@
+use crate::error::Errno;
+use core::convert::TryFrom;
+use core::fmt;
+
+/// Wrapper type for process exit code
+#[derive(Clone, Copy, PartialOrd, Ord, PartialEq, Eq, Debug)]
+#[repr(transparent)]
+pub struct ExitCode(i32);
+
+/// Wrapper type for process ID
+#[derive(Clone, Copy, PartialOrd, Ord, PartialEq, Eq)]
+#[repr(transparent)]
+pub struct Pid(u32);
+
+#[derive(Clone, Copy, PartialOrd, Ord, PartialEq, Eq, Debug)]
+#[repr(transparent)]
+pub struct Pgid(u32);
+
+impl From<i32> for ExitCode {
+    fn from(f: i32) -> Self {
+        Self(f)
+    }
+}
+
+impl From<()> for ExitCode {
+    fn from(_: ()) -> Self {
+        Self(0)
+    }
+}
+
+impl From<ExitCode> for i32 {
+    fn from(f: ExitCode) -> Self {
+        f.0
+    }
+}
+
+impl Pid {
+    /// Kernel idle process always has PID of zero
+    pub const IDLE: Self = Self(Self::KERNEL_BIT);
+
+    const KERNEL_BIT: u32 = 1 << 31;
+
+    /// Constructs an instance of user-space PID
+    pub const fn user(id: u32) -> Self {
+        assert!(id < 256, "PID is too high");
+        Self(id)
+    }
+
+    /// Constructs an instance of kernel-space PID
+    pub const fn kernel(id: u32) -> Self {
+        assert!(id & Self::KERNEL_BIT == 0, "PID is too high");
+        Self(id | Self::KERNEL_BIT)
+    }
+
+    /// Returns `true` if this PID belongs to a kernel process
+    pub fn is_kernel(self) -> bool {
+        self.0 & Self::KERNEL_BIT != 0
+    }
+
+    /// Returns address space ID of a user-space process.
+    ///
+    /// Panics if called on kernel process PID.
+    pub fn asid(self) -> u8 {
+        assert!(!self.is_kernel());
+        self.0 as u8
+    }
+
+    /// Returns bit value of this pid
+    pub const fn value(self) -> u32 {
+        self.0
+    }
+
+    /// Constructs [Pid] from raw [u32] value
+    ///
+    /// # Safety
+    ///
+    /// Unsafe: does not check `num`
+    pub const unsafe fn from_raw(num: u32) -> Self {
+        Self(num)
+    }
+}
+
+impl fmt::Debug for Pid {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        write!(
+            f,
+            "Pid(#{}{})",
+            if self.is_kernel() { "K" } else { "U" },
+            self.0 & !Self::KERNEL_BIT
+        )
+    }
+}
+
+impl TryFrom<Pid> for Pgid {
+    type Error = Errno;
+
+    fn try_from(pid: Pid) -> Result<Pgid, Errno> {
+        if pid.is_kernel() {
+            Err(Errno::InvalidArgument)
+        } else {
+            Ok(Pgid(pid.0))
+        }
+    }
+}
+
+impl From<u32> for Pgid {
+    fn from(p: u32) -> Pgid {
+        Self(p)
+    }
+}
+
+impl From<Pgid> for u32 {
+    fn from(p: Pgid) -> u32 {
+        p.0
+    }
+}

libsys/src/signal.rs

@@ -1,4 +1,5 @@
 use crate::error::Errno;
+use crate::proc::{Pid, Pgid};
 
 #[derive(Clone, Copy, PartialEq, Debug)]
 #[repr(u32)]
@@ -11,6 +12,39 @@ pub enum Signal {
     InvalidSystemCall = 31
 }
 
+#[derive(Clone, Copy, PartialEq, Debug)]
+pub enum SignalDestination {
+    Group(Pgid),
+    Process(Pid),
+    All,
+    This
+}
+
+impl From<isize> for SignalDestination {
+    fn from(num: isize) -> Self {
+        if num > 0 {
+            Self::Process(Pid::user(num as u32))
+        } else if num == 0 {
+            Self::This
+        } else if num == -1 {
+            Self::All
+        } else {
+            Self::Group(Pgid::from((-num) as u32))
+        }
+    }
+}
+
+impl From<SignalDestination> for isize {
+    fn from(p: SignalDestination) -> isize {
+        match p {
+            SignalDestination::Process(pid) => pid.value() as isize,
+            SignalDestination::Group(pgid) => -(u32::from(pgid) as isize),
+            SignalDestination::This => 0,
+            SignalDestination::All => -1
+        }
+    }
+}
+
 impl TryFrom<u32> for Signal {
     type Error = Errno;
 

libusr/src/lib.rs

@@ -7,7 +7,7 @@ pub mod io;
 pub mod os;
 
 pub mod sys {
-    pub use libsys::signal::Signal;
+    pub use libsys::signal::{Signal, SignalDestination};
     pub use libsys::termios;
     pub use libsys::calls::*;
     pub use libsys::stat::{self, STDERR_FILENO, STDIN_FILENO, STDOUT_FILENO};

user/src/shell/main.rs

@@ -4,7 +4,7 @@
 #[macro_use]
 extern crate libusr;
 
-use libusr::sys::Signal;
+use libusr::sys::{Signal, SignalDestination};
 use libusr::sys::stat::FdSet;
 
 fn readline(fd: i32, buf: &mut [u8]) -> Result<&str, ()> {
@@ -48,7 +48,7 @@ fn main() -> i32 {
 
         if line == "test" {
             unsafe {
-                libusr::sys::sys_ex_kill(0, Signal::Interrupt);
+                libusr::sys::sys_ex_kill(SignalDestination::This, Signal::Interrupt);
             }
             trace!("Returned from signal");
             continue;