From 88e0828aa301e09f1fe5c3c9203982716da55ea4 Mon Sep 17 00:00:00 2001 From: David Stevens Date: Mon, 15 Aug 2022 11:00:51 +0900 Subject: [PATCH] vm_control: skip VfioDmabufUnmap when possible Add tracking to VmMemoryRequest.execute so that only requests which were mapped with VfioDmabufMap get unmapped with VfioDmabufUnmap. This fixes a deadlock that occurs when unregistering memory before the viommu device is started, which is triggered by VfioPciDevice on ManaTEE. Note that although a deadlock with registering memory is still possible, this doesn't seem to be a problem in practice. Registration of such buffers is driven by the virtio-gpu driver, and thus tends to occur later during guest boot. BUG=b:235508487 TEST=boot brya-manatee Change-Id: I080d27b43d01a01b7d962b62c986d837be72b288 Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3830133 Tested-by: David Stevens Commit-Queue: Keiichi Watanabe Auto-Submit: David Stevens Commit-Queue: David Stevens Reviewed-by: Keiichi Watanabe --- src/crosvm/sys/unix.rs | 6 ++++- src/sys/windows.rs | 2 +- vm_control/src/lib.rs | 54 +++++++++++++++++++++++++++++------------- 3 files changed, 44 insertions(+), 18 deletions(-) diff --git a/src/crosvm/sys/unix.rs b/src/crosvm/sys/unix.rs index 6913acb1e9..8afc0a7712 100644 --- a/src/crosvm/sys/unix.rs +++ b/src/crosvm/sys/unix.rs @@ -2188,6 +2188,10 @@ fn run_control( DelayedIrqFd, } + let mut iommu_client = iommu_host_tube + .as_ref() + .map(VmMemoryRequestIommuClient::new); + stdin() .set_raw_mode() .expect("failed to set terminal raw mode"); @@ -2562,7 +2566,7 @@ fn run_control( &mut sys_allocator, Arc::clone(&map_request), &mut gralloc, - &iommu_host_tube, + &mut iommu_client, ); if let Err(e) = tube.send(&response) { error!("failed to send VmMemoryControlResponse: {}", e); diff --git a/src/sys/windows.rs b/src/sys/windows.rs index b0b463416d..38975a8f5c 100644 --- a/src/sys/windows.rs +++ b/src/sys/windows.rs @@ -1002,7 +1002,7 @@ fn run_control( &mut sys_allocator_mutex.lock(), Arc::clone(&map_request), &mut gralloc, - &None, + &mut None, ); if let Err(e) = tube.send(&response) { error!("failed to send VmMemoryControlResponse: {}", e); diff --git a/vm_control/src/lib.rs b/vm_control/src/lib.rs index ecb433927a..03a6f89327 100644 --- a/vm_control/src/lib.rs +++ b/vm_control/src/lib.rs @@ -22,6 +22,7 @@ pub mod client; pub mod display; pub mod sys; +use std::collections::BTreeSet; use std::convert::TryInto; use std::fmt; use std::fmt::Display; @@ -392,6 +393,22 @@ pub enum VmMemoryRequest { UnregisterMemory(MemSlot), } +/// Struct for managing `VmMemoryRequest`s IOMMU related state. +pub struct VmMemoryRequestIommuClient<'a> { + tube: &'a Tube, + gpu_memory: BTreeSet, +} + +impl<'a> VmMemoryRequestIommuClient<'a> { + /// Constructs `VmMemoryRequestIommuClient` from a tube for communication with the viommu. + pub fn new(tube: &'a Tube) -> Self { + Self { + tube, + gpu_memory: BTreeSet::new(), + } + } +} + impl VmMemoryRequest { /// Executes this request on the given Vm. /// @@ -408,7 +425,7 @@ impl VmMemoryRequest { sys_allocator: &mut SystemAllocator, map_request: Arc>>, gralloc: &mut RutabagaGralloc, - iommu_host_tube: &Option, + iommu_client: &mut Option, ) -> VmMemoryResponse { use self::VmMemoryRequest::*; match self { @@ -436,7 +453,7 @@ impl VmMemoryRequest { Err(e) => return VmMemoryResponse::Err(e), }; - if let (Some(descriptor), Some(iommu_tube)) = (descriptor, iommu_host_tube) { + if let (Some(descriptor), Some(iommu_client)) = (descriptor, iommu_client) { let request = VirtioIOMMURequest::VfioCommand(VirtioIOMMUVfioCommand::VfioDmabufMap { mem_slot: slot, @@ -445,7 +462,7 @@ impl VmMemoryRequest { dma_buf: descriptor, }); - match virtio_iommu_request(iommu_tube, &request) { + match virtio_iommu_request(iommu_client.tube, &request) { Ok(VirtioIOMMUResponse::VfioResponse(VirtioIOMMUVfioResult::Ok)) => (), resp => { error!("Unexpected message response: {:?}", resp); @@ -453,7 +470,9 @@ impl VmMemoryRequest { let _ = vm.remove_memory_region(slot); return VmMemoryResponse::Err(SysError::new(EINVAL)); } - } + }; + + iommu_client.gpu_memory.insert(slot); } let pfn = guest_addr.0 >> 12; @@ -461,20 +480,23 @@ impl VmMemoryRequest { } UnregisterMemory(slot) => match vm.remove_memory_region(slot) { Ok(_) => { - if let Some(iommu_tube) = iommu_host_tube { - let request = VirtioIOMMURequest::VfioCommand( - VirtioIOMMUVfioCommand::VfioDmabufUnmap(slot), - ); + if let Some(iommu_client) = iommu_client { + if iommu_client.gpu_memory.remove(&slot) { + let request = VirtioIOMMURequest::VfioCommand( + VirtioIOMMUVfioCommand::VfioDmabufUnmap(slot), + ); - match virtio_iommu_request(iommu_tube, &request) { - Ok(VirtioIOMMUResponse::VfioResponse(VirtioIOMMUVfioResult::Ok)) - | Ok(VirtioIOMMUResponse::VfioResponse( - VirtioIOMMUVfioResult::NoSuchMappedDmabuf, - )) => VmMemoryResponse::Ok, - resp => { - error!("Unexpected message response: {:?}", resp); - return VmMemoryResponse::Err(SysError::new(EINVAL)); + match virtio_iommu_request(iommu_client.tube, &request) { + Ok(VirtioIOMMUResponse::VfioResponse( + VirtioIOMMUVfioResult::Ok, + )) => VmMemoryResponse::Ok, + resp => { + error!("Unexpected message response: {:?}", resp); + return VmMemoryResponse::Err(SysError::new(EINVAL)); + } } + } else { + VmMemoryResponse::Ok } } else { VmMemoryResponse::Ok