Skip to content

Commit

Permalink
userlib: remove panics in kipc implementation (#1937)
Browse files Browse the repository at this point in the history
The kipc stubs tend to assert on the rc value received from the kernel.
This isn't ideal: we generally have to trust the kernel, and we can't do
anything useful if it misbehaves. So the assert is merely costing a
bunch of flash for a condition that can never happen. This removes these
asserts.

The system_restart kipc needs to indicate a "noreturn" send, but of
course we have no way of doing that right now. So, it panicked if the
send returned. This created _another_ can't-happen panic site in flash.
I've replaced it with an infinite loop that compiles to a two-byte
branch instruction, and in practice will never even be reached (but the
compiler doesn't know that).

This further reduces text and stack size in El Jefe.

---------

Co-authored-by: Eliza Weisman <[email protected]>
  • Loading branch information
cbiffle and hawkw authored Nov 26, 2024
1 parent 3408448 commit 740fd43
Showing 1 changed file with 22 additions and 16 deletions.
38 changes: 22 additions & 16 deletions sys/userlib/src/kipc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,16 @@
// file, You can obtain one at https://mozilla.org/MPL/2.0/.

//! Operations implemented by IPC with the kernel task.
//!
//! # On checking return values
//!
//! All the functions in this module send IPCs to the kernel directly. It's not
//! generally useful for us to check the return codes, except in cases where the
//! IPC is defined as able to fail. We have no choice but to trust the kernel,
//! since it controls everything.
//!
//! As a result, asserting on return codes and lengths when they can only be zero just
//! wastes flash space in the supervisor.
use core::num::NonZeroUsize;

Expand All @@ -15,14 +25,13 @@ pub fn read_task_status(task: usize) -> abi::TaskState {
// Coerce `task` to a known size (Rust doesn't assume that usize == u32)
let task = task as u32;
let mut response = [0; core::mem::size_of::<abi::TaskState>()];
let (rc, len) = sys_send(
let (_rc, len) = sys_send(
TaskId::KERNEL,
Kipcnum::ReadTaskStatus as u16,
task.as_bytes(),
&mut response,
&[],
);
assert_eq!(rc, 0);
ssmarshal::deserialize(&response[..len]).unwrap_lite().0
}

Expand Down Expand Up @@ -62,14 +71,13 @@ pub fn get_task_dump_region(
ssmarshal::serialize(&mut buf, &msg).unwrap_lite();

let mut response = [0; core::mem::size_of::<Option<abi::TaskDumpRegion>>()];
let (rc, len) = sys_send(
let (_rc, len) = sys_send(
TaskId::KERNEL,
Kipcnum::GetTaskDumpRegion as u16,
&buf,
&mut response,
&[],
);
assert_eq!(rc, 0);
ssmarshal::deserialize(&response[..len]).unwrap_lite().0
}

Expand All @@ -82,14 +90,13 @@ pub fn read_task_dump_region(
let mut buf = [0; core::mem::size_of::<(u32, abi::TaskDumpRegion)>()];
ssmarshal::serialize(&mut buf, &msg).unwrap_lite();

let (rc, len) = sys_send(
let (_rc, len) = sys_send(
TaskId::KERNEL,
Kipcnum::ReadTaskDumpRegion as u16,
&buf,
response,
&[],
);
assert_eq!(rc, 0);
len
}

Expand All @@ -98,45 +105,45 @@ pub fn restart_task(task: usize, start: bool) {
let msg = (task as u32, start);
let mut buf = [0; core::mem::size_of::<(u32, bool)>()];
ssmarshal::serialize(&mut buf, &msg).unwrap_lite();
let (rc, _len) = sys_send(
let (_rc, _len) = sys_send(
TaskId::KERNEL,
Kipcnum::RestartTask as u16,
&buf,
&mut [],
&[],
);
assert_eq!(rc, 0);
}

pub fn fault_task(task: usize) {
// Coerce `task` to a known size (Rust doesn't assume that usize == u32)
let task = task as u32;
let (rc, _len) = sys_send(
let (_rc, _len) = sys_send(
TaskId::KERNEL,
Kipcnum::FaultTask as u16,
task.as_bytes(),
&mut [],
&[],
);
assert_eq!(rc, 0);
}

pub fn system_restart() -> ! {
let _ = sys_send(TaskId::KERNEL, Kipcnum::Reset as u16, &[], &mut [], &[]);
panic!();
loop {
core::sync::atomic::compiler_fence(
core::sync::atomic::Ordering::SeqCst,
);
}
}

pub fn read_image_id() -> u64 {
let mut response = [0; core::mem::size_of::<u64>()];
let (rc, len) = sys_send(
let (_rc, len) = sys_send(
TaskId::KERNEL,
Kipcnum::ReadImageId as u16,
&[],
&mut response,
&[],
);
assert_eq!(rc, 0);
assert_eq!(len, 8); // we *really* expect this to be a u64
ssmarshal::deserialize(&response[..len]).unwrap_lite().0
}

Expand All @@ -147,12 +154,11 @@ pub fn software_irq(task: usize, mask: u32) {
let mut buf = [0; core::mem::size_of::<(u32, u32)>()];
ssmarshal::serialize(&mut buf, &msg).unwrap_lite();

let (rc, _len) = sys_send(
let (_rc, _len) = sys_send(
TaskId::KERNEL,
Kipcnum::SoftwareIrq as u16,
&buf,
&mut [],
&[],
);
assert_eq!(rc, 0);
}

0 comments on commit 740fd43

Please sign in to comment.