Skip to content

Commit d6506e6

Browse files
committed
vmm: fix concurrent access issue of vfio_container
The vfio_container can be accessed both in mmio_write() -> cleanup_vfio_container() and add_device(), which are two threads. It's safe to add a lock to ensure consistency. Fixes: cloud-hypervisor#7364 Signed-off-by: Yi Wang <foxywang@tencent.com>
1 parent d59dfdf commit d6506e6

1 file changed

Lines changed: 27 additions & 19 deletions

File tree

vmm/src/device_manager.rs

Lines changed: 27 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1006,7 +1006,7 @@ pub struct DeviceManager {
10061006
// VFIO container
10071007
// Only one container can be created, therefore it is stored as part of the
10081008
// DeviceManager to be reused.
1009-
vfio_container: Option<Arc<VfioContainer>>,
1009+
vfio_container: Arc<Mutex<Option<Arc<VfioContainer>>>>,
10101010

10111011
// Paravirtualized IOMMU
10121012
iommu_device: Option<Arc<Mutex<virtio_devices::Iommu>>>,
@@ -1320,7 +1320,7 @@ impl DeviceManager {
13201320
msi_interrupt_manager,
13211321
legacy_interrupt_manager: None,
13221322
passthrough_device: None,
1323-
vfio_container: None,
1323+
vfio_container: Arc::new(Mutex::new(None)),
13241324
iommu_device: None,
13251325
iommu_mapping: None,
13261326
iommu_attached_devices: None,
@@ -3724,14 +3724,16 @@ impl DeviceManager {
37243724
}
37253725

37263726
vfio_container
3727-
} else if let Some(vfio_container) = &self.vfio_container {
3728-
Arc::clone(vfio_container)
37293727
} else {
3730-
let vfio_container = self.create_vfio_container()?;
3731-
needs_dma_mapping = true;
3732-
self.vfio_container = Some(Arc::clone(&vfio_container));
3733-
3734-
vfio_container
3728+
let mut vfio_container_guard = self.vfio_container.lock().unwrap();
3729+
if let Some(vfio_container) = vfio_container_guard.as_ref() {
3730+
Arc::clone(vfio_container)
3731+
} else {
3732+
let vfio_container = self.create_vfio_container()?;
3733+
needs_dma_mapping = true;
3734+
*vfio_container_guard = Some(Arc::clone(&vfio_container));
3735+
vfio_container
3736+
}
37353737
};
37363738

37373739
let vfio_device = VfioDevice::new(&device_cfg.path, Arc::clone(&vfio_container))
@@ -4356,14 +4358,17 @@ impl DeviceManager {
43564358
}
43574359

43584360
// Take care of updating the memory for VFIO PCI devices.
4359-
if let Some(vfio_container) = &self.vfio_container {
4360-
vfio_container
4361-
.vfio_dma_map(
4362-
new_region.start_addr().raw_value(),
4363-
new_region.len(),
4364-
new_region.as_ptr() as u64,
4365-
)
4366-
.map_err(DeviceManagerError::UpdateMemoryForVfioPciDevice)?;
4361+
{
4362+
let vfio_container_guard = self.vfio_container.lock().unwrap();
4363+
if let Some(vfio_container) = vfio_container_guard.as_ref() {
4364+
vfio_container
4365+
.vfio_dma_map(
4366+
new_region.start_addr().raw_value(),
4367+
new_region.len(),
4368+
new_region.as_ptr() as u64,
4369+
)
4370+
.map_err(DeviceManagerError::UpdateMemoryForVfioPciDevice)?;
4371+
}
43674372
}
43684373

43694374
// Take care of updating the memory for vfio-user devices.
@@ -4962,9 +4967,12 @@ impl DeviceManager {
49624967

49634968
fn cleanup_vfio_container(&mut self) {
49644969
// Drop the 'vfio container' instance when "Self" is the only reference
4965-
if let Some(1) = self.vfio_container.as_ref().map(Arc::strong_count) {
4970+
let mut vfio_container_guard = self.vfio_container.lock().unwrap();
4971+
if let Some(container) = vfio_container_guard.as_ref()
4972+
&& Arc::strong_count(container) == 1
4973+
{
49664974
debug!("Drop 'vfio container' given no active 'vfio devices'.");
4967-
self.vfio_container = None;
4975+
*vfio_container_guard = None;
49684976
}
49694977
}
49704978
}

0 commit comments

Comments
 (0)