diff --git a/.git_components.yaml b/.git_components.yaml index cfc5dbc2..1bbff0da 100644 --- a/.git_components.yaml +++ b/.git_components.yaml @@ -40,3 +40,4 @@ tests: snapcraft: owners: - artiepoole + - talhaHavadar diff --git a/Cargo.lock b/Cargo.lock index e344796c..7561b3e4 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -279,11 +279,18 @@ version = "2.3.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "37909eebbb50d72f9059c3b6d82c0463f2ff062c9e95845c43a6c9c0355411be" +[[package]] +name = "fdt" +version = "0.1.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "784a4df722dc6267a04af36895398f59d21d07dce47232adf31ec0ff2fa45e67" + [[package]] name = "fpgad" -version = "0.1.0" +version = "0.2.0" dependencies = [ "env_logger", + "fdt", "fpgad_macros", "googletest", "log", @@ -295,7 +302,7 @@ dependencies = [ [[package]] name = "fpgad_cli" -version = "0.1.0" +version = "0.2.0" dependencies = [ "clap", "env_logger", @@ -306,7 +313,7 @@ dependencies = [ [[package]] name = "fpgad_macros" -version = "0.1.0" +version = "0.2.0" dependencies = [ "quote", "syn", diff --git a/Cargo.toml b/Cargo.toml index 988ea15d..27121448 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -3,7 +3,7 @@ resolver = "3" members = ["daemon", "cli", "fpgad_macros"] [workspace.package] -version = "0.1.0" +version = "0.2.0" edition = "2024" license = "GPL-3.0" homepage = "https://github.com/canonical/fpgad" @@ -11,5 +11,5 @@ repository = "https://github.com/canonical/fpgad" authors = ["Talha Can Havadar ", "Artie Poole "] [workspace.dependencies] -fpgad_macros = { version = "0.1.0", path = "fpgad_macros" } +fpgad_macros = { version = "0.2.0", path = "fpgad_macros" } diff --git a/cli/src/main.rs b/cli/src/main.rs index 025eb12b..a9cb0722 100644 --- a/cli/src/main.rs +++ b/cli/src/main.rs @@ -182,7 +182,8 @@ use std::error::Error; /// /// # Examples /// -/// ```bash +/// ```shell +/// /// # Load a bitstream /// fpgad load bitstream /lib/firmware/design.bit.bin /// @@ -191,6 +192,7 @@ use std::error::Error; /// /// # Load an overlay with a specific handle /// fpgad load overlay /lib/firmware/overlay.dtbo --handle=my_overlay +/// /// ``` #[derive(Parser, Debug)] #[command(name = "fpga")] @@ -252,7 +254,14 @@ enum RemoveSubcommand { handle: Option, }, /// Remove bitstream loaded in given `HANDLE` to fpga command - Bitstream, + Bitstream { + /// `HANDLE` is the handle that is given during `load` operation + /// TODO(Artie): document - for dfxmgr it will be the slot - use "" to allow remove latest + /// it is different than device_handle which is being used for platform + /// detection logic. + #[arg(long = "handle")] + handle: Option, + }, } /// Top-level commands supported by the CLI. @@ -320,7 +329,7 @@ enum Commands { async fn main() -> Result<(), Box> { env_logger::init(); let cli = Cli::parse(); - debug!("parsed cli command with {cli:?}"); + debug!("parsed cli command with {cli:#?}"); let result = match cli.command { Commands::Load { command } => load_handler(&cli.handle, &command).await, Commands::Remove { command } => remove_handler(&cli.handle, &command).await, diff --git a/cli/src/proxies/control_proxy.rs b/cli/src/proxies/control_proxy.rs index bbb63f1c..4f5d33d5 100644 --- a/cli/src/proxies/control_proxy.rs +++ b/cli/src/proxies/control_proxy.rs @@ -165,4 +165,27 @@ pub trait Control { /// * `Err(zbus::Error)` - DBus error or FpgadError. /// See [Error Handling](../../index.html#error-handling) async fn write_property_bytes(&self, property_path_str: &str, data: &[u8]) -> Result; + + /// Remove a previously loaded bitstream, identifiable by its `bitstream_handle` or `slot`. + /// + /// # Arguments + /// + /// * `platform_string`: Platform compatibility string. + /// * `overlay_handle`: Handle of the overlay to remove. + /// + /// # Returns: `Result` + /// * `Ok(String)` – Confirmation message including overlay filesystem path. + /// * `Err(fdo::Error)` if overlay or platform cannot be accessed. + /// + /// # Examples + /// + /// ``` + /// assert!(remove_bitstream("xlnx,zynqmp-pcap-fpga", "").is_ok()); + /// ``` + async fn remove_bitstream( + &self, + platform_string: &str, + device_handle: &str, + bitstream_handle: &str, + ) -> Result; } diff --git a/cli/src/proxies/status_proxy.rs b/cli/src/proxies/status_proxy.rs index a9734728..eefd6f72 100644 --- a/cli/src/proxies/status_proxy.rs +++ b/cli/src/proxies/status_proxy.rs @@ -62,6 +62,8 @@ use zbus::{Result, proxy}; default_path = "/com/canonical/fpgad/status" )] pub trait Status { + async fn get_status_message(&self, platform_string: &str) -> Result; + /// Get the current state of an FPGA device. /// /// Returns the device state such as "operating", "unknown", "write init", "write", diff --git a/cli/src/remove.rs b/cli/src/remove.rs index 0d755946..c0b2b7cb 100644 --- a/cli/src/remove.rs +++ b/cli/src/remove.rs @@ -30,23 +30,34 @@ use crate::RemoveSubcommand; use crate::proxies::control_proxy; -use crate::status::{call_get_platform_type, get_first_overlay, get_first_platform}; +use crate::status::{ + call_get_platform_type, get_first_device_handle, get_first_overlay, get_first_platform, +}; use zbus::Connection; -/// Removes a bitstream from an FPGA device. +/// Sends the DBus command to remove a bitstream. /// -/// # Note +/// Communicates with the fpgad daemon via DBus to remove a previously loaded +/// bitstream from the system. /// -/// This functionality is currently not implemented as bitstream removal is -/// vendor-specific and depends on platform capabilities that may be added -/// through softener implementations in the future. +/// # Arguments +/// +/// * `device_handle` - Platform identifier for the [device](../index.html#device-handles) +/// * `bitstream_handle` - the identifier of the bitstream (can be slot ID for dfx-mgr) TODO(Artie): update docs /// /// # Returns: `Result` -/// * `Err(zbus::Error)` - Always returns "Not implemented" error -async fn remove_bitstream() -> Result { - // TODO: so this is confusing because we don't have a way to remove a bitstream but with - // softeners we might end up with this functionality. - Err(zbus::Error::Failure("Not implemented".to_string())) +/// * `Ok(String)` - Success message from the daemon +/// * `Err(zbus::Error)` - DBus communication error, invalid handle(s), or FpgadError. +/// See [Error Handling](../index.html#error-handling) for details. +async fn call_remove_bitstream( + device_handle: &str, + bitstream_handle: &str, +) -> Result { + let connection = Connection::system().await?; + let proxy = control_proxy::ControlProxy::new(&connection).await?; + proxy + .remove_bitstream("", device_handle, bitstream_handle) + .await } /// Sends the DBus command to remove a device tree overlay. @@ -64,12 +75,12 @@ async fn remove_bitstream() -> Result { /// * `Err(zbus::Error)` - DBus communication error, invalid handle(s), or FpgadError. /// See [Error Handling](../index.html#error-handling) for details. async fn call_remove_overlay( - device_handle: &str, + platform_string: &str, overlay_handle: &str, ) -> Result { let connection = Connection::system().await?; let proxy = control_proxy::ControlProxy::new(&connection).await?; - proxy.remove_overlay(device_handle, overlay_handle).await + proxy.remove_overlay(platform_string, overlay_handle).await } /// Removes a device tree overlay with automatic platform and handle detection. @@ -93,7 +104,7 @@ async fn remove_overlay( device_handle: &Option, overlay_handle: &Option, ) -> Result { - let dev = match device_handle { + let platform_string = match device_handle { None => get_first_platform().await?, Some(dev) => call_get_platform_type(dev).await?, }; @@ -101,7 +112,32 @@ async fn remove_overlay( Some(handle) => handle.clone(), None => get_first_overlay().await?, }; - call_remove_overlay(&dev, &handle).await + call_remove_overlay(&platform_string, &handle).await +} + +/// Removes a bitstream from an FPGA device. +/// +/// # Note +/// +/// This functionality is currently not implemented as bitstream removal is +/// vendor-specific and depends on platform capabilities that may be added +/// through softener implementations in the future. +/// +/// # Returns: `Result` +/// * `Err(zbus::Error)` - Always returns "Not implemented" error +async fn remove_bitstream( + device_handle: &Option, + bitstream_handle: &Option, +) -> Result { + let dev = match device_handle { + None => &get_first_device_handle().await?, + Some(dev) => dev, + }; + let handle = match bitstream_handle { + Some(handle) => handle, + None => "", + }; + call_remove_bitstream(dev, handle).await } /// Main handler for the remove command. @@ -125,6 +161,6 @@ pub async fn remove_handler( ) -> Result { match sub_command { RemoveSubcommand::Overlay { handle } => remove_overlay(dev_handle, handle).await, - RemoveSubcommand::Bitstream => remove_bitstream().await, + RemoveSubcommand::Bitstream { handle } => remove_bitstream(dev_handle, handle).await, } } diff --git a/cli/src/status.rs b/cli/src/status.rs index d11ad0b9..77c24e84 100644 --- a/cli/src/status.rs +++ b/cli/src/status.rs @@ -34,6 +34,25 @@ use crate::proxies::status_proxy; use std::collections::HashMap; use zbus::Connection; +/// Retrieve a platform-specific status message from the daemon. +/// +/// Sends the DBus command to get a comprehensive status message for the specified +/// platform, which includes device and overlay information formatted for that platform. +/// +/// # Arguments +/// +/// * `platform` - Platform compatibility string (e.g., "xlnx,zynqmp-pcap-fpga", "universal") +/// +/// # Returns: `Result` +/// * `Ok(String)` - Formatted status message from the platform +/// * `Err(zbus::Error)` - DBus communication error or FpgadError. +/// See [Error Handling](../index.html#error-handling) for details. +pub async fn call_get_status_message(platform: &str) -> Result { + let connection = Connection::system().await?; + let proxy = status_proxy::StatusProxy::new(&connection).await?; + proxy.get_status_message(platform).await +} + /// Parses a newline-separated string of overlays into a Vec /// /// # Arguments @@ -215,35 +234,6 @@ pub async fn call_get_platform_type(device_handle: &str) -> Result` -/// * `String` - Status information for the overlay -/// * `zbus::Error` - DBus communication error, invalid overlay handle, or FpgadError. -/// See [Error Handling](../index.html#error-handling) for details. -/// -/// # Examples -/// -/// ```rust,ignore -/// let status = call_get_overlay_status("universal", "overlay0").await?; -/// println!("Overlay status: {}", status); -/// ``` -async fn call_get_overlay_status( - platform: &str, - overlay_handle: &str, -) -> Result { - let connection = Connection::system().await?; - let proxy = status_proxy::StatusProxy::new(&connection).await?; - proxy.get_overlay_status(platform, overlay_handle).await -} - /// Retrieve all FPGA devices and their platform compatibility strings. /// /// Parses the newline-separated string from the `get_platform_types` DBus interface @@ -357,13 +347,7 @@ pub async fn get_first_device_handle() -> Result { /// println!("{}", message); /// ``` async fn get_fpga_state_message(device_handle: &str) -> Result { - let state = call_get_fpga_state(device_handle).await?; - let platform = call_get_platform_type(device_handle).await?; - Ok(format!( - "---- DEVICE ----\n\ - | dev | platform | state |\n\ - {device_handle} | {platform} | {state}" - )) + call_get_fpga_state(device_handle).await } /// Format comprehensive status information for all FPGA devices and overlays. @@ -387,38 +371,12 @@ async fn get_fpga_state_message(device_handle: &str) -> Result Result { - let mut ret_string = String::from( - "---- DEVICES ----\n\ - | dev | platform | state |\n", - ); - - for (dev, platform) in call_get_platform_types().await? { - let state = call_get_fpga_state(&dev).await?; - ret_string += format!("| {dev} | {platform} | {state} |\n").as_str(); - } - - // If overlayfs not enabled, or interface not connected this will be an error. - let overlays = match call_get_overlays().await { - Ok(overlays) => { - ret_string += "\n---- OVERLAYS ----\n\ - | overlay | status |\n"; - overlays - } - Err(e) => { - ret_string += "\n---- OVERLAYS NOT ACCESSIBLE ----\n\n\ - errors encountered:\n"; - ret_string += e.to_string().as_str(); - Vec::new() - } - }; + let mut ret_string = String::from("\n"); - for overlay in overlays { - // TODO: overlays do not provide enough information to work out what platform to use. - // so maybe the status command can take a platform type instead or something. - // This is tricky. - let p = get_first_platform().await?; - let status = call_get_overlay_status(&p, &overlay).await?; - ret_string.push_str(format!("| {overlay} | {status} |\n").as_ref()); + for (device, platform) in call_get_platform_types().await? { + ret_string += format!("----- device: {device} | platform: {platform} -----\n").as_str(); + ret_string += call_get_status_message(&platform).await?.as_str(); + ret_string += "\n"; } Ok(ret_string) } diff --git a/daemon/Cargo.toml b/daemon/Cargo.toml index 6f243f3d..454242fe 100644 --- a/daemon/Cargo.toml +++ b/daemon/Cargo.toml @@ -24,7 +24,8 @@ env_logger = "0.11.8" tokio = { version = "1.47.0", features = ["full"] } zbus = { version = "5.9.0", default-features = false, features = ["tokio"] } thiserror = "2.0.12" +fdt = "0.1.5" [dev-dependencies] googletest = "0.14.2" -rstest = "0.26.1" \ No newline at end of file +rstest = "0.26.1" diff --git a/daemon/src/comm/README.md b/daemon/src/comm/README.md deleted file mode 100644 index a163173e..00000000 --- a/daemon/src/comm/README.md +++ /dev/null @@ -1,17 +0,0 @@ -# FPGAd Communication Module - -# General principle - -```mermaid -flowchart TD - A[External] -- Call --> B{{Interface}} - B --> C[[Create Platform Object]] - C --> D{need fpga?} - D -- y --> E[[create fpga obj]] - E --> F{need overlay_handler?} - D -- n --> F - F -- y --> G[[create overlay_handler obj]] - G --> H[[attempt to handle the call]] - F -- no --> H - H -- Response --> Z[External] -``` diff --git a/daemon/src/comm/dbus.rs b/daemon/src/comm/dbus.rs index 76dc5bfc..5ebb06d1 100644 --- a/daemon/src/comm/dbus.rs +++ b/daemon/src/comm/dbus.rs @@ -57,31 +57,9 @@ pub mod status_interface; use crate::config; use crate::error::FpgadError; -use crate::system_io::{fs_read, fs_write}; -use log::trace; -use std::path::{Component, Path, PathBuf, absolute}; - -/// Read the current contents of an FPGA device property, e.g. "name". The property path must be a subdirectory of the fpga manager directory (typically, /sys/class/fpga_manager/) -/// -/// # Arguments -/// -/// * `property_path_str`: path to the variable to read e.g. /sys/class/fpga_manager/fpga0/name -/// -/// # Returns: `Result` -/// * `String` - the contents of the property path -/// -/// * `FpgadError::Argument` if the path is not found within the compile time [config::FPGA_MANAGERS_DIR] -/// -/// # Examples -/// -/// ```rust,no_run -/// let device_name = fs_read_property("/sys/class/fpga_manager/fpga0/name")?; -/// assert_eq!(device_name, "Xilinx ZynqMP FPGA Manager\n") -/// ``` -pub fn fs_read_property(property_path_str: &str) -> Result { - let property_path = validate_property_path(Path::new(property_path_str))?; - fs_read(&property_path) -} +use crate::system_io::fs_read; +use std::path; +use std::path::{Component, Path, PathBuf}; /// Validates that a property path is constrained under the fpga manager directory and does not contain explicit parent traversal segments. /// This is used to validate paths for all read/write property access methods in the control and status interfaces. @@ -136,7 +114,7 @@ pub(crate) fn validate_property_path_with_base( ))); } - let canonical_base = absolute(base_path).map_err(|e| { + let canonical_base = path::absolute(base_path).map_err(|e| { FpgadError::Argument(format!( "Cannot access property {}: failed to resolve base path {}: {}", property_path.display(), @@ -144,7 +122,7 @@ pub(crate) fn validate_property_path_with_base( e )) })?; - let canonical_property = absolute(&property_path).map_err(|e| { + let canonical_property = path::absolute(&property_path).map_err(|e| { FpgadError::Argument(format!( "Cannot access property {}: failed to resolve property path: {}", property_path.display(), @@ -164,91 +142,26 @@ pub(crate) fn validate_property_path_with_base( Ok(canonical_property) } -#[allow(dead_code)] -/// Read the currently specified firmware search path. -/// See [these kernel docs](https://docs.kernel.org/driver-api/firmware/fw_search_path.html) -/// for more information on the process. -/// -/// # Returns: `Result` -/// * `String` - The contents of the firmware search path variable. -/// * `FpgadError::IOWrite` (or similar IO error) if writing fails for any reason. -/// -/// # Examples -/// -/// ```rust,no_run -/// let search_path_str = read_firmware_source_dir()?; -/// assert_eq!(search_path_str, "/lib/firmware/my_firmware_dir"); -/// ``` -pub fn read_firmware_source_dir() -> Result { - trace!( - "Reading fw prefix from {}", - config::FIRMWARE_LOC_CONTROL_PATH - ); - let fw_lookup_override = Path::new(config::FIRMWARE_LOC_CONTROL_PATH); - fs_read(fw_lookup_override) -} - -/// Write a specified path to the systems firmware search path. -/// See [these kernel docs](https://docs.kernel.org/driver-api/firmware/fw_search_path.html) -/// for more information on the process. +/// Read the current contents of an FPGA device property, e.g. "name". The property path must be a subdirectory of the fpga manager directory (typically, /sys/class/fpga_manager/) /// /// # Arguments /// -/// * `new_path`: path inside which firmware can be found -/// -/// # Returns: `Result<(), FpgadError>` -/// * `()` on success -/// * `FpgadError::IOWrite` (or similar IO error) if writing fails for any reason. -/// -/// # Examples -/// -/// ```rust,no_run -/// assert!(write_firmware_source_dir("/lib/firmware/my_firmware_dir").is_ok()); -/// ``` -pub fn write_firmware_source_dir(new_path: &str) -> Result<(), FpgadError> { - trace!( - "Writing fw prefix {} to {}", - new_path, - config::FIRMWARE_LOC_CONTROL_PATH - ); - let fw_lookup_override = Path::new(config::FIRMWARE_LOC_CONTROL_PATH); - fs_write(fw_lookup_override, false, new_path) -} - -/// Splits a Path object into its parent directory and basename/filename +/// * `property_path_str`: path to the variable to read e.g. /sys/class/fpga_manager/fpga0/name /// -/// # Arguments +/// # Returns: `Result` +/// * `String` - the contents of the property path /// -/// * `path`: path to be split +/// * `FpgadError::Argument` if the path is not found within the compile time [config::FPGA_MANAGERS_DIR] /// -/// # Returns: `Result<(PathBuf, PathBuf), FpgadError>` -/// * `(PathBuf, PathBuf)` - Tuple of parent directory and basename/filename -/// * `FpgadError::Argument` on invalid `path` or `path` is a root directory (no parent) /// # Examples /// /// ```rust,no_run -/// let (parent, base) = extract_path_and_filename(Path::new("/lib/firmware/file.bin")); -/// assert_eq!(parent.to_string_lossy(), "/lib/firmware"); -/// assert_eq!(base.to_string_lossy(), "file.bin"); +/// let device_name = fs_read_property("/sys/class/fpga_manager/fpga0/name")?; +/// assert_eq!(device_name, "Xilinx ZynqMP FPGA Manager\n") /// ``` -pub fn extract_path_and_filename(path: &Path) -> Result<(PathBuf, PathBuf), FpgadError> { - // Extract filename - let filename = path - .file_name() - .and_then(|f| f.to_str()) - .ok_or(FpgadError::Argument(format!( - "Provided bitstream path {path:?} is not a file or a valid directory." - )))?; - - // Extract parent directory - let base_path = path - .parent() - .and_then(|p| p.to_str()) - .ok_or(FpgadError::Argument(format!( - "Provided bitstream path {path:?} is missing a parent dir." - )))?; - - Ok((base_path.into(), filename.into())) +pub fn fs_read_property(property_path_str: &str) -> Result { + let property_path = validate_property_path(Path::new(property_path_str))?; + fs_read(&property_path) } /// Helper function to check that a device with given handle does exist. @@ -287,114 +200,6 @@ pub(crate) fn validate_device_handle(device_handle: &str) -> Result<(), FpgadErr Ok(()) } -/// Helper function to find the overlap between two paths and to return a tuple of the overlap and -/// the difference. Note: the paths must both share the same root otherwise no overlap will be found -/// -/// Typically, this is used to create a fw_lookup_path and a corresponding relative path which points -/// to the source file -/// -/// # Arguments -/// -/// * `source_path`: the full path to the target file (or containing directory?) -/// * `firmware_path`: the root common path for all files to be loaded by the FW subsystem -/// -/// # Returns: `Result<(PathBuf, PathBuf), FpgadError>` -/// * `(PathBuf, PathBuf)` - A tuple of (prefix, suffix) where prefix is -/// typically used as the fw_lookup_path and the suffix is remaining relative path from that prefix -/// * `FpgadError::Argument` in case `firmware_path` is not within `source_path`, or for inputs -/// resulting in an empty suffix value -/// # Examples -/// -/// ```rust -/// # use std::path::Path; -/// let (prefix, suffix) = make_firmware_pair( -/// Path::new("/lib/firmware/file.bin"), -/// Path::new("/lib/firmware/"), -/// )?; -/// assert_eq!(prefix.to_string_lossy(), "/lib/firmware"); -/// assert_eq!(suffix.to_string_lossy(), "file.bin"); -/// ``` -pub(crate) fn make_firmware_pair( - source_path: &Path, - firmware_path: &Path, -) -> Result<(PathBuf, PathBuf), FpgadError> { - // No firmware search path provided, so just try to use parent dir - if firmware_path.as_os_str().is_empty() { - return extract_path_and_filename(source_path); - } - if let Ok(suffix) = source_path.strip_prefix(firmware_path) { - // Remove leading '/' if present - let cleaned_suffix_path = suffix - .components() - .skip_while(|c| matches!(c, Component::RootDir)) - .collect::(); - if cleaned_suffix_path.as_os_str().is_empty() { - return Err(FpgadError::Argument(format!( - "The resulting filename from stripping {firmware_path:?} from {source_path:?} \ - was empty. Cannot write empty string to fpga." - ))); - } - Ok((firmware_path.to_path_buf(), cleaned_suffix_path)) - } else { - Err(FpgadError::Argument(format!( - "Could not find {source_path:?} inside {firmware_path:?}" - ))) - } -} - -#[cfg(test)] -mod test_make_firmware_pair { - use crate::comm::dbus::make_firmware_pair; - use crate::error::FpgadError; - use googletest::prelude::*; - use rstest::*; - use std::path::PathBuf; - - #[gtest] - #[rstest] - #[case::all_good( - "/lib/firmware/file.bin", - "/lib/firmware/", - "/lib/firmware/", - "file.bin" - )] - #[case::no_fw_path("/lib/firmware/file.bin", "", "/lib/firmware/", "file.bin")] - #[case::no_fw_path_no_file("/lib/firmware/", "", "/lib/", "firmware")] - fn should_pass( - #[case] source: &str, - #[case] fw_path: &str, - #[case] exp_prefix: &str, - #[case] exp_suffix: &str, - ) { - let result = make_firmware_pair(&PathBuf::from(source), &PathBuf::from(fw_path)); - assert_that!( - result, - ok(eq(&(PathBuf::from(exp_prefix), PathBuf::from(exp_suffix)))) - ); - } - - #[gtest] - #[rstest] - #[case::no_file( - "/lib/firmware/", - "/lib/firmware/", - err(displays_as(contains_substring("The resulting filename from stripping"))) - )] - #[case::not_in_dir( - "/lib/firmware/file.bin", - "/snap/x1/data/file.bin", - err(displays_as(contains_substring("Could not find"))) - )] - fn should_fail Matcher<&'a std::result::Result<(PathBuf, PathBuf), FpgadError>>>( - #[case] source: &str, - #[case] fw_path: &str, - #[case] condition: M, - ) { - let result = make_firmware_pair(&PathBuf::from(source), &PathBuf::from(fw_path)); - assert_that!(&result, condition); - } -} - #[cfg(test)] mod test_validate_property_path { use crate::comm::dbus::validate_property_path_with_base; diff --git a/daemon/src/comm/dbus/control_interface.rs b/daemon/src/comm/dbus/control_interface.rs index 0cf5fb3d..c339cfae 100644 --- a/daemon/src/comm/dbus/control_interface.rs +++ b/daemon/src/comm/dbus/control_interface.rs @@ -16,14 +16,16 @@ //! If FPGAd raises the error, then the fdo::Error strings are prepended with the relevant FPGAd error type e.g. `FpgadError::Argument: `. See [crate::comm::dbus] for a summary of this interface's methods. //! -use crate::comm::dbus::{ - make_firmware_pair, validate_device_handle, validate_property_path, write_firmware_source_dir, -}; +use crate::comm::dbus::{validate_device_handle, validate_property_path}; +use crate::error::map_error_io_to_fdo; use crate::platforms::platform::{platform_for_known_platform, platform_from_compat_or_device}; +use crate::softeners::error::FpgadSoftenerError; use crate::system_io::{fs_write, fs_write_bytes}; use log::{info, trace}; +use std::env; use std::path::Path; use std::sync::Arc; +use tokio::process::Command; use tokio::sync::{Mutex, MutexGuard, OnceCell}; use zbus::{fdo, interface}; @@ -108,11 +110,11 @@ impl ControlInterface { device_handle: &str, flags: u32, ) -> Result { + // TODO(Artie): consider whether this should be a platform specific function or not. info!("set_fpga_flags called with name: {device_handle} and flags: {flags}"); validate_device_handle(device_handle)?; let platform = platform_from_compat_or_device(platform_string, device_handle)?; - platform.fpga(device_handle)?.set_flags(flags)?; - Ok(format!("Flags set to 0x{flags:X} for {device_handle}")) + Ok(platform.fpga(device_handle)?.set_flags(flags)?) } /// Trigger a bitstream-only load of a bitstream to a given FPGA device (i.e. no device-tree changes or driver installation). @@ -172,19 +174,17 @@ impl ControlInterface { bitstream_path_str: &str, firmware_lookup_path: &str, ) -> Result { + // TODO(Artie): refactor this code to be platform specific. The dfx-mgr implementation does + // the firmware source dir write under the hood. This could instead be in a different # + // interface for dfx-mgr, but then the platform detection becomes useless. info!("load_firmware called with name: {device_handle} and path_str: {bitstream_path_str}"); validate_device_handle(device_handle)?; let path = Path::new(bitstream_path_str); + let lookup = Path::new(firmware_lookup_path); let _guard = get_write_lock_guard().await; trace!("Got write lock."); let platform = platform_from_compat_or_device(platform_string, device_handle)?; - let (prefix, suffix) = make_firmware_pair(path, Path::new(firmware_lookup_path))?; - write_firmware_source_dir(&prefix.to_string_lossy())?; - platform.fpga(device_handle)?.load_firmware(&suffix)?; - Ok(format!( - "{bitstream_path_str} loaded to {device_handle} using firmware lookup path: '\ - {prefix:?}'" - )) + Ok(platform.fpga(device_handle)?.load_firmware(path, lookup)?) } /// Apply a device-tree overlay to trigger a bitstream load and driver probe events. @@ -260,17 +260,11 @@ impl ControlInterface { trace!("Got write lock."); let platform = platform_for_known_platform(platform_string)?; let overlay_handler = platform.overlay_handler(overlay_handle)?; - let overlay_fs_path = overlay_handler.overlay_fs_path()?; - let (prefix, suffix) = make_firmware_pair( + + Ok(overlay_handler.apply_overlay( Path::new(overlay_source_path), Path::new(firmware_lookup_path), - )?; - write_firmware_source_dir(&prefix.to_string_lossy())?; - overlay_handler.apply_overlay(&suffix)?; - Ok(format!( - "{overlay_source_path} loaded via {overlay_fs_path:?} using firmware lookup path: '\ - {prefix:?}'" - )) + )?) } /// Remove a previously applied overlay, identifiable by its `overlay_handle`. @@ -300,11 +294,47 @@ impl ControlInterface { ); let platform = platform_for_known_platform(platform_string)?; let overlay_handler = platform.overlay_handler(overlay_handle)?; - let overlay_fs_path = overlay_handler.overlay_fs_path()?; - overlay_handler.remove_overlay()?; - Ok(format!( - "{overlay_handle} removed by deleting {overlay_fs_path:?}" - )) + let handle = match overlay_handle { + "" => None, + _ => Some(overlay_handle), + }; + Ok(overlay_handler.remove_overlay(handle)?) + } + + /// Remove a previously loaded bitstream, identifiable by its `bitstream_handle` or `slot`. + /// + /// # Arguments + /// + /// * `platform_string`: Platform compatibility string. + /// * `device_handle`: FPGA device handle (e.g., `fpga0`). + /// * `bitstream_handle`: Handle/slot of the bitstream to remove. + /// + /// # Returns: `Result` + /// * `Ok(String)` – Confirmation message including device and bitstream handle. + /// * `Err(fdo::Error)` if device or platform cannot be accessed. + /// + /// # Examples + /// + /// ``` + /// assert!(remove_bitstream("xlnx,zynqmp-pcap-fpga", "fpga0", "").is_ok()); + /// ``` + async fn remove_bitstream( + &self, + platform_string: &str, + device_handle: &str, + bitstream_handle: &str, + ) -> Result { + info!( + "remove_bitstream called with platform_string: {platform_string}, device_handle:\ + {device_handle} and bitstream_handle: {bitstream_handle}" + ); + let platform = platform_from_compat_or_device(platform_string, device_handle)?; + let fpga = platform.fpga(device_handle)?; + let handle = match bitstream_handle { + "" => None, + _ => Some(bitstream_handle), + }; + Ok(fpga.remove_firmware(handle)?) } /// Write a string value to an arbitrary FPGA device property. @@ -339,6 +369,7 @@ impl ControlInterface { property_path_str: &str, data: &str, ) -> Result { + // TODO(Artie): consider whether this should be a platform specific function or not. info!("write_property called with property_path_str: {property_path_str} and data: {data}"); let property_path = validate_property_path(Path::new(property_path_str))?; fs_write(&property_path, false, data)?; @@ -378,6 +409,7 @@ impl ControlInterface { property_path_str: &str, data: &[u8], ) -> Result { + // TODO(Artie): consider whether this should be a platform specific function or not. info!( "write_property called with property_path_str: {property_path_str} and data: {data:?}" ); @@ -387,6 +419,60 @@ impl ControlInterface { "Byte string successfully written to {property_path_str}" )) } + + async fn dfx_mgr(&self, cmd_string: &str) -> Result { + if cfg!(feature = "xilinx-dfx-mgr") { + let snap_env = env::var("SNAP").map_err(|_| { + fdo::Error::Failed( + "Cannot find dfx-mgr because $SNAP not specified in environment".into(), + ) + })?; + + let dfx_mgr_client_path = format!("{}/usr/bin/dfx-mgr-client", snap_env); + + // Check if dfx-mgr-client exists + if !Path::new(&dfx_mgr_client_path).exists() { + // TODO(Artie): add tests for this + return Err(FpgadSoftenerError::DfxMgr(format!( + "dfx-mgr-client not detected.\n\ + If using snap, please install the dfx-mgr component with \n\ + `[sudo] snap install fpgad+dfx-mgr [options]` \n\ + otherwise ensure that dfx-mgr-client exists at `{dfx_mgr_client_path}`" + )) + .into()); + } + + let output = Command::new(&dfx_mgr_client_path) + .args(cmd_string.split_whitespace()) + .output() + .await + .map_err(|e| { + map_error_io_to_fdo("dfx-mgr-client call failed to produce any output", e) + })?; + + // Exit status + if output.status.success() { + info!("Command ran successfully!"); + } else { + info!("Command failed with code: {:#?}", output.status.code()); + } + + Ok(format!( + "dfx-mgr called with args {}.\nExit status: {}\nStdout:\n{}\nStderr:\n{}", + cmd_string, + output.status, + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr), + )) + } else { + use crate::error::FpgadError; + Err(FpgadError::Feature( + "Cannot use DfxMgr method - FPGAd was compiled without xilinx-dfx-mgr feature" + .into(), + ) + .into()) + } + } } #[cfg(test)] diff --git a/daemon/src/comm/dbus/status_interface.rs b/daemon/src/comm/dbus/status_interface.rs index 5065997b..d84f508c 100644 --- a/daemon/src/comm/dbus/status_interface.rs +++ b/daemon/src/comm/dbus/status_interface.rs @@ -32,6 +32,12 @@ pub struct StatusInterface {} /// [crate::comm::dbus::status_interface] for a summary of this interface in general. #[interface(name = "com.canonical.fpgad.status")] impl StatusInterface { + async fn get_status_message(&self, platform_string: &str) -> Result { + info!("get_fpga_state called with platform_string: {platform_string}"); + let platform = platform_from_compat_or_device(platform_string, "")?; + Ok(platform.status_message()?) + } + /// The device handle (e.g., `fpga0`) of the FPGA. /// /// # Arguments @@ -162,6 +168,8 @@ impl StatusInterface { /// ``` /// async fn get_overlays(&self) -> Result { + // TODO(artie): should this be a platform specific call? - dfx-mgr would require parsing the + // -listPackage output. info!("get_overlays called"); let overlay_handles = fs_read_dir(config::OVERLAY_CONTROL_DIR.as_ref())?; Ok(overlay_handles.join("\n")) @@ -197,7 +205,7 @@ impl StatusInterface { /// * `Ok(String)` – Each line formatted as `:\n`. /// Devices without a valid string appear as /// `:\n`. - /// * `Err(fdo::Error)` if device validation or reading the compatible string fails. + /// * `Err(fdo::Error)` if reading FPGA managers directory or compatible strings fails. /// /// # Examples /// diff --git a/daemon/src/error.rs b/daemon/src/error.rs index 5219046c..0a09bf0b 100644 --- a/daemon/src/error.rs +++ b/daemon/src/error.rs @@ -56,6 +56,7 @@ use zbus::fdo; /// All errors implement `Display` and will be formatted with the `FpgadError:::` /// prefix, making them easily identifiable in logs and error messages sent over DBus. #[derive(Debug, thiserror::Error)] +#[non_exhaustive] pub enum FpgadError { /// Failed to read FPGA programming flags from sysfs. #[error("FpgadError::Flag: Failed to read flags: {0}")] @@ -96,6 +97,10 @@ pub enum FpgadError { #[error("FpgadError::Softener: An error occurred using softener: {0}")] Softener(crate::softeners::error::FpgadSoftenerError), + /// Any other unexpected internal error occurred. + #[error("FpgadError::Feature: Use of disabled feature: {0}")] + Feature(String), + /// Any other unexpected internal error occurred. #[error("FpgadError::Internal: An Internal error occurred: {0}")] Internal(String), @@ -126,3 +131,17 @@ impl From for fdo::Error { } } } + +/// Converter for tokio::io::Error into zbus::fdo::Error types, with added "custom_msg" in order +/// to provide additional context. +/// This allows for "?" on e.g., tokio::io::Command calls +/// +/// # Arguments +/// +/// * `custom_msg`: String to prepend the error - use to add context e.g. "failed when calling nproc" +/// * `err`: the tokio::io::error from the failing function +/// +/// returns: zbus::fdo::Error type suitable for dbus transmission +pub(crate) fn map_error_io_to_fdo(custom_msg: &str, err: impl std::fmt::Display) -> fdo::Error { + fdo::Error::Failed(format!("{custom_msg}:\n{err}")) +} diff --git a/daemon/src/platforms/platform.rs b/daemon/src/platforms/platform.rs index 65a99136..ab9bd009 100644 --- a/daemon/src/platforms/platform.rs +++ b/daemon/src/platforms/platform.rs @@ -40,7 +40,8 @@ //! Platforms and softeners are included or not excluded using cargo "features". //! See [`softeners`](../../softeners/index.html) for more details. //! -//! TODO(Artie): Add examples of how to use the getters for platforms with and without knowing the platform string? - could be called "# Fetching platforms" +//! TODO(Artie): Add examples of how to use the getters for platforms with and without knowing the +//! platform string? - could be called "# Fetching platforms" //! # Examples //! //! in [main.rs]: @@ -124,23 +125,41 @@ pub trait Fpga { /// /// * `flags` - The flags value to set /// - /// # Returns: `Result<(), FpgadError>` - /// * `Ok(())` - Flags set successfully + /// # Returns: `Result` + /// * `Ok(String)` - Confirmation message including flags value and device handle /// * `Err(FpgadError::IOWrite)` - Failed to write flags file - fn set_flags(&self, flags: u32) -> Result<(), FpgadError>; + fn set_flags(&self, flags: u32) -> Result; /// Load a bitstream firmware file to the FPGA device. /// /// # Arguments /// - /// * `bitstream_path_rel` - Path to the bitstream file relative to whatever path the lookup starts in. For universal, this is the firmware search path + /// * `bitstream_path` - Absolute path to the bitstream file + /// * `firmware_lookup_path` - Path to resolve firmware or empty path + /// (automatically uses the parent dir of `bitstream_path`) /// - /// # Returns: `Result<(), FpgadError>` - /// * `Ok(())` - Bitstream loaded successfully + /// # Returns: `Result` + /// * `Ok(String)` - Confirmation message with source and target /// * `Err(FpgadError::IOWrite)` - Failed to write firmware file /// * `Err(FpgadError::FPGAState)` - FPGA not in correct state for loading #[allow(dead_code)] - fn load_firmware(&self, bitstream_path_rel: &Path) -> Result<(), FpgadError>; + fn load_firmware( + &self, + bitstream_path: &Path, + firmware_lookup_path: &Path, + ) -> Result; + + /// Remove a previously loaded firmware/bitstream. + /// + /// # Arguments + /// + /// * `handle` - Optional handle/slot identifier for the firmware to remove + /// + /// # Returns: `Result` + /// * `Ok(String)` - Confirmation message including device and firmware details + /// * `Err(FpgadError::Internal)` - Operation not supported by this platform + /// * `Err(FpgadError)` - Failed to remove firmware + fn remove_firmware(&self, handle: Option<&str>) -> Result; } /// Trait for managing device tree overlays. @@ -150,27 +169,25 @@ pub trait OverlayHandler { /// # Arguments /// /// * `source_path` - Path to the `.dtbo` overlay binary file + /// * `lookup_path` - Path to resolve overlay firmware or empty path + /// (automatically uses the parent dir of `source_path`) /// - /// # Returns: `Result<(), FpgadError>` - /// * `Ok(())` - Overlay applied successfully + /// # Returns: `Result` + /// * `Ok(String)` - Confirmation message with overlay path and firmware prefix /// * `Err(FpgadError::IOWrite)` - Failed to write overlay /// * `Err(FpgadError::OverlayStatus)` - Overlay application failed - fn apply_overlay(&self, source_path: &Path) -> Result<(), FpgadError>; + fn apply_overlay(&self, source_path: &Path, lookup_path: &Path) -> Result; /// Remove a device tree overlay. /// - /// # Returns: `Result<(), FpgadError>` - /// * `Ok(())` - Overlay removed successfully - /// * `Err(FpgadError::IODelete)` - Failed to remove overlay directory - fn remove_overlay(&self) -> Result<(), FpgadError>; - - /// Get the required FPGA flags, however they may be provided. + /// # Arguments /// - /// # Returns: `Result` - /// * `Ok(isize)` - Required flags value - /// * `Err(FpgadError)` - Failed to parse overlay or extract flags - #[allow(dead_code)] - fn required_flags(&self) -> Result; + /// * `handle` - Optional handle/slot identifier for the overlay to remove + /// + /// # Returns: `Result` + /// * `Ok(String)` - Confirmation message including overlay filesystem path + /// * `Err(FpgadError::IODelete)` - Failed to remove overlay directory + fn remove_overlay(&self, handle: Option<&str>) -> Result; /// Get the current status of the overlay. /// @@ -241,6 +258,41 @@ pub trait Platform: Any { /// * `Ok(&dyn OverlayHandler)` - Overlay handler instance /// * `Err(FpgadError::Argument)` - Invalid overlay handle or configfs not available fn overlay_handler(&self, overlay_handle: &str) -> Result<&dyn OverlayHandler, FpgadError>; + + /// Get a formatted status message for this platform. + /// + /// Returns a human-readable status message containing information about devices, + /// overlays, and platform-specific state. The format and content are platform-specific. + /// + /// # Returns: `Result` + /// * `Ok(String)` - Formatted status message + /// * `Err(FpgadError)` - Failed to gather status information + /// + /// # Examples + /// + /// For the Universal platform, this returns a table of devices and overlays. + /// For Xilinx DFX Manager, this returns the output of `dfx-mgr-client -listPackage`. + fn status_message(&self) -> Result; + + /// Get the platform compatibility string. + /// + /// Returns the compatibility string that this platform implementation is registered + /// with. This string matches against device tree compatible properties to determine + /// which platform to use for a device. + /// + /// # Returns: `String` + /// * The platform compatibility string (e.g., "universal", "xlnx,zynqmp-pcap-fpga") + /// + /// # Examples + /// + /// ```rust,no_run + /// # use daemon::platforms::platform::Platform; + /// # fn example(platform: &dyn Platform) { + /// let compat = platform.platform_compat_string(); + /// println!("Platform: {}", compat); + /// # } + /// ``` + fn platform_compat_string(&self) -> String; } /// Match a platform compatibility string to a registered platform. @@ -288,6 +340,7 @@ fn match_platform_string(platform_string: &str) -> Result, Fpg let compat_set: HashSet<&str> = compat_string.split(',').collect(); let compat_found = platform_string.split(',').all(|x| compat_set.contains(x)); if compat_found { + trace!("found `{compat_found}` for platform with compat string `{platform_string}`"); return Ok(platform_constructor()); } } @@ -325,10 +378,16 @@ fn discover_platform(device_handle: &str) -> Result, FpgadErro let compat_string = read_compatible_string(device_handle)?; trace!("Found compatibility string: '{compat_string}'"); - Ok(match_platform_string(&compat_string).unwrap_or({ - warn!("{compat_string} not supported. Defaulting to Universal platform."); - Box::new(UniversalPlatform::new()) - })) + match match_platform_string(&compat_string) { + Ok(platform) => { + trace!("Matched platform for compatibility string: '{compat_string}'"); + Ok(platform) + } + Err(_) => { + warn!("{compat_string} not supported. Defaulting to Universal platform."); + Ok(Box::new(UniversalPlatform::new())) + } + } } /// Read the device tree compatible string for an FPGA device. @@ -512,7 +571,7 @@ pub fn list_fpga_managers() -> Result, FpgadError> { fs_read_dir(config::FPGA_MANAGERS_DIR.as_ref()) } -#[cfg(test)] +#[cfg(all(test, feature = "xilinx-dfx-mgr"))] mod tests { use super::*; use crate::softeners::xilinx_dfx_mgr::XilinxDfxMgrPlatform; diff --git a/daemon/src/platforms/universal.rs b/daemon/src/platforms/universal.rs index fe39e676..f1942361 100644 --- a/daemon/src/platforms/universal.rs +++ b/daemon/src/platforms/universal.rs @@ -45,10 +45,12 @@ //! let state = fpga.state()?; //! ``` +use crate::config; use crate::error::FpgadError; -use crate::platforms::platform::{Fpga, OverlayHandler, Platform}; +use crate::platforms::platform::{Fpga, OverlayHandler, Platform, list_fpga_managers}; use crate::platforms::universal_components::universal_fpga::UniversalFPGA; use crate::platforms::universal_components::universal_overlay_handler::UniversalOverlayHandler; +use crate::system_io::fs_read_dir; use fpgad_macros::platform; use log::trace; use std::sync::OnceLock; @@ -176,4 +178,36 @@ impl Platform for UniversalPlatform { } Ok(handler) } + + fn status_message(&self) -> Result { + let mut ret_string = String::from( + "---- DEVICES ----\n\ + | dev | platform | state |\n", + ); + + for device in list_fpga_managers()? { + let state = self.fpga(&device)?.state()?; + ret_string += format!( + "| {} | {} | {} |\n", + device, + self.platform_compat_string(), + state + ) + .as_str(); + } + ret_string += "\n---- OVERLAYS ----\n\ + | overlay | status |\n"; + + // If overlayfs not enabled, or interface not connected this will be an error. + for overlay in fs_read_dir(config::OVERLAY_CONTROL_DIR.as_ref())? { + let status = self.overlay_handler(&overlay)?.status()?; + ret_string.push_str(format!("| {overlay} | {status} |\n").as_ref()); + } + + Ok(ret_string) + } + + fn platform_compat_string(&self) -> String { + "universal".into() + } } diff --git a/daemon/src/platforms/universal_components.rs b/daemon/src/platforms/universal_components.rs index 051a1955..ff8e4a42 100644 --- a/daemon/src/platforms/universal_components.rs +++ b/daemon/src/platforms/universal_components.rs @@ -10,4 +10,5 @@ // // You should have received a copy of the GNU General Public License along with this program. If not, see http://www.gnu.org/licenses/. pub mod universal_fpga; +mod universal_helpers; pub mod universal_overlay_handler; diff --git a/daemon/src/platforms/universal_components/universal_fpga.rs b/daemon/src/platforms/universal_components/universal_fpga.rs index ec503d94..fee63235 100644 --- a/daemon/src/platforms/universal_components/universal_fpga.rs +++ b/daemon/src/platforms/universal_components/universal_fpga.rs @@ -71,10 +71,11 @@ //! println!("Flags: 0x{:X}", flags); //! ``` -use crate::config; use crate::error::FpgadError; use crate::platforms::platform::Fpga; +use crate::platforms::universal_components::universal_helpers; use crate::system_io::{fs_read, fs_write}; +use crate::{config, system_io}; use log::{error, info, trace, warn}; use std::path::Path; @@ -141,11 +142,11 @@ impl UniversalFPGA { match self.state() { Ok(state) => match state.to_string().as_str() { "operating" => { - info!("{}'s state is 'operating'", self.device_handle); + info!("The state of '{}' is 'operating'", self.device_handle); Ok(()) } _ => Err(FpgadError::FPGAState(format!( - "After loading bitstream, {}'s state should be should be 'operating' but it is '{}'", + "After loading bitstream, the state of '{}' should be should be 'operating' but it is '{}'", self.device_handle, state ))), }, @@ -178,7 +179,7 @@ impl Fpga for UniversalFPGA { let state_path = Path::new(config::FPGA_MANAGERS_DIR) .join(self.device_handle.clone()) .join("state"); - trace!("reading {state_path:?}"); + trace!("reading '{state_path:?}'"); fs_read(&state_path).map(|s| s.trim_end_matches('\n').to_string()) } @@ -217,11 +218,11 @@ impl Fpga for UniversalFPGA { /// * `Err(FpgadError::IOWrite)` - Failed to write flags file /// * `Err(FpgadError::IORead)` - Failed to read back flags or state /// * `Err(FpgadError::Flag)` - Read-back value doesn't match written value - fn set_flags(&self, flags: u32) -> Result<(), FpgadError> { + fn set_flags(&self, flags: u32) -> Result { let flag_path = Path::new(config::FPGA_MANAGERS_DIR) .join(self.device_handle.clone()) .join("flags"); - trace!("Writing 0x'{flags:X}' to '{flag_path:?}"); + trace!("Writing '0x{flags:X}' to '{flag_path:?}'"); if let Err(e) = fs_write(&flag_path, false, format!("0x{flags:X}")) { error!("Failed to read state."); return Err(e); @@ -246,13 +247,16 @@ impl Fpga for UniversalFPGA { }; match self.flags() { - Ok(returned_flags) if returned_flags == flags => Ok(()), + Ok(returned_flags) if returned_flags == flags => Ok(format!( + "Flags set to '0x{:X}' for '{}'", + flags, self.device_handle + )), Ok(returned_flags) => Err(FpgadError::Flag(format!( - "Setting {}'s flags to '{}' failed. Resulting flag was '{}'", + "Setting flags of '{}' to '{}' failed. Resulting flag was '{}'", self.device_handle, flags, returned_flags ))), Err(e) => Err(FpgadError::Flag(format!( - "Failed to read {}'s flags after setting to '{}': {}", + "Failed to read device '{}' flags after setting to '{}': {}", self.device_handle, flags, e ))), } @@ -267,10 +271,12 @@ impl Fpga for UniversalFPGA { /// /// # Arguments /// - /// * `bitstream_path_rel` - Path to bitstream file relative to firmware search path + /// * `bitstream_path` - Absolute path to the bitstream file + /// * `firmware_lookup_path` - Path to resolve firmware or empty path + /// (automatically uses the parent dir of `bitstream_path`) /// - /// # Returns: `Result<(), FpgadError>` - /// * `Ok(())` - Bitstream loaded and FPGA is operating + /// # Returns: `Result` + /// * `Ok(String)` - Confirmation message with source and firmware lookup path /// * `Err(FpgadError::IOWrite)` - Failed to write firmware file /// * `Err(FpgadError::FPGAState)` - FPGA not in "operating" state after loading /// @@ -287,11 +293,27 @@ impl Fpga for UniversalFPGA { /// fpga.load_firmware(Path::new("design.bit.bin"))?; /// println!("Bitstream loaded successfully"); /// ``` - fn load_firmware(&self, bitstream_path_rel: &Path) -> Result<(), FpgadError> { + fn load_firmware( + &self, + bitstream_path: &Path, + firmware_lookup_path: &Path, + ) -> Result { + let (prefix, suffix) = system_io::make_firmware_pair(bitstream_path, firmware_lookup_path)?; + universal_helpers::write_firmware_source_dir(&prefix.to_string_lossy())?; let control_path = Path::new(config::FPGA_MANAGERS_DIR) .join(self.device_handle()) .join("firmware"); - fs_write(&control_path, false, bitstream_path_rel.to_string_lossy())?; - self.assert_state() + fs_write(&control_path, false, suffix.to_string_lossy())?; + self.assert_state()?; + Ok(format!( + "'{:#?}' loaded to '{}' using firmware lookup path '{:#?}'", + bitstream_path, self.device_handle, prefix + )) + } + + fn remove_firmware(&self, _handle: Option<&str>) -> Result { + Err(FpgadError::Internal( + "UniversalPlatform does not support removing bitstreams".to_string(), + )) } } diff --git a/daemon/src/platforms/universal_components/universal_helpers.rs b/daemon/src/platforms/universal_components/universal_helpers.rs new file mode 100644 index 00000000..b8017ff7 --- /dev/null +++ b/daemon/src/platforms/universal_components/universal_helpers.rs @@ -0,0 +1,68 @@ +// This file is part of fpgad, an application to manage FPGA subsystem together with device-tree and kernel modules. +// +// Copyright 2026 Canonical Ltd. +// +// SPDX-License-Identifier: GPL-3.0-only +// +// fpgad is free software: you can redistribute it and/or modify it under the terms of the GNU General Public License version 3, as published by the Free Software Foundation. +// +// fpgad is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; without even the implied warranties of MERCHANTABILITY, SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License along with this program. If not, see http://www.gnu.org/licenses/. + +use crate::config; +use crate::error::FpgadError; +use crate::system_io::{fs_read, fs_write}; +use log::trace; +use std::path::Path; + +/// Write a specified path to the systems firmware search path. +/// See [these kernel docs](https://docs.kernel.org/driver-api/firmware/fw_search_path.html) +/// for more information on the process. +/// +/// # Arguments +/// +/// * `new_path`: path inside which firmware can be found +/// +/// # Returns: `Result<(), FpgadError>` +/// * `()` on success +/// * `FpgadError::IOWrite` (or similar IO error) if writing fails for any reason. +/// +/// # Examples +/// +/// ```rust,no_run +/// assert!(write_firmware_source_dir("/lib/firmware/my_firmware_dir").is_ok()); +/// ``` +pub fn write_firmware_source_dir(new_path: &str) -> Result<(), FpgadError> { + trace!( + "Writing fw prefix {} to {}", + new_path, + config::FIRMWARE_LOC_CONTROL_PATH + ); + let fw_lookup_override = Path::new(config::FIRMWARE_LOC_CONTROL_PATH); + fs_write(fw_lookup_override, false, new_path) +} + +#[allow(dead_code)] +/// Read the currently specified firmware search path. +/// See [these kernel docs](https://docs.kernel.org/driver-api/firmware/fw_search_path.html) +/// for more information on the process. +/// +/// # Returns: `Result` +/// * `String` - The contents of the firmware search path variable. +/// * `FpgadError::IOWrite` (or similar IO error) if writing fails for any reason. +/// +/// # Examples +/// +/// ```rust,no_run +/// let search_path_str = read_firmware_source_dir()?; +/// assert_eq!(search_path_str, "/lib/firmware/my_firmware_dir"); +/// ``` +pub fn read_firmware_source_dir() -> Result { + trace!( + "Reading fw prefix from {}", + config::FIRMWARE_LOC_CONTROL_PATH + ); + let fw_lookup_override = Path::new(config::FIRMWARE_LOC_CONTROL_PATH); + fs_read(fw_lookup_override) +} diff --git a/daemon/src/platforms/universal_components/universal_overlay_handler.rs b/daemon/src/platforms/universal_components/universal_overlay_handler.rs index 6ba2d0ff..c5a15f8e 100644 --- a/daemon/src/platforms/universal_components/universal_overlay_handler.rs +++ b/daemon/src/platforms/universal_components/universal_overlay_handler.rs @@ -52,10 +52,11 @@ //! handler.remove_overlay()?; //! ``` -use crate::config; use crate::error::FpgadError; use crate::platforms::platform::OverlayHandler; +use crate::platforms::universal_components::universal_helpers; use crate::system_io::{fs_create_dir, fs_read, fs_remove_dir, fs_write}; +use crate::{config, system_io}; use log::{info, trace}; use std::path::{Path, PathBuf}; @@ -84,7 +85,7 @@ use std::path::{Path, PathBuf}; /// ``` fn construct_overlay_fs_path(overlay_handle: &str) -> PathBuf { let overlay_fs_path = PathBuf::from(config::OVERLAY_CONTROL_DIR).join(overlay_handle); - trace!("overlay_fs_path will be {overlay_fs_path:?}"); + trace!("overlay_fs_path will be '{overlay_fs_path:?}'"); overlay_fs_path } @@ -104,6 +105,7 @@ fn construct_overlay_fs_path(overlay_handle: &str) -> PathBuf { pub struct UniversalOverlayHandler { /// The path which points to the overlay virtual filesystem's dir which contains /// `path`, `status` and `dtbo` virtual files for overlay control. `dtbo` appears unused? + overlay_handle: String, overlay_fs_path: PathBuf, } @@ -119,7 +121,7 @@ impl UniversalOverlayHandler { /// * `Err(FpgadError::IORead)` - Failed to read status file fn get_vfs_status(&self) -> Result { let status_path = self.overlay_fs_path()?.join("status"); - trace!("Reading from {status_path:?}"); + trace!("Reading from '{status_path:?}'"); fs_read(&status_path).map(|s| s.trim_end_matches('\n').to_string()) } @@ -133,7 +135,7 @@ impl UniversalOverlayHandler { /// * `Err(FpgadError::IORead)` - Failed to read path file fn get_vfs_path(&self) -> Result { let path_path = self.overlay_fs_path()?.join("path"); - trace!("Reading from {path_path:?}"); + trace!("Reading from '{path_path:?}'"); let path_string = fs_read(&path_path).map(|s| s.trim_end_matches('\n').to_string())?; Ok(PathBuf::from(path_string)) } @@ -188,10 +190,12 @@ impl OverlayHandler for UniversalOverlayHandler { /// /// # Arguments /// - /// * `source_path_rel` - Path to the overlay file (can be absolute or relative to firmware path) + /// * `source_path` - Path to the overlay file (can be absolute or relative to firmware path) + /// * `lookup_path` - Path to resolve overlay firmware or empty path + /// (automatically uses the parent dir of `source_path`) /// - /// # Returns: `Result<(), FpgadError>` - /// * `Ok(())` - Overlay applied and verified successfully + /// # Returns: `Result` + /// * `Ok(String)` - Confirmation message with overlay path and firmware prefix /// * `Err(FpgadError::Argument)` - Overlay with this handle already exists /// * `Err(FpgadError::IOCreate)` - Failed to create overlay directory /// * `Err(FpgadError::Internal)` - configfs didn't create `path` file (not mounted?) @@ -204,35 +208,43 @@ impl OverlayHandler for UniversalOverlayHandler { /// - This method is not valid if the dtbo doesn't contain firmware to load /// - The overlay directory must not already exist. [`OverlayHandler::remove_overlay`] can be called /// to remove it first. - fn apply_overlay(&self, source_path_rel: &Path) -> Result<(), FpgadError> { + fn apply_overlay(&self, source_path: &Path, lookup_path: &Path) -> Result { + let (prefix, suffix) = + system_io::make_firmware_pair(Path::new(source_path), Path::new(lookup_path))?; + universal_helpers::write_firmware_source_dir(&prefix.to_string_lossy())?; let overlay_fs_path = self.overlay_fs_path()?; if overlay_fs_path.exists() { return Err(FpgadError::Argument(format!( - "Overlay with this handle already exists at {overlay_fs_path:?}. \ + "Overlay with this handle already exists at '{overlay_fs_path:?}'. \ Remove the overlay and try again." ))); } fs_create_dir(overlay_fs_path)?; - trace!("Created dir {overlay_fs_path:?}"); + trace!("Created dir '{overlay_fs_path:?}'"); let overlay_path_file = overlay_fs_path.join("path"); if !overlay_path_file.exists() { // TODO: consider different error type? return Err(FpgadError::Internal(format!( - "Overlay at {overlay_fs_path:?} did not initialise a new overlay: \ + "Overlay at '{overlay_fs_path:?}' did not initialise a new overlay: \ the `path` virtual file did not get created by the kernel. \ Is the parent dir mounted as a configfs directory?" ))); } - match fs_write(&overlay_path_file, false, source_path_rel.to_string_lossy()) { + match fs_write(&overlay_path_file, false, suffix.to_string_lossy()) { Ok(_) => { - trace!("'{source_path_rel:?}' successfully written to {overlay_path_file:?}"); + trace!("'{suffix:?}' successfully written to '{overlay_path_file:?}'"); } Err(e) => return Err(e), } - self.vfs_check_applied(source_path_rel) + self.vfs_check_applied(suffix.as_path())?; + + Ok(format!( + "'{:#?}' loaded to '{}' via '{:#?}' using firmware lookup path '{:?}'", + source_path, self.overlay_handle, overlay_fs_path, prefix + )) } /// Remove a device tree overlay from configfs. @@ -240,27 +252,20 @@ impl OverlayHandler for UniversalOverlayHandler { /// Removes the overlay directory from configfs, which deactivates the overlay and /// restores the original device tree state. /// - /// # Returns: `Result<(), FpgadError>` - /// * `Ok(())` - Overlay directory removed successfully - /// * `Err(FpgadError::IODelete)` - Failed to remove directory (not empty, doesn't exist, etc.) - fn remove_overlay(&self) -> Result<(), FpgadError> { - let overlay_fs_path = self.overlay_fs_path()?; - fs_remove_dir(overlay_fs_path) - } - - /// Get the required FPGA programming flags from the overlay. - /// - /// # Warning: Not Implemented + /// # Arguments /// - /// This method is intentionally unimplemented for the universal overlay handler. - /// In a platform specific implementation (i.e. a softener) this would implement - /// logic to parse the overlay/relevant package and determine any required FPGA - /// programming flags. + /// * `handle` - Optional handle/slot identifier (unused in universal implementation) /// - /// # Returns: `Result` - /// * `Ok(0)` - Always returns 0 (no flags) - fn required_flags(&self) -> Result { - Ok(0) + /// # Returns: `Result` + /// * `Ok(String)` - Confirmation message including overlay handle and filesystem path + /// * `Err(FpgadError::IODelete)` - Failed to remove directory (not empty, doesn't exist, etc.) + fn remove_overlay(&self, _: Option<&str>) -> Result { + let overlay_fs_path = self.overlay_fs_path()?; + fs_remove_dir(overlay_fs_path)?; + Ok(format!( + "'{}' removed by deleting '{:#?}'", + self.overlay_handle, overlay_fs_path + )) } /// Get the current status of the overlay. @@ -312,6 +317,7 @@ impl UniversalOverlayHandler { /// ``` pub(crate) fn new(overlay_handle: &str) -> Self { UniversalOverlayHandler { + overlay_handle: overlay_handle.to_string(), overlay_fs_path: construct_overlay_fs_path(overlay_handle), } } diff --git a/daemon/src/softeners/error.rs b/daemon/src/softeners/error.rs index 0f32b192..dbac44e3 100644 --- a/daemon/src/softeners/error.rs +++ b/daemon/src/softeners/error.rs @@ -25,11 +25,16 @@ //! Softener errors implement conversion traits to both `fdo::Error` (for DBus responses) //! and `FpgadError` (for internal error handling). All errors are logged when converted. +use crate::error::FpgadError; +use log::error; +use zbus::fdo; + /// Errors specific to FPGA softener implementations. /// /// This enum represents errors that occur in vendor-specific softener code, /// such as failures in communication with external tools or daemons (e.g., dfx-mgr-client). #[derive(Debug, thiserror::Error)] +#[non_exhaustive] pub enum FpgadSoftenerError { /// Error from Xilinx DFX Manager operations. /// @@ -37,5 +42,23 @@ pub enum FpgadSoftenerError { /// dfx-mgrd is not available or returns an error. The string contains /// detailed error information from the dfx-mgr tooling. #[error("FpgadSoftenerError::DfxMgr: {0}")] - DfxMgr(std::io::Error), + DfxMgr(String), +} + +impl From for fdo::Error { + fn from(err: FpgadSoftenerError) -> Self { + error!("{err}"); + match err { + FpgadSoftenerError::DfxMgr(..) => fdo::Error::Failed(err.to_string()), + } + } +} + +impl From for FpgadError { + fn from(err: FpgadSoftenerError) -> Self { + error!("{err}"); + match err { + FpgadSoftenerError::DfxMgr(..) => FpgadError::Softener(err), + } + } } diff --git a/daemon/src/softeners/xilinx_dfx_mgr.rs b/daemon/src/softeners/xilinx_dfx_mgr.rs index 47644c12..e0fdfab4 100644 --- a/daemon/src/softeners/xilinx_dfx_mgr.rs +++ b/daemon/src/softeners/xilinx_dfx_mgr.rs @@ -10,31 +10,109 @@ // // You should have received a copy of the GNU General Public License along with this program. If not, see http://www.gnu.org/licenses/. -use std::io; -use std::process::Command; +//! Xilinx DFX Manager platform implementation. +//! +//! This module provides the Xilinx DFX Manager (dfx-mgr) platform, which is a vendor-specific +//! "softener" implementation for managing Xilinx FPGA devices. It wraps the Xilinx dfx-mgr-client +//! command-line tool to provide enhanced functionality for Xilinx devices including: +//! - Dynamic function exchange (DFX) / partial reconfiguration +//! - Accelerator package management +//! - Multi-slot FPGA management +//! - UIO (User I/O) interface management +//! - Inter-region buffer management +//! +//! # Platform Support +//! +//! This platform registers itself for Xilinx device compatibility strings: +//! - `xlnx,zynqmp-pcap-fpga` - Zynq UltraScale+ MPSoC +//! - `versal-fpga` - Versal ACAP devices +//! - `zynq-devcfg-1.0` - Zynq-7000 devices +//! +//! # DFX Manager Integration +//! +//! The platform communicates with the dfx-mgrd daemon (started via snap daemon wrapper) +//! through the dfx-mgr-client CLI tool. The dfx-mgr-client binary must be available at +//! `$SNAP/usr/bin/dfx-mgr-client`. +//! +//! # Architecture +//! +//! The platform uses lazy initialization via `OnceLock` to create component instances: +//! - [`XilinxDfxMgrFPGA`] - Manages FPGA device operations via dfx-mgr +//! - [`XilinxDfxMgrOverlayHandler`] - Manages overlay operations with bitstream coordination +//! +//! # Feature Flag +//! +//! This module is only compiled when the `xilinx-dfx-mgr` feature is enabled. +//! +//! # Examples +//! +//! ```rust,no_run +//! # use daemon::platforms::platform::platform_for_known_platform; +//! # fn example() -> Result<(), daemon::error::FpgadError> { +//! let platform = platform_for_known_platform("xlnx,zynqmp-pcap-fpga")?; +//! let fpga = platform.fpga("fpga0")?; +//! # Ok(()) +//! # } +//! ``` + +use std::env; +use std::path::Path; use std::sync::OnceLock; -use log::trace; - -use crate::platforms::platform::Platform; -use crate::platforms::universal_components::universal_fpga::UniversalFPGA; -use crate::platforms::universal_components::universal_overlay_handler::UniversalOverlayHandler; +use crate::error::FpgadError; +use crate::platforms::platform::{Fpga, OverlayHandler, Platform}; use crate::softeners::error::FpgadSoftenerError; use fpgad_macros::platform; - +use log::trace; +use xilinx_dfx_mgr_fpga::XilinxDfxMgrFPGA; +use xilinx_dfx_mgr_overlay_handler::XilinxDfxMgrOverlayHandler; + +mod xilinx_dfx_mgr_fpga; +mod xilinx_dfx_mgr_helpers; +mod xilinx_dfx_mgr_overlay_handler; + +/// Xilinx DFX Manager platform implementation for managing Xilinx FPGA devices. +/// +/// This struct provides the platform implementation for Xilinx devices using the +/// dfx-mgr backend. It uses lazy initialization to create FPGA and overlay handler +/// instances on first access. +/// +/// The `#[platform]` macro automatically registers this platform with the Xilinx-specific +/// compatibility strings, making it available for matching against Xilinx device tree +/// compatible properties. +/// +/// # Fields +/// +/// * `fpga` - Lazily initialized Xilinx FPGA device instance +/// * `overlay_handler` - Lazily initialized Xilinx overlay handler instance +/// +/// # Thread Safety +/// +/// This struct is thread-safe thanks to `OnceLock`, which ensures that initialization +/// happens exactly once even with concurrent access. #[platform(compat_string = "xlnx,zynqmp-pcap-fpga,versal-fpga,zynq-devcfg-1.0")] pub struct XilinxDfxMgrPlatform { - fpga: OnceLock, - overlay_handler: OnceLock, -} - -impl Default for XilinxDfxMgrPlatform { - fn default() -> Self { - Self::new() - } + fpga: OnceLock, + overlay_handler: OnceLock, } impl XilinxDfxMgrPlatform { + /// Create a new Xilinx DFX Manager platform instance. + /// + /// Creates an empty platform with uninitialized FPGA and overlay handler instances. + /// The actual components will be lazily initialized on first access through the + /// [`Platform`] trait methods. + /// + /// # Returns: `Self` + /// * New XilinxDfxMgrPlatform instance ready for use + /// + /// # Examples + /// + /// ```rust,no_run + /// use daemon::softeners::xilinx_dfx_mgr::XilinxDfxMgrPlatform; + /// + /// let platform = XilinxDfxMgrPlatform::new(); + /// ``` pub fn new() -> Self { trace!("creating new XilinxDfxMgrPlatform"); XilinxDfxMgrPlatform { @@ -43,26 +121,30 @@ impl XilinxDfxMgrPlatform { } } } + impl Platform for XilinxDfxMgrPlatform { - fn fpga( - &self, - device_handle: &str, - ) -> Result<&dyn crate::platforms::platform::Fpga, crate::error::FpgadError> { - Ok(self.fpga.get_or_init(|| UniversalFPGA::new(device_handle))) + fn fpga(&self, device_handle: &str) -> Result<&dyn Fpga, FpgadError> { + Ok(self + .fpga + .get_or_init(|| XilinxDfxMgrFPGA::new(device_handle))) } - fn overlay_handler( - &self, - overlay_handle: &str, - ) -> Result<&dyn crate::platforms::platform::OverlayHandler, crate::error::FpgadError> { + fn overlay_handler(&self, overlay_handle: &str) -> Result<&dyn OverlayHandler, FpgadError> { Ok(self .overlay_handler - .get_or_init(|| UniversalOverlayHandler::new(overlay_handle))) + .get_or_init(|| XilinxDfxMgrOverlayHandler::new(overlay_handle))) + } + + fn status_message(&self) -> Result { + Ok(list_package()?) + } + + fn platform_compat_string(&self) -> String { + "xlnx,zynqmp-pcap-fpga,versal-fpga,zynq-devcfg-1.0".into() } } /// List locally downloaded accelerator packages -#[allow(dead_code)] pub fn list_package() -> Result { run_dfx_mgr(&["-listPackage"]) } @@ -75,93 +157,83 @@ pub fn load(accel_name: &str) -> Result { /// Unload package previously programmed #[allow(dead_code)] -pub fn remove(slot: u32) -> Result { - run_dfx_mgr(&["-remove", &slot.to_string()]) -} - -/// List accelerator UIOs -#[allow(dead_code)] -pub fn list_uio(slot: Option, uio_name: Option<&str>) -> Result { - let mut args = vec!["-listUIO"]; - if let Some(name) = uio_name { - args.push(name); - } - if let Some(slot) = slot { - let s_slot = slot.to_string(); - args.push(&s_slot); - run_dfx_mgr(&args) - } else { - run_dfx_mgr(&args) - } -} - -/// List inter-RM buffer info -#[allow(dead_code)] -pub fn list_irbuf(slot: Option) -> Result { - let mut args = vec!["-listIRbuf"]; - if let Some(slot) = slot { - let s_slot = slot.to_string(); - args.push(s_slot.as_str()); - run_dfx_mgr(&args) - } else { - run_dfx_mgr(&args) +pub fn remove(slot_handle: Option<&str>) -> Result { + match slot_handle { + Some(slot_handle) => Ok(run_dfx_mgr(&["-remove", slot_handle])?), + None => Ok(run_dfx_mgr(&["-remove"])?), } } -/// Set RM stream from slot a to b -#[allow(dead_code)] -pub fn set_irbuf(a: u32, b: u32) -> Result { - run_dfx_mgr(&["-setIRbuf", &format!("{a},{b}")]) -} - -/// Allocate buffer of size and return its DMA fd and pa -#[allow(dead_code)] -pub fn alloc_buffer(size: u64) -> Result { - run_dfx_mgr(&["-allocBuffer", &size.to_string()]) -} - -/// Free buffer with physical address pa in decimal -#[allow(dead_code)] -pub fn free_buffer(pa: u64) -> Result { - run_dfx_mgr(&["-freeBuffer", &pa.to_string()]) -} - -/// Send ip device FD's over socket -#[allow(dead_code)] -pub fn get_fds(slot: u32) -> Result { - run_dfx_mgr(&["-getFDs", &slot.to_string()]) -} - -/// Get RM info -#[allow(dead_code)] -pub fn get_rm_info() -> Result { - run_dfx_mgr(&["-getRMInfo"]) -} - -/// Get Shell FD -#[allow(dead_code)] -pub fn get_shell_fd() -> Result { - run_dfx_mgr(&["-getShellFD"]) -} - -/// Get Clock FD -#[allow(dead_code)] -pub fn get_clock_fd() -> Result { - run_dfx_mgr(&["-getClockFD"]) +/// Load a bitstream file using dfx-mgr +/// +/// # Arguments +/// +/// * `bitstream_path` - Path to the bitstream file to load +/// +/// # Returns: `Result` +/// * `Ok(String)` - Output from dfx-mgr-client +/// * `Err(FpgadSoftenerError::DfxMgr)` - Path contains invalid UTF-8 or dfx-mgr-client failed +pub fn load_bitstream(bitstream_path: &Path) -> Result { + let path_str = bitstream_path.to_str().ok_or_else(|| { + FpgadSoftenerError::DfxMgr(format!( + "Bitstream path contains invalid UTF-8: {}", + bitstream_path.display() + )) + })?; + run_dfx_mgr(&["-b", path_str]) +} + +/// Load an overlay with bitstream using dfx-mgr +/// +/// # Arguments +/// +/// * `bitstream_path` - Path to the bitstream file +/// * `dtbo_path` - Path to the device tree overlay file +/// +/// # Returns: `Result` +/// * `Ok(String)` - Output from dfx-mgr-client +/// * `Err(FpgadSoftenerError::DfxMgr)` - Path contains invalid UTF-8 or dfx-mgr-client failed +pub fn load_overlay(bitstream_path: &Path, dtbo_path: &Path) -> Result { + let bitstream_str = bitstream_path.to_str().ok_or_else(|| { + FpgadSoftenerError::DfxMgr(format!( + "Bitstream path contains invalid UTF-8: {}", + bitstream_path.display() + )) + })?; + + let dtbo_str = dtbo_path.to_str().ok_or_else(|| { + FpgadSoftenerError::DfxMgr(format!( + "DTBO path contains invalid UTF-8: {}", + dtbo_path.display() + )) + })?; + + run_dfx_mgr(&["-o", dtbo_str, "-b", bitstream_str]) } /// Helper to run the dfx-mgr-client binary with arguments fn run_dfx_mgr(args: &[&str]) -> Result { - let output = Command::new("sudo") - .arg("dfx-mgr-client") + let snap_env = env::var("SNAP").map_err(|e| { + FpgadSoftenerError::DfxMgr(format!( + "Cannot find dfx-mgr because $SNAP not specified in environment: {e}" + )) + })?; + + let dfx_mgr_client_path = format!("{}/usr/bin/dfx-mgr-client", snap_env); + trace!("Calling dfx-mgr with args {:#?}", args); + let output = std::process::Command::new(&dfx_mgr_client_path) .args(args) .output() - .map_err(FpgadSoftenerError::DfxMgr)?; + .map_err(|e| { + FpgadSoftenerError::DfxMgr(format!("dfx-mgr-client failed to produce output: {e}")) + })?; + if output.status.success() { Ok(String::from_utf8_lossy(&output.stdout).to_string()) } else { - Err(FpgadSoftenerError::DfxMgr(io::Error::other( - String::from_utf8_lossy(&output.stderr).to_string(), + Err(FpgadSoftenerError::DfxMgr(format!( + "dfx-mgr-client failed. Exit status: {}\nStdout:\n{:#?}\nStderr:\n{:#?}", + output.status, output.status, output.stderr ))) } } diff --git a/daemon/src/softeners/xilinx_dfx_mgr/xilinx_dfx_mgr_fpga.rs b/daemon/src/softeners/xilinx_dfx_mgr/xilinx_dfx_mgr_fpga.rs new file mode 100644 index 00000000..f5b59191 --- /dev/null +++ b/daemon/src/softeners/xilinx_dfx_mgr/xilinx_dfx_mgr_fpga.rs @@ -0,0 +1,188 @@ +// This file is part of fpgad, an application to manage FPGA subsystem together with device-tree and kernel modules. +// +// Copyright 2026 Canonical Ltd. +// +// SPDX-License-Identifier: GPL-3.0-only +// +// fpgad is free software: you can redistribute it and/or modify it under the terms of the GNU General Public License version 3, as published by the Free Software Foundation. +// +// fpgad is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; without even the implied warranties of MERCHANTABILITY, SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License along with this program. If not, see http://www.gnu.org/licenses/. + +//! Xilinx DFX Manager FPGA device implementation. +//! +//! This module provides the [`XilinxDfxMgrFPGA`] struct, which implements the [`Fpga`] trait +//! for Xilinx FPGA devices using the dfx-mgr backend. It provides a hybrid approach that: +//! - Uses standard sysfs for reading flags +//! - Uses dfx-mgr-client for bitstream loading and package management +//! - Supports dfx-mgr's slot-based management +//! +//! # Key Differences from Universal Platform +//! +//! - **State Query**: Returns dfx-mgr package listing instead of simple sysfs state +//! - **Bitstream Loading**: Uses `dfx-mgr-client -b` instead of direct firmware loading due to snap confinement +//! limitations (temporary?) +//! - **Firmware Removal**: Supports slot-based removal via `dfx-mgr-client -remove` +//! +//! # Examples +//! +//! ```rust,no_run +//! # use daemon::platforms::platform::platform_for_known_platform; +//! # fn example() -> Result<(), daemon::error::FpgadError> { +//! let platform = platform_for_known_platform("xlnx,zynqmp-pcap-fpga")?; +//! let fpga = platform.fpga("fpga0")?; +//! let state = fpga.state()?; // Returns dfx-mgr package listing +//! # Ok(()) +//! # } +//! ``` + +use crate::config; +use crate::error::FpgadError; +use crate::platforms::platform::Fpga; +use crate::softeners::xilinx_dfx_mgr; +use crate::system_io::{fs_read, fs_write}; +use log::{error, trace}; +use std::path::Path; + +/// Xilinx DFX Manager FPGA device implementation. +/// +/// This struct represents a Xilinx FPGA device and provides methods to interact +/// with it through the dfx-mgr backend. It stores only the device handle and +/// uses dfx-mgr-client for most operations. +/// +/// # Fields +/// +/// * `device_handle` - The device identifier (e.g., "fpga0") used to locate the device in sysfs. +/// Only used for sysfs backed functions e.g. flags +/// +/// # Implementation Notes +/// +/// While this implementation uses the dfx-mgr backend, it still reads flags directly +/// from sysfs for consistency with the standard FPGA subsystem interface. +pub struct XilinxDfxMgrFPGA { + device_handle: String, +} + +impl XilinxDfxMgrFPGA { + /// Create a new XilinxDfxMgrFPGA instance for the specified device. + /// + /// This constructor simply stores the device handle. It does not verify that + /// the device exists or that dfx-mgrd is running - validation occurs when + /// methods are called. + /// + /// # Arguments + /// + /// * `device_handle` - The device handle (e.g., "fpga0") + /// + /// # Returns: `Self` + /// * New XilinxDfxMgrFPGA instance + /// + /// # Examples + /// + /// ```rust,no_run + /// use daemon::softeners::xilinx_dfx_mgr_fpga::XilinxDfxMgrFPGA; + /// + /// let fpga = XilinxDfxMgrFPGA::new("fpga0"); + /// ``` + pub(crate) fn new(device_handle: &str) -> Self { + XilinxDfxMgrFPGA { + device_handle: device_handle.to_owned(), + } + } +} + +impl Fpga for XilinxDfxMgrFPGA { + /// Get the device handle for this FPGA. + /// + /// Returns the device handle (e.g., "fpga0") that identifies this FPGA in sysfs. + /// + /// # Returns: `&str` + /// * The device handle string + fn device_handle(&self) -> &str { + self.device_handle.as_str() + } + + /// Get the current state of the FPGA via dfx-mgr package listing. + /// + /// Returns the output of `dfx-mgr-client -listPackage`, which provides a formatted + /// table showing all available accelerator packages, their types, load status, and + /// slot assignments. + /// + /// # Returns: `Result` + /// * `Ok(String)` - Formatted package listing from dfx-mgr + /// * `Err(FpgadSoftenerError)` - Failed to communicate with dfx-mgrd or parse output + fn state(&self) -> Result { + Ok(xilinx_dfx_mgr::list_package()?) + } + + /// Read the current programming flags from sysfs. + /// + /// Reads `/sys/class/fpga_manager//flags`, parses the hexadecimal string + /// (format: "0x...", or undecorated), and returns the flags as u32. + /// + /// # Returns: `Result` + /// * `Ok(u32)` - Current flags value + /// * `Err(FpgadError::IORead)` - Failed to read flags file + /// * `Err(FpgadError::Flag)` - Failed to parse hexadecimal value + fn flags(&self) -> Result { + let flag_path = Path::new(config::FPGA_MANAGERS_DIR) + .join(self.device_handle.clone()) + .join("flags"); + let contents = fs_read(&flag_path)?; + let trimmed = contents.trim().trim_start_matches("0x"); + u32::from_str_radix(trimmed, 16) + .map_err(|_| FpgadError::Flag("Parsing flags failed".into())) + } + + /// Set the programming flags in sysfs. + /// + /// Writes the flags to `/sys/class/fpga_manager//flags` in undecorated + /// hexadecimal (decimal `32` -> undecorated hex `20`) and verifies that the write + /// succeeded by reading the value back. + /// Also checks and logs the FPGA state after setting flags. + /// + /// # Arguments + /// + /// * `flags` - The flags value to set + /// + /// # Returns: `Result<(), FpgadError>` + /// * `Ok(())` - Flags set and verified successfully + /// * `Err(FpgadError::IOWrite)` - Failed to write flags file + /// * `Err(FpgadError::IORead)` - Failed to read back flags or state + /// * `Err(FpgadError::Flag)` - Read-back value doesn't match written value + fn set_flags(&self, flags: u32) -> Result { + let flag_path = Path::new(config::FPGA_MANAGERS_DIR) + .join(self.device_handle.clone()) + .join("flags"); + trace!("Writing '0x{flags:X}' to '{flag_path:#?}'"); + if let Err(e) = fs_write(&flag_path, false, format!("0x{flags:X}")) { + error!("Failed to read state."); + return Err(e); + } + // TODO(Artie): how to check success when doing -listPackage + + match self.flags() { + Ok(returned_flags) if returned_flags == flags => Ok(format!( + "Flags set to '0x{:X}' for '{}'", + flags, self.device_handle + )), + Ok(returned_flags) => Err(FpgadError::Flag(format!( + "Setting '{}'s flags to '{}' failed. Resulting flag was '{}'", + self.device_handle, flags, returned_flags + ))), + Err(e) => Err(FpgadError::Flag(format!( + "Failed to read '{}'s flags after setting to '{}': {}", + self.device_handle, flags, e + ))), + } + } + + fn load_firmware(&self, firmware_path: &Path, _: &Path) -> Result { + Ok(xilinx_dfx_mgr::load_bitstream(firmware_path)?) + } + + fn remove_firmware(&self, slot_handle: Option<&str>) -> Result { + Ok(xilinx_dfx_mgr::remove(slot_handle)?) + } +} diff --git a/daemon/src/softeners/xilinx_dfx_mgr/xilinx_dfx_mgr_helpers.rs b/daemon/src/softeners/xilinx_dfx_mgr/xilinx_dfx_mgr_helpers.rs new file mode 100644 index 00000000..cba678db --- /dev/null +++ b/daemon/src/softeners/xilinx_dfx_mgr/xilinx_dfx_mgr_helpers.rs @@ -0,0 +1,131 @@ +// This file is part of fpgad, an application to manage FPGA subsystem together with device-tree and kernel modules. +// +// Copyright 2026 Canonical Ltd. +// +// SPDX-License-Identifier: GPL-3.0-only +// +// fpgad is free software: you can redistribute it and/or modify it under the terms of the GNU General Public License version 3, as published by the Free Software Foundation. +// +// fpgad is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; without even the implied warranties of MERCHANTABILITY, SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License along with this program. If not, see http://www.gnu.org/licenses/. + +//! Helper functions for Xilinx DFX Manager operations. +//! +//! This module provides utility functions for working with Xilinx device tree overlay +//! files and extracting information needed for dfx-mgr operations. +//! +//! # Key Functions +//! +//! - [`extract_firmware_name`] - Parses .dtbo files to extract the firmware-name property +//! +//! # Device Tree Parsing +//! +//! The module uses the `fdt` crate to parse flattened device tree binary (.dtbo) files +//! and extract properties. This is essential for coordinating bitstream and overlay loading +//! in the Xilinx dfx-mgr workflow. +//! +//! # Examples +//! +//! ```rust,no_run +//! # use std::path::Path; +//! # use daemon::softeners::xilinx_dfx_mgr_helpers::extract_firmware_name; +//! # fn example() -> Result<(), daemon::error::FpgadError> { +//! let firmware = extract_firmware_name(Path::new("/lib/firmware/design.dtbo"))?; +//! println!("Bitstream file: {}", firmware); +//! # Ok(()) +//! # } +//! ``` + +use crate::error::FpgadError; +use crate::softeners::error::FpgadSoftenerError; +use log::trace; +use std::fs; +use std::path::Path; + +/// Extract the firmware-name property from a device tree overlay (.dtbo) file +/// +/// # Arguments +/// +/// * `dtbo_path` - Path to the .dtbo file +/// +/// # Returns: `Result` +/// * `Ok(String)` - The firmware name extracted from the dtbo +/// * `Err(FpgadError)` - If the dtbo file cannot be read or firmware-name is not found +pub fn extract_firmware_name(dtbo_path: &Path) -> Result { + trace!("Extracting firmware-name from '{}'", dtbo_path.display()); + + // Read the dtbo file + let dtb_data = fs::read(dtbo_path).map_err(|e| { + FpgadSoftenerError::DfxMgr(format!( + "Failed to read dtbo file '{}': {}", + dtbo_path.display(), + e + )) + })?; + + // Parse the device tree + let fdt = fdt::Fdt::new(&dtb_data).map_err(|e| { + FpgadSoftenerError::DfxMgr(format!( + "Failed to parse dtbo file '{}': {:?}", + dtbo_path.display(), + e + )) + })?; + + // Search for firmware-name property in all nodes + for node in fdt.all_nodes() { + if let Some(firmware_name_prop) = node.property("firmware-name") { + // The property value is a null-terminated string, extract it + let value = firmware_name_prop.value; + + // Find the null terminator or use the entire value + let end = value.iter().position(|&b| b == 0).unwrap_or(value.len()); + let firmware_name = std::str::from_utf8(&value[..end]).map_err(|e| { + FpgadSoftenerError::DfxMgr(format!( + "Failed to parse firmware-name as UTF-8 string: {}", + e + )) + })?; + + trace!("Found firmware-name='{}' in dtbo", firmware_name); + return Ok(firmware_name.to_string()); + } + } + + Err(FpgadSoftenerError::DfxMgr(format!( + "`firmware-name` property not found in dtbo file '{}'", + dtbo_path.display() + )) + .into()) +} + +#[cfg(test)] +mod tests { + use crate::softeners::xilinx_dfx_mgr::xilinx_dfx_mgr_helpers::extract_firmware_name; + use std::path::PathBuf; + + #[test] + fn test_extract_firmware_name_k26() { + let test_dtbo = PathBuf::from(env!("CARGO_MANIFEST_DIR")) + .join("tests/test_data/k26-starter-kits/k26_starter_kits.dtbo"); + + if test_dtbo.exists() { + let result = extract_firmware_name(&test_dtbo); + assert!(result.is_ok()); + assert_eq!(result.unwrap(), "k26-starter-kits.bit.bin"); + } + } + + #[test] + fn test_extract_firmware_name_k24() { + let test_dtbo = PathBuf::from(env!("CARGO_MANIFEST_DIR")) + .join("tests/test_data/k24-starter-kits/k24_starter_kits.dtbo"); + + if test_dtbo.exists() { + let result = extract_firmware_name(&test_dtbo); + assert!(result.is_ok()); + assert_eq!(result.unwrap(), "k24-starter-kits.bit.bin"); + } + } +} diff --git a/daemon/src/softeners/xilinx_dfx_mgr/xilinx_dfx_mgr_overlay_handler.rs b/daemon/src/softeners/xilinx_dfx_mgr/xilinx_dfx_mgr_overlay_handler.rs new file mode 100644 index 00000000..b3e44d32 --- /dev/null +++ b/daemon/src/softeners/xilinx_dfx_mgr/xilinx_dfx_mgr_overlay_handler.rs @@ -0,0 +1,130 @@ +// This file is part of fpgad, an application to manage FPGA subsystem together with device-tree and kernel modules. +// +// Copyright 2026 Canonical Ltd. +// +// SPDX-License-Identifier: GPL-3.0-only +// +// fpgad is free software: you can redistribute it and/or modify it under the terms of the GNU General Public License version 3, as published by the Free Software Foundation. +// +// fpgad is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; without even the implied warranties of MERCHANTABILITY, SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License along with this program. If not, see http://www.gnu.org/licenses/. + +//! Xilinx DFX Manager overlay handler implementation. +//! +//! This module provides the [`XilinxDfxMgrOverlayHandler`] struct, which implements the +//! [`OverlayHandler`] trait for managing device tree overlays on Xilinx FPGA devices +//! using the dfx-mgr backend. +//! +//! # Key Features +//! +//! - **Slot-based Management**: Uses dfx-mgr's slot system for overlay tracking and removal +//! +//! # Overlay Loading Process +//! +//! 1. Parse the .dtbo file to extract the `firmware-name` property +//! 2. Locate the bitstream file in the same directory as the .dtbo +//! 3. Call `dfx-mgr-client -o -b ` to load both +//! +//! This ensures the bitstream and overlay are applied together, which is required +//! as a temporary workaround while `-load` is not supported due to snap confinement limitations +//! +//! # Examples +//! +//! ```rust,no_run +//! # use daemon::platforms::platform::platform_for_known_platform; +//! # use std::path::Path; +//! # fn example() -> Result<(), daemon::error::FpgadError> { +//! let platform = platform_for_known_platform("xlnx,zynqmp-pcap-fpga")?; +//! let handler = platform.overlay_handler("my_overlay")?; +//! handler.apply_overlay(Path::new("/lib/firmware/design.dtbo"), Path::new(""))?; +//! # Ok(()) +//! # } +//! ``` + +use crate::error::FpgadError; +use crate::platforms::platform::OverlayHandler; +use crate::softeners::error::FpgadSoftenerError; +use crate::softeners::xilinx_dfx_mgr; +use crate::softeners::xilinx_dfx_mgr::xilinx_dfx_mgr_helpers; +use crate::system_io; +use log::{debug, trace}; +use std::option::Option; +use std::path::Path; + +/// Xilinx DFX Manager overlay handler implementation. +/// +/// This struct provides overlay management for Xilinx FPGA devices using the +/// dfx-mgr backend. Unlike the universal overlay handler, it doesn't directly +/// manage configfs overlay directories since dfx-mgr handles that internally. +/// +/// # Implementation Notes +/// +/// The overlay handle parameter is currently unused since dfx-mgr manages +/// overlay lifecycle internally through its slot system. +pub struct XilinxDfxMgrOverlayHandler {} + +impl XilinxDfxMgrOverlayHandler { + /// Create a new XilinxDfxMgrOverlayHandler instance. + /// + /// # Arguments + /// + /// * `_overlay_handle` - Overlay handle (currently unused, slot management WIP) + /// + /// # Returns: `Self` + /// * New XilinxDfxMgrOverlayHandler instance + /// + /// # Examples + /// + /// ```rust,no_run + /// use daemon::softeners::xilinx_dfx_mgr_overlay_handler::XilinxDfxMgrOverlayHandler; + /// + /// let handler = XilinxDfxMgrOverlayHandler::new(""); + /// ``` + pub(crate) fn new(_: &str) -> Self { + XilinxDfxMgrOverlayHandler {} + } +} +impl OverlayHandler for XilinxDfxMgrOverlayHandler { + fn apply_overlay(&self, source_path: &Path, lookup_path: &Path) -> Result { + trace!( + "apply_overlay called with source_path='{}', lookup_path='{}'", + source_path.display(), + lookup_path.display() + ); + let (parent_dir, _) = system_io::extract_path_and_filename(source_path)?; + // Extract firmware-name from the .dtbo file using fdtdump + let firmware_name = xilinx_dfx_mgr_helpers::extract_firmware_name(source_path)?; + debug!("Extracted firmware-name='{}' from dtbo", firmware_name); + + // Construct the bitstream path in the lookup_path + let bitstream_path = parent_dir.join(&firmware_name); + + // Verify the bitstream file exists + if !bitstream_path.exists() { + return Err(FpgadSoftenerError::DfxMgr(format!( + "Bitstream file '{}' not found in lookup path '{}'", + firmware_name, + parent_dir.display() + )) + .into()); + } + + trace!("Found bitstream at '{}'", bitstream_path.display()); + + // Call dfx-mgr with -o -b + xilinx_dfx_mgr::load_overlay(&bitstream_path, source_path).map_err(|e| e.into()) + } + + fn remove_overlay(&self, slot_handle: Option<&str>) -> Result { + Ok(xilinx_dfx_mgr::remove(slot_handle)?) + } + + fn status(&self) -> Result { + Ok(xilinx_dfx_mgr::list_package()?) + } + + fn overlay_fs_path(&self) -> Result<&Path, FpgadError> { + Err(FpgadSoftenerError::DfxMgr("Not Applicable".to_string()).into()) + } +} diff --git a/daemon/src/system_io.rs b/daemon/src/system_io.rs index 80e073e1..60a00c16 100644 --- a/daemon/src/system_io.rs +++ b/daemon/src/system_io.rs @@ -39,7 +39,7 @@ use log::trace; use std::fs::OpenOptions; use std::fs::{create_dir_all, remove_dir}; use std::io::{Read, Write}; -use std::path::Path; +use std::path::{Component, Path, PathBuf}; /// Read the contents of a file to a String. /// @@ -318,3 +318,147 @@ pub fn fs_read_dir(dir: &Path) -> Result, FpgadError> { }, ) } + +/// Helper function to find the overlap between two paths and to return a tuple of the overlap and +/// the difference. Note: the paths must both share the same root otherwise no overlap will be found +/// +/// Typically, this is used to create a fw_lookup_path and a corresponding relative path which points +/// to the source file +/// +/// # Arguments +/// +/// * `source_path`: the full path to the target file (or containing directory?) +/// * `firmware_path`: the root common path for all files to be loaded by the FW subsystem +/// +/// # Returns: `Result<(PathBuf, PathBuf), FpgadError>` +/// * `(PathBuf, PathBuf)` - A tuple of (prefix, suffix) where prefix is +/// typically used as the fw_lookup_path and the suffix is remaining relative path from that prefix +/// * `FpgadError::Argument` in case `firmware_path` is not within `source_path`, or for inputs +/// resulting in an empty suffix value +/// # Examples +/// +/// ``` +/// #use std::path::Path; +/// let (prefix, suffix) = make_firmware_pair( +/// Path::new("/lib/firmware/file.bin"), +/// Path::new("/lib/firmware/"), +/// )?; +/// assert_eq!(prefix, "/lib/firmware"); +/// assert_eq!(suffix, "file.bin"); +/// ``` +pub(crate) fn make_firmware_pair( + source_path: &Path, + firmware_path: &Path, +) -> Result<(PathBuf, PathBuf), FpgadError> { + // No firmware search path provided, so just try to use parent dir + if firmware_path.as_os_str().is_empty() { + return extract_path_and_filename(source_path); + } + if let Ok(suffix) = source_path.strip_prefix(firmware_path) { + // Remove leading '/' if present + let cleaned_suffix_path = suffix + .components() + .skip_while(|c| matches!(c, Component::RootDir)) + .collect::(); + if cleaned_suffix_path.as_os_str().is_empty() { + return Err(FpgadError::Argument(format!( + "The resulting filename from stripping {firmware_path:?} from {source_path:?} \ + was empty. Cannot write empty string to fpga." + ))); + } + Ok((firmware_path.to_path_buf(), cleaned_suffix_path)) + } else { + Err(FpgadError::Argument(format!( + "Could not find {source_path:?} inside {firmware_path:?}" + ))) + } +} + +/// Splits a Path object into its parent directory and basename/filename +/// +/// # Arguments +/// +/// * `path`: path to be split +/// +/// # Returns: `Result<(PathBuf, PathBuf), FpgadError>` +/// * `(PathBuf, PathBuf)` - Tuple of parent directory and basename/filename +/// * `FpgadError::Argument` on invalid `path` or `path` is a root directory (no parent) +/// # Examples +/// +/// ```rust +/// let (parent, base) = extract_path_and_filename(Path::new("/lib/firmware/file.bin")); +/// assert_eq!(parent.to_string_lossy(), "/lib/firmware"); +/// assert_eq!(base.to_string_lossy(), "file.bin"); +/// ``` +pub fn extract_path_and_filename(path: &Path) -> Result<(PathBuf, PathBuf), FpgadError> { + // Extract filename + let filename = path + .file_name() + .and_then(|f| f.to_str()) + .ok_or(FpgadError::Argument(format!( + "Provided bitstream path {path:?} is not a file or a valid directory." + )))?; + + // Extract parent directory + let base_path = path + .parent() + .and_then(|p| p.to_str()) + .ok_or(FpgadError::Argument(format!( + "Provided bitstream path {path:?} is missing a parent dir." + )))?; + + Ok((base_path.into(), filename.into())) +} + +#[cfg(test)] +mod test_make_firmware_pair { + use crate::error::FpgadError; + use crate::system_io::make_firmware_pair; + use googletest::prelude::*; + use rstest::*; + use std::path::PathBuf; + + #[gtest] + #[rstest] + #[case::all_good( + "/lib/firmware/file.bin", + "/lib/firmware/", + "/lib/firmware/", + "file.bin" + )] + #[case::no_fw_path("/lib/firmware/file.bin", "", "/lib/firmware/", "file.bin")] + #[case::no_fw_path_no_file("/lib/firmware/", "", "/lib/", "firmware")] + fn should_pass( + #[case] source: &str, + #[case] fw_path: &str, + #[case] exp_prefix: &str, + #[case] exp_suffix: &str, + ) { + let result = make_firmware_pair(&PathBuf::from(source), &PathBuf::from(fw_path)); + assert_that!( + result, + ok(eq(&(PathBuf::from(exp_prefix), PathBuf::from(exp_suffix)))) + ); + } + + #[gtest] + #[rstest] + #[case::no_file( + "/lib/firmware/", + "/lib/firmware/", + err(displays_as(contains_substring("The resulting filename from stripping"))) + )] + #[case::not_in_dir( + "/lib/firmware/file.bin", + "/snap/x1/data/file.bin", + err(displays_as(contains_substring("Could not find"))) + )] + fn should_fail Matcher<&'a std::result::Result<(PathBuf, PathBuf), FpgadError>>>( + #[case] source: &str, + #[case] fw_path: &str, + #[case] condition: M, + ) { + let result = make_firmware_pair(&PathBuf::from(source), &PathBuf::from(fw_path)); + assert_that!(&result, condition); + } +} diff --git a/daemon/tests/universal/control/write_bitstream_direct.rs b/daemon/tests/universal/control/write_bitstream_direct.rs index 6c535442..1389954b 100644 --- a/daemon/tests/universal/control/write_bitstream_direct.rs +++ b/daemon/tests/universal/control/write_bitstream_direct.rs @@ -165,6 +165,6 @@ async fn should_timeout( ) .await; if let Ok(res) = timeout_result { - panic!("Timeout not reached when expecting a timeout to occur: {res:?}"); + panic!("Timeout not reached when expecting a timeout to occur: {res:#?}"); } } diff --git a/snap/assets/softener_wrapper b/snap/assets/softener_wrapper new file mode 100755 index 00000000..077dfe41 --- /dev/null +++ b/snap/assets/softener_wrapper @@ -0,0 +1,250 @@ +#!/usr/bin/env python3 +""" +Daemon wrapper for managing external daemons required by fpgad. + +This script detects, starts, and monitors external daemon processes, +keeping them alive as long as the wrapper is running. +""" + +import os +import sys +import time +import stat +import subprocess +import signal +from pathlib import Path +from typing import Optional, List, Dict +from dataclasses import dataclass + + +@dataclass +class DaemonConfig: + """Configuration for a softener daemon.""" + + name: str + binary_path: str + socket_path: str + start_timeout: int = 10 + log_file: Optional[str] = None + + def __post_init__(self): + """Set default log file if not provided.""" + if self.log_file is None: + snap_common = os.environ.get("SNAP_COMMON", "/tmp") + log_dir = Path(snap_common) / "log" + log_dir.mkdir(parents=True, exist_ok=True) + self.log_file = str(log_dir / f"{self.name}.log") + + +class DaemonManager: + """Manages lifecycle of softener daemons.""" + + def __init__(self, daemons: List[DaemonConfig]): + """Initialize with list of daemons to manage.""" + self.daemons: List[DaemonConfig] = daemons + self.processes: Dict[str, subprocess.Popen] = {} + self.running = True + + # Set up signal handlers + signal.signal(signal.SIGTERM, self._signal_handler) + signal.signal(signal.SIGINT, self._signal_handler) + + def _signal_handler(self, signum, frame): + """Handle termination signals.""" + print(f"\nReceived signal {signum}, shutting down...") + self.running = False + + def filter_available_daemons(self) -> List[DaemonConfig]: + """Filter configured daemons to only those with available binaries.""" + available = [] + + for daemon in self.daemons: + if os.path.isfile(daemon.binary_path): + print(f"Detected {daemon.name} at {daemon.binary_path}") + available.append(daemon) + else: + print(f"{daemon.name} not found at {daemon.binary_path}, skipping") + + return available + + def _cleanup_stale_socket(self, socket_path: str): + """Remove stale socket file if it exists.""" + if os.path.exists(socket_path): + try: + # Check if it's a socket + mode = os.stat(socket_path).st_mode + if stat.S_ISSOCK(mode): + print(f"Removing stale socket at {socket_path}") + os.unlink(socket_path) + except Exception as e: + print(f"Warning: Could not remove stale socket: {e}") + + def start_daemon(self, daemon: DaemonConfig) -> bool: + """Start a daemon and wait for its socket to appear.""" + print(f"Starting {daemon.name}...") + + # Clean up any stale socket + self._cleanup_stale_socket(daemon.socket_path) + + try: + # Open log file + log_file = open(daemon.log_file, "w") + + # Start the daemon process + process = subprocess.Popen( + [daemon.binary_path], + stdout=log_file, + stderr=subprocess.STDOUT, + start_new_session=True, # Detach from parent session + ) + + self.processes[daemon.name] = process + print(f"{daemon.name} started with PID {process.pid}") + print(f"Logs will be written to: {daemon.log_file}") + + # Wait for socket to appear + print(f"Waiting for socket at {daemon.socket_path}...") + for elapsed in range(daemon.start_timeout): + # Check if socket exists + if os.path.exists(daemon.socket_path): + mode = os.stat(daemon.socket_path).st_mode + if stat.S_ISSOCK(mode): + print(f"{daemon.name} socket detected - startup successful") + return True + + # Check if process is still running + if process.poll() is not None: + print(f"Error: {daemon.name} process terminated unexpectedly") + print(f"Check logs at: {daemon.log_file}") + self._print_log_tail(daemon.log_file) + return False + + time.sleep(1) + + # Timeout reached + if not os.path.exists(daemon.socket_path): + print( + f"Warning: Socket didn't appear after {daemon.start_timeout}s, " + f"but {daemon.name} is running" + ) + + return True + + except Exception as e: + print(f"Error starting {daemon.name}: {e}") + return False + + def _print_log_tail(self, log_file: str, lines: int = 10): + """Print the last N lines of a log file.""" + try: + with open(log_file, "r") as f: + tail_lines = f.readlines()[-lines:] + if tail_lines: + print(f"Last {lines} lines from log:") + print("".join(tail_lines)) + except Exception as e: + print(f"Could not read log file: {e}") + + def check_and_start_daemons(self) -> bool: + """Start all detected daemons.""" + available = self.filter_available_daemons() + + if not available: + print("No daemons to start") + return True + + success = True + for daemon in available: + if not self.start_daemon(daemon): + success = False + + return success + + def monitor_daemons(self): + """Monitor daemon processes and keep them alive.""" + if not self.processes: + print("No daemons to monitor") + return + + print(f"Monitoring {len(self.processes)} daemon(s)...") + + while self.running: + for name, process in list(self.processes.items()): + retcode = process.poll() + if retcode is not None: + print( + f"Error: {name} process died unexpectedly with exit code {retcode}" + ) + # Find the daemon config + daemon = next((d for d in self.daemons if d.name == name), None) + if daemon: + self._print_log_tail(daemon.log_file) + self.running = False + return + + time.sleep(5) + + def cleanup(self): + """Clean up all managed processes.""" + print("Cleaning up daemon processes...") + for name, process in self.processes.items(): + if process.poll() is None: # Still running + print(f"Terminating {name} (PID {process.pid})...") + process.terminate() + try: + process.wait(timeout=5) + except subprocess.TimeoutExpired: + print(f"Force killing {name}...") + process.kill() + + +# ============================================================================ +# Daemon Configuration - Populated at import time +# ============================================================================ + +SNAP_PATH = os.environ.get("SNAP", "") + +# Define all daemons to manage +MANAGED_DAEMONS = [ + DaemonConfig( + name="dfx-mgrd", + binary_path=f"{SNAP_PATH}/usr/bin/dfx-mgrd", + socket_path="/run/dfx-mgrd.socket", + start_timeout=10, + ), + # Add more daemon configurations here as needed + # DaemonConfig( + # name='another-daemon', + # binary_path=f'{SNAP_PATH}/usr/bin/another-daemon', + # socket_path='/run/another-daemon.socket', + # start_timeout=15 + # ), +] + + +def main(): + """Main entry point.""" + # Initialize manager with all configured daemons + manager = DaemonManager(MANAGED_DAEMONS) + + try: + # Start all daemons + if not manager.check_and_start_daemons(): + print("Failed to start one or more daemons") + sys.exit(1) + + # Monitor daemons until termination + manager.monitor_daemons() + + except Exception as e: + print(f"Unexpected error: {e}") + sys.exit(1) + finally: + manager.cleanup() + + print("Daemon wrapper exiting") + sys.exit(0) + + +if __name__ == "__main__": + main() diff --git a/snap/hooks/install b/snap/hooks/install new file mode 100644 index 00000000..00f1a1e8 --- /dev/null +++ b/snap/hooks/install @@ -0,0 +1,13 @@ +#!/bin/sh +set -eu + +CONF_DIR="$SNAP_COMMON/etc/dfx-mgrd" +CONF_FILE="$CONF_DIR/daemon.conf" +DEFAULT_CONF="$SNAP/etc/dfx-mgrd/daemon.conf" + +mkdir -p "$CONF_DIR" + +if [ ! -f "$CONF_FILE" ]; then + echo "Initializing default daemon.conf" + cp "$DEFAULT_CONF" "$CONF_FILE" +fi \ No newline at end of file diff --git a/snap/hooks/post-refresh b/snap/hooks/post-refresh new file mode 100644 index 00000000..8e92d03a --- /dev/null +++ b/snap/hooks/post-refresh @@ -0,0 +1,13 @@ +#!/bin/sh +set -eu + +CONF_DIR="$SNAP_COMMON/etc/dfx-mgrd" +CONF_FILE="$CONF_DIR/daemon.conf" +DEFAULT_CONF="$SNAP/etc/dfx-mgrd/daemon.conf" + +mkdir -p "$CONF_DIR" + +if [ ! -f "$CONF_FILE" ]; then + echo "Restoring default daemon.conf" + cp "$DEFAULT_CONF" "$CONF_FILE" +fi \ No newline at end of file diff --git a/snap/snapcraft.yaml b/snap/snapcraft.yaml index 5cb6d80e..884f9517 100644 --- a/snap/snapcraft.yaml +++ b/snap/snapcraft.yaml @@ -12,6 +12,9 @@ source-code: license: GPL-3.0 issues: - https://github.com/canonical/fpgad/issues +package-repositories: + - type: apt + ppa: ubuntu-xilinx/default platforms: amd64: build-on: [amd64] @@ -38,18 +41,23 @@ plugs: name: com.canonical.fpgad provider-content: interface: content + content: provider-content target: $SNAP_DATA/provider-content device-tree-overlays: interface: system-files - read: - - /sys/kernel/config/device-tree/overlays write: - /sys/kernel/config/device-tree/overlays + dfx-mgr-socket: + interface: system-files + write: + - /run/dfx-mgrd.socket apps: fpgad: command: bin/fpgad_cli plugs: - cli-dbus + aliases: + - cli daemon: command: bin/fpgad daemon: dbus @@ -62,8 +70,21 @@ apps: - fpga - kernel-firmware-control - hardware-observe + - dfx-mgr-socket activates-on: - daemon-dbus + environment: + RUST_LOG: trace + softeners: + command: bin/softener_wrapper + daemon: simple + restart-condition: on-failure + plugs: + - fpga + - kernel-firmware-control + - hardware-observe + - dfx-mgr-socket + - network-bind parts: version: plugin: nil @@ -90,3 +111,28 @@ parts: rust-path: - daemon rust-channel: 'stable' + dfx-mgr: + plugin: nil + stage-packages: + - dfx-mgr + override-stage: | + craftctl default + cat << 'EOF' > $SNAPCRAFT_PART_INSTALL/etc/dfx-mgrd/daemon.conf + { + "firmware_location": [ + "/var/snap/fpgad/current/provider-content/" + ] + } + EOF + softener_wrapper: + plugin: dump + source: snap/assets + organize: + softener_wrapper: bin/softener_wrapper + stage: + - bin/softener_wrapper +# TODO: the daemon.conf should be copied into $SNAP_COMMON on install hook if - and only if - it doesn't already exist +layout: + # For configuration files + /etc/dfx-mgrd: + bind: $SNAP_COMMON/etc/dfx-mgrd