vmm: Break the cyclic dependency between DeviceManager and IO bus

By inserting the DeviceManager on the IO bus, we introduced some cyclic
dependency:

  DeviceManager ---> AddressManager ---> Bus ---> BusDevice
        ^                                             |
        |                                             |
        +---------------------------------------------+

This cycle needs to be broken by inserting a Weak reference instead of
an Arc (considered as a strong reference).

Signed-off-by: Sebastien Boeuf <sebastien.boeuf@intel.com>
This commit is contained in:
Sebastien Boeuf 2020-02-28 18:11:34 +01:00 committed by Rob Bradford
parent c1af13efeb
commit d47f733e51
3 changed files with 34 additions and 12 deletions

View File

@ -507,7 +507,7 @@ impl CpuManager {
let cpu_manager = Arc::new(Mutex::new(CpuManager { let cpu_manager = Arc::new(Mutex::new(CpuManager {
boot_vcpus, boot_vcpus,
max_vcpus, max_vcpus,
io_bus: Arc::downgrade(&device_manager.io_bus().clone()), io_bus: Arc::downgrade(&device_manager.io_bus()),
mmio_bus: device_manager.mmio_bus().clone(), mmio_bus: device_manager.mmio_bus().clone(),
ioapic: device_manager.ioapic().clone(), ioapic: device_manager.ioapic().clone(),
vm_memory: guest_memory, vm_memory: guest_memory,

View File

@ -39,7 +39,6 @@ use std::io::{self, sink, stdout};
use std::os::unix::fs::OpenOptionsExt; use std::os::unix::fs::OpenOptionsExt;
use std::path::PathBuf; use std::path::PathBuf;
use std::result; use std::result;
#[cfg(feature = "pci_support")]
use std::sync::Weak; use std::sync::Weak;
use std::sync::{Arc, Mutex}; use std::sync::{Arc, Mutex};
use tempfile::NamedTempFile; use tempfile::NamedTempFile;
@ -278,7 +277,7 @@ impl Console {
struct AddressManager { struct AddressManager {
allocator: Arc<Mutex<SystemAllocator>>, allocator: Arc<Mutex<SystemAllocator>>,
io_bus: Arc<devices::Bus>, io_bus: Weak<devices::Bus>,
mmio_bus: Arc<devices::Bus>, mmio_bus: Arc<devices::Bus>,
vm_fd: Arc<VmFd>, vm_fd: Arc<VmFd>,
} }
@ -311,6 +310,8 @@ impl DeviceRelocation for AddressManager {
// Update PIO bus // Update PIO bus
self.io_bus self.io_bus
.upgrade()
.unwrap()
.update_range(old_base, len, new_base, len) .update_range(old_base, len, new_base, len)
.map_err(|e| io::Error::new(io::ErrorKind::Other, e))?; .map_err(|e| io::Error::new(io::ErrorKind::Other, e))?;
} }
@ -461,6 +462,7 @@ pub struct DeviceManager {
} }
impl DeviceManager { impl DeviceManager {
#[allow(clippy::too_many_arguments)]
pub fn new( pub fn new(
vm_fd: Arc<VmFd>, vm_fd: Arc<VmFd>,
config: Arc<Mutex<VmConfig>>, config: Arc<Mutex<VmConfig>>,
@ -469,10 +471,8 @@ impl DeviceManager {
_exit_evt: &EventFd, _exit_evt: &EventFd,
reset_evt: &EventFd, reset_evt: &EventFd,
vmm_path: PathBuf, vmm_path: PathBuf,
io_bus: &Arc<devices::Bus>,
) -> DeviceManagerResult<Arc<Mutex<Self>>> { ) -> DeviceManagerResult<Arc<Mutex<Self>>> {
let io_bus = devices::Bus::new();
let mmio_bus = devices::Bus::new();
let mut virtio_devices: Vec<(Arc<Mutex<dyn vm_virtio::VirtioDevice>>, bool)> = Vec::new(); let mut virtio_devices: Vec<(Arc<Mutex<dyn vm_virtio::VirtioDevice>>, bool)> = Vec::new();
let migratable_devices: Vec<Arc<Mutex<dyn Migratable>>> = Vec::new(); let migratable_devices: Vec<Arc<Mutex<dyn Migratable>>> = Vec::new();
let mut _mmap_regions = Vec::new(); let mut _mmap_regions = Vec::new();
@ -482,8 +482,8 @@ impl DeviceManager {
let address_manager = Arc::new(AddressManager { let address_manager = Arc::new(AddressManager {
allocator, allocator,
io_bus: Arc::new(io_bus), io_bus: Arc::downgrade(io_bus),
mmio_bus: Arc::new(mmio_bus), mmio_bus: Arc::new(devices::Bus::new()),
vm_fd: vm_fd.clone(), vm_fd: vm_fd.clone(),
}); });
@ -527,6 +527,8 @@ impl DeviceManager {
#[cfg(feature = "acpi")] #[cfg(feature = "acpi")]
address_manager address_manager
.io_bus .io_bus
.upgrade()
.unwrap()
.insert(memory_manager.clone(), 0xa00, 0x18) .insert(memory_manager.clone(), 0xa00, 0x18)
.map_err(DeviceManagerError::BusError)?; .map_err(DeviceManagerError::BusError)?;
@ -597,6 +599,8 @@ impl DeviceManager {
#[cfg(feature = "acpi")] #[cfg(feature = "acpi")]
address_manager address_manager
.io_bus .io_bus
.upgrade()
.unwrap()
.insert( .insert(
Arc::clone(&device_manager) as Arc<Mutex<dyn BusDevice>>, Arc::clone(&device_manager) as Arc<Mutex<dyn BusDevice>>,
0xae00, 0xae00,
@ -670,6 +674,8 @@ impl DeviceManager {
let pci_config_io = Arc::new(Mutex::new(PciConfigIo::new(Arc::clone(&pci_bus)))); let pci_config_io = Arc::new(Mutex::new(PciConfigIo::new(Arc::clone(&pci_bus))));
self.address_manager self.address_manager
.io_bus .io_bus
.upgrade()
.unwrap()
.insert(pci_config_io, 0xcf8, 0x8) .insert(pci_config_io, 0xcf8, 0x8)
.map_err(DeviceManagerError::BusError)?; .map_err(DeviceManagerError::BusError)?;
let pci_config_mmio = Arc::new(Mutex::new(PciConfigMmio::new(Arc::clone(&pci_bus)))); let pci_config_mmio = Arc::new(Mutex::new(PciConfigMmio::new(Arc::clone(&pci_bus))));
@ -752,6 +758,8 @@ impl DeviceManager {
self.address_manager self.address_manager
.io_bus .io_bus
.upgrade()
.unwrap()
.insert(acpi_device, 0x3c0, 0x4) .insert(acpi_device, 0x3c0, 0x4)
.map_err(DeviceManagerError::BusError)?; .map_err(DeviceManagerError::BusError)?;
@ -783,6 +791,8 @@ impl DeviceManager {
self.address_manager self.address_manager
.io_bus .io_bus
.upgrade()
.unwrap()
.insert(ged_device.clone(), 0xb000, 0x1) .insert(ged_device.clone(), 0xb000, 0x1)
.map_err(DeviceManagerError::BusError)?; .map_err(DeviceManagerError::BusError)?;
Ok(Some(ged_device)) Ok(Some(ged_device))
@ -794,6 +804,8 @@ impl DeviceManager {
self.address_manager self.address_manager
.io_bus .io_bus
.upgrade()
.unwrap()
.insert(i8042, 0x61, 0x4) .insert(i8042, 0x61, 0x4)
.map_err(DeviceManagerError::BusError)?; .map_err(DeviceManagerError::BusError)?;
#[cfg(feature = "cmos")] #[cfg(feature = "cmos")]
@ -819,6 +831,8 @@ impl DeviceManager {
self.address_manager self.address_manager
.io_bus .io_bus
.upgrade()
.unwrap()
.insert(cmos, 0x70, 0x2) .insert(cmos, 0x70, 0x2)
.map_err(DeviceManagerError::BusError)?; .map_err(DeviceManagerError::BusError)?;
} }
@ -864,6 +878,8 @@ impl DeviceManager {
self.address_manager self.address_manager
.io_bus .io_bus
.upgrade()
.unwrap()
.insert(serial.clone(), 0x3f8, 0x8) .insert(serial.clone(), 0x3f8, 0x8)
.map_err(DeviceManagerError::BusError)?; .map_err(DeviceManagerError::BusError)?;
@ -1429,7 +1445,7 @@ impl DeviceManager {
pci.register_mapping( pci.register_mapping(
vfio_pci_device, vfio_pci_device,
self.address_manager.io_bus.as_ref(), self.address_manager.io_bus.upgrade().unwrap().as_ref(),
self.address_manager.mmio_bus.as_ref(), self.address_manager.mmio_bus.as_ref(),
bars, bars,
) )
@ -1534,7 +1550,7 @@ impl DeviceManager {
pci.register_mapping( pci.register_mapping(
virtio_pci_device.clone(), virtio_pci_device.clone(),
self.address_manager.io_bus.as_ref(), self.address_manager.io_bus.upgrade().unwrap().as_ref(),
self.address_manager.mmio_bus.as_ref(), self.address_manager.mmio_bus.as_ref(),
bars, bars,
) )
@ -1606,8 +1622,8 @@ impl DeviceManager {
Ok(()) Ok(())
} }
pub fn io_bus(&self) -> &Arc<devices::Bus> { pub fn io_bus(&self) -> Arc<devices::Bus> {
&self.address_manager.io_bus Arc::clone(&self.address_manager.io_bus.upgrade().unwrap())
} }
pub fn mmio_bus(&self) -> &Arc<devices::Bus> { pub fn mmio_bus(&self) -> &Arc<devices::Bus> {

View File

@ -222,6 +222,8 @@ pub struct Vm {
state: RwLock<VmState>, state: RwLock<VmState>,
cpu_manager: Arc<Mutex<cpu::CpuManager>>, cpu_manager: Arc<Mutex<cpu::CpuManager>>,
memory_manager: Arc<Mutex<MemoryManager>>, memory_manager: Arc<Mutex<MemoryManager>>,
// Hold the strong reference onto the IO bus.
_io_bus: Arc<devices::Bus>,
} }
impl Vm { impl Vm {
@ -329,6 +331,8 @@ impl Vm {
.ok_or(Error::CreateSystemAllocator)?, .ok_or(Error::CreateSystemAllocator)?,
)); ));
let io_bus = Arc::new(devices::Bus::new());
let memory_config = config.lock().unwrap().memory.clone(); let memory_config = config.lock().unwrap().memory.clone();
let memory_manager = MemoryManager::new( let memory_manager = MemoryManager::new(
@ -351,6 +355,7 @@ impl Vm {
&exit_evt, &exit_evt,
&reset_evt, &reset_evt,
vmm_path, vmm_path,
&io_bus,
) )
.map_err(Error::DeviceManager)?; .map_err(Error::DeviceManager)?;
@ -379,6 +384,7 @@ impl Vm {
state: RwLock::new(VmState::Created), state: RwLock::new(VmState::Created),
cpu_manager, cpu_manager,
memory_manager, memory_manager,
_io_bus: io_bus,
}) })
} }