From 9f59c422941eed62bf016686e7fe932ce7cc9fae Mon Sep 17 00:00:00 2001 From: Christina Quast Date: Thu, 3 Sep 2026 13:37:22 +0200 Subject: [PATCH 1/4] pldm: Count update-request notifications in the host test The test drove the orchestrator's UpdateRequestLatch, which put both orchestrator crates in a PLDM test's deps. A local counting sink asserts the same thing: an accepted RequestUpdate notifies once, a rejected one does not, and nothing later does. Absolute counts rather than a drain, so each assertion says how many notifications have happened at that point in the session. --- services/pldm/BUILD.bazel | 2 - services/pldm/tests/firmware_update_host.rs | 60 +++++++++++++-------- 2 files changed, 39 insertions(+), 23 deletions(-) diff --git a/services/pldm/BUILD.bazel b/services/pldm/BUILD.bazel index d420d0b9..6f9ddbc4 100644 --- a/services/pldm/BUILD.bazel +++ b/services/pldm/BUILD.bazel @@ -55,8 +55,6 @@ rust_test( ":pldm_service", "//services/mctp/api:mctp_api", "//services/mctp/server:mctp_server_lib", - "//services/orchestrator/adapters/pldm:orchestrator_pldm_adapter", - "//services/orchestrator/sm:orchestrator_sm", "@rust_crates//:mctp", "@rust_crates//:mctp-lib", "@rust_crates//:pldm-common", diff --git a/services/pldm/tests/firmware_update_host.rs b/services/pldm/tests/firmware_update_host.rs index 1e8d8752..d1dc4027 100644 --- a/services/pldm/tests/firmware_update_host.rs +++ b/services/pldm/tests/firmware_update_host.rs @@ -26,9 +26,9 @@ use mctp::Eid; use mctp_lib::Sender; use openprot_mctp_api::Handle; use openprot_mctp_server::Server; -use openprot_orchestrator_pldm_adapter::UpdateRequestLatch; -use openprot_orchestrator_sm::Event; -use openprot_pldm_service::firmware_device::{FirmwareDevice, RunTerminusResult}; +use openprot_pldm_service::firmware_device::{ + FdEvent, FdEventSink, FirmwareDevice, RunTerminusResult, +}; use openprot_pldm_service::{MctpPldmTransport, PldmServiceError}; use pldm_common::codec::{PldmCodec, PldmCodecWithLifetime}; use pldm_common::message::firmware_update::apply_complete::{ApplyCompleteResponse, ApplyResult}; @@ -179,6 +179,29 @@ impl FdOps for MockFdOps { // Helpers // --------------------------------------------------------------------------- +/// Counts `FdEvent::UpdateRequested` out of `run_terminus`. +/// +/// Implemented for the reference so `run_terminus` can take the sink while the +/// assertions still read the same counter. +#[derive(Default)] +struct UpdateRequestCounter { + notifies: Cell, +} + +impl UpdateRequestCounter { + fn count(&self) -> u32 { + self.notifies.get() + } +} + +impl FdEventSink for &UpdateRequestCounter { + fn notify(&mut self, event: FdEvent) { + if matches!(event, FdEvent::UpdateRequested) { + self.notifies.set(self.notifies.get() + 1); + } + } +} + /// Build a fixed-size PLDM firmware version string. fn fw_string(s: &str) -> PldmFirmwareString { let bytes = s.as_bytes(); @@ -315,9 +338,9 @@ fn firmware_update_full_flow_via_requester() { )); let fd_buf = RefCell::new([0u8; 1024]); - // Orchestrator-facing latch: `run_terminus` marks it on each accepted - // RequestUpdate; the assertions below drain it as `Event::UpdateRequest`. - let update_events = RefCell::new(UpdateRequestLatch::new()); + // Counts what `run_terminus` reports to the orchestrator, so the + // assertions below can pin down which commands notify and which do not. + let update_events = UpdateRequestCounter::default(); // Run one full UA->FD->UA command round-trip and return the PLDM response // payload (without the MCTP framing byte). @@ -339,7 +362,7 @@ fn firmware_update_full_flow_via_requester() { &mut fd_buf.borrow_mut()[..], TIMEOUT_MILLIS, TIMEOUT_MILLIS, - &mut *update_events.borrow_mut(), + &mut &update_events, ) { RunTerminusResult::Completed => {} RunTerminusResult::StoppedByError(PldmServiceError::Mctp(e)) if e.is_timeout() => {} @@ -380,14 +403,9 @@ fn firmware_update_full_flow_via_requester() { "RequestUpdate completion code should be success" ); assert_eq!( - update_events.borrow_mut().take(), - Some(Event::UpdateRequest), - "accepted RequestUpdate should latch exactly one orchestrator event" - ); - assert_eq!( - update_events.borrow_mut().take(), - None, - "the latch must not re-fire once drained" + update_events.count(), + 1, + "accepted RequestUpdate should notify the orchestrator exactly once" ); // ---- Duplicate RequestUpdate: rejected, must not latch an event ---- @@ -411,9 +429,9 @@ fn firmware_update_full_flow_via_requester() { "second RequestUpdate should be rejected while in update mode" ); assert_eq!( - update_events.borrow_mut().take(), - None, - "a rejected RequestUpdate must not latch an orchestrator event" + update_events.count(), + 1, + "a rejected RequestUpdate must not notify the orchestrator" ); // ---- PassComponentTable (Start+End): move to ReadyXfer ---- @@ -496,9 +514,9 @@ fn firmware_update_full_flow_via_requester() { assert!(fd_ops.verified.get(), "firmware should have been verified"); assert!(fd_ops.applied.get(), "firmware should have been applied"); assert_eq!( - update_events.borrow_mut().take(), - None, - "no command after the accepted RequestUpdate should latch an event" + update_events.count(), + 1, + "no command after the accepted RequestUpdate should notify again" ); println!( From d1772629210749ef4ea2f6cfccd9d5306c152b76 Mon Sep 17 00:00:00 2001 From: Christina Quast Date: Thu, 3 Sep 2026 19:59:05 +0200 Subject: [PATCH 2/4] pldm: Add the pldm-notify wire Wire protocol for the PLDM-to-orchestrator notification channel. PLDM initiates, the orchestrator handles. One opcode today (UpdateRequested), two responses (Accepted, Rejected) so the orchestrator can veto before the transfer starts. Header is [op/code:1B][len:1B][reserved:2B] each way. Both enums are non_exhaustive because the two sides are separate processes that may be built from different revisions. Builds and tests on the host, no kernel dependency. Assisted-by: Claude --- services/pldm/notify-api/BUILD.bazel | 21 ++ services/pldm/notify-api/README.md | 30 +++ services/pldm/notify-api/src/lib.rs | 309 +++++++++++++++++++++++++++ 3 files changed, 360 insertions(+) create mode 100644 services/pldm/notify-api/BUILD.bazel create mode 100644 services/pldm/notify-api/README.md create mode 100644 services/pldm/notify-api/src/lib.rs diff --git a/services/pldm/notify-api/BUILD.bazel b/services/pldm/notify-api/BUILD.bazel new file mode 100644 index 00000000..dc54f353 --- /dev/null +++ b/services/pldm/notify-api/BUILD.bazel @@ -0,0 +1,21 @@ +# Licensed under the Apache-2.0 license +# SPDX-License-Identifier: Apache-2.0 + +load("@rules_rust//rust:defs.bzl", "rust_library", "rust_test") + +# Contract only: both processes decode this, so it carries no transport and no +# state and builds on the host. Untagged, so a kernel binary links it as-is. +rust_library( + name = "pldm_notify_api", + srcs = ["src/lib.rs"], + crate_name = "openprot_pldm_notify_api", + edition = "2024", + visibility = ["//visibility:public"], + deps = ["@rust_crates//:zerocopy"], +) + +# Host wire tests: build on the host platform, no kernel/QEMU. +rust_test( + name = "pldm_notify_api_test", + crate = ":pldm_notify_api", +) diff --git a/services/pldm/notify-api/README.md b/services/pldm/notify-api/README.md new file mode 100644 index 00000000..b25a13a9 --- /dev/null +++ b/services/pldm/notify-api/README.md @@ -0,0 +1,30 @@ +# pldm-notify-api + +The wire the PLDM service uses to tell the orchestrator that something has +happened. Contract only, no transport and no state, so both processes depend on +it and it builds and tests on the host. + +PLDM initiates and the orchestrator handles, because PLDM is where these things +are known and the orchestrator's loop already waits on several objects. + +One op today, `NotifyOp::UpdateRequested`, and two answers, `Response::Accepted` +and `Response::Rejected`. All carry no payload, so a frame is 4 bytes each way: +`[op][len][reserved:2]` in, `[code][len][reserved:2]` back. + +To add a notification, give it the next `NotifyOp` discriminant. If it carries +data, put the length in `len` and raise `MAX_REQUEST_SIZE`; `len` is in the +header now so that does not change the frame format. The opcode space is flat +and nothing in it is specific to firmware update, so any PLDM type can take a +range. Reserved fields are zero and the decoder rejects a frame that sets them, +so they can be given a meaning later. + +`Response::Accepted` lets the transfer proceed. `Response::Rejected` tells the +PLDM service that the orchestrator will not act on this request (already +updating, recovering, locked, or policy). The verdict is the caller's, not the +adapter's. + +The orchestrator-side handler is `services/orchestrator/adapters/pldm`. + +Run the tests with: + + bazel test //services/pldm/notify-api:pldm_notify_api_test diff --git a/services/pldm/notify-api/src/lib.rs b/services/pldm/notify-api/src/lib.rs new file mode 100644 index 00000000..cabb32fe --- /dev/null +++ b/services/pldm/notify-api/src/lib.rs @@ -0,0 +1,309 @@ +// Licensed under the Apache-2.0 license +// SPDX-License-Identifier: Apache-2.0 + +//! The wire the PLDM service uses to tell the orchestrator that something has +//! happened. +//! +//! PLDM initiates and the orchestrator handles, because PLDM is where these +//! things are known and the orchestrator's loop already waits on several +//! objects. Contract only, no transport and no state, so both processes depend +//! on it and it builds and tests on the host. +//! +//! ```text +//! Request (4 bytes + payload): Response (4 bytes + payload): +//! ┌─────┬─────┬──────────┐ ┌──────┬─────┬──────────┐ +//! │ op │ len │ reserved │ │ code │ len │ reserved │ +//! │ 1B │ 1B │ 2B │ │ 1B │ 1B │ 2B │ +//! └─────┴─────┴──────────┘ └──────┴─────┴──────────┘ +//! ``` +//! +//! One op today, [`NotifyOp::UpdateRequested`], and two answers, +//! [`Response::Accepted`] and [`Response::Rejected`]. All carry no payload, +//! so a frame is 4 bytes each way. +//! +//! Adding a notification: give it the next [`NotifyOp`] discriminant, and if it +//! carries data, put the length in `len` and raise [`MAX_REQUEST_SIZE`]. `len` +//! exists now so that does not change the frame format. Nothing here is +//! specific to firmware update; the opcode space is flat and any PLDM type can +//! take a range of it. Both enums are `#[non_exhaustive]` because the two sides +//! are separate processes that may be built from different revisions. +//! +//! Coalescing and latching live on the sender side, not here. The firmware +//! device fires `UpdateRequested` only on the `!was_update_mode && +//! is_update_mode()` edge, so a second `RequestUpdate` while already in update +//! mode never reaches the channel. +//! +//! The request side is another process and is treated as untrusted: decoding +//! validates length, opcode, reserved fields, and that `len` is zero on an op +//! that carries nothing. Nothing here panics. + +#![cfg_attr(not(test), no_std)] +#![forbid(unsafe_code)] +#![warn(missing_docs)] + +use zerocopy::{FromBytes, Immutable, IntoBytes, KnownLayout}; + +/// Largest payload any op carries. Zero while every op is a bare +/// notification; raise it with the first op that carries data. +pub const MAX_PAYLOAD: usize = 0; + +/// Request buffer size a handler must provide. +pub const MAX_REQUEST_SIZE: usize = RequestHeader::SIZE + MAX_PAYLOAD; + +/// Response buffer size an initiator must provide. +pub const MAX_RESPONSE_SIZE: usize = ResponseHeader::SIZE; + +/// Why a buffer did not decode. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum WireError { + /// Shorter than the message the header declares. + Truncated, + /// The output buffer cannot hold the encoding. + BufferTooSmall, + /// The op byte names no operation this build knows. + InvalidOpcode(u8), + /// A field carries a value this op does not define: a reserved field set, + /// `len` non-zero on an op that carries nothing, or a response code that + /// names no answer. + InvalidField, +} + +impl core::fmt::Display for WireError { + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { + f.write_str(match self { + WireError::Truncated => "buffer shorter than the message", + WireError::BufferTooSmall => "buffer too small for the message", + WireError::InvalidOpcode(_) => "unknown operation code", + WireError::InvalidField => "field value not defined for this operation", + }) + } +} + +impl core::error::Error for WireError {} + +/// What the PLDM service can report. One per thing that has happened, whatever +/// part of PLDM it came from. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +#[non_exhaustive] +#[repr(u8)] +pub enum NotifyOp { + /// A UA sent a `RequestUpdate`. The orchestrator's answer decides whether + /// the FD accepts. + UpdateRequested = 0, +} + +impl TryFrom for NotifyOp { + type Error = WireError; + + fn try_from(value: u8) -> Result { + match value { + 0 => Ok(NotifyOp::UpdateRequested), + other => Err(WireError::InvalidOpcode(other)), + } + } +} + +/// One notification from the PLDM service to the orchestrator. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +#[non_exhaustive] +pub enum Request { + /// [`NotifyOp::UpdateRequested`]. Carries nothing: the request itself is + /// the whole message. + UpdateRequested, +} + +impl Request { + /// The op this request encodes as. + pub fn op(&self) -> NotifyOp { + match self { + Request::UpdateRequested => NotifyOp::UpdateRequested, + } + } + + /// Encodes into `buf`, returning the encoded length. + pub fn encode(&self, buf: &mut [u8]) -> Result { + let header = RequestHeader { + op_code: self.op() as u8, + len: 0, + reserved: 0, + }; + let out = buf + .get_mut(..RequestHeader::SIZE) + .ok_or(WireError::BufferTooSmall)?; + out.copy_from_slice(header.as_bytes()); + Ok(RequestHeader::SIZE) + } + + /// Decodes one notification out of a handler's read buffer. + pub fn decode(buf: &[u8]) -> Result { + let head = buf.get(..RequestHeader::SIZE).ok_or(WireError::Truncated)?; + let header = RequestHeader::read_from_bytes(head).map_err(|_| WireError::Truncated)?; + if header.reserved != 0 { + return Err(WireError::InvalidField); + } + match NotifyOp::try_from(header.op_code)? { + NotifyOp::UpdateRequested => { + if header.len != 0 { + return Err(WireError::InvalidField); + } + Ok(Request::UpdateRequested) + } + } + } +} + +/// What the orchestrator answers. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +#[non_exhaustive] +#[repr(u8)] +pub enum Response { + /// The orchestrator accepts the notification and will act on it. + Accepted = 0, + /// The orchestrator rejects the request (e.g., already updating, + /// recovering, locked, or policy). + Rejected = 1, +} + +impl Response { + /// Encodes into `buf`, returning the encoded length. + pub fn encode(&self, buf: &mut [u8]) -> Result { + let header = ResponseHeader { + code: *self as u8, + len: 0, + reserved: 0, + }; + let out = buf + .get_mut(..ResponseHeader::SIZE) + .ok_or(WireError::BufferTooSmall)?; + out.copy_from_slice(header.as_bytes()); + Ok(ResponseHeader::SIZE) + } + + /// Decodes the orchestrator's answer. + pub fn decode(buf: &[u8]) -> Result { + let head = buf + .get(..ResponseHeader::SIZE) + .ok_or(WireError::Truncated)?; + let header = ResponseHeader::read_from_bytes(head).map_err(|_| WireError::Truncated)?; + if header.reserved != 0 || header.len != 0 { + return Err(WireError::InvalidField); + } + match header.code { + 0 => Ok(Response::Accepted), + 1 => Ok(Response::Rejected), + _ => Err(WireError::InvalidField), + } + } +} + +/// The 4-byte request header. Fields are private because [`Request::decode`] +/// is what validates them. +#[repr(C, packed)] +#[derive(Debug, Clone, Copy, FromBytes, IntoBytes, Immutable, KnownLayout)] +struct RequestHeader { + op_code: u8, + len: u8, + reserved: u16, +} + +impl RequestHeader { + const SIZE: usize = core::mem::size_of::(); +} + +/// The 4-byte response header. +#[repr(C, packed)] +#[derive(Debug, Clone, Copy, FromBytes, IntoBytes, Immutable, KnownLayout)] +struct ResponseHeader { + code: u8, + len: u8, + reserved: u16, +} + +impl ResponseHeader { + const SIZE: usize = core::mem::size_of::(); +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn every_notification_round_trips() { + let request = Request::UpdateRequested; + let mut buf = [0u8; MAX_REQUEST_SIZE]; + let len = request.encode(&mut buf).unwrap(); + assert_eq!(Request::decode(&buf[..len]), Ok(request)); + } + + #[test] + fn every_answer_round_trips() { + for answer in [Response::Accepted, Response::Rejected] { + let mut buf = [0u8; MAX_RESPONSE_SIZE]; + let len = answer.encode(&mut buf).unwrap(); + assert_eq!(Response::decode(&buf[..len]), Ok(answer)); + } + } + + #[test] + fn a_short_buffer_is_truncated() { + assert_eq!(Request::decode(&[0, 0, 0]), Err(WireError::Truncated)); + assert_eq!(Response::decode(&[0, 0, 0]), Err(WireError::Truncated)); + } + + #[test] + fn an_unknown_opcode_is_refused() { + assert_eq!( + Request::decode(&[0xFF, 0, 0, 0]), + Err(WireError::InvalidOpcode(0xFF)) + ); + } + + #[test] + fn a_set_reserved_field_is_refused() { + assert_eq!( + Request::decode(&[NotifyOp::UpdateRequested as u8, 0, 1, 0]), + Err(WireError::InvalidField) + ); + assert_eq!( + Response::decode(&[Response::Accepted as u8, 0, 0, 1]), + Err(WireError::InvalidField) + ); + } + + /// Every op carries nothing today, so a length is a malformed frame rather + /// than a payload this build should skip. + #[test] + fn a_length_on_an_op_that_carries_nothing_is_refused() { + assert_eq!( + Request::decode(&[NotifyOp::UpdateRequested as u8, 1, 0, 0]), + Err(WireError::InvalidField) + ); + assert_eq!( + Response::decode(&[Response::Accepted as u8, 1, 0, 0]), + Err(WireError::InvalidField) + ); + } + + #[test] + fn an_unknown_answer_is_refused() { + for code in [2u8, 0xFF] { + assert_eq!( + Response::decode(&[code, 0, 0, 0]), + Err(WireError::InvalidField) + ); + } + } + + #[test] + fn an_output_buffer_too_small_is_refused() { + let mut buf = [0u8; RequestHeader::SIZE - 1]; + assert_eq!( + Request::UpdateRequested.encode(&mut buf), + Err(WireError::BufferTooSmall) + ); + assert_eq!( + Response::Accepted.encode(&mut buf), + Err(WireError::BufferTooSmall) + ); + } +} From 13593a1d038f9b4f1013241a7f4d42dacaf04ce6 Mon Sep 17 00:00:00 2001 From: Christina Quast Date: Thu, 3 Sep 2026 19:59:13 +0200 Subject: [PATCH 3/4] orchestrator: Handle a pldm-notify notification dispatch() decodes one notification from the PLDM service, encodes an accept or reject based on the caller's verdict, and returns the notification in orchestrator-side vocabulary. The caller decides the verdict; the adapter translates both ways. Returns Notification, not a state-machine Event: the notification arrives before the FD has accepted, so there is nothing to authenticate yet. The state machine hears about the update later, at complete-time on the intake seam. Depends on the wire crate only, not on orchestrator-sm. Assisted-by: Claude --- .../orchestrator/adapters/pldm/BUILD.bazel | 3 +- .../orchestrator/adapters/pldm/src/lib.rs | 162 +++++++++++------- 2 files changed, 104 insertions(+), 61 deletions(-) diff --git a/services/orchestrator/adapters/pldm/BUILD.bazel b/services/orchestrator/adapters/pldm/BUILD.bazel index ce7c90b9..f9db96e6 100644 --- a/services/orchestrator/adapters/pldm/BUILD.bazel +++ b/services/orchestrator/adapters/pldm/BUILD.bazel @@ -12,8 +12,7 @@ rust_library( edition = "2024", visibility = ["//visibility:public"], deps = [ - "//services/orchestrator/sm:orchestrator_sm", - "//services/pldm:pldm_service", + "//services/pldm/notify-api:pldm_notify_api", ], ) diff --git a/services/orchestrator/adapters/pldm/src/lib.rs b/services/orchestrator/adapters/pldm/src/lib.rs index 3b830451..afc334e1 100644 --- a/services/orchestrator/adapters/pldm/src/lib.rs +++ b/services/orchestrator/adapters/pldm/src/lib.rs @@ -1,88 +1,132 @@ // Licensed under the Apache-2.0 license // SPDX-License-Identifier: Apache-2.0 -//! PLDM-backed adapter for the Boot Orchestrator's update-request input. +//! Orchestrator side of the pldm-notify wire: decodes one notification from the +//! PLDM service and says what it means in the orchestrator's terms. //! -//! [`UpdateRequestLatch`] binds the PLDM firmware-device service's -//! [`FdEventSink`] seam to the orchestrator's -//! [`Event::UpdateRequest`]: the PLDM run loop notifies the latch when the -//! Update Agent's `RequestUpdate` is accepted, and the orchestrator run loop -//! drains it with [`take`](UpdateRequestLatch::take). This crate depends on -//! both stacks by design — the PLDM service stays orchestrator-free and the -//! orchestrator stays transport-free, the same rule that keeps HAL adapters -//! out of `orchestrator-capabilities`. +//! Nothing here reaches the state machine. The notification arrives before the +//! FD has accepted the UA's request, so there is nothing to authenticate yet; +//! the machine hears about an update later, when the candidate is complete. +//! +//! [`dispatch`] is pure, no IPC and no globals, so it tests on the host. The +//! kernel wait-and-respond loop is elsewhere, and it answers before it acts on +//! the notification, since `channel_respond` has to be prompt. +//! +//! Depends on the wire crate only, never on the PLDM stack and never on +//! orchestrator-sm, the same rule the orchestrator's HAL adapters follow. #![cfg_attr(not(test), no_std)] #![forbid(unsafe_code)] #![warn(missing_docs)] -use openprot_orchestrator_sm::Event; -use openprot_pldm_service::firmware_device::{FdEvent, FdEventSink}; +use openprot_pldm_notify_api::{Request, Response}; -/// Latches an accepted PLDM `RequestUpdate` until the orchestrator run loop -/// drains it as [`Event::UpdateRequest`]. +/// What the PLDM service reported, in the orchestrator's words. /// -/// A `bool` latch, not a counter: the FD rejects a second `RequestUpdate` -/// while an update is in progress (`ALREADY_IN_UPDATE_MODE`), so at most one -/// accepted request can be outstanding per update cycle. Should a completed -/// or cancelled cycle admit a new `RequestUpdate` before the previous latch -/// is drained, the two coalesce into one [`Event::UpdateRequest`] — which is -/// what the state machine would do anyway (an update already being handled -/// defers further requests). -#[derive(Default)] -pub struct UpdateRequestLatch { - pending: bool, -} - -impl UpdateRequestLatch { - /// A latch with nothing pending. - pub const fn new() -> Self { - Self { pending: false } - } - - /// Drain the latch: [`Event::UpdateRequest`] if a `RequestUpdate` was - /// accepted since the last call, else `None`. - pub fn take(&mut self) -> Option { - self.pending.then(|| { - self.pending = false; - Event::UpdateRequest - }) - } +/// Translating here is the point of this crate: [`NotifyOp`] is PLDM's +/// vocabulary across a process boundary, and letting it reach the run loop is +/// the coupling the adapter exists to prevent. +/// +/// [`NotifyOp`]: openprot_pldm_notify_api::NotifyOp +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +#[non_exhaustive] +pub enum Notification { + /// A UA sent a `RequestUpdate`. The orchestrator's verdict decides whether + /// the FD accepts; no firmware bytes have moved yet. + UpdateRequested, } -/// Latches [`FdEvent::UpdateRequested`]; other FD lifecycle events have no -/// orchestrator mapping yet and are dropped here by design. -impl FdEventSink for UpdateRequestLatch { - fn notify(&mut self, event: FdEvent) { - if matches!(event, FdEvent::UpdateRequested) { - self.pending = true; - } +/// Decodes one notification, encodes an accept or reject, and returns what +/// was reported. +/// +/// `accept` is the caller's verdict: true to accept the request, false to +/// reject it. The Notification is returned either way so the caller knows +/// what was requested (for logging on reject, watchdog arming on accept). +/// +/// Returns how many bytes of `response` are the answer. `response` must be at +/// least [`MAX_RESPONSE_SIZE`](openprot_pldm_notify_api::MAX_RESPONSE_SIZE) +/// bytes; a shorter one gets nothing written and a return of 0, which the +/// caller must not put on the wire. +/// +/// Never panics. A frame that does not decode gets nothing written, a return +/// of 0, and nothing reported. +pub fn dispatch( + request: &[u8], + response: &mut [u8], + accept: bool, +) -> (usize, Option) { + let notification = match Request::decode(request) { + Ok(Request::UpdateRequested) => Notification::UpdateRequested, + // non_exhaustive: a future op this build doesn't know, or a bad frame. + // No answer, so the UA's transact times out and retries. + Ok(_) | Err(_) => return (0, None), + }; + let answer = if accept { + Response::Accepted + } else { + Response::Rejected + }; + match answer.encode(response) { + Ok(len) => (len, Some(notification)), + Err(_) => (0, None), } } #[cfg(test)] mod tests { use super::*; + use openprot_pldm_notify_api::{NotifyOp, MAX_REQUEST_SIZE, MAX_RESPONSE_SIZE}; + + fn round_trip(request: Request, accept: bool) -> (Option, Option) { + let mut out = [0u8; MAX_REQUEST_SIZE]; + let len = request.encode(&mut out).unwrap(); + let mut back = [0u8; MAX_RESPONSE_SIZE]; + let (answer_len, notification) = dispatch(&out[..len], &mut back, accept); + (notification, Response::decode(&back[..answer_len]).ok()) + } #[test] - fn empty_latch_yields_nothing() { - assert_eq!(UpdateRequestLatch::new().take(), None); + fn an_update_request_that_the_caller_accepts_is_reported_and_confirmed() { + assert_eq!( + round_trip(Request::UpdateRequested, true), + ( + Some(Notification::UpdateRequested), + Some(Response::Accepted) + ) + ); } #[test] - fn accepted_request_yields_one_event() { - let mut latch = UpdateRequestLatch::new(); - latch.notify(FdEvent::UpdateRequested); - assert_eq!(latch.take(), Some(Event::UpdateRequest)); - assert_eq!(latch.take(), None, "a drained latch must not re-fire"); + fn a_rejected_request_update_is_still_reported() { + assert_eq!( + round_trip(Request::UpdateRequested, false), + ( + Some(Notification::UpdateRequested), + Some(Response::Rejected) + ) + ); + } + + #[test] + fn a_frame_that_does_not_decode_gets_no_answer_and_reports_nothing() { + let mut back = [0xFFu8; MAX_RESPONSE_SIZE]; + for bad in [ + &[0u8][..], + &[0xFF, 0, 0, 0][..], + &[NotifyOp::UpdateRequested as u8, 0, 1, 0][..], + &[NotifyOp::UpdateRequested as u8, 1, 0, 0][..], + ] { + assert_eq!(dispatch(bad, &mut back, true), (0, None)); + assert!(back.iter().all(|&b| b == 0xFF)); + } } #[test] - fn undrained_notifications_coalesce() { - let mut latch = UpdateRequestLatch::new(); - latch.notify(FdEvent::UpdateRequested); - latch.notify(FdEvent::UpdateRequested); - assert_eq!(latch.take(), Some(Event::UpdateRequest)); - assert_eq!(latch.take(), None); + fn a_response_buffer_below_the_minimum_gets_nothing() { + let mut out = [0u8; MAX_REQUEST_SIZE]; + let len = Request::UpdateRequested.encode(&mut out).unwrap(); + let mut back = [0xFFu8; MAX_RESPONSE_SIZE - 1]; + assert_eq!(dispatch(&out[..len], &mut back, true), (0, None)); + assert!(back.iter().all(|&b| b == 0xFF)); } } From b091e1905da8763274bc8ca62a568a289fa74607 Mon Sep 17 00:00:00 2001 From: Christina Quast Date: Thu, 3 Sep 2026 19:59:20 +0200 Subject: [PATCH 4/4] target/ast10x0: Wire the pldm-notify channel Two pw_kernel processes on a real channel under QEMU. The PLDM side sends Request::UpdateRequested via channel_transact, the orchestrator side waits, reads, runs dispatch (always accepting for the PoC), and answers. The PLDM side checks for Response::Accepted and shuts down with PASS. Asserts Notification::UpdateRequested on the handler side, consistent with the adapter returning orchestrator-side vocabulary rather than a state-machine event. Assisted-by: Claude --- target/ast10x0/tests/pldm_notify/BUILD.bazel | 124 ++++++++++++++++++ target/ast10x0/tests/pldm_notify/orch_main.rs | 66 ++++++++++ target/ast10x0/tests/pldm_notify/pldm_main.rs | 77 +++++++++++ target/ast10x0/tests/pldm_notify/system.json5 | 77 +++++++++++ target/ast10x0/tests/pldm_notify/target.rs | 40 ++++++ 5 files changed, 384 insertions(+) create mode 100644 target/ast10x0/tests/pldm_notify/BUILD.bazel create mode 100644 target/ast10x0/tests/pldm_notify/orch_main.rs create mode 100644 target/ast10x0/tests/pldm_notify/pldm_main.rs create mode 100644 target/ast10x0/tests/pldm_notify/system.json5 create mode 100644 target/ast10x0/tests/pldm_notify/target.rs diff --git a/target/ast10x0/tests/pldm_notify/BUILD.bazel b/target/ast10x0/tests/pldm_notify/BUILD.bazel new file mode 100644 index 00000000..eb44fa28 --- /dev/null +++ b/target/ast10x0/tests/pldm_notify/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", + ], +) + +# ── Orchestrator app ─────────────────────────────────────────────────────────── +# Handler side: the real dispatch, no state machine and no wait group yet. + +rust_app( + name = "orchestrator", + srcs = ["orch_main.rs"], + codegen_crate_name = "app_orchestrator", + edition = "2024", + system_config = ":system_config", + tags = ["kernel"], + target_compatible_with = TARGET_COMPATIBLE_WITH, + deps = [ + "//services/orchestrator/adapters/pldm:orchestrator_pldm_adapter", + "//services/pldm/notify-api:pldm_notify_api", + "@pigweed//pw_kernel/userspace", + "@pigweed//pw_log/rust:pw_log", + "@pigweed//pw_status/rust:pw_status", + ], +) + +# ── PLDM app ─────────────────────────────────────────────────────────────────── +# Initiator side: sends one notification and reports the round trip. + +rust_app( + name = "pldm", + srcs = ["pldm_main.rs"], + codegen_crate_name = "app_pldm", + edition = "2024", + system_config = ":system_config", + tags = ["kernel"], + target_compatible_with = TARGET_COMPATIBLE_WITH, + deps = [ + "//services/pldm/notify-api:pldm_notify_api", + "@pigweed//pw_kernel/userspace", + "@pigweed//pw_log/rust:pw_log", + "@pigweed//pw_status/rust:pw_status", + ], +) + +# ── System image ─────────────────────────────────────────────────────────────── + +system_image( + name = "pldm_notify_image", + apps = [ + ":orchestrator", + ":pldm", + ], + 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/pldm_notify:pldm_notify_qemu_test + +system_image_test( + name = "pldm_notify_qemu_test", + image = ":pldm_notify_image", + tags = ["qemu_only"], + target_compatible_with = TARGET_COMPATIBLE_WITH, +) + +rust_binary_no_panics_test( + name = "no_panics_test", + binary = ":pldm_notify_image", + tags = ["kernel"], +) diff --git a/target/ast10x0/tests/pldm_notify/orch_main.rs b/target/ast10x0/tests/pldm_notify/orch_main.rs new file mode 100644 index 00000000..dac869d2 --- /dev/null +++ b/target/ast10x0/tests/pldm_notify/orch_main.rs @@ -0,0 +1,66 @@ +// Licensed under the Apache-2.0 license +// SPDX-License-Identifier: Apache-2.0 + +//! Handler side of the PLDM notification channel test. +//! +//! Waits for a notification, runs the real `dispatch`, and answers. A +//! notification that does not decode to `Notification::UpdateRequested` fails +//! the test here; the pldm process reports the pass once its round trip +//! completes. +//! +//! One object to wait on, so it waits on it directly. The wait group arrives +//! with the run loop, when the boot signal and the intake seam are also +//! members. + +#![no_main] +#![no_std] + +use openprot_orchestrator_pldm_adapter::{dispatch, Notification}; +use openprot_pldm_notify_api::{MAX_REQUEST_SIZE, MAX_RESPONSE_SIZE}; +use pw_status::Error; +use userspace::syscall::Signals; +use userspace::time::Instant; +use userspace::{entry, syscall}; + +use app_orchestrator::handle; + +#[entry] +fn entry() { + let mut req = [0u8; MAX_REQUEST_SIZE]; + let mut resp = [0u8; MAX_RESPONSE_SIZE]; + + loop { + // Nothing else wakes this thread, so a failed wait cannot be retried + // into a working one: report it rather than spinning. + if syscall::object_wait(handle::PLDM_NOTIFY, Signals::READABLE, Instant::MAX).is_err() { + pw_log::error!("waiting on the pldm-notify channel failed"); + let _ = syscall::debug_shutdown(Err(Error::Internal)); + } + + let len = match syscall::channel_read(handle::PLDM_NOTIFY, 0, &mut req) { + Ok(n) => n, + Err(_) => continue, + }; + + // PoC: always accept. Veto logic comes with the run loop. + let (answer_len, notification) = dispatch(&req[..len], &mut resp, true); + if notification != Some(Notification::UpdateRequested) { + pw_log::error!("the notification did not decode to UpdateRequested"); + let _ = syscall::debug_shutdown(Err(Error::Internal)); + } + + if answer_len == 0 { + pw_log::error!("dispatch returned no answer"); + let _ = syscall::debug_shutdown(Err(Error::Internal)); + } + + // Answer before acting on the notification: channel_respond has to be + // prompt, and whatever the run loop does with it can take real time. + let _ = syscall::channel_respond(handle::PLDM_NOTIFY, &resp[..answer_len]); + } +} + +#[panic_handler] +fn panic(_info: &core::panic::PanicInfo) -> ! { + loop {} +} diff --git a/target/ast10x0/tests/pldm_notify/pldm_main.rs b/target/ast10x0/tests/pldm_notify/pldm_main.rs new file mode 100644 index 00000000..ca653899 --- /dev/null +++ b/target/ast10x0/tests/pldm_notify/pldm_main.rs @@ -0,0 +1,77 @@ +// Licensed under the Apache-2.0 license +// SPDX-License-Identifier: Apache-2.0 + +//! Initiator side of the PLDM notification channel test. +//! +//! Sends one `UpdateRequested` notification to the orchestrator process and +//! checks the answer, then reports the result: `debug_shutdown(Ok(()))` on a +//! clean round trip, `Err` otherwise. The kernel target turns that into the +//! UART sentinel. +//! +//! The firmware device is not here. Building `services/pldm` for ast10x0 is +//! separate work, and what this test covers is the channel. + +#![no_main] +#![no_std] + +use openprot_pldm_notify_api::{Request, Response, MAX_REQUEST_SIZE, MAX_RESPONSE_SIZE}; +use pw_status::Error; +use userspace::time::{Clock, Duration, Instant, SystemClock}; +use userspace::{entry, syscall}; + +use app_pldm::handle; + +/// A wedged orchestrator must not block the firmware device, so the transact +/// is bounded rather than waiting forever. +const ANSWER_WINDOW: Duration = Duration::from_millis(1000); + +#[entry] +fn entry() { + match notify_update_requested() { + Ok(()) => { + pw_log::info!("pldm-notify round trip PASSED"); + let _ = syscall::debug_shutdown(Ok(())); + } + Err(()) => { + let _ = syscall::debug_shutdown(Err(Error::Internal)); + } + } + #[expect(clippy::empty_loop)] + loop {} +} + +fn notify_update_requested() -> Result<(), ()> { + let mut out = [0u8; MAX_REQUEST_SIZE]; + let len = Request::UpdateRequested.encode(&mut out).map_err(|_| { + pw_log::error!("encoding the notification failed"); + })?; + + let deadline = SystemClock::now() + .checked_add_duration(ANSWER_WINDOW) + .unwrap_or(Instant::MAX); + + let mut back = [0u8; MAX_RESPONSE_SIZE]; + let answer_len = + syscall::channel_transact(handle::PLDM_NOTIFY, &out[..len], &mut back, deadline).map_err( + |_| { + pw_log::error!("the notification did not reach the orchestrator"); + }, + )?; + + match Response::decode(&back[..answer_len]) { + Ok(Response::Accepted) => Ok(()), + Ok(Response::Rejected) => { + pw_log::error!("the orchestrator rejected the request"); + Err(()) + } + _ => { + pw_log::error!("the orchestrator's answer did not decode"); + Err(()) + } + } +} + +#[panic_handler] +fn panic(_info: &core::panic::PanicInfo) -> ! { + loop {} +} diff --git a/target/ast10x0/tests/pldm_notify/system.json5 b/target/ast10x0/tests/pldm_notify/system.json5 new file mode 100644 index 00000000..92ce9f04 --- /dev/null +++ b/target/ast10x0/tests/pldm_notify/system.json5 @@ -0,0 +1,77 @@ +// Licensed under the Apache-2.0 license +// SPDX-License-Identifier: Apache-2.0 + +// AST10x0 PLDM notification channel system image. +// +// Two-process layout, the shape the eRoT will use: +// +// pldm: channel_initiator; sends one UpdateRequested notification +// orchestrator: channel_handler; decodes it and answers +// +// The pldm process reports pass/fail once the whole round trip is done, so +// PASS means the notification encoded, crossed the channel, decoded to +// Event::UpdateRequest, was answered, and the answer decoded. +// +// 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 - 0x000A0000 app RAM (128 KB total; 64 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: "orchestrator", + flash_size_bytes: 131072, + processes: [ + { + name: "orchestrator_process", + ram_size_bytes: 65536, + objects: [ + { + name: "pldm_notify", + type: "channel_handler", + }, + { + type: "thread", + name: "orchestrator_thread", + kernel_stack_size_bytes: 4096, + },], + }, + ], + }, + { + name: "pldm", + flash_size_bytes: 131072, + processes: [ + { + name: "pldm_process", + ram_size_bytes: 65536, + objects: [ + { + name: "pldm_notify", + type: "channel_initiator", + handler_process: "orchestrator_process", + handler_object_name: "pldm_notify", + }, + { + type: "thread", + name: "pldm_thread", + kernel_stack_size_bytes: 4096, + },], + }, + ], + }, + ], +} diff --git a/target/ast10x0/tests/pldm_notify/target.rs b/target/ast10x0/tests/pldm_notify/target.rs new file mode 100644 index 00000000..c9dffc87 --- /dev/null +++ b/target/ast10x0/tests/pldm_notify/target.rs @@ -0,0 +1,40 @@ +// Licensed under the Apache-2.0 license +// SPDX-License-Identifier: Apache-2.0 + +//! Kernel target for the PLDM notification channel QEMU test. +//! +//! Pass/fail is communicated by the pldm process 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 PLDM notification channel 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);