Clean up ContinueOperation (#502)

Some clean up in preparation for more task/signal work:

* Define one `ContinueOperation` for all shims, with just
`ContinueGuest` and `ExitThread`.

* Remove exit status from `ExitThread`--this, plus the distinction
between thread and process exit, will be communicated separately in
future PRs, on a per-shim basis.

* Unify Linux shim entry/exit code.

* Record the `is_exiting` state of a Linux thread in `Task` and use it
to determine whether to return `ExitThread` from a shim entrypoint. This
is a more natural fit for Linux signal/termination handling.

Author: jstarks

litebox/src/shim.rs

@@ -19,10 +19,6 @@ pub trait EnterShim: Send + Sync {
     ///
     /// FUTURE: use a single per-architecture type for all shims and platforms.
     type ExecutionContext;
-    /// The operation the platform should take after returning from the shim.
-    ///
-    /// FUTURE: use a single per-LiteBox type for all shims and platforms.
-    type ContinueOperation;
 
     /// Initialize a new thread. Must be called by the platform exactly once
     /// before running the thread in the guest for the first time.
@@ -41,13 +37,13 @@ pub trait EnterShim: Send + Sync {
     ///     crate::platform::ThreadProvider::spawn_thread
     /// [`ThreadProvider::current_thread`]:
     ///     crate::platform::ThreadProvider::current_thread
-    fn init(&self, ctx: &mut Self::ExecutionContext) -> Self::ContinueOperation;
+    fn init(&self, ctx: &mut Self::ExecutionContext) -> ContinueOperation;
 
     /// Handle a syscall.
     ///
     /// The platform should call this in response to `syscall` on x86_64 and
     /// `int 0x80` on x86.
-    fn syscall(&self, ctx: &mut Self::ExecutionContext) -> Self::ContinueOperation;
+    fn syscall(&self, ctx: &mut Self::ExecutionContext) -> ContinueOperation;
 
     /// Handle a hardware exception.
     ///
@@ -56,15 +52,24 @@ pub trait EnterShim: Send + Sync {
         &self,
         ctx: &mut Self::ExecutionContext,
         info: &ExceptionInfo,
-    ) -> Self::ContinueOperation;
+    ) -> ContinueOperation;
 
     /// Handle an interrupt signaled by
     /// [`ThreadProvider::interrupt_thread`](crate::platform::ThreadProvider::interrupt_thread).
     ///
     /// Note that if another event occurs (e.g., a syscall or exception) while
     /// the thread is interrupted, the platform may just call the corresponding
     /// handler instead of this one.
-    fn interrupt(&self, ctx: &mut Self::ExecutionContext) -> Self::ContinueOperation;
+    fn interrupt(&self, ctx: &mut Self::ExecutionContext) -> ContinueOperation;
+}
+
+/// The operation to perform after returning from a shim handler
+#[derive(Copy, Clone, Debug, PartialEq, Eq)]
+pub enum ContinueOperation {
+    /// Resume execution of the guest.
+    ResumeGuest,
+    /// Exit the current thread.
+    ExitThread,
 }
 
 /// Information about a hardware exception.

litebox_common_linux/src/lib.rs

@@ -1915,9 +1915,7 @@ pub enum SyscallRequest<Platform: litebox::platform::RawPointerProvider> {
         oldact: Option<Platform::RawMutPointer<SigAction>>,
         sigsetsize: usize,
     },
-    RtSigreturn {
-        stack: usize,
-    },
+    RtSigreturn,
     Ioctl {
         fd: i32,
         arg: IoctlArg<Platform>,
@@ -2246,13 +2244,6 @@ pub enum SyscallRequest<Platform: litebox::platform::RawPointerProvider> {
     },
 }
 
-pub enum ContinueOperation {
-    ResumeGuest,
-    ExitThread(i32),
-    ExitProcess(i32),
-    RtSigreturn(usize), // TEMP: remove once this is handled natively by the shim
-}
-
 impl<Platform: litebox::platform::RawPointerProvider> SyscallRequest<Platform> {
     /// Take the raw syscall number and arguments, and provide a stronger-typed `SyscallRequest`.
     ///
@@ -2388,10 +2379,7 @@ impl<Platform: litebox::platform::RawPointerProvider> SyscallRequest<Platform> {
                     return Err(errno::Errno::EINVAL);
                 }
             }
-            #[cfg(target_arch = "x86_64")]
-            Sysno::rt_sigreturn => SyscallRequest::RtSigreturn { stack: ctx.rsp },
-            #[cfg(target_arch = "x86")]
-            Sysno::rt_sigreturn => SyscallRequest::RtSigreturn { stack: ctx.esp },
+            Sysno::rt_sigreturn => SyscallRequest::RtSigreturn,
             Sysno::ioctl => SyscallRequest::Ioctl {
                 fd: ctx.sys_req_arg(0),
                 arg: {

litebox_common_optee/src/lib.rs

@@ -124,12 +124,6 @@ pub enum SyscallRequest<Platform: litebox::platform::RawPointerProvider> {
     },
 }
 
-pub enum ContinueOperation {
-    ResumeGuest,
-    ExitThread(usize),
-    ExitProcess(usize),
-}
-
 // `litebox_common_optee` does use error codes for OP-TEE-like world (TAs) and Linux-like world (the LVBS platform).
 // for the below syscall handling, we use Linux error codes (i.e., `Errno`) because any errors will be returned
 // to the LVBS platform or runner.

litebox_platform_linux_userland/src/lib.rs

@@ -13,30 +13,17 @@ use litebox::fs::OFlags;
 use litebox::platform::UnblockedOrTimedOut;
 use litebox::platform::page_mgmt::{FixedAddressBehavior, MemoryRegionPermissions};
 use litebox::platform::{ImmediatelyWokenUp, RawConstPointer};
+use litebox::shim::ContinueOperation;
 use litebox::utils::{ReinterpretSignedExt, ReinterpretUnsignedExt as _, TruncateExt};
 use litebox_common_linux::{MRemapFlags, MapFlags, ProtFlags, PunchthroughSyscall};
 
 mod syscall_intercept;
 
 extern crate alloc;
 
-cfg_if::cfg_if! {
-    if #[cfg(feature = "linux_syscall")] {
-        use litebox_common_linux::ContinueOperation;
-        pub type SyscallReturnType = litebox_common_linux::ContinueOperation;
-    } else if #[cfg(feature = "optee_syscall")] {
-        use litebox_common_optee::ContinueOperation;
-        pub type SyscallReturnType = litebox_common_optee::ContinueOperation;
-    } else {
-        compile_error!(r##"No syscall handler specified."##);
-    }
-}
 /// The syscall handler passed down from the shim.
 static SHIM: std::sync::OnceLock<
-    &'static dyn litebox::shim::EnterShim<
-        ExecutionContext = litebox_common_linux::PtRegs,
-        ContinueOperation = ContinueOperation,
-    >,
+    &'static dyn litebox::shim::EnterShim<ExecutionContext = litebox_common_linux::PtRegs>,
 > = const { std::sync::OnceLock::new() };
 
 /// The userland Linux platform.
@@ -185,10 +172,7 @@ impl LinuxUserland {
     /// Panics if the function has already been invoked earlier.
     pub fn register_shim(
         &self,
-        shim: &'static dyn litebox::shim::EnterShim<
-            ExecutionContext = litebox_common_linux::PtRegs,
-            ContinueOperation = ContinueOperation,
-        >,
+        shim: &'static dyn litebox::shim::EnterShim<ExecutionContext = litebox_common_linux::PtRegs>,
     ) {
         SHIM.set(shim)
             .ok()
@@ -1674,10 +1658,7 @@ extern "C-unwind" fn interrupt_handler(ctx: &mut litebox_common_linux::PtRegs) {
 fn call_shim(
     ctx: &mut litebox_common_linux::PtRegs,
     f: impl FnOnce(
-        &dyn litebox::shim::EnterShim<
-            ContinueOperation = ContinueOperation,
-            ExecutionContext = litebox_common_linux::PtRegs,
-        >,
+        &dyn litebox::shim::EnterShim<ExecutionContext = litebox_common_linux::PtRegs>,
         &mut litebox_common_linux::PtRegs,
     ) -> ContinueOperation,
 ) {
@@ -1703,24 +1684,7 @@ fn call_shim(
     let op = f(shim, ctx);
     match op {
         ContinueOperation::ResumeGuest => unsafe { switch_to_guest(ctx) },
-        ContinueOperation::ExitThread(status) | ContinueOperation::ExitProcess(status) => {
-            #[cfg(target_arch = "x86_64")]
-            {
-                cfg_if::cfg_if! {
-                    if #[cfg(feature = "linux_syscall")] {
-                        ctx.rax = status.reinterpret_as_unsigned() as usize;
-                    } else if #[cfg(feature = "optee_syscall")] {
-                        ctx.rax = status;
-                    }
-                }
-            }
-            #[cfg(target_arch = "x86")]
-            {
-                ctx.eax = status.reinterpret_as_unsigned() as usize;
-            }
-        }
-        #[cfg(feature = "linux_syscall")]
-        ContinueOperation::RtSigreturn(..) => unreachable!(),
+        ContinueOperation::ExitThread => {}
     }
 }
 

litebox_platform_lvbs/src/lib.rs

@@ -381,10 +381,7 @@ impl<Host: HostInterface> LinuxKernel<Host> {
     /// Register the shim. This function must be called for each core to program
     /// its MSRs.
     pub fn register_shim(
-        shim: &'static dyn litebox::shim::EnterShim<
-            ExecutionContext = litebox_common_linux::PtRegs,
-            ContinueOperation = syscall_entry::SyscallReturnType,
-        >,
+        shim: &'static dyn litebox::shim::EnterShim<ExecutionContext = litebox_common_linux::PtRegs>,
     ) {
         syscall_entry::init(shim);
     }

litebox_platform_lvbs/src/syscall_entry.rs

@@ -5,6 +5,7 @@ use crate::{
     user_context::UserSpaceManagement,
 };
 use core::arch::{asm, naked_asm};
+use litebox::shim::ContinueOperation;
 use litebox_common_linux::PtRegs;
 use litebox_common_optee::SyscallContext;
 use x86_64::{
@@ -38,24 +39,8 @@ use x86_64::{
 // r11: userspace rflags
 // Note. rsp should point to the userspace stack before calling `sysretq`
 
-cfg_if::cfg_if! {
-    if #[cfg(feature = "linux_syscall")] {
-        use litebox_common_linux::ContinueOperation;
-        pub type SyscallReturnType = ContinueOperation;
-    } else if #[cfg(feature = "optee_syscall")] {
-        use litebox_common_optee::ContinueOperation;
-        pub type SyscallReturnType = ContinueOperation;
-    } else {
-        compile_error!(r##"No syscall handler specified."##);
-    }
-}
-
-static SHIM: spin::Once<
-    &'static dyn litebox::shim::EnterShim<
-        ExecutionContext = PtRegs,
-        ContinueOperation = ContinueOperation,
-    >,
-> = spin::Once::new();
+static SHIM: spin::Once<&'static dyn litebox::shim::EnterShim<ExecutionContext = PtRegs>> =
+    spin::Once::new();
 
 #[cfg(target_arch = "x86_64")]
 #[derive(Clone, Copy, Debug)]
@@ -171,20 +156,7 @@ fn syscall_entry(sysnr: u64, ctx_raw: *const SyscallContextRaw) -> usize {
 
     // call the syscall handler passed down from the shim
     let sysret = match shim.syscall(&mut ctx) {
-        ContinueOperation::ResumeGuest => ctx.rax,
-        ContinueOperation::ExitThread(status) | ContinueOperation::ExitProcess(status) => {
-            cfg_if::cfg_if! {
-                if #[cfg(feature = "linux_syscall")] {
-                    status.cast_unsigned() as usize
-                } else if #[cfg(feature = "optee_syscall")] {
-                    status
-                } else {
-                    compile_error!(r##"No syscall handler specified."##);
-                }
-            }
-        }
-        #[cfg(feature = "linux_syscall")]
-        ContinueOperation::RtSigreturn(..) => unreachable!(),
+        ContinueOperation::ResumeGuest | ContinueOperation::ExitThread => ctx.rax,
     };
 
     // TODO: We should decide whether we place this function here, OP-TEE shim, or separate it into
@@ -257,12 +229,7 @@ const STACK_ALIGNMENT: isize = -16;
 /// # Panics
 /// Panics if GDT is not initialized for the current core.
 #[cfg(target_arch = "x86_64")]
-pub(crate) fn init(
-    shim: &'static dyn litebox::shim::EnterShim<
-        ExecutionContext = PtRegs,
-        ContinueOperation = ContinueOperation,
-    >,
-) {
+pub(crate) fn init(shim: &'static dyn litebox::shim::EnterShim<ExecutionContext = PtRegs>) {
     SHIM.call_once(|| shim);
 
     // enable 64-bit syscall/sysret

litebox_platform_windows_userland/src/lib.rs

@@ -18,9 +18,9 @@ use litebox::platform::page_mgmt::{
     AllocationError, FixedAddressBehavior, MemoryRegionPermissions,
 };
 use litebox::platform::{ImmediatelyWokenUp, RawMutPointer};
-use litebox::shim::Exception;
-use litebox::utils::{ReinterpretUnsignedExt as _, TruncateExt as _};
-use litebox_common_linux::{ContinueOperation, PunchthroughSyscall};
+use litebox::shim::{ContinueOperation, Exception};
+use litebox::utils::TruncateExt as _;
+use litebox_common_linux::PunchthroughSyscall;
 
 use windows_sys::Win32::Foundation::{self as Win32_Foundation, FILETIME};
 use windows_sys::Win32::{
@@ -46,10 +46,7 @@ thread_local! {
 
 /// The registered shim.
 static SHIM: std::sync::OnceLock<
-    &'static dyn litebox::shim::EnterShim<
-        ExecutionContext = litebox_common_linux::PtRegs,
-        ContinueOperation = ContinueOperation,
-    >,
+    &'static dyn litebox::shim::EnterShim<ExecutionContext = litebox_common_linux::PtRegs>,
 > = std::sync::OnceLock::new();
 
 /// The userland Windows platform.
@@ -256,10 +253,7 @@ impl WindowsUserland {
     /// Panics if the function has already been invoked earlier.
     pub fn register_shim(
         &self,
-        shim: &'static dyn litebox::shim::EnterShim<
-            ExecutionContext = litebox_common_linux::PtRegs,
-            ContinueOperation = ContinueOperation,
-        >,
+        shim: &'static dyn litebox::shim::EnterShim<ExecutionContext = litebox_common_linux::PtRegs>,
     ) {
         SHIM.set(shim)
             .ok()
@@ -1744,10 +1738,7 @@ unsafe extern "C-unwind" fn interrupt_handler(ctx: &mut litebox_common_linux::Pt
 fn call_shim(
     ctx: &mut litebox_common_linux::PtRegs,
     f: impl FnOnce(
-        &dyn litebox::shim::EnterShim<
-            ContinueOperation = ContinueOperation,
-            ExecutionContext = litebox_common_linux::PtRegs,
-        >,
+        &dyn litebox::shim::EnterShim<ExecutionContext = litebox_common_linux::PtRegs>,
         &mut litebox_common_linux::PtRegs,
         bool,
     ) -> ContinueOperation,
@@ -1760,10 +1751,7 @@ fn call_shim(
     let op = f(shim, ctx, interrupt);
     match op {
         ContinueOperation::ResumeGuest => unsafe { switch_to_guest(ctx) },
-        ContinueOperation::ExitThread(status) | ContinueOperation::ExitProcess(status) => {
-            ctx.rax = status.reinterpret_as_unsigned() as usize;
-        }
-        ContinueOperation::RtSigreturn(..) => unreachable!(),
+        ContinueOperation::ExitThread => {}
     }
 }
 

litebox_runner_snp/src/main.rs

@@ -8,9 +8,8 @@ mod globals;
 extern crate alloc;
 
 use alloc::borrow::ToOwned;
-use litebox::shim::EnterShim as _;
+use litebox::shim::{ContinueOperation, EnterShim as _};
 use litebox::utils::{ReinterpretUnsignedExt as _, TruncateExt as _};
-use litebox_common_linux::ContinueOperation;
 use litebox_platform_linux_kernel::{HostInterface, host::snp::ghcb::ghcb_prints};
 
 #[unsafe(no_mangle)]
@@ -171,11 +170,7 @@ pub extern "C" fn sandbox_task_exit() {
 #[unsafe(no_mangle)]
 pub extern "C" fn do_syscall_64(pt_regs: &mut litebox_common_linux::PtRegs) {
     match litebox_shim_linux::LinuxShimEntrypoints.syscall(pt_regs) {
-        ContinueOperation::ResumeGuest => {}
-        ContinueOperation::ExitThread(status) | ContinueOperation::ExitProcess(status) => {
-            pt_regs.rax = status.cast_unsigned() as usize;
-        }
-        ContinueOperation::RtSigreturn(..) => unreachable!(),
+        ContinueOperation::ResumeGuest | ContinueOperation::ExitThread => {}
     }
 }