Merge branch 'main' into fix/bar-rollback-defensive

Author: CMGS

cloud-hypervisor/tests/common/utils.rs

@@ -255,41 +255,6 @@ pub(crate) fn prepare_swtpm_daemon(tmp_dir: &TempDir) -> (std::process::Command,
     (swtpm_command, swtpm_socket_path)
 }
 
-pub(crate) fn remote_command(api_socket: &str, command: &str, arg: Option<&str>) -> bool {
-    let mut cmd = Command::new(clh_command("ch-remote"));
-    cmd.args([&format!("--api-socket={api_socket}"), command]);
-
-    if let Some(arg) = arg {
-        cmd.arg(arg);
-    }
-    let output = cmd.output().unwrap();
-    if output.status.success() {
-        true
-    } else {
-        eprintln!("Error running ch-remote command: {:?}", &cmd);
-        let stderr = String::from_utf8_lossy(&output.stderr);
-        eprintln!("stderr: {stderr}");
-        false
-    }
-}
-
-pub(crate) fn remote_command_w_output(
-    api_socket: &str,
-    command: &str,
-    arg: Option<&str>,
-) -> (bool, Vec<u8>) {
-    let mut cmd = Command::new(clh_command("ch-remote"));
-    cmd.args([&format!("--api-socket={api_socket}"), command]);
-
-    if let Some(arg) = arg {
-        cmd.arg(arg);
-    }
-
-    let output = cmd.output().expect("Failed to launch ch-remote");
-
-    (output.status.success(), output.stdout)
-}
-
 pub(crate) fn resize_command(
     api_socket: &str,
     desired_vcpus: Option<u8>,

devices/src/ivshmem.rs

@@ -217,7 +217,7 @@ impl BusDevice for IvshmemDevice {
 impl PciDevice for IvshmemDevice {
     fn allocate_bars(
         &mut self,
-        _allocator: &Arc<Mutex<SystemAllocator>>,
+        _allocator: &mut SystemAllocator,
         mmio32_allocator: &mut AddressAllocator,
         mmio64_allocator: &mut AddressAllocator,
         resources: Option<Vec<Resource>>,

devices/src/pvmemcontrol.rs

@@ -5,7 +5,7 @@
 
 use std::collections::HashMap;
 use std::ffi::CString;
-use std::sync::{Arc, Barrier, Mutex, RwLock};
+use std::sync::{Arc, Barrier, RwLock};
 use std::{io, result};
 
 use log::{debug, warn};
@@ -722,7 +722,7 @@ impl PciDevice for PvmemcontrolPciDevice {
 
     fn allocate_bars(
         &mut self,
-        _allocator: &Arc<Mutex<SystemAllocator>>,
+        _allocator: &mut SystemAllocator,
         mmio32_allocator: &mut AddressAllocator,
         _mmio64_allocator: &mut AddressAllocator,
         resources: Option<Vec<Resource>>,

devices/src/pvpanic.rs

@@ -5,7 +5,7 @@
 
 use std::any::Any;
 use std::result;
-use std::sync::{Arc, Barrier, Mutex};
+use std::sync::{Arc, Barrier};
 
 use anyhow::anyhow;
 use event_monitor::event;
@@ -174,7 +174,7 @@ impl PciDevice for PvPanicDevice {
 
     fn allocate_bars(
         &mut self,
-        _allocator: &Arc<Mutex<SystemAllocator>>,
+        _allocator: &mut SystemAllocator,
         mmio32_allocator: &mut AddressAllocator,
         _mmio64_allocator: &mut AddressAllocator,
         resources: Option<Vec<Resource>>,

pci/src/device.rs

@@ -5,7 +5,7 @@
 // SPDX-License-Identifier: Apache-2.0 AND BSD-3-Clause
 
 use std::any::Any;
-use std::sync::{Arc, Barrier, Mutex};
+use std::sync::{Arc, Barrier};
 use std::{io, result};
 
 use serde::{Deserialize, Serialize};
@@ -49,7 +49,7 @@ pub trait PciDevice: Send {
     /// returns an address. Returns a Vec of (GuestAddress, GuestUsize) tuples.
     fn allocate_bars(
         &mut self,
-        _allocator: &Arc<Mutex<SystemAllocator>>,
+        _allocator: &mut SystemAllocator,
         _mmio32_allocator: &mut AddressAllocator,
         _mmio64_allocator: &mut AddressAllocator,
         _resources: Option<Vec<Resource>>,

pci/src/vfio.rs

@@ -602,7 +602,7 @@ impl VfioCommon {
     #[allow(unused_variables)]
     pub(crate) fn allocate_bars(
         &mut self,
-        allocator: &Arc<Mutex<SystemAllocator>>,
+        allocator: &mut SystemAllocator,
         mmio32_allocator: &mut AddressAllocator,
         mmio64_allocator: &mut AddressAllocator,
         resources: Option<&[Resource]>,
@@ -741,8 +741,6 @@ impl VfioCommon {
                 PciBarRegionType::IoRegion => {
                     // The address needs to be 4 bytes aligned.
                     allocator
-                        .lock()
-                        .unwrap()
                         .allocate_io_addresses(restored_bar_addr, region_size, Some(0x4))
                         .ok_or(PciDeviceError::IoAllocationFailed(region_size))?
                 }
@@ -1852,7 +1850,7 @@ const PCI_ROM_EXP_BAR_INDEX: usize = 12;
 impl PciDevice for VfioPciDevice {
     fn allocate_bars(
         &mut self,
-        allocator: &Arc<Mutex<SystemAllocator>>,
+        allocator: &mut SystemAllocator,
         mmio32_allocator: &mut AddressAllocator,
         mmio64_allocator: &mut AddressAllocator,
         resources: Option<Vec<Resource>>,

pci/src/vfio_user.rs

@@ -391,7 +391,7 @@ impl Vfio for VfioUserClientWrapper {
 impl PciDevice for VfioUserPciDevice {
     fn allocate_bars(
         &mut self,
-        allocator: &Arc<Mutex<SystemAllocator>>,
+        allocator: &mut SystemAllocator,
         mmio32_allocator: &mut AddressAllocator,
         mmio64_allocator: &mut AddressAllocator,
         resources: Option<Vec<Resource>>,

performance-metrics/src/performance_tests.rs

@@ -5,7 +5,6 @@
 
 // Performance tests
 
-use std::path::PathBuf;
 use std::time::Duration;
 use std::{fs, thread};
 
@@ -14,8 +13,6 @@ use thiserror::Error;
 
 use crate::{ImageFormat, PerformanceTestControl, PerformanceTestOverrides, mean};
 
-#[cfg(target_arch = "x86_64")]
-pub const FOCAL_IMAGE_NAME: &str = "focal-server-cloudimg-amd64-custom-20210609-0.raw";
 #[cfg(target_arch = "aarch64")]
 pub const FOCAL_IMAGE_NAME: &str = "focal-server-cloudimg-arm64-custom-20210929-0-update-tool.raw";
 
@@ -127,45 +124,6 @@ fn performance_test_new_guest(disk_config: Box<dyn DiskConfig>) -> Guest {
     Guest::new_from_ip_range(disk_config, "172.19", 0)
 }
 
-const DIRECT_KERNEL_BOOT_CMDLINE: &str =
-    "root=/dev/vda1 console=hvc0 rw systemd.journald.forward_to_console=1";
-
-// Creates the path for direct kernel boot and return the path.
-// For x86_64, this function returns the vmlinux kernel path.
-// For AArch64, this function returns the PE kernel path.
-fn direct_kernel_boot_path() -> PathBuf {
-    let mut workload_path = dirs::home_dir().unwrap();
-    workload_path.push("workloads");
-
-    let mut kernel_path = workload_path;
-    #[cfg(target_arch = "x86_64")]
-    kernel_path.push("vmlinux-x86_64");
-    #[cfg(target_arch = "aarch64")]
-    kernel_path.push("Image-arm64");
-
-    kernel_path
-}
-
-fn remote_command(api_socket: &str, command: &str, arg: Option<&str>) -> bool {
-    let mut cmd = std::process::Command::new(clh_command("ch-remote"));
-    cmd.args([&format!("--api-socket={api_socket}"), command]);
-
-    if let Some(arg) = arg {
-        cmd.arg(arg);
-    }
-    let output = cmd.output().unwrap();
-    if output.status.success() {
-        true
-    } else {
-        eprintln!(
-            "Error running ch-remote command: {:?}\nstderr: {}",
-            &cmd,
-            String::from_utf8_lossy(&output.stderr)
-        );
-        false
-    }
-}
-
 pub fn performance_net_throughput(control: &PerformanceTestControl) -> f64 {
     let test_timeout = control.test_timeout;
     let (rx, bandwidth) = control.net_control.unwrap();

test_infra/src/lib.rs

@@ -1776,6 +1776,42 @@ pub fn clh_command(cmd: &str) -> String {
     String::from(full_path.to_str().unwrap())
 }
 
+pub fn remote_command(api_socket: &str, command: &str, arg: Option<&str>) -> bool {
+    let mut cmd = Command::new(clh_command("ch-remote"));
+    cmd.args([&format!("--api-socket={api_socket}"), command]);
+
+    if let Some(arg) = arg {
+        cmd.arg(arg);
+    }
+
+    let output = cmd.output().unwrap();
+    if output.status.success() {
+        true
+    } else {
+        eprintln!("Error running ch-remote command: {:?}", &cmd);
+        let stderr = String::from_utf8_lossy(&output.stderr);
+        eprintln!("stderr: {stderr}");
+        false
+    }
+}
+
+pub fn remote_command_w_output(
+    api_socket: &str,
+    command: &str,
+    arg: Option<&str>,
+) -> (bool, Vec<u8>) {
+    let mut cmd = Command::new(clh_command("ch-remote"));
+    cmd.args([&format!("--api-socket={api_socket}"), command]);
+
+    if let Some(arg) = arg {
+        cmd.arg(arg);
+    }
+
+    let output = cmd.output().expect("Failed to launch ch-remote");
+
+    (output.status.success(), output.stdout)
+}
+
 pub fn parse_iperf3_output(output: &[u8], sender: bool, bandwidth: bool) -> Result<f64, Error> {
     std::panic::catch_unwind(|| {
         let s = String::from_utf8_lossy(output);

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
     }