kvm-ioctls: Add memory fences to dirty ring iterator

davidkleymann · davidkleymann · commit 231dc89fd296 · 2025-10-18T22:24:59.000+02:00
Inserted acquire/release memory fences in KvmDirtyLogRing iterator's next() method for architectures with weak memory consistency. This ensures proper synchronization when reading/writing dirty log entries shared with the kernel.

Signed-off-by: David Kleymann &lt;david@asymmetric.re&gt;
diff --git a/kvm-ioctls/CHANGELOG.md b/kvm-ioctls/CHANGELOG.md
@@ -12,8 +12,9 @@
 - Added `KVM_DIRTY_GFN_F_DIRTY` and `KVM_DIRTY_GFN_F_RESET` bitflags.
 - Added `KvmDirtyLogRing` iterator type for accessing dirty log entries.
 - Added `dirty_log_ring` field to `VcpuFd` to access per-vCpu dirty rings.
-- Added `dirty_log_bytes` field to `VmFd` to automatically map correct size dirty
-  rings for vCpus as they are created.
+- Inserted fences in KvmDirtyLogRing iterator `next` for architectures with weak memory consistency that require Acquire/Release
+- Added `DirtyLogRingInfo` struct and `dirty_log_ring_info` field to `VmFd` to
+  track dirty ring configuration.
 - Added `enable_dirty_log_ring` function on `VmFd` to check corresponding 
   capabilities and enable KVM's dirty log ring.
 - Added `VcpuFd::dirty_log_ring_iter()` to iterate over dirty guest frame numbers.
diff --git a/kvm-ioctls/src/ioctls/mod.rs b/kvm-ioctls/src/ioctls/mod.rs
@@ -8,6 +8,7 @@
 use std::mem::size_of;
 use std::os::unix::io::AsRawFd;
 use std::ptr::{NonNull, null_mut};
