diff --git a/Cargo.lock b/Cargo.lock index ece9f7bd9..8037a27ca 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3154,7 +3154,7 @@ dependencies = [ [[package]] name = "gateway-messages" version = "0.1.0" -source = "git+https://github.com/oxidecomputer/management-gateway-service#745a508cb97b7ca9b4c10ec9592c980eb769b10d" +source = "git+https://github.com/oxidecomputer/management-gateway-service?branch=james%2Fpmbus-please#437f02bcfbf6064d94e34c88774a641c7b467b4f" dependencies = [ "bitflags 2.9.4", "hubpack", @@ -5654,6 +5654,7 @@ name = "task-control-plane-agent" version = "0.1.0" dependencies = [ "anyhow", + "build-i2c", "build-util", "cfg-if", "counters", @@ -5662,6 +5663,8 @@ dependencies = [ "drv-caboose-pos", "drv-cpu-seq-api", "drv-hf-api", + "drv-i2c-api", + "drv-i2c-devices", "drv-ignition-api", "drv-lpc55-update-api", "drv-monorail-api", diff --git a/Cargo.toml b/Cargo.toml index 5c5d47e35..1beb5096a 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -152,7 +152,8 @@ apob = { git = "https://github.com/oxidecomputer/apob", default-features = false # for the migration. attest-data = { git = "https://github.com/oxidecomputer/dice-util", default-features = false, version = "0.4.0", rev = "a0811d06c75c757a6e12c91ed6ea81fde137ba43" } dice-mfg-msgs = { git = "https://github.com/oxidecomputer/dice-util", default-features = false, version = "0.2.1", rev = "a0811d06c75c757a6e12c91ed6ea81fde137ba43" } -gateway-messages = { git = "https://github.com/oxidecomputer/management-gateway-service", default-features = false, features = ["smoltcp"] } +# gateway-messages = { git = "https://github.com/oxidecomputer/management-gateway-service", default-features = false, features = ["smoltcp"] } +gateway-messages = { git = "https://github.com/oxidecomputer/management-gateway-service", branch = "james/pmbus-please", default-features = false, features = ["smoltcp"] } gateway-ereport-messages = { git = "https://github.com/oxidecomputer/management-gateway-service", default-features = false } gimlet-inspector-protocol = { git = "https://github.com/oxidecomputer/gimlet-inspector-protocol", version = "0.1.0" } hif = { git = "https://github.com/oxidecomputer/hif", default-features = false } diff --git a/build/i2c/src/lib.rs b/build/i2c/src/lib.rs index 4305f8ccf..bd35c5484 100644 --- a/build/i2c/src/lib.rs +++ b/build/i2c/src/lib.rs @@ -1381,6 +1381,8 @@ impl ConfigGenerator { all.sort(); for (rail, (device, index)) in &all { + // if we update this code to be more clever than just to-lowercase'ing the rail names, + // you might need to go update the mapping in `control-plane-agent`! write!( &mut self.output, r##" @@ -1776,6 +1778,7 @@ pub fn codegen(settings: impl Into) -> Result<()> { Ok(()) } +#[derive(Debug, Clone)] pub struct I2cDeviceDescription { pub device: String, pub description: String, diff --git a/drv/i2c-devices/src/lib.rs b/drv/i2c-devices/src/lib.rs index d94c9e219..ebfa3fd82 100644 --- a/drv/i2c-devices/src/lib.rs +++ b/drv/i2c-devices/src/lib.rs @@ -66,24 +66,31 @@ macro_rules! pmbus_read { } macro_rules! pmbus_rail_read { - ($device:expr, $rail:expr, $cmd:ident) => { + (@raw => $device:expr, $rail:expr, $cmd_code:expr, $len:expr $(,)?) => { $device - .write_read_reg::( - $cmd::CommandData::code(), + .write_read_reg::( + $cmd_code, &[PAGE::CommandData::code(), $rail], ) .map_err(|code| Error::BadRead { - cmd: $cmd::CommandData::code(), + cmd: $cmd_code, code, }) + }; + + ($device:expr, $rail:expr, $cmd:ident $(,)?) => {{ + let cmd_code = $cmd::CommandData::code(); + const CMD_LEN: usize = $cmd::CommandData::len(); + + pmbus_rail_read!(@raw => $device, $rail, cmd_code, CMD_LEN) .and_then(|rval| { $cmd::CommandData::from_slice(&rval).ok_or(Error::BadData { cmd: $cmd::CommandData::code(), }) }) - }; + }}; - ($device:expr, $rail:expr, $dev:ident::$cmd:ident) => {{ + ($device:expr, $rail:expr, $dev:ident::$cmd:ident $(,)?) => {{ use $dev::{PAGE, $cmd}; pmbus_rail_read!($device, $rail, $cmd) }}; @@ -271,6 +278,69 @@ pub trait Validate> { } } +// grumble grumble, copied from `gateway_messages::sp_to_mgs::PmbusStatus` +// grumble grumble, also basically the same as `ereports/src/pwr` +pub struct PmbusStatus { + pub status_word: u16, + pub status_vout: Result, + pub status_iout: Result, + pub status_temperature: Result, + pub status_cml: Result, + pub status_other: Result, + pub status_input: Result, + pub status_mfr_specific: Result, + pub status_fans_1_2: Result, + pub status_fans_3_4: Result, +} + +/// An error for querying PMBus `STATUS_*` registers. +#[derive(Copy, Clone, Debug, PartialEq)] +pub enum PmbusStatusError { + BadRead { cmd: u8, code: ResponseCode }, + BadData { cmd: u8, }, +} + +impl PmbusStatus { + /// Attempt to read a [`PmbusStatus`] from the given device and rail. + /// + /// This function returns an error if the initial attempt to obtain `STATUS_WORD` from the + /// device fails, otherwise returnining successfully even if "leaf" status bytes were unable + /// to be read, either due to ephemeral hiccups, or that status byte being unsupported by + /// the device queried. + pub fn try_read_from(dev: &I2cDevice, rail_idx: u8) -> Result { + use pmbus::commands::*; + use PmbusStatusError as Error; + + Ok(PmbusStatus { + // Status word *must* succeed, otherwise we don't have reasonable data to return. + // We may want to consider making some/all of these retryable, but for now you either + // get them or you don't. + status_word: pmbus_rail_read!(dev, rail_idx, STATUS_WORD)?.0, + status_vout: pmbus_rail_read!(dev, rail_idx, STATUS_VOUT).map(|v| v.0), + status_iout: pmbus_rail_read!(dev, rail_idx, STATUS_IOUT).map(|v| v.0), + status_temperature: pmbus_rail_read!(dev, rail_idx, STATUS_TEMPERATURE).map(|v| v.0), + status_cml: pmbus_rail_read!(dev, rail_idx, STATUS_CML).map(|v| v.0), + status_other: pmbus_rail_read!(dev, rail_idx, STATUS_OTHER).map(|v| v.0), + status_input: pmbus_rail_read!(dev, rail_idx, STATUS_INPUT).map(|v| v.0), + status_fans_1_2: pmbus_rail_read!(dev, rail_idx, STATUS_FANS_1_2).map(|v| v.0), + status_fans_3_4: pmbus_rail_read!(dev, rail_idx, STATUS_FANS_3_4).map(|v| v.0), + + // Unfortunately, STATUS_MFR_SPECIFIC *is* defined in the pmbus crate, but doesn't have a + // "structured" representation, so instead use a raw accessor. It *could* be argued + // that since we're not actually peeking at any of the introspection stuff, we could do + // the same for all the items above, and save ourselves a little indirection, but for now + // just hole-punch the minimum amount necessary. Indexing is fine here because we know + // (statically) the returned value is `[u8; 1]`. + status_mfr_specific: pmbus_rail_read!( + @raw => dev, + rail_idx, + CommandCode::STATUS_MFR_SPECIFIC as u8, + 1, + ).map(|v| v[0]), + }) + } +} + pub mod adm127x; pub mod adt7420; pub mod at24csw080; diff --git a/task/control-plane-agent/Cargo.toml b/task/control-plane-agent/Cargo.toml index 4ab7d0657..9c81559a9 100644 --- a/task/control-plane-agent/Cargo.toml +++ b/task/control-plane-agent/Cargo.toml @@ -25,6 +25,8 @@ drv-caboose = { path = "../../drv/caboose" } drv-caboose-pos = { path = "../../drv/caboose-pos" } drv-hf-api = { path = "../../drv/hf-api", optional = true } drv-cpu-seq-api = { path = "../../drv/cpu-seq-api", optional = true } +drv-i2c-api = { path = "../../drv/i2c-api" } +drv-i2c-devices = { path = "../../drv/i2c-devices" } drv-ignition-api = { path = "../../drv/ignition-api", optional = true } drv-lpc55-update-api = { path = "../../drv/lpc55-update-api" } drv-monorail-api = { path = "../../drv/monorail-api", optional = true } @@ -53,6 +55,7 @@ userlib = { path = "../../sys/userlib", features = ["panic-messages"] } static-cell = { path = "../../lib/static-cell" } [build-dependencies] +build-i2c = { path = "../../build/i2c" } build-util = { path = "../../build/util" } anyhow = { workspace = true } idol = { workspace = true } diff --git a/task/control-plane-agent/build.rs b/task/control-plane-agent/build.rs index cab8cd7b2..6aaf0c7c6 100644 --- a/task/control-plane-agent/build.rs +++ b/task/control-plane-agent/build.rs @@ -35,6 +35,64 @@ fn main() -> Result<(), Box> { if let Some(cfg) = cfg { write_keys(cfg)?; } + + do_pmbus()?; + + Ok(()) +} + +fn do_pmbus() -> Result<(), Box> { + let out_dir = std::env::var("OUT_DIR")?; + let dest_path = + std::path::Path::new(&out_dir).join("pmbus_mapping.rs"); + let file = std::fs::File::create(&dest_path)?; + let mut file = std::io::BufWriter::new(file); + + // Generate the necessary rail names + if let Err(e) = build_i2c::codegen(build_i2c::Disposition::Devices) { + println!("cargo::error=failed to generate I2C devices: {e}"); + std::process::exit(1); + } + + let devices = build_i2c::device_descriptions() + .collect::>(); + + let mut pmbus_rail_names = std::collections::BTreeSet::new(); + let mut pmbus_rail_dupes = 0; + for dev in devices { + // We only need to map PMBus devices + let Some(ref pmbus) = dev.pmbus else { + continue; + }; + + // pmbus devices must have a refdes + assert!(dev.device_id.is_some()); + + // Aggregate a list of all PMBus-visible rails + for rail in pmbus.rails.iter() { + // `BTreeSet::insert` return value means "is unique", which is the inverse of + // `BTreeMap::insert().is_some()`! + if !pmbus_rail_names.insert(rail.name.clone()) { + pmbus_rail_dupes += 1; + println!("cargo::error=Duplicate Rail name: {:?}", rail.name); + } + } + } + + // Create a mapping between rail names and generated accessor functions for obtaining + // the device handle and rail index + writeln!(file)?; + writeln!(file, "pub const PMBUS_RAIL_TO_I2C_DEVICE_MAP: [(&[u8], fn(userlib::TaskId) -> (drv_i2c_api::I2cDevice, u8)); {}] = [", pmbus_rail_names.len())?; + for rail in pmbus_rail_names.iter() { + write!(file, " (b\"{rail}\", ")?; + // build_i2c *also* only to-lowercases the rail names to make functions + write!(file, "crate::i2c_config::pmbus::{}", rail.to_lowercase())?; + writeln!(file, "),")?; + } + writeln!(file, "];")?; + + assert_eq!(pmbus_rail_dupes, 0); + Ok(()) } diff --git a/task/control-plane-agent/src/inventory.rs b/task/control-plane-agent/src/inventory.rs index 1ccc860bd..6ca19ee07 100644 --- a/task/control-plane-agent/src/inventory.rs +++ b/task/control-plane-agent/src/inventory.rs @@ -130,7 +130,7 @@ impl Inventory { capabilities |= DeviceCapabilities::HAS_MEASUREMENT_CHANNELS; } DeviceDescription { - component: SpComponent { id: device.id }, + component: todo!("AJM"), // SpComponent { id: device.id } device: device.device, description: device.description, capabilities, @@ -179,7 +179,7 @@ impl TryFrom<&'_ SpComponent> for Index { fn try_from(component: &'_ SpComponent) -> Result { if let Ok(entry_idx) = task_validate_api::DEVICE_INDICES_BY_SORTED_ID - .binary_search_by_key(&component.id, |&(id, _)| id) + .binary_search_by_key(component.id(), |&(id, _)| id) { let &(_, index) = task_validate_api::DEVICE_INDICES_BY_SORTED_ID .get(entry_idx) diff --git a/task/control-plane-agent/src/main.rs b/task/control-plane-agent/src/main.rs index 7a30270fc..ce3b2629f 100644 --- a/task/control-plane-agent/src/main.rs +++ b/task/control-plane-agent/src/main.rs @@ -51,6 +51,7 @@ mod ignition_controller; task_slot!(JEFE, jefe); task_slot!(NET, net); task_slot!(SYS, sys); +task_slot!(I2C, i2c_driver); #[allow(dead_code)] // Not all cases are used by all variants #[derive(Clone, Copy, PartialEq, ringbuf::Count)] @@ -627,3 +628,7 @@ mod idl { } include!(concat!(env!("OUT_DIR"), "/notifications.rs")); + +include!(concat!(env!("OUT_DIR"), "/i2c_config.rs")); + +include!(concat!(env!("OUT_DIR"), "/pmbus_mapping.rs")); diff --git a/task/control-plane-agent/src/mgs_compute_sled.rs b/task/control-plane-agent/src/mgs_compute_sled.rs index 503402e01..82a836283 100644 --- a/task/control-plane-agent/src/mgs_compute_sled.rs +++ b/task/control-plane-agent/src/mgs_compute_sled.rs @@ -22,7 +22,7 @@ use gateway_messages::{ PostCode, PowerState, PowerStateTransition, RotBootInfo, RotRequest, RotResponse, SERIAL_CONSOLE_IDLE_TIMEOUT, SensorRequest, SensorResponse, SpComponent, SpError, SpPort as GwSpPort, SpRequest, SpStateV2, - SpUpdatePrepare, UpdateChunk, UpdateId, UpdateStatus, ignition, + SpUpdatePrepare, UpdateChunk, UpdateId, UpdateStatus, ignition, PowerRailName, PmbusStatus, PmbusStatusError }; use heapless::{Deque, Vec}; use host_sp_messages::HostStartupOptions; @@ -1259,6 +1259,48 @@ impl SpHandler for MgsHandler { })); self.host_flash_update.get_hash(slot) } + + fn get_pmbus_status(&mut self, rail: &PowerRailName) + -> Result, SpError> + { + let Some(name) = rail.as_str() else { + panic!(); + }; + // this is silly, I'm mostly just trimming trailing nulls, I should add a method for this + let name = name.as_bytes(); + + // TODO: Define error for "unknown rail" + let idx = crate::PMBUS_RAIL_TO_I2C_DEVICE_MAP.binary_search_by_key(&name, |row| row.0).unwrap(); + let (_name, func) = &crate::PMBUS_RAIL_TO_I2C_DEVICE_MAP[idx]; + let (device, rail) = func(crate::I2C.get_task_id()); + + // TODO: Error for read failed? + let res = drv_i2c_devices::PmbusStatus::try_read_from(&device, rail); + + // TODO: Define mgs::PmbusStatusError variants for errors found in drv_i2c_devices::PmbusStatusError + fn err_fixer(_val: drv_i2c_devices::PmbusStatusError) -> PmbusStatusError { + todo!() + } + + match res { + Ok(status) => Ok(Ok(PmbusStatus { + status_word: status.status_word, + status_vout: status.status_vout.map_err(err_fixer), + status_iout: status.status_iout.map_err(err_fixer), + status_temperature: status.status_temperature.map_err(err_fixer), + status_cml: status.status_cml.map_err(err_fixer), + status_other: status.status_other.map_err(err_fixer), + status_input: status.status_input.map_err(err_fixer), + status_mfr_specific: status.status_mfr_specific.map_err(err_fixer), + status_fans_1_2: status.status_fans_1_2.map_err(err_fixer), + status_fans_3_4: status.status_fans_3_4.map_err(err_fixer), + })), + + // TODO: Should this just become an arm of `SpError`? Is it still worth keeping + // the request? + Err(e) => Ok(Err(err_fixer(e))), + } + } } struct UsartHandler { diff --git a/task/control-plane-agent/src/update/rot.rs b/task/control-plane-agent/src/update/rot.rs index 302bc395a..f5aa91b9f 100644 --- a/task/control-plane-agent/src/update/rot.rs +++ b/task/control-plane-agent/src/update/rot.rs @@ -78,7 +78,7 @@ impl ComponentUpdater for RotUpdate { .map_err(SpError::OtherComponentUpdateInProgress)?; // Which target are we updating? - ringbuf_entry!(Trace::Target(update.component.id[0], update.slot)); + ringbuf_entry!(Trace::Target(update.component.id()[0], update.slot)); let target = match (update.component, update.slot) { (SpComponent::ROT, 0) => UpdateTarget::ImageA, (SpComponent::ROT, 1) => UpdateTarget::ImageB, diff --git a/task/validate-api/build.rs b/task/validate-api/build.rs index 4f0d3c8b4..c06623dd3 100644 --- a/task/validate-api/build.rs +++ b/task/validate-api/build.rs @@ -65,8 +65,8 @@ fn write_pub_device_descriptions() -> anyhow::Result<()> { writeln!(file, " description: {:?},", dev.description)?; if let Some(id) = dev.device_id { if let Ok(component) = SpComponent::try_from(id.as_ref()) { - writeln!(file, " id: {:?},", component.id)?; - if id2idx.insert(component.id, idx).is_some() { + writeln!(file, " id: {:?},", component.id())?; + if id2idx.insert(*component.id(), idx).is_some() { println!("cargo::error=duplicate device id {id:?}",); duplicate_ids += 1; }