Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Convenient no-panic API for just waiting on notifications #1715

Merged
merged 2 commits into from
Apr 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 1 addition & 4 deletions drv/gimlet-seq-server/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -153,10 +153,7 @@ fn main() -> ! {
// irrelevant. But, `rustc` doesn't realize that this should
// never return, we'll stick it in a `loop` anyway so the main
// function can return `!`
//
// We don't care if this returns an error, because we're just
// doing it to die as politely as possible.
let _ = sys_recv_closed(&mut [], 0, TaskId::KERNEL);
sys_recv_notification(0);
}
}
}
Expand Down
14 changes: 3 additions & 11 deletions drv/lpc55-sha256/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@
//! (This restriction would be straightforward to lift if required.)

use core::num::Wrapping;
use userlib::{sys_irq_control, sys_recv_closed, TaskId};
use userlib::{sys_irq_control, sys_recv_notification};

// These constants describe intrinsic properties of the SHA256 algorithm and
// should not be changed.
Expand Down Expand Up @@ -201,11 +201,7 @@ impl<'a> Hasher<'a> {

// Wait for it!
sys_irq_control(self.notification_mask, true);
let _ = sys_recv_closed(
&mut [],
self.notification_mask,
TaskId::KERNEL,
);
sys_recv_notification(self.notification_mask);

// Turn it back off lest it spam us in the future.
self.engine.intenclr.write(|w| w.digest().set_bit());
Expand Down Expand Up @@ -235,11 +231,7 @@ impl<'a> Hasher<'a> {

// Wait for it!
sys_irq_control(self.notification_mask, true);
let _ = sys_recv_closed(
&mut [],
self.notification_mask,
TaskId::KERNEL,
);
sys_recv_notification(self.notification_mask);

// Turn it back off lest it spam us in the future.
self.engine.intenclr.write(|w| w.waiting().set_bit());
Expand Down
6 changes: 1 addition & 5 deletions drv/lpc55-spi-server/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -83,11 +83,7 @@ fn main() -> ! {
let mut rx_done = false;

loop {
if sys_recv_closed(&mut [], notifications::SPI_IRQ_MASK, TaskId::KERNEL)
.is_err()
{
panic!()
}
sys_recv_notification(notifications::SPI_IRQ_MASK);

ringbuf_entry!(Trace::Irq);

Expand Down
9 changes: 2 additions & 7 deletions drv/lpc55-sprot-server/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ use drv_sprot_api::{
use lpc55_pac as device;
use ringbuf::{ringbuf, ringbuf_entry};
use userlib::{
sys_irq_control, sys_recv_closed, task_slot, TaskId, UnwrapLite,
sys_irq_control, sys_recv_notification, task_slot, TaskId, UnwrapLite,
};

mod handler;
Expand Down Expand Up @@ -216,12 +216,7 @@ impl Io {
loop {
sys_irq_control(notifications::SPI_IRQ_MASK, true);

sys_recv_closed(
&mut [],
notifications::SPI_IRQ_MASK,
TaskId::KERNEL,
)
.unwrap_lite();
sys_recv_notification(notifications::SPI_IRQ_MASK);

// Is CSn asserted by the SP?
let intstat = self.spi.intstat();
Expand Down
11 changes: 2 additions & 9 deletions drv/lpc55-update-server/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -639,12 +639,7 @@ fn indirect_flash_read_words(

flash.enable_interrupt_sources();
sys_irq_control(notifications::FLASH_IRQ_MASK, true);
// RECV from the kernel cannot produce an error, so ignore it.
let _ = sys_recv_closed(
&mut [],
notifications::FLASH_IRQ_MASK,
TaskId::KERNEL,
);
sys_recv_notification(notifications::FLASH_IRQ_MASK);
flash.disable_interrupt_sources();
}
}
Expand Down Expand Up @@ -773,9 +768,7 @@ fn do_block_write(

fn wait_for_flash_interrupt() {
sys_irq_control(notifications::FLASH_IRQ_MASK, true);
// RECV from the kernel cannot produce an error, so ignore it.
let _ =
sys_recv_closed(&mut [], notifications::FLASH_IRQ_MASK, TaskId::KERNEL);
sys_recv_notification(notifications::FLASH_IRQ_MASK);
}

fn same_image(which: UpdateTarget) -> bool {
Expand Down
2 changes: 1 addition & 1 deletion drv/psc-seq-server/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,6 @@ fn main() -> ! {
// We have nothing else to do, so sleep forever via waiting for a message
// from the kernel that won't arrive.
loop {
_ = sys_recv_closed(&mut [], 0, TaskId::KERNEL);
sys_recv_notification(0);
}
}
6 changes: 1 addition & 5 deletions drv/stm32h7-eth/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -435,11 +435,7 @@ impl Ethernet {
// has disabled itself before proceeding.
loop {
userlib::sys_irq_control(self.mdio_timer_irq_mask, true);
let _ = userlib::sys_recv_closed(
&mut [],
self.mdio_timer_irq_mask,
userlib::TaskId::KERNEL,
);
userlib::sys_recv_notification(self.mdio_timer_irq_mask);
if !self.mdio_timer.cr1.read().cen().bit() {
break;
}
Expand Down
3 changes: 1 addition & 2 deletions drv/stm32h7-hash/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -311,8 +311,7 @@ impl Hash {
hl::sleep_for(1);
}
}
let _rm = sys_recv_closed(&mut [], self.interrupt, TaskId::KERNEL)
.unwrap();
sys_recv_notification(self.interrupt);
if self.reg.sr.read().dcis().bit() {
break;
}
Expand Down
16 changes: 5 additions & 11 deletions drv/stm32h7-qspi/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ use stm32h7::stm32h743 as device;
use stm32h7::stm32h753 as device;

use drv_qspi_api::Command;
use userlib::{sys_irq_control, sys_recv_closed, TaskId};
use userlib::{sys_irq_control, sys_recv_notification};
use zerocopy::AsBytes;

const FIFO_SIZE: usize = 32;
Expand Down Expand Up @@ -198,9 +198,7 @@ impl Qspi {
// Unmask our interrupt.
sys_irq_control(self.interrupt, true);
// And wait for it to arrive.
let _rm =
sys_recv_closed(&mut [], self.interrupt, TaskId::KERNEL)
.unwrap();
sys_recv_notification(self.interrupt);
if self.reg.sr.read().ftf().bit() {
break;
}
Expand All @@ -218,8 +216,7 @@ impl Qspi {
// Unmask our interrupt.
sys_irq_control(self.interrupt, true);
// And wait for it to arrive.
let _rm = sys_recv_closed(&mut [], self.interrupt, TaskId::KERNEL)
.unwrap();
sys_recv_notification(self.interrupt);
}
self.reg.cr.modify(|_, w| w.tcie().clear_bit());
}
Expand Down Expand Up @@ -284,9 +281,7 @@ impl Qspi {
// Unmask our interrupt.
sys_irq_control(self.interrupt, true);
// And wait for it to arrive.
let _rm =
sys_recv_closed(&mut [], self.interrupt, TaskId::KERNEL)
.unwrap();
sys_recv_notification(self.interrupt);

// Try the check again. We may retry the check on spurious
// wakeups, but, spurious wakeups are expected to be pretty
Expand Down Expand Up @@ -321,8 +316,7 @@ impl Qspi {
// Unmask our interrupt.
sys_irq_control(self.interrupt, true);
// And wait for it to arrive.
let _rm = sys_recv_closed(&mut [], self.interrupt, TaskId::KERNEL)
.unwrap();
sys_recv_notification(self.interrupt);
}

// Clean up by disabling our interrupt sources.
Expand Down
6 changes: 2 additions & 4 deletions drv/stm32h7-spi-server-core/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -537,10 +537,8 @@ impl SpiServerCore {
// Allow the controller interrupt to post to our
// notification set.
sys_irq_control(self.irq_mask, true);
// Wait for our notification set to get, well, set. We ignore
// the result of this because an error would mean the kernel
// violated the ABI, which we can't usefully respond to.
let _ = sys_recv_closed(&mut [], self.irq_mask, TaskId::KERNEL);
// Wait for our notification set to get, well, set.
sys_recv_notification(self.irq_mask);
}
}

Expand Down
7 changes: 1 addition & 6 deletions drv/stm32h7-update-server/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -264,12 +264,7 @@ impl<'a> ServerImpl<'a> {

// Wait for EOP notification via interrupt.
loop {
sys_recv_closed(
&mut [],
notifications::FLASH_IRQ_MASK,
TaskId::KERNEL,
)
.unwrap_lite();
sys_recv_notification(notifications::FLASH_IRQ_MASK);
if self.flash.bank2().sr.read().eop().bit() {
break;
} else {
Expand Down
16 changes: 8 additions & 8 deletions drv/stm32xx-i2c-server/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -343,17 +343,17 @@ fn main() -> ! {
wfi: |notification, timeout| {
const TIMER_NOTIFICATION: u32 = 1 << 31;

let dead = sys_get_timer().now.checked_add(timeout.0).unwrap_lite();
// If the driver passes in a timeout that is large enough that it
// would overflow the kernel's 64-bit timestamp space... well, we'll
// do the best we can without compiling in an unlikely panic.
let dead = sys_get_timer().now.saturating_add(timeout.0);

sys_set_timer(Some(dead), TIMER_NOTIFICATION);

let m = sys_recv_closed(
&mut [],
notification | TIMER_NOTIFICATION,
TaskId::KERNEL,
)
.unwrap_lite();
let notification =
sys_recv_notification(notification | TIMER_NOTIFICATION);

if m.operation == TIMER_NOTIFICATION {
if notification == TIMER_NOTIFICATION {
I2cControlResult::TimedOut
} else {
sys_set_timer(None, TIMER_NOTIFICATION);
Expand Down
63 changes: 51 additions & 12 deletions sys/kern/src/syscalls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -191,12 +191,35 @@ fn send(tasks: &mut [Task], caller: usize) -> Result<NextTask, UserError> {
///
/// `caller` is a valid task index (i.e. not directly from user code).
///
/// # Returns
///
/// If the operation found a sender and delivered a message,
/// `Ok(NextTask::Same)` to drop us right back into the caller.
///
/// If the operation did not find a sender but was otherwise valid (i.e. needs
/// to block the sender), `Ok(something_else)` to context switch away from the
/// sender.
///
/// This may also return `Ok(something_else)` to context switch to the
/// supervisor even if a message was successfully delivered, but only if at
/// least one blocked sender with invalid configuration was found and faulted
/// along the way.
///
/// In terms of errors,
///
/// `Err(UserError::Recoverable(..))` is only used to return Dead Codes, and
/// only in closed receive specifically.
///
/// All other errors are `Err(UserError::Unrecoverable(_))` that result in a
/// fault delivered to the caller. This indicates that the location where the
/// caller requested to receive a delivered message isn't accessible or valid.
///
/// # Panics
///
/// If `caller` is out of range for `tasks`.
fn recv(tasks: &mut [Task], caller: usize) -> Result<NextTask, UserError> {
// We allow tasks to atomically replace their notification mask at each
// receive. We simultaneously find out if there are notifications pending.
// `take_notifications` interprets the new notification mask and finds out
// if notifications are pending.
if let Some(firing) = tasks[caller].take_notifications() {
// Pending! Deliver an artificial message from the kernel.
tasks[caller].save_mut().set_recv_result(
Expand Down Expand Up @@ -226,7 +249,12 @@ fn recv(tasks: &mut [Task], caller: usize) -> Result<NextTask, UserError> {
// outcomes here.

// First possibility: that task you're asking about is DEAD.
//
// N.B. this is actually the only point in the RECV implementation where
// the sender may receive an error code (as opposed to being faulted or
// just blocking waiting for a valid sender).
let sender_idx = task::check_task_id_against_table(tasks, sender_id)?;

// Second possibility: task has a message for us.
if tasks[sender_idx].state().is_sending_to(caller_id) {
// Oh hello sender!
Expand All @@ -238,15 +266,24 @@ fn recv(tasks: &mut [Task], caller: usize) -> Result<NextTask, UserError> {
}
Err(interact) => {
// Delivery failed because of fault events in one or both
// tasks. We need to apply the fault status, and then if we
// didn't have to murder the caller, we'll retry receiving a
// message.
// tasks. `apply_to_src` extracts the src (sender) side and
// applies it directly to the task; if there was a problem
// on the dst (recv'r, us) side we ? it. It's a FaultInfo,
// so if dst screwed up the caller task will be faulted.
//
// If there was no problem, the caller will be informed that
// the task has died, and will generally opt to retry the
// recv as an open recv.
//
// The wake hint here may wake the supervisor before the
// caller regains control.
let wake_hint = interact.apply_to_src(tasks, sender_idx)?;
// No fault in the caller at least, carry on.
next_task = next_task.combine(wake_hint);
}
}
}
// Third possibility: we need to block; fall through below.
// Third possibility: we need to block; fall through below.
} else {
// Open Receive

Expand All @@ -269,20 +306,22 @@ fn recv(tasks: &mut [Task], caller: usize) -> Result<NextTask, UserError> {
}) {
// Oh hello sender!
match deliver(tasks, sender, caller) {
Ok(_) => {
Ok(()) => {
// Delivery succeeded! Sender is now blocked in reply. Go ahead
// and let the caller resume.
return Ok(next_task);
}
Err(interact) => {
// Delivery failed because of fault events in one or both
// tasks. We need to apply the fault status, and then if we
// didn't have to murder the caller, we'll retry receiving a
// message.
// tasks. Because we're transferring a message from the
// sender to the recv'r (us, the caller) we use apply_to_src
// to potentially fault the sender, and then apply the dst
// side to the caller using ?.
let wake_hint = interact.apply_to_src(tasks, sender)?;
// No fault in the caller, at least. This may wake the
// supervisor.
next_task = next_task.combine(wake_hint);
// Okay, if we didn't just return, retry the search from a new
// position.
// Retry the search from our new position.
last = sender;
}
}
Expand Down
6 changes: 3 additions & 3 deletions sys/userlib/src/hl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
//! This is intended to provide a more ergonomic interface than the raw
//! syscalls.

use abi::TaskId;
use abi::{Generation, TaskId};
use core::marker::PhantomData;
use unwrap_lite::UnwrapLite;
use zerocopy::{AsBytes, FromBytes, LayoutVerified};
Expand Down Expand Up @@ -147,8 +147,8 @@ where
O: FromPrimitive,
E: Into<u32>,
{
let rm =
sys_recv(buffer, mask, source).map_err(|_| ClosedRecvError::Dead)?;
let rm = sys_recv(buffer, mask, source)
.map_err(|code| ClosedRecvError::Dead(Generation::from(code as u8)))?;
let sender = rm.sender;
if rm.sender == TaskId::KERNEL {
notify(state, rm.operation);
Expand Down
Loading
Loading