From c12b86dc8258ad9139d52044ca839fa57b659016 Mon Sep 17 00:00:00 2001 From: David Gibson Date: Sun, 13 Dec 2020 17:24:32 +1100 Subject: [PATCH] agent/device: Generalize PCI path resolution to any number of bridges Currently pcipath_to_sysfs(), which translates PCI paths into sysfs paths accepts only pci paths with exactly 2 components; which represents PCI devices separated from the root bus by exactly one PCI to PCI bridge (which could be a virtual P2P bridge, such as a PCI-E root port). There are cases we might reasonably want to support which have devices either plugged directly into the root bus (zero bridges), or under multiple layers of P2P bridge (a PCI-E switch would require at least 2 layers). So, generalize pcipath_to_sysfs to support any number of components in the PCI path. We also make it use the new type for PCI paths internally rather than plain strings. This is a loose forward port of https://github.com/kata-containers/agent/pull/855/commits/9804b1e55d1eae3a5b47ede6c2f40c401af25611 fixes #1040 Signed-off-by: David Gibson --- src/agent/src/device.rs | 88 ++++++++++++++++---------------------- src/agent/src/linux_abi.rs | 1 - 2 files changed, 38 insertions(+), 51 deletions(-) diff --git a/src/agent/src/device.rs b/src/agent/src/device.rs index d1a575868e..7fd66f9109 100644 --- a/src/agent/src/device.rs +++ b/src/agent/src/device.rs @@ -9,11 +9,13 @@ use std::collections::HashMap; use std::fs; use std::os::unix::fs::MetadataExt; use std::path::Path; +use std::str::FromStr; use std::sync::Arc; use tokio::sync::Mutex; use crate::linux_abi::*; use crate::mount::{DRIVER_BLK_TYPE, DRIVER_MMIO_BLK_TYPE, DRIVER_NVDIMM_TYPE, DRIVER_SCSI_TYPE}; +use crate::pci; use crate::sandbox::Sandbox; use crate::{AGENT_CONFIG, GLOBAL_DEVICE_WATCHER}; use anyhow::{anyhow, Result}; @@ -45,60 +47,46 @@ pub fn online_device(path: &str) -> Result<()> { Ok(()) } -// pcipath_to_sysfs fetches the sysfs path for a PCI device, relative -// to the syfs path for the PCI host bridge, based on the PCI path -// provided. The path should be in the format "bridgeAddr/deviceAddr", -// where bridgeAddr is the address at which the brige is attached on -// the root bus, while deviceAddr is the address at which the device -// is attached on the bridge. +// pciPathToSysfs fetches the sysfs path for a PCI path, relative to +// the syfs path for the PCI host bridge, based on the PCI path +// provided. fn pcipath_to_sysfs(pcipath: &str) -> Result { - let tokens: Vec<&str> = pcipath.split('/').collect(); + let pcipath = pci::Path::from_str(pcipath)?; + let mut bus = "0000:00".to_string(); + let mut relpath = String::new(); + let root_bus_sysfs = format!("{}{}", SYSFS_DIR, create_pci_root_bus_path()); - if tokens.len() != 2 { - return Err(anyhow!( - "PCI path for device should be of format [bridgeAddr/deviceAddr], got {:?}", - pcipath - )); + for i in 0..pcipath.len() { + let bdf = format!("{}:{}.0", bus, pcipath[i]); + + relpath = format!("{}/{}", relpath, bdf); + + if i == pcipath.len() - 1 { + // Final device need not be a bridge + break; + } + + // Find out the bus exposed by bridge + let bridgebuspath = format!("{}{}/pci_bus", root_bus_sysfs, relpath); + let mut files: Vec<_> = fs::read_dir(&bridgebuspath)?.collect(); + + if files.len() != 1 { + return Err(anyhow!( + "Expected exactly one PCI bus in {}, got {} instead", + bridgebuspath, + files.len() + )); + } + + // unwrap is safe, because of the length test above + let busfile = files.pop().unwrap()?; + bus = busfile + .file_name() + .into_string() + .map_err(|e| anyhow!("Bad filename under {}: {:?}", &bridgebuspath, e))?; } - let bridge_id = tokens[0]; - let device_id = tokens[1]; - - // Deduce the complete bridge address based on the bridge address identifier passed - // and the fact that bridges are attached on the main bus with function 0. - let pci_bridge_addr = format!("0000:00:{}.0", bridge_id); - - // Find out the bus exposed by bridge - let bridge_bus_path = format!("{}/{}/pci_bus/", SYSFS_PCI_BUS_PREFIX, pci_bridge_addr); - - let files_slice: Vec<_> = fs::read_dir(&bridge_bus_path) - .unwrap() - .map(|res| res.unwrap().path()) - .collect(); - let bus_num = files_slice.len(); - - if bus_num != 1 { - return Err(anyhow!( - "Expected an entry for bus in {}, got {} entries instead", - bridge_bus_path, - bus_num - )); - } - - let bus = files_slice[0].file_name().unwrap().to_str().unwrap(); - - // Device address is based on the bus of the bridge to which it is attached. - // We do not pass devices as multifunction, hence the trailing 0 in the address. - let pci_device_addr = format!("{}:{}.0", bus, device_id); - - let sysfs_rel_path = format!("{}/{}", pci_bridge_addr, pci_device_addr); - - info!( - sl!(), - "Fetched sysfs relative path for PCI device {}\n", sysfs_rel_path - ); - - Ok(sysfs_rel_path) + Ok(relpath) } async fn get_device_name(sandbox: &Arc>, dev_addr: &str) -> Result { diff --git a/src/agent/src/linux_abi.rs b/src/agent/src/linux_abi.rs index c4a08376b6..c6e52fa9e9 100644 --- a/src/agent/src/linux_abi.rs +++ b/src/agent/src/linux_abi.rs @@ -9,7 +9,6 @@ use std::fs; pub const SYSFS_DIR: &str = "/sys"; -pub const SYSFS_PCI_BUS_PREFIX: &str = "/sys/bus/pci/devices"; pub const SYSFS_PCI_BUS_RESCAN_FILE: &str = "/sys/bus/pci/rescan"; #[cfg(any( target_arch = "powerpc64",