+use std::sync::atomic::{Ordering, fence};
 
 use kvm_bindings::{
     KVM_COALESCED_MMIO_PAGE_OFFSET, KVM_DIRTY_GFN_F_DIRTY, KVM_DIRTY_GFN_F_RESET,
@@ -39,6 +40,8 @@ pub(crate) struct KvmDirtyLogRing {
     gfns: NonNull<kvm_dirty_gfn>,
     /// Ring size mask (size-1) for efficient modulo operations
     mask: u64,
+    /// `true` if we need to use Acquire/Release memory ordering
+    use_acq_rel: bool,
 }
 
 // SAFETY: TBD
@@ -50,7 +53,11 @@ impl KvmDirtyLogRing {
     /// # Arguments
     /// * `fd` - vCPU file descriptor to mmap from.
     /// * `size` - Size of memory region in bytes.
-    pub(crate) fn mmap_from_fd<F: AsRawFd>(fd: &F, bytes: usize) -> Result<Self> {
+    pub(crate) fn mmap_from_fd<F: AsRawFd>(
+        fd: &F,
+        bytes: usize,
+        use_acq_rel: bool,
+    ) -> Result<Self> {
         // SAFETY: We trust the sysconf libc function and we're calling it
         // with a correct parameter.
         let page_size = match unsafe { libc::sysconf(libc::_SC_PAGESIZE) } {
@@ -90,6 +97,7 @@ impl KvmDirtyLogRing {
             next_dirty: 0,
             gfns,
             mask: (slots - 1) as u64,
+            use_acq_rel,
         });
     }
 }
@@ -111,19 +119,32 @@ impl Iterator for KvmDirtyLogRing {
     type Item = (u32, u64);
     fn next(&mut self) -> Option<Self::Item> {
         let i = self.next_dirty & self.mask;
-        unsafe {
-            let gfn_ptr = self.gfns.add(i as usize).as_ptr();
-            let gfn = gfn_ptr.read_volatile();
-            if gfn.flags & KVM_DIRTY_GFN_F_DIRTY == 0 {
-                // next_dirty stays the same, it will become the next dirty element
-                return None;
-            } else {
-                self.next_dirty += 1;
-                let mut updated_gfn = gfn;
-                updated_gfn.flags ^= KVM_DIRTY_GFN_F_RESET;
+        // SAFETY: i is not larger than mask, thus is a valid offset into self.gfns,
+        // therefore this operation produces a valid pointer to a kvm_dirty_gfn
+        let gfn_ptr = unsafe { self.gfns.add(i as usize).as_ptr() };
+
+        if self.use_acq_rel {
+            fence(Ordering::Acquire);
+        }
+
+        // SAFETY: Can read a valid pointer to a kvm_dirty_gfn
+        let gfn = unsafe { gfn_ptr.read_volatile() };
+
+        if gfn.flags & KVM_DIRTY_GFN_F_DIRTY == 0 {
+            // next_dirty stays the same, it will become the next dirty element
+            return None;
+        } else {
+            self.next_dirty += 1;
+            let mut updated_gfn = gfn;
+            updated_gfn.flags ^= KVM_DIRTY_GFN_F_RESET;
+            // SAFETY: Can write to a valid pointer to a kvm_dirty_gfn
+            unsafe {
                 gfn_ptr.write_volatile(updated_gfn);
-                return Some((gfn.slot, gfn.offset));
+            };
+            if self.use_acq_rel {
+                fence(Ordering::Release);
             }
+            return Some((gfn.slot, gfn.offset));
         }
     }
 }
diff --git a/kvm-ioctls/src/ioctls/vcpu.rs b/kvm-ioctls/src/ioctls/vcpu.rs
@@ -2110,7 +2110,7 @@ impl VcpuFd {
     /// Gets the dirty log ring iterator if one is mapped.
     ///
     /// Returns an iterator over dirty guest frame numbers as (slot, offset) tuples.
-    /// Returns `None` if no dirty log ring has been mapped via [`map_dirty_log_ring`](VcpuFd::map_dirty_log_ring).
+    /// Returns `None` if no dirty log ring has been mapped.
     ///
     /// # Returns
     ///
@@ -2122,7 +2122,7 @@ impl VcpuFd {
     /// # use kvm_ioctls::Kvm;
     /// # use kvm_ioctls::Cap;
     /// let kvm = Kvm::new().unwrap();
-    /// let vm = kvm.create_vm().unwrap();
+    /// let mut vm = kvm.create_vm().unwrap();
     /// vm.enable_dirty_log_ring(None).unwrap();
     /// let mut vcpu = vm.create_vcpu(0).unwrap();
     /// if kvm.check_extension(Cap::DirtyLogRing) {
diff --git a/kvm-ioctls/src/ioctls/vm.rs b/kvm-ioctls/src/ioctls/vm.rs
@@ -54,12 +54,21 @@ impl From<NoDatamatch> for u64 {
     }
 }
 
+/// Information about dirty log ring configuration.
+#[derive(Debug)]
+struct DirtyLogRingInfo {
+    /// Size of dirty ring in bytes.
+    bytes: usize,
+    /// Whether to use acquire/release semantics.
+    acq_rel: bool,
+}
+
 /// Wrapper over KVM VM ioctls.
 #[derive(Debug)]
 pub struct VmFd {
     vm: File,
     run_size: usize,
-    dirty_ring_bytes: usize,
+    dirty_log_ring_info: Option<DirtyLogRingInfo>,
 }
 
 impl VmFd {
@@ -1215,12 +1224,14 @@ impl VmFd {
 
         let kvm_run_ptr = KvmRunWrapper::mmap_from_fd(&vcpu, self.run_size)?;
 
-        let dirty_log_ring = {
-            if self.dirty_ring_bytes > 0 {
-                Some(KvmDirtyLogRing::mmap_from_fd(&vcpu, self.dirty_ring_bytes)?)
-            } else {
-                None
-            }
+        let dirty_log_ring = if let Some(info) = &self.dirty_log_ring_info {
+            Some(KvmDirtyLogRing::mmap_from_fd(
+                &vcpu,
+                info.bytes,
+                info.acq_rel,
+            )?)
+        } else {
+            None
         };
 
         Ok(new_vcpu(vcpu, kvm_run_ptr, dirty_log_ring))
@@ -1259,12 +1270,14 @@ impl VmFd {
         // SAFETY: we trust the kernel and verified parameters
         let vcpu = unsafe { File::from_raw_fd(fd) };
         let kvm_run_ptr = KvmRunWrapper::mmap_from_fd(&vcpu, self.run_size)?;
-        let dirty_log_ring = {
-            if self.dirty_ring_bytes > 0 {
-                Some(KvmDirtyLogRing::mmap_from_fd(&vcpu, self.dirty_ring_bytes)?)
-            } else {
-                None
-            }
+        let dirty_log_ring = if let Some(info) = &self.dirty_log_ring_info {
+            Some(KvmDirtyLogRing::mmap_from_fd(
+                &vcpu,
+                info.bytes,
+                info.acq_rel,
+            )?)
+        } else {
+            None
         };
         Ok(new_vcpu(vcpu, kvm_run_ptr, dirty_log_ring))
     }
@@ -1931,15 +1944,15 @@ impl VmFd {
     }
 
     /// Enables KVM's dirty log ring for new vCPUs created on this VM. Checks required capabilities and returns
-    /// `true` if the ring needs to be used together with a backup bitmap `KVM_GET_DIRTY_LOG`. Takes optional
-    /// dirty ring size as bytes, if not supplied, will use maximum supported dirty ring size. Enabling the dirty
-    /// log ring is only allowed before any vCPU was created on the VmFd.
+    /// a boolean `use_bitmap` as a result. `use_bitmap` is `true` if the ring needs to be used
+    /// together with a backup bitmap `KVM_GET_DIRTY_LOG`. Takes optional dirty ring size as bytes, if not supplied, will
+    /// use maximum supported dirty ring size. Enabling the dirty log ring is only allowed before any vCPU was
+    /// created on the VmFd.
     /// # Arguments
     ///
     /// * `bytes` - Size of the dirty log ring in bytes. Needs to be multiple of `std::mem::size_of::<kvm_dirty_gfn>()`
     /// and power of two.
-    #[cfg(target_arch = "x86_64")]
-    pub fn enable_dirty_log_ring(&self, bytes: Option<i32>) -> Result<bool> {
+    pub fn enable_dirty_log_ring(&mut self, bytes: Option<i32>) -> Result<bool> {
         // Check if requested size is larger than 0
         if let Some(sz) = bytes {
             if sz <= 0
@@ -1950,7 +1963,7 @@ impl VmFd {
             }
         }
 
-        let (dirty_ring_cap, max_bytes, bitmap) = {
+        let (dirty_ring_cap, max_bytes, use_bitmap) = {
             // Check if KVM_CAP_DIRTY_LOG_RING_ACQ_REL is available, enable if possible
             let acq_rel_sz = self.check_extension_raw(KVM_CAP_DIRTY_LOG_RING_ACQ_REL.into());
             if acq_rel_sz > 0 {
@@ -1987,11 +2000,12 @@ impl VmFd {
             args: [cap_ring_size as u64, 0, 0, 0],
             ..Default::default()
         };
+        let use_acq_rel = dirty_ring_cap == KVM_CAP_DIRTY_LOG_RING_ACQ_REL;
 
         // Enable the ring cap first
         self.enable_cap(&ar_ring_cap)?;
 
-        if bitmap {
+        if use_bitmap {
             let with_bitmap_cap = kvm_enable_cap {
                 cap: KVM_CAP_DIRTY_LOG_RING_WITH_BITMAP,
                 ..Default::default()
@@ -2001,7 +2015,12 @@ impl VmFd {
             self.enable_cap(&with_bitmap_cap)?;
         }
 
-        Ok(bitmap)
+        self.dirty_log_ring_info = Some(DirtyLogRingInfo {
+            bytes: cap_ring_size as usize,
+            acq_rel: use_acq_rel,
+        });
+
+        Ok(use_bitmap)
     }
 
     /// Resets all vCPU's dirty log rings. This notifies the kernel that pages have been harvested
@@ -2013,7 +2032,7 @@ impl VmFd {
     /// # extern crate kvm_ioctls;
     /// # use kvm_ioctls::{Cap, Kvm};
     /// let kvm = Kvm::new().unwrap();
-    /// let vm = kvm.create_vm().unwrap();
+    /// let mut vm = kvm.create_vm().unwrap();
     /// vm.enable_dirty_log_ring(None).unwrap();
     /// if kvm.check_extension(Cap::DirtyLogRing) {
     ///     vm.reset_dirty_rings().unwrap();
@@ -2131,7 +2150,7 @@ pub fn new_vmfd(vm: File, run_size: usize) -> VmFd {
     VmFd {
         vm,
         run_size,
-        dirty_ring_bytes: 0,
+        dirty_log_ring_info: None,
     }
 }
 
@@ -2722,7 +2741,7 @@ mod tests {
         let faulty_vm_fd = VmFd {
             vm: unsafe { File::from_raw_fd(-2) },
             run_size: 0,
-            dirty_ring_bytes: 0,
+            dirty_log_ring_info: None,
         };
 
         let invalid_mem_region = kvm_userspace_memory_region {
