From 1706efc792692e0da7988a66bd361af1ca704b3e Mon Sep 17 00:00:00 2001 From: Chris Frantz Date: Sat, 6 Jun 2026 13:02:36 -0700 Subject: [PATCH 01/12] util: IPC abstraction Signed-off-by: Chris Frantz --- util/ipc/BUILD.bazel | 26 ++++++++++ util/ipc/host.rs | 121 +++++++++++++++++++++++++++++++++++++++++++ util/ipc/lib.rs | 53 +++++++++++++++++++ util/ipc/target.rs | 40 ++++++++++++++ 4 files changed, 240 insertions(+) create mode 100644 util/ipc/BUILD.bazel create mode 100644 util/ipc/host.rs create mode 100644 util/ipc/lib.rs create mode 100644 util/ipc/target.rs diff --git a/util/ipc/BUILD.bazel b/util/ipc/BUILD.bazel new file mode 100644 index 000000000..c748e9212 --- /dev/null +++ b/util/ipc/BUILD.bazel @@ -0,0 +1,26 @@ +# Licensed under the Apache-2.0 license +# SPDX-License-Identifier: Apache-2.0 + +load("@rules_rust//rust:defs.bzl", "rust_library") + +rust_library( + name = "ipc", + srcs = [ + "host.rs", + "lib.rs", + "target.rs", + ], + crate_name = "util_ipc", + edition = "2024", + visibility = ["//visibility:public"], + deps = [ + "@pigweed//pw_status/rust:pw_status", + ] + select({ + "@platforms//os:none": [ + "@pigweed//pw_kernel/userspace", + ], + "//conditions:default": [ + "@pigweed//pw_time/rust:pw_time", + ], + }), +) diff --git a/util/ipc/host.rs b/util/ipc/host.rs new file mode 100644 index 000000000..9abb9caf1 --- /dev/null +++ b/util/ipc/host.rs @@ -0,0 +1,121 @@ +// Licensed under the Apache-2.0 license +// SPDX-License-Identifier: Apache-2.0 + +use super::{IpcChannel, IpcHandle}; + +pub trait AsSyscallBuffer { + fn as_raw(&self) -> (*const u8, usize); + fn as_raw_mut(&mut self) -> (*mut u8, usize); + fn total_size(&self) -> usize; +} + +// Converts a simple u8 slice. +impl AsSyscallBuffer for [u8] { + fn as_raw(&self) -> (*const u8, usize) { + (self.as_ptr(), self.len()) + } + fn as_raw_mut(&mut self) -> (*mut u8, usize) { + (self.as_mut_ptr(), self.len()) + } + fn total_size(&self) -> usize { + self.len() + } +} + +// Converts a simple u8 array. +impl AsSyscallBuffer for [u8; N] { + fn as_raw(&self) -> (*const u8, usize) { + (self.as_ptr(), self.len()) + } + fn as_raw_mut(&mut self) -> (*mut u8, usize) { + (self.as_mut_ptr(), self.len()) + } + fn total_size(&self) -> usize { + self.len() + } +} + +// Converts a slice of u8 slices. +impl AsSyscallBuffer for [&[u8]] { + fn as_raw(&self) -> (*const u8, usize) { + (self.as_ptr().cast::(), self.len().wrapping_neg()) + } + fn as_raw_mut(&mut self) -> (*mut u8, usize) { + (self.as_mut_ptr().cast::(), self.len().wrapping_neg()) + } + fn total_size(&self) -> usize { + self.iter().fold(0, |total, item| total + item.len()) + } +} + +impl AsSyscallBuffer for [&mut [u8]] { + fn as_raw(&self) -> (*const u8, usize) { + (self.as_ptr().cast::(), self.len().wrapping_neg()) + } + fn as_raw_mut(&mut self) -> (*mut u8, usize) { + (self.as_mut_ptr().cast::(), self.len().wrapping_neg()) + } + fn total_size(&self) -> usize { + self.iter().fold(0, |total, item| total + item.len()) + } +} + +// Converts an array of u8 slices. +impl AsSyscallBuffer for [&[u8]; N] { + fn as_raw(&self) -> (*const u8, usize) { + (self.as_ptr().cast::(), self.len().wrapping_neg()) + } + fn as_raw_mut(&mut self) -> (*mut u8, usize) { + (self.as_mut_ptr().cast::(), self.len().wrapping_neg()) + } + fn total_size(&self) -> usize { + self.iter().fold(0, |total, item| total + item.len()) + } +} + +impl AsSyscallBuffer for [&mut [u8]; N] { + fn as_raw(&self) -> (*const u8, usize) { + (self.as_ptr().cast::(), self.len().wrapping_neg()) + } + fn as_raw_mut(&mut self) -> (*mut u8, usize) { + (self.as_mut_ptr().cast::(), self.len().wrapping_neg()) + } + fn total_size(&self) -> usize { + self.iter().fold(0, |total, item| total + item.len()) + } +} + +pub type Instant = pw_time::Instant; + +impl IpcChannel for IpcHandle { + fn transact( + &self, + _send_data: &BufSend, + _recv_data: &mut BufRecv, + _deadline: Instant, + ) -> pw_status::Result + where + BufSend: AsSyscallBuffer + ?Sized, + BufRecv: AsSyscallBuffer + ?Sized, + { + panic!("IpcHandle cannot be used on host"); + } + + fn read(&self, _offset: usize, _buffer: &mut Buf) -> pw_status::Result + where + Buf: AsSyscallBuffer + ?Sized, + { + panic!("IpcHandle cannot be used on host"); + } + + fn respond(&self, _buffer: &Buf) -> pw_status::Result<()> + where + Buf: AsSyscallBuffer + ?Sized, + { + panic!("IpcHandle cannot be used on host"); + } + + fn set_peer_user_signal(&self, _set: bool) -> pw_status::Result<()> { + panic!("IpcHandle cannot be used on host"); + } +} diff --git a/util/ipc/lib.rs b/util/ipc/lib.rs new file mode 100644 index 000000000..7861452b0 --- /dev/null +++ b/util/ipc/lib.rs @@ -0,0 +1,53 @@ +// Licensed under the Apache-2.0 license +// SPDX-License-Identifier: Apache-2.0 + +#![no_std] + +use pw_status::Result; + +/// Trait wrapping basic IPC operations on a channel. +pub trait IpcChannel { + fn transact( + &self, + send_data: &BufSend, + recv_data: &mut BufRecv, + deadline: Instant, + ) -> Result + where + BufSend: AsSyscallBuffer + ?Sized, + BufRecv: AsSyscallBuffer + ?Sized; + + fn read(&self, offset: usize, buffer: &mut Buf) -> Result + where + Buf: AsSyscallBuffer + ?Sized; + + fn respond(&self, buffer: &Buf) -> Result<()> + where + Buf: AsSyscallBuffer + ?Sized; + + /// Set (set=true) or clear (set=false) Signals::USER on the paired peer. + fn set_peer_user_signal(&self, set: bool) -> Result<()>; +} + +/// Transparent wrapper around a raw IPC handle. +#[repr(transparent)] +#[derive(Debug, Copy, Clone, PartialEq, Eq)] +pub struct IpcHandle { + pub handle: u32, +} + +impl IpcHandle { + pub const fn new(handle: u32) -> Self { + Self { handle } + } +} + +#[cfg(target_os = "none")] +mod target; +#[cfg(target_os = "none")] +pub use target::{AsSyscallBuffer, Instant}; + +#[cfg(not(target_os = "none"))] +mod host; +#[cfg(not(target_os = "none"))] +pub use host::{AsSyscallBuffer, Instant}; diff --git a/util/ipc/target.rs b/util/ipc/target.rs new file mode 100644 index 000000000..d278d073d --- /dev/null +++ b/util/ipc/target.rs @@ -0,0 +1,40 @@ +// Licensed under the Apache-2.0 license +// SPDX-License-Identifier: Apache-2.0 + +use super::{IpcChannel, IpcHandle}; + +pub use userspace::buffer::AsSyscallBuffer; +pub use userspace::time::Instant; + +impl IpcChannel for IpcHandle { + fn transact( + &self, + send_data: &BufSend, + recv_data: &mut BufRecv, + deadline: Instant, + ) -> pw_status::Result + where + BufSend: AsSyscallBuffer + ?Sized, + BufRecv: AsSyscallBuffer + ?Sized, + { + userspace::syscall::channel_transact(self.handle, send_data, recv_data, deadline) + } + + fn read(&self, offset: usize, buffer: &mut Buf) -> pw_status::Result + where + Buf: AsSyscallBuffer + ?Sized, + { + userspace::syscall::channel_read(self.handle, offset, buffer) + } + + fn respond(&self, buffer: &Buf) -> pw_status::Result<()> + where + Buf: AsSyscallBuffer + ?Sized, + { + userspace::syscall::channel_respond(self.handle, buffer) + } + + fn set_peer_user_signal(&self, set: bool) -> pw_status::Result<()> { + userspace::syscall::object_set_peer_user_signal(self.handle, set) + } +} From 34e684cdf986c1566e18c97214000a6c96cf7cee Mon Sep 17 00:00:00 2001 From: Anthony Rocha Date: Mon, 21 Sep 2026 19:36:05 -0700 Subject: [PATCH 02/12] util/ipc: extend IpcChannel with async transact/cancel Add async_transact_start (unsafe), async_transact_complete, and async_cancel to IpcChannel, mirroring the kernel's channel_async_transact/_complete/_cancel syscalls. Readiness is polled via object_wait/wait_group_add on Signals::READABLE rather than a Future, matching the kernel's poll-via-signal model. --- util/ipc/host.rs | 20 ++++++++++++++++++++ util/ipc/lib.rs | 23 +++++++++++++++++++++++ util/ipc/target.rs | 32 ++++++++++++++++++++++++++++++++ 3 files changed, 75 insertions(+) diff --git a/util/ipc/host.rs b/util/ipc/host.rs index 9abb9caf1..71c0fee69 100644 --- a/util/ipc/host.rs +++ b/util/ipc/host.rs @@ -101,6 +101,26 @@ impl IpcChannel for IpcHandle { panic!("IpcHandle cannot be used on host"); } + unsafe fn async_transact_start( + &self, + _send_data: &BufSend, + _recv_data: &mut BufRecv, + ) -> pw_status::Result<()> + where + BufSend: AsSyscallBuffer + ?Sized, + BufRecv: AsSyscallBuffer + ?Sized, + { + panic!("IpcHandle cannot be used on host"); + } + + fn async_transact_complete(&self) -> pw_status::Result { + panic!("IpcHandle cannot be used on host"); + } + + fn async_cancel(&self) -> pw_status::Result<()> { + panic!("IpcHandle cannot be used on host"); + } + fn read(&self, _offset: usize, _buffer: &mut Buf) -> pw_status::Result where Buf: AsSyscallBuffer + ?Sized, diff --git a/util/ipc/lib.rs b/util/ipc/lib.rs index 7861452b0..5d555f40a 100644 --- a/util/ipc/lib.rs +++ b/util/ipc/lib.rs @@ -17,6 +17,29 @@ pub trait IpcChannel { BufSend: AsSyscallBuffer + ?Sized, BufRecv: AsSyscallBuffer + ?Sized; + /// Starts a transaction and returns immediately; poll readiness via + /// `object_wait`/`wait_group_add` on this channel's handle + /// (`Signals::READABLE`), then call `async_transact_complete` or + /// `async_cancel`. Fails with `Error::Unavailable` if a transaction + /// (blocking or async) is already pending on this channel. + /// + /// # Safety + /// `send_data`/`recv_data` are borrowed by the kernel until the + /// transaction is completed or cancelled — they must stay valid and + /// unmutated until then. + unsafe fn async_transact_start( + &self, + send_data: &BufSend, + recv_data: &mut BufRecv, + ) -> Result<()> + where + BufSend: AsSyscallBuffer + ?Sized, + BufRecv: AsSyscallBuffer + ?Sized; + + fn async_transact_complete(&self) -> Result; + + fn async_cancel(&self) -> Result<()>; + fn read(&self, offset: usize, buffer: &mut Buf) -> Result where Buf: AsSyscallBuffer + ?Sized; diff --git a/util/ipc/target.rs b/util/ipc/target.rs index d278d073d..f48714244 100644 --- a/util/ipc/target.rs +++ b/util/ipc/target.rs @@ -20,6 +20,38 @@ impl IpcChannel for IpcHandle { userspace::syscall::channel_transact(self.handle, send_data, recv_data, deadline) } + unsafe fn async_transact_start( + &self, + send_data: &BufSend, + recv_data: &mut BufRecv, + ) -> pw_status::Result<()> + where + BufSend: AsSyscallBuffer + ?Sized, + BufRecv: AsSyscallBuffer + ?Sized, + { + let (send_ptr, send_len) = send_data.as_raw(); + let (recv_ptr, recv_len) = recv_data.as_raw_mut(); + // Safety: caller upholds the buffer-lifetime contract per this fn's doc. + // nosemgrep + unsafe { + userspace::syscall::channel_async_transact( + self.handle, + send_ptr, + send_len, + recv_ptr, + recv_len, + ) + } + } + + fn async_transact_complete(&self) -> pw_status::Result { + userspace::syscall::channel_async_transact_complete(self.handle) + } + + fn async_cancel(&self) -> pw_status::Result<()> { + userspace::syscall::channel_async_cancel(self.handle) + } + fn read(&self, offset: usize, buffer: &mut Buf) -> pw_status::Result where Buf: AsSyscallBuffer + ?Sized, From c1516016b7814c53f2831e57c52c4ea79b0216c8 Mon Sep 17 00:00:00 2001 From: Anthony Rocha Date: Tue, 22 Sep 2026 13:05:50 -0700 Subject: [PATCH 03/12] util/ipc: address review feedback, split roles, add AsyncTransaction Splits IpcChannel into IpcInitiator/IpcHandler so a type only implements the operations its channel role can serve. Drops the host.rs stub (crate is now target-only, matching services/i2c/client-ipc and services/mctp/client-ipc) instead of shipping dead panic methods and a drifting copy of buffer.rs. Adds AsyncTransaction, a safe layer over the unsafe async trio that tracks the one-transaction-per-channel invariant locally and cancels on Drop, converging with the design in #482 instead of shipping both. Co-Authored-By: Claude Sonnet 5 --- util/ipc/BUILD.bazel | 17 ++-- util/ipc/async_transaction.rs | 162 ++++++++++++++++++++++++++++++++++ util/ipc/host.rs | 141 ----------------------------- util/ipc/lib.rs | 40 +++++---- util/ipc/target.rs | 21 +++-- 5 files changed, 210 insertions(+), 171 deletions(-) create mode 100644 util/ipc/async_transaction.rs delete mode 100644 util/ipc/host.rs diff --git a/util/ipc/BUILD.bazel b/util/ipc/BUILD.bazel index c748e9212..0c1536e91 100644 --- a/util/ipc/BUILD.bazel +++ b/util/ipc/BUILD.bazel @@ -6,21 +6,20 @@ load("@rules_rust//rust:defs.bzl", "rust_library") rust_library( name = "ipc", srcs = [ - "host.rs", + "async_transaction.rs", "lib.rs", "target.rs", ], crate_name = "util_ipc", edition = "2024", + tags = ["kernel"], + target_compatible_with = select({ + "@platforms//os:none": [], + "//conditions:default": ["@platforms//:incompatible"], + }), visibility = ["//visibility:public"], deps = [ "@pigweed//pw_status/rust:pw_status", - ] + select({ - "@platforms//os:none": [ - "@pigweed//pw_kernel/userspace", - ], - "//conditions:default": [ - "@pigweed//pw_time/rust:pw_time", - ], - }), + "@pigweed//pw_kernel/userspace", + ], ) diff --git a/util/ipc/async_transaction.rs b/util/ipc/async_transaction.rs new file mode 100644 index 000000000..f9202054f --- /dev/null +++ b/util/ipc/async_transaction.rs @@ -0,0 +1,162 @@ +// Licensed under the Apache-2.0 license +// SPDX-License-Identifier: Apache-2.0 + +//! Safe async IPC transaction wrapper, layered on `IpcInitiator`. +//! +//! `AsyncTransaction` wraps the unsafe `async_transact_start`/`_complete`/ +//! `_cancel` trio into a state machine (Idle / Pending) that enforces the +//! kernel's one-transaction-per-channel rule locally and keeps the +//! raw-pointer safety contract in one place. +//! +//! Buffers are `&'static` because the kernel holds raw pointers into them +//! for the duration of the transaction. If `AsyncTransaction` owned the buffers +//! inline and the struct moved after `start()`, those pointers would +//! dangle. Static borrows make soundness independent of moves and +//! `mem::forget`. +//! +//! Every exit from Pending returns both buffers to the caller so they can +//! be reused for the next transaction (the standard embedded-DMA ownership +//! pattern). + +use super::IpcInitiator; +use pw_status::{Error, Result}; + +/// Buffers lent to the kernel for the duration of one async transaction. +#[derive(Debug)] +pub struct Buffers { + pub send: &'static [u8], + pub recv: &'static mut [u8], +} + +/// One-at-a-time async IPC transaction on an `IpcInitiator`. +/// +/// Transitions: Idle -> `start()` -> Pending -> `try_recv()`/`cancel()` -> Idle. +pub struct AsyncTransaction { + handle: H, + inflight: Option, +} + +impl AsyncTransaction { + /// Wrap an initiator handle with no outstanding async transaction. + pub fn new(handle: H) -> Self { + Self { + handle, + inflight: None, + } + } + + /// The wrapped initiator, e.g. to register it with a WaitGroup. + pub fn handle(&self) -> &H { + &self.handle + } + + /// Whether a transaction is in flight. + pub fn is_pending(&self) -> bool { + self.inflight.is_some() + } + + /// Start an async transaction. + /// + /// `send` is the request payload the server will read. `recv` is the + /// buffer the kernel writes the server's response into. Both must be + /// `'static` because the kernel holds raw pointers into them until + /// `try_recv` or `cancel` completes the transaction. + /// + /// On success the buffers are held until `try_recv` or `cancel` + /// returns them. On failure (already pending, or a kernel error) both + /// buffers come back in the `Err` so nothing is lost. + pub fn start( + &mut self, + send: &'static [u8], + recv: &'static mut [u8], + ) -> core::result::Result<(), StartError> { + if self.inflight.is_some() { + return Err(StartError { + error: Error::FailedPrecondition, + send, + recv, + }); + } + + // Safety: send/recv are 'static, so the kernel's raw pointers + // stay valid regardless of what happens to `self`, and they are + // not read, written, or dropped again until try_recv/cancel. + let result = unsafe { self.handle.async_transact_start(send, recv) }; + + match result { + Ok(()) => { + self.inflight = Some(Buffers { send, recv }); + Ok(()) + } + Err(error) => Err(StartError { error, send, recv }), + } + } + + /// Try to complete a pending transaction. + /// + /// Returns `Ok(Completion { len, send, recv })` when the server has + /// responded. `recv[..len]` holds the response payload; the full + /// buffer is returned so it can be reused. + /// + /// Returns `Err(Error::Unavailable)` if READABLE is not set (server + /// has not responded yet); the buffers stay held and another + /// `try_recv` is expected after the next READABLE signal. + /// + /// Returns `Err(Error::FailedPrecondition)` if no transaction is + /// pending. Any other kernel error leaves the transaction pending; + /// use `cancel()` to reclaim the buffers. + pub fn try_recv(&mut self) -> Result { + if self.inflight.is_none() { + return Err(Error::FailedPrecondition); + } + + let len = self.handle.async_transact_complete()?; + let Buffers { send, recv } = self.inflight.take().unwrap(); + Ok(Completion { len, send, recv }) + } + + /// Cancel a pending transaction and reclaim the buffers. + /// + /// If the server has already responded, the response is silently + /// discarded. + /// + /// Returns `Err(Error::FailedPrecondition)` if no transaction is + /// pending. Propagates unexpected kernel errors with the buffers + /// still held (use `cancel()` again or drop the struct). + pub fn cancel(&mut self) -> Result { + if self.inflight.is_none() { + return Err(Error::FailedPrecondition); + } + + self.handle.async_cancel()?; + Ok(self.inflight.take().unwrap()) + } +} + +impl Drop for AsyncTransaction { + fn drop(&mut self) { + if self.inflight.is_some() { + let _ = self.handle.async_cancel(); + } + } +} + +/// Successful completion of an async transaction. +#[derive(Debug)] +pub struct Completion { + /// Number of response bytes written into `recv`. + pub len: usize, + /// The send buffer, returned for reuse. + pub send: &'static [u8], + /// The receive buffer, returned for reuse. `recv[..len]` holds the + /// response payload. + pub recv: &'static mut [u8], +} + +/// Error from `start()`, carrying the buffers back so they are not lost. +#[derive(Debug)] +pub struct StartError { + pub error: Error, + pub send: &'static [u8], + pub recv: &'static mut [u8], +} diff --git a/util/ipc/host.rs b/util/ipc/host.rs deleted file mode 100644 index 71c0fee69..000000000 --- a/util/ipc/host.rs +++ /dev/null @@ -1,141 +0,0 @@ -// Licensed under the Apache-2.0 license -// SPDX-License-Identifier: Apache-2.0 - -use super::{IpcChannel, IpcHandle}; - -pub trait AsSyscallBuffer { - fn as_raw(&self) -> (*const u8, usize); - fn as_raw_mut(&mut self) -> (*mut u8, usize); - fn total_size(&self) -> usize; -} - -// Converts a simple u8 slice. -impl AsSyscallBuffer for [u8] { - fn as_raw(&self) -> (*const u8, usize) { - (self.as_ptr(), self.len()) - } - fn as_raw_mut(&mut self) -> (*mut u8, usize) { - (self.as_mut_ptr(), self.len()) - } - fn total_size(&self) -> usize { - self.len() - } -} - -// Converts a simple u8 array. -impl AsSyscallBuffer for [u8; N] { - fn as_raw(&self) -> (*const u8, usize) { - (self.as_ptr(), self.len()) - } - fn as_raw_mut(&mut self) -> (*mut u8, usize) { - (self.as_mut_ptr(), self.len()) - } - fn total_size(&self) -> usize { - self.len() - } -} - -// Converts a slice of u8 slices. -impl AsSyscallBuffer for [&[u8]] { - fn as_raw(&self) -> (*const u8, usize) { - (self.as_ptr().cast::(), self.len().wrapping_neg()) - } - fn as_raw_mut(&mut self) -> (*mut u8, usize) { - (self.as_mut_ptr().cast::(), self.len().wrapping_neg()) - } - fn total_size(&self) -> usize { - self.iter().fold(0, |total, item| total + item.len()) - } -} - -impl AsSyscallBuffer for [&mut [u8]] { - fn as_raw(&self) -> (*const u8, usize) { - (self.as_ptr().cast::(), self.len().wrapping_neg()) - } - fn as_raw_mut(&mut self) -> (*mut u8, usize) { - (self.as_mut_ptr().cast::(), self.len().wrapping_neg()) - } - fn total_size(&self) -> usize { - self.iter().fold(0, |total, item| total + item.len()) - } -} - -// Converts an array of u8 slices. -impl AsSyscallBuffer for [&[u8]; N] { - fn as_raw(&self) -> (*const u8, usize) { - (self.as_ptr().cast::(), self.len().wrapping_neg()) - } - fn as_raw_mut(&mut self) -> (*mut u8, usize) { - (self.as_mut_ptr().cast::(), self.len().wrapping_neg()) - } - fn total_size(&self) -> usize { - self.iter().fold(0, |total, item| total + item.len()) - } -} - -impl AsSyscallBuffer for [&mut [u8]; N] { - fn as_raw(&self) -> (*const u8, usize) { - (self.as_ptr().cast::(), self.len().wrapping_neg()) - } - fn as_raw_mut(&mut self) -> (*mut u8, usize) { - (self.as_mut_ptr().cast::(), self.len().wrapping_neg()) - } - fn total_size(&self) -> usize { - self.iter().fold(0, |total, item| total + item.len()) - } -} - -pub type Instant = pw_time::Instant; - -impl IpcChannel for IpcHandle { - fn transact( - &self, - _send_data: &BufSend, - _recv_data: &mut BufRecv, - _deadline: Instant, - ) -> pw_status::Result - where - BufSend: AsSyscallBuffer + ?Sized, - BufRecv: AsSyscallBuffer + ?Sized, - { - panic!("IpcHandle cannot be used on host"); - } - - unsafe fn async_transact_start( - &self, - _send_data: &BufSend, - _recv_data: &mut BufRecv, - ) -> pw_status::Result<()> - where - BufSend: AsSyscallBuffer + ?Sized, - BufRecv: AsSyscallBuffer + ?Sized, - { - panic!("IpcHandle cannot be used on host"); - } - - fn async_transact_complete(&self) -> pw_status::Result { - panic!("IpcHandle cannot be used on host"); - } - - fn async_cancel(&self) -> pw_status::Result<()> { - panic!("IpcHandle cannot be used on host"); - } - - fn read(&self, _offset: usize, _buffer: &mut Buf) -> pw_status::Result - where - Buf: AsSyscallBuffer + ?Sized, - { - panic!("IpcHandle cannot be used on host"); - } - - fn respond(&self, _buffer: &Buf) -> pw_status::Result<()> - where - Buf: AsSyscallBuffer + ?Sized, - { - panic!("IpcHandle cannot be used on host"); - } - - fn set_peer_user_signal(&self, _set: bool) -> pw_status::Result<()> { - panic!("IpcHandle cannot be used on host"); - } -} diff --git a/util/ipc/lib.rs b/util/ipc/lib.rs index 5d555f40a..762b8d9d9 100644 --- a/util/ipc/lib.rs +++ b/util/ipc/lib.rs @@ -1,12 +1,22 @@ // Licensed under the Apache-2.0 license // SPDX-License-Identifier: Apache-2.0 +//! IPC abstraction over Pigweed kernel channels. +//! +//! `IpcInitiator` and `IpcHandler` split the two channel roles the kernel +//! itself distinguishes (`ChannelInitiatorObject` vs `ChannelHandlerObject`), +//! so a type only has to implement the operations its role can actually +//! serve. `AsyncTransaction` is a safe layer on top of `IpcInitiator`'s +//! unsafe async trio, tracking the one-transaction-per-channel invariant +//! locally instead of leaving it to the caller. + #![no_std] use pw_status::Result; -/// Trait wrapping basic IPC operations on a channel. -pub trait IpcChannel { +/// Blocking and async operations available on the initiator side of a +/// channel (a `ChannelInitiatorObject` in the kernel). +pub trait IpcInitiator { fn transact( &self, send_data: &BufSend, @@ -24,9 +34,12 @@ pub trait IpcChannel { /// (blocking or async) is already pending on this channel. /// /// # Safety - /// `send_data`/`recv_data` are borrowed by the kernel until the - /// transaction is completed or cancelled — they must stay valid and - /// unmutated until then. + /// The kernel holds raw pointers into `send_data`/`recv_data` (and + /// writes the response into `recv_data`) until the transaction is + /// completed or cancelled — callers must not read or write either + /// buffer, or let them be dropped or moved, until then. `recv_data` + /// must be large enough to hold the response; a response that + /// overflows it is a kernel error, not truncated silently. unsafe fn async_transact_start( &self, send_data: &BufSend, @@ -39,7 +52,11 @@ pub trait IpcChannel { fn async_transact_complete(&self) -> Result; fn async_cancel(&self) -> Result<()>; +} +/// Operations available on the handler side of a channel (a +/// `ChannelHandlerObject` in the kernel). +pub trait IpcHandler { fn read(&self, offset: usize, buffer: &mut Buf) -> Result where Buf: AsSyscallBuffer + ?Sized; @@ -47,9 +64,6 @@ pub trait IpcChannel { fn respond(&self, buffer: &Buf) -> Result<()> where Buf: AsSyscallBuffer + ?Sized; - - /// Set (set=true) or clear (set=false) Signals::USER on the paired peer. - fn set_peer_user_signal(&self, set: bool) -> Result<()>; } /// Transparent wrapper around a raw IPC handle. @@ -65,12 +79,8 @@ impl IpcHandle { } } -#[cfg(target_os = "none")] +mod async_transaction; mod target; -#[cfg(target_os = "none")] -pub use target::{AsSyscallBuffer, Instant}; -#[cfg(not(target_os = "none"))] -mod host; -#[cfg(not(target_os = "none"))] -pub use host::{AsSyscallBuffer, Instant}; +pub use async_transaction::{AsyncTransaction, Buffers, Completion, StartError}; +pub use target::{AsSyscallBuffer, Instant}; diff --git a/util/ipc/target.rs b/util/ipc/target.rs index f48714244..d6ad34a74 100644 --- a/util/ipc/target.rs +++ b/util/ipc/target.rs @@ -1,12 +1,23 @@ // Licensed under the Apache-2.0 license // SPDX-License-Identifier: Apache-2.0 -use super::{IpcChannel, IpcHandle}; +use super::{IpcHandle, IpcHandler, IpcInitiator}; pub use userspace::buffer::AsSyscallBuffer; pub use userspace::time::Instant; -impl IpcChannel for IpcHandle { +impl IpcHandle { + /// Set (set=true) or clear (set=false) Signals::USER on the paired peer. + /// + /// Available on both channel roles, so it lives on the concrete + /// handle rather than being duplicated onto `IpcInitiator` and + /// `IpcHandler`. + pub fn set_peer_user_signal(&self, set: bool) -> pw_status::Result<()> { + userspace::syscall::object_set_peer_user_signal(self.handle, set) + } +} + +impl IpcInitiator for IpcHandle { fn transact( &self, send_data: &BufSend, @@ -51,7 +62,9 @@ impl IpcChannel for IpcHandle { fn async_cancel(&self) -> pw_status::Result<()> { userspace::syscall::channel_async_cancel(self.handle) } +} +impl IpcHandler for IpcHandle { fn read(&self, offset: usize, buffer: &mut Buf) -> pw_status::Result where Buf: AsSyscallBuffer + ?Sized, @@ -65,8 +78,4 @@ impl IpcChannel for IpcHandle { { userspace::syscall::channel_respond(self.handle, buffer) } - - fn set_peer_user_signal(&self, set: bool) -> pw_status::Result<()> { - userspace::syscall::object_set_peer_user_signal(self.handle, set) - } } From 61772b320bfc12d0a1047ac665d61151c4c8c96c Mon Sep 17 00:00:00 2001 From: Anthony Rocha Date: Tue, 22 Sep 2026 13:10:47 -0700 Subject: [PATCH 04/12] util/ipc: sort BUILD.bazel deps alphabetically CI's format check requires deps sorted; fixes the presubmit failure. Co-Authored-By: Claude Sonnet 5 --- util/ipc/BUILD.bazel | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/util/ipc/BUILD.bazel b/util/ipc/BUILD.bazel index 0c1536e91..d684ccda3 100644 --- a/util/ipc/BUILD.bazel +++ b/util/ipc/BUILD.bazel @@ -19,7 +19,7 @@ rust_library( }), visibility = ["//visibility:public"], deps = [ - "@pigweed//pw_status/rust:pw_status", "@pigweed//pw_kernel/userspace", + "@pigweed//pw_status/rust:pw_status", ], ) From 69b1b49aed19a70c67fc7e98fa8fe749f03e2c3f Mon Sep 17 00:00:00 2001 From: Anthony Rocha Date: Tue, 22 Sep 2026 14:17:43 -0700 Subject: [PATCH 05/12] target/ast10x0: add QEMU test exercising util_ipc AsyncTransaction Two-process system image (initiator/handler) modeled on pw_kernel/tests/async_ipc, but built on util_ipc's IpcInitiator/ IpcHandler/AsyncTransaction instead of raw syscalls. Covers blocking transact, async start/cancel (verifying the channel frees up), and async start/try_recv. Verified in QEMU and against rust_binary_no_panics_test. Co-Authored-By: Claude Sonnet 5 --- .../util_ipc/async_transaction/BUILD.bazel | 124 ++++++++++++++++++ .../async_transaction/handler_main.rs | 45 +++++++ .../async_transaction/initiator_main.rs | 106 +++++++++++++++ .../util_ipc/async_transaction/system.json5 | 78 +++++++++++ .../util_ipc/async_transaction/target.rs | 40 ++++++ 5 files changed, 393 insertions(+) create mode 100644 target/ast10x0/tests/util_ipc/async_transaction/BUILD.bazel create mode 100644 target/ast10x0/tests/util_ipc/async_transaction/handler_main.rs create mode 100644 target/ast10x0/tests/util_ipc/async_transaction/initiator_main.rs create mode 100644 target/ast10x0/tests/util_ipc/async_transaction/system.json5 create mode 100644 target/ast10x0/tests/util_ipc/async_transaction/target.rs diff --git a/target/ast10x0/tests/util_ipc/async_transaction/BUILD.bazel b/target/ast10x0/tests/util_ipc/async_transaction/BUILD.bazel new file mode 100644 index 000000000..4e247f6d2 --- /dev/null +++ b/target/ast10x0/tests/util_ipc/async_transaction/BUILD.bazel @@ -0,0 +1,124 @@ +# Licensed under the Apache-2.0 license +# SPDX-License-Identifier: Apache-2.0 + +load("@pigweed//pw_kernel/tooling:rust_app.bzl", "rust_app") +load("@pigweed//pw_kernel/tooling:system_image.bzl", "system_image", "system_image_test") +load("@pigweed//pw_kernel/tooling:target_codegen.bzl", "target_codegen") +load("@pigweed//pw_kernel/tooling:target_linker_script.bzl", "target_linker_script") +load("@pigweed//pw_kernel/tooling/panic_detector:rust_binary_no_panics_test.bzl", "rust_binary_no_panics_test") +load("@rules_rust//rust:defs.bzl", "rust_binary") +load("//target/ast10x0:defs.bzl", "TARGET_COMPATIBLE_WITH") + +# ── System configuration ─────────────────────────────────────────────────────── + +filegroup( + name = "system_config", + srcs = ["system.json5"], +) + +# ── Kernel image ─────────────────────────────────────────────────────────────── + +target_codegen( + name = "codegen", + arch = "@pigweed//pw_kernel/arch/arm_cortex_m:arch_arm_cortex_m", + system_config = ":system_config", + target_compatible_with = TARGET_COMPATIBLE_WITH, +) + +target_linker_script( + name = "linker_script", + system_config = ":system_config", + tags = ["kernel"], + target_compatible_with = TARGET_COMPATIBLE_WITH, + template = "//target/ast10x0:linker_script_template", +) + +rust_binary( + name = "target", + srcs = ["target.rs"], + edition = "2024", + tags = ["kernel"], + target_compatible_with = TARGET_COMPATIBLE_WITH, + deps = [ + ":codegen", + ":linker_script", + "//target/ast10x0:entry", + "@pigweed//pw_kernel/arch/arm_cortex_m:arch_arm_cortex_m", + "@pigweed//pw_kernel/kernel", + "@pigweed//pw_kernel/subsys/console:console_backend", + "@pigweed//pw_kernel/target:target_common", + "@pigweed//pw_kernel/userspace", + "@pigweed//pw_log/rust:pw_log", + ], +) + +# ── Handler app ───────────────────────────────────────────────────────────────── +# Exercises util_ipc::IpcHandler; increments the byte it's sent. + +rust_app( + name = "handler", + srcs = ["handler_main.rs"], + codegen_crate_name = "app_handler", + edition = "2024", + system_config = ":system_config", + tags = ["kernel"], + target_compatible_with = TARGET_COMPATIBLE_WITH, + deps = [ + "//util/ipc:ipc", + "@pigweed//pw_kernel/userspace", + "@pigweed//pw_status/rust:pw_status", + ], +) + +# ── Initiator app ─────────────────────────────────────────────────────────────── +# Exercises util_ipc::IpcInitiator and util_ipc::AsyncTransaction; calls +# debug_shutdown(Ok|Err) to report the result. + +rust_app( + name = "initiator", + srcs = ["initiator_main.rs"], + codegen_crate_name = "app_initiator", + edition = "2024", + system_config = ":system_config", + tags = ["kernel"], + target_compatible_with = TARGET_COMPATIBLE_WITH, + deps = [ + "//util/ipc:ipc", + "@pigweed//pw_kernel/userspace", + "@pigweed//pw_log/rust:pw_log", + "@pigweed//pw_status/rust:pw_status", + ], +) + +# ── System image ─────────────────────────────────────────────────────────────── + +system_image( + name = "async_transaction_image", + apps = [ + ":handler", + ":initiator", + ], + kernel = ":target", + platform = "//target/ast10x0", + system_config = ":system_config", + tags = ["kernel"], + target_compatible_with = TARGET_COMPATIBLE_WITH, + visibility = ["//visibility:public"], +) + +# ── Test target ──────────────────────────────────────────────────────────────── +# Run with: +# bazel test --config=virt_ast10x0 //target/ast10x0/tests/util_ipc/async_transaction:async_transaction_qemu_test + +system_image_test( + name = "async_transaction_qemu_test", + image = ":async_transaction_image", + tags = ["qemu_only"], + target_compatible_with = TARGET_COMPATIBLE_WITH, +) + +rust_binary_no_panics_test( + name = "no_panics_test", + binary = ":async_transaction_image", + tags = ["kernel"], +) diff --git a/target/ast10x0/tests/util_ipc/async_transaction/handler_main.rs b/target/ast10x0/tests/util_ipc/async_transaction/handler_main.rs new file mode 100644 index 000000000..d38a21c30 --- /dev/null +++ b/target/ast10x0/tests/util_ipc/async_transaction/handler_main.rs @@ -0,0 +1,45 @@ +// Licensed under the Apache-2.0 license +// SPDX-License-Identifier: Apache-2.0 + +//! Handler side of the util/ipc AsyncTransaction QEMU test. +//! +//! Exercises `util_ipc::IpcHandler`: waits for a request, reads it, and +//! responds with the request byte incremented by one. + +#![no_main] +#![no_std] + +use app_handler::handle; +use pw_status::Error; +use userspace::syscall::{self, Signals}; +use userspace::time::Instant; +use userspace::entry; +use util_ipc::{IpcHandle, IpcHandler}; + +#[entry] +fn entry() { + let ipc = IpcHandle::new(handle::IPC); + + loop { + if syscall::object_wait(handle::IPC, Signals::READABLE, Instant::MAX).is_err() { + continue; + } + + let mut buf = [0u8; 1]; + match ipc.read(0, &mut buf) { + Ok(1) => { + buf[0] = buf[0].wrapping_add(1); + let _ = ipc.respond(&buf); + } + // Transaction was cancelled by the initiator while we were + // waking up; go back to waiting for the next one. + Ok(_) | Err(Error::Unavailable) => continue, + Err(_) => continue, + } + } +} + +#[panic_handler] +fn panic(_info: &core::panic::PanicInfo) -> ! { + loop {} +} diff --git a/target/ast10x0/tests/util_ipc/async_transaction/initiator_main.rs b/target/ast10x0/tests/util_ipc/async_transaction/initiator_main.rs new file mode 100644 index 000000000..426d78bd7 --- /dev/null +++ b/target/ast10x0/tests/util_ipc/async_transaction/initiator_main.rs @@ -0,0 +1,106 @@ +// Licensed under the Apache-2.0 license +// SPDX-License-Identifier: Apache-2.0 + +//! Initiator side of the util/ipc AsyncTransaction QEMU test. +//! +//! Runs three cases against `handler` and calls `debug_shutdown(Ok(()))` on +//! full pass or `debug_shutdown(Err(_))` on the first failure. The kernel +//! target writes `TEST_RESULT:PASS/FAIL` to UART. +//! +//! | Case | Exercises | Expect | +//! |--------------------|-------------------------------------|-------------------| +//! | blocking transact | `IpcInitiator::transact` | byte incremented | +//! | async cancel | `AsyncTransaction::start`/`cancel` | channel freed | +//! | async roundtrip | `AsyncTransaction::start`/`try_recv`| byte incremented | + +#![no_main] +#![no_std] + +use app_initiator::handle; +use pw_status::{Error, Result}; +use userspace::syscall::{self, Signals}; +use userspace::time::Instant; +use userspace::entry; +use util_ipc::{AsyncTransaction, IpcHandle, IpcInitiator}; + +static SEND_BUF: [u8; 1] = [0x10]; +static mut RECV_BUF: [u8; 1] = [0u8; 1]; + +/// # Safety +/// Only called from this single-threaded app, and only while no +/// `AsyncTransaction` still holds a prior borrow of `RECV_BUF`. +unsafe fn recv_buf() -> &'static mut [u8] { + // Safety: see function doc. + unsafe { &mut *core::ptr::addr_of_mut!(RECV_BUF) } +} + +fn test_blocking_transact() -> Result<()> { + let ipc = IpcHandle::new(handle::IPC); + let send = [0x20u8]; + let mut recv = [0u8; 1]; + + let len = ipc.transact(&send, &mut recv, Instant::MAX)?; + if len != 1 || recv[0] != 0x21 { + pw_log::error!("blocking transact: unexpected response"); + return Err(Error::Internal); + } + Ok(()) +} + +fn test_async_cancel() -> Result<()> { + let mut txn = AsyncTransaction::new(IpcHandle::new(handle::IPC)); + // Safety: no other AsyncTransaction is live right now. + txn.start(&SEND_BUF, unsafe { recv_buf() }) + .map_err(|e| e.error)?; + + txn.cancel()?; + if txn.is_pending() { + pw_log::error!("async cancel: still pending after cancel()"); + return Err(Error::Internal); + } + + // Verify the channel is free again. + // Safety: the previous transaction was cancelled above. + txn.start(&SEND_BUF, unsafe { recv_buf() }) + .map_err(|e| e.error)?; + txn.cancel()?; + Ok(()) +} + +fn test_async_roundtrip() -> Result<()> { + let mut txn = AsyncTransaction::new(IpcHandle::new(handle::IPC)); + // Safety: no other AsyncTransaction is live right now. + txn.start(&SEND_BUF, unsafe { recv_buf() }) + .map_err(|e| e.error)?; + + let raw_handle = txn.handle().handle; + syscall::object_wait(raw_handle, Signals::READABLE, Instant::MAX)?; + + let completion = txn.try_recv()?; + if completion.len != 1 || completion.recv[0] != 0x11 { + pw_log::error!("async roundtrip: unexpected response"); + return Err(Error::Internal); + } + Ok(()) +} + +#[entry] +fn entry() { + let ret = test_blocking_transact() + .and_then(|_| test_async_cancel()) + .and_then(|_| test_async_roundtrip()); + + match &ret { + Ok(()) => pw_log::info!("All test cases PASSED"), + Err(e) => pw_log::error!("FAILED: status code {}", *e as u32), + } + + let _ = syscall::debug_shutdown(ret); + loop {} +} + +#[panic_handler] +fn panic(_info: &core::panic::PanicInfo) -> ! { + let _ = userspace::syscall::debug_shutdown(Err(pw_status::Error::Internal)); + loop {} +} diff --git a/target/ast10x0/tests/util_ipc/async_transaction/system.json5 b/target/ast10x0/tests/util_ipc/async_transaction/system.json5 new file mode 100644 index 000000000..30e14d455 --- /dev/null +++ b/target/ast10x0/tests/util_ipc/async_transaction/system.json5 @@ -0,0 +1,78 @@ +// Licensed under the Apache-2.0 license +// SPDX-License-Identifier: Apache-2.0 + +// AST10x0 util/ipc AsyncTransaction QEMU test. +// +// Two-process layout: +// +// initiator — exercises util_ipc::IpcInitiator (blocking transact) and +// util_ipc::AsyncTransaction (async start/try_recv/cancel); +// calls debug_shutdown(Ok|Err) to report the result. +// handler — util_ipc::IpcHandler read/respond loop that increments the +// single byte it's sent. +// +// Memory map (AST10x0: 768 KB SRAM, no XIP): +// 0x00000000 - 0x00000500 vector table (1280 B) +// 0x00000500 - 0x00020500 kernel flash (~128 KB) +// 0x00020500 - 0x00060500 app flash (256 KB total; 128 KB per app) +// 0x00060000 - 0x00080000 kernel RAM (128 KB) +// 0x00080000 - 0x000C0000 app RAM (256 KB total; 64 KB + 32 KB per process) +{ + arch: { + type: "armv7m", + vector_table_start_address: 0x00000000, + vector_table_size_bytes: 1280, + }, + kernel: { + flash_start_address: 0x00000500, + flash_size_bytes: 129792, + ram_start_address: 0x00060000, + ram_size_bytes: 131072, + }, + apps: [ + { + name: "initiator", + flash_size_bytes: 131072, + processes: [ + { + name: "initiator_process", + ram_size_bytes: 65536, + objects: [ + { + name: "ipc", + type: "channel_initiator", + handler_process: "handler_process", + handler_object_name: "ipc", + }, + { + type: "thread", + name: "initiator_thread", + kernel_stack_size_bytes: 4096, + },], + + }, + ], + }, + { + name: "handler", + flash_size_bytes: 131072, + processes: [ + { + name: "handler_process", + ram_size_bytes: 32768, + objects: [ + { + name: "ipc", + type: "channel_handler", + }, + { + type: "thread", + name: "handler_thread", + kernel_stack_size_bytes: 4096, + },], + + }, + ], + }, + ], +} diff --git a/target/ast10x0/tests/util_ipc/async_transaction/target.rs b/target/ast10x0/tests/util_ipc/async_transaction/target.rs new file mode 100644 index 000000000..022a30193 --- /dev/null +++ b/target/ast10x0/tests/util_ipc/async_transaction/target.rs @@ -0,0 +1,40 @@ +// Licensed under the Apache-2.0 license +// SPDX-License-Identifier: Apache-2.0 + +//! Kernel target for the util/ipc AsyncTransaction QEMU test. +//! +//! Pass/fail is communicated by `initiator` calling +//! `syscall::debug_shutdown(Ok(()) | Err(...))`, which lands here and writes +//! the UART sentinel picked up by qemu_runner.py. + +#![no_std] +#![no_main] + +use console_backend::console_backend_write_all; +use entry as _; +use target_common::{declare_target, TargetInterface}; + +pub struct Target {} + +impl TargetInterface for Target { + const NAME: &'static str = "AST10x0 util/ipc AsyncTransaction test"; + + fn main() -> ! { + codegen::start(); + #[expect(clippy::empty_loop)] + loop {} + } + + fn shutdown(code: u32) -> ! { + let sentinel: &[u8] = if code == 0 { + b"TEST_RESULT:PASS\n" + } else { + b"TEST_RESULT:FAIL\n" + }; + let _ = console_backend_write_all(sentinel); + #[expect(clippy::empty_loop)] + loop {} + } +} + +declare_target!(Target); From 0a325c5547e08f0ed70ae7f216c379c118b7317d Mon Sep 17 00:00:00 2001 From: Anthony Rocha Date: Tue, 22 Sep 2026 14:26:16 -0700 Subject: [PATCH 06/12] util/ipc: add IpcInitiator::as_raw, stop leaking H's internals MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit AsyncTransaction::handle() returned &H, but the only way to get a waitable id from it was reaching into IpcHandle's public field — which only works because H happens to be IpcHandle, defeating the point of being generic over IpcInitiator. as_raw() makes "get the raw handle for object_wait/WaitGroup" part of the trait contract instead. Co-Authored-By: Claude Sonnet 5 --- .../tests/util_ipc/async_transaction/initiator_main.rs | 3 +-- util/ipc/async_transaction.rs | 7 ++++--- util/ipc/lib.rs | 4 ++++ util/ipc/target.rs | 4 ++++ 4 files changed, 13 insertions(+), 5 deletions(-) diff --git a/target/ast10x0/tests/util_ipc/async_transaction/initiator_main.rs b/target/ast10x0/tests/util_ipc/async_transaction/initiator_main.rs index 426d78bd7..626da0753 100644 --- a/target/ast10x0/tests/util_ipc/async_transaction/initiator_main.rs +++ b/target/ast10x0/tests/util_ipc/async_transaction/initiator_main.rs @@ -73,8 +73,7 @@ fn test_async_roundtrip() -> Result<()> { txn.start(&SEND_BUF, unsafe { recv_buf() }) .map_err(|e| e.error)?; - let raw_handle = txn.handle().handle; - syscall::object_wait(raw_handle, Signals::READABLE, Instant::MAX)?; + syscall::object_wait(txn.as_raw(), Signals::READABLE, Instant::MAX)?; let completion = txn.try_recv()?; if completion.len != 1 || completion.recv[0] != 0x11 { diff --git a/util/ipc/async_transaction.rs b/util/ipc/async_transaction.rs index f9202054f..5e61b4ab1 100644 --- a/util/ipc/async_transaction.rs +++ b/util/ipc/async_transaction.rs @@ -45,9 +45,10 @@ impl AsyncTransaction { } } - /// The wrapped initiator, e.g. to register it with a WaitGroup. - pub fn handle(&self) -> &H { - &self.handle + /// The raw channel handle, e.g. to register with a WaitGroup or pass to + /// `object_wait`. + pub fn as_raw(&self) -> u32 { + self.handle.as_raw() } /// Whether a transaction is in flight. diff --git a/util/ipc/lib.rs b/util/ipc/lib.rs index 762b8d9d9..cdb3d3c86 100644 --- a/util/ipc/lib.rs +++ b/util/ipc/lib.rs @@ -52,6 +52,10 @@ pub trait IpcInitiator { fn async_transact_complete(&self) -> Result; fn async_cancel(&self) -> Result<()>; + + /// The raw channel handle, e.g. to register with a WaitGroup or pass to + /// `object_wait`. + fn as_raw(&self) -> u32; } /// Operations available on the handler side of a channel (a diff --git a/util/ipc/target.rs b/util/ipc/target.rs index d6ad34a74..de47d6202 100644 --- a/util/ipc/target.rs +++ b/util/ipc/target.rs @@ -62,6 +62,10 @@ impl IpcInitiator for IpcHandle { fn async_cancel(&self) -> pw_status::Result<()> { userspace::syscall::channel_async_cancel(self.handle) } + + fn as_raw(&self) -> u32 { + self.handle + } } impl IpcHandler for IpcHandle { From 50870bae15b02b8fccbe4a23926d04a3ca9c2c42 Mon Sep 17 00:00:00 2001 From: Anthony Rocha Date: Tue, 22 Sep 2026 14:34:55 -0700 Subject: [PATCH 07/12] util/ipc: Completion/StartError wrap Buffers, relocate signal doc Completion and StartError each duplicated Buffers's send/recv fields instead of embedding it, so the same pair had to be kept in sync in three places. Both now hold a `buffers: Buffers` field. Also moves the "why is set_peer_user_signal on IpcHandle and not the traits" rationale from target.rs (where the impl lives) to lib.rs (where a reader looking at IpcInitiator/IpcHandler would actually ask the question). Co-Authored-By: Claude Sonnet 5 --- .../async_transaction/initiator_main.rs | 2 +- util/ipc/async_transaction.rs | 25 +++++++++---------- util/ipc/lib.rs | 5 ++++ util/ipc/target.rs | 4 --- 4 files changed, 18 insertions(+), 18 deletions(-) diff --git a/target/ast10x0/tests/util_ipc/async_transaction/initiator_main.rs b/target/ast10x0/tests/util_ipc/async_transaction/initiator_main.rs index 626da0753..660ae3b84 100644 --- a/target/ast10x0/tests/util_ipc/async_transaction/initiator_main.rs +++ b/target/ast10x0/tests/util_ipc/async_transaction/initiator_main.rs @@ -76,7 +76,7 @@ fn test_async_roundtrip() -> Result<()> { syscall::object_wait(txn.as_raw(), Signals::READABLE, Instant::MAX)?; let completion = txn.try_recv()?; - if completion.len != 1 || completion.recv[0] != 0x11 { + if completion.len != 1 || completion.buffers.recv[0] != 0x11 { pw_log::error!("async roundtrip: unexpected response"); return Err(Error::Internal); } diff --git a/util/ipc/async_transaction.rs b/util/ipc/async_transaction.rs index 5e61b4ab1..ae3bbe5b8 100644 --- a/util/ipc/async_transaction.rs +++ b/util/ipc/async_transaction.rs @@ -74,8 +74,7 @@ impl AsyncTransaction { if self.inflight.is_some() { return Err(StartError { error: Error::FailedPrecondition, - send, - recv, + buffers: Buffers { send, recv }, }); } @@ -89,7 +88,10 @@ impl AsyncTransaction { self.inflight = Some(Buffers { send, recv }); Ok(()) } - Err(error) => Err(StartError { error, send, recv }), + Err(error) => Err(StartError { + error, + buffers: Buffers { send, recv }, + }), } } @@ -112,8 +114,8 @@ impl AsyncTransaction { } let len = self.handle.async_transact_complete()?; - let Buffers { send, recv } = self.inflight.take().unwrap(); - Ok(Completion { len, send, recv }) + let buffers = self.inflight.take().unwrap(); + Ok(Completion { len, buffers }) } /// Cancel a pending transaction and reclaim the buffers. @@ -145,19 +147,16 @@ impl Drop for AsyncTransaction { /// Successful completion of an async transaction. #[derive(Debug)] pub struct Completion { - /// Number of response bytes written into `recv`. + /// Number of response bytes written into `buffers.recv`. pub len: usize, - /// The send buffer, returned for reuse. - pub send: &'static [u8], - /// The receive buffer, returned for reuse. `recv[..len]` holds the - /// response payload. - pub recv: &'static mut [u8], + /// The buffers, returned for reuse. `recv[..len]` holds the response + /// payload. + pub buffers: Buffers, } /// Error from `start()`, carrying the buffers back so they are not lost. #[derive(Debug)] pub struct StartError { pub error: Error, - pub send: &'static [u8], - pub recv: &'static mut [u8], + pub buffers: Buffers, } diff --git a/util/ipc/lib.rs b/util/ipc/lib.rs index cdb3d3c86..acec2e868 100644 --- a/util/ipc/lib.rs +++ b/util/ipc/lib.rs @@ -71,6 +71,11 @@ pub trait IpcHandler { } /// Transparent wrapper around a raw IPC handle. +/// +/// `set_peer_user_signal` (defined in the target-only impl) is an inherent +/// method here rather than on `IpcInitiator`/`IpcHandler`, since it's usable +/// from either channel role and duplicating it onto both traits would just +/// mean two copies of the same forwarding call to keep in sync. #[repr(transparent)] #[derive(Debug, Copy, Clone, PartialEq, Eq)] pub struct IpcHandle { diff --git a/util/ipc/target.rs b/util/ipc/target.rs index de47d6202..976cb9864 100644 --- a/util/ipc/target.rs +++ b/util/ipc/target.rs @@ -8,10 +8,6 @@ pub use userspace::time::Instant; impl IpcHandle { /// Set (set=true) or clear (set=false) Signals::USER on the paired peer. - /// - /// Available on both channel roles, so it lives on the concrete - /// handle rather than being duplicated onto `IpcInitiator` and - /// `IpcHandler`. pub fn set_peer_user_signal(&self, set: bool) -> pw_status::Result<()> { userspace::syscall::object_set_peer_user_signal(self.handle, set) } From aedf8ba86d3fd75be3c994ed6e69d4758259c03d Mon Sep 17 00:00:00 2001 From: Anthony Rocha Date: Tue, 22 Sep 2026 14:59:59 -0700 Subject: [PATCH 08/12] util/ipc: impl From for Error, drop map_err at call sites start() returns Result<(), StartError> so callers can reclaim buffers on failure, but that meant every call site paid a .map_err(|e| e.error)? tax to unify with surrounding pw_status::Result code, even when the buffers weren't needed back. From for Error lets ? convert directly; StartError is still there for callers who do want the buffers. Co-Authored-By: Claude Sonnet 5 --- .../tests/util_ipc/async_transaction/initiator_main.rs | 9 +++------ util/ipc/async_transaction.rs | 7 +++++++ 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/target/ast10x0/tests/util_ipc/async_transaction/initiator_main.rs b/target/ast10x0/tests/util_ipc/async_transaction/initiator_main.rs index 660ae3b84..7d23c87fd 100644 --- a/target/ast10x0/tests/util_ipc/async_transaction/initiator_main.rs +++ b/target/ast10x0/tests/util_ipc/async_transaction/initiator_main.rs @@ -50,8 +50,7 @@ fn test_blocking_transact() -> Result<()> { fn test_async_cancel() -> Result<()> { let mut txn = AsyncTransaction::new(IpcHandle::new(handle::IPC)); // Safety: no other AsyncTransaction is live right now. - txn.start(&SEND_BUF, unsafe { recv_buf() }) - .map_err(|e| e.error)?; + txn.start(&SEND_BUF, unsafe { recv_buf() })?; txn.cancel()?; if txn.is_pending() { @@ -61,8 +60,7 @@ fn test_async_cancel() -> Result<()> { // Verify the channel is free again. // Safety: the previous transaction was cancelled above. - txn.start(&SEND_BUF, unsafe { recv_buf() }) - .map_err(|e| e.error)?; + txn.start(&SEND_BUF, unsafe { recv_buf() })?; txn.cancel()?; Ok(()) } @@ -70,8 +68,7 @@ fn test_async_cancel() -> Result<()> { fn test_async_roundtrip() -> Result<()> { let mut txn = AsyncTransaction::new(IpcHandle::new(handle::IPC)); // Safety: no other AsyncTransaction is live right now. - txn.start(&SEND_BUF, unsafe { recv_buf() }) - .map_err(|e| e.error)?; + txn.start(&SEND_BUF, unsafe { recv_buf() })?; syscall::object_wait(txn.as_raw(), Signals::READABLE, Instant::MAX)?; diff --git a/util/ipc/async_transaction.rs b/util/ipc/async_transaction.rs index ae3bbe5b8..c0619c67d 100644 --- a/util/ipc/async_transaction.rs +++ b/util/ipc/async_transaction.rs @@ -160,3 +160,10 @@ pub struct StartError { pub error: Error, pub buffers: Buffers, } + +impl From for Error { + /// Drops the reclaimed buffers; use `StartError` directly to reuse them. + fn from(e: StartError) -> Self { + e.error + } +} From aedcfa6d160095d2b44e0ec3fc24062fefec7f53 Mon Sep 17 00:00:00 2001 From: Anthony Rocha Date: Tue, 22 Sep 2026 15:15:21 -0700 Subject: [PATCH 09/12] target/ast10x0: fix CI format check on the new util_ipc test buildifier wants the //util/ipc shorthand (not //util/ipc:ipc), and rustfmt wants imports sorted; both were flagged by the presubmit format check on fd6aee7e. Co-Authored-By: Claude Sonnet 5 --- target/ast10x0/tests/util_ipc/async_transaction/BUILD.bazel | 4 ++-- .../ast10x0/tests/util_ipc/async_transaction/handler_main.rs | 2 +- .../tests/util_ipc/async_transaction/initiator_main.rs | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/target/ast10x0/tests/util_ipc/async_transaction/BUILD.bazel b/target/ast10x0/tests/util_ipc/async_transaction/BUILD.bazel index 4e247f6d2..b2a0e272c 100644 --- a/target/ast10x0/tests/util_ipc/async_transaction/BUILD.bazel +++ b/target/ast10x0/tests/util_ipc/async_transaction/BUILD.bazel @@ -64,7 +64,7 @@ rust_app( tags = ["kernel"], target_compatible_with = TARGET_COMPATIBLE_WITH, deps = [ - "//util/ipc:ipc", + "//util/ipc", "@pigweed//pw_kernel/userspace", "@pigweed//pw_status/rust:pw_status", ], @@ -83,7 +83,7 @@ rust_app( tags = ["kernel"], target_compatible_with = TARGET_COMPATIBLE_WITH, deps = [ - "//util/ipc:ipc", + "//util/ipc", "@pigweed//pw_kernel/userspace", "@pigweed//pw_log/rust:pw_log", "@pigweed//pw_status/rust:pw_status", diff --git a/target/ast10x0/tests/util_ipc/async_transaction/handler_main.rs b/target/ast10x0/tests/util_ipc/async_transaction/handler_main.rs index d38a21c30..d10873962 100644 --- a/target/ast10x0/tests/util_ipc/async_transaction/handler_main.rs +++ b/target/ast10x0/tests/util_ipc/async_transaction/handler_main.rs @@ -11,9 +11,9 @@ use app_handler::handle; use pw_status::Error; +use userspace::entry; use userspace::syscall::{self, Signals}; use userspace::time::Instant; -use userspace::entry; use util_ipc::{IpcHandle, IpcHandler}; #[entry] diff --git a/target/ast10x0/tests/util_ipc/async_transaction/initiator_main.rs b/target/ast10x0/tests/util_ipc/async_transaction/initiator_main.rs index 7d23c87fd..29bb0bdf2 100644 --- a/target/ast10x0/tests/util_ipc/async_transaction/initiator_main.rs +++ b/target/ast10x0/tests/util_ipc/async_transaction/initiator_main.rs @@ -18,9 +18,9 @@ use app_initiator::handle; use pw_status::{Error, Result}; +use userspace::entry; use userspace::syscall::{self, Signals}; use userspace::time::Instant; -use userspace::entry; use util_ipc::{AsyncTransaction, IpcHandle, IpcInitiator}; static SEND_BUF: [u8; 1] = [0x10]; From e9cc337d33314c017f5cc7f1a8fe764f22f812cd Mon Sep 17 00:00:00 2001 From: Anthony Rocha Date: Tue, 22 Sep 2026 15:34:58 -0700 Subject: [PATCH 10/12] util/ipc: suppress semgrep unsafe-usage on async_transact_start call target.rs's equivalent unsafe block already carries this suppression; async_transaction.rs's call site was missing it, which is what's failing Security Audit on PR #479 right now. Co-Authored-By: Claude Sonnet 5 --- util/ipc/async_transaction.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/util/ipc/async_transaction.rs b/util/ipc/async_transaction.rs index c0619c67d..10c77c896 100644 --- a/util/ipc/async_transaction.rs +++ b/util/ipc/async_transaction.rs @@ -81,6 +81,7 @@ impl AsyncTransaction { // Safety: send/recv are 'static, so the kernel's raw pointers // stay valid regardless of what happens to `self`, and they are // not read, written, or dropped again until try_recv/cancel. + // nosemgrep let result = unsafe { self.handle.async_transact_start(send, recv) }; match result { From abde8456659d1c77726b264fea0bc991d3ead477 Mon Sep 17 00:00:00 2001 From: Anthony Rocha Date: Wed, 23 Sep 2026 08:54:38 -0700 Subject: [PATCH 11/12] util/ipc: fix cancel() losing buffers on syscall error, add IpcHandle docs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit cancel() used self.handle.async_cancel()? before taking inflight, so a syscall error left inflight set and the caller's Err carried no buffers back — they became unreachable from safe code. The kernel clears its transaction slot on every path out of the cancel syscall (traced through finish_transaction), so it's always safe to hand the buffers back regardless of the syscall's result; take() them first, unconditionally. Also documents that wrapping the same handle in two AsyncTransactions is a locally-detected Unavailable, not unsoundness, and that new() trusts its caller. Co-Authored-By: chrysh Co-Authored-By: Claude Sonnet 5 --- util/ipc/async_transaction.rs | 15 +++++++++------ util/ipc/lib.rs | 7 +++++++ 2 files changed, 16 insertions(+), 6 deletions(-) diff --git a/util/ipc/async_transaction.rs b/util/ipc/async_transaction.rs index 10c77c896..fba7de927 100644 --- a/util/ipc/async_transaction.rs +++ b/util/ipc/async_transaction.rs @@ -125,15 +125,18 @@ impl AsyncTransaction { /// discarded. /// /// Returns `Err(Error::FailedPrecondition)` if no transaction is - /// pending. Propagates unexpected kernel errors with the buffers - /// still held (use `cancel()` again or drop the struct). + /// pending. The buffers come back even if the cancel syscall errors: the + /// kernel clears its transaction slot on every path, so it no longer + /// holds pointers into them. pub fn cancel(&mut self) -> Result { - if self.inflight.is_none() { + let Some(buffers) = self.inflight.take() else { return Err(Error::FailedPrecondition); - } + }; - self.handle.async_cancel()?; - Ok(self.inflight.take().unwrap()) + // Only failure for a started transaction is Unavailable, meaning the + // transaction was already dropped. The buffers are free either way. + let _ = self.handle.async_cancel(); + Ok(buffers) } } diff --git a/util/ipc/lib.rs b/util/ipc/lib.rs index acec2e868..6c69f2dd7 100644 --- a/util/ipc/lib.rs +++ b/util/ipc/lib.rs @@ -76,6 +76,11 @@ pub trait IpcHandler { /// method here rather than on `IpcInitiator`/`IpcHandler`, since it's usable /// from either channel role and duplicating it onto both traits would just /// mean two copies of the same forwarding call to keep in sync. +/// +/// One transaction per channel is a kernel rule, not something this type +/// enforces: wrapping the same handle in two `AsyncTransaction`s just gets +/// you `Unavailable` on the second `start`, with the buffers handed back — +/// not anything unsound. #[repr(transparent)] #[derive(Debug, Copy, Clone, PartialEq, Eq)] pub struct IpcHandle { @@ -83,6 +88,8 @@ pub struct IpcHandle { } impl IpcHandle { + /// Wraps a raw channel handle. Trusts the caller to pass one the kernel + /// actually gave out. pub const fn new(handle: u32) -> Self { Self { handle } } From d874e15b84d271e8ee98248670907e9824188085 Mon Sep 17 00:00:00 2001 From: Anthony Rocha Date: Wed, 23 Sep 2026 08:54:46 -0700 Subject: [PATCH 12/12] target/ast10x0: add drop-cancel/double-start/try_recv-early test cases Three cases the original three didn't cover: dropping a pending transaction actually cancels it (proves the Drop impl works, not just that it compiles), starting twice while pending hands both buffers back in StartError, and try_recv before the handler responds reports Unavailable with the transaction still pending. The handler gates one request behind a USER signal handshake so the "not yet responded" observation is deterministic instead of a race. The whole sequence now runs 50 rounds to catch state (signals, mostly) leaking between passes. Patch from chrysh's PR review, mutation-tested against 008045a1. Co-Authored-By: chrysh Co-Authored-By: Claude Sonnet 5 --- .../async_transaction/handler_main.rs | 19 +- .../async_transaction/initiator_main.rs | 166 +++++++++++++++++- 2 files changed, 177 insertions(+), 8 deletions(-) diff --git a/target/ast10x0/tests/util_ipc/async_transaction/handler_main.rs b/target/ast10x0/tests/util_ipc/async_transaction/handler_main.rs index d10873962..a9d778316 100644 --- a/target/ast10x0/tests/util_ipc/async_transaction/handler_main.rs +++ b/target/ast10x0/tests/util_ipc/async_transaction/handler_main.rs @@ -4,7 +4,8 @@ //! Handler side of the util/ipc AsyncTransaction QEMU test. //! //! Exercises `util_ipc::IpcHandler`: waits for a request, reads it, and -//! responds with the request byte incremented by one. +//! responds with the request byte incremented by one. A request of +//! `GATED_REQUEST` is held until the initiator raises Signals::USER. #![no_main] #![no_std] @@ -16,6 +17,12 @@ use userspace::syscall::{self, Signals}; use userspace::time::Instant; use util_ipc::{IpcHandle, IpcHandler}; +/// Request byte that parks the handler instead of responding: it raises +/// Signals::USER on the initiator to say it is parked, then waits for the +/// initiator to raise USER back before responding. The parked signal is +/// what makes the initiator's "pending" observation deterministic. +const GATED_REQUEST: u8 = 0x40; + #[entry] fn entry() { let ipc = IpcHandle::new(handle::IPC); @@ -28,6 +35,16 @@ fn entry() { let mut buf = [0u8; 1]; match ipc.read(0, &mut buf) { Ok(1) => { + if buf[0] == GATED_REQUEST { + // Tell the initiator we are parked, wait for its + // release, then lower the parked signal again. + if ipc.set_peer_user_signal(true).is_err() + || syscall::object_wait(handle::IPC, Signals::USER, Instant::MAX).is_err() + { + continue; + } + let _ = ipc.set_peer_user_signal(false); + } buf[0] = buf[0].wrapping_add(1); let _ = ipc.respond(&buf); } diff --git a/target/ast10x0/tests/util_ipc/async_transaction/initiator_main.rs b/target/ast10x0/tests/util_ipc/async_transaction/initiator_main.rs index 29bb0bdf2..eebb38047 100644 --- a/target/ast10x0/tests/util_ipc/async_transaction/initiator_main.rs +++ b/target/ast10x0/tests/util_ipc/async_transaction/initiator_main.rs @@ -3,15 +3,20 @@ //! Initiator side of the util/ipc AsyncTransaction QEMU test. //! -//! Runs three cases against `handler` and calls `debug_shutdown(Ok(()))` on -//! full pass or `debug_shutdown(Err(_))` on the first failure. The kernel -//! target writes `TEST_RESULT:PASS/FAIL` to UART. +//! Runs the cases below against `handler`, `ROUNDS` times over, and calls +//! `debug_shutdown(Ok(()))` on full pass or `debug_shutdown(Err(_))` on the +//! first failure. The kernel target writes `TEST_RESULT:PASS/FAIL` to UART. +//! Repeating catches state left behind by an earlier pass, a signal never +//! lowered above all. //! //! | Case | Exercises | Expect | //! |--------------------|-------------------------------------|-------------------| //! | blocking transact | `IpcInitiator::transact` | byte incremented | //! | async cancel | `AsyncTransaction::start`/`cancel` | channel freed | //! | async roundtrip | `AsyncTransaction::start`/`try_recv`| byte incremented | +//! | drop cancels | `AsyncTransaction::drop` | channel freed | +//! | double start | `start` while pending | buffers returned | +//! | try_recv early | `try_recv` before the response | `Unavailable` | #![no_main] #![no_std] @@ -20,12 +25,20 @@ use app_initiator::handle; use pw_status::{Error, Result}; use userspace::entry; use userspace::syscall::{self, Signals}; -use userspace::time::Instant; +use userspace::time::{Clock, Instant, SystemClock}; use util_ipc::{AsyncTransaction, IpcHandle, IpcInitiator}; static SEND_BUF: [u8; 1] = [0x10]; static mut RECV_BUF: [u8; 1] = [0u8; 1]; +/// Second receive buffer, for the case that starts a transaction while +/// another one still borrows `RECV_BUF`. +static mut RECV_BUF2: [u8; 1] = [0u8; 1]; + +/// Request byte the handler holds until Signals::USER is raised, matching +/// `GATED_REQUEST` in handler_main.rs. +static SEND_GATED: [u8; 1] = [0x40]; + /// # Safety /// Only called from this single-threaded app, and only while no /// `AsyncTransaction` still holds a prior borrow of `RECV_BUF`. @@ -34,6 +47,13 @@ unsafe fn recv_buf() -> &'static mut [u8] { unsafe { &mut *core::ptr::addr_of_mut!(RECV_BUF) } } +/// # Safety +/// Same contract as `recv_buf`, for the second buffer. +unsafe fn recv_buf2() -> &'static mut [u8] { + // Safety: see function doc. + unsafe { &mut *core::ptr::addr_of_mut!(RECV_BUF2) } +} + fn test_blocking_transact() -> Result<()> { let ipc = IpcHandle::new(handle::IPC); let send = [0x20u8]; @@ -80,11 +100,143 @@ fn test_async_roundtrip() -> Result<()> { Ok(()) } +/// Dropping a pending transaction has to cancel it, otherwise the channel +/// stays busy and the next start fails with `Unavailable`. +fn test_drop_cancels() -> Result<()> { + { + let mut txn = AsyncTransaction::new(IpcHandle::new(handle::IPC)); + // Safety: no other AsyncTransaction is live right now. + txn.start(&SEND_BUF, unsafe { recv_buf() })?; + // Dropped here with the transaction still pending. + } + + let mut txn = AsyncTransaction::new(IpcHandle::new(handle::IPC)); + // Safety: the drop above cancelled the transaction. The kernel clears + // its transaction slot on every path out of the cancel syscall, so it + // no longer points into RECV_BUF. + txn.start(&SEND_BUF, unsafe { recv_buf() })?; + txn.cancel()?; + Ok(()) +} + +/// A second start while one is pending fails locally and hands both +/// buffers back in the error. +fn test_double_start() -> Result<()> { + let mut txn = AsyncTransaction::new(IpcHandle::new(handle::IPC)); + // Safety: no other AsyncTransaction is live right now. + txn.start(&SEND_BUF, unsafe { recv_buf() })?; + + // RECV_BUF is still borrowed by the pending transaction, so the second + // start gets its own buffer. + // Safety: nothing else borrows RECV_BUF2. + let result = txn.start(&SEND_BUF, unsafe { recv_buf2() }); + + let outcome = match result { + Ok(()) => { + pw_log::error!("double start: second start() succeeded"); + Err(Error::Internal) + } + Err(e) if e.error != Error::FailedPrecondition => { + pw_log::error!("double start: status code {}", e.error as u32); + Err(Error::Internal) + } + Err(e) => { + let send_returned = core::ptr::eq(e.buffers.send.as_ptr(), SEND_BUF.as_ptr()); + let recv_returned = core::ptr::eq( + e.buffers.recv.as_ptr(), + core::ptr::addr_of!(RECV_BUF2).cast(), + ); + if send_returned && recv_returned { + Ok(()) + } else { + pw_log::error!("double start: buffers not returned"); + Err(Error::Internal) + } + } + }; + + txn.cancel()?; + outcome +} + +/// `try_recv` before the handler responds reports Unavailable and keeps the +/// transaction pending. The handler parks on this request and raises USER +/// to say so, so the observation is not a race: by the time USER arrives +/// the handler has read the request and is not going to respond until +/// released. +fn test_try_recv_before_response() -> Result<()> { + let ipc = IpcHandle::new(handle::IPC); + + // The handler lowers the parked signal before it responds, so USER is + // clear on entry. If it is still set, it leaked from an earlier round + // and the wait below would return without the handler having parked. + // A deadline of now makes this a poll rather than a wait. + if syscall::object_wait(ipc.as_raw(), Signals::USER, SystemClock::now()).is_ok() { + pw_log::error!("try_recv early: stale parked signal"); + return Err(Error::Internal); + } + + let mut txn = AsyncTransaction::new(ipc); + // Safety: no other AsyncTransaction is live right now. + txn.start(&SEND_GATED, unsafe { recv_buf() })?; + + // Wait for the handler to say it is parked. + syscall::object_wait(txn.as_raw(), Signals::USER, Instant::MAX)?; + + match txn.try_recv() { + Err(Error::Unavailable) => {} + Ok(_) => { + pw_log::error!("try_recv early: completed before the handler responded"); + return Err(Error::Internal); + } + Err(e) => { + pw_log::error!("try_recv early: status code {}", e as u32); + return Err(Error::Internal); + } + } + if !txn.is_pending() { + pw_log::error!("try_recv early: transaction no longer pending"); + return Err(Error::Internal); + } + + // Release the handler, then complete as usual. + ipc.set_peer_user_signal(true)?; + syscall::object_wait(txn.as_raw(), Signals::READABLE, Instant::MAX)?; + let completion = txn.try_recv()?; + ipc.set_peer_user_signal(false)?; + + if completion.len != 1 || completion.buffers.recv[0] != 0x41 { + pw_log::error!("try_recv early: unexpected response"); + return Err(Error::Internal); + } + Ok(()) +} + +/// How often the whole sequence runs. Every case leaves the channel idle +/// and both USER signals lowered, so a repeat that fails means state leaked +/// from the pass before it. +const ROUNDS: u32 = 50; + +fn run_all() -> Result<()> { + for round in 0..ROUNDS { + let ret = test_blocking_transact() + .and_then(|_| test_async_cancel()) + .and_then(|_| test_async_roundtrip()) + .and_then(|_| test_drop_cancels()) + .and_then(|_| test_double_start()) + .and_then(|_| test_try_recv_before_response()); + + if ret.is_err() { + pw_log::error!("failed in round {}", round as u32); + return ret; + } + } + Ok(()) +} + #[entry] fn entry() { - let ret = test_blocking_transact() - .and_then(|_| test_async_cancel()) - .and_then(|_| test_async_roundtrip()); + let ret = run_all(); match &ret { Ok(()) => pw_log::info!("All test cases PASSED"),