From 9161b18f9b9ef053550598d5288602e59b9dffee Mon Sep 17 00:00:00 2001 From: Rob Bradford Date: Fri, 3 Jul 2026 09:58:07 +0100 Subject: [PATCH] virtio-devices: iommu: Translate buffers spanning adjacent mappings The virtio-iommu translation required the whole [addr, addr+size) span to be covered by a single mapping. A guest is free to describe one contiguous buffer with several adjacent mappings: the specification lets the driver map at page granularity and the Linux IOMMU core splits a single mapping request at page size boundaries. A descriptor buffer backed this way was rejected as an invalid translation even though every page was mapped, wedging the device. Walk consecutive mappings when no single one covers the span, accepting the translation once the mappings are adjacent in IOVA space and contiguous in guest-physical space. A non-contiguous span cannot be represented by the single returned address and is still rejected. Assisted-by: Claude:Opus-4.8 Signed-off-by: Rob Bradford --- virtio-devices/src/iommu.rs | 115 ++++++++++++++++++++++++++++++++---- 1 file changed, 104 insertions(+), 11 deletions(-) diff --git a/virtio-devices/src/iommu.rs b/virtio-devices/src/iommu.rs index 6829a5f30..0a77c2a03 100644 --- a/virtio-devices/src/iommu.rs +++ b/virtio-devices/src/iommu.rs @@ -993,6 +993,42 @@ fn span_end(addr: u64, size: u64) -> io::Result { }) } +/// Translate `[addr, end]` (inclusive end) into a physical address. The span +/// may cross more than one mapping as long as those mappings are adjacent in +/// IOVA space and contiguous in GPA space, so a buffer the guest mapped with +/// several mappings still resolves to a single physical range. Returns None +/// when `addr` is unmapped, a gap is hit before `end`, or the covered range is +/// not physically contiguous. +fn translate_contiguous_range( + mappings: &BTreeMap, + addr: u64, + end: u64, +) -> Option { + // The mapping containing `addr` is the one with the greatest start <= addr. + let (&start_key, start) = mappings.range(..=addr).next_back()?; + if addr > inclusive_end(start_key, start.size)? { + return None; + } + let base_gpa = addr - start_key + start.gpa; + + let mut covered = start_key.checked_add(start.size)?; + let mut next_gpa = start.gpa.checked_add(start.size)?; + + // Walk forward across adjacent mappings until `end` is covered. + for (&key, value) in mappings.range(start_key.checked_add(1)?..) { + if end < covered { + break; + } + if key != covered || value.gpa != next_gpa { + return None; + } + covered = key.checked_add(value.size)?; + next_gpa = value.gpa.checked_add(value.size)?; + } + + (end < covered).then_some(base_gpa) +} + impl DmaRemapping for IommuMapping { fn translate_gva(&self, id: u32, addr: u64, size: u64) -> io::Result { debug!("Translate GVA addr 0x{addr:x} size 0x{size:x}"); @@ -1005,15 +1041,9 @@ impl DmaRemapping for IommuMapping { return Ok(addr); } - for (&key, &value) in domain.mappings.iter() { - if let Some(mapping_end) = inclusive_end(key, value.size) - && addr >= key - && end <= mapping_end - { - let new_addr = addr - key + value.gpa; - debug!("Into GPA addr 0x{new_addr:x}"); - return Ok(new_addr); - } + if let Some(new_addr) = translate_contiguous_range(&domain.mappings, addr, end) { + debug!("Into GPA addr 0x{new_addr:x}"); + return Ok(new_addr); } } } else if self.bypass.load(Ordering::Acquire) { @@ -1363,14 +1393,17 @@ impl Migratable for Iommu {} #[cfg(test)] mod tests { + use std::collections::BTreeMap; use std::io; - use std::sync::{Arc, Weak}; + use std::sync::atomic::AtomicBool; + use std::sync::{Arc, RwLock, Weak}; use seccompiler::SeccompAction; use vm_device::dma_mapping::ExternalDmaMapping; use vmm_sys_util::eventfd::{EFD_NONBLOCK, EventFd}; - use super::Iommu; + use super::{Domain, Iommu, IommuMapping, Mapping}; + use crate::DmaRemapping; /// Test stub for VfioDmaMapping. struct MockMapping; @@ -1429,4 +1462,64 @@ mod tests { // Removing a bogus ID doesn't crash. assert!(iommu.remove_external_mapping(0x999).is_none()); } + + /// Build an IommuMapping with endpoint 0 attached to a single domain whose + /// mappings are the given `(iova, gpa, size)` tuples. + fn iommu_mapping(entries: &[(u64, u64, u64)]) -> IommuMapping { + let mut mappings = BTreeMap::new(); + for &(iova, gpa, size) in entries { + mappings.insert(iova, Mapping { gpa, size }); + } + let mut domains = BTreeMap::new(); + domains.insert( + 0, + Domain { + mappings, + bypass: false, + }, + ); + let mut endpoints = BTreeMap::new(); + endpoints.insert(0, 0); + IommuMapping { + endpoints: Arc::new(RwLock::new(endpoints)), + domains: Arc::new(RwLock::new(domains)), + bypass: AtomicBool::new(false), + } + } + + #[test] + fn translate_within_single_mapping() { + let m = iommu_mapping(&[(0x1000, 0x4000, 0x1000)]); + assert_eq!(m.translate_gva(0, 0x1200, 0x100).unwrap(), 0x4200); + } + + #[test] + fn translate_spanning_contiguous_mappings() { + // A buffer mapped with two adjacent mappings that resolve to a + // contiguous physical range translates as one range. + let m = iommu_mapping(&[(0x1000, 0x4000, 0x1000), (0x2000, 0x5000, 0x1000)]); + assert_eq!(m.translate_gva(0, 0x1000, 0x2000).unwrap(), 0x4000); + // A sub-range that straddles the boundary also resolves. + assert_eq!(m.translate_gva(0, 0x1f00, 0x200).unwrap(), 0x4f00); + } + + #[test] + fn reject_spanning_noncontiguous_mappings() { + // Adjacent in IOVA but disjoint in GPA cannot be one range. + let m = iommu_mapping(&[(0x1000, 0x4000, 0x1000), (0x2000, 0x9000, 0x1000)]); + m.translate_gva(0, 0x1000, 0x2000).unwrap_err(); + } + + #[test] + fn reject_span_across_iova_gap() { + // A hole between mappings leaves part of the span unmapped. + let m = iommu_mapping(&[(0x1000, 0x4000, 0x1000), (0x3000, 0x6000, 0x1000)]); + m.translate_gva(0, 0x1000, 0x2000).unwrap_err(); + } + + #[test] + fn reject_unmapped_base() { + let m = iommu_mapping(&[(0x1000, 0x4000, 0x1000)]); + m.translate_gva(0, 0x2500, 0x10).unwrap_err(); + } }