From 5d98851cc58826ceb0f342a7870a8cb321bf232b Mon Sep 17 00:00:00 2001
From: Hanna Czenczek <hreitz@redhat.com>
Date: Fri, 1 Mar 2024 15:25:37 +0100
Subject: [PATCH] Disable bitmaps unless F_LOG_ALL
Content-Type: text/plain; charset=UTF-8

---
 vhost-user-backend/src/bitmap.rs  | 20 ++++++++++++++++++--
 vhost-user-backend/src/handler.rs | 19 ++++++++++++++++++-
 2 files changed, 36 insertions(+), 3 deletions(-)

diff --git a/vhost-user-backend/src/bitmap.rs b/vhost-user-backend/src/bitmap.rs
index a9864b1..6d681f7 100644
--- a/vhost-user-backend/src/bitmap.rs
+++ b/vhost-user-backend/src/bitmap.rs
@@ -23,6 +23,10 @@ pub trait BitmapReplace: Bitmap {
 
     /// Replace the internal `Bitmap`
     fn replace(&self, bitmap: Self::InnerBitmap);
+
+    fn clone_inner(&self) -> Option<Self::InnerBitmap> {
+        None
+    }
 }
 
 /// A bitmap relative to a memory region
@@ -30,6 +34,8 @@ pub trait MemRegionBitmap: Sized {
     /// Creates a new bitmap relative to `region`, using the `logmem` as
     /// backing memory for the bitmap
     fn new<R: GuestMemoryRegion>(region: &R, logmem: Arc<MmapLogReg>) -> io::Result<Self>;
+
+    fn set_disabled(&mut self, _disabled: bool) {}
 }
 
 // TODO: This impl is a quick and dirty hack to allow the tests to continue using
@@ -102,6 +108,10 @@ impl BitmapReplace for BitmapMmapRegion {
         let mut inner = self.inner.write().unwrap();
         inner.replace(bitmap);
     }
+
+    fn clone_inner(&self) -> Option<AtomicBitmapMmap> {
+        self.inner.read().unwrap().as_ref().cloned()
+    }
 }
 
 impl BitmapSlice for BitmapMmapRegion {}
@@ -121,11 +131,12 @@ impl NewBitmap for BitmapMmapRegion {
 /// (see `VHOST_USER_PROTOCOL_F_LOG_SHMFD` and `VHOST_USER_SET_LOG_BASE` in the vhost-user protocol
 /// specification). It uses a fixed memory page size of `LOG_PAGE_SIZE` bytes, so it converts
 /// addresses to page numbers before setting or clearing the bits.
-#[derive(Debug)]
+#[derive(Debug, Clone)]
 pub struct AtomicBitmapMmap {
     logmem: Arc<MmapLogReg>,
     pages_before_region: usize, // Number of pages to ignore from the start of the bitmap
     number_of_pages: usize,     // Number of total pages indexed in the bitmap for this region
+    disabled: bool,
 }
 
 // `AtomicBitmapMmap` implements a simple bitmap, it is page-size aware and relative
@@ -164,8 +175,13 @@ impl MemRegionBitmap for AtomicBitmapMmap {
             logmem,
             pages_before_region: offset_pages,
             number_of_pages: size_page,
+            disabled: false,
         })
     }
+
+    fn set_disabled(&mut self, disabled: bool) {
+        self.disabled = disabled;
+    }
 }
 
 impl AtomicBitmapMmap {
@@ -173,7 +189,7 @@ impl AtomicBitmapMmap {
     // so an offset of `0` references the start of the memory region. Any attempt to
     // access beyond the end of the bitmap are simply ignored.
     fn mark_dirty(&self, offset: usize, len: usize) {
-        if len == 0 {
+        if len == 0 || self.disabled {
             return;
         }
 
diff --git a/vhost-user-backend/src/handler.rs b/vhost-user-backend/src/handler.rs
index fa2ee4c..f4be65c 100644
--- a/vhost-user-backend/src/handler.rs
+++ b/vhost-user-backend/src/handler.rs
@@ -262,6 +262,8 @@ where
             return Err(VhostUserError::InvalidParam);
         }
 
+        let logging_toggled =
+            (self.acked_features ^ features) & VhostUserVirtioFeatures::LOG_ALL.bits() != 0;
         self.acked_features = features;
         self.features_acked = true;
 
@@ -279,6 +281,17 @@ where
             }
         }
 
+        if logging_toggled {
+            let logging_enabled = features & VhostUserVirtioFeatures::LOG_ALL.bits() != 0;
+            let mem = self.atomic_mem.memory();
+            for region in mem.iter() {
+                if let Some(mut bmap) = region.bitmap().clone_inner() {
+                    bmap.set_disabled(!logging_enabled);
+                    region.bitmap().replace(bmap);
+                }
+            }
+        }
+
         self.backend.acked_features(self.acked_features);
 
         Ok(())
@@ -739,12 +752,16 @@ where
         // Let's create all bitmaps first before replacing them, in case any of them fails
         let mut bitmaps = Vec::new();
         for region in mem.iter() {
-            let bitmap = <<T as VhostUserBackend>::Bitmap as BitmapReplace>::InnerBitmap::new(
+            let mut bitmap = <<T as VhostUserBackend>::Bitmap as BitmapReplace>::InnerBitmap::new(
                 region,
                 Arc::clone(&logmem),
             )
             .map_err(VhostUserError::ReqHandlerError)?;
 
+            if self.acked_features & VhostUserVirtioFeatures::LOG_ALL.bits() == 0 {
+                bitmap.set_disabled(true);
+            }
+
             bitmaps.push((region, bitmap));
         }
 
-- 
2.43.0