From c315d5fd96dd939dc137c0692c2b393fd9dc78c6 Mon Sep 17 00:00:00 2001 From: Bo Chen Date: Fri, 24 Apr 2026 22:57:43 +0000 Subject: [PATCH] vmm: Enable FD-based VFIO devices Signed-off-by: Bo Chen Assisted-by: Claude:Opus-4.7 --- vmm/src/config.rs | 106 ++++++++++++++++++++++++++++++++++---- vmm/src/device_manager.rs | 103 +++++++++++++++++++++++++++++++++--- vmm/src/vm_config.rs | 9 ++-- 3 files changed, 196 insertions(+), 22 deletions(-) diff --git a/vmm/src/config.rs b/vmm/src/config.rs index aa7136092..613016a77 100644 --- a/vmm/src/config.rs +++ b/vmm/src/config.rs @@ -148,9 +148,6 @@ pub enum Error { /// Failed parsing device parameters #[error("Error parsing --device")] ParseDevice(#[source] OptionParserError), - /// Missing path from device, - #[error("Error parsing --device: path missing")] - ParseDevicePathMissing, /// Failed parsing vsock parameters #[error("Error parsing --vsock")] ParseVsock(#[source] OptionParserError), @@ -346,6 +343,15 @@ pub enum ValidationError { /// Duplicated device path (device added twice) #[error("Duplicated device path: {0}")] DuplicateDevicePath(String), + /// A DeviceConfig specified neither `path` nor `fd`. + #[error("VFIO device config must specify `path=` or `fd=`")] + VfioDeviceNeitherPathNorFd, + /// A DeviceConfig specified both `path` and `fd`. + #[error("VFIO device config must specify at most one of `path=` or `fd=`")] + VfioDeviceBothPathAndFd, + /// FD-based VFIO requires the `iommufd` platform option to be enabled. + #[error("VFIO device `fd=` requires platform `iommufd=on`")] + VfioFdRequiresIommufd, /// Provided MTU is lower than what the VIRTIO specification expects #[error("Provided MTU {0} is lower than 1280 (expected by VIRTIO specification)")] InvalidMtu(u16), @@ -2429,10 +2435,7 @@ impl DeviceConfig { parser.parse(device).map_err(Error::ParseDevice)?; let pci_common = PciDeviceCommonConfig::parse(device)?; - let path = parser - .get("path") - .map(PathBuf::from) - .ok_or(Error::ParseDevicePathMissing)?; + let path = parser.get("path").map(PathBuf::from); let fd = parser.convert::("fd").map_err(Error::ParseDevice)?; let x_nv_gpudirect_clique = parser .convert::("x_nv_gpudirect_clique") @@ -2444,7 +2447,7 @@ impl DeviceConfig { .unwrap_or_default(); Ok(DeviceConfig { pci_common, - path: Some(path), + path, fd, x_nv_gpudirect_clique, x_exclude_mmap_bars, @@ -2454,6 +2457,18 @@ impl DeviceConfig { pub fn validate(&self, vm_config: &VmConfig) -> ValidationResult<()> { self.pci_common.validate(vm_config)?; + match (&self.path, self.fd) { + (None, None) => return Err(ValidationError::VfioDeviceNeitherPathNorFd), + (Some(_), Some(_)) => return Err(ValidationError::VfioDeviceBothPathAndFd), + (None, Some(_)) => { + let iommufd_on = vm_config.platform.as_ref().is_some_and(|p| p.iommufd); + if !iommufd_on { + return Err(ValidationError::VfioFdRequiresIommufd); + } + } + (Some(_), None) => {} + } + if self.x_nv_gpudirect_clique.is_some() { let vfio_p2p_dma = vm_config.platform.as_ref().is_none_or(|p| p.vfio_p2p_dma); if !vfio_p2p_dma { @@ -4733,8 +4748,15 @@ id=\"{id}\",pci_segment={pci_segment},queue_sizes={queue_sizes}" #[test] fn test_device_parsing() -> Result<()> { - // Device must have a path provided - DeviceConfig::parse("").unwrap_err(); + // The parser itself is purely syntactic; the "path or fd is + // required" rule is enforced by VmConfig::validate instead. + assert_eq!( + DeviceConfig::parse("")?, + DeviceConfig { + path: None, + ..device_fixture() + } + ); assert_eq!( DeviceConfig::parse("path=/path/to/device")?, device_fixture() @@ -4786,6 +4808,27 @@ id=\"{id}\",pci_segment={pci_segment},queue_sizes={queue_sizes}" ..device_fixture() } ); + + // `fd=` is accepted alongside or in place of `path=`; exclusivity + // is enforced by DeviceConfig::validate, not by the parser. + assert_eq!( + DeviceConfig::parse("fd=7")?, + DeviceConfig { + path: None, + fd: Some(7), + ..device_fixture() + } + ); + assert_eq!( + DeviceConfig::parse("path=/path/to/device,fd=7")?, + DeviceConfig { + fd: Some(7), + ..device_fixture() + } + ); + // Non-integer fd fails at parse time. + DeviceConfig::parse("fd=notanint").unwrap_err(); + Ok(()) } @@ -6101,6 +6144,49 @@ id=\"{id}\",pci_segment={pci_segment},queue_sizes={queue_sizes}" }, ]); invalid_config.validate().unwrap_err(); + + // An fd-backed DeviceConfig is only valid with iommufd enabled. + let mut fd_valid_config = valid_config.clone(); + fd_valid_config.platform = Some(PlatformConfig { + iommufd: true, + ..platform_fixture() + }); + fd_valid_config.devices = Some(vec![DeviceConfig { + path: None, + fd: Some(7), + ..device_fixture() + }]); + fd_valid_config.validate().unwrap(); + + let mut fd_without_iommufd = fd_valid_config.clone(); + fd_without_iommufd.platform = None; + assert!(matches!( + fd_without_iommufd.validate(), + Err(ValidationError::VfioFdRequiresIommufd), + )); + + // Exactly one of path and fd must be set. + let mut both_path_and_fd = fd_valid_config.clone(); + both_path_and_fd.devices = Some(vec![DeviceConfig { + path: Some("/device1".into()), + fd: Some(7), + ..device_fixture() + }]); + assert!(matches!( + both_path_and_fd.validate(), + Err(ValidationError::VfioDeviceBothPathAndFd), + )); + + let mut neither_path_nor_fd = fd_valid_config.clone(); + neither_path_nor_fd.devices = Some(vec![DeviceConfig { + path: None, + fd: None, + ..device_fixture() + }]); + assert!(matches!( + neither_path_nor_fd.validate(), + Err(ValidationError::VfioDeviceNeitherPathNorFd), + )); #[cfg(feature = "sev_snp")] { // Payload with empty host data diff --git a/vmm/src/device_manager.rs b/vmm/src/device_manager.rs index 1dcf865c0..dbddabdba 100644 --- a/vmm/src/device_manager.rs +++ b/vmm/src/device_manager.rs @@ -10,6 +10,8 @@ // use std::collections::{BTreeMap, BTreeSet, HashMap}; +#[cfg(feature = "kvm")] +use std::fs; use std::fs::{File, OpenOptions}; use std::io::{self, IsTerminal, Seek, SeekFrom, stdout}; use std::num::Wrapping; @@ -376,6 +378,10 @@ pub enum DeviceManagerError { #[error("Cannot create a VFIO device")] VfioCreate(#[source] vfio_ioctls::VfioError), + /// Failed to duplicate an externally-provided vfio cdev FD + #[error("Failed to duplicate VFIO device FD")] + VfioDupFd(#[source] io::Error), + /// Cannot create a VFIO PCI device #[error("Cannot create a VFIO PCI device")] VfioPciCreate(#[source] pci::VfioPciError), @@ -3913,6 +3919,54 @@ impl DeviceManager { } } + // Build a VfioDevice from an externally-opened vfio cdev FD: + // The caller's FD is dup'd so the dup can be owned (and closed + // on drop) by the VfioDevice, while the original FD is kept alive + // for the lifetime of the VM via VmConfig::preserved_fds. This + // lets the cdev FD survive a VM reboot. + // + // Returns the built VfioDevice together with a diagnostic path + // resolved from /proc/self/fd/. + #[cfg(feature = "kvm")] + fn create_vfio_device_from_fd( + &self, + fd: i32, + vfio_ops: Arc, + ) -> DeviceManagerResult<(VfioDevice, PathBuf)> { + assert!( + self.config + .lock() + .unwrap() + .platform + .as_ref() + .is_some_and(|p| p.iommufd), + "DeviceConfig::validate enforces iommufd when fd is set", + ); + + // SAFETY: FFI call to dup. Trivially safe. + let dup_fd = unsafe { libc::dup(fd) }; + if dup_fd < 0 { + return Err(DeviceManagerError::VfioDupFd(io::Error::last_os_error())); + } + // SAFETY: dup_fd is a freshly-opened fd owned by this File. + let file = unsafe { File::from_raw_fd(dup_fd) }; + let vfio_device = + VfioDevice::new_from_fd(file, vfio_ops).map_err(DeviceManagerError::VfioCreate)?; + + // SAFETY: fd is a valid open vfio cdev FD; the VfioDevice only + // holds a dup, so VmConfig can safely take ownership of the + // original. + unsafe { + self.config.lock().unwrap().add_preserved_fds(vec![fd]); + } + + // Diagnostic-only: resolve the FD back to the path it was opened + // from, falling back to /proc/self/fd/. + let fd_link = PathBuf::from(format!("/proc/self/fd/{fd}")); + let device_path = fs::read_link(&fd_link).unwrap_or(fd_link); + Ok((vfio_device, device_path)) + } + fn add_vfio_device( &mut self, device_cfg: &mut DeviceConfig, @@ -3976,13 +4030,25 @@ impl DeviceManager { vfio_ops }; - // The CLI parser and OpenAPI spec enforce that `path` is set - let device_path = device_cfg - .path - .as_deref() - .expect("DeviceConfig::parse enforces a path"); - let vfio_device = VfioDevice::new(device_path, Arc::clone(&vfio_ops) as Arc) - .map_err(DeviceManagerError::VfioCreate)?; + let (vfio_device, device_path) = match (&device_cfg.path, device_cfg.fd) { + (Some(path), None) => { + let vfio_device = VfioDevice::new(path, Arc::clone(&vfio_ops) as Arc) + .map_err(DeviceManagerError::VfioCreate)?; + (vfio_device, path.clone()) + } + (None, Some(fd)) => { + #[cfg(feature = "kvm")] + { + self.create_vfio_device_from_fd(fd, Arc::clone(&vfio_ops) as Arc)? + } + #[cfg(not(feature = "kvm"))] + { + let _ = fd; + return Err(DeviceManagerError::IommufdNotSupported); + } + } + _ => unreachable!("DeviceConfig::validate enforces exactly one of path/fd"), + }; if needs_dma_mapping { // Register DMA mapping in IOMMU. @@ -4067,7 +4133,7 @@ impl DeviceManager { .iter() .map(|bar| *bar as u8) .collect(), - device_path.to_path_buf(), + device_path, ) .map_err(DeviceManagerError::VfioPciCreate)?; @@ -4943,6 +5009,27 @@ impl DeviceManager { | VirtioDeviceType::Vsock => {} _ => return Err(DeviceManagerError::RemovalNotAllowed(device_type)), } + } else if matches!(pci_device_handle, PciDeviceHandle::Vfio(_)) { + // Cleanup externally-provided VFIO cdev FDs: remove the preserved + // original from VmConfig and close it. + let mut config = self.config.lock().unwrap(); + if let Some(devices) = config.devices.as_deref_mut() + && let Some(device_cfg) = devices + .iter_mut() + .find(|d| d.pci_common.id.as_deref() == Some(id)) + && let Some(fd) = device_cfg.fd.take() + { + debug!("Closing preserved FD from VFIO device: id={id}, fd={fd}"); + let fd_removed = config.preserved_fds.as_mut().unwrap().remove(&fd); + assert!( + fd_removed, + "FD {fd} for device id={id} was not in preserved_fds" + ); + // SAFETY: We are closing the only remaining instance of this FD. + unsafe { + libc::close(fd); + } + } } // Update the PCID bitmap diff --git a/vmm/src/vm_config.rs b/vmm/src/vm_config.rs index 2d9283c1d..5e0c52918 100644 --- a/vmm/src/vm_config.rs +++ b/vmm/src/vm_config.rs @@ -786,10 +786,11 @@ where impl ApplyLandlock for DeviceConfig { fn apply_landlock(&self, landlock: &mut Landlock) -> LandlockResult<()> { - let path = self - .path - .as_deref() - .expect("DeviceConfig::parse and OpenAPI spec enforce a path"); + // When the device is supplied via an externally-opened FD, there is no + // path to grant access to: the file is already open. Skip the rule. + let Some(path) = self.path.as_deref() else { + return Ok(()); + }; let device_path = fs::read_link(path).map_err(LandlockError::OpenPath)?; let iommu_group = device_path.file_name(); let iommu_group_str = iommu_group