From 1f478925ce1a8a454a73fbf1d4e028751e73b18e Mon Sep 17 00:00:00 2001 From: Sean Cross Date: Mon, 6 Apr 2020 10:43:26 +0800 Subject: [PATCH] kernel: return context on failure Signed-off-by: Sean Cross --- kernel/src/arch/riscv/irq.rs | 4 +-- kernel/src/arch/riscv/mem.rs | 7 +++-- kernel/src/server.rs | 14 ++++++++++ kernel/src/services.rs | 42 ++++++++++++++++++++++------ kernel/src/syscall.rs | 25 ++++++++++++++--- xous-rs/src/definitions.rs | 34 +++++++++++++++++++++++ xous-rs/src/syscall.rs | 53 +++++++----------------------------- 7 files changed, 119 insertions(+), 60 deletions(-) diff --git a/kernel/src/arch/riscv/irq.rs b/kernel/src/arch/riscv/irq.rs index 031311487..12a00c99f 100644 --- a/kernel/src/arch/riscv/irq.rs +++ b/kernel/src/arch/riscv/irq.rs @@ -134,8 +134,8 @@ pub extern "C" fn trap_handler( // MemoryManagerHandle::get().print_ownership(); MemoryMapping::current().print_map(); panic!( - "error at {:08x}: memory not mapped or reserved for addr {:08x}", - pc, addr + "error {:?} at {:08x}: memory not mapped or reserved for addr {:08x}", + x, pc, addr ); }); let flags = *entry & 0x1ff; diff --git a/kernel/src/arch/riscv/mem.rs b/kernel/src/arch/riscv/mem.rs index 370315cbf..84898d080 100644 --- a/kernel/src/arch/riscv/mem.rs +++ b/kernel/src/arch/riscv/mem.rs @@ -529,11 +529,12 @@ pub fn lend_page_inner( result.map(|_| phys) } +/// Return a page from `src_space` back to `dest_space`. pub fn return_page_inner( - mm: &mut MemoryManager, + _mm: &mut MemoryManager, src_space: &MemoryMapping, src_addr: *mut usize, - dest_pid: PID, + _dest_pid: PID, dest_space: &MemoryMapping, dest_addr: *mut usize, ) -> Result { @@ -570,7 +571,7 @@ pub fn return_page_inner( unsafe { flush_mmu() }; src_space.activate(); - Ok(0) + Ok(phys) } pub fn virt_to_phys(virt: usize) -> Result { diff --git a/kernel/src/server.rs b/kernel/src/server.rs index 6d3f2fd8a..a3cf2dc72 100644 --- a/kernel/src/server.rs +++ b/kernel/src/server.rs @@ -399,6 +399,20 @@ impl Server { } } + /// Return an available context to the blocking list. This is part of the + /// error condition when a message cannot be handled but the context has + /// already been claimed. + /// + /// # Panics + /// + /// If the context cannot be returned because it is already blocking. + pub fn return_available_context(&mut self, ctx_number: CtxID) { + if self.ready_contexts & 1 << ctx_number != 0 { + panic!("tried to return context {}, but it was already blocking", ctx_number); + } + self.ready_contexts |= 1 << ctx_number; + } + /// Add the given context to the list of ready and waiting contexts. pub fn park_context(&mut self, context: CtxID) { self.ready_contexts |= 1 << context; diff --git a/kernel/src/services.rs b/kernel/src/services.rs index 3324fd4a5..5768c2dbb 100644 --- a/kernel/src/services.rs +++ b/kernel/src/services.rs @@ -663,6 +663,15 @@ impl SystemServices { /// /// # Errors /// + /// * **ShareViolation**: Tried to mutably share a region that was already + /// shared + /// * **BadAddress**: The provided address was not valid + /// * **BadAlignment**: The provided address or length was not page-aligned + /// + /// # Panics + /// + /// If the memory should have been able to go into the destination process + /// but failed, then the system panics. pub fn send_memory( &mut self, src_virt: *mut usize, @@ -684,7 +693,6 @@ impl SystemServices { } let current_pid = self.current_pid(); - /// XXX THIS SHOULD UNMAP THESE PAGES ON ERROR let src_mapping = self.get_process(current_pid)?.mapping; let dest_mapping = self.get_process(dest_pid)?.mapping; let mut mm = MemoryManagerHandle::get(); @@ -695,7 +703,6 @@ impl SystemServices { .find_virtual_address(dest_virt, len, MemoryType::Messages) .or_else(|e| { src_mapping.activate(); - /// XXX THIS SHOULD UNMAP THESE PAGES ON ERROR Err(e) })?; src_mapping.activate(); @@ -715,7 +722,7 @@ impl SystemServices { ) .unwrap_or_else(|e| error = Some(e)); } - error.map_or_else(|| Ok(dest_virt), |e| Err(e)) + error.map_or_else(|| Ok(dest_virt), |e| panic!("unable to send: {:?}", e)) } /// Lend memory from one process to another. @@ -739,7 +746,10 @@ impl SystemServices { /// /// # Errors /// - /// * **ShareViolation**: Tried to mutably share a region that was already shared + /// * **ShareViolation**: Tried to mutably share a region that was already + /// shared + /// * **BadAddress**: The provided address was not valid + /// * **BadAlignment**: The provided address or length was not page-aligned pub fn lend_memory( &mut self, src_virt: *mut usize, @@ -752,13 +762,13 @@ impl SystemServices { return Err(xous::Error::BadAddress); } if len & 0xfff != 0 { - return Err(xous::Error::BadAddress); + return Err(xous::Error::BadAlignment); } if src_virt as usize & 0xfff != 0 { - return Err(xous::Error::BadAddress); + return Err(xous::Error::BadAlignment); } if dest_virt as usize & 0xfff != 0 { - return Err(xous::Error::BadAddress); + return Err(xous::Error::BadAlignment); } let current_pid = self.current_pid(); @@ -795,7 +805,7 @@ impl SystemServices { 0 }); } - error.map_or_else(|| Ok(dest_virt), |e| Err(e)) + error.map_or_else(|| Ok(dest_virt), |e| panic!("unable to lend: {:?}", e)) } /// Return memory from one process back to another @@ -854,6 +864,17 @@ impl SystemServices { error.map_or_else(|| Ok(dest_virt), |e| Err(e)) } + /// Create a new thread in the current process. Execution begins at + /// `entrypoint`, with the stack pointer set to `stack_pointer`. A single + /// argument will be passed to the new function. + /// + /// The return address of this thread will be `EXIT_THREAD`, which the + /// kernel can trap on to indicate a thread exited. + /// + /// # Errors + /// + /// * **ContextNotAvailable**: The process has used all of its context + /// slots. pub fn spawn_thread( &mut self, entrypoint: *mut usize, @@ -893,6 +914,11 @@ impl SystemServices { /// Allocate a new server ID for this process and return the address. If the /// server table is full, return an error. + /// + /// # Errors + /// + /// * **OutOfMemory**: A new page could not be assigned to store the server + /// queue. pub fn create_server(&mut self, name: usize) -> Result { println!("Looking through server list for free server"); println!( diff --git a/kernel/src/syscall.rs b/kernel/src/syscall.rs index 4843c139c..63bf2bfe8 100644 --- a/kernel/src/syscall.rs +++ b/kernel/src/syscall.rs @@ -369,7 +369,11 @@ pub fn handle(call: SysCall) -> core::result::Result // Unblock the client context to allow it to continue. println!("Unblocking PID {} CTX {}", client_pid, client_ctx); ss.ready_context(client_pid, client_ctx)?; - ss.set_context_result(client_pid, client_ctx, xous::Result::MessageResult(arg1, arg2))?; + ss.set_context_result( + client_pid, + client_ctx, + xous::Result::MessageResult(arg1, arg2), + )?; Ok(xous::Result::Ok) } SysCall::SendMessage(cid, message) => { @@ -462,11 +466,24 @@ pub fn handle(call: SysCall) -> core::result::Result "There are contexts available to handle this message. Marking PID {} as Ready", server_pid ); - let sender = - ss.remember_server_message(sidx, pid, context_nr, &message, client_address)?; + let sender = ss + .remember_server_message(sidx, pid, context_nr, &message, client_address) + .or_else(|e| { + ss.server_from_sidx(sidx) + .expect("server couldn't be located") + .return_available_context(context_nr); + Err(e) + })?; let envelope = MessageEnvelope { sender, message }; - ss.ready_context(server_pid, ctx_number)?; + ss.ready_context(server_pid, ctx_number) + .or_else(|e| { + ss.server_from_sidx(sidx) + .expect("server couldn't be located") + .return_available_context(context_nr); + Err(e) + })?; + if blocking { println!("Activating Server context and switching away from Client"); ss.activate_process_context(server_pid, ctx_number, !blocking, blocking) diff --git a/xous-rs/src/definitions.rs b/xous-rs/src/definitions.rs index 0816c45dd..00aba6542 100644 --- a/xous-rs/src/definitions.rs +++ b/xous-rs/src/definitions.rs @@ -173,3 +173,37 @@ impl MemoryRange { self.addr.get() as *mut usize } } + + +#[repr(C)] +#[derive(Debug, PartialEq)] +pub enum Result { + Ok, + Error(Error), + MemoryAddress(*mut u8), + MemoryRange(MemoryRange), + ReadyContexts( + usize, /* count */ + usize, + /* pid0 */ usize, /* context0 */ + usize, + /* pid1 */ usize, /* context1 */ + usize, + /* pid2 */ usize, /* context2 */ + ), + ResumeProcess, + ServerID(SID), + ConnectionID(CID), + Message(MessageEnvelope), + ThreadID(CtxID), + MessageResult(usize, usize), + UnknownResult(usize, usize, usize, usize, usize, usize, usize), +} + +impl From for Result { + fn from(e: Error) -> Self { + Result::Error(e) + } +} + +pub type SyscallResult = core::result::Result; diff --git a/xous-rs/src/syscall.rs b/xous-rs/src/syscall.rs index 4af53e175..274c9f548 100644 --- a/xous-rs/src/syscall.rs +++ b/xous-rs/src/syscall.rs @@ -1,6 +1,6 @@ use crate::{ - CpuID, CtxID, Error, MemoryAddress, MemoryFlags, MemoryMessage, MemoryRange, MemorySize, - Message, MessageEnvelope, MessageSender, ScalarMessage, CID, PID, SID, + CpuID, Error, MemoryAddress, MemoryFlags, MemoryMessage, MemoryRange, MemorySize, + Message, MessageEnvelope, MessageSender, Result, ScalarMessage, SyscallResult, CID, PID, SID, }; use num_derive::FromPrimitive; use num_traits::FromPrimitive; @@ -600,39 +600,6 @@ impl SysCall { } } -#[repr(C)] -#[derive(Debug, PartialEq)] -pub enum Result { - Ok, - Error(Error), - MemoryAddress(*mut u8), - MemoryRange(MemoryRange), - ReadyContexts( - usize, /* count */ - usize, - /* pid0 */ usize, /* context0 */ - usize, - /* pid1 */ usize, /* context1 */ - usize, - /* pid2 */ usize, /* context2 */ - ), - ResumeProcess, - ServerID(SID), - ConnectionID(CID), - Message(MessageEnvelope), - ThreadID(CtxID), - MessageResult(usize, usize), - UnknownResult(usize, usize, usize, usize, usize, usize, usize), -} - -impl From for Result { - fn from(e: Error) -> Self { - Result::Error(e) - } -} - -pub type SyscallResult = core::result::Result; - extern "Rust" { fn _xous_syscall_rust( nr: usize, @@ -790,7 +757,10 @@ pub fn receive_message(server: SID) -> core::result::Result core::result::Result<(usize, usize), Error> { +pub fn send_message( + connection: CID, + message: Message, +) -> core::result::Result<(usize, usize), Error> { let result = rsyscall(SysCall::SendMessage(connection, message)).expect("couldn't send message"); if let Result::MessageResult(arg1, arg2) = result { @@ -832,13 +802,10 @@ pub fn rsyscall(call: SysCall) -> SyscallResult { pub unsafe fn dangerous_syscall(call: SysCall) -> SyscallResult { use core::mem::{transmute, MaybeUninit}; let mut ret = MaybeUninit::uninit().assume_init(); - let presto = - transmute::<_, (usize, usize, usize, usize, usize, usize, usize, usize)>(call); - _xous_syscall_rust( - presto.0, presto.1, presto.2, presto.3, presto.4, presto.5, presto.6, presto.7, - &mut ret, - ) - ; + let presto = transmute::<_, (usize, usize, usize, usize, usize, usize, usize, usize)>(call); + _xous_syscall_rust( + presto.0, presto.1, presto.2, presto.3, presto.4, presto.5, presto.6, presto.7, &mut ret, + ); match ret { Result::Error(e) => Err(e), other => Ok(other),