From 1fc6d50f3eec2a3cbf15f62a7604ebff40e3ce81 Mon Sep 17 00:00:00 2001 From: Rob Bradford Date: Fri, 4 Dec 2020 09:23:47 +0000 Subject: [PATCH] misc: Make Bus::write() return an Option> This can be uses to indicate to the caller that it should wait on the barrier before returning as there is some asynchronous activity triggered by the write which requires the KVM exit to block until it's completed. This is useful for having vCPU thread wait for the VMM thread to proceed to activate the virtio devices. See #1863 Signed-off-by: Rob Bradford --- devices/src/acpi.rs | 5 +++-- devices/src/ioapic.rs | 5 +++-- devices/src/legacy/cmos.rs | 8 +++++--- devices/src/legacy/fwdebug.rs | 5 ++++- devices/src/legacy/i8042.rs | 8 +++++--- devices/src/legacy/rtc_pl031.rs | 6 ++++-- devices/src/legacy/serial.rs | 10 ++++++---- pci/src/bus.rs | 14 +++++++++----- pci/src/vfio.rs | 8 +++++--- virtio-devices/src/transport/pci_device.rs | 7 ++++--- vm-device/src/bus.rs | 10 +++++++--- vmm/src/cpu.rs | 3 ++- vmm/src/device_manager.rs | 8 +++++--- vmm/src/memory_manager.rs | 7 ++++--- 14 files changed, 66 insertions(+), 38 deletions(-) diff --git a/devices/src/acpi.rs b/devices/src/acpi.rs index 157ce3fb3..5a7d3c552 100644 --- a/devices/src/acpi.rs +++ b/devices/src/acpi.rs @@ -4,7 +4,7 @@ // use acpi_tables::{aml, aml::Aml}; -use std::sync::Arc; +use std::sync::{Arc, Barrier}; use std::time::Instant; use vm_device::interrupt::InterruptSourceGroup; use vm_device::BusDevice; @@ -36,7 +36,7 @@ impl BusDevice for AcpiShutdownDevice { } } - fn write(&mut self, _base: u64, _offset: u64, data: &[u8]) { + fn write(&mut self, _base: u64, _offset: u64, data: &[u8]) -> Option> { if data[0] == 1 { debug!("ACPI Reboot signalled"); if let Err(e) = self.reset_evt.write(1) { @@ -54,6 +54,7 @@ impl BusDevice for AcpiShutdownDevice { error!("Error triggering ACPI shutdown event: {}", e); } } + None } } diff --git a/devices/src/ioapic.rs b/devices/src/ioapic.rs index 283f645ed..771eab251 100644 --- a/devices/src/ioapic.rs +++ b/devices/src/ioapic.rs @@ -13,7 +13,7 @@ use super::interrupt_controller::{Error, InterruptController}; use anyhow::anyhow; use byteorder::{ByteOrder, LittleEndian}; use std::result; -use std::sync::Arc; +use std::sync::{Arc, Barrier}; use vm_device::interrupt::{ InterruptIndex, InterruptManager, InterruptSourceConfig, InterruptSourceGroup, MsiIrqGroupConfig, MsiIrqSourceConfig, @@ -167,7 +167,7 @@ impl BusDevice for Ioapic { LittleEndian::write_u32(data, value); } - fn write(&mut self, _base: u64, offset: u64, data: &[u8]) { + fn write(&mut self, _base: u64, offset: u64, data: &[u8]) -> Option> { assert!(data.len() == 4); debug!("IOAPIC_W @ offset 0x{:x}", offset); @@ -181,6 +181,7 @@ impl BusDevice for Ioapic { error!("IOAPIC: failed writing at offset {}", offset); } } + None } } diff --git a/devices/src/legacy/cmos.rs b/devices/src/legacy/cmos.rs index 6b1b5380c..3138c5a92 100644 --- a/devices/src/legacy/cmos.rs +++ b/devices/src/legacy/cmos.rs @@ -5,6 +5,7 @@ use libc::{clock_gettime, gmtime_r, time_t, timespec, tm, CLOCK_REALTIME}; use std::cmp::min; use std::mem; +use std::sync::{Arc, Barrier}; use vm_device::BusDevice; const INDEX_MASK: u8 = 0x7f; @@ -44,16 +45,17 @@ impl Cmos { } impl BusDevice for Cmos { - fn write(&mut self, _base: u64, offset: u64, data: &[u8]) { + fn write(&mut self, _base: u64, offset: u64, data: &[u8]) -> Option> { if data.len() != 1 { - return; + return None; } match offset { INDEX_OFFSET => self.index = data[0] & INDEX_MASK, DATA_OFFSET => self.data[self.index as usize] = data[0], o => panic!("bad write offset on CMOS device: {}", o), - } + }; + None } fn read(&mut self, _base: u64, offset: u64, data: &mut [u8]) { diff --git a/devices/src/legacy/fwdebug.rs b/devices/src/legacy/fwdebug.rs index c5efbbf06..e28ee7349 100644 --- a/devices/src/legacy/fwdebug.rs +++ b/devices/src/legacy/fwdebug.rs @@ -7,6 +7,7 @@ // SPDX-License-Identifier: Apache-2.0 AND BSD-3-Clause // +use std::sync::{Arc, Barrier}; use vm_device::BusDevice; /// Provides firmware debug output via I/O port controls @@ -30,11 +31,13 @@ impl BusDevice for FwDebugDevice { } } - fn write(&mut self, _base: u64, _offset: u64, data: &[u8]) { + fn write(&mut self, _base: u64, _offset: u64, data: &[u8]) -> Option> { if data.len() == 1 { print!("{}", data[0] as char); } else { error!("Invalid write size on debug port: {}", data.len()) } + + None } } diff --git a/devices/src/legacy/i8042.rs b/devices/src/legacy/i8042.rs index d6d295467..c38fe735c 100644 --- a/devices/src/legacy/i8042.rs +++ b/devices/src/legacy/i8042.rs @@ -2,9 +2,9 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE-BSD-3-Clause file. -use vmm_sys_util::eventfd::EventFd; - +use std::sync::{Arc, Barrier}; use vm_device::BusDevice; +use vmm_sys_util::eventfd::EventFd; /// A i8042 PS/2 controller that emulates just enough to shutdown the machine. pub struct I8042Device { @@ -32,12 +32,14 @@ impl BusDevice for I8042Device { } } - fn write(&mut self, _base: u64, offset: u64, data: &[u8]) { + fn write(&mut self, _base: u64, offset: u64, data: &[u8]) -> Option> { if data.len() == 1 && data[0] == 0xfe && offset == 3 { debug!("i8042 reset signalled"); if let Err(e) = self.reset_evt.write(1) { error!("Error triggering i8042 reset event: {}", e); } } + + None } } diff --git a/devices/src/legacy/rtc_pl031.rs b/devices/src/legacy/rtc_pl031.rs index fef706008..e5a88e45c 100644 --- a/devices/src/legacy/rtc_pl031.rs +++ b/devices/src/legacy/rtc_pl031.rs @@ -9,7 +9,7 @@ //! a real-time clock input. //! use std::fmt; -use std::sync::Arc; +use std::sync::{Arc, Barrier}; use std::time::Instant; use std::{io, result}; use vm_device::interrupt::InterruptSourceGroup; @@ -371,7 +371,7 @@ impl BusDevice for RTC { } } - fn write(&mut self, _base: u64, offset: u64, data: &[u8]) { + fn write(&mut self, _base: u64, offset: u64, data: &[u8]) -> Option> { if data.len() <= 4 { let v = read_le_u32(&data[..]); if let Err(e) = self.handle_write(offset, v) { @@ -384,6 +384,8 @@ impl BusDevice for RTC { data.len() ); } + + None } } diff --git a/devices/src/legacy/serial.rs b/devices/src/legacy/serial.rs index 76f10a2c9..85caaf6fa 100644 --- a/devices/src/legacy/serial.rs +++ b/devices/src/legacy/serial.rs @@ -7,7 +7,7 @@ use anyhow::anyhow; use std::collections::VecDeque; -use std::sync::Arc; +use std::sync::{Arc, Barrier}; use std::{io, result}; use vm_device::interrupt::InterruptSourceGroup; use vm_device::BusDevice; @@ -275,12 +275,14 @@ impl BusDevice for Serial { }; } - fn write(&mut self, _base: u64, offset: u64, data: &[u8]) { + fn write(&mut self, _base: u64, offset: u64, data: &[u8]) -> Option> { if data.len() != 1 { - return; + return None; } - if let Err(_e) = self.handle_write(offset as u8, data[0]) {} + self.handle_write(offset as u8, data[0]).ok(); + + None } } diff --git a/pci/src/bus.rs b/pci/src/bus.rs index 84c37fe52..459223dad 100644 --- a/pci/src/bus.rs +++ b/pci/src/bus.rs @@ -10,7 +10,7 @@ use byteorder::{ByteOrder, LittleEndian}; use std::any::Any; use std::collections::HashMap; use std::ops::DerefMut; -use std::sync::{Arc, Mutex}; +use std::sync::{Arc, Barrier, Mutex}; use vm_device::{Bus, BusDevice}; use vm_memory::{Address, GuestAddress, GuestUsize}; @@ -312,13 +312,15 @@ impl BusDevice for PciConfigIo { } } - fn write(&mut self, _base: u64, offset: u64, data: &[u8]) { + fn write(&mut self, _base: u64, offset: u64, data: &[u8]) -> Option> { // `offset` is relative to 0xcf8 match offset { o @ 0..=3 => self.set_config_address(o, data), o @ 4..=7 => self.config_space_write(o - 4, data), _ => (), }; + + None } } @@ -407,11 +409,13 @@ impl BusDevice for PciConfigMmio { } } - fn write(&mut self, _base: u64, offset: u64, data: &[u8]) { + fn write(&mut self, _base: u64, offset: u64, data: &[u8]) -> Option> { if offset > u64::from(u32::max_value()) { - return; + return None; } - self.config_space_write(offset as u32, offset % 4, data) + self.config_space_write(offset as u32, offset % 4, data); + + None } } diff --git a/pci/src/vfio.rs b/pci/src/vfio.rs index 9fcbae442..9d1a1cb01 100644 --- a/pci/src/vfio.rs +++ b/pci/src/vfio.rs @@ -15,7 +15,7 @@ use std::any::Any; use std::ops::Deref; use std::os::unix::io::AsRawFd; use std::ptr::null_mut; -use std::sync::Arc; +use std::sync::{Arc, Barrier}; use std::{fmt, io, result}; use vfio_bindings::bindings::vfio::*; use vfio_ioctls::{VfioDevice, VfioError}; @@ -652,8 +652,10 @@ impl BusDevice for VfioPciDevice { self.read_bar(base, offset, data) } - fn write(&mut self, base: u64, offset: u64, data: &[u8]) { - self.write_bar(base, offset, data) + fn write(&mut self, base: u64, offset: u64, data: &[u8]) -> Option> { + self.write_bar(base, offset, data); + + None } } diff --git a/virtio-devices/src/transport/pci_device.rs b/virtio-devices/src/transport/pci_device.rs index 3e244f55e..7a19796a9 100644 --- a/virtio-devices/src/transport/pci_device.rs +++ b/virtio-devices/src/transport/pci_device.rs @@ -31,7 +31,7 @@ use std::io::Write; use std::num::Wrapping; use std::result; use std::sync::atomic::{AtomicU16, AtomicUsize, Ordering}; -use std::sync::{Arc, Mutex}; +use std::sync::{Arc, Barrier, Mutex}; use vm_allocator::SystemAllocator; use vm_device::interrupt::{ InterruptIndex, InterruptManager, InterruptSourceGroup, MsiIrqGroupConfig, @@ -1018,8 +1018,9 @@ impl BusDevice for VirtioPciDevice { self.read_bar(base, offset, data) } - fn write(&mut self, base: u64, offset: u64, data: &[u8]) { - self.write_bar(base, offset, data) + fn write(&mut self, base: u64, offset: u64, data: &[u8]) -> Option> { + self.write_bar(base, offset, data); + None } } diff --git a/vm-device/src/bus.rs b/vm-device/src/bus.rs index 2c95637c1..381859319 100644 --- a/vm-device/src/bus.rs +++ b/vm-device/src/bus.rs @@ -9,7 +9,7 @@ use std::cmp::{Ord, Ordering, PartialEq, PartialOrd}; use std::collections::btree_map::BTreeMap; -use std::sync::{Arc, Mutex, RwLock, Weak}; +use std::sync::{Arc, Barrier, Mutex, RwLock, Weak}; use std::{convert, error, fmt, io, result}; /// Trait for devices that respond to reads or writes in an arbitrary address space. @@ -21,7 +21,9 @@ pub trait BusDevice: Send { /// Reads at `offset` from this device fn read(&mut self, base: u64, offset: u64, data: &mut [u8]) {} /// Writes at `offset` into this device - fn write(&mut self, base: u64, offset: u64, data: &[u8]) {} + fn write(&mut self, base: u64, offset: u64, data: &[u8]) -> Option> { + None + } /// Triggers the `irq_mask` interrupt on this device fn interrupt(&self, irq_mask: u32) {} } @@ -257,10 +259,12 @@ mod tests { } } - fn write(&mut self, _base: u64, offset: u64, data: &[u8]) { + fn write(&mut self, _base: u64, offset: u64, data: &[u8]) -> Option> { for (i, v) in data.iter().enumerate() { assert_eq!(*v, (offset as u8) + (i as u8)) } + + None } } diff --git a/vmm/src/cpu.rs b/vmm/src/cpu.rs index bb598d716..74cb788ef 100644 --- a/vmm/src/cpu.rs +++ b/vmm/src/cpu.rs @@ -446,7 +446,7 @@ impl BusDevice for CpuManager { } } - fn write(&mut self, _base: u64, offset: u64, data: &[u8]) { + fn write(&mut self, _base: u64, offset: u64, data: &[u8]) -> Option> { match offset { CPU_SELECTION_OFFSET => { self.selected_cpu = data[0]; @@ -478,6 +478,7 @@ impl BusDevice for CpuManager { ); } } + None } } diff --git a/vmm/src/device_manager.rs b/vmm/src/device_manager.rs index 681a9d83f..a9687fd09 100644 --- a/vmm/src/device_manager.rs +++ b/vmm/src/device_manager.rs @@ -68,7 +68,7 @@ use std::os::unix::fs::OpenOptionsExt; #[cfg(feature = "kvm")] use std::os::unix::io::FromRawFd; use std::result; -use std::sync::{Arc, Mutex}; +use std::sync::{Arc, Barrier, Mutex}; #[cfg(feature = "kvm")] use vfio_ioctls::{VfioContainer, VfioDevice, VfioDmaMapping}; use virtio_devices::transport::VirtioPciDevice; @@ -3551,7 +3551,7 @@ impl BusDevice for DeviceManager { ) } - fn write(&mut self, base: u64, offset: u64, data: &[u8]) { + fn write(&mut self, base: u64, offset: u64, data: &[u8]) -> Option> { match offset { B0EJ_FIELD_OFFSET => { assert!(data.len() == B0EJ_FIELD_SIZE); @@ -3577,7 +3577,9 @@ impl BusDevice for DeviceManager { debug!( "PCI_HP_REG_W: base 0x{:x}, offset 0x{:x}, data {:?}", base, offset, data - ) + ); + + None } } diff --git a/vmm/src/memory_manager.rs b/vmm/src/memory_manager.rs index 04de12f65..f7a640104 100644 --- a/vmm/src/memory_manager.rs +++ b/vmm/src/memory_manager.rs @@ -26,7 +26,7 @@ use std::ops::Deref; use std::os::unix::io::{AsRawFd, FromRawFd, RawFd}; use std::path::PathBuf; use std::result; -use std::sync::{Arc, Mutex}; +use std::sync::{Arc, Barrier, Mutex}; use url::Url; #[cfg(target_arch = "x86_64")] use vm_allocator::GsiApic; @@ -314,7 +314,7 @@ impl BusDevice for MemoryManager { } } - fn write(&mut self, _base: u64, offset: u64, data: &[u8]) { + fn write(&mut self, _base: u64, offset: u64, data: &[u8]) -> Option> { match offset { SELECTION_OFFSET => { self.selected_slot = usize::from(data[0]); @@ -340,7 +340,8 @@ impl BusDevice for MemoryManager { offset ); } - } + }; + None } }