From e9d1b3a70f8a78fb216f297fba3d9bd2441b8be1 Mon Sep 17 00:00:00 2001 From: John Gallagher Date: Mon, 2 Oct 2023 09:15:32 -0700 Subject: [PATCH] [host-sp-comms] Add `KeySet` message and handler (#1544) --- lib/host-sp-messages/src/lib.rs | 58 +++++++ task/control-plane-agent/src/mgs_gimlet.rs | 12 +- task/host-sp-comms/src/main.rs | 176 ++++++++++++++++++--- 3 files changed, 218 insertions(+), 28 deletions(-) diff --git a/lib/host-sp-messages/src/lib.rs b/lib/host-sp-messages/src/lib.rs index 5e8c0abaa..3115297cf 100644 --- a/lib/host-sp-messages/src/lib.rs +++ b/lib/host-sp-messages/src/lib.rs @@ -111,6 +111,11 @@ pub enum HostToSp { GetInventoryData { index: u32, }, + // KeySet is followed by a binary data blob (the value to set the key to) + KeySet { + // We use a raw `u8` here for the same reason as in `KeyLookup` above. + key: u8, + }, } /// The order of these cases is critical! We are relying on hubpack's encoding @@ -162,6 +167,7 @@ pub enum SpToHost { result: InventoryDataResult, name: [u8; 32], }, + KeySetResult(KeySetResult), } #[derive(Debug, Clone, Copy, PartialEq, Eq, num_derive::FromPrimitive)] @@ -171,6 +177,10 @@ pub enum Key { InstallinatorImageId, /// Returns the max inventory size and version InventorySize, + /// `/etc/system` file content + EtcSystem, + /// `/kernel/drv/dtrace.conf` file content + DtraceConf, } #[derive( @@ -187,6 +197,20 @@ pub enum KeyLookupResult { MaxResponseLenTooShort, } +#[derive( + Debug, Clone, Copy, PartialEq, Eq, Deserialize, Serialize, SerializedSize, +)] +pub enum KeySetResult { + Ok, + /// We don't know the requested key. + InvalidKey, + /// Key is read-only. + ReadOnlyKey, + /// The data in the request is too long for the value associated with the + /// requested key. + DataTooLong, +} + /// Results for an inventory data request /// /// These **cannot be reordered**; the host and SP must agree on them. @@ -727,6 +751,7 @@ mod tests { }, ), (0x0f, HostToSp::GetInventoryData { index: 0 }), + (0x10, HostToSp::KeySet { key: 0 }), ] { let n = hubpack::serialize(&mut buf[..], &variant).unwrap(); assert!(n >= 1); @@ -772,6 +797,7 @@ mod tests { name: [0u8; 32], }, ), + (0x0c, SpToHost::KeySetResult(KeySetResult::Ok)), ] { let n = hubpack::serialize(&mut buf[..], &variant).unwrap(); assert!(n >= 1); @@ -779,6 +805,38 @@ mod tests { } } + #[test] + fn key_lookup_result_values() { + let mut buf = [0; KeyLookupResult::MAX_SIZE]; + + for (expected_cmd, variant) in [ + (0x0, KeyLookupResult::Ok), + (0x1, KeyLookupResult::InvalidKey), + (0x2, KeyLookupResult::NoValueForKey), + (0x3, KeyLookupResult::MaxResponseLenTooShort), + ] { + let n = hubpack::serialize(&mut buf[..], &variant).unwrap(); + assert!(n <= 1); + assert_eq!(expected_cmd, buf[0]); + } + } + + #[test] + fn key_set_result_values() { + let mut buf = [0; KeySetResult::MAX_SIZE]; + + for (expected_cmd, variant) in [ + (0x0, KeySetResult::Ok), + (0x1, KeySetResult::InvalidKey), + (0x2, KeySetResult::ReadOnlyKey), + (0x3, KeySetResult::DataTooLong), + ] { + let n = hubpack::serialize(&mut buf[..], &variant).unwrap(); + assert!(n <= 1); + assert_eq!(expected_cmd, buf[0]); + } + } + #[test] fn inventory_data_result_values() { let mut buf = [0; InventoryDataResult::MAX_SIZE]; diff --git a/task/control-plane-agent/src/mgs_gimlet.rs b/task/control-plane-agent/src/mgs_gimlet.rs index 4341ecfda..be5b70bb2 100644 --- a/task/control-plane-agent/src/mgs_gimlet.rs +++ b/task/control-plane-agent/src/mgs_gimlet.rs @@ -1026,11 +1026,13 @@ impl SpHandler for MgsHandler { .unwrap_lite(); Ok(()) } - Some(Key::Ping) | Some(Key::InventorySize) | None => { - Err(SpError::SetIpccKeyLookupValueFailed( - IpccKeyLookupValueError::InvalidKey, - )) - } + Some(Key::Ping) + | Some(Key::InventorySize) + | Some(Key::EtcSystem) + | Some(Key::DtraceConf) + | None => Err(SpError::SetIpccKeyLookupValueFailed( + IpccKeyLookupValueError::InvalidKey, + )), } } diff --git a/task/host-sp-comms/src/main.rs b/task/host-sp-comms/src/main.rs index c8548721b..f6b03865a 100644 --- a/task/host-sp-comms/src/main.rs +++ b/task/host-sp-comms/src/main.rs @@ -15,9 +15,11 @@ use drv_usart::Usart; use enum_map::Enum; use heapless::Vec; use host_sp_messages::{ - Bsu, DecodeFailureReason, Header, HostToSp, Key, KeyLookupResult, SpToHost, - Status, MAX_MESSAGE_SIZE, MIN_SP_TO_HOST_FILL_DATA_LEN, + Bsu, DecodeFailureReason, Header, HostToSp, Key, KeyLookupResult, + KeySetResult, SpToHost, Status, MAX_MESSAGE_SIZE, + MIN_SP_TO_HOST_FILL_DATA_LEN, }; +use hubpack::SerializedSize; use idol_runtime::{NotificationHandler, RequestError}; use multitimer::{Multitimer, Repeat}; use mutable_statics::mutable_statics; @@ -166,6 +168,76 @@ enum RebootState { WaitingInA2RebootDelay, } +const MAX_ETC_SYSTEM_LEN: usize = 256; +const MAX_DTRACE_CONF_LEN: usize = 4096; + +// Storage we set aside for any messages where the host wants us to remember +// data for later read back (either by the host itself or by the control plane +// via MGS). +struct HostKeyValueStorage { + last_boot_fail: &'static mut [u8; MAX_HOST_FAIL_MESSAGE_LEN], + last_panic: &'static mut [u8; MAX_HOST_FAIL_MESSAGE_LEN], + etc_system: &'static mut [u8; MAX_ETC_SYSTEM_LEN], + etc_system_len: usize, + dtrace_conf: &'static mut [u8; MAX_DTRACE_CONF_LEN], + dtrace_conf_len: usize, +} + +impl HostKeyValueStorage { + fn claim_static_resources() -> Self { + let (last_boot_fail, last_panic, etc_system, dtrace_conf) = mutable_statics! { + static mut LAST_HOST_BOOT_FAIL: [u8; MAX_HOST_FAIL_MESSAGE_LEN] = + [|| 0; _]; + static mut LAST_HOST_PANIC: [u8; MAX_HOST_FAIL_MESSAGE_LEN] = + [|| 0; _]; + static mut HOST_ETC_SYSTEM: [u8; MAX_ETC_SYSTEM_LEN] = + [|| 0; _]; + static mut HOST_DTRACE_CONF: [u8; MAX_DTRACE_CONF_LEN] = + [|| 0; _]; + }; + + Self { + last_boot_fail, + last_panic, + etc_system, + etc_system_len: 0, + dtrace_conf, + dtrace_conf_len: 0, + } + } + + fn key_set(&mut self, key: u8, data: &[u8]) -> KeySetResult { + let Some(key) = Key::from_u8(key) else { + return KeySetResult::InvalidKey; + }; + + let (buf, buf_len) = match key { + // Some keys should not be set by the host: + // + // * `Ping` always returns PONG + // * InstallinatorImageId is set via MGS + // * InventorySize always returns our static inventory size + Key::Ping | Key::InstallinatorImageId | Key::InventorySize => { + return KeySetResult::ReadOnlyKey; + } + Key::EtcSystem => { + (self.etc_system.as_mut_slice(), &mut self.etc_system_len) + } + Key::DtraceConf => { + (self.dtrace_conf.as_mut_slice(), &mut self.dtrace_conf_len) + } + }; + + if data.len() > buf.len() { + KeySetResult::DataTooLong + } else { + buf[..data.len()].copy_from_slice(data); + *buf_len = data.len(); + KeySetResult::Ok + } + } +} + struct ServerImpl { uart: Usart, sys: sys_api::Sys, @@ -179,9 +251,7 @@ struct ServerImpl { cp_agent: ControlPlaneAgent, packrat: Packrat, reboot_state: Option, - - last_host_boot_fail: &'static mut [u8; MAX_HOST_FAIL_MESSAGE_LEN], - last_host_panic: &'static mut [u8; MAX_HOST_FAIL_MESSAGE_LEN], + host_kv_storage: HostKeyValueStorage, } impl ServerImpl { @@ -197,13 +267,6 @@ impl ServerImpl { Some(Repeat::AfterWake(UART_ZERO_DELAY)), ); - let (last_host_boot_fail, last_host_panic) = mutable_statics! { - static mut LAST_HOST_BOOT_FAIL: [u8; MAX_HOST_FAIL_MESSAGE_LEN] = - [|| 0; _]; - static mut LAST_HOST_PANIC: [u8; MAX_HOST_FAIL_MESSAGE_LEN] = - [|| 0; _]; - }; - Self { uart, sys, @@ -219,8 +282,7 @@ impl ServerImpl { ), packrat: Packrat::from(PACKRAT.get_task_id()), reboot_state: None, - last_host_boot_fail, - last_host_panic, + host_kv_storage: HostKeyValueStorage::claim_static_resources(), } } @@ -681,9 +743,13 @@ impl ServerImpl { // // For now, copy it into a static var we can pull out via // `humility readvar LAST_HOST_BOOT_FAIL`. - let n = usize::min(data.len(), self.last_host_boot_fail.len()); - self.last_host_boot_fail[..n].copy_from_slice(&data[..n]); - for b in &mut self.last_host_boot_fail[n..] { + let n = usize::min( + data.len(), + self.host_kv_storage.last_boot_fail.len(), + ); + self.host_kv_storage.last_boot_fail[..n] + .copy_from_slice(&data[..n]); + for b in &mut self.host_kv_storage.last_boot_fail[n..] { *b = 0; } Some(SpToHost::Ack) @@ -693,9 +759,13 @@ impl ServerImpl { // // For now, copy it into a static var we can pull out via // `humility readvar LAST_HOST_PANIC`. - let n = usize::min(data.len(), self.last_host_panic.len()); - self.last_host_panic[..n].copy_from_slice(&data[..n]); - for b in &mut self.last_host_panic[n..] { + let n = usize::min( + data.len(), + self.host_kv_storage.last_panic.len(), + ); + self.host_kv_storage.last_panic[..n] + .copy_from_slice(&data[..n]); + for b in &mut self.host_kv_storage.last_panic[n..] { *b = 0; } Some(SpToHost::Ack) @@ -758,6 +828,9 @@ impl ServerImpl { } Err(err) => Some(SpToHost::KeyLookupResult(err)), }, + HostToSp::KeySet { key } => Some(SpToHost::KeySetResult( + self.host_kv_storage.key_set(key, data), + )), HostToSp::GetInventoryData { index } => { match self.perform_inventory_lookup(header.sequence, index) { Ok(()) => None, @@ -890,13 +963,70 @@ impl ServerImpl { ); response_len } + Key::EtcSystem => { + let response_len = self.host_kv_storage.etc_system_len; + if response_len == 0 { + return Err(KeyLookupResult::NoValueForKey); + } + + self.tx_buf.encode_response( + sequence, + &SpToHost::KeyLookupResult(KeyLookupResult::Ok), + |buf| { + // Statically guarantee we have sufficient space in + // `buf` for longest possible ETC_SYSTEM blob. + const_assert!( + MIN_SP_TO_HOST_FILL_DATA_LEN >= MAX_ETC_SYSTEM_LEN + ); + buf[..response_len].copy_from_slice( + &self.host_kv_storage.etc_system[..response_len], + ); + response_len + }, + ); + response_len + } + Key::DtraceConf => { + let response_len = self.host_kv_storage.dtrace_conf_len; + if response_len == 0 { + return Err(KeyLookupResult::NoValueForKey); + } + + self.tx_buf.encode_response( + sequence, + &SpToHost::KeyLookupResult(KeyLookupResult::Ok), + |buf| { + // `MIN_SP_TO_HOST_FILL_DATA_LEN` is calculated assuming + // `SpToHost::MAX_SIZE`, but we know in this callback + // we're appending to + // `SpToHost::KeyLookupResult(KeyLookupResult::Ok)`, + // which is only 2 bytes. Recompute the exact max space + // we have for our response, then statically guarantee + // we have sufficient space in `buf` for longest + // possible DTRACE_CONF blob. + const SP_TO_HOST_FILL_DATA_LEN: usize = + MIN_SP_TO_HOST_FILL_DATA_LEN + SpToHost::MAX_SIZE + - 2; + const_assert!( + SP_TO_HOST_FILL_DATA_LEN >= MAX_DTRACE_CONF_LEN + ); + + buf[..response_len].copy_from_slice( + &self.host_kv_storage.dtrace_conf[..response_len], + ); + response_len + }, + ); + response_len + } }; + if response_len > max_response_len { self.tx_buf.reset(); - return Err(KeyLookupResult::MaxResponseLenTooShort); + Err(KeyLookupResult::MaxResponseLenTooShort) + } else { + Ok(()) } - - Ok(()) } }