From 3334f62cfcaaa4e2d36d1aacb58bc831649ab99f Mon Sep 17 00:00:00 2001 From: "Rivas Paz, Jose L" Date: Wed, 26 Aug 2026 08:14:40 +0200 Subject: [PATCH] lib: introduce errors for control commands This patch introduces custom errors when the control commands are executed. The logic is using the return codes from the PMT linux driver to indicate why the Crash Log control commands failed and include that information in the Crash Log tool debug log. There is a change made on how a PMT device executes a control command, which is relevant when a PMT device comprehends more than one endpoint: - The control command will be executed in all endpoints, independently of their return values - If at least one endpoint returns `Ok()` the control command of that PMT device will return `Ok()` as well - If all the endpoints returned `Err()` the error of the first failed endpoint will be returned as the `Err()` of the control command Signed-off-by: Rivas Paz, Jose L --- lib/src/error.rs | 6 ++++ lib/src/source/pmt.rs | 59 +++++++++++++++++++++++++++---------- lib/src/source/pmt/sysfs.rs | 17 +++++++++-- 3 files changed, 63 insertions(+), 19 deletions(-) diff --git a/lib/src/error.rs b/lib/src/error.rs index b1aee02..cdb09a2 100644 --- a/lib/src/error.rs +++ b/lib/src/error.rs @@ -39,6 +39,9 @@ pub enum Error { IOError(io::Error), #[cfg(feature = "std")] OsStringError(std::ffi::OsString), + ControlCommandRejected, + CrashLogPending, + SourceBusy, } #[cfg(feature = "std")] @@ -74,6 +77,9 @@ impl fmt::Display for Error { Error::IOError(err) => write!(f, "Encountered IO error: {err}"), #[cfg(feature = "std")] Error::OsStringError(s) => write!(f, "Cannot convert OS string: {s:?}"), + Error::ControlCommandRejected => write!(f, "Control command was rejected"), + Error::CrashLogPending => write!(f, "A Crash Log collection is already pending"), + Error::SourceBusy => write!(f, "Crash Log source is busy"), } } } diff --git a/lib/src/source/pmt.rs b/lib/src/source/pmt.rs index cc4ea9f..8b9ecff 100644 --- a/lib/src/source/pmt.rs +++ b/lib/src/source/pmt.rs @@ -16,6 +16,8 @@ use alloc::{ use std::{collections::BTreeSet, fmt, str::FromStr}; #[cfg(all(target_os = "linux", feature = "std"))] use sysfs::PmtSysFs; +#[cfg(all(target_os = "linux", feature = "control_commands"))] +use sysfs::PmtSysFsEndpoint; pub use bdf::PciBdf; @@ -87,10 +89,7 @@ impl Pmt { #[cfg(all(target_os = "linux", feature = "control_commands"))] pub fn enable_disable(&self, dev: &PmtDeviceId, enable: bool) -> Result<(), Error> { - for endpoint in self.sysfs.get_endpoints(dev) { - endpoint.enable_disable(enable)?; - } - Ok(()) + self.run_on_endpoints(dev, |endpoint| endpoint.enable_disable(enable)) } #[cfg(all(not(target_os = "linux"), feature = "control_commands"))] @@ -100,10 +99,7 @@ impl Pmt { #[cfg(all(target_os = "linux", feature = "control_commands"))] pub fn clear(&self, dev: &PmtDeviceId) -> Result<(), Error> { - for endpoint in self.sysfs.get_endpoints(dev) { - endpoint.clear()?; - } - Ok(()) + self.run_on_endpoints(dev, |endpoint| endpoint.clear()) } #[cfg(all(not(target_os = "linux"), feature = "control_commands"))] @@ -113,10 +109,7 @@ impl Pmt { #[cfg(all(target_os = "linux", feature = "control_commands"))] pub fn rearm(&self, dev: &PmtDeviceId) -> Result<(), Error> { - for endpoint in self.sysfs.get_endpoints(dev) { - endpoint.rearm()?; - } - Ok(()) + self.run_on_endpoints(dev, |endpoint| endpoint.rearm()) } #[cfg(all(not(target_os = "linux"), feature = "control_commands"))] @@ -126,10 +119,7 @@ impl Pmt { #[cfg(all(target_os = "linux", feature = "control_commands"))] pub fn trigger(&self, dev: &PmtDeviceId) -> Result<(), Error> { - for endpoint in self.sysfs.get_endpoints(dev) { - endpoint.trigger()?; - } - Ok(()) + self.run_on_endpoints(dev, |endpoint| endpoint.trigger()) } #[cfg(all(not(target_os = "linux"), feature = "control_commands"))] @@ -137,6 +127,43 @@ impl Pmt { Err(Error::Unsupported) } + #[cfg(all(target_os = "linux", feature = "control_commands"))] + fn run_on_endpoints(&self, dev: &PmtDeviceId, command: F) -> Result<(), Error> + where + F: Fn(&PmtSysFsEndpoint) -> Result<(), Error>, + { + let endpoints = self.sysfs.get_endpoints(dev); + if endpoints.is_empty() { + return Err(Error::NoCrashLogSourceFound); + } + + let mut cmd_success = false; + let mut first_error = None; + + for endpoint in &endpoints { + match command(endpoint) { + Ok(()) => cmd_success = true, + Err(err) => { + if first_error.is_none() { + first_error = Some(err); + } + } + } + } + + if !cmd_success { + // Unreachable `unwrap_or`: `endpoints` is not empty and every failing endpoint + // records an error, so `first_error` is always set when no endpoint succeeded. + return Err(first_error.unwrap_or(Error::InternalError)); + } + + if let Some(err) = first_error { + log::warn!("Crash Log command failed on some endpoints: {err}"); + } + + Ok(()) + } + pub fn description(&self, dev: &PmtDeviceId) -> String { match dev { PmtDeviceId::Name(name) => format!("PMT endpoint ({name})"), diff --git a/lib/src/source/pmt/sysfs.rs b/lib/src/source/pmt/sysfs.rs index 3dceff7..3c77372 100644 --- a/lib/src/source/pmt/sysfs.rs +++ b/lib/src/source/pmt/sysfs.rs @@ -272,15 +272,26 @@ impl PmtSysFsEndpoint { .truncate(true) .open(&path) .map_err(Error::IOError) - .inspect_err(|err| log::warn!("{}: {err}", path.display()))?; + .inspect_err(|err| log::info!("Failed to open {}: {err}", path.display()))?; file.write_all(value) - .map_err(Error::IOError) - .inspect_err(|err| log::warn!("Failed to write to {}: {err}", path.display()))?; + .map_err(Self::to_control_error) + .inspect_err(|err| log::info!("Failed to write to {}: {err}", path.display()))?; Ok(()) } + #[cfg(feature = "control_commands")] + fn to_control_error(err: std::io::Error) -> Error { + log::debug!("Error reported by the PmtSysFs: {err}"); + match err.kind() { + std::io::ErrorKind::AlreadyExists => Error::CrashLogPending, + std::io::ErrorKind::ResourceBusy => Error::SourceBusy, + std::io::ErrorKind::InvalidInput => Error::ControlCommandRejected, + _ => Error::IOError(err), + } + } + pub fn capabilities(&self) -> Capabilities { let mut capabilities: Capabilities = Capabilities::new();