virtio-devices: block: Fix writeback mode update flow

Virtio v1.2 says that if CONFIG_WCE is negotiated
but FLUSH is not, the device must initialize writeback to 0.
It also says that if CONFIG_WCE was not negotiated but FLUSH
was, the driver should assume presence of a writeback cache.

Introduce a pure is_writeback_enabled helper and a
set_writeback_mode helper. This makes the two call flows
explicit:

* write_config resolves the guest requested mode against the
  negotiated features before storing it back
* activate starts from the default writeback preference and then
  resolves it against the negotiated features
* reset restores the initial writeback state

This keeps the config space value and the runtime writeback flag
in sync and makes the spec driven fallback easier to follow.

Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>

Author: weltling

virtio-devices/src/block.rs

@@ -974,24 +974,24 @@ impl Block {
         }
     }
 
-    fn update_writeback(&mut self) {
-        // Use writeback from config if VIRTIO_BLK_F_CONFIG_WCE
-        let writeback = if self.common.feature_acked(VIRTIO_BLK_F_CONFIG_WCE.into()) {
-            self.config.writeback == 1
-        } else {
-            // Else check if VIRTIO_BLK_F_FLUSH negotiated
-            self.common.feature_acked(VIRTIO_BLK_F_FLUSH.into())
-        };
+    /// The virtio v1.2 spec says "If VIRTIO_BLK_F_CONFIG_WCE was not
+    /// negotiated but VIRTIO_BLK_F_FLUSH was, the driver SHOULD assume
+    /// presence of a writeback cache." It also says "If
+    /// VIRTIO_BLK_F_CONFIG_WCE is negotiated but VIRTIO_BLK_F_FLUSH is not,
+    /// the device MUST initialize writeback to 0."
+    fn is_writeback_enabled(&self, desired: bool) -> bool {
+        let flush = self.common.feature_acked(VIRTIO_BLK_F_FLUSH.into());
+        let wce = self.common.feature_acked(VIRTIO_BLK_F_CONFIG_WCE.into());
+        if wce { flush && desired } else { flush }
+    }
 
+    fn set_writeback_mode(&mut self, enabled: bool) {
+        self.config.writeback = enabled as u8;
+        self.writeback.store(enabled, Ordering::Release);
         info!(
             "Changing cache mode to {}",
-            if writeback {
-                "writeback"
-            } else {
-                "writethrough"
-            }
+            if enabled { "writeback" } else { "writethrough" }
         );
-        self.writeback.store(writeback, Ordering::Release);
     }
 
     pub fn resize(&mut self, new_size: u64) -> Result<()> {
@@ -1073,8 +1073,8 @@ impl VirtioDevice for Block {
             return;
         }
 
-        self.config.writeback = data[0];
-        self.update_writeback();
+        let writeback = self.is_writeback_enabled(data[0] == 1);
+        self.set_writeback_mode(writeback);
     }
 
     fn activate(&mut self, context: crate::device::ActivationContext) -> ActivateResult {
@@ -1097,7 +1097,8 @@ impl VirtioDevice for Block {
         // Recompute the barrier size from the queues that are actually activated.
         self.common.paused_sync = Some(Arc::new(Barrier::new(queues.len() + 1)));
 
-        self.update_writeback();
+        let writeback = self.is_writeback_enabled(self.config.writeback == 1);
+        self.set_writeback_mode(writeback);
 
         let mut epoll_threads = Vec::new();
         let event_idx = self.common.feature_acked(VIRTIO_RING_F_EVENT_IDX.into());
@@ -1167,6 +1168,7 @@ impl VirtioDevice for Block {
 
     fn reset(&mut self) -> Option<Arc<dyn VirtioInterrupt>> {
         let result = self.common.reset();
+        self.set_writeback_mode(true);
         event!("virtio-device", "reset", "id", &self.id);
         result
     }