From 6675144346fd781fcab70d3dddaec7e89e73b69f Mon Sep 17 00:00:00 2001 From: Ayush Arora Date: Wed, 26 Aug 2026 15:38:53 +0530 Subject: [PATCH 01/10] add IPMI KCS and SEL core Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: dda9bd55-db87-46d6-a512-15d8a769b9a3 --- Cargo.lock | 10 + Cargo.toml | 2 + vm/devices/chipset/ipmi_kcs/Cargo.toml | 18 + vm/devices/chipset/ipmi_kcs/src/lib.rs | 306 ++++++ vm/devices/chipset/ipmi_kcs/src/protocol.rs | 107 +++ .../chipset/ipmi_kcs/src/save_restore.rs | 243 +++++ vm/devices/chipset/ipmi_kcs/src/sel.rs | 302 ++++++ vm/devices/chipset/ipmi_kcs/src/tests.rs | 889 ++++++++++++++++++ 8 files changed, 1877 insertions(+) create mode 100644 vm/devices/chipset/ipmi_kcs/Cargo.toml create mode 100644 vm/devices/chipset/ipmi_kcs/src/lib.rs create mode 100644 vm/devices/chipset/ipmi_kcs/src/protocol.rs create mode 100644 vm/devices/chipset/ipmi_kcs/src/save_restore.rs create mode 100644 vm/devices/chipset/ipmi_kcs/src/sel.rs create mode 100644 vm/devices/chipset/ipmi_kcs/src/tests.rs diff --git a/Cargo.lock b/Cargo.lock index 20fb197fe5..0de7d9b433 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4063,6 +4063,16 @@ dependencies = [ "thiserror 2.0.16", ] +[[package]] +name = "ipmi_kcs" +version = "0.0.0" +dependencies = [ + "mesh", + "test_with_tracing", + "thiserror 2.0.16", + "vmcore", +] + [[package]] name = "is_terminal_polyfill" version = "1.70.1" diff --git a/Cargo.toml b/Cargo.toml index ff5f3be4e6..59294ac3b8 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -30,6 +30,8 @@ members = [ "vmm_tests/vmm_perf", # hyper-v tooling "hyperv/tools/hypestv", + # standalone device crates + "vm/devices/chipset/ipmi_kcs", # fuzzing "support/inspect/fuzz", "support/mesh/mesh_protobuf/fuzz", diff --git a/vm/devices/chipset/ipmi_kcs/Cargo.toml b/vm/devices/chipset/ipmi_kcs/Cargo.toml new file mode 100644 index 0000000000..dfa45d38ba --- /dev/null +++ b/vm/devices/chipset/ipmi_kcs/Cargo.toml @@ -0,0 +1,18 @@ +# Copyright (c) Microsoft Corporation. +# Licensed under the MIT License. + +[package] +name = "ipmi_kcs" +edition.workspace = true +rust-version.workspace = true + +[dependencies] +mesh.workspace = true +thiserror.workspace = true +vmcore.workspace = true + +[dev-dependencies] +test_with_tracing.workspace = true + +[lints] +workspace = true diff --git a/vm/devices/chipset/ipmi_kcs/src/lib.rs b/vm/devices/chipset/ipmi_kcs/src/lib.rs new file mode 100644 index 0000000000..505088c1f4 --- /dev/null +++ b/vm/devices/chipset/ipmi_kcs/src/lib.rs @@ -0,0 +1,306 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +//! A transport-independent virtual IPMI BMC with a byte-oriented KCS interface. +//! +//! This crate implements the KCS register state machine and a bounded System +//! Event Log (SEL). Platform adapters are intentionally separate: callers map +//! [`IpmiKcs::read_data`], [`IpmiKcs::read_status`], +//! [`IpmiKcs::write_data`], and [`IpmiKcs::write_command`] onto their chosen +//! PIO or MMIO transport. + +#![forbid(unsafe_code)] + +mod protocol; +mod save_restore; +mod sel; + +use sel::RateLimiter; +use sel::SelState; + +/// Maximum KCS request or response size. +pub const KCS_MESSAGE_MAX: usize = 64; + +/// KCS output-buffer-full status bit. +pub const STATUS_OBF: u8 = 0x01; +/// KCS input-buffer-full status bit. +pub const STATUS_IBF: u8 = 0x02; +/// KCS system-management-software-attention status bit. +pub const STATUS_SMS_ATN: u8 = 0x04; +/// KCS command/data status bit. +pub const STATUS_CD: u8 = 0x08; +/// KCS state field mask. +pub const STATUS_STATE_MASK: u8 = 0xc0; + +/// KCS idle state. +pub const KCS_STATE_IDLE: u8 = 0x00; +/// KCS read state. +pub const KCS_STATE_READ: u8 = 0x40; +/// KCS write state. +pub const KCS_STATE_WRITE: u8 = 0x80; +/// KCS error state. +pub const KCS_STATE_ERROR: u8 = 0xc0; + +/// KCS Get Status/Abort command. +pub const KCS_COMMAND_GET_STATUS_ABORT: u8 = 0x60; +/// KCS Write Start command. +pub const KCS_COMMAND_WRITE_START: u8 = 0x61; +/// KCS Write End command. +pub const KCS_COMMAND_WRITE_END: u8 = 0x62; +/// KCS Read Next control byte. +pub const KCS_DATA_READ_NEXT: u8 = 0x68; + +/// Trusted wall-clock source used for SEL timestamps and rate limiting. +/// +/// Implementations must return UTC seconds since the Unix epoch. Negative +/// values are accepted so that startup and test clocks can be represented +/// without lossy conversion. +pub trait TrustedClock { + /// Returns the current trusted host time in Unix seconds. + fn unix_seconds(&mut self) -> i64; +} + +/// Result of a nonblocking SEL event-forwarding attempt. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum SelEventDisposition { + /// The event was accepted by the sink. + Accepted, + /// The sink dropped the event without blocking the virtual BMC. + Dropped, +} + +/// Best-effort, nonblocking sink for finalized SEL records. +pub trait SelEventSink { + /// Attempts to forward one committed SEL record. + fn try_send(&mut self, record_id: u16, record: [u8; 16]) -> SelEventDisposition; +} + +/// Lifetime diagnostic counters for SEL additions. +/// +/// These counters are intentionally excluded from saved state. +#[derive(Debug, Default, Clone, Copy, PartialEq, Eq)] +pub struct SelStats { + /// Records successfully committed to the SEL. + pub committed: u64, + /// Committed records accepted by the configured event sink. + pub forwarded: u64, + /// Committed records not forwarded because the per-second budget was exhausted. + pub rate_limited: u64, + /// Committed records rejected by the configured event sink. + pub sink_dropped: u64, +} + +#[derive(Clone)] +struct KcsTransaction { + status: u8, + data_out: u8, + request: [u8; KCS_MESSAGE_MAX], + request_len: usize, + response: [u8; KCS_MESSAGE_MAX], + response_len: usize, + response_pos: usize, + write_end_pending: bool, +} + +impl Default for KcsTransaction { + fn default() -> Self { + Self { + status: KCS_STATE_IDLE, + data_out: 0, + request: [0; KCS_MESSAGE_MAX], + request_len: 0, + response: [0; KCS_MESSAGE_MAX], + response_len: 0, + response_pos: 0, + write_end_pending: false, + } + } +} + +impl KcsTransaction { + fn state(&self) -> u8 { + self.status & STATUS_STATE_MASK + } + + fn set_state(&mut self, state: u8) { + self.status = (self.status & !STATUS_STATE_MASK) | state; + } + + fn stage_dummy(&mut self) { + self.data_out = 0; + self.status |= STATUS_OBF; + } + + fn clear_buffers(&mut self) { + self.request.fill(0); + self.request_len = 0; + self.response.fill(0); + self.response_len = 0; + self.response_pos = 0; + self.write_end_pending = false; + } +} + +/// A minimal virtual IPMI BMC exposed through byte-oriented KCS registers. +pub struct IpmiKcs { + transaction: KcsTransaction, + sel: SelState, + clock: Box, + sink: Option>, + rate_limiter: RateLimiter, + stats: SelStats, +} + +impl IpmiKcs { + /// Creates a virtual BMC without a SEL event sink. + pub fn new(clock: impl TrustedClock + 'static) -> Self { + Self::from_boxed_parts(Box::new(clock), None) + } + + /// Creates a virtual BMC with a best-effort SEL event sink. + pub fn with_event_sink( + clock: impl TrustedClock + 'static, + sink: impl SelEventSink + 'static, + ) -> Self { + Self::from_boxed_parts(Box::new(clock), Some(Box::new(sink))) + } + + fn from_boxed_parts(clock: Box, sink: Option>) -> Self { + Self { + transaction: KcsTransaction::default(), + sel: SelState::new(), + clock, + sink, + rate_limiter: RateLimiter::default(), + stats: SelStats::default(), + } + } + + /// Replaces or removes the nonblocking SEL event sink. + pub fn set_event_sink(&mut self, sink: Option>) { + self.sink = sink; + } + + /// Reads the KCS data register and clears OBF. + pub fn read_data(&mut self) -> u8 { + let value = self.transaction.data_out; + self.transaction.status &= !STATUS_OBF; + value + } + + /// Reads the KCS status register without changing device state. + pub fn read_status(&self) -> u8 { + self.transaction.status + } + + /// Writes one byte to the KCS command register. + pub fn write_command(&mut self, command: u8) { + self.transaction.status |= STATUS_IBF | STATUS_CD; + + match command { + KCS_COMMAND_WRITE_START => { + self.transaction.request.fill(0); + self.transaction.request_len = 0; + self.transaction.write_end_pending = false; + self.transaction.set_state(KCS_STATE_WRITE); + self.transaction.stage_dummy(); + } + KCS_COMMAND_WRITE_END => { + self.transaction.write_end_pending = true; + self.transaction.set_state(KCS_STATE_WRITE); + self.transaction.stage_dummy(); + } + KCS_COMMAND_GET_STATUS_ABORT => self.handle_abort(), + KCS_DATA_READ_NEXT => {} + _ => self.handle_abort(), + } + + self.transaction.status &= !STATUS_IBF; + } + + /// Writes one byte to the KCS data register. + pub fn write_data(&mut self, byte: u8) { + self.transaction.status |= STATUS_IBF; + self.transaction.status &= !STATUS_CD; + + match self.transaction.state() { + KCS_STATE_WRITE => self.handle_write_data(byte), + KCS_STATE_READ if byte == KCS_DATA_READ_NEXT => self.handle_read_next(), + KCS_STATE_READ => self.enter_error_state(), + KCS_STATE_IDLE | KCS_STATE_ERROR => {} + _ => {} + } + + self.transaction.status &= !STATUS_IBF; + } + + /// Resets volatile KCS transaction state while preserving SEL and BMC time. + pub fn reset(&mut self) { + self.transaction = KcsTransaction::default(); + } + + /// Returns current SEL diagnostic counters. + pub fn stats(&self) -> SelStats { + self.stats + } + + /// Returns the number of records currently stored in the SEL. + pub fn sel_len(&self) -> usize { + self.sel.records.len() + } + + /// Returns a stored SEL record by insertion index. + pub fn sel_record(&self, index: usize) -> Option<&[u8; 16]> { + self.sel.records.get(index) + } + + /// Returns the guest-selected signed SEL time offset in seconds. + pub fn sel_time_offset_seconds(&self) -> i64 { + self.sel.time_offset_seconds + } + + fn handle_write_data(&mut self, byte: u8) { + if self.transaction.request_len < KCS_MESSAGE_MAX { + self.transaction.request[self.transaction.request_len] = byte; + self.transaction.request_len += 1; + } + + if self.transaction.write_end_pending { + self.transaction.write_end_pending = false; + self.process_ipmi_message(); + } else { + self.transaction.stage_dummy(); + } + } + + fn handle_read_next(&mut self) { + if self.transaction.response_pos < self.transaction.response_len { + self.transaction.data_out = self.transaction.response[self.transaction.response_pos]; + self.transaction.response_pos += 1; + self.transaction.status |= STATUS_OBF; + } else { + self.transaction.stage_dummy(); + self.transaction.set_state(KCS_STATE_IDLE); + } + } + + fn handle_abort(&mut self) { + self.transaction.clear_buffers(); + self.transaction.response[0] = 0xff; + self.transaction.response_len = 1; + self.transaction.response_pos = 0; + self.transaction.data_out = 0; + self.transaction.status |= STATUS_OBF; + self.transaction.set_state(KCS_STATE_READ); + } + + fn enter_error_state(&mut self) { + self.transaction.clear_buffers(); + self.transaction.data_out = 0xff; + self.transaction.status |= STATUS_OBF; + self.transaction.set_state(KCS_STATE_ERROR); + } +} + +#[cfg(test)] +mod tests; diff --git a/vm/devices/chipset/ipmi_kcs/src/protocol.rs b/vm/devices/chipset/ipmi_kcs/src/protocol.rs new file mode 100644 index 0000000000..d9ea47b91e --- /dev/null +++ b/vm/devices/chipset/ipmi_kcs/src/protocol.rs @@ -0,0 +1,107 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +use crate::IpmiKcs; +use crate::KCS_MESSAGE_MAX; +use crate::KCS_STATE_READ; +use crate::STATUS_OBF; + +pub(crate) const NETFN_APPLICATION: u8 = 0x06; +pub(crate) const NETFN_STORAGE: u8 = 0x0a; + +pub(crate) const COMMAND_GET_DEVICE_ID: u8 = 0x01; +pub(crate) const COMMAND_GET_SEL_INFO: u8 = 0x40; +pub(crate) const COMMAND_RESERVE_SEL: u8 = 0x42; +pub(crate) const COMMAND_GET_SEL_ENTRY: u8 = 0x43; +pub(crate) const COMMAND_ADD_SEL_ENTRY: u8 = 0x44; +pub(crate) const COMMAND_CLEAR_SEL: u8 = 0x47; +pub(crate) const COMMAND_GET_SEL_TIME: u8 = 0x48; +pub(crate) const COMMAND_SET_SEL_TIME: u8 = 0x49; + +pub(crate) const COMPLETION_SUCCESS: u8 = 0x00; +pub(crate) const COMPLETION_INVALID_COMMAND: u8 = 0xc1; +pub(crate) const COMPLETION_SEL_FULL: u8 = 0xc4; +pub(crate) const COMPLETION_RESERVATION_CANCELED: u8 = 0xc5; +pub(crate) const COMPLETION_INVALID_REQUEST_LENGTH: u8 = 0xc7; +pub(crate) const COMPLETION_PARAMETER_OUT_OF_RANGE: u8 = 0xc9; +pub(crate) const COMPLETION_RECORD_NOT_PRESENT: u8 = 0xcb; +pub(crate) const COMPLETION_INVALID_DATA_FIELD: u8 = 0xcc; + +impl IpmiKcs { + pub(crate) fn process_ipmi_message(&mut self) { + if self.transaction.request_len < 2 { + self.enter_error_state(); + return; + } + + let request = self.transaction.request; + let request_len = self.transaction.request_len; + let netfn_lun = request[0]; + let command = request[1]; + let data = &request[2..request_len]; + let mut body = [0; KCS_MESSAGE_MAX]; + + let body_len = match netfn_lun >> 2 { + NETFN_APPLICATION => self.handle_application_command(command, data, &mut body), + NETFN_STORAGE => self.handle_sel_command(command, data, &mut body), + _ => { + body[0] = COMPLETION_INVALID_COMMAND; + 1 + } + }; + + self.stage_response(netfn_lun, command, &body[..body_len]); + } + + fn stage_response(&mut self, request_netfn_lun: u8, command: u8, body: &[u8]) { + self.transaction.response.fill(0); + self.transaction.response[0] = request_netfn_lun | 0x04; + self.transaction.response[1] = command; + + let body_len = body.len().min(KCS_MESSAGE_MAX - 2); + self.transaction.response[2..2 + body_len].copy_from_slice(&body[..body_len]); + self.transaction.response_len = body_len + 2; + self.transaction.response_pos = 1; + self.transaction.data_out = self.transaction.response[0]; + self.transaction.status |= STATUS_OBF; + self.transaction.set_state(KCS_STATE_READ); + } + + fn handle_application_command( + &mut self, + command: u8, + _data: &[u8], + out: &mut [u8; KCS_MESSAGE_MAX], + ) -> usize { + match command { + COMMAND_GET_DEVICE_ID => { + const RESPONSE: [u8; 12] = [ + COMPLETION_SUCCESS, + 0x20, + 0x01, + 0x02, + 0x00, + 0x02, + 0x04, + 0x00, + 0x00, + 0x00, + 0x00, + 0x00, + ]; + out[..RESPONSE.len()].copy_from_slice(&RESPONSE); + RESPONSE.len() + } + _ => completion(out, COMPLETION_INVALID_COMMAND), + } + } +} + +pub(crate) fn completion(out: &mut [u8; KCS_MESSAGE_MAX], code: u8) -> usize { + out[0] = code; + 1 +} + +pub(crate) fn invalid_length(out: &mut [u8; KCS_MESSAGE_MAX]) -> usize { + completion(out, COMPLETION_INVALID_REQUEST_LENGTH) +} diff --git a/vm/devices/chipset/ipmi_kcs/src/save_restore.rs b/vm/devices/chipset/ipmi_kcs/src/save_restore.rs new file mode 100644 index 0000000000..08eadceb36 --- /dev/null +++ b/vm/devices/chipset/ipmi_kcs/src/save_restore.rs @@ -0,0 +1,243 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +use crate::IpmiKcs; +use crate::KCS_MESSAGE_MAX; +use crate::KCS_STATE_ERROR; +use crate::KCS_STATE_IDLE; +use crate::KCS_STATE_READ; +use crate::KCS_STATE_WRITE; +use crate::KcsTransaction; +use crate::STATUS_IBF; +use crate::STATUS_STATE_MASK; +use crate::SelStats; +use crate::sel::SEL_CAPACITY; +use crate::sel::SEL_RECORD_SIZE; +use crate::sel::SelState; +use mesh::payload::Protobuf; +use vmcore::save_restore::RestoreError; +use vmcore::save_restore::SaveError; +use vmcore::save_restore::SaveRestore; +use vmcore::save_restore::SavedStateRoot; + +/// Serializable state for [`IpmiKcs`]. +#[derive(Clone, Debug, Protobuf, SavedStateRoot)] +#[mesh(package = "chipset.ipmi_kcs")] +pub struct SavedState { + #[mesh(1)] + pub(crate) status: u8, + #[mesh(2)] + pub(crate) data_out: u8, + #[mesh(3)] + pub(crate) request: Vec, + #[mesh(4)] + pub(crate) response: Vec, + #[mesh(5)] + pub(crate) response_position: u32, + #[mesh(6)] + pub(crate) write_end_pending: bool, + #[mesh(7)] + pub(crate) sel_records: Vec>, + #[mesh(8)] + pub(crate) sel_count: u32, + #[mesh(9)] + pub(crate) next_record_id: u16, + #[mesh(10)] + pub(crate) reservation_id: u16, + #[mesh(11)] + pub(crate) time_offset_seconds: i64, + #[mesh(12)] + pub(crate) last_erase_timestamp: u32, +} + +#[derive(Debug, thiserror::Error)] +enum SavedStateValidationError { + #[error("status contains unsupported bits: {0:#04x}")] + UnsupportedStatusBits(u8), + #[error("invalid KCS state: {0:#04x}")] + InvalidKcsState(u8), + #[error("request length {0} exceeds {KCS_MESSAGE_MAX}")] + RequestTooLong(usize), + #[error("response length {0} exceeds {KCS_MESSAGE_MAX}")] + ResponseTooLong(usize), + #[error("response position {position} exceeds response length {length}")] + InvalidResponsePosition { position: usize, length: usize }, + #[error("read state has no response")] + EmptyReadResponse, + #[error("write-end-pending is set outside write state")] + InvalidWriteEndPending, + #[error("SEL count {count} does not match {records} saved records")] + SelCountMismatch { count: usize, records: usize }, + #[error("SEL count {0} exceeds {SEL_CAPACITY}")] + TooManySelRecords(usize), + #[error("SEL record {index} has length {length}, expected {SEL_RECORD_SIZE}")] + InvalidSelRecordLength { index: usize, length: usize }, + #[error("SEL record {index} has reserved record ID {record_id:#06x}")] + InvalidSelRecordId { index: usize, record_id: u16 }, + #[error("SEL contains duplicate record ID {0:#06x}")] + DuplicateSelRecordId(u16), + #[error("next SEL record ID is reserved: {0:#06x}")] + InvalidNextRecordId(u16), + #[error("next SEL record ID {0:#06x} is already present")] + DuplicateNextRecordId(u16), + #[error("saved numeric field does not fit on this host")] + NumericOverflow, +} + +impl SaveRestore for IpmiKcs { + type SavedState = SavedState; + + fn save(&mut self) -> Result { + Ok(SavedState { + status: self.transaction.status, + data_out: self.transaction.data_out, + request: self.transaction.request[..self.transaction.request_len].to_vec(), + response: self.transaction.response[..self.transaction.response_len].to_vec(), + response_position: self.transaction.response_pos as u32, + write_end_pending: self.transaction.write_end_pending, + sel_records: self + .sel + .records + .iter() + .map(|record| record.to_vec()) + .collect(), + sel_count: self.sel.records.len() as u32, + next_record_id: self.sel.next_record_id, + reservation_id: self.sel.reservation_id, + time_offset_seconds: self.sel.time_offset_seconds, + last_erase_timestamp: self.sel.last_erase_timestamp, + }) + } + + fn restore(&mut self, state: Self::SavedState) -> Result<(), RestoreError> { + let restored = validate_saved_state(state) + .map_err(|error| RestoreError::InvalidSavedState(error.into()))?; + + self.transaction = restored.transaction; + self.sel = restored.sel; + self.rate_limiter.reset(); + self.stats = SelStats::default(); + Ok(()) + } +} + +struct RestoredState { + transaction: KcsTransaction, + sel: SelState, +} + +fn validate_saved_state(state: SavedState) -> Result { + let SavedState { + status, + data_out, + request, + response, + response_position, + write_end_pending, + sel_records, + sel_count, + next_record_id, + reservation_id, + time_offset_seconds, + last_erase_timestamp, + } = state; + + if status & STATUS_IBF != 0 || status & 0x30 != 0 { + return Err(SavedStateValidationError::UnsupportedStatusBits(status)); + } + let kcs_state = status & STATUS_STATE_MASK; + if !matches!( + kcs_state, + KCS_STATE_IDLE | KCS_STATE_READ | KCS_STATE_WRITE | KCS_STATE_ERROR + ) { + return Err(SavedStateValidationError::InvalidKcsState(kcs_state)); + } + if request.len() > KCS_MESSAGE_MAX { + return Err(SavedStateValidationError::RequestTooLong(request.len())); + } + if response.len() > KCS_MESSAGE_MAX { + return Err(SavedStateValidationError::ResponseTooLong(response.len())); + } + let response_position = usize::try_from(response_position) + .map_err(|_| SavedStateValidationError::NumericOverflow)?; + if response_position > response.len() { + return Err(SavedStateValidationError::InvalidResponsePosition { + position: response_position, + length: response.len(), + }); + } + if kcs_state == KCS_STATE_READ && response.is_empty() { + return Err(SavedStateValidationError::EmptyReadResponse); + } + if write_end_pending && kcs_state != KCS_STATE_WRITE { + return Err(SavedStateValidationError::InvalidWriteEndPending); + } + + let sel_count = + usize::try_from(sel_count).map_err(|_| SavedStateValidationError::NumericOverflow)?; + if sel_count != sel_records.len() { + return Err(SavedStateValidationError::SelCountMismatch { + count: sel_count, + records: sel_records.len(), + }); + } + if sel_count > SEL_CAPACITY { + return Err(SavedStateValidationError::TooManySelRecords(sel_count)); + } + + let mut records = Vec::with_capacity(sel_count); + for (index, record) in sel_records.into_iter().enumerate() { + let length = record.len(); + let Ok(record) = <[u8; SEL_RECORD_SIZE]>::try_from(record) else { + return Err(SavedStateValidationError::InvalidSelRecordLength { index, length }); + }; + let record_id = u16::from_le_bytes([record[0], record[1]]); + if record_id == 0 || record_id == 0xffff { + return Err(SavedStateValidationError::InvalidSelRecordId { index, record_id }); + } + if records + .iter() + .any(|existing: &[u8; SEL_RECORD_SIZE]| existing[0..2] == record[0..2]) + { + return Err(SavedStateValidationError::DuplicateSelRecordId(record_id)); + } + records.push(record); + } + + if next_record_id == 0 || next_record_id == 0xffff { + return Err(SavedStateValidationError::InvalidNextRecordId( + next_record_id, + )); + } + if records + .iter() + .any(|record| u16::from_le_bytes([record[0], record[1]]) == next_record_id) + { + return Err(SavedStateValidationError::DuplicateNextRecordId( + next_record_id, + )); + } + + let mut transaction = KcsTransaction { + status, + data_out, + request_len: request.len(), + response_len: response.len(), + response_pos: response_position, + write_end_pending, + ..KcsTransaction::default() + }; + transaction.request[..request.len()].copy_from_slice(&request); + transaction.response[..response.len()].copy_from_slice(&response); + + Ok(RestoredState { + transaction, + sel: SelState { + records, + next_record_id, + reservation_id, + time_offset_seconds, + last_erase_timestamp, + }, + }) +} diff --git a/vm/devices/chipset/ipmi_kcs/src/sel.rs b/vm/devices/chipset/ipmi_kcs/src/sel.rs new file mode 100644 index 0000000000..bffd624b0a --- /dev/null +++ b/vm/devices/chipset/ipmi_kcs/src/sel.rs @@ -0,0 +1,302 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +use crate::IpmiKcs; +use crate::KCS_MESSAGE_MAX; +use crate::SelEventDisposition; +use crate::protocol::COMMAND_ADD_SEL_ENTRY; +use crate::protocol::COMMAND_CLEAR_SEL; +use crate::protocol::COMMAND_GET_SEL_ENTRY; +use crate::protocol::COMMAND_GET_SEL_INFO; +use crate::protocol::COMMAND_GET_SEL_TIME; +use crate::protocol::COMMAND_RESERVE_SEL; +use crate::protocol::COMMAND_SET_SEL_TIME; +use crate::protocol::COMPLETION_INVALID_COMMAND; +use crate::protocol::COMPLETION_INVALID_DATA_FIELD; +use crate::protocol::COMPLETION_PARAMETER_OUT_OF_RANGE; +use crate::protocol::COMPLETION_RECORD_NOT_PRESENT; +use crate::protocol::COMPLETION_SEL_FULL; +use crate::protocol::COMPLETION_SUCCESS; +use crate::protocol::completion; +use crate::protocol::invalid_length; + +pub(crate) const SEL_RECORD_SIZE: usize = 16; +pub(crate) const SEL_CAPACITY: usize = 128; +const SEL_VERSION: u8 = 0x51; +const SEL_FORWARD_LIMIT: u32 = 256; + +#[derive(Default)] +pub(crate) struct SelState { + pub(crate) records: Vec<[u8; SEL_RECORD_SIZE]>, + pub(crate) next_record_id: u16, + pub(crate) reservation_id: u16, + pub(crate) time_offset_seconds: i64, + pub(crate) last_erase_timestamp: u32, +} + +impl SelState { + pub(crate) fn new() -> Self { + Self { + next_record_id: 1, + ..Self::default() + } + } +} + +#[derive(Default)] +pub(crate) struct RateLimiter { + window_second: Option, + forwarded_in_window: u32, +} + +impl RateLimiter { + fn allow(&mut self, trusted_second: i64) -> bool { + if self.window_second.is_none() + || trusted_second.saturating_sub(self.window_second.unwrap_or(trusted_second)) >= 1 + { + self.window_second = Some(trusted_second); + self.forwarded_in_window = 0; + } + + if self.forwarded_in_window >= SEL_FORWARD_LIMIT { + false + } else { + self.forwarded_in_window += 1; + true + } + } + + pub(crate) fn reset(&mut self) { + self.window_second = None; + self.forwarded_in_window = 0; + } +} + +impl IpmiKcs { + pub(crate) fn handle_sel_command( + &mut self, + command: u8, + data: &[u8], + out: &mut [u8; KCS_MESSAGE_MAX], + ) -> usize { + match command { + COMMAND_GET_SEL_INFO => self.get_sel_info(out), + COMMAND_RESERVE_SEL => self.reserve_sel(out), + COMMAND_GET_SEL_ENTRY => self.get_sel_entry(data, out), + COMMAND_ADD_SEL_ENTRY => self.add_sel_entry(data, out), + COMMAND_CLEAR_SEL => self.clear_sel(data, out), + COMMAND_GET_SEL_TIME => self.get_sel_time(out), + COMMAND_SET_SEL_TIME => self.set_sel_time(data, out), + _ => completion(out, COMPLETION_INVALID_COMMAND), + } + } + + fn get_sel_info(&mut self, out: &mut [u8; KCS_MESSAGE_MAX]) -> usize { + let count = self.sel.records.len() as u16; + let free_bytes = ((SEL_CAPACITY - self.sel.records.len()) * SEL_RECORD_SIZE) as u16; + let last_addition_timestamp = self + .sel + .records + .last() + .map(|record| u32::from_le_bytes([record[3], record[4], record[5], record[6]])) + .unwrap_or(0); + let mut pos = 0; + out[pos] = COMPLETION_SUCCESS; + pos += 1; + out[pos] = SEL_VERSION; + pos += 1; + put_u16(out, &mut pos, count); + put_u16(out, &mut pos, free_bytes); + put_u32(out, &mut pos, last_addition_timestamp); + put_u32(out, &mut pos, self.sel.last_erase_timestamp); + out[pos] = 0x02; + pos + 1 + } + + fn reserve_sel(&mut self, out: &mut [u8; KCS_MESSAGE_MAX]) -> usize { + self.sel.reservation_id = self.sel.reservation_id.wrapping_add(1); + if self.sel.reservation_id == 0 { + self.sel.reservation_id = 1; + } + + out[0] = COMPLETION_SUCCESS; + out[1..3].copy_from_slice(&self.sel.reservation_id.to_le_bytes()); + 3 + } + + fn get_sel_entry(&mut self, data: &[u8], out: &mut [u8; KCS_MESSAGE_MAX]) -> usize { + let Some(data) = data.get(..6) else { + return invalid_length(out); + }; + + let offset = usize::from(data[4]); + if offset >= SEL_RECORD_SIZE { + return completion(out, COMPLETION_PARAMETER_OUT_OF_RANGE); + } + + let record_id = u16::from_le_bytes([data[2], data[3]]); + let Some(index) = self.find_record(record_id) else { + return completion(out, COMPLETION_RECORD_NOT_PRESENT); + }; + + let next_record_id = self + .sel + .records + .get(index + 1) + .map(|record| u16::from_le_bytes([record[0], record[1]])) + .unwrap_or(0xffff); + let end = offset + .saturating_add(usize::from(data[5])) + .min(SEL_RECORD_SIZE); + let record = &self.sel.records[index]; + let bytes = &record[offset..end]; + + out[0] = COMPLETION_SUCCESS; + out[1..3].copy_from_slice(&next_record_id.to_le_bytes()); + out[3..3 + bytes.len()].copy_from_slice(bytes); + 3 + bytes.len() + } + + fn add_sel_entry(&mut self, data: &[u8], out: &mut [u8; KCS_MESSAGE_MAX]) -> usize { + let Some(data) = data.get(..SEL_RECORD_SIZE) else { + return invalid_length(out); + }; + let mut record = [0; SEL_RECORD_SIZE]; + record.copy_from_slice(data); + + if self.sel.records.len() >= SEL_CAPACITY { + return completion(out, COMPLETION_SEL_FULL); + } + + let Some(record_id) = self.allocate_record_id() else { + return completion(out, COMPLETION_SEL_FULL); + }; + let trusted_seconds = self.clock.unix_seconds(); + let timestamp = adjusted_timestamp(trusted_seconds, self.sel.time_offset_seconds); + record[0..2].copy_from_slice(&record_id.to_le_bytes()); + record[3..7].copy_from_slice(×tamp.to_le_bytes()); + + self.sel.records.push(record); + self.stats.committed = self.stats.committed.saturating_add(1); + + if let Some(sink) = self.sink.as_mut() { + if self.rate_limiter.allow(trusted_seconds) { + match sink.try_send(record_id, record) { + SelEventDisposition::Accepted => { + self.stats.forwarded = self.stats.forwarded.saturating_add(1); + } + SelEventDisposition::Dropped => { + self.stats.sink_dropped = self.stats.sink_dropped.saturating_add(1); + } + } + } else { + self.stats.rate_limited = self.stats.rate_limited.saturating_add(1); + } + } + + out[0] = COMPLETION_SUCCESS; + out[1..3].copy_from_slice(&record_id.to_le_bytes()); + 3 + } + + fn clear_sel(&mut self, data: &[u8], out: &mut [u8; KCS_MESSAGE_MAX]) -> usize { + let Some(data) = data.get(..6) else { + return invalid_length(out); + }; + + if data[2..5] != *b"CLR" { + return completion(out, COMPLETION_INVALID_DATA_FIELD); + } + + match data[5] { + 0xaa => { + self.sel.records.clear(); + self.sel.next_record_id = 1; + let trusted_seconds = self.clock.unix_seconds(); + self.sel.last_erase_timestamp = + adjusted_timestamp(trusted_seconds, self.sel.time_offset_seconds); + } + 0x00 => {} + _ => return completion(out, COMPLETION_INVALID_DATA_FIELD), + } + + out[0] = COMPLETION_SUCCESS; + out[1] = 1; + 2 + } + + fn get_sel_time(&mut self, out: &mut [u8; KCS_MESSAGE_MAX]) -> usize { + let trusted_seconds = self.clock.unix_seconds(); + out[0] = COMPLETION_SUCCESS; + out[1..5].copy_from_slice( + &adjusted_timestamp(trusted_seconds, self.sel.time_offset_seconds).to_le_bytes(), + ); + 5 + } + + fn set_sel_time(&mut self, data: &[u8], out: &mut [u8; KCS_MESSAGE_MAX]) -> usize { + let Some(data) = data.get(..4) else { + return invalid_length(out); + }; + + let requested = i64::from(u32::from_le_bytes([data[0], data[1], data[2], data[3]])); + self.sel.time_offset_seconds = requested.saturating_sub(self.clock.unix_seconds()); + completion(out, COMPLETION_SUCCESS) + } + + fn find_record(&self, record_id: u16) -> Option { + match record_id { + 0 => (!self.sel.records.is_empty()).then_some(0), + 0xffff => self.sel.records.len().checked_sub(1), + _ => self + .sel + .records + .iter() + .position(|record| u16::from_le_bytes([record[0], record[1]]) == record_id), + } + } + + fn allocate_record_id(&mut self) -> Option { + let mut candidate = normalize_record_id(self.sel.next_record_id); + for _ in 0..=SEL_CAPACITY { + let used = self + .sel + .records + .iter() + .any(|record| u16::from_le_bytes([record[0], record[1]]) == candidate); + if !used { + self.sel.next_record_id = increment_record_id(candidate); + return Some(candidate); + } + candidate = increment_record_id(candidate); + } + None + } +} + +fn normalize_record_id(record_id: u16) -> u16 { + if record_id == 0 || record_id == 0xffff { + 1 + } else { + record_id + } +} + +fn increment_record_id(record_id: u16) -> u16 { + normalize_record_id(record_id.wrapping_add(1)) +} + +fn adjusted_timestamp(trusted_seconds: i64, offset_seconds: i64) -> u32 { + let adjusted = trusted_seconds.saturating_add(offset_seconds); + if adjusted < 0 { 0 } else { adjusted as u32 } +} + +fn put_u16(out: &mut [u8; KCS_MESSAGE_MAX], pos: &mut usize, value: u16) { + out[*pos..*pos + 2].copy_from_slice(&value.to_le_bytes()); + *pos += 2; +} + +fn put_u32(out: &mut [u8; KCS_MESSAGE_MAX], pos: &mut usize, value: u32) { + out[*pos..*pos + 4].copy_from_slice(&value.to_le_bytes()); + *pos += 4; +} diff --git a/vm/devices/chipset/ipmi_kcs/src/tests.rs b/vm/devices/chipset/ipmi_kcs/src/tests.rs new file mode 100644 index 0000000000..e8c87d93f7 --- /dev/null +++ b/vm/devices/chipset/ipmi_kcs/src/tests.rs @@ -0,0 +1,889 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +use super::*; +use crate::protocol::*; +use std::cell::Cell; +use std::cell::RefCell; +use std::rc::Rc; +use test_with_tracing::test; +use vmcore::save_restore::RestoreError; +use vmcore::save_restore::SaveRestore; +use vmcore::save_restore::SavedStateBlob; + +const APPLICATION_REQUEST: u8 = NETFN_APPLICATION << 2; +const STORAGE_REQUEST: u8 = NETFN_STORAGE << 2; + +#[derive(Clone)] +struct FakeClock(Rc>); + +impl FakeClock { + fn new(seconds: i64) -> Self { + Self(Rc::new(Cell::new(seconds))) + } + + fn set(&self, seconds: i64) { + self.0.set(seconds); + } +} + +impl TrustedClock for FakeClock { + fn unix_seconds(&mut self) -> i64 { + self.0.get() + } +} + +#[derive(Default)] +struct SinkState { + records: Vec<(u16, [u8; 16])>, +} + +struct SharedSink { + state: Rc>, + accept: Rc>, +} + +impl SelEventSink for SharedSink { + fn try_send(&mut self, record_id: u16, record: [u8; 16]) -> SelEventDisposition { + if !self.accept.get() { + return SelEventDisposition::Dropped; + } + self.state.borrow_mut().records.push((record_id, record)); + SelEventDisposition::Accepted + } +} + +fn device(seconds: i64) -> (FakeClock, IpmiKcs) { + let clock = FakeClock::new(seconds); + (clock.clone(), IpmiKcs::new(clock)) +} + +fn storage_request(command: u8, data: &[u8]) -> Vec { + let mut request = vec![STORAGE_REQUEST, command]; + request.extend_from_slice(data); + request +} + +fn submit_request(device: &mut IpmiKcs, request: &[u8]) { + assert!(!request.is_empty()); + device.write_command(KCS_COMMAND_WRITE_START); + assert_eq!(device.read_status() & STATUS_STATE_MASK, KCS_STATE_WRITE); + assert_eq!(device.read_data(), 0); + + for byte in &request[..request.len() - 1] { + device.write_data(*byte); + assert_eq!(device.read_status() & STATUS_STATE_MASK, KCS_STATE_WRITE); + assert_eq!(device.read_data(), 0); + } + + device.write_command(KCS_COMMAND_WRITE_END); + assert_eq!(device.read_data(), 0); + device.write_data(request[request.len() - 1]); +} + +fn read_response(device: &mut IpmiKcs) -> Vec { + let mut response = Vec::new(); + loop { + assert_eq!(device.read_status() & STATUS_STATE_MASK, KCS_STATE_READ); + assert_ne!(device.read_status() & STATUS_OBF, 0); + response.push(device.read_data()); + assert_eq!(device.read_status() & STATUS_OBF, 0); + + device.write_data(KCS_DATA_READ_NEXT); + if device.read_status() & STATUS_STATE_MASK == KCS_STATE_IDLE { + assert_ne!(device.read_status() & STATUS_OBF, 0); + assert_eq!(device.read_data(), 0); + break; + } + } + response +} + +fn transact(device: &mut IpmiKcs, request: &[u8]) -> Vec { + submit_request(device, request); + read_response(device) +} + +fn assert_completion(response: &[u8], request_netfn_lun: u8, command: u8, completion: u8) { + assert_eq!( + response.get(..3), + Some([request_netfn_lun | 0x04, command, completion].as_slice()) + ); +} + +fn add_record(device: &mut IpmiKcs, fill: u8) -> Vec { + transact(device, &storage_request(COMMAND_ADD_SEL_ENTRY, &[fill; 16])) +} + +fn clear(device: &mut IpmiKcs, reservation: u16, action: u8) -> Vec { + let mut data = Vec::from(reservation.to_le_bytes()); + data.extend_from_slice(b"CLR"); + data.push(action); + transact(device, &storage_request(COMMAND_CLEAR_SEL, &data)) +} + +#[test] +fn kcs_transitions_and_get_device_id() { + let (_, mut device) = device(100); + assert_eq!(device.read_status(), KCS_STATE_IDLE); + + device.write_command(KCS_COMMAND_WRITE_START); + let status = device.read_status(); + assert_eq!(status & STATUS_STATE_MASK, KCS_STATE_WRITE); + assert_ne!(status & STATUS_OBF, 0); + assert_ne!(status & STATUS_CD, 0); + assert_eq!(status & STATUS_IBF, 0); + assert_eq!(device.read_data(), 0); + + device.write_data(APPLICATION_REQUEST); + let status = device.read_status(); + assert_eq!(status & STATUS_STATE_MASK, KCS_STATE_WRITE); + assert_ne!(status & STATUS_OBF, 0); + assert_eq!(status & STATUS_CD, 0); + assert_eq!(device.read_data(), 0); + + device.write_command(KCS_COMMAND_WRITE_END); + assert_eq!(device.read_status() & STATUS_STATE_MASK, KCS_STATE_WRITE); + assert_ne!(device.read_status() & STATUS_CD, 0); + assert_eq!(device.read_data(), 0); + + device.write_data(COMMAND_GET_DEVICE_ID); + assert_eq!(device.read_status() & STATUS_STATE_MASK, KCS_STATE_READ); + assert_eq!(device.read_status() & STATUS_CD, 0); + let response = read_response(&mut device); + assert_eq!( + response, + [ + APPLICATION_REQUEST | 0x04, + COMMAND_GET_DEVICE_ID, + COMPLETION_SUCCESS, + 0x20, + 0x01, + 0x02, + 0x00, + 0x02, + 0x04, + 0x00, + 0x00, + 0x00, + 0x00, + 0x00, + ] + ); +} + +#[test] +fn abort_exposes_status_byte_then_returns_idle() { + let (_, mut device) = device(0); + device.write_command(KCS_COMMAND_WRITE_START); + assert_eq!(device.read_data(), 0); + device.write_data(APPLICATION_REQUEST); + assert_eq!(device.read_data(), 0); + + device.write_command(KCS_COMMAND_GET_STATUS_ABORT); + assert_eq!(device.read_status() & STATUS_STATE_MASK, KCS_STATE_READ); + assert_ne!(device.read_status() & STATUS_CD, 0); + assert_eq!(device.read_data(), 0); + assert_eq!(device.read_status() & STATUS_OBF, 0); + + device.write_data(KCS_DATA_READ_NEXT); + assert_eq!(device.read_status() & STATUS_STATE_MASK, KCS_STATE_READ); + assert_eq!(device.read_data(), 0xff); + + device.write_data(KCS_DATA_READ_NEXT); + assert_eq!(device.read_status() & STATUS_STATE_MASK, KCS_STATE_IDLE); + assert_eq!(device.read_status() & STATUS_CD, 0); + assert_eq!(device.read_data(), 0); +} + +#[test] +fn malformed_kcs_sequences_match_legacy_behavior_without_panicking() { + let (_, mut device) = device(0); + + device.write_data(0); + assert_eq!(device.read_status() & STATUS_STATE_MASK, KCS_STATE_IDLE); + + device.reset(); + device.write_command(KCS_DATA_READ_NEXT); + assert_eq!(device.read_status() & STATUS_STATE_MASK, KCS_STATE_IDLE); + + device.reset(); + device.write_command(0xff); + assert_eq!(device.read_status() & STATUS_STATE_MASK, KCS_STATE_READ); + assert_eq!(device.read_data(), 0); + device.write_data(KCS_DATA_READ_NEXT); + assert_eq!(device.read_data(), 0xff); + + device.reset(); + device.write_command(KCS_COMMAND_WRITE_END); + assert_eq!(device.read_status() & STATUS_STATE_MASK, KCS_STATE_WRITE); + assert_eq!(device.read_data(), 0); + + device.reset(); + submit_request(&mut device, &[APPLICATION_REQUEST]); + assert_eq!(device.read_status() & STATUS_STATE_MASK, KCS_STATE_ERROR); + + device.reset(); + submit_request(&mut device, &[APPLICATION_REQUEST, COMMAND_GET_DEVICE_ID]); + assert_eq!(device.read_data(), APPLICATION_REQUEST | 0x04); + device.write_data(0x69); + assert_eq!(device.read_status() & STATUS_STATE_MASK, KCS_STATE_ERROR); + assert_eq!(device.read_data(), 0xff); +} + +#[test] +fn request_buffer_boundaries_are_safe() { + let (_, mut device) = device(0); + for length in [63, 64] { + let response = transact(&mut device, &vec![0; length]); + assert_completion(&response, 0, 0, COMPLETION_INVALID_COMMAND); + } + + device.write_command(KCS_COMMAND_WRITE_START); + assert_eq!(device.read_data(), 0); + for _ in 0..64 { + device.write_data(0); + assert_eq!(device.read_status() & STATUS_STATE_MASK, KCS_STATE_WRITE); + assert_eq!(device.read_data(), 0); + } + device.write_command(KCS_COMMAND_WRITE_END); + assert_eq!(device.read_data(), 0); + device.write_data(0); + assert_eq!(device.read_status() & STATUS_STATE_MASK, KCS_STATE_READ); + let response = read_response(&mut device); + assert_completion(&response, 0, 0, COMPLETION_INVALID_COMMAND); +} + +#[test] +fn command_lengths_and_unknown_commands_return_completion_codes() { + let (_, mut device) = device(0); + + let response = transact(&mut device, &[APPLICATION_REQUEST, 0xfe]); + assert_completion( + &response, + APPLICATION_REQUEST, + 0xfe, + COMPLETION_INVALID_COMMAND, + ); + let response = transact(&mut device, &storage_request(0xfe, &[])); + assert_completion(&response, STORAGE_REQUEST, 0xfe, COMPLETION_INVALID_COMMAND); + let response = transact( + &mut device, + &[APPLICATION_REQUEST, COMMAND_GET_DEVICE_ID, 0], + ); + assert_completion( + &response, + APPLICATION_REQUEST, + COMMAND_GET_DEVICE_ID, + COMPLETION_SUCCESS, + ); + + for command in [ + COMMAND_GET_SEL_INFO, + COMMAND_RESERVE_SEL, + COMMAND_GET_SEL_TIME, + ] { + let response = transact(&mut device, &storage_request(command, &[0])); + assert_completion(&response, STORAGE_REQUEST, command, COMPLETION_SUCCESS); + } + + for (command, valid_length) in [ + (COMMAND_GET_SEL_ENTRY, 6), + (COMMAND_ADD_SEL_ENTRY, 16), + (COMMAND_CLEAR_SEL, 6), + (COMMAND_SET_SEL_TIME, 4), + ] { + let response = transact( + &mut device, + &storage_request(command, &vec![0; valid_length - 1]), + ); + assert_completion( + &response, + STORAGE_REQUEST, + command, + COMPLETION_INVALID_REQUEST_LENGTH, + ); + } + + let response = transact( + &mut device, + &storage_request(COMMAND_GET_SEL_ENTRY, &[0, 0, 0, 0, 0, 0, 0xff]), + ); + assert_completion( + &response, + STORAGE_REQUEST, + COMMAND_GET_SEL_ENTRY, + COMPLETION_RECORD_NOT_PRESENT, + ); + + for (command, data) in [ + (COMMAND_ADD_SEL_ENTRY, vec![0; 17]), + (COMMAND_CLEAR_SEL, vec![0, 0, b'C', b'L', b'R', 0, 0xff]), + (COMMAND_SET_SEL_TIME, vec![0; 5]), + ] { + let response = transact(&mut device, &storage_request(command, &data)); + assert_completion(&response, STORAGE_REQUEST, command, COMPLETION_SUCCESS); + } +} + +#[test] +fn sel_info_add_and_record_preservation() { + let (_, mut device) = device(0x0102_0304); + + let info = transact(&mut device, &storage_request(COMMAND_GET_SEL_INFO, &[])); + assert_eq!( + info, + [ + STORAGE_REQUEST | 0x04, + COMMAND_GET_SEL_INFO, + COMPLETION_SUCCESS, + 0x51, + 0, + 0, + 0, + 8, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + 0x02, + ] + ); + + let mut record = [0xa5; 16]; + record[2] = 0x02; + record[7] = 0x77; + let response = transact( + &mut device, + &storage_request(COMMAND_ADD_SEL_ENTRY, &record), + ); + assert_eq!( + response, + [ + STORAGE_REQUEST | 0x04, + COMMAND_ADD_SEL_ENTRY, + COMPLETION_SUCCESS, + 1, + 0, + ] + ); + + let stored = device.sel_record(0).unwrap(); + assert_eq!(&stored[0..2], &1u16.to_le_bytes()); + assert_eq!(stored[2], 0x02); + assert_eq!(&stored[3..7], &0x0102_0304u32.to_le_bytes()); + assert_eq!(stored[7], 0x77); + assert_eq!(&stored[8..], &record[8..]); + + let info = transact(&mut device, &storage_request(COMMAND_GET_SEL_INFO, &[])); + assert_eq!(u16::from_le_bytes([info[4], info[5]]), 1); + assert_eq!(u16::from_le_bytes([info[6], info[7]]), 2032); + assert_eq!( + u32::from_le_bytes([info[8], info[9], info[10], info[11]]), + 0x0102_0304 + ); +} + +#[test] +fn sel_entry_ids_sentinels_partial_reads_and_reservations() { + let (_, mut device) = device(10); + let empty = transact( + &mut device, + &storage_request(COMMAND_GET_SEL_ENTRY, &[0, 0, 0, 0, 0, 16]), + ); + assert_completion( + &empty, + STORAGE_REQUEST, + COMMAND_GET_SEL_ENTRY, + COMPLETION_RECORD_NOT_PRESENT, + ); + + assert_eq!(add_record(&mut device, 0x11)[3..5], [1, 0]); + assert_eq!(add_record(&mut device, 0x22)[3..5], [2, 0]); + + let reserve_one = transact(&mut device, &storage_request(COMMAND_RESERVE_SEL, &[])); + let reservation_one = u16::from_le_bytes([reserve_one[3], reserve_one[4]]); + let reserve_two = transact(&mut device, &storage_request(COMMAND_RESERVE_SEL, &[])); + let reservation_two = u16::from_le_bytes([reserve_two[3], reserve_two[4]]); + assert_eq!(reservation_one, 1); + assert_eq!(reservation_two, 2); + + let mut old_reservation = Vec::from(reservation_one.to_le_bytes()); + old_reservation.extend_from_slice(&[0, 0, 0, 16]); + let response = transact( + &mut device, + &storage_request(COMMAND_GET_SEL_ENTRY, &old_reservation), + ); + assert_completion( + &response, + STORAGE_REQUEST, + COMMAND_GET_SEL_ENTRY, + COMPLETION_SUCCESS, + ); + + let mut first_partial = Vec::from(0u16.to_le_bytes()); + first_partial.extend_from_slice(&0u16.to_le_bytes()); + first_partial.extend_from_slice(&[7, 4]); + let response = transact( + &mut device, + &storage_request(COMMAND_GET_SEL_ENTRY, &first_partial), + ); + assert_completion( + &response, + STORAGE_REQUEST, + COMMAND_GET_SEL_ENTRY, + COMPLETION_SUCCESS, + ); + assert_eq!(&response[3..5], &2u16.to_le_bytes()); + assert_eq!(&response[5..], &[0x11; 4]); + + let mut last = Vec::from(reservation_two.to_le_bytes()); + last.extend_from_slice(&0xffffu16.to_le_bytes()); + last.extend_from_slice(&[0, 0xff]); + let response = transact(&mut device, &storage_request(COMMAND_GET_SEL_ENTRY, &last)); + assert_eq!(&response[3..5], &0xffffu16.to_le_bytes()); + assert_eq!(response.len(), 21); + assert_eq!(&response[5..7], &2u16.to_le_bytes()); + + let mut missing = Vec::from(0u16.to_le_bytes()); + missing.extend_from_slice(&999u16.to_le_bytes()); + missing.extend_from_slice(&[0, 16]); + let response = transact( + &mut device, + &storage_request(COMMAND_GET_SEL_ENTRY, &missing), + ); + assert_completion( + &response, + STORAGE_REQUEST, + COMMAND_GET_SEL_ENTRY, + COMPLETION_RECORD_NOT_PRESENT, + ); + + let response = transact( + &mut device, + &storage_request(COMMAND_GET_SEL_ENTRY, &[0, 0, 0, 0, 16, 1]), + ); + assert_completion( + &response, + STORAGE_REQUEST, + COMMAND_GET_SEL_ENTRY, + COMPLETION_PARAMETER_OUT_OF_RANGE, + ); +} + +#[test] +fn sel_capacity_is_bounded_without_overwrite() { + let (_, mut device) = device(1); + for expected_id in 1..=128u16 { + let response = add_record(&mut device, expected_id as u8); + assert_completion( + &response, + STORAGE_REQUEST, + COMMAND_ADD_SEL_ENTRY, + COMPLETION_SUCCESS, + ); + assert_eq!(u16::from_le_bytes([response[3], response[4]]), expected_id); + } + assert_eq!(device.sel_len(), 128); + + let response = add_record(&mut device, 0xff); + assert_completion( + &response, + STORAGE_REQUEST, + COMMAND_ADD_SEL_ENTRY, + COMPLETION_SEL_FULL, + ); + assert_eq!(device.sel_len(), 128); + assert_eq!(device.sel_record(0).unwrap()[7], 1); +} + +#[test] +fn record_and_reservation_ids_roll_over_without_reserved_values() { + let (_, mut source) = device(1); + let mut state = source.save().unwrap(); + state.next_record_id = 0xfffe; + state.reservation_id = 0xffff; + let mut device = restore_target(1, state); + + assert_eq!(add_record(&mut device, 0x11)[3..5], [0xfe, 0xff]); + assert_eq!(add_record(&mut device, 0x22)[3..5], [1, 0]); + let reservation = transact(&mut device, &storage_request(COMMAND_RESERVE_SEL, &[])); + assert_eq!(reservation[3..5], [1, 0]); +} + +#[test] +fn clear_sel_validates_fields_and_resets_store() { + let clock = FakeClock::new(100); + let mut device = IpmiKcs::new(clock.clone()); + add_record(&mut device, 0x33); + + let bad_signature = transact( + &mut device, + &storage_request(COMMAND_CLEAR_SEL, &[0, 0, b'X', b'L', b'R', 0xaa]), + ); + assert_completion( + &bad_signature, + STORAGE_REQUEST, + COMMAND_CLEAR_SEL, + COMPLETION_INVALID_DATA_FIELD, + ); + let bad_action = clear(&mut device, 0, 1); + assert_completion( + &bad_action, + STORAGE_REQUEST, + COMMAND_CLEAR_SEL, + COMPLETION_INVALID_DATA_FIELD, + ); + + assert_eq!( + clear(&mut device, 0xdead, 0), + [ + STORAGE_REQUEST | 0x04, + COMMAND_CLEAR_SEL, + COMPLETION_SUCCESS, + 1, + ] + ); + assert_eq!(device.sel_len(), 1); + + clock.set(120); + assert_eq!( + clear(&mut device, 0xbeef, 0xaa), + [ + STORAGE_REQUEST | 0x04, + COMMAND_CLEAR_SEL, + COMPLETION_SUCCESS, + 1, + ] + ); + assert_eq!(device.sel_len(), 0); + assert_eq!(add_record(&mut device, 0x44)[3..5], [1, 0]); + + let info = transact(&mut device, &storage_request(COMMAND_GET_SEL_INFO, &[])); + assert_eq!( + u32::from_le_bytes([info[12], info[13], info[14], info[15]]), + 120 + ); +} + +#[test] +fn sel_time_supports_positive_negative_and_wrapping_offsets() { + let clock = FakeClock::new(1000); + let mut device = IpmiKcs::new(clock.clone()); + + let time = transact(&mut device, &storage_request(COMMAND_GET_SEL_TIME, &[])); + assert_eq!(u32::from_le_bytes(time[3..7].try_into().unwrap()), 1000); + + let response = transact( + &mut device, + &storage_request(COMMAND_SET_SEL_TIME, &1500u32.to_le_bytes()), + ); + assert_completion( + &response, + STORAGE_REQUEST, + COMMAND_SET_SEL_TIME, + COMPLETION_SUCCESS, + ); + assert_eq!(device.sel_time_offset_seconds(), 500); + clock.set(1100); + let time = transact(&mut device, &storage_request(COMMAND_GET_SEL_TIME, &[])); + assert_eq!(u32::from_le_bytes(time[3..7].try_into().unwrap()), 1600); + + clock.set(1000); + transact( + &mut device, + &storage_request(COMMAND_SET_SEL_TIME, &100u32.to_le_bytes()), + ); + assert_eq!(device.sel_time_offset_seconds(), -900); + clock.set(500); + let time = transact(&mut device, &storage_request(COMMAND_GET_SEL_TIME, &[])); + assert_eq!(u32::from_le_bytes(time[3..7].try_into().unwrap()), 0); + + clock.set(0); + transact( + &mut device, + &storage_request(COMMAND_SET_SEL_TIME, &u32::MAX.to_le_bytes()), + ); + clock.set(10); + let time = transact(&mut device, &storage_request(COMMAND_GET_SEL_TIME, &[])); + assert_eq!(u32::from_le_bytes(time[3..7].try_into().unwrap()), 9); +} + +#[test] +fn sink_results_do_not_change_committed_records() { + let clock = FakeClock::new(10); + let sink_state = Rc::new(RefCell::new(SinkState::default())); + let accept = Rc::new(Cell::new(true)); + let sink = SharedSink { + state: sink_state.clone(), + accept: accept.clone(), + }; + let mut device = IpmiKcs::with_event_sink(clock, sink); + + add_record(&mut device, 0x11); + accept.set(false); + add_record(&mut device, 0x22); + + assert_eq!(device.sel_len(), 2); + assert_eq!( + device.stats(), + SelStats { + committed: 2, + forwarded: 1, + rate_limited: 0, + sink_dropped: 1, + } + ); + let state = sink_state.borrow(); + assert_eq!(state.records.len(), 1); + assert_eq!(state.records[0].0, 1); + assert_eq!(state.records[0].1, *device.sel_record(0).unwrap()); +} + +#[test] +fn sink_forwarding_is_limited_to_256_per_trusted_second() { + let clock = FakeClock::new(10); + let sink_state = Rc::new(RefCell::new(SinkState::default())); + let sink = SharedSink { + state: sink_state.clone(), + accept: Rc::new(Cell::new(true)), + }; + let mut device = IpmiKcs::with_event_sink(clock.clone(), sink); + + for _ in 0..256 { + assert_completion( + &add_record(&mut device, 0x5a), + STORAGE_REQUEST, + COMMAND_ADD_SEL_ENTRY, + COMPLETION_SUCCESS, + ); + clear(&mut device, 0, 0xaa); + } + add_record(&mut device, 0x5b); + + assert_eq!(device.sel_len(), 1); + assert_eq!(device.sel_record(0).unwrap()[7], 0x5b); + assert_eq!(device.stats().committed, 257); + assert_eq!(device.stats().forwarded, 256); + assert_eq!(device.stats().rate_limited, 1); + assert_eq!(sink_state.borrow().records.len(), 256); + + clock.set(11); + clear(&mut device, 0, 0xaa); + add_record(&mut device, 0x5c); + assert_eq!(device.stats().forwarded, 257); + assert_eq!(device.stats().rate_limited, 1); + for _ in 1..256 { + clear(&mut device, 0, 0xaa); + add_record(&mut device, 0x5c); + } + assert_eq!(device.stats().forwarded, 512); + + clock.set(10); + clear(&mut device, 0, 0xaa); + add_record(&mut device, 0x5d); + assert_eq!(device.stats().forwarded, 512); + assert_eq!(device.stats().rate_limited, 2); +} + +#[test] +fn reset_preserves_sel_time_reservation_and_stats() { + let clock = FakeClock::new(100); + let mut device = IpmiKcs::new(clock); + transact( + &mut device, + &storage_request(COMMAND_SET_SEL_TIME, &200u32.to_le_bytes()), + ); + add_record(&mut device, 0x42); + let reservation = transact(&mut device, &storage_request(COMMAND_RESERVE_SEL, &[])); + + device.write_command(KCS_COMMAND_WRITE_START); + assert_eq!(device.read_data(), 0); + device.write_data(APPLICATION_REQUEST); + assert_eq!(device.read_status() & STATUS_STATE_MASK, KCS_STATE_WRITE); + device.reset(); + + assert_eq!(device.read_status(), KCS_STATE_IDLE); + assert_eq!(device.sel_len(), 1); + assert_eq!(device.sel_time_offset_seconds(), 100); + assert_eq!(device.stats().committed, 1); + + let current_reservation = transact(&mut device, &storage_request(COMMAND_RESERVE_SEL, &[])); + assert_eq!( + u16::from_le_bytes([current_reservation[3], current_reservation[4]]), + u16::from_le_bytes([reservation[3], reservation[4]]) + 1 + ); +} + +fn restore_target(seconds: i64, state: save_restore::SavedState) -> IpmiKcs { + let (_, mut target) = device(seconds); + target.restore(state).unwrap(); + target +} + +#[test] +fn save_restore_idle_write_and_read_transactions() { + let (_, mut idle) = device(1); + let idle_state = idle.save().unwrap(); + let idle = restore_target(1, idle_state); + assert_eq!(idle.read_status(), KCS_STATE_IDLE); + + let (_, mut writing) = device(1); + writing.write_command(KCS_COMMAND_WRITE_START); + assert_eq!(writing.read_data(), 0); + writing.write_data(APPLICATION_REQUEST); + assert_eq!(writing.read_data(), 0); + writing.write_command(KCS_COMMAND_WRITE_END); + assert_eq!(writing.read_data(), 0); + let write_state = writing.save().unwrap(); + let mut writing = restore_target(1, write_state); + assert_eq!(writing.read_status() & STATUS_STATE_MASK, KCS_STATE_WRITE); + writing.write_data(COMMAND_GET_DEVICE_ID); + let response = read_response(&mut writing); + assert_completion( + &response, + APPLICATION_REQUEST, + COMMAND_GET_DEVICE_ID, + COMPLETION_SUCCESS, + ); + + let (_, mut reading) = device(1); + submit_request(&mut reading, &[APPLICATION_REQUEST, COMMAND_GET_DEVICE_ID]); + let read_state = reading.save().unwrap(); + let mut reading = restore_target(1, read_state); + assert_eq!(reading.read_status() & STATUS_STATE_MASK, KCS_STATE_READ); + let response = read_response(&mut reading); + assert_completion( + &response, + APPLICATION_REQUEST, + COMMAND_GET_DEVICE_ID, + COMPLETION_SUCCESS, + ); +} + +#[test] +fn save_restore_full_sel_and_adjusted_time() { + let clock = FakeClock::new(1000); + let mut source = IpmiKcs::new(clock.clone()); + transact( + &mut source, + &storage_request(COMMAND_SET_SEL_TIME, &1500u32.to_le_bytes()), + ); + for fill in 0..128u8 { + add_record(&mut source, fill); + } + clear(&mut source, 0, 0); + + let state = source.save().unwrap(); + let state = SavedStateBlob::new(state) + .parse::() + .unwrap(); + let mut restored = restore_target(1100, state); + assert_eq!(restored.sel_len(), 128); + assert_eq!(restored.sel_time_offset_seconds(), 500); + assert_eq!(restored.stats(), SelStats::default()); + let time = transact(&mut restored, &storage_request(COMMAND_GET_SEL_TIME, &[])); + assert_eq!(u32::from_le_bytes(time[3..7].try_into().unwrap()), 1600); + assert_completion( + &add_record(&mut restored, 0xff), + STORAGE_REQUEST, + COMMAND_ADD_SEL_ENTRY, + COMPLETION_SEL_FULL, + ); +} + +fn assert_invalid_state(state: save_restore::SavedState) { + let (_, mut target) = device(0); + assert!(matches!( + target.restore(state), + Err(RestoreError::InvalidSavedState(_)) + )); +} + +#[test] +fn malformed_saved_state_is_rejected() { + let (_, mut source) = device(0); + let valid = source.save().unwrap(); + + let mut state = valid.clone(); + state.status = 0x10; + assert_invalid_state(state); + + let mut state = valid.clone(); + state.status = STATUS_IBF; + assert_invalid_state(state); + + let mut state = valid.clone(); + state.request = vec![0; 65]; + assert_invalid_state(state); + + let mut state = valid.clone(); + state.response = vec![0; 65]; + assert_invalid_state(state); + + let mut state = valid.clone(); + state.response = vec![0]; + state.response_position = 2; + assert_invalid_state(state); + + let mut state = valid.clone(); + state.status = KCS_STATE_READ; + state.response.clear(); + assert_invalid_state(state); + + let mut state = valid.clone(); + state.write_end_pending = true; + assert_invalid_state(state); + + let mut state = valid.clone(); + state.sel_count = 1; + assert_invalid_state(state); + + let mut state = valid.clone(); + state.sel_records = (1..=129u16) + .map(|record_id| { + let mut record = vec![0; 16]; + record[0..2].copy_from_slice(&record_id.to_le_bytes()); + record + }) + .collect(); + state.sel_count = 129; + state.next_record_id = 130; + assert_invalid_state(state); + + let mut state = valid.clone(); + state.sel_records = vec![vec![0; 15]]; + state.sel_count = 1; + assert_invalid_state(state); + + let mut state = valid.clone(); + state.sel_records = vec![vec![0; 16]]; + state.sel_count = 1; + assert_invalid_state(state); + + let mut state = valid.clone(); + let mut record = vec![0; 16]; + record[0..2].copy_from_slice(&1u16.to_le_bytes()); + state.sel_records = vec![record.clone(), record]; + state.sel_count = 2; + state.next_record_id = 2; + assert_invalid_state(state); + + for next_record_id in [0, 0xffff] { + let mut state = valid.clone(); + state.next_record_id = next_record_id; + assert_invalid_state(state); + } + + let mut state = valid; + let mut record = vec![0; 16]; + record[0..2].copy_from_slice(&1u16.to_le_bytes()); + state.sel_records = vec![record]; + state.sel_count = 1; + state.next_record_id = 1; + assert_invalid_state(state); +} From 6a7479443af95b2e0e9262b6c66f0ae9017dfd50 Mon Sep 17 00:00:00 2001 From: Ayush Arora Date: Tue, 8 Sep 2026 06:45:33 +0000 Subject: [PATCH 02/10] openhcl: wire IPMI KCS and SEL forwarding --- Cargo.lock | 9 + Cargo.toml | 3 +- .../src/igvm_attest/get.rs | 8 + openhcl/openvmm_hcl_resources/Cargo.toml | 1 + openhcl/openvmm_hcl_resources/src/lib.rs | 1 + .../src/hardware_key_sealing.rs | 1 + .../src/igvm_attest/mod.rs | 4 +- openhcl/underhill_attestation/src/lib.rs | 5 + openhcl/underhill_core/src/loader/mod.rs | 1 + openhcl/underhill_core/src/worker.rs | 71 +-- vm/devices/chipset/ipmi_kcs/Cargo.toml | 9 + vm/devices/chipset/ipmi_kcs/src/device.rs | 452 ++++++++++++++++++ vm/devices/chipset/ipmi_kcs/src/lib.rs | 43 +- vm/devices/chipset/ipmi_kcs/src/protocol.rs | 1 + vm/devices/chipset/ipmi_kcs/src/resolver.rs | 108 +++++ vm/devices/chipset/ipmi_kcs/src/tests.rs | 40 +- vm/devices/chipset_resources/src/lib.rs | 89 ++++ vm/devices/get/get_protocol/src/dps_json.rs | 11 +- vm/devices/get/get_protocol/src/lib.rs | 57 +++ .../get/guest_emulation_device/src/lib.rs | 10 + .../src/test_utilities.rs | 9 + .../get/guest_emulation_transport/src/api.rs | 4 + .../guest_emulation_transport/src/client.rs | 9 + .../get/guest_emulation_transport/src/lib.rs | 40 ++ .../src/process_loop.rs | 12 + .../guest_emulation_transport/src/resolver.rs | 36 ++ vm/loader/src/uefi/config.rs | 8 +- vmm_core/vm_manifest_builder/src/lib.rs | 86 ++++ 28 files changed, 1046 insertions(+), 82 deletions(-) create mode 100644 vm/devices/chipset/ipmi_kcs/src/device.rs create mode 100644 vm/devices/chipset/ipmi_kcs/src/resolver.rs diff --git a/Cargo.lock b/Cargo.lock index 0de7d9b433..959c87b6d9 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4067,9 +4067,17 @@ dependencies = [ name = "ipmi_kcs" version = "0.0.0" dependencies = [ + "async-trait", + "chipset_device", + "chipset_device_resources", + "chipset_resources", + "inspect", + "local_clock", "mesh", + "parking_lot", "test_with_tracing", "thiserror 2.0.16", + "vm_resource", "vmcore", ] @@ -5972,6 +5980,7 @@ dependencies = [ "firmware_uefi", "guest_watchdog", "hyperv_ic", + "ipmi_kcs", "mesh_worker", "missing_dev", "nvme", diff --git a/Cargo.toml b/Cargo.toml index 59294ac3b8..a3dfde856f 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -30,8 +30,6 @@ members = [ "vmm_tests/vmm_perf", # hyper-v tooling "hyperv/tools/hypestv", - # standalone device crates - "vm/devices/chipset/ipmi_kcs", # fuzzing "support/inspect/fuzz", "support/mesh/mesh_protobuf/fuzz", @@ -250,6 +248,7 @@ cvm_tracing = { path = "vm/cvm_tracing" } chipset = { path = "vm/devices/chipset" } chipset_legacy = { path = "vm/devices/chipset_legacy" } chipset_resources = { path = "vm/devices/chipset_resources" } +ipmi_kcs = { path = "vm/devices/chipset/ipmi_kcs" } firmware_pcat = { path = "vm/devices/firmware/firmware_pcat" } firmware_uefi = { path = "vm/devices/firmware/firmware_uefi" } firmware_uefi_custom_vars = { path = "vm/devices/firmware/firmware_uefi_custom_vars" } diff --git a/openhcl/openhcl_attestation_protocol/src/igvm_attest/get.rs b/openhcl/openhcl_attestation_protocol/src/igvm_attest/get.rs index 5a9c87bcde..8d7e002dd5 100644 --- a/openhcl/openhcl_attestation_protocol/src/igvm_attest/get.rs +++ b/openhcl/openhcl_attestation_protocol/src/igvm_attest/get.rs @@ -468,6 +468,10 @@ pub mod runtime_claims { Signer, } + fn is_false(value: &bool) -> bool { + !value + } + /// VM configuration to be included in the `RuntimeClaims`. #[derive(Clone, Debug, Deserialize, Serialize, MeshPayload)] #[serde(rename_all = "kebab-case")] @@ -481,6 +485,9 @@ pub mod runtime_claims { pub console_enabled: bool, /// Whether the serial console, if enabled, is interactive pub interactive_console_enabled: bool, + /// Whether the IPMI KCS interface is enabled + #[serde(default, skip_serializing_if = "is_false")] + pub ipmi_enabled: bool, /// Whether secure boot is enabled pub secure_boot: bool, /// Whether the TPM is enabled @@ -515,6 +522,7 @@ pub mod runtime_claims { root_cert_thumbprint: String::new(), console_enabled: false, interactive_console_enabled: false, + ipmi_enabled: false, secure_boot: false, tpm_enabled: true, tpm_persisted: true, diff --git a/openhcl/openvmm_hcl_resources/Cargo.toml b/openhcl/openvmm_hcl_resources/Cargo.toml index 31ef6008f3..f229981d8b 100644 --- a/openhcl/openvmm_hcl_resources/Cargo.toml +++ b/openhcl/openvmm_hcl_resources/Cargo.toml @@ -36,6 +36,7 @@ hyperv_ic.workspace = true missing_dev.workspace = true serial_16550.workspace = true guest_watchdog.workspace = true +ipmi_kcs.workspace = true tpm_device = { workspace = true, optional = true, features = ["tpm"] } vmgs_broker.workspace = true diff --git a/openhcl/openvmm_hcl_resources/src/lib.rs b/openhcl/openvmm_hcl_resources/src/lib.rs index f5dc855384..f73d59f2eb 100644 --- a/openhcl/openvmm_hcl_resources/src/lib.rs +++ b/openhcl/openvmm_hcl_resources/src/lib.rs @@ -43,6 +43,7 @@ vm_resource::register_static_resolvers! { serial_pl011::resolver::SerialPl011Resolver, chipset::battery::resolver::BatteryResolver, guest_watchdog::resolver::HyperVGuestWatchdogResolver, + ipmi_kcs::resolver::IpmiKcsResolver, // Non-volatile stores vmcore::non_volatile_store::resources::EphemeralNonVolatileStoreResolver, diff --git a/openhcl/underhill_attestation/src/hardware_key_sealing.rs b/openhcl/underhill_attestation/src/hardware_key_sealing.rs index c3c6ba9600..de14f3ec1f 100644 --- a/openhcl/underhill_attestation/src/hardware_key_sealing.rs +++ b/openhcl/underhill_attestation/src/hardware_key_sealing.rs @@ -357,6 +357,7 @@ mod tests { root_cert_thumbprint: "".to_string(), console_enabled: false, interactive_console_enabled: false, + ipmi_enabled: false, secure_boot: false, tpm_enabled: false, tpm_persisted: false, diff --git a/openhcl/underhill_attestation/src/igvm_attest/mod.rs b/openhcl/underhill_attestation/src/igvm_attest/mod.rs index 751888c12b..91c502a97d 100644 --- a/openhcl/underhill_attestation/src/igvm_attest/mod.rs +++ b/openhcl/underhill_attestation/src/igvm_attest/mod.rs @@ -532,13 +532,14 @@ mod tests { #[test] fn test_vm_configuration_no_time() { - const EXPECTED_JWK: &str = r#"{"root-cert-thumbprint":"","console-enabled":false,"interactive-console-enabled":false,"secure-boot":false,"tpm-enabled":false,"tpm-persisted":false,"filtered-vpci-devices-allowed":true,"vmUniqueId":"","hardware-sealing-policy":"signer"}"#; + const EXPECTED_JWK: &str = r#"{"root-cert-thumbprint":"","console-enabled":false,"interactive-console-enabled":false,"ipmi-enabled":true,"secure-boot":false,"tpm-enabled":false,"tpm-persisted":false,"filtered-vpci-devices-allowed":true,"vmUniqueId":"","hardware-sealing-policy":"signer"}"#; let attestation_vm_config = AttestationVmConfig { current_time: None, root_cert_thumbprint: String::new(), console_enabled: false, interactive_console_enabled: false, + ipmi_enabled: true, secure_boot: false, tpm_enabled: false, tpm_persisted: false, @@ -563,6 +564,7 @@ mod tests { root_cert_thumbprint: String::new(), console_enabled: false, interactive_console_enabled: false, + ipmi_enabled: false, secure_boot: false, tpm_enabled: false, tpm_persisted: false, diff --git a/openhcl/underhill_attestation/src/lib.rs b/openhcl/underhill_attestation/src/lib.rs index e129cd741b..9667adf9ab 100644 --- a/openhcl/underhill_attestation/src/lib.rs +++ b/openhcl/underhill_attestation/src/lib.rs @@ -2133,6 +2133,7 @@ mod tests { root_cert_thumbprint: String::new(), console_enabled: false, interactive_console_enabled: false, + ipmi_enabled: false, secure_boot: false, tpm_enabled: true, tpm_persisted: true, @@ -2707,6 +2708,7 @@ mod tests { root_cert_thumbprint: String::new(), console_enabled: false, interactive_console_enabled: false, + ipmi_enabled: false, secure_boot: false, tpm_enabled: false, tpm_persisted: false, @@ -2786,6 +2788,7 @@ mod tests { root_cert_thumbprint: String::new(), console_enabled: false, interactive_console_enabled: false, + ipmi_enabled: false, secure_boot: false, tpm_enabled: false, tpm_persisted: false, @@ -2830,6 +2833,7 @@ mod tests { root_cert_thumbprint: String::new(), console_enabled: false, interactive_console_enabled: false, + ipmi_enabled: false, secure_boot: false, tpm_enabled: false, tpm_persisted: false, @@ -2900,6 +2904,7 @@ mod tests { root_cert_thumbprint: String::new(), console_enabled: false, interactive_console_enabled: false, + ipmi_enabled: false, secure_boot: false, tpm_enabled: false, tpm_persisted: false, diff --git a/openhcl/underhill_core/src/loader/mod.rs b/openhcl/underhill_core/src/loader/mod.rs index 679e655fc8..59199f4e69 100644 --- a/openhcl/underhill_core/src/loader/mod.rs +++ b/openhcl/underhill_core/src/loader/mod.rs @@ -661,6 +661,7 @@ pub fn write_uefi_config( flags.set_cxl_memory_enabled(platform_config.general.cxl_memory_enabled); flags.set_default_boot_always_attempt(platform_config.general.default_boot_always_attempt); flags.set_force_dma_bounce_enabled(platform_config.general.force_dma_bounce_enabled); + flags.set_ipmi_enabled(platform_config.general.ipmi_enabled); // Some settings do not depend on host config diff --git a/openhcl/underhill_core/src/worker.rs b/openhcl/underhill_core/src/worker.rs index 63a3ac112d..abba08c0fb 100644 --- a/openhcl/underhill_core/src/worker.rs +++ b/openhcl/underhill_core/src/worker.rs @@ -2045,6 +2045,36 @@ async fn new_underhill_vm( tracing::warn!(CVM_ALLOWED, "confidential debug enabled"); } + // Validate UEFI-only settings before deriving runtime claims and hardware keys. + let (firmware_type, mut measured_vtl0_info, load_kind) = { + if let Some(firmware_type) = servicing_state.firmware_type { + (firmware_type.into(), None, LoadKind::None) + } else { + let config = MeasuredVtl0Info::read_from_memory(gm.vtl0()) + .context("failed to read measured vtl0 info")?; + let load_kind = if let Some(kind) = env_cfg.force_load_vtl0_image { + tracing::info!(CVM_ALLOWED, kind, "overriding dps load type"); + match kind.as_str() { + "pcat" => LoadKind::Pcat, + "uefi" => LoadKind::Uefi, + "linux" => LoadKind::Linux, + _ => anyhow::bail!("unexpected force load vtl0 type {kind}"), + } + } else if dps.general.firmware_mode_is_pcat { + LoadKind::Pcat + } else { + LoadKind::Uefi + }; + + let firmware_type: FirmwareType = load_kind.into(); + (firmware_type, Some(config), load_kind) + } + }; + + if dps.general.ipmi_enabled && !matches!(firmware_type, FirmwareType::Uefi) { + anyhow::bail!("IPMI KCS is only supported with UEFI firmware"); + } + // Get VMGS provenance claims. If the provenance doc can't be read or if it // isn't valid, proceed as if it doesn't exist. In that case, OpenHCL will // not produce attestation claims for provenance. It's up to the VM owner's @@ -2127,6 +2157,7 @@ async fn new_underhill_vm( root_cert_thumbprint: String::new(), console_enabled, interactive_console_enabled: interactive_console, + ipmi_enabled: dps.general.ipmi_enabled, secure_boot: dps.general.secure_boot_enabled, tpm_enabled: dps.general.tpm_enabled, // Legacy claim; `stateful` reflects its true meaning (attestation not @@ -2224,6 +2255,9 @@ async fn new_underhill_vm( let mut resolver = ResourceResolver::new(); // Make the GET available for other resources. resolver.add_resolver(get_client.clone()); + resolver.add_resolver( + guest_emulation_transport::resolver::IpmiSelEventSinkResolver(get_client.clone()), + ); let (vmgs_client, vmgs) = if let Some((meta, vmgs)) = vmgs { // Spawn the VMGS client for multi-task access. @@ -2247,34 +2281,6 @@ async fn new_underhill_vm( ), ); - // Read measured config from VTL0 memory. When restoring, it is already gone. - let (firmware_type, mut measured_vtl0_info, load_kind) = { - if let Some(firmware_type) = servicing_state.firmware_type { - (firmware_type.into(), None, LoadKind::None) - } else { - let config = MeasuredVtl0Info::read_from_memory(gm.vtl0()) - .context("failed to read measured vtl0 info")?; - let load_kind = if let Some(kind) = env_cfg.force_load_vtl0_image { - tracing::info!(CVM_ALLOWED, kind, "overriding dps load type"); - match kind.as_str() { - "pcat" => LoadKind::Pcat, - "uefi" => LoadKind::Uefi, - "linux" => LoadKind::Linux, - _ => anyhow::bail!("unexpected force load vtl0 type {kind}"), - } - } else { - if dps.general.firmware_mode_is_pcat { - LoadKind::Pcat - } else { - LoadKind::Uefi - } - }; - - let firmware_type: FirmwareType = load_kind.into(); - (firmware_type, Some(config), load_kind) - } - }; - // Only advertise extended IOAPIC on non-PCAT systems. #[cfg(guest_arch = "x86_64")] let cpuid = { @@ -2522,6 +2528,10 @@ async fn new_underhill_vm( chipset = chipset.with_platform_pm_timer_assist(); } + if dps.general.ipmi_enabled { + chipset = chipset.with_ipmi_kcs(); + } + if with_serial { chipset = chipset.with_serial(serial_inputs); if env_cfg.emulated_serial_wait_for_rts { @@ -3947,6 +3957,7 @@ fn validate_isolated_configuration(dps: &DevicePlatformSettings) -> Result<(), a // Attested to secure_boot_enabled, tpm_enabled: _, + ipmi_enabled: _, com1_enabled: _, com1_vmbus_redirector: _, com2_enabled: _, @@ -4418,7 +4429,9 @@ impl chipset_device_worker::RemoteDynamicResolvers for OpenHclRemoteDynamicResol self, resolver: &mut ResourceResolver, ) -> anyhow::Result<()> { - resolver.add_resolver(self.get); + resolver.add_resolver(self.get.clone()); + resolver + .add_resolver(guest_emulation_transport::resolver::IpmiSelEventSinkResolver(self.get)); if let Some(vmgs) = self.vmgs { resolver.add_resolver(vmgs); } diff --git a/vm/devices/chipset/ipmi_kcs/Cargo.toml b/vm/devices/chipset/ipmi_kcs/Cargo.toml index dfa45d38ba..2a4fff2d6a 100644 --- a/vm/devices/chipset/ipmi_kcs/Cargo.toml +++ b/vm/devices/chipset/ipmi_kcs/Cargo.toml @@ -7,11 +7,20 @@ edition.workspace = true rust-version.workspace = true [dependencies] +chipset_device.workspace = true +chipset_device_resources.workspace = true +chipset_resources.workspace = true +inspect.workspace = true +local_clock = { workspace = true, features = ["inspect"] } mesh.workspace = true thiserror.workspace = true +vm_resource.workspace = true vmcore.workspace = true +async-trait.workspace = true + [dev-dependencies] +parking_lot.workspace = true test_with_tracing.workspace = true [lints] diff --git a/vm/devices/chipset/ipmi_kcs/src/device.rs b/vm/devices/chipset/ipmi_kcs/src/device.rs new file mode 100644 index 0000000000..f7c71745f6 --- /dev/null +++ b/vm/devices/chipset/ipmi_kcs/src/device.rs @@ -0,0 +1,452 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +//! Chipset adapters for the IPMI KCS PIO and MMIO interfaces. + +use crate::IpmiKcs; +use chipset_device::ChipsetDevice; +use chipset_device::io::IoError; +use chipset_device::io::IoResult; +use chipset_device::mmio::MmioIntercept; +use chipset_device::pio::PortIoIntercept; +use chipset_resources::ipmi_kcs::IPMI_KCS_DATA_PORT; +use chipset_resources::ipmi_kcs::IPMI_KCS_MMIO_BASE_ADDRESS_AARCH64; +use chipset_resources::ipmi_kcs::IPMI_KCS_MMIO_DATA_ADDRESS_AARCH64; +use chipset_resources::ipmi_kcs::IPMI_KCS_MMIO_REGION_SIZE_AARCH64; +use chipset_resources::ipmi_kcs::IPMI_KCS_MMIO_STATUS_COMMAND_ADDRESS_AARCH64; +use chipset_resources::ipmi_kcs::IPMI_KCS_STATUS_COMMAND_PORT; +use inspect::InspectMut; +use std::ops::RangeInclusive; +use vmcore::device_state::ChangeDeviceState; +use vmcore::save_restore::RestoreError; +use vmcore::save_restore::SaveError; +use vmcore::save_restore::SaveRestore; + +const PIO_REGIONS: [(&str, RangeInclusive); 1] = [( + "ipmi-kcs", + IPMI_KCS_DATA_PORT..=IPMI_KCS_STATUS_COMMAND_PORT, +)]; +const MMIO_REGIONS: [(&str, RangeInclusive); 1] = [( + "ipmi-kcs", + IPMI_KCS_MMIO_BASE_ADDRESS_AARCH64 + ..=IPMI_KCS_MMIO_BASE_ADDRESS_AARCH64 + IPMI_KCS_MMIO_REGION_SIZE_AARCH64 - 1, +)]; + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum Transport { + Pio, + Mmio, +} + +/// Transport adapter for a virtual IPMI KCS interface. +pub struct IpmiKcsDevice { + core: IpmiKcs, + transport: Transport, +} + +impl IpmiKcsDevice { + /// Creates an AMD64 port-I/O device around the transport-independent KCS core. + pub fn new_pio(core: IpmiKcs) -> Self { + Self { + core, + transport: Transport::Pio, + } + } + + /// Creates an ARM64 MMIO device around the transport-independent KCS core. + pub fn new_mmio(core: IpmiKcs) -> Self { + Self { + core, + transport: Transport::Mmio, + } + } +} + +impl ChangeDeviceState for IpmiKcsDevice { + fn start(&mut self) {} + + async fn stop(&mut self) {} + + async fn reset(&mut self) { + self.core.reset(); + } +} + +impl ChipsetDevice for IpmiKcsDevice { + fn supports_pio(&mut self) -> Option<&mut dyn PortIoIntercept> { + match self.transport { + Transport::Pio => Some(self), + Transport::Mmio => None, + } + } + + fn supports_mmio(&mut self) -> Option<&mut dyn MmioIntercept> { + match self.transport { + Transport::Pio => None, + Transport::Mmio => Some(self), + } + } +} + +impl PortIoIntercept for IpmiKcsDevice { + fn io_read(&mut self, io_port: u16, data: &mut [u8]) -> IoResult { + let [value] = data else { + return IoResult::Err(IoError::InvalidAccessSize); + }; + + *value = match io_port { + IPMI_KCS_DATA_PORT => self.core.read_data(), + IPMI_KCS_STATUS_COMMAND_PORT => self.core.read_status(), + _ => return IoResult::Err(IoError::InvalidRegister), + }; + IoResult::Ok + } + + fn io_write(&mut self, io_port: u16, data: &[u8]) -> IoResult { + let [value] = data else { + return IoResult::Err(IoError::InvalidAccessSize); + }; + + match io_port { + IPMI_KCS_DATA_PORT => self.core.write_data(*value), + IPMI_KCS_STATUS_COMMAND_PORT => self.core.write_command(*value), + _ => return IoResult::Err(IoError::InvalidRegister), + } + IoResult::Ok + } + + fn get_static_regions(&mut self) -> &[(&str, RangeInclusive)] { + &PIO_REGIONS + } +} + +impl MmioIntercept for IpmiKcsDevice { + fn mmio_read(&mut self, address: u64, data: &mut [u8]) -> IoResult { + let [value] = data else { + return IoResult::Err(IoError::InvalidAccessSize); + }; + + *value = match address { + IPMI_KCS_MMIO_DATA_ADDRESS_AARCH64 => self.core.read_data(), + IPMI_KCS_MMIO_STATUS_COMMAND_ADDRESS_AARCH64 => self.core.read_status(), + _ => return IoResult::Err(IoError::InvalidRegister), + }; + IoResult::Ok + } + + fn mmio_write(&mut self, address: u64, data: &[u8]) -> IoResult { + let [value] = data else { + return IoResult::Err(IoError::InvalidAccessSize); + }; + + match address { + IPMI_KCS_MMIO_DATA_ADDRESS_AARCH64 => self.core.write_data(*value), + IPMI_KCS_MMIO_STATUS_COMMAND_ADDRESS_AARCH64 => self.core.write_command(*value), + _ => return IoResult::Err(IoError::InvalidRegister), + } + IoResult::Ok + } + + fn get_static_regions(&mut self) -> &[(&str, RangeInclusive)] { + &MMIO_REGIONS + } +} + +impl InspectMut for IpmiKcsDevice { + fn inspect_mut(&mut self, req: inspect::Request<'_>) { + let stats = self.core.stats(); + req.respond() + .hex("status", self.core.read_status()) + .field("sel_records", self.core.sel_len()) + .field( + "sel_time_offset_seconds", + self.core.sel_time_offset_seconds(), + ) + .field("committed", stats.committed) + .field("forwarded", stats.forwarded) + .field("rate_limited", stats.rate_limited) + .field("sink_dropped", stats.sink_dropped); + } +} + +impl SaveRestore for IpmiKcsDevice { + type SavedState = ::SavedState; + + fn save(&mut self) -> Result { + self.core.save() + } + + fn restore(&mut self, state: Self::SavedState) -> Result<(), RestoreError> { + self.core.restore(state) + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::KCS_COMMAND_WRITE_END; + use crate::KCS_COMMAND_WRITE_START; + use crate::KCS_DATA_READ_NEXT; + use crate::KCS_STATE_IDLE; + use crate::KCS_STATE_READ; + use crate::STATUS_STATE_MASK; + use crate::TrustedClock; + use crate::protocol::COMMAND_ADD_SEL_ENTRY; + use crate::protocol::COMMAND_GET_DEVICE_ID; + use crate::protocol::NETFN_APPLICATION; + use crate::protocol::NETFN_STORAGE; + use std::sync::Arc; + use std::sync::atomic::AtomicI64; + use std::sync::atomic::Ordering; + use test_with_tracing::test; + + #[derive(Clone)] + struct FakeClock(Arc); + + impl FakeClock { + fn new(seconds: i64) -> Self { + Self(Arc::new(AtomicI64::new(seconds))) + } + } + + impl TrustedClock for FakeClock { + fn unix_seconds(&mut self) -> i64 { + self.0.load(Ordering::Relaxed) + } + } + + fn device() -> IpmiKcsDevice { + IpmiKcsDevice::new_pio(IpmiKcs::new(FakeClock::new(100))) + } + + fn mmio_device() -> IpmiKcsDevice { + IpmiKcsDevice::new_mmio(IpmiKcs::new(FakeClock::new(100))) + } + + fn read(device: &mut IpmiKcsDevice, port: u16) -> u8 { + let mut data = [0]; + device.io_read(port, &mut data).unwrap(); + data[0] + } + + fn write(device: &mut IpmiKcsDevice, port: u16, value: u8) { + device.io_write(port, &[value]).unwrap(); + } + + fn transact(device: &mut IpmiKcsDevice, request: &[u8]) -> Vec { + write( + device, + IPMI_KCS_STATUS_COMMAND_PORT, + KCS_COMMAND_WRITE_START, + ); + assert_eq!(read(device, IPMI_KCS_DATA_PORT), 0); + + for byte in &request[..request.len() - 1] { + write(device, IPMI_KCS_DATA_PORT, *byte); + assert_eq!(read(device, IPMI_KCS_DATA_PORT), 0); + } + + write(device, IPMI_KCS_STATUS_COMMAND_PORT, KCS_COMMAND_WRITE_END); + assert_eq!(read(device, IPMI_KCS_DATA_PORT), 0); + write(device, IPMI_KCS_DATA_PORT, request[request.len() - 1]); + + let mut response = Vec::new(); + while read(device, IPMI_KCS_STATUS_COMMAND_PORT) & STATUS_STATE_MASK == KCS_STATE_READ { + response.push(read(device, IPMI_KCS_DATA_PORT)); + write(device, IPMI_KCS_DATA_PORT, KCS_DATA_READ_NEXT); + } + assert_eq!( + read(device, IPMI_KCS_STATUS_COMMAND_PORT) & STATUS_STATE_MASK, + KCS_STATE_IDLE + ); + assert_eq!(read(device, IPMI_KCS_DATA_PORT), 0); + response + } + + #[test] + fn advertises_exact_amd64_pio_region() { + assert_eq!( + PortIoIntercept::get_static_regions(&mut device()), + &[( + "ipmi-kcs", + IPMI_KCS_DATA_PORT..=IPMI_KCS_STATUS_COMMAND_PORT + )] + ); + } + + #[test] + fn rejects_invalid_accesses() { + let mut device = device(); + + assert!(matches!( + device.io_read(IPMI_KCS_DATA_PORT, &mut [0; 2]), + IoResult::Err(IoError::InvalidAccessSize) + )); + assert!(matches!( + device.io_write(IPMI_KCS_DATA_PORT, &[0; 2]), + IoResult::Err(IoError::InvalidAccessSize) + )); + assert!(matches!( + device.io_read(IPMI_KCS_DATA_PORT - 1, &mut [0]), + IoResult::Err(IoError::InvalidRegister) + )); + assert!(matches!( + device.io_write(IPMI_KCS_STATUS_COMMAND_PORT + 1, &[0]), + IoResult::Err(IoError::InvalidRegister) + )); + } + + #[test] + fn get_device_id_works_over_pio() { + let mut device = device(); + let response = transact( + &mut device, + &[NETFN_APPLICATION << 2, COMMAND_GET_DEVICE_ID], + ); + + assert_eq!( + response, + [ + (NETFN_APPLICATION << 2) | 0x04, + COMMAND_GET_DEVICE_ID, + 0x00, + 0x20, + 0x01, + 0x02, + 0x00, + 0x02, + 0x04, + 0x00, + 0x00, + 0x00, + 0x00, + 0x00, + ] + ); + } + + #[test] + fn concrete_device_save_restore_delegates_to_core() { + let mut device = device(); + let mut request = vec![NETFN_STORAGE << 2, COMMAND_ADD_SEL_ENTRY]; + request.extend_from_slice(&[0x5a; 16]); + let response = transact(&mut device, &request); + assert_eq!(response[2], 0); + assert_eq!(device.core.sel_len(), 1); + + let saved = device.save().unwrap(); + let _ = transact(&mut device, &request); + assert_eq!(device.core.sel_len(), 2); + + device.restore(saved).unwrap(); + assert_eq!(device.core.sel_len(), 1); + } + + #[test] + fn advertises_exact_arm64_mmio_region() { + assert_eq!( + MmioIntercept::get_static_regions(&mut mmio_device()), + &[( + "ipmi-kcs", + IPMI_KCS_MMIO_BASE_ADDRESS_AARCH64 + ..=IPMI_KCS_MMIO_BASE_ADDRESS_AARCH64 + IPMI_KCS_MMIO_REGION_SIZE_AARCH64 - 1 + )] + ); + } + + #[test] + fn exposes_exactly_one_configured_transport() { + let mut pio = device(); + assert!(pio.supports_pio().is_some()); + assert!(pio.supports_mmio().is_none()); + + let mut mmio = mmio_device(); + assert!(mmio.supports_pio().is_none()); + assert!(mmio.supports_mmio().is_some()); + } + + #[test] + fn arm64_mmio_rejects_invalid_accesses() { + let mut device = mmio_device(); + + assert!(matches!( + device.mmio_read(IPMI_KCS_MMIO_DATA_ADDRESS_AARCH64, &mut [0; 4]), + IoResult::Err(IoError::InvalidAccessSize) + )); + assert!(matches!( + device.mmio_write(IPMI_KCS_MMIO_DATA_ADDRESS_AARCH64, &[0; 4]), + IoResult::Err(IoError::InvalidAccessSize) + )); + assert!(matches!( + device.mmio_read(IPMI_KCS_MMIO_DATA_ADDRESS_AARCH64 + 1, &mut [0]), + IoResult::Err(IoError::InvalidRegister) + )); + assert!(matches!( + device.mmio_write(IPMI_KCS_MMIO_STATUS_COMMAND_ADDRESS_AARCH64 + 1, &[0]), + IoResult::Err(IoError::InvalidRegister) + )); + } + + #[test] + fn get_device_id_works_over_arm64_mmio() { + let mut device = mmio_device(); + + device + .mmio_write( + IPMI_KCS_MMIO_STATUS_COMMAND_ADDRESS_AARCH64, + &[KCS_COMMAND_WRITE_START], + ) + .unwrap(); + let mut value = [0]; + device + .mmio_read(IPMI_KCS_MMIO_DATA_ADDRESS_AARCH64, &mut value) + .unwrap(); + assert_eq!(value[0], 0); + + device + .mmio_write( + IPMI_KCS_MMIO_DATA_ADDRESS_AARCH64, + &[NETFN_APPLICATION << 2], + ) + .unwrap(); + device + .mmio_read(IPMI_KCS_MMIO_DATA_ADDRESS_AARCH64, &mut value) + .unwrap(); + assert_eq!(value[0], 0); + + device + .mmio_write( + IPMI_KCS_MMIO_STATUS_COMMAND_ADDRESS_AARCH64, + &[KCS_COMMAND_WRITE_END], + ) + .unwrap(); + device + .mmio_read(IPMI_KCS_MMIO_DATA_ADDRESS_AARCH64, &mut value) + .unwrap(); + assert_eq!(value[0], 0); + device + .mmio_write(IPMI_KCS_MMIO_DATA_ADDRESS_AARCH64, &[COMMAND_GET_DEVICE_ID]) + .unwrap(); + + let mut response = Vec::new(); + loop { + device + .mmio_read(IPMI_KCS_MMIO_STATUS_COMMAND_ADDRESS_AARCH64, &mut value) + .unwrap(); + if value[0] & STATUS_STATE_MASK != KCS_STATE_READ { + break; + } + device + .mmio_read(IPMI_KCS_MMIO_DATA_ADDRESS_AARCH64, &mut value) + .unwrap(); + response.push(value[0]); + device + .mmio_write(IPMI_KCS_MMIO_DATA_ADDRESS_AARCH64, &[KCS_DATA_READ_NEXT]) + .unwrap(); + } + + assert_eq!(response[0], (NETFN_APPLICATION << 2) | 0x04); + assert_eq!(response[1], COMMAND_GET_DEVICE_ID); + assert_eq!(response[2], 0x00); + } +} diff --git a/vm/devices/chipset/ipmi_kcs/src/lib.rs b/vm/devices/chipset/ipmi_kcs/src/lib.rs index 505088c1f4..93c1b196ce 100644 --- a/vm/devices/chipset/ipmi_kcs/src/lib.rs +++ b/vm/devices/chipset/ipmi_kcs/src/lib.rs @@ -1,20 +1,22 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -//! A transport-independent virtual IPMI BMC with a byte-oriented KCS interface. +//! A minimal virtual IPMI BMC with a byte-oriented KCS interface. //! -//! This crate implements the KCS register state machine and a bounded System -//! Event Log (SEL). Platform adapters are intentionally separate: callers map -//! [`IpmiKcs::read_data`], [`IpmiKcs::read_status`], -//! [`IpmiKcs::write_data`], and [`IpmiKcs::write_command`] onto their chosen -//! PIO or MMIO transport. +//! This crate implements the KCS register state machine, a bounded System Event +//! Log (SEL), architecture-specific PIO and MMIO chipset devices, and resource +//! resolution for the device's time source and SEL event sink. #![forbid(unsafe_code)] +pub mod device; mod protocol; +pub mod resolver; mod save_restore; mod sel; +pub use chipset_resources::SelEventDisposition; +pub use chipset_resources::SelEventSink; use sel::RateLimiter; use sel::SelState; @@ -55,27 +57,12 @@ pub const KCS_DATA_READ_NEXT: u8 = 0x68; /// Implementations must return UTC seconds since the Unix epoch. Negative /// values are accepted so that startup and test clocks can be represented /// without lossy conversion. -pub trait TrustedClock { +pub trait TrustedClock: Send { /// Returns the current trusted host time in Unix seconds. fn unix_seconds(&mut self) -> i64; } -/// Result of a nonblocking SEL event-forwarding attempt. -#[derive(Debug, Clone, Copy, PartialEq, Eq)] -pub enum SelEventDisposition { - /// The event was accepted by the sink. - Accepted, - /// The sink dropped the event without blocking the virtual BMC. - Dropped, -} - -/// Best-effort, nonblocking sink for finalized SEL records. -pub trait SelEventSink { - /// Attempts to forward one committed SEL record. - fn try_send(&mut self, record_id: u16, record: [u8; 16]) -> SelEventDisposition; -} - -/// Lifetime diagnostic counters for SEL additions. +/// Diagnostic counters for SEL additions. /// /// These counters are intentionally excluded from saved state. #[derive(Debug, Default, Clone, Copy, PartialEq, Eq)] @@ -165,7 +152,10 @@ impl IpmiKcs { Self::from_boxed_parts(Box::new(clock), Some(Box::new(sink))) } - fn from_boxed_parts(clock: Box, sink: Option>) -> Self { + pub(crate) fn from_boxed_parts( + clock: Box, + sink: Option>, + ) -> Self { Self { transaction: KcsTransaction::default(), sel: SelState::new(), @@ -176,11 +166,6 @@ impl IpmiKcs { } } - /// Replaces or removes the nonblocking SEL event sink. - pub fn set_event_sink(&mut self, sink: Option>) { - self.sink = sink; - } - /// Reads the KCS data register and clears OBF. pub fn read_data(&mut self) -> u8 { let value = self.transaction.data_out; diff --git a/vm/devices/chipset/ipmi_kcs/src/protocol.rs b/vm/devices/chipset/ipmi_kcs/src/protocol.rs index d9ea47b91e..632249121c 100644 --- a/vm/devices/chipset/ipmi_kcs/src/protocol.rs +++ b/vm/devices/chipset/ipmi_kcs/src/protocol.rs @@ -21,6 +21,7 @@ pub(crate) const COMMAND_SET_SEL_TIME: u8 = 0x49; pub(crate) const COMPLETION_SUCCESS: u8 = 0x00; pub(crate) const COMPLETION_INVALID_COMMAND: u8 = 0xc1; pub(crate) const COMPLETION_SEL_FULL: u8 = 0xc4; +#[expect(dead_code)] pub(crate) const COMPLETION_RESERVATION_CANCELED: u8 = 0xc5; pub(crate) const COMPLETION_INVALID_REQUEST_LENGTH: u8 = 0xc7; pub(crate) const COMPLETION_PARAMETER_OUT_OF_RANGE: u8 = 0xc9; diff --git a/vm/devices/chipset/ipmi_kcs/src/resolver.rs b/vm/devices/chipset/ipmi_kcs/src/resolver.rs new file mode 100644 index 0000000000..a2ed230883 --- /dev/null +++ b/vm/devices/chipset/ipmi_kcs/src/resolver.rs @@ -0,0 +1,108 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +//! Resource resolver for the IPMI KCS chipset device. + +use crate::IpmiKcs; +use crate::TrustedClock; +use crate::device::IpmiKcsDevice; +use async_trait::async_trait; +use chipset_device_resources::ResolveChipsetDeviceHandleParams; +use chipset_device_resources::ResolvedChipsetDevice; +use chipset_resources::CmosRtcTimeSourceHandleKind; +use chipset_resources::IpmiSelEventSinkHandleKind; +use chipset_resources::ResolvedCmosRtcTimeSource; +use chipset_resources::ResolvedIpmiSelEventSink; +use chipset_resources::ipmi_kcs::IpmiKcsDeviceHandleAArch64; +use chipset_resources::ipmi_kcs::IpmiKcsDeviceHandleX64; +use local_clock::InspectableLocalClock; +use thiserror::Error; +use vm_resource::AsyncResolveResource; +use vm_resource::ResolveError; +use vm_resource::Resource; +use vm_resource::ResourceResolver; +use vm_resource::declare_static_async_resolver; +use vm_resource::kind::ChipsetDeviceHandleKind; + +/// Resource resolver for IPMI KCS device handles. +pub struct IpmiKcsResolver; + +declare_static_async_resolver! { + IpmiKcsResolver, + (ChipsetDeviceHandleKind, IpmiKcsDeviceHandleX64), + (ChipsetDeviceHandleKind, IpmiKcsDeviceHandleAArch64), +} + +/// Error resolving an IPMI KCS device. +#[derive(Debug, Error)] +pub enum ResolveIpmiKcsError { + /// The trusted time source could not be resolved. + #[error("failed to resolve IPMI KCS time source")] + TimeSource(#[source] ResolveError), + /// The SEL event sink could not be resolved. + #[error("failed to resolve IPMI SEL event sink")] + EventSink(#[source] ResolveError), +} + +struct TrustedClockAdapter(Box); + +impl TrustedClock for TrustedClockAdapter { + fn unix_seconds(&mut self) -> i64 { + self.0 + .get_time() + .as_millis_since_unix_epoch() + .div_euclid(1000) + } +} + +async fn resolve_core( + resolver: &ResourceResolver, + event_sink: Resource, + time_source: Resource, +) -> Result { + let ResolvedCmosRtcTimeSource(time_source) = resolver + .resolve(time_source, ()) + .await + .map_err(ResolveIpmiKcsError::TimeSource)?; + let ResolvedIpmiSelEventSink(event_sink) = resolver + .resolve(event_sink, ()) + .await + .map_err(ResolveIpmiKcsError::EventSink)?; + + Ok(IpmiKcs::from_boxed_parts( + Box::new(TrustedClockAdapter(time_source)), + Some(event_sink), + )) +} + +#[async_trait] +impl AsyncResolveResource for IpmiKcsResolver { + type Output = ResolvedChipsetDevice; + type Error = ResolveIpmiKcsError; + + async fn resolve( + &self, + resolver: &ResourceResolver, + resource: IpmiKcsDeviceHandleX64, + _input: ResolveChipsetDeviceHandleParams<'_>, + ) -> Result { + let core = resolve_core(resolver, resource.event_sink, resource.time_source).await?; + Ok(IpmiKcsDevice::new_pio(core).into()) + } +} + +#[async_trait] +impl AsyncResolveResource for IpmiKcsResolver { + type Output = ResolvedChipsetDevice; + type Error = ResolveIpmiKcsError; + + async fn resolve( + &self, + resolver: &ResourceResolver, + resource: IpmiKcsDeviceHandleAArch64, + _input: ResolveChipsetDeviceHandleParams<'_>, + ) -> Result { + let core = resolve_core(resolver, resource.event_sink, resource.time_source).await?; + Ok(IpmiKcsDevice::new_mmio(core).into()) + } +} diff --git a/vm/devices/chipset/ipmi_kcs/src/tests.rs b/vm/devices/chipset/ipmi_kcs/src/tests.rs index e8c87d93f7..75532e627c 100644 --- a/vm/devices/chipset/ipmi_kcs/src/tests.rs +++ b/vm/devices/chipset/ipmi_kcs/src/tests.rs @@ -3,9 +3,11 @@ use super::*; use crate::protocol::*; -use std::cell::Cell; -use std::cell::RefCell; -use std::rc::Rc; +use parking_lot::Mutex; +use std::sync::Arc; +use std::sync::atomic::AtomicBool; +use std::sync::atomic::AtomicI64; +use std::sync::atomic::Ordering; use test_with_tracing::test; use vmcore::save_restore::RestoreError; use vmcore::save_restore::SaveRestore; @@ -15,21 +17,21 @@ const APPLICATION_REQUEST: u8 = NETFN_APPLICATION << 2; const STORAGE_REQUEST: u8 = NETFN_STORAGE << 2; #[derive(Clone)] -struct FakeClock(Rc>); +struct FakeClock(Arc); impl FakeClock { fn new(seconds: i64) -> Self { - Self(Rc::new(Cell::new(seconds))) + Self(Arc::new(AtomicI64::new(seconds))) } fn set(&self, seconds: i64) { - self.0.set(seconds); + self.0.store(seconds, Ordering::Relaxed); } } impl TrustedClock for FakeClock { fn unix_seconds(&mut self) -> i64 { - self.0.get() + self.0.load(Ordering::Relaxed) } } @@ -39,16 +41,16 @@ struct SinkState { } struct SharedSink { - state: Rc>, - accept: Rc>, + state: Arc>, + accept: Arc, } impl SelEventSink for SharedSink { fn try_send(&mut self, record_id: u16, record: [u8; 16]) -> SelEventDisposition { - if !self.accept.get() { + if !self.accept.load(Ordering::Relaxed) { return SelEventDisposition::Dropped; } - self.state.borrow_mut().records.push((record_id, record)); + self.state.lock().records.push((record_id, record)); SelEventDisposition::Accepted } } @@ -197,7 +199,7 @@ fn abort_exposes_status_byte_then_returns_idle() { } #[test] -fn malformed_kcs_sequences_match_legacy_behavior_without_panicking() { +fn malformed_kcs_sequences_are_handled_without_panicking() { let (_, mut device) = device(0); device.write_data(0); @@ -616,8 +618,8 @@ fn sel_time_supports_positive_negative_and_wrapping_offsets() { #[test] fn sink_results_do_not_change_committed_records() { let clock = FakeClock::new(10); - let sink_state = Rc::new(RefCell::new(SinkState::default())); - let accept = Rc::new(Cell::new(true)); + let sink_state = Arc::new(Mutex::new(SinkState::default())); + let accept = Arc::new(AtomicBool::new(true)); let sink = SharedSink { state: sink_state.clone(), accept: accept.clone(), @@ -625,7 +627,7 @@ fn sink_results_do_not_change_committed_records() { let mut device = IpmiKcs::with_event_sink(clock, sink); add_record(&mut device, 0x11); - accept.set(false); + accept.store(false, Ordering::Relaxed); add_record(&mut device, 0x22); assert_eq!(device.sel_len(), 2); @@ -638,7 +640,7 @@ fn sink_results_do_not_change_committed_records() { sink_dropped: 1, } ); - let state = sink_state.borrow(); + let state = sink_state.lock(); assert_eq!(state.records.len(), 1); assert_eq!(state.records[0].0, 1); assert_eq!(state.records[0].1, *device.sel_record(0).unwrap()); @@ -647,10 +649,10 @@ fn sink_results_do_not_change_committed_records() { #[test] fn sink_forwarding_is_limited_to_256_per_trusted_second() { let clock = FakeClock::new(10); - let sink_state = Rc::new(RefCell::new(SinkState::default())); + let sink_state = Arc::new(Mutex::new(SinkState::default())); let sink = SharedSink { state: sink_state.clone(), - accept: Rc::new(Cell::new(true)), + accept: Arc::new(AtomicBool::new(true)), }; let mut device = IpmiKcs::with_event_sink(clock.clone(), sink); @@ -670,7 +672,7 @@ fn sink_forwarding_is_limited_to_256_per_trusted_second() { assert_eq!(device.stats().committed, 257); assert_eq!(device.stats().forwarded, 256); assert_eq!(device.stats().rate_limited, 1); - assert_eq!(sink_state.borrow().records.len(), 256); + assert_eq!(sink_state.lock().records.len(), 256); clock.set(11); clear(&mut device, 0, 0xaa); diff --git a/vm/devices/chipset_resources/src/lib.rs b/vm/devices/chipset_resources/src/lib.rs index cf4e1584f7..2cdcaab33e 100644 --- a/vm/devices/chipset_resources/src/lib.rs +++ b/vm/devices/chipset_resources/src/lib.rs @@ -26,6 +26,95 @@ impl CanResolveTo for CmosRtcTimeSourceHandleKind { type Input<'a> = (); } +/// The size of an IPMI System Event Log record. +pub const IPMI_SEL_RECORD_SIZE: usize = 16; + +/// Result of attempting to forward an IPMI SEL record. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum SelEventDisposition { + /// The sink accepted the record. + Accepted, + /// The sink dropped the record without blocking the virtual BMC. + Dropped, +} + +/// Non-blocking sink for completed IPMI SEL records. +pub trait SelEventSink: Send { + /// Attempts to forward a completed SEL record. + fn try_send( + &mut self, + record_id: u16, + record: [u8; IPMI_SEL_RECORD_SIZE], + ) -> SelEventDisposition; +} + +/// Resource kind for IPMI SEL event sinks. +pub enum IpmiSelEventSinkHandleKind {} + +impl ResourceKind for IpmiSelEventSinkHandleKind { + const NAME: &'static str = "ipmi_sel_event_sink"; +} + +/// Resolved runtime IPMI SEL event sink. +pub struct ResolvedIpmiSelEventSink(pub Box); + +impl CanResolveTo for IpmiSelEventSinkHandleKind { + type Input<'a> = (); +} + +pub mod ipmi_kcs { + //! Resource definitions for the IPMI KCS virtual BMC. + + use super::CmosRtcTimeSourceHandleKind; + use super::IpmiSelEventSinkHandleKind; + use mesh::MeshPayload; + use vm_resource::Resource; + use vm_resource::ResourceId; + use vm_resource::kind::ChipsetDeviceHandleKind; + + /// AMD64 KCS data-register port. + pub const IPMI_KCS_DATA_PORT: u16 = 0xca2; + /// AMD64 KCS status-read/command-write port. + pub const IPMI_KCS_STATUS_COMMAND_PORT: u16 = IPMI_KCS_DATA_PORT + 1; + /// ARM64 KCS MMIO page base address. + pub const IPMI_KCS_MMIO_BASE_ADDRESS_AARCH64: u64 = 0xeffe_7000; + /// ARM64 KCS MMIO register spacing. + pub const IPMI_KCS_MMIO_REGISTER_SPACING_AARCH64: u64 = 4; + /// ARM64 KCS data-register address. + pub const IPMI_KCS_MMIO_DATA_ADDRESS_AARCH64: u64 = IPMI_KCS_MMIO_BASE_ADDRESS_AARCH64; + /// ARM64 KCS status-read/command-write address. + pub const IPMI_KCS_MMIO_STATUS_COMMAND_ADDRESS_AARCH64: u64 = + IPMI_KCS_MMIO_BASE_ADDRESS_AARCH64 + IPMI_KCS_MMIO_REGISTER_SPACING_AARCH64; + /// Size of the ARM64 KCS MMIO aperture. + pub const IPMI_KCS_MMIO_REGION_SIZE_AARCH64: u64 = 0x1000; + + /// A handle to an AMD64 IPMI KCS virtual BMC. + #[derive(MeshPayload)] + pub struct IpmiKcsDeviceHandleX64 { + /// Non-blocking sink for completed SEL records. + pub event_sink: Resource, + /// Trusted wall-clock source used for SEL timestamps. + pub time_source: Resource, + } + + impl ResourceId for IpmiKcsDeviceHandleX64 { + const ID: &'static str = "ipmi-kcs-x64"; + } + + /// A handle to an ARM64 IPMI KCS virtual BMC. + #[derive(MeshPayload)] + pub struct IpmiKcsDeviceHandleAArch64 { + /// Non-blocking sink for completed SEL records. + pub event_sink: Resource, + /// Trusted wall-clock source used for SEL timestamps. + pub time_source: Resource, + } + + impl ResourceId for IpmiKcsDeviceHandleAArch64 { + const ID: &'static str = "ipmi-kcs-aarch64"; + } +} + pub mod cmos_rtc_time_source { //! Resource definitions and resolvers for CMOS RTC time sources. diff --git a/vm/devices/get/get_protocol/src/dps_json.rs b/vm/devices/get/get_protocol/src/dps_json.rs index e36d056ec2..fc8a6b6ac6 100644 --- a/vm/devices/get/get_protocol/src/dps_json.rs +++ b/vm/devices/get/get_protocol/src/dps_json.rs @@ -41,6 +41,7 @@ pub struct HclDevicePlatformSettings { pub enable_battery: bool, pub enable_processor_idle: bool, pub enable_tpm: bool, + pub enable_ipmi: bool, pub com1: HclUartSettings, pub com2: HclUartSettings, #[serde(with = "serde_helpers::as_string")] @@ -304,10 +305,11 @@ mod test { #[test] fn smoke_test_sample() { - serde_json::from_slice::(include_bytes!( + let settings = serde_json::from_slice::(include_bytes!( "dps_test_json.json" )) .unwrap(); + assert!(!settings.v1.enable_ipmi); } #[test] @@ -317,4 +319,11 @@ mod test { )) .unwrap(); } + + #[test] + fn parse_ipmi_enabled() { + let settings = + serde_json::from_str::(r#"{"EnableIpmi":true}"#).unwrap(); + assert!(settings.enable_ipmi); + } } diff --git a/vm/devices/get/get_protocol/src/lib.rs b/vm/devices/get/get_protocol/src/lib.rs index 84cad15c8b..f1875772cc 100644 --- a/vm/devices/get/get_protocol/src/lib.rs +++ b/vm/devices/get/get_protocol/src/lib.rs @@ -17,6 +17,8 @@ use zerocopy::FromBytes; use zerocopy::Immutable; use zerocopy::IntoBytes; use zerocopy::KnownLayout; +use zerocopy::LittleEndian; +use zerocopy::U16; pub mod crash; pub mod dps_json; // TODO: split into separate crate, so get_protocol can be no_std @@ -117,6 +119,7 @@ open_enum! { START_VTL0_COMPLETED = 7, VTL_CRASH = 8, TRIPLE_FAULT = 9, + IPMI_SEL = 13, } } @@ -375,6 +378,60 @@ impl EventLogNotification { } } +pub const IPMI_SEL_RECORD_SIZE: usize = 16; + +#[repr(C)] +#[derive(Copy, Clone, Debug, IntoBytes, FromBytes, Immutable, KnownLayout)] +pub struct IpmiSelNotification { + pub message_header: HeaderHostNotification, + pub record_id: U16, + pub record: [u8; IPMI_SEL_RECORD_SIZE], +} + +const_assert_eq!(22, size_of::()); +const_assert_eq!(0, std::mem::offset_of!(IpmiSelNotification, message_header)); +const_assert_eq!(4, std::mem::offset_of!(IpmiSelNotification, record_id)); +const_assert_eq!(6, std::mem::offset_of!(IpmiSelNotification, record)); + +impl IpmiSelNotification { + pub fn new(record_id: u16, record: [u8; IPMI_SEL_RECORD_SIZE]) -> Self { + Self { + message_header: HeaderGeneric::new(HostNotifications::IPMI_SEL), + record_id: U16::new(record_id), + record, + } + } +} + +#[cfg(test)] +mod ipmi_sel_tests { + use super::*; + + #[test] + fn notification_wire_layout() { + let record_id = 0x1234; + let record = [ + 0x34, 0x12, 0x02, 0x78, 0x56, 0x34, 0x12, 0x20, 0x00, 0x04, 0x01, 0x6f, 0xaa, 0xbb, + 0xcc, 0xdd, + ]; + let notification = IpmiSelNotification::new(record_id, record); + + let mut expected = vec![1, 1, 13, 0, 0x34, 0x12]; + expected.extend_from_slice(&record); + assert_eq!(notification.as_bytes(), expected); + + let (decoded, remaining) = + IpmiSelNotification::read_from_prefix(&expected).expect("valid notification"); + assert!(remaining.is_empty()); + assert_eq!( + decoded.message_header.message_id(), + HostNotifications::IPMI_SEL + ); + assert_eq!(decoded.record_id.get(), record_id); + assert_eq!(decoded.record, record); + } +} + pub const TRACE_MSG_MAX_SIZE: usize = 256; open_enum! { #[derive(IntoBytes, FromBytes, Immutable, KnownLayout)] diff --git a/vm/devices/get/guest_emulation_device/src/lib.rs b/vm/devices/get/guest_emulation_device/src/lib.rs index 6909a22163..c87dce54ed 100644 --- a/vm/devices/get/guest_emulation_device/src/lib.rs +++ b/vm/devices/get/guest_emulation_device/src/lib.rs @@ -1102,6 +1102,9 @@ impl GedChannel { HostNotifications::EVENT_LOG => { self.handle_event_log(state, message_buf)?; } + HostNotifications::IPMI_SEL => { + self.handle_ipmi_sel(message_buf)?; + } HostNotifications::RESTORE_GUEST_VTL2_STATE_COMPLETED => { self.handle_restore_guest_vtl2_state_completed(message_buf)?; } @@ -1124,6 +1127,13 @@ impl GedChannel { Ok(()) } + fn handle_ipmi_sel(&mut self, message_buf: &[u8]) -> Result<(), Error> { + let _ = get_protocol::IpmiSelNotification::read_from_prefix(message_buf) + .map_err(|_| Error::MessageTooSmall)? + .0; // TODO: zerocopy: map_err (https://github.com/microsoft/openvmm/issues/759) + Ok(()) + } + fn handle_power_off( &mut self, message_buf: &[u8], diff --git a/vm/devices/get/guest_emulation_device/src/test_utilities.rs b/vm/devices/get/guest_emulation_device/src/test_utilities.rs index e4000f0387..ca62c5c46e 100644 --- a/vm/devices/get/guest_emulation_device/src/test_utilities.rs +++ b/vm/devices/get/guest_emulation_device/src/test_utilities.rs @@ -147,6 +147,15 @@ impl TestGedChannel { .0; // TODO: zerocopy: from-prefix (read_from_prefix): use-rest-of-range (https://github.com/microsoft/openvmm/issues/759) self.vmgs[0] = notification.event_log_id.0 as u8; } + HostNotifications::IPMI_SEL => { + let notification = get_protocol::IpmiSelNotification::read_from_prefix( + &message_buf[..size_of::()], + ) + .unwrap() + .0; // TODO: zerocopy: from-prefix (read_from_prefix): use-rest-of-range (https://github.com/microsoft/openvmm/issues/759) + let bytes = notification.as_bytes(); + self.vmgs[..bytes.len()].copy_from_slice(bytes); + } HostNotifications::POWER_OFF => { state.power_client.power_request(PowerRequest::PowerOff); } diff --git a/vm/devices/get/guest_emulation_transport/src/api.rs b/vm/devices/get/guest_emulation_transport/src/api.rs index 22f8b22390..7ab8f9ecd4 100644 --- a/vm/devices/get/guest_emulation_transport/src/api.rs +++ b/vm/devices/get/guest_emulation_transport/src/api.rs @@ -18,6 +18,9 @@ pub use get_protocol::ProtocolVersion; pub use get_protocol::SaveGuestVtl2StateFlags; pub use get_protocol::VmgsIoStatus; +/// A completed IPMI System Event Log record. +pub type IpmiSelRecord = [u8; get_protocol::IPMI_SEL_RECORD_SIZE]; + use guid::Guid; use mesh::MeshPayload; use std::time::Duration; @@ -87,6 +90,7 @@ pub mod platform_settings { pub battery_enabled: bool, pub processor_idle_enabled: bool, pub tpm_enabled: bool, + pub ipmi_enabled: bool, pub com1_enabled: bool, pub com1_debugger_mode: bool, diff --git a/vm/devices/get/guest_emulation_transport/src/client.rs b/vm/devices/get/guest_emulation_transport/src/client.rs index 45306cfe3d..7c1f0ec579 100644 --- a/vm/devices/get/guest_emulation_transport/src/client.rs +++ b/vm/devices/get/guest_emulation_transport/src/client.rs @@ -278,6 +278,8 @@ impl GuestEmulationTransportClient { battery_enabled: json.v1.enable_battery, processor_idle_enabled: json.v1.enable_processor_idle, tpm_enabled: json.v1.enable_tpm, + ipmi_enabled: json.v1.enable_ipmi, + com1_enabled: json.v1.com1.enable_port, com1_debugger_mode: json.v1.com1.debugger_mode, com1_vmbus_redirector: json.v1.com1.enable_vmbus_redirector, @@ -485,6 +487,13 @@ impl GuestEmulationTransportClient { self.control.notify(msg::Msg::EventLog(event_log_id.into())); } + /// Forwards a completed IPMI System Event Log record to the host. + /// + /// This function is non-blocking and does not wait for a host response. + pub fn ipmi_sel(&self, record_id: u16, record: crate::api::IpmiSelRecord) { + self.control.notify(msg::Msg::IpmiSel { record_id, record }); + } + /// This async method will only resolve after all outstanding event logs /// are written back to the host. pub async fn event_log_flush(&self) { diff --git a/vm/devices/get/guest_emulation_transport/src/lib.rs b/vm/devices/get/guest_emulation_transport/src/lib.rs index 41a12b679d..1a907e0d9b 100644 --- a/vm/devices/get/guest_emulation_transport/src/lib.rs +++ b/vm/devices/get/guest_emulation_transport/src/lib.rs @@ -349,6 +349,7 @@ mod tests { enable_vmbus_redirector: false, }, enable_firmware_debugging: true, + enable_ipmi: true, ..Default::default() }, v2: get_protocol::dps_json::HclDevicePlatformSettingsV2 { @@ -388,6 +389,7 @@ mod tests { assert_eq!(dps.general.tpm_enabled, false); assert_eq!(dps.general.com1_enabled, true); assert_eq!(dps.general.secure_boot_enabled, false); + assert!(dps.general.ipmi_enabled); assert_eq!(dps.general.legacy_memory_map, true); assert_eq!(dps.general.pxe_ip_v6, true); @@ -428,6 +430,44 @@ mod tests { assert_eq!(read_buf[0], 5); } + #[async_test] + async fn test_send_ipmi_sel_notification(driver: DefaultDriver) { + let record_id = 0x1234; + let record = [ + 0x34, 0x12, 0x02, 0x78, 0x56, 0x34, 0x12, 0x20, 0x00, 0x04, 0x01, 0x6f, 0xaa, 0xbb, + 0xcc, 0xdd, + ]; + let expected = get_protocol::IpmiSelNotification::new(record_id, record) + .as_bytes() + .to_vec(); + + let vmgs_read_response = TestGetResponses::new(Event::Response( + get_protocol::VmgsReadResponse::new(VmgsIoStatus::SUCCESS) + .as_bytes() + .to_vec(), + )); + let ged_responses = vec![TestGetResponses::default(), vmgs_read_response]; + + let get = new_transport_pair( + driver, + Some(ged_responses), + ProtocolVersion::NICKEL_REV2, + None, + None, + ) + .await; + + get.client.ipmi_sel(record_id, record); + + let read_buf = get + .client + .vmgs_read(0, 1, TEST_VMGS_SECTOR_SIZE) + .await + .unwrap(); + + assert_eq!(&read_buf[..expected.len()], expected.as_slice()); + } + #[async_test] async fn notification_in_between_requests(driver: DefaultDriver) { let time_response = TestGetResponses::new(Event::Response( diff --git a/vm/devices/get/guest_emulation_transport/src/process_loop.rs b/vm/devices/get/guest_emulation_transport/src/process_loop.rs index 3f1d203713..aca3d8a473 100644 --- a/vm/devices/get/guest_emulation_transport/src/process_loop.rs +++ b/vm/devices/get/guest_emulation_transport/src/process_loop.rs @@ -347,6 +347,11 @@ pub(crate) mod msg { // Host Notifications (don't require a response) /// Report an event to the host. EventLog(Protocol), + /// Forward a completed IPMI System Event Log record to the host. + IpmiSel { + record_id: u16, + record: [u8; get_protocol::IPMI_SEL_RECORD_SIZE], + }, /// Report a power state change to the host. PowerState(PowerState), /// Report the result of a restore operation to the host. @@ -1296,6 +1301,13 @@ impl ProcessLoop { .to_vec(), ); } + Msg::IpmiSel { record_id, record } => { + self.send_message( + get_protocol::IpmiSelNotification::new(record_id, record) + .as_bytes() + .to_vec(), + ); + } Msg::ReportRestoreResultToHost(success) => self.report_restore_result_to_host(success), Msg::VtlCrashNotification(crash_notification) => { // Send the crash notification right away, jumping the line in front of diff --git a/vm/devices/get/guest_emulation_transport/src/resolver.rs b/vm/devices/get/guest_emulation_transport/src/resolver.rs index 48a50159b1..90f00a92c8 100644 --- a/vm/devices/get/guest_emulation_transport/src/resolver.rs +++ b/vm/devices/get/guest_emulation_transport/src/resolver.rs @@ -4,6 +4,11 @@ //! Resource definitions for the GET client. use crate::GuestEmulationTransportClient; +use chipset_resources::IPMI_SEL_RECORD_SIZE; +use chipset_resources::IpmiSelEventSinkHandleKind; +use chipset_resources::ResolvedIpmiSelEventSink; +use chipset_resources::SelEventDisposition; +use chipset_resources::SelEventSink; use std::convert::Infallible; use vm_resource::CanResolveTo; use vm_resource::PlatformResource; @@ -35,3 +40,34 @@ impl ResolveResource for GuestEmulationTranspor Ok(self.clone()) } } + +struct GetIpmiSelEventSink(GuestEmulationTransportClient); + +impl SelEventSink for GetIpmiSelEventSink { + fn try_send( + &mut self, + record_id: u16, + record: [u8; IPMI_SEL_RECORD_SIZE], + ) -> SelEventDisposition { + self.0.ipmi_sel(record_id, record); + SelEventDisposition::Accepted + } +} + +/// Resolves the platform IPMI SEL event sink to GET. +pub struct IpmiSelEventSinkResolver(pub GuestEmulationTransportClient); + +impl ResolveResource for IpmiSelEventSinkResolver { + type Output = ResolvedIpmiSelEventSink; + type Error = Infallible; + + fn resolve( + &self, + PlatformResource: PlatformResource, + (): (), + ) -> Result { + Ok(ResolvedIpmiSelEventSink(Box::new(GetIpmiSelEventSink( + self.0.clone(), + )))) + } +} diff --git a/vm/loader/src/uefi/config.rs b/vm/loader/src/uefi/config.rs index ae71dfd3e5..71609dab1f 100644 --- a/vm/loader/src/uefi/config.rs +++ b/vm/loader/src/uefi/config.rs @@ -336,8 +336,9 @@ pub struct Flags { pub vmbus_disabled: bool, pub pci_resources_pre_assigned: bool, pub force_dma_bounce_enabled: bool, + pub ipmi_enabled: bool, - #[bits(31)] + #[bits(30)] _reserved: u64, } @@ -454,6 +455,11 @@ pub struct PcieBarApertureEntry { mod tests { use super::*; + #[test] + fn ipmi_enabled_is_bit_33() { + assert_eq!(Flags::new().with_ipmi_enabled(true).into_bits(), 1 << 33); + } + fn read(bytes: &[u8]) -> T where T: FromBytes + Immutable + KnownLayout, diff --git a/vmm_core/vm_manifest_builder/src/lib.rs b/vmm_core/vm_manifest_builder/src/lib.rs index c563273250..f18874e53a 100644 --- a/vmm_core/vm_manifest_builder/src/lib.rs +++ b/vmm_core/vm_manifest_builder/src/lib.rs @@ -26,6 +26,8 @@ use chipset_resources::i440bx_host_pci_bridge::I440BX_HOST_PCI_BRIDGE_BDF; use chipset_resources::i440bx_host_pci_bridge::I440BxHostPciBridgeDeviceHandle; use chipset_resources::i8042::I8042DeviceHandle; use chipset_resources::ioapic::GenericIoApicDeviceHandle; +use chipset_resources::ipmi_kcs::IpmiKcsDeviceHandleAArch64; +use chipset_resources::ipmi_kcs::IpmiKcsDeviceHandleX64; use chipset_resources::isa_dma::GenericIsaDmaDeviceHandle; use chipset_resources::pic::PicDeviceHandle; use chipset_resources::piix4_pci_isa_bridge::PIIX4_PCI_ISA_BRIDGE_BDF; @@ -78,6 +80,7 @@ pub struct VmManifestBuilder { battery_status_recv: Option>, framebuffer: bool, guest_watchdog: bool, + ipmi_kcs: bool, psp: bool, platform_pm_timer_assist: bool, uefi: Option, @@ -290,6 +293,7 @@ impl VmManifestBuilder { battery_status_recv: None, framebuffer: false, guest_watchdog: false, + ipmi_kcs: false, psp: false, platform_pm_timer_assist: false, uefi: None, @@ -388,6 +392,15 @@ impl VmManifestBuilder { self } + /// Enable the architecture-specific IPMI KCS virtual BMC. + /// + /// This is supported only for UEFI-booted guests. + pub fn with_ipmi_kcs(mut self) -> Self { + assert!(matches!(self.ty, BaseChipsetType::HypervGen2Uefi)); + self.ipmi_kcs = true; + self + } + /// Enable the AMD64 PSP device. pub fn with_psp(mut self) -> Self { self.psp = true; @@ -622,6 +635,9 @@ impl VmManifestBuilder { result.attach_guest_watchdog(); } if matches!(self.ty, BaseChipsetType::HypervGen2Uefi) { + if self.ipmi_kcs { + result.attach_ipmi_kcs(self.arch); + } result.attach_uefi( self.uefi .expect("must have called .with_uefi to enable uefi"), @@ -787,6 +803,26 @@ impl VmChipsetResult { self } + fn attach_ipmi_kcs(&mut self, arch: MachineArch) -> &mut Self { + let resource = match arch { + MachineArch::X86_64 => IpmiKcsDeviceHandleX64 { + event_sink: PlatformResource.into_resource(), + time_source: PlatformResource.into_resource(), + } + .into_resource(), + MachineArch::Aarch64 => IpmiKcsDeviceHandleAArch64 { + event_sink: PlatformResource.into_resource(), + time_source: PlatformResource.into_resource(), + } + .into_resource(), + }; + self.chipset_devices.push(ChipsetDeviceHandle { + name: "ipmi-kcs".to_owned(), + resource, + }); + self + } + fn attach_hyperv_power_management(&mut self, platform_pm_timer_assist: bool) -> &mut Self { let pm_timer_assist = platform_pm_timer_assist.then(|| PlatformResource.into_resource()); self.chipset_devices.push(ChipsetDeviceHandle { @@ -1032,6 +1068,19 @@ mod tests { [(); 4].map(|_| None) } + fn uefi_builder(arch: MachineArch) -> VmManifestBuilder { + VmManifestBuilder::new(BaseChipsetType::HypervGen2Uefi, arch).with_uefi(UefiManifest::new( + arch, + None, + None, + false, + LogLevel::default(), + None, + PlatformResource.into_resource(), + None, + )) + } + #[test] fn serial_debugger_mode_builder_flag_defaults_false_and_can_enable() { let builder = VmManifestBuilder::new(BaseChipsetType::HypervGen1, MachineArch::X86_64); @@ -1041,6 +1090,43 @@ mod tests { assert_eq!(builder.serial_debugger_mode, [true, false, false, true]); } + #[test] + fn ipmi_kcs_is_disabled_by_default() { + let manifest = uefi_builder(MachineArch::X86_64).build().unwrap(); + + assert!( + manifest + .chipset_devices + .iter() + .all(|device| device.name != "ipmi-kcs") + ); + } + + #[test] + fn ipmi_kcs_opt_in_uses_arch_specific_resource() { + for (arch, resource_id) in [ + (MachineArch::X86_64, "ipmi-kcs-x64"), + (MachineArch::Aarch64, "ipmi-kcs-aarch64"), + ] { + let manifest = uefi_builder(arch).with_ipmi_kcs().build().unwrap(); + let ipmi_devices: Vec<_> = manifest + .chipset_devices + .iter() + .filter(|device| device.name == "ipmi-kcs") + .collect(); + + assert_eq!(ipmi_devices.len(), 1); + assert_eq!(ipmi_devices[0].resource.id(), resource_id); + } + } + + #[test] + #[should_panic] + fn ipmi_kcs_rejects_non_uefi_boot() { + let _ = VmManifestBuilder::new(BaseChipsetType::HyperVGen2LinuxDirect, MachineArch::X86_64) + .with_ipmi_kcs(); + } + #[test] fn serial_debugger_mode_defaults_false_on_generated_handles() { let serial_16550 = serial_16550_devices(false, [false; 4], no_serial_backends()); From 34a2bda496529a825ee9aa643cc2ff254c2c2929 Mon Sep 17 00:00:00 2001 From: Ayush Arora Date: Wed, 9 Sep 2026 15:38:40 +0530 Subject: [PATCH 03/10] test: add OpenHCL IPMI SEL vmm coverage Enable IPMI in OpenVMM-hosted OpenHCL tests and observe host SEL notifications. Add Linux and Windows guest coverage for the KCS Add SEL path. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: dda9bd55-db87-46d6-a512-15d8a769b9a3 --- openvmm/openvmm_entry/src/lib.rs | 2 + petri/src/vm/mod.rs | 10 + petri/src/vm/openvmm/construct.rs | 9 + petri/src/vm/openvmm/mod.rs | 1 + petri/src/vm/openvmm/runtime.rs | 14 ++ vm/devices/get/get_resources/src/lib.rs | 13 ++ .../get/guest_emulation_device/src/lib.rs | 28 ++- .../guest_emulation_device/src/resolver.rs | 2 + .../src/test_utilities.rs | 2 + vmm_tests/vmm_tests/tests/tests/x86_64.rs | 1 + .../vmm_tests/tests/tests/x86_64/ipmi.rs | 196 ++++++++++++++++++ 11 files changed, 275 insertions(+), 3 deletions(-) create mode 100644 vmm_tests/vmm_tests/tests/tests/x86_64/ipmi.rs diff --git a/openvmm/openvmm_entry/src/lib.rs b/openvmm/openvmm_entry/src/lib.rs index 039791f1d3..7b3937b97c 100644 --- a/openvmm/openvmm_entry/src/lib.rs +++ b/openvmm/openvmm_entry/src/lib.rs @@ -1576,6 +1576,7 @@ async fn vm_config_from_command_line( TpmVersionCli::V185 => get_resources::ged::GedTpmVersion::V185, }), firmware_event_send: None, + ipmi_sel_event_send: None, secure_boot_enabled: opt.secure_boot, secure_boot_template: match opt.secure_boot_template { Some(SecureBootTemplateCli::Windows) => { @@ -1589,6 +1590,7 @@ async fn vm_config_from_command_line( }, }, enable_battery: opt.battery, + enable_ipmi: false, enable_hibernation: opt.hibernation, no_persistent_secrets: true, igvm_attest_test_config: None, diff --git a/petri/src/vm/mod.rs b/petri/src/vm/mod.rs index dd4265e395..b14adb6257 100644 --- a/petri/src/vm/mod.rs +++ b/petri/src/vm/mod.rs @@ -233,6 +233,8 @@ pub struct PetriVmConfig { pub firmware: Firmware, /// Whether to enable guest hibernation support. pub hibernation_enabled: bool, + /// Whether to expose an IPMI KCS interface to the guest. + pub ipmi_enabled: bool, /// The amount of memory, in bytes, to assign to the VM pub memory: MemoryConfig, /// The processor topology for the VM @@ -461,6 +463,7 @@ impl PetriVmBuilder { host_log_levels: None, firmware: artifacts.firmware, hibernation_enabled: false, + ipmi_enabled: false, memory: Default::default(), proc_topology: Default::default(), @@ -543,6 +546,7 @@ impl PetriVmBuilder { host_log_levels: None, firmware: artifacts.firmware, hibernation_enabled: false, + ipmi_enabled: false, memory: Default::default(), proc_topology: Default::default(), @@ -1543,6 +1547,12 @@ impl PetriVmBuilder { self } + /// Enable the IPMI KCS interface for an OpenHCL UEFI VM. + pub fn with_ipmi(mut self, enable: bool) -> Self { + self.config.ipmi_enabled = enable; + self + } + /// Specify the guest state lifetime for the VM pub fn with_guest_state_lifetime( mut self, diff --git a/petri/src/vm/openvmm/construct.rs b/petri/src/vm/openvmm/construct.rs index 4a5e332b78..f10940405e 100644 --- a/petri/src/vm/openvmm/construct.rs +++ b/petri/src/vm/openvmm/construct.rs @@ -122,6 +122,7 @@ impl PetriVmConfigOpenVmm { host_log_levels, firmware, hibernation_enabled, + ipmi_enabled, memory, proc_topology, vmgs, @@ -150,6 +151,7 @@ impl PetriVmConfigOpenVmm { arch, firmware: &firmware, hibernation_enabled, + ipmi_enabled, driver, logger: log_source, vmgs: &vmgs, @@ -296,6 +298,7 @@ impl PetriVmConfigOpenVmm { } let (firmware_event_send, firmware_event_recv) = mesh::mpsc_channel(); + let (ipmi_sel_event_send, ipmi_sel_event_recv) = mesh::mpsc_channel(); let make_vsock_listener = || -> anyhow::Result<(UnixListener, TempPath)> { Ok(tempfile::Builder::new() @@ -309,6 +312,7 @@ impl PetriVmConfigOpenVmm { &mut emulated_serial_config, &mut vmbus_devices, &firmware_event_send, + &ipmi_sel_event_send, framebuffer.is_some(), ) .await?; @@ -744,6 +748,7 @@ impl PetriVmConfigOpenVmm { resources: PetriVmResourcesOpenVmm { log_stream_tasks, firmware_event_recv, + ipmi_sel_event_recv, shutdown_ic_send, kvp_ic_send, ged_send, @@ -778,6 +783,7 @@ struct PetriVmConfigSetupCore<'a> { arch: MachineArch, firmware: &'a Firmware, hibernation_enabled: bool, + ipmi_enabled: bool, driver: &'a DefaultDriver, logger: &'a PetriLogSource, vmgs: &'a PetriVmgsResource, @@ -1109,6 +1115,7 @@ impl PetriVmConfigSetupCore<'_> { serial: &mut [Option>], devices: &mut impl Extend<(DeviceVtl, Resource)>, firmware_event_send: &mesh::Sender, + ipmi_sel_event_send: &mesh::Sender, framebuffer: bool, ) -> anyhow::Result<( get_resources::ged::GuestEmulationDeviceHandle, @@ -1190,6 +1197,7 @@ impl PetriVmConfigSetupCore<'_> { PetriTpmVersion::V138 => get_resources::ged::GedTpmVersion::V138, }), firmware_event_send: Some(firmware_event_send.clone()), + ipmi_sel_event_send: Some(ipmi_sel_event_send.clone()), secure_boot_enabled: *secure_boot_enabled, secure_boot_template: match secure_boot_template { Some(SecureBootTemplate::MicrosoftWindows) => { @@ -1202,6 +1210,7 @@ impl PetriVmConfigSetupCore<'_> { }, enable_battery: false, enable_hibernation: self.hibernation_enabled, + enable_ipmi: self.ipmi_enabled, no_persistent_secrets: self.tpm_config.as_ref().is_some_and(|c| c.no_persistent_secrets), igvm_attest_test_config: None, test_gsp_by_id, diff --git a/petri/src/vm/openvmm/mod.rs b/petri/src/vm/openvmm/mod.rs index 2b00d38147..6b4d3d9a72 100644 --- a/petri/src/vm/openvmm/mod.rs +++ b/petri/src/vm/openvmm/mod.rs @@ -190,6 +190,7 @@ pub struct PetriVmConfigOpenVmm { struct PetriVmResourcesOpenVmm { log_stream_tasks: Vec>>, firmware_event_recv: Receiver, + ipmi_sel_event_recv: Receiver, shutdown_ic_send: Option>, kvp_ic_send: Option>, ged_send: Option>, diff --git a/petri/src/vm/openvmm/runtime.rs b/petri/src/vm/openvmm/runtime.rs index 5470176bfb..082b75efd0 100644 --- a/petri/src/vm/openvmm/runtime.rs +++ b/petri/src/vm/openvmm/runtime.rs @@ -20,6 +20,7 @@ use framebuffer::View; use futures::FutureExt; use futures_concurrency::future::Race; use get_resources::ged::FirmwareEvent; +use get_resources::ged::IpmiSelEvent; use hyperv_ic_resources::shutdown::ShutdownRpc; use mesh::CancelContext; use mesh::Receiver; @@ -274,6 +275,10 @@ impl PetriVmOpenVmm { /// returns that status. pub async fn wait_for_boot_event(&mut self) -> anyhow::Result ); + petri_vm_fn!( + /// Waits for an IPMI SEL notification received from OpenHCL. + pub async fn wait_for_ipmi_sel(&mut self) -> anyhow::Result + ); petri_vm_fn!( /// Waits for the Hyper-V shutdown IC to be ready, returning a receiver /// that will be closed when it is no longer ready. Returns `None` if @@ -444,6 +449,15 @@ impl PetriVmInner { .context("Failed to get firmware boot event") } + async fn wait_for_ipmi_sel(&mut self) -> anyhow::Result { + CancelContext::new() + .with_timeout(Duration::from_secs(30)) + .until_cancelled(self.resources.ipmi_sel_event_recv.recv()) + .await + .context("timed out waiting for an IPMI SEL host notification")? + .context("IPMI SEL host notification channel closed") + } + async fn wait_for_enlightened_shutdown_ready( &mut self, ) -> anyhow::Result>> { diff --git a/vm/devices/get/get_resources/src/lib.rs b/vm/devices/get/get_resources/src/lib.rs index 81fdfe6f16..b09efc0241 100644 --- a/vm/devices/get/get_resources/src/lib.rs +++ b/vm/devices/get/get_resources/src/lib.rs @@ -81,12 +81,16 @@ pub mod ged { pub guest_request_recv: mesh::Receiver, /// Notification of firmware events. pub firmware_event_send: Option>, + /// Test observer for IPMI SEL notifications received from OpenHCL. + pub ipmi_sel_event_send: Option>, /// Enable secure boot. pub secure_boot_enabled: bool, /// The secure boot template type. pub secure_boot_template: GuestSecureBootTemplateType, /// Enable battery. pub enable_battery: bool, + /// Enable the IPMI KCS interface. + pub enable_ipmi: bool, /// Suppress attestation and disable TPM state persistence. pub no_persistent_secrets: bool, /// Test configuration for IGVM Attest message. @@ -103,6 +107,15 @@ pub mod ged { pub smbios: smbios_defs::SmbiosConfig, } + /// An IPMI SEL notification received from OpenHCL. + #[derive(Debug, Clone, Copy, MeshPayload, PartialEq, Eq)] + pub struct IpmiSelEvent { + /// BMC-assigned SEL record identifier. + pub record_id: u16, + /// Completed SEL record. + pub record: [u8; 16], + } + /// The firmware and chipset configuration for the guest. #[derive(MeshPayload)] pub enum GuestFirmwareConfig { diff --git a/vm/devices/get/guest_emulation_device/src/lib.rs b/vm/devices/get/guest_emulation_device/src/lib.rs index d275b6d11e..af30c9cbb5 100644 --- a/vm/devices/get/guest_emulation_device/src/lib.rs +++ b/vm/devices/get/guest_emulation_device/src/lib.rs @@ -50,6 +50,7 @@ use get_protocol::dps_json::PcatBootDevice; use get_resources::ged::FirmwareEvent; use get_resources::ged::GuestEmulationRequest; use get_resources::ged::GuestServicingFlags; +use get_resources::ged::IpmiSelEvent; use get_resources::ged::ModifyVtl2SettingsError; use get_resources::ged::SaveRestoreError; use get_resources::ged::Vtl0StartError; @@ -152,6 +153,8 @@ pub struct GuestConfig { pub secure_boot_template: SecureBootTemplateType, /// Enable battery. pub enable_battery: bool, + /// Enable the IPMI KCS interface. + pub enable_ipmi: bool, /// Enable hibernation. pub enable_hibernation: bool, /// Suppress attestation. @@ -229,6 +232,8 @@ pub struct GuestEmulationDevice { #[inspect(skip)] firmware_event_send: Option>, #[inspect(skip)] + ipmi_sel_event_send: Option>, + #[inspect(skip)] framebuffer_control: Option>, #[inspect(skip)] guest_request_recv: mesh::Receiver, @@ -264,6 +269,7 @@ impl GuestEmulationDevice { config: GuestConfig, power_client: PowerRequestClient, firmware_event_send: Option>, + ipmi_sel_event_send: Option>, guest_request_recv: mesh::Receiver, framebuffer_control: Option>, vmgs_disk: Option, @@ -274,6 +280,7 @@ impl GuestEmulationDevice { config, power_client, firmware_event_send, + ipmi_sel_event_send, framebuffer_control, guest_request_recv, vmgs: vmgs_disk.map(|disk| VmgsState { @@ -294,6 +301,12 @@ impl GuestEmulationDevice { sender.send(event); } } + + fn send_ipmi_sel_event(&self, event: IpmiSelEvent) { + if let Some(sender) = &self.ipmi_sel_event_send { + sender.send(event); + } + } } #[async_trait] @@ -1110,7 +1123,7 @@ impl GedChannel { self.handle_event_log(state, message_buf)?; } HostNotifications::IPMI_SEL => { - self.handle_ipmi_sel(message_buf)?; + self.handle_ipmi_sel(state, message_buf)?; } HostNotifications::RESTORE_GUEST_VTL2_STATE_COMPLETED => { self.handle_restore_guest_vtl2_state_completed(message_buf)?; @@ -1134,10 +1147,18 @@ impl GedChannel { Ok(()) } - fn handle_ipmi_sel(&mut self, message_buf: &[u8]) -> Result<(), Error> { - let _ = get_protocol::IpmiSelNotification::read_from_prefix(message_buf) + fn handle_ipmi_sel( + &mut self, + state: &GuestEmulationDevice, + message_buf: &[u8], + ) -> Result<(), Error> { + let notification = get_protocol::IpmiSelNotification::read_from_prefix(message_buf) .map_err(|_| Error::MessageTooSmall)? .0; // TODO: zerocopy: map_err (https://github.com/microsoft/openvmm/issues/759) + state.send_ipmi_sel_event(IpmiSelEvent { + record_id: notification.record_id.get(), + record: notification.record, + }); Ok(()) } @@ -1413,6 +1434,7 @@ impl GedChannel { _ => panic!("Invalid secure boot template"), }, enable_battery: state.config.enable_battery, + enable_ipmi: state.config.enable_ipmi, enable_hibernation: state.config.enable_hibernation, console_mode: uefi_console_mode.unwrap_or(UefiConsoleMode::DEFAULT).0, bios_guid: if state.test_gsp_by_id { diff --git a/vm/devices/get/guest_emulation_device/src/resolver.rs b/vm/devices/get/guest_emulation_device/src/resolver.rs index c234ec6cf0..1f78732d57 100644 --- a/vm/devices/get/guest_emulation_device/src/resolver.rs +++ b/vm/devices/get/guest_emulation_device/src/resolver.rs @@ -197,6 +197,7 @@ impl AsyncResolveResource } }, enable_battery: resource.enable_battery, + enable_ipmi: resource.enable_ipmi, enable_hibernation: resource.enable_hibernation, no_persistent_secrets: resource.no_persistent_secrets, guest_state_lifetime, @@ -225,6 +226,7 @@ impl AsyncResolveResource }, halt, resource.firmware_event_send, + resource.ipmi_sel_event_send, resource.guest_request_recv, framebuffer_control, vmgs_disk, diff --git a/vm/devices/get/guest_emulation_device/src/test_utilities.rs b/vm/devices/get/guest_emulation_device/src/test_utilities.rs index 3e8494157e..53dc21bb4c 100644 --- a/vm/devices/get/guest_emulation_device/src/test_utilities.rs +++ b/vm/devices/get/guest_emulation_device/src/test_utilities.rs @@ -266,6 +266,7 @@ pub fn create_host_channel( secure_boot_enabled: false, secure_boot_template: SecureBootTemplateType::SECURE_BOOT_DISABLED, enable_battery: false, + enable_ipmi: false, enable_hibernation: false, no_persistent_secrets: true, guest_state_lifetime: Default::default(), @@ -294,6 +295,7 @@ pub fn create_host_channel( guest_config, halt.into(), None, + None, recv, None, Some(disklayer_ram::ram_disk(TEST_VMGS_CAPACITY as u64, false).unwrap()), diff --git a/vmm_tests/vmm_tests/tests/tests/x86_64.rs b/vmm_tests/vmm_tests/tests/tests/x86_64.rs index 9730867968..6a1af29d6c 100644 --- a/vmm_tests/vmm_tests/tests/tests/x86_64.rs +++ b/vmm_tests/vmm_tests/tests/tests/x86_64.rs @@ -3,6 +3,7 @@ //! Integration tests for x86_64 guests. +mod ipmi; mod openhcl_linux_direct; mod openhcl_uefi; mod storage; diff --git a/vmm_tests/vmm_tests/tests/tests/x86_64/ipmi.rs b/vmm_tests/vmm_tests/tests/tests/x86_64/ipmi.rs new file mode 100644 index 0000000000..bcc539e664 --- /dev/null +++ b/vmm_tests/vmm_tests/tests/tests/x86_64/ipmi.rs @@ -0,0 +1,196 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +//! Integration tests for the OpenHCL IPMI KCS interface. + +use anyhow::Context; +use petri::PetriVmBuilder; +use petri::openvmm::OpenVmmPetriBackend; +use petri::pipette::cmd; +use petri_artifacts_common::tags::OsFlavor; +use vmm_test_macros::openvmm_test; + +const LINUX_IPMI_TEST: &str = r#" +import ctypes +import os +import select + +IPMI_SYSTEM_INTERFACE_ADDR_TYPE = 0x0c +IPMI_BMC_CHANNEL = 0x0f +IPMI_RESPONSE_RECV_TYPE = 1 + +class IpmiSystemInterfaceAddr(ctypes.Structure): + _fields_ = [ + ("addr_type", ctypes.c_int), + ("channel", ctypes.c_short), + ("lun", ctypes.c_ubyte), + ] + +class IpmiMsg(ctypes.Structure): + _fields_ = [ + ("netfn", ctypes.c_ubyte), + ("cmd", ctypes.c_ubyte), + ("data_len", ctypes.c_ushort), + ("data", ctypes.c_void_p), + ] + +class IpmiReq(ctypes.Structure): + _fields_ = [ + ("addr", ctypes.c_void_p), + ("addr_len", ctypes.c_uint), + ("msgid", ctypes.c_long), + ("msg", IpmiMsg), + ] + +class IpmiRecv(ctypes.Structure): + _fields_ = [ + ("recv_type", ctypes.c_int), + ("addr", ctypes.c_void_p), + ("addr_len", ctypes.c_uint), + ("msgid", ctypes.c_long), + ("msg", IpmiMsg), + ] + +def ioctl_code(direction, number, size): + return (direction << 30) | (size << 16) | (ord("i") << 8) | number + +def ioctl(fd, request, value): + result = libc.ioctl(fd, request, ctypes.byref(value)) + if result != 0: + error = ctypes.get_errno() + raise OSError(error, os.strerror(error)) + +device_path = next( + (path for path in ("/dev/ipmi0", "/dev/ipmi/0", "/dev/ipmidev/0") if os.path.exists(path)), + None, +) +if device_path is None: + raise RuntimeError("Linux IPMI device was not created") + +libc = ctypes.CDLL(None, use_errno=True) +libc.ioctl.argtypes = [ctypes.c_int, ctypes.c_ulong, ctypes.c_void_p] +libc.ioctl.restype = ctypes.c_int + +address = IpmiSystemInterfaceAddr(IPMI_SYSTEM_INTERFACE_ADDR_TYPE, IPMI_BMC_CHANNEL, 0) +request_data = (ctypes.c_ubyte * 16)( + 0x00, 0x00, 0x02, 0x00, 0x00, 0x00, 0x00, 0x20, + 0x00, 0x04, 0x09, 0x01, 0x6f, 0xde, 0xad, 0xbe, +) +request = IpmiReq( + ctypes.addressof(address), + ctypes.sizeof(address), + 1, + IpmiMsg(0x0a, 0x44, len(request_data), ctypes.addressof(request_data)), +) + +fd = os.open(device_path, os.O_RDWR) +try: + ioctl(fd, ioctl_code(2, 13, ctypes.sizeof(IpmiReq)), request) + if not select.select([fd], [], [], 10)[0]: + raise TimeoutError("timed out waiting for the Add SEL response") + + response_address = (ctypes.c_ubyte * 32)() + response_data = (ctypes.c_ubyte * 64)() + response = IpmiRecv( + 0, + ctypes.addressof(response_address), + len(response_address), + 0, + IpmiMsg(0, 0, len(response_data), ctypes.addressof(response_data)), + ) + ioctl(fd, ioctl_code(3, 11, ctypes.sizeof(IpmiRecv)), response) + + data = bytes(response_data[:response.msg.data_len]) + if response.recv_type != IPMI_RESPONSE_RECV_TYPE: + raise RuntimeError(f"unexpected receive type {response.recv_type}") + if response.msgid != 1 or response.msg.cmd != 0x44: + raise RuntimeError( + f"unexpected response msgid={response.msgid} command={response.msg.cmd:#x}" + ) + if len(data) != 3 or data[0] != 0: + raise RuntimeError(f"Add SEL failed: {data.hex()}") + + print(f"ADDSEL_CC=0 RECORD_ID={int.from_bytes(data[1:3], 'little')}") +finally: + os.close(fd) +"#; + +const WINDOWS_IPMI_TEST: &str = r#" +$ipmi = Get-CimInstance -Namespace root\wmi -ClassName Microsoft_IPMI -ErrorAction Stop +$record = [byte[]]( + 0x00,0x00,0x02,0x00,0x00,0x00,0x00,0x20, + 0x00,0x04,0x09,0x01,0x6f,0xde,0xad,0xbe +) +$response = Invoke-CimMethod -InputObject $ipmi -MethodName RequestResponse -Arguments @{ + NetworkFunction = [byte]0x0A + Lun = [byte]0x00 + ResponderAddress = [byte]0x20 + Command = [byte]0x44 + RequestData = $record + RequestDataSize = [uint32]$record.Length +} -ErrorAction Stop +if ($response.CompletionCode -ne 0) { + throw "Add SEL failed with completion code $($response.CompletionCode)" +} +Write-Output "ADDSEL_CC=0" +"#; + +#[openvmm_test( + openhcl_uefi_x64(vhd(ubuntu_2504_server_x64)), + openhcl_uefi_x64(vhd(windows_datacenter_core_2022_x64)) +)] +async fn ipmi_kcs_add_sel(config: PetriVmBuilder) -> anyhow::Result<()> { + let os_flavor = config.os_flavor(); + let (mut vm, agent) = config.with_ipmi(true).run().await?; + + let output = match os_flavor { + OsFlavor::Linux => { + let shell = agent.unix_shell(); + cmd!(shell, "sudo modprobe ipmi_si").run().await?; + cmd!(shell, "sudo modprobe ipmi_devintf").run().await?; + agent + .write_file("/tmp/ipmi_test.py", LINUX_IPMI_TEST.as_bytes()) + .await + .context("failed to copy the Linux IPMI test into the guest")?; + cmd!(shell, "sudo python3 /tmp/ipmi_test.py").read().await? + } + OsFlavor::Windows => { + let shell = agent.windows_shell(); + cmd!(shell, "powershell.exe") + .args([ + "-NoProfile", + "-NonInteractive", + "-Command", + WINDOWS_IPMI_TEST, + ]) + .read() + .await? + } + _ => unreachable!(), + }; + + anyhow::ensure!( + output.contains("ADDSEL_CC=0"), + "guest did not successfully add an IPMI SEL record: {output}" + ); + + let notification = loop { + let notification = vm.backend().wait_for_ipmi_sel().await?; + if notification.record[2] == 0x02 + && notification.record[7..] == [0x20, 0x00, 0x04, 0x09, 0x01, 0x6f, 0xde, 0xad, 0xbe] + { + break notification; + } + + tracing::info!(?notification, "ignoring an unrelated IPMI SEL notification"); + }; + anyhow::ensure!( + notification.record_id != 0 + && notification.record[0..2] == notification.record_id.to_le_bytes(), + "host received an invalid IPMI SEL record ID: {notification:?}" + ); + + agent.power_off().await?; + vm.wait_for_clean_teardown().await?; + Ok(()) +} From bbd21238c8b6c98c82fec877857b4346d6b27655 Mon Sep 17 00:00:00 2001 From: Ayush Arora Date: Thu, 10 Sep 2026 12:31:35 +0530 Subject: [PATCH 04/10] refactor: address IPMI review feedback Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: dda9bd55-db87-46d6-a512-15d8a769b9a3 --- Cargo.lock | 12 + Cargo.toml | 1 + .../src/igvm_attest/get.rs | 6 +- .../src/igvm_attest/mod.rs | 2 +- vm/devices/chipset/ipmi_kcs/Cargo.toml | 2 + vm/devices/chipset/ipmi_kcs/src/device.rs | 8 +- vm/devices/chipset/ipmi_kcs/src/lib.rs | 70 +-- vm/devices/chipset/ipmi_kcs/src/protocol.rs | 106 ++-- vm/devices/chipset/ipmi_kcs/src/resolver.rs | 8 +- .../chipset/ipmi_kcs/src/save_restore.rs | 4 +- vm/devices/chipset/ipmi_kcs/src/sel.rs | 179 ++++--- vm/devices/chipset/ipmi_kcs/src/tests.rs | 24 +- vm/devices/chipset/ipmi_protocol/Cargo.toml | 14 + vm/devices/chipset/ipmi_protocol/src/lib.rs | 486 ++++++++++++++++++ vm/devices/chipset_resources/Cargo.toml | 3 +- vm/devices/chipset_resources/src/lib.rs | 69 ++- vm/devices/get/get_protocol/Cargo.toml | 1 + vm/devices/get/get_protocol/src/dps_json.rs | 15 +- vm/devices/get/get_protocol/src/lib.rs | 2 +- .../guest_emulation_transport/src/resolver.rs | 18 +- 20 files changed, 771 insertions(+), 259 deletions(-) create mode 100644 vm/devices/chipset/ipmi_protocol/Cargo.toml create mode 100644 vm/devices/chipset/ipmi_protocol/src/lib.rs diff --git a/Cargo.lock b/Cargo.lock index 06faf653df..fb9e4f6eca 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -716,6 +716,7 @@ version = "0.0.0" dependencies = [ "arbitrary", "inspect", + "ipmi_protocol", "local_clock", "memory_range", "mesh", @@ -2939,6 +2940,7 @@ version = "0.0.0" dependencies = [ "bitfield-struct 0.11.0", "guid", + "ipmi_protocol", "open_enum", "serde", "serde_helpers", @@ -4083,6 +4085,7 @@ dependencies = [ "chipset_device_resources", "chipset_resources", "inspect", + "ipmi_protocol", "local_clock", "mesh", "parking_lot", @@ -4090,6 +4093,15 @@ dependencies = [ "thiserror 2.0.16", "vm_resource", "vmcore", + "zerocopy", +] + +[[package]] +name = "ipmi_protocol" +version = "0.0.0" +dependencies = [ + "static_assertions", + "zerocopy", ] [[package]] diff --git a/Cargo.toml b/Cargo.toml index 5ec7e456a4..3206631073 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -249,6 +249,7 @@ chipset = { path = "vm/devices/chipset" } chipset_legacy = { path = "vm/devices/chipset_legacy" } chipset_resources = { path = "vm/devices/chipset_resources" } ipmi_kcs = { path = "vm/devices/chipset/ipmi_kcs" } +ipmi_protocol = { path = "vm/devices/chipset/ipmi_protocol" } firmware_pcat = { path = "vm/devices/firmware/firmware_pcat" } firmware_uefi = { path = "vm/devices/firmware/firmware_uefi" } firmware_uefi_custom_vars = { path = "vm/devices/firmware/firmware_uefi_custom_vars" } diff --git a/openhcl/openhcl_attestation_protocol/src/igvm_attest/get.rs b/openhcl/openhcl_attestation_protocol/src/igvm_attest/get.rs index 5edc49c4f5..39d646ec1a 100644 --- a/openhcl/openhcl_attestation_protocol/src/igvm_attest/get.rs +++ b/openhcl/openhcl_attestation_protocol/src/igvm_attest/get.rs @@ -468,10 +468,6 @@ pub mod runtime_claims { Signer, } - fn is_false(value: &bool) -> bool { - !value - } - /// TPM reference implementation version. #[derive(Clone, Copy, Debug, Deserialize, Serialize, MeshPayload)] pub enum AttestationTpmVersion { @@ -497,7 +493,7 @@ pub mod runtime_claims { /// Whether the serial console, if enabled, is interactive pub interactive_console_enabled: bool, /// Whether the IPMI KCS interface is enabled - #[serde(default, skip_serializing_if = "is_false")] + #[serde(default)] pub ipmi_enabled: bool, /// Whether secure boot is enabled pub secure_boot: bool, diff --git a/openhcl/underhill_attestation/src/igvm_attest/mod.rs b/openhcl/underhill_attestation/src/igvm_attest/mod.rs index 1ccddac21a..0d99bc82a1 100644 --- a/openhcl/underhill_attestation/src/igvm_attest/mod.rs +++ b/openhcl/underhill_attestation/src/igvm_attest/mod.rs @@ -559,7 +559,7 @@ mod tests { #[test] fn test_vm_configuration_with_time() { - const EXPECTED_JWK: &str = r#"{"current-time":1691103220,"root-cert-thumbprint":"","console-enabled":false,"interactive-console-enabled":false,"secure-boot":false,"tpm-enabled":false,"tpm-version":"185","tpm-persisted":false,"filtered-vpci-devices-allowed":true,"vmUniqueId":"","hardware-sealing-policy":"hash"}"#; + const EXPECTED_JWK: &str = r#"{"current-time":1691103220,"root-cert-thumbprint":"","console-enabled":false,"interactive-console-enabled":false,"ipmi-enabled":false,"secure-boot":false,"tpm-enabled":false,"tpm-version":"185","tpm-persisted":false,"filtered-vpci-devices-allowed":true,"vmUniqueId":"","hardware-sealing-policy":"hash"}"#; let attestation_vm_config = AttestationVmConfig { current_time: None, diff --git a/vm/devices/chipset/ipmi_kcs/Cargo.toml b/vm/devices/chipset/ipmi_kcs/Cargo.toml index 2a4fff2d6a..7ba5000e07 100644 --- a/vm/devices/chipset/ipmi_kcs/Cargo.toml +++ b/vm/devices/chipset/ipmi_kcs/Cargo.toml @@ -11,11 +11,13 @@ chipset_device.workspace = true chipset_device_resources.workspace = true chipset_resources.workspace = true inspect.workspace = true +ipmi_protocol.workspace = true local_clock = { workspace = true, features = ["inspect"] } mesh.workspace = true thiserror.workspace = true vm_resource.workspace = true vmcore.workspace = true +zerocopy.workspace = true async-trait.workspace = true diff --git a/vm/devices/chipset/ipmi_kcs/src/device.rs b/vm/devices/chipset/ipmi_kcs/src/device.rs index 15f5031205..f40388e1c4 100644 --- a/vm/devices/chipset/ipmi_kcs/src/device.rs +++ b/vm/devices/chipset/ipmi_kcs/src/device.rs @@ -191,8 +191,8 @@ mod tests { use crate::KCS_STATE_READ; use crate::STATUS_STATE_MASK; use crate::TrustedClock; - use crate::protocol::COMMAND_GET_DEVICE_ID; - use crate::protocol::NETFN_APPLICATION; + use ipmi_protocol::COMMAND_GET_DEVICE_ID; + use ipmi_protocol::NETFN_APPLICATION; use std::sync::Arc; use std::sync::atomic::AtomicI64; use std::sync::atomic::Ordering; @@ -214,11 +214,11 @@ mod tests { } fn device() -> IpmiKcsDevice { - IpmiKcsDevice::new_pio(IpmiKcs::new(FakeClock::new(100))) + IpmiKcsDevice::new_pio(IpmiKcs::new(Box::new(FakeClock::new(100)))) } fn mmio_device() -> IpmiKcsDevice { - IpmiKcsDevice::new_mmio(IpmiKcs::new(FakeClock::new(100))) + IpmiKcsDevice::new_mmio(IpmiKcs::new(Box::new(FakeClock::new(100)))) } fn read(device: &mut IpmiKcsDevice, port: u16) -> u8 { diff --git a/vm/devices/chipset/ipmi_kcs/src/lib.rs b/vm/devices/chipset/ipmi_kcs/src/lib.rs index 93c1b196ce..3fd8d156d3 100644 --- a/vm/devices/chipset/ipmi_kcs/src/lib.rs +++ b/vm/devices/chipset/ipmi_kcs/src/lib.rs @@ -14,44 +14,31 @@ mod protocol; pub mod resolver; mod save_restore; mod sel; +#[cfg(test)] +mod tests; -pub use chipset_resources::SelEventDisposition; -pub use chipset_resources::SelEventSink; +pub use chipset_resources::ipmi_kcs::SelEventSink; +pub use chipset_resources::ipmi_kcs::SendOutcome; +pub use ipmi_protocol::KCS_COMMAND_GET_STATUS_ABORT; +pub use ipmi_protocol::KCS_COMMAND_WRITE_END; +pub use ipmi_protocol::KCS_COMMAND_WRITE_START; +pub use ipmi_protocol::KCS_DATA_READ_NEXT; +pub use ipmi_protocol::KCS_STATE_ERROR; +pub use ipmi_protocol::KCS_STATE_IDLE; +pub use ipmi_protocol::KCS_STATE_READ; +pub use ipmi_protocol::KCS_STATE_WRITE; +pub use ipmi_protocol::STATUS_CD; +pub use ipmi_protocol::STATUS_IBF; +pub use ipmi_protocol::STATUS_OBF; +pub use ipmi_protocol::STATUS_SMS_ATN; +pub use ipmi_protocol::STATUS_STATE_MASK; +use ipmi_protocol::SelRecord; use sel::RateLimiter; use sel::SelState; /// Maximum KCS request or response size. pub const KCS_MESSAGE_MAX: usize = 64; -/// KCS output-buffer-full status bit. -pub const STATUS_OBF: u8 = 0x01; -/// KCS input-buffer-full status bit. -pub const STATUS_IBF: u8 = 0x02; -/// KCS system-management-software-attention status bit. -pub const STATUS_SMS_ATN: u8 = 0x04; -/// KCS command/data status bit. -pub const STATUS_CD: u8 = 0x08; -/// KCS state field mask. -pub const STATUS_STATE_MASK: u8 = 0xc0; - -/// KCS idle state. -pub const KCS_STATE_IDLE: u8 = 0x00; -/// KCS read state. -pub const KCS_STATE_READ: u8 = 0x40; -/// KCS write state. -pub const KCS_STATE_WRITE: u8 = 0x80; -/// KCS error state. -pub const KCS_STATE_ERROR: u8 = 0xc0; - -/// KCS Get Status/Abort command. -pub const KCS_COMMAND_GET_STATUS_ABORT: u8 = 0x60; -/// KCS Write Start command. -pub const KCS_COMMAND_WRITE_START: u8 = 0x61; -/// KCS Write End command. -pub const KCS_COMMAND_WRITE_END: u8 = 0x62; -/// KCS Read Next control byte. -pub const KCS_DATA_READ_NEXT: u8 = 0x68; - /// Trusted wall-clock source used for SEL timestamps and rate limiting. /// /// Implementations must return UTC seconds since the Unix epoch. Negative @@ -140,22 +127,16 @@ pub struct IpmiKcs { impl IpmiKcs { /// Creates a virtual BMC without a SEL event sink. - pub fn new(clock: impl TrustedClock + 'static) -> Self { - Self::from_boxed_parts(Box::new(clock), None) + pub fn new(clock: Box) -> Self { + Self::from_parts(clock, None) } /// Creates a virtual BMC with a best-effort SEL event sink. - pub fn with_event_sink( - clock: impl TrustedClock + 'static, - sink: impl SelEventSink + 'static, - ) -> Self { - Self::from_boxed_parts(Box::new(clock), Some(Box::new(sink))) + pub fn with_event_sink(clock: Box, sink: Box) -> Self { + Self::from_parts(clock, Some(sink)) } - pub(crate) fn from_boxed_parts( - clock: Box, - sink: Option>, - ) -> Self { + fn from_parts(clock: Box, sink: Option>) -> Self { Self { transaction: KcsTransaction::default(), sel: SelState::new(), @@ -235,7 +216,7 @@ impl IpmiKcs { } /// Returns a stored SEL record by insertion index. - pub fn sel_record(&self, index: usize) -> Option<&[u8; 16]> { + pub fn sel_record(&self, index: usize) -> Option<&SelRecord> { self.sel.records.get(index) } @@ -286,6 +267,3 @@ impl IpmiKcs { self.transaction.set_state(KCS_STATE_ERROR); } } - -#[cfg(test)] -mod tests; diff --git a/vm/devices/chipset/ipmi_kcs/src/protocol.rs b/vm/devices/chipset/ipmi_kcs/src/protocol.rs index 632249121c..3c25c35473 100644 --- a/vm/devices/chipset/ipmi_kcs/src/protocol.rs +++ b/vm/devices/chipset/ipmi_kcs/src/protocol.rs @@ -1,73 +1,65 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. +//! IPMI request dispatch and KCS response staging. + use crate::IpmiKcs; use crate::KCS_MESSAGE_MAX; use crate::KCS_STATE_READ; use crate::STATUS_OBF; - -pub(crate) const NETFN_APPLICATION: u8 = 0x06; -pub(crate) const NETFN_STORAGE: u8 = 0x0a; - -pub(crate) const COMMAND_GET_DEVICE_ID: u8 = 0x01; -pub(crate) const COMMAND_GET_SEL_INFO: u8 = 0x40; -pub(crate) const COMMAND_RESERVE_SEL: u8 = 0x42; -pub(crate) const COMMAND_GET_SEL_ENTRY: u8 = 0x43; -pub(crate) const COMMAND_ADD_SEL_ENTRY: u8 = 0x44; -pub(crate) const COMMAND_CLEAR_SEL: u8 = 0x47; -pub(crate) const COMMAND_GET_SEL_TIME: u8 = 0x48; -pub(crate) const COMMAND_SET_SEL_TIME: u8 = 0x49; - -pub(crate) const COMPLETION_SUCCESS: u8 = 0x00; -pub(crate) const COMPLETION_INVALID_COMMAND: u8 = 0xc1; -pub(crate) const COMPLETION_SEL_FULL: u8 = 0xc4; -#[expect(dead_code)] -pub(crate) const COMPLETION_RESERVATION_CANCELED: u8 = 0xc5; -pub(crate) const COMPLETION_INVALID_REQUEST_LENGTH: u8 = 0xc7; -pub(crate) const COMPLETION_PARAMETER_OUT_OF_RANGE: u8 = 0xc9; -pub(crate) const COMPLETION_RECORD_NOT_PRESENT: u8 = 0xcb; -pub(crate) const COMPLETION_INVALID_DATA_FIELD: u8 = 0xcc; +use core::mem::size_of; +use ipmi_protocol::COMMAND_GET_DEVICE_ID; +use ipmi_protocol::COMPLETION_INVALID_COMMAND; +use ipmi_protocol::COMPLETION_INVALID_REQUEST_LENGTH; +use ipmi_protocol::CompletionResponse; +use ipmi_protocol::GetDeviceIdResponse; +use ipmi_protocol::MessageHeader; +use ipmi_protocol::NETFN_APPLICATION; +use ipmi_protocol::NETFN_STORAGE; +use zerocopy::FromBytes; +use zerocopy::Immutable; +use zerocopy::IntoBytes; impl IpmiKcs { + /// Dispatches the accumulated IPMI request and stages its KCS response. pub(crate) fn process_ipmi_message(&mut self) { - if self.transaction.request_len < 2 { + let request = self.transaction.request; + let request = &request[..self.transaction.request_len]; + let Ok((header, data)) = MessageHeader::read_from_prefix(request) else { self.enter_error_state(); return; - } - - let request = self.transaction.request; - let request_len = self.transaction.request_len; - let netfn_lun = request[0]; - let command = request[1]; - let data = &request[2..request_len]; + }; let mut body = [0; KCS_MESSAGE_MAX]; - let body_len = match netfn_lun >> 2 { - NETFN_APPLICATION => self.handle_application_command(command, data, &mut body), - NETFN_STORAGE => self.handle_sel_command(command, data, &mut body), + let body_len = match header.netfn() { + NETFN_APPLICATION => self.handle_application_command(header.command, data, &mut body), + NETFN_STORAGE => self.handle_sel_command(header.command, data, &mut body), _ => { body[0] = COMPLETION_INVALID_COMMAND; 1 } }; - self.stage_response(netfn_lun, command, &body[..body_len]); + self.stage_response(header.response(), &body[..body_len]); } - fn stage_response(&mut self, request_netfn_lun: u8, command: u8, body: &[u8]) { + /// Builds an IPMI response and exposes its first byte through the KCS data register. + fn stage_response(&mut self, header: MessageHeader, body: &[u8]) { self.transaction.response.fill(0); - self.transaction.response[0] = request_netfn_lun | 0x04; - self.transaction.response[1] = command; + self.transaction.response[..size_of::()].copy_from_slice(header.as_bytes()); - let body_len = body.len().min(KCS_MESSAGE_MAX - 2); - self.transaction.response[2..2 + body_len].copy_from_slice(&body[..body_len]); - self.transaction.response_len = body_len + 2; + let header_len = size_of::(); + let body_len = body.len().min(KCS_MESSAGE_MAX - header_len); + self.transaction.response[header_len..header_len + body_len] + .copy_from_slice(&body[..body_len]); + self.transaction.response_len = body_len + header_len; self.transaction.response_pos = 1; self.transaction.data_out = self.transaction.response[0]; self.transaction.status |= STATUS_OBF; self.transaction.set_state(KCS_STATE_READ); } + /// Handles commands in the IPMI application network function. fn handle_application_command( &mut self, command: u8, @@ -75,34 +67,28 @@ impl IpmiKcs { out: &mut [u8; KCS_MESSAGE_MAX], ) -> usize { match command { - COMMAND_GET_DEVICE_ID => { - const RESPONSE: [u8; 12] = [ - COMPLETION_SUCCESS, - 0x20, - 0x01, - 0x02, - 0x00, - 0x02, - 0x04, - 0x00, - 0x00, - 0x00, - 0x00, - 0x00, - ]; - out[..RESPONSE.len()].copy_from_slice(&RESPONSE); - RESPONSE.len() - } + COMMAND_GET_DEVICE_ID => write_response(out, &GetDeviceIdResponse::new()), _ => completion(out, COMPLETION_INVALID_COMMAND), } } } +/// Writes a fixed-format response body into the KCS output buffer. +pub(crate) fn write_response( + out: &mut [u8; KCS_MESSAGE_MAX], + response: &T, +) -> usize { + let bytes = response.as_bytes(); + out[..bytes.len()].copy_from_slice(bytes); + bytes.len() +} + +/// Writes a response containing only an IPMI completion code. pub(crate) fn completion(out: &mut [u8; KCS_MESSAGE_MAX], code: u8) -> usize { - out[0] = code; - 1 + write_response(out, &CompletionResponse::new(code)) } +/// Writes the standard invalid-request-length response. pub(crate) fn invalid_length(out: &mut [u8; KCS_MESSAGE_MAX]) -> usize { completion(out, COMPLETION_INVALID_REQUEST_LENGTH) } diff --git a/vm/devices/chipset/ipmi_kcs/src/resolver.rs b/vm/devices/chipset/ipmi_kcs/src/resolver.rs index a2ed230883..e9ccb306f6 100644 --- a/vm/devices/chipset/ipmi_kcs/src/resolver.rs +++ b/vm/devices/chipset/ipmi_kcs/src/resolver.rs @@ -10,11 +10,11 @@ use async_trait::async_trait; use chipset_device_resources::ResolveChipsetDeviceHandleParams; use chipset_device_resources::ResolvedChipsetDevice; use chipset_resources::CmosRtcTimeSourceHandleKind; -use chipset_resources::IpmiSelEventSinkHandleKind; use chipset_resources::ResolvedCmosRtcTimeSource; -use chipset_resources::ResolvedIpmiSelEventSink; use chipset_resources::ipmi_kcs::IpmiKcsDeviceHandleAArch64; use chipset_resources::ipmi_kcs::IpmiKcsDeviceHandleX64; +use chipset_resources::ipmi_kcs::IpmiSelEventSinkHandleKind; +use chipset_resources::ipmi_kcs::ResolvedIpmiSelEventSink; use local_clock::InspectableLocalClock; use thiserror::Error; use vm_resource::AsyncResolveResource; @@ -69,9 +69,9 @@ async fn resolve_core( .await .map_err(ResolveIpmiKcsError::EventSink)?; - Ok(IpmiKcs::from_boxed_parts( + Ok(IpmiKcs::with_event_sink( Box::new(TrustedClockAdapter(time_source)), - Some(event_sink), + event_sink, )) } diff --git a/vm/devices/chipset/ipmi_kcs/src/save_restore.rs b/vm/devices/chipset/ipmi_kcs/src/save_restore.rs index 08eadceb36..952e31f361 100644 --- a/vm/devices/chipset/ipmi_kcs/src/save_restore.rs +++ b/vm/devices/chipset/ipmi_kcs/src/save_restore.rs @@ -1,6 +1,8 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. +//! Save and restore support for the KCS transaction and SEL state. + use crate::IpmiKcs; use crate::KCS_MESSAGE_MAX; use crate::KCS_STATE_ERROR; @@ -12,8 +14,8 @@ use crate::STATUS_IBF; use crate::STATUS_STATE_MASK; use crate::SelStats; use crate::sel::SEL_CAPACITY; -use crate::sel::SEL_RECORD_SIZE; use crate::sel::SelState; +use ipmi_protocol::SEL_RECORD_SIZE; use mesh::payload::Protobuf; use vmcore::save_restore::RestoreError; use vmcore::save_restore::SaveError; diff --git a/vm/devices/chipset/ipmi_kcs/src/sel.rs b/vm/devices/chipset/ipmi_kcs/src/sel.rs index bffd624b0a..1dc70632f6 100644 --- a/vm/devices/chipset/ipmi_kcs/src/sel.rs +++ b/vm/devices/chipset/ipmi_kcs/src/sel.rs @@ -1,33 +1,55 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. +//! System Event Log command handling and emulator state. + use crate::IpmiKcs; use crate::KCS_MESSAGE_MAX; -use crate::SelEventDisposition; -use crate::protocol::COMMAND_ADD_SEL_ENTRY; -use crate::protocol::COMMAND_CLEAR_SEL; -use crate::protocol::COMMAND_GET_SEL_ENTRY; -use crate::protocol::COMMAND_GET_SEL_INFO; -use crate::protocol::COMMAND_GET_SEL_TIME; -use crate::protocol::COMMAND_RESERVE_SEL; -use crate::protocol::COMMAND_SET_SEL_TIME; -use crate::protocol::COMPLETION_INVALID_COMMAND; -use crate::protocol::COMPLETION_INVALID_DATA_FIELD; -use crate::protocol::COMPLETION_PARAMETER_OUT_OF_RANGE; -use crate::protocol::COMPLETION_RECORD_NOT_PRESENT; -use crate::protocol::COMPLETION_SEL_FULL; -use crate::protocol::COMPLETION_SUCCESS; +use crate::SendOutcome; use crate::protocol::completion; use crate::protocol::invalid_length; +use crate::protocol::write_response; +use ipmi_protocol::AddSelEntryRequest; +use ipmi_protocol::AddSelEntryResponse; +use ipmi_protocol::CLEAR_SEL_ERASE_COMPLETE; +use ipmi_protocol::CLEAR_SEL_GET_STATUS; +use ipmi_protocol::CLEAR_SEL_INITIATE_ERASE; +use ipmi_protocol::CLEAR_SEL_SIGNATURE; +use ipmi_protocol::COMMAND_ADD_SEL_ENTRY; +use ipmi_protocol::COMMAND_CLEAR_SEL; +use ipmi_protocol::COMMAND_GET_SEL_ENTRY; +use ipmi_protocol::COMMAND_GET_SEL_INFO; +use ipmi_protocol::COMMAND_GET_SEL_TIME; +use ipmi_protocol::COMMAND_RESERVE_SEL; +use ipmi_protocol::COMMAND_SET_SEL_TIME; +use ipmi_protocol::COMPLETION_INVALID_COMMAND; +use ipmi_protocol::COMPLETION_INVALID_DATA_FIELD; +use ipmi_protocol::COMPLETION_PARAMETER_OUT_OF_RANGE; +use ipmi_protocol::COMPLETION_RECORD_NOT_PRESENT; +use ipmi_protocol::COMPLETION_SEL_FULL; +use ipmi_protocol::COMPLETION_SUCCESS; +use ipmi_protocol::ClearSelRequest; +use ipmi_protocol::ClearSelResponse; +use ipmi_protocol::GetSelEntryRequest; +use ipmi_protocol::GetSelEntryResponseHeader; +use ipmi_protocol::GetSelInfoResponse; +use ipmi_protocol::GetSelTimeResponse; +use ipmi_protocol::ReserveSelResponse; +use ipmi_protocol::SEL_OPERATION_SUPPORT_RESERVE; +use ipmi_protocol::SEL_RECORD_SIZE; +use ipmi_protocol::SEL_VERSION; +use ipmi_protocol::SelRecord; +use ipmi_protocol::SetSelTimeRequest; +use zerocopy::FromBytes; +use zerocopy::U16; +use zerocopy::U32; -pub(crate) const SEL_RECORD_SIZE: usize = 16; pub(crate) const SEL_CAPACITY: usize = 128; -const SEL_VERSION: u8 = 0x51; const SEL_FORWARD_LIMIT: u32 = 256; #[derive(Default)] pub(crate) struct SelState { - pub(crate) records: Vec<[u8; SEL_RECORD_SIZE]>, + pub(crate) records: Vec, pub(crate) next_record_id: u16, pub(crate) reservation_id: u16, pub(crate) time_offset_seconds: i64, @@ -73,6 +95,7 @@ impl RateLimiter { } impl IpmiKcs { + /// Dispatches an IPMI storage command implemented by the SEL. pub(crate) fn handle_sel_command( &mut self, command: u8, @@ -100,17 +123,18 @@ impl IpmiKcs { .last() .map(|record| u32::from_le_bytes([record[3], record[4], record[5], record[6]])) .unwrap_or(0); - let mut pos = 0; - out[pos] = COMPLETION_SUCCESS; - pos += 1; - out[pos] = SEL_VERSION; - pos += 1; - put_u16(out, &mut pos, count); - put_u16(out, &mut pos, free_bytes); - put_u32(out, &mut pos, last_addition_timestamp); - put_u32(out, &mut pos, self.sel.last_erase_timestamp); - out[pos] = 0x02; - pos + 1 + write_response( + out, + &GetSelInfoResponse { + completion_code: COMPLETION_SUCCESS, + sel_version: SEL_VERSION, + entry_count: U16::new(count), + free_space: U16::new(free_bytes), + last_addition_timestamp: U32::new(last_addition_timestamp), + last_erase_timestamp: U32::new(self.sel.last_erase_timestamp), + operation_support: SEL_OPERATION_SUPPORT_RESERVE, + }, + ) } fn reserve_sel(&mut self, out: &mut [u8; KCS_MESSAGE_MAX]) -> usize { @@ -119,22 +143,26 @@ impl IpmiKcs { self.sel.reservation_id = 1; } - out[0] = COMPLETION_SUCCESS; - out[1..3].copy_from_slice(&self.sel.reservation_id.to_le_bytes()); - 3 + write_response( + out, + &ReserveSelResponse { + completion_code: COMPLETION_SUCCESS, + reservation_id: U16::new(self.sel.reservation_id), + }, + ) } fn get_sel_entry(&mut self, data: &[u8], out: &mut [u8; KCS_MESSAGE_MAX]) -> usize { - let Some(data) = data.get(..6) else { + let Ok((request, _)) = GetSelEntryRequest::read_from_prefix(data) else { return invalid_length(out); }; - let offset = usize::from(data[4]); + let offset = usize::from(request.offset); if offset >= SEL_RECORD_SIZE { return completion(out, COMPLETION_PARAMETER_OUT_OF_RANGE); } - let record_id = u16::from_le_bytes([data[2], data[3]]); + let record_id = request.record_id.get(); let Some(index) = self.find_record(record_id) else { return completion(out, COMPLETION_RECORD_NOT_PRESENT); }; @@ -146,23 +174,27 @@ impl IpmiKcs { .map(|record| u16::from_le_bytes([record[0], record[1]])) .unwrap_or(0xffff); let end = offset - .saturating_add(usize::from(data[5])) + .saturating_add(usize::from(request.bytes_to_read)) .min(SEL_RECORD_SIZE); let record = &self.sel.records[index]; let bytes = &record[offset..end]; - out[0] = COMPLETION_SUCCESS; - out[1..3].copy_from_slice(&next_record_id.to_le_bytes()); - out[3..3 + bytes.len()].copy_from_slice(bytes); - 3 + bytes.len() + let header_len = write_response( + out, + &GetSelEntryResponseHeader { + completion_code: COMPLETION_SUCCESS, + next_record_id: U16::new(next_record_id), + }, + ); + out[header_len..header_len + bytes.len()].copy_from_slice(bytes); + header_len + bytes.len() } fn add_sel_entry(&mut self, data: &[u8], out: &mut [u8; KCS_MESSAGE_MAX]) -> usize { - let Some(data) = data.get(..SEL_RECORD_SIZE) else { + let Ok((request, _)) = AddSelEntryRequest::read_from_prefix(data) else { return invalid_length(out); }; - let mut record = [0; SEL_RECORD_SIZE]; - record.copy_from_slice(data); + let mut record = request.record; if self.sel.records.len() >= SEL_CAPACITY { return completion(out, COMPLETION_SEL_FULL); @@ -182,10 +214,10 @@ impl IpmiKcs { if let Some(sink) = self.sink.as_mut() { if self.rate_limiter.allow(trusted_seconds) { match sink.try_send(record_id, record) { - SelEventDisposition::Accepted => { + SendOutcome::Accepted => { self.stats.forwarded = self.stats.forwarded.saturating_add(1); } - SelEventDisposition::Dropped => { + SendOutcome::Dropped => { self.stats.sink_dropped = self.stats.sink_dropped.saturating_add(1); } } @@ -194,52 +226,65 @@ impl IpmiKcs { } } - out[0] = COMPLETION_SUCCESS; - out[1..3].copy_from_slice(&record_id.to_le_bytes()); - 3 + write_response( + out, + &AddSelEntryResponse { + completion_code: COMPLETION_SUCCESS, + record_id: U16::new(record_id), + }, + ) } fn clear_sel(&mut self, data: &[u8], out: &mut [u8; KCS_MESSAGE_MAX]) -> usize { - let Some(data) = data.get(..6) else { + let Ok((request, _)) = ClearSelRequest::read_from_prefix(data) else { return invalid_length(out); }; - if data[2..5] != *b"CLR" { + if request.signature != CLEAR_SEL_SIGNATURE { return completion(out, COMPLETION_INVALID_DATA_FIELD); } - match data[5] { - 0xaa => { + match request.operation { + CLEAR_SEL_INITIATE_ERASE => { self.sel.records.clear(); self.sel.next_record_id = 1; let trusted_seconds = self.clock.unix_seconds(); self.sel.last_erase_timestamp = adjusted_timestamp(trusted_seconds, self.sel.time_offset_seconds); } - 0x00 => {} + CLEAR_SEL_GET_STATUS => {} _ => return completion(out, COMPLETION_INVALID_DATA_FIELD), } - out[0] = COMPLETION_SUCCESS; - out[1] = 1; - 2 + write_response( + out, + &ClearSelResponse { + completion_code: COMPLETION_SUCCESS, + erase_status: CLEAR_SEL_ERASE_COMPLETE, + }, + ) } fn get_sel_time(&mut self, out: &mut [u8; KCS_MESSAGE_MAX]) -> usize { let trusted_seconds = self.clock.unix_seconds(); - out[0] = COMPLETION_SUCCESS; - out[1..5].copy_from_slice( - &adjusted_timestamp(trusted_seconds, self.sel.time_offset_seconds).to_le_bytes(), - ); - 5 + write_response( + out, + &GetSelTimeResponse { + completion_code: COMPLETION_SUCCESS, + timestamp: U32::new(adjusted_timestamp( + trusted_seconds, + self.sel.time_offset_seconds, + )), + }, + ) } fn set_sel_time(&mut self, data: &[u8], out: &mut [u8; KCS_MESSAGE_MAX]) -> usize { - let Some(data) = data.get(..4) else { + let Ok((request, _)) = SetSelTimeRequest::read_from_prefix(data) else { return invalid_length(out); }; - let requested = i64::from(u32::from_le_bytes([data[0], data[1], data[2], data[3]])); + let requested = i64::from(request.timestamp.get()); self.sel.time_offset_seconds = requested.saturating_sub(self.clock.unix_seconds()); completion(out, COMPLETION_SUCCESS) } @@ -290,13 +335,3 @@ fn adjusted_timestamp(trusted_seconds: i64, offset_seconds: i64) -> u32 { let adjusted = trusted_seconds.saturating_add(offset_seconds); if adjusted < 0 { 0 } else { adjusted as u32 } } - -fn put_u16(out: &mut [u8; KCS_MESSAGE_MAX], pos: &mut usize, value: u16) { - out[*pos..*pos + 2].copy_from_slice(&value.to_le_bytes()); - *pos += 2; -} - -fn put_u32(out: &mut [u8; KCS_MESSAGE_MAX], pos: &mut usize, value: u32) { - out[*pos..*pos + 4].copy_from_slice(&value.to_le_bytes()); - *pos += 4; -} diff --git a/vm/devices/chipset/ipmi_kcs/src/tests.rs b/vm/devices/chipset/ipmi_kcs/src/tests.rs index 75532e627c..ea05f50a5b 100644 --- a/vm/devices/chipset/ipmi_kcs/src/tests.rs +++ b/vm/devices/chipset/ipmi_kcs/src/tests.rs @@ -1,8 +1,10 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. +//! Unit tests for KCS protocol, SEL, and save/restore behavior. + use super::*; -use crate::protocol::*; +use ipmi_protocol::*; use parking_lot::Mutex; use std::sync::Arc; use std::sync::atomic::AtomicBool; @@ -46,18 +48,18 @@ struct SharedSink { } impl SelEventSink for SharedSink { - fn try_send(&mut self, record_id: u16, record: [u8; 16]) -> SelEventDisposition { + fn try_send(&mut self, record_id: u16, record: SelRecord) -> SendOutcome { if !self.accept.load(Ordering::Relaxed) { - return SelEventDisposition::Dropped; + return SendOutcome::Dropped; } self.state.lock().records.push((record_id, record)); - SelEventDisposition::Accepted + SendOutcome::Accepted } } fn device(seconds: i64) -> (FakeClock, IpmiKcs) { let clock = FakeClock::new(seconds); - (clock.clone(), IpmiKcs::new(clock)) + (clock.clone(), IpmiKcs::new(Box::new(clock))) } fn storage_request(command: u8, data: &[u8]) -> Vec { @@ -520,7 +522,7 @@ fn record_and_reservation_ids_roll_over_without_reserved_values() { #[test] fn clear_sel_validates_fields_and_resets_store() { let clock = FakeClock::new(100); - let mut device = IpmiKcs::new(clock.clone()); + let mut device = IpmiKcs::new(Box::new(clock.clone())); add_record(&mut device, 0x33); let bad_signature = transact( @@ -575,7 +577,7 @@ fn clear_sel_validates_fields_and_resets_store() { #[test] fn sel_time_supports_positive_negative_and_wrapping_offsets() { let clock = FakeClock::new(1000); - let mut device = IpmiKcs::new(clock.clone()); + let mut device = IpmiKcs::new(Box::new(clock.clone())); let time = transact(&mut device, &storage_request(COMMAND_GET_SEL_TIME, &[])); assert_eq!(u32::from_le_bytes(time[3..7].try_into().unwrap()), 1000); @@ -624,7 +626,7 @@ fn sink_results_do_not_change_committed_records() { state: sink_state.clone(), accept: accept.clone(), }; - let mut device = IpmiKcs::with_event_sink(clock, sink); + let mut device = IpmiKcs::with_event_sink(Box::new(clock), Box::new(sink)); add_record(&mut device, 0x11); accept.store(false, Ordering::Relaxed); @@ -654,7 +656,7 @@ fn sink_forwarding_is_limited_to_256_per_trusted_second() { state: sink_state.clone(), accept: Arc::new(AtomicBool::new(true)), }; - let mut device = IpmiKcs::with_event_sink(clock.clone(), sink); + let mut device = IpmiKcs::with_event_sink(Box::new(clock.clone()), Box::new(sink)); for _ in 0..256 { assert_completion( @@ -695,7 +697,7 @@ fn sink_forwarding_is_limited_to_256_per_trusted_second() { #[test] fn reset_preserves_sel_time_reservation_and_stats() { let clock = FakeClock::new(100); - let mut device = IpmiKcs::new(clock); + let mut device = IpmiKcs::new(Box::new(clock)); transact( &mut device, &storage_request(COMMAND_SET_SEL_TIME, &200u32.to_le_bytes()), @@ -770,7 +772,7 @@ fn save_restore_idle_write_and_read_transactions() { #[test] fn save_restore_full_sel_and_adjusted_time() { let clock = FakeClock::new(1000); - let mut source = IpmiKcs::new(clock.clone()); + let mut source = IpmiKcs::new(Box::new(clock.clone())); transact( &mut source, &storage_request(COMMAND_SET_SEL_TIME, &1500u32.to_le_bytes()), diff --git a/vm/devices/chipset/ipmi_protocol/Cargo.toml b/vm/devices/chipset/ipmi_protocol/Cargo.toml new file mode 100644 index 0000000000..7e6f333df0 --- /dev/null +++ b/vm/devices/chipset/ipmi_protocol/Cargo.toml @@ -0,0 +1,14 @@ +# Copyright (c) Microsoft Corporation. +# Licensed under the MIT License. + +[package] +name = "ipmi_protocol" +edition.workspace = true +rust-version.workspace = true + +[dependencies] +static_assertions.workspace = true +zerocopy.workspace = true + +[lints] +workspace = true diff --git a/vm/devices/chipset/ipmi_protocol/src/lib.rs b/vm/devices/chipset/ipmi_protocol/src/lib.rs new file mode 100644 index 0000000000..2d872bb71d --- /dev/null +++ b/vm/devices/chipset/ipmi_protocol/src/lib.rs @@ -0,0 +1,486 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +//! Wire-level definitions for the IPMI KCS interface and System Event Log. + +#![forbid(unsafe_code)] + +use core::mem::offset_of; +use core::mem::size_of; +use static_assertions::const_assert_eq; +use zerocopy::FromBytes; +use zerocopy::Immutable; +use zerocopy::IntoBytes; +use zerocopy::KnownLayout; +use zerocopy::LittleEndian; +use zerocopy::U16; +use zerocopy::U32; +use zerocopy::Unaligned; + +/// Size of an IPMI System Event Log record. +pub const SEL_RECORD_SIZE: usize = 16; + +/// IPMI SEL version 1.5. +pub const SEL_VERSION: u8 = 0x51; + +/// The SEL implementation supports reservation commands. +pub const SEL_OPERATION_SUPPORT_RESERVE: u8 = 0x02; + +/// Signature required by the Clear SEL command. +pub const CLEAR_SEL_SIGNATURE: [u8; 3] = *b"CLR"; + +/// Clear SEL operation that queries erase progress. +pub const CLEAR_SEL_GET_STATUS: u8 = 0x00; + +/// Clear SEL operation that initiates an erase. +pub const CLEAR_SEL_INITIATE_ERASE: u8 = 0xaa; + +/// Clear SEL status indicating that the erase is complete. +pub const CLEAR_SEL_ERASE_COMPLETE: u8 = 0x01; + +/// KCS output-buffer-full status bit. +pub const STATUS_OBF: u8 = 0x01; + +/// KCS input-buffer-full status bit. +pub const STATUS_IBF: u8 = 0x02; + +/// KCS system-management-software-attention status bit. +pub const STATUS_SMS_ATN: u8 = 0x04; + +/// KCS command/data status bit. +pub const STATUS_CD: u8 = 0x08; + +/// KCS state field mask. +pub const STATUS_STATE_MASK: u8 = 0xc0; + +/// KCS idle state. +pub const KCS_STATE_IDLE: u8 = 0x00; + +/// KCS read state. +pub const KCS_STATE_READ: u8 = 0x40; + +/// KCS write state. +pub const KCS_STATE_WRITE: u8 = 0x80; + +/// KCS error state. +pub const KCS_STATE_ERROR: u8 = 0xc0; + +/// KCS Get Status/Abort command. +pub const KCS_COMMAND_GET_STATUS_ABORT: u8 = 0x60; + +/// KCS Write Start command. +pub const KCS_COMMAND_WRITE_START: u8 = 0x61; + +/// KCS Write End command. +pub const KCS_COMMAND_WRITE_END: u8 = 0x62; + +/// KCS Read Next control byte. +pub const KCS_DATA_READ_NEXT: u8 = 0x68; + +/// IPMI application network function. +pub const NETFN_APPLICATION: u8 = 0x06; + +/// IPMI storage network function. +pub const NETFN_STORAGE: u8 = 0x0a; + +/// Bit added to a request network function to form its response network function. +pub const NETFN_RESPONSE: u8 = 0x01; + +/// Get Device ID command. +pub const COMMAND_GET_DEVICE_ID: u8 = 0x01; + +/// Get SEL Info command. +pub const COMMAND_GET_SEL_INFO: u8 = 0x40; + +/// Reserve SEL command. +pub const COMMAND_RESERVE_SEL: u8 = 0x42; + +/// Get SEL Entry command. +pub const COMMAND_GET_SEL_ENTRY: u8 = 0x43; + +/// Add SEL Entry command. +pub const COMMAND_ADD_SEL_ENTRY: u8 = 0x44; + +/// Clear SEL command. +pub const COMMAND_CLEAR_SEL: u8 = 0x47; + +/// Get SEL Time command. +pub const COMMAND_GET_SEL_TIME: u8 = 0x48; + +/// Set SEL Time command. +pub const COMMAND_SET_SEL_TIME: u8 = 0x49; + +/// Successful IPMI completion code. +pub const COMPLETION_SUCCESS: u8 = 0x00; + +/// Invalid or unsupported command completion code. +pub const COMPLETION_INVALID_COMMAND: u8 = 0xc1; + +/// SEL full completion code. +pub const COMPLETION_SEL_FULL: u8 = 0xc4; + +/// Reservation canceled or invalid completion code. +pub const COMPLETION_RESERVATION_CANCELED: u8 = 0xc5; + +/// Invalid request length completion code. +pub const COMPLETION_INVALID_REQUEST_LENGTH: u8 = 0xc7; + +/// Parameter out of range completion code. +pub const COMPLETION_PARAMETER_OUT_OF_RANGE: u8 = 0xc9; + +/// Requested record not present completion code. +pub const COMPLETION_RECORD_NOT_PRESENT: u8 = 0xcb; + +/// Invalid data field completion code. +pub const COMPLETION_INVALID_DATA_FIELD: u8 = 0xcc; + +/// A completed IPMI SEL record. +pub type SelRecord = [u8; SEL_RECORD_SIZE]; + +/// Common header for an IPMI request or response carried over KCS. +#[repr(C)] +#[derive(Copy, Clone, Debug, IntoBytes, FromBytes, Immutable, KnownLayout, Unaligned)] +pub struct MessageHeader { + /// Network function in bits 7:2 and logical unit number in bits 1:0. + pub netfn_lun: u8, + /// Command identifier. + pub command: u8, +} + +impl MessageHeader { + /// Returns the six-bit network function. + pub const fn netfn(&self) -> u8 { + self.netfn_lun >> 2 + } + + /// Creates the response header corresponding to this request. + pub const fn response(self) -> Self { + Self { + netfn_lun: self.netfn_lun | (NETFN_RESPONSE << 2), + command: self.command, + } + } +} + +/// A response containing only an IPMI completion code. +#[repr(C)] +#[derive(Copy, Clone, Debug, IntoBytes, FromBytes, Immutable, KnownLayout, Unaligned)] +pub struct CompletionResponse { + /// IPMI completion code. + pub completion_code: u8, +} + +impl CompletionResponse { + /// Creates a completion-only response. + pub const fn new(completion_code: u8) -> Self { + Self { completion_code } + } +} + +/// Get Device ID response body. +#[repr(C, packed)] +#[derive(Copy, Clone, Debug, IntoBytes, FromBytes, Immutable, KnownLayout, Unaligned)] +pub struct GetDeviceIdResponse { + /// IPMI completion code. + pub completion_code: u8, + /// Device identifier. + pub device_id: u8, + /// Device revision. + pub device_revision: u8, + /// Firmware revision byte 1. + pub firmware_revision_1: u8, + /// Firmware revision byte 2. + pub firmware_revision_2: u8, + /// Supported IPMI version. + pub ipmi_version: u8, + /// Additional device support bitmap. + pub additional_device_support: u8, + /// IANA manufacturer identifier. + pub manufacturer_id: [u8; 3], + /// Product identifier. + pub product_id: U16, +} + +impl GetDeviceIdResponse { + /// Creates the identity returned by the virtual BMC. + pub const fn new() -> Self { + Self { + completion_code: COMPLETION_SUCCESS, + device_id: 0x20, + device_revision: 0x01, + firmware_revision_1: 0x02, + firmware_revision_2: 0x00, + ipmi_version: 0x02, + additional_device_support: 0x04, + manufacturer_id: [0; 3], + product_id: U16::ZERO, + } + } +} + +impl Default for GetDeviceIdResponse { + fn default() -> Self { + Self::new() + } +} + +/// Get SEL Info response body. +#[repr(C, packed)] +#[derive(Copy, Clone, Debug, IntoBytes, FromBytes, Immutable, KnownLayout, Unaligned)] +pub struct GetSelInfoResponse { + /// IPMI completion code. + pub completion_code: u8, + /// SEL format version. + pub sel_version: u8, + /// Number of SEL entries. + pub entry_count: U16, + /// Remaining SEL storage in bytes. + pub free_space: U16, + /// Timestamp of the most recently added entry. + pub last_addition_timestamp: U32, + /// Timestamp of the most recent erase. + pub last_erase_timestamp: U32, + /// Supported SEL operations bitmap. + pub operation_support: u8, +} + +/// Reserve SEL response body. +#[repr(C, packed)] +#[derive(Copy, Clone, Debug, IntoBytes, FromBytes, Immutable, KnownLayout, Unaligned)] +pub struct ReserveSelResponse { + /// IPMI completion code. + pub completion_code: u8, + /// New reservation identifier. + pub reservation_id: U16, +} + +/// Get SEL Entry request data. +#[repr(C, packed)] +#[derive(Copy, Clone, Debug, IntoBytes, FromBytes, Immutable, KnownLayout, Unaligned)] +pub struct GetSelEntryRequest { + /// Reservation identifier, or zero when no reservation is used. + pub reservation_id: U16, + /// Requested record identifier. + pub record_id: U16, + /// Byte offset into the SEL record. + pub offset: u8, + /// Maximum number of record bytes to return. + pub bytes_to_read: u8, +} + +/// Fixed header of a Get SEL Entry response body. +#[repr(C, packed)] +#[derive(Copy, Clone, Debug, IntoBytes, FromBytes, Immutable, KnownLayout, Unaligned)] +pub struct GetSelEntryResponseHeader { + /// IPMI completion code. + pub completion_code: u8, + /// Identifier of the next SEL record. + pub next_record_id: U16, +} + +/// Add SEL Entry request data. +#[repr(C)] +#[derive(Copy, Clone, Debug, IntoBytes, FromBytes, Immutable, KnownLayout, Unaligned)] +pub struct AddSelEntryRequest { + /// Record supplied by the management software. + pub record: SelRecord, +} + +/// Add SEL Entry response body. +#[repr(C, packed)] +#[derive(Copy, Clone, Debug, IntoBytes, FromBytes, Immutable, KnownLayout, Unaligned)] +pub struct AddSelEntryResponse { + /// IPMI completion code. + pub completion_code: u8, + /// Identifier assigned to the new record. + pub record_id: U16, +} + +/// Clear SEL request data. +#[repr(C, packed)] +#[derive(Copy, Clone, Debug, IntoBytes, FromBytes, Immutable, KnownLayout, Unaligned)] +pub struct ClearSelRequest { + /// Reservation identifier. + pub reservation_id: U16, + /// Required `CLR` signature. + pub signature: [u8; 3], + /// Erase or status-query operation. + pub operation: u8, +} + +/// Clear SEL response body. +#[repr(C)] +#[derive(Copy, Clone, Debug, IntoBytes, FromBytes, Immutable, KnownLayout, Unaligned)] +pub struct ClearSelResponse { + /// IPMI completion code. + pub completion_code: u8, + /// Current erase status. + pub erase_status: u8, +} + +/// Get SEL Time response body. +#[repr(C, packed)] +#[derive(Copy, Clone, Debug, IntoBytes, FromBytes, Immutable, KnownLayout, Unaligned)] +pub struct GetSelTimeResponse { + /// IPMI completion code. + pub completion_code: u8, + /// Current SEL time in seconds since the Unix epoch. + pub timestamp: U32, +} + +/// Set SEL Time request data. +#[repr(C, packed)] +#[derive(Copy, Clone, Debug, IntoBytes, FromBytes, Immutable, KnownLayout, Unaligned)] +pub struct SetSelTimeRequest { + /// Requested SEL time in seconds since the Unix epoch. + pub timestamp: U32, +} + +const_assert_eq!(size_of::(), 2); +const_assert_eq!(offset_of!(MessageHeader, command), 1); +const_assert_eq!(size_of::(), 1); +const_assert_eq!(size_of::(), 12); +const_assert_eq!(offset_of!(GetDeviceIdResponse, manufacturer_id), 7); +const_assert_eq!(offset_of!(GetDeviceIdResponse, product_id), 10); +const_assert_eq!(size_of::(), 15); +const_assert_eq!(offset_of!(GetSelInfoResponse, entry_count), 2); +const_assert_eq!(offset_of!(GetSelInfoResponse, free_space), 4); +const_assert_eq!(offset_of!(GetSelInfoResponse, last_addition_timestamp), 6); +const_assert_eq!(offset_of!(GetSelInfoResponse, last_erase_timestamp), 10); +const_assert_eq!(offset_of!(GetSelInfoResponse, operation_support), 14); +const_assert_eq!(size_of::(), 3); +const_assert_eq!(offset_of!(ReserveSelResponse, reservation_id), 1); +const_assert_eq!(size_of::(), 6); +const_assert_eq!(offset_of!(GetSelEntryRequest, record_id), 2); +const_assert_eq!(offset_of!(GetSelEntryRequest, offset), 4); +const_assert_eq!(offset_of!(GetSelEntryRequest, bytes_to_read), 5); +const_assert_eq!(size_of::(), 3); +const_assert_eq!(offset_of!(GetSelEntryResponseHeader, next_record_id), 1); +const_assert_eq!(size_of::(), SEL_RECORD_SIZE); +const_assert_eq!(size_of::(), 3); +const_assert_eq!(offset_of!(AddSelEntryResponse, record_id), 1); +const_assert_eq!(size_of::(), 6); +const_assert_eq!(offset_of!(ClearSelRequest, signature), 2); +const_assert_eq!(offset_of!(ClearSelRequest, operation), 5); +const_assert_eq!(size_of::(), 2); +const_assert_eq!(size_of::(), 5); +const_assert_eq!(offset_of!(GetSelTimeResponse, timestamp), 1); +const_assert_eq!(size_of::(), 4); + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn get_device_id_wire_layout() { + assert_eq!( + GetDeviceIdResponse::new().as_bytes(), + [0x00, 0x20, 0x01, 0x02, 0x00, 0x02, 0x04, 0, 0, 0, 0, 0] + ); + } + + #[test] + fn message_header_wire_layout() { + let request = MessageHeader { + netfn_lun: NETFN_STORAGE << 2, + command: COMMAND_GET_SEL_INFO, + }; + + assert_eq!(request.as_bytes(), [0x28, 0x40]); + assert_eq!(request.netfn(), NETFN_STORAGE); + assert_eq!(request.response().as_bytes(), [0x2c, 0x40]); + } + + #[test] + fn get_sel_info_wire_layout() { + let response = GetSelInfoResponse { + completion_code: COMPLETION_SUCCESS, + sel_version: SEL_VERSION, + entry_count: U16::new(2), + free_space: U16::new(0x07e0), + last_addition_timestamp: U32::new(0x12345678), + last_erase_timestamp: U32::new(0x90abcdef), + operation_support: SEL_OPERATION_SUPPORT_RESERVE, + }; + + assert_eq!( + response.as_bytes(), + [ + 0x00, 0x51, 0x02, 0x00, 0xe0, 0x07, 0x78, 0x56, 0x34, 0x12, 0xef, 0xcd, 0xab, 0x90, + 0x02 + ] + ); + } + + #[test] + fn get_sel_entry_request_wire_layout() { + let request = GetSelEntryRequest::read_from_prefix(&[0x01, 0x00, 0x34, 0x12, 0x04, 0x08]) + .unwrap() + .0; + + assert_eq!(request.reservation_id.get(), 1); + assert_eq!(request.record_id.get(), 0x1234); + assert_eq!(request.offset, 4); + assert_eq!(request.bytes_to_read, 8); + } + + #[test] + fn sel_command_wire_layouts() { + assert_eq!( + ReserveSelResponse { + completion_code: COMPLETION_SUCCESS, + reservation_id: U16::new(0x1234), + } + .as_bytes(), + [0x00, 0x34, 0x12] + ); + assert_eq!( + GetSelEntryResponseHeader { + completion_code: COMPLETION_SUCCESS, + next_record_id: U16::new(0xabcd), + } + .as_bytes(), + [0x00, 0xcd, 0xab] + ); + assert_eq!( + AddSelEntryResponse { + completion_code: COMPLETION_SUCCESS, + record_id: U16::new(0x1234), + } + .as_bytes(), + [0x00, 0x34, 0x12] + ); + assert_eq!( + ClearSelRequest { + reservation_id: U16::new(1), + signature: CLEAR_SEL_SIGNATURE, + operation: CLEAR_SEL_INITIATE_ERASE, + } + .as_bytes(), + [0x01, 0x00, b'C', b'L', b'R', 0xaa] + ); + assert_eq!( + ClearSelResponse { + completion_code: COMPLETION_SUCCESS, + erase_status: CLEAR_SEL_ERASE_COMPLETE, + } + .as_bytes(), + [0x00, 0x01] + ); + assert_eq!( + GetSelTimeResponse { + completion_code: COMPLETION_SUCCESS, + timestamp: U32::new(0x12345678), + } + .as_bytes(), + [0x00, 0x78, 0x56, 0x34, 0x12] + ); + assert_eq!( + SetSelTimeRequest { + timestamp: U32::new(0x12345678), + } + .as_bytes(), + [0x78, 0x56, 0x34, 0x12] + ); + } +} diff --git a/vm/devices/chipset_resources/Cargo.toml b/vm/devices/chipset_resources/Cargo.toml index 58ef826cef..7659952adb 100644 --- a/vm/devices/chipset_resources/Cargo.toml +++ b/vm/devices/chipset_resources/Cargo.toml @@ -7,8 +7,9 @@ edition.workspace = true rust-version.workspace = true [dependencies] -vm_resource.workspace = true +ipmi_protocol.workspace = true local_clock.workspace = true +vm_resource.workspace = true inspect.workspace = true memory_range.workspace = true diff --git a/vm/devices/chipset_resources/src/lib.rs b/vm/devices/chipset_resources/src/lib.rs index 2cdcaab33e..76fd05a56b 100644 --- a/vm/devices/chipset_resources/src/lib.rs +++ b/vm/devices/chipset_resources/src/lib.rs @@ -26,50 +26,16 @@ impl CanResolveTo for CmosRtcTimeSourceHandleKind { type Input<'a> = (); } -/// The size of an IPMI System Event Log record. -pub const IPMI_SEL_RECORD_SIZE: usize = 16; - -/// Result of attempting to forward an IPMI SEL record. -#[derive(Debug, Clone, Copy, PartialEq, Eq)] -pub enum SelEventDisposition { - /// The sink accepted the record. - Accepted, - /// The sink dropped the record without blocking the virtual BMC. - Dropped, -} - -/// Non-blocking sink for completed IPMI SEL records. -pub trait SelEventSink: Send { - /// Attempts to forward a completed SEL record. - fn try_send( - &mut self, - record_id: u16, - record: [u8; IPMI_SEL_RECORD_SIZE], - ) -> SelEventDisposition; -} - -/// Resource kind for IPMI SEL event sinks. -pub enum IpmiSelEventSinkHandleKind {} - -impl ResourceKind for IpmiSelEventSinkHandleKind { - const NAME: &'static str = "ipmi_sel_event_sink"; -} - -/// Resolved runtime IPMI SEL event sink. -pub struct ResolvedIpmiSelEventSink(pub Box); - -impl CanResolveTo for IpmiSelEventSinkHandleKind { - type Input<'a> = (); -} - pub mod ipmi_kcs { //! Resource definitions for the IPMI KCS virtual BMC. use super::CmosRtcTimeSourceHandleKind; - use super::IpmiSelEventSinkHandleKind; + use ipmi_protocol::SelRecord; use mesh::MeshPayload; + use vm_resource::CanResolveTo; use vm_resource::Resource; use vm_resource::ResourceId; + use vm_resource::ResourceKind; use vm_resource::kind::ChipsetDeviceHandleKind; /// AMD64 KCS data-register port. @@ -88,6 +54,35 @@ pub mod ipmi_kcs { /// Size of the ARM64 KCS MMIO aperture. pub const IPMI_KCS_MMIO_REGION_SIZE_AARCH64: u64 = 0x1000; + /// Result of attempting to forward an IPMI SEL record. + #[derive(Debug, Clone, Copy, PartialEq, Eq)] + pub enum SendOutcome { + /// The sink accepted the record. + Accepted, + /// The sink dropped the record without blocking the virtual BMC. + Dropped, + } + + /// Non-blocking sink for completed IPMI SEL records. + pub trait SelEventSink: Send { + /// Attempts to forward a completed SEL record. + fn try_send(&mut self, record_id: u16, record: SelRecord) -> SendOutcome; + } + + /// Resource kind for IPMI SEL event sinks. + pub enum IpmiSelEventSinkHandleKind {} + + impl ResourceKind for IpmiSelEventSinkHandleKind { + const NAME: &'static str = "ipmi_sel_event_sink"; + } + + /// Resolved runtime IPMI SEL event sink. + pub struct ResolvedIpmiSelEventSink(pub Box); + + impl CanResolveTo for IpmiSelEventSinkHandleKind { + type Input<'a> = (); + } + /// A handle to an AMD64 IPMI KCS virtual BMC. #[derive(MeshPayload)] pub struct IpmiKcsDeviceHandleX64 { diff --git a/vm/devices/get/get_protocol/Cargo.toml b/vm/devices/get/get_protocol/Cargo.toml index 212794821c..a0e52bb96c 100644 --- a/vm/devices/get/get_protocol/Cargo.toml +++ b/vm/devices/get/get_protocol/Cargo.toml @@ -8,6 +8,7 @@ rust-version.workspace = true [dependencies] guid.workspace = true +ipmi_protocol.workspace = true open_enum.workspace = true serde_helpers.workspace = true diff --git a/vm/devices/get/get_protocol/src/dps_json.rs b/vm/devices/get/get_protocol/src/dps_json.rs index 479f5d633b..b7d31ffbae 100644 --- a/vm/devices/get/get_protocol/src/dps_json.rs +++ b/vm/devices/get/get_protocol/src/dps_json.rs @@ -316,16 +316,21 @@ mod test { use super::*; #[test] - fn ipmi_defaults_false_and_parses_true() { - let settings = serde_json::from_slice::(include_bytes!( + fn smoke_test_sample() { + serde_json::from_slice::(include_bytes!( "dps_test_json.json" )) .unwrap(); - assert!(!settings.v1.enable_ipmi); + } + + #[test] + fn ipmi_defaults_false_and_parses_true() { + let default = serde_json::from_str::("{}").unwrap(); + assert!(!default.enable_ipmi); - let settings = + let enabled = serde_json::from_str::(r#"{"EnableIpmi":true}"#).unwrap(); - assert!(settings.enable_ipmi); + assert!(enabled.enable_ipmi); } #[test] diff --git a/vm/devices/get/get_protocol/src/lib.rs b/vm/devices/get/get_protocol/src/lib.rs index f1875772cc..c14df417ac 100644 --- a/vm/devices/get/get_protocol/src/lib.rs +++ b/vm/devices/get/get_protocol/src/lib.rs @@ -378,7 +378,7 @@ impl EventLogNotification { } } -pub const IPMI_SEL_RECORD_SIZE: usize = 16; +pub use ipmi_protocol::SEL_RECORD_SIZE as IPMI_SEL_RECORD_SIZE; #[repr(C)] #[derive(Copy, Clone, Debug, IntoBytes, FromBytes, Immutable, KnownLayout)] diff --git a/vm/devices/get/guest_emulation_transport/src/resolver.rs b/vm/devices/get/guest_emulation_transport/src/resolver.rs index 90f00a92c8..6794fcbcc4 100644 --- a/vm/devices/get/guest_emulation_transport/src/resolver.rs +++ b/vm/devices/get/guest_emulation_transport/src/resolver.rs @@ -4,11 +4,11 @@ //! Resource definitions for the GET client. use crate::GuestEmulationTransportClient; -use chipset_resources::IPMI_SEL_RECORD_SIZE; -use chipset_resources::IpmiSelEventSinkHandleKind; -use chipset_resources::ResolvedIpmiSelEventSink; -use chipset_resources::SelEventDisposition; -use chipset_resources::SelEventSink; +use chipset_resources::ipmi_kcs::IpmiSelEventSinkHandleKind; +use chipset_resources::ipmi_kcs::ResolvedIpmiSelEventSink; +use chipset_resources::ipmi_kcs::SelEventSink; +use chipset_resources::ipmi_kcs::SendOutcome; +use get_protocol::IPMI_SEL_RECORD_SIZE; use std::convert::Infallible; use vm_resource::CanResolveTo; use vm_resource::PlatformResource; @@ -44,13 +44,9 @@ impl ResolveResource for GuestEmulationTranspor struct GetIpmiSelEventSink(GuestEmulationTransportClient); impl SelEventSink for GetIpmiSelEventSink { - fn try_send( - &mut self, - record_id: u16, - record: [u8; IPMI_SEL_RECORD_SIZE], - ) -> SelEventDisposition { + fn try_send(&mut self, record_id: u16, record: [u8; IPMI_SEL_RECORD_SIZE]) -> SendOutcome { self.0.ipmi_sel(record_id, record); - SelEventDisposition::Accepted + SendOutcome::Accepted } } From b8e9f502a695ae19fdc92060d314befff4f9c1a0 Mon Sep 17 00:00:00 2001 From: Ayush Arora Date: Wed, 16 Sep 2026 13:56:53 +0530 Subject: [PATCH 05/10] ipmi: address protocol review feedback Clarify response lengths and virtual BMC identity, link the IPMI specification, align attestation serialization, and move the Linux ioctl helper into test data. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 7eb1fc4c-f20e-4de8-896b-a168b7c9ff8b --- .../src/igvm_attest/get.rs | 1 - vm/devices/chipset/ipmi_kcs/src/protocol.rs | 5 +- vm/devices/chipset/ipmi_protocol/src/lib.rs | 44 ++++-- vmm_tests/vmm_tests/test_data/ipmi_add_sel.py | 130 ++++++++++++++++++ .../vmm_tests/tests/tests/x86_64/ipmi.rs | 111 +-------------- 5 files changed, 171 insertions(+), 120 deletions(-) create mode 100644 vmm_tests/vmm_tests/test_data/ipmi_add_sel.py diff --git a/openhcl/openhcl_attestation_protocol/src/igvm_attest/get.rs b/openhcl/openhcl_attestation_protocol/src/igvm_attest/get.rs index 39d646ec1a..6e9c0b7b2e 100644 --- a/openhcl/openhcl_attestation_protocol/src/igvm_attest/get.rs +++ b/openhcl/openhcl_attestation_protocol/src/igvm_attest/get.rs @@ -493,7 +493,6 @@ pub mod runtime_claims { /// Whether the serial console, if enabled, is interactive pub interactive_console_enabled: bool, /// Whether the IPMI KCS interface is enabled - #[serde(default)] pub ipmi_enabled: bool, /// Whether secure boot is enabled pub secure_boot: bool, diff --git a/vm/devices/chipset/ipmi_kcs/src/protocol.rs b/vm/devices/chipset/ipmi_kcs/src/protocol.rs index 3c25c35473..ee6502ad3c 100644 --- a/vm/devices/chipset/ipmi_kcs/src/protocol.rs +++ b/vm/devices/chipset/ipmi_kcs/src/protocol.rs @@ -23,6 +23,7 @@ use zerocopy::IntoBytes; impl IpmiKcs { /// Dispatches the accumulated IPMI request and stages its KCS response. pub(crate) fn process_ipmi_message(&mut self) { + // Keep a local copy so the command handlers can mutably borrow the device. let request = self.transaction.request; let request = &request[..self.transaction.request_len]; let Ok((header, data)) = MessageHeader::read_from_prefix(request) else { @@ -67,13 +68,15 @@ impl IpmiKcs { out: &mut [u8; KCS_MESSAGE_MAX], ) -> usize { match command { - COMMAND_GET_DEVICE_ID => write_response(out, &GetDeviceIdResponse::new()), + COMMAND_GET_DEVICE_ID => write_response(out, &GetDeviceIdResponse::virtual_bmc()), _ => completion(out, COMPLETION_INVALID_COMMAND), } } } /// Writes a fixed-format response body into the KCS output buffer. +/// +/// Returns the number of bytes written to the beginning of `out`. pub(crate) fn write_response( out: &mut [u8; KCS_MESSAGE_MAX], response: &T, diff --git a/vm/devices/chipset/ipmi_protocol/src/lib.rs b/vm/devices/chipset/ipmi_protocol/src/lib.rs index 2d872bb71d..b644ff5af0 100644 --- a/vm/devices/chipset/ipmi_protocol/src/lib.rs +++ b/vm/devices/chipset/ipmi_protocol/src/lib.rs @@ -2,6 +2,10 @@ // Licensed under the MIT License. //! Wire-level definitions for the IPMI KCS interface and System Event Log. +//! +//! These definitions follow the [IPMI v2.0 specification][ipmi-spec]. +//! +//! [ipmi-spec]: https://www.intel.com/content/dam/www/public/us/en/documents/product-briefs/ipmi-second-gen-interface-spec-v2-rev1-1.pdf #![forbid(unsafe_code)] @@ -20,6 +24,12 @@ use zerocopy::Unaligned; /// Size of an IPMI System Event Log record. pub const SEL_RECORD_SIZE: usize = 16; +/// IPMI 2.0 version encoding used by Get Device ID. +pub const IPMI_VERSION_2_0: u8 = 0x02; + +/// Additional device support bit indicating that the BMC provides a SEL. +pub const ADDITIONAL_DEVICE_SUPPORT_SEL: u8 = 0x04; + /// IPMI SEL version 1.5. pub const SEL_VERSION: u8 = 0x51; @@ -201,26 +211,36 @@ pub struct GetDeviceIdResponse { pub product_id: U16, } +const VIRTUAL_BMC_DEVICE_ID: u8 = 0x20; +const VIRTUAL_BMC_DEVICE_REVISION: u8 = 0x01; +const VIRTUAL_BMC_FIRMWARE_MAJOR: u8 = 0x02; +const VIRTUAL_BMC_FIRMWARE_MINOR: u8 = 0x00; +const VIRTUAL_BMC_MANUFACTURER_ID: [u8; 3] = [0; 3]; +const VIRTUAL_BMC_PRODUCT_ID: U16 = U16::ZERO; + impl GetDeviceIdResponse { - /// Creates the identity returned by the virtual BMC. - pub const fn new() -> Self { + /// Creates the fixed identity returned by the virtual BMC for Get Device ID. + /// + /// The device, firmware, manufacturer, and product values are synthetic. + /// The IPMI version and additional support fields advertise IPMI 2.0 with SEL support. + pub const fn virtual_bmc() -> Self { Self { completion_code: COMPLETION_SUCCESS, - device_id: 0x20, - device_revision: 0x01, - firmware_revision_1: 0x02, - firmware_revision_2: 0x00, - ipmi_version: 0x02, - additional_device_support: 0x04, - manufacturer_id: [0; 3], - product_id: U16::ZERO, + device_id: VIRTUAL_BMC_DEVICE_ID, + device_revision: VIRTUAL_BMC_DEVICE_REVISION, + firmware_revision_1: VIRTUAL_BMC_FIRMWARE_MAJOR, + firmware_revision_2: VIRTUAL_BMC_FIRMWARE_MINOR, + ipmi_version: IPMI_VERSION_2_0, + additional_device_support: ADDITIONAL_DEVICE_SUPPORT_SEL, + manufacturer_id: VIRTUAL_BMC_MANUFACTURER_ID, + product_id: VIRTUAL_BMC_PRODUCT_ID, } } } impl Default for GetDeviceIdResponse { fn default() -> Self { - Self::new() + Self::virtual_bmc() } } @@ -374,7 +394,7 @@ mod tests { #[test] fn get_device_id_wire_layout() { assert_eq!( - GetDeviceIdResponse::new().as_bytes(), + GetDeviceIdResponse::virtual_bmc().as_bytes(), [0x00, 0x20, 0x01, 0x02, 0x00, 0x02, 0x04, 0, 0, 0, 0, 0] ); } diff --git a/vmm_tests/vmm_tests/test_data/ipmi_add_sel.py b/vmm_tests/vmm_tests/test_data/ipmi_add_sel.py new file mode 100644 index 0000000000..b5264f01eb --- /dev/null +++ b/vmm_tests/vmm_tests/test_data/ipmi_add_sel.py @@ -0,0 +1,130 @@ +# Copyright (c) Microsoft Corporation. +# Licensed under the MIT License. + +import ctypes +import os +import select + +IPMI_SYSTEM_INTERFACE_ADDR_TYPE = 0x0C +IPMI_BMC_CHANNEL = 0x0F +IPMI_RESPONSE_RECV_TYPE = 1 + + +class IpmiSystemInterfaceAddr(ctypes.Structure): + _fields_ = [ + ("addr_type", ctypes.c_int), + ("channel", ctypes.c_short), + ("lun", ctypes.c_ubyte), + ] + + +class IpmiMsg(ctypes.Structure): + _fields_ = [ + ("netfn", ctypes.c_ubyte), + ("cmd", ctypes.c_ubyte), + ("data_len", ctypes.c_ushort), + ("data", ctypes.c_void_p), + ] + + +class IpmiReq(ctypes.Structure): + _fields_ = [ + ("addr", ctypes.c_void_p), + ("addr_len", ctypes.c_uint), + ("msgid", ctypes.c_long), + ("msg", IpmiMsg), + ] + + +class IpmiRecv(ctypes.Structure): + _fields_ = [ + ("recv_type", ctypes.c_int), + ("addr", ctypes.c_void_p), + ("addr_len", ctypes.c_uint), + ("msgid", ctypes.c_long), + ("msg", IpmiMsg), + ] + + +def ioctl_code(direction, number, size): + return (direction << 30) | (size << 16) | (ord("i") << 8) | number + + +def ioctl(fd, request, value): + result = libc.ioctl(fd, request, ctypes.byref(value)) + if result != 0: + error = ctypes.get_errno() + raise OSError(error, os.strerror(error)) + + +device_path = next( + ( + path + for path in ("/dev/ipmi0", "/dev/ipmi/0", "/dev/ipmidev/0") + if os.path.exists(path) + ), + None, +) +if device_path is None: + raise RuntimeError("Linux IPMI device was not created") + +libc = ctypes.CDLL(None, use_errno=True) +libc.ioctl.argtypes = [ctypes.c_int, ctypes.c_ulong, ctypes.c_void_p] +libc.ioctl.restype = ctypes.c_int + +address = IpmiSystemInterfaceAddr(IPMI_SYSTEM_INTERFACE_ADDR_TYPE, IPMI_BMC_CHANNEL, 0) +request_data = (ctypes.c_ubyte * 16)( + 0x00, + 0x00, + 0x02, + 0x00, + 0x00, + 0x00, + 0x00, + 0x20, + 0x00, + 0x04, + 0x09, + 0x01, + 0x6F, + 0xDE, + 0xAD, + 0xBE, +) +request = IpmiReq( + ctypes.addressof(address), + ctypes.sizeof(address), + 1, + IpmiMsg(0x0A, 0x44, len(request_data), ctypes.addressof(request_data)), +) + +fd = os.open(device_path, os.O_RDWR) +try: + ioctl(fd, ioctl_code(2, 13, ctypes.sizeof(IpmiReq)), request) + if not select.select([fd], [], [], 10)[0]: + raise TimeoutError("timed out waiting for the Add SEL response") + + response_address = (ctypes.c_ubyte * 32)() + response_data = (ctypes.c_ubyte * 64)() + response = IpmiRecv( + 0, + ctypes.addressof(response_address), + len(response_address), + 0, + IpmiMsg(0, 0, len(response_data), ctypes.addressof(response_data)), + ) + ioctl(fd, ioctl_code(3, 11, ctypes.sizeof(IpmiRecv)), response) + + data = bytes(response_data[: response.msg.data_len]) + if response.recv_type != IPMI_RESPONSE_RECV_TYPE: + raise RuntimeError(f"unexpected receive type {response.recv_type}") + if response.msgid != 1 or response.msg.cmd != 0x44: + raise RuntimeError( + f"unexpected response msgid={response.msgid} command={response.msg.cmd:#x}" + ) + if len(data) != 3 or data[0] != 0: + raise RuntimeError(f"Add SEL failed: {data.hex()}") + + print(f"ADDSEL_CC=0 RECORD_ID={int.from_bytes(data[1:3], 'little')}") +finally: + os.close(fd) diff --git a/vmm_tests/vmm_tests/tests/tests/x86_64/ipmi.rs b/vmm_tests/vmm_tests/tests/tests/x86_64/ipmi.rs index bcc539e664..c4a2f5bb28 100644 --- a/vmm_tests/vmm_tests/tests/tests/x86_64/ipmi.rs +++ b/vmm_tests/vmm_tests/tests/tests/x86_64/ipmi.rs @@ -10,110 +10,7 @@ use petri::pipette::cmd; use petri_artifacts_common::tags::OsFlavor; use vmm_test_macros::openvmm_test; -const LINUX_IPMI_TEST: &str = r#" -import ctypes -import os -import select - -IPMI_SYSTEM_INTERFACE_ADDR_TYPE = 0x0c -IPMI_BMC_CHANNEL = 0x0f -IPMI_RESPONSE_RECV_TYPE = 1 - -class IpmiSystemInterfaceAddr(ctypes.Structure): - _fields_ = [ - ("addr_type", ctypes.c_int), - ("channel", ctypes.c_short), - ("lun", ctypes.c_ubyte), - ] - -class IpmiMsg(ctypes.Structure): - _fields_ = [ - ("netfn", ctypes.c_ubyte), - ("cmd", ctypes.c_ubyte), - ("data_len", ctypes.c_ushort), - ("data", ctypes.c_void_p), - ] - -class IpmiReq(ctypes.Structure): - _fields_ = [ - ("addr", ctypes.c_void_p), - ("addr_len", ctypes.c_uint), - ("msgid", ctypes.c_long), - ("msg", IpmiMsg), - ] - -class IpmiRecv(ctypes.Structure): - _fields_ = [ - ("recv_type", ctypes.c_int), - ("addr", ctypes.c_void_p), - ("addr_len", ctypes.c_uint), - ("msgid", ctypes.c_long), - ("msg", IpmiMsg), - ] - -def ioctl_code(direction, number, size): - return (direction << 30) | (size << 16) | (ord("i") << 8) | number - -def ioctl(fd, request, value): - result = libc.ioctl(fd, request, ctypes.byref(value)) - if result != 0: - error = ctypes.get_errno() - raise OSError(error, os.strerror(error)) - -device_path = next( - (path for path in ("/dev/ipmi0", "/dev/ipmi/0", "/dev/ipmidev/0") if os.path.exists(path)), - None, -) -if device_path is None: - raise RuntimeError("Linux IPMI device was not created") - -libc = ctypes.CDLL(None, use_errno=True) -libc.ioctl.argtypes = [ctypes.c_int, ctypes.c_ulong, ctypes.c_void_p] -libc.ioctl.restype = ctypes.c_int - -address = IpmiSystemInterfaceAddr(IPMI_SYSTEM_INTERFACE_ADDR_TYPE, IPMI_BMC_CHANNEL, 0) -request_data = (ctypes.c_ubyte * 16)( - 0x00, 0x00, 0x02, 0x00, 0x00, 0x00, 0x00, 0x20, - 0x00, 0x04, 0x09, 0x01, 0x6f, 0xde, 0xad, 0xbe, -) -request = IpmiReq( - ctypes.addressof(address), - ctypes.sizeof(address), - 1, - IpmiMsg(0x0a, 0x44, len(request_data), ctypes.addressof(request_data)), -) - -fd = os.open(device_path, os.O_RDWR) -try: - ioctl(fd, ioctl_code(2, 13, ctypes.sizeof(IpmiReq)), request) - if not select.select([fd], [], [], 10)[0]: - raise TimeoutError("timed out waiting for the Add SEL response") - - response_address = (ctypes.c_ubyte * 32)() - response_data = (ctypes.c_ubyte * 64)() - response = IpmiRecv( - 0, - ctypes.addressof(response_address), - len(response_address), - 0, - IpmiMsg(0, 0, len(response_data), ctypes.addressof(response_data)), - ) - ioctl(fd, ioctl_code(3, 11, ctypes.sizeof(IpmiRecv)), response) - - data = bytes(response_data[:response.msg.data_len]) - if response.recv_type != IPMI_RESPONSE_RECV_TYPE: - raise RuntimeError(f"unexpected receive type {response.recv_type}") - if response.msgid != 1 or response.msg.cmd != 0x44: - raise RuntimeError( - f"unexpected response msgid={response.msgid} command={response.msg.cmd:#x}" - ) - if len(data) != 3 or data[0] != 0: - raise RuntimeError(f"Add SEL failed: {data.hex()}") - - print(f"ADDSEL_CC=0 RECORD_ID={int.from_bytes(data[1:3], 'little')}") -finally: - os.close(fd) -"#; +const LINUX_IPMI_TEST: &str = include_str!("../../../test_data/ipmi_add_sel.py"); const WINDOWS_IPMI_TEST: &str = r#" $ipmi = Get-CimInstance -Namespace root\wmi -ClassName Microsoft_IPMI -ErrorAction Stop @@ -149,10 +46,12 @@ async fn ipmi_kcs_add_sel(config: PetriVmBuilder) -> anyhow cmd!(shell, "sudo modprobe ipmi_si").run().await?; cmd!(shell, "sudo modprobe ipmi_devintf").run().await?; agent - .write_file("/tmp/ipmi_test.py", LINUX_IPMI_TEST.as_bytes()) + .write_file("/tmp/ipmi_add_sel.py", LINUX_IPMI_TEST.as_bytes()) .await .context("failed to copy the Linux IPMI test into the guest")?; - cmd!(shell, "sudo python3 /tmp/ipmi_test.py").read().await? + cmd!(shell, "sudo python3 /tmp/ipmi_add_sel.py") + .read() + .await? } OsFlavor::Windows => { let shell = agent.windows_shell(); From ba8512352fdd1220e1d50bfdfe8165d95c560b14 Mon Sep 17 00:00:00 2001 From: Ayush Arora Date: Wed, 16 Sep 2026 16:20:39 +0530 Subject: [PATCH 06/10] chore: retrigger PR validation Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 7eb1fc4c-f20e-4de8-896b-a168b7c9ff8b From 3dfd08403d0a2627a953cfa590e97277242b3a76 Mon Sep 17 00:00:00 2001 From: Ayush Arora Date: Thu, 17 Sep 2026 10:21:48 +0530 Subject: [PATCH 07/10] test: document IPMI SEL ioctl helper Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 7eb1fc4c-f20e-4de8-896b-a168b7c9ff8b --- vmm_tests/vmm_tests/test_data/ipmi_add_sel.py | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/vmm_tests/vmm_tests/test_data/ipmi_add_sel.py b/vmm_tests/vmm_tests/test_data/ipmi_add_sel.py index b5264f01eb..a038970f50 100644 --- a/vmm_tests/vmm_tests/test_data/ipmi_add_sel.py +++ b/vmm_tests/vmm_tests/test_data/ipmi_add_sel.py @@ -1,6 +1,11 @@ # Copyright (c) Microsoft Corporation. # Licensed under the MIT License. +# Sends an Add SEL command through Linux's /dev/ipmi ioctl interface. Sysfs can +# verify device discovery but cannot submit the command, and ipmitool is not +# installed in the Petri Ubuntu image. This can be replaced by ipmitool if it +# becomes a guaranteed image dependency, or by a dedicated Rust guest utility. + import ctypes import os import select @@ -8,6 +13,8 @@ IPMI_SYSTEM_INTERFACE_ADDR_TYPE = 0x0C IPMI_BMC_CHANNEL = 0x0F IPMI_RESPONSE_RECV_TYPE = 1 +IOC_WRITE = 1 +IOC_READ = 2 class IpmiSystemInterfaceAddr(ctypes.Structure): @@ -100,7 +107,9 @@ def ioctl(fd, request, value): fd = os.open(device_path, os.O_RDWR) try: - ioctl(fd, ioctl_code(2, 13, ctypes.sizeof(IpmiReq)), request) + # Linux defines IPMICTL_SEND_COMMAND with _IOR despite the command sending + # request data from userspace to the kernel. + ioctl(fd, ioctl_code(IOC_READ, 13, ctypes.sizeof(IpmiReq)), request) if not select.select([fd], [], [], 10)[0]: raise TimeoutError("timed out waiting for the Add SEL response") @@ -113,7 +122,7 @@ def ioctl(fd, request, value): 0, IpmiMsg(0, 0, len(response_data), ctypes.addressof(response_data)), ) - ioctl(fd, ioctl_code(3, 11, ctypes.sizeof(IpmiRecv)), response) + ioctl(fd, ioctl_code(IOC_READ | IOC_WRITE, 11, ctypes.sizeof(IpmiRecv)), response) data = bytes(response_data[: response.msg.data_len]) if response.recv_type != IPMI_RESPONSE_RECV_TYPE: From 509291d4595b24d8f1077f669c71546729cc2638 Mon Sep 17 00:00:00 2001 From: Ayush Arora Date: Thu, 17 Sep 2026 12:09:33 +0530 Subject: [PATCH 08/10] test: fix ioctl error handling Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 7eb1fc4c-f20e-4de8-896b-a168b7c9ff8b --- vmm_tests/vmm_tests/test_data/ipmi_add_sel.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vmm_tests/vmm_tests/test_data/ipmi_add_sel.py b/vmm_tests/vmm_tests/test_data/ipmi_add_sel.py index a038970f50..4eee8811bb 100644 --- a/vmm_tests/vmm_tests/test_data/ipmi_add_sel.py +++ b/vmm_tests/vmm_tests/test_data/ipmi_add_sel.py @@ -59,7 +59,7 @@ def ioctl_code(direction, number, size): def ioctl(fd, request, value): result = libc.ioctl(fd, request, ctypes.byref(value)) - if result != 0: + if result == -1: error = ctypes.get_errno() raise OSError(error, os.strerror(error)) From 527dd1ca65cf6e3d02396ed20d89d66ffadbeea3 Mon Sep 17 00:00:00 2001 From: Ayush Arora Date: Fri, 18 Sep 2026 11:23:20 +0530 Subject: [PATCH 09/10] ipmi: address additional review feedback --- Cargo.lock | 1 + openhcl/underhill_core/src/worker.rs | 4 +- vm/devices/chipset/ipmi_kcs/src/device.rs | 23 +--- vm/devices/chipset/ipmi_kcs/src/lib.rs | 1 - vm/devices/chipset/ipmi_kcs/src/sel.rs | 12 +- vm/devices/chipset/ipmi_kcs/src/tests.rs | 113 +++--------------- vm/devices/chipset_resources/src/lib.rs | 23 ++-- vm/devices/get/get_protocol/src/dps_json.rs | 10 -- vm/devices/get/get_protocol/src/lib.rs | 29 ----- vm/devices/get/get_resources/src/lib.rs | 2 +- .../get/guest_emulation_transport/Cargo.toml | 1 + .../get/guest_emulation_transport/src/api.rs | 3 - .../guest_emulation_transport/src/client.rs | 2 +- .../guest_emulation_transport/src/resolver.rs | 5 +- vm/loader/src/uefi/config.rs | 5 - 15 files changed, 37 insertions(+), 197 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 9a4bbd49f4..1547119a04 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3169,6 +3169,7 @@ dependencies = [ "hvdef", "inspect", "inspect_counters", + "ipmi_protocol", "jiff", "mesh", "pal_async", diff --git a/openhcl/underhill_core/src/worker.rs b/openhcl/underhill_core/src/worker.rs index 0fd9958035..d2d21b6ce5 100644 --- a/openhcl/underhill_core/src/worker.rs +++ b/openhcl/underhill_core/src/worker.rs @@ -4498,9 +4498,7 @@ impl chipset_device_worker::RemoteDynamicResolvers for OpenHclRemoteDynamicResol self, resolver: &mut ResourceResolver, ) -> anyhow::Result<()> { - resolver.add_resolver(self.get.clone()); - resolver - .add_resolver(guest_emulation_transport::resolver::IpmiSelEventSinkResolver(self.get)); + resolver.add_resolver(self.get); if let Some(vmgs) = self.vmgs { resolver.add_resolver(vmgs); } diff --git a/vm/devices/chipset/ipmi_kcs/src/device.rs b/vm/devices/chipset/ipmi_kcs/src/device.rs index f40388e1c4..030c8eb007 100644 --- a/vm/devices/chipset/ipmi_kcs/src/device.rs +++ b/vm/devices/chipset/ipmi_kcs/src/device.rs @@ -262,7 +262,7 @@ mod tests { } #[test] - fn advertises_exact_amd64_pio_region() { + fn pio_device_maps_ports_and_dispatches() { let mut device = device(); assert!(device.supports_pio().is_some()); assert!(device.supports_mmio().is_none()); @@ -273,11 +273,6 @@ mod tests { IPMI_KCS_DATA_PORT..=IPMI_KCS_STATUS_COMMAND_PORT )] ); - } - - #[test] - fn rejects_invalid_accesses() { - let mut device = device(); assert!(matches!( device.io_read(IPMI_KCS_DATA_PORT, &mut [0; 2]), @@ -295,11 +290,7 @@ mod tests { device.io_write(IPMI_KCS_STATUS_COMMAND_PORT + 1, &[0]), IoResult::Err(IoError::InvalidRegister) )); - } - #[test] - fn get_device_id_works_over_pio() { - let mut device = device(); let response = transact( &mut device, &[NETFN_APPLICATION << 2, COMMAND_GET_DEVICE_ID], @@ -327,7 +318,7 @@ mod tests { } #[test] - fn advertises_exact_arm64_mmio_region() { + fn mmio_device_maps_region_and_dispatches() { let mut device = mmio_device(); assert!(device.supports_pio().is_none()); assert!(device.supports_mmio().is_some()); @@ -339,11 +330,6 @@ mod tests { ..=IPMI_KCS_MMIO_BASE_ADDRESS_AARCH64 + IPMI_KCS_MMIO_REGION_SIZE_AARCH64 - 1 )] ); - } - - #[test] - fn arm64_mmio_rejects_invalid_accesses() { - let mut device = mmio_device(); assert!(matches!( device.mmio_read(IPMI_KCS_MMIO_DATA_ADDRESS_AARCH64, &mut [0; 4]), @@ -361,11 +347,6 @@ mod tests { device.mmio_write(IPMI_KCS_MMIO_STATUS_COMMAND_ADDRESS_AARCH64 + 1, &[0]), IoResult::Err(IoError::InvalidRegister) )); - } - - #[test] - fn get_device_id_works_over_arm64_mmio() { - let mut device = mmio_device(); device .mmio_write( diff --git a/vm/devices/chipset/ipmi_kcs/src/lib.rs b/vm/devices/chipset/ipmi_kcs/src/lib.rs index 3fd8d156d3..2f1f69d3f0 100644 --- a/vm/devices/chipset/ipmi_kcs/src/lib.rs +++ b/vm/devices/chipset/ipmi_kcs/src/lib.rs @@ -18,7 +18,6 @@ mod sel; mod tests; pub use chipset_resources::ipmi_kcs::SelEventSink; -pub use chipset_resources::ipmi_kcs::SendOutcome; pub use ipmi_protocol::KCS_COMMAND_GET_STATUS_ABORT; pub use ipmi_protocol::KCS_COMMAND_WRITE_END; pub use ipmi_protocol::KCS_COMMAND_WRITE_START; diff --git a/vm/devices/chipset/ipmi_kcs/src/sel.rs b/vm/devices/chipset/ipmi_kcs/src/sel.rs index 1dc70632f6..776ab69025 100644 --- a/vm/devices/chipset/ipmi_kcs/src/sel.rs +++ b/vm/devices/chipset/ipmi_kcs/src/sel.rs @@ -5,7 +5,6 @@ use crate::IpmiKcs; use crate::KCS_MESSAGE_MAX; -use crate::SendOutcome; use crate::protocol::completion; use crate::protocol::invalid_length; use crate::protocol::write_response; @@ -213,13 +212,10 @@ impl IpmiKcs { if let Some(sink) = self.sink.as_mut() { if self.rate_limiter.allow(trusted_seconds) { - match sink.try_send(record_id, record) { - SendOutcome::Accepted => { - self.stats.forwarded = self.stats.forwarded.saturating_add(1); - } - SendOutcome::Dropped => { - self.stats.sink_dropped = self.stats.sink_dropped.saturating_add(1); - } + if sink.try_send(record_id, record) { + self.stats.forwarded = self.stats.forwarded.saturating_add(1); + } else { + self.stats.sink_dropped = self.stats.sink_dropped.saturating_add(1); } } else { self.stats.rate_limited = self.stats.rate_limited.saturating_add(1); diff --git a/vm/devices/chipset/ipmi_kcs/src/tests.rs b/vm/devices/chipset/ipmi_kcs/src/tests.rs index ea05f50a5b..b0cf105b6b 100644 --- a/vm/devices/chipset/ipmi_kcs/src/tests.rs +++ b/vm/devices/chipset/ipmi_kcs/src/tests.rs @@ -48,12 +48,12 @@ struct SharedSink { } impl SelEventSink for SharedSink { - fn try_send(&mut self, record_id: u16, record: SelRecord) -> SendOutcome { + fn try_send(&mut self, record_id: u16, record: SelRecord) -> bool { if !self.accept.load(Ordering::Relaxed) { - return SendOutcome::Dropped; + return false; } self.state.lock().records.push((record_id, record)); - SendOutcome::Accepted + true } } @@ -505,20 +505,6 @@ fn sel_capacity_is_bounded_without_overwrite() { assert_eq!(device.sel_record(0).unwrap()[7], 1); } -#[test] -fn record_and_reservation_ids_roll_over_without_reserved_values() { - let (_, mut source) = device(1); - let mut state = source.save().unwrap(); - state.next_record_id = 0xfffe; - state.reservation_id = 0xffff; - let mut device = restore_target(1, state); - - assert_eq!(add_record(&mut device, 0x11)[3..5], [0xfe, 0xff]); - assert_eq!(add_record(&mut device, 0x22)[3..5], [1, 0]); - let reservation = transact(&mut device, &storage_request(COMMAND_RESERVE_SEL, &[])); - assert_eq!(reservation[3..5], [1, 0]); -} - #[test] fn clear_sel_validates_fields_and_resets_store() { let clock = FakeClock::new(100); @@ -575,7 +561,7 @@ fn clear_sel_validates_fields_and_resets_store() { } #[test] -fn sel_time_supports_positive_negative_and_wrapping_offsets() { +fn sel_time_supports_positive_and_negative_offsets() { let clock = FakeClock::new(1000); let mut device = IpmiKcs::new(Box::new(clock.clone())); @@ -606,15 +592,6 @@ fn sel_time_supports_positive_negative_and_wrapping_offsets() { clock.set(500); let time = transact(&mut device, &storage_request(COMMAND_GET_SEL_TIME, &[])); assert_eq!(u32::from_le_bytes(time[3..7].try_into().unwrap()), 0); - - clock.set(0); - transact( - &mut device, - &storage_request(COMMAND_SET_SEL_TIME, &u32::MAX.to_le_bytes()), - ); - clock.set(10); - let time = transact(&mut device, &storage_request(COMMAND_GET_SEL_TIME, &[])); - assert_eq!(u32::from_le_bytes(time[3..7].try_into().unwrap()), 9); } #[test] @@ -695,7 +672,7 @@ fn sink_forwarding_is_limited_to_256_per_trusted_second() { } #[test] -fn reset_preserves_sel_time_reservation_and_stats() { +fn reset_clears_transaction_and_preserves_sel() { let clock = FakeClock::new(100); let mut device = IpmiKcs::new(Box::new(clock)); transact( @@ -703,7 +680,6 @@ fn reset_preserves_sel_time_reservation_and_stats() { &storage_request(COMMAND_SET_SEL_TIME, &200u32.to_le_bytes()), ); add_record(&mut device, 0x42); - let reservation = transact(&mut device, &storage_request(COMMAND_RESERVE_SEL, &[])); device.write_command(KCS_COMMAND_WRITE_START); assert_eq!(device.read_data(), 0); @@ -714,13 +690,7 @@ fn reset_preserves_sel_time_reservation_and_stats() { assert_eq!(device.read_status(), KCS_STATE_IDLE); assert_eq!(device.sel_len(), 1); assert_eq!(device.sel_time_offset_seconds(), 100); - assert_eq!(device.stats().committed, 1); - - let current_reservation = transact(&mut device, &storage_request(COMMAND_RESERVE_SEL, &[])); - assert_eq!( - u16::from_le_bytes([current_reservation[3], current_reservation[4]]), - u16::from_le_bytes([reservation[3], reservation[4]]) + 1 - ); + assert_eq!(add_record(&mut device, 0x43)[3..5], [2, 0]); } fn restore_target(seconds: i64, state: save_restore::SavedState) -> IpmiKcs { @@ -770,34 +740,28 @@ fn save_restore_idle_write_and_read_transactions() { } #[test] -fn save_restore_full_sel_and_adjusted_time() { +fn save_restore_preserves_sel_and_adjusted_time() { let clock = FakeClock::new(1000); let mut source = IpmiKcs::new(Box::new(clock.clone())); transact( &mut source, &storage_request(COMMAND_SET_SEL_TIME, &1500u32.to_le_bytes()), ); - for fill in 0..128u8 { - add_record(&mut source, fill); - } - clear(&mut source, 0, 0); + add_record(&mut source, 0x11); + add_record(&mut source, 0x22); let state = source.save().unwrap(); let state = SavedStateBlob::new(state) .parse::() .unwrap(); let mut restored = restore_target(1100, state); - assert_eq!(restored.sel_len(), 128); + assert_eq!(restored.sel_len(), 2); + assert_eq!(restored.sel_record(0).unwrap()[7], 0x11); + assert_eq!(restored.sel_record(1).unwrap()[7], 0x22); assert_eq!(restored.sel_time_offset_seconds(), 500); - assert_eq!(restored.stats(), SelStats::default()); let time = transact(&mut restored, &storage_request(COMMAND_GET_SEL_TIME, &[])); assert_eq!(u32::from_le_bytes(time[3..7].try_into().unwrap()), 1600); - assert_completion( - &add_record(&mut restored, 0xff), - STORAGE_REQUEST, - COMMAND_ADD_SEL_ENTRY, - COMPLETION_SEL_FULL, - ); + assert_eq!(add_record(&mut restored, 0x33)[3..5], [3, 0]); } fn assert_invalid_state(state: save_restore::SavedState) { @@ -817,58 +781,19 @@ fn malformed_saved_state_is_rejected() { state.status = 0x10; assert_invalid_state(state); - let mut state = valid.clone(); - state.status = STATUS_IBF; - assert_invalid_state(state); - let mut state = valid.clone(); state.request = vec![0; 65]; assert_invalid_state(state); - let mut state = valid.clone(); - state.response = vec![0; 65]; - assert_invalid_state(state); - - let mut state = valid.clone(); - state.response = vec![0]; - state.response_position = 2; - assert_invalid_state(state); - - let mut state = valid.clone(); - state.status = KCS_STATE_READ; - state.response.clear(); - assert_invalid_state(state); - - let mut state = valid.clone(); - state.write_end_pending = true; - assert_invalid_state(state); - let mut state = valid.clone(); state.sel_count = 1; assert_invalid_state(state); - let mut state = valid.clone(); - state.sel_records = (1..=129u16) - .map(|record_id| { - let mut record = vec![0; 16]; - record[0..2].copy_from_slice(&record_id.to_le_bytes()); - record - }) - .collect(); - state.sel_count = 129; - state.next_record_id = 130; - assert_invalid_state(state); - let mut state = valid.clone(); state.sel_records = vec![vec![0; 15]]; state.sel_count = 1; assert_invalid_state(state); - let mut state = valid.clone(); - state.sel_records = vec![vec![0; 16]]; - state.sel_count = 1; - assert_invalid_state(state); - let mut state = valid.clone(); let mut record = vec![0; 16]; record[0..2].copy_from_slice(&1u16.to_le_bytes()); @@ -877,17 +802,7 @@ fn malformed_saved_state_is_rejected() { state.next_record_id = 2; assert_invalid_state(state); - for next_record_id in [0, 0xffff] { - let mut state = valid.clone(); - state.next_record_id = next_record_id; - assert_invalid_state(state); - } - let mut state = valid; - let mut record = vec![0; 16]; - record[0..2].copy_from_slice(&1u16.to_le_bytes()); - state.sel_records = vec![record]; - state.sel_count = 1; - state.next_record_id = 1; + state.next_record_id = 0; assert_invalid_state(state); } diff --git a/vm/devices/chipset_resources/src/lib.rs b/vm/devices/chipset_resources/src/lib.rs index 76fd05a56b..cefbe8558f 100644 --- a/vm/devices/chipset_resources/src/lib.rs +++ b/vm/devices/chipset_resources/src/lib.rs @@ -54,19 +54,10 @@ pub mod ipmi_kcs { /// Size of the ARM64 KCS MMIO aperture. pub const IPMI_KCS_MMIO_REGION_SIZE_AARCH64: u64 = 0x1000; - /// Result of attempting to forward an IPMI SEL record. - #[derive(Debug, Clone, Copy, PartialEq, Eq)] - pub enum SendOutcome { - /// The sink accepted the record. - Accepted, - /// The sink dropped the record without blocking the virtual BMC. - Dropped, - } - /// Non-blocking sink for completed IPMI SEL records. pub trait SelEventSink: Send { - /// Attempts to forward a completed SEL record. - fn try_send(&mut self, record_id: u16, record: SelRecord) -> SendOutcome; + /// Attempts to forward a completed SEL record, returning whether it was accepted. + fn try_send(&mut self, record_id: u16, record: SelRecord) -> bool; } /// Resource kind for IPMI SEL event sinks. @@ -88,7 +79,10 @@ pub mod ipmi_kcs { pub struct IpmiKcsDeviceHandleX64 { /// Non-blocking sink for completed SEL records. pub event_sink: Resource, - /// Trusted wall-clock source used for SEL timestamps. + /// Wall-clock source used for Unix-epoch SEL timestamps. + /// + /// This resource supplies time to UEFI and is not dependent on a + /// guest-visible CMOS device. pub time_source: Resource, } @@ -101,7 +95,10 @@ pub mod ipmi_kcs { pub struct IpmiKcsDeviceHandleAArch64 { /// Non-blocking sink for completed SEL records. pub event_sink: Resource, - /// Trusted wall-clock source used for SEL timestamps. + /// Wall-clock source used for Unix-epoch SEL timestamps. + /// + /// This resource supplies time to UEFI and is not dependent on a + /// guest-visible CMOS device. pub time_source: Resource, } diff --git a/vm/devices/get/get_protocol/src/dps_json.rs b/vm/devices/get/get_protocol/src/dps_json.rs index dce6cb61e0..9a20c4a842 100644 --- a/vm/devices/get/get_protocol/src/dps_json.rs +++ b/vm/devices/get/get_protocol/src/dps_json.rs @@ -314,16 +314,6 @@ mod test { .unwrap(); } - #[test] - fn ipmi_defaults_false_and_parses_true() { - let default = serde_json::from_str::("{}").unwrap(); - assert!(!default.enable_ipmi); - - let enabled = - serde_json::from_str::(r#"{"EnableIpmi":true}"#).unwrap(); - assert!(enabled.enable_ipmi); - } - #[test] fn smoke_test_sample_with_vtl2settings() { serde_json::from_slice::(include_bytes!( diff --git a/vm/devices/get/get_protocol/src/lib.rs b/vm/devices/get/get_protocol/src/lib.rs index c14df417ac..4e90bdb34e 100644 --- a/vm/devices/get/get_protocol/src/lib.rs +++ b/vm/devices/get/get_protocol/src/lib.rs @@ -403,35 +403,6 @@ impl IpmiSelNotification { } } -#[cfg(test)] -mod ipmi_sel_tests { - use super::*; - - #[test] - fn notification_wire_layout() { - let record_id = 0x1234; - let record = [ - 0x34, 0x12, 0x02, 0x78, 0x56, 0x34, 0x12, 0x20, 0x00, 0x04, 0x01, 0x6f, 0xaa, 0xbb, - 0xcc, 0xdd, - ]; - let notification = IpmiSelNotification::new(record_id, record); - - let mut expected = vec![1, 1, 13, 0, 0x34, 0x12]; - expected.extend_from_slice(&record); - assert_eq!(notification.as_bytes(), expected); - - let (decoded, remaining) = - IpmiSelNotification::read_from_prefix(&expected).expect("valid notification"); - assert!(remaining.is_empty()); - assert_eq!( - decoded.message_header.message_id(), - HostNotifications::IPMI_SEL - ); - assert_eq!(decoded.record_id.get(), record_id); - assert_eq!(decoded.record, record); - } -} - pub const TRACE_MSG_MAX_SIZE: usize = 256; open_enum! { #[derive(IntoBytes, FromBytes, Immutable, KnownLayout)] diff --git a/vm/devices/get/get_resources/src/lib.rs b/vm/devices/get/get_resources/src/lib.rs index f9c84042b5..06b347f18e 100644 --- a/vm/devices/get/get_resources/src/lib.rs +++ b/vm/devices/get/get_resources/src/lib.rs @@ -81,7 +81,7 @@ pub mod ged { pub guest_request_recv: mesh::Receiver, /// Notification of firmware events. pub firmware_event_send: Option>, - /// Test observer for IPMI SEL notifications received from OpenHCL. + /// Optional Petri observer for IPMI SEL notifications already received over GET. pub ipmi_sel_event_send: Option>, /// Enable secure boot. pub secure_boot_enabled: bool, diff --git a/vm/devices/get/guest_emulation_transport/Cargo.toml b/vm/devices/get/guest_emulation_transport/Cargo.toml index 8ddf1d1c74..889df8347e 100644 --- a/vm/devices/get/guest_emulation_transport/Cargo.toml +++ b/vm/devices/get/guest_emulation_transport/Cargo.toml @@ -27,6 +27,7 @@ chipset_resources.workspace = true guid = { workspace = true, features = ["inspect"] } inspect.workspace = true inspect_counters.workspace = true +ipmi_protocol.workspace = true mesh.workspace = true pal_async.workspace = true tracing_helpers.workspace = true diff --git a/vm/devices/get/guest_emulation_transport/src/api.rs b/vm/devices/get/guest_emulation_transport/src/api.rs index f74235d91b..c8d52b4582 100644 --- a/vm/devices/get/guest_emulation_transport/src/api.rs +++ b/vm/devices/get/guest_emulation_transport/src/api.rs @@ -18,9 +18,6 @@ pub use get_protocol::ProtocolVersion; pub use get_protocol::SaveGuestVtl2StateFlags; pub use get_protocol::VmgsIoStatus; -/// A completed IPMI System Event Log record. -pub type IpmiSelRecord = [u8; get_protocol::IPMI_SEL_RECORD_SIZE]; - use guid::Guid; use mesh::MeshPayload; use std::time::Duration; diff --git a/vm/devices/get/guest_emulation_transport/src/client.rs b/vm/devices/get/guest_emulation_transport/src/client.rs index 68947878a3..f1eae0dec1 100644 --- a/vm/devices/get/guest_emulation_transport/src/client.rs +++ b/vm/devices/get/guest_emulation_transport/src/client.rs @@ -484,7 +484,7 @@ impl GuestEmulationTransportClient { /// Forwards a completed IPMI System Event Log record to the host. /// /// This function is non-blocking and does not wait for a host response. - pub fn ipmi_sel(&self, record_id: u16, record: crate::api::IpmiSelRecord) { + pub fn ipmi_sel(&self, record_id: u16, record: ipmi_protocol::SelRecord) { self.control.notify(msg::Msg::IpmiSel { record_id, record }); } diff --git a/vm/devices/get/guest_emulation_transport/src/resolver.rs b/vm/devices/get/guest_emulation_transport/src/resolver.rs index 6794fcbcc4..1f5a013a7c 100644 --- a/vm/devices/get/guest_emulation_transport/src/resolver.rs +++ b/vm/devices/get/guest_emulation_transport/src/resolver.rs @@ -7,7 +7,6 @@ use crate::GuestEmulationTransportClient; use chipset_resources::ipmi_kcs::IpmiSelEventSinkHandleKind; use chipset_resources::ipmi_kcs::ResolvedIpmiSelEventSink; use chipset_resources::ipmi_kcs::SelEventSink; -use chipset_resources::ipmi_kcs::SendOutcome; use get_protocol::IPMI_SEL_RECORD_SIZE; use std::convert::Infallible; use vm_resource::CanResolveTo; @@ -44,9 +43,9 @@ impl ResolveResource for GuestEmulationTranspor struct GetIpmiSelEventSink(GuestEmulationTransportClient); impl SelEventSink for GetIpmiSelEventSink { - fn try_send(&mut self, record_id: u16, record: [u8; IPMI_SEL_RECORD_SIZE]) -> SendOutcome { + fn try_send(&mut self, record_id: u16, record: [u8; IPMI_SEL_RECORD_SIZE]) -> bool { self.0.ipmi_sel(record_id, record); - SendOutcome::Accepted + true } } diff --git a/vm/loader/src/uefi/config.rs b/vm/loader/src/uefi/config.rs index f316141580..174804fab6 100644 --- a/vm/loader/src/uefi/config.rs +++ b/vm/loader/src/uefi/config.rs @@ -455,11 +455,6 @@ pub struct PcieBarApertureEntry { mod tests { use super::*; - #[test] - fn ipmi_enabled_is_bit_33() { - assert_eq!(Flags::new().with_ipmi_enabled(true).into_bits(), 1 << 33); - } - fn read(bytes: &[u8]) -> T where T: FromBytes + Immutable + KnownLayout, From 93720979267c8204e9bc76e9041828c8384692bf Mon Sep 17 00:00:00 2001 From: Ayush Arora Date: Sat, 19 Sep 2026 00:48:15 +0530 Subject: [PATCH 10/10] ipmi: address remaining review feedback --- Cargo.lock | 1 + Guide/src/SUMMARY.md | 1 + Guide/src/reference/emulated/ipmi_kcs.md | 54 ++++++++++++++++++++++++ vm/devices/chipset/ipmi_kcs/src/sel.rs | 4 ++ vm/devices/get/get_resources/Cargo.toml | 1 + vm/devices/get/get_resources/src/lib.rs | 2 +- 6 files changed, 62 insertions(+), 1 deletion(-) create mode 100644 Guide/src/reference/emulated/ipmi_kcs.md diff --git a/Cargo.lock b/Cargo.lock index 75fc5e1dfb..7c9faca41b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2954,6 +2954,7 @@ name = "get_resources" version = "0.0.0" dependencies = [ "inspect", + "ipmi_protocol", "mesh", "smbios_defs", "thiserror 2.0.16", diff --git a/Guide/src/SUMMARY.md b/Guide/src/SUMMARY.md index e1d8d7c0eb..ba5b206169 100644 --- a/Guide/src/SUMMARY.md +++ b/Guide/src/SUMMARY.md @@ -112,6 +112,7 @@ - [framebuffer]() - [input]() - [Emulated]() + - [IPMI KCS](./reference/emulated/ipmi_kcs.md) - [vTPM]() - [NVMe]() - [Overview](./reference/emulated/NVMe/overview.md) diff --git a/Guide/src/reference/emulated/ipmi_kcs.md b/Guide/src/reference/emulated/ipmi_kcs.md new file mode 100644 index 0000000000..1e6955a1dd --- /dev/null +++ b/Guide/src/reference/emulated/ipmi_kcs.md @@ -0,0 +1,54 @@ +# IPMI KCS + +OpenVMM provides a minimal virtual IPMI baseboard management controller (BMC) +using the Keyboard Controller Style (KCS) system interface. The device allows +UEFI guests to write and query a bounded System Event Log (SEL). + +The implementation is provided by the `ipmi_kcs` crate. Wire-level command and +record definitions are in the `ipmi_protocol` crate. + +## Configuration + +The device is created when the platform configuration enables IPMI. OpenHCL +accepts this setting only for UEFI guests; PCAT and Linux-direct boot modes are +not supported. + +The guest-visible register interface depends on the architecture: + +| Architecture | Interface | Registers | +| --- | --- | --- | +| x86-64 | Port I/O | Data at `0xCA2`; status/command at `0xCA3` | +| AArch64 | MMIO | Data at `0xEFFE7000`; status/command at `0xEFFE7004` | + +Only one-byte register accesses are supported. + +## Supported commands + +The virtual BMC implements the IPMI application `Get Device ID` command and +these SEL storage commands: + +- Get SEL Info +- Reserve SEL +- Get SEL Entry +- Add SEL Entry +- Clear SEL +- Get SEL Time +- Set SEL Time + +The SEL stores at most 128 records. Reservations are exposed for guest software +compatibility but are not enforced because the device has one serialized KCS +requestor and no independent SEL mutators. + +Completed SEL records are retained by the device and forwarded to the host on +a best-effort basis. Forwarding is limited to 256 records per trusted +wall-clock second; reaching that limit does not remove records from the SEL. + +## Servicing + +Saved state includes the KCS transaction, SEL records, record allocation state, +reservation identifier, and guest-selected SEL time offset. Diagnostic +forwarding counters and rate-limiter state are reset after restore. + +See the +[`ipmi_kcs` rustdoc](https://openvmm.dev/rustdoc/ipmi_kcs/index.html) for the +device API. diff --git a/vm/devices/chipset/ipmi_kcs/src/sel.rs b/vm/devices/chipset/ipmi_kcs/src/sel.rs index 776ab69025..490fc4d157 100644 --- a/vm/devices/chipset/ipmi_kcs/src/sel.rs +++ b/vm/devices/chipset/ipmi_kcs/src/sel.rs @@ -156,6 +156,9 @@ impl IpmiKcs { return invalid_length(out); }; + // Reservations are accepted for software compatibility but are not + // enforced because this virtual BMC has one serialized KCS requestor + // and no independent SEL mutators. let offset = usize::from(request.offset); if offset >= SEL_RECORD_SIZE { return completion(out, COMPLETION_PARAMETER_OUT_OF_RANGE); @@ -236,6 +239,7 @@ impl IpmiKcs { return invalid_length(out); }; + // See get_sel_entry for why request.reservation_id is not validated. if request.signature != CLEAR_SEL_SIGNATURE { return completion(out, COMPLETION_INVALID_DATA_FIELD); } diff --git a/vm/devices/get/get_resources/Cargo.toml b/vm/devices/get/get_resources/Cargo.toml index 4d91336a2b..12d0724dbe 100644 --- a/vm/devices/get/get_resources/Cargo.toml +++ b/vm/devices/get/get_resources/Cargo.toml @@ -7,6 +7,7 @@ edition.workspace = true rust-version.workspace = true [dependencies] +ipmi_protocol.workspace = true smbios_defs.workspace = true vm_resource.workspace = true diff --git a/vm/devices/get/get_resources/src/lib.rs b/vm/devices/get/get_resources/src/lib.rs index 06b347f18e..ea69ddbb77 100644 --- a/vm/devices/get/get_resources/src/lib.rs +++ b/vm/devices/get/get_resources/src/lib.rs @@ -113,7 +113,7 @@ pub mod ged { /// BMC-assigned SEL record identifier. pub record_id: u16, /// Completed SEL record. - pub record: [u8; 16], + pub record: ipmi_protocol::SelRecord, } /// The firmware and chipset configuration for the guest.