diff --git a/Cargo.lock b/Cargo.lock index 00f478d..e49c304 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -150,6 +150,12 @@ dependencies = [ "husky-rs", ] +[[package]] +name = "diff" +version = "0.1.13" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "56254986775e3233ffa9c4d7d3faaf6d36a2c09d30b20687e9f88bc8bafc16c8" + [[package]] name = "ec-service-lib" version = "0.1.0" @@ -161,6 +167,12 @@ dependencies = [ "uuid", ] +[[package]] +name = "embedded-crc-macros" +version = "1.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4f1c75747a43b086df1a87fb2a889590bc0725e0abf54bba6d0c4bf7bd9e762c" + [[package]] name = "equivalent" version = "1.0.2" @@ -336,6 +348,17 @@ version = "0.4.27" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "13dc2df351e3202783a1fe0d44375f7295ffb4049267b0f3018346dc122a1d94" +[[package]] +name = "mctp-rs" +version = "0.1.0" +source = "git+https://github.com/dymk/mctp-rs?rev=3d941ba5205ca7781bf37e3dc7c5dfdc99a082d6#3d941ba5205ca7781bf37e3dc7c5dfdc99a082d6" +dependencies = [ + "bit-register", + "num_enum", + "smbus-pec", + "thiserror", +] + [[package]] name = "memchr" version = "2.7.4" @@ -353,18 +376,19 @@ dependencies = [ [[package]] name = "num_enum" -version = "0.7.3" +version = "0.7.6" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "4e613fc340b2220f734a8595782c551f1250e969d87d3be1ae0579e8d4065179" +checksum = "5d0bca838442ec211fa11de3a8b0e0e8f3a4522575b5c4c06ed722e005036f26" dependencies = [ "num_enum_derive", + "rustversion", ] [[package]] name = "num_enum_derive" -version = "0.7.3" +version = "0.7.6" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "af1844ef2428cc3e1cb900be36181049ef3d3193c63e43026cfe202983b27a56" +checksum = "680998035259dcfcafe653688bf2aa6d3e2dc05e98be6ab46afb089dc84f1df8" dependencies = [ "proc-macro2", "quote", @@ -399,6 +423,16 @@ version = "0.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "8b870d8c151b6f2fb93e84a13146138f05d02ed11c7e7c54f8826aaaf7c9f184" +[[package]] +name = "pretty_assertions" +version = "1.4.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3ae130e2f271fbc2ac3a40fb1d07180839cdbbe443c7a27e1e3c13c5cac0116d" +dependencies = [ + "diff", + "yansi", +] + [[package]] name = "proc-macro-crate" version = "3.3.0" @@ -542,6 +576,23 @@ dependencies = [ "autocfg", ] +[[package]] +name = "smbus-pec" +version = "1.0.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ca0763a680cd5d72b28f7bfc8a054c117d8841380a6ad4f72f05bd2a34217d3e" +dependencies = [ + "embedded-crc-macros", +] + +[[package]] +name = "sp-mctp-framer" +version = "0.1.0" +dependencies = [ + "mctp-rs", + "pretty_assertions", +] + [[package]] name = "static_assertions" version = "1.1.0" @@ -584,18 +635,18 @@ dependencies = [ [[package]] name = "thiserror" -version = "2.0.12" +version = "2.0.18" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "567b8a2dae586314f7be2a752ec7474332959c6460e02bde30d702a66d488708" +checksum = "4288b5bcbc7920c07a1149a35cf9590a2aa808e0bc1eafaade0b80947865fbc4" dependencies = [ "thiserror-impl", ] [[package]] name = "thiserror-impl" -version = "2.0.12" +version = "2.0.18" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7f7cf42b4507d8ea322120659672cf1b9dbb93f8f2d4ecfd6e51350ff5b17a1d" +checksum = "ebc4ee7f67670e9b64d05fa4253e753e016c6c95ff35b89b7941d6b856dec1d5" dependencies = [ "proc-macro2", "quote", @@ -765,3 +816,9 @@ checksum = "c06928c8748d81b05c9be96aad92e1b6ff01833332f281e8cfca3be4b35fc9ec" dependencies = [ "memchr", ] + +[[package]] +name = "yansi" +version = "1.0.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "cfe53a6657fd280eaa890a3bc59152892ffa3e30101319d168b781ed6529b049" diff --git a/Cargo.toml b/Cargo.toml index 167af94..4e99c1f 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -9,11 +9,13 @@ members = [ "platform/ihv1-sp", "platform/qemu-sp", "aarch64-haf", + "sp-mctp-framer", ] [workspace.package] version = "0.1.0" edition = "2021" +rust-version = "1.92" [workspace.dependencies] aarch64-cpu = "10.0.0" @@ -33,12 +35,14 @@ espi-device-stub = { path = "espi-device-stub" } odp-ffa = { path = "odp-ffa" } hafnium = { path = "hafnium" } log = { version = "0.4", default-features = false } +mctp-rs = { git = "https://github.com/dymk/mctp-rs", rev = "3d941ba5205ca7781bf37e3dc7c5dfdc99a082d6", default-features = false } mockall = "0.13.1" -num_enum = { version = "0.7.3", default-features = false } +num_enum = { version = "0.7.4", default-features = false } num-traits = { version = "0.2.19", default-features = false } subenum = { version = "1.1.2", default-features = false } uuid = { version = "1.0", default-features = false, features = ["v1"] } rstest = "0.26.1" +pretty_assertions = "1.4.1" [workspace.lints.clippy] suspicious = "deny" diff --git a/aarch64-haf/Cargo.toml b/aarch64-haf/Cargo.toml index 17f51b8..7ea7fd5 100644 --- a/aarch64-haf/Cargo.toml +++ b/aarch64-haf/Cargo.toml @@ -2,6 +2,7 @@ name = "aarch64-haf" version = "0.1.0" edition = "2024" +rust-version.workspace = true license = "MIT" authors = ["Dylan Knutson "] description = "AArch64 HAL for Hafnium" diff --git a/dev-hooks/Cargo.toml b/dev-hooks/Cargo.toml index b531e5d..6b39772 100644 --- a/dev-hooks/Cargo.toml +++ b/dev-hooks/Cargo.toml @@ -2,6 +2,7 @@ name = "dev-hooks" version = "0.1.0" edition = "2021" +rust-version.workspace = true publish = false description = "Git hooks setup (husky-rs); not published" license = "MIT" diff --git a/ec-service-lib/Cargo.toml b/ec-service-lib/Cargo.toml index fcb60fa..724fd41 100644 --- a/ec-service-lib/Cargo.toml +++ b/ec-service-lib/Cargo.toml @@ -3,6 +3,7 @@ name = "ec-service-lib" categories = ["embedded", "no-std"] version = "0.1.0" edition = "2021" +rust-version.workspace = true license = "MIT" description = "Provides standard EC services to OS" readme = "README.md" diff --git a/espi-device-stub/Cargo.toml b/espi-device-stub/Cargo.toml index 5c23675..1bcd757 100644 --- a/espi-device-stub/Cargo.toml +++ b/espi-device-stub/Cargo.toml @@ -2,6 +2,7 @@ name = "espi-device-stub" version = "0.1.0" edition = "2021" +rust-version.workspace = true authors = ["Dylan Knutson "] license = "MIT" description = "Stub implementation of the eSPI driver" diff --git a/espi-device/Cargo.toml b/espi-device/Cargo.toml index 899c0e8..3f0bf2e 100644 --- a/espi-device/Cargo.toml +++ b/espi-device/Cargo.toml @@ -2,6 +2,7 @@ name = "espi-device" version = "0.1.0" edition = "2021" +rust-version.workspace = true description = "eSPI device driver interface" license = "MIT" authors = ["Dylan Knutson "] diff --git a/platform/qemu-sp/Cargo.toml b/platform/qemu-sp/Cargo.toml index 48735cd..7c17034 100644 --- a/platform/qemu-sp/Cargo.toml +++ b/platform/qemu-sp/Cargo.toml @@ -2,6 +2,7 @@ name = "qemu-ec-sp" version = "0.1.0" edition = "2021" +rust-version.workspace = true license = "MIT OR Apache-2.0" description = "QEMU Embedded Controller Secure Partition Service" authors = ["Phil Weber "] diff --git a/rust-toolchain.toml b/rust-toolchain.toml index e097ffd..3a8621a 100644 --- a/rust-toolchain.toml +++ b/rust-toolchain.toml @@ -1,3 +1,4 @@ [toolchain] +channel = "1.92.0" components = ["rust-src", "llvm-tools-preview", "rustfmt", "rust-analyzer"] targets = ["aarch64-unknown-none-softfloat"] diff --git a/sp-mctp-framer/Cargo.toml b/sp-mctp-framer/Cargo.toml new file mode 100644 index 0000000..2869ffd --- /dev/null +++ b/sp-mctp-framer/Cargo.toml @@ -0,0 +1,29 @@ +[package] +name = "sp-mctp-framer" +version = "0.1.0" +edition = "2021" +rust-version.workspace = true +license = "MIT" +description = "SP-side MCTP framer over SmbusEspiMedium for SP↔EC interop" +repository = "https://github.com/OpenDevicePartnership/odp-secure-services" +readme = "README.md" +keywords = ["mctp", "smbus", "no-std", "embedded", "sp"] +categories = ["embedded", "no-std"] + +[features] +# Re-export Phase 12 golden capture fixtures for downstream test suites +# (e.g. `mod/secure-services/platform/src/mctp_ping.rs` host tests). Keeps +# the wire-format goldens in a single place — see lib.rs::test_fixtures. +test-fixtures = [] + +[dependencies] +mctp-rs.workspace = true + +[dev-dependencies] +pretty_assertions.workspace = true + +[lints] +workspace = true + +[package.metadata.docs.rs] +all-features = true diff --git a/sp-mctp-framer/README.md b/sp-mctp-framer/README.md new file mode 100644 index 0000000..b59d592 --- /dev/null +++ b/sp-mctp-framer/README.md @@ -0,0 +1,30 @@ +# sp-mctp-framer + +SP-side MCTP framer over `SmbusEspiMedium`. Encodes ODP-relay requests and +decodes ODP-relay responses byte-for-byte against EC's +`embedded-services::uart-service` framing. Service-agnostic — caller +supplies `service_id`, `message_id`, and the per-service payload bytes. + +## Usage + +```rust +use sp_mctp_framer::{encode_request, decode_response}; + +const BATTERY_SVC_ID: u8 = 0x08; +const BATTERY_GETSTA_MSG_ID: u16 = 15; + +let mut tx = [0u8; 32]; +let n = encode_request(&mut tx, BATTERY_SVC_ID, BATTERY_GETSTA_MSG_ID, &[/* battery_id */ 0])?; +// transmit tx[..n] over the wire ... +let resp = decode_response(&rx[..])?; +``` + +Runtime path uses only stack/static buffers (no heap). Maximum per-service +payload is `MAX_PAYLOAD_LEN` bytes — larger payloads return `BufTooSmall`. + +## Features + +- `test-fixtures` — re-export captured ping-pong wire bytes + (`PHASE_12_TX_18B`, `PHASE_12_RX_13B`) for downstream test crates. + These are the captured Battery::GetSta ping-pong from QEMU SBSA spike-up + and serve as the reference for both encode and decode validation. diff --git a/sp-mctp-framer/src/lib.rs b/sp-mctp-framer/src/lib.rs new file mode 100644 index 0000000..afa24d4 --- /dev/null +++ b/sp-mctp-framer/src/lib.rs @@ -0,0 +1,609 @@ +//! SP-side MCTP framer over `SmbusEspiMedium` for SP↔EC interop. +//! +//! Service-agnostic: the public API takes `service_id`, `message_id`, and +//! a per-service payload, and produces / parses the SmbusEspi+MCTP+ODP-relay +//! wire framing that EC's `embedded-services::uart-service` expects. +//! Mirrors that crate byte-for-byte (verified against captured wire bytes +//! from a successful Battery::GetSta SP→EC→SP ping-pong on the QEMU SBSA +//! secure UART at `0x60030000`). +//! +//! Runtime path uses only stack/static buffers — no heap. + +#![no_std] + +use core::fmt; + +use mctp_rs::smbus_espi::{SmbusEspiMedium, SmbusEspiReplyContext}; +use mctp_rs::{ + EndpointId, MctpMessageHeaderTrait, MctpMessageTrait, MctpPacketContext, MctpPacketError, + MctpPacketResult, MctpReplyContext, MctpSequenceNumber, MctpMessageTag, +}; + +// --------------------------------------------------------------------------- +// Wire constants +// --------------------------------------------------------------------------- +// +// All on-wire constants below were derived from a captured SP→EC ping-pong +// session and cross-checked against: +// +// * EC's `embedded-services::uart-service::process_response` +// (uart-service/src/lib.rs:65-95) — slave-address values + endpoint-id / +// message-tag conventions. +// * `mctp-rs`'s `SmbusEspiMedium::serialize` +// (medium/smbus_espi.rs:58-102) — confirms reply_context source/dest +// slave addresses are SWAPPED on serialize, and the bit_register layout +// puts `destination_slave_address` in bits 25..31 of the LE u32 that is +// then written `to_be_bytes()` (so it lands in wire byte 0). +// * `mctp-rs`'s `serialize_packet` / `SerializePacketState::next` +// (mctp_packet_context.rs:155-187, serialize.rs:25-85) — confirms the +// transport-header source_endpoint_id / destination_endpoint_id are +// SWAPPED on serialize (line 62-63 of serialize.rs), and that +// `packet_sequence_number` is `.inc()`'d before being serialized (so the +// initial value 0 yields wire seq=1 → matches captured byte 0xd3). +// * `embedded-services::relay::mctp` ODP relay header `bitfield!` +// (embedded-service/src/relay/mod.rs:285-303) — formal bit layout for +// the ODP relay header u32 (big-endian on wire): +// bit 25 : is_request (1 bit) +// bits 23..16: service_id (8 bits) +// bit 15 : is_error (1 bit) +// bits 14..0 : message_id (15 bits) +// +// Names below mirror `uart-service/src/lib.rs:69-78` so the SP↔EC interop +// surface is auditable side-by-side. + +/// SP-side MCTP endpoint ID (matches `uart-service` source_endpoint_id 0x80). +const SP_EID: u8 = 0x80; +/// MCTP message tag — `uart-service` hard-codes 3. +const MCTP_MSG_TAG: u8 = 3; +/// SmbusEspi destination slave (semantic — wire byte ordering is swapped by +/// `SmbusEspiMedium::serialize`). Matches `uart-service` line 75: `destination_slave_address: 1`. +const SMBUS_DST_SLAVE: u8 = 1; +/// SmbusEspi source slave. Matches `uart-service` line 76: `source_slave_address: 0`. +const SMBUS_SRC_SLAVE: u8 = 0; +/// Number of leading wire bytes consumed by everything in front of the +/// per-service payload: 4-byte SmbusEspi header + 4-byte MCTP transport +/// header + 1-byte MCTP message-type + 4-byte ODP relay header. +const FRAME_OVERHEAD: usize = 4 + 4 + 1 + 4; +/// Internal mctp-rs scratch buffer size. Sized for a single-packet +/// SmbusEspi frame (max body 32 + 4 header + 1 PEC = 37) plus the prefix +/// `serialize_packet` carves off (msg_type 1 + odp header 4 = 5). Round to 64. +const SCRATCH_SIZE: usize = 64; +/// Maximum per-service payload the framer can encode/decode in one call. +/// Bounded by `SCRATCH_SIZE` minus `FRAME_OVERHEAD` minus the 1-byte PEC slot +/// the deserializer expects. Callers passing larger payloads get `BufTooSmall`. +pub const MAX_PAYLOAD_LEN: usize = SCRATCH_SIZE - FRAME_OVERHEAD - 1; + +// --------------------------------------------------------------------------- +// Public API +// --------------------------------------------------------------------------- + +/// Errors surfaced by the framer. No allocator required. +#[derive(Debug, PartialEq, Eq)] +pub enum FramerError { + /// Output / scratch buffer too small for the requested operation. + BufTooSmall, + /// Input buffer too short to contain the expected wire framing. + Truncated, + /// Underlying mctp-rs decoder/encoder rejected the input. + DecodeFailed, + /// Underlying mctp-rs decoder/encoder rejected the encode. + EncodeFailed, +} + +impl fmt::Display for FramerError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + FramerError::BufTooSmall => f.write_str("output buffer too small"), + FramerError::Truncated => f.write_str("input truncated"), + FramerError::DecodeFailed => f.write_str("decode failed"), + FramerError::EncodeFailed => f.write_str("encode failed"), + } + } +} + +/// Structured view of a decoded EC→SP ODP-relay response. +/// +/// The `body` slice borrows from the caller's input buffer and contains the +/// per-service result payload (i.e. everything after the 4-byte ODP relay +/// header echo). +#[derive(Debug, PartialEq, Eq)] +pub struct OdpRelayResponse<'a> { + pub is_request: bool, + pub service_id: u8, + pub is_error: bool, + pub message_id: u16, + pub body: &'a [u8], +} + +/// Encode a generic ODP-relay request into `buf` over the SmbusEspiMedium +/// framing used by EC's `uart-service`. +/// +/// The framer prepends the 4-byte ODP relay header echo to `payload` before +/// handing it to mctp-rs, so callers pass only the per-service payload bytes +/// (e.g. `&[battery_id]` for `Battery::GetSta`). +/// +/// `service_id` is used both as the MCTP destination endpoint ID on the wire +/// and as the `service_id` field of the ODP relay header — these are the same +/// value by ODP convention (each service is its own MCTP endpoint). +/// +/// Returns the number of bytes written. The output is the on-wire byte +/// sequence WITHOUT the trailing PEC byte — `uart-service` strips PEC before +/// writing to UART (uart-service/src/lib.rs:88). +pub fn encode_request( + buf: &mut [u8], + service_id: u8, + message_id: u16, + payload: &[u8], +) -> Result { + if payload.len() > MAX_PAYLOAD_LEN { + return Err(FramerError::BufTooSmall); + } + // Cheap up-front guard so callers with obviously-too-small buffers fail + // fast. + let min_output = FRAME_OVERHEAD + payload.len(); + if buf.len() < min_output { + return Err(FramerError::BufTooSmall); + } + + let mut assembly_buf = [0u8; SCRATCH_SIZE]; + let mut ctx = + MctpPacketContext::::new(SmbusEspiMedium, &mut assembly_buf); + + let reply_ctx = MctpReplyContext:: { + // Note the swap convention (see file-header comment): `source_*` + // here becomes the on-wire DESTINATION after `SmbusEspiMedium::serialize` + // / `SerializePacketState::next` swap them. So to put the target + // service on wire-dst and SP on wire-src, we set source=service, dest=SP. + source_endpoint_id: EndpointId::Id(service_id), + destination_endpoint_id: EndpointId::Id(SP_EID), + packet_sequence_number: MctpSequenceNumber::new(0), + message_tag: MctpMessageTag::try_from(MCTP_MSG_TAG) + .map_err(|_| FramerError::EncodeFailed)?, + medium_context: SmbusEspiReplyContext { + destination_slave_address: SMBUS_DST_SLAVE, + source_slave_address: SMBUS_SRC_SLAVE, + }, + }; + + // Outbound ODP relay header for the request. + let odp_header = OdpRelayHeader { + is_request: true, + service_id, + is_error: false, + message_id, + }; + + // ODP body = 4-byte relay-header echo (so EC can dispatch without + // re-parsing the transport header) followed by the per-service payload. + // Derive the first 4 bytes from `odp_header` rather than hand-encoding + // them so a future header-layout tweak can't silently desync against the + // body. + let header_bytes = odp_header.to_u32().to_be_bytes(); + let mut body_buf = [0u8; MAX_PAYLOAD_LEN + 4]; + body_buf[..4].copy_from_slice(&header_bytes); + body_buf[4..4 + payload.len()].copy_from_slice(payload); + let body = OdpRelayBody { + bytes: &body_buf[..4 + payload.len()], + }; + + let mut packet_state = ctx + .serialize_packet(reply_ctx, (odp_header, body)) + .map_err(|_| FramerError::EncodeFailed)?; + + // The framer is single-packet (the supported services all fit in one MCTP + // body), so we expect exactly one `next()`. + let packet = packet_state + .next() + .ok_or(FramerError::EncodeFailed)? + .map_err(|_| FramerError::EncodeFailed)?; + + // Drop trailing PEC byte to match the on-wire form `uart-service` + // produces. + let stripped = packet + .get(..packet.len().saturating_sub(1)) + .ok_or(FramerError::EncodeFailed)?; + + if buf.len() < stripped.len() { + return Err(FramerError::BufTooSmall); + } + buf[..stripped.len()].copy_from_slice(stripped); + Ok(stripped.len()) +} + +/// Decode an EC-originated SmbusEspi+MCTP framed ODP-relay response from `buf`. +/// +/// `buf` is the on-wire byte sequence as produced by `uart-service` +/// (i.e. PEC byte already stripped). The returned `body` slice borrows from +/// `buf` and contains the per-service result payload (everything past the +/// 4-byte ODP relay header echo). +/// +/// Callers gate on `service_id` themselves — a successful decode does NOT +/// imply the response targets any particular service. +pub fn decode_response(buf: &[u8]) -> Result, FramerError> { + // mctp-rs's `SmbusEspiMedium::deserialize` requires a trailing PEC byte + // (medium/smbus_espi.rs:46-55). The on-wire form `uart-service` produces + // does NOT include PEC. Re-attach a placeholder PEC byte so mctp-rs can + // parse — the deserializer reads but does not validate the PEC value. + if buf.len() < 4 { + return Err(FramerError::Truncated); + } + let byte_count = buf[2] as usize; + let needed = 4 + byte_count; + if buf.len() < needed { + return Err(FramerError::Truncated); + } + + let mut padded = [0u8; SCRATCH_SIZE]; + if needed + 1 > padded.len() { + // Wire-claimed size exceeds our internal padded scratch. Surface as + // BufTooSmall (the requested operation cannot fit our buffer) rather + // than DecodeFailed (which implies semantic rejection). + return Err(FramerError::BufTooSmall); + } + padded[..needed].copy_from_slice(&buf[..needed]); + padded[needed] = 0; // dummy PEC + + let mut assembly_buf = [0u8; SCRATCH_SIZE]; + let mut ctx = + MctpPacketContext::::new(SmbusEspiMedium, &mut assembly_buf); + + let message = ctx + .deserialize_packet(&padded[..needed + 1]) + .map_err(|_| FramerError::DecodeFailed)? + .ok_or(FramerError::DecodeFailed)?; + + let (header, body) = message + .parse_as::() + .map_err(|_| FramerError::DecodeFailed)?; + + // `body.0` is the per-service result payload (everything past the 4-byte + // ODP header). It borrows from `assembly_buf` which lives only for this + // call; copy the few-byte borrow into a slice that borrows from the + // caller's input instead. We do this by re-slicing `buf` directly. + // + // Layout of `buf` (PEC stripped): + // [0..4) SmbusEspi header + // [4..8) MCTP transport header + // [8] MCTP message type byte (0x7d) + // [9..13) ODP relay header + // [13..) per-service result body + // + // Defense-in-depth: if `byte_count` is small enough that the header alone + // doesn't fit (`needed < 13`), `buf.get(13..needed)` is an inverted range. + // Treat that as truncation rather than silently substituting `&[]`, since + // a downstream caller that gates only on `service_id` could otherwise + // accept a malformed-but-mctp-rs-parsed frame. + let _ = body; + let result_body = buf.get(13..needed).ok_or(FramerError::Truncated)?; + + Ok(OdpRelayResponse { + is_request: header.is_request, + service_id: header.service_id, + is_error: header.is_error, + message_id: header.message_id, + body: result_body, + }) +} + +// --------------------------------------------------------------------------- +// ODP relay header + body — minimal local impls of mctp-rs's +// `MctpMessageHeaderTrait` / `MctpMessageTrait` so we can drive +// `serialize_packet` / `parse_as` with the ODP message type (0x7d). +// --------------------------------------------------------------------------- + +/// MCTP message type byte that ODP uses for relay traffic (captured wire byte 8). +const ODP_MSG_TYPE: u8 = 0x7d; + +#[derive(Copy, Clone, Debug, PartialEq, Eq)] +struct OdpRelayHeader { + is_request: bool, + service_id: u8, + is_error: bool, + message_id: u16, +} + +impl OdpRelayHeader { + /// Bit-extraction layout per `embedded-services::relay::mctp` `bitfield!` + /// (embedded-service/src/relay/mod.rs:285-303), wire bytes are + /// big-endian: + /// bit 25 : is_request (1) + /// bits 23..16: service_id (8) + /// bit 15 : is_error (1) + /// bits 14..0 : message_id (15) + fn from_u32(raw: u32) -> Self { + Self { + is_request: ((raw >> 25) & 1) != 0, + service_id: ((raw >> 16) & 0xff) as u8, + is_error: ((raw >> 15) & 1) != 0, + message_id: (raw & 0x7fff) as u16, + } + } + + fn to_u32(self) -> u32 { + ((self.is_request as u32) << 25) + | ((self.service_id as u32) << 16) + | ((self.is_error as u32) << 15) + | ((self.message_id as u32) & 0x7fff) + } +} + +impl MctpMessageHeaderTrait for OdpRelayHeader { + fn serialize(self, buffer: &mut [u8]) -> MctpPacketResult { + if buffer.len() < 4 { + return Err(MctpPacketError::SerializeError( + "buffer too small for odp relay header", + )); + } + buffer[..4].copy_from_slice(&self.to_u32().to_be_bytes()); + Ok(4) + } + + fn deserialize( + buffer: &[u8], + ) -> MctpPacketResult<(Self, &[u8]), M> { + if buffer.len() < 4 { + return Err(MctpPacketError::HeaderParseError( + "buffer too small for odp relay header", + )); + } + let raw = u32::from_be_bytes([buffer[0], buffer[1], buffer[2], buffer[3]]); + Ok((OdpRelayHeader::from_u32(raw), &buffer[4..])) + } +} + +/// Encode-side ODP body — borrows the assembled (4-byte header echo + +/// per-service payload) byte sequence so the framer can support variable-length +/// per-service payloads without a const generic. Encode-only — see +/// `OdpRelayBodyDecode` for the decode-side counterpart. +struct OdpRelayBody<'a> { + bytes: &'a [u8], +} + +impl<'buf, 'a> MctpMessageTrait<'buf> for OdpRelayBody<'a> { + type Header = OdpRelayHeader; + const MESSAGE_TYPE: u8 = ODP_MSG_TYPE; + + fn serialize(self, buffer: &mut [u8]) -> MctpPacketResult { + if buffer.len() < self.bytes.len() { + return Err(MctpPacketError::SerializeError( + "buffer too small for odp relay body", + )); + } + buffer[..self.bytes.len()].copy_from_slice(self.bytes); + Ok(self.bytes.len()) + } + + fn deserialize( + _: &Self::Header, + _buffer: &'buf [u8], + ) -> MctpPacketResult { + // Encode-side type — should never be used on the decode path. + Err(MctpPacketError::HeaderParseError( + "OdpRelayBody is encode-only", + )) + } +} + +/// Decode-side marker type. Carries no payload — the body bytes are +/// re-sliced from the caller's input buffer in `decode_battery_response` +/// (so they outlive `mctp-rs`'s transient assembly buffer). Renamed from +/// the misleading `OdpRelayBodyOwned`: it owns nothing. +struct OdpRelayBodyDecode; + +impl<'buf> MctpMessageTrait<'buf> for OdpRelayBodyDecode { + type Header = OdpRelayHeader; + const MESSAGE_TYPE: u8 = ODP_MSG_TYPE; + + fn serialize(self, _: &mut [u8]) -> MctpPacketResult { + Err(MctpPacketError::SerializeError( + "OdpRelayBodyDecode is decode-only", + )) + } + + fn deserialize( + _: &Self::Header, + _buffer: &'buf [u8], + ) -> MctpPacketResult { + Ok(OdpRelayBodyDecode) + } +} + +// --------------------------------------------------------------------------- +// Test fixtures (re-exported for downstream test crates under the +// `test-fixtures` feature). Kept here as the single source of truth for the +// captured ping-pong wire bytes — duplicating them across submodule +// boundaries (e.g. `mctp_ping.rs`'s host tests) silently masks wire-format +// drift if one copy is updated and the other is not. +// --------------------------------------------------------------------------- + +#[cfg(any(test, feature = "test-fixtures"))] +pub mod test_fixtures { + /// Captured TX (SP→EC, Battery::GetSta request, battery_id=0). + /// + /// Byte breakdown: + /// 00 0f 0e 03 SmbusEspi header (dst_slave=0, src_slave=1, byte_count=14, cmd=0x0f MCTP) + /// 01 08 80 d3 MCTP transport header (ver=1, dst_eid=0x08 EC, src_eid=0x80 SP, SOM/EOM/seq=1/tag=3) + /// 7d MCTP message type byte (ODP relay) + /// 02 08 00 0f ODP relay header (is_request=1, service_id=0x08, msg_id=15) + /// 02 08 00 0f 00 ODP body: relay-header re-echo + battery_id=0 + pub const PHASE_12_TX_18B: [u8; 18] = [ + 0x00, 0x0f, 0x0e, 0x03, + 0x01, 0x08, 0x80, 0xd3, + 0x7d, + 0x02, 0x08, 0x00, 0x0f, + 0x02, 0x08, 0x00, 0x0f, 0x00, + ]; + + /// Captured RX (EC→SP, Battery error response, message_id=1). + /// + /// Byte breakdown: + /// 00 0f 09 03 SmbusEspi header (byte_count=0x09) + /// 01 80 08 d3 MCTP transport header (dst_eid=0x80 SP, src_eid=0x08 EC battery) + /// 7d MCTP message type byte (ODP relay) + /// 00 08 80 01 ODP relay header (is_request=0, service_id=0x08, is_error=1, msg_id=1) + pub const PHASE_12_RX_13B: [u8; 13] = [ + 0x00, 0x0f, 0x09, 0x03, + 0x01, 0x80, 0x08, 0xd3, + 0x7d, + 0x00, 0x08, 0x80, 0x01, + ]; +} + +#[cfg(test)] +mod tests { + use super::test_fixtures::{PHASE_12_RX_13B, PHASE_12_TX_18B}; + use super::{decode_response, encode_request, FramerError}; + use pretty_assertions::assert_eq; + + // Concrete service constants used to drive the captured Battery::GetSta + // ping-pong fixture. The framer itself is service-agnostic — these live + // in the tests because the fixtures they validate are battery traffic. + const BATTERY_SVC_ID: u8 = 0x08; + const BATTERY_GETSTA_MSG_ID: u16 = 15; + + #[test] + fn encode_request_matches_phase12_battery_capture() { + let mut buf = [0u8; 64]; + let n = encode_request( + &mut buf, + BATTERY_SVC_ID, + BATTERY_GETSTA_MSG_ID, + &[/*battery_id=*/ 0u8], + ) + .expect("encode ok"); + assert_eq!(n, PHASE_12_TX_18B.len(), "encoded length mismatch"); + assert_eq!(&buf[..n], &PHASE_12_TX_18B[..], "encoded bytes diverge from captured wire"); + } + + #[test] + fn decode_response_parses_phase12_battery_rx() { + let resp = decode_response(&PHASE_12_RX_13B).expect("decode ok"); + assert!(!resp.is_request, "expected response (is_request=false)"); + assert_eq!(resp.service_id, 0x08, "expected Battery service"); + assert!(resp.is_error, "expected is_error=1 from spike capture"); + assert_eq!(resp.message_id, 1, "expected message_id=1"); + // Captured RX has byte_count=9 → needed=13, so the per-service body + // is empty (just an ODP relay header echo). Pin that explicitly so a + // future regression to the body slice offset is caught. + assert_eq!(resp.body, &[][..], "Phase 12 RX has zero body bytes"); + } + + #[test] + fn decode_response_extracts_body_bytes() { + // Synthetic frame with a non-empty per-service body: header byte_count + // bumped to 0x0d so needed=17, then 4 trailing bytes (0xDE 0xAD 0xBE + // 0xEF) appended after the 13-byte header. Validates that + // `decode_response` returns the correct body slice (catches any future + // regression to the `13..needed` re-slice offset). + let mut frame = [0u8; 17]; + frame[..PHASE_12_RX_13B.len()].copy_from_slice(&PHASE_12_RX_13B); + frame[2] = 0x0d; // byte_count = 4 (header) + 9 (transport+ODP) → needed=17 + frame[13] = 0xDE; + frame[14] = 0xAD; + frame[15] = 0xBE; + frame[16] = 0xEF; + let resp = decode_response(&frame).expect("decode ok"); + assert_eq!(resp.body, &[0xDE, 0xAD, 0xBE, 0xEF][..]); + } + + /// Roundtrip: encode each interesting battery_id, decode the result, + /// assert header fields are preserved. Catches encoder regressions where + /// the literal byte happens to match the captured fixture but the + /// semantic field has drifted. + #[test] + fn encode_decode_roundtrip_preserves_header_fields() { + for &battery_id in &[0u8, 1, 0x7D, 0x7E, 0xFF] { + let mut tx = [0u8; 64]; + let n = encode_request(&mut tx, BATTERY_SVC_ID, BATTERY_GETSTA_MSG_ID, &[battery_id]) + .unwrap_or_else(|e| panic!("encode failed for battery_id={battery_id}: {e:?}")); + // The encoder produces a REQUEST frame; `decode_response` is + // permissive about request vs response and just decodes the ODP + // relay header. Assert the fields the SP cares about. + let resp = decode_response(&tx[..n]) + .unwrap_or_else(|e| panic!("decode failed for battery_id={battery_id}: {e:?}")); + assert!(resp.is_request, "encoded frame must mark is_request=1"); + assert_eq!(resp.service_id, BATTERY_SVC_ID, "service_id must round-trip"); + assert!(!resp.is_error, "request frames have is_error=0"); + assert_eq!(resp.message_id, BATTERY_GETSTA_MSG_ID, "message_id must round-trip"); + assert_eq!( + resp.body.last().copied(), + Some(battery_id), + "payload byte must appear at end of body" + ); + } + } + + #[test] + fn encode_short_buf_errors() { + // Pin the BufTooSmall boundary for a 1-byte payload at 18 bytes + // (FRAME_OVERHEAD=13 + 1 payload byte → 14? no — the header echo + // adds 4 bytes inside the ODP body, so total = 13 + 4 + 1 = 18). + let mut too_small = [0u8; 17]; + let result = encode_request(&mut too_small, BATTERY_SVC_ID, BATTERY_GETSTA_MSG_ID, &[0]); + assert!( + matches!(result, Err(FramerError::BufTooSmall)), + "expected BufTooSmall at 17 bytes, got {result:?}" + ); + + let mut just_enough = [0u8; 18]; + let result = encode_request(&mut just_enough, BATTERY_SVC_ID, BATTERY_GETSTA_MSG_ID, &[0]); + assert!( + result.is_ok(), + "expected Ok at 18 bytes, got {result:?}" + ); + } + + #[test] + fn encode_oversize_payload_errors_as_buf_too_small() { + // Payload exceeds MAX_PAYLOAD_LEN — must surface as BufTooSmall + // (configuration error) not EncodeFailed (mctp-rs rejection). + let mut buf = [0u8; 256]; + let oversize = [0u8; super::MAX_PAYLOAD_LEN + 1]; + let result = encode_request(&mut buf, 0x10, 0, &oversize); + assert!( + matches!(result, Err(FramerError::BufTooSmall)), + "expected BufTooSmall for oversized payload, got {result:?}" + ); + } + + #[test] + fn decode_truncated_header_errors() { + let result = decode_response(&[0x00, 0x0f]); // truncated SmbusEspi header + assert!( + matches!( + result, + Err(FramerError::Truncated) | Err(FramerError::DecodeFailed) + ), + "expected Truncated or DecodeFailed, got {result:?}" + ); + } + + #[test] + fn decode_partial_body_errors() { + // Header says byte_count=9 → needed=13, but only 6 bytes supplied. + // Exercises the second truncation guard (`buf.len() < needed`), + // distinct from the 4-byte header guard above. + let frame = [0x00u8, 0x0f, 0x09, 0x03, 0x01, 0x80]; + let result = decode_response(&frame); + assert_eq!( + result, + Err(FramerError::Truncated), + "partial body must surface as Truncated" + ); + } + + #[test] + fn decode_oversize_byte_count_errors_as_buf_too_small() { + // byte_count=200 → needed=204, exceeding the internal 64-byte + // padded scratch. Regression test for the oversize-claimed-length + // path — must surface as BufTooSmall (not DecodeFailed). + let mut big = [0u8; 204]; + big[2] = 200; + let result = decode_response(&big); + assert_eq!( + result, + Err(FramerError::BufTooSmall), + "wire-claimed oversize must surface as BufTooSmall (not DecodeFailed)" + ); + } +}