diff --git a/alioth/src/hv/hv.rs b/alioth/src/hv/hv.rs index 6858d0ad..ae89f1c7 100644 --- a/alioth/src/hv/hv.rs +++ b/alioth/src/hv/hv.rs @@ -260,9 +260,12 @@ pub trait MsiSender: Debug + Send + Sync + 'static { fn create_irqfd(&self) -> Result; } -pub trait NotifierRegistry: Debug + Send + Sync + 'static { +pub trait NotifierRegistry: Debug + Send + Sync + Clone + 'static { fn register(&self, notifier: &Notifier, gpa: u64, len: u8, data: Option) -> Result<()>; - fn deregister(&self, notifier: &Notifier) -> Result<()>; + /// Undoes a [`NotifierRegistry::register()`] call. The parameters must + /// match the ones the notifier was registered with, since the same + /// notifier can be registered at multiple addresses. + fn deregister(&self, notifier: &Notifier, gpa: u64, len: u8, data: Option) -> Result<()>; } pub trait IrqFd: Debug + Send + Sync + AsFd + 'static { diff --git a/alioth/src/hv/hv_test.rs b/alioth/src/hv/hv_test.rs index bdb68835..c4a05d4f 100644 --- a/alioth/src/hv/hv_test.rs +++ b/alioth/src/hv/hv_test.rs @@ -149,10 +149,10 @@ pub struct RegisteredAddr { pub data: Option, } -#[derive(Debug, Default)] +#[derive(Debug, Default, Clone)] pub struct TestNotifierRegistry { pub registered: Arc>>, - pub deregistered: Arc>, + pub deregistered: Arc>>, } impl super::NotifierRegistry for TestNotifierRegistry { @@ -163,8 +163,10 @@ impl super::NotifierRegistry for TestNotifierRegistry { Ok(()) } - fn deregister(&self, _notifier: &Notifier) -> Result<()> { - *self.deregistered.lock() += 1; + fn deregister(&self, _notifier: &Notifier, gpa: u64, len: u8, data: Option) -> Result<()> { + self.deregistered + .lock() + .push(RegisteredAddr { gpa, len, data }); Ok(()) } } diff --git a/alioth/src/hv/hvf/vm.rs b/alioth/src/hv/hvf/vm.rs index 2f233671..b303ba98 100644 --- a/alioth/src/hv/hvf/vm.rs +++ b/alioth/src/hv/hvf/vm.rs @@ -111,11 +111,17 @@ impl MsiSender for HvfMsiSender { /// Hypervisor.framework has no `KVM_IOEVENTFD` equivalent, so this registry is /// deliberately uninhabited: [`HvfVm::create_notifier_registry()`] always fails /// and no value of this type can ever be constructed. -#[derive(Debug)] +#[derive(Debug, Clone, Copy)] pub enum HvfNotifierRegistry {} impl NotifierRegistry for HvfNotifierRegistry { - fn deregister(&self, _notifier: &Notifier) -> Result<()> { + fn deregister( + &self, + _notifier: &Notifier, + _gpa: u64, + _len: u8, + _data: Option, + ) -> Result<()> { match *self {} } diff --git a/alioth/src/hv/kvm/vm/vm.rs b/alioth/src/hv/kvm/vm/vm.rs index f7e3099f..11ccfe52 100644 --- a/alioth/src/hv/kvm/vm/vm.rs +++ b/alioth/src/hv/kvm/vm/vm.rs @@ -73,7 +73,6 @@ pub struct VmInner { pub fd: OwnedFd, pub vcpu_mmap_size: usize, memfd: Option, - ioeventfds: Mutex>, msi_table: RwLock>, next_msi_gsi: AtomicU32, pin_map: AtomicU32, @@ -406,13 +405,13 @@ impl MsiSender for KvmMsiSender { } } -#[derive(Debug)] +#[derive(Debug, Clone)] pub struct KvmIoeventFdRegistry { vm: Arc, } -impl NotifierRegistry for KvmIoeventFdRegistry { - fn register(&self, notifier: &Notifier, gpa: u64, len: u8, data: Option) -> Result<()> { +impl KvmIoeventFdRegistry { + fn request(notifier: &Notifier, gpa: u64, len: u8, data: Option) -> KvmIoEventFd { let mut request = KvmIoEventFd { addr: gpa, len: len as u32, @@ -423,18 +422,21 @@ impl NotifierRegistry for KvmIoeventFdRegistry { request.datamatch = data; request.flags |= KvmIoEventFdFlag::DATA_MATCH; } + request + } +} + +impl NotifierRegistry for KvmIoeventFdRegistry { + fn register(&self, notifier: &Notifier, gpa: u64, len: u8, data: Option) -> Result<()> { + let request = Self::request(notifier, gpa, len, data); unsafe { kvm_ioeventfd(&self.vm.fd, &request) }.context(error::Notifier)?; - let mut fds = self.vm.ioeventfds.lock(); - fds.insert(request.fd, request); Ok(()) } - fn deregister(&self, notifier: &Notifier) -> Result<()> { - let mut fds = self.vm.ioeventfds.lock(); - if let Some(mut request) = fds.remove(¬ifier.as_fd().as_raw_fd()) { - request.flags |= KvmIoEventFdFlag::DEASSIGN; - unsafe { kvm_ioeventfd(&self.vm.fd, &request) }.context(error::Notifier)?; - } + fn deregister(&self, notifier: &Notifier, gpa: u64, len: u8, data: Option) -> Result<()> { + let mut request = Self::request(notifier, gpa, len, data); + request.flags |= KvmIoEventFdFlag::DEASSIGN; + unsafe { kvm_ioeventfd(&self.vm.fd, &request) }.context(error::Notifier)?; Ok(()) } } @@ -465,7 +467,6 @@ impl KvmVm { fd, vcpu_mmap_size, memfd, - ioeventfds: Mutex::new(HashMap::new()), msi_table: RwLock::new(HashMap::new()), next_msi_gsi: AtomicU32::new(0), pin_map: AtomicU32::new(0), diff --git a/alioth/src/mem/mem.rs b/alioth/src/mem/mem.rs index 04f3a46c..3add4685 100644 --- a/alioth/src/mem/mem.rs +++ b/alioth/src/mem/mem.rs @@ -160,8 +160,8 @@ pub struct MemRegionEntry { pub trait MemRegionCallback: Debug + Send + Sync + Any + 'static { fn mapped(&self, addr: u64) -> Result<()>; - fn unmapped(&self) -> Result<()> { - log::debug!("{} unmapped", type_name::()); + fn unmapped(&self, addr: u64) -> Result<()> { + log::debug!("{} unmapped from {addr:#x}", type_name::()); Ok(()) } } @@ -410,7 +410,7 @@ impl Memory { } let region_callbacks = region.callbacks.lock(); for callback in region_callbacks.iter() { - callback.unmapped()?; + callback.unmapped(addr)?; } Ok(()) } @@ -496,7 +496,7 @@ impl Memory { } let callbacks = region.callbacks.lock(); for callback in callbacks.iter() { - callback.unmapped()?; + callback.unmapped(port as u64)?; } Ok(()) } diff --git a/alioth/src/vfio/device.rs b/alioth/src/vfio/device.rs index a2ae19e6..aa005b40 100644 --- a/alioth/src/vfio/device.rs +++ b/alioth/src/vfio/device.rs @@ -17,9 +17,11 @@ use std::fs::File; use std::mem::size_of; use std::os::fd::{AsFd, AsRawFd, BorrowedFd, FromRawFd, OwnedFd}; use std::os::unix::fs::FileExt; +use std::sync::Arc; use crate::errors::BoxTrace; use crate::mem; +use crate::sync::notifier::Notifier; use crate::sys::vfio::{ DeviceFeature, VfioDeviceFeature, VfioDeviceFeatureDmaBuf, VfioDeviceFeatureFlag, VfioDeviceInfo, VfioIrqInfo, VfioIrqSet, VfioIrqSetData, VfioIrqSetFlag, VfioRegionDmaRange, @@ -28,6 +30,22 @@ use crate::sys::vfio::{ }; use crate::vfio::Result; +/// A sub-region of a device region whose accesses the hypervisor can signal +/// with a notifier, instead of exiting to the VMM and forwarding them to the +/// device. +#[derive(Debug)] +pub struct RegionNotifier { + /// Offset of the sub-region within the region. + pub offset: u64, + /// Access size the notifier covers, 0 if any size matches. + pub size: u8, + /// Value the access must carry to signal the notifier, if any. + pub datamatch: Option, + /// The notifier, shared since a device may back several sub-regions with + /// the same file descriptor. + pub notifier: Arc, +} + pub trait Device: Debug + Send + Sync + 'static { fn get_info(&self) -> Result; fn get_region_info(&self, index: u32) -> Result; @@ -50,6 +68,16 @@ pub trait Device: Debug + Send + Sync + 'static { fn get_dma_buf_fd(&self, index: u32, offset: u64, size: usize) -> Result; + /// Returns the sub-regions of region `index` that the VMM should watch + /// with a notifier rather than emulate. + /// + /// Kernel VFIO devices have nothing to offer here: `VFIO_DEVICE_IOEVENTFD` + /// goes the other way round, it hands an eventfd to the kernel so that it + /// writes a fixed value to the device when the VMM signals it. + fn get_region_notifiers(&self, _index: u32) -> Result> { + Ok(vec![]) + } + // Helper methods for single-value read/write fn read(&self, region: &VfioRegionInfo, offset: u64, size: u8) -> mem::Result { let mut bytes = [0u8; 8]; diff --git a/alioth/src/vfio/device_test.rs b/alioth/src/vfio/device_test.rs index 7595eec1..1beaa5c2 100644 --- a/alioth/src/vfio/device_test.rs +++ b/alioth/src/vfio/device_test.rs @@ -21,17 +21,21 @@ use std::sync::atomic::{AtomicUsize, Ordering}; use assert_matches::assert_matches; use parking_lot::{Mutex, RwLock}; +use crate::sync::notifier::Notifier; use crate::sys::vfio::{ VfioDeviceInfo, VfioDeviceInfoFlag, VfioIrqInfo, VfioPciRegion, VfioRegionInfo, VfioRegionInfoFlag, }; use crate::vfio::Result; -use crate::vfio::device::Device; +use crate::vfio::device::{Device, RegionNotifier}; pub type RegionMap = RwLock)>>; pub type IrqMap = RwLock>; pub type MmapFdMap = RwLock>; pub type IrqEventFdList = Mutex>)>>; +/// Sub-regions [`MockVfioDevice::get_region_notifiers()`] reports, as +/// `(offset, size, datamatch)`. +pub type NotifierMap = RwLock)>>>; #[derive(Debug)] pub struct MockVfioDevice { @@ -42,6 +46,7 @@ pub struct MockVfioDevice { pub resets: AtomicUsize, pub irq_eventfds: IrqEventFdList, pub disabled_irqs: Mutex>, + pub notifiers: NotifierMap, } impl Default for MockVfioDevice { @@ -64,6 +69,7 @@ impl MockVfioDevice { resets: AtomicUsize::new(0), irq_eventfds: Mutex::new(Vec::new()), disabled_irqs: Mutex::new(Vec::new()), + notifiers: RwLock::new(HashMap::new()), }; for index in 0..=5 { dev.add_bar(index, 0, VfioRegionInfoFlag::empty(), 0); @@ -101,6 +107,14 @@ impl MockVfioDevice { self.add_region(index, size, flags, offset, vec![0u8; size as usize]) } + pub fn add_notifier(&self, index: u32, offset: u64, size: u8, datamatch: Option) { + let mut notifiers = self.notifiers.write(); + notifiers + .entry(index) + .or_default() + .push((offset, size, datamatch)); + } + pub fn add_config(&self, config: Vec) -> VfioRegionInfo { self.add_region( VfioPciRegion::CONFIG.raw(), @@ -198,6 +212,24 @@ impl Device for MockVfioDevice { fn get_dma_buf_fd(&self, _index: u32, _offset: u64, _size: usize) -> Result { Err(std::io::Error::from(std::io::ErrorKind::Unsupported).into()) } + + fn get_region_notifiers(&self, index: u32) -> Result> { + let notifiers = self.notifiers.read(); + let Some(specs) = notifiers.get(&index) else { + return Ok(vec![]); + }; + specs + .iter() + .map(|&(offset, size, datamatch)| { + Ok(RegionNotifier { + offset, + size, + datamatch, + notifier: Arc::new(Notifier::new()?), + }) + }) + .collect() + } } impl Device for Arc { @@ -245,6 +277,10 @@ impl Device for Arc { fn get_dma_buf_fd(&self, index: u32, offset: u64, size: usize) -> Result { (**self).get_dma_buf_fd(index, offset, size) } + + fn get_region_notifiers(&self, index: u32) -> Result> { + (**self).get_region_notifiers(index) + } } #[test] diff --git a/alioth/src/vfio/pci.rs b/alioth/src/vfio/pci.rs index 5105ad8b..3302474d 100644 --- a/alioth/src/vfio/pci.rs +++ b/alioth/src/vfio/pci.rs @@ -25,10 +25,10 @@ use zerocopy::{FromBytes, IntoBytes}; use crate::device::Pause; use crate::errors::BoxTrace; -use crate::hv::{IrqFd, MsiSender}; +use crate::hv::{IrqFd, MsiSender, NotifierRegistry}; use crate::mem::emulated::{Action, Mmio, MmioBus}; use crate::mem::mapped::ArcMemPages; -use crate::mem::{IoRegion, MemRange, MemRegion, MemRegionEntry, MemRegionType}; +use crate::mem::{IoRegion, MemRange, MemRegion, MemRegionCallback, MemRegionEntry, MemRegionType}; use crate::pci::cap::{ MsiCapHdr, MsiCapMmio, MsixCap, MsixCapMmio, MsixTableEntry, MsixTableMmio, MsixTableMmioEntry, NullCap, PciCap, PciCapHdr, PciCapId, @@ -41,7 +41,7 @@ use crate::pci::{self, Pci, PciBar}; use crate::sys::vfio::{ VfioDeviceInfoFlag, VfioPciIrq, VfioPciRegion, VfioRegionInfo, VfioRegionInfoFlag, }; -use crate::vfio::device::Device; +use crate::vfio::device::{Device, RegionNotifier}; use crate::vfio::{Result, error}; use crate::{align_down, align_up, mem}; @@ -384,6 +384,49 @@ where } } +/// Registers the notifiers of a device region while its BAR is mapped, so +/// that accesses the device wants to watch never reach the emulation path. +#[derive(Debug)] +struct RegionNotifierCallback { + name: Arc, + index: u32, + registry: R, + notifiers: Vec, +} + +impl MemRegionCallback for RegionNotifierCallback +where + R: NotifierRegistry, +{ + fn mapped(&self, addr: u64) -> mem::Result<()> { + for n in &self.notifiers { + let gpa = addr + n.offset; + self.registry + .register(&n.notifier, gpa, n.size, n.datamatch)?; + log::info!( + "{}: BAR {}: notifier registered at {gpa:#x}", + self.name, + self.index + ); + } + Ok(()) + } + + fn unmapped(&self, addr: u64) -> mem::Result<()> { + for n in &self.notifiers { + let gpa = addr + n.offset; + self.registry + .deregister(&n.notifier, gpa, n.size, n.datamatch)?; + log::info!( + "{}: BAR {}: notifier de-registered from {gpa:#x}", + self.name, + self.index + ); + } + Ok(()) + } +} + #[derive(Debug)] pub struct VfioPciDev where @@ -425,7 +468,15 @@ where M: MsiSender, D: Device, { - pub fn new(name: Arc, dev: D, msi_sender: M) -> Result> { + pub fn new( + name: Arc, + dev: D, + msi_sender: M, + notifier_reg: Option, + ) -> Result> + where + R: NotifierRegistry, + { let flags = dev.get_info()?.flags; let cdev = Arc::new(VfioDev { dev, name, flags }); @@ -589,7 +640,8 @@ where if region_info.size == 0 { continue; } - let region = create_bar_region( + let region_size = region_info.size; + let mut region = create_bar_region( cdev.clone(), index, region_info, @@ -597,9 +649,52 @@ where msix_table.clone(), msi_sender.clone(), )?; + let mut notifiers = match ¬ifier_reg { + Some(_) => cdev.dev.get_region_notifiers(index)?, + None => vec![], + }; + let bar_val = bar_vals[index as usize]; + let is_io = bar_val & BAR_IO == BAR_IO; + if let Some(registry) = ¬ifier_reg + && !notifiers.is_empty() + { + if is_io { + // A notifier of a port IO BAR would need + // KVM_IOEVENTFD_FLAG_PIO, which crate::hv cannot express. + log::warn!( + "{}: BAR {index}: ignoring {} notifiers of a port IO BAR", + cdev.name, + notifiers.len() + ); + } else { + // The device is not trusted to stay within its own region. + notifiers.retain(|n| { + let within = n + .offset + .checked_add(max(n.size, 1) as u64) + .is_some_and(|end| end <= region_size); + if !within { + log::error!( + "{}: BAR {index}: notifier at {:#x} exceeds the BAR size {region_size:#x}", + cdev.name, + n.offset, + ); + } + within + }); + if !notifiers.is_empty() { + let callbacks = region.callbacks.get_mut(); + callbacks.push(Box::new(RegionNotifierCallback { + name: cdev.name.clone(), + index, + registry: registry.clone(), + notifiers, + })); + } + } + } let index = index as usize; - let bar_val = bar_vals[index]; - if bar_val & BAR_IO == BAR_IO { + if is_io { let MemRange::Emulated(range) = ®ion.ranges[0] else { unreachable!() }; diff --git a/alioth/src/vfio/pci_test.rs b/alioth/src/vfio/pci_test.rs index d5515ce0..687170ec 100644 --- a/alioth/src/vfio/pci_test.rs +++ b/alioth/src/vfio/pci_test.rs @@ -19,7 +19,7 @@ use assert_matches::assert_matches; use rstest::rstest; use zerocopy::{FromBytes, IntoBytes}; -use crate::hv::tests::TestMsiSender; +use crate::hv::tests::{RegisteredAddr, TestMsiSender, TestNotifierRegistry}; use crate::mem::MemRange; use crate::mem::emulated::{Action, Mmio}; use crate::pci::cap::{ @@ -34,6 +34,11 @@ use crate::vfio::Error; use crate::vfio::device::tests::MockVfioDevice; use crate::vfio::pci::{PthBarRegion, VfioDev, VfioPciDev}; +/// No hypervisor notifier support, i.e. everything is emulated. +fn no_registry() -> Option { + None +} + fn create_mock_pci_config_space() -> Vec { let mut config = vec![0u8; 4096]; @@ -94,7 +99,13 @@ fn test_vfio_pci_dev_creation_and_config() { ); let msi_sender = TestMsiSender::default(); - let dev = VfioPciDev::new(Arc::from("test-vfio-pci"), mock.clone(), msi_sender).unwrap(); + let dev = VfioPciDev::new( + Arc::from("test-vfio-pci"), + mock.clone(), + msi_sender, + no_registry(), + ) + .unwrap(); // Verify name assert_eq!(dev.name(), "test-vfio-pci"); @@ -140,7 +151,12 @@ fn test_vfio_pci_dev_unsupported_header_type() { mock.add_config(config_bytes); - let res = VfioPciDev::new(Arc::from("test-vfio"), mock, TestMsiSender::default()); + let res = VfioPciDev::new( + Arc::from("test-vfio"), + mock, + TestMsiSender::default(), + no_registry(), + ); assert_matches!(res, Err(Error::NotSupportedHeader { ty: 1, .. })); } @@ -157,7 +173,13 @@ fn test_vfio_pci_dev_msix_bar_and_irqfd() { ); let msi_sender = TestMsiSender::default(); - let dev = VfioPciDev::new(Arc::from("test-vfio"), mock.clone(), msi_sender).unwrap(); + let dev = VfioPciDev::new( + Arc::from("test-vfio"), + mock.clone(), + msi_sender, + no_registry(), + ) + .unwrap(); let PciBar::Mem(bar0) = &dev.config.header.bars[0] else { panic!("expected Mem BAR for BAR 0"); @@ -233,7 +255,13 @@ fn test_vfio_pci_dev_msi_only() { mock.add_config(config); let msi_sender = TestMsiSender::default(); - let dev = VfioPciDev::new(Arc::from("test-msi"), mock.clone(), msi_sender).unwrap(); + let dev = VfioPciDev::new( + Arc::from("test-msi"), + mock.clone(), + msi_sender, + no_registry(), + ) + .unwrap(); // Verify MSI irq eventfds were registered let events = mock.irq_eventfds.lock().clone(); @@ -294,7 +322,13 @@ fn test_vfio_pci_dev_both_msi_and_msix() { 0, ); - let dev = VfioPciDev::new(Arc::from("test-both"), mock, TestMsiSender::default()).unwrap(); + let dev = VfioPciDev::new( + Arc::from("test-both"), + mock, + TestMsiSender::default(), + no_registry(), + ) + .unwrap(); let config = dev.config(); // MSI at 0x40 was masked with NullCap (id reads 0, next reads 0x60) @@ -329,7 +363,13 @@ fn test_vfio_pci_dev_bar_types_and_splitting() { 0, ); - let dev = VfioPciDev::new(Arc::from("test-bars"), mock, TestMsiSender::default()).unwrap(); + let dev = VfioPciDev::new( + Arc::from("test-bars"), + mock, + TestMsiSender::default(), + no_registry(), + ) + .unwrap(); // BAR 0 should be PciBar::Mem with 2 ranges (page 0 emulated table/PBA, and remaining range) let PciBar::Mem(bar0) = &dev.config.header.bars[0] else { @@ -394,7 +434,13 @@ fn test_vfio_pci_dev_mmap_bar() { 0, ); - let dev = VfioPciDev::new(Arc::from("test-mmap"), mock, TestMsiSender::default()).unwrap(); + let dev = VfioPciDev::new( + Arc::from("test-mmap"), + mock, + TestMsiSender::default(), + no_registry(), + ) + .unwrap(); assert_eq!(dev.config.header.bars.len(), 6); let PciBar::Mem(bar0) = &dev.config.header.bars[0] else { panic!("expected Mem BAR"); @@ -420,7 +466,13 @@ fn test_vfio_pci_dev_bar_mem64() { 0, ); - let dev = VfioPciDev::new(Arc::from("test-bar64"), mock, TestMsiSender::default()).unwrap(); + let dev = VfioPciDev::new( + Arc::from("test-bar64"), + mock, + TestMsiSender::default(), + no_registry(), + ) + .unwrap(); assert_matches!(dev.config.header.bars[0], PciBar::Mem(_)); assert_matches!(dev.config.header.bars[1], PciBar::Empty); } @@ -444,6 +496,7 @@ fn test_vfio_pci_cap_parsing_malformed(#[case] offset: u8) { Arc::from("test-malformed-cap"), mock, TestMsiSender::default(), + no_registry(), ) .unwrap(); assert_eq!(dev.name(), "test-malformed-cap"); @@ -478,6 +531,7 @@ fn test_vfio_pci_disjoint_and_reversed_msix_bar() { Arc::from("test-disjoint-bar"), mock.clone(), TestMsiSender::default(), + no_registry(), ) .unwrap(); let PciBar::Mem(bar0) = &dev.config.header.bars[0] else { @@ -507,3 +561,85 @@ fn test_vfio_pci_disjoint_and_reversed_msix_bar() { table_mmio.write(0x2000c - 0x20000, 4, 0x0).unwrap(); assert_eq!(mock.irq_eventfds.lock().len(), 1); } + +#[test] +fn test_vfio_pci_bar_notifiers() { + let mock = Arc::new(MockVfioDevice::default()); + mock.add_config(create_mock_pci_config_space()); + mock.add_bar( + VfioPciRegion::BAR0.raw(), + 0x1000, + VfioRegionInfoFlag::READ | VfioRegionInfoFlag::WRITE, + 0, + ); + mock.add_notifier(VfioPciRegion::BAR0.raw(), 0x10, 4, None); + mock.add_notifier(VfioPciRegion::BAR0.raw(), 0x20, 2, Some(0xbeef)); + // Out of the BAR, the server is not trusted to stay within bounds. + mock.add_notifier(VfioPciRegion::BAR0.raw(), 0x1000, 4, None); + mock.add_notifier(VfioPciRegion::BAR0.raw(), u64::MAX, 4, None); + + let registry = TestNotifierRegistry::default(); + let dev = VfioPciDev::new( + Arc::from("test-notifiers"), + mock.clone(), + TestMsiSender::default(), + Some(registry.clone()), + ) + .unwrap(); + + let PciBar::Mem(bar0) = &dev.config.header.bars[0] else { + panic!("expected Mem BAR"); + }; + let callbacks = bar0.callbacks.lock(); + // One callback updates the BAR register, the other one the notifiers. + assert_eq!(callbacks.len(), 2); + + let bar_addr = 0x8000_0000; + for callback in callbacks.iter() { + callback.mapped(bar_addr).unwrap(); + } + let expected = vec![ + RegisteredAddr { + gpa: bar_addr + 0x10, + len: 4, + data: None, + }, + RegisteredAddr { + gpa: bar_addr + 0x20, + len: 2, + data: Some(0xbeef), + }, + ]; + assert_eq!(*registry.registered.lock(), expected); + + for callback in callbacks.iter() { + callback.unmapped(bar_addr).unwrap(); + } + assert_eq!(*registry.deregistered.lock(), expected); +} + +#[test] +fn test_vfio_pci_bar_notifiers_without_registry() { + let mock = Arc::new(MockVfioDevice::default()); + mock.add_config(create_mock_pci_config_space()); + mock.add_bar( + VfioPciRegion::BAR0.raw(), + 0x1000, + VfioRegionInfoFlag::READ | VfioRegionInfoFlag::WRITE, + 0, + ); + mock.add_notifier(VfioPciRegion::BAR0.raw(), 0x10, 4, None); + + let dev = VfioPciDev::new( + Arc::from("test-no-registry"), + mock.clone(), + TestMsiSender::default(), + no_registry(), + ) + .unwrap(); + + let PciBar::Mem(bar0) = &dev.config.header.bars[0] else { + panic!("expected Mem BAR"); + }; + assert_eq!(bar0.callbacks.lock().len(), 1); +} diff --git a/alioth/src/vfio/user/bindings.rs b/alioth/src/vfio/user/bindings.rs index 9c4fc5d8..324c0a5f 100644 --- a/alioth/src/vfio/user/bindings.rs +++ b/alioth/src/vfio/user/bindings.rs @@ -128,3 +128,55 @@ pub struct VfioUserDmaUnmap { pub addr: u64, pub size: u64, } + +consts! { + pub struct VfioUserIoFdType(u32) { + IOEVENTFD = 0; + IOREGIONFD = 1; + IOEVENTFD_SHADOW = 2; + } +} + +bitflags! { + /// Flags of an ioeventfd sub-region, passing through the values of the + /// `KVM_IOEVENTFD_FLAG_*` constants. + pub struct VfioUserIoeventFdFlag(u32) { + DATA_MATCH = 1 << 0; + PIO = 1 << 1; + } +} + +/// Header of a `VFIO_USER_DEVICE_GET_REGION_IO_FDS` request and reply. +/// +/// In a request, `argsz` is the maximum reply payload the client accepts and +/// `flags` and `count` must be zero. In a reply, `argsz` is the payload size +/// needed for all sub-regions and `count` is their number. The server only +/// appends the sub-region array if the requested `argsz` can hold all of it. +#[derive(Debug, Copy, Clone, FromBytes, IntoBytes, Immutable, Default)] +#[repr(C)] +pub struct VfioUserRegionIoFds { + pub argsz: u32, + pub flags: u32, + pub index: u32, + pub count: u32, +} + +/// One entry of the sub-region array of a `VFIO_USER_DEVICE_GET_REGION_IO_FDS` +/// reply. +/// +/// This follows libvfio-user, which sends this 48-byte layout for every +/// [`VfioUserIoFdType`], zeroing the shadow fields of a plain ioeventfd. The +/// vfio-user protocol documentation instead describes a 40-byte layout without +/// them, which predates the addition of shadow ioeventfds. +#[derive(Debug, Copy, Clone, FromBytes, IntoBytes, Immutable, Default)] +#[repr(C)] +pub struct VfioUserSubRegionIoeventFd { + pub offset: u64, + pub size: u64, + pub fd_index: u32, + pub type_: VfioUserIoFdType, + pub flags: VfioUserIoeventFdFlag, + pub shadow_mem_fd_index: u32, + pub shadow_offset: u64, + pub datamatch: u64, +} diff --git a/alioth/src/vfio/user/conn.rs b/alioth/src/vfio/user/conn.rs index a88add10..21b87b03 100644 --- a/alioth/src/vfio/user/conn.rs +++ b/alioth/src/vfio/user/conn.rs @@ -22,13 +22,21 @@ use parking_lot::Mutex; use zerocopy::IntoBytes; use crate::sys::vfio::{VfioIrqInfo, VfioRegionInfo}; -use crate::utils::uds::{recv_msg_with_fds, send_msg_with_fds}; +use crate::utils::uds::{UDS_MAX_FD, recv_msg_with_fds, send_msg_with_fds}; use crate::vfio::user::bindings::{ VfioUserCmd, VfioUserDeviceInfo, VfioUserDmaMap, VfioUserDmaUnmap, VfioUserHeader, - VfioUserHeaderFlag, VfioUserIrqSet, VfioUserMessageType, VfioUserRegionAccess, VfioUserVersion, + VfioUserHeaderFlag, VfioUserIrqSet, VfioUserMessageType, VfioUserRegionAccess, + VfioUserRegionIoFds, VfioUserSubRegionIoeventFd, VfioUserVersion, }; use crate::vfio::user::{Result, error}; +/// Number of sub-regions [`VfioUserSession::get_region_io_fds()`] makes room +/// for in its first request. +const IO_FDS_BATCH: u32 = 32; +/// Maximum payload size advertised in `max_data_xfer_size` during version +/// negotiation, matching `VFIO_USER_DEFAULT_MAX_DATA_XFER_SIZE`. +const MAX_DATA_XFER_SIZE: usize = 1 << 20; + #[derive(Debug)] struct Session { stream: UnixStream, @@ -239,6 +247,76 @@ impl VfioUserSession { Ok((resp, resp_fd)) } + /// Queries the sub-regions of region `index` that can be watched with a + /// file descriptor. + /// + /// The server replies with the sub-region array only if the `argsz` we ask + /// for can hold all of it, so request room for [`IO_FDS_BATCH`] entries + /// up front and retry with the size the server asks for if that was not + /// enough. + pub fn get_region_io_fds( + &self, + index: u32, + ) -> Result<( + Vec, + [Option; UDS_MAX_FD], + )> { + let entry_size = size_of::(); + let mut capacity = IO_FDS_BATCH; + loop { + let req = VfioUserRegionIoFds { + argsz: (size_of::() + capacity as usize * entry_size) as u32, + index, + ..Default::default() + }; + let mut resp = VfioUserRegionIoFds::default(); + let mut entries = vec![VfioUserSubRegionIoeventFd::default(); capacity as usize]; + let mut fds = [const { None }; UDS_MAX_FD]; + + let reply = self.transact( + VfioUserCmd::DEVICE_GET_REGION_IO_FDS, + (req.as_bytes(), &[]), + &[], + (resp.as_mut_bytes(), entries.as_mut_bytes()), + &mut fds, + )?; + + let want = (resp.count as usize) + .checked_mul(entry_size) + .and_then(|n| n.checked_add(size_of::())); + if want != Some(resp.argsz as usize) || resp.argsz as usize > MAX_DATA_XFER_SIZE { + return error::IoFds { + index, + argsz: resp.argsz, + count: resp.count, + } + .fail(); + } + if resp.count > capacity { + if capacity != IO_FDS_BATCH { + return error::IoFds { + index, + argsz: resp.argsz, + count: resp.count, + } + .fail(); + } + capacity = resp.count; + continue; + } + let want_msg_size = size_of::() + resp.argsz as usize; + if reply.msg_size as usize != want_msg_size { + return error::PartialRead { + want: want_msg_size, + done: reply.msg_size as usize, + } + .fail(); + } + entries.truncate(resp.count as usize); + return Ok((entries, fds)); + } + } + pub fn get_irq_info(&self, index: u32) -> Result { let req = VfioIrqInfo { argsz: size_of::() as u32, diff --git a/alioth/src/vfio/user/conn_test.rs b/alioth/src/vfio/user/conn_test.rs index 26e39b99..96bc5f9e 100644 --- a/alioth/src/vfio/user/conn_test.rs +++ b/alioth/src/vfio/user/conn_test.rs @@ -13,16 +13,19 @@ // limitations under the License. use std::io::{IoSlice, IoSliceMut}; +use std::os::fd::{AsFd, BorrowedFd}; use std::os::unix::net::UnixStream; use std::thread; use assert_matches::assert_matches; use zerocopy::{FromBytes, IntoBytes}; +use crate::sync::notifier::Notifier; use crate::utils::uds::{recv_msg_with_fds, send_msg_with_fds}; use crate::vfio::user::Error; use crate::vfio::user::bindings::{ - VfioUserCmd, VfioUserHeader, VfioUserHeaderFlag, VfioUserMessageType, VfioUserVersion, + VfioUserCmd, VfioUserHeader, VfioUserHeaderFlag, VfioUserIoFdType, VfioUserMessageType, + VfioUserRegionIoFds, VfioUserSubRegionIoeventFd, VfioUserVersion, }; use crate::vfio::user::conn::VfioUserSession; @@ -99,3 +102,214 @@ fn test_vfio_user_server_error_response() { server_handle.join().unwrap(); } + +/// Receives a `DEVICE_GET_REGION_IO_FDS` request and replies with `entries`, +/// as libvfio-user does: the sub-region array is only sent if the client left +/// room for all of it, otherwise the reply is the header alone. +fn serve_region_io_fds( + server: &UnixStream, + entries: &[VfioUserSubRegionIoeventFd], + fds: &[BorrowedFd<'_>], +) -> VfioUserRegionIoFds { + let mut req_header = VfioUserHeader::default(); + let mut req = VfioUserRegionIoFds::default(); + let mut req_slice = [ + IoSliceMut::new(req_header.as_mut_bytes()), + IoSliceMut::new(req.as_mut_bytes()), + ]; + recv_msg_with_fds(server, &mut req_slice, &mut []).unwrap(); + assert_eq!(req_header.cmd, VfioUserCmd::DEVICE_GET_REGION_IO_FDS); + assert_eq!(req.flags, 0); + assert_eq!(req.count, 0); + + let argsz = size_of::() + size_of_val(entries); + let reply = VfioUserRegionIoFds { + argsz: argsz as u32, + flags: 0, + index: req.index, + count: entries.len() as u32, + }; + let fits = req.argsz as usize >= argsz; + let (payload, fds): (&[u8], &[BorrowedFd]) = if fits { + (entries.as_bytes(), fds) + } else { + (&[], &[]) + }; + let reply_hdr = VfioUserHeader { + msg_id: req_header.msg_id, + cmd: req_header.cmd, + msg_size: (size_of::() + size_of::() + payload.len()) + as u32, + flags: VfioUserHeaderFlag::new(VfioUserMessageType::REPLY, false, false), + error_no: 0, + }; + let slices = [ + IoSlice::new(reply_hdr.as_bytes()), + IoSlice::new(reply.as_bytes()), + IoSlice::new(payload), + ]; + send_msg_with_fds(server, &slices, fds).unwrap(); + req +} + +fn ioeventfd(offset: u64, fd_index: u32) -> VfioUserSubRegionIoeventFd { + VfioUserSubRegionIoeventFd { + offset, + size: 4, + fd_index, + type_: VfioUserIoFdType::IOEVENTFD, + ..Default::default() + } +} + +#[test] +fn test_vfio_user_get_region_io_fds() { + let (client, server) = UnixStream::pair().unwrap(); + let notifier = Notifier::new().unwrap(); + let server_handle = thread::spawn(move || { + // Two sub-regions sharing one file descriptor. + let entries = [ioeventfd(0x1000, 0), ioeventfd(0x1004, 0)]; + serve_region_io_fds(&server, &entries, &[notifier.as_fd()]) + }); + + let session = VfioUserSession::new(client); + let (entries, fds) = session.get_region_io_fds(2).unwrap(); + + let req = server_handle.join().unwrap(); + assert_eq!(req.index, 2); + assert_eq!(entries.len(), 2); + assert_eq!(entries[0].offset, 0x1000); + assert_eq!(entries[1].offset, 0x1004); + assert_eq!(entries[1].fd_index, 0); + assert!(fds[0].is_some()); + assert!(fds[1].is_none()); +} + +#[test] +fn test_vfio_user_get_region_io_fds_empty() { + let (client, server) = UnixStream::pair().unwrap(); + let server_handle = thread::spawn(move || serve_region_io_fds(&server, &[], &[])); + + let session = VfioUserSession::new(client); + let (entries, fds) = session.get_region_io_fds(0).unwrap(); + + server_handle.join().unwrap(); + assert!(entries.is_empty()); + assert!(fds.iter().all(Option::is_none)); +} + +#[test] +fn test_vfio_user_get_region_io_fds_too_many() { + // More sub-regions than the first request makes room for, so the server + // replies with the header only and the client has to ask again. + let count = 40; + let (client, server) = UnixStream::pair().unwrap(); + let notifier = Notifier::new().unwrap(); + let server_handle = thread::spawn(move || { + let entries: Vec<_> = (0..count) + .map(|i| ioeventfd(0x1000 + i as u64 * 4, 0)) + .collect(); + let first = serve_region_io_fds(&server, &entries, &[notifier.as_fd()]); + let second = serve_region_io_fds(&server, &entries, &[notifier.as_fd()]); + (first, second) + }); + + let session = VfioUserSession::new(client); + let (entries, fds) = session.get_region_io_fds(0).unwrap(); + + let (first, second) = server_handle.join().unwrap(); + let entry_size = size_of::(); + assert_eq!( + first.argsz as usize, + size_of::() + 32 * entry_size + ); + assert_eq!( + second.argsz as usize, + size_of::() + count * entry_size + ); + assert_eq!(entries.len(), count); + assert!(fds[0].is_some()); +} + +#[test] +fn test_vfio_user_get_region_io_fds_bad_argsz() { + let (client, server) = UnixStream::pair().unwrap(); + let server_handle = thread::spawn(move || { + let mut req_header = VfioUserHeader::default(); + let mut req = VfioUserRegionIoFds::default(); + let mut req_slice = [ + IoSliceMut::new(req_header.as_mut_bytes()), + IoSliceMut::new(req.as_mut_bytes()), + ]; + recv_msg_with_fds(&server, &mut req_slice, &mut []).unwrap(); + + // count and argsz disagree, e.g. a server sending 40-byte entries. + let reply = VfioUserRegionIoFds { + argsz: (size_of::() + 40) as u32, + flags: 0, + index: req.index, + count: 1, + }; + let reply_hdr = VfioUserHeader { + msg_id: req_header.msg_id, + cmd: req_header.cmd, + msg_size: (size_of::() + size_of::() + 40) as u32, + flags: VfioUserHeaderFlag::new(VfioUserMessageType::REPLY, false, false), + error_no: 0, + }; + let slices = [ + IoSlice::new(reply_hdr.as_bytes()), + IoSlice::new(reply.as_bytes()), + IoSlice::new(&[0u8; 40]), + ]; + send_msg_with_fds(&server, &slices, &[]).unwrap(); + }); + + let session = VfioUserSession::new(client); + let res = session.get_region_io_fds(1); + assert_matches!(res, Err(Error::IoFds { count: 1, .. })); + + server_handle.join().unwrap(); +} + +#[test] +fn test_vfio_user_get_region_io_fds_short_reply() { + let (client, server) = UnixStream::pair().unwrap(); + let server_handle = thread::spawn(move || { + let mut req_header = VfioUserHeader::default(); + let mut req = VfioUserRegionIoFds::default(); + let mut req_slice = [ + IoSliceMut::new(req_header.as_mut_bytes()), + IoSliceMut::new(req.as_mut_bytes()), + ]; + recv_msg_with_fds(&server, &mut req_slice, &mut []).unwrap(); + + // count <= capacity and argsz matches count, but the reply omits the + // sub-region array payload. + let reply = VfioUserRegionIoFds { + argsz: (size_of::() + size_of::()) + as u32, + flags: 0, + index: req.index, + count: 1, + }; + let reply_hdr = VfioUserHeader { + msg_id: req_header.msg_id, + cmd: req_header.cmd, + msg_size: (size_of::() + size_of::()) as u32, + flags: VfioUserHeaderFlag::new(VfioUserMessageType::REPLY, false, false), + error_no: 0, + }; + let slices = [ + IoSlice::new(reply_hdr.as_bytes()), + IoSlice::new(reply.as_bytes()), + ]; + send_msg_with_fds(&server, &slices, &[]).unwrap(); + }); + + let session = VfioUserSession::new(client); + let res = session.get_region_io_fds(1); + assert_matches!(res, Err(Error::PartialRead { .. })); + + server_handle.join().unwrap(); +} diff --git a/alioth/src/vfio/user/device.rs b/alioth/src/vfio/user/device.rs index 1217db7d..f168bbaa 100644 --- a/alioth/src/vfio/user/device.rs +++ b/alioth/src/vfio/user/device.rs @@ -27,11 +27,13 @@ use crate::errors::BoxTrace; use crate::mem; use crate::mem::LayoutChanged; use crate::mem::mapped::ArcMemPages; +use crate::sync::notifier::Notifier; use crate::sys::vfio::{VfioDeviceInfo, VfioIrqInfo, VfioIrqSetFlag, VfioRegionInfo}; use crate::vfio::Result; -use crate::vfio::device::Device; +use crate::vfio::device::{Device, RegionNotifier}; use crate::vfio::user::bindings::{ - VfioUserDmaMap, VfioUserDmaMapFlag, VfioUserDmaUnmap, VfioUserIrqSet, VfioUserRegionAccess, + VfioUserDmaMap, VfioUserDmaMapFlag, VfioUserDmaUnmap, VfioUserIoFdType, VfioUserIoeventFdFlag, + VfioUserIrqSet, VfioUserRegionAccess, }; use crate::vfio::user::conn::VfioUserSession; @@ -161,6 +163,57 @@ impl Device for VfioUserDevice { ) .into()) } + + fn get_region_notifiers(&self, index: u32) -> Result> { + let (entries, mut fds) = self.session.get_region_io_fds(index)?; + let mut notifiers = Vec::new(); + // The server may back several sub-regions with the same file + // descriptor, in which case they all share one notifier. + let mut shared: HashMap> = HashMap::new(); + for entry in entries { + if entry.type_ != VfioUserIoFdType::IOEVENTFD { + // An ioregionfd needs a KVM feature that is not upstream, and a + // shadow ioeventfd additionally needs the value of the write in + // a shared buffer, which upstream KVM cannot provide either. + log::warn!("region {index}: ignoring sub-region {entry:x?}: unsupported type"); + continue; + } + if entry.flags.contains(VfioUserIoeventFdFlag::PIO) { + log::warn!("region {index}: ignoring sub-region {entry:x?}: port IO"); + continue; + } + let Ok(size) = u8::try_from(entry.size) else { + log::warn!("region {index}: ignoring sub-region {entry:x?}: bad size"); + continue; + }; + let notifier = match shared.get(&entry.fd_index) { + Some(notifier) => notifier.clone(), + None => { + let Some(Some(fd)) = fds.get_mut(entry.fd_index as usize).map(Option::take) + else { + log::warn!("region {index}: ignoring sub-region {entry:x?}: no fd"); + continue; + }; + let notifier = Arc::new(Notifier::try_from(fd)?); + shared.insert(entry.fd_index, notifier.clone()); + notifier + } + }; + let datamatch = if entry.flags.contains(VfioUserIoeventFdFlag::DATA_MATCH) { + Some(entry.datamatch) + } else { + None + }; + log::debug!("region {index}: sub-region {entry:x?} -> {notifier:?}"); + notifiers.push(RegionNotifier { + offset: entry.offset, + size, + datamatch, + notifier, + }); + } + Ok(notifiers) + } } #[derive(Debug)] diff --git a/alioth/src/vfio/user/device_test.rs b/alioth/src/vfio/user/device_test.rs index 973486fb..1fcc2f23 100644 --- a/alioth/src/vfio/user/device_test.rs +++ b/alioth/src/vfio/user/device_test.rs @@ -23,6 +23,7 @@ use zerocopy::{FromBytes, IntoBytes}; use crate::mem::LayoutChanged; use crate::mem::mapped::ArcMemPages; +use crate::sync::notifier::Notifier; use crate::sys::vfio::{ VfioDeviceInfoFlag, VfioIrqInfo, VfioIrqInfoFlag, VfioRegionInfo, VfioRegionInfoFlag, }; @@ -30,7 +31,8 @@ use crate::utils::uds::{recv_msg_with_fds, send_msg_with_fds}; use crate::vfio::device::Device; use crate::vfio::user::bindings::{ VfioUserCmd, VfioUserDeviceInfo, VfioUserDmaUnmap, VfioUserHeader, VfioUserHeaderFlag, - VfioUserMessageType, VfioUserRegionAccess, VfioUserVersion, + VfioUserIoFdType, VfioUserIoeventFdFlag, VfioUserMessageType, VfioUserRegionAccess, + VfioUserRegionIoFds, VfioUserSubRegionIoeventFd, VfioUserVersion, }; use crate::vfio::user::conn::VfioUserSession; use crate::vfio::user::device::{UpdateVfioUserMapping, VfioUserDevice}; @@ -360,3 +362,108 @@ fn test_vfio_user_device_full_lifecycle() { drop(session); server_handle.join().unwrap(); } + +#[test] +fn test_vfio_user_device_region_notifiers() { + let (client, server) = UnixStream::pair().unwrap(); + let eventfd_a = Notifier::new().unwrap(); + let eventfd_b = Notifier::new().unwrap(); + + let server_handle = thread::spawn(move || { + let mut req_header = VfioUserHeader::default(); + let mut req = VfioUserRegionIoFds::default(); + let mut req_slice = [ + IoSliceMut::new(req_header.as_mut_bytes()), + IoSliceMut::new(req.as_mut_bytes()), + ]; + recv_msg_with_fds(&server, &mut req_slice, &mut []).unwrap(); + + let plain = VfioUserSubRegionIoeventFd { + size: 4, + type_: VfioUserIoFdType::IOEVENTFD, + ..Default::default() + }; + let entries = [ + // Two sub-regions sharing one eventfd, the first with a datamatch. + VfioUserSubRegionIoeventFd { + offset: 0x100, + size: 2, + flags: VfioUserIoeventFdFlag::DATA_MATCH, + datamatch: 0x2, + ..plain + }, + VfioUserSubRegionIoeventFd { + offset: 0x104, + ..plain + }, + // Everything below is dropped. + VfioUserSubRegionIoeventFd { + offset: 0x200, + fd_index: 1, + type_: VfioUserIoFdType::IOREGIONFD, + ..plain + }, + VfioUserSubRegionIoeventFd { + offset: 0x204, + fd_index: 1, + type_: VfioUserIoFdType::IOEVENTFD_SHADOW, + shadow_mem_fd_index: 1, + ..plain + }, + VfioUserSubRegionIoeventFd { + offset: 0x208, + fd_index: 1, + flags: VfioUserIoeventFdFlag::PIO, + ..plain + }, + VfioUserSubRegionIoeventFd { + offset: 0x20c, + fd_index: 7, + ..plain + }, + // A second eventfd. + VfioUserSubRegionIoeventFd { + offset: 0x300, + fd_index: 1, + ..plain + }, + ]; + let reply = VfioUserRegionIoFds { + argsz: (size_of::() + size_of_val(&entries)) as u32, + flags: 0, + index: req.index, + count: entries.len() as u32, + }; + let reply_hdr = VfioUserHeader { + msg_id: req_header.msg_id, + cmd: req_header.cmd, + msg_size: (size_of::() + reply.argsz as usize) as u32, + flags: VfioUserHeaderFlag::new(VfioUserMessageType::REPLY, false, false), + error_no: 0, + }; + let slices = [ + IoSlice::new(reply_hdr.as_bytes()), + IoSlice::new(reply.as_bytes()), + IoSlice::new(entries.as_bytes()), + ]; + let fds = [eventfd_a.as_fd(), eventfd_b.as_fd()]; + send_msg_with_fds(&server, &slices, &fds).unwrap(); + }); + + let session = Arc::new(VfioUserSession::new(client)); + let dev = VfioUserDevice::new(session).unwrap(); + let notifiers = dev.get_region_notifiers(1).unwrap(); + server_handle.join().unwrap(); + + assert_eq!(notifiers.len(), 3); + assert_eq!(notifiers[0].offset, 0x100); + assert_eq!(notifiers[0].size, 2); + assert_eq!(notifiers[0].datamatch, Some(0x2)); + assert_eq!(notifiers[1].offset, 0x104); + assert_eq!(notifiers[1].size, 4); + assert_eq!(notifiers[1].datamatch, None); + assert_eq!(notifiers[2].offset, 0x300); + // Sub-regions sharing a file descriptor share one notifier. + assert!(Arc::ptr_eq(¬ifiers[0].notifier, ¬ifiers[1].notifier)); + assert!(!Arc::ptr_eq(¬ifiers[0].notifier, ¬ifiers[2].notifier)); +} diff --git a/alioth/src/vfio/user/user.rs b/alioth/src/vfio/user/user.rs index 9b314e77..adc359ec 100644 --- a/alioth/src/vfio/user/user.rs +++ b/alioth/src/vfio/user/user.rs @@ -41,6 +41,10 @@ pub enum Error { Version { major: u16, minor: u16 }, #[snafu(display("Server error: cmd {cmd:?}, error code {code}"))] ServerErr { cmd: VfioUserCmd, code: u32 }, + #[snafu(display( + "Region {index}: unexpected sub-region IO fd array, argsz {argsz}, count {count}" + ))] + IoFds { index: u32, argsz: u32, count: u32 }, } pub type Result = std::result::Result; diff --git a/alioth/src/virtio/pci.rs b/alioth/src/virtio/pci.rs index a3d728d6..af8e2bcd 100644 --- a/alioth/src/virtio/pci.rs +++ b/alioth/src/virtio/pci.rs @@ -625,24 +625,34 @@ where notifiers: Arc<[Notifier]>, } +impl NotifierCallback +where + R: NotifierRegistry, +{ + fn notify_addr(bar_addr: u64, q_index: usize) -> u64 { + let base_addr = bar_addr + (12 << 10) + VirtioPciRegister::OFFSET_QUEUE_NOTIFY as u64; + base_addr + (q_index * size_of::()) as u64 + } +} + impl MemRegionCallback for NotifierCallback where R: NotifierRegistry, { fn mapped(&self, addr: u64) -> mem::Result<()> { for (q_index, notifier) in self.notifiers.iter().enumerate() { - let base_addr = addr + (12 << 10) + VirtioPciRegister::OFFSET_QUEUE_NOTIFY as u64; - let notify_addr = base_addr + (q_index * size_of::()) as u64; + let notify_addr = Self::notify_addr(addr, q_index); self.registry.register(notifier, notify_addr, 0, None)?; log::info!("q-{q_index} notifier registered at {notify_addr:x}",) } Ok(()) } - fn unmapped(&self) -> mem::Result<()> { - for notifier in self.notifiers.iter() { - self.registry.deregister(notifier)?; - log::info!("notifier {notifier:?} de-registered") + fn unmapped(&self, addr: u64) -> mem::Result<()> { + for (q_index, notifier) in self.notifiers.iter().enumerate() { + let notify_addr = Self::notify_addr(addr, q_index); + self.registry.deregister(notifier, notify_addr, 0, None)?; + log::info!("q-{q_index} notifier de-registered from {notify_addr:x}") } Ok(()) } diff --git a/alioth/src/virtio/pci_test.rs b/alioth/src/virtio/pci_test.rs index 1361ef3d..b1aad476 100644 --- a/alioth/src/virtio/pci_test.rs +++ b/alioth/src/virtio/pci_test.rs @@ -1487,8 +1487,8 @@ fn test_virtio_pci_device_notifier_callback() { ); // Test unmapped callback - assert_matches!(callbacks[0].unmapped(), Ok(())); - assert_eq!(*deregistered.lock(), 2); + assert_matches!(callbacks[0].unmapped(base_addr), Ok(())); + assert_eq!(*deregistered.lock(), *registered); } #[test] diff --git a/alioth/src/vm/vm.rs b/alioth/src/vm/vm.rs index 376ab718..a393ac88 100644 --- a/alioth/src/vm/vm.rs +++ b/alioth/src/vm/vm.rs @@ -272,6 +272,22 @@ where Ok(fw_cfg) } + /// Creates a notifier registry, or returns `None` if the hypervisor has no + /// notifier support and every access must exit to the VMM. + fn create_notifier_registry( + &self, + name: &str, + ) -> Result::NotifierRegistry>, Error> { + match self.ctx.board.vm.create_notifier_registry() { + Ok(registry) => Ok(Some(registry)), + Err(hv::Error::Notifier { error, .. }) if error.kind() == ErrorKind::Unsupported => { + log::debug!("{name}: notifiers are not supported"); + Ok(None) + } + Err(e) => Err(e.into()), + } + } + pub fn add_virtio_dev( &self, name: impl Into>, @@ -293,15 +309,7 @@ where if let Some(callback) = dev.mem_change_callback() { self.ctx.board.memory.register_change_callback(callback)?; } - let registry = match self.ctx.board.vm.create_notifier_registry() { - Ok(registry) => Some(registry), - // The hypervisor does not support notifiers, fall back to VM exits. - Err(hv::Error::Notifier { error, .. }) if error.kind() == ErrorKind::Unsupported => { - log::debug!("{name}: notifiers are not supported"); - None - } - Err(e) => return Err(e.into()), - }; + let registry = self.create_notifier_registry(&name)?; let virtio_dev = VirtioDevice::new( name.clone(), dev, @@ -381,7 +389,8 @@ where #[cfg(target_arch = "aarch64")] u32::from(bdf.0), )?; - let dev = VfioPciDev::new(name.clone(), cdev, msi_sender)?; + let none = None::<::NotifierRegistry>; + let dev = VfioPciDev::new(name.clone(), cdev, msi_sender, none)?; self.add_pci_dev(Some(bdf), Arc::new(dev))?; Ok(()) } @@ -402,7 +411,8 @@ where #[cfg(target_arch = "aarch64")] u32::from(bdf.0), )?; - let dev = VfioPciDev::new(name.clone(), dev, msi_sender)?; + let registry = self.create_notifier_registry(&name)?; + let dev = VfioPciDev::new(name.clone(), dev, msi_sender, registry)?; self.add_pci_dev(Some(bdf), Arc::new(dev))?; let update = Box::new(UpdateVfioUserMapping::new(session.clone())); @@ -468,7 +478,8 @@ where #[cfg(target_arch = "aarch64")] u32::from(bdf.0), )?; - let dev = VfioPciDev::new(name.clone(), devfd, msi_sender)?; + let none = None::<::NotifierRegistry>; + let dev = VfioPciDev::new(name.clone(), devfd, msi_sender, none)?; self.add_pci_dev(Some(bdf), Arc::new(dev)) } }